[PATCH v5 03/21] KVM: ARM64: Add offset defines for PMU registers
marc.zyngier at arm.com
Mon Dec 7 06:55:20 PST 2015
On 07/12/15 14:31, Shannon Zhao wrote:
> On 2015/12/7 22:06, Marc Zyngier wrote:
>> On 03/12/15 06:11, Shannon Zhao wrote:
>>> From: Shannon Zhao <shannon.zhao at linaro.org>
>>> We are about to trap and emulate acccesses to each PMU register
>>> individually. This adds the context offsets for the AArch64 PMU
>>> registers and their AArch32 counterparts.
>>> Signed-off-by: Shannon Zhao <shannon.zhao at linaro.org>
>>> arch/arm64/include/asm/kvm_asm.h | 55 ++++++++++++++++++++++++++++++++++++----
>>> 1 file changed, 50 insertions(+), 5 deletions(-)
>>> diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
>>> index 5e37710..4f804c1 100644
>>> --- a/arch/arm64/include/asm/kvm_asm.h
>>> +++ b/arch/arm64/include/asm/kvm_asm.h
>>> @@ -48,12 +48,34 @@
>>> #define MDSCR_EL1 22 /* Monitor Debug System Control Register */
>>> #define MDCCINT_EL1 23 /* Monitor Debug Comms Channel Interrupt Enable Reg */
>> Coming back to this patch, it gives a clear view of where you have state
>>> +/* Performance Monitors Registers */
>>> +#define PMCR_EL0 24 /* Control Register */
>>> +#define PMOVSSET_EL0 25 /* Overflow Flag Status Set Register */
>>> +#define PMOVSCLR_EL0 26 /* Overflow Flag Status Clear Register */
>> This should only be a single state. You don't even have to represent it
>> in the sysreg array, to be honest.
> Yeah, I could store the state in one of them and drop one of them here.
>>> +#define PMSELR_EL0 27 /* Event Counter Selection Register */
>>> +#define PMCEID0_EL0 28 /* Common Event Identification Register 0 */
>>> +#define PMCEID1_EL0 29 /* Common Event Identification Register 1 */
>>> +#define PMEVCNTR0_EL0 30 /* Event Counter Register (0-30) */
>>> +#define PMEVCNTR30_EL0 60
>>> +#define PMCCNTR_EL0 61 /* Cycle Counter Register */
>>> +#define PMEVTYPER0_EL0 62 /* Event Type Register (0-30) */
>>> +#define PMEVTYPER30_EL0 92
>>> +#define PMCCFILTR_EL0 93 /* Cycle Count Filter Register */
>>> +#define PMXEVCNTR_EL0 94 /* Selected Event Count Register */
>>> +#define PMXEVTYPER_EL0 95 /* Selected Event Type Register */
>> These "select" registers aren't real ones, but just a way to pick the
>> real register. They should be removed.
> I think these two could be retained, since it's convenient to handle the
> guest accessing by using "case PMXEVCNTR_EL0"
Not really. This is just occupying pointless space is the register file,
and it would be clearer to have a couple of helpers called directly from
the sys_reg trap table.
Please get rid of it.
Jazz is not dead. It just smells funny...
More information about the linux-arm-kernel