Ingo Molnar wrote: > * Han, Weidong <weidong.han@xxxxxxxxx> wrote: > >> Ingo Molnar wrote: >>> * Han, Weidong <weidong.han@xxxxxxxxx> wrote: >>> >>>> Siddha, Suresh B wrote: >>>>> On Wed, 2009-05-06 at 23:16 -0700, Han, Weidong wrote: >>>>>> @@ -634,6 +694,44 @@ static int ir_parse_ioapic_scope(struct >>>>>> acpi_dmar_header *header, " 0x%Lx\n", >>>>>> scope->enumeration_id, drhd->address); >>>>>> >>>>>> + bus = pci_find_bus(drhd->segment, scope->bus); >>>>>> + path = (struct acpi_dmar_pci_path *)(scope + 1); + count = >>>>>> (scope->length - + sizeof(struct acpi_dmar_device_scope)) >>>>>> + / sizeof(struct acpi_dmar_pci_path); >>>>>> + >>>>>> + while (count) { >>>>>> + if (pdev) >>>>>> + pci_dev_put(pdev); >>>>>> + >>>>>> + if (!bus) >>>>>> + break; >>>>>> + >>>>>> + pdev = pci_get_slot(bus, >>>>>> + PCI_DEVFN(path->dev, path->fn)); >>>>>> + if (!pdev) >>>>>> + break; >>>>> >>>>> ir_parse_ioapic_scope() happens very early in the boot. So, I >>>>> don't think we can do the pci related discovery here. >>>>> >>>> >>>> Thanks for your pointing it out. It should enable the source-id >>>> checking for io-apic's after the pci subsystem is up. I will >>>> change it. >>> >>> Note, there's ways to do early PCI quirks too, check >>> arch/x86/kernel/early-quirks.c. It's done by reading the PCI >>> configuration space directly via a careful early-capable subset of >>> the PCI config space APIs. >>> >>> But it's a method of last resort. >>> >> >> Thanks for your reminder. It can use direct PCI access here as >> follows. It's easy and clean. I think it's better than adding the >> source-id checking for io-apic's after the pci subsystem is up. I >> will send out updated patches after some tests. >> >> @@ -634,6 +695,24 @@ static int ir_parse_ioapic_scope(struct >> acpi_dmar_header *header, " 0x%Lx\n", >> scope->enumeration_id, drhd->address); >> >> + bus = scope->bus; >> + path = (struct acpi_dmar_pci_path *)(scope + >> 1); + count = (scope->length - >> + sizeof(struct >> acpi_dmar_device_scope)) + / >> sizeof(struct acpi_dmar_pci_path); + + while >> (--count > 0) { + /* Access PCI >> directly due to the PCI + * subsystem >> isn't initialized yet. + */ >> + bus = read_pci_config_byte(bus, >> path->dev, + path->fn, >> PCI_SECONDARY_BUS); + path++; >> + } >> + >> + ir_ioapic[ir_ioapic_num].bus = bus; >> + ir_ioapic[ir_ioapic_num].devfn = >> + PCI_DEVFN(path->dev, >> path->fn); > > looks good IMO, beyond the obligatory comment-style nitpick [*] :-) > Also, the function above seems to be way too large - please split it > into a couple of natural helper functions. > > Thanks, > > Ingo > > [*] > > Please use the customary comment style: > > /* > * Comment ..... > * ...... goes here: > */ > > specified in Documentation/CodingStyle. I have sent out the updated patches. Thanks! Regards, Weidong-- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html