[0/3] Add power domain driver support for i.mx8m family
mbox series

Message ID 20190417053211.2195-1-ping.bai@nxp.com
Headers show
Series
  • Add power domain driver support for i.mx8m family
Related show

Message

Jacky Bai April 17, 2019, 5:27 a.m. UTC
The i.MX8M family is a set of NXP product focus on delivering
the latest and greatest video and audio experience combining
state-of-the-art media-specific features with high-performance
processing while optimized for lowest power consumption.
i.MX8MQ, i.MX8MM, i.MX8MN, even the furture i.MX8MP are all
belong to this family.

The GPC module is used to manage the PU power domains' power
on/off. For the whole i.MX8M family, different SoC has differnt
power domain design. the power up sequence has significant difference.
all the power sequence must be guaranteed by SW. Some domains' power
up sequence need to access the SRC module or sub-system specific GPR.
the SRC register & SS's register are not in in the GPC's memory range.

it makes us hard to use the GPCv2 driver to cover all the different
power up requirement. Each time, a new SoC is added, we must modify
the GPCv2 driver to make it resuable for it. a lot of code need to
be added in GPCv2 to support it. we need to access the SRC & SS' GPR,
then the GPCv2 driver can NOT be self-contained. Accessing the non-driver
specific module's register is a bad practice. Although, the GPC module
provided the similar function for PU power domain, but it is not 100%
compatible with GPCv2.

The most important thing is that the GPC & SRC module is a security
critical resource that security permission must be considered when
building the security system. The GPC module is not only used by
PU power domain power on/off. It is also used by the TF-A PSCI code
to do the CPU core power management. the SRC module control the
CPU CORE reset and the CPU reset vector address. if we give the
non-secure world write permission to SRC. System can be easily
induced to malicious code.

This patchset add a more generic power domain driver that give
us the possibility to use one driver to cover the whole i.MX8M
family power domain in kernel side. kernel side doesn't need to
handle the power domain difference anymore, all the sequence can be
abstracted & handled in TF-A side. Most important, We don't need to
care if the GPC & SRC is security protected.

Jacky Bai (3):
  dt-bindings: power: Add power domain binding for i.mx8m family
  soc: imx: Add power domain driver support for i.mx8m family
  arm64: dts: freescale: Add power domain nodes for i.mx8mm

 .../bindings/power/fsl,imx8m-genpd.txt        |  46 ++++
 arch/arm64/boot/dts/freescale/imx8mm.dtsi     | 103 ++++++++
 drivers/soc/imx/Kconfig                       |   6 +
 drivers/soc/imx/Makefile                      |   1 +
 drivers/soc/imx/imx8m_pm_domains.c            | 224 ++++++++++++++++++
 include/soc/imx/imx_sip.h                     |  12 +
 6 files changed, 392 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/fsl,imx8m-genpd.txt
 create mode 100644 drivers/soc/imx/imx8m_pm_domains.c
 create mode 100644 include/soc/imx/imx_sip.h

Comments

Aisheng Dong April 17, 2019, 11:16 a.m. UTC | #1
> From: Jacky Bai
> Sent: Wednesday, April 17, 2019 1:27 PM
> 
> The i.MX8M family is a set of NXP product focus on delivering the latest and
> greatest video and audio experience combining state-of-the-art media-specific
> features with high-performance processing while optimized for lowest power
> consumption.
> i.MX8MQ, i.MX8MM, i.MX8MN, even the furture i.MX8MP are all belong to
> this family.
> 
> The GPC module is used to manage the PU power domains' power on/off. For
> the whole i.MX8M family, different SoC has differnt power domain design. the
> power up sequence has significant difference.
> all the power sequence must be guaranteed by SW. Some domains' power up
> sequence need to access the SRC module or sub-system specific GPR.
> the SRC register & SS's register are not in in the GPC's memory range.
> 
> it makes us hard to use the GPCv2 driver to cover all the different power up
> requirement. Each time, a new SoC is added, we must modify the GPCv2 driver
> to make it resuable for it. a lot of code need to be added in GPCv2 to support it.
> we need to access the SRC & SS' GPR, then the GPCv2 driver can NOT be
> self-contained. Accessing the non-driver specific module's register is a bad
> practice. Although, the GPC module provided the similar function for PU power
> domain, but it is not 100% compatible with GPCv2.
> 
> The most important thing is that the GPC & SRC module is a security critical
> resource that security permission must be considered when building the
> security system. The GPC module is not only used by PU power domain power
> on/off. It is also used by the TF-A PSCI code to do the CPU core power
> management. the SRC module control the CPU CORE reset and the CPU reset
> vector address. if we give the non-secure world write permission to SRC.
> System can be easily induced to malicious code.
> 

Considering the security issue, it looks to me a right direction to move GPC
power handling into ATF.
It also helps build a more generic driver and ease other OS integration
needed by customers (e.g. QNX, Win10).

Lucas,
How do you think of it?

Regards
Dong Aisheng

> This patchset add a more generic power domain driver that give us the
> possibility to use one driver to cover the whole i.MX8M family power domain
> in kernel side. kernel side doesn't need to handle the power domain difference
> anymore, all the sequence can be abstracted & handled in TF-A side. Most
> important, We don't need to care if the GPC & SRC is security protected.
> 
> Jacky Bai (3):
>   dt-bindings: power: Add power domain binding for i.mx8m family
>   soc: imx: Add power domain driver support for i.mx8m family
>   arm64: dts: freescale: Add power domain nodes for i.mx8mm
> 
>  .../bindings/power/fsl,imx8m-genpd.txt        |  46 ++++
>  arch/arm64/boot/dts/freescale/imx8mm.dtsi     | 103 ++++++++
>  drivers/soc/imx/Kconfig                       |   6 +
>  drivers/soc/imx/Makefile                      |   1 +
>  drivers/soc/imx/imx8m_pm_domains.c            | 224
> ++++++++++++++++++
>  include/soc/imx/imx_sip.h                     |  12 +
>  6 files changed, 392 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/power/fsl,imx8m-genpd.txt
>  create mode 100644 drivers/soc/imx/imx8m_pm_domains.c
>  create mode 100644 include/soc/imx/imx_sip.h
> 
> --
> 2.21.0
Lucas Stach April 17, 2019, 12:13 p.m. UTC | #2
Am Mittwoch, den 17.04.2019, 11:16 +0000 schrieb Aisheng Dong:
> > From: Jacky Bai
> > Sent: Wednesday, April 17, 2019 1:27 PM
> > 
> > The i.MX8M family is a set of NXP product focus on delivering the latest and
> > greatest video and audio experience combining state-of-the-art media-specific
> > features with high-performance processing while optimized for lowest power
> > consumption.
> > i.MX8MQ, i.MX8MM, i.MX8MN, even the furture i.MX8MP are all belong to
> > this family.
> > 
> > The GPC module is used to manage the PU power domains' power on/off. For
> > the whole i.MX8M family, different SoC has differnt power domain design. the
> > power up sequence has significant difference.
> > all the power sequence must be guaranteed by SW. Some domains' power up
> > sequence need to access the SRC module or sub-system specific GPR.
> > the SRC register & SS's register are not in in the GPC's memory range.
> > 
> > it makes us hard to use the GPCv2 driver to cover all the different power up
> > requirement. Each time, a new SoC is added, we must modify the GPCv2 driver
> > to make it resuable for it. a lot of code need to be added in GPCv2 to support it.
> > we need to access the SRC & SS' GPR, then the GPCv2 driver can NOT be
> > self-contained. Accessing the non-driver specific module's register is a bad
> > practice. Although, the GPC module provided the similar function for PU power
> > domain, but it is not 100% compatible with GPCv2.
> > 
> > The most important thing is that the GPC & SRC module is a security critical
> > resource that security permission must be considered when building the
> > security system. The GPC module is not only used by PU power domain power
> > on/off. It is also used by the TF-A PSCI code to do the CPU core power
> > management. the SRC module control the CPU CORE reset and the CPU reset
> > vector address. if we give the non-secure world write permission to SRC.
> > System can be easily induced to malicious code.
> > 
> 
> Considering the security issue, it looks to me a right direction to move GPC
> power handling into ATF.
> It also helps build a more generic driver and ease other OS integration
> needed by customers (e.g. QNX, Win10).
> 
> Lucas,
> How do you think of it?

I don't yet buy the security argument. There are many more shared parts
on the SoC, like the clock controller, that would need to be taken away
from the non-secure world if one would want to run an untrusted OS
kernel on a i.MX8M system.

To properly implement security on any i.MX8M based system the firmware
would need to grow something like a full ARM SCPI implementation, so
all shared critical peripherals are solely under firmware control.

I agree that it might make sense to move some parts into the firmware
and have much simpler OS level drivers, but I don't agree on the
implementation direction taken here. Growing custom PSCI extension
interfaces will only get us so far, without solving the system security
issue in a holistic way. It is my strong believe that only a complete
rearchitecture of the OS support on top of a ARM SCPI firmware
interface can solve this properly.

