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

[Suravee Suthikulanit <suravee.suthikulpanit@xxxxxxx>]

On 7/29/2013 5:43 AM, Tim Deegan wrote:
> Hi,
>
> At 16:46 -0500 on 26 Jul (1374857167), suravee.suthikulpanit@xxxxxxx wrote:
> > From: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
> >
> > 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@xxxxxxx>
> This looks much better, but I have a few comments still:
>
> > +static struct page_info *
> > +_get_vmcb_page(struct domain *d, uint64_t vmcbaddr)
> Can you give this a name that makes it clearer that it's for nested
> VMCBs and not part of the handling of 'real' VMCBs?  Also, please drop
> the leading underscore.
What about "get_nvmcb_page"?

> > +    {
> > +        gdprintk(XENLOG_ERR,
> > +            "VMLOAD: mapping vmcb L1-GPA to MPA failed, injecting #UD\n");
> > +        ret = TRAP_invalid_op;
> The documentation for VMLOAD suggests TRAP_gp_fault for this case.
OK, I have also checked other exceptions injected in 
svm_vmexit_do_vmsave and svm_vm_exit_do_vmload, and the following should 
probably also changed to #GP as well.
    if (!nestedsvm_vmcb_map(v, vmcbaddr)) {
        gdprintk(XENLOG_ERR, "VMSAVE: mapping vmcb failed, injecting 
#UD\n");
        ret = TRAP_invalid_op;
        goto inject;
    }

> > +        goto inject;
> > +    }
> > +
> > +    svm_vmload_pa(page_to_mfn(page) << PAGE_SHIFT);
> Please use page_to_maddr() for this.
Ah... That's what I needed.  Thanks.

Suravee


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel