Re: [SeaBIOS] [PATCH] coreboot: update memory table before handling execution to payload/system
I don't think SeaBIOS should be altering coreboot information. (Doing so leads to all sorts of painful debugging problems, for example.) Well, currently it is marking at least first couple of kilobytes of memory (4 if I recall correctly) as free to use RAM. There coreboot tables are located, or at least a pointer to tables in higher memory. Because of that these tables can get overwritten by SeaBIOS or OS that starts later. Having a device marked as an available RAM is even worse - writing unchecked values to some device registers (e.g. by a coreboot-aware system which makes use of outdated memory tables) can lead to undefined behaviour (best case scenario) and even physically damage the platform or connected devices. Trying to debug this can be even worse, especially with address space randomisation implemented in most modern operating systems. This patch affects not only memtest, it is just clearly and immediately visible there. ___ SeaBIOS mailing list SeaBIOS@seabios.org https://mail.coreboot.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] coreboot: update memory table before handling execution to payload/system
On Mon, Nov 05, 2018 at 01:27:10PM +0100, Krystian Hebel wrote: > SeaBIOS modifies its internal e820 structure, but does not propagate > these changes back to coreboot tables. This resulted in multiple errors > in MemTest86 when run on 2 GB platforms, probably because of some > memory-mapped devices. > > This patch copies back modified e820 tables before booting an OS or > a payload. Thanks, but I'm not sure that at a "high level" this is a good idea. I don't think SeaBIOS should be altering coreboot information. (Doing so leads to all sorts of painful debugging problems, for example.) If memtest86 is having problems with the coreboot tables I suspect it would be best to alter memtest86 to not read the coreboot tables. -Kevin ___ SeaBIOS mailing list SeaBIOS@seabios.org https://mail.coreboot.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] coreboot: update memory table before handling execution to payload/system
On 11/05/18 13:32, Paul Menzel wrote: Thank you for the patch. If it’s the common problem, then it was > fixed in “coreboot’s” MemTest86+ [1]. Can you reproduce the problem > with that version? Yes, it is the version on which I tested. It worked OK before [1], because when no coreboot tables were found MemTest86+ resorted to e820. It was reproduced on PC Engines apu3 (2 GB version) and apu1, the latter requires SPD fix [2]. 4 GB versions of apu doesn't seem to be affected. [1]: https://review.coreboot.org/cgit/memtest86plus.git/commit/?id=0704ab381a7ebd3c0100d2d7be9359076f713c43 [2]: https://review.coreboot.org/c/memtest86plus/+/29372 Regards, Krystian ___ SeaBIOS mailing list SeaBIOS@seabios.org https://mail.coreboot.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] coreboot: update memory table before handling execution to payload/system
Dear Krystian, On 11/05/18 13:27, Krystian Hebel wrote: > SeaBIOS modifies its internal e820 structure, but does not propagate > these changes back to coreboot tables. This resulted in multiple errors > in MemTest86 when run on 2 GB platforms, probably because of some > memory-mapped devices. > > This patch copies back modified e820 tables before booting an OS or > a payload. Thank you for the patch. If it’s the common problem, then it was fixed in “coreboot’s” MemTest86+ [1]. Can you reproduce the problem with that version? Kind regards, Paul [1]: https://review.coreboot.org/cgit/memtest86plus.git smime.p7s Description: S/MIME Cryptographic Signature ___ SeaBIOS mailing list SeaBIOS@seabios.org https://mail.coreboot.org/mailman/listinfo/seabios
