Re: [Qemu-devel] [PATCH 28/35] kvm: x86: Introduce kvmclock device to save/restore its state

[Anthony Liguori]

On 01/20/2011 03:33 AM, Jan Kiszka wrote:
> On 2011-01-19 20:32, Blue Swirl wrote:
>    
> > On Wed, Jan 19, 2011 at 4:57 PM, Anthony Liguori
> > <address@hidden>  wrote:
> >      
> > > On 01/19/2011 07:15 AM, Markus Armbruster wrote:
> > >        
> > > > So they interact with KVM (need kvm_state), and they interact with the
> > > > emulated PCI bus.  Could you elaborate on the fundamental difference
> > > > between the two interactions that makes you choose the (hypothetical)
> > > > KVM bus over the PCI bus as device parent?
> > > >
> > > >          
> > > It's almost arbitrary, but I would say it's the direction that I/Os flow.
> > >
> > > But if the underlying observation is that the device tree is not really a
> > > tree, you're 100% correct.  This is part of why a factory interface that
> > > just takes a parent bus is too simplistic.
> > >
> > > I think we ought to introduce a -pci-device option that is specifically for
> > > creating PCI devices that doesn't require a parent bus argument but provides
> > > a way to specify stable addressing (for instancing, using a linear index).
> > >        
> > I think kvm_state should not be a property of any device or bus. It
> > should be split to more logical pieces.
> >
> > Some parts of it could remain in CPUState, because they are associated
> > with a VCPU.
> >
> > Also, for example irqfd could be considered to be similar object to
> > char or block devices provided by QEMU to devices. Would it make sense
> > to introduce new host types for passing parts of kvm_state to devices?
> >
> > I'd also make coalesced MMIO stuff part of memory object. We are not
> > passing any state references when using cpu_physical_memory_rw(), but
> > that could be changed.
> >      
> There are currently no VCPU-specific bits remaining in kvm_state. It may
> be a good idea to introduce an arch-specific kvm_state and move related
> bits over. It may also once be feasible to carve out memory management
> related fields if we have proper abstractions for that, but I'm not
> completely sure here.
>
> Anyway, all these things are secondary. The primary topic here is how to
> deal with kvm_state and its fields that have VM-global scope.
>    

The debate is really:

1) should we remove all passing of kvm_state and just assume it's static

2) deal with a couple places in the code where we need to figure out how 
to get at kvm_state

I think we've only identified 1 real instance of (2) and it's resulted 
in some good discussions about how to model KVM devices vs. emulated 
devices.  Honestly, (1) just stinks.  I see absolutely no advantage to 
it at all.   In the very worst case scenario, the thing we need to do is 
just reference an extern variable in a few places.  That completely 
avoids all of the modelling discussions for now (while leaving for 
placeholder FIXMEs so the problem can be tackled later).

I don't understand the resistance here.

Regards,

Anthony Liguori

> Jan
>
>