Re: [PATCH v4 02/22] iommu: introduce device fault data

[Auger Eric]

Hi Jean,
On 3/6/19 5:07 PM, Jean-Philippe Brucker wrote:
> On 06/03/2019 14:30, Auger Eric wrote:
>>>> +#define IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE	(1 << 1)
>>>> +#define IOMMU_FAULT_PAGE_REQUEST_PRIV_DATA	(1 << 2)
>>>> +	__u32   flags;
>>>> +	__u32	pasid;
>>>> +	__u32	grpid;
>>>> +	__u32	perm;
>>>> +	__u64	addr;
>>>
>>> Given that we'll be reporting stall faults using this struct, it would
>>> be good to have the fetch_addr field and flag here as well.
>> As the stall model looks really ARM specific shouldn't we introduce a
>> dedicated struct and iommu_fault_type enum value?
> 
> There is no reason for the generic page fault handler to differentiate
> between stall and PRI, they are page requests. For a stall we write STAG
> into grpid and set LAST_PAGE=1. Then the SMMU driver writes the page
> response either as a PRI_RESP or a RESUME depending on the device type.

OK. After reading the spec I thought STALL faults could have a larger
scope than just PRI.
> 
>> Also for stall faults don't we need to expose the stall tag (STAG) that,
>> as far as I understand is going to be used by the guest we it wants to
>> retry or terminate the faulted transaction. In practice doesn't the
>> stall fault have the same fields of the unrecoverable fault + STAG? I am
>> afraid adding the fetch_addr in the page request struct may "pollute"
>> the PRI struct that can be understood by both aarch64 and x86 parties atm.
> 
> Let's leave out the fetch_addr field then, I was suggesting it for
> completeness but I don't need it immediately, at least not for host SVA.
> For dual-stage SVA (where both stage-1 and stage-2 are shared with the
> CPU) we'll need the IPA field, but that's still a long way away.
OK So I don't add fetch_addr at the moment.
> 
>> Also couldn't we envision to put this STALL struct in a new revision of
>> the fault ABI.
> As said above, generic code doesn't have to know the difference until we
> start implementing nested SVA. Also, we need stall support in the fault
> handler soon, since there is hardware supporting it.

OK so we will use page request structs.

Thanks

Eric
> 
> Thanks,
> Jean
>