diff mbox series

[1/1] arm64: dts: qcom: msm8994: Reserve gpio ranges

Message ID 20210405200259.23525-1-petr.vorel@gmail.com (mailing list archive)
State Superseded
Headers show
Series [1/1] arm64: dts: qcom: msm8994: Reserve gpio ranges | expand

Commit Message

Petr Vorel April 5, 2021, 8:02 p.m. UTC
Reserve pins 0-3 and 85-88 as these aren't meant to be accessible
from the application CPUs. Fix similar to 9134586715e3.

Fixes: 3edfb7bd76bd ("gpiolib: Show correct direction from the beginning")

Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
---
Hi,

tested on latest qcom/for-next.

Simple testing with /sys/class/gpio/export showed that 85-88.
3 disables UART. I expect 0-2 are also reserved as on other msm8998.

for i in $(seq 0 146); do echo $i > /sys/class/gpio/export; done

I expect it's just angler specific, thus I haven't added it to msm8994.dtsi
(otherwise Konrad would have fixed it).

Kind regards,
Petr

 arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Ricardo Ribalda Delgado April 5, 2021, 8:09 p.m. UTC | #1
Hi Petr

On Mon, Apr 5, 2021 at 10:03 PM Petr Vorel <petr.vorel@gmail.com> wrote:
>
> Reserve pins 0-3 and 85-88 as these aren't meant to be accessible
> from the application CPUs. Fix similar to 9134586715e3.
>
> Fixes: 3edfb7bd76bd ("gpiolib: Show correct direction from the beginning")

Why the Fixes?

Is the behaviour different if that patch is not applied?

>
> Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
> ---
> Hi,
>
> tested on latest qcom/for-next.
>
> Simple testing with /sys/class/gpio/export showed that 85-88.
> 3 disables UART. I expect 0-2 are also reserved as on other msm8998.
>
> for i in $(seq 0 146); do echo $i > /sys/class/gpio/export; done
>
> I expect it's just angler specific, thus I haven't added it to msm8994.dtsi
> (otherwise Konrad would have fixed it).
>
> Kind regards,
> Petr
>
>  arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts b/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts
> index baa55643b40f..0dc94101d5de 100644
> --- a/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts
> +++ b/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /* Copyright (c) 2015, Huawei Inc. All rights reserved.
>   * Copyright (c) 2016, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2021 Petr Vorel <petr.vorel@gmail.com>
>   */
>
>  /dts-v1/;
> @@ -32,3 +33,7 @@ serial@f991e000 {
>                 };
>         };
>  };
> +
> +&tlmm {
> +       gpio-reserved-ranges = <0 4>, <85 4>;
> +};
> --
> 2.30.2
>
Petr Vorel April 5, 2021, 8:15 p.m. UTC | #2
Hi Ricardo,

> Hi Petr

> On Mon, Apr 5, 2021 at 10:03 PM Petr Vorel <petr.vorel@gmail.com> wrote:

> > Reserve pins 0-3 and 85-88 as these aren't meant to be accessible
> > from the application CPUs. Fix similar to 9134586715e3.

> > Fixes: 3edfb7bd76bd ("gpiolib: Show correct direction from the beginning")

> Why the Fixes?

> Is the behaviour different if that patch is not applied?

Yes, the issue was not presented before 3edfb7bd76bd. It's similar to fix
9134586715e3 (msm8998-mtp had also problems after this commit). Feel free to
remove "Fixes: 3edfb7bd76bd" if you're convinced it's incorrect, but please at
least mention it in the commit message, similar to 3edfb7bd76bd).

Kind regards,
Petr
Bjorn Andersson April 5, 2021, 10:52 p.m. UTC | #3
On Mon 05 Apr 15:02 CDT 2021, Petr Vorel wrote:

> Reserve pins 0-3 and 85-88 as these aren't meant to be accessible
> from the application CPUs. Fix similar to 9134586715e3.
> 
> Fixes: 3edfb7bd76bd ("gpiolib: Show correct direction from the beginning")
> 
> Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
> ---
> Hi,
> 
> tested on latest qcom/for-next.
> 
> Simple testing with /sys/class/gpio/export showed that 85-88.
> 3 disables UART. I expect 0-2 are also reserved as on other msm8998.
> 

Are you saying that once you export these gpios the uart stops working?

