linux-kernel - Re: [PATCH v3 08/16] 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:   Thu, 11 Jan 2018 13:10:55 -0800
From:   Jacob Pan <jacob.jun.pan@...ux.intel.com>
To:     Jean-Philippe Brucker <jean-philippe.brucker@....com>
Cc:     "iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Joerg Roedel <joro@...tes.org>,
        David Woodhouse <dwmw2@...radead.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Rafael Wysocki <rafael.j.wysocki@...el.com>,
        Alex Williamson <alex.williamson@...hat.com>,
        Lan Tianyu <tianyu.lan@...el.com>,
        Yi L <yi.l.liu@...ux.intel.com>,
        "Liu@...l.linuxfoundation.org" <Liu@...l.linuxfoundation.org>,
        jacob.jun.pan@...ux.intel.com
Subject: Re: [PATCH v3 08/16] iommu: introduce device fault data

On Wed, 10 Jan 2018 11:41:58 +0000
Jean-Philippe Brucker <jean-philippe.brucker@....com> wrote:

> Hi Jacob,
> 
> On 17/11/17 18:55, Jacob Pan wrote:
> [...]
> > +/**
> > + * struct iommu_fault_event - Generic per device fault data
> > + *
> > + * - PCI and non-PCI devices
> > + * - Recoverable faults (e.g. page request), information based on
> > PCI ATS
> > + * and PASID spec.
> > + * - Un-recoverable faults of device interest
> > + * - DMA remapping and IRQ remapping faults
> > +
> > + * @type contains fault type.
> > + * @reason fault reasons if relevant outside IOMMU driver, IOMMU
> > driver internal
> > + *         faults are not reported
> > + * @addr: tells the offending page address
> > + * @pasid: contains process address space ID, used in shared
> > virtual memory(SVM)
> > + * @rid: requestor ID
> > + * @page_req_group_id: page request group index
> > + * @last_req: last request in a page request group
> > + * @pasid_valid: indicates if the PRQ has a valid PASID
> > + * @prot: page access protection flag, e.g. IOMMU_FAULT_READ,
> > IOMMU_FAULT_WRITE
> > + * @device_private: if present, uniquely identify device-specific
> > + *                  private data for an individual page request.
> > + * @iommu_private: used by the IOMMU driver for storing
> > fault-specific
> > + *                 data. Users should not modify this field before
> > + *                 sending the fault response.
> > + */
> > +struct iommu_fault_event {
> > +	enum iommu_fault_type type;
> > +	enum iommu_fault_reason reason;
> > +	u64 addr;
> > +	u32 pasid;
> > +	u32 page_req_group_id : 9;  
> 
> As I've been rebasing my work onto your series, I have a few more
> comments about this structure. Is there any advantage in limiting the
> PRGI as a bitfield? PCI uses 9 bits, but others might need more. For
> instance ARM Stall uses 16-bit IDs to identify a fault event.
> 
> Could you please make it a u32 (as well as in page_response_msg), and
> could page_req_group_id be renamed to simply "id"?
> 
sure, I will make it u32 in v4 version of the patchset. I was using PCI
standard as a base with no specific advantage.
I am running into little bit problem with testing, so perhaps next week.
> > +	u32 last_req : 1;
> > +	u32 pasid_valid : 1;  
> I noticed that page_response_msg in patch 15/16 calls this bit
> "pasid_present". Could you rename it to "pasid_valid" for consistency?
> 
make sense.
> Thanks,
> Jean

[Jacob Pan]