Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Thomas Gleixner <tglx@linutronix.de>
To:     Evan Green <evgreen@chromium.org>, Rajat Jain <rajatja@google.com>
Cc:     Bjorn Helgaas <bhelgaas@google.com>,
        linux-pci <linux-pci@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] PCI/MSI: Avoid torn updates to MSI pairs
In-Reply-To: <CAE=gft6fKQWExW-=xjZGzXs30XohfpA5SKggvL2WtYXAHmzMew@mail.gmail.com>
References: <20200117162444.v2.1.I9c7e72144ef639cc135ea33ef332852a6b33730f@changeid> <CACK8Z6Ft95qj4e_fsA32r_bcz2SsHOW1xxqZJt3_DBAJw=NMGA@mail.gmail.com> <CAE=gft6fKQWExW-=xjZGzXs30XohfpA5SKggvL2WtYXAHmzMew@mail.gmail.com>
Date:   Thu, 23 Jan 2020 09:49:13 +0100
Message-ID: <87y2tytv5i.fsf@nanos.tec.linutronix.de>
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Evan Green <evgreen@chromium.org> writes:
> In my experiments, the driver no longer misses the interrupt. XHCI is
> particularly sensitive to this, if it misses one interrupt it seems to
> completely wedge the driver.

That does not make the approach more correct.

> I think in my case the device pends the interrupts until MSIs are
> re-enabled, because I don't see anything other than MSI for xhci in
> /proc/interrupts. But I'm not sure if other devices may fall back to
> line-based interrupts for a moment, and if that's a problem.

Yes they can according to standard and it _IS_ a problem.

> Although, I already see we call pci_msi_set_enable(0) whenever we set
> up MSIs, presumably for this same reason of avoiding torn MSIs.

Please stop making random assumptions. This as absolutely nothing to do
with torn MSIs. The way how MSI setup works requires this. And this is
happening on init _before_ any interrupt can be requested on the device.
Different reason, different context.

> So my fix is really just doing the same thing for an additional
> case.

No, it's absolutely not the same. Your device is active and not in
reset/init state.

> And if getting stuck in a never-to-be-handled line based interrupt
> were a problem, you'd think it would also be a problem in
> pci_restore_msi_state(), where the same thing is done.

Again. I told you already it's not the same thing.

> Maybe my fix is at the wrong level, and should be up in
> pci_msi_domain_write_msg() instead? Though I see a lot of callers to
> pci_write_msi_msg() that I worry have the same problem.

This is not yet debugged fully and as this is happening on MSI-X I'm not
really convinced yet that your 'torn write' theory holds.

Thanks,

        tglx