| Message ID | 20230315090558.731029-8-luca.fancellu@arm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | SVE feature for arm guests | expand |
On 15.03.2023 10:05, Luca Fancellu wrote: > --- a/xen/include/public/sysctl.h > +++ b/xen/include/public/sysctl.h > @@ -94,6 +94,9 @@ struct xen_sysctl_tbuf_op { > /* Max XEN_SYSCTL_PHYSCAP_* constant. Used for ABI checking. */ > #define XEN_SYSCTL_PHYSCAP_MAX XEN_SYSCTL_PHYSCAP_gnttab_v2 > > +#define XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK (0x1FU) > +#define XEN_SYSCTL_PHYSCAP_ARM_SVE_SHFT (0) The second of these can be inferred from the first, so I'd like to ask that redundant definitions be omitted from the public headers. For the code using the constant we specifically have MASK_INSR(). Just like there already are x86-specific sections in this file, I think the remaining single #define also wants enclosing in "#ifdef __aarch64__" here. Jan
> On 15 Mar 2023, at 09:41, Jan Beulich <jbeulich@suse.com> wrote: > > On 15.03.2023 10:05, Luca Fancellu wrote: >> --- a/xen/include/public/sysctl.h >> +++ b/xen/include/public/sysctl.h >> @@ -94,6 +94,9 @@ struct xen_sysctl_tbuf_op { >> /* Max XEN_SYSCTL_PHYSCAP_* constant. Used for ABI checking. */ >> #define XEN_SYSCTL_PHYSCAP_MAX XEN_SYSCTL_PHYSCAP_gnttab_v2 >> >> +#define XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK (0x1FU) >> +#define XEN_SYSCTL_PHYSCAP_ARM_SVE_SHFT (0) > > The second of these can be inferred from the first, so I'd like to ask > that redundant definitions be omitted from the public headers. For the > code using the constant we specifically have MASK_INSR(). > > Just like there already are x86-specific sections in this file, I think > the remaining single #define also wants enclosing in "#ifdef __aarch64__" > here. > Thank you, I wasn’t aware of that useful macro, I will use it in the next version and I’ll enclose the mask using ifdef. Are you ok for the position of the mask define or should I declare it somewhere else? > Jan
On 15.03.2023 11:39, Luca Fancellu wrote: >> On 15 Mar 2023, at 09:41, Jan Beulich <jbeulich@suse.com> wrote: >> On 15.03.2023 10:05, Luca Fancellu wrote: >>> --- a/xen/include/public/sysctl.h >>> +++ b/xen/include/public/sysctl.h >>> @@ -94,6 +94,9 @@ struct xen_sysctl_tbuf_op { >>> /* Max XEN_SYSCTL_PHYSCAP_* constant. Used for ABI checking. */ >>> #define XEN_SYSCTL_PHYSCAP_MAX XEN_SYSCTL_PHYSCAP_gnttab_v2 >>> >>> +#define XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK (0x1FU) >>> +#define XEN_SYSCTL_PHYSCAP_ARM_SVE_SHFT (0) >> >> The second of these can be inferred from the first, so I'd like to ask >> that redundant definitions be omitted from the public headers. For the >> code using the constant we specifically have MASK_INSR(). >> >> Just like there already are x86-specific sections in this file, I think >> the remaining single #define also wants enclosing in "#ifdef __aarch64__" >> here. > > Thank you, I wasn’t aware of that useful macro, I will use it in the next version and I’ll > enclose the mask using ifdef. > Are you ok for the position of the mask define or should I declare it somewhere else? The placement looks reasonable to me. Jan
diff --git a/xen/arch/arm/sysctl.c b/xen/arch/arm/sysctl.c index b0a78a8b10d0..075f52801453 100644 --- a/xen/arch/arm/sysctl.c +++ b/xen/arch/arm/sysctl.c @@ -11,11 +11,22 @@ #include <xen/lib.h> #include <xen/errno.h> #include <xen/hypercall.h> +#include <asm/arm64/sve.h> #include <public/sysctl.h> void arch_do_physinfo(struct xen_sysctl_physinfo *pi) { pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm | XEN_SYSCTL_PHYSCAP_hap; + + if ( cpu_has_sve ) + { + /* Vector length is divided by 128 to save some space */ + uint32_t sve_vl = get_sys_vl_len() / SVE_VL_MULTIPLE_VAL; + + sve_vl &= XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK; + + pi->arch_capabilities |= sve_vl << XEN_SYSCTL_PHYSCAP_ARM_SVE_SHFT; + } } long arch_do_sysctl(struct xen_sysctl *sysctl, diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h index e8dded9fb94a..ec15d8c5ab47 100644 --- a/xen/include/public/sysctl.h +++ b/xen/include/public/sysctl.h @@ -94,6 +94,9 @@ struct xen_sysctl_tbuf_op { /* Max XEN_SYSCTL_PHYSCAP_* constant. Used for ABI checking. */ #define XEN_SYSCTL_PHYSCAP_MAX XEN_SYSCTL_PHYSCAP_gnttab_v2 +#define XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK (0x1FU) +#define XEN_SYSCTL_PHYSCAP_ARM_SVE_SHFT (0) + struct xen_sysctl_physinfo { uint32_t threads_per_core; uint32_t cores_per_socket;
When the arm platform supports SVE, advertise the feature in the field arch_capabilities in struct xen_sysctl_physinfo by encoding the SVE vector length in it. Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> --- Changes from v1: - Use only arch_capabilities and some defines to encode SVE VL (Bertrand, Stefano, Jan) Changes from RFC: - new patch --- xen/arch/arm/sysctl.c | 11 +++++++++++ xen/include/public/sysctl.h | 3 +++ 2 files changed, 14 insertions(+)