Re: [PATCH] x86: Fix build with the get/set_reg() infrastructure

[Andrew Cooper <amc96@xxxxxxxx>]

On 21/01/2022 11:03, Jan Beulich wrote:
> On 21.01.2022 11:49, Andrew Cooper wrote:
>> I clearly messed up concluding that the stubs were safe to drop.
>>
>> The is_{pv,hvm}_domain() predicates are not symmetrical with both CONFIG_PV
>> and CONFIG_HVM.  As a result logic of the form `if ( pv/hvm ) ... else ...`
>> will always have one side which can't be DCE'd.
>>
>> While technically only the hvm stubs are needed, due to the use of the
>> is_pv_domain() predicate in guest_{rd,wr}msr(), sort out the pv stubs too to
>> avoid leaving a bear trap for future users.
> Well, as said on irc - only the PV stubs ought to be needed if the
> conditionals always used "if ( is_hvm_...() )" / "if ( !is_hvm_...() )".
> Despite us making use of this property elsewhere, you appear to dislike
> that though ...

Yes - I consider that an accident at best, and not something which
people can reasonably be expected to know or use.

If nothing else, the pv form is the more useful one to use, given a
potential pvh-dom0 future where CONFIG_PV might credibly be dropped by
downstreams.

>
> Hence / nevertheless:
>
>> Fixes: 88d3ff7ab15d ("x86/guest: Introduce {get,set}_reg() infrastructure")
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks, but CI still says no.

Some compilers don't tolerate the lack of a return value in a non-void
function, despite the ASSERT_UNREACHABLE().  It might be down to the
absence of __builtin_unreahcable().

I've folded "return 0;" into the two get stubs, and will see how that
fairs, but given how trivial it is, I don't intend to post a v2 before
committing.

~Andrew