| Message ID | d3aa65ad0510cdafd5d7dcbc54bc250feb6aa59b.1570503331.git.qi1.zhang@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | TM field check failed | expand |
On Tue, Nov 19, 2019 at 08:28:13PM +0800, qi1.zhang@intel.com wrote: > @@ -3547,15 +3548,17 @@ 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[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[8] = VTD_SPTE_LPAGE_L4_RSVD_MASK(s->aw_bits); > + vtd_spte_rsvd[0] = ~0ULL; > + vtd_spte_rsvd[1] = VTD_SPTE_PAGE_L1_RSVD_MASK(s->aw_bits); > + vtd_spte_rsvd[2] = VTD_SPTE_PAGE_L2_RSVD_MASK(s->aw_bits); > + vtd_spte_rsvd[3] = VTD_SPTE_PAGE_L3_RSVD_MASK(s->aw_bits); > + vtd_spte_rsvd[4] = VTD_SPTE_PAGE_L4_RSVD_MASK(s->aw_bits); > + > + vtd_spte_rsvd_large[0] = ~0ULL; > + vtd_spte_rsvd_large[1] = VTD_SPTE_LPAGE_L1_RSVD_MASK(s->aw_bits); > + vtd_spte_rsvd_large[2] = VTD_SPTE_LPAGE_L2_RSVD_MASK(s->aw_bits); > + vtd_spte_rsvd_large[3] = VTD_SPTE_LPAGE_L3_RSVD_MASK(s->aw_bits); > + vtd_spte_rsvd_large[4] = VTD_SPTE_LPAGE_L4_RSVD_MASK(s->aw_bits); This looks good to me in general, but... Since we're at it, do you think we can directly drop VTD_SPTE_LPAGE_L1_RSVD_MASK and VTD_SPTE_LPAGE_L4_RSVD_MASK? Are they really useful? I think I'm using the latest vt-d spec now (June, 2019) and it only supports 2M/1G huge pages, corresponds to VTD_SPTE_LPAGE_L2_RSVD_MASK and VTD_SPTE_LPAGE_L3_RSVD_MASK.
> -----Original Message----- > From: Peter Xu <peterx@redhat.com> > Sent: Wednesday, November 20, 2019 12:06 AM > To: Zhang, Qi1 <qi1.zhang@intel.com> > Cc: qemu-devel@nongnu.org; ehabkost@redhat.com; mst@redhat.com; > pbonzini@redhat.com; Qi, Yadong <yadong.qi@intel.com>; rth@twiddle.net > Subject: Re: [PATCH v2 1/2] intel_iommu: split the resevred fields arrays into > two ones > > On Tue, Nov 19, 2019 at 08:28:13PM +0800, qi1.zhang@intel.com wrote: > > @@ -3547,15 +3548,17 @@ 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[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[8] = VTD_SPTE_LPAGE_L4_RSVD_MASK(s- > >aw_bits); > > + vtd_spte_rsvd[0] = ~0ULL; > > + vtd_spte_rsvd[1] = VTD_SPTE_PAGE_L1_RSVD_MASK(s->aw_bits); > > + vtd_spte_rsvd[2] = VTD_SPTE_PAGE_L2_RSVD_MASK(s->aw_bits); > > + vtd_spte_rsvd[3] = VTD_SPTE_PAGE_L3_RSVD_MASK(s->aw_bits); > > + vtd_spte_rsvd[4] = VTD_SPTE_PAGE_L4_RSVD_MASK(s->aw_bits); > > + > > + vtd_spte_rsvd_large[0] = ~0ULL; > > + vtd_spte_rsvd_large[1] = VTD_SPTE_LPAGE_L1_RSVD_MASK(s->aw_bits); > > + vtd_spte_rsvd_large[2] = VTD_SPTE_LPAGE_L2_RSVD_MASK(s->aw_bits); > > + vtd_spte_rsvd_large[3] = VTD_SPTE_LPAGE_L3_RSVD_MASK(s->aw_bits); > > + vtd_spte_rsvd_large[4] = VTD_SPTE_LPAGE_L4_RSVD_MASK(s->aw_bits); > > This looks good to me in general, but... Since we're at it, do you think we can > directly drop VTD_SPTE_LPAGE_L1_RSVD_MASK and > VTD_SPTE_LPAGE_L4_RSVD_MASK? Are they really useful? Yes, the LPAGE_L1 and LPAGE_L4 are useless. I will try to make the change. Best Regard Yadong
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index f1de8fdb75..a118efaeaf 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -910,18 +910,19 @@ static dma_addr_t vtd_get_iova_pgtbl_base(IntelIOMMUState *s, /* * Rsvd field masks for spte: - * Index [1] to [4] 4k pages - * Index [5] to [8] large pages + * vtd_spte_rsvd 4k pages + * vtd_spte_rsvd_large large pages */ -static uint64_t vtd_paging_entry_rsvd_field[9]; +static uint64_t vtd_spte_rsvd[5]; +static uint64_t vtd_spte_rsvd_large[5]; 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]; + return slpte & vtd_spte_rsvd_large[level]; } else { - return slpte & vtd_paging_entry_rsvd_field[level]; + return slpte & vtd_spte_rsvd[level]; } } @@ -3547,15 +3548,17 @@ 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[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[8] = VTD_SPTE_LPAGE_L4_RSVD_MASK(s->aw_bits); + vtd_spte_rsvd[0] = ~0ULL; + vtd_spte_rsvd[1] = VTD_SPTE_PAGE_L1_RSVD_MASK(s->aw_bits); + vtd_spte_rsvd[2] = VTD_SPTE_PAGE_L2_RSVD_MASK(s->aw_bits); + vtd_spte_rsvd[3] = VTD_SPTE_PAGE_L3_RSVD_MASK(s->aw_bits); + vtd_spte_rsvd[4] = VTD_SPTE_PAGE_L4_RSVD_MASK(s->aw_bits); + + vtd_spte_rsvd_large[0] = ~0ULL; + vtd_spte_rsvd_large[1] = VTD_SPTE_LPAGE_L1_RSVD_MASK(s->aw_bits); + vtd_spte_rsvd_large[2] = VTD_SPTE_LPAGE_L2_RSVD_MASK(s->aw_bits); + vtd_spte_rsvd_large[3] = VTD_SPTE_LPAGE_L3_RSVD_MASK(s->aw_bits); + vtd_spte_rsvd_large[4] = VTD_SPTE_LPAGE_L4_RSVD_MASK(s->aw_bits); if (x86_iommu_ir_supported(x86_iommu)) { s->ecap |= VTD_ECAP_IR | VTD_ECAP_MHMV;