From: David Gibson <david@gibson.dropbear.id.au>
To: "Bruno Larsen (billionai)" <bruno.larsen@eldorado.org.br>
Cc: farosas@linux.ibm.com,
Richard Henderson <richard.henderson@linaro.org>,
qemu-devel@nongnu.org, Greg Kurz <groug@kaod.org>,
lucas.araujo@eldorado.org.br, fernando.valle@eldorado.org.br,
qemu-ppc@nongnu.org, clg@kaod.org, matheus.ferst@eldorado.org.br,
luis.pires@eldorado.org.br
Subject: Re: [PATCH v2 07/10] target/ppc: Split out ppc_jumbo_xlate
Date: Thu, 24 Jun 2021 16:30:47 +1000 [thread overview]
Message-ID: <YNQml4olwaspHlnp@yekko> (raw)
In-Reply-To: <20210621125115.67717-8-bruno.larsen@eldorado.org.br>
[-- Attachment #1: Type: text/plain, Size: 15691 bytes --]
On Mon, Jun 21, 2021 at 09:51:12AM -0300, Bruno Larsen (billionai) wrote:
> From: Richard Henderson <richard.henderson@linaro.org>
>
> Mirror the interface of ppc_radix64_xlate (mostly), putting all
> of the logic for older mmu translation into a single entry point.
> For booke, we need to add mmu_idx to the xlate-style interface.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Applied to ppc-for-6.1. So, I noticed reviewing this that the
existing debug path is wrong - a bunch of the get_physical_address()
functions make guest visible state changes. Oh well, this doesn't
make it any worse.
> ---
> target/ppc/mmu_helper.c | 179 +++++++++++++++++++++-------------------
> 1 file changed, 96 insertions(+), 83 deletions(-)
>
> diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> index c4b1c93e47..2e92deb105 100644
> --- a/target/ppc/mmu_helper.c
> +++ b/target/ppc/mmu_helper.c
> @@ -1435,48 +1435,6 @@ static int get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
> }
> #endif
>
> -hwaddr ppc_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
> -{
> - PowerPCCPU *cpu = POWERPC_CPU(cs);
> - CPUPPCState *env = &cpu->env;
> - mmu_ctx_t ctx;
> -
> - switch (env->mmu_model) {
> -#if defined(TARGET_PPC64)
> - case POWERPC_MMU_64B:
> - case POWERPC_MMU_2_03:
> - case POWERPC_MMU_2_06:
> - case POWERPC_MMU_2_07:
> - return ppc_hash64_get_phys_page_debug(cpu, addr);
> - case POWERPC_MMU_3_00:
> - return ppc64_v3_get_phys_page_debug(cpu, addr);
> -#endif
> -
> - case POWERPC_MMU_32B:
> - case POWERPC_MMU_601:
> - return ppc_hash32_get_phys_page_debug(cpu, addr);
> -
> - default:
> - ;
> - }
> -
> - if (unlikely(get_physical_address(env, &ctx, addr, MMU_DATA_LOAD,
> - ACCESS_INT) != 0)) {
> -
> - /*
> - * Some MMUs have separate TLBs for code and data. If we only
> - * try an ACCESS_INT, we may not be able to read instructions
> - * mapped by code TLBs, so we also try a ACCESS_CODE.
> - */
> - if (unlikely(get_physical_address(env, &ctx, addr, MMU_INST_FETCH,
> - ACCESS_CODE) != 0)) {
> - return -1;
> - }
> - }
> -
> - return ctx.raddr & TARGET_PAGE_MASK;
> -}
> -
> static void booke206_update_mas_tlb_miss(CPUPPCState *env, target_ulong address,
> MMUAccessType access_type, int mmu_idx)
> {
> @@ -1532,30 +1490,38 @@ static void booke206_update_mas_tlb_miss(CPUPPCState *env, target_ulong address,
> }
>
> /* Perform address translation */
> -static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
> - MMUAccessType access_type, int mmu_idx)
> +/* TODO: Split this by mmu_model. */
> +static bool ppc_jumbo_xlate(PowerPCCPU *cpu, vaddr eaddr,
> + MMUAccessType access_type,
> + hwaddr *raddrp, int *psizep, int *protp,
> + int mmu_idx, bool guest_visible)
> {
> - CPUState *cs = env_cpu(env);
> - PowerPCCPU *cpu = POWERPC_CPU(cs);
> + CPUState *cs = CPU(cpu);
> + CPUPPCState *env = &cpu->env;
> mmu_ctx_t ctx;
> int type;
> - int ret = 0;
> + int ret;
>
> if (access_type == MMU_INST_FETCH) {
> /* code access */
> type = ACCESS_CODE;
> - } else {
> + } else if (guest_visible) {
> /* data access */
> type = env->access_type;
> + } else {
> + type = ACCESS_INT;
> }
> - ret = get_physical_address_wtlb(env, &ctx, address, access_type,
> +
> + ret = get_physical_address_wtlb(env, &ctx, eaddr, access_type,
> type, mmu_idx);
> if (ret == 0) {
> - tlb_set_page(cs, address & TARGET_PAGE_MASK,
> - ctx.raddr & TARGET_PAGE_MASK, ctx.prot,
> - mmu_idx, TARGET_PAGE_SIZE);
> - ret = 0;
> - } else if (ret < 0) {
> + *raddrp = ctx.raddr;
> + *protp = ctx.prot;
> + *psizep = TARGET_PAGE_BITS;
> + return true;
> + }
> +
> + if (guest_visible) {
> LOG_MMU_STATE(cs);
> if (type == ACCESS_CODE) {
> switch (ret) {
> @@ -1565,7 +1531,7 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
> case POWERPC_MMU_SOFT_6xx:
> cs->exception_index = POWERPC_EXCP_IFTLB;
> env->error_code = 1 << 18;
> - env->spr[SPR_IMISS] = address;
> + env->spr[SPR_IMISS] = eaddr;
> env->spr[SPR_ICMP] = 0x80000000 | ctx.ptem;
> goto tlb_miss;
> case POWERPC_MMU_SOFT_74xx:
> @@ -1575,29 +1541,25 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
> case POWERPC_MMU_SOFT_4xx_Z:
> cs->exception_index = POWERPC_EXCP_ITLB;
> env->error_code = 0;
> - env->spr[SPR_40x_DEAR] = address;
> + env->spr[SPR_40x_DEAR] = eaddr;
> env->spr[SPR_40x_ESR] = 0x00000000;
> break;
> case POWERPC_MMU_BOOKE206:
> - booke206_update_mas_tlb_miss(env, address, 2, mmu_idx);
> + booke206_update_mas_tlb_miss(env, eaddr, 2, mmu_idx);
> /* fall through */
> case POWERPC_MMU_BOOKE:
> cs->exception_index = POWERPC_EXCP_ITLB;
> env->error_code = 0;
> - env->spr[SPR_BOOKE_DEAR] = address;
> + env->spr[SPR_BOOKE_DEAR] = eaddr;
> env->spr[SPR_BOOKE_ESR] = mmubooke206_esr(mmu_idx, MMU_DATA_LOAD);
> - return -1;
> + break;
> case POWERPC_MMU_MPC8xx:
> - /* XXX: TODO */
> cpu_abort(cs, "MPC8xx MMU model is not implemented\n");
> - break;
> case POWERPC_MMU_REAL:
> cpu_abort(cs, "PowerPC in real mode should never raise "
> "any MMU exceptions\n");
> - return -1;
> default:
> cpu_abort(cs, "Unknown or invalid MMU model\n");
> - return -1;
> }
> break;
> case -2:
> @@ -1634,7 +1596,7 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
> cs->exception_index = POWERPC_EXCP_DLTLB;
> env->error_code = 0;
> }
> - env->spr[SPR_DMISS] = address;
> + env->spr[SPR_DMISS] = eaddr;
> env->spr[SPR_DCMP] = 0x80000000 | ctx.ptem;
> tlb_miss:
> env->error_code |= ctx.key << 19;
> @@ -1652,7 +1614,7 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
> tlb_miss_74xx:
> /* Implement LRU algorithm */
> env->error_code = ctx.key << 19;
> - env->spr[SPR_TLBMISS] = (address & ~((target_ulong)0x3)) |
> + env->spr[SPR_TLBMISS] = (eaddr & ~((target_ulong)0x3)) |
> ((env->last_way + 1) & (env->nb_ways - 1));
> env->spr[SPR_PTEHI] = 0x80000000 | ctx.ptem;
> break;
> @@ -1660,7 +1622,7 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
> case POWERPC_MMU_SOFT_4xx_Z:
> cs->exception_index = POWERPC_EXCP_DTLB;
> env->error_code = 0;
> - env->spr[SPR_40x_DEAR] = address;
> + env->spr[SPR_40x_DEAR] = eaddr;
> if (access_type == MMU_DATA_STORE) {
> env->spr[SPR_40x_ESR] = 0x00800000;
> } else {
> @@ -1670,23 +1632,20 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
> case POWERPC_MMU_MPC8xx:
> /* XXX: TODO */
> cpu_abort(cs, "MPC8xx MMU model is not implemented\n");
> - break;
> case POWERPC_MMU_BOOKE206:
> - booke206_update_mas_tlb_miss(env, address, access_type, mmu_idx);
> + booke206_update_mas_tlb_miss(env, eaddr, access_type, mmu_idx);
> /* fall through */
> case POWERPC_MMU_BOOKE:
> cs->exception_index = POWERPC_EXCP_DTLB;
> env->error_code = 0;
> - env->spr[SPR_BOOKE_DEAR] = address;
> + env->spr[SPR_BOOKE_DEAR] = eaddr;
> env->spr[SPR_BOOKE_ESR] = mmubooke206_esr(mmu_idx, access_type);
> - return -1;
> + break;
> case POWERPC_MMU_REAL:
> cpu_abort(cs, "PowerPC in real mode should never raise "
> "any MMU exceptions\n");
> - return -1;
> default:
> cpu_abort(cs, "Unknown or invalid MMU model\n");
> - return -1;
> }
> break;
> case -2:
> @@ -1695,16 +1654,16 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
> env->error_code = 0;
> if (env->mmu_model == POWERPC_MMU_SOFT_4xx
> || env->mmu_model == POWERPC_MMU_SOFT_4xx_Z) {
> - env->spr[SPR_40x_DEAR] = address;
> + env->spr[SPR_40x_DEAR] = eaddr;
> if (access_type == MMU_DATA_STORE) {
> env->spr[SPR_40x_ESR] |= 0x00800000;
> }
> } else if ((env->mmu_model == POWERPC_MMU_BOOKE) ||
> (env->mmu_model == POWERPC_MMU_BOOKE206)) {
> - env->spr[SPR_BOOKE_DEAR] = address;
> + env->spr[SPR_BOOKE_DEAR] = eaddr;
> env->spr[SPR_BOOKE_ESR] = mmubooke206_esr(mmu_idx, access_type);
> } else {
> - env->spr[SPR_DAR] = address;
> + env->spr[SPR_DAR] = eaddr;
> if (access_type == MMU_DATA_STORE) {
> env->spr[SPR_DSISR] = 0x0A000000;
> } else {
> @@ -1719,13 +1678,13 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
> /* Floating point load/store */
> cs->exception_index = POWERPC_EXCP_ALIGN;
> env->error_code = POWERPC_EXCP_ALIGN_FP;
> - env->spr[SPR_DAR] = address;
> + env->spr[SPR_DAR] = eaddr;
> break;
> case ACCESS_RES:
> /* lwarx, ldarx or stwcx. */
> cs->exception_index = POWERPC_EXCP_DSI;
> env->error_code = 0;
> - env->spr[SPR_DAR] = address;
> + env->spr[SPR_DAR] = eaddr;
> if (access_type == MMU_DATA_STORE) {
> env->spr[SPR_DSISR] = 0x06000000;
> } else {
> @@ -1736,7 +1695,7 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
> /* eciwx or ecowx */
> cs->exception_index = POWERPC_EXCP_DSI;
> env->error_code = 0;
> - env->spr[SPR_DAR] = address;
> + env->spr[SPR_DAR] = eaddr;
> if (access_type == MMU_DATA_STORE) {
> env->spr[SPR_DSISR] = 0x06100000;
> } else {
> @@ -1748,16 +1707,14 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
> cs->exception_index = POWERPC_EXCP_PROGRAM;
> env->error_code =
> POWERPC_EXCP_INVAL | POWERPC_EXCP_INVAL_INVAL;
> - env->spr[SPR_DAR] = address;
> + env->spr[SPR_DAR] = eaddr;
> break;
> }
> break;
> }
> }
> - ret = 1;
> }
> -
> - return ret;
> + return false;
> }
>
> #ifdef CONFIG_TCG
> @@ -2942,6 +2899,62 @@ void helper_check_tlb_flush_global(CPUPPCState *env)
>
> /*****************************************************************************/
>
> +static int cpu_ppc_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
> + MMUAccessType access_type, int mmu_idx)
> +{
> + CPUState *cs = CPU(cpu);
> + int page_size, prot;
> + hwaddr raddr;
> +
> + if (!ppc_jumbo_xlate(cpu, eaddr, access_type, &raddr,
> + &page_size, &prot, mmu_idx, true)) {
> + return 1;
> + }
> +
> + tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr & TARGET_PAGE_MASK,
> + prot, mmu_idx, 1UL << page_size);
> + return 0;
> +}
> +
> +hwaddr ppc_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
> +{
> + PowerPCCPU *cpu = POWERPC_CPU(cs);
> + CPUPPCState *env = &cpu->env;
> + hwaddr raddr;
> + int s, p;
> +
> + switch (env->mmu_model) {
> +#if defined(TARGET_PPC64)
> + case POWERPC_MMU_64B:
> + case POWERPC_MMU_2_03:
> + case POWERPC_MMU_2_06:
> + case POWERPC_MMU_2_07:
> + return ppc_hash64_get_phys_page_debug(cpu, addr);
> + case POWERPC_MMU_3_00:
> + return ppc64_v3_get_phys_page_debug(cpu, addr);
> +#endif
> +
> + case POWERPC_MMU_32B:
> + case POWERPC_MMU_601:
> + return ppc_hash32_get_phys_page_debug(cpu, addr);
> +
> + default:
> + ;
> + }
> +
> + /*
> + * Some MMUs have separate TLBs for code and data. If we only
> + * try an MMU_DATA_LOAD, we may not be able to read instructions
> + * mapped by code TLBs, so we also try a MMU_INST_FETCH.
> + */
> + if (ppc_jumbo_xlate(cpu, addr, MMU_DATA_LOAD, &raddr, &s, &p, 0, false) ||
> + ppc_jumbo_xlate(cpu, addr, MMU_INST_FETCH, &raddr, &s, &p, 0, false)) {
> + return raddr & TARGET_PAGE_MASK;
> + }
> + return -1;
> +}
> +
> +
> bool ppc_cpu_tlb_fill(CPUState *cs, vaddr addr, int size,
> MMUAccessType access_type, int mmu_idx,
> bool probe, uintptr_t retaddr)
> @@ -2969,7 +2982,7 @@ bool ppc_cpu_tlb_fill(CPUState *cs, vaddr addr, int size,
> break;
>
> default:
> - ret = cpu_ppc_handle_mmu_fault(env, addr, access_type, mmu_idx);
> + ret = cpu_ppc_handle_mmu_fault(cpu, addr, access_type, mmu_idx);
> break;
> }
> if (unlikely(ret != 0)) {
--
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:[~2021-06-24 6:56 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-21 12:51 [PATCH v2 00/10] Clean up MMU translation Bruno Larsen (billionai)
2021-06-21 12:51 ` [PATCH v2 01/10] target/ppc: Remove PowerPCCPUClass.handle_mmu_fault Bruno Larsen (billionai)
2021-06-22 10:49 ` Greg Kurz
2021-06-24 1:40 ` David Gibson
2021-06-21 12:51 ` [PATCH v2 02/10] target/ppc: Use MMUAccessType with *_handle_mmu_fault Bruno Larsen (billionai)
2021-06-22 12:05 ` Greg Kurz
2021-06-24 3:19 ` David Gibson
2021-06-21 12:51 ` [PATCH v2 03/10] target/ppc: Push real-mode handling into ppc_radix64_xlate Bruno Larsen (billionai)
2021-06-24 3:29 ` David Gibson
2021-06-21 12:51 ` [PATCH v2 04/10] target/ppc: Use bool success for ppc_radix64_xlate Bruno Larsen (billionai)
2021-06-24 3:31 ` David Gibson
2021-06-21 12:51 ` [PATCH v2 05/10] target/ppc: Split out ppc_hash64_xlate Bruno Larsen (billionai)
2021-06-24 5:55 ` David Gibson
2021-06-21 12:51 ` [PATCH v2 06/10] target/ppc: Split out ppc_hash32_xlate Bruno Larsen (billionai)
2021-06-21 12:51 ` [PATCH v2 07/10] target/ppc: Split out ppc_jumbo_xlate Bruno Larsen (billionai)
2021-06-24 6:30 ` David Gibson [this message]
2021-06-21 12:51 ` [PATCH v2 08/10] target/ppc: Introduce ppc_xlate Bruno Larsen (billionai)
2021-06-24 6:34 ` David Gibson
2021-06-21 12:51 ` [PATCH v2 09/10] target/ppc: Restrict ppc_cpu_tlb_fill to TCG Bruno Larsen (billionai)
2021-06-24 6:35 ` David Gibson
2021-06-21 12:51 ` [PATCH v2 10/10] target/ppc: fix address translation bug for radix mmus Bruno Larsen (billionai)
2021-06-24 6:48 ` 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=YNQml4olwaspHlnp@yekko \
--to=david@gibson.dropbear.id.au \
--cc=bruno.larsen@eldorado.org.br \
--cc=clg@kaod.org \
--cc=farosas@linux.ibm.com \
--cc=fernando.valle@eldorado.org.br \
--cc=groug@kaod.org \
--cc=lucas.araujo@eldorado.org.br \
--cc=luis.pires@eldorado.org.br \
--cc=matheus.ferst@eldorado.org.br \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
--cc=richard.henderson@linaro.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).