[PATCH v2 3/3] drm/i915/gvt: add VFIO EDID region
Hang Yuan
hang.yuan at linux.intel.com
Mon Jan 28 10:12:08 UTC 2019
On 1/25/19 1:16 PM, Zhenyu Wang wrote:
> 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.
Henry: Will address it in next version. Thanks.
>> +
>> 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 = ®ion->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;
Henry: Sounds like it will not reduce the number of 'if'. Then still
need to check if (data == ...STATE_UP && drm_edid_block_valid(...))
emulate_hotplug();
>> + 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;
>> +}
>> +
More information about the intel-gvt-dev
mailing list