Patchwork [for-1.4,02/19] target-ppc: Fix target_ulong vs. hwaddr format mismatches

login
register
mail settings
Submitter Andreas Färber
Date Jan. 27, 2013, 1:32 p.m.
Message ID <1359293537-8251-3-git-send-email-afaerber@suse.de>
Download mbox | patch
Permalink /patch/215992/
State New
Headers show

Comments

Andreas Färber - Jan. 27, 2013, 1:32 p.m.
To keep log format backwards compatible, cast to target_ulong
rather than using HWADDR_PRIx.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-ppc/mmu_helper.c |    8 +++++---
 1 Datei geändert, 5 Zeilen hinzugefügt(+), 3 Zeilen entfernt(-)
Alexander Graf - Jan. 30, 2013, 10:15 a.m.
On 27.01.2013, at 14:32, Andreas Färber wrote:

> To keep log format backwards compatible, cast to target_ulong
> rather than using HWADDR_PRIx.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
> target-ppc/mmu_helper.c |    8 +++++---
> 1 Datei geändert, 5 Zeilen hinzugefügt(+), 3 Zeilen entfernt(-)
> 
> diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
> index 0aee7a9..14fa25a 100644
> --- a/target-ppc/mmu_helper.c
> +++ b/target-ppc/mmu_helper.c
> @@ -589,7 +589,8 @@ static inline int find_pte2(CPUPPCState *env, mmu_ctx_t *ctx, int is_64b, int h,
>             r = pte64_check(ctx, pte0, pte1, h, rw, type);
>             LOG_MMU("Load pte from " TARGET_FMT_lx " => " TARGET_FMT_lx " "
>                     TARGET_FMT_lx " %d %d %d " TARGET_FMT_lx "\n",
> -                    pteg_off + (i * 16), pte0, pte1, (int)(pte0 & 1), h,
> +                    (target_ulong)pteg_off + (i * 16), pte0, pte1,
> +                    (int)(pte0 & 1), h,

Please change the format string to reflect the variables' types instead of doing these casts please :). Same for the ones below.


Alex

>                     (int)((pte0 >> 1) & 1), ctx->ptem);
>         } else
> #endif
> @@ -604,7 +605,8 @@ static inline int find_pte2(CPUPPCState *env, mmu_ctx_t *ctx, int is_64b, int h,
>             r = pte32_check(ctx, pte0, pte1, h, rw, type);
>             LOG_MMU("Load pte from " TARGET_FMT_lx " => " TARGET_FMT_lx " "
>                     TARGET_FMT_lx " %d %d %d " TARGET_FMT_lx "\n",
> -                    pteg_off + (i * 8), pte0, pte1, (int)(pte0 >> 31), h,
> +                    (target_ulong)pteg_off + (i * 8), pte0, pte1,
> +                    (int)(pte0 >> 31), h,
>                     (int)((pte0 >> 6) & 1), ctx->ptem);
>         }
>         switch (r) {
> @@ -634,7 +636,7 @@ static inline int find_pte2(CPUPPCState *env, mmu_ctx_t *ctx, int is_64b, int h,
>     if (good != -1) {
>     done:
>         LOG_MMU("found PTE at addr " TARGET_FMT_lx " prot=%01x ret=%d\n",
> -                ctx->raddr, ctx->prot, ret);
> +                (target_ulong)ctx->raddr, ctx->prot, ret);
>         /* Update page flags */
>         pte1 = ctx->raddr;
>         if (pte_update_flags(ctx, &pte1, ret, rw) == 1) {
> -- 
> 1.7.10.4
>
Andreas Färber - Jan. 30, 2013, 10:40 a.m.
Am 30.01.2013 11:15, schrieb Alexander Graf:
> 
> On 27.01.2013, at 14:32, Andreas Färber wrote:
> 
>> To keep log format backwards compatible, cast to target_ulong
>> rather than using HWADDR_PRIx.
>>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>> ---
>> target-ppc/mmu_helper.c |    8 +++++---
>> 1 Datei geändert, 5 Zeilen hinzugefügt(+), 3 Zeilen entfernt(-)
>>
>> diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
>> index 0aee7a9..14fa25a 100644
>> --- a/target-ppc/mmu_helper.c
>> +++ b/target-ppc/mmu_helper.c
>> @@ -589,7 +589,8 @@ static inline int find_pte2(CPUPPCState *env, mmu_ctx_t *ctx, int is_64b, int h,
>>             r = pte64_check(ctx, pte0, pte1, h, rw, type);
>>             LOG_MMU("Load pte from " TARGET_FMT_lx " => " TARGET_FMT_lx " "
>>                     TARGET_FMT_lx " %d %d %d " TARGET_FMT_lx "\n",
>> -                    pteg_off + (i * 16), pte0, pte1, (int)(pte0 & 1), h,
>> +                    (target_ulong)pteg_off + (i * 16), pte0, pte1,
>> +                    (int)(pte0 & 1), h,
> 
> Please change the format string to reflect the variables' types instead of doing these casts please :). Same for the ones below.

Please specify how you would like the format string to look like then.
As indicated above, we only have HWADDR_PRIx, so we must hardcode the
amount of zeroes to use then. With TARGET_FMT_lx it uses %08 for
ppc[emb] and %016 for ppc64. An #ifdef TARGET_PPC64 seemed uglier. ;)

Andreas

>>                     (int)((pte0 >> 1) & 1), ctx->ptem);
>>         } else
>> #endif
>> @@ -604,7 +605,8 @@ static inline int find_pte2(CPUPPCState *env, mmu_ctx_t *ctx, int is_64b, int h,
>>             r = pte32_check(ctx, pte0, pte1, h, rw, type);
>>             LOG_MMU("Load pte from " TARGET_FMT_lx " => " TARGET_FMT_lx " "
>>                     TARGET_FMT_lx " %d %d %d " TARGET_FMT_lx "\n",
>> -                    pteg_off + (i * 8), pte0, pte1, (int)(pte0 >> 31), h,
>> +                    (target_ulong)pteg_off + (i * 8), pte0, pte1,
>> +                    (int)(pte0 >> 31), h,
>>                     (int)((pte0 >> 6) & 1), ctx->ptem);
>>         }
>>         switch (r) {
>> @@ -634,7 +636,7 @@ static inline int find_pte2(CPUPPCState *env, mmu_ctx_t *ctx, int is_64b, int h,
>>     if (good != -1) {
>>     done:
>>         LOG_MMU("found PTE at addr " TARGET_FMT_lx " prot=%01x ret=%d\n",
>> -                ctx->raddr, ctx->prot, ret);
>> +                (target_ulong)ctx->raddr, ctx->prot, ret);
>>         /* Update page flags */
>>         pte1 = ctx->raddr;
>>         if (pte_update_flags(ctx, &pte1, ret, rw) == 1) {
>> -- 
>> 1.7.10.4
Alexander Graf - Jan. 30, 2013, 11:02 a.m.
On 30.01.2013, at 11:40, Andreas Färber wrote:

> Am 30.01.2013 11:15, schrieb Alexander Graf:
>> 
>> On 27.01.2013, at 14:32, Andreas Färber wrote:
>> 
>>> To keep log format backwards compatible, cast to target_ulong
>>> rather than using HWADDR_PRIx.
>>> 
>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>> ---
>>> target-ppc/mmu_helper.c |    8 +++++---
>>> 1 Datei geändert, 5 Zeilen hinzugefügt(+), 3 Zeilen entfernt(-)
>>> 
>>> diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
>>> index 0aee7a9..14fa25a 100644
>>> --- a/target-ppc/mmu_helper.c
>>> +++ b/target-ppc/mmu_helper.c
>>> @@ -589,7 +589,8 @@ static inline int find_pte2(CPUPPCState *env, mmu_ctx_t *ctx, int is_64b, int h,
>>>            r = pte64_check(ctx, pte0, pte1, h, rw, type);
>>>            LOG_MMU("Load pte from " TARGET_FMT_lx " => " TARGET_FMT_lx " "
>>>                    TARGET_FMT_lx " %d %d %d " TARGET_FMT_lx "\n",
>>> -                    pteg_off + (i * 16), pte0, pte1, (int)(pte0 & 1), h,
>>> +                    (target_ulong)pteg_off + (i * 16), pte0, pte1,
>>> +                    (int)(pte0 & 1), h,
>> 
>> Please change the format string to reflect the variables' types instead of doing these casts please :). Same for the ones below.
> 
> Please specify how you would like the format string to look like then.
> As indicated above, we only have HWADDR_PRIx, so we must hardcode the
> amount of zeroes to use then. With TARGET_FMT_lx it uses %08 for
> ppc[emb] and %016 for ppc64. An #ifdef TARGET_PPC64 seemed uglier. ;)

Just make it %08 always. Higher addresses on PPC usually have the highest 4 bits set, so it would still align well.


Alex
Andreas Färber - Jan. 30, 2013, 11:07 a.m.
Am 30.01.2013 12:02, schrieb Alexander Graf:
> 
> On 30.01.2013, at 11:40, Andreas Färber wrote:
> 
>> Am 30.01.2013 11:15, schrieb Alexander Graf:
>>>
>>> On 27.01.2013, at 14:32, Andreas Färber wrote:
>>>
>>>> To keep log format backwards compatible, cast to target_ulong
>>>> rather than using HWADDR_PRIx.
>>>>
>>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>>> ---
>>>> target-ppc/mmu_helper.c |    8 +++++---
>>>> 1 Datei geändert, 5 Zeilen hinzugefügt(+), 3 Zeilen entfernt(-)
>>>>
>>>> diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
>>>> index 0aee7a9..14fa25a 100644
>>>> --- a/target-ppc/mmu_helper.c
>>>> +++ b/target-ppc/mmu_helper.c
>>>> @@ -589,7 +589,8 @@ static inline int find_pte2(CPUPPCState *env, mmu_ctx_t *ctx, int is_64b, int h,
>>>>            r = pte64_check(ctx, pte0, pte1, h, rw, type);
>>>>            LOG_MMU("Load pte from " TARGET_FMT_lx " => " TARGET_FMT_lx " "
>>>>                    TARGET_FMT_lx " %d %d %d " TARGET_FMT_lx "\n",
>>>> -                    pteg_off + (i * 16), pte0, pte1, (int)(pte0 & 1), h,
>>>> +                    (target_ulong)pteg_off + (i * 16), pte0, pte1,
>>>> +                    (int)(pte0 & 1), h,
>>>
>>> Please change the format string to reflect the variables' types instead of doing these casts please :). Same for the ones below.
>>
>> Please specify how you would like the format string to look like then.
>> As indicated above, we only have HWADDR_PRIx, so we must hardcode the
>> amount of zeroes to use then. With TARGET_FMT_lx it uses %08 for
>> ppc[emb] and %016 for ppc64. An #ifdef TARGET_PPC64 seemed uglier. ;)
> 
> Just make it %08 always. Higher addresses on PPC usually have the highest 4 bits set, so it would still align well.

Fine with me.

Andreas

Patch

diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
index 0aee7a9..14fa25a 100644
--- a/target-ppc/mmu_helper.c
+++ b/target-ppc/mmu_helper.c
@@ -589,7 +589,8 @@  static inline int find_pte2(CPUPPCState *env, mmu_ctx_t *ctx, int is_64b, int h,
             r = pte64_check(ctx, pte0, pte1, h, rw, type);
             LOG_MMU("Load pte from " TARGET_FMT_lx " => " TARGET_FMT_lx " "
                     TARGET_FMT_lx " %d %d %d " TARGET_FMT_lx "\n",
-                    pteg_off + (i * 16), pte0, pte1, (int)(pte0 & 1), h,
+                    (target_ulong)pteg_off + (i * 16), pte0, pte1,
+                    (int)(pte0 & 1), h,
                     (int)((pte0 >> 1) & 1), ctx->ptem);
         } else
 #endif
@@ -604,7 +605,8 @@  static inline int find_pte2(CPUPPCState *env, mmu_ctx_t *ctx, int is_64b, int h,
             r = pte32_check(ctx, pte0, pte1, h, rw, type);
             LOG_MMU("Load pte from " TARGET_FMT_lx " => " TARGET_FMT_lx " "
                     TARGET_FMT_lx " %d %d %d " TARGET_FMT_lx "\n",
-                    pteg_off + (i * 8), pte0, pte1, (int)(pte0 >> 31), h,
+                    (target_ulong)pteg_off + (i * 8), pte0, pte1,
+                    (int)(pte0 >> 31), h,
                     (int)((pte0 >> 6) & 1), ctx->ptem);
         }
         switch (r) {
@@ -634,7 +636,7 @@  static inline int find_pte2(CPUPPCState *env, mmu_ctx_t *ctx, int is_64b, int h,
     if (good != -1) {
     done:
         LOG_MMU("found PTE at addr " TARGET_FMT_lx " prot=%01x ret=%d\n",
-                ctx->raddr, ctx->prot, ret);
+                (target_ulong)ctx->raddr, ctx->prot, ret);
         /* Update page flags */
         pte1 = ctx->raddr;
         if (pte_update_flags(ctx, &pte1, ret, rw) == 1) {