On 6/17/2016 11:38 AM, Jan Beulich wrote:
On 17.06.16 at 10:25, <cz...@bitdefender.com> wrote:
On 6/16/2016 6:16 PM, Jan Beulich wrote:
And looking at all the uses of this variable I get the impression that
you really want a shorthand for &d->arch.monitor (if any such
helper variable is worthwhile to have here in the first place).
Well, this was a simple cut-paste operation, not very old content aware :)
Personally I prefer the current shorthand (ad) (seems more intuitive and
is consistent with the other XEN_DOMCTL_MONITOR_EVENT_* cases), but if
you prefer I'll change that shorthand to am = &d->arch.monitor?
I'd prefer either no shorthand, or one eliminating the longest common
prefix across all uses.
am = &d->arch.monitor it is then.
--- a/xen/include/asm-x86/monitor.h
+++ b/xen/include/asm-x86/monitor.h
@@ -24,8 +24,6 @@
#include <xen/sched.h>
-#define monitor_ctrlreg_bitmask(ctrlreg_index) (1U << (ctrlreg_index))
-
static inline
int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op
*mop)
{
--- a/xen/include/xen/monitor.h
+++ b/xen/include/xen/monitor.h
@@ -25,6 +25,10 @@
struct domain;
struct xen_domctl_monitor_op;
+#if CONFIG_X86
+#define monitor_ctrlreg_bitmask(ctrlreg_index) (1U << (ctrlreg_index))
+#endif
What's the point in removing this from the x86 header if then it
needs to be put in such a conditional? If the conditional gets
dropped in the next patch, then I think you have two options:
Leave it where it is here, and move it there. Or move it here,
but omit the conditional right away - I can't see this definition
being present to harm the ARM build in any way.
Meld comment above.
I don't follow - having split the patches for reviewability was
a good idea. I merely commented on some oddities resulting
from that split, which - afaict - could be easily corrected without
folding the patches together.
Jan
Ooh, haha, sorry I've re-read your comment now. You're right, there's no
point in that change, I'll leave it on the X86 side until the ARM part
is actually implemented (last patch).
Corneliu.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel