Re: [PATCH 1/5] VMX: dump full guest state

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

On 20/01/15 11:05, Jan Beulich wrote:
> Several guest state fields did not get dumped so far. Where suitable
> (to reduce the amount of output) make some of the dumping conditional
> upon guest settings (this isn't required for correctness as vmr()
> already uses __vmread_safe(), i.e. it is fine to access non-existing
> fields).
>
> Move CR3_TARGET_* and TSC_OFFSET processing into the control state
> section, at once making the upper bound of CR3_TARGET_VALUEn printed
> depend on CR3_TARGET_COUNT (which architecturally can be higher than
> 4).
>
> Also rename GUEST_PDPTRn to GUEST_PDPTEn (matching the SDM naming) and
> group them as well as CR3_TARGET_VALUEn similar to EOI_EXIT_BITMAP.
>
> Finally, drop casts - they haven't been needed anymore since the
> dropping of 32-bit support (and some of them were not really needed in
> the first place). Introduce vmr16() and vmr32() helper macros to avoid
> the "l" printk format modifier and at the same time validate that only
> 16-/32-bit fields get accessed this way.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> Is cpu_has_vmx_pat properly defined? Shouldn't this take into
> consideration all three respective flags (VM_EXIT_SAVE_GUEST_PAT,
> VM_EXIT_LOAD_HOST_PAT, and VM_ENTRY_LOAD_GUEST_PAT)?
>
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -1395,6 +1395,16 @@ static inline unsigned long vmr(unsigned
>      return __vmread_safe(field, &val) ? val : 0;
>  }
>  
> +#define vmr16(fld) ({             \
> +    BUILD_BUG_ON((fld) & 0x6001); \
> +    (uint16_t)vmr(fld);           \
> +})
> +
> +#define vmr32(fld) ({                         \
> +    BUILD_BUG_ON(((fld) & 0x6001) != 0x4000); \
> +    (uint16_t)vmr(fld);                       \

uint32_t ?

Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>