mbox series

[v5,0/7] Add virtio-iommu driver

Message ID 20181122193801.50510-1-jean-philippe.brucker@arm.com
Headers show
Series Add virtio-iommu driver | expand

Message

Jean-Philippe Brucker Nov. 22, 2018, 7:37 p.m. UTC
Implement the virtio-iommu driver, following specification v0.9 [1].

Since v4 [2] I fixed the issues reported by Eric, and added Reviewed-by
from Eric and Rob. Thanks!

I changed the specification to fix one inconsistency discussed in v4.
That the device fills the probe buffer with zeroes is now a "SHOULD"
instead of a "MAY", since it's the only way for the driver to know if
the device wrote the status. Existing devices already do this. In
addition the device now needs to fill the three padding bytes at the
tail with zeroes.

You can find Linux driver and kvmtool device on branches
virtio-iommu/v0.9 [3]. I also lightly tested with Eric's latest QEMU
device [4].

[1] Virtio-iommu specification v0.9, sources, pdf and diff from v0.8
    git://linux-arm.org/virtio-iommu.git virtio-iommu/v0.9
    http://jpbrucker.net/virtio-iommu/spec/v0.9/virtio-iommu-v0.9.pdf
    http://jpbrucker.net/virtio-iommu/spec/diffs/virtio-iommu-pdf-diff-v0.8-v0.9.pdf

[2] [PATCH v4 0/7] Add virtio-iommu driver
    https://lists.linuxfoundation.org/pipermail/iommu/2018-November/031074.html

[3] git://linux-arm.org/linux-jpb.git virtio-iommu/v0.9
    git://linux-arm.org/kvmtool-jpb.git virtio-iommu/v0.9

[4] [RFC v9 00/17] VIRTIO-IOMMU device
    https://www.mail-archive.com/qemu-devel@nongnu.org/msg575578.html

Jean-Philippe Brucker (7):
  dt-bindings: virtio-mmio: Add IOMMU description
  dt-bindings: virtio: Add virtio-pci-iommu node
  of: Allow the iommu-map property to omit untranslated devices
  PCI: OF: Initialize dev->fwnode appropriately
  iommu: Add virtio-iommu driver
  iommu/virtio: Add probe request
  iommu/virtio: Add event queue

 .../devicetree/bindings/virtio/iommu.txt      |   66 +
 .../devicetree/bindings/virtio/mmio.txt       |   30 +
 MAINTAINERS                                   |    7 +
 drivers/iommu/Kconfig                         |   11 +
 drivers/iommu/Makefile                        |    1 +
 drivers/iommu/virtio-iommu.c                  | 1157 +++++++++++++++++
 drivers/of/base.c                             |   10 +-
 drivers/pci/of.c                              |    7 +
 include/uapi/linux/virtio_ids.h               |    1 +
 include/uapi/linux/virtio_iommu.h             |  161 +++
 10 files changed, 1448 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/virtio/iommu.txt
 create mode 100644 drivers/iommu/virtio-iommu.c
 create mode 100644 include/uapi/linux/virtio_iommu.h

Comments

Eric Auger Nov. 23, 2018, 8:28 a.m. UTC | #1
Hi Jean,

On 11/22/18 8:37 PM, Jean-Philippe Brucker wrote:
> Implement the virtio-iommu driver, following specification v0.9 [1].
> 
> Since v4 [2] I fixed the issues reported by Eric, and added Reviewed-by
> from Eric and Rob. Thanks!
> 
> I changed the specification to fix one inconsistency discussed in v4.
> That the device fills the probe buffer with zeroes is now a "SHOULD"
> instead of a "MAY", since it's the only way for the driver to know if
> the device wrote the status. Existing devices already do this. In
> addition the device now needs to fill the three padding bytes at the
> tail with zeroes.
> 
> You can find Linux driver and kvmtool device on branches
> virtio-iommu/v0.9 [3]. I also lightly tested with Eric's latest QEMU
> device [4].
> 
> [1] Virtio-iommu specification v0.9, sources, pdf and diff from v0.8
>     git://linux-arm.org/virtio-iommu.git virtio-iommu/v0.9
>     http://jpbrucker.net/virtio-iommu/spec/v0.9/virtio-iommu-v0.9.pdf
>     http://jpbrucker.net/virtio-iommu/spec/diffs/virtio-iommu-pdf-diff-v0.8-v0.9.pdf
> 
> [2] [PATCH v4 0/7] Add virtio-iommu driver
>     https://lists.linuxfoundation.org/pipermail/iommu/2018-November/031074.html
> 
> [3] git://linux-arm.org/linux-jpb.git virtio-iommu/v0.9
>     git://linux-arm.org/kvmtool-jpb.git virtio-iommu/v0.9
> 
> [4] [RFC v9 00/17] VIRTIO-IOMMU device
>     https://www.mail-archive.com/qemu-devel@nongnu.org/msg575578.html
> 
> Jean-Philippe Brucker (7):
>   dt-bindings: virtio-mmio: Add IOMMU description
>   dt-bindings: virtio: Add virtio-pci-iommu node
>   of: Allow the iommu-map property to omit untranslated devices
>   PCI: OF: Initialize dev->fwnode appropriately
>   iommu: Add virtio-iommu driver
>   iommu/virtio: Add probe request
>   iommu/virtio: Add event queue
> 
>  .../devicetree/bindings/virtio/iommu.txt      |   66 +
>  .../devicetree/bindings/virtio/mmio.txt       |   30 +
>  MAINTAINERS                                   |    7 +
>  drivers/iommu/Kconfig                         |   11 +
>  drivers/iommu/Makefile                        |    1 +
>  drivers/iommu/virtio-iommu.c                  | 1157 +++++++++++++++++
>  drivers/of/base.c                             |   10 +-
>  drivers/pci/of.c                              |    7 +
>  include/uapi/linux/virtio_ids.h               |    1 +
>  include/uapi/linux/virtio_iommu.h             |  161 +++
>  10 files changed, 1448 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/virtio/iommu.txt
>  create mode 100644 drivers/iommu/virtio-iommu.c
>  create mode 100644 include/uapi/linux/virtio_iommu.h
> 
for the whole series
Tested-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric
Bharat Bhushan Nov. 27, 2018, 7:09 a.m. UTC | #2
Hi Jean,