We use gpio-reserved-ranges to denote GPIOs that are owned by TZ, so
touching their registers causes the device to reboot. And per the
gpiolib patch you reference, this would happen as we register the
gpiochip.

It sounds instead like what you want is to make sure that these pins are
considered busy, muxing in the uart (i.e define a state for uart).

Regards,
Bjorn

> for i in $(seq 0 146); do echo $i > /sys/class/gpio/export; done
> 
> I expect it's just angler specific, thus I haven't added it to msm8994.dtsi
> (otherwise Konrad would have fixed it).
> 
> Kind regards,
> Petr
> 
>  arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts b/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts
> index baa55643b40f..0dc94101d5de 100644
> --- a/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts
> +++ b/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /* Copyright (c) 2015, Huawei Inc. All rights reserved.
>   * Copyright (c) 2016, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2021 Petr Vorel <petr.vorel@gmail.com>
>   */
>  
>  /dts-v1/;
> @@ -32,3 +33,7 @@ serial@f991e000 {
>  		};
>  	};
>  };
> +
> +&tlmm {
> +	gpio-reserved-ranges = <0 4>, <85 4>;
> +};
> -- 
> 2.30.2
>
Petr Vorel April 6, 2021, 4:38 a.m. UTC | #4
Hi,

> Are you saying that once you export these gpios the uart stops working?
I'm sorry, I wasn't clear. Only exporting GPIO 3 disables UART. Exporting 85-88
causes reboot. And I have no info about 0-2 (it just most of
gpio-reserved-ranges diables 0-3). Therefore probably only <85 4> should go to
gpio-reserved-ranges. I'll send v2.

Kind regards,
Petr

> We use gpio-reserved-ranges to denote GPIOs that are owned by TZ, so
> touching their registers causes the device to reboot. And per the
> gpiolib patch you reference, this would happen as we register the
> gpiochip.

> It sounds instead like what you want is to make sure that these pins are
> considered busy, muxing in the uart (i.e define a state for uart).

> Regards,
> Bjorn
Linus Walleij April 8, 2021, 7:17 a.m. UTC | #5
On Tue, Apr 6, 2021 at 12:52 AM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> On Mon 05 Apr 15:02 CDT 2021, Petr Vorel wrote:

> > Simple testing with /sys/class/gpio/export showed that 85-88.
> > 3 disables UART. I expect 0-2 are also reserved as on other msm8998.
> >
>
> Are you saying that once you export these gpios the uart stops working?

That might be a Fixes: but certainly not a regression.

Using GPIO sysfs access is dangerous and if someone enables it into
their kernel (which requires setting CONFIG_EXPERT) they are
certainly aware that they are taking a risk. (Same goes for using
the character device from userspace.)

Yours,
Linus Walleij
Petr Vorel April 8, 2021, 7:02 p.m. UTC | #6
> On Tue, Apr 6, 2021 at 12:52 AM Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> > On Mon 05 Apr 15:02 CDT 2021, Petr Vorel wrote:

> > > Simple testing with /sys/class/gpio/export showed that 85-88.
> > > 3 disables UART. I expect 0-2 are also reserved as on other msm8998.


> > Are you saying that once you export these gpios the uart stops working?

> That might be a Fixes: but certainly not a regression.

> Using GPIO sysfs access is dangerous and if someone enables it into
> their kernel (which requires setting CONFIG_EXPERT) they are
> certainly aware that they are taking a risk. (Same goes for using
> the character device from userspace.)

Not sure if we understand each other. You might think I export GPIO from
userspace on mainline kernel or use CONFIG_EXPERT. None of these is true.

The real problem is that mainline kernel compiled with defconfig resets really
early, thus v2 of this patch [2] is needed.

I just exported GPIO on *downstream* kernel to observe the behaviour.
It was just a simple way for me to find GPIOs which cause reset.
(having docs would be great)

And there is similar reset much later but still before reaching initramfs
in loop_init since a99163e9e708 (during loop_add() on lo->lo_number: 3). And
if I disable CONFIG_BLK_DEV_LOOP, it resets right after reaching initramfs.
I guess that's another problem with DT, but haven't got much further.
But that's a separate problem, gpio-reserved-ranges = <85 4> on tlmm [2]
is still some improvement.

