* [PULL 01/20] i386/sev: Don't allow automatic fallback to legacy KVM_SEV*_INIT
2024-07-17 5:03 [PULL 00/20] i386, bugfix changes for QEMU 9.1 soft freeze Paolo Bonzini
@ 2024-07-17 5:03 ` Paolo Bonzini
2024-07-17 5:03 ` [PULL 02/20] Revert "qemu-char: do not operate on sources from finalize callbacks" Paolo Bonzini
` (19 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2024-07-17 5:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Roth, Daniel P . Berrangé, kvm
From: Michael Roth <michael.roth@amd.com>
Currently if the 'legacy-vm-type' property of the sev-guest object is
'on', QEMU will attempt to use the newer KVM_SEV_INIT2 kernel
interface in conjunction with the newer KVM_X86_SEV_VM and
KVM_X86_SEV_ES_VM KVM VM types.
This can lead to measurement changes if, for instance, an SEV guest was
created on a host that originally had an older kernel that didn't
support KVM_SEV_INIT2, but is booted on the same host later on after the
host kernel was upgraded.
Instead, if legacy-vm-type is 'off', QEMU should fail if the
KVM_SEV_INIT2 interface is not provided by the current host kernel.
Modify the fallback handling accordingly.
In the future, VMSA features and other flags might be added to QEMU
which will require legacy-vm-type to be 'off' because they will rely
on the newer KVM_SEV_INIT2 interface. It may be difficult to convey to
users what values of legacy-vm-type are compatible with which
features/options, so as part of this rework, switch legacy-vm-type to a
tri-state OnOffAuto option. 'auto' in this case will automatically
switch to using the newer KVM_SEV_INIT2, but only if it is required to
make use of new VMSA features or other options only available via
KVM_SEV_INIT2.
Defining 'auto' in this way would avoid inadvertantly breaking
compatibility with older kernels since it would only be used in cases
where users opt into newer features that are only available via
KVM_SEV_INIT2 and newer kernels, and provide better default behavior
than the legacy-vm-type=off behavior that was previously in place, so
make it the default for 9.1+ machine types.
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
cc: kvm@vger.kernel.org
Signed-off-by: Michael Roth <michael.roth@amd.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Link: https://lore.kernel.org/r/20240710041005.83720-1-michael.roth@amd.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qapi/qom.json | 18 ++++++----
hw/i386/pc.c | 2 +-
target/i386/sev.c | 87 +++++++++++++++++++++++++++++++++++++++--------
3 files changed, 84 insertions(+), 23 deletions(-)
diff --git a/qapi/qom.json b/qapi/qom.json
index 8e75a419c30..7eccd2e14e2 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -924,12 +924,16 @@
# @handle: SEV firmware handle (default: 0)
#
# @legacy-vm-type: Use legacy KVM_SEV_INIT KVM interface for creating the VM.
-# The newer KVM_SEV_INIT2 interface syncs additional vCPU
-# state when initializing the VMSA structures, which will
-# result in a different guest measurement. Set this to
-# maintain compatibility with older QEMU or kernel versions
-# that rely on legacy KVM_SEV_INIT behavior.
-# (default: false) (since 9.1)
+# The newer KVM_SEV_INIT2 interface, from Linux >= 6.10, syncs
+# additional vCPU state when initializing the VMSA structures,
+# which will result in a different guest measurement. Set
+# this to 'on' to force compatibility with older QEMU or kernel
+# versions that rely on legacy KVM_SEV_INIT behavior. 'auto'
+# will behave identically to 'on', but will automatically
+# switch to using KVM_SEV_INIT2 if the user specifies any
+# additional options that require it. If set to 'off', QEMU
+# will require KVM_SEV_INIT2 unconditionally.
+# (default: off) (since 9.1)
#
# Since: 2.12
##
@@ -939,7 +943,7 @@
'*session-file': 'str',
'*policy': 'uint32',
'*handle': 'uint32',
- '*legacy-vm-type': 'bool' } }
+ '*legacy-vm-type': 'OnOffAuto' } }
##
# @SevSnpGuestProperties:
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 4fbc5774708..c74931d577a 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -83,7 +83,7 @@ GlobalProperty pc_compat_9_0[] = {
{ TYPE_X86_CPU, "x-amd-topoext-features-only", "false" },
{ TYPE_X86_CPU, "x-l1-cache-per-thread", "false" },
{ TYPE_X86_CPU, "guest-phys-bits", "0" },
- { "sev-guest", "legacy-vm-type", "true" },
+ { "sev-guest", "legacy-vm-type", "on" },
{ TYPE_X86_CPU, "legacy-multi-node", "on" },
};
const size_t pc_compat_9_0_len = G_N_ELEMENTS(pc_compat_9_0);
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 2ba5f517228..a1157c0ede6 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -144,7 +144,7 @@ struct SevGuestState {
uint32_t policy;
char *dh_cert_file;
char *session_file;
- bool legacy_vm_type;
+ OnOffAuto legacy_vm_type;
};
struct SevSnpGuestState {
@@ -1369,6 +1369,17 @@ sev_vm_state_change(void *opaque, bool running, RunState state)
}
}
+/*
+ * This helper is to examine sev-guest properties and determine if any options
+ * have been set which rely on the newer KVM_SEV_INIT2 interface and associated
+ * KVM VM types.
+ */
+static bool sev_init2_required(SevGuestState *sev_guest)
+{
+ /* Currently no KVM_SEV_INIT2-specific options are exposed via QEMU */
+ return false;
+}
+
static int sev_kvm_type(X86ConfidentialGuest *cg)
{
SevCommonState *sev_common = SEV_COMMON(cg);
@@ -1379,14 +1390,39 @@ static int sev_kvm_type(X86ConfidentialGuest *cg)
goto out;
}
- kvm_type = (sev_guest->policy & SEV_POLICY_ES) ?
- KVM_X86_SEV_ES_VM : KVM_X86_SEV_VM;
- if (kvm_is_vm_type_supported(kvm_type) && !sev_guest->legacy_vm_type) {
- sev_common->kvm_type = kvm_type;
- } else {
+ /* These are the only cases where legacy VM types can be used. */
+ if (sev_guest->legacy_vm_type == ON_OFF_AUTO_ON ||
+ (sev_guest->legacy_vm_type == ON_OFF_AUTO_AUTO &&
+ !sev_init2_required(sev_guest))) {
sev_common->kvm_type = KVM_X86_DEFAULT_VM;
+ goto out;
}
+ /*
+ * Newer VM types are required, either explicitly via legacy-vm-type=on, or
+ * implicitly via legacy-vm-type=auto along with additional sev-guest
+ * properties that require the newer VM types.
+ */
+ kvm_type = (sev_guest->policy & SEV_POLICY_ES) ?
+ KVM_X86_SEV_ES_VM : KVM_X86_SEV_VM;
+ if (!kvm_is_vm_type_supported(kvm_type)) {
+ if (sev_guest->legacy_vm_type == ON_OFF_AUTO_AUTO) {
+ error_report("SEV: host kernel does not support requested %s VM type, which is required "
+ "for the set of options specified. To allow use of the legacy "
+ "KVM_X86_DEFAULT_VM VM type, please disable any options that are not "
+ "compatible with the legacy VM type, or upgrade your kernel.",
+ kvm_type == KVM_X86_SEV_VM ? "KVM_X86_SEV_VM" : "KVM_X86_SEV_ES_VM");
+ } else {
+ error_report("SEV: host kernel does not support requested %s VM type. To allow use of "
+ "the legacy KVM_X86_DEFAULT_VM VM type, the 'legacy-vm-type' argument "
+ "must be set to 'on' or 'auto' for the sev-guest object.",
+ kvm_type == KVM_X86_SEV_VM ? "KVM_X86_SEV_VM" : "KVM_X86_SEV_ES_VM");
+ }
+
+ return -1;
+ }
+
+ sev_common->kvm_type = kvm_type;
out:
return sev_common->kvm_type;
}
@@ -1477,14 +1513,24 @@ static int sev_common_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
}
trace_kvm_sev_init();
- if (x86_klass->kvm_type(X86_CONFIDENTIAL_GUEST(sev_common)) == KVM_X86_DEFAULT_VM) {
+ switch (x86_klass->kvm_type(X86_CONFIDENTIAL_GUEST(sev_common))) {
+ case KVM_X86_DEFAULT_VM:
cmd = sev_es_enabled() ? KVM_SEV_ES_INIT : KVM_SEV_INIT;
ret = sev_ioctl(sev_common->sev_fd, cmd, NULL, &fw_error);
- } else {
+ break;
+ case KVM_X86_SEV_VM:
+ case KVM_X86_SEV_ES_VM:
+ case KVM_X86_SNP_VM: {
struct kvm_sev_init args = { 0 };
ret = sev_ioctl(sev_common->sev_fd, KVM_SEV_INIT2, &args, &fw_error);
+ break;
+ }
+ default:
+ error_setg(errp, "%s: host kernel does not support the requested SEV configuration.",
+ __func__);
+ return -1;
}
if (ret) {
@@ -2074,14 +2120,23 @@ sev_guest_set_session_file(Object *obj, const char *value, Error **errp)
SEV_GUEST(obj)->session_file = g_strdup(value);
}
-static bool sev_guest_get_legacy_vm_type(Object *obj, Error **errp)
+static void sev_guest_get_legacy_vm_type(Object *obj, Visitor *v,
+ const char *name, void *opaque,
+ Error **errp)
{
- return SEV_GUEST(obj)->legacy_vm_type;
+ SevGuestState *sev_guest = SEV_GUEST(obj);
+ OnOffAuto legacy_vm_type = sev_guest->legacy_vm_type;
+
+ visit_type_OnOffAuto(v, name, &legacy_vm_type, errp);
}
-static void sev_guest_set_legacy_vm_type(Object *obj, bool value, Error **errp)
+static void sev_guest_set_legacy_vm_type(Object *obj, Visitor *v,
+ const char *name, void *opaque,
+ Error **errp)
{
- SEV_GUEST(obj)->legacy_vm_type = value;
+ SevGuestState *sev_guest = SEV_GUEST(obj);
+
+ visit_type_OnOffAuto(v, name, &sev_guest->legacy_vm_type, errp);
}
static void
@@ -2107,9 +2162,9 @@ sev_guest_class_init(ObjectClass *oc, void *data)
sev_guest_set_session_file);
object_class_property_set_description(oc, "session-file",
"guest owners session parameters (encoded with base64)");
- object_class_property_add_bool(oc, "legacy-vm-type",
- sev_guest_get_legacy_vm_type,
- sev_guest_set_legacy_vm_type);
+ object_class_property_add(oc, "legacy-vm-type", "OnOffAuto",
+ sev_guest_get_legacy_vm_type,
+ sev_guest_set_legacy_vm_type, NULL, NULL);
object_class_property_set_description(oc, "legacy-vm-type",
"use legacy VM type to maintain measurement compatibility with older QEMU or kernel versions.");
}
@@ -2125,6 +2180,8 @@ sev_guest_instance_init(Object *obj)
object_property_add_uint32_ptr(obj, "policy", &sev_guest->policy,
OBJ_PROP_FLAG_READWRITE);
object_apply_compat_props(obj);
+
+ sev_guest->legacy_vm_type = ON_OFF_AUTO_AUTO;
}
/* guest info specific sev/sev-es */
--
2.45.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PULL 02/20] Revert "qemu-char: do not operate on sources from finalize callbacks"
2024-07-17 5:03 [PULL 00/20] i386, bugfix changes for QEMU 9.1 soft freeze Paolo Bonzini
2024-07-17 5:03 ` [PULL 01/20] i386/sev: Don't allow automatic fallback to legacy KVM_SEV*_INIT Paolo Bonzini
@ 2024-07-17 5:03 ` Paolo Bonzini
2024-07-17 5:03 ` [PULL 03/20] cpu: Free queued CPU work Paolo Bonzini
` (18 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2024-07-17 5:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Sergey Dyasli
From: Sergey Dyasli <sergey.dyasli@nutanix.com>
This reverts commit 2b316774f60291f57ca9ecb6a9f0712c532cae34.
After 038b4217884c ("Revert "chardev: use a child source for qio input
source"") we've been observing the "iwp->src == NULL" assertion
triggering periodically during the initial capabilities querying by
libvirtd. One of possible backtraces:
Thread 1 (Thread 0x7f16cd4f0700 (LWP 43858)):
0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
1 0x00007f16c6c21e65 in __GI_abort () at abort.c:79
2 0x00007f16c6c21d39 in __assert_fail_base at assert.c:92
3 0x00007f16c6c46e86 in __GI___assert_fail (assertion=assertion@entry=0x562e9bcdaadd "iwp->src == NULL", file=file@entry=0x562e9bcdaac8 "../chardev/char-io.c", line=line@entry=99, function=function@entry=0x562e9bcdab10 <__PRETTY_FUNCTION__.20549> "io_watch_poll_finalize") at assert.c:101
4 0x0000562e9ba20c2c in io_watch_poll_finalize (source=<optimized out>) at ../chardev/char-io.c:99
5 io_watch_poll_finalize (source=<optimized out>) at ../chardev/char-io.c:88
6 0x00007f16c904aae0 in g_source_unref_internal () from /lib64/libglib-2.0.so.0
7 0x00007f16c904baf9 in g_source_destroy_internal () from /lib64/libglib-2.0.so.0
8 0x0000562e9ba20db0 in io_remove_watch_poll (source=0x562e9d6720b0) at ../chardev/char-io.c:147
9 remove_fd_in_watch (chr=chr@entry=0x562e9d5f3800) at ../chardev/char-io.c:153
10 0x0000562e9ba23ffb in update_ioc_handlers (s=0x562e9d5f3800) at ../chardev/char-socket.c:592
11 0x0000562e9ba2072f in qemu_chr_fe_set_handlers_full at ../chardev/char-fe.c:279
12 0x0000562e9ba207a9 in qemu_chr_fe_set_handlers at ../chardev/char-fe.c:304
13 0x0000562e9ba2ca75 in monitor_qmp_setup_handlers_bh (opaque=0x562e9d4c2c60) at ../monitor/qmp.c:509
14 0x0000562e9bb6222e in aio_bh_poll (ctx=ctx@entry=0x562e9d4c2f20) at ../util/async.c:216
15 0x0000562e9bb4de0a in aio_poll (ctx=0x562e9d4c2f20, blocking=blocking@entry=true) at ../util/aio-posix.c:722
16 0x0000562e9b99dfaa in iothread_run (opaque=0x562e9d4c26f0) at ../iothread.c:63
17 0x0000562e9bb505a4 in qemu_thread_start (args=0x562e9d4c7ea0) at ../util/qemu-thread-posix.c:543
18 0x00007f16c70081ca in start_thread (arg=<optimized out>) at pthread_create.c:479
19 0x00007f16c6c398d3 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
io_remove_watch_poll(), which makes sure that iwp->src is NULL, calls
g_source_destroy() which finds that iwp->src is not NULL in the finalize
callback. This can only happen if another thread has managed to trigger
io_watch_poll_prepare() callback in the meantime.
Move iwp->src destruction back to the finalize callback to prevent the
described race, and also remove the stale comment. The deadlock glib bug
was fixed back in 2010 by b35820285668 ("gmain: move finalization of
GSource outside of context lock").
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Sergey Dyasli <sergey.dyasli@nutanix.com>
Link: https://lore.kernel.org/r/20240712092659.216206-1-sergey.dyasli@nutanix.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
chardev/char-io.c | 19 +++++--------------
1 file changed, 5 insertions(+), 14 deletions(-)
diff --git a/chardev/char-io.c b/chardev/char-io.c
index dab77b112e3..3be17b51ca5 100644
--- a/chardev/char-io.c
+++ b/chardev/char-io.c
@@ -87,16 +87,12 @@ static gboolean io_watch_poll_dispatch(GSource *source, GSourceFunc callback,
static void io_watch_poll_finalize(GSource *source)
{
- /*
- * Due to a glib bug, removing the last reference to a source
- * inside a finalize callback causes recursive locking (and a
- * deadlock). This is not a problem inside other callbacks,
- * including dispatch callbacks, so we call io_remove_watch_poll
- * to remove this source. At this point, iwp->src must
- * be NULL, or we would leak it.
- */
IOWatchPoll *iwp = io_watch_poll_from_source(source);
- assert(iwp->src == NULL);
+ if (iwp->src) {
+ g_source_destroy(iwp->src);
+ g_source_unref(iwp->src);
+ iwp->src = NULL;
+ }
}
static GSourceFuncs io_watch_poll_funcs = {
@@ -139,11 +135,6 @@ static void io_remove_watch_poll(GSource *source)
IOWatchPoll *iwp;
iwp = io_watch_poll_from_source(source);
- if (iwp->src) {
- g_source_destroy(iwp->src);
- g_source_unref(iwp->src);
- iwp->src = NULL;
- }
g_source_destroy(&iwp->parent);
}
--
2.45.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PULL 03/20] cpu: Free queued CPU work
2024-07-17 5:03 [PULL 00/20] i386, bugfix changes for QEMU 9.1 soft freeze Paolo Bonzini
2024-07-17 5:03 ` [PULL 01/20] i386/sev: Don't allow automatic fallback to legacy KVM_SEV*_INIT Paolo Bonzini
2024-07-17 5:03 ` [PULL 02/20] Revert "qemu-char: do not operate on sources from finalize callbacks" Paolo Bonzini
@ 2024-07-17 5:03 ` Paolo Bonzini
2024-07-17 5:03 ` [PULL 04/20] disas: Fix build against Capstone v6 Paolo Bonzini
` (17 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2024-07-17 5:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Akihiko Odaki
From: Akihiko Odaki <akihiko.odaki@daynix.com>
Running qemu-system-aarch64 -M virt -nographic and terminating it will
result in a LeakSanitizer error due to remaining queued CPU work so
free it.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Link: https://lore.kernel.org/r/20240714-cpu-v1-1-19c2f8de2055@daynix.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/hw/core/cpu.h | 6 ++++++
cpu-common.c | 11 +++++++++++
hw/core/cpu-common.c | 1 +
3 files changed, 18 insertions(+)
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index a2c8536943f..8e6466c1dda 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -1000,6 +1000,12 @@ void cpu_resume(CPUState *cpu);
*/
void cpu_remove_sync(CPUState *cpu);
+/**
+ * free_queued_cpu_work() - free all items on CPU work queue
+ * @cpu: The CPU which work queue to free.
+ */
+void free_queued_cpu_work(CPUState *cpu);
+
/**
* process_queued_cpu_work() - process all items on CPU work queue
* @cpu: The CPU which work queue to process.
diff --git a/cpu-common.c b/cpu-common.c
index ce78273af59..7ae136f98ca 100644
--- a/cpu-common.c
+++ b/cpu-common.c
@@ -331,6 +331,17 @@ void async_safe_run_on_cpu(CPUState *cpu, run_on_cpu_func func,
queue_work_on_cpu(cpu, wi);
}
+void free_queued_cpu_work(CPUState *cpu)
+{
+ while (!QSIMPLEQ_EMPTY(&cpu->work_list)) {
+ struct qemu_work_item *wi = QSIMPLEQ_FIRST(&cpu->work_list);
+ QSIMPLEQ_REMOVE_HEAD(&cpu->work_list, node);
+ if (wi->free) {
+ g_free(wi);
+ }
+ }
+}
+
void process_queued_cpu_work(CPUState *cpu)
{
struct qemu_work_item *wi;
diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
index b19e1fdacf2..d2e3e4570ab 100644
--- a/hw/core/cpu-common.c
+++ b/hw/core/cpu-common.c
@@ -281,6 +281,7 @@ static void cpu_common_finalize(Object *obj)
g_free(cpu->plugin_state);
}
#endif
+ free_queued_cpu_work(cpu);
g_array_free(cpu->gdb_regs, TRUE);
qemu_lockcnt_destroy(&cpu->in_ioctl_lock);
qemu_mutex_destroy(&cpu->work_mutex);
--
2.45.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PULL 04/20] disas: Fix build against Capstone v6
2024-07-17 5:03 [PULL 00/20] i386, bugfix changes for QEMU 9.1 soft freeze Paolo Bonzini
` (2 preceding siblings ...)
2024-07-17 5:03 ` [PULL 03/20] cpu: Free queued CPU work Paolo Bonzini
@ 2024-07-17 5:03 ` Paolo Bonzini
2024-07-17 5:03 ` [PULL 05/20] hw/scsi/lsi53c895a: bump instruction limit in scripts processing to fix regression Paolo Bonzini
` (16 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2024-07-17 5:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Gustavo Romero, Peter Maydell, Richard Henderson
From: Gustavo Romero <gustavo.romero@linaro.org>
Capstone v6 made major changes, such as renaming for AArch64, which
broke programs using the old headers, like QEMU. However, Capstone v6
provides the CAPSTONE_AARCH64_COMPAT_HEADER compatibility definition
allowing to build against v6 with the old definitions, so fix the QEMU
build using it.
We can lift that definition and switch to the new naming once our
supported distros have Capstone v6 in place.
Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Link: https://lore.kernel.org/r/20240715213943.1210355-1-gustavo.romero@linaro.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/disas/capstone.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/disas/capstone.h b/include/disas/capstone.h
index e29068dd977..a11985151d3 100644
--- a/include/disas/capstone.h
+++ b/include/disas/capstone.h
@@ -3,6 +3,7 @@
#ifdef CONFIG_CAPSTONE
+#define CAPSTONE_AARCH64_COMPAT_HEADER
#include <capstone.h>
#else
--
2.45.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PULL 05/20] hw/scsi/lsi53c895a: bump instruction limit in scripts processing to fix regression
2024-07-17 5:03 [PULL 00/20] i386, bugfix changes for QEMU 9.1 soft freeze Paolo Bonzini
` (3 preceding siblings ...)
2024-07-17 5:03 ` [PULL 04/20] disas: Fix build against Capstone v6 Paolo Bonzini
@ 2024-07-17 5:03 ` Paolo Bonzini
2024-07-17 5:03 ` [PULL 06/20] scsi: fix regression and honor bootindex again for legacy drives Paolo Bonzini
` (15 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2024-07-17 5:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Fiona Ebner, qemu-stable, Sven Schnelle
From: Fiona Ebner <f.ebner@proxmox.com>
Commit 9876359990 ("hw/scsi/lsi53c895a: add timer to scripts
processing") reduced the maximum allowed instruction count by
a factor of 100 all the way down to 100.
This causes the "Check Point R81.20 Gaia" appliance [0] to fail to
boot after fully finishing the installation via the appliance's web
interface (there is already one reboot before that).
With a limit of 150, the appliance still fails to boot, while with a
limit of 200, it works. Bump to 500 to fix the regression and be on
the safe side.
Originally reported in the Proxmox community forum[1].
[0]: https://support.checkpoint.com/results/download/124397
[1]: https://forum.proxmox.com/threads/149772/post-683459
Cc: qemu-stable@nongnu.org
Fixes: 9876359990 ("hw/scsi/lsi53c895a: add timer to scripts processing")
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Acked-by: Sven Schnelle <svens@stackframe.org>
Link: https://lore.kernel.org/r/20240715131403.223239-1-f.ebner@proxmox.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi/lsi53c895a.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index eb9828dd5ef..f1935e53280 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -188,7 +188,7 @@ static const char *names[] = {
#define LSI_TAG_VALID (1 << 16)
/* Maximum instructions to process. */
-#define LSI_MAX_INSN 100
+#define LSI_MAX_INSN 500
typedef struct lsi_request {
SCSIRequest *req;
--
2.45.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PULL 06/20] scsi: fix regression and honor bootindex again for legacy drives
2024-07-17 5:03 [PULL 00/20] i386, bugfix changes for QEMU 9.1 soft freeze Paolo Bonzini
` (4 preceding siblings ...)
2024-07-17 5:03 ` [PULL 05/20] hw/scsi/lsi53c895a: bump instruction limit in scripts processing to fix regression Paolo Bonzini
@ 2024-07-17 5:03 ` Paolo Bonzini
2024-07-17 5:03 ` [PULL 07/20] qemu/timer: Add host ticks function for LoongArch Paolo Bonzini
` (14 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2024-07-17 5:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Fiona Ebner, qemu-stable, Kevin Wolf
From: Fiona Ebner <f.ebner@proxmox.com>
Commit 3089637461 ("scsi: Don't ignore most usb-storage properties")
removed the call to object_property_set_int() and thus the 'set'
method for the bootindex property was also not called anymore. Here
that method is device_set_bootindex() (as configured by
scsi_dev_instance_init() -> device_add_bootindex_property()) which as
a side effect registers the device via add_boot_device_path().
As reported by a downstream user [0], the bootindex property did not
have the desired effect anymore for legacy drives. Fix the regression
by explicitly calling the add_boot_device_path() function after
checking that the bootindex is not yet used (to avoid
add_boot_device_path() calling exit()).
[0]: https://forum.proxmox.com/threads/149772/post-679433
Cc: qemu-stable@nongnu.org
Fixes: 3089637461 ("scsi: Don't ignore most usb-storage properties")
Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.kernel.org/r/20240710152529.1737407-1-f.ebner@proxmox.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi/scsi-bus.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 9e40b0c920b..53eff5dd3d6 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -384,6 +384,7 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockBackend *blk,
DeviceState *dev;
SCSIDevice *s;
DriveInfo *dinfo;
+ Error *local_err = NULL;
if (blk_is_sg(blk)) {
driver = "scsi-generic";
@@ -403,6 +404,14 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockBackend *blk,
s = SCSI_DEVICE(dev);
s->conf = *conf;
+ check_boot_index(conf->bootindex, &local_err);
+ if (local_err) {
+ object_unparent(OBJECT(dev));
+ error_propagate(errp, local_err);
+ return NULL;
+ }
+ add_boot_device_path(conf->bootindex, dev, NULL);
+
qdev_prop_set_uint32(dev, "scsi-id", unit);
if (object_property_find(OBJECT(dev), "removable")) {
qdev_prop_set_bit(dev, "removable", removable);
--
2.45.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PULL 07/20] qemu/timer: Add host ticks function for LoongArch
2024-07-17 5:03 [PULL 00/20] i386, bugfix changes for QEMU 9.1 soft freeze Paolo Bonzini
` (5 preceding siblings ...)
2024-07-17 5:03 ` [PULL 06/20] scsi: fix regression and honor bootindex again for legacy drives Paolo Bonzini
@ 2024-07-17 5:03 ` Paolo Bonzini
2024-07-17 5:03 ` [PULL 08/20] docs: Update description of 'user=username' for '-run-with' Paolo Bonzini
` (13 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2024-07-17 5:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Song Gao
From: Song Gao <gaosong@loongson.cn>
Signed-off-by: Song Gao <gaosong@loongson.cn>
Link: https://lore.kernel.org/r/20240716031500.4193498-1-gaosong@loongson.cn
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qemu/timer.h | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 5ce83c79112..fa56ec9481d 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -1016,6 +1016,15 @@ static inline int64_t cpu_get_host_ticks(void)
return val;
}
+#elif defined(__loongarch64)
+static inline int64_t cpu_get_host_ticks(void)
+{
+ uint64_t val;
+
+ asm volatile("rdtime.d %0, $zero" : "=r"(val));
+ return val;
+}
+
#else
/* The host CPU doesn't have an easily accessible cycle counter.
Just return a monotonically increasing value. This will be
--
2.45.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PULL 08/20] docs: Update description of 'user=username' for '-run-with'
2024-07-17 5:03 [PULL 00/20] i386, bugfix changes for QEMU 9.1 soft freeze Paolo Bonzini
` (6 preceding siblings ...)
2024-07-17 5:03 ` [PULL 07/20] qemu/timer: Add host ticks function for LoongArch Paolo Bonzini
@ 2024-07-17 5:03 ` Paolo Bonzini
2024-07-17 5:03 ` [PULL 09/20] hpet: fix clamping of period Paolo Bonzini
` (12 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2024-07-17 5:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Boqiao Fu
From: Boqiao Fu <bfu@redhat.com>
The description of '-runas' and '-run-with' didn't explain that QEMU
will use setuid/setgid to implement the option, so the user might get
confused if using 'elevateprivileges=deny' as well.
Since '-runas' is going to be deprecated and replaced by '-run-with'
in the coming qemu9.1, add the message there.
Signed-off-by: Boqiao Fu <bfu@redhat.com>
Link: https://lore.kernel.org/r/CAFRHJ6J9uMk+HMZL+W+KE1yoRCOLPgbPUVVDku55sdXYiGXXHg@mail.gmail.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu-options.hx | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/qemu-options.hx b/qemu-options.hx
index ad6521ef5e7..694fa37f284 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -5024,8 +5024,11 @@ SRST
in combination with -runas.
``user=username`` or ``user=uid:gid`` can be used to drop root privileges
- by switching to the specified user (via username) or user and group
- (via uid:gid) immediately before starting guest execution.
+ before starting guest execution. QEMU will use the ``setuid`` and ``setgid``
+ system calls to switch to the specified identity. Note that the
+ ``user=username`` syntax will also apply the full set of supplementary
+ groups for the user, whereas the ``user=uid:gid`` will use only the
+ ``gid`` group.
ERST
#endif
--
2.45.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PULL 09/20] hpet: fix clamping of period
2024-07-17 5:03 [PULL 00/20] i386, bugfix changes for QEMU 9.1 soft freeze Paolo Bonzini
` (7 preceding siblings ...)
2024-07-17 5:03 ` [PULL 08/20] docs: Update description of 'user=username' for '-run-with' Paolo Bonzini
@ 2024-07-17 5:03 ` Paolo Bonzini
2024-07-17 5:03 ` [PULL 10/20] hpet: fix HPET_TN_SETVAL for high 32-bits of the comparator Paolo Bonzini
` (11 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2024-07-17 5:03 UTC (permalink / raw)
To: qemu-devel
When writing a new period, the clamping should use a maximum value
rather tyhan a bit mask. Also, when writing the high bits new_val
is shifted right by 32, so the maximum allowed period should also
be shifted right.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/timer/hpet.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index 01efe4885db..ad881448bf3 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -548,7 +548,9 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
* FIXME: Clamp period to reasonable min value?
* Clamp period to reasonable max value
*/
- new_val &= (timer->config & HPET_TN_32BIT ? ~0u : ~0ull) >> 1;
+ if (timer->config & HPET_TN_32BIT) {
+ new_val = MIN(new_val, ~0u >> 1);
+ }
timer->period =
(timer->period & 0xffffffff00000000ULL) | new_val;
}
@@ -567,7 +569,7 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
* FIXME: Clamp period to reasonable min value?
* Clamp period to reasonable max value
*/
- new_val &= (timer->config & HPET_TN_32BIT ? ~0u : ~0ull) >> 1;
+ new_val = MIN(new_val, ~0u >> 1);
timer->period =
(timer->period & 0xffffffffULL) | new_val << 32;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PULL 10/20] hpet: fix HPET_TN_SETVAL for high 32-bits of the comparator
2024-07-17 5:03 [PULL 00/20] i386, bugfix changes for QEMU 9.1 soft freeze Paolo Bonzini
` (8 preceding siblings ...)
2024-07-17 5:03 ` [PULL 09/20] hpet: fix clamping of period Paolo Bonzini
@ 2024-07-17 5:03 ` Paolo Bonzini
2024-07-17 5:03 ` [PULL 11/20] target/i386/tcg: fix POP to memory in long mode Paolo Bonzini
` (10 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2024-07-17 5:03 UTC (permalink / raw)
To: qemu-devel; +Cc: TaiseiIto
Commit 3787324101b ("hpet: Fix emulation of HPET_TN_SETVAL (Jan Kiszka)",
2009-04-17) applied the fix only to the low 32-bits of the comparator, but
it should be done for the high bits as well. Otherwise, the high 32-bits
of the comparator cannot be written and they remain fixed to 0xffffffff.
Co-developed-by: TaiseiIto <taisei1212@outlook.jp>
Signed-off-by: TaiseiIto <taisei1212@outlook.jp>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/timer/hpet.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index ad881448bf3..4cb5393c0b5 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -554,6 +554,10 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
timer->period =
(timer->period & 0xffffffff00000000ULL) | new_val;
}
+ /*
+ * FIXME: on a 64-bit write, HPET_TN_SETVAL should apply to the
+ * high bits part as well.
+ */
timer->config &= ~HPET_TN_SETVAL;
if (hpet_enabled(s)) {
hpet_set_timer(timer);
@@ -564,7 +568,8 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
if (!timer_is_periodic(timer)
|| (timer->config & HPET_TN_SETVAL)) {
timer->cmp = (timer->cmp & 0xffffffffULL) | new_val << 32;
- } else {
+ }
+ if (timer_is_periodic(timer)) {
/*
* FIXME: Clamp period to reasonable min value?
* Clamp period to reasonable max value
@@ -572,12 +577,12 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
new_val = MIN(new_val, ~0u >> 1);
timer->period =
(timer->period & 0xffffffffULL) | new_val << 32;
- }
- timer->config &= ~HPET_TN_SETVAL;
- if (hpet_enabled(s)) {
- hpet_set_timer(timer);
- }
- break;
+ }
+ timer->config &= ~HPET_TN_SETVAL;
+ if (hpet_enabled(s)) {
+ hpet_set_timer(timer);
+ }
+ break;
case HPET_TN_ROUTE:
timer->fsb = (timer->fsb & 0xffffffff00000000ULL) | new_val;
break;
--
2.45.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PULL 11/20] target/i386/tcg: fix POP to memory in long mode
2024-07-17 5:03 [PULL 00/20] i386, bugfix changes for QEMU 9.1 soft freeze Paolo Bonzini
` (9 preceding siblings ...)
2024-07-17 5:03 ` [PULL 10/20] hpet: fix HPET_TN_SETVAL for high 32-bits of the comparator Paolo Bonzini
@ 2024-07-17 5:03 ` Paolo Bonzini
2024-07-17 5:03 ` [PULL 12/20] target/i386/tcg: Remove SEG_ADDL Paolo Bonzini
` (9 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2024-07-17 5:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Clément Chigot, Richard Henderson
In long mode, POP to memory will write a full 64-bit value. However,
the call to gen_writeback() in gen_POP will use MO_32 because the
decoding table is incorrect.
The bug was latent until commit aea49fbb01a ("target/i386: use gen_writeback()
within gen_POP()", 2024-06-08), and then became visible because gen_op_st_v
now receives op->ot instead of the "ot" returned by gen_pop_T0.
Analyzed-by: Clément Chigot <chigot@adacore.com>
Fixes: 5e9e21bcc4d ("target/i386: move 60-BF opcodes to new decoder", 2024-05-07)
Tested-by: Clément Chigot <chigot@adacore.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/decode-new.c.inc | 2 +-
target/i386/tcg/emit.c.inc | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/i386/tcg/decode-new.c.inc b/target/i386/tcg/decode-new.c.inc
index 0d846c32c22..d2da1d396d5 100644
--- a/target/i386/tcg/decode-new.c.inc
+++ b/target/i386/tcg/decode-new.c.inc
@@ -1717,7 +1717,7 @@ static const X86OpEntry opcodes_root[256] = {
[0x8C] = X86_OP_ENTRYwr(MOV, E,v, S,w, op0_Mw),
[0x8D] = X86_OP_ENTRYwr(LEA, G,v, M,v, nolea),
[0x8E] = X86_OP_ENTRYwr(MOV, S,w, E,w),
- [0x8F] = X86_OP_GROUPw(group1A, E,v),
+ [0x8F] = X86_OP_GROUPw(group1A, E,d64),
[0x98] = X86_OP_ENTRY1(CBW, 0,v), /* rAX */
[0x99] = X86_OP_ENTRYwr(CWD, 2,v, 0,v), /* rDX, rAX */
diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc
index fc7477833bc..016dce81464 100644
--- a/target/i386/tcg/emit.c.inc
+++ b/target/i386/tcg/emit.c.inc
@@ -2788,6 +2788,7 @@ static void gen_POP(DisasContext *s, X86DecodedInsn *decode)
X86DecodedOp *op = &decode->op[0];
MemOp ot = gen_pop_T0(s);
+ assert(ot >= op->ot);
if (op->has_ea || op->unit == X86_OP_SEG) {
/* NOTE: order is important for MMU exceptions */
gen_writeback(s, decode, 0, s->T0);
--
2.45.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PULL 12/20] target/i386/tcg: Remove SEG_ADDL
2024-07-17 5:03 [PULL 00/20] i386, bugfix changes for QEMU 9.1 soft freeze Paolo Bonzini
` (10 preceding siblings ...)
2024-07-17 5:03 ` [PULL 11/20] target/i386/tcg: fix POP to memory in long mode Paolo Bonzini
@ 2024-07-17 5:03 ` Paolo Bonzini
2024-07-17 5:03 ` [PULL 13/20] target/i386/tcg: Allow IRET from user mode to user mode with SMAP Paolo Bonzini
` (8 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2024-07-17 5:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Richard Henderson
From: Richard Henderson <richard.henderson@linaro.org>
This truncation is now handled by MMU_*32_IDX. The introduction of
MMU_*32_IDX in fact applied correct 32-bit wraparound to 16-bit accesses
with a high segment base (e.g. big real mode or vm86 mode), which did
not use SEG_ADDL.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Link: https://lore.kernel.org/r/20240617161210.4639-3-richard.henderson@linaro.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/seg_helper.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c
index aee3d19f29b..19d6b41a589 100644
--- a/target/i386/tcg/seg_helper.c
+++ b/target/i386/tcg/seg_helper.c
@@ -579,10 +579,6 @@ int exception_has_error_code(int intno)
} while (0)
#endif
-/* in 64-bit machines, this can overflow. So this segment addition macro
- * can be used to trim the value to 32-bit whenever needed */
-#define SEG_ADDL(ssp, sp, sp_mask) ((uint32_t)((ssp) + (sp & (sp_mask))))
-
/* XXX: add a is_user flag to have proper security support */
#define PUSHW_RA(ssp, sp, sp_mask, val, ra) \
{ \
@@ -593,7 +589,7 @@ int exception_has_error_code(int intno)
#define PUSHL_RA(ssp, sp, sp_mask, val, ra) \
{ \
sp -= 4; \
- cpu_stl_kernel_ra(env, SEG_ADDL(ssp, sp, sp_mask), (uint32_t)(val), ra); \
+ cpu_stl_kernel_ra(env, (ssp) + (sp & (sp_mask)), (val), ra); \
}
#define POPW_RA(ssp, sp, sp_mask, val, ra) \
@@ -604,7 +600,7 @@ int exception_has_error_code(int intno)
#define POPL_RA(ssp, sp, sp_mask, val, ra) \
{ \
- val = (uint32_t)cpu_ldl_kernel_ra(env, SEG_ADDL(ssp, sp, sp_mask), ra); \
+ val = (uint32_t)cpu_ldl_kernel_ra(env, (ssp) + (sp & (sp_mask)), ra); \
sp += 4; \
}
--
2.45.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PULL 13/20] target/i386/tcg: Allow IRET from user mode to user mode with SMAP
2024-07-17 5:03 [PULL 00/20] i386, bugfix changes for QEMU 9.1 soft freeze Paolo Bonzini
` (11 preceding siblings ...)
2024-07-17 5:03 ` [PULL 12/20] target/i386/tcg: Remove SEG_ADDL Paolo Bonzini
@ 2024-07-17 5:03 ` Paolo Bonzini
2024-07-17 5:03 ` [PULL 14/20] target/i386/tcg: use PUSHL/PUSHW for error code Paolo Bonzini
` (7 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2024-07-17 5:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Robert R . Henry, Richard Henderson
This fixes a bug wherein i386/tcg assumed an interrupt return using
the IRET instruction was always returning from kernel mode to either
kernel mode or user mode. This assumption is violated when IRET is used
as a clever way to restore thread state, as for example in the dotnet
runtime. There, IRET returns from user mode to user mode.
This bug is that stack accesses from IRET and RETF, as well as accesses
to the parameters in a call gate, are normal data accesses using the
current CPL. This manifested itself as a page fault in the guest Linux
kernel due to SMAP preventing the access.
This bug appears to have been in QEMU since the beginning.
Analyzed-by: Robert R. Henry <rrh.henry@gmail.com>
Co-developed-by: Robert R. Henry <rrh.henry@gmail.com>
Signed-off-by: Robert R. Henry <rrh.henry@gmail.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/seg_helper.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c
index 19d6b41a589..224e73e9ed0 100644
--- a/target/i386/tcg/seg_helper.c
+++ b/target/i386/tcg/seg_helper.c
@@ -594,13 +594,13 @@ int exception_has_error_code(int intno)
#define POPW_RA(ssp, sp, sp_mask, val, ra) \
{ \
- val = cpu_lduw_kernel_ra(env, (ssp) + (sp & (sp_mask)), ra); \
+ val = cpu_lduw_data_ra(env, (ssp) + (sp & (sp_mask)), ra); \
sp += 2; \
}
#define POPL_RA(ssp, sp, sp_mask, val, ra) \
{ \
- val = (uint32_t)cpu_ldl_kernel_ra(env, (ssp) + (sp & (sp_mask)), ra); \
+ val = (uint32_t)cpu_ldl_data_ra(env, (ssp) + (sp & (sp_mask)), ra); \
sp += 4; \
}
@@ -847,7 +847,7 @@ static void do_interrupt_protected(CPUX86State *env, int intno, int is_int,
#define POPQ_RA(sp, val, ra) \
{ \
- val = cpu_ldq_kernel_ra(env, sp, ra); \
+ val = cpu_ldq_data_ra(env, sp, ra); \
sp += 8; \
}
@@ -1797,18 +1797,18 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
PUSHL_RA(ssp, sp, sp_mask, env->segs[R_SS].selector, GETPC());
PUSHL_RA(ssp, sp, sp_mask, env->regs[R_ESP], GETPC());
for (i = param_count - 1; i >= 0; i--) {
- val = cpu_ldl_kernel_ra(env, old_ssp +
- ((env->regs[R_ESP] + i * 4) &
- old_sp_mask), GETPC());
+ val = cpu_ldl_data_ra(env,
+ old_ssp + ((env->regs[R_ESP] + i * 4) & old_sp_mask),
+ GETPC());
PUSHL_RA(ssp, sp, sp_mask, val, GETPC());
}
} else {
PUSHW_RA(ssp, sp, sp_mask, env->segs[R_SS].selector, GETPC());
PUSHW_RA(ssp, sp, sp_mask, env->regs[R_ESP], GETPC());
for (i = param_count - 1; i >= 0; i--) {
- val = cpu_lduw_kernel_ra(env, old_ssp +
- ((env->regs[R_ESP] + i * 2) &
- old_sp_mask), GETPC());
+ val = cpu_lduw_data_ra(env,
+ old_ssp + ((env->regs[R_ESP] + i * 2) & old_sp_mask),
+ GETPC());
PUSHW_RA(ssp, sp, sp_mask, val, GETPC());
}
}
--
2.45.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PULL 14/20] target/i386/tcg: use PUSHL/PUSHW for error code
2024-07-17 5:03 [PULL 00/20] i386, bugfix changes for QEMU 9.1 soft freeze Paolo Bonzini
` (12 preceding siblings ...)
2024-07-17 5:03 ` [PULL 13/20] target/i386/tcg: Allow IRET from user mode to user mode with SMAP Paolo Bonzini
@ 2024-07-17 5:03 ` Paolo Bonzini
2024-07-17 5:03 ` [PULL 15/20] target/i386/tcg: Reorg push/pop within seg_helper.c Paolo Bonzini
` (6 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2024-07-17 5:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Richard Henderson
Do not pre-decrement esp, let the macros subtract the appropriate
operand size.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/seg_helper.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c
index 224e73e9ed0..b985382d704 100644
--- a/target/i386/tcg/seg_helper.c
+++ b/target/i386/tcg/seg_helper.c
@@ -670,22 +670,20 @@ static void do_interrupt_protected(CPUX86State *env, int intno, int is_int,
}
shift = switch_tss(env, intno * 8, e1, e2, SWITCH_TSS_CALL, old_eip);
if (has_error_code) {
- uint32_t mask;
-
/* push the error code */
if (env->segs[R_SS].flags & DESC_B_MASK) {
- mask = 0xffffffff;
+ sp_mask = 0xffffffff;
} else {
- mask = 0xffff;
+ sp_mask = 0xffff;
}
- esp = (env->regs[R_ESP] - (2 << shift)) & mask;
- ssp = env->segs[R_SS].base + esp;
+ esp = env->regs[R_ESP];
+ ssp = env->segs[R_SS].base;
if (shift) {
- cpu_stl_kernel(env, ssp, error_code);
+ PUSHL(ssp, esp, sp_mask, error_code);
} else {
- cpu_stw_kernel(env, ssp, error_code);
+ PUSHW(ssp, esp, sp_mask, error_code);
}
- SET_ESP(esp, mask);
+ SET_ESP(esp, sp_mask);
}
return;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PULL 15/20] target/i386/tcg: Reorg push/pop within seg_helper.c
2024-07-17 5:03 [PULL 00/20] i386, bugfix changes for QEMU 9.1 soft freeze Paolo Bonzini
` (13 preceding siblings ...)
2024-07-17 5:03 ` [PULL 14/20] target/i386/tcg: use PUSHL/PUSHW for error code Paolo Bonzini
@ 2024-07-17 5:03 ` Paolo Bonzini
2024-07-17 5:03 ` [PULL 16/20] target/i386/tcg: Introduce x86_mmu_index_{kernel_,}pl Paolo Bonzini
` (5 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2024-07-17 5:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Richard Henderson
From: Richard Henderson <richard.henderson@linaro.org>
Interrupts and call gates should use accesses with the DPL as
the privilege level. While computing the applicable MMU index
is easy, the harder thing is how to plumb it in the code.
One possibility could be to add a single argument to the PUSH* macros
for the privilege level, but this is repetitive and risks confusion
between the involved privilege levels.
Another possibility is to pass both CPL and DPL, and adjusting both
PUSH* and POP* to use specific privilege levels (instead of using
cpu_{ld,st}*_data). This makes the code more symmetric.
However, a more complicated but much nicer approach is to use a structure
to contain the stack parameters, env, unwind return address, and rewrite
the macros into functions. The struct provides an easy home for the MMU
index as well.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Link: https://lore.kernel.org/r/20240617161210.4639-4-richard.henderson@linaro.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/seg_helper.c | 481 +++++++++++++++++++----------------
1 file changed, 259 insertions(+), 222 deletions(-)
diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c
index b985382d704..b6902ca3fba 100644
--- a/target/i386/tcg/seg_helper.c
+++ b/target/i386/tcg/seg_helper.c
@@ -28,6 +28,68 @@
#include "helper-tcg.h"
#include "seg_helper.h"
+#ifdef TARGET_X86_64
+#define SET_ESP(val, sp_mask) \
+ do { \
+ if ((sp_mask) == 0xffff) { \
+ env->regs[R_ESP] = (env->regs[R_ESP] & ~0xffff) | \
+ ((val) & 0xffff); \
+ } else if ((sp_mask) == 0xffffffffLL) { \
+ env->regs[R_ESP] = (uint32_t)(val); \
+ } else { \
+ env->regs[R_ESP] = (val); \
+ } \
+ } while (0)
+#else
+#define SET_ESP(val, sp_mask) \
+ do { \
+ env->regs[R_ESP] = (env->regs[R_ESP] & ~(sp_mask)) | \
+ ((val) & (sp_mask)); \
+ } while (0)
+#endif
+
+/* XXX: use mmu_index to have proper DPL support */
+typedef struct StackAccess
+{
+ CPUX86State *env;
+ uintptr_t ra;
+ target_ulong ss_base;
+ target_ulong sp;
+ target_ulong sp_mask;
+} StackAccess;
+
+static void pushw(StackAccess *sa, uint16_t val)
+{
+ sa->sp -= 2;
+ cpu_stw_kernel_ra(sa->env, sa->ss_base + (sa->sp & sa->sp_mask),
+ val, sa->ra);
+}
+
+static void pushl(StackAccess *sa, uint32_t val)
+{
+ sa->sp -= 4;
+ cpu_stl_kernel_ra(sa->env, sa->ss_base + (sa->sp & sa->sp_mask),
+ val, sa->ra);
+}
+
+static uint16_t popw(StackAccess *sa)
+{
+ uint16_t ret = cpu_lduw_data_ra(sa->env,
+ sa->ss_base + (sa->sp & sa->sp_mask),
+ sa->ra);
+ sa->sp += 2;
+ return ret;
+}
+
+static uint32_t popl(StackAccess *sa)
+{
+ uint32_t ret = cpu_ldl_data_ra(sa->env,
+ sa->ss_base + (sa->sp & sa->sp_mask),
+ sa->ra);
+ sa->sp += 4;
+ return ret;
+}
+
int get_pg_mode(CPUX86State *env)
{
int pg_mode = 0;
@@ -559,68 +621,19 @@ int exception_has_error_code(int intno)
return 0;
}
-#ifdef TARGET_X86_64
-#define SET_ESP(val, sp_mask) \
- do { \
- if ((sp_mask) == 0xffff) { \
- env->regs[R_ESP] = (env->regs[R_ESP] & ~0xffff) | \
- ((val) & 0xffff); \
- } else if ((sp_mask) == 0xffffffffLL) { \
- env->regs[R_ESP] = (uint32_t)(val); \
- } else { \
- env->regs[R_ESP] = (val); \
- } \
- } while (0)
-#else
-#define SET_ESP(val, sp_mask) \
- do { \
- env->regs[R_ESP] = (env->regs[R_ESP] & ~(sp_mask)) | \
- ((val) & (sp_mask)); \
- } while (0)
-#endif
-
-/* XXX: add a is_user flag to have proper security support */
-#define PUSHW_RA(ssp, sp, sp_mask, val, ra) \
- { \
- sp -= 2; \
- cpu_stw_kernel_ra(env, (ssp) + (sp & (sp_mask)), (val), ra); \
- }
-
-#define PUSHL_RA(ssp, sp, sp_mask, val, ra) \
- { \
- sp -= 4; \
- cpu_stl_kernel_ra(env, (ssp) + (sp & (sp_mask)), (val), ra); \
- }
-
-#define POPW_RA(ssp, sp, sp_mask, val, ra) \
- { \
- val = cpu_lduw_data_ra(env, (ssp) + (sp & (sp_mask)), ra); \
- sp += 2; \
- }
-
-#define POPL_RA(ssp, sp, sp_mask, val, ra) \
- { \
- val = (uint32_t)cpu_ldl_data_ra(env, (ssp) + (sp & (sp_mask)), ra); \
- sp += 4; \
- }
-
-#define PUSHW(ssp, sp, sp_mask, val) PUSHW_RA(ssp, sp, sp_mask, val, 0)
-#define PUSHL(ssp, sp, sp_mask, val) PUSHL_RA(ssp, sp, sp_mask, val, 0)
-#define POPW(ssp, sp, sp_mask, val) POPW_RA(ssp, sp, sp_mask, val, 0)
-#define POPL(ssp, sp, sp_mask, val) POPL_RA(ssp, sp, sp_mask, val, 0)
-
/* protected mode interrupt */
static void do_interrupt_protected(CPUX86State *env, int intno, int is_int,
int error_code, unsigned int next_eip,
int is_hw)
{
SegmentCache *dt;
- target_ulong ptr, ssp;
+ target_ulong ptr;
int type, dpl, selector, ss_dpl, cpl;
int has_error_code, new_stack, shift;
- uint32_t e1, e2, offset, ss = 0, esp, ss_e1 = 0, ss_e2 = 0;
- uint32_t old_eip, sp_mask, eflags;
+ uint32_t e1, e2, offset, ss = 0, ss_e1 = 0, ss_e2 = 0;
+ uint32_t old_eip, eflags;
int vm86 = env->eflags & VM_MASK;
+ StackAccess sa;
bool set_rf;
has_error_code = 0;
@@ -662,6 +675,9 @@ static void do_interrupt_protected(CPUX86State *env, int intno, int is_int,
raise_exception_err(env, EXCP0D_GPF, intno * 8 + 2);
}
+ sa.env = env;
+ sa.ra = 0;
+
if (type == 5) {
/* task gate */
/* must do that check here to return the correct error code */
@@ -672,18 +688,18 @@ static void do_interrupt_protected(CPUX86State *env, int intno, int is_int,
if (has_error_code) {
/* push the error code */
if (env->segs[R_SS].flags & DESC_B_MASK) {
- sp_mask = 0xffffffff;
+ sa.sp_mask = 0xffffffff;
} else {
- sp_mask = 0xffff;
+ sa.sp_mask = 0xffff;
}
- esp = env->regs[R_ESP];
- ssp = env->segs[R_SS].base;
+ sa.sp = env->regs[R_ESP];
+ sa.ss_base = env->segs[R_SS].base;
if (shift) {
- PUSHL(ssp, esp, sp_mask, error_code);
+ pushl(&sa, error_code);
} else {
- PUSHW(ssp, esp, sp_mask, error_code);
+ pushw(&sa, error_code);
}
- SET_ESP(esp, sp_mask);
+ SET_ESP(sa.sp, sa.sp_mask);
}
return;
}
@@ -717,6 +733,7 @@ static void do_interrupt_protected(CPUX86State *env, int intno, int is_int,
}
if (dpl < cpl) {
/* to inner privilege */
+ uint32_t esp;
get_ss_esp_from_tss(env, &ss, &esp, dpl, 0);
if ((ss & 0xfffc) == 0) {
raise_exception_err(env, EXCP0A_TSS, ss & 0xfffc);
@@ -740,17 +757,18 @@ static void do_interrupt_protected(CPUX86State *env, int intno, int is_int,
raise_exception_err(env, EXCP0A_TSS, ss & 0xfffc);
}
new_stack = 1;
- sp_mask = get_sp_mask(ss_e2);
- ssp = get_seg_base(ss_e1, ss_e2);
+ sa.sp = esp;
+ sa.sp_mask = get_sp_mask(ss_e2);
+ sa.ss_base = get_seg_base(ss_e1, ss_e2);
} else {
/* to same privilege */
if (vm86) {
raise_exception_err(env, EXCP0D_GPF, selector & 0xfffc);
}
new_stack = 0;
- sp_mask = get_sp_mask(env->segs[R_SS].flags);
- ssp = env->segs[R_SS].base;
- esp = env->regs[R_ESP];
+ sa.sp = env->regs[R_ESP];
+ sa.sp_mask = get_sp_mask(env->segs[R_SS].flags);
+ sa.ss_base = env->segs[R_SS].base;
}
shift = type >> 3;
@@ -775,36 +793,36 @@ static void do_interrupt_protected(CPUX86State *env, int intno, int is_int,
if (shift == 1) {
if (new_stack) {
if (vm86) {
- PUSHL(ssp, esp, sp_mask, env->segs[R_GS].selector);
- PUSHL(ssp, esp, sp_mask, env->segs[R_FS].selector);
- PUSHL(ssp, esp, sp_mask, env->segs[R_DS].selector);
- PUSHL(ssp, esp, sp_mask, env->segs[R_ES].selector);
+ pushl(&sa, env->segs[R_GS].selector);
+ pushl(&sa, env->segs[R_FS].selector);
+ pushl(&sa, env->segs[R_DS].selector);
+ pushl(&sa, env->segs[R_ES].selector);
}
- PUSHL(ssp, esp, sp_mask, env->segs[R_SS].selector);
- PUSHL(ssp, esp, sp_mask, env->regs[R_ESP]);
+ pushl(&sa, env->segs[R_SS].selector);
+ pushl(&sa, env->regs[R_ESP]);
}
- PUSHL(ssp, esp, sp_mask, eflags);
- PUSHL(ssp, esp, sp_mask, env->segs[R_CS].selector);
- PUSHL(ssp, esp, sp_mask, old_eip);
+ pushl(&sa, eflags);
+ pushl(&sa, env->segs[R_CS].selector);
+ pushl(&sa, old_eip);
if (has_error_code) {
- PUSHL(ssp, esp, sp_mask, error_code);
+ pushl(&sa, error_code);
}
} else {
if (new_stack) {
if (vm86) {
- PUSHW(ssp, esp, sp_mask, env->segs[R_GS].selector);
- PUSHW(ssp, esp, sp_mask, env->segs[R_FS].selector);
- PUSHW(ssp, esp, sp_mask, env->segs[R_DS].selector);
- PUSHW(ssp, esp, sp_mask, env->segs[R_ES].selector);
+ pushw(&sa, env->segs[R_GS].selector);
+ pushw(&sa, env->segs[R_FS].selector);
+ pushw(&sa, env->segs[R_DS].selector);
+ pushw(&sa, env->segs[R_ES].selector);
}
- PUSHW(ssp, esp, sp_mask, env->segs[R_SS].selector);
- PUSHW(ssp, esp, sp_mask, env->regs[R_ESP]);
+ pushw(&sa, env->segs[R_SS].selector);
+ pushw(&sa, env->regs[R_ESP]);
}
- PUSHW(ssp, esp, sp_mask, eflags);
- PUSHW(ssp, esp, sp_mask, env->segs[R_CS].selector);
- PUSHW(ssp, esp, sp_mask, old_eip);
+ pushw(&sa, eflags);
+ pushw(&sa, env->segs[R_CS].selector);
+ pushw(&sa, old_eip);
if (has_error_code) {
- PUSHW(ssp, esp, sp_mask, error_code);
+ pushw(&sa, error_code);
}
}
@@ -822,10 +840,10 @@ static void do_interrupt_protected(CPUX86State *env, int intno, int is_int,
cpu_x86_load_seg_cache(env, R_GS, 0, 0, 0, 0);
}
ss = (ss & ~3) | dpl;
- cpu_x86_load_seg_cache(env, R_SS, ss,
- ssp, get_seg_limit(ss_e1, ss_e2), ss_e2);
+ cpu_x86_load_seg_cache(env, R_SS, ss, sa.ss_base,
+ get_seg_limit(ss_e1, ss_e2), ss_e2);
}
- SET_ESP(esp, sp_mask);
+ SET_ESP(sa.sp, sa.sp_mask);
selector = (selector & ~3) | dpl;
cpu_x86_load_seg_cache(env, R_CS, selector,
@@ -837,20 +855,18 @@ static void do_interrupt_protected(CPUX86State *env, int intno, int is_int,
#ifdef TARGET_X86_64
-#define PUSHQ_RA(sp, val, ra) \
- { \
- sp -= 8; \
- cpu_stq_kernel_ra(env, sp, (val), ra); \
- }
+static void pushq(StackAccess *sa, uint64_t val)
+{
+ sa->sp -= 8;
+ cpu_stq_kernel_ra(sa->env, sa->sp, val, sa->ra);
+}
-#define POPQ_RA(sp, val, ra) \
- { \
- val = cpu_ldq_data_ra(env, sp, ra); \
- sp += 8; \
- }
-
-#define PUSHQ(sp, val) PUSHQ_RA(sp, val, 0)
-#define POPQ(sp, val) POPQ_RA(sp, val, 0)
+static uint64_t popq(StackAccess *sa)
+{
+ uint64_t ret = cpu_ldq_data_ra(sa->env, sa->sp, sa->ra);
+ sa->sp += 8;
+ return ret;
+}
static inline target_ulong get_rsp_from_tss(CPUX86State *env, int level)
{
@@ -893,8 +909,9 @@ static void do_interrupt64(CPUX86State *env, int intno, int is_int,
int type, dpl, selector, cpl, ist;
int has_error_code, new_stack;
uint32_t e1, e2, e3, ss, eflags;
- target_ulong old_eip, esp, offset;
+ target_ulong old_eip, offset;
bool set_rf;
+ StackAccess sa;
has_error_code = 0;
if (!is_int && !is_hw) {
@@ -962,10 +979,15 @@ static void do_interrupt64(CPUX86State *env, int intno, int is_int,
if (e2 & DESC_C_MASK) {
dpl = cpl;
}
+
+ sa.env = env;
+ sa.ra = 0;
+ sa.sp_mask = -1;
+ sa.ss_base = 0;
if (dpl < cpl || ist != 0) {
/* to inner privilege */
new_stack = 1;
- esp = get_rsp_from_tss(env, ist != 0 ? ist + 3 : dpl);
+ sa.sp = get_rsp_from_tss(env, ist != 0 ? ist + 3 : dpl);
ss = 0;
} else {
/* to same privilege */
@@ -973,9 +995,9 @@ static void do_interrupt64(CPUX86State *env, int intno, int is_int,
raise_exception_err(env, EXCP0D_GPF, selector & 0xfffc);
}
new_stack = 0;
- esp = env->regs[R_ESP];
+ sa.sp = env->regs[R_ESP];
}
- esp &= ~0xfLL; /* align stack */
+ sa.sp &= ~0xfLL; /* align stack */
/* See do_interrupt_protected. */
eflags = cpu_compute_eflags(env);
@@ -983,13 +1005,13 @@ static void do_interrupt64(CPUX86State *env, int intno, int is_int,
eflags |= RF_MASK;
}
- PUSHQ(esp, env->segs[R_SS].selector);
- PUSHQ(esp, env->regs[R_ESP]);
- PUSHQ(esp, eflags);
- PUSHQ(esp, env->segs[R_CS].selector);
- PUSHQ(esp, old_eip);
+ pushq(&sa, env->segs[R_SS].selector);
+ pushq(&sa, env->regs[R_ESP]);
+ pushq(&sa, eflags);
+ pushq(&sa, env->segs[R_CS].selector);
+ pushq(&sa, old_eip);
if (has_error_code) {
- PUSHQ(esp, error_code);
+ pushq(&sa, error_code);
}
/* interrupt gate clear IF mask */
@@ -1002,7 +1024,7 @@ static void do_interrupt64(CPUX86State *env, int intno, int is_int,
ss = 0 | dpl;
cpu_x86_load_seg_cache(env, R_SS, ss, 0, 0, dpl << DESC_DPL_SHIFT);
}
- env->regs[R_ESP] = esp;
+ env->regs[R_ESP] = sa.sp;
selector = (selector & ~3) | dpl;
cpu_x86_load_seg_cache(env, R_CS, selector,
@@ -1074,10 +1096,11 @@ static void do_interrupt_real(CPUX86State *env, int intno, int is_int,
int error_code, unsigned int next_eip)
{
SegmentCache *dt;
- target_ulong ptr, ssp;
+ target_ulong ptr;
int selector;
- uint32_t offset, esp;
+ uint32_t offset;
uint32_t old_cs, old_eip;
+ StackAccess sa;
/* real mode (simpler!) */
dt = &env->idt;
@@ -1087,8 +1110,13 @@ static void do_interrupt_real(CPUX86State *env, int intno, int is_int,
ptr = dt->base + intno * 4;
offset = cpu_lduw_kernel(env, ptr);
selector = cpu_lduw_kernel(env, ptr + 2);
- esp = env->regs[R_ESP];
- ssp = env->segs[R_SS].base;
+
+ sa.env = env;
+ sa.ra = 0;
+ sa.sp = env->regs[R_ESP];
+ sa.sp_mask = 0xffff;
+ sa.ss_base = env->segs[R_SS].base;
+
if (is_int) {
old_eip = next_eip;
} else {
@@ -1096,12 +1124,12 @@ static void do_interrupt_real(CPUX86State *env, int intno, int is_int,
}
old_cs = env->segs[R_CS].selector;
/* XXX: use SS segment size? */
- PUSHW(ssp, esp, 0xffff, cpu_compute_eflags(env));
- PUSHW(ssp, esp, 0xffff, old_cs);
- PUSHW(ssp, esp, 0xffff, old_eip);
+ pushw(&sa, cpu_compute_eflags(env));
+ pushw(&sa, old_cs);
+ pushw(&sa, old_eip);
/* update processor state */
- env->regs[R_ESP] = (env->regs[R_ESP] & ~0xffff) | (esp & 0xffff);
+ SET_ESP(sa.sp, sa.sp_mask);
env->eip = offset;
env->segs[R_CS].selector = selector;
env->segs[R_CS].base = (selector << 4);
@@ -1544,21 +1572,23 @@ void helper_ljmp_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
void helper_lcall_real(CPUX86State *env, uint32_t new_cs, uint32_t new_eip,
int shift, uint32_t next_eip)
{
- uint32_t esp, esp_mask;
- target_ulong ssp;
+ StackAccess sa;
+
+ sa.env = env;
+ sa.ra = GETPC();
+ sa.sp = env->regs[R_ESP];
+ sa.sp_mask = get_sp_mask(env->segs[R_SS].flags);
+ sa.ss_base = env->segs[R_SS].base;
- esp = env->regs[R_ESP];
- esp_mask = get_sp_mask(env->segs[R_SS].flags);
- ssp = env->segs[R_SS].base;
if (shift) {
- PUSHL_RA(ssp, esp, esp_mask, env->segs[R_CS].selector, GETPC());
- PUSHL_RA(ssp, esp, esp_mask, next_eip, GETPC());
+ pushl(&sa, env->segs[R_CS].selector);
+ pushl(&sa, next_eip);
} else {
- PUSHW_RA(ssp, esp, esp_mask, env->segs[R_CS].selector, GETPC());
- PUSHW_RA(ssp, esp, esp_mask, next_eip, GETPC());
+ pushw(&sa, env->segs[R_CS].selector);
+ pushw(&sa, next_eip);
}
- SET_ESP(esp, esp_mask);
+ SET_ESP(sa.sp, sa.sp_mask);
env->eip = new_eip;
env->segs[R_CS].selector = new_cs;
env->segs[R_CS].base = (new_cs << 4);
@@ -1570,9 +1600,10 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
{
int new_stack, i;
uint32_t e1, e2, cpl, dpl, rpl, selector, param_count;
- uint32_t ss = 0, ss_e1 = 0, ss_e2 = 0, type, ss_dpl, sp_mask;
+ uint32_t ss = 0, ss_e1 = 0, ss_e2 = 0, type, ss_dpl;
uint32_t val, limit, old_sp_mask;
- target_ulong ssp, old_ssp, offset, sp;
+ target_ulong old_ssp, offset;
+ StackAccess sa;
LOG_PCALL("lcall %04x:" TARGET_FMT_lx " s=%d\n", new_cs, new_eip, shift);
LOG_PCALL_STATE(env_cpu(env));
@@ -1584,6 +1615,10 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
}
cpl = env->hflags & HF_CPL_MASK;
LOG_PCALL("desc=%08x:%08x\n", e1, e2);
+
+ sa.env = env;
+ sa.ra = GETPC();
+
if (e2 & DESC_S_MASK) {
if (!(e2 & DESC_CS_MASK)) {
raise_exception_err_ra(env, EXCP0D_GPF, new_cs & 0xfffc, GETPC());
@@ -1611,14 +1646,14 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
#ifdef TARGET_X86_64
/* XXX: check 16/32 bit cases in long mode */
if (shift == 2) {
- target_ulong rsp;
-
/* 64 bit case */
- rsp = env->regs[R_ESP];
- PUSHQ_RA(rsp, env->segs[R_CS].selector, GETPC());
- PUSHQ_RA(rsp, next_eip, GETPC());
+ sa.sp = env->regs[R_ESP];
+ sa.sp_mask = -1;
+ sa.ss_base = 0;
+ pushq(&sa, env->segs[R_CS].selector);
+ pushq(&sa, next_eip);
/* from this point, not restartable */
- env->regs[R_ESP] = rsp;
+ env->regs[R_ESP] = sa.sp;
cpu_x86_load_seg_cache(env, R_CS, (new_cs & 0xfffc) | cpl,
get_seg_base(e1, e2),
get_seg_limit(e1, e2), e2);
@@ -1626,15 +1661,15 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
} else
#endif
{
- sp = env->regs[R_ESP];
- sp_mask = get_sp_mask(env->segs[R_SS].flags);
- ssp = env->segs[R_SS].base;
+ sa.sp = env->regs[R_ESP];
+ sa.sp_mask = get_sp_mask(env->segs[R_SS].flags);
+ sa.ss_base = env->segs[R_SS].base;
if (shift) {
- PUSHL_RA(ssp, sp, sp_mask, env->segs[R_CS].selector, GETPC());
- PUSHL_RA(ssp, sp, sp_mask, next_eip, GETPC());
+ pushl(&sa, env->segs[R_CS].selector);
+ pushl(&sa, next_eip);
} else {
- PUSHW_RA(ssp, sp, sp_mask, env->segs[R_CS].selector, GETPC());
- PUSHW_RA(ssp, sp, sp_mask, next_eip, GETPC());
+ pushw(&sa, env->segs[R_CS].selector);
+ pushw(&sa, next_eip);
}
limit = get_seg_limit(e1, e2);
@@ -1642,7 +1677,7 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
raise_exception_err_ra(env, EXCP0D_GPF, new_cs & 0xfffc, GETPC());
}
/* from this point, not restartable */
- SET_ESP(sp, sp_mask);
+ SET_ESP(sa.sp, sa.sp_mask);
cpu_x86_load_seg_cache(env, R_CS, (new_cs & 0xfffc) | cpl,
get_seg_base(e1, e2), limit, e2);
env->eip = new_eip;
@@ -1737,13 +1772,13 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
/* to inner privilege */
#ifdef TARGET_X86_64
if (shift == 2) {
- sp = get_rsp_from_tss(env, dpl);
ss = dpl; /* SS = NULL selector with RPL = new CPL */
new_stack = 1;
- sp_mask = 0;
- ssp = 0; /* SS base is always zero in IA-32e mode */
+ sa.sp = get_rsp_from_tss(env, dpl);
+ sa.sp_mask = -1;
+ sa.ss_base = 0; /* SS base is always zero in IA-32e mode */
LOG_PCALL("new ss:rsp=%04x:%016llx env->regs[R_ESP]="
- TARGET_FMT_lx "\n", ss, sp, env->regs[R_ESP]);
+ TARGET_FMT_lx "\n", ss, sa.sp, env->regs[R_ESP]);
} else
#endif
{
@@ -1752,7 +1787,6 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
LOG_PCALL("new ss:esp=%04x:%08x param_count=%d env->regs[R_ESP]="
TARGET_FMT_lx "\n", ss, sp32, param_count,
env->regs[R_ESP]);
- sp = sp32;
if ((ss & 0xfffc) == 0) {
raise_exception_err_ra(env, EXCP0A_TSS, ss & 0xfffc, GETPC());
}
@@ -1775,63 +1809,64 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
raise_exception_err_ra(env, EXCP0A_TSS, ss & 0xfffc, GETPC());
}
- sp_mask = get_sp_mask(ss_e2);
- ssp = get_seg_base(ss_e1, ss_e2);
+ sa.sp = sp32;
+ sa.sp_mask = get_sp_mask(ss_e2);
+ sa.ss_base = get_seg_base(ss_e1, ss_e2);
}
/* push_size = ((param_count * 2) + 8) << shift; */
-
old_sp_mask = get_sp_mask(env->segs[R_SS].flags);
old_ssp = env->segs[R_SS].base;
+
#ifdef TARGET_X86_64
if (shift == 2) {
/* XXX: verify if new stack address is canonical */
- PUSHQ_RA(sp, env->segs[R_SS].selector, GETPC());
- PUSHQ_RA(sp, env->regs[R_ESP], GETPC());
+ pushq(&sa, env->segs[R_SS].selector);
+ pushq(&sa, env->regs[R_ESP]);
/* parameters aren't supported for 64-bit call gates */
} else
#endif
if (shift == 1) {
- PUSHL_RA(ssp, sp, sp_mask, env->segs[R_SS].selector, GETPC());
- PUSHL_RA(ssp, sp, sp_mask, env->regs[R_ESP], GETPC());
+ pushl(&sa, env->segs[R_SS].selector);
+ pushl(&sa, env->regs[R_ESP]);
for (i = param_count - 1; i >= 0; i--) {
val = cpu_ldl_data_ra(env,
old_ssp + ((env->regs[R_ESP] + i * 4) & old_sp_mask),
GETPC());
- PUSHL_RA(ssp, sp, sp_mask, val, GETPC());
+ pushl(&sa, val);
}
} else {
- PUSHW_RA(ssp, sp, sp_mask, env->segs[R_SS].selector, GETPC());
- PUSHW_RA(ssp, sp, sp_mask, env->regs[R_ESP], GETPC());
+ pushw(&sa, env->segs[R_SS].selector);
+ pushw(&sa, env->regs[R_ESP]);
for (i = param_count - 1; i >= 0; i--) {
val = cpu_lduw_data_ra(env,
old_ssp + ((env->regs[R_ESP] + i * 2) & old_sp_mask),
GETPC());
- PUSHW_RA(ssp, sp, sp_mask, val, GETPC());
+ pushw(&sa, val);
}
}
new_stack = 1;
} else {
/* to same privilege */
- sp = env->regs[R_ESP];
- sp_mask = get_sp_mask(env->segs[R_SS].flags);
- ssp = env->segs[R_SS].base;
+ sa.sp = env->regs[R_ESP];
+ sa.sp_mask = get_sp_mask(env->segs[R_SS].flags);
+ sa.ss_base = env->segs[R_SS].base;
/* push_size = (4 << shift); */
new_stack = 0;
}
#ifdef TARGET_X86_64
if (shift == 2) {
- PUSHQ_RA(sp, env->segs[R_CS].selector, GETPC());
- PUSHQ_RA(sp, next_eip, GETPC());
+ pushq(&sa, env->segs[R_CS].selector);
+ pushq(&sa, next_eip);
} else
#endif
if (shift == 1) {
- PUSHL_RA(ssp, sp, sp_mask, env->segs[R_CS].selector, GETPC());
- PUSHL_RA(ssp, sp, sp_mask, next_eip, GETPC());
+ pushl(&sa, env->segs[R_CS].selector);
+ pushl(&sa, next_eip);
} else {
- PUSHW_RA(ssp, sp, sp_mask, env->segs[R_CS].selector, GETPC());
- PUSHW_RA(ssp, sp, sp_mask, next_eip, GETPC());
+ pushw(&sa, env->segs[R_CS].selector);
+ pushw(&sa, next_eip);
}
/* from this point, not restartable */
@@ -1845,7 +1880,7 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
{
ss = (ss & ~3) | dpl;
cpu_x86_load_seg_cache(env, R_SS, ss,
- ssp,
+ sa.ss_base,
get_seg_limit(ss_e1, ss_e2),
ss_e2);
}
@@ -1856,7 +1891,7 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
get_seg_base(e1, e2),
get_seg_limit(e1, e2),
e2);
- SET_ESP(sp, sp_mask);
+ SET_ESP(sa.sp, sa.sp_mask);
env->eip = offset;
}
}
@@ -1864,26 +1899,28 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
/* real and vm86 mode iret */
void helper_iret_real(CPUX86State *env, int shift)
{
- uint32_t sp, new_cs, new_eip, new_eflags, sp_mask;
- target_ulong ssp;
+ uint32_t new_cs, new_eip, new_eflags;
int eflags_mask;
+ StackAccess sa;
+
+ sa.env = env;
+ sa.ra = GETPC();
+ sa.sp_mask = 0xffff; /* XXXX: use SS segment size? */
+ sa.sp = env->regs[R_ESP];
+ sa.ss_base = env->segs[R_SS].base;
- sp_mask = 0xffff; /* XXXX: use SS segment size? */
- sp = env->regs[R_ESP];
- ssp = env->segs[R_SS].base;
if (shift == 1) {
/* 32 bits */
- POPL_RA(ssp, sp, sp_mask, new_eip, GETPC());
- POPL_RA(ssp, sp, sp_mask, new_cs, GETPC());
- new_cs &= 0xffff;
- POPL_RA(ssp, sp, sp_mask, new_eflags, GETPC());
+ new_eip = popl(&sa);
+ new_cs = popl(&sa) & 0xffff;
+ new_eflags = popl(&sa);
} else {
/* 16 bits */
- POPW_RA(ssp, sp, sp_mask, new_eip, GETPC());
- POPW_RA(ssp, sp, sp_mask, new_cs, GETPC());
- POPW_RA(ssp, sp, sp_mask, new_eflags, GETPC());
+ new_eip = popw(&sa);
+ new_cs = popw(&sa);
+ new_eflags = popw(&sa);
}
- env->regs[R_ESP] = (env->regs[R_ESP] & ~sp_mask) | (sp & sp_mask);
+ SET_ESP(sa.sp, sa.sp_mask);
env->segs[R_CS].selector = new_cs;
env->segs[R_CS].base = (new_cs << 4);
env->eip = new_eip;
@@ -1936,47 +1973,49 @@ static inline void helper_ret_protected(CPUX86State *env, int shift,
uint32_t new_es, new_ds, new_fs, new_gs;
uint32_t e1, e2, ss_e1, ss_e2;
int cpl, dpl, rpl, eflags_mask, iopl;
- target_ulong ssp, sp, new_eip, new_esp, sp_mask;
+ target_ulong new_eip, new_esp;
+ StackAccess sa;
+
+ sa.env = env;
+ sa.ra = retaddr;
#ifdef TARGET_X86_64
if (shift == 2) {
- sp_mask = -1;
+ sa.sp_mask = -1;
} else
#endif
{
- sp_mask = get_sp_mask(env->segs[R_SS].flags);
+ sa.sp_mask = get_sp_mask(env->segs[R_SS].flags);
}
- sp = env->regs[R_ESP];
- ssp = env->segs[R_SS].base;
+ sa.sp = env->regs[R_ESP];
+ sa.ss_base = env->segs[R_SS].base;
new_eflags = 0; /* avoid warning */
#ifdef TARGET_X86_64
if (shift == 2) {
- POPQ_RA(sp, new_eip, retaddr);
- POPQ_RA(sp, new_cs, retaddr);
- new_cs &= 0xffff;
+ new_eip = popq(&sa);
+ new_cs = popq(&sa) & 0xffff;
if (is_iret) {
- POPQ_RA(sp, new_eflags, retaddr);
+ new_eflags = popq(&sa);
}
} else
#endif
{
if (shift == 1) {
/* 32 bits */
- POPL_RA(ssp, sp, sp_mask, new_eip, retaddr);
- POPL_RA(ssp, sp, sp_mask, new_cs, retaddr);
- new_cs &= 0xffff;
+ new_eip = popl(&sa);
+ new_cs = popl(&sa) & 0xffff;
if (is_iret) {
- POPL_RA(ssp, sp, sp_mask, new_eflags, retaddr);
+ new_eflags = popl(&sa);
if (new_eflags & VM_MASK) {
goto return_to_vm86;
}
}
} else {
/* 16 bits */
- POPW_RA(ssp, sp, sp_mask, new_eip, retaddr);
- POPW_RA(ssp, sp, sp_mask, new_cs, retaddr);
+ new_eip = popw(&sa);
+ new_cs = popw(&sa);
if (is_iret) {
- POPW_RA(ssp, sp, sp_mask, new_eflags, retaddr);
+ new_eflags = popw(&sa);
}
}
}
@@ -2012,7 +2051,7 @@ static inline void helper_ret_protected(CPUX86State *env, int shift,
raise_exception_err_ra(env, EXCP0B_NOSEG, new_cs & 0xfffc, retaddr);
}
- sp += addend;
+ sa.sp += addend;
if (rpl == cpl && (!(env->hflags & HF_CS64_MASK) ||
((env->hflags & HF_CS64_MASK) && !is_iret))) {
/* return to same privilege level */
@@ -2024,21 +2063,19 @@ static inline void helper_ret_protected(CPUX86State *env, int shift,
/* return to different privilege level */
#ifdef TARGET_X86_64
if (shift == 2) {
- POPQ_RA(sp, new_esp, retaddr);
- POPQ_RA(sp, new_ss, retaddr);
- new_ss &= 0xffff;
+ new_esp = popq(&sa);
+ new_ss = popq(&sa) & 0xffff;
} else
#endif
{
if (shift == 1) {
/* 32 bits */
- POPL_RA(ssp, sp, sp_mask, new_esp, retaddr);
- POPL_RA(ssp, sp, sp_mask, new_ss, retaddr);
- new_ss &= 0xffff;
+ new_esp = popl(&sa);
+ new_ss = popl(&sa) & 0xffff;
} else {
/* 16 bits */
- POPW_RA(ssp, sp, sp_mask, new_esp, retaddr);
- POPW_RA(ssp, sp, sp_mask, new_ss, retaddr);
+ new_esp = popw(&sa);
+ new_ss = popw(&sa);
}
}
LOG_PCALL("new ss:esp=%04x:" TARGET_FMT_lx "\n",
@@ -2088,14 +2125,14 @@ static inline void helper_ret_protected(CPUX86State *env, int shift,
get_seg_base(e1, e2),
get_seg_limit(e1, e2),
e2);
- sp = new_esp;
+ sa.sp = new_esp;
#ifdef TARGET_X86_64
if (env->hflags & HF_CS64_MASK) {
- sp_mask = -1;
+ sa.sp_mask = -1;
} else
#endif
{
- sp_mask = get_sp_mask(ss_e2);
+ sa.sp_mask = get_sp_mask(ss_e2);
}
/* validate data segments */
@@ -2104,9 +2141,9 @@ static inline void helper_ret_protected(CPUX86State *env, int shift,
validate_seg(env, R_FS, rpl);
validate_seg(env, R_GS, rpl);
- sp += addend;
+ sa.sp += addend;
}
- SET_ESP(sp, sp_mask);
+ SET_ESP(sa.sp, sa.sp_mask);
env->eip = new_eip;
if (is_iret) {
/* NOTE: 'cpl' is the _old_ CPL */
@@ -2126,12 +2163,12 @@ static inline void helper_ret_protected(CPUX86State *env, int shift,
return;
return_to_vm86:
- POPL_RA(ssp, sp, sp_mask, new_esp, retaddr);
- POPL_RA(ssp, sp, sp_mask, new_ss, retaddr);
- POPL_RA(ssp, sp, sp_mask, new_es, retaddr);
- POPL_RA(ssp, sp, sp_mask, new_ds, retaddr);
- POPL_RA(ssp, sp, sp_mask, new_fs, retaddr);
- POPL_RA(ssp, sp, sp_mask, new_gs, retaddr);
+ new_esp = popl(&sa);
+ new_ss = popl(&sa);
+ new_es = popl(&sa);
+ new_ds = popl(&sa);
+ new_fs = popl(&sa);
+ new_gs = popl(&sa);
/* modify processor state */
cpu_load_eflags(env, new_eflags, TF_MASK | AC_MASK | ID_MASK |
--
2.45.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PULL 16/20] target/i386/tcg: Introduce x86_mmu_index_{kernel_,}pl
2024-07-17 5:03 [PULL 00/20] i386, bugfix changes for QEMU 9.1 soft freeze Paolo Bonzini
` (14 preceding siblings ...)
2024-07-17 5:03 ` [PULL 15/20] target/i386/tcg: Reorg push/pop within seg_helper.c Paolo Bonzini
@ 2024-07-17 5:03 ` Paolo Bonzini
2024-07-17 5:03 ` [PULL 17/20] target/i386/tcg: Compute MMU index once Paolo Bonzini
` (4 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2024-07-17 5:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Richard Henderson
From: Richard Henderson <richard.henderson@linaro.org>
Disconnect mmu index computation from the current pl
as stored in env->hflags.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Link: https://lore.kernel.org/r/20240617161210.4639-2-richard.henderson@linaro.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/cpu.h | 11 ++---------
target/i386/cpu.c | 27 ++++++++++++++++++++++++---
2 files changed, 26 insertions(+), 12 deletions(-)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index c43ac01c794..1e121acef54 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2445,15 +2445,8 @@ static inline bool is_mmu_index_32(int mmu_index)
return mmu_index & 1;
}
-static inline int cpu_mmu_index_kernel(CPUX86State *env)
-{
- int mmu_index_32 = (env->hflags & HF_LMA_MASK) ? 0 : 1;
- int mmu_index_base =
- !(env->hflags & HF_SMAP_MASK) ? MMU_KNOSMAP64_IDX :
- ((env->hflags & HF_CPL_MASK) < 3 && (env->eflags & AC_MASK)) ? MMU_KNOSMAP64_IDX : MMU_KSMAP64_IDX;
-
- return mmu_index_base + mmu_index_32;
-}
+int x86_mmu_index_pl(CPUX86State *env, unsigned pl);
+int cpu_mmu_index_kernel(CPUX86State *env);
#define CC_DST (env->cc_dst)
#define CC_SRC (env->cc_src)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index c05765eeafc..4688d140c2d 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -8122,18 +8122,39 @@ 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 *cs, bool ifetch)
+int x86_mmu_index_pl(CPUX86State *env, unsigned pl)
{
- CPUX86State *env = cpu_env(cs);
int mmu_index_32 = (env->hflags & HF_CS64_MASK) ? 0 : 1;
int mmu_index_base =
- (env->hflags & HF_CPL_MASK) == 3 ? MMU_USER64_IDX :
+ pl == 3 ? MMU_USER64_IDX :
!(env->hflags & HF_SMAP_MASK) ? MMU_KNOSMAP64_IDX :
(env->eflags & AC_MASK) ? MMU_KNOSMAP64_IDX : MMU_KSMAP64_IDX;
return mmu_index_base + mmu_index_32;
}
+static int x86_cpu_mmu_index(CPUState *cs, bool ifetch)
+{
+ CPUX86State *env = cpu_env(cs);
+ return x86_mmu_index_pl(env, env->hflags & HF_CPL_MASK);
+}
+
+static int x86_mmu_index_kernel_pl(CPUX86State *env, unsigned pl)
+{
+ int mmu_index_32 = (env->hflags & HF_LMA_MASK) ? 0 : 1;
+ int mmu_index_base =
+ !(env->hflags & HF_SMAP_MASK) ? MMU_KNOSMAP64_IDX :
+ (pl < 3 && (env->eflags & AC_MASK)
+ ? MMU_KNOSMAP64_IDX : MMU_KSMAP64_IDX);
+
+ return mmu_index_base + mmu_index_32;
+}
+
+int cpu_mmu_index_kernel(CPUX86State *env)
+{
+ return x86_mmu_index_kernel_pl(env, env->hflags & HF_CPL_MASK);
+}
+
static void x86_disas_set_info(CPUState *cs, disassemble_info *info)
{
X86CPU *cpu = X86_CPU(cs);
--
2.45.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PULL 17/20] target/i386/tcg: Compute MMU index once
2024-07-17 5:03 [PULL 00/20] i386, bugfix changes for QEMU 9.1 soft freeze Paolo Bonzini
` (15 preceding siblings ...)
2024-07-17 5:03 ` [PULL 16/20] target/i386/tcg: Introduce x86_mmu_index_{kernel_,}pl Paolo Bonzini
@ 2024-07-17 5:03 ` Paolo Bonzini
2024-07-17 5:03 ` [PULL 18/20] target/i386/tcg: check for correct busy state before switching to a new task Paolo Bonzini
` (3 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2024-07-17 5:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Richard Henderson
Add the MMU index to the StackAccess struct, so that it can be cached
or (in the next patch) computed from information that is not in
CPUX86State.
Co-developed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/seg_helper.c | 35 ++++++++++++++++++++++-------------
1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c
index b6902ca3fba..8a6d92b3583 100644
--- a/target/i386/tcg/seg_helper.c
+++ b/target/i386/tcg/seg_helper.c
@@ -56,36 +56,37 @@ typedef struct StackAccess
target_ulong ss_base;
target_ulong sp;
target_ulong sp_mask;
+ int mmu_index;
} StackAccess;
static void pushw(StackAccess *sa, uint16_t val)
{
sa->sp -= 2;
- cpu_stw_kernel_ra(sa->env, sa->ss_base + (sa->sp & sa->sp_mask),
- val, sa->ra);
+ cpu_stw_mmuidx_ra(sa->env, sa->ss_base + (sa->sp & sa->sp_mask),
+ val, sa->mmu_index, sa->ra);
}
static void pushl(StackAccess *sa, uint32_t val)
{
sa->sp -= 4;
- cpu_stl_kernel_ra(sa->env, sa->ss_base + (sa->sp & sa->sp_mask),
- val, sa->ra);
+ cpu_stl_mmuidx_ra(sa->env, sa->ss_base + (sa->sp & sa->sp_mask),
+ val, sa->mmu_index, sa->ra);
}
static uint16_t popw(StackAccess *sa)
{
- uint16_t ret = cpu_lduw_data_ra(sa->env,
- sa->ss_base + (sa->sp & sa->sp_mask),
- sa->ra);
+ uint16_t ret = cpu_lduw_mmuidx_ra(sa->env,
+ sa->ss_base + (sa->sp & sa->sp_mask),
+ sa->mmu_index, sa->ra);
sa->sp += 2;
return ret;
}
static uint32_t popl(StackAccess *sa)
{
- uint32_t ret = cpu_ldl_data_ra(sa->env,
- sa->ss_base + (sa->sp & sa->sp_mask),
- sa->ra);
+ uint32_t ret = cpu_ldl_mmuidx_ra(sa->env,
+ sa->ss_base + (sa->sp & sa->sp_mask),
+ sa->mmu_index, sa->ra);
sa->sp += 4;
return ret;
}
@@ -677,6 +678,7 @@ static void do_interrupt_protected(CPUX86State *env, int intno, int is_int,
sa.env = env;
sa.ra = 0;
+ sa.mmu_index = cpu_mmu_index_kernel(env);
if (type == 5) {
/* task gate */
@@ -858,12 +860,12 @@ static void do_interrupt_protected(CPUX86State *env, int intno, int is_int,
static void pushq(StackAccess *sa, uint64_t val)
{
sa->sp -= 8;
- cpu_stq_kernel_ra(sa->env, sa->sp, val, sa->ra);
+ cpu_stq_mmuidx_ra(sa->env, sa->sp, val, sa->mmu_index, sa->ra);
}
static uint64_t popq(StackAccess *sa)
{
- uint64_t ret = cpu_ldq_data_ra(sa->env, sa->sp, sa->ra);
+ uint64_t ret = cpu_ldq_mmuidx_ra(sa->env, sa->sp, sa->mmu_index, sa->ra);
sa->sp += 8;
return ret;
}
@@ -982,6 +984,7 @@ static void do_interrupt64(CPUX86State *env, int intno, int is_int,
sa.env = env;
sa.ra = 0;
+ sa.mmu_index = cpu_mmu_index_kernel(env);
sa.sp_mask = -1;
sa.ss_base = 0;
if (dpl < cpl || ist != 0) {
@@ -1116,6 +1119,7 @@ static void do_interrupt_real(CPUX86State *env, int intno, int is_int,
sa.sp = env->regs[R_ESP];
sa.sp_mask = 0xffff;
sa.ss_base = env->segs[R_SS].base;
+ sa.mmu_index = cpu_mmu_index_kernel(env);
if (is_int) {
old_eip = next_eip;
@@ -1579,6 +1583,7 @@ void helper_lcall_real(CPUX86State *env, uint32_t new_cs, uint32_t new_eip,
sa.sp = env->regs[R_ESP];
sa.sp_mask = get_sp_mask(env->segs[R_SS].flags);
sa.ss_base = env->segs[R_SS].base;
+ sa.mmu_index = cpu_mmu_index_kernel(env);
if (shift) {
pushl(&sa, env->segs[R_CS].selector);
@@ -1618,6 +1623,7 @@ void helper_lcall_protected(CPUX86State *env, int new_cs, target_ulong new_eip,
sa.env = env;
sa.ra = GETPC();
+ sa.mmu_index = cpu_mmu_index_kernel(env);
if (e2 & DESC_S_MASK) {
if (!(e2 & DESC_CS_MASK)) {
@@ -1905,6 +1911,7 @@ void helper_iret_real(CPUX86State *env, int shift)
sa.env = env;
sa.ra = GETPC();
+ sa.mmu_index = x86_mmu_index_pl(env, 0);
sa.sp_mask = 0xffff; /* XXXX: use SS segment size? */
sa.sp = env->regs[R_ESP];
sa.ss_base = env->segs[R_SS].base;
@@ -1976,8 +1983,11 @@ static inline void helper_ret_protected(CPUX86State *env, int shift,
target_ulong new_eip, new_esp;
StackAccess sa;
+ cpl = env->hflags & HF_CPL_MASK;
+
sa.env = env;
sa.ra = retaddr;
+ sa.mmu_index = x86_mmu_index_pl(env, cpl);
#ifdef TARGET_X86_64
if (shift == 2) {
@@ -2032,7 +2042,6 @@ static inline void helper_ret_protected(CPUX86State *env, int shift,
!(e2 & DESC_CS_MASK)) {
raise_exception_err_ra(env, EXCP0D_GPF, new_cs & 0xfffc, retaddr);
}
- cpl = env->hflags & HF_CPL_MASK;
rpl = new_cs & 3;
if (rpl < cpl) {
raise_exception_err_ra(env, EXCP0D_GPF, new_cs & 0xfffc, retaddr);
--
2.45.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PULL 18/20] target/i386/tcg: check for correct busy state before switching to a new task
2024-07-17 5:03 [PULL 00/20] i386, bugfix changes for QEMU 9.1 soft freeze Paolo Bonzini
` (16 preceding siblings ...)
2024-07-17 5:03 ` [PULL 17/20] target/i386/tcg: Compute MMU index once Paolo Bonzini
@ 2024-07-17 5:03 ` Paolo Bonzini
2024-07-17 5:03 ` [PULL 19/20] target/i386/tcg: use X86Access for TSS access Paolo Bonzini
` (2 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2024-07-17 5:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Richard Henderson
This step is listed in the Intel manual: "Checks that the new task is available
(call, jump, exception, or interrupt) or busy (IRET return)".
The AMD manual lists the same operation under the "Preventing recursion"
paragraph of "12.3.4 Nesting Tasks", though it is not clear if the processor
checks the busy bit in the IRET case.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/seg_helper.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c
index 8a6d92b3583..a5d5ce61f59 100644
--- a/target/i386/tcg/seg_helper.c
+++ b/target/i386/tcg/seg_helper.c
@@ -369,6 +369,11 @@ static int switch_tss_ra(CPUX86State *env, int tss_selector,
old_tss_limit_max = 43;
}
+ /* new TSS must be busy iff the source is an IRET instruction */
+ if (!!(e2 & DESC_TSS_BUSY_MASK) != (source == SWITCH_TSS_IRET)) {
+ raise_exception_err_ra(env, EXCP0A_TSS, tss_selector & 0xfffc, retaddr);
+ }
+
/* read all the registers from the new TSS */
if (type & 8) {
/* 32 bit */
--
2.45.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PULL 19/20] target/i386/tcg: use X86Access for TSS access
2024-07-17 5:03 [PULL 00/20] i386, bugfix changes for QEMU 9.1 soft freeze Paolo Bonzini
` (17 preceding siblings ...)
2024-07-17 5:03 ` [PULL 18/20] target/i386/tcg: check for correct busy state before switching to a new task Paolo Bonzini
@ 2024-07-17 5:03 ` Paolo Bonzini
2024-07-17 5:03 ` [PULL 20/20] target/i386/tcg: save current task state before loading new one Paolo Bonzini
2024-07-18 0:07 ` [PULL 00/20] i386, bugfix changes for QEMU 9.1 soft freeze Richard Henderson
20 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2024-07-17 5:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Richard Henderson
This takes care of probing the vaddr range in advance, and is also faster
because it avoids repeated TLB lookups. It also matches the Intel manual
better, as it says "Checks that the current (old) TSS, new TSS, and all
segment descriptors used in the task switch are paged into system memory";
note however that it's not clear how the processor checks for segment
descriptors, and this check is not included in the AMD manual.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/seg_helper.c | 110 ++++++++++++++++++-----------------
1 file changed, 58 insertions(+), 52 deletions(-)
diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c
index a5d5ce61f59..36d2f089cae 100644
--- a/target/i386/tcg/seg_helper.c
+++ b/target/i386/tcg/seg_helper.c
@@ -27,6 +27,7 @@
#include "exec/log.h"
#include "helper-tcg.h"
#include "seg_helper.h"
+#include "access.h"
#ifdef TARGET_X86_64
#define SET_ESP(val, sp_mask) \
@@ -313,14 +314,15 @@ static int switch_tss_ra(CPUX86State *env, int tss_selector,
uint32_t e1, uint32_t e2, int source,
uint32_t next_eip, uintptr_t retaddr)
{
- int tss_limit, tss_limit_max, type, old_tss_limit_max, old_type, v1, v2, i;
+ int tss_limit, tss_limit_max, type, old_tss_limit_max, old_type, i;
target_ulong tss_base;
uint32_t new_regs[8], new_segs[6];
uint32_t new_eflags, new_eip, new_cr3, new_ldt, new_trap;
uint32_t old_eflags, eflags_mask;
SegmentCache *dt;
- int index;
+ int mmu_index, index;
target_ulong ptr;
+ X86Access old, new;
type = (e2 >> DESC_TYPE_SHIFT) & 0xf;
LOG_PCALL("switch_tss: sel=0x%04x type=%d src=%d\n", tss_selector, type,
@@ -374,35 +376,45 @@ static int switch_tss_ra(CPUX86State *env, int tss_selector,
raise_exception_err_ra(env, EXCP0A_TSS, tss_selector & 0xfffc, retaddr);
}
+ /* X86Access avoids memory exceptions during the task switch */
+ mmu_index = cpu_mmu_index_kernel(env);
+ access_prepare_mmu(&old, env, env->tr.base, old_tss_limit_max,
+ MMU_DATA_STORE, mmu_index, retaddr);
+
+ if (source == SWITCH_TSS_CALL) {
+ /* Probe for future write of parent task */
+ probe_access(env, tss_base, 2, MMU_DATA_STORE,
+ mmu_index, retaddr);
+ }
+ access_prepare_mmu(&new, env, tss_base, tss_limit,
+ MMU_DATA_LOAD, mmu_index, retaddr);
+
/* read all the registers from the new TSS */
if (type & 8) {
/* 32 bit */
- new_cr3 = cpu_ldl_kernel_ra(env, tss_base + 0x1c, retaddr);
- new_eip = cpu_ldl_kernel_ra(env, tss_base + 0x20, retaddr);
- new_eflags = cpu_ldl_kernel_ra(env, tss_base + 0x24, retaddr);
+ new_cr3 = access_ldl(&new, tss_base + 0x1c);
+ new_eip = access_ldl(&new, tss_base + 0x20);
+ new_eflags = access_ldl(&new, tss_base + 0x24);
for (i = 0; i < 8; i++) {
- new_regs[i] = cpu_ldl_kernel_ra(env, tss_base + (0x28 + i * 4),
- retaddr);
+ new_regs[i] = access_ldl(&new, tss_base + (0x28 + i * 4));
}
for (i = 0; i < 6; i++) {
- new_segs[i] = cpu_lduw_kernel_ra(env, tss_base + (0x48 + i * 4),
- retaddr);
+ new_segs[i] = access_ldw(&new, tss_base + (0x48 + i * 4));
}
- new_ldt = cpu_lduw_kernel_ra(env, tss_base + 0x60, retaddr);
- new_trap = cpu_ldl_kernel_ra(env, tss_base + 0x64, retaddr);
+ new_ldt = access_ldw(&new, tss_base + 0x60);
+ new_trap = access_ldl(&new, tss_base + 0x64);
} else {
/* 16 bit */
new_cr3 = 0;
- new_eip = cpu_lduw_kernel_ra(env, tss_base + 0x0e, retaddr);
- new_eflags = cpu_lduw_kernel_ra(env, tss_base + 0x10, retaddr);
+ new_eip = access_ldw(&new, tss_base + 0x0e);
+ new_eflags = access_ldw(&new, tss_base + 0x10);
for (i = 0; i < 8; i++) {
- new_regs[i] = cpu_lduw_kernel_ra(env, tss_base + (0x12 + i * 2), retaddr);
+ new_regs[i] = access_ldw(&new, tss_base + (0x12 + i * 2));
}
for (i = 0; i < 4; i++) {
- new_segs[i] = cpu_lduw_kernel_ra(env, tss_base + (0x22 + i * 2),
- retaddr);
+ new_segs[i] = access_ldw(&new, tss_base + (0x22 + i * 2));
}
- new_ldt = cpu_lduw_kernel_ra(env, tss_base + 0x2a, retaddr);
+ new_ldt = access_ldw(&new, tss_base + 0x2a);
new_segs[R_FS] = 0;
new_segs[R_GS] = 0;
new_trap = 0;
@@ -412,16 +424,6 @@ static int switch_tss_ra(CPUX86State *env, int tss_selector,
chapters 12.2.5 and 13.2.4 on how to implement TSS Trap bit */
(void)new_trap;
- /* NOTE: we must avoid memory exceptions during the task switch,
- so we make dummy accesses before */
- /* XXX: it can still fail in some cases, so a bigger hack is
- necessary to valid the TLB after having done the accesses */
-
- v1 = cpu_ldub_kernel_ra(env, env->tr.base, retaddr);
- v2 = cpu_ldub_kernel_ra(env, env->tr.base + old_tss_limit_max, retaddr);
- cpu_stb_kernel_ra(env, env->tr.base, v1, retaddr);
- cpu_stb_kernel_ra(env, env->tr.base + old_tss_limit_max, v2, retaddr);
-
/* clear busy bit (it is restartable) */
if (source == SWITCH_TSS_JMP || source == SWITCH_TSS_IRET) {
tss_set_busy(env, env->tr.selector, 0, retaddr);
@@ -434,35 +436,35 @@ static int switch_tss_ra(CPUX86State *env, int tss_selector,
/* save the current state in the old TSS */
if (old_type & 8) {
/* 32 bit */
- cpu_stl_kernel_ra(env, env->tr.base + 0x20, next_eip, retaddr);
- cpu_stl_kernel_ra(env, env->tr.base + 0x24, old_eflags, retaddr);
- cpu_stl_kernel_ra(env, env->tr.base + (0x28 + 0 * 4), env->regs[R_EAX], retaddr);
- cpu_stl_kernel_ra(env, env->tr.base + (0x28 + 1 * 4), env->regs[R_ECX], retaddr);
- cpu_stl_kernel_ra(env, env->tr.base + (0x28 + 2 * 4), env->regs[R_EDX], retaddr);
- cpu_stl_kernel_ra(env, env->tr.base + (0x28 + 3 * 4), env->regs[R_EBX], retaddr);
- cpu_stl_kernel_ra(env, env->tr.base + (0x28 + 4 * 4), env->regs[R_ESP], retaddr);
- cpu_stl_kernel_ra(env, env->tr.base + (0x28 + 5 * 4), env->regs[R_EBP], retaddr);
- cpu_stl_kernel_ra(env, env->tr.base + (0x28 + 6 * 4), env->regs[R_ESI], retaddr);
- cpu_stl_kernel_ra(env, env->tr.base + (0x28 + 7 * 4), env->regs[R_EDI], retaddr);
+ access_stl(&old, env->tr.base + 0x20, next_eip);
+ access_stl(&old, env->tr.base + 0x24, old_eflags);
+ access_stl(&old, env->tr.base + (0x28 + 0 * 4), env->regs[R_EAX]);
+ access_stl(&old, env->tr.base + (0x28 + 1 * 4), env->regs[R_ECX]);
+ access_stl(&old, env->tr.base + (0x28 + 2 * 4), env->regs[R_EDX]);
+ access_stl(&old, env->tr.base + (0x28 + 3 * 4), env->regs[R_EBX]);
+ access_stl(&old, env->tr.base + (0x28 + 4 * 4), env->regs[R_ESP]);
+ access_stl(&old, env->tr.base + (0x28 + 5 * 4), env->regs[R_EBP]);
+ access_stl(&old, env->tr.base + (0x28 + 6 * 4), env->regs[R_ESI]);
+ access_stl(&old, env->tr.base + (0x28 + 7 * 4), env->regs[R_EDI]);
for (i = 0; i < 6; i++) {
- cpu_stw_kernel_ra(env, env->tr.base + (0x48 + i * 4),
- env->segs[i].selector, retaddr);
+ access_stw(&old, env->tr.base + (0x48 + i * 4),
+ env->segs[i].selector);
}
} else {
/* 16 bit */
- cpu_stw_kernel_ra(env, env->tr.base + 0x0e, next_eip, retaddr);
- cpu_stw_kernel_ra(env, env->tr.base + 0x10, old_eflags, retaddr);
- cpu_stw_kernel_ra(env, env->tr.base + (0x12 + 0 * 2), env->regs[R_EAX], retaddr);
- cpu_stw_kernel_ra(env, env->tr.base + (0x12 + 1 * 2), env->regs[R_ECX], retaddr);
- cpu_stw_kernel_ra(env, env->tr.base + (0x12 + 2 * 2), env->regs[R_EDX], retaddr);
- cpu_stw_kernel_ra(env, env->tr.base + (0x12 + 3 * 2), env->regs[R_EBX], retaddr);
- cpu_stw_kernel_ra(env, env->tr.base + (0x12 + 4 * 2), env->regs[R_ESP], retaddr);
- cpu_stw_kernel_ra(env, env->tr.base + (0x12 + 5 * 2), env->regs[R_EBP], retaddr);
- cpu_stw_kernel_ra(env, env->tr.base + (0x12 + 6 * 2), env->regs[R_ESI], retaddr);
- cpu_stw_kernel_ra(env, env->tr.base + (0x12 + 7 * 2), env->regs[R_EDI], retaddr);
+ access_stw(&old, env->tr.base + 0x0e, next_eip);
+ access_stw(&old, env->tr.base + 0x10, old_eflags);
+ access_stw(&old, env->tr.base + (0x12 + 0 * 2), env->regs[R_EAX]);
+ access_stw(&old, env->tr.base + (0x12 + 1 * 2), env->regs[R_ECX]);
+ access_stw(&old, env->tr.base + (0x12 + 2 * 2), env->regs[R_EDX]);
+ access_stw(&old, env->tr.base + (0x12 + 3 * 2), env->regs[R_EBX]);
+ access_stw(&old, env->tr.base + (0x12 + 4 * 2), env->regs[R_ESP]);
+ access_stw(&old, env->tr.base + (0x12 + 5 * 2), env->regs[R_EBP]);
+ access_stw(&old, env->tr.base + (0x12 + 6 * 2), env->regs[R_ESI]);
+ access_stw(&old, env->tr.base + (0x12 + 7 * 2), env->regs[R_EDI]);
for (i = 0; i < 4; i++) {
- cpu_stw_kernel_ra(env, env->tr.base + (0x22 + i * 2),
- env->segs[i].selector, retaddr);
+ access_stw(&old, env->tr.base + (0x22 + i * 2),
+ env->segs[i].selector);
}
}
@@ -470,7 +472,11 @@ static int switch_tss_ra(CPUX86State *env, int tss_selector,
context */
if (source == SWITCH_TSS_CALL) {
- cpu_stw_kernel_ra(env, tss_base, env->tr.selector, retaddr);
+ /*
+ * Thanks to the probe_access above, we know the first two
+ * bytes addressed by &new are writable too.
+ */
+ access_stw(&new, tss_base, env->tr.selector);
new_eflags |= NT_MASK;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PULL 20/20] target/i386/tcg: save current task state before loading new one
2024-07-17 5:03 [PULL 00/20] i386, bugfix changes for QEMU 9.1 soft freeze Paolo Bonzini
` (18 preceding siblings ...)
2024-07-17 5:03 ` [PULL 19/20] target/i386/tcg: use X86Access for TSS access Paolo Bonzini
@ 2024-07-17 5:03 ` Paolo Bonzini
2024-07-18 0:07 ` [PULL 00/20] i386, bugfix changes for QEMU 9.1 soft freeze Richard Henderson
20 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2024-07-17 5:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Richard Henderson
This is how the steps are ordered in the manual. EFLAGS.NT is
overwritten after the fact in the saved image.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/seg_helper.c | 85 +++++++++++++++++++-----------------
1 file changed, 45 insertions(+), 40 deletions(-)
diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c
index 36d2f089cae..aac092a356b 100644
--- a/target/i386/tcg/seg_helper.c
+++ b/target/i386/tcg/seg_helper.c
@@ -389,6 +389,42 @@ static int switch_tss_ra(CPUX86State *env, int tss_selector,
access_prepare_mmu(&new, env, tss_base, tss_limit,
MMU_DATA_LOAD, mmu_index, retaddr);
+ /* save the current state in the old TSS */
+ old_eflags = cpu_compute_eflags(env);
+ if (old_type & 8) {
+ /* 32 bit */
+ access_stl(&old, env->tr.base + 0x20, next_eip);
+ access_stl(&old, env->tr.base + 0x24, old_eflags);
+ access_stl(&old, env->tr.base + (0x28 + 0 * 4), env->regs[R_EAX]);
+ access_stl(&old, env->tr.base + (0x28 + 1 * 4), env->regs[R_ECX]);
+ access_stl(&old, env->tr.base + (0x28 + 2 * 4), env->regs[R_EDX]);
+ access_stl(&old, env->tr.base + (0x28 + 3 * 4), env->regs[R_EBX]);
+ access_stl(&old, env->tr.base + (0x28 + 4 * 4), env->regs[R_ESP]);
+ access_stl(&old, env->tr.base + (0x28 + 5 * 4), env->regs[R_EBP]);
+ access_stl(&old, env->tr.base + (0x28 + 6 * 4), env->regs[R_ESI]);
+ access_stl(&old, env->tr.base + (0x28 + 7 * 4), env->regs[R_EDI]);
+ for (i = 0; i < 6; i++) {
+ access_stw(&old, env->tr.base + (0x48 + i * 4),
+ env->segs[i].selector);
+ }
+ } else {
+ /* 16 bit */
+ access_stw(&old, env->tr.base + 0x0e, next_eip);
+ access_stw(&old, env->tr.base + 0x10, old_eflags);
+ access_stw(&old, env->tr.base + (0x12 + 0 * 2), env->regs[R_EAX]);
+ access_stw(&old, env->tr.base + (0x12 + 1 * 2), env->regs[R_ECX]);
+ access_stw(&old, env->tr.base + (0x12 + 2 * 2), env->regs[R_EDX]);
+ access_stw(&old, env->tr.base + (0x12 + 3 * 2), env->regs[R_EBX]);
+ access_stw(&old, env->tr.base + (0x12 + 4 * 2), env->regs[R_ESP]);
+ access_stw(&old, env->tr.base + (0x12 + 5 * 2), env->regs[R_EBP]);
+ access_stw(&old, env->tr.base + (0x12 + 6 * 2), env->regs[R_ESI]);
+ access_stw(&old, env->tr.base + (0x12 + 7 * 2), env->regs[R_EDI]);
+ for (i = 0; i < 4; i++) {
+ access_stw(&old, env->tr.base + (0x22 + i * 2),
+ env->segs[i].selector);
+ }
+ }
+
/* read all the registers from the new TSS */
if (type & 8) {
/* 32 bit */
@@ -428,49 +464,16 @@ static int switch_tss_ra(CPUX86State *env, int tss_selector,
if (source == SWITCH_TSS_JMP || source == SWITCH_TSS_IRET) {
tss_set_busy(env, env->tr.selector, 0, retaddr);
}
- old_eflags = cpu_compute_eflags(env);
+
if (source == SWITCH_TSS_IRET) {
old_eflags &= ~NT_MASK;
+ if (old_type & 8) {
+ access_stl(&old, env->tr.base + 0x24, old_eflags);
+ } else {
+ access_stw(&old, env->tr.base + 0x10, old_eflags);
+ }
}
- /* save the current state in the old TSS */
- if (old_type & 8) {
- /* 32 bit */
- access_stl(&old, env->tr.base + 0x20, next_eip);
- access_stl(&old, env->tr.base + 0x24, old_eflags);
- access_stl(&old, env->tr.base + (0x28 + 0 * 4), env->regs[R_EAX]);
- access_stl(&old, env->tr.base + (0x28 + 1 * 4), env->regs[R_ECX]);
- access_stl(&old, env->tr.base + (0x28 + 2 * 4), env->regs[R_EDX]);
- access_stl(&old, env->tr.base + (0x28 + 3 * 4), env->regs[R_EBX]);
- access_stl(&old, env->tr.base + (0x28 + 4 * 4), env->regs[R_ESP]);
- access_stl(&old, env->tr.base + (0x28 + 5 * 4), env->regs[R_EBP]);
- access_stl(&old, env->tr.base + (0x28 + 6 * 4), env->regs[R_ESI]);
- access_stl(&old, env->tr.base + (0x28 + 7 * 4), env->regs[R_EDI]);
- for (i = 0; i < 6; i++) {
- access_stw(&old, env->tr.base + (0x48 + i * 4),
- env->segs[i].selector);
- }
- } else {
- /* 16 bit */
- access_stw(&old, env->tr.base + 0x0e, next_eip);
- access_stw(&old, env->tr.base + 0x10, old_eflags);
- access_stw(&old, env->tr.base + (0x12 + 0 * 2), env->regs[R_EAX]);
- access_stw(&old, env->tr.base + (0x12 + 1 * 2), env->regs[R_ECX]);
- access_stw(&old, env->tr.base + (0x12 + 2 * 2), env->regs[R_EDX]);
- access_stw(&old, env->tr.base + (0x12 + 3 * 2), env->regs[R_EBX]);
- access_stw(&old, env->tr.base + (0x12 + 4 * 2), env->regs[R_ESP]);
- access_stw(&old, env->tr.base + (0x12 + 5 * 2), env->regs[R_EBP]);
- access_stw(&old, env->tr.base + (0x12 + 6 * 2), env->regs[R_ESI]);
- access_stw(&old, env->tr.base + (0x12 + 7 * 2), env->regs[R_EDI]);
- for (i = 0; i < 4; i++) {
- access_stw(&old, env->tr.base + (0x22 + i * 2),
- env->segs[i].selector);
- }
- }
-
- /* now if an exception occurs, it will occurs in the next task
- context */
-
if (source == SWITCH_TSS_CALL) {
/*
* Thanks to the probe_access above, we know the first two
@@ -486,7 +489,9 @@ static int switch_tss_ra(CPUX86State *env, int tss_selector,
}
/* set the new CPU state */
- /* from this point, any exception which occurs can give problems */
+
+ /* now if an exception occurs, it will occur in the next task context */
+
env->cr[0] |= CR0_TS_MASK;
env->hflags |= HF_TS_MASK;
env->tr.selector = tss_selector;
--
2.45.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PULL 00/20] i386, bugfix changes for QEMU 9.1 soft freeze
2024-07-17 5:03 [PULL 00/20] i386, bugfix changes for QEMU 9.1 soft freeze Paolo Bonzini
` (19 preceding siblings ...)
2024-07-17 5:03 ` [PULL 20/20] target/i386/tcg: save current task state before loading new one Paolo Bonzini
@ 2024-07-18 0:07 ` Richard Henderson
20 siblings, 0 replies; 22+ messages in thread
From: Richard Henderson @ 2024-07-18 0:07 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel
On 7/17/24 15:03, Paolo Bonzini wrote:
> The following changes since commit 959269e910944c03bc13f300d65bf08b060d5d0f:
>
> Merge tag 'python-pull-request' ofhttps://gitlab.com/jsnow/qemu into staging (2024-07-16 06:45:23 +1000)
>
> are available in the Git repository at:
>
> https://gitlab.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 6a079f2e68e1832ebca0e7d64bc31ffebde9b2dd:
>
> target/i386/tcg: save current task state before loading new one (2024-07-16 18:18:25 +0200)
>
> ----------------------------------------------------------------
> * target/i386/tcg: fixes for seg_helper.c
> * SEV: Don't allow automatic fallback to legacy KVM_SEV_INIT,
> but also don't use it by default
> * scsi: honor bootindex again for legacy drives
> * hpet, utils, scsi, build, cpu: miscellaneous bugfixes
Applied, thanks.
r~
^ permalink raw reply [flat|nested] 22+ messages in thread