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

[Suravee Suthikulanit <suravee.suthikulpanit@xxxxxxx>]

On 7/5/2013 2:53 AM, Jan Beulich wrote:
> > > > On 04.07.13 at 23:48, Tim Deegan <tim@xxxxxxx> wrote:
> > At 20:42 +0100 on 04 Jul (1372970576), Andrew Cooper wrote:
> > > On 04/07/13 20:36, suravee.suthikulpanit@xxxxxxx wrote:
> > > > +static inline void nestedsvm_vmload(uint64_t vmcb)
> > > unsigned long if this is actually an address.
> > IIUC this is a physical address, so paddr_t is the correct type.  Also,
> > it might be nicer to call these svm_vm{save,load}_by_paddr() or similar
> > to make it clear what they do.
> So would I think. And the existing functions then could simply
> wrap the new ones.
>
> However, looking at the call sites of svm_vmexit_do_vm(), I don't
> think this is a host physical address in all cases: At least the uses
> from svm_vmexit_do_vm*() in svm.c suggest that these are GPAs,
> and hence can't be passed to vmload/vmsave without translation.
>
> > > But more importantly, if virt_to_maddr() fails an assertion because the
> > > virtual address is not a persistent mapping, what is going to happen
> > > when the virtual mapping (potentially) changes while the vvmcx is in use?
> > I think the virtual mapping is ok from that point of view -- it's mapped
> > with map_domain_page_global().
> And anyway, the virtual mapping isn't being used in the resulting
> code.
>
> > I worry that we might run out of mapping
> > slots if we keep a lot of these permanent mappings around, though.
> Afaict there's a single such mapping per vCPU, so not that much to
> worry about I think.
>
> Jan
Thank you all for comments. I am sending out V2 in a separate thread.

Suravee


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