All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: qemu-devel@nongnu.org
Cc: qemu-stable@nongnu.org, Michael Brown <mcb30@ipxe.org>
Subject: [PULL 06/10] target/i386: Fix physical address truncation
Date: Wed, 28 Feb 2024 09:06:42 +0100	[thread overview]
Message-ID: <20240228080646.261365-7-pbonzini@redhat.com> (raw)
In-Reply-To: <20240228080646.261365-1-pbonzini@redhat.com>

The address translation logic in get_physical_address() will currently
truncate physical addresses to 32 bits unless long mode is enabled.
This is incorrect when using physical address extensions (PAE) outside
of long mode, with the result that a 32-bit operating system using PAE
to access memory above 4G will experience undefined behaviour.

The truncation code was originally introduced in commit 33dfdb5 ("x86:
only allow real mode to access 32bit without LMA"), where it applied
only to translations performed while paging is disabled (and so cannot
affect guests using PAE).

Commit 9828198 ("target/i386: Add MMU_PHYS_IDX and MMU_NESTED_IDX")
rearranged the code such that the truncation also applied to the use
of MMU_PHYS_IDX and MMU_NESTED_IDX.  Commit 4a1e9d4 ("target/i386: Use
atomic operations for pte updates") brought this truncation into scope
for page table entry accesses, and is the first commit for which a
Windows 10 32-bit guest will reliably fail to boot if memory above 4G
is present.

The truncation code however is not completely redundant.  Even though the
maximum address size for any executed instruction is 32 bits, helpers for
operations such as BOUND, FSAVE or XSAVE may ask get_physical_address()
to translate an address outside of the 32-bit range, if invoked with an
argument that is close to the 4G boundary.  Likewise for processor
accesses, for example TSS or IDT accesses, when EFER.LMA==0.

So, move the address truncation in get_physical_address() so that it
applies to 32-bit MMU indexes, but not to MMU_PHYS_IDX and MMU_NESTED_IDX.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2040
Fixes: 4a1e9d4d11c ("target/i386: Use atomic operations for pte updates", 2022-10-18)
Cc: qemu-stable@nongnu.org
Co-developed-by: Michael Brown <mcb30@ipxe.org>
Signed-off-by: Michael Brown <mcb30@ipxe.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/cpu.h                    |  6 ++++++
 target/i386/cpu.c                    |  2 +-
 target/i386/tcg/sysemu/excp_helper.c | 12 +++++-------
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index ee4ad372021..952174bb6f5 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2326,6 +2326,12 @@ static inline bool is_mmu_index_user(int mmu_index)
     return (mmu_index & ~1) == MMU_USER64_IDX;
 }
 
+static inline bool is_mmu_index_32(int mmu_index)
+{
+    assert(mmu_index < MMU_PHYS_IDX);
+    return mmu_index & 1;
+}
+
 static inline int cpu_mmu_index_kernel(CPUX86State *env)
 {
     int mmu_index_32 = (env->hflags & HF_LMA_MASK) ? 1 : 0;
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 647371198c7..ba6d7b80a7f 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7732,7 +7732,7 @@ static bool x86_cpu_has_work(CPUState *cs)
     return x86_cpu_pending_interrupt(cs, cs->interrupt_request) != 0;
 }
 
-static int x86_cpu_mmu_index(CPUState *env, bool ifetch)
+static int x86_cpu_mmu_index(CPUState *cs, bool ifetch)
 {
     CPUX86State *env = cpu_env(cs);
     int mmu_index_32 = (env->hflags & HF_CS64_MASK) ? 1 : 0;
diff --git a/target/i386/tcg/sysemu/excp_helper.c b/target/i386/tcg/sysemu/excp_helper.c
index b2c525e1a92..8bcdd2906d5 100644
--- a/target/i386/tcg/sysemu/excp_helper.c
+++ b/target/i386/tcg/sysemu/excp_helper.c
@@ -558,6 +558,10 @@ static bool get_physical_address(CPUX86State *env, vaddr addr,
         break;
 
     default:
+        if (is_mmu_index_32(mmu_idx)) {
+            addr = (uint32_t)addr;
+        }
+
         if (likely(env->cr[0] & CR0_PG_MASK)) {
             in.cr3 = env->cr[3];
             in.mmu_idx = mmu_idx;
@@ -581,14 +585,8 @@ static bool get_physical_address(CPUX86State *env, vaddr addr,
         break;
     }
 
-    /* Translation disabled. */
+    /* No translation needed. */
     out->paddr = addr & x86_get_a20_mask(env);
-#ifdef TARGET_X86_64
-    if (!(env->hflags & HF_LMA_MASK)) {
-        /* Without long mode we can only address 32bits in real mode */
-        out->paddr = (uint32_t)out->paddr;
-    }
-#endif
     out->prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
     out->page_size = TARGET_PAGE_SIZE;
     return true;
-- 
2.43.2



  parent reply	other threads:[~2024-02-28  8:08 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-28  8:06 [PULL 00/10] pc, target/i486 changes for 2024-02-27 Paolo Bonzini
2024-02-28  8:06 ` [PULL 01/10] vl, pc: turn -no-fd-bootchk into a machine property Paolo Bonzini
2024-02-28  8:06 ` [PULL 02/10] target/i386: mask high bits of CR3 in 32-bit mode Paolo Bonzini
2024-02-28  8:06 ` [PULL 03/10] target/i386: check validity of VMCB addresses Paolo Bonzini
2024-02-28  8:06 ` [PULL 04/10] target/i386: introduce function to query MMU indices Paolo Bonzini
2024-02-28  8:06 ` [PULL 05/10] target/i386: use separate MMU indexes for 32-bit accesses Paolo Bonzini
2024-02-28  8:06 ` Paolo Bonzini [this message]
2024-02-28 18:13   ` [PULL 06/10] target/i386: Fix physical address truncation Michael Tokarev
2024-02-28 20:18     ` Paolo Bonzini
2024-02-28  8:06 ` [PULL 07/10] target/i386: remove unnecessary/wrong application of the A20 mask Paolo Bonzini
2024-02-28  8:06 ` [PULL 08/10] target/i386: leave the A20 bit set in the final NPT walk Paolo Bonzini
2024-02-28  8:06 ` [PULL 09/10] ide: collapse parameters to ide_init_drive Paolo Bonzini
2024-02-28  8:06 ` [PULL 10/10] ide, vl: turn -win2k-hack into a property on IDE devices Paolo Bonzini
2024-02-28 17:26 ` [PULL 00/10] pc, target/i486 changes for 2024-02-27 Peter Maydell

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=20240228080646.261365-7-pbonzini@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=mcb30@ipxe.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.