Re: [git pull] "big box" x86 changes, PCI
From: Jesse Barnes
Date: Mon Apr 28 2008 - 16:35:13 EST
On Saturday, April 26, 2008 2:55 pm Ingo Molnar wrote:
> @@ -184,51 +322,80 @@ static void __init pci_mmcfg_reject_broken(int type)
>
> cfg = &pci_mmcfg_config[0];
>
> - /*
> - * Handle more broken MCFG tables on Asus etc.
> - * They only contain a single entry for bus 0-0.
> - */
> - if (pci_mmcfg_config_num == 1 &&
> - cfg->pci_segment == 0 &&
> - (cfg->start_bus_number | cfg->end_bus_number) == 0) {
> - printk(KERN_ERR "PCI: start and end of bus number is 0. "
> - "Rejected as broken MCFG.\n");
> - goto reject;
> + for (i = 0; i < pci_mmcfg_config_num; i++) {
> + int valid = 0;
> + u32 size = (cfg->end_bus_number + 1) << 20;
> + cfg = &pci_mmcfg_config[i];
> + printk(KERN_NOTICE "PCI: MCFG configuration %d: base %lx "
> + "segment %hu buses %u - %u\n",
> + i, (unsigned long)cfg->address, cfg->pci_segment,
> + (unsigned int)cfg->start_bus_number,
> + (unsigned int)cfg->end_bus_number);
> +
> + if (!early &&
> + is_acpi_reserved(cfg->address, cfg->address + size -
> 1)) { + printk(KERN_NOTICE "PCI: MCFG area at %Lx
> reserved " + "in ACPI motherboard
> resources\n",
> + cfg->address);
> + valid = 1;
> + }
> +
> + if (valid)
> + continue;
> +
> + if (!early)
> + printk(KERN_ERR "PCI: BIOS Bug: MCFG area at %Lx is
> not" + " reserved in ACPI motherboard
> resources\n", + cfg->address);
> + /* Don't try to do this check unless configuration
> + type 1 is available. how about type 2 ?*/
> + if (raw_pci_ops && e820_all_mapped(cfg->address,
> + cfg->address + size - 1,
> + E820_RESERVED)) {
> + printk(KERN_NOTICE
> + "PCI: MCFG area at %Lx reserved in E820\n",
> + cfg->address);
> + valid = 1;
> + }
> +
> + if (!valid)
> + goto reject;
> }
This loop is a bit messy, is there some way of making it clearer? Maybe the
early vs. late stuff should be split into separate routines entirely...
> @@ -842,11 +842,14 @@ static void set_pcie_port_type(struct pci_dev *pdev)
> * reading the dword at 0x100 which must either be 0 or a valid extended
> * capability header.
> */
> -int pci_cfg_space_size(struct pci_dev *dev)
> +int pci_cfg_space_size_ext(struct pci_dev *dev, unsigned check_exp_pcix)
> {
> int pos;
> u32 status;
>
> + if (!check_exp_pcix)
> + goto skip;
> +
Rather than adding a flag to pci_cfg_space_size, you could either factor out
the extended space probe into a separate routine and use it from both
pci_cfg_space_size and the fixup code, or just make the fixup code do the
probe & cfg_size setting by hand, moving the PCI_CFG_SPACE_SIZE and
PCI_CFG_SPACE_EXP_SIZE to pci.h.
In both cases I'm just trying to avoid having a flag you pass to a routine
that changes its behavior significantly enough that a new function would make
things more readable.
Other than that, things look pretty good. And the resulting kernel boots fine
on my test box, which is nice...
Thanks,
Jesse
¢éì¹»®&Þ~º&¶¬?+-±éݶ¥?w®?Ë?±Êâméb?ìdz¹Þ?)í?æèw*jg¬±¨¶????Ý¢j/?êäz¹Þ??à2?Þ?¨èÚ&¢)ß¡«a¶Úþø®G«?éh®æj:+v?¨?wè?Ù¥>W?±êÞiÛaxPjØm¶?ÿÃ-»+?ùd?_