> -----Original Message-----
> From: Auger Eric <eric.auger@redhat.com>
> Sent: Friday, November 23, 2018 1:59 PM
> To: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>;
> iommu@lists.linux-foundation.org; linux-pci@vger.kernel.org;
> devicetree@vger.kernel.org; virtualization@lists.linux-foundation.org; virtio-
> dev@lists.oasis-open.org; joro@8bytes.org; mst@redhat.com
> Cc: jasowang@redhat.com; robh+dt@kernel.org; mark.rutland@arm.com;
> bhelgaas@google.com; frowand.list@gmail.com;
> kvmarm@lists.cs.columbia.edu; tnowicki@caviumnetworks.com;
> kevin.tian@intel.com; marc.zyngier@arm.com; robin.murphy@arm.com;
> will.deacon@arm.com; lorenzo.pieralisi@arm.com; Bharat Bhushan
> <bharat.bhushan@nxp.com>
> Subject: Re: [PATCH v5 0/7] Add virtio-iommu driver
> 
> Hi Jean,
> 
> On 11/22/18 8:37 PM, Jean-Philippe Brucker wrote:
> > Implement the virtio-iommu driver, following specification v0.9 [1].
> >
> > Since v4 [2] I fixed the issues reported by Eric, and added
> > Reviewed-by from Eric and Rob. Thanks!
> >
> > I changed the specification to fix one inconsistency discussed in v4.
> > That the device fills the probe buffer with zeroes is now a "SHOULD"
> > instead of a "MAY", since it's the only way for the driver to know if
> > the device wrote the status. Existing devices already do this. In
> > addition the device now needs to fill the three padding bytes at the
> > tail with zeroes.
> >
> > You can find Linux driver and kvmtool device on branches
> > virtio-iommu/v0.9 [3]. I also lightly tested with Eric's latest QEMU
> > device [4].
> >
> > [1] Virtio-iommu specification v0.9, sources, pdf and diff from v0.8
> >     git://linux-arm.org/virtio-iommu.git virtio-iommu/v0.9
> >
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fjpbr
> ucker.net%2Fvirtio-iommu%2Fspec%2Fv0.9%2Fvirtio-iommu-
> v0.9.pdf&amp;data=02%7C01%7Cbharat.bhushan%40nxp.com%7C6e7180e7
> df8e41943d4108d6511db8ed%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%
> 7C1%7C636785585424990803&amp;sdata=la0tSTLcOI5HkQ65a%2BCHKeI3H5iu
> qZ%2F8r6Q5YF8tfsU%3D&amp;reserved=0
> >
> >
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fjpbr
> > ucker.net%2Fvirtio-iommu%2Fspec%2Fdiffs%2Fvirtio-iommu-pdf-diff-v0.8-
> v
> >
> 0.9.pdf&amp;data=02%7C01%7Cbharat.bhushan%40nxp.com%7C6e7180e7d
> f8e4194
> >
> 3d4108d6511db8ed%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C6
> 3678558
> >
> 5424990803&amp;sdata=AEXEib4lihcpfE6O6wLf%2BMElPtA7ZLGYE2mj0288PZ
> k%3D&
> > amp;reserved=0
> >
> > [2] [PATCH v4 0/7] Add virtio-iommu driver
> >
> >
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
> > ts.linuxfoundation.org%2Fpipermail%2Fiommu%2F2018-
> November%2F031074.ht
> >
> ml&amp;data=02%7C01%7Cbharat.bhushan%40nxp.com%7C6e7180e7df8e4
> 1943d410
> >
> 8d6511db8ed%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C636785
> 5854249
> >
> 90803&amp;sdata=mUUSBQ%2FjEeRGaisGBK20G9WmfXPwlERKDaeeRqHW4
> 08%3D&amp;r
> > eserved=0
> >
> > [3] git://linux-arm.org/linux-jpb.git virtio-iommu/v0.9
> >     git://linux-arm.org/kvmtool-jpb.git virtio-iommu/v0.9
> >
> > [4] [RFC v9 00/17] VIRTIO-IOMMU device
> >
> >
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fw
> ww
> > .mail-archive.com%2Fqemu-
> devel%40nongnu.org%2Fmsg575578.html&amp;data=
> >
> 02%7C01%7Cbharat.bhushan%40nxp.com%7C6e7180e7df8e41943d4108d651
> 1db8ed%
> >
> 7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C636785585424990803&
> amp;sd
> >
> ata=fo9WKE33Nm%2FdW2C2XcSVmv9itWjEyRN1irgEZgOWtZI%3D&amp;rese
> rved=0
> >
> > Jean-Philippe Brucker (7):
> >   dt-bindings: virtio-mmio: Add IOMMU description
> >   dt-bindings: virtio: Add virtio-pci-iommu node
> >   of: Allow the iommu-map property to omit untranslated devices
> >   PCI: OF: Initialize dev->fwnode appropriately
> >   iommu: Add virtio-iommu driver
> >   iommu/virtio: Add probe request
> >   iommu/virtio: Add event queue
> >
> >  .../devicetree/bindings/virtio/iommu.txt      |   66 +
> >  .../devicetree/bindings/virtio/mmio.txt       |   30 +
> >  MAINTAINERS                                   |    7 +
> >  drivers/iommu/Kconfig                         |   11 +
> >  drivers/iommu/Makefile                        |    1 +
> >  drivers/iommu/virtio-iommu.c                  | 1157 +++++++++++++++++
> >  drivers/of/base.c                             |   10 +-
> >  drivers/pci/of.c                              |    7 +
> >  include/uapi/linux/virtio_ids.h               |    1 +
> >  include/uapi/linux/virtio_iommu.h             |  161 +++
> >  10 files changed, 1448 insertions(+), 3 deletions(-)  create mode
> > 100644 Documentation/devicetree/bindings/virtio/iommu.txt
> >  create mode 100644 drivers/iommu/virtio-iommu.c  create mode 100644
> > include/uapi/linux/virtio_iommu.h
> >
> for the whole series
> Tested-by: Eric Auger <eric.auger@redhat.com>

I have tested this series with virtio/vfio both
Tested-by: Bharat Bhushan <bharat.bhushan@nxp.com>


Thanks
-Bharat

> 
> Thanks
> 
> Eric
Michael S. Tsirkin Nov. 27, 2018, 4:53 p.m. UTC | #3
On Thu, Nov 22, 2018 at 07:37:54PM +0000, Jean-Philippe Brucker wrote:
> Implement the virtio-iommu driver, following specification v0.9 [1].
> 
> Since v4 [2] I fixed the issues reported by Eric, and added Reviewed-by
> from Eric and Rob. Thanks!
> 
> I changed the specification to fix one inconsistency discussed in v4.
> That the device fills the probe buffer with zeroes is now a "SHOULD"
> instead of a "MAY", since it's the only way for the driver to know if
> the device wrote the status. Existing devices already do this. In
> addition the device now needs to fill the three padding bytes at the
> tail with zeroes.
> 
> You can find Linux driver and kvmtool device on branches
> virtio-iommu/v0.9 [3]. I also lightly tested with Eric's latest QEMU
> device [4].

I tried to get this to work on my x86 box but without
success. Any hints? Does this have to do with the IORT table?
I think we really should just reserve our own table ID
and avoid the pain of trying to add things to the IORT spec.
I'm reluctant to merge lots of code that I can't easily test.
Again, if we found a way to push more configuration into
virtio config space the problem space would be smaller.

> [1] Virtio-iommu specification v0.9, sources, pdf and diff from v0.8
>     git://linux-arm.org/virtio-iommu.git virtio-iommu/v0.9
>     http://jpbrucker.net/virtio-iommu/spec/v0.9/virtio-iommu-v0.9.pdf
>     http://jpbrucker.net/virtio-iommu/spec/diffs/virtio-iommu-pdf-diff-v0.8-v0.9.pdf
> 
> [2] [PATCH v4 0/7] Add virtio-iommu driver
>     https://lists.linuxfoundation.org/pipermail/iommu/2018-November/031074.html
> 
> [3] git://linux-arm.org/linux-jpb.git virtio-iommu/v0.9
>     git://linux-arm.org/kvmtool-jpb.git virtio-iommu/v0.9
> 
> [4] [RFC v9 00/17] VIRTIO-IOMMU device
>     https://www.mail-archive.com/qemu-devel@nongnu.org/msg575578.html
> 
> Jean-Philippe Brucker (7):
>   dt-bindings: virtio-mmio: Add IOMMU description
>   dt-bindings: virtio: Add virtio-pci-iommu node
>   of: Allow the iommu-map property to omit untranslated devices
>   PCI: OF: Initialize dev->fwnode appropriately
>   iommu: Add virtio-iommu driver
>   iommu/virtio: Add probe request
>   iommu/virtio: Add event queue
> 
>  .../devicetree/bindings/virtio/iommu.txt      |   66 +
>  .../devicetree/bindings/virtio/mmio.txt       |   30 +
>  MAINTAINERS                                   |    7 +
>  drivers/iommu/Kconfig                         |   11 +
>  drivers/iommu/Makefile                        |    1 +
>  drivers/iommu/virtio-iommu.c                  | 1157 +++++++++++++++++
>  drivers/of/base.c                             |   10 +-
>  drivers/pci/of.c                              |    7 +
>  include/uapi/linux/virtio_ids.h               |    1 +
>  include/uapi/linux/virtio_iommu.h             |  161 +++
>  10 files changed, 1448 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/virtio/iommu.txt
>  create mode 100644 drivers/iommu/virtio-iommu.c
>  create mode 100644 include/uapi/linux/virtio_iommu.h
> 
> -- 
> 2.19.1
Eric Auger Nov. 27, 2018, 5:09 p.m. UTC | #4
Hi Michael,

On 11/27/18 5:53 PM, Michael S. Tsirkin wrote:
> On Thu, Nov 22, 2018 at 07:37:54PM +0000, Jean-Philippe Brucker wrote:
>> Implement the virtio-iommu driver, following specification v0.9 [1].
>>
>> Since v4 [2] I fixed the issues reported by Eric, and added Reviewed-by
>> from Eric and Rob. Thanks!
>>
>> I changed the specification to fix one inconsistency discussed in v4.
>> That the device fills the probe buffer with zeroes is now a "SHOULD"
>> instead of a "MAY", since it's the only way for the driver to know if
>> the device wrote the status. Existing devices already do this. In
>> addition the device now needs to fill the three padding bytes at the
>> tail with zeroes.
>>
>> You can find Linux driver and kvmtool device on branches
>> virtio-iommu/v0.9 [3]. I also lightly tested with Eric's latest QEMU
>> device [4].
> 
> I tried to get this to work on my x86 box but without
> success. Any hints? Does this have to do with the IORT table?
> I think we really should just reserve our own table ID
> and avoid the pain of trying to add things to the IORT spec.
> I'm reluctant to merge lots of code that I can't easily test.
> Again, if we found a way to push more configuration into
> virtio config space the problem space would be smaller.

You can at least test it with QEMU ARM virt in TCG mode. Then I have
worked on the IORT integration in PC/Q35 but this is not yet functional.
I need to debug what's wrong on guest ACPI probing. I plan to work on
this this week.

Thanks

Eric
> 
>> [1] Virtio-iommu specification v0.9, sources, pdf and diff from v0.8
>>     git://linux-arm.org/virtio-iommu.git virtio-iommu/v0.9
>>     http://jpbrucker.net/virtio-iommu/spec/v0.9/virtio-iommu-v0.9.pdf
>>     http://jpbrucker.net/virtio-iommu/spec/diffs/virtio-iommu-pdf-diff-v0.8-v0.9.pdf
>>
>> [2] [PATCH v4 0/7] Add virtio-iommu driver
>>     https://lists.linuxfoundation.org/pipermail/iommu/2018-November/031074.html
>>
>> [3] git://linux-arm.org/linux-jpb.git virtio-iommu/v0.9
>>     git://linux-arm.org/kvmtool-jpb.git virtio-iommu/v0.9
>>
>> [4] [RFC v9 00/17] VIRTIO-IOMMU device
>>     https://www.mail-archive.com/qemu-devel@nongnu.org/msg575578.html
>>
>> Jean-Philippe Brucker (7):
>>   dt-bindings: virtio-mmio: Add IOMMU description
>>   dt-bindings: virtio: Add virtio-pci-iommu node
>>   of: Allow the iommu-map property to omit untranslated devices
>>   PCI: OF: Initialize dev->fwnode appropriately
>>   iommu: Add virtio-iommu driver
>>   iommu/virtio: Add probe request
>>   iommu/virtio: Add event queue
>>
>>  .../devicetree/bindings/virtio/iommu.txt      |   66 +
>>  .../devicetree/bindings/virtio/mmio.txt       |   30 +
>>  MAINTAINERS                                   |    7 +
>>  drivers/iommu/Kconfig                         |   11 +
>>  drivers/iommu/Makefile                        |    1 +
>>  drivers/iommu/virtio-iommu.c                  | 1157 +++++++++++++++++
>>  drivers/of/base.c                             |   10 +-
>>  drivers/pci/of.c                              |    7 +
>>  include/uapi/linux/virtio_ids.h               |    1 +
>>  include/uapi/linux/virtio_iommu.h             |  161 +++
>>  10 files changed, 1448 insertions(+), 3 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/virtio/iommu.txt
>>  create mode 100644 drivers/iommu/virtio-iommu.c
>>  create mode 100644 include/uapi/linux/virtio_iommu.h
>>
>> -- 
>> 2.19.1
Michael S. Tsirkin Nov. 27, 2018, 5:16 p.m. UTC | #5
On Tue, Nov 27, 2018 at 06:09:25PM +0100, Auger Eric wrote:
> Hi Michael,
> 
> On 11/27/18 5:53 PM, Michael S. Tsirkin wrote:
> > On Thu, Nov 22, 2018 at 07:37:54PM +0000, Jean-Philippe Brucker wrote:
> >> Implement the virtio-iommu driver, following specification v0.9 [1].
> >>
> >> Since v4 [2] I fixed the issues reported by Eric, and added Reviewed-by
> >> from Eric and Rob. Thanks!
> >>
> >> I changed the specification to fix one inconsistency discussed in v4.
> >> That the device fills the probe buffer with zeroes is now a "SHOULD"
> >> instead of a "MAY", since it's the only way for the driver to know if
> >> the device wrote the status. Existing devices already do this. In
> >> addition the device now needs to fill the three padding bytes at the
> >> tail with zeroes.
> >>
> >> You can find Linux driver and kvmtool device on branches
> >> virtio-iommu/v0.9 [3]. I also lightly tested with Eric's latest QEMU
> >> device [4].
> > 
> > I tried to get this to work on my x86 box but without
> > success. Any hints? Does this have to do with the IORT table?
> > I think we really should just reserve our own table ID
> > and avoid the pain of trying to add things to the IORT spec.
> > I'm reluctant to merge lots of code that I can't easily test.
> > Again, if we found a way to push more configuration into
> > virtio config space the problem space would be smaller.
> 
> You can at least test it with QEMU ARM virt in TCG mode.

It's slow enough that I generally just focus on KVM.

> Then I have
> worked on the IORT integration in PC/Q35 but this is not yet functional.
> I need to debug what's wrong on guest ACPI probing. I plan to work on
> this this week.
> 
> Thanks
> 
> Eric

Sounds good. Did you need to make changes to IORT?  I don't remember. If
yes it would be very easy to just have a virtio specific ACPI table -
I am assuming ARM guys will be just as hostile to virt changes
to IORT as they were to minor changes to ARM vIOMMU.


> > 
> >> [1] Virtio-iommu specification v0.9, sources, pdf and diff from v0.8
> >>     git://linux-arm.org/virtio-iommu.git virtio-iommu/v0.9
> >>     http://jpbrucker.net/virtio-iommu/spec/v0.9/virtio-iommu-v0.9.pdf
> >>     http://jpbrucker.net/virtio-iommu/spec/diffs/virtio-iommu-pdf-diff-v0.8-v0.9.pdf
> >>
> >> [2] [PATCH v4 0/7] Add virtio-iommu driver
> >>     https://lists.linuxfoundation.org/pipermail/iommu/2018-November/031074.html
> >>
> >> [3] git://linux-arm.org/linux-jpb.git virtio-iommu/v0.9
> >>     git://linux-arm.org/kvmtool-jpb.git virtio-iommu/v0.9
> >>
> >> [4] [RFC v9 00/17] VIRTIO-IOMMU device
> >>     https://www.mail-archive.com/qemu-devel@nongnu.org/msg575578.html
> >>
> >> Jean-Philippe Brucker (7):
> >>   dt-bindings: virtio-mmio: Add IOMMU description
> >>   dt-bindings: virtio: Add virtio-pci-iommu node
> >>   of: Allow the iommu-map property to omit untranslated devices
> >>   PCI: OF: Initialize dev->fwnode appropriately
> >>   iommu: Add virtio-iommu driver
> >>   iommu/virtio: Add probe request
> >>   iommu/virtio: Add event queue
> >>
> >>  .../devicetree/bindings/virtio/iommu.txt      |   66 +
> >>  .../devicetree/bindings/virtio/mmio.txt       |   30 +
> >>  MAINTAINERS                                   |    7 +
> >>  drivers/iommu/Kconfig                         |   11 +
> >>  drivers/iommu/Makefile                        |    1 +
> >>  drivers/iommu/virtio-iommu.c                  | 1157 +++++++++++++++++
> >>  drivers/of/base.c                             |   10 +-
> >>  drivers/pci/of.c                              |    7 +
> >>  include/uapi/linux/virtio_ids.h               |    1 +
> >>  include/uapi/linux/virtio_iommu.h             |  161 +++
> >>  10 files changed, 1448 insertions(+), 3 deletions(-)
> >>  create mode 100644 Documentation/devicetree/bindings/virtio/iommu.txt
> >>  create mode 100644 drivers/iommu/virtio-iommu.c
> >>  create mode 100644 include/uapi/linux/virtio_iommu.h
> >>
> >> -- 
> >> 2.19.1
Eric Auger Nov. 27, 2018, 6:02 p.m. UTC | #6
Hi Michael,

