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 <pet...@redhat.com> > > > Sent: Friday, September 27, 2019 5:32 PM > > > To: Zhang, Qi1 <qi1.zh...@intel.com> > > > Cc: qemu-devel@nongnu.org; ehabk...@redhat.com; m...@redhat.com; > > > pbonz...@redhat.com; r...@twiddle.net; Qi, Yadong <yadong...@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 <pet...@redhat.com> > > > > > Sent: Friday, September 27, 2019 2:10 PM > > > > > To: Zhang, Qi1 <qi1.zh...@intel.com> > > > > > Cc: qemu-devel@nongnu.org; ehabk...@redhat.com; m...@redhat.com; > > > > > pbonz...@redhat.com; r...@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.zh...@intel.com wrote: > > > > > > From: "Zhang, Qi" <qi1.zh...@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.zh...@intel.com> > > > > > > Signed-off-by: Qi, Yadong <yadong...@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
