public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: "Ni, Ray" <ray.ni@intel.com>, "Bi, Dandan" <dandan.bi@intel.com>
Cc: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
	"Wu, Hao A" <hao.a.wu@intel.com>,
	"Doran, Mark" <mark.doran@intel.com>
Subject: Re: [patch 2/2] MdeModulePkg/BmBoot: Report status when fail to load/start boot option
Date: Wed, 20 Feb 2019 10:24:52 +0100	[thread overview]
Message-ID: <0b3fbf1c-8acb-e1fd-d29e-98c36c9b81d1@redhat.com> (raw)
In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D5C026CBB@SHSMSX104.ccr.corp.intel.com>

+Mark, for my comments on the process

On 02/20/19 03:21, Ni, Ray wrote:
> Laszlo,
> Thanks for catching this.
> 
> GenPerformMemoryTest() in
> MdeModulePkg\Universal\MemoryTest\GenericMemoryTestDxe\LightMemoryTest.c
> uses the same technics as you suggested.
> 
> I give up to propose another option: having pack(1) for the new status structure.

I think that byte-packing EFI_RETURN_STATUS_EXTENDED_DATA in the PI-1.7
spec would have been viable, but we should have thought about that in
advance. The PI and UEFI specs require natural alignment for structure
members, unless stated otherwise. There *are* a number of examples of
"stated otherwise" (the term that the specs use is frequently
"byte-packed structure"). Again, we should have thought of that in
advance, before PI-1.7 was released, so now we can only fix the way the
object is populated.

More generally, I think this demonstrates a flaw in the standardization
process. PIWG and USWG should only standardize existing practice; that
is, features that have at least one functional implementation. (Not
necessarily open source, but the interfaces should be battle-hardened
*first*.) The restriction where we can't even propose patches for edk2
before the standards are updated is very counter-productive. There's
practically zero chance for getting an actual implementation
peer-reviewed and peer-tested before proposing the API for the standard.

Personally I have suggested in the past to implement some new features
as edk2 extensions first, and once they work, to upstream them to the
standards. This idea is usually rejected by maintainers, because
maintainers worry about becoming stuck with more and more edk2
extensions to core code (i.e., they worry about the feature proponents
not making good on their promise to standardize). Unfortunately, the
alternative (= the current practice) is that we standardize speculative
interfaces, which then prove suboptimal once we implement them.

Thanks
Laszlo


  reply	other threads:[~2019-02-20  9:24 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-15  8:51 [patch 0/2] Report error status when fail to load/start boot option Dandan Bi
2019-02-15  8:51 ` [patch 1/2] MdePkg/StatusCodeDataTypeId.h: Add new definition per PI1.7 Spec Dandan Bi
2019-02-15  8:51 ` [patch 2/2] MdeModulePkg/BmBoot: Report status when fail to load/start boot option Dandan Bi
2019-02-20  1:19   ` Laszlo Ersek
2019-02-20  1:36     ` Laszlo Ersek
2019-02-20  2:21     ` Ni, Ray
2019-02-20  9:24       ` Laszlo Ersek [this message]
2019-02-20 17:19         ` Doran, Mark
2019-02-21  8:55           ` Laszlo Ersek
2019-02-20  2:35     ` Bi, Dandan
2019-02-15 13:40 ` [patch 0/2] Report error " Laszlo Ersek
2019-02-15 13:58   ` Ni, Ray

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=0b3fbf1c-8acb-e1fd-d29e-98c36c9b81d1@redhat.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