| Message ID | 1502322098-23702-3-git-send-email-tina.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On to, 2017-08-10 at 07:41 +0800, Tina Zhang wrote: > Enable the guest i915 full ppgtt functionality when host can provide this > capability. vgt_caps is introduced to guest i915 driver to get the vgpu > capabilities from the device model. VGT_CPAS_FULL_PPGTT is one of the > capabilities type to let guest i915 dirver know that the guest i915 full > ppgtt is supported by device model. > > Notice that the minor version of pvinfo isn't bumped because of this > vgt_caps introduction, due to older guest would be broken by simply > increasing the pvinfo version. Although the pvinfo minor version doesn't > increase, the compatibility won't be blocked. The compatibility is ensured > by checking the value of caps field in pvinfo. Zero means no full ppgtt > support and BIT(2) means this feature is provided. > > Changes since v1: > - Use u32 instead of uint32_t (Joonas) > - Move VGT_CAPS_FULL_PPGTT introduction to this patch and use #define > instead of enum (Joonas) > - Rewrite the vgpu full ppgtt capability checking logic. (Joonas) > - Some coding style refine. (Joonas) > > Changes since v2: > - Divide the whole patch set into two separate patch series, with one > patch in i915 side to check guest i915 full ppgtt capability and enable > it when this capability is supported by the device model, and the other > one in gvt side which fixs the blocking issue and enables the device > model to provide the capability to guest. And this patch focuses on guest > i915 side. (Joonas) > - Change the title from "introduce vgt_caps to pvinfo" to > "Enable guest i915 full ppgtt functionality". (Tina) > > Change since v3: > - Add some comments about pvinfo caps and version. (Joonas) > > Change since v4: > - Tested by Tina Zhang. > > Change since v5: > - Add limitation about supporting 32bit full ppgtt. > > Change since v6: > - Change the fallback to 48bit full ppgtt if i915.ppgtt_enable=2. (Zhenyu) > > Change in v9: > - Remove the fixme comment due to no plan for 32bit full ppgtt > support. (Zhenyu) > - Reorder the patch-set to fix compiling issue with git-bisect. (Zhenyu) > - Add print log when forcing guest 48bit full ppgtt. (Zhenyu) > > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> in v2 > Signed-off-by: Tina Zhang <tina.zhang@intel.com> <SNIP> > @@ -141,14 +141,19 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv, > > has_aliasing_ppgtt = dev_priv->info.has_aliasing_ppgtt; > has_full_ppgtt = dev_priv->info.has_full_ppgtt; > - has_full_48bit_ppgtt = dev_priv->info.has_full_48bit_ppgtt; Leave above line. > > if (intel_vgpu_active(dev_priv)) { > - /* emulation is too hard */ > - has_full_ppgtt = false; > - has_full_48bit_ppgtt = false; has_full_ppgtt = false; has_full_48bit_ppgtt = intel_vgpu_has_full_48bit_ppgtt(dev_priv); > + has_full_ppgtt = intel_vgpu_has_full_ppgtt(dev_priv); > + /* GVT-g has no support for 32bit ppgtt */ > + if (enable_ppgtt == 2 && has_full_ppgtt) { > + DRM_DEBUG_DRIVER("Force 48bit ppgtt for vGPU\n"); > + enable_ppgtt = 3; Lets not do an upgrade for the user if they explicitly requested lower level. I'd just leave the option to be and it'll get downgraded to aliasing ppgtt. > + } > } > > + has_full_48bit_ppgtt = has_full_ppgtt && > + dev_priv->info.has_full_48bit_ppgtt; > + This can then be dropped too. I'll send a patch to disconnect the has_full_ppgtt and has_full_48bit_ppgtt flags. > if (!has_aliasing_ppgtt) > return 0; > > @@ -49,12 +49,18 @@ enum vgt_g2v_type { > > VGT_G2V_MAX, > }; > > +/* > + * VGT capabilities type > + */ > +#define VGT_CAPS_FULL_PPGTT_48BIT BIT(2) VGT_CAPS_FULL_48BIT_PPGTT for consistency > @@ -75,10 +75,17 @@ void i915_check_vgpu(struct drm_i915_private *dev_priv) > return; > } > > + dev_priv->vgpu.caps = __raw_i915_read32(dev_priv, vgtif_reg(vgt_caps)); > + > dev_priv->vgpu.active = true; > DRM_INFO("Virtual GPU for Intel GVT-g detected.\n"); > } > > +bool intel_vgpu_has_full_ppgtt(struct drm_i915_private *dev_priv) This should really be has_full_48bit_ppgtt() too. Regards, Joonas
On 2017.08.11 12:46:48 +0300, Joonas Lahtinen wrote: > > > @@ -141,14 +141,19 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv, > > > > has_aliasing_ppgtt = dev_priv->info.has_aliasing_ppgtt; > > has_full_ppgtt = dev_priv->info.has_full_ppgtt; > > - has_full_48bit_ppgtt = dev_priv->info.has_full_48bit_ppgtt; > > Leave above line. > > > > > if (intel_vgpu_active(dev_priv)) { > > - /* emulation is too hard */ > > - has_full_ppgtt = false; > > - has_full_48bit_ppgtt = false; > > has_full_ppgtt = false; > has_full_48bit_ppgtt = > intel_vgpu_has_full_48bit_ppgtt(dev_priv); > > > + has_full_ppgtt = intel_vgpu_has_full_ppgtt(dev_priv); > > + /* GVT-g has no support for 32bit ppgtt */ > > + if (enable_ppgtt == 2 && has_full_ppgtt) { > > + DRM_DEBUG_DRIVER("Force 48bit ppgtt for vGPU\n"); > > + enable_ppgtt = 3; > > Lets not do an upgrade for the user if they explicitly requested lower > level. I'd just leave the option to be and it'll get downgraded to > aliasing ppgtt. > If user specified enable_ppgtt=2, it will get enable_ppgtt=2 instead of aliasing ppgtt, so we try to guard against that for current 48bit ppgtt support on vgpu. Well either upgrade to full 48bit or downgrade to aliasing. > > + } > > } > > > > + has_full_48bit_ppgtt = has_full_ppgtt && > > + dev_priv->info.has_full_48bit_ppgtt; > > + > > This can then be dropped too. > > I'll send a patch to disconnect the has_full_ppgtt and > has_full_48bit_ppgtt flags. > That'll be good. As we have validated this full ppgtt on vgpu for quite some time now and plan to push for 4.14. Are you able to align with that? So I might send first 4.14 pull without this series, then add them later for dinf. > > > if (!has_aliasing_ppgtt) > > return 0; > > > > @@ -49,12 +49,18 @@ enum vgt_g2v_type { > > > VGT_G2V_MAX, > > }; > > > > +/* > > + * VGT capabilities type > > + */ > > +#define VGT_CAPS_FULL_PPGTT_48BIT BIT(2) > > VGT_CAPS_FULL_48BIT_PPGTT for consistency > > > @@ -75,10 +75,17 @@ void i915_check_vgpu(struct drm_i915_private *dev_priv) > > return; > > } > > > > + dev_priv->vgpu.caps = __raw_i915_read32(dev_priv, vgtif_reg(vgt_caps)); > > + > > dev_priv->vgpu.active = true; > > DRM_INFO("Virtual GPU for Intel GVT-g detected.\n"); > > } > > > > +bool intel_vgpu_has_full_ppgtt(struct drm_i915_private *dev_priv) > > This should really be has_full_48bit_ppgtt() too. > > Regards, Joonas > -- > Joonas Lahtinen > Open Source Technology Center > Intel Corporation
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b051122..1b95069 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1879,6 +1879,7 @@ struct i915_workarounds { struct i915_virtual_gpu { bool active; + u32 caps; }; /* used in computing the new watermarks state */ diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 10aa776..090ceb7 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -141,14 +141,19 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv, has_aliasing_ppgtt = dev_priv->info.has_aliasing_ppgtt; has_full_ppgtt = dev_priv->info.has_full_ppgtt; - has_full_48bit_ppgtt = dev_priv->info.has_full_48bit_ppgtt; if (intel_vgpu_active(dev_priv)) { - /* emulation is too hard */ - has_full_ppgtt = false; - has_full_48bit_ppgtt = false; + has_full_ppgtt = intel_vgpu_has_full_ppgtt(dev_priv); + /* GVT-g has no support for 32bit ppgtt */ + if (enable_ppgtt == 2 && has_full_ppgtt) { + DRM_DEBUG_DRIVER("Force 48bit ppgtt for vGPU\n"); + enable_ppgtt = 3; + } } + has_full_48bit_ppgtt = has_full_ppgtt && + dev_priv->info.has_full_48bit_ppgtt; + if (!has_aliasing_ppgtt) return 0; diff --git a/drivers/gpu/drm/i915/i915_pvinfo.h b/drivers/gpu/drm/i915/i915_pvinfo.h index 2cfe96d3..7e958d7 100644 --- a/drivers/gpu/drm/i915/i915_pvinfo.h +++ b/drivers/gpu/drm/i915/i915_pvinfo.h @@ -49,12 +49,18 @@ enum vgt_g2v_type { VGT_G2V_MAX, }; +/* + * VGT capabilities type + */ +#define VGT_CAPS_FULL_PPGTT_48BIT BIT(2) + struct vgt_if { u64 magic; /* VGT_MAGIC */ u16 version_major; u16 version_minor; u32 vgt_id; /* ID of vGT instance */ - u32 rsv1[12]; /* pad to offset 0x40 */ + u32 vgt_caps; /* VGT capabilities */ + u32 rsv1[11]; /* pad to offset 0x40 */ /* * Data structure to describe the balooning info of resources. * Each VM can only have one portion of continuous area for now. diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c index cf7a958..e85e27c 100644 --- a/drivers/gpu/drm/i915/i915_vgpu.c +++ b/drivers/gpu/drm/i915/i915_vgpu.c @@ -75,10 +75,17 @@ void i915_check_vgpu(struct drm_i915_private *dev_priv) return; } + dev_priv->vgpu.caps = __raw_i915_read32(dev_priv, vgtif_reg(vgt_caps)); + dev_priv->vgpu.active = true; DRM_INFO("Virtual GPU for Intel GVT-g detected.\n"); } +bool intel_vgpu_has_full_ppgtt(struct drm_i915_private *dev_priv) +{ + return dev_priv->vgpu.caps & VGT_CAPS_FULL_PPGTT_48BIT; +} + struct _balloon_info_ { /* * There are up to 2 regions per mappable/unmappable graphic diff --git a/drivers/gpu/drm/i915/i915_vgpu.h b/drivers/gpu/drm/i915/i915_vgpu.h index 3c3b2d2..b4e04eb 100644 --- a/drivers/gpu/drm/i915/i915_vgpu.h +++ b/drivers/gpu/drm/i915/i915_vgpu.h @@ -27,6 +27,9 @@ #include "i915_pvinfo.h" void i915_check_vgpu(struct drm_i915_private *dev_priv); + +bool intel_vgpu_has_full_ppgtt(struct drm_i915_private *dev_priv); + int intel_vgt_balloon(struct drm_i915_private *dev_priv); void intel_vgt_deballoon(struct drm_i915_private *dev_priv);