qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Aurelien Jarno <aurelien@aurel32.net>
To: James Hogan <james.hogan@imgtec.com>
Cc: qemu-devel@nongnu.org, Leon Alrae <leon.alrae@imgtec.com>,
	Yongbok Kim <yongbok.kim@imgtec.com>
Subject: Re: [Qemu-devel] [PATCH] target-mips: Fix RDHWR exception host PC
Date: Thu, 28 Apr 2016 10:51:28 +0200	[thread overview]
Message-ID: <20160428085128.GC13339@aurel32.net> (raw)
In-Reply-To: <1461795666-4704-1-git-send-email-james.hogan@imgtec.com>

On 2016-04-27 23:21, James Hogan wrote:
> Commit b00c72180c36 ("target-mips: add PC, XNP reg numbers to RDHWR")
> changed the rdhwr helpers to use check_hwrena() to check the register
> being accessed is enabled in CP0_HWREna when used from user mode. If
> that check fails an EXCP_RI exception is raised at the host PC
> calculated with GETPC().
> 
> However check_hwrena() may not be fully inlined as the
> do_raise_exception() part of it is common regardless of the arguments.
> This causes GETPC() to calculate the address in the call in the helper
> instead of the generated code calling the helper. No TB will be found
> and the EPC reported with the resulting guest RI exception points to the
> beginning of the TB instead of the RDHWR instruction.
> 
> We can't reliably force check_hwrena() to be inlined, and converting it
> to a macro would be ugly, so instead pass the host PC in as an argument,
> with each rdhwr helper passing GETPC(). This should avoid any dependence
> on compiler behaviour, and in practice seems to prevent the partial
> inlining of check_hwrena() on x86_64.
> 
> This issue causes failures when running a MIPS KVM (trap & emulate)
> guest in a MIPS QEMU TCG guest, as the inner guest kernel will do a
> RDHWR of counter, which is disabled in the outer guest's CP0_HWREna by
> KVM so it can emulate the inner guest's counter. The emulation fails and
> the RI exception is passed to the inner guest.
> 
> Fixes: b00c72180c36 ("target-mips: add PC, XNP reg numbers to RDHWR")
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Leon Alrae <leon.alrae@imgtec.com>
> Cc: Yongbok Kim <yongbok.kim@imgtec.com>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  target-mips/op_helper.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)

Thanks for the detailed analysis. The other solution would have been to
declare the function as __attribute__((__always_inline__)), but I think
your solution is even better.

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

  reply	other threads:[~2016-04-28  8:51 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-27 22:21 [Qemu-devel] [PATCH] target-mips: Fix RDHWR exception host PC James Hogan
2016-04-28  8:51 ` Aurelien Jarno [this message]
2016-04-28  8:55   ` James Hogan
2016-04-28  8:53 ` Leon Alrae
2016-04-28  9:01   ` 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=20160428085128.GC13339@aurel32.net \
    --to=aurelien@aurel32.net \
    --cc=james.hogan@imgtec.com \
    --cc=leon.alrae@imgtec.com \
    --cc=qemu-devel@nongnu.org \
    --cc=yongbok.kim@imgtec.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).