Raw writes to PMCNTENCLR and PMCNTENCLR_EL0 incorrectly used their
default write function, which clears written bits instead of writes the
raw value.

PMINTENCLR and PMINTENCLR_EL1 are similar registers, but they instead
had ARM_CP_NO_RAW. Commit 7a0e58fa6487 ("target-arm: Split NO_MIGRATE
into ALIAS and NO_RAW") sugguests ARM_CP_ALIAS should be used instead of
ARM_CP_NO_RAW in such a case:

> We currently mark ARM coprocessor/system register definitions with
> the flag ARM_CP_NO_MIGRATE for two different reasons:
> 1) register is an alias on to state that's also visible via
>    some other register, and that other register is the one
>    responsible for migrating the state
> 2) register is not actually state at all (for instance the TLB
>    or cache maintenance operation "registers") and it makes no
>    sense to attempt to migrate it or otherwise access the raw state
>
> This works fine for identifying which registers should be ignored
> when performing migration, but we also use the same functions for
> synchronizing system register state between QEMU and the kernel
> when using KVM. In this case we don't want to try to sync state
> into registers in category 2, but we do want to sync into registers
> in category 1, because the kernel might have picked a different
> one of the aliases as its choice for which one to expose for
> migration.

These registers fall in category 1 (ARM_CP_ALIAS), not category 2
(ARM_CP_NO_RAW).

ARM_CP_NO_RAW also has another undesired side effect that hides
registers from GDB.

Properly set raw write functions and drop the ARM_CP_NO_RAW flag from
PMINTENCLR and PMINTENCLR_EL1; this fixes GDB/KVM state synchronization
of PMCNTENCLR and PMCNTENCLR_EL0, and exposes all these four registers
to GDB.

It is not necessary to add ARM_CP_ALIAS to these registers because the
flag is already set.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
Supersedes: <20250317-raw-v1-0-09e2dfff0e90@daynix.com>
("[PATCH 0/4] target/arm: Flag PMCNTENCLR with ARM_CP_NO_RAW")
---
Changes in v3:
- Added a reference to commit 7a0e58fa6487
  ("target-arm: Split NO_MIGRATE into ALIAS and NO_RAW")
- Link to v2: https://lore.kernel.org/qemu-devel/20250314-clr-v2-1-7c7220c177c9@daynix.com

Changes in v2:
- Added raw write functions to PMCNTENCLR and PMINTENCLR.
- Dropped the ARM_CP_NO_RAW flag from PMINTENCLR and PMINTENCLR_EL1.
- Link to v1: https://lore.kernel.org/qemu-devel/20250313-clr-v1-1-2cc49df40fe9@daynix.com
---
 target/arm/helper.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 76312102879b..889d30880794 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -1904,7 +1904,7 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
       .fieldoffset = offsetoflow32(CPUARMState, cp15.c9_pmcnten),
       .accessfn = pmreg_access,
       .fgt = FGT_PMCNTEN,
-      .writefn = pmcntenclr_write,
+      .writefn = pmcntenclr_write, .raw_writefn = raw_write,
       .type = ARM_CP_ALIAS | ARM_CP_IO },
     { .name = "PMCNTENCLR_EL0", .state = ARM_CP_STATE_AA64,
       .opc0 = 3, .opc1 = 3, .crn = 9, .crm = 12, .opc2 = 2,
@@ -1912,7 +1912,7 @@ 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),
@@ -2029,16 +2029,16 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
     { .name = "PMINTENCLR", .cp = 15, .crn = 9, .crm = 14, .opc1 = 0, .opc2 = 2,
       .access = PL1_RW, .accessfn = access_tpm,
       .fgt = FGT_PMINTEN,
-      .type = ARM_CP_ALIAS | ARM_CP_IO | ARM_CP_NO_RAW,
+      .type = ARM_CP_ALIAS | ARM_CP_IO,
       .fieldoffset = offsetof(CPUARMState, cp15.c9_pminten),
-      .writefn = pmintenclr_write, },
+      .writefn = pmintenclr_write, .raw_writefn = raw_write },
     { .name = "PMINTENCLR_EL1", .state = ARM_CP_STATE_AA64,
       .opc0 = 3, .opc1 = 0, .crn = 9, .crm = 14, .opc2 = 2,
       .access = PL1_RW, .accessfn = access_tpm,
       .fgt = FGT_PMINTEN,
-      .type = ARM_CP_ALIAS | ARM_CP_IO | ARM_CP_NO_RAW,
+      .type = ARM_CP_ALIAS | ARM_CP_IO,
       .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,

---
base-commit: f0737158b483e7ec2b2512145aeab888b85cc1f7
change-id: 20250313-clr-6a34831628b7

Best regards,
-- 
Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>

On Sat, 31 May 2025 at 13:11, Akihiko Odaki
<odaki@rsg.ci.i.u-tokyo.ac.jp> wrote:
>
> Raw writes to PMCNTENCLR and PMCNTENCLR_EL0 incorrectly used their
> default write function, which clears written bits instead of writes the
> raw value.
>
> PMINTENCLR and PMINTENCLR_EL1 are similar registers, but they instead
> had ARM_CP_NO_RAW. Commit 7a0e58fa6487 ("target-arm: Split NO_MIGRATE
> into ALIAS and NO_RAW") sugguests ARM_CP_ALIAS should be used instead of
> ARM_CP_NO_RAW in such a case:
>
> > We currently mark ARM coprocessor/system register definitions with
> > the flag ARM_CP_NO_MIGRATE for two different reasons:
> > 1) register is an alias on to state that's also visible via
> >    some other register, and that other register is the one
> >    responsible for migrating the state
> > 2) register is not actually state at all (for instance the TLB
> >    or cache maintenance operation "registers") and it makes no
> >    sense to attempt to migrate it or otherwise access the raw state
> >
> > This works fine for identifying which registers should be ignored
> > when performing migration, but we also use the same functions for
> > synchronizing system register state between QEMU and the kernel
> > when using KVM. In this case we don't want to try to sync state
> > into registers in category 2, but we do want to sync into registers
> > in category 1, because the kernel might have picked a different
> > one of the aliases as its choice for which one to expose for
> > migration.
>
> These registers fall in category 1 (ARM_CP_ALIAS), not category 2
> (ARM_CP_NO_RAW).
>
> ARM_CP_NO_RAW also has another undesired side effect that hides
> registers from GDB.
>
> Properly set raw write functions and drop the ARM_CP_NO_RAW flag from
> PMINTENCLR and PMINTENCLR_EL1; this fixes GDB/KVM state synchronization
> of PMCNTENCLR and PMCNTENCLR_EL0, and exposes all these four registers
> to GDB.
>
> It is not necessary to add ARM_CP_ALIAS to these registers because the
> flag is already set.
>
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>



Applied to target-arm.next, thanks.

-- PMM