[PATCH] x86: mmap kernel for direct kernel boot

[PATCH] x86: mmap kernel for direct kernel boot

[Gerd Hoffmann]

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

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

[John Snow]

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>

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
>

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

[John Snow]

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

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

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

[Michael S. Tsirkin]

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

ok

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

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

[Michael S. Tsirkin]

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

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

[John Snow]

On Sat, Nov 7, 2026 at 8:24 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> 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.

Patch was on-list well before freeze.

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

Looks like we do use this function when loading the initrd, easy
enough to fix here.

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

OK, if 2 and 3 are valid, it sounds like we just want a fallback to
fread. Mind you, we already use this technique for initrd images, so I
don't think we are doing anything fundamentally wacky here that QEMU
doesn't already do.

>
> 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 wouldn't worry about this; it's the same as "Well, what if you
modify a .qcow2 under a running QEMU?" Well, don't do that.

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

I think matching initrd loading behavior is all the justification we
need for treating the kernel identically.

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

Observed gain is that if you are loading a 19 MiB kernel, this change
allows that memory to be mapped and allocated as RssFile memory
instead of RssAnon memory. This means that if you run multiple QEMU
instances using the same kernel, that memory is shared and excluded
from USS instead of being charged per-instance.

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

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

[Michael S. Tsirkin]

On Sat, Nov 07, 2026 at 01:59:12PM -0500, John Snow wrote:
> On Sat, Nov 7, 2026 at 8:24 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > 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.
> 
> Patch was on-list well before freeze.

Yes but it has to be in a maintainer tree with pull on list, not just
patch on list.

> > 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.
> 
> Looks like we do use this function when loading the initrd, easy
> enough to fix here.
> 
> >
> > 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.
> 
> OK, if 2 and 3 are valid, it sounds like we just want a fallback to
> fread. Mind you, we already use this technique for initrd images, so I
> don't think we are doing anything fundamentally wacky here that QEMU
> doesn't already do.

Better safe I guess.

> >
> > 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 wouldn't worry about this; it's the same as "Well, what if you
> modify a .qcow2 under a running QEMU?" Well, don't do that.

OK.

> >
> >
> > 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.
> 
> I think matching initrd loading behavior is all the justification we
> need for treating the kernel identically.

I'd say the reverse, let's fix initrd loading)

> >
> > Also, pls include the observed gain in the commit log.
> 
> Observed gain is that if you are loading a 19 MiB kernel, this change
> allows that memory to be mapped and allocated as RssFile memory
> instead of RssAnon memory. This means that if you run multiple QEMU
> instances using the same kernel, that memory is shared and excluded
> from USS instead of being charged per-instance.

So do this? Run many QEMU's and measure a practical gain?

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