[PATCH 0/3] target/i386: kvm-unit-tests fixes related to page table lookup

[PATCH 0/3] target/i386: kvm-unit-tests fixes related to page table lookup

[Paolo Bonzini]

The first two patches allow access.flat to pass with TCG and "-cpu
max,phys-bits=52", and the last one fixes most NPT tests in svm.flat.

I found these while trying to unify the NPT and regular page table
walk, but those other patches are not appropriate for soft freeze.

Paolo

Paolo Bonzini (3):
  target/i386: allow modifying TCG phys-addr-bits
  target/i386: fail if toggling LA57 in 64-bit mode
  target/i386: svm: do not discard high 32 bits of EXITINFO1

 target/i386/cpu.c             | 23 ++++++++---------------
 target/i386/cpu.h             |  1 -
 target/i386/tcg/excp_helper.c |  7 ++++---
 target/i386/tcg/helper-tcg.h  | 14 +++++---------
 target/i386/tcg/misc_helper.c |  4 ++++
 target/i386/tcg/seg_helper.c  |  4 ++--
 target/i386/tcg/svm_helper.c  | 15 +++++++--------
 7 files changed, 30 insertions(+), 38 deletions(-)

-- 
2.26.2

[PATCH 1/3] target/i386: allow modifying TCG phys-addr-bits

[Paolo Bonzini]

TCG can support any number of physical address bits as long as the
core memory API does.  We only need to compute the reserved bits
mask dynamically.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/cpu.c             | 23 ++++++++---------------
 target/i386/cpu.h             |  1 -
 target/i386/tcg/excp_helper.c |  7 ++++---
 target/i386/tcg/helper-tcg.h  | 12 ++++--------
 4 files changed, 16 insertions(+), 27 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index ae9fd9f31d..6b3e9467f1 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6785,21 +6785,14 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
                     cpu->phys_bits = cpu->host_phys_bits_limit;
                 }
             }
-
-            if (cpu->phys_bits &&
-                (cpu->phys_bits > TARGET_PHYS_ADDR_SPACE_BITS ||
-                cpu->phys_bits < 32)) {
-                error_setg(errp, "phys-bits should be between 32 and %u "
-                                 " (but is %u)",
-                                 TARGET_PHYS_ADDR_SPACE_BITS, cpu->phys_bits);
-                return;
-            }
-        } else {
-            if (cpu->phys_bits && cpu->phys_bits != TCG_PHYS_ADDR_BITS) {
-                error_setg(errp, "TCG only supports phys-bits=%u",
-                                  TCG_PHYS_ADDR_BITS);
-                return;
-            }
+        }
+        if (cpu->phys_bits &&
+            (cpu->phys_bits > TARGET_PHYS_ADDR_SPACE_BITS ||
+            cpu->phys_bits < 32)) {
+            error_setg(errp, "phys-bits should be between 32 and %u "
+                             " (but is %u)",
+                             TARGET_PHYS_ADDR_SPACE_BITS, cpu->phys_bits);
+            return;
         }
         /* 0 means it was not explicitly set by the user (or by machine
          * compat_props or by the host code above). In this case, the default
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index b4b136cd0d..570f916878 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -290,7 +290,6 @@ typedef enum X86Seg {
 #define PG_GLOBAL_MASK   (1 << PG_GLOBAL_BIT)
 #define PG_PSE_PAT_MASK  (1 << PG_PSE_PAT_BIT)
 #define PG_ADDRESS_MASK  0x000ffffffffff000LL
-#define PG_HI_RSVD_MASK  (PG_ADDRESS_MASK & ~PHYS_ADDR_MASK)
 #define PG_HI_USER_MASK  0x7ff0000000000000LL
 #define PG_PKRU_MASK     (15ULL << PG_PKRU_BIT)
 #define PG_NX_MASK       (1ULL << PG_NX_BIT)
diff --git a/target/i386/tcg/excp_helper.c b/target/i386/tcg/excp_helper.c
index b7d6259e4a..1e71e44510 100644
--- a/target/i386/tcg/excp_helper.c
+++ b/target/i386/tcg/excp_helper.c
@@ -142,8 +142,9 @@ void raise_exception_ra(CPUX86State *env, int exception_index, uintptr_t retaddr
 static hwaddr get_hphys(CPUState *cs, hwaddr gphys, MMUAccessType access_type,
                         int *prot)
 {
-    CPUX86State *env = &X86_CPU(cs)->env;
-    uint64_t rsvd_mask = PG_HI_RSVD_MASK;
+    X86CPU *cpu = X86_CPU(cs);
+    CPUX86State *env = &cpu->env;
+    uint64_t rsvd_mask = PG_ADDRESS_MASK & ~MAKE_64BIT_MASK(0, cpu->phys_bits);
     uint64_t ptep, pte;
     uint64_t exit_info_1 = 0;
     target_ulong pde_addr, pte_addr;
@@ -358,7 +359,7 @@ static int handle_mmu_fault(CPUState *cs, vaddr addr, int size,
     int error_code = 0;
     int is_dirty, prot, page_size, is_write, is_user;
     hwaddr paddr;
-    uint64_t rsvd_mask = PG_HI_RSVD_MASK;
+    uint64_t rsvd_mask = PG_ADDRESS_MASK & ~MAKE_64BIT_MASK(0, cpu->phys_bits);
     uint32_t page_offset;
     target_ulong vaddr;
     uint32_t pkr;
diff --git a/target/i386/tcg/helper-tcg.h b/target/i386/tcg/helper-tcg.h
index c133c63555..ef60e2e04b 100644
--- a/target/i386/tcg/helper-tcg.h
+++ b/target/i386/tcg/helper-tcg.h
@@ -25,17 +25,13 @@
 /* Maximum instruction code size */
 #define TARGET_MAX_INSN_SIZE 16
 