On 11/27/18 6:16 PM, Michael S. Tsirkin wrote:
> On Tue, Nov 27, 2018 at 06:09:25PM +0100, Auger Eric wrote:
>> Hi Michael,
>>
>> On 11/27/18 5:53 PM, Michael S. Tsirkin wrote:
>>> On Thu, Nov 22, 2018 at 07:37:54PM +0000, Jean-Philippe Brucker wrote:
>>>> Implement the virtio-iommu driver, following specification v0.9 [1].
>>>>
>>>> Since v4 [2] I fixed the issues reported by Eric, and added Reviewed-by
>>>> from Eric and Rob. Thanks!
>>>>
>>>> I changed the specification to fix one inconsistency discussed in v4.
>>>> That the device fills the probe buffer with zeroes is now a "SHOULD"
>>>> instead of a "MAY", since it's the only way for the driver to know if
>>>> the device wrote the status. Existing devices already do this. In
>>>> addition the device now needs to fill the three padding bytes at the
>>>> tail with zeroes.
>>>>
>>>> You can find Linux driver and kvmtool device on branches
>>>> virtio-iommu/v0.9 [3]. I also lightly tested with Eric's latest QEMU
>>>> device [4].
>>>
>>> I tried to get this to work on my x86 box but without
>>> success. Any hints? Does this have to do with the IORT table?
>>> I think we really should just reserve our own table ID
>>> and avoid the pain of trying to add things to the IORT spec.
>>> I'm reluctant to merge lots of code that I can't easily test.
>>> Again, if we found a way to push more configuration into
>>> virtio config space the problem space would be smaller.
>>
>> You can at least test it with QEMU ARM virt in TCG mode.
> 
> It's slow enough that I generally just focus on KVM.
fair enough
> 
>> Then I have
>> worked on the IORT integration in PC/Q35 but this is not yet functional.
>> I need to debug what's wrong on guest ACPI probing. I plan to work on
>> this this week.
>>
>> Thanks
>>
>> Eric
> 
> Sounds good. Did you need to make changes to IORT?  I don't remember. If
> yes it would be very easy to just have a virtio specific ACPI table -
> I am assuming ARM guys will be just as hostile to virt changes
> to IORT as they were to minor changes to ARM vIOMMU.

Well the only difference is that on ARM we have 3 nodes in the IORT: the
root complex node -> the VIRTIO-IOMMU node -> the ITS node. The ITS is
the ARM MSI controller which does MSI translation. So the ARM IORT
describes the chained ID mappings from the end point RID through the
StreamID SMMU input to the device ID MSI controller input. On Intel we
don't have the last node.

But again my integration is not yet functional and I don't know yet if I
have captured the whole picture.

Thanks

