[Xen-devel] [PATCH 2/2] xen: Drop DOMCTL_getmemlist and xc_get_pfn_list()
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
---
CC: Jan Beulich
CC: Ian Jackson
CC: Wei Liu
CC: Christian Lindig
---
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++) {
Re: [Xen-devel] [PATCH 2/2] xen: Drop DOMCTL_getmemlist and xc_get_pfn_list()
>>> On 19.01.18 at 20:19, wrote:
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -1117,7 +1117,7 @@ struct xen_domctl {
> #define XEN_DOMCTL_pausedomain3
> #define XEN_DOMCTL_unpausedomain 4
> #define XEN_DOMCTL_getdomaininfo 5
> -#define XEN_DOMCTL_getmemlist 6
> +/* #define XEN_DOMCTL_getmemlist 6 Obsolete */
> /* #define XEN_DOMCTL_getpageframeinfo7 Obsolete - use
> getpageframeinfo3 */
> /* #define XEN_DOMCTL_getpageframeinfo2 8 Obsolete - use
> getpageframeinfo3 */
> #define XEN_DOMCTL_setvcpuaffinity9
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
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/2] xen: Drop DOMCTL_getmemlist and xc_get_pfn_list()
On 22/01/18 12:41, Jan Beulich wrote:
On 19.01.18 at 20:19, wrote:
>> --- a/xen/include/public/domctl.h
>> +++ b/xen/include/public/domctl.h
>> @@ -1117,7 +1117,7 @@ struct xen_domctl {
>> #define XEN_DOMCTL_pausedomain3
>> #define XEN_DOMCTL_unpausedomain 4
>> #define XEN_DOMCTL_getdomaininfo 5
>> -#define XEN_DOMCTL_getmemlist 6
>> +/* #define XEN_DOMCTL_getmemlist 6 Obsolete */
>> /* #define XEN_DOMCTL_getpageframeinfo7 Obsolete - use
>> getpageframeinfo3 */
>> /* #define XEN_DOMCTL_getpageframeinfo2 8 Obsolete - use
>> getpageframeinfo3 */
>> #define XEN_DOMCTL_setvcpuaffinity9
> 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: [Xen-devel] [PATCH 2/2] xen: Drop DOMCTL_getmemlist and xc_get_pfn_list()
>>> On 22.01.18 at 13:52, wrote:
> On 22/01/18 12:41, Jan Beulich wrote:
> On 19.01.18 at 20:19, wrote:
>>> --- a/xen/include/public/domctl.h
>>> +++ b/xen/include/public/domctl.h
>>> @@ -1117,7 +1117,7 @@ struct xen_domctl {
>>> #define XEN_DOMCTL_pausedomain3
>>> #define XEN_DOMCTL_unpausedomain 4
>>> #define XEN_DOMCTL_getdomaininfo 5
>>> -#define XEN_DOMCTL_getmemlist 6
>>> +/* #define XEN_DOMCTL_getmemlist 6 Obsolete */
>>> /* #define XEN_DOMCTL_getpageframeinfo7 Obsolete - use
> getpageframeinfo3 */
>>> /* #define XEN_DOMCTL_getpageframeinfo2 8 Obsolete - use
> getpageframeinfo3 */
>>> #define XEN_DOMCTL_setvcpuaffinity9
>> 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: [Xen-devel] [PATCH 2/2] xen: Drop DOMCTL_getmemlist and xc_get_pfn_list()
On 22/01/18 13:01, Jan Beulich wrote:
On 22.01.18 at 13:52, wrote:
>> On 22/01/18 12:41, Jan Beulich wrote:
>> On 19.01.18 at 20:19, wrote:
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1117,7 +1117,7 @@ struct xen_domctl {
#define XEN_DOMCTL_pausedomain3
#define XEN_DOMCTL_unpausedomain 4
#define XEN_DOMCTL_getdomaininfo 5
-#define XEN_DOMCTL_getmemlist 6
+/* #define XEN_DOMCTL_getmemlist 6 Obsolete */
/* #define XEN_DOMCTL_getpageframeinfo7 Obsolete - use
>> getpageframeinfo3 */
/* #define XEN_DOMCTL_getpageframeinfo2 8 Obsolete - use
>> getpageframeinfo3 */
#define XEN_DOMCTL_setvcpuaffinity9
>>> 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: [Xen-devel] [PATCH 2/2] xen: Drop DOMCTL_getmemlist and xc_get_pfn_list()
>>> On 22.01.18 at 14:29, wrote:
> On 22/01/18 13:01, Jan Beulich wrote:
> On 22.01.18 at 13:52, wrote:
>>> On 22/01/18 12:41, Jan Beulich wrote:
>>> On 19.01.18 at 20:19, wrote:
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -1117,7 +1117,7 @@ struct xen_domctl {
> #define XEN_DOMCTL_pausedomain3
> #define XEN_DOMCTL_unpausedomain 4
> #define XEN_DOMCTL_getdomaininfo 5
> -#define XEN_DOMCTL_getmemlist 6
> +/* #define XEN_DOMCTL_getmemlist 6 Obsolete */
> /* #define XEN_DOMCTL_getpageframeinfo7 Obsolete - use
>>> getpageframeinfo3 */
> /* #define XEN_DOMCTL_getpageframeinfo2 8 Obsolete - use
>>> getpageframeinfo3 */
> #define XEN_DOMCTL_setvcpuaffinity9
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
Re: [Xen-devel] [PATCH 2/2] xen: Drop DOMCTL_getmemlist and xc_get_pfn_list()
On Fri, Jan 19, 2018 at 07:19:03PM +, 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 Acked-by: Wei Liu (Assuming no pending objection) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
