Re: [Qemu-devel] [PULL 18/76] optionrom: add new PVH option rom

[Philippe Mathieu-Daudé]

Hi Stefano,

On 2/5/19 7:14 PM, Paolo Bonzini wrote:
> From: Stefano Garzarella <sgarzare@redhat.com>
> 
> The new pvh.bin option rom can be used with SeaBIOS to boot
> uncompressed kernel using the x86/HVM direct boot ABI.
> 
> pvh.S contains the entry point of the option rom. It runs
> in real mode, loads the e820 table querying the BIOS, and
> then it switches to 32bit protected mode and jumps to the
> pvh_load_kernel() written in pvh_main.c.
> pvh_load_kernel() loads the cmdline and kernel entry_point
> using fw_cfg, then it looks for RSDP, fills the
> hvm_start_info required by x86/HVM ABI, and finally jumps
> to the kernel entry_point.
> 
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Reviewed-by: Liam Merwick <liam.merwick@oracle.com>
> ---
>  .gitignore                   |   4 +
>  Makefile                     |   2 +-
>  pc-bios/optionrom/Makefile   |   5 +-
>  pc-bios/optionrom/pvh.S      | 200 
> +++++++++++++++++++++++++++++++++++++++++++
>  pc-bios/optionrom/pvh_main.c | 116 +++++++++++++++++++++++++
>  pc-bios/pvh.bin              | Bin 0 -> 1536 bytes
>  6 files changed, 325 insertions(+), 2 deletions(-)
>  create mode 100644 pc-bios/optionrom/pvh.S
>  create mode 100644 pc-bios/optionrom/pvh_main.c
>  create mode 100644 pc-bios/pvh.bin

> +++ b/pc-bios/optionrom/Makefile
> @@ -37,7 +37,7 @@ Wa = -Wa,
>  ASFLAGS += -32
>  QEMU_CFLAGS += $(call cc-c-option, $(QEMU_CFLAGS), $(Wa)-32)
>  
> -build-all: multiboot.bin linuxboot.bin linuxboot_dma.bin kvmvapic.bin
> +build-all: multiboot.bin linuxboot.bin linuxboot_dma.bin kvmvapic.bin pvh.bin
>  
> +++ b/pc-bios/optionrom/pvh_main.c
> @@ -0,0 +1,116 @@
> +/*
> + * PVH Option ROM for fw_cfg DMA
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + *
> + * Copyright (c) 2019 Red Hat Inc.
> + *   Authors:
> + *     Stefano Garzarella <sgarzare@redhat.com>
> + */
> +
> +asm (".code32"); /* this code will be executed in protected mode */
> +
> +#include <stddef.h>
> +#include <stdint.h>
> +#include "optrom.h"
> +#include "optrom_fw_cfg.h"
> +#include "../../include/hw/xen/start_info.h"
> +
> +#define RSDP_SIGNATURE          0x2052545020445352LL /* "RSD PTR " */
> +#define RSDP_AREA_ADDR          0x000E0000
> +#define RSDP_AREA_SIZE          2048
> +#define EBDA_BASE_ADDR          0x0000040E
> +#define EBDA_SIZE               1024
> +
> +#define E820_MAXENTRIES         128
> +#define CMDLINE_BUFSIZE         4096
> +
> +/* e820 table filled in pvh.S using int 0x15 */
> +struct pvh_e820_table {
> +    uint32_t entries;
> +    uint32_t reserved;
> +    struct hvm_memmap_table_entry table[E820_MAXENTRIES];
> +};
> +
> +struct pvh_e820_table pvh_e820 asm("pvh_e820") __attribute__ ((aligned));
> +
> +static struct hvm_start_info start_info;
> +static uint8_t cmdline_buffer[CMDLINE_BUFSIZE];
> +
> +
> +/* Search RSDP signature. */
> +static uintptr_t search_rsdp(uint32_t start_addr, uint32_t end_addr)
> +{
> +    uint64_t *rsdp_p;
> +
> +    /* RSDP signature is always on a 16 byte boundary */
> +    for (rsdp_p = (uint64_t *)start_addr; rsdp_p < (uint64_t *)end_addr;
> +         rsdp_p += 2) {
> +        if (*rsdp_p == RSDP_SIGNATURE) {
> +            return (uintptr_t)rsdp_p;
> +        }
> +    }
> +
> +    return 0;
> +}

gcc 10.2.1 "cc (Alpine 10.2.1_pre2) 10.2.1 20210313" reports:

pc-bios/optionrom/pvh_main.c: In function 'search_rsdp':
pc-bios/optionrom/pvh_main.c:61:21: warning: comparison is always false
due to limited range of data type [-Wtype-limits]
   61 |         if (*rsdp_p == RSDP_SIGNATURE) {
      |                     ^~

Can you have a look?

Thanks,

Phil.