[Xen-devel] Re: [PATCH][VTD] pci mmcfg patch for x86-64 - version 2

["Jan Beulich" <jbeulich@xxxxxxxxxx>]

>>> "Kay, Allen M" <allen.m.kay@xxxxxxxxx> 08.12.08 23:33 >>>
>+#define PCI_MCFG_VIRT_START     (PML4_ADDR(257))
>+#define PCI_MCFG_VIRT_END       (RDWR_MPT_VIRT_START + PML4_ENTRY_BYTES)

The latter line is certainly wrong.

>+static void __iomem * __init mcfg_ioremap(struct acpi_mcfg_allocation *cfg)
>+{
>+      void __iomem *addr;
>+      unsigned long virt;
>+      unsigned long mfn;
>+      unsigned long size, nr_mfn;
>+
>+        /* see asm-x86/config.h, only 2048 PCI segments are supported) */
>+        BUG_ON(cfg->pci_segment >= 2048);

This seems an inappropriate use of BUG_ON() - should either return NULL
here or map just the first 2048 segments (and check in other places
accordingly). Also, I'd say you shouldn't have a literal 2048 here, but rather
calculate the number from PCI_MCFG_VIRT_END.

>+      virt = PCI_MCFG_VIRT_START + (cfg->pci_segment * (1 << 22)) +
>+               (cfg->start_bus_number * (1 << 20));

Isn't the first (segment) shift value supposed to be 28? Also, you have to
use 1UL there, otherwise the multiplication will get truncated to 32 bits.

>+      mfn = cfg->address >> PAGE_SHIFT;
>+        size = (cfg->end_bus_number - cfg->start_bus_number) << 20;

Since end_bus_number is the last valid number, you need to add 1 here
I would think.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel