Re: [Qemu-devel] [RFC ppc-next PATCH 6/6] kvm/openpic: in-kernel mpic support

[Scott Wood]

On 03/21/2013 04:29:02 PM, Alexander Graf wrote:
> Am 21.03.2013 um 21:50 schrieb Scott Wood <address@hidden>:
>
> > On 03/21/2013 03:41:19 AM, Alexander Graf wrote:
> >> Can't all the stuff above here just simply go into the qdev init  
> function?
> >
> > Not if you want platform code to be able to fall back to a QEMU  
> mpic if an in-kernel mpic is unavailable.
>
> Do we want that? We used to have a default like that in qemu-kvm back  
> in the day. That was very confusing, as people started to report that  
> their VMs turned out to be really slow.
>
> I think we should not have fallback code. It makes things easier and  
> more obvious. The default should just depend on the host's  
> capabilities.

I don't follow.  What is the difference between "falling back" and  
"depending on the host's capabilities"?  Either we can create an  
in-kernel MPIC or we can't.  We could use KVM_CREATE_DEVICE_TEST to see  
if the device type is supported separately from actually creating it,  
but I don't see what that would accomplish other than adding more code.

> >> >     /* MPIC */
> >> >     mpic = g_new(qemu_irq, 256);
> >> > -    dev = qdev_create(NULL, "openpic");
> >> > -    qdev_prop_set_uint32(dev, "nb_cpus", smp_cpus);
> >> > -    qdev_prop_set_uint32(dev, "model", params->mpic_version);
> >> > +
> >> > +    if (kvm_irqchip_wanted()) {
> >> > +        dev = kvm_openpic_create(NULL, params->mpic_version);
> >> This really should be just a
> >>    dev = qdev_create(NULL, kvm_irqchip_wanted() ? "kvm-openpic" :  
> "openpic");
> >> The logic whether an in-kernel irqchip is available belongs into  
> the default setting of kvm_irqchip_wanted.
> >
> > That is exactly what I was trying to avoid by introducing  
> kvm_irqchip_wanted.  We're no longer testing some vague generic  
> irqchip capability, but the presence of a specific type of device  
> (and version thereof).  How would the code that sets  
> kvm_irqchip_wanted know what to test for?
>
> Then move the default code into the board file and check for the  
> in-kernel mpic cap.

I'm not quite sure what you mean by "the default code" -- if you mean  
the part that makes the decision whether to fall back or error out,  
that's already in board code.

-Scott