[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3 12/16] x86/VPMU: Handle PMU interrupts for PV guests



>>> On 16.01.14 at 15:58, Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx> wrote:
> On 01/16/2014 04:15 AM, Jan Beulich wrote:
>>> @@ -82,7 +87,23 @@ int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
>>> ...
>>> +        {
>>> +            vpmu_set(vpmu, VPMU_CONTEXT_SAVE);
>>> +            (void)vpmu->arch_vpmu_ops->arch_vpmu_save(current);
>> What's this cast good for?
> 
> These routines return an int that indicates whether this was full 
> context save or just that the counters have been stopped. Depending on 
> where the routines are called from we may ignore the return value 
> (having VPMU_CONTEXT_SAVE forces "full save").
> 
> I did it so that compiler won't complain about ignoring return value (I 
> don't think it does now but I imagine some version still may).

If any were, we'd have many more places where such casts to void
would be needed. The only place where such make sense are macros
that produce a value which is to be ignored.

>>> +    /* dom0 will handle this interrupt */
>>> +    if ( v->domain->domain_id >= DOMID_FIRST_RESERVED )
>>> +    {
>>> +        if ( smp_processor_id() >= dom0->max_vcpus )
>>> +            return 0;
>>> +        v = dom0->vcpu[smp_processor_id()];
>>> +    }
>> Ugly new uses of "dom0". And the correlation between
>> smp_processor_id() and dom0->max_vcpus doesn't look sane
>> either.
> 
> Yes, this needs to be fixed. Maybe
> 
>      v = dom0->vcpu[smp_processor_id() % dom0->max_vcpus];
> 
> And drop the check. (And the same change in pmu_softnmi() in a later patch).

Looks odd, but as long as the kernel side can cope...

> Not sure what to do about dom0. What else can I use instead? I need to 
> choose dom0's vcpu.

Just think the disaggregation way - ultimately it may not be Dom0
that handles this. But I said "ugly", not "unacceptable", knowing
that for the time being you likely have no reasonable alternative.

Jan


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.