All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Wei Liu" <wl@xen.org>, "Roger Pau Monné" <roger.pau@citrix.com>
Subject: [PATCH v2 1/2] x86: correct is_pv_domain() when !CONFIG_PV
Date: Thu, 15 Apr 2021 11:34:55 +0200	[thread overview]
Message-ID: <9667345e-835e-7c55-8d6d-2774008b0017@suse.com> (raw)
In-Reply-To: <3bc868c1-1003-2f80-1617-d64a1e7441bc@suse.com>

On x86, idle and other system domains are implicitly PV. While I
couldn't spot any cases where this is actively a problem, some cases
required quite close inspection to be certain there couldn't e.g. be
some ASSERT_UNREACHABLE() that would trigger in this case. Let's be on
the safe side and make sure these always have is_pv_domain() returning
true.

For the build to still work, this requires a few adjustments elsewhere.
In particular is_pv_64bit_domain() now gains a CONFIG_PV dependency,
which means that is_pv_32bit_domain() || is_pv_64bit_domain() is no
longer guaranteed to be the same as is_pv_domain().

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Add comment.

--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -571,7 +571,7 @@ int __init construct_dom0(struct domain
 
     if ( is_hvm_domain(d) )
         rc = dom0_construct_pvh(d, image, image_headroom, initrd, cmdline);
-    else if ( is_pv_domain(d) )
+    else if ( is_pv_64bit_domain(d) || is_pv_32bit_domain(d) )
         rc = dom0_construct_pv(d, image, image_headroom, initrd, cmdline);
     else
         panic("Cannot construct Dom0. No guest interface available\n");
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1564,6 +1564,7 @@ arch_do_vcpu_op(
  */
 static void load_segments(struct vcpu *n)
 {
+#ifdef CONFIG_PV
     struct cpu_user_regs *uregs = &n->arch.user_regs;
     unsigned long gsb = 0, gss = 0;
     bool compat = is_pv_32bit_vcpu(n);
@@ -1729,6 +1730,7 @@ static void load_segments(struct vcpu *n
         regs->cs            = FLAT_KERNEL_CS;
         regs->rip           = pv->failsafe_callback_eip;
     }
+#endif
 }
 
 /*
@@ -1743,6 +1745,7 @@ static void load_segments(struct vcpu *n
  */
 static void save_segments(struct vcpu *v)
 {
+#ifdef CONFIG_PV
     struct cpu_user_regs *regs = &v->arch.user_regs;
 
     read_sregs(regs);
@@ -1768,6 +1771,7 @@ static void save_segments(struct vcpu *v
         else
             v->arch.pv.gs_base_user = gs_base;
     }
+#endif
 }
 
 void paravirt_ctxt_switch_from(struct vcpu *v)
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -457,13 +457,13 @@ long arch_do_domctl(
     case XEN_DOMCTL_set_address_size:
         if ( is_hvm_domain(d) )
             ret = -EOPNOTSUPP;
+        else if ( is_pv_64bit_domain(d) && domctl->u.address_size.size == 32 )
+            ret = switch_compat(d);
         else if ( is_pv_domain(d) )
         {
             if ( ((domctl->u.address_size.size == 64) && !d->arch.pv.is_32bit) ||
                  ((domctl->u.address_size.size == 32) &&  d->arch.pv.is_32bit) )
                 ret = 0;
-            else if ( domctl->u.address_size.size == 32 )
-                ret = switch_compat(d);
             else
                 ret = -EINVAL;
         }
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -1036,9 +1036,14 @@ static always_inline bool is_control_dom
 
 #define VM_ASSIST(d, t) (test_bit(VMASST_TYPE_ ## t, &(d)->vm_assist))
 
+/*
+ * Note that is_pv_domain() can return true (for system domains) even when
+ * both is_pv_64bit_domain() and is_pv_32bit_domain() return false. IOW
+ * system domains can be considered PV without specific bitness.
+ */
 static always_inline bool is_pv_domain(const struct domain *d)
 {
-    return IS_ENABLED(CONFIG_PV) &&
+    return IS_ENABLED(CONFIG_X86) &&
         evaluate_nospec(!(d->options & XEN_DOMCTL_CDF_hvm));
 }
 
@@ -1064,7 +1069,7 @@ static always_inline bool is_pv_32bit_vc
 
 static always_inline bool is_pv_64bit_domain(const struct domain *d)
 {
-    if ( !is_pv_domain(d) )
+    if ( !IS_ENABLED(CONFIG_PV) || !is_pv_domain(d) )
         return false;
 
 #ifdef CONFIG_PV32



  reply	other threads:[~2021-04-15  9:35 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-15  9:33 [PATCH v2 0/2] x86: is_pv*domain() adjustments Jan Beulich
2021-04-15  9:34 ` Jan Beulich [this message]
2021-04-15 10:53   ` [PATCH v2 1/2] x86: correct is_pv_domain() when !CONFIG_PV Roger Pau Monné
2021-04-15 11:20     ` Jan Beulich
2021-04-15  9:36 ` [PATCH v2 2/2] x86: use is_pv_64bit_domain() to avoid double evaluate_nospec() Jan Beulich
2021-04-15 12:25   ` Roger Pau Monné

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9667345e-835e-7c55-8d6d-2774008b0017@suse.com \
    --to=jbeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=roger.pau@citrix.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.