On 25/05/2023 10:55, Luca Fancellu wrote:
>
>
>> On 25 May 2023, at 09:52, Michal Orzel <michal.or...@amd.com> wrote:
>>
>> Hi Luca,
>>
>> Sorry for jumping into this but I just wanted to read the dt binding doc and
>> spotted one thing by accident.
>>
>> On 24/05/2023 17:20, Bertrand Marquis wrote:
>>>
>>>
>>> Hi Luca,
>>>
>>>> On 23 May 2023, at 09:43, Luca Fancellu <luca.fance...@arm.com> wrote:
>>>>
>>>> Add a device tree property in the dom0less domU configuration
>>>> to enable the guest to use SVE.
>>>>
>>>> Update documentation.
>>>>
>>>> Signed-off-by: Luca Fancellu <luca.fance...@arm.com>
>>>
>>> Reviewed-by: Bertrand Marquis <bertrand.marq...@arm.com>
>>
>> (...)
>>>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>>>> index 9202a96d9c28..ba4fe9e165ee 100644
>>>> --- a/xen/arch/arm/domain_build.c
>>>> +++ b/xen/arch/arm/domain_build.c
>>>> @@ -4008,6 +4008,34 @@ void __init create_domUs(void)
>>>> d_cfg.max_maptrack_frames = val;
>>>> }
>>>>
>>>> + if ( dt_get_property(node, "sve", &val) )
>>>> + {
>>>> +#ifdef CONFIG_ARM64_SVE
>>>> + unsigned int sve_vl_bits;
>>>> + bool ret = false;
>>>> +
>>>> + if ( !val )
>>>> + {
>>>> + /* Property found with no value, means max HW VL
>>>> supported */
>>>> + ret = sve_domctl_vl_param(-1, &sve_vl_bits);
>>>> + }
>>>> + else
>>>> + {
>>>> + if ( dt_property_read_u32(node, "sve", &val) )
>>>> + ret = sve_domctl_vl_param(val, &sve_vl_bits);
>>>> + else
>>>> + panic("Error reading 'sve' property");
>> Both here and ...
>>
>>>> + }
>>>> +
>>>> + if ( ret )
>>>> + d_cfg.arch.sve_vl = sve_encode_vl(sve_vl_bits);
>>>> + else
>>>> + panic("SVE vector length error\n");
>>>> +#else
>>>> + panic("'sve' property found, but CONFIG_ARM64_SVE not
>>>> selected");
>> here you are missing \n at the end of string. If you take a look at panic()
>> implementation,
>> new line char is not added so in your case it would result in an ugly
>> formatted panic message.
>
> Hi Michal,
>
> Thank you for pointing that out! Indeed there might be some issues, I will
> fix in the next push.
With that fixed,
Reviewed-by: Michal Orzel <michal.or...@amd.com>
~Michal
