Re: [PATCH 2/3] xen/vt-d: mask interrupt message generation

["Zhang, Yang Z" <yang.z.zhang@intel.com>]

Chen, Tiejun wrote on 2015-05-04:
> While initializing VT-D we should mask interrupt message generation
> to avoid receiving any interrupt as pending before enable DMA
> translation, and also mask that before disable DMA engine.
> 
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> ---
>  xen/drivers/passthrough/vtd/iommu.c | 31 +++++++++++++++++++++++--------
>  1 file changed, 23 insertions(+), 8 deletions(-)
> diff --git a/xen/drivers/passthrough/vtd/iommu.c
> b/xen/drivers/passthrough/vtd/iommu.c index c7bda73..72cd854 100644 ---
> a/xen/drivers/passthrough/vtd/iommu.c +++
> b/xen/drivers/passthrough/vtd/iommu.c @@ -1000,15 +1000,21 @@ static
> void dma_msi_unmask(struct irq_desc *desc)
>      iommu->msi.msi_attrib.masked = 0;
>  }
> -static void dma_msi_mask(struct irq_desc *desc)
> +static void mask_dma_interrupt(struct iommu *iommu)
>  {
>      unsigned long flags;
> -    struct iommu *iommu = desc->action->dev_id;
> 
> -    /* mask it */
>      spin_lock_irqsave(&iommu->register_lock, flags);
>      dmar_writel(iommu->reg, DMAR_FECTL_REG, DMA_FECTL_IM);
>      spin_unlock_irqrestore(&iommu->register_lock, flags);
> +}
> +
> +static void dma_msi_mask(struct irq_desc *desc)
> +{
> +    struct iommu *iommu = desc->action->dev_id;
> +
> +    /* mask it */
> +    mask_dma_interrupt(iommu);
>      iommu->msi.msi_attrib.masked = 1;
>  }
> @@ -1997,7 +2003,6 @@ static int init_vtd_hw(void)
>      struct iommu *iommu;
>      struct iommu_flush *flush = NULL;
>      int ret;
> -    unsigned long flags;
> 
>      /*
>       * Basic VT-d HW init: set VT-d interrupt, clear VT-d faults.
> @@ -2008,11 +2013,16 @@ static int init_vtd_hw(void)
> 
>          iommu = drhd->iommu;
> -        clear_fault_bits(iommu);
> +        /*
> +         * We shouldn't receive any VT-d interrupt while initializing
> +         * VT-d so just mask interrupt message generation.
> +         */
> +        mask_dma_interrupt(iommu);
> 
> -        spin_lock_irqsave(&iommu->register_lock, flags);
> -        dmar_writel(iommu->reg, DMAR_FECTL_REG, 0);
> -        spin_unlock_irqrestore(&iommu->register_lock, flags);
> +        /*
> +         * And then clear all previous faults.
> +         */
> +        clear_fault_bits(iommu);
>      }
>      
>      /*
> @@ -2350,6 +2360,11 @@ static void vtd_suspend(void)
>          if ( force_iommu )
>              continue;
> +        /*
> +         * Mask interrupt message generation.
> +         */
> +        mask_dma_interrupt(iommu);
> +
>          iommu_disable_translation(iommu);
>          
>          /* If interrupt remapping is enabled, queued invalidation

Just curious that do you observe a real issue or just catch it through reading code?

Best regards,
Yang