2022-12-13 14:48:32

by Sibi Sankar

[permalink] [raw]
Subject: [PATCH 0/4] Fix XPU violation during modem metadata authentication

The memory region allocated using dma_alloc_attr with no kernel mapping
attribute set would still be a part of the linear kernel map. Any access
to this region by the application processor after assigning it to the
remote Q6 will result in a XPU violation. Fix this by replacing the
dynamically allocated memory region with a no-map carveout and unmap the
modem metadata memory region before passing control to the remote Q6.
The addition of the carveout and memunmap is required only on SoCs that
mandate memory protection before transferring control to Q6, hence the
driver falls back to dynamic memory allocation in the absence of the
modem metadata carveout.

Relevant discussions on the mailing list:
https://lore.kernel.org/lkml/[email protected]/

Depends on:
https://patchwork.kernel.org/project/linux-arm-msm/cover/[email protected]/

Reported-by: Amit Pundir <[email protected]>
https://people.linaro.org/~amit.pundir/linaro-sid-developer-dragonboard-845c-569/6.1-rc4_defconfig
Reproduced with ^^ defconfig SDM845 SoCs

Sibi Sankar (4):
arm64: dts: qcom: Introduce a carveout for modem metadata
dt-bindings: remoteproc: qcom: sc7180: Update memory-region
requirements
dt-bindings: remoteproc: qcom: q6v5: Update memory region requirements
remoteproc: qcom_q6v5_mss: Use a carveout to authenticate modem
headers

.../bindings/remoteproc/qcom,q6v5.txt | 29 ++++++-
.../remoteproc/qcom,sc7180-mss-pil.yaml | 3 +-
.../remoteproc/qcom,sc7280-mss-pil.yaml | 3 +-
.../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 +-
.../dts/qcom/sc7280-herobrine-lte-sku.dtsi | 7 +-
arch/arm64/boot/dts/qcom/sdm845.dtsi | 9 ++
drivers/remoteproc/qcom_q6v5_mss.c | 85 +++++++++++++------
11 files changed, 142 insertions(+), 32 deletions(-)

--
2.17.1


2022-12-13 15:07:44

by Sibi Sankar

[permalink] [raw]
Subject: [PATCH 1/4] arm64: dts: qcom: Introduce a carveout for modem metadata

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 <[email protected]>
---
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 {
+ 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";
--
2.17.1

2022-12-13 20:15:23

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/4] arm64: dts: qcom: Introduce a carveout for modem metadata

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 <[email protected]>

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

2022-12-14 07:04:09

by Sibi Sankar

[permalink] [raw]
Subject: Re: [PATCH 1/4] arm64: dts: qcom: Introduce a carveout for modem metadata

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 <[email protected]>
>
> 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
>

2022-12-14 08:58:34

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/4] arm64: dts: qcom: Introduce a carveout for modem metadata

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 <[email protected]>
>>
>> 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

2022-12-14 11:42:19

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/4] arm64: dts: qcom: Introduce a carveout for modem metadata

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 <[email protected]>
> ---
> 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

2022-12-14 12:18:06

by Sibi Sankar

[permalink] [raw]
Subject: Re: [PATCH 1/4] arm64: dts: qcom: Introduce a carveout for modem metadata



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 <[email protected]>
>> ---
>> 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
>

2022-12-14 13:00:53

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/4] arm64: dts: qcom: Introduce a carveout for modem metadata

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 <[email protected]>
>>> ---
>>> 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

2022-12-27 13:28:16

by Amit Pundir

[permalink] [raw]
Subject: Re: [PATCH 0/4] Fix XPU violation during modem metadata authentication

On Tue, 13 Dec 2022 at 19:37, Sibi Sankar <[email protected]> wrote:
>
> The memory region allocated using dma_alloc_attr with no kernel mapping
> attribute set would still be a part of the linear kernel map. Any access
> to this region by the application processor after assigning it to the
> remote Q6 will result in a XPU violation. Fix this by replacing the
> dynamically allocated memory region with a no-map carveout and unmap the
> modem metadata memory region before passing control to the remote Q6.
> The addition of the carveout and memunmap is required only on SoCs that
> mandate memory protection before transferring control to Q6, hence the
> driver falls back to dynamic memory allocation in the absence of the
> modem metadata carveout.
>
> Relevant discussions on the mailing list:
> https://lore.kernel.org/lkml/[email protected]/
>
> Depends on:
> https://patchwork.kernel.org/project/linux-arm-msm/cover/[email protected]/
>
> Reported-by: Amit Pundir <[email protected]>

Smoke tested this series on db845c (SDM845) running v6.2-rc1, with the
upstream workaround (b7d9aae40484 Revert "arm64: dma: Drop cache
invalidation from arch_dma_prep_coherent()") reverted and I can no
longer reproduce the above crash in my limited (10+) test runs so far.
So for the entire series:

Tested-by: Amit Pundir <[email protected]>

Regards,
Amit Pundir

> https://people.linaro.org/~amit.pundir/linaro-sid-developer-dragonboard-845c-569/6.1-rc4_defconfig
> Reproduced with ^^ defconfig SDM845 SoCs
>
> Sibi Sankar (4):
> arm64: dts: qcom: Introduce a carveout for modem metadata
> dt-bindings: remoteproc: qcom: sc7180: Update memory-region
> requirements
> dt-bindings: remoteproc: qcom: q6v5: Update memory region requirements
> remoteproc: qcom_q6v5_mss: Use a carveout to authenticate modem
> headers
>
> .../bindings/remoteproc/qcom,q6v5.txt | 29 ++++++-
> .../remoteproc/qcom,sc7180-mss-pil.yaml | 3 +-
> .../remoteproc/qcom,sc7280-mss-pil.yaml | 3 +-
> .../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 +-
> .../dts/qcom/sc7280-herobrine-lte-sku.dtsi | 7 +-
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 9 ++
> drivers/remoteproc/qcom_q6v5_mss.c | 85 +++++++++++++------
> 11 files changed, 142 insertions(+), 32 deletions(-)
>
> --
> 2.17.1
>