Hi Philippe, On 2025/7/15 04:04, Philippe Mathieu-Daudé wrote: > On 14/7/25 18:01, Zenghui Yu wrote: > > Quoting Peter Maydell: > > > > " hvf_sysreg_read_cp() and hvf_sysreg_write_cp() do not check the .access > > field of the ARMCPRegInfo to ensure that they forbid writes to registers > > that are marked with a .access field that says they're read-only (and > > ditto reads to write-only registers). " > > > > Before we add more registers in GIC sysreg handlers, let's get it correct > > by adding the .access checks to hvf_sysreg_read_cp() and > > hvf_sysreg_write_cp(). With that, a sysreg access with invalid permission > > will result in an UNDEFINED exception. > > > > Suggested-by: Peter Maydell <peter.mayd...@linaro.org> > > Signed-off-by: Zenghui Yu <zenghui...@linux.dev> > > --- > > > > I hard-code the @current_el parameter of cp_access_ok() to 1 because > > > > * we only support EL0 and EL1 in HVF, and > > This might change with this work: > https://lore.kernel.org/qemu-devel/20250620172751.94231-1-phi...@linaro.org/ > and plan to leverage M3/M4 for EL2 support: > https://developer.apple.com/documentation/hypervisor/hv_vm_config_set_el2_enabled(_:_:)
Thanks for the heads-up! I hadn't noticed that and need to have a further look at both. An alternative would be using arm_current_el() as the @current_el [1], plus a cpu_synchronize_state() before cp_access_ok() to synchronize env->pstate from HVF. I'm not sure if it works for the new split-accel. P.S., there is another arm_current_el() (in hvf.c, pmswinc_write()/ pmu_counter_enabled()) for which we haven't called cpu_synchronize_state() to synchronize env->pstate. Is it wrong? Probably we can do an overall cpu_synchronize_state() on every "handle VMEXIT", to fix at least the above issue, which however definitely hurts the performance.. What do you think? [1] https://lore.kernel.org/r/d9c8d200-4453-48d7-b14a-8e15a7cf6...@linux.dev Thanks, Zenghui
