Mukesh Rathor
2013-Jan-12 01:51 UTC
[RFC PATCH 6/16]: PVH xen: Define pvh guest and header changes..
In this patch, we define pvh guest in struct domain{}. That was found
to be easiest, I tried with just using PV type, and then just hvm type,
but too many if statements to see if it was running in HVM container.
adding is_pvh seems easiest.
There are some other header changes for use by PVH code later.
Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
diff -r fadb3f2f5d97 -r 5af39353f3f9 xen/include/asm-x86/desc.h
--- a/xen/include/asm-x86/desc.h Fri Jan 11 16:25:27 2013 -0800
+++ b/xen/include/asm-x86/desc.h Fri Jan 11 16:27:46 2013 -0800
@@ -38,7 +38,8 @@
#ifndef __ASSEMBLY__
-#define GUEST_KERNEL_RPL(d) (is_pv_32bit_domain(d) ? 1 : 3)
+#define GUEST_KERNEL_RPL(d) (is_pvh_domain(d) ? 0 : \
+ is_pv_32bit_domain(d) ? 1 : 3)
/* Fix up the RPL of a guest segment selector. */
#define __fixup_guest_selector(d, sel) \
diff -r fadb3f2f5d97 -r 5af39353f3f9 xen/include/asm-x86/domain.h
--- a/xen/include/asm-x86/domain.h Fri Jan 11 16:25:27 2013 -0800
+++ b/xen/include/asm-x86/domain.h Fri Jan 11 16:27:46 2013 -0800
@@ -16,7 +16,7 @@
#define is_pv_32on64_domain(d) (is_pv_32bit_domain(d))
#define is_pv_32on64_vcpu(v) (is_pv_32on64_domain((v)->domain))
-#define is_hvm_pv_evtchn_domain(d) (is_hvm_domain(d) && \
+#define is_hvm_pv_evtchn_domain(d) (is_hvm_or_pvh_domain(d) && \
d->arch.hvm_domain.irq.callback_via_type == HVMIRQ_callback_vector)
#define is_hvm_pv_evtchn_vcpu(v) (is_hvm_pv_evtchn_domain(v->domain))
@@ -252,10 +252,9 @@ struct arch_domain
struct list_head pdev_list;
- union {
- struct pv_domain pv_domain;
- struct hvm_domain hvm_domain;
- };
+ /* PVH : pvh uses fields from both pv and hvm, so separate the union */
+ struct pv_domain pv_domain;
+ struct hvm_domain hvm_domain;
struct paging_domain paging;
struct p2m_domain *p2m;
diff -r fadb3f2f5d97 -r 5af39353f3f9 xen/include/asm-x86/event.h
--- a/xen/include/asm-x86/event.h Fri Jan 11 16:25:27 2013 -0800
+++ b/xen/include/asm-x86/event.h Fri Jan 11 16:27:46 2013 -0800
@@ -18,7 +18,7 @@ int hvm_local_events_need_delivery(struc
static inline int local_events_need_delivery(void)
{
struct vcpu *v = current;
- return (is_hvm_vcpu(v) ? hvm_local_events_need_delivery(v) :
+ return (is_hvm_or_pvh_vcpu(v) ? hvm_local_events_need_delivery(v) :
(vcpu_info(v, evtchn_upcall_pending) &&
!vcpu_info(v, evtchn_upcall_mask)));
}
diff -r fadb3f2f5d97 -r 5af39353f3f9 xen/include/asm-x86/guest_access.h
--- a/xen/include/asm-x86/guest_access.h Fri Jan 11 16:25:27 2013 -0800
+++ b/xen/include/asm-x86/guest_access.h Fri Jan 11 16:27:46 2013 -0800
@@ -14,27 +14,27 @@
/* Raw access functions: no type checking. */
#define raw_copy_to_guest(dst, src, len) \
- (is_hvm_vcpu(current) ? \
+ (is_hvm_or_pvh_vcpu(current) ? \
copy_to_user_hvm((dst), (src), (len)) : \
copy_to_user((dst), (src), (len)))
#define raw_copy_from_guest(dst, src, len) \
- (is_hvm_vcpu(current) ? \
+ (is_hvm_or_pvh_vcpu(current) ? \
copy_from_user_hvm((dst), (src), (len)) : \
copy_from_user((dst), (src), (len)))
#define raw_clear_guest(dst, len) \
- (is_hvm_vcpu(current) ? \
+ (is_hvm_or_pvh_vcpu(current) ? \
clear_user_hvm((dst), (len)) : \
clear_user((dst), (len)))
#define __raw_copy_to_guest(dst, src, len) \
- (is_hvm_vcpu(current) ? \
+ (is_hvm_or_pvh_vcpu(current) ? \
copy_to_user_hvm((dst), (src), (len)) : \
__copy_to_user((dst), (src), (len)))
#define __raw_copy_from_guest(dst, src, len) \
- (is_hvm_vcpu(current) ? \
+ (is_hvm_or_pvh_vcpu(current) ? \
copy_from_user_hvm((dst), (src), (len)) : \
__copy_from_user((dst), (src), (len)))
#define __raw_clear_guest(dst, len) \
- (is_hvm_vcpu(current) ? \
+ (is_hvm_or_pvh_vcpu(current) ? \
clear_user_hvm((dst), (len)) : \
clear_user((dst), (len)))
diff -r fadb3f2f5d97 -r 5af39353f3f9 xen/include/asm-x86/system.h
--- a/xen/include/asm-x86/system.h Fri Jan 11 16:25:27 2013 -0800
+++ b/xen/include/asm-x86/system.h Fri Jan 11 16:27:46 2013 -0800
@@ -4,9 +4,15 @@
#include <xen/lib.h>
#include <asm/bitops.h>
+/* We need vcpu because during context switch, going from pure PV to PVH,
+ * in save_segments(), current has been updated to next, and no longer pointing
+ * to the pure PV. BTW, for PVH, we update regs->selectors on each vmexit */
#define read_segment_register(vcpu, regs, name) \
({ u16 __sel; \
- asm volatile ( "movw %%" STR(name) ",%0" :
"=r" (__sel) ); \
+ if (is_pvh_vcpu(v)) \
+ __sel = regs->name; \
+ else \
+ asm volatile ( "movw %%" STR(name) ",%0" :
"=r" (__sel) ); \
__sel; \
})
diff -r fadb3f2f5d97 -r 5af39353f3f9 xen/include/asm-x86/x86_64/regs.h
--- a/xen/include/asm-x86/x86_64/regs.h Fri Jan 11 16:25:27 2013 -0800
+++ b/xen/include/asm-x86/x86_64/regs.h Fri Jan 11 16:27:46 2013 -0800
@@ -11,9 +11,10 @@
#define ring_3(r) (((r)->cs & 3) == 3)
#define guest_kernel_mode(v, r) \
+ ( is_pvh_vcpu(v) ? ({BUG_ON(v!=current); ring_0(r);}) : \
(!is_pv_32bit_vcpu(v) ? \
(ring_3(r) && ((v)->arch.flags & TF_kernel_mode)) :
\
- (ring_1(r)))
+ (ring_1(r))) )
#define permit_softint(dpl, v, r) \
((dpl) >= (guest_kernel_mode(v, r) ? 1 : 3))
diff -r fadb3f2f5d97 -r 5af39353f3f9 xen/include/public/domctl.h
--- a/xen/include/public/domctl.h Fri Jan 11 16:25:27 2013 -0800
+++ b/xen/include/public/domctl.h Fri Jan 11 16:27:46 2013 -0800
@@ -89,6 +89,10 @@ struct xen_domctl_getdomaininfo {
/* Being debugged. */
#define _XEN_DOMINF_debugged 6
#define XEN_DOMINF_debugged (1U<<_XEN_DOMINF_debugged)
+ /* domain is PVH */
+#define _XEN_DOMINF_pvh_guest 7
+#define XEN_DOMINF_pvh_guest (1U<<_XEN_DOMINF_pvh_guest)
+
/* XEN_DOMINF_shutdown guest-supplied code. */
#define XEN_DOMINF_shutdownmask 255
#define XEN_DOMINF_shutdownshift 16
diff -r fadb3f2f5d97 -r 5af39353f3f9 xen/include/xen/lib.h
--- a/xen/include/xen/lib.h Fri Jan 11 16:25:27 2013 -0800
+++ b/xen/include/xen/lib.h Fri Jan 11 16:27:46 2013 -0800
@@ -45,6 +45,14 @@ do {
#define ASSERT(p) do { if ( 0 && (p) ); } while (0)
#endif
+/* While PVH feature is experimental, make it an unconditional assert */
+#define PVH_ASSERT(p) \
+ do { if ( unlikely(!(p)) ) assert_failed(#p); } while (0)
+#define NO_PVH_ASSERT_VCPU(v) \
+ do { if (is_pvh_vcpu(v)) PVH_ASSERT(0); } while (0)
+#define NO_PVH_ASSERT_DOMAIN(d) \
+ do { if (is_pvh_domain(d)) PVH_ASSERT(0); } while (0)
+
#define ABS(_x) ({ \
typeof(_x) __x = (_x); \
(__x < 0) ? -__x : __x; \
diff -r fadb3f2f5d97 -r 5af39353f3f9 xen/include/xen/sched.h
--- a/xen/include/xen/sched.h Fri Jan 11 16:25:27 2013 -0800
+++ b/xen/include/xen/sched.h Fri Jan 11 16:27:46 2013 -0800
@@ -230,6 +230,9 @@ struct mem_event_per_domain
struct mem_event_domain access;
};
+/* PVH is a PV guest running in HVM container. While is_hvm is false for it,
+ * it uses many of the HVM data structs.
+ */
struct domain
{
domid_t domain_id;
@@ -278,6 +281,7 @@ struct domain
/* Is this an HVM guest? */
bool_t is_hvm;
+ bool_t is_pvh; /* see above for description */
#ifdef HAS_PASSTHROUGH
/* Does this guest need iommu mappings? */
bool_t need_iommu;
@@ -449,6 +453,10 @@ struct domain *domain_create(
/* DOMCRF_oos_off: dont use out-of-sync optimization for shadow page tables */
#define _DOMCRF_oos_off 4
#define DOMCRF_oos_off (1U<<_DOMCRF_oos_off)
+ /* DOMCRF_pvh: Create PV domain in HVM container */
+#define _DOMCRF_pvh 5
+#define DOMCRF_pvh (1U<<_DOMCRF_pvh)
+
/*
* rcu_lock_domain_by_id() is more efficient than get_domain_by_id().
@@ -713,8 +721,12 @@ void watchdog_domain_destroy(struct doma
#define is_hvm_domain(d) ((d)->is_hvm)
#define is_hvm_vcpu(v) (is_hvm_domain(v->domain))
+#define is_pvh_domain(d) ((d)->is_pvh)
+#define is_pvh_vcpu(v) (is_pvh_domain(v->domain))
#define is_pinned_vcpu(v) ((v)->domain->is_pinned || \
cpumask_weight((v)->cpu_affinity) == 1)
+#define is_hvm_or_pvh_domain(d) (is_hvm_domain(d) || is_pvh_domain(d))
+#define is_hvm_or_pvh_vcpu(v) (is_hvm_or_pvh_domain(v->domain))
#ifdef HAS_PASSTHROUGH
#define need_iommu(d) ((d)->need_iommu)
#else
Jan Beulich
2013-Jan-14 11:38 UTC
Re: [RFC PATCH 6/16]: PVH xen: Define pvh guest and header changes..
>>> On 12.01.13 at 02:51, Mukesh Rathor <mukesh.rathor@oracle.com> wrote: > +/* We need vcpu because during context switch, going from pure PV to PVH, > + * in save_segments(), current has been updated to next, and no longer pointing > + * to the pure PV. BTW, for PVH, we update regs->selectors on each vmexit */ > #define read_segment_register(vcpu, regs, name) \I can only hope that at the end of this patch set the comment matches reality - at this point in the series it doesn''t afaict.> --- a/xen/include/asm-x86/x86_64/regs.h Fri Jan 11 16:25:27 2013 -0800 > +++ b/xen/include/asm-x86/x86_64/regs.h Fri Jan 11 16:27:46 2013 -0800 > @@ -11,9 +11,10 @@ > #define ring_3(r) (((r)->cs & 3) == 3) > > #define guest_kernel_mode(v, r) \ > + ( is_pvh_vcpu(v) ? ({BUG_ON(v!=current); ring_0(r);}) : \If this BUG_ON() really has to stay here, you ought to add white space inside the braces and around the !=.> (!is_pv_32bit_vcpu(v) ? \ > (ring_3(r) && ((v)->arch.flags & TF_kernel_mode)) : \ > - (ring_1(r))) > + (ring_1(r))) )As you add a level of parentheses, you also ought to adjust indentation.> --- a/xen/include/xen/lib.h Fri Jan 11 16:25:27 2013 -0800 > +++ b/xen/include/xen/lib.h Fri Jan 11 16:27:46 2013 -0800 > @@ -45,6 +45,14 @@ do { > #define ASSERT(p) do { if ( 0 && (p) ); } while (0) > #endif > > +/* While PVH feature is experimental, make it an unconditional assert */ > +#define PVH_ASSERT(p) \ > + do { if ( unlikely(!(p)) ) assert_failed(#p); } while (0) > +#define NO_PVH_ASSERT_VCPU(v) \ > + do { if (is_pvh_vcpu(v)) PVH_ASSERT(0); } while (0) > +#define NO_PVH_ASSERT_DOMAIN(d) \ > + do { if (is_pvh_domain(d)) PVH_ASSERT(0); } while (0)What''s this? At the very least, you want e.g. + do { PVH_ASSERT(!is_pvh_vcpu(v)); } while (0) for the printed string to be meaningful (and then you can also drop the do/while). But the defines, if needed at all, are grossly misplaced in any case; there ought to be a pvh header for such stuff.> @@ -278,6 +281,7 @@ struct domain > > /* Is this an HVM guest? */ > bool_t is_hvm; > + bool_t is_pvh; /* see above for description */These are mutually exclusive (also with PV), so perhaps better to have a single enum-type variable? Jan> #ifdef HAS_PASSTHROUGH > /* Does this guest need iommu mappings? */ > bool_t need_iommu;
Mukesh Rathor
2013-Jan-16 00:37 UTC
Re: [RFC PATCH 6/16]: PVH xen: Define pvh guest and header changes..
On Mon, 14 Jan 2013 11:38:41 +0000 "Jan Beulich" <JBeulich@suse.com> wrote:> >>> On 12.01.13 at 02:51, Mukesh Rathor <mukesh.rathor@oracle.com> > >>> wrote: > > +/* We need vcpu because during context switch, going from pure PV > > to PVH, > > + * in save_segments(), current has been updated to next, and no > > longer pointing > > + * to the pure PV. BTW, for PVH, we update regs->selectors on each > > vmexit */ #define read_segment_register(vcpu, regs, > > name) \ > > I can only hope that at the end of this patch set the comment > matches reality - at this point in the series it doesn''t afaict.It''s a big patch, tough to break to have things together this way. Each has to be compilable. It may help to apply all patches to the xen tree (c/s: 26124) and then cscope it? Just a thought. I realize it''s tough to review, but not sure how else I can break it and still keep all parts small.> > --- a/xen/include/asm-x86/x86_64/regs.h Fri Jan 11 16:25:27 > > 2013 -0800 +++ b/xen/include/asm-x86/x86_64/regs.h Fri Jan > > 11 16:27:46 2013 -0800 @@ -11,9 +11,10 @@ > > #define ring_3(r) (((r)->cs & 3) == 3) > > > > If this BUG_ON() really has to stay here, you ought to add > white space inside the braces and around the !=.Ok, done.> As you add a level of parentheses, you also ought to adjust > indentation.It''s already indented single space like the previous macro was. Do you want me to 4 space it?> At the very least, you want e.g. > > + do { PVH_ASSERT(!is_pvh_vcpu(v)); } while (0)Duh! fixed.> But the defines, if needed at all, are grossly misplaced in any case; > there ought to be a pvh header for such stuff.Well, I imagine those asserts while PVH is still being stabilized, and then removed. Do you still want me to create a new header with just 3 defines that will be deleted in near future?> > @@ -278,6 +281,7 @@ struct domain > > > > /* Is this an HVM guest? */ > > bool_t is_hvm; > > + bool_t is_pvh; /* see above for description */ > > These are mutually exclusive (also with PV), so perhaps better > to have a single enum-type variable?I imagine in future there would be no PV, so is_hvm==0 ==> pvh. Too optimistic? thanks, Mukesh
Jan Beulich
2013-Jan-16 09:53 UTC
Re: [RFC PATCH 6/16]: PVH xen: Define pvh guest and header changes..
>>> On 16.01.13 at 01:37, Mukesh Rathor <mukesh.rathor@oracle.com> wrote: > On Mon, 14 Jan 2013 11:38:41 +0000 > "Jan Beulich" <JBeulich@suse.com> wrote: > >> >>> On 12.01.13 at 02:51, Mukesh Rathor <mukesh.rathor@oracle.com> >> >>> wrote: >> > +/* We need vcpu because during context switch, going from pure PV >> > to PVH, >> > + * in save_segments(), current has been updated to next, and no >> > longer pointing >> > + * to the pure PV. BTW, for PVH, we update regs->selectors on each >> > vmexit */ #define read_segment_register(vcpu, regs, >> > name) \ >> >> I can only hope that at the end of this patch set the comment >> matches reality - at this point in the series it doesn''t afaict. > > It''s a big patch, tough to break to have things together this way. > Each has to be compilable. It may help to apply all patches to the > xen tree (c/s: 26124) and then cscope it? Just a thought. I realize > it''s tough to review, but not sure how else I can break it and still > keep all parts small.I understand that this is difficult to break up. But I don''t see why comments can''t be added when they (or adjusted multiple times to) match reality.>> As you add a level of parentheses, you also ought to adjust >> indentation. > > It''s already indented single space like the previous macro was. Do you > want me to 4 space it?No - I want you to add a single space of indentation for each added level of opening parentheses that don''t have seeing their matching closing ones yet.>> But the defines, if needed at all, are grossly misplaced in any case; >> there ought to be a pvh header for such stuff. > > Well, I imagine those asserts while PVH is still being stabilized, > and then removed. Do you still want me to create a new header with just > 3 defines that will be deleted in near future?Are you telling me that there''s nothing else pvh-specific that needs declaring/defining? And even if so, xen/lib.h clearly is the wrong place - I''m not even convinced PVH is a cross- architecture concept...>> > @@ -278,6 +281,7 @@ struct domain >> > >> > /* Is this an HVM guest? */ >> > bool_t is_hvm; >> > + bool_t is_pvh; /* see above for description */ >> >> These are mutually exclusive (also with PV), so perhaps better >> to have a single enum-type variable? > > I imagine in future there would be no PV, so is_hvm==0 ==> pvh. > Too optimistic?Why would you want to close the road to run old guests? Jan
Tim Deegan
2013-Jan-24 15:34 UTC
Re: [RFC PATCH 6/16]: PVH xen: Define pvh guest and header changes..
At 17:51 -0800 on 11 Jan (1357926698), Mukesh Rathor wrote:> diff -r fadb3f2f5d97 -r 5af39353f3f9 xen/include/asm-x86/guest_access.h > --- a/xen/include/asm-x86/guest_access.h Fri Jan 11 16:25:27 2013 -0800 > +++ b/xen/include/asm-x86/guest_access.h Fri Jan 11 16:27:46 2013 -0800 > @@ -14,27 +14,27 @@ > > /* Raw access functions: no type checking. */ > #define raw_copy_to_guest(dst, src, len) \ > - (is_hvm_vcpu(current) ? \ > + (is_hvm_or_pvh_vcpu(current) ? \ > copy_to_user_hvm((dst), (src), (len)) : \ > copy_to_user((dst), (src), (len))) > #define raw_copy_from_guest(dst, src, len) \ > - (is_hvm_vcpu(current) ? \ > + (is_hvm_or_pvh_vcpu(current) ? \ > copy_from_user_hvm((dst), (src), (len)) : \ > copy_from_user((dst), (src), (len))) > #define raw_clear_guest(dst, len) \ > - (is_hvm_vcpu(current) ? \ > + (is_hvm_or_pvh_vcpu(current) ? \ > clear_user_hvm((dst), (len)) : \ > clear_user((dst), (len))) > #define __raw_copy_to_guest(dst, src, len) \ > - (is_hvm_vcpu(current) ? \ > + (is_hvm_or_pvh_vcpu(current) ? \ > copy_to_user_hvm((dst), (src), (len)) : \ > __copy_to_user((dst), (src), (len))) > #define __raw_copy_from_guest(dst, src, len) \ > - (is_hvm_vcpu(current) ? \ > + (is_hvm_or_pvh_vcpu(current) ? \ > copy_from_user_hvm((dst), (src), (len)) : \ > __copy_from_user((dst), (src), (len))) > #define __raw_clear_guest(dst, len) \ > - (is_hvm_vcpu(current) ? \ > + (is_hvm_or_pvh_vcpu(current) ? \ > clear_user_hvm((dst), (len)) : \ > clear_user((dst), (len)))Please don''t mess up these nicely aligned backslashes. :)> > diff -r fadb3f2f5d97 -r 5af39353f3f9 xen/include/asm-x86/system.h > --- a/xen/include/asm-x86/system.h Fri Jan 11 16:25:27 2013 -0800 > +++ b/xen/include/asm-x86/system.h Fri Jan 11 16:27:46 2013 -0800 > @@ -4,9 +4,15 @@ > #include <xen/lib.h> > #include <asm/bitops.h> > > +/* We need vcpu because during context switch, going from pure PV to PVH, > + * in save_segments(), current has been updated to next, and no longer pointing > + * to the pure PV. BTW, for PVH, we update regs->selectors on each vmexit */ > #define read_segment_register(vcpu, regs, name) \ > ({ u16 __sel; \ > - asm volatile ( "movw %%" STR(name) ",%0" : "=r" (__sel) ); \ > + if (is_pvh_vcpu(v)) \ > + __sel = regs->name; \ > + else \ > + asm volatile ( "movw %%" STR(name) ",%0" : "=r" (__sel) ); \Or these.> __sel; \ > }) > > diff -r fadb3f2f5d97 -r 5af39353f3f9 xen/include/asm-x86/x86_64/regs.h > --- a/xen/include/asm-x86/x86_64/regs.h Fri Jan 11 16:25:27 2013 -0800 > +++ b/xen/include/asm-x86/x86_64/regs.h Fri Jan 11 16:27:46 2013 -0800 > @@ -11,9 +11,10 @@ > #define ring_3(r) (((r)->cs & 3) == 3) > > #define guest_kernel_mode(v, r) \ > + ( is_pvh_vcpu(v) ? ({BUG_ON(v!=current); ring_0(r);}) : \ > (!is_pv_32bit_vcpu(v) ? \ > (ring_3(r) && ((v)->arch.flags & TF_kernel_mode)) : \ > - (ring_1(r))) > + (ring_1(r))) )Guess what I''m going to say here. :) Tim.
Mukesh Rathor
2013-Jan-25 01:56 UTC
Re: [RFC PATCH 6/16]: PVH xen: Define pvh guest and header changes..
On Thu, 24 Jan 2013 15:34:31 +0000 Tim Deegan <tim@xen.org> wrote:> At 17:51 -0800 on 11 Jan (1357926698), Mukesh Rathor wrote: > > diff -r fadb3f2f5d97 -r 5af39353f3f9 > > xen/include/asm-x86/guest_access.h --- > > a/xen/include/asm-x86/guest_access.h Fri Jan 11 16:25:27 > > 2013 -0800 +++ b/xen/include/asm-x86/guest_access.h Fri Jan > > 11 16:27:46 2013 -0800 @@ -14,27 +14,27 @@ > > /* Raw access functions: no type checking. */ > > #define raw_copy_to_guest(dst, src, len) \ > > + (is_hvm_or_pvh_vcpu(current) ? \ > > copy_to_user_hvm((dst), (src), (len)) : \ > > __copy_to_user((dst), (src), (len))) > > #define __raw_copy_from_guest(dst, src, len) \ > > - (is_hvm_vcpu(current) ? \ > > + (is_hvm_or_pvh_vcpu(current) ? \ > > copy_from_user_hvm((dst), (src), (len)) : \ > > __copy_from_user((dst), (src), (len))) > > #define __raw_clear_guest(dst, len) \ > > - (is_hvm_vcpu(current) ? \ > > + (is_hvm_or_pvh_vcpu(current) ? \ > > clear_user_hvm((dst), (len)) : \ > > clear_user((dst), (len))) > > Please don''t mess up these nicely aligned backslashes. :)Ooopsy!! My bad. Fixed.
Mukesh Rathor
2013-Jan-31 01:21 UTC
Re: [RFC PATCH 6/16]: PVH xen: Define pvh guest and header changes..
On Wed, 16 Jan 2013 09:53:11 +0000 "Jan Beulich" <JBeulich@suse.com> wrote:> >>> On 16.01.13 at 01:37, Mukesh Rathor <mukesh.rathor@oracle.com> > >>> wrote: > > On Mon, 14 Jan 2013 11:38:41 +0000 > > "Jan Beulich" <JBeulich@suse.com> wrote: > > > > >> As you add a level of parentheses, you also ought to adjust > >> indentation. > > > > It''s already indented single space like the previous macro was. Do > > you want me to 4 space it? > > No - I want you to add a single space of indentation for each > added level of opening parentheses that don''t have seeing > their matching closing ones yet.Done.> >> But the defines, if needed at all, are grossly misplaced in any > >> case; there ought to be a pvh header for such stuff. > > > > Well, I imagine those asserts while PVH is still being stabilized, > > and then removed. Do you still want me to create a new header with > > just 3 defines that will be deleted in near future? > > Are you telling me that there''s nothing else pvh-specific that > needs declaring/defining? And even if so, xen/lib.h clearly is > the wrong place - I''m not even convinced PVH is a cross- > architecture concept... > > >> > @@ -278,6 +281,7 @@ struct domain > >> > > >> > /* Is this an HVM guest? */ > >> > bool_t is_hvm; > >> > + bool_t is_pvh; /* see above for description > >> > */ > >> > >> These are mutually exclusive (also with PV), so perhaps better > >> to have a single enum-type variable?Ok, made it enum. I already know you are not going to like my naming ;)...