[PATCH xen-4.6] xen: Remove CONFIG_X86_SUPERVISOR_MODE_KERNEL as x86_32 builds are unsupported

[PATCH xen-4.6] xen: Remove CONFIG_X86_SUPERVISOR_MODE_KERNEL as x86_32 builds are unsupported

[Andrew Cooper]

supervisor_mode_kernel was an x86_32-only feature which permitted a PV dom0 to
run in ring 0, but at the expense of not being able to start any domUs.

As the x86_32 Xen build has been removed from tree, removing the remaining
supervisor_mode_kernel code.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Stefano Stabellini <stefano.stabellini@citrix.com>
CC: Tim Deegan <tim@xen.org>

---

One complication is that PVH has reused XENFEAT_supervisor_mode_kernel with a
modified meaning, and the Linux PVH code actively uses the flag as to indicate
running as a PVH guest.

This causes an discontinuity between PVH and HVM guests, both of which run
their kernels with the PVH-taken meaning of XENFEAT_supervisor_mode_kernel.

It also means that a dom0 kernel is unable to express "PVH-only" by requiring
this feature, as a 64bit Xen will validly reject an attempt to require a
32bit-only Xen feature.

Someone with more tuits than I currently have will need to see about
untangling this as part of unifying the PVH and HVM guest types, preferably
before the ABI is declared stable.
---
 xen/arch/x86/Rules.mk        |   10 ----------
 xen/arch/x86/domain.c        |    3 ---
 xen/arch/x86/domain_build.c  |   24 ++++++------------------
 xen/common/domctl.c          |    3 +--
 xen/common/kernel.c          |    2 --
 xen/include/asm-arm/config.h |    2 --
 xen/include/asm-x86/config.h |    6 ------
 xen/include/asm-x86/desc.h   |   10 ----------
 8 files changed, 7 insertions(+), 53 deletions(-)

diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
index 6775cb5..ef8c222 100644
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/Rules.mk
@@ -15,12 +15,6 @@ HAS_GDBSX := y
 HAS_PDX := y
 xenoprof := y
 
-#
-# If you change any of these configuration options then you must
-# 'make clean' before rebuilding.
-#
-supervisor_mode_kernel ?= n
-
 CFLAGS += -I$(BASEDIR)/include 
 CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-generic
 CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-default
@@ -34,10 +28,6 @@ $(call as-insn-check,CFLAGS,CC,"vmcall",-DHAVE_GAS_VMX)
 $(call as-insn-check,CFLAGS,CC,"invept (%rax)$$(comma)%rax",-DHAVE_GAS_EPT)
 $(call as-insn-check,CFLAGS,CC,"rdfsbase %rax",-DHAVE_GAS_FSGSBASE)
 
