From: David Gibson <david@gibson.dropbear.id.au>
To: Greg Kurz <groug@kaod.org>
Cc: qemu-ppc@nongnu.org, "Cédric Le Goater" <clg@kaod.org>,
qemu-devel@nongnu.org
Subject: Re: [PATCH 6/6] target/ppc: Don't update radix PTE R/C bits with gdbstub
Date: Mon, 11 May 2020 11:43:48 +1000 [thread overview]
Message-ID: <20200511014348.GN2183@umbus.fritz.box> (raw)
In-Reply-To: <158887243487.1564424.7276382177976503972.stgit@bahia.lan>
[-- Attachment #1: Type: text/plain, Size: 6832 bytes --]
On Thu, May 07, 2020 at 07:27:15PM +0200, Greg Kurz wrote:
> gdbstub shouldn't silently change guest visible state when doing address
> translation. While here drop a not very useful comment.
>
> This was found while reading the code. I could verify that this affects
> both powernv and pseries, but I failed to observe any actual bug.
>
> Fixes: d04ea940c597 "target/ppc: Add support for Radix partition-scoped translation"
> Signed-off-by: Greg Kurz <groug@kaod.org>
It's a real fix. But AFAICT we'll always have cause_excp ==
cause_rc_update, and I can't see any reason we'd ever them different.
So I'd prefer to just rename the flag and use it for both tests.
Maybe just 'guest_visible' ?
> ---
> target/ppc/mmu-radix64.c | 36 ++++++++++++++++++++++++------------
> 1 file changed, 24 insertions(+), 12 deletions(-)
>
> diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
> index ceeb3dfe2d49..bc51cd89a079 100644
> --- a/target/ppc/mmu-radix64.c
> +++ b/target/ppc/mmu-radix64.c
> @@ -270,7 +270,8 @@ static int ppc_radix64_partition_scoped_xlate(PowerPCCPU *cpu, int rwx,
> ppc_v3_pate_t pate,
> hwaddr *h_raddr, int *h_prot,
> int *h_page_size, bool pde_addr,
> - bool cause_excp)
> + bool cause_excp,
> + bool cause_rc_update)
> {
> int fault_cause = 0;
> hwaddr pte_addr;
> @@ -291,8 +292,9 @@ static int ppc_radix64_partition_scoped_xlate(PowerPCCPU *cpu, int rwx,
> return 1;
> }
>
> - /* Update Reference and Change Bits */
> - ppc_radix64_set_rc(cpu, rwx, pte, pte_addr, h_prot);
> + if (cause_rc_update) {
> + ppc_radix64_set_rc(cpu, rwx, pte, pte_addr, h_prot);
> + }
>
> return 0;
> }
> @@ -301,7 +303,8 @@ static int ppc_radix64_process_scoped_xlate(PowerPCCPU *cpu, int rwx,
> vaddr eaddr, uint64_t pid,
> ppc_v3_pate_t pate, hwaddr *g_raddr,
> int *g_prot, int *g_page_size,
> - bool cause_excp)
> + bool cause_excp,
> + bool cause_rc_update)
> {
> CPUState *cs = CPU(cpu);
> CPUPPCState *env = &cpu->env;
> @@ -336,7 +339,8 @@ static int ppc_radix64_process_scoped_xlate(PowerPCCPU *cpu, int rwx,
> ret = ppc_radix64_partition_scoped_xlate(cpu, 0, eaddr, prtbe_addr,
> pate, &h_raddr, &h_prot,
> &h_page_size, true,
> - cause_excp);
> + cause_excp,
> + cause_rc_update);
> if (ret) {
> return ret;
> }
> @@ -376,7 +380,8 @@ static int ppc_radix64_process_scoped_xlate(PowerPCCPU *cpu, int rwx,
> ret = ppc_radix64_partition_scoped_xlate(cpu, 0, eaddr, pte_addr,
> pate, &h_raddr, &h_prot,
> &h_page_size, true,
> - cause_excp);
> + cause_excp,
> + cause_rc_update);
> if (ret) {
> return ret;
> }
> @@ -408,7 +413,9 @@ static int ppc_radix64_process_scoped_xlate(PowerPCCPU *cpu, int rwx,
> return 1;
> }
>
> - ppc_radix64_set_rc(cpu, rwx, pte, pte_addr, g_prot);
> + if (cause_rc_update) {
> + ppc_radix64_set_rc(cpu, rwx, pte, pte_addr, g_prot);
> + }
>
> return 0;
> }
> @@ -433,7 +440,8 @@ static int ppc_radix64_process_scoped_xlate(PowerPCCPU *cpu, int rwx,
> static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr, int rwx,
> bool relocation,
> hwaddr *raddr, int *psizep, int *protp,
> - bool cause_excp)
> + bool cause_excp,
> + bool cause_rc_update)
> {
> CPUPPCState *env = &cpu->env;
> uint64_t lpid, pid;
> @@ -483,7 +491,9 @@ static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr, int rwx,
> if (relocation) {
> int ret = ppc_radix64_process_scoped_xlate(cpu, rwx, eaddr, pid,
> pate, &g_raddr, &prot,
> - &psize, cause_excp);
> + &psize,
> + cause_excp,
> + cause_rc_update);
> if (ret) {
> return ret;
> }
> @@ -506,7 +516,9 @@ static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr, int rwx,
>
> ret = ppc_radix64_partition_scoped_xlate(cpu, rwx, eaddr, g_raddr,
> pate, raddr, &prot, &psize,
> - 0, cause_excp);
> + 0,
> + cause_excp,
> + cause_rc_update);
> if (ret) {
> return ret;
> }
> @@ -562,7 +574,7 @@ int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
>
> /* Translate eaddr to raddr (where raddr is addr qemu needs for access) */
> if (ppc_radix64_xlate(cpu, eaddr, rwx, relocation, &raddr,
> - &page_size, &prot, true)) {
> + &page_size, &prot, true, true)) {
> return 1;
> }
>
> @@ -584,7 +596,7 @@ hwaddr ppc_radix64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong eaddr)
> }
>
> if (ppc_radix64_xlate(cpu, eaddr, 0, msr_dr, &raddr, &psize,
> - &prot, false)) {
> + &prot, false, false)) {
> return -1;
> }
>
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2020-05-11 1:50 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-07 17:26 [PATCH 0/6] target/ppc: Various clean-up and fixes for radix64 Greg Kurz
2020-05-07 17:26 ` [PATCH 1/6] target/ppc: Pass const pointer to ppc_radix64_get_prot_amr() Greg Kurz
2020-05-07 17:26 ` [PATCH 2/6] target/ppc: Pass const pointer to ppc_radix64_get_fully_qualified_addr() Greg Kurz
2020-05-07 17:26 ` [PATCH 3/6] target/ppc: Don't initialize some local variables in ppc_radix64_xlate() Greg Kurz
2020-05-11 9:07 ` Cédric Le Goater
2020-05-11 10:12 ` Greg Kurz
2020-05-07 17:27 ` [PATCH 4/6] target/ppc: Add missing braces in ppc_radix64_partition_scoped_xlate() Greg Kurz
2020-05-07 17:27 ` [PATCH 5/6] target/ppc: Fix arguments to ppc_radix64_partition_scoped_xlate() Greg Kurz
2020-05-07 17:27 ` [PATCH 6/6] target/ppc: Don't update radix PTE R/C bits with gdbstub Greg Kurz
2020-05-11 1:43 ` David Gibson [this message]
2020-05-11 9:30 ` Greg Kurz
2020-05-11 1:44 ` [PATCH 0/6] target/ppc: Various clean-up and fixes for radix64 David Gibson
2020-05-11 16:55 ` Greg Kurz
2020-05-12 0:00 ` David Gibson
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=20200511014348.GN2183@umbus.fritz.box \
--to=david@gibson.dropbear.id.au \
--cc=clg@kaod.org \
--cc=groug@kaod.org \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@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 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).