Regards,
Lucas
Leonard Crestez April 17, 2019, 12:40 p.m. UTC | #3
On 4/17/2019 3:13 PM, Lucas Stach wrote:
> Am Mittwoch, den 17.04.2019, 11:16 +0000 schrieb Aisheng Dong:
>>> From: Jacky Bai
>>> Sent: Wednesday, April 17, 2019 1:27 PM
>>>
>>> The i.MX8M family is a set of NXP product focus on delivering the latest and
>>> greatest video and audio experience combining state-of-the-art media-specific
>>> features with high-performance processing while optimized for lowest power
>>> consumption.
>>> i.MX8MQ, i.MX8MM, i.MX8MN, even the furture i.MX8MP are all belong to
>>> this family.
>>>
>>> The GPC module is used to manage the PU power domains' power on/off. For
>>> the whole i.MX8M family, different SoC has differnt power domain design. the
>>> power up sequence has significant difference.
>>> all the power sequence must be guaranteed by SW. Some domains' power up
>>> sequence need to access the SRC module or sub-system specific GPR.
>>> the SRC register & SS's register are not in in the GPC's memory range.
>>>
>>> it makes us hard to use the GPCv2 driver to cover all the different power up
>>> requirement. Each time, a new SoC is added, we must modify the GPCv2 driver
>>> to make it resuable for it. a lot of code need to be added in GPCv2 to support it.
>>> we need to access the SRC & SS' GPR, then the GPCv2 driver can NOT be
>>> self-contained. Accessing the non-driver specific module's register is a bad
>>> practice. Although, the GPC module provided the similar function for PU power
>>> domain, but it is not 100% compatible with GPCv2.
>>>
>>> The most important thing is that the GPC & SRC module is a security critical
>>> resource that security permission must be considered when building the
>>> security system. The GPC module is not only used by PU power domain power
>>> on/off. It is also used by the TF-A PSCI code to do the CPU core power
>>> management. the SRC module control the CPU CORE reset and the CPU reset
>>> vector address. if we give the non-secure world write permission to SRC.
>>> System can be easily induced to malicious code.
>>
>> Considering the security issue, it looks to me a right direction to move GPC
>> power handling into ATF.
>> It also helps build a more generic driver and ease other OS integration
>> needed by customers (e.g. QNX, Win10).
>>
>> Lucas,
>> How do you think of it?
> 
> I don't yet buy the security argument. There are many more shared parts
> on the SoC, like the clock controller, that would need to be taken away
> from the non-secure world if one would want to run an untrusted OS
> kernel on a i.MX8M system.
> 
> To properly implement security on any i.MX8M based system the firmware
> would need to grow something like a full ARM SCPI implementation, so
> all shared critical peripherals are solely under firmware control.

It might be possible to rework this to use some form of SCMI-over-SMC 
instead of vendor-specific SMCCC SIP calls

+SCMI maintainer

> I agree that it might make sense to move some parts into the firmware
> and have much simpler OS level drivers, but I don't agree on the
> implementation direction taken here. Growing custom PSCI extension
> interfaces will only get us so far, without solving the system security
> issue in a holistic way. It is my strong believe that only a complete
> rearchitecture of the OS support on top of a ARM SCPI firmware
> interface can solve this properly.
Hiding everything critical for security (especially CCM) behind a SCMI 
interface would be a large amount of work but introducing SCMI 
incrementally (starting with imx8mm power) would be useful by itself 
because it simplifies OS implementation.

Many at NXP have attempted to evaluate SCMI and their conclusion has 
always been that "many extensions are required".

--
Regards,
Leonard
Lucas Stach April 17, 2019, 12:54 p.m. UTC | #4
Am Mittwoch, den 17.04.2019, 12:40 +0000 schrieb Leonard Crestez:
> On 4/17/2019 3:13 PM, Lucas Stach wrote:
> > Am Mittwoch, den 17.04.2019, 11:16 +0000 schrieb Aisheng Dong:
> > > > From: Jacky Bai
> > > > Sent: Wednesday, April 17, 2019 1:27 PM
> > > > 
> > > > The i.MX8M family is a set of NXP product focus on delivering the latest and
> > > > greatest video and audio experience combining state-of-the-art media-specific
> > > > features with high-performance processing while optimized for lowest power
> > > > consumption.
> > > > i.MX8MQ, i.MX8MM, i.MX8MN, even the furture i.MX8MP are all belong to
> > > > this family.
> > > > 
> > > > The GPC module is used to manage the PU power domains' power on/off. For
> > > > the whole i.MX8M family, different SoC has differnt power domain design. the
> > > > power up sequence has significant difference.
> > > > all the power sequence must be guaranteed by SW. Some domains' power up
> > > > sequence need to access the SRC module or sub-system specific GPR.
> > > > the SRC register & SS's register are not in in the GPC's memory range.
> > > > 
> > > > it makes us hard to use the GPCv2 driver to cover all the different power up
> > > > requirement. Each time, a new SoC is added, we must modify the GPCv2 driver
> > > > to make it resuable for it. a lot of code need to be added in GPCv2 to support it.
> > > > we need to access the SRC & SS' GPR, then the GPCv2 driver can NOT be
> > > > self-contained. Accessing the non-driver specific module's register is a bad
> > > > practice. Although, the GPC module provided the similar function for PU power
> > > > domain, but it is not 100% compatible with GPCv2.
> > > > 
> > > > The most important thing is that the GPC & SRC module is a security critical
> > > > resource that security permission must be considered when building the
> > > > security system. The GPC module is not only used by PU power domain power
> > > > on/off. It is also used by the TF-A PSCI code to do the CPU core power
> > > > management. the SRC module control the CPU CORE reset and the CPU reset
> > > > vector address. if we give the non-secure world write permission to SRC.
> > > > System can be easily induced to malicious code.
> > > 
> > > Considering the security issue, it looks to me a right direction to move GPC
> > > power handling into ATF.
> > > It also helps build a more generic driver and ease other OS integration
> > > needed by customers (e.g. QNX, Win10).
> > > 
> > > Lucas,
> > > How do you think of it?
> > 
> > I don't yet buy the security argument. There are many more shared parts
> > on the SoC, like the clock controller, that would need to be taken away
> > from the non-secure world if one would want to run an untrusted OS
> > kernel on a i.MX8M system.
> > 
> > To properly implement security on any i.MX8M based system the firmware
> > would need to grow something like a full ARM SCPI implementation, so
> > all shared critical peripherals are solely under firmware control.
> 
> It might be possible to rework this to use some form of SCMI-over-SMC 
> instead of vendor-specific SMCCC SIP calls
> 
> +SCMI maintainer
> 
> > I agree that it might make sense to move some parts into the firmware
> > and have much simpler OS level drivers, but I don't agree on the
> > implementation direction taken here. Growing custom PSCI extension
> > interfaces will only get us so far, without solving the system security
> > issue in a holistic way. It is my strong believe that only a complete
> > rearchitecture of the OS support on top of a ARM SCPI firmware
> > interface can solve this properly.
> 
> Hiding everything critical for security (especially CCM) behind a SCMI 
> interface would be a large amount of work but introducing SCMI 
> incrementally (starting with imx8mm power) would be useful by itself 
> because it simplifies OS implementation.

I'm totally on-board with baby steps if it gets us into the right
direction, so what you propose makes sense to me.

What I'm not okay with is tying the upstream kernel into an ad-hoc
firmware interface in the name of system security, if there is no
chance that proper security will ever be able to be achieved with the
ad-hoc interface. Having dependencies between the OS kernel and system
firmware is a burden on system integrators and I'm only willing to
accept that burden if it's clear that there is some real value to it.

Regards,
Lucas
Peng Fan April 17, 2019, 12:54 p.m. UTC | #5
> Subject: Re: [PATCH 0/3] Add power domain driver support for i.mx8m family
> 
> On 4/17/2019 3:13 PM, Lucas Stach wrote:
> > Am Mittwoch, den 17.04.2019, 11:16 +0000 schrieb Aisheng Dong:
> >>> From: Jacky Bai
> >>> Sent: Wednesday, April 17, 2019 1:27 PM
> >>>
> >>> The i.MX8M family is a set of NXP product focus on delivering the
> >>> latest and greatest video and audio experience combining
> >>> state-of-the-art media-specific features with high-performance
> >>> processing while optimized for lowest power consumption.
> >>> i.MX8MQ, i.MX8MM, i.MX8MN, even the furture i.MX8MP are all belong
> >>> to this family.
> >>>
> >>> The GPC module is used to manage the PU power domains' power on/off.
> >>> For the whole i.MX8M family, different SoC has differnt power domain
> >>> design. the power up sequence has significant difference.
> >>> all the power sequence must be guaranteed by SW. Some domains'
> power
> >>> up sequence need to access the SRC module or sub-system specific GPR.
> >>> the SRC register & SS's register are not in in the GPC's memory range.
> >>>
> >>> it makes us hard to use the GPCv2 driver to cover all the different
> >>> power up requirement. Each time, a new SoC is added, we must modify
> >>> the GPCv2 driver to make it resuable for it. a lot of code need to be added
> in GPCv2 to support it.
> >>> we need to access the SRC & SS' GPR, then the GPCv2 driver can NOT
> >>> be self-contained. Accessing the non-driver specific module's
> >>> register is a bad practice. Although, the GPC module provided the
> >>> similar function for PU power domain, but it is not 100% compatible with
> GPCv2.
> >>>
> >>> The most important thing is that the GPC & SRC module is a security
> >>> critical resource that security permission must be considered when
> >>> building the security system. The GPC module is not only used by PU
> >>> power domain power on/off. It is also used by the TF-A PSCI code to
> >>> do the CPU core power management. the SRC module control the CPU
> >>> CORE reset and the CPU reset vector address. if we give the non-secure
> world write permission to SRC.
> >>> System can be easily induced to malicious code.
> >>
> >> Considering the security issue, it looks to me a right direction to
> >> move GPC power handling into ATF.
> >> It also helps build a more generic driver and ease other OS
> >> integration needed by customers (e.g. QNX, Win10).
> >>
> >> Lucas,
> >> How do you think of it?
> >
> > I don't yet buy the security argument. There are many more shared
> > parts on the SoC, like the clock controller, that would need to be
> > taken away from the non-secure world if one would want to run an
> > untrusted OS kernel on a i.MX8M system.
> >
> > To properly implement security on any i.MX8M based system the firmware
> > would need to grow something like a full ARM SCPI implementation, so
> > all shared critical peripherals are solely under firmware control.
> 
> It might be possible to rework this to use some form of SCMI-over-SMC
> instead of vendor-specific SMCCC SIP calls

Whether SCMI or just SIP, it will make it easy to support virtualization(partition)
or TEE.

> 
> +SCMI maintainer

We need implement firmware in ATF, and use SMC as the mailbox.
I have taken Andre's previous patch to support smc mailbox and addressed
some comments, and trying integrate with SCMI.
The major issue is SCMI spec does not include SMC support.

Sudeep, do you have any suggestions?

Thanks,
Peng.
> 
> > I agree that it might make sense to move some parts into the firmware
> > and have much simpler OS level drivers, but I don't agree on the
> > implementation direction taken here. Growing custom PSCI extension
> > interfaces will only get us so far, without solving the system
> > security issue in a holistic way. It is my strong believe that only a
> > complete rearchitecture of the OS support on top of a ARM SCPI
> > firmware interface can solve this properly.
> Hiding everything critical for security (especially CCM) behind a SCMI
> interface would be a large amount of work but introducing SCMI
> incrementally (starting with imx8mm power) would be useful by itself because
> it simplifies OS implementation.
> 
> Many at NXP have attempted to evaluate SCMI and their conclusion has
> always been that "many extensions are required".
> 
> --
> Regards,
> Leonard
Sudeep Holla April 17, 2019, 1:23 p.m. UTC | #6
On 17/04/2019 13:13, Lucas Stach wrote:
> Am Mittwoch, den 17.04.2019, 11:16 +0000 schrieb Aisheng Dong:
>>> From: Jacky Bai
>>> Sent: Wednesday, April 17, 2019 1:27 PM
>>>
>>> The i.MX8M family is a set of NXP product focus on delivering the latest and
>>> greatest video and audio experience combining state-of-the-art media-specific
>>> features with high-performance processing while optimized for lowest power
>>> consumption.
>>> i.MX8MQ, i.MX8MM, i.MX8MN, even the furture i.MX8MP are all belong to
>>> this family.
>>>
>>> The GPC module is used to manage the PU power domains' power on/off. For
>>> the whole i.MX8M family, different SoC has differnt power domain design. the
>>> power up sequence has significant difference.
>>> all the power sequence must be guaranteed by SW. Some domains' power up
>>> sequence need to access the SRC module or sub-system specific GPR.
>>> the SRC register & SS's register are not in in the GPC's memory range.
>>>
>>> it makes us hard to use the GPCv2 driver to cover all the different power up
>>> requirement. Each time, a new SoC is added, we must modify the GPCv2 driver
>>> to make it resuable for it. a lot of code need to be added in GPCv2 to support it.
>>> we need to access the SRC & SS' GPR, then the GPCv2 driver can NOT be
>>> self-contained. Accessing the non-driver specific module's register is a bad
>>> practice. Although, the GPC module provided the similar function for PU power
>>> domain, but it is not 100% compatible with GPCv2.
>>>
>>> The most important thing is that the GPC & SRC module is a security critical
>>> resource that security permission must be considered when building the
>>> security system. The GPC module is not only used by PU power domain power
>>> on/off. It is also used by the TF-A PSCI code to do the CPU core power
>>> management. the SRC module control the CPU CORE reset and the CPU reset
>>> vector address. if we give the non-secure world write permission to SRC.
>>> System can be easily induced to malicious code.
>>>
>>
>> Considering the security issue, it looks to me a right direction to move GPC
>> power handling into ATF.
>> It also helps build a more generic driver and ease other OS integration
>> needed by customers (e.g. QNX, Win10).
>>
>> Lucas,
>> How do you think of it?
>
> I don't yet buy the security argument. There are many more shared parts
> on the SoC, like the clock controller, that would need to be taken away
> from the non-secure world if one would want to run an untrusted OS
> kernel on a i.MX8M system.
>
> To properly implement security on any i.MX8M based system the firmware
> would need to grow something like a full ARM SCPI implementation, so
> all shared critical peripherals are solely under firmware control.
>
> I agree that it might make sense to move some parts into the firmware
> and have much simpler OS level drivers, but I don't agree on the
> implementation direction taken here. Growing custom PSCI extension
> interfaces will only get us so far, without solving the system security
> issue in a holistic way. It is my strong believe that only a complete
> rearchitecture of the OS support on top of a ARM SCPI firmware
> interface can solve this properly.
>

+1 for all the above.

For me, though the intention looks good, this implementation is half
cooked and just creating more mess with lots of custom SMC extensions.
You can use them in products, but I would not like in upstream which is
more focused on long term maintenance.

--
Regards,
Sudeep



--
Regards,
Sudeep
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Sudeep Holla April 17, 2019, 1:25 p.m. UTC | #7
On 17/04/2019 13:54, Lucas Stach wrote:
> Am Mittwoch, den 17.04.2019, 12:40 +0000 schrieb Leonard Crestez:
>> On 4/17/2019 3:13 PM, Lucas Stach wrote:
>>> Am Mittwoch, den 17.04.2019, 11:16 +0000 schrieb Aisheng Dong:
>>>>> From: Jacky Bai
>>>>> Sent: Wednesday, April 17, 2019 1:27 PM
>>>>>
>>>>> The i.MX8M family is a set of NXP product focus on delivering the latest and
>>>>> greatest video and audio experience combining state-of-the-art media-specific
>>>>> features with high-performance processing while optimized for lowest power
>>>>> consumption.
>>>>> i.MX8MQ, i.MX8MM, i.MX8MN, even the furture i.MX8MP are all belong to
>>>>> this family.
>>>>>
>>>>> The GPC module is used to manage the PU power domains' power on/off. For
>>>>> the whole i.MX8M family, different SoC has differnt power domain design. the
>>>>> power up sequence has significant difference.
>>>>> all the power sequence must be guaranteed by SW. Some domains' power up
>>>>> sequence need to access the SRC module or sub-system specific GPR.
>>>>> the SRC register & SS's register are not in in the GPC's memory range.
>>>>>
>>>>> it makes us hard to use the GPCv2 driver to cover all the different power up
>>>>> requirement. Each time, a new SoC is added, we must modify the GPCv2 driver
>>>>> to make it resuable for it. a lot of code need to be added in GPCv2 to support it.
>>>>> we need to access the SRC & SS' GPR, then the GPCv2 driver can NOT be
>>>>> self-contained. Accessing the non-driver specific module's register is a bad
>>>>> practice. Although, the GPC module provided the similar function for PU power
>>>>> domain, but it is not 100% compatible with GPCv2.
>>>>>
>>>>> The most important thing is that the GPC & SRC module is a security critical
>>>>> resource that security permission must be considered when building the
>>>>> security system. The GPC module is not only used by PU power domain power
>>>>> on/off. It is also used by the TF-A PSCI code to do the CPU core power
>>>>> management. the SRC module control the CPU CORE reset and the CPU reset
>>>>> vector address. if we give the non-secure world write permission to SRC.
>>>>> System can be easily induced to malicious code.
>>>>
>>>> Considering the security issue, it looks to me a right direction to move GPC
>>>> power handling into ATF.
>>>> It also helps build a more generic driver and ease other OS integration
>>>> needed by customers (e.g. QNX, Win10).
>>>>
>>>> Lucas,
>>>> How do you think of it?
>>>
>>> I don't yet buy the security argument. There are many more shared parts
>>> on the SoC, like the clock controller, that would need to be taken away
>>> from the non-secure world if one would want to run an untrusted OS
>>> kernel on a i.MX8M system.
>>>
>>> To properly implement security on any i.MX8M based system the firmware
>>> would need to grow something like a full ARM SCPI implementation, so
>>> all shared critical peripherals are solely under firmware control.
>>
>> It might be possible to rework this to use some form of SCMI-over-SMC 
>> instead of vendor-specific SMCCC SIP calls
>>
>> +SCMI maintainer
>>
>>> I agree that it might make sense to move some parts into the firmware
>>> and have much simpler OS level drivers, but I don't agree on the
>>> implementation direction taken here. Growing custom PSCI extension
>>> interfaces will only get us so far, without solving the system security
>>> issue in a holistic way. It is my strong believe that only a complete
>>> rearchitecture of the OS support on top of a ARM SCPI firmware
>>> interface can solve this properly.
>>
>> Hiding everything critical for security (especially CCM) behind a SCMI 
>> interface would be a large amount of work but introducing SCMI 
>> incrementally (starting with imx8mm power) would be useful by itself 
>> because it simplifies OS implementation.
> 
> I'm totally on-board with baby steps if it gets us into the right
> direction, so what you propose makes sense to me.
> 
> What I'm not okay with is tying the upstream kernel into an ad-hoc
> firmware interface in the name of system security,

Exactly, sorry I should have read through the thread. I just responded
in other email with similar thoughts/concerns.
Sudeep Holla April 17, 2019, 1:33 p.m. UTC | #8
On 17/04/2019 13:40, Leonard Crestez wrote:
> On 4/17/2019 3:13 PM, Lucas Stach wrote:

[...]

