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

["Alejandro Vallejo" <alejandro.vallejo@xxxxxxxxx>]

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.

Cheers,
Alejandro