Change in coreboot[master]: Revert "soc/intel/xeon_sp: Improve performance efficiencies"
Hello build bot (Jenkins), Marc Jones, Patrick Georgi, Jonathan Zhang, John Zhao, Tim Wawrzynczak, Angel Pons, Patrick Rudolph,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/44998
to review the following change.
Change subject: Revert "soc/intel/xeon_sp: Improve performance efficiencies" ......................................................................
Revert "soc/intel/xeon_sp: Improve performance efficiencies"
This reverts commit d51449d017410fedb55e93f71fb322749ba888b5.
Reason for revert: https://qa.coreboot.org/job/coreboot/16544/
Change-Id: I7050060f1db7b9a9b5a77b5a6245c8fda05623a4 --- M src/soc/intel/xeon_sp/cpx/acpi.c 1 file changed, 13 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/44998/1
diff --git a/src/soc/intel/xeon_sp/cpx/acpi.c b/src/soc/intel/xeon_sp/cpx/acpi.c index 1b6f1a3..cd497c5 100644 --- a/src/soc/intel/xeon_sp/cpx/acpi.c +++ b/src/soc/intel/xeon_sp/cpx/acpi.c @@ -605,16 +605,16 @@ * in the context of ATSR subtable, it adds ATSR subtable when it is first called. */ static unsigned long acpi_create_dmar_ds_pci_br_for_port(unsigned long current, - int port, int stack, const IIO_RESOURCE_INSTANCE *iio_resource, uint32_t pcie_seg, + int port, int stack, IIO_RESOURCE_INSTANCE iio_resource, uint32_t pcie_seg, bool is_atsr, bool *first) {
if (get_stack_for_port(port) != stack) return 0;
- const uint32_t bus = iio_resource->StackRes[stack].BusBase; - const uint32_t dev = iio_resource->PcieInfo.PortInfo[port].Device; - const uint32_t func = iio_resource->PcieInfo.PortInfo[port].Function; + const uint32_t bus = iio_resource.StackRes[stack].BusBase; + const uint32_t dev = iio_resource.PcieInfo.PortInfo[port].Device; + const uint32_t func = iio_resource.PcieInfo.PortInfo[port].Function;
const uint32_t id = pci_mmio_read_config32(PCI_DEV(bus, dev, func), PCI_VENDOR_ID); @@ -703,8 +703,8 @@
// Add PCIe Ports if (socket != 0 || stack != CSTACK) { - const IIO_RESOURCE_INSTANCE *iio_resource = - &hob->PlatformData.IIO_resource[socket]; + IIO_RESOURCE_INSTANCE iio_resource = + hob->PlatformData.IIO_resource[socket]; for (int p = PORT_0; p < MAX_PORTS; ++p) current += acpi_create_dmar_ds_pci_br_for_port(current, p, stack, iio_resource, pcie_seg, false, NULL); @@ -748,12 +748,12 @@ uint32_t pcie_seg = hob->PlatformData.CpuQpiInfo[socket].PcieSegment; unsigned long tmp = current; bool first = true; - const IIO_RESOURCE_INSTANCE *iio_resource = - &hob->PlatformData.IIO_resource[socket]; + IIO_RESOURCE_INSTANCE iio_resource = + hob->PlatformData.IIO_resource[socket];
for (int stack = 0; stack <= PSTACK2; ++stack) { - uint32_t bus = iio_resource->StackRes[stack].BusBase; - uint32_t vtd_base = iio_resource->StackRes[stack].VtdBarAddress; + uint32_t bus = iio_resource.StackRes[stack].BusBase; + uint32_t vtd_base = iio_resource.StackRes[stack].VtdBarAddress; if (!vtd_base) continue; uint64_t vtd_mmio_cap = read64((void *)(vtd_base + VTD_EXT_CAP_LOW)); @@ -821,10 +821,10 @@ assert(hob != NULL && hob_size != 0);
for (int socket = 0; socket < hob->PlatformData.numofIIO; ++socket) { - IIO_RESOURCE_INSTANCE *iio_resource = - &hob->PlatformData.IIO_resource[socket]; + IIO_RESOURCE_INSTANCE iio_resource = + hob->PlatformData.IIO_resource[socket]; for (int stack = 0; stack <= PSTACK2; ++stack) { - uint32_t vtd_base = iio_resource->StackRes[stack].VtdBarAddress; + uint32_t vtd_base = iio_resource.StackRes[stack].VtdBarAddress; if (!vtd_base) continue;
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44998 )
Change subject: Revert "soc/intel/xeon_sp: Improve performance efficiencies" ......................................................................
Patch Set 1:
ToT is broken
src/soc/intel/xeon_sp/cpx/acpi.c: In function 'acpi_create_rhsa': src/soc/intel/xeon_sp/cpx/acpi.c:825:4: error: initialization discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers] &hob->PlatformData.IIO_resource[socket]; ^
Hello build bot (Jenkins), Marc Jones, Patrick Georgi, Jonathan Zhang, John Zhao, Tim Wawrzynczak, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44998
to look at the new patch set (#2).
Change subject: Revert "soc/intel/xeon_sp: Improve performance efficiencies" ......................................................................
Revert "soc/intel/xeon_sp: Improve performance efficiencies"
This reverts commit d51449d017410fedb55e93f71fb322749ba888b5.
Reason for revert: https://qa.coreboot.org/job/coreboot/16544/
Change-Id: I7050060f1db7b9a9b5a77b5a6245c8fda05623a4 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/xeon_sp/cpx/acpi.c 1 file changed, 13 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/44998/2
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44998 )
Change subject: Revert "soc/intel/xeon_sp: Improve performance efficiencies" ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44998/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44998/2//COMMIT_MSG@11 PS2, Line 11: Reason for revert: https://qa.coreboot.org/job/coreboot/16544/ Retention of these logs isn't very long, so this commit message won't be informative in a week or two. Please elaborate here.
Hello build bot (Jenkins), Marc Jones, Patrick Georgi, Jonathan Zhang, John Zhao, Tim Wawrzynczak, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44998
to look at the new patch set (#3).
Change subject: Revert "soc/intel/xeon_sp: Improve performance efficiencies" ......................................................................
Revert "soc/intel/xeon_sp: Improve performance efficiencies"
This reverts commit d51449d017410fedb55e93f71fb322749ba888b5.
Reason for revert: Causing compilation issue as below
src/soc/intel/xeon_sp/cpx/acpi.c: In function 'acpi_create_rhsa': src/soc/intel/xeon_sp/cpx/acpi.c:825:4: error: initialization discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers] &hob->PlatformData.IIO_resource[socket]; ^ Change-Id: I7050060f1db7b9a9b5a77b5a6245c8fda05623a4 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/xeon_sp/cpx/acpi.c 1 file changed, 13 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/44998/3
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44998 )
Change subject: Revert "soc/intel/xeon_sp: Improve performance efficiencies" ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44998/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44998/2//COMMIT_MSG@11 PS2, Line 11: Reason for revert: https://qa.coreboot.org/job/coreboot/16544/
Retention of these logs isn't very long, so this commit message won't be informative in a week or tw […]
Ack
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44998 )
Change subject: Revert "soc/intel/xeon_sp: Improve performance efficiencies" ......................................................................
Patch Set 3: Code-Review+2
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44998 )
Change subject: Revert "soc/intel/xeon_sp: Improve performance efficiencies" ......................................................................
Patch Set 3: Code-Review+2
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44998 )
Change subject: Revert "soc/intel/xeon_sp: Improve performance efficiencies" ......................................................................
Patch Set 3:
Patrick, can we merge this ? i need to rebase all patches 😊
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44998 )
Change subject: Revert "soc/intel/xeon_sp: Improve performance efficiencies" ......................................................................
Patch Set 3: Code-Review+2
Let's go with this for now.
Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44998 )
Change subject: Revert "soc/intel/xeon_sp: Improve performance efficiencies" ......................................................................
Revert "soc/intel/xeon_sp: Improve performance efficiencies"
This reverts commit d51449d017410fedb55e93f71fb322749ba888b5.
Reason for revert: Causing compilation issue as below
src/soc/intel/xeon_sp/cpx/acpi.c: In function 'acpi_create_rhsa': src/soc/intel/xeon_sp/cpx/acpi.c:825:4: error: initialization discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers] &hob->PlatformData.IIO_resource[socket]; ^ Change-Id: I7050060f1db7b9a9b5a77b5a6245c8fda05623a4 Signed-off-by: Subrata Banik subrata.banik@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/44998 Reviewed-by: Maxim Polyakov max.senia.poliak@gmail.com Reviewed-by: Patrick Georgi pgeorgi@google.com Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/xeon_sp/cpx/acpi.c 1 file changed, 13 insertions(+), 13 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved Angel Pons: Looks good to me, approved Maxim Polyakov: Looks good to me, approved
diff --git a/src/soc/intel/xeon_sp/cpx/acpi.c b/src/soc/intel/xeon_sp/cpx/acpi.c index 1b6f1a3..cd497c5 100644 --- a/src/soc/intel/xeon_sp/cpx/acpi.c +++ b/src/soc/intel/xeon_sp/cpx/acpi.c @@ -605,16 +605,16 @@ * in the context of ATSR subtable, it adds ATSR subtable when it is first called. */ static unsigned long acpi_create_dmar_ds_pci_br_for_port(unsigned long current, - int port, int stack, const IIO_RESOURCE_INSTANCE *iio_resource, uint32_t pcie_seg, + int port, int stack, IIO_RESOURCE_INSTANCE iio_resource, uint32_t pcie_seg, bool is_atsr, bool *first) {
if (get_stack_for_port(port) != stack) return 0;
- const uint32_t bus = iio_resource->StackRes[stack].BusBase; - const uint32_t dev = iio_resource->PcieInfo.PortInfo[port].Device; - const uint32_t func = iio_resource->PcieInfo.PortInfo[port].Function; + const uint32_t bus = iio_resource.StackRes[stack].BusBase; + const uint32_t dev = iio_resource.PcieInfo.PortInfo[port].Device; + const uint32_t func = iio_resource.PcieInfo.PortInfo[port].Function;
const uint32_t id = pci_mmio_read_config32(PCI_DEV(bus, dev, func), PCI_VENDOR_ID); @@ -703,8 +703,8 @@
// Add PCIe Ports if (socket != 0 || stack != CSTACK) { - const IIO_RESOURCE_INSTANCE *iio_resource = - &hob->PlatformData.IIO_resource[socket]; + IIO_RESOURCE_INSTANCE iio_resource = + hob->PlatformData.IIO_resource[socket]; for (int p = PORT_0; p < MAX_PORTS; ++p) current += acpi_create_dmar_ds_pci_br_for_port(current, p, stack, iio_resource, pcie_seg, false, NULL); @@ -748,12 +748,12 @@ uint32_t pcie_seg = hob->PlatformData.CpuQpiInfo[socket].PcieSegment; unsigned long tmp = current; bool first = true; - const IIO_RESOURCE_INSTANCE *iio_resource = - &hob->PlatformData.IIO_resource[socket]; + IIO_RESOURCE_INSTANCE iio_resource = + hob->PlatformData.IIO_resource[socket];
for (int stack = 0; stack <= PSTACK2; ++stack) { - uint32_t bus = iio_resource->StackRes[stack].BusBase; - uint32_t vtd_base = iio_resource->StackRes[stack].VtdBarAddress; + uint32_t bus = iio_resource.StackRes[stack].BusBase; + uint32_t vtd_base = iio_resource.StackRes[stack].VtdBarAddress; if (!vtd_base) continue; uint64_t vtd_mmio_cap = read64((void *)(vtd_base + VTD_EXT_CAP_LOW)); @@ -821,10 +821,10 @@ assert(hob != NULL && hob_size != 0);
for (int socket = 0; socket < hob->PlatformData.numofIIO; ++socket) { - IIO_RESOURCE_INSTANCE *iio_resource = - &hob->PlatformData.IIO_resource[socket]; + IIO_RESOURCE_INSTANCE iio_resource = + hob->PlatformData.IIO_resource[socket]; for (int stack = 0; stack <= PSTACK2; ++stack) { - uint32_t vtd_base = iio_resource->StackRes[stack].VtdBarAddress; + uint32_t vtd_base = iio_resource.StackRes[stack].VtdBarAddress; if (!vtd_base) continue;
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44998 )
Change subject: Revert "soc/intel/xeon_sp: Improve performance efficiencies" ......................................................................
Patch Set 4:
Patch Set 3: Code-Review+2
Let's go with this for now.
Thanks Angel.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44998 )
Change subject: Revert "soc/intel/xeon_sp: Improve performance efficiencies" ......................................................................
Patch Set 4:
is xeon_sp not build-tested?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44998 )
Change subject: Revert "soc/intel/xeon_sp: Improve performance efficiencies" ......................................................................
Patch Set 4:
Patch Set 4:
is xeon_sp not build-tested?
Its there Tim, i think those build was failing but not sure how it got +1? 😮
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44998 )
Change subject: Revert "soc/intel/xeon_sp: Improve performance efficiencies" ......................................................................
Patch Set 4:
Patch Set 4:
Patch Set 4:
is xeon_sp not build-tested?
Its there Tim, i think those build was failing but not sure how it got +1? 😮
There was a problem with Jenkins and most boards didn't get build-tested: https://qa.coreboot.org/job/coreboot-gerrit/142527/testReport/(root)/board/
Picture of this page, since the test report will eventually disappear: https://imgur.com/839m3TJ.png
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44998 )
Change subject: Revert "soc/intel/xeon_sp: Improve performance efficiencies" ......................................................................
Patch Set 4:
Patch Set 4:
Patch Set 4:
Patch Set 4:
is xeon_sp not build-tested?
Its there Tim, i think those build was failing but not sure how it got +1? 😮
There was a problem with Jenkins and most boards didn't get build-tested: https://qa.coreboot.org/job/coreboot-gerrit/142527/testReport/(root)/board/
Picture of this page, since the test report will eventually disappear: https://imgur.com/839m3TJ.png
Wow, good catch there
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44998 )
Change subject: Revert "soc/intel/xeon_sp: Improve performance efficiencies" ......................................................................
Patch Set 4:
There was a problem with Jenkins and most boards didn't get build-tested: https://qa.coreboot.org/job/coreboot-gerrit/142527/testReport/(root)/board/
Picture of this page, since the test report will eventually disappear: https://imgur.com/839m3TJ.png
They were built, but the results were overwritten before they got reported back to jenkins. The problem was because another build step was inserted that only built that one qemu board. It's now fixed (so this isn't a flake in one direction or the other)
-
Angel Pons (Code Review) -
Maxim Polyakov (Code Review)
-
Patrick Georgi (Code Review)
-
Subrata Banik (Code Review)
-
Tim Wawrzynczak (Code Review)