[PATCH v3 51/59] hw/arm/virt: Warn when HVF doesn't report IPA bit length

Philippe Mathieu-Daudé posted 59 patches 3 months, 2 weeks ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Cameron Esfahani <dirty@apple.com>, Roman Bolshakov <rbolshakov@ddn.com>, Phil Dennis-Jordan <phil@philjordan.eu>, Mads Ynddal <mads@ynddal.dk>, Peter Maydell <peter.maydell@linaro.org>, Alexander Graf <agraf@csgraf.de>, Stefan Hajnoczi <stefanha@redhat.com>
There is a newer version of this series
[PATCH v3 51/59] hw/arm/virt: Warn when HVF doesn't report IPA bit length
Posted by Philippe Mathieu-Daudé 3 months, 2 weeks ago
Emit a warning when HVF doesn't return the IPA bit length
and return -1 as "this accelerator is not usable", allowing
QEMU to try with the next one (when using '-accel hvf:tcg').

Reported-by: Ivan Krasilnikov
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2981
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/virt.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 175023897a7..1d65fa471dc 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -3225,8 +3225,12 @@ static int virt_hvf_get_physical_address_range(MachineState *ms)
 {
     VirtMachineState *vms = VIRT_MACHINE(ms);
 
-    int default_ipa_size = hvf_arm_get_default_ipa_bit_size();
-    int max_ipa_size = hvf_arm_get_max_ipa_bit_size();
+    uint32_t default_ipa_size = hvf_arm_get_default_ipa_bit_size();
+    uint32_t max_ipa_size = hvf_arm_get_max_ipa_bit_size();
+    if (!default_ipa_size || !max_ipa_size) {
+        warn_report("HVF didn't report IPA bit length");
+        return -1;
+    }
 
     /* We freeze the memory map to compute the highest gpa */
     virt_set_memmap(vms, max_ipa_size);
-- 
2.51.0


Re: [PATCH v3 51/59] hw/arm/virt: Warn when HVF doesn't report IPA bit length
Posted by Richard Henderson 3 months, 2 weeks ago
On 10/28/25 06:42, Philippe Mathieu-Daudé wrote:
> Emit a warning when HVF doesn't return the IPA bit length
> and return -1 as "this accelerator is not usable", allowing
> QEMU to try with the next one (when using '-accel hvf:tcg').
> 
> Reported-by: Ivan Krasilnikov
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2981
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/arm/virt.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 175023897a7..1d65fa471dc 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -3225,8 +3225,12 @@ static int virt_hvf_get_physical_address_range(MachineState *ms)
>   {
>       VirtMachineState *vms = VIRT_MACHINE(ms);
>   
> -    int default_ipa_size = hvf_arm_get_default_ipa_bit_size();
> -    int max_ipa_size = hvf_arm_get_max_ipa_bit_size();
> +    uint32_t default_ipa_size = hvf_arm_get_default_ipa_bit_size();
> +    uint32_t max_ipa_size = hvf_arm_get_max_ipa_bit_size();
> +    if (!default_ipa_size || !max_ipa_size) {
> +        warn_report("HVF didn't report IPA bit length");
> +        return -1;
> +    }

I suppose this goes back to the previous patch.
It might have been slightly less confusing to merge them, but the underlying questions 
about when and how this can fail remain.


r~

Re: [PATCH v3 51/59] hw/arm/virt: Warn when HVF doesn't report IPA bit length
Posted by Philippe Mathieu-Daudé 3 weeks, 1 day ago
On 28/10/25 13:07, Richard Henderson wrote:
> On 10/28/25 06:42, Philippe Mathieu-Daudé wrote:
>> Emit a warning when HVF doesn't return the IPA bit length
>> and return -1 as "this accelerator is not usable", allowing
>> QEMU to try with the next one (when using '-accel hvf:tcg').
>>
>> Reported-by: Ivan Krasilnikov
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2981
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/arm/virt.c | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 175023897a7..1d65fa471dc 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -3225,8 +3225,12 @@ static int 
>> virt_hvf_get_physical_address_range(MachineState *ms)
>>   {
>>       VirtMachineState *vms = VIRT_MACHINE(ms);
>> -    int default_ipa_size = hvf_arm_get_default_ipa_bit_size();
>> -    int max_ipa_size = hvf_arm_get_max_ipa_bit_size();
>> +    uint32_t default_ipa_size = hvf_arm_get_default_ipa_bit_size();
>> +    uint32_t max_ipa_size = hvf_arm_get_max_ipa_bit_size();
>> +    if (!default_ipa_size || !max_ipa_size) {
>> +        warn_report("HVF didn't report IPA bit length");
>> +        return -1;
>> +    }
> 
> I suppose this goes back to the previous patch.
> It might have been slightly less confusing to merge them, but the 
> underlying questions about when and how this can fail remain.

Right. I'm dropping this patch.

Re: [PATCH v3 51/59] hw/arm/virt: Warn when HVF doesn't report IPA bit length
Posted by Philippe Mathieu-Daudé 3 weeks ago
On 18/1/26 23:03, Philippe Mathieu-Daudé wrote:
> On 28/10/25 13:07, Richard Henderson wrote:
>> On 10/28/25 06:42, Philippe Mathieu-Daudé wrote:
>>> Emit a warning when HVF doesn't return the IPA bit length
>>> and return -1 as "this accelerator is not usable", allowing
>>> QEMU to try with the next one (when using '-accel hvf:tcg').
>>>
>>> Reported-by: Ivan Krasilnikov
>>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2981
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>   hw/arm/virt.c | 8 ++++++--
>>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>>> index 175023897a7..1d65fa471dc 100644
>>> --- a/hw/arm/virt.c
>>> +++ b/hw/arm/virt.c
>>> @@ -3225,8 +3225,12 @@ static int 
>>> virt_hvf_get_physical_address_range(MachineState *ms)
>>>   {
>>>       VirtMachineState *vms = VIRT_MACHINE(ms);
>>> -    int default_ipa_size = hvf_arm_get_default_ipa_bit_size();
>>> -    int max_ipa_size = hvf_arm_get_max_ipa_bit_size();
>>> +    uint32_t default_ipa_size = hvf_arm_get_default_ipa_bit_size();
>>> +    uint32_t max_ipa_size = hvf_arm_get_max_ipa_bit_size();
>>> +    if (!default_ipa_size || !max_ipa_size) {
>>> +        warn_report("HVF didn't report IPA bit length");
>>> +        return -1;
>>> +    }
>>
>> I suppose this goes back to the previous patch.
>> It might have been slightly less confusing to merge them, but the 
>> underlying questions about when and how this can fail remain.
> 
> Right. I'm dropping this patch.

BTW I was looking at the extra-commits in GetUTM and noticed one
related to this:

commit c387fd021064cfb7b895877d0a04660a795887ee
Author: Joelle van Dyne <j@getutm.app>
Date:   Mon Dec 23 00:15:08 2024 -0800

     hw/arm/virt: handle hvf with unknown max IPA size

     When it is not possible to determine the max IPA bit size, the helper
     function will return 0. We do not try to set up the memmap in this case
     and instead fall back to the default in machvirt_init().

     Signed-off-by: Joelle van Dyne <j@getutm.app>

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 5b1e375726d..251fc58b42c 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -3047,6 +3047,11 @@ static int 
virt_hvf_get_physical_address_range(MachineState *ms)
      int default_ipa_size = hvf_arm_get_default_ipa_bit_size();
      int max_ipa_size = hvf_arm_get_max_ipa_bit_size();

+    /* Unknown max ipa size, we'll let the caller figure it out */
+    if (max_ipa_size == 0) {
+        return 0;
+    }
+
      /* We freeze the memory map to compute the highest gpa */
      virt_set_memmap(vms, max_ipa_size);

(https://github.com/utmapp/qemu/commit/c387fd021064cfb7b895877d0a04660a795887ee)


Re: [PATCH v3 51/59] hw/arm/virt: Warn when HVF doesn't report IPA bit length
Posted by Peter Maydell 3 weeks ago
On Mon, 19 Jan 2026 at 12:13, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 18/1/26 23:03, Philippe Mathieu-Daudé wrote:
> > On 28/10/25 13:07, Richard Henderson wrote:
> >> On 10/28/25 06:42, Philippe Mathieu-Daudé wrote:
> >>> Emit a warning when HVF doesn't return the IPA bit length
> >>> and return -1 as "this accelerator is not usable", allowing
> >>> QEMU to try with the next one (when using '-accel hvf:tcg').
> >>>
> >>> Reported-by: Ivan Krasilnikov
> >>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2981
> >>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> >>> ---
> >>>   hw/arm/virt.c | 8 ++++++--
> >>>   1 file changed, 6 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> >>> index 175023897a7..1d65fa471dc 100644
> >>> --- a/hw/arm/virt.c
> >>> +++ b/hw/arm/virt.c
> >>> @@ -3225,8 +3225,12 @@ static int
> >>> virt_hvf_get_physical_address_range(MachineState *ms)
> >>>   {
> >>>       VirtMachineState *vms = VIRT_MACHINE(ms);
> >>> -    int default_ipa_size = hvf_arm_get_default_ipa_bit_size();
> >>> -    int max_ipa_size = hvf_arm_get_max_ipa_bit_size();
> >>> +    uint32_t default_ipa_size = hvf_arm_get_default_ipa_bit_size();
> >>> +    uint32_t max_ipa_size = hvf_arm_get_max_ipa_bit_size();
> >>> +    if (!default_ipa_size || !max_ipa_size) {
> >>> +        warn_report("HVF didn't report IPA bit length");
> >>> +        return -1;
> >>> +    }
> >>
> >> I suppose this goes back to the previous patch.
> >> It might have been slightly less confusing to merge them, but the
> >> underlying questions about when and how this can fail remain.
> >
> > Right. I'm dropping this patch.
>
> BTW I was looking at the extra-commits in GetUTM and noticed one
> related to this:

IIRC, the failure in the associated gitlab issue is because
in hvf_accel_init() the very first thing we try to do with HVF
is "find out the IPA range", so we can create the VM.
So any kind of "HVF doesn't exist/is not usable" problem manifests
as "this call to try to get the IPA bit size returned an error".
So we either need to have hvf_accel_init() determine "HVF
is basically working" as the first thing it does so that we
can know that failures in this function really are specific
to the IPA range check, or else have the error be propagated
back from getting the IPA range so we can do something sensible
in hvf_accel_init().

(I haven't looked at the rest of this series, so maybe it
already does something like that.)

thanks
-- PMM