> Date: Mon, 28 Nov 2022 18:26:47 +0100
> From: Claudio Jeker <cje...@diehard.n-r-g.com>
> 
> 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 claudio@

ok kettenis@ too FWIW

> PS: diff did not apply, a space got stripped below diff should apply
> -- 
> :wq Claudio
> 
> Index: loadfile_elf.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/vmd/loadfile_elf.c,v
> retrieving revision 1.42
> diff -u -p -r1.42 loadfile_elf.c
> --- loadfile_elf.c    28 Jan 2022 06:33:27 -0000      1.42
> +++ loadfile_elf.c    28 Nov 2022 17:07:20 -0000
> @@ -382,9 +382,10 @@ create_bios_memmap(struct vm_create_para
>   * 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, siz
>       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