On 12/01/2016 02:20 PM, Mark Salter wrote: > On Thu, 2016-12-01 at 12:33 -0600, Bjorn Helgaas wrote: >>> + csr = &xgene_v1_csr_res[root->segment]; >> This makes me nervous because root->segment comes from the ACPI _SEG, >> and if firmware gives us junk in _SEG, we will reference something in >> the weeds. > > The SoC provide some number of RC bridges, each with a different base > for some mmio registers. Even if segment is legitimate in MCFG, there > is still a problem if a platform doesn't use the segment ordering > implied by the code. But the PNP0A03 _CRS does have this base address > as the first memory resource, so we could get it from there and not > have hard-coded addresses and implied ording in the quirk code. > > I have tested a modified version of these quirks using this to > get the CSR base and it works on the 3 different platforms I have > access to. > > static int xgene_pcie_get_csr(struct device *dev, struct resource *r) > { > struct acpi_device *adev = to_acpi_device(dev); > unsigned long flags; > struct list_head list; > struct resource_entry *entry; > int ret; > > INIT_LIST_HEAD(&list); > flags = IORESOURCE_MEM; > ret = acpi_dev_get_resources(adev, &list, > acpi_dev_filter_resource_type_cb, > (void *)flags); > if (ret < 0) { > dev_err(dev, "failed to parse _CRS, error: %d\n", ret); > return ret; > } else if (ret == 0) { > dev_err(dev, "no memory resources present in _CRS\n"); > return -EINVAL; > } > > entry = list_first_entry(&list, struct resource_entry, node); > *r = *entry->res; > acpi_dev_free_resource_list(&list); > return 0; > } This seems a lot safer. At some point trusting firmware to provide the correct _CRS for the RC in use is better than hard coding for every possible implementation configuration of an X-Gene SoC. Jon. -- Computer Architect | Sent from my Fedora powered laptop -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html