linux-kernel - Re: [PATCH v3 1/5] iommu/amd: Disable AMD IOMMU if CMPXCHG16B feature is not supported

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]

Message-ID: <5ff8adab-42c9-403e-9822-8f4fbc259d07@amd.com>
Date: Mon, 16 Sep 2024 23:11:46 +0700
From: "Suthikulpanit, Suravee" <suravee.suthikulpanit@....com>
To: Jason Gunthorpe <jgg@...dia.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 v3 1/5] iommu/amd: Disable AMD IOMMU if CMPXCHG16B feature
 is not supported



On 9/6/2024 11:38 PM, Jason Gunthorpe wrote:
> On Fri, Sep 06, 2024 at 12:13:04PM +0000, Suravee Suthikulpanit wrote:
>> According to the AMD IOMMU spec, the IOMMU reads the entire DTE either
>> in two 128-bit transactions or a single 256-bit transaction.
> 
> .. if two 128-bit transaction on the read side is possible then you
> need flushing! :(
> 
> For instance this:
> 
>    IOMMU         CPU
> Read [0]
>                Write [0]
>                Write [1]
> Read [1]
> 
> Will result in the iommu seeing torn incorrect data - the Guest paging
> mode may not match the page table pointer, or the VIOMMU data may
> become mismatched to the host translation.
> 
> Avoiding flushing is only possible if the full 256 bits are read
> atomically.

I have verified with the hardware designer, and they have now confirmed 
that the IOMMU hardware has always been implemented with 256-bit read. 
The next revision of the IOMMU spec will be updated to correctly 
describe this part. Therefore, I will update the commit message and 
implement the code accordingly.

>> It is recommended to update DTE using 128-bit operation followed by
>> an INVALIDATE_DEVTAB_ENTYRY command when the IV=1b or V=1b.
> 
> This advice only works when going from non-valid to valid.

Actually, if we change the DTE when IV=1 or V=1, we would need to 
invalidate as well.

>> Suggested-by: Jason Gunthorpe <jgg@...dia.com>
>> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@....com>
>> ---
>>   drivers/iommu/amd/init.c | 23 +++++++++--------------
>>   1 file changed, 9 insertions(+), 14 deletions(-)
> 
> Reviewed-by: Jason Gunthorpe <jgg@...dia.com>
> 
> Jason

Thanks,
Suravee