Re: [edk2] An unkempt git guide for edk2 contributors and maintainers

2016-02-13 Thread Laszlo Ersek
On 02/12/16 02:09, Laszlo Ersek wrote:
> [...]

This guide is now available in the TianoCore wiki, at

https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers

with a few topical fixups (I applied a suggestion from Paolo, thx), and
more formatting / usability improvements. (Click the "revisions" link if
you are interested.)

In particular, all steps were extended with anchors, and all references
to steps became intra-page hyperlinks. (While the gray I have in my hair
doubled.) This should allow readers to jump to referenced steps quickly,
and also to return to the referer with the browser's Back button.

Kudos to Jordan for the initial MarkDown conversion (which required
renumbering the Maintainer workflow steps and references)!

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


Re: [edk2] [PATCH 2/2] NetworkPkg: Use Http11 definitions in HttpDxe and HttpBootDxe

2016-02-13 Thread Fu, Siyuan
Hi, Samer

I think we'd better not to rename a defined macro in the public header file. 
How about we keep the macro "HTTP_VERSION" in IndustryStandard/Http11.h, and in 
HTTP driver we could just use this macro, or 
#define HTTP_VERSION_STR   HTTP_VERSION


Best Regards
Siyuan

> -Original Message-
> From: Samer El-Haj-Mahmoud [mailto:samer.el-haj-mahm...@hpe.com]
> Sent: Thursday, February 11, 2016 8:15 AM
> To: edk2-devel@lists.01.org
> Cc: Fu, Siyuan ; Samer El-Haj-Mahmoud  haj-mahm...@hpe.com>; Samer El-Haj-Mahmoud 
> Subject: [PATCH 2/2] NetworkPkg: Use Http11 definitions in HttpDxe and
> HttpBootDxe
> 
> Change HttpDxe and HttpBootDxe to use the standard definitions from
> Http11.h instead of private duplicate definitions.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Samer El-Haj-Mahmoud 
> ---
>  NetworkPkg/HttpBootDxe/HttpBootClient.c |  7 ---
>  NetworkPkg/HttpBootDxe/HttpBootClient.h |  4 +---
>  NetworkPkg/HttpBootDxe/HttpBootDxe.h|  2 ++
>  NetworkPkg/HttpDxe/HttpDriver.h |  3 ++-
>  NetworkPkg/HttpDxe/HttpImpl.h   | 11 +--
>  NetworkPkg/HttpDxe/HttpProto.c  | 16 ++--
>  6 files changed, 20 insertions(+), 23 deletions(-)
> 
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.c
> b/NetworkPkg/HttpBootDxe/HttpBootClient.c
> index dd835c4..2ccac8c 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootClient.c
> +++ b/NetworkPkg/HttpBootDxe/HttpBootClient.c
> @@ -2,6 +2,7 @@
>Implementation of the boot file download function.
> 
>  Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
> +(C) Copyright 2016 Hewlett Packard Enterprise Development LP
>  This program and the accompanying materials are licensed and made
> available under
>  the terms and conditions of the BSD License that accompanies this
> distribution.
>  The full text of the license may be found at
> @@ -807,7 +808,7 @@ HttpBootGetBootFile (
>}
>Status = HttpBootSetHeader (
>   HttpIoHeader,
> - HTTP_FIELD_NAME_HOST,
> + HTTP_HEADER_HOST,
>   HostName
>   );
>FreePool (HostName);
> @@ -820,7 +821,7 @@ HttpBootGetBootFile (
>//
>Status = HttpBootSetHeader (
>   HttpIoHeader,
> - HTTP_FIELD_NAME_ACCEPT,
> + HTTP_HEADER_ACCEPT,
>   "*/*"
>   );
>if (EFI_ERROR (Status)) {
> @@ -832,7 +833,7 @@ HttpBootGetBootFile (
>//
>Status = HttpBootSetHeader (
>   HttpIoHeader,
> - HTTP_FIELD_NAME_USER_AGENT,
> + HTTP_HEADER_USER_AGENT,
>   HTTP_USER_AGENT_EFI_HTTP_BOOT
>   );
>if (EFI_ERROR (Status)) {
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.h
> b/NetworkPkg/HttpBootDxe/HttpBootClient.h
> index e618316..b929fa7 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootClient.h
> +++ b/NetworkPkg/HttpBootDxe/HttpBootClient.h
> @@ -2,6 +2,7 @@
>Declaration of the boot file download function.
> 
>  Copyright (c) 2015, Intel Corporation. All rights reserved.
> +(C) Copyright 2016 Hewlett Packard Enterprise Development LP
>  This program and the accompanying materials are licensed and made
> available under
>  the terms and conditions of the BSD License that accompanies this
> distribution.
>  The full text of the license may be found at
> @@ -18,9 +19,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
>  #define HTTP_BOOT_REQUEST_TIMEOUT5000  // 5 seconds in uints
> of millisecond.
>  #define HTTP_BOOT_BLOCK_SIZE 1500
> 
> -#define HTTP_FIELD_NAME_USER_AGENT   "User-Agent"
> -#define HTTP_FIELD_NAME_HOST "Host"
> -#define HTTP_FIELD_NAME_ACCEPT   "Accept"
> 
> 
>  #define HTTP_USER_AGENT_EFI_HTTP_BOOT"UefiHttpBoot/1.0"
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootDxe.h
> b/NetworkPkg/HttpBootDxe/HttpBootDxe.h
> index 452c8f4..fb6ab1a 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootDxe.h
> +++ b/NetworkPkg/HttpBootDxe/HttpBootDxe.h
> @@ -2,6 +2,7 @@
>UEFI HTTP boot driver's private data structure and interfaces declaration.
> 
>  Copyright (c) 2015, Intel Corporation. All rights reserved.
> +(C) Copyright 2016 Hewlett Packard Enterprise Development LP
>  This program and the accompanying materials are licensed and made
> available under
>  the terms and conditions of the BSD License that accompanies this
> distribution.
>  The full text of the license may be found at
> @@ -16,6 +17,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
>  #define __EFI_HTTP_BOOT_DXE_H__
> 
>  #include 
> +#include 
> 
>  //
>  // Libraries
> diff --git a/NetworkPkg/HttpDxe/HttpDriver.h
> b/NetworkPkg/HttpDxe/HttpDriver.h
> index 138f56c..4de06d7 100644
> --- a/NetworkPkg/HttpDxe/HttpDriver.h
> +++ b/NetworkPkg/HttpDxe/HttpDriver.h
> @@ -2,7 +2,7 @@
>

Re: [edk2] [PATCH 2/2] NetworkPkg: Use Http11 definitions in HttpDxe and HttpBootDxe

2016-02-13 Thread Fu, Siyuan
Better to make a new patch, I don't know whether Liming has more comments about 
the patch.

Siyuan

From: El-Haj-Mahmoud, Samer [mailto:samer.el-haj-mahm...@hpe.com]
Sent: Sunday, February 14, 2016 11:47 AM
To: edk2-devel@lists.01.org; Fu, Siyuan 
Subject: RE: [PATCH 2/2] NetworkPkg: Use Http11 definitions in HttpDxe and 
HttpBootDxe

Siyuan

Either way is fine. Do you want to make the changes yourself or wait for me to 
submit a patch next week?




-Original Message-
From: Fu, Siyuan [siyuan...@intel.com]
Received: Saturday, 13 Feb 2016, 8:53PM
To: El-Haj-Mahmoud, Samer [samer.el-haj-mahm...@hpe.com]; 
edk2-devel@lists.01.org 
[edk2-devel@lists.01.org]
CC: El-Haj-Mahmoud, Samer [samer.el-haj-mahm...@hpe.com]
Subject: RE: [PATCH 2/2] NetworkPkg: Use Http11 definitions in HttpDxe and 
HttpBootDxe
Hi, Samer

I think we'd better not to rename a defined macro in the public header file. 
How about we keep the macro "HTTP_VERSION" in IndustryStandard/Http11.h, and in 
HTTP driver we could just use this macro, or
#define HTTP_VERSION_STR   HTTP_VERSION


Best Regards
Siyuan

> -Original Message-
> From: Samer El-Haj-Mahmoud [mailto:samer.el-haj-mahm...@hpe.com]
> Sent: Thursday, February 11, 2016 8:15 AM
> To: edk2-devel@lists.01.org
> Cc: Fu, Siyuan >; Samer 
> El-Haj-Mahmoud  haj-mahm...@hpe.com>; Samer El-Haj-Mahmoud 
> >
> Subject: [PATCH 2/2] NetworkPkg: Use Http11 definitions in HttpDxe and
> HttpBootDxe
>
> Change HttpDxe and HttpBootDxe to use the standard definitions from
> Http11.h instead of private duplicate definitions.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Samer El-Haj-Mahmoud >
> ---
>  NetworkPkg/HttpBootDxe/HttpBootClient.c |  7 ---
>  NetworkPkg/HttpBootDxe/HttpBootClient.h |  4 +---
>  NetworkPkg/HttpBootDxe/HttpBootDxe.h|  2 ++
>  NetworkPkg/HttpDxe/HttpDriver.h |  3 ++-
>  NetworkPkg/HttpDxe/HttpImpl.h   | 11 +--
>  NetworkPkg/HttpDxe/HttpProto.c  | 16 ++--
>  6 files changed, 20 insertions(+), 23 deletions(-)
>
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.c
> b/NetworkPkg/HttpBootDxe/HttpBootClient.c
> index dd835c4..2ccac8c 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootClient.c
> +++ b/NetworkPkg/HttpBootDxe/HttpBootClient.c
> @@ -2,6 +2,7 @@
>Implementation of the boot file download function.
>
>  Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
> +(C) Copyright 2016 Hewlett Packard Enterprise Development LP
>  This program and the accompanying materials are licensed and made
> available under
>  the terms and conditions of the BSD License that accompanies this
> distribution.
>  The full text of the license may be found at
> @@ -807,7 +808,7 @@ HttpBootGetBootFile (
>}
>Status = HttpBootSetHeader (
>   HttpIoHeader,
> - HTTP_FIELD_NAME_HOST,
> + HTTP_HEADER_HOST,
>   HostName
>   );
>FreePool (HostName);
> @@ -820,7 +821,7 @@ HttpBootGetBootFile (
>//
>Status = HttpBootSetHeader (
>   HttpIoHeader,
> - HTTP_FIELD_NAME_ACCEPT,
> + HTTP_HEADER_ACCEPT,
>   "*/*"
>   );
>if (EFI_ERROR (Status)) {
> @@ -832,7 +833,7 @@ HttpBootGetBootFile (
>//
>Status = HttpBootSetHeader (
>   HttpIoHeader,
> - HTTP_FIELD_NAME_USER_AGENT,
> + HTTP_HEADER_USER_AGENT,
>   HTTP_USER_AGENT_EFI_HTTP_BOOT
>   );
>if (EFI_ERROR (Status)) {
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.h
> b/NetworkPkg/HttpBootDxe/HttpBootClient.h
> index e618316..b929fa7 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootClient.h
> +++ b/NetworkPkg/HttpBootDxe/HttpBootClient.h
> @@ -2,6 +2,7 @@
>Declaration of the boot file download function.
>
>  Copyright (c) 2015, Intel Corporation. All rights reserved.
> +(C) Copyright 2016 Hewlett Packard Enterprise Development LP
>  This program and the accompanying materials are licensed and made
> available under
>  the terms and conditions of the BSD License that accompanies this
> distribution.
>  The full text of the license may be found at
> @@ -18,9 +19,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
>  #define HTTP_BOOT_REQUEST_TIMEOUT5000  // 5 seconds in uints
> of millisecond.
>  #define HTTP_BOOT_BLOCK_SIZE 1500
>
> -#define HTTP_FIELD_NAME_USER_AGENT   "User-Agent"
> -#define HTTP_FIELD_NAME_HOST "Host"
> -#define HTTP_FIELD_NAME_ACCEPT   "Accept"
>
>
>  #define HTTP_USER_AGENT_EFI_HTTP_BOOT"UefiHttpBoot/1.0"
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootDxe.h
> 

Re: [edk2] [PATCH 2/2] NetworkPkg: Use Http11 definitions in HttpDxe and HttpBootDxe

2016-02-13 Thread El-Haj-Mahmoud, Samer
Siyuan

Either way is fine. Do you want to make the changes yourself or wait for me to 
submit a patch next week?




-Original Message-
From: Fu, Siyuan [siyuan...@intel.com]
Received: Saturday, 13 Feb 2016, 8:53PM
To: El-Haj-Mahmoud, Samer [samer.el-haj-mahm...@hpe.com]; 
edk2-devel@lists.01.org [edk2-devel@lists.01.org]
CC: El-Haj-Mahmoud, Samer [samer.el-haj-mahm...@hpe.com]
Subject: RE: [PATCH 2/2] NetworkPkg: Use Http11 definitions in HttpDxe and 
HttpBootDxe

Hi, Samer

I think we'd better not to rename a defined macro in the public header file. 
How about we keep the macro "HTTP_VERSION" in IndustryStandard/Http11.h, and in 
HTTP driver we could just use this macro, or
#define HTTP_VERSION_STR   HTTP_VERSION


Best Regards
Siyuan

> -Original Message-
> From: Samer El-Haj-Mahmoud [mailto:samer.el-haj-mahm...@hpe.com]
> Sent: Thursday, February 11, 2016 8:15 AM
> To: edk2-devel@lists.01.org
> Cc: Fu, Siyuan ; Samer El-Haj-Mahmoud  haj-mahm...@hpe.com>; Samer El-Haj-Mahmoud 
> Subject: [PATCH 2/2] NetworkPkg: Use Http11 definitions in HttpDxe and
> HttpBootDxe
>
> Change HttpDxe and HttpBootDxe to use the standard definitions from
> Http11.h instead of private duplicate definitions.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Samer El-Haj-Mahmoud 
> ---
>  NetworkPkg/HttpBootDxe/HttpBootClient.c |  7 ---
>  NetworkPkg/HttpBootDxe/HttpBootClient.h |  4 +---
>  NetworkPkg/HttpBootDxe/HttpBootDxe.h|  2 ++
>  NetworkPkg/HttpDxe/HttpDriver.h |  3 ++-
>  NetworkPkg/HttpDxe/HttpImpl.h   | 11 +--
>  NetworkPkg/HttpDxe/HttpProto.c  | 16 ++--
>  6 files changed, 20 insertions(+), 23 deletions(-)
>
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.c
> b/NetworkPkg/HttpBootDxe/HttpBootClient.c
> index dd835c4..2ccac8c 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootClient.c
> +++ b/NetworkPkg/HttpBootDxe/HttpBootClient.c
> @@ -2,6 +2,7 @@
>Implementation of the boot file download function.
>
>  Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
> +(C) Copyright 2016 Hewlett Packard Enterprise Development LP
>  This program and the accompanying materials are licensed and made
> available under
>  the terms and conditions of the BSD License that accompanies this
> distribution.
>  The full text of the license may be found at
> @@ -807,7 +808,7 @@ HttpBootGetBootFile (
>}
>Status = HttpBootSetHeader (
>   HttpIoHeader,
> - HTTP_FIELD_NAME_HOST,
> + HTTP_HEADER_HOST,
>   HostName
>   );
>FreePool (HostName);
> @@ -820,7 +821,7 @@ HttpBootGetBootFile (
>//
>Status = HttpBootSetHeader (
>   HttpIoHeader,
> - HTTP_FIELD_NAME_ACCEPT,
> + HTTP_HEADER_ACCEPT,
>   "*/*"
>   );
>if (EFI_ERROR (Status)) {
> @@ -832,7 +833,7 @@ HttpBootGetBootFile (
>//
>Status = HttpBootSetHeader (
>   HttpIoHeader,
> - HTTP_FIELD_NAME_USER_AGENT,
> + HTTP_HEADER_USER_AGENT,
>   HTTP_USER_AGENT_EFI_HTTP_BOOT
>   );
>if (EFI_ERROR (Status)) {
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.h
> b/NetworkPkg/HttpBootDxe/HttpBootClient.h
> index e618316..b929fa7 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootClient.h
> +++ b/NetworkPkg/HttpBootDxe/HttpBootClient.h
> @@ -2,6 +2,7 @@
>Declaration of the boot file download function.
>
>  Copyright (c) 2015, Intel Corporation. All rights reserved.
> +(C) Copyright 2016 Hewlett Packard Enterprise Development LP
>  This program and the accompanying materials are licensed and made
> available under
>  the terms and conditions of the BSD License that accompanies this
> distribution.
>  The full text of the license may be found at
> @@ -18,9 +19,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
>  #define HTTP_BOOT_REQUEST_TIMEOUT5000  // 5 seconds in uints
> of millisecond.
>  #define HTTP_BOOT_BLOCK_SIZE 1500
>
> -#define HTTP_FIELD_NAME_USER_AGENT   "User-Agent"
> -#define HTTP_FIELD_NAME_HOST "Host"
> -#define HTTP_FIELD_NAME_ACCEPT   "Accept"
>
>
>  #define HTTP_USER_AGENT_EFI_HTTP_BOOT"UefiHttpBoot/1.0"
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootDxe.h
> b/NetworkPkg/HttpBootDxe/HttpBootDxe.h
> index 452c8f4..fb6ab1a 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootDxe.h
> +++ b/NetworkPkg/HttpBootDxe/HttpBootDxe.h
> @@ -2,6 +2,7 @@
>UEFI HTTP boot driver's private data structure and interfaces declaration.
>
>  Copyright (c) 2015, Intel Corporation. All rights reserved.
> +(C) Copyright 2016 Hewlett Packard Enterprise Development LP
>  This program and the accompanying materials are licensed and made
> available under
>  the terms and conditions of the BSD License that accompanies this
> 

Re: [edk2] [PATCH] NetworkPkg: Reword PXE download message

2016-02-13 Thread Fu, Siyuan
Reviewed-by: Siyuan Fu 


> -Original Message-
> From: Samer El-Haj-Mahmoud [mailto:samer.el-haj-mahm...@hpe.com]
> Sent: Friday, February 12, 2016 8:39 AM
> To: edk2-devel@lists.01.org
> Cc: Fu, Siyuan ; Samer El-Haj-Mahmoud  haj-mahm...@hpe.com>; Samer El-Haj-Mahmoud 
> Subject: [PATCH] NetworkPkg: Reword PXE download message
> 
> Fix a minor grammatical error in the PXE boot message.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Samer El-Haj-Mahmoud 
> ---
>  NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
> b/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
> index 2531153..5be82dc 100644
> --- a/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
> +++ b/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
> @@ -2,6 +2,7 @@
>Boot functions implementation for UefiPxeBc Driver.
> 
>Copyright (c) 2009 - 2014, Intel Corporation. All rights reserved.
> +  (C) Copyright 2016 Hewlett Packard Enterprise Development LP
> 
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the BSD
> License
> @@ -1228,7 +1229,7 @@ ON_EXIT:
>PxeBcUninstallCallback(Private, NewMakeCallback);
> 
>if (Status == EFI_SUCCESS) {
> -AsciiPrint ("\n  Succeed to download NBP file.\n");
> +AsciiPrint ("\n  NBP file downloaded successfully.\n");
>  return EFI_SUCCESS;
>} else if (Status == EFI_BUFFER_TOO_SMALL && Buffer != NULL) {
>  AsciiPrint ("\n  PXE-E05: Buffer size is smaller than the requested 
> file.\n");
> --
> 2.6.3.windows.1

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


Re: [edk2] [PATCH] NetworkPkg: better sanity check on Ipv6 prefix length

2016-02-13 Thread Fu, Siyuan
Reviewed-by: Fu Siyuan 

> -Original Message-
> From: Samer El-Haj-Mahmoud [mailto:samer.el-haj-mahm...@hpe.com]
> Sent: Friday, February 12, 2016 7:58 AM
> To: edk2-devel@lists.01.org
> Cc: Fu, Siyuan ; Samer El-Haj-Mahmoud  haj-mahm...@hpe.com>; Samer El-Haj-Mahmoud 
> Subject: [PATCH] NetworkPkg: better sanity check on Ipv6 prefix length
> 
> Fix a possible buffer overrun issue that could occur if PrefixLength >
> 128 . Changed == 128 to >= 128. Also remove check for Byte < 16, which
> is no longer possible because of the first change.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Samer El-Haj-Mahmoud 
> ---
>  NetworkPkg/Ip6Dxe/Ip6Icmp.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/NetworkPkg/Ip6Dxe/Ip6Icmp.c b/NetworkPkg/Ip6Dxe/Ip6Icmp.c
> index db40b81..f6a9bb4 100644
> --- a/NetworkPkg/Ip6Dxe/Ip6Icmp.c
> +++ b/NetworkPkg/Ip6Dxe/Ip6Icmp.c
> @@ -2,7 +2,8 @@
>The ICMPv6 handle routines to process the ICMPv6 control messages.
> 
>Copyright (c) 2009 - 2010, Intel Corporation. All rights reserved.
> -
> +  (C) Copyright 2016 Hewlett Packard Enterprise Development LP
> +
>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
> @@ -479,7 +480,7 @@ Ip6GetPrefix (
>  return ;
>}
> 
> -  if (PrefixLength == IP6_PREFIX_NUM - 1) {
> +  if (PrefixLength >= IP6_PREFIX_NUM - 1) {
>  return ;
>}
> 
> @@ -487,7 +488,7 @@ Ip6GetPrefix (
>Bit   = (UINT8) (PrefixLength % 8);
>Value = Prefix->Addr[Byte];
> 
> -  if ((Byte > 0) && (Byte < 16)) {
> +  if (Byte > 0) {
>  ZeroMem (Prefix->Addr + Byte, 16 - Byte);
>}
> 
> --
> 2.6.3.windows.1

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


Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support.

2016-02-13 Thread Yao, Jiewen
HI
Thanks to discuss this properties table issue.
The purpose of this patch is to *add* UEFI2.6 memory attributes table. This 
patch does not handle any UEFI2.5 properties table.
If we want to change EDKII code for properties table, I suggest we separate it 
from this patch.

Is there any comment for adding UEFI2.6 memory attributes table?


For UEFI2.5 properties table, let me summarize. Currently, we have several 
options:
0) Keep it as is. Current setting is PcdPropertiesTableEnable TRUE in 
MdeModulePkg.DEC.
1) Update PcdPropertiesTableEnable to FALSE in XXXPlatform.DSC file. (Platform 
choice. It can be static PCD or dynamic PCD.)
2) Update PcdPropertiesTableEnable to FALSE in MdeModulePkg.DEC file. (Disable 
Default. BIOS will not publish this table by default. If platform wants this 
table, it can set PCD to true.)
3) Remove PcdPropertiesTableEnable, and remove PropertiesTable support code in 
DxeCore. (PropertiesTable support is removed.)

Personally, I do not suggest we choose 3), because this table is still defined 
in UEFI specification. We can remove it after UEFI spec removes it later.
I do not have strong opinion for other options.

Thank you
Yao Jiewen

-Original Message-
From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] 
Sent: Monday, February 01, 2016 2:28 PM
To: Laszlo Ersek
Cc: Yao, Jiewen; edk2-de...@ml01.01.org; Gao, Liming
Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support.

On 31 January 2016 at 21:36, Laszlo Ersek  wrote:
> On 01/30/16 11:25, Ard Biesheuvel wrote:
>> On 30 January 2016 at 04:17, Yao, Jiewen  wrote:
>>> Thanks for the clarification. I think you are right.
>>>
>>> I said "This table is used to retire old PropertiesTable", because UEFI2.6 
>>> does not recommend using PropertiesTable to report RT information.
>>> The future BIOS/OS should always produce/consume MemoryAttributesTable as 
>>> better solution.
>>>
>>
>> I strongly feel we should remove the original PropertiesTable. It 
>> breaks backward compatibility, and was replaced by the 
>> MemoryAttributesTable for a good reason. The OS side will not be 
>> implemented in Linux (i.e., it will ignore the RO and XP attributes 
>> in the standard UEFI memory map), and the only reason the 
>> PropertiesTable was not removed entirely from the specification is 
>> because of business interests of one of the participating OS vendors.
>>
>> I understand that we need to keep the underlying machinery to produce 
>> either table. But we should remove the functionality that results in 
>> each PE/COFF image to be split into several regions marked RO or XP 
>> in the ordinary UEFI memory map.
>
> We can mitigate this by setting PcdPropertiesTableEnable to FALSE in 
> "ArmVirt.dsc.inc" as well.
>

No, we should remove it.

> (Hmmm, I forget why SVN r18140 and r18566 don't already result in a 
> problematic memmap for Linux... Probably due to your kernel commit
> 0ce3cc008ec0.)
>

It will result in a problematic memmap for older Linux kernels but also for 
less recent versions of Windows 8, for instance. And the only OS version that 
actually consumes the properties table is Windows 10, which will we updated in 
the future to use the MemoryAttributesTable instead.


>>> -Original Message-
>>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>>> Sent: Saturday, January 30, 2016 10:24 AM
>>> To: Yao, Jiewen
>>> Cc: edk2-de...@ml01.01.org; Gao, Liming
>>> Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support.
>>>
>>> On 01/30/16 01:31, Yao, Jiewen wrote:
 Comment below:

 -Original Message-
 From: Laszlo Ersek [mailto:ler...@redhat.com]
 Sent: Saturday, January 30, 2016 1:13 AM
 To: Yao, Jiewen
 Cc: edk2-de...@ml01.01.org; Gao, Liming
 Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support.

 Hello Jiewen,

 On 01/29/16 13:12, jiewen yao wrote:
> This series patches add UEFI2.6 MemoryAttributesTable support.
> This table is used to retire old PropertiesTable.
> This is standalone table published by DxeCore, so there is no 
> compatibility issue.
>
> The patch is validated with or without properties table.

 Do you mean, tested with PcdPropertiesTableEnable set to FALSE vs. TRUE?
 [Jiewen] Yes, you are right. No matter PcdPropertiestableEnable set to 
 FALSE or TRUE, MemoryAttributesTable is always published.
 That is one compatibility we want to maintain.


 I skimmed the commit messages, and it looks like the properties table is 
 preserved, but now it serves only as foundation for the memory attributes 
 table.
 [Jiewen] Yes, the DxeCore global variable - properties table is always 
 preserved. And the system UEFI configuration table for properties table is 
 controlled by PcdPropertiestableEnable.
 No matter properties table or memory attributes table, we always need a 
 

Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support.

2016-02-13 Thread Laszlo Ersek
On 02/14/16 06:44, Yao, Jiewen wrote:
> HI
> Thanks to discuss this properties table issue.
> The purpose of this patch is to *add* UEFI2.6 memory attributes table. This 
> patch does not handle any UEFI2.5 properties table.
> If we want to change EDKII code for properties table, I suggest we separate 
> it from this patch.
> 
> Is there any comment for adding UEFI2.6 memory attributes table?

Not from my side, thanks.

> For UEFI2.5 properties table, let me summarize. Currently, we have several 
> options:
> 0) Keep it as is. Current setting is PcdPropertiesTableEnable TRUE in 
> MdeModulePkg.DEC.
> 1) Update PcdPropertiesTableEnable to FALSE in XXXPlatform.DSC file. 
> (Platform choice. It can be static PCD or dynamic PCD.)
> 2) Update PcdPropertiesTableEnable to FALSE in MdeModulePkg.DEC file. 
> (Disable Default. BIOS will not publish this table by default. If platform 
> wants this table, it can set PCD to true.)
> 3) Remove PcdPropertiesTableEnable, and remove PropertiesTable support code 
> in DxeCore. (PropertiesTable support is removed.)
> 
> Personally, I do not suggest we choose 3), because this table is still 
> defined in UEFI specification. We can remove it after UEFI spec removes it 
> later.
> I do not have strong opinion for other options.

I agree the implementation should follow the spec -- at least offer the
feature as long as the spec defines it. My vote would be (2).

Thanks
Laszlo

> 
> Thank you
> Yao Jiewen
> 
> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] 
> Sent: Monday, February 01, 2016 2:28 PM
> To: Laszlo Ersek
> Cc: Yao, Jiewen; edk2-de...@ml01.01.org; Gao, Liming
> Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support.
> 
> On 31 January 2016 at 21:36, Laszlo Ersek  wrote:
>> On 01/30/16 11:25, Ard Biesheuvel wrote:
>>> On 30 January 2016 at 04:17, Yao, Jiewen  wrote:
 Thanks for the clarification. I think you are right.

 I said "This table is used to retire old PropertiesTable", because UEFI2.6 
 does not recommend using PropertiesTable to report RT information.
 The future BIOS/OS should always produce/consume MemoryAttributesTable as 
 better solution.

>>>
>>> I strongly feel we should remove the original PropertiesTable. It 
>>> breaks backward compatibility, and was replaced by the 
>>> MemoryAttributesTable for a good reason. The OS side will not be 
>>> implemented in Linux (i.e., it will ignore the RO and XP attributes 
>>> in the standard UEFI memory map), and the only reason the 
>>> PropertiesTable was not removed entirely from the specification is 
>>> because of business interests of one of the participating OS vendors.
>>>
>>> I understand that we need to keep the underlying machinery to produce 
>>> either table. But we should remove the functionality that results in 
>>> each PE/COFF image to be split into several regions marked RO or XP 
>>> in the ordinary UEFI memory map.
>>
>> We can mitigate this by setting PcdPropertiesTableEnable to FALSE in 
>> "ArmVirt.dsc.inc" as well.
>>
> 
> No, we should remove it.
> 
>> (Hmmm, I forget why SVN r18140 and r18566 don't already result in a 
>> problematic memmap for Linux... Probably due to your kernel commit
>> 0ce3cc008ec0.)
>>
> 
> It will result in a problematic memmap for older Linux kernels but also for 
> less recent versions of Windows 8, for instance. And the only OS version that 
> actually consumes the properties table is Windows 10, which will we updated 
> in the future to use the MemoryAttributesTable instead.
> 
> 
 -Original Message-
 From: Laszlo Ersek [mailto:ler...@redhat.com]
 Sent: Saturday, January 30, 2016 10:24 AM
 To: Yao, Jiewen
 Cc: edk2-de...@ml01.01.org; Gao, Liming
 Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support.

 On 01/30/16 01:31, Yao, Jiewen wrote:
> Comment below:
>
> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Saturday, January 30, 2016 1:13 AM
> To: Yao, Jiewen
> Cc: edk2-de...@ml01.01.org; Gao, Liming
> Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support.
>
> Hello Jiewen,
>
> On 01/29/16 13:12, jiewen yao wrote:
>> This series patches add UEFI2.6 MemoryAttributesTable support.
>> This table is used to retire old PropertiesTable.
>> This is standalone table published by DxeCore, so there is no 
>> compatibility issue.
>>
>> The patch is validated with or without properties table.
>
> Do you mean, tested with PcdPropertiesTableEnable set to FALSE vs. TRUE?
> [Jiewen] Yes, you are right. No matter PcdPropertiestableEnable set to 
> FALSE or TRUE, MemoryAttributesTable is always published.
> That is one compatibility we want to maintain.
>
>
> I skimmed the commit messages, and it looks like the properties table is 
>