Messages in this thread | | | Date | Mon, 8 Apr 2024 16:30:26 +0200 | Subject | Re: [PATCH v2] firmware: qcom: uefisecapp: Fix memory related IO errors and crashes | From | Konrad Dybcio <> |
| |
On 4/6/24 15:01, Maximilian Luz wrote: > It turns out that while the QSEECOM APP_SEND command has specific fields > for request and response buffers, uefisecapp expects them both to be in > a single memory region. Failure to adhere to this has (so far) resulted > in either no response being written to the response buffer (causing an > EIO to be emitted down the line), the SCM call to fail with EINVAL > (i.e., directly from TZ/firmware), or the device to be hard-reset. > > While this issue can be triggered deterministically, in the current form > it seems to happen rather sporadically (which is why it has gone > unnoticed during earlier testing). This is likely due to the two > kzalloc() calls (for request and response) being directly after each > other. Which means that those likely return consecutive regions most of > the time, especially when not much else is going on in the system. > > Fix this by allocating a single memory region for both request and > response buffers, properly aligning both structs inside it. This > unfortunately also means that the qcom_scm_qseecom_app_send() interface > needs to be restructured, as it should no longer map the DMA regions > separately. Therefore, move the responsibility of DMA allocation (or > mapping) to the caller. > > Fixes: 759e7a2b62eb ("firmware: Add support for Qualcomm UEFI Secure Application") > Cc: stable@vger.kernel.org # 6.7 > Tested-by: Johan Hovold <johan+linaro@kernel.org> > Reviewed-by: Johan Hovold <johan+linaro@kernel.org> > Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com> > --- > > Changes in v2: > - rename DMA related variables > - replace _phys suffix with _dma > - drop _virt suffix > - use DMA-based naming in comments instead of referring to physical > addresses/memory > > ---
Tested-by: Konrad Dybcio <konrad.dybcio@linaro.org> # X13s
I've been running this for quite some time now, no explosions so far
Konrad
| |