On Thu, Jun 02, 2016 at 03:14:36PM +0200, Jan Kiszka wrote:
> On 2016-06-02 15:00, Alex Williamson wrote:
> > On Thu, 2 Jun 2016 16:44:39 +0800
> > Peter Xu <pet...@redhat.com> wrote:
> >
> >> 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.a...@gmail.com>
> >>>>
> >>>> This flag tells the guest to invalidate tlb cache also after unmap
> >>>> operations.
> >>>>
> >>>> Signed-off-by: Aviv Ben-David <bd.a...@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.
> >
> >
> > Why do we need to support VT-d for guests that do not support CM=1?
I don't think we need to do this. Spec is rather clear on this point.
Just fix the guests.
Support for CM=0 might still be useful because CM=1 is not required if
device is restartable (most software devices would be).
I prefer cmbit=on by default, with cmbit=off, don't allow VFIO.
> > The VT-d spec indicates that software should be written to handle both
> > caching modes (6.1). Granted this is a *should* and not a *must*,
> > but can't we consider guests that do not support CM=1 incompatible with
> > emulated VT-d? If CM=0 needs to be supported then we need to shadow
> > all of the remapping structures since vfio effectively becomes a cache
> > of the that would otherwise depend on the invalidation of both present
> > and non-present entries. What guests do not support CM=1? Thanks,
>
> - there is at least one guest that does not support CM=1 yet (Jailhouse)
> - there might be more or there might be broken ones as hardware
> generally doesn't have CM=1, thus this case is typically untested
> - an AMD IOMMU (to my current understanding) will require shadowing
> anyway has it has no comparable concept,
I was rather sure this is it:
26 NpCache: not present table entries cached. RO. Reset Xb. 1=Indicates
that the IOMMU caches page table entries that are marked as not present.
When this bit is set, software must issue an invalidate after any change
to a PDE or PTE. 0=Indicates that the IOMMU caches only page table
entries that are marked as present. When NpCache is clear, software must
issue an invalidate after any change to a PDE or PTE marked present
before the change. Implementation note: For hardware implementations of
the IOMMU, this bit must be 0b.
this implementation seems to set this bit.
> thus we will eventually be
> able to use that strategy also for VT-d
>
> Jan
>
>
