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

[Valentine Sinitsyn <valentine.sinitsyn@gmail.com>]

On 09.10.2015 17:27, David kiarie wrote:
> On Fri, Oct 9, 2015 at 3:24 PM, Valentine Sinitsyn
> <valentine.sinitsyn@gmail.com> wrote:
>> On 09.10.2015 17:22, David kiarie wrote:
>>>
>>> On Fri, Oct 9, 2015 at 10:17 AM, Valentine Sinitsyn
>>> <valentine.sinitsyn@gmail.com> 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