[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.