[PATCH v5 0/5] VIRTIO-IOMMU probe request support and MSI bypass on ARM

Eric Auger posted 5 patches 3 years, 10 months ago
Test FreeBSD passed
Test docker-quick@centos7 passed
Test checkpatch passed
Test docker-mingw@fedora passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200624132625.27453-1-eric.auger@redhat.com
Maintainers: Eric Auger <eric.auger@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, "Michael S. Tsirkin" <mst@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>
There is a newer version of this series
include/exec/memory.h            |   6 ++
include/hw/arm/virt.h            |   7 ++
include/hw/qdev-properties.h     |   3 +
include/hw/virtio/virtio-iommu.h |   2 +
include/qemu/typedefs.h          |   1 +
hw/arm/virt.c                    |  18 +++++
hw/core/qdev-properties.c        |  89 ++++++++++++++++++++++++
hw/virtio/virtio-iommu-pci.c     |   3 +
hw/virtio/virtio-iommu.c         | 112 +++++++++++++++++++++++++++++--
hw/virtio/trace-events           |   1 +
10 files changed, 238 insertions(+), 4 deletions(-)
[PATCH v5 0/5] VIRTIO-IOMMU probe request support and MSI bypass on ARM
Posted by Eric Auger 3 years, 10 months ago
By default the virtio-iommu translates MSI transactions. This
behavior is inherited from ARM SMMU. However the virt machine
code knows where the MSI doorbells are, so we can easily
declare those regions as VIRTIO_IOMMU_RESV_MEM_T_MSI. With that
setting the guest iommu subsystem will not need to map MSIs.
This setup will simplify the VFIO integration.

In this series, the ITS or GICV2M doorbells are declared as
HW MSI regions to be bypassed by the VIRTIO-IOMMU.

This also paves the way to the x86 integration where the MSI
region, [0xFEE00000,0xFEEFFFFF], will be exposed by the q35
machine.  However this will be handled in a separate series
when not-DT support gets resolved.

Best Regards

Eric

This series can be found at:
https://github.com/eauger/qemu/tree/v5.0.0-virtio-iommu-msi-bypass-v5

History:

v4 -> v5:
- Take into account some additional comments from Markus:
  - reserved region type becomes an unsigned + some comment/desc
    rewording
  - assert if the type is not RESERVED or MSI

v3 -> v4:
- collected Jean and markus's R-bs
- tool into account all Markus' comments in [1/5] (except removal of
  goto)
- use ':' as delimitor instead of commas
- add example in 4/5 commit message as suggested by Markus

v2 -> v3:
- Introduce VIRT_MSI_CTRL_NONE in VirtMSIControllerType
- do not fill the remainder of the probe buffer

v1 -> v2:
- check which MSI controller is in use and advertise the
  corresponding MSI doorbell
- managed for both ITS and GICv2M
- various fixes spotted by Peter and Jean-Philippe, see
  individual logs

v1: Most of those patches were respinned from
  [PATCH for-5.0 v11 00/20] VIRTIO-IOMMU device
  except the last one which is new


Eric Auger (5):
  qdev: Introduce DEFINE_PROP_RESERVED_REGION
  virtio-iommu: Implement RESV_MEM probe request
  virtio-iommu: Handle reserved regions in the translation process
  virtio-iommu-pci: Add array of Interval properties
  hw/arm/virt: Let the virtio-iommu bypass MSIs

 include/exec/memory.h            |   6 ++
 include/hw/arm/virt.h            |   7 ++
 include/hw/qdev-properties.h     |   3 +
 include/hw/virtio/virtio-iommu.h |   2 +
 include/qemu/typedefs.h          |   1 +
 hw/arm/virt.c                    |  18 +++++
 hw/core/qdev-properties.c        |  89 ++++++++++++++++++++++++
 hw/virtio/virtio-iommu-pci.c     |   3 +
 hw/virtio/virtio-iommu.c         | 112 +++++++++++++++++++++++++++++--
 hw/virtio/trace-events           |   1 +
 10 files changed, 238 insertions(+), 4 deletions(-)

