| Message ID | 20250303104755.584897-1-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] target/ppc: Do not set HPTE R/C bits on !guest_visible xlate | expand |
On Mon, 3 Mar 2025, Nicholas Piggin wrote: > Perform !guest_visible memory accesses without modifying R/C bits. > > It's arguable whether !guest_visible memory accesses should modify > R/C bits. i386 seems to set accessed/dirty bit updates for "probe" > accesses, but ppc with radix MMU does not. Follow the ppc/radix > lead and perform the accesses without updating R/C bits. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > target/ppc/mmu-hash32.c | 27 ++++++++++++++------------- > target/ppc/mmu-hash64.c | 27 ++++++++++++++------------- > 2 files changed, 28 insertions(+), 26 deletions(-) > > diff --git a/target/ppc/mmu-hash32.c b/target/ppc/mmu-hash32.c > index 1f791a7f2f7..b8d7f87507b 100644 > --- a/target/ppc/mmu-hash32.c > +++ b/target/ppc/mmu-hash32.c > @@ -410,19 +410,20 @@ bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type, > qemu_log_mask(CPU_LOG_MMU, "PTE access granted !\n"); > > /* 8. Update PTE referenced and changed bits if necessary */ > - > - if (!(pte.pte1 & HPTE32_R_R)) { > - ppc_hash32_set_r(cpu, pte_offset, pte.pte1); > - } > - if (!(pte.pte1 & HPTE32_R_C)) { > - if (access_type == MMU_DATA_STORE) { > - ppc_hash32_set_c(cpu, pte_offset, pte.pte1); > - } else { > - /* > - * Treat the page as read-only for now, so that a later write > - * will pass through this function again to set the C bit > - */ > - prot &= ~PAGE_WRITE; > + if (guest_visible) { Are these unlikely() ? Not sure if that makes a difference but if we know it may help some compilers. Regards, BALATON Zoltan > + if (!(pte.pte1 & HPTE32_R_R)) { > + ppc_hash32_set_r(cpu, pte_offset, pte.pte1); > + } > + if (!(pte.pte1 & HPTE32_R_C)) { > + if (access_type == MMU_DATA_STORE) { > + ppc_hash32_set_c(cpu, pte_offset, pte.pte1); > + } else { > + /* > + * Treat the page as read-only for now, so that a later write > + * will pass through this function again to set the C bit > + */ > + prot &= ~PAGE_WRITE; > + } > } > } > *protp = prot; > diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c > index 5ca4faee2ab..de5eb5fb221 100644 > --- a/target/ppc/mmu-hash64.c > +++ b/target/ppc/mmu-hash64.c > @@ -1183,19 +1183,20 @@ bool ppc_hash64_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type, > qemu_log_mask(CPU_LOG_MMU, "PTE access granted !\n"); > > /* 6. Update PTE referenced and changed bits if necessary */ > - > - if (!(pte.pte1 & HPTE64_R_R)) { > - ppc_hash64_set_r(cpu, ptex, pte.pte1); > - } > - if (!(pte.pte1 & HPTE64_R_C)) { > - if (access_type == MMU_DATA_STORE) { > - ppc_hash64_set_c(cpu, ptex, pte.pte1); > - } else { > - /* > - * Treat the page as read-only for now, so that a later write > - * will pass through this function again to set the C bit > - */ > - prot &= ~PAGE_WRITE; > + if (guest_visible) { > + if (!(pte.pte1 & HPTE64_R_R)) { > + ppc_hash64_set_r(cpu, ptex, pte.pte1); > + } > + if (!(pte.pte1 & HPTE64_R_C)) { > + if (access_type == MMU_DATA_STORE) { > + ppc_hash64_set_c(cpu, ptex, pte.pte1); > + } else { > + /* > + * Treat the page as read-only for now, so that a later write > + * will pass through this function again to set the C bit > + */ > + prot &= ~PAGE_WRITE; > + } > } > } > >
diff --git a/target/ppc/mmu-hash32.c b/target/ppc/mmu-hash32.c index 1f791a7f2f7..b8d7f87507b 100644 --- a/target/ppc/mmu-hash32.c +++ b/target/ppc/mmu-hash32.c @@ -410,19 +410,20 @@ bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type, qemu_log_mask(CPU_LOG_MMU, "PTE access granted !\n"); /* 8. Update PTE referenced and changed bits if necessary */ - - if (!(pte.pte1 & HPTE32_R_R)) { - ppc_hash32_set_r(cpu, pte_offset, pte.pte1); - } - if (!(pte.pte1 & HPTE32_R_C)) { - if (access_type == MMU_DATA_STORE) { - ppc_hash32_set_c(cpu, pte_offset, pte.pte1); - } else { - /* - * Treat the page as read-only for now, so that a later write - * will pass through this function again to set the C bit - */ - prot &= ~PAGE_WRITE; + if (guest_visible) { + if (!(pte.pte1 & HPTE32_R_R)) { + ppc_hash32_set_r(cpu, pte_offset, pte.pte1); + } + if (!(pte.pte1 & HPTE32_R_C)) { + if (access_type == MMU_DATA_STORE) { + ppc_hash32_set_c(cpu, pte_offset, pte.pte1); + } else { + /* + * Treat the page as read-only for now, so that a later write + * will pass through this function again to set the C bit + */ + prot &= ~PAGE_WRITE; + } } } *protp = prot; diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c index 5ca4faee2ab..de5eb5fb221 100644 --- a/target/ppc/mmu-hash64.c +++ b/target/ppc/mmu-hash64.c @@ -1183,19 +1183,20 @@ bool ppc_hash64_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type, qemu_log_mask(CPU_LOG_MMU, "PTE access granted !\n"); /* 6. Update PTE referenced and changed bits if necessary */ - - if (!(pte.pte1 & HPTE64_R_R)) { - ppc_hash64_set_r(cpu, ptex, pte.pte1); - } - if (!(pte.pte1 & HPTE64_R_C)) { - if (access_type == MMU_DATA_STORE) { - ppc_hash64_set_c(cpu, ptex, pte.pte1); - } else { - /* - * Treat the page as read-only for now, so that a later write - * will pass through this function again to set the C bit - */ - prot &= ~PAGE_WRITE; + if (guest_visible) { + if (!(pte.pte1 & HPTE64_R_R)) { + ppc_hash64_set_r(cpu, ptex, pte.pte1); + } + if (!(pte.pte1 & HPTE64_R_C)) { + if (access_type == MMU_DATA_STORE) { + ppc_hash64_set_c(cpu, ptex, pte.pte1); + } else { + /* + * Treat the page as read-only for now, so that a later write + * will pass through this function again to set the C bit + */ + prot &= ~PAGE_WRITE; + } } }
Perform !guest_visible memory accesses without modifying R/C bits. It's arguable whether !guest_visible memory accesses should modify R/C bits. i386 seems to set accessed/dirty bit updates for "probe" accesses, but ppc with radix MMU does not. Follow the ppc/radix lead and perform the accesses without updating R/C bits. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- target/ppc/mmu-hash32.c | 27 ++++++++++++++------------- target/ppc/mmu-hash64.c | 27 ++++++++++++++------------- 2 files changed, 28 insertions(+), 26 deletions(-)