-/*
- * XXX: This value should match the one returned by CPUID
- * and in exec.c
- */
-# if defined(TARGET_X86_64)
+#if defined(TARGET_X86_64)
 # define TCG_PHYS_ADDR_BITS 40
-# else
+#else
 # define TCG_PHYS_ADDR_BITS 36
-# endif
+#endif
 
-#define PHYS_ADDR_MASK MAKE_64BIT_MASK(0, TCG_PHYS_ADDR_BITS)
+QEMU_BUILD_BUG_ON(TCG_PHYS_ADDR_BITS > TARGET_PHYS_ADDR_SPACE_BITS);
 
 /**
  * x86_cpu_do_interrupt:
-- 
2.26.2

[PATCH 2/3] target/i386: fail if toggling LA57 in 64-bit mode

[Paolo Bonzini]

This fixes kvm-unit-tests access.flat with -cpu qemu64,la57.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/tcg/misc_helper.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/target/i386/tcg/misc_helper.c b/target/i386/tcg/misc_helper.c
index 90b87fdef0..a25428c36e 100644
--- a/target/i386/tcg/misc_helper.c
+++ b/target/i386/tcg/misc_helper.c
@@ -167,6 +167,10 @@ void helper_write_crN(CPUX86State *env, int reg, target_ulong t0)
         cpu_x86_update_cr3(env, t0);
         break;
     case 4:
+        if (((t0 ^ env->cr[4]) & CR4_LA57_MASK) &&
+            (env->hflags & HF_CS64_MASK)) {
+             raise_exception_ra(env, EXCP0D_GPF, GETPC());
+        }
         cpu_x86_update_cr4(env, t0);
         break;
     case 8:
-- 
2.26.2

[PATCH 3/3] target/i386: svm: do not discard high 32 bits of EXITINFO1

[Paolo Bonzini]

env->error_code is only 32-bits wide, so the high 32 bits of EXITINFO1
are being lost.  However, even though saving guest state and restoring
host state must be delayed to do_vmexit, because they might take tb_lock,
it is always possible to write to the VMCB.  So do this for the exit
code and EXITINFO1, just like it is already being done for EXITINFO2.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/tcg/helper-tcg.h |  2 +-
 target/i386/tcg/seg_helper.c |  4 ++--
 target/i386/tcg/svm_helper.c | 15 +++++++--------
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/target/i386/tcg/helper-tcg.h b/target/i386/tcg/helper-tcg.h
index ef60e2e04b..bcdfca06f6 100644
--- a/target/i386/tcg/helper-tcg.h
+++ b/target/i386/tcg/helper-tcg.h
@@ -80,7 +80,7 @@ void cpu_load_eflags(CPUX86State *env, int eflags, int update_mask);
 /* svm_helper.c */
 void QEMU_NORETURN cpu_vmexit(CPUX86State *nenv, uint32_t exit_code,
                               uint64_t exit_info_1, uintptr_t retaddr);
-void do_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t exit_info_1);
+void do_vmexit(CPUX86State *env);
 
 /* seg_helper.c */
 void do_interrupt_x86_hardirq(CPUX86State *env, int intno, int is_hw);
diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c
index 180d47f0e9..d180a381d1 100644
--- a/target/i386/tcg/seg_helper.c
+++ b/target/i386/tcg/seg_helper.c
@@ -1305,9 +1305,9 @@ void x86_cpu_do_interrupt(CPUState *cs)
     /* successfully delivered */
     env->old_exception = -1;
 #else
-    if (cs->exception_index >= EXCP_VMEXIT) {
+    if (cs->exception_index == EXCP_VMEXIT) {
         assert(env->old_exception == -1);
-        do_vmexit(env, cs->exception_index - EXCP_VMEXIT, env->error_code);
+        do_vmexit(env);
     } else {
         do_interrupt_all(cpu, cs->exception_index,
                          env->exception_is_int,
diff --git a/target/i386/tcg/svm_helper.c b/target/i386/tcg/svm_helper.c
index 097bb9b83d..0145afceae 100644
--- a/target/i386/tcg/svm_helper.c
+++ b/target/i386/tcg/svm_helper.c
@@ -621,15 +621,19 @@ void cpu_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t exit_info_1,
                                                    control.exit_info_2)),
                   env->eip);
 
-    cs->exception_index = EXCP_VMEXIT + exit_code;
-    env->error_code = exit_info_1;
+    cs->exception_index = EXCP_VMEXIT;
+    x86_stq_phys(cs, env->vm_vmcb + offsetof(struct vmcb, control.exit_code),
+             exit_code);
+
+    x86_stq_phys(cs, env->vm_vmcb + offsetof(struct vmcb,
+                                             control.exit_info_1), exit_info_1),
 
     /* remove any pending exception */
     env->old_exception = -1;
     cpu_loop_exit(cs);
 }
 
-void do_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t exit_info_1)
+void do_vmexit(CPUX86State *env)
 {
     CPUState *cs = env_cpu(env);
     uint32_t int_ctl;
@@ -762,11 +766,6 @@ void do_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t exit_info_1)
                           env->vm_hsave + offsetof(struct vmcb, save.dr7));
 
     /* other setups */
-    x86_stq_phys(cs, env->vm_vmcb + offsetof(struct vmcb, control.exit_code),
-             exit_code);
-    x86_stq_phys(cs, env->vm_vmcb + offsetof(struct vmcb, control.exit_info_1),
-             exit_info_1);
-
     x86_stl_phys(cs,
              env->vm_vmcb + offsetof(struct vmcb, control.exit_int_info),
              x86_ldl_phys(cs, env->vm_vmcb + offsetof(struct vmcb,
-- 
2.26.2

Re: [PATCH 0/3] target/i386: kvm-unit-tests fixes related to page table lookup

[no-reply]

Patchew URL: https://patchew.org/QEMU/20210318150022.1824646-1-pbonzini@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210318150022.1824646-1-pbonzini@redhat.com
Subject: [PATCH 0/3] target/i386: kvm-unit-tests fixes related to page table lookup

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
   b12498f..1db136a  master     -> master
 - [tag update]      patchew/20210317032217.1460684-1-michael.roth@amd.com -> patchew/20210317032217.1460684-1-michael.roth@amd.com
 - [tag update]      patchew/20210317035242.24418-1-crosa@redhat.com -> patchew/20210317035242.24418-1-crosa@redhat.com
 * [new tag]         patchew/20210318150022.1824646-1-pbonzini@redhat.com -> patchew/20210318150022.1824646-1-pbonzini@redhat.com
Switched to a new branch 'test'
2b850d1 target/i386: svm: do not discard high 32 bits of EXITINFO1
9870ea6 target/i386: fail if toggling LA57 in 64-bit mode
7c5ca0b target/i386: allow modifying TCG phys-addr-bits

=== OUTPUT BEGIN ===
1/3 Checking commit 7c5ca0b4b6bd (target/i386: allow modifying TCG phys-addr-bits)
2/3 Checking commit 9870ea6556c6 (target/i386: fail if toggling LA57 in 64-bit mode)
ERROR: suspect code indent for conditional statements (8, 13)
#23: FILE: target/i386/tcg/misc_helper.c:170:
+        if (((t0 ^ env->cr[4]) & CR4_LA57_MASK) &&
[...]
+             raise_exception_ra(env, EXCP0D_GPF, GETPC());

total: 1 errors, 0 warnings, 10 lines checked

Patch 2/3 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/3 Checking commit 2b850d1e6ac9 (target/i386: svm: do not discard high 32 bits of EXITINFO1)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210318150022.1824646-1-pbonzini@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com