Re: [Xen-devel] [PATCH v2 10/13] x86/PMU: Add support for PMU registes handling on PV guests

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 20.09.13 at 11:42, Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx> wrote:
> Intercept accesses to PMU MSRs and LVTPC APIC vector (only
> APIC_LVT_MASKED bit is processed) and process them in VPMU
> module.

Having scrolled through this more than once, I still can't see where
any APIC interception is happening here.

> @@ -2486,6 +2488,17 @@ static int emulate_privileged_op(struct cpu_user_regs 
> *regs)
>              if ( wrmsr_safe(regs->ecx, msr_content) != 0 )
>                  goto fail;
>              break;
> +        case MSR_P6_PERFCTR0...MSR_P6_PERFCTR1:
> +        case MSR_P6_EVNTSEL0...MSR_P6_EVNTSEL1:
> +        case MSR_CORE_PERF_FIXED_CTR0...MSR_CORE_PERF_FIXED_CTR2:
> +        case MSR_CORE_PERF_FIXED_CTR_CTRL...MSR_CORE_PERF_GLOBAL_OVF_CTRL:
> +        case MSR_AMD_FAM15H_EVNTSEL0...MSR_AMD_FAM15H_PERFCTR5:
> +            if ( !vpmu_do_wrmsr(regs->ecx, msr_content) )
> +            {
> +                if ( (vpmu_mode & XENPMU_MODE_PRIV) && (v->domain == dom0) )

This is identical to checking ->dom_id against zero, yet we started
moving away from that model.

> @@ -2574,6 +2587,24 @@ static int emulate_privileged_op(struct cpu_user_regs 
> *regs)
>              regs->eax = (uint32_t)msr_content;
>              regs->edx = (uint32_t)(msr_content >> 32);
>              break;
> +        case MSR_IA32_PERF_CAPABILITIES:
> +            if ( rdmsr_safe(regs->ecx, msr_content) )
> +                goto fail;
> +            /* Full-Width Writes not supported */
> +            regs->eax = (uint32_t)msr_content & ~(1 << 13);
> +            regs->edx = (uint32_t)(msr_content >> 32);

Rather than black listing, please white list know good features
here.

> +            break;
> +        case MSR_P6_PERFCTR0...MSR_P6_PERFCTR1:
> +        case MSR_P6_EVNTSEL0...MSR_P6_EVNTSEL1:
> +        case MSR_CORE_PERF_FIXED_CTR0...MSR_CORE_PERF_FIXED_CTR2:
> +        case MSR_CORE_PERF_FIXED_CTR_CTRL...MSR_CORE_PERF_GLOBAL_OVF_CTRL:
> +        case MSR_AMD_FAM15H_EVNTSEL0...MSR_AMD_FAM15H_PERFCTR5:
> +            if ( vpmu_do_rdmsr(regs->ecx, &msr_content) ) {

Coding style.

> +    case 0x33: /* RDPMC */
> +        rdpmc(regs->ecx, regs->eax, regs->edx);
> +        break;

This will #GP on invalid counter index, i.e. you're creating a DoS here.

Jan


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