On 24.04.2023 17:18, Luca Fancellu wrote:
>> On 24 Apr 2023, at 16:06, Jan Beulich <jbeul...@suse.com> wrote:
>> On 24.04.2023 16:57, Luca Fancellu wrote:
>>>> On 24 Apr 2023, at 15:05, Jan Beulich <jbeul...@suse.com> wrote:
>>>> On 24.04.2023 16:00, Luca Fancellu wrote:
>>>>>> On 24 Apr 2023, at 12:34, Jan Beulich <jbeul...@suse.com> 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().
Jan
> arm-linux-gnueabihf-ld: (.init.text+0x136b4): undefined reference to
> `sve_domctl_vl_param'
> arm-linux-gnueabihf-ld: ./.xen-syms.0: hidden symbol `sve_domctl_vl_param'
> isn't defined
> arm-linux-gnueabihf-ld: final link failed: bad value
> make[3]: *** [/data_sdc/lucfan01/kirkstone_xen/xen/xen/arch/arm/Makefile:95:
> xen-syms] Error 1
> make[2]: *** [/data_sdc/lucfan01/kirkstone_xen/xen/xen/./build.mk:90: xen]
> Error 2
> make[1]: *** [/data_sdc/lucfan01/kirkstone_xen/xen/xen/Makefile:590: xen]
> Error 2
> make[1]: Leaving directory
> '/data_sdc/lucfan01/kirkstone_xen/build/xen-qemu-arm32'
> make: *** [Makefile:181: __sub-make] Error 2
> make: Leaving directory '/data_sdc/lucfan01/kirkstone_xen/xen/xen’
>
> These are the modification I’ve done:
>
> diff --git a/xen/arch/arm/include/asm/arm64/sve.h
> b/xen/arch/arm/include/asm/arm64/sve.h
> index 71bddb41f19c..330c47ea8864 100644
> --- a/xen/arch/arm/include/asm/arm64/sve.h
> +++ b/xen/arch/arm/include/asm/arm64/sve.h
> @@ -24,6 +24,8 @@ static inline unsigned int sve_encode_vl(unsigned int
> sve_vl_bits)
> return sve_vl_bits / SVE_VL_MULTIPLE_VAL;
> }
>
> +bool sve_domctl_vl_param(int val, unsigned int *out);
> +
> #ifdef CONFIG_ARM64_SVE
>
> extern int opt_dom0_sve;
> @@ -37,7 +39,6 @@ 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 */
>
> @@ -68,11 +69,6 @@ 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;
> -}
> -
> #endif /* CONFIG_ARM64_SVE */
>
> #endif /* _ARM_ARM64_SVE_H */
