Re: [PATCH 1/1] x86/AMD: Fix nested svm crash due to assertion in __virt_to_maddr

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

On 04/07/13 20:36, suravee.suthikulpanit@amd.com wrote:
> From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>
> Fix assertion in __virt_to_maddr when starting nested SVM guest
> in debug mode. Investigation has shown that svm_vmsave/svm_vmload
> make use of __pa() with invalid address.
>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>  xen/arch/x86/hvm/svm/svm.c        |    4 ++--
>  xen/include/asm-x86/hvm/svm/svm.h |   14 ++++++++++++++
>  2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index acd2d49..944569a 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -1809,7 +1809,7 @@ svm_vmexit_do_vmload(struct vmcb_struct *vmcb,
>          goto inject;
>      }
>  
> -    svm_vmload(nv->nv_vvmcx);
> +    nestedsvm_vmload(nv->nv_vvmcxaddr);
>      /* State in L1 VMCB is stale now */
>      v->arch.hvm_svm.vmcb_in_sync = 0;
>  
> @@ -1845,7 +1845,7 @@ svm_vmexit_do_vmsave(struct vmcb_struct *vmcb,
>          goto inject;
>      }
>  
> -    svm_vmsave(nv->nv_vvmcx);
> +    nestedsvm_vmsave(nv->nv_vvmcxaddr);
>  
>      __update_guest_eip(regs, inst_len);
>      return;
> diff --git a/xen/include/asm-x86/hvm/svm/svm.h b/xen/include/asm-x86/hvm/svm/svm.h
> index 64e7e25..909e8a1 100644
> --- a/xen/include/asm-x86/hvm/svm/svm.h
> +++ b/xen/include/asm-x86/hvm/svm/svm.h
> @@ -55,6 +55,20 @@ static inline void svm_vmsave(void *vmcb)
>          : : "a" (__pa(vmcb)) : "memory" );
>  }
>  
> +static inline void nestedsvm_vmload(uint64_t vmcb)

unsigned long if this is actually an address.

But more importantly, if virt_to_maddr() fails an assertion because the
virtual address is not a persistent mapping, what is going to happen
when the virtual mapping (potentially) changes while the vvmcx is in use?

~Andrew

> +{
> +    asm volatile (
> +        ".byte 0x0f,0x01,0xda" /* vmload */
> +        : : "a" (vmcb) : "memory" );
> +}
> +
> +static inline void nestedsvm_vmsave(uint64_t vmcb)
> +{
> +    asm volatile (
> +        ".byte 0x0f,0x01,0xdb" /* vmsave */
> +        : : "a" (vmcb) : "memory" );
> +}
> +
>  static inline void svm_invlpga(unsigned long vaddr, uint32_t asid)
>  {
>      asm volatile (