On Fri, Oct 14, 2016 at 08:40:49PM +0800, Peter Xu wrote: > QEMU edu device is a pci device that is originally written for > educational purpose, however it also suits for IOMMU unit test. Adding > helpers for this specific device to implement the device logic. > > The device supports lots of functions, here only DMA operation is > supported. > > The spec of the device can be found at: > > https://github.com/qemu/qemu/blob/master/docs/specs/edu.txt > > Signed-off-by: Peter Xu <peterx@xxxxxxxxxx> > --- > lib/pci-edu.c | 128 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > lib/pci-edu.h | 29 +++++++++++++ > 2 files changed, 157 insertions(+) > create mode 100644 lib/pci-edu.c > create mode 100644 lib/pci-edu.h > > diff --git a/lib/pci-edu.c b/lib/pci-edu.c > new file mode 100644 > index 0000000..4d1a5ab > --- /dev/null > +++ b/lib/pci-edu.c > @@ -0,0 +1,128 @@ > +/* > + * Edu PCI device. > + * > + * Copyright (C) 2016 Red Hat, Inc. > + * > + * Authors: > + * Peter Xu <peterx@xxxxxxxxxx>, > + * > + * This work is licensed under the terms of the GNU LGPL, version 2 or > + * later. > + */ > + > +#include "pci-edu.h" > + > +#define PCI_VENDOR_ID_QEMU (0x1234) > +#define PCI_DEVICE_ID_EDU (0x11e8) > + > +/* The only bar used by EDU device */ > +#define EDU_BAR_MEM (0) > +#define EDU_MAGIC (0xed) > +#define EDU_VERSION (0x100) > +#define EDU_DMA_BUF_SIZE (1 << 20) > +#define EDU_INPUT_BUF_SIZE (256) > + > +#define EDU_REG_ID (0x0) > +#define EDU_REG_ALIVE (0x4) > +#define EDU_REG_FACTORIAL (0x8) > +#define EDU_REG_STATUS (0x20) > +#define EDU_REG_DMA_SRC (0x80) > +#define EDU_REG_DMA_DST (0x88) > +#define EDU_REG_DMA_COUNT (0x90) > +#define EDU_REG_DMA_CMD (0x98) > + > +#define EDU_CMD_DMA_START (0x01) > +#define EDU_CMD_DMA_FROM (0x02) > +#define EDU_CMD_DMA_TO (0x00) > + > +#define EDU_STATUS_FACTORIAL (0x1) > +#define EDU_STATUS_INT_ENABLE (0x80) > + > +#define EDU_DMA_START (0x40000) > +#define EDU_DMA_SIZE_MAX (4096) shouldn't the above defines be in the header? > + > +uint64_t edu_reg_readq(pci_edu_dev_t *dev, int reg) > +{ > + return *(volatile uint64_t *)(dev->pci_dev.pci_bar[EDU_BAR_MEM] + reg); > +} > + > +uint32_t edu_reg_read(pci_edu_dev_t *dev, int reg) > +{ > + return *(volatile uint32_t *)(dev->pci_dev.pci_bar[EDU_BAR_MEM] + reg); > +} > + > +void edu_reg_writeq(pci_edu_dev_t *dev, int reg, uint64_t val) > +{ > + *(volatile uint64_t *)(dev->pci_dev.pci_bar[EDU_BAR_MEM] + reg) = val; > +} > + > +void edu_reg_write(pci_edu_dev_t *dev, int reg, uint32_t val) > +{ > + *(volatile uint32_t *)(dev->pci_dev.pci_bar[EDU_BAR_MEM] + reg) = val; > +} I'd put these accessors in the header as static inlines too > + > +/* Return true if alive */ > +bool edu_check_alive(pci_edu_dev_t *dev) > +{ > + static uint32_t live_count = 1; > + uint32_t value; > + > + edu_reg_write(dev, EDU_REG_ALIVE, live_count++); > + value = edu_reg_read(dev, EDU_REG_ALIVE); > + return (live_count - 1 == ~value); > +} Even edu_check_alive could be a static inline in the header, if it's something you'll do frequently. If it's only needed for a sanity init test then I'd just inline it directly in the one caller, edu_init > + > +int edu_init(pci_edu_dev_t *dev) > +{ > + int ret; > + > + ret = pci_dev_init(&dev->pci_dev, PCI_VENDOR_ID_QEMU, > + PCI_DEVICE_ID_EDU); > + if (ret) > + return ret; > + > + if (!edu_check_alive(dev)) { > + printf("edu device not alive!\n"); > + return -1; should this ever fail? Or would an assert by fine here? alive = edu_check_alive(dev) assert(alive); > + } > + > + return 0; > +} > + > +uint32_t edu_status(pci_edu_dev_t *dev) > +{ > + return edu_reg_read(dev, EDU_REG_STATUS); > +} please, no unnecessary wrappers > + > +void edu_dma(pci_edu_dev_t *dev, iova_t iova, > + size_t size, int dev_offset, pci_dma_dir_t dir) > +{ > + uint64_t from, to; > + uint32_t cmd = EDU_CMD_DMA_START; > + > + assert(size <= EDU_DMA_SIZE_MAX); > + assert(dev_offset < EDU_DMA_SIZE_MAX && > + dev_offset >= 0); > + > + printf("edu device DMA start %s addr %p size 0x%lu off 0x%x\n", > + dir == PCI_DMA_FROM_DEVICE ? "FROM" : "TO", > + (void *)iova, size, dev_offset); is pci_dma_dir_t just a binary enum? If so, then I find it sort of crufty. Can't we just have a 'bool write'? > + > + if (dir == PCI_DMA_FROM_DEVICE) { > + from = dev_offset + EDU_DMA_START; > + to = iova; > + cmd |= EDU_CMD_DMA_FROM; > + } else { > + from = iova; > + to = EDU_DMA_START + dev_offset; > + cmd |= EDU_CMD_DMA_TO; > + } > + > + edu_reg_writeq(dev, EDU_REG_DMA_SRC, from); > + edu_reg_writeq(dev, EDU_REG_DMA_DST, to); > + edu_reg_writeq(dev, EDU_REG_DMA_COUNT, size); > + edu_reg_write(dev, EDU_REG_DMA_CMD, cmd); > + > + /* Wait until DMA finished */ > + while (edu_reg_read(dev, EDU_REG_DMA_CMD) & EDU_CMD_DMA_START); You may consider adding a cpu_relax() to lib/x86/asm/barrier.h, like arm and ppc have. I noticed a big difference when running with tcg on how fast a 'while (state-not-changed);' loop can complete when adding that barrier. > +} > diff --git a/lib/pci-edu.h b/lib/pci-edu.h > new file mode 100644 > index 0000000..6b7dbfd > --- /dev/null > +++ b/lib/pci-edu.h > @@ -0,0 +1,29 @@ > +/* > + * Edu PCI device header. > + * > + * Copyright (C) 2016 Red Hat, Inc. > + * > + * Authors: > + * Peter Xu <peterx@xxxxxxxxxx>, > + * > + * This work is licensed under the terms of the GNU LGPL, version 2 or > + * later. > + * > + * Edu device is a virtualized device in QEMU. Please refer to > + * docs/specs/edu.txt in QEMU repository for EDU device manual. > + */ > +#ifndef __PCI_EDU_H__ > +#define __PCI_EDU_H__ > + > +#include "pci.h" > + > +struct pci_edu_dev { > + pci_dev pci_dev; > +}; > +typedef struct pci_edu_dev pci_edu_dev_t; Why wrap pci_dev in this pci_edu_dev struct? Will there ever be more state? Should we just wait and see? Also, why use a typedef for pci_dev (assuming we want it)? 'struct pci_dev' would be more kernel like. Thanks, drew > + > +int edu_init(pci_edu_dev_t *dev); > +void edu_dma(pci_edu_dev_t *dev, iova_t iova, > + size_t size, int dev_offset, pci_dma_dir_t dir); > + > +#endif > -- > 2.7.4 > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
