On Tue, 29 Oct 2024 at 10:02, Sughosh Ganu <sughosh.g...@linaro.org> wrote:
>
> On Tue, 29 Oct 2024 at 09:19, Sughosh Ganu <sughosh.g...@linaro.org> wrote:
> >
> > On Tue, 29 Oct 2024 at 02:50, Janne Grunau <ja...@jannau.net> wrote:
> > >
> > > On Mon, Aug 26, 2024 at 05:29:18PM +0530, Sughosh Ganu wrote:
> > > > The current LMB API's for allocating and reserving memory use a
> > > > per-caller based memory view. Memory allocated by a caller can then be
> > > > overwritten by another caller. Make these allocations and reservations
> > > > persistent using the alloced list data structure.
> > > >
> > > > Two alloced lists are declared -- one for the available(free) memory,
> > > > and one for the used memory. Once full, the list can then be extended
> > > > at runtime.
> > > >
> > > > Signed-off-by: Sughosh Ganu <sughosh.g...@linaro.org>
> > > > Signed-off-by: Simon Glass <s...@chromium.org>
> > > > [sjg: Optimise the logic to add a region in lmb_add_region_flags()]
> > > > [sjg: Use a stack to store pointer of lmb struct when running lmb tests]
> > > > ---
> > > > Changes since V3:
> > > > * Fix checkpatch warnings of spaces between function name and
> > > > open parantheses.
> > > > * s/uint64_t/u64 as suggested by checkpatch.
> > > > * Remove unneccessary parantheses in lmb.c as suggested by checkpatch.
> > > > * Fix alignment in test/cmd/bdinfo.c as suggested by checkpatch.
> > >
> > > [...]
> > >
> > > > drivers/iommu/apple_dart.c | 8 +-
> > >
> > > [...]
> > >
> > > > diff --git a/drivers/iommu/apple_dart.c b/drivers/iommu/apple_dart.c
> > > > index 9327dea1e3..611ac7cd6d 100644
> > > > --- a/drivers/iommu/apple_dart.c
> > > > +++ b/drivers/iommu/apple_dart.c
> > > > @@ -70,7 +70,6 @@
> > > >
> > > > struct apple_dart_priv {
> > > > void *base;
> > > > - struct lmb lmb;
> > > > u64 *l1, *l2;
> > > > int bypass, shift;
> > > >
> > > > @@ -124,7 +123,7 @@ static dma_addr_t apple_dart_map(struct udevice
> > > > *dev, void *addr, size_t size)
> > > > off = (phys_addr_t)addr - paddr;
> > > > psize = ALIGN(size + off, DART_PAGE_SIZE);
> > > >
> > > > - dva = lmb_alloc(&priv->lmb, psize, DART_PAGE_SIZE);
> > > > + dva = lmb_alloc(psize, DART_PAGE_SIZE);
> > > >
> > > > idx = dva / DART_PAGE_SIZE;
> > > > for (i = 0; i < psize / DART_PAGE_SIZE; i++) {
> > > > @@ -160,7 +159,7 @@ static void apple_dart_unmap(struct udevice *dev,
> > > > dma_addr_t addr, size_t size)
> > > > (unsigned long)&priv->l2[idx + i]);
> > > > priv->flush_tlb(priv);
> > > >
> > > > - lmb_free(&priv->lmb, dva, psize);
> > > > + lmb_free(dva, psize);
> > > > }
> > > >
> > > > static struct iommu_ops apple_dart_ops = {
> > > > @@ -213,8 +212,7 @@ static int apple_dart_probe(struct udevice *dev)
> > > > priv->dvabase = DART_PAGE_SIZE;
> > > > priv->dvaend = SZ_4G - DART_PAGE_SIZE;
> > > >
> > > > - lmb_init(&priv->lmb);
> > > > - lmb_add(&priv->lmb, priv->dvabase, priv->dvaend - priv->dvabase);
> > > > + lmb_add(priv->dvabase, priv->dvaend - priv->dvabase);
> > >
> > > no. This breaks everything. apple_dart is an iommu and used struct lmb
> > > to manage the IO virtual address space. This has nothing to do
> > > with system memory.
> > > Depending on the allocation strategy this will cause all sorts of
> > > problems since the IO and the physical memory address space does not
> > > overlap on apple silicon devices. IOVA is for many devices only 32-bit
> > > but physical memory starts at 0x10_0000_0000 or 0x100_0000_0000.
> > > Every device has its own iommu / IOVA space so sharing the space between
> > > all of them is another problem. I'd assume only theoretical due to the
> > > limited driver support and memory use in u-boot.
> > >
> > > My current plan to fix this is to resurrect the old lmb code under a
> > > different name.
> >
> > Firstly, apologies for breaking the driver. Looking at the code, it
> > looks like the driver works with a dva address range in the first 4GB.
> > I think what the driver expects is fairly straightforward, with a
> > top-down allocation scheme. I have sent a patch [1] which replaces the
> > LMB calls with a call to the allocation function. Can you please check
> > if this works on the board? Looking at how the driver is obtaining the
> > dva addresses, I think this logic should suffice. Thanks.
> >
> > -sughosh
> >
> > [1] -
> > https://patchwork.ozlabs.org/project/uboot/patch/20241029033915.423211-1-sughosh.g...@linaro.org/
>
> This would not work as it does not consider freeing of the va
> addresses. So there might be a need for a parallel lmb instance for
> this.
>
> -sughosh
>
> >
> >
> > -sughosh
> >
> > >
> > > Not sure about the same change in sandbox_iommu but I guess it could be
> > > ok as there is no sandbox.
I have pushed the changes for the driver to one of my github branch
[1]. Can you please try it out and let me know. Thanks.
-sughosh
[1] - https://github.com/sughoshg/u-boot/tree/apple_dart_changes
> > >
> > > Janne
