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 >