[PATCH 1/2] tools/ocaml: Drop coredump infrastructure

[PATCH 1/2] tools/ocaml: Drop coredump infrastructure

[Andrew Cooper]

It is unused, and uses an obsolete hypercall which has never ever functioned
for HVM guests.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Christian Lindig <christian.lindig@citrix.com>
---
 tools/ocaml/libs/xc/xenctrl.ml      | 86 -------------------------------------
 tools/ocaml/libs/xc/xenctrl.mli     | 16 -------
 tools/ocaml/libs/xc/xenctrl_stubs.c | 41 ------------------
 3 files changed, 143 deletions(-)

diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
index 9116aa2..a3ba488 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -126,14 +126,6 @@ exception Error of string
 
 type handle
 
-(* this is only use by coredumping *)
-external sizeof_core_header: unit -> int
-       = "stub_sizeof_core_header"
-external sizeof_vcpu_guest_context: unit -> int
-       = "stub_sizeof_vcpu_guest_context"
-external sizeof_xen_pfn: unit -> int = "stub_sizeof_xen_pfn"
-(* end of use *)
-
 external interface_open: unit -> handle = "stub_xc_interface_open"
 external interface_close: handle -> unit = "stub_xc_interface_close"
 
@@ -275,84 +267,6 @@ external get_cpu_featureset : handle -> featureset_index -> int64 array = "stub_
 external watchdog : handle -> int -> int32 -> int
   = "stub_xc_watchdog"
 
