Re: [Xen-devel] [PATCH 2/2] xen: Drop XEN_DOMCTL_{get, set}_machine_address_size

["Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>]

[-- Attachment #1.1: Type: text/plain, Size: 11918 bytes --]

On Tue, Aug 13, 2019 at 11:53:52AM +0100, Andrew Cooper wrote:
> This functionality is obsolete.  It was introduced by c/s 41296317a31 into
> Xend, but was never exposed in libxl.
> 
> Nothing limits this to PV guests, but it makes no sense for HVM guests.
> 
> Looking through the XenServer templates, this was used to work around bugs in
> the 32bit RHEL/CentOS 4.7 and 4.8 kernels (fixed in 4.9) and RHEL/CentOS/OEL
> 5.2 and 5.3 kernels (fixed in 5.4).  RHEL 4 as a major version went out of
> support in 2017, whereas the 5.2/5.3 kernels went out of support when 5.4 was
> released in 2009.
> 
> 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>
> CC: Christian Lindig <christian.lindig@citrix.com>
> CC: Rob Hoes <Rob.Hoes@citrix.com>
> 
> There may be some resulting simplifications which can be made to the heap
> allocator, but that involves untangling the other address clamping logic
> first.
> ---
>  tools/libxc/include/xenctrl.h       |  6 ------
>  tools/libxc/xc_domain.c             | 29 -----------------------------
>  tools/ocaml/libs/xc/xenctrl.ml      |  5 -----
>  tools/ocaml/libs/xc/xenctrl.mli     |  5 -----
>  tools/ocaml/libs/xc/xenctrl_stubs.c | 26 --------------------------
>  tools/python/xen/lowlevel/xc/xc.c   | 23 -----------------------
>  xen/arch/x86/domctl.c               | 12 ------------
>  xen/include/public/domctl.h         | 11 ++---------
>  xen/xsm/flask/hooks.c               |  2 --
>  xen/xsm/flask/policy/access_vectors |  4 ++--
>  10 files changed, 4 insertions(+), 119 deletions(-)
> 
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index a36896034a..c92386aab8 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -1781,12 +1781,6 @@ int xc_domain_unbind_pt_spi_irq(xc_interface *xch,
>                                  uint16_t vspi,
>                                  uint16_t spi);
>  
> -int xc_domain_set_machine_address_size(xc_interface *xch,
> -				       uint32_t domid,
> -				       unsigned int width);
> -int xc_domain_get_machine_address_size(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 64ca513aae..e544218d2e 100644
> --- a/tools/libxc/xc_domain.c
> +++ b/tools/libxc/xc_domain.c
> @@ -2161,35 +2161,6 @@ int xc_domain_subscribe_for_suspend(
>      return do_domctl(xch, &domctl);
>  }
>  
> -int xc_domain_set_machine_address_size(xc_interface *xch,
> -                                       uint32_t domid,
> -                                       unsigned int width)
> -{
> -    DECLARE_DOMCTL;
> -
> -    memset(&domctl, 0, sizeof(domctl));
> -    domctl.domain = domid;
> -    domctl.cmd    = XEN_DOMCTL_set_machine_address_size;
> -    domctl.u.address_size.size = width;
> -
> -    return do_domctl(xch, &domctl);
> -}
> -
> -
> -int xc_domain_get_machine_address_size(xc_interface *xch, uint32_t domid)
> -{
> -    DECLARE_DOMCTL;
> -    int rc;
> -
> -    memset(&domctl, 0, sizeof(domctl));
> -    domctl.domain = domid;
> -    domctl.cmd    = XEN_DOMCTL_get_machine_address_size;
> -
> -    rc = do_domctl(xch, &domctl);
> -
> -    return rc == 0 ? domctl.u.address_size.size : rc;
> -}
> -
>  int xc_domain_debug_control(xc_interface *xc, uint32_t domid, uint32_t sop, uint32_t vcpu)
>  {
>      DECLARE_DOMCTL;
> diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
> index a57130a3c3..35958b94d5 100644
> --- a/tools/ocaml/libs/xc/xenctrl.ml
> +++ b/tools/ocaml/libs/xc/xenctrl.ml
> @@ -241,11 +241,6 @@ external domain_set_memmap_limit: handle -> domid -> int64 -> unit
>  external domain_memory_increase_reservation: handle -> domid -> int64 -> unit
>         = "stub_xc_domain_memory_increase_reservation"
>  
> -external domain_set_machine_address_size: handle -> domid -> int -> unit
> -       = "stub_xc_domain_set_machine_address_size"
> -external domain_get_machine_address_size: handle -> domid -> int
> -       = "stub_xc_domain_get_machine_address_size"
> -
>  external domain_cpuid_set: handle -> domid -> (int64 * (int64 option))
>                          -> string option array
>                          -> string option array
> diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
> index 476bbecb90..6c4268d453 100644
> --- a/tools/ocaml/libs/xc/xenctrl.mli
> +++ b/tools/ocaml/libs/xc/xenctrl.mli
> @@ -202,11 +202,6 @@ val pages_to_mib : int64 -> int64
>  external watchdog : handle -> int -> int32 -> int
>    = "stub_xc_watchdog"
>  
> -external domain_set_machine_address_size: handle -> domid -> int -> unit
> -  = "stub_xc_domain_set_machine_address_size"
> -external domain_get_machine_address_size: handle -> domid -> int
> -       = "stub_xc_domain_get_machine_address_size"
> -
>  external domain_cpuid_set: handle -> domid -> (int64 * (int64 option))
>                          -> string option array
>                          -> string option array
> diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c
> index c4fdc58b2d..2e1b29ce33 100644
> --- a/tools/ocaml/libs/xc/xenctrl_stubs.c
> +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
> @@ -759,32 +759,6 @@ CAMLprim value stub_xc_domain_memory_increase_reservation(value xch,
>  	CAMLreturn(Val_unit);
>  }
>  
> -CAMLprim value stub_xc_domain_set_machine_address_size(value xch,
> -						       value domid,
> -						       value width)
> -{
> -	CAMLparam3(xch, domid, width);
> -	uint32_t c_domid = _D(domid);
> -	int c_width = Int_val(width);
> -
> -	int retval = xc_domain_set_machine_address_size(_H(xch), c_domid, c_width);
> -	if (retval)
> -		failwith_xc(_H(xch));
> -	CAMLreturn(Val_unit);
> -}
> -
> -CAMLprim value stub_xc_domain_get_machine_address_size(value xch,
> -                                                       value domid)
> -{
> -	CAMLparam2(xch, domid);
> -	int retval;
> -
> -	retval = xc_domain_get_machine_address_size(_H(xch), _D(domid));
> -	if (retval < 0)
> -		failwith_xc(_H(xch));
> -	CAMLreturn(Val_int(retval));
> -}
> -
>  CAMLprim value stub_xc_domain_cpuid_set(value xch, value domid,
>                                          value input,
>                                          value config)
> diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
> index 7e831a26a7..f0430ca85e 100644
> --- a/tools/python/xen/lowlevel/xc/xc.c
> +++ b/tools/python/xen/lowlevel/xc/xc.c
> @@ -770,22 +770,6 @@ static PyObject *pyxc_dom_set_cpuid(XcObject *self,
>  
>      return pyxc_create_cpuid_dict(regs_transform);
>  }
> -
> -static PyObject *pyxc_dom_set_machine_address_size(XcObject *self,
> -						   PyObject *args,
> -						   PyObject *kwds)
> -{
> -    uint32_t dom, width;
> -
> -    if (!PyArg_ParseTuple(args, "ii", &dom, &width))
> -	return NULL;
> -
> -    if (xc_domain_set_machine_address_size(self->xc_handle, dom, width) != 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,
> @@ -2413,13 +2397,6 @@ static PyMethodDef pyxc_methods[] = {
>        "Set the default cpuid policy for a domain.\n"
>        " dom [int]: Identifier of domain.\n\n"
>        "Returns: [int] 0 on success; exception on error.\n" },
> -
> -    { "domain_set_machine_address_size",
> -      (PyCFunction)pyxc_dom_set_machine_address_size,
> -      METH_VARARGS, "\n"
> -      "Set maximum machine address size for this domain.\n"
> -      " dom [int]: Identifier of domain.\n"
> -      " width [int]: Maximum machine address width.\n" },
>  #endif
>  
>      { "dom_set_memshr", 
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index 34a6f88b8a..1e98fc8009 100644
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -643,18 +643,6 @@ long arch_do_domctl(
>              ASSERT_UNREACHABLE();
>          break;
>  
> -    case XEN_DOMCTL_set_machine_address_size:
> -        if ( d->tot_pages > 0 )
> -            ret = -EBUSY;
> -        else
> -            d->arch.physaddr_bitsize = domctl->u.address_size.size;
> -        break;
> -
> -    case XEN_DOMCTL_get_machine_address_size:
> -        domctl->u.address_size.size = d->arch.physaddr_bitsize;
> -        copyback = true;
> -        break;
> -
>      case XEN_DOMCTL_sendtrigger:
>      {
>          struct vcpu *v;
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index 726ce675e8..72d5133cba 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -691,13 +691,6 @@ struct xen_domctl_subscribe {
>      uint32_t port; /* IN */
>  };
>  
> -/*
> - * Define the maximum machine address size which should be allocated
> - * to a guest.
> - */
> -/* XEN_DOMCTL_set_machine_address_size */
> -/* XEN_DOMCTL_get_machine_address_size */
> -
>  /* XEN_DOMCTL_debug_op */
>  #define XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_OFF         0
>  #define XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_ON          1
> @@ -1165,8 +1158,8 @@ struct xen_domctl {
>  #define XEN_DOMCTL_unbind_pt_irq                 48
>  #define XEN_DOMCTL_set_cpuid                     49
>  #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_set_machine_address_size   51 - Obsolete */
> +/* #define XEN_DOMCTL_get_machine_address_size   52 - Obsolete */
>  /* #define XEN_DOMCTL_suppress_spurious_page_faults 53 - Obsolete */
>  #define XEN_DOMCTL_debug_op                      54
>  #define XEN_DOMCTL_gethvmcontext_partial         55
> diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
> index fd5ec992cf..6800f2d9a0 100644
> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -684,11 +684,9 @@ static int flask_domctl(struct domain *d, int cmd)
>          return current_has_perm(d, SECCLASS_HVM, HVM__GETHVMC);
>  
>      case XEN_DOMCTL_set_address_size:
> -    case XEN_DOMCTL_set_machine_address_size:
>          return current_has_perm(d, SECCLASS_DOMAIN, DOMAIN__SETADDRSIZE);
>  
>      case XEN_DOMCTL_get_address_size:
> -    case XEN_DOMCTL_get_machine_address_size:
>          return current_has_perm(d, SECCLASS_DOMAIN, DOMAIN__GETADDRSIZE);
>  
>      case XEN_DOMCTL_mem_sharing_op:
> diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors
> index c9ebd0f37e..76f3d60ddd 100644
> --- a/xen/xsm/flask/policy/access_vectors
> +++ b/xen/xsm/flask/policy/access_vectors
> @@ -166,9 +166,9 @@ class domain
>      set_target
>  # SCHEDOP_remote_shutdown
>      shutdown
> -# XEN_DOMCTL_set{,_machine}_address_size
> +# XEN_DOMCTL_set_address_size
>      setaddrsize
> -# XEN_DOMCTL_get{,_machine}_address_size
> +# XEN_DOMCTL_get_address_size
>      getaddrsize
>  # XEN_DOMCTL_sendtrigger
>      trigger

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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