Hej, On Tue, Oct 29, 2024 at 12:48:46PM +0530, Sughosh Ganu wrote: > The LMB module is typically used for managing the platform's RAM > memory. RAM memory gets added to the module's memory map, and > subsequently allocated through various LMB API's. > > The IOMMU driver for the apple platforms also uses the LMB API's for > allocating device virtual addresses(VA). These addresses are different > from the RAM addresses, and cannot be added to the RAM memory map. Add > a separate instance of LMB memory map for the device VA's, which gets > managed by the IOMMU driver. Use lmb_push() and lmb_pop() functions to > set-up the relevant lmb instance.
thanks, this fixes the initial brokenness when setting up dma mappings but I still see SError due to translation fault. I don't have time right now to look into it so it could be unrelated to the iommu. > Signed-off-by: Sughosh Ganu <[email protected]> > --- > drivers/iommu/apple_dart.c | 67 +++++++++++++++++++++++++++++++++++++- > include/lmb.h | 2 ++ > lib/lmb.c | 1 - > 3 files changed, 68 insertions(+), 2 deletions(-) > > diff --git a/drivers/iommu/apple_dart.c b/drivers/iommu/apple_dart.c > index 611ac7cd6de..55f60287b0e 100644 > --- a/drivers/iommu/apple_dart.c > +++ b/drivers/iommu/apple_dart.c > @@ -3,6 +3,7 @@ > * Copyright (C) 2021 Mark Kettenis <[email protected]> > */ > > +#include <alist.h> > #include <cpu_func.h> > #include <dm.h> > #include <iommu.h> > @@ -70,6 +71,8 @@ > > struct apple_dart_priv { > void *base; > + struct lmb apple_dart_lmb; > + struct lmb ram_lmb; > u64 *l1, *l2; > int bypass, shift; > > @@ -87,6 +90,56 @@ struct apple_dart_priv { > void (*flush_tlb)(struct apple_dart_priv *priv); > }; > > +static int apple_dart_lmb_init(struct apple_dart_priv *priv) > +{ > + bool ret; > + struct lmb *almb = &priv->apple_dart_lmb; > + > + ret = alist_init(&almb->free_mem, sizeof(struct lmb_region), > + (uint)LMB_ALIST_INITIAL_SIZE); > + if (!ret) { > + log_debug("Unable to initialise the list for LMB free > memory\n"); > + return -ENOMEM; > + } > + > + ret = alist_init(&almb->used_mem, sizeof(struct lmb_region), > + (uint)LMB_ALIST_INITIAL_SIZE); > + if (!ret) { > + log_debug("Unable to initialise the list for LMB used > memory\n"); > + return -ENOMEM; > + } > + > + return 0; > +} > + > +static void apple_dart_lmb_uninit(struct apple_dart_priv *priv) > +{ > + struct lmb *almb = &priv->apple_dart_lmb; > + > + alist_uninit(&almb->free_mem); > + alist_uninit(&almb->used_mem); > +} > + > +static void apple_dart_lmb_setup(struct apple_dart_priv *priv) > +{ > + struct lmb *almb = &priv->apple_dart_lmb; > + struct lmb *rlmb = &priv->ram_lmb; > + > + lmb_push(rlmb); > + > + lmb_pop(almb); This doesn't look look like a good solution to me. We're conflating two different concepts into the global lmb. This looks error prone to me. I was looking into adding iomb_* entry points into the lmb points which take a pointer. Janne