Eric
> 
> 
>>>
>>>> [1] Virtio-iommu specification v0.9, sources, pdf and diff from v0.8
>>>>     git://linux-arm.org/virtio-iommu.git virtio-iommu/v0.9
>>>>     http://jpbrucker.net/virtio-iommu/spec/v0.9/virtio-iommu-v0.9.pdf
>>>>     http://jpbrucker.net/virtio-iommu/spec/diffs/virtio-iommu-pdf-diff-v0.8-v0.9.pdf
>>>>
>>>> [2] [PATCH v4 0/7] Add virtio-iommu driver
>>>>     https://lists.linuxfoundation.org/pipermail/iommu/2018-November/031074.html
>>>>
>>>> [3] git://linux-arm.org/linux-jpb.git virtio-iommu/v0.9
>>>>     git://linux-arm.org/kvmtool-jpb.git virtio-iommu/v0.9
>>>>
>>>> [4] [RFC v9 00/17] VIRTIO-IOMMU device
>>>>     https://www.mail-archive.com/qemu-devel@nongnu.org/msg575578.html
>>>>
>>>> Jean-Philippe Brucker (7):
>>>>   dt-bindings: virtio-mmio: Add IOMMU description
>>>>   dt-bindings: virtio: Add virtio-pci-iommu node
>>>>   of: Allow the iommu-map property to omit untranslated devices
>>>>   PCI: OF: Initialize dev->fwnode appropriately
>>>>   iommu: Add virtio-iommu driver
>>>>   iommu/virtio: Add probe request
>>>>   iommu/virtio: Add event queue
>>>>
>>>>  .../devicetree/bindings/virtio/iommu.txt      |   66 +
>>>>  .../devicetree/bindings/virtio/mmio.txt       |   30 +
>>>>  MAINTAINERS                                   |    7 +
>>>>  drivers/iommu/Kconfig                         |   11 +
>>>>  drivers/iommu/Makefile                        |    1 +
>>>>  drivers/iommu/virtio-iommu.c                  | 1157 +++++++++++++++++
>>>>  drivers/of/base.c                             |   10 +-
>>>>  drivers/pci/of.c                              |    7 +
>>>>  include/uapi/linux/virtio_ids.h               |    1 +
>>>>  include/uapi/linux/virtio_iommu.h             |  161 +++
>>>>  10 files changed, 1448 insertions(+), 3 deletions(-)
>>>>  create mode 100644 Documentation/devicetree/bindings/virtio/iommu.txt
>>>>  create mode 100644 drivers/iommu/virtio-iommu.c
>>>>  create mode 100644 include/uapi/linux/virtio_iommu.h
>>>>
>>>> -- 
>>>> 2.19.1
Eric Auger Dec. 3, 2018, 1:27 p.m. UTC | #7
Hi Michael,

On 11/27/18 6:16 PM, Michael S. Tsirkin wrote:
> On Tue, Nov 27, 2018 at 06:09:25PM +0100, Auger Eric wrote:
>> Hi Michael,
>>
>> On 11/27/18 5:53 PM, Michael S. Tsirkin wrote:
>>> On Thu, Nov 22, 2018 at 07:37:54PM +0000, Jean-Philippe Brucker wrote:
>>>> Implement the virtio-iommu driver, following specification v0.9 [1].
>>>>
>>>> Since v4 [2] I fixed the issues reported by Eric, and added Reviewed-by
>>>> from Eric and Rob. Thanks!
>>>>
>>>> I changed the specification to fix one inconsistency discussed in v4.
>>>> That the device fills the probe buffer with zeroes is now a "SHOULD"
>>>> instead of a "MAY", since it's the only way for the driver to know if
>>>> the device wrote the status. Existing devices already do this. In
>>>> addition the device now needs to fill the three padding bytes at the
>>>> tail with zeroes.
>>>>
>>>> You can find Linux driver and kvmtool device on branches
>>>> virtio-iommu/v0.9 [3]. I also lightly tested with Eric's latest QEMU
>>>> device [4].
>>>
>>> I tried to get this to work on my x86 box but without
>>> success. Any hints? Does this have to do with the IORT table?
>>> I think we really should just reserve our own table ID
>>> and avoid the pain of trying to add things to the IORT spec.
>>> I'm reluctant to merge lots of code that I can't easily test.
>>> Again, if we found a way to push more configuration into
>>> virtio config space the problem space would be smaller.
>>
>> You can at least test it with QEMU ARM virt in TCG mode.
> 
> It's slow enough that I generally just focus on KVM.
> 
>> Then I have
>> worked on the IORT integration in PC/Q35 but this is not yet functional.
>> I need to debug what's wrong on guest ACPI probing. I plan to work on
>> this this week.
>>
>> Thanks
>>
>> Eric
> 
> Sounds good. Did you need to make changes to IORT?  I don't remember. If
> yes it would be very easy to just have a virtio specific ACPI table -
> I am assuming ARM guys will be just as hostile to virt changes
> to IORT as they were to minor changes to ARM vIOMMU.

I eventually succeeded to prototype the IORT integration on x86. Please
find below the branches to use for testing:

- QEMU: https://github.com/eauger/qemu.git v3.1.0-rc3-virtio-iommu-v0.9-x86
  On top of "[RFC v9 00/17] VIRTIO-IOMMU device", I added the IORT build
for pc machine
- LINUX GUEST: https://github.com/eauger/linux.git
virtio-iommu-v0.9-iort-x86
  Jean's virtio-iommu/devel branch + 2 hacks (to be discussed separately)
  Make sure the CONFIG_VIRTIO_IOMMU config is set

Used qemu cmd line featuring -device virtio-iommu-pci,addr=0xa:

