Re: [PATCH] x86: mmap kernel for direct kernel boot

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, Nov 06, 2026 at 01:02:33PM -0500, John Snow wrote:
> On Wed, Oct 21, 2026 at 12:02 PM John Snow <jsnow@redhat.com> wrote:
> >
> > On Wed, Oct 21, 2026 at 2:31 AM Gerd Hoffmann <kraxel@redhat.com> wrote:
> > >
> > > Reduce memory footprint by mapping the kernel into memory instead of
> > > reading into allocated memory in the common case (no -dtb $file on the
> > > qemu command line).
> > >
> > > Cc: John Snow <jsnow@redhat.com>
> > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> >
> > Tested-by: John Snow <jsnow@redhat.com>
> >
> 
> Ping, is someone going to pick up this patch?
> 
> --js

First we are in freeze. So no rush.
Here's some ai based review - the arguments make sense to me.

1. No length validation on the mapping. kernel_size comes from
get_file_size() (fstat at open time), but the patch never checks
g_mapped_file_get_length() == kernel_size. If the file shrinks between
the stat and the mmap (or is a weird file), you get an OOB read / SIGBUS
instead of the old clean fread() failed exit. Further, if the
file is truncated later (QEMU running, guest hasn't read the kernel
through fw_cfg yet), guest reads hit SIGBUS and kill QEMU. fread copies
up front, so this race didn't exist before.

2. Files that are readable but not mappable. fread works on anything
stdio can read; mmap fails on some FUSE mounts, files with outstanding
mandatory/lease locks, etc. g_mapped_file_new failing exits QEMU, so a
previously booting config on such a filesystem stops booting.

3. Windows share modes. stdio fopen("rb") opens with permissive sharing,
while gio's file mapping opens the file with a more restrictive share
mode. A kernel image that another process still holds open for writing
(download in progress, installer, antivirus scanning with exclusive
handle) currently boots and would now fail to map. Windows is a
supported platform, so this counts.

4. Torn reads on concurrent modification. With fread you snapshot the
file once; with a private mapping, pages faulted after an external write
see new content, so a guest could get a mix of old and new kernel bytes.
Only matters for someone rewriting the image under a running QEMU, which
is already dubious.


I would not worry about torn reads too much. But failure seems
theoretically legit.  It's hard to know this does not break any existing
configs.  It might be safer to just add an option.

Also, pls include the observed gain in the commit log.


> > Using this patch, I shifted about 19MiB of resources from RssAnon to
> > RssFile. This theoretically allows sharing resources between multiple
> > QEMU instances in the trivial case using -kernel and -initrd. This
> > matches how we already load initrd.
> >
> > --js
> >
> > > ---
> > >  hw/i386/x86-common.c | 25 ++++++++++++++++++++-----
> > >  1 file changed, 20 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/hw/i386/x86-common.c b/hw/i386/x86-common.c
> > > index fa18bf1aeceb..25e141aa8537 100644
> > > --- a/hw/i386/x86-common.c
> > > +++ b/hw/i386/x86-common.c
> > > @@ -911,16 +911,31 @@ void x86_load_linux(X86MachineState *x86ms,
> > >      }
> > >
> > >      setup  = g_malloc(setup_size);
> > > -    kernel = g_malloc(kernel_size);
> > >      fseek(f, 0, SEEK_SET);
> > >      if (fread(setup, 1, setup_size, f) != setup_size) {
> > >          fprintf(stderr, "fread() failed\n");
> > >          exit(1);
> > >      }
> > > -    fseek(f, 0, SEEK_SET);
> > > -    if (fread(kernel, 1, kernel_size, f) != kernel_size) {
> > > -        fprintf(stderr, "fread() failed\n");
> > > -        exit(1);
> > > +    if (dtb_filename) {
> > > +        /* read kernel into allocated memory, so we can realloc for the dtb */
> > > +        kernel = g_malloc(kernel_size);
> > > +        fseek(f, 0, SEEK_SET);
> > > +        if (fread(kernel, 1, kernel_size, f) != kernel_size) {
> > > +            fprintf(stderr, "fread() failed\n");
> > > +            exit(1);
> > > +        }
> > > +    } else {
> > > +        /* mmap kernel */
> > > +        GMappedFile *mapped_file;
> > > +        GError *gerr = NULL;
> > > +
> > > +        mapped_file = g_mapped_file_new(kernel_filename, false, &gerr);
> > > +        if (!mapped_file) {
> > > +            fprintf(stderr, "qemu: error reading kernel %s: %s\n",
> > > +                    kernel_filename, gerr->message);
> > > +            exit(1);
> > > +        }
> > > +        kernel = (uint8_t *)g_mapped_file_get_contents(mapped_file);
> > >      }
> > >      fclose(f);
> > >
> > > --
> > > 2.56.0
> > >