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

[Scott Wood]

On 03/21/2013 06:45:31 PM, Alexander Graf wrote:
> On 21.03.2013, at 22:59, Scott Wood wrote:
>
> > 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.

> We usually have settled on a tri-state way to change QEMU behavior  
> for most machine options:
>
>   -machine <opt> is not specified -> best possible behavior in the  
> current system
>   -machine <opt>=on -> turn the option on, fail if that doesn't work
>   -machine <opt>=off -> turn the option off always
>
> So for the in-kernel irqchip, we should follow the same model. If the  
> -machine option is not passed in, we should try to allocate an  
> in-kernel irqchip if possible.

That's fine, I just misunderstood what the semantics were supposed to  
be.

> If the kernel advertises that it can do an in-kernel irqchip, but in  
> fact it can't, I would consider that simply a bug we shouldn't worry  
> about.

I'm not worried about it.  I just don't see any benefit to deferring  
the creation of the device at that point, and we can't do the test any  
earlier because we won't know what type of irqchip to inquire about.

> >> > 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.
>
> No, currently that lives mostly in kvm-all.c.

By "currently" I mean in the current state of this patch, not how it's  
currently done for x86.  It's e500.c code that checks whether a "kvm  
openpic" was created.  If it wasn't -- due to the kernel not supporting  
it, or due to the user not wanting it -- it creates a normal openpic  
device.

This would change to error out if a kvm mpic was explicitly requested  
but unable to be created.

> I'm talking about the code that checks  
> qemu_opt_get_bool("kernel_irqchip") and decides what to do based on  
> that.

The only thing that it "decides what to do" for MPIC is record the  
preference for board code's consumption, and I agreed that there's no  
point leaving that there if it's more complicated than a simple bool.

-Scott