* [PATCH V2] intel_iommu: TM field should not be in reserved bits
@ 2019-09-27 4:58 qi1.zhang
2019-09-27 6:10 ` Peter Xu
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: qi1.zhang @ 2019-09-27 4:58 UTC (permalink / raw)
To: qemu-devel; +Cc: qi1.zhang, ehabkost, mst, pbonzini, rth
From: "Zhang, Qi" <qi1.zhang@intel.com>
When dt is supported, TM field should not be Reserved(0).
Refer to VT-d Spec 9.8
Signed-off-by: Zhang, Qi <qi1.zhang@intel.com>
Signed-off-by: Qi, Yadong <yadong.qi@intel.com>
---
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);
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))
-#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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH V2] intel_iommu: TM field should not be in reserved bits 2019-09-27 4:58 [PATCH V2] intel_iommu: TM field should not be in reserved bits qi1.zhang @ 2019-09-27 6:10 ` Peter Xu 2019-09-27 8:03 ` Zhang, Qi1 2019-09-27 9:33 ` Michael S. Tsirkin 2019-09-27 20:44 ` no-reply 2 siblings, 1 reply; 10+ messages in thread From: Peter Xu @ 2019-09-27 6:10 UTC (permalink / raw) To: qi1.zhang; +Cc: pbonzini, rth, mst, qemu-devel, ehabkost On Fri, Sep 27, 2019 at 12:58:38PM +0800, qi1.zhang@intel.com wrote: > From: "Zhang, Qi" <qi1.zhang@intel.com> > > When dt is supported, TM field should not be Reserved(0). > > Refer to VT-d Spec 9.8 > > Signed-off-by: Zhang, Qi <qi1.zhang@intel.com> > Signed-off-by: Qi, Yadong <yadong.qi@intel.com> > --- > 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH V2] intel_iommu: TM field should not be in reserved bits 2019-09-27 6:10 ` Peter Xu @ 2019-09-27 8:03 ` Zhang, Qi1 2019-09-27 9:32 ` Peter Xu 0 siblings, 1 reply; 10+ messages in thread From: Zhang, Qi1 @ 2019-09-27 8:03 UTC (permalink / raw) To: Peter Xu Cc: ehabkost@redhat.com, mst@redhat.com, qemu-devel@nongnu.org, pbonzini@redhat.com, Qi, Yadong, rth@twiddle.net > -----Original Message----- > From: Peter Xu <peterx@redhat.com> > Sent: Friday, September 27, 2019 2:10 PM > To: Zhang, Qi1 <qi1.zhang@intel.com> > Cc: qemu-devel@nongnu.org; ehabkost@redhat.com; mst@redhat.com; > pbonzini@redhat.com; rth@twiddle.net > Subject: Re: [PATCH V2] intel_iommu: TM field should not be in reserved bits > > On Fri, Sep 27, 2019 at 12:58:38PM +0800, qi1.zhang@intel.com wrote: > > From: "Zhang, Qi" <qi1.zhang@intel.com> > > > > When dt is supported, TM field should not be Reserved(0). > > > > Refer to VT-d Spec 9.8 > > > > Signed-off-by: Zhang, Qi <qi1.zhang@intel.com> > > Signed-off-by: Qi, Yadong <yadong.qi@intel.com> > > --- > > 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? Yes. TM bit only affects. To this array, index 1, 5,6,7 may be leaf. Will update a new patchset for it. > > > > > 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? VTD_SL_IGN_COM 0xbff0000000000000ULL, TM field is cleared by ~ VTD_SL_IGN_COM > > Meanwhile when I'm reading the spec I see at least bits 61-52 ignored rather > than reserved. Yes. Bit 61~52 is ignored. Such as the index 5 of this array is 0xfff8000000800. > > 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V2] intel_iommu: TM field should not be in reserved bits 2019-09-27 8:03 ` Zhang, Qi1 @ 2019-09-27 9:32 ` Peter Xu 2019-09-29 1:11 ` Zhang, Qi1 0 siblings, 1 reply; 10+ messages in thread From: Peter Xu @ 2019-09-27 9:32 UTC (permalink / raw) To: Zhang, Qi1 Cc: ehabkost@redhat.com, mst@redhat.com, qemu-devel@nongnu.org, pbonzini@redhat.com, Qi, Yadong, rth@twiddle.net On Fri, Sep 27, 2019 at 08:03:21AM +0000, Zhang, Qi1 wrote: > > > > -----Original Message----- > > From: Peter Xu <peterx@redhat.com> > > Sent: Friday, September 27, 2019 2:10 PM > > To: Zhang, Qi1 <qi1.zhang@intel.com> > > Cc: qemu-devel@nongnu.org; ehabkost@redhat.com; mst@redhat.com; > > pbonzini@redhat.com; rth@twiddle.net > > Subject: Re: [PATCH V2] intel_iommu: TM field should not be in reserved bits > > > > On Fri, Sep 27, 2019 at 12:58:38PM +0800, qi1.zhang@intel.com wrote: > > > From: "Zhang, Qi" <qi1.zhang@intel.com> > > > > > > When dt is supported, TM field should not be Reserved(0). > > > > > > Refer to VT-d Spec 9.8 > > > > > > Signed-off-by: Zhang, Qi <qi1.zhang@intel.com> > > > Signed-off-by: Qi, Yadong <yadong.qi@intel.com> > > > --- > > > 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? > Yes. TM bit only affects. To this array, index 1, 5,6,7 may be leaf. Will update a new patchset for it. Could I ask why index 7 may be leaf? > > > > > > > > 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? > VTD_SL_IGN_COM 0xbff0000000000000ULL, TM field is cleared by ~ VTD_SL_IGN_COM > > > > Meanwhile when I'm reading the spec I see at least bits 61-52 ignored rather > > than reserved. > Yes. Bit 61~52 is ignored. Such as the index 5 of this array is 0xfff8000000800. Oops, my poor eye obviously didn't see that the "~" operator is applied over the whole (VTD_HAW_MASK(aw) | VTD_SL_IGN_COM)... :) Btw, you should only touch up the macros that are leaves here. Non-leaves should still keep that bit as reserved. Thanks, -- Peter Xu ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH V2] intel_iommu: TM field should not be in reserved bits 2019-09-27 9:32 ` Peter Xu @ 2019-09-29 1:11 ` Zhang, Qi1 2019-09-29 2:02 ` Peter Xu 0 siblings, 1 reply; 10+ messages in thread From: Zhang, Qi1 @ 2019-09-29 1:11 UTC (permalink / raw) To: Peter Xu Cc: ehabkost@redhat.com, mst@redhat.com, qemu-devel@nongnu.org, pbonzini@redhat.com, Qi, Yadong, rth@twiddle.net > -----Original Message----- > From: Peter Xu <peterx@redhat.com> > Sent: Friday, September 27, 2019 5:32 PM > To: Zhang, Qi1 <qi1.zhang@intel.com> > Cc: qemu-devel@nongnu.org; ehabkost@redhat.com; mst@redhat.com; > pbonzini@redhat.com; rth@twiddle.net; Qi, Yadong <yadong.qi@intel.com> > Subject: Re: [PATCH V2] intel_iommu: TM field should not be in reserved bits > > On Fri, Sep 27, 2019 at 08:03:21AM +0000, Zhang, Qi1 wrote: > > > > > > > -----Original Message----- > > > From: Peter Xu <peterx@redhat.com> > > > Sent: Friday, September 27, 2019 2:10 PM > > > To: Zhang, Qi1 <qi1.zhang@intel.com> > > > Cc: qemu-devel@nongnu.org; ehabkost@redhat.com; mst@redhat.com; > > > pbonzini@redhat.com; rth@twiddle.net > > > Subject: Re: [PATCH V2] intel_iommu: TM field should not be in > > > reserved bits > > > > > > On Fri, Sep 27, 2019 at 12:58:38PM +0800, qi1.zhang@intel.com wrote: > > > > From: "Zhang, Qi" <qi1.zhang@intel.com> > > > > > > > > When dt is supported, TM field should not be Reserved(0). > > > > > > > > Refer to VT-d Spec 9.8 > > > > > > > > Signed-off-by: Zhang, Qi <qi1.zhang@intel.com> > > > > Signed-off-by: Qi, Yadong <yadong.qi@intel.com> > > > > --- > > > > hw/i386/intel_iommu.c | 12 ++++++------ > > > > hw/i386/intel_iommu_internal.h | 25 +++++++++++++++++++------ > > > > 2 files changed, 25 insertions(+), 12 deletions(-) > > > > --- > VTD_SPTE_PAGE_L2_RSVD_MASK(s- > > > >aw_bits); > > > > - vtd_paging_entry_rsvd_field[3] = > > > >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? > > Yes. TM bit only affects. To this array, index 1, 5,6,7 may be leaf. Will update > a new patchset for it. > > Could I ask why index 7 may be leaf? Index 7 is PDPE 1G GB leaf. > > > > > > > > > > > > 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? > > VTD_SL_IGN_COM 0xbff0000000000000ULL, TM field is cleared by ~ > > VTD_SL_IGN_COM > > > > > > Meanwhile when I'm reading the spec I see at least bits 61-52 > > > ignored rather than reserved. > > Yes. Bit 61~52 is ignored. Such as the index 5 of this array is 0xfff8000000800. > > Oops, my poor eye obviously didn't see that the "~" operator is applied over > the whole (VTD_HAW_MASK(aw) | VTD_SL_IGN_COM)... :) > > Btw, you should only touch up the macros that are leaves here. > Non-leaves should still keep that bit as reserved. Yes. I will. > > Thanks, > > -- > Peter Xu ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V2] intel_iommu: TM field should not be in reserved bits 2019-09-29 1:11 ` Zhang, Qi1 @ 2019-09-29 2:02 ` Peter Xu 2019-09-29 2:10 ` Zhang, Qi1 2019-09-29 2:19 ` Peter Xu 0 siblings, 2 replies; 10+ messages in thread From: Peter Xu @ 2019-09-29 2:02 UTC (permalink / raw) To: Zhang, Qi1 Cc: ehabkost@redhat.com, mst@redhat.com, qemu-devel@nongnu.org, pbonzini@redhat.com, Qi, Yadong, rth@twiddle.net On Sun, Sep 29, 2019 at 01:11:12AM +0000, Zhang, Qi1 wrote: > > > > -----Original Message----- > > From: Peter Xu <peterx@redhat.com> > > Sent: Friday, September 27, 2019 5:32 PM > > To: Zhang, Qi1 <qi1.zhang@intel.com> > > Cc: qemu-devel@nongnu.org; ehabkost@redhat.com; mst@redhat.com; > > pbonzini@redhat.com; rth@twiddle.net; Qi, Yadong <yadong.qi@intel.com> > > Subject: Re: [PATCH V2] intel_iommu: TM field should not be in reserved bits > > > > On Fri, Sep 27, 2019 at 08:03:21AM +0000, Zhang, Qi1 wrote: > > > > > > > > > > -----Original Message----- > > > > From: Peter Xu <peterx@redhat.com> > > > > Sent: Friday, September 27, 2019 2:10 PM > > > > To: Zhang, Qi1 <qi1.zhang@intel.com> > > > > Cc: qemu-devel@nongnu.org; ehabkost@redhat.com; mst@redhat.com; > > > > pbonzini@redhat.com; rth@twiddle.net > > > > Subject: Re: [PATCH V2] intel_iommu: TM field should not be in > > > > reserved bits > > > > > > > > On Fri, Sep 27, 2019 at 12:58:38PM +0800, qi1.zhang@intel.com wrote: > > > > > From: "Zhang, Qi" <qi1.zhang@intel.com> > > > > > > > > > > When dt is supported, TM field should not be Reserved(0). > > > > > > > > > > Refer to VT-d Spec 9.8 > > > > > > > > > > Signed-off-by: Zhang, Qi <qi1.zhang@intel.com> > > > > > Signed-off-by: Qi, Yadong <yadong.qi@intel.com> > > > > > --- > > > > > hw/i386/intel_iommu.c | 12 ++++++------ > > > > > hw/i386/intel_iommu_internal.h | 25 +++++++++++++++++++------ > > > > > 2 files changed, 25 insertions(+), 12 deletions(-) > > > > > --- > > VTD_SPTE_PAGE_L2_RSVD_MASK(s- > > > > >aw_bits); > > > > > - vtd_paging_entry_rsvd_field[3] = > > > > >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). [1] > > > > While this reminded me that I'm totally confused on why we have had > > > > entry 7, 8 after all... Are they really used? > > > Yes. TM bit only affects. To this array, index 1, 5,6,7 may be leaf. Will update > > a new patchset for it. > > > > Could I ask why index 7 may be leaf? > Index 7 is PDPE 1G GB leaf. I thought 1G was index 6. I've listed my understanding above [1]. Would you please double confirm? Thanks, -- Peter Xu ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH V2] intel_iommu: TM field should not be in reserved bits 2019-09-29 2:02 ` Peter Xu @ 2019-09-29 2:10 ` Zhang, Qi1 2019-09-29 2:19 ` Peter Xu 1 sibling, 0 replies; 10+ messages in thread From: Zhang, Qi1 @ 2019-09-29 2:10 UTC (permalink / raw) To: Peter Xu Cc: ehabkost@redhat.com, mst@redhat.com, qemu-devel@nongnu.org, pbonzini@redhat.com, Qi, Yadong, rth@twiddle.net > -----Original Message----- > From: Peter Xu <peterx@redhat.com> > Sent: Sunday, September 29, 2019 10:02 AM > To: Zhang, Qi1 <qi1.zhang@intel.com> > Cc: qemu-devel@nongnu.org; ehabkost@redhat.com; mst@redhat.com; > pbonzini@redhat.com; rth@twiddle.net; Qi, Yadong <yadong.qi@intel.com> > Subject: Re: [PATCH V2] intel_iommu: TM field should not be in reserved bits > > On Sun, Sep 29, 2019 at 01:11:12AM +0000, Zhang, Qi1 wrote: > > > > > > > -----Original Message----- > > > From: Peter Xu <peterx@redhat.com> > > > Sent: Friday, September 27, 2019 5:32 PM > > > To: Zhang, Qi1 <qi1.zhang@intel.com> > > > Cc: qemu-devel@nongnu.org; ehabkost@redhat.com; mst@redhat.com; > > > pbonzini@redhat.com; rth@twiddle.net; Qi, Yadong > > > <yadong.qi@intel.com> > > > Subject: Re: [PATCH V2] intel_iommu: TM field should not be in > > > reserved bits > > > > > > On Fri, Sep 27, 2019 at 08:03:21AM +0000, Zhang, Qi1 wrote: > > > > > > > > > > > > > -----Original Message----- > > > > > From: Peter Xu <peterx@redhat.com> > > > > > Sent: Friday, September 27, 2019 2:10 PM > > > > > To: Zhang, Qi1 <qi1.zhang@intel.com> > > > > > Cc: qemu-devel@nongnu.org; ehabkost@redhat.com; > mst@redhat.com; > > > > > pbonzini@redhat.com; rth@twiddle.net > > > > > Subject: Re: [PATCH V2] intel_iommu: TM field should not be in > > > > > reserved bits > > > > > > > > > > On Fri, Sep 27, 2019 at 12:58:38PM +0800, qi1.zhang@intel.com wrote: > > > > > > From: "Zhang, Qi" <qi1.zhang@intel.com> > > > > > > > > > > > > When dt is supported, TM field should not be Reserved(0). > > > > > > > > > > > > Refer to VT-d Spec 9.8 > > > > > > > > > > > > Signed-off-by: Zhang, Qi <qi1.zhang@intel.com> > > > > > > Signed-off-by: Qi, Yadong <yadong.qi@intel.com> > > > > > > --- > > > > > > hw/i386/intel_iommu.c | 12 ++++++------ > > > > > > hw/i386/intel_iommu_internal.h | 25 +++++++++++++++++++------ > > > > > > 2 files changed, 25 insertions(+), 12 deletions(-) > > > > > > --- > > > VTD_SPTE_PAGE_L2_RSVD_MASK(s- > > > > > >aw_bits); > > > > > > - vtd_paging_entry_rsvd_field[3] = > > > > > >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). > > [1] > > > > > > While this reminded me that I'm totally confused on why we have > > > > > had entry 7, 8 after all... Are they really used? > > > > Yes. TM bit only affects. To this array, index 1, 5,6,7 may be > > > > leaf. Will update > > > a new patchset for it. > > > > > > Could I ask why index 7 may be leaf? > > Index 7 is PDPE 1G GB leaf. > > I thought 1G was index 6. I've listed my understanding above [1]. > Would you please double confirm? Thanks, Check the existing function. When level is 3 VTD_SL_PDP_LEVEL and the entry is leaf, it is PDPE 1G leaf and the corresponding index of this array 7. static bool vtd_slpte_nonzero_rsvd(uint64_t slpte, uint32_t level) { if (slpte & VTD_SL_PT_PAGE_SIZE_MASK) { /* Maybe large page */ return slpte & vtd_paging_entry_rsvd_field[level + 4]; } else { return slpte & vtd_paging_entry_rsvd_field[level]; } } > > -- > Peter Xu ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V2] intel_iommu: TM field should not be in reserved bits 2019-09-29 2:02 ` Peter Xu 2019-09-29 2:10 ` Zhang, Qi1 @ 2019-09-29 2:19 ` Peter Xu 1 sibling, 0 replies; 10+ messages in thread From: Peter Xu @ 2019-09-29 2:19 UTC (permalink / raw) To: Zhang, Qi1 Cc: ehabkost@redhat.com, mst@redhat.com, qemu-devel@nongnu.org, pbonzini@redhat.com, Qi, Yadong, rth@twiddle.net On Sun, Sep 29, 2019 at 10:02:20AM +0800, Peter Xu wrote: > On Sun, Sep 29, 2019 at 01:11:12AM +0000, Zhang, Qi1 wrote: > > > > > > > -----Original Message----- > > > From: Peter Xu <peterx@redhat.com> > > > Sent: Friday, September 27, 2019 5:32 PM > > > To: Zhang, Qi1 <qi1.zhang@intel.com> > > > Cc: qemu-devel@nongnu.org; ehabkost@redhat.com; mst@redhat.com; > > > pbonzini@redhat.com; rth@twiddle.net; Qi, Yadong <yadong.qi@intel.com> > > > Subject: Re: [PATCH V2] intel_iommu: TM field should not be in reserved bits > > > > > > On Fri, Sep 27, 2019 at 08:03:21AM +0000, Zhang, Qi1 wrote: > > > > > > > > > > > > > -----Original Message----- > > > > > From: Peter Xu <peterx@redhat.com> > > > > > Sent: Friday, September 27, 2019 2:10 PM > > > > > To: Zhang, Qi1 <qi1.zhang@intel.com> > > > > > Cc: qemu-devel@nongnu.org; ehabkost@redhat.com; mst@redhat.com; > > > > > pbonzini@redhat.com; rth@twiddle.net > > > > > Subject: Re: [PATCH V2] intel_iommu: TM field should not be in > > > > > reserved bits > > > > > > > > > > On Fri, Sep 27, 2019 at 12:58:38PM +0800, qi1.zhang@intel.com wrote: > > > > > > From: "Zhang, Qi" <qi1.zhang@intel.com> > > > > > > > > > > > > When dt is supported, TM field should not be Reserved(0). > > > > > > > > > > > > Refer to VT-d Spec 9.8 > > > > > > > > > > > > Signed-off-by: Zhang, Qi <qi1.zhang@intel.com> > > > > > > Signed-off-by: Qi, Yadong <yadong.qi@intel.com> > > > > > > --- > > > > > > hw/i386/intel_iommu.c | 12 ++++++------ > > > > > > hw/i386/intel_iommu_internal.h | 25 +++++++++++++++++++------ > > > > > > 2 files changed, 25 insertions(+), 12 deletions(-) > > > > > > --- > > > VTD_SPTE_PAGE_L2_RSVD_MASK(s- > > > > > >aw_bits); > > > > > > - vtd_paging_entry_rsvd_field[3] = > > > > > >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). > > [1] > > > > > > While this reminded me that I'm totally confused on why we have had > > > > > entry 7, 8 after all... Are they really used? > > > > Yes. TM bit only affects. To this array, index 1, 5,6,7 may be leaf. Will update > > > a new patchset for it. > > > > > > Could I ask why index 7 may be leaf? > > Index 7 is PDPE 1G GB leaf. > > I thought 1G was index 6. I've listed my understanding above [1]. > Would you please double confirm? Thanks, Oh wait, You are right... Index 6 should be for 1G because index 5 seems to be unused as well. However then again we should drop 5 instead of 7? I think we can do this in two patches: Patch 1 to clean these up by only let vtd_paging_rsvd (we can rename it to shorter one like this if going to touch it) to keep reserved bits for non-huge pages. Then we define a new struct (e.g. vtd_paging_rsvd_huge) to only keep the huge page entries. The thing is that I see no point in keeping huge && non-huge in a single array (and I believe that's why it caused confusion so far...). That new one should be a size of 2 array. Meanwhile we need to fix vtd_slpte_nonzero_rsvd() too using the new arrays. Then in patch 2 we do the DT bit change. Does that look ok? Thanks, -- Peter Xu ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V2] intel_iommu: TM field should not be in reserved bits 2019-09-27 4:58 [PATCH V2] intel_iommu: TM field should not be in reserved bits qi1.zhang 2019-09-27 6:10 ` Peter Xu @ 2019-09-27 9:33 ` Michael S. Tsirkin 2019-09-27 20:44 ` no-reply 2 siblings, 0 replies; 10+ messages in thread From: Michael S. Tsirkin @ 2019-09-27 9:33 UTC (permalink / raw) To: qi1.zhang; +Cc: pbonzini, ehabkost, qemu-devel, rth On Fri, Sep 27, 2019 at 12:58:38PM +0800, qi1.zhang@intel.com wrote: > From: "Zhang, Qi" <qi1.zhang@intel.com> > > When dt is supported, TM field should not be Reserved(0). > > Refer to VT-d Spec 9.8 > > Signed-off-by: Zhang, Qi <qi1.zhang@intel.com> > Signed-off-by: Qi, Yadong <yadong.qi@intel.com> > --- > 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); > > 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? \ space before ? please. > + (0x800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM | VTD_SL_TM)) : \ > (0x800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM)) cleaner: 0x800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM | (dt_supported ? VTD_SL_TM : 0x0)) > -#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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V2] intel_iommu: TM field should not be in reserved bits 2019-09-27 4:58 [PATCH V2] intel_iommu: TM field should not be in reserved bits qi1.zhang 2019-09-27 6:10 ` Peter Xu 2019-09-27 9:33 ` Michael S. Tsirkin @ 2019-09-27 20:44 ` no-reply 2 siblings, 0 replies; 10+ messages in thread From: no-reply @ 2019-09-27 20:44 UTC (permalink / raw) To: qi1.zhang; +Cc: qi1.zhang, ehabkost, mst, qemu-devel, pbonzini, rth Patchew URL: https://patchew.org/QEMU/20190927045838.2968-1-qi1.zhang@intel.com/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20190927045838.2968-1-qi1.zhang@intel.com Subject: [PATCH V2] intel_iommu: TM field should not be in reserved bits === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Switched to a new branch 'test' 4cd505b intel_iommu: TM field should not be in reserved bits === OUTPUT BEGIN === ERROR: line over 90 characters #26: FILE: hw/i386/intel_iommu.c:3551: + vtd_paging_entry_rsvd_field[1] = VTD_SPTE_PAGE_L1_RSVD_MASK(s->aw_bits, x86_iommu->dt_supported); ERROR: line over 90 characters #27: FILE: hw/i386/intel_iommu.c:3552: + vtd_paging_entry_rsvd_field[2] = VTD_SPTE_PAGE_L2_RSVD_MASK(s->aw_bits, x86_iommu->dt_supported); ERROR: line over 90 characters #28: FILE: hw/i386/intel_iommu.c:3553: + vtd_paging_entry_rsvd_field[3] = VTD_SPTE_PAGE_L3_RSVD_MASK(s->aw_bits, x86_iommu->dt_supported); ERROR: line over 90 characters #33: FILE: hw/i386/intel_iommu.c:3555: + vtd_paging_entry_rsvd_field[5] = VTD_SPTE_LPAGE_L1_RSVD_MASK(s->aw_bits, x86_iommu->dt_supported); ERROR: line over 90 characters #34: FILE: hw/i386/intel_iommu.c:3556: + vtd_paging_entry_rsvd_field[6] = VTD_SPTE_LPAGE_L2_RSVD_MASK(s->aw_bits, x86_iommu->dt_supported); ERROR: line over 90 characters #35: FILE: hw/i386/intel_iommu.c:3557: + vtd_paging_entry_rsvd_field[7] = VTD_SPTE_LPAGE_L3_RSVD_MASK(s->aw_bits, x86_iommu->dt_supported); ERROR: spaces required around that '?' (ctx:VxE) #49: FILE: hw/i386/intel_iommu_internal.h:391: + dt_supported? \ ^ ERROR: spaces required around that '?' (ctx:VxE) #54: FILE: hw/i386/intel_iommu_internal.h:395: + dt_supported? \ ^ ERROR: spaces required around that '?' (ctx:VxE) #59: FILE: hw/i386/intel_iommu_internal.h:399: + dt_supported? \ ^ ERROR: spaces required around that '?' (ctx:VxE) #66: FILE: hw/i386/intel_iommu_internal.h:405: + dt_supported? \ ^ ERROR: spaces required around that '?' (ctx:VxE) #71: FILE: hw/i386/intel_iommu_internal.h:409: + dt_supported? \ ^ ERROR: spaces required around that '?' (ctx:VxE) #76: FILE: hw/i386/intel_iommu_internal.h:413: + dt_supported? \ ^ total: 12 errors, 0 warnings, 62 lines checked Commit 4cd505b83cac (intel_iommu: TM field should not be in reserved bits) has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20190927045838.2968-1-qi1.zhang@intel.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-09-29 2:20 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-09-27 4:58 [PATCH V2] intel_iommu: TM field should not be in reserved bits qi1.zhang 2019-09-27 6:10 ` Peter Xu 2019-09-27 8:03 ` Zhang, Qi1 2019-09-27 9:32 ` Peter Xu 2019-09-29 1:11 ` Zhang, Qi1 2019-09-29 2:02 ` Peter Xu 2019-09-29 2:10 ` Zhang, Qi1 2019-09-29 2:19 ` Peter Xu 2019-09-27 9:33 ` Michael S. Tsirkin 2019-09-27 20:44 ` no-reply
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).