Re: [PATCH 1/2][KVM] nVMX x86: Add a check for reserved bits [11:0] of PML address

Krish Sadhukhan <krish.sadhukhan@xxxxxxxxxx> · Wed, 26 Sep 2018 14:15:19 -0700

On 09/26/2018 01:03 PM, Jim Mattson wrote:
On Wed, Sep 26, 2018 at 11:18 AM, Krish Sadhukhan
<krish.sadhukhan@xxxxxxxxxx> wrote:
According to section "Checks on VMX Controls" in Intel SDM vol 3C, bits 11:0
of the PML address must be 0.

Signed-off-by: Krish Sadhukhan <krish.sadhukhan@xxxxxxxxxx>
Reviewed-by: Mark Kanda <mark.kanda@xxxxxxxxxx>
---
  arch/x86/include/asm/vmx.h | 2 ++
  arch/x86/kvm/vmx.c         | 3 ++-
  2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index 9527ba5..2c118ad 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -339,6 +339,8 @@ enum vmcs_field {
         HOST_RIP                        = 0x00006c16,
  };

+#define        PML_ADDRESS_RESV_BITS           0xfff
+
  /*
   * Interruption-information format
   */
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 533a327..49e707d 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -11712,7 +11712,8 @@ static int nested_vmx_check_pml_controls(struct kvm_vcpu *vcpu,
         if (nested_cpu_has2(vmcs12, SECONDARY_EXEC_ENABLE_PML)) {
                 if (!nested_cpu_has_ept(vmcs12) ||
                     !IS_ALIGNED(address, 4096)  ||
-                   address >> maxphyaddr)
+                   address >> maxphyaddr ||
+                   address & PML_ADDRESS_RESV_BITS)
As Sean points out, the reserved bit check is redundant. Rather than

Agreed.

open-coding the checks, this should be probably just be:
         if (!nested_cpu_has_ept(vmcs12) ||
             !page_address_valid(vcpu, vmcs12->pml_address))

Yes, we should use page_address_valid().  Also, we should use 
nested_cpu_has_pml() instead of using nested_cpu_has2():

     if (nested_cpu_has_pml() && (!nested_cpu_has_ept(vmcs12) ||
            !page_address_valid(vcpu, vmcs12->pml_address))
    .            return -EINVAL;






