Re: [edk2] [PATCH v3] NetworkPkg: Add URI configuration form to HTTP boot driver.

[Fu, Siyuan]

Hi, Laszlo and Lin

Thanks for your detailed analysis on this problem, I created a patch to fix it 
and tested on my side. The connect/disconnect/reconnect could works well with 
this patch applied. Could you please help to verify it on the OVMF platform to 
see whether it works?

Best Regards
Siyuan

From: Gary Lin [mailto:g...@suse.com]
Sent: Monday, March 14, 2016 11:58 AM
To: Laszlo Ersek <ler...@redhat.com>
Cc: Ye, Ting <ting...@intel.com>; Fu, Siyuan <siyuan...@intel.com>; Wu, Jiaxin 
<jiaxin...@intel.com>; edk2-de...@ml01.01.org
Subject: Re: [edk2] [PATCH v3] NetworkPkg: Add URI configuration form to HTTP 
boot driver.

On Fri, Mar 11, 2016 at 04:46:49PM +0100, Laszlo Ersek wrote:
> On 03/11/16 05:26, Gary Lin wrote:
> > On Thu, Mar 10, 2016 at 12:31:18PM +0100, Laszlo Ersek wrote:
> >> On 03/10/16 11:02, Gary Lin wrote:
> >>> On Thu, Mar 10, 2016 at 10:20:12AM +0100, Laszlo Ersek wrote:
> >>>> On 03/10/16 08:49, Gary Lin wrote:
> >>
> >>>>> I found that it's related to iPXE. If I disable iPXE with
> >>>>>
> >>>>> "-netdev user,id=hostnet0 -device 
> >>>>> virtio-net-pci,romfile=,netdev=hostnet0"
> >>>>>
> >>>>> then everything works as expected. I'll try to dig deeper to find more
> >>>>> information.
> >>>>
> >>>> How fresh is your ipxe? And did you build it with CONFIG=qemu?
> >>>>
> >>> It comes from the qemu 2.5.0 tarball and I just installed it from
> >>> openSUSE Virtualization repo.
> >>> https://build.opensuse.org/package/show/Virtualization/qemu
> >>
> >> QEMU 2.5 bundles iPXE binaries built with CONFIG=qemu, but that
> >> configuration doesn't enable IPv6 for the moment.
> >>
> >> Apparently, combining iPXE (CONFIG=qemu) with NETWORK_IP6_ENABLE (set in
> >> OVMF) causes problems. I don't understand how this causes side effects,
> >> because CONFIG=qemu instructs iPXE to provide SNP drivers only. SNP is
> >> independent of IP version.
> >>
> >> With this combination, IPv6 PXE should work (not be absent), and IPv6
> >> HTTP Boot should work too (not blow up).
> >>
> >> Can you perhaps remove grub2 from the equation? What if you play with
> >> disconnect / reconnect / connect in the UEFI shell?
> >>
> > hmmm I found another way to crash the system with a different assert.
> >
> > Just disconnect/reconnect the handle providing HttpServiceBinding, and
> > OVMF crashed with/without iPXE:
> >
> > ASSERT /home/gary/git/edk2/NetworkPkg/HttpBootDxe/HttpBootDxe.c(796): CR 
> > has Bad Signature
> >
> > Here is the function that issues the assert.
> >
> > EFIAPI HttpBootIp6DxeDriverBindingStart ()
> >   ...
> >   if (!EFI_ERROR (Status)) {
> > Private = HTTP_BOOT_PRIVATE_DATA_FROM_ID(Id); < CRASH
> >   } else {
> >   ...
> >
> > This also only happens after fa848a4048943251fc057fe8d6c5a82e01d2ffb6.
>
> I found the bug. There are two key UEFI facts that are necessary for
> understanding it.
>
> (1) The first fact is that for any given handle, at most one instance of
> a given protocol interface (= GUID) can be installed on it. In other
> words, if you pick a handle, and pick a GUID, the handle either has the
> protocol interface installed, or not. The handle cannot have two or more
> instances of the same protocol.
>
> (2) The second fact is how the DisconnectController() boot service
> works, in case the DriverImageHandle parameter is specified as non-NULL,
> and the ChildHandle parameter is NULL. (This case means "disconnect the
> given driver from the given controller".) Let me quote the spec:
>
> A driver is disconnected from a controller by calling the Stop()
> service of the EFI_DRIVER_BINDING_PROTOCOL. The
> EFI_DRIVER_BINDING_PROTOCOL is on the driver image handle, and the
> handle of the controller is passed into the Stop() service.
>
> Now put the two facts together. You have a driver. For the driver, you
> have one ImageHandle. On that handle, you can install one instance of
> the driver binding protocol (due to fact (1)). So, when someone wants to
> disconnect a given controller from your driver, and calls
> gBS->DisconnectController() accordingly, exactly *one* Stop() function
> in your driver will be called. After that, the DisconnectController()
> boot service will return, and a complete successful disconnection
> between your driver and the controller will be assumed by the system.
>
> The HttpBootDxe driver violates this.
&

Re: [edk2] [PATCH v3] NetworkPkg: Add URI configuration form to HTTP boot driver.

[Gary Lin]

On Fri, Mar 11, 2016 at 04:46:49PM +0100, Laszlo Ersek wrote:
> On 03/11/16 05:26, Gary Lin wrote:
> > On Thu, Mar 10, 2016 at 12:31:18PM +0100, Laszlo Ersek wrote:
> >> On 03/10/16 11:02, Gary Lin wrote:
> >>> On Thu, Mar 10, 2016 at 10:20:12AM +0100, Laszlo Ersek wrote:
>  On 03/10/16 08:49, Gary Lin wrote:
> >>
> > I found that it's related to iPXE. If I disable iPXE with
> >
> > "-netdev user,id=hostnet0 -device 
> > virtio-net-pci,romfile=,netdev=hostnet0"
> >
> > then everything works as expected. I'll try to dig deeper to find more
> > information.
> 
>  How fresh is your ipxe? And did you build it with CONFIG=qemu?
> 
> >>> It comes from the qemu 2.5.0 tarball and I just installed it from
> >>> openSUSE Virtualization repo.
> >>> https://build.opensuse.org/package/show/Virtualization/qemu
> >>
> >> QEMU 2.5 bundles iPXE binaries built with CONFIG=qemu, but that
> >> configuration doesn't enable IPv6 for the moment.
> >>
> >> Apparently, combining iPXE (CONFIG=qemu) with NETWORK_IP6_ENABLE (set in
> >> OVMF) causes problems. I don't understand how this causes side effects,
> >> because CONFIG=qemu instructs iPXE to provide SNP drivers only. SNP is
> >> independent of IP version.
> >>
> >> With this combination, IPv6 PXE should work (not be absent), and IPv6
> >> HTTP Boot should work too (not blow up).
> >>
> >> Can you perhaps remove grub2 from the equation? What if you play with
> >> disconnect / reconnect / connect in the UEFI shell?
> >>
> > hmmm I found another way to crash the system with a different assert.
> > 
> > Just disconnect/reconnect the handle providing HttpServiceBinding, and
> > OVMF crashed with/without iPXE:
> > 
> > ASSERT /home/gary/git/edk2/NetworkPkg/HttpBootDxe/HttpBootDxe.c(796): CR 
> > has Bad Signature
> > 
> > Here is the function that issues the assert.
> > 
> > EFIAPI HttpBootIp6DxeDriverBindingStart ()
> >   ...
> >   if (!EFI_ERROR (Status)) {
> > Private = HTTP_BOOT_PRIVATE_DATA_FROM_ID(Id); < CRASH
> >   } else {
> >   ...
> > 
> > This also only happens after fa848a4048943251fc057fe8d6c5a82e01d2ffb6.
> 
> I found the bug. There are two key UEFI facts that are necessary for
> understanding it.
> 
> (1) The first fact is that for any given handle, at most one instance of
> a given protocol interface (= GUID) can be installed on it. In other
> words, if you pick a handle, and pick a GUID, the handle either has the
> protocol interface installed, or not. The handle cannot have two or more
> instances of the same protocol.
> 
> (2) The second fact is how the DisconnectController() boot service
> works, in case the DriverImageHandle parameter is specified as non-NULL,
> and the ChildHandle parameter is NULL. (This case means "disconnect the
> given driver from the given controller".) Let me quote the spec:
> 
> A driver is disconnected from a controller by calling the Stop()
> service of the EFI_DRIVER_BINDING_PROTOCOL. The
> EFI_DRIVER_BINDING_PROTOCOL is on the driver image handle, and the
> handle of the controller is passed into the Stop() service.
> 
> Now put the two facts together. You have a driver. For the driver, you
> have one ImageHandle. On that handle, you can install one instance of
> the driver binding protocol (due to fact (1)). So, when someone wants to
> disconnect a given controller from your driver, and calls
> gBS->DisconnectController() accordingly, exactly *one* Stop() function
> in your driver will be called. After that, the DisconnectController()
> boot service will return, and a complete successful disconnection
> between your driver and the controller will be assumed by the system.
> 
> The HttpBootDxe driver violates this.
> 
> If you look at its entry point function (called
> HttpBootDxeDriverEntryPoint()), you see that it installs *two* instances
> of the driver binding protocol -- but only the first (the IPv4 one) goes
> on the driver's genuine ImageHandle. The second instance (the IPv6 one)
> goes on to a NULL handle.
> 
> The EfiLibInstallDriverBindingComponentName2() utility function seems to
> support this. A new handle is created by the system, and the
> "gHttpBootIp6DxeDriverBinding" protocol instance is installed on that
> separate, new handle.
> 
> Now let us consider the HttpBootConfigFormInit() and
> HttpBootConfigFormUnload() functions that were introduced by the commit
> you found. The HttpBootConfigFormInit() function adds a permanent
> reference to the parent handle's HTTP SB protocol, using the attribute
> EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER. The HttpBootConfigFormUnload()
> function removes this reference.
> 
> The IPv4 and IPv6 variants of the driver binding start & stop functions
> implement a kind of reference counting between each other. Namely,
> regardless of which one of the IPv4 or IPv6 binding start functions is
> called first, *that* first one will call HttpBootConfigFormInit(), and
> the second one will not call it. The resources set up 

Re: [edk2] [PATCH v3] NetworkPkg: Add URI configuration form to HTTP boot driver.

[Laszlo Ersek]

On 03/11/16 05:26, Gary Lin wrote:
> On Thu, Mar 10, 2016 at 12:31:18PM +0100, Laszlo Ersek wrote:
>> On 03/10/16 11:02, Gary Lin wrote:
>>> On Thu, Mar 10, 2016 at 10:20:12AM +0100, Laszlo Ersek wrote:
 On 03/10/16 08:49, Gary Lin wrote:
>>
> I found that it's related to iPXE. If I disable iPXE with
>
> "-netdev user,id=hostnet0 -device virtio-net-pci,romfile=,netdev=hostnet0"
>
> then everything works as expected. I'll try to dig deeper to find more
> information.

 How fresh is your ipxe? And did you build it with CONFIG=qemu?

>>> It comes from the qemu 2.5.0 tarball and I just installed it from
>>> openSUSE Virtualization repo.
>>> https://build.opensuse.org/package/show/Virtualization/qemu
>>
>> QEMU 2.5 bundles iPXE binaries built with CONFIG=qemu, but that
>> configuration doesn't enable IPv6 for the moment.
>>
>> Apparently, combining iPXE (CONFIG=qemu) with NETWORK_IP6_ENABLE (set in
>> OVMF) causes problems. I don't understand how this causes side effects,
>> because CONFIG=qemu instructs iPXE to provide SNP drivers only. SNP is
>> independent of IP version.
>>
>> With this combination, IPv6 PXE should work (not be absent), and IPv6
>> HTTP Boot should work too (not blow up).
>>
>> Can you perhaps remove grub2 from the equation? What if you play with
>> disconnect / reconnect / connect in the UEFI shell?
>>
> hmmm I found another way to crash the system with a different assert.
> 
> Just disconnect/reconnect the handle providing HttpServiceBinding, and
> OVMF crashed with/without iPXE:
> 
> ASSERT /home/gary/git/edk2/NetworkPkg/HttpBootDxe/HttpBootDxe.c(796): CR has 
> Bad Signature
> 
> Here is the function that issues the assert.
> 
> EFIAPI HttpBootIp6DxeDriverBindingStart ()
>   ...
>   if (!EFI_ERROR (Status)) {
> Private = HTTP_BOOT_PRIVATE_DATA_FROM_ID(Id); < CRASH
>   } else {
>   ...
> 
> This also only happens after fa848a4048943251fc057fe8d6c5a82e01d2ffb6.

I found the bug. There are two key UEFI facts that are necessary for
understanding it.

(1) The first fact is that for any given handle, at most one instance of
a given protocol interface (= GUID) can be installed on it. In other
words, if you pick a handle, and pick a GUID, the handle either has the
protocol interface installed, or not. The handle cannot have two or more
instances of the same protocol.

(2) The second fact is how the DisconnectController() boot service
works, in case the DriverImageHandle parameter is specified as non-NULL,
and the ChildHandle parameter is NULL. (This case means "disconnect the
given driver from the given controller".) Let me quote the spec:

A driver is disconnected from a controller by calling the Stop()
service of the EFI_DRIVER_BINDING_PROTOCOL. The
EFI_DRIVER_BINDING_PROTOCOL is on the driver image handle, and the
handle of the controller is passed into the Stop() service.

Now put the two facts together. You have a driver. For the driver, you
have one ImageHandle. On that handle, you can install one instance of
the driver binding protocol (due to fact (1)). So, when someone wants to
disconnect a given controller from your driver, and calls
gBS->DisconnectController() accordingly, exactly *one* Stop() function
in your driver will be called. After that, the DisconnectController()
boot service will return, and a complete successful disconnection
between your driver and the controller will be assumed by the system.

The HttpBootDxe driver violates this.

If you look at its entry point function (called
HttpBootDxeDriverEntryPoint()), you see that it installs *two* instances
of the driver binding protocol -- but only the first (the IPv4 one) goes
on the driver's genuine ImageHandle. The second instance (the IPv6 one)
goes on to a NULL handle.

The EfiLibInstallDriverBindingComponentName2() utility function seems to
support this. A new handle is created by the system, and the
"gHttpBootIp6DxeDriverBinding" protocol instance is installed on that
separate, new handle.

Now let us consider the HttpBootConfigFormInit() and
HttpBootConfigFormUnload() functions that were introduced by the commit
you found. The HttpBootConfigFormInit() function adds a permanent
reference to the parent handle's HTTP SB protocol, using the attribute
EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER. The HttpBootConfigFormUnload()
function removes this reference.

The IPv4 and IPv6 variants of the driver binding start & stop functions
implement a kind of reference counting between each other. Namely,
regardless of which one of the IPv4 or IPv6 binding start functions is
called first, *that* first one will call HttpBootConfigFormInit(), and
the second one will not call it. The resources set up by the
HttpBootConfigFormInit() function (including the child reference to HTTP
SB) are shared between IPv4 and IPv6.

Similarly, when the stop functions are called, their ordering is
irrelevant; it is always the second one that is supposed to call
HttpBootConfigFormUnload().

Re: [edk2] [PATCH v3] NetworkPkg: Add URI configuration form to HTTP boot driver.

[Laszlo Ersek]

On 03/11/16 05:26, Gary Lin wrote:
> On Thu, Mar 10, 2016 at 12:31:18PM +0100, Laszlo Ersek wrote:
>> On 03/10/16 11:02, Gary Lin wrote:
>>> On Thu, Mar 10, 2016 at 10:20:12AM +0100, Laszlo Ersek wrote:
 On 03/10/16 08:49, Gary Lin wrote:
>>
> I found that it's related to iPXE. If I disable iPXE with
>
> "-netdev user,id=hostnet0 -device virtio-net-pci,romfile=,netdev=hostnet0"
>
> then everything works as expected. I'll try to dig deeper to find more
> information.

 How fresh is your ipxe? And did you build it with CONFIG=qemu?

>>> It comes from the qemu 2.5.0 tarball and I just installed it from
>>> openSUSE Virtualization repo.
>>> https://build.opensuse.org/package/show/Virtualization/qemu
>>
>> QEMU 2.5 bundles iPXE binaries built with CONFIG=qemu, but that
>> configuration doesn't enable IPv6 for the moment.
>>
>> Apparently, combining iPXE (CONFIG=qemu) with NETWORK_IP6_ENABLE (set in
>> OVMF) causes problems. I don't understand how this causes side effects,
>> because CONFIG=qemu instructs iPXE to provide SNP drivers only. SNP is
>> independent of IP version.
>>
>> With this combination, IPv6 PXE should work (not be absent), and IPv6
>> HTTP Boot should work too (not blow up).
>>
>> Can you perhaps remove grub2 from the equation? What if you play with
>> disconnect / reconnect / connect in the UEFI shell?
>>
> hmmm I found another way to crash the system with a different assert.
> 
> Just disconnect/reconnect the handle providing HttpServiceBinding, and
> OVMF crashed with/without iPXE:
> 
> ASSERT /home/gary/git/edk2/NetworkPkg/HttpBootDxe/HttpBootDxe.c(796): CR has 
> Bad Signature
> 
> Here is the function that issues the assert.
> 
> EFIAPI HttpBootIp6DxeDriverBindingStart ()
>   ...
>   if (!EFI_ERROR (Status)) {
> Private = HTTP_BOOT_PRIVATE_DATA_FROM_ID(Id); < CRASH
>   } else {
>   ...
> 
> This also only happens after fa848a4048943251fc057fe8d6c5a82e01d2ffb6.

Here's a stack dump:

#0  0x7e86cd8c in CpuDeadLoop () at 
MdePkg/Library/BaseLib/CpuDeadLoop.c:37
#1  0x7e86756c in DebugAssert (FileName=0x7e86d7c8 
"NetworkPkg/HttpBootDxe/HttpBootDxe.c", LineNumber=796, Description=0x7e86d823 
"CR has Bad Signature")
at OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c:153
#2  0x7e85a8bf in HttpBootIp6DxeDriverBindingStart (This=0x7e871e60, 
ControllerHandle=0x7f040d98, RemainingDevicePath=0x0) at 
NetworkPkg/HttpBootDxe/HttpBootDxe.c:796
#3  0x7ff6afc0 in CoreConnectSingleController 
(ControllerHandle=0x7f040d98, ContextDriverImageHandles=0x0, 
RemainingDevicePath=0x0) at MdeModulePkg/Core/Dxe/Hand/DriverSupport.c:646
#4  0x7ff6a2cf in CoreConnectController (ControllerHandle=0x7f040d98, 
DriverImageHandle=0x0, RemainingDevicePath=0x0, Recursive=1 '\001') at 
MdeModulePkg/Core/Dxe/Hand/DriverSupport.c:137
#5  0x7ff6d887 in CoreDisconnectControllersUsingProtocolInterface 
(UserHandle=0x7f040d98, Prot=0x7eecfb98) at 
MdeModulePkg/Core/Dxe/Hand/Handle.c:687
#6  0x7ff6d955 in CoreUninstallProtocolInterface 
(UserHandle=0x7f040d98, Protocol=0x7e885f60 
, Interface=0x7eecfce0) at 
MdeModulePkg/Core/Dxe/Hand/Handle.c:754
#7  0x7e87536b in HttpDxeStop (This=0x7e8860e0, 
ControllerHandle=0x7eed3cd8, NumberOfChildren=0, ChildHandleBuffer=0x0, 
IpVersion=6 '\006') at NetworkPkg/HttpDxe/HttpDriver.c:576
#8  0x7e875509 in HttpDxeIp6DriverBindingStop (This=0x7e8860e0, 
ControllerHandle=0x7eed3cd8, NumberOfChildren=0, ChildHandleBuffer=0x0) at 
NetworkPkg/HttpDxe/HttpDriver.c:891
#9  0x7ff6b987 in CoreDisconnectController 
(ControllerHandle=0x7eed3cd8, DriverImageHandle=0x7f17ebd8, ChildHandle=0x0) at 
MdeModulePkg/Core/Dxe/Hand/DriverSupport.c:938
#10 0x7ff6d6fc in CoreDisconnectControllersUsingProtocolInterface 
(UserHandle=0x7eed3cd8, Prot=0x7eed3c58) at 
MdeModulePkg/Core/Dxe/Hand/Handle.c:651
#11 0x7ff6d955 in CoreUninstallProtocolInterface 
(UserHandle=0x7eed3cd8, Protocol=0x7e948f10 , 
Interface=0x7eed3a38) at MdeModulePkg/Core/Dxe/Hand/Handle.c:754
#12 0x7ff6dafd in CoreUninstallMultipleProtocolInterfaces 
(Handle=0x7eed3cd8) at MdeModulePkg/Core/Dxe/Hand/Handle.c:854
#13 0x7e935f6c in SockDestroy (Sock=0x7eed3918) at 
NetworkPkg/TcpDxe/SockImpl.c:849
#14 0x7e92af15 in SockDestroyChild (Sock=0x7eed3918) at 
NetworkPkg/TcpDxe/SockInterface.c:195
#15 0x7e92aaf6 in TcpServiceBindingDestroyChild (This=0x7e78dd00, 
ChildHandle=0x7eed3cd8) at NetworkPkg/TcpDxe/TcpDriver.c:1002
#16 0x7e92a018 in TcpDestroyChildEntryInHandleBuffer (Entry=0x7eed3940, 
Context=0x7ff5dc20) at NetworkPkg/TcpDxe/TcpDriver.c:401
#17 0x7e93fd63 in NetDestroyLinkList (List=0x7e78dd10, 
CallBack=0x7e929f2d , Context=0x7ff5dc20, 
ListLength=0x0) at MdeModulePkg/Library/DxeNetLib/DxeNetLib.c:1117
#18 0x7e92a1ee in TcpDestroyService (Controller=0x7ee18d18, 
ImageHandle=0x7f1852d8, NumberOfChildren=2, ChildHandleBuffer=0x7ee52d98, 

