hw/arm/virt: Support larger highmem MMIO regions

Commit Message

Matt Ochs Jan. 28, 2025, 4:02 p.m. UTC

The MMIO region size required to support virtualized environments with
large PCI BAR regions can exceed the hardcoded limit configured in QEMU.
For example, a VM with multiple NVIDIA Grace-Hopper GPUs passed through
requires more MMIO memory than the amount provided by VIRT_HIGH_PCIE_MMIO
(currently 512GB). Instead of updating VIRT_HIGH_PCIE_MMIO, introduce a
new parameter, highmem-mmio-size, that specifies the MMIO size required
to support the VM configuration.

Example usage with 1TB MMIO region size:
	-machine virt,gic-version=3,highmem-mmio-size=1099511627776

Signed-off-by: Matthew R. Ochs <[e-mail]>
---
 docs/system/arm/virt.rst |  4 ++++
 hw/arm/virt.c            | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

Comments

Shameerali Kolothum Thodi Jan. 28, 2025, 5:36 p.m. UTC | #1

> -----Original Message-----
> From: Matthew R. Ochs <[e-mail]>
> Sent: Tuesday, January 28, 2025 4:03 PM
> To: [e-mail]; Shameerali Kolothum Thodi
> <[e-mail]>; [e-mail]
> Cc: [e-mail]; [e-mail]; [e-mail];
> [e-mail]
> Subject: [PATCH] hw/arm/virt: Support larger highmem MMIO regions
> 
> The MMIO region size required to support virtualized environments with
> large PCI BAR regions can exceed the hardcoded limit configured in QEMU.
> For example, a VM with multiple NVIDIA Grace-Hopper GPUs passed
> through
> requires more MMIO memory than the amount provided by
> VIRT_HIGH_PCIE_MMIO
> (currently 512GB). Instead of updating VIRT_HIGH_PCIE_MMIO, introduce a
> new parameter, highmem-mmio-size, that specifies the MMIO size required
> to support the VM configuration.
> 
> Example usage with 1TB MMIO region size:
> 	-machine virt,gic-version=3,highmem-mmio-size=1099511627776

I guess you could do highmem-mmio-size=1024G as well.

> 
> Signed-off-by: Matthew R. Ochs <[e-mail]>
> ---
>  docs/system/arm/virt.rst |  4 ++++
>  hw/arm/virt.c            | 36 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 40 insertions(+)
> 
> diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst
> index e67e7f0f7c50..36344554788a 100644
> --- a/docs/system/arm/virt.rst
> +++ b/docs/system/arm/virt.rst
> @@ -138,6 +138,10 @@ highmem-mmio
>    Set ``on``/``off`` to enable/disable the high memory region for PCI MMIO.
>    The default is ``on``.
> 
> +highmem-mmio-size
> +  Set extended MMIO memory map size. Must be a power-of-2 and greater
> than
> +  the default size.
> +
>  gic-version
>    Specify the version of the Generic Interrupt Controller (GIC) to provide.
>    Valid values are:
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 49eb0355ef0c..43d47ffedd9a 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -2773,6 +2773,35 @@ static void virt_set_highmem_mmio(Object *obj,
> bool value, Error **errp)
>      vms->highmem_mmio = value;
>  }
> 
> +static void virt_get_highmem_mmio_size(Object *obj, Visitor *v, const
> char *name,
> +                          void *opaque, Error **errp)
> +{
> +    uint64_t size = extended_memmap[VIRT_HIGH_PCIE_MMIO].size;
> +
> +    visit_type_size(v, name, &size, errp);
> +}
> +
> +static void virt_set_highmem_mmio_size(Object *obj, Visitor *v, const char
> *name,
> +                          void *opaque, Error **errp)
> +{
> +    uint64_t size;
> +
> +    if (!visit_type_size(v, name, &size, errp))
> +        return;

Qemu style mandates braces around.

> +
> +    if (!is_power_of_2(size)) {
> +        error_setg(errp, "highmem_mmio_size is not a power-of-2");
> +        return;
> +    }
> +
> +    if (size < extended_memmap[VIRT_HIGH_PCIE_MMIO].size) {

Not sure it is better to fallback to default size here instead of setting error.

> +        error_setg(errp, "highmem_mmio_size is less than the default (%lu)",
> +                   extended_memmap[VIRT_HIGH_PCIE_MMIO].size);
> +        return;
> +    }
> +
> +    extended_memmap[VIRT_HIGH_PCIE_MMIO].size = size;
> +}
> 
>  static bool virt_get_its(Object *obj, Error **errp)
>  {
> @@ -3446,6 +3475,13 @@ static void virt_machine_class_init(ObjectClass
> *oc, void *data)
>                                            "Set on/off to enable/disable high "
>                                            "memory region for PCI MMIO");
> 
> +    object_class_property_add(oc, "highmem-mmio-size", "size",
> +                                   virt_get_highmem_mmio_size,
> +                                   virt_set_highmem_mmio_size,
> +                                   NULL, NULL);
> +    object_class_property_set_description(oc, "highmem-mmio-size",
> +                                          "Set extended MMIO memory map size");
> +

I think this probably needs backward compatibility to keep migration happy.
Isn't it? See the no_highmem_compact handling.

Thanks,
Shameer

Eric Auger Jan. 28, 2025, 5:52 p.m. UTC | #2

Hi Matthew, Shameer,


On 1/28/25 6:36 PM, Shameerali Kolothum Thodi wrote:
>
>> -----Original Message-----
>> From: Matthew R. Ochs <[e-mail]>
>> Sent: Tuesday, January 28, 2025 4:03 PM
>> To: [e-mail]; Shameerali Kolothum Thodi
>> <[e-mail]>; [e-mail]
>> Cc: [e-mail]; [e-mail]; [e-mail];
>> [e-mail]
>> Subject: [PATCH] hw/arm/virt: Support larger highmem MMIO regions
>>
>> The MMIO region size required to support virtualized environments with
>> large PCI BAR regions can exceed the hardcoded limit configured in QEMU.
>> For example, a VM with multiple NVIDIA Grace-Hopper GPUs passed
>> through
>> requires more MMIO memory than the amount provided by
>> VIRT_HIGH_PCIE_MMIO
>> (currently 512GB). Instead of updating VIRT_HIGH_PCIE_MMIO, introduce a
>> new parameter, highmem-mmio-size, that specifies the MMIO size required
>> to support the VM configuration.
>>
>> Example usage with 1TB MMIO region size:
>> 	-machine virt,gic-version=3,highmem-mmio-size=1099511627776
> I guess you could do highmem-mmio-size=1024G as well.
>
>> Signed-off-by: Matthew R. Ochs <[e-mail]>
>> ---
>>  docs/system/arm/virt.rst |  4 ++++
>>  hw/arm/virt.c            | 36 ++++++++++++++++++++++++++++++++++++
>>  2 files changed, 40 insertions(+)
>>
>> diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst
>> index e67e7f0f7c50..36344554788a 100644
>> --- a/docs/system/arm/virt.rst
>> +++ b/docs/system/arm/virt.rst
>> @@ -138,6 +138,10 @@ highmem-mmio
>>    Set ``on``/``off`` to enable/disable the high memory region for PCI MMIO.
>>    The default is ``on``.
>>
>> +highmem-mmio-size
>> +  Set extended MMIO memory map size. Must be a power-of-2 and greater
maybe keep the existing terminology, ie. high PCIE MMIO region.
extended_memmap
Would deserve to add some comments on top of extended_memmap[] too.
>> than
>> +  the default size.
>> +
>>  gic-version
>>    Specify the version of the Generic Interrupt Controller (GIC) to provide.
>>    Valid values are:
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 49eb0355ef0c..43d47ffedd9a 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -2773,6 +2773,35 @@ static void virt_set_highmem_mmio(Object *obj,
>> bool value, Error **errp)
>>      vms->highmem_mmio = value;
>>  }
>>
>> +static void virt_get_highmem_mmio_size(Object *obj, Visitor *v, const
>> char *name,
>> +                          void *opaque, Error **errp)
>> +{
>> +    uint64_t size = extended_memmap[VIRT_HIGH_PCIE_MMIO].size;
>> +
>> +    visit_type_size(v, name, &size, errp);
>> +}
>> +
>> +static void virt_set_highmem_mmio_size(Object *obj, Visitor *v, const char
>> *name,
>> +                          void *opaque, Error **errp)
>> +{
>> +    uint64_t size;
>> +
>> +    if (!visit_type_size(v, name, &size, errp))
>> +        return;
> Qemu style mandates braces around.
>
>> +
>> +    if (!is_power_of_2(size)) {
>> +        error_setg(errp, "highmem_mmio_size is not a power-of-2");
>> +        return;
>> +    }
>> +
>> +    if (size < extended_memmap[VIRT_HIGH_PCIE_MMIO].size) {
> Not sure it is better to fallback to default size here instead of setting error.
I think if the user sets a value it shall be obeyed
Note that per the dynamic memory map algo, changing the size will also
change the base address. See

virt_set_high_memmap(). By the wayn why do we forbid a smaller size?

>
>> +        error_setg(errp, "highmem_mmio_size is less than the default (%lu)",
>> +                   extended_memmap[VIRT_HIGH_PCIE_MMIO].size);
>> +        return;
>> +    }
>> +
>> +    extended_memmap[VIRT_HIGH_PCIE_MMIO].size = size;
>> +}
>>
>>  static bool virt_get_its(Object *obj, Error **errp)
>>  {
>> @@ -3446,6 +3475,13 @@ static void virt_machine_class_init(ObjectClass
>> *oc, void *data)
>>                                            "Set on/off to enable/disable high "
>>                                            "memory region for PCI MMIO");
>>
>> +    object_class_property_add(oc, "highmem-mmio-size", "size",
>> +                                   virt_get_highmem_mmio_size,
>> +                                   virt_set_highmem_mmio_size,
>> +                                   NULL, NULL);
>> +    object_class_property_set_description(oc, "highmem-mmio-size",
>> +                                          "Set extended MMIO memory map size");
>> +
> I think this probably needs backward compatibility to keep migration happy.
> Isn't it? See the no_highmem_compact handling.
I guess if we keep the same value as default we are good. The difference
with highmem_compact is it was set by default from 7.2 onwards hence
changing the mmio layout. Here by default you keep the same IIUC.

