On Fri, Oct 14, 2016 at 08:40:52PM +0800, Peter Xu wrote:
> First of all, vtd_setup_msi() is provided. It setup IRTE entries,
> meanwhile, setup PCI device MSI vectors corresponding to VT-d spec.
>
> The basic IR test is carried out by a edu factorial request. When write
> to the factorial register, calculation starts in the background of
> device, MSI is triggered as long as it finishes.
>
> Signed-off-by: Peter Xu <peterx@xxxxxxxxxx>
> ---
> lib/pci-edu.c | 18 ++++++++--
> lib/pci-edu.h | 7 ++++
> lib/x86/intel-iommu.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++
> lib/x86/intel-iommu.h | 1 +
> x86/intel-iommu.c | 45 +++++++++++++++++++++++++
> 5 files changed, 161 insertions(+), 3 deletions(-)
>
> diff --git a/lib/pci-edu.c b/lib/pci-edu.c
> index 4d1a5ab..9a90a63 100644
> --- a/lib/pci-edu.c
> +++ b/lib/pci-edu.c
> @@ -35,9 +35,6 @@
> #define EDU_CMD_DMA_FROM (0x02)
> #define EDU_CMD_DMA_TO (0x00)
>
> -#define EDU_STATUS_FACTORIAL (0x1)
> -#define EDU_STATUS_INT_ENABLE (0x80)
> -
I think these and all EDU_ defines should have been in
pci-edu.h in the first place.
> #define EDU_DMA_START (0x40000)
> #define EDU_DMA_SIZE_MAX (4096)
>
> @@ -94,6 +91,21 @@ uint32_t edu_status(pci_edu_dev_t *dev)
> return edu_reg_read(dev, EDU_REG_STATUS);
> }
>
> +void edu_setup_intr(pci_edu_dev_t *dev, bool enabled)
> +{
> + edu_reg_write(dev, EDU_REG_STATUS, EDU_STATUS_INT_ENABLE);
> +}
> +
> +void edu_fact_write(pci_edu_dev_t *dev, uint32_t in)
> +{
> + edu_reg_write(dev, EDU_REG_FACTORIAL, in);
> +}
> +
> +uint32_t edu_fact_read(pci_edu_dev_t *dev)
> +{
> + return edu_reg_read(dev, EDU_REG_FACTORIAL);
> +}
More wrappers...
> +
> void edu_dma(pci_edu_dev_t *dev, iova_t iova,
> size_t size, int dev_offset, pci_dma_dir_t dir)
> {
> diff --git a/lib/pci-edu.h b/lib/pci-edu.h
> index 6b7dbfd..dcf5b76 100644
> --- a/lib/pci-edu.h
> +++ b/lib/pci-edu.h
> @@ -22,8 +22,15 @@ struct pci_edu_dev {
> };
> typedef struct pci_edu_dev pci_edu_dev_t;
>
> +#define EDU_STATUS_FACTORIAL (0x1)
> +#define EDU_STATUS_INT_ENABLE (0x80)
> +
> 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);
> +void edu_setup_intr(pci_edu_dev_t *dev, bool enabled);
> +void edu_fact_write(pci_edu_dev_t *dev, uint32_t in);
> +uint32_t edu_fact_read(pci_edu_dev_t *dev);
> +uint32_t edu_status(pci_edu_dev_t *dev);
>
> #endif
> diff --git a/lib/x86/intel-iommu.c b/lib/x86/intel-iommu.c
> index 30e2c97..de6c732 100644
> --- a/lib/x86/intel-iommu.c
> +++ b/lib/x86/intel-iommu.c
> @@ -12,6 +12,7 @@
>
> #include "intel-iommu.h"
> #include "asm-generic/page.h"
> +#include "pci.h"
>
> /*
> * VT-d in QEMU currently only support 39 bits address width, which is
> @@ -48,6 +49,26 @@ struct vtd_context_entry {
> } __attribute__ ((packed));
> typedef struct vtd_context_entry vtd_ce_t;
>
> +struct vtd_irte {
> + uint32_t present:1;
> + uint32_t fault_disable:1; /* Fault Processing Disable */
> + uint32_t dest_mode:1; /* Destination Mode */
> + uint32_t redir_hint:1; /* Redirection Hint */
> + uint32_t trigger_mode:1; /* Trigger Mode */
> + uint32_t delivery_mode:3; /* Delivery Mode */
> + uint32_t __avail:4; /* Available spaces for software */
> + uint32_t __reserved_0:3; /* Reserved 0 */
> + uint32_t irte_mode:1; /* IRTE Mode */
> + uint32_t vector:8; /* Interrupt Vector */
> + uint32_t __reserved_1:8; /* Reserved 1 */
> + uint32_t dest_id; /* Destination ID */
> + uint16_t source_id:16; /* Source-ID */
> + uint64_t sid_q:2; /* Source-ID Qualifier */
> + uint64_t sid_vtype:2; /* Source-ID Validation Type */
> + uint64_t __reserved_2:44; /* Reserved 2 */
> +} __attribute__ ((packed));
> +typedef struct vtd_irte vtd_irte_t;
> +
> void vtd_reg_write_4(unsigned int reg, uint32_t value)
> {
> *(uint32_t *)(Q35_HOST_BRIDGE_IOMMU_ADDR + reg) = value;
> @@ -255,6 +276,78 @@ void vtd_map_range(uint16_t sid, iova_t iova, phys_addr_t pa, size_t size)
> }
> }
>
> +static uint16_t vtd_intr_index_alloc(void)
> +{
> + static int index_ctr = 0;
> + assert(index_ctr < 65535);
> + return index_ctr++;
> +}
> +
> +static void vtd_setup_irte(pci_dev *dev, vtd_irte_t *irte,
> + int vector, int dest_id)
> +{
> + assert(sizeof(vtd_irte_t) == 16);
> + memset(irte, 0, sizeof(*irte));
> + irte->fault_disable = 1;
> + irte->dest_mode = 0; /* physical */
> + irte->trigger_mode = 0; /* edge */
> + irte->delivery_mode = 0; /* fixed */
> + irte->irte_mode = 0; /* remapped */
> + irte->vector = vector;
> + irte->dest_id = dest_id;
> + irte->source_id = dev->pci_addr;
> + irte->sid_q = 0;
> + irte->sid_vtype = 1; /* full-sid verify */
> + irte->present = 1;
> +}
> +
> +struct vtd_msi_addr {
> + uint32_t __dont_care:2;
> + uint32_t handle_15:1; /* handle[15] */
> + uint32_t shv:1;
> + uint32_t interrupt_format:1;
> + uint32_t handle_0_14:15; /* handle[0:14] */
> + uint32_t head:12; /* 0xfee */
> + uint32_t addr_hi; /* not used except with x2apic */
> +} __attribute__ ((packed));
> +typedef struct vtd_msi_addr vtd_msi_addr_t;
> +
> +struct vtd_msi_data {
> + uint16_t __reserved;
> + uint16_t subhandle;
> +} __attribute__ ((packed));
> +typedef struct vtd_msi_data vtd_msi_data_t;
> +
> +/**
> + * vtd_setup_msi - setup MSI message for a device
> + *
> + * @dev: PCI device to setup MSI
> + * @vector: interrupt vector
> + * @dest_id: destination processor
> + */
> +void vtd_setup_msi(pci_dev *dev, int vector, int dest_id)
> +{
> + vtd_msi_data_t msi_data = {};
> + vtd_msi_addr_t msi_addr = {};
> + vtd_irte_t *irte = phys_to_virt(vtd_ir_table());
> + uint16_t index = vtd_intr_index_alloc();
> +
> + assert(sizeof(vtd_msi_addr_t) == 8);
> + assert(sizeof(vtd_msi_data_t) == 4);
> +
> + printf("INTR: setup IRTE index %d\n", index);
> + vtd_setup_irte(dev, irte + index, vector, dest_id);
> +
> + msi_addr.handle_15 = index >> 15 & 1;
> + msi_addr.shv = 0;
> + msi_addr.interrupt_format = 1;
> + msi_addr.handle_0_14 = index & 0x7fff;
> + msi_addr.head = 0xfee;
> + msi_data.subhandle = 0;
> +
> + pci_setup_msi(dev, *(uint64_t *)&msi_addr, *(uint32_t *)&msi_data);
> +}
I skipped reviewing all the VTD stuff because that would require
I read a spec... Sorry.
> +
> void vtd_init(void)
> {
> setup_vm();
> diff --git a/lib/x86/intel-iommu.h b/lib/x86/intel-iommu.h
> index f1340cd..fb6b6a6 100644
> --- a/lib/x86/intel-iommu.h
> +++ b/lib/x86/intel-iommu.h
> @@ -125,5 +125,6 @@ void vtd_enable_dmar(void);
> void vtd_enable_ir(void);
> void vtd_init(void);
> void vtd_map_range(uint16_t sid, phys_addr_t iova, phys_addr_t pa, size_t size);
> +void vtd_setup_msi(pci_dev *dev, int vector, int dest_id);
>
> #endif
> diff --git a/x86/intel-iommu.c b/x86/intel-iommu.c
> index 0c3ab9b..c826af8 100644
> --- a/x86/intel-iommu.c
> +++ b/x86/intel-iommu.c
> @@ -12,6 +12,7 @@
>
> #include "intel-iommu.h"
> #include "pci-edu.h"
> +#include "x86/apic.h"
>
> void vtd_test_dmar(pci_edu_dev_t *dev)
> {
> @@ -48,6 +49,49 @@ void vtd_test_dmar(pci_edu_dev_t *dev)
> free_page(page);
> }
>
> +static uint32_t factorial(uint32_t in)
> +{
> + uint32_t v = 1;
> + while (in) v *= in--;
> + return v;
> +}
What, no recursion! That's a disgrace to this classic
problem! Actually I don't think you should bother with
the function at all. You hard code the input, so just
precalculate the result.
> +
> +static volatile int edu_done;
> +
> +static void edu_fact_isr(isr_regs_t *regs)
> +{
> + edu_done = 1;
nit: bool/true
> + eoi();
> +}
> +
> +static void vtd_test_ir(pci_edu_dev_t *dev)
> +{
> +#define VTD_TEST_VECTOR (0xee)
> + /* Choose any number to calculate the factorial value. */
> + const uint32_t fact = 0x8;
Nobody is choosing anything. How about just
const uint32_t fact_input = 8; /* fact(8) == 40320 */
> +
> + /*
> + * Enable EDU device interrupt, so when factorial task
> + * finishes, IRQ will be triggered.
> + */
> + edu_setup_intr(dev, 1);
> +
> + /*
> + * Setup EDU PCI device MSI, using interrupt remapping. By
> + * default, EDU device is using INTx.
> + */
> + vtd_setup_msi(&dev->pci_dev, VTD_TEST_VECTOR, 0);
> +
> + handle_irq(VTD_TEST_VECTOR, edu_fact_isr);
> + irq_enable();
> +
> + edu_fact_write(dev, fact);
> + while (!edu_done);
I guess you depend on the test framework's timeout support to
deal with never getting the interrupt. Make sure you set a
reasonable timeout in unittests.cfg
> + /* Now results should be put back to edu fact register */
> + report("EDU factorial INTR test",
> + edu_fact_read(dev) == factorial(fact));
Do we really need this factorial stuff? Can't we just raise an
interrupt with the EDU register offset 0x60?
> +}
> +
> int main(int argc, char *argv[])
> {
> int ret;
> @@ -88,6 +132,7 @@ int main(int argc, char *argv[])
> }
>
> vtd_test_dmar(&dev);
> + vtd_test_ir(&dev);
>
> return report_summary();
> }
> --
> 2.7.4
>
Thanks,
drew
--
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
