Re: [PATCH] Replace bios_relocate hook with bios_load hook in hvmloader

[Keir Fraser <keir@xen.org>]

On 23/07/2011 20:02, "Bei Guan" <gbtju85@gmail.com> wrote:

> Hi,
> 
> These set of patches are affected by replacing bios_relocate hook with
> bios_load hook in hvmloader. The patches for code files config.h and
> hvmloader.c also contains part of the contents of Enabling UEFI BIOS(OVMF)
> support in Xen-unstable HVM. Is there any problem with these patches? Thank
> you very much.

As of xen-unstable:23745 I've made some improvements to hvmloader which
include your new bios_load hook. You can rebase your remaining patches on
top of that.

 -- Keir

> 
> 
> # HG changeset patch
> # User gbtju85@gmail.com
> #
> 
> Replace bios_relocate hook with bios_load hook in hvmloader.
> This patch also contains part of the contents of Enabling UEFI BIOS(OVMF)
> support in Xen-unstable HVM
> 
> Sign-off-by: Bei Guan <gbtju85@gmail.com>
> 
> diff -r 42edf1481c57 tools/firmware/hvmloader/config.h
> --- a/tools/firmware/hvmloader/config.h Fri Jul 22 08:55:19 2011 +0100
> +++ b/tools/firmware/hvmloader/config.h Sun Jul 24 02:22:42 2011 +0800
> @@ -3,7 +3,7 @@
>  
>  #include <stdint.h>
>  
> -enum virtual_vga { VGA_none, VGA_std, VGA_cirrus, VGA_pt };
> +enum virtual_vga { VGA_none, VGA_std, VGA_cirrus, VGA_pt, VGA_custom };
>  extern enum virtual_vga virtual_vga;
>  
>  struct bios_config {
> @@ -16,6 +16,9 @@
>      /* Physical address to load at */
>      unsigned int bios_address;
>  
> +    /* Custom load function. */
> +    void (*load)(const struct bios_config *config);
> +
>      /* ROMS */
>      int load_roms;
>      unsigned int optionrom_start, optionrom_end;
> @@ -23,8 +26,6 @@
>      void (*bios_info_setup)(void);
>      void (*bios_info_finish)(void);
>  
> -    void (*bios_relocate)(void);
> -
>      void (*vm86_setup)(void);
>      void (*e820_setup)(void);
>  
> @@ -36,6 +37,8 @@
>  
>  extern struct bios_config rombios_config;
>  extern struct bios_config seabios_config;
> +extern struct bios_config ovmf32_config;
> +extern struct bios_config ovmf64_config;
>  
>  #define PAGE_SHIFT 12
>  #define PAGE_SIZE  (1ul << PAGE_SHIFT)
> diff -r 42edf1481c57 tools/firmware/hvmloader/hvmloader.c
> --- a/tools/firmware/hvmloader/hvmloader.c Fri Jul 22 08:55:19 2011 +0100
> +++ b/tools/firmware/hvmloader/hvmloader.c Sun Jul 24 02:22:42 2011 +0800
> @@ -360,6 +360,8 @@
>  #ifdef ENABLE_SEABIOS
>      { "seabios", &seabios_config, },
>  #endif
> +    { "ovmf-ia32", &ovmf32_config, },
> +    { "ovmf-x64", &ovmf64_config, },
>      { NULL, NULL }
>  };
>  
> @@ -416,12 +418,13 @@
>          bios->create_smbios_tables();
>      }
>  
> -    printf("Loading %s ...\n", bios->name);
> -    memcpy((void *)bios->bios_address, bios->image,
> -           bios->image_size);
> -
> -    if (bios->bios_relocate)
> -        bios->bios_relocate();
> +    if (bios->load) {
> +        bios->load(bios);
> +    } else {
> +        printf("Loading %s ...\n", bios->name);
> +        memcpy((void *)bios->bios_address, bios->image,
> +               bios->image_size);
> +    }
>  
>      if ( (hvm_info->nr_vcpus > 1) || hvm_info->apic_mode ) {
>          if ( bios->create_mp_tables )
> @@ -451,6 +454,8 @@
>              vgabios_sz = round_option_rom(
>                  (*(uint8_t *)(VGABIOS_PHYSICAL_ADDRESS+2)) * 512);
>              break;
> +        case VGA_custom:
> +            break;
>          default:
>              printf("No emulated VGA adaptor ...\n");
>              break;
> diff -r 42edf1481c57 tools/firmware/hvmloader/rombios.c
> --- a/tools/firmware/hvmloader/rombios.c Fri Jul 22 08:55:19 2011 +0100
> +++ b/tools/firmware/hvmloader/rombios.c Sun Jul 24 02:22:42 2011 +0800
> @@ -81,11 +81,15 @@
>      memset(info, 0, sizeof(*info));
>  }
>  
> -static void rombios_relocate(void)
> +static void rombios_load(const struct bios_config *config)
>  {
>      uint32_t bioshigh;
>      struct rombios_info *info;
>  
> +    printf("Loading %s ...\n", config->name);
> +    memcpy((void *)config->bios_address, config->image,
> +           config->image_size);
> +
>      bioshigh = rombios_highbios_setup();
>  
>      info = (struct rombios_info *)BIOS_INFO_PHYSICAL_ADDRESS;
> @@ -163,6 +167,7 @@
>      .image_size = sizeof(rombios),
>  
>      .bios_address = ROMBIOS_PHYSICAL_ADDRESS,
> +    .load = rombios_load,
>  
>      .load_roms = 1,
>  
> @@ -172,8 +177,6 @@
>      .bios_info_setup = rombios_setup_bios_info,
>      .bios_info_finish = NULL,
>  
> -    .bios_relocate = rombios_relocate,
> -
>      .vm86_setup = rombios_init_vm86_tss,
>      .e820_setup = rombios_setup_e820,
>  
> diff -r 42edf1481c57 tools/firmware/hvmloader/seabios.c
> --- a/tools/firmware/hvmloader/seabios.c Fri Jul 22 08:55:19 2011 +0100
> +++ b/tools/firmware/hvmloader/seabios.c Sun Jul 24 02:22:42 2011 +0800
> @@ -132,6 +132,7 @@
>      .image_size = sizeof(seabios),
>  
>      .bios_address = SEABIOS_PHYSICAL_ADDRESS,
> +    .load = NULL,
>  
>      .load_roms = 0,
>  
> @@ -141,8 +142,6 @@
>      .bios_info_setup = seabios_setup_bios_info,
>      .bios_info_finish = seabios_finish_bios_info,
>  
> -    .bios_relocate = NULL,
> -
>      .vm86_setup = NULL,
>      .e820_setup = seabios_setup_e820,
>  
> 
> 
> 
> 
> Best Regards,
> Bei Guan
> 
> 
> 
> 
> 2011/7/24 Keir Fraser <keir.xen@gmail.com>
>> On 23/07/2011 16:18, "Bei Guan" <gbtju85@gmail.com> wrote:
>> 
>>> Do you mean that put the bios_relocate hook in the "else" statement? Just
>>> like
>>> this:
>>> 
>>>     if (bios->load) {
>>>         bios->load(bios);
>>>     } else {
>>>         printf("Loading %s ...\n", bios->name);
>>>         memcpy((void *)bios->bios_address, bios->image,
>>>                bios->image_size);
>>> 
>>>         if (bios->bios_relocate)
>>>             bios->bios_relocate(); 
>>>    }
>> 
>> No I mean remove the bios_relocate hook entirely, and modify the rombios
>> handler to use your new hook instead. It should be quite easy.
>> 
>>  -- Keir
>> 
>> 
> 
>