[PATCH v2] soc: qcom: cmd-db: Map shared memory as WC, not WB

[PATCH v2] soc: qcom: cmd-db: Map shared memory as WC, not WB

[Maulik Shah]

From: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>

Linux does not write into cmd-db region. This region of memory is write
protected by XPU. XPU may sometime falsely detect clean cache eviction
as "write" into the write protected region leading to secure interrupt
which causes an endless loop somewhere in Trust Zone.

The only reason it is working right now is because Qualcomm Hypervisor
maps the same region as Non-Cacheable memory in Stage 2 translation
tables. The issue manifests if we want to use another hypervisor (like
Xen or KVM), which does not know anything about those specific mappings.

Changing the mapping of cmd-db memory from MEMREMAP_WB to MEMREMAP_WT/WC
removes dependency on correct mappings in Stage 2 tables. This patch
fixes the issue by updating the mapping to MEMREMAP_WC.

I tested this on SA8155P with Xen.

Fixes: 312416d9171a ("drivers: qcom: add command DB driver")
Cc: stable@vger.kernel.org # 5.4+
Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
Tested-by: Nikita Travkin <nikita@trvn.ru> # sc7180 WoA in EL2
Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>
---
Changes in v2:
 - Use MEMREMAP_WC instead of MEMREMAP_WT
 - Update commit message from comments in v1
 - Add Fixes tag and Cc to stable
 - Link to v1: https://lore.kernel.org/lkml/20240327200917.2576034-1-volodymyr_babchuk@epam.com
---
 drivers/soc/qcom/cmd-db.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c
