Re: svn commit: r320234 - head/sys/boot/efi/include

[Ed Maste]

On 22 June 2017 at 14:23, Dimitry Andric  wrote:
>> Modified: head/sys/boot/efi/include/efidef.h
>> ==
>> --- head/sys/boot/efi/include/efidef.hThu Jun 22 13:31:44 2017   
>>  (r320233)
>> +++ head/sys/boot/efi/include/efidef.hThu Jun 22 14:30:09 2017   
>>  (r320234)
>> @@ -178,6 +178,7 @@ typedef enum {
>> #define EFI_MEMORY_DESCRIPTOR_VERSION  1
>> typedef struct {
>> UINT32  Type;   // Field size is 32 bits 
>> followed by 32 bit pad
>> +UINT32  Pad;
>> EFI_PHYSICAL_ADDRESSPhysicalStart;  // Field size is 64 bits
>> EFI_VIRTUAL_ADDRESS VirtualStart;   // Field size is 64 bits
>> UINT64  NumberOfPages;  // Field size is 64 bits
>
> If you explicitly insert padding, it is better to mark the struct as
> __packed, I think.  Or at least also specify the alignment explicitly.

Perhaps, although this struct now matches the definition in the edk
and u-boot EFI headers.
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r320234 - head/sys/boot/efi/include

[Toomas Soome]

> On 22. juuni 2017, at 21:23, Dimitry Andric  wrote:
> 
> On 22 Jun 2017, at 16:30, Ed Maste  wrote:
>> 
>> Author: emaste
>> Date: Thu Jun 22 14:30:09 2017
>> New Revision: 320234
>> URL: https://svnweb.freebsd.org/changeset/base/320234
>> 
>> Log:
>> Make structure padding explicit in EFI_MEMORY_DESCRIPTOR
>> 
>> The EFI memory descriptor 64-bit aligns PhysicalStart on both 32- and
>> 64-bit platforms.  Make the padding explicit for i386 EFI.
>> 
>> Submitted by:Siva Mahadevan 
>> MFC after:   3 weeks
>> Sponsored by:The FreeBSD Foundation
>> Differential Revision:   https://reviews.freebsd.org/D11301
>> 
>> Modified:
>> head/sys/boot/efi/include/efidef.h
>> 
>> Modified: head/sys/boot/efi/include/efidef.h
>> ==
>> --- head/sys/boot/efi/include/efidef.h   Thu Jun 22 13:31:44 2017
>> (r320233)
>> +++ head/sys/boot/efi/include/efidef.h   Thu Jun 22 14:30:09 2017
>> (r320234)
>> @@ -178,6 +178,7 @@ typedef enum {
>> #define EFI_MEMORY_DESCRIPTOR_VERSION  1
>> typedef struct {
>>UINT32  Type;   // Field size is 32 bits 
>> followed by 32 bit pad
>> +UINT32  Pad;
>>EFI_PHYSICAL_ADDRESSPhysicalStart;  // Field size is 64 bits
>>EFI_VIRTUAL_ADDRESS VirtualStart;   // Field size is 64 bits
>>UINT64  NumberOfPages;  // Field size is 64 bits
> 
> If you explicitly insert padding, it is better to mark the struct as
> __packed, I think.  Or at least also specify the alignment explicitly.
> 
> -Dimitry
> 


There is also second one with the same issue:

in sys/boot/efi/include/efiprot.h
@@ -90,12 +90,14 @@ typedef struct {
 BOOLEAN LogicalPartition;
 BOOLEAN ReadOnly;
 BOOLEAN WriteCaching;
+UINT8   pad1[3];
 
 UINT32  BlockSize;
 UINT32  IoAlign;
+UINT8   pad2[4];
 
 EFI_LBA LastBlock;
-} EFI_BLOCK_IO_MEDIA;
+} __packed EFI_BLOCK_IO_MEDIA;

Without this, the EFI32 will get disk sizes wrong.

rgds,
toomas

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r320234 - head/sys/boot/efi/include

[Dimitry Andric]

On 22 Jun 2017, at 16:30, Ed Maste  wrote:
> 
> Author: emaste
> Date: Thu Jun 22 14:30:09 2017
> New Revision: 320234
> URL: https://svnweb.freebsd.org/changeset/base/320234
> 
> Log:
>  Make structure padding explicit in EFI_MEMORY_DESCRIPTOR
> 
>  The EFI memory descriptor 64-bit aligns PhysicalStart on both 32- and
>  64-bit platforms.  Make the padding explicit for i386 EFI.
> 
>  Submitted by:Siva Mahadevan 
>  MFC after:   3 weeks
>  Sponsored by:The FreeBSD Foundation
>  Differential Revision:   https://reviews.freebsd.org/D11301
> 
> Modified:
>  head/sys/boot/efi/include/efidef.h
> 
> Modified: head/sys/boot/efi/include/efidef.h
> ==
> --- head/sys/boot/efi/include/efidef.hThu Jun 22 13:31:44 2017
> (r320233)
> +++ head/sys/boot/efi/include/efidef.hThu Jun 22 14:30:09 2017
> (r320234)
> @@ -178,6 +178,7 @@ typedef enum {
> #define EFI_MEMORY_DESCRIPTOR_VERSION  1
> typedef struct {
> UINT32  Type;   // Field size is 32 bits 
> followed by 32 bit pad
> +UINT32  Pad;
> EFI_PHYSICAL_ADDRESSPhysicalStart;  // Field size is 64 bits
> EFI_VIRTUAL_ADDRESS VirtualStart;   // Field size is 64 bits
> UINT64  NumberOfPages;  // Field size is 64 bits

If you explicitly insert padding, it is better to mark the struct as
__packed, I think.  Or at least also specify the alignment explicitly.

-Dimitry



signature.asc
Description: Message signed with OpenPGP

svn commit: r320234 - head/sys/boot/efi/include

[Ed Maste]

Author: emaste
Date: Thu Jun 22 14:30:09 2017
New Revision: 320234
URL: https://svnweb.freebsd.org/changeset/base/320234

Log:
  Make structure padding explicit in EFI_MEMORY_DESCRIPTOR
  
  The EFI memory descriptor 64-bit aligns PhysicalStart on both 32- and
  64-bit platforms.  Make the padding explicit for i386 EFI.
  
  Submitted by: Siva Mahadevan 
  MFC after:3 weeks
  Sponsored by: The FreeBSD Foundation
  Differential Revision:https://reviews.freebsd.org/D11301

Modified:
  head/sys/boot/efi/include/efidef.h

Modified: head/sys/boot/efi/include/efidef.h
==
--- head/sys/boot/efi/include/efidef.h  Thu Jun 22 13:31:44 2017
(r320233)
+++ head/sys/boot/efi/include/efidef.h  Thu Jun 22 14:30:09 2017
(r320234)
@@ -178,6 +178,7 @@ typedef enum {
 #define EFI_MEMORY_DESCRIPTOR_VERSION  1
 typedef struct {
 UINT32  Type;   // Field size is 32 bits 
followed by 32 bit pad
+UINT32  Pad;
 EFI_PHYSICAL_ADDRESSPhysicalStart;  // Field size is 64 bits
 EFI_VIRTUAL_ADDRESS VirtualStart;   // Field size is 64 bits
 UINT64  NumberOfPages;  // Field size is 64 bits
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"