Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1229630 > unrolled thread

Re: [PATCH v7] pci: quirk to skip msi disable on shutdown

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v7] pci: quirk to skip msi disable on shutdown Bjorn Helgaas <bhelgaas@google.com> - 2015-09-21 20:30 +0200
    Re: [PATCH v7] pci: quirk to skip msi disable on shutdown "Michael S. Tsirkin" <mst@redhat.com> - 2015-09-21 21:50 +0200
      Re: [PATCH v7] pci: quirk to skip msi disable on shutdown Bjorn Helgaas <bhelgaas@google.com> - 2015-09-22 00:20 +0200
        Re: [PATCH v7] pci: quirk to skip msi disable on shutdown "Michael S. Tsirkin" <mst@redhat.com> - 2015-09-22 13:30 +0200
          Re: [PATCH v7] pci: quirk to skip msi disable on shutdown Bjorn Helgaas <bhelgaas@google.com> - 2015-09-22 14:40 +0200
            Re: [PATCH v7] pci: quirk to skip msi disable on shutdown "Michael S. Tsirkin" <mst@redhat.com> - 2015-09-22 16:10 +0200
              Re: [PATCH v7] pci: quirk to skip msi disable on shutdown Bjorn Helgaas <bhelgaas@google.com> - 2015-09-22 17:50 +0200

#1229630 — Re: [PATCH v7] pci: quirk to skip msi disable on shutdown

On Sun, Sep 06, 2015 at 06:32:35PM +0300, Michael S. Tsirkin wrote:
> On some hypervisors, virtio devices tend to generate spurious interrupts
> when switching between MSI and non-MSI mode.  Normally, either MSI or
> non-MSI is used and all is well, but during shutdown, linux disables MSI
> which then causes an "irq %d: nobody cared" message, with irq being
> subsequently disabled.

My understanding is:

  Linux disables MSI/MSI-X during device shutdown.  If the device
  signals an interrupt after that, it may use INTx.

This INTx interrupt is not necessarily spurious.  Using INTx to signal an
interrupt that occurs when MSI is disabled seems like reasonable behavior
for any PCI device.

And it doesn't seem related to switching between MSI and non-MSI mode.
Yes, the INTx happens *after* disabling MSI, but it is not at all
*because* we disabled MSI.  So I wouldn't say "they generate spurious
interrupts when switching between MSI and non-MSI."

Why doesn't virtio-pci just register an INTx handler in addition to an MSI
handler?

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1229697

On Mon, Sep 21, 2015 at 01:21:47PM -0500, Bjorn Helgaas wrote:
> On Sun, Sep 06, 2015 at 06:32:35PM +0300, Michael S. Tsirkin wrote:
> > On some hypervisors, virtio devices tend to generate spurious interrupts
> > when switching between MSI and non-MSI mode.  Normally, either MSI or
> > non-MSI is used and all is well, but during shutdown, linux disables MSI
> > which then causes an "irq %d: nobody cared" message, with irq being
> > subsequently disabled.
> 
> My understanding is:
> 
>   Linux disables MSI/MSI-X during device shutdown.  If the device
>   signals an interrupt after that, it may use INTx.
> 
> This INTx interrupt is not necessarily spurious.  Using INTx to signal an
> interrupt that occurs when MSI is disabled seems like reasonable behavior
> for any PCI device.
> And it doesn't seem related to switching between MSI and non-MSI mode.
> Yes, the INTx happens *after* disabling MSI, but it is not at all
> *because* we disabled MSI.  So I wouldn't say "they generate spurious
> interrupts when switching between MSI and non-MSI."
> 
> Why doesn't virtio-pci just register an INTx handler in addition to an MSI
> handler?
> 
> Bjorn

The handler causes an expensive exit to the hypervisor,
and the INTx lines are shared with other devices.

Seems silly to slow them down just so we can do something
that triggers the device bug.  The bus master is disabled by that time,
if linux can just desist from touching MSI enable device won't
send either INTx (because MSI is on) or MSI
(because bus master is on) and all will be well.

-- 
MST
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1229783

