RE: [PATCH 2/2] Intel-IOMMU, intr-remap: source-id checking
Ingo Molnar wrote: * Han, Weidong weidong@intel.com wrote: Ingo Molnar wrote: * Han, Weidong weidong@intel.com 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/2] Intel-IOMMU, intr-remap: source-id checking
Ingo Molnar wrote: * Han, Weidong weidong@intel.com 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); ir_ioapic[ir_ioapic_num].iommu = iommu; ir_ioapic[ir_ioapic_num].id = scope-enumeration_id; ir_ioapic_num++; -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/2] Intel-IOMMU, intr-remap: source-id checking
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. Regards, Weidong-- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Intel-IOMMU, intr-remap: source-id checking
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, suresh + + path++; + count--; + bus = pdev-subordinate; + } + + if (pdev) { /* PCI discoverable IOAPIC*/ + ir_ioapic[ir_ioapic_num].bus = + pdev-bus-number; + ir_ioapic[ir_ioapic_num].devfn = pdev-devfn; + } else { /* Not PCI discoverable IOAPIC */ + if (!bus) + ir_ioapic[ir_ioapic_num].bus = + scope-bus; + else + ir_ioapic[ir_ioapic_num].bus = + bus-number; + ir_ioapic[ir_ioapic_num].devfn = + PCI_DEVFN(path-dev, path-fn); + } + ir_ioapic[ir_ioapic_num].iommu = iommu; ir_ioapic[ir_ioapic_num].id = scope-enumeration_id; ir_ioapic_num++; -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html