Re: [PATCH v2 1/1] pcie: Add hotplug detect state register to cmask

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, Jul 06, 2023 at 03:02:07PM -0400, Peter Xu wrote:
> On Thu, Jul 06, 2023 at 02:50:20PM -0400, Michael S. Tsirkin wrote:
> > On Thu, Jul 06, 2023 at 02:14:37PM -0400, Peter Xu wrote:
> > > On Thu, Jul 06, 2023 at 03:07:40PM -0300, Leonardo Bras Soares Passos wrote:
> > > > > I asked the same question, and I still keep confused: whether there's a
> > > > > first bad commit?  Starting from when it fails?
> > > > >
> > > > > For example, is this broken on 6.0 binaries too with pc-q35-6.0?
> > > > 
> > > > I tested for qemu 6.0, and it still reproduces, but have not pursued
> > > > this any further.
> > > 
> > > I see, thanks!
> > > 
> > > But then do you know why it's never hit before?  I assume it means this bug
> > > has been there for a long time.
> > 
> > It's a race - you have to migrate after the bit has been set before
> > the bit got cleared.
> > cmask is exactly for bits that qemu modifies itself.
> 
> Michael, do you mean that Leo's patch is wrong?


I mean his patch is exactly right. cmask was designed with this
kind of use case in mind.
Will queue.

> I just got understood why it got cleared - I think Leo didn't mention that
> the device was actually offlined before migration, IIUC that's why the PDS
> bit got cleared, if PDS was trying to describe that of the slot.
> 
> According to:
> 
>     /* Used to enable checks on load. Note that writable bits are
>      * never checked even if set in cmask. */
>     uint8_t *cmask;
> 
> It does sound reasonable to me to have PDS cleared when device offlined.
> Since hypervisor doesn't really know what the condition the slot presence
> bit would be when migrating, it seems we should just clear the bit in
> cmask.
> 
> So with the last reply from Leo, the patch looks all right to me.  It's
> just that as Leo mentioned, we should mention the offline process if that's
> the case, because that's definitely an important step to reproduce the issue.
> 
> Thanks,

If you want to suggest more text to the commit log, for the benefit
of backporters, that is fine by me.

> -- 
> Peter Xu