linux-kernel - Re: [PATCH 1/2] arm64: dts: qcom: sm8250: Disable Adreno and Venus by default

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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 11 Jun 2021 11:54:14 -0700
From:   Rob Clark <robdclark@...il.com>
To:     Konrad Dybcio <konrad.dybcio@...ainline.org>
Cc:     ~postmarketos/upstreaming@...ts.sr.ht,
        Martin Botka <martin.botka@...ainline.org>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...ainline.org>,
        Marijn Suijten <marijn.suijten@...ainline.org>,
        jamipkettunen@...ainline.org, Andy Gross <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] arm64: dts: qcom: sm8250: Disable Adreno and Venus by default

On Fri, Jun 11, 2021 at 4:35 AM Konrad Dybcio
<konrad.dybcio@...ainline.org> wrote:
>
> Components that rely on proprietary (not to mention signed!) firmware should
> not be enabled by default, as lack of the aforementioned firmware could cause
> various issues, from random errors to straight-up failing to boot.

I admit I probably don't test much without fw, but I think maybe
*that* is the issue to solve instead.

Taking a guess, I think the situation is that you have *some* of the
fw (ie. the unsigned parts coming from linux-firmware) but not
specifically the zap fw.  So adreno_zap_shader_load() returns -ENODEV,
which is interpreted to mean "this is a platform that doesn't need zap
to take GPU out of secure mode", and then things go boom when we try
to write RBBM_SECVID_TRUST_CNTL ?

Which I think is down to missing the "firmware-name" property in the
device specific dts

BR,
-R

> Signed-off-by: Konrad Dybcio <konrad.dybcio@...ainline.org>
> ---
>  arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 10 ++++++++++
>  arch/arm64/boot/dts/qcom/sm8250-hdk.dts  | 12 ++++++++++++
>  arch/arm64/boot/dts/qcom/sm8250-mtp.dts  | 10 ++++++++++
>  arch/arm64/boot/dts/qcom/sm8250.dtsi     |  6 ++++++
>  4 files changed, 38 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
> index 5f41de20aa22..a5b742325261 100644
> --- a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
> +++ b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
> @@ -552,7 +552,13 @@ &dsi0_phy {
>         vdds-supply = <&vreg_l5a_0p88>;
>  };
>
> +&gmu {
> +       status = "okay";
> +};
> +
>  &gpu {
> +       status = "okay";
> +
>         zap-shader {
>                 memory-region = <&gpu_mem>;
>                 firmware-name = "qcom/sm8250/a650_zap.mbn";
> @@ -1352,6 +1358,10 @@ &vamacro {
>         qcom,dmic-sample-rate = <600000>;
>  };
>
> +&venus {
> +       status = "okay";
> +};
> +
>  /* PINCTRL - additions to nodes defined in sm8250.dtsi */
>  &qup_spi0_cs_gpio {
>         drive-strength = <6>;
> diff --git a/arch/arm64/boot/dts/qcom/sm8250-hdk.dts b/arch/arm64/boot/dts/qcom/sm8250-hdk.dts
> index c3a2c5aa6fe9..397359ee2f85 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250-hdk.dts
> +++ b/arch/arm64/boot/dts/qcom/sm8250-hdk.dts
> @@ -365,6 +365,14 @@ vreg_l7f_1p8: ldo7 {
>         };
>  };
>
> +&gmu {
> +       status = "okay";
> +};
> +
> +&gpu {
> +       status = "okay";
> +};
> +
>  &qupv3_id_1 {
>         status = "okay";
>  };
> @@ -452,3 +460,7 @@ &usb_1_dwc3 {
>  &usb_2_dwc3 {
>         dr_mode = "host";
>  };
> +
> +&venus {
> +       status = "okay";
> +};
> diff --git a/arch/arm64/boot/dts/qcom/sm8250-mtp.dts b/arch/arm64/boot/dts/qcom/sm8250-mtp.dts
> index cfc4d1febe0f..062b944be91d 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250-mtp.dts
> +++ b/arch/arm64/boot/dts/qcom/sm8250-mtp.dts
> @@ -465,7 +465,13 @@ &cdsp {
>         firmware-name = "qcom/sm8250/cdsp.mbn";
>  };
>
> +&gmu {
> +       status = "okay";
> +};
> +
>  &gpu {
> +       status = "okay";
> +
>         zap-shader {
>                 memory-region = <&gpu_mem>;
>                 firmware-name = "qcom/sm8250/a650_zap.mbn";
> @@ -691,3 +697,7 @@ &usb_2_qmpphy {
>         vdda-phy-supply = <&vreg_l9a_1p2>;
>         vdda-pll-supply = <&vreg_l18a_0p9>;
>  };
> +
> +&venus {
> +       status = "okay";
> +};
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index 4c0de12aaba6..fc1049c2bb11 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -1746,6 +1746,8 @@ gpu: gpu@...0000 {
>
>                         qcom,gmu = <&gmu>;
>
> +                       status = "disabled";
> +
>                         zap-shader {
>                                 memory-region = <&gpu_mem>;
>                         };
> @@ -1819,6 +1821,8 @@ gmu: gmu@...a000 {
>
>                         operating-points-v2 = <&gmu_opp_table>;
>
> +                       status = "disabled";
> +
>                         gmu_opp_table: opp-table {
>                                 compatible = "operating-points-v2";
>
> @@ -2323,6 +2327,8 @@ venus: video-codec@...0000 {
>                                  <&videocc VIDEO_CC_MVS0C_CLK_ARES>;
>                         reset-names = "bus", "core";
>
> +                       status = "disabled";
> +
>                         video-decoder {
>                                 compatible = "venus-decoder";
>                         };
> --
> 2.32.0
>