-(* core dump structure *)
-type core_magic = Magic_hvm | Magic_pv
-
-type core_header = {
-	xch_magic: core_magic;
-	xch_nr_vcpus: int;
-	xch_nr_pages: nativeint;
-	xch_index_offset: int64;
-	xch_ctxt_offset: int64;
-	xch_pages_offset: int64;
-}
-
-external marshall_core_header: core_header -> string = "stub_marshall_core_header"
-
-(* coredump *)
-let coredump xch domid fd =
-	let dump s =
-		let wd = Unix.write fd s 0 (String.length s) in
-		if wd <> String.length s then
-			failwith "error while writing";
-		in
-
-	let info = domain_getinfo xch domid in
-
-	let nrpages = info.total_memory_pages in
-	let ctxt = Array.make info.max_vcpu_id None in
-	let nr_vcpus = ref 0 in
-	for i = 0 to info.max_vcpu_id - 1
-	do
-		ctxt.(i) <- try
-			let v = vcpu_context_get xch domid i in
-			incr nr_vcpus;
-			Some v
-			with _ -> None
-	done;
-
-	(* FIXME page offset if not rounded to sup *)
-	let page_offset =
-		Int64.add
-			(Int64.of_int (sizeof_core_header () +
-			 (sizeof_vcpu_guest_context () * !nr_vcpus)))
-			(Int64.of_nativeint (
-				Nativeint.mul
-					(Nativeint.of_int (sizeof_xen_pfn ()))
-					nrpages)
-				)
-		in
-
-	let header = {
-		xch_magic = if info.hvm_guest then Magic_hvm else Magic_pv;
-		xch_nr_vcpus = !nr_vcpus;
-		xch_nr_pages = nrpages;
-		xch_ctxt_offset = Int64.of_int (sizeof_core_header ());
-		xch_index_offset = Int64.of_int (sizeof_core_header ()
-					+ sizeof_vcpu_guest_context ());
-		xch_pages_offset = page_offset;
-	} in
-
-	dump (marshall_core_header header);
-	for i = 0 to info.max_vcpu_id - 1
-	do
-		match ctxt.(i) with
-		| None -> ()
-		| Some ctxt_i -> dump ctxt_i
-	done;
-	let pfns = domain_get_pfn_list xch domid nrpages in
-	if Array.length pfns <> Nativeint.to_int nrpages then
-		failwith "could not get the page frame list";
-
-	let page_size = Xenmmap.getpagesize () in
-	for i = 0 to Nativeint.to_int nrpages - 1
-	do
-		let page = map_foreign_range xch domid page_size pfns.(i) in
-		let data = Xenmmap.read page 0 page_size in
-		Xenmmap.unmap page;
-		dump data
-	done
-
 (* ** Misc ** *)
 
 (**
diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
index 54c099c..ed02124 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -95,10 +95,6 @@ type domain_create_flag = CDF_HVM | CDF_HAP
 
 exception Error of string
 type handle
-external sizeof_core_header : unit -> int = "stub_sizeof_core_header"
-external sizeof_vcpu_guest_context : unit -> int
-  = "stub_sizeof_vcpu_guest_context"
-external sizeof_xen_pfn : unit -> int = "stub_sizeof_xen_pfn"
 external interface_open : unit -> handle = "stub_xc_interface_open"
 external interface_close : handle -> unit = "stub_xc_interface_close"
 val with_intf : (handle -> 'a) -> 'a
@@ -179,18 +175,6 @@ external version_capabilities : handle -> string
 type featureset_index = Featureset_raw | Featureset_host | Featureset_pv | Featureset_hvm
 external get_cpu_featureset : handle -> featureset_index -> int64 array = "stub_xc_get_cpu_featureset"
 
-type core_magic = Magic_hvm | Magic_pv
-type core_header = {
-  xch_magic : core_magic;
-  xch_nr_vcpus : int;
-  xch_nr_pages : nativeint;
-  xch_index_offset : int64;
-  xch_ctxt_offset : int64;
-  xch_pages_offset : int64;
-}
-external marshall_core_header : core_header -> string
-  = "stub_marshall_core_header"
-val coredump : handle -> domid -> Unix.file_descr -> unit
 external pages_to_kib : int64 -> int64 = "stub_pages_to_kib"
 val pages_to_mib : int64 -> int64
 external watchdog : handle -> int -> int32 -> int
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c
index dd6000c..d1801e1 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -72,47 +72,6 @@ static void Noreturn failwith_xc(xc_interface *xch)
 	caml_raise_with_string(*caml_named_value("xc.error"), error_str);
 }
 
-CAMLprim value stub_sizeof_core_header(value unit)
-{
-	CAMLparam1(unit);
-	CAMLreturn(Val_int(sizeof(struct xc_core_header)));
-}
-
-CAMLprim value stub_sizeof_vcpu_guest_context(value unit)
-{
-	CAMLparam1(unit);
-	CAMLreturn(Val_int(sizeof(struct vcpu_guest_context)));
-}
-
-CAMLprim value stub_sizeof_xen_pfn(value unit)
-{
-	CAMLparam1(unit);
-	CAMLreturn(Val_int(sizeof(xen_pfn_t)));
-}
-
-#define XC_CORE_MAGIC     0xF00FEBED
-#define XC_CORE_MAGIC_HVM 0xF00FEBEE
-
-CAMLprim value stub_marshall_core_header(value header)
-{
-	CAMLparam1(header);
-	CAMLlocal1(s);
-	struct xc_core_header c_header;
-
-	c_header.xch_magic = (Field(header, 0))
-		? XC_CORE_MAGIC
-		: XC_CORE_MAGIC_HVM;
-	c_header.xch_nr_vcpus = Int_val(Field(header, 1));
-	c_header.xch_nr_pages = Nativeint_val(Field(header, 2));
-	c_header.xch_ctxt_offset = Int64_val(Field(header, 3));
-	c_header.xch_index_offset = Int64_val(Field(header, 4));
-	c_header.xch_pages_offset = Int64_val(Field(header, 5));
-
-	s = caml_alloc_string(sizeof(c_header));
-	memcpy(String_val(s), (char *) &c_header, sizeof(c_header));
-	CAMLreturn(s);
-}
-
 CAMLprim value stub_xc_interface_open(void)
 {
 	CAMLparam0();
-- 
2.1.4


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

[PATCH 2/2] xen: Drop DOMCTL_getmemlist and xc_get_pfn_list()

[Andrew Cooper]

c/s 4ddf474e2 "tools/xen-mceinj: Pass in GPA when injecting through
MSR_MCI_ADDR" removed the remaining user of hypercall.

It has been listed as broken, deprecated and wont-fix since XSA-74, so take
this opportunity to remove it completely.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Christian Lindig <christian.lindig@citrix.com>
---
 tools/libxc/include/xenctrl.h       |  7 -----
 tools/libxc/xc_private.c            | 27 ------------------
 tools/ocaml/libs/xc/xenctrl.ml      |  3 --
 tools/ocaml/libs/xc/xenctrl.mli     |  3 --
 tools/ocaml/libs/xc/xenctrl_stubs.c | 32 ---------------------
 xen/arch/x86/domctl.c               | 56 -------------------------------------
 xen/include/public/domctl.h         |  2 +-
 7 files changed, 1 insertion(+), 129 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index ecb0312..30171a2 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1520,13 +1520,6 @@ unsigned long xc_translate_foreign_address(xc_interface *xch, uint32_t dom,
                                            int vcpu, unsigned long long virt);
 
 
-/**
- * DEPRECATED.  Avoid using this, as it does not correctly account for PFNs
- * without a backing MFN.
- */
-int xc_get_pfn_list(xc_interface *xch, uint32_t domid, uint64_t *pfn_buf,
-                    unsigned long max_pfns);
-
 int xc_copy_to_domain_page(xc_interface *xch, uint32_t domid,
                            unsigned long dst_pfn, const char *src_page);
 
diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c
index 36ead5f..fcda981 100644
--- a/tools/libxc/xc_private.c
+++ b/tools/libxc/xc_private.c
@@ -387,33 +387,6 @@ int xc_machphys_mfn_list(xc_interface *xch,
     return rc;
 }
 
-int xc_get_pfn_list(xc_interface *xch,
-                    uint32_t domid,
-                    uint64_t *pfn_buf,
-                    unsigned long max_pfns)
-{
-    DECLARE_DOMCTL;
-    DECLARE_HYPERCALL_BOUNCE(pfn_buf, max_pfns * sizeof(*pfn_buf), XC_HYPERCALL_BUFFER_BOUNCE_OUT);
-    int ret;
-
-    if ( xc_hypercall_bounce_pre(xch, pfn_buf) )
-    {
-        PERROR("xc_get_pfn_list: pfn_buf bounce failed");
-        return -1;
-    }
-
-    domctl.cmd = XEN_DOMCTL_getmemlist;
-    domctl.domain = domid;
-    domctl.u.getmemlist.max_pfns = max_pfns;
-    set_xen_guest_handle(domctl.u.getmemlist.buffer, pfn_buf);
-
-    ret = do_domctl(xch, &domctl);
-
-    xc_hypercall_bounce_post(xch, pfn_buf);
-
-    return (ret < 0) ? -1 : domctl.u.getmemlist.num_pfns;
-}
-
 long xc_get_tot_pages(xc_interface *xch, uint32_t domid)
 {
     xc_dominfo_t info;
diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
index a3ba488..1a01faa 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -244,9 +244,6 @@ external map_foreign_range: handle -> domid -> int
                          -> nativeint -> Xenmmap.mmap_interface
        = "stub_map_foreign_range"
 
-external domain_get_pfn_list: handle -> domid -> nativeint -> nativeint array
-       = "stub_xc_domain_get_pfn_list"
-
 external domain_assign_device: handle -> domid -> (int * int * int * int) -> unit
        = "stub_xc_domain_assign_device"
 external domain_deassign_device: handle -> domid -> (int * int * int * int) -> unit
diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
index ed02124..7d2e6f0 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -154,9 +154,6 @@ external domain_memory_increase_reservation :
 external map_foreign_range :
   handle -> domid -> int -> nativeint -> Xenmmap.mmap_interface
   = "stub_map_foreign_range"
-external domain_get_pfn_list :
-  handle -> domid -> nativeint -> nativeint array
-  = "stub_xc_domain_get_pfn_list"
 
 external domain_assign_device: handle -> domid -> (int * int * int * int) -> unit
        = "stub_xc_domain_assign_device"
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c
index d1801e1..f97070c 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -1009,38 +1009,6 @@ CAMLprim value stub_shadow_allocation_set(value xch, value domid,
 	CAMLreturn(Val_unit);
 }
 
-CAMLprim value stub_xc_domain_get_pfn_list(value xch, value domid,
-                                           value nr_pfns)
-{
-	CAMLparam3(xch, domid, nr_pfns);
-	CAMLlocal2(array, v);
-	unsigned long c_nr_pfns;
-	long ret, i;
-	uint64_t *c_array;
-
-	c_nr_pfns = Nativeint_val(nr_pfns);
-
-	c_array = malloc(sizeof(uint64_t) * c_nr_pfns);
-	if (!c_array)
-		caml_raise_out_of_memory();
-
-	ret = xc_get_pfn_list(_H(xch), _D(domid),
-			      c_array, c_nr_pfns);
-	if (ret < 0) {
-		free(c_array);
-		failwith_xc(_H(xch));
-	}
-
-	array = caml_alloc(ret, 0);
-	for (i = 0; i < ret; i++) {
-		v = caml_copy_nativeint(c_array[i]);
-		Store_field(array, i, v);
-	}
-	free(c_array);
-
-	CAMLreturn(array);
-}
-
 CAMLprim value stub_xc_domain_ioport_permission(value xch, value domid,
 					       value start_port, value nr_ports,
 					       value allow)
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 2585d4e..129c24e 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -395,62 +395,6 @@ long arch_do_domctl(
         break;
     }
 
-    case XEN_DOMCTL_getmemlist:
-    {
-        unsigned long max_pfns = domctl->u.getmemlist.max_pfns;
-        uint64_t mfn;
-        struct page_info *page;
-
-        if ( unlikely(d->is_dying) )
-        {
-            ret = -EINVAL;
-            break;
-        }
-
-        /*
-         * XSA-74: This sub-hypercall is broken in several ways:
-         * - lock order inversion (p2m locks inside page_alloc_lock)
-         * - no preemption on huge max_pfns input
-         * - not (re-)checking d->is_dying with page_alloc_lock held
-         * - not honoring start_pfn input (which libxc also doesn't set)
-         * Additionally it is rather useless, as the result is stale by the
-         * time the caller gets to look at it.
-         * As it only has a single, non-production consumer (xen-mceinj),
-         * rather than trying to fix it we restrict it for the time being.
-         */
-        if ( /* No nested locks inside copy_to_guest_offset(). */
-             paging_mode_external(currd) ||
-             /* Arbitrary limit capping processing time. */
-             max_pfns > GB(4) / PAGE_SIZE )
-        {
-            ret = -EOPNOTSUPP;
-            break;
-        }
-
-        spin_lock(&d->page_alloc_lock);
-
-        ret = i = 0;
-        page_list_for_each(page, &d->page_list)
-        {
-            if ( i >= max_pfns )
-                break;
-            mfn = page_to_mfn(page);
-            if ( copy_to_guest_offset(domctl->u.getmemlist.buffer,
-                                      i, &mfn, 1) )
-            {
-                ret = -EFAULT;
-                break;
-            }
-			++i;
-		}
-
-        spin_unlock(&d->page_alloc_lock);
-
-        domctl->u.getmemlist.num_pfns = i;
-        copyback = true;
-        break;
-    }
-
     case XEN_DOMCTL_getpageframeinfo3:
     {
         unsigned int num = domctl->u.getpageframeinfo3.num;
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index a8921dd..7f99d1b 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1117,7 +1117,7 @@ struct xen_domctl {
 #define XEN_DOMCTL_pausedomain                    3
 #define XEN_DOMCTL_unpausedomain                  4
 #define XEN_DOMCTL_getdomaininfo                  5
-#define XEN_DOMCTL_getmemlist                     6
+/* #define XEN_DOMCTL_getmemlist                  6 Obsolete */
 /* #define XEN_DOMCTL_getpageframeinfo            7 Obsolete - use getpageframeinfo3 */
 /* #define XEN_DOMCTL_getpageframeinfo2           8 Obsolete - use getpageframeinfo3 */
 #define XEN_DOMCTL_setvcpuaffinity                9
-- 
2.1.4


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

Re: [PATCH 2/2] xen: Drop DOMCTL_getmemlist and xc_get_pfn_list()

[Jan Beulich]

>>> On 19.01.18 at 20:19, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -1117,7 +1117,7 @@ struct xen_domctl {
>  #define XEN_DOMCTL_pausedomain                    3
>  #define XEN_DOMCTL_unpausedomain                  4
>  #define XEN_DOMCTL_getdomaininfo                  5
> -#define XEN_DOMCTL_getmemlist                     6
> +/* #define XEN_DOMCTL_getmemlist                  6 Obsolete */
>  /* #define XEN_DOMCTL_getpageframeinfo            7 Obsolete - use getpageframeinfo3 */
>  /* #define XEN_DOMCTL_getpageframeinfo2           8 Obsolete - use getpageframeinfo3 */
>  #define XEN_DOMCTL_setvcpuaffinity                9

Just like mentioned upon someone else's recent submission to
remove a domctl sub-op: You want to bump the interface version
(remember that the bump done for the shim doesn't count as long
as there is a possible plan to make that other recent commit part
of a 4.10.x stable release). Plus I again question whether
"Obsolete" is an appropriate description for something that's no
longer part of the interface (rather than just being suggested to
no longer be used). Is there any point in keeping the old sub-op
as a comment in the first place?

With this suitably addressed, the hypervisor side is
Acked-by: Jan Beulich <jbeulich@suse.com>

Jan


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

Re: [PATCH 2/2] xen: Drop DOMCTL_getmemlist and xc_get_pfn_list()

[Andrew Cooper]

On 22/01/18 12:41, Jan Beulich wrote:
>>>> On 19.01.18 at 20:19, <andrew.cooper3@citrix.com> wrote:
>> --- a/xen/include/public/domctl.h
>> +++ b/xen/include/public/domctl.h
>> @@ -1117,7 +1117,7 @@ struct xen_domctl {
>>  #define XEN_DOMCTL_pausedomain                    3
>>  #define XEN_DOMCTL_unpausedomain                  4
>>  #define XEN_DOMCTL_getdomaininfo                  5
>> -#define XEN_DOMCTL_getmemlist                     6
>> +/* #define XEN_DOMCTL_getmemlist                  6 Obsolete */
>>  /* #define XEN_DOMCTL_getpageframeinfo            7 Obsolete - use getpageframeinfo3 */
>>  /* #define XEN_DOMCTL_getpageframeinfo2           8 Obsolete - use getpageframeinfo3 */
>>  #define XEN_DOMCTL_setvcpuaffinity                9
> Just like mentioned upon someone else's recent submission to
> remove a domctl sub-op: You want to bump the interface version
> (remember that the bump done for the shim doesn't count as long
> as there is a possible plan to make that other recent commit part
> of a 4.10.x stable release).

There has already been a version bump for 4.11.

> Plus I again question whether
> "Obsolete" is an appropriate description for something that's no
> longer part of the interface (rather than just being suggested to
> no longer be used). Is there any point in keeping the old sub-op
> as a comment in the first place?

To avoid the number being reused.  It also serves as a marker to locate
the change which removed the hypercall if anyone is doing archaeology in
the future.

How about removed instead of obsolete?

~Andrew

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

Re: [PATCH 2/2] xen: Drop DOMCTL_getmemlist and xc_get_pfn_list()

[Jan Beulich]

>>> On 22.01.18 at 13:52, <andrew.cooper3@citrix.com> wrote:
> On 22/01/18 12:41, Jan Beulich wrote:
>>>>> On 19.01.18 at 20:19, <andrew.cooper3@citrix.com> wrote:
>>> --- a/xen/include/public/domctl.h
>>> +++ b/xen/include/public/domctl.h
>>> @@ -1117,7 +1117,7 @@ struct xen_domctl {
>>>  #define XEN_DOMCTL_pausedomain                    3
>>>  #define XEN_DOMCTL_unpausedomain                  4
>>>  #define XEN_DOMCTL_getdomaininfo                  5
>>> -#define XEN_DOMCTL_getmemlist                     6
>>> +/* #define XEN_DOMCTL_getmemlist                  6 Obsolete */
>>>  /* #define XEN_DOMCTL_getpageframeinfo            7 Obsolete - use 
> getpageframeinfo3 */
>>>  /* #define XEN_DOMCTL_getpageframeinfo2           8 Obsolete - use 
> getpageframeinfo3 */
>>>  #define XEN_DOMCTL_setvcpuaffinity                9
>> Just like mentioned upon someone else's recent submission to
>> remove a domctl sub-op: You want to bump the interface version
>> (remember that the bump done for the shim doesn't count as long
>> as there is a possible plan to make that other recent commit part
>> of a 4.10.x stable release).
> 
> There has already been a version bump for 4.11.

I know, hence the longer explanation, which I had given also
when the shim series was first posted: If that domctl change is
to be backported to 4.10, interface version 0xf will be burnt
for _just that change_. That other bump is sufficient only when
there is no plan whatsoever to backport the earlier change.

>> Plus I again question whether
>> "Obsolete" is an appropriate description for something that's no
>> longer part of the interface (rather than just being suggested to
>> no longer be used). Is there any point in keeping the old sub-op
>> as a comment in the first place?
> 
> To avoid the number being reused.  It also serves as a marker to locate
> the change which removed the hypercall if anyone is doing archaeology in
> the future.

The number getting re-used with a higher interface version is no
problem at all, afaics.

> How about removed instead of obsolete?

That would be fine with me.

Jan


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

Re: [PATCH 2/2] xen: Drop DOMCTL_getmemlist and xc_get_pfn_list()

[Andrew Cooper]

On 22/01/18 13:01, Jan Beulich wrote:
>>>> On 22.01.18 at 13:52, <andrew.cooper3@citrix.com> wrote:
>> On 22/01/18 12:41, Jan Beulich wrote:
>>>>>> On 19.01.18 at 20:19, <andrew.cooper3@citrix.com> wrote:
>>>> --- a/xen/include/public/domctl.h
>>>> +++ b/xen/include/public/domctl.h
>>>> @@ -1117,7 +1117,7 @@ struct xen_domctl {
>>>>  #define XEN_DOMCTL_pausedomain                    3
>>>>  #define XEN_DOMCTL_unpausedomain                  4
>>>>  #define XEN_DOMCTL_getdomaininfo                  5
>>>> -#define XEN_DOMCTL_getmemlist                     6
>>>> +/* #define XEN_DOMCTL_getmemlist                  6 Obsolete */
>>>>  /* #define XEN_DOMCTL_getpageframeinfo            7 Obsolete - use 
>> getpageframeinfo3 */
>>>>  /* #define XEN_DOMCTL_getpageframeinfo2           8 Obsolete - use 
>> getpageframeinfo3 */
>>>>  #define XEN_DOMCTL_setvcpuaffinity                9
>>> Just like mentioned upon someone else's recent submission to
>>> remove a domctl sub-op: You want to bump the interface version
>>> (remember that the bump done for the shim doesn't count as long
>>> as there is a possible plan to make that other recent commit part
>>> of a 4.10.x stable release).
>> There has already been a version bump for 4.11.
> I know, hence the longer explanation, which I had given also
> when the shim series was first posted: If that domctl change is
> to be backported to 4.10, interface version 0xf will be burnt
> for _just that change_. That other bump is sufficient only when
> there is no plan whatsoever to backport the earlier change.

If that change is backported to 4.10, that is the time to burn another
interface version.  Not in this patch.

Also, this demonstrates the inherent problems with the interface
version.  This trick can only ever be played on the most recently
released branch.  It is a dire trainwreck in terms of versioning, and
serves only to make it almost impossible to make changes to an installed
system.

>
>>> Plus I again question whether
>>> "Obsolete" is an appropriate description for something that's no
>>> longer part of the interface (rather than just being suggested to
>>> no longer be used). Is there any point in keeping the old sub-op
>>> as a comment in the first place?
>> To avoid the number being reused.  It also serves as a marker to locate
>> the change which removed the hypercall if anyone is doing archaeology in
>> the future.
> The number getting re-used with a higher interface version is no
> problem at all, afaics.

Yes it is.  do_domctl() (which inserts the domctl version) is remote
from the choice of op to use, so reusing numbers means that the language
subs around libxc can issue completely erroneous hypercalls without
suffering a build or version failure.  (Again, see trainwreck of a
versioning scheme.)

~Andrew

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

Re: [PATCH 2/2] xen: Drop DOMCTL_getmemlist and xc_get_pfn_list()

[Jan Beulich]

>>> On 22.01.18 at 14:29, <andrew.cooper3@citrix.com> wrote:
> On 22/01/18 13:01, Jan Beulich wrote:
>>>>> On 22.01.18 at 13:52, <andrew.cooper3@citrix.com> wrote:
>>> On 22/01/18 12:41, Jan Beulich wrote:
>>>>>>> On 19.01.18 at 20:19, <andrew.cooper3@citrix.com> wrote:
>>>>> --- a/xen/include/public/domctl.h
>>>>> +++ b/xen/include/public/domctl.h
>>>>> @@ -1117,7 +1117,7 @@ struct xen_domctl {
>>>>>  #define XEN_DOMCTL_pausedomain                    3
>>>>>  #define XEN_DOMCTL_unpausedomain                  4
>>>>>  #define XEN_DOMCTL_getdomaininfo                  5
>>>>> -#define XEN_DOMCTL_getmemlist                     6
>>>>> +/* #define XEN_DOMCTL_getmemlist                  6 Obsolete */
>>>>>  /* #define XEN_DOMCTL_getpageframeinfo            7 Obsolete - use 
>>> getpageframeinfo3 */
>>>>>  /* #define XEN_DOMCTL_getpageframeinfo2           8 Obsolete - use 
>>> getpageframeinfo3 */
>>>>>  #define XEN_DOMCTL_setvcpuaffinity                9
>>>> Just like mentioned upon someone else's recent submission to
>>>> remove a domctl sub-op: You want to bump the interface version
>>>> (remember that the bump done for the shim doesn't count as long
>>>> as there is a possible plan to make that other recent commit part
>>>> of a 4.10.x stable release).
>>> There has already been a version bump for 4.11.
>> I know, hence the longer explanation, which I had given also
>> when the shim series was first posted: If that domctl change is
>> to be backported to 4.10, interface version 0xf will be burnt
>> for _just that change_. That other bump is sufficient only when
>> there is no plan whatsoever to backport the earlier change.
> 
> If that change is backported to 4.10, that is the time to burn another
> interface version.  Not in this patch.

Not if the backport happens only after 4.11 has shipped. And
even it the backport happened earlier, we're liable to forget if
we don't do it now. If there was just a remote chance of that
backport to happen, I probably wouldn't insist, but aiui there's
a pretty determined plan to do so.

I also find it strange that you didn't respond back when I had
first outlined this extra requirement.

> Also, this demonstrates the inherent problems with the interface
> version.  This trick can only ever be played on the most recently
> released branch.  It is a dire trainwreck in terms of versioning, and
> serves only to make it almost impossible to make changes to an installed
> system.

It's not optimal, but I have yet to see a proposal of a mechanism
that's more flexible than this one, but provides at least the same
minimal protection against mismatches.

As to changes to an installed system - the domctl interface should
be in sufficiently usable a shape that such won't be necessary. Or
in the worst case new sub-ops could always be added.

>>>> Plus I again question whether
>>>> "Obsolete" is an appropriate description for something that's no
>>>> longer part of the interface (rather than just being suggested to
>>>> no longer be used). Is there any point in keeping the old sub-op
>>>> as a comment in the first place?
>>> To avoid the number being reused.  It also serves as a marker to locate
>>> the change which removed the hypercall if anyone is doing archaeology in
>>> the future.
>> The number getting re-used with a higher interface version is no
>> problem at all, afaics.
> 
> Yes it is.  do_domctl() (which inserts the domctl version) is remote
> from the choice of op to use, so reusing numbers means that the language
> subs around libxc can issue completely erroneous hypercalls without
> suffering a build or version failure.  (Again, see trainwreck of a
> versioning scheme.)

do_domctl() itself shouldn't be available for use outside of libxc.
And the actual libxc wrapper for a removed sub-op would be
unavailable in the shared object matching the underlying Xen.

Jan


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

[PING] [PATCH 2/2] xen: Drop DOMCTL_getmemlist and xc_get_pfn_list()

[Andrew Cooper]

On 19/01/18 19:19, Andrew Cooper wrote:
> c/s 4ddf474e2 "tools/xen-mceinj: Pass in GPA when injecting through
> MSR_MCI_ADDR" removed the remaining user of hypercall.
>
> It has been listed as broken, deprecated and wont-fix since XSA-74, so take
> this opportunity to remove it completely.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Christian Lindig <christian.lindig@citrix.com>
> ---
>  tools/libxc/include/xenctrl.h       |  7 -----
>  tools/libxc/xc_private.c            | 27 ------------------
>  tools/ocaml/libs/xc/xenctrl.ml      |  3 --
>  tools/ocaml/libs/xc/xenctrl.mli     |  3 --
>  tools/ocaml/libs/xc/xenctrl_stubs.c | 32 ---------------------
>  xen/arch/x86/domctl.c               | 56 -------------------------------------
>  xen/include/public/domctl.h         |  2 +-
>  7 files changed, 1 insertion(+), 129 deletions(-)
>
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index ecb0312..30171a2 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -1520,13 +1520,6 @@ unsigned long xc_translate_foreign_address(xc_interface *xch, uint32_t dom,
>                                             int vcpu, unsigned long long virt);
>  
>  
> -/**
> - * DEPRECATED.  Avoid using this, as it does not correctly account for PFNs
> - * without a backing MFN.
> - */
> -int xc_get_pfn_list(xc_interface *xch, uint32_t domid, uint64_t *pfn_buf,
> -                    unsigned long max_pfns);
> -
>  int xc_copy_to_domain_page(xc_interface *xch, uint32_t domid,
>                             unsigned long dst_pfn, const char *src_page);
>  
> diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c
> index 36ead5f..fcda981 100644
> --- a/tools/libxc/xc_private.c
> +++ b/tools/libxc/xc_private.c
> @@ -387,33 +387,6 @@ int xc_machphys_mfn_list(xc_interface *xch,
>      return rc;
>  }
>  
> -int xc_get_pfn_list(xc_interface *xch,
> -                    uint32_t domid,
> -                    uint64_t *pfn_buf,
> -                    unsigned long max_pfns)
> -{
> -    DECLARE_DOMCTL;
> -    DECLARE_HYPERCALL_BOUNCE(pfn_buf, max_pfns * sizeof(*pfn_buf), XC_HYPERCALL_BUFFER_BOUNCE_OUT);
> -    int ret;
> -
> -    if ( xc_hypercall_bounce_pre(xch, pfn_buf) )
> -    {
> -        PERROR("xc_get_pfn_list: pfn_buf bounce failed");
> -        return -1;
> -    }
> -
> -    domctl.cmd = XEN_DOMCTL_getmemlist;
> -    domctl.domain = domid;
> -    domctl.u.getmemlist.max_pfns = max_pfns;
> -    set_xen_guest_handle(domctl.u.getmemlist.buffer, pfn_buf);
> -
> -    ret = do_domctl(xch, &domctl);
> -
> -    xc_hypercall_bounce_post(xch, pfn_buf);
> -
> -    return (ret < 0) ? -1 : domctl.u.getmemlist.num_pfns;
> -}
> -
>  long xc_get_tot_pages(xc_interface *xch, uint32_t domid)
>  {
>      xc_dominfo_t info;
> diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
> index a3ba488..1a01faa 100644
> --- a/tools/ocaml/libs/xc/xenctrl.ml
> +++ b/tools/ocaml/libs/xc/xenctrl.ml
> @@ -244,9 +244,6 @@ external map_foreign_range: handle -> domid -> int
>                           -> nativeint -> Xenmmap.mmap_interface
>         = "stub_map_foreign_range"
>  
> -external domain_get_pfn_list: handle -> domid -> nativeint -> nativeint array
> -       = "stub_xc_domain_get_pfn_list"
> -
>  external domain_assign_device: handle -> domid -> (int * int * int * int) -> unit
>         = "stub_xc_domain_assign_device"
>  external domain_deassign_device: handle -> domid -> (int * int * int * int) -> unit
> diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
> index ed02124..7d2e6f0 100644
> --- a/tools/ocaml/libs/xc/xenctrl.mli
> +++ b/tools/ocaml/libs/xc/xenctrl.mli
> @@ -154,9 +154,6 @@ external domain_memory_increase_reservation :
>  external map_foreign_range :
>    handle -> domid -> int -> nativeint -> Xenmmap.mmap_interface
>    = "stub_map_foreign_range"
> -external domain_get_pfn_list :
> -  handle -> domid -> nativeint -> nativeint array
> -  = "stub_xc_domain_get_pfn_list"
>  
>  external domain_assign_device: handle -> domid -> (int * int * int * int) -> unit
>         = "stub_xc_domain_assign_device"
> diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c
> index d1801e1..f97070c 100644
> --- a/tools/ocaml/libs/xc/xenctrl_stubs.c
> +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
> @@ -1009,38 +1009,6 @@ CAMLprim value stub_shadow_allocation_set(value xch, value domid,
>  	CAMLreturn(Val_unit);
>  }
>  
> -CAMLprim value stub_xc_domain_get_pfn_list(value xch, value domid,
> -                                           value nr_pfns)
> -{
> -	CAMLparam3(xch, domid, nr_pfns);
> -	CAMLlocal2(array, v);
> -	unsigned long c_nr_pfns;
> -	long ret, i;
> -	uint64_t *c_array;
> -
> -	c_nr_pfns = Nativeint_val(nr_pfns);
> -
> -	c_array = malloc(sizeof(uint64_t) * c_nr_pfns);
> -	if (!c_array)
> -		caml_raise_out_of_memory();
> -
> -	ret = xc_get_pfn_list(_H(xch), _D(domid),
> -			      c_array, c_nr_pfns);
> -	if (ret < 0) {
> -		free(c_array);
> -		failwith_xc(_H(xch));
> -	}
> -
> -	array = caml_alloc(ret, 0);
> -	for (i = 0; i < ret; i++) {
> -		v = caml_copy_nativeint(c_array[i]);
> -		Store_field(array, i, v);
> -	}
> -	free(c_array);
> -
> -	CAMLreturn(array);
> -}
> -
>  CAMLprim value stub_xc_domain_ioport_permission(value xch, value domid,
>  					       value start_port, value nr_ports,
>  					       value allow)
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index 2585d4e..129c24e 100644
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -395,62 +395,6 @@ long arch_do_domctl(
>          break;
>      }
>  
> -    case XEN_DOMCTL_getmemlist:
> -    {
> -        unsigned long max_pfns = domctl->u.getmemlist.max_pfns;
> -        uint64_t mfn;
> -        struct page_info *page;
> -
> -        if ( unlikely(d->is_dying) )
> -        {
> -            ret = -EINVAL;
> -            break;
> -        }
> -
> -        /*
> -         * XSA-74: This sub-hypercall is broken in several ways:
> -         * - lock order inversion (p2m locks inside page_alloc_lock)
> -         * - no preemption on huge max_pfns input
> -         * - not (re-)checking d->is_dying with page_alloc_lock held
> -         * - not honoring start_pfn input (which libxc also doesn't set)
> -         * Additionally it is rather useless, as the result is stale by the
> -         * time the caller gets to look at it.
> -         * As it only has a single, non-production consumer (xen-mceinj),
> -         * rather than trying to fix it we restrict it for the time being.
> -         */
> -        if ( /* No nested locks inside copy_to_guest_offset(). */
> -             paging_mode_external(currd) ||
> -             /* Arbitrary limit capping processing time. */
> -             max_pfns > GB(4) / PAGE_SIZE )
> -        {
> -            ret = -EOPNOTSUPP;
> -            break;
> -        }
> -
> -        spin_lock(&d->page_alloc_lock);
> -
> -        ret = i = 0;
> -        page_list_for_each(page, &d->page_list)
> -        {
> -            if ( i >= max_pfns )
> -                break;
> -            mfn = page_to_mfn(page);
> -            if ( copy_to_guest_offset(domctl->u.getmemlist.buffer,
> -                                      i, &mfn, 1) )
> -            {
> -                ret = -EFAULT;
> -                break;
> -            }
> -			++i;
> -		}
> -
> -        spin_unlock(&d->page_alloc_lock);
> -
> -        domctl->u.getmemlist.num_pfns = i;
> -        copyback = true;
> -        break;
> -    }
> -
>      case XEN_DOMCTL_getpageframeinfo3:
>      {
>          unsigned int num = domctl->u.getpageframeinfo3.num;
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index a8921dd..7f99d1b 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -1117,7 +1117,7 @@ struct xen_domctl {
>  #define XEN_DOMCTL_pausedomain                    3
>  #define XEN_DOMCTL_unpausedomain                  4
>  #define XEN_DOMCTL_getdomaininfo                  5
> -#define XEN_DOMCTL_getmemlist                     6
> +/* #define XEN_DOMCTL_getmemlist                  6 Obsolete */
>  /* #define XEN_DOMCTL_getpageframeinfo            7 Obsolete - use getpageframeinfo3 */
>  /* #define XEN_DOMCTL_getpageframeinfo2           8 Obsolete - use getpageframeinfo3 */
>  #define XEN_DOMCTL_setvcpuaffinity                9


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

Re: [PATCH 2/2] xen: Drop DOMCTL_getmemlist and xc_get_pfn_list()

[Wei Liu]

On Fri, Jan 19, 2018 at 07:19:03PM +0000, Andrew Cooper wrote:
> c/s 4ddf474e2 "tools/xen-mceinj: Pass in GPA when injecting through
> MSR_MCI_ADDR" removed the remaining user of hypercall.
> 
> It has been listed as broken, deprecated and wont-fix since XSA-74, so take
> this opportunity to remove it completely.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

(Assuming no pending objection)

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

[PING] Re: [PATCH 1/2] tools/ocaml: Drop coredump infrastructure

[Andrew Cooper]

On 19/01/18 19:19, Andrew Cooper wrote:
> It is unused, and uses an obsolete hypercall which has never ever functioned
> for HVM guests.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Christian Lindig <christian.lindig@citrix.com>
> ---
>  tools/ocaml/libs/xc/xenctrl.ml      | 86 -------------------------------------
>  tools/ocaml/libs/xc/xenctrl.mli     | 16 -------
>  tools/ocaml/libs/xc/xenctrl_stubs.c | 41 ------------------
>  3 files changed, 143 deletions(-)
>
> diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
> index 9116aa2..a3ba488 100644
> --- a/tools/ocaml/libs/xc/xenctrl.ml
> +++ b/tools/ocaml/libs/xc/xenctrl.ml
> @@ -126,14 +126,6 @@ exception Error of string
>  
>  type handle
>  
> -(* this is only use by coredumping *)
> -external sizeof_core_header: unit -> int
> -       = "stub_sizeof_core_header"
> -external sizeof_vcpu_guest_context: unit -> int
> -       = "stub_sizeof_vcpu_guest_context"
> -external sizeof_xen_pfn: unit -> int = "stub_sizeof_xen_pfn"
> -(* end of use *)
> -
>  external interface_open: unit -> handle = "stub_xc_interface_open"
>  external interface_close: handle -> unit = "stub_xc_interface_close"
>  
> @@ -275,84 +267,6 @@ external get_cpu_featureset : handle -> featureset_index -> int64 array = "stub_
>  external watchdog : handle -> int -> int32 -> int
>    = "stub_xc_watchdog"
>  
> -(* core dump structure *)
> -type core_magic = Magic_hvm | Magic_pv
> -
> -type core_header = {
> -	xch_magic: core_magic;
> -	xch_nr_vcpus: int;
> -	xch_nr_pages: nativeint;
> -	xch_index_offset: int64;
> -	xch_ctxt_offset: int64;
> -	xch_pages_offset: int64;
> -}
> -
> -external marshall_core_header: core_header -> string = "stub_marshall_core_header"
> -
> -(* coredump *)
> -let coredump xch domid fd =
> -	let dump s =
> -		let wd = Unix.write fd s 0 (String.length s) in
> -		if wd <> String.length s then
> -			failwith "error while writing";
> -		in
> -
> -	let info = domain_getinfo xch domid in
> -
> -	let nrpages = info.total_memory_pages in
> -	let ctxt = Array.make info.max_vcpu_id None in
> -	let nr_vcpus = ref 0 in
> -	for i = 0 to info.max_vcpu_id - 1
> -	do
> -		ctxt.(i) <- try
> -			let v = vcpu_context_get xch domid i in
> -			incr nr_vcpus;
> -			Some v
> -			with _ -> None
> -	done;
> -
> -	(* FIXME page offset if not rounded to sup *)
> -	let page_offset =
> -		Int64.add
> -			(Int64.of_int (sizeof_core_header () +
> -			 (sizeof_vcpu_guest_context () * !nr_vcpus)))
> -			(Int64.of_nativeint (
> -				Nativeint.mul
> -					(Nativeint.of_int (sizeof_xen_pfn ()))
> -					nrpages)
> -				)
> -		in
> -
> -	let header = {
> -		xch_magic = if info.hvm_guest then Magic_hvm else Magic_pv;
> -		xch_nr_vcpus = !nr_vcpus;
> -		xch_nr_pages = nrpages;
> -		xch_ctxt_offset = Int64.of_int (sizeof_core_header ());
> -		xch_index_offset = Int64.of_int (sizeof_core_header ()
> -					+ sizeof_vcpu_guest_context ());
> -		xch_pages_offset = page_offset;
> -	} in
> -
> -	dump (marshall_core_header header);
> -	for i = 0 to info.max_vcpu_id - 1
> -	do
> -		match ctxt.(i) with
> -		| None -> ()
> -		| Some ctxt_i -> dump ctxt_i
> -	done;
> -	let pfns = domain_get_pfn_list xch domid nrpages in
> -	if Array.length pfns <> Nativeint.to_int nrpages then
> -		failwith "could not get the page frame list";
> -
> -	let page_size = Xenmmap.getpagesize () in
> -	for i = 0 to Nativeint.to_int nrpages - 1
> -	do
> -		let page = map_foreign_range xch domid page_size pfns.(i) in
> -		let data = Xenmmap.read page 0 page_size in
> -		Xenmmap.unmap page;
> -		dump data
> -	done
> -
>  (* ** Misc ** *)
>  
>  (**
> diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
> index 54c099c..ed02124 100644
> --- a/tools/ocaml/libs/xc/xenctrl.mli
> +++ b/tools/ocaml/libs/xc/xenctrl.mli
> @@ -95,10 +95,6 @@ type domain_create_flag = CDF_HVM | CDF_HAP
>  
>  exception Error of string
>  type handle
> -external sizeof_core_header : unit -> int = "stub_sizeof_core_header"
> -external sizeof_vcpu_guest_context : unit -> int
> -  = "stub_sizeof_vcpu_guest_context"
> -external sizeof_xen_pfn : unit -> int = "stub_sizeof_xen_pfn"
>  external interface_open : unit -> handle = "stub_xc_interface_open"
>  external interface_close : handle -> unit = "stub_xc_interface_close"
>  val with_intf : (handle -> 'a) -> 'a
> @@ -179,18 +175,6 @@ external version_capabilities : handle -> string
>  type featureset_index = Featureset_raw | Featureset_host | Featureset_pv | Featureset_hvm
>  external get_cpu_featureset : handle -> featureset_index -> int64 array = "stub_xc_get_cpu_featureset"
>  
> -type core_magic = Magic_hvm | Magic_pv
> -type core_header = {
> -  xch_magic : core_magic;
> -  xch_nr_vcpus : int;
> -  xch_nr_pages : nativeint;
> -  xch_index_offset : int64;
> -  xch_ctxt_offset : int64;
> -  xch_pages_offset : int64;
> -}
> -external marshall_core_header : core_header -> string
> -  = "stub_marshall_core_header"
> -val coredump : handle -> domid -> Unix.file_descr -> unit
>  external pages_to_kib : int64 -> int64 = "stub_pages_to_kib"
>  val pages_to_mib : int64 -> int64
>  external watchdog : handle -> int -> int32 -> int
> diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c
> index dd6000c..d1801e1 100644
> --- a/tools/ocaml/libs/xc/xenctrl_stubs.c
> +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
> @@ -72,47 +72,6 @@ static void Noreturn failwith_xc(xc_interface *xch)
>  	caml_raise_with_string(*caml_named_value("xc.error"), error_str);
>  }
>  
> -CAMLprim value stub_sizeof_core_header(value unit)
> -{
> -	CAMLparam1(unit);
> -	CAMLreturn(Val_int(sizeof(struct xc_core_header)));
> -}
> -
> -CAMLprim value stub_sizeof_vcpu_guest_context(value unit)
> -{
> -	CAMLparam1(unit);
> -	CAMLreturn(Val_int(sizeof(struct vcpu_guest_context)));
> -}
> -
> -CAMLprim value stub_sizeof_xen_pfn(value unit)
> -{
> -	CAMLparam1(unit);
> -	CAMLreturn(Val_int(sizeof(xen_pfn_t)));
> -}
> -
> -#define XC_CORE_MAGIC     0xF00FEBED
> -#define XC_CORE_MAGIC_HVM 0xF00FEBEE
> -
> -CAMLprim value stub_marshall_core_header(value header)
> -{
> -	CAMLparam1(header);
> -	CAMLlocal1(s);
> -	struct xc_core_header c_header;
> -
> -	c_header.xch_magic = (Field(header, 0))
> -		? XC_CORE_MAGIC
> -		: XC_CORE_MAGIC_HVM;
> -	c_header.xch_nr_vcpus = Int_val(Field(header, 1));
> -	c_header.xch_nr_pages = Nativeint_val(Field(header, 2));
> -	c_header.xch_ctxt_offset = Int64_val(Field(header, 3));
> -	c_header.xch_index_offset = Int64_val(Field(header, 4));
> -	c_header.xch_pages_offset = Int64_val(Field(header, 5));
> -
> -	s = caml_alloc_string(sizeof(c_header));
> -	memcpy(String_val(s), (char *) &c_header, sizeof(c_header));
> -	CAMLreturn(s);
> -}
> -
>  CAMLprim value stub_xc_interface_open(void)
>  {
>  	CAMLparam0();


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

Re: [PATCH 1/2] tools/ocaml: Drop coredump infrastructure

[Christian Lindig]

> On 19. Jan 2018, at 19:19, Andrew Cooper <Andrew.Cooper3@citrix.com> wrote:
> 
> It is unused, and uses an obsolete hypercall which has never ever functioned
> for HVM guests.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Christian Lindig <christian.lindig@citrix.com>
> ---
> tools/ocaml/libs/xc/xenctrl.ml      | 86 -------------------------------------
> tools/ocaml/libs/xc/xenctrl.mli     | 16 -------
> tools/ocaml/libs/xc/xenctrl_stubs.c | 41 ------------------
> 3 files changed, 143 deletions(-)

I’m fine with this.

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

Re: [PATCH 1/2] tools/ocaml: Drop coredump infrastructure

[Andrew Cooper]

On 25/01/18 19:47, Christian Lindig wrote:
>
>> On 19. Jan 2018, at 19:19, Andrew Cooper <Andrew.Cooper3@citrix.com> wrote:
>>
>> It is unused, and uses an obsolete hypercall which has never ever functioned
>> for HVM guests.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>> CC: Christian Lindig <christian.lindig@citrix.com>
>> ---
>> tools/ocaml/libs/xc/xenctrl.ml      | 86 -------------------------------------
>> tools/ocaml/libs/xc/xenctrl.mli     | 16 -------
>> tools/ocaml/libs/xc/xenctrl_stubs.c | 41 ------------------
>> 3 files changed, 143 deletions(-)
> I’m fine with this.

Is that an acked-by or reviewed-by then?

~Andrew

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

Re: [PATCH 1/2] tools/ocaml: Drop coredump infrastructure

[Christian Lindig]

> On 25. Jan 2018, at 19:50, Andrew Cooper <Andrew.Cooper3@citrix.com> wrote:
> 
> On 25/01/18 19:47, Christian Lindig wrote:
>> 
>>> On 19. Jan 2018, at 19:19, Andrew Cooper <Andrew.Cooper3@citrix.com> wrote:
>>> 
>>> It is unused, and uses an obsolete hypercall which has never ever functioned
>>> for HVM guests.
>>> 
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> ---
>>> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
>>> CC: Wei Liu <wei.liu2@citrix.com>
>>> CC: Christian Lindig <christian.lindig@citrix.com>
>>> ---
>>> tools/ocaml/libs/xc/xenctrl.ml      | 86 -------------------------------------
>>> tools/ocaml/libs/xc/xenctrl.mli     | 16 -------
>>> tools/ocaml/libs/xc/xenctrl_stubs.c | 41 ------------------
>>> 3 files changed, 143 deletions(-)
>> I’m fine with this.
> 
> Is that an acked-by or reviewed-by then?
> 
> ~Andrew

Acked-by: Christian Lindig <christian.lindig@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel