1. Patchew
  2. QEMU
  3. [PATCH v4 0/3] VIRTIO-IOMMU: Set default granule to host page size
  4. View patch

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

Eric Auger posted 3 patches 9 months, 1 week ago
There is a newer version of this series
[PATCH v4 3/3] virtio-iommu: Change the default granule to the host page size
Posted by Eric Auger 9 months, 1 week ago 
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" },
 };
 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(),
 };
 
-- 
2.41.0
Re: [PATCH v4 3/3] virtio-iommu: Change the default granule to the host page size
Posted by Philippe Mathieu-Daudé 9 months, 1 week ago 
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(),
>   };
>
Re: [PATCH v4 3/3] virtio-iommu: Change the default granule to the host page size
Posted by Eric Auger 9 months, 1 week ago 

On 2/23/24 09:08, Philippe Mathieu-Daudé wrote:
> 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?
The virtio-iommu currently is only supported on ARM and x86 where
qemu_target_page_mask() corresponds to a 4K granule. So I don't think it
makes any issue.

Thanks

Eric
>
>>   };
>>   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(),
>>   };
>>   
>
Re: [PATCH v4 3/3] virtio-iommu: Change the default granule to the host page size
Posted by Philippe Mathieu-Daudé 9 months ago 
On 23/2/24 09:38, Eric Auger wrote:
> 
> 
> On 2/23/24 09:08, Philippe Mathieu-Daudé wrote:
>> 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?
> The virtio-iommu currently is only supported on ARM and x86 where
> qemu_target_page_mask() corresponds to a 4K granule. So I don't think it
> makes any issue.

Thanks, please explicit that in the commit description.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Regards,

Phil.

>>>    };
>>>    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(),
>>>    };
>>>    
>>
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.