|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4 09/28] x86/vvtd: Set Interrupt Remapping Table Pointer through GCMD
On Fri, Feb 09, 2018 at 04:59:11PM +0000, Roger Pau Monné wrote:
>On Fri, Nov 17, 2017 at 02:22:16PM +0800, Chao Gao wrote:
>> Software sets SIRTP field of GCMD to set/update the interrupt remapping
>> table pointer used by hardware. The interrupt remapping table pointer is
>> specified through the Interrupt Remapping Table Address (IRTA_REG)
>> register.
>>
>> This patch emulates this operation and adds some new fields in VVTD to track
>> info (e.g. the table's gfn and max supported entries) of interrupt remapping
>> table.
>>
>> Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
>> Signed-off-by: Lan Tianyu <tianyu.lan@xxxxxxxxx>
>>
>> ---
>> v4:
>> - declare eim_enabled as bool and irt as gfn_t
>> - rename vvtd_handle_gcmd_sirtp() to write_gcmd_sirtp()
>>
>> v3:
>> - ignore unaligned r/w of vt-d hardware registers and return X86EMUL_OK
>> ---
>> xen/drivers/passthrough/vtd/iommu.h | 16 ++++++-
>> xen/drivers/passthrough/vtd/vvtd.c | 86
>> +++++++++++++++++++++++++++++++++++++
>> 2 files changed, 100 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/drivers/passthrough/vtd/iommu.h
>> b/xen/drivers/passthrough/vtd/iommu.h
>> index f2ef3dd..8579843 100644
>> --- a/xen/drivers/passthrough/vtd/iommu.h
>> +++ b/xen/drivers/passthrough/vtd/iommu.h
>> @@ -48,7 +48,8 @@
>> #define DMAR_IQT_REG 0x88 /* invalidation queue tail */
>> #define DMAR_IQA_REG 0x90 /* invalidation queue addr */
>> #define DMAR_IECTL_REG 0xa0 /* invalidation event control register
>> */
>> -#define DMAR_IRTA_REG 0xb8 /* intr remap */
>> +#define DMAR_IRTA_REG 0xb8 /* base address of intr remap table */
>> +#define DMAR_IRTUA_REG 0xbc /* upper address of intr remap table */
>>
>> #define OFFSET_STRIDE (9)
>> #define dmar_readl(dmar, reg) readl((dmar) + (reg))
>> @@ -150,6 +151,9 @@
>> #define DMA_GCMD_SIRTP (((u64)1) << 24)
>> #define DMA_GCMD_CFI (((u64)1) << 23)
>>
>> +/* mask of one-shot bits */
>> +#define DMA_GCMD_ONE_SHOT_MASK 0x96ffffff
>> +
>> /* GSTS_REG */
>> #define DMA_GSTS_TES (((u64)1) << 31)
>> #define DMA_GSTS_RTPS (((u64)1) << 30)
>> @@ -157,10 +161,18 @@
>> #define DMA_GSTS_AFLS (((u64)1) << 28)
>> #define DMA_GSTS_WBFS (((u64)1) << 27)
>> #define DMA_GSTS_QIES (((u64)1) <<26)
>> +#define DMA_GSTS_SIRTPS_SHIFT 24
>> +#define DMA_GSTS_SIRTPS (((u64)1) << DMA_GSTS_SIRTPS_SHIFT)
>> #define DMA_GSTS_IRES (((u64)1) <<25)
>> -#define DMA_GSTS_SIRTPS (((u64)1) << 24)
>> #define DMA_GSTS_CFIS (((u64)1) <<23)
>>
>> +/* IRTA_REG */
>> +/* The base of 4KB aligned interrupt remapping table */
>> +#define DMA_IRTA_ADDR(val) ((val) & ~0xfffULL)
>> +/* The size of remapping table is 2^(x+1), where x is the size field in
>> IRTA */
>> +#define DMA_IRTA_S(val) (val & 0xf)
>> +#define DMA_IRTA_SIZE(val) (1UL << (DMA_IRTA_S(val) + 1))
>> +
>> /* PMEN_REG */
>> #define DMA_PMEN_EPM (((u32)1) << 31)
>> #define DMA_PMEN_PRS (((u32)1) << 0)
>> diff --git a/xen/drivers/passthrough/vtd/vvtd.c
>> b/xen/drivers/passthrough/vtd/vvtd.c
>> index d78d878..f0476fe 100644
>> --- a/xen/drivers/passthrough/vtd/vvtd.c
>> +++ b/xen/drivers/passthrough/vtd/vvtd.c
>> @@ -36,6 +36,12 @@
>> #define VVTD_MAX_OFFSET VVTD_FRCD_END
>>
>> struct hvm_hw_vvtd {
>> + bool eim_enabled;
>> +
>> + /* Interrupt remapping table base gfn and the max of entries */
>> + uint16_t irt_max_entry;
>> + gfn_t irt;
>> +
>> uint32_t regs[VVTD_MAX_OFFSET/sizeof(uint32_t)];
>> };
>>
>> @@ -73,6 +79,16 @@ boolean_runtime_param("viommu_verbose", viommu_verbose);
>>
>> #define VVTD_REG_POS(vvtd, offset) &(vvtd->hw.regs[offset/sizeof(uint32_t)])
>>
>> +static inline void vvtd_set_bit(struct vvtd *vvtd, uint32_t reg, int nr)
>> +{
>> + __set_bit(nr, VVTD_REG_POS(vvtd, reg));
>> +}
>> +
>> +static inline void vvtd_clear_bit(struct vvtd *vvtd, uint32_t reg, int nr)
>> +{
>> + __clear_bit(nr, VVTD_REG_POS(vvtd, reg));
>> +}
>> +
>> static inline void vvtd_set_reg(struct vvtd *vvtd, uint32_t reg, uint32_t
>> value)
>> {
>> *VVTD_REG_POS(vvtd, reg) = value;
>> @@ -102,6 +118,52 @@ static void *domain_vvtd(const struct domain *d)
>> return NULL;
>> }
>>
>> +static void write_gcmd_sirtp(struct vvtd *vvtd, uint32_t val)
>> +{
>> + uint64_t irta = vvtd_get_reg_quad(vvtd, DMAR_IRTA_REG);
>> +
>> + if ( !(val & DMA_GCMD_SIRTP) )
>
>I think you likely want to do put_gfn here (see my comment below).
>
>> + return;
>> +
>> + /*
>> + * Hardware clears this bit when software sets the SIRTPS field in
>> + * the Global Command register and sets it when hardware completes
>> + * the 'Set Interrupt Remap Table Pointer' operation.
>> + */
>> + vvtd_clear_bit(vvtd, DMAR_GSTS_REG, DMA_GSTS_SIRTPS_SHIFT);
>> +
>> + if ( gfn_x(vvtd->hw.irt) != PFN_DOWN(DMA_IRTA_ADDR(irta)) ||
>> + vvtd->hw.irt_max_entry != DMA_IRTA_SIZE(irta) )
>> + {
>> + vvtd->hw.irt = _gfn(PFN_DOWN(DMA_IRTA_ADDR(irta)));
>
>I'm not sure about the usage of this gfn (I guess I will figure out in
>further patches), but I think you should probably use get_gfn so that
>you take a reference to it. Using PFN_DOWN and _gfn is clearly
>defeating the purpose of the whole gfn infrastructure.
>
>Note that you then need to use put_gfn when releasing it.
The steps to enable interrupt remapping is:
1. write to IRTA. Software should write the physcial address of interrupt
remapping table to this register.
2. write GCMD with SIRTP set. According to VT-d spec 10.4.4, software
sets SIRTP to set/update the interrupt reampping table pointer used by
hardware.
3. write GCMD with IRE set.
In this version, we get a reference in step3 (in next patch, through
map/unmap guest IRT) other than in step2. The benefit is when guest
tries to write SIRTP many times before enabling interrupt remapping,
vvtd doesn't need to perform map/unmap guest IRT each time.
>
>> + vvtd->hw.irt_max_entry = DMA_IRTA_SIZE(irta);
>> + vvtd->hw.eim_enabled = !!(irta & IRTA_EIME);
>> + vvtd_info("Update IR info (addr=%lx eim=%d size=%d)\n",
>> + gfn_x(vvtd->hw.irt), vvtd->hw.eim_enabled,
>> + vvtd->hw.irt_max_entry);
>> + }
>> + vvtd_set_bit(vvtd, DMAR_GSTS_REG, DMA_GSTS_SIRTPS_SHIFT);
>> +}
>> +
>> +static void vvtd_write_gcmd(struct vvtd *vvtd, uint32_t val)
>> +{
>> + uint32_t orig = vvtd_get_reg(vvtd, DMAR_GSTS_REG);
>> + uint32_t changed;
>> +
>> + orig = orig & DMA_GCMD_ONE_SHOT_MASK; /* reset the one-shot bits */
>> + changed = orig ^ val;
>> +
>> + if ( !changed )
>> + return;
>> +
>> + if ( changed & (changed - 1) )
>> + vvtd_info("Write %x to GCMD (current %x), updating multiple fields",
>> + val, orig);
>
>I'm not sure I see the purpose of the above message.
I will remove this. My original throught is when we could get a warning
that guest driver doesn't completely follow VT-d spec 10.4.4:
If multiple control fields in this register need to be modified,
software much serialize the modification through multiple writes to this
register.
Thanks
Chao
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |