Re: [PATCH v4 08/14] hvmloader: Locate the BIOS blob

["Jan Beulich" <JBeulich@suse.com>]

>>> On 14.03.16 at 18:55, <anthony.perard@citrix.com> wrote:
> --- a/tools/firmware/hvmloader/hvmloader.c
> +++ b/tools/firmware/hvmloader/hvmloader.c
> @@ -253,10 +253,40 @@ static void acpi_enable_sci(void)
>      BUG_ON(!(pm1a_cnt_val & ACPI_PM1C_SCI_EN));
>  }
>  
> +const struct hvm_modlist_entry *get_module_entry(
> +    const struct hvm_start_info *info,
> +    const char *name)
> +{
> +    const struct hvm_modlist_entry *modlist =
> +        (struct hvm_modlist_entry *)info->modlist_paddr;

This cast puzzles me (as at the first glance I would expect it to
cause a compiler warning): Roger, how come cmdline_paddr,
modlist_paddr, and rsdp_paddr are 32-bit quantities? While on
x86 that _may_ be fine, what about other architectures we may
want to run Xen on?

> +    unsigned int i;
> +
> +    if ( !modlist )
> +        return NULL;
> +
> +    for ( i = 0; i < info->nr_modules; i++ )
> +    {
> +        uint32_t module_name = modlist[i].cmdline_paddr;
> +
> +        BUG_ON(!modlist[i].cmdline_paddr ||
> +               modlist[i].cmdline_paddr > UINT_MAX);

While you can't use the local variable for the latter check, you can
for the former.

> @@ -292,8 +322,16 @@ int main(void)
>      }
>  
>      printf("Loading %s ...\n", bios->name);
> -    if ( bios->bios_load )
> -        bios->bios_load(bios);
> +    bios_module = get_module_entry(hvm_start_info, "bios");

Isn't "bios" a bit vague, as there could be multiple (system, video,
add-on card)?

> +    if ( bios_module && bios->bios_load )
> +    {
> +        uint32_t paddr = bios_module->paddr;
> +        bios->bios_load(bios, (void*)paddr, bios_module->size);

Blank line between declaration(s) and statement(s) please.

> +    }
> +    else if ( bios->bios_load )
> +    {
> +        bios->bios_load(bios, 0, 0);

        bios->bios_load(bios, NULL, 0);

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel