tcg/ppc: Improve unaligned load/store handling on 64-bit backend
PATCH v3 1/1 tcg/ppc: Improve unaligned load/store handling on 64-bit backend
Currently, we get to the slow path for any unaligned access in the backend, because we effectively preserve the bottom address bits below the alignment requirement when comparing with the TLB entry, so any non-0 bit there will cause the compare to fail.
For the same number of instructions, we can instead add the access size - 1 to the address and stick to clearing all the bottom bits.
That means that normal unaligned accesses will not fallback (the HW will handle them fine). Only when crossing a page boundary well we end up having a mismatch because we'll end up pointing to the next page which cannot possibly be in that same TLB entry.
---
v3. Add MO_ALIGN support for front-ends requiring aligned accesses v2. This is the correct version of the patch, the one that actually works :-)
tcg/ppc/tcg-target.c | 41 +++++++++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 10 deletions(-)
tcg/ppc/tcg-target.c +31 -10
@@ -1361,7 +1361,7 @@ static void * const qemu_st_helpers[16] = {in CR7, loads the addend of the TLB into R3, and returns the register containing the guest address (zero-extended into R4). Clobbers R0 and R2. */ -static TCGReg tcg_out_tlb_read (TCGContext *s,TCGMemOp s_bits, +static TCGReg tcg_out_tlb_read (TCGContext *s,TCGMemOp opc,TCGReg addrlo,TCGReg addrhi,int mem_index,bool is_read) {
@@ -1371,6 +1371,7 @@ static TCGReg tcg_out_tlb_read(TCGContext *s, TCGMemOp s_bits, : offsetof(CPUArchState, tlb_table[mem_index][0].addr_write)); int add_off = offsetof(CPUArchState, tlb_table[mem_index][0].addend);TCGReg base =TCG_AREG0 ; +TCGMemOp s_bits = opc &MO_SIZE ;/* Extract the page index, shifted into place for tlb index. */ if (TCG_TARGET_REG_BITS ==64 ) {
@@ -1422,17 +1423,37 @@ static TCGReg tcg_out_tlb_read(TCGContext *s, TCGMemOp s_bits,to minimize any load use delay. */ tcg_out_ld (s,TCG_TYPE_PTR ,TCG_REG_R3 ,TCG_REG_R3 , add_off); -/* Clear the non-page, non-alignment bits from the address. */ +/* Clear the non-page, non-alignment bits from the address */ if (TCG_TARGET_REG_BITS ==32 ||TARGET_LONG_BITS ==32 ) { +/* We don't support unaligned accesses on 32-bits, preserve +* the bottom bits and thus trigger a comparison failure on +* unaligned accesses +*/ tcg_out_rlw (s,RLWINM ,TCG_REG_R0 , addrlo,0 , (32 - s_bits) &31 ,31 -TARGET_PAGE_BITS ); - }else if (!s_bits) { -tcg_out_rld (s,RLDICR ,TCG_REG_R0 , addrlo, -0 ,63 -TARGET_PAGE_BITS ); + }else if (s_bits) { +/* > byte access, we need to handle alignment */ +if ((opc &MO_AMASK ) ==MO_ALIGN ) { +/* Alignment required by the front-end, same as 32-bits */ +tcg_out_rld (s,RLDICL ,TCG_REG_R0 , addrlo, +64 -TARGET_PAGE_BITS ,TARGET_PAGE_BITS - s_bits); +tcg_out_rld (s,RLDICL ,TCG_REG_R0 ,TCG_REG_R0 ,TARGET_PAGE_BITS ,0 ); + }else { +/* We support unaligned accesses, we need to make sure we fail +* if we cross a page boundary. The trick is to add the +* access_size-1 to the address before masking the low bits. +* That will make the address overflow to the next page if we +* cross a page boundary which will then force a mismatch of +* the TLB compare since the next page cannot possibly be in +* the same TLB index. +*/ +tcg_out32 (s,ADDI |TAI (TCG_REG_R0 , addrlo, (1 << s_bits) -1 )); +tcg_out_rld (s,RLDICR ,TCG_REG_R0 ,TCG_REG_R0 , +0 ,63 -TARGET_PAGE_BITS ); + } }else { -tcg_out_rld (s,RLDICL ,TCG_REG_R0 , addrlo, -64 -TARGET_PAGE_BITS ,TARGET_PAGE_BITS - s_bits); -tcg_out_rld (s,RLDICL ,TCG_REG_R0 ,TCG_REG_R0 ,TARGET_PAGE_BITS ,0 ); +/* Byte access, just chop off the bits below the page index */ +tcg_out_rld (s,RLDICR ,TCG_REG_R0 , addrlo,0 ,63 -TARGET_PAGE_BITS ); }if (TCG_TARGET_REG_BITS <TARGET_LONG_BITS ) {
@@ -1592,7 +1613,7 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args, bool is_64)#ifdef CONFIG_SOFTMMU mem_index =get_mmuidx (oi); - addrlo =tcg_out_tlb_read (s, s_bits, addrlo, addrhi, mem_index, true); + addrlo =tcg_out_tlb_read (s, opc, addrlo, addrhi, mem_index, true);/* Load a pointer into the current opcode w/conditional branch-link. */ label_ptr = s->code_ptr;
@@ -1667,7 +1688,7 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args, bool is_64)#ifdef CONFIG_SOFTMMU mem_index =get_mmuidx (oi); - addrlo =tcg_out_tlb_read (s, s_bits, addrlo, addrhi, mem_index, false); + addrlo =tcg_out_tlb_read (s, opc, addrlo, addrhi, mem_index, false);/* Load a pointer into the current opcode w/conditional branch-link. */ label_ptr = s->code_ptr;
tcg/ppc/tcg-target.c static TCGReg tcg_out_tlb_read(TCGContext *s, TCGMemOp s_bits, · line 1433
Quoted 8 lines · Benjamin Herrenschmidt
tcg/ppc/tcg-target.c +5
-0+/* Clear the non-page, non-alignment bits from the address */ if (TCG_TARGET_REG_BITS ==32 ||TARGET_LONG_BITS ==32 ) { +/* We don't support unaligned accesses on 32-bits, preserve +* the bottom bits and thus trigger a comparison failure on +* unaligned accesses +*/ tcg_out_rlw (s,RLWINM ,TCG_REG_R0 , addrlo,0 , (32 - s_bits) &31 ,31 -TARGET_PAGE_BITS );
Why don't you support this unaligned acess with 32-bit guests?
Quoted 29 lines
tcg/ppc/tcg-target.c +22
-6- }else if (!s_bits) { -tcg_out_rld (s,RLDICR ,TCG_REG_R0 , addrlo, -0 ,63 -TARGET_PAGE_BITS ); + }else if (s_bits) { +/* > byte access, we need to handle alignment */ +if ((opc &MO_AMASK ) ==MO_ALIGN ) { +/* Alignment required by the front-end, same as 32-bits */ +tcg_out_rld (s,RLDICL ,TCG_REG_R0 , addrlo, +64 -TARGET_PAGE_BITS ,TARGET_PAGE_BITS - s_bits); +tcg_out_rld (s,RLDICL ,TCG_REG_R0 ,TCG_REG_R0 ,TARGET_PAGE_BITS ,0 ); + }else { +/* We support unaligned accesses, we need to make sure we fail +* if we cross a page boundary. The trick is to add the +* access_size-1 to the address before masking the low bits. +* That will make the address overflow to the next page if we +* cross a page boundary which will then force a mismatch of +* the TLB compare since the next page cannot possibly be in +* the same TLB index. +*/ +tcg_out32 (s,ADDI |TAI (TCG_REG_R0 , addrlo, (1 << s_bits) -1 )); +tcg_out_rld (s,RLDICR ,TCG_REG_R0 ,TCG_REG_R0 , +0 ,63 -TARGET_PAGE_BITS ); + } }else { -tcg_out_rld (s,RLDICL ,TCG_REG_R0 , addrlo, -64 -TARGET_PAGE_BITS ,TARGET_PAGE_BITS - s_bits); -tcg_out_rld (s,RLDICL ,TCG_REG_R0 ,TCG_REG_R0 ,TARGET_PAGE_BITS ,0 ); +/* Byte access, just chop off the bits below the page index */ +tcg_out_rld (s,RLDICR ,TCG_REG_R0 , addrlo,0 ,63 -TARGET_PAGE_BITS );
Quoted 11 lines · Richard Henderson
tcg/ppc/tcg-target.c static TCGReg tcg_out_tlb_read(TCGContext *s, TCGMemOp s_bits, · line 1433
Quoted 8 lines · Benjamin Herrenschmidt
tcg/ppc/tcg-target.c +5
-0+/* Clear the non-page, non-alignment bits from the address */ if (TCG_TARGET_REG_BITS ==32 ||TARGET_LONG_BITS ==32 ) { +/* We don't support unaligned accesses on 32-bits, preserve +* the bottom bits and thus trigger a comparison failure on +* unaligned accesses +*/ tcg_out_rlw (s,RLWINM ,TCG_REG_R0 , addrlo,0 , (32 - s_bits) &31 ,31 -TARGET_PAGE_BITS );Why don't you support this unaligned acess with 32-bit guests?
No reason, I just didn't get to do it yet. It's possible, I was just lazy :-) It also adds one instruction. On 64-bit we always have 2 instructions anyway so it wasn't adding any overhead really, on 32-bit we get away with a single rlwinm, while adding the unaligned support would make it an addi + rlwinm.
Quoted 29 lines
Quoted 29 lines
tcg/ppc/tcg-target.c +22
-6- }else if (!s_bits) { -tcg_out_rld (s,RLDICR ,TCG_REG_R0 , addrlo, -0 ,63 -TARGET_PAGE_BITS ); + }else if (s_bits) { +/* > byte access, we need to handle alignment */ +if ((opc &MO_AMASK ) ==MO_ALIGN ) { +/* Alignment required by the front-end, same as 32-bits */ +tcg_out_rld (s,RLDICL ,TCG_REG_R0 , addrlo, +64 -TARGET_PAGE_BITS ,TARGET_PAGE_BITS - s_bits); +tcg_out_rld (s,RLDICL ,TCG_REG_R0 ,TCG_REG_R0 ,TARGET_PAGE_BITS ,0 ); + }else { +/* We support unaligned accesses, we need to make sure we fail +* if we cross a page boundary. The trick is to add the +* access_size-1 to the address before masking the low bits. +* That will make the address overflow to the next page if we +* cross a page boundary which will then force a mismatch of +* the TLB compare since the next page cannot possibly be in +* the same TLB index. +*/ +tcg_out32 (s,ADDI |TAI (TCG_REG_R0 , addrlo, (1 << s_bits) -1 )); +tcg_out_rld (s,RLDICR ,TCG_REG_R0 ,TCG_REG_R0 , +0 ,63 -TARGET_PAGE_BITS ); + } }else { -tcg_out_rld (s,RLDICL ,TCG_REG_R0 , addrlo, -64 -TARGET_PAGE_BITS ,TARGET_PAGE_BITS - s_bits); -tcg_out_rld (s,RLDICL ,TCG_REG_R0 ,TCG_REG_R0 ,TARGET_PAGE_BITS ,0 ); +/* Byte access, just chop off the bits below the page index */ +tcg_out_rld (s,RLDICR ,TCG_REG_R0 , addrlo,0 ,63 -TARGET_PAGE_BITS );
Richard Henderson
Reviewed-byQuoted 11 lines · Benjamin Herrenschmidt
Quoted 10 lines · Richard Henderson
Quoted 8 lines · Benjamin Herrenschmidt
tcg/ppc/tcg-target.c +5
-0+/* Clear the non-page, non-alignment bits from the address */ if (TCG_TARGET_REG_BITS ==32 ||TARGET_LONG_BITS ==32 ) { +/* We don't support unaligned accesses on 32-bits, preserve +* the bottom bits and thus trigger a comparison failure on +* unaligned accesses +*/ tcg_out_rlw (s,RLWINM ,TCG_REG_R0 , addrlo,0 , (32 - s_bits) &31 ,31 -TARGET_PAGE_BITS );
Quoted 7 lines
Quoted 1 line
Why don't you support this unaligned acess with 32-bit guests?
No reason, I just didn't get to do it yet. It's possible, I was just lazy :-) It also adds one instruction. On 64-bit we always have 2 instructions anyway so it wasn't adding any overhead really, on 32-bit we get away with a single rlwinm, while adding the unaligned support would make it an addi + rlwinm.
Ah, ok. I wondered if some older 32-bit host ppc didn't allow it, and the 32-bit guest paid the price. Anyway,
r~
Quoted 14 lines · Richard Henderson
Quoted 13 lines · Benjamin Herrenschmidt
Quoted 11 lines · Richard Henderson
tcg/ppc/tcg-target.c static TCGReg tcg_out_tlb_read(TCGContext *s, TCGMemOp s_bits, · line 1433
Quoted 8 lines · Benjamin Herrenschmidt
tcg/ppc/tcg-target.c +5
-0+/* Clear the non-page, non-alignment bits from the address */ if (TCG_TARGET_REG_BITS ==32 ||TARGET_LONG_BITS ==32 ) { +/* We don't support unaligned accesses on 32-bits, preserve +* the bottom bits and thus trigger a comparison failure on +* unaligned accesses +*/ tcg_out_rlw (s,RLWINM ,TCG_REG_R0 , addrlo,0 , (32 - s_bits) &31 ,31 -TARGET_PAGE_BITS );Why don't you support this unaligned acess with 32-bit guests?
Quoted 8 lines
Quoted 5 lines
No reason, I just didn't get to do it yet. It's possible, I was just lazy :-) It also adds one instruction. On 64-bit we always have 2 instructions anyway so it wasn't adding any overhead really, on 32-bit we get away with a single rlwinm, while adding the unaligned support would make it an addi + rlwinm.
Ah, ok. I wondered if some older 32-bit host ppc didn't allow it, and the 32-bit guest paid the price. Anyway,
No, most implementations support unaligned accesses in common cases. I think the worst embedded variants might trap when crossing page boundaries but that's about it. I don't think we bother emulating this though.
Quoted 1 line
Reviewed-by Richard Henderson <[email protected]>
Thanks.
Cheers,
Ben.
Quoted 2 lines
r~
Quoted 20 lines · Benjamin Herrenschmidt
Quoted 11 lines · Richard Henderson
Quoted 10 lines · Benjamin Herrenschmidt
Quoted 9 lines · Richard Henderson
Quoted 8 lines · Benjamin Herrenschmidt
tcg/ppc/tcg-target.c +5
-0+/* Clear the non-page, non-alignment bits from the address */ if (TCG_TARGET_REG_BITS ==32 ||TARGET_LONG_BITS ==32 ) { +/* We don't support unaligned accesses on 32-bits, preserve +* the bottom bits and thus trigger a comparison failure on +* unaligned accesses +*/ tcg_out_rlw (s,RLWINM ,TCG_REG_R0 , addrlo,0 , (32 - s_bits) &31 ,31 -TARGET_PAGE_BITS );Quoted 8 lines
Quoted 6 lines
Quoted 1 line
Why don't you support this unaligned acess with 32-bit guests?
No reason, I just didn't get to do it yet. It's possible, I was just lazy :-) It also adds one instruction. On 64-bit we always have 2 instructions anyway so it wasn't adding any overhead really, on 32-bit we get away with a single rlwinm, while adding the unaligned support would make it an addi + rlwinm.
Ah, ok. I wondered if some older 32-bit host ppc didn't allow it, and the 32-bit guest paid the price. Anyway,
Quoted 7 lines
No, most implementations support unaligned accesses in common cases. I think the worst embedded variants might trap when crossing page boundaries but that's about it. I don't think we bother emulating this though.
Quoted 1 line
Reviewed-by Richard Henderson <[email protected]>Thanks.
So who can pick those up? PPC TCG is pretty much unmaintained since Malc disappeared, no?
Alex
Quoted 5 lines · Alexander Graf
Quoted 2 lines
Quoted 1 line
Reviewed-by Richard Henderson <[email protected]>Thanks.
So who can pick those up? PPC TCG is pretty much unmaintained since Malc disappeared, no?
Did you just raise your hand ? :-)
Worst case I'm happy to just keep a branch of my things that have had external reviews and directly get "whoever's the boss" pull from that (Paolo ?) but I'm trying to not take new maintainer responsibilities lately, too busy bossing people around :-)
Cheers,
Ben.
Quoted 7 lines · Benjamin Herrenschmidt
Quoted 2 lines
Quoted 2 lines
So who can pick those up? PPC TCG is pretty much unmaintained since Malc disappeared, no?
Did you just raise your hand ? :-)
Worst case I'm happy to just keep a branch of my things that have had external reviews and directly get "whoever's the boss" pull from that (Paolo ?)
The boss would be either Peter or Alex, certainly not me. :)
Paolo
Quoted 10 lines · Paolo Bonzini
Quoted 7 lines · Benjamin Herrenschmidt
Quoted 2 lines
Quoted 2 lines
So who can pick those up? PPC TCG is pretty much unmaintained since Malc disappeared, no?
Did you just raise your hand ? :-)
Worst case I'm happy to just keep a branch of my things that have had external reviews and directly get "whoever's the boss" pull from that (Paolo ?)
The boss would be either Peter or Alex, certainly not me. :)
I can happily take on this after my paternity leave, but considering that I'm out for 3 months in 1 month from now and have a pretty huge pile of stuff that needs to get done until then I'd rather keep things low right now ;)
Ben. how about you just push them directly to Peter with Reviewed-bys from Richard in a pull request? Chances that there are patches from someone who's not you for ppc tcg are pretty low atm. And for those we can always improvise when the time comes.
Alex
Quoted 11 lines · Alexander Graf
I can happily take on this after my paternity leave, but considering that I'm out for 3 months in 1 month from now and have a pretty huge pile of stuff that needs to get done until then I'd rather keep things
low right now ;)Ben. how about you just push them directly to Peter with Reviewed-bys from Richard in a pull request? Chances that there are patches from someone who's not you for ppc tcg are pretty low atm. And for those we
can always improvise when the time comes.
Ok, let's see if I get reviews for the other stuff first (the split I/D mmu index and flush improvements) and I'll put something together.
Cheers
Ben.
Quoted 11 lines · Paolo Bonzini
Quoted 7 lines · Benjamin Herrenschmidt
Quoted 2 lines
Quoted 2 lines
So who can pick those up? PPC TCG is pretty much unmaintained since Malc disappeared, no?
Did you just raise your hand ? :-)
Worst case I'm happy to just keep a branch of my things that have had external reviews and directly get "whoever's the boss" pull from that (Paolo ?)
The boss would be either Peter or Alex, certainly not me. :)
I didn't know who replaced Anthony :-)
Cheers,
Ben.
Quoted 26 lines · Alexander Graf
Quoted 25 lines · Benjamin Herrenschmidt
Quoted 11 lines · Richard Henderson
Quoted 10 lines · Benjamin Herrenschmidt
Quoted 9 lines · Richard Henderson
Quoted 8 lines · Benjamin Herrenschmidt
tcg/ppc/tcg-target.c +5
-0+/* Clear the non-page, non-alignment bits from the address */ if (TCG_TARGET_REG_BITS ==32 ||TARGET_LONG_BITS ==32 ) { +/* We don't support unaligned accesses on 32-bits, preserve +* the bottom bits and thus trigger a comparison failure on +* unaligned accesses +*/ tcg_out_rlw (s,RLWINM ,TCG_REG_R0 , addrlo,0 , (32 - s_bits) &31 ,31 -TARGET_PAGE_BITS );Quoted 8 lines
Quoted 6 lines
Quoted 1 line
Why don't you support this unaligned acess with 32-bit guests?
No reason, I just didn't get to do it yet. It's possible, I was just lazy :-) It also adds one instruction. On 64-bit we always have 2 instructions anyway so it wasn't adding any overhead really, on 32-bit we get away with a single rlwinm, while adding the unaligned support would make it an addi + rlwinm.
Ah, ok. I wondered if some older 32-bit host ppc didn't allow it, and the 32-bit guest paid the price. Anyway,
No, most implementations support unaligned accesses in common cases. I think the worst embedded variants might trap when crossing page boundaries but that's about it. I don't think we bother emulating this though.
Quoted 6 lines
Quoted 2 lines
Quoted 1 line
Reviewed-by Richard Henderson <[email protected]>Thanks.
So who can pick those up? PPC TCG is pretty much unmaintained since Malc disappeared, no?
Richard has reviewed this patch, and there are already generic TCG patches for 2.5 on the list. I guess he can squeeze a few PPC specific commits like this one in the pull request.
Signature · 2 lines
Aurelien Jarno GPG: 4096R/1DDD8C9B [email protected] http://www.aurel32.net
Quoted 8 lines · Benjamin Herrenschmidt
Currently, we get to the slow path for any unaligned access in the backend, because we effectively preserve the bottom address bits below the alignment requirement when comparing with the TLB entry, so any non-0 bit there will cause the compare to fail.
For the same number of instructions, we can instead add the access size - 1 to the address and stick to clearing all the bottom bits.
Quoted 6 lines
That means that normal unaligned accesses will not fallback (the HW will handle them fine). Only when crossing a page boundary well we end up having a mismatch because we'll end up pointing to the next page which cannot possibly be in that same TLB entry.
Signed-off-by Benjamin Herrenschmidt <[email protected]>
Signature · 2 lines
Aurelien Jarno GPG: 4096R/1DDD8C9B [email protected] http://www.aurel32.net
Richard Henderson
AppliedQuoted 14 lines · Benjamin Herrenschmidt
Currently, we get to the slow path for any unaligned access in the backend, because we effectively preserve the bottom address bits below the alignment requirement when comparing with the TLB entry, so any non-0 bit there will cause the compare to fail.
For the same number of instructions, we can instead add the access size - 1 to the address and stick to clearing all the bottom bits.
That means that normal unaligned accesses will not fallback (the HW will handle them fine). Only when crossing a page boundary well we end up having a mismatch because we'll end up pointing to the next page which cannot possibly be in that same TLB entry.
Signed-off-by Benjamin Herrenschmidt <[email protected]>
Quoted 8 lines
---
v3. Add
MO_ALIGNsupport for front-ends requiring aligned accesses v2. This is the correct version of the patch, the one that actually works :-)tcg/ppc/tcg-target.c | 41 +++++++++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 10 deletions(-)
Applied to tcg-next.
r~