On Mon, Sep 21, 2015 at 10:42:13PM +0300, Michael S. Tsirkin wrote:
> On Mon, Sep 21, 2015 at 01:21:47PM -0500, Bjorn Helgaas wrote:
> > On Sun, Sep 06, 2015 at 06:32:35PM +0300, Michael S. Tsirkin wrote:
> > > On some hypervisors, virtio devices tend to generate spurious interrupts
> > > when switching between MSI and non-MSI mode.  Normally, either MSI or
> > > non-MSI is used and all is well, but during shutdown, linux disables MSI
> > > which then causes an "irq %d: nobody cared" message, with irq being
> > > subsequently disabled.
> > 
> > My understanding is:
> > 
> >   Linux disables MSI/MSI-X during device shutdown.  If the device
> >   signals an interrupt after that, it may use INTx.
> > 
> > This INTx interrupt is not necessarily spurious.  Using INTx to signal an
> > interrupt that occurs when MSI is disabled seems like reasonable behavior
> > for any PCI device.
> > And it doesn't seem related to switching between MSI and non-MSI mode.
> > Yes, the INTx happens *after* disabling MSI, but it is not at all
> > *because* we disabled MSI.  So I wouldn't say "they generate spurious
> > interrupts when switching between MSI and non-MSI."
> > 
> > Why doesn't virtio-pci just register an INTx handler in addition to an MSI
> > handler?
> 
> The handler causes an expensive exit to the hypervisor,
> and the INTx lines are shared with other devices.

Do we care?  Is this a performance path?  I thought we were in a kexec
shutdown path.

> Seems silly to slow them down just so we can do something
> that triggers the device bug.  The bus master is disabled by that time,
> if linux can just desist from touching MSI enable device won't
> send either INTx (because MSI is on) or MSI
> (because bus master is on) and all will be well.

It would also be silly to put special-purpose code in the PCI core
if there's a reasonable way to handle this in a driver.

Can you describe exactly what the device bug is?  Apparently you're
saying that if we shut down MSI, it triggers the bug?  And I guess
you're talking about a virtio device as implemented in qemu or other
hypervisors?

If we leave MSI enabled (as your patch does), then the device has MSI
enabled and Bus Master disabled.  I can see these possibilities:

  1) the device never recognizes an interrupt condition
  2) the device sets the pending bit but doesn't issue the MSI write,
     so the OS doesn't see the interrupt unless it polls for it
  3) the device signals MSI and we still have an MSI handler
     registered, so we silently handle it
  4) the device signals INTx

You seem to suggest that if we leave MSI enabled (as your patch does),
we're in case 1.  But I doubt that disabling MSI causes the device to
interrupt.

Case 2 seems more likely to me: the device recognized an interrupt
condition, e.g., an event occurred, and the OS simply doesn't see the
interrupt because the device can't issue the MSI message.

Case 3 does seem like it would be a device bug, because the device
shouldn't do an MSI write when Bus Master is disabled.  I don't see
this case mentioned explicitly in the PCI spec, but PCIe r3.0 spec sec
7.5.1.1 does make it clear that disabling Bus Master also disables MSI
messages.

I don't know whether case 4 would be legal or not.  But apparently it
doesn't happen with the virtio device anyway, so it's not really a
concern here.

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1230085

On Mon, Sep 21, 2015 at 05:10:43PM -0500, Bjorn Helgaas wrote:
> On Mon, Sep 21, 2015 at 10:42:13PM +0300, Michael S. Tsirkin wrote:
> > On Mon, Sep 21, 2015 at 01:21:47PM -0500, Bjorn Helgaas wrote:
> > > On Sun, Sep 06, 2015 at 06:32:35PM +0300, Michael S. Tsirkin wrote:
> > > > On some hypervisors, virtio devices tend to generate spurious interrupts
> > > > when switching between MSI and non-MSI mode.  Normally, either MSI or
> > > > non-MSI is used and all is well, but during shutdown, linux disables MSI
> > > > which then causes an "irq %d: nobody cared" message, with irq being
> > > > subsequently disabled.
> > > 
> > > My understanding is:
> > > 
> > >   Linux disables MSI/MSI-X during device shutdown.  If the device
> > >   signals an interrupt after that, it may use INTx.
> > > 
> > > This INTx interrupt is not necessarily spurious.  Using INTx to signal an
> > > interrupt that occurs when MSI is disabled seems like reasonable behavior
> > > for any PCI device.
> > > And it doesn't seem related to switching between MSI and non-MSI mode.
> > > Yes, the INTx happens *after* disabling MSI, but it is not at all
> > > *because* we disabled MSI.  So I wouldn't say "they generate spurious
> > > interrupts when switching between MSI and non-MSI."
> > > 
> > > Why doesn't virtio-pci just register an INTx handler in addition to an MSI
> > > handler?
> > 
> > The handler causes an expensive exit to the hypervisor,
> > and the INTx lines are shared with other devices.
> 
> Do we care?  Is this a performance path?  I thought we were in a kexec
> shutdown path.

