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

[Tim Deegan <tim@xxxxxxx>]

At 09:01 +0100 on 12 Jul (1373619676), Jan Beulich wrote:
> >>> On 11.07.13 at 19:34, <suravee.suthikulpanit@xxxxxxx> wrote:
> > --- a/xen/arch/x86/mm/hap/nested_hap.c
> > +++ b/xen/arch/x86/mm/hap/nested_hap.c
> > @@ -191,6 +191,19 @@ out:
> >      return rc;
> >  }
> >  
> > +int
> > +nestedhvm_walk_L0_p2m(struct vcpu *v, paddr_t L1_gpa, paddr_t *L0_gpa)
> > +{
> > +    p2m_type_t p2mt_10;
> > +    unsigned int page_order_10;
> > +    p2m_access_t p2ma_10 = p2m_access_rwx;
> 
> Pointless initializer?

Indeed, p2ma_10 could be left alone.

> > +
> > +    return nestedhap_walk_L0_p2m ( p2m_get_hostp2m(v->domain),
> 
> Extra spaces around "(".
> 
> > +                                 L1_gpa, L0_gpa,
> > +                                 &p2mt_10, &p2ma_10, &page_order_10,
> > +                                 0, 0, 0);
> 
> Wouldn't the caller's use of the GPA imply that you want read and
> write access here?

Yes, at least for the callers you've added.

> I'm not clear about the need for this new wrapper: Is it really
> benign to the caller what type, access, and order get returned
> here? Is it really too much of a burden to have the two call
> sites do the call here directly? The more that (see above) you'd
> really need to give the caller control over the access requested?

Yeah, I'm not sure the wrapper is needed.  Can the callers just use
get_page_from_gfn() to do the translation from guest-MFN -- i.e. will we
always be in non-nested mode when we're emulating VMLOAD/VMSAVE?

Cheers,

Tim.

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