Re: [Qemu-ppc] [PULL 25/26] spapr_pci: factorize the use of SPAPR_MACHINE_GET_CLASS()

[Greg Kurz]

On Mon, 27 Aug 2018 11:03:39 +0200
Greg Kurz <address@hidden> wrote:

> On Mon, 27 Aug 2018 08:21:48 +0200
> Thomas Huth <address@hidden> wrote:
> 
> > On 2018-08-24 18:43, Cédric Le Goater wrote:  
> > > On 08/24/2018 05:38 PM, Greg Kurz wrote:    
> > >> On Fri, 24 Aug 2018 17:30:12 +0200
> > >> Cédric Le Goater <address@hidden> wrote:
> > >>    
> > >>> On 08/24/2018 05:09 PM, Peter Maydell wrote:    
> > >>>> On 21 August 2018 at 05:33, David Gibson <address@hidden> wrote:      
> > >>>>> From: Cédric Le Goater <address@hidden>    
> > [...]  
> > >>> Is there a way to specify which device type can or can not be 
> > >>> plugged on a machine ? 
> > >>>
> > >>> I suppose we cannot use : 
> > >>>
> > >>>         machine_class_allow_dynamic_sysbus_dev() 
> > >>>
> > >>> for cold plugged devices. Or can we ? That would be better.
> > >>>    
> > >>
> > >> Hmm... not sure this would help. The root problem is that many
> > >> places in spapr_pci and spapr_cpu_core assume the machine is
> > >> sPAPR.    
> > > 
> > > which is a perfectly legitimate assumption for a sPAPR only device,
> > > same for spapr_cpu_core. I would think. Shouldn't we enforce 
> > > the restriction at the machine level instead and not at the device
> > > level ? 
> > > 
> > > I thought that was the purpose of commit 0bd1909da606 ("machine: 
> > > Replace has_dynamic_sysbus with list of allowed devices"), to 
> > > make sure machines had a predefined list of user-creatable devices.    
> > 
> > The "spapr-pci-host-bridge" is explicitly marked with
> > "dc->user_creatable = true" - so it is creatable everywhere. You could
> > try whether it is possible to make it only creatable via the white list
> > instead  
> 
> Hmm... how would you do that ?
> 

The white list is checked in machine_init_notify() which gets called way after
spapr_phb_realize()... we can't rely on this to check the machine and the PHB
are compatible. Maybe add a dedicated bus for the PHBs in the spapr machine ?

> > ... not sure whether that works though, since there is a class
> > hierarchy (TYPE_PCI_HOST_BRIDGE) in between?
> >   
> 
> Also, as said above, we have the very same problem with spapr_cpu_core,
> which is definitely not a sysbus device...
> 
> Cheers,
> 
> --
> Greg
> 
> >  Thomas  
> 
>