[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [V10 PATCH 10/23] PVH xen: domain create, context switch related code changes
On 08/08/13 08:29, Jan Beulich wrote:
On 08.08.13 at 03:40, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> wrote:
On Wed, 7 Aug 2013 11:24:42 +0100
George Dunlap <George.Dunlap@xxxxxxxxxxxxx> wrote:
.........
index 412971e..ece11e4 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4334,6 +4334,9 @@ void destroy_gdt(struct vcpu *v)
int i;
unsigned long pfn;
+ if ( is_pvh_vcpu(v) )
+ return;
There seems to be some inconsistency with where this is supposed to be
checked -- in domain_relinquish_resources(), destroy_gdt() is only
called for pv domains (gated on is_pv_domain); but in
arch_set_info_guest(), it *was* gated on being PV, but with the PVH
changes it's still being called.
Either this should only be called for PV domains (and this check
should be an ASSERT), or it should be called regardless of the type of
domain. I prefer the first if possible.
In the original version it was being called for pv domains only, and I
had checks in the caller. But, Jan preferred the check in destroy_gdt()
so I moved it to destroy_gdt().
But that perspective may have changed with other code changes:
If all callers now suppress the call for PVH guests, this should indeed
be an assertion (if anything). If all but one caller checks for PV (or
are in PV only code paths), the better approach now may still be to
have the one odd caller do the check and have an assertion in the
function. Iirc it was at least two call sites you had to adjust originally,
which then warranted to do the check in just one place (in the
function itself).
Overall I think checking before calling would make the code easier to
understand. All the functions which call this have is_foo_domain()
sprinkled all over anyway, and so it's easier for someone reading the
code to understand immediately that HVM and PVH guests don't need their
gdt destroyed. But my main point was that if we check inside the
function, we should avoid checking outside the function for consistency.
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
