| Message ID | 20170605030041.9619-1-zhenyuw@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On ma, 2017-06-05 at 11:00 +0800, Zhenyu Wang wrote: > Current it's strictly checked if PVINFO version matches 1.0 > for GVT-g i915 guest which doesn't help for compatibility at > all and forces GVT-g host can't extend PVINFO easily with version > bump for real compatibility check. > > This fixes that to check minimal required PVINFO version instead > and currently minimal 1.0 is required. > > Cc: Chuanxiao Dong <chuanxiao.dong@intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: stable@vger.kernel.org # 4.10+ > Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com> <SNIP> > +++ b/drivers/gpu/drm/i915/i915_pvinfo.h > @@ -37,7 +37,8 @@ > #define VGT_VERSION_MINOR 0 > > #define INTEL_VGT_IF_VERSION_ENCODE(major, minor) ((major) << 16 | (minor)) > -#define INTEL_VGT_IF_VERSION \ > +/* minimal required PVINFO version */ > +#define INTEL_VGT_IF_VERSION_REQUIRED \ > INTEL_VGT_IF_VERSION_ENCODE(VGT_VERSION_MAJOR, VGT_VERSION_MINOR) I think this can be dropped completely. > @@ -72,7 +72,7 @@ void i915_check_vgpu(struct drm_i915_private *dev_priv) > version = INTEL_VGT_IF_VERSION_ENCODE( > __raw_i915_read16(dev_priv, vgtif_reg(version_major)), > __raw_i915_read16(dev_priv, vgtif_reg(version_minor))); Here one would check that if (version_major != VGT_VERSION_MAJOR) DRM_INFO("VGT interface version mismatch!\n"); return; } That makes sure that major version must always match (incrementing major version is a direct indication of interface incompatibility). I'm not sure if we want to check the minor version at all, at load time. Instead we should check the minor version for each new feature we know has been added. That way an older i915 module could keep running in the DOM0, and the newer i915 modules in VMs just wouldn't attempt to use the new features until the DOM0 gets an update, too. Regards, Joonas
On 2017.06.05 16:40:09 +0300, Joonas Lahtinen wrote: > > On ma, 2017-06-05 at 11:00 +0800, Zhenyu Wang wrote: > > Current it's strictly checked if PVINFO version matches 1.0 > > for GVT-g i915 guest which doesn't help for compatibility at > > all and forces GVT-g host can't extend PVINFO easily with version > > bump for real compatibility check. > > > > This fixes that to check minimal required PVINFO version instead > > and currently minimal 1.0 is required. > > > > Cc: Chuanxiao Dong <chuanxiao.dong@intel.com> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > Cc: stable@vger.kernel.org # 4.10+ > > Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com> > > <SNIP> > > > +++ b/drivers/gpu/drm/i915/i915_pvinfo.h > > @@ -37,7 +37,8 @@ > > #define VGT_VERSION_MINOR 0 > > > > #define INTEL_VGT_IF_VERSION_ENCODE(major, minor) ((major) << 16 | (minor)) > > -#define INTEL_VGT_IF_VERSION \ > > +/* minimal required PVINFO version */ > > +#define INTEL_VGT_IF_VERSION_REQUIRED \ > > INTEL_VGT_IF_VERSION_ENCODE(VGT_VERSION_MAJOR, VGT_VERSION_MINOR) > > I think this can be dropped completely. > > > @@ -72,7 +72,7 @@ void i915_check_vgpu(struct drm_i915_private *dev_priv) > > version = INTEL_VGT_IF_VERSION_ENCODE( > > __raw_i915_read16(dev_priv, vgtif_reg(version_major)), > > __raw_i915_read16(dev_priv, vgtif_reg(version_minor))); > > Here one would check that > > if (version_major != VGT_VERSION_MAJOR) > DRM_INFO("VGT interface version mismatch!\n"); > return; > } > > That makes sure that major version must always match (incrementing > major version is a direct indication of interface incompatibility). I'm > not sure if we want to check the minor version at all, at load time. We only plan to extend PVInfo definition (use previous reserved/unused area) but not break compatibility as no requirement to do incompatibility change for pvinfo, so like to keep compat with older kernel. So this version check is just for sanity on minimal require, agree to only check major is enough. > > Instead we should check the minor version for each new feature we know > has been added. That way an older i915 module could keep running in the > DOM0, and the newer i915 modules in VMs just wouldn't attempt to use > the new features until the DOM0 gets an update, too. > yeah, minor version would be bumped for each new feature support, e.g guc/huc for guest etc, structural extension will get major version bumped.
On pe, 2017-06-09 at 08:10 +0000, Patchwork wrote: > == Series Details == > > Series: drm/i915: Fix GVT-g PVINFO version compatibility check (rev3) > URL : https://patchwork.freedesktop.org/series/25275/ > State : success Applied. Thanks for the patch. Regards, Joonas
diff --git a/drivers/gpu/drm/i915/i915_pvinfo.h b/drivers/gpu/drm/i915/i915_pvinfo.h index c0cb2974caac..96434707857e 100644 --- a/drivers/gpu/drm/i915/i915_pvinfo.h +++ b/drivers/gpu/drm/i915/i915_pvinfo.h @@ -37,7 +37,8 @@ #define VGT_VERSION_MINOR 0 #define INTEL_VGT_IF_VERSION_ENCODE(major, minor) ((major) << 16 | (minor)) -#define INTEL_VGT_IF_VERSION \ +/* minimal required PVINFO version */ +#define INTEL_VGT_IF_VERSION_REQUIRED \ INTEL_VGT_IF_VERSION_ENCODE(VGT_VERSION_MAJOR, VGT_VERSION_MINOR) /* diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c index 4ab8a973b61f..5798572e6db3 100644 --- a/drivers/gpu/drm/i915/i915_vgpu.c +++ b/drivers/gpu/drm/i915/i915_vgpu.c @@ -72,7 +72,7 @@ void i915_check_vgpu(struct drm_i915_private *dev_priv) version = INTEL_VGT_IF_VERSION_ENCODE( __raw_i915_read16(dev_priv, vgtif_reg(version_major)), __raw_i915_read16(dev_priv, vgtif_reg(version_minor))); - if (version != INTEL_VGT_IF_VERSION) { + if (version < INTEL_VGT_IF_VERSION_REQUIRED) { DRM_INFO("VGT interface version mismatch!\n"); return; }
Current it's strictly checked if PVINFO version matches 1.0 for GVT-g i915 guest which doesn't help for compatibility at all and forces GVT-g host can't extend PVINFO easily with version bump for real compatibility check. This fixes that to check minimal required PVINFO version instead and currently minimal 1.0 is required. Cc: Chuanxiao Dong <chuanxiao.dong@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: stable@vger.kernel.org # 4.10+ Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com> --- drivers/gpu/drm/i915/i915_pvinfo.h | 3 ++- drivers/gpu/drm/i915/i915_vgpu.c | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-)