Re: [PATCH] x86/vpmu_intel: Fix hypervisor crash by catching wrmsr fault

[Mohit Gambhir <mohit.gambhir@oracle.com>]

On 04/24/2017 12:00 PM, Boris Ostrovsky wrote:
>> Also, from the description in the SDM, PC flag bit it seems very
>> disruptive to me.
>> SDM says that if the bit is set then the processor toggles the PMi pin
>> (generating a performance monitoring interrupt?)
>> every time the event occurs. So if we program the counter to count
>> "unhaulted core cycles", and set PC flag bit
>> it will generate an interrupts every cycle!?
> I checked how Linux treats this bit and there is this interesting commit:
>
>
> commit a7b9d2ccc3d86303ee9314612d301966e04011c7
> Author: Gleb Natapov <gleb@redhat.com>
> Date:   Sun Feb 26 16:55:40 2012 +0200
>
>      KVM: PMU: warn when pin control is set in eventsel msr
>     
>      Print warning once if pin control bit is set in eventsel msr since
>      emulation does not support it yet.
>     
>      Signed-off-by: Gleb Natapov <gleb@redhat.com>
>      Signed-off-by: Avi Kivity <avi@redhat.com>
>
> diff --git a/arch/x86/include/asm/perf_event.h
> b/arch/x86/include/asm/perf_event.h
> index 096c975..f1f7182 100644
> --- a/arch/x86/include/asm/perf_event.h
> +++ b/arch/x86/include/asm/perf_event.h
> @@ -23,6 +23,7 @@
>   #define ARCH_PERFMON_EVENTSEL_USR                      (1ULL << 16)
>   #define ARCH_PERFMON_EVENTSEL_OS                       (1ULL << 17)
>   #define ARCH_PERFMON_EVENTSEL_EDGE                     (1ULL << 18)
> +#define ARCH_PERFMON_EVENTSEL_PIN_CONTROL              (1ULL << 19)
>   #define ARCH_PERFMON_EVENTSEL_INT                      (1ULL << 20)
>   #define ARCH_PERFMON_EVENTSEL_ANY                      (1ULL << 21)
>   #define ARCH_PERFMON_EVENTSEL_ENABLE                   (1ULL << 22)
> diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
> index 3e48c1d..6af9a54 100644
> --- a/arch/x86/kvm/pmu.c
> +++ b/arch/x86/kvm/pmu.c
> @@ -210,6 +210,9 @@ static void reprogram_gp_counter(struct kvm_pmc
> *pmc, u64 eventsel)
>          unsigned config, type = PERF_TYPE_RAW;
>          u8 event_select, unit_mask;
>   
> +       if (eventsel & ARCH_PERFMON_EVENTSEL_PIN_CONTROL)
> +               printk_once("kvm pmu: pin control bit is ignored\n");
> +
>          pmc->eventsel = eventsel;
>   
>          stop_counter(pmc);

Given that we know this now, will it makes sense to go with solution 1 
in my cover letter for this patch -

" 1. Mask the PC bit in the VPMU so as to not allow any writes to it from guests
on any Intel machine. "

But even then the question remains whether we should return an error when user tries to
set this bit or should we just silently mask it the way KVM does?

Mohit


>
>
> -boris
>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel