linux-kernel - Re: [PATCH v2 3/5] iommu/amd: Introduce helper functions to access and update 256-bit DTE

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] [day] [month] [year] [list]

Message-ID: <20240906160132.GJ1358970@nvidia.com>
Date: Fri, 6 Sep 2024 13:01:32 -0300
From: Jason Gunthorpe <jgg@...dia.com>
To: "Suthikulpanit, Suravee" <suravee.suthikulpanit@....com>
Cc: linux-kernel@...r.kernel.org, iommu@...ts.linux.dev, joro@...tes.org,
	robin.murphy@....com, vasant.hegde@....com, ubizjak@...il.com,
	jon.grimm@....com, santosh.shukla@....com, pandoh@...gle.com,
	kumaranand@...gle.com
Subject: Re: [PATCH v2 3/5] iommu/amd: Introduce helper functions to access
 and update 256-bit DTE

On Fri, Sep 06, 2024 at 09:08:06PM +0700, Suthikulpanit, Suravee wrote:
> On 9/6/2024 1:21 AM, Jason Gunthorpe wrote:
> > > > I don't think you should restore, this should reflect a locking error
> > > > but we still need to move forward and put some kind of correct
> > > > data.. The code can't go backwards so it should try to move forwards..
> > > In case of error, what if we pr_warn and put the device in blocking mode
> > > since we need to prevent malicious DMAs.
> > IMHO a WARN_ON is fine, and alerts to the possible machine corruption
> > No need to do blocking, you should have a perfectly valid target DTE
> > that represents the state the HW is expected to be in. Resolve the
> > race by making it bin that state and move forwards.
> 
> What do you mean by "making it bin that state".

Sorry, "be in that state"

> > The guidelines in "2.2.2.2 Making Device Table Entry Changes" make
> > this clear. The indivudal CPU writes smaller than 256 bits have to be
> > sequenced right.
> 
> For the interrupt remapping part, no special step is needed if we can write
> do 64-bit write. 

Yes

> Similary, for the address translation part, no special step is
> needed if we can do 128-bit write.

Except for GuestPagingMode, as below.

> > This section looks like it was written before translation bits were
> > placed in the other 128 bit word - it assumes a single 128 bit write
> > is always sufficient which isn't true anymore.
> > 
> > So you still have the issue of having to decide if you write 128 bit
> > [0] or [1] first.
> 
> The GuestPagingMode bit is in effect when GV=1. So, the higher 128-bit
> (which contains GuestPagingMode bit) should be written first, and follow by
> lower 128-bit (which contans GV bit).

Yes, exactly. That is what I mean by ordering.

When clearing GV=0 you have to do the reverse ordering, write the low
128 then the high.

Jason