* [PATCH v5] usb: misc: qcom_eud: Access EUD_MODE_MANAGER2 through secure calls
@ 2025-07-15 11:45 Komal Bajaj
2025-07-15 12:33 ` Dmitry Baryshkov
0 siblings, 1 reply; 2+ messages in thread
From: Komal Bajaj @ 2025-07-15 11:45 UTC (permalink / raw)
To: Greg Kroah-Hartman, Souradeep Chowdhury
Cc: linux-usb, linux-kernel, linux-arm-msm, Melody Olvera,
Komal Bajaj, Konrad Dybcio
EUD_MODE_MANAGER2 register is mapped to a memory region that is marked
as read-only for HLOS, enforcing access restrictions that prohibit
direct memory-mapped writes via writel().
Attempts to write to this region from HLOS can result in silent failures
or memory access violations, particularly when toggling EUD (Embedded
USB Debugger) state. To ensure secure register access, modify the driver
to use qcom_scm_io_writel(), which routes the write operation to Qualcomm
Secure Channel Monitor (SCM). SCM has the necessary permissions to access
protected memory regions, enabling reliable control over EUD state.
SC7280, the only user of EUD is also affected, indicating that this could
never have worked on a properly fused device.
Fixes: 9a1bf58ccd44 ("usb: misc: eud: Add driver support for Embedded USB Debugger(EUD)")
Signed-off-by: Melody Olvera <quic_molvera@quicinc.com>
Signed-off-by: Komal Bajaj <komal.bajaj@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
Changes in v5:
* Changed select QCOM_SCM to depends on QCOM_SCM in Kconfig per Greg's review
* Link to v4: https://lore.kernel.org/all/20250709065533.25724-1-komal.bajaj@oss.qualcomm.com/
Changes in v4:
* Added error logging in disable_eud() for SCM write failures, per Konrad’s suggestion
* Link to v3: https://lore.kernel.org/all/20250708085208.19089-1-komal.bajaj@oss.qualcomm.com/
Changes in v3:
* Moved secure write before normal writes
* Added error checking in disable_eud()
* Use ENOMEM error code if platform_get_resource() fails
* Select QCOM_SCM driver if USB_QCOM_EUD is enabled
* Link to v2: https://lore.kernel.org/all/20250627125131.27606-1-komal.bajaj@oss.qualcomm.com/
Changes in v2:
* Drop separate compatible to be added for secure eud
* Use secure call to access EUD mode manager register
* Link to v1: https://lore.kernel.org/all/20240807183205.803847-1-quic_molvera@quicinc.com/
---
drivers/usb/misc/Kconfig | 1 +
drivers/usb/misc/qcom_eud.c | 27 +++++++++++++++++++++------
2 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
index 6497c4e81e951a14201ad965dadc29f9888f8254..73ebd3257625e4567f33636cdfd756344b9ed4e7 100644
--- a/drivers/usb/misc/Kconfig
+++ b/drivers/usb/misc/Kconfig
@@ -147,6 +147,7 @@ config USB_APPLEDISPLAY
config USB_QCOM_EUD
tristate "QCOM Embedded USB Debugger(EUD) Driver"
depends on ARCH_QCOM || COMPILE_TEST
+ depends on QCOM_SCM
select USB_ROLE_SWITCH
help
This module enables support for Qualcomm Technologies, Inc.
diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c
index 83079c414b4f281b2136d0d1eb39418c7f94ff8c..a58081f53db32935f09c685e4599985285133891 100644
--- a/drivers/usb/misc/qcom_eud.c
+++ b/drivers/usb/misc/qcom_eud.c
@@ -15,6 +15,7 @@
#include <linux/slab.h>
#include <linux/sysfs.h>
#include <linux/usb/role.h>
+#include <linux/firmware/qcom/qcom_scm.h>
#define EUD_REG_INT1_EN_MASK 0x0024
#define EUD_REG_INT_STATUS_1 0x0044
@@ -34,7 +35,7 @@ struct eud_chip {
struct device *dev;
struct usb_role_switch *role_sw;
void __iomem *base;
- void __iomem *mode_mgr;
+ phys_addr_t mode_mgr;
unsigned int int_status;
int irq;
bool enabled;
@@ -43,18 +44,30 @@ struct eud_chip {
static int enable_eud(struct eud_chip *priv)
{
+ int ret;
+
+ ret = qcom_scm_io_writel(priv->mode_mgr + EUD_REG_EUD_EN2, 1);
+ if (ret)
+ return ret;
+
writel(EUD_ENABLE, priv->base + EUD_REG_CSR_EUD_EN);
writel(EUD_INT_VBUS | EUD_INT_SAFE_MODE,
priv->base + EUD_REG_INT1_EN_MASK);
- writel(1, priv->mode_mgr + EUD_REG_EUD_EN2);
return usb_role_switch_set_role(priv->role_sw, USB_ROLE_DEVICE);
}
static void disable_eud(struct eud_chip *priv)
{
+ int ret;
+
+ ret = qcom_scm_io_writel(priv->mode_mgr + EUD_REG_EUD_EN2, 0);
+ if (ret) {
+ dev_err(priv->dev, "failed to disable eud\n");
+ return;
+ }
+
writel(0, priv->base + EUD_REG_CSR_EUD_EN);
- writel(0, priv->mode_mgr + EUD_REG_EUD_EN2);
}
static ssize_t enable_show(struct device *dev,
@@ -178,6 +191,7 @@ static void eud_role_switch_release(void *data)
static int eud_probe(struct platform_device *pdev)
{
struct eud_chip *chip;
+ struct resource *res;
int ret;
chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
@@ -200,9 +214,10 @@ static int eud_probe(struct platform_device *pdev)
if (IS_ERR(chip->base))
return PTR_ERR(chip->base);
- chip->mode_mgr = devm_platform_ioremap_resource(pdev, 1);
- if (IS_ERR(chip->mode_mgr))
- return PTR_ERR(chip->mode_mgr);
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ if (!res)
+ return -ENODEV;
+ chip->mode_mgr = res->start;
chip->irq = platform_get_irq(pdev, 0);
if (chip->irq < 0)
---
base-commit: 347e9f5043c89695b01e66b3ed111755afcf1911
change-id: 20250715-eud_mode_manager_secure_access-6e57e3c71ec2
Best regards,
--
Komal Bajaj <komal.bajaj@oss.qualcomm.com>
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v5] usb: misc: qcom_eud: Access EUD_MODE_MANAGER2 through secure calls
2025-07-15 11:45 [PATCH v5] usb: misc: qcom_eud: Access EUD_MODE_MANAGER2 through secure calls Komal Bajaj
@ 2025-07-15 12:33 ` Dmitry Baryshkov
0 siblings, 0 replies; 2+ messages in thread
From: Dmitry Baryshkov @ 2025-07-15 12:33 UTC (permalink / raw)
To: Komal Bajaj
Cc: Greg Kroah-Hartman, Souradeep Chowdhury, linux-usb, linux-kernel,
linux-arm-msm, Melody Olvera, Konrad Dybcio
On Tue, Jul 15, 2025 at 05:15:27PM +0530, Komal Bajaj wrote:
> EUD_MODE_MANAGER2 register is mapped to a memory region that is marked
> as read-only for HLOS, enforcing access restrictions that prohibit
> direct memory-mapped writes via writel().
>
> Attempts to write to this region from HLOS can result in silent failures
> or memory access violations, particularly when toggling EUD (Embedded
> USB Debugger) state. To ensure secure register access, modify the driver
> to use qcom_scm_io_writel(), which routes the write operation to Qualcomm
> Secure Channel Monitor (SCM). SCM has the necessary permissions to access
> protected memory regions, enabling reliable control over EUD state.
>
> SC7280, the only user of EUD is also affected, indicating that this could
> never have worked on a properly fused device.
>
> Fixes: 9a1bf58ccd44 ("usb: misc: eud: Add driver support for Embedded USB Debugger(EUD)")
> Signed-off-by: Melody Olvera <quic_molvera@quicinc.com>
> Signed-off-by: Komal Bajaj <komal.bajaj@oss.qualcomm.com>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> ---
> Changes in v5:
> * Changed select QCOM_SCM to depends on QCOM_SCM in Kconfig per Greg's review
> * Link to v4: https://lore.kernel.org/all/20250709065533.25724-1-komal.bajaj@oss.qualcomm.com/
>
> Changes in v4:
> * Added error logging in disable_eud() for SCM write failures, per Konrad’s suggestion
> * Link to v3: https://lore.kernel.org/all/20250708085208.19089-1-komal.bajaj@oss.qualcomm.com/
>
> Changes in v3:
> * Moved secure write before normal writes
> * Added error checking in disable_eud()
> * Use ENOMEM error code if platform_get_resource() fails
> * Select QCOM_SCM driver if USB_QCOM_EUD is enabled
> * Link to v2: https://lore.kernel.org/all/20250627125131.27606-1-komal.bajaj@oss.qualcomm.com/
>
> Changes in v2:
> * Drop separate compatible to be added for secure eud
> * Use secure call to access EUD mode manager register
> * Link to v1: https://lore.kernel.org/all/20240807183205.803847-1-quic_molvera@quicinc.com/
> ---
> drivers/usb/misc/Kconfig | 1 +
> drivers/usb/misc/qcom_eud.c | 27 +++++++++++++++++++++------
> 2 files changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
> index 6497c4e81e951a14201ad965dadc29f9888f8254..73ebd3257625e4567f33636cdfd756344b9ed4e7 100644
> --- a/drivers/usb/misc/Kconfig
> +++ b/drivers/usb/misc/Kconfig
> @@ -147,6 +147,7 @@ config USB_APPLEDISPLAY
> config USB_QCOM_EUD
> tristate "QCOM Embedded USB Debugger(EUD) Driver"
> depends on ARCH_QCOM || COMPILE_TEST
> + depends on QCOM_SCM
> select USB_ROLE_SWITCH
> help
> This module enables support for Qualcomm Technologies, Inc.
> diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c
> index 83079c414b4f281b2136d0d1eb39418c7f94ff8c..a58081f53db32935f09c685e4599985285133891 100644
> --- a/drivers/usb/misc/qcom_eud.c
> +++ b/drivers/usb/misc/qcom_eud.c
> @@ -15,6 +15,7 @@
> #include <linux/slab.h>
> #include <linux/sysfs.h>
> #include <linux/usb/role.h>
> +#include <linux/firmware/qcom/qcom_scm.h>
>
> #define EUD_REG_INT1_EN_MASK 0x0024
> #define EUD_REG_INT_STATUS_1 0x0044
> @@ -34,7 +35,7 @@ struct eud_chip {
> struct device *dev;
> struct usb_role_switch *role_sw;
> void __iomem *base;
> - void __iomem *mode_mgr;
> + phys_addr_t mode_mgr;
> unsigned int int_status;
> int irq;
> bool enabled;
> @@ -43,18 +44,30 @@ struct eud_chip {
>
> static int enable_eud(struct eud_chip *priv)
> {
> + int ret;
> +
> + ret = qcom_scm_io_writel(priv->mode_mgr + EUD_REG_EUD_EN2, 1);
> + if (ret)
> + return ret;
> +
> writel(EUD_ENABLE, priv->base + EUD_REG_CSR_EUD_EN);
> writel(EUD_INT_VBUS | EUD_INT_SAFE_MODE,
> priv->base + EUD_REG_INT1_EN_MASK);
> - writel(1, priv->mode_mgr + EUD_REG_EUD_EN2);
>
> return usb_role_switch_set_role(priv->role_sw, USB_ROLE_DEVICE);
> }
>
> static void disable_eud(struct eud_chip *priv)
> {
> + int ret;
> +
> + ret = qcom_scm_io_writel(priv->mode_mgr + EUD_REG_EUD_EN2, 0);
> + if (ret) {
> + dev_err(priv->dev, "failed to disable eud\n");
> + return;
Change this (and enable_store()) to propagate the error code.
> + }
> +
> writel(0, priv->base + EUD_REG_CSR_EUD_EN);
> - writel(0, priv->mode_mgr + EUD_REG_EUD_EN2);
> }
>
> static ssize_t enable_show(struct device *dev,
> @@ -178,6 +191,7 @@ static void eud_role_switch_release(void *data)
> static int eud_probe(struct platform_device *pdev)
> {
> struct eud_chip *chip;
> + struct resource *res;
> int ret;
>
> chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
> @@ -200,9 +214,10 @@ static int eud_probe(struct platform_device *pdev)
> if (IS_ERR(chip->base))
> return PTR_ERR(chip->base);
>
> - chip->mode_mgr = devm_platform_ioremap_resource(pdev, 1);
> - if (IS_ERR(chip->mode_mgr))
> - return PTR_ERR(chip->mode_mgr);
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + if (!res)
> + return -ENODEV;
> + chip->mode_mgr = res->start;
>
> chip->irq = platform_get_irq(pdev, 0);
> if (chip->irq < 0)
>
> ---
> base-commit: 347e9f5043c89695b01e66b3ed111755afcf1911
> change-id: 20250715-eud_mode_manager_secure_access-6e57e3c71ec2
>
> Best regards,
> --
> Komal Bajaj <komal.bajaj@oss.qualcomm.com>
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-07-15 12:33 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-15 11:45 [PATCH v5] usb: misc: qcom_eud: Access EUD_MODE_MANAGER2 through secure calls Komal Bajaj
2025-07-15 12:33 ` Dmitry Baryshkov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).