Date   

Re: Load Option passing. Either bugs or my confusion.

Laszlo Ersek
 

On 04/22/20 09:42, Hou Qiming wrote:
A little off topic thing: isn't the default resolution supposed to be
1024x768?
No.

This is the Microsoft regulation which all my physical devices
seem to follow:

https://docs.microsoft.com/en-us/windows-hardware/test/hlk/testref/6afc8979-df62-4d86-8f6a-99f05bbdc7f3
Key term being "Microsoft regulation".

The UEFI spec requires discrete ("plug-in") graphics devices to support
at least either 800x600x32 or 640x480x32.

And the edk2 (not just OVMF) default for the console resolution is
800x600. (See PcdVideoHorizontalResolution and
PcdVideoVerticalResolution in "MdeModulePkg/MdeModulePkg.dec".)

And when the user provides an EDID one should parse it and set the default
resolution to match it. But that's a less important feature.
It's more complex than you might think, and (to me personally) it seems
to require more time than its importance justifies.

https://bugzilla.redhat.com/show_bug.cgi?id=1749250

Laszlo


Re: Query: Add GCD IO Space Map

Brian J. Johnson
 

Wasim,

GCD I/O Space is intended for IA32/X64 I/O Port space: the legacy I/O ports used on PCs for accessing various bits of legacy hardware, such as the system reset register at port 0xcf9. Very little modern hardware uses it.

It looks like you're interested in adding memory-mapped I/O space. For that I believe you'd use gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo, 0x9000010000, 0x10000) or a resource descriptor HOB of type EFI_RESOURCE_MEMORY_MAPPED_IO.

Brian

-------- Original Message --------
From: Wasim Khan [mailto:wasim.khan@nxp.com]
Sent: Thursday, April 2, 2020, 11:24 PM
To: discuss@edk2.groups.io <discuss@edk2.groups.io>, ruiyu.ni@intel.com <ruiyu.ni@intel.com>, dandan.bi@intel.com <dandan.bi@intel.com>, liming.gao@intel.com <liming.gao@intel.com>, hao.a.wu@intel.com <hao.a.wu@intel.com>, leif.lindholm@linaro.org <leif.lindholm@linaro.org>
Subject: [edk2-discuss] Query: Add GCD IO Space Map

Hi ,

Any clue on this ?

Get Outlook for Android<https://aka.ms/ghei36
________________________________
From: discuss@edk2.groups.io <discuss@edk2.groups.io> on behalf of Wasim Khan via groups.io <wasim.khan=nxp.com@groups.io>
Sent: Friday, April 3, 2020 1:18:13 AM
To: discuss@edk2.groups.io <discuss@edk2.groups.io>; ruiyu.ni@intel.com <ruiyu.ni@intel.com>; dandan.bi@intel.com <dandan.bi@intel.com>; liming.gao@intel.com <liming.gao@intel.com>; hao.a.wu@intel.com <hao.a.wu@intel.com>
Subject: [edk2-discuss] Query: Add GCD IO Space Map

Hi All,

The Initial GCD I/O Space Map printed from DxeMain is :

GCD:Initial GCD I/O Space Map
GCDIoType Range
========== =================================
NonExist 0000000000000000-00000000000FFFFF


I require some IO memory (for PciHostBridgeDxe) , i need NonExist IO chuck from 0x90_00010000 - 0x90_0001ffff , for which I tried BuildResourceDescriptorHob() from PEI Phase

BuildResourceDescriptorHob (
EFI_RESOURCE_IO,
ResourceAttributes,
0x9000010000,
0x10000
);

And gDS->AddIoSpace (EfiGcdIoTypeIo, 0x9000010000, 0x10000) from a DXE driver . But in both case GCD AddIoSpace fails with Status as Unsupported.

GCD:AddIoSpace(Base=0000009000010000,Length=0000000000010000)
GcdIoType = I/O
Status = Unsupported
GCDIoType Range
========== =================================
NonExist 0000000000000000-00000000000FFFFF



Can someone help that how can I add a NonExist GCDIoType memory ?




Also, why does the initial GCD I/O space map shows small range as compared with GCD Memory Space Map?

GCD:Initial GCD Memory Space Map
GCDMemType Range Capabilities Attributes
========== ================================= ================ ================
NonExist 0000000000000000-00000FFFFFFFFFFF 0000000000000000 0000000000000000

GCD:Initial GCD I/O Space Map
GCDIoType Range
========== =================================
NonExist 0000000000000000-00000000000FFFFF


Regards,
Wasim











--
Brian J. Johnson
Enterprise X86 Lab

Hewlett Packard Enterprise


Re: EDK2 Build

Bob Feng
 

Hi Jim,

I think on Win10, you can use VS 2017 or other VS versions. You can refer to edk2\BaseTools\Conf\tools_def.template where list all supports toolchain.

You can also try Clang9 toolchain. Here is the introduction about Clang9
https://github.com/tianocore/tianocore.github.io/wiki/CLANG9-Tools-Chain

Thanks,
Bob

-----Original Message-----
From: discuss@edk2.groups.io <discuss@edk2.groups.io> On Behalf Of jim slaughter
Sent: Wednesday, April 22, 2020 7:14 AM
To: discuss@edk2.groups.io
Subject: [edk2-discuss] EDK2 Build

Hello,
I am using Win10. Need to build uEFI.
What tools do I use for compile, link, and other parts to build a binary image?
I suspect GNU but does GNU come in a Windows based set of tools?
It looks like uEFI does use the "Command Window" under WIN10 Any help would be appreciated.
--
Jim Slaughter


EDK2 Build

jim.slaughter@...
 

Hello,
I am using Win10. Need to build uEFI.
What tools do I use for compile, link, and other parts to build a binary
image?
I suspect GNU but does GNU come in a Windows based set of tools?
It looks like uEFI does use the "Command Window" under WIN10
Any help would be appreciated.
--
Jim Slaughter


Re: Load Option passing. Either bugs or my confusion.

Laszlo Ersek
 

On 04/20/20 16:13, Gerd Hoffmann wrote:
Hi,

So I would say that the symptom you see is a QEMU v4.1.0 regression.
The QemuRamfbGraphicsOutputSetMode() function in the OVMF ramfb
driver certainly needs the QemuFwCfgWriteBytes() call to work, for
changing the resolution.
Oh? QemuRamfbGraphicsOutputSetMode() can be called multiple times?
How does that happen?
QemuRamfbGraphicsOutputSetMode() is the "SetMode" member function of the
EFI_GRAPHICS_OUTPUT_PROTOCOL instance that QemuRamfbDxe produces.

This is a standard protocol; UEFI drivers and applications are free to
locate it and to use it.

(1) When you launch OVMF, you get the splash screen in a particular
resolution. This resolution:
- is configured by OvmfPkg/PlatformDxe,
- is inherited by an OS boot loader,
- is reconfigurable with OvmfPkg/PlatformDxe, for the next boot, via the
Setup TUI,
- defaults to 800x600 (taking effect when no particular choice is
configured).

(2) UiApp -- the Setup TUI itself -- uses its own resolution. Under
OVMF, this resolution is fixed 640x480. When UiApp is entered,
ultimately a call is made to QemuRamfbGraphicsOutputSetMode() -- i.e., a
GOP.SetMode() member function -- for setting this 640x480 resolution.

Using the following command:

qemu-system-x86_64 \
-nodefaults \
-boot menu=on,splash-time=5000 \
-enable-kvm \
-device ramfb \
-drive if=pflash,readonly,format=raw,file=$PREFIX/share/qemu/edk2-x86_64-code.fd \
-drive if=pflash,snapshot,format=raw,file=$PREFIX/share/qemu/edk2-i386-vars.fd \
-debugcon file:ovmf.log \
-global isa-debugcon.iobase=0x402

when you first see the progress bar, the graphical resolution (1) is
800x600. Accordingly, QEMU prints to stderr:

ramfb_fw_cfg_write: 800x600 @ 0x6702000
Once you hit ESC to interrupt the progress bar and to enter the Setup
TUI, UiApp switches to resolution (2), 640x480. QEMU prints:

ramfb_fw_cfg_write: 640x480 @ 0x6702000
ramfb_fw_cfg_write: resolution locked, change rejected
And you get garbage in the Setup window.

Thanks,
Laszlo


Re: Load Option passing. Either bugs or my confusion.

Laszlo Ersek
 

On 04/17/20 05:22, Hou Qiming wrote:
I'm glad we can reach a consensus that ramfb needs sanity checks. And well,
I'm probably at fault with the hijacking.

Your QEMU/TCG in QEMU/TCG example also made me realize a deeper problem,
though: your setting still can't escape the host display / physical GPU
issue. The middle display layers be bochs or whatever, but as long as the
framebuffer content and resolution values are propagated, and the end
result is displayed at all on the host, the host GPU attack surface remains
exposed to the L2 guest, and checks are needed. Everything shown on the
screen involves the display driver - GPU stack, GTK or SDL or tty, you
can't avoid that. ramfb-kvmgt just happened to be the shortest pipeline
where every stage neglected the checks, which exposed this problem.
Good point.

Blaming
this on ramfb is unfair since in your scenario the checks are better done
in the display subsystems.

TL;DR You made me realize right now, it's a very real risk that an AARCH64
Windows guest could exploit a x64 host's display driver by specifying a
crafted framebuffer with overflowing resolution. I don't want to break it,
but I'd prefer a broken state over an insecure state.

I'm not quite sure what this thread is. But I think with the scope this
discussion is going, maybe it's more of a bug than a regression.
All display devices (frontends) emulated by QEMU have to set bounds for
the permissible resolutions, for the guest. And those limits must never
break the capabilities of the display backends. So this is not a new
problem. How is it handled with other frontends? Like bochs-display, for
example -- I assume bochs-display too is purely virtual, i.e. the
resolution is fully controller (between bounds) by the guest. How is the
guest currently prevented from setting a bochs-display resolution that
"breaks SDL" (whatever that means)?

I'm inclined to agree that we're just seeing two sides of the same bug
-- the first state was too lax, and the current state is too strict.

Thanks
Laszlo


Re: Load Option passing. Either bugs or my confusion.

Laszlo Ersek
 

On 04/16/20 06:38, Hou Qiming wrote:
Very good point, I did neglect ramfb resolution changes... But there is one
important thing: it *can* cause a QEMU crash, a potentially exploitable
one, not always a guest crash. That's what motivated my heavy-handed
approach since allowing resolution change would have necessitated a good
deal of security checks. It has crashed my host *kernel* quite a few times.

The point is, while the QemuRamfbDxe driver may behave properly, nothing
prevents the guest from writing garbage or *malicious* values to the ramfb
config space. Then the values are sent to the display component without any
sanity check. For some GUI frontends, this means allocating an OpenGL
texture with guest-supplied dimensions and uploading guest memory content
to it, which means that guest memory content goes straight into a *kernel
driver*, *completely unchecked*. Some integer overflow and a lenient GPU
driver later, and the guest escapes straight to kernel.

The proper way to enable ramfb resolution change again is adding sanity
checks for ramfb resolution / pointer / etc. on the QEMU side. We have to
make sure it doesn't exceed what the host GPU driver supports. Maybe clamp
both width and height to between 1 and 2048? We also need to validate that
OpenGL texture dimension update succeeds. Note that OpenGL is not obliged
to validate anything and everything has to be checked on the QEMU side.
I agree that QEMU should sanity check the resolution requested by the
guest. I also agree that "arbitrary" limits are acceptable, for
preventing integer overflows and -- hopefully -- memory allocation
failures too.

But I don't see the host kernel / OpenGL / physical GPU angle, at least
not directly. That angle seems to be specific to your particular use
case (particular choice of display backend).

For example, if you nest QEMU/TCG in QEMU/TCG, with no KVM and no device
assignment in the picture anywhere, and OVMF drives ramfb in L2, and the
display *backend* (such as GTK or SDL GUI window) for the QEMU process
running in L1 sits on top of a virtual device (such as bochs-display)
provided by QEMU running in L0, then the ramfb stuff (including the
resolution changes and the range checks) should work just the same,
between L2 and L1.

I kinda feel like ramfb has been hijacked for providing a boot time
display crutch for kvmgt. (I might not be using the correct terminology
here; sorry about that). That's *not* what ramfb was originally intended
for, as far as I recall. Compare:

