diff mbox series

[1/2] drm/i915/reg: fix undefined behavior due to shift overflowing the constant

Message ID 20220518113315.1305027-1-jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show
Series [1/2] drm/i915/reg: fix undefined behavior due to shift overflowing the constant | expand

Commit Message

Jani Nikula May 18, 2022, 11:33 a.m. UTC 
Use REG_GENMASK() and REG_FIELD_PREP() to avoid errors due to
-fsanitize=shift.

References: https://lore.kernel.org/r/20220405151517.29753-12-bp@alien8.de
Reported-by: Borislav Petkov <bp@suse.de>
Reported-by: Ruiqi GONG <gongruiqi1@huawei.com>
Cc: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

Comments

Ville Syrjälä May 18, 2022, 12:53 p.m. UTC | #1 
On Wed, May 18, 2022 at 02:33:14PM +0300, Jani Nikula wrote:
> Use REG_GENMASK() and REG_FIELD_PREP() to avoid errors due to
> -fsanitize=shift.

I presume it's just unhappy about shifting into the sign bit?

Changes look correct:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 
> References: https://lore.kernel.org/r/20220405151517.29753-12-bp@alien8.de
> Reported-by: Borislav Petkov <bp@suse.de>
> Reported-by: Ruiqi GONG <gongruiqi1@huawei.com>
> Cc: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 321a08281a3f..dff3f88d8090 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7607,25 +7607,25 @@ enum skl_power_gate {
>  #define _PORT_CLK_SEL_A			0x46100
>  #define _PORT_CLK_SEL_B			0x46104
>  #define PORT_CLK_SEL(port) _MMIO_PORT(port, _PORT_CLK_SEL_A, _PORT_CLK_SEL_B)
> -#define  PORT_CLK_SEL_LCPLL_2700	(0 << 29)
> -#define  PORT_CLK_SEL_LCPLL_1350	(1 << 29)
> -#define  PORT_CLK_SEL_LCPLL_810		(2 << 29)
> -#define  PORT_CLK_SEL_SPLL		(3 << 29)
> -#define  PORT_CLK_SEL_WRPLL(pll)	(((pll) + 4) << 29)
> -#define  PORT_CLK_SEL_WRPLL1		(4 << 29)
> -#define  PORT_CLK_SEL_WRPLL2		(5 << 29)
> -#define  PORT_CLK_SEL_NONE		(7 << 29)
> -#define  PORT_CLK_SEL_MASK		(7 << 29)
> +#define  PORT_CLK_SEL_MASK		REG_GENMASK(31, 29)
> +#define  PORT_CLK_SEL_LCPLL_2700	REG_FIELD_PREP(PORT_CLK_SEL_MASK, 0)
> +#define  PORT_CLK_SEL_LCPLL_1350	REG_FIELD_PREP(PORT_CLK_SEL_MASK, 1)
> +#define  PORT_CLK_SEL_LCPLL_810		REG_FIELD_PREP(PORT_CLK_SEL_MASK, 2)
> +#define  PORT_CLK_SEL_SPLL		REG_FIELD_PREP(PORT_CLK_SEL_MASK, 3)
> +#define  PORT_CLK_SEL_WRPLL(pll)	REG_FIELD_PREP(PORT_CLK_SEL_MASK, 4 + (pll))
> +#define  PORT_CLK_SEL_WRPLL1		REG_FIELD_PREP(PORT_CLK_SEL_MASK, 4)
> +#define  PORT_CLK_SEL_WRPLL2		REG_FIELD_PREP(PORT_CLK_SEL_MASK, 5)
> +#define  PORT_CLK_SEL_NONE		REG_FIELD_PREP(PORT_CLK_SEL_MASK, 7)
>  
>  /* On ICL+ this is the same as PORT_CLK_SEL, but all bits change. */
>  #define DDI_CLK_SEL(port)		PORT_CLK_SEL(port)
> -#define  DDI_CLK_SEL_NONE		(0x0 << 28)
> -#define  DDI_CLK_SEL_MG			(0x8 << 28)
> -#define  DDI_CLK_SEL_TBT_162		(0xC << 28)
> -#define  DDI_CLK_SEL_TBT_270		(0xD << 28)
> -#define  DDI_CLK_SEL_TBT_540		(0xE << 28)
> -#define  DDI_CLK_SEL_TBT_810		(0xF << 28)
> -#define  DDI_CLK_SEL_MASK		(0xF << 28)
> +#define  DDI_CLK_SEL_MASK		REG_GENMASK(31, 28)
> +#define  DDI_CLK_SEL_NONE		REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0x0)
> +#define  DDI_CLK_SEL_MG			REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0x8)
> +#define  DDI_CLK_SEL_TBT_162		REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0xC)
> +#define  DDI_CLK_SEL_TBT_270		REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0xD)
> +#define  DDI_CLK_SEL_TBT_540		REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0xE)
> +#define  DDI_CLK_SEL_TBT_810		REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0xF)
>  
>  /* Transcoder clock selection */
>  #define _TRANS_CLK_SEL_A		0x46140
> -- 
> 2.30.2
Jani Nikula May 18, 2022, 1:52 p.m. UTC | #2 
On Wed, 18 May 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, May 18, 2022 at 02:33:14PM +0300, Jani Nikula wrote:
>> Use REG_GENMASK() and REG_FIELD_PREP() to avoid errors due to
>> -fsanitize=shift.
>
> I presume it's just unhappy about shifting into the sign bit?

Yeah, and apparently it also only happens on some GCC versions. *shrug*.

>
> Changes look correct:
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thanks,
Jani.

>
>> 
>> References: https://lore.kernel.org/r/20220405151517.29753-12-bp@alien8.de
>> Reported-by: Borislav Petkov <bp@suse.de>
>> Reported-by: Ruiqi GONG <gongruiqi1@huawei.com>
>> Cc: Randy Dunlap <rdunlap@infradead.org>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h | 32 ++++++++++++++++----------------
>>  1 file changed, 16 insertions(+), 16 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 321a08281a3f..dff3f88d8090 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -7607,25 +7607,25 @@ enum skl_power_gate {
>>  #define _PORT_CLK_SEL_A			0x46100
>>  #define _PORT_CLK_SEL_B			0x46104
>>  #define PORT_CLK_SEL(port) _MMIO_PORT(port, _PORT_CLK_SEL_A, _PORT_CLK_SEL_B)
>> -#define  PORT_CLK_SEL_LCPLL_2700	(0 << 29)
>> -#define  PORT_CLK_SEL_LCPLL_1350	(1 << 29)
>> -#define  PORT_CLK_SEL_LCPLL_810		(2 << 29)
>> -#define  PORT_CLK_SEL_SPLL		(3 << 29)
>> -#define  PORT_CLK_SEL_WRPLL(pll)	(((pll) + 4) << 29)
>> -#define  PORT_CLK_SEL_WRPLL1		(4 << 29)
>> -#define  PORT_CLK_SEL_WRPLL2		(5 << 29)
>> -#define  PORT_CLK_SEL_NONE		(7 << 29)
>> -#define  PORT_CLK_SEL_MASK		(7 << 29)
>> +#define  PORT_CLK_SEL_MASK		REG_GENMASK(31, 29)
>> +#define  PORT_CLK_SEL_LCPLL_2700	REG_FIELD_PREP(PORT_CLK_SEL_MASK, 0)
>> +#define  PORT_CLK_SEL_LCPLL_1350	REG_FIELD_PREP(PORT_CLK_SEL_MASK, 1)
>> +#define  PORT_CLK_SEL_LCPLL_810		REG_FIELD_PREP(PORT_CLK_SEL_MASK, 2)
>> +#define  PORT_CLK_SEL_SPLL		REG_FIELD_PREP(PORT_CLK_SEL_MASK, 3)
>> +#define  PORT_CLK_SEL_WRPLL(pll)	REG_FIELD_PREP(PORT_CLK_SEL_MASK, 4 + (pll))
>> +#define  PORT_CLK_SEL_WRPLL1		REG_FIELD_PREP(PORT_CLK_SEL_MASK, 4)
>> +#define  PORT_CLK_SEL_WRPLL2		REG_FIELD_PREP(PORT_CLK_SEL_MASK, 5)
>> +#define  PORT_CLK_SEL_NONE		REG_FIELD_PREP(PORT_CLK_SEL_MASK, 7)
>>  
>>  /* On ICL+ this is the same as PORT_CLK_SEL, but all bits change. */
>>  #define DDI_CLK_SEL(port)		PORT_CLK_SEL(port)
>> -#define  DDI_CLK_SEL_NONE		(0x0 << 28)
>> -#define  DDI_CLK_SEL_MG			(0x8 << 28)
>> -#define  DDI_CLK_SEL_TBT_162		(0xC << 28)
>> -#define  DDI_CLK_SEL_TBT_270		(0xD << 28)
>> -#define  DDI_CLK_SEL_TBT_540		(0xE << 28)
>> -#define  DDI_CLK_SEL_TBT_810		(0xF << 28)
>> -#define  DDI_CLK_SEL_MASK		(0xF << 28)
>> +#define  DDI_CLK_SEL_MASK		REG_GENMASK(31, 28)
>> +#define  DDI_CLK_SEL_NONE		REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0x0)
>> +#define  DDI_CLK_SEL_MG			REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0x8)
>> +#define  DDI_CLK_SEL_TBT_162		REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0xC)
>> +#define  DDI_CLK_SEL_TBT_270		REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0xD)
>> +#define  DDI_CLK_SEL_TBT_540		REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0xE)
>> +#define  DDI_CLK_SEL_TBT_810		REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0xF)
>>  
>>  /* Transcoder clock selection */
>>  #define _TRANS_CLK_SEL_A		0x46140
>> -- 
>> 2.30.2
Jani Nikula May 19, 2022, 8:25 a.m. UTC | #3 
On Wed, 18 May 2022, Jani Nikula <jani.nikula@intel.com> wrote:
> On Wed, 18 May 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> On Wed, May 18, 2022 at 02:33:14PM +0300, Jani Nikula wrote:
>>> Use REG_GENMASK() and REG_FIELD_PREP() to avoid errors due to
>>> -fsanitize=shift.
>>
>> I presume it's just unhappy about shifting into the sign bit?
>
> Yeah, and apparently it also only happens on some GCC versions. *shrug*.
>
>>
>> Changes look correct:
>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Thanks,
> Jani.

And pushed 1/2 to drm-intel-next.

BR,
Jani.


>
>>
>>> 
>>> References: https://lore.kernel.org/r/20220405151517.29753-12-bp@alien8.de
>>> Reported-by: Borislav Petkov <bp@suse.de>
>>> Reported-by: Ruiqi GONG <gongruiqi1@huawei.com>
>>> Cc: Randy Dunlap <rdunlap@infradead.org>
>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/i915_reg.h | 32 ++++++++++++++++----------------
>>>  1 file changed, 16 insertions(+), 16 deletions(-)
>>> 
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>> index 321a08281a3f..dff3f88d8090 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -7607,25 +7607,25 @@ enum skl_power_gate {
>>>  #define _PORT_CLK_SEL_A			0x46100
>>>  #define _PORT_CLK_SEL_B			0x46104
>>>  #define PORT_CLK_SEL(port) _MMIO_PORT(port, _PORT_CLK_SEL_A, _PORT_CLK_SEL_B)
>>> -#define  PORT_CLK_SEL_LCPLL_2700	(0 << 29)
>>> -#define  PORT_CLK_SEL_LCPLL_1350	(1 << 29)
>>> -#define  PORT_CLK_SEL_LCPLL_810		(2 << 29)
>>> -#define  PORT_CLK_SEL_SPLL		(3 << 29)
>>> -#define  PORT_CLK_SEL_WRPLL(pll)	(((pll) + 4) << 29)
>>> -#define  PORT_CLK_SEL_WRPLL1		(4 << 29)
>>> -#define  PORT_CLK_SEL_WRPLL2		(5 << 29)
>>> -#define  PORT_CLK_SEL_NONE		(7 << 29)
>>> -#define  PORT_CLK_SEL_MASK		(7 << 29)
>>> +#define  PORT_CLK_SEL_MASK		REG_GENMASK(31, 29)
>>> +#define  PORT_CLK_SEL_LCPLL_2700	REG_FIELD_PREP(PORT_CLK_SEL_MASK, 0)
>>> +#define  PORT_CLK_SEL_LCPLL_1350	REG_FIELD_PREP(PORT_CLK_SEL_MASK, 1)
>>> +#define  PORT_CLK_SEL_LCPLL_810		REG_FIELD_PREP(PORT_CLK_SEL_MASK, 2)
>>> +#define  PORT_CLK_SEL_SPLL		REG_FIELD_PREP(PORT_CLK_SEL_MASK, 3)
>>> +#define  PORT_CLK_SEL_WRPLL(pll)	REG_FIELD_PREP(PORT_CLK_SEL_MASK, 4 + (pll))
>>> +#define  PORT_CLK_SEL_WRPLL1		REG_FIELD_PREP(PORT_CLK_SEL_MASK, 4)
>>> +#define  PORT_CLK_SEL_WRPLL2		REG_FIELD_PREP(PORT_CLK_SEL_MASK, 5)
>>> +#define  PORT_CLK_SEL_NONE		REG_FIELD_PREP(PORT_CLK_SEL_MASK, 7)
>>>  
>>>  /* On ICL+ this is the same as PORT_CLK_SEL, but all bits change. */
>>>  #define DDI_CLK_SEL(port)		PORT_CLK_SEL(port)
>>> -#define  DDI_CLK_SEL_NONE		(0x0 << 28)
>>> -#define  DDI_CLK_SEL_MG			(0x8 << 28)
>>> -#define  DDI_CLK_SEL_TBT_162		(0xC << 28)
>>> -#define  DDI_CLK_SEL_TBT_270		(0xD << 28)
>>> -#define  DDI_CLK_SEL_TBT_540		(0xE << 28)
>>> -#define  DDI_CLK_SEL_TBT_810		(0xF << 28)
>>> -#define  DDI_CLK_SEL_MASK		(0xF << 28)
>>> +#define  DDI_CLK_SEL_MASK		REG_GENMASK(31, 28)
>>> +#define  DDI_CLK_SEL_NONE		REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0x0)
>>> +#define  DDI_CLK_SEL_MG			REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0x8)
>>> +#define  DDI_CLK_SEL_TBT_162		REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0xC)
>>> +#define  DDI_CLK_SEL_TBT_270		REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0xD)
>>> +#define  DDI_CLK_SEL_TBT_540		REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0xE)
>>> +#define  DDI_CLK_SEL_TBT_810		REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0xF)
>>>  
>>>  /* Transcoder clock selection */
>>>  #define _TRANS_CLK_SEL_A		0x46140
>>> -- 
>>> 2.30.2
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 321a08281a3f..dff3f88d8090 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7607,25 +7607,25 @@  enum skl_power_gate {
 #define _PORT_CLK_SEL_A			0x46100
 #define _PORT_CLK_SEL_B			0x46104
 #define PORT_CLK_SEL(port) _MMIO_PORT(port, _PORT_CLK_SEL_A, _PORT_CLK_SEL_B)
-#define  PORT_CLK_SEL_LCPLL_2700	(0 << 29)
-#define  PORT_CLK_SEL_LCPLL_1350	(1 << 29)
-#define  PORT_CLK_SEL_LCPLL_810		(2 << 29)
-#define  PORT_CLK_SEL_SPLL		(3 << 29)
-#define  PORT_CLK_SEL_WRPLL(pll)	(((pll) + 4) << 29)
-#define  PORT_CLK_SEL_WRPLL1		(4 << 29)
-#define  PORT_CLK_SEL_WRPLL2		(5 << 29)
-#define  PORT_CLK_SEL_NONE		(7 << 29)
-#define  PORT_CLK_SEL_MASK		(7 << 29)
+#define  PORT_CLK_SEL_MASK		REG_GENMASK(31, 29)
+#define  PORT_CLK_SEL_LCPLL_2700	REG_FIELD_PREP(PORT_CLK_SEL_MASK, 0)
+#define  PORT_CLK_SEL_LCPLL_1350	REG_FIELD_PREP(PORT_CLK_SEL_MASK, 1)
+#define  PORT_CLK_SEL_LCPLL_810		REG_FIELD_PREP(PORT_CLK_SEL_MASK, 2)
+#define  PORT_CLK_SEL_SPLL		REG_FIELD_PREP(PORT_CLK_SEL_MASK, 3)
+#define  PORT_CLK_SEL_WRPLL(pll)	REG_FIELD_PREP(PORT_CLK_SEL_MASK, 4 + (pll))
+#define  PORT_CLK_SEL_WRPLL1		REG_FIELD_PREP(PORT_CLK_SEL_MASK, 4)
+#define  PORT_CLK_SEL_WRPLL2		REG_FIELD_PREP(PORT_CLK_SEL_MASK, 5)
+#define  PORT_CLK_SEL_NONE		REG_FIELD_PREP(PORT_CLK_SEL_MASK, 7)
 
 /* On ICL+ this is the same as PORT_CLK_SEL, but all bits change. */
 #define DDI_CLK_SEL(port)		PORT_CLK_SEL(port)
-#define  DDI_CLK_SEL_NONE		(0x0 << 28)
-#define  DDI_CLK_SEL_MG			(0x8 << 28)
-#define  DDI_CLK_SEL_TBT_162		(0xC << 28)
-#define  DDI_CLK_SEL_TBT_270		(0xD << 28)
-#define  DDI_CLK_SEL_TBT_540		(0xE << 28)
-#define  DDI_CLK_SEL_TBT_810		(0xF << 28)
-#define  DDI_CLK_SEL_MASK		(0xF << 28)
+#define  DDI_CLK_SEL_MASK		REG_GENMASK(31, 28)
+#define  DDI_CLK_SEL_NONE		REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0x0)
+#define  DDI_CLK_SEL_MG			REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0x8)
+#define  DDI_CLK_SEL_TBT_162		REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0xC)
+#define  DDI_CLK_SEL_TBT_270		REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0xD)
+#define  DDI_CLK_SEL_TBT_540		REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0xE)
+#define  DDI_CLK_SEL_TBT_810		REG_FIELD_PREP(DDI_CLK_SEL_MASK, 0xF)
 
 /* Transcoder clock selection */
 #define _TRANS_CLK_SEL_A		0x46140