All of lore.kernel.org
 help / color / mirror / Atom feed
* [Xen-devel] [PATCH] x86/sysctl: Don't return cpu policy data for compiled-out support
@ 2020-02-25 17:31 Andrew Cooper
  2020-02-26  9:32 ` Jan Beulich
  0 siblings, 1 reply; 4+ messages in thread
From: Andrew Cooper @ 2020-02-25 17:31 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich, Roger Pau Monné

Policy objects aren't tiny, and the derivation logic isn't trivial.  We are
about to increase the number of policy objects, so take this opportunity to
drop logic and storage space based on CONFIG_{PV,HVM}.

Start by causing XEN_SYSCTL_get_cpu_policy to fail with -EOPNOTSUPP when
requesting data for a compiled-out subsystem.  Update xen-cpuid to cope and
continue to further system policies, seeing as the indicies are interleaved.

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>
---
 tools/misc/xen-cpuid.c      |  9 +++++++++
 xen/arch/x86/sysctl.c       | 20 +++++++++++++++-----
 xen/include/public/sysctl.h |  2 ++
 3 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
index 7726c4ed3c..f55b67640a 100644
--- a/tools/misc/xen-cpuid.c
+++ b/tools/misc/xen-cpuid.c
@@ -480,7 +480,16 @@ int main(int argc, char **argv)
 
                 if ( xc_get_system_cpu_policy(xch, i, &nr_leaves, leaves,
                                               &nr_msrs, msrs) )
+                {
+                    if ( errno == EOPNOTSUPP )
+                    {
+                        printf("%s policy not supported by Xen\n",
+                               sys_policies[i]);
+                        continue;
+                    }
+
                     err(1, "xc_get_system_cpu_policy(, %s,,)", sys_policies[i]);
+                }
 
                 print_policy(sys_policies[i], leaves, nr_leaves,
                              msrs, nr_msrs);
diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
index 4a76f0f47f..59a384023b 100644
--- a/xen/arch/x86/sysctl.c
+++ b/xen/arch/x86/sysctl.c
@@ -33,7 +33,7 @@
 #include <asm/psr.h>
 #include <asm/cpuid.h>
 
-const struct cpu_policy system_policies[] = {
+const struct cpu_policy system_policies[6] = {
     [ XEN_SYSCTL_cpu_policy_raw ] = {
         &raw_cpuid_policy,
         &raw_msr_policy,
@@ -42,22 +42,26 @@ const struct cpu_policy system_policies[] = {
         &host_cpuid_policy,
         &host_msr_policy,
     },
+#ifdef CONFIG_PV
     [ XEN_SYSCTL_cpu_policy_pv_max ] = {
         &pv_max_cpuid_policy,
         &pv_max_msr_policy,
     },
-    [ XEN_SYSCTL_cpu_policy_hvm_max ] = {
-        &hvm_max_cpuid_policy,
-        &hvm_max_msr_policy,
-    },
     [ XEN_SYSCTL_cpu_policy_pv_default ] = {
         &pv_max_cpuid_policy,
         &pv_max_msr_policy,
     },
+#endif
+#ifdef CONFIG_HVM
+    [ XEN_SYSCTL_cpu_policy_hvm_max ] = {
+        &hvm_max_cpuid_policy,
+        &hvm_max_msr_policy,
+    },
     [ XEN_SYSCTL_cpu_policy_hvm_default ] = {
         &hvm_max_cpuid_policy,
         &hvm_max_msr_policy,
     },
+#endif
 };
 
 struct l3_cache_info {
@@ -426,6 +430,12 @@ long arch_do_sysctl(
             array_index_nospec(sysctl->u.cpu_policy.index,
                                ARRAY_SIZE(system_policies))];
 
+        if ( !policy->cpuid || !policy->msr )
+        {
+            ret = -EOPNOTSUPP;
+            break;
+        }
+
         /* Process the CPUID leaves. */
         if ( guest_handle_is_null(sysctl->u.cpu_policy.cpuid_policy) )
             sysctl->u.cpu_policy.nr_leaves = CPUID_MAX_SERIALISED_LEAVES;
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 7e43bfe1bd..4dfba39ed8 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -1051,6 +1051,8 @@ struct xen_sysctl_set_parameter {
  *               experimental features outside of security support.
  *  - Default_*: Default set of features a PV or HVM guest can use.  This is
  *               the security supported set.
+ * May fail with -EOPNOTSUPP if querying for PV or HVM data when support is
+ * compiled out of Xen.
  */
 struct xen_sysctl_cpu_policy {
 #define XEN_SYSCTL_cpu_policy_raw          0
-- 
2.11.0


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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Xen-devel] [PATCH] x86/sysctl: Don't return cpu policy data for compiled-out support
  2020-02-25 17:31 [Xen-devel] [PATCH] x86/sysctl: Don't return cpu policy data for compiled-out support Andrew Cooper
@ 2020-02-26  9:32 ` Jan Beulich
  2020-02-26  9:58   ` Andrew Cooper
  0 siblings, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2020-02-26  9:32 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Xen-devel, Wei Liu, Roger Pau Monné

On 25.02.2020 18:31, Andrew Cooper wrote:
> Policy objects aren't tiny, and the derivation logic isn't trivial.  We are
> about to increase the number of policy objects, so take this opportunity to
> drop logic and storage space based on CONFIG_{PV,HVM}.

It doesn't look as if you were dropping either logic or storage space. With
this aspect taken care of (by adjusting wording, or by clarification of what
is meant) ...

> Start by causing XEN_SYSCTL_get_cpu_policy to fail with -EOPNOTSUPP when
> requesting data for a compiled-out subsystem.  Update xen-cpuid to cope and
> continue to further system policies, seeing as the indicies are interleaved.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Xen-devel] [PATCH] x86/sysctl: Don't return cpu policy data for compiled-out support
  2020-02-26  9:32 ` Jan Beulich
