From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eduard - Gabriel Munteanu Subject: Re: [Qemu-devel] [PATCH 2/7] pci: memory access API and IOMMU support Date: Thu, 2 Sep 2010 12:08:13 +0300 Message-ID: <20100902090812.GA7319@localhost> References: <1283119703-9781-1-git-send-email-eduard.munteanu@linux360.ro> <4C7EB336.40003@mail.berlios.de> <20100902060045.GA5410@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: 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: "Michael S. Tsirkin" Return-path: Received: from mail-fx0-f46.google.com ([209.85.161.46]:54002 "EHLO mail-fx0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752146Ab0IBJKA (ORCPT ); Thu, 2 Sep 2010 05:10:00 -0400 Received: by fxm13 with SMTP id 13so56858fxm.19 for ; Thu, 02 Sep 2010 02:09:59 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20100902060045.GA5410@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 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. Eduard > -- > MST From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=33633 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Or5o1-0005Rb-LG for qemu-devel@nongnu.org; Thu, 02 Sep 2010 05:10:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1Or5o0-0001qV-Ez for qemu-devel@nongnu.org; Thu, 02 Sep 2010 05:10:01 -0400 Received: from mail-fx0-f45.google.com ([209.85.161.45]:46562) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Or5o0-0001qR-7b for qemu-devel@nongnu.org; Thu, 02 Sep 2010 05:10:00 -0400 Received: by fxm7 with SMTP id 7so65034fxm.4 for ; Thu, 02 Sep 2010 02:09:59 -0700 (PDT) Sender: Eduard - Gabriel Munteanu Date: Thu, 2 Sep 2010 12:08:13 +0300 From: Eduard - Gabriel Munteanu Subject: Re: [Qemu-devel] [PATCH 2/7] pci: memory access API and IOMMU support Message-ID: <20100902090812.GA7319@localhost> References: <1283119703-9781-1-git-send-email-eduard.munteanu@linux360.ro> <4C7EB336.40003@mail.berlios.de> <20100902060045.GA5410@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20100902060045.GA5410@redhat.com> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: kvm@vger.kernel.org, yamahata@valinux.co.jp, joro@8bytes.org, qemu-devel@nongnu.org, blauwirbel@gmail.com, paul@codesourcery.com, avi@redhat.com 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. Eduard > -- > MST