Re: [PATCH 3/6] x86/vmx: Introduce and use struct vmx_msr_bitmap

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 27/07/2017 07:02, Tian, Kevin wrote:
>> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
>> Sent: Wednesday, July 19, 2017 7:58 PM
>>
>> This avoids opencoding the bitmap bases in accessor functions.  Introduce a
>> build_assertions() function to check the structure layout against the manual
>> definiton.  In addition, drop some stale comments and ASSERT() that callers
>> pass an in-range MSR.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Acked-by: Kevin Tian <kevin.tian@intel.com>, with a small comment:
> [...]
>
>> diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h b/xen/include/asm-
>> x86/hvm/vmx/vmcs.h
>> index e318dc2..926e792 100644
>> --- a/xen/include/asm-x86/hvm/vmx/vmcs.h
>> +++ b/xen/include/asm-x86/hvm/vmx/vmcs.h
>> @@ -64,6 +64,14 @@ struct vmx_domain {
>>      unsigned int status;
>>  };
>>
>> +/* Layout of the MSR bitmap, as interpreted by hardware. */
>> +struct vmx_msr_bitmap {
>> +    unsigned long read_low  [0x2000 / BITS_PER_LONG];
>> +    unsigned long read_high [0x2000 / BITS_PER_LONG];
>> +    unsigned long write_low [0x2000 / BITS_PER_LONG];
>> +    unsigned long write_high[0x2000 / BITS_PER_LONG];
>> +};
>> +
> what about taking this chance to define 0x2000 into a macro
> for better readability?

What would you suggest to make this more readable?

The current way it is expressed by the manuals is that the bitmaps
covers MSRs 0 -> 0x1fff and 0xc0000000 -> 0xc0001fff which is where the
0x2000 is derived from.

Would this be better?

/* Layout of the MSR bitmap, as interpreted by hardware. */
struct vmx_msr_bitmap {
    /* Covers MSRs 0 -> 0x1fff. */
    unsigned long read_low  [0x2000 / BITS_PER_LONG];
    unsigned long read_high [0x2000 / BITS_PER_LONG];
    /* Covers MSRs 0xc0000000 -> 0xc0001fff. */
    unsigned long write_low [0x2000 / BITS_PER_LONG];
    unsigned long write_high[0x2000 / BITS_PER_LONG];
};

~Andrew

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