mbox series

[v2,00/50] target/i386 translate cleanups

Message ID 20210514151342.384376-1-richard.henderson@linaro.org
Headers show
Series target/i386 translate cleanups | expand

Message

Richard Henderson May 14, 2021, 3:12 p.m. UTC 
Supercedes: 20210228232321.322053-1-richard.henderson@linaro.org
("i386 cleanup part 3")

Claudio's prerequite patch set ("i386 cleanup part 2"), is now merged.
Version 2 here fixes some minor rebase conflicts that crept in.
Only 4 of the 50 patches have been reviewed.

Blurb from v1:
This started out to address a fixme in Claudio's patch set, then
wandered a bit, with cleanups and bug fixes in the code that I was
touching.  I stopped when I reached my original goal of removing
target/i386/tcg/user/*_stubs.c.

There are a lot of patches, but hopefully they're all small
enough to be obvious.


r~


Richard Henderson (50):
  target/i386: Split out gen_exception_gpf
  target/i386: Split out check_cpl0
  target/i386: Unify code paths for IRET
  target/i386: Split out check_vm86_iopl
  target/i386: Split out check_iopl
  target/i386: Assert PE is set for user-only
  target/i386: Assert CPL is 3 for user-only
  target/i386: Assert IOPL is 0 for user-only
  target/i386: Assert !VM86 for x86_64 user-only
  target/i386: Assert CODE32 for x86_64 user-only
  target/i386: Assert SS32 for x86_64 user-only
  target/i386: Assert CODE64 for x86_64 user-only
  target/i386: Assert LMA for x86_64 user-only
  target/i386: Assert !ADDSEG for x86_64 user-only
  target/i386: Introduce REX_PREFIX
  target/i386: Tidy REX_B, REX_X definition
  target/i386: Move rex_r into DisasContext
  target/i386: Move rex_w into DisasContext
  target/i386: Remove DisasContext.f_st as unused
  target/i386: Reduce DisasContext.flags to uint32_t
  target/i386: Reduce DisasContext.override to int8_t
  target/i386: Reduce DisasContext.prefix to uint8_t
  target/i386: Reduce DisasContext.vex_[lv] to uint8_t
  target/i386: Reduce DisasContext popl_esp_hack and rip_offset to
    uint8_t
  target/i386: Leave TF in DisasContext.flags
  target/i386: Reduce DisasContext jmp_opt, repz_opt to bool
  target/i386: Fix the comment for repz_opt
  target/i386: Reorder DisasContext members
  target/i386: Add stub generator for helper_set_dr
  target/i386: Assert !SVME for user-only
  target/i386: Assert !GUEST for user-only
  target/i386: Implement skinit in translate.c
  target/i386: Eliminate SVM helpers for user-only
  target/i386: Mark some helpers as noreturn
  target/i386: Simplify gen_debug usage
  target/i386: Tidy svm_check_intercept from tcg
  target/i386: Remove pc_start argument to gen_svm_check_intercept
  target/i386: Remove user stub for cpu_vmexit
  target/i386: Cleanup read_crN, write_crN, lmsw
  target/i386: Pass env to do_pause and do_hlt
  target/i386: Move invlpg, hlt, monitor, mwait to sysemu
  target/i386: Unify invlpg, invlpga
  target/i386: Inline user cpu_svm_check_intercept_param
  target/i386: Eliminate user stubs for read/write_crN, rd/wrmsr
  target/i386: Exit tb after wrmsr
  target/i386: Tidy gen_check_io
  target/i386: Pass in port to gen_check_io
  target/i386: Create helper_check_io
  target/i386: Move helper_check_io to sysemu
  target/i386: Remove user-only i/o stubs

 target/i386/cpu.h                    |   8 +
 target/i386/helper.h                 |  43 +-
 target/i386/tcg/helper-tcg.h         |   5 +-
 target/i386/tcg/bpt_helper.c         |   2 +-
 target/i386/tcg/excp_helper.c        |  18 +-
 target/i386/tcg/misc_helper.c        |  79 +--
 target/i386/tcg/seg_helper.c         |  43 --
 target/i386/tcg/sysemu/misc_helper.c |  52 +-
 target/i386/tcg/sysemu/seg_helper.c  |  29 +
 target/i386/tcg/sysemu/svm_helper.c  |  30 +-
 target/i386/tcg/translate.c          | 891 ++++++++++++++-------------
 target/i386/tcg/user/misc_stubs.c    |  75 ---
 target/i386/tcg/user/svm_stubs.c     |  76 ---
 target/i386/tcg/user/meson.build     |   2 -
 14 files changed, 600 insertions(+), 753 deletions(-)
 delete mode 100644 target/i386/tcg/user/misc_stubs.c
 delete mode 100644 target/i386/tcg/user/svm_stubs.c

-- 
2.25.1

Comments

no-reply@patchew.org May 14, 2021, 4:09 p.m. UTC | #1 
Patchew URL: https://patchew.org/QEMU/20210514151342.384376-1-richard.henderson@linaro.org/



Hi,

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

Type: series
Message-id: 20210514151342.384376-1-richard.henderson@linaro.org
Subject: [PATCH v2 00/50] target/i386 translate cleanups

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
   2d3fc4e..9666299  master     -> master
 - [tag update]      patchew/20210325153529.75831-1-andrew@daynix.com -> patchew/20210325153529.75831-1-andrew@daynix.com
 - [tag update]      patchew/20210512040250.203953-1-iii@linux.ibm.com -> patchew/20210512040250.203953-1-iii@linux.ibm.com
 - [tag update]      patchew/20210512214642.2803189-1-jsnow@redhat.com -> patchew/20210512214642.2803189-1-jsnow@redhat.com
 * [new tag]         patchew/20210514151342.384376-1-richard.henderson@linaro.org -> patchew/20210514151342.384376-1-richard.henderson@linaro.org
Switched to a new branch 'test'
e202c8c target/i386: Remove user-only i/o stubs
1d820c0 target/i386: Move helper_check_io to sysemu
50e4fb4 target/i386: Create helper_check_io
6814811 target/i386: Pass in port to gen_check_io
8f6e5bb target/i386: Tidy gen_check_io
4282091 target/i386: Exit tb after wrmsr
5555941 target/i386: Eliminate user stubs for read/write_crN, rd/wrmsr
0010e63 target/i386: Inline user cpu_svm_check_intercept_param
946b94c target/i386: Unify invlpg, invlpga
46e15b6 target/i386: Move invlpg, hlt, monitor, mwait to sysemu
fc622c8 target/i386: Pass env to do_pause and do_hlt
eff322a target/i386: Cleanup read_crN, write_crN, lmsw
f975359 target/i386: Remove user stub for cpu_vmexit
01ec488 target/i386: Remove pc_start argument to gen_svm_check_intercept
e533b1a target/i386: Tidy svm_check_intercept from tcg
07bc5bb target/i386: Simplify gen_debug usage
33b7b33 target/i386: Mark some helpers as noreturn
9e4eefd target/i386: Eliminate SVM helpers for user-only
a4b7e86 target/i386: Implement skinit in translate.c
11f0dd5 target/i386: Assert !GUEST for user-only
0b9152d target/i386: Assert !SVME for user-only
1619090 target/i386: Add stub generator for helper_set_dr
6b31254 target/i386: Reorder DisasContext members
a196b94 target/i386: Fix the comment for repz_opt
05d9ac8 target/i386: Reduce DisasContext jmp_opt, repz_opt to bool
2248cea target/i386: Leave TF in DisasContext.flags
3e5c314 target/i386: Reduce DisasContext popl_esp_hack and rip_offset to uint8_t
d2ff6f0 target/i386: Reduce DisasContext.vex_[lv] to uint8_t
1045677 target/i386: Reduce DisasContext.prefix to uint8_t
b052ed9 target/i386: Reduce DisasContext.override to int8_t
04bd526 target/i386: Reduce DisasContext.flags to uint32_t
25a9164 target/i386: Remove DisasContext.f_st as unused
9c8cd54 target/i386: Move rex_w into DisasContext
be9c7a6 target/i386: Move rex_r into DisasContext
8f59d5d target/i386: Tidy REX_B, REX_X definition
86d68fb target/i386: Introduce REX_PREFIX
df0e9ba target/i386: Assert !ADDSEG for x86_64 user-only
afbeac9 target/i386: Assert LMA for x86_64 user-only
ef457bd target/i386: Assert CODE64 for x86_64 user-only
bb9e685 target/i386: Assert SS32 for x86_64 user-only
926c3fc target/i386: Assert CODE32 for x86_64 user-only
47c6cc1 target/i386: Assert !VM86 for x86_64 user-only
4b8ee2f target/i386: Assert IOPL is 0 for user-only
46331b6 target/i386: Assert CPL is 3 for user-only
e5377a8 target/i386: Assert PE is set for user-only
f3dcdd9 target/i386: Split out check_iopl
f6f133e target/i386: Split out check_vm86_iopl
5497a8a target/i386: Unify code paths for IRET
7f7ed03 target/i386: Split out check_cpl0
9eee238 target/i386: Split out gen_exception_gpf

=== OUTPUT BEGIN ===
1/50 Checking commit 9eee2384d266 (target/i386: Split out gen_exception_gpf)
2/50 Checking commit 7f7ed0354a5f (target/i386: Split out check_cpl0)
3/50 Checking commit 5497a8ae5aba (target/i386: Unify code paths for IRET)
4/50 Checking commit f6f133e635f0 (target/i386: Split out check_vm86_iopl)
5/50 Checking commit f3dcdd958c30 (target/i386: Split out check_iopl)
6/50 Checking commit e5377a824c04 (target/i386: Assert PE is set for user-only)
ERROR: braces {} are necessary for all arms of this statement
#128: FILE: target/i386/tcg/translate.c:7310:
+            if (!PE(s) || s->vm86)
[...]

ERROR: braces {} are necessary for all arms of this statement
#146: FILE: target/i386/tcg/translate.c:7329:
+            if (!PE(s) || s->vm86)
[...]

ERROR: braces {} are necessary for all arms of this statement
#164: FILE: target/i386/tcg/translate.c:7349:
+            if (!PE(s) || s->vm86)
[...]

ERROR: braces {} are necessary for all arms of this statement
#236: FILE: target/i386/tcg/translate.c:7720:
+            if (!PE(s) || s->vm86)
[...]

ERROR: braces {} are necessary for all arms of this statement
#245: FILE: target/i386/tcg/translate.c:7768:
+            if (!PE(s) || s->vm86)
[...]

total: 5 errors, 0 warnings, 244 lines checked

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

7/50 Checking commit 46331b60bf94 (target/i386: Assert CPL is 3 for user-only)
8/50 Checking commit 4b8ee2f3f077 (target/i386: Assert IOPL is 0 for user-only)
9/50 Checking commit 47c6cc104278 (target/i386: Assert !VM86 for x86_64 user-only)
ERROR: braces {} are necessary for all arms of this statement
#130: FILE: target/i386/tcg/translate.c:7322:
+            if (!PE(s) || VM86(s))
[...]

ERROR: braces {} are necessary for all arms of this statement
#148: FILE: target/i386/tcg/translate.c:7341:
+            if (!PE(s) || VM86(s))
[...]

ERROR: braces {} are necessary for all arms of this statement
#166: FILE: target/i386/tcg/translate.c:7361:
+            if (!PE(s) || VM86(s))
[...]

ERROR: braces {} are necessary for all arms of this statement
#175: FILE: target/i386/tcg/translate.c:7732:
+            if (!PE(s) || VM86(s))
[...]

ERROR: braces {} are necessary for all arms of this statement
#184: FILE: target/i386/tcg/translate.c:7780:
+            if (!PE(s) || VM86(s))
[...]

total: 5 errors, 0 warnings, 159 lines checked

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

10/50 Checking commit 926c3fc7270b (target/i386: Assert CODE32 for x86_64 user-only)
11/50 Checking commit bb9e68525fbe (target/i386: Assert SS32 for x86_64 user-only)
12/50 Checking commit ef457bdf9f4f (target/i386: Assert CODE64 for x86_64 user-only)
13/50 Checking commit afbeac994eb8 (target/i386: Assert LMA for x86_64 user-only)
14/50 Checking commit df0e9ba61de2 (target/i386: Assert !ADDSEG for x86_64 user-only)
15/50 Checking commit 86d68fb10f01 (target/i386: Introduce REX_PREFIX)
16/50 Checking commit 8f59d5d4bb24 (target/i386: Tidy REX_B, REX_X definition)
17/50 Checking commit be9c7a6cea20 (target/i386: Move rex_r into DisasContext)
18/50 Checking commit 9c8cd545d35c (target/i386: Move rex_w into DisasContext)
19/50 Checking commit 25a91640ee31 (target/i386: Remove DisasContext.f_st as unused)
20/50 Checking commit 04bd52617731 (target/i386: Reduce DisasContext.flags to uint32_t)
21/50 Checking commit b052ed9d2425 (target/i386: Reduce DisasContext.override to int8_t)
22/50 Checking commit 104567771296 (target/i386: Reduce DisasContext.prefix to uint8_t)
23/50 Checking commit d2ff6f020e1d (target/i386: Reduce DisasContext.vex_[lv] to uint8_t)
24/50 Checking commit 3e5c31499dbd (target/i386: Reduce DisasContext popl_esp_hack and rip_offset to uint8_t)
25/50 Checking commit 2248cea6f655 (target/i386: Leave TF in DisasContext.flags)
26/50 Checking commit 05d9ac8f6f1c (target/i386: Reduce DisasContext jmp_opt, repz_opt to bool)
27/50 Checking commit a196b94506cc (target/i386: Fix the comment for repz_opt)
28/50 Checking commit 6b3125424e7e (target/i386: Reorder DisasContext members)
29/50 Checking commit 1619090fbe83 (target/i386: Add stub generator for helper_set_dr)
30/50 Checking commit 0b9152d9326a (target/i386: Assert !SVME for user-only)
31/50 Checking commit 11f0dd55a271 (target/i386: Assert !GUEST for user-only)
32/50 Checking commit a4b7e867551d (target/i386: Implement skinit in translate.c)
33/50 Checking commit 9e4eefdeb4b0 (target/i386: Eliminate SVM helpers for user-only)
34/50 Checking commit 33b7b33509a7 (target/i386: Mark some helpers as noreturn)
35/50 Checking commit 07bc5bb234b8 (target/i386: Simplify gen_debug usage)
36/50 Checking commit e533b1a32fe7 (target/i386: Tidy svm_check_intercept from tcg)
37/50 Checking commit 01ec488d9b2f (target/i386: Remove pc_start argument to gen_svm_check_intercept)
WARNING: line over 80 characters
#178: FILE: target/i386/tcg/translate.c:7718:
+            gen_svm_check_intercept(s, (b & 2) ? SVM_EXIT_INVD : SVM_EXIT_WBINVD);

total: 0 errors, 1 warnings, 176 lines checked

Patch 37/50 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
38/50 Checking commit f975359de16a (target/i386: Remove user stub for cpu_vmexit)
39/50 Checking commit eff322ae04ac (target/i386: Cleanup read_crN, write_crN, lmsw)
40/50 Checking commit fc622c84f2fe (target/i386: Pass env to do_pause and do_hlt)
41/50 Checking commit 46e15b62a0e1 (target/i386: Move invlpg, hlt, monitor, mwait to sysemu)
42/50 Checking commit 946b94c8cd37 (target/i386: Unify invlpg, invlpga)
43/50 Checking commit 0010e63fa742 (target/i386: Inline user cpu_svm_check_intercept_param)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#51: 
deleted file mode 100644

total: 0 errors, 1 warnings, 22 lines checked

Patch 43/50 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
44/50 Checking commit 5555941839dc (target/i386: Eliminate user stubs for read/write_crN, rd/wrmsr)
45/50 Checking commit 428209161b08 (target/i386: Exit tb after wrmsr)
46/50 Checking commit 8f6e5bb5a6b5 (target/i386: Tidy gen_check_io)
47/50 Checking commit 6814811fafd4 (target/i386: Pass in port to gen_check_io)
48/50 Checking commit 50e4fb44910d (target/i386: Create helper_check_io)
49/50 Checking commit 1d820c0db728 (target/i386: Move helper_check_io to sysemu)
50/50 Checking commit e202c8cd8ba5 (target/i386: Remove user-only i/o stubs)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#70: 
deleted file mode 100644

total: 0 errors, 1 warnings, 34 lines checked

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

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210514151342.384376-1-richard.henderson@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Paolo Bonzini May 18, 2021, 10:31 a.m. UTC | #2 
On 14/05/21 17:12, Richard Henderson wrote:
> Supercedes: 20210228232321.322053-1-richard.henderson@linaro.org

> ("i386 cleanup part 3")

> 

> Claudio's prerequite patch set ("i386 cleanup part 2"), is now merged.

> Version 2 here fixes some minor rebase conflicts that crept in.

> Only 4 of the 50 patches have been reviewed.

> 

> Blurb from v1:

> This started out to address a fixme in Claudio's patch set, then

> wandered a bit, with cleanups and bug fixes in the code that I was

> touching.  I stopped when I reached my original goal of removing

> target/i386/tcg/user/*_stubs.c.

> 

> There are a lot of patches, but hopefully they're all small

> enough to be obvious.


Nice, thanks.  I only left minor comments on 27 and 30, feel free to do 
a pull request without v3.

Paolo
Richard Henderson May 18, 2021, 10:59 a.m. UTC | #3 
On 5/18/21 5:31 AM, Paolo Bonzini wrote:
> On 14/05/21 17:12, Richard Henderson wrote:

>> Supercedes: 20210228232321.322053-1-richard.henderson@linaro.org

>> ("i386 cleanup part 3")

>>

>> Claudio's prerequite patch set ("i386 cleanup part 2"), is now merged.

>> Version 2 here fixes some minor rebase conflicts that crept in.

>> Only 4 of the 50 patches have been reviewed.

>>

>> Blurb from v1:

>> This started out to address a fixme in Claudio's patch set, then

>> wandered a bit, with cleanups and bug fixes in the code that I was

>> touching.  I stopped when I reached my original goal of removing

>> target/i386/tcg/user/*_stubs.c.

>>

>> There are a lot of patches, but hopefully they're all small

>> enough to be obvious.

> 

> Nice, thanks.  I only left minor comments on 27 and 30, feel free to do a pull 

> request without v3.


Shall I take your comments then as "... Otherwise, R-B"?
Because you didn't actually give them for those two, and they're the only two 
without r-b tags.

You treated patch 49 similarly, but that one already had Phil's r-b.


r~
Paolo Bonzini May 18, 2021, 12:33 p.m. UTC | #4 
On 18/05/21 12:59, Richard Henderson wrote:
> On 5/18/21 5:31 AM, Paolo Bonzini wrote:

>> On 14/05/21 17:12, Richard Henderson wrote:

>>> Supercedes: 20210228232321.322053-1-richard.henderson@linaro.org

>>> ("i386 cleanup part 3")

>>>

>>> Claudio's prerequite patch set ("i386 cleanup part 2"), is now merged.

>>> Version 2 here fixes some minor rebase conflicts that crept in.

>>> Only 4 of the 50 patches have been reviewed.

>>>

>>> Blurb from v1:

>>> This started out to address a fixme in Claudio's patch set, then

>>> wandered a bit, with cleanups and bug fixes in the code that I was

>>> touching.  I stopped when I reached my original goal of removing

>>> target/i386/tcg/user/*_stubs.c.

>>>

>>> There are a lot of patches, but hopefully they're all small

>>> enough to be obvious.

>>

>> Nice, thanks.  I only left minor comments on 27 and 30, feel free to 

>> do a pull request without v3.

> 

> Shall I take your comments then as "... Otherwise, R-B"?

> Because you didn't actually give them for those two, and they're the 

> only two without r-b tags.

> 

> You treated patch 49 similarly, but that one already had Phil's r-b.


Well you're a maintainer so you don't really need R-b's but just to be 
precise: 27 would be completely different, while 30 should have had a 
r-b (it's just a commit message suggestion) and 49 should have had an 
"otherwise r-b".

Paolo