Attention is currently required from: Angel Pons, Arthur Heymans, Felix Held, Krystian Hebel, Kyösti Mälkki, Michał Żygowski.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77338?usp=email )
Change subject: device/pciexp_device.c: Fix setting Max Payload Size
......................................................................
Patch Set 17: Code-Review+1
(11 comments)
File src/device/pciexp_device.c:
https://review.coreboot.org/c/coreboot/+/77338/comment/b0ff2479_c8c36687 :
PS11, Line 598: awlays
> Thanks for the update and your patience. […]
This ended up in the wrong thread, so closing here.
https://review.coreboot.org/c/coreboot/+/77338/comment/c813a81c_03703a19 :
PS11, Line 630: root->max_payload_set = 1;
> Ad.2. […]
Current implementation looks good to me. Krystian, Patrick, do you want to
have another look?
File src/device/pciexp_device.c:
https://review.coreboot.org/c/coreboot/+/77338/comment/125ac8bc_50c74bdd :
PS15, Line 638: pciexp_dev_set_max_payload_size(child, max_payload);
> Indeed, we only care about the MPS propagation upwards. Removed.
Acknowledged
https://review.coreboot.org/c/coreboot/+/77338/comment/04aae74e_074de614 :
PS15, Line 641: if (max_payload != child_max_payload)
: printk(BIOS_INFO, "%s: Max_Payload_Size adjusted to %d\n", dev_path(child),
: (1 << (max_payload + 7)));
> Removed.
Acknowledged
https://review.coreboot.org/c/coreboot/+/77338/comment/76bcb193_3e6c9a23 :
PS15, Line 691: };
> The whole function is not needed at all actually. […]
Acknowledged
https://review.coreboot.org/c/coreboot/+/77338/comment/aaf3f2af_868bcb91 :
PS15, Line 728: pciexp_dev_set_max_payload_size(dev, pciexp_dev_get_max_payload_size_cap(dev));
> Yes, limited it to express endpoints and legacy endpoints.
Acknowledged
https://review.coreboot.org/c/coreboot/+/77338/comment/b58846aa_4032625d :
PS15, Line 740: pciexp_dev_set_max_payload_size(bus->dev, max_payload);
> Right, not sure what I was thinking... Removed.
Acknowledged
File src/device/pciexp_device.c:
https://review.coreboot.org/c/coreboot/+/77338/comment/9af8cd9b_0cee147d :
PS17, Line 651: }
Nit, pciexp_dev_set_max_payload_size() might be a better place for this check.
https://review.coreboot.org/c/coreboot/+/77338/comment/3dd6cea0_dbe8cecb :
PS17, Line 771: Paylaod
Payl*oa*d
https://review.coreboot.org/c/coreboot/+/77338/comment/6d28084c_eb7e2825 :
PS17, Line 774: dvices
d*e*vices
https://review.coreboot.org/c/coreboot/+/77338/comment/d517d683_d28cb894 :
PS17, Line 785: }
Just a note: We could also have a special _scan_bus() function for root ports,
that would save us the `if`. I'm not sure how much work this would be and if
it would be really cleaner, so I'll leave it for me to look into later.
--
To view, visit https://review.coreboot.org/c/coreboot/+/77338?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I24386dc208363b7d94fea46dec25c231a3968225
Gerrit-Change-Number: 77338
Gerrit-PatchSet: 17
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Angel Pons <angel.pons(a)9elements.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Attention: Angel Pons <angel.pons(a)9elements.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Wed, 07 Feb 2024 21:33:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Comment-In-Reply-To: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Felix Held, Fred Reitberger, Jason Glenesk.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80407?usp=email )
Change subject: soc/amd/common/data_fabric/domain: don't report DRAM as MMIO producer
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/80407?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7b6d3fd8c7f89aa4364de7963d745aef8d6b6f42
Gerrit-Change-Number: 80407
Gerrit-PatchSet: 3
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Wed, 07 Feb 2024 21:29:19 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Fred Reitberger, Jason Glenesk, Matt DeVillier.
Hello Fred Reitberger, Jason Glenesk, Matt DeVillier, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80407?usp=email
to look at the new patch set (#3).
Change subject: soc/amd/common/data_fabric/domain: don't report DRAM as MMIO producer
......................................................................
soc/amd/common/data_fabric/domain: don't report DRAM as MMIO producer
In commit 30f36c35e75a ("soc/amd: rework DRAM and fixed resource
reporting") the reporting of the DRAM resources was moved from the
northbridge PCI device to the domain device. amd_pci_domain_fill_ssdt
didn't skip those DRAM resources when generation the resource producer
ranges which made Windows 10 very unhappy when it tried to evaluating
the ACPI tables causing it to reboot in a loop. To fix this, add a check
to also skip the resources that have the IORESOURCE_STORED flag set when
generating the resource producer ranges for the PCI root.
TEST=Windows 10 now successfully boots and reboots again on Mandolin
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: I7b6d3fd8c7f89aa4364de7963d745aef8d6b6f42
---
M src/soc/amd/common/block/data_fabric/domain.c
1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/80407/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/80407?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7b6d3fd8c7f89aa4364de7963d745aef8d6b6f42
Gerrit-Change-Number: 80407
Gerrit-PatchSet: 3
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
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
Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80409?usp=email )
Change subject: include/device/device: fix soft_reserved_ram_resource macro
......................................................................
include/device/device: fix soft_reserved_ram_resource macro
The soft_reserved_ram_resource expanded to the non-existent
fixed_mem_resource function. Using the existing fixed_mem_resource_kb
function seems to be the correct thing to do here and is also consistent
with the macros above and below.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: I6b454175c6530e539aa24dffb771368b0aea6da9
---
M src/include/device/device.h
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/80409/1
diff --git a/src/include/device/device.h b/src/include/device/device.h
index 27ef82a..e17bcef 100644
--- a/src/include/device/device.h
+++ b/src/include/device/device.h
@@ -384,7 +384,7 @@
| IORESOURCE_RESERVE)
#define soft_reserved_ram_resource(dev, idx, basek, sizek) \
- fixed_mem_resource(dev, idx, basek, sizek, IORESOURCE_SOFT_RESERVE)
+ fixed_mem_resource_kb(dev, idx, basek, sizek, IORESOURCE_SOFT_RESERVE)
#define bad_ram_resource_kb(dev, idx, basek, sizek) \
reserved_ram_resource_kb((dev), (idx), (basek), (sizek))
--
To view, visit https://review.coreboot.org/c/coreboot/+/80409?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I6b454175c6530e539aa24dffb771368b0aea6da9
Gerrit-Change-Number: 80409
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.
Hello Fred Reitberger, Jason Glenesk, Matt DeVillier, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80407?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Code-Review+2 by Matt DeVillier, Verified+1 by build bot (Jenkins)
The change is no longer submittable: Code-Review and Verified are unsatisfied now.
Change subject: soc/amd/common/data_fabric/domain: don't report DRAM as MMIO producer
......................................................................
soc/amd/common/data_fabric/domain: don't report DRAM as MMIO producer
In commit 30f36c35e75a ("soc/amd: rework DRAM and fixed resource
reporting") the reporting of the DRAM resources was moved from the
northbridge PCI device to the domain device. amd_pci_domain_fill_ssdt
didn't skip those DRAM resources when generation the resource producer
ranges which made Windows 10 very unhappy when it tried to evaluating
the ACPI tables causing it to reboot in a loop. To fix this, add a check
to also skip the resources that have the IORESOURCE_STORED flag set when
generating the resource producer ranges for the PCI root.
TEST=Windows 10 now successfully boots and reboots again on Mandolin
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: I7b6d3fd8c7f89aa4364de7963d745aef8d6b6f42
---
M src/soc/amd/common/block/data_fabric/domain.c
1 file changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/80407/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/80407?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7b6d3fd8c7f89aa4364de7963d745aef8d6b6f42
Gerrit-Change-Number: 80407
Gerrit-PatchSet: 2
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
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, Fred Reitberger, Jason Glenesk.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80407?usp=email )
Change subject: soc/amd/common/data_fabric/domain: don't report DRAM as MMIO producer
......................................................................
Patch Set 1:
(1 comment)
File src/soc/amd/common/block/data_fabric/domain.c:
https://review.coreboot.org/c/coreboot/+/80407/comment/cbf7945b_f153a1a9 :
PS1, Line 285: IORESOURCE_CACHEABLE
> would IORESOURCE_STORED be more appropriate?
that should cover both IORESOURCE_RESERVE and IORESOURCE_CACHEABLE cases, right?
--
To view, visit https://review.coreboot.org/c/coreboot/+/80407?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7b6d3fd8c7f89aa4364de7963d745aef8d6b6f42
Gerrit-Change-Number: 80407
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Comment-Date: Wed, 07 Feb 2024 20:47:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80408?usp=email )
Change subject: soc/amd/common/data_fabric/domain: drop unneeded parenthesis
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/80408?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I84a7b7b1b2c45b773c6f10b39e7813db3f96546e
Gerrit-Change-Number: 80408
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-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Wed, 07 Feb 2024 20:47:04 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80407?usp=email )
Change subject: soc/amd/common/data_fabric/domain: don't report DRAM as MMIO producer
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/80407?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7b6d3fd8c7f89aa4364de7963d745aef8d6b6f42
Gerrit-Change-Number: 80407
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Wed, 07 Feb 2024 20:46:41 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80408?usp=email )
Change subject: soc/amd/common/data_fabric/domain: drop unneeded parenthesis
......................................................................
soc/amd/common/data_fabric/domain: drop unneeded parenthesis
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: I84a7b7b1b2c45b773c6f10b39e7813db3f96546e
---
M src/soc/amd/common/block/data_fabric/domain.c
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/80408/1
diff --git a/src/soc/amd/common/block/data_fabric/domain.c b/src/soc/amd/common/block/data_fabric/domain.c
index 128d39b..600c500 100644
--- a/src/soc/amd/common/block/data_fabric/domain.c
+++ b/src/soc/amd/common/block/data_fabric/domain.c
@@ -279,7 +279,7 @@
continue;
/* Don't add MMIO producer ranges for reserved MMIO regions from non-PCI
devices */
- if ((res->flags & IORESOURCE_RESERVE))
+ if (res->flags & IORESOURCE_RESERVE)
continue;
/* Don't add MMIO producer ranges for DRAM regions */
if (res->flags & IORESOURCE_CACHEABLE)
--
To view, visit https://review.coreboot.org/c/coreboot/+/80408?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I84a7b7b1b2c45b773c6f10b39e7813db3f96546e
Gerrit-Change-Number: 80408
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newchange
Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80407?usp=email )
Change subject: soc/amd/common/data_fabric/domain: don't report DRAM as MMIO producer
......................................................................
Patch Set 1:
(1 comment)
File src/soc/amd/common/block/data_fabric/domain.c:
https://review.coreboot.org/c/coreboot/+/80407/comment/0d3401ec_b9d764b4 :
PS1, Line 285: IORESOURCE_CACHEABLE
would IORESOURCE_STORED be more appropriate?
--
To view, visit https://review.coreboot.org/c/coreboot/+/80407?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7b6d3fd8c7f89aa4364de7963d745aef8d6b6f42
Gerrit-Change-Number: 80407
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-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Wed, 07 Feb 2024 20:41:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment