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.au...@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~
>
