From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anthony Liguori Subject: Re: [Qemu-devel] [PATCH 2/7] pci: memory access API and IOMMU support Date: Thu, 02 Sep 2010 08:24:05 -0500 Message-ID: <4C7FA575.8000806@codemonkey.ws> References: <1283119703-9781-1-git-send-email-eduard.munteanu@linux360.ro> <4C7EB336.40003@mail.berlios.de> <20100902060045.GA5410@redhat.com> <20100902090812.GA7319@localhost> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: "Michael S. Tsirkin" , Stefan Weil , kvm@vger.kernel.org, joro@8bytes.org, qemu-devel@nongnu.org, blauwirbel@gmail.com, yamahata@valinux.co.jp, paul@codesourcery.com, avi@redhat.com To: Eduard - Gabriel Munteanu Return-path: Received: from mail-qy0-f174.google.com ([209.85.216.174]:44232 "EHLO mail-qy0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752156Ab0IBNYK (ORCPT ); Thu, 2 Sep 2010 09:24:10 -0400 Received: by qyk36 with SMTP id 36so1985655qyk.19 for ; Thu, 02 Sep 2010 06:24:09 -0700 (PDT) In-Reply-To: <20100902090812.GA7319@localhost> Sender: kvm-owner@vger.kernel.org List-ID: On 09/02/2010 04:08 AM, Eduard - Gabriel Munteanu wrote: > On Thu, Sep 02, 2010 at 09:00:46AM +0300, Michael S. Tsirkin wrote: > >> On Wed, Sep 01, 2010 at 10:10:30PM +0200, Stefan Weil wrote: >> >>>> +static inline void pci_memory_read(PCIDevice *dev, >>>> + pcibus_t addr, >>>> + uint8_t *buf, >>>> + pcibus_t len) >>>> +{ >>>> + pci_memory_rw(dev, addr, buf, len, 0); >>>> +} >>>> + >>>> +static inline void pci_memory_write(PCIDevice *dev, >>>> + pcibus_t addr, >>>> + const uint8_t *buf, >>>> + pcibus_t len) >>>> +{ >>>> + pci_memory_rw(dev, addr, (uint8_t *) buf, len, 1); >>>> +} >>>> + >>>> #endif >>>> >>> The functions pci_memory_read and pci_memory_write not only read >>> or write byte data but many different data types which leads to >>> a lot of type casts in your other patches. >>> >>> I'd prefer "void *buf" and "const void *buf" in the argument lists. >>> Then all those type casts could be removed. >>> >>> Regards >>> Stefan Weil >>> >> Further, I am not sure pcibus_t is a good type to use here. >> This also forces use of pci specific types in e.g. ide, or resorting to >> casts as this patch does. We probably should use a more generic type >> for this. >> > It only forces use of PCI-specific types in the IDE controller, which is > already a PCI device. > But IDE controllers are not always PCI devices... This isn't an issue with your patch, per-say, but with how we're modelling the IDE controller today. There's no great solution but I think your patch is an improvement over what we have today. I do agree with Stefan though that void * would make a lot more sense. Regards, Anthony Liguori > Eduard > > >> -- >> MST >> > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=47397 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Or9nl-0004re-VA for qemu-devel@nongnu.org; Thu, 02 Sep 2010 09:26:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1Or9ly-0006nY-HJ for qemu-devel@nongnu.org; Thu, 02 Sep 2010 09:24:11 -0400 Received: from mail-qw0-f45.google.com ([209.85.216.45]:41038) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Or9ly-0006nM-Cj for qemu-devel@nongnu.org; Thu, 02 Sep 2010 09:24:10 -0400 Received: by qwh5 with SMTP id 5so504750qwh.4 for ; Thu, 02 Sep 2010 06:24:09 -0700 (PDT) Message-ID: <4C7FA575.8000806@codemonkey.ws> Date: Thu, 02 Sep 2010 08:24:05 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 2/7] pci: memory access API and IOMMU support References: <1283119703-9781-1-git-send-email-eduard.munteanu@linux360.ro> <4C7EB336.40003@mail.berlios.de> <20100902060045.GA5410@redhat.com> <20100902090812.GA7319@localhost> In-Reply-To: <20100902090812.GA7319@localhost> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduard - Gabriel Munteanu Cc: kvm@vger.kernel.org, "Michael S. Tsirkin" , joro@8bytes.org, qemu-devel@nongnu.org, blauwirbel@gmail.com, paul@codesourcery.com, avi@redhat.com, yamahata@valinux.co.jp On 09/02/2010 04:08 AM, Eduard - Gabriel Munteanu wrote: > On Thu, Sep 02, 2010 at 09:00:46AM +0300, Michael S. Tsirkin wrote: > >> On Wed, Sep 01, 2010 at 10:10:30PM +0200, Stefan Weil wrote: >> >>>> +static inline void pci_memory_read(PCIDevice *dev, >>>> + pcibus_t addr, >>>> + uint8_t *buf, >>>> + pcibus_t len) >>>> +{ >>>> + pci_memory_rw(dev, addr, buf, len, 0); >>>> +} >>>> + >>>> +static inline void pci_memory_write(PCIDevice *dev, >>>> + pcibus_t addr, >>>> + const uint8_t *buf, >>>> + pcibus_t len) >>>> +{ >>>> + pci_memory_rw(dev, addr, (uint8_t *) buf, len, 1); >>>> +} >>>> + >>>> #endif >>>> >>> The functions pci_memory_read and pci_memory_write not only read >>> or write byte data but many different data types which leads to >>> a lot of type casts in your other patches. >>> >>> I'd prefer "void *buf" and "const void *buf" in the argument lists. >>> Then all those type casts could be removed. >>> >>> Regards >>> Stefan Weil >>> >> Further, I am not sure pcibus_t is a good type to use here. >> This also forces use of pci specific types in e.g. ide, or resorting to >> casts as this patch does. We probably should use a more generic type >> for this. >> > It only forces use of PCI-specific types in the IDE controller, which is > already a PCI device. > But IDE controllers are not always PCI devices... This isn't an issue with your patch, per-say, but with how we're modelling the IDE controller today. There's no great solution but I think your patch is an improvement over what we have today. I do agree with Stefan though that void * would make a lot more sense. Regards, Anthony Liguori > Eduard > > >> -- >> MST >> > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >