Attention is currently required from: Angel Pons, Arthur Heymans, Felix Held, Felix Singer, Jeff Daly, Lance Zhao, Matt DeVillier, Paul Menzel, Sean Rhodes, Tim Wawrzynczak, Vanessa Eusebio.
Hello Angel Pons, Felix Held, Jeff Daly, Lance Zhao, Matt DeVillier, Nico Huber, Sean Rhodes, Tim Wawrzynczak, Vanessa Eusebio, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/75729?usp=email
to look at the new patch set (#8).
The following approvals got outdated and were removed:
Verified-1 by build bot (Jenkins)
Change subject: acpi: Move ECAM resource below PNP0C02 device in a common place
......................................................................
acpi: Move ECAM resource below PNP0C02 device in a common place
From the Linux documentation (Documentation/PCI/acpi-info.rst):
[6] PCI Firmware 3.2, sec 4.1.2:
If the operating system does not natively comprehend reserving the
MMCFG region, the MMCFG region must be reserved by firmware. The
address range reported in the MCFG table or by _CBA method (see Section
4.1.3) must be reserved by declaring a motherboard resource. For most
systems, the motherboard resource would appear at the root of the ACPI
namespace (under \_SB) in a node with a _HID of EISAID (PNP0C02), and
the resources in this case should not be claimed in the root PCI bus’s
_CRS. The resources can optionally be returned in Int15 E820 or
EFIGetMemoryMap as reserved memory but must always be reported through
ACPI as a motherboard resource.
So in order for the OS to use ECAM MMCONF over legacy PCI IO
configuration, a PNP0C02 HID device needs to reserve this region.
As no AMD platform has this defined in DSDT this fixes Linux using
legacy PCI IO configuration over MMCONF. Tianocore messes with e820
table in such a way that it prevents Linux from using PCIe ECAM. This
change fixes that problem.
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
Change-Id: I852e393726a1b086cf582f4d2d707e7cde05cbf4
---
M src/acpi/dsdt_top.asl
M src/northbridge/intel/gm45/acpi/gm45.asl
M src/northbridge/intel/haswell/acpi/hostbridge.asl
M src/northbridge/intel/i945/acpi/i945.asl
M src/northbridge/intel/ironlake/acpi/ironlake.asl
M src/northbridge/intel/pineview/acpi/pineview.asl
M src/northbridge/intel/sandybridge/acpi/sandybridge.asl
M src/northbridge/intel/x4x/acpi/x4x.asl
M src/soc/intel/apollolake/acpi/northbridge.asl
M src/soc/intel/baytrail/acpi/southcluster.asl
M src/soc/intel/braswell/acpi/southcluster.asl
M src/soc/intel/common/block/acpi/acpi/northbridge.asl
M src/soc/intel/denverton_ns/acpi/northcluster.asl
M src/soc/intel/skylake/acpi/systemagent.asl
14 files changed, 23 insertions(+), 44 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/75729/8
--
To view, visit https://review.coreboot.org/c/coreboot/+/75729?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I852e393726a1b086cf582f4d2d707e7cde05cbf4
Gerrit-Change-Number: 75729
Gerrit-PatchSet: 8
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jeff Daly <jeffd(a)silicom-usa.com>
Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Attention: Jeff Daly <jeffd(a)silicom-usa.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Attention: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nicolas Provost.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/75995?usp=email
to look at the new patch set (#5).
Change subject: cpu/intel/turbo: rework of Intel Turbo Boost code
......................................................................
cpu/intel/turbo: rework of Intel Turbo Boost code
The CPUID(6) flag associated with the Turbo Boost mode is not a
capability one, but is set to one once Turbo Boost has been enabled by
clearing the bit 38 of msr IA32_MISC_ENABLE (see issue #439).
This patch also adds the CPU_INTEL_TURBO_DISABLE configuration option
to disallow activation of the feature.
See the NOTE in cpu/intel/turbo/turbo.c for more details.
This was tested on a Lenovo L420.
Change-Id: I1e9b51e52812600b36eba65eee5d6b5521ce2af2
Signed-off-by: Nicolas Provost <dev(a)npsoft.fr>
---
M src/cpu/intel/turbo/Kconfig
M src/cpu/intel/turbo/turbo.c
M src/include/cpu/intel/turbo.h
3 files changed, 37 insertions(+), 85 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/75995/5
--
To view, visit https://review.coreboot.org/c/coreboot/+/75995?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1e9b51e52812600b36eba65eee5d6b5521ce2af2
Gerrit-Change-Number: 75995
Gerrit-PatchSet: 5
Gerrit-Owner: Nicolas Provost
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nicolas Provost
Gerrit-MessageType: newpatchset
Attention is currently required from: Angel Pons, Arthur Heymans, Felix Held, Felix Singer, Jeff Daly, Lance Zhao, Matt DeVillier, Paul Menzel, Sean Rhodes, Tim Wawrzynczak, Vanessa Eusebio.
Hello Angel Pons, Felix Held, Jeff Daly, Lance Zhao, Matt DeVillier, Nico Huber, Sean Rhodes, Tim Wawrzynczak, Vanessa Eusebio, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/75729?usp=email
to look at the new patch set (#7).
The following approvals got outdated and were removed:
Verified-1 by build bot (Jenkins)
Change subject: acpi: Move ECAM resource below PNP0C02 device in a common place
......................................................................
acpi: Move ECAM resource below PNP0C02 device in a common place
From the Linux documentation (Documentation/PCI/acpi-info.rst):
[6] PCI Firmware 3.2, sec 4.1.2:
If the operating system does not natively comprehend reserving the
MMCFG region, the MMCFG region must be reserved by firmware. The
address range reported in the MCFG table or by _CBA method (see Section
4.1.3) must be reserved by declaring a motherboard resource. For most
systems, the motherboard resource would appear at the root of the ACPI
namespace (under \_SB) in a node with a _HID of EISAID (PNP0C02), and
the resources in this case should not be claimed in the root PCI bus’s
_CRS. The resources can optionally be returned in Int15 E820 or
EFIGetMemoryMap as reserved memory but must always be reported through
ACPI as a motherboard resource.
So in order for the OS to use ECAM MMCONF over legacy PCI IO
configuration, a PNP0C02 HID device needs to reserve this region.
As no AMD platform has this defined in DSDT this fixes Linux using
legacy PCI IO configuration over MMCONF. Tianocore messes with e820
table in such a way that it prevents Linux from using PCIe ECAM. This
change fixes that problem.
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
Change-Id: I852e393726a1b086cf582f4d2d707e7cde05cbf4
---
M src/acpi/dsdt_top.asl
M src/northbridge/intel/gm45/acpi/gm45.asl
M src/northbridge/intel/haswell/acpi/hostbridge.asl
M src/northbridge/intel/i945/acpi/i945.asl
M src/northbridge/intel/ironlake/acpi/ironlake.asl
M src/northbridge/intel/pineview/acpi/pineview.asl
M src/northbridge/intel/sandybridge/acpi/sandybridge.asl
M src/northbridge/intel/x4x/acpi/x4x.asl
M src/soc/intel/apollolake/acpi/northbridge.asl
M src/soc/intel/baytrail/acpi/southcluster.asl
M src/soc/intel/braswell/acpi/southcluster.asl
M src/soc/intel/common/block/acpi/acpi/northbridge.asl
M src/soc/intel/denverton_ns/acpi/northcluster.asl
M src/soc/intel/skylake/acpi/systemagent.asl
14 files changed, 23 insertions(+), 44 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/75729/7
--
To view, visit https://review.coreboot.org/c/coreboot/+/75729?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I852e393726a1b086cf582f4d2d707e7cde05cbf4
Gerrit-Change-Number: 75729
Gerrit-PatchSet: 7
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jeff Daly <jeffd(a)silicom-usa.com>
Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Attention: Jeff Daly <jeffd(a)silicom-usa.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Attention: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nicolas Provost.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/75995?usp=email
to look at the new patch set (#4).
Change subject: cpu/intel/turbo: rework of Intel Turbo Boost code
......................................................................
cpu/intel/turbo: rework of Intel Turbo Boost code
The CPUID(6) flag associated with the Turbo Boost mode is not a
capability one, but is set to one once Turbo Boost has been enabled by
clearing the bit 38 of msr IA32_MISC_ENABLE (see issue #439).
This patch also adds the CPU_INTEL_TURBO_DISABLE configuration option
to disallow activation of the feature.
See the NOTE in cpu/intel/turbo/turbo.c for more details.
This was tested on a Lenovo L420.
Change-Id: I1e9b51e52812600b36eba65eee5d6b5521ce2af2
Signed-off-by: Nicolas Provost <dev(a)npsoft.fr>
---
M src/cpu/intel/turbo/Kconfig
M src/cpu/intel/turbo/turbo.c
M src/include/cpu/intel/turbo.h
3 files changed, 37 insertions(+), 88 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/75995/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/75995?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1e9b51e52812600b36eba65eee5d6b5521ce2af2
Gerrit-Change-Number: 75995
Gerrit-PatchSet: 4
Gerrit-Owner: Nicolas Provost
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nicolas Provost
Gerrit-MessageType: newpatchset
Attention is currently required from: Angel Pons, Arthur Heymans, Eric Lai, Felix Held, Felix Singer, Jeff Li, Johnny Lin, Jonathan Zhang, Martin L Roth, Martin Roth, Nico Huber, Nill Ge, Paul Menzel, Subrata Banik, TangYiwei, Ziang Wang, niehaitao(a)bytedance.com.
Lean Sheng Tan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75737?usp=email )
Change subject: arch/x86/smbios: Add a config string for BIOS Vendor in SMBIOS Type 0
......................................................................
Patch Set 5:
(3 comments)
File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/75737/comment/0dfc6573_5027814a :
PS2, Line 894: BIOS_VENDOR
> The other attributes and methods related to the type0 table don't start with MAINBOARD_ as well. […]
Done
File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/75737/comment/a339c792_e130e51c :
PS4, Line 895: string
: prompt "SMBIOS BIOS Vendor string"
> To align with the style of the other options below, please write like it like this. […]
Done
https://review.coreboot.org/c/coreboot/+/75737/comment/5fa6b24e_95ea9406 :
PS4, Line 899: string
> Same here, please remove.
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/75737?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6dfcca338ffc48b150c966b9aefcefe928704d24
Gerrit-Change-Number: 75737
Gerrit-PatchSet: 5
Gerrit-Owner: TangYiwei
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jeff Li <lijinfeng01(a)inspur.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Nill Ge <geshijian(a)bytedance.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Ziang Wang <ziang.wang(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: niehaitao(a)bytedance.com
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Martin L Roth <gaumless(a)gmail.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Nill Ge <geshijian(a)bytedance.com>
Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: TangYiwei
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: niehaitao(a)bytedance.com
Gerrit-Attention: Ziang Wang <ziang.wang(a)intel.com>
Gerrit-Attention: Jeff Li <lijinfeng01(a)inspur.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Wed, 21 Jun 2023 11:23:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Comment-In-Reply-To: TangYiwei
Gerrit-MessageType: comment
Attention is currently required from: Nicolas Provost.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/75995?usp=email
to look at the new patch set (#3).
Change subject: cpu/intel/turbo: rework of Intel Turbo Boost code
......................................................................
cpu/intel/turbo: rework of Intel Turbo Boost code
The CPUID(6) flag associated with the Turbo Boost mode is not a
capability one, but is set to one once Turbo Boost has been enabled by
clearing the bit 38 of msr IA32_MISC_ENABLE (see issue #439).
This patch also adds the CPU_INTEL_TURBO_DISABLE configuration option
to disallow activation of the feature.
See the NOTE in cpu/intel/turbo/turbo.c for more details.
This was tested on a Lenovo L420.
Change-Id: I1e9b51e52812600b36eba65eee5d6b5521ce2af2
Signed-off-by: Nicolas Provost <dev(a)npsoft.fr>
---
M src/cpu/intel/turbo/Kconfig
M src/cpu/intel/turbo/turbo.c
M src/include/cpu/intel/turbo.h
3 files changed, 38 insertions(+), 88 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/75995/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/75995?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1e9b51e52812600b36eba65eee5d6b5521ce2af2
Gerrit-Change-Number: 75995
Gerrit-PatchSet: 3
Gerrit-Owner: Nicolas Provost
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nicolas Provost
Gerrit-MessageType: newpatchset
Attention is currently required from: Arthur Heymans, Christian Walter, David Hendricks, Elyes Haouas, Felix Singer, Jonathan Zhang, Nico Huber, Nill Ge, Patrick Rudolph, Paul Menzel, TangYiwei, niehaitao(a)bytedance.com.
Lean Sheng Tan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75722?usp=email )
Change subject: mb/bytedance: Add 2 SPR sockets server board bd_egs
......................................................................
Patch Set 12:
(1 comment)
File src/mainboard/bytedance/bd_egs/Kconfig:
https://review.coreboot.org/c/coreboot/+/75722/comment/3c5192b4_92c62017 :
PS10, Line 32: default "\\_SB.C%03d"
> As I mentioned it's uncommon to override this at the mainboard […]
Let me confirm this part with other people and get back here, on what is the best value to put it in.
--
To view, visit https://review.coreboot.org/c/coreboot/+/75722?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I091bc78e39cd76b3c6b9a10a1fcf58e9d671ef5d
Gerrit-Change-Number: 75722
Gerrit-PatchSet: 12
Gerrit-Owner: TangYiwei
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Nill Ge <geshijian(a)bytedance.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: niehaitao(a)bytedance.com
Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: laykong <lay.kong(a)intel.com>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: niehaitao(a)bytedance.com
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Nill Ge <geshijian(a)bytedance.com>
Gerrit-Attention: TangYiwei
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Comment-Date: Wed, 21 Jun 2023 11:17:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: TangYiwei
Gerrit-MessageType: comment
Attention is currently required from: Nicolas Provost.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/75995?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Verified-1 by build bot (Jenkins)
Change subject: cpu/intel/turbo: rework of Intel Turbo Boost code
......................................................................
cpu/intel/turbo: rework of Intel Turbo Boost code
The CPUID(6) flag associated with the Turbo Boost mode is not a
capability one, but is set to one once Turbo Boost has been enabled by
clearing the bit 38 of msr IA32_MISC_ENABLE (see issue #439).
This patch also adds the CPU_INTEL_TURBO_DISABLE configuration option
to disallow activation of the feature.
See the NOTE in cpu/intel/turbo/turbo.c for more details.
This was tested on a Lenovo L420.
Change-Id: I1e9b51e52812600b36eba65eee5d6b5521ce2af2
Signed-off-by: Nicolas Provost <dev(a)npsoft.fr>
---
M src/cpu/intel/turbo/Kconfig
M src/cpu/intel/turbo/turbo.c
M src/include/cpu/intel/turbo.h
3 files changed, 38 insertions(+), 88 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/75995/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/75995?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1e9b51e52812600b36eba65eee5d6b5521ce2af2
Gerrit-Change-Number: 75995
Gerrit-PatchSet: 2
Gerrit-Owner: Nicolas Provost
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nicolas Provost
Gerrit-MessageType: newpatchset
Nicolas Provost has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/75995?usp=email )
Change subject: cpu/intel/turbo: rework of Intel Turbo Boost code
......................................................................
cpu/intel/turbo: rework of Intel Turbo Boost code
The CPUID(6) flag associated with the Turbo Boost mode is not a
capability one, but is set to one once Turbo Boost has been enabled by
clearing the bit 38 of msr IA32_MISC_ENABLE (see issue #439).
This patch also adds the CPU_INTEL_TURBO_DISABLE configuration option
to disallow activation of the feature.
See the NOTE in cpu/intel/turbo/turbo.c for more details.
This was tested on a Lenovo L420.
Change-Id: I1e9b51e52812600b36eba65eee5d6b5521ce2af2
Signed-off-by: Nicolas Provost <dev(a)npsoft.fr>
---
M src/cpu/intel/turbo/Kconfig
M src/cpu/intel/turbo/turbo.c
2 files changed, 37 insertions(+), 81 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/75995/1
diff --git a/src/cpu/intel/turbo/Kconfig b/src/cpu/intel/turbo/Kconfig
index 5432c28..f631235 100644
--- a/src/cpu/intel/turbo/Kconfig
+++ b/src/cpu/intel/turbo/Kconfig
@@ -1,6 +1,14 @@
+config CPU_INTEL_TURBO_DISABLE
+ bool "Disallow activation of Intel Turbo Boost"
+ def_bool n
+ depends on CPU_INTEL_COMMON
+ help
+ If set, this option will disallow Turbo Boost to be enabled on
+ Intel cpus (if turbo mode is available).
config CPU_INTEL_TURBO_NOT_PACKAGE_SCOPED
def_bool n
+ depends on ! CPU_INTEL_TURBO_DISABLE
help
This option indicates that the turbo mode setting is not package
scoped. i.e. enable_turbo() needs to be called on not just the bsp
diff --git a/src/cpu/intel/turbo/turbo.c b/src/cpu/intel/turbo/turbo.c
index 0b3b782..2698c70 100644
--- a/src/cpu/intel/turbo/turbo.c
+++ b/src/cpu/intel/turbo/turbo.c
@@ -5,82 +5,33 @@
#include <cpu/intel/turbo.h>
#include <cpu/x86/msr.h>
-#if CONFIG(CPU_INTEL_TURBO_NOT_PACKAGE_SCOPED)
-static inline int get_global_turbo_state(void)
-{
- return TURBO_UNKNOWN;
-}
-
-static inline void set_global_turbo_state(int state)
-{
-}
-#else
-static int g_turbo_state = TURBO_UNKNOWN;
-
-static inline int get_global_turbo_state(void)
-{
- return g_turbo_state;
-}
-
-static inline void set_global_turbo_state(int state)
-{
- g_turbo_state = state;
-}
-#endif
-
-static const char *const turbo_state_desc[] = {
- [TURBO_UNKNOWN] = "unknown",
- [TURBO_UNAVAILABLE] = "unavailable",
- [TURBO_DISABLED] = "available but hidden",
- [TURBO_ENABLED] = "available and visible"
-};
-
-/*
- * Try to update the global Turbo state.
+/* NOTE:
+ *
+ * The CPUID(6) PM_CAP_TURBO_MODE bit is not a capability flag, but
+ * indicates that Turbo Mode has been enabled previously (Intel IA32
+ * architecture Developer[s manual volume 3 14.3.2.2).
+ *
+ * There is no way to know if the cpu has support for Turbo Boost
+ * but has previously been disabled. Commonly, if turbo is supported,
+ * the associated bit in IA32_MISC_ENABLE msr (38) is 1 on startup, and
+ * then cleared to enable turbo.
+ *
+ * Furthermore, some cpus require to enable Turbo Mode on each core
+ * (config option CPU_INTEL_TURBO_NOT_PACKAGE_SCOPED) whereas others
+ * need this only for the boot one (BSP).
*/
-static int update_turbo_state(void)
-{
- struct cpuid_result cpuid_regs;
- int turbo_en, turbo_cap;
- msr_t msr;
- int turbo_state = get_global_turbo_state();
-
- cpuid_regs = cpuid(CPUID_LEAF_PM);
- turbo_cap = !!(cpuid_regs.eax & PM_CAP_TURBO_MODE);
-
- msr = rdmsr(IA32_MISC_ENABLE);
- turbo_en = !(msr.hi & H_MISC_DISABLE_TURBO);
-
- if (!turbo_cap && turbo_en) {
- /* Unavailable */
- turbo_state = TURBO_UNAVAILABLE;
- } else if (!turbo_cap && !turbo_en) {
- /* Available but disabled */
- turbo_state = TURBO_DISABLED;
- } else if (turbo_cap && turbo_en) {
- /* Available */
- turbo_state = TURBO_ENABLED;
- }
-
- set_global_turbo_state(turbo_state);
- printk(BIOS_INFO, "Turbo is %s\n", turbo_state_desc[turbo_state]);
-
- return turbo_state;
-}
/*
- * Determine the current state of Turbo and cache it for later. Turbo is package
- * level config so it does not need to be enabled on every core.
+ * Returns TURBO_ENABLED if Turbo Boost has been enabled, or
+ * TURBO_DISABLED.
*/
int get_turbo_state(void)
{
- int turbo_state = get_global_turbo_state();
+ struct cpuid_result cpuid_regs;
- /* Return cached state if available */
- if (turbo_state == TURBO_UNKNOWN)
- turbo_state = update_turbo_state();
-
- return turbo_state;
+ cpuid_regs = cpuid(CPUID_LEAF_PM);
+ return (cpuid_regs.eax & PM_CAP_TURBO_MODE) ?
+ TURBO_ENABLED : TURBO_DISABLED;
}
/*
@@ -88,17 +39,17 @@
*/
void enable_turbo(void)
{
+ if (CONFIG(CPU_INTEL_TURBO_DISABLE))
+ return;
+
msr_t msr;
- /* Only possible if turbo is available but hidden */
- if (get_turbo_state() == TURBO_DISABLED) {
+ msr = rdmsr(IA32_MISC_ENABLE);
+ if (msr.hi & H_MISC_DISABLE_TURBO) {
/* Clear Turbo Disable bit in Misc Enables */
- msr = rdmsr(IA32_MISC_ENABLE);
- msr.hi &= ~H_MISC_DISABLE_TURBO;
+ msr.hi &= ~ H_MISC_DISABLE_TURBO;
wrmsr(IA32_MISC_ENABLE, msr);
-
- /* Update cached turbo state */
- update_turbo_state();
+ printk(BIOS_INFO, "Turbo Boost is enabled\n");
}
}
@@ -109,14 +60,11 @@
{
msr_t msr;
- /* Only possible if turbo is available and visible */
- if (get_turbo_state() == TURBO_ENABLED) {
+ if (get_turbo_state () == TURBO_ENABLED) {
/* Set Turbo Disable bit in Misc Enables */
msr = rdmsr(IA32_MISC_ENABLE);
msr.hi |= H_MISC_DISABLE_TURBO;
wrmsr(IA32_MISC_ENABLE, msr);
-
- /* Update cached turbo state */
- update_turbo_state();
+ printk(BIOS_INFO, "Turbo Boost is disabled\n");
}
}
--
To view, visit https://review.coreboot.org/c/coreboot/+/75995?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1e9b51e52812600b36eba65eee5d6b5521ce2af2
Gerrit-Change-Number: 75995
Gerrit-PatchSet: 1
Gerrit-Owner: Nicolas Provost
Gerrit-MessageType: newchange
Eric Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75756?usp=email )
Change subject: arch/x86: Introduce DUMP_SMBIOS_TYPE17 config
......................................................................
Patch Set 15:
(1 comment)
File src/arch/x86/Kconfig:
https://review.coreboot.org/c/coreboot/+/75756/comment/af0fcc97_dd9136d8 :
PS15, Line 363: .
> Option name strings should not end with a period
I'll clean it up next week.
--
To view, visit https://review.coreboot.org/c/coreboot/+/75756?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2b5ca1f4a59598531a6cba500672c2717f2a7b00
Gerrit-Change-Number: 75756
Gerrit-PatchSet: 15
Gerrit-Owner: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Comment-Date: Wed, 21 Jun 2023 11:12:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-MessageType: comment