On 11/02/2020 17:47, Ian Jackson wrote:
> Andrew Cooper writes ("[PATCH 5/6] tools/libx[cl]: Don't use
> HVM_PARAM_PAE_ENABLED as a function parameter"):
>> The sole use of HVM_PARAM_PAE_ENABLED is as a non-standard calling convention
>> for xc_cpuid_apply_policy(). Pass PAE as a regular parameter instead.
>>
>> Leave a rather better explaination of why only HVM guests have a choice in
>> PAE
>> setting.
> I am inclined believe you that this is right (since you are evidently
> familiar with this whole area and I'm not), but the explanations leave
> me confused.
>
>> int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid,
>> - const uint32_t *featureset, unsigned int
>> nr_features)
>> + const uint32_t *featureset, unsigned int
>> nr_features,
>> + bool pae)
>> {
>> int rc;
>> xc_dominfo_t di;
>> @@ -579,8 +580,6 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t
>> domid,
>> }
>> else
>> {
>> - uint64_t val;
>> -
>> /*
>> * Topology for HVM guests is entirely controlled by Xen. For now,
>> we
>> * hardcode APIC_ID = vcpu_id * 2 to give the illusion of no SMT.
>> @@ -635,14 +634,10 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t
>> domid,
>> }
>>
>> /*
>> - * HVM_PARAM_PAE_ENABLED is a parameter to this function, stashed in
>> - * Xen. Nothing else has ever taken notice of the value.
>> + * PAE used to be a parameter passed to this function by
>> + * HVM_PARAM_PAE_ENABLED. It is now passed normally.
> In particular, I don't understand what these comments mean by
> "HVM_PARAM_PAE_ENABLED is a parameter to this function" and "PAE used
> to be a parameter passed to this function by HVM_PARAM_PAE_ENABLED".
>
> Maybe this is some loose use of the term "parameter" ?
>
> If you could explain more clearly (ideally, explain the meaning of the
> old comment in the commit message, and make the new comment
> unambiguous) then that would be great.
HVM_PARAM_PAE_ENABLED encapsulates a boolean meaning "should I advertise
the PAE feature to the guest?".
It has only ever been used in a way which should have been "bool pae"
passed into xc_cpuid_apply_policy(). This patch tries to do just that.
I think there might be confusion as to which comment the commit message
referred to.
In xc_cpuid_apply_policy(), I want a comment explaining why we have this
weird pae parameter. It will disappear from the new way of doing CPUID
at boot, but will have to remain for the pre-4.14 compatibility.
The comment I was referring to in the commit message was actually the
libxl comment, explaining why PV and PVH guests don't get a choice to
hide the PAE feature.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
