Re: [PATCH v2 5/4] x86/hvm: Drop hvm_{get,set}_guest_bndcfgs() and use {get,set}_regs() instead

From: Jan Beulich <jbeulich@suse.com>
To: Andrew Cooper <amc96@srcf.net>,
	Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>,
	"Jun Nakajima" <jun.nakajima@intel.com>,
	"Kevin Tian" <kevin.tian@intel.com>,
	Xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH v2 5/4] x86/hvm: Drop hvm_{get,set}_guest_bndcfgs() and use {get,set}_regs() instead
Date: Thu, 20 Jan 2022 09:20:18 +0100	[thread overview]
Message-ID: <0323edc1-338c-a1a4-d5e0-a59950bb8dba@suse.com> (raw)
In-Reply-To: <00b2e4ee-eb60-d336-b9af-04e410fdc5e2@srcf.net>

On 19.01.2022 17:53, Andrew Cooper wrote:
> On 19/01/2022 13:50, Jan Beulich wrote:
>> On 17.01.2022 20:25, Andrew Cooper wrote:
>>> @@ -323,10 +324,9 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val)
>>>          break;
>>>  
>>>      case MSR_IA32_BNDCFGS:
>>> -        if ( !cp->feat.mpx || !is_hvm_domain(d) ||
>>> -             !hvm_get_guest_bndcfgs(v, val) )
>>> +        if ( !cp->feat.mpx ) /* Implies Intel HVM only */
>> Wouldn't it make sense to accompany this comment by ...
>>
>>>              goto gp_fault;
>>> -        break;
>>     ASSERT(is_hvm_domain(d));
>>
>> (and then the same on the "set" path)?
> 
> So this is the reason for the default logic in the {get,set}_reg()
> path.  The absence of MSR_BNDCFGS in the PV and SVM paths will cause the
> VM to be crashed cleanly.  If you're on a VMX on a non-MPX capable
> system, the VMREAD/VMWRITE will hit a BUG (which in due course I want to
> downgrade to a domain crash).
> 
> It's a bit more friendly than an ASSERT() (doesn't take the system
> down), is present in release builds too, and more precise as it excludes
> SVM too.

I see, makes sense.

Jan