Re: [PATCH v4 3/3] virtio-iommu: Change the default granule to the host page size

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

Hi Eric,

On 23/2/24 08:27, Eric Auger wrote:
> We used to set the default granule to 4KB but with VFIO assignment
> it makes more sense to use the actual host page size.
> 
> Indeed when hotplugging a VFIO device protected by a virtio-iommu
> on a 64kB/64kB host/guest config, we current get a qemu crash:
> 
> "vfio: DMA mapping failed, unable to continue"
> 
> This is due to the hot-attached VFIO device calling
> memory_region_iommu_set_page_size_mask() with 64kB granule
> whereas the virtio-iommu granule was already frozen to 4KB on
> machine init done.
> 
> Set the granule property to "host" and introduce a new compat.
> 
> Note that the new default will prevent 4kB guest on 64kB host
> because the granule will be set to 64kB which would be larger
> than the guest page size. In that situation, the virtio-iommu
> driver fails on viommu_domain_finalise() with
> "granule 0x10000 larger than system page size 0x1000".
> 
> In that case the workaround is to request 4K granule.
> 
> The current limitation of global granule in the virtio-iommu
> should be removed and turned into per domain granule. But
> until we get this upgraded, this new default is probably
> better because I don't think anyone is currently interested in
> running a 4KB page size guest with virtio-iommu on a 64KB host.
> However supporting 64kB guest on 64kB host with virtio-iommu and
> VFIO looks a more important feature.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>   hw/core/machine.c        | 1 +
>   hw/virtio/virtio-iommu.c | 2 +-
>   2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 70ac96954c..38851df4b8 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -35,6 +35,7 @@
>   
>   GlobalProperty hw_compat_8_2[] = {
>       { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "64" },
> +    { TYPE_VIRTIO_IOMMU_PCI, "granule", "4K" },

IIUC the current value is qemu_target_page_mask(), not 4KiB.
Could this be an issue for arm / mips / mips / sparc64 guests?

>   };
>   const size_t hw_compat_8_2_len = G_N_ELEMENTS(hw_compat_8_2);
>   
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index 0461b87ef2..e9e44a8ad8 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -1558,7 +1558,7 @@ static Property virtio_iommu_properties[] = {
>       DEFINE_PROP_BOOL("boot-bypass", VirtIOIOMMU, boot_bypass, true),
>       DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 0),
>       DEFINE_PROP_GRANULE_MODE("granule", VirtIOIOMMU, granule_mode,
> -                             GRANULE_MODE_4K),
> +                             GRANULE_MODE_HOST),
>       DEFINE_PROP_END_OF_LIST(),
>   };
>