On Mon, Nov 28, 2022 at 11:32:32AM -0500, Dave Voutila wrote:
> 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?
>
ok mlarkin
> -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));
> }
>
> /*
