| Message ID | 20240718-cmd_db_uncached-v2-1-f6cf53164c90@quicinc.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | f9bb896eab221618927ae6a2f1d566567999839d |
| Headers | show |
| Series | [v2] soc: qcom: cmd-db: Map shared memory as WC, not WB | expand |
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
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,
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
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
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
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
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,
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;