[PATCH] drm/i915/gvt: Fix aperture read/write emulation when enable x-no-mmap=on

Zhenyu Wang zhenyuw at linux.intel.com
Tue Jan 30 02:56:30 UTC 2018


On 2018.01.26 14:58:48 +0800, changbin.du at intel.com wrote:
> From: Changbin Du <changbin.du at intel.com>
> 
> When add 'x-no-mmap=on' for vfio-pci option, aperture access in guest
> is emulated. But the vgpu_aperture_rw() function take wrong offset when
> do memcpy, since vgpu->gm.aperture_va is not the base of entire aperture.
> This mistake cause GPU command in guest get lost and so the seqno is not
> updated in engine HWSP.
> 
> This patch fix this, and it also move the emulation code to kvmgt.
> Because only vfio need to emulate it. Put aperture rw to MMIO emulation
> path breaks assumptions in xengt.
> 
> Fixes: f090a00df9ec ("drm/i915/gvt: Add emulation for BAR2 (aperture) with normal file RW approach")
> Signed-off-by: Changbin Du <changbin.du at intel.com>
> ---
>  drivers/gpu/drm/i915/gvt/cfg_space.c | 15 +------------
>  drivers/gpu/drm/i915/gvt/gvt.h       |  1 -
>  drivers/gpu/drm/i915/gvt/kvmgt.c     | 36 +++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/gvt/mmio.c      | 42 ------------------------------------
>  4 files changed, 35 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/cfg_space.c b/drivers/gpu/drm/i915/gvt/cfg_space.c
> index 97bfc00..c62346f 100644
> --- a/drivers/gpu/drm/i915/gvt/cfg_space.c
> +++ b/drivers/gpu/drm/i915/gvt/cfg_space.c
> @@ -119,16 +119,6 @@ static int map_aperture(struct intel_vgpu *vgpu, bool map)
>  	if (map == vgpu->cfg_space.bar[INTEL_GVT_PCI_BAR_APERTURE].tracked)
>  		return 0;
>  
> -	if (map) {
> -		vgpu->gm.aperture_va = memremap(aperture_pa, aperture_sz,
> -						MEMREMAP_WC);
> -		if (!vgpu->gm.aperture_va)
> -			return -ENOMEM;
> -	} else {
> -		memunmap(vgpu->gm.aperture_va);
> -		vgpu->gm.aperture_va = NULL;
> -	}
> -
>  	val = vgpu_cfg_space(vgpu)[PCI_BASE_ADDRESS_2];
>  	if (val & PCI_BASE_ADDRESS_MEM_TYPE_64)
>  		val = *(u64 *)(vgpu_cfg_space(vgpu) + PCI_BASE_ADDRESS_2);
> @@ -141,11 +131,8 @@ static int map_aperture(struct intel_vgpu *vgpu, bool map)
>  						  aperture_pa >> PAGE_SHIFT,
>  						  aperture_sz >> PAGE_SHIFT,
>  						  map);
> -	if (ret) {
> -		memunmap(vgpu->gm.aperture_va);
> -		vgpu->gm.aperture_va = NULL;
> +	if (ret)
>  		return ret;
> -	}
>  
>  	vgpu->cfg_space.bar[INTEL_GVT_PCI_BAR_APERTURE].tracked = map;
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> index 7dc7a80..84b04f6 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -82,7 +82,6 @@ struct intel_gvt_device_info {
>  struct intel_vgpu_gm {
>  	u64 aperture_sz;
>  	u64 hidden_sz;
> -	void *aperture_va;
>  	struct drm_mm_node low_gm_node;
>  	struct drm_mm_node high_gm_node;
>  };
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 70f03e9..3c68727 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -651,6 +651,39 @@ static int intel_vgpu_bar_rw(struct intel_vgpu *vgpu, int bar, uint64_t off,
>  	return ret;
>  }
>  
> +static inline bool intel_vgpu_in_aperture(struct intel_vgpu *vgpu, uint64_t off)
> +{
> +	return off >= vgpu_aperture_offset(vgpu) &&
> +	       off < vgpu_aperture_offset(vgpu) + vgpu_aperture_sz(vgpu);
> +}
> +
> +static int intel_vgpu_aperture_rw(struct intel_vgpu *vgpu, uint64_t off,
> +		void *buf, unsigned long count, bool is_write)
> +{
> +	void *aperture_va;
> +
> +	if (!intel_vgpu_in_aperture(vgpu, off) ||
> +	    !intel_vgpu_in_aperture(vgpu, off + count)) {
> +		gvt_vgpu_err("Invalid aperture offset %llu\n", off);
> +		return -EINVAL;
> +	}
> +
> +	aperture_va = io_mapping_map_wc(&vgpu->gvt->dev_priv->ggtt.iomap,
> +					ALIGN_DOWN(off, PAGE_SIZE),
> +					PAGE_ALIGN(count + offset_in_page(off)));

I don't think we need page aligned size? Other looks fine to me.

> +	if (!aperture_va)
> +		return -EIO;
> +
> +	if (is_write)
> +		memcpy(aperture_va + offset_in_page(off), buf, count);
> +	else
> +		memcpy(buf, aperture_va + offset_in_page(off), count);
> +
> +	io_mapping_unmap(aperture_va);
> +
> +	return 0;
> +}
> +
>  static ssize_t intel_vgpu_rw(struct mdev_device *mdev, char *buf,
>  			size_t count, loff_t *ppos, bool is_write)
>  {
> @@ -679,8 +712,7 @@ static ssize_t intel_vgpu_rw(struct mdev_device *mdev, char *buf,
>  					buf, count, is_write);
>  		break;
>  	case VFIO_PCI_BAR2_REGION_INDEX:
> -		ret = intel_vgpu_bar_rw(vgpu, PCI_BASE_ADDRESS_2, pos,
> -					buf, count, is_write);
> +		ret = intel_vgpu_aperture_rw(vgpu, pos, buf, count, is_write);
>  		break;
>  	case VFIO_PCI_BAR1_REGION_INDEX:
>  	case VFIO_PCI_BAR3_REGION_INDEX:
> diff --git a/drivers/gpu/drm/i915/gvt/mmio.c b/drivers/gpu/drm/i915/gvt/mmio.c
> index 562b5ad..5c869e3 100644
> --- a/drivers/gpu/drm/i915/gvt/mmio.c
> +++ b/drivers/gpu/drm/i915/gvt/mmio.c
> @@ -56,38 +56,6 @@ int intel_vgpu_gpa_to_mmio_offset(struct intel_vgpu *vgpu, u64 gpa)
>  	(reg >= gvt->device_info.gtt_start_offset \
>  	 && reg < gvt->device_info.gtt_start_offset + gvt_ggtt_sz(gvt))
>  
> -static bool vgpu_gpa_is_aperture(struct intel_vgpu *vgpu, uint64_t gpa)
> -{
> -	u64 aperture_gpa = intel_vgpu_get_bar_gpa(vgpu, PCI_BASE_ADDRESS_2);
> -	u64 aperture_sz = vgpu_aperture_sz(vgpu);
> -
> -	return gpa >= aperture_gpa && gpa < aperture_gpa + aperture_sz;
> -}
> -
> -static int vgpu_aperture_rw(struct intel_vgpu *vgpu, uint64_t gpa,
> -			    void *pdata, unsigned int size, bool is_read)
> -{
> -	u64 aperture_gpa = intel_vgpu_get_bar_gpa(vgpu, PCI_BASE_ADDRESS_2);
> -	u64 offset = gpa - aperture_gpa;
> -
> -	if (!vgpu_gpa_is_aperture(vgpu, gpa + size - 1)) {
> -		gvt_vgpu_err("Aperture rw out of range, offset %llx, size %d\n",
> -			     offset, size);
> -		return -EINVAL;
> -	}
> -
> -	if (!vgpu->gm.aperture_va) {
> -		gvt_vgpu_err("BAR is not enabled\n");
> -		return -ENXIO;
> -	}
> -
> -	if (is_read)
> -		memcpy(pdata, vgpu->gm.aperture_va + offset, size);
> -	else
> -		memcpy(vgpu->gm.aperture_va + offset, pdata, size);
> -	return 0;
> -}
> -
>  static void failsafe_emulate_mmio_rw(struct intel_vgpu *vgpu, uint64_t pa,
>  		void *p_data, unsigned int bytes, bool read)
>  {
> @@ -144,11 +112,6 @@ int intel_vgpu_emulate_mmio_read(struct intel_vgpu *vgpu, uint64_t pa,
>  	}
>  	mutex_lock(&gvt->lock);
>  
> -	if (vgpu_gpa_is_aperture(vgpu, pa)) {
> -		ret = vgpu_aperture_rw(vgpu, pa, p_data, bytes, true);
> -		goto out;
> -	}
> -
>  	offset = intel_vgpu_gpa_to_mmio_offset(vgpu, pa);
>  
>  	if (WARN_ON(bytes > 8))
> @@ -222,11 +185,6 @@ int intel_vgpu_emulate_mmio_write(struct intel_vgpu *vgpu, uint64_t pa,
>  
>  	mutex_lock(&gvt->lock);
>  
> -	if (vgpu_gpa_is_aperture(vgpu, pa)) {
> -		ret = vgpu_aperture_rw(vgpu, pa, p_data, bytes, false);
> -		goto out;
> -	}
> -
>  	offset = intel_vgpu_gpa_to_mmio_offset(vgpu, pa);
>  
>  	if (WARN_ON(bytes > 8))
> -- 
> 2.7.4
> 
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

-- 
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/20180130/672e445a/attachment.sig>


More information about the intel-gvt-dev mailing list