On Fri, 2 May 2025 at 12:21, Ilias Apalodimas
<ilias.apalodi...@linaro.org> wrote:
>
> Hi Sughosh,
>
> Thanks for cleaning this
>
> [...]
>
> > + * @flags: Memory region attributes to be set
> > + *
> > + * Allocate a region of memory where the allocation is based on the
> > parameters
> > + * that have been passed to the function.The first parameter specifies the
> > + * type of allocation that is being requested. The align parameter is used
> > + * to specify if the allocation is to be made with a particular alignment.
> > + * It is 0 if there is no alignment requirement. The addr parameter is used
>
> Use 0 for no alignment requirements
Okay. Will change.
>
> > + * to return the base address of the allocated region. Depending on the
> > type
> > + * of allocation request, it might also contain a particular address being
> > + * requested, or the maximum address of the requested allocation. The flags
> > + * parameter is used to specify the memory attributes of the requested
> > region.
> > + *
> > + * Return: 0 on success, -ve value on failure
> > + *
> > + * When the allocation is of type LMB_MEM_ALLOC_ADDR, the return value can
> > + * be -EINVAL if the requested memory region is not part of the LMB memory
> > + * map, and -EEXIST if the requested region is already allocated.
> > + */
> > +int lmb_allocate_mem(enum lmb_mem_type type, u64 align, phys_addr_t *addr,
> > + phys_size_t size, u32 flags);
> > +
>
> [...]
>
> > +}
> > +
> > +int lmb_allocate_mem(enum lmb_mem_type type, u64 align, phys_addr_t *addr,
> > + phys_size_t size, u32 flags)
> > +{
> > + int ret = -1;
> > +
> > + if (!size)
> > + return 0;
>
> Shouldn't this be an error as well?
So I had this return an -EINVAL in my original set of changes.
However, I hit a corner case issue with my testing where a tftp
transfer of a payload which happens to be a multiple of the tftp block
size, and that results in the final receive loop having a length of 0.
Jerome confirmed that this is indeed a valid scenario with tftp. So
this can be an issue with any kind of loop based reservation requests,
where the final iteration of the loop might present a size of 0.
Moreover, I checked the implementation of other allocation functions
(including efi_allocate_pages()), and they do not return an error if
the size passed to the function is 0. Hence this behaviour.
-sughosh
>
> > +
> > + if (!addr)
> > + return -EINVAL;
> > +
> > + switch (type) {
> > + case LMB_MEM_ALLOC_ADDR:
> > + ret = _lmb_alloc_addr(*addr, size, flags);
> > + break;
> > + default:
> > + log_debug("%s: Invalid memory allocation type requested %d\n",
> > + __func__, type);
> > }
> >
> > - return -1;
> > + return ret;
> > }
> >
> > /* Return number of bytes from a given address that are free */
> > diff --git a/test/lib/lmb.c b/test/lib/lmb.c
> > index 3bf558f7f4f..f80115570e7 100644
> > --- a/test/lib/lmb.c
> > +++ b/test/lib/lmb.c
> > @@ -71,6 +71,19 @@ static int setup_lmb_test(struct unit_test_state *uts,
> > struct lmb *store,
> > return 0;
> > }
> >
>
> [...]
>
> Thanks
> /Ilias
