Re: [PATCH 1/2] intel_iommu: Support IR-only mode without DMA translation

[Jason Wang]

On Fri, Dec 3, 2021 at 8:14 PM Peter Xu <peterx@redhat.com> wrote:
>
> On Fri, Dec 03, 2021 at 10:46:46AM +0000, David Woodhouse wrote:
> > On Fri, 2021-12-03 at 15:38 +0800, Peter Xu wrote:
> > > On Thu, Dec 02, 2021 at 11:49:25AM +0800, Jason Wang wrote:
> > > > On Thu, Dec 2, 2021 at 4:55 AM David Woodhouse <dwmw2@infradead.org> 
> > > > wrote:
> > > > > From: David Woodhouse <dwmw@amazon.co.uk>
> > > > >
> > > > > 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@amazon.co.uk>
> > > > > ---
> > > > >  hw/i386/intel_iommu.c         | 14 ++++++++++----
> > > > >  include/hw/i386/intel_iommu.h |  1 +
> > > > >  2 files changed, 11 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> > > > > index 294499ee20..ffc852d110 100644
> > > > > --- a/hw/i386/intel_iommu.c
> > > > > +++ b/hw/i386/intel_iommu.c
> > > > > @@ -2202,7 +2202,7 @@ static void 
> > > > > vtd_handle_gcmd_write(IntelIOMMUState *s)
> > > > >      uint32_t changed = status ^ val;
> > > > >
> > > > >      trace_vtd_reg_write_gcmd(status, val);
> > > > > -    if (changed & VTD_GCMD_TE) {
> > > > > +    if ((changed & VTD_GCMD_TE) && s->dma_translation) {
> > > > >          /* Translation enable/disable */
> > > > >          vtd_handle_gcmd_te(s, val & VTD_GCMD_TE);
> > > > >      }
> > > > > @@ -3100,6 +3100,7 @@ static Property vtd_properties[] = {
> > > > >      DEFINE_PROP_BOOL("caching-mode", IntelIOMMUState, caching_mode, 
> > > > > FALSE),
> > > > >      DEFINE_PROP_BOOL("x-scalable-mode", IntelIOMMUState, 
> > > > > scalable_mode, FALSE),
> > > > >      DEFINE_PROP_BOOL("dma-drain", IntelIOMMUState, dma_drain, true),
> > > > > +    DEFINE_PROP_BOOL("dma-translation", IntelIOMMUState, 
> > > > > dma_translation, true),
> > > > >      DEFINE_PROP_END_OF_LIST(),
> > > > >  };
> > > > >
> > > > > @@ -3605,12 +3606,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;
> > > > > +            }
> > > > >      }
> > > >
> > > > Just wonder if this is the hardware behaviour as I see 0 is reserved
> > > > for SAGAW in vtd 3.3 spec.
> > >
> > > Yes I have the same question.  But if latest Linux & Windows work fine 
> > > then it
> > > seems ok if we have explicit use scenario with enabling IR only.
> >
> > Bit zero is reserved. The *value* zero is just what you get when none
> > of the bits are set.
> >
> >       "A value of 1 in any of these bits indicates the corresponding
> >       adjusted guest address width is supported.The adjusted guest
> >       address widths corresponding to various bit positions within
> >       this field are:
> >
> >        • 0: Reserved
> >        • 1: 39-bit AGAW (3-level page-table)
> >        • 2: 48-bit AGAW (4-level page-table)
> >        • 3: 57-bit AGAW (5-level page-table)
> >        • 4: Reserved
> >
> >       Software must ensure that the adjusted guest address width used
> >       to set up the page tables is one of the supported guest address
> >       widths reported in this field.
> >
> >       Hardware implementations reporting second-level translation
> >       support (SLTS) field as Clear also report this field as 0.
>
> I see.

Right.

Acked-by: Jason Wang <jasowang@redhat.com>