Thanks

Eric
>
> Thanks,
> Shameer
>

Matt Ochs Jan. 28, 2025, 9:28 p.m. UTC | #3

> On Jan 28, 2025, at 11:52 AM, Eric Auger <[e-mail]> wrote:
> 
> Hi Matthew, Shameer,
> 
> On 1/28/25 6:36 PM, Shameerali Kolothum Thodi wrote:
>> 
>>> -----Original Message-----
>>> From: Matthew R. Ochs <[e-mail]>
>>> Sent: Tuesday, January 28, 2025 4:03 PM
>>> To: [e-mail]; Shameerali Kolothum Thodi
>>> <[e-mail]>; [e-mail]
>>> Cc: [e-mail]; [e-mail]; [e-mail];
>>> [e-mail]
>>> Subject: [PATCH] hw/arm/virt: Support larger highmem MMIO regions
>>> 
>>> The MMIO region size required to support virtualized environments with
>>> large PCI BAR regions can exceed the hardcoded limit configured in QEMU.
>>> For example, a VM with multiple NVIDIA Grace-Hopper GPUs passed
>>> through
>>> requires more MMIO memory than the amount provided by
>>> VIRT_HIGH_PCIE_MMIO
>>> (currently 512GB). Instead of updating VIRT_HIGH_PCIE_MMIO, introduce a
>>> new parameter, highmem-mmio-size, that specifies the MMIO size required
>>> to support the VM configuration.
>>> 
>>> Example usage with 1TB MMIO region size:
>>> -machine virt,gic-version=3,highmem-mmio-size=1099511627776
>> I guess you could do highmem-mmio-size=1024G as well.

Sure, the visit_type_size() helper can understand the common unit suffixes
used for sizes, e.g. 1T. To clarify, I’ll update the example in v2.

>>> +highmem-mmio-size
>>> +  Set extended MMIO memory map size. Must be a power-of-2 and greater
> maybe keep the existing terminology, ie. high PCIE MMIO region.
> extended_memmap
> Would deserve to add some comments on top of extended_memmap[] too.

Thanks for the suggestion, will update the patch to use existing terminology.

>>> +    if (!visit_type_size(v, name, &size, errp))
>>> +        return;
>> Qemu style mandates braces around.

Will fix in v2.

>> 
>>> +
>>> +    if (!is_power_of_2(size)) {
>>> +        error_setg(errp, "highmem_mmio_size is not a power-of-2");
>>> +        return;
>>> +    }
>>> +
>>> +    if (size < extended_memmap[VIRT_HIGH_PCIE_MMIO].size) {
>> Not sure it is better to fallback to default size here instead of setting error.
> I think if the user sets a value it shall be obeyed

Agreed.

> Note that per the dynamic memory map algo, changing the size will also
> change the base address. See
> 
> virt_set_high_memmap(). By the wayn why do we forbid a smaller size?

That’s a good point, I will remove this check.

>>> 
>>> +    object_class_property_add(oc, "highmem-mmio-size", "size",
>>> +                                   virt_get_highmem_mmio_size,
>>> +                                   virt_set_highmem_mmio_size,
>>> +                                   NULL, NULL);
>>> +    object_class_property_set_description(oc, "highmem-mmio-size",
>>> +                                          "Set extended MMIO memory map size");
>>> +
>> I think this probably needs backward compatibility to keep migration happy.
>> Isn't it? See the no_highmem_compact handling.
> I guess if we keep the same value as default we are good. The difference
> with highmem_compact is it was set by default from 7.2 onwards hence
> changing the mmio layout. Here by default you keep the same IIUC.

I’m not sure I see an issue since the code is directly modifying the size value
in the extended_memmap array.

Eric Auger Jan. 29, 2025, 7:55 a.m. UTC | #4

Hi,


On 1/28/25 10:28 PM, Matt Ochs wrote:
>> On Jan 28, 2025, at 11:52 AM, Eric Auger <[e-mail]> wrote:
>>
>> Hi Matthew, Shameer,
>>
>> On 1/28/25 6:36 PM, Shameerali Kolothum Thodi wrote:
>>>> -----Original Message-----
>>>> From: Matthew R. Ochs <[e-mail]>
>>>> Sent: Tuesday, January 28, 2025 4:03 PM
>>>> To: [e-mail]; Shameerali Kolothum Thodi
>>>> <[e-mail]>; [e-mail]
>>>> Cc: [e-mail]; [e-mail]; [e-mail];
>>>> [e-mail]
>>>> Subject: [PATCH] hw/arm/virt: Support larger highmem MMIO regions
>>>>
>>>> The MMIO region size required to support virtualized environments with
>>>> large PCI BAR regions can exceed the hardcoded limit configured in QEMU.
>>>> For example, a VM with multiple NVIDIA Grace-Hopper GPUs passed
>>>> through
>>>> requires more MMIO memory than the amount provided by
>>>> VIRT_HIGH_PCIE_MMIO
>>>> (currently 512GB). Instead of updating VIRT_HIGH_PCIE_MMIO, introduce a
>>>> new parameter, highmem-mmio-size, that specifies the MMIO size required
>>>> to support the VM configuration.
>>>>
>>>> Example usage with 1TB MMIO region size:
>>>> -machine virt,gic-version=3,highmem-mmio-size=1099511627776
>>> I guess you could do highmem-mmio-size=1024G as well.
> Sure, the visit_type_size() helper can understand the common unit suffixes
> used for sizes, e.g. 1T. To clarify, I’ll update the example in v2.
>
>>>> +highmem-mmio-size
>>>> +  Set extended MMIO memory map size. Must be a power-of-2 and greater
>> maybe keep the existing terminology, ie. high PCIE MMIO region.
>> extended_memmap
>> Would deserve to add some comments on top of extended_memmap[] too.
> Thanks for the suggestion, will update the patch to use existing terminology.
>
>>>> +    if (!visit_type_size(v, name, &size, errp))
>>>> +        return;
>>> Qemu style mandates braces around.
> Will fix in v2.
>
>>>> +
>>>> +    if (!is_power_of_2(size)) {
>>>> +        error_setg(errp, "highmem_mmio_size is not a power-of-2");
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    if (size < extended_memmap[VIRT_HIGH_PCIE_MMIO].size) {
>>> Not sure it is better to fallback to default size here instead of setting error.
>> I think if the user sets a value it shall be obeyed
> Agreed.
>
>> Note that per the dynamic memory map algo, changing the size will also
>> change the base address. See
>>
>> virt_set_high_memmap(). By the wayn why do we forbid a smaller size?
> That’s a good point, I will remove this check.
>
>>>> +    object_class_property_add(oc, "highmem-mmio-size", "size",
>>>> +                                   virt_get_highmem_mmio_size,
>>>> +                                   virt_set_highmem_mmio_size,
>>>> +                                   NULL, NULL);
>>>> +    object_class_property_set_description(oc, "highmem-mmio-size",
>>>> +                                          "Set extended MMIO memory map size");
>>>> +
>>> I think this probably needs backward compatibility to keep migration happy.
>>> Isn't it? See the no_highmem_compact handling.
>> I guess if we keep the same value as default we are good. The difference
>> with highmem_compact is it was set by default from 7.2 onwards hence
>> changing the mmio layout. Here by default you keep the same IIUC.
> I’m not sure I see an issue since the code is directly modifying the size value
> in the extended_memmap array.

I meant that if by default we keep the size value equal to 512G (the
existing default value), I don't think we need to care about compats.

Eric
>
>

Shameerali Kolothum Thodi Jan. 29, 2025, 8:10 a.m. UTC | #5

> -----Original Message-----
> From: Eric Auger <[e-mail]>
> Sent: Wednesday, January 29, 2025 7:56 AM
> To: Matt Ochs <[e-mail]>; Shameerali Kolothum Thodi
> <[e-mail]>
> Cc: [e-mail]; Nathan Chen <[e-mail]>;
> [e-mail]; Nicolin Chen <[e-mail]>; Ankit Agrawal
> <[e-mail]>
> Subject: Re: [PATCH] hw/arm/virt: Support larger highmem MMIO regions


> >>>> +    if (size < extended_memmap[VIRT_HIGH_PCIE_MMIO].size) {
> >>> Not sure it is better to fallback to default size here instead of setting
> error.
> >> I think if the user sets a value it shall be obeyed
> > Agreed.
> >
> >> Note that per the dynamic memory map algo, changing the size will also
> >> change the base address. See
> >>
> >> virt_set_high_memmap(). By the wayn why do we forbid a smaller size?
> > That’s a good point, I will remove this check.

Is there really a use case where a user will want a smaller size than default?

> >
> >>>> +    object_class_property_add(oc, "highmem-mmio-size", "size",
> >>>> +                                   virt_get_highmem_mmio_size,
> >>>> +                                   virt_set_highmem_mmio_size,
> >>>> +                                   NULL, NULL);
> >>>> +    object_class_property_set_description(oc, "highmem-mmio-size",
> >>>> +                                          "Set extended MMIO memory map size");
> >>>> +
> >>> I think this probably needs backward compatibility to keep migration
> happy.
> >>> Isn't it? See the no_highmem_compact handling.
> >> I guess if we keep the same value as default we are good. The difference
> >> with highmem_compact is it was set by default from 7.2 onwards hence
> >> changing the mmio layout. Here by default you keep the same IIUC.
> > I’m not sure I see an issue since the code is directly modifying the size
> value
> > in the extended_memmap array.
> 
> I meant that if by default we keep the size value equal to 512G (the
> existing default value), I don't think we need to care about compats.

Yeah. If it has default size , it should be Ok I guess. But what if,

Your source is something like,

./qemu-new -machine virt-9.1,...,highmem-mmio-size=XXX

and has a target VM started with

./qemu-9.1  -machine virt,...

The migration will be still successful but will have memory map differences, right?

Or the above is not considered as a valid use case and the onus of making
sure we are using it correctly with default size is on the user. 

Thanks,
SHameer

Eric Auger Jan. 29, 2025, 8:15 a.m. UTC | #6

Hi Shameer,

On 1/29/25 9:10 AM, Shameerali Kolothum Thodi wrote:
>
>> -----Original Message-----
>> From: Eric Auger <[e-mail]>
>> Sent: Wednesday, January 29, 2025 7:56 AM
>> To: Matt Ochs <[e-mail]>; Shameerali Kolothum Thodi
>> <[e-mail]>
>> Cc: [e-mail]; Nathan Chen <[e-mail]>;
>> [e-mail]; Nicolin Chen <[e-mail]>; Ankit Agrawal
>> <[e-mail]>
>> Subject: Re: [PATCH] hw/arm/virt: Support larger highmem MMIO regions
>
>>>>>> +    if (size < extended_memmap[VIRT_HIGH_PCIE_MMIO].size) {
>>>>> Not sure it is better to fallback to default size here instead of setting
>> error.
>>>> I think if the user sets a value it shall be obeyed
>>> Agreed.
>>>
>>>> Note that per the dynamic memory map algo, changing the size will also
>>>> change the base address. See
>>>>
>>>> virt_set_high_memmap(). By the wayn why do we forbid a smaller size?
>>> That’s a good point, I will remove this check.
> Is there really a use case where a user will want a smaller size than default?
>
>>>>>> +    object_class_property_add(oc, "highmem-mmio-size", "size",
>>>>>> +                                   virt_get_highmem_mmio_size,
>>>>>> +                                   virt_set_highmem_mmio_size,
>>>>>> +                                   NULL, NULL);
>>>>>> +    object_class_property_set_description(oc, "highmem-mmio-size",
>>>>>> +                                          "Set extended MMIO memory map size");
>>>>>> +
>>>>> I think this probably needs backward compatibility to keep migration
>> happy.
>>>>> Isn't it? See the no_highmem_compact handling.
>>>> I guess if we keep the same value as default we are good. The difference
>>>> with highmem_compact is it was set by default from 7.2 onwards hence
>>>> changing the mmio layout. Here by default you keep the same IIUC.
>>> I’m not sure I see an issue since the code is directly modifying the size
>> value
>>> in the extended_memmap array.
>> I meant that if by default we keep the size value equal to 512G (the
>> existing default value), I don't think we need to care about compats.
> Yeah. If it has default size , it should be Ok I guess. But what if,
>
> Your source is something like,
>
> ./qemu-new -machine virt-9.1,...,highmem-mmio-size=XXX
>
> and has a target VM started with
>
> ./qemu-9.1  -machine virt,...
>
> The migration will be still successful but will have memory map differences, right?
>
> Or the above is not considered as a valid use case and the onus of making
> sure we are using it correctly with default size is on the user. 
To me when you migrate the qemu cmd line is supposed to be the same on
both src and dst. But worth to be tested.

Eric
>
> Thanks,
> SHameer
>
>
>
>

Philippe Mathieu-Daudé Jan. 29, 2025, 8:25 a.m. UTC | #7

Hi Matthew,

On 28/1/25 17:02, Matthew R. Ochs wrote:
> The MMIO region size required to support virtualized environments with
> large PCI BAR regions can exceed the hardcoded limit configured in QEMU.
> For example, a VM with multiple NVIDIA Grace-Hopper GPUs passed through
> requires more MMIO memory than the amount provided by VIRT_HIGH_PCIE_MMIO
> (currently 512GB). Instead of updating VIRT_HIGH_PCIE_MMIO, introduce a
> new parameter, highmem-mmio-size, that specifies the MMIO size required
> to support the VM configuration.
> 
> Example usage with 1TB MMIO region size:
> 	-machine virt,gic-version=3,highmem-mmio-size=1099511627776
> 
> Signed-off-by: Matthew R. Ochs <[e-mail]>
> ---
>   docs/system/arm/virt.rst |  4 ++++
>   hw/arm/virt.c            | 36 ++++++++++++++++++++++++++++++++++++
>   2 files changed, 40 insertions(+)


> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 49eb0355ef0c..43d47ffedd9a 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -2773,6 +2773,35 @@ static void virt_set_highmem_mmio(Object *obj, bool value, Error **errp)
>       vms->highmem_mmio = value;
>   }
>   
> +static void virt_get_highmem_mmio_size(Object *obj, Visitor *v, const char *name,
> +                          void *opaque, Error **errp)
> +{
> +    uint64_t size = extended_memmap[VIRT_HIGH_PCIE_MMIO].size;
> +
> +    visit_type_size(v, name, &size, errp);

Since you correctly use visit_type_size(), maybe use in commit
description example "highmem-mmio-size=1TB" rather than 1099511627776?

> +}

Matt Ochs Jan. 29, 2025, 4:59 p.m. UTC | #8

On Jan 29, 2025, at 2:25 AM, Philippe Mathieu-Daudé <[e-mail]> wrote:

Hi Matthew,

On 28/1/25 17:02, Matthew R. Ochs wrote:
The MMIO region size required to support virtualized environments with
large PCI BAR regions can exceed the hardcoded limit configured in QEMU.
For example, a VM with multiple NVIDIA Grace-Hopper GPUs passed through
requires more MMIO memory than the amount provided by VIRT_HIGH_PCIE_MMIO
(currently 512GB). Instead of updating VIRT_HIGH_PCIE_MMIO, introduce a
new parameter, highmem-mmio-size, that specifies the MMIO size required
to support the VM configuration.
Example usage with 1TB MMIO region size:
-machine virt,gic-version=3,highmem-mmio-size=1099511627776
Signed-off-by: Matthew R. Ochs <[e-mail]>
---
 docs/system/arm/virt.rst |  4 ++++
 hw/arm/virt.c            | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)


diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 49eb0355ef0c..43d47ffedd9a 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2773,6 +2773,35 @@ static void virt_set_highmem_mmio(Object *obj, bool value, Error **errp)
     vms->highmem_mmio = value;
 }
 +static void virt_get_highmem_mmio_size(Object *obj, Visitor *v, const char *name,
+                          void *opaque, Error **errp)
+{
+    uint64_t size = extended_memmap[VIRT_HIGH_PCIE_MMIO].size;
+
+    visit_type_size(v, name, &size, errp);

Since you correctly use visit_type_size(), maybe use in commit
description example "highmem-mmio-size=1TB" rather than 1099511627776?

Yes, this will be updated in v2.

Patch

diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst
index e67e7f0f7c50..36344554788a 100644
--- a/docs/system/arm/virt.rst
+++ b/docs/system/arm/virt.rst
@@ -138,6 +138,10 @@  highmem-mmio
   Set ``on``/``off`` to enable/disable the high memory region for PCI MMIO.
   The default is ``on``.
 
+highmem-mmio-size
+  Set extended MMIO memory map size. Must be a power-of-2 and greater than
+  the default size.
+
 gic-version
   Specify the version of the Generic Interrupt Controller (GIC) to provide.
   Valid values are:
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 49eb0355ef0c..43d47ffedd9a 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2773,6 +2773,35 @@  static void virt_set_highmem_mmio(Object *obj, bool value, Error **errp)
     vms->highmem_mmio = value;
 }
 
+static void virt_get_highmem_mmio_size(Object *obj, Visitor *v, const char *name,
+                          void *opaque, Error **errp)
+{
+    uint64_t size = extended_memmap[VIRT_HIGH_PCIE_MMIO].size;
+
+    visit_type_size(v, name, &size, errp);
+}
+
+static void virt_set_highmem_mmio_size(Object *obj, Visitor *v, const char *name,
+                          void *opaque, Error **errp)
+{
+    uint64_t size;
+
+    if (!visit_type_size(v, name, &size, errp))
+        return;
+
+    if (!is_power_of_2(size)) {
+        error_setg(errp, "highmem_mmio_size is not a power-of-2");
+        return;
+    }
+
+    if (size < extended_memmap[VIRT_HIGH_PCIE_MMIO].size) {
+        error_setg(errp, "highmem_mmio_size is less than the default (%lu)",
+                   extended_memmap[VIRT_HIGH_PCIE_MMIO].size);
+        return;
+    }
+
+    extended_memmap[VIRT_HIGH_PCIE_MMIO].size = size;
+}
 
 static bool virt_get_its(Object *obj, Error **errp)
 {
@@ -3446,6 +3475,13 @@  static void virt_machine_class_init(ObjectClass *oc, void *data)
                                           "Set on/off to enable/disable high "
                                           "memory region for PCI MMIO");
 
+    object_class_property_add(oc, "highmem-mmio-size", "size",
+                                   virt_get_highmem_mmio_size,
+                                   virt_set_highmem_mmio_size,
+                                   NULL, NULL);
+    object_class_property_set_description(oc, "highmem-mmio-size",
+                                          "Set extended MMIO memory map size");
+
     object_class_property_add_str(oc, "gic-version", virt_get_gic_version,
                                   virt_set_gic_version);
     object_class_property_set_description(oc, "gic-version",