Re: [PATCH v2 2/2] amd_iommu: Support 64 bit address for IOTLB lookup

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Oct 14, 2025 at 02:34:28PM +0530, Sairaj Kodilkar wrote:
> 
> 
> On 10/13/2025 1:49 PM, Michael S. Tsirkin wrote:
> > On Mon, Oct 13, 2025 at 10:30:46AM +0530, Sairaj Kodilkar wrote:
> > > Physical AMD IOMMU supports up to 64 bits of DMA address. When device tries
> > > to read or write from a given DMA address, IOMMU translates the address
> > > using page table assigned to that device. Since IOMMU uses per device page
> > > tables, the emulated IOMMU should use the cache tag of 68 bits
> > > (64 bit address - 12 bit page alignment + 16 bit device ID).
> > > 
> > > Current emulated AMD IOMMU uses GLib hash table to create software iotlb
> > > and uses 64 bit key to store the IOVA and deviceID, which limits the IOVA
> > > to 60 bits. This causes failure while setting up the device when guest is
> > > booted with "iommu.forcedac=1".
> > > 
> > > To solve this problem, Use 64 bit IOVA and 16 bit devid as key to store
> > > entries in IOTLB; Use upper 52 bits of IOVA (GFN) and lower 12 bits of
> > > the device ID to construct the 64 bit hash key in order avoid the
> > > truncation as much as possible (reducing hash collisions).
> > > 
> > > Fixes: d29a09ca6842 ("hw/i386: Introduce AMD IOMMU")
> > > Signed-off-by: Sairaj Kodilkar<sarunkod@amd.com>
> > I am wondering whether we need to limit how much host memory
> > can the shadow take. Because with so many bits, the sky is the limit ...
> > OTOH it's not directly caused by this patch, but it's something
> > we should think about maybe.
> 
> I don't think I fully understand this. Do you mean the host memory
> taken by the pages used to build shadow page table ?

the memory used by the hash table.

> 
> > Something more to improve:
> > 
> > 
> > > ---
> > >   hw/i386/amd_iommu.c | 57 ++++++++++++++++++++++++++++++---------------
> > >   hw/i386/amd_iommu.h |  4 ++--
> > >   2 files changed, 40 insertions(+), 21 deletions(-)
> > > 
> > > diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> > > index b194e3294dd7..a218d147e53d 100644
> > > --- a/hw/i386/amd_iommu.c
> > > +++ b/hw/i386/amd_iommu.c
> > > @@ -106,6 +106,11 @@ typedef struct amdvi_as_key {
> > >       uint8_t devfn;
> > >   } amdvi_as_key;
> > > +typedef struct amdvi_iotlb_key {
> > > +    uint64_t gfn;
> > > +    uint16_t devid;
> > > +} amdvi_iotlb_key;
> > > +
> > Pls change struct and typedef names to match qemu coding style.
> Thanks
> Sairaj