Re: [Xen-devel] [PATCH 1/2] x86/vmx: dump MSR load area

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.08.16 at 08:59, <msw@xxxxxxxx> wrote:
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -1740,6 +1740,20 @@ static void vmx_dump_sel2(char *name, uint32_t lim)
>      printk("%s:            %08x %016"PRIx64"\n", name, limit, base);
>  }
>  
> +static void vmx_dump_vcpu_msr_area(struct vcpu *v)

const

> +{
> +    const struct vmx_msr_entry *msr_area = v->arch.hvm_vmx.msr_area;
> +    unsigned int i, msr_count = v->arch.hvm_vmx.msr_count;
> +
> +    printk("  msr_count = %d\n", v->arch.hvm_vmx.msr_count);

Please use the local variable you have just latched and %u.

> +    for ( i = 0; i < msr_count; i++ )
> +    {
> +        printk("  msr_area[%d].index=0x%08x .data=0x%016lx\n",

[%u], and I'm not sure the 0x prefixes are really all that helpful
here. I think it would also be beneficial to shorten the logged text
a little, as this may consume quite a bit of serial line bandwidth.
How about "   MSR[%08x]=%016lx\n"?

> @@ -1879,6 +1893,13 @@ void vmcs_dump_vcpu(struct vcpu *v)
>           (SECONDARY_EXEC_ENABLE_VPID | SECONDARY_EXEC_ENABLE_VM_FUNCTIONS) 
> )
>          printk("Virtual processor ID = 0x%04x VMfunc controls = %016lx\n",
>                 vmr16(VIRTUAL_PROCESSOR_ID), vmr(VM_FUNCTION_CONTROL));
> +    printk("EXIT MSR load count = 0x%04x\n",
> +           (uint32_t)vmr(VM_EXIT_MSR_LOAD_COUNT));
> +    printk("EXIT MSR store count = 0x%04x\n",
> +           (uint32_t)vmr(VM_EXIT_MSR_STORE_COUNT));
> +    printk("ENTRY MSR load count = 0x%04x\n",
> +           (uint32_t)vmr(VM_ENTRY_MSR_LOAD_COUNT));

First - do you really need to make three log lines out of these? And
then, please use vmr32(), as the neighboring vmr16() suggests.
Plus finally - please log all four counts consistently either in hex
or in dec.

Jan


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