Re: [PATCH][QEMU] Use a separate device for in-kernel PIT

["Yang, Sheng" <sheng.yang@intel.com>]

On Thursday 13 March 2008 06:13:48 Anthony Liguori wrote:
> Dor Laor wrote:
> > On Wed, 2008-03-12 at 12:38 -0500, Anthony Liguori wrote:
> >> Part of the feedback we received from Fabrice about the KVM patches
> >> for QEMU
> >> is that we should create a separate device for the in-kernel APIC to
> >> avoid
> >> having lots of if (kvm_enabled()) within the APIC code that were
> >> difficult to
> >> understand why there were needed.
> >>
> >> This patch separates the in-kernel PIT into a separate device.  It
> >> also
> >> introduces some configure logic to only compile in support for the
> >> in-kernel
> >> PIT if it's available.
> >>
> >> The result of this is that we now only need a single if
> >> (kvm_enabled()) to
> >> determine which device to use.  Besides making it more upstream
> >> friendly, I
> >> think this makes the code much easier to understand.
> >
> > Seems like a good idea.
> >
> > [snip]
> > ..
> >
> >> +static void pit_reset(void *opaque)
> >> +{
> >> +    PITState *pit = opaque;
> >> +    PITChannelState *s;
> >> +    int i;
> >> +
> >> +    for(i = 0;i < 3; i++) {
> >> +        s = &pit->channels[i];
> >> +        s->mode = 3;
> >> +        s->gate = (i != 2);
> >> +        pit_load_count(s, 0);
> >> +    }
> >> +}
> >> +
> >
> > Seems like pit_reset won't change the in-kernel pit state.
>
> Yeah, that seemed suspicious to me too.  I didn't want to change the
> existing behavior though for fear of introducing regressions.  Perhaps
> Sheng can comment on whether it's necessary to even have a reset handler
> in userspace?

IMO the reset support is needed even it would introduce regression (e would 
finally solve the regression if exists, won't we? :) ).

And we got two choices in userspace: one ioctl to reset all kvm devices, or 
one ioctl for each device. For we are separating in kernel device into 
separate devices, seems the later is more proper. But would it bring other 
troubles like inconsistent state for smp? 

Thanks
Yang, Sheng

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/