Re: [PATCH v2 2/2] x86/Kconfig: Introduce CONFIG_{AMD,INTEL} and conditionalise ucode

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Oct 27, 2023 at 08:18:18PM +0100, Andrew Cooper wrote:
> On 27/10/2023 2:47 pm, Roger Pau Monné wrote:
> > On Fri, Oct 27, 2023 at 09:12:40AM +0200, Jan Beulich wrote:
> >> On 26.10.2023 22:55, Andrew Cooper wrote:
> >>> We eventually want to be able to build a stripped down Xen for a single
> >>> platform.  Make a start with CONFIG_{AMD,INTEL} (hidden behind EXPERT, but
> >>> available to randconfig), and adjust the microcode logic.
> >>>
> >>> No practical change.
> >>>
> >>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> >>> ---
> >>> CC: Jan Beulich <JBeulich@xxxxxxxx>
> >>> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> >>> CC: Wei Liu <wl@xxxxxxx>
> >>> CC: Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>
> >>> CC: Stefano Stabellini <stefano.stabellini@xxxxxxx>
> >>> CC: Xenia Ragiadakou <xenia.ragiadakou@xxxxxxx>
> >>>
> >>> I've intentionally ignored the other vendors for now.  They can be put 
> >>> into
> >>> Kconfig by whomever figures out the actual dependencies between their init
> >>> routines.
> >>>
> >>> v2:
> >>>  * Tweak text
> >> What about the indentation issues mentioned in reply to v1?
> >>
> >> As to using un-amended AMD and INTEL - Roger, what's your view here?
> > I think it would be good to add a suffix, like we do for
> > {AMD,INTEL}_IOMMU options, and reserve the plain AMD and INTEL options
> > as platform/system level options that enable both VENDOR_{CPU,IOMMU}
> > sub options.
> >
> > So yes, {INTEL,AMD}_CPU seems a good option.
> 
> Really?  You do realise that, unlike the IOMMU names, this is going to
> be plastered all over the Makefiles and header files?

What's it different from using INTEL_CPU than just plain INTEL?  It's
still going to be plastered all over the Makefiles and header files.

> And it breaks the careful attempt not to use the ambigous term when
> describing what the symbol means.
> 
> I'll send out a v2.5 so you can see it in context, but I'm going to say
> straight up - I think this is a mistake.

My point is that we might want to reserve the top level names (iow:
CONFIG_INTEL, CONFIG_AMD, CONFIG_{VENDOR}) for system wide options
that enable both the CPU and the IOMMU code needed for the selected
vendor.

I'm happy to use a name different than {INTEL,AMD}_CPU for the vendor
CPU/platform code.

Alternatively, I would be fine to use CONFIG_{INTEL,AMD} as long as
the existing CONFIG_{AMD,INTEL}_IOMMU Kconfig options are made
dependent on the newly introduced CONFIG_{INTEL,AMD} options, so when
disabling CONFIG_AMD CONFIG_AMD_IOMMU also gets disabled.

Maybe that's already the intention of the suggested CONFIG_{AMD,INTEL}
and I'm missing the point.

Thanks, Roger.