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

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

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@xxxxxxxxxx>
> ---
> Â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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel