diff mbox series

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

Message ID 20231025180630.3230010-3-andrew.cooper3@citrix.com (mailing list archive)
State Superseded
Headers show
Series x86: ucode and CPU Kconfig | expand

Commit Message

Andrew Cooper Oct. 25, 2023, 6:06 p.m. UTC
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.

CC Stefano/Xenia as I know you want to go down this line, but I don't recall
patches to this effect yet.
---
 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

Comments

Jan Beulich Oct. 26, 2023, 7:55 a.m. UTC | #1
On 25.10.2023 20:06, 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.

Linux uses different names for the Kconfig symbols. While I'm unconvinced
of the SUP part, I wonder whether we wouldn't better use CPU in the names.
One immediate question here is how the IOMMU interaction is intended to
end up: Do we want to permit either vendor's CPUs with the other vendor's
IOMMUs to be usable?

> --- /dev/null
> +++ b/xen/arch/x86/Kconfig.cpu
> @@ -0,0 +1,22 @@
> +menu "Supported processor vendors"
> +	visible if EXPERT
> +
> +config AMD
> +	bool "AMD"
> +        default y
> +        help
> +          Detection, tunings and quirks for AMD processors.
> +
> +	  May be turned off in builds targetting other vendors.  Otherwise,
> +	  must be enabled for Xen to work suitably on AMD processors.
> +
> +config INTEL
> +	bool "Intel"
> +        default y
> +        help
> +          Detection, tunings and quirks for Intel processors.
> +
> +	  May be turned off in builds targetting other vendors.  Otherwise,
> +	  must be enabled for Xen to work suitably on Intel processors.
> +
> +endmenu

Nit: Throughout this hunk there's an inconsistency with indentation
(hard tabs not used in some places where they ought to be).

Also, depending on the verdict on the aspect mentioned at the top,
"processors" may want replacing by "systems" or "platforms" or some
such if we mean these to cover more than just the CPUs.

Jan
Andrew Cooper Oct. 26, 2023, 11:10 a.m. UTC | #2
On 26/10/2023 8:55 am, Jan Beulich wrote:
> On 25.10.2023 20:06, 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.
> Linux uses different names for the Kconfig symbols. While I'm unconvinced
> of the SUP part, I wonder whether we wouldn't better use CPU in the names.

I don't see what that gets us, other than a longer name.

> One immediate question here is how the IOMMU interaction is intended to
> end up: Do we want to permit either vendor's CPUs with the other vendor's
> IOMMUs to be usable?

From a randconfig point of view, yes.  These options are only targetting
a specific platform, and we can absolutely make that the end user's
responsibility to describe their platform correctly.


The more interesting question is perhaps VT-x and SVM, given that VIA
have shipped VT-x and Hygon have shipped SVM and AMD-Vi.

I do specifically want to to integrate the HVM setup better with CPU
init - KVM dropped an enormous amount of complexity by doing this - but
I expect we'll end up with VTX and SVM options rather than using
INTEL/AMD for this.

There is a bit of linkage between EPT/VT-d and NPT/AMD-Vi (in principle
at least) in the form of HAP/IOMMU pagetable sharing, but as it's "just"
an exchange of superpage sizes, iommu-pt pointer and height, I think we
can make an abstraction which doesn't force a vendor match.
>> --- /dev/null
>> +++ b/xen/arch/x86/Kconfig.cpu
>> @@ -0,0 +1,22 @@
>> +menu "Supported processor vendors"
>> +	visible if EXPERT
>> +
>> +config AMD
>> +	bool "AMD"
>> +        default y
>> +        help
>> +          Detection, tunings and quirks for AMD processors.
>> +
>> +	  May be turned off in builds targetting other vendors.  Otherwise,
>> +	  must be enabled for Xen to work suitably on AMD processors.
>> +
>> +config INTEL
>> +	bool "Intel"
>> +        default y
>> +        help
>> +          Detection, tunings and quirks for Intel processors.
>> +
>> +	  May be turned off in builds targetting other vendors.  Otherwise,
>> +	  must be enabled for Xen to work suitably on Intel processors.
>> +
>> +endmenu
> Nit: Throughout this hunk there's an inconsistency with indentation
> (hard tabs not used in some places where they ought to be).

Oh yes, that's unintended.

> Also, depending on the verdict on the aspect mentioned at the top,
> "processors" may want replacing by "systems" or "platforms" or some
> such if we mean these to cover more than just the CPUs.

I really don't want to use CPU because that term is overloaded enough
already.  Maybe it's ok in the overall menu text, but "plaform/system
vendor" would be the OEMs rather than the processor vendors.

We do have various platform quirks in Xen, but they're almost all DMI or
PCI based, rather than vendor based.

I could be persuaded to use CPU in the menu, and s/processors/platforms/
elsewhere.

