Re: [PATCH for-6.0 0/4] Don't treat all sysbus devices as hotpluggable

[Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>]

On 25/03/2021 15:33, Peter Maydell wrote:

> On the two machines which have the "platform bus" (ppc e500 and arm
> virt) we currently treat all TYPE_SYS_BUS_DEVICE devices as being
> hotpluggable in the device callbacks, and try to plug those devices
> into the platform bus.  This is far too broad, because only a handful
> of devices are actually valid to plug into the platform bus.
> Moreover, if a device which is pluggable for some other reason (like
> a PCI device) happens to use a sysbus device internally as part of
> its implementation, the hotplug callback will incorrectly grab that
> sysbus device, probably resulting in an assertion failure.
> 
> Mostly PCI devices don't use sysbus devices internally, so the only
> case we've encountered so far is the not-valid-anyway
>   qemu-system-ppc64 -M ppce500 -device macio-oldworld
> but we might create more in future.
> 
> This series restricts hotpluggability of sysbus devices on these
> platforms to those devices which are on the dynamic sysbus whitelist
> (which we were maintaining anyway).  With it, the above ppc
> commandline stops asserting and instead fails cleanly with
>    qemu-system-ppc64: Device heathrow is not supported by this machine yet.
> 
> Patch 1 is an API doc improvement while I was in the header file
> anyway.
> 
> thanks
> -- PMM
> 
> Peter Maydell (4):
>    include/hw/boards.h: Document machine_class_allow_dynamic_sysbus_dev()
>    machine: Provide a function to check the dynamic sysbus whitelist
>    hw/arm/virt: Only try to add valid dynamic sysbus devices to platform
>      bus
>    hw/ppc/e500plat: Only try to add valid dynamic sysbus devices to
>      platform bus
> 
>   include/hw/boards.h | 38 ++++++++++++++++++++++++++++++++++++++
>   hw/arm/virt.c       |  8 ++++++--
>   hw/core/machine.c   | 21 ++++++++++++++++-----
>   hw/ppc/e500plat.c   |  8 ++++++--
>   4 files changed, 66 insertions(+), 9 deletions(-)

Looks good to me having been poking around the code earlier today so:

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.