On 29/08/20 02:48, Krish Sadhukhan wrote:
> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@xxxxxxxxxx>
> ---
> arch/x86/kvm/svm/nested.c | 51 ++++++++++++++++++++++-----------------
> 1 file changed, 29 insertions(+), 22 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index fb68467e6049..7a51ce465f3e 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -215,9 +215,35 @@ static bool nested_vmcb_check_controls(struct vmcb_control_area *control)
> return true;
> }
>
> +static bool nested_vmcb_check_cr3_cr4(struct vcpu_svm *svm,
> + struct vmcb_save_area *save)
> +{
> + bool nested_vmcb_lma =
> + (save->efer & EFER_LME) &&
> + (save->cr0 & X86_CR0_PG);
> +
> + if (!nested_vmcb_lma) {
> + if (save->cr4 & X86_CR4_PAE) {
> + if (save->cr3 & MSR_CR3_LEGACY_PAE_RESERVED_MASK)
> + return false;
> + } else {
> + if (save->cr3 & MSR_CR3_LEGACY_RESERVED_MASK)
> + return false;
> + }
> + } else {
> + if (!(save->cr4 & X86_CR4_PAE) ||
> + !(save->cr0 & X86_CR0_PE) ||
> + (save->cr3 & MSR_CR3_LONG_MBZ_MASK))
> + return false;
> + }
> + if (kvm_valid_cr4(&svm->vcpu, save->cr4))
> + return false;
> +
> + return true;
> +}
> +
> static bool nested_vmcb_checks(struct vcpu_svm *svm, struct vmcb *vmcb)
> {
> - bool nested_vmcb_lma;
> if ((vmcb->save.efer & EFER_SVME) == 0)
> return false;
>
> @@ -228,25 +254,7 @@ static bool nested_vmcb_checks(struct vcpu_svm *svm, struct vmcb *vmcb)
> if (!kvm_dr6_valid(vmcb->save.dr6) || !kvm_dr7_valid(vmcb->save.dr7))
> return false;
>
> - nested_vmcb_lma =
> - (vmcb->save.efer & EFER_LME) &&
> - (vmcb->save.cr0 & X86_CR0_PG);
> -
> - if (!nested_vmcb_lma) {
> - if (vmcb->save.cr4 & X86_CR4_PAE) {
> - if (vmcb->save.cr3 & MSR_CR3_LEGACY_PAE_RESERVED_MASK)
> - return false;
> - } else {
> - if (vmcb->save.cr3 & MSR_CR3_LEGACY_RESERVED_MASK)
> - return false;
> - }
> - } else {
> - if (!(vmcb->save.cr4 & X86_CR4_PAE) ||
> - !(vmcb->save.cr0 & X86_CR0_PE) ||
> - (vmcb->save.cr3 & MSR_CR3_LONG_RESERVED_MASK))
> - return false;
> - }
> - if (kvm_valid_cr4(&svm->vcpu, vmcb->save.cr4))
> + if (!nested_vmcb_check_cr3_cr4(svm, &(vmcb->save)))
> return false;
>
> return nested_vmcb_check_controls(&vmcb->control);
> @@ -1114,9 +1122,8 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
> /*
> * Validate host state saved from before VMRUN (see
> * nested_svm_check_permissions).
> - * TODO: validate reserved bits for all saved state.
> */
> - if (!(save.cr0 & X86_CR0_PG))
> + if (!nested_vmcb_check_cr3_cr4(svm, &save))
> return -EINVAL;
Removing the "if" is incorrect. Also are there really no more reserved
bits to check?
Paolo
