On 02.04.2025 01:34, Andrew Cooper wrote:
> With the new toolchain baseline, we can make use of asm goto() in certain
> places, and the VMXON invocation is one example.
>
> This removes the logic to set up rc (including a fixup section where bactraces
> have no connection to the invoking function), the logic to decode it, and the
> default case which was dead but in a way the compiler couldn't prove
> previously.
>
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
> ---
> CC: Jan Beulich <jbeul...@suse.com>
> CC: Roger Pau Monné <roger....@citrix.com>
> CC: dm...@proton.me
>
> RFC. To be rebased over Denis' general cleanup.
LGTM. Can't this actually replace some of his cleanup? Judging from
base-commit: at the bottom this isn't based on his work. In which case:
Reviewed-by: Jan Beulich <jbeul...@suse.com>
> In principle, we can split fail into fail_valid and fail_invalid, allowing us
> to spot the VMfail("VMXON executed in VMX root operation") case from the
> pseduocode. However, getting that involves a VMREAD of VM_INSTRUCTION_ERROR,
> and error handling in case there isn't a loaded VMCS, so I think the
> complexity is unwarranted in this case.
+1
> Bloat-o-meter:
> add/remove: 0/0 grow/shrink: 1/1 up/down: 13/-32 (-19)
> Function old new delta
> _vmx_cpu_up.cold 2460 2473 +13
> _vmx_cpu_up 1803 1771 -32
>
> The if ( 0 ) isn't terribly nice, but it's the least bad option I could come
> up with. It does allow the structure of the switch() to remain largely
> intact.
For the purpose of the diff here I agree. In a subsequent change we could then
still move the whole blob to the end of the function. Especially if some of
the static analysis tools didn't like the "if ( 0 )".
Jan
