[PATCH for-4.16 0/4] gnttab: fallout/improvements from max version

[PATCH for-4.16 0/4] gnttab: fallout/improvements from max version

[Roger Pau Monne]

Hello,

The first patch introduces some syntactic sugar for setting the max
grant version on the domctl, while the later 3 patches fix missing
occurrences of xen_domctl_createdomain that didn't set the max grant
version. It's likely patches 2 and 3 could be squashed.

All should be considered for the release, or else we need to revert the
grant table version per domain change.

Thanks, Roger.

Roger Pau Monne (4):
  domctl: introduce a macro to set the grant table max version
  tests/resource: set grant version for created domains
  test/tsx: set grant version for created domains
  tools/python: fix python libxc bindings to pass a max grant version

 tools/helpers/init-xenstore-domain.c |  2 +-
 tools/libs/light/libxl_create.c      |  2 +-
 tools/ocaml/libs/xc/xenctrl_stubs.c  |  3 ++-
 tools/python/xen/lowlevel/xc/xc.c    | 10 ++++++----
 tools/tests/resource/test-resource.c |  3 +++
 tools/tests/tsx/test-tsx.c           |  4 ++++
 xen/arch/arm/domain_build.c          |  4 ++--
 xen/arch/x86/setup.c                 |  2 +-
 xen/include/public/domctl.h          |  1 +
 9 files changed, 21 insertions(+), 10 deletions(-)

-- 
2.33.0

[PATCH for-4.16 1/4] domctl: introduce a macro to set the grant table max version

[Roger Pau Monne]

Such macro just clamps the passed version to fit in the designated
bits of the domctl field. The main purpose is to make it clearer in
the code when max grant version is being set in the grant_opts field.

Existing users that where setting the version in the grant_opts field
are switched to use the macro.

No functional change intended.

Requested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Ian Jackson <iwj@xenproject.org>

Given it's a non functional change (or at least that's the intention)
it shouldn't have any impact on the release, it's just syntactic
sugar.
---
 tools/helpers/init-xenstore-domain.c | 2 +-
 tools/libs/light/libxl_create.c      | 2 +-
 tools/ocaml/libs/xc/xenctrl_stubs.c  | 3 ++-
 xen/arch/arm/domain_build.c          | 4 ++--
 xen/arch/x86/setup.c                 | 2 +-
 xen/include/public/domctl.h          | 1 +
 6 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/tools/helpers/init-xenstore-domain.c b/tools/helpers/init-xenstore-domain.c
index 60469161bd..b205a79ee6 100644
--- a/tools/helpers/init-xenstore-domain.c
+++ b/tools/helpers/init-xenstore-domain.c
@@ -89,7 +89,7 @@ static int build(xc_interface *xch)
          */
         .max_grant_frames = 4,
         .max_maptrack_frames = 128,
-        .grant_opts = 1,
+        .grant_opts = XEN_DOMCTL_GRANT_version(1),
     };
 
     xs_fd = open("/dev/xen/xenbus_backend", O_RDWR);
diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
index b6855c7b46..dcd09d32ba 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -629,7 +629,7 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
             .max_evtchn_port = b_info->event_channels,
             .max_grant_frames = b_info->max_grant_frames,
             .max_maptrack_frames = b_info->max_maptrack_frames,
-            .grant_opts = b_info->max_grant_version,
+            .grant_opts = XEN_DOMCTL_GRANT_version(b_info->max_grant_version),
             .vmtrace_size = ROUNDUP(b_info->vmtrace_buf_kb << 10, XC_PAGE_SHIFT),
         };
 
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c
index eca0b8b334..5b4fe72c8d 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -199,7 +199,8 @@ CAMLprim value stub_xc_domain_create(value xch, value wanted_domid, value config
 		.max_evtchn_port = Int_val(VAL_MAX_EVTCHN_PORT),
 		.max_grant_frames = Int_val(VAL_MAX_GRANT_FRAMES),
 		.max_maptrack_frames = Int_val(VAL_MAX_MAPTRACK_FRAMES),
-		.grant_opts = Int_val(VAL_MAX_GRANT_VERSION),
+		.grant_opts =
+		    XEN_DOMCTL_GRANT_version(Int_val(VAL_MAX_GRANT_VERSION)),
 	};
 
 	domain_handle_of_uuid_string(cfg.handle, String_val(VAL_HANDLE));
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 19487c79da..d02bacbcd1 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2986,7 +2986,7 @@ void __init create_domUs(void)
             .max_evtchn_port = -1,
             .max_grant_frames = -1,
             .max_maptrack_frames = -1,
