Re: [Qemu-devel] [PATCH v3 064/197] killall VIOsPAPRDeviceInfo

[Anthony Liguori]

On 12/12/2011 08:04 PM, Michael Ellerman wrote:
> On Mon, 2011-12-12 at 14:19 -0600, Anthony Liguori wrote:
> > This was doing something evil building a dt tree so we broke the device.
>
> > @@ -711,8 +711,12 @@ VIOsPAPRBus *spapr_vio_bus_init(void)
> >       spapr_rtas_register("ibm,set-tce-bypass", rtas_set_tce_bypass);
> >       spapr_rtas_register("quiesce", rtas_quiesce);
> >
> > +#if 0
> > +    /* Evil and broken */
>
> By which you mean: works fine, broken by your patch?

These patches were never supposed to go out.  Ignore this series entirely.

> > +
> >       for (qinfo = device_info_list; qinfo; qinfo = qinfo->next) {
> >           VIOsPAPRDeviceInfo *info = (VIOsPAPRDeviceInfo *)qinfo;
> > +        VIOsPAPRDeviceClass *pc = VIO_SPAPR_DEVICE_GET_CLASS(dev);
> >
> >           if (qinfo->bus_info !=&spapr_vio_bus_info) {
> >               continue;
> > @@ -722,6 +726,7 @@ VIOsPAPRBus *spapr_vio_bus_init(void)
> >               info->hcalls(bus);
> >           }
> >       }
> > +#endif
>
> It's registering hcalls for each class of device we find on the spapr
> vio bus. I don't understand why that is evil, but what do you suggest we
> do instead?

I talked to David about this, the hcalls can just be registered as part the 
device_init entry points.

If you must initialize them via a per-device callback, then you should walk it 
from the bus's children, not by walking the entire device model.  That was the 
bit that I was referring to as evil.   It's a layering violation.

Regards,

Anthony Liguori


> cheers