./x86_64-softmmu/qemu-system-x86_64 -M
q35,accel=kvm,usb=off,sata=off,dump-guest-core=off -cpu
Haswell,-hle,-rtm -smp 4,sockets=4,cores=1,threads=1 -m 4G -display none
--enable-kvm -serial tcp:localhost:4444,server -device
virtio-iommu-pci,addr=0xa -trace
events=/home/augere/UPSTREAM/qemu/hw-virtio-iommu -qmp
unix:/home/augere/TEST/QEMU/qmp-sock,server,nowait -rtc
base=utc,driftfix=slew -global kvm-pit.lost_tick_policy=delay -realtime
mlock=off -no-hpet -no-shutdown -device
virtio-blk-pci,scsi=off,drive=drv0,id=virtio-disk0,bootindex=1,iommu_platform,disable-modern=off,disable-legacy=on
-drive
file=/home/augere/VM/IMAGES/vm0.qcow2,format=qcow2,if=none,id=drv0
-device
virtio-net-pci,bus=pcie.0,netdev=nic0,mac=6a:f5:10:b1:3d:d2,iommu_platform,disable-modern=off,disable-legacy=on
-netdev
tap,id=nic0,script=/home/augere/TEST/SCRIPTS/qemu-ifup,downscript=/home/augere/TEST/SCRIPTS/qemu-ifdown
-kernel
/home/augere/VM/BOOT/vmlinuz-4.20.0-rc3-guest-defconfig-virtio-iommu-0.9-x86+
-initrd
/home/augere/VM/BOOT/initrd.img-4.20.0-rc3-guest-defconfig-virtio-iommu-0.9-x86+
-append 'root=/dev/mapper/rhel_dhcp156--238-root ro crashkernel=auto
rd.lvm.lv=rhel_dhcp156-238/root rd.lvm.lv=rhel_dhcp156-238/swap rw
ignore_loglevel console=tty0 console=ttyS0,115200n8 serial acpi=force'
-net none -d guest_errors

I put sata=off otherwise I have early untranslated transactions that
prevent the guest from booting.

Please let me know if you encounter any issue reproducing the
environment on your side.

Thanks

Eric


> 
> 
>>>
>>>> [1] Virtio-iommu specification v0.9, sources, pdf and diff from v0.8
>>>>     git://linux-arm.org/virtio-iommu.git virtio-iommu/v0.9
>>>>     http://jpbrucker.net/virtio-iommu/spec/v0.9/virtio-iommu-v0.9.pdf
>>>>     http://jpbrucker.net/virtio-iommu/spec/diffs/virtio-iommu-pdf-diff-v0.8-v0.9.pdf
>>>>
>>>> [2] [PATCH v4 0/7] Add virtio-iommu driver
>>>>     https://lists.linuxfoundation.org/pipermail/iommu/2018-November/031074.html
>>>>
>>>> [3] git://linux-arm.org/linux-jpb.git virtio-iommu/v0.9
>>>>     git://linux-arm.org/kvmtool-jpb.git virtio-iommu/v0.9
>>>>
>>>> [4] [RFC v9 00/17] VIRTIO-IOMMU device
>>>>     https://www.mail-archive.com/qemu-devel@nongnu.org/msg575578.html
>>>>
>>>> Jean-Philippe Brucker (7):
>>>>   dt-bindings: virtio-mmio: Add IOMMU description
>>>>   dt-bindings: virtio: Add virtio-pci-iommu node
>>>>   of: Allow the iommu-map property to omit untranslated devices
>>>>   PCI: OF: Initialize dev->fwnode appropriately
>>>>   iommu: Add virtio-iommu driver
>>>>   iommu/virtio: Add probe request
>>>>   iommu/virtio: Add event queue
>>>>
>>>>  .../devicetree/bindings/virtio/iommu.txt      |   66 +
>>>>  .../devicetree/bindings/virtio/mmio.txt       |   30 +
>>>>  MAINTAINERS                                   |    7 +
>>>>  drivers/iommu/Kconfig                         |   11 +
>>>>  drivers/iommu/Makefile                        |    1 +
>>>>  drivers/iommu/virtio-iommu.c                  | 1157 +++++++++++++++++
>>>>  drivers/of/base.c                             |   10 +-
>>>>  drivers/pci/of.c                              |    7 +
>>>>  include/uapi/linux/virtio_ids.h               |    1 +
>>>>  include/uapi/linux/virtio_iommu.h             |  161 +++
>>>>  10 files changed, 1448 insertions(+), 3 deletions(-)
>>>>  create mode 100644 Documentation/devicetree/bindings/virtio/iommu.txt
>>>>  create mode 100644 drivers/iommu/virtio-iommu.c
>>>>  create mode 100644 include/uapi/linux/virtio_iommu.h
>>>>
>>>> -- 
>>>> 2.19.1