On 2022-12-13 16:07, Manivannan Sadhasivam wrote:
On Tue, Dec 13, 2022 at 09:27:04PM +0530, Sibi Sankar wrote:
Hey Robin,
Thanks for taking time to review the series.
On 12/13/22 20:37, Robin Murphy wrote:
On 2022-12-13 14:07, Sibi Sankar wrote:
The memory region allocated using dma_alloc_attr with no kernel mapping
attribute set would still be a part of the linear kernel map. Any access
to this region by the application processor after assigning it to the
remote Q6 will result in a XPU violation. Fix this by replacing the
dynamically allocated memory region with a no-map carveout and unmap the
modem metadata memory region before passing control to the remote Q6.
Reported-by: Amit Pundir <amit.pundir@xxxxxxxxxx>
Fixes: 6c5a9dc2481b ("remoteproc: qcom: Make secure world call for
mem ownership switch")
Signed-off-by: Sibi Sankar <quic_sibis@xxxxxxxxxxx>
---
The addition of the carveout and memunmap is required only on SoCs that
mandate memory protection before transferring control to Q6, hence the
driver falls back to dynamic memory allocation in the absence of the
modem metadata carveout.
The DMA_ATTR_NO_KERNEL_MAPPING stuff is still broken and pointless, so
I'd expect to see this solution replacing it, not being added alongside.
It's just silly to say pass the "I don't need a CPU mapping" flag, then
manually open-code the same CPU mapping you would have got if you
hadn't, in a way that only works at all when a cacheable alias exists
anyway.
only a subset of SoCs supported by the driver are affected by
the bug i.e. on the others dma_alloc_attr would still work
without problems. I can perhaps drop the NO_KERNEL_MAPPING along
with the vmap/vunmap and simplify things for those SoCs.
Or perhaps revert fc156629b23a?
Oh, indeed, if it's already self-contained that's even neater. Basically
that whole commit is based on a misunderstanding, doesn't actually do
what it thinks it does, and you'd be far better off not maintaining the
extra code.
Thanks,
Robin.
