|
From: | Jack Schwartz |
Subject: | Re: [Qemu-devel] [PATCH QEMU v1 0/4] multiboot: bss_end_addr can be zero / cleanup |
Date: | Fri, 19 Jan 2018 16:18:07 -0800 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 |
Hi Anatol, Daniel and Kevin. On 01/19/18 10:36, Anatol Pomozov wrote:
OK. I'm consulting with my company's legal department and waiting for their approvals for delivery of a test "kernel". I'll get in touch will everyone once I have an answer about that. I anticipate about a week before taking next steps to deliver.Hello Jack On Wed, Jan 17, 2018 at 12:06 PM, Jack Schwartz <address@hidden> wrote:Hi Kevin and Anatol. Kevin, thanks for your review. More inline below... On 01/15/18 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.Anatol, from my side, there are pros and cons to either patch set going in first, but advantages to either are pretty negligible. Pro for you going first: I can use the constants you will define in header files. Pro for me going first: your merge should be about the same as if you went first (since my changes are small, more localized and affect only multiboot.c) and my merge will be easier. What are your thoughts?Please move ahead with your patches. I'll rebase my changes on top of yours.
Kevin and Daniel, thanks for your inputs on this issue (different subthread), which I have forwarded to our legal department for review.
Thanks, Jack
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?Kevin and alias, Before I proceed with adding my multiboot test file, I'll clarify here that I started with a version from the grub2 tree. In that file I expanded a header file, also from the same tree. Neither file had any license header, though the tree I got them from (Dated October 2017) contains the GNU GPLv3 license file. I'll need to check with my company before I can say I can deliver this file. If I deliver it, I'll add a header stating the GPL license, that it came from grub2 and to check its repository for contributors.Jack Schwartz (4): multiboot: bss_end_addr can be zero multiboot: Remove unused variables from multiboot.c multiboot: Use header names when displaying fields multiboot: fprintf(stderr...) -> error_report()Apart from the conflicts, the patches look good to me.Thanks, JackKevin
[Prev in Thread] | Current Thread | [Next in Thread] |