Re: [PATCH v4 2/4] x86/hvm: Disable cross-vendor handling in #UD handler

[Alejandro Vallejo <alejandro.garciavallejo@xxxxxxx>]

On Wed Mar 11, 2026 at 3:59 PM CET, Jan Beulich wrote:
> On 11.03.2026 15:27, Alejandro Vallejo wrote:
>> Remove cross-vendor support now that VMs can no longer have a different
>> vendor than the host.
>> 
>> While at it, refactor the function to exit early and skip initialising
>> the emulation context when FEP is not enabled.
>> 
>> No functional change intended.
>> 
>> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@xxxxxxx>
>> ---
>> v4:
>>   * Reverted refactor of the `walk` variable assignment
>
> "Revert" as in "move it even farther away from the original".

Revert as in not split the assignment and restore the orignal syntax _of the
assignment_, which was the main focus of the prior discussion.

It's hardly my intention to add unrequested changes, but I can't address that
which isn't explicitly requested.

> As said, you want re-indentation,

This is an ambiguous piece of advice.

Of what? That can mean moving the prior logic back to its original location and
crate a minimal diff (1) or simply collapsing the indentation of the block (2).

(1) can't be done with hvm context initialiser moving after the early exit,
which I explicitly mentioned in the commit message I wanted to do.

(2) can't happen because declarations and statements cannot be mixed (though I
really wish we dropped that rule).

There's a third option of keeping a silly { ... } around just for indentation
purposes, but that's worse than either of the other 2 options.

Maybe there's a fourth code arrangement in your head that does all this in a
way you find less intrusive and I just don't see it. If so, feel free to send
a patch I can review. It'll be faster for the both of us. Or tell me precisely
what's at fault here.

If it's the diff, I'll go for option (1) above. I don't care enough about it to
argue.

> so please do just that, nothing else that isn't
> explicitly justified (like the moving of hvm_emulate_init_once() is).

I'm not sure if you're fine with that motion because it's in the commit message
or not because it's a refactor that shouldn't be in the patch. This statement
can be read either way.

> With
> this put back in its original shape (can do while committing, I suppose):
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

I don't think it's very obvious what you mean to do on commit, so it wouldn't be
appropriate to agree to your adjustments, seeing how I just don't know what they
are. I'm happy to send a v4.5 on this particular patch with whatever else needs
modifying. Or a full v5 even. Or review whatever you wish to send as a v4.5 of
this patch.

Your pick.

>> + reinject:
>
> I'm inclined to suggest to indent this the same as the case labels.

I didn't notice the extra statement in CODING_STYLE for labels inside switches.
I tend to do that myself, but thought it wasn't in Xen's style. Sounds good
then.

Cheers,
Alejandro