Re: [Qemu-devel] [PATCH] hw/ppc: disable hotplug before CAS is completed

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3127 bytes --]

On Tue, Aug 15, 2017 at 05:28:46PM -0300, Daniel Henrique Barboza wrote:
> This patch is a follow up on the discussions that started with
> Laurent's patch series "spapr: disable hotplugging without OS" [1]
> and discussions made at patch "spapr: reset DRCs on migration
> pre_load" [2].
> 
> At this moment, we do not support CPU/memory hotplug in early
> boot stages, before CAS. The reason is that the hotplug event
> can't be handled at SLOF level (or even in PRELAUNCH runstate) and
> at the same time can't be canceled. This leads to devices being
> unable to be hot unplugged and, in some cases, guest kernel Ooops.
> After CAS, with the FDT in place, the guest can handle the hotplug
> events and everything works as usual.
> 
> An attempt to try to support hotplug before CAS was made, but not
> successful. The key difference in the current code flow between a
> coldplugged and a hotplugged device, in the PRELAUNCH state, is that
> the coldplugged device is registered at the base FDT, allowing its
> DRC to go straight to CONFIGURED state. In theory, this can also be
> done with a hotplugged device if we can add it to the base of the
> existing FDT. However, tampering with the FDT after writing in the
> guest memory, besides being a dubitable idea, is also not
> possible. The FDT is written in ppc_spapr_reset and there is no
> way to retrieve it - we can calculate the fdt_address but the
> fdt_size isn't stored. Storing the fdt_size to allow for
> later retrieval is yet another state that would need to be
> migrated. In short, it is not worth the trouble.
> 
> All this said, this patch opted to disable CPU/mem hotplug at early
> boot stages. CAS detection is made by checking if there are
> any bits set in ov5_cas to avoid adding an extra state that
> would need tracking/migration. The patch also makes sure that
> it doesn't interfere with hotplug in the INMIGRATE state.
> 
> [1] https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg05226.html
> [2] https://lists.gnu.org/archive/html/qemu-devel/2017-07/msg01989.html
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>

I don't think this is a good idea.

1) After my DRC cleanups, early hotplug works just fine for me.  I'm
not sure why it isn't for you: we need to understand that before
proceeding.

2) libvirt actually uses early hotplug fairly often (before even
starting the firmware).  At the moment this works - at least in some
cases (see above), though there are some wrinkles to work out.  This
will break it completely and require an entirely different approach to
fix again.

3) There's no fundamental reason early hotplug shouldn't work - the
event will just be queued until the OS boots and processes it.

I know I suggested disabling early hotplug earlier, but that was
before I'd dug into the DRC layer and properly understood what was
going on here.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]