Hi, On Fri, May 2, 2025 at 3:36 PM Robin Murphy <robin.mur...@arm.com> wrote: > > On 02/05/2025 10:59 am, Jens Wiklander wrote: > > If a parent device is supplied to tee_device_alloc(), copy the dma_mask > > field into the new device. This avoids future warnings when mapping a > > DMA-buf for the device. > > That also sounds dodgy. If the parent device is the hardware device > physically performing the DMA, then that is the device which should be > passed to the DMA API. Trying to copy random bits of one device's > configuration to another device and hoping it will work is not robust - > not only is DMA-relevant information all over the place, including in > archdata and/or bus/IOMMU driver-private data, but it can also opens up > a whole can of subtle lifecycle issues...
We have a reference to the parent device until the teedev goes away. The dma_maks needed by tee_shm_register_fd() in https://lore.kernel.org/lkml/20250502100049.1746335-9-jens.wiklan...@linaro.org/ to be able to extract the PA from a DMA-buf allocated from another DMA heap. We can drop this patch and support for unrelated DMA heaps in tee_shm_register_fd() without losing critical features from the patch set if we can't handle dma_mask in this way. > > > Signed-off-by: Jens Wiklander <jens.wiklan...@linaro.org> > > Reviewed-by: Sumit Garg <sumit.g...@kernel.org> > > --- > > drivers/tee/tee_core.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c > > index d113679b1e2d..685afcaa3ea1 100644 > > --- a/drivers/tee/tee_core.c > > +++ b/drivers/tee/tee_core.c > > @@ -922,6 +922,8 @@ struct tee_device *tee_device_alloc(const struct > > tee_desc *teedesc, > > teedev->dev.class = &tee_class; > > teedev->dev.release = tee_release_device; > > teedev->dev.parent = dev; > > + if (dev) > > + teedev->dev.dma_mask = dev->dma_mask; > > ...for instance, I don't see any obvious guarantee that "dev" can't go > away during the lifetime of "teedev" and leave this pointer dangling. A successful call to tee_device_alloc() must be followed by a call to tee_device_register() or tee_device_unregister(). The former calls cdev_device_add(), which results in a call to device_add() and an increased reference to teedev->dev.parent, "dev" in question. Thanks, Jens > > Thanks, > Robin. > > > > > teedev->dev.devt = MKDEV(MAJOR(tee_devt), teedev->id); > >
