Re: multiboot2 and module2 boot issues via GRUB2
On 01/04/2021 09:44, Roger Pau Monné wrote: > On Thu, Apr 01, 2021 at 09:31:07AM +0200, Jan Beulich wrote: >> On 01.04.2021 03:06, Roman Shaposhnik wrote: >>> And the obvious next question: is my EVE usecase esoteric enough that >>> I should just go ahead and do a custom GRUB patch or is there a more >>> general interest in this? >> Not sure if it ought to be a grub patch - the issue could as well >> be dealt with in Xen, by concatenating modules to form a monolithic >> initrd. > I would rather have it done in the loader than Xen, mostly because > it's a Linux boot specific format, and hence I don't think Xen should > have any knowledge about it. > > If it turns out to be impossible to implement on the loader side we > should consider doing it in Xen, but that's not my first option. Concatenating random things which may or may not be initrds is absolutely not something Xen should do. We don't have enough context to do it safely/sensibly. Honestly, I like the idea of supporting something like this generally in grub. Linux already commonly has initrd preparation prepending an uncompressed microcode CPIO archive, and I can see a usability advantage from maintaining the initrd fragments separately. Looking at the grub manual, this behaviour of the `initrd` command isn't even documented. Perhaps that should be fixed first, and then maybe `module2_multi` added too? ~Andrew ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH for 2.06] video/fbfill: use unsigned integers for width/height
Since commit 7ce3259f67ac ("video/fb/fbfill: Fix potential integer
overflow"), clang builds of grub-emu have failed with messages like:
/usr/bin/ld: libgrubmods.a(libgrubmods_a-fbfill.o): in function
`grub_video_fbfill_direct24':
fbfill.c:(.text+0x28e): undefined reference to `__muloti4'
This appears to be due to a weird quirk in how clang compiles
grub_mul (dst->mode_info->bytes_per_pixel, width, &rowskip)
which is grub_mul (unsigned int, int, &grub_size_t).
It looks like clang somewhere promotes everything to 128-bit maths
before ultimately reducing down to 64 bit for grub_size_t. I think
this is because width is signed, and indeed converting width to an
unsigned int makes the problem go away.
This conversion also makes more sense generally:
- the caller of all the fbfill_directN functions is
grub_video_fb_fill_dispatch and it takes width and height as
unsigned ints already,
- it doesn't make sense to fill a negative width or height.
Convert the width and height arguments and associated loop counters
to unsigned ints.
Fixes: 7ce3259f67ac ("video/fb/fbfill: Fix potential integer overflow")
Signed-off-by: Daniel Axtens
---
This should go in for 2.06, otherwise the Clang build will be broken.
---
grub-core/video/fb/fbfill.c | 30 +++---
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/grub-core/video/fb/fbfill.c b/grub-core/video/fb/fbfill.c
index a37acd1e2939..51b1f6557007 100644
--- a/grub-core/video/fb/fbfill.c
+++ b/grub-core/video/fb/fbfill.c
@@ -38,10 +38,10 @@
static void
grub_video_fbfill (struct grub_video_fbblit_info *dst,
grub_video_color_t color, int x, int y,
- int width, int height)
+ unsigned int width, unsigned int height)
{
- int i;
- int j;
+ unsigned int i;
+ unsigned int j;
for (j = 0; j < height; j++)
for (i = 0; i < width; i++)
@@ -53,10 +53,10 @@ grub_video_fbfill (struct grub_video_fbblit_info *dst,
static void
grub_video_fbfill_direct32 (struct grub_video_fbblit_info *dst,
grub_video_color_t color, int x, int y,
- int width, int height)
+ unsigned int width, unsigned int height)
{
- int i;
- int j;
+ unsigned int i;
+ unsigned int j;
grub_uint32_t *dstptr;
grub_size_t rowskip;
@@ -84,10 +84,10 @@ grub_video_fbfill_direct32 (struct grub_video_fbblit_info
*dst,
static void
grub_video_fbfill_direct24 (struct grub_video_fbblit_info *dst,
grub_video_color_t color, int x, int y,
- int width, int height)
+ unsigned int width, unsigned int height)
{
- int i;
- int j;
+ unsigned int i;
+ unsigned int j;
grub_size_t rowskip;
grub_uint8_t *dstptr;
#ifndef GRUB_CPU_WORDS_BIGENDIAN
@@ -127,10 +127,10 @@ grub_video_fbfill_direct24 (struct grub_video_fbblit_info
*dst,
static void
grub_video_fbfill_direct16 (struct grub_video_fbblit_info *dst,
grub_video_color_t color, int x, int y,
- int width, int height)
+ unsigned int width, unsigned int height)
{
- int i;
- int j;
+ unsigned int i;
+ unsigned int j;
grub_size_t rowskip;
grub_uint16_t *dstptr;
@@ -158,10 +158,10 @@ grub_video_fbfill_direct16 (struct grub_video_fbblit_info
*dst,
static void
grub_video_fbfill_direct8 (struct grub_video_fbblit_info *dst,
grub_video_color_t color, int x, int y,
- int width, int height)
+ unsigned int width, unsigned int height)
{
- int i;
- int j;
+ unsigned int i;
+ unsigned int j;
grub_size_t rowskip;
grub_uint8_t *dstptr;
grub_uint8_t fill = (grub_uint8_t)color & 0xFF;
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: multiboot2 and module2 boot issues via GRUB2
On 01.04.2021 10:44, Roger Pau Monné via Grub-devel wrote: On Thu, Apr 01, 2021 at 09:31:07AM +0200, Jan Beulich wrote: On 01.04.2021 03:06, Roman Shaposhnik wrote: And the obvious next question: is my EVE usecase esoteric enough that I should just go ahead and do a custom GRUB patch or is there a more general interest in this? Not sure if it ought to be a grub patch - the issue could as well be dealt with in Xen, by concatenating modules to form a monolithic initrd. I would rather have it done in the loader than Xen, mostly because it's a Linux boot specific format, and hence I don't think Xen should have any knowledge about it. Keep in mind that loader is loading Xen, using multiboot2 which is kind of generic boot protocol, not Linux with its specific protocol. There may be other multiboot2 kernels that depend on modules being _not_ concatenated, such change would most likely break those. Best regards, -- Krystian Hebel Firmware Engineer https://3mdeb.com | @3mdeb_com If it turns out to be impossible to implement on the loader side we should consider doing it in Xen, but that's not my first option. Thanks, Roger. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: multiboot2 and module2 boot issues via GRUB2
On Thu, Apr 01, 2021 at 09:31:07AM +0200, Jan Beulich wrote: > On 01.04.2021 03:06, Roman Shaposhnik wrote: > > And the obvious next question: is my EVE usecase esoteric enough that > > I should just go ahead and do a custom GRUB patch or is there a more > > general interest in this? > > Not sure if it ought to be a grub patch - the issue could as well > be dealt with in Xen, by concatenating modules to form a monolithic > initrd. I would rather have it done in the loader than Xen, mostly because it's a Linux boot specific format, and hence I don't think Xen should have any knowledge about it. If it turns out to be impossible to implement on the loader side we should consider doing it in Xen, but that's not my first option. Thanks, Roger. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: multiboot2 and module2 boot issues via GRUB2
On 01.04.2021 03:06, Roman Shaposhnik wrote: > And the obvious next question: is my EVE usecase esoteric enough that > I should just go ahead and do a custom GRUB patch or is there a more > general interest in this? Not sure if it ought to be a grub patch - the issue could as well be dealt with in Xen, by concatenating modules to form a monolithic initrd. Jan ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
