[v7,08/10] PCI, x86: add MMCFG information on demand

This patch changes mmconfig logic on x86 platforms to add MMCFG
information on demand instead of adding all MMCFG entries from
the ACPI MCFG table at boot time. So only MMCFG address ranges
used by active PCI host bridges will be actually mapped.

Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
 arch/x86/include/asm/pci_x86.h |    5 +++
 arch/x86/pci/legacy.c          |    1 +
 arch/x86/pci/mmconfig-shared.c |   54 +++++++++++++++++++++++++++++++++++++---
 3 files changed, 56 insertions(+), 4 deletions(-)

On Sat, May 26, 2012 at 2:54 AM, Jiang Liu <jiang.liu@huawei.com> wrote:
> This patch changes mmconfig logic on x86 platforms to add MMCFG
> information on demand instead of adding all MMCFG entries from
> the ACPI MCFG table at boot time. So only MMCFG address ranges
> used by active PCI host bridges will be actually mapped.
>
> Signed-off-by: Jiang Liu <liuj97@gmail.com>
> ---
>  arch/x86/include/asm/pci_x86.h |    5 +++
>  arch/x86/pci/legacy.c          |    1 +
>  arch/x86/pci/mmconfig-shared.c |   54 +++++++++++++++++++++++++++++++++++++---
>  3 files changed, 56 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/include/asm/pci_x86.h b/arch/x86/include/asm/pci_x86.h
> index 3e5f43c..4a1a9aa 100644
> --- a/arch/x86/include/asm/pci_x86.h
> +++ b/arch/x86/include/asm/pci_x86.h
> @@ -142,6 +142,11 @@ extern int __devinit pci_mmconfig_insert(struct device *dev,
>                                         uint16_t seg, uint8_t start,
>                                         uint8_t end, phys_addr_t addr);
>  extern int pci_mmconfig_delete(uint16_t seg, uint8_t start, uint8_t end);
> +#ifdef CONFIG_ACPI
> +extern void pci_mmconfig_probe(uint8_t start);
> +#else
> +static inline void pci_mmconfig_probe(uint8_t start) { }
> +#endif
>  extern struct pci_mmcfg_region *pci_mmconfig_lookup(int segment, int bus);
>
>  extern struct list_head pci_mmcfg_list;
> diff --git a/arch/x86/pci/legacy.c b/arch/x86/pci/legacy.c
> index a1df191..e9a2384 100644
> --- a/arch/x86/pci/legacy.c
> +++ b/arch/x86/pci/legacy.c
> @@ -49,6 +49,7 @@ void __devinit pcibios_scan_specific_bus(int busn)
>                    l != 0x0000 && l != 0xffff) {
>                        DBG("Found device at %02x:%02x [%04x]\n", busn, devfn, l);
>                        printk(KERN_INFO "PCI: Discovered peer bus %02x\n", busn);
> +                       pci_mmconfig_probe(busn);
>                        pci_scan_bus_on_node(busn, &pci_root_ops, node);
>                        return;
>                }
> diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c
> index fa0aa90..e80b5c2 100644
> --- a/arch/x86/pci/mmconfig-shared.c
> +++ b/arch/x86/pci/mmconfig-shared.c
> @@ -19,6 +19,7 @@
>  #include <linux/slab.h>
>  #include <linux/mutex.h>
>  #include <linux/rculist.h>
> +#include <linux/pci-acpi.h>
>  #include <asm/e820.h>
>  #include <asm/pci_x86.h>
>  #include <asm/acpi.h>
> @@ -616,6 +617,16 @@ static int __init pci_parse_mcfg(struct acpi_table_header *header)
>                        return -ENODEV;
>                }
>
> +               /*
> +                * MMCFG information for host brideges will be added on demand
> +                * by pci_root driver if ACPI is enabled. But there are special
> +                * requirements for devices on segment 0, MMCFG information may
> +                * be needed for fixing hardware quirks and probing for hidden
> +                * buses.
> +                */
> +               if (!acpi_disabled && cfg->pci_segment)
> +                       continue;
> +
>                if (pci_mmconfig_add(cfg->pci_segment, cfg->start_bus_number,
>                                   cfg->end_bus_number, cfg->address) == NULL) {
>                        printk(KERN_WARNING PREFIX
> @@ -625,6 +636,13 @@ static int __init pci_parse_mcfg(struct acpi_table_header *header)
>                }
>        }
>
> +       i = entries * sizeof(*cfg_table);
> +       pci_acpi_mcfg_array = kmalloc(i, GFP_KERNEL);
> +       if (pci_acpi_mcfg_array) {
> +               memcpy(pci_acpi_mcfg_array, cfg_table, i);
> +               pci_acpi_mcfg_entries = entries;
> +       }
> +

here cache cfg too early.  should do that after

pci_mmcfg_reject_broken().

otherwise will use mcfg even try to reject that before.

