On Tue, Oct 14, 2025 at 02:42:12PM +0530, Sairaj Kodilkar wrote:
>
>
> On 10/14/2025 2:35 PM, Michael S. Tsirkin wrote:
> > 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<[email protected]>
> > > > 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.
>
> Right now it defines macro 'AMDVI_IOTLB_MAX_SIZE'. If iotlb hash
> size is greater than that, code resets the hash table.
> Techinically, right way is to implement a policy such as LRU, which
> we can work on in future patches.
>
> Thanks
> Sairaj
sounds good, thanks!
> > > > 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
