All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Andreas Färber" <afaerber@suse.de>
To: "Hervé Poussineau" <hpoussin@reactos.org>,
	"Peter Crosthwaite" <peter.crosthwaite@xilinx.com>,
	"Edgar E. Iglesias" <edgar.iglesias@gmail.com>
Cc: Stefan Weil <sw@weilnetz.de>,
	qemu-devel@nongnu.org,
	Peter Jovanovic <petar.jovanovic@imgtec.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Leon Alrae <leon.alrae@imgtec.com>,
	Aurelien Jarno <aurelien@aurel32.net>
Subject: Re: [Qemu-devel] [PATCH for-1.6] target-mips: do not raise exceptions when accessing invalid memory
Date: Sat, 27 Jul 2013 18:49:18 +0200	[thread overview]
Message-ID: <51F3FA0E.2050300@suse.de> (raw)
In-Reply-To: <1374941897-11956-1-git-send-email-hpoussin@reactos.org>

Am 27.07.2013 18:18, schrieb Hervé Poussineau:
> c658b94f6e8c206c59d02aa6fbac285b86b53d2c ("cpu: Turn cpu_unassigned_access()
> into a CPUState hook") made MIPS raise exceptions when accessing
> invalid memory for data, by unconditionally calling CPUState unassigned hook.
> 
> While this seems to be the right behaviour, this breaks a lot of guests
> (Linux on Malta, NetBSD on Magnum...) which try to access not emulated devices
> and crash because they don't handle the data load/store exception.
> 
> Revert to previous behaviour by not handling the !is_exec case in MIPS CPU hook.
> 
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>

So before my refactoring the following targets called it in files...

alpha: cputlb.c and memory.c
microblaze: memory.c
mips: cputlb.c
sparc: cputlb.c and memory.c

... and now all four call it in both places, breaking mips.

The proposed solution looks acceptable to me, but I am no mips expert;
CC'ing Aurélien, Stefan and some Imagination guys.

As a reminder, 1.6-rc0 is due on Monday.

> ---
> 
> Another solution would be to add a big dummy memory regions on all MIPS boards
> to catch memory accesses and not raise an exception. However, this means that
> each MIPS board will have its own unassigned memory handler, different from the
> global QEMU one.

sparc uses the empty_slot device to catch accesses to devices that we
are not yet emulating IIUC. I.e., empty_slot_init(addr, size).

Peter/Edgar, can you double-check whether calling the unassigned_access
handler from cputlb.c rather than cpu_abort()ing is OK for microblaze?

Thanks,
Andreas

> ---
>  target-mips/op_helper.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
> index 5cf1c3f..94f1692 100644
> --- a/target-mips/op_helper.c
> +++ b/target-mips/op_helper.c
> @@ -2156,7 +2156,8 @@ void mips_cpu_unassigned_access(CPUState *cs, hwaddr addr,
>      if (is_exec) {
>          helper_raise_exception(env, EXCP_IBE);
>      } else {
> -        helper_raise_exception(env, EXCP_DBE);
> +        qemu_log_mask(LOG_UNIMP, "should raise DBE exception "
> +                      "due to accessing memory at %" HWADDR_PRIx "\n", addr);
>      }
>  }
>  #endif /* !CONFIG_USER_ONLY */
> 


-- 
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:[~2013-07-27 17:02 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-27 16:18 [Qemu-devel] [PATCH for-1.6] target-mips: do not raise exceptions when accessing invalid memory Hervé Poussineau
2013-07-27 16:49 ` Andreas Färber [this message]
2013-07-27 17:43 ` Peter Maydell
2013-07-27 19:37   ` Stefan Weil
2013-07-27 20:43     ` Andreas Färber
2013-07-27 20:57       ` Peter Maydell
2013-07-27 20:58       ` Stefan Weil
2013-07-29 20:35         ` Stefan Weil
2013-08-04 22:04           ` Aurélien Jarno
2013-08-04 22:37             ` Peter Maydell
2013-08-05  5:19               ` Stefan Weil
2013-08-05 12:27                 ` Aurélien Jarno
2013-08-05  8:45             ` Andreas Färber
2013-08-05  8:47               ` Jan Kiszka
2013-08-05 13:31               ` Aurélien Jarno
2013-08-05 13:45                 ` Andreas Färber
2013-08-05 13:53                   ` Hervé Poussineau
2013-08-05 14:07                     ` Aurélien Jarno
2013-08-05 14:15                       ` Peter Maydell

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=51F3FA0E.2050300@suse.de \
    --to=afaerber@suse.de \
    --cc=aurelien@aurel32.net \
    --cc=edgar.iglesias@gmail.com \
    --cc=hpoussin@reactos.org \
    --cc=leon.alrae@imgtec.com \
    --cc=pbonzini@redhat.com \
    --cc=petar.jovanovic@imgtec.com \
    --cc=peter.crosthwaite@xilinx.com \
    --cc=qemu-devel@nongnu.org \
    --cc=sw@weilnetz.de \
    /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.