Re: [edk2] [PATCH v3] NetworkPkg: Add URI configuration form to HTTP boot driver.

[Gary Lin]

On Thu, Mar 10, 2016 at 12:31:18PM +0100, Laszlo Ersek wrote:
> On 03/10/16 11:02, Gary Lin wrote:
> > On Thu, Mar 10, 2016 at 10:20:12AM +0100, Laszlo Ersek wrote:
> >> On 03/10/16 08:49, Gary Lin wrote:
> 
> >>> I found that it's related to iPXE. If I disable iPXE with
> >>>
> >>> "-netdev user,id=hostnet0 -device virtio-net-pci,romfile=,netdev=hostnet0"
> >>>
> >>> then everything works as expected. I'll try to dig deeper to find more
> >>> information.
> >>
> >> How fresh is your ipxe? And did you build it with CONFIG=qemu?
> >>
> > It comes from the qemu 2.5.0 tarball and I just installed it from
> > openSUSE Virtualization repo.
> > https://build.opensuse.org/package/show/Virtualization/qemu
> 
> QEMU 2.5 bundles iPXE binaries built with CONFIG=qemu, but that
> configuration doesn't enable IPv6 for the moment.
> 
> Apparently, combining iPXE (CONFIG=qemu) with NETWORK_IP6_ENABLE (set in
> OVMF) causes problems. I don't understand how this causes side effects,
> because CONFIG=qemu instructs iPXE to provide SNP drivers only. SNP is
> independent of IP version.
> 
> With this combination, IPv6 PXE should work (not be absent), and IPv6
> HTTP Boot should work too (not blow up).
> 
> Can you perhaps remove grub2 from the equation? What if you play with
> disconnect / reconnect / connect in the UEFI shell?
> 
hmmm I found another way to crash the system with a different assert.

Just disconnect/reconnect the handle providing HttpServiceBinding, and
OVMF crashed with/without iPXE:

ASSERT /home/gary/git/edk2/NetworkPkg/HttpBootDxe/HttpBootDxe.c(796): CR has 
Bad Signature

Here is the function that issues the assert.

EFIAPI HttpBootIp6DxeDriverBindingStart ()
  ...
  if (!EFI_ERROR (Status)) {
Private = HTTP_BOOT_PRIVATE_DATA_FROM_ID(Id); < CRASH
  } else {
  ...

This also only happens after fa848a4048943251fc057fe8d6c5a82e01d2ffb6.

Thanks,

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

Re: [edk2] [PATCH v3] NetworkPkg: Add URI configuration form to HTTP boot driver.

[Laszlo Ersek]

On 03/10/16 11:02, Gary Lin wrote:
> On Thu, Mar 10, 2016 at 10:20:12AM +0100, Laszlo Ersek wrote:
>> On 03/10/16 08:49, Gary Lin wrote:

>>> I found that it's related to iPXE. If I disable iPXE with
>>>
>>> "-netdev user,id=hostnet0 -device virtio-net-pci,romfile=,netdev=hostnet0"
>>>
>>> then everything works as expected. I'll try to dig deeper to find more
>>> information.
>>
>> How fresh is your ipxe? And did you build it with CONFIG=qemu?
>>
> It comes from the qemu 2.5.0 tarball and I just installed it from
> openSUSE Virtualization repo.
> https://build.opensuse.org/package/show/Virtualization/qemu

QEMU 2.5 bundles iPXE binaries built with CONFIG=qemu, but that
configuration doesn't enable IPv6 for the moment.

Apparently, combining iPXE (CONFIG=qemu) with NETWORK_IP6_ENABLE (set in
OVMF) causes problems. I don't understand how this causes side effects,
because CONFIG=qemu instructs iPXE to provide SNP drivers only. SNP is
independent of IP version.

With this combination, IPv6 PXE should work (not be absent), and IPv6
HTTP Boot should work too (not blow up).

Can you perhaps remove grub2 from the equation? What if you play with
disconnect / reconnect / connect in the UEFI shell?

Thanks
Laszlo

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

Re: [edk2] [PATCH v3] NetworkPkg: Add URI configuration form to HTTP boot driver.

[Gary Lin]

On Thu, Mar 10, 2016 at 10:20:12AM +0100, Laszlo Ersek wrote:
> On 03/10/16 08:49, Gary Lin wrote:
> > On Wed, Mar 09, 2016 at 04:54:08PM +0800, Gary Lin wrote:
> >> On Fri, Mar 04, 2016 at 04:38:22PM +0800, Fu Siyuan wrote:
> >>> This patch updates the HTTP boot driver to produce a setup page for the 
> >>> boot
> >>> file URI configuration. A new boot option will be created for the manual
> >>> configured URI address. This change is made to support the HTTP boot usage
> >>> in home environment.
> >>>
> >> Hi Siyuan,
> >>
> >> After updating the latest git HEAD, my qemu spitted this after grub2
> >> loaded the linux kernel:
> >>
> >> ASSERT /home/gary/git/edk2/NetworkPkg/HttpDxe/HttpDriver.c(555): CR has 
> >> Bad Signature
> >>
> >> The result of bisect shows the first bad commit is 
> >>
> >> fa848a4048943251fc057fe8d6c5a82e01d2ffb6
> >> NetworkPkg: Add URI configuration form to HTTP boot driver.
> >>
> >> Would you mind to check this?
> >>
> > I found that it's related to iPXE. If I disable iPXE with
> > 
> > "-netdev user,id=hostnet0 -device virtio-net-pci,romfile=,netdev=hostnet0"
> > 
> > then everything works as expected. I'll try to dig deeper to find more
> > information.
> 
> How fresh is your ipxe? And did you build it with CONFIG=qemu?
> 
It comes from the qemu 2.5.0 tarball and I just installed it from
openSUSE Virtualization repo.
https://build.opensuse.org/package/show/Virtualization/qemu

I kind of suspect it's related to the IPv6 support in iPXE. A few more
things I observed:

1. The symptom disappeared after I removed "NETWORK_IP6_ENABLE" from
   my OVMF build config.

2. With iPXE, there is no IPv6 PXE entry in the BootManager menu, but
   HTTP Boot IPv6 showed.

3. I added a few more debug messages and here is the log:
   http://paste.opensuse.org/81288048

   In the end of the log, the parameters to HttpDxeStop were printed:

HttpDxeIp4DriverBindingStop
HttpDxeStop
This=3E878CC0, ControllerHandle=3EF28C58, NumberOfChildren=0, 
ChildHandleBuffer=0, IpVersion=4
HttpDxeStop NicHandle=3F0ABBD8, This->DriverBindingHandle=3F16A558, 
ServiceBinding=3EF25C60

HttpDxeIp6DriverBindingStop
HttpDxeStop
This=3E878D00, ControllerHandle=3EF2CCD8, NumberOfChildren=0, 
ChildHandleBuffer=0, IpVersion=6
HttpDxeStop NicHandle=3F0ABBD8, This->DriverBindingHandle=3F169858, 
ServiceBinding=3EF25C60

HttpDxeStart
This=3E878D00, ControllerHandle=3F0ABBD8, RemainingDevicePath=0, IpVersion=6
HttpDxeStart This->DriverBindingHandle=3F169858, ServiceBinding=3EF25C60

HttpDxeIp6DriverBindingStop
HttpDxeStop
This=3E878D00, ControllerHandle=3EF2CCD8, NumberOfChildren=0, 
ChildHandleBuffer=0, IpVersion=6
HttpDxeStop NicHandle=3F0ABBD8, This->DriverBindingHandle=3F169858, 
ServiceBinding=3EF25C60

ASSERT /home/gary/git/edk2/NetworkPkg/HttpDxe/HttpDriver.c(567): CR has Bad 
Signature

   HttpDxeStop was invoked twice with the same parameters, and this is
   probably why the assert was raised.

   Furthermore, without iPXE or NETWORK_IP6_ENABLE, HttpDxeStop was
   never invoked.

The HTTPBoot driver may need to detect IPv6 support from UNDI just like the
PXEBoot driver.

2bbe9553c495bb9024b4b51743142a0a50e0d370
Add IPV6 support from UNDI

Cheers,

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

Re: [edk2] [PATCH v3] NetworkPkg: Add URI configuration form to HTTP boot driver.

[Laszlo Ersek]

On 03/10/16 08:49, Gary Lin wrote:
> On Wed, Mar 09, 2016 at 04:54:08PM +0800, Gary Lin wrote:
>> On Fri, Mar 04, 2016 at 04:38:22PM +0800, Fu Siyuan wrote:
>>> This patch updates the HTTP boot driver to produce a setup page for the boot
>>> file URI configuration. A new boot option will be created for the manual
>>> configured URI address. This change is made to support the HTTP boot usage
>>> in home environment.
>>>
>> Hi Siyuan,
>>
>> After updating the latest git HEAD, my qemu spitted this after grub2
>> loaded the linux kernel:
>>
>> ASSERT /home/gary/git/edk2/NetworkPkg/HttpDxe/HttpDriver.c(555): CR has Bad 
>> Signature
>>
>> The result of bisect shows the first bad commit is 
>>
>> fa848a4048943251fc057fe8d6c5a82e01d2ffb6
>> NetworkPkg: Add URI configuration form to HTTP boot driver.
>>
>> Would you mind to check this?
>>
> I found that it's related to iPXE. If I disable iPXE with
> 
> "-netdev user,id=hostnet0 -device virtio-net-pci,romfile=,netdev=hostnet0"
> 
> then everything works as expected. I'll try to dig deeper to find more
> information.

How fresh is your ipxe? And did you build it with CONFIG=qemu?

Thanks
Laszlo

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

Re: [edk2] [PATCH v3] NetworkPkg: Add URI configuration form to HTTP boot driver.

[Gary Lin]

On Wed, Mar 09, 2016 at 04:54:08PM +0800, Gary Lin wrote:
> On Fri, Mar 04, 2016 at 04:38:22PM +0800, Fu Siyuan wrote:
> > This patch updates the HTTP boot driver to produce a setup page for the boot
> > file URI configuration. A new boot option will be created for the manual
> > configured URI address. This change is made to support the HTTP boot usage
> > in home environment.
> > 
> Hi Siyuan,
> 
> After updating the latest git HEAD, my qemu spitted this after grub2
> loaded the linux kernel:
> 
> ASSERT /home/gary/git/edk2/NetworkPkg/HttpDxe/HttpDriver.c(555): CR has Bad 
> Signature
> 
> The result of bisect shows the first bad commit is 
> 
> fa848a4048943251fc057fe8d6c5a82e01d2ffb6
> NetworkPkg: Add URI configuration form to HTTP boot driver.
> 
> Would you mind to check this?
> 
I found that it's related to iPXE. If I disable iPXE with

"-netdev user,id=hostnet0 -device virtio-net-pci,romfile=,netdev=hostnet0"

then everything works as expected. I'll try to dig deeper to find more
information.

Cheers,

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

Re: [edk2] [PATCH v3] NetworkPkg: Add URI configuration form to HTTP boot driver.

[Gary Lin]

On Fri, Mar 04, 2016 at 04:38:22PM +0800, Fu Siyuan wrote:
> This patch updates the HTTP boot driver to produce a setup page for the boot
> file URI configuration. A new boot option will be created for the manual
> configured URI address. This change is made to support the HTTP boot usage
> in home environment.
> 
Hi Siyuan,

After updating the latest git HEAD, my qemu spitted this after grub2
loaded the linux kernel:

ASSERT /home/gary/git/edk2/NetworkPkg/HttpDxe/HttpDriver.c(555): CR has Bad 
Signature

The result of bisect shows the first bad commit is 

fa848a4048943251fc057fe8d6c5a82e01d2ffb6
NetworkPkg: Add URI configuration form to HTTP boot driver.

Would you mind to check this?

Thanks,

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

Re: [edk2] [PATCH v3] NetworkPkg: Add URI configuration form to HTTP boot driver.

[Gao, Liming]

Laszlo:
  I have used GCC windows binary tools in my daily work. It is very helpful for 
the windows user. GCC Windows binary can be got from 
http://sourceforge.net/projects/edk2developertoolsforwindows/files/Tool%20Chain%20Binaries/x86.
 I would suggest we add wiki in GitHub to let user know how to set up GCC 
windows build environment.

Thanks
Liming
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
Ersek
Sent: Monday, March 7, 2016 6:41 PM
To: Fu, Siyuan <siyuan...@intel.com>; edk2-de...@ml01.01.org
Cc: Ye, Ting <ting...@intel.com>; Wu, Jiaxin <jiaxin...@intel.com>
Subject: Re: [edk2] [PATCH v3] NetworkPkg: Add URI configuration form to HTTP 
boot driver.

This patch (which is only ~1900 lines in size, including context...)

On 03/04/16 09:38, Fu Siyuan wrote:
> This patch updates the HTTP boot driver to produce a setup page for the boot
> file URI configuration. A new boot option will be created for the manual
> configured URI address. This change is made to support the HTTP boot usage
> in home environment.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Fu Siyuan
> Cc: Wu Jiaxin
> Cc: Ye Ting
> ---
> NetworkPkg/HttpBootDxe/HttpBootClient.c | 99 +--
> NetworkPkg/HttpBootDxe/HttpBootConfig.c | 723 +
> NetworkPkg/HttpBootDxe/HttpBootConfig.h | 78 +++
> NetworkPkg/HttpBootDxe/HttpBootConfigNVDataStruc.h | 43 ++
> NetworkPkg/HttpBootDxe/HttpBootConfigStrings.uni | Bin 0 -> 2926 bytes
> NetworkPkg/HttpBootDxe/HttpBootConfigVfr.vfr | 53 ++
> NetworkPkg/HttpBootDxe/HttpBootDhcp4.c | 111 +++-
> NetworkPkg/HttpBootDxe/HttpBootDhcp4.h | 9 +-
> NetworkPkg/HttpBootDxe/HttpBootDhcp6.c | 6 +-
> NetworkPkg/HttpBootDxe/HttpBootDxe.c | 44 +-
> NetworkPkg/HttpBootDxe/HttpBootDxe.h | 33 +-
> NetworkPkg/HttpBootDxe/HttpBootDxe.inf | 17 +-
> NetworkPkg/HttpBootDxe/HttpBootImpl.c | 71 +-
> NetworkPkg/HttpBootDxe/HttpBootSupport.c | 63 ++
> NetworkPkg/HttpBootDxe/HttpBootSupport.h | 18 +
> NetworkPkg/Include/Guid/HttpBootConfigHii.h | 25 +
> NetworkPkg/NetworkPkg.dec | 5 +-
> 17 files changed, 1288 insertions(+), 110 deletions(-)
> create mode 100644 NetworkPkg/HttpBootDxe/HttpBootConfig.c
> create mode 100644 NetworkPkg/HttpBootDxe/HttpBootConfig.h
> create mode 100644 NetworkPkg/HttpBootDxe/HttpBootConfigNVDataStruc.h
> create mode 100644 NetworkPkg/HttpBootDxe/HttpBootConfigStrings.uni
> create mode 100644 NetworkPkg/HttpBootDxe/HttpBootConfigVfr.vfr
> create mode 100644 NetworkPkg/Include/Guid/HttpBootConfigHii.h

breaks the gcc build here:

> + //
> + // Get current "BootOrder" variable and find a free target.
> + //
> + Length = 0;
> + Status = GetVariable2 (
> + L"BootOrder",
> + ,
> + ,
> + 
> + );

with the following error message:


NetworkPkg/HttpBootDxe/HttpBootConfig.c: In function
'HttpBootAddBootOption':
NetworkPkg/HttpBootDxe/HttpBootConfig.c:148:14: error: passing argument
3 of 'GetVariable2' from incompatible pointer type [-Werror]
);
^
In file included from NetworkPkg/HttpBootDxe/HttpBootDxe.h:31:0,
from NetworkPkg/HttpBootDxe/HttpBootConfig.c:15:
MdePkg/Include/Library/UefiLib.h:708:1: note: expected 'void **' but
argument is of type 'CHAR16 **'
GetVariable2 (
^
cc1: all warnings being treated as errors


First, the BootOrder variable is not composed of CHAR16 elements (it is
not a UCS-2 string); it is composed of UINT16 elements. From the UEFI spec:

The BootOrder variable contains an array of UINT16's that make up
an ordered list of the Boot options.

Second, can we please figure out a way for Windows using developers to
build edk2 with at least one version of gcc? These build failures are
*incredibly* annoying. The build breaks, someone submits a patch, the
reviewer lives on the other side of the planet, a day passes, review is
in, another day passes, the submitter commits it, okay, it builds again
now. Terrible.

I would absolutely test-build my edk2 patches with a VS release if one
was available to me at no cost. I could run it in a virtual machine. I
don't do it because I can't justify the $$$ to mgmt just for this. But
gcc binaries are available to Windows people at zero cost, and gcc's use
on Windows has been repeatedly discussed on this list. I find this state
desperate.

Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org<mailto: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 v3] NetworkPkg: Add URI configuration form to HTTP boot driver.

