linux-kernel - Re: [PATCH v2 2/4] drm/msm/dsi/phy: Protect PHY_CMN_CLK

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <12275e11-eadc-48be-b8c3-9463cdf92698@linaro.org>
Date: Tue, 4 Feb 2025 10:21:25 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc: Rob Clark <robdclark@...il.com>, Abhinav Kumar
 <quic_abhinavk@...cinc.com>, Sean Paul <sean@...rly.run>,
 Marijn Suijten <marijn.suijten@...ainline.org>,
 David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
 linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org,
 freedreno@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/4] drm/msm/dsi/phy: Protect PHY_CMN_CLK_CFG1 against
 clock driver

On 03/02/2025 18:41, Dmitry Baryshkov wrote:
> On Mon, Feb 03, 2025 at 06:29:19PM +0100, Krzysztof Kozlowski wrote:
>> PHY_CMN_CLK_CFG1 register is updated by the PHY driver and by a mux
>> clock from Common Clock Framework:
>> devm_clk_hw_register_mux_parent_hws().  There could be a path leading to
>> concurrent and conflicting updates between PHY driver and clock
>> framework, e.g. changing the mux and enabling PLL clocks.
>>
>> Add dedicated spinlock to be sure all PHY_CMN_CLK_CFG1 updates are
>> synchronized.
>>
>> Fixes: 1ef7c99d145c ("drm/msm/dsi: add support for 7nm DSI PHY/PLL")
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
>>
>> ---
>>
>> Changes in v2:
>> 1. Store BIT(4) and BIT(5) in local var in dsi_pll_enable_global_clk()
>> ---
>>  drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 35 +++++++++++++++++++------------
>>  1 file changed, 22 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
>> index c164f845653816291ad96c863257f75462ef58e7..e26f53f7cde8f0f6419a633f5d39784dc2e5bb98 100644
>> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
>> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
>> @@ -83,6 +83,9 @@ struct dsi_pll_7nm {
>>  	/* protects REG_DSI_7nm_PHY_CMN_CLK_CFG0 register */
>>  	spinlock_t postdiv_lock;
>>  
>> +	/* protects REG_DSI_7nm_PHY_CMN_CLK_CFG1 register */
>> +	spinlock_t pclk_mux_lock;
>> +
>>  	struct pll_7nm_cached_state cached_state;
>>  
>>  	struct dsi_pll_7nm *slave;
>> @@ -381,22 +384,32 @@ static void dsi_pll_cmn_clk_cfg0_write(struct dsi_pll_7nm *pll, u32 val)
>>  	spin_unlock_irqrestore(&pll->postdiv_lock, flags);
>>  }
>>  
>> -static void dsi_pll_disable_global_clk(struct dsi_pll_7nm *pll)
>> +static void dsi_pll_cmn_clk_cfg1_update(struct dsi_pll_7nm *pll, u32 mask,
>> +					u32 val)
>>  {
>> +	unsigned long flags;
>>  	u32 data;
>>  
>> +	spin_lock_irqsave(&pll->pclk_mux_lock, flags);
>>  	data = readl(pll->phy->base + REG_DSI_7nm_PHY_CMN_CLK_CFG1);
>> -	writel(data & ~BIT(5), pll->phy->base + REG_DSI_7nm_PHY_CMN_CLK_CFG1);
>> +	data &= ~mask;
>> +	data |= val & mask;
>> +
>> +	writel(data, pll->phy->base + REG_DSI_7nm_PHY_CMN_CLK_CFG1);
>> +	spin_unlock_irqrestore(&pll->pclk_mux_lock, flags);
>> +}
>> +
>> +static void dsi_pll_disable_global_clk(struct dsi_pll_7nm *pll)
>> +{
>> +	dsi_pll_cmn_clk_cfg1_update(pll, BIT(5), 0);
>>  }
>>  
>>  static void dsi_pll_enable_global_clk(struct dsi_pll_7nm *pll)
>>  {
>> -	u32 data;
>> +	u32 cfg_1 = BIT(5) | BIT(4);
> 
> Please define these two bits too.

Why? They were not defined before. This only moving existing code.


Best regards,
Krzysztof