- 59926de9987c ("Merge remote-tracking branch
'remotes/kraxel/tags/vga-20180618-pull-request' into staging", 2018-06-19)

- dddb37495b84 ("Merge remote-tracking branch
'remotes/awilliam/tags/vfio-updates-20181015.0' into staging", 2018-10-15)

IIRC, Gerd originally invented ramfb for giving AARCH64 Windows the
linear framebuffer that the latter so badly wants, in particular so that
the framebuffer exist in guest RAM (not in guest MMIO), in order to
avoid the annoying S1/S2 caching behavior of AARCH64/KVM when the guest
maps an area as MMIO that is mapped as RAM on the host [1]. See:

- https://bugzilla.tianocore.org/show_bug.cgi?id=785#c4
- https://bugzilla.tianocore.org/show_bug.cgi?id=785#c7
- https://bugzilla.tianocore.org/show_bug.cgi?id=785#c8

and the further references given in those bugzilla comments.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1679680#c0

Component reuse is obviously *hugely* important, and it would be silly
for me to argue against reusing ramfb wherever it applies. Just please
don't break the original use case.

Should I file a bug report in LaunchPad, or is this thread enough for
tracking the QEMU regression?

Thanks
Laszlo


Qiming


On Wed, Apr 15, 2020 at 11:05 PM Laszlo Ersek <lersek@redhat.com> wrote:

(CC Gerd, Qiming, Marcel, qemu-devel for ramfb:)

On 04/14/20 23:20, valerij zaporogeci wrote:

[snip]

There is a Boot Manager UI display problem, I don't know if this is
qemu problem, but with the ARM (both 32 and 64 bits, the qemu version
is 4.2.0, the OVMF is fresh), and using "ramfb" device, the Boot
Manager has troubles with drawing - it's interfase looks entirely
broken, like this (I'll try to attach the screenshot). UEFI shell
doesn't have this problem. switching to "serial" (which is -serial vc)
doesn't produce it too. Only when ramfb is chosen, the Boot Manager UI
gets smeared. But it takes input and presumable works properly, except
displaying things. qemu writes these messages in the command prompt:
ramfb_fw_cfg_write: 640x480 @ 0x4bd00000
ramfb_fw_cfg_write: resolution locked, change rejected
ramfb_fw_cfg_write: 800x600 @ 0x4bd00000
ramfb_fw_cfg_write: resolution locked, change rejected
Gerd contributed the OVMF QemuRamfbDxe driver in edk2 commit
1d25ff51af5c ("OvmfPkg: add QemuRamfbDxe", 2018-06-14). Note the date:
June 2018.

The then-latest (released) QEMU version was v2.12.0, and v2.12.1 /
v3.0.0 were in the making.

At that time, the resolution change definitely worked -- note my
"Tested-by" on the edk2 commit message.


Running "git blame" on the QEMU source, I now find commit a9e0cb67b7f4
("hw/display/ramfb: lock guest resolution after it's set", 2019-05-24).

Again, note the date: May 2019 (and this commit was released with QEMU
v4.1.0).

So I would say that the symptom you see is a QEMU v4.1.0 regression. The
QemuRamfbGraphicsOutputSetMode() function in the OVMF ramfb driver
certainly needs the QemuFwCfgWriteBytes() call to work, for changing the
resolution.


Now, I'm not familiar with the reasons behind QEMU commit a9e0cb67b7f4.
It says it intends to "prevent[] a crash when the guest writes garbage
to the configuration space (e.g. when rebooting)".

But I don't understand why locking the resolution was necessary for
preventing "a crash":

(1) Registering a device reset handler in QEMU seems sufficient, so that
QEMU forget about the currently shared RAMFB area at platform reset.

(2) The crash in question is apparently not a *QEMU* crash -- which
might otherwise justify a heavy-handed approach. Instead, it is a
*guest* crash. See the references below:

(2a)
http://mid.mail-archive.com/CABSdmrmU7FK90Bupq_ySowcc9Uk=8nQxNLHgzvDsNYdp_QLogA@mail.gmail.com
https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg02299.html

(2b) https://github.com/intel/gvt-linux/issues/23#issuecomment-483651476

Therefore, I don't think that locking the resolution was justified!

Importantly:

- The QemuRamfbDxe driver allocates the framebuffer in *reserved*
memory, therefore any well-behaving OS will *never* touch the
framebuffer.

- The QemuRamfbDxe driver allocates the framebuffer memory only once,
namely for such a resolution that needs the largest amount of
framebuffer memory. Therefore, framebuffer re-allocations in the guest
driver -- and thereby guest RAM *re-mapping* in QEMU -- are *not*
necessary, upon resolution change.

The ramfb device reset handler in QEMU is justified (for unmapping /
forgetting the previously shared RAMFB area).

The resolution locking is *NOT* justified, and it breaks the OVMF
driver. I suggest backing out the resolution locking from QEMU.

Reference (2a) above indicates 'It could be a misguided attempt to
"resize ramfb" by the guest Intel driver'. If that is the case, then
please fix the Intel guest driver, without regressing the QEMU device
model.

I'm sad that the QEMU device model change was not regression-tested
against the *upstream* OVMF driver (which, by then, had been upstream
for almost a year).

Laszlo


Re: Load Option passing. Either bugs or my confusion.

Laszlo Ersek
 

(CC Gerd, Qiming, Marcel, qemu-devel for ramfb:)

On 04/14/20 23:20, valerij zaporogeci wrote:

[snip]

There is a Boot Manager UI display problem, I don't know if this is
qemu problem, but with the ARM (both 32 and 64 bits, the qemu version
is 4.2.0, the OVMF is fresh), and using "ramfb" device, the Boot
Manager has troubles with drawing - it's interfase looks entirely
broken, like this (I'll try to attach the screenshot). UEFI shell
doesn't have this problem. switching to "serial" (which is -serial vc)
doesn't produce it too. Only when ramfb is chosen, the Boot Manager UI
gets smeared. But it takes input and presumable works properly, except
displaying things. qemu writes these messages in the command prompt:
ramfb_fw_cfg_write: 640x480 @ 0x4bd00000
ramfb_fw_cfg_write: resolution locked, change rejected
ramfb_fw_cfg_write: 800x600 @ 0x4bd00000
ramfb_fw_cfg_write: resolution locked, change rejected
Gerd contributed the OVMF QemuRamfbDxe driver in edk2 commit
1d25ff51af5c ("OvmfPkg: add QemuRamfbDxe", 2018-06-14). Note the date:
June 2018.

The then-latest (released) QEMU version was v2.12.0, and v2.12.1 /
v3.0.0 were in the making.

At that time, the resolution change definitely worked -- note my
"Tested-by" on the edk2 commit message.


Running "git blame" on the QEMU source, I now find commit a9e0cb67b7f4
("hw/display/ramfb: lock guest resolution after it's set", 2019-05-24).

Again, note the date: May 2019 (and this commit was released with QEMU
v4.1.0).

So I would say that the symptom you see is a QEMU v4.1.0 regression. The
QemuRamfbGraphicsOutputSetMode() function in the OVMF ramfb driver
certainly needs the QemuFwCfgWriteBytes() call to work, for changing the
resolution.


Now, I'm not familiar with the reasons behind QEMU commit a9e0cb67b7f4.
It says it intends to "prevent[] a crash when the guest writes garbage
to the configuration space (e.g. when rebooting)".

But I don't understand why locking the resolution was necessary for
preventing "a crash":

(1) Registering a device reset handler in QEMU seems sufficient, so that
QEMU forget about the currently shared RAMFB area at platform reset.

(2) The crash in question is apparently not a *QEMU* crash -- which
might otherwise justify a heavy-handed approach. Instead, it is a
*guest* crash. See the references below:

(2a) http://mid.mail-archive.com/CABSdmrmU7FK90Bupq_ySowcc9Uk=8nQxNLHgzvDsNYdp_QLogA@mail.gmail.com
https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg02299.html

(2b) https://github.com/intel/gvt-linux/issues/23#issuecomment-483651476

Therefore, I don't think that locking the resolution was justified!

Importantly:

- The QemuRamfbDxe driver allocates the framebuffer in *reserved*
memory, therefore any well-behaving OS will *never* touch the
framebuffer.

- The QemuRamfbDxe driver allocates the framebuffer memory only once,
namely for such a resolution that needs the largest amount of
framebuffer memory. Therefore, framebuffer re-allocations in the guest
driver -- and thereby guest RAM *re-mapping* in QEMU -- are *not*
necessary, upon resolution change.

The ramfb device reset handler in QEMU is justified (for unmapping /
forgetting the previously shared RAMFB area).

The resolution locking is *NOT* justified, and it breaks the OVMF
driver. I suggest backing out the resolution locking from QEMU.

Reference (2a) above indicates 'It could be a misguided attempt to
"resize ramfb" by the guest Intel driver'. If that is the case, then
please fix the Intel guest driver, without regressing the QEMU device
model.

I'm sad that the QEMU device model change was not regression-tested
against the *upstream* OVMF driver (which, by then, had been upstream
for almost a year).

Laszlo


Re: Load Option passing. Either bugs or my confusion.

valerij zaporogeci
 

Thank you, Laszlo! Again, sorry for the impatience, I started to suspect, my posts aren't visible.

Yes, I mistook the LoadOptions field of the Loaded Image Protocol for the one, describing image's EFI_LOAD_OPTION descriptor. because, well, "load options" is a term in the spec and I thought, the field named as such is about that entity. Later on, when I looked at the LoadImage() service description, I got it's not about Load Option Descriptor.

Either way, I don't think those "elements" are passed to the loaded
image in any direct way. You could perhaps get back at the
EFI_LOAD_OPTION once the loaded image is running with the help of the
"BootCurrent" standard UEFI variable.
This is exactly what I need! The way of knowing the current, chosen load option and ability of getting its data. Thanks for your explanation.

Yet one thing, I know you are occupied with OVMF, right? There is a Boot Manager UI display problem, I don't know if this is qemu problem, but with the ARM (both 32 and 64 bits, the qemu version is 4.2.0, the OVMF is fresh), and using "ramfb" device, the Boot Manager has troubles with drawing - it's interfase looks entirely broken, like this (I'll try to attach the screenshot). UEFI shell doesn't have this problem. switching to "serial" (which is -serial vc) doesn't produce it too. Only when ramfb is chosen, the Boot Manager UI gets smeared. But it takes input and presumable works properly, except displaying things. qemu writes these messages in the command prompt:
ramfb_fw_cfg_write: 640x480 @ 0x4bd00000
ramfb_fw_cfg_write: resolution locked, change rejected
ramfb_fw_cfg_write: 800x600 @ 0x4bd00000
ramfb_fw_cfg_write: resolution locked, change rejected

Thank you. :)


Re: Load Option passing. Either bugs or my confusion.

Laszlo Ersek
 

On 04/14/20 02:44, valerij zaporogeci wrote:

1. what this pointer (OS Loader
ImageHandle)->(LOADED_IMAGE_PROTOCOL)->LoadOptions points to?
According to the UEFI spec <https://uefi.org/specifications>, section
"9.1 EFI Loaded Image Protocol":

LoadOptionsSize The size in bytes of LoadOptions.
LoadOptions A pointer to the image's binary load options.

[...]

Each loaded image has an image handle that supports
EFI_LOADED_IMAGE_PROTOCOL. When an image is started, it is passed the
image handle for itself. The image can use the handle to obtain its
relevant image data stored in the EFI_LOADED_IMAGE_PROTOCOL structure,
such as its load options.

And from section "7.4 Image Services", near the LoadImage() boot
service:

Once the image is loaded, firmware creates and returns an EFI_HANDLE
that identifies the image and supports EFI_LOADED_IMAGE_PROTOCOL and
the EFI_LOADED_IMAGE_DEVICE_PATH_PROTOCOL. The caller may fill in the
image's "load options" data, or add additional protocol support to the
handle.

So an agent calls the LoadImage() boot service to load the image. Then
it looks up the EFI_LOADED_IMAGE_PROTOCOL instance on the handle that
was output by LoadImage(). The agent populates LoadOptionsSize and
LoadOptions in said EFI_LOADED_IMAGE_PROTOCOL instance, and then calls
StartImage(). The started image can then consume LoadOptionsSize and
LoadOptions.

I think you mistook these fields for describing an EFI_LOAD_OPTION
structure (from section "3.1.3 Load Options"). EFI_LOAD_OPTION is a
different thing -- the EFI_LOADED_IMAGE_PROTOCOL.LoadOptions field
points to a binary blob whose interpretation is specific to the image
being started. Its internals are not regulated by the spec.

More below:


if it points to the Load Option Descriptor, then on OVMF it doesn't,
since it points to only OptionalData[].
if it was meant to point to OptionalData[], then:

1. how does an OS Loader get _its_ Load Option FilePathList[]? ALL
elements.
and
Again, EFI_LOADED_IMAGE_PROTOCOL.LoadOptions does not point to an
EFI_LOAD_OPTION structure.

Still, your question #1 makes sense; indeed an OS boot loader (or
another UEFI app) is "entitled" to see the UEFI device path from which
it was loaded from. The passage I quoted above already mentions
EFI_LOADED_IMAGE_DEVICE_PATH_PROTOCOL. It is specified in detail in
section "9.2 EFI Loaded Image Device Path Protocol". Excerpt:

The Loaded Image Device Path Protocol must be installed onto the image
handle of a PE/COFF image loaded through the EFI Boot Service
LoadImage(). A copy of the device path specified by the DevicePath
parameter to the EFI Boot Service LoadImage() is made before it is
installed onto the image handle. It is legal to call LoadImage() for a
buffer in memory with a NULL DevicePath parameter. In this case, the
Loaded Image Device Path Protocol is installed with a NULL interface
pointer.

Summary:

- Passing arguments to a UEFI image: locate EFI_LOADED_IMAGE_PROTOCOL on
the image handle, and populate the LoadOptions / LoadOptionsSize
members. The "arguments" are a binary blob, only defined by the
receiving application.

- Finding where a UEFI image was loaded from: locate
EFI_LOADED_IMAGE_DEVICE_PATH_PROTOCOL on the image handle.

- Regarding "EFI_LOAD_OPTION.OptionalData": in case you boot a Boot####
option, it is "EFI_LOAD_OPTION.OptionalData" that will be passed to the
image in "EFI_LOADED_IMAGE_PROTOCOL.LoadOptions".

More below:

2. should FilePathList[] be a multiinstance Device Path (ending with
type 7F, subtype 1, except the last), or every element should end with
{7F, FF}?
I cannot answer this. The spec writes, about "FilePathList":

A packed array of UEFI device paths. The first element of the array is
a device path that describes the device and location of the Image for
this load option. The FilePathList[0] is specific to the device type.
Other device paths may optionally exist in the FilePathList, but their
usage is OSV specific. Each element in the array is variable length,
and ends at the device path end structure. Because the size of
Description is arbitrary, this data structure is not guaranteed to be
aligned on a natural boundary. This data structure may have to be
copied to an aligned natural boundary before it is used.

I'm not sure if the "non-first" elements in this array are supposed to
be 2nd and later devpath instances in a single multi-instance device
path, or if they are entirely separate (themselves single or
multi-instance) device paths.

Either way, I don't think those "elements" are passed to the loaded
image in any direct way. You could perhaps get back at the
EFI_LOAD_OPTION once the loaded image is running with the help of the
"BootCurrent" standard UEFI variable.

Thanks
Laszlo


Re: Load Option passing. Either bugs or my confusion.

Laszlo Ersek
 

On 04/14/20 02:44, valerij zaporogeci wrote:
excuse me my impatience, but is this thread visible at all? I can't believe noone in here could answer my simple questions about such a core functionality. maybe it's because I was too verbose, let's try terse.
You posted

[edk2-devel] Bugs when starting an UEFI application from the shell. OVMF, UEFI shell.

on April 9th (in my time zone: a few minutes before April 10th), and started this thread (on edk2-discuss) on April 11th.

In many countries, April 10th and 13th are public holidays. And the 11th and 12th between them fall on a weekend. Put more simply, these days qualify as "Easter long weekend" in many countries.

I plan to look at your question later.

Laszlo


Re: Load Option passing. Either bugs or my confusion.

valerij zaporogeci
 

excuse me my impatience, but is this thread visible at all? I can't believe noone in here could answer my simple questions about such a core functionality. maybe it's because I was too verbose, let's try terse.

1. what this pointer (OS Loader ImageHandle)->(LOADED_IMAGE_PROTOCOL)->LoadOptions points to?

if it points to the Load Option Descriptor, then on OVMF it doesn't, since it points to only OptionalData[].
if it was meant to point to OptionalData[], then:

1. how does an OS Loader get _its_ Load Option FilePathList[]? ALL elements.
and
2. should FilePathList[] be a multiinstance Device Path (ending with type 7F, subtype 1, except the last), or every element should end with {7F, FF}?


Re: Reconnect item in form browser

Li, Walon
 

Attach demo code again.


Reconnect item in form browser

Li, Walon
 

Attachment removed as per the following specifications:
https://hpe.sharepoint.com/sites/F4/OGC/cybersecurity/Pages/dangerous-attachment-blocking-specification.aspx

======================================================================
Hi edk2 guys,

I have a question about edk2-formbrowser.
According to UEFI 2.8 spec p.1858, system reconnects controller when EFI_IFR_FLAG_RESET_REQUIRED is set. The timing of reconnecting is exiting formset which would happen with moving to different driver page normally.
However, it may happen with form-callback, some driver's callback would update form inside. It sets UI_ACTION_REFRESH_FORMSET and driver will be reconnected as above mention. After reconnecting driver, this is a new driver for BIOS so system can't back to original destination page (Dynamic Form) and then back to its parent page (Device Manager).
This issue is reproducible on edk2-emulater as well but I'm not sure that is valid coding convention or edk2 implementation bug. Could you check my demo code (HiiFormTestDxe_0406.zip) for helping?

Reproducible step.
1. Press F2 to enter Setup.
2. Go to Device Manager => Form Test Formset
3-1. Go to Dynamic Form, can see "Dynamic Item1" => PASS
3-2. Change "Item 1 - RECONNECT" to "Option 2". Press F10 for save. Go to Dynamic Form, system will return to Device Manager => FAIL

Thanks,
Walon


Re: Load Option passing. Either bugs or my confusion.

valerij zaporogeci
 

Now looking closer to the supposed to be Attributes and FilePathListLength fileds of the Load Option descriptor, I noticed that it's just first 3 characters of the string. the string that with the Boot Manager case, you create as an "optional data" and with the UEFI shell, it's image file path string (that's why it started with a colon ":", because "fs0" was omitted because of thinking it's a descriptor and thus first 3 16 bit words got omitted (UINT32 + UINT16)).

So, the LoadOptions pointer of the Loaded Image Protocol doesn't point to Load Options. Is it normal?

And then the question arises, - if this is "normal", then how does the loader obtain its real Load Option? Yeah, the device from which it has been loaded is reported by another field, the file path part to the loader itself as well, but how about FilePathList[1] element, - "an OSV specific" and needed. for pointing to the OS boot volume e.g.?


Re: Load Option passing. Either bugs or my confusion.

valerij zaporogeci
 

anyone, please?

The more I try to figure it out, the more it reveals that it's a bug. After that unsuccess with the LoadOptions in the image's Loaded Image Protocol instance, I went and enumerated BootXXXX variables, dumping their Attributes, FilePathListLength, Description and FilePathList[] itself. And this way, it worked - the load option has all those descriptor fields valid and the FilePathList is there and valid. So now, in comparison, when I take a Load Option through the Loaded Image Protocol LoadOptions field, I get:
1) wrong Attributes
2) invalid FilePathListLength (since FilePath is not present at all)
3) Description field has not the human readable name of the option (say "My OS") as is with when you get the option through GetValue() but what Boot Mahager suggests to enter as "optional data". btw, when these data are omitted during the Load Option creation, then the LoadOptions field in the Loaded Image Protocol is NULL at all, see the first post.
4) FilePathList[] is absent
again, when I get the same option by enumerating all the created BootXXXX variables with the GetValue(), all the fields are valid.

