[1/2] xen: Drop XEN_DOMCTL_suppress_spurious_page_faults
diff mbox series

Message ID 20190813105352.32412-2-andrew.cooper3@citrix.com
State New
Headers show
Series
  • xen: Drop obsolete DOMCTLs
Related show

Commit Message

Andrew Cooper Aug. 13, 2019, 10:53 a.m. UTC
This functionality is obsolete.  It was introduced by c/s 39407bed9c0 into
Xend, but never exposed in libxl.

While not explicitly limited to PV guests, this is PV-only by virtue of its
position in the pagefault handler.

Looking though the XenServer templates, this was used to work around bugs in
the 32bit RHEL/CentOS 4.{5..7} kernels (fixed in 4.8).  RHEL 4 as a major
version when out if support in 2017.

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>
CC: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
CC: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 tools/libxc/include/xenctrl.h       |  3 ---
 tools/libxc/xc_domain.c             | 12 ------------
 tools/python/xen/lowlevel/xc/xc.c   | 22 ----------------------
 xen/arch/x86/domctl.c               |  4 ----
 xen/arch/x86/traps.c                | 14 --------------
 xen/include/asm-x86/domain.h        |  3 ---
 xen/include/public/domctl.h         |  7 +------
 xen/xsm/flask/hooks.c               |  1 -
 xen/xsm/flask/policy/access_vectors |  3 +--
 9 files changed, 2 insertions(+), 67 deletions(-)

Comments

Marek Marczykowski-Górecki Aug. 13, 2019, 6:58 p.m. UTC | #1
On Tue, Aug 13, 2019 at 11:53:51AM +0100, Andrew Cooper wrote:
> This functionality is obsolete.  It was introduced by c/s 39407bed9c0 into
> Xend, but never exposed in libxl.
> 
> While not explicitly limited to PV guests, this is PV-only by virtue of its
> position in the pagefault handler.
> 
> Looking though the XenServer templates, this was used to work around bugs in
> the 32bit RHEL/CentOS 4.{5..7} kernels (fixed in 4.8).  RHEL 4 as a major
> version when out if support in 2017.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Python part:
Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Also, I confirm it isn't used in Qubes OS.

