Re: [PATCH rc] iommu/amd: Fix geometry.aperture_end for V2 tables

[Jason Gunthorpe <jgg@nvidia.com>]

On Thu, Apr 24, 2025 at 01:26:21PM +0530, Vasant Hegde wrote:
> Hi Jason,
> 
> On 4/17/2025 9:51 PM, Jason Gunthorpe wrote:
> > The AMD IOMMU documentation seems pretty clear that the V2 table follows
> > the normal CPU expectation of sign extension. This is shown in
> > 
> >   Figure 25: AMD64 Long Mode 4-Kbyte Page Address Translation
> > 
> > Where bits Sign-Extend [63:57] == [56]. This is typical for x86 which
> > would have three regions in the page table: lower, non-canonical, upper.
> > 
> > The manual describes that the V1 table does not sign extend in section
> > 2.2.4 Sharing AMD64 Processor and IOMMU Page Tables GPA-to-SPA
> > 
> > The iommu domain geometry does not directly support sign extended page
> > tables. The driver should report only one of the lower/upper spaces. Solve
> > this by removing the top VA bit from the geometry to use only the lower
> > space.
> > 
> > Adjust dma_max_address() to do this. It now returns:
> > 
> > 5 Level:
> >   Before 0x1ffffffffffffff
> >   After  0x0ffffffffffffff
> > 4 Level:
> >   Before 0xffffffffffff
> >   After  0x7fffffffffff
> > 
> > Fixes: 11c439a19466 ("iommu/amd/pgtbl_v2: Fix domain max address")
> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> > ---
> >  drivers/iommu/amd/iommu.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > AMD folks: I'm just reading the documentation, it would be good to confirm
> > this understanding. I'm a bit surprised nobody hit this, but given the domain
> > aperture was wildly wrong up till 2023 maybe it never gets exercised
> > carefully.
> 
> We have tested this with 4 and 5 level w/ some bench marks as well as w/
> forcedac=1. It works fine. My understanding is IOMMU uses bit 56/47 as well for
> address translation.

Yes, it should use bit 56 for address translation, that is part of the
page table architecture.

The question is what happen if a device uses IOVA 0x0100000000000000
with the iommu. This is a non-canonical address, so I think
architecturally on x86 it should be rejected. I would not be surprised
if some HW treats it the same as 0xFF00000000000000 - though that
would be dangerous.

There is a significant correctness issue here with ATS, the IOMMU
*must not* allow address aliases to exist, so if it responds to ATS
queries at both 0x0100000000000000 and 0xFF00000000000000 with the
same PTE then it is security broken. The device ATC is only flushed
based on the canonical IOVA, so any aliases can remain in the ATC and
trigger UAF issues. This can possibly be triggered by userspace when
using VFIO :\

So the question is not about if bit 56/47 is used, but if the IOMMU hw
is validating the sign extension. Assuming it is validating then we
must not tell the iommu core code to use
 0x0100000000000000 -> 0x01FFFFFFFFFFFFFF
as IOVA since it is not legal IOVA. This is why the bit width is
reduced by one when computing the aperture.

Given the security sensistivity with ATS the sign validation behavior
should be understood because I do plan to come with a patch to enable
the high address space for iommufd and if some AMD implementations
need to block that we should know :)

Thanks,
Jason