RE: [PATCH v2 15/22] intel_iommu: bind/unbind guest page table to host

[Liu, Yi L]

> From: Peter Xu <address@hidden>
> Sent: Friday, April 3, 2020 2:09 AM
> To: Liu, Yi L <address@hidden>
> Subject: Re: [PATCH v2 15/22] intel_iommu: bind/unbind guest page table to 
> host
> 
> On Sun, Mar 29, 2020 at 09:24:54PM -0700, Liu Yi L wrote:
> > +static int vtd_bind_guest_pasid(IntelIOMMUState *s, VTDBus *vtd_bus,
> > +                                int devfn, int pasid, VTDPASIDEntry *pe,
> > +                                VTDPASIDOp op) {
> > +    VTDHostIOMMUContext *vtd_dev_icx;
> > +    HostIOMMUContext *iommu_ctx;
> > +    DualIOMMUStage1BindData *bind_data;
> > +    struct iommu_gpasid_bind_data *g_bind_data;
> > +    int ret = -1;
> > +
> > +    vtd_dev_icx = vtd_bus->dev_icx[devfn];
> > +    if (!vtd_dev_icx) {
> > +        /* means no need to go further, e.g. for emulated devices */
> > +        return 0;
> > +    }
> > +
> > +    iommu_ctx = vtd_dev_icx->iommu_ctx;
> > +    if (!iommu_ctx) {
> > +        return -EINVAL;
> > +    }
> > +
> > +    if (!(iommu_ctx->stage1_formats
> > +             & IOMMU_PASID_FORMAT_INTEL_VTD)) {
> > +        error_report_once("IOMMU Stage 1 format is not compatible!\n");
> > +        return -EINVAL;
> > +    }
> > +
> > +    bind_data = g_malloc0(sizeof(*bind_data));
> > +    bind_data->pasid = pasid;
> > +    g_bind_data = &bind_data->bind_data.gpasid_bind;
> > +
> > +    g_bind_data->flags = 0;
> > +    g_bind_data->vtd.flags = 0;
> > +    switch (op) {
> > +    case VTD_PASID_BIND:
> > +        g_bind_data->version = IOMMU_UAPI_VERSION;
> > +        g_bind_data->format = IOMMU_PASID_FORMAT_INTEL_VTD;
> > +        g_bind_data->gpgd = vtd_pe_get_flpt_base(pe);
> > +        g_bind_data->addr_width = vtd_pe_get_fl_aw(pe);
> > +        g_bind_data->hpasid = pasid;
> > +        g_bind_data->gpasid = pasid;
> > +        g_bind_data->flags |= IOMMU_SVA_GPASID_VAL;
> > +        g_bind_data->vtd.flags =
> > +                             (VTD_SM_PASID_ENTRY_SRE_BIT(pe->val[2])
> > + ? 1 : 0)
> 
> This evaluates to 1 if VTD_SM_PASID_ENTRY_SRE_BIT(pe->val[2]), or 0.
> Do you want to use IOMMU_SVA_VTD_GPASID_SRE instead of 1?  Same question to
> all the rest.

oops, yes it is. you are right. thanks for catching it. During verification, 
only
the SRE bit is used, so it's not spotted in testing.

Regards,
Yi Liu