[PATCH v4 0/4] hvmloader: make OVMF work with Xen

[PATCH v4 0/4] hvmloader: make OVMF work with Xen

[Wei Liu]

This is version 4 of Xen side patches, but it matches version 3 of OVMF
patches. :-P

Looks like that we are close to establish interface between Xen and OVMF.

Wei.

Changes in V4:
* go back to dedicated OVMF info structure, 64 bit ready

Changes in V3:
* move seabios_info to common code
* e820 entry number is 16 again, one can bump it when necessary
* reserve low reset vector in E820 map in OVMF

Changes in V2:
* ovmf_info is now almost the same as seabios_info
* bump e820 entry number to 128
* modify build_e820_table to avoid BIOS region reservation if there's none

Wei Liu (4):
  hvmloader/ovmf: remove hardcoded OVMF loading location
  hvmloader/ovmf: show OVMF_BEGIN as bios address
  hvmloader/ovmf: setup ovmf_info
  hvmloader/ovmf: setup E820 map

 tools/firmware/hvmloader/ovmf.c |   67 +++++++++++++++++++++++++++++++++++----
 1 file changed, 61 insertions(+), 6 deletions(-)

-- 
1.7.10.4

[PATCH v4 1/4] hvmloader/ovmf: remove hardcoded OVMF loading location

[Wei Liu]

Load OVMF from (100000000ULL - sizeof(ovmf)) to 0xFFFFFFFF, with proper
rounding down.

Tested with OVMF release build (1 MB) and debug build (2 MB), both
worked fine.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/firmware/hvmloader/ovmf.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/firmware/hvmloader/ovmf.c b/tools/firmware/hvmloader/ovmf.c
index ee4cbbf..148102a 100644
--- a/tools/firmware/hvmloader/ovmf.c
+++ b/tools/firmware/hvmloader/ovmf.c
@@ -38,9 +38,9 @@
 #define ROM_INCLUDE_OVMF
 #include "roms.inc"
 
-#define OVMF_BEGIN              0xFFF00000ULL
-#define OVMF_SIZE               0x00100000ULL
+#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
-- 
1.7.10.4

Re: [PATCH v4 1/4] hvmloader/ovmf: remove hardcoded OVMF loading location

[Ian Campbell]

On Tue, 2013-11-26 at 19:31 +0000, Wei Liu wrote:
> Load OVMF from (100000000ULL - sizeof(ovmf)) to 0xFFFFFFFF, with proper
> rounding down.
> 
> Tested with OVMF release build (1 MB) and debug build (2 MB), both
> worked fine.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

I applied this and #2. #3 and #4 I think need to wait for the struct to
be finally agreed with ovmf.

> ---
>  tools/firmware/hvmloader/ovmf.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/firmware/hvmloader/ovmf.c b/tools/firmware/hvmloader/ovmf.c
> index ee4cbbf..148102a 100644
> --- a/tools/firmware/hvmloader/ovmf.c
> +++ b/tools/firmware/hvmloader/ovmf.c
> @@ -38,9 +38,9 @@
>  #define ROM_INCLUDE_OVMF
>  #include "roms.inc"
>  
> -#define OVMF_BEGIN              0xFFF00000ULL
> -#define OVMF_SIZE               0x00100000ULL
> +#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

[PATCH v4 2/4] hvmloader/ovmf: show OVMF_BEGIN as bios address

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/firmware/hvmloader/ovmf.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/firmware/hvmloader/ovmf.c b/tools/firmware/hvmloader/ovmf.c
index 148102a..a67a2de 100644
--- a/tools/firmware/hvmloader/ovmf.c
+++ b/tools/firmware/hvmloader/ovmf.c
@@ -99,7 +99,7 @@ struct bios_config ovmf_config =  {
     .image = ovmf,
     .image_size = sizeof(ovmf),
 
-    .bios_address = 0,
+    .bios_address = OVMF_BEGIN,
     .bios_load = ovmf_load,
 
     .load_roms = 0,
-- 
1.7.10.4

[PATCH v4 3/4] hvmloader/ovmf: setup ovmf_info

[Wei Liu]

OVMF info contains E820 map allocated by hvmloader. This info is passed
to OVMF to help it do proper initialization.

Currently only E820 is necessary, but we reserve spaces for other tables
in ovmf_info for later usage.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/firmware/hvmloader/ovmf.c |   48 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 46 insertions(+), 2 deletions(-)

diff --git a/tools/firmware/hvmloader/ovmf.c b/tools/firmware/hvmloader/ovmf.c
index a67a2de..d536501 100644
--- a/tools/firmware/hvmloader/ovmf.c
+++ b/tools/firmware/hvmloader/ovmf.c
@@ -46,10 +46,54 @@
 #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[];
 extern int dsdt_anycpu_len;
 
+#define OVMF_INFO_MAX_TABLES 4
+struct ovmf_info {
+    char signature[14]; /* XenHVMOVMF\0\0\0\0 */
+    uint8_t length;     /* Length of this struct */
+    uint8_t checksum;   /* Set such that the sum over bytes 0..length == 0 */
+    /*
+     * Physical address of an array of tables_nr elements.
+     *
+     * Each element is a 64 bit value containing the physical address
+     * of a BIOS table.
+     */
+    uint64_t tables;
+    uint32_t tables_nr;
+    /*
+     * Physical address of the e820 table, contains e820_nr entries.
+     */
+    uint64_t e820;
+    uint32_t e820_nr;
+} __attribute__ ((packed));
+
+static void ovmf_setup_bios_info(void)
+{
+    struct ovmf_info *info = (void *)OVMF_INFO_PHYSICAL_ADDRESS;
+
+    memset(info, 0, sizeof(*info));
+
+    memcpy(info->signature, "XenHVMOVMF", sizeof(info->signature));
+    info->length = sizeof(*info);
+}
+
+static void ovmf_finish_bios_info(void)
+{
+    struct ovmf_info *info = (void *)OVMF_INFO_PHYSICAL_ADDRESS;
+    uint32_t i;
+    uint8_t checksum;
+
+    checksum = 0;
+    for ( i = 0; i < info->length; i++ )
+        checksum += ((uint8_t *)(info))[i];
+
+    info->checksum = -checksum;
+}
+
 static void ovmf_load(const struct bios_config *config)
 {
     xen_pfn_t mfn;
@@ -104,8 +148,8 @@ struct bios_config ovmf_config =  {
 
     .load_roms = 0,
 
-    .bios_info_setup = NULL,
-    .bios_info_finish = NULL,
+    .bios_info_setup = ovmf_setup_bios_info,
+    .bios_info_finish = ovmf_finish_bios_info,
 
     .e820_setup = NULL,
 
-- 
1.7.10.4

Re: [PATCH v4 3/4] hvmloader/ovmf: setup ovmf_info

[Konrad Rzeszutek Wilk]

On Tue, Nov 26, 2013 at 07:31:43PM +0000, Wei Liu wrote:
> OVMF info contains E820 map allocated by hvmloader. This info is passed
> to OVMF to help it do proper initialization.
> 
> Currently only E820 is necessary, but we reserve spaces for other tables
> in ovmf_info for later usage.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  tools/firmware/hvmloader/ovmf.c |   48 +++++++++++++++++++++++++++++++++++++--
>  1 file changed, 46 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/firmware/hvmloader/ovmf.c b/tools/firmware/hvmloader/ovmf.c
> index a67a2de..d536501 100644
> --- a/tools/firmware/hvmloader/ovmf.c
> +++ b/tools/firmware/hvmloader/ovmf.c
> @@ -46,10 +46,54 @@
>  #define LOWCHUNK_SIZE           0x00010000
>  #define LOWCHUNK_MAXOFFSET      0x0000FFFF
>  #define LOWCHUNK_END            (OVMF_BEGIN + OVMF_SIZE)
> +#define OVMF_INFO_PHYSICAL_ADDRESS 0X00001000
                                       ^-x

>  
>  extern unsigned char dsdt_anycpu[];
>  extern int dsdt_anycpu_len;
>  
> +#define OVMF_INFO_MAX_TABLES 4
> +struct ovmf_info {
> +    char signature[14]; /* XenHVMOVMF\0\0\0\0 */
> +    uint8_t length;     /* Length of this struct */
> +    uint8_t checksum;   /* Set such that the sum over bytes 0..length == 0 */
> +    /*
> +     * Physical address of an array of tables_nr elements.
> +     *
> +     * Each element is a 64 bit value containing the physical address
> +     * of a BIOS table.
> +     */
> +    uint64_t tables;
> +    uint32_t tables_nr;
> +    /*
> +     * Physical address of the e820 table, contains e820_nr entries.
> +     */
> +    uint64_t e820;
> +    uint32_t e820_nr;
> +} __attribute__ ((packed));
> +
> +static void ovmf_setup_bios_info(void)
> +{
> +    struct ovmf_info *info = (void *)OVMF_INFO_PHYSICAL_ADDRESS;
> +
> +    memset(info, 0, sizeof(*info));
> +
> +    memcpy(info->signature, "XenHVMOVMF", sizeof(info->signature));
> +    info->length = sizeof(*info);
> +}
> +
> +static void ovmf_finish_bios_info(void)
> +{
> +    struct ovmf_info *info = (void *)OVMF_INFO_PHYSICAL_ADDRESS;
> +    uint32_t i;
> +    uint8_t checksum;
> +
> +    checksum = 0;
> +    for ( i = 0; i < info->length; i++ )
> +        checksum += ((uint8_t *)(info))[i];
> +
> +    info->checksum = -checksum;
> +}
> +
>  static void ovmf_load(const struct bios_config *config)
>  {
>      xen_pfn_t mfn;
> @@ -104,8 +148,8 @@ struct bios_config ovmf_config =  {
>  
>      .load_roms = 0,
>  
> -    .bios_info_setup = NULL,
> -    .bios_info_finish = NULL,
> +    .bios_info_setup = ovmf_setup_bios_info,
> +    .bios_info_finish = ovmf_finish_bios_info,
>  
>      .e820_setup = NULL,
>  
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

[PATCH v4 4/4] hvmloader/ovmf: setup E820 map

[Wei Liu]

E820 map will be used by OVMF to create memory map.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/firmware/hvmloader/ovmf.c |   13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/tools/firmware/hvmloader/ovmf.c b/tools/firmware/hvmloader/ovmf.c
index d536501..3f8cf3e 100644
--- a/tools/firmware/hvmloader/ovmf.c
+++ b/tools/firmware/hvmloader/ovmf.c
@@ -137,6 +137,17 @@ static void ovmf_create_smbios_tables(void)
         SMBIOS_PHYSICAL_END);
 }
 
+static void ovmf_setup_e820(void)
+{
+    struct ovmf_info *info = (void *)OVMF_INFO_PHYSICAL_ADDRESS;
+    struct e820entry *e820 = scratch_alloc(sizeof(struct e820entry)*16, 0);
+    info->e820 = (uint32_t)e820;
+
+    /* Reserve LOWCHUNK_BEGIN to 0x100000 as well, that's reset vector. */
+    info->e820_nr = build_e820_table(e820, 0, LOWCHUNK_BEGIN);
+    dump_e820_table(e820, info->e820_nr);
+}
+
 struct bios_config ovmf_config =  {
     .name = "OVMF",
 
@@ -151,7 +162,7 @@ struct bios_config ovmf_config =  {
     .bios_info_setup = ovmf_setup_bios_info,
     .bios_info_finish = ovmf_finish_bios_info,
 
-    .e820_setup = NULL,
+    .e820_setup = ovmf_setup_e820,
 
     .acpi_build_tables = ovmf_acpi_build_tables,
     .create_mp_tables = NULL,
-- 
1.7.10.4

Re: [PATCH v4 4/4] hvmloader/ovmf: setup E820 map

[Jordan Justen]

On Tue, Nov 26, 2013 at 11:31 AM, Wei Liu <wei.liu2@citrix.com> wrote:
> E820 map will be used by OVMF to create memory map.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  tools/firmware/hvmloader/ovmf.c |   13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/tools/firmware/hvmloader/ovmf.c b/tools/firmware/hvmloader/ovmf.c
> index d536501..3f8cf3e 100644
> --- a/tools/firmware/hvmloader/ovmf.c
> +++ b/tools/firmware/hvmloader/ovmf.c
> @@ -137,6 +137,17 @@ static void ovmf_create_smbios_tables(void)
>          SMBIOS_PHYSICAL_END);
>  }
>
> +static void ovmf_setup_e820(void)
> +{
> +    struct ovmf_info *info = (void *)OVMF_INFO_PHYSICAL_ADDRESS;
> +    struct e820entry *e820 = scratch_alloc(sizeof(struct e820entry)*16, 0);
> +    info->e820 = (uint32_t)e820;

uint32_t => uintptr_t?

> +    /* Reserve LOWCHUNK_BEGIN to 0x100000 as well, that's reset vector. */
> +    info->e820_nr = build_e820_table(e820, 0, LOWCHUNK_BEGIN);
> +    dump_e820_table(e820, info->e820_nr);
> +}
> +
>  struct bios_config ovmf_config =  {
>      .name = "OVMF",
>
> @@ -151,7 +162,7 @@ struct bios_config ovmf_config =  {
>      .bios_info_setup = ovmf_setup_bios_info,
>      .bios_info_finish = ovmf_finish_bios_info,
>
> -    .e820_setup = NULL,
> +    .e820_setup = ovmf_setup_e820,
>
>      .acpi_build_tables = ovmf_acpi_build_tables,
>      .create_mp_tables = NULL,
> --
> 1.7.10.4
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH v4 4/4] hvmloader/ovmf: setup E820 map

[Wei Liu]

On Sat, Nov 30, 2013 at 04:05:10PM -0800, Jordan Justen wrote:
> On Tue, Nov 26, 2013 at 11:31 AM, Wei Liu <wei.liu2@citrix.com> wrote:
> > E820 map will be used by OVMF to create memory map.
> >
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> >  tools/firmware/hvmloader/ovmf.c |   13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/firmware/hvmloader/ovmf.c b/tools/firmware/hvmloader/ovmf.c
> > index d536501..3f8cf3e 100644
> > --- a/tools/firmware/hvmloader/ovmf.c
> > +++ b/tools/firmware/hvmloader/ovmf.c
> > @@ -137,6 +137,17 @@ static void ovmf_create_smbios_tables(void)
> >          SMBIOS_PHYSICAL_END);
> >  }
> >
> > +static void ovmf_setup_e820(void)
> > +{
> > +    struct ovmf_info *info = (void *)OVMF_INFO_PHYSICAL_ADDRESS;
> > +    struct e820entry *e820 = scratch_alloc(sizeof(struct e820entry)*16, 0);
> > +    info->e820 = (uint32_t)e820;
> 
> uint32_t => uintptr_t?
> 

I think following SeaBIOS convention is good enough at this stage. We
don't have uintptr_t now, everything has explicit size.

Wei.