diff mbox

[04/18] KVM: ARM64: Add reset and access handlers for PMCR_EL0 register

Message ID 1436149068-3784-5-git-send-email-shannon.zhao@linaro.org
State New
Headers show

Commit Message

Shannon Zhao July 6, 2015, 2:17 a.m. UTC 
From: Shannon Zhao <shannon.zhao@linaro.org>

Add reset handler which gets host value of PMCR_EL0 and make writable
bits architecturally UNKNOWN. Add access handler which emulates
writing and reading PMCR_EL0 register.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 arch/arm64/kvm/sys_regs.c | 41 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

Comments

Christoffer Dall July 16, 2015, 7:55 p.m. UTC | #1 
On Mon, Jul 06, 2015 at 10:17:34AM +0800, shannon.zhao@linaro.org wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Add reset handler which gets host value of PMCR_EL0 and make writable
> bits architecturally UNKNOWN. Add access handler which emulates
> writing and reading PMCR_EL0 register.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  arch/arm64/kvm/sys_regs.c | 41 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index c370b40..152ee17 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -33,6 +33,7 @@
>  #include <asm/kvm_emulate.h>
>  #include <asm/kvm_host.h>
>  #include <asm/kvm_mmu.h>
> +#include <asm/pmu.h>
>  
>  #include <trace/events/kvm.h>
>  
> @@ -236,6 +237,44 @@ static void reset_mpidr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
>  	vcpu_sys_reg(vcpu, MPIDR_EL1) = (1ULL << 31) | mpidr;
>  }
>  
> +static void reset_pmcr_el0(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
> +{
> +	u32 pmcr;
> +
> +	asm volatile("mrs %0, pmcr_el0\n" : "=r" (pmcr));
> +	vcpu_sys_reg(vcpu, PMCR_EL0) = (pmcr & ~ARMV8_PMCR_MASK)
> +				       | (ARMV8_PMCR_MASK & 0xdecafbad);

You could add a comment that this resets to UNKNOWN as to not make
people confused about the pseudo-random hex value.

Have we thought about whether we want to tell the guest that it has the
same PMU as available on the real hardware, or does the virtualization
layer suggest to us that we should adjust this somehow?


> +}
> +
> +/* PMCR_EL0 accessor. Only called as long as MDCR_EL2.TPMCR is set. */
> +static bool access_pmcr(struct kvm_vcpu *vcpu,
> +			const struct sys_reg_params *p,
> +			const struct sys_reg_desc *r)
> +{
> +	unsigned long val;
> +
> +	if (p->is_write) {
> +		/* Only update writeable bits of PMCR */
> +		if (!p->is_aarch32)
> +			val = vcpu_sys_reg(vcpu, r->reg);
> +		else
> +			val = vcpu_cp15(vcpu, r->reg);

don't you need to add this function as the handler in the cp15_regs
array as well?

> +		val &= ~ARMV8_PMCR_MASK;
> +		val |= *vcpu_reg(vcpu, p->Rt) & ARMV8_PMCR_MASK;
> +		if (!p->is_aarch32)
> +			vcpu_sys_reg(vcpu, r->reg) = val;
> +		else
> +			vcpu_cp15(vcpu, r->reg) = val;
> +	} else {
> +		if (!p->is_aarch32)
> +			*vcpu_reg(vcpu, p->Rt) = vcpu_sys_reg(vcpu, r->reg);
> +		else
> +			*vcpu_reg(vcpu, p->Rt) = vcpu_cp15(vcpu, r->reg);
> +	}
> +
> +	return true;
> +}
> +
>  /* Silly macro to expand the DBG{BCR,BVR,WVR,WCR}n_EL1 registers in one go */
>  #define DBG_BCR_BVR_WCR_WVR_EL1(n)					\
>  	/* DBGBVRn_EL1 */						\
> @@ -427,7 +466,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
>  
>  	/* PMCR_EL0 */
>  	{ Op0(0b11), Op1(0b011), CRn(0b1001), CRm(0b1100), Op2(0b000),
> -	  trap_raz_wi },
> +	  access_pmcr, reset_pmcr_el0, PMCR_EL0, },
>  	/* PMCNTENSET_EL0 */
>  	{ Op0(0b11), Op1(0b011), CRn(0b1001), CRm(0b1100), Op2(0b001),
>  	  trap_raz_wi },
> -- 
> 2.1.0
> 
Thanks,
-Christoffer
Shannon Zhao July 17, 2015, 8:45 a.m. UTC | #2 
On 2015/7/17 3:55, Christoffer Dall wrote:
> On Mon, Jul 06, 2015 at 10:17:34AM +0800, shannon.zhao@linaro.org wrote:
>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>
>> Add reset handler which gets host value of PMCR_EL0 and make writable
>> bits architecturally UNKNOWN. Add access handler which emulates
>> writing and reading PMCR_EL0 register.
>>
>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> ---
>>  arch/arm64/kvm/sys_regs.c | 41 ++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 40 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
>> index c370b40..152ee17 100644
>> --- a/arch/arm64/kvm/sys_regs.c
>> +++ b/arch/arm64/kvm/sys_regs.c
>> @@ -33,6 +33,7 @@
>>  #include <asm/kvm_emulate.h>
>>  #include <asm/kvm_host.h>
>>  #include <asm/kvm_mmu.h>
>> +#include <asm/pmu.h>
>>  
>>  #include <trace/events/kvm.h>
>>  
>> @@ -236,6 +237,44 @@ static void reset_mpidr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
>>  	vcpu_sys_reg(vcpu, MPIDR_EL1) = (1ULL << 31) | mpidr;
>>  }
>>  
>> +static void reset_pmcr_el0(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
>> +{
>> +	u32 pmcr;
>> +
>> +	asm volatile("mrs %0, pmcr_el0\n" : "=r" (pmcr));
>> +	vcpu_sys_reg(vcpu, PMCR_EL0) = (pmcr & ~ARMV8_PMCR_MASK)
>> +				       | (ARMV8_PMCR_MASK & 0xdecafbad);
> 
> You could add a comment that this resets to UNKNOWN as to not make
> people confused about the pseudo-random hex value.
> 
Ok.

> Have we thought about whether we want to tell the guest that it has the
> same PMU as available on the real hardware, or does the virtualization
> layer suggest to us that we should adjust this somehow?
> 
I guess here the number of PMU counters is what we can adjust, right?
Are we worried about that the host will run out of counters when guest
and host register lots of events?
The PMU of cortex-a57 has 6 counters. IIUC, if one of the guest vcpu
process registers 6 events and some host process register 6 events too,
these events will be monitored in real hardware PMU counter when the
related process runs on the cpu. And when other processes are scheduled
to run, it will switch the contexts of PMU counters.

> 
>> +}
>> +
>> +/* PMCR_EL0 accessor. Only called as long as MDCR_EL2.TPMCR is set. */
>> +static bool access_pmcr(struct kvm_vcpu *vcpu,
>> +			const struct sys_reg_params *p,
>> +			const struct sys_reg_desc *r)
>> +{
>> +	unsigned long val;
>> +
>> +	if (p->is_write) {
>> +		/* Only update writeable bits of PMCR */
>> +		if (!p->is_aarch32)
>> +			val = vcpu_sys_reg(vcpu, r->reg);
>> +		else
>> +			val = vcpu_cp15(vcpu, r->reg);
> 
> don't you need to add this function as the handler in the cp15_regs
> array as well?
> 
Sorry, I'm not very clear about this. Will look at the codes and
understand the use of cp15_regs.

>> +		val &= ~ARMV8_PMCR_MASK;
>> +		val |= *vcpu_reg(vcpu, p->Rt) & ARMV8_PMCR_MASK;
>> +		if (!p->is_aarch32)
>> +			vcpu_sys_reg(vcpu, r->reg) = val;
>> +		else
>> +			vcpu_cp15(vcpu, r->reg) = val;
>> +	} else {
>> +		if (!p->is_aarch32)
>> +			*vcpu_reg(vcpu, p->Rt) = vcpu_sys_reg(vcpu, r->reg);
>> +		else
>> +			*vcpu_reg(vcpu, p->Rt) = vcpu_cp15(vcpu, r->reg);
>> +	}
>> +
>> +	return true;
>> +}
>> +
>>  /* Silly macro to expand the DBG{BCR,BVR,WVR,WCR}n_EL1 registers in one go */
>>  #define DBG_BCR_BVR_WCR_WVR_EL1(n)					\
>>  	/* DBGBVRn_EL1 */						\
>> @@ -427,7 +466,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
>>  
>>  	/* PMCR_EL0 */
>>  	{ Op0(0b11), Op1(0b011), CRn(0b1001), CRm(0b1100), Op2(0b000),
>> -	  trap_raz_wi },
>> +	  access_pmcr, reset_pmcr_el0, PMCR_EL0, },
>>  	/* PMCNTENSET_EL0 */
>>  	{ Op0(0b11), Op1(0b011), CRn(0b1001), CRm(0b1100), Op2(0b001),
>>  	  trap_raz_wi },
>> -- 
>> 2.1.0
>>
> Thanks,
> -Christoffer
>
Christoffer Dall July 17, 2015, 10:21 a.m. UTC | #3 
On Fri, Jul 17, 2015 at 04:45:44PM +0800, Shannon Zhao wrote:
> 
> 
> On 2015/7/17 3:55, Christoffer Dall wrote:
> > On Mon, Jul 06, 2015 at 10:17:34AM +0800, shannon.zhao@linaro.org wrote:
> >> From: Shannon Zhao <shannon.zhao@linaro.org>
> >>
> >> Add reset handler which gets host value of PMCR_EL0 and make writable
> >> bits architecturally UNKNOWN. Add access handler which emulates
> >> writing and reading PMCR_EL0 register.
> >>
> >> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> >> ---
> >>  arch/arm64/kvm/sys_regs.c | 41 ++++++++++++++++++++++++++++++++++++++++-
> >>  1 file changed, 40 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> >> index c370b40..152ee17 100644
> >> --- a/arch/arm64/kvm/sys_regs.c
> >> +++ b/arch/arm64/kvm/sys_regs.c
> >> @@ -33,6 +33,7 @@
> >>  #include <asm/kvm_emulate.h>
> >>  #include <asm/kvm_host.h>
> >>  #include <asm/kvm_mmu.h>
> >> +#include <asm/pmu.h>
> >>  
> >>  #include <trace/events/kvm.h>
> >>  
> >> @@ -236,6 +237,44 @@ static void reset_mpidr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
> >>  	vcpu_sys_reg(vcpu, MPIDR_EL1) = (1ULL << 31) | mpidr;
> >>  }
> >>  
> >> +static void reset_pmcr_el0(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
> >> +{
> >> +	u32 pmcr;
> >> +
> >> +	asm volatile("mrs %0, pmcr_el0\n" : "=r" (pmcr));
> >> +	vcpu_sys_reg(vcpu, PMCR_EL0) = (pmcr & ~ARMV8_PMCR_MASK)
> >> +				       | (ARMV8_PMCR_MASK & 0xdecafbad);
> > 
> > You could add a comment that this resets to UNKNOWN as to not make
> > people confused about the pseudo-random hex value.
> > 
> Ok.
> 
> > Have we thought about whether we want to tell the guest that it has the
> > same PMU as available on the real hardware, or does the virtualization
> > layer suggest to us that we should adjust this somehow?
> > 
> I guess here the number of PMU counters is what we can adjust, right?
> Are we worried about that the host will run out of counters when guest
> and host register lots of events?

that's what I wonder; if perf itself reserves a counter for example,
then we'll at best be able to measure with N-1 counters for the guest (N
being the number of counters on the physical CPU), so why tell the guest
we have N counters?

Of course, we can never even be guaranteed to have N-1 counters
avaialable either, so maybe the sane choice is just to tell the guest
what kind of hardware we have, and then fiddle the best we can with the
remaining counters?  After all, correct functionality of the guest
doesn't depend on this, it's a best-effort kind of thing...

Thoughts?

> The PMU of cortex-a57 has 6 counters. IIUC, if one of the guest vcpu
> process registers 6 events and some host process register 6 events too,
> these events will be monitored in real hardware PMU counter when the
> related process runs on the cpu. And when other processes are scheduled
> to run, it will switch the contexts of PMU counters.

That depends on the way the counters are used by perf I think.  What if
you have system wide events?  What if the KVM (vcpu) process itself is
being monitored for some events?

> 
> > 
> >> +}
> >> +
> >> +/* PMCR_EL0 accessor. Only called as long as MDCR_EL2.TPMCR is set. */
> >> +static bool access_pmcr(struct kvm_vcpu *vcpu,
> >> +			const struct sys_reg_params *p,
> >> +			const struct sys_reg_desc *r)
> >> +{
> >> +	unsigned long val;
> >> +
> >> +	if (p->is_write) {
> >> +		/* Only update writeable bits of PMCR */
> >> +		if (!p->is_aarch32)
> >> +			val = vcpu_sys_reg(vcpu, r->reg);
> >> +		else
> >> +			val = vcpu_cp15(vcpu, r->reg);
> > 
> > don't you need to add this function as the handler in the cp15_regs
> > array as well?
> > 
> Sorry, I'm not very clear about this. Will look at the codes and
> understand the use of cp15_regs.
> 

I think the point is that you cannot use the same value of r->reg to
index into both arrays, so the cp15 index must be passed from the
cp15_regs array first.

Thanks,
-Christoffer
Shannon Zhao July 21, 2015, 1:16 a.m. UTC | #4 
On 2015/7/17 18:21, Christoffer Dall wrote:
> On Fri, Jul 17, 2015 at 04:45:44PM +0800, Shannon Zhao wrote:
>>
>>
>> On 2015/7/17 3:55, Christoffer Dall wrote:
>>> On Mon, Jul 06, 2015 at 10:17:34AM +0800, shannon.zhao@linaro.org wrote:
>>>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>>>
>>>> Add reset handler which gets host value of PMCR_EL0 and make writable
>>>> bits architecturally UNKNOWN. Add access handler which emulates
>>>> writing and reading PMCR_EL0 register.
>>>>
>>>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>>>> ---
>>>>  arch/arm64/kvm/sys_regs.c | 41 ++++++++++++++++++++++++++++++++++++++++-
>>>>  1 file changed, 40 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
>>>> index c370b40..152ee17 100644
>>>> --- a/arch/arm64/kvm/sys_regs.c
>>>> +++ b/arch/arm64/kvm/sys_regs.c
>>>> @@ -33,6 +33,7 @@
>>>>  #include <asm/kvm_emulate.h>
>>>>  #include <asm/kvm_host.h>
>>>>  #include <asm/kvm_mmu.h>
>>>> +#include <asm/pmu.h>
>>>>  
>>>>  #include <trace/events/kvm.h>
>>>>  
>>>> @@ -236,6 +237,44 @@ static void reset_mpidr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
>>>>  	vcpu_sys_reg(vcpu, MPIDR_EL1) = (1ULL << 31) | mpidr;
>>>>  }
>>>>  
>>>> +static void reset_pmcr_el0(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
>>>> +{
>>>> +	u32 pmcr;
>>>> +
>>>> +	asm volatile("mrs %0, pmcr_el0\n" : "=r" (pmcr));
>>>> +	vcpu_sys_reg(vcpu, PMCR_EL0) = (pmcr & ~ARMV8_PMCR_MASK)
>>>> +				       | (ARMV8_PMCR_MASK & 0xdecafbad);
>>>
>>> You could add a comment that this resets to UNKNOWN as to not make
>>> people confused about the pseudo-random hex value.
>>>
>> Ok.
>>
>>> Have we thought about whether we want to tell the guest that it has the
>>> same PMU as available on the real hardware, or does the virtualization
>>> layer suggest to us that we should adjust this somehow?
>>>
>> I guess here the number of PMU counters is what we can adjust, right?
>> Are we worried about that the host will run out of counters when guest
>> and host register lots of events?
> 
> that's what I wonder; if perf itself reserves a counter for example,
> then we'll at best be able to measure with N-1 counters for the guest (N
> being the number of counters on the physical CPU), so why tell the guest
> we have N counters?
> 

I'm not sure whether perf itself reserves one counter. Here I just pass
the hardware information to guest.

> Of course, we can never even be guaranteed to have N-1 counters
> avaialable either, so maybe the sane choice is just to tell the guest
> what kind of hardware we have, and then fiddle the best we can with the
> remaining counters?  After all, correct functionality of the guest
> doesn't depend on this, it's a best-effort kind of thing...
> 
> Thoughts?
> 
>> The PMU of cortex-a57 has 6 counters. IIUC, if one of the guest vcpu
>> process registers 6 events and some host process register 6 events too,
>> these events will be monitored in real hardware PMU counter when the
>> related process runs on the cpu. And when other processes are scheduled
>> to run, it will switch the contexts of PMU counters.
> 
> That depends on the way the counters are used by perf I think.  What if
> you have system wide events?  What if the KVM (vcpu) process itself is
> being monitored for some events?
> 

Not sure what will happen when the number of monitored events is greater
than counters. I guess the perf layer may balance the events?

>>
>>>
>>>> +}
>>>> +
>>>> +/* PMCR_EL0 accessor. Only called as long as MDCR_EL2.TPMCR is set. */
>>>> +static bool access_pmcr(struct kvm_vcpu *vcpu,
>>>> +			const struct sys_reg_params *p,
>>>> +			const struct sys_reg_desc *r)
>>>> +{
>>>> +	unsigned long val;
>>>> +
>>>> +	if (p->is_write) {
>>>> +		/* Only update writeable bits of PMCR */
>>>> +		if (!p->is_aarch32)
>>>> +			val = vcpu_sys_reg(vcpu, r->reg);
>>>> +		else
>>>> +			val = vcpu_cp15(vcpu, r->reg);
>>>
>>> don't you need to add this function as the handler in the cp15_regs
>>> array as well?
>>>
>> Sorry, I'm not very clear about this. Will look at the codes and
>> understand the use of cp15_regs.
>>
> 
> I think the point is that you cannot use the same value of r->reg to
> index into both arrays, so the cp15 index must be passed from the
> cp15_regs array first.
> 
> Thanks,
> -Christoffer
>
Christoffer Dall Aug. 3, 2015, 7:39 p.m. UTC | #5 
On Tue, Jul 21, 2015 at 09:16:57AM +0800, Shannon Zhao wrote:
> 
> 
> On 2015/7/17 18:21, Christoffer Dall wrote:
> > On Fri, Jul 17, 2015 at 04:45:44PM +0800, Shannon Zhao wrote:
> >>
> >>
> >> On 2015/7/17 3:55, Christoffer Dall wrote:
> >>> On Mon, Jul 06, 2015 at 10:17:34AM +0800, shannon.zhao@linaro.org wrote:
> >>>> From: Shannon Zhao <shannon.zhao@linaro.org>
> >>>>
> >>>> Add reset handler which gets host value of PMCR_EL0 and make writable
> >>>> bits architecturally UNKNOWN. Add access handler which emulates
> >>>> writing and reading PMCR_EL0 register.
> >>>>
> >>>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> >>>> ---
> >>>>  arch/arm64/kvm/sys_regs.c | 41 ++++++++++++++++++++++++++++++++++++++++-
> >>>>  1 file changed, 40 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> >>>> index c370b40..152ee17 100644
> >>>> --- a/arch/arm64/kvm/sys_regs.c
> >>>> +++ b/arch/arm64/kvm/sys_regs.c
> >>>> @@ -33,6 +33,7 @@
> >>>>  #include <asm/kvm_emulate.h>
> >>>>  #include <asm/kvm_host.h>
> >>>>  #include <asm/kvm_mmu.h>
> >>>> +#include <asm/pmu.h>
> >>>>  
> >>>>  #include <trace/events/kvm.h>
> >>>>  
> >>>> @@ -236,6 +237,44 @@ static void reset_mpidr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
> >>>>  	vcpu_sys_reg(vcpu, MPIDR_EL1) = (1ULL << 31) | mpidr;
> >>>>  }
> >>>>  
> >>>> +static void reset_pmcr_el0(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
> >>>> +{
> >>>> +	u32 pmcr;
> >>>> +
> >>>> +	asm volatile("mrs %0, pmcr_el0\n" : "=r" (pmcr));
> >>>> +	vcpu_sys_reg(vcpu, PMCR_EL0) = (pmcr & ~ARMV8_PMCR_MASK)
> >>>> +				       | (ARMV8_PMCR_MASK & 0xdecafbad);
> >>>
> >>> You could add a comment that this resets to UNKNOWN as to not make
> >>> people confused about the pseudo-random hex value.
> >>>
> >> Ok.
> >>
> >>> Have we thought about whether we want to tell the guest that it has the
> >>> same PMU as available on the real hardware, or does the virtualization
> >>> layer suggest to us that we should adjust this somehow?
> >>>
> >> I guess here the number of PMU counters is what we can adjust, right?
> >> Are we worried about that the host will run out of counters when guest
> >> and host register lots of events?
> > 
> > that's what I wonder; if perf itself reserves a counter for example,
> > then we'll at best be able to measure with N-1 counters for the guest (N
> > being the number of counters on the physical CPU), so why tell the guest
> > we have N counters?
> > 
> 
> I'm not sure whether perf itself reserves one counter. Here I just pass
> the hardware information to guest.
> 
> > Of course, we can never even be guaranteed to have N-1 counters
> > avaialable either, so maybe the sane choice is just to tell the guest
> > what kind of hardware we have, and then fiddle the best we can with the
> > remaining counters?  After all, correct functionality of the guest
> > doesn't depend on this, it's a best-effort kind of thing...
> > 
> > Thoughts?
> > 
> >> The PMU of cortex-a57 has 6 counters. IIUC, if one of the guest vcpu
> >> process registers 6 events and some host process register 6 events too,
> >> these events will be monitored in real hardware PMU counter when the
> >> related process runs on the cpu. And when other processes are scheduled
> >> to run, it will switch the contexts of PMU counters.
> > 
> > That depends on the way the counters are used by perf I think.  What if
> > you have system wide events?  What if the KVM (vcpu) process itself is
> > being monitored for some events?
> > 
> 
> Not sure what will happen when the number of monitored events is greater
> than counters. I guess the perf layer may balance the events?
> 
I'm not sure either, but we need to have thought about these things, I
suppose.  You should probably look at the code or try it out.

Thanks,
-Christoffer
diff mbox

Patch

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index c370b40..152ee17 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -33,6 +33,7 @@ 
 #include <asm/kvm_emulate.h>
 #include <asm/kvm_host.h>
 #include <asm/kvm_mmu.h>
+#include <asm/pmu.h>
 
 #include <trace/events/kvm.h>
 
@@ -236,6 +237,44 @@  static void reset_mpidr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
 	vcpu_sys_reg(vcpu, MPIDR_EL1) = (1ULL << 31) | mpidr;
 }
 
+static void reset_pmcr_el0(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
+{
+	u32 pmcr;
+
+	asm volatile("mrs %0, pmcr_el0\n" : "=r" (pmcr));
+	vcpu_sys_reg(vcpu, PMCR_EL0) = (pmcr & ~ARMV8_PMCR_MASK)
+				       | (ARMV8_PMCR_MASK & 0xdecafbad);
+}
+
+/* PMCR_EL0 accessor. Only called as long as MDCR_EL2.TPMCR is set. */
+static bool access_pmcr(struct kvm_vcpu *vcpu,
+			const struct sys_reg_params *p,
+			const struct sys_reg_desc *r)
+{
+	unsigned long val;
+
+	if (p->is_write) {
+		/* Only update writeable bits of PMCR */
+		if (!p->is_aarch32)
+			val = vcpu_sys_reg(vcpu, r->reg);
+		else
+			val = vcpu_cp15(vcpu, r->reg);
+		val &= ~ARMV8_PMCR_MASK;
+		val |= *vcpu_reg(vcpu, p->Rt) & ARMV8_PMCR_MASK;
+		if (!p->is_aarch32)
+			vcpu_sys_reg(vcpu, r->reg) = val;
+		else
+			vcpu_cp15(vcpu, r->reg) = val;
+	} else {
+		if (!p->is_aarch32)
+			*vcpu_reg(vcpu, p->Rt) = vcpu_sys_reg(vcpu, r->reg);
+		else
+			*vcpu_reg(vcpu, p->Rt) = vcpu_cp15(vcpu, r->reg);
+	}
+
+	return true;
+}
+
 /* Silly macro to expand the DBG{BCR,BVR,WVR,WCR}n_EL1 registers in one go */
 #define DBG_BCR_BVR_WCR_WVR_EL1(n)					\
 	/* DBGBVRn_EL1 */						\
@@ -427,7 +466,7 @@  static const struct sys_reg_desc sys_reg_descs[] = {
 
 	/* PMCR_EL0 */
 	{ Op0(0b11), Op1(0b011), CRn(0b1001), CRm(0b1100), Op2(0b000),
-	  trap_raz_wi },
+	  access_pmcr, reset_pmcr_el0, PMCR_EL0, },
 	/* PMCNTENSET_EL0 */
 	{ Op0(0b11), Op1(0b011), CRn(0b1001), CRm(0b1100), Op2(0b001),
 	  trap_raz_wi },