Re: [PATCH v5 3/3] kern/xen: Add Xen command line parsing

[Daniel Kiper <dkiper@xxxxxxxxxxxx>]

On Wed, Aug 13, 2025 at 08:36:45PM -0500, arraybolt3@xxxxxxxxx wrote:
> From: Aaron Rainbolt <arraybolt3@xxxxxxxxx>
>
> Xen traditionally allows customizing guest behavior by passing arguments
> to the VM kernel via the kernel command line. This is no longer possible
> when using GRUB with Xen, as the kernel command line is decided by the
> GRUB configuration file within the guest, not data passed to the guest
> by Xen.
>
> To work around this limitation, enable GRUB to parse a command line
> passed to it by Xen, and expose data from the command line to the GRUB
> configuration as environment variables. These variables can be used in
> the GRUB configuration for any desired purpose, such as extending the
> kernel command line passed to the guest. The command line format is
> inspired by the Linux kernel's command line format.
>
> To reduce the risk of misuse, abuse, or accidents in production, the
> command line will only be parsed if it consists entirely of 7-bit ASCII
> characters, only alphabetical characters and underscores are permitted
> in variable names, and all variable names must start with the string
> "xen_grub_env_". This also allows room for expanding the command line
> arguments accepted by GRUB in the future, should other arguments end up
> becoming desirable in the future.
>
> Signed-off-by: Aaron Rainbolt <arraybolt3@xxxxxxxxx>

Reviewed-by: Daniel Kiper <daniel.kiper@xxxxxxxxxx>

... but two nits below...

> +      else if (is_key_safe (ps.current_word, ps.current_word_len - 1) == 
> true)
> +        {
> +          param_dict_pos = param_dict_len++;
> +          param_keys[param_dict_pos] = grub_malloc (ps.current_word_len);
> +          if (param_keys == NULL)
> +            goto cleanup_main;
> +          param_vals[param_dict_pos] = grub_malloc (2);

grub_zalloc()

> +          if (param_vals == NULL)
> +            goto cleanup_main;
> +
> +          grub_strncpy (param_keys[param_dict_pos], ps.current_word,
> +                     ps.current_word_len);
> +          if (param_keys[param_dict_pos][ps.current_word_len - 1] != '\0' )
> +            grub_fatal ("NUL terminator missing from key during Xen cmdline 
> parsing");
> +          grub_strcpy (param_vals[param_dict_pos], "1");

*param_vals[param_dict_pos] = '1';

I can fix these two issues for you before push...

If I do not hear any objections from Xen community in a week or
so I will push this patch into GRUB upstream git repo.

Anyway, thank you for doing this work!

Daniel