Mailing List Archive: [PATCH v4 08/12] xen/physinfo: encode Arm SVE vector length in arch

[PATCH v4 08/12] xen/physinfo: encode Arm SVE vector length in arch_capabilities

Mar 27, 2023, 3:59 AM

Post #1 of 4 (14 views)

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 v3:
- domainconfig_encode_vl is now named sve_encode_vl
Changes from v2:
- Remove XEN_SYSCTL_PHYSCAP_ARM_SVE_SHFT, use MASK_INSR and
protect with ifdef XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK (Jan)
- Use the helper function sve_arch_cap_physinfo to encode
the VL into physinfo arch_capabilities field.
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/arm64/sve.c | 12 ++++++++++++
xen/arch/arm/include/asm/arm64/sve.h | 2 ++
xen/arch/arm/sysctl.c | 3 +++
xen/include/public/sysctl.h | 4 ++++
4 files changed, 21 insertions(+)

diff --git a/xen/arch/arm/arm64/sve.c b/xen/arch/arm/arm64/sve.c
index 6416403817e3..7b5223117e1b 100644
--- a/xen/arch/arm/arm64/sve.c
+++ b/xen/arch/arm/arm64/sve.c
@@ -124,3 +124,15 @@ int __init sve_parse_dom0_param(const char *str_begin, const char *str_end)
{
return parse_integer("sve", str_begin, str_end, (int*)&opt_dom0_sve);
}
+
+void sve_arch_cap_physinfo(uint32_t *arch_capabilities)
+{
+ if ( cpu_has_sve )
+ {
+ /* Vector length is divided by 128 to save some space */
+ uint32_t sve_vl = MASK_INSR(sve_encode_vl(get_sys_vl_len()),
+ XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK);
+
+ *arch_capabilities |= sve_vl;
+ }
+}
diff --git a/xen/arch/arm/include/asm/arm64/sve.h b/xen/arch/arm/include/asm/arm64/sve.h
index 69a1044c37d9..a6d0fc851f68 100644
--- a/xen/arch/arm/include/asm/arm64/sve.h
+++ b/xen/arch/arm/include/asm/arm64/sve.h
@@ -42,6 +42,7 @@ void sve_context_free(struct vcpu *v);
void sve_save_state(struct vcpu *v);
void sve_restore_state(struct vcpu *v);
int sve_parse_dom0_param(const char *str_begin, const char *str_end);
+void sve_arch_cap_physinfo(uint32_t *arch_capabilities);

#else /* !CONFIG_ARM64_SVE */

@@ -70,6 +71,7 @@ static inline int sve_context_init(struct vcpu *v)
static inline void sve_context_free(struct vcpu *v) {}
static inline void sve_save_state(struct vcpu *v) {}
static inline void sve_restore_state(struct vcpu *v) {}
+static inline void sve_arch_cap_physinfo(uint32_t *arch_capabilities) {}

static inline int sve_parse_dom0_param(const char *str_begin,
const char *str_end)
diff --git a/xen/arch/arm/sysctl.c b/xen/arch/arm/sysctl.c
index b0a78a8b10d0..64e4d3e06a6b 100644
--- a/xen/arch/arm/sysctl.c
+++ b/xen/arch/arm/sysctl.c
@@ -11,11 +11,14 @@
#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;
+
+ sve_arch_cap_physinfo(&pi->arch_capabilities);
}