>>
>> I don't yet buy the security argument. There are many more shared parts
>> on the SoC, like the clock controller, that would need to be taken away
>> from the non-secure world if one would want to run an untrusted OS
>> kernel on a i.MX8M system.
>>
>> To properly implement security on any i.MX8M based system the firmware
>> would need to grow something like a full ARM SCPI implementation, so
>> all shared critical peripherals are solely under firmware control.
> 
> It might be possible to rework this to use some form of SCMI-over-SMC 
> instead of vendor-specific SMCCC SIP calls
> 

Sounds feasible and good if all the custom IDs/calls/..etc are well
hidden in the firmware(TF-A in this case) behind the existing
abstraction in Linux kernel.

> +SCMI maintainer
> 

Thanks for including.

>> I agree that it might make sense to move some parts into the firmware
>> and have much simpler OS level drivers, but I don't agree on the
>> implementation direction taken here. Growing custom PSCI extension
>> interfaces will only get us so far, without solving the system security
>> issue in a holistic way. It is my strong believe that only a complete
>> rearchitecture of the OS support on top of a ARM SCPI firmware
>> interface can solve this properly.

+1 again, just to re-iterate the emphasis on the above and the degree to
which I align with it.

> Hiding everything critical for security (especially CCM) behind a SCMI 
> interface would be a large amount of work but introducing SCMI 
> incrementally (starting with imx8mm power) would be useful by itself 
> because it simplifies OS implementation.
> 

Agreed, but not at the expense of introducing and maintaining *random*
*one-off* *custom* SMC extensions. Sorry, that's not open to debate.

> Many at NXP have attempted to evaluate SCMI and their conclusion has 
> always been that "many extensions are required".
> 

I would like to hear the evaluation. Don't assume that you need to
implement something similar to ARM SCMI reference design. All OSPM like
Linux kernel cares is conformance to the interface, what/how you
implement on the other side is left to you.
Sudeep Holla April 17, 2019, 1:36 p.m. UTC | #9
On Wed, Apr 17, 2019 at 2:23 PM Sudeep Holla <Sudeep.Holla@arm.com> wrote:
>

[...]

> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Sorry, the above disclaimer was completely unintentional. I must have
done something by mistake
for this to happen once in series of replies on this thread.
Leonard Crestez April 17, 2019, 4:21 p.m. UTC | #10
On 4/17/2019 4:33 PM, Sudeep Holla wrote:
>>> I don't yet buy the security argument. There are many more shared parts
>>> on the SoC, like the clock controller, that would need to be taken away
>>> from the non-secure world if one would want to run an untrusted OS
>>> kernel on a i.MX8M system.
>>>
>>> To properly implement security on any i.MX8M based system the firmware
>>> would need to grow something like a full ARM SCPI implementation, so
>>> all shared critical peripherals are solely under firmware control.
>>
>> It might be possible to rework this to use some form of SCMI-over-SMC
>> instead of vendor-specific SMCCC SIP calls
> 
> Sounds feasible and good if all the custom IDs/calls/..etc are well
> hidden in the firmware(TF-A in this case) behind the existing
> abstraction in Linux kernel.

>> Hiding everything critical for security (especially CCM) behind a SCMI
>> interface would be a large amount of work but introducing SCMI
>> incrementally (starting with imx8mm power) would be useful by itself
>> because it simplifies OS implementation.
> 
> Agreed, but not at the expense of introducing and maintaining *random*
> *one-off* *custom* SMC extensions. Sorry, that's not open to debate.
> 
>> Many at NXP have attempted to evaluate SCMI and their conclusion has
>> always been that "many extensions are required".
> 
> I would like to hear the evaluation. Don't assume that you need to
> implement something similar to ARM SCMI reference design. All OSPM like
> Linux kernel cares is conformance to the interface, what/how you
> implement on the other side is left to you.

Brief summary from some attempts at nudging NXP devs towards SCMI:


There is no SCMI-over-SMC support specified? This would make it possible 
to use SCMI as a purely software solution on platforms which did not 
take SCMI into account at hardware design time or which don't have a 
dedicated SCP inside the SOC. This applies to all imx.

Peng has been looking at some out-of-tree arm-smc-mailbox patches so 
it's not just NXP which would like the option of implementing SCMI 
vendor side in ATF. Like this: https://lwn.net/Articles/726861/

Blessing SCMI-over-SMC would allow stuff like intercepting a SCMI 
message in EL2; checking if the guest is allowed to use that resource 
and then either forward to EL3 or return an error.


SCMI clock protocol does not cover muxes? On imx the clk hierarchy is 
very intricate and it relies on many clk core features (including 
notifiers) and occasional assigned-clocks-parents properties to control 
muxes from linux. Moving all that to firmware would be a huge amount of 
effort.

If SCMI included a generic clk mux and allowed keeping the clk hierarchy 
inside linux then the effort required for hiding the CCM would still be 
large, but more approachable. It would not "simplify the rich OS" but it 
would still improve security.


