On Fri, Nov 18, 2016, 13:33 Stanislav Kholmanskikh <
stanislav.kholmansk...@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.kholmansk...@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
>
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