Yes but the handler would always have to be registered, right?

> > Seems silly to slow them down just so we can do something
> > that triggers the device bug.  The bus master is disabled by that time,
> > if linux can just desist from touching MSI enable device won't
> > send either INTx (because MSI is on) or MSI
> > (because bus master is on) and all will be well.
> 
> It would also be silly to put special-purpose code in the PCI core
> if there's a reasonable way to handle this in a driver.
> 
> Can you describe exactly what the device bug is?  Apparently you're
> saying that if we shut down MSI, it triggers the bug?  And I guess
> you're talking about a virtio device as implemented in qemu or other
> hypervisors?

Yes. Basically depending on an internal device state, disabling MSI
sometimes wedges it.  The most easy to debug effect is if it starts
sending INTx interrupts, for which there's no handler currently.
Full system reset always gets us out of the bad state.


> If we leave MSI enabled (as your patch does), then the device has MSI
> enabled and Bus Master disabled.  I can see these possibilities:
> 
>   1) the device never recognizes an interrupt condition
>   2) the device sets the pending bit but doesn't issue the MSI write,
>      so the OS doesn't see the interrupt unless it polls for it
>   3) the device signals MSI and we still have an MSI handler
>      registered, so we silently handle it
>   4) the device signals INTx
> You seem to suggest that if we leave MSI enabled (as your patch does),
> we're in case 1.  But I doubt that disabling MSI causes the device to
> interrupt.
> 
> Case 2 seems more likely to me: the device recognized an interrupt
> condition, e.g., an event occurred, and the OS simply doesn't see the
> interrupt because the device can't issue the MSI message.
> 
> Case 3 does seem like it would be a device bug, because the device
> shouldn't do an MSI write when Bus Master is disabled.  I don't see
> this case mentioned explicitly in the PCI spec, but PCIe r3.0 spec sec
> 7.5.1.1 does make it clear that disabling Bus Master also disables MSI
> messages.
> 
> I don't know whether case 4 would be legal or not.  But apparently it
> doesn't happen with the virtio device anyway, so it's not really a
> concern here.
> 
> Bjorn

It's case 2. Except it doesn't actually set the pending
bit, because PCI spec has language like

	"While a vector is masked, the function is prohibited from sending the
	associated message, and the function must set the associated Pending bit"

but doesn't actually require to set pending if bus master enable
prevents sending MSI.

-- 
MST
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1230125

On Tue, Sep 22, 2015 at 02:29:03PM +0300, Michael S. Tsirkin wrote:
> On Mon, Sep 21, 2015 at 05:10:43PM -0500, Bjorn Helgaas wrote:
> > On Mon, Sep 21, 2015 at 10:42:13PM +0300, Michael S. Tsirkin wrote:
> > > On Mon, Sep 21, 2015 at 01:21:47PM -0500, Bjorn Helgaas wrote:
> > > > On Sun, Sep 06, 2015 at 06:32:35PM +0300, Michael S. Tsirkin wrote:
> > > > > On some hypervisors, virtio devices tend to generate spurious interrupts
> > > > > when switching between MSI and non-MSI mode.  Normally, either MSI or
> > > > > non-MSI is used and all is well, but during shutdown, linux disables MSI
> > > > > which then causes an "irq %d: nobody cared" message, with irq being
> > > > > subsequently disabled.
> > > > 
> > > > My understanding is:
> > > > 
> > > >   Linux disables MSI/MSI-X during device shutdown.  If the device
> > > >   signals an interrupt after that, it may use INTx.
> > > > 
> > > > This INTx interrupt is not necessarily spurious.  Using INTx to signal an
> > > > interrupt that occurs when MSI is disabled seems like reasonable behavior
> > > > for any PCI device.
> > > > And it doesn't seem related to switching between MSI and non-MSI mode.
> > > > Yes, the INTx happens *after* disabling MSI, but it is not at all
> > > > *because* we disabled MSI.  So I wouldn't say "they generate spurious
> > > > interrupts when switching between MSI and non-MSI."
> > > > 
> > > > Why doesn't virtio-pci just register an INTx handler in addition to an MSI
> > > > handler?
> > > 
> > > The handler causes an expensive exit to the hypervisor,
> > > and the INTx lines are shared with other devices.
> > 
> > Do we care?  Is this a performance path?  I thought we were in a kexec
> > shutdown path.
> 
> Yes but the handler would always have to be registered, right?