[Laszlo Ersek]

This patch (which is only ~1900 lines in size, including context...)

On 03/04/16 09:38, Fu Siyuan wrote:
> This patch updates the HTTP boot driver to produce a setup page for the boot
> file URI configuration. A new boot option will be created for the manual
> configured URI address. This change is made to support the HTTP boot usage
> in home environment.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Fu Siyuan 
> Cc: Wu Jiaxin 
> Cc: Ye Ting 
> ---
>  NetworkPkg/HttpBootDxe/HttpBootClient.c|  99 +--
>  NetworkPkg/HttpBootDxe/HttpBootConfig.c| 723 
> +
>  NetworkPkg/HttpBootDxe/HttpBootConfig.h|  78 +++
>  NetworkPkg/HttpBootDxe/HttpBootConfigNVDataStruc.h |  43 ++
>  NetworkPkg/HttpBootDxe/HttpBootConfigStrings.uni   | Bin 0 -> 2926 bytes
>  NetworkPkg/HttpBootDxe/HttpBootConfigVfr.vfr   |  53 ++
>  NetworkPkg/HttpBootDxe/HttpBootDhcp4.c | 111 +++-
>  NetworkPkg/HttpBootDxe/HttpBootDhcp4.h |   9 +-
>  NetworkPkg/HttpBootDxe/HttpBootDhcp6.c |   6 +-
>  NetworkPkg/HttpBootDxe/HttpBootDxe.c   |  44 +-
>  NetworkPkg/HttpBootDxe/HttpBootDxe.h   |  33 +-
>  NetworkPkg/HttpBootDxe/HttpBootDxe.inf |  17 +-
>  NetworkPkg/HttpBootDxe/HttpBootImpl.c  |  71 +-
>  NetworkPkg/HttpBootDxe/HttpBootSupport.c   |  63 ++
>  NetworkPkg/HttpBootDxe/HttpBootSupport.h   |  18 +
>  NetworkPkg/Include/Guid/HttpBootConfigHii.h|  25 +
>  NetworkPkg/NetworkPkg.dec  |   5 +-
>  17 files changed, 1288 insertions(+), 110 deletions(-)
>  create mode 100644 NetworkPkg/HttpBootDxe/HttpBootConfig.c
>  create mode 100644 NetworkPkg/HttpBootDxe/HttpBootConfig.h
>  create mode 100644 NetworkPkg/HttpBootDxe/HttpBootConfigNVDataStruc.h
>  create mode 100644 NetworkPkg/HttpBootDxe/HttpBootConfigStrings.uni
>  create mode 100644 NetworkPkg/HttpBootDxe/HttpBootConfigVfr.vfr
>  create mode 100644 NetworkPkg/Include/Guid/HttpBootConfigHii.h

breaks the gcc build here:

> +  //
> +  // Get current "BootOrder" variable and find a free target.
> +  //
> +  Length = 0;
> +  Status = GetVariable2 (
> + L"BootOrder",
> + ,
> + ,
> +  
> + );

with the following error message:


NetworkPkg/HttpBootDxe/HttpBootConfig.c: In function
'HttpBootAddBootOption':
NetworkPkg/HttpBootDxe/HttpBootConfig.c:148:14: error: passing argument
3 of 'GetVariable2' from incompatible pointer type [-Werror]
  );
  ^
In file included from NetworkPkg/HttpBootDxe/HttpBootDxe.h:31:0,
 from NetworkPkg/HttpBootDxe/HttpBootConfig.c:15:
MdePkg/Include/Library/UefiLib.h:708:1: note: expected 'void **' but
argument is of type 'CHAR16 **'
 GetVariable2 (
 ^
cc1: all warnings being treated as errors


First, the BootOrder variable is not composed of CHAR16 elements (it is
not a UCS-2 string); it is composed of UINT16 elements. From the UEFI spec:

The BootOrder variable contains an array of UINT16’s that make up
an ordered list of the Boot options.

Second, can we please figure out a way for Windows using developers to
build edk2 with at least one version of gcc? These build failures are
*incredibly* annoying. The build breaks, someone submits a patch, the
reviewer lives on the other side of the planet, a day passes, review is
in, another day passes, the submitter commits it, okay, it builds again
now. Terrible.

I would absolutely test-build my edk2 patches with a VS release if one
was available to me at no cost. I could run it in a virtual machine. I
don't do it because I can't justify the $$$ to mgmt just for this. But
gcc binaries are available to Windows people at zero cost, and gcc's use
on Windows has been repeatedly discussed on this list. I find this state
desperate.

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

Re: [edk2] [PATCH v3] NetworkPkg: Add URI configuration form to HTTP boot driver.

[Wu, Jiaxin]

Reviewed-by: Jiaxin Wu 

> -Original Message-
> From: Fu, Siyuan
> Sent: Friday, March 4, 2016 4:38 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Jiaxin ; Ye, Ting 
> Subject: [PATCH v3] NetworkPkg: Add URI configuration form to HTTP boot
> driver.
> 
> This patch updates the HTTP boot driver to produce a setup page for the
> boot
> file URI configuration. A new boot option will be created for the manual
> configured URI address. This change is made to support the HTTP boot usage
> in home environment.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Fu Siyuan 
> Cc: Wu Jiaxin 
> Cc: Ye Ting 
> ---
>  NetworkPkg/HttpBootDxe/HttpBootClient.c|  99 +--
>  NetworkPkg/HttpBootDxe/HttpBootConfig.c| 723
> +
>  NetworkPkg/HttpBootDxe/HttpBootConfig.h|  78 +++
>  NetworkPkg/HttpBootDxe/HttpBootConfigNVDataStruc.h |  43 ++
>  NetworkPkg/HttpBootDxe/HttpBootConfigStrings.uni   | Bin 0 -> 2926 bytes
>  NetworkPkg/HttpBootDxe/HttpBootConfigVfr.vfr   |  53 ++
>  NetworkPkg/HttpBootDxe/HttpBootDhcp4.c | 111 +++-
>  NetworkPkg/HttpBootDxe/HttpBootDhcp4.h |   9 +-
>  NetworkPkg/HttpBootDxe/HttpBootDhcp6.c |   6 +-
>  NetworkPkg/HttpBootDxe/HttpBootDxe.c   |  44 +-
>  NetworkPkg/HttpBootDxe/HttpBootDxe.h   |  33 +-
>  NetworkPkg/HttpBootDxe/HttpBootDxe.inf |  17 +-
>  NetworkPkg/HttpBootDxe/HttpBootImpl.c  |  71 +-
>  NetworkPkg/HttpBootDxe/HttpBootSupport.c   |  63 ++
>  NetworkPkg/HttpBootDxe/HttpBootSupport.h   |  18 +
>  NetworkPkg/Include/Guid/HttpBootConfigHii.h|  25 +
>  NetworkPkg/NetworkPkg.dec  |   5 +-
>  17 files changed, 1288 insertions(+), 110 deletions(-)
>  create mode 100644 NetworkPkg/HttpBootDxe/HttpBootConfig.c
>  create mode 100644 NetworkPkg/HttpBootDxe/HttpBootConfig.h
>  create mode 100644
> NetworkPkg/HttpBootDxe/HttpBootConfigNVDataStruc.h
>  create mode 100644 NetworkPkg/HttpBootDxe/HttpBootConfigStrings.uni
>  create mode 100644 NetworkPkg/HttpBootDxe/HttpBootConfigVfr.vfr
>  create mode 100644 NetworkPkg/Include/Guid/HttpBootConfigHii.h
> 
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.c
> b/NetworkPkg/HttpBootDxe/HttpBootClient.c
> index 2ccac8c..aae4527 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootClient.c
> +++ b/NetworkPkg/HttpBootDxe/HttpBootClient.c
> @@ -168,18 +168,35 @@ HttpBootDhcp4ExtractUriInfo (
>// HttpOffer contains the boot file URL.
>//
>SelectOffer = >OfferBuffer[SelectIndex].Dhcp4;
> -  if ((SelectOffer->OfferType == HttpOfferTypeDhcpIpUri) || (SelectOffer-
> >OfferType == HttpOfferTypeDhcpNameUriDns)) {
> -HttpOffer = SelectOffer;
> +  if (Private->FilePathUri == NULL) {
> +//
> +// In Corporate environment, we need a HttpOffer.
> +//
> +if ((SelectOffer->OfferType == HttpOfferTypeDhcpIpUri) ||
> +(SelectOffer->OfferType == HttpOfferTypeDhcpIpUriDns) ||
> +(SelectOffer->OfferType == HttpOfferTypeDhcpNameUriDns)) {
> +  HttpOffer = SelectOffer;
> +} else {
> +  ASSERT (Private->SelectProxyType != HttpOfferTypeMax);
> +  ProxyIndex = Private->OfferIndex[Private->SelectProxyType][0];
> +  HttpOffer = >OfferBuffer[ProxyIndex].Dhcp4;
> +}
> +Private->BootFileUriParser = HttpOffer->UriParser;
> +Private->BootFileUri = (CHAR8*) HttpOffer-
> >OptList[HTTP_BOOT_DHCP4_TAG_INDEX_BOOTFILE]->Data;
>} else {
> -ASSERT (Private->SelectProxyType != HttpOfferTypeMax);
> -ProxyIndex = Private->OfferIndex[Private->SelectProxyType][0];
> -HttpOffer = >OfferBuffer[ProxyIndex].Dhcp4;
> +//
> +// In Home environment the BootFileUri comes from the FilePath.
> +//
> +Private->BootFileUriParser = Private->FilePathUriParser;
> +Private->BootFileUri = Private->FilePathUri;
>}
> 
>//
>// Configure the default DNS server if server assigned.
>//
> -  if ((SelectOffer->OfferType == HttpOfferTypeDhcpNameUriDns) ||
> (SelectOffer->OfferType == HttpOfferTypeDhcpDns)) {
> +  if ((SelectOffer->OfferType == HttpOfferTypeDhcpNameUriDns) ||
> +  (SelectOffer->OfferType == HttpOfferTypeDhcpDns) ||
> +  (SelectOffer->OfferType == HttpOfferTypeDhcpIpUriDns)) {
>  Option = SelectOffer-
> >OptList[HTTP_BOOT_DHCP4_TAG_INDEX_DNS_SERVER];
>  ASSERT (Option != NULL);
>  Status = HttpBootRegisterIp4Dns (
> @@ -196,8 +213,8 @@ HttpBootDhcp4ExtractUriInfo (
>// Extract the port from URL, and use default HTTP port 80 if not provided.
>//
>Status = HttpUrlGetPort (
> - (CHAR8*) HttpOffer-
> >OptList[HTTP_BOOT_DHCP4_TAG_INDEX_BOOTFILE]->Data,
> - HttpOffer->UriParser,
> + Private->BootFileUri,
> + Private->BootFileUriParser,
>   >Port
>   );
>if 

Re: [edk2] [PATCH v3] NetworkPkg: Add URI configuration form to HTTP boot driver.

[Ye, Ting]

Looks good to me.
Reviewed-by: Ye Ting <ting...@intel.com> 

-Original Message-
From: Fu, Siyuan 
Sent: Friday, March 04, 2016 4:40 PM
To: Fu, Siyuan <siyuan...@intel.com>; edk2-devel@lists.01.org
Cc: Ye, Ting <ting...@intel.com>; Wu, Jiaxin <jiaxin...@intel.com>
Subject: RE: [edk2] [PATCH v3] NetworkPkg: Add URI configuration form to HTTP 
boot driver.

V3 changes:
Update the driver to handle an empty URI device node.


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Fu
> Siyuan
> Sent: Friday, March 4, 2016 4:38 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting <ting...@intel.com>; Wu, Jiaxin <jiaxin...@intel.com>
> Subject: [edk2] [PATCH v3] NetworkPkg: Add URI configuration form to HTTP
> boot driver.
> 
> This patch updates the HTTP boot driver to produce a setup page for the boot
> file URI configuration. A new boot option will be created for the manual
> configured URI address. This change is made to support the HTTP boot usage
> in home environment.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Fu Siyuan <siyuan...@intel.com>
> Cc: Wu Jiaxin <jiaxin...@intel.com>
> Cc: Ye Ting <ting...@intel.com>
> ---
>  NetworkPkg/HttpBootDxe/HttpBootClient.c|  99 +--
>  NetworkPkg/HttpBootDxe/HttpBootConfig.c| 723
> +
>  NetworkPkg/HttpBootDxe/HttpBootConfig.h|  78 +++
>  NetworkPkg/HttpBootDxe/HttpBootConfigNVDataStruc.h |  43 ++
>  NetworkPkg/HttpBootDxe/HttpBootConfigStrings.uni   | Bin 0 -> 2926 bytes
>  NetworkPkg/HttpBootDxe/HttpBootConfigVfr.vfr   |  53 ++
>  NetworkPkg/HttpBootDxe/HttpBootDhcp4.c | 111 +++-
>  NetworkPkg/HttpBootDxe/HttpBootDhcp4.h |   9 +-
>  NetworkPkg/HttpBootDxe/HttpBootDhcp6.c |   6 +-
>  NetworkPkg/HttpBootDxe/HttpBootDxe.c   |  44 +-
>  NetworkPkg/HttpBootDxe/HttpBootDxe.h   |  33 +-
>  NetworkPkg/HttpBootDxe/HttpBootDxe.inf |  17 +-
>  NetworkPkg/HttpBootDxe/HttpBootImpl.c  |  71 +-
>  NetworkPkg/HttpBootDxe/HttpBootSupport.c   |  63 ++
>  NetworkPkg/HttpBootDxe/HttpBootSupport.h   |  18 +
>  NetworkPkg/Include/Guid/HttpBootConfigHii.h|  25 +
>  NetworkPkg/NetworkPkg.dec  |   5 +-
>  17 files changed, 1288 insertions(+), 110 deletions(-)
>  create mode 100644 NetworkPkg/HttpBootDxe/HttpBootConfig.c
>  create mode 100644 NetworkPkg/HttpBootDxe/HttpBootConfig.h
>  create mode 100644
> NetworkPkg/HttpBootDxe/HttpBootConfigNVDataStruc.h
>  create mode 100644 NetworkPkg/HttpBootDxe/HttpBootConfigStrings.uni
>  create mode 100644 NetworkPkg/HttpBootDxe/HttpBootConfigVfr.vfr
>  create mode 100644 NetworkPkg/Include/Guid/HttpBootConfigHii.h
> 
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.c
> b/NetworkPkg/HttpBootDxe/HttpBootClient.c
> index 2ccac8c..aae4527 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootClient.c
> +++ b/NetworkPkg/HttpBootDxe/HttpBootClient.c
> @@ -168,18 +168,35 @@ HttpBootDhcp4ExtractUriInfo (
>// HttpOffer contains the boot file URL.
>//
>SelectOffer = >OfferBuffer[SelectIndex].Dhcp4;
> -  if ((SelectOffer->OfferType == HttpOfferTypeDhcpIpUri) || (SelectOffer-
> >OfferType == HttpOfferTypeDhcpNameUriDns)) {
> -HttpOffer = SelectOffer;
> +  if (Private->FilePathUri == NULL) {
> +//
> +// In Corporate environment, we need a HttpOffer.
> +//
> +if ((SelectOffer->OfferType == HttpOfferTypeDhcpIpUri) ||
> +(SelectOffer->OfferType == HttpOfferTypeDhcpIpUriDns) ||
> +(SelectOffer->OfferType == HttpOfferTypeDhcpNameUriDns)) {
> +  HttpOffer = SelectOffer;
> +} else {
> +  ASSERT (Private->SelectProxyType != HttpOfferTypeMax);
> +  ProxyIndex = Private->OfferIndex[Private->SelectProxyType][0];
> +  HttpOffer = >OfferBuffer[ProxyIndex].Dhcp4;
> +}
> +Private->BootFileUriParser = HttpOffer->UriParser;
> +Private->BootFileUri = (CHAR8*) HttpOffer-
> >OptList[HTTP_BOOT_DHCP4_TAG_INDEX_BOOTFILE]->Data;
>} else {
> -ASSERT (Private->SelectProxyType != HttpOfferTypeMax);
> -ProxyIndex = Private->OfferIndex[Private->SelectProxyType][0];
> -HttpOffer = >OfferBuffer[ProxyIndex].Dhcp4;
> +//
> +// In Home environment the BootFileUri comes from the FilePath.
> +//
> +Private->BootFileUriParser = Private->FilePathUriParser;
> +Private->BootFileUri = Private->FilePathUri;
>}
> 
>//
>// Configure the default DNS server if server assigned.
>//
> -  if ((SelectOffer->OfferType == HttpOf