-            .grant_opts = opt_gnttab_max_version,
+            .grant_opts = XEN_DOMCTL_GRANT_version(opt_gnttab_max_version),
         };
 
         if ( !dt_device_is_compatible(node, "xen,domain") )
@@ -3094,7 +3094,7 @@ void __init create_dom0(void)
         .max_evtchn_port = -1,
         .max_grant_frames = gnttab_dom0_frames(),
         .max_maptrack_frames = -1,
-        .grant_opts = opt_gnttab_max_version,
+        .grant_opts = XEN_DOMCTL_GRANT_version(opt_gnttab_max_version),
     };
 
     /* The vGIC for DOM0 is exactly emulating the hardware GIC */
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index ec6e686fac..da47cdea14 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -755,7 +755,7 @@ static struct domain *__init create_dom0(const module_t *image,
         .max_evtchn_port = -1,
         .max_grant_frames = -1,
         .max_maptrack_frames = -1,
-        .grant_opts = opt_gnttab_max_version,
+        .grant_opts = XEN_DOMCTL_GRANT_version(opt_gnttab_max_version),
         .max_vcpus = dom0_max_vcpus(),
         .arch = {
             .misc_flags = opt_dom0_msr_relaxed ? XEN_X86_MSR_RELAXED : 0,
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 1c21d4dc75..b85e6170b0 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -99,6 +99,7 @@ struct xen_domctl_createdomain {
 
 /* Grant version, use low 4 bits. */
 #define XEN_DOMCTL_GRANT_version_mask    0xf
+#define XEN_DOMCTL_GRANT_version(v)      ((v) & XEN_DOMCTL_GRANT_version_mask)
 
     uint32_t grant_opts;
 
-- 
2.33.0

[PATCH for-4.16 2/4] tests/resource: set grant version for created domains

[Roger Pau Monne]

Set the grant table version for the created domains to use version 1,
as that's the used by the test cases. Without setting the grant
version the domains for the tests cannot be created.

Fixes: 7379f9e10a ('gnttab: allow setting max version per-domain')
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Ian Jackson <iwj@xenproject.org>

This patch only modifies a test, so it should be safe to commit as
it's not going to cause any changes to the hypervisor or the tools.
Worse that could happen is it makes the test even more broken, but
it's already unusable.
---
 tools/tests/resource/test-resource.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/tests/resource/test-resource.c b/tools/tests/resource/test-resource.c
index 988f96f7c1..0557f8a1b5 100644
--- a/tools/tests/resource/test-resource.c
+++ b/tools/tests/resource/test-resource.c
@@ -120,6 +120,7 @@ static void test_domain_configurations(void)
             .create = {
                 .max_vcpus = 2,
                 .max_grant_frames = 40,
+                .grant_opts = XEN_DOMCTL_GRANT_version(1),
             },
         },
         {
@@ -128,6 +129,7 @@ static void test_domain_configurations(void)
                 .flags = XEN_DOMCTL_CDF_hvm,
                 .max_vcpus = 2,
                 .max_grant_frames = 40,
+                .grant_opts = XEN_DOMCTL_GRANT_version(1),
                 .arch = {
                     .emulation_flags = XEN_X86_EMU_LAPIC,
                 },
@@ -140,6 +142,7 @@ static void test_domain_configurations(void)
                 .flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap,
                 .max_vcpus = 2,
                 .max_grant_frames = 40,
+                .grant_opts = XEN_DOMCTL_GRANT_version(1),
             },
         },
 #endif
-- 
2.33.0

[PATCH for-4.16 3/4] test/tsx: set grant version for created domains

[Roger Pau Monne]

Set the grant table version for the created domains to use version 1,
as such tests domains don't require the usage of the grant table at
all. A TODO note is added to switch those dummy domains to not have a
grant table at all when possible. Without setting the grant version
the domains for the tests cannot be created.

Fixes: 7379f9e10a ('gnttab: allow setting max version per-domain')
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reported-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Ian Jackson <iwj@xenproject.org>

This patch only modifies a test, so it should be safe to commit as
it's not going to cause any changes to the hypervisor or the tools.
Worse that could happen is it makes the test even more broken, but
it's already unusable.
---
 tools/tests/tsx/test-tsx.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/tests/tsx/test-tsx.c b/tools/tests/tsx/test-tsx.c
index fab99c135e..f1dcff4c30 100644
--- a/tools/tests/tsx/test-tsx.c
+++ b/tools/tests/tsx/test-tsx.c
@@ -444,6 +444,8 @@ static void test_guests(void)
         struct xen_domctl_createdomain c = {
             .max_vcpus = 1,
             .max_grant_frames = 1,
+            /* TODO: switch to 0 once support for no grant table is added. */
+            .grant_opts = XEN_DOMCTL_GRANT_version(1),
         };
 
         printf("Testing PV guest\n");
@@ -456,6 +458,8 @@ static void test_guests(void)
             .flags = XEN_DOMCTL_CDF_hvm,
             .max_vcpus = 1,
             .max_grant_frames = 1,
+            /* TODO: switch to 0 once support for no grant table is added. */
+            .grant_opts = XEN_DOMCTL_GRANT_version(1),
             .arch = {
                 .emulation_flags = XEN_X86_EMU_LAPIC,
             },
-- 
2.33.0

[PATCH for-4.16 4/4] tools/python: fix python libxc bindings to pass a max grant version

[Roger Pau Monne]

Such max version should be provided by the caller, otherwise the
bindings will default to specifying a max version of 2, which is
inline with the current defaults in the hypervisor.

Fixes: 7379f9e10a ('gnttab: allow setting max version per-domain')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Ian Jackson <iwj@xenproject.org>

Without this fix the python bindings won't be usable, as they will
attempt to pass a max version of 0 which will be refused by the
hypervisor. The main risk would be breaking the bindings, but given
the current state it's unlikely to result in a worse outcome.
---
 tools/python/xen/lowlevel/xc/xc.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
index 6293d205a3..fd00861032 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -116,7 +116,7 @@ static PyObject *pyxc_domain_create(XcObject *self,
                                     PyObject *args,
                                     PyObject *kwds)
 {
-    uint32_t dom = 0, target = 0;
+    uint32_t dom = 0, target = 0, max_grant_version = 2;
     int      ret;
     size_t   i;
     PyObject *pyhandle = NULL;
@@ -132,12 +132,13 @@ static PyObject *pyxc_domain_create(XcObject *self,
     };
 
     static char *kwd_list[] = { "domid", "ssidref", "handle", "flags",
-                                "target", "max_vcpus", NULL };
+                                "target", "max_vcpus", "max_grant_version",
+                                NULL };
 
-    if ( !PyArg_ParseTupleAndKeywords(args, kwds, "|iiOiii", kwd_list,
+    if ( !PyArg_ParseTupleAndKeywords(args, kwds, "|iiOiiii", kwd_list,
                                       &dom, &config.ssidref, &pyhandle,
                                       &config.flags, &target,
-                                      &config.max_vcpus) )
+                                      &config.max_vcpus, &max_grant_version) )
         return NULL;
     if ( pyhandle != NULL )
     {
@@ -162,6 +163,7 @@ static PyObject *pyxc_domain_create(XcObject *self,
 #else
 #error Architecture not supported
 #endif
+    config.grant_opts = XEN_DOMCTL_GRANT_version(max_grant_version);
 
     if ( (ret = xc_domain_create(self->xc_handle, &dom, &config)) < 0 )
         return pyxc_error_to_exception(self->xc_handle);
-- 
2.33.0

Re: [PATCH for-4.16 1/4] domctl: introduce a macro to set the grant table max version

[Jan Beulich]

On 15.11.2021 13:17, Roger Pau Monne wrote:
> Such macro just clamps the passed version to fit in the designated
> bits of the domctl field. The main purpose is to make it clearer in
> the code when max grant version is being set in the grant_opts field.
> 
> Existing users that where setting the version in the grant_opts field
> are switched to use the macro.
> 
> No functional change intended.
> 
> Requested-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH for-4.16 2/4] tests/resource: set grant version for created domains

[Jan Beulich]

On 15.11.2021 13:17, Roger Pau Monne wrote:
> Set the grant table version for the created domains to use version 1,
> as that's the used by the test cases. Without setting the grant
> version the domains for the tests cannot be created.
> 
> Fixes: 7379f9e10a ('gnttab: allow setting max version per-domain')
> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH for-4.16 3/4] test/tsx: set grant version for created domains

[Jan Beulich]

On 15.11.2021 13:17, Roger Pau Monne wrote:
> Set the grant table version for the created domains to use version 1,
> as such tests domains don't require the usage of the grant table at
> all. A TODO note is added to switch those dummy domains to not have a
> grant table at all when possible. Without setting the grant version
> the domains for the tests cannot be created.
> 
> Fixes: 7379f9e10a ('gnttab: allow setting max version per-domain')
> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reported-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH for-4.16 1/4] domctl: introduce a macro to set the grant table max version

[Andrew Cooper]

On 15/11/2021 12:17, Roger Pau Monne wrote:
> Such macro just clamps the passed version to fit in the designated
> bits of the domctl field. The main purpose is to make it clearer in
> the code when max grant version is being set in the grant_opts field.
>
> Existing users that where setting the version in the grant_opts field
> are switched to use the macro.
>
> No functional change intended.
>
> Requested-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

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

Re: [PATCH for-4.16 2/4] tests/resource: set grant version for created domains

[Andrew Cooper]

On 15/11/2021 12:17, Roger Pau Monne wrote:
> Set the grant table version for the created domains to use version 1,
> as that's the used by the test cases. Without setting the grant
> version the domains for the tests cannot be created.
>
> Fixes: 7379f9e10a ('gnttab: allow setting max version per-domain')
> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

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

Re: [PATCH for-4.16 3/4] test/tsx: set grant version for created domains

[Andrew Cooper]

On 15/11/2021 12:17, Roger Pau Monne wrote:
> Set the grant table version for the created domains to use version 1,
> as such tests domains don't require the usage of the grant table at
> all. A TODO note is added to switch those dummy domains to not have a
> grant table at all when possible. Without setting the grant version
> the domains for the tests cannot be created.
>
> Fixes: 7379f9e10a ('gnttab: allow setting max version per-domain')
> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reported-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Cc: Ian Jackson <iwj@xenproject.org>
>
> This patch only modifies a test, so it should be safe to commit as
> it's not going to cause any changes to the hypervisor or the tools.
> Worse that could happen is it makes the test even more broken, but
> it's already unusable.

What do you mean unusable?  Other than this, the test works.

> ---
>  tools/tests/tsx/test-tsx.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/tools/tests/tsx/test-tsx.c b/tools/tests/tsx/test-tsx.c
> index fab99c135e..f1dcff4c30 100644
> --- a/tools/tests/tsx/test-tsx.c
> +++ b/tools/tests/tsx/test-tsx.c
> @@ -444,6 +444,8 @@ static void test_guests(void)
>          struct xen_domctl_createdomain c = {
>              .max_vcpus = 1,
>              .max_grant_frames = 1,
> +            /* TODO: switch to 0 once support for no grant table is added. */

I'd avoid these TODOs.  It's test code, so really doesn't matter too much.

However, there is a further task for this test to actually boot enough
of a guest to dump CPUID as visible inside the guest, and cross-check
with the toolstack's view of the data.

How exactly to get that data out of the VM is an open question, but it
might involve xenconsoled.

~Andrew

Re: [PATCH for-4.16 4/4] tools/python: fix python libxc bindings to pass a max grant version

[Andrew Cooper]

On 15/11/2021 12:17, Roger Pau Monne wrote:
> Such max version should be provided by the caller, otherwise the
> bindings will default to specifying a max version of 2, which is
> inline with the current defaults in the hypervisor.
>
> Fixes: 7379f9e10a ('gnttab: allow setting max version per-domain')
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

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

Re: [PATCH for-4.16 1/4] domctl: introduce a macro to set the grant table max version

[Bertrand Marquis]

Hi Roger,

> On 15 Nov 2021, at 12:17, Roger Pau Monne <roger.pau@citrix.com> wrote:
> 
> Such macro just clamps the passed version to fit in the designated
> bits of the domctl field. The main purpose is to make it clearer in
> the code when max grant version is being set in the grant_opts field.
> 
> Existing users that where setting the version in the grant_opts field
> are switched to use the macro.
> 
> No functional change intended.
> 
> Requested-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand

> ---
> Cc: Ian Jackson <iwj@xenproject.org>
> 
> Given it's a non functional change (or at least that's the intention)
> it shouldn't have any impact on the release, it's just syntactic
> sugar.
> ---
> tools/helpers/init-xenstore-domain.c | 2 +-
> tools/libs/light/libxl_create.c      | 2 +-
> tools/ocaml/libs/xc/xenctrl_stubs.c  | 3 ++-
> xen/arch/arm/domain_build.c          | 4 ++--
> xen/arch/x86/setup.c                 | 2 +-
> xen/include/public/domctl.h          | 1 +
> 6 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/helpers/init-xenstore-domain.c b/tools/helpers/init-xenstore-domain.c
> index 60469161bd..b205a79ee6 100644
> --- a/tools/helpers/init-xenstore-domain.c
> +++ b/tools/helpers/init-xenstore-domain.c
> @@ -89,7 +89,7 @@ static int build(xc_interface *xch)
>          */
>         .max_grant_frames = 4,
>         .max_maptrack_frames = 128,
> -        .grant_opts = 1,
> +        .grant_opts = XEN_DOMCTL_GRANT_version(1),
>     };
> 
>     xs_fd = open("/dev/xen/xenbus_backend", O_RDWR);
> diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
> index b6855c7b46..dcd09d32ba 100644
> --- a/tools/libs/light/libxl_create.c
> +++ b/tools/libs/light/libxl_create.c
> @@ -629,7 +629,7 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
>             .max_evtchn_port = b_info->event_channels,
>             .max_grant_frames = b_info->max_grant_frames,
>             .max_maptrack_frames = b_info->max_maptrack_frames,
> -            .grant_opts = b_info->max_grant_version,
> +            .grant_opts = XEN_DOMCTL_GRANT_version(b_info->max_grant_version),
>             .vmtrace_size = ROUNDUP(b_info->vmtrace_buf_kb << 10, XC_PAGE_SHIFT),
>         };
> 
> diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c
> index eca0b8b334..5b4fe72c8d 100644
> --- a/tools/ocaml/libs/xc/xenctrl_stubs.c
> +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
> @@ -199,7 +199,8 @@ CAMLprim value stub_xc_domain_create(value xch, value wanted_domid, value config
> 		.max_evtchn_port = Int_val(VAL_MAX_EVTCHN_PORT),
> 		.max_grant_frames = Int_val(VAL_MAX_GRANT_FRAMES),
> 		.max_maptrack_frames = Int_val(VAL_MAX_MAPTRACK_FRAMES),
> -		.grant_opts = Int_val(VAL_MAX_GRANT_VERSION),
> +		.grant_opts =
> +		    XEN_DOMCTL_GRANT_version(Int_val(VAL_MAX_GRANT_VERSION)),
> 	};
> 
> 	domain_handle_of_uuid_string(cfg.handle, String_val(VAL_HANDLE));
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 19487c79da..d02bacbcd1 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2986,7 +2986,7 @@ void __init create_domUs(void)
>             .max_evtchn_port = -1,
>             .max_grant_frames = -1,
>             .max_maptrack_frames = -1,
> -            .grant_opts = opt_gnttab_max_version,
> +            .grant_opts = XEN_DOMCTL_GRANT_version(opt_gnttab_max_version),
>         };
> 
>         if ( !dt_device_is_compatible(node, "xen,domain") )
> @@ -3094,7 +3094,7 @@ void __init create_dom0(void)
>         .max_evtchn_port = -1,
>         .max_grant_frames = gnttab_dom0_frames(),
>         .max_maptrack_frames = -1,
> -        .grant_opts = opt_gnttab_max_version,
> +        .grant_opts = XEN_DOMCTL_GRANT_version(opt_gnttab_max_version),
>     };
> 
>     /* The vGIC for DOM0 is exactly emulating the hardware GIC */
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index ec6e686fac..da47cdea14 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -755,7 +755,7 @@ static struct domain *__init create_dom0(const module_t *image,
>         .max_evtchn_port = -1,
>         .max_grant_frames = -1,
>         .max_maptrack_frames = -1,
> -        .grant_opts = opt_gnttab_max_version,
> +        .grant_opts = XEN_DOMCTL_GRANT_version(opt_gnttab_max_version),
>         .max_vcpus = dom0_max_vcpus(),
>         .arch = {
>             .misc_flags = opt_dom0_msr_relaxed ? XEN_X86_MSR_RELAXED : 0,
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index 1c21d4dc75..b85e6170b0 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -99,6 +99,7 @@ struct xen_domctl_createdomain {
> 
> /* Grant version, use low 4 bits. */
> #define XEN_DOMCTL_GRANT_version_mask    0xf
> +#define XEN_DOMCTL_GRANT_version(v)      ((v) & XEN_DOMCTL_GRANT_version_mask)
> 
>     uint32_t grant_opts;
> 
> -- 
> 2.33.0
> 

Re: [PATCH for-4.16 3/4] test/tsx: set grant version for created domains

[Roger Pau Monné]

On Mon, Nov 15, 2021 at 12:43:34PM +0000, Andrew Cooper wrote:
> On 15/11/2021 12:17, Roger Pau Monne wrote:
> > Set the grant table version for the created domains to use version 1,
> > as such tests domains don't require the usage of the grant table at
> > all. A TODO note is added to switch those dummy domains to not have a
> > grant table at all when possible. Without setting the grant version
> > the domains for the tests cannot be created.
> >
> > Fixes: 7379f9e10a ('gnttab: allow setting max version per-domain')
> > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > Reported-by: Jan Beulich <jbeulich@suse.com>
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > ---
> > Cc: Ian Jackson <iwj@xenproject.org>
> >
> > This patch only modifies a test, so it should be safe to commit as
> > it's not going to cause any changes to the hypervisor or the tools.
> > Worse that could happen is it makes the test even more broken, but
> > it's already unusable.
> 
> What do you mean unusable?  Other than this, the test works.

I mean, it's unusable because I broke it with the gnttab change, and
that's it's current status unless this patch is applied.

> > ---
> >  tools/tests/tsx/test-tsx.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/tools/tests/tsx/test-tsx.c b/tools/tests/tsx/test-tsx.c
> > index fab99c135e..f1dcff4c30 100644
> > --- a/tools/tests/tsx/test-tsx.c
> > +++ b/tools/tests/tsx/test-tsx.c
> > @@ -444,6 +444,8 @@ static void test_guests(void)
> >          struct xen_domctl_createdomain c = {
> >              .max_vcpus = 1,
> >              .max_grant_frames = 1,
> > +            /* TODO: switch to 0 once support for no grant table is added. */
> 
> I'd avoid these TODOs.  It's test code, so really doesn't matter too much.

I'm fine with this, when looking at the test code I didn't see any
reason why gnttab was needed, so I thought it would be a fine use-case
for opting out to grant table. As you say it's a test case, so it
doesn't matter much.

Do you want me to repost with the comments removed?

Thanks, Roger.

Re: [PATCH for-4.16 1/4] domctl: introduce a macro to set the grant table max version

[Julien Grall]

Hi Roger,

On 15/11/2021 12:17, Roger Pau Monne wrote:
> Such macro just clamps the passed version to fit in the designated
> bits of the domctl field. The main purpose is to make it clearer in
> the code when max grant version is being set in the grant_opts field.
> 
> Existing users that where setting the version in the grant_opts field
> are switched to use the macro.
> 
> No functional change intended.
> 
> Requested-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Acked-by: Julien Grall <jgrall@amazon.com>

Cheers,

-- 
Julien Grall

Re: [PATCH for-4.16 3/4] test/tsx: set grant version for created domains

[Andrew Cooper]

On 15/11/2021 13:58, Roger Pau Monné wrote:
> On Mon, Nov 15, 2021 at 12:43:34PM +0000, Andrew Cooper wrote:
>> On 15/11/2021 12:17, Roger Pau Monne wrote:
>>> Set the grant table version for the created domains to use version 1,
>>> as such tests domains don't require the usage of the grant table at
>>> all. A TODO note is added to switch those dummy domains to not have a
>>> grant table at all when possible. Without setting the grant version
>>> the domains for the tests cannot be created.
>>>
>>> Fixes: 7379f9e10a ('gnttab: allow setting max version per-domain')
>>> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> Reported-by: Jan Beulich <jbeulich@suse.com>
>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>> ---
>>> Cc: Ian Jackson <iwj@xenproject.org>
>>>
>>> This patch only modifies a test, so it should be safe to commit as
>>> it's not going to cause any changes to the hypervisor or the tools.
>>> Worse that could happen is it makes the test even more broken, but
>>> it's already unusable.
>> What do you mean unusable?  Other than this, the test works.
> I mean, it's unusable because I broke it with the gnttab change, and
> that's it's current status unless this patch is applied.
>
>>> ---
>>>  tools/tests/tsx/test-tsx.c | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/tools/tests/tsx/test-tsx.c b/tools/tests/tsx/test-tsx.c
>>> index fab99c135e..f1dcff4c30 100644
>>> --- a/tools/tests/tsx/test-tsx.c
>>> +++ b/tools/tests/tsx/test-tsx.c
>>> @@ -444,6 +444,8 @@ static void test_guests(void)
>>>          struct xen_domctl_createdomain c = {
>>>              .max_vcpus = 1,
>>>              .max_grant_frames = 1,
>>> +            /* TODO: switch to 0 once support for no grant table is added. */
>> I'd avoid these TODOs.  It's test code, so really doesn't matter too much.
> I'm fine with this, when looking at the test code I didn't see any
> reason why gnttab was needed, so I thought it would be a fine use-case
> for opting out to grant table. As you say it's a test case, so it
> doesn't matter much.
>
> Do you want me to repost with the comments removed?

I'm happy to fix on commit, if nothing else needs reposting.

~Andrew

Re: [PATCH for-4.16 4/4] tools/python: fix python libxc bindings to pass a max grant version

[Ian Jackson]

Roger Pau Monne writes ("[PATCH for-4.16 4/4] tools/python: fix python libxc bindings to pass a max grant version"):
> Such max version should be provided by the caller, otherwise the
> bindings will default to specifying a max version of 2, which is
> inline with the current defaults in the hypervisor.
> 
> Fixes: 7379f9e10a ('gnttab: allow setting max version per-domain')
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Cc: Ian Jackson <iwj@xenproject.org>
> 
> Without this fix the python bindings won't be usable, as they will
> attempt to pass a max version of 0 which will be refused by the
> hypervisor. The main risk would be breaking the bindings, but given
> the current state it's unlikely to result in a worse outcome.

Release-Acked-by: Ian Jackson <iwj@xenproject.org>

Re: [PATCH for-4.16 1/4] domctl: introduce a macro to set the grant table max version

[Ian Jackson]

Julien Grall writes ("Re: [PATCH for-4.16 1/4] domctl: introduce a macro to set the grant table max version"):
> On 15/11/2021 12:17, Roger Pau Monne wrote:
> > Such macro just clamps the passed version to fit in the designated
> > bits of the domctl field. The main purpose is to make it clearer in
> > the code when max grant version is being set in the grant_opts field.
> > 
> > Existing users that where setting the version in the grant_opts field
> > are switched to use the macro.
> > 
> > No functional change intended.
> > 
> > Requested-by: Jan Beulich <jbeulich@suse.com>
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> Acked-by: Julien Grall <jgrall@amazon.com>

Reviewed-by: Ian Jackson <iwj@xenproject.org>
Release-Acked-by: Ian Jackson <iwj@xenproject.org>

Re: [PATCH for-4.16 3/4] test/tsx: set grant version for created domains

[Ian Jackson]

Roger Pau Monne writes ("[PATCH for-4.16 3/4] test/tsx: set grant version for created domains"):
> Set the grant table version for the created domains to use version 1,
> as such tests domains don't require the usage of the grant table at
> all. A TODO note is added to switch those dummy domains to not have a
> grant table at all when possible. Without setting the grant version
> the domains for the tests cannot be created.
> 
> Fixes: 7379f9e10a ('gnttab: allow setting max version per-domain')
> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reported-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Release-Acked-by: Ian Jackson <iwj@xenproject.org>

Re: [PATCH for-4.16 0/4] gnttab: fallout/improvements from max version

[Jan Beulich]

On 15.11.2021 13:17, Roger Pau Monne wrote:
> Hello,
> 
> The first patch introduces some syntactic sugar for setting the max
> grant version on the domctl, while the later 3 patches fix missing
> occurrences of xen_domctl_createdomain that didn't set the max grant
> version. It's likely patches 2 and 3 could be squashed.
> 
> All should be considered for the release, or else we need to revert the
> grant table version per domain change.
> 
> Thanks, Roger.
> 
> Roger Pau Monne (4):
>   domctl: introduce a macro to set the grant table max version
>   tests/resource: set grant version for created domains
>   test/tsx: set grant version for created domains

I've committed these, but ...

>   tools/python: fix python libxc bindings to pass a max grant version

... this one still lacks a maintainer ack (ignoring the slightly malformed
shape of the ./MAINTAINERS entry, which imo ought to refer to tools/python/
to designate that's a directory).

Jan

Re: [PATCH for-4.16 4/4] tools/python: fix python libxc bindings to pass a max grant version

[Ian Jackson]

Andrew Cooper writes ("Re: [PATCH for-4.16 4/4] tools/python: fix python libxc bindings to pass a max grant version"):
> On 15/11/2021 12:17, Roger Pau Monne wrote:
> > Such max version should be provided by the caller, otherwise the
> > bindings will default to specifying a max version of 2, which is
> > inline with the current defaults in the hypervisor.
> >
> > Fixes: 7379f9e10a ('gnttab: allow setting max version per-domain')
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

FTR

Acked-by: Ian Jackson <iwj@xenproject.org>

Re: [PATCH for-4.16 4/4] tools/python: fix python libxc bindings to pass a max grant version

[Marek Marczykowski-Górecki]

[-- Attachment #1: Type: text/plain, Size: 2800 bytes --]

On Mon, Nov 15, 2021 at 01:17:41PM +0100, Roger Pau Monne wrote:
> Such max version should be provided by the caller, otherwise the
> bindings will default to specifying a max version of 2, which is
> inline with the current defaults in the hypervisor.
> 
> Fixes: 7379f9e10a ('gnttab: allow setting max version per-domain')
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

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

> ---
> Cc: Ian Jackson <iwj@xenproject.org>
> 
> Without this fix the python bindings won't be usable, as they will
> attempt to pass a max version of 0 which will be refused by the
> hypervisor. The main risk would be breaking the bindings, but given
> the current state it's unlikely to result in a worse outcome.
> ---
>  tools/python/xen/lowlevel/xc/xc.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
> index 6293d205a3..fd00861032 100644
> --- a/tools/python/xen/lowlevel/xc/xc.c
> +++ b/tools/python/xen/lowlevel/xc/xc.c
> @@ -116,7 +116,7 @@ static PyObject *pyxc_domain_create(XcObject *self,
>                                      PyObject *args,
>                                      PyObject *kwds)
>  {
> -    uint32_t dom = 0, target = 0;
> +    uint32_t dom = 0, target = 0, max_grant_version = 2;
>      int      ret;
>      size_t   i;
>      PyObject *pyhandle = NULL;
> @@ -132,12 +132,13 @@ static PyObject *pyxc_domain_create(XcObject *self,
>      };
>  
>      static char *kwd_list[] = { "domid", "ssidref", "handle", "flags",
> -                                "target", "max_vcpus", NULL };
> +                                "target", "max_vcpus", "max_grant_version",
> +                                NULL };
>  
> -    if ( !PyArg_ParseTupleAndKeywords(args, kwds, "|iiOiii", kwd_list,
> +    if ( !PyArg_ParseTupleAndKeywords(args, kwds, "|iiOiiii", kwd_list,
>                                        &dom, &config.ssidref, &pyhandle,
>                                        &config.flags, &target,
> -                                      &config.max_vcpus) )
> +                                      &config.max_vcpus, &max_grant_version) )
>          return NULL;
>      if ( pyhandle != NULL )
>      {
> @@ -162,6 +163,7 @@ static PyObject *pyxc_domain_create(XcObject *self,
>  #else
>  #error Architecture not supported
>  #endif
> +    config.grant_opts = XEN_DOMCTL_GRANT_version(max_grant_version);
>  
>      if ( (ret = xc_domain_create(self->xc_handle, &dom, &config)) < 0 )
>          return pyxc_error_to_exception(self->xc_handle);
> -- 
> 2.33.0
> 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

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