On 10/11/2012 03:53 PM, Paolo Bonzini wrote: > 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.
Maybe, I just typed in code until it compiled, then forgot to copy Alex. I have no idea how to test it, so I kept the changes minimal. > > And DMAContext can disappear and will be replaced with just an > AddressSpace *. Indeed that's the plan. address_space_map() and dma_memory_map() simply duplicate each other. The only thing remaining is dma_memory_set(). > I like it! Me too, except when I'm coding it. -- error compiling committee.c: too many arguments to function