On 08/12/15 08:09, Shannon Zhao wrote: > Hi Marc, > > On 2015/12/7 22:55, Marc Zyngier wrote: >> 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. >>>>>> > > Re-think about this. Since there are different operates to SET/CLR > registers, maybe it should keep both of them while only storing the > state in one of them. > > To SET: > vcpu_sys_reg(vcpu, r->reg) |= val; > To CLR: > vcpu_sys_reg(vcpu, r->reg) &= ~val; There is really no point keeping both, because they are two views of the same state. They perform different action on the same data, so the way to look at it is to have different functions/methods that modify the same state. > Or keep one of them and within the access handler, according to the > operates encoding value to judge whether it's SET or CLR. That's indeed the way it should be. You just have to register different functions in the trap table. You could even move the register outside of the sys_reg array into the kvm_pmu structure. 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
