Change in ...coreboot[master]: arch/x86/cpu: Add functions to determine CPU vendor

[Patrick Rudolph (Code Review)]

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32130

Change subject: arch/x86/cpu: Add functions to determine CPU vendor
......................................................................

arch/x86/cpu: Add functions to determine CPU vendor

Add two functions to determine if CPU is made by a specific vendor.
Use Kconfig symbols and CPUID to determine the CPU vendor.

Change-Id: I1bd6c3b59cfd992f7ba507bc9f9269669920b24f
Signed-off-by: Patrick Rudolph 
---
M src/arch/x86/cpu.c
M src/arch/x86/include/arch/cpu.h
2 files changed, 34 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/32130/1

diff --git a/src/arch/x86/cpu.c b/src/arch/x86/cpu.c
index 80d4d0d..e82df00 100644
--- a/src/arch/x86/cpu.c
+++ b/src/arch/x86/cpu.c
@@ -196,6 +196,36 @@
    }
 }

+bool cpu_is_amd(void)
+{
+	if (CONFIG(CPU_AMD_AGESA) || CONFIG(CPU_AMD_PI))
+		return true;
+	else if (CONFIG(CPU_INTEL_COMMON) || CONFIG(SOC_INTEL_COMMON))
+		return false;
+	else if (cpu_have_cpuid()) {
+		struct device cpu;
+		identify_cpu(&cpu);
+		return cpu.vendor == X86_VENDOR_AMD;
+	}
+
+	return false;
+}
+
+bool cpu_is_intel(void)
+{
+	if (CONFIG(CPU_INTEL_COMMON) || CONFIG(SOC_INTEL_COMMON))
+		return true;
+	else if (CONFIG(CPU_AMD_AGESA) || CONFIG(CPU_AMD_PI))
+		return false;
+	else if (cpu_have_cpuid()) {
+		struct device cpu;
+		identify_cpu(&cpu);
+		return cpu.vendor == X86_VENDOR_INTEL;
+	}
+
+	return false;
+}
+
 struct cpu_driver *find_cpu_driver(struct device *cpu)
 {
    struct cpu_driver *driver;
diff --git a/src/arch/x86/include/arch/cpu.h b/src/arch/x86/include/arch/cpu.h
index b40dd1b..ce4ae11 100644
--- a/src/arch/x86/include/arch/cpu.h
+++ b/src/arch/x86/include/arch/cpu.h
@@ -217,6 +217,10 @@
    ci = cpu_info();
    return ci->index;
 }
+
+bool cpu_is_amd(void);
+bool cpu_is_intel(void);
+
 #endif

#ifndef __ROMCC__ // romcc is segfaulting in some cases

[Julien Viard de Galbert (Code Review)]

Julien Viard de Galbert has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32130 )

Change subject: arch/x86/cpu: Add functions to determine CPU vendor
......................................................................

Patch Set 1: Code-Review+1

Hello Patrick,

If I understand correctly you are trying to skip the call to `identify_cpu` by looking at the Kconfig options for some AMD and some Intel cpu/soc but not all of them (so you still need the call as a fallback).

I don't really see the point of having the two functions, you could call `identify_cpu`
and test cpu.vendor directly. I my opinion, the code will be easy to read in both cases.

But I might be missing something.

Anyway the code looks correct, so +1.

Best Regards,

Julien VdG

[Patrick Rudolph (Code Review)]

Hello Kyösti Mälkki, Sven Schnelle, Kane Chen, Lee Leahy, Julien Viard de Galbert, Philipp Deppenwiese, build bot (Jenkins), Patrick Georgi, Martin Roth,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/32130

to look at the new patch set (#2).

Change subject: arch/x86/cpu: Add functions to determine CPU vendor
......................................................................

arch/x86/cpu: Add functions to determine CPU vendor

Add two functions to determine if CPU is made by a specific vendor.
Use Kconfig symbols to allow link time optimizations.

Change-Id: I1bd6c3b59cfd992f7ba507bc9f9269669920b24f
Signed-off-by: Patrick Rudolph 
---
M src/arch/x86/include/arch/cpu.h
1 file changed, 10 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/32130/2

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32130 )

Change subject: arch/x86/cpu: Add functions to determine CPU vendor
......................................................................

Patch Set 2:

I decided to go the Kconfig way, as it will allow link time optimizations.

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32130 )

Change subject: arch/x86/cpu: Add functions to determine CPU vendor
......................................................................

Patch Set 2: Code-Review+1

[Julien Viard de Galbert (Code Review)]

Julien Viard de Galbert has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32130 )

Change subject: arch/x86/cpu: Add functions to determine CPU vendor
......................................................................

Patch Set 2: Code-Review+1

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32130 )

Change subject: arch/x86/cpu: Add functions to determine CPU vendor
......................................................................

Patch Set 2: Code-Review+2

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32130 )

Change subject: arch/x86/cpu: Add functions to determine CPU vendor
......................................................................

arch/x86/cpu: Add functions to determine CPU vendor

Add two functions to determine if CPU is made by a specific vendor.
Use Kconfig symbols to allow link time optimizations.

Change-Id: I1bd6c3b59cfd992f7ba507bc9f9269669920b24f
Signed-off-by: Patrick Rudolph 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32130
Tested-by: build bot (Jenkins) 
Reviewed-by: HAOUAS Elyes 
Reviewed-by: Julien Viard de Galbert 
Reviewed-by: Patrick Georgi 
---
M src/arch/x86/include/arch/cpu.h
1 file changed, 10 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Patrick Georgi: Looks good to me, approved
  HAOUAS Elyes: Looks good to me, but someone else must approve
  Julien Viard de Galbert: Looks good to me, but someone else must approve

diff --git a/src/arch/x86/include/arch/cpu.h b/src/arch/x86/include/arch/cpu.h
index b40dd1b..3e464e4 100644
--- a/src/arch/x86/include/arch/cpu.h
+++ b/src/arch/x86/include/arch/cpu.h
@@ -166,6 +166,16 @@
 void smm_lock(void);
 void smm_setup_structures(void *gnvs, void *tcg, void *smi1);

+static inline bool cpu_is_amd(void)
+{
+	return CONFIG(CPU_AMD_AGESA) || CONFIG(CPU_AMD_PI);
+}
+
+static inline bool cpu_is_intel(void)
+{
+	return CONFIG(CPU_INTEL_COMMON) || CONFIG(SOC_INTEL_COMMON);
+}
+
 #ifndef __SIMPLE_DEVICE__

struct device;