Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44210 )
Change subject: cpu/intel/common: Add `intel_ht_supported` function ......................................................................
cpu/intel/common: Add `intel_ht_supported` function
Change-Id: I90c0378c4042dec39c8c86c1d2339a5cbcfe78e3 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/cpu/intel/common/common.h M src/cpu/intel/common/hyperthreading.c 2 files changed, 14 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/44210/1
diff --git a/src/cpu/intel/common/common.h b/src/cpu/intel/common/common.h index 57a5fe6..df14668 100644 --- a/src/cpu/intel/common/common.h +++ b/src/cpu/intel/common/common.h @@ -3,7 +3,7 @@ #ifndef _CPU_INTEL_COMMON_H #define _CPU_INTEL_COMMON_H
-#include <stdint.h> +#include <types.h>
void set_vmx_and_lock(void); void set_feature_ctrl_vmx(void); @@ -18,6 +18,11 @@ void cpu_init_cppc_config(struct cppc_config *config, u32 version);
/* + * Returns true if CPU supports Hyper-Threading. + */ +bool intel_ht_supported(void); + +/* * Returns true if it's not thread 0 on a hyperthreading enabled core. */ bool intel_ht_sibling(void); diff --git a/src/cpu/intel/common/hyperthreading.c b/src/cpu/intel/common/hyperthreading.c index d686149..fce7576 100644 --- a/src/cpu/intel/common/hyperthreading.c +++ b/src/cpu/intel/common/hyperthreading.c @@ -3,6 +3,13 @@ #include <cpu/x86/lapic.h> #include <cpu/intel/common/common.h> #include <arch/cpu.h> +#include <types.h> + +bool intel_ht_supported(void) +{ + /* Is HyperThreading supported? */ + return !!(cpuid_edx(1) & CPUID_FEAURE_HTT); +}
/* * Return true if running thread does not have the smallest lapic ID @@ -13,8 +20,7 @@ struct cpuid_result result; unsigned int core_ids, apic_ids, threads;
- /* Is Hyper-Threading supported */ - if (!(cpuid_edx(1) & CPUID_FEAURE_HTT)) + if (!intel_ht_supported()) return false;
apic_ids = 1;
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44210 )
Change subject: cpu/intel/common: Add `intel_ht_supported` function ......................................................................
Patch Set 1: Code-Review+2
Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44210 )
Change subject: cpu/intel/common: Add `intel_ht_supported` function ......................................................................
cpu/intel/common: Add `intel_ht_supported` function
Change-Id: I90c0378c4042dec39c8c86c1d2339a5cbcfe78e3 Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/44210 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Patrick Rudolph siro@das-labor.org --- M src/cpu/intel/common/common.h M src/cpu/intel/common/hyperthreading.c 2 files changed, 14 insertions(+), 3 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Rudolph: Looks good to me, approved
diff --git a/src/cpu/intel/common/common.h b/src/cpu/intel/common/common.h index 57a5fe6..df14668 100644 --- a/src/cpu/intel/common/common.h +++ b/src/cpu/intel/common/common.h @@ -3,7 +3,7 @@ #ifndef _CPU_INTEL_COMMON_H #define _CPU_INTEL_COMMON_H
-#include <stdint.h> +#include <types.h>
void set_vmx_and_lock(void); void set_feature_ctrl_vmx(void); @@ -18,6 +18,11 @@ void cpu_init_cppc_config(struct cppc_config *config, u32 version);
/* + * Returns true if CPU supports Hyper-Threading. + */ +bool intel_ht_supported(void); + +/* * Returns true if it's not thread 0 on a hyperthreading enabled core. */ bool intel_ht_sibling(void); diff --git a/src/cpu/intel/common/hyperthreading.c b/src/cpu/intel/common/hyperthreading.c index d686149..fce7576 100644 --- a/src/cpu/intel/common/hyperthreading.c +++ b/src/cpu/intel/common/hyperthreading.c @@ -3,6 +3,13 @@ #include <cpu/x86/lapic.h> #include <cpu/intel/common/common.h> #include <arch/cpu.h> +#include <types.h> + +bool intel_ht_supported(void) +{ + /* Is HyperThreading supported? */ + return !!(cpuid_edx(1) & CPUID_FEAURE_HTT); +}
/* * Return true if running thread does not have the smallest lapic ID @@ -13,8 +20,7 @@ struct cpuid_result result; unsigned int core_ids, apic_ids, threads;
- /* Is Hyper-Threading supported */ - if (!(cpuid_edx(1) & CPUID_FEAURE_HTT)) + if (!intel_ht_supported()) return false;
apic_ids = 1;
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44210 )
Change subject: cpu/intel/common: Add `intel_ht_supported` function ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44210/2/src/cpu/intel/common/hypert... File src/cpu/intel/common/hyperthreading.c:
https://review.coreboot.org/c/coreboot/+/44210/2/src/cpu/intel/common/hypert... PS2, Line 6: include <types.h> ??
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44210 )
Change subject: cpu/intel/common: Add `intel_ht_supported` function ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44210/2/src/cpu/intel/common/hypert... File src/cpu/intel/common/hyperthreading.c:
https://review.coreboot.org/c/coreboot/+/44210/2/src/cpu/intel/common/hypert... PS2, Line 6: include <types.h>
??
When adding a single header, I'd rather include more than necessary than accidentally rely on indirect inclusion. Especially for types.
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44210 )
Change subject: cpu/intel/common: Add `intel_ht_supported` function ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44210/2/src/cpu/intel/common/hypert... File src/cpu/intel/common/hyperthreading.c:
https://review.coreboot.org/c/coreboot/+/44210/2/src/cpu/intel/common/hypert... PS2, Line 6: include <types.h>
When adding a single header, I'd rather include more than necessary than accidentally rely on indire […]
for a single macro you added 5 herders ! in that case why don we merge all src/include files into one file .... ?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44210 )
Change subject: cpu/intel/common: Add `intel_ht_supported` function ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44210/2/src/cpu/intel/common/hypert... File src/cpu/intel/common/hyperthreading.c:
https://review.coreboot.org/c/coreboot/+/44210/2/src/cpu/intel/common/hypert... PS2, Line 6: include <types.h>
for a single macro you added 5 herders ! […]
Which single macro?
-
Angel Pons (Code Review) -
HAOUAS Elyes (Code Review)
-
Patrick Rudolph (Code Review)