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@xxxxxxxxxx wrote:
>>>> From: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
>>>>
>>>> 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@xxxxxxxxxx>
>>>> ---
>>>> 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
>
--
Shannon
--
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
