* Re: [PATCH] pmdomain: rockchip: Check if smcc could be handled by TA
2025-02-19 0:58 [PATCH] pmdomain: rockchip: Check if smcc could be handled by TA Shawn Lin
@ 2025-02-19 8:17 ` Heiko Stübner
2025-02-19 9:34 ` Steven Price
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Heiko Stübner @ 2025-02-19 8:17 UTC (permalink / raw)
To: Ulf Hansson, Rafael J . Wysocki, Shawn Lin
Cc: linux-rockchip, linux-pm, Shawn Lin, Steven Price
Am Mittwoch, 19. Februar 2025, 01:58:09 MEZ schrieb Shawn Lin:
> Non-existent trusted-firmware could lead smcc calls into some
> unset location which breaks the system.
>
> Reported-by: Steven Price <steven.price@arm.com>
> Cc: Steven Price <steven.price@arm.com>
> Suggested-by: Heiko Stuebner <heiko@sntech.de>
> Fixes: 58ebba35ddab ("pmdomain: rockchip: Add smc call to inform firmware")
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> ---
> Hi Ulf, this's a follow-up patch fixing the issue Steven saw.
>
> drivers/pmdomain/rockchip/pm-domains.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
> index 49842f1..27a5c68 100644
> --- a/drivers/pmdomain/rockchip/pm-domains.c
> +++ b/drivers/pmdomain/rockchip/pm-domains.c
> @@ -572,9 +572,10 @@ static void rockchip_do_pmu_set_power_domain(struct rockchip_pm_domain *pd,
> }
>
> /* Inform firmware to keep this pd on or off */
> - arm_smccc_smc(ROCKCHIP_SIP_SUSPEND_MODE, ROCKCHIP_SLEEP_PD_CONFIG,
> - pmu->info->pwr_offset + pd_pwr_offset,
> - pd->info->pwr_mask, on, 0, 0, 0, &res);
> + if (arm_smccc_1_1_get_conduit() != SMCCC_CONDUIT_NONE)
> + arm_smccc_smc(ROCKCHIP_SIP_SUSPEND_MODE, ROCKCHIP_SLEEP_PD_CONFIG,
> + pmu->info->pwr_offset + pd_pwr_offset,
> + pd->info->pwr_mask, on, 0, 0, 0, &res);
> }
>
> static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] pmdomain: rockchip: Check if smcc could be handled by TA
2025-02-19 0:58 [PATCH] pmdomain: rockchip: Check if smcc could be handled by TA Shawn Lin
2025-02-19 8:17 ` Heiko Stübner
@ 2025-02-19 9:34 ` Steven Price
2025-02-19 11:51 ` Ulf Hansson
2025-02-19 10:03 ` Sudeep Holla
` (2 subsequent siblings)
4 siblings, 1 reply; 7+ messages in thread
From: Steven Price @ 2025-02-19 9:34 UTC (permalink / raw)
To: Shawn Lin, Ulf Hansson, Heiko Stuebner, Rafael J . Wysocki
Cc: linux-rockchip, linux-pm
On 19/02/2025 00:58, Shawn Lin wrote:
> Non-existent trusted-firmware could lead smcc calls into some
> unset location which breaks the system.
>
> Reported-by: Steven Price <steven.price@arm.com>
> Cc: Steven Price <steven.price@arm.com>
> Suggested-by: Heiko Stuebner <heiko@sntech.de>
> Fixes: 58ebba35ddab ("pmdomain: rockchip: Add smc call to inform firmware")
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
Tested-by: Steven Price <steven.price@arm.com>
Although one note below...
> ---
> Hi Ulf, this's a follow-up patch fixing the issue Steven saw.
>
> drivers/pmdomain/rockchip/pm-domains.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
> index 49842f1..27a5c68 100644
> --- a/drivers/pmdomain/rockchip/pm-domains.c
> +++ b/drivers/pmdomain/rockchip/pm-domains.c
> @@ -572,9 +572,10 @@ static void rockchip_do_pmu_set_power_domain(struct rockchip_pm_domain *pd,
> }
>
> /* Inform firmware to keep this pd on or off */
> - arm_smccc_smc(ROCKCHIP_SIP_SUSPEND_MODE, ROCKCHIP_SLEEP_PD_CONFIG,
> - pmu->info->pwr_offset + pd_pwr_offset,
> - pd->info->pwr_mask, on, 0, 0, 0, &res);
> + if (arm_smccc_1_1_get_conduit() != SMCCC_CONDUIT_NONE)
> + arm_smccc_smc(ROCKCHIP_SIP_SUSPEND_MODE, ROCKCHIP_SLEEP_PD_CONFIG,
> + pmu->info->pwr_offset + pd_pwr_offset,
> + pd->info->pwr_mask, on, 0, 0, 0, &res);
Note that if the conduit is SMCCC_CONDUIT_HVC then this will still
attempt an SMC. I'm not sure if this situation can happen in practice.
There is a (horrifyingly complex) macro arm_smccc_1_1_invoke() which
will automatically use the correct conduit, and even copes with the
SMCCC_CONDUIT_NONE case (by simply failing the call).
Steve
> }
>
> static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] pmdomain: rockchip: Check if smcc could be handled by TA
2025-02-19 9:34 ` Steven Price
@ 2025-02-19 11:51 ` Ulf Hansson
0 siblings, 0 replies; 7+ messages in thread
From: Ulf Hansson @ 2025-02-19 11:51 UTC (permalink / raw)
To: Shawn Lin, Steven Price
Cc: Heiko Stuebner, Rafael J . Wysocki, linux-rockchip, linux-pm,
Sudeep Holla
+ Sudeep
On Wed, 19 Feb 2025 at 10:34, Steven Price <steven.price@arm.com> wrote:
>
> On 19/02/2025 00:58, Shawn Lin wrote:
> > Non-existent trusted-firmware could lead smcc calls into some
> > unset location which breaks the system.
> >
> > Reported-by: Steven Price <steven.price@arm.com>
> > Cc: Steven Price <steven.price@arm.com>
> > Suggested-by: Heiko Stuebner <heiko@sntech.de>
> > Fixes: 58ebba35ddab ("pmdomain: rockchip: Add smc call to inform firmware")
> > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>
> Tested-by: Steven Price <steven.price@arm.com>
>
> Although one note below...
>
> > ---
> > Hi Ulf, this's a follow-up patch fixing the issue Steven saw.
> >
> > drivers/pmdomain/rockchip/pm-domains.c | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
> > index 49842f1..27a5c68 100644
> > --- a/drivers/pmdomain/rockchip/pm-domains.c
> > +++ b/drivers/pmdomain/rockchip/pm-domains.c
> > @@ -572,9 +572,10 @@ static void rockchip_do_pmu_set_power_domain(struct rockchip_pm_domain *pd,
> > }
> >
> > /* Inform firmware to keep this pd on or off */
> > - arm_smccc_smc(ROCKCHIP_SIP_SUSPEND_MODE, ROCKCHIP_SLEEP_PD_CONFIG,
> > - pmu->info->pwr_offset + pd_pwr_offset,
> > - pd->info->pwr_mask, on, 0, 0, 0, &res);
> > + if (arm_smccc_1_1_get_conduit() != SMCCC_CONDUIT_NONE)
> > + arm_smccc_smc(ROCKCHIP_SIP_SUSPEND_MODE, ROCKCHIP_SLEEP_PD_CONFIG,
> > + pmu->info->pwr_offset + pd_pwr_offset,
> > + pd->info->pwr_mask, on, 0, 0, 0, &res);
>
> Note that if the conduit is SMCCC_CONDUIT_HVC then this will still
> attempt an SMC. I'm not sure if this situation can happen in practice.
>
> There is a (horrifyingly complex) macro arm_smccc_1_1_invoke() which
> will automatically use the correct conduit, and even copes with the
> SMCCC_CONDUIT_NONE case (by simply failing the call).
>
> Steve
Thanks for letting us know!
Note that, arm_smccc_1_1_invoke() is also a bit problematic, as it
doesn't compile with Clang-20 and Thumb2 mode. See commit
885f5669f2ab.
[...]
As the current approach seems fine too, I decided to pick up the
$patch as is and by updating the commit message, according to Sudeep's
comment.
Kind regards
Uffe
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] pmdomain: rockchip: Check if smcc could be handled by TA
2025-02-19 0:58 [PATCH] pmdomain: rockchip: Check if smcc could be handled by TA Shawn Lin
2025-02-19 8:17 ` Heiko Stübner
2025-02-19 9:34 ` Steven Price
@ 2025-02-19 10:03 ` Sudeep Holla
2025-02-20 6:04 ` kernel test robot
2025-02-20 6:46 ` kernel test robot
4 siblings, 0 replies; 7+ messages in thread
From: Sudeep Holla @ 2025-02-19 10:03 UTC (permalink / raw)
To: Shawn Lin
Cc: Ulf Hansson, Heiko Stuebner, Rafael J . Wysocki, Sudeep Holla,
linux-rockchip, linux-pm, Steven Price
On Wed, Feb 19, 2025 at 08:58:09AM +0800, Shawn Lin wrote:
> Non-existent trusted-firmware could lead smcc calls into some
> unset location which breaks the system.
>
s/smcc/SMC or s/smcc/SMCCC please. There is nothing called smcc
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] pmdomain: rockchip: Check if smcc could be handled by TA
2025-02-19 0:58 [PATCH] pmdomain: rockchip: Check if smcc could be handled by TA Shawn Lin
` (2 preceding siblings ...)
2025-02-19 10:03 ` Sudeep Holla
@ 2025-02-20 6:04 ` kernel test robot
2025-02-20 6:46 ` kernel test robot
4 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2025-02-20 6:04 UTC (permalink / raw)
To: Shawn Lin, Ulf Hansson, Heiko Stuebner, Rafael J . Wysocki
Cc: oe-kbuild-all, linux-rockchip, linux-pm, Shawn Lin, Steven Price
Hi Shawn,
kernel test robot noticed the following build errors:
[auto build test ERROR on next-20250218]
[cannot apply to v6.14-rc3 v6.14-rc2 v6.14-rc1 linus/master v6.14-rc3]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Shawn-Lin/pmdomain-rockchip-Check-if-smcc-could-be-handled-by-TA/20250219-122924
base: next-20250218
patch link: https://lore.kernel.org/r/1739926689-151827-1-git-send-email-shawn.lin%40rock-chips.com
patch subject: [PATCH] pmdomain: rockchip: Check if smcc could be handled by TA
config: i386-buildonly-randconfig-001-20250220 (https://download.01.org/0day-ci/archive/20250220/202502201601.rQYwZmA8-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250220/202502201601.rQYwZmA8-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202502201601.rQYwZmA8-lkp@intel.com/
All errors (new ones prefixed by >>):
ld: drivers/pmdomain/rockchip/pm-domains.o: in function `rockchip_do_pmu_set_power_domain':
>> drivers/pmdomain/rockchip/pm-domains.c:575: undefined reference to `arm_smccc_1_1_get_conduit'
vim +575 drivers/pmdomain/rockchip/pm-domains.c
537
538 static void rockchip_do_pmu_set_power_domain(struct rockchip_pm_domain *pd,
539 bool on)
540 {
541 struct rockchip_pmu *pmu = pd->pmu;
542 struct generic_pm_domain *genpd = &pd->genpd;
543 u32 pd_pwr_offset = pd->info->pwr_offset;
544 bool is_on, is_mem_on = false;
545 struct arm_smccc_res res;
546
547 if (pd->info->pwr_mask == 0)
548 return;
549
550 if (on && pd->info->mem_status_mask)
551 is_mem_on = rockchip_pmu_domain_is_mem_on(pd);
552
553 if (pd->info->pwr_w_mask)
554 regmap_write(pmu->regmap, pmu->info->pwr_offset + pd_pwr_offset,
555 on ? pd->info->pwr_w_mask :
556 (pd->info->pwr_mask | pd->info->pwr_w_mask));
557 else
558 regmap_update_bits(pmu->regmap, pmu->info->pwr_offset + pd_pwr_offset,
559 pd->info->pwr_mask, on ? 0 : -1U);
560
561 wmb();
562
563 if (is_mem_on && rockchip_pmu_domain_mem_reset(pd))
564 return;
565
566 if (readx_poll_timeout_atomic(rockchip_pmu_domain_is_on, pd, is_on,
567 is_on == on, 0, 10000)) {
568 dev_err(pmu->dev,
569 "failed to set domain '%s', val=%d\n",
570 genpd->name, is_on);
571 return;
572 }
573
574 /* Inform firmware to keep this pd on or off */
> 575 if (arm_smccc_1_1_get_conduit() != SMCCC_CONDUIT_NONE)
576 arm_smccc_smc(ROCKCHIP_SIP_SUSPEND_MODE, ROCKCHIP_SLEEP_PD_CONFIG,
577 pmu->info->pwr_offset + pd_pwr_offset,
578 pd->info->pwr_mask, on, 0, 0, 0, &res);
579 }
580
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] pmdomain: rockchip: Check if smcc could be handled by TA
2025-02-19 0:58 [PATCH] pmdomain: rockchip: Check if smcc could be handled by TA Shawn Lin
` (3 preceding siblings ...)
2025-02-20 6:04 ` kernel test robot
@ 2025-02-20 6:46 ` kernel test robot
4 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2025-02-20 6:46 UTC (permalink / raw)
To: Shawn Lin, Ulf Hansson, Heiko Stuebner, Rafael J . Wysocki
Cc: oe-kbuild-all, linux-rockchip, linux-pm, Shawn Lin, Steven Price
Hi Shawn,
kernel test robot noticed the following build errors:
[auto build test ERROR on next-20250218]
[cannot apply to v6.14-rc3 v6.14-rc2 v6.14-rc1 linus/master v6.14-rc3]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Shawn-Lin/pmdomain-rockchip-Check-if-smcc-could-be-handled-by-TA/20250219-122924
base: next-20250218
patch link: https://lore.kernel.org/r/1739926689-151827-1-git-send-email-shawn.lin%40rock-chips.com
patch subject: [PATCH] pmdomain: rockchip: Check if smcc could be handled by TA
config: x86_64-buildonly-randconfig-005-20250220 (https://download.01.org/0day-ci/archive/20250220/202502201459.wqX8VWY9-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250220/202502201459.wqX8VWY9-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202502201459.wqX8VWY9-lkp@intel.com/
All errors (new ones prefixed by >>):
ld: drivers/pmdomain/rockchip/pm-domains.o: in function `rockchip_pd_power':
>> pm-domains.c:(.text+0x3bad): undefined reference to `arm_smccc_1_1_get_conduit'
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 7+ messages in thread