[PATCH v3 1/3] xen+tools: Introduce XEN_SYSCTL_PHYSCAP_vpmu

Michal Orzel posted 3 patches 2 months ago

[PATCH v3 1/3] xen+tools: Introduce XEN_SYSCTL_PHYSCAP_vpmu

Posted by Michal Orzel 2 months ago
Introduce flag XEN_SYSCTL_PHYSCAP_vpmu which
indicates whether the platform supports vPMU
functionality. Modify Xen and tools accordingly.

Take the opportunity and fix XEN_SYSCTL_PHYSCAP_vmtrace
definition in sysctl.h which wrongly use (1<<6)
instead of (1u<<6).

Signed-off-by: Michal Orzel <michal.orzel@arm.com>
---
Changes since v2:
-do not define bit position and mask separately
Changes since v1:
-new in v2
---
 tools/golang/xenlight/helpers.gen.go | 2 ++
 tools/golang/xenlight/types.gen.go   | 1 +
 tools/include/libxl.h                | 6 ++++++
 tools/libs/light/libxl.c             | 1 +
 tools/libs/light/libxl_types.idl     | 1 +
 tools/ocaml/libs/xc/xenctrl.ml       | 1 +
 tools/ocaml/libs/xc/xenctrl.mli      | 1 +
 tools/xl/xl_info.c                   | 5 +++--
 xen/common/domain.c                  | 2 ++
 xen/common/sysctl.c                  | 3 +++
 xen/include/public/sysctl.h          | 6 ++++--
 xen/include/xen/domain.h             | 2 ++
 12 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/tools/golang/xenlight/helpers.gen.go b/tools/golang/xenlight/helpers.gen.go
index bfc1e7f312..c8669837d8 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -3360,6 +3360,7 @@ x.CapHap = bool(xc.cap_hap)
 x.CapShadow = bool(xc.cap_shadow)
 x.CapIommuHapPtShare = bool(xc.cap_iommu_hap_pt_share)
 x.CapVmtrace = bool(xc.cap_vmtrace)
+x.CapVpmu = bool(xc.cap_vpmu)
 
  return nil}
 
@@ -3391,6 +3392,7 @@ xc.cap_hap = C.bool(x.CapHap)
 xc.cap_shadow = C.bool(x.CapShadow)
 xc.cap_iommu_hap_pt_share = C.bool(x.CapIommuHapPtShare)
 xc.cap_vmtrace = C.bool(x.CapVmtrace)
+xc.cap_vpmu = C.bool(x.CapVpmu)
 
  return nil
  }
diff --git a/tools/golang/xenlight/types.gen.go b/tools/golang/xenlight/types.gen.go
index 09a3bb67e2..45f2cba3d2 100644
--- a/tools/golang/xenlight/types.gen.go
+++ b/tools/golang/xenlight/types.gen.go
@@ -1008,6 +1008,7 @@ CapHap bool
 CapShadow bool
 CapIommuHapPtShare bool
 CapVmtrace bool
+CapVpmu bool
 }
 
 type Connectorinfo struct {
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index b9ba16d698..ec5e3badae 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -502,6 +502,12 @@
  */
 #define LIBXL_HAVE_X86_MSR_RELAXED 1
 
+/*
+ * LIBXL_HAVE_PHYSINFO_CAP_VPMU indicates that libxl_physinfo has a cap_vpmu
+ * field, which indicates the availability of vPMU functionality.
+ */
+#define LIBXL_HAVE_PHYSINFO_CAP_VPMU 1
+
 /*
  * libxl ABI compatibility
  *
diff --git a/tools/libs/light/libxl.c b/tools/libs/light/libxl.c
index 204eb0be2d..a032723fde 100644
--- a/tools/libs/light/libxl.c
+++ b/tools/libs/light/libxl.c
@@ -404,6 +404,7 @@ int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo *physinfo)
         !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share);
     physinfo->cap_vmtrace =
         !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_vmtrace);
+    physinfo->cap_vpmu = !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_vpmu);
 
     GC_FREE;
     return 0;
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index 3f9fff653a..993e83acca 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -1061,6 +1061,7 @@ libxl_physinfo = Struct("physinfo", [
     ("cap_shadow", bool),
     ("cap_iommu_hap_pt_share", bool),
     ("cap_vmtrace", bool),
+    ("cap_vpmu", bool),
     ], dir=DIR_OUT)
 
 libxl_connectorinfo = Struct("connectorinfo", [
diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
index a5588c643f..6da3ed3c6f 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -121,6 +121,7 @@ type physinfo_cap_flag =
 	| CAP_Shadow
 	| CAP_IOMMU_HAP_PT_SHARE
 	| CAP_Vmtrace
+	| CAP_Vpmu
 
 type physinfo =
 {
diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
index 6e94940a8a..b8faff6721 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -106,6 +106,7 @@ type physinfo_cap_flag =
   | CAP_Shadow
   | CAP_IOMMU_HAP_PT_SHARE
   | CAP_Vmtrace
+  | CAP_Vpmu
 
 type physinfo = {
   threads_per_core : int;
diff --git a/tools/xl/xl_info.c b/tools/xl/xl_info.c
index 8383e4a6df..2c86b317b7 100644
--- a/tools/xl/xl_info.c
+++ b/tools/xl/xl_info.c
@@ -210,7 +210,7 @@ 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%s%s%s%s\n",
+    maybe_printf("virt_caps              :%s%s%s%s%s%s%s%s%s\n",
          info.cap_pv ? " pv" : "",
          info.cap_hvm ? " hvm" : "",
          info.cap_hvm && info.cap_hvm_directio ? " hvm_directio" : "",
@@ -218,7 +218,8 @@ static void output_physinfo(void)
          info.cap_hap ? " hap" : "",
          info.cap_shadow ? " shadow" : "",
          info.cap_iommu_hap_pt_share ? " iommu_hap_pt_share" : "",
-         info.cap_vmtrace ? " vmtrace" : ""
+         info.cap_vmtrace ? " vmtrace" : "",
+         info.cap_vpmu ? " vpmu" : ""
         );
 
     vinfo = libxl_get_version_info(ctx);
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 6ee5d033b0..4d0e909eec 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -84,6 +84,8 @@ vcpu_info_t dummy_vcpu_info;
 
 bool __read_mostly vmtrace_available;
 
+bool __read_mostly vpmu_is_available;
+
 static void __domain_finalise_shutdown(struct domain *d)
 {
     struct vcpu *v;
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index 3558641cd9..6e7189bb3c 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -280,6 +280,9 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
         if ( vmtrace_available )
             pi->capabilities |= XEN_SYSCTL_PHYSCAP_vmtrace;
 
+        if ( vpmu_is_available )
+            pi->capabilities |= XEN_SYSCTL_PHYSCAP_vpmu;
+
         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 039ccf885c..b794c9d351 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -100,10 +100,12 @@ struct xen_sysctl_tbuf_op {
 #define _XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share 5
 #define XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share  \
     (1u << _XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share)
-#define XEN_SYSCTL_PHYSCAP_vmtrace       (1 << 6)
+#define XEN_SYSCTL_PHYSCAP_vmtrace       (1u<<6)
+/* The platform supports vPMU. */
+#define XEN_SYSCTL_PHYSCAP_vpmu          (1u<<7)
 
 /* Max XEN_SYSCTL_PHYSCAP_* constant.  Used for ABI checking. */
-#define XEN_SYSCTL_PHYSCAP_MAX XEN_SYSCTL_PHYSCAP_vmtrace
+#define XEN_SYSCTL_PHYSCAP_MAX XEN_SYSCTL_PHYSCAP_vpmu
 
 struct xen_sysctl_physinfo {
     uint32_t threads_per_core;
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index 1708c36964..160c8dbdab 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -133,4 +133,6 @@ static inline void vnuma_destroy(struct vnuma_info *vnuma) { ASSERT(!vnuma); }
 
 extern bool vmtrace_available;
 
+extern bool vpmu_is_available;
+
 #endif /* __XEN_DOMAIN_H__ */
-- 
2.29.0


Re: [PATCH v3 1/3] xen+tools: Introduce XEN_SYSCTL_PHYSCAP_vpmu

Posted by Stefano Stabellini 2 months ago
On Fri, 8 Oct 2021, Michal Orzel wrote:
> Introduce flag XEN_SYSCTL_PHYSCAP_vpmu which
> indicates whether the platform supports vPMU
> functionality. Modify Xen and tools accordingly.
> 
> Take the opportunity and fix XEN_SYSCTL_PHYSCAP_vmtrace
> definition in sysctl.h which wrongly use (1<<6)
> instead of (1u<<6).
> 
> Signed-off-by: Michal Orzel <michal.orzel@arm.com>

Aside from the minor style issue Jan pointed out:

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> Changes since v2:
> -do not define bit position and mask separately
> Changes since v1:
> -new in v2
> ---
>  tools/golang/xenlight/helpers.gen.go | 2 ++
>  tools/golang/xenlight/types.gen.go   | 1 +
>  tools/include/libxl.h                | 6 ++++++
>  tools/libs/light/libxl.c             | 1 +
>  tools/libs/light/libxl_types.idl     | 1 +
>  tools/ocaml/libs/xc/xenctrl.ml       | 1 +
>  tools/ocaml/libs/xc/xenctrl.mli      | 1 +
>  tools/xl/xl_info.c                   | 5 +++--
>  xen/common/domain.c                  | 2 ++
>  xen/common/sysctl.c                  | 3 +++
>  xen/include/public/sysctl.h          | 6 ++++--
>  xen/include/xen/domain.h             | 2 ++
>  12 files changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/golang/xenlight/helpers.gen.go b/tools/golang/xenlight/helpers.gen.go
> index bfc1e7f312..c8669837d8 100644
> --- a/tools/golang/xenlight/helpers.gen.go
> +++ b/tools/golang/xenlight/helpers.gen.go
> @@ -3360,6 +3360,7 @@ x.CapHap = bool(xc.cap_hap)
>  x.CapShadow = bool(xc.cap_shadow)
>  x.CapIommuHapPtShare = bool(xc.cap_iommu_hap_pt_share)
>  x.CapVmtrace = bool(xc.cap_vmtrace)
> +x.CapVpmu = bool(xc.cap_vpmu)
>  
>   return nil}
>  
> @@ -3391,6 +3392,7 @@ xc.cap_hap = C.bool(x.CapHap)
>  xc.cap_shadow = C.bool(x.CapShadow)
>  xc.cap_iommu_hap_pt_share = C.bool(x.CapIommuHapPtShare)
>  xc.cap_vmtrace = C.bool(x.CapVmtrace)
> +xc.cap_vpmu = C.bool(x.CapVpmu)
>  
>   return nil
>   }
> diff --git a/tools/golang/xenlight/types.gen.go b/tools/golang/xenlight/types.gen.go
> index 09a3bb67e2..45f2cba3d2 100644
> --- a/tools/golang/xenlight/types.gen.go
> +++ b/tools/golang/xenlight/types.gen.go
> @@ -1008,6 +1008,7 @@ CapHap bool
>  CapShadow bool
>  CapIommuHapPtShare bool
>  CapVmtrace bool
> +CapVpmu bool
>  }
>  
>  type Connectorinfo struct {
> diff --git a/tools/include/libxl.h b/tools/include/libxl.h
> index b9ba16d698..ec5e3badae 100644
> --- a/tools/include/libxl.h
> +++ b/tools/include/libxl.h
> @@ -502,6 +502,12 @@
>   */
>  #define LIBXL_HAVE_X86_MSR_RELAXED 1
>  
> +/*
> + * LIBXL_HAVE_PHYSINFO_CAP_VPMU indicates that libxl_physinfo has a cap_vpmu
> + * field, which indicates the availability of vPMU functionality.
> + */
> +#define LIBXL_HAVE_PHYSINFO_CAP_VPMU 1
> +
>  /*
>   * libxl ABI compatibility
>   *
> diff --git a/tools/libs/light/libxl.c b/tools/libs/light/libxl.c
> index 204eb0be2d..a032723fde 100644
> --- a/tools/libs/light/libxl.c
> +++ b/tools/libs/light/libxl.c
> @@ -404,6 +404,7 @@ int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo *physinfo)
>          !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share);
>      physinfo->cap_vmtrace =
>          !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_vmtrace);
> +    physinfo->cap_vpmu = !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_vpmu);
>  
>      GC_FREE;
>      return 0;
> diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
> index 3f9fff653a..993e83acca 100644
> --- a/tools/libs/light/libxl_types.idl
> +++ b/tools/libs/light/libxl_types.idl
> @@ -1061,6 +1061,7 @@ libxl_physinfo = Struct("physinfo", [
>      ("cap_shadow", bool),
>      ("cap_iommu_hap_pt_share", bool),
>      ("cap_vmtrace", bool),
> +    ("cap_vpmu", bool),
>      ], dir=DIR_OUT)
>  
>  libxl_connectorinfo = Struct("connectorinfo", [
> diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
> index a5588c643f..6da3ed3c6f 100644
> --- a/tools/ocaml/libs/xc/xenctrl.ml
> +++ b/tools/ocaml/libs/xc/xenctrl.ml
> @@ -121,6 +121,7 @@ type physinfo_cap_flag =
>  	| CAP_Shadow
>  	| CAP_IOMMU_HAP_PT_SHARE
>  	| CAP_Vmtrace
> +	| CAP_Vpmu
>  
>  type physinfo =
>  {
> diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
> index 6e94940a8a..b8faff6721 100644
> --- a/tools/ocaml/libs/xc/xenctrl.mli
> +++ b/tools/ocaml/libs/xc/xenctrl.mli
> @@ -106,6 +106,7 @@ type physinfo_cap_flag =
>    | CAP_Shadow
>    | CAP_IOMMU_HAP_PT_SHARE
>    | CAP_Vmtrace
> +  | CAP_Vpmu
>  
>  type physinfo = {
>    threads_per_core : int;
> diff --git a/tools/xl/xl_info.c b/tools/xl/xl_info.c
> index 8383e4a6df..2c86b317b7 100644
> --- a/tools/xl/xl_info.c
> +++ b/tools/xl/xl_info.c
> @@ -210,7 +210,7 @@ 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%s%s%s%s\n",
> +    maybe_printf("virt_caps              :%s%s%s%s%s%s%s%s%s\n",
>           info.cap_pv ? " pv" : "",
>           info.cap_hvm ? " hvm" : "",
>           info.cap_hvm && info.cap_hvm_directio ? " hvm_directio" : "",
> @@ -218,7 +218,8 @@ static void output_physinfo(void)
>           info.cap_hap ? " hap" : "",
>           info.cap_shadow ? " shadow" : "",
>           info.cap_iommu_hap_pt_share ? " iommu_hap_pt_share" : "",
> -         info.cap_vmtrace ? " vmtrace" : ""
> +         info.cap_vmtrace ? " vmtrace" : "",
> +         info.cap_vpmu ? " vpmu" : ""
>          );
>  
>      vinfo = libxl_get_version_info(ctx);
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 6ee5d033b0..4d0e909eec 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -84,6 +84,8 @@ vcpu_info_t dummy_vcpu_info;
>  
>  bool __read_mostly vmtrace_available;
>  
> +bool __read_mostly vpmu_is_available;
> +
>  static void __domain_finalise_shutdown(struct domain *d)
>  {
>      struct vcpu *v;
> diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
> index 3558641cd9..6e7189bb3c 100644
> --- a/xen/common/sysctl.c
> +++ b/xen/common/sysctl.c
> @@ -280,6 +280,9 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
>          if ( vmtrace_available )
>              pi->capabilities |= XEN_SYSCTL_PHYSCAP_vmtrace;
>  
> +        if ( vpmu_is_available )
> +            pi->capabilities |= XEN_SYSCTL_PHYSCAP_vpmu;
> +
>          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 039ccf885c..b794c9d351 100644
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -100,10 +100,12 @@ struct xen_sysctl_tbuf_op {
>  #define _XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share 5
>  #define XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share  \
>      (1u << _XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share)
> -#define XEN_SYSCTL_PHYSCAP_vmtrace       (1 << 6)
> +#define XEN_SYSCTL_PHYSCAP_vmtrace       (1u<<6)
> +/* The platform supports vPMU. */
> +#define XEN_SYSCTL_PHYSCAP_vpmu          (1u<<7)
>  
>  /* Max XEN_SYSCTL_PHYSCAP_* constant.  Used for ABI checking. */
> -#define XEN_SYSCTL_PHYSCAP_MAX XEN_SYSCTL_PHYSCAP_vmtrace
> +#define XEN_SYSCTL_PHYSCAP_MAX XEN_SYSCTL_PHYSCAP_vpmu
>  
>  struct xen_sysctl_physinfo {
>      uint32_t threads_per_core;
> diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
> index 1708c36964..160c8dbdab 100644
> --- a/xen/include/xen/domain.h
> +++ b/xen/include/xen/domain.h
> @@ -133,4 +133,6 @@ static inline void vnuma_destroy(struct vnuma_info *vnuma) { ASSERT(!vnuma); }
>  
>  extern bool vmtrace_available;
>  
> +extern bool vpmu_is_available;
> +
>  #endif /* __XEN_DOMAIN_H__ */
> -- 
> 2.29.0
> 

Re: [PATCH v3 1/3] xen+tools: Introduce XEN_SYSCTL_PHYSCAP_vpmu

Posted by Nick Rosbrook 2 months ago
On Fri, Oct 08, 2021 at 10:19:31AM +0200, Michal Orzel wrote:
> Introduce flag XEN_SYSCTL_PHYSCAP_vpmu which
> indicates whether the platform supports vPMU
> functionality. Modify Xen and tools accordingly.
> 
> Take the opportunity and fix XEN_SYSCTL_PHYSCAP_vmtrace
> definition in sysctl.h which wrongly use (1<<6)
> instead of (1u<<6).
> 
> Signed-off-by: Michal Orzel <michal.orzel@arm.com>
> ---
> Changes since v2:
> -do not define bit position and mask separately
> Changes since v1:
> -new in v2
> ---
>  tools/golang/xenlight/helpers.gen.go | 2 ++
>  tools/golang/xenlight/types.gen.go   | 1 +
>  tools/include/libxl.h                | 6 ++++++
>  tools/libs/light/libxl.c             | 1 +
>  tools/libs/light/libxl_types.idl     | 1 +
>  tools/ocaml/libs/xc/xenctrl.ml       | 1 +
>  tools/ocaml/libs/xc/xenctrl.mli      | 1 +
>  tools/xl/xl_info.c                   | 5 +++--
>  xen/common/domain.c                  | 2 ++
>  xen/common/sysctl.c                  | 3 +++
>  xen/include/public/sysctl.h          | 6 ++++--
>  xen/include/xen/domain.h             | 2 ++
>  12 files changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/golang/xenlight/helpers.gen.go b/tools/golang/xenlight/helpers.gen.go
> index bfc1e7f312..c8669837d8 100644
> --- a/tools/golang/xenlight/helpers.gen.go
> +++ b/tools/golang/xenlight/helpers.gen.go
> @@ -3360,6 +3360,7 @@ x.CapHap = bool(xc.cap_hap)
>  x.CapShadow = bool(xc.cap_shadow)
>  x.CapIommuHapPtShare = bool(xc.cap_iommu_hap_pt_share)
>  x.CapVmtrace = bool(xc.cap_vmtrace)
> +x.CapVpmu = bool(xc.cap_vpmu)
>  
>   return nil}
>  
> @@ -3391,6 +3392,7 @@ xc.cap_hap = C.bool(x.CapHap)
>  xc.cap_shadow = C.bool(x.CapShadow)
>  xc.cap_iommu_hap_pt_share = C.bool(x.CapIommuHapPtShare)
>  xc.cap_vmtrace = C.bool(x.CapVmtrace)
> +xc.cap_vpmu = C.bool(x.CapVpmu)
>  
>   return nil
>   }
> diff --git a/tools/golang/xenlight/types.gen.go b/tools/golang/xenlight/types.gen.go
> index 09a3bb67e2..45f2cba3d2 100644
> --- a/tools/golang/xenlight/types.gen.go
> +++ b/tools/golang/xenlight/types.gen.go
> @@ -1008,6 +1008,7 @@ CapHap bool
>  CapShadow bool
>  CapIommuHapPtShare bool
>  CapVmtrace bool
> +CapVpmu bool
>  }

