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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 6 Mar 2019 18:32:54 +0100
From:   Auger Eric <eric.auger@...hat.com>
To:     Jean-Philippe Brucker <jean-philippe.brucker@....com>,
        eric.auger.pro@...il.com, iommu@...ts.linux-foundation.org,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        kvmarm@...ts.cs.columbia.edu, joro@...tes.org,
        alex.williamson@...hat.com, jacob.jun.pan@...ux.intel.com,
        yi.l.liu@...ux.intel.com, will.deacon@....com, robin.murphy@....com
Cc:     marc.zyngier@....com, peter.maydell@...aro.org,
        kevin.tian@...el.com, ashok.raj@...el.com, christoffer.dall@....com
Subject: Re: [PATCH v4 02/22] iommu: introduce device fault data

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
>