Re: [PATCH 2/2][RFC] Kernel changes for HPET legacy mode (v7)

[Jan Kiszka <jan.kiszka@siemens.com>]

Beth Kon wrote:
> Avi Kivity wrote:
>> On 06/22/2009 12:14 PM, Jan Kiszka wrote:
>>>>> Hmm, stead of introducing a new pair of singe-purpose IOCTLs, why not
>>>>> add KVM_GET/SET_PIT2 which exchanges an extended kvm_pit_state2. And
>>>>> that struct should also include some flags field and enough padding to
>>>>> be potentially extended yet again in the future. In that case I see no
>>>>> problem having also a mode read-back interface.
>>>>>
>>>>>        
>>>> We'd only add kernel hpet if we were forced to (I imagine the same
>>>> applications/kernels that forced the PIT into the kernel will do the
>>>> same for HPET).
>>>>
>>>>      
>>>
>>> Answer and citation does not yet correlate for me.
>>>    
>>
>> Misquote.  I meant to reply to your 'Is it planned to add in-kernel
>> hpet support?' question.  Must be early in the morning in some timezone.
>>
>>> Could you comment more explicitly if your are fine with Beth's proposed
>>> interface, rather prefer something like my suggestion or even want
>>> something totally different?
>>>    
>>
>> GET/SET PIT2 looks like the best choice to me, at least until I find
>> whoever designed the HPET/PIT interdependency and make him take it back.
>>
> It seems to me that GET/SET PIT2 adds a good deal of complexity without
> any gain. PIT is not a very dynamic technology. GET/SET PIT works for
> PIT operational needs as defined by the PIT specifications. This whole
> problem existe because of the unfortunate requirement that hpet disable
> PIT interrupts, which is quite outside normal PIT operation. If I create
> a GET/SET PIT2, and a PITState2 that is a superset of PITState1, then I
> need to address all the cases where PITState is currently set/referenced
> and properly use PITState2/PITState, depending on whether the kernel
> supports PITState2. It just seems unnecessary, since hpet_legacy, and
> probably any other future "control" of the PIT will likely be outside of
> "normal" PIT operation.  I really think a separate ioctl that transfers
> just control information (of which one of the flag bits would be
> hpet_legacy) would be preferable and cleaner. Am I missing some
> advantage to PITState2? Or is there some simple way to implement this
> that I'm missing?

I think you just need to refactor out the processing of
kvm_pit_channel_state and call it from the legacy get/set handler as
well as from the new one. And the new handlers will also process the
additional fields that will come with kvm_pit_state2. Not really more
complex than the current proposal.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux