Re: [Qemu-devel] [PATCH v2 00/38] Delay destruction of memory regions to instance_finalize

[Michael S. Tsirkin]

On Tue, Sep 17, 2013 at 09:07:05PM +0200, Paolo Bonzini wrote:
> Il 17/09/2013 19:26, Michael S. Tsirkin ha scritto:
> > On Tue, Sep 17, 2013 at 07:16:15PM +0200, Paolo Bonzini wrote:
> >> Il 17/09/2013 19:07, Michael S. Tsirkin ha scritto:
> >>> After memory_region_del_subregion returns,
> >>> it's a promise that there will not be accesses
> >>> to the region.
> >>
> >> It's racy anyway.  You can have memory_region_del_subregion happen one
> >> clock cycle after the other (physical) CPU has done checked that there
> >> will not be accesses to the region.
> > 
> > Fix it then. Stick synchronize_rcu() in memory_region_del_subregion.
> 
> Easier said than done since you shouldn't do synchronize_rcu() with the
> BQL taken.
> 
> > Are you trying to convince me there's no way to have
> > synchronous APIs in presence of RCU?
> 
> No, I'm not.  But in the presence of coarse locks it's harder to use
> synchronous APIs.  Sticking to past QEMU experiences, the AIO
> synchronous cancellation API is a nightmare because the monitor cannot
> run while I/O is cancelled.
>
> >> A real bus has a "big PCI lock" (there can be only one transaction at a
> >> time), which is exactly what we want to get rid of.
> > 
> > Not really, in a bridged setup transactions on the
> > secondary bus are handled in parallel with transactions
> > on the primary bus.
> 
> That's not what you're suggesting though.  A device removal on the
> secondary bus,
> triggered by configuration space access on the primary
> bus, wouldn't complete the configuration space access on the primary bus
> until the removal on the secondary bus is complete.  So you still have a
> big lock around transactions on each bus.


I'm not really concerned about device removal.  We can work it out.

A much more interesting case is e.g. disabling memory.
E.g.

config write (disable memory)
read (flush out outstanding writes)
write <- must now have no effect


Or disabling bus master:

config write (disable bus master)
read (flush in outstanding writes)
 <- device must now not change memory


And these rules absolutely must be obeyed,
if you don't you'll break guests.


So I think we really should find a way to make the above work correctly.
Removal will follow almost automatically, since it disables memory and
mastering by itself.

Note: I'm talking about writes done by CPU here.
When writes are initiated by device and target MMIO, it's less
practical.  The only case we emulate that I'm familiar with is MSI
writes into APIC, and this MR is never disabled resized or moved.


> Would a PCI bridge behave any differently?

A real bridge? Yes what happens on the primary bus doesn't
affect the secondary bus.

> It's still too coarse for emulation purposes.

It's possible that we'll have to implement real
transaction ordering. If you want to remove BQL for MMIOs I'd
say that's almost a given?

> > Also, there are split transactions completed
> > asynchronously: guests that care about ordering
> > do flushes e.g. using memory reads.
> 
> Can you do split burst transactions?  If not, the granularity is still
> measured in bytes, not pages or more.
> 
> Paolo

I think we can but I'll have to check.

> > The logic is in the device/memory region really.
> > 
> >>> So I'm not even sure we really need to move destroy to finalize anymore 
> >>> ...
> >>
> >> We definitely need to move it.  Even if we added a flag that we set in
> >> memory_region_del_subregion, we need to check it later when AIO completes.
> >>
> >> Paolo
> > 
> > Flag where? region can become subregion of another
> > region entirely.
> > 
> > The solution for RCU is to flush on changes, not add
> > more flags and locks.
> >