On 30.03.2023 12:41, Luca Fancellu wrote: > > >> On 28 Mar 2023, at 11:14, Jan Beulich <[email protected]> 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
