qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Palmer Dabbelt <palmerdabbelt@google.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-riscv@nongnu.org,          qemu-devel@nongnu.org,
	Alistair Francis <alistair.francis@wdc.com>,
	Bin Meng <bmeng.cn@gmail.com>,
	Palmer Dabbelt <palmerdabbelt@google.com>
Subject: [PULL 1/6] target/riscv: Don't set write permissions on dirty PTEs
Date: Tue, 21 Apr 2020 12:09:56 -0700	[thread overview]
Message-ID: <20200421191001.92644-2-palmerdabbelt@google.com> (raw)
In-Reply-To: <20200421191001.92644-1-palmerdabbelt@google.com>

From: Alistair Francis <alistair.francis@wdc.com>

The RISC-V spec specifies that when a write happens and the D bit is
clear the implementation will set the bit in the PTE. It does not
describe that the PTE being dirty means that we should provide write
access. This patch removes the write access granted to pages when the
dirty bit is set.

Following the prot variable we can see that it affects all of these
functions:
 riscv_cpu_tlb_fill()
   tlb_set_page()
     tlb_set_page_with_attrs()
       address_space_translate_for_iotlb()

Looking at the cputlb code (tlb_set_page_with_attrs() and
address_space_translate_for_iotlb()) it looks like the main affect of
setting write permissions is that the page can be marked as TLB_NOTDIRTY.

I don't see any other impacts (related to the dirty bit) for giving a
page write permissions.

Setting write permission on dirty PTEs results in userspace inside a
Hypervisor guest (VU) becoming corrupted. This appears to be because it
ends up with write permission in the second stage translation in cases
where we aren't doing a store.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com>
---
 target/riscv/cpu_helper.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index d3ba9efb02..e2da2a4787 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -572,10 +572,8 @@ restart:
             if ((pte & PTE_X)) {
                 *prot |= PAGE_EXEC;
             }
-            /* add write permission on stores or if the page is already dirty,
-               so that we TLB miss on later writes to update the dirty bit */
-            if ((pte & PTE_W) &&
-                    (access_type == MMU_DATA_STORE || (pte & PTE_D))) {
+            /* add write permission on stores */
+            if ((pte & PTE_W) && (access_type == MMU_DATA_STORE)) {
                 *prot |= PAGE_WRITE;
             }
             return TRANSLATE_SUCCESS;
-- 
2.26.1.301.g55bc3eb7cb9-goog



  reply	other threads:[~2020-04-21 19:20 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-21 19:09 [PULL] RISC-V Patches for 5.0-rc4 Palmer Dabbelt
2020-04-21 19:09 ` Palmer Dabbelt [this message]
2020-04-21 19:20   ` [PULL 1/6] target/riscv: Don't set write permissions on dirty PTEs Alistair Francis
2020-04-21 19:09 ` [PULL 2/6] riscv: Don't use stage-2 PTE lookup protection flags Palmer Dabbelt
2020-04-21 19:09 ` [PULL 3/6] riscv: AND stage-1 and stage-2 " Palmer Dabbelt
2020-04-21 19:09 ` [PULL 4/6] riscv/sifive_u: Fix up file ordering Palmer Dabbelt
2020-04-21 19:10 ` [PULL 5/6] riscv/sifive_u: Add a serial property to the sifive_u SoC Palmer Dabbelt
2020-04-21 19:10 ` [PULL 6/6] riscv/sifive_u: Add a serial property to the sifive_u machine Palmer Dabbelt
2020-04-21 19:27 ` [PULL] RISC-V Patches for 5.0-rc4 Peter Maydell
2020-04-21 19:32   ` Palmer Dabbelt

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=20200421191001.92644-2-palmerdabbelt@google.com \
    --to=palmerdabbelt@google.com \
    --cc=alistair.francis@wdc.com \
    --cc=bmeng.cn@gmail.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-riscv@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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 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).