Patchwork [kvmtool,4/6] arm: Support firmware loading

login
register
mail settings
Submitter Julien Thierry
Date Dec. 4, 2018, 11:14 a.m.
Message ID <1543922073-55530-5-git-send-email-julien.thierry@arm.com>
Download mbox | patch
Permalink /patch/671805/
State New
Headers show

Comments

Julien Thierry - Dec. 4, 2018, 11:14 a.m.
Implement firmware image loading for arm and set the boot start address
to the firmware address.

Add an option for the user to specify where to map the firmware.

Signed-off-by: Julien Thierry <julien.thierry@arm.com>
---
 arm/fdt.c                                | 14 +++++++-
 arm/include/arm-common/kvm-config-arch.h |  5 ++-
 arm/kvm.c                                | 58 +++++++++++++++++++++++++++++++-
 3 files changed, 74 insertions(+), 3 deletions(-)
Andre Przywara - Dec. 14, 2018, 6:08 p.m.
On Tue,  4 Dec 2018 11:14:31 +0000
Julien Thierry <julien.thierry@arm.com> wrote:

Hi,

> Implement firmware image loading for arm and set the boot start
> address to the firmware address.
> 
> Add an option for the user to specify where to map the firmware.

How is the user supposed to know this address? Will EDKII be linked
against a certain address, which has to be reflected in this parameter?

Wouldn't it be feasible to provide a default value, say the beginning
of DRAM? (See below)

