From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH v3] IO: Intelligent device lookup on bus Date: Wed, 27 Jul 2011 15:37:30 +0300 Message-ID: <4E30068A.8010905@redhat.com> References: <1311488156-21998-1-git-send-email-levinsasha928@gmail.com> <4E2FF817.2090601@redhat.com> <1311768097.19123.11.camel@lappy> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, Marcelo Tosatti To: Sasha Levin Return-path: Received: from mx1.redhat.com ([209.132.183.28]:19547 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753078Ab1G0Mhe (ORCPT ); Wed, 27 Jul 2011 08:37:34 -0400 In-Reply-To: <1311768097.19123.11.camel@lappy> Sender: kvm-owner@vger.kernel.org List-ID: On 07/27/2011 03:01 PM, Sasha Levin wrote: > > > > > > -static inline struct kvm_pic *to_pic(struct kvm_io_device *dev) > > > +static inline struct kvm_pic *to_pic(struct kvm_io_device *dev, gpa_t addr) > > > { > > > - return container_of(dev, struct kvm_pic, dev); > > > + switch (addr) { > > > + case 0x20: > > > + case 0x21: > > > + return container_of(dev, struct kvm_pic, dev_master); > > > + case 0xa0: > > > + case 0xa1: > > > + return container_of(dev, struct kvm_pic, dev_slave); > > > + case 0x4d0: > > > + case 0x4d1: > > > + return container_of(dev, struct kvm_pic, dev_eclr); > > > + } > > > + > > > + return NULL; > > > } > > > > Somewhat ugly. I think > > > > int picdev_write_master(...) > > { > > return pcidev_write(container_of(...), ...); > > } > > > > is nicer, no? > > It would mean we need a total of 6 wrappers for master, slave and eclr > instead of this switch, if that sounds ok I'll change it. IMO, they're better than the switch. > > > > > > > if (ret< 0) > > > goto out_free_dev; > > > list_add_tail(&dev->list,&kvm->coalesced_zones); > > > diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c > > > index 73358d2..f59c1e8 100644 > > > --- a/virt/kvm/eventfd.c > > > +++ b/virt/kvm/eventfd.c > > > @@ -586,7 +586,8 @@ kvm_assign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args) > > > > > > kvm_iodevice_init(&p->dev,&ioeventfd_ops); > > > > > > - ret = kvm_io_bus_register_dev(kvm, bus_idx,&p->dev); > > > + ret = kvm_io_bus_register_dev(kvm, bus_idx, p->addr, p->length, > > > + &p->dev); > > > > Should this be p->length or 1? > > We register p->length since when we process a write, the operation > should be fully contained within the IO space of the device. > > We verify that the write happens on the first byte within ioeventfd > write handler. Ok. > > > > > #include > > > #include > > > @@ -2391,24 +2393,94 @@ static void kvm_io_bus_destroy(struct kvm_io_bus *bus) > > > int i; > > > > > > for (i = 0; i< bus->dev_count; i++) { > > > - struct kvm_io_device *pos = bus->devs[i]; > > > + struct kvm_io_device *pos = bus->range[i].dev; > > > > > > > This will call the destructor three times for the PIC. Is this safe? > > PIC doesn't have a destructor for devices, the code above just does > nothing for PIC devices. Ok. -- error compiling committee.c: too many arguments to function