On Wed, Nov 06, 2024 at 12:48:45AM +0530, Sughosh Ganu wrote:
> On Fri, 1 Nov 2024 at 15:47, Janne Grunau <j...@jannau.net> wrote:
> >
> > These functions can be used with struct lmb pointers and will be used to
> > manage IOVA space in the apple_dart iommu driver. This restores part of
> > the pointer base struct lmb API from before commit ed17a33fed29 ("lmb:
> > make LMB memory map persistent and global").
> > io_lmb_add() and io_lmb_free() can trivially reuse exisiting lmb
> > functions. io_lmb_setup() is separate for unique error log messages.
> > io_lmb_alloc() is a simplified copy of _lmb_alloc_base() since the
> > later has unused features and internal use of the global LMB memory map.
> >
> > Signed-off-by: Janne Grunau <j...@jannau.net>
> > ---
> > include/lmb.h | 47 +++++++++++++++++++++++++++++++++++++
> > lib/lmb.c | 74
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 121 insertions(+)
> >
> > diff --git a/include/lmb.h b/include/lmb.h
> > index
> > 2201d6f2b67bb605dbff015fa2a6a008b780c57a..9cb3ba8289e10ab92bea135d248580e44880e3e2
> > 100644
> > --- a/include/lmb.h
> > +++ b/include/lmb.h
> > @@ -156,6 +156,53 @@ static inline int lmb_read_check(phys_addr_t addr,
> > phys_size_t len)
> > return lmb_alloc_addr(addr, len) == addr ? 0 : -1;
> > }
> >
> > +/**
> > + * io_lmb_setup() - Initialize LMB struct
> > + * @lmb: IO LMB to initialize
> > + *
> > + * Add the IOVA space [base, base + size] to be managed by io_lmb.
> > + *
> > + * Returns: 0 on success, negative error code on failure
> > + */
> > +int io_lmb_setup(struct lmb *io_lmb);
> > +
> > +/**
> > + * io_lmb_add() - Add an IOVA range for allocations
> > + * @io_lmb: LMB to add the space to
> > + * @base: Base Address of region to add
> > + * @size: Size of the region to add
> > + *
> > + * Add the IOVA space [base, base + size] to be managed by io_lmb.
> > + *
> > + * Returns: 0 if the region addition was successful, -1 on failure
> > + */
> > +long io_lmb_add(struct lmb *io_lmb, phys_addr_t base, phys_size_t size);
> > +
> > +/**
> > + * io_lmb_alloc() - Allocate specified IO memory address with specified
> > alignment
> > + * @io_lmb: LMB to alloc from
> > + * @size: Size of the region requested
> > + * @align: Required address and size alignment
> > + *
> > + * Allocate a region of IO memory. The base parameter is used to specify
> > the
> > + * base address of the requested region.
> > + *
> > + * Return: base IO address on success, 0 on error
> > + */
> > +phys_addr_t io_lmb_alloc(struct lmb *io_lmb, phys_size_t size, ulong
> > align);
> > +
> > +/**
> > + * io_lmb_free() - Free up a region of IOVA space
> > + * @io_lmb: LMB to return the IO address space to
> > + * @base: Base Address of region to be freed
> > + * @size: Size of the region to be freed
> > + *
> > + * Free up a region of IOVA space.
> > + *
> > + * Return: 0 if successful, -1 on failure
> > + */
> > +long io_lmb_free(struct lmb *io_lmb, phys_addr_t base, phys_size_t size);
> > +
> > #endif /* __KERNEL__ */
> >
> > #endif /* _LINUX_LMB_H */
> > diff --git a/lib/lmb.c b/lib/lmb.c
> > index
> > 2ce91f7521cfe58577b092fdaea8004b17f0eeff..cbf1ee0df93a9cb0cfea571a4caebae09646595e
> > 100644
> > --- a/lib/lmb.c
> > +++ b/lib/lmb.c
> > @@ -351,6 +351,80 @@ static phys_addr_t lmb_align_down(phys_addr_t addr,
> > phys_size_t size)
> > return addr & ~(size - 1);
> > }
> >
> > +/*
> > + * IOVA LMB memory maps using lmb pointers instead of the global LMB
> > memory map.
> > + */
> > +
> > +int io_lmb_setup(struct lmb *io_lmb)
> > +{
> > + int ret;
> > +
> > + ret = alist_init(&io_lmb->free_mem, sizeof(struct lmb_region),
> > + (uint)LMB_ALIST_INITIAL_SIZE);
> > + if (!ret) {
> > + log_debug("Unable to initialise the list for LMB free
> > IOVA\n");
> > + return -ENOMEM;
> > + }
> > +
> > + ret = alist_init(&io_lmb->used_mem, sizeof(struct lmb_region),
> > + (uint)LMB_ALIST_INITIAL_SIZE);
> > + if (!ret) {
> > + log_debug("Unable to initialise the list for LMB used
> > IOVA\n");
> > + return -ENOMEM;
> > + }
> > +
> > + io_lmb->test = false;
> > +
> > + return 0;
> > +}
>
> In the current use case, the above data structure is getting allocated
> as part of the driver's probe function. The resource should be free'd
> from the driver's remove function -- that should happen through a
> corresponding alist_uninit() call.
yes, I was mislead by that not happening for the global lmb. I'll add a
io_lmb_teardown() and use it in the remove callback.
I'll send a v2 with above change once it's clear that this is an
acceptable approach. See my reply to [Patch 0/4].
Janne
