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 <patr...@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 <patr...@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.
>>>
>>
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
