Re: [Xen-devel] [PATCH v4 15/17] x86/VPMU: NMI-based VPMU support

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 21.01.14 at 20:09, Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx> wrote:
> Add support for using NMIs as PMU interrupts.
> 
> Most of processing is still performed by vpmu_do_interrupt(). However, since
> certain operations are not NMI-safe we defer them to a softint that 
> vpmu_do_interrupt()
> will schedule:
> * For PV guests that would be send_guest_vcpu_virq() and 
> hvm_get_segment_register().

Makes no sense - why would hvm_get_segment_register() be of any
relevance to PV guests?

And then I'm still missing a reasonable level of analysis that the
previously non-NMI-only interrupt handler is now safe to use in NMI
context.

> +uint32_t vpmu_apic_vector = PMU_APIC_VECTOR;

Considering that you store APIC_DM_NMI into this variable in the
NMI case, it needs to be named differently (or else I'd be tempted
to convert it to uint8_t the first time I stumble across it).

> +static void vpmu_send_nmi(struct vcpu *v)
> +{
> +    struct vlapic *vlapic = vcpu_vlapic(v);

Please ASSERT() that you have HVM data available before doing
anything that would be unsafe in PV (and maybe PVH?) context.
This will then at once serve as documentation, clarifying that the
function must only be used for suitable vCPU-s.

> +/* Process the softirq set by PMU NMI handler */
> +static void pmu_softnmi(void)
> +{
> +    struct cpu_user_regs *regs;
> +    struct vcpu *v, *sampled = per_cpu(sampled_vcpu, smp_processor_id());
> +
> +    if ( vpmu_mode & XENPMU_MODE_PRIV ||

() around the & please.

>  static int pvpmu_init(struct domain *d, xen_pmu_params_t *params)
>  {
>      struct vcpu *v;
>      struct page_info *page;
>      uint64_t gmfn = params->d.val;
> -
> +    static int pvpmu_initted = 0;

bool_t? __read_mostly?

Jan


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