[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH V2] intel_iommu: TM field should not be in reserved bits
From: |
Peter Xu |
Subject: |
Re: [PATCH V2] intel_iommu: TM field should not be in reserved bits |
Date: |
Fri, 27 Sep 2019 14:10:11 +0800 |
User-agent: |
Mutt/1.11.4 (2019-03-13) |
On Fri, Sep 27, 2019 at 12:58:38PM +0800, address@hidden wrote:
> From: "Zhang, Qi" <address@hidden>
>
> When dt is supported, TM field should not be Reserved(0).
>
> Refer to VT-d Spec 9.8
>
> Signed-off-by: Zhang, Qi <address@hidden>
> Signed-off-by: Qi, Yadong <address@hidden>
> ---
> hw/i386/intel_iommu.c | 12 ++++++------
> hw/i386/intel_iommu_internal.h | 25 +++++++++++++++++++------
> 2 files changed, 25 insertions(+), 12 deletions(-)
> ---
> Changelog V2:
> move dt_supported flag to VTD_SPTE_PAGE_LX_RSVD_MASK and
> VTD_SPTE_LPAGE_LX_RSVD_MASK
>
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index f1de8fdb75..35222cf55c 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -3548,13 +3548,13 @@ static void vtd_init(IntelIOMMUState *s)
> * Rsvd field masks for spte
> */
> vtd_paging_entry_rsvd_field[0] = ~0ULL;
> - vtd_paging_entry_rsvd_field[1] = VTD_SPTE_PAGE_L1_RSVD_MASK(s->aw_bits);
> - vtd_paging_entry_rsvd_field[2] = VTD_SPTE_PAGE_L2_RSVD_MASK(s->aw_bits);
> - vtd_paging_entry_rsvd_field[3] = VTD_SPTE_PAGE_L3_RSVD_MASK(s->aw_bits);
> + vtd_paging_entry_rsvd_field[1] = VTD_SPTE_PAGE_L1_RSVD_MASK(s->aw_bits,
> x86_iommu->dt_supported);
> + vtd_paging_entry_rsvd_field[2] = VTD_SPTE_PAGE_L2_RSVD_MASK(s->aw_bits,
> x86_iommu->dt_supported);
> + vtd_paging_entry_rsvd_field[3] = VTD_SPTE_PAGE_L3_RSVD_MASK(s->aw_bits,
> x86_iommu->dt_supported);
> vtd_paging_entry_rsvd_field[4] = VTD_SPTE_PAGE_L4_RSVD_MASK(s->aw_bits);
> - vtd_paging_entry_rsvd_field[5] = VTD_SPTE_LPAGE_L1_RSVD_MASK(s->aw_bits);
> - vtd_paging_entry_rsvd_field[6] = VTD_SPTE_LPAGE_L2_RSVD_MASK(s->aw_bits);
> - vtd_paging_entry_rsvd_field[7] = VTD_SPTE_LPAGE_L3_RSVD_MASK(s->aw_bits);
> + vtd_paging_entry_rsvd_field[5] = VTD_SPTE_LPAGE_L1_RSVD_MASK(s->aw_bits,
> x86_iommu->dt_supported);
> + vtd_paging_entry_rsvd_field[6] = VTD_SPTE_LPAGE_L2_RSVD_MASK(s->aw_bits,
> x86_iommu->dt_supported);
> + vtd_paging_entry_rsvd_field[7] = VTD_SPTE_LPAGE_L3_RSVD_MASK(s->aw_bits,
> x86_iommu->dt_supported);
> vtd_paging_entry_rsvd_field[8] = VTD_SPTE_LPAGE_L4_RSVD_MASK(s->aw_bits);
Should this TM bit only affects leaves? Say, entry 1 (4K), 5 (2M), 6
(1G). While this reminded me that I'm totally confused on why we have
had entry 7, 8 after all... Are they really used?
>
> if (x86_iommu_ir_supported(x86_iommu)) {
> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
> index c1235a7063..01f1aa6c86 100644
> --- a/hw/i386/intel_iommu_internal.h
> +++ b/hw/i386/intel_iommu_internal.h
> @@ -387,19 +387,31 @@ typedef union VTDInvDesc VTDInvDesc;
> #define VTD_INV_DESC_DEVICE_IOTLB_RSVD_LO 0xffff0000ffe0fff8
>
> /* Rsvd field masks for spte */
> -#define VTD_SPTE_PAGE_L1_RSVD_MASK(aw) \
> +#define VTD_SPTE_PAGE_L1_RSVD_MASK(aw, dt_supported) \
> + dt_supported? \
> + (0x800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM | VTD_SL_TM)) : \
> (0x800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
This seems strange too in that ~VTD_HAW_MASK(aw) probably covered bits
63-48 for aw==48 case so it should already cover VTD_SL_TM?
Meanwhile when I'm reading the spec I see at least bits 61-52 ignored
rather than reserved.
Thanks,
> -#define VTD_SPTE_PAGE_L2_RSVD_MASK(aw) \
> +#define VTD_SPTE_PAGE_L2_RSVD_MASK(aw, dt_supported) \
> + dt_supported? \
> + (0x800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM | VTD_SL_TM)) : \
> (0x800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
> -#define VTD_SPTE_PAGE_L3_RSVD_MASK(aw) \
> +#define VTD_SPTE_PAGE_L3_RSVD_MASK(aw, dt_supported) \
> + dt_supported? \
> + (0x800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM | VTD_SL_TM)) : \
> (0x800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
> #define VTD_SPTE_PAGE_L4_RSVD_MASK(aw) \
> (0x880ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
> -#define VTD_SPTE_LPAGE_L1_RSVD_MASK(aw) \
> +#define VTD_SPTE_LPAGE_L1_RSVD_MASK(aw, dt_supported) \
> + dt_supported? \
> + (0x800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM | VTD_SL_TM)) : \
> (0x800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
> -#define VTD_SPTE_LPAGE_L2_RSVD_MASK(aw) \
> +#define VTD_SPTE_LPAGE_L2_RSVD_MASK(aw, dt_supported) \
> + dt_supported? \
> + (0x1ff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM | VTD_SL_TM)) : \
> (0x1ff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
> -#define VTD_SPTE_LPAGE_L3_RSVD_MASK(aw) \
> +#define VTD_SPTE_LPAGE_L3_RSVD_MASK(aw, dt_supported) \
> + dt_supported? \
> + (0x3ffff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM | VTD_SL_TM)) :
> \
> (0x3ffff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
> #define VTD_SPTE_LPAGE_L4_RSVD_MASK(aw) \
> (0x880ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
> @@ -506,5 +518,6 @@ typedef struct VTDRootEntry VTDRootEntry;
> #define VTD_SL_W (1ULL << 1)
> #define VTD_SL_PT_BASE_ADDR_MASK(aw) (~(VTD_PAGE_SIZE - 1) &
> VTD_HAW_MASK(aw))
> #define VTD_SL_IGN_COM 0xbff0000000000000ULL
> +#define VTD_SL_TM (1ULL << 62)
>
> #endif
> --
> 2.20.1
>
>
--
Peter Xu
- [PATCH V2] intel_iommu: TM field should not be in reserved bits, qi1 . zhang, 2019/09/27
- Re: [PATCH V2] intel_iommu: TM field should not be in reserved bits,
Peter Xu <=
- RE: [PATCH V2] intel_iommu: TM field should not be in reserved bits, Zhang, Qi1, 2019/09/27
- Re: [PATCH V2] intel_iommu: TM field should not be in reserved bits, Peter Xu, 2019/09/27
- RE: [PATCH V2] intel_iommu: TM field should not be in reserved bits, Zhang, Qi1, 2019/09/28
- Re: [PATCH V2] intel_iommu: TM field should not be in reserved bits, Peter Xu, 2019/09/28
- RE: [PATCH V2] intel_iommu: TM field should not be in reserved bits, Zhang, Qi1, 2019/09/28
- Re: [PATCH V2] intel_iommu: TM field should not be in reserved bits, Peter Xu, 2019/09/28
Re: [PATCH V2] intel_iommu: TM field should not be in reserved bits, Michael S. Tsirkin, 2019/09/27
Re: [PATCH V2] intel_iommu: TM field should not be in reserved bits, no-reply, 2019/09/27