Re: [PATCH v4 10/12] xen/tools: add sve parameter in XL configuration

[Luca Fancellu <Luca.Fancellu@xxxxxxx>]

Hi Anthony,

>> 
>> Yes I can change it, a need to do it anyway because I think also here, the 
>> suggestion
>> From Jan can apply and we could pass a negative value that means “max VL 
>> supported
>> by the platform"
> 
> Well, it's a config file, not a C ABI, so max allowed here doesn't have to be
> spelled "-1", it could also be "max", "max-allowed",
> "max-size-supported", ... So fill free deviate from the restricted C
> ABI. But "-1" works as long as it's the only allowed negative number.

Yes while working on the patch I’ve found that I could declare this type in 
Libxl:

libxl_sve_type = Enumeration("sve_type", [
    (0, "disabled"),
    (128, "128"),
    (256, "256"),
    (384, "384"),
    (512, "512"),
    (640, "640"),
    (768, "768"),
    (896, "896"),
    (1024, "1024"),
    (1152, "1152"),
    (1280, "1280"),
    (1408, "1408"),
    (1536, "1536"),
    (1664, "1664"),
    (1792, "1792"),
    (1920, "1920"),
    (2048, "2048"),
    (-1, "hw")
    ], init_val = "LIBXL_SVE_TYPE_DISABLED”)

So that in xl I can just use libxl_sve_type_from_string

> 
>>> 
>>>> +supported bits value, then the domain creation will fail.
>>>> +A value equal to zero is the default and it means this guest is not 
>>>> allowed to
>>>> +use SVE.
>>>> +
>>>> +=back
>>>> +
>>>> =head3 x86
>>>> 
>>>> =over 4
>>>> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
>>>> index ddc7b2a15975..16a49031fd51 100644
>>>> --- a/tools/libs/light/libxl_arm.c
>>>> +++ b/tools/libs/light/libxl_arm.c
>>>> @@ -211,6 +211,8 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
>>>>        return ERROR_FAIL;
>>>>    }
>>>> 
>>>> +    config->arch.sve_vl = d_config->b_info.arch_arm.sve;
>>> 
>>> This truncate a 16bit value into an 8bit value, I think you should check
>>> that the value can actually fit.
>>> 
>>> And maybe check `d_config->b_info.arch_arm.sve` value here instead of
>>> `xl` as commented later.
>> 
>> Yes I can do it, one question, can I use here xc_physinfo to retrieve the 
>> maximum
>> Vector length from arch_capabilities?
>> I mean, is there a better way or I can go for that?
> 
> Yeah, there might be a "better" way. I think me suggestion to check the
> sve value here was wrong. I still want to have it checked in libxl, but
> it might be better to do that in the previous step, that is
> "libxl__domain_config_setdefault". libxl__arch_domain_build_info_setdefault()
> will have `physinfo` so you won't have to call xc_physinfo().

Right, I’ve seen it before but I was unsure if it was the right way, now that 
you
suggested it, I will go for that.

Thank you.

Cheers,
Luca