Re: [Xen-devel] [PATCH 5/6] tools/libx[cl]: Don't use HVM_PARAM_PAE_ENABLED as a function parameter

From: Ian Jackson <ian.jackson@citrix.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Anthony Perard <anthony.perard@citrix.com>,
	Xen-devel <xen-devel@lists.xenproject.org>, Wei Liu <wl@xen.org>
Subject: Re: [Xen-devel] [PATCH 5/6] tools/libx[cl]: Don't use HVM_PARAM_PAE_ENABLED as a function parameter
Date: Tue, 11 Feb 2020 17:47:15 +0000	[thread overview]
Message-ID: <24130.59555.850210.126017@mariner.uk.xensource.com> (raw)
In-Reply-To: <20200205165056.11734-6-andrew.cooper3@citrix.com>

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.

Thanks,
Ian.

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