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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.08.13 at 10:31, <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>

Tim - have all your earlier comments been addressed in this version?

> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -1779,15 +1779,15 @@ static void
>  svm_vmexit_do_vmrun(struct cpu_user_regs *regs,
>                      struct vcpu *v, uint64_t vmcbaddr)
>  {
> -    if (!nestedhvm_enabled(v->domain)) {
> +    if ( !nestedhvm_enabled(v->domain) || !hvm_svm_enabled(v) ) {

Suravee, why is this change needed (here and further down)?
Can we really get here when hvm_svm_enabled(v) returns false?
I don't recall this having been there in earlier versions.

Also, if the change _is_ needed, just like done further down you
should have taken the opportunity and fix the placement of the
closing brace (also again later in this function).

> +static struct page_info *
> +nsvm_get_nvmcb_page(struct vcpu *v, uint64_t vmcbaddr)
> +{
> +    p2m_type_t p2mt;
> +    struct page_info *page;
> +    struct nestedvcpu *nv = &vcpu_nestedhvm(v);
> +
> +    if (!nestedsvm_vmcb_map(v, vmcbaddr))

Coding style.

> +     return NULL;

Hard tab.

Jan


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