Re: [Qemu-devel] [PATCH QEMU v1 0/4] multiboot: bss_end_addr can be zero / cleanup

[Kevin Wolf]

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_report
> 
> 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.
> 
>   Thanks,
>   Jack

Thanks, applied to my multiboot branch.

Kevin

Re: [Qemu-devel] [PATCH QEMU v1 0/4] multiboot: bss_end_addr can be zero / cleanup

[Jack Schwartz]

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_report

There 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

Re: [Qemu-devel] [PATCH QEMU v1 0/4] multiboot: bss_end_addr can be zero / cleanup

[Kevin Wolf]

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_report
> > There 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

Re: [Qemu-devel] [PATCH QEMU v1 0/4] multiboot: bss_end_addr can be zero / cleanup

[Jack Schwartz]

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_report

There 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.


Patchwork links, for 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



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.

Kevin

Re: [Qemu-devel] [PATCH QEMU v1 0/4] multiboot: bss_end_addr can be zero / cleanup

[Daniel P. Berrange]

On Fri, Jan 19, 2018 at 04:18:07PM -0800, Jack Schwartz wrote:
> Hi Anatol, Daniel and Kevin.
> 
> On 01/19/18 10:36, Anatol Pomozov wrote:
> > Hello Jack
> > 
> > On Wed, Jan 17, 2018 at 12:06 PM, Jack Schwartz
> >  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_report
> > > > There 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.
> 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.
> 
> Kevin and Daniel, thanks for your inputs on this issue (different
> subthread), which I have forwarded to our legal department for review.

FWIW, I don't think it needs to be your responsibility to decide this. I
think the QEMU community / maintainer taking the patches needs to decide
whether it is acceptable / desirable.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH QEMU v1 0/4] multiboot: bss_end_addr can be zero / cleanup

[Jack Schwartz]

Hi Anatol, Daniel and Kevin.

On 01/19/18 10:36, Anatol Pomozov wrote:

Hello Jack

On Wed, Jan 17, 2018 at 12:06 PM, Jack Schwartz
 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_report

There 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.
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.


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,
 Jack


Kevin

Re: [Qemu-devel] [PATCH QEMU v1 0/4] multiboot: bss_end_addr can be zero / cleanup

[Anatol Pomozov]

Hello Jack

On Wed, Jan 17, 2018 at 12:06 PM, Jack Schwartz
 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_report
>>
>> There 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.

>>
>> 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,
> Jack
>>
>>
>> Kevin
>>
>

Re: [Qemu-devel] [PATCH QEMU v1 0/4] multiboot: bss_end_addr can be zero / cleanup

[Daniel P. Berrange]

On Thu, Jan 18, 2018 at 12:35:00PM +0100, Kevin Wolf wrote:
> Am 17.01.2018 um 21:06 hat Jack Schwartz geschrieben:
> > 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 see. QEMU as a whole is GPLv2, so this might be a problem. It's
> probably not as bad as merging GPLv3 code into QEMU proper because it's
> a standalone test kernel that I suppose could have a different license.
> But IANAL and maybe it's safer not to go there.

As long as the GPLv3 code is not linked / otherwise combined with any
of the GPLv2 code in QEMU that will be ok. Since it is a self-contained
test kernel, that would not be an issue in this case - it only interfaces
to QEMU via the x86 machine ABI.

It just means that the QEMU source tar.gz will be under a conjunction
of licenses  "GPLv2 and GPLv2+ and GPLv3 and ...all our other licenss..."

The resulting binaries for emulators/tools will still be GPLv2 as they're
only linking in the GPLv2 + GPLv2+ source, never linking the GPlv3 test
image.

For simplicity of understanding though, it could be desirable to avoid
this if not an unreasonable amount of extra work

> Maybe it would be less hassle to just reimplement the tests, based on
> the MIT licensed tests that are already in tests/multiboot/.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH QEMU v1 0/4] multiboot: bss_end_addr can be zero / cleanup

[Kevin Wolf]

Am 17.01.2018 um 21:06 hat Jack Schwartz geschrieben:
> 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 see. QEMU as a whole is GPLv2, so this might be a problem. It's
probably not as bad as merging GPLv3 code into QEMU proper because it's
a standalone test kernel that I suppose could have a different license.
But IANAL and maybe it's safer not to go there.

Maybe it would be less hassle to just reimplement the tests, based on
the MIT licensed tests that are already in tests/multiboot/.

Kevin

Re: [Qemu-devel] [PATCH QEMU v1 0/4] multiboot: bss_end_addr can be zero / cleanup

[Jack Schwartz]

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_report

There 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?

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,
    Jack


Kevin

Re: [Qemu-devel] [PATCH QEMU v1 0/4] multiboot: bss_end_addr can be zero / cleanup

[Kevin Wolf]

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_report

There 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?

> 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.

Kevin

[Qemu-devel] [PATCH QEMU v1 0/4] multiboot: bss_end_addr can be zero / cleanup

[Jack Schwartz]

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_report

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.

Thanks,
Jack

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()

 hw/i386/multiboot.c | 77 ++---
 1 file changed, 38 insertions(+), 39 deletions(-)

-- 
1.8.3.1