[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <fo4hek5twtgl7fa5ncqefatowz5nw64iwifaibl3yyzonucpqz@uwwxf5jdke2x>
Date: Fri, 25 Jul 2025 16:37:37 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Komal Bajaj <komal.bajaj@....qualcomm.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Souradeep Chowdhury <quic_schowdhu@...cinc.com>,
linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-msm@...r.kernel.org,
Melody Olvera <quic_molvera@...cinc.com>,
Konrad Dybcio <konrad.dybcio@....qualcomm.com>
Subject: Re: [PATCH v7] usb: misc: qcom_eud: Access EUD_MODE_MANAGER2 through
secure calls
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@...uxfoundation.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@...cinc.com>
> > > Signed-off-by: Komal Bajaj <komal.bajaj@....qualcomm.com>
> > > Reviewed-by: Konrad Dybcio <konrad.dybcio@....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
Powered by blists - more mailing lists