Re: [PATCH 1/6] x86/boot: Rework dom0 feature configuration

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.05.2023 11:45, Andrew Cooper wrote:
> On 16/05/2023 8:58 am, Jan Beulich wrote:
>> On 15.05.2023 16:42, Andrew Cooper wrote:
>>> @@ -858,7 +839,7 @@ void __init init_dom0_cpuid_policy(struct domain *d)
>>>       * so dom0 can turn off workarounds as appropriate.  Temporary, until 
>>> the
>>>       * domain policy logic gains a better understanding of MSRs.
>>>       */
>>> -    if ( cpu_has_arch_caps )
>>> +    if ( is_hardware_domain(d) && cpu_has_arch_caps )
>>>          p->feat.arch_caps = true;
>> As a result of this, ...
>>
>>> @@ -876,8 +857,32 @@ void __init init_dom0_cpuid_policy(struct domain *d)
>>>          }
>>>  
>>>          x86_cpu_featureset_to_policy(fs, p);
>>> +    }
>>> +
>>> +    /*
>>> +     * PV Control domains used to require unfiltered CPUID.  This was 
>>> fixed in
>>> +     * Xen 4.13, but there is an cmdline knob to restore the prior 
>>> behaviour.
>>> +     *
>>> +     * If the domain is getting unfiltered CPUID, don't let the guest 
>>> kernel
>>> +     * play with CPUID faulting either, as Xen's CPUID path won't cope.
>>> +     */
>>> +    if ( !opt_dom0_cpuid_faulting && is_control_domain(d) && 
>>> is_pv_domain(d) )
>>> +        p->platform_info.cpuid_faulting = false;
>>>  
>>> -        recalculate_cpuid_policy(d);
>>> +    recalculate_cpuid_policy(d);
>>> +
>>> +    if ( is_hardware_domain(d) && cpu_has_arch_caps )
>> ... it would feel slightly more logical if p->feat.arch_caps was used here.
>> Whether that's to replace the entire condition or merely the right side of
>> the && depends on what the subsequent changes require (which I haven't
>> looked at yet).
> 
> I'd really prefer to leave it as-is.
> 
> You're likely right, but this entire block is deleted in patch 6 and it
> has been a giant pain already making this series bisectable WRT all our
> special cases.
> 
> For the sake of a few patches, it's safer to go with it in the
> pre-existing form.

Oh, sure, if this goes away anyway, then there's not that much point in
making such a change.

Jan