Re: [Qemu-devel] [0.14?][PATCH 3/4] ioapic: Prepare for base address relocation

[Jan Kiszka]

On 2011-02-03 20:30, Blue Swirl wrote:
> On Thu, Feb 3, 2011 at 7:25 PM, Jan Kiszka <address@hidden> wrote:
>> On 2011-02-03 20:11, Blue Swirl wrote:
>>> On Thu, Feb 3, 2011 at 7:06 PM, Jan Kiszka <address@hidden> wrote:
>>>> On 2011-02-03 20:01, Blue Swirl wrote:
>>>>> On Thu, Feb 3, 2011 at 6:01 PM, Jan Kiszka <address@hidden> wrote:
>>>>>> On 2011-02-03 18:54, Blue Swirl wrote:
>>>>>>> On Thu, Feb 3, 2011 at 5:43 PM, Jan Kiszka <address@hidden> wrote:
>>>>>>>> On 2011-02-03 18:36, Blue Swirl wrote:
>>>>>>>>> On Thu, Feb 3, 2011 at 5:18 PM, Jan Kiszka <address@hidden> wrote:
>>>>>>>>>> On 2011-02-03 18:03, Blue Swirl wrote:
>>>>>>>>>>> On Thu, Feb 3, 2011 at 2:55 PM, Jan Kiszka <address@hidden> wrote:
>>>>>>>>>>>> The registers of real IOAPICs can be relocated during runtime (via
>>>>>>>>>>>> chipset registers). We don't support this yet, but qemu-kvm 
>>>>>>>>>>>> carries the
>>>>>>>>>>>> current base address in its version 2 vmstate.
>>>>>>>>>>>>
>>>>>>>>>>>> To align both implementations for migratability, add the proper
>>>>>>>>>>>> infrastructure to accept initial as well as updated base addresses 
>>>>>>>>>>>> and
>>>>>>>>>>>> include the current address in the vmstate. This is done in a way 
>>>>>>>>>>>> that
>>>>>>>>>>>> will also allow multiple IOAPICs in the future.
>>>>>>>>>>>
>>>>>>>>>>> Nack, the addresses should be device properties.
>>>>>>>>>>
>>>>>>>>>> Hmm.... we could make default_base_address a property. Will change 
>>>>>>>>>> that.
>>>>>>>>>> But current_base_address is just the same as apicbase and can't be a
>>>>>>>>>> property.
>>>>>>>>>
>>>>>>>>> Oh, right. What will current_base_address used for? Why can't board
>>>>>>>>> just unmap IOAPIC from current address and remap it at the new
>>>>>>>>> address? Then the device would not need to know its base address.
>>>>>>>>
>>>>>>>> The board could do this. The question is where we put this service, in
>>>>>>>> the context if the IOAPIC as ioapic_set_base_address (compare to
>>>>>>>> cpu_set_apic_base - which is buggy as it lacks sysbus_mmio_map) or into
>>>>>>>> each and every board code. In the latter case, the boards would also be
>>>>>>>> responsible for saving/restoring the address.
>>>>>>>
>>>>>>> How is the device relocated? Where are the chipset registers you 
>>>>>>> mention?
>>>>>>
>>>>>> Intel's PIIX chipsets contain a register called APICBASE (but it means
>>>>>> the IOAPIC), and that defines the location. The analogy in the APIC
>>>>>> world is the MSR_IA32_APICBASE which we maintain via the APIC state.
>>>>>
>>>>> In ICH10 the register is called OIC—Other Interrupt Control Register
>>>>> and the interesting bits APIC Range Select (ASEL).
>>>>>
>>>>> So actually PIIX should manage IOAPIC mapping, not board level.
>>>>
>>>> The point is we need ioapic_set_base_address logic in multiple places
>>>> (once chipsets start to implement it). Better push it to a central place
>>>> from the beginning. Also the bit keeping. There is no difference to
>>>> apicbase.
>>>
>>> In that case, the function should be made inline version in ioapic.h.
>>
>> That still replicates the bit keeping.
>>
>> I don't see the benefit of moving it over, even less when we want to
>> consolidate with a vmstate layout that is already in use.
> 
> The benefit is that the device model is improved.

I disagree about the benefit, but I will simply drop this patch and
instead add a dummy field to a vmstate version 3 so that qemu-kvm can
remove the base_address evaluation logic when updating.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux