Maciej,

On 04/01/21 16:24, Maciej Rabeda wrote:

Hi,

Sorry for the very late response.

Dropping iSCSI overall is a no-go - too many users + this is the only
remote block I/O we seem to support in EDKII.
As for RFC compliance vs EDKII policy on MD5... Naturally, CHAP with MD5
does not bring any security features due to MD5's vulnerability.
However, since MD5 is the only hash algorithm for CHAP supported by
IScsiDxe, removing MD5 implies removing CHAP-related code from IScsiDxe
overall, which I would be pretty hesitant to do.

RFC states that MD5 has to be supported, though I can see that CHAP
algorithm allows for different hash algorithms
(https://www.iana.org/assignments/ppp-numbers/ppp-numbers.xhtml#ppp-numbers-9).

We could support CHAP with SHA-x in IScsiDxe, which removes the MD5
dependency and keeps the CHAP-related code in iSCSI still in place.

The question is: do OS-based initiators support hash algorithms other
than MD5 for CHAP?
I am pretty sure RHEL does (controlled via /etc/iscsi/iscsid.conf), but
I am not sure about others: Windows, VMware, ...

Looking at the code for a few hours, I've had the following (roundabout)
thought process.

(1) The IScsiCHAP.[hc] files hardwire the MD5 dependency, including
direct MD5 API calls to BaseCryptLib. The idea is to generalize this to
a table of digest algos, indexed / matched by CHAP_A.

The first step would be to just turn the present MD5 support into such a
"hash algo lookup" form (with an eye towards adding a SHA256 entry to
the digest algo table later).

(2) Add a SHA256 entry to the table, using CHAP_A value 7, and using
pointers to the BaseCryptLib functions Sha256GetContextSize() and friends.

(3) Make the MD5 entry conditional on ENABLE_MD5_DEPRECATED_INTERFACES.

(4) Unfortunately, step (2) would more or less reproduce the "mHashInfo"
table we already have in "SecurityPkg/Hash2DxeCrypto/Hash2DxeCrypto.c"
(that is, in the implementation of EFI_HASH2_PROTOCOL).

Duplicating that table (especially with the function pointer typedefs,
for the members in the EFI_HASH_INFO record type) is ugly as heck.

Extracting said funcptr types is a PITA too -- they incorrectly use the
EFI_ prefix (ex. EFI_HASH_UPDATE), so simply moving them to a public
header file won't fly.

(5) Well, how about rebasing IScsiDxe from BaseCryptLib to
EFI_HASH2_PROTOCOL?

It turns out that IScsiDxe depends on BaseCryptLib -- what's more, on
CryptoPkg altogether! -- only for the MD5 APIs called in
IScsiCHAPCalculateResponse() [NetworkPkg/IScsiDxe/IScsiCHAP.c].
Therefore, the digest algo generalization should actually be started by
replacing the direct MD5 calls with EFI_HASH2_PROTOCOL usage, using the
standard GUID "gEfiHashAlgorithmMD5Guid".

(6) For this, a table would still be needed, but it would map CHAP_A
values to hash algo GUIDs *only* (so it'd be a much simpler table than
the one from (2)).

(7) However... using EFI_HASH2_PROTOCOL would break compatibility, for
two reasons.

The first reason is that our EFI_HASH2_PROTOCOL implementation does not
support MD5 at all, since commit 0a1b6d0be337
("SecurityPkg/Hash2DxeCrypto: Remove MD5 support", 2020-11-17). So
platforms could not opt in to MD5 even if they wanted to.

The second reason is that platforms may include IScsiDxe (via
NETWORK_ISCSI_ENABLE), but not Hash2DxeCrypto. On such platforms, CHAP
auth would always fail (the gBS->LocateProtocol() call for
EFI_HASH2_PROTOCOL would fail). The solution for this would be to either
(a) find all platforms in edk2 *and* edk2-platforms that expose
NETWORK_ISCSI_ENABLE, and make sure they include Hash2DxeCrypto too; or
(b) modify "NetworkComponents.dsc.inc", for including Hash2DxeCrypto
alongside IScsiDxe.

The first compat breakage does not disturb me. We've already agreed that
breaking RFC compat by not offering CHAP_A=5 is fine.

The second compat breakage does bother me: I'm very much not looking
forward to (a) extending umpteen DSC files in *edk2-platforms* for
including Hash2DxeCrypto, nor do I feel happy about (b) referring to
Hash2DxeCrypto from *SecurityPkg* in the *NetworkPkg* DSC include file.

I feel that the second compat breakage invalidates the whole
EFI_HASH2_PROTOCOL consumption idea, unfortunately.

(8) Ultimately, the simplest approach is to keep IScsiDxe self-contained
(= directly dependent on BaseCryptLib), and to *replace* MD5, as the
sole supported CHAP algo, with SHA256. IScsiDxe would remain
"single-digest", so to say; there would be no change to the control flow
anywhere.

ACK?

Thanks
Laszlo

Thanks,
Maciej

On 19-Mar-21 18:39, Desimone, Nathaniel L wrote:

I'm honestly surprised that the iSCSI RFC has not been updated to drop
CHAP-MD5 at this point. I realize that is probably a bigger effort but
it seems like something the industry should do. I agree that having a
FeaturePcd for CHAP MD5 and having it default to False is probably the
right thing to do.

-----Original Message-----
From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Samer El-Haj-
Mahmoud
Sent: Friday, March 19, 2021 10:08 AM
To: rfc@edk2.groups.io; simo@...; Laszlo Ersek
<lersek@...>
Cc: Maciej Rabeda <maciej.rabeda@...>; Wu, Jiaxin
<jiaxin.wu@...>; Fu, Siyuan <siyuan.fu@...>; Daniel P.
Berrangé <berrange@...>; Yash Mankad <ymankad@...>;
Pete Batard <pete@...>; Samer El-Haj-Mahmoud <Samer.El-Haj-
Mahmoud@...>
Subject: Re: [edk2-rfc] removing CHAP-MD5 from IScsiDxe

When the RPi UEFI FW dropped iSCSI (because it was removed from
NetworkDefines.dsc.inc), we got multiple users complaining since they
had
existing use cases that depended on that. See for instance the RPi4 UEFI
reported bug: https://github.com/pftf/RPi4/issues/125

Some use-cases and user scenarios for iSCSI on RPi4:
https://blogs.vmware.com/arm/2020/10/17/esxi-arm-with-iscsi/
https://tech.xlab.si/blog/pxe-boot-raspberry-pi-iscsi/
https://www.virtuallyghetto.com/2020/07/two-methods-to-network-boot-
raspberry-pi-4.html

The RPi4 users preferred having iSCSI (without CHAP) than not having
it at all.
We resorted to enabling it out-of-tree using a build switch.

 From my experience, I think the EDK2 iScsiDxe software initiator is
being
wildly used on many servers (and OSes) across the industry (just google
"UEFI iSCSI"). Having it silently dropped from EDK2 is not optimal. I
like
Laszlo's idea of a PCD to break the RFC (and using an iSCSI boot
variant that
does not support CHAP), and I think it is a good compromise. The
alternative
is for downstream implementations to resort to enabling it (probably
just set
NETWORK_ISCSI_ENABLE=TRUE) to keep their users happy.

Thanks,
--Samer

-----Original Message-----
From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Simo Sorce
via groups.io
Sent: Friday, March 19, 2021 9:26 AM
To: Laszlo Ersek <lersek@...>; edk2-rfc-groups-io
<rfc@edk2.groups.io>
Cc: Maciej Rabeda <maciej.rabeda@...>; Jiaxin Wu
<jiaxin.wu@...>; Siyuan Fu <siyuan.fu@...>; Daniel P.
Berrangé <berrange@...>; Yash Mankad

<ymankad@...>

Subject: Re: [edk2-rfc] removing CHAP-MD5 from IScsiDxe

On Fri, 2021-03-19 at 14:15 +0100, Laszlo Ersek wrote:

Hi,

RFC 7143 requires CHAP-MD5 as a mandatory option offered by an iscsi
client (initiator).

At the same time, edk2 has deprecated MD5 in general, as a
cryptographically weak hash algorithm.

Consequently, the "NetworkDefines.dsc.inc" file defines
NETWORK_ISCSI_ENABLE with default value FALSE (commit

4ecb1ba5efa3 /

TianoCore#3003). Platforms that want to include IScsiDxe need to opt
in consciously to the presence of CHAP-MD5 in the code.

We're not happy with this granularity. We'd prefer:

- explicitly breaking RFC 7143 conformance,
- removing CHAP-MD5,
- and using an IScsiDxe variant that is honest about having no
confidentiality / integrity.

IScsiDxe is safe on a trusted network, and only on a trusted network.
The presence of CHAP-MD5 suggests it may be safe on an untrusted
network too, and that implication (not the whole iscsi client
functionality) is what we should rid ourselves of.

Lazlo,
This sounds like the right direction to me.

My 2C,
Simo.

Are NetworkPkg maintainers open to breaking RFC 7143 conformance in
IScsiDxe (perhaps with a feature PCD?), or should we look into this
only downstream?

Downstream, we might decide to drop IScsiDxe altogether, in sync
with the upstream NETWORK_ISCSI_ENABLE=FALSE default -- that
decision has not been made yet. Now I'm just testing whether keeping
IScsiDxe enabled down-stream would require us to carry downstream-

only patches.

Thanks
Laszlo

--
Simo Sorce
RHEL Crypto Team
Red Hat, Inc

IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended
recipient,
please notify the sender immediately and do not disclose the contents
to any
other person, use it for any purpose, or store or copy the
information in any
medium. Thank you.

-----Original Message-----
From: Kinney, Michael D [mailto:michael.d.kinney@...]
Sent: Wednesday, April 7, 2021 4:30 AM
To: rfc@edk2.groups.io; lersek@...; Chang, Abner (HPS SW/FW
Technologist) <abner.chang@...>; Kinney, Michael D
<michael.d.kinney@...>
Subject: RE: [edk2-rfc] [RFC] Documents under RedfishPkg

Hi Abner,

PlantUML is a way to embed some graphics in GitHub markdown text. Avoids
checking in SVG or BMP or PNG binaries.

INVALID URI REMOVED
3A__plantuml.com_&d=DwIGaQ&c=C5b8zRQO1miGmBeVZ2LFWg&r=_SN6F
ZBN4Vgi4Ulkskz6qU3NYRO03nHp9P7Z5q59A3E&m=dopb0lYExPus6V854vF8a
pisNnwzidkdZGS2XDjtA14&s=9nCqxY2Suxl1CLUlz-
Azhi6FWaZORPo3oQLiTfq4fI0&e=

INVALID URI REMOVED
3A__plantuml.com_en_guide&d=DwIGaQ&c=C5b8zRQO1miGmBeVZ2LFWg
&r=_SN6FZBN4Vgi4Ulkskz6qU3NYRO03nHp9P7Z5q59A3E&m=dopb0lYExPus6
V854vF8apisNnwzidkdZGS2XDjtA14&s=EjaowDShCFlt013uCcS-
QiSe0br8c9eGeLYMQyGKG0o&e=


Here is an example of using PlatUML from a Wiki page with the PlantUML
source text file also checked in.

https://github.com/tianocore-docs/edk2-
TemplateSpecification/wiki/TianoCore-Documents-GitBook-
Overview#introduction

GitHub Markdown source line that uses PlantUML server to convert
PlantUML source to a SVG when the page is viewed.

![GitBookOverview](INVALID URI REMOVED
3A__www.plantuml.com_plantuml_proxy-3Fsrc-3Dhttps-
3A__raw.githubusercontent.com_wiki_tianocore-2Ddocs_edk2-
2DTemplateSpecification_plantuml_GitBookOverview.puml&d=DwIGaQ&c=
C5b8zRQO1miGmBeVZ2LFWg&r=_SN6FZBN4Vgi4Ulkskz6qU3NYRO03nHp9P7
Z5q59A3E&m=dopb0lYExPus6V854vF8apisNnwzidkdZGS2XDjtA14&s=x2nZ6fZ
0N29Q5-WRynU6bf1vwK7pOrOID4cqIS41Rfw&e= )

PlantUML source file

INVALID URI REMOVED
3A__raw.githubusercontent.com_wiki_tianocore-2Ddocs_edk2-
2DTemplateSpecification_plantuml_GitBookOverview.puml&d=DwIGaQ&c=
C5b8zRQO1miGmBeVZ2LFWg&r=_SN6FZBN4Vgi4Ulkskz6qU3NYRO03nHp9P7
Z5q59A3E&m=dopb0lYExPus6V854vF8apisNnwzidkdZGS2XDjtA14&s=ToF_Ks
UzwXf97ohi5HZhARPubr7s1zqqrPnlSiSXUks&e=


Mike

-----Original Message-----
From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Laszlo Ersek
Sent: Tuesday, April 6, 2021 7:06 AM
To: rfc@edk2.groups.io; abner.chang@...
Subject: Re: [edk2-rfc] [RFC] Documents under RedfishPkg

On 03/26/21 16:42, Abner Chang wrote:

Hi edk2 Stewards,
Is that ok to put some EDK2 Redfish port guide or design block diagrams in

a folder under RedfishPkg? e.g.

RedfishPkg/Documents?

Any practices we have on the current edk2 repo?

If you can add the documents in some text-based representation (markup
or plaintext), where "reviewing patches" will make sense in the future,
then I welcome adding the documentation under RedfishPkg.

(If you have diagrams, I think SVG might be good for those.)

Other packages have introduced porting and other accompanying
documentation in the wiki, but I find that inferior. I don't like it
when documentation lives in a separate repository from the code that it
documents.

Thanks
Laszlo

-----Original Message-----
From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Laszlo Ersek
Sent: Tuesday, April 6, 2021 7:06 AM
To: rfc@edk2.groups.io; abner.chang@...
Subject: Re: [edk2-rfc] [RFC] Documents under RedfishPkg

On 03/26/21 16:42, Abner Chang wrote:

Hi edk2 Stewards,
Is that ok to put some EDK2 Redfish port guide or design block diagrams in a folder under RedfishPkg? e.g.

RedfishPkg/Documents?

Any practices we have on the current edk2 repo?

If you can add the documents in some text-based representation (markup
or plaintext), where "reviewing patches" will make sense in the future,
then I welcome adding the documentation under RedfishPkg.

(If you have diagrams, I think SVG might be good for those.)

Other packages have introduced porting and other accompanying
documentation in the wiki, but I find that inferior. I don't like it
when documentation lives in a separate repository from the code that it
documents.

Thanks
Laszlo

-----Original Message-----
From: Rebecca Cran <rebecca@...>
Sent: Sunday, February 14, 2021 4:43 PM
To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>; rfc@edk2.groups.io; 'Bret Barkelew'
<Bret.Barkelew@...>; Laszlo Ersek <lersek@...>; Gao, Liming <liming.gao@...>
Subject: Re: [edk2-devel] [RFC] UnitTestFrameworkPkg cmocka submodule alternatives

On 12/19/20 11:58 AM, Michael D Kinney wrote:

Hello,

There have been a few suggestions to create a mirror of cmocka in TianoCore
org in GitHub.

I have found a GitHub action that can do a repo sync.

https://github.com/marketplace/actions/github-repo-sync

I have created a temporary mirror of cmocka in my personal GitHub area that
uses this GitHub action to sync all branches and all tags once a day.

https://github.com/mdkinney/mirror-cmocka

Here is the GitHub workflow file. It must be in the default branch for the
repo using a branch name that is not present in the repo being mirrored.
In this case, I used a branch name of 'repo-sync'.

https://github.com/mdkinney/mirror-cmocka/blob/repo-sync/.github/workflows/repo-sync.yml

Please provide feedback on this approach. If we like this approach, then
I suggest we create a new repo in TianoCore called edk2-cmocka that is a
mirror that is synced once a day and we update the cmocka submodule in the
edk2 repo to use edk2-cmocka.

I just noticed this never got committed:
https://github.com/tianocore/edk2/blob/master/.gitmodules still shows
the old URL.

--
Rebecca Cran

On Thu, Apr 01, 2021 at 04:24:27PM +0200, Rabeda, Maciej wrote:

Hi,

Sorry for the very late response.

Dropping iSCSI overall is a no-go - too many users + this is the only remote
block I/O we seem to support in EDKII.
As for RFC compliance vs EDKII policy on MD5... Naturally, CHAP with MD5
does not bring any security features due to MD5's vulnerability.
However, since MD5 is the only hash algorithm for CHAP supported by
IScsiDxe, removing MD5 implies removing CHAP-related code from IScsiDxe
overall, which I would be pretty hesitant to do.

RFC states that MD5 has to be supported, though I can see that CHAP
algorithm allows for different hash algorithms (https://www.iana.org/assignments/ppp-numbers/ppp-numbers.xhtml#ppp-numbers-9).
We could support CHAP with SHA-x in IScsiDxe, which removes the MD5
dependency and keeps the CHAP-related code in iSCSI still in place.

The question is: do OS-based initiators support hash algorithms other than
MD5 for CHAP?
I am pretty sure RHEL does (controlled via /etc/iscsi/iscsid.conf), but I am
not sure about others: Windows, VMware, ...

Linux kernel gained support for the SHA* family of hashes:

commit a572d24af4d16e70743feb0b4decb17aaae7ce43
Author: Maurizio Lombardi <mlombard@...>
Date: Mon Oct 28 13:38:20 2019 +0100

scsi: target: iscsi: CHAP: add support for SHA1, SHA256 and SHA3-256
This patch modifies the chap_server_compute_hash() function to make it
agnostic to the choice of hash algorithm that is used. It also adds
support to three new hash algorithms: SHA1, SHA256 and SHA3-256.
The chap_got_response() function has been removed because the digest type
validity is already checked by chap_server_open()
Link: https://lore.kernel.org/r/20191028123822.5864-2-mlombard@redhat.com
Signed-off-by: Maurizio Lombardi <mlombard@...>
Tested-by: Chris Leech <cleech@...>
Signed-off-by: Martin K. Petersen <martin.petersen@...>

NB SHA1 is just as undesirable as MD5 these days, so only the other two
are especially interesting/useful.

Regards,
Daniel

I'm honestly surprised that the iSCSI RFC has not been updated to drop CHAP-MD5 at this point. I realize that is probably a bigger effort but it seems like something the industry should do. I agree that having a FeaturePcd for CHAP MD5 and having it default to False is probably the right thing to do.

-----Original Message-----
From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Samer El-Haj-
Mahmoud
Sent: Friday, March 19, 2021 10:08 AM
To: rfc@edk2.groups.io; simo@...; Laszlo Ersek
<lersek@...>
Cc: Maciej Rabeda <maciej.rabeda@...>; Wu, Jiaxin
<jiaxin.wu@...>; Fu, Siyuan <siyuan.fu@...>; Daniel P.
Berrangé <berrange@...>; Yash Mankad <ymankad@...>;
Pete Batard <pete@...>; Samer El-Haj-Mahmoud <Samer.El-Haj-
Mahmoud@...>
Subject: Re: [edk2-rfc] removing CHAP-MD5 from IScsiDxe

When the RPi UEFI FW dropped iSCSI (because it was removed from
NetworkDefines.dsc.inc), we got multiple users complaining since they had
existing use cases that depended on that. See for instance the RPi4 UEFI
reported bug: https://github.com/pftf/RPi4/issues/125

Some use-cases and user scenarios for iSCSI on RPi4:
https://blogs.vmware.com/arm/2020/10/17/esxi-arm-with-iscsi/
https://tech.xlab.si/blog/pxe-boot-raspberry-pi-iscsi/
https://www.virtuallyghetto.com/2020/07/two-methods-to-network-boot-
raspberry-pi-4.html

The RPi4 users preferred having iSCSI (without CHAP) than not having it at all.
We resorted to enabling it out-of-tree using a build switch.

From my experience, I think the EDK2 iScsiDxe software initiator is being
wildly used on many servers (and OSes) across the industry (just google
"UEFI iSCSI"). Having it silently dropped from EDK2 is not optimal. I like
Laszlo's idea of a PCD to break the RFC (and using an iSCSI boot variant that
does not support CHAP), and I think it is a good compromise. The alternative
is for downstream implementations to resort to enabling it (probably just set
NETWORK_ISCSI_ENABLE=TRUE) to keep their users happy.

Thanks,
--Samer

-----Original Message-----
From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Simo Sorce
via groups.io
Sent: Friday, March 19, 2021 9:26 AM
To: Laszlo Ersek <lersek@...>; edk2-rfc-groups-io
<rfc@edk2.groups.io>
Cc: Maciej Rabeda <maciej.rabeda@...>; Jiaxin Wu
<jiaxin.wu@...>; Siyuan Fu <siyuan.fu@...>; Daniel P.
Berrangé <berrange@...>; Yash Mankad

<ymankad@...>

Subject: Re: [edk2-rfc] removing CHAP-MD5 from IScsiDxe

On Fri, 2021-03-19 at 14:15 +0100, Laszlo Ersek wrote:

Hi,

RFC 7143 requires CHAP-MD5 as a mandatory option offered by an iscsi
client (initiator).

At the same time, edk2 has deprecated MD5 in general, as a
cryptographically weak hash algorithm.

Consequently, the "NetworkDefines.dsc.inc" file defines
NETWORK_ISCSI_ENABLE with default value FALSE (commit

4ecb1ba5efa3 /

TianoCore#3003). Platforms that want to include IScsiDxe need to opt
in consciously to the presence of CHAP-MD5 in the code.

We're not happy with this granularity. We'd prefer:

- explicitly breaking RFC 7143 conformance,
- removing CHAP-MD5,
- and using an IScsiDxe variant that is honest about having no
confidentiality / integrity.

IScsiDxe is safe on a trusted network, and only on a trusted network.
The presence of CHAP-MD5 suggests it may be safe on an untrusted
network too, and that implication (not the whole iscsi client
functionality) is what we should rid ourselves of.

Lazlo,
This sounds like the right direction to me.

My 2C,
Simo.

Are NetworkPkg maintainers open to breaking RFC 7143 conformance in
IScsiDxe (perhaps with a feature PCD?), or should we look into this
only downstream?

Downstream, we might decide to drop IScsiDxe altogether, in sync
with the upstream NETWORK_ISCSI_ENABLE=FALSE default -- that
decision has not been made yet. Now I'm just testing whether keeping
IScsiDxe enabled down-stream would require us to carry downstream-

only patches.

Thanks
Laszlo

--
Simo Sorce
RHEL Crypto Team
Red Hat, Inc

IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended recipient,
please notify the sender immediately and do not disclose the contents to any
other person, use it for any purpose, or store or copy the information in any
medium. Thank you.

-----Original Message-----
From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of gaoliming
Sent: Friday, March 26, 2021 9:51 AM
To: Wang, Nickle (HPS SW) <nickle.wang@...>; rfc@edk2.groups.io;
lersek@...
Cc: 'Dandan Bi' <dandan.bi@...>; Chang, Abner (HPS SW/FW
Technologist) <abner.chang@...>
Subject: 回复: [edk2-rfc] [RFC] Have new interface in
EDKII_FORM_BROWSER_EXTENSION2_PROTOCOL to disable hotkey support

Nickle:

-----邮件原件-----
发件人: Wang, Nickle (HPS SW) <nickle.wang@...>
发送时间: 2021年3月25日 15:41
收件人: rfc@edk2.groups.io; gaoliming@...;

lersek@...

抄送: 'Dandan Bi' <dandan.bi@...>; Chang, Abner (HPS SW/FW
Technologist) <abner.chang@...>; Wang, Nickle (HPS SW)
<nickle.wang@...>
主题: RE: [edk2-rfc] [RFC] Have new interface in
EDKII_FORM_BROWSER_EXTENSION2_PROTOCOL to disable hotkey

support

-----Original Message-----
From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of gaoliming
Sent: Thursday, March 25, 2021 11:40 AM
To: rfc@edk2.groups.io; lersek@...; Wang, Nickle (HPS SW)
<nickle.wang@...>
Cc: 'Dandan Bi' <dandan.bi@...>; Chang, Abner (HPS SW/FW
Technologist) <abner.chang@...>
Subject: 回复: [edk2-rfc] [RFC] Have new interface in
EDKII_FORM_BROWSER_EXTENSION2_PROTOCOL to disable hotkey

support

-----邮件原件-----
发件人: rfc@edk2.groups.io <rfc@edk2.groups.io> 代表 Laszlo Ersek
发送时间: 2021年3月24日 19:37
收件人: rfc@edk2.groups.io; nickle.wang@...
抄送: Dandan Bi <dandan.bi@...>; Chang, Abner (HPS SW/FW
Technologist) <abner.chang@...>
主题: Re: [edk2-rfc] [RFC] Have new interface in
EDKII_FORM_BROWSER_EXTENSION2_PROTOCOL to disable hotkey

support

On 03/24/21 09:21, Nickle Wang wrote:

Hi All,

I am having trouble to disable hotkey support in HII browser
with existing

interface. I am wondering if we could have new interface to
disable hotkey support in form browser extension2 protocol.

1. Background:
In some HII driver, there is no need to have hotkey support in
certain HII

menu due to its design. The examples are EDK2 iSCSI driver, secure
boot configuration and Tls auth configuration driver. In these HII
menus, there is HII button to do submit or discard job and they do
not answer to hotkey action. I also see this design in 3rd party
option card driver. In this case, user feels confused because it
dose not do anything while pressing hotkey. The only way to save
or discard data is by pressing the button on HII menu. So, we need
to disable hotkey

support and force user to use HII button.

I agree with the problem statement. I've noticed this too, and
found it confusing.

I wonder if there are HII guidelines or best known practices to
deal with it -- I assume that one desirable solution might be if
we could control the hotkeys just from the VFR, one way or
another. But indeed I don't know if that's feasible by design.

UEFI driver write guide 12.6.1 Minimize callbacks section defines
the guide line.
https://github.com/tianocore/tianocore.github.io/wiki/UEFI-Driver-
Writer%27s-Guide

But, some HII drivers still like to control submit/discard/exit in
its form by Callback function.
Hotkey way is to consume HII driver ConfigAccess protocol
RouteConfig() function for data saving.
So, HotKey doesn't work on those drivers.

For this proposal, I want to understand when new interface will be
trigged and what module calls this new API.

The module to call this new API is the HII driver which has no hotkey
support in its HII menu. The HII driver could also be option card driver.

As for when the new interface will be triggered, there are two cases
in my
mind:

1. Driver wants to disable hotkey support for entire HII form-set.
1.1 Driver will call new interface in ExtractConfig() function with
Scope "FormSetLevel".
1.2 Driver calls new interface in Callback() function when Action is
set to EFI_BROWSER_ACTION_FORM_OPEN. With Scope " FormLevel", this
will disable hotkey for every HII menu.
2. Driver only disable hotkey support for certain HII menu.
2. Same as 1.2 but driver have condition check to see if this is
target HII menu or not. The Scope is set to " FormLevel" and the
hotkey support is restored automatically by display engine when user

leaves this HII menu.

So, your proposal is to let HII driver call new API to disable or enable Hotkey
on its page.
There are still some issues here.
1. UEFI option rom driver is the independent driver. It refers to the standard
UEFI interface.
But, form browser extension2 protocol is edk2 extension. Other UEFI
implementation may not provide this protocol.

2. Browser may have the different Hotkey. UEFI driver doesn't know to
disable which Hotkey.

In fact, UEFI spec defines HII ConfigAccess protocol for data read and write.
HII driver should follow this way.

If HII driver doesn't use this way, it can't be handled by Browser. I think this is
the expected behavior.
We don't need to introduce new API for it.

Thanks,
Nickle

Thanks
Liming

Thanks
Laszlo

2. Issue:
Currently we have

EDKII_FORM_BROWSER_EXTENSION2_PROTOCOL.RegisterHotKey to

create or

delete hotkey support. With value 0 in "Action", we can delete
corresponding hotkey.

typedef
EFI_STATUS
(EFIAPI *REGISTER_HOT_KEY) (
IN EFI_INPUT_KEY *KeyData,
IN UINT32 Action,
IN UINT16 DefaultId,
IN EFI_STRING HelpString OPTIONAL
);

However, there are some troubles for driver or option card
driver to use it

and delete hotkey support.

* Usually, display engine is the one who creates hotkey in HII

browser.

So driver has no information about the key data. And there is no
way to get these hotkey information programmatically.

* If driver tries to delete hotkey for its HII menu, it requires driver

to

restore hokey support when user leaves its HII menu. This could
easily break the hotkey support in HII browser because display
engine has no control to this.

3. Proposal:
I would like to propose new interface in

EDKII_FORM_BROWSER_EXTENSION2_PROTOCOL so that driver can

temporarily

disable hotkey support for certain HII menu in its own form-set.
And display engine will handle the hotkey support accordingly.

struct EDKII_FORM_BROWSER_EXTENSION2_PROTOCOL {
...
REGISTER_HOT_KEY RegisterHotKey;
DISABLE_HOT_KEY DisableHotKey;
...
};

/**
Disable the hotkey by given its browser action.

@param[in] Action Action value to disable corresponding

hotkey.

@param[in] Scope Hotkey scope level to be disabled.

@retval EFI_SUCCESS Hotkey is disabled.
@retval EFI_NOT_FOUND Hotkey with given Action is not found.
**/
typedef
EFI_STATUS
(EFIAPI *DISABLE_HOT_KEY) (
IN UINT32 Action,
IN BROWSER_SETTING_SCOPE Scope );



May I have your comment about this? Is this good idea for driver to
disable

hotkey support?

Thanks,
Nickle

-----Original Message-----
From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of gaoliming
Sent: Thursday, March 25, 2021 11:40 AM
To: rfc@edk2.groups.io; lersek@...; Wang, Nickle (HPS SW)
<nickle.wang@...>
Cc: 'Dandan Bi' <dandan.bi@...>; Chang, Abner (HPS SW/FW
Technologist) <abner.chang@...>
Subject: 回复: [edk2-rfc] [RFC] Have new interface in
EDKII_FORM_BROWSER_EXTENSION2_PROTOCOL to disable hotkey support

-----邮件原件-----
发件人: rfc@edk2.groups.io <rfc@edk2.groups.io> 代表 Laszlo Ersek
发送时间: 2021年3月24日 19:37
收件人: rfc@edk2.groups.io; nickle.wang@...
抄送: Dandan Bi <dandan.bi@...>; Chang, Abner (HPS SW/FW
Technologist) <abner.chang@...>
主题: Re: [edk2-rfc] [RFC] Have new interface in
EDKII_FORM_BROWSER_EXTENSION2_PROTOCOL to disable hotkey

support

On 03/24/21 09:21, Nickle Wang wrote:

Hi All,

I am having trouble to disable hotkey support in HII browser with
existing

interface. I am wondering if we could have new interface to disable
hotkey support in form browser extension2 protocol.

1. Background:
In some HII driver, there is no need to have hotkey support in
certain HII

menu due to its design. The examples are EDK2 iSCSI driver, secure
boot configuration and Tls auth configuration driver. In these HII
menus, there is HII button to do submit or discard job and they do not
answer to hotkey action. I also see this design in 3rd party option
card driver. In this case, user feels confused because it dose not do
anything while pressing hotkey. The only way to save or discard data
is by pressing the button on HII menu. So, we need to disable hotkey

support and force user to use HII button.

I agree with the problem statement. I've noticed this too, and found
it confusing.

I wonder if there are HII guidelines or best known practices to deal
with it -- I assume that one desirable solution might be if we could
control the hotkeys just from the VFR, one way or another. But indeed
I don't know if that's feasible by design.

UEFI driver write guide 12.6.1 Minimize callbacks section defines the guide
line.
https://github.com/tianocore/tianocore.github.io/wiki/UEFI-Driver-
Writer%27s-Guide

But, some HII drivers still like to control submit/discard/exit in its form by
Callback function.
Hotkey way is to consume HII driver ConfigAccess protocol RouteConfig()
function for data saving.
So, HotKey doesn't work on those drivers.

For this proposal, I want to understand when new interface will be trigged
and what module calls this new API.

Thanks
Liming

Thanks
Laszlo

2. Issue:
Currently we have

EDKII_FORM_BROWSER_EXTENSION2_PROTOCOL.RegisterHotKey to

create or

delete hotkey support. With value 0 in "Action", we can delete
corresponding hotkey.

typedef
EFI_STATUS
(EFIAPI *REGISTER_HOT_KEY) (
IN EFI_INPUT_KEY *KeyData,
IN UINT32 Action,
IN UINT16 DefaultId,
IN EFI_STRING HelpString OPTIONAL
);

However, there are some troubles for driver or option card driver to
use it

and delete hotkey support.

* Usually, display engine is the one who creates hotkey in HII browser.

So driver has no information about the key data. And there is no way
to get these hotkey information programmatically.

* If driver tries to delete hotkey for its HII menu, it requires driver to

restore hokey support when user leaves its HII menu. This could easily
break the hotkey support in HII browser because display engine has no
control to this.

3. Proposal:
I would like to propose new interface in

EDKII_FORM_BROWSER_EXTENSION2_PROTOCOL so that driver can

temporarily

disable hotkey support for certain HII menu in its own form-set.
And display engine will handle the hotkey support accordingly.

struct EDKII_FORM_BROWSER_EXTENSION2_PROTOCOL {
...
REGISTER_HOT_KEY RegisterHotKey;
DISABLE_HOT_KEY DisableHotKey;
...
};

/**
Disable the hotkey by given its browser action.

@param[in] Action Action value to disable corresponding hotkey.
@param[in] Scope Hotkey scope level to be disabled.

@retval EFI_SUCCESS Hotkey is disabled.
@retval EFI_NOT_FOUND Hotkey with given Action is not found.
**/
typedef
EFI_STATUS
(EFIAPI *DISABLE_HOT_KEY) (
IN UINT32 Action,
IN BROWSER_SETTING_SCOPE Scope
);



May I have your comment about this? Is this good idea for driver to
disable

hotkey support?

Thanks,
Nickle

-----Original Message-----
From: Laszlo Ersek <lersek@...>
Sent: Wednesday, March 24, 2021 7:37 PM
To: rfc@edk2.groups.io; Wang, Nickle (HPS SW) <nickle.wang@...>
Cc: Dandan Bi <dandan.bi@...>; Chang, Abner (HPS SW/FW
Technologist) <abner.chang@...>
Subject: Re: [edk2-rfc] [RFC] Have new interface in
EDKII_FORM_BROWSER_EXTENSION2_PROTOCOL to disable hotkey support

On 03/24/21 09:21, Nickle Wang wrote:

Hi All,

I am having trouble to disable hotkey support in HII browser with existing

interface. I am wondering if we could have new interface to disable hotkey
support in form browser extension2 protocol.

1. Background:
In some HII driver, there is no need to have hotkey support in certain HII

menu due to its design. The examples are EDK2 iSCSI driver, secure boot
configuration and Tls auth configuration driver. In these HII menus, there is
HII button to do submit or discard job and they do not answer to hotkey
action. I also see this design in 3rd party option card driver. In this case, user
feels confused because it dose not do anything while pressing hotkey. The
only way to save or discard data is by pressing the button on HII menu. So,
we need to disable hotkey support and force user to use HII button.

I agree with the problem statement. I've noticed this too, and found it
confusing.

I wonder if there are HII guidelines or best known practices to deal with it -- I
assume that one desirable solution might be if we could control the hotkeys
just from the VFR, one way or another. But indeed I don't know if that's
feasible by design.

Thanks
Laszlo

2. Issue:
Currently we have

EDKII_FORM_BROWSER_EXTENSION2_PROTOCOL.RegisterHotKey to create
or delete hotkey support. With value 0 in "Action", we can delete
corresponding hotkey.

typedef
EFI_STATUS
(EFIAPI *REGISTER_HOT_KEY) (
IN EFI_INPUT_KEY *KeyData,
IN UINT32 Action,
IN UINT16 DefaultId,
IN EFI_STRING HelpString OPTIONAL
);

However, there are some troubles for driver or option card driver to use it

and delete hotkey support.

* Usually, display engine is the one who creates hotkey in HII browser. So

driver has no information about the key data. And there is no way to get
these hotkey information programmatically.

* If driver tries to delete hotkey for its HII menu, it requires driver to

restore hokey support when user leaves its HII menu. This could easily break
the hotkey support in HII browser because display engine has no control to
this.

3. Proposal:
I would like to propose new interface in

EDKII_FORM_BROWSER_EXTENSION2_PROTOCOL so that driver can
temporarily disable hotkey support for certain HII menu in its own form-set.
And display engine will handle the hotkey support accordingly.

struct EDKII_FORM_BROWSER_EXTENSION2_PROTOCOL {
...
REGISTER_HOT_KEY RegisterHotKey;
DISABLE_HOT_KEY DisableHotKey;
...
};

/**
Disable the hotkey by given its browser action.

@param[in] Action Action value to disable corresponding hotkey.
@param[in] Scope Hotkey scope level to be disabled.

@retval EFI_SUCCESS Hotkey is disabled.
@retval EFI_NOT_FOUND Hotkey with given Action is not found.
**/
typedef
EFI_STATUS
(EFIAPI *DISABLE_HOT_KEY) (
IN UINT32 Action,
IN BROWSER_SETTING_SCOPE Scope
);



May I have your comment about this? Is this good idea for driver to disable

hotkey support?

Thanks,
Nickle