Re: [edk2] [PATCH] MdeModulePkg: add PartitionName protocol

[Tian, Feng]

My concern is the name string is also not reliable. Different partition tools 
may have different naming rules.

Yes, "private protocol" I meant is using it as your local implementation.

Last, still  has a little curious on your usage model, could you explain it 
more? Maybe there is a better way to solve your problem.

Thanks
Feng

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Michael 
Zimmermann
Sent: Wednesday, September 09, 2015 15:48
To: Tian, Feng
Cc: Ni, Ruiyu; edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] MdeModulePkg: add PartitionName protocol

UniquePartitionGUID isn't suitable because the device vendor's don't set proper 
values(they're auto generated each time the partition table changes).
Instead names like "boot" are used so you can find the correct partition.

by "Implement it as a private protocol" do u just mean "use your own fork"
or is there some different protocol type?

On Wed, Sep 9, 2015 at 9:34 AM, Tian, Feng  wrote:

> Michael,
>
> IMHO, I don't think it's valuable to introducing this new protocol to 
> get a name string. It's too burden. If only you have this use case, I 
> would suggest you implement it as a private protocol rather than the public 
> one.
>
> PS: Why UniquePartitionGUID doesn't work for your case?
>
> Thanks
> Feng
>
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Michael Zimmermann
> Sent: Wednesday, September 09, 2015 15:28
> To: Ni, Ruiyu
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] MdeModulePkg: add PartitionName protocol
>
> Hi,
>
> I need the partition name because I need to boot from partitions by 
> their GPT label(filesystem label and GUID aren't set/reliable).
>
> Usually I would've just added a name field to the existing DiskIo 
> Protocol, but since it's a UEFI standard we can't/shouldn't do this.
>
> I chose option 3 over 1/2 because the others would mean that u have to 
> parse the whole partition table again which would lead to redundant code.
>
>
> Michael
>
> On Sun, Sep 6, 2015 at 8:33 AM, Ni, Ruiyu  wrote:
>
> > Zimmermann,
> > There are three options to get the name of a partition:
> > 1. Directly use BlockIo interface to access the GPT meta data.
> > 2. Group the above code into a library 3. Group the above code into 
> > a driver and expose the partition name as a protocol.
> >
> > I really do not think we need to go to option 3 directly. Can you 
> > tell us the reason why a protocol is needed?
> >
> > Thanks,
> > Ray
> >
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf 
> > Of Michael Zimmermann
> > Sent: Thursday, September 3, 2015 2:23 PM
> > To: edk2-devel@lists.01.org
> > Subject: [edk2] [PATCH] MdeModulePkg: add PartitionName protocol
> >
> > This allows to retrieve the name of a partition.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Michael Zimmermann 
> > ---
> >  MdeModulePkg/Include/Protocol/PartitionName.h  | 42
> > ++
> >  MdeModulePkg/MdeModulePkg.dec  |  3 ++
> >  .../Universal/Disk/PartitionDxe/ElTorito.c |  3 +-
> >  MdeModulePkg/Universal/Disk/PartitionDxe/Gpt.c |  3 +-
> >  MdeModulePkg/Universal/Disk/PartitionDxe/Mbr.c |  6 ++--
> >  .../Universal/Disk/PartitionDxe/Partition.c| 13 ++-
> >  .../Universal/Disk/PartitionDxe/Partition.h|  5 ++-
> >  .../Universal/Disk/PartitionDxe/PartitionDxe.inf   |  2 ++
> >  8 files changed, 71 insertions(+), 6 deletions(-)  create mode 
> > 100644 MdeModulePkg/Include/Protocol/PartitionName.h
> >
> > diff --git a/MdeModulePkg/Include/Protocol/PartitionName.h
> > b/MdeModulePkg/Include/Protocol/PartitionName.h
> > new file mode 100644
> > index 000..460ab27
> > --- /dev/null
> > +++ b/MdeModulePkg/Include/Protocol/PartitionName.h
> > @@ -0,0 +1,42 @@
> > +#ifndef __PARTITION_NAME_H__
> > +#define __PARTITION_NAME_H__
> > +
> > +#include 
> > +
> > +#define EFI_PARTITION_NAME_PROTOCOL_GUID \
> > +  { \
> > +0xf3287350, 0x384a, 0x44ec, {0x95, 0xcf, 0xdb, 0x4b, 0x86, 
> > +0x7c, 0x96,
> > 0x8c } \
> > +  }
> > +
> > +typedef struct _EFI_PARTITION_NAME_PROTOCOL 
> > +EFI_PARTITION_NAME_PROTOCOL;
> > +
> > +///
> > +/// Protocol GUID name
> > +///
> > +#define PARTITION_NAME_PROTOCOL   EFI_PARTITION_NAME_PROTOCOL_GUID
> > +
> > +///
> > +/// Protocol
> > +///
> > +typedef EFI_PARTITION_NAME_PROTOCOL   EFI_PARTITION_NAME;
> > +
> > +#define EFI_PARTITION_NAME_PROTOCOL_REVISION  0x0001 #define
> > +EFI_PARTITION_NAME_PROTOCOL_REVISION2 0x00020001 #define
> > +EFI_PARTITION_NAME_PROTOCOL_REVISION3 0x00020031
> > +
> > +///
> > +/// Revision
> > +///
> > +#define EFI_PARTITION_NAME_INTERFACE_REVISION
> > EFI_PARTITION_NAME_PROTOCOL_REVISION
> > +
> > +///
> > +///  This protocol provides the name of a partition /// struct 
> > 

Re: [edk2] [PATCH] MdeModulePkg: ScsiDiskDxe: adapt SectorCount when shortening transfers

[Tian, Feng]

I agreed ScsiDisk misses the handling for EFI_BAD_BUFFER_SIZE. That's why I 
proposed a patch although it's not good:-)

The uncertain part from my side at previous discussion is about the value of 
In/OutTransferLength when EFI_BAD_BUFFER_SIZE is returned.

In UEFI spec, there are two sentences to explain EFI_BAD_BUFFER_SIZE:

1) If InTransferLength is larger than the SCSI controller can handle, no data 
will be transferred, InTransferLength will be updated to contain the number of 
bytes that the SCSI controller is able to transfer, and EFI_BAD_BUFFER_SIZE 
will be returned.

2) If the data buffer described by InDataBuffer and InTransferLength is too big 
to be transferred in a single command, then no data is transferred and 
EFI_BAD_BUFFER_SIZE is returned. The number of bytes that can be transferred in 
a single command are returned in InTransferLength.

I used to only refer to #2 and thought it means the target's capability rather 
than host. So I thought for read/write cmd the In/OutTransferLength == 
SectorCount * BlockSize.

But now it looks Paolo's explanation makes more sense. So I agree with Laszlo's 
2nd patch. 

Reviewed-by: Feng Tian 

Thanks
Feng

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
Ersek
Sent: Tuesday, September 08, 2015 18:23
To: Paolo Bonzini; Tian, Feng
Cc: edk2-de...@ml01.01.org
Subject: Re: [edk2] [PATCH] MdeModulePkg: ScsiDiskDxe: adapt SectorCount when 
shortening transfers

On 09/08/15 10:07, Paolo Bonzini wrote:
> 
> 
> On 08/09/2015 09:02, Tian, Feng wrote:
>> Hi, Laszlo
>>
>> 1. I don't think the code in VirtioScsi.c is correct. It hardcodes 
>> the block size as 512bytes. The driver should send READ_CAPACITY cmd 
>> to get block size, then convert it to InTransferLength & OutTransferLength.
> 
> This is the controller's maximum transfer size, not the LUN.  The 
> LUN's limits are available from VPD and depend on the block size.  The 
> controller's limit apply to all LUNs and to all commands, even those 
> that transfer bytes rather than blocks.
> 
> For historical reasons the controller's maximum transfer size is 
> provided in 512-byte units.  In fact, Laszlo raised this point to the 
> virtio committee just to be sure that his code is correct, which it is.

Thanks.

>> 2. Although the UEFI spec doesn't say the relationship of 
>> InTransferLength/OutTransferLength and CDB.TransferLength, but I 
>> think it's implied that InTransferLength/OutTransferLength = 
>> CDB.TransferLength * BlockSize. Otherwise how the lowest layer host 
>> controller driver constructs transfer descriptor? How many bytes to 
>> read? Is InTransferLength/OutTransferLength or the one of 
>> CDB.TransferLength * BlockSize?
> 
> It's always InTransferLength/OutTransferLength.  If it is < or > 
> CDB.TransferLength * BlockSize you get respectively an underrun or an 
> overrun.  An underrun is fatal, an overrun is not.
> 
> InTransferLength/OutTransferLength exist because the controller may 
> not know how to infer the transfer length for all CDB opcodes (some 
> express it in bytes, some express it in blocks, some are vendor 
> specific, some are just crazy and you have to inspect multiple CDB 
> fields to compute the transfer length).
> 
> Of course ScsiDisk *does* know how to infer the transfer length for 
> the CDBs it builds.  Therefore, it makes sense for ScsiDisk's own READ 
> and WRITE CDBs to always have InTransferLength/OutTransferLength = 
> CDB.TransferLength * BlockSize.  However, the controller driver must 
> not rely on this.

I'd like to add something here (which might be simply rephrasing what you said):

I think Feng's question is about my former statement, which goes like:

In case the host controller (ie. not the LUN) is unable to satisfy
the request, due to its size (InTransferLength/OutTransferLength)
being too large, then it is allowed to return any kind of shorter
InTransferLength/OutTransferLength, and the returned max transfer
size *need not* be an integral multiple of any kind of block size.
Ad absurdum, it could be a prime number too.

Feng's patch allowed the controller to return a lower transfer length, but it 
also expected said lower transfer length to be a multiple of "the" block size.

But this is incorrect, because at the host controller level, the concept of 
"block size" doesn't exist at all! Some LUN hanging off the same host 
controller could have a block size of 2KB, another 512B, yet another 4KB.

So, going beyond the statement "the controller driver must not rely on this", I 
state that the controller driver doesn't even *know* about any block size! The 
condition that the host controller's maximum transfer size is exceeded is 
detected *much earlier* than talking to any LUN.

Therefore, if the host controller driver returns a value saying "hey this is my 
max transfer size", it is the responsibility of the LUN- and media-specific 
higher level driver 

Re: [edk2] [patch] MdeModulePkg: PXE Driver's LoadFile protocol should check FilePath

[Fu, Siyuan]

Reviewed-by: Fu Siyuan 



-Original Message-
From: Zhang, Lubo 
Sent: Wednesday, September 9, 2015 5:09 PM
To: edk2-devel@lists.01.org
Cc: Fu, Siyuan ; Ye, Ting 
Subject: [patch] MdeModulePkg: PXE Driver's LoadFile protocol should check 
FilePath

PXE driver's LoadFile protocol should check the input parameter FilePath to see 
whether it's a supported device path.If not, it should return invalid 
parameter, do not continue PXE boot.

Cc: Fu Siyuan
Cc: Ye Ting
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
---
 MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcImpl.c | 4   
MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcImpl.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcImpl.c 
b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcImpl.c
index 4dd7944..72923f1 100644
--- a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcImpl.c
+++ b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcImpl.c
@@ -2774,10 +2774,14 @@ EfiPxeLoadFile (
   BOOLEAN NewMakeCallback;
   EFI_STATUS  Status;
   UINT64  TmpBufSize;
   BOOLEAN MediaPresent;
 
+  if (FilePath == NULL || !IsDevicePathEnd (FilePath)) {
+return EFI_INVALID_PARAMETER;
+  }
+  
   Private = PXEBC_PRIVATE_DATA_FROM_LOADFILE (This);
   PxeBc   = >PxeBc;
   NewMakeCallback = FALSE;
   Status  = EFI_DEVICE_ERROR;
 
diff --git a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcImpl.h 
b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcImpl.h
index 6e88dea..ce8d8ad 100644
--- a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcImpl.h
+++ b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcImpl.h
@@ -31,10 +31,11 @@ typedef struct _PXEBC_PRIVATE_DATA  PXEBC_PRIVATE_DATA;  
#include   #include   #include 

 
 #include 
+#include 
 #include 
 #include   #include 
  #include 
 #include 
--
1.9.5.msysgit.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg: add PartitionName protocol

[Tian, Feng]

Michael,

IMHO, I don't think it's valuable to introducing this new protocol to get a 
name string. It's too burden. If only you have this use case, I would suggest 
you implement it as a private protocol rather than the public one.

PS: Why UniquePartitionGUID doesn't work for your case?

Thanks
Feng

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Michael 
Zimmermann
Sent: Wednesday, September 09, 2015 15:28
To: Ni, Ruiyu
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] MdeModulePkg: add PartitionName protocol

Hi,

I need the partition name because I need to boot from partitions by their GPT 
label(filesystem label and GUID aren't set/reliable).

Usually I would've just added a name field to the existing DiskIo Protocol, but 
since it's a UEFI standard we can't/shouldn't do this.

I chose option 3 over 1/2 because the others would mean that u have to parse 
the whole partition table again which would lead to redundant code.


Michael

On Sun, Sep 6, 2015 at 8:33 AM, Ni, Ruiyu  wrote:

> Zimmermann,
> There are three options to get the name of a partition:
> 1. Directly use BlockIo interface to access the GPT meta data.
> 2. Group the above code into a library 3. Group the above code into a 
> driver and expose the partition name as a protocol.
>
> I really do not think we need to go to option 3 directly. Can you tell 
> us the reason why a protocol is needed?
>
> Thanks,
> Ray
>
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Michael Zimmermann
> Sent: Thursday, September 3, 2015 2:23 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH] MdeModulePkg: add PartitionName protocol
>
> This allows to retrieve the name of a partition.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Michael Zimmermann 
> ---
>  MdeModulePkg/Include/Protocol/PartitionName.h  | 42
> ++
>  MdeModulePkg/MdeModulePkg.dec  |  3 ++
>  .../Universal/Disk/PartitionDxe/ElTorito.c |  3 +-
>  MdeModulePkg/Universal/Disk/PartitionDxe/Gpt.c |  3 +-
>  MdeModulePkg/Universal/Disk/PartitionDxe/Mbr.c |  6 ++--
>  .../Universal/Disk/PartitionDxe/Partition.c| 13 ++-
>  .../Universal/Disk/PartitionDxe/Partition.h|  5 ++-
>  .../Universal/Disk/PartitionDxe/PartitionDxe.inf   |  2 ++
>  8 files changed, 71 insertions(+), 6 deletions(-)  create mode 100644 
> MdeModulePkg/Include/Protocol/PartitionName.h
>
> diff --git a/MdeModulePkg/Include/Protocol/PartitionName.h
> b/MdeModulePkg/Include/Protocol/PartitionName.h
> new file mode 100644
> index 000..460ab27
> --- /dev/null
> +++ b/MdeModulePkg/Include/Protocol/PartitionName.h
> @@ -0,0 +1,42 @@
> +#ifndef __PARTITION_NAME_H__
> +#define __PARTITION_NAME_H__
> +
> +#include 
> +
> +#define EFI_PARTITION_NAME_PROTOCOL_GUID \
> +  { \
> +0xf3287350, 0x384a, 0x44ec, {0x95, 0xcf, 0xdb, 0x4b, 0x86, 0x7c, 
> +0x96,
> 0x8c } \
> +  }
> +
> +typedef struct _EFI_PARTITION_NAME_PROTOCOL  
> +EFI_PARTITION_NAME_PROTOCOL;
> +
> +///
> +/// Protocol GUID name
> +///
> +#define PARTITION_NAME_PROTOCOL   EFI_PARTITION_NAME_PROTOCOL_GUID
> +
> +///
> +/// Protocol
> +///
> +typedef EFI_PARTITION_NAME_PROTOCOL   EFI_PARTITION_NAME;
> +
> +#define EFI_PARTITION_NAME_PROTOCOL_REVISION  0x0001 #define 
> +EFI_PARTITION_NAME_PROTOCOL_REVISION2 0x00020001 #define 
> +EFI_PARTITION_NAME_PROTOCOL_REVISION3 0x00020031
> +
> +///
> +/// Revision
> +///
> +#define EFI_PARTITION_NAME_INTERFACE_REVISION
> EFI_PARTITION_NAME_PROTOCOL_REVISION
> +
> +///
> +///  This protocol provides the name of a partition /// struct 
> +_EFI_PARTITION_NAME_PROTOCOL {
> +  CHAR16  Name[36];
> +};
> +
> +extern EFI_GUID gEfiPartitionNameProtocolGuid;
> +
> +#endif
> +
> diff --git a/MdeModulePkg/MdeModulePkg.dec 
> b/MdeModulePkg/MdeModulePkg.dec index 5c1d29a..290ecaf 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -449,6 +449,9 @@
>## Include/Protocol/SmmReadyToBoot.h
>gEdkiiSmmReadyToBootProtocolGuid = { 0x6e057ecf, 0xfa99, 0x4f39, { 
> 0x95, 0xbc, 0x59, 0xf9, 0x92, 0x1d, 0x17, 0xe4 } }
>
> +  ## Include/Protocol/PartitionName.h
> +  gEfiPartitionNameProtocolGuid= { 0xf3287350, 0x384a, 0x44ec, { 0x95,
> 0xcf, 0xdb, 0x4b, 0x86, 0x7c, 0x96, 0x8c } }
> +
>  #
>  # [Error.gEfiMdeModulePkgTokenSpaceGuid]
>  #   0x8001 | Invalid value provided.
> diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/ElTorito.c
> b/MdeModulePkg/Universal/Disk/PartitionDxe/ElTorito.c
> index 2238449..094ef09 100644
> --- a/MdeModulePkg/Universal/Disk/PartitionDxe/ElTorito.c
> +++ b/MdeModulePkg/Universal/Disk/PartitionDxe/ElTorito.c
> @@ -260,7 +260,8 @@ PartitionInstallElToritoChildHandles (
>  Catalog->Boot.Lba * (SIZE_2KB / Media->BlockSize),
>  MultU64x32 (Catalog->Boot.Lba + CdDev.PartitionSize - 
> 1, 

Re: [edk2] [PATCH] MdeModulePkg: ScsiDiskDxe: adapt SectorCount when shortening transfers

[Laszlo Ersek]

On 09/09/15 09:11, Tian, Feng wrote:
> I agreed ScsiDisk misses the handling for EFI_BAD_BUFFER_SIZE. That's why I 
> proposed a patch although it's not good:-)
> 
> The uncertain part from my side at previous discussion is about the value of 
> In/OutTransferLength when EFI_BAD_BUFFER_SIZE is returned.
> 
> In UEFI spec, there are two sentences to explain EFI_BAD_BUFFER_SIZE:
> 
> 1) If InTransferLength is larger than the SCSI controller can handle, no data 
> will be transferred, InTransferLength will be updated to contain the number 
> of bytes that the SCSI controller is able to transfer, and 
> EFI_BAD_BUFFER_SIZE will be returned.
> 
> 2) If the data buffer described by InDataBuffer and InTransferLength is too 
> big to be transferred in a single command, then no data is transferred and 
> EFI_BAD_BUFFER_SIZE is returned. The number of bytes that can be transferred 
> in a single command are returned in InTransferLength.
> 
> I used to only refer to #2 and thought it means the target's capability 
> rather than host. So I thought for read/write cmd the In/OutTransferLength == 
> SectorCount * BlockSize.
> 
> But now it looks Paolo's explanation makes more sense. So I agree with 
> Laszlo's 2nd patch. 
> 
> Reviewed-by: Feng Tian 

Thank you.

I'll now repost the full series (two patches), because I'd like to make
sure that you agree with the commit message I wrote for the separate
EFI_BAD_BUFFER_SIZE handling patch, and also just to be sure that your
Reviewed-by covers both patches.

Therefore I will not add your R-b from above just yet to those two
patches -- can you please respond separately with your R-b to the series
I'm about to post? There won't be any changes in that series relative to
what you've seen here.

(Sorry about the churn, but I think this is safer than me going ahead
and committing it now, and then realizing you meant something else, and
having to revert / update the patches.)

Thank you!
Laszlo

> Thanks
> Feng
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
> Ersek
> Sent: Tuesday, September 08, 2015 18:23
> To: Paolo Bonzini; Tian, Feng
> Cc: edk2-de...@ml01.01.org
> Subject: Re: [edk2] [PATCH] MdeModulePkg: ScsiDiskDxe: adapt SectorCount when 
> shortening transfers
> 
> On 09/08/15 10:07, Paolo Bonzini wrote:
>>
>>
>> On 08/09/2015 09:02, Tian, Feng wrote:
>>> Hi, Laszlo
>>>
>>> 1. I don't think the code in VirtioScsi.c is correct. It hardcodes 
>>> the block size as 512bytes. The driver should send READ_CAPACITY cmd 
>>> to get block size, then convert it to InTransferLength & OutTransferLength.
>>
>> This is the controller's maximum transfer size, not the LUN.  The 
>> LUN's limits are available from VPD and depend on the block size.  The 
>> controller's limit apply to all LUNs and to all commands, even those 
>> that transfer bytes rather than blocks.
>>
>> For historical reasons the controller's maximum transfer size is 
>> provided in 512-byte units.  In fact, Laszlo raised this point to the 
>> virtio committee just to be sure that his code is correct, which it is.
> 
> Thanks.
> 
>>> 2. Although the UEFI spec doesn't say the relationship of 
>>> InTransferLength/OutTransferLength and CDB.TransferLength, but I 
>>> think it's implied that InTransferLength/OutTransferLength = 
>>> CDB.TransferLength * BlockSize. Otherwise how the lowest layer host 
>>> controller driver constructs transfer descriptor? How many bytes to 
>>> read? Is InTransferLength/OutTransferLength or the one of 
>>> CDB.TransferLength * BlockSize?
>>
>> It's always InTransferLength/OutTransferLength.  If it is < or > 
>> CDB.TransferLength * BlockSize you get respectively an underrun or an 
>> overrun.  An underrun is fatal, an overrun is not.
>>
>> InTransferLength/OutTransferLength exist because the controller may 
>> not know how to infer the transfer length for all CDB opcodes (some 
>> express it in bytes, some express it in blocks, some are vendor 
>> specific, some are just crazy and you have to inspect multiple CDB 
>> fields to compute the transfer length).
>>
>> Of course ScsiDisk *does* know how to infer the transfer length for 
>> the CDBs it builds.  Therefore, it makes sense for ScsiDisk's own READ 
>> and WRITE CDBs to always have InTransferLength/OutTransferLength = 
>> CDB.TransferLength * BlockSize.  However, the controller driver must 
>> not rely on this.
> 
> I'd like to add something here (which might be simply rephrasing what you 
> said):
> 
> I think Feng's question is about my former statement, which goes like:
> 
> In case the host controller (ie. not the LUN) is unable to satisfy
> the request, due to its size (InTransferLength/OutTransferLength)
> being too large, then it is allowed to return any kind of shorter
> InTransferLength/OutTransferLength, and the returned max transfer
> size *need not* be an integral multiple of any kind of block size.
> Ad absurdum, it 

[edk2] [patch] MdeModulePkg: PXE Driver's LoadFile protocol should check FilePath

[Zhang Lubo]

PXE driver's LoadFile protocol should check the input parameter
FilePath to see whether it's a supported device path.If not,
it should return invalid parameter, do not continue PXE boot.

Cc: Fu Siyuan
Cc: Ye Ting
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
---
 MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcImpl.c | 4 
 MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcImpl.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcImpl.c 
b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcImpl.c
index 4dd7944..72923f1 100644
--- a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcImpl.c
+++ b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcImpl.c
@@ -2774,10 +2774,14 @@ EfiPxeLoadFile (
   BOOLEAN NewMakeCallback;
   EFI_STATUS  Status;
   UINT64  TmpBufSize;
   BOOLEAN MediaPresent;
 
+  if (FilePath == NULL || !IsDevicePathEnd (FilePath)) {
+return EFI_INVALID_PARAMETER;
+  }
+  
   Private = PXEBC_PRIVATE_DATA_FROM_LOADFILE (This);
   PxeBc   = >PxeBc;
   NewMakeCallback = FALSE;
   Status  = EFI_DEVICE_ERROR;
 
diff --git a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcImpl.h 
b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcImpl.h
index 6e88dea..ce8d8ad 100644
--- a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcImpl.h
+++ b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcImpl.h
@@ -31,10 +31,11 @@ typedef struct _PXEBC_PRIVATE_DATA  PXEBC_PRIVATE_DATA;
 #include 
 #include 
 #include 
 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
-- 
1.9.5.msysgit.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] Why I get assert in PCD\Dxe\Service.c

[王晓峰]

Dear PCD module owner,
   I have meet an assert in PCDDXE UDK2014 revision. The Assert happens when 
PCDdxe initlize.
ASSERT e:\code\MdeModulePkg\Universal\PCD\Dxe\Service.c(1137): TokenNumber + 1 
< mPcdTotalTokenCount + 1
  it seems that build tool automatically generate PEI and DXE token number 
account , but what's the algorithm for this mPcdTotalTokenCount ?
  Which part should I check that may be wrong?





___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdePkg: Refine UefiFileHandleLib to avoid write non-ASCII char into ASCII file.

[Gao, Liming]

Reviewed-by: Liming Gao 

-Original Message-
From: Qiu, Shumin 
Sent: Tuesday, September 08, 2015 3:05 PM
To: edk2-devel@lists.01.org
Cc: Qiu, Shumin; Carsey, Jaben; Gao, Liming
Subject: [PATCH] MdePkg: Refine UefiFileHandleLib to avoid write non-ASCII char 
into ASCII file.

Cc: Jaben Carsey 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qiu Shumin 
---
 MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c 
b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
index 04a2f18..dfec5fa 100644
--- a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
+++ b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
@@ -1079,6 +1079,7 @@ FileHandleWriteLine(
   EFI_STATUS  Status;
   CHAR16  CharBuffer;
   UINTN   Size;
+  UINTN   Index;
   UINTN   CharSize;
   UINT64  FileSize;
   UINT64  OriginalFilePosition;
@@ -1136,6 +1137,12 @@ FileHandleWriteLine(
   return EFI_OUT_OF_RESOURCES;
 }
 UnicodeStrToAsciiStr (Buffer, AsciiBuffer);
+for (Index = 0; Index < Size; Index++) {
+  if (!((AsciiBuffer[Index] >= 0) && (AsciiBuffer[Index] < 128))){
+FreePool(AsciiBuffer);
+return EFI_INVALID_PARAMETER;
+  }
+}
 
 Size = AsciiStrSize(AsciiBuffer) - sizeof(CHAR8);
 Status = FileHandleWrite(Handle, , AsciiBuffer);
-- 
1.9.5.msysgit.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] OVMF BoF @ KVM Forum 2015

[Laszlo Ersek]

On 08/10/15 18:24, Laszlo Ersek wrote:
> Hi.
> 
> Let's do an OVMF BoF at this year's KVM Forum too.

Here's a preliminary task list, after some off-list discussion (I tried
to incorporate comments):

- create GPL'd fork called "ovmf" for expediting virt development
  (OvmfPkg, ArmVirtPkg)
  - maybe leverage the feature under
 for
setting up a separate "tianocore/edk2-gpl" repo, for GPL'd
contributions [Jordan]
- repo separation by license could make things harder for packagers
  and QEMU bundling [Laszlo]
- document the rules / justification for "ovmf" (licensing
  conflicts, non-technical blockage on edk2 etc).
- No new mailing list needed
- push RH's downstream-only patches to "ovmf", wherever that makes
  sense
- remove encumbered FAT driver
- import Peter Batard's GPL r/o FAT driver port of GRUB's
- secure OpenSSL linking exception for the former from the copyright
  holders (Peter Batard, GRUB project)
- "ovmf" should be periodically rebased / should fetch+merge edk2 as
  master (arguments both for and against merging); distros should
  then track "ovmf" as their upstream, not edk2
- get OVMF into Fedora (as pkg) and QEMU (as bundled binary)
- do OVMF releases, maybe in sync with QEMU's releases
  - we can probably build from known good revisions from git [Alex]
- revive Q35 SATA driver work / poke Reza
  - Hannes and Gabriel have refreshed patches, but their versions differ

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [patch] NetworkPkg: PXE Driver's LoadFile protocol should check FilePath

[Zhang Lubo]

PXE driver's LoadFile protocol should check the input parameter
FilePath to see whether it's a supported device path.If not,
it should return invalid parameter, do not continue PXE boot.

Cc: Fu Siyuan
Cc: Ye Ting
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
---
 NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c 