So, please tell me, is the LoadOptions pointer of the Loaded Image Protocol instance of the OSL image supposed to point to the Load Option descriptor in the format described in the Boot Manager section? If yes, then this behavior described is clearly a bug and it's not only important for me to figure this out, but for the edk2 as well.


Load Option passing. Either bugs or my confusion.

valerij zaporogeci
 

Hello. Do I understand correctly, that LoadOptions field of the LOADED_IMAGE_PROTOCOL instance of an OS loader image, points to the Load Option descriptor, the format of which is described in the Boot Manager section of the spec? If yes, then I faced a behavior, that looks like bugs. It's on OVMF x64 (qemu) (and arm64 too, but only from the UEFI shell, since "ramfb" device shows broken view of what Boot Manager draws, that makes it unusable).
1) when I start my OSL from the UEFI shell, the LoadOptions field is not NULL, but the "load option" referenced, has only Description field valid, Attributes field doesn't seem valid, it contains 0x00730066. Most importantly, - FilePathListLength field is set to a nonzero value (48), but, - there is no valid FilePathList[] Device Path array after the Decsription field. Scanning memory there, reveals all zeros. Isn't it a bug?
2) when I create a Load Option in the Boot Manager, pointing to the same OSL and don't add "optional data" during the creation, the LoadOptions field is NULL. Why? How my OSL is supposed to get FilePathList[] then? For distinction between preinstallation run and postinstallation one, the OSL relies on the presence or absence of the Load Option or if there is one, - on the presence or absence FilePathList[1] Device Path, which points to the OS boot volume for postinstallation or is absent otherwise. But I cannot find FilePathList at all.
3) If I create a Load Option as in 2), but with "optional data", LoadOptions is not NULL, those "optional data" turn to be a "Description" field of the Load Option descriptor reported, and still, there is no FilePathList.
In all the above cases, where LoadOptions is not NULL, LoadOptionsSize equals exactly sizeof(Attributes) + sizeof(FilePathListLength) + sizeof(Description) + 2. the latter is NULL CHAR16. there is no FilePathList. Still FilePathListLength field reports some values.

