[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH RFC v12 05/21] Introduce pv guest type and has_hvm_container macros
On 20/09/13 09:11, Jan Beulich wrote:
On 19.09.13 at 18:27, George Dunlap <george.dunlap@xxxxxxxxxxxxx> wrote:
On 18/09/13 14:46, Jan Beulich wrote:
On 13.09.13 at 18:25, George Dunlap <george.dunlap@xxxxxxxxxxxxx> wrote:
@@ -72,7 +72,7 @@ void *map_domain_page(unsigned long mfn)
#endif
v = mapcache_current_vcpu();
- if ( !v || is_hvm_vcpu(v) )
+ if ( !v || has_hvm_container_vcpu(v) )
return mfn_to_virt(mfn);
dcache = &v->domain->arch.pv_domain.mapcache;
@@ -177,7 +177,7 @@ void unmap_domain_page(const void *ptr)
ASSERT(va >= MAPCACHE_VIRT_START && va < MAPCACHE_VIRT_END);
v = mapcache_current_vcpu();
- ASSERT(v && !is_hvm_vcpu(v));
+ ASSERT(v && is_pv_vcpu(v));
This is inconsistent with the above change to map_domain_page()
and the changes later in this file.
You mean, making this "is_pv_vcpu" instead of
"!has_hvm_container_vcpu"? I guess that's true.
Actually I meant the cited one above to be converted - the map
cache, following your argumentation further down, is something
that is _not_ needed when there is a HVM container (i.e. external
paging); the need for it is not tied to the PV-ness of a guest.
But anyway, this is-PV and doesn't-have-HVM-container distinction
is sort of fuzzy anyway, and you seem to half way agree following
later parts of your response.
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -119,6 +119,7 @@ static void show_guest_stack(struct vcpu *v, struct
cpu_user_regs *regs)
unsigned long *stack, addr;
unsigned long mask = STACK_SIZE;
+ /* Avoid HVM as we don't know what the stack looks like */
if ( is_hvm_vcpu(v) )
So why do you not change this one to !is_pv_vcpu()? In particular
the do_page_walk() further down is inherently PV.
I guess it depends. I was thinking that the reason we didn't do this
for HVM guests was that there was no guarantee what the stack looked
like -- it looks like another aspect is that for VMs that are not
current, whether the PTs are controlled by Xen or the guest is a factor.
But if we make do_page_walk gated on is_pv_vcpu(), then it seems like it
should work when v==current, and fail gracefully when v!=current.
No, the right (long term) thing would in fact be to do the right
form of page walk in the PVH case here too. After all you're right
that the original reason for filtering out HVM guests was that we
can't make assumptions about their stacks (whether making any
such assumptions of PV guests is really appropriate is another
question). Hence I wouldn't want to see do_page_walk() to be
fiddled with; suppressing the call in show_guest_stack() would be
fine with me.
What I meant was, do_page_walk() already had a conditional at the top to
return NULL if is_hvm_domain() was true, and callers should all be
written to handle that case. If we change that to if(!is_pv_domain())
(which should probably be done anyway), then for PVH domains we'll get a
stack for v==current, but not for others. do_page_walk() can be extended
for PVH domains at some point in the future.
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
