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).
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
