On Mon, Mar 18, 2019 at 09:46:22PM -0400, Krish Sadhukhan wrote:
> According to section "Checks on Host Control Registers and MSRs" in Intel
> SDM vol 3C, the following check is performed on vmentry of L2 guests:
I think you can leave off "of L2 guests". That implies VM-Entry to other
levels are not subject to this check.
>
> If the "load IA32_PAT" VM-exit control is 1, the value of the field
> for the IA32_PAT MSR must be one that could be written by WRMSR
> without fault at CPL 0. Specifically, each of the 8 bytes in the
> field must have one of the values 0 (UC), 1 (WC), 4 (WT), 5 (WP),
> 6 (WB), or 7 (UC-).
>
> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@xxxxxxxxxx>
> Reviewed-by: Karl Heubaum <karl.heubaum@xxxxxxxxxx>
> ---
> arch/x86/kvm/vmx/nested.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index 3170e291215d..a1b44d930d26 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -2583,6 +2583,24 @@ static int nested_check_vm_entry_controls(struct kvm_vcpu *vcpu,
> return 0;
> }
>
> +/*
> + * Fields in IA32_PAT MSR are allowed to contain one of the
> + * following values:
> + * 0 (UC), 1 (WC), 4 (WT), 5 (WP), 6 (WB), or 7 (UC-)
> + */
> +static int nested_check_pat_fields(u64 msr)
Maybe just nested_check_pat()? The SDM uses "field" to refer to the
VMCS field, not the individual bytes in the MSR.
> +{
> + int i;
> + u64 tmp = (msr >> i * 8) & 0xff;
This uses 'i' uninitialized and is obviously intended to be in the for
loop. You can actually avoid 'i' and the multiplication entirely since
'0' is a legal PAT value (UC), e.g.:
u64 tmp;
for ( ; msr; msr >>= 8) {
tmp = msr & 0xff;
if (tmp == 0x2 || tmp == 0x3 || tmp > 0x7)
return -EINVAL;
}
return 0;
> +
> + for (i = 0; i < 8; i++) {
> + if (tmp == 0x2 || tmp == 0x3 || tmp > 0x7)
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> /*
> * Checks related to Host Control Registers and MSRs
> */
> @@ -2595,6 +2613,12 @@ static int nested_check_host_control_regs(struct kvm_vcpu *vcpu,
> !nested_host_cr4_valid(vcpu, vmcs12->host_cr4) ||
> !nested_cr3_valid(vcpu, vmcs12->host_cr3))
> return -EINVAL;
> +
> + if (vmcs12->vm_exit_controls & VM_EXIT_LOAD_IA32_PAT) {
> + if (nested_check_pat_fields(vmcs12->host_ia32_pat))
This can be a single if statement connected with &&.
> + return -EINVAL;
> + }
> +
> /*
> * If the load IA32_EFER VM-exit control is 1, bits reserved in the
> * IA32_EFER MSR must be 0 in the field for that register. In addition,
> --
> 2.17.2
>
