VT-d: correct way to submit command to GCMD register

Per VT-d spec, software should submit only one "incremental" command
at a time to Global Command reigster. Current implementation uses a
variable (gcmd) to record the state of Global Status register. It's
error prone.

Signed-off-by: Weidong Han <weidong.han@intel.com>

xen/drivers/passthrough/vtd/intremap.c

@@ -534,7 +534,7 @@ void msi_msg_write_remap_rte(
 int enable_intremap(struct iommu *iommu)
 {
     struct ir_ctrl *ir_ctrl;
-    u32 sts;
+    u32 sts, gcmd;
 
     ASSERT(ecap_intr_remap(iommu->ecap) && iommu_intremap);
 
@@ -561,22 +561,23 @@ int enable_intremap(struct iommu *iommu)
     dmar_writeq(iommu->reg, DMAR_IRTA_REG, ir_ctrl->iremap_maddr);
 
     /* set SIRTP */
-    iommu->gcmd |= DMA_GCMD_SIRTP;
-    dmar_writel(iommu->reg, DMAR_GCMD_REG, iommu->gcmd);
+    gcmd = dmar_readl(iommu->reg, DMAR_GSTS_REG);
+    gcmd |= DMA_GCMD_SIRTP;
+    dmar_writel(iommu->reg, DMAR_GCMD_REG, gcmd);
 
     IOMMU_WAIT_OP(iommu, DMAR_GSTS_REG, dmar_readl,
                   (sts & DMA_GSTS_SIRTPS), sts);
  
     /* enable comaptiblity format interrupt pass through */
-    iommu->gcmd |= DMA_GCMD_CFI;
-    dmar_writel(iommu->reg, DMAR_GCMD_REG, iommu->gcmd);
+    gcmd |= DMA_GCMD_CFI;
+    dmar_writel(iommu->reg, DMAR_GCMD_REG, gcmd);
 
     IOMMU_WAIT_OP(iommu, DMAR_GSTS_REG, dmar_readl,
                   (sts & DMA_GSTS_CFIS), sts);
 
     /* enable interrupt remapping hardware */
-    iommu->gcmd |= DMA_GCMD_IRE;
-    dmar_writel(iommu->reg, DMAR_GCMD_REG, iommu->gcmd);
+    gcmd |= DMA_GCMD_IRE;
+    dmar_writel(iommu->reg, DMAR_GCMD_REG, gcmd);
 
     IOMMU_WAIT_OP(iommu, DMAR_GSTS_REG, dmar_readl,
                   (sts & DMA_GSTS_IRES), sts);
@@ -593,8 +594,8 @@ void disable_intremap(struct iommu *iommu)
 
     ASSERT(ecap_intr_remap(iommu->ecap) && iommu_intremap);
 
-    iommu->gcmd &= ~(DMA_GCMD_SIRTP | DMA_GCMD_CFI | DMA_GCMD_IRE);
-    dmar_writel(iommu->reg, DMAR_GCMD_REG, iommu->gcmd);
+    sts = dmar_readl(iommu->reg, DMAR_GSTS_REG);
+    dmar_writel(iommu->reg, DMAR_GCMD_REG, sts & (~DMA_GCMD_IRE));
 
     IOMMU_WAIT_OP(iommu, DMAR_GSTS_REG, dmar_readl,
                   !(sts & DMA_GSTS_IRES), sts);

xen/drivers/passthrough/vtd/iommu.c

@@ -233,10 +233,10 @@ static void iommu_flush_write_buffer(struct iommu *iommu)
 
     if ( !rwbf_quirk && !cap_rwbf(iommu->cap) )
         return;
-    val = iommu->gcmd | DMA_GCMD_WBF;
 
     spin_lock_irqsave(&iommu->register_lock, flag);
-    dmar_writel(iommu->reg, DMAR_GCMD_REG, val);
+    val = dmar_readl(iommu->reg, DMAR_GSTS_REG);
+    dmar_writel(iommu->reg, DMAR_GCMD_REG, val | DMA_GCMD_WBF);
 
     /* Make sure hardware complete it */
     IOMMU_WAIT_OP(iommu, DMAR_GSTS_REG, dmar_readl,
@@ -548,7 +548,7 @@ static void iommu_free_pagetable(u64 pt_maddr, int level)
 
 static int iommu_set_root_entry(struct iommu *iommu)
 {
-    u32 cmd, sts;
+    u32 sts;
     unsigned long flags;
 
     spin_lock(&iommu->lock);
@@ -564,8 +564,9 @@ static int iommu_set_root_entry(struct iommu *iommu)
     spin_unlock(&iommu->lock);
     spin_lock_irqsave(&iommu->register_lock, flags);
     dmar_writeq(iommu->reg, DMAR_RTADDR_REG, iommu->root_maddr);
-    cmd = iommu->gcmd | DMA_GCMD_SRTP;
-    dmar_writel(iommu->reg, DMAR_GCMD_REG, cmd);
+
+    sts = dmar_readl(iommu->reg, DMAR_GSTS_REG);
+    dmar_writel(iommu->reg, DMAR_GCMD_REG, sts | DMA_GCMD_SRTP);
 
     /* Make sure hardware complete it */
     IOMMU_WAIT_OP(iommu, DMAR_GSTS_REG, dmar_readl,
@@ -583,8 +584,8 @@ static void iommu_enable_translation(struct iommu *iommu)
     dprintk(XENLOG_INFO VTDPREFIX,
             "iommu_enable_translation: iommu->reg = %p\n", iommu->reg);
     spin_lock_irqsave(&iommu->register_lock, flags);
-    iommu->gcmd |= DMA_GCMD_TE;
-    dmar_writel(iommu->reg, DMAR_GCMD_REG, iommu->gcmd);
+    sts = dmar_readl(iommu->reg, DMAR_GSTS_REG);
+    dmar_writel(iommu->reg, DMAR_GCMD_REG, sts | DMA_GCMD_TE);
 
     /* Make sure hardware complete it */
     IOMMU_WAIT_OP(iommu, DMAR_GSTS_REG, dmar_readl,
@@ -601,8 +602,8 @@ static void iommu_disable_translation(struct iommu *iommu)
     unsigned long flags;
 
     spin_lock_irqsave(&iommu->register_lock, flags);
-    iommu->gcmd &= ~ DMA_GCMD_TE;
-    dmar_writel(iommu->reg, DMAR_GCMD_REG, iommu->gcmd);
+    sts = dmar_readl(iommu->reg, DMAR_GSTS_REG);
+    dmar_writel(iommu->reg, DMAR_GCMD_REG, sts & (~DMA_GCMD_TE));
 
     /* Make sure hardware complete it */
     IOMMU_WAIT_OP(iommu, DMAR_GSTS_REG, dmar_readl,

xen/drivers/passthrough/vtd/qinval.c

@@ -454,8 +454,8 @@ int enable_qinval(struct iommu *iommu)
     dmar_writeq(iommu->reg, DMAR_IQT_REG, 0);
 
     /* enable queued invalidation hardware */
-    iommu->gcmd |= DMA_GCMD_QIE;
-    dmar_writel(iommu->reg, DMAR_GCMD_REG, iommu->gcmd);
+    sts = dmar_readl(iommu->reg, DMAR_GSTS_REG);
+    dmar_writel(iommu->reg, DMAR_GCMD_REG, sts | DMA_GCMD_QIE);
 
     /* Make sure hardware complete it */
     IOMMU_WAIT_OP(iommu, DMAR_GSTS_REG, dmar_readl,
@@ -471,8 +471,8 @@ void disable_qinval(struct iommu *iommu)
 
     ASSERT(ecap_queued_inval(iommu->ecap) && iommu_qinval);
 
-    iommu->gcmd &= ~DMA_GCMD_QIE;
-    dmar_writel(iommu->reg, DMAR_GCMD_REG, iommu->gcmd);
+    sts = dmar_readl(iommu->reg, DMAR_GSTS_REG);
+    dmar_writel(iommu->reg, DMAR_GCMD_REG, sts & (~DMA_GCMD_QIE));
 
     /* Make sure hardware complete it */
     IOMMU_WAIT_OP(iommu, DMAR_GSTS_REG, dmar_readl,

xen/include/xen/iommu.h

@@ -47,7 +47,6 @@ struct iommu {
     struct list_head list;
     void __iomem *reg; /* Pointer to hardware regs, virtual addr */
     u32	index;         /* Sequence number of iommu */
-    u32	gcmd;          /* Holds TE, EAFL. Don't need SRTP, SFL, WBF */
     u32 nr_pt_levels;
     u64	cap;
     u64	ecap;