From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web12.1053.1587386589239287581 for ; Mon, 20 Apr 2020 05:43:09 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: ard.biesheuvel@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 755A11FB; Mon, 20 Apr 2020 05:43:08 -0700 (PDT) Received: from [192.168.1.81] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 7FE8B3F237; Mon, 20 Apr 2020 05:43:07 -0700 (PDT) Subject: Re: [edk2-devel] [edk2-platform][PATCH v1 0/4] Platform/RaspberryPi : Enable TFTP shell command To: Pete Batard , awarkentin@vmware.com, Andrew Fish , "devel@edk2.groups.io" Cc: Samer El-Haj-Mahmoud , Leif Lindholm References: <20200419130417.3298-1-samer@elhajmahmoud.com> <8d59e616-9910-4935-2e1f-5da75fc1d34a@arm.com> <831705c6-0915-ba1a-adc0-3078b2af1a43@akeo.ie> <28ac6ff0-86eb-fde7-63f6-c1c7dc0f0724@akeo.ie> <2046f801-45c2-f591-fdf8-18e2edb093d3@akeo.ie> <160753416257B0CA.29096@groups.io> <10bee111-6628-e111-1057-9608bbd31521@arm.com> From: "Ard Biesheuvel" Message-ID: <267c888b-73bb-8a5e-dbd3-17ac25099d55@arm.com> Date: Mon, 20 Apr 2020 14:43:06 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable 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= =20 >> only thing they achieve is losing the ability to boot an OS that=20 >> predates ACPI 6.3. Every piece of the platform currently being=20 >> described via ACPI can be described using 5.1 tables. >=20 > Actually, that is not technically correct. >=20 > But I will concede that you probably aren't aware of this, because we=20 > didn't bring up this point when we discussed the matter earlier. >=20 > One thing you may want to know is that most of the binary blobs we=20 > picked up from the Microsoft IoT endeavour were ACPI 6.0, and,=20 > especially, the MADT we got had actually been abusing the GICR Base=20 > Address field, which was introduced in 6.0, to place then=A0 "nonsensic= al"=20 > value of 1 in there. >=20 > And for reasons that only Microsoft knows, unless you do have a value o= f=20 > 1 there, Windows 10 on the Pi 3 will not boot at all. In other words, i= f=20 > you are to use ACPI 6.x with carry-over defaults from 5.1, Windows 10=20 > can simply not boot, at least on the Pi 3. >=20 > Now, what happened is that, when we started introducing some 5.1 tables= =20 > elsewhere, I went with trying to harmonize everything to 5.1 and=20 > effectively *downgraded* the 6.0 tables we got from Microsoft to 5.1=20 > (which nobody seemed to mind then, on account, I will also assert, that= =20 > nobody realized that we actually had 6.0 tables where 6.0 features migh= t=20 > be relevant). >=20 > It turns out that, if you do use 5.1, Windows 10 *seems* to be happy=20 > about the MADT, even as it is obviously missing a GICR Base Address=20 > field with a 1 value, and *appears* to work as expected. >=20 > However, because of the proprietary nature of the OS, we have absolutel= y=20 > no idea whether having a "properly manipulated" GICR Base Address for=20 > Windows is important or not. The obvious reasoning is that, if Microsof= t=20 > abused that field in their tables and especially if, when using ACPI=20 > 6.x, Windows 10 doesn't boot unless it sees the expected value there, a= n=20 > ACPI 6.x MADT with the relevant value populated might be something that= =20 > we actually want to see. >=20 I agree that it makes sense to harmonize the table versions, and avoid=20 mixing and matching different versions of the spec. The structures are=20 usually defined in a way where new fields added at then end, and the=20 sizes are explicitly defined, so that an older OS can consume newer=20 versions of the tables. At least, that is the theory ... >> ACPI 6.3 tables are not 'better' or 'more current' than 5.1 ones. >=20 > The example above begs to differ (though of course, short of actually=20 > knowing why the heck Microsoft decided to abuse GICR Base Address, it's= =20 > hard to know what 'better' means in this case). >=20 > It does seem to me like Microsoft were effectively using the most recen= t=20 > ACPI version they could use when they produced their IoT tables. And we= =20 > certainly did happen to downgrade some of that, and may have gotten=20 > lucky that it all seemed to work (with the point being that, if we seem= =20 > to be happy that downgrading ACPI tables from 6.0 to 5.1 and validating= =20 > that we aren't observing obvious ill effects is okay, then upgrading=20 > ACPI tables to 6.3 and validating that we aren't observing ill effects=20 > should be fine too). >=20 If compatibility with Microsoft Windows 10 requires a certain minimum=20 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= . >=20 > I disagree here again, on account of trying to showcase elements where=20 > we can, and this is one place we can do this. >=20 > I see absolutely zero technical reasons to want to stick with ACPI 5.1=20 > especially as we already have one 6.3 table (PPTT) and we're going to=20 > have to upgrade a couple others to at least 6.0 anyway to get back to=20 > the position we were in with the Microsoft's blobs, so that we can be=20 > more confident about Windows' behaviour. >=20 > As a matter of fact, using ACPI 6.3 before it is effectively needed is=20 > exactly the kind of move I see as wanting to apply when we know that a=20 > platform is going to be used as a de facto showcase. It may seem like a= =20 > simple "newer is always better!" push to you, but the way I see it is=20 > that, by doing so, we are going to help developers of new platforms, wh= o=20 > will be looking at the Pi for reference (whether we like it or not) and= ,=20 > if they are developing for modern hardware, are going to be looking at=20 > using new features that introduced only in recent ACPI. Thus, even if w= e=20 > don't make any use of these features on the Pi's, those developers migh= t=20 > be grateful to find that there exists a working example of using a=20 > relatively up to date ACPI, and that they won't have to reinvent the wh= eel. >=20 > Especially, as opposed to what might be the case for other platforms, w= e=20 > don't have a baggage of "older" OSes that we may hinder support for if=20 > we do move to ACPI 6.3 (especially as, outside of Windows, most Pi 3=20 > OSes will be using DT rather than ACPI, and, without SD and Genet=20 > support, the idea of older OSes needing to work with Pi 4 is a bit=20 > ludicrous). >=20 > So, whereas I agree that your reasoning is generally sound for most=20 > platforms, the fact that the Pi is going to be used as a de-facto=20 > showcase or starting point by folks interested in developing a new ARM=20 > platform, makes, I will assert, the situation a bit different here, and= =20 > I would really appreciate if you could give some more thoughts to the=20 > points I am trying to make. In this case, I believe that the Pi should=20 > be the "exception that proves the rule". >=20 > Furthermore, and this IMO is the most important point, we are not going= =20 > to be sitting idle if we do upgrade to 6.3 and someone comes to us with= =20 > a report that using modern ACPI appears to be causing them trouble=20 > (which, I will also assert, may actually be something that might be of=20 > great interest to folks on this list if that happens). As opposed to=20 > proprietary platforms, where ACPI is pretty much set in stone by the=20 > vendor and where addressing issues is a struggle, and, even if the=20 > vendor is very reactive and produces an update, where flashing a new=20 > firmware can be a tricky operation for some users, we are Open Source,=20 > easy to reach for reports, and the firmware "update" process of the Pi=20 > could not be any simpler (copy a file to an SD card and you're done). S= o=20 > I'd really like to help squelch the idea that if we do decide to upgrad= e=20 > to ACPI 6.3, and we effectively find out that it causes issues, we're=20 > simply going to stand by and let these unaddressed, whilst telling user= s=20 > of the platform "too bad". None of what we are doing is ever meant to b= e=20 > static or immutable, even after a goal has been reached. Thus, if there= =20 > are issues with upgrading to 6.3, we're going to make darn sure that we= =20 > address them, in the same way we have been planning to address issues=20 > that might be raised from some of our forced downgrades from 6.0 to 5.1= . >=20 Thanks Pete. I still don't agree with the whole showcase aspect of this discussion,=20 but since there are apparently other good reasons to upgrade these=20 tables that nobody brought up before, I am not going to oppose any=20 changes on this front, provided that the commit logs articulate in=20 sufficient detail why we think this is necessary (so that people looking=20 at this port for guidance have the background as well) Out of curiosity, why did we need a 6.3 PPTT for RPi4?