HVF traps accesses to CNTHCTL_EL2. For nested guests, HVF traps accesses to MDCCINT_EL1.
Pass through those accesses to the Hypervisor.framework library.

Signed-off-by: Mohamed Mediouni <mohamed@unpredictable.fr>
---
 target/arm/hvf/hvf.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index f14a3a3cbd..eefae3069f 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -297,6 +297,10 @@ void hvf_arm_init_debug(void)
 #define SYSREG_DBGWVR15_EL1   SYSREG(2, 0, 0, 15, 6)
 #define SYSREG_DBGWCR15_EL1   SYSREG(2, 0, 0, 15, 7)
 
+/* EL2 registers */
+#define SYSREG_CNTHCTL_EL2    SYSREG(3, 4, 14, 1, 0)
+#define SYSREG_MDCCINT_EL1    SYSREG(2, 0, 0, 2, 0)
+
 #define WFX_IS_WFE (1 << 0)
 
 #define TMR_CTL_ENABLE  (1 << 0)
@@ -1372,6 +1376,12 @@ static int hvf_sysreg_read(CPUState *cpu, uint32_t reg, uint64_t *val)
     case SYSREG_OSDLR_EL1:
         /* Dummy register */
         return 0;
+    case SYSREG_CNTHCTL_EL2:
+        assert_hvf_ok(hv_vcpu_get_sys_reg(cpu->accel->fd, HV_SYS_REG_CNTHCTL_EL2, val));
+        return 0;
+    case SYSREG_MDCCINT_EL1:
+        assert_hvf_ok(hv_vcpu_get_sys_reg(cpu->accel->fd, HV_SYS_REG_MDCCINT_EL1, val));
+        return 0;
     case SYSREG_ICC_AP0R0_EL1:
     case SYSREG_ICC_AP0R1_EL1:
     case SYSREG_ICC_AP0R2_EL1:
@@ -1689,6 +1699,12 @@ static int hvf_sysreg_write(CPUState *cpu, uint32_t reg, uint64_t val)
     case SYSREG_OSDLR_EL1:
         /* Dummy register */
         return 0;
+    case SYSREG_CNTHCTL_EL2:
+        assert_hvf_ok(hv_vcpu_set_sys_reg(cpu->accel->fd, HV_SYS_REG_CNTHCTL_EL2, val));
+        return 0;
+    case SYSREG_MDCCINT_EL1:
+        assert_hvf_ok(hv_vcpu_set_sys_reg(cpu->accel->fd, HV_SYS_REG_MDCCINT_EL1, val));
+        return 0;
     case SYSREG_LORC_EL1:
         /* Dummy register */
         return 0;
-- 
2.39.5 (Apple Git-154)

On 28/7/25 15:41, Mohamed Mediouni wrote:
> HVF traps accesses to CNTHCTL_EL2. For nested guests, HVF traps accesses to MDCCINT_EL1.
> Pass through those accesses to the Hypervisor.framework library.
> 
> Signed-off-by: Mohamed Mediouni <mohamed@unpredictable.fr>
> ---
>   target/arm/hvf/hvf.c | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
> 
> diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
> index f14a3a3cbd..eefae3069f 100644
> --- a/target/arm/hvf/hvf.c
> +++ b/target/arm/hvf/hvf.c
> @@ -297,6 +297,10 @@ void hvf_arm_init_debug(void)
>   #define SYSREG_DBGWVR15_EL1   SYSREG(2, 0, 0, 15, 6)
>   #define SYSREG_DBGWCR15_EL1   SYSREG(2, 0, 0, 15, 7)
>   
> +/* EL2 registers */
> +#define SYSREG_CNTHCTL_EL2    SYSREG(3, 4, 14, 1, 0)
> +#define SYSREG_MDCCINT_EL1    SYSREG(2, 0, 0, 2, 0)
> +
>   #define WFX_IS_WFE (1 << 0)
>   
>   #define TMR_CTL_ENABLE  (1 << 0)
> @@ -1372,6 +1376,12 @@ static int hvf_sysreg_read(CPUState *cpu, uint32_t reg, uint64_t *val)
>       case SYSREG_OSDLR_EL1:
>           /* Dummy register */
>           return 0;
> +    case SYSREG_CNTHCTL_EL2:
> +        assert_hvf_ok(hv_vcpu_get_sys_reg(cpu->accel->fd, HV_SYS_REG_CNTHCTL_EL2, val));
> +        return 0;

We'd like to remove the assert_hvf_ok() calls, so adding more isn't
really helping. Anyhow,

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

