On 2025/03/14 3:34, Peter Maydell wrote:
On Thu, 13 Mar 2025 at 07:16, Akihiko Odaki <akihiko.od...@daynix.com> wrote:PMCNTENCLR_EL0 and PMINTENCLR_EL1 clears written bits so we need an alternative raw write functions, which will be used to copy KVM kernel coprocessor state into userspace. Signed-off-by: Akihiko Odaki <akihiko.od...@daynix.com> --- target/arm/helper.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index f0ead22937bf..30883cd3a989 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -1907,7 +1907,8 @@ static const ARMCPRegInfo v7_cp_reginfo[] = { .fgt = FGT_PMCNTEN, .type = ARM_CP_ALIAS | ARM_CP_IO, .fieldoffset = offsetof(CPUARMState, cp15.c9_pmcnten), - .writefn = pmcntenclr_write }, + .writefn = pmcntenclr_write, + .raw_writefn = raw_write }, { .name = "PMOVSR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 3, .access = PL0_RW, .type = ARM_CP_IO, .fieldoffset = offsetoflow32(CPUARMState, cp15.c9_pmovsr), @@ -2033,7 +2034,8 @@ static const ARMCPRegInfo v7_cp_reginfo[] = { .fgt = FGT_PMINTEN, .type = ARM_CP_ALIAS | ARM_CP_IO | ARM_CP_NO_RAW, .fieldoffset = offsetof(CPUARMState, cp15.c9_pminten), - .writefn = pmintenclr_write }, + .writefn = pmintenclr_write, + .raw_writefn = raw_write }, { .name = "CCSIDR", .state = ARM_CP_STATE_BOTH, .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 1, .opc2 = 0, .access = PL1_R,Hmm, looking more closely at this, I think this second one should not need a raw_writefn, because it's marked as ARM_CP_NO_RAW (meaning nothing should try to do a raw write to it).
Good catch; I didn't notice ARM_CP_NO_RAW.
And the first one is marked ARM_CP_ALIAS, so I'm not sure why we would be using it in KVM register sync: add_cpreg_to_list() skips ARM_CP_ALIAS (and ARM_CP_NO_RAW) registers when we construct the cpreg_tuples[] array that defines which sysregs we sync to and from KVM.
The register list is initialized with kvm_arm_init_cpreg_list() for KVM, which ignores those flags.
target/arm/cpregs.h explicitly says: "registers marked ARM_CP_ALIAS will not be migrated but may have their state set by syncing of register state from KVM."
ARM_CP_NO_RAW is still respected for KVM by write_cpustate_to_list() and write_list_to_cpustate().
(We should arguably be consistent about our usage of the NO_RAW flag between the pmintenclr and pmcntenclr registers.)
I sent v2 to drop the flag. target/arm/cpregs.h suggests ARM_CP_NO_RAW is not a flag for these registers:
> Flag: Register has no underlying state and does not support raw access > for state saving/loading; it will not be used for either migration or > KVM state synchronization. Typically this is for "registers" which are > actually used as instructions for cache maintenance and so on. These registers have underlying states and can support raw access. Regards, Akihiko Odaki
thanks -- PMM