@ 2020-02-26  9:58   ` Andrew Cooper
  2020-02-26 10:43     ` Jan Beulich
  0 siblings, 1 reply; 4+ messages in thread
From: Andrew Cooper @ 2020-02-26  9:58 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Xen-devel, Wei Liu, Roger Pau Monné

On 26/02/2020 09:32, Jan Beulich wrote:
> On 25.02.2020 18:31, Andrew Cooper wrote:
>> Policy objects aren't tiny, and the derivation logic isn't trivial.  We are
>> about to increase the number of policy objects, so take this opportunity to
>> drop logic and storage space based on CONFIG_{PV,HVM}.
> It doesn't look as if you were dropping either logic or storage space. With
> this aspect taken care of (by adjusting wording, or by clarification of what
> is meant) ...
>
>> Start by

That is this is here to signify.

No logic or storage space can be dropped until some #ifdef-ary is
sprinkled around cpuid.c and msr.c, but I can't do any of that while
there are unguarded external references to the objects.

~Andrew

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Xen-devel] [PATCH] x86/sysctl: Don't return cpu policy data for compiled-out support
  2020-02-26  9:58   ` Andrew Cooper
@ 2020-02-26 10:43     ` Jan Beulich
  0 siblings, 0 replies; 4+ messages in thread
From: Jan Beulich @ 2020-02-26 10:43 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Xen-devel, Wei Liu, Roger Pau Monné

On 26.02.2020 10:58, Andrew Cooper wrote:
> On 26/02/2020 09:32, Jan Beulich wrote:
>> On 25.02.2020 18:31, Andrew Cooper wrote:
>>> Policy objects aren't tiny, and the derivation logic isn't trivial.  We are
>>> about to increase the number of policy objects, so take this opportunity to
>>> drop logic and storage space based on CONFIG_{PV,HVM}.
>> It doesn't look as if you were dropping either logic or storage space. With
>> this aspect taken care of (by adjusting wording, or by clarification of what
>> is meant) ...
>>
>>> Start by
> 
> That is this is here to signify.

How about s/take this/will have the/ in the sentence further up then?
To me, "Start by" in no way indicates that really there aren't any
savings yet. Might be my non-native English understanding of this
phrase, of course.

> No logic or storage space can be dropped until some #ifdef-ary is
> sprinkled around cpuid.c and msr.c, but I can't do any of that while
> there are unguarded external references to the objects.

Of course.

Jan

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-02-26 10:43 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-25 17:31 [Xen-devel] [PATCH] x86/sysctl: Don't return cpu policy data for compiled-out support Andrew Cooper
2020-02-26  9:32 ` Jan Beulich
2020-02-26  9:58   ` Andrew Cooper
2020-02-26 10:43     ` Jan Beulich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.