kvmtool: handle x86 firmware smaller than 128k

Commit Message

Gerd Hoffmann Nov. 6, 2017, 11:48 a.m. UTC

---
 x86/boot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jean-Philippe Brucker Nov. 6, 2017, 5:09 p.m. UTC | #1

Hi Gerd,

This change deserves a comment, as it's not obvious what we're doing here.
How about:

"""
When using an external firmware on x86, kvmtool follows the legacy BIOS
standard, by entering firmware at address 0xffff0 in 16bit real mode.
SeaBIOS images built for emulators follow this convention by having their
reset vector 16 bytes before the end of the image. In order to support
images of arbitrary size, move the image at the end of the BIOS region.
"""

Thanks,
Jean

On 06/11/17 11:48, Gerd Hoffmann wrote:
> ---
>  x86/boot.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/x86/boot.c b/x86/boot.c
> index 61535eb57b..b7a4262d89 100644
> --- a/x86/boot.c
> +++ b/x86/boot.c
> @@ -28,7 +28,7 @@ bool kvm__load_firmware(struct kvm *kvm, const char *firmware_filename)
>  	if (st.st_size > MB_FIRMWARE_BIOS_SIZE)
>  		die("firmware image %s is too big to fit in memory (%Lu KB).\n", firmware_filename, (u64)(st.st_size / 1024));
>  
> -	p = guest_flat_to_host(kvm, MB_FIRMWARE_BIOS_BEGIN);
> +	p = guest_flat_to_host(kvm, MB_BIOS_END + 1 - st.st_size);
>  
>  	while ((nr = read(fd, p, st.st_size)) > 0)
>  		p += nr;
>

Gerd Hoffmann Nov. 7, 2017, 7:33 a.m. UTC | #2

On Mon, 2017-11-06 at 17:09 +0000, Jean-Philippe Brucker wrote:
> Hi Gerd,
> 
> This change deserves a comment, as it's not obvious what we're doing
> here.
> How about:
> 
> """
> When using an external firmware on x86, kvmtool follows the legacy
> BIOS
> standard, by entering firmware at address 0xffff0 in 16bit real mode.

This has nothing to do with the legacy bios standards.  The cpu starts
executing at 0xffff0 after reset, no matter what the firmware is.  It's
the same for uefi and coreboot.

But, yes, mentioning the reset vector is reasonable, it you don't know
that x86 detail it isn't obvious indeed.  Sending v2 in a moment.

cheers,
  Gerd

Patch

diff --git a/x86/boot.c b/x86/boot.c
index 61535eb57b..b7a4262d89 100644
--- a/x86/boot.c
+++ b/x86/boot.c
@@ -28,7 +28,7 @@  bool kvm__load_firmware(struct kvm *kvm, const char *firmware_filename)
 	if (st.st_size > MB_FIRMWARE_BIOS_SIZE)
 		die("firmware image %s is too big to fit in memory (%Lu KB).\n", firmware_filename, (u64)(st.st_size / 1024));
 
-	p = guest_flat_to_host(kvm, MB_FIRMWARE_BIOS_BEGIN);
+	p = guest_flat_to_host(kvm, MB_BIOS_END + 1 - st.st_size);
 
 	while ((nr = read(fd, p, st.st_size)) > 0)
 		p += nr;