Re: [RFC PATCH v2 10/16] hvmloader: Load OVMF from modules

[Ian Campbell <ian.campbell@citrix.com>]

On Mon, 2015-10-26 at 16:03 +0000, Anthony PERARD wrote:
> ... and do not include the OVMF ROM into hvmloader anymore.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  tools/firmware/hvmloader/ovmf.c | 22 +++++++---------------
>  1 file changed, 7 insertions(+), 15 deletions(-)
> 
> diff --git a/tools/firmware/hvmloader/ovmf.c
> b/tools/firmware/hvmloader/ovmf.c
> index 2be9752..3c0ec91 100644
> --- a/tools/firmware/hvmloader/ovmf.c
> +++ b/tools/firmware/hvmloader/ovmf.c
> @@ -34,17 +34,9 @@
>  #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[];
> @@ -96,12 +88,16 @@ static void ovmf_finish_bios_info(void)
>  static void ovmf_load(const struct bios_config *config,
>                        void *bios_addr, uint32_t bios_length)
>  {
> +#define OVMF_MAXOFFSET  0x000FFFFFULL
> +#define OVMF_BEGIN      (0x100000000ULL - ((bios_length + OVMF_MAXOFFSET) & ~OVMF_MAXOFFSET))
> +#define OVMF_END        (OVMF_BEGIN + bios_length)

Would be far better converted to proper (possibly const) local variables
IMHO. But if you don't want to do that (and can give a good reason not to)
then you should at least #undef them when the things they refer to go out
of scope.

>      xen_pfn_t mfn;
>      uint64_t addr = OVMF_BEGIN;
> +    unsigned int dest = OVMF_BEGIN;

Two things both assigned OVMF_BEGIN, but with very differently sized types.
One of them is suspicious (IMHO the new one)
>  

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