Re: [PATCH v3 11/16] x86/VPMU: Add support for PMU register handling on PV guests

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

On 01/13/2014 10:58 AM, Jan Beulich wrote:
>>>> On 13.01.14 at 16:44, Boris Ostrovsky <boris.ostrovsky@oracle.com> wrote:
>> On 01/13/2014 09:12 AM, Jan Beulich wrote:
>>>>>> On 06.01.14 at 20:24, Boris Ostrovsky <boris.ostrovsky@oracle.com> wrote:
>>>> @@ -875,7 +875,9 @@ void pv_cpuid(struct cpu_user_regs *regs)
>>>>        unsupported:
>>>>            a = b = c = d = 0;
>>>>            break;
>>>> -
>>>> +    case 0x0000000a: /* Architectural Performance Monitor Features (Intel) */
>>>> +        vpmu_do_cpuid(0xa, &a, &b, &c, &d);
>>>> +        break;
>>>>        default:
>>> Rather than removing a blank line here, you ought to insert a
>>> second one so that there's one before _and_ after the added
>>> code block.
>>>
>>> Furthermore the need to pass 0xa as the first argument suggests
>>> that you're not in line with the intentions of vpmu_do_cpuid():
>>> Either you drop the first parameter from the function, or you get
>>> your code in line with the existing caller.
>> Not sure I understand the problem. We fill a, b, c and d with HW values
>> for dom0 and then call vpmu_do_cpuid() to adjust them if needed. And
>> whether or not this is needed is based on the first argument.
> Did you look at the other call site? Iirc it calls the function for all
> input values (and hence the need for the current first parameter
> of the function). I take it that the expectation was that further
> perfctr related leaves might be added later and/or perfctr code
> may need to override more than just leaf 0xa values.
>


Oh, you meant the problem is that I don't cover leaves *other* than 0xa.

I can move vpmu_do_cpuid() down to the exit path ("out" label) so that 
all cpuid calls are checked. This will add latency to every cpuid 
invocation in the guest but I guess it should be negligible (and this is 
what HVM does already).

Incidentally, this will also solve my non-dom0 CPUID bug that I mentioned.

-boris