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

[Espen Skoglund <espen.skoglund@xxxxxxxxxxxxx>]

[Allen M Kay]
> This is version 3 of mmconfig patch that addresses all of the
> feedbacks I received from Jan and Espen yesterday.

> Signed-off-by: Allen Kay <allen.m.kay@xxxxxxxxx>

Some comments inline.

        eSk


================================================================

> diff -r 6595393a3d28 xen/arch/x86/mmconfig_32.c
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/xen/arch/x86/mmconfig_32.c      Tue Dec 09 06:42:04 2008 -0800

> +int pci_mmcfg_read(unsigned int seg, unsigned int bus,
> +                       unsigned int devfn, int reg, int len, u32 *value)
> +{
> +     *value = -1;
> +     return 0;
> +}
> +
> +int pci_mmcfg_write(unsigned int seg, unsigned int bus,
> +                        unsigned int devfn, int reg, int len, u32 value)
> +{
> +     return -EINVAL;
> +}

These two can be removed (see further down).

> +int __init pci_mmcfg_arch_init(void)
> +{
> +     return 1;
> +}

Should return 0.  And add a dummy pci_dev_base() function.


> diff -r 6595393a3d28 xen/arch/x86/mmconfig_64.c
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/xen/arch/x86/mmconfig_64.c      Tue Dec 09 06:42:13 2008 -0800

Put following two functions in generic x86.

> +int pci_mmcfg_read(unsigned int seg, unsigned int bus,
> +                       unsigned int devfn, int reg, int len, u32 *value)
> +{
> +     char __iomem *addr;
> +
> +     /* Why do we have this when nobody checks it. How about a BUG()!? -AK */
> +     if (unlikely((bus > 255) || (devfn > 255) || (reg > 4095))) {
> +err:         *value = -1;
> +             return -EINVAL;
> +     }

Also fail if (pci_probe & PCI_PROBE_MMCONF) == 0.  Or alternatively do
this check in pci_dev_base().

> +     addr = pci_dev_base(seg, bus, devfn);
> +     if (!addr)
> +             goto err;
> +
> +     switch (len) {
> +     case 1:
> +             *value = mmio_config_readb(addr + reg);
> +             break;
> +     case 2:
> +             *value = mmio_config_readw(addr + reg);
> +             break;
> +     case 4:
> +             *value = mmio_config_readl(addr + reg);
> +             break;
> +     }
> +
> +     return 0;
> +}
> +
> +int pci_mmcfg_write(unsigned int seg, unsigned int bus,
> +                        unsigned int devfn, int reg, int len, u32 value)
> +{
> +     char __iomem *addr;
> +
> +     /* Why do we have this when nobody checks it. How about a BUG()!? -AK */
> +     if (unlikely((bus > 255) || (devfn > 255) || (reg > 4095)))
> +             return -EINVAL;

Also fail if (pci_probe & PCI_PROBE_MMCONF) == 0.

> +     addr = pci_dev_base(seg, bus, devfn);
> +     if (!addr)
> +             return -EINVAL;
> +
> +     switch (len) {
> +     case 1:
> +             mmio_config_writeb(addr + reg, value);
> +             break;
> +     case 2:
> +             mmio_config_writew(addr + reg, value);
> +             break;
> +     case 4:
> +             mmio_config_writel(addr + reg, value);
> +             break;
> +     }
> +
> +     return 0;
> +}


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