| Message ID | 20240221205926.40066-2-eric.auger@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | VIRTIO-IOMMU: Set default granule to host page size | expand |
On 2/21/24 10:58, Eric Auger wrote: > Introduce a new enum type property allowing to set an > IOMMU granule. Values are 4K, 16K, 64K and host. This > latter indicates the vIOMMU granule will matches the > host page size. > > A subsequent patch will add such a property to the > virtio-iommu device. > > Signed-off-by: Eric Auger <eric.auger@redhat.com> > --- > include/hw/qdev-properties-system.h | 3 +++ > include/hw/virtio/virtio-iommu.h | 11 +++++++++++ > hw/core/qdev-properties-system.c | 15 +++++++++++++++ > hw/virtio/virtio-iommu.c | 10 ++++++++++ > 4 files changed, 39 insertions(+) > > diff --git a/include/hw/qdev-properties-system.h b/include/hw/qdev-properties-system.h > index 06c359c190..626be87dd3 100644 > --- a/include/hw/qdev-properties-system.h > +++ b/include/hw/qdev-properties-system.h > @@ -8,6 +8,7 @@ extern const PropertyInfo qdev_prop_macaddr; > extern const PropertyInfo qdev_prop_reserved_region; > extern const PropertyInfo qdev_prop_multifd_compression; > extern const PropertyInfo qdev_prop_mig_mode; > +extern const PropertyInfo qdev_prop_granule_mode; > extern const PropertyInfo qdev_prop_losttickpolicy; > extern const PropertyInfo qdev_prop_blockdev_on_error; > extern const PropertyInfo qdev_prop_bios_chs_trans; > @@ -47,6 +48,8 @@ extern const PropertyInfo qdev_prop_iothread_vq_mapping_list; > #define DEFINE_PROP_MIG_MODE(_n, _s, _f, _d) \ > DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_mig_mode, \ > MigMode) > +#define DEFINE_PROP_GRANULE_MODE(_n, _s, _f, _d) \ > + DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_granule_mode, GranuleMode) > #define DEFINE_PROP_LOSTTICKPOLICY(_n, _s, _f, _d) \ > DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_losttickpolicy, \ > LostTickPolicy) > diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h > index 5fbe4677c2..a82c4fa471 100644 > --- a/include/hw/virtio/virtio-iommu.h > +++ b/include/hw/virtio/virtio-iommu.h > @@ -31,6 +31,16 @@ OBJECT_DECLARE_SIMPLE_TYPE(VirtIOIOMMU, VIRTIO_IOMMU) > > #define TYPE_VIRTIO_IOMMU_MEMORY_REGION "virtio-iommu-memory-region" > > +typedef enum GranuleMode { > + GRANULE_MODE_4K, > + GRANULE_MODE_16K, > + GRANULE_MODE_64K, > + GRANULE_MODE_HOST, > + GRANULE_MODE__MAX, > +} GranuleMode; > + > +extern const QEnumLookup GranuleMode_lookup; > + > typedef struct IOMMUDevice { > void *viommu; > PCIBus *bus; > @@ -67,6 +77,7 @@ struct VirtIOIOMMU { > Notifier machine_done; > bool granule_frozen; > uint8_t aw_bits; > + GranuleMode granule_mode; > }; > > #endif > diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c > index 1a396521d5..3a0b36a7a7 100644 > --- a/hw/core/qdev-properties-system.c > +++ b/hw/core/qdev-properties-system.c > @@ -34,6 +34,7 @@ > #include "net/net.h" > #include "hw/pci/pci.h" > #include "hw/pci/pcie.h" > +#include "hw/virtio/virtio-iommu.h" > #include "hw/i386/x86.h" > #include "util/block-helpers.h" > > @@ -679,6 +680,20 @@ const PropertyInfo qdev_prop_mig_mode = { > .set_default_value = qdev_propinfo_set_default_value_enum, > }; > > +/* --- GranuleMode --- */ > + > +QEMU_BUILD_BUG_ON(sizeof(GranuleMode) != sizeof(int)); > + > +const PropertyInfo qdev_prop_granule_mode = { > + .name = "GranuleMode", > + .description = "granule_mode values, " > + "4K, 16K, 64K, host", > + .enum_table = &GranuleMode_lookup, > + .get = qdev_propinfo_get_enum, > + .set = qdev_propinfo_set_enum, > + .set_default_value = qdev_propinfo_set_default_value_enum, > +}; > + > /* --- Reserved Region --- */ > > /* > diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c > index 2ec5ef3cd1..2ce5839c60 100644 > --- a/hw/virtio/virtio-iommu.c > +++ b/hw/virtio/virtio-iommu.c > @@ -44,6 +44,16 @@ > #define VIOMMU_DEFAULT_QUEUE_SIZE 256 > #define VIOMMU_PROBE_SIZE 512 > > +const QEnumLookup GranuleMode_lookup = { > + .array = (const char *const[]) { > + [GRANULE_MODE_4K] = "4K", > + [GRANULE_MODE_16K] = "16K", > + [GRANULE_MODE_64K] = "64K", > + [GRANULE_MODE_HOST] = "host", > + }, > + .size = GRANULE_MODE__MAX > +}; Does it ever make sense to use anything other than host? Especially since 4k fails on a 64k host, as you report. Why would we want to be able to select a granule that doesn't work? There are a few older hosts with 8k pages, e.g. Alpha, MIPS, Sparc. r~
Hi Richard, On 2/21/24 22:58, Richard Henderson wrote: > On 2/21/24 10:58, Eric Auger wrote: >> Introduce a new enum type property allowing to set an >> IOMMU granule. Values are 4K, 16K, 64K and host. This >> latter indicates the vIOMMU granule will matches the >> host page size. >> >> A subsequent patch will add such a property to the >> virtio-iommu device. >> >> Signed-off-by: Eric Auger <eric.auger@redhat.com> >> --- >> include/hw/qdev-properties-system.h | 3 +++ >> include/hw/virtio/virtio-iommu.h | 11 +++++++++++ >> hw/core/qdev-properties-system.c | 15 +++++++++++++++ >> hw/virtio/virtio-iommu.c | 10 ++++++++++ >> 4 files changed, 39 insertions(+) >> >> diff --git a/include/hw/qdev-properties-system.h >> b/include/hw/qdev-properties-system.h >> index 06c359c190..626be87dd3 100644 >> --- a/include/hw/qdev-properties-system.h >> +++ b/include/hw/qdev-properties-system.h >> @@ -8,6 +8,7 @@ extern const PropertyInfo qdev_prop_macaddr; >> extern const PropertyInfo qdev_prop_reserved_region; >> extern const PropertyInfo qdev_prop_multifd_compression; >> extern const PropertyInfo qdev_prop_mig_mode; >> +extern const PropertyInfo qdev_prop_granule_mode; >> extern const PropertyInfo qdev_prop_losttickpolicy; >> extern const PropertyInfo qdev_prop_blockdev_on_error; >> extern const PropertyInfo qdev_prop_bios_chs_trans; >> @@ -47,6 +48,8 @@ extern const PropertyInfo >> qdev_prop_iothread_vq_mapping_list; >> #define DEFINE_PROP_MIG_MODE(_n, _s, _f, _d) \ >> DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_mig_mode, \ >> MigMode) >> +#define DEFINE_PROP_GRANULE_MODE(_n, _s, _f, _d) \ >> + DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_granule_mode, >> GranuleMode) >> #define DEFINE_PROP_LOSTTICKPOLICY(_n, _s, _f, _d) \ >> DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_losttickpolicy, \ >> LostTickPolicy) >> diff --git a/include/hw/virtio/virtio-iommu.h >> b/include/hw/virtio/virtio-iommu.h >> index 5fbe4677c2..a82c4fa471 100644 >> --- a/include/hw/virtio/virtio-iommu.h >> +++ b/include/hw/virtio/virtio-iommu.h >> @@ -31,6 +31,16 @@ OBJECT_DECLARE_SIMPLE_TYPE(VirtIOIOMMU, VIRTIO_IOMMU) >> #define TYPE_VIRTIO_IOMMU_MEMORY_REGION "virtio-iommu-memory-region" >> +typedef enum GranuleMode { >> + GRANULE_MODE_4K, >> + GRANULE_MODE_16K, >> + GRANULE_MODE_64K, >> + GRANULE_MODE_HOST, >> + GRANULE_MODE__MAX, >> +} GranuleMode; >> + >> +extern const QEnumLookup GranuleMode_lookup; >> + >> typedef struct IOMMUDevice { >> void *viommu; >> PCIBus *bus; >> @@ -67,6 +77,7 @@ struct VirtIOIOMMU { >> Notifier machine_done; >> bool granule_frozen; >> uint8_t aw_bits; >> + GranuleMode granule_mode; >> }; >> #endif >> diff --git a/hw/core/qdev-properties-system.c >> b/hw/core/qdev-properties-system.c >> index 1a396521d5..3a0b36a7a7 100644 >> --- a/hw/core/qdev-properties-system.c >> +++ b/hw/core/qdev-properties-system.c >> @@ -34,6 +34,7 @@ >> #include "net/net.h" >> #include "hw/pci/pci.h" >> #include "hw/pci/pcie.h" >> +#include "hw/virtio/virtio-iommu.h" >> #include "hw/i386/x86.h" >> #include "util/block-helpers.h" >> @@ -679,6 +680,20 @@ const PropertyInfo qdev_prop_mig_mode = { >> .set_default_value = qdev_propinfo_set_default_value_enum, >> }; >> +/* --- GranuleMode --- */ >> + >> +QEMU_BUILD_BUG_ON(sizeof(GranuleMode) != sizeof(int)); >> + >> +const PropertyInfo qdev_prop_granule_mode = { >> + .name = "GranuleMode", >> + .description = "granule_mode values, " >> + "4K, 16K, 64K, host", >> + .enum_table = &GranuleMode_lookup, >> + .get = qdev_propinfo_get_enum, >> + .set = qdev_propinfo_set_enum, >> + .set_default_value = qdev_propinfo_set_default_value_enum, >> +}; >> + >> /* --- Reserved Region --- */ >> /* >> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c >> index 2ec5ef3cd1..2ce5839c60 100644 >> --- a/hw/virtio/virtio-iommu.c >> +++ b/hw/virtio/virtio-iommu.c >> @@ -44,6 +44,16 @@ >> #define VIOMMU_DEFAULT_QUEUE_SIZE 256 >> #define VIOMMU_PROBE_SIZE 512 >> +const QEnumLookup GranuleMode_lookup = { >> + .array = (const char *const[]) { >> + [GRANULE_MODE_4K] = "4K", >> + [GRANULE_MODE_16K] = "16K", >> + [GRANULE_MODE_64K] = "64K", >> + [GRANULE_MODE_HOST] = "host", >> + }, >> + .size = GRANULE_MODE__MAX >> +}; > > Does it ever make sense to use anything other than host? > Especially since 4k fails on a 64k host, as you report. Let me reuse Jean-Philippe's support matrix: Before this series, 4KB granule was always used by the virtio-iommu. The support matrix was: Host | Guest | virtio-net | IGB passthrough 4k | 4k | Y | Y 64k | 64k | Y | N 64k | 4k | Y | N 4k | 64k | Y | Y with this series, host becomes the default. It fixes the 64KB/64KB config by default. It allows 64KB/4KB (virtio only) with manual 4K granule setting. If we don't have this 4K granule we cannot support that use case. Host | Guest | virtio-net | IGB passthrough 4k | 4k | Y | Y 64k | 64k | Y | Y 64k | 4k | 4K | N 4k | 64k | Y | Y Besides I need 4KB to handle the compat for older machine types. > Why would we want to be able to select a granule that doesn't work? > There are a few older hosts with 8k pages, e.g. Alpha, MIPS, Sparc. OK. those are not integrated with virtio-iommu but I can definitively add 8K Eric > > > r~ >
diff --git a/include/hw/qdev-properties-system.h b/include/hw/qdev-properties-system.h index 06c359c190..626be87dd3 100644 --- a/include/hw/qdev-properties-system.h +++ b/include/hw/qdev-properties-system.h @@ -8,6 +8,7 @@ extern const PropertyInfo qdev_prop_macaddr; extern const PropertyInfo qdev_prop_reserved_region; extern const PropertyInfo qdev_prop_multifd_compression; extern const PropertyInfo qdev_prop_mig_mode; +extern const PropertyInfo qdev_prop_granule_mode; extern const PropertyInfo qdev_prop_losttickpolicy; extern const PropertyInfo qdev_prop_blockdev_on_error; extern const PropertyInfo qdev_prop_bios_chs_trans; @@ -47,6 +48,8 @@ extern const PropertyInfo qdev_prop_iothread_vq_mapping_list; #define DEFINE_PROP_MIG_MODE(_n, _s, _f, _d) \ DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_mig_mode, \ MigMode) +#define DEFINE_PROP_GRANULE_MODE(_n, _s, _f, _d) \ + DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_granule_mode, GranuleMode) #define DEFINE_PROP_LOSTTICKPOLICY(_n, _s, _f, _d) \ DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_losttickpolicy, \ LostTickPolicy) diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h index 5fbe4677c2..a82c4fa471 100644 --- a/include/hw/virtio/virtio-iommu.h +++ b/include/hw/virtio/virtio-iommu.h @@ -31,6 +31,16 @@ OBJECT_DECLARE_SIMPLE_TYPE(VirtIOIOMMU, VIRTIO_IOMMU) #define TYPE_VIRTIO_IOMMU_MEMORY_REGION "virtio-iommu-memory-region" +typedef enum GranuleMode { + GRANULE_MODE_4K, + GRANULE_MODE_16K, + GRANULE_MODE_64K, + GRANULE_MODE_HOST, + GRANULE_MODE__MAX, +} GranuleMode; + +extern const QEnumLookup GranuleMode_lookup; + typedef struct IOMMUDevice { void *viommu; PCIBus *bus; @@ -67,6 +77,7 @@ struct VirtIOIOMMU { Notifier machine_done; bool granule_frozen; uint8_t aw_bits; + GranuleMode granule_mode; }; #endif diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c index 1a396521d5..3a0b36a7a7 100644 --- a/hw/core/qdev-properties-system.c +++ b/hw/core/qdev-properties-system.c @@ -34,6 +34,7 @@ #include "net/net.h" #include "hw/pci/pci.h" #include "hw/pci/pcie.h" +#include "hw/virtio/virtio-iommu.h" #include "hw/i386/x86.h" #include "util/block-helpers.h" @@ -679,6 +680,20 @@ const PropertyInfo qdev_prop_mig_mode = { .set_default_value = qdev_propinfo_set_default_value_enum, }; +/* --- GranuleMode --- */ + +QEMU_BUILD_BUG_ON(sizeof(GranuleMode) != sizeof(int)); + +const PropertyInfo qdev_prop_granule_mode = { + .name = "GranuleMode", + .description = "granule_mode values, " + "4K, 16K, 64K, host", + .enum_table = &GranuleMode_lookup, + .get = qdev_propinfo_get_enum, + .set = qdev_propinfo_set_enum, + .set_default_value = qdev_propinfo_set_default_value_enum, +}; + /* --- Reserved Region --- */ /* diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c index 2ec5ef3cd1..2ce5839c60 100644 --- a/hw/virtio/virtio-iommu.c +++ b/hw/virtio/virtio-iommu.c @@ -44,6 +44,16 @@ #define VIOMMU_DEFAULT_QUEUE_SIZE 256 #define VIOMMU_PROBE_SIZE 512 +const QEnumLookup GranuleMode_lookup = { + .array = (const char *const[]) { + [GRANULE_MODE_4K] = "4K", + [GRANULE_MODE_16K] = "16K", + [GRANULE_MODE_64K] = "64K", + [GRANULE_MODE_HOST] = "host", + }, + .size = GRANULE_MODE__MAX +}; + typedef struct VirtIOIOMMUDomain { uint32_t id; bool bypass;
Introduce a new enum type property allowing to set an IOMMU granule. Values are 4K, 16K, 64K and host. This latter indicates the vIOMMU granule will matches the host page size. A subsequent patch will add such a property to the virtio-iommu device. Signed-off-by: Eric Auger <eric.auger@redhat.com> --- include/hw/qdev-properties-system.h | 3 +++ include/hw/virtio/virtio-iommu.h | 11 +++++++++++ hw/core/qdev-properties-system.c | 15 +++++++++++++++ hw/virtio/virtio-iommu.c | 10 ++++++++++ 4 files changed, 39 insertions(+)