On 12/8/25 02:20, Richard Henderson wrote:
On 8/12/25 03:06, Philippe Mathieu-Daudé wrote:From: Mohamed Mediouni <moha...@unpredictable.fr>When starting up the VM at EL2, more sysregs are available. Sync the state of those.Signed-off-by: Mohamed Mediouni <moha...@unpredictable.fr> [PMD: Adapted to host_cpu_feature_supported() API] Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org> --- target/arm/hvf/hvf.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c index 5174973991f..778dc3cedf7 100644 --- a/target/arm/hvf/hvf.c +++ b/target/arm/hvf/hvf.c@@ -400,6 +400,7 @@ static const struct hvf_reg_match hvf_fpreg_match[] = {struct hvf_sreg_match { int reg; uint32_t key; + bool el2; uint32_t cp_idx; }; @@ -545,6 +546,27 @@ static struct hvf_sreg_match hvf_sreg_match[] = { { HV_SYS_REG_CNTV_CTL_EL0, HVF_SYSREG(14, 3, 3, 3, 1) }, { HV_SYS_REG_CNTV_CVAL_EL0, HVF_SYSREG(14, 3, 3, 3, 2) }, { HV_SYS_REG_SP_EL1, HVF_SYSREG(4, 1, 3, 4, 0) },
[*]
+ /* EL2 */ + { HV_SYS_REG_CPTR_EL2, HVF_SYSREG(1, 1, 3, 4, 2), .el2 = true }, + { HV_SYS_REG_ELR_EL2, HVF_SYSREG(4, 0, 3, 4, 1), .el2 = true }, + { HV_SYS_REG_ESR_EL2, HVF_SYSREG(5, 2, 3, 4, 0), .el2 = true }, + { HV_SYS_REG_FAR_EL2, HVF_SYSREG(6, 0, 3, 4, 0), .el2 = true }, + { HV_SYS_REG_HCR_EL2, HVF_SYSREG(1, 1, 3, 4, 0), .el2 = true }, + { HV_SYS_REG_HPFAR_EL2, HVF_SYSREG(6, 0, 3, 4, 4), .el2 = true }, + { HV_SYS_REG_MAIR_EL2, HVF_SYSREG(10, 2, 3, 4, 0), .el2 = true }, + { HV_SYS_REG_MDCR_EL2, HVF_SYSREG(1, 1, 3, 4, 1), .el2 = true }, + { HV_SYS_REG_SCTLR_EL2, HVF_SYSREG(1, 0, 3, 4, 0), .el2 = true }, + { HV_SYS_REG_SPSR_EL2, HVF_SYSREG(4, 0, 3, 4, 0), .el2 = true }, + { HV_SYS_REG_SP_EL2, HVF_SYSREG(4, 1, 3, 6, 0), .el2 = true}, + { HV_SYS_REG_TCR_EL2, HVF_SYSREG(2, 0, 3, 4, 2), .el2 = true }, + { HV_SYS_REG_TPIDR_EL2, HVF_SYSREG(13, 0, 3, 4, 2), .el2 = true }, + { HV_SYS_REG_TTBR0_EL2, HVF_SYSREG(2, 0, 3, 4, 0), .el2 = true }, + { HV_SYS_REG_TTBR1_EL2, HVF_SYSREG(2, 0, 3, 4, 1), .el2 = true }, + { HV_SYS_REG_VBAR_EL2, HVF_SYSREG(12, 0, 3, 4, 0), .el2 = true }, + { HV_SYS_REG_VMPIDR_EL2, HVF_SYSREG(0, 0, 3, 4, 5), .el2 = true }, + { HV_SYS_REG_VPIDR_EL2, HVF_SYSREG(0, 0, 3, 4, 0), .el2 = true }, + { HV_SYS_REG_VTCR_EL2, HVF_SYSREG(2, 1, 3, 4, 2), .el2 = true }, + { HV_SYS_REG_VTTBR_EL2, HVF_SYSREG(2, 1, 3, 4, 0), .el2 = true },Note that el2 == opc1 in {4,5}.
Does this also work with HV_SYS_REG_SP_EL1 (see [*])?
I suppose so, after reading commit 67d10fc4737 ("target/arm: Trap
sysreg accesses for FEAT_NV") addition:
/**
* arm_cpreg_trap_in_nv: Return true if cpreg traps in nested
* virtualization
*
* Return true if this cpreg is one which should be trapped to EL2
* if it is executed at EL1 when nested virtualization is enabled
* via HCR_EL2.NV.
*/
static inline bool arm_cpreg_traps_in_nv(const ARMCPRegInfo *ri)
{
/*
* The Arm ARM defines the registers to be trapped in terms of
* their names (I_TZTZL). However the underlying principle is "if
* it would UNDEF at EL1 but work at EL2 then it should trap", and
* the way the encoding of sysregs and system instructions is done
* means that the right set of registers is exactly those where
* the opc1 field is 4 or 5. (You can see this also in the assert
* we do that the opc1 field and the permissions mask line up in
* define_one_arm_cp_reg_with_opaque().)
* Checking the opc1 field is easier for us and avoids the problem
* that we do not consistently use the right architectural names
* for all sysregs, since we treat the name field as largely for
* debug.
*
* However we do this check, it is going to be at least potentially
* fragile to future new sysregs, but this seems the least likely
* to break.
*
* In particular, note that the released sysreg XML defines that
* the FEAT_MEC sysregs and instructions do not follow this FEAT_NV
* trapping rule, so we will need to add an ARM_CP_* flag to
* indicate "register does not trap on NV" to handle those if/when
* we implement FEAT_MEC.
*/
return ri->opc1 == 4 || ri->opc1 == 5;
}
From v2:
>> @@ -608,6 +630,11 @@ int hvf_get_registers(CPUState *cpu)
>> continue;
>> }
>> + if (hvf_sreg_match[i].el2
>> + && !host_cpu_feature_supported(ARM_FEATURE_EL2)) {
>> + continue;
>> + }
>
> I thought you were going to change this to arm_feature(&cpu->env,
> ARM_FEATURE_EL2).
We want to use HVF to get these registers on M3 / M4, but not on
M1 / M2 when split-accel in use where we emulate them.
Expected logic:
if (
// this is an EL2 register ...
hvf_reg_traps_in_nv(i)
// ... and CPU doesn't provide EL2 ...
&& (!arm_feature(&cpu->env, ARM_FEATURE_EL2)
// ... or we are emulating EL2 (split-accel) ...
|| tcg_enabled())
) {
// ... then do not get the register with the HVF API ...
continue;
}
// ... otherwise by default use HVF.
hv_vcpu_get_sys_reg(cpu->accel->fd, hvf_sreg_match[i].reg, &val);
