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

[Bjorn Helgaas <bhelgaas@xxxxxxxxxx>]

On Wed, Jan 28, 2015 at 3:05 PM, Boris Ostrovsky
<boris.ostrovsky@xxxxxxxxxx> wrote:
> On 01/28/2015 01:13 PM, Bjorn Helgaas wrote:
>>
>>
>> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
>> index fd60806..c3e7dfc 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;
>> Thanks, this looks better.
>>
>> There's similar code in xen_initdom_setup_msi_irqs() that looks like it
>> might require a similar fix.
>
>
> Right, I think it does.
>
> One question: do we need to check flags for IORESOURCE_DISABLED as well?
> Currently IORESOURCE_DISABLED and IORESOURCE_UNSET are set together for PCI
> so it probably doesn't matter right now but if this changes we won't want to
> use BAR that's disabled, will we?

That's a good question.  My intent was to use IORESOURCE_DISABLED for
cases where we don't want to even try to assign resources to a BAR,
e.g., for BARs that want more than 4GB of space when the kernel isn't
compiled with support for 64-bit BARs.  In that case, I intended to
set IORESOURCE_UNSET as well.

So I think we're OK with only testing IORESOURCE_UNSET.

Yijing, do you want to expand this patch to fix
xen_initdom_setup_msi_irqs() as well?

Bjorn

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