-ifeq ($(supervisor_mode_kernel),y)
-CFLAGS += -DCONFIG_X86_SUPERVISOR_MODE_KERNEL=1
-endif
-
 x86 := y
 x86_32 := n
 x86_64 := y
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 11c7d9f..b699510 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1726,9 +1726,6 @@ unsigned long hypercall_create_continuation(
         }
         else
         {
-            if ( supervisor_mode_kernel )
-                regs->eip &= ~31; /* re-execute entire hypercall entry stub */
-
             for ( i = 0; *p != '\0'; i++ )
             {
                 arg = next_arg(p, args);
diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
index 7993b17..7a912e9 100644
--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -1410,24 +1410,12 @@ int __init construct_dom0(
             paging_update_paging_modes(v);
     }
 
-    if ( supervisor_mode_kernel )
-    {
-        v->arch.pv_vcpu.kernel_ss &= ~3;
-        v->arch.user_regs.ss &= ~3;
-        v->arch.user_regs.es &= ~3;
-        v->arch.user_regs.ds &= ~3;
-        v->arch.user_regs.fs &= ~3;
-        v->arch.user_regs.gs &= ~3;
-        printk("Dom0 runs in ring 0 (supervisor mode)\n");
-        if ( !test_bit(XENFEAT_supervisor_mode_kernel,
-                       parms.f_supported) )
-            panic("Dom0 does not support supervisor-mode execution");
-    }
-    else
-    {
-        if ( test_bit(XENFEAT_supervisor_mode_kernel, parms.f_required) )
-            panic("Dom0 requires supervisor-mode execution");
-    }
+    /*
+     * PVH Fixme: XENFEAT_supervisor_mode_kernel has been reused in PVH with a
+     * different meaning.
+     */
+    if ( test_bit(XENFEAT_supervisor_mode_kernel, parms.f_required) )
+        panic("Dom0 requires supervisor-mode execution");
 
     rc = 0;
 
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index f15dcfe..edadf74 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -534,8 +534,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
         unsigned int domcr_flags;
 
         ret = -EINVAL;
-        if ( supervisor_mode_kernel ||
-             (op->u.createdomain.flags &
+        if ( (op->u.createdomain.flags &
              ~(XEN_DOMCTL_CDF_hvm_guest
                | XEN_DOMCTL_CDF_pvh_guest
                | XEN_DOMCTL_CDF_hap
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index d23c422..0d9e519 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -308,8 +308,6 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
                 fi.submap |= 
                     (1U << XENFEAT_writable_page_tables) |
                     (1U << XENFEAT_auto_translated_physmap);
-            if ( supervisor_mode_kernel )
-                fi.submap |= 1U << XENFEAT_supervisor_mode_kernel;
             if ( is_hardware_domain(current->domain) )
                 fi.submap |= 1U << XENFEAT_dom0;
 #ifdef CONFIG_X86
diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index 264e2c1..9e165db 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -196,8 +196,6 @@ extern unsigned long xenheap_phys_end;
 extern unsigned long frametable_virt_end;
 #endif
 
-#define supervisor_mode_kernel (0)
-
 #define watchdog_disable() ((void)0)
 #define watchdog_enable()  ((void)0)
 
diff --git a/xen/include/asm-x86/config.h b/xen/include/asm-x86/config.h
index 3802721..ad52d5b 100644
--- a/xen/include/asm-x86/config.h
+++ b/xen/include/asm-x86/config.h
@@ -70,12 +70,6 @@
 /* Maximum we can support with current vLAPIC ID mapping. */
 #define MAX_HVM_VCPUS 128
 
-#ifdef CONFIG_X86_SUPERVISOR_MODE_KERNEL
-# define supervisor_mode_kernel (1)
-#else
-# define supervisor_mode_kernel (0)
-#endif
-
 /* Linkage for x86 */
 #define __ALIGN .align 16,0x90
 #define __ALIGN_STR ".align 16,0x90"
diff --git a/xen/include/asm-x86/desc.h b/xen/include/asm-x86/desc.h
index 225913a..20c47d2 100644
--- a/xen/include/asm-x86/desc.h
+++ b/xen/include/asm-x86/desc.h
@@ -47,17 +47,7 @@
     (sel) = (((sel) & 3) >= _rpl) ? (sel) : (((sel) & ~3) | _rpl); \
 })
 
-/* Stack selectors don't need fixing up if the kernel runs in ring 0. */
-#ifdef CONFIG_X86_SUPERVISOR_MODE_KERNEL
-#define fixup_guest_stack_selector(d, ss) ((void)0)
-#else
 #define fixup_guest_stack_selector(d, ss) __fixup_guest_selector(d, ss)
-#endif
-
-/*
- * Code selectors are always fixed up. It allows the Xen exit stub to detect
- * return to guest context, even when the guest kernel runs in ring 0.
- */
 #define fixup_guest_code_selector(d, cs)  __fixup_guest_selector(d, cs)
 
 /*
-- 
1.7.10.4

Re: [PATCH xen-4.6] xen: Remove CONFIG_X86_SUPERVISOR_MODE_KERNEL as x86_32 builds are unsupported

[Tim Deegan]

At 19:12 +0000 on 02 Jan (1420222343), Andrew Cooper wrote:
> supervisor_mode_kernel was an x86_32-only feature which permitted a PV dom0 to
> run in ring 0, but at the expense of not being able to start any domUs.
> 
> As the x86_32 Xen build has been removed from tree, removing the remaining
> supervisor_mode_kernel code.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Tim Deegan <tim@xen.org>

Re: [PATCH xen-4.6] xen: Remove CONFIG_X86_SUPERVISOR_MODE_KERNEL as x86_32 builds are unsupported

[Ian Campbell]

On Fri, 2015-01-02 at 19:12 +0000, Andrew Cooper wrote:
> supervisor_mode_kernel was an x86_32-only feature which permitted a PV dom0 to
> run in ring 0, but at the expense of not being able to start any domUs.
> 
> As the x86_32 Xen build has been removed from tree, removing the remaining
> supervisor_mode_kernel code.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Keir Fraser <keir@xen.org>
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Stefano Stabellini <stefano.stabellini@citrix.com>
> CC: Tim Deegan <tim@xen.org>
> 
> ---
> 
> One complication is that PVH has reused XENFEAT_supervisor_mode_kernel with a
> modified meaning, and the Linux PVH code actively uses the flag as to indicate
> running as a PVH guest.

What is the modification? Doesn't a PVH kernel just use it to indicate
that it should (or wants) run in ring0 instead of ring1/ring3? That was
the original intention IIRC.

Regardless, supervisor_mode_kernel was never anything more than a
prototype, I'm pretty certain there can be no kernels out there relying
on it, so rather than breaking pvh dom0 as you seem to do here I think
it would be better to retcon the meaning of the flag as needed.

> This causes an discontinuity between PVH and HVM guests, both of which run
> their kernels with the PVH-taken meaning of XENFEAT_supervisor_mode_kernel.
> 
> It also means that a dom0 kernel is unable to express "PVH-only" by requiring
> this feature, as a 64bit Xen will validly reject an attempt to require a
> 32bit-only Xen feature.

I wouldn't describe XENFEAT_supervisor_mode_kernel as a "32-bit only Xen
feature". It was only ever implemented for 32-bit, but conceptually it
isn't specific to 32-bit but to any setup where PV requires you to run
the kernel at a lower then normal privilege level.

Ian.

Re: [PATCH xen-4.6] xen: Remove CONFIG_X86_SUPERVISOR_MODE_KERNEL as x86_32 builds are unsupported

[Andrew Cooper]

On 05/01/15 15:16, Ian Campbell wrote:
> On Fri, 2015-01-02 at 19:12 +0000, Andrew Cooper wrote:
>> supervisor_mode_kernel was an x86_32-only feature which permitted a PV dom0 to
>> run in ring 0, but at the expense of not being able to start any domUs.
>>
>> As the x86_32 Xen build has been removed from tree, removing the remaining
>> supervisor_mode_kernel code.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> CC: Keir Fraser <keir@xen.org>
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Ian Campbell <ian.campbell@citrix.com>
>> CC: Stefano Stabellini <stefano.stabellini@citrix.com>
>> CC: Tim Deegan <tim@xen.org>
>>
>> ---
>>
>> One complication is that PVH has reused XENFEAT_supervisor_mode_kernel with a
>> modified meaning, and the Linux PVH code actively uses the flag as to indicate
>> running as a PVH guest.
> What is the modification? Doesn't a PVH kernel just use it to indicate
> that it should (or wants) run in ring0 instead of ring1/ring3? That was
> the original intention IIRC.
>
> Regardless, supervisor_mode_kernel was never anything more than a
> prototype, I'm pretty certain there can be no kernels out there relying
> on it, so rather than breaking pvh dom0 as you seem to do here I think
> it would be better to retcon the meaning of the flag as needed.
>
>> This causes an discontinuity between PVH and HVM guests, both of which run
>> their kernels with the PVH-taken meaning of XENFEAT_supervisor_mode_kernel.
>>
>> It also means that a dom0 kernel is unable to express "PVH-only" by requiring
>> this feature, as a 64bit Xen will validly reject an attempt to require a
>> 32bit-only Xen feature.
> I wouldn't describe XENFEAT_supervisor_mode_kernel as a "32-bit only Xen
> feature". It was only ever implemented for 32-bit, but conceptually it
> isn't specific to 32-bit but to any setup where PV requires you to run
> the kernel at a lower then normal privilege level.

Answering out-of-order

This patch has no functional change WRT PVH.  The hunk commented on is
simply changed via indentation due to the removal of the conditional it
is in.  It was never been possible for a PVH kernel boot with
"!XENFEAT_supervisor_mode_kernel" in its feature string.

If retcon'ing this feature flag is acceptable then the problem goes
away, as can the commented hunk.  However, given the meaning of the
flag, it really should be advertised to plain HVM guests as well,
because their kernel does run in ring0.  At that point, it becomes more
of a general "running in an hvm container" flag.

What usecase was supervisor_mode_kernel developed for?  It seems
counter-intuitive, but I can't find anything in the history explaining
its use.

~Andrew

Re: [PATCH xen-4.6] xen: Remove CONFIG_X86_SUPERVISOR_MODE_KERNEL as x86_32 builds are unsupported

[Ian Campbell]

On Mon, 2015-01-05 at 15:35 +0000, Andrew Cooper wrote:
> Answering out-of-order
> 
> This patch has no functional change WRT PVH.  The hunk commented on is
> simply changed via indentation due to the removal of the conditional it
> is in.  It was never been possible for a PVH kernel boot with
> "!XENFEAT_supervisor_mode_kernel" in its feature string.

Oh, good!

> If retcon'ing this feature flag is acceptable then the problem goes
> away, as can the commented hunk.  However, given the meaning of the
> flag, it really should be advertised to plain HVM guests as well,
> because their kernel does run in ring0.  At that point, it becomes more
> of a general "running in an hvm container" flag.

Conversely one could argue that it is a PV* only feature which makes no
sense to an HVM guest (which I think is approximately what it means now)

> What usecase was supervisor_mode_kernel developed for?  It seems
> counter-intuitive, but I can't find anything in the history explaining
> its use.

It was a prototype from the pre-pvops days to see if it would be
feasible to have a single kernel binary which ran either on Xen or on a
stub hypervisor which ran it "as native" with little or no loss of
performance^TM (e.g. for distro's convenience to avoid the multiple
kernel issue).

It never went beyond a prototype with Xen proper instead of the proposed
stub hypervisor and then pvops came along and was a much more sensible
idea...

Ian.

Re: [PATCH xen-4.6] xen: Remove CONFIG_X86_SUPERVISOR_MODE_KERNEL as x86_32 builds are unsupported

[Andrew Cooper]

On 05/01/15 15:41, Ian Campbell wrote:
> On Mon, 2015-01-05 at 15:35 +0000, Andrew Cooper wrote:
>> Answering out-of-order
>>
>> This patch has no functional change WRT PVH.  The hunk commented on is
>> simply changed via indentation due to the removal of the conditional it
>> is in.  It was never been possible for a PVH kernel boot with
>> "!XENFEAT_supervisor_mode_kernel" in its feature string.
> Oh, good!
>
>> If retcon'ing this feature flag is acceptable then the problem goes
>> away, as can the commented hunk.  However, given the meaning of the
>> flag, it really should be advertised to plain HVM guests as well,
>> because their kernel does run in ring0.  At that point, it becomes more
>> of a general "running in an hvm container" flag.
> Conversely one could argue that it is a PV* only feature which makes no
> sense to an HVM guest (which I think is approximately what it means now)

This is going to cause problems with the effort to unify PVH and HVM
into a single guest type, which is why I raised the point.  The Linux
PVH code is already using it as a PVH vs non-PVH flag.

Either way - these are not problems which need fixing right now, but do
need to be considered as the unification work progresses.

>
>> What usecase was supervisor_mode_kernel developed for?  It seems
>> counter-intuitive, but I can't find anything in the history explaining
>> its use.
> It was a prototype from the pre-pvops days to see if it would be
> feasible to have a single kernel binary which ran either on Xen or on a
> stub hypervisor which ran it "as native" with little or no loss of
> performance^TM (e.g. for distro's convenience to avoid the multiple
> kernel issue).
>
> It never went beyond a prototype with Xen proper instead of the proposed
> stub hypervisor and then pvops came along and was a much more sensible
> idea...

Considering the implications of running dom0 in ring0, pvops seems like
a much more sensible idea.

~Andrew

Re: [PATCH xen-4.6] xen: Remove CONFIG_X86_SUPERVISOR_MODE_KERNEL as x86_32 builds are unsupported

[Ian Campbell]

On Mon, 2015-01-05 at 16:07 +0000, Andrew Cooper wrote:
> >> What usecase was supervisor_mode_kernel developed for?  It seems
> >> counter-intuitive, but I can't find anything in the history explaining
> >> its use.
> > It was a prototype from the pre-pvops days to see if it would be
> > feasible to have a single kernel binary which ran either on Xen or on a
> > stub hypervisor which ran it "as native" with little or no loss of
> > performance^TM (e.g. for distro's convenience to avoid the multiple
> > kernel issue).
> >
> > It never went beyond a prototype with Xen proper instead of the proposed
> > stub hypervisor and then pvops came along and was a much more sensible
> > idea...
> 
> Considering the implications of running dom0 in ring0, pvops seems like
> a much more sensible idea.

It wouldn't have been a dom0, it would have just been a native system
which happened to use some Xen interfaces, the intention was never to be
able to run guests or anything, just to allow distros to only support
one binary.

Ian.

Re: [PATCH xen-4.6] xen: Remove CONFIG_X86_SUPERVISOR_MODE_KERNEL as x86_32 builds are unsupported

[Mukesh Rathor]

On Mon, 5 Jan 2015 15:35:27 +0000
Andrew Cooper <andrew.cooper3@citrix.com> wrote:

> On 05/01/15 15:16, Ian Campbell wrote:
> > On Fri, 2015-01-02 at 19:12 +0000, Andrew Cooper wrote:
> >> supervisor_mode_kernel was an x86_32-only feature which permitted
> >> a PV dom0 to run in ring 0, but at the expense of not being able
> >> to start any domUs.
> >>
> >> As the x86_32 Xen build has been removed from tree, removing the
> >> remaining supervisor_mode_kernel code.
> >>
> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >> CC: Keir Fraser <keir@xen.org>
> >> CC: Jan Beulich <JBeulich@suse.com>
> >> CC: Ian Campbell <ian.campbell@citrix.com>
> >> CC: Stefano Stabellini <stefano.stabellini@citrix.com>
> >> CC: Tim Deegan <tim@xen.org>
> >>
> >> ---
> >>
> >> One complication is that PVH has reused
> >> XENFEAT_supervisor_mode_kernel with a modified meaning, and the
> >> Linux PVH code actively uses the flag as to indicate running as a
> >> PVH guest.
> > What is the modification? Doesn't a PVH kernel just use it to
> > indicate that it should (or wants) run in ring0 instead of
> > ring1/ring3? That was the original intention IIRC.

That flag has confused me too, and it was added later to pvh. 
Since, PVH guest is able to run in ring 0 ir-respective of the flag,
imho, XENFEAT_supervisor_mode_kernel can be just removed. The important
ones are really:

XENFEAT_auto_translated_physmap
XENFEAT_hvm_callback_vector

thanks
Mukesh