[U-Boot] [PATCH 2/2] efi_loader: set the dhcp ack received flag

[Alexander Graf]

On 26.03.18 15:28, Patrick Wildt wrote:
> On Mon, Mar 26, 2018 at 06:39:06AM +0200, Heinrich Schuchardt wrote:
>> On 03/25/2018 10:23 PM, Patrick Wildt wrote:
>>> On Sun, Mar 25, 2018 at 10:04:55PM +0200, Heinrich Schuchardt wrote:
>>>> 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
>>>
>>> I have noticed that, yes.  I think explicitly padding the struct gives
>>> better visibility of the issue, instead of relying on an implicit
>>> alignment.  Two other structures in u-boot EFI headers contain explicit
>>> "pad" members.  I'd feel safer to go that route.  What do you think
>>> about the following?
>>>
>>> Best regards,
>>> Patrick
>>
>> I think it is fine to use a padding byte. But still the alignment should
>> be specified for efi_ip_address. Otherwise we might pass data with wrong
>> alignment.
>>
>> Best regards
>>
>> Heinrich
> 
> EDK2 does this by defining EFI_IP_ADDRESS to a union which includes a
> uint32_t addr[4], but I guess the attribute makes it more explicit.
> This should do:

Looks good to me. Can you please resubmit as real patch with proper
patch description, SoB line and CC everyone on this thread?


Thanks!

Alex

> 
> Best regards,
> Patrick
> 
> diff --git a/include/efi_api.h b/include/efi_api.h
> index 3ba650e57e..06789acdd1 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -662,7 +662,7 @@ struct efi_mac_address {
>  
>  struct efi_ip_address {
>  	u8 ip_addr[16];
> -};
> +} __attribute__((aligned(4)));
>  
>  enum efi_simple_network_state {
>  	EFI_NETWORK_STOPPED,
> @@ -756,7 +756,28 @@ struct efi_pxe_packet {
>  
>  struct efi_pxe_mode
>  {
> -	u8 unused[52];
> +	u8 started;
> +	u8 ipv6_available;
> +	u8 ipv6_supported;
> +	u8 using_ipv6;
> +	u8 bis_supported;
> +	u8 bis_detected;
> +	u8 auto_arp;
> +	u8 send_guid;
> +	u8 dhcp_discover_valid;
> +	u8 dhcp_ack_received;
> +	u8 proxy_offer_received;
> +	u8 pxe_discovervalid;
> +	u8 pxe_reply_received;
> +	u8 pxe_bis_reply_received;
> +	u8 icmp_error_received;
> +	u8 tftp_error_received;
> +	u8 make_callbacks;
> +	u8 ttl;
> +	u8 tos;
> +	u8 pad;
> +	struct efi_ip_address station_ip;
> +	struct efi_ip_address subnet_mask;
>  	struct efi_pxe_packet dhcp_discover;
>  	struct efi_pxe_packet dhcp_ack;
>  	struct efi_pxe_packet proxy_offer;
>