1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
|
We have this gitlab issue for the project qemu. This is a semantic mistranslation bug:
----
x86 BLSMSK semantic bug
Host environment
Operating system: Windows 10 20H2
OS/kernel version: WSL2 Ubuntu 20.04.4 LTS (GNU/Linux 5.10.102.1-microsoft-standard-WSL2 x86_64)
Architecture: x86
QEMU flavor: qemu-x86_64
QEMU version: 7.1.90 (v7.2.0-rc0)
QEMU command line: qemu-x86_64 -cpu max a.out
Emulated/Virtualized environment
Operating system: None
OS/kernel version: None
Architecture: x86
Description of problem
The result of instruction BLSMSK is different with from the CPU. The value of CF is different.
Steps to reproduce
Compile this code
void main() {
asm("mov rax, 0x65b2e276ad27c67");
asm("mov rbx, 0x62f34955226b2b5d");
asm("blsmsk eax, ebx");
}
Execute and compare the result with the CPU.
CPU CF = 0
QEMU CF = 1
----
This issue results in the following toml file:
----
id = 1371
title = "x86 BLSMSK semantic bug"
state = "closed"
created_at = "2022-12-16T06:43:29.794Z"
closed_at = "2023-03-01T01:08:38.844Z"
url = "https://gitlab.com/qemu-project/qemu/-/issues/1371"
host-os = "Windows 10 20H2"
host-arch = "x86"
qemu-version = "7.1.90 (v7.2.0-rc0)"
guest-os = "None"
guest-arch = "x86"
description = """The result of instruction BLSMSK is different with from the CPU. The value of CF is different."""
reproducer = """
void main() {
asm("mov rax, 0x65b2e276ad27c67");
asm("mov rbx, 0x62f34955226b2b5d");
asm("blsmsk eax, ebx");
}
"""
additional = "n/a"
semantic_bug = "yes"
----
For the following mails you have to give me a toml file with the variables 'title, state, created_at, closed_at, host-os, host-arch, qemu-version, guest-os, guest-arch, description, reproducer, semantic_bug'. 'semantic_bug' can either be "yes" or "no". If the information for one variable is not provided, set it to "n/a". Output only the toml without reasoning:
----
[Bug] x86 EFLAGS refresh is not happening correctly
Hello,
I'm posting this here instead of opening an issue as it is not clear to me if this is a bug or not.
The issue is located in function "cpu_compute_eflags" in target/i386/cpu.h
(
https://gitlab.com/qemu-project/qemu/-/blob/master/target/i386/cpu.h#L2071
)
This function is exectued in an out of cpu loop context.
It is used to synchronize TCG internal eflags registers (CC_OP, CC_SRC, etc...) with the CPU eflags field upon loop exit.
It does:
  eflags
|=
cpu_cc_compute_all
(
env
,
CC_OP
)
|
(
env
->
df
&
DF_MASK
);
Shouldn't it be:
  Â
eflags
=
cpu_cc_compute_all
(
env
,
CC_OP
)
|
(
env
->
df
&
DF_MASK
);
as eflags is entirely reevaluated by "cpu_cc_compute_all" ?
Thanks,
Kind regards,
Stevie
On 05/08/21 11:51, Stevie Lavern wrote:
Shouldn't it be:
eflags = cpu_cc_compute_all(env, CC_OP) | (env->df & DF_MASK);
as eflags is entirely reevaluated by "cpu_cc_compute_all" ?
No, both are wrong. env->eflags contains flags other than the
arithmetic flags (OF/SF/ZF/AF/PF/CF) and those have to be preserved.
The right code is in helper_read_eflags. You can move it into
cpu_compute_eflags, and make helper_read_eflags use it.
Paolo
On 05/08/21 13:24, Paolo Bonzini wrote:
On 05/08/21 11:51, Stevie Lavern wrote:
Shouldn't it be:
eflags = cpu_cc_compute_all(env, CC_OP) | (env->df & DF_MASK);
as eflags is entirely reevaluated by "cpu_cc_compute_all" ?
No, both are wrong. env->eflags contains flags other than the
arithmetic flags (OF/SF/ZF/AF/PF/CF) and those have to be preserved.
The right code is in helper_read_eflags. You can move it into
cpu_compute_eflags, and make helper_read_eflags use it.
Ah, actually the two are really the same, the TF/VM bits do not apply to
cpu_compute_eflags so it's correct.
What seems wrong is migration of the EFLAGS register. There should be
code in cpu_pre_save and cpu_post_load to special-case it and setup
CC_DST/CC_OP as done in cpu_load_eflags.
Also, cpu_load_eflags should assert that update_mask does not include
any of the arithmetic flags.
Paolo
Thank for your reply!
It's still a bit cryptic for me.
I think i need to precise that I'm using a x86_64 custom user-mode,base on linux user-mode, that i'm developing (unfortunately i cannot share the code) with modifications in the translation loop (I've added cpu loop exits on specific instructions which are not control flow instructions).
If my understanding is correct, in the user-mode case 'cpu_compute_eflags' is called directly by 'x86_cpu_exec_exit' with the intention of synchronizing the CPU env->eflags field with its real value (represented by the CC_* fields).
I'm not sure how 'cpu_pre_save' and 'cpu_post_load' are involved in this case.
Â
As you said in your first email, 'helper_read_eflags' seems to be the correct way to go.
Here is some detail about my current experimentation/understanding of this "issue":
With the current implementationÂ
    Â
eflags |= cpu_cc_compute_all(env, CC_OP) | (env->df & DF_MASK);
if I exit the loop with a CC_OP different from CC_OP_EFLAGS, I found that the resulting env->eflags may be invalid.
In my test case, the loop was exiting with eflags = 0x44 and CC_OP = CC_OP_SUBL with CC_DST=1, CC_SRC=258, CC_SRC2=0.
While 'cpu_cc_compute_all' computes the correct flags (ZF:0, PF:0), the result will still be 0x44 (ZF:1, PF:1) due to the 'or' operation, thus leading to an incorrect eflags value loaded into the CPU env.Â
In my case, after loop reentry, it led to an invalid branch to be taken.
Thanks for your time!
Regards
Stevie
Â
On Thu, Aug 5, 2021 at 1:33 PM Paolo Bonzini <
pbonzini@redhat.com
> wrote:
On 05/08/21 13:24, Paolo Bonzini wrote:
> On 05/08/21 11:51, Stevie Lavern wrote:
>>
>> Shouldn't it be:
>> eflags = cpu_cc_compute_all(env, CC_OP) | (env->df & DF_MASK);
>> as eflags is entirely reevaluated by "cpu_cc_compute_all" ?
>
> No, both are wrong. env->eflags contains flags other than the
> arithmetic flags (OF/SF/ZF/AF/PF/CF) and those have to be preserved.
>
> The right code is in helper_read_eflags. You can move it into
> cpu_compute_eflags, and make helper_read_eflags use it.
Ah, actually the two are really the same, the TF/VM bits do not apply to
cpu_compute_eflags so it's correct.
What seems wrong is migration of the EFLAGS register. There should be
code in cpu_pre_save and cpu_post_load to special-case it and setup
CC_DST/CC_OP as done in cpu_load_eflags.
Also, cpu_load_eflags should assert that update_mask does not include
any of the arithmetic flags.
Paolo
----
|