> From: Sergey Dyasli [mailto:[email protected]]
> Sent: Monday, July 24, 2017 9:48 PM
>
> This structure provides a convenient way of accessing contents of
> VMX MSRs: every bit value is accessible by its name. Bit names match
> existing Xen's definitions as close as possible. The structure also
> contains the bitmap of available MSRs since not all of them may be
> available on a particular H/W.
>
> A set of helper functions is introduced to provide a simple way of
> interacting with the new structure.
>
> Signed-off-by: Sergey Dyasli <[email protected]>
> ---
> v1 --> v2:
> - Replaced MSR indices with MSR names in struct vmx_msr_policy's
> comments
> - Named "always zero bit" 31 of basic msr as mbz
> - Added placeholder bits into union vmfunc
> - Added structures cr0_bits and cr4_bits
> - Added MSR_IA32_VMX_LAST define to use instead of
> MSR_IA32_VMX_VMFUNC
> - vmx_msr_available() now uses pointer to const struct vmx_msr_policy
> - build_assertions() now uses local struct vmx_msr_policy
> - Added BUILD_BUG_ON to check that width of vmx_msr_policy::available
> bitmap is enough for all existing VMX MSRs
> - Helpers get_vmx_msr_val(), get_vmx_msr_ptr() and gen_vmx_msr_mask()
> are added
>
> xen/arch/x86/hvm/vmx/vmcs.c | 78 ++++++++
> xen/include/asm-x86/hvm/vmx/vmcs.h | 380
> +++++++++++++++++++++++++++++++++++++
> xen/include/asm-x86/msr-index.h | 1 +
> 3 files changed, 459 insertions(+)
>
> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
> index 8103b20d29..33715748f0 100644
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -144,6 +144,40 @@ static void __init vmx_display_features(void)
> printk(" - none\n");
> }
>
> +bool vmx_msr_available(const struct vmx_msr_policy *p, uint32_t msr)
regarding to naming, many functions use vmx_ as prefix
with later strings represent the actual purpose e.g.
vmx_msr_read_intercept which is about general msr read
interception. while here your intention is whether "vmx_msr"
is available, which is for specific VMX MSRs. similar for
vmx_msr_policy which reads more general than the real
intention here. Can we find a way to differentiate? SDM
calls this category as "VMX CAPABILITY REPORTING FACILITY",
maybe using vmxcap_msr?
> +{
> + if ( msr < MSR_IA32_VMX_BASIC || msr > MSR_IA32_VMX_LAST )
> + return 0;
then why not also introducing MSR_IA32_VMX_FIRST for
better readability?
> +
> + return p->available & (1u << (msr - MSR_IA32_VMX_BASIC));
> +}
> +
> +uint64_t get_vmx_msr_val(const struct vmx_msr_policy *p, uint32_t msr)
> +{
> + if ( !vmx_msr_available(p, msr))
> + return 0;
0 is a valid MSR value. better return value in a pointer parameter and
then return error number here.
> +
> + return p->msr[msr - MSR_IA32_VMX_BASIC];
> +}
Thanks
Kevin
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
