tools/libxc: Reduce feature handling complexity in xc_cpuid_apply_policy()
diff mbox series

Message ID 20200303182326.16739-1-andrew.cooper3@citrix.com
State New
Headers show
Series
  • tools/libxc: Reduce feature handling complexity in xc_cpuid_apply_policy()
Related show

Commit Message

Andrew Cooper March 3, 2020, 6:23 p.m. UTC
xc_cpuid_apply_policy() is gaining extra parameters to untangle CPUID
complexity in Xen.  While an improvement in general, it does have the
unfortunate side effect of duplicating some settings across muliple
parameters.

Rearrange the logic to only consider 'pae' if no explicit featureset is
provided.  This reduces the complexity for callers who have already provided a
pae setting in the featureset.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wl@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Ian Jackson <Ian.Jackson@citrix.com>
---
 tools/libxc/include/xenctrl.h | 6 ++++++
 tools/libxc/xc_cpuid_x86.c    | 7 +++++--
 2 files changed, 11 insertions(+), 2 deletions(-)

Comments

Durrant, Paul March 4, 2020, 9:36 a.m. UTC | #1
> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Andrew Cooper
> Sent: 03 March 2020 18:23
> To: Xen-devel <xen-devel@lists.xenproject.org>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Wei Liu
> <wl@xen.org>; Jan Beulich <JBeulich@suse.com>; Roger Pau Monné <roger.pau@citrix.com>
> Subject: [Xen-devel] [PATCH] tools/libxc: Reduce feature handling complexity in
> xc_cpuid_apply_policy()
> 
> xc_cpuid_apply_policy() is gaining extra parameters to untangle CPUID
> complexity in Xen.  While an improvement in general, it does have the
> unfortunate side effect of duplicating some settings across muliple
> parameters.
> 
> Rearrange the logic to only consider 'pae' if no explicit featureset is
> provided.  This reduces the complexity for callers who have already provided a
> pae setting in the featureset.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Paul Durrant <pdurrant@amzn.com>
Andrew Cooper May 8, 2020, 3:30 p.m. UTC | #2
Tools ping?

~Andrew

On 03/03/2020 18:23, Andrew Cooper wrote:
> xc_cpuid_apply_policy() is gaining extra parameters to untangle CPUID
> complexity in Xen.  While an improvement in general, it does have the
> unfortunate side effect of duplicating some settings across muliple
> parameters.
>
> Rearrange the logic to only consider 'pae' if no explicit featureset is
> provided.  This reduces the complexity for callers who have already provided a
> pae setting in the featureset.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Wei Liu <wl@xen.org>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Ian Jackson <Ian.Jackson@citrix.com>
> ---
>  tools/libxc/include/xenctrl.h | 6 ++++++
>  tools/libxc/xc_cpuid_x86.c    | 7 +++++--
>  2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index fc6e57a1a0..8d13a7e20b 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -1798,6 +1798,12 @@ int xc_cpuid_set(xc_interface *xch,
>                   const unsigned int *input,
>                   const char **config,
>                   char **config_transformed);
> +/*
> + * Make adjustments to the CPUID settings for a domain.
> + *
> + * Either pass a full new @featureset (and @nr_features), or adjust individual
> + * features (@pae).
> + */
>  int xc_cpuid_apply_policy(xc_interface *xch,
>                            uint32_t domid,
>                            const uint32_t *featureset,
> diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
> index 5ced6d18b9..f045b03223 100644
> --- a/tools/libxc/xc_cpuid_x86.c
> +++ b/tools/libxc/xc_cpuid_x86.c
> @@ -532,6 +532,11 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid,
>  
>          cpuid_featureset_to_policy(feat, p);
>      }
> +    else
> +    {
> +        if ( di.hvm )
> +            p->basic.pae = pae;
> +    }
>  
>      if ( !di.hvm )
>      {
> @@ -615,8 +620,6 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid,
>              break;
>          }
>  
> -        p->basic.pae = pae;
> -
>          /*
>           * These settings are necessary to cause earlier HVM_PARAM_NESTEDHVM /
>           * XEN_DOMCTL_disable_migrate settings to be reflected correctly in
Wei Liu May 13, 2020, 1:22 p.m. UTC | #3
On Tue, Mar 03, 2020 at 06:23:26PM +0000, Andrew Cooper wrote:
> xc_cpuid_apply_policy() is gaining extra parameters to untangle CPUID
> complexity in Xen.  While an improvement in general, it does have the
> unfortunate side effect of duplicating some settings across muliple
> parameters.
> 
> Rearrange the logic to only consider 'pae' if no explicit featureset is
> provided.  This reduces the complexity for callers who have already provided a
> pae setting in the featureset.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Wei Liu <wl@xen.org>

Patch
diff mbox series

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index fc6e57a1a0..8d13a7e20b 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1798,6 +1798,12 @@  int xc_cpuid_set(xc_interface *xch,
                  const unsigned int *input,
                  const char **config,
                  char **config_transformed);
+/*
+ * Make adjustments to the CPUID settings for a domain.
+ *
+ * Either pass a full new @featureset (and @nr_features), or adjust individual
+ * features (@pae).
+ */
 int xc_cpuid_apply_policy(xc_interface *xch,
                           uint32_t domid,
                           const uint32_t *featureset,
diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index 5ced6d18b9..f045b03223 100644
--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -532,6 +532,11 @@  int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid,
 
         cpuid_featureset_to_policy(feat, p);
     }
+    else
+    {
+        if ( di.hvm )
+            p->basic.pae = pae;
+    }
 
     if ( !di.hvm )
     {
@@ -615,8 +620,6 @@  int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid,
             break;
         }
 
-        p->basic.pae = pae;
-
         /*
          * These settings are necessary to cause earlier HVM_PARAM_NESTEDHVM /
          * XEN_DOMCTL_disable_migrate settings to be reflected correctly in