The pci_device_shutdown() path you're modifying calls drv->shutdown()
immediately before disabling MSI, so I suppose you could register a
handler in a virtio shutdown method.

> > > Seems silly to slow them down just so we can do something
> > > that triggers the device bug.  The bus master is disabled by that time,
> > > if linux can just desist from touching MSI enable device won't
> > > send either INTx (because MSI is on) or MSI
> > > (because bus master is on) and all will be well.
> > 
> > It would also be silly to put special-purpose code in the PCI core
> > if there's a reasonable way to handle this in a driver.
> > 
> > Can you describe exactly what the device bug is?  Apparently you're
> > saying that if we shut down MSI, it triggers the bug?  And I guess
> > you're talking about a virtio device as implemented in qemu or other
> > hypervisors?
> 
> Yes. Basically depending on an internal device state, disabling MSI
> sometimes wedges it.  The most easy to debug effect is if it starts
> sending INTx interrupts, for which there's no handler currently.
> Full system reset always gets us out of the bad state.

If disabling MSI causes the device to use INTx interrupts, that sounds
perfectly normal to me.

If disabling MSI causes the device to hang, *that* sounds like a bug.
Since this is virtio, we should be able to figure out exactly where
that happens.  Do you have a pointer to a virtio bug report, or even a
QEMU commit that fixes this virtio bug?

I understand that even if there is a virtio fix in QEMU, we want a
solution that works even with an old QEMU that doesn't contain the
fix.  But a pointer to a QEMU fix would really help understand and
document the Linux fix.

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1230239

On Tue, Sep 22, 2015 at 07:36:40AM -0500, Bjorn Helgaas wrote:
> On Tue, Sep 22, 2015 at 02:29:03PM +0300, Michael S. Tsirkin wrote:
> > On Mon, Sep 21, 2015 at 05:10:43PM -0500, Bjorn Helgaas wrote:
> > > On Mon, Sep 21, 2015 at 10:42:13PM +0300, Michael S. Tsirkin wrote:
> > > > On Mon, Sep 21, 2015 at 01:21:47PM -0500, Bjorn Helgaas wrote:
> > > > > On Sun, Sep 06, 2015 at 06:32:35PM +0300, Michael S. Tsirkin wrote:
> > > > > > On some hypervisors, virtio devices tend to generate spurious interrupts
> > > > > > when switching between MSI and non-MSI mode.  Normally, either MSI or
> > > > > > non-MSI is used and all is well, but during shutdown, linux disables MSI
> > > > > > which then causes an "irq %d: nobody cared" message, with irq being
> > > > > > subsequently disabled.
> > > > > 
> > > > > My understanding is:
> > > > > 
> > > > >   Linux disables MSI/MSI-X during device shutdown.  If the device
> > > > >   signals an interrupt after that, it may use INTx.
> > > > > 
> > > > > This INTx interrupt is not necessarily spurious.  Using INTx to signal an
> > > > > interrupt that occurs when MSI is disabled seems like reasonable behavior
> > > > > for any PCI device.
> > > > > And it doesn't seem related to switching between MSI and non-MSI mode.
> > > > > Yes, the INTx happens *after* disabling MSI, but it is not at all
> > > > > *because* we disabled MSI.  So I wouldn't say "they generate spurious
> > > > > interrupts when switching between MSI and non-MSI."
> > > > > 
> > > > > Why doesn't virtio-pci just register an INTx handler in addition to an MSI
> > > > > handler?
> > > > 
> > > > The handler causes an expensive exit to the hypervisor,
> > > > and the INTx lines are shared with other devices.
> > > 
> > > Do we care?  Is this a performance path?  I thought we were in a kexec
> > > shutdown path.
> > 
> > Yes but the handler would always have to be registered, right?
> 
> The pci_device_shutdown() path you're modifying calls drv->shutdown()
> immediately before disabling MSI, so I suppose you could register a
> handler in a virtio shutdown method.

I guess we could. Not sure what we'd do e.g. if that fails.