> Signed-off-by: Julien Thierry <julien.thierry@arm.com>
> ---
>  arm/fdt.c                                | 14 +++++++-
>  arm/include/arm-common/kvm-config-arch.h |  5 ++-
>  arm/kvm.c                                | 58
> +++++++++++++++++++++++++++++++- 3 files changed, 74 insertions(+), 3
> deletions(-)
> 
> diff --git a/arm/fdt.c b/arm/fdt.c
> index 664bb62..2936986 100644
> --- a/arm/fdt.c
> +++ b/arm/fdt.c
> @@ -131,7 +131,19 @@ static int setup_fdt(struct kvm *kvm)
>  	/* /chosen */
>  	_FDT(fdt_begin_node(fdt, "chosen"));
>  	_FDT(fdt_property_cell(fdt, "linux,pci-probe-only", 1));
> -	_FDT(fdt_property_string(fdt, "bootargs",
> kvm->cfg.real_cmdline)); +
> +	if (kvm->cfg.firmware_filename) {
> +		/*
> +		 * When using a firmware, command line is not passed
> through DT,
> +		 * or the firmware can add it itself
> +		 */
> +		if (kvm->cfg.kernel_cmdline)
> +			pr_warning("Ignoring custom bootargs: %s\n",
> +				   kvm->cfg.kernel_cmdline);
> +	} else
> +		_FDT(fdt_property_string(fdt, "bootargs",
> +					 kvm->cfg.real_cmdline));
> +
>  	_FDT(fdt_property_u64(fdt, "kaslr-seed",
> kvm->cfg.arch.kaslr_seed)); 
>  	/* Initrd */
> diff --git a/arm/include/arm-common/kvm-config-arch.h
> b/arm/include/arm-common/kvm-config-arch.h index 6a196f1..5734c46
> 100644 --- a/arm/include/arm-common/kvm-config-arch.h
> +++ b/arm/include/arm-common/kvm-config-arch.h
> @@ -11,6 +11,7 @@ struct kvm_config_arch {
>  	bool		has_pmuv3;
>  	u64		kaslr_seed;
>  	enum irqchip_type irqchip;
> +	u64		fw_addr;
>  };
>  
>  int irqchip_parser(const struct option *opt, const char *arg, int
> unset); @@ -30,6 +31,8 @@ int irqchip_parser(const struct option
> *opt, const char *arg, int unset); OPT_CALLBACK('\0', "irqchip",
> &(cfg)->irqchip,				\
> "[gicv2|gicv2m|gicv3|gicv3-its]",				\
> "Type of interrupt controller to emulate in the guest",	\
> -		     irqchip_parser, NULL),
> +		     irqchip_parser,
> NULL),					\
> +	OPT_U64('\0', "firmware-address",
> &(cfg)->fw_addr,			\
> +		"Address where firmware should be loaded"),
>  
>  #endif /* ARM_COMMON__KVM_CONFIG_ARCH_H */
> diff --git a/arm/kvm.c b/arm/kvm.c
> index c6843e5..d5bbbc3 100644
> --- a/arm/kvm.c
> +++ b/arm/kvm.c
> @@ -169,9 +169,65 @@ bool kvm__arch_load_kernel_image(struct kvm
> *kvm, int fd_kernel, int fd_initrd, return true;
>  }
>  
> +static bool validate_fw_addr(struct kvm *kvm)
> +{
> +	u64 fw_addr = kvm->cfg.arch.fw_addr;
> +	u64 ram_phys;
> +
> +	ram_phys = host_to_guest_flat(kvm, kvm->ram_start);
> +
> +	if (fw_addr < ram_phys || fw_addr >= ram_phys +
> kvm->ram_size) {
> +		pr_err("Provide --firmware-address an address in
> RAM: "
> +		       "0x%016llx - 0x%016llx",
> +		       ram_phys, ram_phys + kvm->ram_size);
> +
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
>  bool kvm__load_firmware(struct kvm *kvm, const char
> *firmware_filename) {
> -	return false;
> +	u64 fw_addr = kvm->cfg.arch.fw_addr;
> +	void *host_pos;
> +	void *limit;
> +	ssize_t fw_sz;
> +	int fd;
> +
> +	limit = kvm->ram_start + kvm->ram_size;

What about we check here for fw_addr being 0, which is the default
value and would be rejected by the next call.
So can't we set it to some sensible default value here?

	/* If not specified, use default load address at start of RAM */
	if (fw_addr == 0)
		fw_addr = ARM_MEMORY_AREA;

Cheers,
Andre.

> +	if (!validate_fw_addr(kvm))
> +		die("Bad firmware destination: 0x%016llx", fw_addr);
> +
> +	fd = open(firmware_filename, O_RDONLY);
> +	if (fd < 0)
> +		return false;
> +
> +	host_pos = guest_flat_to_host(kvm, fw_addr);
> +	if (!host_pos || host_pos < kvm->ram_start)
> +		return false;
> +
> +	fw_sz = read_file(fd, host_pos, limit - host_pos);
> +	if (fw_sz < 0)
> +		die("failed to load firmware");
> +	close(fd);
> +
> +	/* Kernel isn't loaded by kvm, point start address to
> firmware */
> +	kvm->arch.kern_guest_start = fw_addr;
> +
> +	/* Load dtb just after the firmware image*/
> +	host_pos += fw_sz;
> +	if (host_pos + FDT_MAX_SIZE > limit)
> +		die("not enough space to load fdt");
> +
> +	kvm->arch.dtb_guest_start = ALIGN(host_to_guest_flat(kvm,
> host_pos),
> +					  FDT_ALIGN);
> +	pr_info("Placing fdt at 0x%llx - 0x%llx",
> +		kvm->arch.dtb_guest_start,
> +		kvm->arch.dtb_guest_start + FDT_MAX_SIZE);
> +
> +	return true;
>  }
>  
>  int kvm__arch_setup_firmware(struct kvm *kvm)
Julien Thierry - Dec. 17, 2018, 10:05 a.m.
Hi,

On 14/12/2018 18:08, Andre Przywara wrote:
> On Tue,  4 Dec 2018 11:14:31 +0000
> Julien Thierry <julien.thierry@arm.com> wrote:
> 
> Hi,
> 
>> Implement firmware image loading for arm and set the boot start
>> address to the firmware address.
>>
>> Add an option for the user to specify where to map the firmware.
> 
> How is the user supposed to know this address? Will EDKII be linked
> against a certain address, which has to be reflected in this parameter?
> 

For EDKII I believe any address in RAM is fine (although I'm unsure
whether there are other alignment properties).

For other firmwares/bootloaders, only the user can know depending on
what they are loading.

> Wouldn't it be feasible to provide a default value, say the beginning
> of DRAM? (See below)
> 
>> Signed-off-by: Julien Thierry <julien.thierry@arm.com>
>> ---
>>  arm/fdt.c                                | 14 +++++++-
>>  arm/include/arm-common/kvm-config-arch.h |  5 ++-
>>  arm/kvm.c                                | 58
>> +++++++++++++++++++++++++++++++- 3 files changed, 74 insertions(+), 3
>> deletions(-)
>>
>> diff --git a/arm/fdt.c b/arm/fdt.c
>> index 664bb62..2936986 100644
>> --- a/arm/fdt.c
>> +++ b/arm/fdt.c
>> @@ -131,7 +131,19 @@ static int setup_fdt(struct kvm *kvm)
>>  	/* /chosen */
>>  	_FDT(fdt_begin_node(fdt, "chosen"));
>>  	_FDT(fdt_property_cell(fdt, "linux,pci-probe-only", 1));
>> -	_FDT(fdt_property_string(fdt, "bootargs",
>> kvm->cfg.real_cmdline)); +
>> +	if (kvm->cfg.firmware_filename) {
>> +		/*
>> +		 * When using a firmware, command line is not passed
>> through DT,
>> +		 * or the firmware can add it itself
>> +		 */
>> +		if (kvm->cfg.kernel_cmdline)
>> +			pr_warning("Ignoring custom bootargs: %s\n",
>> +				   kvm->cfg.kernel_cmdline);
>> +	} else
>> +		_FDT(fdt_property_string(fdt, "bootargs",
>> +					 kvm->cfg.real_cmdline));
>> +
>>  	_FDT(fdt_property_u64(fdt, "kaslr-seed",
>> kvm->cfg.arch.kaslr_seed)); 
>>  	/* Initrd */
>> diff --git a/arm/include/arm-common/kvm-config-arch.h
>> b/arm/include/arm-common/kvm-config-arch.h index 6a196f1..5734c46
>> 100644 --- a/arm/include/arm-common/kvm-config-arch.h
>> +++ b/arm/include/arm-common/kvm-config-arch.h
>> @@ -11,6 +11,7 @@ struct kvm_config_arch {
>>  	bool		has_pmuv3;
>>  	u64		kaslr_seed;
>>  	enum irqchip_type irqchip;
>> +	u64		fw_addr;
>>  };
>>  
>>  int irqchip_parser(const struct option *opt, const char *arg, int
>> unset); @@ -30,6 +31,8 @@ int irqchip_parser(const struct option
>> *opt, const char *arg, int unset); OPT_CALLBACK('\0', "irqchip",
>> &(cfg)->irqchip,				\
>> "[gicv2|gicv2m|gicv3|gicv3-its]",				\
>> "Type of interrupt controller to emulate in the guest",	\
>> -		     irqchip_parser, NULL),
>> +		     irqchip_parser,
>> NULL),					\
>> +	OPT_U64('\0', "firmware-address",
>> &(cfg)->fw_addr,			\
>> +		"Address where firmware should be loaded"),
>>  
>>  #endif /* ARM_COMMON__KVM_CONFIG_ARCH_H */
>> diff --git a/arm/kvm.c b/arm/kvm.c
>> index c6843e5..d5bbbc3 100644
>> --- a/arm/kvm.c
>> +++ b/arm/kvm.c
>> @@ -169,9 +169,65 @@ bool kvm__arch_load_kernel_image(struct kvm
>> *kvm, int fd_kernel, int fd_initrd, return true;
>>  }
>>  
>> +static bool validate_fw_addr(struct kvm *kvm)
>> +{
>> +	u64 fw_addr = kvm->cfg.arch.fw_addr;
>> +	u64 ram_phys;
>> +
>> +	ram_phys = host_to_guest_flat(kvm, kvm->ram_start);
>> +
>> +	if (fw_addr < ram_phys || fw_addr >= ram_phys +
>> kvm->ram_size) {
>> +		pr_err("Provide --firmware-address an address in
>> RAM: "
>> +		       "0x%016llx - 0x%016llx",
>> +		       ram_phys, ram_phys + kvm->ram_size);
>> +
>> +		return false;
>> +	}
>> +
>> +	return true;
>> +}
>> +
>>  bool kvm__load_firmware(struct kvm *kvm, const char
>> *firmware_filename) {
>> -	return false;
>> +	u64 fw_addr = kvm->cfg.arch.fw_addr;
>> +	void *host_pos;
>> +	void *limit;
>> +	ssize_t fw_sz;
>> +	int fd;
>> +
>> +	limit = kvm->ram_start + kvm->ram_size;
> 
> What about we check here for fw_addr being 0, which is the default
> value and would be rejected by the next call.
> So can't we set it to some sensible default value here?
> 
> 	/* If not specified, use default load address at start of RAM */
> 	if (fw_addr == 0)
> 		fw_addr = ARM_MEMORY_AREA;
> 

Seems a bit random to me, but I guess we can. After all for the kernel
image we just to put it at the end of DRAM.

Thanks,

Julien

> 
>> +	if (!validate_fw_addr(kvm))
>> +		die("Bad firmware destination: 0x%016llx", fw_addr);
>> +
>> +	fd = open(firmware_filename, O_RDONLY);
>> +	if (fd < 0)
>> +		return false;
>> +
>> +	host_pos = guest_flat_to_host(kvm, fw_addr);
>> +	if (!host_pos || host_pos < kvm->ram_start)
>> +		return false;
>> +
>> +	fw_sz = read_file(fd, host_pos, limit - host_pos);
>> +	if (fw_sz < 0)
>> +		die("failed to load firmware");
>> +	close(fd);
>> +
>> +	/* Kernel isn't loaded by kvm, point start address to
>> firmware */
>> +	kvm->arch.kern_guest_start = fw_addr;
>> +
>> +	/* Load dtb just after the firmware image*/
>> +	host_pos += fw_sz;
>> +	if (host_pos + FDT_MAX_SIZE > limit)
>> +		die("not enough space to load fdt");
>> +
>> +	kvm->arch.dtb_guest_start = ALIGN(host_to_guest_flat(kvm,
>> host_pos),
>> +					  FDT_ALIGN);
>> +	pr_info("Placing fdt at 0x%llx - 0x%llx",
>> +		kvm->arch.dtb_guest_start,
>> +		kvm->arch.dtb_guest_start + FDT_MAX_SIZE);
>> +
>> +	return true;
>>  }
>>  
>>  int kvm__arch_setup_firmware(struct kvm *kvm)
>
Andre Przywara - Dec. 17, 2018, 12:01 p.m.
On 17/12/2018 10:05, Julien Thierry wrote:

Hi Julien,

> On 14/12/2018 18:08, Andre Przywara wrote:
>> On Tue,  4 Dec 2018 11:14:31 +0000
>> Julien Thierry <julien.thierry@arm.com> wrote:
>>
>> Hi,
>>
>>> Implement firmware image loading for arm and set the boot start
>>> address to the firmware address.
>>>
>>> Add an option for the user to specify where to map the firmware.
>>
>> How is the user supposed to know this address? Will EDKII be linked
>> against a certain address, which has to be reflected in this parameter?
>>
> 
> For EDKII I believe any address in RAM is fine (although I'm unsure
> whether there are other alignment properties).

Yes, speaking with Sami on Friday and actually trying it indeed EDKII
does not require a certain load address.

> For other firmwares/bootloaders, only the user can know depending on
> what they are loading.

Sure, but the majority of people will use EDKII, so let's make this
easier for them, without falling back to the old ARM habit of requiring
users to provide magic addresses just for getting something to boot ;-)

Thanks,
Andre.

>> Wouldn't it be feasible to provide a default value, say the beginning
>> of DRAM? (See below)
>>
>>> Signed-off-by: Julien Thierry <julien.thierry@arm.com>
>>> ---
>>>  arm/fdt.c                                | 14 +++++++-
>>>  arm/include/arm-common/kvm-config-arch.h |  5 ++-
>>>  arm/kvm.c                                | 58
>>> +++++++++++++++++++++++++++++++- 3 files changed, 74 insertions(+), 3
>>> deletions(-)
>>>
>>> diff --git a/arm/fdt.c b/arm/fdt.c
>>> index 664bb62..2936986 100644
>>> --- a/arm/fdt.c
>>> +++ b/arm/fdt.c
>>> @@ -131,7 +131,19 @@ static int setup_fdt(struct kvm *kvm)
>>>  	/* /chosen */
>>>  	_FDT(fdt_begin_node(fdt, "chosen"));
>>>  	_FDT(fdt_property_cell(fdt, "linux,pci-probe-only", 1));
>>> -	_FDT(fdt_property_string(fdt, "bootargs",
>>> kvm->cfg.real_cmdline)); +
>>> +	if (kvm->cfg.firmware_filename) {
>>> +		/*
>>> +		 * When using a firmware, command line is not passed
>>> through DT,
>>> +		 * or the firmware can add it itself
>>> +		 */
>>> +		if (kvm->cfg.kernel_cmdline)
>>> +			pr_warning("Ignoring custom bootargs: %s\n",
>>> +				   kvm->cfg.kernel_cmdline);
>>> +	} else
>>> +		_FDT(fdt_property_string(fdt, "bootargs",
>>> +					 kvm->cfg.real_cmdline));
>>> +
>>>  	_FDT(fdt_property_u64(fdt, "kaslr-seed",
>>> kvm->cfg.arch.kaslr_seed)); 
>>>  	/* Initrd */
>>> diff --git a/arm/include/arm-common/kvm-config-arch.h
>>> b/arm/include/arm-common/kvm-config-arch.h index 6a196f1..5734c46
>>> 100644 --- a/arm/include/arm-common/kvm-config-arch.h
>>> +++ b/arm/include/arm-common/kvm-config-arch.h
>>> @@ -11,6 +11,7 @@ struct kvm_config_arch {
>>>  	bool		has_pmuv3;
>>>  	u64		kaslr_seed;
>>>  	enum irqchip_type irqchip;
>>> +	u64		fw_addr;
>>>  };
>>>  
>>>  int irqchip_parser(const struct option *opt, const char *arg, int
>>> unset); @@ -30,6 +31,8 @@ int irqchip_parser(const struct option
>>> *opt, const char *arg, int unset); OPT_CALLBACK('\0', "irqchip",
>>> &(cfg)->irqchip,				\
>>> "[gicv2|gicv2m|gicv3|gicv3-its]",				\
>>> "Type of interrupt controller to emulate in the guest",	\
>>> -		     irqchip_parser, NULL),
>>> +		     irqchip_parser,
>>> NULL),					\
>>> +	OPT_U64('\0', "firmware-address",
>>> &(cfg)->fw_addr,			\
>>> +		"Address where firmware should be loaded"),
>>>  
>>>  #endif /* ARM_COMMON__KVM_CONFIG_ARCH_H */
>>> diff --git a/arm/kvm.c b/arm/kvm.c
>>> index c6843e5..d5bbbc3 100644
>>> --- a/arm/kvm.c
>>> +++ b/arm/kvm.c
>>> @@ -169,9 +169,65 @@ bool kvm__arch_load_kernel_image(struct kvm
>>> *kvm, int fd_kernel, int fd_initrd, return true;
>>>  }
>>>  
>>> +static bool validate_fw_addr(struct kvm *kvm)
>>> +{
>>> +	u64 fw_addr = kvm->cfg.arch.fw_addr;
>>> +	u64 ram_phys;
>>> +
>>> +	ram_phys = host_to_guest_flat(kvm, kvm->ram_start);
>>> +
>>> +	if (fw_addr < ram_phys || fw_addr >= ram_phys +
>>> kvm->ram_size) {
>>> +		pr_err("Provide --firmware-address an address in
>>> RAM: "
>>> +		       "0x%016llx - 0x%016llx",
>>> +		       ram_phys, ram_phys + kvm->ram_size);
>>> +
>>> +		return false;
>>> +	}
>>> +
>>> +	return true;
>>> +}
>>> +
>>>  bool kvm__load_firmware(struct kvm *kvm, const char
>>> *firmware_filename) {
>>> -	return false;
>>> +	u64 fw_addr = kvm->cfg.arch.fw_addr;
>>> +	void *host_pos;
>>> +	void *limit;
>>> +	ssize_t fw_sz;
>>> +	int fd;
>>> +
>>> +	limit = kvm->ram_start + kvm->ram_size;
>>
>> What about we check here for fw_addr being 0, which is the default
>> value and would be rejected by the next call.
>> So can't we set it to some sensible default value here?
>>
>> 	/* If not specified, use default load address at start of RAM */
>> 	if (fw_addr == 0)
>> 		fw_addr = ARM_MEMORY_AREA;
>>
> 
> Seems a bit random to me, but I guess we can. After all for the kernel
> image we just to put it at the end of DRAM.
> 
> Thanks,
> 
> Julien
> 
>>
>>> +	if (!validate_fw_addr(kvm))
>>> +		die("Bad firmware destination: 0x%016llx", fw_addr);
>>> +
>>> +	fd = open(firmware_filename, O_RDONLY);
>>> +	if (fd < 0)
>>> +		return false;
>>> +
>>> +	host_pos = guest_flat_to_host(kvm, fw_addr);
>>> +	if (!host_pos || host_pos < kvm->ram_start)
>>> +		return false;
>>> +
>>> +	fw_sz = read_file(fd, host_pos, limit - host_pos);
>>> +	if (fw_sz < 0)
>>> +		die("failed to load firmware");
>>> +	close(fd);
>>> +
>>> +	/* Kernel isn't loaded by kvm, point start address to
>>> firmware */
>>> +	kvm->arch.kern_guest_start = fw_addr;
>>> +
>>> +	/* Load dtb just after the firmware image*/
>>> +	host_pos += fw_sz;
>>> +	if (host_pos + FDT_MAX_SIZE > limit)
>>> +		die("not enough space to load fdt");
>>> +
>>> +	kvm->arch.dtb_guest_start = ALIGN(host_to_guest_flat(kvm,
>>> host_pos),
>>> +					  FDT_ALIGN);
>>> +	pr_info("Placing fdt at 0x%llx - 0x%llx",
>>> +		kvm->arch.dtb_guest_start,
>>> +		kvm->arch.dtb_guest_start + FDT_MAX_SIZE);
>>> +
>>> +	return true;
>>>  }
>>>  
>>>  int kvm__arch_setup_firmware(struct kvm *kvm)
>>
>