> ---
> 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>
> CC: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> CC: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> ---
>  tools/libxc/include/xenctrl.h       |  3 ---
>  tools/libxc/xc_domain.c             | 12 ------------
>  tools/python/xen/lowlevel/xc/xc.c   | 22 ----------------------
>  xen/arch/x86/domctl.c               |  4 ----
>  xen/arch/x86/traps.c                | 14 --------------
>  xen/include/asm-x86/domain.h        |  3 ---
>  xen/include/public/domctl.h         |  7 +------
>  xen/xsm/flask/hooks.c               |  1 -
>  xen/xsm/flask/policy/access_vectors |  3 +--
>  9 files changed, 2 insertions(+), 67 deletions(-)
> 
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index 0ff6ed9e70..a36896034a 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -1787,9 +1787,6 @@ int xc_domain_set_machine_address_size(xc_interface *xch,
>  int xc_domain_get_machine_address_size(xc_interface *xch,
>  				       uint32_t domid);
>  
> -int xc_domain_suppress_spurious_page_faults(xc_interface *xch,
> -					  uint32_t domid);
> -
>  /* Set the target domain */
>  int xc_domain_set_target(xc_interface *xch,
>                           uint32_t domid,
> diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
> index 05d771f2ce..64ca513aae 100644
> --- a/tools/libxc/xc_domain.c
> +++ b/tools/libxc/xc_domain.c
> @@ -2190,18 +2190,6 @@ int xc_domain_get_machine_address_size(xc_interface *xch, uint32_t domid)
>      return rc == 0 ? domctl.u.address_size.size : rc;
>  }
>  
> -int xc_domain_suppress_spurious_page_faults(xc_interface *xc, uint32_t domid)
> -{
> -    DECLARE_DOMCTL;
> -
> -    memset(&domctl, 0, sizeof(domctl));
> -    domctl.domain = domid;
> -    domctl.cmd    = XEN_DOMCTL_suppress_spurious_page_faults;
> -
> -    return do_domctl(xc, &domctl);
> -
> -}
> -
>  int xc_domain_debug_control(xc_interface *xc, uint32_t domid, uint32_t sop, uint32_t vcpu)
>  {
>      DECLARE_DOMCTL;
> diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
> index 188bfa34da..7e831a26a7 100644
> --- a/tools/python/xen/lowlevel/xc/xc.c
> +++ b/tools/python/xen/lowlevel/xc/xc.c
> @@ -786,22 +786,6 @@ static PyObject *pyxc_dom_set_machine_address_size(XcObject *self,
>      Py_INCREF(zero);
>      return zero;
>  }
> -
> -static PyObject *pyxc_dom_suppress_spurious_page_faults(XcObject *self,
> -						      PyObject *args,
> -						      PyObject *kwds)
> -{
> -    uint32_t dom;
> -
> -    if (!PyArg_ParseTuple(args, "i", &dom))
> -	return NULL;
> -
> -    if (xc_domain_suppress_spurious_page_faults(self->xc_handle, dom) != 0)
> -	return pyxc_error_to_exception(self->xc_handle);
> -
> -    Py_INCREF(zero);
> -    return zero;
> -}
>  #endif /* __i386__ || __x86_64__ */
>  
>  static PyObject *pyxc_gnttab_hvm_seed(XcObject *self,
> @@ -2436,12 +2420,6 @@ static PyMethodDef pyxc_methods[] = {
>        "Set maximum machine address size for this domain.\n"
>        " dom [int]: Identifier of domain.\n"
>        " width [int]: Maximum machine address width.\n" },
> -
> -    { "domain_suppress_spurious_page_faults",
> -      (PyCFunction)pyxc_dom_suppress_spurious_page_faults,
> -      METH_VARARGS, "\n"
> -      "Do not propagate spurious page faults to this guest.\n"
> -      " dom [int]: Identifier of domain.\n" },
>  #endif
>  
>      { "dom_set_memshr", 
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index 2d45e5b8a8..34a6f88b8a 100644
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -988,10 +988,6 @@ long arch_do_domctl(
>          }
>          break;
>  
> -    case XEN_DOMCTL_suppress_spurious_page_faults:
> -        d->arch.suppress_spurious_page_faults = 1;
> -        break;
> -
>  #ifdef CONFIG_HVM
>      case XEN_DOMCTL_debug_op:
>      {
> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
> index 23069e25ec..350903add5 100644
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -1452,20 +1452,6 @@ void do_page_fault(struct cpu_user_regs *regs)
>                error_code, _p(addr));
>      }
>  
> -    if ( unlikely(current->domain->arch.suppress_spurious_page_faults) )
> -    {
> -        pf_type = spurious_page_fault(addr, regs);
> -        if ( (pf_type == smep_fault) || (pf_type == smap_fault))
> -        {
> -            printk(XENLOG_G_ERR "%pv fatal SM%cP violation\n",
> -                   current, (pf_type == smep_fault) ? 'E' : 'A');
> -
> -            domain_crash(current->domain);
> -        }
> -        if ( pf_type != real_fault )
> -            return;
> -    }
> -
>      if ( unlikely(regs->error_code & PFEC_reserved_bit) )
>          reserved_bit_page_fault(addr, regs);
>  
> diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
> index 933b85901f..8523c3f5e0 100644
> --- a/xen/include/asm-x86/domain.h
> +++ b/xen/include/asm-x86/domain.h
> @@ -339,9 +339,6 @@ struct arch_domain
>      /* Is shared-info page in 32-bit format? */
>      bool_t has_32bit_shinfo;
>  
> -    /* Domain cannot handle spurious page faults? */
> -    bool_t suppress_spurious_page_faults;
> -
>      /* Is PHYSDEVOP_eoi to automatically unmask the event channel? */
>      bool_t auto_unmask;
>  
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index 19486d5e32..726ce675e8 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -698,11 +698,6 @@ struct xen_domctl_subscribe {
>  /* XEN_DOMCTL_set_machine_address_size */
>  /* XEN_DOMCTL_get_machine_address_size */
>  
> -/*
> - * Do not inject spurious page faults into this domain.
> - */
> -/* XEN_DOMCTL_suppress_spurious_page_faults */
> -
>  /* XEN_DOMCTL_debug_op */
>  #define XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_OFF         0
>  #define XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_ON          1
> @@ -1172,7 +1167,7 @@ struct xen_domctl {
>  #define XEN_DOMCTL_get_device_group              50
>  #define XEN_DOMCTL_set_machine_address_size      51
>  #define XEN_DOMCTL_get_machine_address_size      52
> -#define XEN_DOMCTL_suppress_spurious_page_faults 53
> +/* #define XEN_DOMCTL_suppress_spurious_page_faults 53 - Obsolete */
>  #define XEN_DOMCTL_debug_op                      54
>  #define XEN_DOMCTL_gethvmcontext_partial         55
>  #define XEN_DOMCTL_vm_event_op                   56
> diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
> index 791c1f66af..fd5ec992cf 100644
> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -712,7 +712,6 @@ static int flask_domctl(struct domain *d, int cmd)
>  
>      case XEN_DOMCTL_subscribe:
>      case XEN_DOMCTL_disable_migrate:
> -    case XEN_DOMCTL_suppress_spurious_page_faults:
>          return current_has_perm(d, SECCLASS_DOMAIN, DOMAIN__SET_MISC_INFO);
>  
>      case XEN_DOMCTL_set_virq_handler:
> diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors
> index 194d743a71..c9ebd0f37e 100644
> --- a/xen/xsm/flask/policy/access_vectors
> +++ b/xen/xsm/flask/policy/access_vectors
> @@ -176,8 +176,7 @@ class domain
>      getpodtarget
>  # XENMEM_set_pod_target
>      setpodtarget
> -# XEN_DOMCTL_subscribe, XEN_DOMCTL_disable_migrate,
> -# XEN_DOMCTL_suppress_spurious_page_faults
> +# XEN_DOMCTL_subscribe, XEN_DOMCTL_disable_migrate
>      set_misc_info
>  # XEN_DOMCTL_set_virq_handler
>      set_virq_handler
Jan Beulich Aug. 27, 2019, 3:39 p.m. UTC | #2
On 13.08.2019 12:53, Andrew Cooper wrote:
> This functionality is obsolete.  It was introduced by c/s 39407bed9c0 into
> Xend, but never exposed in libxl.

This is good enough a reason I think (hope), while ...

> While not explicitly limited to PV guests, this is PV-only by virtue of its
> position in the pagefault handler.
> 
> Looking though the XenServer templates, this was used to work around bugs in
> the 32bit RHEL/CentOS 4.{5..7} kernels (fixed in 4.8).  RHEL 4 as a major
> version when out if support in 2017.

... I wouldn't accept this as justification, since people are
free to use out-of-support software for as long as they wish.

> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Hypervisor parts
Reviewed-by: Jan Beulich <jbeulich@suse.com>
with one further suggestion:

> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -1452,20 +1452,6 @@ void do_page_fault(struct cpu_user_regs *regs)
>                 error_code, _p(addr));
>       }
>   
> -    if ( unlikely(current->domain->arch.suppress_spurious_page_faults) )
> -    {
> -        pf_type = spurious_page_fault(addr, regs);
> -        if ( (pf_type == smep_fault) || (pf_type == smap_fault))
> -        {
> -            printk(XENLOG_G_ERR "%pv fatal SM%cP violation\n",
> -                   current, (pf_type == smep_fault) ? 'E' : 'A');
> -
> -            domain_crash(current->domain);
> -        }
> -        if ( pf_type != real_fault )
> -            return;
> -    }

You may want to consider reducing pf_type's scope at the same
time.

Jan
Andrew Cooper Aug. 28, 2019, 2:30 p.m. UTC | #3
On 27/08/2019 16:39, Jan Beulich wrote:
> On 13.08.2019 12:53, Andrew Cooper wrote:
>> This functionality is obsolete.  It was introduced by c/s 39407bed9c0
>> into
>> Xend, but never exposed in libxl.
>
> This is good enough a reason I think (hope), while ...
>
>> While not explicitly limited to PV guests, this is PV-only by virtue
>> of its
>> position in the pagefault handler.
>>
>> Looking though the XenServer templates, this was used to work around
>> bugs in
>> the 32bit RHEL/CentOS 4.{5..7} kernels (fixed in 4.8).  RHEL 4 as a
>> major
>> version when out if support in 2017.
>
> ... I wouldn't accept this as justification,

I'd be approaching this differently if support were in libxl, but...

> since people are
> free to use out-of-support software for as long as they wish.

... there are a lot of very good reasons not to keep on using
out-of-support software.  A recent example is the WannaCry outbreak
which spread in part due to the still-prolific use of Windows XP.

Furthermore, people running kernels which are that obsolete will also be
running similarly-obsolete versions of Xen, so this change isn't going
to affect them.

>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> Hypervisor parts
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Thanks,

> with one further suggestion:
>
>> --- a/xen/arch/x86/traps.c
>> +++ b/xen/arch/x86/traps.c
>> @@ -1452,20 +1452,6 @@ void do_page_fault(struct cpu_user_regs *regs)
>>                 error_code, _p(addr));
>>       }
>>   -    if (
>> unlikely(current->domain->arch.suppress_spurious_page_faults) )
>> -    {
>> -        pf_type = spurious_page_fault(addr, regs);
>> -        if ( (pf_type == smep_fault) || (pf_type == smap_fault))
>> -        {
>> -            printk(XENLOG_G_ERR "%pv fatal SM%cP violation\n",
>> -                   current, (pf_type == smep_fault) ? 'E' : 'A');
>> -
>> -            domain_crash(current->domain);
>> -        }
>> -        if ( pf_type != real_fault )
>> -            return;
>> -    }
>
> You may want to consider reducing pf_type's scope at the same
> time.

This isn't too much of a change, so I've included it.

~Andrew

Patch
diff mbox series

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 0ff6ed9e70..a36896034a 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1787,9 +1787,6 @@  int xc_domain_set_machine_address_size(xc_interface *xch,
 int xc_domain_get_machine_address_size(xc_interface *xch,
 				       uint32_t domid);
 
-int xc_domain_suppress_spurious_page_faults(xc_interface *xch,
-					  uint32_t domid);
-
 /* Set the target domain */
 int xc_domain_set_target(xc_interface *xch,
                          uint32_t domid,
diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index 05d771f2ce..64ca513aae 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -2190,18 +2190,6 @@  int xc_domain_get_machine_address_size(xc_interface *xch, uint32_t domid)
     return rc == 0 ? domctl.u.address_size.size : rc;
 }
 
-int xc_domain_suppress_spurious_page_faults(xc_interface *xc, uint32_t domid)
-{
-    DECLARE_DOMCTL;
-
-    memset(&domctl, 0, sizeof(domctl));
-    domctl.domain = domid;
-    domctl.cmd    = XEN_DOMCTL_suppress_spurious_page_faults;
-
-    return do_domctl(xc, &domctl);
-
-}
-
 int xc_domain_debug_control(xc_interface *xc, uint32_t domid, uint32_t sop, uint32_t vcpu)
 {
     DECLARE_DOMCTL;
diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
index 188bfa34da..7e831a26a7 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -786,22 +786,6 @@  static PyObject *pyxc_dom_set_machine_address_size(XcObject *self,
     Py_INCREF(zero);
     return zero;
 }
-
-static PyObject *pyxc_dom_suppress_spurious_page_faults(XcObject *self,
-						      PyObject *args,
-						      PyObject *kwds)
-{
-    uint32_t dom;
-
-    if (!PyArg_ParseTuple(args, "i", &dom))
-	return NULL;
-
-    if (xc_domain_suppress_spurious_page_faults(self->xc_handle, dom) != 0)
-	return pyxc_error_to_exception(self->xc_handle);
-
-    Py_INCREF(zero);
-    return zero;
-}
 #endif /* __i386__ || __x86_64__ */
 
 static PyObject *pyxc_gnttab_hvm_seed(XcObject *self,
@@ -2436,12 +2420,6 @@  static PyMethodDef pyxc_methods[] = {
       "Set maximum machine address size for this domain.\n"
       " dom [int]: Identifier of domain.\n"
       " width [int]: Maximum machine address width.\n" },
-
-    { "domain_suppress_spurious_page_faults",
-      (PyCFunction)pyxc_dom_suppress_spurious_page_faults,
-      METH_VARARGS, "\n"
-      "Do not propagate spurious page faults to this guest.\n"
-      " dom [int]: Identifier of domain.\n" },
 #endif
 
     { "dom_set_memshr", 
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 2d45e5b8a8..34a6f88b8a 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -988,10 +988,6 @@  long arch_do_domctl(
         }
         break;
 
-    case XEN_DOMCTL_suppress_spurious_page_faults:
-        d->arch.suppress_spurious_page_faults = 1;
-        break;
-
 #ifdef CONFIG_HVM
     case XEN_DOMCTL_debug_op:
     {
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 23069e25ec..350903add5 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1452,20 +1452,6 @@  void do_page_fault(struct cpu_user_regs *regs)
               error_code, _p(addr));
     }
 
-    if ( unlikely(current->domain->arch.suppress_spurious_page_faults) )
-    {
-        pf_type = spurious_page_fault(addr, regs);
-        if ( (pf_type == smep_fault) || (pf_type == smap_fault))
-        {
-            printk(XENLOG_G_ERR "%pv fatal SM%cP violation\n",
-                   current, (pf_type == smep_fault) ? 'E' : 'A');
-
-            domain_crash(current->domain);
-        }
-        if ( pf_type != real_fault )
-            return;
-    }
-
     if ( unlikely(regs->error_code & PFEC_reserved_bit) )
         reserved_bit_page_fault(addr, regs);
 
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 933b85901f..8523c3f5e0 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -339,9 +339,6 @@  struct arch_domain
     /* Is shared-info page in 32-bit format? */
     bool_t has_32bit_shinfo;
 
-    /* Domain cannot handle spurious page faults? */
-    bool_t suppress_spurious_page_faults;
-
     /* Is PHYSDEVOP_eoi to automatically unmask the event channel? */
     bool_t auto_unmask;
 
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 19486d5e32..726ce675e8 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -698,11 +698,6 @@  struct xen_domctl_subscribe {
 /* XEN_DOMCTL_set_machine_address_size */
 /* XEN_DOMCTL_get_machine_address_size */
 
-/*
- * Do not inject spurious page faults into this domain.
- */
-/* XEN_DOMCTL_suppress_spurious_page_faults */
-
 /* XEN_DOMCTL_debug_op */
 #define XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_OFF         0
 #define XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_ON          1
@@ -1172,7 +1167,7 @@  struct xen_domctl {
 #define XEN_DOMCTL_get_device_group              50
 #define XEN_DOMCTL_set_machine_address_size      51
 #define XEN_DOMCTL_get_machine_address_size      52
-#define XEN_DOMCTL_suppress_spurious_page_faults 53
+/* #define XEN_DOMCTL_suppress_spurious_page_faults 53 - Obsolete */
 #define XEN_DOMCTL_debug_op                      54
 #define XEN_DOMCTL_gethvmcontext_partial         55
 #define XEN_DOMCTL_vm_event_op                   56
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 791c1f66af..fd5ec992cf 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -712,7 +712,6 @@  static int flask_domctl(struct domain *d, int cmd)
 
     case XEN_DOMCTL_subscribe:
     case XEN_DOMCTL_disable_migrate:
-    case XEN_DOMCTL_suppress_spurious_page_faults:
         return current_has_perm(d, SECCLASS_DOMAIN, DOMAIN__SET_MISC_INFO);
 
     case XEN_DOMCTL_set_virq_handler:
diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors
index 194d743a71..c9ebd0f37e 100644
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -176,8 +176,7 @@  class domain
     getpodtarget
 # XENMEM_set_pod_target
     setpodtarget
-# XEN_DOMCTL_subscribe, XEN_DOMCTL_disable_migrate,
-# XEN_DOMCTL_suppress_spurious_page_faults
+# XEN_DOMCTL_subscribe, XEN_DOMCTL_disable_migrate
     set_misc_info
 # XEN_DOMCTL_set_virq_handler
     set_virq_handler