On Mon, Mar 14, 2022 at 03:45:47PM +0000, David Woodhouse wrote: > On Mon, 2022-03-14 at 11:24 -0400, Michael S. Tsirkin wrote: > > On Mon, Mar 14, 2022 at 02:25:42PM +0000, David Woodhouse wrote: > > > From: David Woodhouse <dwmw@xxxxxxxxxxxx> > > > > > > By setting none of the SAGAW bits we can indicate to a guest that DMA > > > translation isn't supported. Tested by booting Windows 10, as well as > > > Linux guests with the fix at https://git.kernel.org/torvalds/c/c40aaaac10 > > > > > > > > > Signed-off-by: David Woodhouse <dwmw@xxxxxxxxxxxx> > > > Reviewed-by: Peter Xu <peterx@xxxxxxxxxx> > > > Acked-by: Jason Wang <jasowang@xxxxxxxxxx> > > > > this is borderline like a feature, but ... > > It's the opposite of a feature — it's turning the feature *off* ;) Right. Still - do you believe it's appropriate in soft freeze and if yes why? > > > > > > @@ -3627,12 +3628,17 @@ static void vtd_init(IntelIOMMUState *s) > > > s->next_frcd_reg = 0; > > > s->cap = VTD_CAP_FRO | VTD_CAP_NFR | VTD_CAP_ND | > > > VTD_CAP_MAMV | VTD_CAP_PSI | VTD_CAP_SLLPS | > > > - VTD_CAP_SAGAW_39bit | VTD_CAP_MGAW(s->aw_bits); > > > + VTD_CAP_MGAW(s->aw_bits); > > > if (s->dma_drain) { > > > s->cap |= VTD_CAP_DRAIN; > > > } > > > - if (s->aw_bits == VTD_HOST_AW_48BIT) { > > > - s->cap |= VTD_CAP_SAGAW_48bit; > > > + if (s->dma_translation) { > > > + if (s->aw_bits >= VTD_HOST_AW_39BIT) { > > > + s->cap |= VTD_CAP_SAGAW_39bit; > > > + } > > > + if (s->aw_bits >= VTD_HOST_AW_48BIT) { > > > + s->cap |= VTD_CAP_SAGAW_48bit; > > > + } > > > } > > > s->ecap = VTD_ECAP_QI | VTD_ECAP_IRO; > > > > > > > > > ... this looks like you are actually fixing aw_bits < VTD_HOST_AW_39BIT, > > right? So maybe this patch is ok like this since it also fixes a > > bug. Pls add this to commit log though. > > Nah, aw_bits cannot be < VTD_HOST_AW_39BIT. We explicitly check in > vtd_decide_config(), and only 39 or 48 bits are supported, > corresponding to 3-level or 4-level page tables. Oh right. So not a bugfix then. > The only time we'd want to *not* advertise 39-bit support is if we > aren't advertising DMA translation at all. Which is the (anti-)feature > introduced by this patch. >