Patch

diff --git a/arm/fdt.c b/arm/fdt.c
index 664bb62..2936986 100644
--- a/arm/fdt.c
+++ b/arm/fdt.c
@@ -131,7 +131,19 @@  static int setup_fdt(struct kvm *kvm)
 	/* /chosen */
 	_FDT(fdt_begin_node(fdt, "chosen"));
 	_FDT(fdt_property_cell(fdt, "linux,pci-probe-only", 1));
-	_FDT(fdt_property_string(fdt, "bootargs", kvm->cfg.real_cmdline));
+
+	if (kvm->cfg.firmware_filename) {
+		/*
+		 * When using a firmware, command line is not passed through DT,
+		 * or the firmware can add it itself
+		 */
+		if (kvm->cfg.kernel_cmdline)
+			pr_warning("Ignoring custom bootargs: %s\n",
+				   kvm->cfg.kernel_cmdline);
+	} else
+		_FDT(fdt_property_string(fdt, "bootargs",
+					 kvm->cfg.real_cmdline));
+
 	_FDT(fdt_property_u64(fdt, "kaslr-seed", kvm->cfg.arch.kaslr_seed));
 
 	/* Initrd */
diff --git a/arm/include/arm-common/kvm-config-arch.h b/arm/include/arm-common/kvm-config-arch.h
index 6a196f1..5734c46 100644
--- a/arm/include/arm-common/kvm-config-arch.h
+++ b/arm/include/arm-common/kvm-config-arch.h
@@ -11,6 +11,7 @@  struct kvm_config_arch {
 	bool		has_pmuv3;
 	u64		kaslr_seed;
 	enum irqchip_type irqchip;
+	u64		fw_addr;
 };
 
 int irqchip_parser(const struct option *opt, const char *arg, int unset);
@@ -30,6 +31,8 @@  int irqchip_parser(const struct option *opt, const char *arg, int unset);
         OPT_CALLBACK('\0', "irqchip", &(cfg)->irqchip,				\
 		     "[gicv2|gicv2m|gicv3|gicv3-its]",				\
 		     "Type of interrupt controller to emulate in the guest",	\
-		     irqchip_parser, NULL),
+		     irqchip_parser, NULL),					\
+	OPT_U64('\0', "firmware-address", &(cfg)->fw_addr,			\
+		"Address where firmware should be loaded"),
 
 #endif /* ARM_COMMON__KVM_CONFIG_ARCH_H */
diff --git a/arm/kvm.c b/arm/kvm.c
index c6843e5..d5bbbc3 100644
--- a/arm/kvm.c
+++ b/arm/kvm.c
@@ -169,9 +169,65 @@  bool kvm__arch_load_kernel_image(struct kvm *kvm, int fd_kernel, int fd_initrd,
 	return true;
 }
 
+static bool validate_fw_addr(struct kvm *kvm)
+{
+	u64 fw_addr = kvm->cfg.arch.fw_addr;
+	u64 ram_phys;
+
+	ram_phys = host_to_guest_flat(kvm, kvm->ram_start);
+
+	if (fw_addr < ram_phys || fw_addr >= ram_phys + kvm->ram_size) {
+		pr_err("Provide --firmware-address an address in RAM: "
+		       "0x%016llx - 0x%016llx",
+		       ram_phys, ram_phys + kvm->ram_size);
+
+		return false;
+	}
+
+	return true;
+}
+
 bool kvm__load_firmware(struct kvm *kvm, const char *firmware_filename)
 {
-	return false;
+	u64 fw_addr = kvm->cfg.arch.fw_addr;
+	void *host_pos;
+	void *limit;
+	ssize_t fw_sz;
+	int fd;
+
+	limit = kvm->ram_start + kvm->ram_size;
+
+	if (!validate_fw_addr(kvm))
+		die("Bad firmware destination: 0x%016llx", fw_addr);
+
+	fd = open(firmware_filename, O_RDONLY);
+	if (fd < 0)
+		return false;
+
+	host_pos = guest_flat_to_host(kvm, fw_addr);
+	if (!host_pos || host_pos < kvm->ram_start)
+		return false;
+
+	fw_sz = read_file(fd, host_pos, limit - host_pos);
+	if (fw_sz < 0)
+		die("failed to load firmware");
+	close(fd);
+
+	/* Kernel isn't loaded by kvm, point start address to firmware */
+	kvm->arch.kern_guest_start = fw_addr;
+
+	/* Load dtb just after the firmware image*/
+	host_pos += fw_sz;
+	if (host_pos + FDT_MAX_SIZE > limit)
+		die("not enough space to load fdt");
+
+	kvm->arch.dtb_guest_start = ALIGN(host_to_guest_flat(kvm, host_pos),
+					  FDT_ALIGN);
+	pr_info("Placing fdt at 0x%llx - 0x%llx",
+		kvm->arch.dtb_guest_start,
+		kvm->arch.dtb_guest_start + FDT_MAX_SIZE);
+
+	return true;
 }
 
 int kvm__arch_setup_firmware(struct kvm *kvm)