Re: [PATCH v6 07/12] xen: enable Dom0 to use SVE feature

[Luca Fancellu <Luca.Fancellu@xxxxxxx>]

> On 24 Apr 2023, at 16:41, Jan Beulich <jbeulich@xxxxxxxx> wrote:
> 
> On 24.04.2023 17:34, Luca Fancellu wrote:
>>> On 24 Apr 2023, at 16:25, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>> On 24.04.2023 17:18, Luca Fancellu wrote:
>>>>> On 24 Apr 2023, at 16:06, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>>>> On 24.04.2023 16:57, Luca Fancellu wrote:
>>>>>>> On 24 Apr 2023, at 15:05, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>>>>>> On 24.04.2023 16:00, Luca Fancellu wrote:
>>>>>>>>> On 24 Apr 2023, at 12:34, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>>>>>>>> On 24.04.2023 08:02, Luca Fancellu wrote:
>>>>>>>>>> @@ -30,9 +37,11 @@ int sve_context_init(struct vcpu *v);
>>>>>>>>>> void sve_context_free(struct vcpu *v);
>>>>>>>>>> void sve_save_state(struct vcpu *v);
>>>>>>>>>> void sve_restore_state(struct vcpu *v);
>>>>>>>>>> +bool sve_domctl_vl_param(int val, unsigned int *out);
>>>>>>>>>> 
>>>>>>>>>> #else /* !CONFIG_ARM64_SVE */
>>>>>>>>>> 
>>>>>>>>>> +#define opt_dom0_sve     (0)
>>>>>>>>>> #define is_sve_domain(d) (0)
>>>>>>>>>> 
>>>>>>>>>> static inline register_t compute_max_zcr(void)
>>>>>>>>>> @@ -59,6 +68,11 @@ static inline void sve_context_free(struct vcpu 
>>>>>>>>>> *v) {}
>>>>>>>>>> static inline void sve_save_state(struct vcpu *v) {}
>>>>>>>>>> static inline void sve_restore_state(struct vcpu *v) {}
>>>>>>>>>> 
>>>>>>>>>> +static inline bool sve_domctl_vl_param(int val, unsigned int *out)
>>>>>>>>>> +{
>>>>>>>>>> +    return false;
>>>>>>>>>> +}
>>>>>>>>> 
>>>>>>>>> Once again I don't see the need for this stub: opt_dom0_sve is 
>>>>>>>>> #define-d
>>>>>>>>> to plain zero when !ARM64_SVE, so the only call site merely requires a
>>>>>>>>> visible declaration, and DCE will take care of eliminating the actual 
>>>>>>>>> call.
>>>>>>>> 
>>>>>>>> I’ve tried to do that, I’ve put the declaration outside the ifdef so 
>>>>>>>> that it was always included
>>>>>>>> and I removed the stub, but I got errors on compilation because of 
>>>>>>>> undefined function.
>>>>>>>> For that reason  I left that change out.
>>>>>>> 
>>>>>>> Interesting. I don't see where the reference would be coming from.
>>>>>> 
>>>>>> Could it be because the declaration is visible, outside the ifdef, but 
>>>>>> the definition is not compiled in? 
>>>>> 
>>>>> Well, yes, likely. But the question isn't that but "Why did the reference
>>>>> not get removed, when it's inside an if(0) block?"
>>>> 
>>>> Oh ok, I don’t know, here what I get if for example I build arm32:
>>>> 
>>>> arm-linux-gnueabihf-ld -EL -T arch/arm/xen.lds -N prelink.o \
>>>> ./common/symbols-dummy.o -o ./.xen-syms.0
>>>> arm-linux-gnueabihf-ld: prelink.o: in function `create_domUs':
>>>> (.init.text+0x13464): undefined reference to `sve_domctl_vl_param'
>>> 
>>> In particular with seeing this: What you copied here is a build with the
>>> series applied only up to this patch? I ask because the patch here adds a
>>> call only out of create_dom0().
>> 
>> No I’ve do the changes on top of the serie, I’ve tried it now, only to this 
>> patch and it builds correctly,
>> It was my mistake to don’t read carefully the error output.
>> 
>> Anyway I guess this change is not applicable because we don’t have a symbol 
>> that is plain 0 for domUs
>> to be placed inside create_domUs.
> 
> Possible, but would you mind first telling me in which other patch(es) the
> further reference(s) are being introduced, so I could take a look without
> (again) digging through the entire series?

Sure, the other references to the function are introduced in "xen/arm: add sve 
property for dom0less domUs” patch 11

> 
> Jan