On Mon, May 08, 2017 at 10:38:09AM +0000, Liu, Yi L wrote:
> On Thu, 27 Apr 2017 18:53:17 +0800
> Peter Xu <peterx@xxxxxxxxxx> wrote:
>
> > On Wed, Apr 26, 2017 at 06:06:33PM +0800, Liu, Yi L wrote:
> > > Expose "Shared Virtual Memory" to guest by using "svm" option.
> > > Also use "svm" to expose SVM related capabilities to guest.
> > > e.g. "-device intel-iommu, svm=on"
> > >
> > > Signed-off-by: Liu, Yi L <yi.l.liu@xxxxxxxxxxxxxxx>
> > > ---
> > > hw/i386/intel_iommu.c | 10 ++++++++++
> > > hw/i386/intel_iommu_internal.h | 5 +++++
> > > include/hw/i386/intel_iommu.h | 1 +
> > > 3 files changed, 16 insertions(+)
> > >
> > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index
> > > bf98fa5..ba1e7eb 100644
> > > --- a/hw/i386/intel_iommu.c
> > > +++ b/hw/i386/intel_iommu.c
> > > @@ -2453,6 +2453,7 @@ static Property vtd_properties[] = {
> > > DEFINE_PROP_BOOL("x-buggy-eim", IntelIOMMUState, buggy_eim, false),
> > > DEFINE_PROP_BOOL("caching-mode", IntelIOMMUState, caching_mode,
> > FALSE),
> > > DEFINE_PROP_BOOL("ecs", IntelIOMMUState, ecs, FALSE),
> > > + DEFINE_PROP_BOOL("svm", IntelIOMMUState, svm, FALSE),
> > > DEFINE_PROP_END_OF_LIST(),
> > > };
> > >
> > > @@ -2973,6 +2974,15 @@ static void vtd_init(IntelIOMMUState *s)
> > > s->ecap |= VTD_ECAP_ECS;
> > > }
> > >
> > > + if (s->svm) {
> > > + if (!s->ecs || !x86_iommu->pt_supported || !s->caching_mode) {
> > > + error_report("Need to set ecs, pt, caching-mode for svm");
> > > + exit(1);
> > > + }
> > > + s->cap |= VTD_CAP_DWD | VTD_CAP_DRD;
> > > + s->ecap |= VTD_ECAP_PRS | VTD_ECAP_PTS | VTD_ECAP_PASID28;
> > > + }
> > > +
> > > if (s->caching_mode) {
> > > s->cap |= VTD_CAP_CM;
> > > }
> > > diff --git a/hw/i386/intel_iommu_internal.h
> > > b/hw/i386/intel_iommu_internal.h index 71a1c1e..f2a7d12 100644
> > > --- a/hw/i386/intel_iommu_internal.h
> > > +++ b/hw/i386/intel_iommu_internal.h
> > > @@ -191,6 +191,9 @@
> > > #define VTD_ECAP_PT (1ULL << 6)
> > > #define VTD_ECAP_MHMV (15ULL << 20)
> > > #define VTD_ECAP_ECS (1ULL << 24)
> > > +#define VTD_ECAP_PASID28 (1ULL << 28)
> >
> > Could I ask what's this bit? On my spec, it says this bit is reserved and defunct (spec
> > version: June 2016).
>
> As Ashok confirmed, yes it should be bit 40. would update it.
Ok.
>
> > > +#define VTD_ECAP_PRS (1ULL << 29)
> > > +#define VTD_ECAP_PTS (0xeULL << 35)
> >
> > Would it better we avoid using 0xe here, or at least add some comment?
>
> For this value, it must be no more than the bits host supports. So it may be
> better to have a default value and meanwhile expose an option to let user
> set it. how about your opinion?
I think a more important point is that we need to make sure this value
is no larger than hardware support? Since you are also working on the
vfio interface for virt-svm... would it be possible that we can talk
to kernel in some way so that we can know the supported pasid size in
host IOMMU? So that when guest specifies something bigger, we can stop
the user.
I don't know the practical value for this field, if it's static
enough, I think it's also okay we make it static here as well. But
again, I would prefer at least some comment, like:
/* Value N indicates PASID field of N+1 bits, here 0xe stands for.. */
>
> >
> > >
> > > /* CAP_REG */
> > > /* (offset >> 4) << 24 */
> > > @@ -207,6 +210,8 @@
> > > #define VTD_CAP_PSI (1ULL << 39)
> > > #define VTD_CAP_SLLPS ((1ULL << 34) | (1ULL << 35))
> > > #define VTD_CAP_CM (1ULL << 7)
> > > +#define VTD_CAP_DWD (1ULL << 54)
> > > +#define VTD_CAP_DRD (1ULL << 55)
> >
> > Just to confirm: after this series, we should support drain read/write then, right?
>
> I haven’t done special process against it in IOMMU emulator. It's set to keep
> consistence with VT-d spec since DWD and DRW is required capability when
> PASID it reported as Set. However, I think it should be fine if guest issue QI
> with drain read/write set in the descriptor. Host should be able to process it.
I see. IIUC the point here is we need to deliver these requests to
host IOMMU, and I guess we need to be able to do this in a synchronous
way as well.
Thanks,
--
Peter Xu