index d84572662017..ae66c2623d25 100644
--- a/drivers/soc/qcom/cmd-db.c
+++ b/drivers/soc/qcom/cmd-db.c
@@ -349,7 +349,7 @@ static int cmd_db_dev_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	cmd_db_header = memremap(rmem->base, rmem->size, MEMREMAP_WB);
+	cmd_db_header = memremap(rmem->base, rmem->size, MEMREMAP_WC);
 	if (!cmd_db_header) {
 		ret = -ENOMEM;
 		cmd_db_header = NULL;

---
base-commit: 797012914d2d031430268fe512af0ccd7d8e46ef
change-id: 20240718-cmd_db_uncached-e896da5c5296

Best regards,
-- 
Maulik Shah <quic_mkshah@quicinc.com>

Re: [PATCH v2] soc: qcom: cmd-db: Map shared memory as WC, not WB

[Pavan Kondeti]

On Thu, Jul 18, 2024 at 11:33:23AM +0530, Maulik Shah wrote:
> From: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
> 
> Linux does not write into cmd-db region. This region of memory is write
> protected by XPU. XPU may sometime falsely detect clean cache eviction
> as "write" into the write protected region leading to secure interrupt
> which causes an endless loop somewhere in Trust Zone.
> 
> The only reason it is working right now is because Qualcomm Hypervisor
> maps the same region as Non-Cacheable memory in Stage 2 translation
> tables. The issue manifests if we want to use another hypervisor (like
> Xen or KVM), which does not know anything about those specific mappings.
> 
> Changing the mapping of cmd-db memory from MEMREMAP_WB to MEMREMAP_WT/WC
> removes dependency on correct mappings in Stage 2 tables. This patch
> fixes the issue by updating the mapping to MEMREMAP_WC.
> 
> I tested this on SA8155P with Xen.
> 
> Fixes: 312416d9171a ("drivers: qcom: add command DB driver")
> Cc: stable@vger.kernel.org # 5.4+
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> Tested-by: Nikita Travkin <nikita@trvn.ru> # sc7180 WoA in EL2
> Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>
> ---
> Changes in v2:
>  - Use MEMREMAP_WC instead of MEMREMAP_WT
>  - Update commit message from comments in v1
>  - Add Fixes tag and Cc to stable
>  - Link to v1: https://lore.kernel.org/lkml/20240327200917.2576034-1-volodymyr_babchuk@epam.com
> ---
>  drivers/soc/qcom/cmd-db.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c
> index d84572662017..ae66c2623d25 100644
> --- a/drivers/soc/qcom/cmd-db.c
> +++ b/drivers/soc/qcom/cmd-db.c
> @@ -349,7 +349,7 @@ static int cmd_db_dev_probe(struct platform_device *pdev)
>  		return -EINVAL;
>  	}
>  
> -	cmd_db_header = memremap(rmem->base, rmem->size, MEMREMAP_WB);
> +	cmd_db_header = memremap(rmem->base, rmem->size, MEMREMAP_WC);
>  	if (!cmd_db_header) {
>  		ret = -ENOMEM;
>  		cmd_db_header = NULL;
> 

Thanks Maulik for sharing the patch. It works as expected. Feel free to
use

Tested-by: Pavankumar Kondeti <quic_pkondeti@quicinc.com>

Thanks,
Pavan

Re: [PATCH v2] soc: qcom: cmd-db: Map shared memory as WC, not WB

[Caleb Connolly]

On 18/07/2024 08:03, Maulik Shah wrote:
> From: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
> 
> Linux does not write into cmd-db region. This region of memory is write
> protected by XPU. XPU may sometime falsely detect clean cache eviction
> as "write" into the write protected region leading to secure interrupt
> which causes an endless loop somewhere in Trust Zone.
> 
> The only reason it is working right now is because Qualcomm Hypervisor
> maps the same region as Non-Cacheable memory in Stage 2 translation
> tables. The issue manifests if we want to use another hypervisor (like
> Xen or KVM), which does not know anything about those specific mappings.
> 
> Changing the mapping of cmd-db memory from MEMREMAP_WB to MEMREMAP_WT/WC
> removes dependency on correct mappings in Stage 2 tables. This patch
> fixes the issue by updating the mapping to MEMREMAP_WC.
> 
> I tested this on SA8155P with Xen.
> 
> Fixes: 312416d9171a ("drivers: qcom: add command DB driver")
> Cc: stable@vger.kernel.org # 5.4+
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> Tested-by: Nikita Travkin <nikita@trvn.ru> # sc7180 WoA in EL2
> Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>

Reviewed-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
> Changes in v2:
>   - Use MEMREMAP_WC instead of MEMREMAP_WT
>   - Update commit message from comments in v1
>   - Add Fixes tag and Cc to stable
>   - Link to v1: https://lore.kernel.org/lkml/20240327200917.2576034-1-volodymyr_babchuk@epam.com
> ---
>   drivers/soc/qcom/cmd-db.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c
> index d84572662017..ae66c2623d25 100644
> --- a/drivers/soc/qcom/cmd-db.c
> +++ b/drivers/soc/qcom/cmd-db.c
> @@ -349,7 +349,7 @@ static int cmd_db_dev_probe(struct platform_device *pdev)
>   		return -EINVAL;
>   	}
>   
> -	cmd_db_header = memremap(rmem->base, rmem->size, MEMREMAP_WB);
> +	cmd_db_header = memremap(rmem->base, rmem->size, MEMREMAP_WC);
>   	if (!cmd_db_header) {
>   		ret = -ENOMEM;
>   		cmd_db_header = NULL;
> 
> ---
> base-commit: 797012914d2d031430268fe512af0ccd7d8e46ef
> change-id: 20240718-cmd_db_uncached-e896da5c5296
> 
> Best regards,

-- 
// Caleb (they/them)

Re: [PATCH v2] soc: qcom: cmd-db: Map shared memory as WC, not WB

[Caleb Connolly]

Hi Pavan,

> 
> Thanks Maulik for sharing the patch. It works as expected. Feel free to
> use

Can I ask how you're testing this?

Kind regards,
> 
> Tested-by: Pavankumar Kondeti <quic_pkondeti@quicinc.com>
> 
> Thanks,
> Pavan

-- 
// Caleb (they/them)

Re: [PATCH v2] soc: qcom: cmd-db: Map shared memory as WC, not WB

[Pavan Kondeti]

On Thu, Jul 18, 2024 at 09:42:27AM +0200, Caleb Connolly wrote:
> Hi Pavan,
> 
> > 
> > Thanks Maulik for sharing the patch. It works as expected. Feel free to
> > use
> 
> Can I ask how you're testing this?
> 
> Kind regards,
> > 
> > Tested-by: Pavankumar Kondeti <quic_pkondeti@quicinc.com>
> > 
> 

The QCM6490 RB3 board boots from upstream kernel. As part of releases
here [1] we plan to support booting Linux in EL2. Currently, I have an
internal board/build with firmware allowing this already. I have boot tested
Maulik's patch (and as well v1 patch). The targets gets reset early in
the boot up w/o this patch and I could boot w/ this patch.

Thanks,
Pavan

[1] https://github.com/quic-yocto/qcom-manifest

Re: [PATCH v2] soc: qcom: cmd-db: Map shared memory as WC, not WB

[Caleb Connolly]

On 18/07/2024 09:55, Pavan Kondeti wrote:
> On Thu, Jul 18, 2024 at 09:42:27AM +0200, Caleb Connolly wrote:
>> Hi Pavan,
>>
>>>
>>> Thanks Maulik for sharing the patch. It works as expected. Feel free to
>>> use
>>
>> Can I ask how you're testing this?
>>
>> Kind regards,
>>>
>>> Tested-by: Pavankumar Kondeti <quic_pkondeti@quicinc.com>
>>>
>>
> 
> The QCM6490 RB3 board boots from upstream kernel. As part of releases
> here [1] we plan to support booting Linux in EL2. Currently, I have an
> internal board/build with firmware allowing this already. I have boot tested
> Maulik's patch (and as well v1 patch). The targets gets reset early in
> the boot up w/o this patch and I could boot w/ this patch.

Ahh awesome! Thanks for the info :)
> 
> Thanks,
> Pavan
> 
> [1] https://github.com/quic-yocto/qcom-manifest

-- 
// Caleb (they/them)

Re: [PATCH v2] soc: qcom: cmd-db: Map shared memory as WC, not WB

[Pavan Kondeti]

Hi Bjorn,

On Thu, Jul 18, 2024 at 09:41:34AM +0200, Caleb Connolly wrote:
> 
> 
> On 18/07/2024 08:03, Maulik Shah wrote:
> > From: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
> > 
> > Linux does not write into cmd-db region. This region of memory is write
> > protected by XPU. XPU may sometime falsely detect clean cache eviction
> > as "write" into the write protected region leading to secure interrupt
> > which causes an endless loop somewhere in Trust Zone.
> > 
> > The only reason it is working right now is because Qualcomm Hypervisor
> > maps the same region as Non-Cacheable memory in Stage 2 translation
> > tables. The issue manifests if we want to use another hypervisor (like
> > Xen or KVM), which does not know anything about those specific mappings.
> > 
> > Changing the mapping of cmd-db memory from MEMREMAP_WB to MEMREMAP_WT/WC
> > removes dependency on correct mappings in Stage 2 tables. This patch
> > fixes the issue by updating the mapping to MEMREMAP_WC.
> > 
> > I tested this on SA8155P with Xen.
> > 
> > Fixes: 312416d9171a ("drivers: qcom: add command DB driver")
> > Cc: stable@vger.kernel.org # 5.4+
> > Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> > Tested-by: Nikita Travkin <nikita@trvn.ru> # sc7180 WoA in EL2
> > Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>
> 
> Reviewed-by: Caleb Connolly <caleb.connolly@linaro.org>

Is it possible to include it in v6.11-rc?

Thanks,
Pavan

Re: [PATCH v2] soc: qcom: cmd-db: Map shared memory as WC, not WB

[Bjorn Andersson]

On Thu, 18 Jul 2024 11:33:23 +0530, Maulik Shah wrote:
> Linux does not write into cmd-db region. This region of memory is write
> protected by XPU. XPU may sometime falsely detect clean cache eviction
> as "write" into the write protected region leading to secure interrupt
> which causes an endless loop somewhere in Trust Zone.
> 
> The only reason it is working right now is because Qualcomm Hypervisor
> maps the same region as Non-Cacheable memory in Stage 2 translation
> tables. The issue manifests if we want to use another hypervisor (like
> Xen or KVM), which does not know anything about those specific mappings.
> 
> [...]

Applied, thanks!

[1/1] soc: qcom: cmd-db: Map shared memory as WC, not WB
      commit: f9bb896eab221618927ae6a2f1d566567999839d

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>