All of
 help / color / mirror / Atom feed
From: Bruno Piazera Larsen <>
To: Richard Henderson <>,
	Greg Kurz <>,,,,,
Subject: Re: [RFC PATCH] target/ppc: fix address translation bug for hash table mmus
Date: Tue, 8 Jun 2021 13:37:51 -0300	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

[-- Attachment #1: Type: text/plain, Size: 2398 bytes --]

On 08/06/2021 12:35, Richard Henderson wrote:
> On 6/8/21 7:39 AM, Bruno Piazera Larsen wrote:
>>> That's odd.  We already have more arguments than the number of 
>>> argument registers...  A 5x slowdown is distinctly odd.
>> I did some more digging and the problem is not with 
>> ppc_radix64_check_prot, the problem is ppc_radix64_xlate, which 
>> currently has 7 arguments and we're increasing to 8. 7 feels like the 
>> correct number, but I couldn't find docs supporting it, so I could be 
>> wrong.
> According to tcg/ppc/, there are 8 argument registers 
> for ppc hosts.  But now I see you didn't actually say on which host 
> you observed the problem...  It's 6 argument registers for x86_64 host.

Oh, yes, sorry. I'm experiencing it in a POWER9 machine (ppc64le 
architecture). According to tcg this shouldn't be the issue, then, so 
idk if that's the real reason or not. All I know is that as soon as gcc 
can't optimize an argument away it happens (fprintf in radix64_xlate, 
using one of the mmuidx_* functions, defining those as macros).

I'll test it in my x86_64 machine and see if such a slowdown happens. 
It's not conclusive evidence, but the function is too complex for me to 
follow the disassembly if I can avoid it...

>> That means we'd have to define radix_ctx_t (or however we call it) in 
>> radix64.h, setup the struct on ppc_xlate, then pass it to 
>> ppc_radix64_xlate.
> Well, if you're going to change the xlate interface, you want to do 
> that across all of them.  So, not call it radix_ctx_t.
I wouldn't change ppc_xlate's interface, I'd set up the struct in that 
function and call ppc_radix64_xlate using the struct
>>  From looking at the code, it seems the most useful bits to put in 
>> the struct are: eaddr, g_addr, h_addr, {h,g}_prot, {g,h}_page_size, 
>> mmu_idx and guest_visible. They all seem reasonable to me, but I 
>> might be missing something again.
> I don't think h/g should be in this struct.  I think h/g should use 
> different struct instances, because they are different accesses.
Ok, makes sense
> r~
Bruno Piazera Larsen
Instituto de Pesquisas ELDORADO 
Departamento Computação Embarcada
Analista de Software Trainee
Aviso Legal - Disclaimer <>

[-- Attachment #2: Type: text/html, Size: 3745 bytes --]

  reply	other threads:[~2021-06-08 16:40 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-02 19:18 [RFC PATCH] target/ppc: fix address translation bug for hash table mmus Bruno Larsen (billionai)
2021-06-02 19:26 ` Richard Henderson
2021-06-02 19:58   ` Bruno Piazera Larsen
2021-06-02 22:19     ` Richard Henderson
2021-06-07 19:29       ` Bruno Piazera Larsen
2021-06-07 21:06         ` Richard Henderson
2021-06-08 14:39           ` Bruno Piazera Larsen
2021-06-08 15:35             ` Richard Henderson
2021-06-08 16:37               ` Bruno Piazera Larsen [this message]
2021-06-08 18:39                 ` Bruno Piazera Larsen

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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \ \ \

* 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.