Kind regards,
Petr

[2] https://lore.kernel.org/linux-arm-msm/20210406202936.22500-1-petr.vorel

> Yours,
> Linus Walleij
Konrad Dybcio April 8, 2021, 8:05 p.m. UTC | #7
Hi,

to clear up some confusion:


On Qualcomm boards GPIOs that are used for "secure" (duh) peripherals,
like a fingerprint scanner, are not allowed to be controlled from Linux (the "non-secure world").
Trying to do so causes an immediate reboot due to "attempting to violate the security".


The GPIOs seem to all be iterated over on boot, except for the ones specified in "gpio-reserved-ranges".
As a result, if such "secure" GPIOs are not declared in the DT, the board essentially dies on TLMM (pinctrl) probe
(which happens veeeery early - so that all other peripherals can set the pins as they see fit)
and that's very unpleasant to debug. Without this patch, Petr's device will simply not boot.


So, why did it work before!?


Well, either the GPIOs weren't iterated over, or the TLMM (pinctrl) driver wasn't in place back then.


As for the initrd crash.. perhaps you have an Android initrd which dies as soon as it doesn't detect SELINUX and a couple of other options.. You might want to try postmarketOS's one, or any other Linux distro's armv7/aarch64 initrd. To replace it, simply use abootimg like so:


abootimg -u boot.img -r ramdisk.img



If it says something something "too small", add


-c "bootsize=30000000"


to make the boot.img exactly 30 million bytes (or change it as you see fit).


Konrad
Linus Walleij April 8, 2021, 9:35 p.m. UTC | #8
On Thu, Apr 8, 2021 at 9:02 PM Petr Vorel <petr.vorel@gmail.com> wrote:

> The real problem is that mainline kernel compiled with defconfig resets really
> early, thus v2 of this patch [2] is needed.

Ugh OK I get it. That's a firm regression.

Yours,
Linus Walleij
Linus Walleij April 8, 2021, 9:40 p.m. UTC | #9
On Thu, Apr 8, 2021 at 10:05 PM Konrad Dybcio <konradybcio@gmail.com> wrote:

> On Qualcomm boards GPIOs that are used for "secure" (duh) peripherals,
> like a fingerprint scanner, are not allowed to be controlled from Linux (the "non-secure world").
> Trying to do so causes an immediate reboot due to "attempting to violate the security".

OK I see. Yeah HW security is pretty neat, making it cause a reboot
seems like maybe not the best choice, but hey we know for sure what
is wrong when it happens so it gives a good feeling of having everything
fully inder control. Which is nice, despite the annoyance.

> The GPIOs seem to all be iterated over on boot, except for the ones specified in
> "gpio-reserved-ranges".
(...)
> So, why did it work before!?

We do things like read all direction registers to check if GPIO lines are set up
as input or output. If that causes reset then, well.

> As a result, if such "secure" GPIOs are not declared in the DT, the board essentially dies on TLMM (pinctrl) probe
> (which happens veeeery early - so that all other peripherals can set the pins as they see fit)
> and that's very unpleasant to debug. Without this patch, Petr's device will simply not boot.

In a way specifying it is a very correct thing to do.

When they were registered with the GPIO subsystem before they were,
well registered with the GPIO subsystem which means they are supposedly
available for general-purpose input-output. Which they were not.
They seem highly special-purpose to me. So
reserving them is the right thing to do.

Yours,
Linus Walleij
Petr Vorel April 9, 2021, 3:19 a.m. UTC | #10
Hi Konrad,
> Hi,

> to clear up some confusion:


> On Qualcomm boards GPIOs that are used for "secure" (duh) peripherals,
> like a fingerprint scanner, are not allowed to be controlled from Linux (the "non-secure world").
> Trying to do so causes an immediate reboot due to "attempting to violate the security".
Thanks for an explanation.

> The GPIOs seem to all be iterated over on boot, except for the ones specified in "gpio-reserved-ranges".
> As a result, if such "secure" GPIOs are not declared in the DT, the board essentially dies on TLMM (pinctrl) probe
> (which happens veeeery early - so that all other peripherals can set the pins as they see fit)
> and that's very unpleasant to debug. Without this patch, Petr's device will simply not boot.
Exactly.

> So, why did it work before!?