> > > > Seems silly to slow them down just so we can do something
> > > > that triggers the device bug.  The bus master is disabled by that time,
> > > > if linux can just desist from touching MSI enable device won't
> > > > send either INTx (because MSI is on) or MSI
> > > > (because bus master is on) and all will be well.
> > > 
> > > It would also be silly to put special-purpose code in the PCI core
> > > if there's a reasonable way to handle this in a driver.
> > > 
> > > Can you describe exactly what the device bug is?  Apparently you're
> > > saying that if we shut down MSI, it triggers the bug?  And I guess
> > > you're talking about a virtio device as implemented in qemu or other
> > > hypervisors?
> > 
> > Yes. Basically depending on an internal device state, disabling MSI
> > sometimes wedges it.  The most easy to debug effect is if it starts
> > sending INTx interrupts, for which there's no handler currently.
> > Full system reset always gets us out of the bad state.
> 
> If disabling MSI causes the device to use INTx interrupts, that sounds
> perfectly normal to me.
> 
> If disabling MSI causes the device to hang, *that* sounds like a bug.
> Since this is virtio, we should be able to figure out exactly where
> that happens.  Do you have a pointer to a virtio bug report, or even a
> QEMU commit that fixes this virtio bug?
> 
> I understand that even if there is a virtio fix in QEMU, we want a
> solution that works even with an old QEMU that doesn't contain the
> fix.  But a pointer to a QEMU fix would really help understand and
> document the Linux fix.
> 
> Bjorn

I'm not sure we ever understood it completely.

I think some of it has to do with the way the whole virtio 0
device register layout changes when you enable/disable MSI.  So should
be ok when using the modern virtio 1 model since we fixed this thing.

I was hoping that since disabling MSI in pci core is only useful as a
work-around (for devices with a broken bus master enable - even though I
don't think we know what these are exactly), a flag for not disabling it
won't be held to such a high standard.

-- 
MST
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1230356

On Tue, Sep 22, 2015 at 05:07:19PM +0300, Michael S. Tsirkin wrote:
> On Tue, Sep 22, 2015 at 07:36:40AM -0500, Bjorn Helgaas wrote:
> > On Tue, Sep 22, 2015 at 02:29:03PM +0300, Michael S. Tsirkin wrote:
> > > On Mon, Sep 21, 2015 at 05:10:43PM -0500, Bjorn Helgaas wrote:

> > > > Can you describe exactly what the device bug is?  Apparently you're
> > > > saying that if we shut down MSI, it triggers the bug?  And I guess
> > > > you're talking about a virtio device as implemented in qemu or other
> > > > hypervisors?
> > > 
> > > Yes. Basically depending on an internal device state, disabling MSI
> > > sometimes wedges it.  The most easy to debug effect is if it starts
> > > sending INTx interrupts, for which there's no handler currently.
> > > Full system reset always gets us out of the bad state.
> > 
> > If disabling MSI causes the device to use INTx interrupts, that sounds
> > perfectly normal to me.
> > 
> > If disabling MSI causes the device to hang, *that* sounds like a bug.
> > Since this is virtio, we should be able to figure out exactly where
> > that happens.  Do you have a pointer to a virtio bug report, or even a
> > QEMU commit that fixes this virtio bug?
> > 
> > I understand that even if there is a virtio fix in QEMU, we want a
> > solution that works even with an old QEMU that doesn't contain the
> > fix.  But a pointer to a QEMU fix would really help understand and
> > document the Linux fix.
> 
> I'm not sure we ever understood it completely.
> 
> I think some of it has to do with the way the whole virtio 0
> device register layout changes when you enable/disable MSI.  So should
> be ok when using the modern virtio 1 model since we fixed this thing.

If you know that:

  - pci_device_shutdown() disables MSI,
  - disabling MSI changes the virtio register layout, and
  - the driver may touch the device after pci_device_shutdown(),

it seems like you might want a virtio shutdown method so you can
find out when the register layout changes.  Changing the register
layout sounds completely broken, and dealing with it sounds racy, but
it sounds like a mess that should be handled by the driver.

> I was hoping that since disabling MSI in pci core is only useful as a
> work-around (for devices with a broken bus master enable - even though I
> don't think we know what these are exactly), a flag for not disabling it
> won't be held to such a high standard.

Well, I suppose you see the problem with adding workarounds on top of
workarounds, especially when we don't have a clear understanding of
why we need them.  The "disable MSI on shutdown" code was added here:

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=d52877c7b1af

and there is no information in that patch about it being a workaround
for devices with broken bus master enable.

The cumulative effect of stuff like this is that it becomes impossible
to do any meaningful restructuring in the core without breaking some
corner case.

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web