[coreboot-gerrit] Change in coreboot[master]: device/pci: Don't enabled unassigned resources

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41553 )

Change subject: device/pci: Don't enabled unassigned resources
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/41553/1/src/device/pci_device.c 
File src/device/pci_device.c:

https://review.coreboot.org/c/coreboot/+/41553/1/src/device/pci_device.c@575 
PS1, Line 575: for (res = dev->resource_list; res; res = res->next)
             : 		pci_set_resource(dev, res, &command_mask);
             : 	/* If there are unassigned resources, we might
             : 	   have to disable others of the same type. */
             : 	dev->command &= command_mask;
             : 
I am thinking if we should just split bridge and non-bridge resource setting completely. That will also allow us to keep the entire logic in one place for each type:

void pci_dev_set_bridge_resources(void)
{
    for (res = dev->resource_list; res; res = res->next) {
          if (!(res->flags & IORESOURCE_PCI_BRIDGE))
              continue;

if (!resource_needs_storing(res))
              continue;

if (!(res->flags & IORESOURCE_ASSIGNED))
              res->size = 0;

if (res->size == 0) {
               base = res->limit;
               end = res->limit - (1 << res->gran);
               res->base = base;
          } else {
               base = res->base; 
               end = resource_end(res);
          }

dev->command |= PCI_COMMAND_MASTER;
          pci_dev_write_bridge_resource(dev, res->index, base, limit);
    }
}

void pci_dev_set_nonbridge_resources_by_type(unsigned long type_match, uint32_t command_mask, uint32_t bar_mask)
{
    for (res = dev->resource_list; res; res = res->next) {
          if ((res->flags & type_match) != type_match)
              continue;

if (!resource_needs_storing(res))
               continue;

if (!(res->flags & IORESOURCE_ASSIGNED) || !res->size) {
                printk(BIOS_ERR, "ERROR: %s %02lx %s size: 0x%010llx not "
    		       "assigned\n", dev_path(dev), res->index,
    		       resource_type(res), res->size);
                dev->command &= ~command_mask;
                return;
           }
           dev->command |= command_mask;
           pci_dev_write_nonbridge_resource(dev, res, bar_mask);
    }
}

static void pci_dev_set_nonbridge_resources(void)
{
    pci_dev_set_nonbridge_resources_by_type(IORESOURCE_IO, PCI_COMMAND_IO, PCI_BASE_ADDRESS_SPACE_IO);
    pci_dev_set_nonbridge_resources_by_type(IORESOURCE_MEM, PCI_COMMAND_MEM, 0);
}

pci_dev_set_resources(struct device *dev)
{
    pci_dev_set_bridge_resources();
    pci_dev_set_nonbridge_resources();

...
}