[PATCH v1 2/5] drm/i915/gvt: GVTg read_shared_page implementation

[Zhenyu Wang]

On 2018.11.06 05:33:06 +0000, Zhang, Xiaolin wrote:
> >> @@ -1252,6 +1252,11 @@ static int pvinfo_mmio_write(struct intel_vgpu *vgpu, unsigned int offset,
> >>  			vgpu_vreg(vgpu, offset) = 0;
> >>  		}
> >>  		break;
> >> +	case _vgtif_reg(shared_page_gpa.lo):
> >> +	case _vgtif_reg(shared_page_gpa.hi):
> >> +		vgpu->shared_page_gpa = vgpu_vreg64_t(vgpu,
> >> +			vgtif_reg(shared_page_gpa));
> > Looks you don't track if both lo and hi address has been updated properly?
> this register update is handled with the similar mechanism as pdp[4]
> register within vgt_if. but in guest, there is a check to make sure
> register updated correctly.  the code change as below for your reference.

We should always consider arbitrary guest behavior if you can't take
guest as secured, and do error check gracefully.

> +        gpa = __pa(dev_priv->vgpu.shared_page);
> +        __raw_i915_write32(dev_priv, vgtif_reg(shared_page_gpa.lo),
> +                lower_32_bits(gpa));
> +        __raw_i915_write32(dev_priv, vgtif_reg(shared_page_gpa.hi),
> +                upper_32_bits(gpa));
> +        if (gpa != __raw_i915_read64(dev_priv,
> +                vgtif_reg(shared_page_gpa))) {
> +            DRM_ERROR("vgpu: passed shared_page_gpa failed\n");
> +            free_page((unsigned long)dev_priv->vgpu.shared_page);
> +            dev_priv->vgpu.pv_caps = 0;
> +            return;
> +        }
> 
> >
> >> +		break;
> >>  	/* add xhot and yhot to handled list to avoid error log */
> >>  	case _vgtif_reg(cursor_x_hot):
> >>  	case _vgtif_reg(cursor_y_hot):
> >> diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
> >> index d1674db..44507c9 100644
> >> --- a/drivers/gpu/drm/i915/gvt/vgpu.c
> >> +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
> >> @@ -591,3 +591,17 @@ void intel_gvt_reset_vgpu(struct intel_vgpu *vgpu)
> >>  	intel_gvt_reset_vgpu_locked(vgpu, true, 0);
> >>  	mutex_unlock(&vgpu->vgpu_lock);
> >>  }
> >> +
> >> +/**
> >> + * intel_gvt_read_shared_page - read content from shared page
> >> + */
> >> +void intel_gvt_read_shared_page(struct intel_vgpu *vgpu,
> >> +		unsigned int offset, void *buf, unsigned long len)
> >> +{
> >> +	int ret = 0;
> >> +	unsigned long gpa = vgpu->shared_page_gpa + offset;
> >> +
> >> +	ret = intel_gvt_hypervisor_read_gpa(vgpu, gpa, buf, len);
> >> +	if (ret)
> >> +		gvt_vgpu_err("read shared page (offset %x) failed", offset);
> >> +}
> > No error handling to return? And if for invalid gpa, should we take guest
> > as invalid bad driver?
> >
> yes, for invalid gpa, we should take guest as invalid bad driver. will
> add offset check in next version to make sure gpa is illegal.
> 

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-gvt-dev/attachments/20181106/8ce8d672/attachment-0001.sig>