| Message ID | 20231027191926.3283871-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On Fri, 27 Oct 2023, 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@citrix.com> I read the discussion here: https://marc.info/?l=xen-devel&m=169865507432363 https://lore.kernel.org/xen-devel/ZT9yNrdoCKZs3_uY@macbook/ I think we want two top-level simple CONFIG_AMD and CONFIG_INTEL options. Do we also want finer granular sub-options such as CONFIG_AMD_CPU and CONFIG_INTEL_CPU, which would be controlled by the top-level options CONFIG_AMD and CONFIG_INTEL? I think we could go either way. CONFIG_{AMD,INTEL} could be sufficient, but also having them control CONFIG_{AMD,INTEL}_CPU and CONFIG_{AMD,INTEL}_IOMMU is fine too. We already have CONFIG_{AMD,INTEL}_IOMMU. At the time I wondered if we could have just used CONFIG_{AMD,INTEL} for evertything. But given we have CONFIG_{AMD,INTEL}_IOMMU, I can see why the reviewers suggested to use CONFIG_{AMD,INTEL}_CPU. I would have introduced CONFIG_{AMD,INTEL} only, given that it is not possible to have an AMD CPU with an INTEL IOMMU and vice versa. Anyway, I don't really have a strong opinion either way but we need this patch to move forward one way or another so: Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> (I would also ck different versions of this patch.) > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Roger Pau Monné <roger.pau@citrix.com> > CC: Wei Liu <wl@xen.org> > CC: Alejandro Vallejo <alejandro.vallejo@cloud.com> > CC: Stefano Stabellini <stefano.stabellini@amd.com> > CC: Xenia Ragiadakou <xenia.ragiadakou@amd.com> > > 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 > v2.5: > * Fix indentation > * Rename with _CPU suffixes as requested > --- > xen/arch/x86/Kconfig | 2 ++ > xen/arch/x86/Kconfig.cpu | 22 ++++++++++++++++++++++ > xen/arch/x86/cpu/microcode/Makefile | 4 ++-- > xen/arch/x86/cpu/microcode/private.h | 9 +++++++++ > 4 files changed, 35 insertions(+), 2 deletions(-) > create mode 100644 xen/arch/x86/Kconfig.cpu > > diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig > index eac77573bd75..d9eacdd7e0fa 100644 > --- a/xen/arch/x86/Kconfig > +++ b/xen/arch/x86/Kconfig > @@ -49,6 +49,8 @@ config HAS_CC_CET_IBT > > menu "Architecture Features" > > +source "arch/x86/Kconfig.cpu" > + > source "arch/Kconfig" > > config PV > diff --git a/xen/arch/x86/Kconfig.cpu b/xen/arch/x86/Kconfig.cpu > new file mode 100644 > index 000000000000..b68c41977a3b > --- /dev/null > +++ b/xen/arch/x86/Kconfig.cpu > @@ -0,0 +1,22 @@ > +menu "Supported CPU vendors" > + visible if EXPERT > + > +config AMD_CPU > + bool "AMD" > + default y > + help > + Detection, tunings and quirks for AMD platforms. > + > + May be turned off in builds targetting other vendors. Otherwise, > + must be enabled for Xen to work suitably on AMD platforms. > + > +config INTEL_CPU > + bool "Intel" > + default y > + help > + Detection, tunings and quirks for Intel platforms. > + > + May be turned off in builds targetting other vendors. Otherwise, > + must be enabled for Xen to work suitably on Intel platforms. > + > +endmenu > diff --git a/xen/arch/x86/cpu/microcode/Makefile b/xen/arch/x86/cpu/microcode/Makefile > index aae235245b06..194ddc239ee3 100644 > --- a/xen/arch/x86/cpu/microcode/Makefile > +++ b/xen/arch/x86/cpu/microcode/Makefile > @@ -1,3 +1,3 @@ > -obj-y += amd.o > +obj-$(CONFIG_AMD_CPU) += amd.o > obj-y += core.o > -obj-y += intel.o > +obj-$(CONFIG_INTEL_CPU) += intel.o > diff --git a/xen/arch/x86/cpu/microcode/private.h b/xen/arch/x86/cpu/microcode/private.h > index b58611e908aa..bb329933ac89 100644 > --- a/xen/arch/x86/cpu/microcode/private.h > +++ b/xen/arch/x86/cpu/microcode/private.h > @@ -70,7 +70,16 @@ struct microcode_ops { > * support available) and (not) ops->apply_microcode (i.e. read only). > * Otherwise, all hooks must be filled in. > */ > +#ifdef CONFIG_AMD_CPU > void ucode_probe_amd(struct microcode_ops *ops); > +#else > +static inline void ucode_probe_amd(struct microcode_ops *ops) {} > +#endif > + > +#ifdef CONFIG_INTEL_CPU > void ucode_probe_intel(struct microcode_ops *ops); > +#else > +static inline void ucode_probe_intel(struct microcode_ops *ops) {} > +#endif > > #endif /* ASM_X86_MICROCODE_PRIVATE_H */ > -- > 2.30.2 > >
On 09/04/2024 10:42 pm, Stefano Stabellini wrote: > On Fri, 27 Oct 2023, 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@citrix.com> > I read the discussion here: > https://marc.info/?l=xen-devel&m=169865507432363 > https://lore.kernel.org/xen-devel/ZT9yNrdoCKZs3_uY@macbook/ > > I think we want two top-level simple CONFIG_AMD and CONFIG_INTEL > options. Do we also want finer granular sub-options such as > CONFIG_AMD_CPU and CONFIG_INTEL_CPU, which would be controlled by the > top-level options CONFIG_AMD and CONFIG_INTEL? I think we could go > either way. CONFIG_{AMD,INTEL} could be sufficient, but also having > them control CONFIG_{AMD,INTEL}_CPU and CONFIG_{AMD,INTEL}_IOMMU is > fine too. > > We already have CONFIG_{AMD,INTEL}_IOMMU. At the time I wondered if we > could have just used CONFIG_{AMD,INTEL} for evertything. But given we > have CONFIG_{AMD,INTEL}_IOMMU, I can see why the reviewers suggested to > use CONFIG_{AMD,INTEL}_CPU. > > I would have introduced CONFIG_{AMD,INTEL} only, given that it is not > possible to have an AMD CPU with an INTEL IOMMU and vice versa. > > Anyway, I don't really have a strong opinion either way but we need this > patch to move forward one way or another so: > > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> FWIW, I have a very strong preference for the v2 of this patch (which is just simply CONFIG_{AMD,INTEL}), rather than this instance of it. Subdividing it is a mistake IMO, as it draws boundaries that doesn't exist in reality. There's nothing CONFIG_{AMD,INTEL} can reasonably be used for which isn't this purpose. Having IOMMU separate makes sense at least in principle. There are (well - were) some systems without an IOMMU, and you could be targetting one of those. However, there's nothing you can reasonably to to pick and choose between microcode loading vs the other large number of $foo/{intel,amd}.c splits we have through the codebase. ~Andrew
diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig index eac77573bd75..d9eacdd7e0fa 100644 --- a/xen/arch/x86/Kconfig +++ b/xen/arch/x86/Kconfig @@ -49,6 +49,8 @@ config HAS_CC_CET_IBT menu "Architecture Features" +source "arch/x86/Kconfig.cpu" + source "arch/Kconfig" config PV diff --git a/xen/arch/x86/Kconfig.cpu b/xen/arch/x86/Kconfig.cpu new file mode 100644 index 000000000000..b68c41977a3b --- /dev/null +++ b/xen/arch/x86/Kconfig.cpu @@ -0,0 +1,22 @@ +menu "Supported CPU vendors" + visible if EXPERT + +config AMD_CPU + bool "AMD" + default y + help + Detection, tunings and quirks for AMD platforms. + + May be turned off in builds targetting other vendors. Otherwise, + must be enabled for Xen to work suitably on AMD platforms. + +config INTEL_CPU + bool "Intel" + default y + help + Detection, tunings and quirks for Intel platforms. + + May be turned off in builds targetting other vendors. Otherwise, + must be enabled for Xen to work suitably on Intel platforms. + +endmenu diff --git a/xen/arch/x86/cpu/microcode/Makefile b/xen/arch/x86/cpu/microcode/Makefile index aae235245b06..194ddc239ee3 100644 --- a/xen/arch/x86/cpu/microcode/Makefile +++ b/xen/arch/x86/cpu/microcode/Makefile @@ -1,3 +1,3 @@ -obj-y += amd.o +obj-$(CONFIG_AMD_CPU) += amd.o obj-y += core.o -obj-y += intel.o +obj-$(CONFIG_INTEL_CPU) += intel.o diff --git a/xen/arch/x86/cpu/microcode/private.h b/xen/arch/x86/cpu/microcode/private.h index b58611e908aa..bb329933ac89 100644 --- a/xen/arch/x86/cpu/microcode/private.h +++ b/xen/arch/x86/cpu/microcode/private.h @@ -70,7 +70,16 @@ struct microcode_ops { * support available) and (not) ops->apply_microcode (i.e. read only). * Otherwise, all hooks must be filled in. */ +#ifdef CONFIG_AMD_CPU void ucode_probe_amd(struct microcode_ops *ops); +#else +static inline void ucode_probe_amd(struct microcode_ops *ops) {} +#endif + +#ifdef CONFIG_INTEL_CPU void ucode_probe_intel(struct microcode_ops *ops); +#else +static inline void ucode_probe_intel(struct microcode_ops *ops) {} +#endif #endif /* ASM_X86_MICROCODE_PRIVATE_H */
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@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Wei Liu <wl@xen.org> CC: Alejandro Vallejo <alejandro.vallejo@cloud.com> CC: Stefano Stabellini <stefano.stabellini@amd.com> CC: Xenia Ragiadakou <xenia.ragiadakou@amd.com> 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 v2.5: * Fix indentation * Rename with _CPU suffixes as requested --- xen/arch/x86/Kconfig | 2 ++ xen/arch/x86/Kconfig.cpu | 22 ++++++++++++++++++++++ xen/arch/x86/cpu/microcode/Makefile | 4 ++-- xen/arch/x86/cpu/microcode/private.h | 9 +++++++++ 4 files changed, 35 insertions(+), 2 deletions(-) create mode 100644 xen/arch/x86/Kconfig.cpu