long arch_do_sysctl(struct xen_sysctl *sysctl,
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index e8dded9fb94a..99ea3fa0740b 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -94,6 +94,10 @@ struct xen_sysctl_tbuf_op {
/* Max XEN_SYSCTL_PHYSCAP_* constant. Used for ABI checking. */
#define XEN_SYSCTL_PHYSCAP_MAX XEN_SYSCTL_PHYSCAP_gnttab_v2

+#ifdef __aarch64__
+#define XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK (0x1FU)
+#endif
+
struct xen_sysctl_physinfo {
uint32_t threads_per_core;
uint32_t cores_per_socket;
--
2.34.1

Re: [PATCH v4 08/12] xen/physinfo: encode Arm SVE vector length in arch_capabilities [ In reply to ]

jbeulich at suse

Mar 28, 2023, 3:14 AM

Post #2 of 4 (14 views)

Permalink

On 27.03.2023 12:59, Luca Fancellu wrote:
> --- a/xen/arch/arm/arm64/sve.c
> +++ b/xen/arch/arm/arm64/sve.c
> @@ -124,3 +124,15 @@ int __init sve_parse_dom0_param(const char *str_begin, const char *str_end)
> {
> return parse_integer("sve", str_begin, str_end, (int*)&opt_dom0_sve);
> }
> +
> +void sve_arch_cap_physinfo(uint32_t *arch_capabilities)
> +{
> + if ( cpu_has_sve )
> + {
> + /* Vector length is divided by 128 to save some space */
> + uint32_t sve_vl = MASK_INSR(sve_encode_vl(get_sys_vl_len()),
> + XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK);
> +
> + *arch_capabilities |= sve_vl;
> + }
> +}

I'm again wondering why a separate function is needed, when everything
that's needed is ...

> --- a/xen/arch/arm/sysctl.c
> +++ b/xen/arch/arm/sysctl.c
> @@ -11,11 +11,14 @@
> #include <xen/lib.h>
> #include <xen/errno.h>
> #include <xen/hypercall.h>
> +#include <asm/arm64/sve.h>

... becoming available here for use ...

> #include <public/sysctl.h>
>
> void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
> {
> pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm | XEN_SYSCTL_PHYSCAP_hap;
> +
> + sve_arch_cap_physinfo(&pi->arch_capabilities);

... here. That would be even more so if, like suggested before,
get_sys_vl_len() returned 0 when !cpu_has_sve.

Jan

Re: [PATCH v4 08/12] xen/physinfo: encode Arm SVE vector length in arch_capabilities [ In reply to ]

Luca.Fancellu at arm

Mar 30, 2023, 3:41 AM

Post #3 of 4 (14 views)

Permalink

> On 28 Mar 2023, at 11:14, Jan Beulich <jbeulich@suse.com> wrote:
>
> On 27.03.2023 12:59, Luca Fancellu wrote:
>> --- a/xen/arch/arm/arm64/sve.c
>> +++ b/xen/arch/arm/arm64/sve.c
>> @@ -124,3 +124,15 @@ int __init sve_parse_dom0_param(const char *str_begin, const char *str_end)
>> {
>> return parse_integer("sve", str_begin, str_end, (int*)&opt_dom0_sve);
>> }
>> +
>> +void sve_arch_cap_physinfo(uint32_t *arch_capabilities)
>> +{
>> + if ( cpu_has_sve )
>> + {
>> + /* Vector length is divided by 128 to save some space */
>> + uint32_t sve_vl = MASK_INSR(sve_encode_vl(get_sys_vl_len()),
>> + XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK);
>> +
>> + *arch_capabilities |= sve_vl;
>> + }
>> +}
>
> I'm again wondering why a separate function is needed, when everything
> that's needed is ...
>
>> --- a/xen/arch/arm/sysctl.c
>> +++ b/xen/arch/arm/sysctl.c
>> @@ -11,11 +11,14 @@
>> #include <xen/lib.h>
>> #include <xen/errno.h>
>> #include <xen/hypercall.h>
>> +#include <asm/arm64/sve.h>
>
> ... becoming available here for use ...
>
>> #include <public/sysctl.h>
>>
>> void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
>> {
>> pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm | XEN_SYSCTL_PHYSCAP_hap;
>> +
>> + sve_arch_cap_physinfo(&pi->arch_capabilities);
>
> ... here. That would be even more so if, like suggested before,
> get_sys_vl_len() returned 0 when !cpu_has_sve.

I’ve had a look on this, I can do everything In arch_do_physinfo if in xen/include/public/sysctl.h
the XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK is protected by __aarch64__ or __arm__ .

Do you agree on that?

>
> Jan

Re: [PATCH v4 08/12] xen/physinfo: encode Arm SVE vector length in arch_capabilities [ In reply to ]

jbeulich at suse

Mar 30, 2023, 3:49 AM

Post #4 of 4 (14 views)

Permalink

On 30.03.2023 12:41, Luca Fancellu wrote:
>
>
>> On 28 Mar 2023, at 11:14, Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 27.03.2023 12:59, Luca Fancellu wrote:
>>> --- a/xen/arch/arm/arm64/sve.c
>>> +++ b/xen/arch/arm/arm64/sve.c
>>> @@ -124,3 +124,15 @@ int __init sve_parse_dom0_param(const char *str_begin, const char *str_end)
>>> {
>>> return parse_integer("sve", str_begin, str_end, (int*)&opt_dom0_sve);
>>> }
>>> +
>>> +void sve_arch_cap_physinfo(uint32_t *arch_capabilities)
>>> +{
>>> + if ( cpu_has_sve )
>>> + {
>>> + /* Vector length is divided by 128 to save some space */
>>> + uint32_t sve_vl = MASK_INSR(sve_encode_vl(get_sys_vl_len()),
>>> + XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK);
>>> +
>>> + *arch_capabilities |= sve_vl;
>>> + }
>>> +}
>>
>> I'm again wondering why a separate function is needed, when everything
>> that's needed is ...
>>
>>> --- a/xen/arch/arm/sysctl.c
>>> +++ b/xen/arch/arm/sysctl.c
>>> @@ -11,11 +11,14 @@
>>> #include <xen/lib.h>
>>> #include <xen/errno.h>
>>> #include <xen/hypercall.h>
>>> +#include <asm/arm64/sve.h>
>>
>> ... becoming available here for use ...
>>
>>> #include <public/sysctl.h>
>>>
>>> void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
>>> {
>>> pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm | XEN_SYSCTL_PHYSCAP_hap;
>>> +
>>> + sve_arch_cap_physinfo(&pi->arch_capabilities);
>>
>> ... here. That would be even more so if, like suggested before,
>> get_sys_vl_len() returned 0 when !cpu_has_sve.
>
> I’ve had a look on this, I can do everything In arch_do_physinfo if in xen/include/public/sysctl.h
> the XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK is protected by __aarch64__ or __arm__ .
>
> Do you agree on that?

I don't see the connection, but guarding the #define in the public header
looks not only okay, but even desirable to me.

Jan