> We'd like to remove the assert_hvf_ok() calls, so adding more isn't
> really helping. Anyhow,
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>

What is the preferred method going forward?

Apple designed the HV API to be able to fail at every function, and it's
rarely something that can be recovered from.

In [PATCH v4 04/15] of this series, we saw an issue that was hidden
because the return code was not properly checked (not calling from the
owning thread). Previously, I submitted a patch (d5bd8d8267) for the
same issue, because Apple had changed a function's behavior. This was
caught by an assert_hvf_ok.

I agree with you, that generally adding asserts is a bad design, but at
the same time, HV is designed in a way, that the code would be littered
with checks otherwise.

On Mon, 11 Aug 2025 at 11:08, Mads Ynddal <mads@ynddal.dk> wrote:
>
>
> > We'd like to remove the assert_hvf_ok() calls, so adding more isn't
> > really helping. Anyhow,
> >
> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> What is the preferred method going forward?
>
> Apple designed the HV API to be able to fail at every function, and it's
> rarely something that can be recovered from.
>
> In [PATCH v4 04/15] of this series, we saw an issue that was hidden
> because the return code was not properly checked (not calling from the
> owning thread). Previously, I submitted a patch (d5bd8d8267) for the
> same issue, because Apple had changed a function's behavior. This was
> caught by an assert_hvf_ok.
>
> I agree with you, that generally adding asserts is a bad design, but at
> the same time, HV is designed in a way, that the code would be littered
> with checks otherwise.

This suggestion was I think mine, and I think partly I was
misled by the name of assert_hvf_ok(), because in fact it
isn't an assert(). (assert() should be specifically "if we
hit this this is a bug in QEMU", and "hvf returned an error"
is generally not that. It does call abort(), though, which
isn't much better.)

But also I think the existence of assert_hvf_ok() encourages
it to be used in cases where actually we should be doing
something more sensible with an error return. For instance,
in hvf_accel_init() if we can't init HVF then we should
return an error code to the caller, which might fall back
to the TCG accelerator -- but instead we have an assert_hvf_ok(),
so fallback won't work because we'll just bomb out.

The KVM API is also one where any API call (ioctl) can return an
error, but we don't generally assert() that they succeeded, except
in a few cases of "given where we are, for this to fail would
mean the kernel was broken". Instead we propagate error values
upwards when the function has a mechanism to do that,
and where appropriate we print an error message that's
hopefully reasonably comprehensible to the user and exit.

-- PMM

>>> We'd like to remove the assert_hvf_ok() calls, so adding more isn't
>>> really helping. Anyhow,
>>> 
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> 
>> What is the preferred method going forward?
>> 
>> Apple designed the HV API to be able to fail at every function, and it's
>> rarely something that can be recovered from.
>> 
>> In [PATCH v4 04/15] of this series, we saw an issue that was hidden
>> because the return code was not properly checked (not calling from the
>> owning thread). Previously, I submitted a patch (d5bd8d8267) for the
>> same issue, because Apple had changed a function's behavior. This was
>> caught by an assert_hvf_ok.
>> 
>> I agree with you, that generally adding asserts is a bad design, but at
>> the same time, HV is designed in a way, that the code would be littered
>> with checks otherwise.
> 
> This suggestion was I think mine, and I think partly I was
> misled by the name of assert_hvf_ok(), because in fact it
> isn't an assert(). (assert() should be specifically "if we
> hit this this is a bug in QEMU", and "hvf returned an error"
> is generally not that. It does call abort(), though, which
> isn't much better.)
> 
> But also I think the existence of assert_hvf_ok() encourages
> it to be used in cases where actually we should be doing
> something more sensible with an error return. For instance,
> in hvf_accel_init() if we can't init HVF then we should
> return an error code to the caller, which might fall back
> to the TCG accelerator -- but instead we have an assert_hvf_ok(),
> so fallback won't work because we'll just bomb out.
> 
> The KVM API is also one where any API call (ioctl) can return an
> error, but we don't generally assert() that they succeeded, except
> in a few cases of "given where we are, for this to fail would
> mean the kernel was broken". Instead we propagate error values
> upwards when the function has a mechanism to do that,
> and where appropriate we print an error message that's
> hopefully reasonably comprehensible to the user and exit.
> 
> -- PMM

Thanks for the clarification. I think I understand the sentiment.
Propagating the error codes when possible, is much preferred. But
letting the calls go unchecked is problematic too, so I don't know if we
can get around assert_hvf_ok in some shape or form.

You're right, that it might encourage use as an easy fix. I'll keep an
eye on it in my reviews.