| Message ID | 1506049330-11196-28-git-send-email-tianyu.lan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Sep 21, 2017 at 11:02:08PM -0400, Lan Tianyu wrote: > From: Chao Gao <chao.gao@intel.com> > > Software writes to QIE field of GCMD to enable or disable queued > invalidations. This patch emulates QIE field of GCMD. > > Signed-off-by: Chao Gao <chao.gao@intel.com> > Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> > --- > xen/drivers/passthrough/vtd/iommu.h | 3 ++- > xen/drivers/passthrough/vtd/vvtd.c | 17 +++++++++++++++++ > 2 files changed, 19 insertions(+), 1 deletion(-) > > diff --git a/xen/drivers/passthrough/vtd/iommu.h b/xen/drivers/passthrough/vtd/iommu.h > index e19b045..c69cd21 100644 > --- a/xen/drivers/passthrough/vtd/iommu.h > +++ b/xen/drivers/passthrough/vtd/iommu.h > @@ -162,7 +162,8 @@ > #define DMA_GSTS_FLS (((u64)1) << 29) > #define DMA_GSTS_AFLS (((u64)1) << 28) > #define DMA_GSTS_WBFS (((u64)1) << 27) > -#define DMA_GSTS_QIES (((u64)1) <<26) > +#define DMA_GSTS_QIES_SHIFT 26 > +#define DMA_GSTS_QIES (((u64)1) << DMA_GSTS_QIES_SHIFT) > #define DMA_GSTS_IRES_SHIFT 25 > #define DMA_GSTS_IRES (((u64)1) << DMA_GSTS_IRES_SHIFT) > #define DMA_GSTS_SIRTPS_SHIFT 24 > diff --git a/xen/drivers/passthrough/vtd/vvtd.c b/xen/drivers/passthrough/vtd/vvtd.c > index 745941c..55f7a46 100644 > --- a/xen/drivers/passthrough/vtd/vvtd.c > +++ b/xen/drivers/passthrough/vtd/vvtd.c > @@ -496,6 +496,19 @@ static void vvtd_handle_gcmd_ire(struct vvtd *vvtd, uint32_t val) > } > } > > +static void vvtd_handle_gcmd_qie(struct vvtd *vvtd, uint32_t val) I would use 'write' intead of 'handle', since this is only used by the write path. Also you should consider dropping the vvtd prefixes from the static functions. It's quite clear they are vvtd related, and since they are static there's no need to add such a prefix. > +{ > + vvtd_info("%sable Queue Invalidation", (val & DMA_GCMD_QIE) ? "En" : "Dis"); > + > + if ( val & DMA_GCMD_QIE ) > + vvtd_set_bit(vvtd, DMAR_GSTS_REG, DMA_GSTS_QIES_SHIFT); > + else > + { > + vvtd_set_reg_quad(vvtd, DMAR_IQH_REG, 0); > + vvtd_clear_bit(vvtd, DMAR_GSTS_REG, DMA_GSTS_QIES_SHIFT); > + } > +} Since I've seen this pattern in other functions, it might be worth adding a helper that does: VVTD_SET_BIT(reg, bit, val) { if ( val ) set_bit(...); else clear_bit(...); } Then the above function could be reduced to: VVTD_SET_BIT(reg, bit, val); if ( !val ) vvtd_set_reg_quad(vvtd, DMAR_IQH_REG, 0); I expect other functions can also be simplified by this macro. Thanks, Roger.
diff --git a/xen/drivers/passthrough/vtd/iommu.h b/xen/drivers/passthrough/vtd/iommu.h index e19b045..c69cd21 100644 --- a/xen/drivers/passthrough/vtd/iommu.h +++ b/xen/drivers/passthrough/vtd/iommu.h @@ -162,7 +162,8 @@ #define DMA_GSTS_FLS (((u64)1) << 29) #define DMA_GSTS_AFLS (((u64)1) << 28) #define DMA_GSTS_WBFS (((u64)1) << 27) -#define DMA_GSTS_QIES (((u64)1) <<26) +#define DMA_GSTS_QIES_SHIFT 26 +#define DMA_GSTS_QIES (((u64)1) << DMA_GSTS_QIES_SHIFT) #define DMA_GSTS_IRES_SHIFT 25 #define DMA_GSTS_IRES (((u64)1) << DMA_GSTS_IRES_SHIFT) #define DMA_GSTS_SIRTPS_SHIFT 24 diff --git a/xen/drivers/passthrough/vtd/vvtd.c b/xen/drivers/passthrough/vtd/vvtd.c index 745941c..55f7a46 100644 --- a/xen/drivers/passthrough/vtd/vvtd.c +++ b/xen/drivers/passthrough/vtd/vvtd.c @@ -496,6 +496,19 @@ static void vvtd_handle_gcmd_ire(struct vvtd *vvtd, uint32_t val) } } +static void vvtd_handle_gcmd_qie(struct vvtd *vvtd, uint32_t val) +{ + vvtd_info("%sable Queue Invalidation", (val & DMA_GCMD_QIE) ? "En" : "Dis"); + + if ( val & DMA_GCMD_QIE ) + vvtd_set_bit(vvtd, DMAR_GSTS_REG, DMA_GSTS_QIES_SHIFT); + else + { + vvtd_set_reg_quad(vvtd, DMAR_IQH_REG, 0); + vvtd_clear_bit(vvtd, DMAR_GSTS_REG, DMA_GSTS_QIES_SHIFT); + } +} + static void vvtd_handle_gcmd_sirtp(struct vvtd *vvtd, uint32_t val) { uint64_t irta = vvtd_get_reg_quad(vvtd, DMAR_IRTA_REG); @@ -535,6 +548,10 @@ static int vvtd_write_gcmd(struct vvtd *vvtd, uint32_t val) vvtd_handle_gcmd_sirtp(vvtd, val); if ( changed & DMA_GCMD_IRE ) vvtd_handle_gcmd_ire(vvtd, val); + if ( changed & DMA_GCMD_QIE ) + vvtd_handle_gcmd_qie(vvtd, val); + if ( changed & ~(DMA_GCMD_SIRTP | DMA_GCMD_IRE | DMA_GCMD_QIE) ) + vvtd_info("Only SIRTP, IRE, QIE in GCMD are handled"); return X86EMUL_OKAY; }