All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Andreas Färber" <afaerber@suse.de>
To: James Hogan <james.hogan@imgtec.com>, qemu-devel@nongnu.org
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Aurelien Jarno <aurelien@aurel32.net>
Subject: Re: [Qemu-devel] [PATCH] hw/mips: malta: Don't boot from flash with KVM T&E
Date: Fri, 20 Jun 2014 15:17:42 +0200	[thread overview]
Message-ID: <53A43476.7060901@suse.de> (raw)
In-Reply-To: <1403264879-13247-1-git-send-email-james.hogan@imgtec.com>

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

  reply	other threads:[~2014-06-20 13:17 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-20 11:47 [Qemu-devel] [PATCH] hw/mips: malta: Don't boot from flash with KVM T&E James Hogan
2014-06-20 13:17 ` Andreas Färber [this message]
2014-06-20 13:35   ` James Hogan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53A43476.7060901@suse.de \
    --to=afaerber@suse.de \
    --cc=aurelien@aurel32.net \
    --cc=james.hogan@imgtec.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.