On 15.03.2023 11:09, Luca Fancellu wrote:
>> On 15 Mar 2023, at 09:27, Jan Beulich <jbeul...@suse.com> wrote:
>> On 15.03.2023 10:05, Luca Fancellu wrote:
>>> --- a/xen/include/public/arch-arm.h
>>> +++ b/xen/include/public/arch-arm.h
>>> @@ -304,6 +304,9 @@ struct xen_arch_domainconfig {
>>> uint16_t tee_type;
>>> /* IN */
>>> uint32_t nr_spis;
>>> + /* IN */
>>> + uint16_t sve_vl_bits;
>>> + uint16_t _pad1;
>>
>> Can register sizes be a non-power-of-2 in size?
>From logic in later patches I infer the answer is "yes" here.
>> If not, by representing
>> the value here as log2(bits) the existing 8-bit padding field could be
>> used instead. Sadly, because of it not having been checked to be zero,
>> that wouldn't avoid ...
>>
>>> --- a/xen/include/public/domctl.h
>>> +++ b/xen/include/public/domctl.h
>>> @@ -21,7 +21,7 @@
>>> #include "hvm/save.h"
>>> #include "memory.h"
>>>
>>> -#define XEN_DOMCTL_INTERFACE_VERSION 0x00000015
>>> +#define XEN_DOMCTL_INTERFACE_VERSION 0x00000016
>>
>> ... the need for the bump here. Yet you want to avoid repeating that
>> mistake and hence check that the new padding field you introduce (if
>> the value needs to remain 16 bits wide) is zero on input.
>
> Hi Jan,
>
> Thank you for your suggestion, just to be sure we are on the same page:
>
> I can store the vector length as VL/128, doing that I can use just 8 bits, so
> I can do this:
>
> struct xen_arch_domainconfig {
> /* IN/OUT */
> uint8_t gic_version;
> /* IN */
> uint16_t tee_type;
> /* IN */
> uint8_t sve_vl_bits_enc;
> /* IN */
> uint32_t nr_spis;
> [...]
>
> To take advantage of the existing not explicit padding.
>
> Is that what you meant?
Almost:
struct xen_arch_domainconfig {
/* IN/OUT */
uint8_t gic_version;
/* IN */
uint8_t sve_vl_bits_enc;
/* IN */
uint16_t tee_type;
/* IN */
uint32_t nr_spis;
[...]
I'm inclined to suggest to name the new field "sve_vl" only, with the
comment extended to clarify in what units the value is.
Jan
