[RFC] Submitting RaspberryPiPkg to edk2-platforms

[pbatard]

Hi Andrei,

I am planning to submit your RaspberryPiPkg for integration into edk2-platforms, since I think it's gotten stable enough to try to officialize things and I am also hoping that having an inexpensive and widely popular ARM platform there will help getting more people interested in EDK2/UEFI development.

Before submitting your code for integration however, I want to give everyone interested an opportunity to comment on the proposal, a version of which can be found at https://github.com/pbatard/RaspberryPiPkg

Now, to try to keep things short, the points of interest I would like to bring up on that proposal are the following:

• I have obviously removed all the files that would be deemed unnecessary for edk2-platform submission, such as the prebuilt binaries or transient files like leap422_unpatched.dts.
• I have rewritten the Readme so that it matches what I expect edk2 will want for submission (as defined here).
• Following what is done for VExpressPkg, I have moved the Leap device tree tables into a DeviceTree/ subdirectory at the root of the package.
• For the time being, I have moved the precompiled Arm Trusted Firmware blobs as the sole content of the Binary/ directory. When submitting to EDK2, I am planning to remove the Binary/ directory altogether and instead submit these against edk2-non-osi, as this is what the other platforms that provide ATF binaries do, such as the Hisilicon D03. Note that I have no plans of trying to update the ATF binaries against the latest ARM Trusted Firmware tree.
• Likewise, since you point out that the license for DwUsbHostDxe is no BSD licensed, I am planning to remove that specific driver from the current package, and submit it to a mirror directory in edk2-non-osi (which is what you are supposed to do for these kind of items).
• After some consideration, I am proposing that the package should exist under Platform/Broadcom/Bcm283x/ in the edk2-platforms repo. The reasoning behind this is that I see is no reason Raspberry Pi clones, based on the same CPUs, could not be handled under that same tree, and therefore, it is probably best to avoid explicitly mentioning Raspberry Pi in the subfolders (but of course, I see no issue with the package being called RaspberryPiPkg, which is why the .dsc/.fdf have not changed names). As to the use of Bcm283x rather than Bcm2837, this is because I see that there is some effort to potentially bring the firmware to Bcm2836, and I also expect future iterations of the Raspberry Pi, which we'll probably be able to handle under the same platform, to keep with the Bcm283x lineage. Obviously, it means that all the references from the .dsc and .inf that used RaspberryPiPkg/ had to be changed. But then again, since the package would at the very least have had to move under Platform/ or Silicon/ to conform to the edk2-platforms conventions, those would have needed to be edited anyway. So I hope using Platform/Broadcom/Bcm283x/ as the official directory where that platform should reside is seen as acceptable to all.
• The current proposal should build fine against the latest edk2/edk2-platforms without requiring the application of any edk2 patches. For the record, I removed the need for 0002-BaseTools-tools_def-support-ASLC-files-on-AArch64.patch by duplicating the compiler options used by AMD's Overdrive board, which successfully compiles ACPI tables without requiring edk2 patching. I also don't see 0005 being needed at all (if it was, I would expect a build failure, which I am definitely not seeing).
• On the subject of ACPI tables, I found zero need for running the pre-built script that generates and copies DSDT.offset.h (which I can't help but find strange). Is this still needed? Neither the build process nor Windows 10 ARM64 seem to be affected by DSDT.offset.h being missing. At any rate, I believe that, should the header actually be needed, rather than rely on a pre-build script, we should be able to take a page from AMD's Styx, that references a <Dsdt.offset.h> generated by the ASL compiler by simply adding something like -I$(BIN_DIR)/Silicon/AMD/Styx/AcpiTables/AcpiAml/OUTPUT to its CC flags.
• Now, obviously, not applying patches 0002 and 0003 means the progress bar and the logo don't display properly. But since these aren't showstoppers for boot, I'd rather handle those after we have the package in edk2-platforms, as there should be better traction to fix these non-blocking items then, when they can be officially being presented as bugs.
• The only edk2 patch that I have a question at the moment is therefore 0001. My understanding is that, just like 0002 and 0003, if it is symptomatic of an actual issue, it is a non-blocking one that can be addressed at a later date. But if you see it differently, please let me know.
• I have tested both the DEBUG and RELEASE of the RPI_EFI.fd binary (compiled in the manner detailed under the "Building" section of my Readme) with a Pi 3 Model B with Windows 10 on ARM (running from a uSD card), and saw no issue. Especially, as far as I could see, Windows seemed to be happy enough about the ACPI data.
• To make compilation under current edk2 work, I had to fix an undefined reference, but this was no big deal.

At this stage, depending on the feedback provided here, I am aiming at starting submitting patches for edk2-platforms integration around the middle of next week.

I'll conclude by saying a massive THANKS to Andrei and everybody who contributed to making a usable UEFI firmware for the Raspberry Pi a reality, which is something I have been wanting to see for a long time. Hopefully, we can now push it to the next step and make its formal integration into EDK2 a reality...

[pbatard]

Just to mention that a v2 of the EDK2 patch was submitted, since they wanted a more manageable break down of the components.

Once they start reviewing it in earnest, we'll probably need to submit a few more versions, but that's the just the regular EDK2 submission workflow.

[andreiw]

Congrats! Very cool.

P.S. I've not had a chance to fix HypDxe yet but I'll get there soon :).

A

On Mon, Dec 10, 2018 at 12:45 PM Pete Batard notifications@github.com wrote:

Just to mention that a v2 of the EDK2 patch https://lists.01.org/pipermail/edk2-devel/2018-December/033686.html was submitted, since they wanted a more manageable break down of the components.

Once they start reviewing it in earnest, we'll probably need to submit a few more versions, but that's the just the regular EDK2 submission workflow.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/andreiw/RaspberryPiPkg/issues/88#issuecomment-445905964, or mute the thread https://github.com/notifications/unsubscribe-auth/AAsta4n4qUO2jtIPGUCpY2zxWzgt-IP5ks5u3p2ngaJpZM4YKFNy .

-- A

[pbatard]

No worries about HypDxe. We can sort that out after edk2 integration.

Just one question, that I wondered about and that also got raised from the initial patches review: What does AddAnd mean in the AddAndRTSData() (and other AddAnd...) function calls of MemoryInitPeiLib.c?

I think I can address the other points that were raised (see Ard's latest replies here), but that one question I can't answer.

[andreiw]

MemoryPeim function is supposed to create the memory map.

AddAndXXX creates a new resource range and then immediately reserves it away with a certain memory allocation type. There are multiple of these, because different ranges call for different resource range attributes (such as cache-ability) and of course different types.

The Variable portion of the FD has to be accessible after ExitBootServices, and so must be reserved as RuntimeServicesData.

A

13 дек. 2018 г., в 5:23, Pete Batard notifications@github.com написал(а):

No worries about HypDxe. We can sort that out after edk2 integration.

Just one question, that I wondered about and that also got raised from the initial patches review: What does AddAnd mean in the AddAndRTSData() (and other AddAnd...) function calls of MemoryInitPeiLib.c?

I think I can address the other points that were raised (see Ard's latest replies here), but that one question I can't answer.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[andreiw]

Yes, you can drop the MMIO range, as its not used in RT services. (Don’t drop it from the ARM_MEMORY_REGION_DESCRIPTOR, though, if you like booting - it needs to be mapped in the page tables)

A

13 дек. 2018 г., в 5:23, Pete Batard notifications@github.com написал(а):

No worries about HypDxe. We can sort that out after edk2 integration.

Just one question, that I wondered about and that also got raised from the initial patches review: What does AddAnd mean in the AddAndRTSData() (and other AddAnd...) function calls of MemoryInitPeiLib.c?

I think I can address the other points that were raised (see Ard's latest replies here), but that one question I can't answer.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[pbatard]

For those who wonder what's the status on the EDK2 submission, I'm just going to point out that I am still working on the v3 patch submission process and that you can follow the progress of it through the corresponding github project on my branch.

Because I'm a bit busy with other stuff, and splitting the content between Platform and Silicon is likely to take some time, I probably won't be able to submit the v3 until next week at the earliest.

[andreiw]

Thanks for the update. I still owe you a HypDxe fix. Time...

[pbatard]

v3 patchset has now been posted to edk2 mailing list (see here). Sorry it took so long.

I'll start working on a new v4 patchset right away, since we have the usual set of additional requests, but one thing I could use some help with is the feedback we just got on the ACPI tables:

• Looks like the _CID (Compatible ID) name string need to follow a specific format (4 letters + 4 digits? I'm still trying to figure out where this is documented...), so we need to change that. However, I'd rather have you pick the IDs you think will suit us best, rather than improvise stuff on my own, since you're more familiar with the platform.
• Ard also wonders is there's any chance we could make these ACPI tables compliant with the 6.0+ specs, since they appear to be 5.0 and rather Windows centric. This looks unlikely to me (or at least, it's not something I can see myself doing on my own in any reasonable timeframe) and it is not a showstopper, but I'd still like to have your thoughts on that.

[pbatard]

Also, I got the following question (off list):

Arasan implements the SDHCI spec and edk2 has a driver for this already - Can you use the NonDiscoverbleDevice infrastructure to use the existing driver and avoid introducing another one?

Any thoughts on that?

[andreiw]

Yeah, the Arasan controller has a bunch of nasty quirks like using 32-bit only accesses.

I am not saying it's impossible, but the stock SDHCI driver will not work, and it will take a serious effort to get anywhere. Effort that I am sure no one from the "helpful reviewer camp" wants to front.

A

On Mon, Jan 28, 2019 at 5:52 PM Pete Batard notifications@github.com wrote:

Also, I got the following question (off list):

Arasan implements the SDHCI spec and edk2 has a driver for this already https://github.com/tianocore/edk2/tree/master/MdeModulePkg/Bus/Sd/SdBlockIoPei

• Can you use the NonDiscoverbleDevice infrastructure to use the existing driver and avoid introducing another one?

Any thoughts on that?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/andreiw/RaspberryPiPkg/issues/88#issuecomment-458334769, or mute the thread https://github.com/notifications/unsubscribe-auth/AAsta5EkETthLDj2PBrJqqte31CqJokuks5vH387gaJpZM4YKFNy .

-- A

[andreiw]

Yikes. v4? Sorry you have to deal with this. You know, considering the general quality of stuff in edk2, I'm curious why there's so much cage rattling going on. Surely once the big items (e.g. code structure) are in, it's easier to fix the rest of the nits in place, esp. when many of these (e.g. SDHCI) require domain experience or just plain effort that a single person can't meaningfully provide, pro bono, in a reasonable time frame. If you get frustrated and give up, what's the value behind any of these "helpful reviews?"

The ACPI feedback you got is exactly useless. There is only one OS on the planet, publicly, that these will boot. The tables came from Microsoft. They are not even ACPI compliant (there's no definition of ACPI that includes the RPI interrupt controller!). You do as little as breathe at them the wrong way, and that only OS (Windows) will probably not boot - I remember having to maintain the compiler IDs across iasl invocations to match the M$ ones. It was a nightmare, because the wrong IDs meant the system was not being detected as the Pi (with obvious bugcheck consequences, completely undebuggable without symbols).

We can't randomly change the PNP IDs, just because Ard would like it that way, because the Microsoft Windows drivers expect those exact IDs.

A

On Mon, Jan 28, 2019 at 11:38 AM Pete Batard notifications@github.com wrote:

v3 patchset has now been posted to edk2 mailing list (see here https://lists.01.org/pipermail/edk2-devel/2019-January/thread.html#35789). Sorry it took so long.

I'll start working on a new v4 patchset right away, since we have the usual set of additional requests, but one thing I could use some help with is the feedback we just got on the ACPI tables https://lists.01.org/pipermail/edk2-devel/2019-January/035798.html:

• Looks like the _CID (Compatible ID) name string need to follow a specific format (4 letters + 4 digits? I'm still trying to figure out where this is documented...), so we need to change that. However, I'd rather have you pick the IDs you think will suit us best, rather than improvise stuff on my own, since you're more familiar with the platform.
• Ard also wonders is there's any chance we could make these ACPI tables compliant with the 6.0+ specs, since they appear to be 5.0 and rather Windows centric. This looks unlikely to me (or at least, it's not something I can see myself doing on my own in any reasonable timeframe) and it is not a showstopper, but I'd still like to have your thoughts on that.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/andreiw/RaspberryPiPkg/issues/88#issuecomment-458204841, or mute the thread https://github.com/notifications/unsubscribe-auth/AAsta_cau9MjYlfb_lWqrM8_ksO7xII6ks5vHyd9gaJpZM4YKFNy .

-- A

[pbatard]

Thanks for the updates. I kind of figured as much for the Arasan driver replacement, and I had a hunch that the ACPI changes might be a lot more problematic than Ard thinks they are. Can't blame the EDK2 people for trying to see how much compliance and reuse we can squeeze into the Pi UEFI firmware, especially as a lot of people may come to look at that firmware due to the Pi popularity. So they want to make sure that things are as close to a nice spotless reference platform as can be. But of course, the Pi hardware and the ACPI implementation Microsoft chose to use will alter that deal quite a bit...

With this new feedback, I should be able to submit a v4 later on today.

[andreiw]

I don't think it's ever going to be spotless, and frankly, that's still better than what they have today for Pi (nothing at all) or some of the other embedded boards (usually just UART + MMC)

Some obvious areas that need work, in case there are folks yearning to do something on the Pi:

• Adopting a shimmed FreeBSD driver for dwc2, possibly improved for DMA use, is likely the shortest path to fixing all sorts of shenanigans with USB1 devices
• SdHost and Arasan drivers ought to have the sd/mmc-specific command support factored out into an MmcDxe replacement. Shoehorning Arasan or SdHost (the later is not SDHCI-related at all) into the big ol' blob of Intel SDHCI support out of MdeModulePkg is probably a mistake (unless someone really wants to use this as a well-funded exercise to turn that SDHCI stack into a generic SDMMC stack... not a bad idea).

A

On Tue, Jan 29, 2019 at 7:05 AM Pete Batard notifications@github.com wrote:

Thanks for the updates. I kind of figured as much for the Arasan driver replacement, and I had a hunch that the ACPI changes might be a lot more problematic than Ard thinks they are. Can't blame the EDK2 people for trying to see how much compliance and reuse we can squeeze into the Pi UEFI firmware, especially as a lot of people may come to look at that firmware due to the Pi popularity. So they want to make sure that things are as close to a nice spotless reference platform as can be. But of course, the Pi hardware and the ACPI implementation Microsoft chose to use will alter that deal quite a bit...

With this new feedback, I should be able to submit a v4 later on today.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/andreiw/RaspberryPiPkg/issues/88#issuecomment-458516016, or mute the thread https://github.com/notifications/unsubscribe-auth/AAsta56V30ja6Utvp5rb9AlZnOoaGdbqks5vIDjugaJpZM4YKFNy .

-- A

[pbatard]

I have now posted a v4 patchset.

Quite frankly, I don't think the "pushback" from the EDK2 is that bad, especially compared to some other projects that are a way more autocratic in their decisionmaking. At least the EDK2 feedback is rational and looks fair to me since, of course, they can't know everything about how the source took shape, so it's natural they'll ask about things that they believe might be improved, while not realizing that it's not as easy as it may seem. Plus some of the elements they asked to fix were problems that I introduced myself (such as trying to slide the GpioLib function defs in IndustryProtocol to spare us a header, which I reckon wasn't probably that great an idea).

Besides, I went into this expecting that it'll probably take us at least 6 pacthset submissions before we get the platform integrated, so I knew what I was signing for...

And just so you know, Ard, who has now tested a build on real hardware, is very enthusiastic about it. 😄

[hansmbakker]

@pbatard it looks like you (and others too, but I see you driving this recent work on the mailing list) did a tremendous effort, well done! Really nice to get such feedback, too!

And in general - thanks to all people working on this repo! Knowledge of these low-level things is not that common, so it's really nice that you spend your effort on improving RPi support! I think if this is merged in upstream, it is a really well-deserved :+1: for all maintainers here!

[andreiw]

Good! Glad people like it - that’s definitely incentive to push on.

I had recently fixed the weird handling of boot options (esp between 3B/3B+ and between Arasan/SdHost), and will also soon push a DisplayDxe/ConfigDxe improvement around virtual resolution handling, which will make the 7” Pi screen actually usable.

A

29 янв. 2019 г., в 16:11, Pete Batard notifications@github.com написал(а):

I have now posted a v4 patchset.

Quite frankly, I don't think the "pushback" from the EDK2 is that bad, especially compared to some other projects that are a way more autocratic in their decisionmaking. At least the EDK2 feedback is rational and looks fair to me since, of course, they cant' know everything about how the source took shape, so it's natural they'll ask about things that they believe might be improved, while not realizing that it's notas easy as it may seem. Plus some of the elements they asked to fix were problems that I introduced myself (such as trying to slide the GpioLib function defs in IndustryProtocol to spare us a header, which I reckon wasn't probably that great an idea).

Besides, I went into this expecting that it'll probably take us at least 6 pacthset submissions before we get the platform integrated, so I knew what I was signing for...

And just so you know, Ard, who has now tested a build on real hardware, is very enthusiastic about it. 😄

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[pbatard]

Just going to drop a link to the v5 that was submitted about a week ago. I expect this one to take a bit longer to be reviewed, as it also depends on an patch that moves VirtualRealTimeClockLib into edk2's EmbeddedPkg.

Now, it is possible that these 2 patchsets are the ones that finally lead to RPi being integrated as a platform into the EDK2, but I wouldn't hold my breath... Still, even if the process is slow, things are continuing to move steadily towards achieving that goal.

[pbatard]

And we're finally in!! 😄

As of today, if you pull the latest edk2, edk2-platforms and edk2-non-osi, then you can build the RPi3 firmware right off the authoritative UEFI Open Source repositories. Isn't that just awesome? 😉

Andrei, I really can't thank you enough for the work you did on this project and all the help you provided. Even if it may have felt slow at times, turning your repository into something that the EDK2 would accept was a relative breeze, so you really did an outstanding job in the first place here!

And while I did have to rework a few things, some of which might make the EDK2 version look quite different from yours in some places (mostly due to reworking the code style, in order to keep the EDK2 folks happy), I hope you too will be thrilled with the result.

Oh, and just so you know, more EDK2 people have come forward to indicate how impressed they are after having tested the firmware.

Since there's nothing more to do here, I will therefore close this issue.

[andreiw]

Thanks Pete, this is an amazing achievement. Thank you for your work.

Aside from stylistical changes and HypDxe, what are the functional differences?

[pbatard]

From looking at the history from https://github.com/pbatard/RaspberryPiPkg/commits/master this is what I can come up with:

• VirtualRTC was dropped (now comes from EDK2)
• As part of the above, need to pass BUILD_EPOCH during build was dropped (the epoch is set automatically when building from a shell environment)
• Utils.h was dropped
• Some function names in MemoryInit were clarified and AddAndMmio() was dropped
• PcdFirmwareVersionString is set to edk2-1.0 to indicate that this is the first major version being built from the EDK2 source
• BUILD_DATE and BUILD_COMMIT were dropped (__DATE__ + __TIME__ should be fine to insert build date)
• IScsiDxe was dropped due to a recent EDK2 change that broke the build, and some networking dependencies were changed in the .dsc/.fdf
• NOOPT was added to BUILD_TARGETS
• Mbox usage was made more explicit (some headers were renamed, some RPI_FW_### defines were changed to RPI_MBOX_###
• DW2_USB_BASE_ADDRESS was moved to DwcHw.h
• Some sources (e.g. RpiFdtDxe) were renamed to drop the Rpi, RaspberryPi or Bcm2836 prefix
• Official ATF is used
• Source is split between Platform/RaspberryPi/RPi3/ and Silicon/Broadcom/Bcm283x

There's probably more stuff that I'm forgetting, but this should give you some idea. Overall no real major changes.

By the way, one thing I would advise you to do, instead of committing your firmware binaries in the source itself, which is a practice that's generally frowned upon because it forces everyone who wants to clone or fork the source to carry a lot of cumbersome deadweight, is to instead use the GitHub Releases feature to upload these same binaries, like I did here for instance. Unless they are needed for the build, there's no good reason to include binaries in a source, especially when working with a GitHub repo.

Also, if you'd like, now that the platform has been integrated, I would also encourage you to create your own clone of edk2-platforms and upload GitHub Releases for the EDK2 binaries there, so that you can point people to trying these alternate versions if they want to. Of course, I could do that myself too, but I think it makes more sense if you were the one to do it, as you have come to be known as the provider of UEFI RPi firmwares (and for good measure, since it's mostly your work :wink:).

[andreiw]

Thanks Pete - that’s the plan, I will re-host myself in top of your upstream now and start providing builds the preferred way.

One question: Who is the maintainer for the upstream? Again, sadly, I’ll barely have time to continue adding value to the firmware, which excludes upstreaming activities for further work...

[pbatard]

Well, as per Maintainers.txt, Ard. Leif and Michael (who all participated in the review process) are now the official maintainers of the platform.

But I guess your question is more "Who will take the job of sending patches and fixes I might add to my repo to EDK2 upstream?" in which case, provided I have the time, I will try to continue to do so.

However, as you have seen, this takes quite a bit of time, and I too will be trying to wind down my RPi related activities so that I can concentrate on other matters, so I can't promise anything...

[pbatard]

As a matter of fact, I'm going to point out that Ard is already submitting improvements of his own against the platform, such as adding a Random Number Generator driver, so, you'll definitely want to monitor upstream for changes...

[andreiw]

Ok, thanks

A

16 февр. 2019 г., в 3:07, Pete Batard notifications@github.com написал(а):

As a matter of fact, I'm going to point out that Ard is already submitting improvements of his own against the platform, such as adding a Random Number Generator driver, so, you'll definitely want to monitor upstream for changes...

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub, or mute the thread.

[andreiw]

What does it take to be added to Maintainers.txt? Or is it not per platform, but global?

[pbatard]

It's global. AFAIK, the regular edk-2platforms contributors and maintainers tend to take ownership, since there's no guarantee that the person who submitted a platform is actually going to stick around.