Re: [PATCH] drm/i915/opregion: rvda is relative from opregion base, not absolute

[Jani Nikula <jani.nikula@intel.com>]

On Thu, 13 Dec 2018, Jani Nikula <jani.nikula@intel.com> wrote:
> We've supported the opregion RVDA/RVDS fields for VBT size >= 6 KB since
> commit 04ebaadb9f2d ("drm/i915/opregion: handle VBT sizes bigger than 6
> KB"). That's three years, almost to the date.
>
> The implementation was based on spec only, in anticipation of systems
> with big VBT. Now, the spec has been changed. The RVDA is supposed to be
> relative from the beginning of opregion, not absolute address.
>
> This is obviously a backward/forward incompatible change. I've been told
> there are no systems out there using the field. Fingers crossed. This
> will still be problematic for older kernels, and we can only try to
> backport the fix.

Should add this to the commit message:

While at it, add the missing memunmap() on the failure path for
completeness.

>
> Fixes: 04ebaadb9f2d ("drm/i915/opregion: handle VBT sizes bigger than 6 KB")
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_opregion.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> index b8f106d9ecf8..700fddaa8d9e 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -119,7 +119,7 @@ struct opregion_asle {
>  	u64 fdss;
>  	u32 fdsp;
>  	u32 stat;
> -	u64 rvda;	/* Physical address of raw vbt data */
> +	u64 rvda;	/* Address of raw vbt data, relative from opregion */
>  	u32 rvds;	/* Size of raw vbt data */
>  	u8 rsvd[58];
>  } __packed;
> @@ -955,7 +955,13 @@ int intel_opregion_setup(struct drm_i915_private *dev_priv)
>  
>  	if (opregion->header->opregion_ver >= 2 && opregion->asle &&
>  	    opregion->asle->rvda && opregion->asle->rvds) {
> -		opregion->rvda = memremap(opregion->asle->rvda,
> +		/*
> +		 * rvda is unsigned, relative from opregion base, and should
> +		 * never point within opregion.
> +		 */
> +		WARN_ON(opregion->asle->rvda < OPREGION_SIZE);
> +
> +		opregion->rvda = memremap(asls + opregion->asle->rvda,
>  					  opregion->asle->rvds,
>  					  MEMREMAP_WB);
>  		vbt = opregion->rvda;
> @@ -967,6 +973,8 @@ int intel_opregion_setup(struct drm_i915_private *dev_priv)
>  			goto out;
>  		} else {
>  			DRM_DEBUG_KMS("Invalid VBT in ACPI OpRegion (RVDA)\n");
> +			memunmap(opregion->rvda);
> +			opregion->rvda = NULL;
>  		}
>  	}

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx