qemu-devel
[Top][All Lists]
Advanced

[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: Zhang, Qi1
Subject: RE: [PATCH V2] intel_iommu: TM field should not be in reserved bits
Date: Sun, 29 Sep 2019 01:11:12 +0000


> -----Original Message-----
> From: Peter Xu <address@hidden>
> Sent: Friday, September 27, 2019 5:32 PM
> To: Zhang, Qi1 <address@hidden>
> Cc: address@hidden; address@hidden; address@hidden;
> address@hidden; address@hidden; Qi, Yadong <address@hidden>
> 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 <address@hidden>
> > > Sent: Friday, September 27, 2019 2:10 PM
> > > To: Zhang, Qi1 <address@hidden>
> > > Cc: address@hidden; address@hidden; address@hidden;
> > > address@hidden; address@hidden
> > > Subject: Re: [PATCH V2] intel_iommu: TM field should not be in
> > > reserved bits
> > >
> > > 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(-)
> > > > ---
> 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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]