b/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c
index cdcf2f0..367a135 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c
@@ -2326,10 +2326,14 @@ EfiPxeLoadFile (
   EFI_PXE_BASE_CODE_PROTOCOL  *PxeBc;
   BOOLEAN UsingIpv6;
   EFI_STATUS  Status;
   BOOLEAN MediaPresent;
 
+  if (FilePath == NULL || !IsDevicePathEnd (FilePath)) {
+return EFI_INVALID_PARAMETER;
+  }
+  
   VirtualNic = PXEBC_VIRTUAL_NIC_FROM_LOADFILE (This);
   Private= VirtualNic->Private;
   PxeBc  = >PxeBc;
   UsingIpv6  = FALSE;
   Status = EFI_DEVICE_ERROR;
-- 
1.9.5.msysgit.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [patch] MdePkg/UefiScsiLib: comments update to add EFI_INVALID_PARAMETER status

[Zeng, Star]

On 2015/9/9 13:34, Tian Feng wrote:

EFI_SCSI_IO_PROTOCOL has alignment requirement on any data buffer
used in SCSI data transfer. As a wrap of this protocol, UefiScsiLib
have same request. Adding EFI_INVALID_PARAMETER return status in
function comments to ask the caller to guarantee this alignment.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Feng Tian 
Cc: Star Zeng 
---
  MdePkg/Include/Library/UefiScsiLib.h | 379 ++
  MdePkg/Library/UefiScsiLib/UefiScsiLib.c | 385 +++
  2 files changed, 469 insertions(+), 295 deletions(-)


Reviewed-by: Star Zeng 



diff --git a/MdePkg/Include/Library/UefiScsiLib.h 
b/MdePkg/Include/Library/UefiScsiLib.h
index 768a912..26e4aa4 100644
--- a/MdePkg/Include/Library/UefiScsiLib.h
+++ b/MdePkg/Include/Library/UefiScsiLib.h
@@ -5,7 +5,7 @@
for hard drive, CD and DVD devices that are the most common SCSI boot 
targets used by UEFI platforms.
This library class depends on SCSI I/O Protocol defined in UEFI 
Specification and SCSI-2 industry standard.

-Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
  This program and the accompanying materials
  are licensed and made available under the terms and conditions of the BSD 
License
  which accompanies this distribution.  The full text of the license may be 
found at
@@ -32,6 +32,9 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
If HostAdapterStatus is NULL, then ASSERT().
If TargetStatus is NULL, then ASSERT().

+  If SenseDataLength is non-zero and SenseData is not NULL, SenseData must 
meet buffer
+  alignment requirement defined in EFI_SCSI_IO_PROTOCOL. Otherwise 
EFI_INVALID_PARAMETER
+  gets returned.

@param[in] ScsiIo A pointer to the SCSI I/O Protocol 
instance
  for the specific SCSI target.
@@ -61,27 +64,28 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
  Protocol in the UEFI Specification for 
details on
  the possible return values.

-  @retval EFI_SUCCESS  The command was executed successfully.
-   See HostAdapterStatus, TargetStatus, 
SenseDataLength,
-   and SenseData in that order for additional 
status
-   information.
-  @retval EFI_NOT_READYThe SCSI Request Packet could not be sent 
because
-   there are too many SCSI Command Packets already
-   queued. The SCSI Request Packet was not sent, so
-   no additional status information is available.
-   The caller may retry again later.
-  @retval EFI_DEVICE_ERROR A device error occurred while attempting to send
-   SCSI Request Packet.  See HostAdapterStatus,
-   TargetStatus, SenseDataLength, and SenseData in 
that
-   order for additional status information.
-  @retval EFI_UNSUPPORTED  The command described by the SCSI Request Packet
-   is not supported by the SCSI initiator(i.e., 
SCSI
-   Host Controller). The SCSI Request Packet was 
not
-   sent, so no additional status information is 
available.
-  @retval EFI_TIMEOUT  A timeout occurred while waiting for the SCSI 
Request
-   Packet to execute.  See HostAdapterStatus, 
TargetStatus,
-   SenseDataLength, and SenseData in that order for
-   additional status information.
+  @retval EFI_SUCCESS   The command was executed successfully.
+See HostAdapterStatus, TargetStatus, 
SenseDataLength,
+and SenseData in that order for additional 
status
+information.
+  @retval EFI_NOT_READY The SCSI Request Packet could not be sent 
because
+there are too many SCSI Command Packets 
already
+queued. The SCSI Request Packet was not 
sent, so
+no additional status information is 
available.
+The caller may retry again later.
+  @retval EFI_DEVICE_ERROR  A device error occurred while attempting 
to send
+SCSI Request Packet.  See 
HostAdapterStatus,
+TargetStatus, SenseDataLength, and 
SenseData in that
+order for additional 

Re: [edk2] Section Alignment of elf binaries compiled with GCC(Linux)

[Michael Zimmermann]

Unfortunately I missed the replies but I debugged this problem further and
the Problem is that GenFw set's the alignment based on "sh_addralign" in
the Elf header.

The 'common-page-size' flag doesn't change the value of this field though.
what it does change is the Alignment value of the Program Headers:
Program Headers:
  Type   Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  LOAD   0x01 0x 0x 0x042f8 0x042f8 R E 0x1
  LOAD   0x0142f8 0x000142f8 0x000142f8 0x000d4 0x000f4 RW  0x1
  GNU_STACK  0x00 0x 0x 0x0 0x0 RWE 0x10

the section alignment still stays at 0/1/4.

On Tue, Aug 18, 2015 at 4:49 AM, Yao, Jiewen  wrote:

> OK
>
> -Original Message-
> From: Gao, Liming
> Sent: Tuesday, August 18, 2015 10:48 AM
> To: Yao, Jiewen; Michael Zimmermann; edk2-devel@lists.01.org
> Subject: RE: [edk2] Section Alignment of elf binaries compiled with
> GCC(Linux)
>
> Jiewen:
>   The updated message is useful. I suggest to change error level from
> EFI_D_ERROR to EFI_D_INFO.
>
> Thanks
> Liming
> -Original Message-
> From: Yao, Jiewen
> Sent: Tuesday, August 18, 2015 10:46 AM
> To: Gao, Liming; Michael Zimmermann; edk2-devel@lists.01.org
> Subject: RE: [edk2] Section Alignment of elf binaries compiled with
> GCC(Linux)
>
> Hi
> How about we update debug message like below:
>
> Index: PropertiesTable.c
> ===
> --- PropertiesTable.c   (revision 18191)
> +++ PropertiesTable.c   (working copy)
> @@ -1120,7 +1120,7 @@
>
>SetPropertiesTableSectionAlignment (SectionAlignment);
>if ((SectionAlignment & (EFI_ACPI_RUNTIME_PAGE_ALLOCATION_ALIGNMENT -
> 1)) != 0) {
> -DEBUG ((EFI_D_ERROR, "  InsertImageRecord - Section
> Alignment(0x%x) is not %dK  \n",
> +DEBUG ((EFI_D_ERROR, "  UEFI2.5 PropertiesTable - Runtime
> Driver Section Alignment(0x%x) is not %dK  \n",
>SectionAlignment, EFI_ACPI_RUNTIME_PAGE_ALLOCATION_ALIGNMENT >>
> 10));
>  PdbPointer = PeCoffLoaderGetPdbPointer ((VOID*) (UINTN) ImageAddress);
>  if (PdbPointer != NULL) {
>
>
>
>
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Gao, Liming
> Sent: Tuesday, August 18, 2015 10:39 AM
> To: Michael Zimmermann; edk2-devel@lists.01.org
> Subject: Re: [edk2] Section Alignment of elf binaries compiled with
> GCC(Linux)
>
> Hi,
>   This is a warning message that describes the runtime driver alignment is
> not 4K.
>
>   UEFI PropertiesTable table feature expects all runtime driver alignment
> is 4K. When DxeCore loads Runtime driver, it will check its alignment and
> report such warning message if it doesn't meet with the alignment. If you
> want to enable this feature, you need to make sure all runtime driver at
> 4K. If you don't enable it, you can just ignore this message.
>
>   To configure runtime driver with 4K alignment, you can modify DSC file
> to add the following section.
> [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
>   GCC:  *_*_*_DLINK_FLAGS = -z common-page-size=0x1000
>   MSFT: *_*_*_DLINK_FLAGS = /ALIGN:4096
>
> Thanks
> Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Michael Zimmermann
> Sent: Sunday, August 16, 2015 12:32 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] Section Alignment of elf binaries compiled with GCC(Linux)
>
> When booting(a new device I'm currently working on) I get these warnings:
>   InsertImageRecord - Section Alignment(0x20) is not 4K  
>
> the warning is raised by "MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c"
> and when compiling using GCC from Linux, the SectionAlignment is set by
> "BaseTools/Source/C/GenFw/Elf32Convert.c".
>
> I checked the resulting binaries using "readpe" and indeed they have a
> SectionAlignment of 0x20, while the precompiled binaries like Shell.efi
> have a Alignment of 0x1000(4K).
>
> So, is this a bug of my GCC compiler or of EDK2?
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg: add PartitionName protocol

[Michael Zimmermann]

UniquePartitionGUID isn't suitable because the device vendor's don't set
proper values(they're auto generated each time the partition table changes).
Instead names like "boot" are used so you can find the correct partition.

by "Implement it as a private protocol" do u just mean "use your own fork"
or is there some different protocol type?

On Wed, Sep 9, 2015 at 9:34 AM, Tian, Feng  wrote:

> Michael,
>
> IMHO, I don't think it's valuable to introducing this new protocol to get
> a name string. It's too burden. If only you have this use case, I would
> suggest you implement it as a private protocol rather than the public one.
>
> PS: Why UniquePartitionGUID doesn't work for your case?
>
> Thanks
> Feng
>
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Michael Zimmermann
> Sent: Wednesday, September 09, 2015 15:28
> To: Ni, Ruiyu
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] MdeModulePkg: add PartitionName protocol
>
> Hi,
>
> I need the partition name because I need to boot from partitions by their
> GPT label(filesystem label and GUID aren't set/reliable).
>
> Usually I would've just added a name field to the existing DiskIo
> Protocol, but since it's a UEFI standard we can't/shouldn't do this.
>
> I chose option 3 over 1/2 because the others would mean that u have to
> parse the whole partition table again which would lead to redundant code.
>
>
> Michael
>
> On Sun, Sep 6, 2015 at 8:33 AM, Ni, Ruiyu  wrote:
>
> > Zimmermann,
> > There are three options to get the name of a partition:
> > 1. Directly use BlockIo interface to access the GPT meta data.
> > 2. Group the above code into a library 3. Group the above code into a
> > driver and expose the partition name as a protocol.
> >
> > I really do not think we need to go to option 3 directly. Can you tell
> > us the reason why a protocol is needed?
> >
> > Thanks,
> > Ray
> >
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Michael Zimmermann
> > Sent: Thursday, September 3, 2015 2:23 PM
> > To: edk2-devel@lists.01.org
> > Subject: [edk2] [PATCH] MdeModulePkg: add PartitionName protocol
> >
> > This allows to retrieve the name of a partition.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Michael Zimmermann 
> > ---
> >  MdeModulePkg/Include/Protocol/PartitionName.h  | 42
> > ++
> >  MdeModulePkg/MdeModulePkg.dec  |  3 ++
> >  .../Universal/Disk/PartitionDxe/ElTorito.c |  3 +-
> >  MdeModulePkg/Universal/Disk/PartitionDxe/Gpt.c |  3 +-
> >  MdeModulePkg/Universal/Disk/PartitionDxe/Mbr.c |  6 ++--
> >  .../Universal/Disk/PartitionDxe/Partition.c| 13 ++-
> >  .../Universal/Disk/PartitionDxe/Partition.h|  5 ++-
> >  .../Universal/Disk/PartitionDxe/PartitionDxe.inf   |  2 ++
> >  8 files changed, 71 insertions(+), 6 deletions(-)  create mode 100644
> > MdeModulePkg/Include/Protocol/PartitionName.h
> >
> > diff --git a/MdeModulePkg/Include/Protocol/PartitionName.h
> > b/MdeModulePkg/Include/Protocol/PartitionName.h
> > new file mode 100644
> > index 000..460ab27
> > --- /dev/null
> > +++ b/MdeModulePkg/Include/Protocol/PartitionName.h
> > @@ -0,0 +1,42 @@
> > +#ifndef __PARTITION_NAME_H__
> > +#define __PARTITION_NAME_H__
> > +
> > +#include 
> > +
> > +#define EFI_PARTITION_NAME_PROTOCOL_GUID \
> > +  { \
> > +0xf3287350, 0x384a, 0x44ec, {0x95, 0xcf, 0xdb, 0x4b, 0x86, 0x7c,
> > +0x96,
> > 0x8c } \
> > +  }
> > +
> > +typedef struct _EFI_PARTITION_NAME_PROTOCOL
> > +EFI_PARTITION_NAME_PROTOCOL;
> > +
> > +///
> > +/// Protocol GUID name
> > +///
> > +#define PARTITION_NAME_PROTOCOL   EFI_PARTITION_NAME_PROTOCOL_GUID
> > +
> > +///
> > +/// Protocol
> > +///
> > +typedef EFI_PARTITION_NAME_PROTOCOL   EFI_PARTITION_NAME;
> > +
> > +#define EFI_PARTITION_NAME_PROTOCOL_REVISION  0x0001 #define
> > +EFI_PARTITION_NAME_PROTOCOL_REVISION2 0x00020001 #define
> > +EFI_PARTITION_NAME_PROTOCOL_REVISION3 0x00020031
> > +
> > +///
> > +/// Revision
> > +///
> > +#define EFI_PARTITION_NAME_INTERFACE_REVISION
> > EFI_PARTITION_NAME_PROTOCOL_REVISION
> > +
> > +///
> > +///  This protocol provides the name of a partition /// struct
> > +_EFI_PARTITION_NAME_PROTOCOL {
> > +  CHAR16  Name[36];
> > +};
> > +
> > +extern EFI_GUID gEfiPartitionNameProtocolGuid;
> > +
> > +#endif
> > +
> > diff --git a/MdeModulePkg/MdeModulePkg.dec
> > b/MdeModulePkg/MdeModulePkg.dec index 5c1d29a..290ecaf 100644
> > --- a/MdeModulePkg/MdeModulePkg.dec
> > +++ b/MdeModulePkg/MdeModulePkg.dec
> > @@ -449,6 +449,9 @@
> >## Include/Protocol/SmmReadyToBoot.h
> >gEdkiiSmmReadyToBootProtocolGuid = { 0x6e057ecf, 0xfa99, 0x4f39, {
> > 0x95, 0xbc, 0x59, 0xf9, 0x92, 0x1d, 0x17, 0xe4 } }
> >
> > +  ## Include/Protocol/PartitionName.h
> > +  gEfiPartitionNameProtocolGuid= { 0xf3287350, 

Re: [edk2] [PATCH] BaseTools/GenFw: align RVA of debug

[Ard Biesheuvel]

On 9 September 2015 at 11:46, Gao, Liming  wrote:
> Ard:
>   Dose this patch fix Section Alignment of elf binaries compiled with 
> GCC(Linux) reported by Michael Zimmermann ?
>

No. This fixes a problem on 32-bit ARM where the debug entry is
resolved before the MMU is up, which results in an alignment fault.

The issue you refer to is about the properties table memory protection
feature, right?


> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Wednesday, September 09, 2015 5:44 PM
> To: edk2-devel@lists.01.org; Gao, Liming; Liu, Yingke D
> Cc: leif.lindh...@linaro.org; ler...@redhat.com; Ard Biesheuvel
> Subject: [PATCH] BaseTools/GenFw: align RVA of debug
>
> SVN commit r18077 ("BaseTools/GenFw: move .debug contents to .data to save 
> space") removed the separate .debug section after moving its contents into 
> .text or .data. However, this change does not take into account that some of 
> these contents need to appear at a 32-bit aligned offset. So align the debug 
> data RVA to 32 bits.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  BaseTools/Source/C/GenFw/Elf32Convert.c | 13 +++--  
> BaseTools/Source/C/GenFw/Elf64Convert.c | 13 +++--
>  2 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/BaseTools/Source/C/GenFw/Elf32Convert.c 
> b/BaseTools/Source/C/GenFw/Elf32Convert.c
> index e1b92ebd713e..4b330a61669f 100644
> --- a/BaseTools/Source/C/GenFw/Elf32Convert.c
> +++ b/BaseTools/Source/C/GenFw/Elf32Convert.c
> @@ -218,6 +218,15 @@ CoffAlign (
>return (Offset + mCoffAlignment - 1) & ~(mCoffAlignment - 1);  }
>
> +STATIC
> +UINT32
> +DebugRvaAlign (
> +  UINT32 Offset
> +  )
> +{
> +  return (Offset + 3) & ~3;
> +}
> +
>  //
>  // filter functions
>  //
> @@ -365,7 +374,7 @@ ScanSections32 (
>  assert (FALSE);
>}
>
> -  mDebugOffset = mCoffOffset;
> +  mDebugOffset = DebugRvaAlign(mCoffOffset);
>
>if (mEhdr->e_machine != EM_ARM) {
>  mCoffOffset = CoffAlign(mCoffOffset); @@ -423,7 +432,7 @@ ScanSections32 
> (
>// section alignment.
>//
>if (SectionCount > 0) {
> -mDebugOffset = mCoffOffset;
> +mDebugOffset = DebugRvaAlign(mCoffOffset);
>}
>mCoffOffset = mDebugOffset + sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY) +
>  sizeof(EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY) + diff --git 
> a/BaseTools/Source/C/GenFw/Elf64Convert.c 
> b/BaseTools/Source/C/GenFw/Elf64Convert.c
> index c758ed9d64a6..a3f1f5d3d4c7 100644
> --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
> +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
> @@ -211,6 +211,15 @@ CoffAlign (
>return (Offset + mCoffAlignment - 1) & ~(mCoffAlignment - 1);  }
>
> +STATIC
> +UINT32
> +DebugRvaAlign (
> +  UINT32 Offset
> +  )
> +{
> +  return (Offset + 3) & ~3;
> +}
> +
>  //
>  // filter functions
>  //
> @@ -359,7 +368,7 @@ ScanSections64 (
>  assert (FALSE);
>}
>
> -  mDebugOffset = mCoffOffset;
> +  mDebugOffset = DebugRvaAlign(mCoffOffset);
>
>if (mEhdr->e_machine != EM_ARM) {
>  mCoffOffset = CoffAlign(mCoffOffset); @@ -412,7 +421,7 @@ ScanSections64 
> (
>// section alignment.
>//
>if (SectionCount > 0) {
> -mDebugOffset = mCoffOffset;
> +mDebugOffset = DebugRvaAlign(mCoffOffset);
>}
>mCoffOffset = mDebugOffset + sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY) +
>  sizeof(EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY) +
> --
> 1.9.1
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Xen-devel] OVMF/Xen, Debian wheezy can't boot with NX on stack (Was: Re: [PATCH] OvmfPkg: prevent code execution from DXE stack)

[Laszlo Ersek]

On 09/09/15 11:37, Ian Campbell wrote:
> On Wed, 2015-09-09 at 01:06 -0600, Jan Beulich wrote:
> On 09.09.15 at 00:23,  wrote:
>>> On 09/08/15 19:26, Anthony PERARD wrote:
 And I get this on the console:
 Welcome to GRUB!

  X64 Exception Type - 0E(#PF - Page-Fault)  CPU Apic ID -
  
 RIP  - 0F5F8918, CS  - 0028, RFLAGS -
 00210206
 ExceptionData - 0011
 RAX  - , RCX - 07FCE000, RDX -
 
 RBX  - 0B6092C0, RSP - 0F5F8590, RBP -
 0B608EA0
 RSI  - 0F5F8838, RDI - 0B608EA0
 R8   - , R9  - 0B609200, R10 -
 
 R11  - 000A, R12 - , R13 -
 001B
 R14  - 0B609360, R15 - 
 DS   - 0008, ES  - 0008, FS  -
 0008
 GS   - 0008, SS  - 0008
 CR0  - 8033, CR2 - 0F5F8918, CR3 -
 0F597000
 CR4  - 0668, CR8 - 
 DR0  - , DR1 - , DR2 -
 
 DR3  - , DR6 - 0FF0, DR7 -
 0400
 GDTR - 0F57BF18 003F, LDTR - 
 IDTR - 0EEA5018 0FFF,   TR - 
 FXSAVE_STATE - 0F5F81F0
  Find PE image 
>>> /build/xen-unstable/src/xen-unstable/tools/firmware/ovmf-dir
>>> -remote/Build
>>> /OvmfX64/DEBUG_GCC49/X64/IntelFrameworkModulePkg/Universal/StatusCode/R
>>> untime
>>> Dxe/StatusCodeRuntimeDxe/DEBUG/StatusCodeRuntimeDxe.dll 
>>> (ImageBase=0F556000, EntryPoint=0F55628F) 

 I did check with other guest (Windows, Ubuntu, Debian Jessie), and
 they are
 working correctly. Debian Wheezy is the only one that fail.
>>>
>>> I don't have an environment to reproduce this in. I think we should try
>>> to understand this problem better, before deciding how to make it go
>>> away.
>>>
>>> Please locate the "StatusCodeRuntimeDxe.debug" file in your Build
>>> directory (ie. under the location listed in the error report). Then,
>>> please disassemble it with "objdump -S". The fault location in the
>>> disassembly can be found based on RIP, ImageBase and EntryPoint;
>>
>> I don't think the exact instruction at that address really matters. The
>> main question appears to be why RIP and RSP both point into the
>> same page (see also the subject of Anthony's mail).
> 
> I'm not 100% what is going on,

me neither :)

> but if this (executable code on stack) is
> happening in grub is there something which is explicitly forbidden to UEFI
> apps by the UEFI spec?

Yes, there is. This small OvmfPkg patch only enables the edk2 feature
added by Star Zeng in
 for OVMF. That patch
(also referenced in my commit message by SVN rev) says,

This feature is added for UEFI spec that says
"Stack may be marked as non-executable in identity mapped page
tables".
A PCD PcdSetNxForStack is added to turn on/off this feature, and it
is FALSE by default.

A UEFI app runs (well, *starts*, anyway) before ExitBootServices() /
SetVirtualAddressMap(), so it's bound by the above.

The spec passage above is quoted from "2.3.2 IA-32 Platforms", and
"2.3.4 x64 Platforms", in chapter "2.3 Calling Conventions", where the
boot services time environment is specified.

This is new in UEFI-2.5, and it comes from Mantis ticket 1224: "Adding
support for No executable data areas".

... The question could be then if grub (in Wheezy) should be adapted to
UEFI-2.5 (if that's possible) or if OVMF should be built without this
feature.

Hmmm. Actually, I'm torn about the default for PcdSetNxForStack.

Namely, Mantis ticket 1224 has come up before. There's another edk2
sub-feature related to this UEFI spec feature / Mantis ticket; the
properties table (controlled by "PcdPropertiesTableEnable"), and the
effects it has on the UEFI memory map, and the requirements it presents
for UEFI OSes.

*That* sub-feature is extremely intrusive.
"MdeModulePkg/MdeModulePkg.dec" sets "PcdPropertiesTableEnable" TRUE by
default, and OvmfPkg inherits it. I have not overridden that default
just yet in OvmfPkg because the properties table feature depends on
something *else* too: sections in runtime DXE driver binaries must be
aligned at 4KB, and that is not ensured for OvmfPkg (yet?). Therefore,
the properties table feature is not active in OVMF at the moment due to
a "random build tools circumstance" -- and not because the feature flag
is actually disabled.

Now, the properties table feature absolutely cannot be (effectively)
enabled in OVMF as a default. It would break Windows 7 / Windows Server
2008 R2. A huge number of users run such guests for GPU passthrough.

The idea 

Re: [edk2] [Xen-devel] OVMF/Xen, Debian wheezy can't boot with NX on stack (Was: Re: [PATCH] OvmfPkg: prevent code execution from DXE stack)

[Paolo Bonzini]

On 09/09/2015 13:07, Ian Campbell wrote:
> I have a question: What attack vector is setting the stack as Nx in OVMF
> (or even UEFI generally) trying to protect against? Or is this being done
> for a reason other than security?
> 
> I understand why it is done for kernels and apps, but where does the
> untrusted element which is being protected against come from when running
> UEFI?

I guess something could attack shim.efi or GRUB, and subvert secure
boot's chain of trust.

Paolo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Xen-devel] OVMF/Xen, Debian wheezy can't boot with NX on stack (Was: Re: [PATCH] OvmfPkg: prevent code execution from DXE stack)

[Laszlo Ersek]

On 09/09/15 13:07, Ian Campbell wrote:
> On Wed, 2015-09-09 at 12:48 +0200, Laszlo Ersek wrote:
> 
> Thanks for all the info, I think I get it (although its not clear to me
> whether how an app can claim to be UEFI 2.5 capable and what the transition
> plan for legacy applications was going to be).
> 
>> ... The question could be then if grub (in Wheezy) should be adapted to
>> UEFI-2.5 (if that's possible)
> 
> I don't know either I'm afraid.
> 
> [...]
>> Hmmm. Actually, I'm torn about the default for PcdSetNxForStack.
> 
> I have a question: What attack vector is setting the stack as Nx in OVMF
> (or even UEFI generally) trying to protect against? Or is this being done
> for a reason other than security?

I think it is being done for security, generally speaking (ie. as far as
edk2 and the UEFI spec are concerned).

Specifically for OVMF, I wrote the patch because Paolo suggested it,
after (I think) he saw Star's feature patch on the edk2 mailing list. I
thought it was a good suggestion, even in case we had no particular
attack in mind: OVMF is frequently used as a UEFI development / test
bed, and we try to enable new edk2 / UEFI features in it, unless the
firmware size increase would be large, or something would break. (In
which cases we usually bracket the new feature with build time flags.)

I did consider regressions while writing this patch. In the commit
message I mentioned "-cpu ,-nx" as a way to turn of NX support in
the virtual CPU (which the edk2 feature dynamically detects and depends
on), should anything break. I did not expect two things: (a) old grub
actually executes stuff from the stack, (b) Xen has no way (?) to
disable NX in a VCPU (or maybe that would be detrimental for other reasons).

> I understand why it is done for kernels and apps, but where does the
> untrusted element which is being protected against come from when running
> UEFI?

I'm sure this is explained in the five ECR documents attached to Mantis
ticket 1224:

https://mantis.uefi.org/mantis/view.php?id=1224

Unfortunately, I don't think I can just publish those ECRs here (or dump
the mantis ticket).

I have no clue why UEFI development is so secretive. I had been annoyed
by not seeing mantis tickets myself (and thereby missing the
justification behind new UEFI features), so at some point I applied for
"non-voter, observer-only" membership in both the PIWG and the USWG, and
I got them. (I'm fuzzy on the details if this was helped by my being
@redhat.com -- it probably was.)

Perhaps you can also apply for membership (and read the ECRs on the
ticket), or hopefully Star (who implemented the feature) could summarize
the purpose here?

(I won't try, based on the ECRs, because that will unavoidably put me in
the position where I have to explain and defend the feature. Thanks, but
no, thanks :))

Cheers
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 1/2] MdeModulePkg: ScsiDiskDxe: recognize EFI_BAD_BUFFER_SIZE

[Laszlo Ersek]

Acting specifically upon this error condition from UefiScsiLib (and
ultimately from EFI_EXT_SCSI_PASS_THRU_PROTOCOL.PassThru()) in the

- ScsiDiskRead10(),
- ScsiDiskWrite10(),
- ScsiDiskRead16(),
- ScsiDiskWrite16()

functions allows us to retry these operations from ScsiDiskReadSectors()
and ScsiDiskWriteSectors(), with adjusted transfer sizes, without
investigating further error details like Host Adapter Status, Target
Status, and Sense Data.

Cc: Feng Tian 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---

Notes:
v2:
- new in v2

 MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c 
b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
index e7abe54..6da1751 100644
--- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
+++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
@@ -2107,7 +2107,7 @@ BackOff:
SectorCount
);
 
-  if (ReturnStatus == EFI_NOT_READY) {
+  if (ReturnStatus == EFI_NOT_READY || ReturnStatus == EFI_BAD_BUFFER_SIZE) {
 *NeedRetry = TRUE;
 return EFI_DEVICE_ERROR;
   } else if ((ReturnStatus == EFI_INVALID_PARAMETER) || (ReturnStatus == 
EFI_UNSUPPORTED)) {
@@ -2231,7 +2231,7 @@ BackOff:
StartLba,
SectorCount
);
-  if (ReturnStatus == EFI_NOT_READY) {
+  if (ReturnStatus == EFI_NOT_READY || ReturnStatus == EFI_BAD_BUFFER_SIZE) {
 *NeedRetry = TRUE;
 return EFI_DEVICE_ERROR;
   } else if ((ReturnStatus == EFI_INVALID_PARAMETER) || (ReturnStatus == 
EFI_UNSUPPORTED)) {
@@ -2354,7 +2354,7 @@ BackOff:
StartLba,
SectorCount
);
-  if (ReturnStatus == EFI_NOT_READY) {
+  if (ReturnStatus == EFI_NOT_READY || ReturnStatus == EFI_BAD_BUFFER_SIZE) {
 *NeedRetry = TRUE;
 return EFI_DEVICE_ERROR;
   } else if ((ReturnStatus == EFI_INVALID_PARAMETER) || (ReturnStatus == 
EFI_UNSUPPORTED)) {
@@ -2478,7 +2478,7 @@ BackOff:
StartLba,
SectorCount
);
-  if (ReturnStatus == EFI_NOT_READY) {
+  if (ReturnStatus == EFI_NOT_READY || ReturnStatus == EFI_BAD_BUFFER_SIZE) {
 *NeedRetry = TRUE;
 return EFI_DEVICE_ERROR;
   } else if ((ReturnStatus == EFI_INVALID_PARAMETER) || (ReturnStatus == 
EFI_UNSUPPORTED)) {
-- 
1.8.3.1


___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 0/2] MdeModulePkg: ScsiDiskDxe: handle EFI_BAD_BUFFER_SIZE from SCSI host adapter

[Laszlo Ersek]

No changes in either patch, relative to what has been discussed /
proposed in the v1 thread.

I *think* Feng's Reviewed-by in

already covers both patches, but I'm not 100% sure, therefore I'm
reposting in order for Feng to confirm.

Cc: Feng Tian 

Thanks!
Laszlo

Laszlo Ersek (2):
  MdeModulePkg: ScsiDiskDxe: recognize EFI_BAD_BUFFER_SIZE
  MdeModulePkg: ScsiDiskDxe: adapt SectorCount when shortening transfers

 MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c | 49 ++--
 1 file changed, 45 insertions(+), 4 deletions(-)

-- 
1.8.3.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 2/2] MdeModulePkg: ScsiDiskDxe: adapt SectorCount when shortening transfers

[Laszlo Ersek]

The specification of the EFI_EXT_SCSI_PASS_THRU_PROTOCOL.PassThru()
function documents the EFI_BAD_BUFFER_SIZE return status, and the
EFI_EXT_SCSI_STATUS_HOST_ADAPTER_DATA_OVERRUN_UNDERRUN host adapter
status.

These allow an EFI_EXT_SCSI_PASS_THRU_PROTOCOL implementation to request
higher layers in the stack (in this instance, UefiScsiLib and ScsiDiskDxe)
to break up the transfer into smaller pieces.

These conditions percolate up the stack correctly: the retry loops in
ScsiDiskDxe's ScsiDiskReadSectors() and ScsiDiskWriteSectors() functions
correctly and transparently update the transfer size (ByteCount),
accommodating any shortening requested by lower levels of the stack. After
the loop -- if the request ultimately succeeds -- SectorCount is even
recalculated from the final ByteCount, to see how many sectors the outer
loop should advance.

However, the inner (ie. retry) loops both have the same error: when the
underlying protocols request the transfer to be shortened, the decrease in
transfer size (ie. ByteCount) should immediately be reflected in
SectorCount. Otherwise the sector count encoded in the CDB will exceed the
transfer size, which is a permanent error.

This issue has been witnessed while booting

  en_windows_8.1_pro_n_vl_with_update_x86_dvd_6051127.iso

on the 32-bit build of OVMF, from a virtio-scsi CD-ROM:

(1) "cdboot.efi" correctly requested (from far atop) a long read:

Timeout=94000
CdbLength=10
DataDir=Read
InTransferLength=134215680
OutTransferLength=0
SenseDataLength=108

Cdb: 28 00 00 00 25 DD 00 FF FF 00
 ^ ^^^^
 |  ||
 |  | number of 2KB sectors to read,
 |  | corresponding to 2048 * 65535 = 134215680 bytes
 |  | (see InTransferLength above)
 |  |
 |   LBA to read from
 |
  READ (10)

(2) In turn, the EFI_EXT_SCSI_PASS_THRU_PROTOCOL.PassThru() function
provided by "OvmfPkg/VirtioScsiDxe/VirtioScsi.c" asked for the request
to be shortened:

InTransferLength=16776704
OutTransferLength=16776704
SenseDataLength=0
HostAdapterStatus=EFI_EXT_SCSI_STATUS_HOST_ADAPTER_DATA_OVERRUN_UNDERRUN
TargetStatus=0
Status=EFI_BAD_BUFFER_SIZE

(3) Then ScsiDiskReadSectors() in
"MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c" retried the request
with correctly shortened transfer length, but incorrectly unchanged
sector count:

Timeout=94000
CdbLength=10
DataDir=Read
InTransferLength=16776704 <--- updated as requested
OutTransferLength=0
SenseDataLength=108

Cdb: 28 00 00 00 25 DD 00 FF FF 00
 ^ ^^^^
 |  ||
 |  |   not changed!
 |  |
 |   LBA to read from
 |
  READ (10)

(4) Since 65535 sectors of 2KB each wouldn't fit in a buffer of approx.
16MB, QEMU's virtio-scsi controller unconditionally rejected this
request with VIRTIO_SCSI_S_OVERRUN, which VirtioScsiDxe then mapped
to:

InTransferLength=16776704
OutTransferLength=0
SenseDataLength=0
HostAdapterStatus=EFI_EXT_SCSI_STATUS_HOST_ADAPTER_DATA_OVERRUN_UNDERRUN
TargetStatus=0
Status=EFI_DEVICE_ERROR

(5) After two more tries of the same, ScsiDiskDxe passed up the error,
which ultimately caused "cdboot.efi" to BSOD.

Many thanks to Larry Cleeton from Microsoft for helping debug
"cdboot.efi".

Cc: Feng Tian 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---

Notes:
v2:
- unchanged
- normal patch formatting

v1:
- For ease of review, this patch has been formatted with the
  "--function-context" git option. Although including extra context can
  lead to unnecessary conflicts during patch application, I expect zero
  churn in this file in the meantime, so that aspect of
  "--function-context" should not be an issue. (In any case I intend to
  commit the patch myself, if Feng approves the patch.)

 MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c | 41 
 1 file changed, 41 insertions(+)

diff --git a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c 
b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
index 6da1751..282e9c2 100644
--- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
+++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
@@ -1789,6 +1789,7 @@ ScsiDiskReadSectors (
   UINT32  ByteCount;
   UINT32  MaxBlock;
   UINT32  SectorCount;
+  UINT32  NextSectorCount;
   UINT64  Timeout;
   EFI_STATUS  Status;
   UINT8   Index;
@@ -1889,6 +1890,25 @@ ScsiDiskReadSectors (
 return EFI_DEVICE_ERROR;
   }
 
+  //
+  // We need to retry. However, if ScsiDiskRead10() or ScsiDiskRead16() has
+  // lowered ByteCount on output, we 

Re: [edk2] [PATCH v2 5/5] ArmPkg/Mmu: Fix potential page table memory leak

[Ard Biesheuvel]

On 9 September 2015 at 11:53, Heyi Guo  wrote:
> During page entry attribute update, if there are table entries
> between starting BlockEntry and LastBlockEntry, table entries will be
> set as block entries and the allocated memory of the tables will be
> leaked.
>
> so we break the inner loop when we find a table entry and run outer
> loop again to step into the table by the same logic.
>

Another nice find!

> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Heyi Guo 
> Cc: Leif Lindholm 
> Cc: Ard Biesheuvel 
> ---
>  ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c 
> b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
> index 3be6b70..ae1834f 100644
> --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
> +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
> @@ -457,7 +457,11 @@ FillTranslationTable (
>RegionStart += BlockEntrySize;
>RemainingRegionLength -= BlockEntrySize;
>BlockEntry++;
> -} while ((RemainingRegionLength >= BlockEntrySize) && (BlockEntry <= 
> LastBlockEntry));
> +// Break the inner loop when next block is a table
> +// Rerun GetBlockEntryListFromAddress to avoid page table memory leak
> +} while ((RemainingRegionLength >= BlockEntrySize) &&
> + (BlockEntry <= LastBlockEntry) &&
> + !(TableLevel != 3 && (*BlockEntry & TT_TYPE_MASK) == 
> TT_TYPE_TABLE_ENTRY));

Could we get a 'if () break;' inside the loop instead of
folding it into the while() condition?

>} while (RemainingRegionLength != 0);
>
>return RETURN_SUCCESS;
> --
> 2.5.0
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Xen-devel] OVMF/Xen, Debian wheezy can't boot with NX on stack (Was: Re: [PATCH] OvmfPkg: prevent code execution from DXE stack)

[Laszlo Ersek]

On 09/09/15 13:30, Paolo Bonzini wrote:
> 
> 
> On 09/09/2015 13:07, Ian Campbell wrote:
>> I have a question: What attack vector is setting the stack as Nx in OVMF
>> (or even UEFI generally) trying to protect against? Or is this being done
>> for a reason other than security?
>>
>> I understand why it is done for kernels and apps, but where does the
>> untrusted element which is being protected against come from when running
>> UEFI?
> 
> I guess something could attack shim.efi or GRUB, and subvert secure
> boot's chain of trust.

... or the firmware could be fed some malicious data over the network,
when the fw (e.g. shim) boots off PXE (or, in case of edk2, HTTP), and a
buffer overflow could lead to the execution of arbitrary code?...


___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 2/5] ArmPkg/Mmu: Fix page level calculation bug

[Heyi Guo]

The bug can be triggered when alignment of Base is larger than Length
by 2 level of page granularity, e.g.

Base is 0x4000_, Length is 0x1000

The original code will change 2MB page level and we will get a
negative remaining length.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Heyi Guo 
Cc: Leif Lindholm 
Cc: Ard Biesheuvel 
Reviewed-by: Ard Biesheuvel 
---
 ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c 
b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
index b84f42a..d5d20a2 100644
--- a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
@@ -314,7 +314,7 @@ GetBlockEntryListFromAddress (
   // If the required size is smaller than the current block size then we need 
to go to the page below.
   // The PageLevel was calculated on the Base Address alignment but did not 
take in account the alignment
   // of the allocation size
-  if (*BlockEntrySize < TT_BLOCK_ENTRY_SIZE_AT_LEVEL (PageLevel)) {
+  while (*BlockEntrySize < TT_BLOCK_ENTRY_SIZE_AT_LEVEL (PageLevel)) {
 // It does not fit so we need to go a page level above
 PageLevel++;
   }
-- 
2.5.0

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 5/5] ArmPkg/Mmu: Fix potential page table memory leak

[Heyi Guo]

During page entry attribute update, if there are table entries
between starting BlockEntry and LastBlockEntry, table entries will be
set as block entries and the allocated memory of the tables will be
leaked.

so we break the inner loop when we find a table entry and run outer
loop again to step into the table by the same logic.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Heyi Guo 
Cc: Leif Lindholm 
Cc: Ard Biesheuvel 
---
 ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c 
b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
index 3be6b70..ae1834f 100644
--- a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
@@ -457,7 +457,11 @@ FillTranslationTable (
   RegionStart += BlockEntrySize;
   RemainingRegionLength -= BlockEntrySize;
   BlockEntry++;
-} while ((RemainingRegionLength >= BlockEntrySize) && (BlockEntry <= 
LastBlockEntry));
+// Break the inner loop when next block is a table
+// Rerun GetBlockEntryListFromAddress to avoid page table memory leak
+} while ((RemainingRegionLength >= BlockEntrySize) &&
+ (BlockEntry <= LastBlockEntry) &&
+ !(TableLevel != 3 && (*BlockEntry & TT_TYPE_MASK) == 
TT_TYPE_TABLE_ENTRY));
   } while (RemainingRegionLength != 0);
 
   return RETURN_SUCCESS;
-- 
2.5.0

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 1/5] ArmPkg/Mmu: Fix bug of aligning new allocated page table

[Heyi Guo]

The code has a simple bug on calculating aligned page table address.
We can just use AllocateAlignedPages in MemoryAllocationLib instead.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Heyi Guo 
Cc: Leif Lindholm 
Cc: Ard Biesheuvel 
---
 ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c 
b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
index 3d58d5d..b84f42a 100644
--- a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
@@ -377,11 +377,10 @@ GetBlockEntryListFromAddress (
 }
 
 // Create a new translation table
-TranslationTable = (UINT64*)AllocatePages 
(EFI_SIZE_TO_PAGES((TT_ENTRY_COUNT * sizeof(UINT64)) + 
TT_ALIGNMENT_DESCRIPTION_TABLE));
+TranslationTable = (UINT64*)AllocateAlignedPages 
(EFI_SIZE_TO_PAGES(TT_ENTRY_COUNT * sizeof(UINT64)), 
TT_ALIGNMENT_DESCRIPTION_TABLE);
 if (TranslationTable == NULL) {
   return NULL;
 }
-TranslationTable = (UINT64*)((UINTN)TranslationTable & 
TT_ADDRESS_MASK_DESCRIPTION_TABLE);
 
 // Populate the newly created lower level table
 SubTableBlockEntry = TranslationTable;
@@ -405,11 +404,10 @@ GetBlockEntryListFromAddress (
 //
 
 // Create a new translation table
-TranslationTable = (UINT64*)AllocatePages 
(EFI_SIZE_TO_PAGES((TT_ENTRY_COUNT * sizeof(UINT64)) + 
TT_ALIGNMENT_DESCRIPTION_TABLE));
+TranslationTable = (UINT64*)AllocateAlignedPages 
(EFI_SIZE_TO_PAGES(TT_ENTRY_COUNT * sizeof(UINT64)), 
TT_ALIGNMENT_DESCRIPTION_TABLE);
 if (TranslationTable == NULL) {
   return NULL;
 }
-TranslationTable = (UINT64*)((UINTN)TranslationTable & 
TT_ADDRESS_MASK_DESCRIPTION_TABLE);
 
 ZeroMem (TranslationTable, TT_ENTRY_COUNT * sizeof(UINT64));
 
@@ -619,12 +617,11 @@ ArmConfigureMmu (
   ArmSetTCR (TCR);
 
   // Allocate pages for translation table
-  TranslationTablePageCount = EFI_SIZE_TO_PAGES((RootTableEntryCount * 
sizeof(UINT64)) + TT_ALIGNMENT_DESCRIPTION_TABLE);
-  TranslationTable = AllocatePages (TranslationTablePageCount);
+  TranslationTablePageCount = EFI_SIZE_TO_PAGES(RootTableEntryCount * 
sizeof(UINT64));
+  TranslationTable = (UINT64*)AllocateAlignedPages (TranslationTablePageCount, 
TT_ALIGNMENT_DESCRIPTION_TABLE);
   if (TranslationTable == NULL) {
 return RETURN_OUT_OF_RESOURCES;
   }
-  TranslationTable = (VOID*)((UINTN)TranslationTable & 
TT_ADDRESS_MASK_DESCRIPTION_TABLE);
   // We set TTBR0 just after allocating the table to retrieve its location 
from the subsequent
   // functions without needing to pass this value across the functions. The 
MMU is only enabled
   // after the translation tables are populated.
-- 
2.5.0

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] BaseTools/GenFw: align RVA of debug

[Ard Biesheuvel]

SVN commit r18077 ("BaseTools/GenFw: move .debug contents to .data to
save space") removed the separate .debug section after moving its
contents into .text or .data. However, this change does not take into
account that some of these contents need to appear at a 32-bit aligned
offset. So align the debug data RVA to 32 bits.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 BaseTools/Source/C/GenFw/Elf32Convert.c | 13 +++--
 BaseTools/Source/C/GenFw/Elf64Convert.c | 13 +++--
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Source/C/GenFw/Elf32Convert.c 
b/BaseTools/Source/C/GenFw/Elf32Convert.c
index e1b92ebd713e..4b330a61669f 100644
--- a/BaseTools/Source/C/GenFw/Elf32Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf32Convert.c
@@ -218,6 +218,15 @@ CoffAlign (
   return (Offset + mCoffAlignment - 1) & ~(mCoffAlignment - 1);
 }
 
+STATIC
+UINT32
+DebugRvaAlign (
+  UINT32 Offset
+  )
+{
+  return (Offset + 3) & ~3;
+}
+
 //
 // filter functions
 //
@@ -365,7 +374,7 @@ ScanSections32 (
 assert (FALSE);
   }
 
-  mDebugOffset = mCoffOffset;
+  mDebugOffset = DebugRvaAlign(mCoffOffset);
 
   if (mEhdr->e_machine != EM_ARM) {
 mCoffOffset = CoffAlign(mCoffOffset);
@@ -423,7 +432,7 @@ ScanSections32 (
   // section alignment.
   //
   if (SectionCount > 0) {
-mDebugOffset = mCoffOffset;
+mDebugOffset = DebugRvaAlign(mCoffOffset);
   }
   mCoffOffset = mDebugOffset + sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY) +
 sizeof(EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY) +
diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c 
b/BaseTools/Source/C/GenFw/Elf64Convert.c
index c758ed9d64a6..a3f1f5d3d4c7 100644
--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -211,6 +211,15 @@ CoffAlign (
   return (Offset + mCoffAlignment - 1) & ~(mCoffAlignment - 1);
 }
 
+STATIC
+UINT32
+DebugRvaAlign (
+  UINT32 Offset
+  )
+{
+  return (Offset + 3) & ~3;
+}
+
 //
 // filter functions
 //
@@ -359,7 +368,7 @@ ScanSections64 (
 assert (FALSE);
   }
 
-  mDebugOffset = mCoffOffset;
+  mDebugOffset = DebugRvaAlign(mCoffOffset);
 
   if (mEhdr->e_machine != EM_ARM) {
 mCoffOffset = CoffAlign(mCoffOffset);
@@ -412,7 +421,7 @@ ScanSections64 (
   // section alignment.
   //
   if (SectionCount > 0) {
-mDebugOffset = mCoffOffset;
+mDebugOffset = DebugRvaAlign(mCoffOffset);
   }
   mCoffOffset = mDebugOffset + sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY) +
 sizeof(EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY) +
-- 
1.9.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] BaseTools/GenFw: align RVA of debug

[Ard Biesheuvel]

On 9 September 2015 at 11:54, Gao, Liming  wrote:
> Ard:
>   He reports that the 'common-page-size' flag doesn't work. When he 
> configures its value to 4096, the alignment in the converted EFI image is 
> still default 32. Could you help check it?
>
>   The detail report is attached below.
> The 'common-page-size' flag doesn't change the value of this field though. 
> what it does change is the Alignment value of the Program Headers:
> Program Headers:
>   Type   Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
>   LOAD   0x01 0x 0x 0x042f8 0x042f8 R E 0x1
>   LOAD   0x0142f8 0x000142f8 0x000142f8 0x000d4 0x000f4 RW  0x1
>   GNU_STACK  0x00 0x 0x 0x0 0x0 RWE 0x10
> the section alignment still stays at 0/1/4.
>

We should get confirmation that he is using the GCC linker script in
addition to setting -z common-page-size, since the builtin linker
script uses those macros in a slightly different way (as is obvious
from the readelf program header dump above)

-- 
Ard.


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard 
> Biesheuvel
> Sent: Wednesday, September 09, 2015 5:51 PM
> To: Gao, Liming
> Cc: edk2-devel@lists.01.org; ler...@redhat.com; Liu, Yingke D; 
> leif.lindh...@linaro.org
> Subject: Re: [edk2] [PATCH] BaseTools/GenFw: align RVA of debug
>
> On 9 September 2015 at 11:46, Gao, Liming  wrote:
>> Ard:
>>   Dose this patch fix Section Alignment of elf binaries compiled with 
>> GCC(Linux) reported by Michael Zimmermann ?
>>
>
> No. This fixes a problem on 32-bit ARM where the debug entry is resolved 
> before the MMU is up, which results in an alignment fault.
>
> The issue you refer to is about the properties table memory protection 
> feature, right?
>
>
>> -Original Message-
>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>> Sent: Wednesday, September 09, 2015 5:44 PM
>> To: edk2-devel@lists.01.org; Gao, Liming; Liu, Yingke D
>> Cc: leif.lindh...@linaro.org; ler...@redhat.com; Ard Biesheuvel
>> Subject: [PATCH] BaseTools/GenFw: align RVA of debug
>>
>> SVN commit r18077 ("BaseTools/GenFw: move .debug contents to .data to save 
>> space") removed the separate .debug section after moving its contents into 
>> .text or .data. However, this change does not take into account that some of 
>> these contents need to appear at a 32-bit aligned offset. So align the debug 
>> data RVA to 32 bits.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel 
>> ---
>>  BaseTools/Source/C/GenFw/Elf32Convert.c | 13 +++--
>> BaseTools/Source/C/GenFw/Elf64Convert.c | 13 +++--
>>  2 files changed, 22 insertions(+), 4 deletions(-)
>>
>> diff --git a/BaseTools/Source/C/GenFw/Elf32Convert.c
>> b/BaseTools/Source/C/GenFw/Elf32Convert.c
>> index e1b92ebd713e..4b330a61669f 100644
>> --- a/BaseTools/Source/C/GenFw/Elf32Convert.c
>> +++ b/BaseTools/Source/C/GenFw/Elf32Convert.c
>> @@ -218,6 +218,15 @@ CoffAlign (
>>return (Offset + mCoffAlignment - 1) & ~(mCoffAlignment - 1);  }
>>
>> +STATIC
>> +UINT32
>> +DebugRvaAlign (
>> +  UINT32 Offset
>> +  )
>> +{
>> +  return (Offset + 3) & ~3;
>> +}
>> +
>>  //
>>  // filter functions
>>  //
>> @@ -365,7 +374,7 @@ ScanSections32 (
>>  assert (FALSE);
>>}
>>
>> -  mDebugOffset = mCoffOffset;
>> +  mDebugOffset = DebugRvaAlign(mCoffOffset);
>>
>>if (mEhdr->e_machine != EM_ARM) {
>>  mCoffOffset = CoffAlign(mCoffOffset); @@ -423,7 +432,7 @@ 
>> ScanSections32 (
>>// section alignment.
>>//
>>if (SectionCount > 0) {
>> -mDebugOffset = mCoffOffset;
>> +mDebugOffset = DebugRvaAlign(mCoffOffset);
>>}
>>mCoffOffset = mDebugOffset + sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY) +
>>  sizeof(EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY) + diff
>> --git a/BaseTools/Source/C/GenFw/Elf64Convert.c
>> b/BaseTools/Source/C/GenFw/Elf64Convert.c
>> index c758ed9d64a6..a3f1f5d3d4c7 100644
>> --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
>> +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
>> @@ -211,6 +211,15 @@ CoffAlign (
>>return (Offset + mCoffAlignment - 1) & ~(mCoffAlignment - 1);  }
>>
>> +STATIC
>> +UINT32
>> +DebugRvaAlign (
>> +  UINT32 Offset
>> +  )
>> +{
>> +  return (Offset + 3) & ~3;
>> +}
>> +
>>  //
>>  // filter functions
>>  //
>> @@ -359,7 +368,7 @@ ScanSections64 (
>>  assert (FALSE);
>>}
>>
>> -  mDebugOffset = mCoffOffset;
>> +  mDebugOffset = DebugRvaAlign(mCoffOffset);
>>
>>if (mEhdr->e_machine != EM_ARM) {
>>  mCoffOffset = CoffAlign(mCoffOffset); @@ -412,7 +421,7 @@ 
>> ScanSections64 (
>>// section alignment.
>>//
>>if (SectionCount > 0) {
>> -mDebugOffset = mCoffOffset;
>> +mDebugOffset = DebugRvaAlign(mCoffOffset);
>>}
>>mCoffOffset = mDebugOffset + 

Re: [edk2] [PATCH] BaseTools/GenFw: align RVA of debug

[Gao, Liming]

Ard:
  Dose this patch fix Section Alignment of elf binaries compiled with 
GCC(Linux) reported by Michael Zimmermann ?

Thanks
Liming
-Original Message-
From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] 
Sent: Wednesday, September 09, 2015 5:44 PM
To: edk2-devel@lists.01.org; Gao, Liming; Liu, Yingke D
Cc: leif.lindh...@linaro.org; ler...@redhat.com; Ard Biesheuvel
Subject: [PATCH] BaseTools/GenFw: align RVA of debug

SVN commit r18077 ("BaseTools/GenFw: move .debug contents to .data to save 
space") removed the separate .debug section after moving its contents into 
.text or .data. However, this change does not take into account that some of 
these contents need to appear at a 32-bit aligned offset. So align the debug 
data RVA to 32 bits.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 BaseTools/Source/C/GenFw/Elf32Convert.c | 13 +++--  
BaseTools/Source/C/GenFw/Elf64Convert.c | 13 +++--
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Source/C/GenFw/Elf32Convert.c 
b/BaseTools/Source/C/GenFw/Elf32Convert.c
index e1b92ebd713e..4b330a61669f 100644
--- a/BaseTools/Source/C/GenFw/Elf32Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf32Convert.c
@@ -218,6 +218,15 @@ CoffAlign (
   return (Offset + mCoffAlignment - 1) & ~(mCoffAlignment - 1);  }
 
+STATIC
+UINT32
+DebugRvaAlign (
+  UINT32 Offset
+  )
+{
+  return (Offset + 3) & ~3;
+}
+
 //
 // filter functions
 //
@@ -365,7 +374,7 @@ ScanSections32 (
 assert (FALSE);
   }
 
-  mDebugOffset = mCoffOffset;
+  mDebugOffset = DebugRvaAlign(mCoffOffset);
 
   if (mEhdr->e_machine != EM_ARM) {
 mCoffOffset = CoffAlign(mCoffOffset); @@ -423,7 +432,7 @@ ScanSections32 (
   // section alignment.
   //
   if (SectionCount > 0) {
-mDebugOffset = mCoffOffset;
+mDebugOffset = DebugRvaAlign(mCoffOffset);
   }
   mCoffOffset = mDebugOffset + sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY) +
 sizeof(EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY) + diff --git 
a/BaseTools/Source/C/GenFw/Elf64Convert.c 
b/BaseTools/Source/C/GenFw/Elf64Convert.c
index c758ed9d64a6..a3f1f5d3d4c7 100644
--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -211,6 +211,15 @@ CoffAlign (
   return (Offset + mCoffAlignment - 1) & ~(mCoffAlignment - 1);  }
 
+STATIC
+UINT32
+DebugRvaAlign (
+  UINT32 Offset
+  )
+{
+  return (Offset + 3) & ~3;
+}
+
 //
 // filter functions
 //
@@ -359,7 +368,7 @@ ScanSections64 (
 assert (FALSE);
   }
 
-  mDebugOffset = mCoffOffset;
+  mDebugOffset = DebugRvaAlign(mCoffOffset);
 
   if (mEhdr->e_machine != EM_ARM) {
 mCoffOffset = CoffAlign(mCoffOffset); @@ -412,7 +421,7 @@ ScanSections64 (
   // section alignment.
   //
   if (SectionCount > 0) {
-mDebugOffset = mCoffOffset;
+mDebugOffset = DebugRvaAlign(mCoffOffset);
   }
   mCoffOffset = mDebugOffset + sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY) +
 sizeof(EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY) +
--
1.9.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 3/5] ArmPkg/Mmu: Fix literal number left shift bug

[Ard Biesheuvel]

On 9 September 2015 at 11:53, Heyi Guo  wrote:
> There is a hidden bug for below code:
>
> (1 << BaseAddressAlignment) & *BlockEntrySize
>
> From disassembly code, we can the literal number 1 will be treated as
> INT32 by compiler by default, and we'll get 0x8000 when
> BaseAddressAlignment is equal to 31. So we will always get 31 when
> alignment is larger than 31.
>
> if ((1 << BaseAddressAlignment) & *BlockEntrySize) {
> 5224: f9404be0  ldr x0, [sp,#144]
> 5228: 2a0003e1  mov w1, w0
> 522c: 52800020  mov w0, #0x1// #1
> 5230: 1ac12000  lsl w0, w0, w1
> 5234: 93407c01  sxtw  x1, w0
>
> The bug can be replayed on QEMU AARCH64; by adding some debug print,
> we can see lots of level 1 tables created (for block of 1GB) even
> when the region is large enough to use 512GB block size.
>
> Use LowBitSet64() in BaseLib instead to fix the bug.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Heyi Guo 
> Cc: Leif Lindholm 
> Cc: Ard Biesheuvel 

Reviewed-by: Ard Biesheuvel 

> ---
>  ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c | 16 
>  1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c 
> b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
> index d5d20a2..20f1a7b 100644
> --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
> +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
> @@ -276,8 +276,8 @@ GetBlockEntryListFromAddress (
>  return NULL;
>}
>
> -  // Ensure the required size is aligned on 4KB boundary
> -  if ((*BlockEntrySize & (SIZE_4KB - 1)) != 0) {
> +  // Ensure the required size is aligned on 4KB boundary and not 0
> +  if ((*BlockEntrySize & (SIZE_4KB - 1)) != 0 || *BlockEntrySize == 0) {
>  ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER);
>  return NULL;
>}
> @@ -294,18 +294,10 @@ GetBlockEntryListFromAddress (
>// If the start address is 0x0 then we use the size of the region to 
> identify the alignment
>if (RegionStart == 0) {
>  // Identify the highest possible alignment for the Region Size
> -for (BaseAddressAlignment = 0; BaseAddressAlignment < 64; 
> BaseAddressAlignment++) {
> -  if ((1 << BaseAddressAlignment) & *BlockEntrySize) {
> -break;
> -  }
> -}
> +BaseAddressAlignment = LowBitSet64 (*BlockEntrySize);
>} else {
>  // Identify the highest possible alignment for the Base Address
> -for (BaseAddressAlignment = 0; BaseAddressAlignment < 64; 
> BaseAddressAlignment++) {
> -  if ((1 << BaseAddressAlignment) & RegionStart) {
> -break;
> -  }
> -}
> +BaseAddressAlignment = LowBitSet64 (RegionStart);
>}
>
>// Identify the Page Level the RegionStart must belongs to
> --
> 2.5.0
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 1/5] ArmPkg/Mmu: Fix bug of aligning new allocated page table

[Ard Biesheuvel]

On 9 September 2015 at 11:53, Heyi Guo  wrote:
> The code has a simple bug on calculating aligned page table address.
> We can just use AllocateAlignedPages in MemoryAllocationLib instead.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Heyi Guo 
> Cc: Leif Lindholm 
> Cc: Ard Biesheuvel 

Reviewed-by: Ard Biesheuvel 

> ---
>  ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c | 11 ---
>  1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c 
> b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
> index 3d58d5d..b84f42a 100644
> --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
> +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
> @@ -377,11 +377,10 @@ GetBlockEntryListFromAddress (
>  }
>
>  // Create a new translation table
> -TranslationTable = (UINT64*)AllocatePages 
> (EFI_SIZE_TO_PAGES((TT_ENTRY_COUNT * sizeof(UINT64)) + 
> TT_ALIGNMENT_DESCRIPTION_TABLE));
> +TranslationTable = (UINT64*)AllocateAlignedPages 
> (EFI_SIZE_TO_PAGES(TT_ENTRY_COUNT * sizeof(UINT64)), 
> TT_ALIGNMENT_DESCRIPTION_TABLE);
>  if (TranslationTable == NULL) {
>return NULL;
>  }
> -TranslationTable = (UINT64*)((UINTN)TranslationTable & 
> TT_ADDRESS_MASK_DESCRIPTION_TABLE);
>
>  // Populate the newly created lower level table
>  SubTableBlockEntry = TranslationTable;
> @@ -405,11 +404,10 @@ GetBlockEntryListFromAddress (
>  //
>
>  // Create a new translation table
> -TranslationTable = (UINT64*)AllocatePages 
> (EFI_SIZE_TO_PAGES((TT_ENTRY_COUNT * sizeof(UINT64)) + 
> TT_ALIGNMENT_DESCRIPTION_TABLE));
> +TranslationTable = (UINT64*)AllocateAlignedPages 
> (EFI_SIZE_TO_PAGES(TT_ENTRY_COUNT * sizeof(UINT64)), 
> TT_ALIGNMENT_DESCRIPTION_TABLE);
>  if (TranslationTable == NULL) {
>return NULL;
>  }
> -TranslationTable = (UINT64*)((UINTN)TranslationTable & 
> TT_ADDRESS_MASK_DESCRIPTION_TABLE);
>
>  ZeroMem (TranslationTable, TT_ENTRY_COUNT * sizeof(UINT64));
>
> @@ -619,12 +617,11 @@ ArmConfigureMmu (
>ArmSetTCR (TCR);
>
>// Allocate pages for translation table
> -  TranslationTablePageCount = EFI_SIZE_TO_PAGES((RootTableEntryCount * 
> sizeof(UINT64)) + TT_ALIGNMENT_DESCRIPTION_TABLE);
> -  TranslationTable = AllocatePages (TranslationTablePageCount);
> +  TranslationTablePageCount = EFI_SIZE_TO_PAGES(RootTableEntryCount * 
> sizeof(UINT64));
> +  TranslationTable = (UINT64*)AllocateAlignedPages 
> (TranslationTablePageCount, TT_ALIGNMENT_DESCRIPTION_TABLE);
>if (TranslationTable == NULL) {
>  return RETURN_OUT_OF_RESOURCES;
>}
> -  TranslationTable = (VOID*)((UINTN)TranslationTable & 
> TT_ADDRESS_MASK_DESCRIPTION_TABLE);
>// We set TTBR0 just after allocating the table to retrieve its location 
> from the subsequent
>// functions without needing to pass this value across the functions. The 
> MMU is only enabled
>// after the translation tables are populated.
> --
> 2.5.0
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 1/3] ArmPlatformPkg/MemoryInitPeim: handle memory above 4 GB on 32-bit ARM

[Laszlo Ersek]

On 09/08/15 19:35, Ard Biesheuvel wrote:
> Make sure that the PEI memory region is carved out of memory that is
> 32-bit addressable, by taking MAX_ADDRESS into account (which is
> defined as '4 GB - 1' on ARM)
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c 
> b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
> index 93ab16ca4a74..9f26d26a04d3 100755
> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
> @@ -96,7 +96,7 @@ InitializeMemory (
>  {
>EFI_STATUSStatus;
>UINTN SystemMemoryBase;
> -  UINTN SystemMemoryTop;
> +  UINT64SystemMemoryTop;
>UINTN FdBase;
>UINTN FdTop;
>UINTN UefiMemoryBase;
> @@ -115,7 +115,10 @@ InitializeMemory (
>ASSERT (PcdGet64 (PcdSystemMemorySize) != 0);
>  
>SystemMemoryBase = (UINTN)PcdGet64 (PcdSystemMemoryBase);
> -  SystemMemoryTop = SystemMemoryBase + (UINTN)PcdGet64 (PcdSystemMemorySize);
> +  SystemMemoryTop = SystemMemoryBase + PcdGet64 (PcdSystemMemorySize);
> +  if (SystemMemoryTop - 1 > MAX_ADDRESS) {

MAX_ADDRESS is an inclusive limit. SystemMemoryTop is an exclusive limit
(it comes from base + size, where size is exclusive). In order to see if
clamping is needed, we should compare inclusive to inclusive, or
exclusive to exclusive, and act if SystemMemoryTop exceeds the maximum
(of the same kind). I usually prefer exclusives.

Mathematically (ie. regardless of C types etc) that would be:

  SystemMemoryTop > (MAX_ADDRESS + 1)

The RHS is problematic to evaluate in C. The way you solved it (by
subtracting 1 from both sides) is okay.

Also, this will never fire in the AARCH64 build.

> +SystemMemoryTop = (UINT64) MAX_ADDRESS + 1;

Please remove the space right after the cast operator, and repost (or
fix it up at commit time). With reference to:

http://thread.gmane.org/gmane.comp.bios.edk2.devel/1881/focus=1889

With that change:

Reviewed-by: Laszlo Ersek 

> +  }
>FdBase = (UINTN)PcdGet64 (PcdFdBaseAddress);
>FdTop = FdBase + (UINTN)PcdGet32 (PcdFdSize);
>  
> 

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 2/3] ArmVirtPkg/ArmVirtMemoryInitPeiLib: handle memory above 4 GB on 32-bit ARM

[Laszlo Ersek]

On 09/08/15 19:35, Ard Biesheuvel wrote:
> On 32-bit ARM, split system memory into a region below (and up to) 4 GB
> and a region above 4 GB. This is necessary to get the DXE core to consider
> the former as the resource descriptor that describes the primary memory
> region that also covers the PHIT region.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c | 31 
> 
>  1 file changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git 
> a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c 
> b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
> index 8ce63b4596e2..310d51b19358 100644
> --- a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
> +++ b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
> @@ -56,6 +56,7 @@ MemoryPeim (
>)
>  {
>EFI_RESOURCE_ATTRIBUTE_TYPE ResourceAttributes;
> +  UINT64  SystemMemoryTop;
>  
>// Ensure PcdSystemMemorySize has been set
>ASSERT (PcdGet64 (PcdSystemMemorySize) != 0);
> @@ -73,12 +74,30 @@ MemoryPeim (
>EFI_RESOURCE_ATTRIBUTE_TESTED
>);
>  
> -  BuildResourceDescriptorHob (
> -  EFI_RESOURCE_SYSTEM_MEMORY,
> -  ResourceAttributes,
> -  PcdGet64 (PcdSystemMemoryBase),
> -  PcdGet64 (PcdSystemMemorySize)
> -  );
> +  SystemMemoryTop = PcdGet64 (PcdSystemMemoryBase) +
> +PcdGet64 (PcdSystemMemorySize);
> +
> +  if (SystemMemoryTop - 1 > MAX_ADDRESS) {
> +BuildResourceDescriptorHob (
> +EFI_RESOURCE_SYSTEM_MEMORY,
> +ResourceAttributes,
> +PcdGet64 (PcdSystemMemoryBase),
> +(UINT64) MAX_ADDRESS - PcdGet64 (PcdSystemMemoryBase) + 1
> +);
> +BuildResourceDescriptorHob (
> +EFI_RESOURCE_SYSTEM_MEMORY,
> +ResourceAttributes,
> +(UINT64) MAX_ADDRESS + 1,

Please drop the space between the cast operator and its operand. With
that change:

Reviewed-by: Laszlo Ersek 

> +SystemMemoryTop - MAX_ADDRESS - 1
> +);
> +  } else {
> +BuildResourceDescriptorHob (
> +EFI_RESOURCE_SYSTEM_MEMORY,
> +ResourceAttributes,
> +PcdGet64 (PcdSystemMemoryBase),
> +PcdGet64 (PcdSystemMemorySize)
> +);
> +  }
>  
>//
>// When running under virtualization, the PI/UEFI memory region may be
> 

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 5/5] ArmPkg/Mmu: Fix potential page table memory leak

[Ard Biesheuvel]

On 9 September 2015 at 12:35, Ard Biesheuvel  wrote:
> On 9 September 2015 at 11:53, Heyi Guo  wrote:
>> During page entry attribute update, if there are table entries
>> between starting BlockEntry and LastBlockEntry, table entries will be
>> set as block entries and the allocated memory of the tables will be
>> leaked.
>>
>> so we break the inner loop when we find a table entry and run outer
>> loop again to step into the table by the same logic.
>>
>
> Another nice find!
>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Heyi Guo 
>> Cc: Leif Lindholm 
>> Cc: Ard Biesheuvel 
>> ---
>>  ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c | 6 +-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c 
>> b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
>> index 3be6b70..ae1834f 100644
>> --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
>> +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
>> @@ -457,7 +457,11 @@ FillTranslationTable (
>>RegionStart += BlockEntrySize;
>>RemainingRegionLength -= BlockEntrySize;
>>BlockEntry++;
>> -} while ((RemainingRegionLength >= BlockEntrySize) && (BlockEntry <= 
>> LastBlockEntry));
>> +// Break the inner loop when next block is a table
>> +// Rerun GetBlockEntryListFromAddress to avoid page table memory leak
>> +} while ((RemainingRegionLength >= BlockEntrySize) &&
>> + (BlockEntry <= LastBlockEntry) &&
>> + !(TableLevel != 3 && (*BlockEntry & TT_TYPE_MASK) == 
>> TT_TYPE_TABLE_ENTRY));
>
> Could we get a 'if () break;' inside the loop instead of
> folding it into the while() condition?
>

I fixed this up myself when committing.

Series committed as SVN r18421 .. r18425

Thanks!

-- 
Ard.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 3/3] ArmVirtPkg: set max physical address width to 40 bits

[Ard Biesheuvel]

On 9 September 2015 at 15:59, Laszlo Ersek  wrote:
> On 09/08/15 19:35, Ard Biesheuvel wrote:
>> When executing on a LPAE capable 32-bit ARM platform, we support
>> up to 40 bits of physical address space so set PcdPrePiCpuMemorySize
>> accordingly.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel 
>> ---
>>  ArmVirtPkg/ArmVirt.dsc.inc | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
>> index b49c1bfb8b04..c1b78be84e74 100644
>> --- a/ArmVirtPkg/ArmVirt.dsc.inc
>> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
>> @@ -371,6 +371,9 @@ [PcdsFixedAtBuild.common]
>>gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x80, 0x6d, 0x91, 0x7d, 
>> 0xb1, 0x5b, 0x8c, 0x45, 0xa4, 0x8f, 0xe2, 0x5f, 0xdd, 0x51, 0xef, 0x94}
>>  !endif
>>
>> +[PcdsFixedAtBuild.ARM]
>> +  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
>> +
>>  [Components.common]
>>#
>># Networking stack
>>
>
> Right, the "ArmPkg/Drivers/CpuPei/CpuPei.inf" builds the CPU HOB based
> on this, and then the DXE core determines the size of the GCD Memory
> Space Map's first (= "all is nonexistent") entry from that.
>

Indeed. Otherwise, memory above 4 GB just gets ignored, which means
its existence does not get advertised to the OS either.

> Plus, you probably don't have to care about any size increase in page
> tables that are allocated from the permanent PEI RAM (cf. SVN rev 17719).
>

The 1:1 mapping only goes to 4 GB, so anything beyond that is never
mapped anyway.

> Reviewed-by: Laszlo Ersek 

Thanks!
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 1/3] ArmPlatformPkg/MemoryInitPeim: handle memory above 4 GB on 32-bit ARM

[Leif Lindholm]

On Tue, Sep 08, 2015 at 07:35:40PM +0200, Ard Biesheuvel wrote:
> Make sure that the PEI memory region is carved out of memory that is
> 32-bit addressable, by taking MAX_ADDRESS into account (which is
> defined as '4 GB - 1' on ARM)
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c 
> b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
> index 93ab16ca4a74..9f26d26a04d3 100755
> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
> @@ -96,7 +96,7 @@ InitializeMemory (
>  {
>EFI_STATUSStatus;
>UINTN SystemMemoryBase;
> -  UINTN SystemMemoryTop;
> +  UINT64SystemMemoryTop;
>UINTN FdBase;
>UINTN FdTop;
>UINTN UefiMemoryBase;
> @@ -115,7 +115,10 @@ InitializeMemory (
>ASSERT (PcdGet64 (PcdSystemMemorySize) != 0);
>  
>SystemMemoryBase = (UINTN)PcdGet64 (PcdSystemMemoryBase);
> -  SystemMemoryTop = SystemMemoryBase + (UINTN)PcdGet64 (PcdSystemMemorySize);
> +  SystemMemoryTop = SystemMemoryBase + PcdGet64 (PcdSystemMemorySize);
> +  if (SystemMemoryTop - 1 > MAX_ADDRESS) {
> +SystemMemoryTop = (UINT64) MAX_ADDRESS + 1;
> +  }
>FdBase = (UINTN)PcdGet64 (PcdFdBaseAddress);
>FdTop = FdBase + (UINTN)PcdGet32 (PcdFdSize);
>  
> -- 
> 1.9.1

So, this is mainly an OCD thing on my end, but is there any real
benefit to _not_ converting SystemMemoryBase to 64-bit? Doing so would
get rid of the other UINTN cast.

Either way:
Reviewed-by: Leif Lindholm 

/
Leif
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Xen-devel] OVMF/Xen, Debian wheezy can't boot with NX on stack (Was: Re: [PATCH] OvmfPkg: prevent code execution from DXE stack)

[Laszlo Ersek]

On 09/09/15 14:08, Jan Beulich wrote:
 On 09.09.15 at 12:48,  wrote:
>> Personally I think that this dynamic approach is overkill (mainly
>> because I'm fine with being unable to install Debian Wheezy guests, both
>> wearing and not wearing my red fedora; and because the properties table
>> feature is not active for *any* OVMF guests anyway, in practice).
> 
> I can only guess that PCD stands for "Platform Configuration Data".

Yes, PCD stands for Platform Configuration Database.

> However, I would want to suggest an even more dynamic approach:
> Assuming that within the core UEFI code it ought to be possible to
> flip between executable and non-executable mapping of the stack,
> and considering that PE headers can carry target version numbers,
> how about reverting to an executable stack as long as there's at
> least one binary loaded that isn't claiming to be 2.5 compatible?

This would require very intrusive changes (to be implemented by people
other than me). Other concerns I have:

- I'm not sure if UEFI applications have any means to advertize what
  revision of the specification they target. (As you mention.)

- I could be mistaken, but this looks like it could weaken the
  protection offered by the platform feature. UEFI applications and
  UEFI drivers are expected to come from third parties. It could be
  argued that a non-compatible UEFI app or driver (like Wheezy's grub
  in this case) *should* preferably crash, immediately, in a controlled
  manner, instead of silently downgrading the protection for the stack,
  and thereby opening up an avenue for attacks.

  I'm not fully convinced about this, but it appears this should be
  controlled somewhere in the platform code. (Like a knob in the BIOS
  setup on physical platforms, or some enlighted info channel in
  guests.) Assuming we'd like it dynamic.

Thanks
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 3/3] ArmVirtPkg: set max physical address width to 40 bits

[Laszlo Ersek]

On 09/08/15 19:35, Ard Biesheuvel wrote:
> When executing on a LPAE capable 32-bit ARM platform, we support
> up to 40 bits of physical address space so set PcdPrePiCpuMemorySize
> accordingly.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmVirtPkg/ArmVirt.dsc.inc | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index b49c1bfb8b04..c1b78be84e74 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -371,6 +371,9 @@ [PcdsFixedAtBuild.common]
>gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x80, 0x6d, 0x91, 0x7d, 
> 0xb1, 0x5b, 0x8c, 0x45, 0xa4, 0x8f, 0xe2, 0x5f, 0xdd, 0x51, 0xef, 0x94}
>  !endif
>  
> +[PcdsFixedAtBuild.ARM]
> +  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
> +
>  [Components.common]
>#
># Networking stack
> 

Right, the "ArmPkg/Drivers/CpuPei/CpuPei.inf" builds the CPU HOB based
on this, and then the DXE core determines the size of the GCD Memory
Space Map's first (= "all is nonexistent") entry from that.

Plus, you probably don't have to care about any size increase in page
tables that are allocated from the permanent PEI RAM (cf. SVN rev 17719).

Reviewed-by: Laszlo Ersek 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 1/3] ArmPlatformPkg/MemoryInitPeim: handle memory above 4 GB on 32-bit ARM

[Leif Lindholm]

On Wed, Sep 09, 2015 at 05:11:33PM +0200, Ard Biesheuvel wrote:
> On 9 September 2015 at 16:53, Leif Lindholm  wrote:
> > On Wed, Sep 09, 2015 at 04:38:08PM +0200, Ard Biesheuvel wrote:
> >> On 9 September 2015 at 16:31, Leif Lindholm  
> >> wrote:
> >> > On Tue, Sep 08, 2015 at 07:35:40PM +0200, Ard Biesheuvel wrote:
> >> >> Make sure that the PEI memory region is carved out of memory that is
> >> >> 32-bit addressable, by taking MAX_ADDRESS into account (which is
> >> >> defined as '4 GB - 1' on ARM)
> >> >>
> >> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> >> Signed-off-by: Ard Biesheuvel 
> >> >> ---
> >> >>  ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c | 7 +--
> >> >>  1 file changed, 5 insertions(+), 2 deletions(-)
> >> >>
> >> >> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c 
> >> >> b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
> >> >> index 93ab16ca4a74..9f26d26a04d3 100755
> >> >> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
> >> >> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
> >> >> @@ -96,7 +96,7 @@ InitializeMemory (
> >> >>  {
> >> >>EFI_STATUSStatus;
> >> >>UINTN SystemMemoryBase;
> >> >> -  UINTN SystemMemoryTop;
> >> >> +  UINT64SystemMemoryTop;
> >> >>UINTN FdBase;
> >> >>UINTN FdTop;
> >> >>UINTN UefiMemoryBase;
> >> >> @@ -115,7 +115,10 @@ InitializeMemory (
> >> >>ASSERT (PcdGet64 (PcdSystemMemorySize) != 0);
> >> >>
> >> >>SystemMemoryBase = (UINTN)PcdGet64 (PcdSystemMemoryBase);
> >> >> -  SystemMemoryTop = SystemMemoryBase + (UINTN)PcdGet64 
> >> >> (PcdSystemMemorySize);
> >> >> +  SystemMemoryTop = SystemMemoryBase + PcdGet64 (PcdSystemMemorySize);
> >> >> +  if (SystemMemoryTop - 1 > MAX_ADDRESS) {
> >> >> +SystemMemoryTop = (UINT64) MAX_ADDRESS + 1;
> >> >> +  }
> >> >>FdBase = (UINTN)PcdGet64 (PcdFdBaseAddress);
> >> >>FdTop = FdBase + (UINTN)PcdGet32 (PcdFdSize);
> >> >>
> >> >> --
> >> >> 1.9.1
> >> >
> >> > So, this is mainly an OCD thing on my end, but is there any real
> >> > benefit to _not_ converting SystemMemoryBase to 64-bit? Doing so would
> >> > get rid of the other UINTN cast.
> >> >
> >>
> >> Not really, other than the fact that UINTN is more suitable for the
> >> base of DRAM, since you won't be able to run 32-bit UEFI if RAM starts
> >> above 4 GB anyway, so changing it to UINT64 seemed wrong to me.
> >
> > Sure, but having it 64-bit would permit an assert to noticeably tell
> > someone they had put an extra 0 into their Pcd, instead of
> > successfully passing this function and failing mysteriously later on.
> >
> 
> Shall I add this on top?
> 
> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
> b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
> index 25baac170c6c..e7880d30b1c8 100755
> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
> @@ -113,6 +113,7 @@ InitializeMemory (
> 
>// Ensure PcdSystemMemorySize has been set
>ASSERT (PcdGet64 (PcdSystemMemorySize) != 0);
> +  ASSERT (PcdGet64 (PcdSystemMemoryBase) < (UINT64)MAX_ADDRESS);
> 
>SystemMemoryBase = (UINTN)PcdGet64 (PcdSystemMemoryBase);
>SystemMemoryTop = SystemMemoryBase + PcdGet64 (PcdSystemMemorySize);

That would be lovely, thanks.

/
Leif
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Why I get assert in PCD\Dxe\Service.c

[Andrew Fish]

> On Sep 9, 2015, at 12:28 AM, 王晓峰  wrote:
> 
> Dear PCD module owner,
>   I have meet an assert in PCDDXE UDK2014 revision. The Assert happens when 
> PCDdxe initlize.
> ASSERT e:\code\MdeModulePkg\Universal\PCD\Dxe\Service.c(1137): TokenNumber + 
> 1 < mPcdTotalTokenCount + 1
>  it seems that build tool automatically generate PEI and DXE token number 
> account , but what's the algorithm for this mPcdTotalTokenCount ?
>  Which part should I check that may be wrong?
> 
> 

I’ve never seen this failure. I would guess you have some code doing a a call 
to the PCD Protocol to SET a PCD value and it is not valid from the PCD drivers 
database point of view. 
https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Protocol/Pcd.h 


The protocol may be abstracted by a library call: 
https://github.com/tianocore/edk2/tree/master/MdePkg/Library/DxePcdLib 


You can only used the build generated tokens for a platform that was compiled 
together in a single build. If you have other modules they need to use the Ex 
form that uses a GUID and fixed token number. The build generated tokens only 
exist to save space when all the code is compiled together. 

Thanks,

Andrew Fish

> 
> 
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.01.org_mailman_listinfo_edk2-2Ddevel=BQICAg=eEvniauFctOgLOKGJOplqw=1HnUuXD1wDvw67rut5_idw=Siy1WxHhaKOPlzUkx39CZuK_LN9QKHhZI3MnlF94MMQ=zNEWtbblF0A_50mppbQ1ugbmQTYIEJiV5TaAYuGVxUE=
>  

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 2/2] ShellPkg: Fix Shell fail with redundant space following delay number.

[Carsey, Jaben]

Reviewed-by: Jaben Carsey 

> -Original Message-
> From: Qiu, Shumin
> Sent: Sunday, September 06, 2015 8:06 PM
> To: edk2-devel@lists.01.org
> Cc: Qiu, Shumin ; Carsey, Jaben
> 
> Subject: [PATCH 2/2] ShellPkg: Fix Shell fail with redundant space following
> delay number.
> Importance: High
> 
> When boot from Shell we can use '-delay [num]' as optional data. If blank
> space exist after '[num]' Shell will fail. This patch add error handling to 
> avoid
> this failure.
> 
> Cc: Jaben Carsey 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Qiu Shumin 
> ---
>  ShellPkg/Application/Shell/Shell.h   | 11 +++
>  ShellPkg/Application/Shell/ShellParametersProtocol.c |  1 +
>  2 files changed, 12 insertions(+)
> 
> diff --git a/ShellPkg/Application/Shell/Shell.h
> b/ShellPkg/Application/Shell/Shell.h
> index 4d804fc..b668625 100644
> --- a/ShellPkg/Application/Shell/Shell.h
> +++ b/ShellPkg/Application/Shell/Shell.h
> @@ -360,5 +360,16 @@ FindFirstCharacter(
>IN CONST CHAR16 EscapeCharacter
>);
> 
> +/**
> +  Cleans off leading and trailing spaces and tabs.
> +
> +  @param[in] String pointer to the string to trim them off.
> +**/
> +EFI_STATUS
> +EFIAPI
> +TrimSpaces(
> +  IN CHAR16 **String
> +  );
> +
>  #endif //_SHELL_INTERNAL_HEADER_
> 
> diff --git a/ShellPkg/Application/Shell/ShellParametersProtocol.c
> b/ShellPkg/Application/Shell/ShellParametersProtocol.c
> index 1c1367b..bc19df7 100644
> --- a/ShellPkg/Application/Shell/ShellParametersProtocol.c
> +++ b/ShellPkg/Application/Shell/ShellParametersProtocol.c
> @@ -359,6 +359,7 @@ CreatePopulateInstallShellParametersProtocol (
>  //
>  // Populate Argc and Argv
>  //
> +TrimSpaces ();
>  Status = ParseCommandLineToArgs(FullCommandLine,
>  &(*NewShellParameters)->Argv,
>  &(*NewShellParameters)->Argc);
> --
> 1.9.5.msysgit.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] EDK II & GPL - Re: OVMF BoF @ KVM Forum 2015

[Laszlo Ersek]

On 09/09/15 18:17, Jordan Justen wrote:
> On 2015-09-09 01:57:51, Laszlo Ersek wrote:
>> On 08/10/15 18:24, Laszlo Ersek wrote:
>>> Hi.
>>>
>>> Let's do an OVMF BoF at this year's KVM Forum too.
>>
>> Here's a preliminary task list, after some off-list discussion (I tried
>> to incorporate comments):
>>
>> - create GPL'd fork called "ovmf" for expediting virt development
>>   (OvmfPkg, ArmVirtPkg)
>>   - maybe leverage the feature under
>>  for
>> setting up a separate "tianocore/edk2-gpl" repo, for GPL'd
>> contributions [Jordan]
>> - repo separation by license could make things harder for packagers
>>   and QEMU bundling [Laszlo]
> 
> I would like OVMF to follow a plan for GPL that the whole EDK II
> community decides on.
> 
> I would also like to see EDK II add a (permissively licensed; BSD,
> MIT, etc) DriverPkg (DriversPkg?). We discussed this on the list
> recently:
> 
> http://thread.gmane.org/gmane.comp.bios.tianocore.devel/17544/focus=676
> 
> So, related to this, I wonder how the community would feel about a
> GplDriverPkg. Would the community allow it as a new package in EDK II
> directly, or would a separate repo be required?
> 
> With regards to adding it directly into the EDK II tree, here are some
> potential concerns that I might anticipate hearing from the community:
> 
> * It will make it easier for contributors to choose GPL compared to a
>   permissive license, thereby limiting some users of the contribution
> 
> * GPL code will more easily be copied into the permissively licensed
>   packages
> 
> * Some might refuse to look at EDK II entirely if it has a directory
>   with GPL source code in it
> 
> Of these, I personally only sympathize with the first.
> 
> Regarding the separate OVMF repo, my hope is that it is more of a OVMF
> specific working area, rather than a 'GPL OVMF'. For example, patches
> or features that we've not yet figured out how to upstream, but we
> hopefully plan to.

Yes, the "OVMF specific working area" is also my main goal with this.

If we can find a way to (a) replace the FAT driver with the libre
software one, and (b) more generally, accept GPL drivers, without
depending on this separate OVMF repo, that's great. So "owning" those
modules is not a goal per se, just something the new repo should cover
in case we can't solve it within edk2. (E.g. with your GplDriverPkg idea.)

> Additionally, it makes sense to use it as needed for OVMF specific
> releases. (Ie, OVMF release tags)

Good idea.

Thanks!
Laszlo

> 
> -Jordan
> 
>> - document the rules / justification for "ovmf" (licensing
>>   conflicts, non-technical blockage on edk2 etc).
>> - No new mailing list needed
>> - push RH's downstream-only patches to "ovmf", wherever that makes
>>   sense
>> - remove encumbered FAT driver
>> - import Peter Batard's GPL r/o FAT driver port of GRUB's
>> - secure OpenSSL linking exception for the former from the copyright
>>   holders (Peter Batard, GRUB project)
>> - "ovmf" should be periodically rebased / should fetch+merge edk2 as
>>   master (arguments both for and against merging); distros should
>>   then track "ovmf" as their upstream, not edk2
>> - get OVMF into Fedora (as pkg) and QEMU (as bundled binary)
>> - do OVMF releases, maybe in sync with QEMU's releases
>>   - we can probably build from known good revisions from git [Alex]
>> - revive Q35 SATA driver work / poke Reza
>>   - Hannes and Gabriel have refreshed patches, but their versions differ
>>
>> ___
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Section Alignment of elf binaries compiled with GCC(Linux)

[Michael Zimmermann]

Yes I'm using 32bit ARM :)
thx for the patches - unfortunatelythe patches fail for me.

On Wed, Sep 9, 2015 at 5:33 PM, Ard Biesheuvel 
wrote:

> On 9 September 2015 at 17:26, Gao, Liming  wrote:
> > Michael:
> >   Do you use the linker script BaseTools/Scripts/GccBase.lds and -z
> common-page-size=4096?
> >
>
> Are you building for 32-bit ARM by any chance? That does not have this
> feature wired up yet. I posted a v2 of my series that addresses this
> yesterday.
>
>
>
> > From: Michael Zimmermann [mailto:sigmaepsilo...@gmail.com]
> > Sent: Wednesday, September 9, 2015 3:22 PM
> > To: Yao, Jiewen
> > Cc: Gao, Liming; edk2-devel@lists.01.org
> > Subject: Re: [edk2] Section Alignment of elf binaries compiled with
> GCC(Linux)
> >
> > Unfortunately I missed the replies but I debugged this problem further
> and the Problem is that GenFw set's the alignment based on "sh_addralign"
> in the Elf header.
> >
> > The 'common-page-size' flag doesn't change the value of this field
> though. what it does change is the Alignment value of the Program Headers:
> > Program Headers:
> >   Type   Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
> >   LOAD   0x01 0x 0x 0x042f8 0x042f8 R E
> 0x1
> >   LOAD   0x0142f8 0x000142f8 0x000142f8 0x000d4 0x000f4 RW
> 0x1
> >   GNU_STACK  0x00 0x 0x 0x0 0x0 RWE 0x10
> > the section alignment still stays at 0/1/4.
> >
> > On Tue, Aug 18, 2015 at 4:49 AM, Yao, Jiewen  > wrote:
> > OK
> >
> > -Original Message-
> > From: Gao, Liming
> > Sent: Tuesday, August 18, 2015 10:48 AM
> > To: Yao, Jiewen; Michael Zimmermann; edk2-devel@lists.01.org edk2-devel@lists.01.org>
> > Subject: RE: [edk2] Section Alignment of elf binaries compiled with
> GCC(Linux)
> >
> > Jiewen:
> >   The updated message is useful. I suggest to change error level from
> EFI_D_ERROR to EFI_D_INFO.
> >
> > Thanks
> > Liming
> > -Original Message-
> > From: Yao, Jiewen
> > Sent: Tuesday, August 18, 2015 10:46 AM
> > To: Gao, Liming; Michael Zimmermann; edk2-devel@lists.01.org edk2-devel@lists.01.org>
> > Subject: RE: [edk2] Section Alignment of elf binaries compiled with
> GCC(Linux)
> >
> > Hi
> > How about we update debug message like below:
> >
> > Index: PropertiesTable.c
> > ===
> > --- PropertiesTable.c   (revision 18191)
> > +++ PropertiesTable.c   (working copy)
> > @@ -1120,7 +1120,7 @@
> >
> >SetPropertiesTableSectionAlignment (SectionAlignment);
> >if ((SectionAlignment & (EFI_ACPI_RUNTIME_PAGE_ALLOCATION_ALIGNMENT -
> 1)) != 0) {
> > -DEBUG ((EFI_D_ERROR, "  InsertImageRecord - Section
> Alignment(0x%x) is not %dK  \n",
> > +DEBUG ((EFI_D_ERROR, "  UEFI2.5 PropertiesTable - Runtime
> Driver Section Alignment(0x%x) is not %dK  \n",
> >SectionAlignment, EFI_ACPI_RUNTIME_PAGE_ALLOCATION_ALIGNMENT >>
> 10));
> >  PdbPointer = PeCoffLoaderGetPdbPointer ((VOID*) (UINTN)
> ImageAddress);
> >  if (PdbPointer != NULL) {
> >
> >
> >
> >
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org edk2-devel-boun...@lists.01.org>] On Behalf Of Gao, Liming
> > Sent: Tuesday, August 18, 2015 10:39 AM
> > To: Michael Zimmermann; edk2-devel@lists.01.org edk2-devel@lists.01.org>
> > Subject: Re: [edk2] Section Alignment of elf binaries compiled with
> GCC(Linux)
> >
> > Hi,
> >   This is a warning message that describes the runtime driver alignment
> is not 4K.
> >
> >   UEFI PropertiesTable table feature expects all runtime driver
> alignment is 4K. When DxeCore loads Runtime driver, it will check its
> alignment and report such warning message if it doesn't meet with the
> alignment. If you want to enable this feature, you need to make sure all
> runtime driver at 4K. If you don't enable it, you can just ignore this
> message.
> >
> >   To configure runtime driver with 4K alignment, you can modify DSC file
> to add the following section.
> > [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
> >   GCC:  *_*_*_DLINK_FLAGS = -z common-page-size=0x1000
> >   MSFT: *_*_*_DLINK_FLAGS = /ALIGN:4096
> >
> > Thanks
> > Liming
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org edk2-devel-boun...@lists.01.org>] On Behalf Of Michael Zimmermann
> > Sent: Sunday, August 16, 2015 12:32 PM
> > To: edk2-devel@lists.01.org
> > Subject: [edk2] Section Alignment of elf binaries compiled with
> GCC(Linux)
> >
> > When booting(a new device I'm currently working on) I get these warnings:
> >   InsertImageRecord - Section Alignment(0x20) is not 4K  
> >
> > the warning is raised by "MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c"
> and when compiling using GCC from Linux, the SectionAlignment is set by

Re: [edk2] [PATCH 1/3] ArmPlatformPkg/MemoryInitPeim: handle memory above 4 GB on 32-bit ARM

[Andrew Fish]

> On Sep 9, 2015, at 7:38 AM, Ard Biesheuvel  wrote:
> 
> On 9 September 2015 at 16:31, Leif Lindholm  > wrote:
>> On Tue, Sep 08, 2015 at 07:35:40PM +0200, Ard Biesheuvel wrote:
>>> Make sure that the PEI memory region is carved out of memory that is
>>> 32-bit addressable, by taking MAX_ADDRESS into account (which is
>>> defined as '4 GB - 1' on ARM)
>>> 
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Ard Biesheuvel 
>>> ---
>>> ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c | 7 +--
>>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c 
>>> b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
>>> index 93ab16ca4a74..9f26d26a04d3 100755
>>> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
>>> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
>>> @@ -96,7 +96,7 @@ InitializeMemory (
>>> {
>>>   EFI_STATUSStatus;
>>>   UINTN SystemMemoryBase;
>>> -  UINTN SystemMemoryTop;
>>> +  UINT64SystemMemoryTop;
>>>   UINTN FdBase;
>>>   UINTN FdTop;
>>>   UINTN UefiMemoryBase;
>>> @@ -115,7 +115,10 @@ InitializeMemory (
>>>   ASSERT (PcdGet64 (PcdSystemMemorySize) != 0);
>>> 
>>>   SystemMemoryBase = (UINTN)PcdGet64 (PcdSystemMemoryBase);
>>> -  SystemMemoryTop = SystemMemoryBase + (UINTN)PcdGet64 
>>> (PcdSystemMemorySize);
>>> +  SystemMemoryTop = SystemMemoryBase + PcdGet64 (PcdSystemMemorySize);
>>> +  if (SystemMemoryTop - 1 > MAX_ADDRESS) {
>>> +SystemMemoryTop = (UINT64) MAX_ADDRESS + 1;
>>> +  }
>>>   FdBase = (UINTN)PcdGet64 (PcdFdBaseAddress);
>>>   FdTop = FdBase + (UINTN)PcdGet32 (PcdFdSize);
>>> 
>>> --
>>> 1.9.1
>> 
>> So, this is mainly an OCD thing on my end, but is there any real
>> benefit to _not_ converting SystemMemoryBase to 64-bit? Doing so would
>> get rid of the other UINTN cast.
>> 
> 
> Not really, other than the fact that UINTN is more suitable for the
> base of DRAM, since you won't be able to run 32-bit UEFI if RAM starts
> above 4 GB anyway, so changing it to UINT64 seemed wrong to me.
> 

We have traditionally use EFI_PHYSICAL_ADDRESS for things that can be a system 
address. On x86 PEI can be 32-bit and DXE 64-bit. Not to mention 32-bit x86 
supports a larger physical address space than virtual address space. So for 
example the EFI memory map for a 32-bit system can return memory > 4GB. 

Thanks,

Andrew Fish


>> Either way:
>> Reviewed-by: Leif Lindholm > >
>> 
> 
> Thanks!
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org 
> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.01.org_mailman_listinfo_edk2-2Ddevel=BQICAg=eEvniauFctOgLOKGJOplqw=1HnUuXD1wDvw67rut5_idw=wRwkH_9UpA6VbYV7sByhOHwzStCaABHrZXMc611QIbw=wonkQazyiyGXX348zzGwzDdWaTSpa83YZKuqP4TEcfM=
>  
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Section Alignment of elf binaries compiled with GCC(Linux)

[Michael Zimmermann]

what I meant is that git am fails:
Applying: BaseTools/GenFw: remove ARM and RVCT references from ELF64 code
error: patch failed: BaseTools/Source/C/GenFw/Elf64Convert.c:360
error: BaseTools/Source/C/GenFw/Elf64Convert.c: patch does not apply
Patch failed at 0001 BaseTools/GenFw: remove ARM and RVCT references from
ELF64 code
The copy of the patch that failed is found in:
   /media/Data/repositories/git/efidroid/uefi/edk2/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

maybe I did sth. wrong?(I've never used a mailing list patch before). I
downloaded the mbox from gmane:
http://download.gmane.org/gmane.comp.bios.edk2.devel/2036/2037

On Wed, Sep 9, 2015 at 6:17 PM, Ard Biesheuvel 
wrote:

>
>
> On 9 sep. 2015, at 18:10, Michael Zimmermann 
> wrote:
>
> Yes I'm using 32bit ARM :)
> thx for the patches - unfortunatelythe patches fail for me.
>
>
> Did you regenerate Conf/tools_def.txt  and rebuild the BaseTools/ ?
>
> On Wed, Sep 9, 2015 at 5:33 PM, Ard Biesheuvel 
> wrote:
>
>> On 9 September 2015 at 17:26, Gao, Liming  wrote:
>> > Michael:
>> >   Do you use the linker script BaseTools/Scripts/GccBase.lds and -z
>> common-page-size=4096?
>> >
>>
>> Are you building for 32-bit ARM by any chance? That does not have this
>> feature wired up yet. I posted a v2 of my series that addresses this
>> yesterday.
>>
>>
>>
>> > From: Michael Zimmermann [mailto:sigmaepsilo...@gmail.com]
>> > Sent: Wednesday, September 9, 2015 3:22 PM
>> > To: Yao, Jiewen
>> > Cc: Gao, Liming; edk2-devel@lists.01.org
>> > Subject: Re: [edk2] Section Alignment of elf binaries compiled with
>> GCC(Linux)
>> >
>> > Unfortunately I missed the replies but I debugged this problem further
>> and the Problem is that GenFw set's the alignment based on "sh_addralign"
>> in the Elf header.
>> >
>> > The 'common-page-size' flag doesn't change the value of this field
>> though. what it does change is the Alignment value of the Program Headers:
>> > Program Headers:
>> >   Type   Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg
>> Align
>> >   LOAD   0x01 0x 0x 0x042f8 0x042f8 R E
>> 0x1
>> >   LOAD   0x0142f8 0x000142f8 0x000142f8 0x000d4 0x000f4 RW
>> 0x1
>> >   GNU_STACK  0x00 0x 0x 0x0 0x0 RWE 0x10
>> > the section alignment still stays at 0/1/4.
>> >
>> > On Tue, Aug 18, 2015 at 4:49 AM, Yao, Jiewen > > wrote:
>> > OK
>> >
>> > -Original Message-
>> > From: Gao, Liming
>> > Sent: Tuesday, August 18, 2015 10:48 AM
>> > To: Yao, Jiewen; Michael Zimmermann; edk2-devel@lists.01.org> edk2-devel@lists.01.org>
>> > Subject: RE: [edk2] Section Alignment of elf binaries compiled with
>> GCC(Linux)
>> >
>> > Jiewen:
>> >   The updated message is useful. I suggest to change error level from
>> EFI_D_ERROR to EFI_D_INFO.
>> >
>> > Thanks
>> > Liming
>> > -Original Message-
>> > From: Yao, Jiewen
>> > Sent: Tuesday, August 18, 2015 10:46 AM
>> > To: Gao, Liming; Michael Zimmermann; edk2-devel@lists.01.org> edk2-devel@lists.01.org>
>> > Subject: RE: [edk2] Section Alignment of elf binaries compiled with
>> GCC(Linux)
>> >
>> > Hi
>> > How about we update debug message like below:
>> >
>> > Index: PropertiesTable.c
>> > ===
>> > --- PropertiesTable.c   (revision 18191)
>> > +++ PropertiesTable.c   (working copy)
>> > @@ -1120,7 +1120,7 @@
>> >
>> >SetPropertiesTableSectionAlignment (SectionAlignment);
>> >if ((SectionAlignment & (EFI_ACPI_RUNTIME_PAGE_ALLOCATION_ALIGNMENT
>> - 1)) != 0) {
>> > -DEBUG ((EFI_D_ERROR, "  InsertImageRecord - Section
>> Alignment(0x%x) is not %dK  \n",
>> > +DEBUG ((EFI_D_ERROR, "  UEFI2.5 PropertiesTable - Runtime
>> Driver Section Alignment(0x%x) is not %dK  \n",
>> >SectionAlignment, EFI_ACPI_RUNTIME_PAGE_ALLOCATION_ALIGNMENT >>
>> 10));
>> >  PdbPointer = PeCoffLoaderGetPdbPointer ((VOID*) (UINTN)
>> ImageAddress);
>> >  if (PdbPointer != NULL) {
>> >
>> >
>> >
>> >
>> > -Original Message-
>> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org> edk2-devel-boun...@lists.01.org>] On Behalf Of Gao, Liming
>> > Sent: Tuesday, August 18, 2015 10:39 AM
>> > To: Michael Zimmermann; edk2-devel@lists.01.org> edk2-devel@lists.01.org>
>> > Subject: Re: [edk2] Section Alignment of elf binaries compiled with
>> GCC(Linux)
>> >
>> > Hi,
>> >   This is a warning message that describes the runtime driver alignment
>> is not 4K.
>> >
>> >   UEFI PropertiesTable table feature expects all runtime driver
>> alignment is 4K. When DxeCore loads Runtime driver, it will check its
>> 

Re: [edk2] [PATCH 1/3] ArmPlatformPkg/MemoryInitPeim: handle memory above 4 GB on 32-bit ARM

[Leif Lindholm]

On Wed, Sep 09, 2015 at 04:38:08PM +0200, Ard Biesheuvel wrote:
> On 9 September 2015 at 16:31, Leif Lindholm  wrote:
> > On Tue, Sep 08, 2015 at 07:35:40PM +0200, Ard Biesheuvel wrote:
> >> Make sure that the PEI memory region is carved out of memory that is
> >> 32-bit addressable, by taking MAX_ADDRESS into account (which is
> >> defined as '4 GB - 1' on ARM)
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Ard Biesheuvel 
> >> ---
> >>  ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c | 7 +--
> >>  1 file changed, 5 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c 
> >> b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
> >> index 93ab16ca4a74..9f26d26a04d3 100755
> >> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
> >> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
> >> @@ -96,7 +96,7 @@ InitializeMemory (
> >>  {
> >>EFI_STATUSStatus;
> >>UINTN SystemMemoryBase;
> >> -  UINTN SystemMemoryTop;
> >> +  UINT64SystemMemoryTop;
> >>UINTN FdBase;
> >>UINTN FdTop;
> >>UINTN UefiMemoryBase;
> >> @@ -115,7 +115,10 @@ InitializeMemory (
> >>ASSERT (PcdGet64 (PcdSystemMemorySize) != 0);
> >>
> >>SystemMemoryBase = (UINTN)PcdGet64 (PcdSystemMemoryBase);
> >> -  SystemMemoryTop = SystemMemoryBase + (UINTN)PcdGet64 
> >> (PcdSystemMemorySize);
> >> +  SystemMemoryTop = SystemMemoryBase + PcdGet64 (PcdSystemMemorySize);
> >> +  if (SystemMemoryTop - 1 > MAX_ADDRESS) {
> >> +SystemMemoryTop = (UINT64) MAX_ADDRESS + 1;
> >> +  }
> >>FdBase = (UINTN)PcdGet64 (PcdFdBaseAddress);
> >>FdTop = FdBase + (UINTN)PcdGet32 (PcdFdSize);
> >>
> >> --
> >> 1.9.1
> >
> > So, this is mainly an OCD thing on my end, but is there any real
> > benefit to _not_ converting SystemMemoryBase to 64-bit? Doing so would
> > get rid of the other UINTN cast.
> >
> 
> Not really, other than the fact that UINTN is more suitable for the
> base of DRAM, since you won't be able to run 32-bit UEFI if RAM starts
> above 4 GB anyway, so changing it to UINT64 seemed wrong to me.

Sure, but having it 64-bit would permit an assert to noticeably tell
someone they had put an extra 0 into their Pcd, instead of
successfully passing this function and failing mysteriously later on.

> > Either way:
> > Reviewed-by: Leif Lindholm 

/
Leif
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 1/3] ArmPlatformPkg/MemoryInitPeim: handle memory above 4 GB on 32-bit ARM

[Ard Biesheuvel]

On 9 September 2015 at 16:53, Leif Lindholm  wrote:
> On Wed, Sep 09, 2015 at 04:38:08PM +0200, Ard Biesheuvel wrote:
>> On 9 September 2015 at 16:31, Leif Lindholm  wrote:
>> > On Tue, Sep 08, 2015 at 07:35:40PM +0200, Ard Biesheuvel wrote:
>> >> Make sure that the PEI memory region is carved out of memory that is
>> >> 32-bit addressable, by taking MAX_ADDRESS into account (which is
>> >> defined as '4 GB - 1' on ARM)
>> >>
>> >> Contributed-under: TianoCore Contribution Agreement 1.0
>> >> Signed-off-by: Ard Biesheuvel 
>> >> ---
>> >>  ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c | 7 +--
>> >>  1 file changed, 5 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c 
>> >> b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
>> >> index 93ab16ca4a74..9f26d26a04d3 100755
>> >> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
>> >> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
>> >> @@ -96,7 +96,7 @@ InitializeMemory (
>> >>  {
>> >>EFI_STATUSStatus;
>> >>UINTN SystemMemoryBase;
>> >> -  UINTN SystemMemoryTop;
>> >> +  UINT64SystemMemoryTop;
>> >>UINTN FdBase;
>> >>UINTN FdTop;
>> >>UINTN UefiMemoryBase;
>> >> @@ -115,7 +115,10 @@ InitializeMemory (
>> >>ASSERT (PcdGet64 (PcdSystemMemorySize) != 0);
>> >>
>> >>SystemMemoryBase = (UINTN)PcdGet64 (PcdSystemMemoryBase);
>> >> -  SystemMemoryTop = SystemMemoryBase + (UINTN)PcdGet64 
>> >> (PcdSystemMemorySize);
>> >> +  SystemMemoryTop = SystemMemoryBase + PcdGet64 (PcdSystemMemorySize);
>> >> +  if (SystemMemoryTop - 1 > MAX_ADDRESS) {
>> >> +SystemMemoryTop = (UINT64) MAX_ADDRESS + 1;
>> >> +  }
>> >>FdBase = (UINTN)PcdGet64 (PcdFdBaseAddress);
>> >>FdTop = FdBase + (UINTN)PcdGet32 (PcdFdSize);
>> >>
>> >> --
>> >> 1.9.1
>> >
>> > So, this is mainly an OCD thing on my end, but is there any real
>> > benefit to _not_ converting SystemMemoryBase to 64-bit? Doing so would
>> > get rid of the other UINTN cast.
>> >
>>
>> Not really, other than the fact that UINTN is more suitable for the
>> base of DRAM, since you won't be able to run 32-bit UEFI if RAM starts
>> above 4 GB anyway, so changing it to UINT64 seemed wrong to me.
>
> Sure, but having it 64-bit would permit an assert to noticeably tell
> someone they had put an extra 0 into their Pcd, instead of
> successfully passing this function and failing mysteriously later on.
>

Shall I add this on top?

diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
index 25baac170c6c..e7880d30b1c8 100755
--- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
+++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
@@ -113,6 +113,7 @@ InitializeMemory (

   // Ensure PcdSystemMemorySize has been set
   ASSERT (PcdGet64 (PcdSystemMemorySize) != 0);
+  ASSERT (PcdGet64 (PcdSystemMemoryBase) < (UINT64)MAX_ADDRESS);

   SystemMemoryBase = (UINTN)PcdGet64 (PcdSystemMemoryBase);
   SystemMemoryTop = SystemMemoryBase + PcdGet64 (PcdSystemMemorySize);
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Section Alignment of elf binaries compiled with GCC(Linux)

[Gao, Liming]

Michael:
  Do you use the linker script BaseTools/Scripts/GccBase.lds and -z 
common-page-size=4096?

Thanks
Liming
From: Michael Zimmermann [mailto:sigmaepsilo...@gmail.com]
Sent: Wednesday, September 9, 2015 3:22 PM
To: Yao, Jiewen
Cc: Gao, Liming; edk2-devel@lists.01.org
Subject: Re: [edk2] Section Alignment of elf binaries compiled with GCC(Linux)

Unfortunately I missed the replies but I debugged this problem further and the 
Problem is that GenFw set's the alignment based on "sh_addralign" in the Elf 
header.

The 'common-page-size' flag doesn't change the value of this field though. what 
it does change is the Alignment value of the Program Headers:
Program Headers:
  Type   Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  LOAD   0x01 0x 0x 0x042f8 0x042f8 R E 0x1
  LOAD   0x0142f8 0x000142f8 0x000142f8 0x000d4 0x000f4 RW  0x1
  GNU_STACK  0x00 0x 0x 0x0 0x0 RWE 0x10
the section alignment still stays at 0/1/4.

On Tue, Aug 18, 2015 at 4:49 AM, Yao, Jiewen 
> wrote:
OK

-Original Message-
From: Gao, Liming
Sent: Tuesday, August 18, 2015 10:48 AM
To: Yao, Jiewen; Michael Zimmermann; 
edk2-devel@lists.01.org
Subject: RE: [edk2] Section Alignment of elf binaries compiled with GCC(Linux)

Jiewen:
  The updated message is useful. I suggest to change error level from 
EFI_D_ERROR to EFI_D_INFO.

Thanks
Liming
-Original Message-
From: Yao, Jiewen
Sent: Tuesday, August 18, 2015 10:46 AM
To: Gao, Liming; Michael Zimmermann; 
edk2-devel@lists.01.org
Subject: RE: [edk2] Section Alignment of elf binaries compiled with GCC(Linux)

Hi
How about we update debug message like below:

Index: PropertiesTable.c
===
--- PropertiesTable.c   (revision 18191)
+++ PropertiesTable.c   (working copy)
@@ -1120,7 +1120,7 @@

   SetPropertiesTableSectionAlignment (SectionAlignment);
   if ((SectionAlignment & (EFI_ACPI_RUNTIME_PAGE_ALLOCATION_ALIGNMENT - 1)) != 
0) {
-DEBUG ((EFI_D_ERROR, "  InsertImageRecord - Section 
Alignment(0x%x) is not %dK  \n",
+DEBUG ((EFI_D_ERROR, "  UEFI2.5 PropertiesTable - Runtime Driver 
Section Alignment(0x%x) is not %dK  \n",
   SectionAlignment, EFI_ACPI_RUNTIME_PAGE_ALLOCATION_ALIGNMENT >> 10));
 PdbPointer = PeCoffLoaderGetPdbPointer ((VOID*) (UINTN) ImageAddress);
 if (PdbPointer != NULL) {




-Original Message-
From: edk2-devel 
[mailto:edk2-devel-boun...@lists.01.org]
 On Behalf Of Gao, Liming
Sent: Tuesday, August 18, 2015 10:39 AM
To: Michael Zimmermann; edk2-devel@lists.01.org
Subject: Re: [edk2] Section Alignment of elf binaries compiled with GCC(Linux)

Hi,
  This is a warning message that describes the runtime driver alignment is not 
4K.

  UEFI PropertiesTable table feature expects all runtime driver alignment is 
4K. When DxeCore loads Runtime driver, it will check its alignment and report 
such warning message if it doesn't meet with the alignment. If you want to 
enable this feature, you need to make sure all runtime driver at 4K. If you 
don't enable it, you can just ignore this message.

  To configure runtime driver with 4K alignment, you can modify DSC file to add 
the following section.
[BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
  GCC:  *_*_*_DLINK_FLAGS = -z common-page-size=0x1000
  MSFT: *_*_*_DLINK_FLAGS = /ALIGN:4096

Thanks
Liming
-Original Message-
From: edk2-devel 
[mailto:edk2-devel-boun...@lists.01.org]
 On Behalf Of Michael Zimmermann
Sent: Sunday, August 16, 2015 12:32 PM
To: edk2-devel@lists.01.org
Subject: [edk2] Section Alignment of elf binaries compiled with GCC(Linux)

When booting(a new device I'm currently working on) I get these warnings:
  InsertImageRecord - Section Alignment(0x20) is not 4K  

the warning is raised by "MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c" and 
when compiling using GCC from Linux, the SectionAlignment is set by 
"BaseTools/Source/C/GenFw/Elf32Convert.c".

I checked the resulting binaries using "readpe" and indeed they have a 
SectionAlignment of 0x20, while the precompiled binaries like Shell.efi have a 
Alignment of 0x1000(4K).

So, is this a bug of my GCC compiler or of EDK2?
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

___
edk2-devel mailing list

Re: [edk2] [PATCH 1/3] ArmPlatformPkg/MemoryInitPeim: handle memory above 4 GB on 32-bit ARM

[Ard Biesheuvel]

On 9 September 2015 at 16:31, Leif Lindholm  wrote:
> On Tue, Sep 08, 2015 at 07:35:40PM +0200, Ard Biesheuvel wrote:
>> Make sure that the PEI memory region is carved out of memory that is
>> 32-bit addressable, by taking MAX_ADDRESS into account (which is
>> defined as '4 GB - 1' on ARM)
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel 
>> ---
>>  ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c | 7 +--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c 
>> b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
>> index 93ab16ca4a74..9f26d26a04d3 100755
>> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
>> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
>> @@ -96,7 +96,7 @@ InitializeMemory (
>>  {
>>EFI_STATUSStatus;
>>UINTN SystemMemoryBase;
>> -  UINTN SystemMemoryTop;
>> +  UINT64SystemMemoryTop;
>>UINTN FdBase;
>>UINTN FdTop;
>>UINTN UefiMemoryBase;
>> @@ -115,7 +115,10 @@ InitializeMemory (
>>ASSERT (PcdGet64 (PcdSystemMemorySize) != 0);
>>
>>SystemMemoryBase = (UINTN)PcdGet64 (PcdSystemMemoryBase);
>> -  SystemMemoryTop = SystemMemoryBase + (UINTN)PcdGet64 
>> (PcdSystemMemorySize);
>> +  SystemMemoryTop = SystemMemoryBase + PcdGet64 (PcdSystemMemorySize);
>> +  if (SystemMemoryTop - 1 > MAX_ADDRESS) {
>> +SystemMemoryTop = (UINT64) MAX_ADDRESS + 1;
>> +  }
>>FdBase = (UINTN)PcdGet64 (PcdFdBaseAddress);
>>FdTop = FdBase + (UINTN)PcdGet32 (PcdFdSize);
>>
>> --
>> 1.9.1
>
> So, this is mainly an OCD thing on my end, but is there any real
> benefit to _not_ converting SystemMemoryBase to 64-bit? Doing so would
> get rid of the other UINTN cast.
>

Not really, other than the fact that UINTN is more suitable for the
base of DRAM, since you won't be able to run 32-bit UEFI if RAM starts
above 4 GB anyway, so changing it to UINT64 seemed wrong to me.

> Either way:
> Reviewed-by: Leif Lindholm 
>

Thanks!
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Section Alignment of elf binaries compiled with GCC(Linux)

[Ard Biesheuvel]

On 9 September 2015 at 17:26, Gao, Liming  wrote:
> Michael:
>   Do you use the linker script BaseTools/Scripts/GccBase.lds and -z 
> common-page-size=4096?
>

Are you building for 32-bit ARM by any chance? That does not have this
feature wired up yet. I posted a v2 of my series that addresses this
yesterday.



> From: Michael Zimmermann [mailto:sigmaepsilo...@gmail.com]
> Sent: Wednesday, September 9, 2015 3:22 PM
> To: Yao, Jiewen
> Cc: Gao, Liming; edk2-devel@lists.01.org
> Subject: Re: [edk2] Section Alignment of elf binaries compiled with GCC(Linux)
>
> Unfortunately I missed the replies but I debugged this problem further and 
> the Problem is that GenFw set's the alignment based on "sh_addralign" in the 
> Elf header.
>
> The 'common-page-size' flag doesn't change the value of this field though. 
> what it does change is the Alignment value of the Program Headers:
> Program Headers:
>   Type   Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
>   LOAD   0x01 0x 0x 0x042f8 0x042f8 R E 0x1
>   LOAD   0x0142f8 0x000142f8 0x000142f8 0x000d4 0x000f4 RW  0x1
>   GNU_STACK  0x00 0x 0x 0x0 0x0 RWE 0x10
> the section alignment still stays at 0/1/4.
>
> On Tue, Aug 18, 2015 at 4:49 AM, Yao, Jiewen 
> > wrote:
> OK
>
> -Original Message-
> From: Gao, Liming
> Sent: Tuesday, August 18, 2015 10:48 AM
> To: Yao, Jiewen; Michael Zimmermann; 
> edk2-devel@lists.01.org
> Subject: RE: [edk2] Section Alignment of elf binaries compiled with GCC(Linux)
>
> Jiewen:
>   The updated message is useful. I suggest to change error level from 
> EFI_D_ERROR to EFI_D_INFO.
>
> Thanks
> Liming
> -Original Message-
> From: Yao, Jiewen
> Sent: Tuesday, August 18, 2015 10:46 AM
> To: Gao, Liming; Michael Zimmermann; 
> edk2-devel@lists.01.org
> Subject: RE: [edk2] Section Alignment of elf binaries compiled with GCC(Linux)
>
> Hi
> How about we update debug message like below:
>
> Index: PropertiesTable.c
> ===
> --- PropertiesTable.c   (revision 18191)
> +++ PropertiesTable.c   (working copy)
> @@ -1120,7 +1120,7 @@
>
>SetPropertiesTableSectionAlignment (SectionAlignment);
>if ((SectionAlignment & (EFI_ACPI_RUNTIME_PAGE_ALLOCATION_ALIGNMENT - 1)) 
> != 0) {
> -DEBUG ((EFI_D_ERROR, "  InsertImageRecord - Section 
> Alignment(0x%x) is not %dK  \n",
> +DEBUG ((EFI_D_ERROR, "  UEFI2.5 PropertiesTable - Runtime Driver 
> Section Alignment(0x%x) is not %dK  \n",
>SectionAlignment, EFI_ACPI_RUNTIME_PAGE_ALLOCATION_ALIGNMENT >> 10));
>  PdbPointer = PeCoffLoaderGetPdbPointer ((VOID*) (UINTN) ImageAddress);
>  if (PdbPointer != NULL) {
>
>
>
>
> -Original Message-
> From: edk2-devel 
> [mailto:edk2-devel-boun...@lists.01.org]
>  On Behalf Of Gao, Liming
> Sent: Tuesday, August 18, 2015 10:39 AM
> To: Michael Zimmermann; 
> edk2-devel@lists.01.org
> Subject: Re: [edk2] Section Alignment of elf binaries compiled with GCC(Linux)
>
> Hi,
>   This is a warning message that describes the runtime driver alignment is 
> not 4K.
>
>   UEFI PropertiesTable table feature expects all runtime driver alignment is 
> 4K. When DxeCore loads Runtime driver, it will check its alignment and report 
> such warning message if it doesn't meet with the alignment. If you want to 
> enable this feature, you need to make sure all runtime driver at 4K. If you 
> don't enable it, you can just ignore this message.
>
>   To configure runtime driver with 4K alignment, you can modify DSC file to 
> add the following section.
> [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
>   GCC:  *_*_*_DLINK_FLAGS = -z common-page-size=0x1000
>   MSFT: *_*_*_DLINK_FLAGS = /ALIGN:4096
>
> Thanks
> Liming
> -Original Message-
> From: edk2-devel 
> [mailto:edk2-devel-boun...@lists.01.org]
>  On Behalf Of Michael Zimmermann
> Sent: Sunday, August 16, 2015 12:32 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] Section Alignment of elf binaries compiled with GCC(Linux)
>
> When booting(a new device I'm currently working on) I get these warnings:
>   InsertImageRecord - Section Alignment(0x20) is not 4K  
>
> the warning is raised by "MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c" and 
> when compiling using GCC from Linux, the SectionAlignment is set by 
> "BaseTools/Source/C/GenFw/Elf32Convert.c".
>
> I checked the resulting binaries using "readpe" and indeed they have a 
> SectionAlignment of 0x20, while the precompiled binaries like Shell.efi have 
> a Alignment of 0x1000(4K).
>
> So, is this a bug of my GCC compiler or of EDK2?
> 

[edk2] EDK II & GPL - Re: OVMF BoF @ KVM Forum 2015

[Jordan Justen]

On 2015-09-09 01:57:51, Laszlo Ersek wrote:
> On 08/10/15 18:24, Laszlo Ersek wrote:
> > Hi.
> > 
> > Let's do an OVMF BoF at this year's KVM Forum too.
> 
> Here's a preliminary task list, after some off-list discussion (I tried
> to incorporate comments):
> 
> - create GPL'd fork called "ovmf" for expediting virt development
>   (OvmfPkg, ArmVirtPkg)
>   - maybe leverage the feature under
>  for
> setting up a separate "tianocore/edk2-gpl" repo, for GPL'd
> contributions [Jordan]
> - repo separation by license could make things harder for packagers
>   and QEMU bundling [Laszlo]

I would like OVMF to follow a plan for GPL that the whole EDK II
community decides on.

I would also like to see EDK II add a (permissively licensed; BSD,
MIT, etc) DriverPkg (DriversPkg?). We discussed this on the list
recently:

http://thread.gmane.org/gmane.comp.bios.tianocore.devel/17544/focus=676

So, related to this, I wonder how the community would feel about a
GplDriverPkg. Would the community allow it as a new package in EDK II
directly, or would a separate repo be required?

With regards to adding it directly into the EDK II tree, here are some
potential concerns that I might anticipate hearing from the community:

* It will make it easier for contributors to choose GPL compared to a
  permissive license, thereby limiting some users of the contribution

* GPL code will more easily be copied into the permissively licensed
  packages

* Some might refuse to look at EDK II entirely if it has a directory
  with GPL source code in it

Of these, I personally only sympathize with the first.

Regarding the separate OVMF repo, my hope is that it is more of a OVMF
specific working area, rather than a 'GPL OVMF'. For example, patches
or features that we've not yet figured out how to upstream, but we
hopefully plan to.

Additionally, it makes sense to use it as needed for OVMF specific
releases. (Ie, OVMF release tags)

-Jordan

> - document the rules / justification for "ovmf" (licensing
>   conflicts, non-technical blockage on edk2 etc).
> - No new mailing list needed
> - push RH's downstream-only patches to "ovmf", wherever that makes
>   sense
> - remove encumbered FAT driver
> - import Peter Batard's GPL r/o FAT driver port of GRUB's
> - secure OpenSSL linking exception for the former from the copyright
>   holders (Peter Batard, GRUB project)
> - "ovmf" should be periodically rebased / should fetch+merge edk2 as
>   master (arguments both for and against merging); distros should
>   then track "ovmf" as their upstream, not edk2
> - get OVMF into Fedora (as pkg) and QEMU (as bundled binary)
> - do OVMF releases, maybe in sync with QEMU's releases
>   - we can probably build from known good revisions from git [Alex]
> - revive Q35 SATA driver work / poke Reza
>   - Hannes and Gabriel have refreshed patches, but their versions differ
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Section Alignment of elf binaries compiled with GCC(Linux)

[Ard Biesheuvel]

> On 9 sep. 2015, at 18:10, Michael Zimmermann  wrote:
> 
> Yes I'm using 32bit ARM :)
> thx for the patches - unfortunatelythe patches fail for me.
> 

Did you regenerate Conf/tools_def.txt  and rebuild the BaseTools/ ?

>> On Wed, Sep 9, 2015 at 5:33 PM, Ard Biesheuvel  
>> wrote:
>> On 9 September 2015 at 17:26, Gao, Liming  wrote:
>> > Michael:
>> >   Do you use the linker script BaseTools/Scripts/GccBase.lds and -z 
>> > common-page-size=4096?
>> >
>> 
>> Are you building for 32-bit ARM by any chance? That does not have this
>> feature wired up yet. I posted a v2 of my series that addresses this
>> yesterday.
>> 
>> 
>> 
>> > From: Michael Zimmermann [mailto:sigmaepsilo...@gmail.com]
>> > Sent: Wednesday, September 9, 2015 3:22 PM
>> > To: Yao, Jiewen
>> > Cc: Gao, Liming; edk2-devel@lists.01.org
>> > Subject: Re: [edk2] Section Alignment of elf binaries compiled with 
>> > GCC(Linux)
>> >
>> > Unfortunately I missed the replies but I debugged this problem further and 
>> > the Problem is that GenFw set's the alignment based on "sh_addralign" in 
>> > the Elf header.
>> >
>> > The 'common-page-size' flag doesn't change the value of this field though. 
>> > what it does change is the Alignment value of the Program Headers:
>> > Program Headers:
>> >   Type   Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
>> >   LOAD   0x01 0x 0x 0x042f8 0x042f8 R E 0x1
>> >   LOAD   0x0142f8 0x000142f8 0x000142f8 0x000d4 0x000f4 RW  0x1
>> >   GNU_STACK  0x00 0x 0x 0x0 0x0 RWE 0x10
>> > the section alignment still stays at 0/1/4.
>> >
>> > On Tue, Aug 18, 2015 at 4:49 AM, Yao, Jiewen 
>> > > wrote:
>> > OK
>> >
>> > -Original Message-
>> > From: Gao, Liming
>> > Sent: Tuesday, August 18, 2015 10:48 AM
>> > To: Yao, Jiewen; Michael Zimmermann; 
>> > edk2-devel@lists.01.org
>> > Subject: RE: [edk2] Section Alignment of elf binaries compiled with 
>> > GCC(Linux)
>> >
>> > Jiewen:
>> >   The updated message is useful. I suggest to change error level from 
>> > EFI_D_ERROR to EFI_D_INFO.
>> >
>> > Thanks
>> > Liming
>> > -Original Message-
>> > From: Yao, Jiewen
>> > Sent: Tuesday, August 18, 2015 10:46 AM
>> > To: Gao, Liming; Michael Zimmermann; 
>> > edk2-devel@lists.01.org
>> > Subject: RE: [edk2] Section Alignment of elf binaries compiled with 
>> > GCC(Linux)
>> >
>> > Hi
>> > How about we update debug message like below:
>> >
>> > Index: PropertiesTable.c
>> > ===
>> > --- PropertiesTable.c   (revision 18191)
>> > +++ PropertiesTable.c   (working copy)
>> > @@ -1120,7 +1120,7 @@
>> >
>> >SetPropertiesTableSectionAlignment (SectionAlignment);
>> >if ((SectionAlignment & (EFI_ACPI_RUNTIME_PAGE_ALLOCATION_ALIGNMENT - 
>> > 1)) != 0) {
>> > -DEBUG ((EFI_D_ERROR, "  InsertImageRecord - Section 
>> > Alignment(0x%x) is not %dK  \n",
>> > +DEBUG ((EFI_D_ERROR, "  UEFI2.5 PropertiesTable - Runtime 
>> > Driver Section Alignment(0x%x) is not %dK  \n",
>> >SectionAlignment, EFI_ACPI_RUNTIME_PAGE_ALLOCATION_ALIGNMENT >> 
>> > 10));
>> >  PdbPointer = PeCoffLoaderGetPdbPointer ((VOID*) (UINTN) ImageAddress);
>> >  if (PdbPointer != NULL) {
>> >
>> >
>> >
>> >
>> > -Original Message-
>> > From: edk2-devel 
>> > [mailto:edk2-devel-boun...@lists.01.org]
>> >  On Behalf Of Gao, Liming
>> > Sent: Tuesday, August 18, 2015 10:39 AM
>> > To: Michael Zimmermann; 
>> > edk2-devel@lists.01.org
>> > Subject: Re: [edk2] Section Alignment of elf binaries compiled with 
>> > GCC(Linux)
>> >
>> > Hi,
>> >   This is a warning message that describes the runtime driver alignment is 
>> > not 4K.
>> >
>> >   UEFI PropertiesTable table feature expects all runtime driver alignment 
>> > is 4K. When DxeCore loads Runtime driver, it will check its alignment and 
>> > report such warning message if it doesn't meet with the alignment. If you 
>> > want to enable this feature, you need to make sure all runtime driver at 
>> > 4K. If you don't enable it, you can just ignore this message.
>> >
>> >   To configure runtime driver with 4K alignment, you can modify DSC file 
>> > to add the following section.
>> > [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
>> >   GCC:  *_*_*_DLINK_FLAGS = -z common-page-size=0x1000
>> >   MSFT: *_*_*_DLINK_FLAGS = /ALIGN:4096
>> >
>> > Thanks
>> > Liming
>> > -Original Message-
>> > From: edk2-devel 
>> > [mailto:edk2-devel-boun...@lists.01.org]
>> >  On Behalf Of Michael Zimmermann
>> > Sent: Sunday, August 16, 2015 12:32 PM
>> > To: 

Re: [edk2] EDK II & GPL - Re: OVMF BoF @ KVM Forum 2015

[Blibbet]

> I don’t understand the issue BSD licensed code?
> It should be compatible with the GPL?
> The GPL code could merge bug fixes from the BSD
> source base as needed. It is just the BDS source
> base that can not take back GPL code.

I'm not sure I understand, I presume you understand all BSD/GPL
licensing issues very well. :-)

Sure, mixture of licenses makes life more difficult. Not an excuse for
ignoring non-BSD innovations and embracing Linux OSV/OEM community with
their preferred license. GPL'ed LibreOffice bugfixes can't go upstream
to BSD-like, ASF2 license of Apache OpenOffice, but that's life. Even
with BSD licensed code, vendor's often don't contribute bugs back, look
at XNU.

Issue: Linux community uses often uses GPL, UEFI Forum and Tianocore
forbids any but BSD (except gives Microsoft freedom to pick any license
they want for FAT).

Windows and OSX aside, for FOSS OSVs, like FreeBSD and Linux-based ones,
the open source community often uses open source licenses other than
BSD, including GPL. All of that community contributions are lost to
TIanocore and UEFI Forum. And to closed-source IBVs who can't handle GPL
taint exposing their code.

I'd hope that any new code that the Linux/FreeBSD/FOSS OS community
needs could be BSD-licensed, so it could be upstreamed to Tianocore, if
they deem to approve it. An IBV (or other group) that helps direct any
open source contributions could help manage that. Ungroomed community
contributions are much more likely to be unusable by BSD-centric subset
of firmware community. Apple and Microsoft are last in the list of big
players that don't properly leverage open source community
contributions, along with silicon vendors, all of which control UEFI
Forum, so likely no change there. :-(

>> Look at the coreboot blog, the other week the Purism BIOS developer was
>> talking about a "Free Software port of FSP". That should be a shared
>> effort by all Linux OEMs/vendors, not shouldered by a single OEM. A
>> Linux-centric IBV could be getting help from multiple companies -- like
>> Linaro does with ARM -- to help with this effort. It could be part of
>> Linux Foundation's Core Infrastructure Initiative, maybe.
>>
>> They should also tailor Linux-friendly ACPI, like recent thread about
>> Linux standardization of _DSD. As well as not include a WBPT table in
>> Linux OEM systems, and look at the other tables to see what's should be
>> removed.
>>
>
> In general it is not good form to discuss pre-released industry standard
> stuff not on the standards group mailing list. The idea is to not have
folks
> implementing pre released stuff that causes interoperability issues.

Sorry,  I don't understand this comment. Nothing I mentioned above was
pre-released. Coreboot blog post was public. _DSD mailing list post was
public. I'm not on any non-public firmware communities.

The UEFI Forum only has 1 community, EDK2-devel, to go to for
experience. I'd love to see a non-dev discussion list, where I could
mention some stifling firmware patent issues that scare me, which I've
omitted from discussing on a list with engineers eager to stay ignorant
with patents so they can more easily patent more things. :-)

>> Alternately, the UEFI Forum should create a non-BSD tree to contain
>> this, not just focus on BSD for the fully-closed-source end of the
>> spectrum, and work with some FOSS-centric OSVs to better support UEFI
>> using their community's models.
>>
>
> The UEFI Forum only publishes the UEFI tests, SCTs. The UEFI Forum
> owns the specifications, but does not own or advocate for implementations
> (other than advocating conformance to the specification).

I guess you could frame it that way.

The UEFI forum maintains a BSD codebase where non-FOSS OSVs thrive, and
GPL OSVs are stifled.

If they chose to help FOSS OSVs, the UEFI Forum could choose to also
maintain a codebase with other-than-BSD code, and not ignore the non-BSD
contributions from the FOSS community.

I just mentioned the UEFI Forum could -- if they chose -- also maintain
another codebase, where their FOSS ISVs could also use.

I expect an IBV would probably be a better choice than UEFI Forum for
anything beyond BSD, given current members anti-non-BSD position.

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Xen-devel] OVMF BoF @ KVM Forum 2015

[Laszlo Ersek]

On 09/09/15 18:34, Ian Campbell wrote:
> On Wed, 2015-09-09 at 10:57 +0200, Laszlo Ersek wrote:
>> On 08/10/15 18:24, Laszlo Ersek wrote:
>>> Hi.
>>>
>>> Let's do an OVMF BoF at this year's KVM Forum too.
>>
>> Here's a preliminary task list
> 
> Thanks for including xen-devel in this. Was anyone from the Xen community
> present at the BoF (so far as you know)? Are there any minutes or anything
> like that for those interested in the discussion and reasoning rather than
> just the resulting action items?

Okay, second attempt at an answer. I *have* forgotten a great deal of
the sentences that were uttered at the BoF, but I mostly remember why we
wanted these things. :)

So let me see if I can elaborate on these (after all you are interested
in the reasoning in general, not just the discussions that I fail to
recall in detail):

>> , after some off-list discussion (I tried
>> to incorporate comments):
>>
>> - create GPL'd fork called "ovmf" for expediting virt development
>>   (OvmfPkg, ArmVirtPkg)
>>   - maybe leverage the feature under
>>  for
>> setting up a separate "tianocore/edk2-gpl" repo, for GPL'd
>> contributions [Jordan]
>> - repo separation by license could make things harder for packagers
>>   and QEMU bundling [Laszlo]

So in this item (which, in the above form, is already obsolete) I
managed to mix up three separate things. (But Jordan then went on and
clarified those today.)

(1) One thing is a virt-oriented fork of edk2, without explicit
licensing-oriented goals. The main purpose of this fork would be to
expedite virt development (OvmfPkg, ArmVirtPkg, and central packages on
which the former two depend).

It has sometimes occurred that an OVMF feature got tied up in
inter-maintainer disagreement, or other non-technical reasons (lack of
review capacity etc). This holds back virt development, plus it has the
potential to force downstreams (let's be concrete: Red Hat at minimum)
to carry some patches in downstream only.

The fork would accelerate things here. We'd have a set of rules for
committing to the fork (and the divergence would have to be rebased
periodically, or alternatively edk2 patches would have to be merged into
the fork, periodically).

Considering this purpose (ie. expediting virt development) in isolation,
the fork should stick with the original edk2 licenses.

(2) The *ability* to provide the end user with free software (in the
FSF's definition) or open source software (in the OSI's definition),
which the current FAT driver is regrettably none of, is extremely important.

Currently the only alternative that appears feasible is Peter Batard's
UEFI port of GRUB's read-only FAT driver. That driver comes under the
GPL, and therefore it ties into topic (3) below.

However, note that if the in-tree FAT driver were liberated (= made Free
or Open Source Software, for example by dropping the "Additional Terms"
from its license, thereby rendering it 3-BSDL), then goal (2) would be
immediately achieved, and it wouldn't tie into goal (3) below.

(3) Accommodating drivers that contributors can (or are willing to)
submit only under the GPL would be nice. For example there has been such
a contribution from Ludovic Rousseau, a smart card reader protocol
implementation (a port from Linux).

Jordan's most recent suggestion was to actually keep GPL'd drivers
within edk2, under "GplDriverPkg". This is being discussed on
edk2-devel, so I'll move on.

>> - document the rules / justification for "ovmf" (licensing
>>   conflicts, non-technical blockage on edk2 etc).
>> - No new mailing list needed

Okay, so the rules. They would go like (obviously they are up for
discussion):

- submit your stuff to edk2-devel as always

- work with the reviewers / maintainers

- if you get demonstrably stuck, due to inter-maintainer deadlock (ie.
  you'd be fine implementing either maintainer's request, but their
  requirements *together* are unsatisfiable, because they conflict),
  the patches can be committed to the fork, subject to review *for* the
  fork

- same if there's just no feedback for a patchset

- same if the patches are blocked due to non-technical criticism

- maybe the same if the technical feedback would require
  *disproportionate* effort (ie. cross-package refactorings), esp.
  involving client (=platform) packages that are not related to virt.
  We have to be careful with this one (it's not a blanket excuse, and
  arguments are bound to be somewhat subjective here), but such
  unjustified / overarching / disproportionate requirements can block
  the upstreaming of a feature (developed under a deadline) for good.

  Example: consider a refactoring that straddles DuetPkg or EmulatorPkg
  *and* MdeModulePkg, so that you can use a feature in OvmfPkg. You
  just turned a two week development effort into 6-8 weeks.

  No. You might not even have the environment to test DuetPkg. Etc.

- Initial set of committers to this 

Re: [edk2] EDK II & GPL - Re: OVMF BoF @ KVM Forum 2015

[Andrew Fish]

> On Sep 9, 2015, at 3:24 PM, Jordan Justen  wrote:
> 
> On 2015-09-09 12:11:26, El-Haj-Mahmoud, Samer wrote:
>> The recent expansions beyond BSD where all permissive licenses (BSD
>> like) as far as I can tell.
>> 
>> I agree with Andrew, opening the door for GPL licensed code in EDK2
>> will have severe consequences for products that are built using
>> EDK2.
> 
> I don't think simply having a GplDriverPkg in the tree would have any
> consequences for a platform that doesn't use any code in that package.
> Obviously we could not make any core packages rely on that package.
> 

So you have a legal degree and are speaking on behalf of your employer on this 
subject? 

> This would just be a sanctioned, clear landing place for people that
> cannot, or will not provide their driver under a permissive license.
> 
> This license will limit who can use drivers from this package. For
> that reason, I hope that we will always ask if a contribution can be
> permissively licensed instead.
> 
> Personally, I would prefer a 2-clause BSD only tree for simplicity,
> but unfortunately, that sort of restriction has its own drawbacks as
> well. (frustrated contributors and less contributions)
> 
> FWIW, I don't mind if the consensus is that GplDriverPkg must live in
> a separate repo. But, it would be nice to hear a good reason why it
> must live elsewhere.

Because GPL is not a permissive license. An accidental git grep and copying 
some code can change the license of the code that gets the GPL code pasted into 
it. Thus having GPL code in the same repository as BSD code can end up 
accidentally converting BSD code to GPL code over time. If GPL was OK with 
everyone we would have started with GPL. The good thing is the BDS code is GPL 
compatible so it can be used for GPL code and bug fixes in the BDS code can be 
merged into to GPL code, but this is a one way operation. 

If you don’t believe me please feel free to sit down and have a long 
conversation with Intel IP lawyers.


> (And, why that doesn't also apply to FatBinPkg.)
> 

There is no IP leakage from a binary. This FAT driver is licensed for use with 
EFI, and given this is a EFI code base that seemed like a good thing. 

I don’t pretent to understand the GPL FAT thing, I guess it is some kind of 
civil disobedience. it does not mater what license you strap on the code the 
the device makers still have to “pay the man”. 

Thanks,

Andrew Fish

PS As I stated before I’m fine removing all the binaries from the main repo, as 
you don’t really want binaries in your production repo, and source level 
debugging is a nice feature and all. 

> -Jordan
> 
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
>> Jordan Justen
>> Sent: Wednesday, September 09, 2015 12:58 PM
>> To: Andrew Fish 
>> Cc: Lenny Szubowicz ; Karen Noel ; Ard 
>> Biesheuvel ; edk2-devel-01 
>> ; Reza Jelveh ; Alexander Graf 
>> ; qemu devel list ; Hannes Reinecke 
>> ; Gabriel L. Somlo (GMail) ; Peter Jones 
>> ; Peter Batard ; Gerd Hoffmann 
>> ; Cole Robinson ; Paolo Bonzini 
>> ; xen-de...@lists.xen.org; Laszlo Ersek 
>> ; Ademar de Souza Reis Jr. 
>> Subject: Re: [edk2] EDK II & GPL - Re: OVMF BoF @ KVM Forum 2015
>> 
>> On 2015-09-09 10:04:50, Andrew Fish wrote:
>>> 
 On Sep 9, 2015, at 9:17 AM, Jordan Justen  
 wrote:
 
 So, related to this, I wonder how the community would feel about a
 GplDriverPkg. Would the community allow it as a new package in EDK
 II directly, or would a separate repo be required?
 
>>> 
>>> I think we would need a separate repo, like the FAT driver. That is
>>> the only way to deal with the license issues.
>> 
>> There doesn't seem to be any guiding rules here. For example, I
>> think some people are not comfortable with the FatBinPkg being in
>> the tree due to the license. Why is that okay?
>> 
 With regards to adding it directly into the EDK II tree, here are
 some potential concerns that I might anticipate hearing from the community:
 
 * It will make it easier for contributors to choose GPL compared to
 a  permissive license, thereby limiting some users of the
 contribution
 
 * GPL code will more easily be copied into the permissively licensed
 packages
 
 * Some might refuse to look at EDK II entirely if it has a directory
 with GPL source code in it
 
>>> 
>>> Or have their rights to contribute revoked since this is a fundamental
>>> change, and would require employees to get reauthorized by their legal
>>> departments to contribute.
>> 
>> We've 

Re: [edk2] [PATCH v2 0/2] MdeModulePkg: ScsiDiskDxe: handle EFI_BAD_BUFFER_SIZE from SCSI host adapter

[Tian, Feng]

Series Reviewed-by: Feng Tian 

Thanks
Feng

-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Wednesday, September 09, 2015 17:39
To: edk2-de...@ml01.01.org
Cc: Tian, Feng
Subject: [PATCH v2 0/2] MdeModulePkg: ScsiDiskDxe: handle EFI_BAD_BUFFER_SIZE 
from SCSI host adapter

No changes in either patch, relative to what has been discussed / proposed in 
the v1 thread.

I *think* Feng's Reviewed-by in

already covers both patches, but I'm not 100% sure, therefore I'm reposting in 
order for Feng to confirm.

Cc: Feng Tian 

Thanks!
Laszlo

Laszlo Ersek (2):
  MdeModulePkg: ScsiDiskDxe: recognize EFI_BAD_BUFFER_SIZE
  MdeModulePkg: ScsiDiskDxe: adapt SectorCount when shortening transfers

 MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c | 49 ++--
 1 file changed, 45 insertions(+), 4 deletions(-)

--
1.8.3.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg: add PartitionName protocol

[Tian, Feng]

Michael

Is there a spec about the GPT partition naming rule in Android world? Could you 
share the link?

If these GPT partitions have special naming rules (boot, recovery…), why they 
don’t define specific partition type guid just like EFI SYSTEM PARTITION to 
distinguish them?

For example, EFI System Partition type guid is defined by UEFI as 
C12A7328-F81F-11D2-BA4B-00A0C93EC93B

Thanks
Feng

From: Michael Zimmermann [mailto:sigmaepsilo...@gmail.com]
Sent: Wednesday, September 09, 2015 17:34
To: Tian, Feng
Cc: Ni, Ruiyu; edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] MdeModulePkg: add PartitionName protocol

The name is reliable because it's used by all OEM's :)
I'm the founder of EFIDroid, a port of UEFI to Qualcomm based Android devices.
https://github.com/efidroid
https://plus.google.com/u/0/communities/114053643671219382368

Android boot images(a container for kernel,ramdisk, devicetree and bootargs) 
are stored as a raw binary on named partitions(boot, recovery,...). Naming 
partitions is standard in Android and every single partition has a name. 
neither GPT GUID's nor file system labels(these actually can change with every 
format depending on the tools used). are persistent or reliable.

On Wed, Sep 9, 2015 at 10:50 AM, Tian, Feng 
> wrote:
My concern is the name string is also not reliable. Different partition tools 
may have different naming rules.

Yes, "private protocol" I meant is using it as your local implementation.

Last, still  has a little curious on your usage model, could you explain it 
more? Maybe there is a better way to solve your problem.

Thanks
Feng

-Original Message-
From: edk2-devel 
[mailto:edk2-devel-boun...@lists.01.org]
 On Behalf Of Michael Zimmermann
Sent: Wednesday, September 09, 2015 15:48
To: Tian, Feng
Cc: Ni, Ruiyu; edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] MdeModulePkg: add PartitionName protocol

UniquePartitionGUID isn't suitable because the device vendor's don't set proper 
values(they're auto generated each time the partition table changes).
Instead names like "boot" are used so you can find the correct partition.

by "Implement it as a private protocol" do u just mean "use your own fork"
or is there some different protocol type?

On Wed, Sep 9, 2015 at 9:34 AM, Tian, Feng 
> wrote:

> Michael,
>
> IMHO, I don't think it's valuable to introducing this new protocol to
> get a name string. It's too burden. If only you have this use case, I
> would suggest you implement it as a private protocol rather than the public 
> one.
>
> PS: Why UniquePartitionGUID doesn't work for your case?
>
> Thanks
> Feng
>
> -Original Message-
> From: edk2-devel 
> [mailto:edk2-devel-boun...@lists.01.org]
>  On Behalf Of
> Michael Zimmermann
> Sent: Wednesday, September 09, 2015 15:28
> To: Ni, Ruiyu
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] MdeModulePkg: add PartitionName protocol
>
> Hi,
>
> I need the partition name because I need to boot from partitions by
> their GPT label(filesystem label and GUID aren't set/reliable).
>
> Usually I would've just added a name field to the existing DiskIo
> Protocol, but since it's a UEFI standard we can't/shouldn't do this.
>
> I chose option 3 over 1/2 because the others would mean that u have to
> parse the whole partition table again which would lead to redundant code.
>
>
> Michael
>
> On Sun, Sep 6, 2015 at 8:33 AM, Ni, Ruiyu 
> > wrote:
>
> > Zimmermann,
> > There are three options to get the name of a partition:
> > 1. Directly use BlockIo interface to access the GPT meta data.
> > 2. Group the above code into a library 3. Group the above code into
> > a driver and expose the partition name as a protocol.
> >
> > I really do not think we need to go to option 3 directly. Can you
> > tell us the reason why a protocol is needed?
> >
> > Thanks,
> > Ray
> >
> > -Original Message-
> > From: edk2-devel 
> > [mailto:edk2-devel-boun...@lists.01.org]
> >  On Behalf
> > Of Michael Zimmermann
> > Sent: Thursday, September 3, 2015 2:23 PM
> > To: edk2-devel@lists.01.org
> > Subject: [edk2] [PATCH] MdeModulePkg: add PartitionName protocol
> >
> > This allows to retrieve the name of a partition.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Michael Zimmermann  > n...@hp.com>
> > ---
> >  MdeModulePkg/Include/Protocol/PartitionName.h  | 42
> > ++
> >  MdeModulePkg/MdeModulePkg.dec  |  3 ++
> >  .../Universal/Disk/PartitionDxe/ElTorito.c |  3 +-
> >  MdeModulePkg/Universal/Disk/PartitionDxe/Gpt.c |  3 +-
> >  

Re: [edk2] [patch] NetworkPkg: PXE Driver's LoadFile protocol should check FilePath

[Ye, Ting]

Reviewed-by: Ye Ting 


-Original Message-
From: Zhang, Lubo 
Sent: Wednesday, September 09, 2015 5:10 PM
To: edk2-devel@lists.01.org
Cc: Fu, Siyuan; Ye, Ting
Subject: [patch] NetworkPkg: PXE Driver's LoadFile protocol should check 
FilePath

PXE driver's LoadFile protocol should check the input parameter
FilePath to see whether it's a supported device path.If not,
it should return invalid parameter, do not continue PXE boot.

Cc: Fu Siyuan
Cc: Ye Ting
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
---
 NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c 
b/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c
index cdcf2f0..367a135 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c
@@ -2326,10 +2326,14 @@ EfiPxeLoadFile (
   EFI_PXE_BASE_CODE_PROTOCOL  *PxeBc;
   BOOLEAN UsingIpv6;
   EFI_STATUS  Status;
   BOOLEAN MediaPresent;
 
+  if (FilePath == NULL || !IsDevicePathEnd (FilePath)) {
+return EFI_INVALID_PARAMETER;
+  }
+  
   VirtualNic = PXEBC_VIRTUAL_NIC_FROM_LOADFILE (This);
   Private= VirtualNic->Private;
   PxeBc  = >PxeBc;
   UsingIpv6  = FALSE;
   Status = EFI_DEVICE_ERROR;
-- 
1.9.5.msysgit.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] EDK II & GPL - Re: OVMF BoF @ KVM Forum 2015

[Jordan Justen]

On 2015-09-09 20:26:54, Andrew Fish wrote:
> > On Sep 9, 2015, at 5:41 PM, Jordan Justen  wrote:
> > On 2015-09-09 16:05:20, Andrew Fish wrote:
> >> So you have a legal degree and are speaking on behalf of your
> >> employer on this subject?
> > 
> > No and no. How about you? :)
> 
> No but I have to review any code contributed to the open source
> project to make sure it follows the corporate polices.

Is Apple corporate policy that you could never contribute to a project
that has a GPL directory in the tree?

> > Nevertheless, I have not heard the interpretation that just having GPL
> > in a source tree would impact your code, even if you do not include,
> > nor link to it. Is this Apple's interpretation of how GPL works?
> 
> No but thanks for making my point for me. 1st off the rules are made
> by lawyers and managers so you trying to argue logic is kind of
> funny. What does logic have to do with it.

Whoa! What's next in this crazy world? Dogs and cats living together!
Mass hysteria! How can we be sure that the lawyers won't decide that
BSD means GPL and vice versa? ;)

> Your company started this edk2 project as a BSD project, and I
> assume there was a reason for that.

And then more licenses were added.

> The reasons rules like this end up getting made is that developers
> like you are confused about the company policy regarding open
> source, closed source and protecting intellectual property rights.
> So your very smart and well versed and you are confused, so

I don't think I'm confused (or smart :), but you are trying hard to
make it seem confusing and scary.

Anyway, you are correct. We do have rules. But, I don't think those
rules prevent us from discussing *possible* changes to those rules.

> some more jr. engineer has no hope of getting it right and would
> copy the GPL code and be clueless to what he just did. As I always
> say a development process exists to slow down the best developer, at
> the price of preventing the most jr. developers from doing something
> stupid.

If we have another repo with GplDriverPkg, then I guess the same jr.
developer might just go find the code over there and copy it.

> > I would be more worried about the GPL based drivers becoming too
> > featureful over time, and the permissively licensed code not being
> > very useful. For example, I'm worried that the non-GPL OVMF may end up
> > missing a lot of features.
> 
> Then logically you should just make OVMF a GPL project?

Not if you are someone that prefers permissive source licenses, such
as myself.

I'm basically trying to argue the other side of this to not let the
GPL FUD go by unimpeded.

Laszlo's email raised the GPL question, but I was not sure what the
EDK II community would accept with regards to GPL. Thus ... I asked. I
guess I'm getting a better idea with regards to Apple and HP. :)

In your opinion, would we be able to discuss patches for a *separate*
repo with GplDriverPkg on edk2-devel?

Thanks,

-Jordan
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] UEFI and NIST SP-147 compliance

[Bill Paul]

Of all the gin joints in all the towns in all the world, Blibbet had to walk 
into mine at 11:14:29 on Wednesday 09 September 2015 and say:

> I have some questions about UEFI and the below excerpts from NIST
> SP-147, from sections 3.1.1 and 3.2.
> 
> Is this "gold master image" possible with UEFI? Are any enterprises
> practicing this? What tools are they using? I can't any information on
> any enterprise who does this today.
> 
> I currently doubt it is possible with UEFI, given how little information
> -- by UEFI Forum, IBVs, OEMs or IHVs -- is provided to system owners on
> how to backup/restore their BIOS.  At least I can't find any info so far.
> 
> Can anyone point me to a case where an organization can "assert greater
> control" or "actively participate in the update process", like below?
> 
> Most of the security in these guidelines are based on crypto, yet I've
> yet to find a CRL or OSCP URL by an IBV/OEM/IHV for any of their UEFI
> signed code.
> 
> Can anyone point to a case where someone is able to test the security of
> this signed code?
> 
> How can "multi-party control" with below PKI if the enterprise can't
> even access the original keys?
> 
> I could see the below with UEFI in a fully-open source firmware
> scenerio. But all UEFI IBVs are closed-source, and Intel's FSP is
> closed-source, so I don't see how mainstream UEFI-based systems can be
> used with 147 enterprise guidelines.
> 
> If anyone can point to any more info on this, please speak up.
> 
> I'm giving a talk to some sysadmins on integrating SP-147 with UEFI
> along with traditional hardware lifecycle models on Thursday... :-)

Oh sure, no pressure.

As you say, the closed source nature of most BIOSes makes complying with these 
requirements nearly impossible for most organizations. The only exceptions I 
can think of are big companies with connections to the IBVs (e.g. Intel, 
Microsoft) or the government/military. Something tells me none of them will in 
any rush to talk to you though. :)

-Bill

> Thanks,
> Lee
> RSS: http://firmwaresecurity.com
> 
> snip
> 
> Some organizations may wish to assert greater control over BIOS updates
> in high-security environments. The authenticated update mechanism may be
> designed to permit organizational control over the update process, where
> updates to the BIOS or rollbacks of the BIOS to an earlier version are
> permitted only if the update or rollback has been authorized by the
> organization. For example, specific BIOS images could be authorized by
> an organization by countersigning them with an organization-controlled
> key, which would be verified during the update process.
> 
> Provisioning Phase:
> It is crucial that the organization institute a mechanism for
> identifying, inventorying, and tracking the different computer systems
> across the enterprise throughout their life cycle. Identifying and
> monitoring the BIOS image characteristics such as manufacturer name,
> version, or time stamp allows the organization to perform update,
> rollback, and recovery. The organization should maintain a “golden
> master image” for each approved system BIOS, including superseded
> versions, in secure offline storage.
> If the platform has a configurable Root of Trust for Update (RTU), the
> organization needs to maintain a copy of the key store and signature
> verification algorithm. If the RTU is integrated into the system BIOS
> then this guideline is satisfied by maintaining the golden BIOS image.
> If the RTU is not integrated into the system BIOS, the security afforded
> the RTU should be at least as strong as that for the golden BIOS image.
> Most organizations will rely upon the manufacturer as the source for the
> authenticated BIOS. In this case, the organization does not maintain any
> private keys, and the RTU contains only public keys provided by the
> manufacturer. Where the organization prefers to participate actively in
> the BIOS authentication process by countersigning some or all approved
> system BIOS updates, the RTU may contain one or more public keys
> associated with the organization. In this case, the organization must
> securely maintain the corresponding private key so that the next BIOS
> update can be signed. Private keys should be maintained under
> multi-party control to protect against insider attacks. For
> organizational keys, the corresponding public keys must also be
> maintained securely (to ensure authentication of origin).
> 
> Operation and Maintenance Phase:
> Where the organization participates actively in the update process, the
> multi-party control process must be executed to retrieve the private key
> from secure storage and generate the digital signature. The BIOS
> installation package should also be signed, and the digital signature
> should be verified before execution. Once the update has executed
> successfully, the configuration baseline should be validated to confirm
> that the computer system is still in compliance with 

Re: [edk2] [Xen-devel] OVMF BoF @ KVM Forum 2015

[Laszlo Ersek]

On 09/09/15 18:34, Ian Campbell wrote:
> On Wed, 2015-09-09 at 10:57 +0200, Laszlo Ersek wrote:
>> On 08/10/15 18:24, Laszlo Ersek wrote:
>>> Hi.
>>>
>>> Let's do an OVMF BoF at this year's KVM Forum too.
>>
>> Here's a preliminary task list
> 
> Thanks for including xen-devel in this. Was anyone from the Xen community
> present at the BoF (so far as you know)?

Noone was. And I know it for a fact, because all of the handful of BoF
participants were sitting around one table. :)

(If someone speaks up now, "hey I was there, and I'm a Xen guy", then
I'll just dig myself a pit.)

> Are there any minutes or anything
> like that for those interested in the discussion and reasoning rather than
> just the resulting action items?

Not really; I think this todo list is pretty comprehensive. I remember
talking a lot and having trouble breathing (I had gotten sick just a few
days earlier -- yay crowded conferences and parties), but I tend to
speak in many details about few things, and not concisely about many things.

Should you care about those details, I've now forgotten them all. They
probably weren't important anyway. I recall trying to convince Jordan
politely to review my SMM patches. We probably discussed some SMM details.

... Please ask Jordan or the other participants, they weren't sick, and
they don't have a chronic verbosity problem.

... Man, it must be great to work with me! ;)

Thanks for your interest!
Laszlo

> 
> Thanks,
> Ian.
> 
>> , after some off-list discussion (I tried
>> to incorporate comments):
>>
>> - create GPL'd fork called "ovmf" for expediting virt development
>>   (OvmfPkg, ArmVirtPkg)
>>   - maybe leverage the feature under
>>  for
>> setting up a separate "tianocore/edk2-gpl" repo, for GPL'd
>> contributions [Jordan]
>> - repo separation by license could make things harder for packagers
>>   and QEMU bundling [Laszlo]
>> - document the rules / justification for "ovmf" (licensing
>>   conflicts, non-technical blockage on edk2 etc).
>> - No new mailing list needed
>> - push RH's downstream-only patches to "ovmf", wherever that makes
>>   sense
>> - remove encumbered FAT driver
>> - import Peter Batard's GPL r/o FAT driver port of GRUB's
>> - secure OpenSSL linking exception for the former from the copyright
>>   holders (Peter Batard, GRUB project)
>> - "ovmf" should be periodically rebased / should fetch+merge edk2 as
>>   master (arguments both for and against merging); distros should
>>   then track "ovmf" as their upstream, not edk2
>> - get OVMF into Fedora (as pkg) and QEMU (as bundled binary)
>> - do OVMF releases, maybe in sync with QEMU's releases
>>   - we can probably build from known good revisions from git [Alex]
>> - revive Q35 SATA driver work / poke Reza
>>   - Hannes and Gabriel have refreshed patches, but their versions differ
>>
>>
>> ___
>> Xen-devel mailing list
>> xen-de...@lists.xen.org
>> http://lists.xen.org/xen-devel

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] EDK II & GPL - Re: OVMF BoF @ KVM Forum 2015

[Andrew Fish]

> On Sep 9, 2015, at 9:17 AM, Jordan Justen  wrote:
> 
> On 2015-09-09 01:57:51, Laszlo Ersek wrote:
>> On 08/10/15 18:24, Laszlo Ersek wrote:
>>> Hi.
>>> 
>>> Let's do an OVMF BoF at this year's KVM Forum too.
>> 
>> Here's a preliminary task list, after some off-list discussion (I tried
>> to incorporate comments):
>> 
>> - create GPL'd fork called "ovmf" for expediting virt development
>>  (OvmfPkg, ArmVirtPkg)
>>  - maybe leverage the feature under
>>
>> >  > for
>>setting up a separate "tianocore/edk2-gpl" repo, for GPL'd
>>contributions [Jordan]
>>- repo separation by license could make things harder for packagers
>>  and QEMU bundling [Laszlo]
> 
> I would like OVMF to follow a plan for GPL that the whole EDK II
> community decides on.
> 
> I would also like to see EDK II add a (permissively licensed; BSD,
> MIT, etc) DriverPkg (DriversPkg?). We discussed this on the list
> recently:
> 
> https://urldefense.proofpoint.com/v2/url?u=http-3A__thread.gmane.org_gmane.comp.bios.tianocore.devel_17544_focus-3D676=BQICAg=eEvniauFctOgLOKGJOplqw=1HnUuXD1wDvw67rut5_idw=Izfl0sCkWmvl39u6ZohbZ52fhf2U-GzLJ9cQpL9sdnA=HauTLYzaQYz935VMYQCCR8xzuSADw1ZcWSSjSyEXIpw=
>  
> 
> So, related to this, I wonder how the community would feel about a
> GplDriverPkg. Would the community allow it as a new package in EDK II
> directly, or would a separate repo be required?
> 

I think we would need a separate repo, like the FAT driver. That is the only 
way to deal with the license issues. 

> With regards to adding it directly into the EDK II tree, here are some
> potential concerns that I might anticipate hearing from the community:
> 
> * It will make it easier for contributors to choose GPL compared to a
>  permissive license, thereby limiting some users of the contribution
> 
> * GPL code will more easily be copied into the permissively licensed
>  packages
> 
> * Some might refuse to look at EDK II entirely if it has a directory
>  with GPL source code in it
> 

Or have their rights to contribute revoked since this is a fundamental change, 
and would require employees to get reauthorized by their legal departments to 
contribute. 

> Of these, I personally only sympathize with the first.
> 

Your employer probably has issues with all of this. 

> Regarding the separate OVMF repo, my hope is that it is more of a OVMF
> specific working area, rather than a 'GPL OVMF'. For example, patches
> or features that we've not yet figured out how to upstream, but we
> hopefully plan to.
> 

Mixing is never going to work. If you mix then the pure BSD edk2 will fork and 
continue, I’m guessing a lot of the Intel resources will work on that version 
as it will be the one enabling Intel products. 

> Additionally, it makes sense to use it as needed for OVMF specific
> releases. (Ie, OVMF release tags)
> 

Seems to me we are really solving the wrong problem. What we need is a smaller 
BSD edk2 project that supports the industry standards and works on some set of 
BSD platforms. There can be down stream consumers that have more platform 
support, and different licenses. 

Seems the real problem is the unpredictable changes to MdePkg libraries, and 
MdeModulePkg infrastructure. So it is the lack of a transparent plan, and 
discussion about upcoming changes that break compatibility between different 
projects. So the solution is to keep everything in the tree working on master. 
We should fix the edk2 process, and place a process in place to communicate 
pending non backward compatible changes in the edk2 to down stream consumers. 

Don’t forget a lot (majority) of edk2 based products that ship today live in a 
separate repo that likely contains a UDK or specific version of edk2 with 
cherry picked patches. So this processes is kind of already happening in the 
non-GPL world….

Thanks,

Andrew Fish

> -Jordan
> 
>> - document the rules / justification for "ovmf" (licensing
>>  conflicts, non-technical blockage on edk2 etc).
>> - No new mailing list needed
>> - push RH's downstream-only patches to "ovmf", wherever that makes
>>  sense
>> - remove encumbered FAT driver
>> - import Peter Batard's GPL r/o FAT driver port of GRUB's
>> - secure OpenSSL linking exception for the former from the copyright
>>  holders (Peter Batard, GRUB project)
>> - "ovmf" should be periodically rebased / should fetch+merge edk2 as
>>  master (arguments both for and against merging); distros should
>>  then track "ovmf" as their upstream, not edk2
>> - get OVMF into Fedora (as pkg) and QEMU (as bundled binary)
>> - do OVMF releases, maybe in sync with QEMU's releases
>>  - we can probably build from known good revisions from git [Alex]
>> - revive Q35 SATA driver work / poke Reza

Re: [edk2] Section Alignment of elf binaries compiled with GCC(Linux)

[Michael Zimmermann]

yes, I tested it and it works.
I use Linaro's linux-gnueabi 4.9 toolchain from here:
http://releases.linaro.org/15.05/components/toolchain/binaries/arm-linux-gnueabi/

I've copied the manifest info so you don't have to download the toolchain:
http://pastebin.com/jvyPcMkz

On Wed, Sep 9, 2015 at 7:43 PM, Ard Biesheuvel 
wrote:

> On 9 September 2015 at 18:49, Michael Zimmermann
>  wrote:
> > thx, the patches work just fine.(they apply and the runtime warnings are
> > gone).
>
> OK, great. May I take that as a tested-by? Which GCC version are you using
> btw?
>
> > if edk2 patches aren't that usable, how the maintainers test it? because
> > usually people don't post git links.
> >
> > On Wed, Sep 9, 2015 at 6:31 PM, Ard Biesheuvel <
> ard.biesheu...@linaro.org>
> > wrote:
> >>
> >> On 9 September 2015 at 18:21, Michael Zimmermann
> >>  wrote:
> >> > what I meant is that git am fails:
> >> > Applying: BaseTools/GenFw: remove ARM and RVCT references from ELF64
> >> > code
> >> > error: patch failed: BaseTools/Source/C/GenFw/Elf64Convert.c:360
> >> > error: BaseTools/Source/C/GenFw/Elf64Convert.c: patch does not apply
> >> > Patch failed at 0001 BaseTools/GenFw: remove ARM and RVCT references
> >> > from
> >> > ELF64 code
> >> > The copy of the patch that failed is found in:
> >> >
> >> >
> /media/Data/repositories/git/efidroid/uefi/edk2/.git/rebase-apply/patch
> >> > When you have resolved this problem, run "git am --continue".
> >> > If you prefer to skip this patch, run "git am --skip" instead.
> >> > To restore the original branch and stop patching, run "git am
> --abort".
> >> >
> >> > maybe I did sth. wrong?(I've never used a mailing list patch before).
> I
> >> > downloaded the mbox from gmane:
> >> > http://download.gmane.org/gmane.comp.bios.edk2.devel/2036/2037
> >> >
> >>
> >> No, it is just painful, with the CR/LF conversion etc
> >>
> >> What I usually do is
> >>
> >> $ cat |todos |git am
> >>
> >> and then paste it, followed by Ctrl-D. That will usually work, unless
> >> the patch adds or removes files.
> >>
> >> I pushed the patches here:
> >>
> >>
> https://git.linaro.org/people/ard.biesheuvel/uefi-next.git/shortlog/refs/heads/arm-memprot
> >>
> >> So after applying you need to
> >> - git clean -dxf BaseTools/ ; make -C BaseTools
> >> - rm Conf/tools_def.txt ; source edksetup.sh
> >>
> >> to make sure everything is up to date after the changes
> >>
> >> --
> >> Ard.
> >>
> >> > On Wed, Sep 9, 2015 at 6:17 PM, Ard Biesheuvel
> >> > 
> >> > wrote:
> >> >>
> >> >>
> >> >>
> >> >> On 9 sep. 2015, at 18:10, Michael Zimmermann <
> sigmaepsilo...@gmail.com>
> >> >> wrote:
> >> >>
> >> >> Yes I'm using 32bit ARM :)
> >> >> thx for the patches - unfortunatelythe patches fail for me.
> >> >>
> >> >>
> >> >> Did you regenerate Conf/tools_def.txt  and rebuild the BaseTools/ ?
> >> >>
> >> >> On Wed, Sep 9, 2015 at 5:33 PM, Ard Biesheuvel
> >> >> 
> >> >> wrote:
> >> >>>
> >> >>> On 9 September 2015 at 17:26, Gao, Liming 
> >> >>> wrote:
> >> >>> > Michael:
> >> >>> >   Do you use the linker script BaseTools/Scripts/GccBase.lds and
> -z
> >> >>> > common-page-size=4096?
> >> >>> >
> >> >>>
> >> >>> Are you building for 32-bit ARM by any chance? That does not have
> this
> >> >>> feature wired up yet. I posted a v2 of my series that addresses this
> >> >>> yesterday.
> >> >>>
> >> >>>
> >> >>>
> >> >>> > From: Michael Zimmermann [mailto:sigmaepsilo...@gmail.com]
> >> >>> > Sent: Wednesday, September 9, 2015 3:22 PM
> >> >>> > To: Yao, Jiewen
> >> >>> > Cc: Gao, Liming; edk2-devel@lists.01.org
> >> >>> > Subject: Re: [edk2] Section Alignment of elf binaries compiled
> with
> >> >>> > GCC(Linux)
> >> >>> >
> >> >>> > Unfortunately I missed the replies but I debugged this problem
> >> >>> > further
> >> >>> > and the Problem is that GenFw set's the alignment based on
> >> >>> > "sh_addralign" in
> >> >>> > the Elf header.
> >> >>> >
> >> >>> > The 'common-page-size' flag doesn't change the value of this field
> >> >>> > though. what it does change is the Alignment value of the Program
> >> >>> > Headers:
> >> >>> > Program Headers:
> >> >>> >   Type   Offset   VirtAddr   PhysAddr   FileSiz MemSiz
> Flg
> >> >>> > Align
> >> >>> >   LOAD   0x01 0x 0x 0x042f8 0x042f8 R
> E
> >> >>> > 0x1
> >> >>> >   LOAD   0x0142f8 0x000142f8 0x000142f8 0x000d4 0x000f4 RW
> >> >>> > 0x1
> >> >>> >   GNU_STACK  0x00 0x 0x 0x0 0x0
> RWE
> >> >>> > 0x10
> >> >>> > the section alignment still stays at 0/1/4.
> >> >>> >
> >> >>> > On Tue, Aug 18, 2015 at 4:49 AM, Yao, Jiewen
> >> >>> > > wrote:
> >> >>> > OK
> >> >>> >
> >> >>> > -Original Message-
> >> >>> > From: Gao, Liming
> >> >>> > Sent: Tuesday, August 18, 2015 10:48 AM
> >> >>> > To: 

Re: [edk2] EDK II & GPL - Re: OVMF BoF @ KVM Forum 2015

[Blibbet]

Short term an OVMF-centric solution is good.

But long term, I think Linux needs a Linux-friendly IBV to build native
UEFI -- as well as OVMF-flavored UEFI -- with non-BSD licensed community
code. If you restrict this to just OVMF, any GPL innovations will only
happen at virtual level. Right now, boot loaders (eg, rEFInd) and VM
projects (VirtualBox) are the only ones to benefit from UEFI
non-BSD-licensed code. An IBV could offer these enhancements to Linux
OEMs, not just server VMs. Linaro is already an IBV, to a degree, they
produce UEFI binaries for the supported dev boards, as part of BSP build
process.

Look at the coreboot blog, the other week the Purism BIOS developer was
talking about a "Free Software port of FSP". That should be a shared
effort by all Linux OEMs/vendors, not shouldered by a single OEM. A
Linux-centric IBV could be getting help from multiple companies -- like
Linaro does with ARM -- to help with this effort. It could be part of
Linux Foundation's Core Infrastructure Initiative, maybe.

They should also tailor Linux-friendly ACPI, like recent thread about
Linux standardization of _DSD. As well as not include a WBPT table in
Linux OEM systems, and look at the other tables to see what's should be
removed.

Alternately, the UEFI Forum should create a non-BSD tree to contain
this, not just focus on BSD for the fully-closed-source end of the
spectrum, and work with some FOSS-centric OSVs to better support UEFI
using their community's models.

I hope an IBV is listening. Create a separate project from your current
closed-source code, and fully-embrace the open source community.

Today, Sage Engineering is the only Open Source-friendly IBV that I'm
aware of. They mostly focus on coreboot, not UEFI, though. (Like
Phoenix, their blob recently went dark, I hope they're doing ok.)

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Section Alignment of elf binaries compiled with GCC(Linux)

[Laszlo Ersek]

On 09/09/15 18:21, Michael Zimmermann wrote:
> what I meant is that git am fails:
> Applying: BaseTools/GenFw: remove ARM and RVCT references from ELF64 code
> error: patch failed: BaseTools/Source/C/GenFw/Elf64Convert.c:360
> error: BaseTools/Source/C/GenFw/Elf64Convert.c: patch does not apply
> Patch failed at 0001 BaseTools/GenFw: remove ARM and RVCT references from
> ELF64 code
> The copy of the patch that failed is found in:
>/media/Data/repositories/git/efidroid/uefi/edk2/.git/rebase-apply/patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
> 
> maybe I did sth. wrong?(I've never used a mailing list patch before). I
> downloaded the mbox from gmane:
> http://download.gmane.org/gmane.comp.bios.edk2.devel/2036/2037

Applying specifically edk2 patches from the mailing list is not easy;
the source files have CRLF line terminators, which leads to all kinds of
"fun". It's doable, but you shouldn't burden yourself with that. Rather
ask Ard to push his work somewhere public, and then you should fetch it
with git.

Thanks
Laszlo


> On Wed, Sep 9, 2015 at 6:17 PM, Ard Biesheuvel 
> wrote:
> 
>>
>>
>> On 9 sep. 2015, at 18:10, Michael Zimmermann 
>> wrote:
>>
>> Yes I'm using 32bit ARM :)
>> thx for the patches - unfortunatelythe patches fail for me.
>>
>>
>> Did you regenerate Conf/tools_def.txt  and rebuild the BaseTools/ ?
>>
>> On Wed, Sep 9, 2015 at 5:33 PM, Ard Biesheuvel 
>> wrote:
>>
>>> On 9 September 2015 at 17:26, Gao, Liming  wrote:
 Michael:
   Do you use the linker script BaseTools/Scripts/GccBase.lds and -z
>>> common-page-size=4096?

>>>
>>> Are you building for 32-bit ARM by any chance? That does not have this
>>> feature wired up yet. I posted a v2 of my series that addresses this
>>> yesterday.
>>>
>>>
>>>
 From: Michael Zimmermann [mailto:sigmaepsilo...@gmail.com]
 Sent: Wednesday, September 9, 2015 3:22 PM
 To: Yao, Jiewen
 Cc: Gao, Liming; edk2-devel@lists.01.org
 Subject: Re: [edk2] Section Alignment of elf binaries compiled with
>>> GCC(Linux)

 Unfortunately I missed the replies but I debugged this problem further
>>> and the Problem is that GenFw set's the alignment based on "sh_addralign"
>>> in the Elf header.

 The 'common-page-size' flag doesn't change the value of this field
>>> though. what it does change is the Alignment value of the Program Headers:
 Program Headers:
   Type   Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg
>>> Align
   LOAD   0x01 0x 0x 0x042f8 0x042f8 R E
>>> 0x1
   LOAD   0x0142f8 0x000142f8 0x000142f8 0x000d4 0x000f4 RW
>>> 0x1
   GNU_STACK  0x00 0x 0x 0x0 0x0 RWE 0x10
 the section alignment still stays at 0/1/4.

 On Tue, Aug 18, 2015 at 4:49 AM, Yao, Jiewen >> > wrote:
 OK

 -Original Message-
 From: Gao, Liming
 Sent: Tuesday, August 18, 2015 10:48 AM
 To: Yao, Jiewen; Michael Zimmermann; edk2-devel@lists.01.org>> edk2-devel@lists.01.org>
 Subject: RE: [edk2] Section Alignment of elf binaries compiled with
>>> GCC(Linux)

 Jiewen:
   The updated message is useful. I suggest to change error level from
>>> EFI_D_ERROR to EFI_D_INFO.

 Thanks
 Liming
 -Original Message-
 From: Yao, Jiewen
 Sent: Tuesday, August 18, 2015 10:46 AM
 To: Gao, Liming; Michael Zimmermann; edk2-devel@lists.01.org>> edk2-devel@lists.01.org>
 Subject: RE: [edk2] Section Alignment of elf binaries compiled with
>>> GCC(Linux)

 Hi
 How about we update debug message like below:

 Index: PropertiesTable.c
 ===
 --- PropertiesTable.c   (revision 18191)
 +++ PropertiesTable.c   (working copy)
 @@ -1120,7 +1120,7 @@

SetPropertiesTableSectionAlignment (SectionAlignment);
if ((SectionAlignment & (EFI_ACPI_RUNTIME_PAGE_ALLOCATION_ALIGNMENT
>>> - 1)) != 0) {
 -DEBUG ((EFI_D_ERROR, "  InsertImageRecord - Section
>>> Alignment(0x%x) is not %dK  \n",
 +DEBUG ((EFI_D_ERROR, "  UEFI2.5 PropertiesTable - Runtime
>>> Driver Section Alignment(0x%x) is not %dK  \n",
SectionAlignment, EFI_ACPI_RUNTIME_PAGE_ALLOCATION_ALIGNMENT >>
>>> 10));
  PdbPointer = PeCoffLoaderGetPdbPointer ((VOID*) (UINTN)
>>> ImageAddress);
  if (PdbPointer != NULL) {




 -Original Message-
 From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org>> edk2-devel-boun...@lists.01.org>] On Behalf Of Gao, Liming
 Sent: Tuesday, August 18, 2015 

Re: [edk2] Section Alignment of elf binaries compiled with GCC(Linux)

[Michael Zimmermann]

thx, the patches work just fine.(they apply and the runtime warnings are
gone).
if edk2 patches aren't that usable, how the maintainers test it? because
usually people don't post git links.

On Wed, Sep 9, 2015 at 6:31 PM, Ard Biesheuvel 
wrote:

> On 9 September 2015 at 18:21, Michael Zimmermann
>  wrote:
> > what I meant is that git am fails:
> > Applying: BaseTools/GenFw: remove ARM and RVCT references from ELF64 code
> > error: patch failed: BaseTools/Source/C/GenFw/Elf64Convert.c:360
> > error: BaseTools/Source/C/GenFw/Elf64Convert.c: patch does not apply
> > Patch failed at 0001 BaseTools/GenFw: remove ARM and RVCT references from
> > ELF64 code
> > The copy of the patch that failed is found in:
> >
> /media/Data/repositories/git/efidroid/uefi/edk2/.git/rebase-apply/patch
> > When you have resolved this problem, run "git am --continue".
> > If you prefer to skip this patch, run "git am --skip" instead.
> > To restore the original branch and stop patching, run "git am --abort".
> >
> > maybe I did sth. wrong?(I've never used a mailing list patch before). I
> > downloaded the mbox from gmane:
> > http://download.gmane.org/gmane.comp.bios.edk2.devel/2036/2037
> >
>
> No, it is just painful, with the CR/LF conversion etc
>
> What I usually do is
>
> $ cat |todos |git am
>
> and then paste it, followed by Ctrl-D. That will usually work, unless
> the patch adds or removes files.
>
> I pushed the patches here:
>
> https://git.linaro.org/people/ard.biesheuvel/uefi-next.git/shortlog/refs/heads/arm-memprot
>
> So after applying you need to
> - git clean -dxf BaseTools/ ; make -C BaseTools
> - rm Conf/tools_def.txt ; source edksetup.sh
>
> to make sure everything is up to date after the changes
>
> --
> Ard.
>
> > On Wed, Sep 9, 2015 at 6:17 PM, Ard Biesheuvel <
> ard.biesheu...@linaro.org>
> > wrote:
> >>
> >>
> >>
> >> On 9 sep. 2015, at 18:10, Michael Zimmermann 
> >> wrote:
> >>
> >> Yes I'm using 32bit ARM :)
> >> thx for the patches - unfortunatelythe patches fail for me.
> >>
> >>
> >> Did you regenerate Conf/tools_def.txt  and rebuild the BaseTools/ ?
> >>
> >> On Wed, Sep 9, 2015 at 5:33 PM, Ard Biesheuvel <
> ard.biesheu...@linaro.org>
> >> wrote:
> >>>
> >>> On 9 September 2015 at 17:26, Gao, Liming 
> wrote:
> >>> > Michael:
> >>> >   Do you use the linker script BaseTools/Scripts/GccBase.lds and -z
> >>> > common-page-size=4096?
> >>> >
> >>>
> >>> Are you building for 32-bit ARM by any chance? That does not have this
> >>> feature wired up yet. I posted a v2 of my series that addresses this
> >>> yesterday.
> >>>
> >>>
> >>>
> >>> > From: Michael Zimmermann [mailto:sigmaepsilo...@gmail.com]
> >>> > Sent: Wednesday, September 9, 2015 3:22 PM
> >>> > To: Yao, Jiewen
> >>> > Cc: Gao, Liming; edk2-devel@lists.01.org
> >>> > Subject: Re: [edk2] Section Alignment of elf binaries compiled with
> >>> > GCC(Linux)
> >>> >
> >>> > Unfortunately I missed the replies but I debugged this problem
> further
> >>> > and the Problem is that GenFw set's the alignment based on
> "sh_addralign" in
> >>> > the Elf header.
> >>> >
> >>> > The 'common-page-size' flag doesn't change the value of this field
> >>> > though. what it does change is the Alignment value of the Program
> Headers:
> >>> > Program Headers:
> >>> >   Type   Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg
> >>> > Align
> >>> >   LOAD   0x01 0x 0x 0x042f8 0x042f8 R E
> >>> > 0x1
> >>> >   LOAD   0x0142f8 0x000142f8 0x000142f8 0x000d4 0x000f4 RW
> >>> > 0x1
> >>> >   GNU_STACK  0x00 0x 0x 0x0 0x0 RWE
> >>> > 0x10
> >>> > the section alignment still stays at 0/1/4.
> >>> >
> >>> > On Tue, Aug 18, 2015 at 4:49 AM, Yao, Jiewen
> >>> > > wrote:
> >>> > OK
> >>> >
> >>> > -Original Message-
> >>> > From: Gao, Liming
> >>> > Sent: Tuesday, August 18, 2015 10:48 AM
> >>> > To: Yao, Jiewen; Michael Zimmermann;
> >>> > edk2-devel@lists.01.org
> >>> > Subject: RE: [edk2] Section Alignment of elf binaries compiled with
> >>> > GCC(Linux)
> >>> >
> >>> > Jiewen:
> >>> >   The updated message is useful. I suggest to change error level from
> >>> > EFI_D_ERROR to EFI_D_INFO.
> >>> >
> >>> > Thanks
> >>> > Liming
> >>> > -Original Message-
> >>> > From: Yao, Jiewen
> >>> > Sent: Tuesday, August 18, 2015 10:46 AM
> >>> > To: Gao, Liming; Michael Zimmermann;
> >>> > edk2-devel@lists.01.org
> >>> > Subject: RE: [edk2] Section Alignment of elf binaries compiled with
> >>> > GCC(Linux)
> >>> >
> >>> > Hi
> >>> > How about we update debug message like below:
> >>> >
> >>> > Index: PropertiesTable.c
> >>> > ===
> >>> > --- PropertiesTable.c   (revision 18191)
> >>> > +++ PropertiesTable.c   

Re: [edk2] Section Alignment of elf binaries compiled with GCC(Linux)

[Ard Biesheuvel]

On 9 September 2015 at 18:49, Michael Zimmermann
 wrote:
> thx, the patches work just fine.(they apply and the runtime warnings are
> gone).

OK, great. May I take that as a tested-by? Which GCC version are you using btw?

> if edk2 patches aren't that usable, how the maintainers test it? because
> usually people don't post git links.
>
> On Wed, Sep 9, 2015 at 6:31 PM, Ard Biesheuvel 
> wrote:
>>
>> On 9 September 2015 at 18:21, Michael Zimmermann
>>  wrote:
>> > what I meant is that git am fails:
>> > Applying: BaseTools/GenFw: remove ARM and RVCT references from ELF64
>> > code
>> > error: patch failed: BaseTools/Source/C/GenFw/Elf64Convert.c:360
>> > error: BaseTools/Source/C/GenFw/Elf64Convert.c: patch does not apply
>> > Patch failed at 0001 BaseTools/GenFw: remove ARM and RVCT references
>> > from
>> > ELF64 code
>> > The copy of the patch that failed is found in:
>> >
>> > /media/Data/repositories/git/efidroid/uefi/edk2/.git/rebase-apply/patch
>> > When you have resolved this problem, run "git am --continue".
>> > If you prefer to skip this patch, run "git am --skip" instead.
>> > To restore the original branch and stop patching, run "git am --abort".
>> >
>> > maybe I did sth. wrong?(I've never used a mailing list patch before). I
>> > downloaded the mbox from gmane:
>> > http://download.gmane.org/gmane.comp.bios.edk2.devel/2036/2037
>> >
>>
>> No, it is just painful, with the CR/LF conversion etc
>>
>> What I usually do is
>>
>> $ cat |todos |git am
>>
>> and then paste it, followed by Ctrl-D. That will usually work, unless
>> the patch adds or removes files.
>>
>> I pushed the patches here:
>>
>> https://git.linaro.org/people/ard.biesheuvel/uefi-next.git/shortlog/refs/heads/arm-memprot
>>
>> So after applying you need to
>> - git clean -dxf BaseTools/ ; make -C BaseTools
>> - rm Conf/tools_def.txt ; source edksetup.sh
>>
>> to make sure everything is up to date after the changes
>>
>> --
>> Ard.
>>
>> > On Wed, Sep 9, 2015 at 6:17 PM, Ard Biesheuvel
>> > 
>> > wrote:
>> >>
>> >>
>> >>
>> >> On 9 sep. 2015, at 18:10, Michael Zimmermann 
>> >> wrote:
>> >>
>> >> Yes I'm using 32bit ARM :)
>> >> thx for the patches - unfortunatelythe patches fail for me.
>> >>
>> >>
>> >> Did you regenerate Conf/tools_def.txt  and rebuild the BaseTools/ ?
>> >>
>> >> On Wed, Sep 9, 2015 at 5:33 PM, Ard Biesheuvel
>> >> 
>> >> wrote:
>> >>>
>> >>> On 9 September 2015 at 17:26, Gao, Liming 
>> >>> wrote:
>> >>> > Michael:
>> >>> >   Do you use the linker script BaseTools/Scripts/GccBase.lds and -z
>> >>> > common-page-size=4096?
>> >>> >
>> >>>
>> >>> Are you building for 32-bit ARM by any chance? That does not have this
>> >>> feature wired up yet. I posted a v2 of my series that addresses this
>> >>> yesterday.
>> >>>
>> >>>
>> >>>
>> >>> > From: Michael Zimmermann [mailto:sigmaepsilo...@gmail.com]
>> >>> > Sent: Wednesday, September 9, 2015 3:22 PM
>> >>> > To: Yao, Jiewen
>> >>> > Cc: Gao, Liming; edk2-devel@lists.01.org
>> >>> > Subject: Re: [edk2] Section Alignment of elf binaries compiled with
>> >>> > GCC(Linux)
>> >>> >
>> >>> > Unfortunately I missed the replies but I debugged this problem
>> >>> > further
>> >>> > and the Problem is that GenFw set's the alignment based on
>> >>> > "sh_addralign" in
>> >>> > the Elf header.
>> >>> >
>> >>> > The 'common-page-size' flag doesn't change the value of this field
>> >>> > though. what it does change is the Alignment value of the Program
>> >>> > Headers:
>> >>> > Program Headers:
>> >>> >   Type   Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg
>> >>> > Align
>> >>> >   LOAD   0x01 0x 0x 0x042f8 0x042f8 R E
>> >>> > 0x1
>> >>> >   LOAD   0x0142f8 0x000142f8 0x000142f8 0x000d4 0x000f4 RW
>> >>> > 0x1
>> >>> >   GNU_STACK  0x00 0x 0x 0x0 0x0 RWE
>> >>> > 0x10
>> >>> > the section alignment still stays at 0/1/4.
>> >>> >
>> >>> > On Tue, Aug 18, 2015 at 4:49 AM, Yao, Jiewen
>> >>> > > wrote:
>> >>> > OK
>> >>> >
>> >>> > -Original Message-
>> >>> > From: Gao, Liming
>> >>> > Sent: Tuesday, August 18, 2015 10:48 AM
>> >>> > To: Yao, Jiewen; Michael Zimmermann;
>> >>> > edk2-devel@lists.01.org
>> >>> > Subject: RE: [edk2] Section Alignment of elf binaries compiled with
>> >>> > GCC(Linux)
>> >>> >
>> >>> > Jiewen:
>> >>> >   The updated message is useful. I suggest to change error level
>> >>> > from
>> >>> > EFI_D_ERROR to EFI_D_INFO.
>> >>> >
>> >>> > Thanks
>> >>> > Liming
>> >>> > -Original Message-
>> >>> > From: Yao, Jiewen
>> >>> > Sent: Tuesday, August 18, 2015 10:46 AM
>> >>> > To: Gao, Liming; Michael Zimmermann;
>> >>> > edk2-devel@lists.01.org
>> >>> > Subject: RE: 

Re: [edk2] EDK II & GPL - Re: OVMF BoF @ KVM Forum 2015

[Jordan Justen]

On 2015-09-09 10:04:50, Andrew Fish wrote:
> 
> > On Sep 9, 2015, at 9:17 AM, Jordan Justen  wrote:
> > 
> > So, related to this, I wonder how the community would feel about a
> > GplDriverPkg. Would the community allow it as a new package in EDK II
> > directly, or would a separate repo be required?
> > 
> 
> I think we would need a separate repo, like the FAT driver. That is
> the only way to deal with the license issues.

There doesn't seem to be any guiding rules here. For example, I think
some people are not comfortable with the FatBinPkg being in the tree
due to the license. Why is that okay?

> > With regards to adding it directly into the EDK II tree, here are some
> > potential concerns that I might anticipate hearing from the community:
> > 
> > * It will make it easier for contributors to choose GPL compared to a
> >  permissive license, thereby limiting some users of the contribution
> > 
> > * GPL code will more easily be copied into the permissively licensed
> >  packages
> > 
> > * Some might refuse to look at EDK II entirely if it has a directory
> >  with GPL source code in it
> > 
> 
> Or have their rights to contribute revoked since this is a
> fundamental change, and would require employees to get reauthorized
> by their legal departments to contribute.

We've recently expanded beyond just allowing BSD code into the tree,
and that appeared to be no big deal. No one brought this up as a
fundamental change.

Just to be clear, are you saying Apple probably won't be able to
contribute to EDK II if there is any GPL licensed code in the tree?
(Even if it is contained in a clearly indicated package.) I guess
using dual-licensed BSD/GPL is okay though?
(EmbeddedPkg/Library/FdtLib)

-Jordan
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 3/3] ArmVirtPkg: set max physical address width to 40 bits

[Andrew Fish]

> On Sep 9, 2015, at 10:02 AM, Ard Biesheuvel  wrote:
> 
>> 
>> 
>> The 1:1 mapping only goes to 4 GB, so anything beyond that is never
>> mapped anyway.
>> 
>> 
>> This is the state when you hand off to the OS, but it is possible as part of
>> the boot process to have a driver do some alternate mapping. This was done
>> on x86 servers for a software based memory test for example.
>> 
> 
> OK, thanks Andrew. I have no idea if the 32-bit ARM MMU code would
> deal with that at all (you would probably know better than I), but I
> was just trying to get my 32-bit QEMU working with > 3 GB of memory.
> 
> So in patch #2, I needed to split the memory into two regions to get
> the DXE core to handle it correctly, one below 4 GB and one above 4
> GB. Is that expected?

There could be some assumptions, like the memory > 4GB is passed up to the DXE 
Core untested, or added later. 
This seems like an area that could be documented better in the PI 
specification. Maybe the MdeModulePkg maintainer would have some insight into 
the assumptions in the DXE Core implementation?

Thanks,

Andrew Fish
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Section Alignment of elf binaries compiled with GCC(Linux)

[Laszlo Ersek]

On 09/09/15 18:49, Michael Zimmermann wrote:
> thx, the patches work just fine.(they apply and the runtime warnings are
> gone).
> if edk2 patches aren't that usable, how the maintainers test it? because
> usually people don't post git links.

For shorter series, they are willing to mangle some (not all!) line
terminators in the patches. For longer series, it is considered part of
the contributor "etiquette" to push your patches without being asked (or
at least after being asked). The expectations also vary by maintainer /
top level Pkg.

... Actually, taking a step back: you expect maintainers to *test*
patches from contributors? Ain't nobody got time for that! ;)

Thanks
Laszlo

> 
> On Wed, Sep 9, 2015 at 6:31 PM, Ard Biesheuvel 
> wrote:
> 
>> On 9 September 2015 at 18:21, Michael Zimmermann
>>  wrote:
>>> what I meant is that git am fails:
>>> Applying: BaseTools/GenFw: remove ARM and RVCT references from ELF64 code
>>> error: patch failed: BaseTools/Source/C/GenFw/Elf64Convert.c:360
>>> error: BaseTools/Source/C/GenFw/Elf64Convert.c: patch does not apply
>>> Patch failed at 0001 BaseTools/GenFw: remove ARM and RVCT references from
>>> ELF64 code
>>> The copy of the patch that failed is found in:
>>>
>> /media/Data/repositories/git/efidroid/uefi/edk2/.git/rebase-apply/patch
>>> When you have resolved this problem, run "git am --continue".
>>> If you prefer to skip this patch, run "git am --skip" instead.
>>> To restore the original branch and stop patching, run "git am --abort".
>>>
>>> maybe I did sth. wrong?(I've never used a mailing list patch before). I
>>> downloaded the mbox from gmane:
>>> http://download.gmane.org/gmane.comp.bios.edk2.devel/2036/2037
>>>
>>
>> No, it is just painful, with the CR/LF conversion etc
>>
>> What I usually do is
>>
>> $ cat |todos |git am
>>
>> and then paste it, followed by Ctrl-D. That will usually work, unless
>> the patch adds or removes files.
>>
>> I pushed the patches here:
>>
>> https://git.linaro.org/people/ard.biesheuvel/uefi-next.git/shortlog/refs/heads/arm-memprot
>>
>> So after applying you need to
>> - git clean -dxf BaseTools/ ; make -C BaseTools
>> - rm Conf/tools_def.txt ; source edksetup.sh
>>
>> to make sure everything is up to date after the changes
>>
>> --
>> Ard.
>>
>>> On Wed, Sep 9, 2015 at 6:17 PM, Ard Biesheuvel <
>> ard.biesheu...@linaro.org>
>>> wrote:



 On 9 sep. 2015, at 18:10, Michael Zimmermann 
 wrote:

 Yes I'm using 32bit ARM :)
 thx for the patches - unfortunatelythe patches fail for me.


 Did you regenerate Conf/tools_def.txt  and rebuild the BaseTools/ ?

 On Wed, Sep 9, 2015 at 5:33 PM, Ard Biesheuvel <
>> ard.biesheu...@linaro.org>
 wrote:
>
> On 9 September 2015 at 17:26, Gao, Liming 
>> wrote:
>> Michael:
>>   Do you use the linker script BaseTools/Scripts/GccBase.lds and -z
>> common-page-size=4096?
>>
>
> Are you building for 32-bit ARM by any chance? That does not have this
> feature wired up yet. I posted a v2 of my series that addresses this
> yesterday.
>
>
>
>> From: Michael Zimmermann [mailto:sigmaepsilo...@gmail.com]
>> Sent: Wednesday, September 9, 2015 3:22 PM
>> To: Yao, Jiewen
>> Cc: Gao, Liming; edk2-devel@lists.01.org
>> Subject: Re: [edk2] Section Alignment of elf binaries compiled with
>> GCC(Linux)
>>
>> Unfortunately I missed the replies but I debugged this problem
>> further
>> and the Problem is that GenFw set's the alignment based on
>> "sh_addralign" in
>> the Elf header.
>>
>> The 'common-page-size' flag doesn't change the value of this field
>> though. what it does change is the Alignment value of the Program
>> Headers:
>> Program Headers:
>>   Type   Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg
>> Align
>>   LOAD   0x01 0x 0x 0x042f8 0x042f8 R E
>> 0x1
>>   LOAD   0x0142f8 0x000142f8 0x000142f8 0x000d4 0x000f4 RW
>> 0x1
>>   GNU_STACK  0x00 0x 0x 0x0 0x0 RWE
>> 0x10
>> the section alignment still stays at 0/1/4.
>>
>> On Tue, Aug 18, 2015 at 4:49 AM, Yao, Jiewen
>> > wrote:
>> OK
>>
>> -Original Message-
>> From: Gao, Liming
>> Sent: Tuesday, August 18, 2015 10:48 AM
>> To: Yao, Jiewen; Michael Zimmermann;
>> edk2-devel@lists.01.org
>> Subject: RE: [edk2] Section Alignment of elf binaries compiled with
>> GCC(Linux)
>>
>> Jiewen:
>>   The updated message is useful. I suggest to change error level from
>> EFI_D_ERROR to EFI_D_INFO.
>>
>> Thanks
>> Liming
>> -Original Message-
>> From: Yao, Jiewen
>> Sent: Tuesday, August 18, 

[edk2] [PATCH] ArmPlatformPkg/ARMJunoPkg: Correct Interrupt Numbers for PCIe Root port

[Supreeth Venkatesh]

Support for PCI IO range with ACPI on JUNO.
Interrupt Number Reference: 
http://www.arm.com/files/pdf/DDI0515D1a_juno_arm_development_platform_soc_trm.pdf
 table 3-3 page 3-7

Contributed-under: TianoCore Contribution Agreement 1.0

Signed-off-by: Supreeth Venkatesh 
---
 .../ArmJunoPkg/AcpiTables/AcpiSsdtRootPci.asl  | 22 ++
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/ArmPlatformPkg/ArmJunoPkg/AcpiTables/AcpiSsdtRootPci.asl 
b/ArmPlatformPkg/ArmJunoPkg/AcpiTables/AcpiSsdtRootPci.asl
index c6d5a9a..7fc7f1e 100644
--- a/ArmPlatformPkg/ArmJunoPkg/AcpiTables/AcpiSsdtRootPci.asl
+++ b/ArmPlatformPkg/ArmJunoPkg/AcpiTables/AcpiSsdtRootPci.asl
@@ -59,10 +59,10 @@ DefinitionBlock("SsdtPci.aml", "SSDT", 1, "ARMLTD", 
"ARM-JUNO", EFI_ACPI_ARM_OEM
 
 // PCI Routing Table
Name(_PRT, Package() {
-   ROOT_PRT_ENTRY(0, 136),   // INTA
-   ROOT_PRT_ENTRY(1, 137),   // INTB
-   ROOT_PRT_ENTRY(2, 138),   // INTC
-   ROOT_PRT_ENTRY(3, 139),   // INTD
+   ROOT_PRT_ENTRY(0, 168),   // INTA
+   ROOT_PRT_ENTRY(1, 169),   // INTB
+   ROOT_PRT_ENTRY(2, 170),   // INTC
+   ROOT_PRT_ENTRY(3, 171),   // INTD
})
 // Root complex resources
Method (_CRS, 0, Serialized) {
@@ -98,6 +98,20 @@ DefinitionBlock("SsdtPci.aml", "SSDT", 1, "ARMLTD", 
"ARM-JUNO", EFI_ACPI_ARM_OEM
0x, 
// Translate
0x1 
// Length
)
+
+   DWordIo ( // IO window
+   ResourceProducer,
+   MinFixed,
+   MaxFixed,
+   PosDecode,
+   EntireRange,
+   0x, 
// Granularity
+   0x5f80, 
// Min Base Address
+   0x5fff, 
// Max Base Address
+   0x5f80, 
// Translate
+   0x0080  
// Length
+   )
+
}) // Name(RBUF)

Return (RBUF)
-- 
2.5.0


___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] EDK II & GPL - Re: OVMF BoF @ KVM Forum 2015

[Paolo Bonzini]

Well, FatPkg is only superficially permissive and not even open source, so 
there is a precedent. (A precedent that, I might add, happens to violate 
SourceForge's the off service).

When we import edk2 into Fedora we just remove FatBinPkg. We would think twice 
before contributing to it, but do not make any kind of fuss about it.

GPL is just the same. For example, it would be possible to have an 
automatically-updated git repo that omits the GPL directory; and development 
would then be easier for people whose legal departments tend not to influence 
the engineers' productivity.

In fact:

1) it is not like, among non-Intel contributors, proprietary software companies 
have the lion's share of edk2 commits, and they probably use Tiano releases. 
Intel could strip any GPL pieces as part of the Tiano release process.

2) the GPL is working just fine for Linux, which is not that different from 
UEFI. So, picture me skeptical. If anything, what Linux can teach edk2 is that 
a closed prices and balkanized trees are a direct cause of the abysmal security 
of those implementations.

Paolo

-Original Message-
From: El-Haj-Mahmoud, Samer [samer.el-haj-mahm...@hpe.com]
Received: mercoledì, 09 set 2015, 21:12
To: Jordan Justen [jordan.l.jus...@intel.com]; Andrew Fish [af...@apple.com]
CC: Lenny Szubowicz [lenn...@redhat.com]; Karen Noel [kn...@redhat.com]; Ard 
Biesheuvel [ard.biesheu...@linaro.org]; edk2-devel-01 [edk2-de...@ml01.01.org]; 
Reza Jelveh [reza.jel...@tuhh.de]; Alexander Graf [ag...@suse.de]; qemu devel 
list [qemu-de...@nongnu.org]; Hannes Reinecke [h...@suse.de]; Gabriel L. Somlo 
(GMail) [gso...@gmail.com]; Peter Jones [pjo...@redhat.com]; Peter Batard 
[p...@akeo.ie]; Gerd Hoffmann [kra...@redhat.com]; Cole Robinson 
[crobi...@redhat.com]; Paolo Bonzini [pbonz...@redhat.com]; 
xen-de...@lists.xen.org [xen-de...@lists.xen.org]; Laszlo Ersek 
[ler...@redhat.com]; Ademar de Souza Reis Jr. [ar...@redhat.com]
Subject: RE: [edk2] EDK II & GPL - Re:  OVMF BoF @ KVM Forum 2015

The recent expansions beyond BSD where all permissive licenses (BSD like) as 
far as I can tell.

I agree with Andrew, opening the door for GPL licensed code in EDK2 will have 
severe consequences for products that are built using EDK2. 



-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Jordan 
Justen
Sent: Wednesday, September 09, 2015 12:58 PM
To: Andrew Fish 
Cc: Lenny Szubowicz ; Karen Noel ; Ard 
Biesheuvel ; edk2-devel-01 
; Reza Jelveh ; Alexander Graf 
; qemu devel list ; Hannes Reinecke 
; Gabriel L. Somlo (GMail) ; Peter Jones 
; Peter Batard ; Gerd Hoffmann 
; Cole Robinson ; Paolo Bonzini 
; xen-de...@lists.xen.org; Laszlo Ersek 
; Ademar de Souza Reis Jr. 
Subject: Re: [edk2] EDK II & GPL - Re: OVMF BoF @ KVM Forum 2015

On 2015-09-09 10:04:50, Andrew Fish wrote:
> 
> > On Sep 9, 2015, at 9:17 AM, Jordan Justen  wrote:
> > 
> > So, related to this, I wonder how the community would feel about a 
> > GplDriverPkg. Would the community allow it as a new package in EDK 
> > II directly, or would a separate repo be required?
> > 
> 
> I think we would need a separate repo, like the FAT driver. That is 
> the only way to deal with the license issues.

There doesn't seem to be any guiding rules here. For example, I think some 
people are not comfortable with the FatBinPkg being in the tree due to the 
license. Why is that okay?

> > With regards to adding it directly into the EDK II tree, here are 
> > some potential concerns that I might anticipate hearing from the community:
> > 
> > * It will make it easier for contributors to choose GPL compared to 
> > a  permissive license, thereby limiting some users of the 
> > contribution
> > 
> > * GPL code will more easily be copied into the permissively licensed  
> > packages
> > 
> > * Some might refuse to look at EDK II entirely if it has a directory  
> > with GPL source code in it
> > 
> 
> Or have their rights to contribute revoked since this is a fundamental 
> change, and would require employees to get reauthorized by their legal 
> departments to contribute.

We've recently expanded beyond just allowing BSD code into the tree, and that 
appeared to be no big deal. No one brought this up as a fundamental change.

Just to be clear, are you saying Apple probably won't be able to contribute to 
EDK II if there is any GPL licensed code in the tree?
(Even if it is contained in a clearly indicated package.) I guess using 
dual-licensed BSD/GPL is okay though?
(EmbeddedPkg/Library/FdtLib)

-Jordan
___
edk2-devel mailing list

Re: [edk2] UEFI and NIST SP-147 compliance

[Blibbet]

On 09/09/2015 11:49 AM, Bill Paul wrote:
[...]
> Oh sure, no pressure.
>
> As you say, the closed source nature of most BIOSes makes complying
with these
> requirements nearly impossible for most organizations. The only
exceptions I
> can think of are big companies with connections to the IBVs (e.g. Intel,
> Microsoft) or the government/military. Something tells me none of them
will in
> any rush to talk to you though. :)

Thanks. I think the answer is: 147's "golden master" -- and any firmware
PKI trust validation with CRL/OSCP URLs (eg, UEFI Secure Boot)--- is
only achievable with full source coreboot and U-Boot, which has the
ability to locally build your firmware from full source (binary-only
libs don't count), and then use tools to update the system's firmware.
With UEFI, it is only achievable with fully-open source firmware, which
isn't an option for most most enterprises, or with most ISAs (eg, Intel
FSP blobs). ARM/AMD may be able to build with full-source firmware, maybe.

Maybe Intel will start licensing FSP sources to F500 sysadmins to aid in
defense?! :-)

I just spend an hour searching online for a single enterprise whitepaper
adopting UEFI and 147 lifecycle, and all I found was this 2013 article,
still mostly vague concepts:
http://www.sans.org/reading-room/whitepapers/basics/implementing-pc-hardware-configuration-bios-baseline-34370
If I missed anything else, please speak up.

Thanks.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel