public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Ni, Ray" <ray.ni@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
	"vit9696@protonmail.com" <vit9696@protonmail.com>,
	Laszlo Ersek <lersek@redhat.com>,
	"Gao, Zhichao" <zhichao.gao@intel.com>
Subject: Re: [edk2-devel] [PATCH 1/1] ShellPkg: Do not connect handles without device paths
Date: Tue, 14 Jan 2020 02:53:14 +0000	[thread overview]
Message-ID: <734D49CCEBEEF84792F5B80ED585239D5C3F3905@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <cCbcWcPQXQ_Gy9dcxCtr3EewGHwAwLbLz1BUqVHXWVcm62RhC9Q5mBlrKCGKExq3rIYWLSDUk1JrEFYOt5WUqqWH5E1EikNB9Y5CYDT6gwU=@protonmail.com>

[-- Attachment #1: Type: text/plain, Size: 2894 bytes --]

Vitaly,
I still have concern to modify the EDKII code to workaround a firmware bug.
Can you just change in your local version?

Thanks,
Ray

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Vitaly Cheptsov via Groups.Io
Sent: Tuesday, January 14, 2020 4:47 AM
To: Laszlo Ersek <lersek@redhat.com>; devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
Subject: Re: [edk2-devel] [PATCH 1/1] ShellPkg: Do not connect handles without device paths


Thanks all for your input,



These explanations seem sufficient to us that it is not a good idea to change the behaviour for everyone. Even so, we still need this to be configurable in some way, as having to patch EDK II is impracticable.



We believe there are three possible routes to approach this problem:



  1.  Introduce a separate ControllerConnectionLib library for this function. While it is small, we found several places in our code that need to call it beyond UEFI Shell. This way different implementations could be used depending on the chosen library.
  2.  Introduce a ConnectRequiresDevicePath PCD, which will choose the preferred logic.
  3.  Introduce a -dp Shell argument for affected commands the way Lazslo suggested.



We believe either route or a combination of multiple routes have their own benefits, and would suggest either going with 1+2 or with 3. Any approach is fine for us.



We will submit V2 of the patch after hearing the opinions.



Best wishes,

Vitaly

On Mon, Jan 13, 2020 at 20:55, Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>> wrote:
On 01/13/20 12:56, Ni, Ray wrote:
> We shouldn't assume that a DriverBindingStart() can only start on a handle with device path installed. DevicePath protocol is just a special protocol.
> It's possible that a bus driver starts on a host controller handle and creates multiple children, each with only a Specific_IO protocol installed.
> Certain device driver can start on the children handle and open the Specific_IO protocol BY_DRIVER.
> I am not sure if certain today's network drivers may work like this. It's allowed per UEFI spec.

I agree.

Under "10.2 EFI Device Path Protocol", the spec says, "If the handle
does not logically map to a physical device, the handle may not
necessarily support the device path protocol."

I think gBS->ConnectController() and
EFI_DRIVER_BINDING_PROTOCOL.Supported() should work on such handles.

If we'd like to work around related issues in drivers, then I'd suggest
new command line options for the "load", "connect", "reconnect" shell
commands (maybe more), for filtering out handles that do not carry
device paths. Such command line options could be added as an extension,
IIUC, such as "-_option". I.e., I believe it's not necessary to start
with UEFI Shell Spec updates.

Thanks
Laszlo




[-- Attachment #2: Type: text/html, Size: 11053 bytes --]

  reply	other threads:[~2020-01-14  2:53 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-12 21:38 [PATCH 0/1] ShellPkg: Do not connect handles without device paths Vitaly Cheptsov
2020-01-12 21:38 ` [PATCH 1/1] " Vitaly Cheptsov
2020-01-13  8:11   ` [edk2-devel] " Gao, Zhichao
2020-01-13  9:19     ` Vitaly Cheptsov
2020-01-13 11:56       ` Ni, Ray
2020-01-13 17:55         ` Laszlo Ersek
2020-01-13 20:46           ` Vitaly Cheptsov
2020-01-14  2:53             ` Ni, Ray [this message]
2020-01-14  8:36               ` Vitaly Cheptsov
2020-01-14 10:34                 ` Vitaly Cheptsov
2020-01-14 12:53                   ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=734D49CCEBEEF84792F5B80ED585239D5C3F3905@SHSMSX104.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox