On 03/25/2018 07:44 PM, Patrick Wildt wrote:
On Fri, Mar 23, 2018 at 08:04:27PM +0100, Heinrich Schuchardt wrote:
On 03/23/2018 08:01 PM, Heinrich Schuchardt wrote:
From 689ada7663efae5ef13d021f3266e081d1d53293 Mon Sep 17 00:00:00 2001
From: Patrick Wildt patrick@blueri.se Date: Fri, 23 Mar 2018 15:38:48 +0100 Subject: [PATCH 2/2] efi_loader: set the dhcp ack received flag
The PXE object contains a flag that specifies whether or not a DHCP ACK has been received. This might be used by programs to find out whether or not it is worth to read the DHCP information ot ouf our object.
Why should we implement this change now without a consumer for the information?
Signed-off-by: Patrick Wildt patrick@blueri.se
include/efi_api.h | 4 +++- lib/efi_loader/efi_net.c | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/efi_api.h b/include/efi_api.h index 3ba650e57e..7dfa17f5c6 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -756,7 +756,9 @@ struct efi_pxe_packet {
struct efi_pxe_mode {
- u8 unused[52];
- u8 unused1[9];
- u8 dhcp_ack_received;
Why use a byte in the middle of the unused region?
The EFI Spec defines shared interfaces. In this case u-boot implements the PXE Base Code Protocol, and "struct efi_pxe_mode" is a definition for a struct that is shared on the interface. The struct is defined in UEFI Spec 2.6 Chapter 23.3 as EFI_PXE_BASE_CODE_MODE. In this struct, theres a boolean called DhcpAckReceivd, which is the 10th member of the struct. Since booleans in EFI are defined to uint8_t, this means it's the 10th byte starting from the beginning of the struct. Since u-boot does not define all of the members of the struct, the first 52 bytes are "unused". Since I am now accessing a field in the interface in the middle of those 52 bytes, it is split up into a first set and a second set of unused bytes, with the new dhcp_ack_received in the middle. Thus, it's not just a simple byte in the middle of an unused region and putting it somewhere else would violate the spec.
The consumer in this case would be the EFI Application being booted using the "bootefi" command.
Please correct me if I'm wrong.
Best regards, Patrick
Thank you for the explanation. I think the right way go ahead is to add all missing fields and to do away with unused[].
Please, carefully observe the alignment. The spec defines BOOLEAN as 8bit value. ToS is the 19th byte followed by an EFI_IP_ADDRESS which is a 16-byte buffer aligned on a 4-byte boundary. So after ToS we need one byte to ensure alignment. We could define a struct efi_ip_address as u8 a[16] __attribute__((aligned(4))).
Best regards
Heinrich
Best regards
Heinrich
- u8 unused2[42]; struct efi_pxe_packet dhcp_discover; struct efi_pxe_packet dhcp_ack; struct efi_pxe_packet proxy_offer;
diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c index 8c5d5b492c..0b9c7b9345 100644 --- a/lib/efi_loader/efi_net.c +++ b/lib/efi_loader/efi_net.c @@ -332,8 +332,10 @@ int efi_net_register(void) netobj->net_mode.max_packet_size = PKTSIZE;
netobj->pxe.mode = &netobj->pxe_mode;
- if (dhcp_ack)
if (dhcp_ack) { netobj->pxe_mode.dhcp_ack = *dhcp_ack;
netobj->pxe_mode.dhcp_ack_received = 1;}
/*
- Create WaitForPacket event.