[Qemu-devel] Re: [PATCH 0/6] Make hpet a compile time option

[Blue Swirl]

On Mon, May 24, 2010 at 6:03 PM, Anthony Liguori <address@hidden> wrote:
> On 05/24/2010 12:54 PM, Juan Quintela wrote:
>>
>> Paul Brook<address@hidden>  wrote:
>>
>>>>
>>>> On 05/24/2010 11:32 AM, Paul Brook wrote:
>>>>
>>>>>>
>>>>>> Notice that this patch was sent against hpet as one example, if we
>>>>>> agree
>>>>>> that this "way" of disabling devices is ok, we could disable more
>>>>>> devices/have more flexibility.  Notice that in general, we (RHEL/KVM)
>>>>>> are interested in a small subset of qemu devices.
>>>>>>
>>>>>
>>>>> IMO this patch is a backwards step.  The device models should be
>>>>> cleaned
>>>>> up so that you don't need to make a compile time decision.
>>>>>
>>>>
>>>> I disagree.  I think the device model should be cleaned up so that no
>>>> CONFIG_HPET is required in code but I think it's still useful to be able
>>>> to exclude device models from the build.  That should just be a matter
>>>> of not building the object though (that's the point of device_init()).
>>>>
>>>
>>> I think we're saying the same thing.
>>>
>>> We already have a mechanism for avoiding things at build time -
>>> specifically
>>> config-devices.mak. We don't have a nice UI for it, but it's there.
>>> At worst your distro specific patch is a 1-line change to default-
>>> configs/i386-softmmu.mak.
>>>
>>> I have no objection to moving hpet.c into Makefile.objs, conditional on
>>> CONFIG_HPET (like e.g. CONFIG_SERIAL/serial.o).  However a necessary
>>> prerequisite is that you fix the device model and machine initialisation
>>> so
>>> that it's possible to omit hpet.o without rebuilding anything else.
>>>
>>
>> We have two exported functions:
>>
>> void hpet_init(qemu_irq *irq);
>> uint32_t hpet_in_legacy_mode(void);
>>
>> This is how one is used in mc14818rtc:
>>
>> #if defined TARGET_I386
>>     if (!hpet_in_legacy_mode())
>> #endif
>>
>
> In real hardware, and HPET would normally emulate an RTC.  The interaction
> problem here is that we aren't modelling that correctly in qemu as we're
> treating the rtc as a separate device.
>
> What could probably work at a hand wave level, is to make the rtc init
> function take a qemu_irq instead of directly grabbing the isa irq.  When an
> HPET is in use, the rtc no longer is directly initiated but instead is
> indirectly initiated by the HPET passing a special qemu_irq to the device
> that masks the actual interrupt line when legacy mode isn't enabled.  When
> the HPET isn't in use, the rtc would be created with an isa allocated
> qemu_irq.

Fully agree, HPET logic should be moved into HPET. I think my
yesterday's patches only went half way.