Is it normal or it's bugs? Plus, there is yet one wierdness: DeviceHandle field (of the same LOADED_IMAGE_PROTOCOL instance for the OSL), which is a device handle (hard drive in this case - Type 4, Subtype - 1), doesn't have EFI_DEVICE_PATH_TO_TEXT_PROTOCOL instance on itself. but shouldn't it have it?

The main question is why there is no FilePathList[] part of the Load Option passed to the OSL in any scenario of its launch? It should be there at least for the case of the Load Option, created via the Boot Manager menu, right?


Re: Submit patch to devel@edk2.groups.io.

Keysound Chang
 

Hi Guomin,

Looks like it works. Thanks for your friendly reminder.

Keysound

-----Original Message-----
From: Jiang, Guomin <guomin.jiang@intel.com>
Sent: Thursday, April 9, 2020 3:50 PM
To: discuss@edk2.groups.io; Keysound Chang <Keysound_Chang@phoenix.com>
Subject: RE: [edk2-discuss] Submit patch to devel@edk2.groups.io.

Hi keysound,

I have ever encountered the issue.

The reason is that you are the NuM(New user Moderated). you will need be approved by moderator and then you can see you mail normally.
Please be patient for the process.

Best Regards
guomin
-----Original Message-----
From: discuss@edk2.groups.io <discuss@edk2.groups.io> On Behalf Of
Keysound Chang
Sent: Thursday, April 9, 2020 3:20 PM
To: discuss@edk2.groups.io
Subject: [edk2-discuss] Submit patch to devel@edk2.groups.io.

Hi,

I encountered an issue when trying to send a patch mail to
devel@edk2.groups.io.

The SMTP server returned 250 after I issue sendmail through git, and I
also received cc. But I didn't see my post appeared on edk2-devel. I
also checked
this:

https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-
Format

Looks like my format is appropriate.

I append my mail here. Your advice/suggestion will be greatly appreciated.

Keysound
==========================================================
=

Subject: [PATCH] NetworkPkg/TlsAuthConfigDxe: Use HiiPopUp() instead
of
CreatePopUp()

From: Keysound Chang <Keysound_Chang@phoenix.com>

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2635

According to EDK2 Driver Writer's Guide For UEFI 2.3.1, 4.2.18 Offer
alternatives to function keys. Configuration of drivers should be
accomplished via HII and via OS-present interfaces.

Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Signed-off-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
---
.../TlsAuthConfigDxe/TlsAuthConfigDxe.inf | 1 +
.../TlsAuthConfigDxeStrings.uni | 2 ++
.../TlsAuthConfigDxe/TlsAuthConfigImpl.c | 20 +++++++++++++------
.../TlsAuthConfigDxe/TlsAuthConfigImpl.h | 1 +
4 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
index 3fc924a1d4..0ada835252 100644
--- a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
+++ b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
@@ -52,6 +52,7 @@
[Protocols] gEfiDevicePathProtocolGuid ## PRODUCES
gEfiHiiConfigAccessProtocolGuid ## PRODUCES+
gEfiHiiPopupProtocolGuid ## CONSUMES [Guids]
gTlsAuthConfigGuid ## PRODUCES ## GUIDdiff --git
a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxeStrings.uni
b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxeStrings.uni
index 973b8b7716..2b4a27d24e 100644
--- a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxeStrings.uni
+++ b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxeStrings.uni
@@ -28,6 +28,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#string STR_TLS_AUTH_CONFIG_SAVE_AND_EXIT #language en-US
"Commit Changes and Exit" #string
STR_TLS_AUTH_CONFIG_NO_SAVE_AND_EXIT #language en-US
"Discard Changes and Exit" +#string STR_TLS_AUTH_ENROLL_CERT_FAILURE
#language en-US "Enroll Cert Failure!"+ #string
STR_CERT_TYPE_PCKS_GUID #language en-US "GUID for CERT" #string
STR_NULL #language en-US ""diff --git
a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
index 2481d1098f..0ef96dfaf2 100644
--- a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
+++ b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
@@ -1383,7 +1383,6 @@ TlsAuthConfigAccessCallback (
OUT EFI_BROWSER_ACTION_REQUEST *ActionRequest ) {-
EFI_INPUT_KEY Key; EFI_STATUS Status;
RETURN_STATUS RStatus; TLS_AUTH_CONFIG_PRIVATE_DATA
*Private;@@ -1391,6 +1390,8 @@ TlsAuthConfigAccessCallback (
TLS_AUTH_CONFIG_IFR_NVDATA *IfrNvData; UINT16
LabelId; EFI_DEVICE_PATH_PROTOCOL *File;+
EFI_HII_POPUP_PROTOCOL *HiiPopUp;+ EFI_HII_POPUP_SELECTION
PopUpSelect; Status = EFI_SUCCESS; File = NULL;@@ -1402,6
+1403,11 @@ TlsAuthConfigAccessCallback (
Private = TLS_AUTH_CONFIG_PRIVATE_FROM_THIS (This);
mTlsAuthPrivateData = Private;+ Status = gBS->LocateProtocol
(&gEfiHiiPopupProtocolGuid, NULL, &HiiPopUp);+ if (EFI_ERROR
(Status)) {+ DEBUG ((DEBUG_ERROR, "Can't find Form PopUp protocol. Exit (%r)\n",
Status));+ return Status;+ } // // Retrieve uncommitted data from
Browser@@ -1460,11 +1466,13 @@ TlsAuthConfigAccessCallback (
if (EFI_ERROR (Status)) { CleanFileContext (Private); -
CreatePopUp (- EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,-
&Key,- L"ERROR: Enroll Cert Failure!",- NULL+ HiiPopUp-
CreatePopup(+ HiiPopUp,+ EfiHiiPopupStyleError,+
EfiHiiPopupTypeOk,+ Private->RegisteredHandle,+
STRING_TOKEN(STR_TLS_AUTH_ENROLL_CERT_FAILURE),+
&PopUpSelect ); } break;diff --git
a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.h
b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.h
index e9af492893..68c16845b3 100644
--- a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.h
+++ b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.h
@@ -14,6 +14,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Protocol/HiiConfigAccess.h> #include
<Protocol/SimpleFileSystem.h>+#include <Protocol/HiiPopup.h> // //
Libraries--
2.23.0.windows.1


Re: Submit patch to devel@edk2.groups.io.

Guomin Jiang
 

Hi keysound,

I have ever encountered the issue.

The reason is that you are the NuM(New user Moderated). you will need be approved by moderator and then you can see you mail normally.
Please be patient for the process.

Best Regards
guomin

-----Original Message-----
From: discuss@edk2.groups.io <discuss@edk2.groups.io> On Behalf Of
Keysound Chang
Sent: Thursday, April 9, 2020 3:20 PM
To: discuss@edk2.groups.io
Subject: [edk2-discuss] Submit patch to devel@edk2.groups.io.

Hi,

I encountered an issue when trying to send a patch mail to
devel@edk2.groups.io.

The SMTP server returned 250 after I issue sendmail through git, and I also
received cc. But I didn't see my post appeared on edk2-devel. I also checked
this:

https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-
Format

Looks like my format is appropriate.

I append my mail here. Your advice/suggestion will be greatly appreciated.

Keysound
==========================================================
=

Subject: [PATCH] NetworkPkg/TlsAuthConfigDxe: Use HiiPopUp() instead of
CreatePopUp()

From: Keysound Chang <Keysound_Chang@phoenix.com>

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2635

According to EDK2 Driver Writer's Guide For UEFI 2.3.1, 4.2.18 Offer
alternatives to function keys. Configuration of drivers should be
accomplished via HII and via OS-present interfaces.

Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Signed-off-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
---
.../TlsAuthConfigDxe/TlsAuthConfigDxe.inf | 1 +
.../TlsAuthConfigDxeStrings.uni | 2 ++
.../TlsAuthConfigDxe/TlsAuthConfigImpl.c | 20 +++++++++++++------
.../TlsAuthConfigDxe/TlsAuthConfigImpl.h | 1 +
4 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
index 3fc924a1d4..0ada835252 100644
--- a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
+++ b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
@@ -52,6 +52,7 @@
[Protocols] gEfiDevicePathProtocolGuid ## PRODUCES
gEfiHiiConfigAccessProtocolGuid ## PRODUCES+
gEfiHiiPopupProtocolGuid ## CONSUMES [Guids]
gTlsAuthConfigGuid ## PRODUCES ## GUIDdiff --git
a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxeStrings.uni
b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxeStrings.uni
index 973b8b7716..2b4a27d24e 100644
--- a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxeStrings.uni
+++ b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxeStrings.uni
@@ -28,6 +28,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#string STR_TLS_AUTH_CONFIG_SAVE_AND_EXIT #language en-US
"Commit Changes and Exit" #string
STR_TLS_AUTH_CONFIG_NO_SAVE_AND_EXIT #language en-US
"Discard Changes and Exit" +#string STR_TLS_AUTH_ENROLL_CERT_FAILURE
#language en-US "Enroll Cert Failure!"+ #string STR_CERT_TYPE_PCKS_GUID
#language en-US "GUID for CERT" #string STR_NULL
#language en-US ""diff --git
a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
index 2481d1098f..0ef96dfaf2 100644
--- a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
+++ b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
@@ -1383,7 +1383,6 @@ TlsAuthConfigAccessCallback (
OUT EFI_BROWSER_ACTION_REQUEST *ActionRequest ) {-
EFI_INPUT_KEY Key; EFI_STATUS Status;
RETURN_STATUS RStatus; TLS_AUTH_CONFIG_PRIVATE_DATA
*Private;@@ -1391,6 +1390,8 @@ TlsAuthConfigAccessCallback (
TLS_AUTH_CONFIG_IFR_NVDATA *IfrNvData; UINT16
LabelId; EFI_DEVICE_PATH_PROTOCOL *File;+
EFI_HII_POPUP_PROTOCOL *HiiPopUp;+ EFI_HII_POPUP_SELECTION
PopUpSelect; Status = EFI_SUCCESS; File = NULL;@@ -1402,6
+1403,11 @@ TlsAuthConfigAccessCallback (
Private = TLS_AUTH_CONFIG_PRIVATE_FROM_THIS (This);
mTlsAuthPrivateData = Private;+ Status = gBS->LocateProtocol
(&gEfiHiiPopupProtocolGuid, NULL, &HiiPopUp);+ if (EFI_ERROR (Status)) {+
DEBUG ((DEBUG_ERROR, "Can't find Form PopUp protocol. Exit (%r)\n",
Status));+ return Status;+ } // // Retrieve uncommitted data from
Browser@@ -1460,11 +1466,13 @@ TlsAuthConfigAccessCallback (
if (EFI_ERROR (Status)) { CleanFileContext (Private); -
CreatePopUp (- EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,-
&Key,- L"ERROR: Enroll Cert Failure!",- NULL+ HiiPopUp-
CreatePopup(+ HiiPopUp,+ EfiHiiPopupStyleError,+
EfiHiiPopupTypeOk,+ Private->RegisteredHandle,+
STRING_TOKEN(STR_TLS_AUTH_ENROLL_CERT_FAILURE),+
&PopUpSelect ); } break;diff --git
a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.h
b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.h
index e9af492893..68c16845b3 100644
--- a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.h
+++ b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.h
@@ -14,6 +14,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Protocol/HiiConfigAccess.h> #include
<Protocol/SimpleFileSystem.h>+#include <Protocol/HiiPopup.h> // //
Libraries--
2.23.0.windows.1


Submit patch to devel@edk2.groups.io.

Keysound Chang
 

Hi,

I encountered an issue when trying to send a patch mail to devel@edk2.groups.io.

The SMTP server returned 250 after I issue sendmail through git, and I also received cc. But I didn't see my post appeared on edk2-devel. I also checked this:

https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format

Looks like my format is appropriate.

I append my mail here. Your advice/suggestion will be greatly appreciated.

Keysound
===========================================================

Subject: [PATCH] NetworkPkg/TlsAuthConfigDxe: Use HiiPopUp() instead of CreatePopUp()

From: Keysound Chang <Keysound_Chang@phoenix.com>

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2635

According to EDK2 Driver Writer's Guide For UEFI 2.3.1, 4.2.18 Offer alternatives to function keys. Configuration of drivers should be accomplished via HII and via OS-present interfaces.

Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Signed-off-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
---
.../TlsAuthConfigDxe/TlsAuthConfigDxe.inf | 1 +
.../TlsAuthConfigDxeStrings.uni | 2 ++
.../TlsAuthConfigDxe/TlsAuthConfigImpl.c | 20 +++++++++++++------
.../TlsAuthConfigDxe/TlsAuthConfigImpl.h | 1 +
4 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
index 3fc924a1d4..0ada835252 100644
--- a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
+++ b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
@@ -52,6 +52,7 @@
[Protocols] gEfiDevicePathProtocolGuid ## PRODUCES gEfiHiiConfigAccessProtocolGuid ## PRODUCES+ gEfiHiiPopupProtocolGuid ## CONSUMES [Guids] gTlsAuthConfigGuid ## PRODUCES ## GUIDdiff --git a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxeStrings.uni b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxeStrings.uni
index 973b8b7716..2b4a27d24e 100644
--- a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxeStrings.uni
+++ b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxeStrings.uni
@@ -28,6 +28,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#string STR_TLS_AUTH_CONFIG_SAVE_AND_EXIT #language en-US "Commit Changes and Exit" #string STR_TLS_AUTH_CONFIG_NO_SAVE_AND_EXIT #language en-US "Discard Changes and Exit" +#string STR_TLS_AUTH_ENROLL_CERT_FAILURE #language en-US "Enroll Cert Failure!"+ #string STR_CERT_TYPE_PCKS_GUID #language en-US "GUID for CERT" #string STR_NULL #language en-US ""diff --git a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
index 2481d1098f..0ef96dfaf2 100644
--- a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
+++ b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
@@ -1383,7 +1383,6 @@ TlsAuthConfigAccessCallback (
OUT EFI_BROWSER_ACTION_REQUEST *ActionRequest ) {- EFI_INPUT_KEY Key; EFI_STATUS Status; RETURN_STATUS RStatus; TLS_AUTH_CONFIG_PRIVATE_DATA *Private;@@ -1391,6 +1390,8 @@ TlsAuthConfigAccessCallback (
TLS_AUTH_CONFIG_IFR_NVDATA *IfrNvData; UINT16 LabelId; EFI_DEVICE_PATH_PROTOCOL *File;+ EFI_HII_POPUP_PROTOCOL *HiiPopUp;+ EFI_HII_POPUP_SELECTION PopUpSelect; Status = EFI_SUCCESS; File = NULL;@@ -1402,6 +1403,11 @@ TlsAuthConfigAccessCallback (
Private = TLS_AUTH_CONFIG_PRIVATE_FROM_THIS (This); mTlsAuthPrivateData = Private;+ Status = gBS->LocateProtocol (&gEfiHiiPopupProtocolGuid, NULL, &HiiPopUp);+ if (EFI_ERROR (Status)) {+ DEBUG ((DEBUG_ERROR, "Can't find Form PopUp protocol. Exit (%r)\n", Status));+ return Status;+ } // // Retrieve uncommitted data from Browser@@ -1460,11 +1466,13 @@ TlsAuthConfigAccessCallback (
if (EFI_ERROR (Status)) { CleanFileContext (Private); - CreatePopUp (- EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,- &Key,- L"ERROR: Enroll Cert Failure!",- NULL+ HiiPopUp->CreatePopup(+ HiiPopUp,+ EfiHiiPopupStyleError,+ EfiHiiPopupTypeOk,+ Private->RegisteredHandle,+ STRING_TOKEN(STR_TLS_AUTH_ENROLL_CERT_FAILURE),+ &PopUpSelect ); } break;diff --git a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.h b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.h
index e9af492893..68c16845b3 100644
--- a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.h
+++ b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.h
@@ -14,6 +14,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Protocol/HiiConfigAccess.h> #include <Protocol/SimpleFileSystem.h>+#include <Protocol/HiiPopup.h> // // Libraries--
2.23.0.windows.1

701 - 720 of 906