Re: [PATCH v2 3/3] x86/vmx: Disallow the use of inactivity states

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 12/01/2024 11:04 am, Jan Beulich wrote:
> On 12.01.2024 11:43, Andrew Cooper wrote:
>> On 12/01/2024 10:37 am, Jan Beulich wrote:
>>> On 12.01.2024 00:13, Andrew Cooper wrote:
>>>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>>>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>>>> @@ -1551,7 +1551,10 @@ static void cf_check vmx_set_nonreg_state(struct 
>>>> vcpu *v,
>>>>  {
>>>>      vmx_vmcs_enter(v);
>>>>  
>>>> -    __vmwrite(GUEST_ACTIVITY_STATE, nrs->vmx.activity_state);
>>>> +    if ( nrs->vmx.activity_state )
>>>> +        domain_crash(v->domain, "Attempt to set activity_state %#lx\n",
>>>> +                     nrs->vmx.activity_state);
>>> Might be useful to log the offending vCPU here?
>> Already covered.  the innards of __domain_crash() does:
>>
>>     else if ( d == current->domain )
>>     {
>>         printk("Domain %d (vcpu#%d) crashed on cpu#%d:\n",
>>         ...
> Except that afaict v != current here at all times (at least as far as
> current use of the function goes).

Hmm.  That's irritating.

In this case, it's a dead logic path - hence why in v1 I simply deleted it.

But I would prefer not to have to rely on a human getting an error
message right in order to get proper logging.

I suppose what we really want is a vcpu_crash(), but this is now firmly
in the realms of the cleanup patch I still haven't had time to repost.

I think I'll extend this with %pv for now, which can be dropped when
vcpu_crash() appears.

~Andrew