[Qemu-devel] [PATCH] hw/mips: malta: Don't boot from flash with KVM T&E

[Qemu-devel] [PATCH] hw/mips: malta: Don't boot from flash with KVM T&E

[James Hogan]

In KVM trap & emulate (T&E) mode the flash reset region at 0xbfc00000
isn't executable, which is why the minimal kernel bootloader is loaded
and executed from the last 1MB of DRAM instead.

Therefore if no kernel is provided on the command line and KVM is
enabled, exit with an error since booting from flash will fail.

Reported-by: Aurelien Jarno <aurelien@aurel32.net>
Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/mips/mips_malta.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 8bc5392b4223..91b0ce566111 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1052,6 +1052,12 @@ void mips_malta_init(MachineState *machine)
                              bootloader_run_addr, kernel_entry);
         }
     } else {
+        /* The flash region isn't executable from a KVM T&E guest */
+        if (kvm_enabled()) {
+            error_report("KVM enabled but no -kernel argument was specified. "
+                         "Booting from flash is not supported with KVM T&E.");
+            exit(1);
+        }
         /* Load firmware from flash. */
         if (!dinfo) {
             /* Load a BIOS image. */
-- 
1.9.3

Re: [Qemu-devel] [PATCH] hw/mips: malta: Don't boot from flash with KVM T&E

[Andreas Färber]

Hi,

Am 20.06.2014 13:47, schrieb James Hogan:
> In KVM trap & emulate (T&E) mode the flash reset region at 0xbfc00000
> isn't executable, which is why the minimal kernel bootloader is loaded
> and executed from the last 1MB of DRAM instead.
> 
> Therefore if no kernel is provided on the command line and KVM is
> enabled, exit with an error since booting from flash will fail.
> 
> Reported-by: Aurelien Jarno <aurelien@aurel32.net>
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/mips/mips_malta.c | 6 ++++++
>  1 file changed, 6 insertions(+)

Previous commits have used just "mips_malta:", so suggest to do the same
for consistency - or clean hw/mips/ dir up by renaming the file to just
malta.c and use "mips/malta:".

http://git.qemu-project.org/?p=qemu.git;a=history;f=hw/mips/mips_malta.c;h=f4a7d47129526f6762eccc47251c8cf3cd27f928;hb=HEAD

That said,

> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index 8bc5392b4223..91b0ce566111 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -1052,6 +1052,12 @@ void mips_malta_init(MachineState *machine)
>                               bootloader_run_addr, kernel_entry);
>          }
>      } else {
> +        /* The flash region isn't executable from a KVM T&E guest */
> +        if (kvm_enabled()) {
> +            error_report("KVM enabled but no -kernel argument was specified. "
> +                         "Booting from flash is not supported with KVM T&E.");
> +            exit(1);
> +        }
>          /* Load firmware from flash. */
>          if (!dinfo) {
>              /* Load a BIOS image. */

Reviewed-by: Andreas Färber <afaerber@suse.de>

Are users expected to know/understand "KVM T&E"? You don't test for that
T&E mode here, so you could either shorten the message to just "KVM" or
replace the check with a more specific one.

Since qtest does not use KVM on MIPS today, exiting without -kernel
should be fine, but may need updating if we ever get to that.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] hw/mips: malta: Don't boot from flash with KVM T&E

[James Hogan]

On 20/06/14 14:17, Andreas Färber wrote:
> Hi,
> 
> Am 20.06.2014 13:47, schrieb James Hogan:
>> In KVM trap & emulate (T&E) mode the flash reset region at 0xbfc00000
>> isn't executable, which is why the minimal kernel bootloader is loaded
>> and executed from the last 1MB of DRAM instead.
>>
>> Therefore if no kernel is provided on the command line and KVM is
>> enabled, exit with an error since booting from flash will fail.
>>
>> Reported-by: Aurelien Jarno <aurelien@aurel32.net>
>> Signed-off-by: James Hogan <james.hogan@imgtec.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  hw/mips/mips_malta.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
> 
> Previous commits have used just "mips_malta:", so suggest to do the same
> for consistency - or clean hw/mips/ dir up by renaming the file to just
> malta.c and use "mips/malta:".
> 
> http://git.qemu-project.org/?p=qemu.git;a=history;f=hw/mips/mips_malta.c;h=f4a7d47129526f6762eccc47251c8cf3cd27f928;hb=HEAD

Noted

> 
> That said,
> 
>> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
>> index 8bc5392b4223..91b0ce566111 100644
>> --- a/hw/mips/mips_malta.c
>> +++ b/hw/mips/mips_malta.c
>> @@ -1052,6 +1052,12 @@ void mips_malta_init(MachineState *machine)
>>                               bootloader_run_addr, kernel_entry);
>>          }
>>      } else {
>> +        /* The flash region isn't executable from a KVM T&E guest */
>> +        if (kvm_enabled()) {
>> +            error_report("KVM enabled but no -kernel argument was specified. "
>> +                         "Booting from flash is not supported with KVM T&E.");
>> +            exit(1);
>> +        }
>>          /* Load firmware from flash. */
>>          if (!dinfo) {
>>              /* Load a BIOS image. */
> 
> Reviewed-by: Andreas Färber <afaerber@suse.de>
> 
> Are users expected to know/understand "KVM T&E"? You don't test for that
> T&E mode here, so you could either shorten the message to just "KVM" or
> replace the check with a more specific one.

Yeh, not mentioning T&E at all would be better actually. The limitation
is technically a MIPS KVM one rather than being T&E specific.

I'll post a v2.

Thanks for reviewing
James

> Since qtest does not use KVM on MIPS today, exiting without -kernel
> should be fine, but may need updating if we ever get to that.
> 
> Regards,
> Andreas
>