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

Re: [Xen-devel] [PATCH 05/10] x86/msr: Compile out unused logic/objects



On 27.02.2020 11:37, Andrew Cooper wrote:
> On 27/02/2020 08:07, Jan Beulich wrote:
>> On 26.02.2020 21:22, Andrew Cooper wrote:
>>> @@ -76,16 +77,27 @@ void __init init_guest_msr_policy(void)
>>>  {
>>>      calculate_raw_policy();
>>>      calculate_host_policy();
>>> -    calculate_hvm_max_policy();
>>> -    calculate_pv_max_policy();
>>> +
>>> +    if ( IS_ENABLED(CONFIG_PV) )
>>> +        calculate_pv_max_policy();
>>> +
>>> +    if ( hvm_enabled )
>>
>> Any chance of talking you into doing things more symmetrically,
>> by either also using IS_ENABLED(CONFIG_HVM) here or ...
>>
>>> +        calculate_hvm_max_policy();
>>>  }
>>>  
>>>  int init_domain_msr_policy(struct domain *d)
>>>  {
>>> -    struct msr_policy *mp =
>>> -        xmemdup(is_pv_domain(d) ?  &pv_max_msr_policy
>>> -                                : &hvm_max_msr_policy);
>>> +    struct msr_policy *mp = is_pv_domain(d)
>>> +        ? (IS_ENABLED(CONFIG_PV)  ?  &pv_max_msr_policy : NULL)
>>> +        : (IS_ENABLED(CONFIG_HVM) ? &hvm_max_msr_policy : NULL);
>> ... (imo preferably) hvm_enabled here? Either way
>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> The asymmetry is deliberate.
> 
> In the former hunk, hvm_enabled is short-circuited to false for
> !CONFIG_HVM, and if I don't use hvm_enabled, here, then I've got to
> retain the logic at the top of calculate_hvm_max_policy().  That seems
> silly.
> 
> In this later hunk, we are looking for the most efficient way to allow
> the compiler to discard the reference to hvm_max_msr_policy.  Using
> hvm_enabled would be logically equivalent, but compile to more code in
> CONFIG_HVM case, as it is a real boolean needing checking.

Fair enough, albeit I don't think the added evaluation of hvm_enabled
would be the end of the world.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

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