From: Bjorn Helgaas <helgaas@kernel.org>
To: Conor Dooley <mail@conchuod.ie>
Cc: Marc Zyngier <maz@kernel.org>,
Conor.Dooley@microchip.com, lorenzo.pieralisi@arm.com,
Daire.McNamara@microchip.com, bhelgaas@google.com,
Cyril.Jean@microchip.com, david.abdurachmanov@gmail.com,
linux-pci@vger.kernel.org, robh@kernel.org
Subject: Re: [RESEND PATCH v1 1/1] PCI: microchip: Fix potential race in interrupt handling
Date: Wed, 4 May 2022 11:59:24 -0500 [thread overview]
Message-ID: <20220504165924.GA453752@bhelgaas> (raw)
In-Reply-To: <199f5479-b212-e1ac-f9e4-d5d13708cb0c@conchuod.ie>
On Wed, May 04, 2022 at 04:12:39PM +0100, Conor Dooley wrote:
> On 02/05/2022 20:22, Bjorn Helgaas wrote:
> > On Sat, Apr 30, 2022 at 12:33:51AM +0100, Marc Zyngier wrote:
> >> On Fri, 29 Apr 2022 22:57:33 +0100,
> >> Bjorn Helgaas <helgaas@kernel.org> wrote:
> >>> On Fri, Apr 29, 2022 at 09:42:52AM +0000, Conor.Dooley@microchip.com wrote:
> >>>> On 28/04/2022 10:29, Lorenzo Pieralisi wrote:
> >>>>> On Tue, Apr 05, 2022 at 12:17:51PM +0100, daire.mcnamara@microchip.com wrote:
> >>>>>> From: Daire McNamara <daire.mcnamara@microchip.com>
> >>>>>>
> >>>>>> Clear MSI bit in ISTATUS register after reading it before
> >>>>>> handling individual MSI bits
> >
> >>>> Clear the MSI bit in ISTATUS register after reading it, but before
> >>>> reading and handling individual MSI bits from the IMSI register.
> >>>> This avoids a potential race where new MSI bits may be set on the
> >>>> IMSI register after it was read and be missed when the MSI bit in
> >>>> the ISTATUS register is cleared.
Restoring the context here:
> >>> "ISTATUS" doesn't appear in the code as a register name.
> >>> Neither does "IMSI". Please use names that match the code.
> Daire is still having the IT issues, so before I resend the patch with
> a new commit message, how is the following:
>
> Clear the MSI bit in ISTATUS_LOCAL register after reading it, but
> before reading and handling individual MSI bits from the ISTATUS_MSI
> register. This avoids a potential race where new MSI bits may be set
> on the ISTATUS_MSI register after it was read and be missed when the
> MSI bit in the ISTATUS_LOCAL register is cleared.
Looks good, thank you!
> >>> And speaking of that, I looked at all the users of
> >>> irq_set_chained_handler_and_data() in drivers/pci. All the handlers
> >>> except mc_handle_intx() and mc_handle_msi() call chained_irq_enter()
> >>> and chained_irq_exit().
> >>>
> >>> Are mc_handle_intx() and mc_handle_msi() just really special, or is
> >>> this a mistake?
> >>
> >> That's just a bug. On the right HW, this would just result in lost
> >> interrupts.
>
> Separate issue, separate patch. Do you want them in a series or as
> another standalone patch?
Agreed, should be a separate patch. Doesn't need to be a series
unless that patch only applies correctly on top of this one.
Bjorn
next prev parent reply other threads:[~2022-05-04 17:13 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-05 11:17 [RESEND PATCH v1 1/1] PCI: microchip: Fix potential race in interrupt handling daire.mcnamara
2022-04-28 6:30 ` Conor.Dooley
2022-04-28 9:29 ` Lorenzo Pieralisi
2022-04-29 9:42 ` Conor.Dooley
2022-04-29 21:57 ` Bjorn Helgaas
2022-04-29 23:33 ` Marc Zyngier
2022-05-02 19:22 ` Bjorn Helgaas
2022-05-04 15:12 ` Conor Dooley
2022-05-04 16:53 ` Lorenzo Pieralisi
2022-05-04 16:57 ` Conor Dooley
2022-05-04 16:59 ` Bjorn Helgaas [this message]
2022-05-11 10:00 ` Conor Dooley
2022-05-11 12:41 ` Lorenzo Pieralisi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220504165924.GA453752@bhelgaas \
--to=helgaas@kernel.org \
--cc=Conor.Dooley@microchip.com \
--cc=Cyril.Jean@microchip.com \
--cc=Daire.McNamara@microchip.com \
--cc=bhelgaas@google.com \
--cc=david.abdurachmanov@gmail.com \
--cc=linux-pci@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=mail@conchuod.ie \
--cc=maz@kernel.org \
--cc=robh@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).