Jacob Garber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46729 )
Change subject: soc/intel/xeon_sp: Pass IIO_RESOURCE_INSTANCE as pointer ......................................................................
soc/intel/xeon_sp: Pass IIO_RESOURCE_INSTANCE as pointer
IIO_RESOURCE_INSTANCE is a large struct, so it should be passed as a constant pointer rather than making a copy.
Found-by: Coverity CID 1432759 Change-Id: Iebbb4d292f4d956e767bda28cbf20b0318586510 --- M src/soc/intel/xeon_sp/nb_acpi.c 1 file changed, 6 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/46729/1
diff --git a/src/soc/intel/xeon_sp/nb_acpi.c b/src/soc/intel/xeon_sp/nb_acpi.c index 1329feb..8bd8c41 100644 --- a/src/soc/intel/xeon_sp/nb_acpi.c +++ b/src/soc/intel/xeon_sp/nb_acpi.c @@ -195,16 +195,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, IIO_RESOURCE_INSTANCE iio_resource, uint32_t pcie_seg, + int port, int stack, const 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); @@ -301,7 +301,7 @@ 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); + &iio_resource, pcie_seg, false, NULL);
// Add VMD if (hob->PlatformData.VMDStackEnable[socket][stack] && @@ -365,7 +365,7 @@ if (socket == 0 && p == PORT_0) continue; current += acpi_create_dmar_ds_pci_br_for_port(current, p, - stack, iio_resource, pcie_seg, true, &first); + stack, &iio_resource, pcie_seg, true, &first); } } if (tmp != current)
Hello Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46729
to look at the new patch set (#2).
Change subject: soc/intel/xeon_sp: Pass IIO_RESOURCE_INSTANCE as pointer ......................................................................
soc/intel/xeon_sp: Pass IIO_RESOURCE_INSTANCE as pointer
IIO_RESOURCE_INSTANCE is a large struct, so it should be passed as a constant pointer rather than making a copy.
Found-by: Coverity CID 1432759 Change-Id: Iebbb4d292f4d956e767bda28cbf20b0318586510 Signed-off-by: Jacob Garber jgarber1@ualberta.ca --- M src/soc/intel/xeon_sp/nb_acpi.c 1 file changed, 6 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/46729/2
Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46729 )
Change subject: soc/intel/xeon_sp: Pass IIO_RESOURCE_INSTANCE as pointer ......................................................................
Patch Set 2: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46729 )
Change subject: soc/intel/xeon_sp: Pass IIO_RESOURCE_INSTANCE as pointer ......................................................................
soc/intel/xeon_sp: Pass IIO_RESOURCE_INSTANCE as pointer
IIO_RESOURCE_INSTANCE is a large struct, so it should be passed as a constant pointer rather than making a copy.
Found-by: Coverity CID 1432759 Change-Id: Iebbb4d292f4d956e767bda28cbf20b0318586510 Signed-off-by: Jacob Garber jgarber1@ualberta.ca Reviewed-on: https://review.coreboot.org/c/coreboot/+/46729 Reviewed-by: Stefan Reinauer stefan.reinauer@coreboot.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/xeon_sp/nb_acpi.c 1 file changed, 6 insertions(+), 6 deletions(-)
Approvals: build bot (Jenkins): Verified Stefan Reinauer: Looks good to me, approved
diff --git a/src/soc/intel/xeon_sp/nb_acpi.c b/src/soc/intel/xeon_sp/nb_acpi.c index f6bf6ed..cf397f7 100644 --- a/src/soc/intel/xeon_sp/nb_acpi.c +++ b/src/soc/intel/xeon_sp/nb_acpi.c @@ -147,16 +147,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, IIO_RESOURCE_INSTANCE iio_resource, uint32_t pcie_seg, + int port, int stack, const IIO_RESOURCE_INSTANCE *iio_resource, uint32_t pcie_seg, bool is_atsr, bool *first) {
if (soc_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); @@ -253,7 +253,7 @@ 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); + &iio_resource, pcie_seg, false, NULL);
// Add VMD if (hob->PlatformData.VMDStackEnable[socket][stack] && @@ -317,7 +317,7 @@ if (socket == 0 && p == PORT_0) continue; current += acpi_create_dmar_ds_pci_br_for_port(current, p, - stack, iio_resource, pcie_seg, true, &first); + stack, &iio_resource, pcie_seg, true, &first); } } if (tmp != current)
-
Jacob Garber (Code Review) -
Patrick Georgi (Code Review)
-
Stefan Reinauer (Code Review)