[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] x86/cpu-policy: Extend the guest max policy max leaf/subleaves
On 30.10.2024 17:51, Andrew Cooper wrote: > On 30/10/2024 3:13 pm, Roger Pau Monné wrote: >> On Wed, Oct 30, 2024 at 02:45:19PM +0000, Andrew Cooper wrote: >>> On 30/10/2024 11:03 am, Roger Pau Monné wrote: >>>> On Wed, Oct 30, 2024 at 10:39:12AM +0000, Andrew Cooper wrote: >>>>> On 30/10/2024 8:59 am, Roger Pau Monné wrote: >>>>>> On Tue, Oct 29, 2024 at 05:55:05PM +0000, Andrew Cooper wrote: >>>>>>> diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c >>>>>>> index b6d9fad56773..78bc9872b09a 100644 >>>>>>> --- a/xen/arch/x86/cpu-policy.c >>>>>>> +++ b/xen/arch/x86/cpu-policy.c >>>>>>> @@ -391,6 +391,27 @@ static void __init calculate_host_policy(void) >>>>>>> p->platform_info.cpuid_faulting = cpu_has_cpuid_faulting; >>>>>>> } >>>>>>> >>>>>>> +/* >>>>>>> + * Guest max policies can have any max leaf/subleaf within bounds. >>>>>>> + * >>>>>>> + * - Some incoming VMs have a larger-than-necessary feat max_subleaf. >>>>>>> + * - Some VMs we'd like to synthesise leaves not present on the host. >>>>>>> + */ >>>>>>> +static void __init guest_common_max_leaves(struct cpu_policy *p) >>>>>>> +{ >>>>>>> + p->basic.max_leaf = ARRAY_SIZE(p->basic.raw) - 1; >>>>>>> + p->feat.max_subleaf = ARRAY_SIZE(p->feat.raw) - 1; >>>>>>> + p->extd.max_leaf = 0x80000000U + ARRAY_SIZE(p->extd.raw) - >>>>>>> 1; >>>>>>> +} >>>>>>> + >>>>>>> +/* Guest default policies inherit the host max leaf/subleaf settings. >>>>>>> */ >>>>>>> +static void __init guest_common_default_leaves(struct cpu_policy *p) >>>>>>> +{ >>>>>>> + p->basic.max_leaf = host_cpu_policy.basic.max_leaf; >>>>>>> + p->feat.max_subleaf = host_cpu_policy.feat.max_subleaf; >>>>>>> + p->extd.max_leaf = host_cpu_policy.extd.max_leaf; >>>>>>> +} >>>>>> I think this what I'm going to ask is future work. After the >>>>>> modifications done to the host policy by max functions >>>>>> (calculate_{hvm,pv}_max_policy()) won't the max {sub,}leaf adjustments >>>>>> better be done taking into account the contents of the policy, rather >>>>>> than capping to the host values? >>>>>> >>>>>> (note this comment is strictly for guest_common_default_leaves(), the >>>>>> max version is fine using ARRAY_SIZE). >>>>> I'm afraid I don't follow. >>>>> >>>>> calculate_{pv,hvm}_max_policy() don't modify the host policy. >>>> Hm, I don't think I've expressed myself clearly, sorry. Let me try >>>> again. >>>> >>>> calculate_{hvm,pv}_max_policy() extends the host policy by possibly >>>> setting new features, and such extended policy is then used as the >>>> base for the PV/HVM default policies. >>>> >>>> Won't the resulting policy in calculate_{hvm,pv}_def_policy() risks >>>> having bits set past the max {sub,}leaf in the host policy, as it's >>>> based in {hvm,pv}_def_cpu_policy that might have such bits set? >>> Oh, right. >>> >>> This patch doesn't change anything WRT that. >> Indeed, didn't intend my comment to block it, just that I think at >> some point the logic in guest_common_default_leaves() will need to be >> expanded. >> >>> But I think you're right that we do risk getting into that case (in >>> principle at least) because of how guest_common_*_feature_adjustment() work. >>> >>> Furthermore, the bug will typically get hidden because we serialise >>> based on the max_leaf/subleaf, and will discard feature words outside of >>> the max_leaf/subleaf bounds. >> Yes, once we serialize it for toolstack consumption the leafs will be >> implicitly zeroed. >> >>> I suppose we probably want a variation of x86_cpu_featureset_to_policy() >>> which extends the max_leaf/subleaf based on non-zero values in leaves. >>> (This already feels like it's going to be an ugly algorithm.) >> Hm, I was thinking that we would need to adjust >> guest_common_default_leaves() to properly shrink the max {sub,}leaf >> fields from the max policies. > > Hmm. What we'd do is have default inherit max's ARRAY_SIZES(), then do > all the existing logic, then as the final step, shrink the default > policies, vaguely per Jan's plan. Yet, beyond what my present patch has, not below anything the tool stack has asked for explicitly. Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |