QEMU-Devel Archive mirror
 help / color / mirror / Atom feed
From: "Nicholas Piggin" <npiggin@gmail.com>
To: "BALATON Zoltan" <balaton@eik.bme.hu>, <qemu-devel@nongnu.org>,
	<qemu-ppc@nongnu.org>
Cc: "Daniel Henrique Barboza" <danielhb413@gmail.com>
Subject: Re: [PATCH v2 22/28] target/ppc: Remove ppc_hash32_pp_prot() and reuse common function
Date: Tue, 07 May 2024 21:35:04 +1000	[thread overview]
Message-ID: <D13DQ21WSMJQ.2KDPTPCN9FM4G@gmail.com> (raw)
In-Reply-To: <cc6221f9333bae65037c42ca9ba0540f9e96c493.1714606359.git.balaton@eik.bme.hu>

On Thu May 2, 2024 at 9:43 AM AEST, BALATON Zoltan wrote:
> The ppc_hash32_pp_prot() function in mmu-hash32.c is the same as
> pp_check() in mmu_common.c. Rename the latter to ppc_pte_prot() and
> merge with ppc_hash32_pp_prot() to remove duplicated code.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  target/ppc/internal.h   |  2 +-
>  target/ppc/mmu-hash32.c | 47 +----------------------------------------
>  target/ppc/mmu_common.c | 19 +++++++++--------
>  3 files changed, 12 insertions(+), 56 deletions(-)
>
> diff --git a/target/ppc/internal.h b/target/ppc/internal.h
> index 61c2aadd0d..d7c923b017 100644
> --- a/target/ppc/internal.h
> +++ b/target/ppc/internal.h
> @@ -255,7 +255,7 @@ static inline int prot_for_access_type(MMUAccessType access_type)
>  #ifndef CONFIG_USER_ONLY
>  
>  /* PowerPC MMU emulation */
> -
> +int ppc_pte_prot(int key, int pp, int nx);

Hmm, these were split by 496272a7018. 64 was being split
out too at the time, so maybe not immediately obvious
they were the same.

Good consolidation but can you keep pp in the name?
It's taking ppc's PTE[pp] (page protection) field and
converting it to QEMU prot value.

ppc_hash32_pp_prot is probably fine, 6xx is hash too.
and could stay in mmu-hash32.c, so just use that
version unchanged?

Thanks,
Nick

