| Message ID | 20221213140724.8612-2-quic_sibis@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix XPU violation during modem metadata authentication | expand |
On 13/12/2022 15:07, Sibi Sankar wrote: > Introduce a new carveout for modem metadata. This will serve as a > replacement for the memory region used by MSA to authenticate modem > ELF headers. > > Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com> Thank you for your patch. There is something to discuss/improve. > > aliases { > @@ -865,7 +870,7 @@ hp_i2c: &i2c9 { > clock-names = "iface", "bus", "nav", "snoc_axi", "mnoc_axi", "xo"; > > iommus = <&apps_smmu 0x461 0x0>, <&apps_smmu 0x444 0x3>; > - memory-region = <&mba_mem &mpss_mem>; > + memory-region = <&mba_mem>, <&mpss_mem>, <&mdata_mem>; > > /* This gets overridden for SKUs with LTE support. */ > firmware-name = "qcom/sc7180-trogdor/modem-nolte/mba.mbn", > diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi > index bf522a64b172..bda0495aa0b5 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi > @@ -17,6 +17,11 @@ > reg = <0x0 0x9c700000 0x0 0x200000>; > no-map; > }; > + > + mdata_mem: memory@9d100000 { > + reg = <0x0 0x9d100000 0x0 0x4000>; > + no-map; > + }; > }; > }; > > @@ -32,7 +37,7 @@ > > iommus = <&apps_smmu 0x124 0x0>, <&apps_smmu 0x488 0x7>; > interconnects = <&mc_virt MASTER_LLCC 0 &mc_virt SLAVE_EBI1 0>; > - memory-region = <&mba_mem>, <&mpss_mem>; > + memory-region = <&mba_mem>, <&mpss_mem>, <&mdata_mem>; Only two memory regions are allowed by bindings... unless you fix it in further patchset. If so, please re-order to have the bindings first. It helps reviewers not to have such questions. :) Best regards, Krzysztof
Hey Krzysztof, Thanks for taking time to review the series. On 12/14/22 01:10, Krzysztof Kozlowski wrote: > On 13/12/2022 15:07, Sibi Sankar wrote: >> Introduce a new carveout for modem metadata. This will serve as a >> replacement for the memory region used by MSA to authenticate modem >> ELF headers. >> >> Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com> > > Thank you for your patch. There is something to discuss/improve. > >> >> aliases { >> @@ -865,7 +870,7 @@ hp_i2c: &i2c9 { >> clock-names = "iface", "bus", "nav", "snoc_axi", "mnoc_axi", "xo"; >> >> iommus = <&apps_smmu 0x461 0x0>, <&apps_smmu 0x444 0x3>; >> - memory-region = <&mba_mem &mpss_mem>; >> + memory-region = <&mba_mem>, <&mpss_mem>, <&mdata_mem>; >> >> /* This gets overridden for SKUs with LTE support. */ >> firmware-name = "qcom/sc7180-trogdor/modem-nolte/mba.mbn", >> diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi >> index bf522a64b172..bda0495aa0b5 100644 >> --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi >> @@ -17,6 +17,11 @@ >> reg = <0x0 0x9c700000 0x0 0x200000>; >> no-map; >> }; >> + >> + mdata_mem: memory@9d100000 { >> + reg = <0x0 0x9d100000 0x0 0x4000>; >> + no-map; >> + }; >> }; >> }; >> >> @@ -32,7 +37,7 @@ >> >> iommus = <&apps_smmu 0x124 0x0>, <&apps_smmu 0x488 0x7>; >> interconnects = <&mc_virt MASTER_LLCC 0 &mc_virt SLAVE_EBI1 0>; >> - memory-region = <&mba_mem>, <&mpss_mem>; >> + memory-region = <&mba_mem>, <&mpss_mem>, <&mdata_mem>; > > Only two memory regions are allowed by bindings... unless you fix it in > further patchset. If so, please re-order to have the bindings first. It > helps reviewers not to have such questions. :) I felt that Rob's dt_bindings check bot might report an error if the dt changes weren't placed before the bindings changes. But since you asked for the logical order I guess the bindings check are done only after the entire series is applied. I'll change the order in the next re-spin. - Sibi > > > Best regards, > Krzysztof >
On 14/12/2022 07:49, Sibi Sankar wrote: > Hey Krzysztof, > > Thanks for taking time to review the series. > > On 12/14/22 01:10, Krzysztof Kozlowski wrote: >> On 13/12/2022 15:07, Sibi Sankar wrote: >>> Introduce a new carveout for modem metadata. This will serve as a >>> replacement for the memory region used by MSA to authenticate modem >>> ELF headers. >>> >>> Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com> >> >> Thank you for your patch. There is something to discuss/improve. >> >>> >>> aliases { >>> @@ -865,7 +870,7 @@ hp_i2c: &i2c9 { >>> clock-names = "iface", "bus", "nav", "snoc_axi", "mnoc_axi", "xo"; >>> >>> iommus = <&apps_smmu 0x461 0x0>, <&apps_smmu 0x444 0x3>; >>> - memory-region = <&mba_mem &mpss_mem>; >>> + memory-region = <&mba_mem>, <&mpss_mem>, <&mdata_mem>; >>> >>> /* This gets overridden for SKUs with LTE support. */ >>> firmware-name = "qcom/sc7180-trogdor/modem-nolte/mba.mbn", >>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi >>> index bf522a64b172..bda0495aa0b5 100644 >>> --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi >>> +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi >>> @@ -17,6 +17,11 @@ >>> reg = <0x0 0x9c700000 0x0 0x200000>; >>> no-map; >>> }; >>> + >>> + mdata_mem: memory@9d100000 { >>> + reg = <0x0 0x9d100000 0x0 0x4000>; >>> + no-map; >>> + }; >>> }; >>> }; >>> >>> @@ -32,7 +37,7 @@ >>> >>> iommus = <&apps_smmu 0x124 0x0>, <&apps_smmu 0x488 0x7>; >>> interconnects = <&mc_virt MASTER_LLCC 0 &mc_virt SLAVE_EBI1 0>; >>> - memory-region = <&mba_mem>, <&mpss_mem>; >>> + memory-region = <&mba_mem>, <&mpss_mem>, <&mdata_mem>; >> >> Only two memory regions are allowed by bindings... unless you fix it in >> further patchset. If so, please re-order to have the bindings first. It >> helps reviewers not to have such questions. :) > > I felt that Rob's dt_bindings check bot might report an error > if the dt changes weren't placed before the bindings changes. > But since you asked for the logical order I guess the bindings > check are done only after the entire series is applied. I'll > change the order in the next re-spin. AFAIR, Rob's bot ignores DTS patches anyway. Best regards, Krzysztof
On 13/12/2022 15:07, Sibi Sankar wrote: > Introduce a new carveout for modem metadata. This will serve as a > replacement for the memory region used by MSA to authenticate modem > ELF headers. > > Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com> > --- > arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi | 6 ++++++ > arch/arm64/boot/dts/qcom/msm8996.dtsi | 9 +++++++++ > arch/arm64/boot/dts/qcom/msm8998.dtsi | 9 +++++++++ > arch/arm64/boot/dts/qcom/sc7180-idp.dts | 7 ++++++- > arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 7 ++++++- > arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi | 7 ++++++- > arch/arm64/boot/dts/qcom/sdm845.dtsi | 9 +++++++++ > 7 files changed, 51 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi b/arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi > index 5b47b8de69da..4242f8587c19 100644 > --- a/arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi > +++ b/arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi > @@ -127,6 +127,12 @@ > reg = <0x0 0xf6f00000 0x0 0x100000>; > no-map; > }; > + > + /delete-node/ memory@91700000; > + mdata_mem: memory@f7100000 { > + reg = <0x0 0xf7100000 0x0 0x4000>; > + no-map; > + }; > }; > > vph_pwr: vph-pwr-regulator { > diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi > index cc65f52bb80f..3f5fb08e2341 100644 > --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi > +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi > @@ -462,6 +462,11 @@ > reg = <0x0 0x91500000 0x0 0x200000>; > no-map; > }; > + > + mdata_mem: memory@91700000 { > + reg = <0x0 0x91700000 0x0 0x4000>; > + no-map; > + }; > }; > > rpm-glink { > @@ -2458,6 +2463,10 @@ > memory-region = <&mpss_mem>; > }; > > + metadata { Does it pass dtbs_check? Best regards, Krzysztof
On 12/14/22 16:57, Krzysztof Kozlowski wrote: > On 13/12/2022 15:07, Sibi Sankar wrote: >> Introduce a new carveout for modem metadata. This will serve as a >> replacement for the memory region used by MSA to authenticate modem >> ELF headers. >> >> Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com> >> --- >> arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi | 6 ++++++ >> arch/arm64/boot/dts/qcom/msm8996.dtsi | 9 +++++++++ >> arch/arm64/boot/dts/qcom/msm8998.dtsi | 9 +++++++++ >> arch/arm64/boot/dts/qcom/sc7180-idp.dts | 7 ++++++- >> arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 7 ++++++- >> arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi | 7 ++++++- >> arch/arm64/boot/dts/qcom/sdm845.dtsi | 9 +++++++++ >> 7 files changed, 51 insertions(+), 3 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi b/arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi >> index 5b47b8de69da..4242f8587c19 100644 >> --- a/arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi >> +++ b/arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi >> @@ -127,6 +127,12 @@ >> reg = <0x0 0xf6f00000 0x0 0x100000>; >> no-map; >> }; >> + >> + /delete-node/ memory@91700000; >> + mdata_mem: memory@f7100000 { >> + reg = <0x0 0xf7100000 0x0 0x4000>; >> + no-map; >> + }; >> }; >> >> vph_pwr: vph-pwr-regulator { >> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi >> index cc65f52bb80f..3f5fb08e2341 100644 >> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi >> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi >> @@ -462,6 +462,11 @@ >> reg = <0x0 0x91500000 0x0 0x200000>; >> no-map; >> }; >> + >> + mdata_mem: memory@91700000 { >> + reg = <0x0 0x91700000 0x0 0x4000>; >> + no-map; >> + }; >> }; >> >> rpm-glink { >> @@ -2458,6 +2463,10 @@ >> memory-region = <&mpss_mem>; >> }; >> >> + metadata { > > Does it pass dtbs_check? ^^ portion of the bindings aren't in yaml yet please see patch 3. > > Best regards, > Krzysztof >
On 14/12/2022 12:44, Sibi Sankar wrote: > > > On 12/14/22 16:57, Krzysztof Kozlowski wrote: >> On 13/12/2022 15:07, Sibi Sankar wrote: >>> Introduce a new carveout for modem metadata. This will serve as a >>> replacement for the memory region used by MSA to authenticate modem >>> ELF headers. >>> >>> Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com> >>> --- >>> arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi | 6 ++++++ >>> arch/arm64/boot/dts/qcom/msm8996.dtsi | 9 +++++++++ >>> arch/arm64/boot/dts/qcom/msm8998.dtsi | 9 +++++++++ >>> arch/arm64/boot/dts/qcom/sc7180-idp.dts | 7 ++++++- >>> arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 7 ++++++- >>> arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi | 7 ++++++- >>> arch/arm64/boot/dts/qcom/sdm845.dtsi | 9 +++++++++ >>> 7 files changed, 51 insertions(+), 3 deletions(-) >>> >>> diff --git a/arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi b/arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi >>> index 5b47b8de69da..4242f8587c19 100644 >>> --- a/arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi >>> +++ b/arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi >>> @@ -127,6 +127,12 @@ >>> reg = <0x0 0xf6f00000 0x0 0x100000>; >>> no-map; >>> }; >>> + >>> + /delete-node/ memory@91700000; >>> + mdata_mem: memory@f7100000 { >>> + reg = <0x0 0xf7100000 0x0 0x4000>; >>> + no-map; >>> + }; >>> }; >>> >>> vph_pwr: vph-pwr-regulator { >>> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi >>> index cc65f52bb80f..3f5fb08e2341 100644 >>> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi >>> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi >>> @@ -462,6 +462,11 @@ >>> reg = <0x0 0x91500000 0x0 0x200000>; >>> no-map; >>> }; >>> + >>> + mdata_mem: memory@91700000 { >>> + reg = <0x0 0x91700000 0x0 0x4000>; >>> + no-map; >>> + }; >>> }; >>> >>> rpm-glink { >>> @@ -2458,6 +2463,10 @@ >>> memory-region = <&mpss_mem>; >>> }; >>> >>> + metadata { >> >> Does it pass dtbs_check? > > ^^ portion of the bindings aren't in yaml yet please see patch 3. Ah, you touch here multiple files. Please split per SoC. Best regards, Krzysztof
diff --git a/arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi b/arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi index 5b47b8de69da..4242f8587c19 100644 --- a/arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi @@ -127,6 +127,12 @@ reg = <0x0 0xf6f00000 0x0 0x100000>; no-map; }; + + /delete-node/ memory@91700000; + mdata_mem: memory@f7100000 { + reg = <0x0 0xf7100000 0x0 0x4000>; + no-map; + }; }; vph_pwr: vph-pwr-regulator { diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi index cc65f52bb80f..3f5fb08e2341 100644 --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi @@ -462,6 +462,11 @@ reg = <0x0 0x91500000 0x0 0x200000>; no-map; }; + + mdata_mem: memory@91700000 { + reg = <0x0 0x91700000 0x0 0x4000>; + no-map; + }; }; rpm-glink { @@ -2458,6 +2463,10 @@ memory-region = <&mpss_mem>; }; + metadata { + memory-region = <&mdata_mem>; + }; + smd-edge { interrupts = <GIC_SPI 449 IRQ_TYPE_EDGE_RISING>; diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi index 539382dab0ad..02e81fd5702d 100644 --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi @@ -59,6 +59,11 @@ qcom,vmid = <15>; }; + mdata_mem: memory@89100000 { + reg = <0x0 0x89100000 0x0 0x4000>; + no-map; + }; + spss_mem: memory@8ab00000 { reg = <0x0 0x8ab00000 0x0 0x700000>; no-map; @@ -1357,6 +1362,10 @@ memory-region = <&mpss_mem>; }; + metadata { + memory-region = <&mdata_mem>; + }; + glink-edge { interrupts = <GIC_SPI 452 IRQ_TYPE_EDGE_RISING>; label = "modem"; diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts b/arch/arm64/boot/dts/qcom/sc7180-idp.dts index b27b5f0e2b6b..ff0ef8bcba2f 100644 --- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts +++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts @@ -80,6 +80,11 @@ reg = <0x0 0x94400000 0x0 0x200000>; no-map; }; + + mdata_mem: memory@94e00000 { + reg = <0x0 0x94e00000 0x0 0x4000>; + no-map; + }; }; }; @@ -382,7 +387,7 @@ clock-names = "iface", "bus", "nav", "snoc_axi", "mnoc_axi", "xo"; iommus = <&apps_smmu 0x461 0x0>, <&apps_smmu 0x444 0x3>; - memory-region = <&mba_mem &mpss_mem>; + memory-region = <&mba_mem>, <&mpss_mem>, <&mdata_mem>; resets = <&aoss_reset AOSS_CC_MSS_RESTART>, <&pdc_reset PDC_MODEM_SYNC_RESET>; diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi index d134d172a3c5..3f2e7175afd8 100644 --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi @@ -81,6 +81,11 @@ reg = <0x0 0x94400000 0x0 0x200000>; no-map; }; + + mdata_mem: memory@94e00000 { + reg = <0x0 0x94e00000 0x0 0x4000>; + no-map; + }; }; aliases { @@ -865,7 +870,7 @@ hp_i2c: &i2c9 { clock-names = "iface", "bus", "nav", "snoc_axi", "mnoc_axi", "xo"; iommus = <&apps_smmu 0x461 0x0>, <&apps_smmu 0x444 0x3>; - memory-region = <&mba_mem &mpss_mem>; + memory-region = <&mba_mem>, <&mpss_mem>, <&mdata_mem>; /* This gets overridden for SKUs with LTE support. */ firmware-name = "qcom/sc7180-trogdor/modem-nolte/mba.mbn", diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi index bf522a64b172..bda0495aa0b5 100644 --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi @@ -17,6 +17,11 @@ reg = <0x0 0x9c700000 0x0 0x200000>; no-map; }; + + mdata_mem: memory@9d100000 { + reg = <0x0 0x9d100000 0x0 0x4000>; + no-map; + }; }; }; @@ -32,7 +37,7 @@ iommus = <&apps_smmu 0x124 0x0>, <&apps_smmu 0x488 0x7>; interconnects = <&mc_virt MASTER_LLCC 0 &mc_virt SLAVE_EBI1 0>; - memory-region = <&mba_mem>, <&mpss_mem>; + memory-region = <&mba_mem>, <&mpss_mem>, <&mdata_mem>; firmware-name = "qcom/sc7280-herobrine/modem/mba.mbn", "qcom/sc7280-herobrine/modem/qdsp6sw.mbn"; diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index 65032b94b46d..56050f35c232 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -122,6 +122,11 @@ qcom,vmid = <15>; }; + mdata_mem: memory@89700000 { + reg = <0 0x89700000 0 0x4000>; + no-map; + }; + qseecom_mem: qseecom@8ab00000 { reg = <0 0x8ab00000 0 0x1400000>; no-map; @@ -3283,6 +3288,10 @@ memory-region = <&mpss_region>; }; + metadata { + memory-region = <&mdata_mem>; + }; + glink-edge { interrupts = <GIC_SPI 449 IRQ_TYPE_EDGE_RISING>; label = "modem";
Introduce a new carveout for modem metadata. This will serve as a replacement for the memory region used by MSA to authenticate modem ELF headers. Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com> --- arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi | 6 ++++++ arch/arm64/boot/dts/qcom/msm8996.dtsi | 9 +++++++++ arch/arm64/boot/dts/qcom/msm8998.dtsi | 9 +++++++++ arch/arm64/boot/dts/qcom/sc7180-idp.dts | 7 ++++++- arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 7 ++++++- arch/arm64/boot/dts/qcom/sc7280-herobrine-lte-sku.dtsi | 7 ++++++- arch/arm64/boot/dts/qcom/sdm845.dtsi | 9 +++++++++ 7 files changed, 51 insertions(+), 3 deletions(-)