* [patch] PCI: set ->mask_pos correctly
@ 2013-04-30 7:44 Dan Carpenter
2013-04-30 16:45 ` Bjorn Helgaas
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2013-04-30 7:44 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: linux-pci, kernel-janitors
The "+" operation has higher precedence than "?:" and ->msi_cap is
always non-zero here so the original statement is equivalent to:
entry->mask_pos = PCI_MSI_MASK_64;
Which wasn't the intent.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index d40bed7..2c10752 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -563,8 +563,10 @@ static int msi_capability_init(struct pci_dev *dev, int nvec)
entry->msi_attrib.default_irq = dev->irq; /* Save IOAPIC IRQ */
entry->msi_attrib.pos = dev->msi_cap;
- entry->mask_pos = dev->msi_cap + (control & PCI_MSI_FLAGS_64BIT) ?
- PCI_MSI_MASK_64 : PCI_MSI_MASK_32;
+ if (control & PCI_MSI_FLAGS_64BIT)
+ entry->mask_pos = dev->msi_cap + PCI_MSI_MASK_64;
+ else
+ entry->mask_pos = dev->msi_cap + PCI_MSI_MASK_32;
/* All MSIs are unmasked by default, Mask them all */
if (entry->msi_attrib.maskbit)
pci_read_config_dword(dev, entry->mask_pos, &entry->masked);
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [patch] PCI: set ->mask_pos correctly
2013-04-30 7:44 [patch] PCI: set ->mask_pos correctly Dan Carpenter
@ 2013-04-30 16:45 ` Bjorn Helgaas
2013-04-30 20:57 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Bjorn Helgaas @ 2013-04-30 16:45 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-pci@vger.kernel.org, kernel-janitors
On Tue, Apr 30, 2013 at 1:44 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> The "+" operation has higher precedence than "?:" and ->msi_cap is
> always non-zero here so the original statement is equivalent to:
>
> entry->mask_pos = PCI_MSI_MASK_64;
>
> Which wasn't the intent.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Ouch, my fault, sorry about that. Thanks for finding this. I put
this in my for-linus branch and will try to get this in before
v3.10-rc1.
How did you find this? I guess MSI didn't work right on a device with
32-bit message address and per-vector masking?
Bjorn
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index d40bed7..2c10752 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -563,8 +563,10 @@ static int msi_capability_init(struct pci_dev *dev, int nvec)
> entry->msi_attrib.default_irq = dev->irq; /* Save IOAPIC IRQ */
> entry->msi_attrib.pos = dev->msi_cap;
>
> - entry->mask_pos = dev->msi_cap + (control & PCI_MSI_FLAGS_64BIT) ?
> - PCI_MSI_MASK_64 : PCI_MSI_MASK_32;
> + if (control & PCI_MSI_FLAGS_64BIT)
> + entry->mask_pos = dev->msi_cap + PCI_MSI_MASK_64;
> + else
> + entry->mask_pos = dev->msi_cap + PCI_MSI_MASK_32;
> /* All MSIs are unmasked by default, Mask them all */
> if (entry->msi_attrib.maskbit)
> pci_read_config_dword(dev, entry->mask_pos, &entry->masked);
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [patch] PCI: set ->mask_pos correctly
2013-04-30 16:45 ` Bjorn Helgaas
@ 2013-04-30 20:57 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2013-04-30 20:57 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: linux-pci@vger.kernel.org, kernel-janitors
On Tue, Apr 30, 2013 at 10:45:35AM -0600, Bjorn Helgaas wrote:
> On Tue, Apr 30, 2013 at 1:44 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > The "+" operation has higher precedence than "?:" and ->msi_cap is
> > always non-zero here so the original statement is equivalent to:
> >
> > entry->mask_pos = PCI_MSI_MASK_64;
> >
> > Which wasn't the intent.
> >
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> Ouch, my fault, sorry about that. Thanks for finding this. I put
> this in my for-linus branch and will try to get this in before
> v3.10-rc1.
>
> How did you find this? I guess MSI didn't work right on a device with
> 32-bit message address and per-vector masking?
>
Static analysis. I have a check that complains about every
condition like:
if (foo + bar) { ...
It has too many false positives to foist on the public though.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-04-30 20:57 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-30 7:44 [patch] PCI: set ->mask_pos correctly Dan Carpenter
2013-04-30 16:45 ` Bjorn Helgaas
2013-04-30 20:57 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox