* [PATCH v2 1/3] xen+tools: Introduce XEN_SYSCTL_PHYSCAP_vpmu
2021-10-06 10:58 [PATCH v2 0/3] Expose PMU to the guests Michal Orzel
@ 2021-10-06 10:58 ` Michal Orzel
2021-10-07 4:45 ` Stefano Stabellini
2021-10-07 7:59 ` Jan Beulich
2021-10-06 10:58 ` [PATCH v2 2/3] xen/arm: Check for PMU platform support Michal Orzel
2021-10-06 10:58 ` [PATCH v2 3/3] xen: Expose the PMU to the guests Michal Orzel
2 siblings, 2 replies; 13+ messages in thread
From: Michal Orzel @ 2021-10-06 10:58 UTC (permalink / raw)
To: xen-devel
Cc: George Dunlap, Nick Rosbrook, Ian Jackson, Wei Liu,
Andrew Cooper, Jan Beulich, Julien Grall, Stefano Stabellini,
Anthony PERARD, Juergen Gross, Christian Lindig, David Scott,
bertrand.marquis
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 uses (1<<6)
instead of (1u<<6) and does not follow the standard
of using separate macro for a flag field.
Signed-off-by: Michal Orzel <michal.orzel@arm.com>
---
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 | 8 ++++++--
xen/include/xen/domain.h | 2 ++
12 files changed, 29 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..546e673409 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -100,10 +100,14 @@ 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 6
+#define XEN_SYSCTL_PHYSCAP_vmtrace (1u<<_XEN_SYSCTL_PHYSCAP_vmtrace)
+/* The platform supports vPMU. */
+#define _XEN_SYSCTL_PHYSCAP_vpmu 7
+#define XEN_SYSCTL_PHYSCAP_vpmu (1u<<_XEN_SYSCTL_PHYSCAP_vpmu)
/* 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 1/3] xen+tools: Introduce XEN_SYSCTL_PHYSCAP_vpmu
2021-10-06 10:58 ` [PATCH v2 1/3] xen+tools: Introduce XEN_SYSCTL_PHYSCAP_vpmu Michal Orzel
@ 2021-10-07 4:45 ` Stefano Stabellini
2021-10-07 7:59 ` Jan Beulich
1 sibling, 0 replies; 13+ messages in thread
From: Stefano Stabellini @ 2021-10-07 4:45 UTC (permalink / raw)
To: Michal Orzel
Cc: xen-devel, George Dunlap, Nick Rosbrook, Ian Jackson, Wei Liu,
Andrew Cooper, Jan Beulich, Julien Grall, Stefano Stabellini,
Anthony PERARD, Juergen Gross, Christian Lindig, David Scott,
bertrand.marquis
On Wed, 6 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 uses (1<<6)
> instead of (1u<<6) and does not follow the standard
> of using separate macro for a flag field.
>
> Signed-off-by: Michal Orzel <michal.orzel@arm.com>
From the xen/ part:
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
> 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 | 8 ++++++--
> xen/include/xen/domain.h | 2 ++
> 12 files changed, 29 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..546e673409 100644
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -100,10 +100,14 @@ 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 6
> +#define XEN_SYSCTL_PHYSCAP_vmtrace (1u<<_XEN_SYSCTL_PHYSCAP_vmtrace)
> +/* The platform supports vPMU. */
> +#define _XEN_SYSCTL_PHYSCAP_vpmu 7
> +#define XEN_SYSCTL_PHYSCAP_vpmu (1u<<_XEN_SYSCTL_PHYSCAP_vpmu)
>
> /* 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
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 1/3] xen+tools: Introduce XEN_SYSCTL_PHYSCAP_vpmu
2021-10-06 10:58 ` [PATCH v2 1/3] xen+tools: Introduce XEN_SYSCTL_PHYSCAP_vpmu Michal Orzel
2021-10-07 4:45 ` Stefano Stabellini
@ 2021-10-07 7:59 ` Jan Beulich
2021-10-07 8:02 ` Michal Orzel
1 sibling, 1 reply; 13+ messages in thread
From: Jan Beulich @ 2021-10-07 7:59 UTC (permalink / raw)
To: Michal Orzel
Cc: George Dunlap, Nick Rosbrook, Ian Jackson, Wei Liu,
Andrew Cooper, Julien Grall, Stefano Stabellini, Anthony PERARD,
Juergen Gross, Christian Lindig, David Scott, bertrand.marquis,
xen-devel
On 06.10.2021 12:58, 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 uses (1<<6)
> instead of (1u<<6) and does not follow the standard
> of using separate macro for a flag field.
While adding the u suffix is fine with me, iirc not introducing
_XEN_SYSCTL_PHYSCAP_vmtrace was a specific review request at the
time. I would similarly ask to avoid introduction of
_XEN_SYSCTL_PHYSCAP_vpmu here, for it not being consumed by
anything other than XEN_SYSCTL_PHYSCAP_vpmu's definition.
Jan
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/3] xen+tools: Introduce XEN_SYSCTL_PHYSCAP_vpmu
2021-10-07 7:59 ` Jan Beulich
@ 2021-10-07 8:02 ` Michal Orzel
2021-10-07 8:05 ` Jan Beulich
0 siblings, 1 reply; 13+ messages in thread
From: Michal Orzel @ 2021-10-07 8:02 UTC (permalink / raw)
To: Jan Beulich
Cc: George Dunlap, Nick Rosbrook, Ian Jackson, Wei Liu,
Andrew Cooper, Julien Grall, Stefano Stabellini, Anthony PERARD,
Juergen Gross, Christian Lindig, David Scott, bertrand.marquis,
xen-devel
Hi,
On 07.10.2021 09:59, Jan Beulich wrote:
> On 06.10.2021 12:58, 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 uses (1<<6)
>> instead of (1u<<6) and does not follow the standard
>> of using separate macro for a flag field.
>
> While adding the u suffix is fine with me, iirc not introducing
> _XEN_SYSCTL_PHYSCAP_vmtrace was a specific review request at the
> time. I would similarly ask to avoid introduction of
> _XEN_SYSCTL_PHYSCAP_vpmu here, for it not being consumed by
> anything other than XEN_SYSCTL_PHYSCAP_vpmu's definition.
>
> Jan
>
Ok I did not know that. I thought we should stick to the previous standard.
Is this something that can be fixed on commit or should I send
a v3 only for that?
Cheers,
Michal
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/3] xen+tools: Introduce XEN_SYSCTL_PHYSCAP_vpmu
2021-10-07 8:02 ` Michal Orzel
@ 2021-10-07 8:05 ` Jan Beulich
0 siblings, 0 replies; 13+ messages in thread
From: Jan Beulich @ 2021-10-07 8:05 UTC (permalink / raw)
To: Michal Orzel
Cc: George Dunlap, Nick Rosbrook, Ian Jackson, Wei Liu,
Andrew Cooper, Julien Grall, Stefano Stabellini, Anthony PERARD,
Juergen Gross, Christian Lindig, David Scott, bertrand.marquis,
xen-devel
On 07.10.2021 10:02, Michal Orzel wrote:
> On 07.10.2021 09:59, Jan Beulich wrote:
>> On 06.10.2021 12:58, 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 uses (1<<6)
>>> instead of (1u<<6) and does not follow the standard
>>> of using separate macro for a flag field.
>>
>> While adding the u suffix is fine with me, iirc not introducing
>> _XEN_SYSCTL_PHYSCAP_vmtrace was a specific review request at the
>> time. I would similarly ask to avoid introduction of
>> _XEN_SYSCTL_PHYSCAP_vpmu here, for it not being consumed by
>> anything other than XEN_SYSCTL_PHYSCAP_vpmu's definition.
>>
> Ok I did not know that. I thought we should stick to the previous standard.
> Is this something that can be fixed on commit or should I send
> a v3 only for that?
You may want to wait for further review feedback first, especially on
the tools side. Considering adjustments to subsequent patches I would
think re-sending will ultimately be better.
Jan
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 2/3] xen/arm: Check for PMU platform support
2021-10-06 10:58 [PATCH v2 0/3] Expose PMU to the guests Michal Orzel
2021-10-06 10:58 ` [PATCH v2 1/3] xen+tools: Introduce XEN_SYSCTL_PHYSCAP_vpmu Michal Orzel
@ 2021-10-06 10:58 ` Michal Orzel
2021-10-07 4:42 ` Stefano Stabellini
2021-10-06 10:58 ` [PATCH v2 3/3] xen: Expose the PMU to the guests Michal Orzel
2 siblings, 1 reply; 13+ messages in thread
From: Michal Orzel @ 2021-10-06 10:58 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk, bertrand.marquis
ID_AA64DFR0_EL1/ID_DFR0_EL1 registers provide
information about PMU support. Replace structure
dbg64/dbg32 with a union and fill in all the
register fields according to document:
ARM Architecture Registers(DDI 0595, 2021-06).
Add macros boot_dbg_feature64/boot_dbg_feature32
to check for a debug feature. Add macro
cpu_has_pmu to check for PMU support.
Signed-off-by: Michal Orzel <michal.orzel@arm.com>
---
xen/include/asm-arm/cpufeature.h | 49 ++++++++++++++++++++++++++++++--
1 file changed, 47 insertions(+), 2 deletions(-)
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index 5ca09b0bff..4fce23844d 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -4,6 +4,7 @@
#ifdef CONFIG_ARM_64
#define cpu_feature64(c, feat) ((c)->pfr64.feat)
#define boot_cpu_feature64(feat) (system_cpuinfo.pfr64.feat)
+#define boot_dbg_feature64(feat) (system_cpuinfo.dbg64.feat)
#define cpu_feature64_has_el0_32(c) (cpu_feature64(c, el0) == 2)
@@ -22,6 +23,7 @@
#define cpu_feature32(c, feat) ((c)->pfr32.feat)
#define boot_cpu_feature32(feat) (system_cpuinfo.pfr32.feat)
+#define boot_dbg_feature32(feat) (system_cpuinfo.dbg32.feat)
#define cpu_has_arm (boot_cpu_feature32(arm) == 1)
#define cpu_has_thumb (boot_cpu_feature32(thumb) >= 1)
@@ -32,8 +34,10 @@
#ifdef CONFIG_ARM_32
#define cpu_has_gentimer (boot_cpu_feature32(gentimer) == 1)
+#define cpu_has_pmu (boot_dbg_feature32(perfmon) >= 1)
#else
#define cpu_has_gentimer (1)
+#define cpu_has_pmu (boot_dbg_feature64(pmu_ver) >= 1)
#endif
#define cpu_has_security (boot_cpu_feature32(security) > 0)
@@ -181,8 +185,28 @@ struct cpuinfo_arm {
};
} pfr64;
- struct {
+ union {
register_t bits[2];
+ struct {
+ /* DFR0 */
+ unsigned long debug_ver:4;
+ unsigned long trace_ver:4;
+ unsigned long pmu_ver:4;
+ unsigned long brps:4;
+ unsigned long __res0:4;
+ unsigned long wrps:4;
+ unsigned long __res1:4;
+ unsigned long ctx_cmps:4;
+ unsigned long pms_ver:4;
+ unsigned long double_lock:4;
+ unsigned long trace_filt:4;
+ unsigned long __res2:4;
+ unsigned long mtpmu:4;
+ unsigned long __res3:12;
+
+ /* DFR1 */
+ unsigned long __res4:64;
+ };
} dbg64;
struct {
@@ -321,8 +345,29 @@ struct cpuinfo_arm {
};
} pfr32;
- struct {
+ union {
register_t bits[2];
+ struct {
+ /* DFR0 */
+ unsigned long copdbg:4;
+ unsigned long copsdbg:4;
+ unsigned long mmapdbg:4;
+ unsigned long coptrc:4;
+ unsigned long mmaptrc:4;
+ unsigned long mprofdbg:4;
+ unsigned long perfmon:4;
+ unsigned long tracefilt:4;
+#ifdef CONFIG_ARM_64
+ unsigned long __res0:32;
+#endif
+
+ /* DFR1 */
+ unsigned long mtpmu:4;
+ unsigned long __res1:28;
+#ifdef CONFIG_ARM_64
+ unsigned long __res2:32;
+#endif
+ };
} dbg32;
struct {
--
2.29.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 2/3] xen/arm: Check for PMU platform support
2021-10-06 10:58 ` [PATCH v2 2/3] xen/arm: Check for PMU platform support Michal Orzel
@ 2021-10-07 4:42 ` Stefano Stabellini
0 siblings, 0 replies; 13+ messages in thread
From: Stefano Stabellini @ 2021-10-07 4:42 UTC (permalink / raw)
To: Michal Orzel
Cc: xen-devel, Stefano Stabellini, Julien Grall, Volodymyr Babchuk,
bertrand.marquis
On Wed, 6 Oct 2021, Michal Orzel wrote:
> ID_AA64DFR0_EL1/ID_DFR0_EL1 registers provide
> information about PMU support. Replace structure
> dbg64/dbg32 with a union and fill in all the
> register fields according to document:
> ARM Architecture Registers(DDI 0595, 2021-06).
>
> Add macros boot_dbg_feature64/boot_dbg_feature32
> to check for a debug feature. Add macro
> cpu_has_pmu to check for PMU support.
>
> Signed-off-by: Michal Orzel <michal.orzel@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
> xen/include/asm-arm/cpufeature.h | 49 ++++++++++++++++++++++++++++++--
> 1 file changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
> index 5ca09b0bff..4fce23844d 100644
> --- a/xen/include/asm-arm/cpufeature.h
> +++ b/xen/include/asm-arm/cpufeature.h
> @@ -4,6 +4,7 @@
> #ifdef CONFIG_ARM_64
> #define cpu_feature64(c, feat) ((c)->pfr64.feat)
> #define boot_cpu_feature64(feat) (system_cpuinfo.pfr64.feat)
> +#define boot_dbg_feature64(feat) (system_cpuinfo.dbg64.feat)
>
> #define cpu_feature64_has_el0_32(c) (cpu_feature64(c, el0) == 2)
>
> @@ -22,6 +23,7 @@
>
> #define cpu_feature32(c, feat) ((c)->pfr32.feat)
> #define boot_cpu_feature32(feat) (system_cpuinfo.pfr32.feat)
> +#define boot_dbg_feature32(feat) (system_cpuinfo.dbg32.feat)
>
> #define cpu_has_arm (boot_cpu_feature32(arm) == 1)
> #define cpu_has_thumb (boot_cpu_feature32(thumb) >= 1)
> @@ -32,8 +34,10 @@
>
> #ifdef CONFIG_ARM_32
> #define cpu_has_gentimer (boot_cpu_feature32(gentimer) == 1)
> +#define cpu_has_pmu (boot_dbg_feature32(perfmon) >= 1)
> #else
> #define cpu_has_gentimer (1)
> +#define cpu_has_pmu (boot_dbg_feature64(pmu_ver) >= 1)
> #endif
> #define cpu_has_security (boot_cpu_feature32(security) > 0)
>
> @@ -181,8 +185,28 @@ struct cpuinfo_arm {
> };
> } pfr64;
>
> - struct {
> + union {
> register_t bits[2];
> + struct {
> + /* DFR0 */
> + unsigned long debug_ver:4;
> + unsigned long trace_ver:4;
> + unsigned long pmu_ver:4;
> + unsigned long brps:4;
> + unsigned long __res0:4;
> + unsigned long wrps:4;
> + unsigned long __res1:4;
> + unsigned long ctx_cmps:4;
> + unsigned long pms_ver:4;
> + unsigned long double_lock:4;
> + unsigned long trace_filt:4;
> + unsigned long __res2:4;
> + unsigned long mtpmu:4;
> + unsigned long __res3:12;
> +
> + /* DFR1 */
> + unsigned long __res4:64;
> + };
> } dbg64;
>
> struct {
> @@ -321,8 +345,29 @@ struct cpuinfo_arm {
> };
> } pfr32;
>
> - struct {
> + union {
> register_t bits[2];
> + struct {
> + /* DFR0 */
> + unsigned long copdbg:4;
> + unsigned long copsdbg:4;
> + unsigned long mmapdbg:4;
> + unsigned long coptrc:4;
> + unsigned long mmaptrc:4;
> + unsigned long mprofdbg:4;
> + unsigned long perfmon:4;
> + unsigned long tracefilt:4;
> +#ifdef CONFIG_ARM_64
> + unsigned long __res0:32;
> +#endif
> +
> + /* DFR1 */
> + unsigned long mtpmu:4;
> + unsigned long __res1:28;
> +#ifdef CONFIG_ARM_64
> + unsigned long __res2:32;
> +#endif
> + };
> } dbg32;
>
> struct {
> --
> 2.29.0
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 3/3] xen: Expose the PMU to the guests
2021-10-06 10:58 [PATCH v2 0/3] Expose PMU to the guests Michal Orzel
2021-10-06 10:58 ` [PATCH v2 1/3] xen+tools: Introduce XEN_SYSCTL_PHYSCAP_vpmu Michal Orzel
2021-10-06 10:58 ` [PATCH v2 2/3] xen/arm: Check for PMU platform support Michal Orzel
@ 2021-10-06 10:58 ` Michal Orzel
2021-10-06 12:31 ` Christian Lindig
2021-10-07 8:03 ` Jan Beulich
2 siblings, 2 replies; 13+ messages in thread
From: Michal Orzel @ 2021-10-06 10:58 UTC (permalink / raw)
To: xen-devel
Cc: Ian Jackson, Wei Liu, Andrew Cooper, George Dunlap, Jan Beulich,
Julien Grall, Stefano Stabellini, Nick Rosbrook, Anthony PERARD,
Juergen Gross, Christian Lindig, David Scott, Volodymyr Babchuk,
Roger Pau Monné,
bertrand.marquis
Add parameter vpmu to xl domain configuration syntax
to enable the access to PMU registers by disabling
the PMU traps(currently only for ARM).
The current status is that the PMU registers are not
virtualized and the physical registers are directly
accessible when this parameter is enabled. There is no
interrupt support and Xen will not save/restore the
register values on context switches.
Please note that this feature is experimental.
Signed-off-by: Michal Orzel <michal.orzel@arm.com>
Signed-off-by: Julien Grall <julien@xen.org>
---
docs/man/xl.cfg.5.pod.in | 17 +++++++++++++++++
tools/golang/xenlight/helpers.gen.go | 6 ++++++
tools/golang/xenlight/types.gen.go | 1 +
tools/include/libxl.h | 6 ++++++
tools/libs/light/libxl_create.c | 4 ++++
tools/libs/light/libxl_types.idl | 2 ++
tools/ocaml/libs/xc/xenctrl.ml | 1 +
tools/ocaml/libs/xc/xenctrl.mli | 1 +
tools/xl/xl_parse.c | 2 ++
xen/arch/arm/domain.c | 10 ++++++++--
xen/arch/arm/setup.c | 1 +
xen/arch/x86/domain.c | 6 ++++++
xen/common/domain.c | 9 ++++++++-
xen/include/asm-arm/domain.h | 1 +
xen/include/public/domctl.h | 5 ++++-
15 files changed, 68 insertions(+), 4 deletions(-)
diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index 4b1e3028d2..55c4881205 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -690,6 +690,23 @@ default.
B<NOTE>: Acceptable values are platform specific. For Intel Processor
Trace, this value must be a power of 2 between 4k and 16M.
+=item B<vpmu=BOOLEAN>
+
+Currently ARM only.
+
+Specifies whether to enable the access to PMU registers by disabling
+the PMU traps.
+
+The PMU registers are not virtualized and the physical registers are directly
+accessible when this parameter is enabled. There is no interrupt support and
+Xen will not save/restore the register values on context switches.
+
+vPMU, by design and purpose, exposes system level performance
+information to the guest. Only to be used by sufficiently privileged
+domains. This feature is currently in experimental state.
+
+If this option is not specified then it will default to B<false>.
+
=back
=head2 Devices
diff --git a/tools/golang/xenlight/helpers.gen.go b/tools/golang/xenlight/helpers.gen.go
index c8669837d8..2449580bad 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -1119,6 +1119,9 @@ return fmt.Errorf("converting field ArchX86.MsrRelaxed: %v", err)
}
x.Altp2M = Altp2MMode(xc.altp2m)
x.VmtraceBufKb = int(xc.vmtrace_buf_kb)
+if err := x.Vpmu.fromC(&xc.vpmu);err != nil {
+return fmt.Errorf("converting field Vpmu: %v", err)
+}
return nil}
@@ -1600,6 +1603,9 @@ return fmt.Errorf("converting field ArchX86.MsrRelaxed: %v", err)
}
xc.altp2m = C.libxl_altp2m_mode(x.Altp2M)
xc.vmtrace_buf_kb = C.int(x.VmtraceBufKb)
+if err := x.Vpmu.toC(&xc.vpmu); err != nil {
+return fmt.Errorf("converting field Vpmu: %v", err)
+}
return nil
}
diff --git a/tools/golang/xenlight/types.gen.go b/tools/golang/xenlight/types.gen.go
index 45f2cba3d2..b2e8bd1a85 100644
--- a/tools/golang/xenlight/types.gen.go
+++ b/tools/golang/xenlight/types.gen.go
@@ -521,6 +521,7 @@ MsrRelaxed Defbool
}
Altp2M Altp2MMode
VmtraceBufKb int
+Vpmu Defbool
}
type DomainBuildInfoTypeUnion interface {
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index ec5e3badae..ee73eb06f1 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -508,6 +508,12 @@
*/
#define LIBXL_HAVE_PHYSINFO_CAP_VPMU 1
+/*
+ * LIBXL_HAVE_VPMU indicates that libxl_domain_build_info has a vpmu parameter,
+ * which allows to enable the access to PMU registers.
+ */
+#define LIBXL_HAVE_VPMU 1
+
/*
* libxl ABI compatibility
*
diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
index e356b2106d..d123c2f703 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -91,6 +91,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
}
libxl_defbool_setdefault(&b_info->device_model_stubdomain, false);
+ libxl_defbool_setdefault(&b_info->vpmu, false);
if (libxl_defbool_val(b_info->device_model_stubdomain) &&
!b_info->device_model_ssidref)
@@ -622,6 +623,9 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
create.flags |= XEN_DOMCTL_CDF_nested_virt;
}
+ if ( libxl_defbool_val(b_info->vpmu) )
+ create.flags |= XEN_DOMCTL_CDF_vpmu;
+
assert(info->passthrough != LIBXL_PASSTHROUGH_DEFAULT);
LOG(DETAIL, "passthrough: %s",
libxl_passthrough_to_string(info->passthrough));
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index 993e83acca..b96fb5c47e 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -655,6 +655,8 @@ libxl_domain_build_info = Struct("domain_build_info",[
# Use zero value to disable this feature.
("vmtrace_buf_kb", integer),
+ ("vpmu", libxl_defbool),
+
], dir=DIR_IN,
copy_deprecated_fn="libxl__domain_build_info_copy_deprecated",
)
diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
index 6da3ed3c6f..addcf4cc59 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -69,6 +69,7 @@ type domain_create_flag =
| CDF_XS_DOMAIN
| CDF_IOMMU
| CDF_NESTED_VIRT
+ | CDF_VPMU
type domain_create_iommu_opts =
| IOMMU_NO_SHAREPT
diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
index b8faff6721..0a5ce529e9 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -62,6 +62,7 @@ type domain_create_flag =
| CDF_XS_DOMAIN
| CDF_IOMMU
| CDF_NESTED_VIRT
+ | CDF_VPMU
type domain_create_iommu_opts =
| IOMMU_NO_SHAREPT
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 17dddb4cd5..c503b9be00 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -2750,6 +2750,8 @@ skip_usbdev:
"If it fixes an issue you are having please report to "
"xen-devel@lists.xenproject.org.\n");
+ xlu_cfg_get_defbool(config, "vpmu", &b_info->vpmu, 0);
+
xlu_cfg_destroy(config);
}
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 19c756ac3d..2dd474da09 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -276,6 +276,8 @@ static void ctxt_switch_to(struct vcpu *n)
* timer. The interrupt needs to be injected into the guest. */
WRITE_SYSREG(n->arch.cntkctl, CNTKCTL_EL1);
virt_timer_restore(n);
+
+ WRITE_SYSREG(n->arch.mdcr_el2, MDCR_EL2);
}
/* Update per-VCPU guest runstate shared memory area (if registered). */
@@ -586,6 +588,10 @@ int arch_vcpu_create(struct vcpu *v)
v->arch.hcr_el2 = get_default_hcr_flags();
+ v->arch.mdcr_el2 = HDCR_TDRA | HDCR_TDOSA | HDCR_TDA;
+ if ( !(v->domain->options & XEN_DOMCTL_CDF_vpmu) )
+ v->arch.mdcr_el2 |= HDCR_TPM | HDCR_TPMCR;
+
if ( (rc = vcpu_vgic_init(v)) != 0 )
goto fail;
@@ -622,8 +628,8 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
{
unsigned int max_vcpus;
- /* HVM and HAP must be set. IOMMU may or may not be */
- if ( (config->flags & ~XEN_DOMCTL_CDF_iommu) !=
+ /* HVM and HAP must be set. IOMMU and PMU may or may not be */
+ if ( (config->flags & ~(XEN_DOMCTL_CDF_iommu | XEN_DOMCTL_CDF_vpmu)) !=
(XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap) )
{
dprintk(XENLOG_INFO, "Unsupported configuration %#x\n",
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 49dc90d198..85386a765a 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -108,6 +108,7 @@ static void __init processor_id(void)
identify_cpu(c);
current_cpu_data = *c;
+ vpmu_is_available = cpu_has_pmu;
if ( c->midr.implementer < ARRAY_SIZE(processor_implementers) &&
processor_implementers[c->midr.implementer] )
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index ef1812dc14..406824a7a5 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -692,6 +692,12 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
return -EINVAL;
}
+ if ( config->flags & XEN_DOMCTL_CDF_vpmu )
+ {
+ dprintk(XENLOG_INFO, "vpmu support not ready yet\n");
+ return -EINVAL;
+ }
+
return 0;
}
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 4d0e909eec..37d8c55c69 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -480,12 +480,13 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
bool hvm = config->flags & XEN_DOMCTL_CDF_hvm;
bool hap = config->flags & XEN_DOMCTL_CDF_hap;
bool iommu = config->flags & XEN_DOMCTL_CDF_iommu;
+ bool vpmu = config->flags & XEN_DOMCTL_CDF_vpmu;
if ( config->flags &
~(XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap |
XEN_DOMCTL_CDF_s3_integrity | XEN_DOMCTL_CDF_oos_off |
XEN_DOMCTL_CDF_xs_domain | XEN_DOMCTL_CDF_iommu |
- XEN_DOMCTL_CDF_nested_virt) )
+ XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu) )
{
dprintk(XENLOG_INFO, "Unknown CDF flags %#x\n", config->flags);
return -EINVAL;
@@ -534,6 +535,12 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
return -EINVAL;
}
+ if ( vpmu && !vpmu_is_available )
+ {
+ dprintk(XENLOG_INFO, "vpmu requested but not available\n");
+ return -EINVAL;
+ }
+
return arch_sanitise_domain_config(config);
}
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index c9277b5c6d..14e575288f 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -166,6 +166,7 @@ struct arch_vcpu
/* HYP configuration */
register_t hcr_el2;
+ register_t mdcr_el2;
uint32_t teecr, teehbr; /* ThumbEE, 32-bit guests only */
#ifdef CONFIG_ARM_32
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 96696e3842..8f2a07cfe8 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -70,9 +70,12 @@ struct xen_domctl_createdomain {
#define XEN_DOMCTL_CDF_iommu (1U<<_XEN_DOMCTL_CDF_iommu)
#define _XEN_DOMCTL_CDF_nested_virt 6
#define XEN_DOMCTL_CDF_nested_virt (1U << _XEN_DOMCTL_CDF_nested_virt)
+/* Should we expose the vPMU to the guest? */
+#define _XEN_DOMCTL_CDF_vpmu 7
+#define XEN_DOMCTL_CDF_vpmu (1U << _XEN_DOMCTL_CDF_vpmu)
/* Max XEN_DOMCTL_CDF_* constant. Used for ABI checking. */
-#define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_nested_virt
+#define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_vpmu
uint32_t flags;
--
2.29.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 3/3] xen: Expose the PMU to the guests
2021-10-06 10:58 ` [PATCH v2 3/3] xen: Expose the PMU to the guests Michal Orzel
@ 2021-10-06 12:31 ` Christian Lindig
2021-10-07 8:03 ` Jan Beulich
1 sibling, 0 replies; 13+ messages in thread
From: Christian Lindig @ 2021-10-06 12:31 UTC (permalink / raw)
To: Michal Orzel
Cc: xen-devel, Ian Jackson, Wei Liu, Andrew Cooper, George Dunlap,
Jan Beulich, Julien Grall, Stefano Stabellini, Nick Rosbrook,
Anthony Perard, Juergen Gross, David Scott, Volodymyr Babchuk,
Roger Pau Monne, bertrand.marquis
[-- Attachment #1: Type: text/plain, Size: 1593 bytes --]
On 6 Oct 2021, at 11:58, Michal Orzel <michal.orzel@arm.com<mailto:michal.orzel@arm.com>> wrote:
Add parameter vpmu to xl domain configuration syntax
to enable the access to PMU registers by disabling
the PMU traps(currently only for ARM).
The current status is that the PMU registers are not
virtualized and the physical registers are directly
accessible when this parameter is enabled. There is no
interrupt support and Xen will not save/restore the
register values on context switches.
Please note that this feature is experimental.
Signed-off-by: Michal Orzel <michal.orzel@arm.com<mailto:michal.orzel@arm.com>>
Signed-off-by: Julien Grall <julien@xen.org<mailto:julien@xen.org>>
---
docs/man/xl.cfg.5.pod.in | 17 +++++++++++++++++
tools/golang/xenlight/helpers.gen.go | 6 ++++++
tools/golang/xenlight/types.gen.go | 1 +
tools/include/libxl.h | 6 ++++++
tools/libs/light/libxl_create.c | 4 ++++
tools/libs/light/libxl_types.idl | 2 ++
tools/ocaml/libs/xc/xenctrl.ml | 1 +
tools/ocaml/libs/xc/xenctrl.mli | 1 +
tools/xl/xl_parse.c | 2 ++
xen/arch/arm/domain.c | 10 ++++++++--
xen/arch/arm/setup.c | 1 +
xen/arch/x86/domain.c | 6 ++++++
xen/common/domain.c | 9 ++++++++-
xen/include/asm-arm/domain.h | 1 +
xen/include/public/domctl.h | 5 ++++-
15 files changed, 68 insertions(+), 4 deletions(-)
Acked-by: Christian Lindig <christian.lindig@citrix.com<mailto:christian.lindig@citrix.com>>
[-- Attachment #2: Type: text/html, Size: 25756 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/3] xen: Expose the PMU to the guests
2021-10-06 10:58 ` [PATCH v2 3/3] xen: Expose the PMU to the guests Michal Orzel
2021-10-06 12:31 ` Christian Lindig
@ 2021-10-07 8:03 ` Jan Beulich
2021-10-07 8:21 ` Michal Orzel
1 sibling, 1 reply; 13+ messages in thread
From: Jan Beulich @ 2021-10-07 8:03 UTC (permalink / raw)
To: Michal Orzel
Cc: Ian Jackson, Wei Liu, Andrew Cooper, George Dunlap, Julien Grall,
Stefano Stabellini, Nick Rosbrook, Anthony PERARD, Juergen Gross,
Christian Lindig, David Scott, Volodymyr Babchuk,
Roger Pau Monné,
bertrand.marquis, xen-devel
On 06.10.2021 12:58, Michal Orzel wrote:
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -692,6 +692,12 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
> return -EINVAL;
> }
>
> + if ( config->flags & XEN_DOMCTL_CDF_vpmu )
> + {
> + dprintk(XENLOG_INFO, "vpmu support not ready yet\n");
> + return -EINVAL;
> + }
I consider this message potentially misleading (as x86 does have vPMU
support, it merely doesn't get enabled this way). But isn't this redundant
with ...
> @@ -534,6 +535,12 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
> return -EINVAL;
> }
>
> + if ( vpmu && !vpmu_is_available )
> + {
> + dprintk(XENLOG_INFO, "vpmu requested but not available\n");
> + return -EINVAL;
> + }
... this? (This message is again potentially misleading.)
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -70,9 +70,12 @@ struct xen_domctl_createdomain {
> #define XEN_DOMCTL_CDF_iommu (1U<<_XEN_DOMCTL_CDF_iommu)
> #define _XEN_DOMCTL_CDF_nested_virt 6
> #define XEN_DOMCTL_CDF_nested_virt (1U << _XEN_DOMCTL_CDF_nested_virt)
> +/* Should we expose the vPMU to the guest? */
> +#define _XEN_DOMCTL_CDF_vpmu 7
> +#define XEN_DOMCTL_CDF_vpmu (1U << _XEN_DOMCTL_CDF_vpmu)
Like for the earlier patch, I think we should stop with the bad habit of
defining both the bit position and the mask separately.
Jan
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 3/3] xen: Expose the PMU to the guests
2021-10-07 8:03 ` Jan Beulich
@ 2021-10-07 8:21 ` Michal Orzel
2021-10-07 8:34 ` Jan Beulich
0 siblings, 1 reply; 13+ messages in thread
From: Michal Orzel @ 2021-10-07 8:21 UTC (permalink / raw)
To: Jan Beulich
Cc: Ian Jackson, Wei Liu, Andrew Cooper, George Dunlap, Julien Grall,
Stefano Stabellini, Nick Rosbrook, Anthony PERARD, Juergen Gross,
Christian Lindig, David Scott, Volodymyr Babchuk,
Roger Pau Monné,
bertrand.marquis, xen-devel
Hi Jan,
On 07.10.2021 10:03, Jan Beulich wrote:
> On 06.10.2021 12:58, Michal Orzel wrote:
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -692,6 +692,12 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
>> return -EINVAL;
>> }
>>
>> + if ( config->flags & XEN_DOMCTL_CDF_vpmu )
>> + {
>> + dprintk(XENLOG_INFO, "vpmu support not ready yet\n");
>> + return -EINVAL;
>> + }
>
> I consider this message potentially misleading (as x86 does have vPMU
> support, it merely doesn't get enabled this way). But isn't this redundant
> with ...
>
>> @@ -534,6 +535,12 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
>> return -EINVAL;
>> }
>>
>> + if ( vpmu && !vpmu_is_available )
>> + {
>> + dprintk(XENLOG_INFO, "vpmu requested but not available\n");
>> + return -EINVAL;
>> + }
>
> ... this? (This message is again potentially misleading.)
>
Ok. vpmu_is_available is false for x86 so the check in x86's arch_sanitise_domain_config is redundant.
I will fix it. When it comes to the message itself "vpmu requested but not available".
Does the following sound better for you?
"vpmu requested but the platform does not support it"
If not, can you please suggest a better message?
>> --- a/xen/include/public/domctl.h
>> +++ b/xen/include/public/domctl.h
>> @@ -70,9 +70,12 @@ struct xen_domctl_createdomain {
>> #define XEN_DOMCTL_CDF_iommu (1U<<_XEN_DOMCTL_CDF_iommu)
>> #define _XEN_DOMCTL_CDF_nested_virt 6
>> #define XEN_DOMCTL_CDF_nested_virt (1U << _XEN_DOMCTL_CDF_nested_virt)
>> +/* Should we expose the vPMU to the guest? */
>> +#define _XEN_DOMCTL_CDF_vpmu 7
>> +#define XEN_DOMCTL_CDF_vpmu (1U << _XEN_DOMCTL_CDF_vpmu)
>
> Like for the earlier patch, I think we should stop with the bad habit of
> defining both the bit position and the mask separately.
>
Ok. I can change it. However there are lots of other places in Xen (e.g. asm-x86/hvm/irq.h)
when such methodology is used. But I agree with you and I will change it.
> Jan
>
Cheers,
Michal
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 3/3] xen: Expose the PMU to the guests
2021-10-07 8:21 ` Michal Orzel
@ 2021-10-07 8:34 ` Jan Beulich
0 siblings, 0 replies; 13+ messages in thread
From: Jan Beulich @ 2021-10-07 8:34 UTC (permalink / raw)
To: Michal Orzel
Cc: Ian Jackson, Wei Liu, Andrew Cooper, George Dunlap, Julien Grall,
Stefano Stabellini, Nick Rosbrook, Anthony PERARD, Juergen Gross,
Christian Lindig, David Scott, Volodymyr Babchuk,
Roger Pau Monné,
bertrand.marquis, xen-devel
On 07.10.2021 10:21, Michal Orzel wrote:
> Hi Jan,
>
> On 07.10.2021 10:03, Jan Beulich wrote:
>> On 06.10.2021 12:58, Michal Orzel wrote:
>>> --- a/xen/arch/x86/domain.c
>>> +++ b/xen/arch/x86/domain.c
>>> @@ -692,6 +692,12 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
>>> return -EINVAL;
>>> }
>>>
>>> + if ( config->flags & XEN_DOMCTL_CDF_vpmu )
>>> + {
>>> + dprintk(XENLOG_INFO, "vpmu support not ready yet\n");
>>> + return -EINVAL;
>>> + }
>>
>> I consider this message potentially misleading (as x86 does have vPMU
>> support, it merely doesn't get enabled this way). But isn't this redundant
>> with ...
>>
>>> @@ -534,6 +535,12 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
>>> return -EINVAL;
>>> }
>>>
>>> + if ( vpmu && !vpmu_is_available )
>>> + {
>>> + dprintk(XENLOG_INFO, "vpmu requested but not available\n");
>>> + return -EINVAL;
>>> + }
>>
>> ... this? (This message is again potentially misleading.)
>>
> Ok. vpmu_is_available is false for x86 so the check in x86's arch_sanitise_domain_config is redundant.
> I will fix it. When it comes to the message itself "vpmu requested but not available".
> Does the following sound better for you?
> "vpmu requested but the platform does not support it"
> If not, can you please suggest a better message?
While it gets a little long then, appending "at domain creation time" would
disambiguate the text. Or maybe "vPMU cannot be enabled this way"? It's a
debug-only message after all, so its wording can quite well be developer-
focused imo.
Jan
^ permalink raw reply [flat|nested] 13+ messages in thread