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.z...@linaro.org>
>>>
>>> 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.z...@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
>> 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
