On Fri, Nov 18, 2016, 13:33 Stanislav Kholmanskikh <
stanislav.kholmanskikh@oracle.com> wrote:

> Hi, Daniel.
>
> Thank you for review.
>
> My comments are below.
>
> On 11/16/2016 12:22 AM, Daniel Kiper wrote:
> > On Tue, Apr 12, 2016 at 03:39:55PM +0300, Stanislav Kholmanskikh wrote:
> >> Add wrappers for memory allocation using
> >> alloc-mem and free-mem commands from the User Interface.
> >
> > Please tell why it is needed. Additionally, please forgive me if it is
> stupid
> > question, why are you using command line to allocate/free memory? There
> is
> > a lack of better API in IEEE 1275?
>
> In the current git code in grub-core/net/drivers/ieee1275/ofnet.c the
> search_net_devices() function uses the "alloc-mem" command for
> allocation of the transmit buffer for the case when
> GRUB_IEEE1275_FLAG_VIRT_TO_REAL_BROKEN is set.
>
> Yes, besides "alloc-mem", "free-mem", there are other options for
> allocating memory. But, to be honest, I don't know why grub_malloc()
> cannot be used for the receive buffer on those systems. From the flag's
> name it seems those systems don't have a MMU. Ok, but grub_malloc() is
> called from many places in grub, and, presumably, grub works on those
> systems. I don't have such a MacRISC system to try grub_malloc() for the
> buffers (grub-core/kern/ieee1275/cmain.c).
>
On my test system if I try to use grub_malloc, then the network driver
tries to do some dubious pointer arithmetic and puts data in a completely
wrong location unless alloc-mem was used

>
> In patch 2 I'm implementing a receive buffer, so I decided to keep this
> case as is, but move "alloc-mem", "free-mem" into functions.
>
> Does it answer your questions? If yes, I'll update the commit message
> for V2 with a more detailed explanation why these functions are needed.
>
> >
> >> Signed-off-by: Stanislav Kholmanskikh <
> stanislav.kholmanskikh@oracle.com>
> >> ---
> >>  grub-core/kern/ieee1275/openfw.c |   68
> ++++++++++++++++++++++++++++++++++++++
> >>  include/grub/ieee1275/ieee1275.h |    2 +
> >>  2 files changed, 70 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/grub-core/kern/ieee1275/openfw.c
> b/grub-core/kern/ieee1275/openfw.c
> >> index ddb7783..35225ec 100644
> >> --- a/grub-core/kern/ieee1275/openfw.c
> >> +++ b/grub-core/kern/ieee1275/openfw.c
> >> @@ -561,3 +561,71 @@ grub_ieee1275_canonicalise_devname (const char
> *path)
> >>    return NULL;
> >>  }
> >>
> >> +/* Allocate memory with alloc-mem */
> >> +void *
> >> +grub_ieee1275_alloc_mem (grub_size_t len)
> >> +{
> >> +  struct alloc_args
> >> +  {
> >> +    struct grub_ieee1275_common_hdr common;
> >> +    grub_ieee1275_cell_t method;
> >> +    grub_ieee1275_cell_t len;
> >> +    grub_ieee1275_cell_t catch;
> >> +    grub_ieee1275_cell_t result;
> >> +  }
> >> +  args;
> >> +
> >> +  if (grub_ieee1275_test_flag (GRUB_IEEE1275_FLAG_CANNOT_INTERPRET))
> >> +    {
> >> +      grub_error (GRUB_ERR_UNKNOWN_COMMAND, N_("interpret is not
> supported"));
> >> +      return NULL;
> >> +    }
> >> +
> >> +  INIT_IEEE1275_COMMON (&args.common, "interpret", 2, 2);
> >> +  args.len = len;
> >> +  args.method = (grub_ieee1275_cell_t) "alloc-mem";
> >> +
> >> +  if (IEEE1275_CALL_ENTRY_FN (&args) == -1
> >> +      || args.catch)
> >
> > I think that this can be in one line.
>
> Ok.
>
> >
> >> +    {
> >> +      grub_error (GRUB_ERR_INVALID_COMMAND, N_("alloc-mem failed"));
> >> +      return NULL;
> >> +    }
> >> +  else
> >> +    return (void *)args.result;
> >> +}
> >> +
> >> +/* Free memory allocated by alloc-mem */
> >> +grub_err_t
> >> +grub_ieee1275_free_mem (void *addr, grub_size_t len)
> >> +{
> >> +  struct free_args
> >> +  {
> >> +    struct grub_ieee1275_common_hdr common;
> >> +    grub_ieee1275_cell_t method;
> >> +    grub_ieee1275_cell_t len;
> >> +    grub_ieee1275_cell_t addr;
> >> +    grub_ieee1275_cell_t catch;
> >> +  }
> >> +  args;
> >> +
> >> +  if (grub_ieee1275_test_flag (GRUB_IEEE1275_FLAG_CANNOT_INTERPRET))
> >> +    {
> >> +      grub_error (GRUB_ERR_UNKNOWN_COMMAND, N_("interpret is not
> supported"));
> >> +      return grub_errno;
> >> +    }
> >> +
> >> +  INIT_IEEE1275_COMMON (&args.common, "interpret", 3, 1);
> >> +  args.addr = (grub_ieee1275_cell_t)addr;
> >> +  args.len = len;
> >> +  args.method = (grub_ieee1275_cell_t) "free-mem";
> >> +
> >> +  if (IEEE1275_CALL_ENTRY_FN(&args) == -1
> >> +      || args.catch)
> >
> > Ditto.
>
> Ok.
>
> >
> > Daniel
> >
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> >
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>