On 02/04/2025 10:40 am, Jan Beulich wrote:
> 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>
Oh. I was expecting there to be far more debate on this.
In which case I expect it will be easiest for this patch to go in first,
and supersede Denis' cleanup to __vmxon(), so as not to rework it twice
in quick succession. (Sorry.)
>
>> 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 )".
Actually, doing that is still a nice diff to read. I think I'll take
this approach. I'll send out a v2 in just a moment.
~Andrew
