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

[Peter Xu <peterx@redhat.com>]

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 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,

-- 
Peter Xu