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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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


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