-- 
2.20.1


Re: [PATCH v5 0/5] VIRTIO-IOMMU probe request support and MSI bypass on ARM
Posted by Michael S. Tsirkin 3 years, 10 months ago
On Wed, Jun 24, 2020 at 03:26:20PM +0200, Eric Auger wrote:
> By default the virtio-iommu translates MSI transactions. This
> behavior is inherited from ARM SMMU. However the virt machine
> code knows where the MSI doorbells are, so we can easily
> declare those regions as VIRTIO_IOMMU_RESV_MEM_T_MSI. With that
> setting the guest iommu subsystem will not need to map MSIs.
> This setup will simplify the VFIO integration.
> 
> In this series, the ITS or GICV2M doorbells are declared as
> HW MSI regions to be bypassed by the VIRTIO-IOMMU.
> 
> This also paves the way to the x86 integration where the MSI
> region, [0xFEE00000,0xFEEFFFFF], will be exposed by the q35
> machine.  However this will be handled in a separate series
> when not-DT support gets resolved.
> 
> Best Regards
> 
> Eric


Virtio bits look OK:

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

Who's merging this? ARM tree?

> This series can be found at:
> https://github.com/eauger/qemu/tree/v5.0.0-virtio-iommu-msi-bypass-v5
> 
> History:
> 
> v4 -> v5:
> - Take into account some additional comments from Markus:
>   - reserved region type becomes an unsigned + some comment/desc
>     rewording
>   - assert if the type is not RESERVED or MSI
> 
> v3 -> v4:
> - collected Jean and markus's R-bs
> - tool into account all Markus' comments in [1/5] (except removal of
>   goto)
> - use ':' as delimitor instead of commas
> - add example in 4/5 commit message as suggested by Markus
> 
> v2 -> v3:
> - Introduce VIRT_MSI_CTRL_NONE in VirtMSIControllerType
> - do not fill the remainder of the probe buffer
> 
> v1 -> v2:
> - check which MSI controller is in use and advertise the
>   corresponding MSI doorbell
> - managed for both ITS and GICv2M
> - various fixes spotted by Peter and Jean-Philippe, see
>   individual logs
> 
> v1: Most of those patches were respinned from
>   [PATCH for-5.0 v11 00/20] VIRTIO-IOMMU device
>   except the last one which is new
> 
> 
> Eric Auger (5):
>   qdev: Introduce DEFINE_PROP_RESERVED_REGION
>   virtio-iommu: Implement RESV_MEM probe request
>   virtio-iommu: Handle reserved regions in the translation process
>   virtio-iommu-pci: Add array of Interval properties
>   hw/arm/virt: Let the virtio-iommu bypass MSIs
> 
>  include/exec/memory.h            |   6 ++
>  include/hw/arm/virt.h            |   7 ++
>  include/hw/qdev-properties.h     |   3 +
>  include/hw/virtio/virtio-iommu.h |   2 +
>  include/qemu/typedefs.h          |   1 +
>  hw/arm/virt.c                    |  18 +++++
>  hw/core/qdev-properties.c        |  89 ++++++++++++++++++++++++
>  hw/virtio/virtio-iommu-pci.c     |   3 +
>  hw/virtio/virtio-iommu.c         | 112 +++++++++++++++++++++++++++++--
>  hw/virtio/trace-events           |   1 +
>  10 files changed, 238 insertions(+), 4 deletions(-)
> 
> -- 
> 2.20.1


Re: [PATCH v5 0/5] VIRTIO-IOMMU probe request support and MSI bypass on ARM
Posted by Peter Maydell 3 years, 10 months ago
On Wed, 24 Jun 2020 at 16:16, Michael S. Tsirkin <mst@redhat.com> wrote:
> Virtio bits look OK:
>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

