* [RFC PATCH] target/ppc: Do not set HPTE R/C bits on !guest_visible xlate
@ 2025-03-03 10:47 Nicholas Piggin
2025-03-03 10:58 ` BALATON Zoltan
0 siblings, 1 reply; 3+ messages in thread
From: Nicholas Piggin @ 2025-03-03 10:47 UTC (permalink / raw)
To: qemu-ppc; +Cc: Nicholas Piggin, qemu-devel
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) {
+ 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;
+ }
}
}
--
2.47.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [RFC PATCH] target/ppc: Do not set HPTE R/C bits on !guest_visible xlate
2025-03-03 10:47 [RFC PATCH] target/ppc: Do not set HPTE R/C bits on !guest_visible xlate Nicholas Piggin
@ 2025-03-03 10:58 ` BALATON Zoltan
2025-03-05 6:45 ` Nicholas Piggin
0 siblings, 1 reply; 3+ messages in thread
From: BALATON Zoltan @ 2025-03-03 10:58 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: qemu-ppc, qemu-devel
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;
> + }
> }
> }
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [RFC PATCH] target/ppc: Do not set HPTE R/C bits on !guest_visible xlate
2025-03-03 10:58 ` BALATON Zoltan
@ 2025-03-05 6:45 ` Nicholas Piggin
0 siblings, 0 replies; 3+ messages in thread
From: Nicholas Piggin @ 2025-03-05 6:45 UTC (permalink / raw)
To: BALATON Zoltan; +Cc: qemu-ppc, qemu-devel
On Mon Mar 3, 2025 at 8:58 PM AEST, BALATON Zoltan wrote:
> 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.
Yes it probably is. Although we don't tend to use unlikely very much. I
guess we have to start somewhere.
Thanks,
Nick
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-03-05 6:45 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-03 10:47 [RFC PATCH] target/ppc: Do not set HPTE R/C bits on !guest_visible xlate Nicholas Piggin
2025-03-03 10:58 ` BALATON Zoltan
2025-03-05 6:45 ` Nicholas Piggin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).