Quoting Xiaolin Zhang (2018-10-11 07:24:19)
> To enable pvmmio feature, we need to prepare one 4K shared page
> which will be accessed by both guest and backend i915 driver.
>
> guest i915 allocate one page memory and then the guest physical address is
> passed to backend i915 driver through PVINFO register so that backend i915
> driver can access this shared page without hypeviser trap cost for shared
> data exchagne via hyperviser read_gpa functionality.
>
> v1: addressed RFC comment to move both shared_page_lock and shared_page
> to i915_virtual_gpu structure
> v0: RFC
>
> Signed-off-by: Xiaolin Zhang <xiaolin.zh...@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 8 ++++++++
> drivers/gpu/drm/i915/i915_drv.h | 2 ++
> drivers/gpu/drm/i915/i915_pvinfo.h | 24 +++++++++++++++++++++++-
> drivers/gpu/drm/i915/i915_vgpu.c | 19 ++++++++++++++++++-
> 4 files changed, 51 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 19302342..9b25bb7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -987,6 +987,10 @@ static void i915_mmio_cleanup(struct drm_i915_private
> *dev_priv)
>
> intel_teardown_mchbar(dev_priv);
> pci_iounmap(pdev, dev_priv->regs);
> + if (intel_vgpu_active(dev_priv) && dev_priv->vgpu.shared_page) {
> + free_page((unsigned long)dev_priv->vgpu.shared_page);
> + dev_priv->vgpu.shared_page = NULL;
> + }
intel_vgpu_(teardown,cleanup,fini)
setting to NULL? fetch_and_zero() if you must, but here it is a little
pointless.
> }
>
> /**
> @@ -1029,6 +1033,10 @@ static int i915_driver_init_mmio(struct
> drm_i915_private *dev_priv)
> return 0;
>
> err_uncore:
> + if (intel_vgpu_active(dev_priv) && dev_priv->vgpu.shared_page) {
> + free_page((unsigned long)dev_priv->vgpu.shared_page);
> + dev_priv->vgpu.shared_page = NULL;
Same again. Remember that shared_page will never be !0 unless active
anyway.
> + }
> intel_uncore_fini(dev_priv);
> err_bridge:
> pci_dev_put(dev_priv->bridge_dev);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d22154a..2c131d4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1345,6 +1345,8 @@ struct i915_virtual_gpu {
> bool active;
> u32 caps;
> u32 pv_caps;
> + spinlock_t shared_page_lock;
> + struct gvt_shared_page *shared_page;
Ugh, be considerate of struct packing and/or cachelines.
> };
>
> /* used in computing the new watermarks state */
> diff --git a/drivers/gpu/drm/i915/i915_pvinfo.h
> b/drivers/gpu/drm/i915/i915_pvinfo.h
> index 26709e8..179d558 100644
> --- a/drivers/gpu/drm/i915/i915_pvinfo.h
> +++ b/drivers/gpu/drm/i915/i915_pvinfo.h
> @@ -49,6 +49,24 @@ enum vgt_g2v_type {
> VGT_G2V_MAX,
> };
>
> +struct pv_ppgtt_update {
> + u64 pdp;
> + u64 start;
> + u64 length;
> + u32 cache_level;
> +};
Not used. Only introduce interfaces as they are being used, or just
before.
> +/*
> + * shared page(4KB) between gvt and VM, could be allocated by guest driver
> + * or a fixed location in PCI bar 0 region
> + */
> +struct gvt_shared_page {
> + u32 elsp_data[4];
> + u32 reg_addr;
> + u32 disable_irq;
> + struct pv_ppgtt_update pv_ppgtt;
> +};
> +
> #define VGPU_PVMMIO(vgpu) vgpu_vreg_t(vgpu, vgtif_reg(enable_pvmmio))
>
> /*
> @@ -121,8 +139,12 @@ struct vgt_if {
> u32 execlist_context_descriptor_lo;
> u32 execlist_context_descriptor_hi;
> u32 enable_pvmmio;
> + struct {
> + u32 lo;
> + u32 hi;
> + } shared_page_gpa;
>
> - u32 rsv7[0x200 - 25]; /* pad to one page */
> + u32 rsv7[0x200 - 27]; /* pad to one page */
> } __packed;
>
> #define vgtif_reg(x) \
> diff --git a/drivers/gpu/drm/i915/i915_vgpu.c
> b/drivers/gpu/drm/i915/i915_vgpu.c
> index 907bbd2..609eefe 100644
> --- a/drivers/gpu/drm/i915/i915_vgpu.c
> +++ b/drivers/gpu/drm/i915/i915_vgpu.c
> @@ -62,6 +62,7 @@ void i915_check_vgpu(struct drm_i915_private *dev_priv)
> {
> u64 magic;
> u16 version_major;
> + u64 shared_page_gpa;
Scope.
> BUILD_BUG_ON(sizeof(struct vgt_if) != VGT_PVINFO_SIZE);
>
> @@ -91,7 +92,23 @@ void i915_check_vgpu(struct drm_i915_private *dev_priv)
> dev_priv->vgpu.pv_caps);
> dev_priv->vgpu.pv_caps = __raw_i915_read32(dev_priv,
> vgtif_reg(enable_pvmmio));
> -
> + if (intel_vgpu_active(dev_priv) && dev_priv->vgpu.pv_caps) {
> + dev_priv->vgpu.shared_page = (struct gvt_shared_page *)
> + get_zeroed_page(GFP_KERNEL);
> + if (!dev_priv->vgpu.shared_page) {
> + DRM_ERROR("out of memory for shared page memory\n");
> + return;
> + }
> + shared_page_gpa = __pa(dev_priv->vgpu.shared_page);
> + __raw_i915_write32(dev_priv, vgtif_reg(shared_page_gpa.lo),
> + lower_32_bits(shared_page_gpa));
> + __raw_i915_write32(dev_priv, vgtif_reg(shared_page_gpa.hi),
> + upper_32_bits(shared_page_gpa));
Shouldn't there be an ack? 12bits there, at least one should be used for
a valid flag.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
