Re: [Qemu-devel] [PATCH] spapr-pci: enable adding PHB via -device

[Alexey Kardashevskiy <aik@ozlabs.ru>]

On 01/21/2014 02:27 AM, Markus Armbruster wrote:
> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
> 
>> Recent changes introduced cannot_instantiate_with_device_add_yet
>> and removed capability of adding yet another PCI host bridge via
>> command line for SPAPR platform (POWERPC64 server).
> 
> Specifically:
> 
> commit 837d37167dc446af8a91189108b363c04609e296
> Author: Markus Armbruster <armbru@redhat.com>
> Date:   Thu Nov 28 17:26:55 2013 +0100
> 
>     sysbus: Set cannot_instantiate_with_device_add_yet
>     
>     device_add plugs devices into suitable bus.  For "real" buses, that
>     actually connects the device.  For sysbus, the connections need to be
>     made separately, and device_add can't do that.  The device would be
>     left unconnected, and could not possibly work.
>     
>     Quite a few, but not all sysbus devices already set
>     cannot_instantiate_with_device_add_yet in their class init function.
>     
>     Set it in their abstract base's class init function
>     sysbus_device_class_init(), and remove the now redundant assignments
>     from device class init functions.
>     
>     Signed-off-by: Markus Armbruster <armbru@redhat.com>
>     Reviewed-by: Marcel Apfelbaum <marcel.a@redhat.com>
>     Signed-off-by: Andreas Färber <afaerber@suse.de>
> 
> Always good to point to specific commits in commit messages instead of
> hand-waving "recent changes".


My bad, I'll do this next time. Just lost myself in that patch series.


>> This brings the capability back and puts SPAPR PHB into "bridge"
>> category.
> 
> Look, a sysbus device that grabs the resources it needs from its init()
> callback instead of getting connected to them by the code that creates
> it!  I'm not sure that's proper, but if it works...  Maybe Andreas
> (cc'ed) can advise.

Sorry, I am not following you. SPAPR PHB allocates resources (memory
regions...) as (for example) E1000 ethernet device does.


>> This is not much use for emulated PHB but it is absolutely required
>> for VFIO as we put an IOMMU group onto a separate PHB on SPAPR.
>>
>> Cc: Markus Armbruster <armbru@redhat.com>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>>
>> Are -device and device_add considered synonims? SPAPR PHB can be added
>> via the command line just fine but cannot from device_add as
>> "Bus 'main-system-bus' does not support hotplugging".
> 
> -device is cold plug, device_add is hot plug.  device_add could be
> improved to do cold plug when used before the machine starts.


Soooo? We figured that out on IRC :) At the moment it is regression -
-device used to work for PHB and now it does not.

Alex Graf applied to his ppc-next, just to be clear - are you ack'ing or
nack'ing this patch?

In any cace, what do you think I should change in what I do in spapr_pci.c?
I most probably will, just need some directions.


-- 
Alexey