public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Ard Biesheuvel" <ard.biesheuvel@arm.com>
To: Pete Batard <pete@akeo.ie>,
	awarkentin@vmware.com, Andrew Fish <afish@apple.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: 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 14:43:06 +0200	[thread overview]
Message-ID: <267c888b-73bb-8a5e-dbd3-17ac25099d55@arm.com> (raw)
In-Reply-To: <c3f18ffd-6775-0551-e023-27bfa714c667@akeo.ie>

On 4/20/20 1:55 PM, Pete Batard wrote:
> On 2020.04.20 07:45, Ard Biesheuvel wrote:
...
>> 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.
> 
> Actually, that is not technically correct.
> 
> But I will concede that you probably aren't aware of this, because we 
> didn't bring up this point when we discussed the matter earlier.
> 
> One thing you may want to know is that most of the binary blobs we 
> picked up from the Microsoft IoT endeavour were ACPI 6.0, and, 
> especially, the MADT we got had actually been abusing the GICR Base 
> Address field, which was introduced in 6.0, to place then  "nonsensical" 
> value of 1 in there.
> 
> And for reasons that only Microsoft knows, unless you do have a value of 
> 1 there, Windows 10 on the Pi 3 will not boot at all. In other words, if 
> you are to use ACPI 6.x with carry-over defaults from 5.1, Windows 10 
> can simply not boot, at least on the Pi 3.
> 
> Now, what happened is that, when we started introducing some 5.1 tables 
> elsewhere, I went with trying to harmonize everything to 5.1 and 
> effectively *downgraded* the 6.0 tables we got from Microsoft to 5.1 
> (which nobody seemed to mind then, on account, I will also assert, that 
> nobody realized that we actually had 6.0 tables where 6.0 features might 
> be relevant).
> 
> It turns out that, if you do use 5.1, Windows 10 *seems* to be happy 
> about the MADT, even as it is obviously missing a GICR Base Address 
> field with a 1 value, and *appears* to work as expected.
> 
> However, because of the proprietary nature of the OS, we have absolutely 
> no idea whether having a "properly manipulated" GICR Base Address for 
> Windows is important or not. The obvious reasoning is that, if Microsoft 
> abused that field in their tables and especially if, when using ACPI 
> 6.x, Windows 10 doesn't boot unless it sees the expected value there, an 
> ACPI 6.x MADT with the relevant value populated might be something that 
> we actually want to see.
> 

I agree that it makes sense to harmonize the table versions, and avoid 
mixing and matching different versions of the spec. The structures are 
usually defined in a way where new fields added at then end, and the 
sizes are explicitly defined, so that an older OS can consume newer 
versions of the tables.

At least, that is the theory ...

>> ACPI 6.3 tables are not 'better' or 'more current' than 5.1 ones.
> 
> The example above begs to differ (though of course, short of actually 
> knowing why the heck Microsoft decided to abuse GICR Base Address, it's 
> hard to know what 'better' means in this case).
> 
> It does seem to me like Microsoft were effectively using the most recent 
> ACPI version they could use when they produced their IoT tables. And we 
> certainly did happen to downgrade some of that, and may have gotten 
> lucky that it all seemed to work (with the point being that, if we seem 
> to be happy that downgrading ACPI tables from 6.0 to 5.1 and validating 
> that we aren't observing obvious ill effects is okay, then upgrading 
> ACPI tables to 6.3 and validating that we aren't observing ill effects 
> should be fine too).
> 

If compatibility with Microsoft Windows 10 requires a certain minimum 
version of the tables, then I don't mind adhering to that.

>> ACPI 6.3 tables should be used if/when they are needed, and not before.
> 
> I disagree here again, on account of trying to showcase elements where 
> we can, and this is one place we can do this.
> 
> I see absolutely zero technical reasons to want to stick with ACPI 5.1 
> especially as we already have one 6.3 table (PPTT) and we're going to 
> have to upgrade a couple others to at least 6.0 anyway to get back to 
> the position we were in with the Microsoft's blobs, so that we can be 
> more confident about Windows' behaviour.
> 
> As a matter of fact, using ACPI 6.3 before it is effectively needed is 
> exactly the kind of move I see as wanting to apply when we know that a 
> platform is going to be used as a de facto showcase. It may seem like a 
> simple "newer is always better!" push to you, but the way I see it is 
> that, by doing so, we are going to help developers of new platforms, who 
> will be looking at the Pi for reference (whether we like it or not) and, 
> if they are developing for modern hardware, are going to be looking at 
> using new features that introduced only in recent ACPI. Thus, even if we 
> don't make any use of these features on the Pi's, those developers might 
> be grateful to find that there exists a working example of using a 
> relatively up to date ACPI, and that they won't have to reinvent the wheel.
> 
> Especially, as opposed to what might be the case for other platforms, we 
> don't have a baggage of "older" OSes that we may hinder support for if 
> we do move to ACPI 6.3 (especially as, outside of Windows, most Pi 3 
> OSes will be using DT rather than ACPI, and, without SD and Genet 
> support, the idea of older OSes needing to work with Pi 4 is a bit 
> ludicrous).
> 
> So, whereas I agree that your reasoning is generally sound for most 
> platforms, the fact that the Pi is going to be used as a de-facto 
> showcase or starting point by folks interested in developing a new ARM 
> platform, makes, I will assert, the situation a bit different here, and 
> I would really appreciate if you could give some more thoughts to the 
> points I am trying to make. In this case, I believe that the Pi should 
> be the "exception that proves the rule".
> 
> Furthermore, and this IMO is the most important point, we are not going 
> to be sitting idle if we do upgrade to 6.3 and someone comes to us with 
> a report that using modern ACPI appears to be causing them trouble 
> (which, I will also assert, may actually be something that might be of 
> great interest to folks on this list if that happens). As opposed to 
> proprietary platforms, where ACPI is pretty much set in stone by the 
> vendor and where addressing issues is a struggle, and, even if the 
> vendor is very reactive and produces an update, where flashing a new 
> firmware can be a tricky operation for some users, we are Open Source, 
> easy to reach for reports, and the firmware "update" process of the Pi 
> could not be any simpler (copy a file to an SD card and you're done). So 
> I'd really like to help squelch the idea that if we do decide to upgrade 
> to ACPI 6.3, and we effectively find out that it causes issues, we're 
> simply going to stand by and let these unaddressed, whilst telling users 
> of the platform "too bad". None of what we are doing is ever meant to be 
> static or immutable, even after a goal has been reached. Thus, if there 
> are issues with upgrading to 6.3, we're going to make darn sure that we 
> address them, in the same way we have been planning to address issues 
> that might be raised from some of our forced downgrades from 6.0 to 5.1.
> 

Thanks Pete.

I still don't agree with the whole showcase aspect of this discussion, 
but since there are apparently other good reasons to upgrade these 
tables that nobody brought up before, I am not going to oppose any 
changes on this front, provided that the commit logs articulate in 
sufficient detail why we think this is necessary (so that people looking 
at this port for guidance have the background as well)

Out of curiosity, why did we need a 6.3 PPTT for RPi4?




  reply	other threads:[~2020-04-20 12:43 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
2020-04-20 11:55                       ` Pete Batard
2020-04-20 12:43                         ` Ard Biesheuvel [this message]
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=267c888b-73bb-8a5e-dbd3-17ac25099d55@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