Re: [Xen-devel] [PATCH v3 3/4] x86/hvm: Add HVM-specific hypervisor CPUID leaf

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 14.03.14 at 15:41, Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx> wrote:
> On 03/14/2014 04:14 AM, Jan Beulich wrote:
>>>>> On 13.03.14 at 19:08, Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx> wrote:
>>> +void hvm_hypervisor_cpuid_leaf(uint32_t idx, uint32_t sub_idx,
>>> +                               uint32_t *eax, uint32_t *ebx,
>>> +                               uint32_t *ecx, uint32_t *edx)
>>> +{
>>> +    if ( idx != 4 )
>>> +        return;
>> What's the point of this check?
> 
> Indeed unnecessary, this is already checked at the caller.
> 
>> Why is "idx" being passed in here in
>> the first place? With you making use of "sub_idx", there's absolutely
>> no reason to expect the need for another leaf to ever get funneled
>> into here.
> 
> I am passing the arguments directly from cpuid_hypervisor_leaves() which
> already has idx and sub_idx and I wasn't sure I can assume that they will be
> in eax and ecx (which they currently are).
> 
> As for sub_idx, I thought that at some point in the future we we might 
> use it
> so I kept it. But I can drop it since it's rather unlikely.

You didn't understand me then: I'd like you to drop "idx", and not
because the value is available elsewhere (and I don't think *eax
and *ecx have anything other than zeros), but because you're
passing a value that can only ever be 4. And I didn't ask you to
drop sub_idx at all, but rather to check that it's zero.

Jan


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