Re: [PATCH v3] xen: handle paged gfn in wrmsr_hypervisor_regs

[Keir Fraser <keir.xen@gmail.com>]

On 03/05/2013 16:17, "Olaf Hering" <olaf@aepfle.de> wrote:

> # HG changeset patch
> # User Olaf Hering <olaf@aepfle.de>
> # Date 1367593457 -7200
> # Node ID b8af60cf8282bfddb13cc10e4ffaf0c396a15104
> # Parent  9df019eef776d129c2abb130d1458914fe1ecac4
> xen: handle paged gfn in wrmsr_hypervisor_regs

Acked-by: Keir Fraser <keir@xen.org>

> If xenpaging is started very early for a guest the gfn for the hypercall
> page may be paged-out already. This leads to a guest crash:
> 
> ...
> (XEN) HVM10: Allocated Xen hypercall page at 169ff000
> (XEN) traps.c:654:d10 Bad GMFN 169ff (MFN 3e900000000) to MSR 40000000
> (XEN) HVM10: Detected Xen v4.3
> (XEN) io.c:201:d10 MMIO emulation failed @ 0008:c2c2c2c2: 18 7c 55 6d 03 83 ff
> ff 10 7c
> (XEN) hvm.c:1253:d10 Triple fault on VCPU0 - invoking HVM shutdown action 1.
> (XEN) HVM11: HVM Loader
> ...
> 
> Update return codes of wrmsr_hypervisor_regs, update callers to deal
> with the new return codes:
>  0: not handled
>  1: handled
>  -EINVAL: error during handling
>  -EAGAIN: retry
> 
> 
> Also update the gdprintk to handle a page value of NULL to avoid
> printing a bogus MFN value. Update also computing of MSR value in
> gdprintk, the idx was always zero.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> 
> diff -r 9df019eef776 -r b8af60cf8282 xen/arch/x86/hvm/svm/svm.c
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -1569,7 +1569,7 @@ static int svm_msr_read_intercept(unsign
>  
>  static int svm_msr_write_intercept(unsigned int msr, uint64_t msr_content)
>  {
> -    int ret;
> +    int ret, result = X86EMUL_OKAY;
>      struct vcpu *v = current;
>      struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
>      int sync = 0;
> @@ -1682,14 +1682,24 @@ static int svm_msr_write_intercept(unsig
>          if ( wrmsr_viridian_regs(msr, msr_content) )
>              break;
>  
> -        wrmsr_hypervisor_regs(msr, msr_content);
> +        switch ( wrmsr_hypervisor_regs(msr, msr_content) )
> +        {
> +            case -EAGAIN:
> +                result = X86EMUL_RETRY;
> +                break;
> +            case 0:
> +            case 1:
> +                break;
> +            default:
> +         goto gpf;
> +        }
>          break;
>      }
>  
>      if ( sync )
>          svm_vmload(vmcb);
>  
> -    return X86EMUL_OKAY;
> +    return result;
>  
>   gpf:
>      hvm_inject_hw_exception(TRAP_gp_fault, 0);
> diff -r 9df019eef776 -r b8af60cf8282 xen/arch/x86/hvm/vmx/vmx.c
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2088,7 +2088,16 @@ static int vmx_msr_write_intercept(unsig
>              case HNDL_unhandled:
>                  if ( (vmx_write_guest_msr(msr, msr_content) != 0) &&
>                       !is_last_branch_msr(msr) )
> -                    wrmsr_hypervisor_regs(msr, msr_content);
> +                    switch ( wrmsr_hypervisor_regs(msr, msr_content) )
> +                    {
> +                        case -EAGAIN:
> +                            return X86EMUL_RETRY;
> +                        case 0:
> +                        case 1:
> +                            break;
> +                        default:
> +                            goto gp_fault;
> +                    }
>                  break;
>              case HNDL_exception_raised:
>                  return X86EMUL_EXCEPTION;
> diff -r 9df019eef776 -r b8af60cf8282 xen/arch/x86/traps.c
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -634,25 +634,33 @@ int wrmsr_hypervisor_regs(uint32_t idx,
>          unsigned long gmfn = val >> 12;
>          unsigned int idx  = val & 0xfff;
>          struct page_info *page;
> +        p2m_type_t t;
>  
>          if ( idx > 0 )
>          {
>              gdprintk(XENLOG_WARNING,
>                       "Out of range index %u to MSR %08x\n",
>                       idx, 0x40000000);
> -            return 0;
> +            return -EINVAL;
>          }
>  
> -        page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC);
> +        page = get_page_from_gfn(d, gmfn, &t, P2M_ALLOC);
>  
>          if ( !page || !get_page_type(page, PGT_writable_page) )
>          {
>              if ( page )
>                  put_page(page);
> +
> +            if ( p2m_is_paging(t) )
> +            {
> +                p2m_mem_paging_populate(d, gmfn);
> +                return -EAGAIN;
> +            }
> +
>              gdprintk(XENLOG_WARNING,
>                       "Bad GMFN %lx (MFN %lx) to MSR %08x\n",
> -                     gmfn, page_to_mfn(page), base + idx);
> -            return 0;
> +                     gmfn, page ? page_to_mfn(page) : -1UL, base);
> +            return -EINVAL;
>          }
>  
>          hypercall_page = __map_domain_page(page);
> @@ -2490,7 +2498,7 @@ static int emulate_privileged_op(struct
>                  goto fail;
>              break;
>          default:
> -            if ( wrmsr_hypervisor_regs(regs->ecx, msr_content) )
> +            if ( wrmsr_hypervisor_regs(regs->ecx, msr_content) == 1 )
>                  break;
>  
>              rc = vmce_wrmsr(regs->ecx, msr_content);
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel