PPC: Fix large page support in TCG
PATCH 1/1 PPC: Fix large page support in TCG
Fix large page support in TCG. The old code would overwrite the large page table entry with the fake 4 KB
one generated here whenever the ref/change bits were updated, causing it to point to the wrong area of memory. Instead of creating a fake PTE, just update the real address at the end.
--- target-ppc/helper.c | 11 +++++------ 1 files changed, 5 insertions(+), 6 deletions(-)
target-ppc/helper.c +0 -0
@@ -597,12 +597,6 @@ static inline int _find_pte(CPUState *env, mmu_ctx_t *ctx,
int is_64b, int h,
pte1 = ldq_phys(env->htab_base + pteg_off + (i * 16) + 8);
}- /* We have a TLB that saves 4K pages, so let's
- * split a huge page to 4k chunks */
- if (target_page_bits != TARGET_PAGE_BITS)
- pte1 |= (ctx->eaddr & (( 1 << target_page_bits ) - 1))
- & TARGET_PAGE_MASK;
-
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",
@@ -678,6 +672,11 @@ static inline int _find_pte(CPUState *env, mmu_ctx_t *ctx,
int is_64b, int h,
}
}+ /* We have a TLB that saves 4K pages, so let's
+ * split a huge page to 4k chunks */
+ if (target_page_bits != TARGET_PAGE_BITS)
+ ctx->raddr |= (ctx->eaddr & (( 1 << target_page_bits ) - 1))
+ & TARGET_PAGE_MASK;
return ret;}
--
1.7.9
Am 03.03.2012 17:39, schrieb Nathan Whitehorn:
Quoted 7 lines
Fix large page support in TCG. The old code would overwrite the large page table entry with the fake 4 KB
one generated here whenever the ref/change bits were updated, causing it to point to the wrong area of memory. Instead of creating a fake PTE, just update the real address at the end.Signed-off-by Nathan Whitehorn <[email protected]>
cc'ing Alex and qemu-ppc.
/-F
Quoted 40 lines
--- target-ppc/helper.c | 11 +++++------ 1 files changed, 5 insertions(+), 6 deletions(-)target-ppc/helper.c +0
-6@@ -597,12 +597,6 @@ static inline int _find_pte(CPUState *env,mmu_ctx_t *ctx, int is_64b, int h, pte1 =ldq_phys (env->htab_base + pteg_off + (i *16 ) +8 ); } -/* We have a TLB that saves 4K pages, so let's -* split a huge page to 4k chunks */ -if (target_page_bits !=TARGET_PAGE_BITS ) - pte1 |= (ctx->eaddr & ((1 << target_page_bits ) -1 )) - &TARGET_PAGE_MASK ; - 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",target-ppc/helper.c +5
-0@@ -678,6 +672,11 @@ static inline int _find_pte(CPUState *env,mmu_ctx_t *ctx, int is_64b, int h, } } + /* We have a TLB that saves 4K pages, so let's + * split a huge page to 4k chunks */ + if (target_page_bits != TARGET_PAGE_BITS) + ctx->raddr |= (ctx->eaddr & (( 1 << target_page_bits ) - 1)) + & TARGET_PAGE_MASK;return ret;}
Signature · 1 line
1.7.9
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Quoted 9 lines · Andreas Färber
Am 03.03.2012 17:39, schrieb Nathan Whitehorn:
Quoted 7 lines
Fix large page support in TCG. The old code would overwrite the large page table entry with the fake 4 KB
one generated here whenever the ref/change bits were updated, causing it to point to the wrong area of memory. Instead of creating a fake PTE, just update the real address at the end.Signed-off-by Nathan Whitehorn<[email protected]>cc'ing Alex and qemu-ppc.
David? Could you please ack?
Alex
Quoted 42 lines
/-F
Quoted 40 lines
--- target-ppc/helper.c | 11 +++++------ 1 files changed, 5 insertions(+), 6 deletions(-)target-ppc/helper.c +0
-6@@ -597,12 +597,6 @@ static inline int _find_pte(CPUState *env,mmu_ctx_t *ctx, int is_64b, int h, pte1 =ldq_phys (env->htab_base + pteg_off + (i *16 ) +8 ); } -/* We have a TLB that saves 4K pages, so let's -* split a huge page to 4k chunks */ -if (target_page_bits !=TARGET_PAGE_BITS ) - pte1 |= (ctx->eaddr& ((1 << target_page_bits ) -1 )) -&TARGET_PAGE_MASK ; - 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",target-ppc/helper.c +5
-0@@ -678,6 +672,11 @@ static inline int _find_pte(CPUState *env,mmu_ctx_t *ctx, int is_64b, int h, } } + /* We have a TLB that saves 4K pages, so let's + * split a huge page to 4k chunks */ + if (target_page_bits != TARGET_PAGE_BITS) + ctx->raddr |= (ctx->eaddr& (( 1<< target_page_bits ) - 1)) +& TARGET_PAGE_MASK;return ret;}Signature · 1 line
1.7.9
Quoted 7 lines · Nathan Whitehorn
Fix large page support in TCG. The old code would overwrite the large page table entry with the fake 4 KB
one generated here whenever the ref/change bits were updated, causing it to point to the wrong area of memory. Instead of creating a fake PTE, just update the real address at the end.Signed-off-by Nathan Whitehorn <[email protected]>
Hrm. This looks like a cleaner way of handling things, but I don't really follow what exactly was going wrong in the old way. Can you spell out in more detail where the modified pte1 value caused problems?
Quoted 38 lines
--- target-ppc/helper.c | 11 +++++------ 1 files changed, 5 insertions(+), 6 deletions(-)target-ppc/helper.c +0
-6@@ -597,12 +597,6 @@ static inline int _find_pte(CPUState *env,mmu_ctx_t *ctx, int is_64b, int h, pte1 =ldq_phys (env->htab_base + pteg_off + (i *16 ) +8 ); } -/* We have a TLB that saves 4K pages, so let's -* split a huge page to 4k chunks */ -if (target_page_bits !=TARGET_PAGE_BITS ) - pte1 |= (ctx->eaddr & ((1 << target_page_bits ) -1 )) - &TARGET_PAGE_MASK ; - 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",target-ppc/helper.c +5
-0@@ -678,6 +672,11 @@ static inline int _find_pte(CPUState *env,mmu_ctx_t *ctx, int is_64b, int h, } } + /* We have a TLB that saves 4K pages, so let's + * split a huge page to 4k chunks */ + if (target_page_bits != TARGET_PAGE_BITS) + ctx->raddr |= (ctx->eaddr & (( 1 << target_page_bits ) - 1)) + & TARGET_PAGE_MASK;return ret;}
Signature · 4 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Quoted 7 lines · David Gibson
Quoted 6 lines · Nathan Whitehorn
Fix large page support in TCG. The old code would overwrite the large page table entry with the fake 4 KB
one generated here whenever the ref/change bits were updated, causing it to point to the wrong area of memory. Instead of creating a fake PTE, just update the real address at the end.
Quoted 6 lines
Quoted 1 line
Signed-off-by Nathan Whitehorn <[email protected]>Hrm. This looks like a cleaner way of handling things, but I don't really follow what exactly was going wrong in the old way. Can you spell out in more detail where the modified pte1 value caused problems?
The problem was that pte1 would get extra bits added into it in _find_pte() to produce a new, fake 4KB page table entry. When the ref/change bits were updated, pte1 would be written back to the page table -- *including* the bits added to make a fake 4K page. At the next access, since this function does not clear the low bits of large pages (which is probably itself a bug) when it interprets them, the generated address would be the large page base, ored with the large page remainder for this access, ored with the large page remainder from the *previous* access, etc. and you would get a progressively more bogus address in the end. -Nathan
Quoted 47 lines
Quoted 41 lines
---
target-ppc/helper.c| 11 +++++------1 files changed, 5 insertions(+), 6 deletions(-)target-ppc/helper.c +0
-4@@ -597,12 +597,6 @@ static inline int _find_pte(CPUState *env,mmu_ctx_t *ctx, int is_64b, int h, pte1 =ldq_phys (env->htab_base + pteg_off + (i *16 ) +8 ); } -/* We have a TLB that saves 4K pages, so let's -* split a huge page to 4k chunks */ -if (target_page_bits !=TARGET_PAGE_BITS ) - pte1 |= (ctx->eaddr & (( 1 << target_page_bits ) -1))
- &TARGET_PAGE_MASK;
-
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",target-ppc/helper.c +5
-0@@ -678,6 +672,11 @@ static inline int _find_pte(CPUState *env,mmu_ctx_t *ctx, int is_64b, int h, } } + /* We have a TLB that saves 4K pages, so let's + * split a huge page to 4k chunks */ + if (target_page_bits != TARGET_PAGE_BITS) + ctx->raddr |= (ctx->eaddr & (( 1 << target_page_bits ) - 1)) + & TARGET_PAGE_MASK;return ret;}
Signature · 4 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
David Gibson
Acked-byQuoted 17 lines · Nathan Whitehorn
Quoted 7 lines · David Gibson
Quoted 6 lines · Nathan Whitehorn
Fix large page support in TCG. The old code would overwrite the large page table entry with the fake 4 KB
one generated here whenever the ref/change bits were updated, causing it to point to the wrong area of memory. Instead of creating a fake PTE, just update the real address at the end.Quoted 6 lines
Quoted 1 line
Signed-off-by Nathan Whitehorn <[email protected]>Hrm. This looks like a cleaner way of handling things, but I don't really follow what exactly was going wrong in the old way. Can you spell out in more detail where the modified pte1 value caused problems?
Quoted 10 lines
The problem was that pte1 would get extra bits added into it in
_find_pte()to produce a new, fake 4KB page table entry. When the ref/change bits were updated, pte1 would be written back to the page table -- *including* the bits added to make a fake 4K page. At the next access, since this function does not clear the low bits of large pages (which is probably itself a bug) when it interprets them, the generated address would be the large page base, ored with the large page remainder for this access, ored with the large page remainder from the *previous* access, etc. and you would get a progressively more bogus address in the end.
Ah, yes, I see it now. Good catch.
Signature · 4 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Quoted 29 lines · David Gibson
Quoted 17 lines · Nathan Whitehorn
Quoted 7 lines · David Gibson
Quoted 6 lines · Nathan Whitehorn
Fix large page support in TCG. The old code would overwrite the large page table entry with the fake 4 KB
one generated here whenever the ref/change bits were updated, causing it to point to the wrong area of memory. Instead of creating a fake PTE, just update the real address at the end.Quoted 6 lines
Quoted 1 line
Signed-off-by Nathan Whitehorn <[email protected]>Hrm. This looks like a cleaner way of handling things, but I don't really follow what exactly was going wrong in the old way. Can you spell out in more detail where the modified pte1 value caused problems?
Quoted 10 lines
The problem was that pte1 would get extra bits added into it in
_find_pte()to produce a new, fake 4KB page table entry. When the ref/change bits were updated, pte1 would be written back to the page table -- *including* the bits added to make a fake 4K page. At the next access, since this function does not clear the low bits of large pages (which is probably itself a bug) when it interprets them, the generated address would be the large page base, ored with the large page remainder for this access, ored with the large page remainder from the *previous* access, etc. and you would get a progressively more bogus address in the end.
Quoted 3 lines
Ah, yes, I see it now. Good catch.
Acked-by David Gibson <[email protected]>
Hrm - the patch doesn't apply for me. Could you please resend as something that's applyable? :) Also, please make sure to always CC qemu-ppc on ppc patches, otherwise there's a good chance they slip off my radar.
Alex
Quoted 27 lines · Alexander Graf
Quoted 25 lines · David Gibson
Quoted 24 lines · Nathan Whitehorn
Quoted 7 lines · David Gibson
Quoted 6 lines · Nathan Whitehorn
Fix large page support in TCG. The old code would overwrite the large page table entry with the fake 4 KB
one generated here whenever the ref/change bits were updated, causing it to point to the wrong area of memory. Instead of creating a fake PTE, just update the real address at the end.Quoted 5 lines
Quoted 1 line
Signed-off-by Nathan Whitehorn<[email protected]>Hrm. This looks like a cleaner way of handling things, but I don't really follow what exactly was going wrong in the old way. Can you spell out in more detail where the modified pte1 value caused problems?
The problem was that pte1 would get extra bits added into it in
_find_pte()to produce a new, fake 4KB page table entry. When the ref/change bits were updated, pte1 would be written back to the page table -- *including* the bits added to make a fake 4K page. At the next access, since this function does not clear the low bits of large pages (which is probably itself a bug) when it interprets them, the generated address would be the large page base, ored with the large page remainder for this access, ored with the large page remainder from the *previous* access, etc. and you would get a progressively more bogus address in the end.
Quoted 9 lines
Quoted 3 lines
Ah, yes, I see it now. Good catch.
Acked-by David Gibson<[email protected]>Hrm - the patch doesn't apply for me. Could you please resend as something that's applyable? :) Also, please make sure to always CC qemu-ppc on ppc patches, otherwise there's a good chance they slip off my radar.
Alex
Weird. I've provided it as an attachment, which should hopefully work this time. -Nathan
Fix large page support in TCG. The old code would overwrite the large page table entry with the fake 4 KB one generated here whenever the ref/change bits were updated, causing it to point to the wrong area of memory.
--- target-ppc/helper.c | 11 +++++------ 1 files changed, 5 insertions(+), 6 deletions(-)
target-ppc/helper.c +5 -6
@@ -597,12 +597,6 @@ static inline int _find_pte(CPUState *env, mmu_ctx_t *ctx, int is_64b, int h, pte1 =ldq_phys (env->htab_base + pteg_off + (i *16 ) +8 ); } -/* We have a TLB that saves 4K pages, so let's -* split a huge page to 4k chunks */ -if (target_page_bits !=TARGET_PAGE_BITS ) - pte1 |= (ctx->eaddr & ((1 << target_page_bits ) -1 )) - &TARGET_PAGE_MASK ; - 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",
@@ -678,6 +672,11 @@ static inline int _find_pte(CPUState *env, mmu_ctx_t *ctx, int is_64b, int h, } } +/* We have a TLB that saves 4K pages, so let's +* split a huge page to 4k chunks */ +if (target_page_bits !=TARGET_PAGE_BITS ) + ctx->raddr |= (ctx->eaddr & ((1 << target_page_bits ) -1 )) + &TARGET_PAGE_MASK ;return ret; }
Signature · 1 line
1.7.9
Quoted 35 lines · Nathan Whitehorn
Quoted 34 lines · Alexander Graf
Quoted 25 lines · David Gibson
Quoted 14 lines · Nathan Whitehorn
Quoted 7 lines · David Gibson
Quoted 6 lines · Nathan Whitehorn
Fix large page support in TCG. The old code would overwrite the large page table entry with the fake 4 KB
one generated here whenever the ref/change bits were updated, causing it to point to the wrong area of memory. Instead of creating a fake PTE, just update the real address at the end.Quoted 5 lines
Quoted 1 line
Signed-off-by Nathan Whitehorn<[email protected]>Hrm. This looks like a cleaner way of handling things, but I don't really follow what exactly was going wrong in the old way. Can you spell out in more detail where the modified pte1 value caused problems?
Quoted 10 lines
The problem was that pte1 would get extra bits added into it in
_find_pte()to produce a new, fake 4KB page table entry. When the ref/change bits were updated, pte1 would be written back to the page table -- *including* the bits added to make a fake 4K page. At the next access, since this function does not clear the low bits of large pages (which is probably itself a bug) when it interprets them, the generated address would be the large page base, ored with the large page remainder for this access, ored with the large page remainder from the *previous* access, etc. and you would get a progressively more bogus address in the end.Quoted 3 lines
Ah, yes, I see it now. Good catch.
Acked-by David Gibson<[email protected]>Hrm - the patch doesn't apply for me. Could you please resend as something that's applyable? :) Also, please make sure to always CC qemu-ppc on ppc patches, otherwise there's a good chance they slip off my radar.
Quoted 6 lines
Quoted 2 lines
Alex
Weird. I've provided it as an attachment, which should hopefully work this time. -Nathan
<0005-Fix-large-page-support-in-TCG.-The-old-code-would-ov.patch>
Ok, thanks. I fixed the checkpatch errors:
WARNING: braces {} are necessary for all arms of this statement #86: FILE: target-ppc/helper.c:677:
+ if (target_page_bits != TARGET_PAGE_BITS)
[...]
ERROR: code indent should never use tabs
#87: FILE: target-ppc/helper.c:678:
+^Ictx->raddr |= (ctx->eaddr & (( 1 << target_page_bits ) - 1))$
ERROR: space prohibited after that open parenthesis '(' #87: FILE: target-ppc/helper.c:678:
+ ctx->raddr |= (ctx->eaddr & (( 1 << target_page_bits ) - 1))
ERROR: space prohibited before that close parenthesis ')' #87: FILE: target-ppc/helper.c:678:
+ ctx->raddr |= (ctx->eaddr & (( 1 << target_page_bits ) - 1))
ERROR: code indent should never use tabs
#88: FILE: target-ppc/helper.c:679:
+^I^I& TARGET_PAGE_MASK;$
total: 4 errors, 1 warnings, 23 lines checked
Your patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS.
and applied the patch to ppc-next.
Alex