[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC v1 4/7] pci: switch iommu to using the memory API
From: |
Avi Kivity |
Subject: |
Re: [Qemu-devel] [RFC v1 4/7] pci: switch iommu to using the memory API |
Date: |
Thu, 11 Oct 2012 15:56:22 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120911 Thunderbird/15.0.1 |
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