Last point is that SCMI does not cover pinctrl? This is a large chunk of 
firmware functionality on some imx and security control over individual 
pins is desirable.

--
Regards,
Leonard
Sudeep Holla April 18, 2019, 2:43 p.m. UTC | #11
On Wed, Apr 17, 2019 at 04:21:55PM +0000, Leonard Crestez wrote:
> On 4/17/2019 4:33 PM, Sudeep Holla wrote:
> >>> I don't yet buy the security argument. There are many more shared parts
> >>> on the SoC, like the clock controller, that would need to be taken away
> >>> from the non-secure world if one would want to run an untrusted OS
> >>> kernel on a i.MX8M system.
> >>>
> >>> To properly implement security on any i.MX8M based system the firmware
> >>> would need to grow something like a full ARM SCPI implementation, so
> >>> all shared critical peripherals are solely under firmware control.
> >>
> >> It might be possible to rework this to use some form of SCMI-over-SMC
> >> instead of vendor-specific SMCCC SIP calls
> >
> > Sounds feasible and good if all the custom IDs/calls/..etc are well
> > hidden in the firmware(TF-A in this case) behind the existing
> > abstraction in Linux kernel.
>
> >> Hiding everything critical for security (especially CCM) behind a SCMI
> >> interface would be a large amount of work but introducing SCMI
> >> incrementally (starting with imx8mm power) would be useful by itself
> >> because it simplifies OS implementation.
> >
> > Agreed, but not at the expense of introducing and maintaining *random*
> > *one-off* *custom* SMC extensions. Sorry, that's not open to debate.
> >
> >> Many at NXP have attempted to evaluate SCMI and their conclusion has
> >> always been that "many extensions are required".
> >
> > I would like to hear the evaluation. Don't assume that you need to
> > implement something similar to ARM SCMI reference design. All OSPM like
> > Linux kernel cares is conformance to the interface, what/how you
> > implement on the other side is left to you.
>
> Brief summary from some attempts at nudging NXP devs towards SCMI:
>

Thanks for providing the evaluation details.

>
> There is no SCMI-over-SMC support specified? This would make it possible
> to use SCMI as a purely software solution on platforms which did not
> take SCMI into account at hardware design time or which don't have a
> dedicated SCP inside the SOC. This applies to all imx.
>

While I admit, the section of SCMI specification that touches transport
is quite lean, I would view it as done intentionally as the specification
is mainly concentrated on it's subject matter which is protocol and not
the transport itself. So did the evaluation attempted to consider/try
SMC as transport retaining protocol as is ?

I can't see any issues with that and hence I am asking it loud here.

> Peng has been looking at some out-of-tree arm-smc-mailbox patches so
> it's not just NXP which would like the option of implementing SCMI
> vendor side in ATF. Like this: https://lwn.net/Articles/726861/
>

OK, any inputs from that study ?

> Blessing SCMI-over-SMC would allow stuff like intercepting a SCMI
> message in EL2; checking if the guest is allowed to use that resource
> and then either forward to EL3 or return an error.
>

Why are you mixing virtualisation and EL2 here ? Yes we need them but
it should be optional and if a platform doesn't need it, it should be
possible to skip it.

>
> SCMI clock protocol does not cover muxes? On imx the clk hierarchy is
> very intricate and it relies on many clk core features (including
> notifiers) and occasional assigned-clocks-parents properties to control
> muxes from linux. Moving all that to firmware would be a huge amount of
> effort.
>

Yes it may be huge amount of work. But is it completely safe as claimed ?
Giving access to mux controls in OSPM is no so safe/secure in the modern
world. So you either make it safe with that extra huge effort needed or
just keep everything in OSPM. But IMO anything in between is not worth it.

> If SCMI included a generic clk mux and allowed keeping the clk hierarchy
> inside linux then the effort required for hiding the CCM would still be
> large, but more approachable. It would not "simplify the rich OS" but it
> would still improve security.
>

Why do you need to keep the clk hierarchy in Linux ?

>
> Last point is that SCMI does not cover pinctrl? This is a large chunk of
> firmware functionality on some imx and security control over individual
> pins is desirable.
>

Yes but is that something available at runtime ? Can't that be one-off
firmware setting. Will Linux need to configure it differently on each boot ?
Just trying to understand. You say security control here and is it really
safe to give OS access to control those ?

In short, if you had a full blown protocol like few other platforms, the
push back would have been minimal. Instead, i.MX chose to implement a
solution which doesn't have a design thought before and custom SMC APIs
added on fly whenever a new request is raised by OSPM to control things
that it thinks it should. I am sure, the very next platform will have it's
own set of requirements and custom SMC APIs/interface and no one has even
bothered about long term maintenance of these.

So assuming that trend, I would NACK this as it stands.

--
Regards,
Sudeep