~Andrew
Jan Beulich Oct. 26, 2023, 11:35 a.m. UTC | #3
On 26.10.2023 13:10, Andrew Cooper wrote:
> On 26/10/2023 8:55 am, Jan Beulich wrote:
>> On 25.10.2023 20:06, 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.
>> Linux uses different names for the Kconfig symbols. While I'm unconvinced
>> of the SUP part, I wonder whether we wouldn't better use CPU in the names.
> 
> I don't see what that gets us, other than a longer name.

Just to mention the (I think) obvious - on the IOMMU side we already have
AMD_IOMMU and INTEL_IOMMU. It would be odd to have just AMD and INTEL here,
yet then ...

>> One immediate question here is how the IOMMU interaction is intended to
>> end up: Do we want to permit either vendor's CPUs with the other vendor's
>> IOMMUs to be usable?
> 
> From a randconfig point of view, yes.  These options are only targetting
> a specific platform, and we can absolutely make that the end user's
> responsibility to describe their platform correctly.

... <vendor>_IOMMU not depending on <vendor>. Whereas the lack of a
dependency on <vendor>_CPU would be quite natural, imo.

> The more interesting question is perhaps VT-x and SVM, given that VIA
> have shipped VT-x and Hygon have shipped SVM and AMD-Vi.
> 
> I do specifically want to to integrate the HVM setup better with CPU
> init - KVM dropped an enormous amount of complexity by doing this - but
> I expect we'll end up with VTX and SVM options rather than using
> INTEL/AMD for this.

I'd certainly prefer us using VTX/SVM (and those then having dependencies
on the main || niche vendors), with the caveat that SVM also has had a
meaning for Intel for quite some time, iirc.

Jan
Andrew Cooper Oct. 26, 2023, 1:22 p.m. UTC | #4
On 26/10/2023 12:35 pm, Jan Beulich wrote:
> On 26.10.2023 13:10, Andrew Cooper wrote:
>> On 26/10/2023 8:55 am, Jan Beulich wrote:
>>> On 25.10.2023 20:06, 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.
>>> Linux uses different names for the Kconfig symbols. While I'm unconvinced
>>> of the SUP part, I wonder whether we wouldn't better use CPU in the names.
>> I don't see what that gets us, other than a longer name.
> Just to mention the (I think) obvious - on the IOMMU side we already have
> AMD_IOMMU and INTEL_IOMMU. It would be odd to have just AMD and INTEL here,
> yet then ...
>
>>> One immediate question here is how the IOMMU interaction is intended to
>>> end up: Do we want to permit either vendor's CPUs with the other vendor's
>>> IOMMUs to be usable?
>> From a randconfig point of view, yes.  These options are only targetting
>> a specific platform, and we can absolutely make that the end user's
>> responsibility to describe their platform correctly.
> ... <vendor>_IOMMU not depending on <vendor>.

Odd possibly, but not something to worry about.

It's mostly because of asymmetric marketing because while VTD is fine
and recognisable, AMD-Vi has AMD's name in it even for the non-AMD vendors.

Anyone liable to even notice in the first place will probably know
enough to understand why it's like that.

Furthermore, ...

>  Whereas the lack of a
> dependency on <vendor>_CPU would be quite natural, imo.

... this doesn't really work either as IOMMUs are non-really-optional
uncore components these days.


Names are just that - names.  They can be changed if needs be, and it's
the help text which matters to clarify the intent.

~Andrew
diff mbox series

Patch

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..0ce09b292045
--- /dev/null
+++ b/xen/arch/x86/Kconfig.cpu
@@ -0,0 +1,22 @@ 
+menu "Supported processor vendors"
+	visible if EXPERT
+
+config AMD
+	bool "AMD"
+        default y
+        help
+          Detection, tunings and quirks for AMD processors.
+
+	  May be turned off in builds targetting other vendors.  Otherwise,
+	  must be enabled for Xen to work suitably on AMD processors.
+
+config INTEL
+	bool "Intel"
+        default y
+        help
+          Detection, tunings and quirks for Intel processors.
+
+	  May be turned off in builds targetting other vendors.  Otherwise,
+	  must be enabled for Xen to work suitably on Intel processors.
+
+endmenu
diff --git a/xen/arch/x86/cpu/microcode/Makefile b/xen/arch/x86/cpu/microcode/Makefile
index aae235245b06..30d600544f45 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) += amd.o
 obj-y += core.o
-obj-y += intel.o
+obj-$(CONFIG_INTEL) += intel.o
diff --git a/xen/arch/x86/cpu/microcode/private.h b/xen/arch/x86/cpu/microcode/private.h
index b58611e908aa..da556fe5060a 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
 void ucode_probe_amd(struct microcode_ops *ops);
+#else
+static inline void ucode_probe_amd(struct microcode_ops *ops) {}
+#endif
+
+#ifdef CONFIG_INTEL
 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 */