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. > 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. > 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. Thanks, Jean
