Re: [PATCH 2/2] hvf: arm: disable unavailable features on older macOS

[Joelle van Dyne]

I think it is acceptable policy to not test against older macOS
versions, although I want to point out that a significant population
of macOS users are still on macOS 11-13. It is bad practice to just
crash when a function is not found. This is why the compiler throws a
warning asking you to explicitly add availability guards. If we were
to ever treat warnings more strictly, this would be an issue. If we
consider older hosts unsupported, we can just test these changes on
newer macOS, confirm that it doesn't cause any regression, and make no
promises on what happens on older macOS.

On Mon, Dec 23, 2024 at 3:31 AM Phil Dennis-Jordan <lists@philjordan.eu> wrote:
>
> QEMU has a policy of only supporting macOS host versions for 2 years, see 
> docs/about/build-platforms.rst. So you'll probably not be able to get this 
> upstreamed as only macOS 14 and 15 are now supported. (I'm not a great fan of 
> this policy personally, but I guess it makes CI etc easier to handle.)
>
> On Mon, 23 Dec 2024 at 05:11, Joelle van Dyne <j@getutm.app> wrote:
>>
>> IPA size queries were introduced in macOS 13. When QEMU is built targeting
>> a lower version, the compile will fail. If targeting a higher version and
>> the binary is executed on an older version, QEMU will crash. This will
>> restore the behaviour before IPA max size querying was added which means
>> VMs with 64+ GB of RAM will not work if running on < macOS 13.
>>
>> Signed-off-by: Joelle van Dyne <j@getutm.app>
>> ---
>>  target/arm/hvf/hvf.c | 55 ++++++++++++++++++++++++++++----------------
>>  1 file changed, 35 insertions(+), 20 deletions(-)
>>
>> diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
>> index 0afd96018e..da60476dbc 100644
>> --- a/target/arm/hvf/hvf.c
>> +++ b/target/arm/hvf/hvf.c
>> @@ -897,7 +897,9 @@ static bool 
>> hvf_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>>      r |= hv_vcpu_get_sys_reg(fd, HV_SYS_REG_MIDR_EL1, &ahcf->midr);
>>      r |= hv_vcpu_destroy(fd);
>>
>> -    clamp_id_aa64mmfr0_parange_to_ipa_size(&host_isar.id_aa64mmfr0);
>> +    if (__builtin_available(macOS 13.0, *)) {
>> +        clamp_id_aa64mmfr0_parange_to_ipa_size(&host_isar.id_aa64mmfr0);
>> +    }
>>
>>      ahcf->isar = host_isar;
>>
>> @@ -923,26 +925,34 @@ static bool 
>> hvf_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>>
>>  uint32_t hvf_arm_get_default_ipa_bit_size(void)
>>  {
>> -    uint32_t default_ipa_size;
>> -    hv_return_t ret = hv_vm_config_get_default_ipa_size(&default_ipa_size);
>> -    assert_hvf_ok(ret);
>> +    if (__builtin_available(macOS 13.0, *)) {
>> +        uint32_t default_ipa_size;
>> +        hv_return_t ret = 
>> hv_vm_config_get_default_ipa_size(&default_ipa_size);
>> +        assert_hvf_ok(ret);
>>
>> -    return default_ipa_size;
>> +        return default_ipa_size;
>> +    } else {
>> +        return 0;
>> +    }
>>  }
>>
>>  uint32_t hvf_arm_get_max_ipa_bit_size(void)
>>  {
>> -    uint32_t max_ipa_size;
>> -    hv_return_t ret = hv_vm_config_get_max_ipa_size(&max_ipa_size);
>> -    assert_hvf_ok(ret);
>> +    if (__builtin_available(macOS 13.0, *)) {
>> +        uint32_t max_ipa_size;
>> +        hv_return_t ret = hv_vm_config_get_max_ipa_size(&max_ipa_size);
>> +        assert_hvf_ok(ret);
>>
>> -    /*
>> -     * We clamp any IPA size we want to back the VM with to a valid PARange
>> -     * value so the guest doesn't try and map memory outside of the valid 
>> range.
>> -     * This logic just clamps the passed in IPA bit size to the first valid
>> -     * PARange value <= to it.
>> -     */
>> -    return round_down_to_parange_bit_size(max_ipa_size);
>> +        /*
>> +         * We clamp any IPA size we want to back the VM with to a valid 
>> PARange
>> +         * value so the guest doesn't try and map memory outside of the 
>> valid
>> +         * range. This logic just clamps the passed in IPA bit size to the 
>> first
>> +         * valid PARange value <= to it.
>> +         */
>> +        return round_down_to_parange_bit_size(max_ipa_size);
>> +    } else {
>> +        return 0;
>> +    }
>>  }
>>
>>  void hvf_arm_set_cpu_features_from_host(ARMCPU *cpu)
>> @@ -973,13 +983,18 @@ void hvf_arch_vcpu_destroy(CPUState *cpu)
>>  hv_return_t hvf_arch_vm_create(MachineState *ms, uint32_t pa_range)
>>  {
>>      hv_return_t ret;
>> -    hv_vm_config_t config = hv_vm_config_create();
>> +    hv_vm_config_t config;
>>
>> -    ret = hv_vm_config_set_ipa_size(config, pa_range);
>> -    if (ret != HV_SUCCESS) {
>> -        goto cleanup;
>> +    if (__builtin_available(macOS 13.0, *)) {
>> +        config = hv_vm_config_create();
>> +        ret = hv_vm_config_set_ipa_size(config, pa_range);
>> +        if (ret != HV_SUCCESS) {
>> +            goto cleanup;
>> +        }
>> +        chosen_ipa_bit_size = pa_range;
>> +    } else {
>> +        config = NULL;
>>      }
>> -    chosen_ipa_bit_size = pa_range;
>>
>>      ret = hv_vm_create(config);
>>
>> --
>> 2.41.0
>>
>>