On 06/05/2020 16:10, Jan Beulich wrote:
> On 05.05.2020 19:32, Andrew Cooper wrote:
>> @@ -435,17 +435,13 @@ static int nsvm_vmcb_prepare4vmrun(struct vcpu *v,
>> struct cpu_user_regs *regs)
>> ASSERT(n2vmcb != NULL);
>>
>> /* Check if virtual VMCB cleanbits are valid */
>> - vcleanbits_valid = 1;
>> - if ( svm->ns_ovvmcb_pa == INVALID_PADDR )
>> - vcleanbits_valid = 0;
>> - if (svm->ns_ovvmcb_pa != nv->nv_vvmcxaddr)
>> - vcleanbits_valid = 0;
>> -
>> -#define vcleanbit_set(_name) \
>> - (vcleanbits_valid && ns_vmcb->cleanbits.fields._name)
>> + if ( svm->ns_ovvmcb_pa != INVALID_PADDR &&
>> + svm->ns_ovvmcb_pa != nv->nv_vvmcxaddr )
>> + clean = ns_vmcb->cleanbits;
> It looks to me as if the proper inversion of the original condition
> would mean == on the right side of &&, not != .
Oops, yes. Fixed.
>
>> --- a/xen/include/asm-x86/hvm/svm/vmcb.h
>> +++ b/xen/include/asm-x86/hvm/svm/vmcb.h
>> @@ -384,34 +384,21 @@ typedef union
>>
>> typedef union
>> {
>> - uint32_t bytes;
>> - struct
>> - {
>> - /* cr_intercepts, dr_intercepts, exception_intercepts,
>> - * general{1,2}_intercepts, pause_filter_count, tsc_offset */
>> - uint32_t intercepts: 1;
>> - /* iopm_base_pa, msrpm_base_pa */
>> - uint32_t iopm: 1;
>> - /* guest_asid */
>> - uint32_t asid: 1;
>> - /* vintr */
>> - uint32_t tpr: 1;
>> - /* np_enable, h_cr3, g_pat */
>> - uint32_t np: 1;
>> - /* cr0, cr3, cr4, efer */
>> - uint32_t cr: 1;
>> - /* dr6, dr7 */
>> - uint32_t dr: 1;
>> - /* gdtr, idtr */
>> - uint32_t dt: 1;
>> - /* cs, ds, es, ss, cpl */
>> - uint32_t seg: 1;
>> - /* cr2 */
>> - uint32_t cr2: 1;
>> - /* debugctlmsr, last{branch,int}{to,from}ip */
>> - uint32_t lbr: 1;
>> - uint32_t resv: 21;
>> - } fields;
>> + struct {
>> + bool intercepts:1; /* 0: cr/dr/exception/general1/2_intercepts,
>> + * pause_filter_count, tsc_offset */
> Could I talk you into omitting the 1/2 part, as there's going to
> be a 3 for at least MCOMMIT? Just "general" ought to be clear
> enough, I would think.
Can do. I'm not overly happy about this spilling onto two lines, but I
can't think of how to usefully shrink the comment further without losing
information.
~Andrew
