[PATCH] drm/i915/gvt: clean up vGPU resource r/w length

[Dong, Chuanxiao]

> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces at lists.freedesktop.org] On
> Behalf Of Zhenyu Wang
> Sent: Wednesday, August 2, 2017 1:27 PM
> To: intel-gvt-dev at lists.freedesktop.org
> Cc: Zhang, Xiong Y <xiong.y.zhang at intel.com>
> Subject: [PATCH] drm/i915/gvt: clean up vGPU resource r/w length
> 
> We can do max of 64bit for vGPU resource e.g MMIO r/w access, but
> original KVMGT code just always force to do max of 32bit access.
> This cleans up for old duplicated code block and allows for 64bit access in
> one call. Also remove unnecessary resource length check for both MMIO and
> config space access.

Hi Zhenyu,

Looks like this can fix an regression issue we found? If so, do you think if we need a Fixes line here for cherrypick by the stable kernels?

Thanks
Chuanxiao
> 
> Cc: Xiong Zhang <xiong.y.zhang at intel.com>
> Signed-off-by: Zhenyu Wang <zhenyuw at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/gvt/cfg_space.c |  6 ---
> drivers/gpu/drm/i915/gvt/handlers.c  |  3 --
>  drivers/gpu/drm/i915/gvt/kvmgt.c     | 86 +++++++++---------------------------
>  3 files changed, 22 insertions(+), 73 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/cfg_space.c
> b/drivers/gpu/drm/i915/gvt/cfg_space.c
> index 40af17ec6312..7cf90db5ec00 100644
> --- a/drivers/gpu/drm/i915/gvt/cfg_space.c
> +++ b/drivers/gpu/drm/i915/gvt/cfg_space.c
> @@ -98,9 +98,6 @@ static void vgpu_pci_cfg_mem_write(struct intel_vgpu
> *vgpu, unsigned int off,  int intel_vgpu_emulate_cfg_read(struct intel_vgpu
> *vgpu, unsigned int offset,
>  	void *p_data, unsigned int bytes)
>  {
> -	if (WARN_ON(bytes > 4))
> -		return -EINVAL;
> -
>  	if (WARN_ON(offset + bytes > INTEL_GVT_MAX_CFG_SPACE_SZ))
>  		return -EINVAL;
> 
> @@ -285,9 +282,6 @@ int intel_vgpu_emulate_cfg_write(struct intel_vgpu
> *vgpu, unsigned int offset,  {
>  	int ret;
> 
> -	if (WARN_ON(bytes > 4))
> -		return -EINVAL;
> -
>  	if (WARN_ON(offset + bytes > INTEL_GVT_MAX_CFG_SPACE_SZ))
>  		return -EINVAL;
> 
> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c
> b/drivers/gpu/drm/i915/gvt/handlers.c
> index d85264d48585..b8efda3374fa 100644
> --- a/drivers/gpu/drm/i915/gvt/handlers.c
> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> @@ -3034,9 +3034,6 @@ int intel_vgpu_mmio_reg_rw(struct intel_vgpu
> *vgpu, unsigned int offset,
>  	gvt_mmio_func func;
>  	int ret;
> 
> -	if (WARN_ON(bytes > 4))
> -		return -EINVAL;
> -
>  	/*
>  	 * Handle special MMIO blocks.
>  	 */
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
> b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index fd0c85f9ef3c..9e8e3562bdef 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -692,47 +692,26 @@ static ssize_t intel_vgpu_read(struct mdev_device
> *mdev, char __user *buf,  {
>  	unsigned int done = 0;
>  	int ret;
> +	char tmp[8];
> 
>  	while (count) {
>  		size_t filled;
> 
> -		if (count >= 4 && !(*ppos % 4)) {
> -			u32 val;
> -
> -			ret = intel_vgpu_rw(mdev, (char *)&val, sizeof(val),
> -					ppos, false);
> -			if (ret <= 0)
> -				goto read_err;
> -
> -			if (copy_to_user(buf, &val, sizeof(val)))
> -				goto read_err;
> -
> +		if (count >= 8 && !(*ppos % 8)) {
> +			filled = 8;
> +		} else if (count >= 4 && !(*ppos % 4)) {
>  			filled = 4;
>  		} else if (count >= 2 && !(*ppos % 2)) {
> -			u16 val;
> -
> -			ret = intel_vgpu_rw(mdev, (char *)&val, sizeof(val),
> -					ppos, false);
> -			if (ret <= 0)
> -				goto read_err;
> -
> -			if (copy_to_user(buf, &val, sizeof(val)))
> -				goto read_err;
> -
>  			filled = 2;
> -		} else {
> -			u8 val;
> -
> -			ret = intel_vgpu_rw(mdev, &val, sizeof(val), ppos,
> -					false);
> -			if (ret <= 0)
> -				goto read_err;
> +		} else
> +			filled = 1;
> 
> -			if (copy_to_user(buf, &val, sizeof(val)))
> -				goto read_err;
> +		ret = intel_vgpu_rw(mdev, tmp, filled, ppos, false);
> +		if (ret <= 0)
> +			goto read_err;
> 
> -			filled = 1;
> -		}
> +		if (copy_to_user(buf, tmp, filled))
> +			goto read_err;
> 
>  		count -= filled;
>  		done += filled;
> @@ -752,47 +731,26 @@ static ssize_t intel_vgpu_write(struct mdev_device
> *mdev,  {
>  	unsigned int done = 0;
>  	int ret;
> +	char val[8];
> 
>  	while (count) {
>  		size_t filled;
> 
> -		if (count >= 4 && !(*ppos % 4)) {
> -			u32 val;
> -
> -			if (copy_from_user(&val, buf, sizeof(val)))
> -				goto write_err;
> -
> -			ret = intel_vgpu_rw(mdev, (char *)&val, sizeof(val),
> -					ppos, true);
> -			if (ret <= 0)
> -				goto write_err;
> -
> +		if (count >= 8 && !(*ppos % 8)) {
> +			filled = 8;
> +		} else if (count >= 4 && !(*ppos % 4)) {
>  			filled = 4;
>  		} else if (count >= 2 && !(*ppos % 2)) {
> -			u16 val;
> -
> -			if (copy_from_user(&val, buf, sizeof(val)))
> -				goto write_err;
> -
> -			ret = intel_vgpu_rw(mdev, (char *)&val,
> -					sizeof(val), ppos, true);
> -			if (ret <= 0)
> -				goto write_err;
> -
>  			filled = 2;
> -		} else {
> -			u8 val;
> -
> -			if (copy_from_user(&val, buf, sizeof(val)))
> -				goto write_err;
> +		} else
> +			filled = 1;
> 
> -			ret = intel_vgpu_rw(mdev, &val, sizeof(val),
> -					ppos, true);
> -			if (ret <= 0)
> -				goto write_err;
> +		if (copy_from_user(val, buf, filled))
> +			goto write_err;
> 
> -			filled = 1;
> -		}
> +		ret = intel_vgpu_rw(mdev, val, filled, ppos, true);
> +		if (ret <= 0)
> +			goto write_err;
> 
>  		count -= filled;
>  		done += filled;
> --
> 2.13.2
> 
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev