[Xen-devel] [PATCH v2 0/2] libxl: choose a sane default for HAP

[Xen-devel] [PATCH v2 0/2] libxl: choose a sane default for HAP

[Roger Pau Monne]

Hello,

First patch is a preparatory change to also make use of the physcaps on
ARM, second patch introduces a new physcap (HAP) in order for the
toolstack to decide whether to use HAP if the user hasn't made a
selection.

The series can also be found at:

git://xenbits.xen.org/people/royger/xen.git hap_info_v2

Thanks, Roger.

Roger Pau Monne (2):
  sysctl: report existing physcaps on ARM
  sysctl/libxl: choose a sane default for HAP

 tools/libxl/libxl.c         |  1 +
 tools/libxl/libxl_create.c  |  8 +++++++-
 tools/libxl/libxl_types.idl |  1 +
 tools/xl/xl_info.c          |  5 +++--
 xen/arch/arm/sysctl.c       |  5 ++++-
 xen/arch/x86/sysctl.c       |  4 ++--
 xen/common/sysctl.c         |  2 ++
 xen/include/public/sysctl.h | 10 +++++++---
 8 files changed, 27 insertions(+), 9 deletions(-)

-- 
2.22.0


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

[Xen-devel] [PATCH v2 1/2] sysctl: report existing physcaps on ARM

[Roger Pau Monne]

Current physcaps in XEN_SYSCTL_physinfo are only used by x86, albeit
the capabilities themselves are not x86 specific.

This patch adds support for also reporting the current capabilities on
ARM hardware. Note that on ARM PHYSCAP_hvm is always reported, and
setting PHYSCAP_directio has been moved to common code since the same
logic to set it is used by x86 and ARM.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Changes since v1:
 - New in this version.
---
 xen/arch/arm/sysctl.c       | 5 ++++-
 xen/arch/x86/sysctl.c       | 2 --
 xen/common/sysctl.c         | 2 ++
 xen/include/public/sysctl.h | 6 +++---
 4 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/sysctl.c b/xen/arch/arm/sysctl.c
index fbfdb44eff..92ac99c928 100644
--- a/xen/arch/arm/sysctl.c
+++ b/xen/arch/arm/sysctl.c
@@ -12,7 +12,10 @@
 #include <xen/hypercall.h>
 #include <public/sysctl.h>
 
-void arch_do_physinfo(struct xen_sysctl_physinfo *pi) { }
+void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
+{
+    pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm;
+}
 
 long arch_do_sysctl(struct xen_sysctl *sysctl,
                     XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
index c50d910a1c..7ec6174e6b 100644
--- a/xen/arch/x86/sysctl.c
+++ b/xen/arch/x86/sysctl.c
@@ -163,8 +163,6 @@ void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
         pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm;
     if ( IS_ENABLED(CONFIG_PV) )
         pi->capabilities |= XEN_SYSCTL_PHYSCAP_pv;
-    if ( iommu_enabled )
-        pi->capabilities |= XEN_SYSCTL_PHYSCAP_directio;
 }
 
 long arch_do_sysctl(
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index fcf2d2fd7c..92b4ea0d21 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -267,6 +267,8 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
         pi->cpu_khz = cpu_khz;
         pi->max_mfn = get_upper_mfn_bound();
         arch_do_physinfo(pi);
+        if ( iommu_enabled )
+            pi->capabilities |= XEN_SYSCTL_PHYSCAP_directio;
 
         if ( copy_to_guest(u_sysctl, op, 1) )
             ret = -EFAULT;
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 91c48dcae0..36b3f8c429 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -81,13 +81,13 @@ struct xen_sysctl_tbuf_op {
  * Get physical information about the host machine
  */
 /* XEN_SYSCTL_physinfo */
- /* (x86) The platform supports HVM guests. */
+ /* The platform supports HVM guests. */
 #define _XEN_SYSCTL_PHYSCAP_hvm          0
 #define XEN_SYSCTL_PHYSCAP_hvm           (1u<<_XEN_SYSCTL_PHYSCAP_hvm)
- /* (x86) The platform supports PV guests. */
+ /* The platform supports PV guests. */
 #define _XEN_SYSCTL_PHYSCAP_pv           1
 #define XEN_SYSCTL_PHYSCAP_pv            (1u<<_XEN_SYSCTL_PHYSCAP_pv)
- /* (x86) The platform supports direct access to I/O devices with IOMMU. */
+ /* The platform supports direct access to I/O devices with IOMMU. */
 #define _XEN_SYSCTL_PHYSCAP_directio     2
 #define XEN_SYSCTL_PHYSCAP_directio  (1u<<_XEN_SYSCTL_PHYSCAP_directio)
 struct xen_sysctl_physinfo {
-- 
2.22.0


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

[Xen-devel] [PATCH v2 2/2] sysctl/libxl: choose a sane default for HAP

[Roger Pau Monne]

Current libxl code will always enable Hardware Assisted Paging (HAP),
expecting that the hypervisor will fallback to shadow if HAP is not
available. With the changes to the domain builder that's not the case
any longer, and the hypervisor will raise an error if HAP is not
available instead of silently falling back to shadow.

In order to keep the previous functionality report whether HAP is
available or not in XEN_SYSCTL_physinfo, so that the toolstack can
select a sane default if there's no explicit user selection of whether
HAP should be used.

Note that on ARM hardware HAP capability is always reported since it's
a required feature in order to run Xen.

Fixes: d0c0ba7d3de ('x86/hvm/domain: remove the 'hap_enabled' flag')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Paul Durrant <Paul.Durrant@citrix.com>
---
Changes since v1:
 - Also report HAP capability for ARM.
 - Print hap capability in xl info.
---
 tools/libxl/libxl.c         | 1 +
 tools/libxl/libxl_create.c  | 8 +++++++-
 tools/libxl/libxl_types.idl | 1 +
 tools/xl/xl_info.c          | 5 +++--
 xen/arch/arm/sysctl.c       | 2 +-
 xen/arch/x86/sysctl.c       | 2 ++
 xen/include/public/sysctl.h | 4 ++++
 7 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index ec71574e99..5c0fcf320e 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -399,6 +399,7 @@ int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo *physinfo)
     physinfo->cap_pv = !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_pv);
     physinfo->cap_hvm_directio =
         !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_directio);
+    physinfo->cap_hap = !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_hap);
 
     GC_FREE;
     return 0;
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 03ce166f4f..6a556dea8f 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -38,7 +38,13 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc,
     libxl__arch_domain_create_info_setdefault(gc, c_info);
 
     if (c_info->type != LIBXL_DOMAIN_TYPE_PV) {
-        libxl_defbool_setdefault(&c_info->hap, true);
+        libxl_physinfo info;
+        int rc = libxl_get_physinfo(CTX, &info);
+
+        if (rc)
+            return rc;
+
+        libxl_defbool_setdefault(&c_info->hap, info.cap_hap);
         libxl_defbool_setdefault(&c_info->oos, true);
     }
 
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index b61399ce36..9e1f8515d3 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -1025,6 +1025,7 @@ libxl_physinfo = Struct("physinfo", [
     ("cap_hvm", bool),
     ("cap_pv", bool),
     ("cap_hvm_directio", bool), # No longer HVM specific
+    ("cap_hap", bool),
     ], dir=DIR_OUT)
 
 libxl_connectorinfo = Struct("connectorinfo", [
diff --git a/tools/xl/xl_info.c b/tools/xl/xl_info.c
index 46d9c9f712..aa6724bc7f 100644
--- a/tools/xl/xl_info.c
+++ b/tools/xl/xl_info.c
@@ -210,11 +210,12 @@ static void output_physinfo(void)
          info.hw_cap[4], info.hw_cap[5], info.hw_cap[6], info.hw_cap[7]
         );
 
-    maybe_printf("virt_caps              :%s%s%s%s\n",
+    maybe_printf("virt_caps              :%s%s%s%s%s\n",
          info.cap_pv ? " pv" : "",
          info.cap_hvm ? " hvm" : "",
          info.cap_hvm && info.cap_hvm_directio ? " hvm_directio" : "",
-         info.cap_pv && info.cap_hvm_directio ? " pv_directio" : ""
+         info.cap_pv && info.cap_hvm_directio ? " pv_directio" : "",
+         info.cap_hap ? " hap" : ""
         );
 
     vinfo = libxl_get_version_info(ctx);
diff --git a/xen/arch/arm/sysctl.c b/xen/arch/arm/sysctl.c
index 92ac99c928..f87944e847 100644
--- a/xen/arch/arm/sysctl.c
+++ b/xen/arch/arm/sysctl.c
@@ -14,7 +14,7 @@
 
 void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
 {
-    pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm;
+    pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm | XEN_SYSCTL_PHYSCAP_hap;
 }
 
 long arch_do_sysctl(struct xen_sysctl *sysctl,
diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
index 7ec6174e6b..5777a05ffc 100644
--- a/xen/arch/x86/sysctl.c
+++ b/xen/arch/x86/sysctl.c
@@ -163,6 +163,8 @@ void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
         pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm;
     if ( IS_ENABLED(CONFIG_PV) )
         pi->capabilities |= XEN_SYSCTL_PHYSCAP_pv;
+    if ( hvm_hap_supported() )
+        pi->capabilities |= XEN_SYSCTL_PHYSCAP_hap;
 }
 
 long arch_do_sysctl(
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 36b3f8c429..e02d7ce4c6 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -90,6 +90,10 @@ struct xen_sysctl_tbuf_op {
  /* The platform supports direct access to I/O devices with IOMMU. */
 #define _XEN_SYSCTL_PHYSCAP_directio     2
 #define XEN_SYSCTL_PHYSCAP_directio  (1u<<_XEN_SYSCTL_PHYSCAP_directio)
+/* The platform supports Hardware Assisted Paging. */
+#define _XEN_SYSCTL_PHYSCAP_hap          3
+#define XEN_SYSCTL_PHYSCAP_hap           (1u<<_XEN_SYSCTL_PHYSCAP_hap)
+
 struct xen_sysctl_physinfo {
     uint32_t threads_per_core;
     uint32_t cores_per_socket;
-- 
2.22.0


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

Re: [Xen-devel] [PATCH v2 2/2] sysctl/libxl: choose a sane default for HAP

[Paul Durrant]

> -----Original Message-----
> From: Roger Pau Monne <roger.pau@citrix.com>
> Sent: 05 September 2019 14:27
> To: xen-devel@lists.xenproject.org
> Cc: Roger Pau Monne <roger.pau@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Wei Liu
> <wl@xen.org>; Anthony Perard <anthony.perard@citrix.com>; Andrew Cooper <Andrew.Cooper3@citrix.com>;
> George Dunlap <George.Dunlap@citrix.com>; Jan Beulich <jbeulich@suse.com>; Julien Grall
> <julien.grall@arm.com>; Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; Stefano Stabellini
> <sstabellini@kernel.org>; Tim (Xen.org) <tim@xen.org>; Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>;
> Paul Durrant <Paul.Durrant@citrix.com>
> Subject: [PATCH v2 2/2] sysctl/libxl: choose a sane default for HAP
> 
> Current libxl code will always enable Hardware Assisted Paging (HAP),
> expecting that the hypervisor will fallback to shadow if HAP is not
> available. With the changes to the domain builder that's not the case
> any longer, and the hypervisor will raise an error if HAP is not
> available instead of silently falling back to shadow.
> 
> In order to keep the previous functionality report whether HAP is
> available or not in XEN_SYSCTL_physinfo, so that the toolstack can
> select a sane default if there's no explicit user selection of whether
> HAP should be used.
> 
> Note that on ARM hardware HAP capability is always reported since it's
> a required feature in order to run Xen.
> 
> Fixes: d0c0ba7d3de ('x86/hvm/domain: remove the 'hap_enabled' flag')
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

LGTM

Reviewed-by: Paul Durrant <paul.durrant@citrix.com>

> ---
> Cc: Paul Durrant <Paul.Durrant@citrix.com>
> ---
> Changes since v1:
>  - Also report HAP capability for ARM.
>  - Print hap capability in xl info.
> ---
>  tools/libxl/libxl.c         | 1 +
>  tools/libxl/libxl_create.c  | 8 +++++++-
>  tools/libxl/libxl_types.idl | 1 +
>  tools/xl/xl_info.c          | 5 +++--
>  xen/arch/arm/sysctl.c       | 2 +-
>  xen/arch/x86/sysctl.c       | 2 ++
>  xen/include/public/sysctl.h | 4 ++++
>  7 files changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index ec71574e99..5c0fcf320e 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -399,6 +399,7 @@ int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo *physinfo)
>      physinfo->cap_pv = !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_pv);
>      physinfo->cap_hvm_directio =
>          !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_directio);
> +    physinfo->cap_hap = !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_hap);
> 
>      GC_FREE;
>      return 0;
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 03ce166f4f..6a556dea8f 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -38,7 +38,13 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc,
>      libxl__arch_domain_create_info_setdefault(gc, c_info);
> 
>      if (c_info->type != LIBXL_DOMAIN_TYPE_PV) {
> -        libxl_defbool_setdefault(&c_info->hap, true);
> +        libxl_physinfo info;
> +        int rc = libxl_get_physinfo(CTX, &info);
> +
> +        if (rc)
> +            return rc;
> +
> +        libxl_defbool_setdefault(&c_info->hap, info.cap_hap);
>          libxl_defbool_setdefault(&c_info->oos, true);
>      }
> 
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index b61399ce36..9e1f8515d3 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -1025,6 +1025,7 @@ libxl_physinfo = Struct("physinfo", [
>      ("cap_hvm", bool),
>      ("cap_pv", bool),
>      ("cap_hvm_directio", bool), # No longer HVM specific
> +    ("cap_hap", bool),
>      ], dir=DIR_OUT)
> 
>  libxl_connectorinfo = Struct("connectorinfo", [
> diff --git a/tools/xl/xl_info.c b/tools/xl/xl_info.c
> index 46d9c9f712..aa6724bc7f 100644
> --- a/tools/xl/xl_info.c
> +++ b/tools/xl/xl_info.c
> @@ -210,11 +210,12 @@ static void output_physinfo(void)
>           info.hw_cap[4], info.hw_cap[5], info.hw_cap[6], info.hw_cap[7]
>          );
> 
> -    maybe_printf("virt_caps              :%s%s%s%s\n",
> +    maybe_printf("virt_caps              :%s%s%s%s%s\n",
>           info.cap_pv ? " pv" : "",
>           info.cap_hvm ? " hvm" : "",
>           info.cap_hvm && info.cap_hvm_directio ? " hvm_directio" : "",
> -         info.cap_pv && info.cap_hvm_directio ? " pv_directio" : ""
> +         info.cap_pv && info.cap_hvm_directio ? " pv_directio" : "",
> +         info.cap_hap ? " hap" : ""
>          );
> 
>      vinfo = libxl_get_version_info(ctx);
> diff --git a/xen/arch/arm/sysctl.c b/xen/arch/arm/sysctl.c
> index 92ac99c928..f87944e847 100644
> --- a/xen/arch/arm/sysctl.c
> +++ b/xen/arch/arm/sysctl.c
> @@ -14,7 +14,7 @@
> 
>  void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
>  {
> -    pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm;
> +    pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm | XEN_SYSCTL_PHYSCAP_hap;
>  }
> 
>  long arch_do_sysctl(struct xen_sysctl *sysctl,
> diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
> index 7ec6174e6b..5777a05ffc 100644
> --- a/xen/arch/x86/sysctl.c
> +++ b/xen/arch/x86/sysctl.c
> @@ -163,6 +163,8 @@ void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
>          pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm;
>      if ( IS_ENABLED(CONFIG_PV) )
>          pi->capabilities |= XEN_SYSCTL_PHYSCAP_pv;
> +    if ( hvm_hap_supported() )
> +        pi->capabilities |= XEN_SYSCTL_PHYSCAP_hap;
>  }
> 
>  long arch_do_sysctl(
> diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
> index 36b3f8c429..e02d7ce4c6 100644
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -90,6 +90,10 @@ struct xen_sysctl_tbuf_op {
>   /* The platform supports direct access to I/O devices with IOMMU. */
>  #define _XEN_SYSCTL_PHYSCAP_directio     2
>  #define XEN_SYSCTL_PHYSCAP_directio  (1u<<_XEN_SYSCTL_PHYSCAP_directio)
> +/* The platform supports Hardware Assisted Paging. */
> +#define _XEN_SYSCTL_PHYSCAP_hap          3
> +#define XEN_SYSCTL_PHYSCAP_hap           (1u<<_XEN_SYSCTL_PHYSCAP_hap)
> +
>  struct xen_sysctl_physinfo {
>      uint32_t threads_per_core;
>      uint32_t cores_per_socket;
> --
> 2.22.0

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

Re: [Xen-devel] [PATCH v2 1/2] sysctl: report existing physcaps on ARM

[Paul Durrant]

> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Roger Pau Monne
> Sent: 05 September 2019 14:27
> To: xen-devel@lists.xenproject.org
> Cc: Stefano Stabellini <sstabellini@kernel.org>; Wei Liu <wl@xen.org>; Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com>; George Dunlap <George.Dunlap@citrix.com>; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Tim (Xen.org) <tim@xen.org>; Julien
> Grall <julien.grall@arm.com>; Jan Beulich <jbeulich@suse.com>; Volodymyr Babchuk
> <Volodymyr_Babchuk@epam.com>; Roger Pau Monne <roger.pau@citrix.com>
> Subject: [Xen-devel] [PATCH v2 1/2] sysctl: report existing physcaps on ARM
> 
> Current physcaps in XEN_SYSCTL_physinfo are only used by x86, albeit
> the capabilities themselves are not x86 specific.
> 
> This patch adds support for also reporting the current capabilities on
> ARM hardware. Note that on ARM PHYSCAP_hvm is always reported, and
> setting PHYSCAP_directio has been moved to common code since the same
> logic to set it is used by x86 and ARM.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Paul Durrant <paul.durrant@citrix.com>

> ---
> Changes since v1:
>  - New in this version.
> ---
>  xen/arch/arm/sysctl.c       | 5 ++++-
>  xen/arch/x86/sysctl.c       | 2 --
>  xen/common/sysctl.c         | 2 ++
>  xen/include/public/sysctl.h | 6 +++---
>  4 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/arm/sysctl.c b/xen/arch/arm/sysctl.c
> index fbfdb44eff..92ac99c928 100644
> --- a/xen/arch/arm/sysctl.c
> +++ b/xen/arch/arm/sysctl.c
> @@ -12,7 +12,10 @@
>  #include <xen/hypercall.h>
>  #include <public/sysctl.h>
> 
> -void arch_do_physinfo(struct xen_sysctl_physinfo *pi) { }
> +void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
> +{
> +    pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm;
> +}
> 
>  long arch_do_sysctl(struct xen_sysctl *sysctl,
>                      XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
> diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
> index c50d910a1c..7ec6174e6b 100644
> --- a/xen/arch/x86/sysctl.c
> +++ b/xen/arch/x86/sysctl.c
> @@ -163,8 +163,6 @@ void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
>          pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm;
>      if ( IS_ENABLED(CONFIG_PV) )
>          pi->capabilities |= XEN_SYSCTL_PHYSCAP_pv;
> -    if ( iommu_enabled )
> -        pi->capabilities |= XEN_SYSCTL_PHYSCAP_directio;
>  }
> 
>  long arch_do_sysctl(
> diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
> index fcf2d2fd7c..92b4ea0d21 100644
> --- a/xen/common/sysctl.c
> +++ b/xen/common/sysctl.c
> @@ -267,6 +267,8 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
>          pi->cpu_khz = cpu_khz;
>          pi->max_mfn = get_upper_mfn_bound();
>          arch_do_physinfo(pi);
> +        if ( iommu_enabled )
> +            pi->capabilities |= XEN_SYSCTL_PHYSCAP_directio;
> 
>          if ( copy_to_guest(u_sysctl, op, 1) )
>              ret = -EFAULT;
> diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
> index 91c48dcae0..36b3f8c429 100644
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -81,13 +81,13 @@ struct xen_sysctl_tbuf_op {
>   * Get physical information about the host machine
>   */
>  /* XEN_SYSCTL_physinfo */
> - /* (x86) The platform supports HVM guests. */
> + /* The platform supports HVM guests. */
>  #define _XEN_SYSCTL_PHYSCAP_hvm          0
>  #define XEN_SYSCTL_PHYSCAP_hvm           (1u<<_XEN_SYSCTL_PHYSCAP_hvm)
> - /* (x86) The platform supports PV guests. */
> + /* The platform supports PV guests. */
>  #define _XEN_SYSCTL_PHYSCAP_pv           1
>  #define XEN_SYSCTL_PHYSCAP_pv            (1u<<_XEN_SYSCTL_PHYSCAP_pv)
> - /* (x86) The platform supports direct access to I/O devices with IOMMU. */
> + /* The platform supports direct access to I/O devices with IOMMU. */
>  #define _XEN_SYSCTL_PHYSCAP_directio     2
>  #define XEN_SYSCTL_PHYSCAP_directio  (1u<<_XEN_SYSCTL_PHYSCAP_directio)
>  struct xen_sysctl_physinfo {
> --
> 2.22.0
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2 1/2] sysctl: report existing physcaps on ARM

[Jan Beulich]

On 05.09.2019 15:53, Paul Durrant wrote:
>> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Roger Pau Monne
>> Sent: 05 September 2019 14:27
>>
>> Current physcaps in XEN_SYSCTL_physinfo are only used by x86, albeit
>> the capabilities themselves are not x86 specific.
>>
>> This patch adds support for also reporting the current capabilities on
>> ARM hardware. Note that on ARM PHYSCAP_hvm is always reported, and
>> setting PHYSCAP_directio has been moved to common code since the same
>> logic to set it is used by x86 and ARM.
>>
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> Reviewed-by: Paul Durrant <paul.durrant@citrix.com>

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

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

Re: [Xen-devel] [PATCH v2 2/2] sysctl/libxl: choose a sane default for HAP

[Jan Beulich]

On 05.09.2019 15:52, Paul Durrant wrote:
>> From: Roger Pau Monne <roger.pau@citrix.com>
>> Sent: 05 September 2019 14:27
>>
>> Current libxl code will always enable Hardware Assisted Paging (HAP),
>> expecting that the hypervisor will fallback to shadow if HAP is not
>> available. With the changes to the domain builder that's not the case
>> any longer, and the hypervisor will raise an error if HAP is not
>> available instead of silently falling back to shadow.
>>
>> In order to keep the previous functionality report whether HAP is
>> available or not in XEN_SYSCTL_physinfo, so that the toolstack can
>> select a sane default if there's no explicit user selection of whether
>> HAP should be used.
>>
>> Note that on ARM hardware HAP capability is always reported since it's
>> a required feature in order to run Xen.
>>
>> Fixes: d0c0ba7d3de ('x86/hvm/domain: remove the 'hap_enabled' flag')
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> LGTM
> 
> Reviewed-by: Paul Durrant <paul.durrant@citrix.com>

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


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

Re: [Xen-devel] [PATCH v2 2/2] sysctl/libxl: choose a sane default for HAP

[Paul Durrant]

> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Paul Durrant
> Sent: 05 September 2019 14:52
> To: Roger Pau Monne <roger.pau@citrix.com>; xen-devel@lists.xenproject.org
> Cc: Stefano Stabellini <sstabellini@kernel.org>; Wei Liu <wl@xen.org>; Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com>; Andrew Cooper <Andrew.Cooper3@citrix.com>; Tim (Xen.org) <tim@xen.org>;
> George Dunlap <George.Dunlap@citrix.com>; Julien Grall <julien.grall@arm.com>; Jan Beulich
> <jbeulich@suse.com>; Anthony Perard <anthony.perard@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>;
> Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Roger Pau Monne <roger.pau@citrix.com>
> Subject: Re: [Xen-devel] [PATCH v2 2/2] sysctl/libxl: choose a sane default for HAP
> 
> > -----Original Message-----
> > From: Roger Pau Monne <roger.pau@citrix.com>
> > Sent: 05 September 2019 14:27
> > To: xen-devel@lists.xenproject.org
> > Cc: Roger Pau Monne <roger.pau@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Wei Liu
> > <wl@xen.org>; Anthony Perard <anthony.perard@citrix.com>; Andrew Cooper <Andrew.Cooper3@citrix.com>;
> > George Dunlap <George.Dunlap@citrix.com>; Jan Beulich <jbeulich@suse.com>; Julien Grall
> > <julien.grall@arm.com>; Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; Stefano Stabellini
> > <sstabellini@kernel.org>; Tim (Xen.org) <tim@xen.org>; Volodymyr Babchuk
> <Volodymyr_Babchuk@epam.com>;
> > Paul Durrant <Paul.Durrant@citrix.com>
> > Subject: [PATCH v2 2/2] sysctl/libxl: choose a sane default for HAP
> >
> > Current libxl code will always enable Hardware Assisted Paging (HAP),
> > expecting that the hypervisor will fallback to shadow if HAP is not
> > available. With the changes to the domain builder that's not the case
> > any longer, and the hypervisor will raise an error if HAP is not
> > available instead of silently falling back to shadow.
> >
> > In order to keep the previous functionality report whether HAP is
> > available or not in XEN_SYSCTL_physinfo, so that the toolstack can
> > select a sane default if there's no explicit user selection of whether
> > HAP should be used.
> >
> > Note that on ARM hardware HAP capability is always reported since it's
> > a required feature in order to run Xen.
> >
> > Fixes: d0c0ba7d3de ('x86/hvm/domain: remove the 'hap_enabled' flag')
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> LGTM
> 
> Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
> 
> > ---
> > Cc: Paul Durrant <Paul.Durrant@citrix.com>
> > ---
> > Changes since v1:
> >  - Also report HAP capability for ARM.
> >  - Print hap capability in xl info.
> > ---
> >  tools/libxl/libxl.c         | 1 +
> >  tools/libxl/libxl_create.c  | 8 +++++++-
> >  tools/libxl/libxl_types.idl | 1 +
> >  tools/xl/xl_info.c          | 5 +++--
> >  xen/arch/arm/sysctl.c       | 2 +-
> >  xen/arch/x86/sysctl.c       | 2 ++
> >  xen/include/public/sysctl.h | 4 ++++
> >  7 files changed, 19 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > index ec71574e99..5c0fcf320e 100644
> > --- a/tools/libxl/libxl.c
> > +++ b/tools/libxl/libxl.c
> > @@ -399,6 +399,7 @@ int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo *physinfo)
> >      physinfo->cap_pv = !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_pv);
> >      physinfo->cap_hvm_directio =
> >          !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_directio);
> > +    physinfo->cap_hap = !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_hap);
> >
> >      GC_FREE;
> >      return 0;
> > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> > index 03ce166f4f..6a556dea8f 100644
> > --- a/tools/libxl/libxl_create.c
> > +++ b/tools/libxl/libxl_create.c
> > @@ -38,7 +38,13 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc,
> >      libxl__arch_domain_create_info_setdefault(gc, c_info);
> >
> >      if (c_info->type != LIBXL_DOMAIN_TYPE_PV) {
> > -        libxl_defbool_setdefault(&c_info->hap, true);
> > +        libxl_physinfo info;
> > +        int rc = libxl_get_physinfo(CTX, &info);
> > +
> > +        if (rc)
> > +            return rc;
> > +
> > +        libxl_defbool_setdefault(&c_info->hap, info.cap_hap);
> >          libxl_defbool_setdefault(&c_info->oos, true);
> >      }
> >
> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > index b61399ce36..9e1f8515d3 100644
> > --- a/tools/libxl/libxl_types.idl
> > +++ b/tools/libxl/libxl_types.idl
> > @@ -1025,6 +1025,7 @@ libxl_physinfo = Struct("physinfo", [
> >      ("cap_hvm", bool),
> >      ("cap_pv", bool),
> >      ("cap_hvm_directio", bool), # No longer HVM specific
> > +    ("cap_hap", bool),

Actually Julien's review of one of my patches points out that this idl change should be accompanied by an associated LIBXL_HAVE_CAP_HAP definition.

  Paul

> >      ], dir=DIR_OUT)
> >
> >  libxl_connectorinfo = Struct("connectorinfo", [
> > diff --git a/tools/xl/xl_info.c b/tools/xl/xl_info.c
> > index 46d9c9f712..aa6724bc7f 100644
> > --- a/tools/xl/xl_info.c
> > +++ b/tools/xl/xl_info.c
> > @@ -210,11 +210,12 @@ static void output_physinfo(void)
> >           info.hw_cap[4], info.hw_cap[5], info.hw_cap[6], info.hw_cap[7]
> >          );
> >
> > -    maybe_printf("virt_caps              :%s%s%s%s\n",
> > +    maybe_printf("virt_caps              :%s%s%s%s%s\n",
> >           info.cap_pv ? " pv" : "",
> >           info.cap_hvm ? " hvm" : "",
> >           info.cap_hvm && info.cap_hvm_directio ? " hvm_directio" : "",
> > -         info.cap_pv && info.cap_hvm_directio ? " pv_directio" : ""
> > +         info.cap_pv && info.cap_hvm_directio ? " pv_directio" : "",
> > +         info.cap_hap ? " hap" : ""
> >          );
> >
> >      vinfo = libxl_get_version_info(ctx);
> > diff --git a/xen/arch/arm/sysctl.c b/xen/arch/arm/sysctl.c
> > index 92ac99c928..f87944e847 100644
> > --- a/xen/arch/arm/sysctl.c
> > +++ b/xen/arch/arm/sysctl.c
> > @@ -14,7 +14,7 @@
> >
> >  void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
> >  {
> > -    pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm;
> > +    pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm | XEN_SYSCTL_PHYSCAP_hap;
> >  }
> >
> >  long arch_do_sysctl(struct xen_sysctl *sysctl,
> > diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
> > index 7ec6174e6b..5777a05ffc 100644
> > --- a/xen/arch/x86/sysctl.c
> > +++ b/xen/arch/x86/sysctl.c
> > @@ -163,6 +163,8 @@ void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
> >          pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm;
> >      if ( IS_ENABLED(CONFIG_PV) )
> >          pi->capabilities |= XEN_SYSCTL_PHYSCAP_pv;
> > +    if ( hvm_hap_supported() )
> > +        pi->capabilities |= XEN_SYSCTL_PHYSCAP_hap;
> >  }
> >
> >  long arch_do_sysctl(
> > diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
> > index 36b3f8c429..e02d7ce4c6 100644
> > --- a/xen/include/public/sysctl.h
> > +++ b/xen/include/public/sysctl.h
> > @@ -90,6 +90,10 @@ struct xen_sysctl_tbuf_op {
> >   /* The platform supports direct access to I/O devices with IOMMU. */
> >  #define _XEN_SYSCTL_PHYSCAP_directio     2
> >  #define XEN_SYSCTL_PHYSCAP_directio  (1u<<_XEN_SYSCTL_PHYSCAP_directio)
> > +/* The platform supports Hardware Assisted Paging. */
> > +#define _XEN_SYSCTL_PHYSCAP_hap          3
> > +#define XEN_SYSCTL_PHYSCAP_hap           (1u<<_XEN_SYSCTL_PHYSCAP_hap)
> > +
> >  struct xen_sysctl_physinfo {
> >      uint32_t threads_per_core;
> >      uint32_t cores_per_socket;
> > --
> > 2.22.0
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2 2/2] sysctl/libxl: choose a sane default for HAP

[Roger Pau Monné]

On Fri, Sep 06, 2019 at 03:54:10PM +0200, Paul Durrant wrote:
> > -----Original Message-----
> > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Paul Durrant
> > Sent: 05 September 2019 14:52
> > To: Roger Pau Monne <roger.pau@citrix.com>; xen-devel@lists.xenproject.org
> > Cc: Stefano Stabellini <sstabellini@kernel.org>; Wei Liu <wl@xen.org>; Konrad Rzeszutek Wilk
> > <konrad.wilk@oracle.com>; Andrew Cooper <Andrew.Cooper3@citrix.com>; Tim (Xen.org) <tim@xen.org>;
> > George Dunlap <George.Dunlap@citrix.com>; Julien Grall <julien.grall@arm.com>; Jan Beulich
> > <jbeulich@suse.com>; Anthony Perard <anthony.perard@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>;
> > Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Roger Pau Monne <roger.pau@citrix.com>
> > Subject: Re: [Xen-devel] [PATCH v2 2/2] sysctl/libxl: choose a sane default for HAP
> > 
> > > -----Original Message-----
> > > From: Roger Pau Monne <roger.pau@citrix.com>
> > > Sent: 05 September 2019 14:27
> > > To: xen-devel@lists.xenproject.org
> > > Cc: Roger Pau Monne <roger.pau@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Wei Liu
> > > <wl@xen.org>; Anthony Perard <anthony.perard@citrix.com>; Andrew Cooper <Andrew.Cooper3@citrix.com>;
> > > George Dunlap <George.Dunlap@citrix.com>; Jan Beulich <jbeulich@suse.com>; Julien Grall
> > > <julien.grall@arm.com>; Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; Stefano Stabellini
> > > <sstabellini@kernel.org>; Tim (Xen.org) <tim@xen.org>; Volodymyr Babchuk
> > <Volodymyr_Babchuk@epam.com>;
> > > Paul Durrant <Paul.Durrant@citrix.com>
> > > Subject: [PATCH v2 2/2] sysctl/libxl: choose a sane default for HAP
> > >
> > > Current libxl code will always enable Hardware Assisted Paging (HAP),
> > > expecting that the hypervisor will fallback to shadow if HAP is not
> > > available. With the changes to the domain builder that's not the case
> > > any longer, and the hypervisor will raise an error if HAP is not
> > > available instead of silently falling back to shadow.
> > >
> > > In order to keep the previous functionality report whether HAP is
> > > available or not in XEN_SYSCTL_physinfo, so that the toolstack can
> > > select a sane default if there's no explicit user selection of whether
> > > HAP should be used.
> > >
> > > Note that on ARM hardware HAP capability is always reported since it's
> > > a required feature in order to run Xen.
> > >
> > > Fixes: d0c0ba7d3de ('x86/hvm/domain: remove the 'hap_enabled' flag')
> > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > 
> > LGTM
> > 
> > Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
> > 
> > > ---
> > > Cc: Paul Durrant <Paul.Durrant@citrix.com>
> > > ---
> > > Changes since v1:
> > >  - Also report HAP capability for ARM.
> > >  - Print hap capability in xl info.
> > > ---
> > >  tools/libxl/libxl.c         | 1 +
> > >  tools/libxl/libxl_create.c  | 8 +++++++-
> > >  tools/libxl/libxl_types.idl | 1 +
> > >  tools/xl/xl_info.c          | 5 +++--
> > >  xen/arch/arm/sysctl.c       | 2 +-
> > >  xen/arch/x86/sysctl.c       | 2 ++
> > >  xen/include/public/sysctl.h | 4 ++++
> > >  7 files changed, 19 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > > index ec71574e99..5c0fcf320e 100644
> > > --- a/tools/libxl/libxl.c
> > > +++ b/tools/libxl/libxl.c
> > > @@ -399,6 +399,7 @@ int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo *physinfo)
> > >      physinfo->cap_pv = !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_pv);
> > >      physinfo->cap_hvm_directio =
> > >          !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_directio);
> > > +    physinfo->cap_hap = !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_hap);
> > >
> > >      GC_FREE;
> > >      return 0;
> > > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> > > index 03ce166f4f..6a556dea8f 100644
> > > --- a/tools/libxl/libxl_create.c
> > > +++ b/tools/libxl/libxl_create.c
> > > @@ -38,7 +38,13 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc,
> > >      libxl__arch_domain_create_info_setdefault(gc, c_info);
> > >
> > >      if (c_info->type != LIBXL_DOMAIN_TYPE_PV) {
> > > -        libxl_defbool_setdefault(&c_info->hap, true);
> > > +        libxl_physinfo info;
> > > +        int rc = libxl_get_physinfo(CTX, &info);
> > > +
> > > +        if (rc)
> > > +            return rc;
> > > +
> > > +        libxl_defbool_setdefault(&c_info->hap, info.cap_hap);
> > >          libxl_defbool_setdefault(&c_info->oos, true);
> > >      }
> > >
> > > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > > index b61399ce36..9e1f8515d3 100644
> > > --- a/tools/libxl/libxl_types.idl
> > > +++ b/tools/libxl/libxl_types.idl
> > > @@ -1025,6 +1025,7 @@ libxl_physinfo = Struct("physinfo", [
> > >      ("cap_hvm", bool),
> > >      ("cap_pv", bool),
> > >      ("cap_hvm_directio", bool), # No longer HVM specific
> > > +    ("cap_hap", bool),
> 
> Actually Julien's review of one of my patches points out that this idl change should be accompanied by an associated LIBXL_HAVE_CAP_HAP definition.

Ouch, yes, I always forget those. I will add now, and keep your RB and
Jan's Ack unless any of you tell me otherwise.

Thanks, Roger.

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