On Thu, Mar 21, 2019 at 01:02:27PM -0700, Sodagudi Prasad wrote: > On 2019-03-21 06:34, Julien Thierry wrote: > > Hi Prasad, > > > > On 21/03/2019 02:07, Prasad Sodagudi wrote: > > > Preserves the bitfields of PMCR_EL0(AArch64) during PMU reset. > > > Reset routine should write a 1 to PMCR.C and PMCR.P fields only > > > to reset the counters. Other fields should not be changed > > > as they could be set before PMU initialization and their > > > value must be preserved even after reset. > > > > Are there any particular bit you are concerned about? Apart from the RO > > ones and the Res0 ones (to which we are already writing 0), I see: > > > > DP -> irrelevant for non-secure > > X -> This one is the only potentially interesting, however it resets to > > an architecturally unknown value, so unless we know for a fact it was > > set before hand, we probably want to clear it > > D -> ignored when we have LC set (and we do) > > E -> Since this is the function we use to reset the PMU on the current > > CPU, we probably want to set this bit to 0 regardless of its previous > > value > > > > So, is there any issue this patch is solving? > > Hi Julien, > > Thanks for taking a look into this patch. Yes. On our Qualcomm platforms, > observed that X bit is getting cleared by kernel. > This bit is getting set by firmware for Qualcomm use cases and non-secure > world is resetting without this patch.
Given the kernel reconfigures the PMU dynamically at runtime (e.g. to multiplex events over counters), I don't follow how this is useful. Could you elaborate on how specifically you intend to use this? > I think, changing that register this register modifications to > read-modify-write style make sense. I disagree. In general, RESx bits may be allocated new meanings in future (and will reset to UNKNOWN values), so it is not safe to simply preserve the reset value. Given that, any kernel _must_ explicitly reset registers in order to ensure the expected behaviour. Thanks Mark.
