qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RFC PATCH 1/4] pci: memory access API and IOMMU suppo


From: Blue Swirl
Subject: Re: [Qemu-devel] [RFC PATCH 1/4] pci: memory access API and IOMMU support
Date: Thu, 5 Aug 2010 21:23:30 +0000

On Wed, Aug 4, 2010 at 10:32 PM, Eduard - Gabriel Munteanu
<address@hidden> wrote:
> PCI devices should access memory through pci_memory_*() instead of
> cpu_physical_memory_*(). This also provides support for translation and
> access checking in case an IOMMU is emulated.
>
> Memory maps are treated as remote IOTLBs (that is, translation caches
> belonging to the IOMMU-aware device itself). Clients (devices) must
> provide callbacks for map invalidation in case these maps are
> persistent beyond the current I/O context, e.g. AIO DMA transfers.
>
> Signed-off-by: Eduard - Gabriel Munteanu <address@hidden>
> ---
>  hw/pci.c      |  145 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/pci.h      |  130 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  qemu-common.h |    1 +
>  3 files changed, 276 insertions(+), 0 deletions(-)
>
> diff --git a/hw/pci.c b/hw/pci.c
> index 6871728..ce2734b 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -58,6 +58,10 @@ struct PCIBus {
>        Keep a count of the number of devices with raised IRQs.  */
>     int nirq;
>     int *irq_count;
> +
> +#ifdef CONFIG_PCI_IOMMU

The code should not be conditional.

> +    PCIIOMMU *iommu;
> +#endif
>  };
>
>  static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent);
> @@ -2029,6 +2033,147 @@ static void pcibus_dev_print(Monitor *mon, 
> DeviceState *dev, int indent)
>     }
>  }
>
> +#ifdef CONFIG_PCI_IOMMU
> +
> +void pci_register_iommu(PCIDevice *dev, PCIIOMMU *iommu)
> +{
> +    dev->bus->iommu = iommu;
> +}
> +
> +void pci_memory_rw(PCIDevice *dev,
> +                   pci_addr_t addr,
> +                   uint8_t *buf,
> +                   pci_addr_t len,
> +                   int is_write)
> +{
> +    int err, plen;
> +    unsigned perms;
> +    PCIIOMMU *iommu = dev->bus->iommu;
> +    target_phys_addr_t paddr;
> +
> +    if (!iommu || !iommu->translate)
> +        return cpu_physical_memory_rw(addr, buf, len, is_write);

Instead of these kind of checks, please add default handlers which
call cpu_physical_memory_rw() etc.

> +
> +    perms = is_write ? IOMMU_PERM_WRITE : IOMMU_PERM_READ;

Is this useful? How about just passing is_write as perms?

> +
> +    while (len) {
> +        err = iommu->translate(iommu, dev, addr, &paddr, &plen, perms);
> +        if (err)
> +            return;
> +
> +        /* The translation might be valid for larger regions. */
> +        if (plen > len)
> +            plen = len;
> +
> +        cpu_physical_memory_rw(paddr, buf, plen, is_write);
> +
> +        len -= plen;
> +        addr += plen;
> +        buf += plen;
> +    }
> +}
> +
> +void *pci_memory_map(PCIDevice *dev,
> +                     PCIInvalidateIOTLBFunc *cb,
> +                     void *opaque,
> +                     pci_addr_t addr,
> +                     target_phys_addr_t *len,
> +                     int is_write)
> +{
> +    int err, plen;
> +    unsigned perms;
> +    PCIIOMMU *iommu = dev->bus->iommu;
> +    target_phys_addr_t paddr;
> +
> +    if (!iommu || !iommu->translate)
> +        return cpu_physical_memory_map(addr, len, is_write);
> +
> +    perms = is_write ? IOMMU_PERM_WRITE : IOMMU_PERM_READ;
> +
> +    plen = *len;
> +    err = iommu->translate(iommu, dev, addr, &paddr, &plen, perms);
> +    if (err)
> +        return NULL;
> +
> +    /*
> +     * If this is true, the virtual region is contiguous,
> +     * but the translated physical region isn't. We just
> +     * clamp *len, much like cpu_physical_memory_map() does.
> +     */
> +    if (plen < *len)
> +        *len = plen;
> +
> +    /* We treat maps as remote TLBs to cope with stuff like AIO. */
> +    if (cb && iommu->register_iotlb_invalidator)
> +        iommu->register_iotlb_invalidator(iommu, dev, addr, cb, opaque);
> +
> +    return cpu_physical_memory_map(paddr, len, is_write);
> +}
> +
> +void pci_memory_unmap(PCIDevice *dev,
> +                      void *buffer,
> +                      target_phys_addr_t len,
> +                      int is_write,
> +                      target_phys_addr_t access_len)
> +{
> +    cpu_physical_memory_unmap(buffer, len, is_write, access_len);
> +}
> +
> +#define DEFINE_PCI_LD(suffix, size)                                       \
> +uint##size##_t pci_ld##suffix(PCIDevice *dev, pci_addr_t addr)            \
> +{                                                                         \
> +    PCIIOMMU *iommu = dev->bus->iommu;                                    \
> +    target_phys_addr_t paddr;                                             \
> +    int plen, err;                                                        \
> +                                                                          \
> +    if (!iommu || !iommu->translate)                                      \
> +        return ld##suffix##_phys(addr);                                   \
> +                                                                          \
> +    err = iommu->translate(iommu, dev,                                    \
> +                           addr, &paddr, &plen, IOMMU_PERM_READ);         \
> +    if (err || (plen < size / 8))                                         \
> +        return 0;                                                         \
> +                                                                          \
> +    return ld##suffix##_phys(paddr);                                      \
> +}
> +
> +#define DEFINE_PCI_ST(suffix, size)                                       \
> +void pci_st##suffix(PCIDevice *dev, pci_addr_t addr, uint##size##_t val)  \
> +{                                                                         \
> +    PCIIOMMU *iommu = dev->bus->iommu;                                    \
> +    target_phys_addr_t paddr;                                             \
> +    int plen, err;                                                        \
> +                                                                          \
> +    if (!iommu || !iommu->translate) {                                    \
> +        st##suffix##_phys(addr, val);                                     \
> +        return;                                                           \
> +    }                                                                     \
> +                                                                          \
> +    err = iommu->translate(iommu, dev,                                    \
> +                           addr, &paddr, &plen, IOMMU_PERM_WRITE);        \
> +    if (err || (plen < size / 8))                                         \
> +        return;                                                           \
> +                                                                          \
> +    st##suffix##_phys(paddr, val);                                        \
> +}
> +
> +#else /* !defined(CONFIG_PCI_IOMMU) */
> +
> +#define DEFINE_PCI_LD(suffix, size)
> +#define DEFINE_PCI_ST(suffix, size)
> +
> +#endif /* CONFIG_PCI_IOMMU */
> +
> +DEFINE_PCI_LD(ub, 8)
> +DEFINE_PCI_LD(uw, 16)
> +DEFINE_PCI_LD(l, 32)
> +DEFINE_PCI_LD(q, 64)
> +
> +DEFINE_PCI_ST(b, 8)
> +DEFINE_PCI_ST(w, 16)
> +DEFINE_PCI_ST(l, 32)
> +DEFINE_PCI_ST(q, 64)
> +
>  static PCIDeviceInfo bridge_info = {
>     .qdev.name    = "pci-bridge",
>     .qdev.size    = sizeof(PCIBridge),
> diff --git a/hw/pci.h b/hw/pci.h
> index 4bd8a1a..bd8c21b 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -430,4 +430,134 @@ static inline int ranges_overlap(uint64_t first1, 
> uint64_t len1,
>     return !(last2 < first1 || last1 < first2);
>  }
>
> +/*
> + * Memory I/O and PCI IOMMU definitions.
> + */
> +
> +typedef target_phys_addr_t pci_addr_t;

There is already pcibus_t.

> +
> +typedef int PCIInvalidateIOTLBFunc(void *opaque);

I think some type safety tricks could be used with for example PCIDevice *.

> +
> +#ifndef CONFIG_PCI_IOMMU
> +
> +static inline void pci_memory_rw(PCIDevice *dev,
> +                                 pci_addr_t addr,
> +                                 uint8_t *buf,
> +                                 pci_addr_t len,
> +                                 int is_write)
> +{
> +    cpu_physical_memory_rw(addr, buf, len, is_write);
> +}
> +
> +static inline void *pci_memory_map(PCIDevice *dev,
> +                                   PCIInvalidateIOTLBFunc *cb,
> +                                   void *opaque,
> +                                   pci_addr_t addr,
> +                                   target_phys_addr_t *len,
> +                                   int is_write)
> +{
> +    return cpu_physical_memory_map(addr, plen, is_write);
> +}
> +
> +static inline void pci_memory_unmap(PCIDevice *dev,
> +                                    void *buffer,
> +                                    target_phys_addr_t len,
> +                                    int is_write,
> +                                    target_phys_addr_t access_len)
> +{
> +    cpu_physical_memory_unmap(buffer, len, is_write, access_len);
> +}
> +
> +#define DECLARE_PCI_LD(suffix, size)                                    \
> +static inline uint##size##_t pci_ld##suffix(PCIDevice *dev,             \
> +                                            pci_addr_t addr)            \
> +{                                                                       \
> +    return ld##suffix##_phys(addr);                                     \
> +}
> +
> +#define DECLARE_PCI_ST(suffix, size)                                    \
> +static inline void pci_st##suffix(PCIDevice *dev,                       \
> +                                  pci_addr_t addr,                      \
> +                                  uint##size##_t val)                   \
> +{                                                                       \
> +    st##suffix##_phys(addr, val);                                       \
> +}
> +
> +#else /* defined(CONFIG_PCI_IOMMU) */
> +
> +struct PCIIOMMU {
> +    void *opaque;
> +
> +    void (*register_iotlb_invalidator)(PCIIOMMU *iommu,
> +                                       PCIDevice *dev,
> +                                       pci_addr_t addr,
> +                                       PCIInvalidateIOTLBFunc *cb,
> +                                       void *opaque);
> +    int (*translate)(PCIIOMMU *iommu,
> +                     PCIDevice *dev,
> +                     pci_addr_t addr,
> +                     target_phys_addr_t *paddr,
> +                     int *len,
> +                     unsigned perms);
> +};
> +
> +#define IOMMU_PERM_READ     (1 << 0)
> +#define IOMMU_PERM_WRITE    (1 << 1)
> +#define IOMMU_PERM_RW       (IOMMU_PERM_READ | IOMMU_PERM_WRITE)
> +
> +extern void pci_memory_rw(PCIDevice *dev,
> +                          pci_addr_t addr,
> +                          uint8_t *buf,
> +                          pci_addr_t len,
> +                          int is_write);
> +extern void *pci_memory_map(PCIDevice *dev,
> +                            PCIInvalidateIOTLBFunc *cb,
> +                            void *opaque,
> +                            pci_addr_t addr,
> +                            target_phys_addr_t *len,
> +                            int is_write);
> +extern void pci_memory_unmap(PCIDevice *dev,
> +                             void *buffer,
> +                             target_phys_addr_t len,
> +                             int is_write,
> +                             target_phys_addr_t access_len);
> +extern void pci_register_iommu(PCIDevice *dev,
> +                               PCIIOMMU *iommu);
> +
> +#define DECLARE_PCI_LD(suffix, size)                                    \
> +extern uint##size##_t pci_ld##suffix(PCIDevice *dev,  pci_addr_t addr);
> +
> +#define DECLARE_PCI_ST(suffix, size)                                    \
> +extern void pci_st##suffix(PCIDevice *dev,                              \
> +                           pci_addr_t addr,                             \
> +                           uint##size##_t val);
> +
> +#endif /* CONFIG_PCI_IOMMU */
> +
> +static inline void pci_memory_read(PCIDevice *dev,
> +                                   pci_addr_t addr,
> +                                   uint8_t *buf,
> +                                   pci_addr_t len)
> +{
> +    pci_memory_rw(dev, addr, buf, len, 0);
> +}
> +
> +static inline void pci_memory_write(PCIDevice *dev,
> +                                    pci_addr_t addr,
> +                                    const uint8_t *buf,
> +                                    pci_addr_t len)
> +{
> +    pci_memory_rw(dev, addr, (uint8_t *) buf, len, 1);
> +}
> +
> +DECLARE_PCI_LD(ub, 8)
> +DECLARE_PCI_LD(uw, 16)
> +DECLARE_PCI_LD(l, 32)
> +DECLARE_PCI_LD(q, 64)
> +
> +DECLARE_PCI_ST(b, 8)
> +DECLARE_PCI_ST(w, 16)
> +DECLARE_PCI_ST(l, 32)
> +DECLARE_PCI_ST(q, 64)
> +
>  #endif
> diff --git a/qemu-common.h b/qemu-common.h
> index 3fb2f0b..8daf962 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -219,6 +219,7 @@ typedef struct PCIHostState PCIHostState;
>  typedef struct PCIExpressHost PCIExpressHost;
>  typedef struct PCIBus PCIBus;
>  typedef struct PCIDevice PCIDevice;
> +typedef struct PCIIOMMU PCIIOMMU;
>  typedef struct SerialState SerialState;
>  typedef struct IRQState *qemu_irq;
>  typedef struct PCMCIACardState PCMCIACardState;
> --
> 1.7.1
>
>
>

reply via email to

[Prev in Thread] Current Thread [Next in Thread]