For the golang re-gen,

Acked-by: Nick Rosbrook <rosbrookn@ainfosec.com>

Thanks,
NR

Re: [PATCH v3 1/3] xen+tools: Introduce XEN_SYSCTL_PHYSCAP_vpmu

Posted by Jan Beulich 2 months ago
On 08.10.2021 10:19, Michal Orzel wrote:
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -100,10 +100,12 @@ struct xen_sysctl_tbuf_op {
>  #define _XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share 5
>  #define XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share  \
>      (1u << _XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share)
> -#define XEN_SYSCTL_PHYSCAP_vmtrace       (1 << 6)
> +#define XEN_SYSCTL_PHYSCAP_vmtrace       (1u<<6)
> +/* The platform supports vPMU. */
> +#define XEN_SYSCTL_PHYSCAP_vpmu          (1u<<7)

While purely cosmetic and easily fixable while committing, I still
fail to understand why you did drop the blanks from the expression
you adjust and why you didn't add blanks to the new expression.
./CODING_STYLE is quite clear in this respect, and even the code
in context does not suggest any alternative style. (While code just
outside of visible context does, it should not be used as excuse
to introduce further style violations.)

Jan


Re: [PATCH v3 1/3] xen+tools: Introduce XEN_SYSCTL_PHYSCAP_vpmu

Posted by Michal Orzel 2 months ago
Hi Jan,

On 08.10.2021 10:33, Jan Beulich wrote:
> On 08.10.2021 10:19, Michal Orzel wrote:
>> --- a/xen/include/public/sysctl.h
>> +++ b/xen/include/public/sysctl.h
>> @@ -100,10 +100,12 @@ struct xen_sysctl_tbuf_op {
>>  #define _XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share 5
>>  #define XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share  \
>>      (1u << _XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share)
>> -#define XEN_SYSCTL_PHYSCAP_vmtrace       (1 << 6)
>> +#define XEN_SYSCTL_PHYSCAP_vmtrace       (1u<<6)
>> +/* The platform supports vPMU. */
>> +#define XEN_SYSCTL_PHYSCAP_vpmu          (1u<<7)
> 
> While purely cosmetic and easily fixable while committing, I still
> fail to understand why you did drop the blanks from the expression
> you adjust and why you didn't add blanks to the new expression.
> ./CODING_STYLE is quite clear in this respect, and even the code
> in context does not suggest any alternative style. (While code just
> outside of visible context does, it should not be used as excuse
> to introduce further style violations.)
> 
I was just biased by the previous entries.
Previous entries were not added at once but commit after commit.
No one did any remarks on that so I decided to stick to the "section standard".
From now one I will be looking only at the coding style even though the section
is doing something else. If it is possible, please fix it while commiting.

> Jan
> 

Cheers,
Michal

Re: [PATCH v3 1/3] xen+tools: Introduce XEN_SYSCTL_PHYSCAP_vpmu

Posted by Bertrand Marquis 2 months ago
Hi Michal

> On 8 Oct 2021, at 09:19, Michal Orzel <Michal.Orzel@arm.com> wrote:
> 
> Introduce flag XEN_SYSCTL_PHYSCAP_vpmu which
> indicates whether the platform supports vPMU
> functionality. Modify Xen and tools accordingly.
> 
> Take the opportunity and fix XEN_SYSCTL_PHYSCAP_vmtrace
> definition in sysctl.h which wrongly use (1<<6)
> instead of (1u<<6).
> 
> Signed-off-by: Michal Orzel <michal.orzel@arm.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand

> ---
> Changes since v2:
> -do not define bit position and mask separately
> Changes since v1:
> -new in v2
> ---
> tools/golang/xenlight/helpers.gen.go | 2 ++
> tools/golang/xenlight/types.gen.go   | 1 +
> tools/include/libxl.h                | 6 ++++++
> tools/libs/light/libxl.c             | 1 +
> tools/libs/light/libxl_types.idl     | 1 +
> tools/ocaml/libs/xc/xenctrl.ml       | 1 +
> tools/ocaml/libs/xc/xenctrl.mli      | 1 +
> tools/xl/xl_info.c                   | 5 +++--
> xen/common/domain.c                  | 2 ++
> xen/common/sysctl.c                  | 3 +++
> xen/include/public/sysctl.h          | 6 ++++--
> xen/include/xen/domain.h             | 2 ++
> 12 files changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/golang/xenlight/helpers.gen.go b/tools/golang/xenlight/helpers.gen.go
> index bfc1e7f312..c8669837d8 100644
> --- a/tools/golang/xenlight/helpers.gen.go
> +++ b/tools/golang/xenlight/helpers.gen.go
> @@ -3360,6 +3360,7 @@ x.CapHap = bool(xc.cap_hap)
> x.CapShadow = bool(xc.cap_shadow)
> x.CapIommuHapPtShare = bool(xc.cap_iommu_hap_pt_share)
> x.CapVmtrace = bool(xc.cap_vmtrace)
> +x.CapVpmu = bool(xc.cap_vpmu)
> 
>  return nil}
> 
> @@ -3391,6 +3392,7 @@ xc.cap_hap = C.bool(x.CapHap)
> xc.cap_shadow = C.bool(x.CapShadow)
> xc.cap_iommu_hap_pt_share = C.bool(x.CapIommuHapPtShare)
> xc.cap_vmtrace = C.bool(x.CapVmtrace)
> +xc.cap_vpmu = C.bool(x.CapVpmu)
> 
>  return nil
>  }
> diff --git a/tools/golang/xenlight/types.gen.go b/tools/golang/xenlight/types.gen.go
> index 09a3bb67e2..45f2cba3d2 100644
> --- a/tools/golang/xenlight/types.gen.go
> +++ b/tools/golang/xenlight/types.gen.go
> @@ -1008,6 +1008,7 @@ CapHap bool
> CapShadow bool
> CapIommuHapPtShare bool
> CapVmtrace bool
> +CapVpmu bool
> }
> 
> type Connectorinfo struct {
> diff --git a/tools/include/libxl.h b/tools/include/libxl.h
> index b9ba16d698..ec5e3badae 100644
> --- a/tools/include/libxl.h
> +++ b/tools/include/libxl.h
> @@ -502,6 +502,12 @@
>  */
> #define LIBXL_HAVE_X86_MSR_RELAXED 1
> 
> +/*
> + * LIBXL_HAVE_PHYSINFO_CAP_VPMU indicates that libxl_physinfo has a cap_vpmu
> + * field, which indicates the availability of vPMU functionality.
> + */
> +#define LIBXL_HAVE_PHYSINFO_CAP_VPMU 1
> +
> /*
>  * libxl ABI compatibility
>  *
> diff --git a/tools/libs/light/libxl.c b/tools/libs/light/libxl.c
> index 204eb0be2d..a032723fde 100644
> --- a/tools/libs/light/libxl.c
> +++ b/tools/libs/light/libxl.c
> @@ -404,6 +404,7 @@ int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo *physinfo)
>         !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share);
>     physinfo->cap_vmtrace =
>         !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_vmtrace);
> +    physinfo->cap_vpmu = !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_vpmu);
> 
>     GC_FREE;
>     return 0;
> diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
> index 3f9fff653a..993e83acca 100644
> --- a/tools/libs/light/libxl_types.idl
> +++ b/tools/libs/light/libxl_types.idl
> @@ -1061,6 +1061,7 @@ libxl_physinfo = Struct("physinfo", [
>     ("cap_shadow", bool),
>     ("cap_iommu_hap_pt_share", bool),
>     ("cap_vmtrace", bool),
> +    ("cap_vpmu", bool),
>     ], dir=DIR_OUT)
> 
> libxl_connectorinfo = Struct("connectorinfo", [
> diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
> index a5588c643f..6da3ed3c6f 100644
> --- a/tools/ocaml/libs/xc/xenctrl.ml
> +++ b/tools/ocaml/libs/xc/xenctrl.ml
> @@ -121,6 +121,7 @@ type physinfo_cap_flag =
> 	| CAP_Shadow
> 	| CAP_IOMMU_HAP_PT_SHARE
> 	| CAP_Vmtrace
> +	| CAP_Vpmu
> 
> type physinfo =
> {
> diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
> index 6e94940a8a..b8faff6721 100644
> --- a/tools/ocaml/libs/xc/xenctrl.mli
> +++ b/tools/ocaml/libs/xc/xenctrl.mli
> @@ -106,6 +106,7 @@ type physinfo_cap_flag =
>   | CAP_Shadow
>   | CAP_IOMMU_HAP_PT_SHARE
>   | CAP_Vmtrace
> +  | CAP_Vpmu
> 
> type physinfo = {
>   threads_per_core : int;
> diff --git a/tools/xl/xl_info.c b/tools/xl/xl_info.c
> index 8383e4a6df..2c86b317b7 100644
> --- a/tools/xl/xl_info.c
> +++ b/tools/xl/xl_info.c
> @@ -210,7 +210,7 @@ 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%s%s%s%s\n",
> +    maybe_printf("virt_caps              :%s%s%s%s%s%s%s%s%s\n",
>          info.cap_pv ? " pv" : "",
>          info.cap_hvm ? " hvm" : "",
>          info.cap_hvm && info.cap_hvm_directio ? " hvm_directio" : "",
> @@ -218,7 +218,8 @@ static void output_physinfo(void)
>          info.cap_hap ? " hap" : "",
>          info.cap_shadow ? " shadow" : "",
>          info.cap_iommu_hap_pt_share ? " iommu_hap_pt_share" : "",
> -         info.cap_vmtrace ? " vmtrace" : ""
> +         info.cap_vmtrace ? " vmtrace" : "",
> +         info.cap_vpmu ? " vpmu" : ""
>         );
> 
>     vinfo = libxl_get_version_info(ctx);
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 6ee5d033b0..4d0e909eec 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -84,6 +84,8 @@ vcpu_info_t dummy_vcpu_info;
> 
> bool __read_mostly vmtrace_available;
> 
> +bool __read_mostly vpmu_is_available;
> +
> static void __domain_finalise_shutdown(struct domain *d)
> {
>     struct vcpu *v;
> diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
> index 3558641cd9..6e7189bb3c 100644
> --- a/xen/common/sysctl.c
> +++ b/xen/common/sysctl.c
> @@ -280,6 +280,9 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
>         if ( vmtrace_available )
>             pi->capabilities |= XEN_SYSCTL_PHYSCAP_vmtrace;
> 
> +        if ( vpmu_is_available )
> +            pi->capabilities |= XEN_SYSCTL_PHYSCAP_vpmu;
> +
>         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 039ccf885c..b794c9d351 100644
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -100,10 +100,12 @@ struct xen_sysctl_tbuf_op {
> #define _XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share 5
> #define XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share  \
>     (1u << _XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share)
> -#define XEN_SYSCTL_PHYSCAP_vmtrace       (1 << 6)
> +#define XEN_SYSCTL_PHYSCAP_vmtrace       (1u<<6)
> +/* The platform supports vPMU. */
> +#define XEN_SYSCTL_PHYSCAP_vpmu          (1u<<7)
> 
> /* Max XEN_SYSCTL_PHYSCAP_* constant.  Used for ABI checking. */
> -#define XEN_SYSCTL_PHYSCAP_MAX XEN_SYSCTL_PHYSCAP_vmtrace
> +#define XEN_SYSCTL_PHYSCAP_MAX XEN_SYSCTL_PHYSCAP_vpmu
> 
> struct xen_sysctl_physinfo {
>     uint32_t threads_per_core;
> diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
> index 1708c36964..160c8dbdab 100644
> --- a/xen/include/xen/domain.h
> +++ b/xen/include/xen/domain.h
> @@ -133,4 +133,6 @@ static inline void vnuma_destroy(struct vnuma_info *vnuma) { ASSERT(!vnuma); }
> 
> extern bool vmtrace_available;
> 
> +extern bool vpmu_is_available;
> +
> #endif /* __XEN_DOMAIN_H__ */
> -- 
> 2.29.0
> 
>