In the course of dealing with a problem in Xen's use of SeaBIOS when the resulting image exceeds 128k I found it desirable to not add unnecessarily much padding to the beginning of the image. Going in 64k steps should be sufficient; perhaps one could use even smaller ones, thus increasing the chances of there always being a window of available space at C0000h.
Signed-off-by: Jan Beulich jbeulich@suse.com
--- a/tools/checkrom.py +++ b/tools/checkrom.py @@ -32,11 +32,14 @@ def main(): rawdata = f.read() f.close() datasize = len(rawdata) - finalsize = 64*1024 - if datasize > 64*1024: + if datasize > 192*1024: + finalsize = 256*1024 + elif datasize > 128*1024: + finalsize = 192*1024 + elif datasize > 64*1024: finalsize = 128*1024 - if datasize > 128*1024: - finalsize = 256*1024 + else: + finalsize = 64*1024
# Sanity checks start = symbols['code32flat_start'].offset
On Fri, Aug 23, 2013 at 02:19:07PM +0100, Jan Beulich wrote:
In the course of dealing with a problem in Xen's use of SeaBIOS when the resulting image exceeds 128k I found it desirable to not add unnecessarily much padding to the beginning of the image. Going in 64k steps should be sufficient; perhaps one could use even smaller ones, thus increasing the chances of there always being a window of available space at C0000h.
Older versions of QEMU did not support 192K roms, but it does look like the latest version of QEMU does.
However, SeaBIOS should be managing the space between 0xc0000-0xfffff, and SeaBIOS does not care about the padding. So I'm not sure how this patch helps.
-Kevin
On 24.08.13 at 18:30, Kevin O'Connor kevin@koconnor.net wrote:
On Fri, Aug 23, 2013 at 02:19:07PM +0100, Jan Beulich wrote:
In the course of dealing with a problem in Xen's use of SeaBIOS when the resulting image exceeds 128k I found it desirable to not add unnecessarily much padding to the beginning of the image. Going in 64k steps should be sufficient; perhaps one could use even smaller ones, thus increasing the chances of there always being a window of available space at C0000h.
Older versions of QEMU did not support 192K roms, but it does look like the latest version of QEMU does.
However, SeaBIOS should be managing the space between 0xc0000-0xfffff, and SeaBIOS does not care about the padding. So I'm not sure how this patch helps.
As said in the description, I consider it desirable, as the pointless padding was at least confusing to me. If it being merely cosmetic makes the patch undesirable to you, I'll have to live with that (and hope to remember to not be confused the next time I have to look at the rather difficult to understand build logic).
Jan
Hi,
However, SeaBIOS should be managing the space between 0xc0000-0xfffff, and SeaBIOS does not care about the padding. So I'm not sure how this patch helps.
As said in the description, I consider it desirable, as the pointless padding was at least confusing to me.
Flash chips (as real hardware) usually have power-of-two sizes. I would be confused by a 192k rom ...
If it being merely cosmetic makes the patch undesirable to you, I'll have to live with that (and hope to remember to not be confused the next time I have to look at the rather difficult to understand build logic).
When touching this logic I think we should go for a config option.
Current qemu seabios builds end up being around 128k in size. Depending on the compiler used you'll get a 128k (gcc 4.4) or a 256k rom (gcc 4.7 +). The size of the rom changing is a problem because it breaks qemu live migration.
Explicitly specifying the size would solve this. You can specify size=128k then and the build will fail if it doesn't fit. You can specify size=256k and you'll get a 256k rom even if it would fit into 128k. And we could allow size=192k too ;)
cheers, Gerd
On 28.08.13 at 09:36, Gerd Hoffmann kraxel@redhat.com wrote:
Explicitly specifying the size would solve this. You can specify size=128k then and the build will fail if it doesn't fit.
Not really nice, as there's no immediate way to address such a build failure. Guessing upon compiler versions (and having one at hand that produces a small enough image) is not really someone ought to be concerned about who just needs to build SeaBIOS as a prerequisite (i.e. not themselves knowing much about SeaBIOS - that was the fundamental issue I ran into: Knowing nothing about it, I had this strange runtime failure [which admittedly is even worse than a build time one], and had to inspect stuff I should never have had a need to look at).
Jan
On Mi, 2013-08-28 at 08:51 +0100, Jan Beulich wrote:
On 28.08.13 at 09:36, Gerd Hoffmann kraxel@redhat.com wrote:
Explicitly specifying the size would solve this. You can specify size=128k then and the build will fail if it doesn't fit.
Not really nice, as there's no immediate way to address such a build failure.
Yes. If it doesn't fit, then it doesn't fit.
If you are okay with the bios changing the size you can just re-configure it with 192k, 256k or whatever you want.
If you want to keep it at a certain size for compatibility reasons, well, then you have to figure a way to make it fit. Other compiler, turn off features, whatever. Might not be obvious indeed. But there is no magic way around it.
And as long as software can't read our minds I prefer to explicitly configure things the way I wanna have them rather than software guessing and getting it wrong.
- that was the fundamental issue I ran into: Knowing nothing
about it, I had this strange runtime failure [which admittedly is even worse than a build time one], and had to inspect stuff I should never have had a need to look at).
And your patch fixes the mentioned runtime failure? Previous message sounded like the patch was purely cosmetic ...
If it fixes a failure, care to explain? That information belongs into the commit log.
thanks, Gerd
On 28.08.13 at 13:10, Gerd Hoffmann kraxel@redhat.com wrote:
And your patch fixes the mentioned runtime failure? Previous message sounded like the patch was purely cosmetic ...
No, the fix for the runtime failure was a different one (outside of SeaBIOS itself). It just confused me in the process of finding the root cause that the image consumed the whole 256k of ROM space for no good reason.
Jan
On Wed, Aug 28, 2013 at 12:38:57PM +0100, Jan Beulich wrote:
On 28.08.13 at 13:10, Gerd Hoffmann kraxel@redhat.com wrote:
And your patch fixes the mentioned runtime failure? Previous message sounded like the patch was purely cosmetic ...
No, the fix for the runtime failure was a different one (outside of SeaBIOS itself). It just confused me in the process of finding the root cause that the image consumed the whole 256k of ROM space for no good reason.
That is not correct. There is a reason for the rom to be 256K - old versions of QEMU did not support 192K and the configured SeaBIOS code required more than 128K. The 256K is not consumed by SeaBIOS - the first thing SeaBIOS does is turn the area into memory, relocate most of its code out of the area, and then open up the space for option roms, internal ram storage, and UMBs.
-Kevin
On 28.08.13 at 14:54, Kevin O'Connor kevin@koconnor.net wrote:
On Wed, Aug 28, 2013 at 12:38:57PM +0100, Jan Beulich wrote:
On 28.08.13 at 13:10, Gerd Hoffmann kraxel@redhat.com wrote:
And your patch fixes the mentioned runtime failure? Previous message sounded like the patch was purely cosmetic ...
No, the fix for the runtime failure was a different one (outside of SeaBIOS itself). It just confused me in the process of finding the root cause that the image consumed the whole 256k of ROM space for no good reason.
That is not correct. There is a reason for the rom to be 256K - old versions of QEMU did not support 192K and the configured SeaBIOS code required more than 128K. The 256K is not consumed by SeaBIOS - the first thing SeaBIOS does is turn the area into memory, relocate most of its code out of the area, and then open up the space for option roms, internal ram storage, and UMBs.
Yeah, I wasn't precise enough in my statement - I should have added "initially".
Jan
-
Gerd Hoffmann -
Jan Beulich -
Kevin O'Connor