>        return 0;
>  }
>
> @@ -634,14 +652,14 @@ static void __init __pci_mmcfg_init(int early)
>        if ((pci_probe & PCI_PROBE_MMCONF) == 0)
>                return;
>
> -       /* MMCONFIG already enabled */
> -       if (!early && !(pci_probe & PCI_PROBE_MASK & ~PCI_PROBE_MMCONF))
> -               return;
> -
>        /* for late to exit */
>        if (known_bridge)
>                return;
>
> +       /* MMCONFIG already enabled */
> +       if (!early && !(pci_probe & PCI_PROBE_MASK & ~PCI_PROBE_MMCONF))
> +               goto out;
> +
>        if (early) {
>                if (pci_mmcfg_check_hostbridge())
>                        known_bridge = 1;
> @@ -675,6 +693,14 @@ static void __init __pci_mmcfg_init(int early)
>                pci_mmcfg_resources_inserted = 1;
>                pci_mmcfg_arch_init_failed = true;
>        }
> +
> +out:
> +       /*
> +        * Free all MCFG entries if ACPI is enabled. MCFG information will
> +        * be added back on demand by the pci_root driver later.
> +        */
> +       if (!early && !acpi_disabled && !known_bridge && pci_acpi_mcfg_array)
> +               free_all_mmcfg();

that really change the logic.

looks like it will break mrst/sfi path.

the scan from pci_legacy_init() for mrst/sfi will not have ext_pci_ops
set for bus 0.

| int __init pci_subsys_init(void)
| {
|        /*
|         * The init function returns an non zero value when
|         * pci_legacy_init should be invoked.
|         */
|        if (x86_init.pci.init())
|                pci_legacy_init();
|
|        pcibios_fixup_peer_bridges();


Yinghai

>  }
>
>  void __init pci_mmcfg_early_init(void)
> @@ -809,3 +835,23 @@ int pci_mmconfig_delete(uint16_t seg, uint8_t start, uint8_t end)
>
>        return -ENOENT;
>  }
> +
> +/* Probe MMCFG information for PCI bus blind probe */
> +void __devinit pci_mmconfig_probe(uint8_t start)
> +{
> +       int end_bus, temp;
> +       phys_addr_t addr;
> +
> +       if (!(pci_probe & PCI_PROBE_MMCONF) || pci_mmcfg_arch_init_failed)
> +               return;
> +
> +       addr = acpi_pci_root_get_mcfg_addr(NULL, 0, start, &end_bus);
> +       if (addr && end_bus >= 0 && end_bus <= 255) {
> +               for (temp = start + 1; temp <= end_bus; temp++)
> +                       if (pci_find_bus(0, temp))
> +                               break;
> +
> +               temp--;
> +               pci_mmconfig_insert(NULL, 0, start, (uint8_t)temp, addr);
> +       }
> +}
> --
> 1.7.1
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

I wrote:

> But the code in the init path is ridiculously, embarrassingly complicated ...

On Fri, Jun 15, 2012 at 1:15 AM, Yinghai Lu <yinghai@kernel.org> wrote:

> here cache cfg too early.  should do that after
>
> pci_mmcfg_reject_broken().
>
> otherwise will use mcfg even try to reject that before.
...
> that really change the logic.
>
> looks like it will break mrst/sfi path.
>
> the scan from pci_legacy_init() for mrst/sfi will not have ext_pci_ops
> set for bus 0.

I rest my case.  The current init path is unmaintainable.

I'll wait for a v8 (or later) with fixes for the build issues
Fengguang found and Yinghai's ack.  It will make things somewhat
easier for me if you start with my topic/jiang-mmconfig-v7 branch.

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 06/15/2012 11:46 PM, Bjorn Helgaas wrote:
> I wrote:
> 
>> But the code in the init path is ridiculously, embarrassingly complicated ...
> 
> On Fri, Jun 15, 2012 at 1:15 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> 
>> here cache cfg too early.  should do that after
>>
>> pci_mmcfg_reject_broken().
>>
>> otherwise will use mcfg even try to reject that before.
> ...
This is an issue and I have worked out a patch for this, actually the patch
produces better readable code.

>> that really change the logic.
>>
>> looks like it will break mrst/sfi path.
>>
>> the scan from pci_legacy_init() for mrst/sfi will not have ext_pci_ops
>> set for bus 0.
> 
> I rest my case.  The current init path is unmaintainable.
I think it doesn't change the logic here. I have paid special attention
to the SFI staff when preparing the patch. On systems with SFI instead of
ACPI, such as Moorsetown, there's no ACPI root pointer table available,
so acpi_initialize() will fail and set acpi_disabled to true. We have
checked "!acpi_disabled" before calling free_all_mmcfg(), that means
we won't call free_all_mmcfg() on SFI platforms. 

> 
> I'll wait for a v8 (or later) with fixes for the build issues
> Fengguang found and Yinghai's ack.  It will make things somewhat
> easier for me if you start with my topic/jiang-mmconfig-v7 branch.
Sure, will rebase my v8 patches onto this branch tomorrow.

> 
> Bjorn

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Fri, Jun 15, 2012 at 8:46 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>
>> But the code in the init path is ridiculously, embarrassingly complicated ...
>
> I rest my case.  The current init path is unmaintainable.
>
> I'll wait for a v8 (or later) with fixes for the build issues
> Fengguang found and Yinghai's ack.  It will make things somewhat
> easier for me if you start with my topic/jiang-mmconfig-v7 branch.

yes. probe/free then install is not good.

should make __pci_mmcfg_init() to cache needed stuff for all path, and
then consume stored
entry just before scan root bus.

Thanks

Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html