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 ef6be3660f5..062812bf252 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -3149,8 +3149,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.49.0

On Wed, 16 Jul 2025 at 18:28, Philippe Mathieu-Daudé <philmd@linaro.org> 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 ef6be3660f5..062812bf252 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -3149,8 +3149,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;
> +    }

Even if we avoid blowing up in this function, won't
hvf_accel_init() immediately fail in hvf_arch_vm_create()
and either exit(1) or assert() depending on what error
code it got back ?

It's unfortunate that there's no convenient way we can
check "hvf is basically going to work" in hvf_accel_init()
before we get into the machine-specific hook. "HVF didn't
report IPA bit length" isn't really a very good message
to report for "HVF isn't going to work at all here".

(More generally, I think the hvf code rather overuses
assert_hvf_ok(). I don't think the KVM code uses asserts
for error checking like this.)

thanks
-- PMM

> On 16 Jul 2025, at 19.28, Philippe Mathieu-Daudé <philmd@linaro.org> 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(-)

I haven't been able to verify that hv_vm_config_get_max_ipa_size and
hv_vm_config_get_default_ipa_size fail if HVF is not available, but
assuming so, it looks fine to me.

Reviewed-by: Mads Ynddal <mads@ynddal.dk>

On 17/7/25 12:06, Mads Ynddal wrote:
> 
>> On 16 Jul 2025, at 19.28, Philippe Mathieu-Daudé <philmd@linaro.org> 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(-)
> 
> I haven't been able to verify that hv_vm_config_get_max_ipa_size and
> hv_vm_config_get_default_ipa_size fail if HVF is not available, but

This happens with nested macOS guest. Maybe we are missing an earlier
check whether HVF is usable or not, but we shouldn't brutally abort().

I'll try to update the patch description. Annoyingly the GitLab issue
reporter isn't Cc'ed via the mailing list.

> assuming so, it looks fine to me.
> 
> Reviewed-by: Mads Ynddal <mads@ynddal.dk>

> This happens with nested macOS guest.

I took another look at the stack trace in the issue. Everything should
be fine then.

> Maybe we are missing an earlier check whether HVF is usable or not, but
> we shouldn't brutally abort().

I agree. I think with this patchset you're doing it practically as early
as possible with accel_init_machine.