Re: [Qemu-devel] [PATCH v5 00/16] uq/master: Introduce basic irqchip support

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 3611 bytes --]

On 2011-12-20 02:19, Jan Kiszka wrote:
> On 2011-12-20 02:08, Anthony Liguori wrote:
>> On 12/19/2011 06:37 PM, Jan Kiszka wrote:
>>> On 2011-12-20 01:32, Anthony Liguori wrote:
>>>> On 12/19/2011 05:49 PM, Jan Kiszka wrote:
>>>>> On 2011-12-19 23:24, Anthony Liguori wrote:
>>>>>> On 12/19/2011 03:17 PM, Marcelo Tosatti wrote:
>>>>>>>
>>>>>>> Anthony,
>>>>>>>
>>>>>>> Can you please review&    ACK?
>>>>>>>
>>>>>>> You could even apply directly but well do a kvm-autotest run through
>>>>>>> uq/master. Still, your review is needed.
>>>>>>
>>>>>> Overall, it looks good except for the backend/frontend split.  This
>>>>>> should be done in terms of qdev inheritance.
>>>>>
>>>>> I cannot follow your idea here yet. There is no inheritance as we
>>>>> end up
>>>>> with only a single class that permutes (selects a different backend) on
>>>>> creation. I'm not sure how to model two classes that will still only
>>>>> mean a single qdev registration.
>>>>
>>>> See other reply in thread.
>>>>
>>>> We should model this as two separate qdev devices.  We can avoid
>>>> regressing migration in qemu-kvm by just having a common vmstate name.
>>>>
>>>> apic is a no-user device so there's no way that changing the name of it
>>>> in qemu-kvm can affect users.
>>>
>>> Look down http://thread.gmane.org/gmane.comp.emulators.kvm.devel/82598
>>> for the discussion of that model.
>>
>> Let me say that I know this is the last bit of qemu-kvm that needs
>> merging and that this has been an epic effort.  I wouldn't refuse to
>> merge a pull request that came in with this in its current form.
>>
>> If we merged this now, I would be submitting patches in the not too
>> distant future to remove all of this backend stuff in favor of proper
>> modeling (including using two separate devices).
>>
>> There's lot of inconsistency in qdev already today so adding a little
>> more isn't the end of the world.  We're going to need to eventually have
>> this debate soon so it's up to you whether you want to just get this
>> merged now and worry about this another day or resolve this before merge.
>>
>> I don't see any compatibility issues here so I'm not really concerned
>> about introducing a regression by breaking it into two devices.
> 
> I don't want to see yet another attempt merged that requires foreseeable
> refactoring later on. The point of this one is to do it in a way that is
> providing a sound foundation for all those other features that still
> wait in qemu-kvm for refactoring.
> 
> The point is that migration support between in-kernel on/off is a
> worthwhile feature we should design for.

Forgot to state the why: This allows seamless migration from older,
non-accelerated setups and to switch between both models in case on
faces some issues. That not only applies to the APIC but to all those
various in-kernel device models we have and will add in the future.

> That either means skipping the
> backend property on device tree migration (maybe a feature we want in
> other use cases as well) or provide an alias naming scheme where you can
> address APICs, IOAPICs, i8259, i8254 and all the chips that non-x86 will
> bring us without knowing where they are implemented and without worrying
> to migrate between those variants. If you have a good model for that in
> mind, rolling back to v1, rebasing improvements from v5 over it would
> not be a big deal. But everyone in this round should agree on this
> first. I don't wanna port back and forth nor refactor all this again
> when once it's in.
> 
> Jan

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]