On Tue, 29 Oct 2024 at 02:50, Janne Grunau <[email protected]> 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 <[email protected]> > > Signed-off-by: Simon Glass <[email protected]> > > [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/[email protected]/ -sughosh > > Not sure about the same change in sandbox_iommu but I guess it could be > ok as there is no sandbox. > > Janne
