Re: [Xen-devel] [PATCH 4/4] SVM: clean up svm_vmcb_isvalid()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 31/05/17 15:50, Boris Ostrovsky wrote:
> On 05/31/2017 08:14 AM, Andrew Cooper wrote:
>> On 31/05/17 08:23, Jan Beulich wrote:
>>> - correct CR3 and CR4 checks
>>> - add vcpu parameter (to include in log messages) and constify vmcb one
>>> - use bool/true/false
>>> - use accessors
>>> - adjust formatting
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>>
>>> --- a/xen/arch/x86/hvm/svm/nestedsvm.c
>>> +++ b/xen/arch/x86/hvm/svm/nestedsvm.c
>>> @@ -658,13 +658,13 @@ static int nsvm_vmcb_prepare4vmrun(struc
>>>      /* Cleanbits */
>>>      n2vmcb->cleanbits.bytes = 0;
>>>  
>>> -    rc = svm_vmcb_isvalid(__func__, ns_vmcb, 1);
>>> +    rc = svm_vmcb_isvalid(__func__, ns_vmcb, v, true);
>>>      if (rc) {
>>>          gdprintk(XENLOG_ERR, "virtual vmcb invalid\n");
>>>          return NSVM_ERROR_VVMCB;
>>>      }
>>>  
>>> -    rc = svm_vmcb_isvalid(__func__, n2vmcb, 1);
>>> +    rc = svm_vmcb_isvalid(__func__, n2vmcb, v, true);
>> As these are the only two callsites, I don't think the __func__ or
>> verbose parameters are useful.  I'd just drop them.
> I actually think keeping this is useful. We indeed have only two
> invocations but someone debugging a problem may want to add a few more.

Why?  Its clear where it is being called from by the following "$FOO
invalid" log message.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel