[PATCH v7] usb: misc: qcom_eud: Access EUD_MODE_MANAGER2 through secure calls

[PATCH v7] usb: misc: qcom_eud: Access EUD_MODE_MANAGER2 through secure calls

[Komal Bajaj]

EUD_MODE_MANAGER2 register is mapped to a memory region that is marked
as read-only for operating system running at EL1, 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 v7:
- Updated the commit message as per Greg's comment
- Link to v6: https://lore.kernel.org/r/20250721-eud_mode_manager_secure_access-v6-1-fe603325ac04@oss.qualcomm.com

Changes in v6:
- Propagating the error code from disable_eud(), per Dmitry's suggestion
- Link to v5: https://lore.kernel.org/r/20250715-eud_mode_manager_secure_access-v5-1-e769be308d4a@oss.qualcomm.com

usb: misc: qcom_eud: Access EUD_MODE_MANAGER2 through secure calls

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 | 33 ++++++++++++++++++++++++---------
 2 files changed, 25 insertions(+), 9 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..05c8bdc943a88dab6159a05c2d770484c084f7b7 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,29 @@ 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)
+static int disable_eud(struct eud_chip *priv)
 {
+	int ret;
+
+	ret = qcom_scm_io_writel(priv->mode_mgr + EUD_REG_EUD_EN2, 0);
+	if (ret)
+		return ret;
+
 	writel(0, priv->base + EUD_REG_CSR_EUD_EN);
-	writel(0, priv->mode_mgr + EUD_REG_EUD_EN2);
+	return 0;
 }
 
 static ssize_t enable_show(struct device *dev,
@@ -82,11 +94,12 @@ static ssize_t enable_store(struct device *dev,
 			chip->enabled = enable;
 		else
 			disable_eud(chip);
+
 	} else {
-		disable_eud(chip);
+		ret = disable_eud(chip);
 	}
 
-	return count;
+	return ret < 0 ? ret : count;
 }
 
 static DEVICE_ATTR_RW(enable);
@@ -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>

Re: [PATCH v7] usb: misc: qcom_eud: Access EUD_MODE_MANAGER2 through secure calls

[Souradeep Chowdhury]

On 7/22/2025 5:01 PM, Komal Bajaj wrote:
> EUD_MODE_MANAGER2 register is mapped to a memory region that is marked
> as read-only for operating system running at EL1, 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>
> ---
Reviewed-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
> Changes in v7:
> - Updated the commit message as per Greg's comment
> - Link to v6: https://lore.kernel.org/r/20250721-eud_mode_manager_secure_access-v6-1-fe603325ac04@oss.qualcomm.com
>
> Changes in v6:
> - Propagating the error code from disable_eud(), per Dmitry's suggestion
> - Link to v5: https://lore.kernel.org/r/20250715-eud_mode_manager_secure_access-v5-1-e769be308d4a@oss.qualcomm.com
>
> usb: misc: qcom_eud: Access EUD_MODE_MANAGER2 through secure calls
>
> 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 | 33 ++++++++++++++++++++++++---------
>   2 files changed, 25 insertions(+), 9 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..05c8bdc943a88dab6159a05c2d770484c084f7b7 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,29 @@ 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)
> +static int disable_eud(struct eud_chip *priv)
>   {
> +	int ret;
> +
> +	ret = qcom_scm_io_writel(priv->mode_mgr + EUD_REG_EUD_EN2, 0);
> +	if (ret)
> +		return ret;
> +
>   	writel(0, priv->base + EUD_REG_CSR_EUD_EN);
> -	writel(0, priv->mode_mgr + EUD_REG_EUD_EN2);
> +	return 0;
>   }
>   
>   static ssize_t enable_show(struct device *dev,
> @@ -82,11 +94,12 @@ static ssize_t enable_store(struct device *dev,
>   			chip->enabled = enable;
>   		else
>   			disable_eud(chip);
> +
>   	} else {
> -		disable_eud(chip);
> +		ret = disable_eud(chip);
>   	}
>   
> -	return count;
> +	return ret < 0 ? ret : count;
>   }
>   
>   static DEVICE_ATTR_RW(enable);
> @@ -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,

Re: [PATCH v7] usb: misc: qcom_eud: Access EUD_MODE_MANAGER2 through secure calls

[Dmitry Baryshkov]

On Tue, Jul 22, 2025 at 05:01:53PM +0530, Komal Bajaj wrote:
> EUD_MODE_MANAGER2 register is mapped to a memory region that is marked
> as read-only for operating system running at EL1, 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 v7:
> - Updated the commit message as per Greg's comment
> - Link to v6: https://lore.kernel.org/r/20250721-eud_mode_manager_secure_access-v6-1-fe603325ac04@oss.qualcomm.com
> 
> Changes in v6:
> - Propagating the error code from disable_eud(), per Dmitry's suggestion
> - Link to v5: https://lore.kernel.org/r/20250715-eud_mode_manager_secure_access-v5-1-e769be308d4a@oss.qualcomm.com
> 
> usb: misc: qcom_eud: Access EUD_MODE_MANAGER2 through secure calls
> 
> 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 | 33 ++++++++++++++++++++++++---------
>  2 files changed, 25 insertions(+), 9 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

Re: [PATCH v7] usb: misc: qcom_eud: Access EUD_MODE_MANAGER2 through secure calls

[Greg Kroah-Hartman]

On Tue, Jul 22, 2025 at 05:01:53PM +0530, Komal Bajaj wrote:
> EUD_MODE_MANAGER2 register is mapped to a memory region that is marked
> as read-only for operating system running at EL1, 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 v7:
> - Updated the commit message as per Greg's comment
> - Link to v6: https://lore.kernel.org/r/20250721-eud_mode_manager_secure_access-v6-1-fe603325ac04@oss.qualcomm.com
> 
> Changes in v6:
> - Propagating the error code from disable_eud(), per Dmitry's suggestion
> - Link to v5: https://lore.kernel.org/r/20250715-eud_mode_manager_secure_access-v5-1-e769be308d4a@oss.qualcomm.com
> 
> usb: misc: qcom_eud: Access EUD_MODE_MANAGER2 through secure calls
> 
> 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 | 33 ++++++++++++++++++++++++---------
>  2 files changed, 25 insertions(+), 9 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

You now are preventing this code from ever being able to be built in any
testing systems, including mine, so I don't even know if this patch
builds or not.

You did not even document this in the changelog :(

{sigh}

greg k-h

Re: [PATCH v7] usb: misc: qcom_eud: Access EUD_MODE_MANAGER2 through secure calls

[Komal Bajaj]

On Thu, Jul 24, 2025 at 3:06 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Tue, Jul 22, 2025 at 05:01:53PM +0530, Komal Bajaj wrote:
> > EUD_MODE_MANAGER2 register is mapped to a memory region that is marked
> > as read-only for operating system running at EL1, 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 v7:
> > - Updated the commit message as per Greg's comment
> > - Link to v6: https://lore.kernel.org/r/20250721-eud_mode_manager_secure_access-v6-1-fe603325ac04@oss.qualcomm.com
> >
> > Changes in v6:
> > - Propagating the error code from disable_eud(), per Dmitry's suggestion
> > - Link to v5: https://lore.kernel.org/r/20250715-eud_mode_manager_secure_access-v5-1-e769be308d4a@oss.qualcomm.com
> >
> > usb: misc: qcom_eud: Access EUD_MODE_MANAGER2 through secure calls
> >
> > 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 | 33 ++++++++++++++++++++++++---------
> >  2 files changed, 25 insertions(+), 9 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
>
> You now are preventing this code from ever being able to be built in any
> testing systems, including mine, so I don't even know if this patch
> builds or not.
>
> You did not even document this in the changelog :(

QCOM_SCM is essential for QCOM_EUD for its functionality.
One option I'm considering is using select QCOM_SCM, which ensures
dependency is enabled when QCOM_EUD is selected. QCOM_SCM facilitates
communication with secure world, this approach should not cause issues even
when COMPILE_TEST is enabled on non-ARCH_QCOM platforms.

Alternatively, I could use a conditional depends expression like:
depends on (ARCH_QCOM && QCOM_SCM) || COMPILE_TEST

This would allow the driver to be built under COMPILE_TEST while ensuring
QCOM_SCM is present on actual QCOM platforms. However, this would
require proper stubbing in the qcom_scm driver to avoid build failures during
compile testing.

Thanks
Komal

>
> {sigh}
>
> greg k-h

Re: [PATCH v7] usb: misc: qcom_eud: Access EUD_MODE_MANAGER2 through secure calls

[Dmitry Baryshkov]

On Fri, Jul 25, 2025 at 06:19:21PM +0530, Komal Bajaj wrote:
> On Thu, Jul 24, 2025 at 3:06 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Tue, Jul 22, 2025 at 05:01:53PM +0530, Komal Bajaj wrote:
> > > EUD_MODE_MANAGER2 register is mapped to a memory region that is marked
> > > as read-only for operating system running at EL1, 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>

[...]

> > > 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
> >
> > You now are preventing this code from ever being able to be built in any
> > testing systems, including mine, so I don't even know if this patch
> > builds or not.
> >
> > You did not even document this in the changelog :(
> 
> QCOM_SCM is essential for QCOM_EUD for its functionality.
> One option I'm considering is using select QCOM_SCM, which ensures
> dependency is enabled when QCOM_EUD is selected. QCOM_SCM facilitates
> communication with secure world, this approach should not cause issues even
> when COMPILE_TEST is enabled on non-ARCH_QCOM platforms.

QCOM_SCM is not user-selectable, to it is not correct to depend on it.
Have you had used `git grep`, you'd have seen that absolute majority of
the drivers uses `select QCOM_SCM`.

> Alternatively, I could use a conditional depends expression like:
> depends on (ARCH_QCOM && QCOM_SCM) || COMPILE_TEST
> 
> This would allow the driver to be built under COMPILE_TEST while ensuring
> QCOM_SCM is present on actual QCOM platforms. However, this would
> require proper stubbing in the qcom_scm driver to avoid build failures during
> compile testing.

-- 
With best wishes
Dmitry

Re: [PATCH v7] usb: misc: qcom_eud: Access EUD_MODE_MANAGER2 through secure calls

[Komal Bajaj]

On Fri, Jul 25, 2025 at 7:07 PM Dmitry Baryshkov
<dmitry.baryshkov@oss.qualcomm.com> wrote:
>
> On Fri, Jul 25, 2025 at 06:19:21PM +0530, Komal Bajaj wrote:
> > On Thu, Jul 24, 2025 at 3:06 PM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Tue, Jul 22, 2025 at 05:01:53PM +0530, Komal Bajaj wrote:
> > > > EUD_MODE_MANAGER2 register is mapped to a memory region that is marked
> > > > as read-only for operating system running at EL1, 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>
>
> [...]
>
> > > > 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
> > >
> > > You now are preventing this code from ever being able to be built in any
> > > testing systems, including mine, so I don't even know if this patch
> > > builds or not.
> > >
> > > You did not even document this in the changelog :(
> >
> > QCOM_SCM is essential for QCOM_EUD for its functionality.
> > One option I'm considering is using select QCOM_SCM, which ensures
> > dependency is enabled when QCOM_EUD is selected. QCOM_SCM facilitates
> > communication with secure world, this approach should not cause issues even
> > when COMPILE_TEST is enabled on non-ARCH_QCOM platforms.
>
> QCOM_SCM is not user-selectable, to it is not correct to depend on it.
> Have you had used `git grep`, you'd have seen that absolute majority of
> the drivers uses `select QCOM_SCM`.

I had initially used select QCOM_SCM in an earlier patch, but based on
the concern
raised about enabling it under COMPILE_TEST on non-ARCH_QCOM platforms,
I revised it to use a depends on condition.

>
> > Alternatively, I could use a conditional depends expression like:
> > depends on (ARCH_QCOM && QCOM_SCM) || COMPILE_TEST
> >
> > This would allow the driver to be built under COMPILE_TEST while ensuring
> > QCOM_SCM is present on actual QCOM platforms. However, this would
> > require proper stubbing in the qcom_scm driver to avoid build failures during
> > compile testing.
>
> --
> With best wishes
> Dmitry

Re: [PATCH v7] usb: misc: qcom_eud: Access EUD_MODE_MANAGER2 through secure calls

[Dmitry Baryshkov]

On 28/07/2025 13:19, Komal Bajaj wrote:
> On Fri, Jul 25, 2025 at 7:07 PM Dmitry Baryshkov
> <dmitry.baryshkov@oss.qualcomm.com> wrote:
>>
>> On Fri, Jul 25, 2025 at 06:19:21PM +0530, Komal Bajaj wrote:
>>> On Thu, Jul 24, 2025 at 3:06 PM Greg Kroah-Hartman
>>> <gregkh@linuxfoundation.org> wrote:
>>>>
>>>> On Tue, Jul 22, 2025 at 05:01:53PM +0530, Komal Bajaj wrote:
>>>>> EUD_MODE_MANAGER2 register is mapped to a memory region that is marked
>>>>> as read-only for operating system running at EL1, 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>
>>
>> [...]
>>
>>>>> 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
>>>>
>>>> You now are preventing this code from ever being able to be built in any
>>>> testing systems, including mine, so I don't even know if this patch
>>>> builds or not.
>>>>
>>>> You did not even document this in the changelog :(
>>>
>>> QCOM_SCM is essential for QCOM_EUD for its functionality.
>>> One option I'm considering is using select QCOM_SCM, which ensures
>>> dependency is enabled when QCOM_EUD is selected. QCOM_SCM facilitates
>>> communication with secure world, this approach should not cause issues even
>>> when COMPILE_TEST is enabled on non-ARCH_QCOM platforms.
>>
>> QCOM_SCM is not user-selectable, to it is not correct to depend on it.
>> Have you had used `git grep`, you'd have seen that absolute majority of
>> the drivers uses `select QCOM_SCM`.
> 
> I had initially used select QCOM_SCM in an earlier patch, but based on
> the concern
> raised about enabling it under COMPILE_TEST on non-ARCH_QCOM platforms,
> I revised it to use a depends on condition.

QCOM_SCM can be selected on non-QCOM platforms, so there should be no 
concerns for that.

> 
>>
>>> Alternatively, I could use a conditional depends expression like:
>>> depends on (ARCH_QCOM && QCOM_SCM) || COMPILE_TEST
>>>
>>> This would allow the driver to be built under COMPILE_TEST while ensuring
>>> QCOM_SCM is present on actual QCOM platforms. However, this would
>>> require proper stubbing in the qcom_scm driver to avoid build failures during
>>> compile testing.
>>
>> --
>> With best wishes
>> Dmitry


-- 
With best wishes
Dmitry

Re: [PATCH v7] usb: misc: qcom_eud: Access EUD_MODE_MANAGER2 through secure calls

[Komal Bajaj]

On Mon, Jul 28, 2025 at 4:00 PM Dmitry Baryshkov
<dmitry.baryshkov@oss.qualcomm.com> wrote:
>
> On 28/07/2025 13:19, Komal Bajaj wrote:
> > On Fri, Jul 25, 2025 at 7:07 PM Dmitry Baryshkov
> > <dmitry.baryshkov@oss.qualcomm.com> wrote:
> >>
> >> On Fri, Jul 25, 2025 at 06:19:21PM +0530, Komal Bajaj wrote:
> >>> On Thu, Jul 24, 2025 at 3:06 PM Greg Kroah-Hartman
> >>> <gregkh@linuxfoundation.org> wrote:
> >>>>
> >>>> On Tue, Jul 22, 2025 at 05:01:53PM +0530, Komal Bajaj wrote:
> >>>>> EUD_MODE_MANAGER2 register is mapped to a memory region that is marked
> >>>>> as read-only for operating system running at EL1, 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>
> >>
> >> [...]
> >>
> >>>>> 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
> >>>>
> >>>> You now are preventing this code from ever being able to be built in any
> >>>> testing systems, including mine, so I don't even know if this patch
> >>>> builds or not.
> >>>>
> >>>> You did not even document this in the changelog :(
> >>>
> >>> QCOM_SCM is essential for QCOM_EUD for its functionality.
> >>> One option I'm considering is using select QCOM_SCM, which ensures
> >>> dependency is enabled when QCOM_EUD is selected. QCOM_SCM facilitates
> >>> communication with secure world, this approach should not cause issues even
> >>> when COMPILE_TEST is enabled on non-ARCH_QCOM platforms.
> >>
> >> QCOM_SCM is not user-selectable, to it is not correct to depend on it.
> >> Have you had used `git grep`, you'd have seen that absolute majority of
> >> the drivers uses `select QCOM_SCM`.
> >
> > I had initially used select QCOM_SCM in an earlier patch, but based on
> > the concern
> > raised about enabling it under COMPILE_TEST on non-ARCH_QCOM platforms,
> > I revised it to use a depends on condition.
>
> QCOM_SCM can be selected on non-QCOM platforms, so there should be no
> concerns for that.

Understood. I will update the next patch to use select QCOM_SCM accordingly.

Thanks
Komal

>
> >
> >>
> >>> Alternatively, I could use a conditional depends expression like:
> >>> depends on (ARCH_QCOM && QCOM_SCM) || COMPILE_TEST
> >>>
> >>> This would allow the driver to be built under COMPILE_TEST while ensuring
> >>> QCOM_SCM is present on actual QCOM platforms. However, this would
> >>> require proper stubbing in the qcom_scm driver to avoid build failures during
> >>> compile testing.
> >>
> >> --
> >> With best wishes
> >> Dmitry
>
>
> --
> With best wishes
> Dmitry