From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58646) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TMJCc-0007X8-6B for qemu-devel@nongnu.org; Thu, 11 Oct 2012 09:53:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TMJCS-0000tJ-Gz for qemu-devel@nongnu.org; Thu, 11 Oct 2012 09:53:30 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49432) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TMJCS-0000tF-8I for qemu-devel@nongnu.org; Thu, 11 Oct 2012 09:53:20 -0400 Message-ID: <5076CF49.7080300@redhat.com> Date: Thu, 11 Oct 2012 15:53:13 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1349962023-560-1-git-send-email-avi@redhat.com> <1349962023-560-5-git-send-email-avi@redhat.com> In-Reply-To: <1349962023-560-5-git-send-email-avi@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC v1 4/7] pci: switch iommu to using the memory API List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Avi Kivity Cc: "Michael S. Tsirkin" , liu ping fan , qemu-devel@nongnu.org, Blue Swirl , Alex Williamson , Anthony Liguori Il 11/10/2012 15:27, Avi Kivity ha scritto: > -static int spapr_tce_translate(DMAContext *dma, > - dma_addr_t addr, > - target_phys_addr_t *paddr, > - target_phys_addr_t *len, > - DMADirection dir) > +IOMMUTLBEntry spapr_tce_translate(DMAContext *dma, target_phys_addr_t addr, > + bool is_write) > { > sPAPRTCETable *tcet = DO_UPCAST(sPAPRTCETable, dma, dma); > - enum sPAPRTCEAccess access = (dir == DMA_DIRECTION_FROM_DEVICE) > - ? SPAPR_TCE_WO : SPAPR_TCE_RO; > + enum sPAPRTCEAccess access = is_write ? SPAPR_TCE_WO : SPAPR_TCE_RO; > uint64_t tce; > > #ifdef DEBUG_TCE > @@ -84,29 +80,27 @@ static int spapr_tce_translate(DMAContext *dma, > #ifdef DEBUG_TCE > fprintf(stderr, "spapr_tce_translate out of bounds\n"); > #endif > - return -EFAULT; > + return (IOMMUTLBEntry) { .valid = false }; > } > > tce = tcet->table[addr >> SPAPR_TCE_PAGE_SHIFT].tce; > > /* Check TCE */ > if (!(tce & access)) { > - return -EPERM; > + return (IOMMUTLBEntry) { .valid = false }; > } > > - /* How much til end of page ? */ > - *len = ((~addr) & SPAPR_TCE_PAGE_MASK) + 1; > - > - /* Translate */ > - *paddr = (tce & ~SPAPR_TCE_PAGE_MASK) | > - (addr & SPAPR_TCE_PAGE_MASK); > - > #ifdef DEBUG_TCE > fprintf(stderr, " -> *paddr=0x" TARGET_FMT_plx ", *len=0x" > - TARGET_FMT_plx "\n", *paddr, *len); > + TARGET_FMT_plx "\n", (tce & ~SPAPR_TCE_PAGE_MASK), SPAPR_TCE_PAGE_MASK + 1); > #endif > > - return 0; > + return (IOMMUTLBEntry) { > + .device_addr = addr & SPAPR_TCE_PAGE_MASK, > + .translated_addr = (tce & ~SPAPR_TCE_PAGE_MASK), > + .addr_mask = SPAPR_TCE_PAGE_MASK, > + .valid = true, > + }; > } > > DMAContext *spapr_tce_new_dma_context(uint32_t liobn, size_t window_size) > @@ -118,7 +112,7 @@ DMAContext *spapr_tce_new_dma_context(uint32_t liobn, size_t window_size) > } > > tcet = g_malloc0(sizeof(*tcet)); > - dma_context_init(&tcet->dma, &address_space_memory, spapr_tce_translate, NULL, NULL); > + dma_context_init(&tcet->dma, &address_space_memory, NULL, NULL, NULL); > > tcet->liobn = liobn; > tcet->window_size = window_size; > @@ -253,7 +247,8 @@ int spapr_tcet_dma_dt(void *fdt, int node_off, const char *propname, > return 0; > } > > - if (iommu->translate == spapr_tce_translate) { > + /* FIXME: WHAT?? */ > + if (true) { > sPAPRTCETable *tcet = DO_UPCAST(sPAPRTCETable, dma, iommu); > return spapr_dma_dt(fdt, node_off, propname, > tcet->liobn, 0, tcet->window_size); > diff --git a/hw/spapr_pci.c b/hw/spapr_pci.c > index 661c05b..24f5b46 100644 > --- a/hw/spapr_pci.c > +++ b/hw/spapr_pci.c > @@ -506,14 +506,30 @@ static void spapr_msi_write(void *opaque, target_phys_addr_t addr, > /* > * PHB PCI device > */ > -static DMAContext *spapr_pci_dma_context_fn(PCIBus *bus, void *opaque, > - int devfn) > +static MemoryRegion *spapr_pci_dma_iommu_new(PCIBus *bus, void *opaque, int devfn) > { > sPAPRPHBState *phb = opaque; > > - return phb->dma; > + return &phb->iommu; > } > > +static void spapr_pci_dma_iommu_destroy(MemoryRegion *iommu) > +{ > + /* iommu is shared among devices, do nothing */ > +} > + > +static IOMMUTLBEntry spapr_phb_translate(MemoryRegion *iommu, target_phys_addr_t addr, > + bool is_write) > +{ > + sPAPRPHBState *phb = container_of(iommu, sPAPRPHBState, iommu); > + > + return spapr_tce_translate(phb->dma, addr, is_write); > +} IIUC, sPAPRTCETable could drop the DMAContext field and just include the actual translation info. spapr_tce_new_dma_context becomes spapr_tce_new and returns an opaque sPAPRTCETable struct that is passed back to spapr_tce_translate. And DMAContext can disappear and will be replaced with just an AddressSpace *. I like it! Paolo