Re: [edk2] [Patch] NetworkPkg: Add RAM disk boot support to HTTP Boot driver.

2016-04-06 Thread Ye, Ting 
Siyuan,

Some minor comments to the patch:

1. Add perform parameter check  for ImageType in HttpBootGetFileFromCache().
2. Update function description of HttpBootStart/HttpBootLoadFile for 
EFI_INVALID_PARAMETER since we now add further check.
3. Add EFI_INVALID_PARAMETER to function description of HttpBootCheckImageType
4. In HttpBootCheckImageType, ImageType is checked but others are skipped. Not 
necessary?
5. Is EFI_NOT_FOUND irrelevant to HttpBootRegisterRamDisk? I see 
EFI_UNSUPPORTED returned below. Please double check.

Others are good to me.

Reviewed-by: Ye Ting 

Thanks,
Ting


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Fu Siyuan
Sent: Thursday, March 31, 2016 11:37 AM
To: edk2-devel@lists.01.org
Cc: Ye, Ting ; Wu, Jiaxin 
Subject: [edk2] [Patch] NetworkPkg: Add RAM disk boot support to HTTP Boot 
driver.

This patch updates the HTTP Boot driver to support the download and boot a RAM 
disk image from HTTP server.
The HTTP RAM disk boot is described in section 23.7 "HTTP Boot" in UEFI 2.6. 
HTTP server could provide either an UEFI image or a RAM disk image for the HTTP 
boot client to use. The RAM disk image must contain a UEFI compliant file 
system in it.
HTTP boot driver will identify the image type either by the "Content-Type"
entity header filed or by the file name extension as below:
  "application/efi" or *.efi -> EFI Image
  *.iso  -> CD/DVD Image
  *.img  -> Virtual Disk Image

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
Cc: Ye Ting 
Cc: Wu Jiaxin 
Cc: El-Haj-Mahmoud Samer 
---
 NetworkPkg/HttpBootDxe/HttpBootClient.c  |  35 ++--
 NetworkPkg/HttpBootDxe/HttpBootClient.h  |   5 +-
 NetworkPkg/HttpBootDxe/HttpBootDxe.h |  19 +
 NetworkPkg/HttpBootDxe/HttpBootDxe.inf   |   3 +
 NetworkPkg/HttpBootDxe/HttpBootImpl.c| 130 ++---
 NetworkPkg/HttpBootDxe/HttpBootSupport.c | 135 +++ 
 NetworkPkg/HttpBootDxe/HttpBootSupport.h |  44 ++
 7 files changed, 319 insertions(+), 52 deletions(-)

diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.c 
b/NetworkPkg/HttpBootDxe/HttpBootClient.c
index 0c47293..0e30b22 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootClient.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootClient.c
@@ -570,6 +570,7 @@ HttpBootFreeCacheList (
   @param[out] Buffer  The memory buffer to transfer the file 
to. IF Buffer is NULL,
   then the size of the requested file is 
returned in
   BufferSize.
+  @param[out] ImageType   The image type of the downloaded file.
 
   @retval EFI_SUCCESS  Successfully created.
   @retval Others   Failed to create HttpIo.
@@ -580,7 +581,8 @@ HttpBootGetFileFromCache (
   IN HTTP_BOOT_PRIVATE_DATA   *Private,
   IN CHAR16   *Uri,
   IN OUT UINTN*BufferSize,
- OUT UINT8*Buffer
+ OUT UINT8*Buffer,
+ OUT HTTP_BOOT_IMAGE_TYPE *ImageType
   )
 {
   LIST_ENTRY  *Entry;
@@ -603,7 +605,12 @@ HttpBootGetFileFromCache (
 (StrCmp (Uri, Cache->RequestData->Url) == 0)) 
 {
   //
-  // Hit cache, check buffer size.
+  // Hit in cache, record image type.
+  //
+  *ImageType  = Cache->ImageType;
+
+  //
+  // Check buffer size.
   //
   if (*BufferSize < Cache->EntityLength) {
 *BufferSize = Cache->EntityLength; @@ -712,6 +719,7 @@ 
HttpBootGetBootFileCallback (
   @param[out]  Buffer  The memory buffer to transfer the file to. 
IF Buffer is NULL,
then the size of the requested file is 
returned in
BufferSize.
+  @param[out]  ImageType   The image type of the downloaded file.
 
   @retval EFI_SUCCESS  The file was loaded.
   @retval EFI_INVALID_PARAMETERBufferSize is NULL or Buffer Size is not 
NULL but Buffer is NULL.
@@ -727,7 +735,8 @@ HttpBootGetBootFile (
   IN HTTP_BOOT_PRIVATE_DATA   *Private,
   IN BOOLEAN  HeaderOnly,
   IN OUT UINTN*BufferSize,
- OUT UINT8*Buffer
+ OUT UINT8*Buffer,
+ OUT HTTP_BOOT_IMAGE_TYPE *ImageType
   )
 {
   EFI_STATUS Status;
@@ -750,7 +759,7 @@ HttpBootGetBootFile (
   ASSERT (Private != NULL);
   ASSERT (Private->HttpCreated);
 
-  if (BufferSize == NULL) {
+  if (BufferSize == NULL || ImageType == NULL) {
 return EFI_INVALID_PARAMETER;
   }
 
@@ -767,7 +776,7 @@ HttpBootGetBootFile (
   }
   AsciiStrToUnicodeStr (Private->BootFileUri, Url);
   if (!HeaderOnly) {
-Status =

Re: [edk2] [Patch] NetworkPkg: Add RAM disk boot support to HTTP Boot driver.

2016-04-01 Thread El-Haj-Mahmoud, Samer 
The application/efi type is "temporarily approved". It is still waiting for 
final approval from IANA internal technical review. I do not think there are 
any issues, but it is just a matter of time.

I am fine keeping the definition private in HttpBootDxe until IANA approves the 
type and publishes it in the main official list. I can submit a patch to move 
it to Http11.h once that happens.

The rest of the patch looks ok to me.

Reviewed-by: Samer El-Haj-Mahmoud <el...@hpe.com>



-Original Message-
From: Fu, Siyuan [mailto:siyuan...@intel.com] 
Sent: Thursday, March 31, 2016 7:46 PM
To: El-Haj-Mahmoud, Samer <samer.el-haj-mahm...@hpe.com>; 
edk2-devel@lists.01.org
Cc: Ye, Ting <ting...@intel.com>; Wu, Jiaxin <jiaxin...@intel.com>
Subject: RE: [edk2] [Patch] NetworkPkg: Add RAM disk boot support to HTTP Boot 
driver.

Hi, Samer

Does the "Provisional Standard Media Type Registry" means the type has been 
finalized by IANA? I see the web has below declaration, says its " for 
development and test purposes only". That's why I temporarily put it in the 
HTTP boot driver.

Note
This registry, unlike some other provisional IANA registries, is only for 
temporary use. Entries in this registry are either finalized and moved to the 
main media types registry, or are abandoned and deleted. Entries in this 
registry are suitable for use for use for development and test purposes only.

Best Regards
Siyuan


> -Original Message-
> From: El-Haj-Mahmoud, Samer [mailto:samer.el-haj-mahm...@hpe.com]
> Sent: Friday, April 1, 2016 12:26 AM
> To: Fu, Siyuan <siyuan...@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting...@intel.com>; Wu, Jiaxin <jiaxin...@intel.com>; 
> El-Haj- Mahmoud, Samer <samer.el-haj-mahm...@hpe.com>
> Subject: RE: [edk2] [Patch] NetworkPkg: Add RAM disk boot support to 
> HTTP Boot driver.
> 
> Since this is industry standard, it should be moved to 
> MdePkg/IndustryStandard/Http11.h, next to the similarly defined 
> content types, like HTTP_CONTENT_TYPE_APP_JSON, 
> HTTP_CONTENT_TYPE_APP_OCTET_STREAM,
> HTTP_CONTENT_TYPE_IMAGE_JPEG, etc...
> 
> //
> +// Provisional Standard Media Types defined in // 
> +http://www.iana.org/assignments/provisional-standard-media-types/prov
> +is
> +ional-standard-media-types.xhtml
> +//
> +#define HTTP_CONTENT_TYPE_APP_EFI   "application/efi"
> 
> 
> Thanks,
> --Samer
> 
> 
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Fu Siyuan
> Sent: Wednesday, March 30, 2016 10:37 PM
> To: edk2-devel@lists.01.org
> Cc: Ye Ting <ting...@intel.com>; Wu Jiaxin <jiaxin...@intel.com>
> Subject: [edk2] [Patch] NetworkPkg: Add RAM disk boot support to HTTP 
> Boot driver.
> 
> This patch updates the HTTP Boot driver to support the download and 
> boot a RAM disk image from HTTP server.
> The HTTP RAM disk boot is described in section 23.7 "HTTP Boot" in UEFI 2.6.
> HTTP server could provide either an UEFI image or a RAM disk image for 
> the HTTP boot client to use. The RAM disk image must contain a UEFI 
> compliant file system in it.
> HTTP boot driver will identify the image type either by the "Content-Type"
> entity header filed or by the file name extension as below:
>   "application/efi" or *.efi -> EFI Image
>   *.iso  -> CD/DVD Image
>   *.img  -> Virtual Disk Image
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Fu Siyuan <siyuan...@intel.com>
> Cc: Ye Ting <ting...@intel.com>
> Cc: Wu Jiaxin <jiaxin...@intel.com>
> Cc: El-Haj-Mahmoud Samer <samer.el-haj-mahm...@hpe.com>
> ---
>  NetworkPkg/HttpBootDxe/HttpBootClient.c  |  35 ++--
>  NetworkPkg/HttpBootDxe/HttpBootClient.h  |   5 +-
>  NetworkPkg/HttpBootDxe/HttpBootDxe.h |  19 +
>  NetworkPkg/HttpBootDxe/HttpBootDxe.inf   |   3 +
>  NetworkPkg/HttpBootDxe/HttpBootImpl.c| 130 ++---
> 
>  NetworkPkg/HttpBootDxe/HttpBootSupport.c | 135
> +++
> NetworkPkg/HttpBootDxe/HttpBootSupport.h |  44 ++
>  7 files changed, 319 insertions(+), 52 deletions(-)
> 
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.c
> b/NetworkPkg/HttpBootDxe/HttpBootClient.c
> index 0c47293..0e30b22 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootClient.c
> +++ b/NetworkPkg/HttpBootDxe/HttpBootClient.c
> @@ -570,6 +570,7 @@ HttpBootFreeCacheList (
>@param[out] Buffer  The memory buffer to transfer the file 
> to. IF
> Buffer is NULL,
>then the size of the requ

Re: [edk2] [Patch] NetworkPkg: Add RAM disk boot support to HTTP Boot driver.

2016-03-31 Thread Fu, Siyuan 
Hi, Samer

Does the "Provisional Standard Media Type Registry" means the type has been 
finalized by IANA? I see the web has below declaration, says its " for 
development and test purposes only". That's why I temporarily put it in the 
HTTP boot driver.

Note
This registry, unlike some other provisional IANA registries, is
only for temporary use. Entries in this registry are either
finalized and moved to the main media types registry, or are
abandoned and deleted. Entries in this registry are suitable for
use for use for development and test purposes only.

Best Regards
Siyuan


> -Original Message-
> From: El-Haj-Mahmoud, Samer [mailto:samer.el-haj-mahm...@hpe.com]
> Sent: Friday, April 1, 2016 12:26 AM
> To: Fu, Siyuan <siyuan...@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting...@intel.com>; Wu, Jiaxin <jiaxin...@intel.com>; El-Haj-
> Mahmoud, Samer <samer.el-haj-mahm...@hpe.com>
> Subject: RE: [edk2] [Patch] NetworkPkg: Add RAM disk boot support to HTTP
> Boot driver.
> 
> Since this is industry standard, it should be moved to
> MdePkg/IndustryStandard/Http11.h, next to the similarly defined content
> types, like HTTP_CONTENT_TYPE_APP_JSON,
> HTTP_CONTENT_TYPE_APP_OCTET_STREAM,
> HTTP_CONTENT_TYPE_IMAGE_JPEG, etc...
> 
> //
> +// Provisional Standard Media Types defined in //
> +http://www.iana.org/assignments/provisional-standard-media-types/provis
> +ional-standard-media-types.xhtml
> +//
> +#define HTTP_CONTENT_TYPE_APP_EFI   "application/efi"
> 
> 
> Thanks,
> --Samer
> 
> 
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Fu
> Siyuan
> Sent: Wednesday, March 30, 2016 10:37 PM
> To: edk2-devel@lists.01.org
> Cc: Ye Ting <ting...@intel.com>; Wu Jiaxin <jiaxin...@intel.com>
> Subject: [edk2] [Patch] NetworkPkg: Add RAM disk boot support to HTTP Boot
> driver.
> 
> This patch updates the HTTP Boot driver to support the download and boot a
> RAM disk image from HTTP server.
> The HTTP RAM disk boot is described in section 23.7 "HTTP Boot" in UEFI 2.6.
> HTTP server could provide either an UEFI image or a RAM disk image for the
> HTTP boot client to use. The RAM disk image must contain a UEFI compliant
> file system in it.
> HTTP boot driver will identify the image type either by the "Content-Type"
> entity header filed or by the file name extension as below:
>   "application/efi" or *.efi -> EFI Image
>   *.iso  -> CD/DVD Image
>   *.img  -> Virtual Disk Image
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Fu Siyuan <siyuan...@intel.com>
> Cc: Ye Ting <ting...@intel.com>
> Cc: Wu Jiaxin <jiaxin...@intel.com>
> Cc: El-Haj-Mahmoud Samer <samer.el-haj-mahm...@hpe.com>
> ---
>  NetworkPkg/HttpBootDxe/HttpBootClient.c  |  35 ++--
>  NetworkPkg/HttpBootDxe/HttpBootClient.h  |   5 +-
>  NetworkPkg/HttpBootDxe/HttpBootDxe.h |  19 +
>  NetworkPkg/HttpBootDxe/HttpBootDxe.inf   |   3 +
>  NetworkPkg/HttpBootDxe/HttpBootImpl.c| 130 ++---
> 
>  NetworkPkg/HttpBootDxe/HttpBootSupport.c | 135
> +++
> NetworkPkg/HttpBootDxe/HttpBootSupport.h |  44 ++
>  7 files changed, 319 insertions(+), 52 deletions(-)
> 
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.c
> b/NetworkPkg/HttpBootDxe/HttpBootClient.c
> index 0c47293..0e30b22 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootClient.c
> +++ b/NetworkPkg/HttpBootDxe/HttpBootClient.c
> @@ -570,6 +570,7 @@ HttpBootFreeCacheList (
>@param[out] Buffer  The memory buffer to transfer the file 
> to. IF
> Buffer is NULL,
>then the size of the requested file is 
> returned in
>BufferSize.
> +  @param[out] ImageType   The image type of the downloaded file.
> 
>@retval EFI_SUCCESS  Successfully created.
>@retval Others   Failed to create HttpIo.
> @@ -580,7 +581,8 @@ HttpBootGetFileFromCache (
>IN HTTP_BOOT_PRIVATE_DATA   *Private,
>IN CHAR16   *Uri,
>IN OUT UINTN*BufferSize,
> - OUT UINT8*Buffer
> + OUT UINT8*Buffer,
> + OUT HTTP_BOOT_IMAGE_TYPE *ImageType
>)
>  {
>LIST_ENTRY  *Entry;
> @@ -603,7 +605,12 @@ HttpBootGetFileFromCache (
>  (StrCmp (Uri, Cache->RequestData->Url) == 0))
>  {
>//
> -  // Hit cache, c

Re: [edk2] [Patch] NetworkPkg: Add RAM disk boot support to HTTP Boot driver.

2016-03-31 Thread El-Haj-Mahmoud, Samer 
Since this is industry standard, it should be moved to 
MdePkg/IndustryStandard/Http11.h, next to the similarly defined content types, 
like HTTP_CONTENT_TYPE_APP_JSON, HTTP_CONTENT_TYPE_APP_OCTET_STREAM, 
HTTP_CONTENT_TYPE_IMAGE_JPEG, etc...

//
+// Provisional Standard Media Types defined in // 
+http://www.iana.org/assignments/provisional-standard-media-types/provis
+ional-standard-media-types.xhtml
+//
+#define HTTP_CONTENT_TYPE_APP_EFI   "application/efi"


Thanks,
--Samer



-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Fu Siyuan
Sent: Wednesday, March 30, 2016 10:37 PM
To: edk2-devel@lists.01.org
Cc: Ye Ting ; Wu Jiaxin 
Subject: [edk2] [Patch] NetworkPkg: Add RAM disk boot support to HTTP Boot 
driver.

This patch updates the HTTP Boot driver to support the download and boot a RAM 
disk image from HTTP server.
The HTTP RAM disk boot is described in section 23.7 "HTTP Boot" in UEFI 2.6. 
HTTP server could provide either an UEFI image or a RAM disk image for the HTTP 
boot client to use. The RAM disk image must contain a UEFI compliant file 
system in it.
HTTP boot driver will identify the image type either by the "Content-Type"
entity header filed or by the file name extension as below:
  "application/efi" or *.efi -> EFI Image
  *.iso  -> CD/DVD Image
  *.img  -> Virtual Disk Image

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
Cc: Ye Ting 
Cc: Wu Jiaxin 
Cc: El-Haj-Mahmoud Samer 
---
 NetworkPkg/HttpBootDxe/HttpBootClient.c  |  35 ++--
 NetworkPkg/HttpBootDxe/HttpBootClient.h  |   5 +-
 NetworkPkg/HttpBootDxe/HttpBootDxe.h |  19 +
 NetworkPkg/HttpBootDxe/HttpBootDxe.inf   |   3 +
 NetworkPkg/HttpBootDxe/HttpBootImpl.c| 130 ++---
 NetworkPkg/HttpBootDxe/HttpBootSupport.c | 135 +++ 
 NetworkPkg/HttpBootDxe/HttpBootSupport.h |  44 ++
 7 files changed, 319 insertions(+), 52 deletions(-)

diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.c 
b/NetworkPkg/HttpBootDxe/HttpBootClient.c
index 0c47293..0e30b22 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootClient.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootClient.c
@@ -570,6 +570,7 @@ HttpBootFreeCacheList (
   @param[out] Buffer  The memory buffer to transfer the file 
to. IF Buffer is NULL,
   then the size of the requested file is 
returned in
   BufferSize.
+  @param[out] ImageType   The image type of the downloaded file.
 
   @retval EFI_SUCCESS  Successfully created.
   @retval Others   Failed to create HttpIo.
@@ -580,7 +581,8 @@ HttpBootGetFileFromCache (
   IN HTTP_BOOT_PRIVATE_DATA   *Private,
   IN CHAR16   *Uri,
   IN OUT UINTN*BufferSize,
- OUT UINT8*Buffer
+ OUT UINT8*Buffer,
+ OUT HTTP_BOOT_IMAGE_TYPE *ImageType
   )
 {
   LIST_ENTRY  *Entry;
@@ -603,7 +605,12 @@ HttpBootGetFileFromCache (
 (StrCmp (Uri, Cache->RequestData->Url) == 0)) 
 {
   //
-  // Hit cache, check buffer size.
+  // Hit in cache, record image type.
+  //
+  *ImageType  = Cache->ImageType;
+
+  //
+  // Check buffer size.
   //
   if (*BufferSize < Cache->EntityLength) {
 *BufferSize = Cache->EntityLength; @@ -712,6 +719,7 @@ 
HttpBootGetBootFileCallback (
   @param[out]  Buffer  The memory buffer to transfer the file to. 
IF Buffer is NULL,
then the size of the requested file is 
returned in
BufferSize.
+  @param[out]  ImageType   The image type of the downloaded file.
 
   @retval EFI_SUCCESS  The file was loaded.
   @retval EFI_INVALID_PARAMETERBufferSize is NULL or Buffer Size is not 
NULL but Buffer is NULL.
@@ -727,7 +735,8 @@ HttpBootGetBootFile (
   IN HTTP_BOOT_PRIVATE_DATA   *Private,
   IN BOOLEAN  HeaderOnly,
   IN OUT UINTN*BufferSize,
- OUT UINT8*Buffer
+ OUT UINT8*Buffer,
+ OUT HTTP_BOOT_IMAGE_TYPE *ImageType
   )
 {
   EFI_STATUS Status;
@@ -750,7 +759,7 @@ HttpBootGetBootFile (
   ASSERT (Private != NULL);
   ASSERT (Private->HttpCreated);
 
-  if (BufferSize == NULL) {
+  if (BufferSize == NULL || ImageType == NULL) {
 return EFI_INVALID_PARAMETER;
   }
 
@@ -767,7 +776,7 @@ HttpBootGetBootFile (
   }
   AsciiStrToUnicodeStr (Private->BootFileUri, Url);
   if (!HeaderOnly) {
-Status = HttpBootGetFileFromCache (Private, Url, BufferSize, Buffer);
+Status = HttpBootGetFileFromCache (Private, Url, BufferSize, 
+