RE: pert stat in KVM guest can not get LLC-loads hardware cache event
> > Inside the guest, I am using "perf stat -e dTLB-load-misses -e > > dTLB-loads -e L1-dcache-loads -e L1-dcache-load-misses -e > > L1-dcache-prefetch-misses ", followed by the parsec command. > > The misses/hit radio is the first number after "#". For your case, 0.00% is the misses/hit radio for dTLB cache. 0.74% is the misses/hit radio for L1dcache. I have no idea what does 80.00% mean. > > > > On Wed, Aug 27, 2014 at 12:28 PM, Liang, Kan wrote: > > > > > >> > Hi, Kan, > > >> > > > >> > The dTLB-load-misses is 0, but it shows 80.00%hit, does that mean > > >> > the > > >> > TLB- load miss is 0.8 * (dTLB-loads). Thanks. > > >> > > > >> > > > >> > 0 dTLB-load-misses #0.00% of all dTLB > > >> > cache hits [80.00%] > > >> > > > >> >782,565,273,315 dTLB-loads > > >> > [80.00%] > > >> > > > >> >782,552,911,616 L1-dcache-loads > > >> > [80.00%] > > >> > > > >> > 5,810,697,456 L1-dcache-load-misses #0.74% of all > > >> > L1-dcache hits [80.00%] > > >> > > > >> > 2,145,907,209 L1-dcache-prefetch-misses > > >> > [80.00%] > > >> > > > >> > - Hui
RE: pert stat in KVM guest can not get LLC-loads hardware cache event
> > Hi, Kan,
> >
> > The dTLB-load-misses is 0, but it shows 80.00%hit, does that mean the
> > TLB- load miss is 0.8 * (dTLB-loads). Thanks.
> >
> > Performance counter stats for './parsecmgmt -a run -i native -c
> > gcc-hooks -n
> > 1 -p freqmine':
I'm not familiar with parsecmgmt. What’s the perf commands it calls?
> >
> >
> > 0 dTLB-load-misses #0.00% of all dTLB
> > cache hits [80.00%]
> >
> >782,565,273,315 dTLB-loads
> > [80.00%]
> >
> >782,552,911,616 L1-dcache-loads
> > [80.00%]
> >
> > 5,810,697,456 L1-dcache-load-misses #0.74% of all
> > L1-dcache hits [80.00%]
> >
> > 2,145,907,209 L1-dcache-prefetch-misses
> > [80.00%]
> >
> > - Hui
> >
> > On Wed, Aug 27, 2014 at 9:05 AM, Liang, Kan wrote:
> > >
> > >
> > >>
> > >> Dear KVM developers:
> > >> I am trying use perf stat inside a VM to obtain some hardware cache
> > >> performance counter values.
> > >> The perf stat can report some numbers for L1 and TLB related
> > >> counters. But for the LLC-loads and LLC-load-misses, the numbers
> > >> are always 0. It seems that the these offcore events are not
> > >> exposed to the
> > guest.
> > >>
> > >> Is this a bug in Qemu or KVM?
> > >>
> > >
> > > There is no offcore virtualization support in KVM yet.
> > > For you case, I guess you are using paravirt for guest, so it should be 0.
> > > Otherwise, you should get #GP in guest.
> > >
> > >
> > >> My testbed is
> > >>
> > >> Host kernel: 3.12.26
> > >> Qemu: 2.1.0
> > >> CPU: Intel Ivy bridge 2620
> > >> VM boosted by qemu, with -cpu host.
> > >>
> > >> Thanks.
> > >>
> > >> - Hui Kang
N�r��yb�X��ǧv�^�){.n�+h����ܨ}���Ơz�&j:+v���zZ+��+zf���h���~i���z� �w���?�&�)ߢf
RE: pert stat in KVM guest can not get LLC-loads hardware cache event
> > 1. If the guest is non-paravirt, can I get the LLC-loads number? No, the guest will crash. > 2. Do you know any method that can capture the LLC-loads for the guest? I don't know. > Thanks. >
RE: pert stat in KVM guest can not get LLC-loads hardware cache event
> > Dear KVM developers: > I am trying use perf stat inside a VM to obtain some hardware cache > performance counter values. > The perf stat can report some numbers for L1 and TLB related counters. But > for the LLC-loads and LLC-load-misses, the numbers are always 0. It seems > that the these offcore events are not exposed to the guest. > > Is this a bug in Qemu or KVM? > There is no offcore virtualization support in KVM yet. For you case, I guess you are using paravirt for guest, so it should be 0. Otherwise, you should get #GP in guest. > My testbed is > > Host kernel: 3.12.26 > Qemu: 2.1.0 > CPU: Intel Ivy bridge 2620 > VM boosted by qemu, with -cpu host. > > Thanks. > > - Hui Kang
RE: [PATCH V6 1/2] perf ignore LBR and extra_rsp
> > > Since nobody ever treats EVENT_EXTRA_END as an actual event, the value > of .extra_msr_access is irrelevant, this leaves the only 'possible' > value 'true' and we can delete all those changes. > Right. > Which, combined with a few whitespace cleanups, gives the below patch. Thanks. Your change is perfect. Do I need to resubmit the patch to the mailing list? Thanks, Kan -- 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
RE: [PATCH V5 1/2] perf ignore LBR and extra_regs
> > diff --git a/arch/x86/kernel/cpu/perf_event.h
> > b/arch/x86/kernel/cpu/perf_event.h
> > index 3b2f9bd..992c678 100644
> > --- a/arch/x86/kernel/cpu/perf_event.h
> > +++ b/arch/x86/kernel/cpu/perf_event.h
> > @@ -464,6 +464,12 @@ struct x86_pmu {
> > */
> > struct extra_reg *extra_regs;
> > unsigned int er_flags;
> > + /*
> > +* EXTRA REG MSR can be accessed
> > +* The extra registers are completely unrelated to each other.
> > +* So it needs a flag for each extra register.
> > +*/
> > + boolextra_msr_access[EXTRA_REG_MAX];
>
> So why not in struct extra_reg again? You didn't give a straight answer there.
I think I did in the email.
You mentioned that there's still (only) 4 empty bytes at the tail of extra_reg
itself.
However, the extra_reg_type may be extended in the near future.
So that may not be a reason to move to extra_reg.
Furthermore, if we move extra_msr_access to extra_reg,
I guess we have to modify all the related micros (i.e EVENT_EXTRA_REG) to
initialize the new items.
That could be a big change.
On the other side, in x86_pmu structure, there are extra_regs related items
defined under the comments "Extra registers for events".
And the bit holes are enough for current usage and future extension.
So I guess x86_pmu should be a good place to store the availability of the reg.
/* --- cacheline 6 boundary (384 bytes) --- */
bool lbr_double_abort; /* 384 1 */
/* XXX 7 bytes hole, try to pack */
struct extra_reg * extra_regs; /* 392 8 */
unsigned int er_flags; /* 400 4 */
/* XXX 4 bytes hole, try to pack */
struct perf_guest_switch_msr * (*guest_get_msrs)(int *); /* 408
8 */
/* size: 416, cachelines: 7, members: 64 */
/* sum members: 391, holes: 6, sum holes: 25 */
/* bit holes: 1, sum bit holes: 27 bits */
/* last cacheline: 32 bytes */
>
> > +/*
> > + * Under certain circumstances, access certain MSR may cause #GP.
> > + * The function tests if the input MSR can be safely accessed.
> > + */
> > +static inline bool check_msr(unsigned long msr) {
>
> This reads like a generic function;
>
> > + u64 val_old, val_new, val_tmp;
> > +
> > + /*
> > +* Read the current value, change it and read it back to see if it
> > +* matches, this is needed to detect certain hardware emulators
> > +* (qemu/kvm) that don't trap on the MSR access and always return
> 0s.
> > +*/
> > + if (rdmsrl_safe(msr, &val_old))
> > + goto msr_fail;
> > + /*
> > +* Only chagne it slightly,
> > +* since the higher bits of some MSRs cannot be updated by wrmsrl.
> > +* E.g. MSR_LBR_TOS
> > +*/
> > + val_tmp = val_old ^ 0x3UL;
>
> but this is not generally true; not all MSRs can write the 2 LSB, can they?
> One
> option would be to extend the function with a u64 mask.
Right, the function should be easily used to check all MSRs, not just for the
MSRs I tested.
I will pass a mask as a parameter of the function.
>
> > + if (wrmsrl_safe(msr, val_tmp) ||
> > + rdmsrl_safe(msr, &val_new))
> > + goto msr_fail;
> > +
> > + if (val_new != val_tmp)
> > + goto msr_fail;
> > +
> > + /* Here it's sure that the MSR can be safely accessed.
> > +* Restore the old value and return.
> > +*/
> > + wrmsrl(msr, val_old);
> > +
> > + return true;
> > +
> > +msr_fail:
> > + return false;
> > +}
>
> Also, by now this function is far too large to be inline and in a header.
OK. I will move it to perf_event_intel.c as a static function.
--
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
RE: [PATCH V5 1/2] perf ignore LBR and extra_regs
> -Original Message- > From: Paolo Bonzini [mailto:pbonz...@redhat.com] > Sent: Monday, July 14, 2014 9:40 AM > To: Liang, Kan; Peter Zijlstra > Cc: a...@firstfloor.org; linux-ker...@vger.kernel.org; kvm@vger.kernel.org > Subject: Re: [PATCH V5 1/2] perf ignore LBR and extra_regs > > Il 14/07/2014 15:36, Liang, Kan ha scritto: > >> > Kan Liang, what happens if CONFIG_PARAVIRT=y? Do you get garbage > >> > or just no events reported? > >> > > > Guest rdmsr/wrmsr will eventually call rdmsr_safe/wrmsr_safe. They will > handle the #GP. So there is no error report in guest. > > Yeah, but what's the effect on the output of perf? They can be still counted/sampled, but the results are garbage. Kan > > Paolo -- 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
RE: [PATCH V5 1/2] perf ignore LBR and extra_regs
> >>> For reproducing the issue, please build the kernel with > CONFIG_KVM_INTEL = y (for host kernel). > >>> And CONFIG_PARAVIRT = n and CONFIG_KVM_GUEST = n (for guest > kernel). > >> > >> I'm not sure this is a useful patch. > >> > >> This is #GP'ing just because of a limitation in the PMU; just compile > >> the kernel with CONFIG_PARAVIRT > > > > How's that going to help? If you run kvm -host the VM is lying through > > its teeth is the kernel is going to assume all those MSRs present, > > PARAVIRT isn't going to help with this. > > > >> , or split the "rdmsr is always rdmsr_safe" > >> behavior out of CONFIG_PARAVIRT and into a new Kconfig symbol. > > > > That's not useful either, because non of these code-paths are going to > > check the return value. > > Hmmm, I thought rdmsr_safe was going to return zero, but it just returns > whatever happened to be in edx:eax which maybe should also be fixed. > > Kan Liang, what happens if CONFIG_PARAVIRT=y? Do you get garbage or > just no events reported? > Guest rdmsr/wrmsr will eventually call rdmsr_safe/wrmsr_safe. They will handle the #GP. So there is no error report in guest. -- 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
RE: [PATCH V4 1/2] perf ignore LBR and extra_regs.
> > >
> > > On Wed, Jul 09, 2014 at 02:32:28PM +, Liang, Kan wrote:
> > > >
> > > >
> > > > > On Tue, Jul 08, 2014 at 09:49:40AM -0700, kan.li...@intel.com wrote:
> > > > > > +/*
> > > > > > + * Under certain circumstances, access certain MSR may cause #GP.
> > > > > > + * The function tests if the input MSR can be safely accessed.
> > > > > > + */
> > > > > > +static inline bool check_msr(unsigned long msr) {
> > > > > > + u64 value;
> > > > > > +
> > > > > > + if (rdmsrl_safe(msr, &value) < 0)
> > > > > > + return false;
> > > > > > + if (wrmsrl_safe(msr, value) < 0)
> > > > > > + return false;
> > > > > > + return true;
> > > > > > +}
> > > > >
> > > > > What does this thing return after patch 2? does the write still
> > > > > fault or will KVM silently take writes too?
> > > >
> > > > If applying patch 2, the function will return true. The KVM just
> > > > simply ignore
> > > the reads/writes.
> > >
> > > OK, then that's broken too. We want a function to return false for
> > > any malfunctioning MSR, ignoring writes and returning 0s is not
> > > proper functioning.
> >
> > The patch 2 is to handle the case that the administrator can only
> > patch the host. Don't need to force user to upgrade their guest to fix
> > the crash. And ignore the annoying "unhandled" KVM messages
>
> Sure; but what I meant was, check_msr() is broken when ran on such a
> kernel. You need to fix check_msr() to return failure on these 'ignored'
> MSRs, after all they don't function as expected, they're effectively broken.
The function is designed to check if the MSRs can be safely accessed (no #GP).
It cannot guarantee the correctness of the MSRs.
If KVM applied patch 2 and guest applied patch 1, from the guest's perspective,
the MSRs can be accessed (no #GP triggered). So return true is expected. It
should not be a broken.
The only unexpected thing for guest is that the counting/sampling result for
LBR/extra reg is always 0. But the patch is a short term fix to stop things
from crashing. I think it should be acceptable.
--
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
RE: [PATCH V4 1/2] perf ignore LBR and extra_regs.
> -Original Message-
> From: Peter Zijlstra [mailto:pet...@infradead.org]
> Sent: Wednesday, July 09, 2014 10:58 AM
> To: Liang, Kan
> Cc: a...@firstfloor.org; linux-ker...@vger.kernel.org; kvm@vger.kernel.org
> Subject: Re: [PATCH V4 1/2] perf ignore LBR and extra_regs.
>
> On Wed, Jul 09, 2014 at 02:32:28PM +, Liang, Kan wrote:
> >
> >
> > > On Tue, Jul 08, 2014 at 09:49:40AM -0700, kan.li...@intel.com wrote:
> > > > +/*
> > > > + * Under certain circumstances, access certain MSR may cause #GP.
> > > > + * The function tests if the input MSR can be safely accessed.
> > > > + */
> > > > +static inline bool check_msr(unsigned long msr) {
> > > > + u64 value;
> > > > +
> > > > + if (rdmsrl_safe(msr, &value) < 0)
> > > > + return false;
> > > > + if (wrmsrl_safe(msr, value) < 0)
> > > > + return false;
> > > > + return true;
> > > > +}
> > >
> > > What does this thing return after patch 2? does the write still
> > > fault or will KVM silently take writes too?
> >
> > If applying patch 2, the function will return true. The KVM just simply
> > ignore
> the reads/writes.
>
> OK, then that's broken too. We want a function to return false for any
> malfunctioning MSR, ignoring writes and returning 0s is not proper
> functioning.
The patch 2 is to handle the case that the administrator can only patch the
host. Don't need to force user to upgrade their guest to fix the crash.
And ignore the annoying "unhandled" KVM messages
--
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
RE: [PATCH V4 1/2] perf ignore LBR and extra_regs.
> On Tue, Jul 08, 2014 at 09:49:40AM -0700, kan.li...@intel.com wrote:
> > +/*
> > + * Under certain circumstances, access certain MSR may cause #GP.
> > + * The function tests if the input MSR can be safely accessed.
> > + */
> > +static inline bool check_msr(unsigned long msr) {
> > + u64 value;
> > +
> > + if (rdmsrl_safe(msr, &value) < 0)
> > + return false;
> > + if (wrmsrl_safe(msr, value) < 0)
> > + return false;
> > + return true;
> > +}
>
> What does this thing return after patch 2? does the write still fault or will
> KVM silently take writes too?
If applying patch 2, the function will return true. The KVM just simply ignore
the reads/writes.
--
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
RE: [PATCH V4 1/2] perf ignore LBR and extra_regs.
>
> On Tue, Jul 08, 2014 at 09:49:40AM -0700, kan.li...@intel.com wrote:
> > --- a/arch/x86/kernel/cpu/perf_event.h
> > +++ b/arch/x86/kernel/cpu/perf_event.h
> > @@ -464,6 +464,12 @@ struct x86_pmu {
> > */
> > struct extra_reg *extra_regs;
> > unsigned int er_flags;
> > + /*
> > +* EXTRA REG MSR can be accessed
> > +* The extra registers are completely unrelated to each other.
> > +* So it needs a flag for each extra register.
> > +*/
> > + boolextra_msr_access[EXTRA_REG_MAX];
> >
> > /*
> > * Intel host/guest support (KVM)
>
> # pahole -C extra_reg arch/x86/kernel/cpu/perf_event_intel.o
> struct extra_reg {
> unsigned int event;/* 0 4 */
> unsigned int msr; /* 4 4 */
> u64config_mask; /* 8 8 */
> u64valid_mask; /*16 8 */
> intidx; /*24 4 */
>
> /* size: 32, cachelines: 1, members: 5 */
> /* padding: 4 */
> /* last cacheline: 32 bytes */
> };
>
> There's still 4 empty bytes at the tail of extra_reg itself; would it make
> sense
> to store the availability of the reg in there?
>
Now, it perfectly matches the 4 empty bytes. However, the extra_reg_type may be
extended in the near future.
> After all; the place we use it (x86_pmu_extra_regs) already has the pointer
> to the structure.
Yes, it's doable. However, if we move extra_msr_access to extra_reg, I guess we
have to modify all the related micros (i.e EVENT_EXTRA_REG) to initialize the
new items. That could be a big change.
On the other side, in x86_pmu structure, there are extra_regs related items
defined under the comments "Extra registers for events". And the bit holes are
enough for current usage and future extension.
I guess x86_pmu should be a good place to store the availability of the reg.
/* --- cacheline 6 boundary (384 bytes) --- */
bool lbr_double_abort; /* 384 1 */
/* XXX 7 bytes hole, try to pack */
struct extra_reg * extra_regs; /* 392 8 */
unsigned int er_flags; /* 400 4 */
/* XXX 4 bytes hole, try to pack */
struct perf_guest_switch_msr * (*guest_get_msrs)(int *); /* 408 8
*/
/* size: 416, cachelines: 7, members: 64 */
/* sum members: 391, holes: 6, sum holes: 25 */
/* bit holes: 1, sum bit holes: 27 bits */
/* last cacheline: 32 bytes */
Thanks,
Kan
--
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
RE: [PATCH V3 1/2] perf ignore LBR and offcore_rsp.
> -Original Message-
> From: Peter Zijlstra [mailto:pet...@infradead.org]
> Sent: Tuesday, July 08, 2014 5:29 AM
> To: Liang, Kan
> Cc: a...@firstfloor.org; linux-ker...@vger.kernel.org; kvm@vger.kernel.org
> Subject: Re: [PATCH V3 1/2] perf ignore LBR and offcore_rsp.
>
> On Mon, Jul 07, 2014 at 06:34:25AM -0700, kan.li...@intel.com wrote:
> > @@ -555,7 +577,11 @@ static inline void __x86_pmu_enable_event(struct
> > hw_perf_event *hwc, {
> > u64 disable_mask =
> > __this_cpu_read(cpu_hw_events.perf_ctr_virt_mask);
> >
> > - if (hwc->extra_reg.reg)
> > + if (hwc->extra_reg.reg &&
> > + ((hwc->extra_reg.idx == EXTRA_REG_RSP_0) ?
> > + x86_pmu.extra_msr_access[0] : true) &&
> > + ((hwc->extra_reg.idx == EXTRA_REG_RSP_1) ?
> > + x86_pmu.extra_msr_access[1] : true))
> > wrmsrl(hwc->extra_reg.reg, hwc->extra_reg.config);
> > wrmsrl(hwc->config_base, (hwc->config | enable_mask) &
> > ~disable_mask); }
>
> > diff --git a/arch/x86/kernel/cpu/perf_event_intel.c
> > b/arch/x86/kernel/cpu/perf_event_intel.c
> > index adb02aa..3d18765 100644
> > --- a/arch/x86/kernel/cpu/perf_event_intel.c
> > +++ b/arch/x86/kernel/cpu/perf_event_intel.c
>
> > + /*
> > +* Access extra MSR may cause #GP under certain circumstances.
> > +* E.g. KVM doesn't support offcore event
> > +* Check all extra_regs here.
> > +*/
> > + if (x86_pmu.extra_regs) {
> > + x86_pmu.extra_msr_access[0] =
> > +
> check_msr(x86_pmu.extra_regs[EXTRA_REG_RSP_0].msr);
> > +
> > + /* Not all platforms have EXTRA_REG_RSP_1 */
> > + if (x86_pmu.extra_regs[EXTRA_REG_RSP_1].idx ==
> EXTRA_REG_RSP_1)
> > + x86_pmu.extra_msr_access[1] =
> > +
> check_msr(x86_pmu.extra_regs[EXTRA_REG_RSP_1].msr);
> > + /*
> > +* If there is no EXTRA_REG_RSP_1 support,
> > +* just set the flag to be true.
> > +* So it is ignored at the runtime check.
> > +*/
> > + else
> > + x86_pmu.extra_msr_access[1] = true;
> > + }
>
> This too is wrong in many ways; there's more than 2 extra_msrs on many
> systems.
>
Right, there are four extra reg types on Intel systems. Since my previous test
only triggers the crash with RSP_0 and RSP_1, so I only handle these two msrs.
I will handle all the extra msrs then.
> And the place you check is abysmal, if we know at init time that we don't
> have those MSRs, WTF do you allow event creation that would use them,
> only to then misbehave?
Right, we can check it at all the possible creation places.
I think the most common place to check should be x86_pmu_extra_regs.
For RSP_0 and RSP_1, I also plan to do the check in intel_fixup_er, so
extra_reg will not be updated.
For LBR select, lbr_sel_map will be cleared at runtime once check_msr failed.
Besides the three places, is there any place I missed?
Thanks,
Kan
--
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
RE: [PATCH V3 1/2] perf ignore LBR and offcore_rsp.
>
> On Mon, Jul 07, 2014 at 06:34:25AM -0700, kan.li...@intel.com wrote:
> > + /*
> > +* Access LBR MSR may cause #GP under certain circumstances.
> > +* E.g. KVM doesn't support LBR MSR
> > +* Check all LBT MSR here.
> > +* Disable LBR access if any LBR MSRs can not be accessed.
> > +*/
> > + if (x86_pmu.lbr_nr) {
> > + x86_pmu.lbr_msr_access = check_msr(x86_pmu.lbr_tos);
> > + for (i = 0; i < x86_pmu.lbr_nr; i++) {
> > + x86_pmu.lbr_msr_access &=
> > + check_msr(x86_pmu.lbr_from + i);
> > + x86_pmu.lbr_msr_access &=
> > + check_msr(x86_pmu.lbr_to + i);
> > + }
> > + }
>
> So I was going to refer you to an email I send earlier telling you that this
> was
> wrong, but then found it got stuck in a mailqueue on my laptop :-(
>
> In any case its wrong; just clear lbr_nr and kill lbr_msr_access. We already
> check lbr_nr in all the right places and you added lbr_msr_access to far too
> few of them (also the fact that every place you check lbr_msr_access already
> had an lbr_nr test should've been a clue).
OK. If lbr_nr is well checked at runtime, I will simply set lbr_nr to 0 here
(once check_msr failed).
Thanks,
Kan
--
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
RE: [PATCH V2 2/3] perf protect LBR when Intel PT is enabled.
> > On Thu, Jul 03, 2014 at 05:52:37PM +0200, Andi Kleen wrote: > > > If there's active LBR users out there, we should refuse to enable PT > > > and vice versa. > > > > This doesn't work, e.g. hardware debuggers can take over at any time. > > Tough cookies. Hardware debuggers get to deal with whatever crap they > cause. If so, I think I may discard this patch (2/3). I will resubmit the other two patches as a patch set to only handle the KVM issue we found. It checks the access of LBR and extra MSRs at the initialization time and set the flags. So we just need to check the flags at runtime and avoid the protection by _safe(). For Intel PT and LBR handling, since the PT codes haven't been integrated yet, I will try to implement another patch later. The patch will add flags for LBR and PT. When enabling PT, it checks LBR flag and update the PT flag. When enabling LBR, it checks PT flag and update the LBR flag. When disabling LBR/PT, we just update the related flags. we don't need to add _safe or extra rmsr in fast path. How do you think? Thanks, Kan -- 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
RE: [PATCH V2 1/3] perf ignore LBR and offcore_rsp.
> > Signed-off-by: Andi Kleen > > I did not contribute to this patch, so please remove that SOB. > OK > > Signed-off-by: Kan Liang > > > struct extra_reg *extra_regs; > > unsigned int er_flags; > > + boolextra_msr_access; /* EXTRA REG MSR can be > accessed */ > > > > This doesn't look right, needs a flag for each extra register. > > They are completely unrelated to each other. The extra register is either MSR_OFFCORE_RSP_0 or MSR_OFFCORE_RSP_1. I will add two variables to handle them. > > BTW this will also cause KVM messages at each boot now. > Do you mean the "unhandled wrmsr" or " unhandled rdmsr " messages? You should not observer such error message for offcore msrs and LBR from/to msrs. But I forget to handle LBR TOS in KVM patch. You may observed unhandled rdmsr: 0x1c9 when doing LBR in guest. I will fix it in next version. > > wrmsrl(hwc->extra_reg.reg, hwc->extra_reg.config); > > wrmsrl(hwc->config_base, (hwc->config | enable_mask) & > > ~disable_mask); } diff --git a/arch/x86/kernel/cpu/perf_event_intel.c > > b/arch/x86/kernel/cpu/perf_event_intel.c > > index adb02aa..8011d42 100644 > > --- a/arch/x86/kernel/cpu/perf_event_intel.c > > +++ b/arch/x86/kernel/cpu/perf_event_intel.c > > @@ -2565,6 +2565,13 @@ __init int intel_pmu_init(void) > > } > > } > > > > + /* Access LBR MSR may cause #GP under certain circumstances. E.g. > KVM doesn't support LBR MSR */ > > + if (x86_pmu.lbr_nr) > > + x86_pmu.lbr_msr_access = > test_msr_access(x86_pmu.lbr_tos) & > > +test_msr_access(x86_pmu.lbr_from); > > s/&/&&/ > > And also this doesn't cover the case when someone takes over the LBRs and > they start #GPing later. > So for LBR the test has to be still at each access. In the second patch, the LBR test has been moved to runtime check. This case has been handled. Kan > > -Andi -- 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
RE: [PATCH V2 1/3] perf ignore LBR and offcore_rsp.
> > On Wed, Jul 2, 2014 at 2:14 PM, wrote: > > From: Kan Liang > > > > x86, perf: Protect LBR and offcore rsp against KVM lying > > > > With -cpu host, KVM reports LBR and offcore support, if the host has > support. > > When the guest perf driver tries to access LBR or offcore_rsp MSR, it > > #GPs all MSR accesses,since KVM doesn't handle LBR and offcore support. > > So check the related MSRs access right once at initialization time to avoid > the error access at runtime. > > > > For reproducing the issue, please build the kernel with > CONFIG_KVM_INTEL = y. > This is for host kernel, > > And CONFIG_PARAVIRT = n and CONFIG_KVM_GUEST = n. > And this is for guest kernel, right? > Right. > -Jidong
