From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH] Support for in-kernel mmio handlers Date: Sun, 08 Apr 2007 11:49:24 +0300 Message-ID: <4618AC94.3040700@qumranet.com> References: <4613C73F.BA47.005A.0@novell.com> <4614A03C.2050707@qumranet.com> <4614C844.BA47.005A.0@novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org To: Gregory Haskins Return-path: In-reply-to: <4614C844.BA47.005A.0-Et1tbQHTxzrQT0dZR+AlfA@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org Errors-To: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: kvm.vger.kernel.org This has significant changes, so merits a review. Gregory Haskins wrote: > diff --git a/drivers/kvm/kvm.h b/drivers/kvm/kvm.h > index fceeb84..c1923df 100644 > --- a/drivers/kvm/kvm.h > +++ b/drivers/kvm/kvm.h > @@ -236,6 +236,56 @@ struct kvm_pio_request { > int rep; > }; > > +struct kvm_io_device { > + unsigned long (*read)(struct kvm_io_device *this, > + gpa_t addr, > + int length); > + void (*write)(struct kvm_io_device *this, > + gpa_t addr, > + int length, > + unsigned long val); > + int (*in_range)(struct kvm_io_device *this, gpa_t addr); > + > + void *private; > +}; > + > +/* It would be nice to use something smarter than a linear search, TBD... > + * Thankfully we dont expect many devices to register (famous last words :), > + * so until then it will suffice. At least its abstracted so we can change > + * in one place. > + */ > /* * comment */ > +struct kvm_io_bus { > + int dev_count; > +#define NR_IOBUS_DEVS 6 > + struct kvm_io_device *devs[NR_IOBUS_DEVS]; > +}; > + > +static inline void kvm_io_bus_init(struct kvm_io_bus *bus) > +{ > + memset(bus, 0, sizeof(*bus)); > +} > + > +static inline struct kvm_io_device *kvm_io_bus_find_dev(struct kvm_io_bus *bus, gpa_t addr) > +{ > + int i; > + > + for(i=0; idev_count; i++) { > spaces, after for and around operators > + struct kvm_io_device *pos = bus->devs[i]; > + > + if (pos->in_range(pos, addr)) > + return pos; > + } > + > + return NULL; > +} > this is too long for an inline function. the others can also be made out-of-line. > + > +static inline void kvm_io_bus_register_dev(struct kvm_io_bus *bus, struct kvm_io_device *dev) > +{ > + BUG_ON(bus->dev_count >= (NR_IOBUS_DEVS-1)); > + > + bus->devs[bus->dev_count++] = dev; > +} > + > I actually meant an array of objects, not pointers, but in the interest of reducing the amount of churn we'll change it if and when we see a problem there. > > +static struct kvm_io_device* vcpu_find_mmio_dev(struct kvm_vcpu *vcpu, > + gpa_t addr) > "struct kvm_io_device *..." -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. ------------------------------------------------------------------------- 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