Re: [Intel-gfx] [PATCH v1 02/12] drm/i915: vgpu shared memory setup for pv support
On Thu, 2020-09-10 at 16:16 +0300, Jani Nikula wrote:
> On Sat, 05 Sep 2020, Xiaolin Zhang wrote:
> > To support vgpu pv features, a common shared memory is setup used
> > for
> > communication and data exchange between guest and host GVTg to
> > reduce
> > data access overhead and trap cost.
> >
> > guest i915 will allocate this common memory (1 page size) and then
> > pass
> > it's physical address to host GVTg through PVINFO register so that
> > host
> > GVTg can access this shared guest page meory without trap cost with
> > hyperviser's facility.
> >
> > guest i915 will send VGT_G2V_SHARED_PAGE_SETUP notification to host
> > GVTg
> > once shared memory setup succcessfully finished.
> >
> > the layout of the shared_page also defined as well, the first part
> > is the
> > PV vervsion information used for compabilty support.
> >
> > Signed-off-by: Xiaolin Zhang
> > ---
> > drivers/gpu/drm/i915/i915_drv.c| 2 +
> > drivers/gpu/drm/i915/i915_drv.h| 4 +-
> > drivers/gpu/drm/i915/i915_pvinfo.h | 5 +-
> > drivers/gpu/drm/i915/i915_vgpu.c | 94
> > ++
> > drivers/gpu/drm/i915/i915_vgpu.h | 14 ++
> > 5 files changed, 117 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index 00292a8..5fbb4ab 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -1071,6 +1071,8 @@ static void i915_driver_release(struct
> > drm_device *dev)
> >
> > disable_rpm_wakeref_asserts(rpm);
> >
> > + intel_vgpu_destroy(dev_priv);
> > +
> > i915_gem_driver_release(dev_priv);
> >
> > intel_memory_regions_driver_release(dev_priv);
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 16d1b51..3cde2c5f 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -809,7 +809,9 @@ struct i915_virtual_gpu {
> > bool active;
> > u32 caps;
> > u32 pv_caps;
> > -};
> > +
> > + struct i915_virtual_gpu_pv *pv;
> > +} __packed;
>
> I'm unsure why this struct should be packed.
Thanks your point it out. it is not necessary. will remove this next
version.
>
> >
> > struct intel_cdclk_config {
> > unsigned int cdclk, vco, ref, bypass;
> > diff --git a/drivers/gpu/drm/i915/i915_pvinfo.h
> > b/drivers/gpu/drm/i915/i915_pvinfo.h
> > index 8b0dc25..1d44876 100644
> > --- a/drivers/gpu/drm/i915/i915_pvinfo.h
> > +++ b/drivers/gpu/drm/i915/i915_pvinfo.h
> > @@ -48,6 +48,7 @@ enum vgt_g2v_type {
> > VGT_G2V_PPGTT_L4_PAGE_TABLE_DESTROY,
> > VGT_G2V_EXECLIST_CONTEXT_CREATE,
> > VGT_G2V_EXECLIST_CONTEXT_DESTROY,
> > + VGT_G2V_SHARED_PAGE_REGISTER,
> > VGT_G2V_MAX,
> > };
> >
> > @@ -112,7 +113,9 @@ struct vgt_if {
> >
> > u32 pv_caps;
> >
> > - u32 rsv7[0x200 - 25];/* pad to one page */
> > + u64 shared_page_gpa;
> > +
> > + u32 rsv7[0x200 - 27];/* pad to one page */
> > } __packed;
> >
> > #define vgtif_offset(x) (offsetof(struct vgt_if, x))
> > diff --git a/drivers/gpu/drm/i915/i915_vgpu.c
> > b/drivers/gpu/drm/i915/i915_vgpu.c
> > index 10960125..8b2b451 100644
> > --- a/drivers/gpu/drm/i915/i915_vgpu.c
> > +++ b/drivers/gpu/drm/i915/i915_vgpu.c
> > @@ -110,6 +110,17 @@ void intel_vgpu_detect(struct drm_i915_private
> > *dev_priv)
> > pci_iounmap(pdev, shared_area);
> > }
> >
> > +void intel_vgpu_destroy(struct drm_i915_private *i915)
> > +{
> > + struct i915_virtual_gpu_pv *pv = i915->vgpu.pv;
> > +
> > + if (!intel_vgpu_active(i915) || !pv)
> > + return;
> > +
> > + __free_page(virt_to_page(pv->shared_page));
> > + kfree(pv);
> > +}
> > +
> > void intel_vgpu_register(struct drm_i915_private *i915)
> > {
> > /*
> > @@ -360,6 +371,83 @@ int intel_vgt_balloon(struct i915_ggtt *ggtt)
> > */
> >
> > /*
> > + * shared_page setup for VGPU PV features
> > + */
> > +static int intel_vgpu_setup_shared_page(struct drm_i915_private
> > *i915,
> > + void __iomem *shared_area)
> > +{
> > + void __iomem *addr;
> > + struct i915_virtual_gpu_pv *pv;
> > + struct gvt_shared_page *base;
> > + u64 gpa;
> > + u16 ver_maj, ver_min;
> > + int ret = 0;
> > +
> > + /* We allocate 1 page shared between guest and GVT for data
> > exchange.
> > +* ___
> > +* |version|
> > +* |___PAGE/8
> > +* | |
> > +* |___PAGE/4
> > +* | |
> > +* | |
> > +* | |
> > +* |___PAGE/2
> > +* | |
> > +* | |
> > +* | |
> > +* |
Re: [Intel-gfx] [PATCH v1 02/12] drm/i915: vgpu shared memory setup for pv support
On Sat, 05 Sep 2020, Xiaolin Zhang wrote:
> To support vgpu pv features, a common shared memory is setup used for
> communication and data exchange between guest and host GVTg to reduce
> data access overhead and trap cost.
>
> guest i915 will allocate this common memory (1 page size) and then pass
> it's physical address to host GVTg through PVINFO register so that host
> GVTg can access this shared guest page meory without trap cost with
> hyperviser's facility.
>
> guest i915 will send VGT_G2V_SHARED_PAGE_SETUP notification to host GVTg
> once shared memory setup succcessfully finished.
>
> the layout of the shared_page also defined as well, the first part is the
> PV vervsion information used for compabilty support.
>
> Signed-off-by: Xiaolin Zhang
> ---
> drivers/gpu/drm/i915/i915_drv.c| 2 +
> drivers/gpu/drm/i915/i915_drv.h| 4 +-
> drivers/gpu/drm/i915/i915_pvinfo.h | 5 +-
> drivers/gpu/drm/i915/i915_vgpu.c | 94
> ++
> drivers/gpu/drm/i915/i915_vgpu.h | 14 ++
> 5 files changed, 117 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 00292a8..5fbb4ab 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1071,6 +1071,8 @@ static void i915_driver_release(struct drm_device *dev)
>
> disable_rpm_wakeref_asserts(rpm);
>
> + intel_vgpu_destroy(dev_priv);
> +
> i915_gem_driver_release(dev_priv);
>
> intel_memory_regions_driver_release(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 16d1b51..3cde2c5f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -809,7 +809,9 @@ struct i915_virtual_gpu {
> bool active;
> u32 caps;
> u32 pv_caps;
> -};
> +
> + struct i915_virtual_gpu_pv *pv;
> +} __packed;
I'm unsure why this struct should be packed.
>
> struct intel_cdclk_config {
> unsigned int cdclk, vco, ref, bypass;
> diff --git a/drivers/gpu/drm/i915/i915_pvinfo.h
> b/drivers/gpu/drm/i915/i915_pvinfo.h
> index 8b0dc25..1d44876 100644
> --- a/drivers/gpu/drm/i915/i915_pvinfo.h
> +++ b/drivers/gpu/drm/i915/i915_pvinfo.h
> @@ -48,6 +48,7 @@ enum vgt_g2v_type {
> VGT_G2V_PPGTT_L4_PAGE_TABLE_DESTROY,
> VGT_G2V_EXECLIST_CONTEXT_CREATE,
> VGT_G2V_EXECLIST_CONTEXT_DESTROY,
> + VGT_G2V_SHARED_PAGE_REGISTER,
> VGT_G2V_MAX,
> };
>
> @@ -112,7 +113,9 @@ struct vgt_if {
>
> u32 pv_caps;
>
> - u32 rsv7[0x200 - 25];/* pad to one page */
> + u64 shared_page_gpa;
> +
> + u32 rsv7[0x200 - 27];/* pad to one page */
> } __packed;
>
> #define vgtif_offset(x) (offsetof(struct vgt_if, x))
> diff --git a/drivers/gpu/drm/i915/i915_vgpu.c
> b/drivers/gpu/drm/i915/i915_vgpu.c
> index 10960125..8b2b451 100644
> --- a/drivers/gpu/drm/i915/i915_vgpu.c
> +++ b/drivers/gpu/drm/i915/i915_vgpu.c
> @@ -110,6 +110,17 @@ void intel_vgpu_detect(struct drm_i915_private *dev_priv)
> pci_iounmap(pdev, shared_area);
> }
>
> +void intel_vgpu_destroy(struct drm_i915_private *i915)
> +{
> + struct i915_virtual_gpu_pv *pv = i915->vgpu.pv;
> +
> + if (!intel_vgpu_active(i915) || !pv)
> + return;
> +
> + __free_page(virt_to_page(pv->shared_page));
> + kfree(pv);
> +}
> +
> void intel_vgpu_register(struct drm_i915_private *i915)
> {
> /*
> @@ -360,6 +371,83 @@ int intel_vgt_balloon(struct i915_ggtt *ggtt)
> */
>
> /*
> + * shared_page setup for VGPU PV features
> + */
> +static int intel_vgpu_setup_shared_page(struct drm_i915_private *i915,
> + void __iomem *shared_area)
> +{
> + void __iomem *addr;
> + struct i915_virtual_gpu_pv *pv;
> + struct gvt_shared_page *base;
> + u64 gpa;
> + u16 ver_maj, ver_min;
> + int ret = 0;
> +
> + /* We allocate 1 page shared between guest and GVT for data exchange.
> + * ___
> + * |version|
> + * |___PAGE/8
> + * | |
> + * |___PAGE/4
> + * | |
> + * | |
> + * | |
> + * |___PAGE/2
> + * | |
> + * | |
> + * | |
> + * | |
> + * | |
> + * | |
> + * | |
> + * |___|
> + *
> + * 0 offset: PV version area
> + */
> +
> + base = (struct gvt_shared_page *)get_zeroed_page(GFP_KERNEL);
> +
