Re: [XEN PATCH v1 1/2] x86/intel: optional build of intel.c

[Sergiy Kibrik <sergiy_kibrik@xxxxxxxx>]

12.08.24 15:24, Alejandro Vallejo:
> On Mon Aug 12, 2024 at 10:58 AM BST, Jan Beulich wrote:
> > On 12.08.2024 11:40, Sergiy Kibrik wrote:
> > > 09.08.24 13:36, Alejandro Vallejo:
> > > > On Fri Aug 9, 2024 at 11:09 AM BST, Sergiy Kibrik wrote:
> > > > > --- a/xen/arch/x86/cpu/Makefile
> > > > > +++ b/xen/arch/x86/cpu/Makefile
> > > > > @@ -6,10 +6,10 @@ obj-y += amd.o
> > > > >    obj-y += centaur.o
> > > > >    obj-y += common.o
> > > > >    obj-y += hygon.o
> > > > > -obj-y += intel.o
> > > > > -obj-y += intel_cacheinfo.o
> > > > > +obj-$(CONFIG_INTEL) += intel.o
> > > > > +obj-$(CONFIG_INTEL) += intel_cacheinfo.o
> > > > >    obj-y += mwait-idle.o
> > > > > -obj-y += shanghai.o
> > > > > +obj-$(CONFIG_INTEL) += shanghai.o
> > > > Why pick this one too? It's based on VIA IP, aiui.
> > > shanghai.c and intel.c both use init_intel_cacheinfo() routine, so
> > > there's build dependency on Intel code.
> My point is that the use of Intel functions on Shanghai and not Centaur is
> accidental. If shanghai goes under Intel so should Centaur (imo).
>
> > Yet Shanghai isn't as directly a clone of Intel CPUs as Hygon ones are
> > for AMD. So at the very least you want to justify your choice in the
> > description. After all there's also the alternative of having a separate
> > SHANGHAI Kconfig setting, which would merely have "select INTEL" or
> > "depends on INTEL".
> >
> > Jan
> That's one option, another is for the Kconfig options to explicitly state which
> vendors they apply to. I'd be fine with either. It's less fine for CONFIG_INTEL
> to cover a VIA derivative and not the other.
I think I'll go with separate kconfig options -- for Centaur, Shanghai & 
Hygon, as we already got a separate submenu for CPUs & everything.
Also this way it's more up to the task of config tuning.

Thanks for clarification!

   -Sergiy