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

[Julien Grall <julien@xxxxxxx>]

Hi Luca,

Sorry for the late reply.

On 25/04/2023 07:04, Luca Fancellu wrote:
> > On 24 Apr 2023, at 17:10, Jan Beulich <jbeulich@xxxxxxxx> wrote:
> >
> > On 24.04.2023 17:43, Luca Fancellu wrote:
> > > > 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:
> > > > > > > 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
> > Personally I'm inclined to suggest adding "#ifdef CONFIG_ARM64_SVE" there.
> > But I guess that may again go against your desire to not ignore inapplicable
> > options. Still I can't resist to at least ask how an "sve" node on Arm32 is
> > different from an entirely unknown one.
> It would be ok for me to use #ifdef CONFIG_ARM64_SVE and fail in the #else 
> branch,
> but I had the feeling in the past that Arm maintainers are not very happy with 
> #ifdefs, I might
> be wrong so I’ll wait for them to give an opinion and then I will be happy to 
> follow.
IIRC, your suggestion is for patch #11. In this case, my preference is 
the #ifdef + throwing an error in the #else branch. This would avoid to 
silently ignore the property if SVE is not enabled (both Bertrand and I 
agreed this should not be ignored, see [1]).
Cheers,

[1] 
https://lore.kernel.org/all/7614AE25-F59D-430A-9C3E-30B1CE0E1580@xxxxxxx/
--
Julien Grall