> Well, either the GPIOs weren't iterated over, or the TLMM (pinctrl) driver wasn't in place back then.
I suppose GPIOs not being iterated over is the case for first fix (i.e. fixing
3edfb7bd76bd "gpiolib: Show correct direction from the beginning").

> As for the initrd crash.. perhaps you have an Android initrd which dies as soon as it doesn't detect SELINUX and a couple of other options.. You might want to try postmarketOS's one, or any other Linux distro's armv7/aarch64 initrd. To replace it, simply use abootimg like so:
No, that's postmarketOS initrd which dies

before a99163e9e708d5d773b7de6da952fcddc341f977:
[   17.421112] ALSA device list:
[   17.426233]   No soundcard?[   17.436163] Freeing unused kernel memory: 5760K
[   17.436462] Run /init as init process
[   17.439499]   with arguments:
[   17.443330]     /init
[   17.446277]     PMOS_NO_OUTPUT_REDIRECT
[   17.448535]   with environment:
[   17.452172]     HOME=/
[   17.455303]     TERM=linux
### postmarketOS initramfs ###
Configuring kernel firmware image search path
/init: line 56: can't create /proc/sys/kernel/hotplug: nonexistent directory
Trying to mount subpartitions for 10 seconds...

after a99163e9e708d5d773b7de6da952fcddc341f977:
[   17.383267] calling  regulator_init_complete+0x0/0x4c @ 1
[   17.390129] initcall regulator_init_complete+0x0/0x4c returned 0 after 6 usecs
[   17.395682] calling  of_platform_sync_state_init+0x0/0x18 @ 1
[   17.402800] initcall of_platform_sync_state_init+0x0/0x18 returned 0 after 3 usecs
[   17.408616] calling  alsa_sound_last_init+0x0/0x88 @ 1
[   17.416077] ALSA device list:
[   17.421198]   No soundcardű[   17.431360] Freeing unused kernel memory: 5824K
[   17.431633] Run /init as init process
[   17.434700]   with arguments:
[   17.438535]     /init
[   17.441477]     PMOS_NO_OUTPUT_REDIRECT
[   17.443737]   with environment:
[   17.447381]     HOME=/
[   17.450496]     TERM=linux
D -     15494 - pm_driver_init, Delta

> abootimg -u boot.img -r ramdisk.img



> If it says something something "too small", add


> -c "bootsize=30000000"


> to make the boot.img exactly 30 million bytes (or change it as you see fit).
abootimg is really ok, this is not the issue.

> Konrad

Kind regards,
Petr
Bjorn Andersson April 9, 2021, 3:37 a.m. UTC | #11
On Thu 08 Apr 22:19 CDT 2021, Petr Vorel wrote:

> Hi Konrad,
> > Hi,
> 
> > to clear up some confusion:
> 
> 
> > On Qualcomm boards GPIOs that are used for "secure" (duh) peripherals,
> > like a fingerprint scanner, are not allowed to be controlled from Linux (the "non-secure world").
> > Trying to do so causes an immediate reboot due to "attempting to violate the security".
> Thanks for an explanation.
> 
> > The GPIOs seem to all be iterated over on boot, except for the ones specified in "gpio-reserved-ranges".
> > As a result, if such "secure" GPIOs are not declared in the DT, the board essentially dies on TLMM (pinctrl) probe
> > (which happens veeeery early - so that all other peripherals can set the pins as they see fit)
> > and that's very unpleasant to debug. Without this patch, Petr's device will simply not boot.
> Exactly.
> 
> > So, why did it work before!?
> 
> 
> > Well, either the GPIOs weren't iterated over, or the TLMM (pinctrl) driver wasn't in place back then.
> I suppose GPIOs not being iterated over is the case for first fix (i.e. fixing
> 3edfb7bd76bd "gpiolib: Show correct direction from the beginning").
> 

We had a long discussion about this in the past, and this resulted in
gpio-reserved-ranges and flagging off GPIOs that shouldn't be touched.

It seems we introduced the angler dts prior to said changes in the
gpiolib, so it's probably right to say that it's a regression. However,
the introduction of this was done 3 years ago and we're happy with it on
all other devices.

There's no harm in introducing this property prior to the introduction
of the related gpiolib patches, so if you really care about it being backported
I would suggest you say:

Fixes: feeaf56ac78d ("arm64: dts: msm8994 SoC and Huawei Angler (Nexus 6P) support")

