RE: [PATCH 2/2] Intel-IOMMU, intr-remap: source-id checking

2009-05-19 Thread Han, Weidong
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

2009-05-18 Thread Han, Weidong
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

2009-05-11 Thread Han, Weidong
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

2009-05-07 Thread Suresh Siddha
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