>  bool ppc_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
>                        hwaddr *raddrp, int *psizep, int *protp,
>                        int mmu_idx, bool guest_visible);
> diff --git a/target/ppc/mmu-hash32.c b/target/ppc/mmu-hash32.c
> index 3976416840..ee9df351ae 100644
> --- a/target/ppc/mmu-hash32.c
> +++ b/target/ppc/mmu-hash32.c
> @@ -42,51 +42,6 @@ struct mmu_ctx_hash32 {
>      int key;                       /* Access key                */
>  };
>  
> -static int ppc_hash32_pp_prot(int key, int pp, int nx)
> -{
> -    int prot;
> -
> -    if (key == 0) {
> -        switch (pp) {
> -        case 0x0:
> -        case 0x1:
> -        case 0x2:
> -            prot = PAGE_READ | PAGE_WRITE;
> -            break;
> -
> -        case 0x3:
> -            prot = PAGE_READ;
> -            break;
> -
> -        default:
> -            abort();
> -        }
> -    } else {
> -        switch (pp) {
> -        case 0x0:
> -            prot = 0;
> -            break;
> -
> -        case 0x1:
> -        case 0x3:
> -            prot = PAGE_READ;
> -            break;
> -
> -        case 0x2:
> -            prot = PAGE_READ | PAGE_WRITE;
> -            break;
> -
> -        default:
> -            abort();
> -        }
> -    }
> -    if (nx == 0) {
> -        prot |= PAGE_EXEC;
> -    }
> -
> -    return prot;
> -}
> -
>  static int ppc_hash32_pte_prot(int mmu_idx,
>                                 target_ulong sr, ppc_hash_pte32_t pte)
>  {
> @@ -95,7 +50,7 @@ static int ppc_hash32_pte_prot(int mmu_idx,
>      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));
> +    return ppc_pte_prot(key, pp, !!(sr & SR32_NX));
>  }
>  
>  static target_ulong hash32_bat_size(int mmu_idx,
> diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
> index 41ef174ab4..0ce5c1e841 100644
> --- a/target/ppc/mmu_common.c
> +++ b/target/ppc/mmu_common.c
> @@ -75,22 +75,23 @@ void ppc_store_sdr1(CPUPPCState *env, target_ulong value)
>  /*****************************************************************************/
>  /* PowerPC MMU emulation */
>  
> -static int pp_check(int key, int pp, int nx)
> +int ppc_pte_prot(int key, int pp, int nx)
>  {
>      int access;
>  
>      /* Compute access rights */
> -    access = 0;
>      if (key == 0) {
>          switch (pp) {
>          case 0x0:
>          case 0x1:
>          case 0x2:
> -            access |= PAGE_WRITE;
> -            /* fall through */
> +            access = PAGE_READ | PAGE_WRITE;
> +            break;
>          case 0x3:
> -            access |= PAGE_READ;
> +            access = PAGE_READ;
>              break;
> +        default:
> +            g_assert_not_reached();
>          }
>      } else {
>          switch (pp) {
> @@ -104,6 +105,8 @@ static int pp_check(int key, int pp, int nx)
>          case 0x2:
>              access = PAGE_READ | PAGE_WRITE;
>              break;
> +        default:
> +            g_assert_not_reached();
>          }
>      }
>      if (nx == 0) {
> @@ -140,7 +143,7 @@ static int ppc6xx_tlb_pte_check(mmu_ctx_t *ctx, target_ulong pte0,
>                                  MMUAccessType access_type)
>  {
>      target_ulong ptem, mmask;
> -    int access, ret, pteh, ptev, pp;
> +    int ret, pteh, ptev, pp;
>  
>      ret = -1;
>      /* Check validity and table match */
> @@ -159,11 +162,9 @@ static int ppc6xx_tlb_pte_check(mmu_ctx_t *ctx, target_ulong pte0,
>                      return -3;
>                  }
>              }
> -            /* Compute access rights */
> -            access = pp_check(ctx->key, pp, ctx->nx);
>              /* Keep the matching PTE information */
>              ctx->raddr = pte1;
> -            ctx->prot = access;
> +            ctx->prot = ppc_pte_prot(ctx->key, pp, ctx->nx);
>              ret = check_prot(ctx->prot, access_type);
>              if (ret == 0) {
>                  /* Access granted */



  reply	other threads:[~2024-05-07 11:36 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-01 23:43 [PATCH v2 00/28] Misc PPC exception and BookE MMU clean ups BALATON Zoltan
2024-05-01 23:43 ` [PATCH v2 01/28] target/ppc: Fix gen_sc to use correct nip BALATON Zoltan
2024-05-01 23:43 ` [PATCH v2 02/28] target/ppc: Move patching nip from exception handler to helper_scv BALATON Zoltan
2024-05-01 23:43 ` [PATCH v2 03/28] target/ppc: Simplify syscall exception handlers BALATON Zoltan
2024-05-01 23:43 ` [PATCH v2 04/28] target/ppc: Remove unused helper BALATON Zoltan
2024-05-07  9:18   ` Nicholas Piggin
2024-05-01 23:43 ` [PATCH v2 05/28] target/ppc/mmu_common.c: Move calculation of a value closer to its usage BALATON Zoltan
2024-05-07  9:19   ` Nicholas Piggin
2024-05-01 23:43 ` [PATCH v2 06/28] " BALATON Zoltan
2024-05-07  9:20   ` Nicholas Piggin
2024-05-01 23:43 ` [PATCH v2 07/28] target/ppc/mmu_common.c: Remove unneeded local variable BALATON Zoltan
2024-05-07  9:30   ` Nicholas Piggin
2024-05-01 23:43 ` [PATCH v2 08/28] target/ppc/mmu_common.c: Simplify checking for real mode BALATON Zoltan
2024-05-07  9:34   ` Nicholas Piggin
2024-05-01 23:43 ` [PATCH v2 09/28] target/ppc/mmu_common.c: Drop cases for unimplemented MPC8xx MMU BALATON Zoltan
2024-05-07  9:36   ` Nicholas Piggin
2024-05-01 23:43 ` [PATCH v2 10/28] target/ppc/mmu_common.c: Introduce mmu6xx_get_physical_address() BALATON Zoltan
2024-05-07  9:42   ` Nicholas Piggin
2024-05-01 23:43 ` [PATCH v2 11/28] target/ppc/mmu_common.c: Rename get_bat_6xx_tlb() BALATON Zoltan
2024-05-07  9:43   ` Nicholas Piggin
2024-05-01 23:43 ` [PATCH v2 12/28] target/ppc/mmu_common.c: Split out BookE cases before checking real mode BALATON Zoltan
2024-05-07  9:50   ` Nicholas Piggin
2024-05-01 23:43 ` [PATCH v2 13/28] target/ppc/mmu_common.c: Split off real mode cases in get_physical_address_wtlb() BALATON Zoltan
2024-05-07  9:58   ` Nicholas Piggin
2024-05-01 23:43 ` [PATCH v2 14/28] target/ppc/mmu_common.c: Inline and remove check_physical() BALATON Zoltan
2024-05-07 10:00   ` Nicholas Piggin
2024-05-01 23:43 ` [PATCH v2 15/28] target/ppc/mmu_common.c: Simplify mmubooke_get_physical_address() BALATON Zoltan
2024-05-07 10:03   ` Nicholas Piggin
2024-05-01 23:43 ` [PATCH v2 16/28] target/ppc/mmu_common.c: Simplify mmubooke206_get_physical_address() BALATON Zoltan
2024-05-07 10:04   ` Nicholas Piggin
2024-05-01 23:43 ` [PATCH v2 17/28] target/ppc/mmu_common.c: Fix misindented qemu_log_mask() calls BALATON Zoltan
2024-05-07 10:05   ` Nicholas Piggin
2024-05-01 23:43 ` [PATCH v2 18/28] target/ppc/mmu_common.c: Deindent ppc_jumbo_xlate() BALATON Zoltan
2024-05-07 10:06   ` Nicholas Piggin
2024-05-01 23:43 ` [PATCH v2 19/28] target/ppc/mmu_common.c: Replace hard coded constants in ppc_jumbo_xlate() BALATON Zoltan
2024-05-07 10:11   ` Nicholas Piggin
2024-05-01 23:43 ` [PATCH v2 20/28] target/ppc/mmu_common.c: Make get_physical_address_wtlb() static BALATON Zoltan
2024-05-07 10:47   ` Nicholas Piggin
2024-05-07 15:31     ` BALATON Zoltan
2024-05-01 23:43 ` [PATCH v2 21/28] target/ppc: Move mmu_ctx_t definition to mmu_common.c BALATON Zoltan
2024-05-07 10:49   ` Nicholas Piggin
2024-05-01 23:43 ` [PATCH v2 22/28] target/ppc: Remove ppc_hash32_pp_prot() and reuse common function BALATON Zoltan
2024-05-07 11:35   ` Nicholas Piggin [this message]
2024-05-01 23:43 ` [PATCH v2 23/28] target/ppc/mmu_common.c: Split off BookE handling from ppc_jumbo_xlate() BALATON Zoltan
2024-05-07 11:51   ` Nicholas Piggin
2024-05-01 23:43 ` [PATCH v2 24/28] target/ppc/mmu_common.c: Remove BookE handling from get_physical_address_wtlb() BALATON Zoltan
2024-05-07 12:05   ` Nicholas Piggin
2024-05-07 23:40     ` BALATON Zoltan
2024-05-08 12:54       ` Nicholas Piggin
2024-05-01 23:43 ` [PATCH v2 25/28] target/ppc/mmu_common.c: Simplify ppc_booke_xlate() BALATON Zoltan
2024-05-07 12:15   ` Nicholas Piggin
2024-05-01 23:43 ` [PATCH v2 26/28] target/ppc/mmu_common.c: Move BookE MMU functions together BALATON Zoltan
2024-05-07 12:17   ` Nicholas Piggin
2024-05-07 12:31     ` BALATON Zoltan
2024-05-08 12:30       ` Nicholas Piggin
2024-05-08 23:33         ` BALATON Zoltan
2024-05-09  5:57           ` Nicholas Piggin
2024-05-07 15:54     ` BALATON Zoltan
2024-05-01 23:43 ` [PATCH v2 27/28] target/ppc: Remove id_tlbs flag from CPU env BALATON Zoltan
2024-05-07 12:30   ` Nicholas Piggin
2024-05-07 16:02     ` BALATON Zoltan
2024-05-08 12:37       ` Nicholas Piggin
2024-05-01 23:43 ` [PATCH v2 28/28] target/ppc: Split off common 4xx TLB init BALATON Zoltan
2024-05-07 12:40   ` Nicholas Piggin
2024-05-07 12:45 ` [PATCH v2 00/28] Misc PPC exception and BookE MMU clean ups Nicholas Piggin
2024-05-07 12:51   ` BALATON Zoltan

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=D13DQ21WSMJQ.2KDPTPCN9FM4G@gmail.com \
    --to=npiggin@gmail.com \
    --cc=balaton@eik.bme.hu \
    --cc=danielhb413@gmail.com \
    --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 read-only IMAP folder(s) and NNTP newsgroup(s).