On 11/8/25 15:33, Smail AIDER wrote:
Hi Philippe,

It is not just some refactoring. The last patch v3 is a squash of two previous 
patches v1 and v2.
Maybe I need to change the history description if not clear (I was talking from 
v3 point of view).
The purpose of the series is the main description itself. Please check the v1 
below:

https://patchew.org/QEMU/[email protected]/

Then please add a Cc tag (maintainer can do it if this v3 is OK, no need for v4):

Cc: [email protected]

Other than that, the argument (is_pmcr) is correct. "isread" is not used in 
this case.

Right, I missed it during review. Maybe we want to forward the arguments
for clarity?

-static CPAccessResult pmreg_access(CPUARMState *env, const ARMCPRegInfo *ri,
 -                                   bool isread)
+static CPAccessResult do_pmreg_access(CPUARMState *env, const ARMCPRegInfo *ri,
 +                                   bool isread, bool is_pmcr)

Anyhow I'll let Richard review. No objection.


--
Best Regards,
Smail AIDER
E-Mail: [email protected]
Operating System Researcher/Developer
Dresden Research Center, OS Kernel Lab
Huawei Technologies Co., Ltd

-----Original Message-----
From: Philippe Mathieu-Daudé <[email protected]>
Sent: Monday, August 11, 2025 2:36 PM
To: Smail AIDER <[email protected]>; [email protected]
Cc: Alexander Spyridakis <[email protected]>; zhangyue (BA) 
<[email protected]>; Liuyutao(DRC) <[email protected]>; [email protected]; Peter 
Maydell <[email protected]>; [email protected]; [email protected]
Subject: Re: [PATCH v3 1/1] target/arm: Trap PMCR when MDCR_EL2.TPMCR is set

Hi Smail,

(no need to Cc qemu-stable with this patch, it is a simple refactor)

On 11/8/25 13:21, Smail AIDER via wrote:
From: Smail AIDER via <[email protected]>

Trap PMCR_EL0 or PMCR accesses to EL2 when MDCR_EL2.TPMCR is set.
Similar to MDCR_EL2.TPM, MDCR_EL2.TPMCR allows trapping EL0 and EL1
accesses to the PMCR register to EL2.

Signed-off-by: Smail AIDER <[email protected]>
Reviewed-by: Richard Henderson <[email protected]>
Message-Id: <[email protected]>
---
   target/arm/cpregs-pmu.c | 33 +++++++++++++++++++++++++--------
   1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/target/arm/cpregs-pmu.c b/target/arm/cpregs-pmu.c
index 9c4431c18b..13392ddc4c 100644
--- a/target/arm/cpregs-pmu.c
+++ b/target/arm/cpregs-pmu.c
@@ -228,22 +228,27 @@ static bool event_supported(uint16_t number)
       return supported_event_map[number] != UNSUPPORTED_EVENT;
   }
-static CPAccessResult pmreg_access(CPUARMState *env, const ARMCPRegInfo *ri,
-                                   bool isread)
+static CPAccessResult do_pmreg_access(CPUARMState *env, bool is_pmcr)

"bool is_pmcr" vs ...

+static CPAccessResult pmreg_access(CPUARMState *env, const ARMCPRegInfo *ri,
+                                   bool isread)

... "bool isread".

I suppose we want to use "is_pmcr" here instead of "isread".

+{
+    return do_pmreg_access(env, false);
+}
+
+static CPAccessResult pmreg_access_pmcr(CPUARMState *env, const ARMCPRegInfo 
*ri,
+                                   bool isread)
+{
+    return do_pmreg_access(env, true);
+}
+
   static CPAccessResult pmreg_access_xevcntr(CPUARMState *env,
                                              const ARMCPRegInfo *ri,
                                              bool isread)
@@ -1187,14 +1204,14 @@ void define_pm_cpregs(ARMCPU *cpu)
               .fgt = FGT_PMCR_EL0,
               .type = ARM_CP_IO | ARM_CP_ALIAS,
               .fieldoffset = offsetoflow32(CPUARMState, cp15.c9_pmcr),
-            .accessfn = pmreg_access,
+            .accessfn = pmreg_access_pmcr,
               .readfn = pmcr_read, .raw_readfn = raw_read,
               .writefn = pmcr_write, .raw_writefn = raw_write,
           };
           const ARMCPRegInfo pmcr64 = {
               .name = "PMCR_EL0", .state = ARM_CP_STATE_AA64,
               .opc0 = 3, .opc1 = 3, .crn = 9, .crm = 12, .opc2 = 0,
-            .access = PL0_RW, .accessfn = pmreg_access,
+            .access = PL0_RW, .accessfn = pmreg_access_pmcr,
               .fgt = FGT_PMCR_EL0,
               .type = ARM_CP_IO,
               .fieldoffset = offsetof(CPUARMState, cp15.c9_pmcr),



Reply via email to