+Tom Hi Alex,
On 16 May 2016 at 12:06, Alexander Graf <[email protected]> wrote: > > > On 16.05.16 15:24, Simon Glass wrote: >> Hi Alexander, >> >> On 14 May 2016 at 14:34, Alexander Graf <[email protected]> wrote: >>> >>> >>>> Am 14.05.2016 um 21:49 schrieb Simon Glass <[email protected]>: >>>> >>>> Hi Alexander, >>>> >>>>> On 10 May 2016 at 15:25, Alexander Graf <[email protected]> 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 <[email protected]> >>>>> >>>>> --- >>>>> >>>>> 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 _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
