Re: [Qemu-devel] [PATCH v3 1/3] IOMMU: add VTD_CAP_CM to vIOMMU capability exposed to guest

[Peter Xu <peterx@redhat.com>]

On Sat, May 21, 2016 at 06:42:03PM +0200, Jan Kiszka wrote:
> On 2016-05-21 18:19, Aviv B.D wrote:
> > From: "Aviv Ben-David" <bd.aviv@gmail.com>
> > 
> > This flag tells the guest to invalidate tlb cache also after unmap operations.
> > 
> > Signed-off-by: Aviv Ben-David <bd.aviv@gmail.com>
> > ---
> >  hw/i386/intel_iommu.c          | 3 ++-
> >  hw/i386/intel_iommu_internal.h | 1 +
> >  2 files changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> > index 347718f..1af8da8 100644
> > --- a/hw/i386/intel_iommu.c
> > +++ b/hw/i386/intel_iommu.c
> > @@ -1949,7 +1949,8 @@ static void vtd_init(IntelIOMMUState *s)
> >      s->iq_last_desc_type = VTD_INV_DESC_NONE;
> >      s->next_frcd_reg = 0;
> >      s->cap = VTD_CAP_FRO | VTD_CAP_NFR | VTD_CAP_ND | VTD_CAP_MGAW |
> > -             VTD_CAP_SAGAW | VTD_CAP_MAMV | VTD_CAP_PSI | VTD_CAP_SLLPS;
> > +             VTD_CAP_SAGAW | VTD_CAP_MAMV | VTD_CAP_PSI | VTD_CAP_SLLPS |
> > +             VTD_CAP_CM;
> 
> Again, needs to be optional because not all guests will support it or
> behave differently when it's set (I've one that refuses to work).

There should be more than one way to make it optional. Which is
better? What I can think of:

(Assume we have Marcel's "-device intel_iommu" working already)

1. Let the CM bit optional, or say, we need to specify something like
   "-device intel_iommu,cmbit=on" or we will disable CM bit. If we
   have CM disabled but with VFIO device, let QEMU raise error.

2. We automatically detect whether we need CM bit. E.g., if we have
   VFIO and vIOMMU both enabled, we automatically set the bit. Another
   case is maybe we would in the future support nested vIOMMU? If so,
   we can do the same thing for the nested feature.

-- peterx