Re: [PATCH] Support for in-kernel mmio handlers

[Chris Wright <chrisw-69jw2NvuJkxg9hUCZPvPmw@public.gmane.org>]

* Gregory Haskins (ghaskins-Et1tbQHTxzrQT0dZR+AlfA@public.gmane.org) wrote:
> LAPICs can be remapped on a per-cpu basis via an MSR, whereas something
> like an IOAPIC is a system-wide resource.

Yes, I see now, no vcpu in kvm_io_device callbacks' context (admittedly,
I'm used to the Xen implementation ;-)

> >> +struct kvm_io_device {
> >> +	unsigned long (*read)(struct kvm_io_device *this,
> >> +			      gpa_t addr,
> >> +			      unsigned long length);
> >> +	void (*write)(struct kvm_io_device *this,
> >> +		      gpa_t addr,
> >> +		      unsigned long length,
> >> +		      unsigned long val);
> >> +	int (*in_range)(struct kvm_io_device *this, gpa_t addr);
> >> +
> >> +	void             *private;
> > 
> > This looks unused, what is it meant for?
> 
> Its unused in this patch, because the primary consumer is a follow on
> patch that is not yet released.  The original patch had this logic + the
> logic that used it all together and it was requested to break them apart.

Makes sense, I'll wait to see a user to understand how it'w used.

> >> +++ b/drivers/kvm/kvm_main.c
> >> @@ - 294,6 +294,7 @@ static struct kvm *kvm_create_vm(void)
> >>  
> >>  	spin_lock_init(&kvm- >lock);
> >>  	INIT_LIST_HEAD(&kvm- >active_mmu_pages);
> >> +	kvm_io_bus_init(&kvm- >mmio_bus);
> > 
> > I'd just do INIT_LIST_HEAD, unless you havd bigger plans for this wrapper?
> 
> The motivation for wrapping the init is because I want to abstract
> the fact that its a list.   This means I can update the mechanism to
> do something more intelligent with address lookup (e.g. b-tree, etc)
> without changing code all over the place.  Right now there are only
> two consumers, put I envision there will be some more.  For instance,
> I would like to get PIOs using this mechanism at some point (so I can
> snarf accesses to the 8259s at 0x20/0xa0)

Right, you even alluded to that in your comments.  I didn't expect a list
to really become that long where it needed a more complex data structure.

thanks,
-chris

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV