Re: [PATCH v1 02/18] introduction of generalized boot info

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.07.2022 23:04, Daniel P. Smith wrote:
> --- /dev/null
> +++ b/xen/arch/x86/include/asm/bootinfo.h
> @@ -0,0 +1,48 @@
> +#ifndef __ARCH_X86_BOOTINFO_H__
> +#define __ARCH_X86_BOOTINFO_H__
> +
> +/* unused for x86 */
> +struct arch_bootstring { };
> +
> +struct __packed arch_bootmodule {
> +#define BOOTMOD_FLAG_X86_RELOCATED      1U << 0

Such macro expansions need parenthesizing.

> +    uint32_t flags;
> +    uint32_t headroom;
> +};

Since you're not following any external spec, on top of what Julien
said about the __packed attribute I'd also like to point out that
in many cases here there's no need to use fixed-width types.

> +struct __packed arch_boot_info {
> +    uint32_t flags;
> +#define BOOTINFO_FLAG_X86_MEMLIMITS          1U << 0
> +#define BOOTINFO_FLAG_X86_BOOTDEV            1U << 1
> +#define BOOTINFO_FLAG_X86_CMDLINE            1U << 2
> +#define BOOTINFO_FLAG_X86_MODULES            1U << 3
> +#define BOOTINFO_FLAG_X86_AOUT_SYMS          1U << 4
> +#define BOOTINFO_FLAG_X86_ELF_SYMS           1U << 5
> +#define BOOTINFO_FLAG_X86_MEMMAP             1U << 6
> +#define BOOTINFO_FLAG_X86_DRIVES             1U << 7
> +#define BOOTINFO_FLAG_X86_BIOSCONFIG         1U << 8
> +#define BOOTINFO_FLAG_X86_LOADERNAME         1U << 9
> +#define BOOTINFO_FLAG_X86_APM                1U << 10
> +
> +    bool xen_guest;

As the example of this, with just the header files being introduced
here it is not really possible to figure what these fields are to
be used for and hence whether they're legitimately represented here.

> +    char *boot_loader_name;
> +    char *kextra;

const?

Jan