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

[Suravee Suthikulanit <suravee.suthikulpanit@amd.com>]

On 7/29/2013 5:43 AM, Tim Deegan wrote:
> Hi,
>
> At 16:46 -0500 on 26 Jul (1374857167), 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>
> 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