tech@ et. al.,

When kettenis@ introduced a newer version of BOOTARG_CONSDEV to add
additional params for the AMD Ryzen V1000 family, vmd's code that
configures bootargs to support direct booting a ramdisk kernel didn't
adjust with it.

Mischa Peters found this and shared a simple reproducer on 7.2 and
-current:

# vmctl start -c -b /bsd.rd -m 4G test

Where /bsd.rd is a 7.2 or -current ramdisk kernel.

Interestingly, this is only seen when using 4G (or more) memory for the
guest. I think it's just a happy coincedence it works < 4G because of
the resulting BOOTARG_MEMMAP sizing things so the BOOTARG_CONSDEV works.

Diff below fixes the issue by simply zero'ing the BOOTARG_CONSDEV
structure before assigning to members.

While here, I also cleaned up some things like using literal values that
could be more descriptive boot arg names and also made the arithmetic
explicitly use the same type (uint32_t) throughout instead of mixing it
with int.

ok?

-dv

diff refs/heads/master refs/heads/vmd-ramdisk
commit - 8cbcfb178c36f28f6fcb28289719a4f0547eabb4
commit + 0be12dfaa063ded82837d3a6b2ce8df7ea7e1c2d
blob - b367721e32b61892955bbf835b873034875c85ec
blob + d560b8e8eb2cdd87a60c63e8ecb7fed56e5c60dc
--- usr.sbin/vmd/loadfile_elf.c
+++ usr.sbin/vmd/loadfile_elf.c
@@ -382,9 +382,10 @@ create_bios_memmap(struct vm_create_params *vcp, bios_
  * Parameters:
  *  memmap: the BIOS memory map
  *  n: number of entries in memmap
+ *  bootmac: optional PXE boot MAC address
  *
  * Return values:
- *  The size of the bootargs
+ *  The size of the bootargs in bytes
  */
 static uint32_t
 push_bootargs(bios_memmap_t *memmap, size_t n, bios_bootmac_t *bootmac)
@@ -393,40 +394,41 @@ push_bootargs(bios_memmap_t *memmap, size_t n, bios_bo
        bios_consdev_t consdev;
        uint32_t ba[1024];

-       memmap_sz = 3 * sizeof(int) + n * sizeof(bios_memmap_t);
-       ba[0] = 0x0;    /* memory map */
+       memmap_sz = 3 * sizeof(uint32_t) + n * sizeof(bios_memmap_t);
+       ba[0] = BOOTARG_MEMMAP;
        ba[1] = memmap_sz;
-       ba[2] = memmap_sz;      /* next */
+       ba[2] = memmap_sz;
        memcpy(&ba[3], memmap, n * sizeof(bios_memmap_t));
-       i = memmap_sz / sizeof(int);
+       i = memmap_sz / sizeof(uint32_t);

        /* Serial console device, COM1 @ 0x3f8 */
-       consdev.consdev = makedev(8, 0);        /* com1 @ 0x3f8 */
+       memset(&consdev, 0, sizeof(consdev));
+       consdev.consdev = makedev(8, 0);
        consdev.conspeed = 115200;
        consdev.consaddr = 0x3f8;
-       consdev.consfreq = 0;

-       consdev_sz = 3 * sizeof(int) + sizeof(bios_consdev_t);
-       ba[i] = 0x5;   /* consdev */
+       consdev_sz = 3 * sizeof(uint32_t) + sizeof(bios_consdev_t);
+       ba[i] = BOOTARG_CONSDEV;
        ba[i + 1] = consdev_sz;
        ba[i + 2] = consdev_sz;
        memcpy(&ba[i + 3], &consdev, sizeof(bios_consdev_t));
-       i += consdev_sz / sizeof(int);
+       i += consdev_sz / sizeof(uint32_t);

        if (bootmac) {
-               bootmac_sz = 3 * sizeof(int) + (sizeof(bios_bootmac_t) + 3) & 
~3;
-               ba[i] = 0x7;   /* bootmac */
+               bootmac_sz = 3 * sizeof(uint32_t) +
+                   (sizeof(bios_bootmac_t) + 3) & ~3;
+               ba[i] = BOOTARG_BOOTMAC;
                ba[i + 1] = bootmac_sz;
                ba[i + 2] = bootmac_sz;
                memcpy(&ba[i + 3], bootmac, sizeof(bios_bootmac_t));
-               i += bootmac_sz / sizeof(int);
+               i += bootmac_sz / sizeof(uint32_t);
        }

        ba[i++] = 0xFFFFFFFF; /* BOOTARG_END */

        write_mem(BOOTARGS_PAGE, ba, PAGE_SIZE);

-       return (i * sizeof(int));
+       return (i * sizeof(uint32_t));
 }

 /*

Reply via email to