Hi Kenji,
Thanks for your careful review and comments.
On 04/09/2012 07:43 PM, Kenji Kaneshige wrote:
> Your patch looks good to me.
>
> I have some comments.
>
> (2012/04/09 2:12), Jiang Liu wrote:
>> This patch enhances pci_root driver to update MMCFG information when
>> hot-plugging PCI root bridges on x86 platforms.
>>
>
> Do you have the patch that can be applied to Bjorn's pci tree?
>
> <snip.>
Will try to generate a version against Bjorn's version. Could you please tell
me the exact git link for that? I haven't pull from Bjorn's tree yet.
>
>> +int arch_acpi_pci_root_add(struct acpi_pci_root *root)
>> +{
>> + int result = 0;
>> + acpi_status status;
>> + unsigned long long base_addr;
>> + struct pci_mmcfg_region *cfg;
>> +
>> + /*
>> + * Try to insert MMCFG information for host bridges with _CBA method
>> + */
>> + status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA,
>> + NULL,&base_addr);
>> + if (ACPI_SUCCESS(status)) {
>> + result = pci_mmconfig_insert(root->segment,
>> + root->secondary.start,
>> + root->secondary.end,
>> + base_addr);
>> + /*
>> + * MMCFG information for hot-pluggable host bridges may have
>> + * already been added by __pci_mmcfg_init();
>> + */
>> + if (result == -EEXIST)
>> + result = 0;
>
> Just for confirmation.
> From my interpretation of PCI firmware spec, MCFG doesn't have any entry
> for hot-pluggable hostbridge. So I assume this is for the machine that
> is not compliant to the spec. Is my understanding same as yours?
>
> <snip.>
You are right, it's defined to that way in PCI FW Spec 3.1.
Here I have some concerns about the PCI buses to host all Ubox components
on Intel NHM/WSM/SNB/IVB processors. BIOS people are prone to declare
MMCFG information for those host bridges by MCFG table instead of _CBA method,
though those host bridge will disappear after hot-removing a physical processor.
>
>> static int __devinit acpi_pci_root_add(struct acpi_device *device)
>> {
>> unsigned long long segment, bus;
>> @@ -504,6 +514,14 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
>> strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS);
>> device->driver_data = root;
>>
>> + if (arch_acpi_pci_root_add(root)) {
>> + printk(KERN_ERR PREFIX
>> + "can't add MMCFG information for Bus %04x:%02x\n",
>> + root->segment, (unsigned int)root->secondary.start);
>> + result = -ENODEV;
>> + goto out_free;
>> + }
>
> Desn't this break the system that doesn't support MMCONFIG?
>
> In my understanding, arch_acpi_pci_root_add() returns -ENODEV if
> mmconfig information is found neither in MCFG table nor _CBA. And
> pci root bridge initialization seems to fail arch_acpi_pci_root_add()
> returns non-zero value.
Good catch, will add following code into arch_acpi_pci_root_add() and
arch_acpi_pci_root_remove() to solve this issue.
---
/* MMCONFIG disabled */
if ((pci_probe & PCI_PROBE_MMCONF) == 0)
return 0;
---
>
> Regards,
> Kenji Kaneshige
--
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
