> -----Original Message----- > From: Chen, Xiaoguang > Sent: Thursday, May 18, 2017 7:20 PM > To: He, Min <min.he@xxxxxxxxx>; alex.williamson@xxxxxxxxxx; > kraxel@xxxxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx; zhenyuw@xxxxxxxxxxxxxxx; Lv, Zhiyuan > <zhiyuan.lv@xxxxxxxxx>; intel-gvt-dev@xxxxxxxxxxxxxxxxxxxxx; Wang, Zhi A > <zhi.a.wang@xxxxxxxxx>; Tian, Kevin <kevin.tian@xxxxxxxxx> > Cc: Niu, Bing <bing.niu@xxxxxxxxx> > Subject: RE: [PATCH v2 2/5] drm/i915/gvt: OpRegion support for GVT-g > > Hi min, > > >-----Original Message----- > >From: He, Min > >Sent: Thursday, May 18, 2017 11:44 PM > >To: Chen, Xiaoguang <xiaoguang.chen@xxxxxxxxx>; > >alex.williamson@xxxxxxxxxx; kraxel@xxxxxxxxxx; intel- > >gfx@xxxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > >zhenyuw@xxxxxxxxxxxxxxx; Lv, Zhiyuan <zhiyuan.lv@xxxxxxxxx>; intel-gvt- > >dev@xxxxxxxxxxxxxxxxxxxxx; Wang, Zhi A <zhi.a.wang@xxxxxxxxx>; Tian, Kevin > ><kevin.tian@xxxxxxxxx> > >Cc: Niu, Bing <bing.niu@xxxxxxxxx>; Chen, Xiaoguang > ><xiaoguang.chen@xxxxxxxxx> > >Subject: RE: [PATCH v2 2/5] drm/i915/gvt: OpRegion support for GVT-g > > > >Xiaoguang, > >I have some comments inline. Thanks. > > > >> -----Original Message----- > >> From: intel-gvt-dev > >> [mailto:intel-gvt-dev-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of > >> Xiaoguang Chen > >> Sent: Thursday, May 18, 2017 2:50 AM > >> To: alex.williamson@xxxxxxxxxx; kraxel@xxxxxxxxxx; intel- > >> gfx@xxxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > >> zhenyuw@xxxxxxxxxxxxxxx; Lv, Zhiyuan <zhiyuan.lv@xxxxxxxxx>; > >> intel-gvt- dev@xxxxxxxxxxxxxxxxxxxxx; Wang, Zhi A > >> <zhi.a.wang@xxxxxxxxx>; Tian, Kevin <kevin.tian@xxxxxxxxx> > >> Cc: Niu, Bing <bing.niu@xxxxxxxxx>; Chen, Xiaoguang > >> <xiaoguang.chen@xxxxxxxxx> > >> Subject: [PATCH v2 2/5] drm/i915/gvt: OpRegion support for GVT-g > >> > >> OpRegion is needed to support display related operation for intel > >> vgpu. > >> > >> A vfio device region is added to intel vgpu to deliver the host > >> OpRegion information to user space so user space can construct the > >> OpRegion for vgpu. > >> > >> Signed-off-by: Bing Niu <bing.niu@xxxxxxxxx> > >> Signed-off-by: Xiaoguang Chen <xiaoguang.chen@xxxxxxxxx> > >> --- > >> drivers/gpu/drm/i915/gvt/kvmgt.c | 97 > >> +++++++++++++++++++++++++++++++++++++ > >> drivers/gpu/drm/i915/gvt/opregion.c | 12 ++++- > >> 2 files changed, 107 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c > >> b/drivers/gpu/drm/i915/gvt/kvmgt.c > >> index 3c6a02b..389f072 100644 > >> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c > >> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c > >> @@ -53,6 +53,8 @@ > >> #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 OPREGION_SIGNATURE "IntelGraphicsMem" > >> + > >> struct vfio_region; > >> struct intel_vgpu_regops { > >> size_t (*rw)(struct intel_vgpu *vgpu, char *buf, @@ -436,6 +438,92 > >> @@ static void kvmgt_protect_table_del(struct kvmgt_guest_info *info, > >> } > >> } > >> > >> +static size_t intel_vgpu_reg_rw_opregion(struct intel_vgpu *vgpu, char *buf, > >> + size_t count, loff_t *ppos, bool iswrite) { > >> + unsigned int i = VFIO_PCI_OFFSET_TO_INDEX(*ppos) - > >> + VFIO_PCI_NUM_REGIONS; > >> + void *base = vgpu->vdev.region[i].data; > >> + loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK; > >> + > >> + if (pos >= vgpu->vdev.region[i].size || iswrite) { > >> + gvt_vgpu_err("invalid op or offset for Intel vgpu OpRegion\n"); > >> + return -EINVAL; > >> + } > >> + count = min(count, (size_t)(vgpu->vdev.region[i].size - pos)); > >> + memcpy(buf, base + pos, count); > >> + > >> + return count; > >> +} > >> + > >> +static void intel_vgpu_reg_release_opregion(struct intel_vgpu *vgpu, > >> + struct vfio_region *region) > >> +{ > >> + memunmap(region->data); > >> +} > >> + > >> +static const struct intel_vgpu_regops intel_vgpu_regops_opregion = { > >> + .rw = intel_vgpu_reg_rw_opregion, > >> + .release = intel_vgpu_reg_release_opregion, }; > >> + > >> +static int intel_vgpu_register_reg(struct intel_vgpu *vgpu, > >> + unsigned int type, unsigned int subtype, > >> + const struct intel_vgpu_regops *ops, > >> + size_t size, u32 flags, void *data) { > >> + struct vfio_region *region; > >> + > >> + region = krealloc(vgpu->vdev.region, > >> + (vgpu->vdev.num_regions + 1) * sizeof(*region), > >> + GFP_KERNEL); > >> + if (!region) > >> + return -ENOMEM; > >> + > >> + vgpu->vdev.region = region; > >> + vgpu->vdev.region[vgpu->vdev.num_regions].type = type; > >> + vgpu->vdev.region[vgpu->vdev.num_regions].subtype = subtype; > >> + vgpu->vdev.region[vgpu->vdev.num_regions].ops = ops; > >> + vgpu->vdev.region[vgpu->vdev.num_regions].size = size; > >> + vgpu->vdev.region[vgpu->vdev.num_regions].flags = flags; > >> + vgpu->vdev.region[vgpu->vdev.num_regions].data = data; > >> + vgpu->vdev.num_regions++; > >> + > >> + return 0; > >> +} > >> + > >> +static int intel_vgpu_reg_init_opregion(struct intel_vgpu *vgpu) { > >> + unsigned int addr; > >> + void *base; > >> + int ret; > >> + > >> + addr = vgpu->gvt->opregion.opregion_pa; > >> + if (!addr || !(~addr)) > >> + return -ENODEV; > >> + > >> + base = memremap(addr, OPREGION_SIZE, MEMREMAP_WB); > >> + if (!base) > >> + return -ENOMEM; > >> + > >> + if (memcmp(base, OPREGION_SIGNATURE, 16)) { > >> + memunmap(base); > >> + return -EINVAL; > >> + } > >> + > >> + ret = intel_vgpu_register_reg(vgpu, > >> + PCI_VENDOR_ID_INTEL | > >> VFIO_REGION_TYPE_PCI_VENDOR_TYPE, > >> + VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION, > >> + &intel_vgpu_regops_opregion, OPREGION_SIZE, > >> + VFIO_REGION_INFO_FLAG_READ, base); > >In current GVT code, we already have a function init_vgpu_opregion, which will > >copy the physical opregion into a per vgpu structure: > >vgpu_opregion(vgpu)->va, why don’t you reuse this function and set the > >vgpu_opregion(vgpu)->va to the intel_vgpu_register_reg function? > The reason is we do not the know the gpa of the OpRegion. > For KVMGT the gpa of the OpRegion is allocated by seabios. Maybe you can consolidate them into one function to remove duplicate code. _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
