Re: [Xen-devel] [PATCH v3] PCI: Add guard to avoid mapping a invalid msix base address

[Yijing Wang <wangyijing@xxxxxxxxxx>]

> I applied this to pci/msi for v3.20, thanks!  I reworked the changelog as
> follows; let me know if it still doesn't make things clear:
> 

It's more clear, thanks for your improvement very much!

Thanks!
Yijing.

> 
> commit 6a878e5085fe97bd1e222b7883a1b815fcbbe4ed
> Author: Yijing Wang <wangyijing@xxxxxxxxxx>
> Date:   Wed Jan 28 09:52:17 2015 +0800
> 
>     PCI: Fail MSI-X mappings if there's no space assigned to MSI-X BAR
>     
>     Unlike MSI, which is configured via registers in the MSI capability in
>     Configuration Space, MSI-X is configured via tables in Memory Space.
>     These MSI-X tables are mapped by a device BAR, and if no Memory Space
>     has been assigned to the BAR, MSI-X cannot be used.
>     
>     Fail MSI-X setup if no space has been assigned for the BAR.
>     
>     Previously, we ioremapped the MSI-X table even if the resource hadn't been
>     assigned.  In this case, the resource address is undefined (and is often
>     zero), which may lead to warnings or oopses in this path:
>     
>       pci_enable_msix
>         msix_capability_init
>           msix_map_region
>             ioremap_nocache
>     
>     The PCI core sets resource flags to zero when it can't assign space for 
> the
>     resource (see reset_resource()).  There are also some cases where it sets
>     the IORESOURCE_UNSET flag, e.g., pci_reassigndev_resource_alignment(),
>     pci_assign_resource(), etc.  So we must check for both cases.
>     
>     [bhelgaas: changelog]
>     Reported-by: Zhang Jukuo <zhangjukuo@xxxxxxxxxx>
>     Tested-by: Zhang Jukuo <zhangjukuo@xxxxxxxxxx>
>     Signed-off-by: Yijing Wang <wangyijing@xxxxxxxxxx>
>     Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> 
> diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
> index c489ef2c1a39..34fc4189ebf0 100644
> --- a/arch/x86/pci/xen.c
> +++ b/arch/x86/pci/xen.c
> @@ -298,12 +298,16 @@ static int xen_initdom_setup_msi_irqs(struct pci_dev 
> *dev, int nvec, int type)
>                       map_irq.entry_nr = nvec;
>               } else if (type == PCI_CAP_ID_MSIX) {
>                       int pos;
> +                     unsigned long flags;
>                       u32 table_offset, bir;
>  
>                       pos = dev->msix_cap;
>                       pci_read_config_dword(dev, pos + PCI_MSIX_TABLE,
>                                             &table_offset);
>                       bir = (u8)(table_offset & PCI_MSIX_TABLE_BIR);
> +                     flags = pci_resource_flags(dev, bir);
> +                     if (!flags || (flags & IORESOURCE_UNSET))
> +                             return -EINVAL;
>  
>                       map_irq.table_base = pci_resource_start(dev, bir);
>                       map_irq.entry_nr = msidesc->msi_attrib.entry_nr;
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index fd60806d3fd0..c3e7dfcf9ff5 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -694,11 +694,16 @@ static void __iomem *msix_map_region(struct pci_dev 
> *dev, unsigned nr_entries)
>  {
>       resource_size_t phys_addr;
>       u32 table_offset;
> +     unsigned long flags;
>       u8 bir;
>  
>       pci_read_config_dword(dev, dev->msix_cap + PCI_MSIX_TABLE,
>                             &table_offset);
>       bir = (u8)(table_offset & PCI_MSIX_TABLE_BIR);
> +     flags = pci_resource_flags(dev, bir);
> +     if (!flags || (flags & IORESOURCE_UNSET))
> +             return NULL;
> +
>       table_offset &= PCI_MSIX_TABLE_OFFSET;
>       phys_addr = pci_resource_start(dev, bir) + table_offset;
>  
> 
> .
> 


-- 
Thanks!
Yijing


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel