| Message ID | 4b647c0d-86de-fed3-aca6-d248b9aff762@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kernel after 5.0-rc2 may not boot using BBL | expand |
On Thu, Feb 07, 2019 at 10:08:14PM -0800, Atish Patra wrote: > If you are building a larger kernel image, it may not boot using BBL after > 5.0-rc2. It works fine with OpenSBI though. > > Here is the reasoning in case anybody else is facing the same issue. > > The following patch was added 5.0-rc2 which helps reduce the flat image size > for Linux kernel. > > "22e6a2e1 RISC-V: Make BSS section as the last section in vmlinux.lds.S" > > As the BSS section is moved to the end of the ELF, flat image will not > include it. As BBL copies the DTB at the next 2MB page boundary, BSS > section may overlap with DTB address. Kernel clears BSS section during early > boot which may clear the DTB as well leading to a boot failure. Breaking everyones (qemu) setups that we only got working two release ago is really annoying. I'd suggest reverting the commit for now, or at least making it an optional config option.
On Fri, Feb 8, 2019 at 2:02 PM Christoph Hellwig <hch@infradead.org> wrote: > > On Thu, Feb 07, 2019 at 10:08:14PM -0800, Atish Patra wrote: > > If you are building a larger kernel image, it may not boot using BBL after > > 5.0-rc2. It works fine with OpenSBI though. > > > > Here is the reasoning in case anybody else is facing the same issue. > > > > The following patch was added 5.0-rc2 which helps reduce the flat image size > > for Linux kernel. > > > > "22e6a2e1 RISC-V: Make BSS section as the last section in vmlinux.lds.S" > > > > As the BSS section is moved to the end of the ELF, flat image will not > > include it. As BBL copies the DTB at the next 2MB page boundary, BSS > > section may overlap with DTB address. Kernel clears BSS section during early > > boot which may clear the DTB as well leading to a boot failure. > > Breaking everyones (qemu) setups that we only got working two release > ago is really annoying. I'd suggest reverting the commit for now, or > at least making it an optional config option. It is a bug in BBL because it is not keeping sufficient gap between Linux kernel end and DTB. The change pointed by Atish actually reduced flat kernel binary size by almost 1MB. Rather than: 1. First revert in Linux 2. Fix in BBL 3. Bring back in Linux I would suggest just do: 1. Fix in BBL Here's my BBL pull request: https://github.com/riscv/riscv-pk/pull/144 Regards, Anup > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On Fri, Feb 08, 2019 at 02:32:51PM +0530, Anup Patel wrote: > It is a bug in BBL because it is not keeping sufficient gap between > Linux kernel end and DTB. The change pointed by Atish actually > reduced flat kernel binary size by almost 1MB. And that is all great. But it breaks peoples few existing RISC-V setups, which is the best way to frustate every single of the few users out there. And that is best way to lose any interested in this already hard to use mess. > Here's my BBL pull request: > https://github.com/riscv/riscv-pk/pull/144 And that isn't going to help anyone who doesn't want to update every single fragile piece of crap involved in booting RISC-V every day..
On Fri, Feb 8, 2019 at 2:43 PM Christoph Hellwig <hch@infradead.org> wrote: > > On Fri, Feb 08, 2019 at 02:32:51PM +0530, Anup Patel wrote: > > It is a bug in BBL because it is not keeping sufficient gap between > > Linux kernel end and DTB. The change pointed by Atish actually > > reduced flat kernel binary size by almost 1MB. > > And that is all great. But it breaks peoples few existing RISC-V > setups, which is the best way to frustate every single of the few > users out there. And that is best way to lose any interested in this > already hard to use mess. There is no issue if you are building Linux RISC-V defconfig. The issue starts manifesting only if you more drivers enabled. Again, with growing kernel not everyone will see this issue because it will only show-up only when gap between payload_end and next 2MB aligned address is not sufficient to cover BSS section. Regards, Anup
On Fri, 08 Feb 2019 01:17:00 PST (-0800), anup@brainfault.org wrote: > On Fri, Feb 8, 2019 at 2:43 PM Christoph Hellwig <hch@infradead.org> wrote: >> >> On Fri, Feb 08, 2019 at 02:32:51PM +0530, Anup Patel wrote: >> > It is a bug in BBL because it is not keeping sufficient gap between >> > Linux kernel end and DTB. The change pointed by Atish actually >> > reduced flat kernel binary size by almost 1MB. >> >> And that is all great. But it breaks peoples few existing RISC-V >> setups, which is the best way to frustate every single of the few >> users out there. And that is best way to lose any interested in this >> already hard to use mess. > > There is no issue if you are building Linux RISC-V defconfig. > > The issue starts manifesting only if you more drivers enabled. > Again, with growing kernel not everyone will see this issue because > it will only show-up only when gap between payload_end and next > 2MB aligned address is not sufficient to cover BSS section. I agree with Christoph here: we should revert the commit. There are various versions of BBL floating around that are a big headache to update, and since all the patch in question does is reduce the flat image size it's not worth breaking existing setups. I also don't like the proposed fix, as that just statically sizes a 16MiB BSS. While that's fairly large, it is in essence just kicking the can down the road. The kernel on my laptop has a 7MiB BSS, and it's just an arbitrary smattering of the drivers I happen to use. I'm not sure what the right long-term solution is here. I kind of want to say "just use an ELF", as someone has already sat down and figured out how to put everything into a file that's required to make a binary run. I feel like doing anything else here is really just going to involve re-inventing the wheel. That's a bigger discussion, I've sent out the revert and will include it next week unless someone has an objection.
On Fri, Feb 8, 2019 at 10:59 PM Palmer Dabbelt <palmer@sifive.com> wrote: > > On Fri, 08 Feb 2019 01:17:00 PST (-0800), anup@brainfault.org wrote: > > On Fri, Feb 8, 2019 at 2:43 PM Christoph Hellwig <hch@infradead.org> wrote: > >> > >> On Fri, Feb 08, 2019 at 02:32:51PM +0530, Anup Patel wrote: > >> > It is a bug in BBL because it is not keeping sufficient gap between > >> > Linux kernel end and DTB. The change pointed by Atish actually > >> > reduced flat kernel binary size by almost 1MB. > >> > >> And that is all great. But it breaks peoples few existing RISC-V > >> setups, which is the best way to frustate every single of the few > >> users out there. And that is best way to lose any interested in this > >> already hard to use mess. > > > > There is no issue if you are building Linux RISC-V defconfig. > > > > The issue starts manifesting only if you more drivers enabled. > > Again, with growing kernel not everyone will see this issue because > > it will only show-up only when gap between payload_end and next > > 2MB aligned address is not sufficient to cover BSS section. > > I agree with Christoph here: we should revert the commit. There are various > versions of BBL floating around that are a big headache to update, and since > all the patch in question does is reduce the flat image size it's not worth > breaking existing setups. > > I also don't like the proposed fix, as that just statically sizes a 16MiB BSS. > While that's fairly large, it is in essence just kicking the can down the road. > The kernel on my laptop has a 7MiB BSS, and it's just an arbitrary smattering > of the drivers I happen to use. I'm not sure what the right long-term solution > is here. I kind of want to say "just use an ELF", as someone has already sat > down and figured out how to put everything into a file that's required to make > a binary run. I feel like doing anything else here is really just going to > involve re-inventing the wheel. IMHO, best solution would be to use mature open-source bootloader as payload (i.e. U-Boot or similar) and always boot Linux from bootloader. Using Linux as BBL/OpenSBI payload can still continue to exist but it can be more of a developer usage. Regards, Anup
On Sat, Feb 09, 2019 at 09:00:27AM +0530, Anup Patel wrote: > IMHO, best solution would be to use mature open-source bootloader as > payload (i.e. U-Boot or similar) and always boot Linux from bootloader. Using > Linux as BBL/OpenSBI payload can still continue to exist but it can be more > of a developer usage. Yes, that is a good long term plan. But in the meantime we should not gratiously break the precious setups people got working..
On Mon, Feb 11, 2019 at 1:55 PM Christoph Hellwig <hch@infradead.org> wrote: > > On Sat, Feb 09, 2019 at 09:00:27AM +0530, Anup Patel wrote: > > IMHO, best solution would be to use mature open-source bootloader as > > payload (i.e. U-Boot or similar) and always boot Linux from bootloader. Using > > Linux as BBL/OpenSBI payload can still continue to exist but it can be more > > of a developer usage. > > Yes, that is a good long term plan. But in the meantime we should not > gratiously break the precious setups people got working.. The kernel breakage totally unintentional. I had done all testing using BBL and OpenSBI with Linux compiled using defconfig but never saw the issue. Regards, Anup
On Mon, 11 Feb 2019 00:25:06 PST (-0800), Christoph Hellwig wrote: > On Sat, Feb 09, 2019 at 09:00:27AM +0530, Anup Patel wrote: >> IMHO, best solution would be to use mature open-source bootloader as >> payload (i.e. U-Boot or similar) and always boot Linux from bootloader. Using >> Linux as BBL/OpenSBI payload can still continue to exist but it can be more >> of a developer usage. > > Yes, that is a good long term plan. But in the meantime we should not > gratiously break the precious setups people got working.. I agree. I don't think anyone wants to move away from BBL more than I do, and while the OpenSBI+u-boot stuff is progressing well we can't just instantly deprecate every other boot flow -- particularly not the one that I use :).
On 11/02/19 9:45 PM, Anup Patel wrote: > On Mon, Feb 11, 2019 at 1:55 PM Christoph Hellwig <hch@infradead.org> wrote: >> >> On Sat, Feb 09, 2019 at 09:00:27AM +0530, Anup Patel wrote: >>> IMHO, best solution would be to use mature open-source bootloader as >>> payload (i.e. U-Boot or similar) and always boot Linux from bootloader. Using >>> Linux as BBL/OpenSBI payload can still continue to exist but it can be more >>> of a developer usage. >> >> Yes, that is a good long term plan. But in the meantime we should not >> gratiously break the precious setups people got working.. This has been a longstanding issue. I believe some Berkeley graduates are in control of the repos (some of them are very intelligent young Doctors). However, the Berkeley software doesn't even version numbers. We can't call the release management shocking because it is non-existent. i.e.it is not even at the linux-0.11 style versioning scheme, (although Linus didn't have version control hence tarball's). Methinks they don't want CVEs being assigned as that would taint their impeccable security reputation. For example, one can't really assign a CVE to master:HEAD on a git repo. One needs an actual release. I am sure it is deliberate. That said, assigning CVEs to students, with them having a black mark at the start of their careers is probably also a bad idea. I think these gratuitous breakages are a ploy, either to demonstrate how easily trust can be subverted. i.e. the breakages are actually to troll long-standing kernel overdevelops, showing how easy it is to slip in breaking changes using the "trusted lieutenants" model, and it is not really fixable until someone with release engineering experience takes control of the foundation repositories, or the students step up and take responsibility for their work, adding due professionalism, such as CI. They are getting paid now. Ideal situation would be gated repository with CI in front that locks out changes that regress any of a suite of tests run against the open source tools (and now simulation of the underlying open source chip and its firmware), core libraries and the kernel. I am aware that some of the breakages are deliberate based on traces in GitHub issues, and artifacts in the git history. This century's beginning is marked by the ubiquity of CI, well, certainly the last decade, so I see no reason why it should not be applied here, as these changes would not have made it into a project that has CI. Twerps. They need a punch. Not a gut punch. Just a friendly but firm punch, in the shoulder. Serious, but not hard enough to cause any permanent injury. In the right shoulder because the left shoulder has bursitis, and not onto the bone as that would hurt one's fist. We need to think about how to achieve this with a little finesse. Phone books don't leave bruises but they are bulky to carry around, especially if you lure one of these developers into a bar in a social setting. The Phone book will look out of place. I think just a regular firm fist punch would be best. We don't want assault charges and we certainly wouldn't want an email trail leading back to someone threatening punches to key members of the RISC-V community. That could be damaging (insert emoji :Grinning Face With Smiling Eyes:). Anyway, they can't get to me because I am in another country, so probability of me receiving a physical punch is very low, so just a virtual punch, and it would be a round of exchanging punches. Like I punch you, you punch me back sort of thing. i.e. it needs to be fair. > The kernel breakage totally unintentional. I had done all testing using BBL > and OpenSBI with Linux compiled using defconfig but never saw the issue. I agree with Christoph. It is no excuse. This is bad because it shows lack of basic testing. It's like nobody boot tests riscv-qemu with BBL, which is where all of the work on PMP is. In this century, most mature adults are using CI so these sorts of changes would never even make it into the master branch after checks are made against the PR. I think they are just demonstrating how easily trust without verification can be subverted, as they have visibly sabotaged attempts to have CI set up for the RISC-V port of the kernel, tools and C library. I have evidence of this in writing. The commit in BBL that breaks PMP also has terrible code smell as it is hidden out-of-line from the existing PNP code, is hard-coded for a number of registers that differs from the specification, and the writing style is quite different from the other code. We can't even rule out the possibility of a forged commit given sha128 weaknesses. There are so many vectors for which they can inject breaking changes that go undetected without CI. Of course if it was written in ink, we could do hand-writing analysis, but in this case it is GitHub keys. Also, the OpenSBI plan has somehow been made fiat by the RISC-V Foundation by recently appearing in its GitHub organization, however, I am unsure that it was actually an agreed upon direction. There was no public review period. It just appeared from nowhere. Certainly there are different consensus processes operating here, one an orthodox one and one a heterodox one. I personally think merges need to be done by a bot, and; the consensus from the adults on the mailing list is that SBI should not be required for what should be accomplished by drivers; perhaps only to maintain backwards compatibility with the silicon that hardly anyone can get their hands on. Before I got fired, I was working on the CLIC which potentially allowed S-mode to have its own timers and IPIs. I personally think not giving S-mode timers and IPIs is kind of dumb. The number of flops for another timer and interrupt source is tiny. That said, good job from many other angles as there is an open source chip that runs Linux and we have Debian. The SBI firmware issue is just roughness around the edges and we can't expect everything to be perfect on day one, and it shows one can save a few flops (i.e. nanocents) at the expense of transitioning to RISC code in M-mode as micro-code using the memory bus for state versus one more timer comparator register. If we have CI, we should test riscv-pk. Many folk are using it and there are several derivatives. It is the original boot loader for RISC-V. I've been a BBL user for several years. I think it was Dec 2015 when I ordered an FPGA to run lowRISC untethered. BTW Palmer, Thanks for firing me. Last time I had a difficult boss I quit, but getting fired is actually better, because we have a 3 month stand down for our social security system here in NZ. If one voluntarily leaves one's job, one is not entitled to our unemployment benefit. If one gets fired, then one is entitled to the benefit immediately. You still need to pay us for the work up until December. I guess I got fired because I was super annoyed by being blocked from working on CI which was in my contract as an exhibit, and was what I volunteered to do. I didn't really want to do it anyhow; it is work; it's just something I have experience with (Cloud CI yada yada yada). The current setup is really bad from what I've used commercially. Also working on Open Source under an NDA is a bit weird too. One can be fired at any time for no reason. Much worse if I was in California on an H1B, where one essentially needs to leave the country if one gets fired. In this case, I just need to move somewhere cheaper to live, oddly enough Palmerston North (it's not really possible to survive in Auckland on social security as rents are comparatively high; like the Bay Area). Lucky I was fired before moving. That be much worse for my partner and step-son if I was fired after already moving to California. Hope you find someone to look after QEMU. If I was getting paid, I wouldn't mind to do some part-time work on CI, but I have another project that I want to work on. Regards, Michael. -- Trust without verification is corruption
On Mon, 11 Feb 2019 17:24:05 PST (-0800), Michael Clark wrote: > > > On 11/02/19 9:45 PM, Anup Patel wrote: >> On Mon, Feb 11, 2019 at 1:55 PM Christoph Hellwig <hch@infradead.org> wrote: >>> >>> On Sat, Feb 09, 2019 at 09:00:27AM +0530, Anup Patel wrote: >>>> IMHO, best solution would be to use mature open-source bootloader as >>>> payload (i.e. U-Boot or similar) and always boot Linux from bootloader. Using >>>> Linux as BBL/OpenSBI payload can still continue to exist but it can be more >>>> of a developer usage. >>> >>> Yes, that is a good long term plan. But in the meantime we should not >>> gratiously break the precious setups people got working.. > > This has been a longstanding issue. I believe some Berkeley graduates > are in control of the repos (some of them are very intelligent young > Doctors). However, the Berkeley software doesn't even version numbers. > We can't call the release management shocking because it is > non-existent. i.e.it is not even at the linux-0.11 style versioning > scheme, (although Linus didn't have version control hence tarball's). > Methinks they don't want CVEs being assigned as that would taint their > impeccable security reputation. For example, one can't really assign a > CVE to master:HEAD on a git repo. One needs an actual release. I am sure > it is deliberate. That said, assigning CVEs to students, with them > having a black mark at the start of their careers is probably also a bad > idea. > > I think these gratuitous breakages are a ploy, either to demonstrate how > easily trust can be subverted. i.e. the breakages are actually to troll > long-standing kernel overdevelops, showing how easy it is to slip in > breaking changes using the "trusted lieutenants" model, and it is not > really fixable until someone with release engineering experience takes > control of the foundation repositories, or the students step up and take > responsibility for their work, adding due professionalism, such as CI. > They are getting paid now. I'd like to assure everyone that the breakage was not intentional. We're all just doing our best to make things work here and unfortunately somethings things still break. Of course it's better when things don't break, but there's always a risk of breaking something whenever a change is introduced. In this case we were testing against BBL, it just turns out that the Linux configuration that everyone was testing didn't trigger the bug before the patch got merged. As far as I understand it this is what the merge window / RC process was designed for: during the merge window we accept changes that are then stabilized over the course of the RCs. It's better to not rely on that safety net, but in this case it did work. > Ideal situation would be gated repository with CI in front that locks > out changes that regress any of a suite of tests run against the open > source tools (and now simulation of the underlying open source chip and > its firmware), core libraries and the kernel. I am aware that some of > the breakages are deliberate based on traces in GitHub issues, and > artifacts in the git history. This century's beginning is marked by the > ubiquity of CI, well, certainly the last decade, so I see no reason why > it should not be applied here, as these changes would not have made it > into a project that has CI. > > Twerps. They need a punch. Not a gut punch. Just a friendly but firm Please keep things civil. So far I've been able to get away without properly reading the code of conduct and I'd like to keep it that way if possible :) > punch, in the shoulder. Serious, but not hard enough to cause any > permanent injury. In the right shoulder because the left shoulder has > bursitis, and not onto the bone as that would hurt one's fist. We need > to think about how to achieve this with a little finesse. Phone books > don't leave bruises but they are bulky to carry around, especially if > you lure one of these developers into a bar in a social setting. The > Phone book will look out of place. I think just a regular firm fist > punch would be best. We don't want assault charges and we certainly > wouldn't want an email trail leading back to someone threatening punches > to key members of the RISC-V community. That could be damaging (insert > emoji :Grinning Face With Smiling Eyes:). Anyway, they can't get to me > because I am in another country, so probability of me receiving a > physical punch is very low, so just a virtual punch, and it would be a > round of exchanging punches. Like I punch you, you punch me back sort of > thing. i.e. it needs to be fair. > >> The kernel breakage totally unintentional. I had done all testing using BBL >> and OpenSBI with Linux compiled using defconfig but never saw the issue. > > I agree with Christoph. > > It is no excuse. This is bad because it shows lack of basic testing. > It's like nobody boot tests riscv-qemu with BBL, which is where all of > the work on PMP is. In this century, most mature adults are using CI so > these sorts of changes would never even make it into the master branch > after checks are made against the PR. I think they are just > demonstrating how easily trust without verification can be subverted, as > they have visibly sabotaged attempts to have CI set up for the RISC-V > port of the kernel, tools and C library. I have evidence of this in > writing. The commit in BBL that breaks PMP also has terrible code smell > as it is hidden out-of-line from the existing PNP code, is hard-coded > for a number of registers that differs from the specification, and the > writing style is quite different from the other code. We can't even rule > out the possibility of a forged commit given sha128 weaknesses. There > are so many vectors for which they can inject breaking changes that go > undetected without CI. Of course if it was written in ink, we could do > hand-writing analysis, but in this case it is GitHub keys. > > Also, the OpenSBI plan has somehow been made fiat by the RISC-V > Foundation by recently appearing in its GitHub organization, however, I > am unsure that it was actually an agreed upon direction. There was no > public review period. It just appeared from nowhere. Certainly there are > different consensus processes operating here, one an orthodox one and > one a heterodox one. > > I personally think merges need to be done by a bot, and; the consensus > from the adults on the mailing list is that SBI should not be required > for what should be accomplished by drivers; perhaps only to maintain > backwards compatibility with the silicon that hardly anyone can get > their hands on. Before I got fired, I was working on the CLIC which > potentially allowed S-mode to have its own timers and IPIs. I personally > think not giving S-mode timers and IPIs is kind of dumb. The number of > flops for another timer and interrupt source is tiny. That said, good > job from many other angles as there is an open source chip that runs > Linux and we have Debian. The SBI firmware issue is just roughness > around the edges and we can't expect everything to be perfect on day > one, and it shows one can save a few flops (i.e. nanocents) at the > expense of transitioning to RISC code in M-mode as micro-code using the > memory bus for state versus one more timer comparator register. > > If we have CI, we should test riscv-pk. Many folk are using it and there > are several derivatives. It is the original boot loader for RISC-V. I've > been a BBL user for several years. I think it was Dec 2015 when I > ordered an FPGA to run lowRISC untethered. > > BTW Palmer, Thanks for firing me. Last time I had a difficult boss I > quit, but getting fired is actually better, because we have a 3 month > stand down for our social security system here in NZ. If one voluntarily > leaves one's job, one is not entitled to our unemployment benefit. If > one gets fired, then one is entitled to the benefit immediately. You > still need to pay us for the work up until December. I guess I got fired > because I was super annoyed by being blocked from working on CI which > was in my contract as an exhibit, and was what I volunteered to do. I > didn't really want to do it anyhow; it is work; it's just something I > have experience with (Cloud CI yada yada yada). The current setup is > really bad from what I've used commercially. Also working on Open Source > under an NDA is a bit weird too. One can be fired at any time for no > reason. Much worse if I was in California on an H1B, where one > essentially needs to leave the country if one gets fired. In this case, > I just need to move somewhere cheaper to live, oddly enough Palmerston > North (it's not really possible to survive in Auckland on social > security as rents are comparatively high; like the Bay Area). Lucky I > was fired before moving. That be much worse for my partner and step-son > if I was fired after already moving to California. Hope you find someone > to look after QEMU. If I was getting paid, I wouldn't mind to do some > part-time work on CI, but I have another project that I want to work on. > > Regards, > > Michael.
diff --git a/bbl/bbl.c b/bbl/bbl.c index 1b96a9d5..0d448d82 100644 --- a/bbl/bbl.c +++ b/bbl/bbl.c @@ -14,7 +14,7 @@ static uintptr_t dtb_output() { extern char _payload_end; uintptr_t end = (uintptr_t) &_payload_end; - return (end + MEGAPAGE_SIZE - 1) / MEGAPAGE_SIZE * MEGAPAGE_SIZE; + return (end + 16 * MEGAPAGE_SIZE - 1) / MEGAPAGE_SIZE * MEGAPAGE_SIZE; } Thanks Anup for the explanation and above fix.