[prev in list] [next in list] [prev in thread] [next in thread]
List: qemu-ppc
Subject: Re: [PATCH v7 35/61] target/ppc: Remove pp_check() and reuse ppc_hash32_pp_prot()
From: BALATON Zoltan <balaton () eik ! bme ! hu>
Date: 2024-05-17 8:58:54
Message-ID: 006b775c-e2b3-c729-f758-4302a98fa07b () eik ! bme ! hu
[Download RAW message or body]
On Fri, 17 May 2024, Nicholas Piggin wrote:
> On Mon May 13, 2024 at 9:28 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, merge these to remove duplicated code.
>> Define the common function as static lnline otherwise exporting the
>> function from mmu-hash32.c would stop the compiler inlining it which
>> results in slightly lower performance.
>>
>
> It's already hard to review patches that move code around, it's better
> to keep the changes before/after the move unless really necessary.
I could try to split this further but the series was already quite long
and this is not too complex and also there's git diff --color-moved so I
though this could be in in one patch.
> For mmu_common.c hunks,
>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Hmm, I can't apply Rb to hunks so I think I have to ignore it for now and
wait until you send Rb for whole patch.
Just to make it simpler could you please send a pull request for the
patches that are already reviewed at the beginning of the series to reduce
the number of patches I need to resend? I've already added some more and
still have some plans to continue so moving the patches that are OK out of
the way could help. Then I could just resend the patches starting from the
first that's not yet reviewed. Thank you for taking time to review these.
Regards,
BALATON Zoltan
> Thanks,
> Nick
>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>> target/ppc/mmu-hash32.c | 45 -----------------------------------------
>> target/ppc/mmu-hash32.h | 36 +++++++++++++++++++++++++++++++++
>> target/ppc/mmu_common.c | 44 ++--------------------------------------
>> 3 files changed, 38 insertions(+), 87 deletions(-)
>>
>> diff --git a/target/ppc/mmu-hash32.c b/target/ppc/mmu-hash32.c
>> index 1e8f1df0f0..d5f2057eb1 100644
>> --- a/target/ppc/mmu-hash32.c
>> +++ b/target/ppc/mmu-hash32.c
>> @@ -37,51 +37,6 @@
>> # define LOG_BATS(...) do { } while (0)
>> #endif
>>
>> -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)
>> {
>> diff --git a/target/ppc/mmu-hash32.h b/target/ppc/mmu-hash32.h
>> index 7119a63d97..bf99161858 100644
>> --- a/target/ppc/mmu-hash32.h
>> +++ b/target/ppc/mmu-hash32.h
>> @@ -102,6 +102,42 @@ static inline void ppc_hash32_store_hpte1(PowerPCCPU *cpu,
>> stl_phys(CPU(cpu)->as, base + pte_offset + HASH_PTE_SIZE_32 / 2, pte1);
>> }
>>
>> +static inline int ppc_hash32_pp_prot(bool key, int pp, bool nx)
>> +{
>> + int prot;
>> +
>> + if (key) {
>> + switch (pp) {
>> + case 0x0:
>> + prot = 0;
>> + break;
>> + case 0x1:
>> + case 0x3:
>> + prot = PAGE_READ;
>> + break;
>> + case 0x2:
>> + prot = PAGE_READ | PAGE_WRITE;
>> + break;
>> + default:
>> + g_assert_not_reached();
>> + }
>> + } else {
>> + switch (pp) {
>> + case 0x0:
>> + case 0x1:
>> + case 0x2:
>> + prot = PAGE_READ | PAGE_WRITE;
>> + break;
>> + case 0x3:
>> + prot = PAGE_READ;
>> + break;
>> + default:
>> + g_assert_not_reached();
>> + }
>> + }
>> + return nx ? prot : prot | PAGE_EXEC;
>> +}
>> +
>> typedef struct {
>> uint32_t pte0, pte1;
>> } ppc_hash_pte32_t;
>> diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
>> index e1462a25dd..9e0bfbda67 100644
>> --- a/target/ppc/mmu_common.c
>> +++ b/target/ppc/mmu_common.c
>> @@ -77,44 +77,6 @@ void ppc_store_sdr1(CPUPPCState *env, target_ulong value)
>> /*****************************************************************************/
>> /* PowerPC MMU emulation */
>>
>> -static int pp_check(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 */
>> - case 0x3:
>> - access |= PAGE_READ;
>> - break;
>> - }
>> - } else {
>> - switch (pp) {
>> - case 0x0:
>> - access = 0;
>> - break;
>> - case 0x1:
>> - case 0x3:
>> - access = PAGE_READ;
>> - break;
>> - case 0x2:
>> - access = PAGE_READ | PAGE_WRITE;
>> - break;
>> - }
>> - }
>> - if (nx == 0) {
>> - access |= PAGE_EXEC;
>> - }
>> -
>> - return access;
>> -}
>> -
>> int ppc6xx_tlb_getnum(CPUPPCState *env, target_ulong eaddr,
>> int way, int is_code)
>> {
>> @@ -137,7 +99,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 */
>> @@ -156,11 +118,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_hash32_pp_prot(ctx->key, pp, ctx->nx);
>> if (check_prot_access_type(ctx->prot, access_type)) {
>> /* Access granted */
>> qemu_log_mask(CPU_LOG_MMU, "PTE access granted !\n");
>
>
>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic