Changed hash32 address translation to use the supplied mmu_idx, instead
of using what was stored in the msr, for parity purposes (radix64
already uses that).
Signed-off-by: Bruno Larsen (billionai) <bruno.larsen@eldorado.org.br>
---
target/ppc/mmu-hash32.c | 40 +++++++++++++++++++---------------------
target/ppc/mmu-hash32.h | 2 +-
target/ppc/mmu_helper.c | 2 +-
3 files changed, 21 insertions(+), 23 deletions(-)
diff --git a/target/ppc/mmu-hash32.c b/target/ppc/mmu-hash32.c
index 6a07c345e4..0691d553a3 100644
--- a/target/ppc/mmu-hash32.c
+++ b/target/ppc/mmu-hash32.c
@@ -25,6 +25,7 @@
#include "kvm_ppc.h"
#include "internal.h"
#include "mmu-hash32.h"
+#include "mmu-book3s-v3.h"
#include "exec/log.h"
/* #define DEBUG_BAT */
@@ -86,25 +87,22 @@ static int ppc_hash32_pp_prot(int key, int pp, int nx)
return prot;
}
-static int ppc_hash32_pte_prot(PowerPCCPU *cpu,
+static int ppc_hash32_pte_prot(int mmu_idx,
target_ulong sr, ppc_hash_pte32_t pte)
{
- CPUPPCState *env = &cpu->env;
unsigned pp, key;
- key = !!(msr_pr ? (sr & SR32_KP) : (sr & SR32_KS));
+ key = !!(mmuidx_pr(mmu_idx) ? (sr & SR32_KP) : (sr & SR32_KS));
pp = pte.pte1 & HPTE32_R_PP;
return ppc_hash32_pp_prot(key, pp, !!(sr & SR32_NX));
}
-static target_ulong hash32_bat_size(PowerPCCPU *cpu,
+static target_ulong hash32_bat_size(int mmu_idx,
target_ulong batu, target_ulong batl)
{
- CPUPPCState *env = &cpu->env;
-
- if ((msr_pr && !(batu & BATU32_VP))
- || (!msr_pr && !(batu & BATU32_VS))) {
+ if ((mmuidx_pr(mmu_idx) && !(batu & BATU32_VP))
+ || (!mmuidx_pr(mmu_idx) && !(batu & BATU32_VS))) {
return 0;
}
@@ -137,14 +135,13 @@ static target_ulong hash32_bat_601_size(PowerPCCPU *cpu,
return BATU32_BEPI & ~((batl & BATL32_601_BL) << 17);
}
-static int hash32_bat_601_prot(PowerPCCPU *cpu,
+static int hash32_bat_601_prot(int mmu_idx,
target_ulong batu, target_ulong batl)
{
- CPUPPCState *env = &cpu->env;
int key, pp;
pp = batu & BATU32_601_PP;
- if (msr_pr == 0) {
+ if (mmuidx_pr(mmu_idx) == 0) {
key = !!(batu & BATU32_601_KS);
} else {
key = !!(batu & BATU32_601_KP);
@@ -153,7 +150,8 @@ static int hash32_bat_601_prot(PowerPCCPU *cpu,
}
static hwaddr ppc_hash32_bat_lookup(PowerPCCPU *cpu, target_ulong ea,
- MMUAccessType access_type, int *prot)
+ MMUAccessType access_type, int *prot,
+ int mmu_idx)
{
CPUPPCState *env = &cpu->env;
target_ulong *BATlt, *BATut;
@@ -177,7 +175,7 @@ static hwaddr ppc_hash32_bat_lookup(PowerPCCPU *cpu, target_ulong ea,
if (unlikely(env->mmu_model == POWERPC_MMU_601)) {
mask = hash32_bat_601_size(cpu, batu, batl);
} else {
- mask = hash32_bat_size(cpu, batu, batl);
+ mask = hash32_bat_size(mmu_idx, batu, batl);
}
LOG_BATS("%s: %cBAT%d v " TARGET_FMT_lx " BATu " TARGET_FMT_lx
" BATl " TARGET_FMT_lx "\n", __func__,
@@ -187,7 +185,7 @@ static hwaddr ppc_hash32_bat_lookup(PowerPCCPU *cpu, target_ulong ea,
hwaddr raddr = (batl & mask) | (ea & ~mask);
if (unlikely(env->mmu_model == POWERPC_MMU_601)) {
- *prot = hash32_bat_601_prot(cpu, batu, batl);
+ *prot = hash32_bat_601_prot(mmu_idx, batu, batl);
} else {
*prot = hash32_bat_prot(cpu, batu, batl);
}
@@ -221,12 +219,12 @@ static hwaddr ppc_hash32_bat_lookup(PowerPCCPU *cpu, target_ulong ea,
static bool ppc_hash32_direct_store(PowerPCCPU *cpu, target_ulong sr,
target_ulong eaddr,
MMUAccessType access_type,
- hwaddr *raddr, int *prot,
+ hwaddr *raddr, int *prot, int mmu_idx,
bool guest_visible)
{
CPUState *cs = CPU(cpu);
CPUPPCState *env = &cpu->env;
- int key = !!(msr_pr ? (sr & SR32_KP) : (sr & SR32_KS));
+ int key = !!(mmuidx_pr(mmu_idx) ? (sr & SR32_KP) : (sr & SR32_KS));
qemu_log_mask(CPU_LOG_MMU, "direct store...\n");
@@ -425,7 +423,7 @@ static hwaddr ppc_hash32_pte_raddr(target_ulong sr, ppc_hash_pte32_t pte,
}
bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
- hwaddr *raddrp, int *psizep, int *protp,
+ hwaddr *raddrp, int *psizep, int *protp, int mmu_idx,
bool guest_visible)
{
CPUState *cs = CPU(cpu);
@@ -441,7 +439,7 @@ bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
*psizep = TARGET_PAGE_BITS;
/* 1. Handle real mode accesses */
- if (access_type == MMU_INST_FETCH ? !msr_ir : !msr_dr) {
+ if (mmuidx_real(mmu_idx)) {
/* Translation is off */
*raddrp = eaddr;
*protp = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
@@ -452,7 +450,7 @@ bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
/* 2. Check Block Address Translation entries (BATs) */
if (env->nb_BATs != 0) {
- raddr = ppc_hash32_bat_lookup(cpu, eaddr, access_type, protp);
+ raddr = ppc_hash32_bat_lookup(cpu, eaddr, access_type, protp, mmu_idx);
if (raddr != -1) {
if (need_prot & ~*protp) {
if (guest_visible) {
@@ -483,7 +481,7 @@ bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
/* 4. Handle direct store segments */
if (sr & SR32_T) {
return ppc_hash32_direct_store(cpu, sr, eaddr, access_type,
- raddrp, protp, guest_visible);
+ raddrp, protp, mmu_idx, guest_visible);
}
/* 5. Check for segment level no-execute violation */
@@ -520,7 +518,7 @@ bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
/* 7. Check access permissions */
- prot = ppc_hash32_pte_prot(cpu, sr, pte);
+ prot = ppc_hash32_pte_prot(mmu_idx, sr, pte);
if (need_prot & ~prot) {
/* Access right violation */
diff --git a/target/ppc/mmu-hash32.h b/target/ppc/mmu-hash32.h
index 8694eccabd..807d9bc6e8 100644
--- a/target/ppc/mmu-hash32.h
+++ b/target/ppc/mmu-hash32.h
@@ -5,7 +5,7 @@
hwaddr get_pteg_offset32(PowerPCCPU *cpu, hwaddr hash);
bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
- hwaddr *raddrp, int *psizep, int *protp,
+ hwaddr *raddrp, int *psizep, int *protp, int mmu_idx,
bool guest_visible);
/*
diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index 9dcdf88597..a3381e1aa0 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -2922,7 +2922,7 @@ static bool ppc_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
case POWERPC_MMU_32B:
case POWERPC_MMU_601:
return ppc_hash32_xlate(cpu, eaddr, access_type,
- raddrp, psizep, protp, guest_visible);
+ raddrp, psizep, protp, mmu_idx, guest_visible);
default:
return ppc_jumbo_xlate(cpu, eaddr, access_type, raddrp,
--
2.17.1
On Mon, Jun 28, 2021 at 10:36:09AM -0300, Bruno Larsen (billionai) wrote:
> Changed hash32 address translation to use the supplied mmu_idx, instead
> of using what was stored in the msr, for parity purposes (radix64
> already uses that).
Well.. parity and conceptual correctness. The translation is supposed
to use mmu_idx, not look at the CPU again to get the right context.
AFAIK there isn't a situation in hash32 where they'll get out of sync,
but nothing guarantees that.
>
> Signed-off-by: Bruno Larsen (billionai) <bruno.larsen@eldorado.org.br>
> ---
> target/ppc/mmu-hash32.c | 40 +++++++++++++++++++---------------------
> target/ppc/mmu-hash32.h | 2 +-
> target/ppc/mmu_helper.c | 2 +-
> 3 files changed, 21 insertions(+), 23 deletions(-)
>
> diff --git a/target/ppc/mmu-hash32.c b/target/ppc/mmu-hash32.c
> index 6a07c345e4..0691d553a3 100644
> --- a/target/ppc/mmu-hash32.c
> +++ b/target/ppc/mmu-hash32.c
> @@ -25,6 +25,7 @@
> #include "kvm_ppc.h"
> #include "internal.h"
> #include "mmu-hash32.h"
> +#include "mmu-book3s-v3.h"
So, the mmu_idx values happen to have the same values for hash32 as
for book3sv3, but that's arguably a coincidence, and including a
book3sv3 header in hash32 code looks really dubious.
I think the right approach is to duplicate the helper macros in
mmu-hash32.h for now. We can unify them later with a more thorough
review (which would probably involve creating a new header for things
common to all BookS family MMUs).
Apart from those nits,
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> #include "exec/log.h"
>
> /* #define DEBUG_BAT */
> @@ -86,25 +87,22 @@ static int ppc_hash32_pp_prot(int key, int pp, int nx)
> return prot;
> }
>
> -static int ppc_hash32_pte_prot(PowerPCCPU *cpu,
> +static int ppc_hash32_pte_prot(int mmu_idx,
> target_ulong sr, ppc_hash_pte32_t pte)
> {
> - CPUPPCState *env = &cpu->env;
> unsigned pp, key;
>
> - key = !!(msr_pr ? (sr & SR32_KP) : (sr & SR32_KS));
> + key = !!(mmuidx_pr(mmu_idx) ? (sr & SR32_KP) : (sr & SR32_KS));
> pp = pte.pte1 & HPTE32_R_PP;
>
> return ppc_hash32_pp_prot(key, pp, !!(sr & SR32_NX));
> }
>
> -static target_ulong hash32_bat_size(PowerPCCPU *cpu,
> +static target_ulong hash32_bat_size(int mmu_idx,
> target_ulong batu, target_ulong batl)
> {
> - CPUPPCState *env = &cpu->env;
> -
> - if ((msr_pr && !(batu & BATU32_VP))
> - || (!msr_pr && !(batu & BATU32_VS))) {
> + if ((mmuidx_pr(mmu_idx) && !(batu & BATU32_VP))
> + || (!mmuidx_pr(mmu_idx) && !(batu & BATU32_VS))) {
> return 0;
> }
>
> @@ -137,14 +135,13 @@ static target_ulong hash32_bat_601_size(PowerPCCPU *cpu,
> return BATU32_BEPI & ~((batl & BATL32_601_BL) << 17);
> }
>
> -static int hash32_bat_601_prot(PowerPCCPU *cpu,
> +static int hash32_bat_601_prot(int mmu_idx,
> target_ulong batu, target_ulong batl)
> {
> - CPUPPCState *env = &cpu->env;
> int key, pp;
>
> pp = batu & BATU32_601_PP;
> - if (msr_pr == 0) {
> + if (mmuidx_pr(mmu_idx) == 0) {
> key = !!(batu & BATU32_601_KS);
> } else {
> key = !!(batu & BATU32_601_KP);
> @@ -153,7 +150,8 @@ static int hash32_bat_601_prot(PowerPCCPU *cpu,
> }
>
> static hwaddr ppc_hash32_bat_lookup(PowerPCCPU *cpu, target_ulong ea,
> - MMUAccessType access_type, int *prot)
> + MMUAccessType access_type, int *prot,
> + int mmu_idx)
> {
> CPUPPCState *env = &cpu->env;
> target_ulong *BATlt, *BATut;
> @@ -177,7 +175,7 @@ static hwaddr ppc_hash32_bat_lookup(PowerPCCPU *cpu, target_ulong ea,
> if (unlikely(env->mmu_model == POWERPC_MMU_601)) {
> mask = hash32_bat_601_size(cpu, batu, batl);
> } else {
> - mask = hash32_bat_size(cpu, batu, batl);
> + mask = hash32_bat_size(mmu_idx, batu, batl);
> }
> LOG_BATS("%s: %cBAT%d v " TARGET_FMT_lx " BATu " TARGET_FMT_lx
> " BATl " TARGET_FMT_lx "\n", __func__,
> @@ -187,7 +185,7 @@ static hwaddr ppc_hash32_bat_lookup(PowerPCCPU *cpu, target_ulong ea,
> hwaddr raddr = (batl & mask) | (ea & ~mask);
>
> if (unlikely(env->mmu_model == POWERPC_MMU_601)) {
> - *prot = hash32_bat_601_prot(cpu, batu, batl);
> + *prot = hash32_bat_601_prot(mmu_idx, batu, batl);
> } else {
> *prot = hash32_bat_prot(cpu, batu, batl);
> }
> @@ -221,12 +219,12 @@ static hwaddr ppc_hash32_bat_lookup(PowerPCCPU *cpu, target_ulong ea,
> static bool ppc_hash32_direct_store(PowerPCCPU *cpu, target_ulong sr,
> target_ulong eaddr,
> MMUAccessType access_type,
> - hwaddr *raddr, int *prot,
> + hwaddr *raddr, int *prot, int mmu_idx,
> bool guest_visible)
> {
> CPUState *cs = CPU(cpu);
> CPUPPCState *env = &cpu->env;
> - int key = !!(msr_pr ? (sr & SR32_KP) : (sr & SR32_KS));
> + int key = !!(mmuidx_pr(mmu_idx) ? (sr & SR32_KP) : (sr & SR32_KS));
>
> qemu_log_mask(CPU_LOG_MMU, "direct store...\n");
>
> @@ -425,7 +423,7 @@ static hwaddr ppc_hash32_pte_raddr(target_ulong sr, ppc_hash_pte32_t pte,
> }
>
> bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
> - hwaddr *raddrp, int *psizep, int *protp,
> + hwaddr *raddrp, int *psizep, int *protp, int mmu_idx,
> bool guest_visible)
> {
> CPUState *cs = CPU(cpu);
> @@ -441,7 +439,7 @@ bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
> *psizep = TARGET_PAGE_BITS;
>
> /* 1. Handle real mode accesses */
> - if (access_type == MMU_INST_FETCH ? !msr_ir : !msr_dr) {
> + if (mmuidx_real(mmu_idx)) {
> /* Translation is off */
> *raddrp = eaddr;
> *protp = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
> @@ -452,7 +450,7 @@ bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
>
> /* 2. Check Block Address Translation entries (BATs) */
> if (env->nb_BATs != 0) {
> - raddr = ppc_hash32_bat_lookup(cpu, eaddr, access_type, protp);
> + raddr = ppc_hash32_bat_lookup(cpu, eaddr, access_type, protp, mmu_idx);
> if (raddr != -1) {
> if (need_prot & ~*protp) {
> if (guest_visible) {
> @@ -483,7 +481,7 @@ bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
> /* 4. Handle direct store segments */
> if (sr & SR32_T) {
> return ppc_hash32_direct_store(cpu, sr, eaddr, access_type,
> - raddrp, protp, guest_visible);
> + raddrp, protp, mmu_idx, guest_visible);
> }
>
> /* 5. Check for segment level no-execute violation */
> @@ -520,7 +518,7 @@ bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
>
> /* 7. Check access permissions */
>
> - prot = ppc_hash32_pte_prot(cpu, sr, pte);
> + prot = ppc_hash32_pte_prot(mmu_idx, sr, pte);
>
> if (need_prot & ~prot) {
> /* Access right violation */
> diff --git a/target/ppc/mmu-hash32.h b/target/ppc/mmu-hash32.h
> index 8694eccabd..807d9bc6e8 100644
> --- a/target/ppc/mmu-hash32.h
> +++ b/target/ppc/mmu-hash32.h
> @@ -5,7 +5,7 @@
>
> hwaddr get_pteg_offset32(PowerPCCPU *cpu, hwaddr hash);
> bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
> - hwaddr *raddrp, int *psizep, int *protp,
> + hwaddr *raddrp, int *psizep, int *protp, int mmu_idx,
> bool guest_visible);
>
> /*
> diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> index 9dcdf88597..a3381e1aa0 100644
> --- a/target/ppc/mmu_helper.c
> +++ b/target/ppc/mmu_helper.c
> @@ -2922,7 +2922,7 @@ static bool ppc_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
> case POWERPC_MMU_32B:
> case POWERPC_MMU_601:
> return ppc_hash32_xlate(cpu, eaddr, access_type,
> - raddrp, psizep, protp, guest_visible);
> + raddrp, psizep, protp, mmu_idx, guest_visible);
>
> default:
> return ppc_jumbo_xlate(cpu, eaddr, access_type, raddrp,
--
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
On 05/07/2021 01:24, David Gibson wrote: > > Changed hash32 address translation to use the supplied mmu_idx, instead > > of using what was stored in the msr, for parity purposes (radix64 > > already uses that). > Well.. parity and conceptual correctness. The translation is supposed > to use mmu_idx, not look at the CPU again to get the right context. > AFAIK there isn't a situation in hash32 where they'll get out of sync, > but nothing guarantees that. Fair point, I can change the description if I do end up with a new version, but > I think the right approach is to duplicate the helper macros in > mmu-hash32.h for now. We can unify them later with a more thorough > review (which would probably involve creating a new header for things > common to all BookS family MMUs). This doesn't work directly. I'd need to put in an ifndef PPC_MMU_BOOK3S_V3_H, which also feels a bit dubious to me. I can go with whichever one you prefer -- Bruno Piazera Larsen Instituto de Pesquisas ELDORADO <https://www.eldorado.org.br/?utm_campaign=assinatura_de_e-mail&utm_medium=email&utm_source=RD+Station> Departamento Computação Embarcada Analista de Software Trainee Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
On Mon, Jul 05, 2021 at 11:31:13AM -0300, Bruno Piazera Larsen wrote: > > On 05/07/2021 01:24, David Gibson wrote: > > > > Changed hash32 address translation to use the supplied mmu_idx, instead > > > of using what was stored in the msr, for parity purposes (radix64 > > > already uses that). > > > Well.. parity and conceptual correctness. The translation is supposed > > to use mmu_idx, not look at the CPU again to get the right context. > > AFAIK there isn't a situation in hash32 where they'll get out of sync, > > but nothing guarantees that. > > > Fair point, I can change the description if I do end up with a new > version, but > > > I think the right approach is to duplicate the helper macros in > > mmu-hash32.h for now. We can unify them later with a more thorough > > review (which would probably involve creating a new header for things > > common to all BookS family MMUs). > > This doesn't work directly. I'd need to put in an ifndef > PPC_MMU_BOOK3S_V3_H, which also feels a bit dubious to me. I can go > with whichever one you prefer Ah... good point, because both headers are included in some places. Ok, in that case let's jump ahead instead. Let's create a new mmu-books.h to cover all MMUs based on the "classic" powerpc model, and put them in there. hash32 and book3s-v3 can include that, BookE, 4xx etc. will not. For now these will be the only things in there, but there will probably be more we can add later on. -- 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
© 2016 - 2026 Red Hat, Inc.