Re: [PATCH v3 2/3] drm/i915/opregion: rvda is relative from opregion base in opregion 2.1+

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

On Fri, 08 Feb 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Fri, Feb 08, 2019 at 09:00:59PM +0200, Jani Nikula wrote:
>> On Fri, 08 Feb 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> > On Fri, Feb 08, 2019 at 08:42:53PM +0200, Jani Nikula wrote:
>> >> Starting from opregion version 2.1 (roughly corresponding to ICL+) the
>> >> RVDA field is relative from the beginning of opregion, not absolute
>> >> address.
>> >> 
>> >> Fix the error path while at it.
>> >> 
>> >> v2: Make relative vs. absolute conditional on the opregion version,
>> >>     bumped for the purpose. Turned out there are machines relying on
>> >>     absolute RVDA in the wild.
>> >> 
>> >> v3: Fix the version checks
>> >> 
>> >> 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 | 24 +++++++++++++++++++++---
>> >>  1 file changed, 21 insertions(+), 3 deletions(-)
>> >> 
>> >> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
>> >> index f1b841580521..5e00ee9270b5 100644
>> >> --- a/drivers/gpu/drm/i915/intel_opregion.c
>> >> +++ b/drivers/gpu/drm/i915/intel_opregion.c
>> >> @@ -123,7 +123,8 @@ struct opregion_asle {
>> >>  	u64 fdss;
>> >>  	u32 fdsp;
>> >>  	u32 stat;
>> >> -	u64 rvda;	/* Physical address of raw vbt data */
>> >> +	u64 rvda;	/* Physical (2.0) or relative from opregion (2.1+)
>> >> +			 * address of raw VBT data. */
>> >>  	u32 rvds;	/* Size of raw vbt data */
>> >>  	u8 rsvd[58];
>> >>  } __packed;
>> >> @@ -964,9 +965,24 @@ int intel_opregion_setup(struct drm_i915_private *dev_priv)
>> >>  
>> >>  	if (opregion->header->over.major >= 2 && opregion->asle &&
>> 
>> Push this line to short term memory.
>
> Stack overflow.
>
> Series is
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thanks, pushed patches 1&2 to dinq.

BR,
Jani.

>
>> 
>> >>  	    opregion->asle->rvda && opregion->asle->rvds) {
>> >> -		opregion->rvda = memremap(opregion->asle->rvda,
>> >> -					  opregion->asle->rvds,
>> >> +		resource_size_t rvda = opregion->asle->rvda;
>> >> +
>> >> +		/*
>> >> +		 * opregion 2.0: rvda is the physical VBT address.
>> >> +		 *
>> >> +		 * opregion 2.1+: rvda is unsigned, relative offset from
>> >> +		 * opregion base, and should never point within opregion.
>> >> +		 */
>> >> +		if (opregion->header->over.major > 2 ||
>> >> +		    opregion->header->over.minor >= 1) {
>> >
>> > What happens with version 1.1?
>> 
>> Pop. ;)
>> 
>> BR,
>> Jani.
>> 
>> >
>> >> +			WARN_ON(rvda < OPREGION_SIZE);
>> >> +
>> >> +			rvda += asls;
>> >> +		}
>> >> +
>> >> +		opregion->rvda = memremap(rvda, opregion->asle->rvds,
>> >>  					  MEMREMAP_WB);
>> >> +
>> >>  		vbt = opregion->rvda;
>> >>  		vbt_size = opregion->asle->rvds;
>> >>  		if (intel_bios_is_valid_vbt(vbt, vbt_size)) {
>> >> @@ -976,6 +992,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;
>> >>  		}
>> >>  	}
>> >>  
>> >> -- 
>> >> 2.20.1
>> 
>> -- 
>> Jani Nikula, Intel Open Source Graphics Center

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