But I presume based on the awesome work you guys are putting into the
8994 platform people shouldn't run "old" kernels anyways, so I think it
would be fine with us just ignoring the Fixes as well...

Regards,
Bjorn
Petr Vorel April 10, 2021, 5:52 a.m. UTC | #12
> On Thu 08 Apr 22:19 CDT 2021, Petr Vorel wrote:

> > Hi Konrad,
> > > Hi,

> > > to clear up some confusion:


> > > On Qualcomm boards GPIOs that are used for "secure" (duh) peripherals,
> > > like a fingerprint scanner, are not allowed to be controlled from Linux (the "non-secure world").
> > > Trying to do so causes an immediate reboot due to "attempting to violate the security".
> > Thanks for an explanation.

> > > The GPIOs seem to all be iterated over on boot, except for the ones specified in "gpio-reserved-ranges".
> > > As a result, if such "secure" GPIOs are not declared in the DT, the board essentially dies on TLMM (pinctrl) probe
> > > (which happens veeeery early - so that all other peripherals can set the pins as they see fit)
> > > and that's very unpleasant to debug. Without this patch, Petr's device will simply not boot.
> > Exactly.

> > > So, why did it work before!?


> > > Well, either the GPIOs weren't iterated over, or the TLMM (pinctrl) driver wasn't in place back then.
> > I suppose GPIOs not being iterated over is the case for first fix (i.e. fixing
> > 3edfb7bd76bd "gpiolib: Show correct direction from the beginning").


> We had a long discussion about this in the past, and this resulted in
> gpio-reserved-ranges and flagging off GPIOs that shouldn't be touched.

> It seems we introduced the angler dts prior to said changes in the
> gpiolib, so it's probably right to say that it's a regression. However,
> the introduction of this was done 3 years ago and we're happy with it on
> all other devices.

> There's no harm in introducing this property prior to the introduction
> of the related gpiolib patches, so if you really care about it being backported
> I would suggest you say:

> Fixes: feeaf56ac78d ("arm64: dts: msm8994 SoC and Huawei Angler (Nexus 6P) support")
You're right. I'd also note that the problem manifested only after 3edfb7bd76bd.
But yes, that's a minor detail. Documenting precisely is good, because problem
is not solved yet due that second reset.

Konrad, is there any public docs about GPIOs on this secure peripherals?
It it somehow related to Chain of Trust? [1].  I guess it's not, because once we
boot Linux all bootloader stuff is over.

I guess with gpio-reserved-ranges = <85 4> I solved problems with gpiolib,
which controls tlmm, right?

Is there any other GPIO related setup?

BTW downstream kernel (3.10.73) uses gpiochip:

ls -la /sys/class/gpio/
gpiochip0 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpiochip0
gpiochip1005 -> ../../devices/soc.0/qpnp-pin-14/gpio/gpiochip1005
gpiochip1006 -> ../../devices/soc.0/qpnp-pin-13/gpio/gpiochip1006
gpiochip1008 -> ../../devices/soc.0/qpnp-pin-4/gpio/gpiochip1008
gpiochip1011 -> ../../devices/soc.0/qpnp-pin-3/gpio/gpiochip1011
gpiochip685 -> ../../devices/soc.0/qcom,smp2pgpio-ssr-smp2p-1-out.16/gpio/gpiochip685
gpiochip717 -> ../../devices/soc.0/qcom,smp2pgpio-ssr-smp2p-1-in.15/gpio/gpiochip717
gpiochip749 -> ../../devices/soc.0/qcom,smp2pgpio-ssr-smp2p-2-out.14/gpio/gpiochip749
gpiochip781 -> ../../devices/soc.0/qcom,smp2pgpio-ssr-smp2p-2-in.13/gpio/gpiochip781
gpiochip813 -> ../../devices/soc.0/qcom,smp2pgpio-smp2p-2-out.11/gpio/gpiochip813
gpiochip845 -> ../../devices/soc.0/qcom,smp2pgpio-smp2p-2-in.9/gpio/gpiochip845
gpiochip877 -> ../../devices/soc.0/qcom,smp2pgpio-smp2p-1-out.7/gpio/gpiochip877
gpiochip909 -> ../../devices/soc.0/qcom,smp2pgpio-smp2p-1-in.5/gpio/gpiochip909
gpiochip941 -> ../../devices/soc.0/qcom,smp2pgpio-smp2p-7-out.3/gpio/gpiochip941
gpiochip973 -> ../../devices/soc.0/qcom,smp2pgpio-smp2p-7-in.1/gpio/gpiochip973

After /sys/class/gpio/export of 0-2 5-84 89-146 it looks like:

for i in $(seq 0 2) $(seq 5 84) $(seq 89 146); do echo $i > /sys/class/gpio/export; sleep 1; done
ls -la /sys/class/gpio/
gpio0 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio0
gpio1 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio1
gpio2 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio2
gpio5 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio5
gpio6 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio6
gpio7 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio7
gpio10 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio10
gpio11 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio11
gpio12 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio12
gpio13 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio13
gpio14 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio14
gpio15 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio15
gpio17 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio17
gpio18 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio18
gpio91 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio91
gpio92 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio92
gpio95 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio95
gpio97 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio97
gpio98 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio98
gpio99 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio99
gpio100 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio100
gpio101 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio101
gpio103 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio103
gpio104 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio104
gpio105 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio105
gpio106 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio106
gpio107 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio107
gpio109 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio109
gpio110 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio110
gpio111 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio111
gpio112 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio112
gpio114 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio114
gpio115 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio115
gpio116 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio116
gpio117 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio117
gpio118 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio118
gpio119 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio119
gpio120 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio120
gpio121 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio121
gpio122 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio122
gpio123 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio123
gpio124 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio124
gpio125 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio125
gpio126 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio126
gpio127 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio127
gpio128 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio128
gpio129 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio129
gpio130 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio130
gpio131 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio131
gpio132 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio132
gpio133 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio133
gpio134 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio134
gpio135 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio135
gpio136 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio136
gpio137 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio137
gpio138 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio138
gpio139 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio139
gpio140 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio140
gpio141 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio141
gpio142 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio142
gpio143 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio143
gpio144 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio144
gpio145 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpio145
gpiochip0 -> ../../devices/soc.0/fd510000.pinctrl/gpio/gpiochip0
gpiochip685 -> ../../devices/soc.0/qcom,smp2pgpio-ssr-smp2p-1-out.16/gpio/gpiochip685
gpiochip717 -> ../../devices/soc.0/qcom,smp2pgpio-ssr-smp2p-1-in.15/gpio/gpiochip717
gpiochip749 -> ../../devices/soc.0/qcom,smp2pgpio-ssr-smp2p-2-out.14/gpio/gpiochip749
gpiochip781 -> ../../devices/soc.0/qcom,smp2pgpio-ssr-smp2p-2-in.13/gpio/gpiochip781
gpiochip813 -> ../../devices/soc.0/qcom,smp2pgpio-smp2p-2-out.11/gpio/gpiochip813
gpiochip845 -> ../../devices/soc.0/qcom,smp2pgpio-smp2p-2-in.9/gpio/gpiochip845
gpiochip877 -> ../../devices/soc.0/qcom,smp2pgpio-smp2p-1-out.7/gpio/gpiochip877
gpiochip909 -> ../../devices/soc.0/qcom,smp2pgpio-smp2p-1-in.5/gpio/gpiochip909
gpiochip941 -> ../../devices/soc.0/qcom,smp2pgpio-smp2p-7-out.3/gpio/gpiochip941
gpiochip973 -> ../../devices/soc.0/qcom,smp2pgpio-smp2p-7-in.1/gpio/gpiochip973
gpiochip1005 -> ../../devices/soc.0/qpnp-pin-14/gpio/gpiochip1005
gpiochip1006 -> ../../devices/soc.0/qpnp-pin-13/gpio/gpiochip1006
gpiochip1008 -> ../../devices/soc.0/qpnp-pin-4/gpio/gpiochip1008
gpiochip1011 -> ../../devices/soc.0/qpnp-pin-3/gpio/gpiochip1011

Kind regards,
Petr

[1] https://lineageos.org/engineering/Qualcomm-Firmware/

> But I presume based on the awesome work you guys are putting into the
> 8994 platform people shouldn't run "old" kernels anyways, so I think it
> would be fine with us just ignoring the Fixes as well...

> Regards,
> Bjorn
Konrad Dybcio April 10, 2021, 9:16 a.m. UTC | #13
> Konrad, is there any public docs about GPIOs on this secure peripherals?
> It it somehow related to Chain of Trust? [1].  I guess it's not, because once we
> boot Linux all bootloader stuff is over.

No, Qualcomm pretty much does security through obscurity. It's *probably* not even that very secure considering how big in size their TZ+HYP stack is - number of bugs rises exponentially with code size. But not many people tried breaking into it considering the complexity and QCOM's legal team size.

There is no public documentation on that, and even if there were - you are not allowed to flash the "secure" partitions on *your device that you unlocked the bootloader of by choice* (which is absurd).

Also, while "all bootloader stuff is over", the platform is still under control of the proprietary hypervisor and the "Trust Zone". For example if you try to write to some IOMMU registers on certain platforms, the hypervisor will treat that as a security violation and shut down the entire device. 

This is essentially the same as your issue. You're trying to poke a thing that Qualcomm *really* doesn't want you to (the fingerprint SPI pins) and since *they* are in control, they say "nonono" and your device dies. All you can do is comply with that (or find a way to replace the blobs or politely ask Google to release a set of unsecure binaries for your Nexus - which they won't do).

Konrad
Petr Vorel April 10, 2021, 5:20 p.m. UTC | #14
> > Konrad, is there any public docs about GPIOs on this secure peripherals?
> > It it somehow related to Chain of Trust? [1].  I guess it's not, because once we
> > boot Linux all bootloader stuff is over.

> No, Qualcomm pretty much does security through obscurity. It's *probably* not even that very secure considering how big in size their TZ+HYP stack is - number of bugs rises exponentially with code size. But not many people tried breaking into it considering the complexity and QCOM's legal team size.

> There is no public documentation on that, and even if there were - you are not allowed to flash the "secure" partitions on *your device that you unlocked the bootloader of by choice* (which is absurd).

> Also, while "all bootloader stuff is over", the platform is still under control of the proprietary hypervisor and the "Trust Zone". For example if you try to write to some IOMMU registers on certain platforms, the hypervisor will treat that as a security violation and shut down the entire device. 

> This is essentially the same as your issue. You're trying to poke a thing that Qualcomm *really* doesn't want you to (the fingerprint SPI pins) and since *they* are in control, they say "nonono" and your device dies. All you can do is comply with that (or find a way to replace the blobs or politely ask Google to release a set of unsecure binaries for your Nexus - which they won't do).

Again, thanks a lot for info. I looked into downstream sources to see that
really pins 85-88 (which I've sent a patch to add into gpio-reserved-ranges) are
used for fingerprint. I also wonder if downstream commit d45c35c7b586 ("angler:
fingerprint: remove all the code about spi") [1] confirms that also downstream
kernel would reset or it's a security (it would not reset, thus they removed
the access). It's probably aosp issue tracker [2], but "Access denied" for me.

I also did some testing and this is maximum range which can be disabled:
gpio-reserved-ranges = <0 4>, <6 139> and it does not help to solve second
reset (in loop_init() or later when starting initramfs).
Removing access to GPIO 4 or 5 causes reset right immediately (no message from
kernel).

I still don't understand what changed in a99163e9e708 ("Merge tag
'devicetree-for-5.12' of git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux")
I checked both 882d6edfc45c cb8be8b4b27f, which it merges and they're ok.

Kind regards,
Petr

[1] https://android.googlesource.com/kernel/msm/+/d45c35c7b586711e757eb7e3239db5c88d114e0e
[2] https://issuetracker.google.com/issues/23756466

> Konrad
Petr Vorel April 12, 2021, 5:48 p.m. UTC | #15
Hi,

[ Cc KarimAllah Ahmed, as the author of 86588296acbf; Rob merged it ]

> > > Konrad, is there any public docs about GPIOs on this secure peripherals?
> > > It it somehow related to Chain of Trust? [1].  I guess it's not, because once we
> > > boot Linux all bootloader stuff is over.

> > No, Qualcomm pretty much does security through obscurity. It's *probably* not even that very secure considering how big in size their TZ+HYP stack is - number of bugs rises exponentially with code size. But not many people tried breaking into it considering the complexity and QCOM's legal team size.

> > There is no public documentation on that, and even if there were - you are not allowed to flash the "secure" partitions on *your device that you unlocked the bootloader of by choice* (which is absurd).

> > Also, while "all bootloader stuff is over", the platform is still under control of the proprietary hypervisor and the "Trust Zone". For example if you try to write to some IOMMU registers on certain platforms, the hypervisor will treat that as a security violation and shut down the entire device. 

> > This is essentially the same as your issue. You're trying to poke a thing that Qualcomm *really* doesn't want you to (the fingerprint SPI pins) and since *they* are in control, they say "nonono" and your device dies. All you can do is comply with that (or find a way to replace the blobs or politely ask Google to release a set of unsecure binaries for your Nexus - which they won't do).

> Again, thanks a lot for info. I looked into downstream sources to see that
> really pins 85-88 (which I've sent a patch to add into gpio-reserved-ranges) are
> used for fingerprint. I also wonder if downstream commit d45c35c7b586 ("angler:
> fingerprint: remove all the code about spi") [1] confirms that also downstream
> kernel would reset or it's a security (it would not reset, thus they removed
> the access). It's probably aosp issue tracker [2], but "Access denied" for me.

> I also did some testing and this is maximum range which can be disabled:
> gpio-reserved-ranges = <0 4>, <6 139> and it does not help to solve second
> reset (in loop_init() or later when starting initramfs).
> Removing access to GPIO 4 or 5 causes reset right immediately (no message from
> kernel).

> I still don't understand what changed in a99163e9e708 ("Merge tag
> 'devicetree-for-5.12' of git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux")
> I checked both 882d6edfc45c cb8be8b4b27f, which it merges and they're ok.

I've found the other problem preventing booting. Appart from v2 [3] is also needed
to revert 86588296acbf ("fdt: Properly handle "no-map" field in the memory region").

I'm pretty sure, that this commit is needed, but what should I change in
arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts in order to get my angler
booting again? With it it reset again after loop_init:

[   12.077756] initcall devcoredump_init+0x0/0x30 returned 0 after 22 usecs
[   12.082425] calling  loop_init+0x0/0x158 @ 1

And with disabled CONFIG_BLK_DEV_LOOP it get's reset before reaching initramfs:
~/tmp/hackweek/loop_init.debug.a99163e9e708.disabled-CONFIG_BLK_DEV_LOOP/2021-04-07_21-01-34.log
[   17.383267] calling  regulator_init_complete+0x0/0x4c @ 1
[   17.390129] initcall regulator_init_complete+0x0/0x4c returned 0 after 6 usecs
[   17.395682] calling  of_platform_sync_state_init+0x0/0x18 @ 1
[   17.402800] initcall of_platform_sync_state_init+0x0/0x18 returned 0 after 3 usecs
[   17.408616] calling  alsa_sound_last_init+0x0/0x88 @ 1
[   17.416077] ALSA device list:
[   17.421198]   No soundcardű[   17.431360] Freeing unused kernel memory: 5824K
[   17.431633] Run /init as init process
[   17.434700]   with arguments:
[   17.438535]     /init
[   17.441477]     PMOS_NO_OUTPUT_REDIRECT
[   17.443737]   with environment:
[   17.447381]     HOME=/
[   17.450496]     TERM=linux
D -     15494 - pm_driver_init, Delta

Kind regards,
Petr

> Kind regards,
> Petr

> [1] https://android.googlesource.com/kernel/msm/+/d45c35c7b586711e757eb7e3239db5c88d114e0e
> [2] https://issuetracker.google.com/issues/23756466
[3] https://lore.kernel.org/linux-arm-msm/20210406202936.22500-1-petr.vorel@gmail.com/T/#u

> > Konrad
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts b/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts
index baa55643b40f..0dc94101d5de 100644
--- a/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts
+++ b/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts
@@ -1,6 +1,7 @@ 
 // SPDX-License-Identifier: GPL-2.0-only
 /* Copyright (c) 2015, Huawei Inc. All rights reserved.
  * Copyright (c) 2016, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2021 Petr Vorel <petr.vorel@gmail.com>
  */
 
 /dts-v1/;
@@ -32,3 +33,7 @@  serial@f991e000 {
 		};
 	};
 };
+
+&tlmm {
+	gpio-reserved-ranges = <0 4>, <85 4>;
+};