diff mbox

[v4,11/14] hvmloader: Load OVMF from modules

Message ID 1457978150-27201-12-git-send-email-anthony.perard@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Anthony PERARD March 14, 2016, 5:55 p.m. UTC
... and do not include the OVMF ROM into hvmloader anymore.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
Change in V4:
- check if source and dest of ovmf binary does not overlaps

Change in V3:
- change makefile to not include ovmf rom into roms.inc.
- update the struct bios_config with bios destination
---
 tools/firmware/hvmloader/Makefile | 15 +--------------
 tools/firmware/hvmloader/ovmf.c   | 30 +++++++++++++-----------------
 2 files changed, 14 insertions(+), 31 deletions(-)

Comments

Konrad Rzeszutek Wilk March 16, 2016, 1:36 a.m. UTC | #1
On Mon, Mar 14, 2016 at 05:55:46PM +0000, Anthony PERARD wrote:
> ... and do not include the OVMF ROM into hvmloader anymore.
> 

Yeey..

> diff --git a/tools/firmware/hvmloader/ovmf.c b/tools/firmware/hvmloader/ovmf.c
> index 858a2d4..6607e57 100644
> --- a/tools/firmware/hvmloader/ovmf.c
> +++ b/tools/firmware/hvmloader/ovmf.c
> @@ -34,17 +34,10 @@
>  #include <xen/hvm/ioreq.h>
>  #include <xen/memory.h>
>  
> -#define ROM_INCLUDE_OVMF
> -#include "roms.inc"
> -
> -#define OVMF_SIZE               (sizeof(ovmf))
>  #define OVMF_MAXOFFSET          0x000FFFFFULL
> -#define OVMF_BEGIN              (0x100000000ULL - ((OVMF_SIZE + OVMF_MAXOFFSET) & ~OVMF_MAXOFFSET))
> -#define OVMF_END                (OVMF_BEGIN + OVMF_SIZE)
>  #define LOWCHUNK_BEGIN          0x000F0000
>  #define LOWCHUNK_SIZE           0x00010000
>  #define LOWCHUNK_MAXOFFSET      0x0000FFFF
> -#define LOWCHUNK_END            (OVMF_BEGIN + OVMF_SIZE)
>  #define OVMF_INFO_PHYSICAL_ADDRESS 0x00001000
>  
>  extern unsigned char dsdt_anycpu_qemu_xen[];
> @@ -97,24 +90,31 @@ static void ovmf_load(const struct bios_config *config,
>                        void *bios_addr, uint32_t bios_length)
>  {
>      xen_pfn_t mfn;
> -    uint64_t addr = OVMF_BEGIN;
> +    uint64_t addr = 0x100000000ULL

Perhaps still leave an macro for 0x100000000ULL?

> +        - ((bios_length + OVMF_MAXOFFSET) & ~OVMF_MAXOFFSET);
> +    uint64_t ovmf_end = addr + bios_length;
> +
> +    ovmf_config.bios_address = addr;
> +    ovmf_config.image_size = bios_length;
>  
>      /* Copy low-reset vector portion. */
> -    memcpy((void *) LOWCHUNK_BEGIN, (uint8_t *) config->image
> -           + OVMF_SIZE
> -           - LOWCHUNK_SIZE,
> +    memcpy((void *) LOWCHUNK_BEGIN,
> +           (uint8_t *) bios_addr + bios_length - LOWCHUNK_SIZE,
>             LOWCHUNK_SIZE);
>  
>      /* Ensure we have backing page prior to moving FD. */
> -    while ( (addr >> PAGE_SHIFT) != (OVMF_END >> PAGE_SHIFT) )
> +    while ( (addr >> PAGE_SHIFT) != (ovmf_end >> PAGE_SHIFT) )
>      {
>          mfn = (uint32_t) (addr >> PAGE_SHIFT);
>          addr += PAGE_SIZE;
>          mem_hole_populate_ram(mfn, 1);
>      }
>  
> +    /* Check that source and destination does not overlaps. */

s/overlaps/overlap/

> +    BUG_ON(addr + bios_length > (unsigned)bios_addr
> +           && addr < (unsigned)bios_addr + bios_length);
>      /* Copy FD. */
> -    memcpy((void *) OVMF_BEGIN, config->image, OVMF_SIZE);
> +    memcpy((void *) ovmf_config.bios_address, bios_addr, bios_length);
>  }
>  
>  static void ovmf_acpi_build_tables(void)
> @@ -151,10 +151,6 @@ static void ovmf_setup_e820(void)
>  struct bios_config ovmf_config =  {
>      .name = "OVMF",
>  
> -    .image = ovmf,
> -    .image_size = sizeof(ovmf),
> -
> -    .bios_address = OVMF_BEGIN,
>      .bios_load = ovmf_load,
>  
>      .load_roms = 0,
> -- 
> Anthony PERARD
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
Jan Beulich April 5, 2016, 1:16 p.m. UTC | #2
>>> On 14.03.16 at 18:55, <anthony.perard@citrix.com> wrote:
> ... and do not include the OVMF ROM into hvmloader anymore.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>
with some cosmetics left to be done:

>      /* Copy low-reset vector portion. */
> -    memcpy((void *) LOWCHUNK_BEGIN, (uint8_t *) config->image
> -           + OVMF_SIZE
> -           - LOWCHUNK_SIZE,
> +    memcpy((void *) LOWCHUNK_BEGIN,
> +           (uint8_t *) bios_addr + bios_length - LOWCHUNK_SIZE,

Please take the opportunity and remove the stray blanks here ...

>      /* Copy FD. */
> -    memcpy((void *) OVMF_BEGIN, config->image, OVMF_SIZE);
> +    memcpy((void *) ovmf_config.bios_address, bios_addr, bios_length);

... and here (between cast's closing paren and casted expression).

Jan
diff mbox

Patch

diff --git a/tools/firmware/hvmloader/Makefile b/tools/firmware/hvmloader/Makefile
index ed0bfad..dee1c6b 100644
--- a/tools/firmware/hvmloader/Makefile
+++ b/tools/firmware/hvmloader/Makefile
@@ -43,7 +43,6 @@  endif
 
 CIRRUSVGA_DEBUG ?= n
 
-OVMF_DIR := ../ovmf-dir
 ROMBIOS_DIR := ../rombios
 
 ifeq ($(CONFIG_ROMBIOS),y)
@@ -61,12 +60,6 @@  ROMS :=
 ifeq ($(CONFIG_OVMF),y)
 OBJS += ovmf.o
 CFLAGS += -DENABLE_OVMF
-ifeq ($(OVMF_PATH),)
-	OVMF_ROM := $(OVMF_DIR)/ovmf.bin
-else
-	OVMF_ROM := $(OVMF_PATH)
-endif
-ROMS += $(OVMF_ROM)
 endif
 
 ifeq ($(CONFIG_ROMBIOS),y)
@@ -85,7 +78,7 @@  endif
 all: subdirs-all
 	$(MAKE) hvmloader
 
-ovmf.o rombios.o: roms.inc
+rombios.o: roms.inc
 smbios.o: CFLAGS += -D__SMBIOS_DATE__="\"$(SMBIOS_REL_DATE)\""
 
 hvmloader: $(OBJS) acpi/acpi.a
@@ -102,12 +95,6 @@  ifneq ($(ROMBIOS_ROM),)
 	echo "#endif" >> $@.new
 endif
 
-ifneq ($(OVMF_ROM),)
-	echo "#ifdef ROM_INCLUDE_OVMF" >> $@.new
-	sh ./mkhex ovmf $(OVMF_ROM) >> $@.new
-	echo "#endif" >> $@.new	
-endif 
-
 ifneq ($(STDVGA_ROM),)
 	echo "#ifdef ROM_INCLUDE_VGABIOS" >> $@.new
 	sh ./mkhex vgabios_stdvga $(STDVGA_ROM) >> $@.new
diff --git a/tools/firmware/hvmloader/ovmf.c b/tools/firmware/hvmloader/ovmf.c
index 858a2d4..6607e57 100644
--- a/tools/firmware/hvmloader/ovmf.c
+++ b/tools/firmware/hvmloader/ovmf.c
@@ -34,17 +34,10 @@ 
 #include <xen/hvm/ioreq.h>
 #include <xen/memory.h>
 
-#define ROM_INCLUDE_OVMF
-#include "roms.inc"
-
-#define OVMF_SIZE               (sizeof(ovmf))
 #define OVMF_MAXOFFSET          0x000FFFFFULL
-#define OVMF_BEGIN              (0x100000000ULL - ((OVMF_SIZE + OVMF_MAXOFFSET) & ~OVMF_MAXOFFSET))
-#define OVMF_END                (OVMF_BEGIN + OVMF_SIZE)
 #define LOWCHUNK_BEGIN          0x000F0000
 #define LOWCHUNK_SIZE           0x00010000
 #define LOWCHUNK_MAXOFFSET      0x0000FFFF
-#define LOWCHUNK_END            (OVMF_BEGIN + OVMF_SIZE)
 #define OVMF_INFO_PHYSICAL_ADDRESS 0x00001000
 
 extern unsigned char dsdt_anycpu_qemu_xen[];
@@ -97,24 +90,31 @@  static void ovmf_load(const struct bios_config *config,
                       void *bios_addr, uint32_t bios_length)
 {
     xen_pfn_t mfn;
-    uint64_t addr = OVMF_BEGIN;
+    uint64_t addr = 0x100000000ULL
+        - ((bios_length + OVMF_MAXOFFSET) & ~OVMF_MAXOFFSET);
+    uint64_t ovmf_end = addr + bios_length;
+
+    ovmf_config.bios_address = addr;
+    ovmf_config.image_size = bios_length;
 
     /* Copy low-reset vector portion. */
-    memcpy((void *) LOWCHUNK_BEGIN, (uint8_t *) config->image
-           + OVMF_SIZE
-           - LOWCHUNK_SIZE,
+    memcpy((void *) LOWCHUNK_BEGIN,
+           (uint8_t *) bios_addr + bios_length - LOWCHUNK_SIZE,
            LOWCHUNK_SIZE);
 
     /* Ensure we have backing page prior to moving FD. */
-    while ( (addr >> PAGE_SHIFT) != (OVMF_END >> PAGE_SHIFT) )
+    while ( (addr >> PAGE_SHIFT) != (ovmf_end >> PAGE_SHIFT) )
     {
         mfn = (uint32_t) (addr >> PAGE_SHIFT);
         addr += PAGE_SIZE;
         mem_hole_populate_ram(mfn, 1);
     }
 
+    /* Check that source and destination does not overlaps. */
+    BUG_ON(addr + bios_length > (unsigned)bios_addr
+           && addr < (unsigned)bios_addr + bios_length);
     /* Copy FD. */
-    memcpy((void *) OVMF_BEGIN, config->image, OVMF_SIZE);
+    memcpy((void *) ovmf_config.bios_address, bios_addr, bios_length);
 }
 
 static void ovmf_acpi_build_tables(void)
@@ -151,10 +151,6 @@  static void ovmf_setup_e820(void)
 struct bios_config ovmf_config =  {
     .name = "OVMF",
 
-    .image = ovmf,
-    .image_size = sizeof(ovmf),
-
-    .bios_address = OVMF_BEGIN,
     .bios_load = ovmf_load,
 
     .load_roms = 0,