I can take it via the arm tree once it's ready. I had a
comment on patch 5 and it looks like Markus had a question
about patch 2, though.

thanks
-- PMM

Re: [PATCH v5 0/5] VIRTIO-IOMMU probe request support and MSI bypass on ARM
Posted by Michael S. Tsirkin 3 years, 10 months ago
On Wed, Jun 24, 2020 at 03:26:20PM +0200, Eric Auger wrote:
> By default the virtio-iommu translates MSI transactions. This
> behavior is inherited from ARM SMMU. However the virt machine
> code knows where the MSI doorbells are, so we can easily
> declare those regions as VIRTIO_IOMMU_RESV_MEM_T_MSI. With that
> setting the guest iommu subsystem will not need to map MSIs.
> This setup will simplify the VFIO integration.
> 
> In this series, the ITS or GICV2M doorbells are declared as
> HW MSI regions to be bypassed by the VIRTIO-IOMMU.


> This also paves the way to the x86 integration where the MSI
> region, [0xFEE00000,0xFEEFFFFF], will be exposed by the q35
> machine.  However this will be handled in a separate series
> when not-DT support gets resolved.

What's going on with that btw?
I think the next step is to put the spec up for virtio tc vote,
then we can merge that, right? acpi parts will need to be
ratified by the acpi sig.

> Best Regards
> 
> Eric
> 
> This series can be found at:
> https://github.com/eauger/qemu/tree/v5.0.0-virtio-iommu-msi-bypass-v5
> 
> History:
> 
> v4 -> v5:
> - Take into account some additional comments from Markus:
>   - reserved region type becomes an unsigned + some comment/desc
>     rewording
>   - assert if the type is not RESERVED or MSI
> 
> v3 -> v4:
> - collected Jean and markus's R-bs
> - tool into account all Markus' comments in [1/5] (except removal of
>   goto)
> - use ':' as delimitor instead of commas
> - add example in 4/5 commit message as suggested by Markus
> 
> v2 -> v3:
> - Introduce VIRT_MSI_CTRL_NONE in VirtMSIControllerType
> - do not fill the remainder of the probe buffer
> 
> v1 -> v2:
> - check which MSI controller is in use and advertise the
>   corresponding MSI doorbell
> - managed for both ITS and GICv2M
> - various fixes spotted by Peter and Jean-Philippe, see
>   individual logs
> 
> v1: Most of those patches were respinned from
>   [PATCH for-5.0 v11 00/20] VIRTIO-IOMMU device
>   except the last one which is new
> 
> 
> Eric Auger (5):
>   qdev: Introduce DEFINE_PROP_RESERVED_REGION
>   virtio-iommu: Implement RESV_MEM probe request
>   virtio-iommu: Handle reserved regions in the translation process
>   virtio-iommu-pci: Add array of Interval properties
>   hw/arm/virt: Let the virtio-iommu bypass MSIs
> 
>  include/exec/memory.h            |   6 ++
>  include/hw/arm/virt.h            |   7 ++
>  include/hw/qdev-properties.h     |   3 +
>  include/hw/virtio/virtio-iommu.h |   2 +
>  include/qemu/typedefs.h          |   1 +
>  hw/arm/virt.c                    |  18 +++++
>  hw/core/qdev-properties.c        |  89 ++++++++++++++++++++++++
>  hw/virtio/virtio-iommu-pci.c     |   3 +
>  hw/virtio/virtio-iommu.c         | 112 +++++++++++++++++++++++++++++--
>  hw/virtio/trace-events           |   1 +
>  10 files changed, 238 insertions(+), 4 deletions(-)
> 
> -- 
> 2.20.1


Re: [PATCH v5 0/5] VIRTIO-IOMMU probe request support and MSI bypass on ARM
Posted by Jean-Philippe Brucker 3 years, 10 months ago
On Wed, Jun 24, 2020 at 09:47:59AM -0400, Michael S. Tsirkin wrote:
> On Wed, Jun 24, 2020 at 03:26:20PM +0200, Eric Auger wrote:
> > By default the virtio-iommu translates MSI transactions. This
> > behavior is inherited from ARM SMMU. However the virt machine
> > code knows where the MSI doorbells are, so we can easily
> > declare those regions as VIRTIO_IOMMU_RESV_MEM_T_MSI. With that
> > setting the guest iommu subsystem will not need to map MSIs.
> > This setup will simplify the VFIO integration.
> > 
> > In this series, the ITS or GICV2M doorbells are declared as
> > HW MSI regions to be bypassed by the VIRTIO-IOMMU.
> 
> 
> > This also paves the way to the x86 integration where the MSI
> > region, [0xFEE00000,0xFEEFFFFF], will be exposed by the q35
> > machine.  However this will be handled in a separate series
> > when not-DT support gets resolved.
> 
> What's going on with that btw?
> I think the next step is to put the spec up for virtio tc vote,
> then we can merge that, right? acpi parts will need to be
> ratified by the acpi sig.

Yes I'm being unreasonably slow on this. I'll look at both the virtio spec
and ACPI after coming back from Holiday in July. I'm afraid the situation
for the built-in description will be the same on the Linux side (asked to
use ACPI instead), but it's worth retrying as we have some progress on
ACPI support.

Thanks,
Jean

> 
> > Best Regards
> > 
> > Eric
> > 
> > This series can be found at:
> > https://github.com/eauger/qemu/tree/v5.0.0-virtio-iommu-msi-bypass-v5
> > 
> > History:
> > 
> > v4 -> v5:
> > - Take into account some additional comments from Markus:
> >   - reserved region type becomes an unsigned + some comment/desc
> >     rewording
> >   - assert if the type is not RESERVED or MSI
> > 
> > v3 -> v4:
> > - collected Jean and markus's R-bs
> > - tool into account all Markus' comments in [1/5] (except removal of
> >   goto)
> > - use ':' as delimitor instead of commas
> > - add example in 4/5 commit message as suggested by Markus
> > 
> > v2 -> v3:
> > - Introduce VIRT_MSI_CTRL_NONE in VirtMSIControllerType
> > - do not fill the remainder of the probe buffer
> > 
> > v1 -> v2:
> > - check which MSI controller is in use and advertise the
> >   corresponding MSI doorbell
> > - managed for both ITS and GICv2M
> > - various fixes spotted by Peter and Jean-Philippe, see
> >   individual logs
> > 
> > v1: Most of those patches were respinned from
> >   [PATCH for-5.0 v11 00/20] VIRTIO-IOMMU device
> >   except the last one which is new
> > 
> > 
> > Eric Auger (5):
> >   qdev: Introduce DEFINE_PROP_RESERVED_REGION
> >   virtio-iommu: Implement RESV_MEM probe request
> >   virtio-iommu: Handle reserved regions in the translation process
> >   virtio-iommu-pci: Add array of Interval properties
> >   hw/arm/virt: Let the virtio-iommu bypass MSIs
> > 
> >  include/exec/memory.h            |   6 ++
> >  include/hw/arm/virt.h            |   7 ++
> >  include/hw/qdev-properties.h     |   3 +
> >  include/hw/virtio/virtio-iommu.h |   2 +
> >  include/qemu/typedefs.h          |   1 +
> >  hw/arm/virt.c                    |  18 +++++
> >  hw/core/qdev-properties.c        |  89 ++++++++++++++++++++++++
> >  hw/virtio/virtio-iommu-pci.c     |   3 +
> >  hw/virtio/virtio-iommu.c         | 112 +++++++++++++++++++++++++++++--
> >  hw/virtio/trace-events           |   1 +
> >  10 files changed, 238 insertions(+), 4 deletions(-)
> > 
> > -- 
> > 2.20.1
>