[PATCH v2 3/3] drm/i915/gvt: add VFIO EDID region

Zhenyu Wang zhenyuw at linux.intel.com
Fri Jan 25 05:16:00 UTC 2019


On 2019.01.24 13:46:37 +0800, hang.yuan at linux.intel.com wrote:
> From: Hang Yuan <hang.yuan at linux.intel.com>
> 
> Implement VFIO EDID region for vgpu. Support EDID blob update and notify
> guest on link state change via hotplug event.
> 
> v2: add EDID sanity check and size update <zhenyu>
> 
> Tested-by: Gerd Hoffmann <kraxel at redhat.com>
> Signed-off-by: Hang Yuan <hang.yuan at linux.intel.com>
> ---

Looks fine to me, some little comments below.

>  drivers/gpu/drm/i915/gvt/gvt.h       |   7 ++
>  drivers/gpu/drm/i915/gvt/hypercall.h |   1 +
>  drivers/gpu/drm/i915/gvt/kvmgt.c     | 138 +++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/gvt/mpt.h       |  17 +++++
>  drivers/gpu/drm/i915/gvt/vgpu.c      |   6 ++
>  5 files changed, 169 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> index 8bce09d..5793619 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -127,6 +127,13 @@ struct intel_vgpu_opregion {
>  
>  #define vgpu_opregion(vgpu) (&(vgpu->opregion))
>  
> +#if IS_ENABLED(CONFIG_DRM_I915_GVT_KVMGT)
> +	struct intel_vgpu_edid_region {
> +		struct vfio_region_gfx_edid vfio_edid_regs;
> +		void *edid_blob;
> +	};
> +#endif

I think better just move this in kvmgt.c itself which is only used there.

> +
>  struct intel_vgpu_display {
>  	struct intel_vgpu_i2c_edid i2c_edid;
>  	struct intel_vgpu_port ports[I915_MAX_PORTS];
> diff --git a/drivers/gpu/drm/i915/gvt/hypercall.h b/drivers/gpu/drm/i915/gvt/hypercall.h
> index 831ab34..4862fb1 100644
> --- a/drivers/gpu/drm/i915/gvt/hypercall.h
> +++ b/drivers/gpu/drm/i915/gvt/hypercall.h
> @@ -67,6 +67,7 @@ struct intel_gvt_mpt {
>  	int (*set_trap_area)(unsigned long handle, u64 start, u64 end,
>  			     bool map);
>  	int (*set_opregion)(void *vgpu);
> +	int (*set_edid)(void *vgpu, int port_num);
>  	int (*get_vfio_device)(void *vgpu);
>  	void (*put_vfio_device)(void *vgpu);
>  	bool (*is_valid_gfn)(unsigned long handle, unsigned long gfn);
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 5bec0b5..ae88f17 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -57,6 +57,8 @@ static const struct intel_gvt_ops *intel_gvt_ops;
>  #define VFIO_PCI_INDEX_TO_OFFSET(index) ((u64)(index) << VFIO_PCI_OFFSET_SHIFT)
>  #define VFIO_PCI_OFFSET_MASK    (((u64)(1) << VFIO_PCI_OFFSET_SHIFT) - 1)
>  
> +#define EDID_BLOB_OFFSET (PAGE_SIZE/2)
> +
>  #define OPREGION_SIGNATURE "IntelGraphicsMem"
>  
>  struct vfio_region;
> @@ -427,6 +429,111 @@ static const struct intel_vgpu_regops intel_vgpu_regops_opregion = {
>  	.release = intel_vgpu_reg_release_opregion,
>  };
>  
> +static int handle_edid_regs(struct intel_vgpu *vgpu,
> +			struct intel_vgpu_edid_region *region, char *buf,
> +			size_t count, u16 offset, bool is_write)
> +{
> +	struct vfio_region_gfx_edid *regs = &region->vfio_edid_regs;
> +	unsigned int data;
> +
> +	if (offset + count > sizeof(*regs))
> +		return -EINVAL;
> +
> +	if (count != 4)
> +		return -EINVAL;
> +
> +	if (is_write) {
> +		data = *((unsigned int *)buf);
> +		switch (offset) {
> +		case offsetof(struct vfio_region_gfx_edid, link_state):
> +			if (data == VFIO_DEVICE_GFX_LINK_STATE_UP) {
> +				if (!drm_edid_block_valid(
> +					(u8 *)region->edid_blob,
> +					0,
> +					true,
> +					NULL)) {
> +					gvt_vgpu_err("invalid EDID blob\n");
> +					return -EINVAL;
> +				}

Might just write if (data == ...STATE_UP && !drm_edid_block_valid(...)) return -EINVAL;

> +				intel_gvt_ops->emulate_hotplug(vgpu, true);
> +			} else if (data == VFIO_DEVICE_GFX_LINK_STATE_DOWN)
> +				intel_gvt_ops->emulate_hotplug(vgpu, false);
> +			else {
> +				gvt_vgpu_err("invalid EDID link state %d\n",
> +					regs->link_state);
> +				return -EINVAL;
> +			}
> +			regs->link_state = data;
> +			break;
> +		case offsetof(struct vfio_region_gfx_edid, edid_size):
> +			if (data > regs->edid_max_size) {
> +				gvt_vgpu_err("EDID size is bigger than %d!\n",
> +					regs->edid_max_size);
> +				return -EINVAL;
> +			}
> +			regs->edid_size = data;
> +			break;
> +		default:
> +			/* read-only regs */
> +			gvt_vgpu_err("write read-only EDID region at offset %d\n",
> +				offset);
> +			return -EPERM;
> +		}
> +	} else {
> +		memcpy(buf, (char *)regs + offset, count);
> +	}
> +
> +	return count;
> +}
> +
> +static int handle_edid_blob(struct intel_vgpu_edid_region *region, char *buf,
> +			size_t count, u16 offset, bool is_write)
> +{
> +	if (offset + count > region->vfio_edid_regs.edid_size)
> +		return -EINVAL;
> +
> +	if (is_write)
> +		memcpy(region->edid_blob + offset, buf, count);
> +	else
> +		memcpy(buf, region->edid_blob + offset, count);
> +
> +	return count;
> +}
> +
> +static size_t intel_vgpu_reg_rw_edid(struct intel_vgpu *vgpu, char *buf,
> +		size_t count, loff_t *ppos, bool iswrite)
> +{
> +	int ret;
> +	unsigned int i = VFIO_PCI_OFFSET_TO_INDEX(*ppos) -
> +			VFIO_PCI_NUM_REGIONS;
> +	struct intel_vgpu_edid_region *region =
> +		(struct intel_vgpu_edid_region *)vgpu->vdev.region[i].data;
> +	loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
> +
> +	if (pos < region->vfio_edid_regs.edid_offset) {
> +		ret = handle_edid_regs(vgpu, region, buf, count, pos, iswrite);
> +	} else {
> +		pos -= EDID_BLOB_OFFSET;
> +		ret = handle_edid_blob(region, buf, count, pos, iswrite);
> +	}
> +
> +	if (ret < 0)
> +		gvt_vgpu_err("failed to access EDID region\n");
> +
> +	return ret;
> +}
> +
> +static void intel_vgpu_reg_release_edid(struct intel_vgpu *vgpu,
> +					struct vfio_region *region)
> +{
> +	kfree(region->data);
> +}
> +
> +static const struct intel_vgpu_regops intel_vgpu_regops_edid = {
> +	.rw = intel_vgpu_reg_rw_edid,
> +	.release = intel_vgpu_reg_release_edid,
> +};
> +
>  static int intel_vgpu_register_reg(struct intel_vgpu *vgpu,
>  		unsigned int type, unsigned int subtype,
>  		const struct intel_vgpu_regops *ops,
> @@ -493,6 +600,36 @@ static int kvmgt_set_opregion(void *p_vgpu)
>  	return ret;
>  }
>  
> +static int kvmgt_set_edid(void *p_vgpu, int port_num)
> +{
> +	struct intel_vgpu *vgpu = (struct intel_vgpu *)p_vgpu;
> +	struct intel_vgpu_port *port = intel_vgpu_port(vgpu, port_num);
> +	struct intel_vgpu_edid_region *base;
> +	int ret;
> +
> +	base = kzalloc(sizeof(*base), GFP_KERNEL);
> +	if (!base)
> +		return -ENOMEM;
> +
> +	/* TODO: Add multi-port and EDID extension block support */
> +	base->vfio_edid_regs.edid_offset = EDID_BLOB_OFFSET;
> +	base->vfio_edid_regs.edid_max_size = EDID_SIZE;
> +	base->vfio_edid_regs.edid_size = EDID_SIZE;
> +	base->vfio_edid_regs.max_xres = vgpu_edid_xres(port->id);
> +	base->vfio_edid_regs.max_yres = vgpu_edid_yres(port->id);
> +	base->edid_blob = port->edid->edid_block;
> +
> +	ret = intel_vgpu_register_reg(vgpu,
> +			VFIO_REGION_TYPE_GFX,
> +			VFIO_REGION_SUBTYPE_GFX_EDID,
> +			&intel_vgpu_regops_edid, EDID_SIZE,
> +			VFIO_REGION_INFO_FLAG_READ |
> +			VFIO_REGION_INFO_FLAG_WRITE |
> +			VFIO_REGION_INFO_FLAG_CAPS, base);
> +
> +	return ret;
> +}
> +
>  static void kvmgt_put_vfio_device(void *vgpu)
>  {
>  	if (WARN_ON(!((struct intel_vgpu *)vgpu)->vdev.vfio_device))
> @@ -1896,6 +2033,7 @@ static struct intel_gvt_mpt kvmgt_mpt = {
>  	.dma_map_guest_page = kvmgt_dma_map_guest_page,
>  	.dma_unmap_guest_page = kvmgt_dma_unmap_guest_page,
>  	.set_opregion = kvmgt_set_opregion,
> +	.set_edid = kvmgt_set_edid,
>  	.get_vfio_device = kvmgt_get_vfio_device,
>  	.put_vfio_device = kvmgt_put_vfio_device,
>  	.is_valid_gfn = kvmgt_is_valid_gfn,
> diff --git a/drivers/gpu/drm/i915/gvt/mpt.h b/drivers/gpu/drm/i915/gvt/mpt.h
> index 5b5995a..0f94401 100644
> --- a/drivers/gpu/drm/i915/gvt/mpt.h
> +++ b/drivers/gpu/drm/i915/gvt/mpt.h
> @@ -314,6 +314,23 @@ static inline int intel_gvt_hypervisor_set_opregion(struct intel_vgpu *vgpu)
>  }
>  
>  /**
> + * intel_gvt_hypervisor_set_edid - Set EDID region for guest
> + * @vgpu: a vGPU
> + * @port_num: display port number
> + *
> + * Returns:
> + * Zero on success, negative error code if failed.
> + */
> +static inline int intel_gvt_hypervisor_set_edid(struct intel_vgpu *vgpu,
> +						int port_num)
> +{
> +	if (!intel_gvt_host.mpt->set_edid)
> +		return 0;
> +
> +	return intel_gvt_host.mpt->set_edid(vgpu, port_num);
> +}
> +
> +/**
>   * intel_gvt_hypervisor_get_vfio_device - increase vfio device ref count
>   * @vgpu: a vGPU
>   *
> diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
> index e1c860f8..720e2b1 100644
> --- a/drivers/gpu/drm/i915/gvt/vgpu.c
> +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
> @@ -428,6 +428,12 @@ static struct intel_vgpu *__intel_gvt_create_vgpu(struct intel_gvt *gvt,
>  	if (ret)
>  		goto out_clean_sched_policy;
>  
> +	/*TODO: add more platforms support */
> +	if (IS_SKYLAKE(gvt->dev_priv) || IS_KABYLAKE(gvt->dev_priv))
> +		ret = intel_gvt_hypervisor_set_edid(vgpu, PORT_D);
> +	if (ret)
> +		goto out_clean_sched_policy;
> +
>  	return vgpu;
>  
>  out_clean_sched_policy:
> -- 
> 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/20190125/ebd5c21d/attachment.sig>


More information about the intel-gvt-dev mailing list