On 07/05/18 20:28, Bjorn Andersson wrote:
On Fri, Mar 2, 2018 at 8:59 AM, Robin Murphy wrote:
On 02/03/18 14:55, srinivas.kandaga...@linaro.org wrote:
From: Srinivas Kandagatla
On Qualcomm SoCs, ADSP exposes many functions like audio and
others. These services need iommu access to allocate any
memory for the DSP. As these drivers are childeren of
rpmsg bus, able to allocate memory from iommus is basic
requirement. So set arm smmu iommu ops for this bus type.
Forgot to answer this and the dma_ops patch seems to be going in the
Documentation/rpmsg.txt: "Every rpmsg device is a communication channel with
a remote processor (thus rpmsg devices are called channels)."
I'd instinctively assume that a remote processor already has its own memory,
and that a communication channel doesn't somehow go directly through an
IOMMU, so that "basic requirement" seems like a pretty big assumption.
As of today rpmsg exclusively uses system memory for implementing the
communication fifos, but this memory is owned/handled by the rpmsg
bus. The need here is for drivers on top of the rpmsg_bus,
implementing some application-level protocol that requires indirection
buffers; e.g. to achieve zero copy of audio or image buffers that the
remote processor is expected to operate on. In this case the device
sitting on top of the rpmsg bus will have to map the buffer to the
appropriate context and can then send application specific control
requests referencing this mapping.
Right, but that's more or less what I was getting at - rpmsg can be used
as a means to signal some DMA master device to start doing a thing, but
that thing itself is unrelated to rpmsg, and it by no means implies that
everything which rpmsg can talk to is always capable of system-wide DMA.
It's no different if that communication channel is a hardware mailbox or
an I2C/SPI/USB/etc. link, rather than virtio; we wouldn't automatically
consider devices on the other end of those to be directly connected to
an IOMMU either.
IOMMU and DMA operations are highly dependent on the physical hardware
topology, which is why I really don't like trying to shoehorn them into
software constructs without modelling the actual hardware reasonably
accurately. For instance it's not unheard of for remote processors in a
SoC to see a different physical memory map from the main application
processors - how would rpmsg try to describe that? What even is the
address space of the rpmsg "bus"?
As different parts of the firmware might operate in different contexts
it's not feasible to utilize the parent's (the rpmsg_bus) context for
these indirection buffers.
Indeed, and I maintain that that wouldn't be the right thing to do
anyway. As before, I think the most accurate way to model the situation
with the tools we have available is to have the actual hardware function
represented by a platform device, which is associated with a
corresponding rpmsg endpoint. Then the driver can manage communication
in the rpmsg context, and physical DMA setup in the 'real' hardware
context, and everything looks sane without questionable abstraction
breakage. Since this looked to be more or less what is actually
implemented anyway, it doesn't seem all that hard to refine; if there
are multiple DMA master functions identified distinctly to the IOMMU,
then they could either be represented as separate platform devices with
explicit IOMMU specifiers, or you could model the actual DSP subsystem
hardware as its own bus-like arrangement with an iommu-map arrangement
translating function identifiers to IOMMU identifiers.
What I don't like is forcing IOMMU drivers to pretend that some data in
a shared memory buffer is itself directly capable of generating
transactions on the interconnect. If other 'indirect' bus abstractions
like CoreSight can get this right, I don't see why rpmsg deserves to be
Signed-off-by: Srinivas Kandagatla
drivers/iommu/arm-smmu.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index e6920d32ac9e..9b63489af15c 100644
@@ -53,6 +53,7 @@
@@ -2168,6 +2169,10 @@ static void arm_smmu_bus_init(void)
Ah, so this will at least build OK with RPMSG=m, but I doubt it does what
you want it to in that case.
Things have been refactored but the core has remained tristate,
causing extra head aches in various areas. I think it's very
reasonable to review the rpmsg config options and make CONFIG_RPMSG
So with the addition of making CONFIG_RPMSG bool the patch has my Acked-by.
That said I'm generally concerned about