Bjorn Helgaas wrote:
> On Wed, 2009-11-11 at 16:31 -0800, Yinghai Lu wrote:
>> Bjorn Helgaas wrote:
>>> This updates printks so they have more context and look more like the
>>> usual PCI stuff.
>>>
>>> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@xxxxxx>
>>> ---
>>> arch/x86/pci/amd_bus.c | 43 ++++++++++++++++++++++++++-----------------
>>> 1 files changed, 26 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/arch/x86/pci/amd_bus.c b/arch/x86/pci/amd_bus.c
>>> index 995f360..9462455 100644
>>> --- a/arch/x86/pci/amd_bus.c
>>> +++ b/arch/x86/pci/amd_bus.c
>>> @@ -49,14 +49,13 @@ void x86_pci_root_bus_res_quirks(struct pci_bus *b)
>>> if (i == pci_root_num)
>>> return;
>>>
>>> - printk(KERN_DEBUG "PCI: peer root bus %02x res updated from pci conf\n",
>>> - b->number);
>>> -
>>> info = &pci_root_info[i];
>>> for (j = 0; j < info->res_num; j++) {
>>> struct resource *res;
>>> struct resource *root;
>>>
>>> + dev_printk(KERN_DEBUG, &b->dev,
>>> + "resource %d %pR (original)\n", j, b->resource[j]);
>> last time when i looked at %pR, it seems it can not handle all 0 (include flags) resource
>>
>> here, old one could have two entries: io root, and mmio_root.
>>
>> new value could have io port, mmio, mmio pref. aka res_num == 3,
>>
>> so may have %pR to handle all 0 res, or skip the print when !b->resouce[i].flags ?
>
> These are pci_bus resources, which are just pointers to the real
> resource somewhere else, and %pR can deal with null pointers fine.
> Here's a sample:
>
> pci_bus 0000:00: resource 2 (null) (original)
> pci_bus 0000:00: resource 2 [mem 0xfe000000-0xffffffff] (updated from
> hardware)
> pci_bus 0000:00: resource 3 (null) (original)
> pci_bus 0000:00: resource 3 [mem 0x4030000000-0xfcffffffff] (updated
> from hardware)
>
>>> res = &info->res[j];
>>> b->resource[j] = res;
>>> if (res->flags & IORESOURCE_IO)
>>> @@ -64,6 +63,8 @@ void x86_pci_root_bus_res_quirks(struct pci_bus *b)
>>> else
>>> root = &iomem_resource;
>>> insert_resource(root, res);
>>> + dev_printk(KERN_DEBUG, &b->dev,
>>> + "resource %d %pR (updated from hardware)\n", j, res);
>>> }
>>> }
>>>
>>> @@ -234,6 +235,7 @@ static int __init early_fill_mp_bus_info(void)
>>> struct res_range range[RANGE_NUM];
>>> u64 val;
>>> u32 address;
>>> + u16 vendor, device;
>>>
>>> if (!early_pci_allowed())
>>> return -1;
>>> @@ -241,8 +243,6 @@ static int __init early_fill_mp_bus_info(void)
>>> found_all_numa_early = 0;
>>> for (i = 0; i < ARRAY_SIZE(pci_probes); i++) {
>>> u32 id;
>>> - u16 device;
>>> - u16 vendor;
>>>
>>> bus = pci_probes[i].bus;
>>> slot = pci_probes[i].slot;
>>> @@ -260,6 +260,9 @@ static int __init early_fill_mp_bus_info(void)
>>> if (!found_all_numa_early)
>>> return 0;
>>>
>>> + printk(KERN_INFO "pci 0000:%02x:%02x.0: AMD Family 10h Northbridge "
>>> + "[%04x:%04x]\n", bus, slot, vendor, device);
>>> +
>> on amd system, it happens you can read all layout from one of host bridge pci conf.
>> that doesn't mean to that hostbridge only.
>> so should link to bus/slot here
>
> If you'd prefer different text than mine here, can you propose
> something?
please just remove bus slot from printk, and it is meaningless.
>
>>> pci_root_num = 0;
>>> for (i = 0; i < 4; i++) {
>>> int min_bus;
>>> @@ -318,7 +321,8 @@ static int __init early_fill_mp_bus_info(void)
>>> continue; /* not found */
>>>
>>> info = &pci_root_info[j];
>>> - printk(KERN_DEBUG "node %d link %d: io port [%llx, %llx]\n",
>>> + printk(KERN_DEBUG "pci 0000:%02x:%02x.1: node %d link %d: "
>>> + "[io %#llx-%#llx]\n", bus, slot,
>>> node, link, (u64)start, (u64)end);
>> not here
>
> I think maybe we should just remove this printk altogether.
we need to keep range. some BIOS have total wrong in _CRS for that, need to print it out
so BIOS could be fixed later ( it ...)
>
>>> /* kernel only handle 16 bit only */
>>> @@ -353,7 +357,8 @@ static int __init early_fill_mp_bus_info(void)
>>> address = MSR_K8_TOP_MEM1;
>>> rdmsrl(address, val);
>>> end = (val & 0xffffff800000ULL);
>>> - printk(KERN_INFO "TOM: %016lx aka %ldM\n", end, end>>20);
>>> + printk(KERN_INFO "pci 0000:%02x:%02x:0: TOM: %#016lx (%ldM)\n",
>>> + bus, slot, end, end>>20);
>> not from TOM
>
> This one is not associated with a PCI device at all, so I'd be glad to
> change this. But "TOM: %016lx aka %ldM" all by itself is completely
> meaningless to the average person, so I'd like to add something to it.
> Maybe a hint about what we use the TOM value for? I'll read that code
> in more detail tomorrow; it looks like you might be just removing the
> RAM range, the MMCONFIG range, etc, from the host bridge apertures?
yes. some hw overwrite sequences. mmconfig range may be intercepted early....
>
>>> if (end < (1ULL<<32))
>>> update_range(range, 0, end - 1);
>>>
>>> @@ -361,7 +366,9 @@ static int __init early_fill_mp_bus_info(void)
>>> get_pci_mmcfg_amd_fam10h_range();
>>> /* need to take out mmconf range */
>>> if (fam10h_mmconf_end) {
>>> - printk(KERN_DEBUG "Fam 10h mmconf [%llx, %llx]\n", fam10h_mmconf_start, fam10h_mmconf_end);
>>> + printk(KERN_DEBUG "pci 0000:%02x:%02x.0: MMCONFIG region "
>>> + "[mem %#llx-%#llx]\n", bus, slot,
>>> + fam10h_mmconf_start, fam10h_mmconf_end);
>> not for mmconf.
>
> This one isn't *read* from a PCI device either, but it's certainly
> *connected* to PCI. Maybe something like this?
>
> MMCONFIG region [mem %#llx-%#llx] for domains %04x-%04x
could just print
Fam 10h MMCONFIG region [mem %#llx-%#llx]
>
>>> update_range(range, fam10h_mmconf_start, fam10h_mmconf_end);
>>> }
>>>
>>> @@ -391,7 +398,8 @@ static int __init early_fill_mp_bus_info(void)
>>>
>>> info = &pci_root_info[j];
>>>
>>> - printk(KERN_DEBUG "node %d link %d: mmio [%llx, %llx]",
>>> + printk(KERN_DEBUG "pci 0000:%02x:%02x.1: node %d link %d: "
>>> + "[mem %#llx-%#llx]", bus, slot,
>>> node, link, (u64)start, (u64)end);
>> not here
>
> Maybe just remove this one too (along with the KERN_CONT lines below)?
>
> Or, if these are really telling us something useful, can you help me
> understand what it is and how it connects to PCI resource management.
just not use bus, slot here and later.
AMD K8 system could have 4 hypertransport chains. and every chain is one peer root bus.
we can read the range io/mmio/pref mmio from 00:18.1 only for whole mapping. even the systems is 1 socket, 2 sockets, 4 sockets, 8 sockets.
the range is about bus range to node/link,
io range to node/link
mmio range to node/link
so use node/link comparing to map io range to bus range.
also need to have left over rnage to default node/link aka the real lpc bus...hanging.
so raw range and final effective range to node/link bus range are print out all.
the corresponding info be 0x:18.1 to 0x1f.1... are the same.
Just remove bus slot reference in your patch.
YH
--
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
