Re: [Xen-devel] [PATCH v2 01/25] x86/cpuid: Introduce guest_cpuid() and struct cpuid_leaf

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 09/01/17 15:40, Jan Beulich wrote:
>>>> On 09.01.17 at 16:09, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 09/01/17 15:04, Jan Beulich wrote:
>>>>>> On 09.01.17 at 12:03, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>> @@ -215,6 +218,39 @@ const uint32_t * __init lookup_deep_deps(uint32_t 
>> feature)
>>>>      return NULL;
>>>>  }
>>>>  
>>>> +void guest_cpuid(const struct vcpu *v, uint32_t leaf,
>>>> +                 uint32_t subleaf, struct cpuid_leaf *res)
>>>> +{
>>>> +    const struct vcpu *curr = current;
>>>> +    const struct domain *d = v->domain;
>>>> +
>>>> +    *res = EMPTY_LEAF;
>>>> +
>>>> +    /* {hvm,pv}_cpuid() have this expectation. */
>>>> +    ASSERT(v == curr);
>>> You don't appear to use curr other than here, so I'd prefer if you
>>> eliminated the local variable.
>> Further uses are added in future changesets.
> Well, having both v and curr available in a function is likely going to
> be (a little) confusing, especially if you mean to use curr (and hence
> apparently ignore the passed in argument). I'd prefer if you ditched
> curr, compared v against current, and used v throughout the
> function (perhaps in anticipation of the expectation above to go
> away eventually).

The two uses of current are this one, and the decision to defer the
dynamic changes.

This one is temporary while the legacy path exists, while the dynamic
one will stay.  I will just use two manual uses of current.

~Andrew

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