Hi Jan, > On 13 Feb 2023, at 09:36, Jan Beulich <[email protected]> wrote: > > On 10.02.2023 16:54, Luca Fancellu wrote: >>> On 2 Feb 2023, at 12:05, Jan Beulich <[email protected]> wrote: >>> On 02.02.2023 12:08, Luca Fancellu wrote: >>>> (is hw_cap only for x86?) >>> >>> I suppose it is, but I also expect it would better go away than be moved. >>> It doesn't hold a complete set of information, and it has been superseded. >>> >>> Question is (and I think I did raise this before, perhaps in different >>> context) whether Arm wouldn't want to follow x86 in how hardware as well >>> as hypervisor derived / used ones are exposed to the tool stack >>> (XEN_SYSCTL_get_cpu_featureset). I guess there's nothing really precluding >>> that data to consist of more than just boolean fields. >> >> Yes I guess that infrastructure could work, however I don’t have the >> bandwidth to >> put it in place at the moment, so I would like the Arm maintainers to give >> me a >> suggestion on how I can expose the vector length to XL if putting its value >> here >> needs to be avoided > > Since you've got a reply from Andrew boiling down to the same suggestion > (or should I even say request), I guess it wants seriously considering > to introduce abstract base infrastructure first. As Andrew says, time not > invested now will very likely mean yet more time to be invested later. > >>>> --- a/xen/include/public/sysctl.h >>>> +++ b/xen/include/public/sysctl.h >>>> @@ -18,7 +18,7 @@ >>>> #include "domctl.h" >>>> #include "physdev.h" >>>> >>>> -#define XEN_SYSCTL_INTERFACE_VERSION 0x00000015 >>>> +#define XEN_SYSCTL_INTERFACE_VERSION 0x00000016 >>> >>> Why? You ... >>> >>>> @@ -104,7 +110,8 @@ struct xen_sysctl_physinfo { >>>> uint32_t cpu_khz; >>>> uint32_t capabilities;/* XEN_SYSCTL_PHYSCAP_??? */ >>>> uint32_t arch_capabilities;/* XEN_SYSCTL_PHYSCAP_{X86,ARM,...}_??? */ >>>> - uint32_t pad; >>>> + uint16_t arm_sve_vl_bits; >>>> + uint16_t pad; >>>> uint64_aligned_t total_pages; >>>> uint64_aligned_t free_pages; >>>> uint64_aligned_t scrub_pages; >>> >>> ... add no new fields, and the only producer of the data zero-fills the >>> struct first thing. >> >> Yes that is right, I will wait to understand how I can proceed here: >> >> 1) rename arch_capabilities to arm_sve_vl_bits and put vector length there. >> 2) leave arch_capabilities untouched, no flag creation/setting, create >> uint32_t arm_sve_vl_bits field removing “pad”, >> Use its value to determine if SVE is present or not. >> 3) ?? > > 3) Introduce suitable #define(s) to use part of arch_capabilities for your > purpose, without renaming the field. (But that's of course only if Arm > maintainers agree with you on _not_ going the proper feature handling route > right away.)
As Luca said, he does not have the required bandwidth to do this so I think it is ok for him to go with your solution 3. @Julien/Stefano: any thoughts here ? Bertrand > > Jan
