[U-Boot] [PATCH v3 1/7] efi_loader: Add network access support

Simon Glass sjg at chromium.org
Wed May 18 19:21:33 CEST 2016


+Tom

Hi Alex,

On 16 May 2016 at 12:06, Alexander Graf <agraf at suse.de> wrote:
>
>
> On 16.05.16 15:24, Simon Glass wrote:
>> Hi Alexander,
>>
>> On 14 May 2016 at 14:34, Alexander Graf <agraf at suse.de> wrote:
>>>
>>>
>>>> Am 14.05.2016 um 21:49 schrieb Simon Glass <sjg at chromium.org>:
>>>>
>>>> Hi Alexander,
>>>>
>>>>> On 10 May 2016 at 15:25, Alexander Graf <agraf at suse.de> wrote:
>>>>> We can now successfully boot EFI applications from disk, but users
>>>>> may want to also run them from a PXE setup.
>>>>>
>>>>> This patch implements rudimentary network support, allowing a payload
>>>>> to send and receive network packets.
>>>>>
>>>>> With this patch, I was able to successfully run grub2 with network
>>>>> access inside of QEMU's -M xlnx-ep108 and on a zcu102 system.
>>>>>
>>>>> Signed-off-by: Alexander Graf <agraf at suse.de>
>>>>>
>>>>> ---
>>>>>
>>>>> v2 -> v3:
>>>>>
>>>>>  - Align initialization sequence with net code
>>>>>  - Set device to initialized after init call
>>>>>  - Align tx buffers to DMA alignment (rx gets memcpy'd)
>>>>>  - Add comment about eth_rx()
>>>>> ---
>>>>> cmd/bootefi.c            |   7 ++
>>>>> include/efi_api.h        | 119 ++++++++++++++++++
>>>>> include/efi_loader.h     |   7 ++
>>>>> include/net.h            |   2 +-
>>>>> lib/efi_loader/Makefile  |   1 +
>>>>> lib/efi_loader/efi_net.c | 314 +++++++++++++++++++++++++++++++++++++++++++++++
>>>>> net/bootp.c              |   2 +
>>>>> net/net.c                |   4 +-
>>>>> net/tftp.c               |   2 +
>>>>> 9 files changed, 455 insertions(+), 3 deletions(-)
>>>>> create mode 100644 lib/efi_loader/efi_net.c
>>>>>
>>>>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>>>>> index 7f552fc..d3a2331 100644
>>>>> --- a/cmd/bootefi.c
>>>>> +++ b/cmd/bootefi.c
>>>>> @@ -197,6 +197,13 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt)
>>>>> #ifdef CONFIG_LCD
>>>>>        efi_gop_register();
>>>>> #endif
>>>>> +#ifdef CONFIG_NET
>>>>> +       void *nethandle = loaded_image_info.device_handle;
>>>>> +       efi_net_register(&nethandle);
>>>>> +
>>>>> +       if (!memcmp(bootefi_device_path[0].str, "N\0e\0t", 6))
>>>>> +               loaded_image_info.device_handle = nethandle;
>>>>> +#endif
>>>>>
>>>>>        /* Call our payload! */
>>>>> #ifdef DEBUG_EFI
>>>>> diff --git a/include/efi_api.h b/include/efi_api.h
>>>>> index 51d7586..20035d7 100644
>>>>> --- a/include/efi_api.h
>>>>> +++ b/include/efi_api.h
>>>>> @@ -412,4 +412,123 @@ struct efi_gop
>>>>>        struct efi_gop_mode *mode;
>>>>> };
>>>>>
>>>>> +#define EFI_SIMPLE_NETWORK_GUID \
>>>>> +       EFI_GUID(0xa19832b9, 0xac25, 0x11d3, \
>>>>> +                0x9a, 0x2d, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
>>>>> +
>>>>> +struct efi_mac_address {
>>>>> +       char mac_addr[32];
>>>>> +};
>>>>> +
>>>>> +struct efi_ip_address {
>>>>> +       u8 ip_addr[16];
>>>>> +};
>>>>> +
>>>>> +enum efi_simple_network_state {
>>>>> +       EFI_NETWORK_STOPPED,
>>>>> +       EFI_NETWORK_STARTED,
>>>>> +       EFI_NETWORK_INITIALIZED,
>>>>> +};
>>>>> +
>>>>> +struct efi_simple_network_mode {
>>>>> +       enum efi_simple_network_state state;
>>>>> +       u32 hwaddr_size;
>>>>> +       u32 media_header_size;
>>>>> +       u32 max_packet_size;
>>>>> +       u32 nvram_size;
>>>>> +       u32 nvram_access_size;
>>>>> +       u32 receive_filter_mask;
>>>>> +       u32 receive_filter_setting;
>>>>> +       u32 max_mcast_filter_count;
>>>>> +       u32 mcast_filter_count;
>>>>> +       struct efi_mac_address mcast_filter[16];
>>>>> +       struct efi_mac_address current_address;
>>>>> +       struct efi_mac_address broadcast_address;
>>>>> +       struct efi_mac_address permanent_address;
>>>>> +       u8 if_type;
>>>>> +       u8 mac_changeable;
>>>>> +       u8 multitx_supported;
>>>>> +       u8 media_present_supported;
>>>>> +       u8 media_present;
>>>>> +};
>>>>> +
>>>>> +#define EFI_SIMPLE_NETWORK_RECEIVE_UNICAST 0x01,
>>>>> +#define EFI_SIMPLE_NETWORK_RECEIVE_MULTICAST             0x02,
>>>>> +#define EFI_SIMPLE_NETWORK_RECEIVE_BROADCAST             0x04,
>>>>> +#define EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS           0x08,
>>>>> +#define EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS_MULTICAST 0x10,
>>>>> +
>>>>> +struct efi_simple_network
>>>>> +{
>>>>> +       u64 revision;
>>>>> +       efi_status_t (EFIAPI *start)(struct efi_simple_network *this);
>>>>> +       efi_status_t (EFIAPI *stop)(struct efi_simple_network *this);
>>>>> +       efi_status_t (EFIAPI *initialize)(struct efi_simple_network *this,
>>>>> +                       ulong extra_rx, ulong extra_tx);
>>>>> +       efi_status_t (EFIAPI *reset)(struct efi_simple_network *this,
>>>>> +                       int extended_verification);
>>>>> +       efi_status_t (EFIAPI *shutdown)(struct efi_simple_network *this);
>>>>> +       efi_status_t (EFIAPI *receive_filters)(struct efi_simple_network *this,
>>>>> +                       u32 enable, u32 disable, int reset_mcast_filter,
>>>>> +                       ulong mcast_filter_count,
>>>>> +                       struct efi_mac_address *mcast_filter);
>>>>> +       efi_status_t (EFIAPI *station_address)(struct efi_simple_network *this,
>>>>> +                       int reset, struct efi_mac_address *new_mac);
>>>>> +       efi_status_t (EFIAPI *statistics)(struct efi_simple_network *this,
>>>>> +                       int reset, ulong *stat_size, void *stat_table);
>>>>> +       efi_status_t (EFIAPI *mcastiptomac)(struct efi_simple_network *this,
>>>>> +                       int ipv6, struct efi_ip_address *ip,
>>>>> +                       struct efi_mac_address *mac);
>>>>> +       efi_status_t (EFIAPI *nvdata)(struct efi_simple_network *this,
>>>>> +                       int read_write, ulong offset, ulong buffer_size,
>>>>> +                       char *buffer);
>>>>> +       efi_status_t (EFIAPI *get_status)(struct efi_simple_network *this,
>>>>> +                       u32 *int_status, void **txbuf);
>>>>> +       efi_status_t (EFIAPI *transmit)(struct efi_simple_network *this,
>>>>> +                       ulong header_size, ulong buffer_size, void *buffer,
>>>>> +                       struct efi_mac_address *src_addr,
>>>>> +                       struct efi_mac_address *dest_addr, u16 *protocol);
>>>>> +       efi_status_t (EFIAPI *receive)(struct efi_simple_network *this,
>>>>> +                       ulong *header_size, ulong *buffer_size, void *buffer,
>>>>> +                       struct efi_mac_address *src_addr,
>>>>> +                       struct efi_mac_address *dest_addr, u16 *protocol);
>>>>> +       void (EFIAPI *waitforpacket)(void);
>>>>> +       struct efi_simple_network_mode *mode;
>>>>> +};
>>>>> +
>>>>> +#define EFI_PXE_GUID \
>>>>> +       EFI_GUID(0x03c4e603, 0xac28, 0x11d3, \
>>>>> +                0x9a, 0x2d, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
>>>>> +
>>>>> +struct efi_pxe_packet {
>>>>> +       u8 packet[1472];
>>>>> +};
>>>>> +
>>>>> +struct efi_pxe_mode
>>>>> +{
>>>>> +       u8 unused[52];
>>>>> +       struct efi_pxe_packet dhcp_discover;
>>>>> +       struct efi_pxe_packet dhcp_ack;
>>>>> +       struct efi_pxe_packet proxy_offer;
>>>>> +       struct efi_pxe_packet pxe_discover;
>>>>> +       struct efi_pxe_packet pxe_reply;
>>>>> +};
>>>>> +
>>>>> +struct efi_pxe {
>>>>> +       u64 rev;
>>>>> +       void (EFIAPI *start)(void);
>>>>> +       void (EFIAPI *stop)(void);
>>>>> +       void (EFIAPI *dhcp)(void);
>>>>> +       void (EFIAPI *discover)(void);
>>>>> +       void (EFIAPI *mftp)(void);
>>>>> +       void (EFIAPI *udpwrite)(void);
>>>>> +       void (EFIAPI *udpread)(void);
>>>>> +       void (EFIAPI *setipfilter)(void);
>>>>> +       void (EFIAPI *arp)(void);
>>>>> +       void (EFIAPI *setparams)(void);
>>>>> +       void (EFIAPI *setstationip)(void);
>>>>> +       void (EFIAPI *setpackets)(void);
>>>>> +       struct efi_pxe_mode *mode;
>>>>> +};
>>>>> +
>>>>> #endif
>>>>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>>>>> index 88b8149..8005454 100644
>>>>> --- a/include/efi_loader.h
>>>>> +++ b/include/efi_loader.h
>>>>> @@ -91,6 +91,12 @@ extern struct list_head efi_obj_list;
>>>>> int efi_disk_register(void);
>>>>> /* Called by bootefi to make GOP (graphical) interface available */
>>>>> int efi_gop_register(void);
>>>>> +/* Called by bootefi to make the network interface available */
>>>>> +int efi_net_register(void **handle);
>>>>> +
>>>>> +/* Called by networking code to memorize the dhcp ack package */
>>>>> +void efi_net_set_dhcp_ack(void *pkt, int len);
>>>>> +
>>>>> /*
>>>>>  * Stub implementation for a protocol opener that just returns the handle as
>>>>>  * interface
>>>>> @@ -157,5 +163,6 @@ static inline void ascii2unicode(u16 *unicode, char *ascii)
>>>>> static inline void efi_restore_gd(void) { }
>>>>> static inline void efi_set_bootdev(const char *dev, const char *devnr,
>>>>>                                   const char *path) { }
>>>>> +static inline void efi_net_set_dhcp_ack(void *pkt, int len) { }
>>>>>
>>>>> #endif
>>>>> diff --git a/include/net.h b/include/net.h
>>>>> index 05800c4..5ee5929 100644
>>>>> --- a/include/net.h
>>>>> +++ b/include/net.h
>>>>> @@ -269,7 +269,7 @@ int eth_getenv_enetaddr_by_index(const char *base_name, int index,
>>>>> int eth_init(void);                    /* Initialize the device */
>>>>> int eth_send(void *packet, int length);           /* Send a packet */
>>>>>
>>>>> -#ifdef CONFIG_API
>>>>> +#if defined(CONFIG_API) || defined(CONFIG_EFI_LOADER)
>>>>> int eth_receive(void *packet, int length); /* Receive a packet*/
>>>>> extern void (*push_packet)(void *packet, int length);
>>>>> #endif
>>>>> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
>>>>> index 83e31f6..2a3849e 100644
>>>>> --- a/lib/efi_loader/Makefile
>>>>> +++ b/lib/efi_loader/Makefile
>>>>> @@ -11,3 +11,4 @@ obj-y += efi_image_loader.o efi_boottime.o efi_runtime.o efi_console.o
>>>>> obj-y += efi_memory.o
>>>>> obj-$(CONFIG_LCD) += efi_gop.o
>>>>> obj-$(CONFIG_PARTITIONS) += efi_disk.o
>>>>> +obj-$(CONFIG_NET) += efi_net.o
>>>>> diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c
>>>>> new file mode 100644
>>>>> index 0000000..a95ff3c
>>>>> --- /dev/null
>>>>> +++ b/lib/efi_loader/efi_net.c
>>>>> @@ -0,0 +1,314 @@
>>>>> +/*
>>>>> + *  EFI application network access support
>>>>> + *
>>>>> + *  Copyright (c) 2016 Alexander Graf
>>>>> + *
>>>>> + *  SPDX-License-Identifier:     GPL-2.0+
>>>>> + */
>>>>> +
>>>>> +#include <common.h>
>>>>> +#include <efi_loader.h>
>>>>> +#include <inttypes.h>
>>>>> +#include <lcd.h>
>>>>> +#include <malloc.h>
>>>>> +
>>>>> +DECLARE_GLOBAL_DATA_PTR;
>>>>> +
>>>>> +static const efi_guid_t efi_net_guid = EFI_SIMPLE_NETWORK_GUID;
>>>>> +static const efi_guid_t efi_pxe_guid = EFI_PXE_GUID;
>>>>> +static struct efi_pxe_packet *dhcp_ack;
>>>>> +static bool new_rx_packet;
>>>>> +static void *new_tx_packet;
>>>>> +
>>>>> +struct efi_net_obj {
>>>>> +       /* Generic EFI object parent class data */
>>>>> +       struct efi_object parent;
>>>>> +       /* EFI Interface callback struct for network */
>>>>> +       struct efi_simple_network net;
>>>>> +       struct efi_simple_network_mode net_mode;
>>>>> +       /* Device path to the network adapter */
>>>>> +       struct efi_device_path_file_path dp[2];
>>>>> +       /* PXE struct to transmit dhcp data */
>>>>> +       struct efi_pxe pxe;
>>>>> +       struct efi_pxe_mode pxe_mode;
>>>>> +};
>>>>> +
>>>>> +static efi_status_t EFIAPI efi_net_start(struct efi_simple_network *this)
>>>>> +{
>>>>> +       EFI_ENTRY("%p", this);
>>>>> +
>>>>> +       return EFI_EXIT(EFI_SUCCESS);
>>>>> +}
>>>>> +
>>>>> +static efi_status_t EFIAPI efi_net_stop(struct efi_simple_network *this)
>>>>> +{
>>>>> +       EFI_ENTRY("%p", this);
>>>>> +
>>>>> +       return EFI_EXIT(EFI_SUCCESS);
>>>>> +}
>>>>> +
>>>>> +static efi_status_t EFIAPI efi_net_initialize(struct efi_simple_network *this,
>>>>> +                                             ulong extra_rx, ulong extra_tx)
>>>>> +{
>>>>> +       struct efi_net_obj *netobj = container_of(this, struct efi_net_obj, net);
>>>>> +       int ret;
>>>>> +       EFI_ENTRY("%p, %lx, %lx", this, extra_rx, extra_tx);
>>>>> +
>>>>> +       eth_halt();
>>>>> +       eth_set_current();
>>>>> +       ret = eth_init();
>>>>> +       if (ret < 0) {
>>>>> +               eth_halt();
>>>>> +               return EFI_EXIT(EFI_DEVICE_ERROR);
>>>>> +       }
>>>>> +
>>>>> +       netobj->net_mode.state = EFI_NETWORK_INITIALIZED;
>>>>> +
>>>>> +       return EFI_EXIT(EFI_SUCCESS);
>>>>> +}
>>>>> +
>>>>> +static efi_status_t EFIAPI efi_net_reset(struct efi_simple_network *this,
>>>>> +                                        int extended_verification)
>>>>> +{
>>>>> +       EFI_ENTRY("%p, %x", this, extended_verification);
>>>>> +
>>>>> +       return EFI_EXIT(EFI_SUCCESS);
>>>>> +}
>>>>> +
>>>>> +static efi_status_t EFIAPI efi_net_shutdown(struct efi_simple_network *this)
>>>>> +{
>>>>> +       EFI_ENTRY("%p", this);
>>>>> +
>>>>> +       return EFI_EXIT(EFI_SUCCESS);
>>>>> +}
>>>>> +
>>>>> +static efi_status_t EFIAPI efi_net_receive_filters(
>>>>> +               struct efi_simple_network *this, u32 enable, u32 disable,
>>>>> +               int reset_mcast_filter, ulong mcast_filter_count,
>>>>> +               struct efi_mac_address *mcast_filter)
>>>>> +{
>>>>> +       EFI_ENTRY("%p, %x, %x, %x, %lx, %p", this, enable, disable,
>>>>> +                 reset_mcast_filter, mcast_filter_count, mcast_filter);
>>>>> +
>>>>> +       /* XXX Do we care? */
>>>>> +
>>>>> +       return EFI_EXIT(EFI_SUCCESS);
>>>>> +}
>>>>> +
>>>>> +static efi_status_t EFIAPI efi_net_station_address(
>>>>> +               struct efi_simple_network *this, int reset,
>>>>> +               struct efi_mac_address *new_mac)
>>>>> +{
>>>>> +       EFI_ENTRY("%p, %x, %p", this, reset, new_mac);
>>>>> +
>>>>> +       return EFI_EXIT(EFI_INVALID_PARAMETER);
>>>>> +}
>>>>> +
>>>>> +static efi_status_t EFIAPI efi_net_statistics(struct efi_simple_network *this,
>>>>> +                                             int reset, ulong *stat_size,
>>>>> +                                             void *stat_table)
>>>>> +{
>>>>> +       EFI_ENTRY("%p, %x, %p, %p", this, reset, stat_size, stat_table);
>>>>> +
>>>>> +       return EFI_EXIT(EFI_INVALID_PARAMETER);
>>>>> +}
>>>>> +
>>>>> +static efi_status_t EFIAPI efi_net_mcastiptomac(struct efi_simple_network *this,
>>>>> +                                               int ipv6,
>>>>> +                                               struct efi_ip_address *ip,
>>>>> +                                               struct efi_mac_address *mac)
>>>>> +{
>>>>> +       EFI_ENTRY("%p, %x, %p, %p", this, ipv6, ip, mac);
>>>>> +
>>>>> +       return EFI_EXIT(EFI_INVALID_PARAMETER);
>>>>> +}
>>>>> +
>>>>> +static efi_status_t EFIAPI efi_net_nvdata(struct efi_simple_network *this,
>>>>> +                                         int read_write, ulong offset,
>>>>> +                                         ulong buffer_size, char *buffer)
>>>>> +{
>>>>> +       EFI_ENTRY("%p, %x, %lx, %lx, %p", this, read_write, offset, buffer_size,
>>>>> +                 buffer);
>>>>> +
>>>>> +       return EFI_EXIT(EFI_INVALID_PARAMETER);
>>>>> +}
>>>>> +
>>>>> +static efi_status_t EFIAPI efi_net_get_status(struct efi_simple_network *this,
>>>>> +                                             u32 *int_status, void **txbuf)
>>>>> +{
>>>>> +       EFI_ENTRY("%p, %p, %p", this, int_status, txbuf);
>>>>> +
>>>>> +       /* We send packets synchronously, so nothing is outstanding */
>>>>> +       if (int_status)
>>>>> +               *int_status = 0;
>>>>> +       if (txbuf)
>>>>> +               *txbuf = new_tx_packet;
>>>>> +
>>>>> +       new_tx_packet = NULL;
>>>>> +
>>>>> +       return EFI_EXIT(EFI_SUCCESS);
>>>>> +}
>>>>> +
>>>>> +static efi_status_t EFIAPI efi_net_transmit(struct efi_simple_network *this,
>>>>> +               ulong header_size, ulong buffer_size, void *buffer,
>>>>> +               struct efi_mac_address *src_addr,
>>>>> +               struct efi_mac_address *dest_addr, u16 *protocol)
>>>>> +{
>>>>> +       EFI_ENTRY("%p, %lx, %lx, %p, %p, %p, %p", this, header_size,
>>>>> +                 buffer_size, buffer, src_addr, dest_addr, protocol);
>>>>> +
>>>>> +       if (header_size) {
>>>>> +               /* We would need to create the header if header_size != 0 */
>>>>> +               return EFI_EXIT(EFI_INVALID_PARAMETER);
>>>>> +       }
>>>>> +
>>>>> +       /*
>>>>> +        * Some drivers need to have DMA aligned buffers, so if
>>>>> +        * it's misaligned, let's use our own.
>>>>> +        */
>>>>> +       if ((ulong)buffer & (ARCH_DMA_MINALIGN - 1)) {
>>>>> +               void *packet = memalign(ARCH_DMA_MINALIGN, buffer_size);
>>>>> +               memcpy(packet, buffer, buffer_size);
>>>>> +               net_send_packet(packet, buffer_size);
>>>>> +               free(packet);
>>>>> +       } else {
>>>>> +               net_send_packet(buffer, buffer_size);
>>>>> +       }
>>>>> +
>>>>> +       new_tx_packet = buffer;
>>>>> +
>>>>> +       return EFI_EXIT(EFI_SUCCESS);
>>>>> +}
>>>>> +
>>>>> +static void efi_net_push(void *pkt, int len)
>>>>> +{
>>>>> +       new_rx_packet = true;
>>>>> +}
>>>>> +
>>>>> +static efi_status_t EFIAPI efi_net_receive(struct efi_simple_network *this,
>>>>> +               ulong *header_size, ulong *buffer_size, void *buffer,
>>>>> +               struct efi_mac_address *src_addr,
>>>>> +               struct efi_mac_address *dest_addr, u16 *protocol)
>>>>> +{
>>>>> +       EFI_ENTRY("%p, %p, %p, %p, %p, %p, %p", this, header_size,
>>>>> +                 buffer_size, buffer, src_addr, dest_addr, protocol);
>>>>> +
>>>>> +       push_packet = efi_net_push;
>>>>> +       /* XXX With device model this reads multiple packets, we only want 1 */
>>>>> +       eth_rx();
>>>>> +       push_packet = NULL;
>>>>> +
>>>>> +       if (!new_rx_packet)
>>>>> +               return EFI_EXIT(EFI_NOT_READY);
>>>>> +
>>>>> +       if (*buffer_size < net_rx_packet_len) {
>>>>> +               /* Packet doesn't fit, try again with bigger buf */
>>>>> +               *buffer_size = net_rx_packet_len;
>>>>> +               return EFI_EXIT(EFI_BUFFER_TOO_SMALL);
>>>>> +       }
>>>>> +
>>>>> +       memcpy(buffer, net_rx_packet, net_rx_packet_len);
>>>>> +       *buffer_size = net_rx_packet_len;
>>>>> +       new_rx_packet = false;
>>>>> +
>>>>> +       return EFI_EXIT(EFI_SUCCESS);
>>>>> +}
>>>>> +
>>>>> +static efi_status_t efi_net_open_dp(void *handle, efi_guid_t *protocol,
>>>>> +                       void **protocol_interface, void *agent_handle,
>>>>> +                       void *controller_handle, uint32_t attributes)
>>>>> +{
>>>>> +       struct efi_simple_network *net = handle;
>>>>> +       struct efi_net_obj *netobj = container_of(net, struct efi_net_obj, net);
>>>>> +
>>>>> +       *protocol_interface = netobj->dp;
>>>>> +
>>>>> +       return EFI_SUCCESS;
>>>>> +}
>>>>> +
>>>>> +static efi_status_t efi_net_open_pxe(void *handle, efi_guid_t *protocol,
>>>>> +                       void **protocol_interface, void *agent_handle,
>>>>> +                       void *controller_handle, uint32_t attributes)
>>>>> +{
>>>>> +       struct efi_simple_network *net = handle;
>>>>> +       struct efi_net_obj *netobj = container_of(net, struct efi_net_obj, net);
>>>>> +
>>>>> +       *protocol_interface = &netobj->pxe;
>>>>> +
>>>>> +       return EFI_SUCCESS;
>>>>> +}
>>>>> +
>>>>> +void efi_net_set_dhcp_ack(void *pkt, int len)
>>>>> +{
>>>>> +       int maxsize = sizeof(*dhcp_ack);
>>>>> +
>>>>> +       if (!dhcp_ack)
>>>>> +               dhcp_ack = malloc(maxsize);
>>>>> +
>>>>> +       memcpy(dhcp_ack, pkt, min(len, maxsize));
>>>>> +}
>>>>> +
>>>>> +/* This gets called from do_bootefi_exec(). */
>>>>> +int efi_net_register(void **handle)
>>>>> +{
>>>>> +       struct efi_net_obj *netobj;
>>>>> +       struct efi_device_path_file_path dp_net = {
>>>>> +               .dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE,
>>>>> +               .dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH,
>>>>> +               .dp.length = sizeof(dp_net),
>>>>> +               .str = { 'N', 'e', 't' },
>>>>> +       };
>>>>> +       struct efi_device_path_file_path dp_end = {
>>>>> +               .dp.type = DEVICE_PATH_TYPE_END,
>>>>> +               .dp.sub_type = DEVICE_PATH_SUB_TYPE_END,
>>>>> +               .dp.length = sizeof(dp_end),
>>>>> +       };
>>>>> +
>>>>> +       if (!eth_get_dev()) {
>>>>> +               /* No eth device active, don't expose any */
>>>>> +               return 0;
>>>>> +       }
>>>>> +
>>>>> +       /* We only expose the "active" eth device, so one is enough */
>>>>> +       netobj = calloc(1, sizeof(*netobj));
>>>>> +
>>>>> +       /* Fill in object data */
>>>>> +       netobj->parent.protocols[0].guid = &efi_net_guid;
>>>>> +       netobj->parent.protocols[0].open = efi_return_handle;
>>>>> +       netobj->parent.protocols[1].guid = &efi_guid_device_path;
>>>>> +       netobj->parent.protocols[1].open = efi_net_open_dp;
>>>>> +       netobj->parent.protocols[2].guid = &efi_pxe_guid;
>>>>> +       netobj->parent.protocols[2].open = efi_net_open_pxe;
>>>>> +       netobj->parent.handle = &netobj->net;
>>>>> +       netobj->net.start = efi_net_start;
>>>>> +       netobj->net.stop = efi_net_stop;
>>>>> +       netobj->net.initialize = efi_net_initialize;
>>>>> +       netobj->net.reset = efi_net_reset;
>>>>> +       netobj->net.shutdown = efi_net_shutdown;
>>>>> +       netobj->net.receive_filters = efi_net_receive_filters;
>>>>> +       netobj->net.station_address = efi_net_station_address;
>>>>> +       netobj->net.statistics = efi_net_statistics;
>>>>> +       netobj->net.mcastiptomac = efi_net_mcastiptomac;
>>>>> +       netobj->net.nvdata = efi_net_nvdata;
>>>>> +       netobj->net.get_status = efi_net_get_status;
>>>>> +       netobj->net.transmit = efi_net_transmit;
>>>>> +       netobj->net.receive = efi_net_receive;
>>>>> +       netobj->net.mode = &netobj->net_mode;
>>>>> +       netobj->net_mode.state = EFI_NETWORK_STARTED;
>>>>> +       netobj->dp[0] = dp_net;
>>>>> +       netobj->dp[1] = dp_end;
>>>>> +       memcpy(netobj->net_mode.current_address.mac_addr, eth_get_ethaddr(), 6);
>>>>> +       netobj->net_mode.max_packet_size = PKTSIZE;
>>>>
>>>> Why is this statically created? Shouldn't it come from the U-Boot
>>>> ethernet tables dynamically when it is needed?
>>>
>>> The driver as is only exposes a single interface for the currently selected network device. My main goal behind this was pxe boot - where you usually load the payload from tftp and continue acting on the same interface later on.
>>>
>>> I guess we could create devices for all network devices as well. I remember that I wanted to only expose network if it's really necessary because of resource usage. But I don't remember exactly what resource the peoblem was otoh ;).
>>
>> I think in general (given that you are writing this code and this is
>> the best opportunity to get the design right) that things like this
>> should look up U-Boot tables rather than creating their own. So a
>> model where the device is scanned as needed seems better. Then if we
>> want to change the constraints later, e.g. devices appear that were
>> not previously present, then we can deal with that.
>>
>> I understand with block devices your point that there is no single
>> table of block devices. However, this support will be applied soon
>> (see u-boot-dm/master), even without driver model. I think we should
>> fix these things rather than work around them, as the code will be
>> more maintainable that way.
>
> I agree wholeheartedly :). The beauty of "proper" open source code is
> that you can actually modify the guts of it to make the code easier at
> the end of the day.
>
>> In this case I wonder if we should have an EFI device uclass, and
>> allow devices to have this as a child? It would allow for easy
>> enumeration and avoid another system-specific device table (something
>> that driver model is trying to reduce).
>
> I'm not 100% sure the EFI driver exposure is actually a child-parent
> relationship with the respective driver model objects. In Java speech
> it's more of an "interface" definition than a subclass.

The parent/child relationship is how this is done in driver model. For
example, MMC devices (UCLASS_MMC) provide a block device interface, as
a child device in UCLASS_BLK.

>
> But that said, I think that it may still make sense to move it into the
> respective driver cores and just have the EFI structs part of the block
> device structs.

But won't you have different types of drivers? E.g. video, network?

>
> However I don't consider it as very high priority at the moment.
> Functionally nothing should change from a payload point of view and the
> current code allows us to iron out bugs and have non-believers gain
> faith in the whole uEFI story without breaking non-uEFI code paths ;).
> And all of the actual interface code shouldn't change with a move into
> device objects.
>
> So how about we start to tackle this once the non-DM code is gone? That
> should simplify things for everyone.

I think instead we should set this up so that it works nicely with DM.
The non-DM code will be around for a long time. I'm not sure how long.

At present the EFI impl really doesn't use it at all. I'm not too
bothered about when, but I think it is worth looking at before too
long. I also suggest that we require DM with new features like this,
to encourage people to move.

Regards,
Simon


More information about the U-Boot mailing list