Attention is currently required from: Arthur Heymans, Lance Zhao, Tim Wawrzynczak.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76008?usp=email )
Change subject: acpi/acpi.c: Split of ACPI table generation into separate files
......................................................................
Patch Set 1:
(1 comment)
File src/acpi/acpi.c:
https://review.coreboot.org/c/coreboot/+/76008/comment/7f27453c_a995693b :
PS1, Line 213: #if ENV_X86
Just wondering.. This pp if is missing in the new files. Is that intended? I would assume it should vanish with CB:76009. Just something I noticed. Please mark as resolved if you want to.
--
To view, visit https://review.coreboot.org/c/coreboot/+/76008?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: I5083b26c0d4cc6764b4e3cb0ff586797cae7e3af
Gerrit-Change-Number: 76008
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Thu, 22 Jun 2023 21:33:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans.
Hello Arthur Heymans,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/76090?usp=email
to look at the new patch set (#2).
Change subject: allocator_v4: Fix top-level allocations w/o IORESOURCE_ABOVE_4G
......................................................................
allocator_v4: Fix top-level allocations w/o IORESOURCE_ABOVE_4G
When moving the code to allocate at the top level in commit 9260ea60bfa4
(allocator_v4: Use memranges only for toplevel), a call to restrict the
limit of the resource was dropped. Probably by accident in one of the
earliest rebases. Without this call to effective_limit(), 64-bit resour-
ces at the top level, i.e. PCI bus 0, were always placed above 4G. Even
when this was not requested with the IORESOURCE_ABOVE_4G flag.
Change-Id: Ied3a0695ef5e91f092bf2d442c1c482057643483
Signed-off-by: Nico Huber <nico.h(a)gmx.de>
---
M src/device/resource_allocator_v4.c
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/76090/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/76090?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: Ied3a0695ef5e91f092bf2d442c1c482057643483
Gerrit-Change-Number: 76090
Gerrit-PatchSet: 2
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: newpatchset
Attention is currently required from: Arthur Heymans.
Hello Arthur Heymans,
I'd like you to do a code review.
Please visit
https://review.coreboot.org/c/coreboot/+/76090?usp=email
to review the following change.
Change subject: allocator_v4: Fix top-level allocations w/o IORESOURCE_ABOVE_4G
......................................................................
allocator_v4: Fix top-level allocations w/o IORESOURCE_ABOVE_4G
When moving the code to allocate at the top level in commit 9260ea60bfa4
(allocator_v4: Use memranges only for toplevel), a call to restrict the
limit of the resource was dropped. Probably by accident in one of the
earliest rebases. Without this call to effective_limit(), 64-bit resour-
ces at the top level, i.e. PCI bus 0, were always placed above 4G. Even
when this was not requestion with the IORESOURCE_ABOVE_4G flag.
Change-Id: Ied3a0695ef5e91f092bf2d442c1c482057643483
Signed-off-by: Nico Huber <nico.h(a)gmx.de>
---
M src/device/resource_allocator_v4.c
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/76090/1
diff --git a/src/device/resource_allocator_v4.c b/src/device/resource_allocator_v4.c
index e64d8ad..96d4488 100644
--- a/src/device/resource_allocator_v4.c
+++ b/src/device/resource_allocator_v4.c
@@ -401,8 +401,8 @@
if (!res->size)
continue;
- if (!memranges_steal(&ranges, res->limit, res->size, res->align, type, &base,
- CONFIG(RESOURCE_ALLOCATION_TOP_DOWN))) {
+ if (!memranges_steal(&ranges, effective_limit(res), res->size, res->align,
+ type, &base, CONFIG(RESOURCE_ALLOCATION_TOP_DOWN))) {
printk(BIOS_ERR, "Resource didn't fit!!!\n");
print_failed_res(dev, res);
continue;
--
To view, visit https://review.coreboot.org/c/coreboot/+/76090?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: Ied3a0695ef5e91f092bf2d442c1c482057643483
Gerrit-Change-Number: 76090
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: newchange
Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/76074?usp=email )
Change subject: soc/amd/common/block/acpi/ivrs: zero-initialize ivhd_hpet struct
......................................................................
soc/amd/common/block/acpi/ivrs: zero-initialize ivhd_hpet struct
Zero-initialize the ivhd_hpet struct right at the beginning of the
ivhd_describe_hpet function to make sure that the whole struct is in a
defined state.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: If4d3563c485eed4a7cb0526a62f7b6c80f763bfd
---
M src/soc/amd/common/block/acpi/ivrs.c
1 file changed, 1 insertion(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/76074/1
diff --git a/src/soc/amd/common/block/acpi/ivrs.c b/src/soc/amd/common/block/acpi/ivrs.c
index 7afabf8..d2080e9 100644
--- a/src/soc/amd/common/block/acpi/ivrs.c
+++ b/src/soc/amd/common/block/acpi/ivrs.c
@@ -36,10 +36,9 @@
static unsigned long ivhd_describe_hpet(unsigned long current, uint8_t hndl, uint16_t src_devid)
{
ivrs_ivhd_special_t *ivhd_hpet = (ivrs_ivhd_special_t *)current;
+ memset(ivhd_hpet, 0, sizeof(*ivhd_hpet));
ivhd_hpet->type = IVHD_DEV_8_BYTE_EXT_SPECIAL_DEV;
- ivhd_hpet->reserved = 0x0000;
- ivhd_hpet->dte_setting = 0x00;
ivhd_hpet->handle = hndl;
ivhd_hpet->source_dev_id = src_devid; /* function 0 of FCH PCI device */
ivhd_hpet->variety = IVHD_SPECIAL_DEV_HPET;
--
To view, visit https://review.coreboot.org/c/coreboot/+/76074?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: If4d3563c485eed4a7cb0526a62f7b6c80f763bfd
Gerrit-Change-Number: 76074
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newchange
Attention is currently required from: Arthur Heymans, Fred Reitberger, Jason Glenesk, Matt DeVillier, Raul Rangel.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75933?usp=email )
Change subject: soc/amd/common/block/acpi/ivrs: conditionally generate eMMC entry
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/75933/comment/78d3944f_98763ba5 :
PS1, Line 12: Where the PCI_DEVFN(0x13, 1) is from is currently unclear to me. There
: is no PCI device 0x13 on bus 0 and the eMMC controller is also an MMIO
: device and not a PCI device.
> you mean a comment pointing out that this is what the reference code but where i'm not really sure w […]
done
--
To view, visit https://review.coreboot.org/c/coreboot/+/75933?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: I00865cb7caf82547e89eb5e77817e3d8ca5d35dd
Gerrit-Change-Number: 75933
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Comment-Date: Thu, 22 Jun 2023 21:19:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: comment
Attention is currently required from: Fred Reitberger, Jason Glenesk, Matt DeVillier, Raul Rangel.
Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/76072?usp=email )
Change subject: soc/amd/common/block/acpi/ivrs: zero-initialize ivhd_f0 struct
......................................................................
soc/amd/common/block/acpi/ivrs: zero-initialize ivhd_f0 struct
Zero-initialize the ivhd_f0 struct right at the beginning of the
ivhd_describe_f0_device function to make sure that the whole struct is
in a defined state.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: Ia6750b58dacb9b9192ed21128eb6e3a4495b96d0
---
M src/soc/amd/common/block/acpi/ivrs.c
1 file changed, 1 insertion(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/76072/1
diff --git a/src/soc/amd/common/block/acpi/ivrs.c b/src/soc/amd/common/block/acpi/ivrs.c
index 79b4ff6..8d127e9 100644
--- a/src/soc/amd/common/block/acpi/ivrs.c
+++ b/src/soc/amd/common/block/acpi/ivrs.c
@@ -52,6 +52,7 @@
uint16_t dev_id, uint8_t datasetting)
{
ivrs_ivhd_f0_entry_t *ivhd_f0 = (ivrs_ivhd_f0_entry_t *)current;
+ memset(ivhd_f0, 0, sizeof(*ivhd_f0));
ivhd_f0->type = IVHD_DEV_VARIABLE;
ivhd_f0->dev_id = dev_id;
@@ -65,11 +66,6 @@
ivhd_f0->hardware_id[6] = '4';
ivhd_f0->hardware_id[7] = '0';
- memset(ivhd_f0->compatible_id, 0, sizeof(ivhd_f0->compatible_id));
-
- ivhd_f0->uuid_format = 0;
- ivhd_f0->uuid_length = 0;
-
current += sizeof(ivrs_ivhd_f0_entry_t);
return current;
}
--
To view, visit https://review.coreboot.org/c/coreboot/+/76072?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: Ia6750b58dacb9b9192ed21128eb6e3a4495b96d0
Gerrit-Change-Number: 76072
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-MessageType: newchange
Attention is currently required from: Arthur Heymans, Fred Reitberger, Jason Glenesk, Matt DeVillier, Raul Rangel.
Hello Arthur Heymans, Fred Reitberger, Jason Glenesk, Matt DeVillier, Raul Rangel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/75933?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Code-Review+1 by Arthur Heymans, Code-Review+1 by Fred Reitberger, Verified+1 by build bot (Jenkins)
Change subject: soc/amd/common/block/acpi/ivrs: conditionally generate eMMC entry
......................................................................
soc/amd/common/block/acpi/ivrs: conditionally generate eMMC entry
The eMMC entry in the IVRS table should only be generated if an eMMC
controller is present in the SoC.
Where the PCI_DEVFN(0x13, 1) is from is currently unclear to me. There
is no PCI device 0x13 on bus 0 and the eMMC controller is also an MMIO
device and not a PCI device, but this is what the reference code does.
My guess would be that it mainly needs to be a unique PCI device that
won't collide with any existing PCI device in the SoC. Add a comment
about this too.
TEST=None
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: I00865cb7caf82547e89eb5e77817e3d8ca5d35dd
---
M src/soc/amd/common/block/acpi/ivrs.c
1 file changed, 9 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/75933/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/75933?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: I00865cb7caf82547e89eb5e77817e3d8ca5d35dd
Gerrit-Change-Number: 75933
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Arthur Heymans, Lance Zhao, Tim Wawrzynczak.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76009?usp=email )
Change subject: acpi/Makefile.inc: Conditionally include X86 specific code
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/76009?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: I63bbac225662377693ad5f29cc8911494c49b422
Gerrit-Change-Number: 76009
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Thu, 22 Jun 2023 21:17:42 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment