Re: [Qemu-devel] [PATCH 0/4] AMD IOMMU v1

[Valentine Sinitsyn]

On 09.10.2015 17:27, David kiarie wrote:
> On Fri, Oct 9, 2015 at 3:24 PM, Valentine Sinitsyn
> <address@hidden> wrote:
> > On 09.10.2015 17:22, David kiarie wrote:
> > > On Fri, Oct 9, 2015 at 10:17 AM, Valentine Sinitsyn
> > > <address@hidden> wrote:
> > > > Hi David,
> > > >
> > > > Thanks for your efforts.
> > > >
> > > >
> > > > On 09.10.2015 07:53, David Kiarie wrote:
> > > > > David (4):
> > > > >      hw/core: Add iommu to machine properties
> > > > >      hw/pci-host: Add AMD IOMMU to PIIX and Q35 pcs
> > > > >      hw/i386: Introduce AMD IOMMU
> > > > >      hw/acpi: ACPI table for AMD IOMMU
> > > > >
> > > > >     hw/core/machine.c           |   25 +
> > > > >     hw/i386/Makefile.objs       |    1 +
> > > > >     hw/i386/acpi-build.c        |   85 +++
> > > > >     hw/i386/amd_iommu.c         | 1266
> > > > > +++++++++++++++++++++++++++++++++++++++++++
> > > > >     hw/i386/amd_iommu.h         |  363 +++++++++++++
> > > > >     hw/pci-host/piix.c          |   10 +
> > > > >     hw/pci-host/q35.c           |   10 +
> > > > >     include/hw/acpi/acpi-defs.h |   55 ++
> > > > >     include/hw/boards.h         |    2 +
> > > > >     9 files changed, 1817 insertions(+)
> > > > >     create mode 100644 hw/i386/amd_iommu.c
> > > > >     create mode 100644 hw/i386/amd_iommu.h
> > > > >
> > > > >
> > > > > AMD IOMMU v1
> > > > >
> > > > > Changes since RFC
> > > > >        -code cleanup around MMIO code
> > > > >        -compacted startup code - some of the 'init' and 'reset' code is
> > > > > similar and can be reused
> > > > >        -Implemented basic IOTLB - it's just a hashtable of already
> > > > > requested
> > > > > virtual address translations whereby if a device requests translation of
> > > > > the
> > > > > same address we don't have to do a page walk, again.
> > > > >        -Implemented event logging - this version logs about all possible
> > > > > SW/HW errors but most of them are very unlikely to occur.
> > > > >        -Changed GVA and HVA to go by the physical host bus
> > > > > implementations -
> > > > > 48 bits virtual and 40 bits physical address space.
> > > >
> > > >
> > > > I haven't looked into this closely yet, but shouldn't you support a whole
> > > > 63
> > > > bits wide addresses (and 6-level page tables),  as per specification?
> > > Am not sure too.
> > >
> > > When I tested this, linux was using 3-level pages.
> >
> > Nobody uses whole 6 page table levels, to the best of my knowledge.
> > Supporting (emulating) them, however, is part of the spec. Besides, it will
> > be needed if one decides to share page tables between IOMMU and CPU
>
> Should I use 6 levels/64 bit virtual address space ?
I think so, but let me look through the patch series first. That was a 
quick question - I just got surprised that you made it narrower than it was.

Valentine

> > > I went with 48 bits as AFAIK that's what current hardware works. This
> > > code should work with 6-levels though I just used 48 to be 'safe'.
> > >
> > > > > TODO
> > > > >       -Implement MMIO reserving code
> > > > >       -Implement r/wc MMIO registers
> > > > >       -Implement interrupts related to r/wc registers
> > > > >
> > > > > IOMMU won't be able to implement PPR logging with the current Qemu
> > > > > infrastructure - Qemu doesn't implement anything that's related to
> > > > > PASID.
> > > > > Similary, guest translation won't be possible.
> > > > >
> > > > > Note: I have not tested this code and it might have a few bugs (silly
> > > > > bugs) of course but the RFC version works so, if present, it's just
> > > > > minor
> > > > > bugs.
> > > >
> > > >
> > > >
> > > > Regards,
> > > > Valentine Sinitsyn
> >
> >
> > Regards,
> > Valentine Sinitsyn