public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Ard Biesheuvel" <ard.biesheuvel@arm.com>
To: awarkentin@vmware.com, Andrew Fish <afish@apple.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: Pete Batard <pete@akeo.ie>,
	Samer El-Haj-Mahmoud <samer@elhajmahmoud.com>,
	Leif Lindholm <leif@nuviainc.com>
Subject: Re: [edk2-devel] [edk2-platform][PATCH v1 0/4] Platform/RaspberryPi : Enable TFTP shell command
Date: Mon, 20 Apr 2020 08:45:17 +0200	[thread overview]
Message-ID: <10bee111-6628-e111-1057-9608bbd31521@arm.com> (raw)
In-Reply-To: <BN6PR05MB341154031D911F0125B1855BB9D40@BN6PR05MB3411.namprd05.prod.outlook.com>

On 4/20/20 4:41 AM, awarkentin@vmware.com wrote:
> Hi Andrew,
> 
> This particular issue revolves around enabling a feature by default in 
> the DSC for Pi 3/4 platforms - the TFTP Shell command. Clearly, enabling 
> this for Pi bears no effect on other Tiano platforms. So far I've heard 
> something about the TFTP code being somehow bad, but what does that have 
> to do with the Pi platform using it? If it's bad, delete it from Tiano. 
> Or rewrite it.
> 

The TFTP command can be included by a -D option on the build command 
line, but I am opposing to enabling it by default. The reason for this 
is that we have been trying to push back on custom vendor tools to do 
firmware upgrades and other things from the shell, and instead, use 
capsule update.

I really don't get what the drama is all about, given that the TFTP 
command is only a -D option away when you want it, and we don't 
distribute binary builds from edk2-platforms anyway.

> .../last/ time we got into a spat about the value of revving the Pi ACPI 
> to 6.3 (the ACPI support we contributed in the first place). So that 
> patch just got ignored. Not even sure what to do about that one...
> 

I rejected ACPI 6.3 table upgrades because, in their current form, the 
only thing they achieve is losing the ability to boot an OS that 
predates ACPI 6.3. Every piece of the platform currently being described 
via ACPI can be described using 5.1 tables.

ACPI 6.3 tables are not 'better' or 'more current' than 5.1 ones. ACPI 
6.3 tables should be used if/when they are needed, and not before.

> I don't know, the maintainers may in fact be in perfect agreement (I 
> certainly do not know) - but certainly at odds with actual developers. 
> So, there appears to be a problem with the way edk2-platforms is run. Or 
> at the very least, I don't understand why it's run the way it's run.
> 
> The RPi3/4 support is developed by a group of folks 
> (https://rpi4-uefi.dev, https://github.com/pftf). These folks (from Arm, 
> from VMware, or unaffiliated) all follow their own goals, with some 
> alignment, but there's overall consensus that working upstream is A Good 
> Thing and is worth the effort. We loosely  coordinate with each other 
> (Discord, have a bug tracker, etc). However, it seems that every patch 
> we propose, to a code base that we largely wrote and chose to upstream 
> into edk2-platforms, gets put under the microscope. That would be fair 
> and square when we're talking about changes to shared components, or 
> code that for technical reasons (quality or design) does not fit the 
> implementation/design goals of Tiano. However, what ends up happening is 
> _maintainers_ taking arbitrary decisions, contrary to the community of 
> developers behind the port. As a final say. I don't mean an abstract 
> mute community of developers. I mean the folks that specifically do all 
> development today for Tiano on Pi platforms. We have no control. We have 
> no say. Maintainers are certainly developers, and may have a stake and 
> interest in the code being reviewed, but at the end of the day they are 
> maintainers - they need to abstract away and agree to disagree on 
> matters that are subjective and opinion-based.
> 

It is funny how it depends on the context whether I am considered just a 
maintainer or one of the developers. Please don't misrepresent my 
involvement as sitting passively on the receiving end of a patch 
pipeline: I wrote the original barebones 64-bit port for Raspberry Pi in 
the first place, and I am [somewhat] active in the development community 
that you started around it.

> Why are maintainers making these calls? We brought these platforms into 
> your garden because we were sold on the idea. Let us water them and take 
> care of them together. Instead, everything we do gets frisked with a 
> strip search. We have to spend our time constantly proving the value of 
> our work instead of using that very little valuable time we have to 
> actually move the Pi support forward.
> 

Upstreaming code is a negotiation (via review) where the contributors 
and the maintainers converge on the terms under which the maintainers 
are willing to take charge of the code, and assume the responsibility 
that it will remain current and in working order going forward.

> The outcomes are pretty serious. If you ask folks in the Arm community, 
> they'll tell you "upstream first", and specifically cite lack of 
> firmware being upstreamed as being a serious problem. However, the way 
> edk2-platforms is structured, that goal is literally unobtainable 
> because edk2-platforms development doesn't scale, as all code gets 
> funneled through the few maintainers. I mean, why would anyone choose to 
> upstream to edk2-platforms? For the lack of positive reinforcement? For 
> the tedious process with no light at the end of the tunnel? It turns 
> developers away. It makes folks feel unvalued and disenfranchised. You 
> get to burn the midnight oil - pro bono - into something only to have 
> others tell you that you should have done something else or just get 
> your proposals ignored. It's not really a community - our choices, our 
> code and our desire to CONTRIBUTE and BE RESPONSIBLE for a proper 
> polished UEFI experience on the Raspberry Pi are worth absolutely 
> nothing here.
> 

Again, can we please cut the drama? I have objected to
a) replacing ACPI 5.1 tables with ACPI 6.3 tables for a piece of 
hardware that is 100% covered by the ACPI 5.1 spec
b) enabling TFTP by default

Beyond that, I don't remember any exchanges where we did not converge in 
the end on something we were all happy with.

> Why does a maintainer have any say over platform-specific code, where 
> there is developer consensus on the trajectory and there are no 
> over-arching technical concerns? Why can't individual platforms have 
> their own maintainers?
> 

If you think 'developer consensus' should be sufficient justification to 
take any change without scrutiny, I think you are looking for a hosting 
platform, not an upstream open source project.

Individual platforms can have their own maintainers. However, 
maintainership is a role you typically have to earn in a community.

What gets lost in the debate is the fact that we are all on the same 
side here: we all want the best possible support for Raspberry Pi 3/4 to 
be available in Tianocore. Escalating a silly difference of opinion such 
as the TFTP issue to these levels only distracts from that.

I have stated before that I *really* like the Raspberry Pi port for its 
polish and looks [none of which are my accomplishments], but the fact 
(intentional or not) that it may become an example followed by others 
when creating EDK2 ports for ARM means that I have a responsibility to 
my employer *and* the community to ensure that it is a good 
approximation of what state of art means for ARM platforms.

So better integration with the UEFI driver model for the onboard 
peripherals is high on my list, as well as moving away from PrePi, so we 
can start thinking about things like Capsule Update and measured boot 
(if you have the hardware).

I am not saying this is your responsibility (or anyone else's) but 
simply that the port is not finished, and so we haven't entered the 
phase yet where we're just dotting Is and crossing Ts. So if enabling 
TFTP support makes it easier to use a board specific hack to update your 
firmware, it shouldn't be enabled by default, and distract from the fact 
that we are still lacking support for UpdateCapsule().

-- 
Ard.


  reply	other threads:[~2020-04-20  6:45 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-19 13:04 [edk2-platform][PATCH v1 0/4] Platform/RaspberryPi : Enable TFTP shell command Samer El-Haj-Mahmoud
2020-04-19 13:04 ` [edk2-platform][PATCH v1 1/4] Platform/RaspberryPi/RPi3: Fix TFTP dynamic command initialization Samer El-Haj-Mahmoud
2020-04-19 13:04 ` [edk2-platform][PATCH v1 2/4] Platform/RaspberryPi/RPi4: " Samer El-Haj-Mahmoud
2020-04-19 13:04 ` [edk2-platform][PATCH v1 3/4] Platform/RaspberryPi/RPi3: Enable TFTP command by default Samer El-Haj-Mahmoud
2020-04-19 13:04 ` [edk2-platform][PATCH v1 4/4] Platform/RaspberryPi/RPi4: " Samer El-Haj-Mahmoud
2020-04-19 13:33 ` [edk2-platform][PATCH v1 0/4] Platform/RaspberryPi : Enable TFTP shell command Ard Biesheuvel
2020-04-19 14:00   ` [edk2-devel] " Samer El-Haj-Mahmoud
2020-04-19 14:04     ` Ard Biesheuvel
2020-04-19 14:19   ` Pete Batard
2020-04-19 19:56     ` Andrei Warkentin
2020-04-19 20:06       ` [edk2-devel] " Pete Batard
2020-04-19 20:21         ` Andrei Warkentin
2020-04-19 20:24           ` Pete Batard
2020-04-19 20:42             ` Andrei Warkentin
     [not found]             ` <160753416257B0CA.29096@groups.io>
2020-04-19 23:50               ` Andrei Warkentin
2020-04-20  1:03                 ` Andrew Fish
2020-04-20  2:41                   ` Andrei Warkentin
2020-04-20  6:45                     ` Ard Biesheuvel [this message]
2020-04-20 11:55                       ` Pete Batard
2020-04-20 12:43                         ` Ard Biesheuvel
2020-04-20 14:18                           ` Pete Batard
2020-04-20 11:23       ` Leif Lindholm
2020-04-20 13:17         ` [edk2-devel] " Samer El-Haj-Mahmoud
2020-04-20 13:24 ` Ard Biesheuvel
2020-04-20 13:30   ` [edk2-devel] " Samer El-Haj-Mahmoud

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=10bee111-6628-e111-1057-9608bbd31521@arm.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