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@xxxxxxxxxx> >>> >>> We are about to trap and emulate acccesses to each PMU register >> >> s/acccesses/accesses/ >> >>> individually. This adds the context offsets for the AArch64 PMU >>> registers and their AArch32 counterparts. >>> >>> Signed-off-by: Shannon Zhao <shannon.zhao@xxxxxxxxxx> >>> --- >>> 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 >> duplication. >> >>> +/* 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. Indeed. >>> +#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. Thanks, M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
