|
From: | Jack Schwartz |
Subject: | Re: [Qemu-devel] [PATCH QEMU v1 0/4] multiboot: bss_end_addr can be zero / cleanup |
Date: | Tue, 6 Mar 2018 17:52:08 -0800 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 |
Hi Kevin and everyone. On 2018-03-05 00:13, Kevin Wolf wrote:
Am 02.03.2018 um 20:32 hat Jack Schwartz geschrieben:Hi Kevin. On 2018-01-15 07:54, Kevin Wolf wrote:Am 21.12.2017 um 18:25 hat Jack Schwartz geschrieben:Properly account for the possibility of multiboot kernels with a zero bss_end_addr. The Multiboot Specification, section 3.1.3 allows for kernels without a bss section, by allowing a zeroed bss_end_addr multiboot header field. Do some cleanup to multiboot.c as well: - Remove some unused variables. - Use more intuitive header names when displaying fields in messages. - Change fprintf(stderr...) to error_reportThere are some conflicts with Anatol's (CCed) multiboot series: https://lists.nongnu.org/archive/html/qemu-devel/2017-10/msg03003.html None if these should be hard to resolve, but it would be good if you could agree with each other whose patch series should come first, and then the other one should be rebased on top of that.Testing: 1) Ran the "make check" test suite. 2) Booted multiboot kernel with bss_end_addr=0. (I rolled my own grub multiboot.elf test "kernel" by modifying source.) Verified with gdb that new code that reads addresses/offsets from multiboot header was accessed. 3) Booted multiboot kernel with non-zero bss_end_addr. 4) Uncommented DEBUG_MULTIBOOT in multiboot.c and verified messages worked. 5) Code has soaked in an internal repo for two months.Can you integrate your test kernel from 2) in tests/multiboot/ so we can keep this as a regression test?If need be, would you be willing to accept updated versions of these patches (with another review, of course) without the test file? I will deliver the test file later once I get company approvals. I don't want the test file to continue holding everything up in the meantime.Sure, let's move forward with what we have now. Please keep me CCed when you send a new version and I'll give it a review and hopeuflly get it merged. Kevin
Thanks, Kevin.Patches have not changed, and I verified they still work on a current repo. (Multiboot.c has had a one-line change regarding a header file, so I rebuilt and re-tested to make sure.)
Links again, for your reference: 1/4 multiboot: bss_end_addr can be zero http://patchwork.ozlabs.org/patch/852049/ 2/4 multiboot: Remove unused variables from multiboot.c http://patchwork.ozlabs.org/patch/852045/ 3/4 multiboot: Use header names when displaying fields http://patchwork.ozlabs.org/patch/852046/ 4/4 multiboot: fprintf(stderr...) -> error_report() http://patchwork.ozlabs.org/patch/852051/ Thanks, Jack
[Prev in Thread] | Current Thread | [Next in Thread] |