* [PATCH V2] xen: event channel arrays are xen_ulong_t and not unsigned long
@ 2013-02-19 14:48 Ian Campbell
2013-02-19 14:49 ` [PATCH LINUX] " Ian Campbell
` (4 more replies)
0 siblings, 5 replies; 36+ messages in thread
From: Ian Campbell @ 2013-02-19 14:48 UTC (permalink / raw)
To: linux-arm-kernel
The following two patches (one for Xen, one for Linux) switch Xen event
channels from unsigned long to xen_ulong_t.
On arm, this enables us to have the same interface on arm32 and arm64
because xen_ulong_t is a 64-bit type on ARM.
On x86 there is no change because xen_ulong_t is unsigned long on x86.
Since this is an ABI change on ARM I hope we can get this in for the 3.9
merge window. I have deferred the other changes which I previously
posted along side this since they are not so critical.
Ian.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH LINUX] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-02-19 14:48 [PATCH V2] xen: event channel arrays are xen_ulong_t and not unsigned long Ian Campbell
@ 2013-02-19 14:49 ` Ian Campbell
2013-02-19 17:11 ` Stefano Stabellini
2013-02-19 14:49 ` [PATCH XEN] " Ian Campbell
` (3 subsequent siblings)
4 siblings, 1 reply; 36+ messages in thread
From: Ian Campbell @ 2013-02-19 14:49 UTC (permalink / raw)
To: linux-arm-kernel
On ARM we want these to be the same size on 32- and 64-bit.
This is an ABI change on ARM. X86 does not change.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Jan Beulich <JBeulich@suse.com>
Cc: Keir (Xen.org) <keir@xen.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: linux-arm-kernel at lists.infradead.org
Cc: xen-devel at lists.xen.org
---
Changes since V1
use find_first_set not __ffs
fix some more unsigned long -> xen_ulong_t
use more generic xchg_xen_ulong instead of ...read_evtchn...
---
arch/arm/include/asm/xen/events.h | 22 ++++++++
arch/x86/include/asm/xen/events.h | 3 +
drivers/xen/events.c | 103 ++++++++++++++++++++-----------------
include/xen/interface/xen.h | 8 ++--
4 files changed, 84 insertions(+), 52 deletions(-)
diff --git a/arch/arm/include/asm/xen/events.h b/arch/arm/include/asm/xen/events.h
index 94b4e90..9bb5f50 100644
--- a/arch/arm/include/asm/xen/events.h
+++ b/arch/arm/include/asm/xen/events.h
@@ -15,4 +15,26 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
return raw_irqs_disabled_flags(regs->ARM_cpsr);
}
+/*
+ * We cannot use xchg because it does not support 8-byte
+ * values. However it is safe to use {ldr,dtd}exd directly because all
+ * platforms which Xen can run on support those instructions.
+ */
+static inline xen_ulong_t xchg_xen_ulong(xen_ulong_t *ptr, xen_ulong_t val)
+{
+ xen_ulong_t oldval;
+ unsigned int tmp;
+
+ wmb();
+ asm volatile("@ read_evtchn_pending_sel\n"
+ "1: ldrexd %0, %H0, [%3]\n"
+ " strexd %1, %2, %H2, [%3]\n"
+ " teq %1, #0\n"
+ " bne 1b"
+ : "=&r" (oldval), "=&r" (tmp)
+ : "r" (val), "r" (ptr)
+ : "memory", "cc");
+ return oldval;
+}
+
#endif /* _ASM_ARM_XEN_EVENTS_H */
diff --git a/arch/x86/include/asm/xen/events.h b/arch/x86/include/asm/xen/events.h
index cc146d5..ca842f2 100644
--- a/arch/x86/include/asm/xen/events.h
+++ b/arch/x86/include/asm/xen/events.h
@@ -16,4 +16,7 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
return raw_irqs_disabled_flags(regs->flags);
}
+/* No need for a barrier -- XCHG is a barrier on x86. */
+#define xchg_xen_ulong(ptr, val) xchg((ptr), (val))
+
#endif /* _ASM_X86_XEN_EVENTS_H */
diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 0be4df3..5618ca0 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -80,6 +80,12 @@ enum xen_irq_type {
};
/*
+ * Make a bitmask (i.e. unsigned long *) of a xen_ulong_t
+ * array. Primarily to avoid long lines (hence the terse name).
+ */
+#define BM(x) (unsigned long *)(x)
+
+/*
* Packed IRQ information:
* type - enum xen_irq_type
* event channel - irq->event channel mapping
@@ -120,7 +126,9 @@ static unsigned long *pirq_eoi_map;
#endif
static bool (*pirq_needs_eoi)(unsigned irq);
-static DEFINE_PER_CPU(unsigned long [NR_EVENT_CHANNELS/BITS_PER_LONG],
+#define BITS_PER_EVTCHN_WORD (sizeof(xen_ulong_t)*8)
+
+static DEFINE_PER_CPU(xen_ulong_t [NR_EVENT_CHANNELS/BITS_PER_EVTCHN_WORD],
cpu_evtchn_mask);
/* Xen will never allocate port zero for any purpose. */
@@ -294,9 +302,9 @@ static bool pirq_needs_eoi_flag(unsigned irq)
return info->u.pirq.flags & PIRQ_NEEDS_EOI;
}
-static inline unsigned long active_evtchns(unsigned int cpu,
- struct shared_info *sh,
- unsigned int idx)
+static inline xen_ulong_t active_evtchns(unsigned int cpu,
+ struct shared_info *sh,
+ unsigned int idx)
{
return sh->evtchn_pending[idx] &
per_cpu(cpu_evtchn_mask, cpu)[idx] &
@@ -312,8 +320,8 @@ static void bind_evtchn_to_cpu(unsigned int chn, unsigned int cpu)
cpumask_copy(irq_to_desc(irq)->irq_data.affinity, cpumask_of(cpu));
#endif
- clear_bit(chn, per_cpu(cpu_evtchn_mask, cpu_from_irq(irq)));
- set_bit(chn, per_cpu(cpu_evtchn_mask, cpu));
+ clear_bit(chn, BM(per_cpu(cpu_evtchn_mask, cpu_from_irq(irq))));
+ set_bit(chn, BM(per_cpu(cpu_evtchn_mask, cpu)));
info_for_irq(irq)->cpu = cpu;
}
@@ -339,19 +347,19 @@ static void init_evtchn_cpu_bindings(void)
static inline void clear_evtchn(int port)
{
struct shared_info *s = HYPERVISOR_shared_info;
- sync_clear_bit(port, &s->evtchn_pending[0]);
+ sync_clear_bit(port, BM(&s->evtchn_pending[0]));
}
static inline void set_evtchn(int port)
{
struct shared_info *s = HYPERVISOR_shared_info;
- sync_set_bit(port, &s->evtchn_pending[0]);
+ sync_set_bit(port, BM(&s->evtchn_pending[0]));
}
static inline int test_evtchn(int port)
{
struct shared_info *s = HYPERVISOR_shared_info;
- return sync_test_bit(port, &s->evtchn_pending[0]);
+ return sync_test_bit(port, BM(&s->evtchn_pending[0]));
}
@@ -375,7 +383,7 @@ EXPORT_SYMBOL_GPL(notify_remote_via_irq);
static void mask_evtchn(int port)
{
struct shared_info *s = HYPERVISOR_shared_info;
- sync_set_bit(port, &s->evtchn_mask[0]);
+ sync_set_bit(port, BM(&s->evtchn_mask[0]));
}
static void unmask_evtchn(int port)
@@ -389,7 +397,7 @@ static void unmask_evtchn(int port)
if (unlikely((cpu != cpu_from_evtchn(port))))
do_hypercall = 1;
else
- evtchn_pending = sync_test_bit(port, &s->evtchn_pending[0]);
+ evtchn_pending = sync_test_bit(port, BM(&s->evtchn_pending[0]));
if (unlikely(evtchn_pending && xen_hvm_domain()))
do_hypercall = 1;
@@ -403,7 +411,7 @@ static void unmask_evtchn(int port)
} else {
struct vcpu_info *vcpu_info = __this_cpu_read(xen_vcpu);
- sync_clear_bit(port, &s->evtchn_mask[0]);
+ sync_clear_bit(port, BM(&s->evtchn_mask[0]));
/*
* The following is basically the equivalent of
@@ -411,8 +419,8 @@ static void unmask_evtchn(int port)
* the interrupt edge' if the channel is masked.
*/
if (evtchn_pending &&
- !sync_test_and_set_bit(port / BITS_PER_LONG,
- &vcpu_info->evtchn_pending_sel))
+ !sync_test_and_set_bit(port / BITS_PER_EVTCHN_WORD,
+ BM(&vcpu_info->evtchn_pending_sel)))
vcpu_info->evtchn_upcall_pending = 1;
}
@@ -1189,7 +1197,7 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
{
struct shared_info *sh = HYPERVISOR_shared_info;
int cpu = smp_processor_id();
- unsigned long *cpu_evtchn = per_cpu(cpu_evtchn_mask, cpu);
+ xen_ulong_t *cpu_evtchn = per_cpu(cpu_evtchn_mask, cpu);
int i;
unsigned long flags;
static DEFINE_SPINLOCK(debug_lock);
@@ -1205,7 +1213,7 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
pending = (get_irq_regs() && i == cpu)
? xen_irqs_disabled(get_irq_regs())
: v->evtchn_upcall_mask;
- printk("%d: masked=%d pending=%d event_sel %0*lx\n ", i,
+ printk("%d: masked=%d pending=%d event_sel %0*"PRI_xen_ulong"\n ", i,
pending, v->evtchn_upcall_pending,
(int)(sizeof(v->evtchn_pending_sel)*2),
v->evtchn_pending_sel);
@@ -1214,49 +1222,52 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
printk("\npending:\n ");
for (i = ARRAY_SIZE(sh->evtchn_pending)-1; i >= 0; i--)
- printk("%0*lx%s", (int)sizeof(sh->evtchn_pending[0])*2,
+ printk("%0*"PRI_xen_ulong"%s",
+ (int)sizeof(sh->evtchn_pending[0])*2,
sh->evtchn_pending[i],
i % 8 == 0 ? "\n " : " ");
printk("\nglobal mask:\n ");
for (i = ARRAY_SIZE(sh->evtchn_mask)-1; i >= 0; i--)
- printk("%0*lx%s",
+ printk("%0*"PRI_xen_ulong"%s",
(int)(sizeof(sh->evtchn_mask[0])*2),
sh->evtchn_mask[i],
i % 8 == 0 ? "\n " : " ");
printk("\nglobally unmasked:\n ");
for (i = ARRAY_SIZE(sh->evtchn_mask)-1; i >= 0; i--)
- printk("%0*lx%s", (int)(sizeof(sh->evtchn_mask[0])*2),
+ printk("%0*"PRI_xen_ulong"%s",
+ (int)(sizeof(sh->evtchn_mask[0])*2),
sh->evtchn_pending[i] & ~sh->evtchn_mask[i],
i % 8 == 0 ? "\n " : " ");
printk("\nlocal cpu%d mask:\n ", cpu);
- for (i = (NR_EVENT_CHANNELS/BITS_PER_LONG)-1; i >= 0; i--)
- printk("%0*lx%s", (int)(sizeof(cpu_evtchn[0])*2),
+ for (i = (NR_EVENT_CHANNELS/BITS_PER_EVTCHN_WORD)-1; i >= 0; i--)
+ printk("%0*"PRI_xen_ulong"%s", (int)(sizeof(cpu_evtchn[0])*2),
cpu_evtchn[i],
i % 8 == 0 ? "\n " : " ");
printk("\nlocally unmasked:\n ");
for (i = ARRAY_SIZE(sh->evtchn_mask)-1; i >= 0; i--) {
- unsigned long pending = sh->evtchn_pending[i]
+ xen_ulong_t pending = sh->evtchn_pending[i]
& ~sh->evtchn_mask[i]
& cpu_evtchn[i];
- printk("%0*lx%s", (int)(sizeof(sh->evtchn_mask[0])*2),
+ printk("%0*"PRI_xen_ulong"%s",
+ (int)(sizeof(sh->evtchn_mask[0])*2),
pending, i % 8 == 0 ? "\n " : " ");
}
printk("\npending list:\n");
for (i = 0; i < NR_EVENT_CHANNELS; i++) {
- if (sync_test_bit(i, sh->evtchn_pending)) {
- int word_idx = i / BITS_PER_LONG;
+ if (sync_test_bit(i, BM(sh->evtchn_pending))) {
+ int word_idx = i / BITS_PER_EVTCHN_WORD;
printk(" %d: event %d -> irq %d%s%s%s\n",
cpu_from_evtchn(i), i,
evtchn_to_irq[i],
- sync_test_bit(word_idx, &v->evtchn_pending_sel)
+ sync_test_bit(word_idx, BM(&v->evtchn_pending_sel))
? "" : " l2-clear",
- !sync_test_bit(i, sh->evtchn_mask)
+ !sync_test_bit(i, BM(sh->evtchn_mask))
? "" : " globally-masked",
- sync_test_bit(i, cpu_evtchn)
+ sync_test_bit(i, BM(cpu_evtchn))
? "" : " locally-masked");
}
}
@@ -1273,7 +1284,7 @@ static DEFINE_PER_CPU(unsigned int, current_bit_idx);
/*
* Mask out the i least significant bits of w
*/
-#define MASK_LSBS(w, i) (w & ((~0UL) << i))
+#define MASK_LSBS(w, i) (w & ((~((xen_ulong_t)0UL)) << i))
/*
* Search the CPUs pending events bitmasks. For each one found, map
@@ -1295,18 +1306,14 @@ static void __xen_evtchn_do_upcall(void)
unsigned count;
do {
- unsigned long pending_words;
+ xen_ulong_t pending_words;
vcpu_info->evtchn_upcall_pending = 0;
if (__this_cpu_inc_return(xed_nesting_count) - 1)
goto out;
-#ifndef CONFIG_X86 /* No need for a barrier -- XCHG is a barrier on x86. */
- /* Clear master flag /before/ clearing selector flag. */
- wmb();
-#endif
- pending_words = xchg(&vcpu_info->evtchn_pending_sel, 0);
+ pending_words = xchg_xen_ulong(&vcpu_info->evtchn_pending_sel, 0);
start_word_idx = __this_cpu_read(current_word_idx);
start_bit_idx = __this_cpu_read(current_bit_idx);
@@ -1314,8 +1321,8 @@ static void __xen_evtchn_do_upcall(void)
word_idx = start_word_idx;
for (i = 0; pending_words != 0; i++) {
- unsigned long pending_bits;
- unsigned long words;
+ xen_ulong_t pending_bits;
+ xen_ulong_t words;
words = MASK_LSBS(pending_words, word_idx);
@@ -1327,7 +1334,7 @@ static void __xen_evtchn_do_upcall(void)
bit_idx = 0;
continue;
}
- word_idx = __ffs(words);
+ word_idx = find_first_bit(BM(&words), sizeof(words));
pending_bits = active_evtchns(cpu, s, word_idx);
bit_idx = 0; /* usually scan entire word from start */
@@ -1342,7 +1349,7 @@ static void __xen_evtchn_do_upcall(void)
}
do {
- unsigned long bits;
+ xen_ulong_t bits;
int port, irq;
struct irq_desc *desc;
@@ -1352,10 +1359,10 @@ static void __xen_evtchn_do_upcall(void)
if (bits == 0)
break;
- bit_idx = __ffs(bits);
+ bit_idx = find_first_bit(BM(&bits), sizeof(bits));
/* Process port. */
- port = (word_idx * BITS_PER_LONG) + bit_idx;
+ port = (word_idx * BITS_PER_EVTCHN_WORD) + bit_idx;
irq = evtchn_to_irq[port];
if (irq != -1) {
@@ -1364,12 +1371,12 @@ static void __xen_evtchn_do_upcall(void)
generic_handle_irq_desc(irq, desc);
}
- bit_idx = (bit_idx + 1) % BITS_PER_LONG;
+ bit_idx = (bit_idx + 1) % BITS_PER_EVTCHN_WORD;
/* Next caller starts at last processed + 1 */
__this_cpu_write(current_word_idx,
bit_idx ? word_idx :
- (word_idx+1) % BITS_PER_LONG);
+ (word_idx+1) % BITS_PER_EVTCHN_WORD);
__this_cpu_write(current_bit_idx, bit_idx);
} while (bit_idx != 0);
@@ -1377,7 +1384,7 @@ static void __xen_evtchn_do_upcall(void)
if ((word_idx != start_word_idx) || (i != 0))
pending_words &= ~(1UL << word_idx);
- word_idx = (word_idx + 1) % BITS_PER_LONG;
+ word_idx = (word_idx + 1) % BITS_PER_EVTCHN_WORD;
}
BUG_ON(!irqs_disabled());
@@ -1487,8 +1494,8 @@ int resend_irq_on_evtchn(unsigned int irq)
if (!VALID_EVTCHN(evtchn))
return 1;
- masked = sync_test_and_set_bit(evtchn, s->evtchn_mask);
- sync_set_bit(evtchn, s->evtchn_pending);
+ masked = sync_test_and_set_bit(evtchn, BM(s->evtchn_mask));
+ sync_set_bit(evtchn, BM(s->evtchn_pending));
if (!masked)
unmask_evtchn(evtchn);
@@ -1536,8 +1543,8 @@ static int retrigger_dynirq(struct irq_data *data)
if (VALID_EVTCHN(evtchn)) {
int masked;
- masked = sync_test_and_set_bit(evtchn, sh->evtchn_mask);
- sync_set_bit(evtchn, sh->evtchn_pending);
+ masked = sync_test_and_set_bit(evtchn, BM(sh->evtchn_mask));
+ sync_set_bit(evtchn, BM(sh->evtchn_pending));
if (!masked)
unmask_evtchn(evtchn);
ret = 1;
diff --git a/include/xen/interface/xen.h b/include/xen/interface/xen.h
index 5d6c272..a9075df 100644
--- a/include/xen/interface/xen.h
+++ b/include/xen/interface/xen.h
@@ -285,7 +285,7 @@ DEFINE_GUEST_HANDLE_STRUCT(multicall_entry);
* Event channel endpoints per domain:
* 1024 if a long is 32 bits; 4096 if a long is 64 bits.
*/
-#define NR_EVENT_CHANNELS (sizeof(unsigned long) * sizeof(unsigned long) * 64)
+#define NR_EVENT_CHANNELS (sizeof(xen_ulong_t) * sizeof(xen_ulong_t) * 64)
struct vcpu_time_info {
/*
@@ -341,7 +341,7 @@ struct vcpu_info {
*/
uint8_t evtchn_upcall_pending;
uint8_t evtchn_upcall_mask;
- unsigned long evtchn_pending_sel;
+ xen_ulong_t evtchn_pending_sel;
struct arch_vcpu_info arch;
struct pvclock_vcpu_time_info time;
}; /* 64 bytes (x86) */
@@ -384,8 +384,8 @@ struct shared_info {
* per-vcpu selector word to be set. Each bit in the selector covers a
* 'C long' in the PENDING bitfield array.
*/
- unsigned long evtchn_pending[sizeof(unsigned long) * 8];
- unsigned long evtchn_mask[sizeof(unsigned long) * 8];
+ xen_ulong_t evtchn_pending[sizeof(xen_ulong_t) * 8];
+ xen_ulong_t evtchn_mask[sizeof(xen_ulong_t) * 8];
/*
* Wallclock time: updated only by control software. Guests should base
--
1.7.2.5
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH XEN] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-02-19 14:48 [PATCH V2] xen: event channel arrays are xen_ulong_t and not unsigned long Ian Campbell
2013-02-19 14:49 ` [PATCH LINUX] " Ian Campbell
@ 2013-02-19 14:49 ` Ian Campbell
2013-02-19 16:42 ` Stefano Stabellini
2013-02-21 17:16 ` Ian Campbell
2013-02-19 17:27 ` [PATCH LINUX v3] " Ian Campbell
` (2 subsequent siblings)
4 siblings, 2 replies; 36+ messages in thread
From: Ian Campbell @ 2013-02-19 14:49 UTC (permalink / raw)
To: linux-arm-kernel
On ARM we want these to be the same size on 32- and 64-bit.
This is an ABI change on ARM. X86 does not change.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Jan Beulich <JBeulich@suse.com>
Cc: Keir (Xen.org) <keir@xen.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: linux-arm-kernel at lists.infradead.org
Cc: xen-devel at lists.xen.org
---
| 6 ++++++
xen/include/public/xen.h | 8 ++++----
2 files changed, 10 insertions(+), 4 deletions(-)
--git a/tools/include/xen-foreign/mkheader.py b/tools/include/xen-foreign/mkheader.py
index d189b07..57681fa 100644
--- a/tools/include/xen-foreign/mkheader.py
+++ b/tools/include/xen-foreign/mkheader.py
@@ -21,13 +21,18 @@ inttypes["arm"] = {
"unsigned long" : "uint32_t",
"long" : "uint32_t",
"xen_pfn_t" : "uint64_t",
+ "xen_ulong_t" : "uint64_t",
};
+header["arm"] = """
+#define __arm___ARM32 1
+""";
# x86_32
inttypes["x86_32"] = {
"unsigned long" : "uint32_t",
"long" : "uint32_t",
"xen_pfn_t" : "uint32_t",
+ "xen_ulong_t" : "uint32_t",
};
header["x86_32"] = """
#define __i386___X86_32 1
@@ -42,6 +47,7 @@ inttypes["x86_64"] = {
"unsigned long" : "__align8__ uint64_t",
"long" : "__align8__ uint64_t",
"xen_pfn_t" : "__align8__ uint64_t",
+ "xen_ulong_t" : "__align8__ uint64_t",
};
header["x86_64"] = """
#if defined(__GNUC__) && !defined(__STRICT_ANSI__)
diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
index 5593066..99c8212 100644
--- a/xen/include/public/xen.h
+++ b/xen/include/public/xen.h
@@ -556,7 +556,7 @@ DEFINE_XEN_GUEST_HANDLE(multicall_entry_t);
* Event channel endpoints per domain:
* 1024 if a long is 32 bits; 4096 if a long is 64 bits.
*/
-#define NR_EVENT_CHANNELS (sizeof(unsigned long) * sizeof(unsigned long) * 64)
+#define NR_EVENT_CHANNELS (sizeof(xen_ulong_t) * sizeof(xen_ulong_t) * 64)
struct vcpu_time_info {
/*
@@ -613,7 +613,7 @@ struct vcpu_info {
*/
uint8_t evtchn_upcall_pending;
uint8_t evtchn_upcall_mask;
- unsigned long evtchn_pending_sel;
+ xen_ulong_t evtchn_pending_sel;
struct arch_vcpu_info arch;
struct vcpu_time_info time;
}; /* 64 bytes (x86) */
@@ -663,8 +663,8 @@ struct shared_info {
* per-vcpu selector word to be set. Each bit in the selector covers a
* 'C long' in the PENDING bitfield array.
*/
- unsigned long evtchn_pending[sizeof(unsigned long) * 8];
- unsigned long evtchn_mask[sizeof(unsigned long) * 8];
+ xen_ulong_t evtchn_pending[sizeof(xen_ulong_t) * 8];
+ xen_ulong_t evtchn_mask[sizeof(xen_ulong_t) * 8];
/*
* Wallclock time: updated only by control software. Guests should base
--
1.7.9.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH XEN] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-02-19 14:49 ` [PATCH XEN] " Ian Campbell
@ 2013-02-19 16:42 ` Stefano Stabellini
2013-02-21 17:16 ` Ian Campbell
1 sibling, 0 replies; 36+ messages in thread
From: Stefano Stabellini @ 2013-02-19 16:42 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 19 Feb 2013, Ian Campbell wrote:
> On ARM we want these to be the same size on 32- and 64-bit.
>
> This is an ABI change on ARM. X86 does not change.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Jan Beulich <JBeulich@suse.com>
> Cc: Keir (Xen.org) <keir@xen.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: xen-devel at lists.xen.org
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> tools/include/xen-foreign/mkheader.py | 6 ++++++
> xen/include/public/xen.h | 8 ++++----
> 2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/tools/include/xen-foreign/mkheader.py b/tools/include/xen-foreign/mkheader.py
> index d189b07..57681fa 100644
> --- a/tools/include/xen-foreign/mkheader.py
> +++ b/tools/include/xen-foreign/mkheader.py
> @@ -21,13 +21,18 @@ inttypes["arm"] = {
> "unsigned long" : "uint32_t",
> "long" : "uint32_t",
> "xen_pfn_t" : "uint64_t",
> + "xen_ulong_t" : "uint64_t",
> };
> +header["arm"] = """
> +#define __arm___ARM32 1
> +""";
>
> # x86_32
> inttypes["x86_32"] = {
> "unsigned long" : "uint32_t",
> "long" : "uint32_t",
> "xen_pfn_t" : "uint32_t",
> + "xen_ulong_t" : "uint32_t",
> };
> header["x86_32"] = """
> #define __i386___X86_32 1
> @@ -42,6 +47,7 @@ inttypes["x86_64"] = {
> "unsigned long" : "__align8__ uint64_t",
> "long" : "__align8__ uint64_t",
> "xen_pfn_t" : "__align8__ uint64_t",
> + "xen_ulong_t" : "__align8__ uint64_t",
> };
> header["x86_64"] = """
> #if defined(__GNUC__) && !defined(__STRICT_ANSI__)
> diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
> index 5593066..99c8212 100644
> --- a/xen/include/public/xen.h
> +++ b/xen/include/public/xen.h
> @@ -556,7 +556,7 @@ DEFINE_XEN_GUEST_HANDLE(multicall_entry_t);
> * Event channel endpoints per domain:
> * 1024 if a long is 32 bits; 4096 if a long is 64 bits.
> */
> -#define NR_EVENT_CHANNELS (sizeof(unsigned long) * sizeof(unsigned long) * 64)
> +#define NR_EVENT_CHANNELS (sizeof(xen_ulong_t) * sizeof(xen_ulong_t) * 64)
>
> struct vcpu_time_info {
> /*
> @@ -613,7 +613,7 @@ struct vcpu_info {
> */
> uint8_t evtchn_upcall_pending;
> uint8_t evtchn_upcall_mask;
> - unsigned long evtchn_pending_sel;
> + xen_ulong_t evtchn_pending_sel;
> struct arch_vcpu_info arch;
> struct vcpu_time_info time;
> }; /* 64 bytes (x86) */
> @@ -663,8 +663,8 @@ struct shared_info {
> * per-vcpu selector word to be set. Each bit in the selector covers a
> * 'C long' in the PENDING bitfield array.
> */
> - unsigned long evtchn_pending[sizeof(unsigned long) * 8];
> - unsigned long evtchn_mask[sizeof(unsigned long) * 8];
> + xen_ulong_t evtchn_pending[sizeof(xen_ulong_t) * 8];
> + xen_ulong_t evtchn_mask[sizeof(xen_ulong_t) * 8];
>
> /*
> * Wallclock time: updated only by control software. Guests should base
> --
> 1.7.9.1
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH LINUX] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-02-19 14:49 ` [PATCH LINUX] " Ian Campbell
@ 2013-02-19 17:11 ` Stefano Stabellini
2013-02-19 17:17 ` Ian Campbell
0 siblings, 1 reply; 36+ messages in thread
From: Stefano Stabellini @ 2013-02-19 17:11 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 19 Feb 2013, Ian Campbell wrote:
> On ARM we want these to be the same size on 32- and 64-bit.
>
> This is an ABI change on ARM. X86 does not change.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Jan Beulich <JBeulich@suse.com>
> Cc: Keir (Xen.org) <keir@xen.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: xen-devel at lists.xen.org
> ---
> Changes since V1
> use find_first_set not __ffs
> fix some more unsigned long -> xen_ulong_t
> use more generic xchg_xen_ulong instead of ...read_evtchn...
> ---
> arch/arm/include/asm/xen/events.h | 22 ++++++++
> arch/x86/include/asm/xen/events.h | 3 +
> drivers/xen/events.c | 103 ++++++++++++++++++++-----------------
> include/xen/interface/xen.h | 8 ++--
> 4 files changed, 84 insertions(+), 52 deletions(-)
You might have to rebase this patch: it doesn't apply on Linux 3.8.
> diff --git a/arch/arm/include/asm/xen/events.h b/arch/arm/include/asm/xen/events.h
> index 94b4e90..9bb5f50 100644
> --- a/arch/arm/include/asm/xen/events.h
> +++ b/arch/arm/include/asm/xen/events.h
> @@ -15,4 +15,26 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
> return raw_irqs_disabled_flags(regs->ARM_cpsr);
> }
>
> +/*
> + * We cannot use xchg because it does not support 8-byte
> + * values. However it is safe to use {ldr,dtd}exd directly because all
> + * platforms which Xen can run on support those instructions.
> + */
> +static inline xen_ulong_t xchg_xen_ulong(xen_ulong_t *ptr, xen_ulong_t val)
> +{
> + xen_ulong_t oldval;
> + unsigned int tmp;
> +
> + wmb();
> + asm volatile("@ read_evtchn_pending_sel\n"
^ do we need this?
> + "1: ldrexd %0, %H0, [%3]\n"
> + " strexd %1, %2, %H2, [%3]\n"
> + " teq %1, #0\n"
> + " bne 1b"
> + : "=&r" (oldval), "=&r" (tmp)
> + : "r" (val), "r" (ptr)
> + : "memory", "cc");
> + return oldval;
> +}
> +
> #endif /* _ASM_ARM_XEN_EVENTS_H */
[...]
> @@ -1295,18 +1306,14 @@ static void __xen_evtchn_do_upcall(void)
> unsigned count;
>
> do {
> - unsigned long pending_words;
> + xen_ulong_t pending_words;
>
> vcpu_info->evtchn_upcall_pending = 0;
>
> if (__this_cpu_inc_return(xed_nesting_count) - 1)
> goto out;
>
> -#ifndef CONFIG_X86 /* No need for a barrier -- XCHG is a barrier on x86. */
> - /* Clear master flag /before/ clearing selector flag. */
> - wmb();
> -#endif
Even though I understand that moving wmb() into xchg_xen_ulong gets rid
of an ugly ifndef, I am not sure whether it is a good thing from the
code readability point of view. I'll let Konrad decide on this one.
> - pending_words = xchg(&vcpu_info->evtchn_pending_sel, 0);
> + pending_words = xchg_xen_ulong(&vcpu_info->evtchn_pending_sel, 0);
>
> start_word_idx = __this_cpu_read(current_word_idx);
> start_bit_idx = __this_cpu_read(current_bit_idx);
> @@ -1314,8 +1321,8 @@ static void __xen_evtchn_do_upcall(void)
> word_idx = start_word_idx;
>
> for (i = 0; pending_words != 0; i++) {
> - unsigned long pending_bits;
> - unsigned long words;
> + xen_ulong_t pending_bits;
> + xen_ulong_t words;
>
> words = MASK_LSBS(pending_words, word_idx);
>
> @@ -1327,7 +1334,7 @@ static void __xen_evtchn_do_upcall(void)
> bit_idx = 0;
> continue;
> }
> - word_idx = __ffs(words);
> + word_idx = find_first_bit(BM(&words), sizeof(words));
>
> pending_bits = active_evtchns(cpu, s, word_idx);
> bit_idx = 0; /* usually scan entire word from start */
is that because find_first_bit can actually cope with a bit number >=
32 and __ffs can't?
In that case it is worth adding a comment somewhere in this file,
reminding people to only use bit operations that can handle size >
unsigned long
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH LINUX] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-02-19 17:11 ` Stefano Stabellini
@ 2013-02-19 17:17 ` Ian Campbell
2013-02-19 17:26 ` Tim Deegan
0 siblings, 1 reply; 36+ messages in thread
From: Ian Campbell @ 2013-02-19 17:17 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 2013-02-19 at 17:11 +0000, Stefano Stabellini wrote:
> On Tue, 19 Feb 2013, Ian Campbell wrote:
> > On ARM we want these to be the same size on 32- and 64-bit.
> >
> > This is an ABI change on ARM. X86 does not change.
> >
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > Cc: Jan Beulich <JBeulich@suse.com>
> > Cc: Keir (Xen.org) <keir@xen.org>
> > Cc: Tim Deegan <tim@xen.org>
> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > Cc: linux-arm-kernel at lists.infradead.org
> > Cc: xen-devel at lists.xen.org
> > ---
> > Changes since V1
> > use find_first_set not __ffs
> > fix some more unsigned long -> xen_ulong_t
> > use more generic xchg_xen_ulong instead of ...read_evtchn...
> > ---
> > arch/arm/include/asm/xen/events.h | 22 ++++++++
> > arch/x86/include/asm/xen/events.h | 3 +
> > drivers/xen/events.c | 103 ++++++++++++++++++++-----------------
> > include/xen/interface/xen.h | 8 ++--
> > 4 files changed, 84 insertions(+), 52 deletions(-)
>
> You might have to rebase this patch: it doesn't apply on Linux 3.8.
Will do.
>
>
> > diff --git a/arch/arm/include/asm/xen/events.h b/arch/arm/include/asm/xen/events.h
> > index 94b4e90..9bb5f50 100644
> > --- a/arch/arm/include/asm/xen/events.h
> > +++ b/arch/arm/include/asm/xen/events.h
> > @@ -15,4 +15,26 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
> > return raw_irqs_disabled_flags(regs->ARM_cpsr);
> > }
> >
> > +/*
> > + * We cannot use xchg because it does not support 8-byte
> > + * values. However it is safe to use {ldr,dtd}exd directly because all
> > + * platforms which Xen can run on support those instructions.
> > + */
> > +static inline xen_ulong_t xchg_xen_ulong(xen_ulong_t *ptr, xen_ulong_t val)
> > +{
> > + xen_ulong_t oldval;
> > + unsigned int tmp;
> > +
> > + wmb();
> > + asm volatile("@ read_evtchn_pending_sel\n"
> ^ do we need this?
It means that the .s files (if you create them) and such are a bit more
readable, it's not that uncommon in the ARM inline assembly.
> > + "1: ldrexd %0, %H0, [%3]\n"
> > + " strexd %1, %2, %H2, [%3]\n"
> > + " teq %1, #0\n"
> > + " bne 1b"
> > + : "=&r" (oldval), "=&r" (tmp)
> > + : "r" (val), "r" (ptr)
> > + : "memory", "cc");
> > + return oldval;
> > +}
> > +
> > #endif /* _ASM_ARM_XEN_EVENTS_H */
>
> [...]
>
> > @@ -1295,18 +1306,14 @@ static void __xen_evtchn_do_upcall(void)
> > unsigned count;
> >
> > do {
> > - unsigned long pending_words;
> > + xen_ulong_t pending_words;
> >
> > vcpu_info->evtchn_upcall_pending = 0;
> >
> > if (__this_cpu_inc_return(xed_nesting_count) - 1)
> > goto out;
> >
> > -#ifndef CONFIG_X86 /* No need for a barrier -- XCHG is a barrier on x86. */
> > - /* Clear master flag /before/ clearing selector flag. */
> > - wmb();
> > -#endif
>
> Even though I understand that moving wmb() into xchg_xen_ulong gets rid
> of an ugly ifndef, I am not sure whether it is a good thing from the
> code readability point of view. I'll let Konrad decide on this one.
I actually see this the other way -- this barrier is logically part of
the exchange operation, implicitly on x86 and explicitly on ARM. I will
add a comment here though, which I think is best of both.
>
>
> > - pending_words = xchg(&vcpu_info->evtchn_pending_sel, 0);
> > + pending_words = xchg_xen_ulong(&vcpu_info->evtchn_pending_sel, 0);
> >
> > start_word_idx = __this_cpu_read(current_word_idx);
> > start_bit_idx = __this_cpu_read(current_bit_idx);
> > @@ -1314,8 +1321,8 @@ static void __xen_evtchn_do_upcall(void)
> > word_idx = start_word_idx;
> >
> > for (i = 0; pending_words != 0; i++) {
> > - unsigned long pending_bits;
> > - unsigned long words;
> > + xen_ulong_t pending_bits;
> > + xen_ulong_t words;
> >
> > words = MASK_LSBS(pending_words, word_idx);
> >
> > @@ -1327,7 +1334,7 @@ static void __xen_evtchn_do_upcall(void)
> > bit_idx = 0;
> > continue;
> > }
> > - word_idx = __ffs(words);
> > + word_idx = find_first_bit(BM(&words), sizeof(words));
> >
> > pending_bits = active_evtchns(cpu, s, word_idx);
> > bit_idx = 0; /* usually scan entire word from start */
>
> is that because find_first_bit can actually cope with a bit number >=
> 32 and __ffs can't?
Yes.
> In that case it is worth adding a comment somewhere in this file,
> reminding people to only use bit operations that can handle size >
> unsigned long
Will do.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH LINUX] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-02-19 17:17 ` Ian Campbell
@ 2013-02-19 17:26 ` Tim Deegan
2013-02-19 17:28 ` Ian Campbell
0 siblings, 1 reply; 36+ messages in thread
From: Tim Deegan @ 2013-02-19 17:26 UTC (permalink / raw)
To: linux-arm-kernel
At 17:17 +0000 on 19 Feb (1361294248), Ian Campbell wrote:
> On Tue, 2013-02-19 at 17:11 +0000, Stefano Stabellini wrote:
> > On Tue, 19 Feb 2013, Ian Campbell wrote:
> > > +/*
> > > + * We cannot use xchg because it does not support 8-byte
> > > + * values. However it is safe to use {ldr,dtd}exd directly because all
> > > + * platforms which Xen can run on support those instructions.
> > > + */
> > > +static inline xen_ulong_t xchg_xen_ulong(xen_ulong_t *ptr, xen_ulong_t val)
> > > +{
> > > + xen_ulong_t oldval;
> > > + unsigned int tmp;
> > > +
> > > + wmb();
> > > + asm volatile("@ read_evtchn_pending_sel\n"
> > ^ do we need this?
>
> It means that the .s files (if you create them) and such are a bit more
> readable, it's not that uncommon in the ARM inline assembly.
+1. s/read_evtchn_pending_sel/xchg_xen_ulong/ though.
Tim.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH LINUX v3] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-02-19 14:48 [PATCH V2] xen: event channel arrays are xen_ulong_t and not unsigned long Ian Campbell
2013-02-19 14:49 ` [PATCH LINUX] " Ian Campbell
2013-02-19 14:49 ` [PATCH XEN] " Ian Campbell
@ 2013-02-19 17:27 ` Ian Campbell
2013-02-19 17:29 ` [PATCH LINUX v4] " Ian Campbell
2013-02-20 11:48 ` [PATCH LINUX v5] " Ian Campbell
4 siblings, 0 replies; 36+ messages in thread
From: Ian Campbell @ 2013-02-19 17:27 UTC (permalink / raw)
To: linux-arm-kernel
On ARM we want these to be the same size on 32- and 64-bit.
This is an ABI change on ARM. X86 does not change.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Jan Beulich <JBeulich@suse.com>
Cc: Keir (Xen.org) <keir@xen.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: linux-arm-kernel at lists.infradead.org
Cc: xen-devel at lists.xen.org
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
Changes since V2
Add comments about the correct bitops to use, and on the ordering/barrier
requirements on xchg_xen_ulong.
Changes since V1
use find_first_set not __ffs
fix some more unsigned long -> xen_ulong_t
use more generic xchg_xen_ulong instead of ...read_evtchn...
---
arch/arm/include/asm/xen/events.h | 22 +++++++
arch/x86/include/asm/xen/events.h | 3 +
drivers/xen/events.c | 113 +++++++++++++++++++++----------------
include/xen/interface/xen.h | 8 +-
4 files changed, 94 insertions(+), 52 deletions(-)
diff --git a/arch/arm/include/asm/xen/events.h b/arch/arm/include/asm/xen/events.h
index 94b4e90..9bb5f50 100644
--- a/arch/arm/include/asm/xen/events.h
+++ b/arch/arm/include/asm/xen/events.h
@@ -15,4 +15,26 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
return raw_irqs_disabled_flags(regs->ARM_cpsr);
}
+/*
+ * We cannot use xchg because it does not support 8-byte
+ * values. However it is safe to use {ldr,dtd}exd directly because all
+ * platforms which Xen can run on support those instructions.
+ */
+static inline xen_ulong_t xchg_xen_ulong(xen_ulong_t *ptr, xen_ulong_t val)
+{
+ xen_ulong_t oldval;
+ unsigned int tmp;
+
+ wmb();
+ asm volatile("@ read_evtchn_pending_sel\n"
+ "1: ldrexd %0, %H0, [%3]\n"
+ " strexd %1, %2, %H2, [%3]\n"
+ " teq %1, #0\n"
+ " bne 1b"
+ : "=&r" (oldval), "=&r" (tmp)
+ : "r" (val), "r" (ptr)
+ : "memory", "cc");
+ return oldval;
+}
+
#endif /* _ASM_ARM_XEN_EVENTS_H */
diff --git a/arch/x86/include/asm/xen/events.h b/arch/x86/include/asm/xen/events.h
index cc146d5..ca842f2 100644
--- a/arch/x86/include/asm/xen/events.h
+++ b/arch/x86/include/asm/xen/events.h
@@ -16,4 +16,7 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
return raw_irqs_disabled_flags(regs->flags);
}
+/* No need for a barrier -- XCHG is a barrier on x86. */
+#define xchg_xen_ulong(ptr, val) xchg((ptr), (val))
+
#endif /* _ASM_X86_XEN_EVENTS_H */
diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 0be4df3..b8d84f5 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -80,6 +80,12 @@ enum xen_irq_type {
};
/*
+ * Make a bitmask (i.e. unsigned long *) of a xen_ulong_t
+ * array. Primarily to avoid long lines (hence the terse name).
+ */
+#define BM(x) (unsigned long *)(x)
+
+/*
* Packed IRQ information:
* type - enum xen_irq_type
* event channel - irq->event channel mapping
@@ -120,7 +126,14 @@ static unsigned long *pirq_eoi_map;
#endif
static bool (*pirq_needs_eoi)(unsigned irq);
-static DEFINE_PER_CPU(unsigned long [NR_EVENT_CHANNELS/BITS_PER_LONG],
+/*
+ * Note sizeof(xen_ulong_t) can be more than sizeof(unsigned long). Be
+ * careful to only use bitops which allow for this (e.g test_bit and
+ * friends but not __ffs).
+ */
+#define BITS_PER_EVTCHN_WORD (sizeof(xen_ulong_t)*8)
+
+static DEFINE_PER_CPU(xen_ulong_t [NR_EVENT_CHANNELS/BITS_PER_EVTCHN_WORD],
cpu_evtchn_mask);
/* Xen will never allocate port zero for any purpose. */
@@ -294,9 +307,9 @@ static bool pirq_needs_eoi_flag(unsigned irq)
return info->u.pirq.flags & PIRQ_NEEDS_EOI;
}
-static inline unsigned long active_evtchns(unsigned int cpu,
- struct shared_info *sh,
- unsigned int idx)
+static inline xen_ulong_t active_evtchns(unsigned int cpu,
+ struct shared_info *sh,
+ unsigned int idx)
{
return sh->evtchn_pending[idx] &
per_cpu(cpu_evtchn_mask, cpu)[idx] &
@@ -312,8 +325,8 @@ static void bind_evtchn_to_cpu(unsigned int chn, unsigned int cpu)
cpumask_copy(irq_to_desc(irq)->irq_data.affinity, cpumask_of(cpu));
#endif
- clear_bit(chn, per_cpu(cpu_evtchn_mask, cpu_from_irq(irq)));
- set_bit(chn, per_cpu(cpu_evtchn_mask, cpu));
+ clear_bit(chn, BM(per_cpu(cpu_evtchn_mask, cpu_from_irq(irq))));
+ set_bit(chn, BM(per_cpu(cpu_evtchn_mask, cpu)));
info_for_irq(irq)->cpu = cpu;
}
@@ -339,19 +352,19 @@ static void init_evtchn_cpu_bindings(void)
static inline void clear_evtchn(int port)
{
struct shared_info *s = HYPERVISOR_shared_info;
- sync_clear_bit(port, &s->evtchn_pending[0]);
+ sync_clear_bit(port, BM(&s->evtchn_pending[0]));
}
static inline void set_evtchn(int port)
{
struct shared_info *s = HYPERVISOR_shared_info;
- sync_set_bit(port, &s->evtchn_pending[0]);
+ sync_set_bit(port, BM(&s->evtchn_pending[0]));
}
static inline int test_evtchn(int port)
{
struct shared_info *s = HYPERVISOR_shared_info;
- return sync_test_bit(port, &s->evtchn_pending[0]);
+ return sync_test_bit(port, BM(&s->evtchn_pending[0]));
}
@@ -375,7 +388,7 @@ EXPORT_SYMBOL_GPL(notify_remote_via_irq);
static void mask_evtchn(int port)
{
struct shared_info *s = HYPERVISOR_shared_info;
- sync_set_bit(port, &s->evtchn_mask[0]);
+ sync_set_bit(port, BM(&s->evtchn_mask[0]));
}
static void unmask_evtchn(int port)
@@ -389,7 +402,7 @@ static void unmask_evtchn(int port)
if (unlikely((cpu != cpu_from_evtchn(port))))
do_hypercall = 1;
else
- evtchn_pending = sync_test_bit(port, &s->evtchn_pending[0]);
+ evtchn_pending = sync_test_bit(port, BM(&s->evtchn_pending[0]));
if (unlikely(evtchn_pending && xen_hvm_domain()))
do_hypercall = 1;
@@ -403,7 +416,7 @@ static void unmask_evtchn(int port)
} else {
struct vcpu_info *vcpu_info = __this_cpu_read(xen_vcpu);
- sync_clear_bit(port, &s->evtchn_mask[0]);
+ sync_clear_bit(port, BM(&s->evtchn_mask[0]));
/*
* The following is basically the equivalent of
@@ -411,8 +424,8 @@ static void unmask_evtchn(int port)
* the interrupt edge' if the channel is masked.
*/
if (evtchn_pending &&
- !sync_test_and_set_bit(port / BITS_PER_LONG,
- &vcpu_info->evtchn_pending_sel))
+ !sync_test_and_set_bit(port / BITS_PER_EVTCHN_WORD,
+ BM(&vcpu_info->evtchn_pending_sel)))
vcpu_info->evtchn_upcall_pending = 1;
}
@@ -1189,7 +1202,7 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
{
struct shared_info *sh = HYPERVISOR_shared_info;
int cpu = smp_processor_id();
- unsigned long *cpu_evtchn = per_cpu(cpu_evtchn_mask, cpu);
+ xen_ulong_t *cpu_evtchn = per_cpu(cpu_evtchn_mask, cpu);
int i;
unsigned long flags;
static DEFINE_SPINLOCK(debug_lock);
@@ -1205,7 +1218,7 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
pending = (get_irq_regs() && i == cpu)
? xen_irqs_disabled(get_irq_regs())
: v->evtchn_upcall_mask;
- printk("%d: masked=%d pending=%d event_sel %0*lx\n ", i,
+ printk("%d: masked=%d pending=%d event_sel %0*"PRI_xen_ulong"\n ", i,
pending, v->evtchn_upcall_pending,
(int)(sizeof(v->evtchn_pending_sel)*2),
v->evtchn_pending_sel);
@@ -1214,49 +1227,52 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
printk("\npending:\n ");
for (i = ARRAY_SIZE(sh->evtchn_pending)-1; i >= 0; i--)
- printk("%0*lx%s", (int)sizeof(sh->evtchn_pending[0])*2,
+ printk("%0*"PRI_xen_ulong"%s",
+ (int)sizeof(sh->evtchn_pending[0])*2,
sh->evtchn_pending[i],
i % 8 == 0 ? "\n " : " ");
printk("\nglobal mask:\n ");
for (i = ARRAY_SIZE(sh->evtchn_mask)-1; i >= 0; i--)
- printk("%0*lx%s",
+ printk("%0*"PRI_xen_ulong"%s",
(int)(sizeof(sh->evtchn_mask[0])*2),
sh->evtchn_mask[i],
i % 8 == 0 ? "\n " : " ");
printk("\nglobally unmasked:\n ");
for (i = ARRAY_SIZE(sh->evtchn_mask)-1; i >= 0; i--)
- printk("%0*lx%s", (int)(sizeof(sh->evtchn_mask[0])*2),
+ printk("%0*"PRI_xen_ulong"%s",
+ (int)(sizeof(sh->evtchn_mask[0])*2),
sh->evtchn_pending[i] & ~sh->evtchn_mask[i],
i % 8 == 0 ? "\n " : " ");
printk("\nlocal cpu%d mask:\n ", cpu);
- for (i = (NR_EVENT_CHANNELS/BITS_PER_LONG)-1; i >= 0; i--)
- printk("%0*lx%s", (int)(sizeof(cpu_evtchn[0])*2),
+ for (i = (NR_EVENT_CHANNELS/BITS_PER_EVTCHN_WORD)-1; i >= 0; i--)
+ printk("%0*"PRI_xen_ulong"%s", (int)(sizeof(cpu_evtchn[0])*2),
cpu_evtchn[i],
i % 8 == 0 ? "\n " : " ");
printk("\nlocally unmasked:\n ");
for (i = ARRAY_SIZE(sh->evtchn_mask)-1; i >= 0; i--) {
- unsigned long pending = sh->evtchn_pending[i]
+ xen_ulong_t pending = sh->evtchn_pending[i]
& ~sh->evtchn_mask[i]
& cpu_evtchn[i];
- printk("%0*lx%s", (int)(sizeof(sh->evtchn_mask[0])*2),
+ printk("%0*"PRI_xen_ulong"%s",
+ (int)(sizeof(sh->evtchn_mask[0])*2),
pending, i % 8 == 0 ? "\n " : " ");
}
printk("\npending list:\n");
for (i = 0; i < NR_EVENT_CHANNELS; i++) {
- if (sync_test_bit(i, sh->evtchn_pending)) {
- int word_idx = i / BITS_PER_LONG;
+ if (sync_test_bit(i, BM(sh->evtchn_pending))) {
+ int word_idx = i / BITS_PER_EVTCHN_WORD;
printk(" %d: event %d -> irq %d%s%s%s\n",
cpu_from_evtchn(i), i,
evtchn_to_irq[i],
- sync_test_bit(word_idx, &v->evtchn_pending_sel)
+ sync_test_bit(word_idx, BM(&v->evtchn_pending_sel))
? "" : " l2-clear",
- !sync_test_bit(i, sh->evtchn_mask)
+ !sync_test_bit(i, BM(sh->evtchn_mask))
? "" : " globally-masked",
- sync_test_bit(i, cpu_evtchn)
+ sync_test_bit(i, BM(cpu_evtchn))
? "" : " locally-masked");
}
}
@@ -1273,7 +1289,7 @@ static DEFINE_PER_CPU(unsigned int, current_bit_idx);
/*
* Mask out the i least significant bits of w
*/
-#define MASK_LSBS(w, i) (w & ((~0UL) << i))
+#define MASK_LSBS(w, i) (w & ((~((xen_ulong_t)0UL)) << i))
/*
* Search the CPUs pending events bitmasks. For each one found, map
@@ -1295,18 +1311,19 @@ static void __xen_evtchn_do_upcall(void)
unsigned count;
do {
- unsigned long pending_words;
+ xen_ulong_t pending_words;
vcpu_info->evtchn_upcall_pending = 0;
if (__this_cpu_inc_return(xed_nesting_count) - 1)
goto out;
-#ifndef CONFIG_X86 /* No need for a barrier -- XCHG is a barrier on x86. */
- /* Clear master flag /before/ clearing selector flag. */
- wmb();
-#endif
- pending_words = xchg(&vcpu_info->evtchn_pending_sel, 0);
+ /*
+ * Master flag must be /before/ clearing selector
+ * flag. xchg_xen_ulong must contain an appropriate
+ * barrier.
+ */
+ pending_words = xchg_xen_ulong(&vcpu_info->evtchn_pending_sel, 0);
start_word_idx = __this_cpu_read(current_word_idx);
start_bit_idx = __this_cpu_read(current_bit_idx);
@@ -1314,8 +1331,8 @@ static void __xen_evtchn_do_upcall(void)
word_idx = start_word_idx;
for (i = 0; pending_words != 0; i++) {
- unsigned long pending_bits;
- unsigned long words;
+ xen_ulong_t pending_bits;
+ xen_ulong_t words;
words = MASK_LSBS(pending_words, word_idx);
@@ -1327,7 +1344,7 @@ static void __xen_evtchn_do_upcall(void)
bit_idx = 0;
continue;
}
- word_idx = __ffs(words);
+ word_idx = find_first_bit(BM(&words), sizeof(words));
pending_bits = active_evtchns(cpu, s, word_idx);
bit_idx = 0; /* usually scan entire word from start */
@@ -1342,7 +1359,7 @@ static void __xen_evtchn_do_upcall(void)
}
do {
- unsigned long bits;
+ xen_ulong_t bits;
int port, irq;
struct irq_desc *desc;
@@ -1352,10 +1369,10 @@ static void __xen_evtchn_do_upcall(void)
if (bits == 0)
break;
- bit_idx = __ffs(bits);
+ bit_idx = find_first_bit(BM(&bits), sizeof(bits));
/* Process port. */
- port = (word_idx * BITS_PER_LONG) + bit_idx;
+ port = (word_idx * BITS_PER_EVTCHN_WORD) + bit_idx;
irq = evtchn_to_irq[port];
if (irq != -1) {
@@ -1364,12 +1381,12 @@ static void __xen_evtchn_do_upcall(void)
generic_handle_irq_desc(irq, desc);
}
- bit_idx = (bit_idx + 1) % BITS_PER_LONG;
+ bit_idx = (bit_idx + 1) % BITS_PER_EVTCHN_WORD;
/* Next caller starts at last processed + 1 */
__this_cpu_write(current_word_idx,
bit_idx ? word_idx :
- (word_idx+1) % BITS_PER_LONG);
+ (word_idx+1) % BITS_PER_EVTCHN_WORD);
__this_cpu_write(current_bit_idx, bit_idx);
} while (bit_idx != 0);
@@ -1377,7 +1394,7 @@ static void __xen_evtchn_do_upcall(void)
if ((word_idx != start_word_idx) || (i != 0))
pending_words &= ~(1UL << word_idx);
- word_idx = (word_idx + 1) % BITS_PER_LONG;
+ word_idx = (word_idx + 1) % BITS_PER_EVTCHN_WORD;
}
BUG_ON(!irqs_disabled());
@@ -1487,8 +1504,8 @@ int resend_irq_on_evtchn(unsigned int irq)
if (!VALID_EVTCHN(evtchn))
return 1;
- masked = sync_test_and_set_bit(evtchn, s->evtchn_mask);
- sync_set_bit(evtchn, s->evtchn_pending);
+ masked = sync_test_and_set_bit(evtchn, BM(s->evtchn_mask));
+ sync_set_bit(evtchn, BM(s->evtchn_pending));
if (!masked)
unmask_evtchn(evtchn);
@@ -1536,8 +1553,8 @@ static int retrigger_dynirq(struct irq_data *data)
if (VALID_EVTCHN(evtchn)) {
int masked;
- masked = sync_test_and_set_bit(evtchn, sh->evtchn_mask);
- sync_set_bit(evtchn, sh->evtchn_pending);
+ masked = sync_test_and_set_bit(evtchn, BM(sh->evtchn_mask));
+ sync_set_bit(evtchn, BM(sh->evtchn_pending));
if (!masked)
unmask_evtchn(evtchn);
ret = 1;
diff --git a/include/xen/interface/xen.h b/include/xen/interface/xen.h
index 5d6c272..a9075df 100644
--- a/include/xen/interface/xen.h
+++ b/include/xen/interface/xen.h
@@ -285,7 +285,7 @@ DEFINE_GUEST_HANDLE_STRUCT(multicall_entry);
* Event channel endpoints per domain:
* 1024 if a long is 32 bits; 4096 if a long is 64 bits.
*/
-#define NR_EVENT_CHANNELS (sizeof(unsigned long) * sizeof(unsigned long) * 64)
+#define NR_EVENT_CHANNELS (sizeof(xen_ulong_t) * sizeof(xen_ulong_t) * 64)
struct vcpu_time_info {
/*
@@ -341,7 +341,7 @@ struct vcpu_info {
*/
uint8_t evtchn_upcall_pending;
uint8_t evtchn_upcall_mask;
- unsigned long evtchn_pending_sel;
+ xen_ulong_t evtchn_pending_sel;
struct arch_vcpu_info arch;
struct pvclock_vcpu_time_info time;
}; /* 64 bytes (x86) */
@@ -384,8 +384,8 @@ struct shared_info {
* per-vcpu selector word to be set. Each bit in the selector covers a
* 'C long' in the PENDING bitfield array.
*/
- unsigned long evtchn_pending[sizeof(unsigned long) * 8];
- unsigned long evtchn_mask[sizeof(unsigned long) * 8];
+ xen_ulong_t evtchn_pending[sizeof(xen_ulong_t) * 8];
+ xen_ulong_t evtchn_mask[sizeof(xen_ulong_t) * 8];
/*
* Wallclock time: updated only by control software. Guests should base
--
1.7.2.5
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH LINUX] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-02-19 17:26 ` Tim Deegan
@ 2013-02-19 17:28 ` Ian Campbell
0 siblings, 0 replies; 36+ messages in thread
From: Ian Campbell @ 2013-02-19 17:28 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 2013-02-19 at 17:26 +0000, Tim Deegan wrote:
> At 17:17 +0000 on 19 Feb (1361294248), Ian Campbell wrote:
> > On Tue, 2013-02-19 at 17:11 +0000, Stefano Stabellini wrote:
> > > On Tue, 19 Feb 2013, Ian Campbell wrote:
> > > > +/*
> > > > + * We cannot use xchg because it does not support 8-byte
> > > > + * values. However it is safe to use {ldr,dtd}exd directly because all
> > > > + * platforms which Xen can run on support those instructions.
> > > > + */
> > > > +static inline xen_ulong_t xchg_xen_ulong(xen_ulong_t *ptr, xen_ulong_t val)
> > > > +{
> > > > + xen_ulong_t oldval;
> > > > + unsigned int tmp;
> > > > +
> > > > + wmb();
> > > > + asm volatile("@ read_evtchn_pending_sel\n"
> > > ^ do we need this?
> >
> > It means that the .s files (if you create them) and such are a bit more
> > readable, it's not that uncommon in the ARM inline assembly.
>
> +1. s/read_evtchn_pending_sel/xchg_xen_ulong/ though.
Damn, yes. I just sent v3 too :-(
Ian.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH LINUX v4] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-02-19 14:48 [PATCH V2] xen: event channel arrays are xen_ulong_t and not unsigned long Ian Campbell
` (2 preceding siblings ...)
2013-02-19 17:27 ` [PATCH LINUX v3] " Ian Campbell
@ 2013-02-19 17:29 ` Ian Campbell
2013-02-19 18:12 ` Stefano Stabellini
2013-02-20 2:07 ` Konrad Rzeszutek Wilk
2013-02-20 11:48 ` [PATCH LINUX v5] " Ian Campbell
4 siblings, 2 replies; 36+ messages in thread
From: Ian Campbell @ 2013-02-19 17:29 UTC (permalink / raw)
To: linux-arm-kernel
On ARM we want these to be the same size on 32- and 64-bit.
This is an ABI change on ARM. X86 does not change.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Jan Beulich <JBeulich@suse.com>
Cc: Keir (Xen.org) <keir@xen.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: linux-arm-kernel at lists.infradead.org
Cc: xen-devel at lists.xen.org
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
Changes since V3
s/read_evtchn_pending_sel/xchg_xen_ulong/ in a comment.
Changes since V2
Add comments about the correct bitops to use, and on the ordering/barrier
requirements on xchg_xen_ulong.
Changes since V1
use find_first_set not __ffs
fix some more unsigned long -> xen_ulong_t
use more generic xchg_xen_ulong instead of ...read_evtchn...
---
arch/arm/include/asm/xen/events.h | 22 +++++++
arch/x86/include/asm/xen/events.h | 3 +
drivers/xen/events.c | 113 +++++++++++++++++++++----------------
include/xen/interface/xen.h | 8 +-
4 files changed, 94 insertions(+), 52 deletions(-)
diff --git a/arch/arm/include/asm/xen/events.h b/arch/arm/include/asm/xen/events.h
index 94b4e90..5c27696 100644
--- a/arch/arm/include/asm/xen/events.h
+++ b/arch/arm/include/asm/xen/events.h
@@ -15,4 +15,26 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
return raw_irqs_disabled_flags(regs->ARM_cpsr);
}
+/*
+ * We cannot use xchg because it does not support 8-byte
+ * values. However it is safe to use {ldr,dtd}exd directly because all
+ * platforms which Xen can run on support those instructions.
+ */
+static inline xen_ulong_t xchg_xen_ulong(xen_ulong_t *ptr, xen_ulong_t val)
+{
+ xen_ulong_t oldval;
+ unsigned int tmp;
+
+ wmb();
+ asm volatile("@ xchg_xen_ulong\n"
+ "1: ldrexd %0, %H0, [%3]\n"
+ " strexd %1, %2, %H2, [%3]\n"
+ " teq %1, #0\n"
+ " bne 1b"
+ : "=&r" (oldval), "=&r" (tmp)
+ : "r" (val), "r" (ptr)
+ : "memory", "cc");
+ return oldval;
+}
+
#endif /* _ASM_ARM_XEN_EVENTS_H */
diff --git a/arch/x86/include/asm/xen/events.h b/arch/x86/include/asm/xen/events.h
index cc146d5..ca842f2 100644
--- a/arch/x86/include/asm/xen/events.h
+++ b/arch/x86/include/asm/xen/events.h
@@ -16,4 +16,7 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
return raw_irqs_disabled_flags(regs->flags);
}
+/* No need for a barrier -- XCHG is a barrier on x86. */
+#define xchg_xen_ulong(ptr, val) xchg((ptr), (val))
+
#endif /* _ASM_X86_XEN_EVENTS_H */
diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 0be4df3..b8d84f5 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -80,6 +80,12 @@ enum xen_irq_type {
};
/*
+ * Make a bitmask (i.e. unsigned long *) of a xen_ulong_t
+ * array. Primarily to avoid long lines (hence the terse name).
+ */
+#define BM(x) (unsigned long *)(x)
+
+/*
* Packed IRQ information:
* type - enum xen_irq_type
* event channel - irq->event channel mapping
@@ -120,7 +126,14 @@ static unsigned long *pirq_eoi_map;
#endif
static bool (*pirq_needs_eoi)(unsigned irq);
-static DEFINE_PER_CPU(unsigned long [NR_EVENT_CHANNELS/BITS_PER_LONG],
+/*
+ * Note sizeof(xen_ulong_t) can be more than sizeof(unsigned long). Be
+ * careful to only use bitops which allow for this (e.g test_bit and
+ * friends but not __ffs).
+ */
+#define BITS_PER_EVTCHN_WORD (sizeof(xen_ulong_t)*8)
+
+static DEFINE_PER_CPU(xen_ulong_t [NR_EVENT_CHANNELS/BITS_PER_EVTCHN_WORD],
cpu_evtchn_mask);
/* Xen will never allocate port zero for any purpose. */
@@ -294,9 +307,9 @@ static bool pirq_needs_eoi_flag(unsigned irq)
return info->u.pirq.flags & PIRQ_NEEDS_EOI;
}
-static inline unsigned long active_evtchns(unsigned int cpu,
- struct shared_info *sh,
- unsigned int idx)
+static inline xen_ulong_t active_evtchns(unsigned int cpu,
+ struct shared_info *sh,
+ unsigned int idx)
{
return sh->evtchn_pending[idx] &
per_cpu(cpu_evtchn_mask, cpu)[idx] &
@@ -312,8 +325,8 @@ static void bind_evtchn_to_cpu(unsigned int chn, unsigned int cpu)
cpumask_copy(irq_to_desc(irq)->irq_data.affinity, cpumask_of(cpu));
#endif
- clear_bit(chn, per_cpu(cpu_evtchn_mask, cpu_from_irq(irq)));
- set_bit(chn, per_cpu(cpu_evtchn_mask, cpu));
+ clear_bit(chn, BM(per_cpu(cpu_evtchn_mask, cpu_from_irq(irq))));
+ set_bit(chn, BM(per_cpu(cpu_evtchn_mask, cpu)));
info_for_irq(irq)->cpu = cpu;
}
@@ -339,19 +352,19 @@ static void init_evtchn_cpu_bindings(void)
static inline void clear_evtchn(int port)
{
struct shared_info *s = HYPERVISOR_shared_info;
- sync_clear_bit(port, &s->evtchn_pending[0]);
+ sync_clear_bit(port, BM(&s->evtchn_pending[0]));
}
static inline void set_evtchn(int port)
{
struct shared_info *s = HYPERVISOR_shared_info;
- sync_set_bit(port, &s->evtchn_pending[0]);
+ sync_set_bit(port, BM(&s->evtchn_pending[0]));
}
static inline int test_evtchn(int port)
{
struct shared_info *s = HYPERVISOR_shared_info;
- return sync_test_bit(port, &s->evtchn_pending[0]);
+ return sync_test_bit(port, BM(&s->evtchn_pending[0]));
}
@@ -375,7 +388,7 @@ EXPORT_SYMBOL_GPL(notify_remote_via_irq);
static void mask_evtchn(int port)
{
struct shared_info *s = HYPERVISOR_shared_info;
- sync_set_bit(port, &s->evtchn_mask[0]);
+ sync_set_bit(port, BM(&s->evtchn_mask[0]));
}
static void unmask_evtchn(int port)
@@ -389,7 +402,7 @@ static void unmask_evtchn(int port)
if (unlikely((cpu != cpu_from_evtchn(port))))
do_hypercall = 1;
else
- evtchn_pending = sync_test_bit(port, &s->evtchn_pending[0]);
+ evtchn_pending = sync_test_bit(port, BM(&s->evtchn_pending[0]));
if (unlikely(evtchn_pending && xen_hvm_domain()))
do_hypercall = 1;
@@ -403,7 +416,7 @@ static void unmask_evtchn(int port)
} else {
struct vcpu_info *vcpu_info = __this_cpu_read(xen_vcpu);
- sync_clear_bit(port, &s->evtchn_mask[0]);
+ sync_clear_bit(port, BM(&s->evtchn_mask[0]));
/*
* The following is basically the equivalent of
@@ -411,8 +424,8 @@ static void unmask_evtchn(int port)
* the interrupt edge' if the channel is masked.
*/
if (evtchn_pending &&
- !sync_test_and_set_bit(port / BITS_PER_LONG,
- &vcpu_info->evtchn_pending_sel))
+ !sync_test_and_set_bit(port / BITS_PER_EVTCHN_WORD,
+ BM(&vcpu_info->evtchn_pending_sel)))
vcpu_info->evtchn_upcall_pending = 1;
}
@@ -1189,7 +1202,7 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
{
struct shared_info *sh = HYPERVISOR_shared_info;
int cpu = smp_processor_id();
- unsigned long *cpu_evtchn = per_cpu(cpu_evtchn_mask, cpu);
+ xen_ulong_t *cpu_evtchn = per_cpu(cpu_evtchn_mask, cpu);
int i;
unsigned long flags;
static DEFINE_SPINLOCK(debug_lock);
@@ -1205,7 +1218,7 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
pending = (get_irq_regs() && i == cpu)
? xen_irqs_disabled(get_irq_regs())
: v->evtchn_upcall_mask;
- printk("%d: masked=%d pending=%d event_sel %0*lx\n ", i,
+ printk("%d: masked=%d pending=%d event_sel %0*"PRI_xen_ulong"\n ", i,
pending, v->evtchn_upcall_pending,
(int)(sizeof(v->evtchn_pending_sel)*2),
v->evtchn_pending_sel);
@@ -1214,49 +1227,52 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
printk("\npending:\n ");
for (i = ARRAY_SIZE(sh->evtchn_pending)-1; i >= 0; i--)
- printk("%0*lx%s", (int)sizeof(sh->evtchn_pending[0])*2,
+ printk("%0*"PRI_xen_ulong"%s",
+ (int)sizeof(sh->evtchn_pending[0])*2,
sh->evtchn_pending[i],
i % 8 == 0 ? "\n " : " ");
printk("\nglobal mask:\n ");
for (i = ARRAY_SIZE(sh->evtchn_mask)-1; i >= 0; i--)
- printk("%0*lx%s",
+ printk("%0*"PRI_xen_ulong"%s",
(int)(sizeof(sh->evtchn_mask[0])*2),
sh->evtchn_mask[i],
i % 8 == 0 ? "\n " : " ");
printk("\nglobally unmasked:\n ");
for (i = ARRAY_SIZE(sh->evtchn_mask)-1; i >= 0; i--)
- printk("%0*lx%s", (int)(sizeof(sh->evtchn_mask[0])*2),
+ printk("%0*"PRI_xen_ulong"%s",
+ (int)(sizeof(sh->evtchn_mask[0])*2),
sh->evtchn_pending[i] & ~sh->evtchn_mask[i],
i % 8 == 0 ? "\n " : " ");
printk("\nlocal cpu%d mask:\n ", cpu);
- for (i = (NR_EVENT_CHANNELS/BITS_PER_LONG)-1; i >= 0; i--)
- printk("%0*lx%s", (int)(sizeof(cpu_evtchn[0])*2),
+ for (i = (NR_EVENT_CHANNELS/BITS_PER_EVTCHN_WORD)-1; i >= 0; i--)
+ printk("%0*"PRI_xen_ulong"%s", (int)(sizeof(cpu_evtchn[0])*2),
cpu_evtchn[i],
i % 8 == 0 ? "\n " : " ");
printk("\nlocally unmasked:\n ");
for (i = ARRAY_SIZE(sh->evtchn_mask)-1; i >= 0; i--) {
- unsigned long pending = sh->evtchn_pending[i]
+ xen_ulong_t pending = sh->evtchn_pending[i]
& ~sh->evtchn_mask[i]
& cpu_evtchn[i];
- printk("%0*lx%s", (int)(sizeof(sh->evtchn_mask[0])*2),
+ printk("%0*"PRI_xen_ulong"%s",
+ (int)(sizeof(sh->evtchn_mask[0])*2),
pending, i % 8 == 0 ? "\n " : " ");
}
printk("\npending list:\n");
for (i = 0; i < NR_EVENT_CHANNELS; i++) {
- if (sync_test_bit(i, sh->evtchn_pending)) {
- int word_idx = i / BITS_PER_LONG;
+ if (sync_test_bit(i, BM(sh->evtchn_pending))) {
+ int word_idx = i / BITS_PER_EVTCHN_WORD;
printk(" %d: event %d -> irq %d%s%s%s\n",
cpu_from_evtchn(i), i,
evtchn_to_irq[i],
- sync_test_bit(word_idx, &v->evtchn_pending_sel)
+ sync_test_bit(word_idx, BM(&v->evtchn_pending_sel))
? "" : " l2-clear",
- !sync_test_bit(i, sh->evtchn_mask)
+ !sync_test_bit(i, BM(sh->evtchn_mask))
? "" : " globally-masked",
- sync_test_bit(i, cpu_evtchn)
+ sync_test_bit(i, BM(cpu_evtchn))
? "" : " locally-masked");
}
}
@@ -1273,7 +1289,7 @@ static DEFINE_PER_CPU(unsigned int, current_bit_idx);
/*
* Mask out the i least significant bits of w
*/
-#define MASK_LSBS(w, i) (w & ((~0UL) << i))
+#define MASK_LSBS(w, i) (w & ((~((xen_ulong_t)0UL)) << i))
/*
* Search the CPUs pending events bitmasks. For each one found, map
@@ -1295,18 +1311,19 @@ static void __xen_evtchn_do_upcall(void)
unsigned count;
do {
- unsigned long pending_words;
+ xen_ulong_t pending_words;
vcpu_info->evtchn_upcall_pending = 0;
if (__this_cpu_inc_return(xed_nesting_count) - 1)
goto out;
-#ifndef CONFIG_X86 /* No need for a barrier -- XCHG is a barrier on x86. */
- /* Clear master flag /before/ clearing selector flag. */
- wmb();
-#endif
- pending_words = xchg(&vcpu_info->evtchn_pending_sel, 0);
+ /*
+ * Master flag must be /before/ clearing selector
+ * flag. xchg_xen_ulong must contain an appropriate
+ * barrier.
+ */
+ pending_words = xchg_xen_ulong(&vcpu_info->evtchn_pending_sel, 0);
start_word_idx = __this_cpu_read(current_word_idx);
start_bit_idx = __this_cpu_read(current_bit_idx);
@@ -1314,8 +1331,8 @@ static void __xen_evtchn_do_upcall(void)
word_idx = start_word_idx;
for (i = 0; pending_words != 0; i++) {
- unsigned long pending_bits;
- unsigned long words;
+ xen_ulong_t pending_bits;
+ xen_ulong_t words;
words = MASK_LSBS(pending_words, word_idx);
@@ -1327,7 +1344,7 @@ static void __xen_evtchn_do_upcall(void)
bit_idx = 0;
continue;
}
- word_idx = __ffs(words);
+ word_idx = find_first_bit(BM(&words), sizeof(words));
pending_bits = active_evtchns(cpu, s, word_idx);
bit_idx = 0; /* usually scan entire word from start */
@@ -1342,7 +1359,7 @@ static void __xen_evtchn_do_upcall(void)
}
do {
- unsigned long bits;
+ xen_ulong_t bits;
int port, irq;
struct irq_desc *desc;
@@ -1352,10 +1369,10 @@ static void __xen_evtchn_do_upcall(void)
if (bits == 0)
break;
- bit_idx = __ffs(bits);
+ bit_idx = find_first_bit(BM(&bits), sizeof(bits));
/* Process port. */
- port = (word_idx * BITS_PER_LONG) + bit_idx;
+ port = (word_idx * BITS_PER_EVTCHN_WORD) + bit_idx;
irq = evtchn_to_irq[port];
if (irq != -1) {
@@ -1364,12 +1381,12 @@ static void __xen_evtchn_do_upcall(void)
generic_handle_irq_desc(irq, desc);
}
- bit_idx = (bit_idx + 1) % BITS_PER_LONG;
+ bit_idx = (bit_idx + 1) % BITS_PER_EVTCHN_WORD;
/* Next caller starts at last processed + 1 */
__this_cpu_write(current_word_idx,
bit_idx ? word_idx :
- (word_idx+1) % BITS_PER_LONG);
+ (word_idx+1) % BITS_PER_EVTCHN_WORD);
__this_cpu_write(current_bit_idx, bit_idx);
} while (bit_idx != 0);
@@ -1377,7 +1394,7 @@ static void __xen_evtchn_do_upcall(void)
if ((word_idx != start_word_idx) || (i != 0))
pending_words &= ~(1UL << word_idx);
- word_idx = (word_idx + 1) % BITS_PER_LONG;
+ word_idx = (word_idx + 1) % BITS_PER_EVTCHN_WORD;
}
BUG_ON(!irqs_disabled());
@@ -1487,8 +1504,8 @@ int resend_irq_on_evtchn(unsigned int irq)
if (!VALID_EVTCHN(evtchn))
return 1;
- masked = sync_test_and_set_bit(evtchn, s->evtchn_mask);
- sync_set_bit(evtchn, s->evtchn_pending);
+ masked = sync_test_and_set_bit(evtchn, BM(s->evtchn_mask));
+ sync_set_bit(evtchn, BM(s->evtchn_pending));
if (!masked)
unmask_evtchn(evtchn);
@@ -1536,8 +1553,8 @@ static int retrigger_dynirq(struct irq_data *data)
if (VALID_EVTCHN(evtchn)) {
int masked;
- masked = sync_test_and_set_bit(evtchn, sh->evtchn_mask);
- sync_set_bit(evtchn, sh->evtchn_pending);
+ masked = sync_test_and_set_bit(evtchn, BM(sh->evtchn_mask));
+ sync_set_bit(evtchn, BM(sh->evtchn_pending));
if (!masked)
unmask_evtchn(evtchn);
ret = 1;
diff --git a/include/xen/interface/xen.h b/include/xen/interface/xen.h
index 5d6c272..a9075df 100644
--- a/include/xen/interface/xen.h
+++ b/include/xen/interface/xen.h
@@ -285,7 +285,7 @@ DEFINE_GUEST_HANDLE_STRUCT(multicall_entry);
* Event channel endpoints per domain:
* 1024 if a long is 32 bits; 4096 if a long is 64 bits.
*/
-#define NR_EVENT_CHANNELS (sizeof(unsigned long) * sizeof(unsigned long) * 64)
+#define NR_EVENT_CHANNELS (sizeof(xen_ulong_t) * sizeof(xen_ulong_t) * 64)
struct vcpu_time_info {
/*
@@ -341,7 +341,7 @@ struct vcpu_info {
*/
uint8_t evtchn_upcall_pending;
uint8_t evtchn_upcall_mask;
- unsigned long evtchn_pending_sel;
+ xen_ulong_t evtchn_pending_sel;
struct arch_vcpu_info arch;
struct pvclock_vcpu_time_info time;
}; /* 64 bytes (x86) */
@@ -384,8 +384,8 @@ struct shared_info {
* per-vcpu selector word to be set. Each bit in the selector covers a
* 'C long' in the PENDING bitfield array.
*/
- unsigned long evtchn_pending[sizeof(unsigned long) * 8];
- unsigned long evtchn_mask[sizeof(unsigned long) * 8];
+ xen_ulong_t evtchn_pending[sizeof(xen_ulong_t) * 8];
+ xen_ulong_t evtchn_mask[sizeof(xen_ulong_t) * 8];
/*
* Wallclock time: updated only by control software. Guests should base
--
1.7.2.5
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH LINUX v4] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-02-19 17:29 ` [PATCH LINUX v4] " Ian Campbell
@ 2013-02-19 18:12 ` Stefano Stabellini
2013-02-19 18:43 ` Konrad Rzeszutek Wilk
2013-02-20 2:07 ` Konrad Rzeszutek Wilk
1 sibling, 1 reply; 36+ messages in thread
From: Stefano Stabellini @ 2013-02-19 18:12 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 19 Feb 2013, Ian Campbell wrote:
> On ARM we want these to be the same size on 32- and 64-bit.
>
> This is an ABI change on ARM. X86 does not change.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Jan Beulich <JBeulich@suse.com>
> Cc: Keir (Xen.org) <keir@xen.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: xen-devel at lists.xen.org
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
> Changes since V3
> s/read_evtchn_pending_sel/xchg_xen_ulong/ in a comment.
> Changes since V2
> Add comments about the correct bitops to use, and on the ordering/barrier
> requirements on xchg_xen_ulong.
> Changes since V1
> use find_first_set not __ffs
> fix some more unsigned long -> xen_ulong_t
> use more generic xchg_xen_ulong instead of ...read_evtchn...
still doesn't apply to 3.8
> do {
> - unsigned long pending_words;
> + xen_ulong_t pending_words;
>
> vcpu_info->evtchn_upcall_pending = 0;
>
> if (__this_cpu_inc_return(xed_nesting_count) - 1)
> goto out;
>
> -#ifndef CONFIG_X86 /* No need for a barrier -- XCHG is a barrier on x86. */
> - /* Clear master flag /before/ clearing selector flag. */
> - wmb();
> -#endif
> - pending_words = xchg(&vcpu_info->evtchn_pending_sel, 0);
> + /*
> + * Master flag must be /before/ clearing selector
> + * flag. xchg_xen_ulong must contain an appropriate
> + * barrier.
> + */
Master flag must be *cleared* ...
> + pending_words = xchg_xen_ulong(&vcpu_info->evtchn_pending_sel, 0);
>
> start_word_idx = __this_cpu_read(current_word_idx);
> start_bit_idx = __this_cpu_read(current_bit_idx);
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH LINUX v4] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-02-19 18:12 ` Stefano Stabellini
@ 2013-02-19 18:43 ` Konrad Rzeszutek Wilk
0 siblings, 0 replies; 36+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-02-19 18:43 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Feb 19, 2013 at 06:12:35PM +0000, Stefano Stabellini wrote:
> On Tue, 19 Feb 2013, Ian Campbell wrote:
> > On ARM we want these to be the same size on 32- and 64-bit.
> >
> > This is an ABI change on ARM. X86 does not change.
> >
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > Cc: Jan Beulich <JBeulich@suse.com>
> > Cc: Keir (Xen.org) <keir@xen.org>
> > Cc: Tim Deegan <tim@xen.org>
> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > Cc: linux-arm-kernel at lists.infradead.org
> > Cc: xen-devel at lists.xen.org
> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > ---
> > Changes since V3
> > s/read_evtchn_pending_sel/xchg_xen_ulong/ in a comment.
> > Changes since V2
> > Add comments about the correct bitops to use, and on the ordering/barrier
> > requirements on xchg_xen_ulong.
> > Changes since V1
> > use find_first_set not __ffs
> > fix some more unsigned long -> xen_ulong_t
> > use more generic xchg_xen_ulong instead of ...read_evtchn...
>
> still doesn't apply to 3.8
Weird. It applied to my tree (stable/for-linus-3.9) without fuss.
>
>
> > do {
> > - unsigned long pending_words;
> > + xen_ulong_t pending_words;
> >
> > vcpu_info->evtchn_upcall_pending = 0;
> >
> > if (__this_cpu_inc_return(xed_nesting_count) - 1)
> > goto out;
> >
> > -#ifndef CONFIG_X86 /* No need for a barrier -- XCHG is a barrier on x86. */
> > - /* Clear master flag /before/ clearing selector flag. */
> > - wmb();
> > -#endif
> > - pending_words = xchg(&vcpu_info->evtchn_pending_sel, 0);
> > + /*
> > + * Master flag must be /before/ clearing selector
> > + * flag. xchg_xen_ulong must contain an appropriate
> > + * barrier.
> > + */
>
> Master flag must be *cleared* ...
>
> > + pending_words = xchg_xen_ulong(&vcpu_info->evtchn_pending_sel, 0);
> >
> > start_word_idx = __this_cpu_read(current_word_idx);
> > start_bit_idx = __this_cpu_read(current_bit_idx);
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH LINUX v4] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-02-19 17:29 ` [PATCH LINUX v4] " Ian Campbell
2013-02-19 18:12 ` Stefano Stabellini
@ 2013-02-20 2:07 ` Konrad Rzeszutek Wilk
2013-02-20 3:09 ` Konrad Rzeszutek Wilk
1 sibling, 1 reply; 36+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-02-20 2:07 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Feb 19, 2013 at 05:29:11PM +0000, Ian Campbell wrote:
> On ARM we want these to be the same size on 32- and 64-bit.
>
> This is an ABI change on ARM. X86 does not change.
Hehe. You didn't test this, did you :-)
It hangs bootup on X86.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Jan Beulich <JBeulich@suse.com>
> Cc: Keir (Xen.org) <keir@xen.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: xen-devel at lists.xen.org
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
> Changes since V3
> s/read_evtchn_pending_sel/xchg_xen_ulong/ in a comment.
> Changes since V2
> Add comments about the correct bitops to use, and on the ordering/barrier
> requirements on xchg_xen_ulong.
> Changes since V1
> use find_first_set not __ffs
> fix some more unsigned long -> xen_ulong_t
> use more generic xchg_xen_ulong instead of ...read_evtchn...
> ---
> arch/arm/include/asm/xen/events.h | 22 +++++++
> arch/x86/include/asm/xen/events.h | 3 +
> drivers/xen/events.c | 113 +++++++++++++++++++++----------------
> include/xen/interface/xen.h | 8 +-
> 4 files changed, 94 insertions(+), 52 deletions(-)
>
> diff --git a/arch/arm/include/asm/xen/events.h b/arch/arm/include/asm/xen/events.h
> index 94b4e90..5c27696 100644
> --- a/arch/arm/include/asm/xen/events.h
> +++ b/arch/arm/include/asm/xen/events.h
> @@ -15,4 +15,26 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
> return raw_irqs_disabled_flags(regs->ARM_cpsr);
> }
>
> +/*
> + * We cannot use xchg because it does not support 8-byte
> + * values. However it is safe to use {ldr,dtd}exd directly because all
> + * platforms which Xen can run on support those instructions.
> + */
> +static inline xen_ulong_t xchg_xen_ulong(xen_ulong_t *ptr, xen_ulong_t val)
> +{
> + xen_ulong_t oldval;
> + unsigned int tmp;
> +
> + wmb();
> + asm volatile("@ xchg_xen_ulong\n"
> + "1: ldrexd %0, %H0, [%3]\n"
> + " strexd %1, %2, %H2, [%3]\n"
> + " teq %1, #0\n"
> + " bne 1b"
> + : "=&r" (oldval), "=&r" (tmp)
> + : "r" (val), "r" (ptr)
> + : "memory", "cc");
> + return oldval;
> +}
> +
> #endif /* _ASM_ARM_XEN_EVENTS_H */
> diff --git a/arch/x86/include/asm/xen/events.h b/arch/x86/include/asm/xen/events.h
> index cc146d5..ca842f2 100644
> --- a/arch/x86/include/asm/xen/events.h
> +++ b/arch/x86/include/asm/xen/events.h
> @@ -16,4 +16,7 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
> return raw_irqs_disabled_flags(regs->flags);
> }
>
> +/* No need for a barrier -- XCHG is a barrier on x86. */
> +#define xchg_xen_ulong(ptr, val) xchg((ptr), (val))
> +
> #endif /* _ASM_X86_XEN_EVENTS_H */
> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> index 0be4df3..b8d84f5 100644
> --- a/drivers/xen/events.c
> +++ b/drivers/xen/events.c
> @@ -80,6 +80,12 @@ enum xen_irq_type {
> };
>
> /*
> + * Make a bitmask (i.e. unsigned long *) of a xen_ulong_t
> + * array. Primarily to avoid long lines (hence the terse name).
> + */
> +#define BM(x) (unsigned long *)(x)
> +
> +/*
> * Packed IRQ information:
> * type - enum xen_irq_type
> * event channel - irq->event channel mapping
> @@ -120,7 +126,14 @@ static unsigned long *pirq_eoi_map;
> #endif
> static bool (*pirq_needs_eoi)(unsigned irq);
>
> -static DEFINE_PER_CPU(unsigned long [NR_EVENT_CHANNELS/BITS_PER_LONG],
> +/*
> + * Note sizeof(xen_ulong_t) can be more than sizeof(unsigned long). Be
> + * careful to only use bitops which allow for this (e.g test_bit and
> + * friends but not __ffs).
> + */
> +#define BITS_PER_EVTCHN_WORD (sizeof(xen_ulong_t)*8)
> +
> +static DEFINE_PER_CPU(xen_ulong_t [NR_EVENT_CHANNELS/BITS_PER_EVTCHN_WORD],
> cpu_evtchn_mask);
>
> /* Xen will never allocate port zero for any purpose. */
> @@ -294,9 +307,9 @@ static bool pirq_needs_eoi_flag(unsigned irq)
> return info->u.pirq.flags & PIRQ_NEEDS_EOI;
> }
>
> -static inline unsigned long active_evtchns(unsigned int cpu,
> - struct shared_info *sh,
> - unsigned int idx)
> +static inline xen_ulong_t active_evtchns(unsigned int cpu,
> + struct shared_info *sh,
> + unsigned int idx)
> {
> return sh->evtchn_pending[idx] &
> per_cpu(cpu_evtchn_mask, cpu)[idx] &
> @@ -312,8 +325,8 @@ static void bind_evtchn_to_cpu(unsigned int chn, unsigned int cpu)
> cpumask_copy(irq_to_desc(irq)->irq_data.affinity, cpumask_of(cpu));
> #endif
>
> - clear_bit(chn, per_cpu(cpu_evtchn_mask, cpu_from_irq(irq)));
> - set_bit(chn, per_cpu(cpu_evtchn_mask, cpu));
> + clear_bit(chn, BM(per_cpu(cpu_evtchn_mask, cpu_from_irq(irq))));
> + set_bit(chn, BM(per_cpu(cpu_evtchn_mask, cpu)));
>
> info_for_irq(irq)->cpu = cpu;
> }
> @@ -339,19 +352,19 @@ static void init_evtchn_cpu_bindings(void)
> static inline void clear_evtchn(int port)
> {
> struct shared_info *s = HYPERVISOR_shared_info;
> - sync_clear_bit(port, &s->evtchn_pending[0]);
> + sync_clear_bit(port, BM(&s->evtchn_pending[0]));
> }
>
> static inline void set_evtchn(int port)
> {
> struct shared_info *s = HYPERVISOR_shared_info;
> - sync_set_bit(port, &s->evtchn_pending[0]);
> + sync_set_bit(port, BM(&s->evtchn_pending[0]));
> }
>
> static inline int test_evtchn(int port)
> {
> struct shared_info *s = HYPERVISOR_shared_info;
> - return sync_test_bit(port, &s->evtchn_pending[0]);
> + return sync_test_bit(port, BM(&s->evtchn_pending[0]));
> }
>
>
> @@ -375,7 +388,7 @@ EXPORT_SYMBOL_GPL(notify_remote_via_irq);
> static void mask_evtchn(int port)
> {
> struct shared_info *s = HYPERVISOR_shared_info;
> - sync_set_bit(port, &s->evtchn_mask[0]);
> + sync_set_bit(port, BM(&s->evtchn_mask[0]));
> }
>
> static void unmask_evtchn(int port)
> @@ -389,7 +402,7 @@ static void unmask_evtchn(int port)
> if (unlikely((cpu != cpu_from_evtchn(port))))
> do_hypercall = 1;
> else
> - evtchn_pending = sync_test_bit(port, &s->evtchn_pending[0]);
> + evtchn_pending = sync_test_bit(port, BM(&s->evtchn_pending[0]));
>
> if (unlikely(evtchn_pending && xen_hvm_domain()))
> do_hypercall = 1;
> @@ -403,7 +416,7 @@ static void unmask_evtchn(int port)
> } else {
> struct vcpu_info *vcpu_info = __this_cpu_read(xen_vcpu);
>
> - sync_clear_bit(port, &s->evtchn_mask[0]);
> + sync_clear_bit(port, BM(&s->evtchn_mask[0]));
>
> /*
> * The following is basically the equivalent of
> @@ -411,8 +424,8 @@ static void unmask_evtchn(int port)
> * the interrupt edge' if the channel is masked.
> */
> if (evtchn_pending &&
> - !sync_test_and_set_bit(port / BITS_PER_LONG,
> - &vcpu_info->evtchn_pending_sel))
> + !sync_test_and_set_bit(port / BITS_PER_EVTCHN_WORD,
> + BM(&vcpu_info->evtchn_pending_sel)))
> vcpu_info->evtchn_upcall_pending = 1;
> }
>
> @@ -1189,7 +1202,7 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
> {
> struct shared_info *sh = HYPERVISOR_shared_info;
> int cpu = smp_processor_id();
> - unsigned long *cpu_evtchn = per_cpu(cpu_evtchn_mask, cpu);
> + xen_ulong_t *cpu_evtchn = per_cpu(cpu_evtchn_mask, cpu);
> int i;
> unsigned long flags;
> static DEFINE_SPINLOCK(debug_lock);
> @@ -1205,7 +1218,7 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
> pending = (get_irq_regs() && i == cpu)
> ? xen_irqs_disabled(get_irq_regs())
> : v->evtchn_upcall_mask;
> - printk("%d: masked=%d pending=%d event_sel %0*lx\n ", i,
> + printk("%d: masked=%d pending=%d event_sel %0*"PRI_xen_ulong"\n ", i,
> pending, v->evtchn_upcall_pending,
> (int)(sizeof(v->evtchn_pending_sel)*2),
> v->evtchn_pending_sel);
> @@ -1214,49 +1227,52 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
>
> printk("\npending:\n ");
> for (i = ARRAY_SIZE(sh->evtchn_pending)-1; i >= 0; i--)
> - printk("%0*lx%s", (int)sizeof(sh->evtchn_pending[0])*2,
> + printk("%0*"PRI_xen_ulong"%s",
> + (int)sizeof(sh->evtchn_pending[0])*2,
> sh->evtchn_pending[i],
> i % 8 == 0 ? "\n " : " ");
> printk("\nglobal mask:\n ");
> for (i = ARRAY_SIZE(sh->evtchn_mask)-1; i >= 0; i--)
> - printk("%0*lx%s",
> + printk("%0*"PRI_xen_ulong"%s",
> (int)(sizeof(sh->evtchn_mask[0])*2),
> sh->evtchn_mask[i],
> i % 8 == 0 ? "\n " : " ");
>
> printk("\nglobally unmasked:\n ");
> for (i = ARRAY_SIZE(sh->evtchn_mask)-1; i >= 0; i--)
> - printk("%0*lx%s", (int)(sizeof(sh->evtchn_mask[0])*2),
> + printk("%0*"PRI_xen_ulong"%s",
> + (int)(sizeof(sh->evtchn_mask[0])*2),
> sh->evtchn_pending[i] & ~sh->evtchn_mask[i],
> i % 8 == 0 ? "\n " : " ");
>
> printk("\nlocal cpu%d mask:\n ", cpu);
> - for (i = (NR_EVENT_CHANNELS/BITS_PER_LONG)-1; i >= 0; i--)
> - printk("%0*lx%s", (int)(sizeof(cpu_evtchn[0])*2),
> + for (i = (NR_EVENT_CHANNELS/BITS_PER_EVTCHN_WORD)-1; i >= 0; i--)
> + printk("%0*"PRI_xen_ulong"%s", (int)(sizeof(cpu_evtchn[0])*2),
> cpu_evtchn[i],
> i % 8 == 0 ? "\n " : " ");
>
> printk("\nlocally unmasked:\n ");
> for (i = ARRAY_SIZE(sh->evtchn_mask)-1; i >= 0; i--) {
> - unsigned long pending = sh->evtchn_pending[i]
> + xen_ulong_t pending = sh->evtchn_pending[i]
> & ~sh->evtchn_mask[i]
> & cpu_evtchn[i];
> - printk("%0*lx%s", (int)(sizeof(sh->evtchn_mask[0])*2),
> + printk("%0*"PRI_xen_ulong"%s",
> + (int)(sizeof(sh->evtchn_mask[0])*2),
> pending, i % 8 == 0 ? "\n " : " ");
> }
>
> printk("\npending list:\n");
> for (i = 0; i < NR_EVENT_CHANNELS; i++) {
> - if (sync_test_bit(i, sh->evtchn_pending)) {
> - int word_idx = i / BITS_PER_LONG;
> + if (sync_test_bit(i, BM(sh->evtchn_pending))) {
> + int word_idx = i / BITS_PER_EVTCHN_WORD;
> printk(" %d: event %d -> irq %d%s%s%s\n",
> cpu_from_evtchn(i), i,
> evtchn_to_irq[i],
> - sync_test_bit(word_idx, &v->evtchn_pending_sel)
> + sync_test_bit(word_idx, BM(&v->evtchn_pending_sel))
> ? "" : " l2-clear",
> - !sync_test_bit(i, sh->evtchn_mask)
> + !sync_test_bit(i, BM(sh->evtchn_mask))
> ? "" : " globally-masked",
> - sync_test_bit(i, cpu_evtchn)
> + sync_test_bit(i, BM(cpu_evtchn))
> ? "" : " locally-masked");
> }
> }
> @@ -1273,7 +1289,7 @@ static DEFINE_PER_CPU(unsigned int, current_bit_idx);
> /*
> * Mask out the i least significant bits of w
> */
> -#define MASK_LSBS(w, i) (w & ((~0UL) << i))
> +#define MASK_LSBS(w, i) (w & ((~((xen_ulong_t)0UL)) << i))
>
> /*
> * Search the CPUs pending events bitmasks. For each one found, map
> @@ -1295,18 +1311,19 @@ static void __xen_evtchn_do_upcall(void)
> unsigned count;
>
> do {
> - unsigned long pending_words;
> + xen_ulong_t pending_words;
>
> vcpu_info->evtchn_upcall_pending = 0;
>
> if (__this_cpu_inc_return(xed_nesting_count) - 1)
> goto out;
>
> -#ifndef CONFIG_X86 /* No need for a barrier -- XCHG is a barrier on x86. */
> - /* Clear master flag /before/ clearing selector flag. */
> - wmb();
> -#endif
> - pending_words = xchg(&vcpu_info->evtchn_pending_sel, 0);
> + /*
> + * Master flag must be /before/ clearing selector
> + * flag. xchg_xen_ulong must contain an appropriate
> + * barrier.
> + */
> + pending_words = xchg_xen_ulong(&vcpu_info->evtchn_pending_sel, 0);
>
> start_word_idx = __this_cpu_read(current_word_idx);
> start_bit_idx = __this_cpu_read(current_bit_idx);
> @@ -1314,8 +1331,8 @@ static void __xen_evtchn_do_upcall(void)
> word_idx = start_word_idx;
>
> for (i = 0; pending_words != 0; i++) {
> - unsigned long pending_bits;
> - unsigned long words;
> + xen_ulong_t pending_bits;
> + xen_ulong_t words;
>
> words = MASK_LSBS(pending_words, word_idx);
>
> @@ -1327,7 +1344,7 @@ static void __xen_evtchn_do_upcall(void)
> bit_idx = 0;
> continue;
> }
> - word_idx = __ffs(words);
> + word_idx = find_first_bit(BM(&words), sizeof(words));
>
> pending_bits = active_evtchns(cpu, s, word_idx);
> bit_idx = 0; /* usually scan entire word from start */
> @@ -1342,7 +1359,7 @@ static void __xen_evtchn_do_upcall(void)
> }
>
> do {
> - unsigned long bits;
> + xen_ulong_t bits;
> int port, irq;
> struct irq_desc *desc;
>
> @@ -1352,10 +1369,10 @@ static void __xen_evtchn_do_upcall(void)
> if (bits == 0)
> break;
>
> - bit_idx = __ffs(bits);
> + bit_idx = find_first_bit(BM(&bits), sizeof(bits));
>
> /* Process port. */
> - port = (word_idx * BITS_PER_LONG) + bit_idx;
> + port = (word_idx * BITS_PER_EVTCHN_WORD) + bit_idx;
> irq = evtchn_to_irq[port];
>
> if (irq != -1) {
> @@ -1364,12 +1381,12 @@ static void __xen_evtchn_do_upcall(void)
> generic_handle_irq_desc(irq, desc);
> }
>
> - bit_idx = (bit_idx + 1) % BITS_PER_LONG;
> + bit_idx = (bit_idx + 1) % BITS_PER_EVTCHN_WORD;
>
> /* Next caller starts at last processed + 1 */
> __this_cpu_write(current_word_idx,
> bit_idx ? word_idx :
> - (word_idx+1) % BITS_PER_LONG);
> + (word_idx+1) % BITS_PER_EVTCHN_WORD);
> __this_cpu_write(current_bit_idx, bit_idx);
> } while (bit_idx != 0);
>
> @@ -1377,7 +1394,7 @@ static void __xen_evtchn_do_upcall(void)
> if ((word_idx != start_word_idx) || (i != 0))
> pending_words &= ~(1UL << word_idx);
>
> - word_idx = (word_idx + 1) % BITS_PER_LONG;
> + word_idx = (word_idx + 1) % BITS_PER_EVTCHN_WORD;
> }
>
> BUG_ON(!irqs_disabled());
> @@ -1487,8 +1504,8 @@ int resend_irq_on_evtchn(unsigned int irq)
> if (!VALID_EVTCHN(evtchn))
> return 1;
>
> - masked = sync_test_and_set_bit(evtchn, s->evtchn_mask);
> - sync_set_bit(evtchn, s->evtchn_pending);
> + masked = sync_test_and_set_bit(evtchn, BM(s->evtchn_mask));
> + sync_set_bit(evtchn, BM(s->evtchn_pending));
> if (!masked)
> unmask_evtchn(evtchn);
>
> @@ -1536,8 +1553,8 @@ static int retrigger_dynirq(struct irq_data *data)
> if (VALID_EVTCHN(evtchn)) {
> int masked;
>
> - masked = sync_test_and_set_bit(evtchn, sh->evtchn_mask);
> - sync_set_bit(evtchn, sh->evtchn_pending);
> + masked = sync_test_and_set_bit(evtchn, BM(sh->evtchn_mask));
> + sync_set_bit(evtchn, BM(sh->evtchn_pending));
> if (!masked)
> unmask_evtchn(evtchn);
> ret = 1;
> diff --git a/include/xen/interface/xen.h b/include/xen/interface/xen.h
> index 5d6c272..a9075df 100644
> --- a/include/xen/interface/xen.h
> +++ b/include/xen/interface/xen.h
> @@ -285,7 +285,7 @@ DEFINE_GUEST_HANDLE_STRUCT(multicall_entry);
> * Event channel endpoints per domain:
> * 1024 if a long is 32 bits; 4096 if a long is 64 bits.
> */
> -#define NR_EVENT_CHANNELS (sizeof(unsigned long) * sizeof(unsigned long) * 64)
> +#define NR_EVENT_CHANNELS (sizeof(xen_ulong_t) * sizeof(xen_ulong_t) * 64)
>
> struct vcpu_time_info {
> /*
> @@ -341,7 +341,7 @@ struct vcpu_info {
> */
> uint8_t evtchn_upcall_pending;
> uint8_t evtchn_upcall_mask;
> - unsigned long evtchn_pending_sel;
> + xen_ulong_t evtchn_pending_sel;
> struct arch_vcpu_info arch;
> struct pvclock_vcpu_time_info time;
> }; /* 64 bytes (x86) */
> @@ -384,8 +384,8 @@ struct shared_info {
> * per-vcpu selector word to be set. Each bit in the selector covers a
> * 'C long' in the PENDING bitfield array.
> */
> - unsigned long evtchn_pending[sizeof(unsigned long) * 8];
> - unsigned long evtchn_mask[sizeof(unsigned long) * 8];
> + xen_ulong_t evtchn_pending[sizeof(xen_ulong_t) * 8];
> + xen_ulong_t evtchn_mask[sizeof(xen_ulong_t) * 8];
>
> /*
> * Wallclock time: updated only by control software. Guests should base
> --
> 1.7.2.5
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH LINUX v4] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-02-20 2:07 ` Konrad Rzeszutek Wilk
@ 2013-02-20 3:09 ` Konrad Rzeszutek Wilk
2013-02-20 9:13 ` Ian Campbell
0 siblings, 1 reply; 36+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-02-20 3:09 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Feb 19, 2013 at 09:07:27PM -0500, Konrad Rzeszutek Wilk wrote:
> On Tue, Feb 19, 2013 at 05:29:11PM +0000, Ian Campbell wrote:
> > On ARM we want these to be the same size on 32- and 64-bit.
> >
> > This is an ABI change on ARM. X86 does not change.
>
> Hehe. You didn't test this, did you :-)
>
> It hangs bootup on X86.
Ah wait. The version that hangs was the first posted - I hadn't
tested this one yet. I will wait until later on Wednesday to do that.
> >
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > Cc: Jan Beulich <JBeulich@suse.com>
> > Cc: Keir (Xen.org) <keir@xen.org>
> > Cc: Tim Deegan <tim@xen.org>
> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > Cc: linux-arm-kernel at lists.infradead.org
> > Cc: xen-devel at lists.xen.org
> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > ---
> > Changes since V3
> > s/read_evtchn_pending_sel/xchg_xen_ulong/ in a comment.
> > Changes since V2
> > Add comments about the correct bitops to use, and on the ordering/barrier
> > requirements on xchg_xen_ulong.
> > Changes since V1
> > use find_first_set not __ffs
> > fix some more unsigned long -> xen_ulong_t
> > use more generic xchg_xen_ulong instead of ...read_evtchn...
> > ---
> > arch/arm/include/asm/xen/events.h | 22 +++++++
> > arch/x86/include/asm/xen/events.h | 3 +
> > drivers/xen/events.c | 113 +++++++++++++++++++++----------------
> > include/xen/interface/xen.h | 8 +-
> > 4 files changed, 94 insertions(+), 52 deletions(-)
> >
> > diff --git a/arch/arm/include/asm/xen/events.h b/arch/arm/include/asm/xen/events.h
> > index 94b4e90..5c27696 100644
> > --- a/arch/arm/include/asm/xen/events.h
> > +++ b/arch/arm/include/asm/xen/events.h
> > @@ -15,4 +15,26 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
> > return raw_irqs_disabled_flags(regs->ARM_cpsr);
> > }
> >
> > +/*
> > + * We cannot use xchg because it does not support 8-byte
> > + * values. However it is safe to use {ldr,dtd}exd directly because all
> > + * platforms which Xen can run on support those instructions.
> > + */
> > +static inline xen_ulong_t xchg_xen_ulong(xen_ulong_t *ptr, xen_ulong_t val)
> > +{
> > + xen_ulong_t oldval;
> > + unsigned int tmp;
> > +
> > + wmb();
> > + asm volatile("@ xchg_xen_ulong\n"
> > + "1: ldrexd %0, %H0, [%3]\n"
> > + " strexd %1, %2, %H2, [%3]\n"
> > + " teq %1, #0\n"
> > + " bne 1b"
> > + : "=&r" (oldval), "=&r" (tmp)
> > + : "r" (val), "r" (ptr)
> > + : "memory", "cc");
> > + return oldval;
> > +}
> > +
> > #endif /* _ASM_ARM_XEN_EVENTS_H */
> > diff --git a/arch/x86/include/asm/xen/events.h b/arch/x86/include/asm/xen/events.h
> > index cc146d5..ca842f2 100644
> > --- a/arch/x86/include/asm/xen/events.h
> > +++ b/arch/x86/include/asm/xen/events.h
> > @@ -16,4 +16,7 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
> > return raw_irqs_disabled_flags(regs->flags);
> > }
> >
> > +/* No need for a barrier -- XCHG is a barrier on x86. */
> > +#define xchg_xen_ulong(ptr, val) xchg((ptr), (val))
> > +
> > #endif /* _ASM_X86_XEN_EVENTS_H */
> > diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> > index 0be4df3..b8d84f5 100644
> > --- a/drivers/xen/events.c
> > +++ b/drivers/xen/events.c
> > @@ -80,6 +80,12 @@ enum xen_irq_type {
> > };
> >
> > /*
> > + * Make a bitmask (i.e. unsigned long *) of a xen_ulong_t
> > + * array. Primarily to avoid long lines (hence the terse name).
> > + */
> > +#define BM(x) (unsigned long *)(x)
> > +
> > +/*
> > * Packed IRQ information:
> > * type - enum xen_irq_type
> > * event channel - irq->event channel mapping
> > @@ -120,7 +126,14 @@ static unsigned long *pirq_eoi_map;
> > #endif
> > static bool (*pirq_needs_eoi)(unsigned irq);
> >
> > -static DEFINE_PER_CPU(unsigned long [NR_EVENT_CHANNELS/BITS_PER_LONG],
> > +/*
> > + * Note sizeof(xen_ulong_t) can be more than sizeof(unsigned long). Be
> > + * careful to only use bitops which allow for this (e.g test_bit and
> > + * friends but not __ffs).
> > + */
> > +#define BITS_PER_EVTCHN_WORD (sizeof(xen_ulong_t)*8)
> > +
> > +static DEFINE_PER_CPU(xen_ulong_t [NR_EVENT_CHANNELS/BITS_PER_EVTCHN_WORD],
> > cpu_evtchn_mask);
> >
> > /* Xen will never allocate port zero for any purpose. */
> > @@ -294,9 +307,9 @@ static bool pirq_needs_eoi_flag(unsigned irq)
> > return info->u.pirq.flags & PIRQ_NEEDS_EOI;
> > }
> >
> > -static inline unsigned long active_evtchns(unsigned int cpu,
> > - struct shared_info *sh,
> > - unsigned int idx)
> > +static inline xen_ulong_t active_evtchns(unsigned int cpu,
> > + struct shared_info *sh,
> > + unsigned int idx)
> > {
> > return sh->evtchn_pending[idx] &
> > per_cpu(cpu_evtchn_mask, cpu)[idx] &
> > @@ -312,8 +325,8 @@ static void bind_evtchn_to_cpu(unsigned int chn, unsigned int cpu)
> > cpumask_copy(irq_to_desc(irq)->irq_data.affinity, cpumask_of(cpu));
> > #endif
> >
> > - clear_bit(chn, per_cpu(cpu_evtchn_mask, cpu_from_irq(irq)));
> > - set_bit(chn, per_cpu(cpu_evtchn_mask, cpu));
> > + clear_bit(chn, BM(per_cpu(cpu_evtchn_mask, cpu_from_irq(irq))));
> > + set_bit(chn, BM(per_cpu(cpu_evtchn_mask, cpu)));
> >
> > info_for_irq(irq)->cpu = cpu;
> > }
> > @@ -339,19 +352,19 @@ static void init_evtchn_cpu_bindings(void)
> > static inline void clear_evtchn(int port)
> > {
> > struct shared_info *s = HYPERVISOR_shared_info;
> > - sync_clear_bit(port, &s->evtchn_pending[0]);
> > + sync_clear_bit(port, BM(&s->evtchn_pending[0]));
> > }
> >
> > static inline void set_evtchn(int port)
> > {
> > struct shared_info *s = HYPERVISOR_shared_info;
> > - sync_set_bit(port, &s->evtchn_pending[0]);
> > + sync_set_bit(port, BM(&s->evtchn_pending[0]));
> > }
> >
> > static inline int test_evtchn(int port)
> > {
> > struct shared_info *s = HYPERVISOR_shared_info;
> > - return sync_test_bit(port, &s->evtchn_pending[0]);
> > + return sync_test_bit(port, BM(&s->evtchn_pending[0]));
> > }
> >
> >
> > @@ -375,7 +388,7 @@ EXPORT_SYMBOL_GPL(notify_remote_via_irq);
> > static void mask_evtchn(int port)
> > {
> > struct shared_info *s = HYPERVISOR_shared_info;
> > - sync_set_bit(port, &s->evtchn_mask[0]);
> > + sync_set_bit(port, BM(&s->evtchn_mask[0]));
> > }
> >
> > static void unmask_evtchn(int port)
> > @@ -389,7 +402,7 @@ static void unmask_evtchn(int port)
> > if (unlikely((cpu != cpu_from_evtchn(port))))
> > do_hypercall = 1;
> > else
> > - evtchn_pending = sync_test_bit(port, &s->evtchn_pending[0]);
> > + evtchn_pending = sync_test_bit(port, BM(&s->evtchn_pending[0]));
> >
> > if (unlikely(evtchn_pending && xen_hvm_domain()))
> > do_hypercall = 1;
> > @@ -403,7 +416,7 @@ static void unmask_evtchn(int port)
> > } else {
> > struct vcpu_info *vcpu_info = __this_cpu_read(xen_vcpu);
> >
> > - sync_clear_bit(port, &s->evtchn_mask[0]);
> > + sync_clear_bit(port, BM(&s->evtchn_mask[0]));
> >
> > /*
> > * The following is basically the equivalent of
> > @@ -411,8 +424,8 @@ static void unmask_evtchn(int port)
> > * the interrupt edge' if the channel is masked.
> > */
> > if (evtchn_pending &&
> > - !sync_test_and_set_bit(port / BITS_PER_LONG,
> > - &vcpu_info->evtchn_pending_sel))
> > + !sync_test_and_set_bit(port / BITS_PER_EVTCHN_WORD,
> > + BM(&vcpu_info->evtchn_pending_sel)))
> > vcpu_info->evtchn_upcall_pending = 1;
> > }
> >
> > @@ -1189,7 +1202,7 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
> > {
> > struct shared_info *sh = HYPERVISOR_shared_info;
> > int cpu = smp_processor_id();
> > - unsigned long *cpu_evtchn = per_cpu(cpu_evtchn_mask, cpu);
> > + xen_ulong_t *cpu_evtchn = per_cpu(cpu_evtchn_mask, cpu);
> > int i;
> > unsigned long flags;
> > static DEFINE_SPINLOCK(debug_lock);
> > @@ -1205,7 +1218,7 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
> > pending = (get_irq_regs() && i == cpu)
> > ? xen_irqs_disabled(get_irq_regs())
> > : v->evtchn_upcall_mask;
> > - printk("%d: masked=%d pending=%d event_sel %0*lx\n ", i,
> > + printk("%d: masked=%d pending=%d event_sel %0*"PRI_xen_ulong"\n ", i,
> > pending, v->evtchn_upcall_pending,
> > (int)(sizeof(v->evtchn_pending_sel)*2),
> > v->evtchn_pending_sel);
> > @@ -1214,49 +1227,52 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
> >
> > printk("\npending:\n ");
> > for (i = ARRAY_SIZE(sh->evtchn_pending)-1; i >= 0; i--)
> > - printk("%0*lx%s", (int)sizeof(sh->evtchn_pending[0])*2,
> > + printk("%0*"PRI_xen_ulong"%s",
> > + (int)sizeof(sh->evtchn_pending[0])*2,
> > sh->evtchn_pending[i],
> > i % 8 == 0 ? "\n " : " ");
> > printk("\nglobal mask:\n ");
> > for (i = ARRAY_SIZE(sh->evtchn_mask)-1; i >= 0; i--)
> > - printk("%0*lx%s",
> > + printk("%0*"PRI_xen_ulong"%s",
> > (int)(sizeof(sh->evtchn_mask[0])*2),
> > sh->evtchn_mask[i],
> > i % 8 == 0 ? "\n " : " ");
> >
> > printk("\nglobally unmasked:\n ");
> > for (i = ARRAY_SIZE(sh->evtchn_mask)-1; i >= 0; i--)
> > - printk("%0*lx%s", (int)(sizeof(sh->evtchn_mask[0])*2),
> > + printk("%0*"PRI_xen_ulong"%s",
> > + (int)(sizeof(sh->evtchn_mask[0])*2),
> > sh->evtchn_pending[i] & ~sh->evtchn_mask[i],
> > i % 8 == 0 ? "\n " : " ");
> >
> > printk("\nlocal cpu%d mask:\n ", cpu);
> > - for (i = (NR_EVENT_CHANNELS/BITS_PER_LONG)-1; i >= 0; i--)
> > - printk("%0*lx%s", (int)(sizeof(cpu_evtchn[0])*2),
> > + for (i = (NR_EVENT_CHANNELS/BITS_PER_EVTCHN_WORD)-1; i >= 0; i--)
> > + printk("%0*"PRI_xen_ulong"%s", (int)(sizeof(cpu_evtchn[0])*2),
> > cpu_evtchn[i],
> > i % 8 == 0 ? "\n " : " ");
> >
> > printk("\nlocally unmasked:\n ");
> > for (i = ARRAY_SIZE(sh->evtchn_mask)-1; i >= 0; i--) {
> > - unsigned long pending = sh->evtchn_pending[i]
> > + xen_ulong_t pending = sh->evtchn_pending[i]
> > & ~sh->evtchn_mask[i]
> > & cpu_evtchn[i];
> > - printk("%0*lx%s", (int)(sizeof(sh->evtchn_mask[0])*2),
> > + printk("%0*"PRI_xen_ulong"%s",
> > + (int)(sizeof(sh->evtchn_mask[0])*2),
> > pending, i % 8 == 0 ? "\n " : " ");
> > }
> >
> > printk("\npending list:\n");
> > for (i = 0; i < NR_EVENT_CHANNELS; i++) {
> > - if (sync_test_bit(i, sh->evtchn_pending)) {
> > - int word_idx = i / BITS_PER_LONG;
> > + if (sync_test_bit(i, BM(sh->evtchn_pending))) {
> > + int word_idx = i / BITS_PER_EVTCHN_WORD;
> > printk(" %d: event %d -> irq %d%s%s%s\n",
> > cpu_from_evtchn(i), i,
> > evtchn_to_irq[i],
> > - sync_test_bit(word_idx, &v->evtchn_pending_sel)
> > + sync_test_bit(word_idx, BM(&v->evtchn_pending_sel))
> > ? "" : " l2-clear",
> > - !sync_test_bit(i, sh->evtchn_mask)
> > + !sync_test_bit(i, BM(sh->evtchn_mask))
> > ? "" : " globally-masked",
> > - sync_test_bit(i, cpu_evtchn)
> > + sync_test_bit(i, BM(cpu_evtchn))
> > ? "" : " locally-masked");
> > }
> > }
> > @@ -1273,7 +1289,7 @@ static DEFINE_PER_CPU(unsigned int, current_bit_idx);
> > /*
> > * Mask out the i least significant bits of w
> > */
> > -#define MASK_LSBS(w, i) (w & ((~0UL) << i))
> > +#define MASK_LSBS(w, i) (w & ((~((xen_ulong_t)0UL)) << i))
> >
> > /*
> > * Search the CPUs pending events bitmasks. For each one found, map
> > @@ -1295,18 +1311,19 @@ static void __xen_evtchn_do_upcall(void)
> > unsigned count;
> >
> > do {
> > - unsigned long pending_words;
> > + xen_ulong_t pending_words;
> >
> > vcpu_info->evtchn_upcall_pending = 0;
> >
> > if (__this_cpu_inc_return(xed_nesting_count) - 1)
> > goto out;
> >
> > -#ifndef CONFIG_X86 /* No need for a barrier -- XCHG is a barrier on x86. */
> > - /* Clear master flag /before/ clearing selector flag. */
> > - wmb();
> > -#endif
> > - pending_words = xchg(&vcpu_info->evtchn_pending_sel, 0);
> > + /*
> > + * Master flag must be /before/ clearing selector
> > + * flag. xchg_xen_ulong must contain an appropriate
> > + * barrier.
> > + */
> > + pending_words = xchg_xen_ulong(&vcpu_info->evtchn_pending_sel, 0);
> >
> > start_word_idx = __this_cpu_read(current_word_idx);
> > start_bit_idx = __this_cpu_read(current_bit_idx);
> > @@ -1314,8 +1331,8 @@ static void __xen_evtchn_do_upcall(void)
> > word_idx = start_word_idx;
> >
> > for (i = 0; pending_words != 0; i++) {
> > - unsigned long pending_bits;
> > - unsigned long words;
> > + xen_ulong_t pending_bits;
> > + xen_ulong_t words;
> >
> > words = MASK_LSBS(pending_words, word_idx);
> >
> > @@ -1327,7 +1344,7 @@ static void __xen_evtchn_do_upcall(void)
> > bit_idx = 0;
> > continue;
> > }
> > - word_idx = __ffs(words);
> > + word_idx = find_first_bit(BM(&words), sizeof(words));
> >
> > pending_bits = active_evtchns(cpu, s, word_idx);
> > bit_idx = 0; /* usually scan entire word from start */
> > @@ -1342,7 +1359,7 @@ static void __xen_evtchn_do_upcall(void)
> > }
> >
> > do {
> > - unsigned long bits;
> > + xen_ulong_t bits;
> > int port, irq;
> > struct irq_desc *desc;
> >
> > @@ -1352,10 +1369,10 @@ static void __xen_evtchn_do_upcall(void)
> > if (bits == 0)
> > break;
> >
> > - bit_idx = __ffs(bits);
> > + bit_idx = find_first_bit(BM(&bits), sizeof(bits));
> >
> > /* Process port. */
> > - port = (word_idx * BITS_PER_LONG) + bit_idx;
> > + port = (word_idx * BITS_PER_EVTCHN_WORD) + bit_idx;
> > irq = evtchn_to_irq[port];
> >
> > if (irq != -1) {
> > @@ -1364,12 +1381,12 @@ static void __xen_evtchn_do_upcall(void)
> > generic_handle_irq_desc(irq, desc);
> > }
> >
> > - bit_idx = (bit_idx + 1) % BITS_PER_LONG;
> > + bit_idx = (bit_idx + 1) % BITS_PER_EVTCHN_WORD;
> >
> > /* Next caller starts at last processed + 1 */
> > __this_cpu_write(current_word_idx,
> > bit_idx ? word_idx :
> > - (word_idx+1) % BITS_PER_LONG);
> > + (word_idx+1) % BITS_PER_EVTCHN_WORD);
> > __this_cpu_write(current_bit_idx, bit_idx);
> > } while (bit_idx != 0);
> >
> > @@ -1377,7 +1394,7 @@ static void __xen_evtchn_do_upcall(void)
> > if ((word_idx != start_word_idx) || (i != 0))
> > pending_words &= ~(1UL << word_idx);
> >
> > - word_idx = (word_idx + 1) % BITS_PER_LONG;
> > + word_idx = (word_idx + 1) % BITS_PER_EVTCHN_WORD;
> > }
> >
> > BUG_ON(!irqs_disabled());
> > @@ -1487,8 +1504,8 @@ int resend_irq_on_evtchn(unsigned int irq)
> > if (!VALID_EVTCHN(evtchn))
> > return 1;
> >
> > - masked = sync_test_and_set_bit(evtchn, s->evtchn_mask);
> > - sync_set_bit(evtchn, s->evtchn_pending);
> > + masked = sync_test_and_set_bit(evtchn, BM(s->evtchn_mask));
> > + sync_set_bit(evtchn, BM(s->evtchn_pending));
> > if (!masked)
> > unmask_evtchn(evtchn);
> >
> > @@ -1536,8 +1553,8 @@ static int retrigger_dynirq(struct irq_data *data)
> > if (VALID_EVTCHN(evtchn)) {
> > int masked;
> >
> > - masked = sync_test_and_set_bit(evtchn, sh->evtchn_mask);
> > - sync_set_bit(evtchn, sh->evtchn_pending);
> > + masked = sync_test_and_set_bit(evtchn, BM(sh->evtchn_mask));
> > + sync_set_bit(evtchn, BM(sh->evtchn_pending));
> > if (!masked)
> > unmask_evtchn(evtchn);
> > ret = 1;
> > diff --git a/include/xen/interface/xen.h b/include/xen/interface/xen.h
> > index 5d6c272..a9075df 100644
> > --- a/include/xen/interface/xen.h
> > +++ b/include/xen/interface/xen.h
> > @@ -285,7 +285,7 @@ DEFINE_GUEST_HANDLE_STRUCT(multicall_entry);
> > * Event channel endpoints per domain:
> > * 1024 if a long is 32 bits; 4096 if a long is 64 bits.
> > */
> > -#define NR_EVENT_CHANNELS (sizeof(unsigned long) * sizeof(unsigned long) * 64)
> > +#define NR_EVENT_CHANNELS (sizeof(xen_ulong_t) * sizeof(xen_ulong_t) * 64)
> >
> > struct vcpu_time_info {
> > /*
> > @@ -341,7 +341,7 @@ struct vcpu_info {
> > */
> > uint8_t evtchn_upcall_pending;
> > uint8_t evtchn_upcall_mask;
> > - unsigned long evtchn_pending_sel;
> > + xen_ulong_t evtchn_pending_sel;
> > struct arch_vcpu_info arch;
> > struct pvclock_vcpu_time_info time;
> > }; /* 64 bytes (x86) */
> > @@ -384,8 +384,8 @@ struct shared_info {
> > * per-vcpu selector word to be set. Each bit in the selector covers a
> > * 'C long' in the PENDING bitfield array.
> > */
> > - unsigned long evtchn_pending[sizeof(unsigned long) * 8];
> > - unsigned long evtchn_mask[sizeof(unsigned long) * 8];
> > + xen_ulong_t evtchn_pending[sizeof(xen_ulong_t) * 8];
> > + xen_ulong_t evtchn_mask[sizeof(xen_ulong_t) * 8];
> >
> > /*
> > * Wallclock time: updated only by control software. Guests should base
> > --
> > 1.7.2.5
> >
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH LINUX v4] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-02-20 3:09 ` Konrad Rzeszutek Wilk
@ 2013-02-20 9:13 ` Ian Campbell
0 siblings, 0 replies; 36+ messages in thread
From: Ian Campbell @ 2013-02-20 9:13 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, 2013-02-20 at 03:09 +0000, Konrad Rzeszutek Wilk wrote:
> On Tue, Feb 19, 2013 at 09:07:27PM -0500, Konrad Rzeszutek Wilk wrote:
> > On Tue, Feb 19, 2013 at 05:29:11PM +0000, Ian Campbell wrote:
> > > On ARM we want these to be the same size on 32- and 64-bit.
> > >
> > > This is an ABI change on ARM. X86 does not change.
> >
> > Hehe. You didn't test this, did you :-)
> >
> > It hangs bootup on X86.
>
> Ah wait. The version that hangs was the first posted - I hadn't
> tested this one yet. I will wait until later on Wednesday to do that.
I don't know what I tested yesterday but it surely wasn't this patch
because it hangs for me too this morning :-/
Sorry about this, will investigate...
Ian.
>
>
> > >
> > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > > Cc: Jan Beulich <JBeulich@suse.com>
> > > Cc: Keir (Xen.org) <keir@xen.org>
> > > Cc: Tim Deegan <tim@xen.org>
> > > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > Cc: linux-arm-kernel at lists.infradead.org
> > > Cc: xen-devel at lists.xen.org
> > > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > > ---
> > > Changes since V3
> > > s/read_evtchn_pending_sel/xchg_xen_ulong/ in a comment.
> > > Changes since V2
> > > Add comments about the correct bitops to use, and on the ordering/barrier
> > > requirements on xchg_xen_ulong.
> > > Changes since V1
> > > use find_first_set not __ffs
> > > fix some more unsigned long -> xen_ulong_t
> > > use more generic xchg_xen_ulong instead of ...read_evtchn...
> > > ---
> > > arch/arm/include/asm/xen/events.h | 22 +++++++
> > > arch/x86/include/asm/xen/events.h | 3 +
> > > drivers/xen/events.c | 113 +++++++++++++++++++++----------------
> > > include/xen/interface/xen.h | 8 +-
> > > 4 files changed, 94 insertions(+), 52 deletions(-)
> > >
> > > diff --git a/arch/arm/include/asm/xen/events.h b/arch/arm/include/asm/xen/events.h
> > > index 94b4e90..5c27696 100644
> > > --- a/arch/arm/include/asm/xen/events.h
> > > +++ b/arch/arm/include/asm/xen/events.h
> > > @@ -15,4 +15,26 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
> > > return raw_irqs_disabled_flags(regs->ARM_cpsr);
> > > }
> > >
> > > +/*
> > > + * We cannot use xchg because it does not support 8-byte
> > > + * values. However it is safe to use {ldr,dtd}exd directly because all
> > > + * platforms which Xen can run on support those instructions.
> > > + */
> > > +static inline xen_ulong_t xchg_xen_ulong(xen_ulong_t *ptr, xen_ulong_t val)
> > > +{
> > > + xen_ulong_t oldval;
> > > + unsigned int tmp;
> > > +
> > > + wmb();
> > > + asm volatile("@ xchg_xen_ulong\n"
> > > + "1: ldrexd %0, %H0, [%3]\n"
> > > + " strexd %1, %2, %H2, [%3]\n"
> > > + " teq %1, #0\n"
> > > + " bne 1b"
> > > + : "=&r" (oldval), "=&r" (tmp)
> > > + : "r" (val), "r" (ptr)
> > > + : "memory", "cc");
> > > + return oldval;
> > > +}
> > > +
> > > #endif /* _ASM_ARM_XEN_EVENTS_H */
> > > diff --git a/arch/x86/include/asm/xen/events.h b/arch/x86/include/asm/xen/events.h
> > > index cc146d5..ca842f2 100644
> > > --- a/arch/x86/include/asm/xen/events.h
> > > +++ b/arch/x86/include/asm/xen/events.h
> > > @@ -16,4 +16,7 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
> > > return raw_irqs_disabled_flags(regs->flags);
> > > }
> > >
> > > +/* No need for a barrier -- XCHG is a barrier on x86. */
> > > +#define xchg_xen_ulong(ptr, val) xchg((ptr), (val))
> > > +
> > > #endif /* _ASM_X86_XEN_EVENTS_H */
> > > diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> > > index 0be4df3..b8d84f5 100644
> > > --- a/drivers/xen/events.c
> > > +++ b/drivers/xen/events.c
> > > @@ -80,6 +80,12 @@ enum xen_irq_type {
> > > };
> > >
> > > /*
> > > + * Make a bitmask (i.e. unsigned long *) of a xen_ulong_t
> > > + * array. Primarily to avoid long lines (hence the terse name).
> > > + */
> > > +#define BM(x) (unsigned long *)(x)
> > > +
> > > +/*
> > > * Packed IRQ information:
> > > * type - enum xen_irq_type
> > > * event channel - irq->event channel mapping
> > > @@ -120,7 +126,14 @@ static unsigned long *pirq_eoi_map;
> > > #endif
> > > static bool (*pirq_needs_eoi)(unsigned irq);
> > >
> > > -static DEFINE_PER_CPU(unsigned long [NR_EVENT_CHANNELS/BITS_PER_LONG],
> > > +/*
> > > + * Note sizeof(xen_ulong_t) can be more than sizeof(unsigned long). Be
> > > + * careful to only use bitops which allow for this (e.g test_bit and
> > > + * friends but not __ffs).
> > > + */
> > > +#define BITS_PER_EVTCHN_WORD (sizeof(xen_ulong_t)*8)
> > > +
> > > +static DEFINE_PER_CPU(xen_ulong_t [NR_EVENT_CHANNELS/BITS_PER_EVTCHN_WORD],
> > > cpu_evtchn_mask);
> > >
> > > /* Xen will never allocate port zero for any purpose. */
> > > @@ -294,9 +307,9 @@ static bool pirq_needs_eoi_flag(unsigned irq)
> > > return info->u.pirq.flags & PIRQ_NEEDS_EOI;
> > > }
> > >
> > > -static inline unsigned long active_evtchns(unsigned int cpu,
> > > - struct shared_info *sh,
> > > - unsigned int idx)
> > > +static inline xen_ulong_t active_evtchns(unsigned int cpu,
> > > + struct shared_info *sh,
> > > + unsigned int idx)
> > > {
> > > return sh->evtchn_pending[idx] &
> > > per_cpu(cpu_evtchn_mask, cpu)[idx] &
> > > @@ -312,8 +325,8 @@ static void bind_evtchn_to_cpu(unsigned int chn, unsigned int cpu)
> > > cpumask_copy(irq_to_desc(irq)->irq_data.affinity, cpumask_of(cpu));
> > > #endif
> > >
> > > - clear_bit(chn, per_cpu(cpu_evtchn_mask, cpu_from_irq(irq)));
> > > - set_bit(chn, per_cpu(cpu_evtchn_mask, cpu));
> > > + clear_bit(chn, BM(per_cpu(cpu_evtchn_mask, cpu_from_irq(irq))));
> > > + set_bit(chn, BM(per_cpu(cpu_evtchn_mask, cpu)));
> > >
> > > info_for_irq(irq)->cpu = cpu;
> > > }
> > > @@ -339,19 +352,19 @@ static void init_evtchn_cpu_bindings(void)
> > > static inline void clear_evtchn(int port)
> > > {
> > > struct shared_info *s = HYPERVISOR_shared_info;
> > > - sync_clear_bit(port, &s->evtchn_pending[0]);
> > > + sync_clear_bit(port, BM(&s->evtchn_pending[0]));
> > > }
> > >
> > > static inline void set_evtchn(int port)
> > > {
> > > struct shared_info *s = HYPERVISOR_shared_info;
> > > - sync_set_bit(port, &s->evtchn_pending[0]);
> > > + sync_set_bit(port, BM(&s->evtchn_pending[0]));
> > > }
> > >
> > > static inline int test_evtchn(int port)
> > > {
> > > struct shared_info *s = HYPERVISOR_shared_info;
> > > - return sync_test_bit(port, &s->evtchn_pending[0]);
> > > + return sync_test_bit(port, BM(&s->evtchn_pending[0]));
> > > }
> > >
> > >
> > > @@ -375,7 +388,7 @@ EXPORT_SYMBOL_GPL(notify_remote_via_irq);
> > > static void mask_evtchn(int port)
> > > {
> > > struct shared_info *s = HYPERVISOR_shared_info;
> > > - sync_set_bit(port, &s->evtchn_mask[0]);
> > > + sync_set_bit(port, BM(&s->evtchn_mask[0]));
> > > }
> > >
> > > static void unmask_evtchn(int port)
> > > @@ -389,7 +402,7 @@ static void unmask_evtchn(int port)
> > > if (unlikely((cpu != cpu_from_evtchn(port))))
> > > do_hypercall = 1;
> > > else
> > > - evtchn_pending = sync_test_bit(port, &s->evtchn_pending[0]);
> > > + evtchn_pending = sync_test_bit(port, BM(&s->evtchn_pending[0]));
> > >
> > > if (unlikely(evtchn_pending && xen_hvm_domain()))
> > > do_hypercall = 1;
> > > @@ -403,7 +416,7 @@ static void unmask_evtchn(int port)
> > > } else {
> > > struct vcpu_info *vcpu_info = __this_cpu_read(xen_vcpu);
> > >
> > > - sync_clear_bit(port, &s->evtchn_mask[0]);
> > > + sync_clear_bit(port, BM(&s->evtchn_mask[0]));
> > >
> > > /*
> > > * The following is basically the equivalent of
> > > @@ -411,8 +424,8 @@ static void unmask_evtchn(int port)
> > > * the interrupt edge' if the channel is masked.
> > > */
> > > if (evtchn_pending &&
> > > - !sync_test_and_set_bit(port / BITS_PER_LONG,
> > > - &vcpu_info->evtchn_pending_sel))
> > > + !sync_test_and_set_bit(port / BITS_PER_EVTCHN_WORD,
> > > + BM(&vcpu_info->evtchn_pending_sel)))
> > > vcpu_info->evtchn_upcall_pending = 1;
> > > }
> > >
> > > @@ -1189,7 +1202,7 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
> > > {
> > > struct shared_info *sh = HYPERVISOR_shared_info;
> > > int cpu = smp_processor_id();
> > > - unsigned long *cpu_evtchn = per_cpu(cpu_evtchn_mask, cpu);
> > > + xen_ulong_t *cpu_evtchn = per_cpu(cpu_evtchn_mask, cpu);
> > > int i;
> > > unsigned long flags;
> > > static DEFINE_SPINLOCK(debug_lock);
> > > @@ -1205,7 +1218,7 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
> > > pending = (get_irq_regs() && i == cpu)
> > > ? xen_irqs_disabled(get_irq_regs())
> > > : v->evtchn_upcall_mask;
> > > - printk("%d: masked=%d pending=%d event_sel %0*lx\n ", i,
> > > + printk("%d: masked=%d pending=%d event_sel %0*"PRI_xen_ulong"\n ", i,
> > > pending, v->evtchn_upcall_pending,
> > > (int)(sizeof(v->evtchn_pending_sel)*2),
> > > v->evtchn_pending_sel);
> > > @@ -1214,49 +1227,52 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
> > >
> > > printk("\npending:\n ");
> > > for (i = ARRAY_SIZE(sh->evtchn_pending)-1; i >= 0; i--)
> > > - printk("%0*lx%s", (int)sizeof(sh->evtchn_pending[0])*2,
> > > + printk("%0*"PRI_xen_ulong"%s",
> > > + (int)sizeof(sh->evtchn_pending[0])*2,
> > > sh->evtchn_pending[i],
> > > i % 8 == 0 ? "\n " : " ");
> > > printk("\nglobal mask:\n ");
> > > for (i = ARRAY_SIZE(sh->evtchn_mask)-1; i >= 0; i--)
> > > - printk("%0*lx%s",
> > > + printk("%0*"PRI_xen_ulong"%s",
> > > (int)(sizeof(sh->evtchn_mask[0])*2),
> > > sh->evtchn_mask[i],
> > > i % 8 == 0 ? "\n " : " ");
> > >
> > > printk("\nglobally unmasked:\n ");
> > > for (i = ARRAY_SIZE(sh->evtchn_mask)-1; i >= 0; i--)
> > > - printk("%0*lx%s", (int)(sizeof(sh->evtchn_mask[0])*2),
> > > + printk("%0*"PRI_xen_ulong"%s",
> > > + (int)(sizeof(sh->evtchn_mask[0])*2),
> > > sh->evtchn_pending[i] & ~sh->evtchn_mask[i],
> > > i % 8 == 0 ? "\n " : " ");
> > >
> > > printk("\nlocal cpu%d mask:\n ", cpu);
> > > - for (i = (NR_EVENT_CHANNELS/BITS_PER_LONG)-1; i >= 0; i--)
> > > - printk("%0*lx%s", (int)(sizeof(cpu_evtchn[0])*2),
> > > + for (i = (NR_EVENT_CHANNELS/BITS_PER_EVTCHN_WORD)-1; i >= 0; i--)
> > > + printk("%0*"PRI_xen_ulong"%s", (int)(sizeof(cpu_evtchn[0])*2),
> > > cpu_evtchn[i],
> > > i % 8 == 0 ? "\n " : " ");
> > >
> > > printk("\nlocally unmasked:\n ");
> > > for (i = ARRAY_SIZE(sh->evtchn_mask)-1; i >= 0; i--) {
> > > - unsigned long pending = sh->evtchn_pending[i]
> > > + xen_ulong_t pending = sh->evtchn_pending[i]
> > > & ~sh->evtchn_mask[i]
> > > & cpu_evtchn[i];
> > > - printk("%0*lx%s", (int)(sizeof(sh->evtchn_mask[0])*2),
> > > + printk("%0*"PRI_xen_ulong"%s",
> > > + (int)(sizeof(sh->evtchn_mask[0])*2),
> > > pending, i % 8 == 0 ? "\n " : " ");
> > > }
> > >
> > > printk("\npending list:\n");
> > > for (i = 0; i < NR_EVENT_CHANNELS; i++) {
> > > - if (sync_test_bit(i, sh->evtchn_pending)) {
> > > - int word_idx = i / BITS_PER_LONG;
> > > + if (sync_test_bit(i, BM(sh->evtchn_pending))) {
> > > + int word_idx = i / BITS_PER_EVTCHN_WORD;
> > > printk(" %d: event %d -> irq %d%s%s%s\n",
> > > cpu_from_evtchn(i), i,
> > > evtchn_to_irq[i],
> > > - sync_test_bit(word_idx, &v->evtchn_pending_sel)
> > > + sync_test_bit(word_idx, BM(&v->evtchn_pending_sel))
> > > ? "" : " l2-clear",
> > > - !sync_test_bit(i, sh->evtchn_mask)
> > > + !sync_test_bit(i, BM(sh->evtchn_mask))
> > > ? "" : " globally-masked",
> > > - sync_test_bit(i, cpu_evtchn)
> > > + sync_test_bit(i, BM(cpu_evtchn))
> > > ? "" : " locally-masked");
> > > }
> > > }
> > > @@ -1273,7 +1289,7 @@ static DEFINE_PER_CPU(unsigned int, current_bit_idx);
> > > /*
> > > * Mask out the i least significant bits of w
> > > */
> > > -#define MASK_LSBS(w, i) (w & ((~0UL) << i))
> > > +#define MASK_LSBS(w, i) (w & ((~((xen_ulong_t)0UL)) << i))
> > >
> > > /*
> > > * Search the CPUs pending events bitmasks. For each one found, map
> > > @@ -1295,18 +1311,19 @@ static void __xen_evtchn_do_upcall(void)
> > > unsigned count;
> > >
> > > do {
> > > - unsigned long pending_words;
> > > + xen_ulong_t pending_words;
> > >
> > > vcpu_info->evtchn_upcall_pending = 0;
> > >
> > > if (__this_cpu_inc_return(xed_nesting_count) - 1)
> > > goto out;
> > >
> > > -#ifndef CONFIG_X86 /* No need for a barrier -- XCHG is a barrier on x86. */
> > > - /* Clear master flag /before/ clearing selector flag. */
> > > - wmb();
> > > -#endif
> > > - pending_words = xchg(&vcpu_info->evtchn_pending_sel, 0);
> > > + /*
> > > + * Master flag must be /before/ clearing selector
> > > + * flag. xchg_xen_ulong must contain an appropriate
> > > + * barrier.
> > > + */
> > > + pending_words = xchg_xen_ulong(&vcpu_info->evtchn_pending_sel, 0);
> > >
> > > start_word_idx = __this_cpu_read(current_word_idx);
> > > start_bit_idx = __this_cpu_read(current_bit_idx);
> > > @@ -1314,8 +1331,8 @@ static void __xen_evtchn_do_upcall(void)
> > > word_idx = start_word_idx;
> > >
> > > for (i = 0; pending_words != 0; i++) {
> > > - unsigned long pending_bits;
> > > - unsigned long words;
> > > + xen_ulong_t pending_bits;
> > > + xen_ulong_t words;
> > >
> > > words = MASK_LSBS(pending_words, word_idx);
> > >
> > > @@ -1327,7 +1344,7 @@ static void __xen_evtchn_do_upcall(void)
> > > bit_idx = 0;
> > > continue;
> > > }
> > > - word_idx = __ffs(words);
> > > + word_idx = find_first_bit(BM(&words), sizeof(words));
> > >
> > > pending_bits = active_evtchns(cpu, s, word_idx);
> > > bit_idx = 0; /* usually scan entire word from start */
> > > @@ -1342,7 +1359,7 @@ static void __xen_evtchn_do_upcall(void)
> > > }
> > >
> > > do {
> > > - unsigned long bits;
> > > + xen_ulong_t bits;
> > > int port, irq;
> > > struct irq_desc *desc;
> > >
> > > @@ -1352,10 +1369,10 @@ static void __xen_evtchn_do_upcall(void)
> > > if (bits == 0)
> > > break;
> > >
> > > - bit_idx = __ffs(bits);
> > > + bit_idx = find_first_bit(BM(&bits), sizeof(bits));
> > >
> > > /* Process port. */
> > > - port = (word_idx * BITS_PER_LONG) + bit_idx;
> > > + port = (word_idx * BITS_PER_EVTCHN_WORD) + bit_idx;
> > > irq = evtchn_to_irq[port];
> > >
> > > if (irq != -1) {
> > > @@ -1364,12 +1381,12 @@ static void __xen_evtchn_do_upcall(void)
> > > generic_handle_irq_desc(irq, desc);
> > > }
> > >
> > > - bit_idx = (bit_idx + 1) % BITS_PER_LONG;
> > > + bit_idx = (bit_idx + 1) % BITS_PER_EVTCHN_WORD;
> > >
> > > /* Next caller starts at last processed + 1 */
> > > __this_cpu_write(current_word_idx,
> > > bit_idx ? word_idx :
> > > - (word_idx+1) % BITS_PER_LONG);
> > > + (word_idx+1) % BITS_PER_EVTCHN_WORD);
> > > __this_cpu_write(current_bit_idx, bit_idx);
> > > } while (bit_idx != 0);
> > >
> > > @@ -1377,7 +1394,7 @@ static void __xen_evtchn_do_upcall(void)
> > > if ((word_idx != start_word_idx) || (i != 0))
> > > pending_words &= ~(1UL << word_idx);
> > >
> > > - word_idx = (word_idx + 1) % BITS_PER_LONG;
> > > + word_idx = (word_idx + 1) % BITS_PER_EVTCHN_WORD;
> > > }
> > >
> > > BUG_ON(!irqs_disabled());
> > > @@ -1487,8 +1504,8 @@ int resend_irq_on_evtchn(unsigned int irq)
> > > if (!VALID_EVTCHN(evtchn))
> > > return 1;
> > >
> > > - masked = sync_test_and_set_bit(evtchn, s->evtchn_mask);
> > > - sync_set_bit(evtchn, s->evtchn_pending);
> > > + masked = sync_test_and_set_bit(evtchn, BM(s->evtchn_mask));
> > > + sync_set_bit(evtchn, BM(s->evtchn_pending));
> > > if (!masked)
> > > unmask_evtchn(evtchn);
> > >
> > > @@ -1536,8 +1553,8 @@ static int retrigger_dynirq(struct irq_data *data)
> > > if (VALID_EVTCHN(evtchn)) {
> > > int masked;
> > >
> > > - masked = sync_test_and_set_bit(evtchn, sh->evtchn_mask);
> > > - sync_set_bit(evtchn, sh->evtchn_pending);
> > > + masked = sync_test_and_set_bit(evtchn, BM(sh->evtchn_mask));
> > > + sync_set_bit(evtchn, BM(sh->evtchn_pending));
> > > if (!masked)
> > > unmask_evtchn(evtchn);
> > > ret = 1;
> > > diff --git a/include/xen/interface/xen.h b/include/xen/interface/xen.h
> > > index 5d6c272..a9075df 100644
> > > --- a/include/xen/interface/xen.h
> > > +++ b/include/xen/interface/xen.h
> > > @@ -285,7 +285,7 @@ DEFINE_GUEST_HANDLE_STRUCT(multicall_entry);
> > > * Event channel endpoints per domain:
> > > * 1024 if a long is 32 bits; 4096 if a long is 64 bits.
> > > */
> > > -#define NR_EVENT_CHANNELS (sizeof(unsigned long) * sizeof(unsigned long) * 64)
> > > +#define NR_EVENT_CHANNELS (sizeof(xen_ulong_t) * sizeof(xen_ulong_t) * 64)
> > >
> > > struct vcpu_time_info {
> > > /*
> > > @@ -341,7 +341,7 @@ struct vcpu_info {
> > > */
> > > uint8_t evtchn_upcall_pending;
> > > uint8_t evtchn_upcall_mask;
> > > - unsigned long evtchn_pending_sel;
> > > + xen_ulong_t evtchn_pending_sel;
> > > struct arch_vcpu_info arch;
> > > struct pvclock_vcpu_time_info time;
> > > }; /* 64 bytes (x86) */
> > > @@ -384,8 +384,8 @@ struct shared_info {
> > > * per-vcpu selector word to be set. Each bit in the selector covers a
> > > * 'C long' in the PENDING bitfield array.
> > > */
> > > - unsigned long evtchn_pending[sizeof(unsigned long) * 8];
> > > - unsigned long evtchn_mask[sizeof(unsigned long) * 8];
> > > + xen_ulong_t evtchn_pending[sizeof(xen_ulong_t) * 8];
> > > + xen_ulong_t evtchn_mask[sizeof(xen_ulong_t) * 8];
> > >
> > > /*
> > > * Wallclock time: updated only by control software. Guests should base
> > > --
> > > 1.7.2.5
> > >
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH LINUX v5] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-02-19 14:48 [PATCH V2] xen: event channel arrays are xen_ulong_t and not unsigned long Ian Campbell
` (3 preceding siblings ...)
2013-02-19 17:29 ` [PATCH LINUX v4] " Ian Campbell
@ 2013-02-20 11:48 ` Ian Campbell
2013-02-21 17:11 ` Stefano Stabellini
2013-03-04 2:45 ` Rob Herring
4 siblings, 2 replies; 36+ messages in thread
From: Ian Campbell @ 2013-02-20 11:48 UTC (permalink / raw)
To: linux-arm-kernel
On ARM we want these to be the same size on 32- and 64-bit.
This is an ABI change on ARM. X86 does not change.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Jan Beulich <JBeulich@suse.com>
Cc: Keir (Xen.org) <keir@xen.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: linux-arm-kernel at lists.infradead.org
Cc: xen-devel at lists.xen.org
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
Changes since V4
Rebase onto v3.8
Fix wording of comment
Fix bitmask length passed to find_first_bit, need sizeof*8 for bits not just
sizeof. Use BITS_PER_EVTCHN_WORD and provide a convenience wrapper.
Changes since V3
s/read_evtchn_pending_sel/xchg_xen_ulong/ in a comment.
Changes since V2
Add comments about the correct bitops to use, and on the ordering/barrier
requirements on xchg_xen_ulong.
Changes since V1
use find_first_set not __ffs
fix some more unsigned long -> xen_ulong_t
use more generic xchg_xen_ulong instead of ...read_evtchn...
---
arch/arm/include/asm/xen/events.h | 22 +++++++
arch/x86/include/asm/xen/events.h | 3 +
drivers/xen/events.c | 115 +++++++++++++++++++++---------------
include/xen/interface/xen.h | 8 +-
4 files changed, 96 insertions(+), 52 deletions(-)
diff --git a/arch/arm/include/asm/xen/events.h b/arch/arm/include/asm/xen/events.h
index 94b4e90..5c27696 100644
--- a/arch/arm/include/asm/xen/events.h
+++ b/arch/arm/include/asm/xen/events.h
@@ -15,4 +15,26 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
return raw_irqs_disabled_flags(regs->ARM_cpsr);
}
+/*
+ * We cannot use xchg because it does not support 8-byte
+ * values. However it is safe to use {ldr,dtd}exd directly because all
+ * platforms which Xen can run on support those instructions.
+ */
+static inline xen_ulong_t xchg_xen_ulong(xen_ulong_t *ptr, xen_ulong_t val)
+{
+ xen_ulong_t oldval;
+ unsigned int tmp;
+
+ wmb();
+ asm volatile("@ xchg_xen_ulong\n"
+ "1: ldrexd %0, %H0, [%3]\n"
+ " strexd %1, %2, %H2, [%3]\n"
+ " teq %1, #0\n"
+ " bne 1b"
+ : "=&r" (oldval), "=&r" (tmp)
+ : "r" (val), "r" (ptr)
+ : "memory", "cc");
+ return oldval;
+}
+
#endif /* _ASM_ARM_XEN_EVENTS_H */
diff --git a/arch/x86/include/asm/xen/events.h b/arch/x86/include/asm/xen/events.h
index cc146d5..ca842f2 100644
--- a/arch/x86/include/asm/xen/events.h
+++ b/arch/x86/include/asm/xen/events.h
@@ -16,4 +16,7 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
return raw_irqs_disabled_flags(regs->flags);
}
+/* No need for a barrier -- XCHG is a barrier on x86. */
+#define xchg_xen_ulong(ptr, val) xchg((ptr), (val))
+
#endif /* _ASM_X86_XEN_EVENTS_H */
diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 74d77df..dfd62b5 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -120,7 +120,22 @@ static unsigned long *pirq_eoi_map;
#endif
static bool (*pirq_needs_eoi)(unsigned irq);
-static DEFINE_PER_CPU(unsigned long [NR_EVENT_CHANNELS/BITS_PER_LONG],
+/*
+ * Note sizeof(xen_ulong_t) can be more than sizeof(unsigned long). Be
+ * careful to only use bitops which allow for this (e.g
+ * test_bit/find_first_bit and friends but not __ffs) and to pass
+ * BITS_PER_EVTCHN_WORD as the bitmask length.
+ */
+#define BITS_PER_EVTCHN_WORD (sizeof(xen_ulong_t)*8)
+/*
+ * Make a bitmask (i.e. unsigned long *) of a xen_ulong_t
+ * array. Primarily to avoid long lines (hence the terse name).
+ */
+#define BM(x) (unsigned long *)(x)
+/* Find the first set bit in a evtchn mask */
+#define EVTCHN_FIRST_BIT(w) find_first_bit(BM(&(w)), BITS_PER_EVTCHN_WORD)
+
+static DEFINE_PER_CPU(xen_ulong_t [NR_EVENT_CHANNELS/BITS_PER_EVTCHN_WORD],
cpu_evtchn_mask);
/* Xen will never allocate port zero for any purpose. */
@@ -294,9 +309,9 @@ static bool pirq_needs_eoi_flag(unsigned irq)
return info->u.pirq.flags & PIRQ_NEEDS_EOI;
}
-static inline unsigned long active_evtchns(unsigned int cpu,
- struct shared_info *sh,
- unsigned int idx)
+static inline xen_ulong_t active_evtchns(unsigned int cpu,
+ struct shared_info *sh,
+ unsigned int idx)
{
return sh->evtchn_pending[idx] &
per_cpu(cpu_evtchn_mask, cpu)[idx] &
@@ -312,8 +327,8 @@ static void bind_evtchn_to_cpu(unsigned int chn, unsigned int cpu)
cpumask_copy(irq_to_desc(irq)->irq_data.affinity, cpumask_of(cpu));
#endif
- clear_bit(chn, per_cpu(cpu_evtchn_mask, cpu_from_irq(irq)));
- set_bit(chn, per_cpu(cpu_evtchn_mask, cpu));
+ clear_bit(chn, BM(per_cpu(cpu_evtchn_mask, cpu_from_irq(irq))));
+ set_bit(chn, BM(per_cpu(cpu_evtchn_mask, cpu)));
info_for_irq(irq)->cpu = cpu;
}
@@ -339,19 +354,19 @@ static void init_evtchn_cpu_bindings(void)
static inline void clear_evtchn(int port)
{
struct shared_info *s = HYPERVISOR_shared_info;
- sync_clear_bit(port, &s->evtchn_pending[0]);
+ sync_clear_bit(port, BM(&s->evtchn_pending[0]));
}
static inline void set_evtchn(int port)
{
struct shared_info *s = HYPERVISOR_shared_info;
- sync_set_bit(port, &s->evtchn_pending[0]);
+ sync_set_bit(port, BM(&s->evtchn_pending[0]));
}
static inline int test_evtchn(int port)
{
struct shared_info *s = HYPERVISOR_shared_info;
- return sync_test_bit(port, &s->evtchn_pending[0]);
+ return sync_test_bit(port, BM(&s->evtchn_pending[0]));
}
@@ -375,7 +390,7 @@ EXPORT_SYMBOL_GPL(notify_remote_via_irq);
static void mask_evtchn(int port)
{
struct shared_info *s = HYPERVISOR_shared_info;
- sync_set_bit(port, &s->evtchn_mask[0]);
+ sync_set_bit(port, BM(&s->evtchn_mask[0]));
}
static void unmask_evtchn(int port)
@@ -389,7 +404,7 @@ static void unmask_evtchn(int port)
if (unlikely((cpu != cpu_from_evtchn(port))))
do_hypercall = 1;
else
- evtchn_pending = sync_test_bit(port, &s->evtchn_pending[0]);
+ evtchn_pending = sync_test_bit(port, BM(&s->evtchn_pending[0]));
if (unlikely(evtchn_pending && xen_hvm_domain()))
do_hypercall = 1;
@@ -403,7 +418,7 @@ static void unmask_evtchn(int port)
} else {
struct vcpu_info *vcpu_info = __this_cpu_read(xen_vcpu);
- sync_clear_bit(port, &s->evtchn_mask[0]);
+ sync_clear_bit(port, BM(&s->evtchn_mask[0]));
/*
* The following is basically the equivalent of
@@ -411,8 +426,8 @@ static void unmask_evtchn(int port)
* the interrupt edge' if the channel is masked.
*/
if (evtchn_pending &&
- !sync_test_and_set_bit(port / BITS_PER_LONG,
- &vcpu_info->evtchn_pending_sel))
+ !sync_test_and_set_bit(port / BITS_PER_EVTCHN_WORD,
+ BM(&vcpu_info->evtchn_pending_sel)))
vcpu_info->evtchn_upcall_pending = 1;
}
@@ -1189,7 +1204,7 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
{
struct shared_info *sh = HYPERVISOR_shared_info;
int cpu = smp_processor_id();
- unsigned long *cpu_evtchn = per_cpu(cpu_evtchn_mask, cpu);
+ xen_ulong_t *cpu_evtchn = per_cpu(cpu_evtchn_mask, cpu);
int i;
unsigned long flags;
static DEFINE_SPINLOCK(debug_lock);
@@ -1205,7 +1220,7 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
pending = (get_irq_regs() && i == cpu)
? xen_irqs_disabled(get_irq_regs())
: v->evtchn_upcall_mask;
- printk("%d: masked=%d pending=%d event_sel %0*lx\n ", i,
+ printk("%d: masked=%d pending=%d event_sel %0*"PRI_xen_ulong"\n ", i,
pending, v->evtchn_upcall_pending,
(int)(sizeof(v->evtchn_pending_sel)*2),
v->evtchn_pending_sel);
@@ -1214,49 +1229,52 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
printk("\npending:\n ");
for (i = ARRAY_SIZE(sh->evtchn_pending)-1; i >= 0; i--)
- printk("%0*lx%s", (int)sizeof(sh->evtchn_pending[0])*2,
+ printk("%0*"PRI_xen_ulong"%s",
+ (int)sizeof(sh->evtchn_pending[0])*2,
sh->evtchn_pending[i],
i % 8 == 0 ? "\n " : " ");
printk("\nglobal mask:\n ");
for (i = ARRAY_SIZE(sh->evtchn_mask)-1; i >= 0; i--)
- printk("%0*lx%s",
+ printk("%0*"PRI_xen_ulong"%s",
(int)(sizeof(sh->evtchn_mask[0])*2),
sh->evtchn_mask[i],
i % 8 == 0 ? "\n " : " ");
printk("\nglobally unmasked:\n ");
for (i = ARRAY_SIZE(sh->evtchn_mask)-1; i >= 0; i--)
- printk("%0*lx%s", (int)(sizeof(sh->evtchn_mask[0])*2),
+ printk("%0*"PRI_xen_ulong"%s",
+ (int)(sizeof(sh->evtchn_mask[0])*2),
sh->evtchn_pending[i] & ~sh->evtchn_mask[i],
i % 8 == 0 ? "\n " : " ");
printk("\nlocal cpu%d mask:\n ", cpu);
- for (i = (NR_EVENT_CHANNELS/BITS_PER_LONG)-1; i >= 0; i--)
- printk("%0*lx%s", (int)(sizeof(cpu_evtchn[0])*2),
+ for (i = (NR_EVENT_CHANNELS/BITS_PER_EVTCHN_WORD)-1; i >= 0; i--)
+ printk("%0*"PRI_xen_ulong"%s", (int)(sizeof(cpu_evtchn[0])*2),
cpu_evtchn[i],
i % 8 == 0 ? "\n " : " ");
printk("\nlocally unmasked:\n ");
for (i = ARRAY_SIZE(sh->evtchn_mask)-1; i >= 0; i--) {
- unsigned long pending = sh->evtchn_pending[i]
+ xen_ulong_t pending = sh->evtchn_pending[i]
& ~sh->evtchn_mask[i]
& cpu_evtchn[i];
- printk("%0*lx%s", (int)(sizeof(sh->evtchn_mask[0])*2),
+ printk("%0*"PRI_xen_ulong"%s",
+ (int)(sizeof(sh->evtchn_mask[0])*2),
pending, i % 8 == 0 ? "\n " : " ");
}
printk("\npending list:\n");
for (i = 0; i < NR_EVENT_CHANNELS; i++) {
- if (sync_test_bit(i, sh->evtchn_pending)) {
- int word_idx = i / BITS_PER_LONG;
+ if (sync_test_bit(i, BM(sh->evtchn_pending))) {
+ int word_idx = i / BITS_PER_EVTCHN_WORD;
printk(" %d: event %d -> irq %d%s%s%s\n",
cpu_from_evtchn(i), i,
evtchn_to_irq[i],
- sync_test_bit(word_idx, &v->evtchn_pending_sel)
+ sync_test_bit(word_idx, BM(&v->evtchn_pending_sel))
? "" : " l2-clear",
- !sync_test_bit(i, sh->evtchn_mask)
+ !sync_test_bit(i, BM(sh->evtchn_mask))
? "" : " globally-masked",
- sync_test_bit(i, cpu_evtchn)
+ sync_test_bit(i, BM(cpu_evtchn))
? "" : " locally-masked");
}
}
@@ -1273,7 +1291,7 @@ static DEFINE_PER_CPU(unsigned int, current_bit_idx);
/*
* Mask out the i least significant bits of w
*/
-#define MASK_LSBS(w, i) (w & ((~0UL) << i))
+#define MASK_LSBS(w, i) (w & ((~((xen_ulong_t)0UL)) << i))
/*
* Search the CPUs pending events bitmasks. For each one found, map
@@ -1295,18 +1313,19 @@ static void __xen_evtchn_do_upcall(void)
unsigned count;
do {
- unsigned long pending_words;
+ xen_ulong_t pending_words;
vcpu_info->evtchn_upcall_pending = 0;
if (__this_cpu_inc_return(xed_nesting_count) - 1)
goto out;
-#ifndef CONFIG_X86 /* No need for a barrier -- XCHG is a barrier on x86. */
- /* Clear master flag /before/ clearing selector flag. */
- wmb();
-#endif
- pending_words = xchg(&vcpu_info->evtchn_pending_sel, 0);
+ /*
+ * Master flag must be cleared /before/ clearing
+ * selector flag. xchg_xen_ulong must contain an
+ * appropriate barrier.
+ */
+ pending_words = xchg_xen_ulong(&vcpu_info->evtchn_pending_sel, 0);
start_word_idx = __this_cpu_read(current_word_idx);
start_bit_idx = __this_cpu_read(current_bit_idx);
@@ -1314,8 +1333,8 @@ static void __xen_evtchn_do_upcall(void)
word_idx = start_word_idx;
for (i = 0; pending_words != 0; i++) {
- unsigned long pending_bits;
- unsigned long words;
+ xen_ulong_t pending_bits;
+ xen_ulong_t words;
words = MASK_LSBS(pending_words, word_idx);
@@ -1327,7 +1346,7 @@ static void __xen_evtchn_do_upcall(void)
bit_idx = 0;
continue;
}
- word_idx = __ffs(words);
+ word_idx = EVTCHN_FIRST_BIT(words);
pending_bits = active_evtchns(cpu, s, word_idx);
bit_idx = 0; /* usually scan entire word from start */
@@ -1342,7 +1361,7 @@ static void __xen_evtchn_do_upcall(void)
}
do {
- unsigned long bits;
+ xen_ulong_t bits;
int port, irq;
struct irq_desc *desc;
@@ -1352,10 +1371,10 @@ static void __xen_evtchn_do_upcall(void)
if (bits == 0)
break;
- bit_idx = __ffs(bits);
+ bit_idx = EVTCHN_FIRST_BIT(bits);
/* Process port. */
- port = (word_idx * BITS_PER_LONG) + bit_idx;
+ port = (word_idx * BITS_PER_EVTCHN_WORD) + bit_idx;
irq = evtchn_to_irq[port];
if (irq != -1) {
@@ -1364,12 +1383,12 @@ static void __xen_evtchn_do_upcall(void)
generic_handle_irq_desc(irq, desc);
}
- bit_idx = (bit_idx + 1) % BITS_PER_LONG;
+ bit_idx = (bit_idx + 1) % BITS_PER_EVTCHN_WORD;
/* Next caller starts at last processed + 1 */
__this_cpu_write(current_word_idx,
bit_idx ? word_idx :
- (word_idx+1) % BITS_PER_LONG);
+ (word_idx+1) % BITS_PER_EVTCHN_WORD);
__this_cpu_write(current_bit_idx, bit_idx);
} while (bit_idx != 0);
@@ -1377,7 +1396,7 @@ static void __xen_evtchn_do_upcall(void)
if ((word_idx != start_word_idx) || (i != 0))
pending_words &= ~(1UL << word_idx);
- word_idx = (word_idx + 1) % BITS_PER_LONG;
+ word_idx = (word_idx + 1) % BITS_PER_EVTCHN_WORD;
}
BUG_ON(!irqs_disabled());
@@ -1487,8 +1506,8 @@ int resend_irq_on_evtchn(unsigned int irq)
if (!VALID_EVTCHN(evtchn))
return 1;
- masked = sync_test_and_set_bit(evtchn, s->evtchn_mask);
- sync_set_bit(evtchn, s->evtchn_pending);
+ masked = sync_test_and_set_bit(evtchn, BM(s->evtchn_mask));
+ sync_set_bit(evtchn, BM(s->evtchn_pending));
if (!masked)
unmask_evtchn(evtchn);
@@ -1536,8 +1555,8 @@ static int retrigger_dynirq(struct irq_data *data)
if (VALID_EVTCHN(evtchn)) {
int masked;
- masked = sync_test_and_set_bit(evtchn, sh->evtchn_mask);
- sync_set_bit(evtchn, sh->evtchn_pending);
+ masked = sync_test_and_set_bit(evtchn, BM(sh->evtchn_mask));
+ sync_set_bit(evtchn, BM(sh->evtchn_pending));
if (!masked)
unmask_evtchn(evtchn);
ret = 1;
diff --git a/include/xen/interface/xen.h b/include/xen/interface/xen.h
index 5d6c272..a9075df 100644
--- a/include/xen/interface/xen.h
+++ b/include/xen/interface/xen.h
@@ -285,7 +285,7 @@ DEFINE_GUEST_HANDLE_STRUCT(multicall_entry);
* Event channel endpoints per domain:
* 1024 if a long is 32 bits; 4096 if a long is 64 bits.
*/
-#define NR_EVENT_CHANNELS (sizeof(unsigned long) * sizeof(unsigned long) * 64)
+#define NR_EVENT_CHANNELS (sizeof(xen_ulong_t) * sizeof(xen_ulong_t) * 64)
struct vcpu_time_info {
/*
@@ -341,7 +341,7 @@ struct vcpu_info {
*/
uint8_t evtchn_upcall_pending;
uint8_t evtchn_upcall_mask;
- unsigned long evtchn_pending_sel;
+ xen_ulong_t evtchn_pending_sel;
struct arch_vcpu_info arch;
struct pvclock_vcpu_time_info time;
}; /* 64 bytes (x86) */
@@ -384,8 +384,8 @@ struct shared_info {
* per-vcpu selector word to be set. Each bit in the selector covers a
* 'C long' in the PENDING bitfield array.
*/
- unsigned long evtchn_pending[sizeof(unsigned long) * 8];
- unsigned long evtchn_mask[sizeof(unsigned long) * 8];
+ xen_ulong_t evtchn_pending[sizeof(xen_ulong_t) * 8];
+ xen_ulong_t evtchn_mask[sizeof(xen_ulong_t) * 8];
/*
* Wallclock time: updated only by control software. Guests should base
--
1.7.2.5
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH LINUX v5] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-02-20 11:48 ` [PATCH LINUX v5] " Ian Campbell
@ 2013-02-21 17:11 ` Stefano Stabellini
2013-03-04 2:45 ` Rob Herring
1 sibling, 0 replies; 36+ messages in thread
From: Stefano Stabellini @ 2013-02-21 17:11 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, 20 Feb 2013, Ian Campbell wrote:
> On ARM we want these to be the same size on 32- and 64-bit.
>
> This is an ABI change on ARM. X86 does not change.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Jan Beulich <JBeulich@suse.com>
> Cc: Keir (Xen.org) <keir@xen.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: xen-devel at lists.xen.org
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Changes since V4
> Rebase onto v3.8
> Fix wording of comment
> Fix bitmask length passed to find_first_bit, need sizeof*8 for bits not just
> sizeof. Use BITS_PER_EVTCHN_WORD and provide a convenience wrapper.
> Changes since V3
> s/read_evtchn_pending_sel/xchg_xen_ulong/ in a comment.
> Changes since V2
> Add comments about the correct bitops to use, and on the ordering/barrier
> requirements on xchg_xen_ulong.
> Changes since V1
> use find_first_set not __ffs
> fix some more unsigned long -> xen_ulong_t
> use more generic xchg_xen_ulong instead of ...read_evtchn...
> ---
> arch/arm/include/asm/xen/events.h | 22 +++++++
> arch/x86/include/asm/xen/events.h | 3 +
> drivers/xen/events.c | 115 +++++++++++++++++++++---------------
> include/xen/interface/xen.h | 8 +-
> 4 files changed, 96 insertions(+), 52 deletions(-)
>
> diff --git a/arch/arm/include/asm/xen/events.h b/arch/arm/include/asm/xen/events.h
> index 94b4e90..5c27696 100644
> --- a/arch/arm/include/asm/xen/events.h
> +++ b/arch/arm/include/asm/xen/events.h
> @@ -15,4 +15,26 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
> return raw_irqs_disabled_flags(regs->ARM_cpsr);
> }
>
> +/*
> + * We cannot use xchg because it does not support 8-byte
> + * values. However it is safe to use {ldr,dtd}exd directly because all
> + * platforms which Xen can run on support those instructions.
> + */
> +static inline xen_ulong_t xchg_xen_ulong(xen_ulong_t *ptr, xen_ulong_t val)
> +{
> + xen_ulong_t oldval;
> + unsigned int tmp;
> +
> + wmb();
> + asm volatile("@ xchg_xen_ulong\n"
> + "1: ldrexd %0, %H0, [%3]\n"
> + " strexd %1, %2, %H2, [%3]\n"
> + " teq %1, #0\n"
> + " bne 1b"
> + : "=&r" (oldval), "=&r" (tmp)
> + : "r" (val), "r" (ptr)
> + : "memory", "cc");
> + return oldval;
> +}
> +
> #endif /* _ASM_ARM_XEN_EVENTS_H */
> diff --git a/arch/x86/include/asm/xen/events.h b/arch/x86/include/asm/xen/events.h
> index cc146d5..ca842f2 100644
> --- a/arch/x86/include/asm/xen/events.h
> +++ b/arch/x86/include/asm/xen/events.h
> @@ -16,4 +16,7 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
> return raw_irqs_disabled_flags(regs->flags);
> }
>
> +/* No need for a barrier -- XCHG is a barrier on x86. */
> +#define xchg_xen_ulong(ptr, val) xchg((ptr), (val))
> +
> #endif /* _ASM_X86_XEN_EVENTS_H */
> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> index 74d77df..dfd62b5 100644
> --- a/drivers/xen/events.c
> +++ b/drivers/xen/events.c
> @@ -120,7 +120,22 @@ static unsigned long *pirq_eoi_map;
> #endif
> static bool (*pirq_needs_eoi)(unsigned irq);
>
> -static DEFINE_PER_CPU(unsigned long [NR_EVENT_CHANNELS/BITS_PER_LONG],
> +/*
> + * Note sizeof(xen_ulong_t) can be more than sizeof(unsigned long). Be
> + * careful to only use bitops which allow for this (e.g
> + * test_bit/find_first_bit and friends but not __ffs) and to pass
> + * BITS_PER_EVTCHN_WORD as the bitmask length.
> + */
> +#define BITS_PER_EVTCHN_WORD (sizeof(xen_ulong_t)*8)
> +/*
> + * Make a bitmask (i.e. unsigned long *) of a xen_ulong_t
> + * array. Primarily to avoid long lines (hence the terse name).
> + */
> +#define BM(x) (unsigned long *)(x)
> +/* Find the first set bit in a evtchn mask */
> +#define EVTCHN_FIRST_BIT(w) find_first_bit(BM(&(w)), BITS_PER_EVTCHN_WORD)
> +
> +static DEFINE_PER_CPU(xen_ulong_t [NR_EVENT_CHANNELS/BITS_PER_EVTCHN_WORD],
> cpu_evtchn_mask);
>
> /* Xen will never allocate port zero for any purpose. */
> @@ -294,9 +309,9 @@ static bool pirq_needs_eoi_flag(unsigned irq)
> return info->u.pirq.flags & PIRQ_NEEDS_EOI;
> }
>
> -static inline unsigned long active_evtchns(unsigned int cpu,
> - struct shared_info *sh,
> - unsigned int idx)
> +static inline xen_ulong_t active_evtchns(unsigned int cpu,
> + struct shared_info *sh,
> + unsigned int idx)
> {
> return sh->evtchn_pending[idx] &
> per_cpu(cpu_evtchn_mask, cpu)[idx] &
> @@ -312,8 +327,8 @@ static void bind_evtchn_to_cpu(unsigned int chn, unsigned int cpu)
> cpumask_copy(irq_to_desc(irq)->irq_data.affinity, cpumask_of(cpu));
> #endif
>
> - clear_bit(chn, per_cpu(cpu_evtchn_mask, cpu_from_irq(irq)));
> - set_bit(chn, per_cpu(cpu_evtchn_mask, cpu));
> + clear_bit(chn, BM(per_cpu(cpu_evtchn_mask, cpu_from_irq(irq))));
> + set_bit(chn, BM(per_cpu(cpu_evtchn_mask, cpu)));
>
> info_for_irq(irq)->cpu = cpu;
> }
> @@ -339,19 +354,19 @@ static void init_evtchn_cpu_bindings(void)
> static inline void clear_evtchn(int port)
> {
> struct shared_info *s = HYPERVISOR_shared_info;
> - sync_clear_bit(port, &s->evtchn_pending[0]);
> + sync_clear_bit(port, BM(&s->evtchn_pending[0]));
> }
>
> static inline void set_evtchn(int port)
> {
> struct shared_info *s = HYPERVISOR_shared_info;
> - sync_set_bit(port, &s->evtchn_pending[0]);
> + sync_set_bit(port, BM(&s->evtchn_pending[0]));
> }
>
> static inline int test_evtchn(int port)
> {
> struct shared_info *s = HYPERVISOR_shared_info;
> - return sync_test_bit(port, &s->evtchn_pending[0]);
> + return sync_test_bit(port, BM(&s->evtchn_pending[0]));
> }
>
>
> @@ -375,7 +390,7 @@ EXPORT_SYMBOL_GPL(notify_remote_via_irq);
> static void mask_evtchn(int port)
> {
> struct shared_info *s = HYPERVISOR_shared_info;
> - sync_set_bit(port, &s->evtchn_mask[0]);
> + sync_set_bit(port, BM(&s->evtchn_mask[0]));
> }
>
> static void unmask_evtchn(int port)
> @@ -389,7 +404,7 @@ static void unmask_evtchn(int port)
> if (unlikely((cpu != cpu_from_evtchn(port))))
> do_hypercall = 1;
> else
> - evtchn_pending = sync_test_bit(port, &s->evtchn_pending[0]);
> + evtchn_pending = sync_test_bit(port, BM(&s->evtchn_pending[0]));
>
> if (unlikely(evtchn_pending && xen_hvm_domain()))
> do_hypercall = 1;
> @@ -403,7 +418,7 @@ static void unmask_evtchn(int port)
> } else {
> struct vcpu_info *vcpu_info = __this_cpu_read(xen_vcpu);
>
> - sync_clear_bit(port, &s->evtchn_mask[0]);
> + sync_clear_bit(port, BM(&s->evtchn_mask[0]));
>
> /*
> * The following is basically the equivalent of
> @@ -411,8 +426,8 @@ static void unmask_evtchn(int port)
> * the interrupt edge' if the channel is masked.
> */
> if (evtchn_pending &&
> - !sync_test_and_set_bit(port / BITS_PER_LONG,
> - &vcpu_info->evtchn_pending_sel))
> + !sync_test_and_set_bit(port / BITS_PER_EVTCHN_WORD,
> + BM(&vcpu_info->evtchn_pending_sel)))
> vcpu_info->evtchn_upcall_pending = 1;
> }
>
> @@ -1189,7 +1204,7 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
> {
> struct shared_info *sh = HYPERVISOR_shared_info;
> int cpu = smp_processor_id();
> - unsigned long *cpu_evtchn = per_cpu(cpu_evtchn_mask, cpu);
> + xen_ulong_t *cpu_evtchn = per_cpu(cpu_evtchn_mask, cpu);
> int i;
> unsigned long flags;
> static DEFINE_SPINLOCK(debug_lock);
> @@ -1205,7 +1220,7 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
> pending = (get_irq_regs() && i == cpu)
> ? xen_irqs_disabled(get_irq_regs())
> : v->evtchn_upcall_mask;
> - printk("%d: masked=%d pending=%d event_sel %0*lx\n ", i,
> + printk("%d: masked=%d pending=%d event_sel %0*"PRI_xen_ulong"\n ", i,
> pending, v->evtchn_upcall_pending,
> (int)(sizeof(v->evtchn_pending_sel)*2),
> v->evtchn_pending_sel);
> @@ -1214,49 +1229,52 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
>
> printk("\npending:\n ");
> for (i = ARRAY_SIZE(sh->evtchn_pending)-1; i >= 0; i--)
> - printk("%0*lx%s", (int)sizeof(sh->evtchn_pending[0])*2,
> + printk("%0*"PRI_xen_ulong"%s",
> + (int)sizeof(sh->evtchn_pending[0])*2,
> sh->evtchn_pending[i],
> i % 8 == 0 ? "\n " : " ");
> printk("\nglobal mask:\n ");
> for (i = ARRAY_SIZE(sh->evtchn_mask)-1; i >= 0; i--)
> - printk("%0*lx%s",
> + printk("%0*"PRI_xen_ulong"%s",
> (int)(sizeof(sh->evtchn_mask[0])*2),
> sh->evtchn_mask[i],
> i % 8 == 0 ? "\n " : " ");
>
> printk("\nglobally unmasked:\n ");
> for (i = ARRAY_SIZE(sh->evtchn_mask)-1; i >= 0; i--)
> - printk("%0*lx%s", (int)(sizeof(sh->evtchn_mask[0])*2),
> + printk("%0*"PRI_xen_ulong"%s",
> + (int)(sizeof(sh->evtchn_mask[0])*2),
> sh->evtchn_pending[i] & ~sh->evtchn_mask[i],
> i % 8 == 0 ? "\n " : " ");
>
> printk("\nlocal cpu%d mask:\n ", cpu);
> - for (i = (NR_EVENT_CHANNELS/BITS_PER_LONG)-1; i >= 0; i--)
> - printk("%0*lx%s", (int)(sizeof(cpu_evtchn[0])*2),
> + for (i = (NR_EVENT_CHANNELS/BITS_PER_EVTCHN_WORD)-1; i >= 0; i--)
> + printk("%0*"PRI_xen_ulong"%s", (int)(sizeof(cpu_evtchn[0])*2),
> cpu_evtchn[i],
> i % 8 == 0 ? "\n " : " ");
>
> printk("\nlocally unmasked:\n ");
> for (i = ARRAY_SIZE(sh->evtchn_mask)-1; i >= 0; i--) {
> - unsigned long pending = sh->evtchn_pending[i]
> + xen_ulong_t pending = sh->evtchn_pending[i]
> & ~sh->evtchn_mask[i]
> & cpu_evtchn[i];
> - printk("%0*lx%s", (int)(sizeof(sh->evtchn_mask[0])*2),
> + printk("%0*"PRI_xen_ulong"%s",
> + (int)(sizeof(sh->evtchn_mask[0])*2),
> pending, i % 8 == 0 ? "\n " : " ");
> }
>
> printk("\npending list:\n");
> for (i = 0; i < NR_EVENT_CHANNELS; i++) {
> - if (sync_test_bit(i, sh->evtchn_pending)) {
> - int word_idx = i / BITS_PER_LONG;
> + if (sync_test_bit(i, BM(sh->evtchn_pending))) {
> + int word_idx = i / BITS_PER_EVTCHN_WORD;
> printk(" %d: event %d -> irq %d%s%s%s\n",
> cpu_from_evtchn(i), i,
> evtchn_to_irq[i],
> - sync_test_bit(word_idx, &v->evtchn_pending_sel)
> + sync_test_bit(word_idx, BM(&v->evtchn_pending_sel))
> ? "" : " l2-clear",
> - !sync_test_bit(i, sh->evtchn_mask)
> + !sync_test_bit(i, BM(sh->evtchn_mask))
> ? "" : " globally-masked",
> - sync_test_bit(i, cpu_evtchn)
> + sync_test_bit(i, BM(cpu_evtchn))
> ? "" : " locally-masked");
> }
> }
> @@ -1273,7 +1291,7 @@ static DEFINE_PER_CPU(unsigned int, current_bit_idx);
> /*
> * Mask out the i least significant bits of w
> */
> -#define MASK_LSBS(w, i) (w & ((~0UL) << i))
> +#define MASK_LSBS(w, i) (w & ((~((xen_ulong_t)0UL)) << i))
>
> /*
> * Search the CPUs pending events bitmasks. For each one found, map
> @@ -1295,18 +1313,19 @@ static void __xen_evtchn_do_upcall(void)
> unsigned count;
>
> do {
> - unsigned long pending_words;
> + xen_ulong_t pending_words;
>
> vcpu_info->evtchn_upcall_pending = 0;
>
> if (__this_cpu_inc_return(xed_nesting_count) - 1)
> goto out;
>
> -#ifndef CONFIG_X86 /* No need for a barrier -- XCHG is a barrier on x86. */
> - /* Clear master flag /before/ clearing selector flag. */
> - wmb();
> -#endif
> - pending_words = xchg(&vcpu_info->evtchn_pending_sel, 0);
> + /*
> + * Master flag must be cleared /before/ clearing
> + * selector flag. xchg_xen_ulong must contain an
> + * appropriate barrier.
> + */
> + pending_words = xchg_xen_ulong(&vcpu_info->evtchn_pending_sel, 0);
>
> start_word_idx = __this_cpu_read(current_word_idx);
> start_bit_idx = __this_cpu_read(current_bit_idx);
> @@ -1314,8 +1333,8 @@ static void __xen_evtchn_do_upcall(void)
> word_idx = start_word_idx;
>
> for (i = 0; pending_words != 0; i++) {
> - unsigned long pending_bits;
> - unsigned long words;
> + xen_ulong_t pending_bits;
> + xen_ulong_t words;
>
> words = MASK_LSBS(pending_words, word_idx);
>
> @@ -1327,7 +1346,7 @@ static void __xen_evtchn_do_upcall(void)
> bit_idx = 0;
> continue;
> }
> - word_idx = __ffs(words);
> + word_idx = EVTCHN_FIRST_BIT(words);
>
> pending_bits = active_evtchns(cpu, s, word_idx);
> bit_idx = 0; /* usually scan entire word from start */
> @@ -1342,7 +1361,7 @@ static void __xen_evtchn_do_upcall(void)
> }
>
> do {
> - unsigned long bits;
> + xen_ulong_t bits;
> int port, irq;
> struct irq_desc *desc;
>
> @@ -1352,10 +1371,10 @@ static void __xen_evtchn_do_upcall(void)
> if (bits == 0)
> break;
>
> - bit_idx = __ffs(bits);
> + bit_idx = EVTCHN_FIRST_BIT(bits);
>
> /* Process port. */
> - port = (word_idx * BITS_PER_LONG) + bit_idx;
> + port = (word_idx * BITS_PER_EVTCHN_WORD) + bit_idx;
> irq = evtchn_to_irq[port];
>
> if (irq != -1) {
> @@ -1364,12 +1383,12 @@ static void __xen_evtchn_do_upcall(void)
> generic_handle_irq_desc(irq, desc);
> }
>
> - bit_idx = (bit_idx + 1) % BITS_PER_LONG;
> + bit_idx = (bit_idx + 1) % BITS_PER_EVTCHN_WORD;
>
> /* Next caller starts at last processed + 1 */
> __this_cpu_write(current_word_idx,
> bit_idx ? word_idx :
> - (word_idx+1) % BITS_PER_LONG);
> + (word_idx+1) % BITS_PER_EVTCHN_WORD);
> __this_cpu_write(current_bit_idx, bit_idx);
> } while (bit_idx != 0);
>
> @@ -1377,7 +1396,7 @@ static void __xen_evtchn_do_upcall(void)
> if ((word_idx != start_word_idx) || (i != 0))
> pending_words &= ~(1UL << word_idx);
>
> - word_idx = (word_idx + 1) % BITS_PER_LONG;
> + word_idx = (word_idx + 1) % BITS_PER_EVTCHN_WORD;
> }
>
> BUG_ON(!irqs_disabled());
> @@ -1487,8 +1506,8 @@ int resend_irq_on_evtchn(unsigned int irq)
> if (!VALID_EVTCHN(evtchn))
> return 1;
>
> - masked = sync_test_and_set_bit(evtchn, s->evtchn_mask);
> - sync_set_bit(evtchn, s->evtchn_pending);
> + masked = sync_test_and_set_bit(evtchn, BM(s->evtchn_mask));
> + sync_set_bit(evtchn, BM(s->evtchn_pending));
> if (!masked)
> unmask_evtchn(evtchn);
>
> @@ -1536,8 +1555,8 @@ static int retrigger_dynirq(struct irq_data *data)
> if (VALID_EVTCHN(evtchn)) {
> int masked;
>
> - masked = sync_test_and_set_bit(evtchn, sh->evtchn_mask);
> - sync_set_bit(evtchn, sh->evtchn_pending);
> + masked = sync_test_and_set_bit(evtchn, BM(sh->evtchn_mask));
> + sync_set_bit(evtchn, BM(sh->evtchn_pending));
> if (!masked)
> unmask_evtchn(evtchn);
> ret = 1;
> diff --git a/include/xen/interface/xen.h b/include/xen/interface/xen.h
> index 5d6c272..a9075df 100644
> --- a/include/xen/interface/xen.h
> +++ b/include/xen/interface/xen.h
> @@ -285,7 +285,7 @@ DEFINE_GUEST_HANDLE_STRUCT(multicall_entry);
> * Event channel endpoints per domain:
> * 1024 if a long is 32 bits; 4096 if a long is 64 bits.
> */
> -#define NR_EVENT_CHANNELS (sizeof(unsigned long) * sizeof(unsigned long) * 64)
> +#define NR_EVENT_CHANNELS (sizeof(xen_ulong_t) * sizeof(xen_ulong_t) * 64)
>
> struct vcpu_time_info {
> /*
> @@ -341,7 +341,7 @@ struct vcpu_info {
> */
> uint8_t evtchn_upcall_pending;
> uint8_t evtchn_upcall_mask;
> - unsigned long evtchn_pending_sel;
> + xen_ulong_t evtchn_pending_sel;
> struct arch_vcpu_info arch;
> struct pvclock_vcpu_time_info time;
> }; /* 64 bytes (x86) */
> @@ -384,8 +384,8 @@ struct shared_info {
> * per-vcpu selector word to be set. Each bit in the selector covers a
> * 'C long' in the PENDING bitfield array.
> */
> - unsigned long evtchn_pending[sizeof(unsigned long) * 8];
> - unsigned long evtchn_mask[sizeof(unsigned long) * 8];
> + xen_ulong_t evtchn_pending[sizeof(xen_ulong_t) * 8];
> + xen_ulong_t evtchn_mask[sizeof(xen_ulong_t) * 8];
>
> /*
> * Wallclock time: updated only by control software. Guests should base
> --
> 1.7.2.5
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH XEN] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-02-19 14:49 ` [PATCH XEN] " Ian Campbell
2013-02-19 16:42 ` Stefano Stabellini
@ 2013-02-21 17:16 ` Ian Campbell
2013-02-21 18:43 ` Keir Fraser
2013-02-22 8:12 ` Jan Beulich
1 sibling, 2 replies; 36+ messages in thread
From: Ian Campbell @ 2013-02-21 17:16 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 2013-02-19 at 14:49 +0000, Ian Campbell wrote:
> On ARM we want these to be the same size on 32- and 64-bit.
>
> This is an ABI change on ARM. X86 does not change.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Jan Beulich <JBeulich@suse.com>
> Cc: Keir (Xen.org) <keir@xen.org>
Are you guys (un)happy with this change from the Xen & x86 side?
> Cc: Tim Deegan <tim@xen.org>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: xen-devel at lists.xen.org
> ---
> tools/include/xen-foreign/mkheader.py | 6 ++++++
> xen/include/public/xen.h | 8 ++++----
> 2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/tools/include/xen-foreign/mkheader.py b/tools/include/xen-foreign/mkheader.py
> index d189b07..57681fa 100644
> --- a/tools/include/xen-foreign/mkheader.py
> +++ b/tools/include/xen-foreign/mkheader.py
> @@ -21,13 +21,18 @@ inttypes["arm"] = {
> "unsigned long" : "uint32_t",
> "long" : "uint32_t",
> "xen_pfn_t" : "uint64_t",
> + "xen_ulong_t" : "uint64_t",
> };
> +header["arm"] = """
> +#define __arm___ARM32 1
> +""";
>
> # x86_32
> inttypes["x86_32"] = {
> "unsigned long" : "uint32_t",
> "long" : "uint32_t",
> "xen_pfn_t" : "uint32_t",
> + "xen_ulong_t" : "uint32_t",
> };
> header["x86_32"] = """
> #define __i386___X86_32 1
> @@ -42,6 +47,7 @@ inttypes["x86_64"] = {
> "unsigned long" : "__align8__ uint64_t",
> "long" : "__align8__ uint64_t",
> "xen_pfn_t" : "__align8__ uint64_t",
> + "xen_ulong_t" : "__align8__ uint64_t",
> };
> header["x86_64"] = """
> #if defined(__GNUC__) && !defined(__STRICT_ANSI__)
> diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
> index 5593066..99c8212 100644
> --- a/xen/include/public/xen.h
> +++ b/xen/include/public/xen.h
> @@ -556,7 +556,7 @@ DEFINE_XEN_GUEST_HANDLE(multicall_entry_t);
> * Event channel endpoints per domain:
> * 1024 if a long is 32 bits; 4096 if a long is 64 bits.
> */
> -#define NR_EVENT_CHANNELS (sizeof(unsigned long) * sizeof(unsigned long) * 64)
> +#define NR_EVENT_CHANNELS (sizeof(xen_ulong_t) * sizeof(xen_ulong_t) * 64)
>
> struct vcpu_time_info {
> /*
> @@ -613,7 +613,7 @@ struct vcpu_info {
> */
> uint8_t evtchn_upcall_pending;
> uint8_t evtchn_upcall_mask;
> - unsigned long evtchn_pending_sel;
> + xen_ulong_t evtchn_pending_sel;
> struct arch_vcpu_info arch;
> struct vcpu_time_info time;
> }; /* 64 bytes (x86) */
> @@ -663,8 +663,8 @@ struct shared_info {
> * per-vcpu selector word to be set. Each bit in the selector covers a
> * 'C long' in the PENDING bitfield array.
> */
> - unsigned long evtchn_pending[sizeof(unsigned long) * 8];
> - unsigned long evtchn_mask[sizeof(unsigned long) * 8];
> + xen_ulong_t evtchn_pending[sizeof(xen_ulong_t) * 8];
> + xen_ulong_t evtchn_mask[sizeof(xen_ulong_t) * 8];
>
> /*
> * Wallclock time: updated only by control software. Guests should base
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH XEN] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-02-21 17:16 ` Ian Campbell
@ 2013-02-21 18:43 ` Keir Fraser
2013-02-22 8:28 ` Ian Campbell
2013-02-22 8:12 ` Jan Beulich
1 sibling, 1 reply; 36+ messages in thread
From: Keir Fraser @ 2013-02-21 18:43 UTC (permalink / raw)
To: linux-arm-kernel
On 21/02/2013 17:16, "Ian Campbell" <Ian.Campbell@citrix.com> wrote:
> On Tue, 2013-02-19 at 14:49 +0000, Ian Campbell wrote:
>> On ARM we want these to be the same size on 32- and 64-bit.
>>
>> This is an ABI change on ARM. X86 does not change.
>>
>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
>> Cc: Jan Beulich <JBeulich@suse.com>
>> Cc: Keir (Xen.org) <keir@xen.org>
>
> Are you guys (un)happy with this change from the Xen & x86 side?
Fine by me.
-- Keir
>> Cc: Tim Deegan <tim@xen.org>
>> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> Cc: linux-arm-kernel at lists.infradead.org
>> Cc: xen-devel at lists.xen.org
>> ---
>> tools/include/xen-foreign/mkheader.py | 6 ++++++
>> xen/include/public/xen.h | 8 ++++----
>> 2 files changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/include/xen-foreign/mkheader.py
>> b/tools/include/xen-foreign/mkheader.py
>> index d189b07..57681fa 100644
>> --- a/tools/include/xen-foreign/mkheader.py
>> +++ b/tools/include/xen-foreign/mkheader.py
>> @@ -21,13 +21,18 @@ inttypes["arm"] = {
>> "unsigned long" : "uint32_t",
>> "long" : "uint32_t",
>> "xen_pfn_t" : "uint64_t",
>> + "xen_ulong_t" : "uint64_t",
>> };
>> +header["arm"] = """
>> +#define __arm___ARM32 1
>> +""";
>>
>> # x86_32
>> inttypes["x86_32"] = {
>> "unsigned long" : "uint32_t",
>> "long" : "uint32_t",
>> "xen_pfn_t" : "uint32_t",
>> + "xen_ulong_t" : "uint32_t",
>> };
>> header["x86_32"] = """
>> #define __i386___X86_32 1
>> @@ -42,6 +47,7 @@ inttypes["x86_64"] = {
>> "unsigned long" : "__align8__ uint64_t",
>> "long" : "__align8__ uint64_t",
>> "xen_pfn_t" : "__align8__ uint64_t",
>> + "xen_ulong_t" : "__align8__ uint64_t",
>> };
>> header["x86_64"] = """
>> #if defined(__GNUC__) && !defined(__STRICT_ANSI__)
>> diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
>> index 5593066..99c8212 100644
>> --- a/xen/include/public/xen.h
>> +++ b/xen/include/public/xen.h
>> @@ -556,7 +556,7 @@ DEFINE_XEN_GUEST_HANDLE(multicall_entry_t);
>> * Event channel endpoints per domain:
>> * 1024 if a long is 32 bits; 4096 if a long is 64 bits.
>> */
>> -#define NR_EVENT_CHANNELS (sizeof(unsigned long) * sizeof(unsigned long) *
>> 64)
>> +#define NR_EVENT_CHANNELS (sizeof(xen_ulong_t) * sizeof(xen_ulong_t) * 64)
>>
>> struct vcpu_time_info {
>> /*
>> @@ -613,7 +613,7 @@ struct vcpu_info {
>> */
>> uint8_t evtchn_upcall_pending;
>> uint8_t evtchn_upcall_mask;
>> - unsigned long evtchn_pending_sel;
>> + xen_ulong_t evtchn_pending_sel;
>> struct arch_vcpu_info arch;
>> struct vcpu_time_info time;
>> }; /* 64 bytes (x86) */
>> @@ -663,8 +663,8 @@ struct shared_info {
>> * per-vcpu selector word to be set. Each bit in the selector covers a
>> * 'C long' in the PENDING bitfield array.
>> */
>> - unsigned long evtchn_pending[sizeof(unsigned long) * 8];
>> - unsigned long evtchn_mask[sizeof(unsigned long) * 8];
>> + xen_ulong_t evtchn_pending[sizeof(xen_ulong_t) * 8];
>> + xen_ulong_t evtchn_mask[sizeof(xen_ulong_t) * 8];
>>
>> /*
>> * Wallclock time: updated only by control software. Guests should base
>
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH XEN] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-02-21 17:16 ` Ian Campbell
2013-02-21 18:43 ` Keir Fraser
@ 2013-02-22 8:12 ` Jan Beulich
2013-02-22 8:28 ` Ian Campbell
1 sibling, 1 reply; 36+ messages in thread
From: Jan Beulich @ 2013-02-22 8:12 UTC (permalink / raw)
To: linux-arm-kernel
>>> On 21.02.13 at 18:16, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Tue, 2013-02-19 at 14:49 +0000, Ian Campbell wrote:
>> On ARM we want these to be the same size on 32- and 64-bit.
>>
>> This is an ABI change on ARM. X86 does not change.
>>
>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
>> Cc: Jan Beulich <JBeulich@suse.com>
>> Cc: Keir (Xen.org) <keir@xen.org>
>
> Are you guys (un)happy with this change from the Xen & x86 side?
I don't see any problem with it.
Jan
>> Cc: Tim Deegan <tim@xen.org>
>> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> Cc: linux-arm-kernel at lists.infradead.org
>> Cc: xen-devel at lists.xen.org
>> ---
>> tools/include/xen-foreign/mkheader.py | 6 ++++++
>> xen/include/public/xen.h | 8 ++++----
>> 2 files changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/include/xen-foreign/mkheader.py
> b/tools/include/xen-foreign/mkheader.py
>> index d189b07..57681fa 100644
>> --- a/tools/include/xen-foreign/mkheader.py
>> +++ b/tools/include/xen-foreign/mkheader.py
>> @@ -21,13 +21,18 @@ inttypes["arm"] = {
>> "unsigned long" : "uint32_t",
>> "long" : "uint32_t",
>> "xen_pfn_t" : "uint64_t",
>> + "xen_ulong_t" : "uint64_t",
>> };
>> +header["arm"] = """
>> +#define __arm___ARM32 1
>> +""";
>>
>> # x86_32
>> inttypes["x86_32"] = {
>> "unsigned long" : "uint32_t",
>> "long" : "uint32_t",
>> "xen_pfn_t" : "uint32_t",
>> + "xen_ulong_t" : "uint32_t",
>> };
>> header["x86_32"] = """
>> #define __i386___X86_32 1
>> @@ -42,6 +47,7 @@ inttypes["x86_64"] = {
>> "unsigned long" : "__align8__ uint64_t",
>> "long" : "__align8__ uint64_t",
>> "xen_pfn_t" : "__align8__ uint64_t",
>> + "xen_ulong_t" : "__align8__ uint64_t",
>> };
>> header["x86_64"] = """
>> #if defined(__GNUC__) && !defined(__STRICT_ANSI__)
>> diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
>> index 5593066..99c8212 100644
>> --- a/xen/include/public/xen.h
>> +++ b/xen/include/public/xen.h
>> @@ -556,7 +556,7 @@ DEFINE_XEN_GUEST_HANDLE(multicall_entry_t);
>> * Event channel endpoints per domain:
>> * 1024 if a long is 32 bits; 4096 if a long is 64 bits.
>> */
>> -#define NR_EVENT_CHANNELS (sizeof(unsigned long) * sizeof(unsigned long) *
> 64)
>> +#define NR_EVENT_CHANNELS (sizeof(xen_ulong_t) * sizeof(xen_ulong_t) * 64)
>>
>> struct vcpu_time_info {
>> /*
>> @@ -613,7 +613,7 @@ struct vcpu_info {
>> */
>> uint8_t evtchn_upcall_pending;
>> uint8_t evtchn_upcall_mask;
>> - unsigned long evtchn_pending_sel;
>> + xen_ulong_t evtchn_pending_sel;
>> struct arch_vcpu_info arch;
>> struct vcpu_time_info time;
>> }; /* 64 bytes (x86) */
>> @@ -663,8 +663,8 @@ struct shared_info {
>> * per-vcpu selector word to be set. Each bit in the selector covers a
>> * 'C long' in the PENDING bitfield array.
>> */
>> - unsigned long evtchn_pending[sizeof(unsigned long) * 8];
>> - unsigned long evtchn_mask[sizeof(unsigned long) * 8];
>> + xen_ulong_t evtchn_pending[sizeof(xen_ulong_t) * 8];
>> + xen_ulong_t evtchn_mask[sizeof(xen_ulong_t) * 8];
>>
>> /*
>> * Wallclock time: updated only by control software. Guests should base
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH XEN] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-02-21 18:43 ` Keir Fraser
@ 2013-02-22 8:28 ` Ian Campbell
0 siblings, 0 replies; 36+ messages in thread
From: Ian Campbell @ 2013-02-22 8:28 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 2013-02-21 at 18:43 +0000, Keir Fraser wrote:
> On 21/02/2013 17:16, "Ian Campbell" <Ian.Campbell@citrix.com> wrote:
>
> > On Tue, 2013-02-19 at 14:49 +0000, Ian Campbell wrote:
> >> On ARM we want these to be the same size on 32- and 64-bit.
> >>
> >> This is an ABI change on ARM. X86 does not change.
> >>
> >> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> >> Cc: Jan Beulich <JBeulich@suse.com>
> >> Cc: Keir (Xen.org) <keir@xen.org>
> >
> > Are you guys (un)happy with this change from the Xen & x86 side?
>
> Fine by me.
I'm going to take that as an Acked-by then, assuming that's ok with you.
I suppose Fine-by: Keir would do too ;-)
Ian.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH XEN] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-02-22 8:12 ` Jan Beulich
@ 2013-02-22 8:28 ` Ian Campbell
2013-02-22 8:48 ` Jan Beulich
0 siblings, 1 reply; 36+ messages in thread
From: Ian Campbell @ 2013-02-22 8:28 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 2013-02-22 at 08:12 +0000, Jan Beulich wrote:
> >>> On 21.02.13 at 18:16, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Tue, 2013-02-19 at 14:49 +0000, Ian Campbell wrote:
> >> On ARM we want these to be the same size on 32- and 64-bit.
> >>
> >> This is an ABI change on ARM. X86 does not change.
> >>
> >> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> >> Cc: Jan Beulich <JBeulich@suse.com>
> >> Cc: Keir (Xen.org) <keir@xen.org>
> >
> > Are you guys (un)happy with this change from the Xen & x86 side?
>
> I don't see any problem with it.
I'll take this as an Acked-by if that's ok with you.
Ian.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH XEN] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-02-22 8:28 ` Ian Campbell
@ 2013-02-22 8:48 ` Jan Beulich
2013-02-22 8:55 ` Ian Campbell
0 siblings, 1 reply; 36+ messages in thread
From: Jan Beulich @ 2013-02-22 8:48 UTC (permalink / raw)
To: linux-arm-kernel
>>> On 22.02.13 at 09:28, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Fri, 2013-02-22 at 08:12 +0000, Jan Beulich wrote:
>> >>> On 21.02.13 at 18:16, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> > On Tue, 2013-02-19 at 14:49 +0000, Ian Campbell wrote:
>> >> On ARM we want these to be the same size on 32- and 64-bit.
>> >>
>> >> This is an ABI change on ARM. X86 does not change.
>> >>
>> >> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
>> >> Cc: Jan Beulich <JBeulich@suse.com>
>> >> Cc: Keir (Xen.org) <keir@xen.org>
>> >
>> > Are you guys (un)happy with this change from the Xen & x86 side?
>>
>> I don't see any problem with it.
>
> I'll take this as an Acked-by if that's ok with you.
Well, I specifically didn't say "ack": I don't really mind the change,
but I'm also not eager see it go in. Not seeing a problem with it
doesn't really mean there's none lurking - type changes in public
interfaces are always an at least slightly risky business.
Jan
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH XEN] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-02-22 8:48 ` Jan Beulich
@ 2013-02-22 8:55 ` Ian Campbell
2013-02-22 9:05 ` Jan Beulich
0 siblings, 1 reply; 36+ messages in thread
From: Ian Campbell @ 2013-02-22 8:55 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 2013-02-22 at 08:48 +0000, Jan Beulich wrote:
> >>> On 22.02.13 at 09:28, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Fri, 2013-02-22 at 08:12 +0000, Jan Beulich wrote:
> >> >>> On 21.02.13 at 18:16, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> >> > On Tue, 2013-02-19 at 14:49 +0000, Ian Campbell wrote:
> >> >> On ARM we want these to be the same size on 32- and 64-bit.
> >> >>
> >> >> This is an ABI change on ARM. X86 does not change.
> >> >>
> >> >> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> >> >> Cc: Jan Beulich <JBeulich@suse.com>
> >> >> Cc: Keir (Xen.org) <keir@xen.org>
> >> >
> >> > Are you guys (un)happy with this change from the Xen & x86 side?
> >>
> >> I don't see any problem with it.
> >
> > I'll take this as an Acked-by if that's ok with you.
>
> Well, I specifically didn't say "ack": I don't really mind the change,
> but I'm also not eager see it go in. Not seeing a problem with it
> doesn't really mean there's none lurking - type changes in public
> interfaces are always an at least slightly risky business.
For x86 there is no change here since:
xen/include/public/arch-x86/xen.h:typedef unsigned long xen_ulong_t;
Also the tools/include/xen-foreign checks show no change in the size of
things on x86.
On ARM the change is deliberate, if there is any fallout we can fix it.
Ian.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH XEN] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-02-22 8:55 ` Ian Campbell
@ 2013-02-22 9:05 ` Jan Beulich
0 siblings, 0 replies; 36+ messages in thread
From: Jan Beulich @ 2013-02-22 9:05 UTC (permalink / raw)
To: linux-arm-kernel
>>> On 22.02.13 at 09:55, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Fri, 2013-02-22 at 08:48 +0000, Jan Beulich wrote:
>> >>> On 22.02.13 at 09:28, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> > On Fri, 2013-02-22 at 08:12 +0000, Jan Beulich wrote:
>> >> >>> On 21.02.13 at 18:16, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> >> > On Tue, 2013-02-19 at 14:49 +0000, Ian Campbell wrote:
>> >> >> On ARM we want these to be the same size on 32- and 64-bit.
>> >> >>
>> >> >> This is an ABI change on ARM. X86 does not change.
>> >> >>
>> >> >> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
>> >> >> Cc: Jan Beulich <JBeulich@suse.com>
>> >> >> Cc: Keir (Xen.org) <keir@xen.org>
>> >> >
>> >> > Are you guys (un)happy with this change from the Xen & x86 side?
>> >>
>> >> I don't see any problem with it.
>> >
>> > I'll take this as an Acked-by if that's ok with you.
>>
>> Well, I specifically didn't say "ack": I don't really mind the change,
>> but I'm also not eager see it go in. Not seeing a problem with it
>> doesn't really mean there's none lurking - type changes in public
>> interfaces are always an at least slightly risky business.
>
> For x86 there is no change here since:
> xen/include/public/arch-x86/xen.h:typedef unsigned long xen_ulong_t;
>
> Also the tools/include/xen-foreign checks show no change in the size of
> things on x86.
Which is why I said I see no problem with it.
> On ARM the change is deliberate, if there is any fallout we can fix it.
I understand that.
Jan
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH LINUX v5] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-02-20 11:48 ` [PATCH LINUX v5] " Ian Campbell
2013-02-21 17:11 ` Stefano Stabellini
@ 2013-03-04 2:45 ` Rob Herring
2013-03-05 3:04 ` Will Deacon
2013-03-05 3:56 ` Ian Campbell
1 sibling, 2 replies; 36+ messages in thread
From: Rob Herring @ 2013-03-04 2:45 UTC (permalink / raw)
To: linux-arm-kernel
On 02/20/2013 05:48 AM, Ian Campbell wrote:
> On ARM we want these to be the same size on 32- and 64-bit.
>
> This is an ABI change on ARM. X86 does not change.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Jan Beulich <JBeulich@suse.com>
> Cc: Keir (Xen.org) <keir@xen.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: xen-devel at lists.xen.org
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
> Changes since V4
> Rebase onto v3.8
> Fix wording of comment
> Fix bitmask length passed to find_first_bit, need sizeof*8 for bits not just
> sizeof. Use BITS_PER_EVTCHN_WORD and provide a convenience wrapper.
> Changes since V3
> s/read_evtchn_pending_sel/xchg_xen_ulong/ in a comment.
> Changes since V2
> Add comments about the correct bitops to use, and on the ordering/barrier
> requirements on xchg_xen_ulong.
> Changes since V1
> use find_first_set not __ffs
> fix some more unsigned long -> xen_ulong_t
> use more generic xchg_xen_ulong instead of ...read_evtchn...
> ---
> arch/arm/include/asm/xen/events.h | 22 +++++++
> arch/x86/include/asm/xen/events.h | 3 +
> drivers/xen/events.c | 115 +++++++++++++++++++++---------------
> include/xen/interface/xen.h | 8 +-
> 4 files changed, 96 insertions(+), 52 deletions(-)
>
I'm seeing some some build failures on randconfig builds with this change:
/tmp/ccJaIZOW.s: Assembler messages:
/tmp/ccJaIZOW.s:831: Error: even register required -- `ldrexd r5,r6,[r4]'
This is with ubuntu 12.04 cross compiler (gcc version 4.6.3
(Ubuntu/Linaro 4.6.3-1ubuntu5)).
This register restriction is on ARM, but not Thumb builds. Comparing
this to atomic64_cmpxchg, I don't see how to fix this. Perhaps Will or
Nico have thoughts.
> diff --git a/arch/arm/include/asm/xen/events.h b/arch/arm/include/asm/xen/events.h
> index 94b4e90..5c27696 100644
> --- a/arch/arm/include/asm/xen/events.h
> +++ b/arch/arm/include/asm/xen/events.h
> @@ -15,4 +15,26 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
> return raw_irqs_disabled_flags(regs->ARM_cpsr);
> }
>
> +/*
> + * We cannot use xchg because it does not support 8-byte
> + * values. However it is safe to use {ldr,dtd}exd directly because all
> + * platforms which Xen can run on support those instructions.
Why does atomic64_cmpxchg not work here?
> + */
> +static inline xen_ulong_t xchg_xen_ulong(xen_ulong_t *ptr, xen_ulong_t val)
> +{
> + xen_ulong_t oldval;
> + unsigned int tmp;
> +
> + wmb();
Based on atomic64_cmpxchg implementation, you could use smp_mb here
which avoids an outer cache flush.
> + asm volatile("@ xchg_xen_ulong\n"
> + "1: ldrexd %0, %H0, [%3]\n"
> + " strexd %1, %2, %H2, [%3]\n"
> + " teq %1, #0\n"
> + " bne 1b"
> + : "=&r" (oldval), "=&r" (tmp)
> + : "r" (val), "r" (ptr)
> + : "memory", "cc");
And a smp_mb is needed here.
Rob
> + return oldval;
> +}
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH LINUX v5] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-03-04 2:45 ` Rob Herring
@ 2013-03-05 3:04 ` Will Deacon
2013-03-05 3:45 ` Ian Campbell
2013-03-05 6:55 ` Rob Herring
2013-03-05 3:56 ` Ian Campbell
1 sibling, 2 replies; 36+ messages in thread
From: Will Deacon @ 2013-03-05 3:04 UTC (permalink / raw)
To: linux-arm-kernel
Hi guys,
On Mon, Mar 04, 2013 at 02:45:33AM +0000, Rob Herring wrote:
> On 02/20/2013 05:48 AM, Ian Campbell wrote:
> > On ARM we want these to be the same size on 32- and 64-bit.
> >
> > This is an ABI change on ARM. X86 does not change.
> >
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > Cc: Jan Beulich <JBeulich@suse.com>
> > Cc: Keir (Xen.org) <keir@xen.org>
> > Cc: Tim Deegan <tim@xen.org>
> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > Cc: linux-arm-kernel at lists.infradead.org
> > Cc: xen-devel at lists.xen.org
> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
[...]
> I'm seeing some some build failures on randconfig builds with this change:
>
> /tmp/ccJaIZOW.s: Assembler messages:
> /tmp/ccJaIZOW.s:831: Error: even register required -- `ldrexd r5,r6,[r4]'
>
> This is with ubuntu 12.04 cross compiler (gcc version 4.6.3
> (Ubuntu/Linaro 4.6.3-1ubuntu5)).
>
> This register restriction is on ARM, but not Thumb builds. Comparing
> this to atomic64_cmpxchg, I don't see how to fix this. Perhaps Will or
> Nico have thoughts.
[...]
> > + asm volatile("@ xchg_xen_ulong\n"
> > + "1: ldrexd %0, %H0, [%3]\n"
> > + " strexd %1, %2, %H2, [%3]\n"
> > + " teq %1, #0\n"
> > + " bne 1b"
> > + : "=&r" (oldval), "=&r" (tmp)
> > + : "r" (val), "r" (ptr)
> > + : "memory", "cc");
I also can't immediately see why GCC would allocate oldval to an odd base
register. Can you share your .config please?
Will
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH LINUX v5] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-03-05 3:04 ` Will Deacon
@ 2013-03-05 3:45 ` Ian Campbell
2013-03-05 6:55 ` Rob Herring
1 sibling, 0 replies; 36+ messages in thread
From: Ian Campbell @ 2013-03-05 3:45 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 2013-03-05 at 03:04 +0000, Will Deacon wrote:
> Hi guys,
>
> On Mon, Mar 04, 2013 at 02:45:33AM +0000, Rob Herring wrote:
> > On 02/20/2013 05:48 AM, Ian Campbell wrote:
> > > On ARM we want these to be the same size on 32- and 64-bit.
> > >
> > > This is an ABI change on ARM. X86 does not change.
> > >
> > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > > Cc: Jan Beulich <JBeulich@suse.com>
> > > Cc: Keir (Xen.org) <keir@xen.org>
> > > Cc: Tim Deegan <tim@xen.org>
> > > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > Cc: linux-arm-kernel at lists.infradead.org
> > > Cc: xen-devel at lists.xen.org
> > > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>
> [...]
>
> > I'm seeing some some build failures on randconfig builds with this change:
> >
> > /tmp/ccJaIZOW.s: Assembler messages:
> > /tmp/ccJaIZOW.s:831: Error: even register required -- `ldrexd r5,r6,[r4]'
> >
> > This is with ubuntu 12.04 cross compiler (gcc version 4.6.3
> > (Ubuntu/Linaro 4.6.3-1ubuntu5)).
> >
> > This register restriction is on ARM, but not Thumb builds. Comparing
> > this to atomic64_cmpxchg, I don't see how to fix this. Perhaps Will or
> > Nico have thoughts.
>
> [...]
>
> > > + asm volatile("@ xchg_xen_ulong\n"
> > > + "1: ldrexd %0, %H0, [%3]\n"
> > > + " strexd %1, %2, %H2, [%3]\n"
> > > + " teq %1, #0\n"
> > > + " bne 1b"
> > > + : "=&r" (oldval), "=&r" (tmp)
> > > + : "r" (val), "r" (ptr)
> > > + : "memory", "cc");
>
> I also can't immediately see why GCC would allocate oldval to an odd base
> register. Can you share your .config please?
I fixed something along these lines before v5 of this patch, although I
must confess I don't recall what it was that I changed (and looking at
the older versions of the patch isn't giving me any clues). I can't
reproduce it now though :-(
I'm using the 4.6.3 cross compiler from kernel.org:
ftp://ftp.kernel.org/pub/tools/crosstool/files/bin/x86_64/4.6.3/
Ian.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH LINUX v5] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-03-04 2:45 ` Rob Herring
2013-03-05 3:04 ` Will Deacon
@ 2013-03-05 3:56 ` Ian Campbell
2013-03-05 14:04 ` Konrad Rzeszutek Wilk
1 sibling, 1 reply; 36+ messages in thread
From: Ian Campbell @ 2013-03-05 3:56 UTC (permalink / raw)
To: linux-arm-kernel
> > diff --git a/arch/arm/include/asm/xen/events.h b/arch/arm/include/asm/xen/events.h
> > index 94b4e90..5c27696 100644
> > --- a/arch/arm/include/asm/xen/events.h
> > +++ b/arch/arm/include/asm/xen/events.h
> > @@ -15,4 +15,26 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
> > return raw_irqs_disabled_flags(regs->ARM_cpsr);
> > }
> >
> > +/*
> > + * We cannot use xchg because it does not support 8-byte
> > + * values. However it is safe to use {ldr,dtd}exd directly because all
> > + * platforms which Xen can run on support those instructions.
>
> Why does atomic64_cmpxchg not work here?
Just that we don't want/need the cmp aspect, we don't mind if an extra
bit gets set as we read the value, so long as we atomically read and set
to zero.
> > + */
> > +static inline xen_ulong_t xchg_xen_ulong(xen_ulong_t *ptr, xen_ulong_t val)
> > +{
> > + xen_ulong_t oldval;
> > + unsigned int tmp;
> > +
> > + wmb();
>
> Based on atomic64_cmpxchg implementation, you could use smp_mb here
> which avoids an outer cache flush.
Good point.
> > + asm volatile("@ xchg_xen_ulong\n"
> > + "1: ldrexd %0, %H0, [%3]\n"
> > + " strexd %1, %2, %H2, [%3]\n"
> > + " teq %1, #0\n"
> > + " bne 1b"
> > + : "=&r" (oldval), "=&r" (tmp)
> > + : "r" (val), "r" (ptr)
> > + : "memory", "cc");
>
> And a smp_mb is needed here.
I think for the specific caller which we have here it isn't strictly
necessary, but for generic correctness I think you are right.
Thanks for reviewing.
Konrad, IIRC you have already picked this up (and sent to Linus?) so an
incremental fix is required? See below.
Ian.
8<------------------------------------
>From 4ed928274dad4c3ed610e769b2ae11eb2d1ea433 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ijc@hellion.org.uk>
Date: Tue, 5 Mar 2013 03:37:23 +0000
Subject: [PATCH] arm: xen: correct barriers in xchg_xen_ulong
We can use an smp_wmb rather than a wmb here and we also need one after the
exchange. Spotted by Rob Herring.
Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
---
arch/arm/include/asm/xen/events.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm/include/asm/xen/events.h b/arch/arm/include/asm/xen/events.h
index 5c27696..0e1f59e 100644
--- a/arch/arm/include/asm/xen/events.h
+++ b/arch/arm/include/asm/xen/events.h
@@ -25,7 +25,7 @@ static inline xen_ulong_t xchg_xen_ulong(xen_ulong_t *ptr, xen_ulong_t val)
xen_ulong_t oldval;
unsigned int tmp;
- wmb();
+ smp_wmb();
asm volatile("@ xchg_xen_ulong\n"
"1: ldrexd %0, %H0, [%3]\n"
" strexd %1, %2, %H2, [%3]\n"
@@ -34,6 +34,7 @@ static inline xen_ulong_t xchg_xen_ulong(xen_ulong_t *ptr, xen_ulong_t val)
: "=&r" (oldval), "=&r" (tmp)
: "r" (val), "r" (ptr)
: "memory", "cc");
+ smp_wmb();
return oldval;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH LINUX v5] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-03-05 3:04 ` Will Deacon
2013-03-05 3:45 ` Ian Campbell
@ 2013-03-05 6:55 ` Rob Herring
2013-03-05 7:03 ` Ian Campbell
2013-03-05 8:08 ` Will Deacon
1 sibling, 2 replies; 36+ messages in thread
From: Rob Herring @ 2013-03-05 6:55 UTC (permalink / raw)
To: linux-arm-kernel
On 03/04/2013 09:04 PM, Will Deacon wrote:
> Hi guys,
>
> On Mon, Mar 04, 2013 at 02:45:33AM +0000, Rob Herring wrote:
>> On 02/20/2013 05:48 AM, Ian Campbell wrote:
>>> On ARM we want these to be the same size on 32- and 64-bit.
>>>
>>> This is an ABI change on ARM. X86 does not change.
>>>
>>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
>>> Cc: Jan Beulich <JBeulich@suse.com>
>>> Cc: Keir (Xen.org) <keir@xen.org>
>>> Cc: Tim Deegan <tim@xen.org>
>>> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>> Cc: linux-arm-kernel at lists.infradead.org
>>> Cc: xen-devel at lists.xen.org
>>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>
> [...]
>
>> I'm seeing some some build failures on randconfig builds with this change:
>>
>> /tmp/ccJaIZOW.s: Assembler messages:
>> /tmp/ccJaIZOW.s:831: Error: even register required -- `ldrexd r5,r6,[r4]'
>>
>> This is with ubuntu 12.04 cross compiler (gcc version 4.6.3
>> (Ubuntu/Linaro 4.6.3-1ubuntu5)).
>>
>> This register restriction is on ARM, but not Thumb builds. Comparing
>> this to atomic64_cmpxchg, I don't see how to fix this. Perhaps Will or
>> Nico have thoughts.
>
> [...]
>
>>> + asm volatile("@ xchg_xen_ulong\n"
>>> + "1: ldrexd %0, %H0, [%3]\n"
>>> + " strexd %1, %2, %H2, [%3]\n"
>>> + " teq %1, #0\n"
>>> + " bne 1b"
>>> + : "=&r" (oldval), "=&r" (tmp)
>>> + : "r" (val), "r" (ptr)
>>> + : "memory", "cc");
>
> I also can't immediately see why GCC would allocate oldval to an odd base
> register. Can you share your .config please?
>
Here's a config:
CONFIG_SYSVIPC=y
CONFIG_POSIX_MQUEUE=y
CONFIG_FHANDLE=y
CONFIG_IKCONFIG=y
CONFIG_CGROUP_FREEZER=y
CONFIG_CGROUP_DEVICE=y
CONFIG_CPUSETS=y
# CONFIG_PROC_PID_CPUSET is not set
CONFIG_CGROUP_CPUACCT=y
CONFIG_RESOURCE_COUNTERS=y
CONFIG_MEMCG=y
CONFIG_MEMCG_KMEM=y
CONFIG_MEMCG_DEBUG_ASYNC_DESTROY=y
CONFIG_CHECKPOINT_RESTORE=y
CONFIG_UIDGID_STRICT_TYPE_CHECKS=y
CONFIG_SCHED_AUTOGROUP=y
CONFIG_RELAY=y
CONFIG_BLK_DEV_INITRD=y
CONFIG_RD_BZIP2=y
CONFIG_RD_LZMA=y
CONFIG_RD_XZ=y
CONFIG_RD_LZO=y
CONFIG_CC_OPTIMIZE_FOR_SIZE=y
# CONFIG_PRINTK is not set
# CONFIG_BUG is not set
# CONFIG_BASE_FULL is not set
# CONFIG_EPOLL is not set
# CONFIG_TIMERFD is not set
# CONFIG_SHMEM is not set
# CONFIG_AIO is not set
CONFIG_EMBEDDED=y
# CONFIG_PERF_EVENTS is not set
CONFIG_PROFILING=y
# CONFIG_BLOCK is not set
CONFIG_ARCH_BCM=y
CONFIG_GPIO_PCA953X=y
CONFIG_ARCH_MXC=y
CONFIG_MACH_IMX51_DT=y
CONFIG_MACH_EUKREA_CPUIMX51SD=y
CONFIG_SOC_IMX53=y
CONFIG_ARCH_SOCFPGA=y
CONFIG_ARCH_SUNXI=y
CONFIG_ARCH_VEXPRESS_CA9X4=y
CONFIG_CPU_ICACHE_DISABLE=y
CONFIG_CPU_DCACHE_DISABLE=y
CONFIG_ARM_ERRATA_430973=y
CONFIG_PL310_ERRATA_727915=y
CONFIG_HAVE_ARM_ARCH_TIMER=y
# CONFIG_COMPACTION is not set
# CONFIG_CROSS_MEMORY_ATTACH is not set
CONFIG_SECCOMP=y
CONFIG_CC_STACKPROTECTOR=y
CONFIG_XEN=y
# CONFIG_ATAGS is not set
CONFIG_ARM_APPENDED_DTB=y
CONFIG_ARM_ATAG_DTB_COMPAT=y
CONFIG_KEXEC=y
CONFIG_CRASH_DUMP=y
CONFIG_FPE_FASTFPE=y
# CONFIG_BINFMT_ELF is not set
CONFIG_PM_AUTOSLEEP=y
CONFIG_PM_RUNTIME=y
CONFIG_PM_DEBUG=y
CONFIG_APM_EMULATION=y
CONFIG_NET=y
CONFIG_XFRM_SUB_POLICY=y
CONFIG_XFRM_MIGRATE=y
CONFIG_NET_KEY=y
CONFIG_NETWORK_PHY_TIMESTAMPING=y
CONFIG_NETFILTER=y
CONFIG_NETFILTER_DEBUG=y
CONFIG_DECNET_NF_GRABULATOR=y
CONFIG_ATM=y
CONFIG_BRIDGE=y
CONFIG_BRIDGE_VLAN_FILTERING=y
CONFIG_VLAN_8021Q=y
CONFIG_DECNET=y
CONFIG_DECNET_ROUTER=y
CONFIG_LLC2=y
CONFIG_ATALK=y
CONFIG_DEV_APPLETALK=y
CONFIG_LAPB=y
CONFIG_NET_SCHED=y
CONFIG_NET_SCH_HTB=y
CONFIG_NET_SCH_HFSC=y
CONFIG_NET_SCH_ATM=y
CONFIG_NET_SCH_PRIO=y
CONFIG_NET_SCH_MULTIQ=y
CONFIG_NET_SCH_SFB=y
CONFIG_NET_SCH_TBF=y
CONFIG_NET_SCH_GRED=y
CONFIG_NET_SCH_NETEM=y
CONFIG_NET_SCH_CHOKE=y
CONFIG_NET_SCH_QFQ=y
CONFIG_NET_SCH_CODEL=y
CONFIG_NET_CLS_TCINDEX=y
CONFIG_NET_CLS_U32=y
CONFIG_NET_CLS_RSVP=y
CONFIG_NET_CLS_ACT=y
CONFIG_NET_ACT_SKBEDIT=y
CONFIG_NET_CLS_IND=y
CONFIG_DCB=y
CONFIG_VSOCKETS=y
CONFIG_BT=y
CONFIG_BT_BNEP=y
CONFIG_BT_BNEP_PROTO_FILTER=y
CONFIG_BT_HCIUART=y
CONFIG_BT_HCIUART_BCSP=y
CONFIG_BT_HCIUART_ATH3K=y
CONFIG_BT_HCIUART_LL=y
CONFIG_BT_HCIBPA10X=y
CONFIG_BT_HCIBFUSB=y
# CONFIG_WIRELESS is not set
CONFIG_WIMAX=y
CONFIG_RFKILL_REGULATOR=y
CONFIG_CAIF=y
CONFIG_CAIF_DEBUG=y
CONFIG_DEVTMPFS=y
# CONFIG_STANDALONE is not set
# CONFIG_FW_LOADER_USER_HELPER is not set
CONFIG_DEBUG_DRIVER=y
CONFIG_CONNECTOR=y
# CONFIG_PROC_EVENTS is not set
CONFIG_ATMEL_PWM=y
CONFIG_ENCLOSURE_SERVICES=y
CONFIG_ISL29003=y
CONFIG_SENSORS_BH1770=y
CONFIG_ARM_CHARLCD=y
CONFIG_BMP085_I2C=y
CONFIG_USB_SWITCH_FSA9480=y
CONFIG_EEPROM_AT24=y
CONFIG_TI_ST=y
CONFIG_ALTERA_STAPL=y
CONFIG_NETDEVICES=y
CONFIG_DUMMY=y
CONFIG_IFB=y
CONFIG_MACVLAN=y
CONFIG_NETCONSOLE=y
CONFIG_VIRTIO_NET=y
CONFIG_CAIF_HSI=y
CONFIG_NET_DSA_MV88E6060=y
# CONFIG_ETHERNET is not set
CONFIG_AMD_PHY=y
CONFIG_MARVELL_PHY=y
CONFIG_DAVICOM_PHY=y
CONFIG_QSEMI_PHY=y
CONFIG_VITESSE_PHY=y
CONFIG_BCM87XX_PHY=y
CONFIG_ICPLUS_PHY=y
CONFIG_REALTEK_PHY=y
CONFIG_NATIONAL_PHY=y
CONFIG_LSI_ET1011C_PHY=y
CONFIG_FIXED_PHY=y
CONFIG_PPP=y
CONFIG_PPP_BSDCOMP=y
CONFIG_PPP_DEFLATE=y
CONFIG_PPP_MPPE=y
CONFIG_PPP_ASYNC=y
CONFIG_PPP_SYNC_TTY=y
CONFIG_USB_KAWETH=y
CONFIG_USB_PEGASUS=y
CONFIG_USB_RTL8150=y
CONFIG_USB_USBNET=y
# CONFIG_USB_NET_AX8817X is not set
CONFIG_USB_NET_CDC_EEM=y
CONFIG_USB_NET_SMSC75XX=y
CONFIG_USB_NET_GL620A=y
CONFIG_USB_NET_MCS7830=y
# CONFIG_USB_NET_CDC_SUBSET is not set
# CONFIG_USB_NET_ZAURUS is not set
CONFIG_USB_NET_CX82310_ETH=y
CONFIG_USB_NET_KALMIA=y
CONFIG_USB_NET_INT51X1=y
# CONFIG_WLAN is not set
# CONFIG_XEN_NETDEV_FRONTEND is not set
CONFIG_ISDN=y
CONFIG_ISDN_I4L=y
CONFIG_ISDN_CAPI=y
# CONFIG_ISDN_DRV_AVMB1_VERBOSE_REASON is not set
# CONFIG_CAPI_TRACE is not set
CONFIG_ISDN_CAPI_CAPI20=y
CONFIG_CAPI_AVM=y
CONFIG_CAPI_EICON=y
CONFIG_ISDN_DRV_GIGASET=y
CONFIG_GIGASET_BASE=y
CONFIG_GIGASET_M105=y
CONFIG_GIGASET_M101=y
CONFIG_GIGASET_DEBUG=y
CONFIG_MISDN=y
# CONFIG_INPUT_MOUSEDEV is not set
CONFIG_INPUT_JOYDEV=y
CONFIG_INPUT_EVDEV=y
CONFIG_INPUT_EVBUG=y
CONFIG_INPUT_APMPOWER=y
# CONFIG_INPUT_KEYBOARD is not set
# CONFIG_INPUT_MOUSE is not set
CONFIG_SERIO_LIBPS2=y
CONFIG_SERIO_RAW=y
CONFIG_SERIO_ALTERA_PS2=y
CONFIG_SERIO_PS2MULT=y
# CONFIG_UNIX98_PTYS is not set
# CONFIG_LEGACY_PTYS is not set
CONFIG_SERIAL_NONSTANDARD=y
CONFIG_N_GSM=y
CONFIG_TRACE_SINK=y
CONFIG_STALDRV=y
CONFIG_SERIAL_AMBA_PL011=y
CONFIG_SERIAL_AMBA_PL011_CONSOLE=y
CONFIG_SERIAL_SCCNXP=y
CONFIG_SERIAL_SCCNXP_CONSOLE=y
CONFIG_SERIAL_ALTERA_UART=y
CONFIG_SERIAL_ALTERA_UART_CONSOLE=y
CONFIG_SERIAL_XILINX_PS_UART=y
CONFIG_HVC_DCC=y
CONFIG_VIRTIO_CONSOLE=y
CONFIG_IPMI_HANDLER=y
CONFIG_IPMI_DEVICE_INTERFACE=y
CONFIG_HW_RANDOM_TIMERIOMEM=y
CONFIG_HW_RANDOM_VIRTIO=y
# CONFIG_HW_RANDOM_TPM is not set
CONFIG_R3964=y
CONFIG_TCG_TIS_I2C_INFINEON=y
CONFIG_I2C_MUX=y
CONFIG_I2C_MUX_PINCTRL=y
# CONFIG_I2C_HELPER_AUTO is not set
CONFIG_I2C_CBUS_GPIO=y
CONFIG_I2C_NOMADIK=y
CONFIG_I2C_OCORES=y
CONFIG_I2C_PCA_PLATFORM=y
CONFIG_I2C_SIMTEC=y
CONFIG_I2C_TAOS_EVM=y
CONFIG_I2C_TINY_USB=y
CONFIG_I2C_DEBUG_CORE=y
CONFIG_HSI=y
CONFIG_NTP_PPS=y
CONFIG_PPS_CLIENT_KTIMER=y
CONFIG_PPS_CLIENT_LDISC=y
CONFIG_PTP_1588_CLOCK_PCH=y
CONFIG_PINCTRL_SINGLE=y
CONFIG_PINCTRL_EXYNOS5440=y
CONFIG_GPIO_SYSFS=y
CONFIG_GPIO_MAX732X=y
CONFIG_GPIO_MAX732X_IRQ=y
CONFIG_GPIO_SX150X=y
CONFIG_GPIO_STMPE=y
CONFIG_GPIO_TWL4030=y
CONFIG_GPIO_WM831X=y
CONFIG_GPIO_WM8350=y
CONFIG_GPIO_ADP5520=y
CONFIG_GPIO_ADP5588=y
CONFIG_GPIO_ADNP=y
CONFIG_GPIO_MCP23S08=y
CONFIG_GPIO_TPS6586X=y
CONFIG_GPIO_VIPERBOARD=y
CONFIG_HWMON_DEBUG_CHIP=y
CONFIG_SENSORS_AD7418=y
CONFIG_SENSORS_ADM1025=y
CONFIG_SENSORS_ADT7410=y
CONFIG_SENSORS_ADT7462=y
CONFIG_SENSORS_ADT7470=y
CONFIG_SENSORS_ASC7621=y
CONFIG_SENSORS_DS620=y
CONFIG_SENSORS_F71805F=y
CONFIG_SENSORS_F75375S=y
CONFIG_SENSORS_GL518SM=y
CONFIG_SENSORS_GL520SM=y
CONFIG_SENSORS_GPIO_FAN=y
CONFIG_SENSORS_HIH6130=y
CONFIG_SENSORS_IBMAEM=y
CONFIG_SENSORS_IBMPEX=y
CONFIG_SENSORS_IT87=y
CONFIG_SENSORS_LM63=y
CONFIG_SENSORS_LM73=y
CONFIG_SENSORS_LM77=y
CONFIG_SENSORS_LM80=y
CONFIG_SENSORS_LM83=y
CONFIG_SENSORS_LM92=y
CONFIG_SENSORS_LTC4215=y
CONFIG_SENSORS_LM95241=y
CONFIG_SENSORS_MAX6639=y
CONFIG_SENSORS_MAX6642=y
CONFIG_SENSORS_MAX6650=y
CONFIG_SENSORS_MAX6697=y
CONFIG_SENSORS_MCP3021=y
CONFIG_SENSORS_NTC_THERMISTOR=y
CONFIG_SENSORS_PCF8591=y
CONFIG_PMBUS=y
# CONFIG_SENSORS_PMBUS is not set
CONFIG_SENSORS_ADM1275=y
CONFIG_SENSORS_LM25066=y
CONFIG_SENSORS_LTC2978=y
CONFIG_SENSORS_MAX34440=y
CONFIG_SENSORS_ZL6100=y
CONFIG_SENSORS_SHT15=y
CONFIG_SENSORS_SMM665=y
CONFIG_SENSORS_EMC1403=y
CONFIG_SENSORS_EMC2103=y
CONFIG_SENSORS_EMC6W201=y
CONFIG_SENSORS_SMSC47M1=y
CONFIG_SENSORS_SCH5627=y
CONFIG_SENSORS_SCH5636=y
CONFIG_SENSORS_ADS7828=y
CONFIG_SENSORS_INA2XX=y
CONFIG_SENSORS_TMP102=y
CONFIG_SENSORS_TMP401=y
CONFIG_SENSORS_TMP421=y
CONFIG_SENSORS_W83791D=y
CONFIG_SENSORS_W83793=y
CONFIG_SENSORS_W83795=y
CONFIG_SENSORS_WM831X=y
CONFIG_SENSORS_WM8350=y
CONFIG_SENSORS_MC13783_ADC=y
CONFIG_WATCHDOG=y
CONFIG_SOFT_WATCHDOG=y
CONFIG_ARM_SP805_WATCHDOG=y
CONFIG_TWL4030_WATCHDOG=y
CONFIG_MAX63XX_WATCHDOG=y
CONFIG_IMX2_WDT=y
CONFIG_RETU_WATCHDOG=y
CONFIG_USBPCWATCHDOG=y
CONFIG_SSB=y
CONFIG_SSB_SDIOHOST=y
CONFIG_SSB_SILENT=y
CONFIG_SSB_DRIVER_GPIO=y
CONFIG_BCMA=y
CONFIG_BCMA_DRIVER_GMAC_CMN=y
CONFIG_BCMA_DRIVER_GPIO=y
CONFIG_MFD_88PM860X=y
CONFIG_MFD_88PM805=y
CONFIG_MFD_SM501=y
CONFIG_MFD_SM501_GPIO=y
CONFIG_HTC_EGPIO=y
CONFIG_HTC_PASIC3=y
CONFIG_HTC_I2CPLD=y
CONFIG_TPS6507X=y
CONFIG_MFD_TPS65217=y
CONFIG_MFD_TPS6586X=y
CONFIG_TWL4030_CORE=y
CONFIG_TWL4030_POWER=y
CONFIG_TWL6040_CORE=y
CONFIG_MFD_STMPE=y
# CONFIG_STMPE_I2C is not set
CONFIG_MFD_T7L66XB=y
CONFIG_MFD_TC6387XB=y
CONFIG_MFD_TC6393XB=y
CONFIG_PMIC_ADP5520=y
CONFIG_MFD_LP8788=y
CONFIG_MFD_MAX77686=y
CONFIG_MFD_MAX77693=y
CONFIG_MFD_MAX8998=y
CONFIG_MFD_SEC_CORE=y
CONFIG_MFD_WM831X_I2C=y
CONFIG_MFD_WM8350_I2C=y
CONFIG_MFD_PCF50633=y
CONFIG_PCF50633_ADC=y
CONFIG_MFD_MC13XXX_I2C=y
CONFIG_ABX500_CORE=y
CONFIG_AB3100_CORE=y
CONFIG_MFD_WL1273_CORE=y
CONFIG_MFD_VIPERBOARD=y
CONFIG_MFD_RETU=y
CONFIG_MFD_AS3711=y
CONFIG_REGULATOR=y
CONFIG_REGULATOR_DUMMY=y
CONFIG_REGULATOR_USERSPACE_CONSUMER=y
CONFIG_REGULATOR_AD5398=y
CONFIG_REGULATOR_MC13892=y
CONFIG_REGULATOR_MAX8660=y
CONFIG_REGULATOR_MAX8998=y
CONFIG_REGULATOR_LP872X=y
CONFIG_REGULATOR_LP8755=y
CONFIG_REGULATOR_PCF50633=y
# CONFIG_REGULATOR_AB3100 is not set
CONFIG_REGULATOR_TPS6586X=y
CONFIG_REGULATOR_VEXPRESS=y
CONFIG_REGULATOR_WM831X=y
CONFIG_REGULATOR_AS3711=y
CONFIG_DRM=y
CONFIG_DRM_I2C_CH7006=y
CONFIG_DRM_I2C_SIL164=y
CONFIG_DRM_I2C_NXP_TDA998X=y
CONFIG_DRM_EXYNOS=y
CONFIG_DRM_EXYNOS_DMABUF=y
CONFIG_DRM_EXYNOS_VIDI=y
CONFIG_VIDEO_OUTPUT_CONTROL=y
CONFIG_OF_DISPLAY_TIMING=y
CONFIG_FB_TILEBLITTING=y
CONFIG_FB_ARMCLCD=y
CONFIG_FB_TMIO=y
CONFIG_FB_SMSCUFX=y
CONFIG_FB_GOLDFISH=y
# CONFIG_XEN_FBDEV_FRONTEND is not set
CONFIG_FB_AUO_K190X=y
CONFIG_FB_AUO_K1901=y
CONFIG_EXYNOS_VIDEO=y
CONFIG_BACKLIGHT_LCD_SUPPORT=y
# CONFIG_BACKLIGHT_GENERIC is not set
CONFIG_BACKLIGHT_WM831X=y
CONFIG_BACKLIGHT_ADP5520=y
CONFIG_BACKLIGHT_PCF50633=y
CONFIG_BACKLIGHT_LM3630=y
CONFIG_BACKLIGHT_LP855X=y
CONFIG_BACKLIGHT_LP8788=y
CONFIG_BACKLIGHT_PANDORA=y
CONFIG_BACKLIGHT_TPS65217=y
CONFIG_BACKLIGHT_AS3711=y
CONFIG_FRAMEBUFFER_CONSOLE=y
CONFIG_FRAMEBUFFER_CONSOLE_ROTATION=y
CONFIG_FONTS=y
CONFIG_FONT_8x8=y
CONFIG_FONT_8x16=y
CONFIG_FONT_MINI_4x6=y
CONFIG_FONT_10x18=y
CONFIG_LOGO=y
CONFIG_SOUND=y
CONFIG_HIDRAW=y
CONFIG_HID_A4TECH=y
CONFIG_HID_AUREAL=y
CONFIG_HID_BELKIN=y
CONFIG_HID_CHERRY=y
CONFIG_HID_CYPRESS=y
CONFIG_HID_DRAGONRISE=y
CONFIG_HID_UCLOGIC=y
CONFIG_HID_GYRATION=y
CONFIG_HID_LENOVO_TPKBD=y
CONFIG_HID_LOGITECH=y
CONFIG_HID_LOGITECH_DJ=y
CONFIG_LOGITECH_FF=y
CONFIG_LOGIRUMBLEPAD2_FF=y
CONFIG_HID_MONTEREY=y
CONFIG_HID_PANTHERLORD=y
CONFIG_HID_PETALYNX=y
CONFIG_HID_PICOLCD=y
CONFIG_HID_PICOLCD_BACKLIGHT=y
CONFIG_HID_PRIMAX=y
CONFIG_HID_ROCCAT=y
CONFIG_HID_SAITEK=y
CONFIG_HID_SAMSUNG=y
CONFIG_HID_SONY=y
CONFIG_HID_SPEEDLINK=y
CONFIG_HID_STEELSERIES=y
CONFIG_HID_SUNPLUS=y
CONFIG_HID_TOPSEED=y
CONFIG_HID_THINGM=y
CONFIG_HID_THRUSTMASTER=y
CONFIG_USB=y
CONFIG_USB_DYNAMIC_MINORS=y
CONFIG_USB_SUSPEND=y
CONFIG_USB_OTG=y
# CONFIG_USB_OTG_WHITELIST is not set
CONFIG_USB_DWC3=y
CONFIG_USB_MON=y
CONFIG_USB_WUSB_CBAF=y
CONFIG_USB_WUSB_CBAF_DEBUG=y
CONFIG_USB_OXU210HP_HCD=y
CONFIG_USB_ISP116X_HCD=y
CONFIG_USB_ISP1760_HCD=y
CONFIG_USB_ISP1362_HCD=y
CONFIG_USB_R8A66597_HCD=y
CONFIG_USB_HCD_SSB=y
CONFIG_USB_CHIPIDEA=y
CONFIG_USB_ACM=y
CONFIG_USB_TMC=y
CONFIG_USB_SERIAL=y
CONFIG_USB_SERIAL_GENERIC=y
CONFIG_USB_SERIAL_AIRCABLE=y
CONFIG_USB_SERIAL_ARK3116=y
CONFIG_USB_SERIAL_BELKIN=y
CONFIG_USB_SERIAL_CH341=y
CONFIG_USB_SERIAL_WHITEHEAT=y
CONFIG_USB_SERIAL_DIGI_ACCELEPORT=y
CONFIG_USB_SERIAL_EMPEG=y
CONFIG_USB_SERIAL_IPAQ=y
CONFIG_USB_SERIAL_IR=y
CONFIG_USB_SERIAL_F81232=y
CONFIG_USB_SERIAL_GARMIN=y
CONFIG_USB_SERIAL_KEYSPAN_PDA=y
CONFIG_USB_SERIAL_KEYSPAN=y
CONFIG_USB_SERIAL_KEYSPAN_MPR=y
CONFIG_USB_SERIAL_KEYSPAN_USA28=y
CONFIG_USB_SERIAL_KEYSPAN_USA28X=y
CONFIG_USB_SERIAL_KEYSPAN_USA28XA=y
CONFIG_USB_SERIAL_KEYSPAN_USA28XB=y
CONFIG_USB_SERIAL_KEYSPAN_USA19=y
CONFIG_USB_SERIAL_KEYSPAN_USA18X=y
CONFIG_USB_SERIAL_KEYSPAN_USA19W=y
CONFIG_USB_SERIAL_KEYSPAN_USA19QI=y
CONFIG_USB_SERIAL_KLSI=y
CONFIG_USB_SERIAL_MCT_U232=y
CONFIG_USB_SERIAL_MOS7720=y
CONFIG_USB_SERIAL_MOTOROLA=y
CONFIG_USB_SERIAL_QCAUX=y
CONFIG_USB_SERIAL_SPCP8X5=y
CONFIG_USB_SERIAL_HP4X=y
CONFIG_USB_SERIAL_SAFE=y
CONFIG_USB_SERIAL_SAFE_PADDED=y
CONFIG_USB_SERIAL_SYMBOL=y
CONFIG_USB_SERIAL_TI=y
CONFIG_USB_SERIAL_CYBERJACK=y
CONFIG_USB_SERIAL_XIRCOM=y
CONFIG_USB_SERIAL_OPTION=y
CONFIG_USB_SERIAL_VIVOPAY_SERIAL=y
CONFIG_USB_SERIAL_ZIO=y
CONFIG_USB_SERIAL_ZTE=y
CONFIG_USB_SERIAL_SSU100=y
CONFIG_USB_SERIAL_DEBUG=y
CONFIG_USB_ADUTUX=y
CONFIG_USB_SEVSEG=y
CONFIG_USB_CYPRESS_CY7C63=y
CONFIG_USB_CYTHERM=y
CONFIG_USB_IDMOUSE=y
CONFIG_USB_FTDI_ELAN=y
CONFIG_USB_LD=y
CONFIG_USB_RCAR_PHY=y
CONFIG_USB_GPIO_VBUS=y
CONFIG_USB_MXS_PHY=y
CONFIG_MMC=y
CONFIG_MMC_DEBUG=y
CONFIG_MMC_TEST=y
CONFIG_MMC_ARMMMCI=y
CONFIG_MMC_SDHCI=y
CONFIG_MMC_VUB300=y
CONFIG_MMC_USHC=y
CONFIG_LEDS_88PM860X=y
CONFIG_LEDS_LM3642=y
CONFIG_LEDS_PCA9532=y
CONFIG_LEDS_GPIO=y
CONFIG_LEDS_LP5521=y
CONFIG_LEDS_LP5523=y
CONFIG_LEDS_PCA955X=y
CONFIG_LEDS_PCA9633=y
CONFIG_LEDS_WM8350=y
CONFIG_LEDS_ADP5520=y
CONFIG_LEDS_RENESAS_TPU=y
CONFIG_LEDS_LM355x=y
CONFIG_LEDS_BLINKM=y
CONFIG_LEDS_TRIGGERS=y
CONFIG_LEDS_TRIGGER_TIMER=y
CONFIG_LEDS_TRIGGER_ONESHOT=y
CONFIG_LEDS_TRIGGER_BACKLIGHT=y
CONFIG_LEDS_TRIGGER_GPIO=y
CONFIG_ACCESSIBILITY=y
CONFIG_A11Y_BRAILLE_CONSOLE=y
CONFIG_UIO=y
CONFIG_UIO_PDRV_GENIRQ=y
CONFIG_VIRTIO_BALLOON=y
CONFIG_VIRTIO_MMIO=y
# CONFIG_XEN_BACKEND is not set
# CONFIG_XEN_SYS_HYPERVISOR is not set
# CONFIG_XEN_GNTDEV is not set
CONFIG_STAGING=y
CONFIG_USBIP_CORE=y
CONFIG_USBIP_DEBUG=y
CONFIG_USB_SERIAL_QUATECH2=y
CONFIG_AD799X=y
CONFIG_ADT7410=y
CONFIG_ADT7316=y
CONFIG_ADT7316_I2C=y
CONFIG_AD7152=y
CONFIG_SENSORS_ISL29018=y
CONFIG_IIO_GPIO_TRIGGER=y
CONFIG_TOUCHSCREEN_SYNAPTICS_I2C_RMI4=y
CONFIG_STAGING_MEDIA=y
CONFIG_USB_WPAN_HCD=y
CONFIG_CED1401=y
CONFIG_DRM_IMX=y
CONFIG_DRM_IMX_PARALLEL_DISPLAY=y
CONFIG_DGRP=y
CONFIG_MAILBOX=y
# CONFIG_IOMMU_SUPPORT is not set
CONFIG_VIRT_DRIVERS=y
CONFIG_PM_DEVFREQ=y
CONFIG_DEVFREQ_GOV_PERFORMANCE=y
CONFIG_DEVFREQ_GOV_USERSPACE=y
CONFIG_EXTCON=y
CONFIG_EXTCON_ADC_JACK=y
CONFIG_EXTCON_MAX77693=y
CONFIG_MEMORY=y
CONFIG_IIO=y
CONFIG_IIO_BUFFER_CB=y
CONFIG_AD5380=y
CONFIG_AD5446=y
CONFIG_MAX517=y
CONFIG_MCP4725=y
CONFIG_IIO_ST_GYRO_3AXIS=y
CONFIG_ITG3200=y
CONFIG_INV_MPU6050_IIO=y
CONFIG_IIO_ST_MAGN_3AXIS=y
CONFIG_IPACK_BUS=y
# CONFIG_FILE_LOCKING is not set
# CONFIG_DNOTIFY is not set
# CONFIG_INOTIFY_USER is not set
# CONFIG_PROC_FS is not set
CONFIG_CONFIGFS_FS=y
# CONFIG_MISC_FILESYSTEMS is not set
# CONFIG_NETWORK_FILESYSTEMS is not set
CONFIG_NLS_CODEPAGE_775=y
CONFIG_NLS_CODEPAGE_850=y
CONFIG_NLS_CODEPAGE_862=y
CONFIG_NLS_CODEPAGE_864=y
CONFIG_NLS_CODEPAGE_865=y
CONFIG_NLS_CODEPAGE_866=y
CONFIG_NLS_CODEPAGE_936=y
CONFIG_NLS_ASCII=y
CONFIG_NLS_ISO8859_2=y
CONFIG_NLS_ISO8859_3=y
CONFIG_NLS_ISO8859_4=y
CONFIG_NLS_ISO8859_6=y
CONFIG_NLS_ISO8859_7=y
CONFIG_NLS_ISO8859_14=y
CONFIG_NLS_MAC_CROATIAN=y
CONFIG_NLS_MAC_GAELIC=y
CONFIG_NLS_MAC_GREEK=y
CONFIG_NLS_MAC_ICELAND=y
CONFIG_NLS_MAC_INUIT=y
CONFIG_NLS_MAC_ROMANIAN=y
# CONFIG_ENABLE_MUST_CHECK is not set
CONFIG_READABLE_ASM=y
CONFIG_HEADERS_CHECK=y
CONFIG_DEBUG_SECTION_MISMATCH=y
CONFIG_PANIC_ON_OOPS=y
CONFIG_DETECT_HUNG_TASK=y
CONFIG_DEBUG_OBJECTS=y
CONFIG_DEBUG_OBJECTS_SELFTEST=y
CONFIG_DEBUG_OBJECTS_TIMERS=y
CONFIG_DEBUG_OBJECTS_WORK=y
CONFIG_DEBUG_OBJECTS_PERCPU_COUNTER=y
CONFIG_RT_MUTEX_TESTER=y
CONFIG_LOCK_STAT=y
CONFIG_DEBUG_LOCKDEP=y
CONFIG_DEBUG_MEMORY_INIT=y
CONFIG_TEST_LIST_SORT=y
CONFIG_DEBUG_NOTIFIERS=y
CONFIG_SPARSE_RCU_POINTER=y
CONFIG_DEBUG_FORCE_WEAK_PER_CPU=y
# CONFIG_FTRACE is not set
CONFIG_DMA_API_DEBUG=y
CONFIG_TEST_KSTRTOX=y
CONFIG_STRICT_DEVMEM=y
CONFIG_DEBUG_USER=y
CONFIG_DEBUG_LL=y
CONFIG_PID_IN_CONTEXTIDR=y
CONFIG_SECURITY_DMESG_RESTRICT=y
CONFIG_SECURITY=y
CONFIG_SECURITY_NETWORK_XFRM=y
CONFIG_SECURITY_TOMOYO=y
CONFIG_SECURITY_APPARMOR=y
CONFIG_SECURITY_YAMA=y
CONFIG_SECURITY_YAMA_STACKED=y
CONFIG_IMA=y
# CONFIG_IMA_AUDIT is not set
CONFIG_CRYPTO_USER=y
CONFIG_CRYPTO_GCM=y
CONFIG_CRYPTO_CBC=y
CONFIG_CRYPTO_CTS=y
CONFIG_CRYPTO_PCBC=y
CONFIG_CRYPTO_XTS=y
CONFIG_CRYPTO_VMAC=y
CONFIG_CRYPTO_MD4=y
CONFIG_CRYPTO_RMD128=y
CONFIG_CRYPTO_RMD160=y
CONFIG_CRYPTO_RMD256=y
CONFIG_CRYPTO_RMD320=y
CONFIG_CRYPTO_SHA512=y
CONFIG_CRYPTO_TGR192=y
CONFIG_CRYPTO_ANUBIS=y
CONFIG_CRYPTO_CAST5=y
CONFIG_CRYPTO_DES=y
CONFIG_CRYPTO_SERPENT=y
CONFIG_CRYPTO_DEFLATE=y
CONFIG_CRYPTO_LZO=y
CONFIG_CRC_ITU_T=y
CONFIG_XZ_DEC_X86=y
CONFIG_XZ_DEC_POWERPC=y
CONFIG_XZ_DEC_IA64=y
CONFIG_AVERAGE=y
CONFIG_VIRTUALIZATION=y
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH LINUX v5] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-03-05 6:55 ` Rob Herring
@ 2013-03-05 7:03 ` Ian Campbell
2013-03-05 8:08 ` Will Deacon
1 sibling, 0 replies; 36+ messages in thread
From: Ian Campbell @ 2013-03-05 7:03 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 2013-03-05 at 06:55 +0000, Rob Herring wrote:
> Here's a config:
Was it truncated? oldconfig is asking a ton of questions, far more than
I would expect for something like variance between our HEADs etc.
If I just answer the default to all the questions then I get a .config
with Xen on etc, but I still don't see the issue.
Ian.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH LINUX v5] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-03-05 6:55 ` Rob Herring
2013-03-05 7:03 ` Ian Campbell
@ 2013-03-05 8:08 ` Will Deacon
2013-03-05 9:29 ` Ian Campbell
1 sibling, 1 reply; 36+ messages in thread
From: Will Deacon @ 2013-03-05 8:08 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Mar 05, 2013 at 06:55:46AM +0000, Rob Herring wrote:
> > I also can't immediately see why GCC would allocate oldval to an odd base
> > register. Can you share your .config please?
> >
>
> Here's a config:
[...]
Cheers Rob, that was enough to reproduce for me. The problem is likely that
CONFIG_AEABI=n, so the ABI doesn't actually mandate even base registers for
64-bit values in registers.
Ian -- this would be fixed if you used our atomic64 routines instead of
inventing your own :)
Will
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH LINUX v5] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-03-05 8:08 ` Will Deacon
@ 2013-03-05 9:29 ` Ian Campbell
2013-03-07 3:17 ` Will Deacon
0 siblings, 1 reply; 36+ messages in thread
From: Ian Campbell @ 2013-03-05 9:29 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 2013-03-05 at 08:08 +0000, Will Deacon wrote:
> Cheers Rob, that was enough to reproduce for me. The problem is likely that
> CONFIG_AEABI=n, so the ABI doesn't actually mandate even base registers for
> 64-bit values in registers.
Me too.
> Ian -- this would be fixed if you used our atomic64 routines instead of
> inventing your own :)
I looked and couldn't see an existing 64 bit xchg, was I looking in the
wrong place? Ah, wait, I see atomic64_xchg now. But that needs an
atomic64_t while I have a xen_ulong_t (which == 64 bits on ARM). This is
a kernel<->hypervisor ABI so I can't just change it to an atomic64_t. I
suppose I could cast (see below, untested) but that seems rather icky.
Ian.
diff --git a/arch/arm/include/asm/xen/events.h b/arch/arm/include/asm/xen/events.h
index 0e1f59e..e86a1b3 100644
--- a/arch/arm/include/asm/xen/events.h
+++ b/arch/arm/include/asm/xen/events.h
@@ -2,6 +2,7 @@
#define _ASM_ARM_XEN_EVENTS_H
#include <asm/ptrace.h>
+#include <asm/atomic.h>
enum ipi_vector {
XEN_PLACEHOLDER_VECTOR,
@@ -15,27 +16,6 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
return raw_irqs_disabled_flags(regs->ARM_cpsr);
}
-/*
- * We cannot use xchg because it does not support 8-byte
- * values. However it is safe to use {ldr,dtd}exd directly because all
- * platforms which Xen can run on support those instructions.
- */
-static inline xen_ulong_t xchg_xen_ulong(xen_ulong_t *ptr, xen_ulong_t val)
-{
- xen_ulong_t oldval;
- unsigned int tmp;
-
- smp_wmb();
- asm volatile("@ xchg_xen_ulong\n"
- "1: ldrexd %0, %H0, [%3]\n"
- " strexd %1, %2, %H2, [%3]\n"
- " teq %1, #0\n"
- " bne 1b"
- : "=&r" (oldval), "=&r" (tmp)
- : "r" (val), "r" (ptr)
- : "memory", "cc");
- smp_wmb();
- return oldval;
-}
+#define xchg_xen_ulong(ptr, val) atomic64_xchg((atomic64_t *)(ptr), (val))
#endif /* _ASM_ARM_XEN_EVENTS_H */
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH LINUX v5] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-03-05 3:56 ` Ian Campbell
@ 2013-03-05 14:04 ` Konrad Rzeszutek Wilk
0 siblings, 0 replies; 36+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-03-05 14:04 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Mar 05, 2013 at 03:56:41AM +0000, Ian Campbell wrote:
> > > diff --git a/arch/arm/include/asm/xen/events.h b/arch/arm/include/asm/xen/events.h
> > > index 94b4e90..5c27696 100644
> > > --- a/arch/arm/include/asm/xen/events.h
> > > +++ b/arch/arm/include/asm/xen/events.h
> > > @@ -15,4 +15,26 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
> > > return raw_irqs_disabled_flags(regs->ARM_cpsr);
> > > }
> > >
> > > +/*
> > > + * We cannot use xchg because it does not support 8-byte
> > > + * values. However it is safe to use {ldr,dtd}exd directly because all
> > > + * platforms which Xen can run on support those instructions.
> >
> > Why does atomic64_cmpxchg not work here?
>
> Just that we don't want/need the cmp aspect, we don't mind if an extra
> bit gets set as we read the value, so long as we atomically read and set
> to zero.
>
> > > + */
> > > +static inline xen_ulong_t xchg_xen_ulong(xen_ulong_t *ptr, xen_ulong_t val)
> > > +{
> > > + xen_ulong_t oldval;
> > > + unsigned int tmp;
> > > +
> > > + wmb();
> >
> > Based on atomic64_cmpxchg implementation, you could use smp_mb here
> > which avoids an outer cache flush.
>
> Good point.
>
> > > + asm volatile("@ xchg_xen_ulong\n"
> > > + "1: ldrexd %0, %H0, [%3]\n"
> > > + " strexd %1, %2, %H2, [%3]\n"
> > > + " teq %1, #0\n"
> > > + " bne 1b"
> > > + : "=&r" (oldval), "=&r" (tmp)
> > > + : "r" (val), "r" (ptr)
> > > + : "memory", "cc");
> >
> > And a smp_mb is needed here.
>
> I think for the specific caller which we have here it isn't strictly
> necessary, but for generic correctness I think you are right.
>
> Thanks for reviewing.
>
> Konrad, IIRC you have already picked this up (and sent to Linus?) so an
Yes.
> incremental fix is required? See below.
Why don't I wait a bit and wait until you are back from conferences and
can post a nice series that fixes the smp_wmb() and also the atomic one
and has been run-time tested with Xen on ARM.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH LINUX v5] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-03-05 9:29 ` Ian Campbell
@ 2013-03-07 3:17 ` Will Deacon
2013-03-07 7:17 ` Ian Campbell
0 siblings, 1 reply; 36+ messages in thread
From: Will Deacon @ 2013-03-07 3:17 UTC (permalink / raw)
To: linux-arm-kernel
Hi Ian,
On Tue, Mar 05, 2013 at 09:29:57AM +0000, Ian Campbell wrote:
> On Tue, 2013-03-05 at 08:08 +0000, Will Deacon wrote:
> > Cheers Rob, that was enough to reproduce for me. The problem is likely that
> > CONFIG_AEABI=n, so the ABI doesn't actually mandate even base registers for
> > 64-bit values in registers.
>
> Me too.
>
> > Ian -- this would be fixed if you used our atomic64 routines instead of
> > inventing your own :)
>
> I looked and couldn't see an existing 64 bit xchg, was I looking in the
> wrong place? Ah, wait, I see atomic64_xchg now. But that needs an
> atomic64_t while I have a xen_ulong_t (which == 64 bits on ARM). This is
> a kernel<->hypervisor ABI so I can't just change it to an atomic64_t. I
> suppose I could cast (see below, untested) but that seems rather icky.
You can play some container_of tricks, like we do in cmpxchg64 to get this
right. Alternatively, we could look at an xchg8 implementation which some
other architectures have (although they seem to be 64-bit machines).
Will
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH LINUX v5] xen: event channel arrays are xen_ulong_t and not unsigned long
2013-03-07 3:17 ` Will Deacon
@ 2013-03-07 7:17 ` Ian Campbell
0 siblings, 0 replies; 36+ messages in thread
From: Ian Campbell @ 2013-03-07 7:17 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 2013-03-07 at 03:17 +0000, Will Deacon wrote:
> > I looked and couldn't see an existing 64 bit xchg, was I looking in the
> > wrong place? Ah, wait, I see atomic64_xchg now. But that needs an
> > atomic64_t while I have a xen_ulong_t (which == 64 bits on ARM). This is
> > a kernel<->hypervisor ABI so I can't just change it to an atomic64_t. I
> > suppose I could cast (see below, untested) but that seems rather icky.
>
> You can play some container_of tricks, like we do in cmpxchg64 to get this
> right. Alternatively, we could look at an xchg8 implementation which some
> other architectures have (although they seem to be 64-bit machines).
I went with the container of trick + appropriate Kconfig depends and
sent a patch out a couple of seconds ago.
Thanks for your help/advice!
Ian.
^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2013-03-07 7:17 UTC | newest]
Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-19 14:48 [PATCH V2] xen: event channel arrays are xen_ulong_t and not unsigned long Ian Campbell
2013-02-19 14:49 ` [PATCH LINUX] " Ian Campbell
2013-02-19 17:11 ` Stefano Stabellini
2013-02-19 17:17 ` Ian Campbell
2013-02-19 17:26 ` Tim Deegan
2013-02-19 17:28 ` Ian Campbell
2013-02-19 14:49 ` [PATCH XEN] " Ian Campbell
2013-02-19 16:42 ` Stefano Stabellini
2013-02-21 17:16 ` Ian Campbell
2013-02-21 18:43 ` Keir Fraser
2013-02-22 8:28 ` Ian Campbell
2013-02-22 8:12 ` Jan Beulich
2013-02-22 8:28 ` Ian Campbell
2013-02-22 8:48 ` Jan Beulich
2013-02-22 8:55 ` Ian Campbell
2013-02-22 9:05 ` Jan Beulich
2013-02-19 17:27 ` [PATCH LINUX v3] " Ian Campbell
2013-02-19 17:29 ` [PATCH LINUX v4] " Ian Campbell
2013-02-19 18:12 ` Stefano Stabellini
2013-02-19 18:43 ` Konrad Rzeszutek Wilk
2013-02-20 2:07 ` Konrad Rzeszutek Wilk
2013-02-20 3:09 ` Konrad Rzeszutek Wilk
2013-02-20 9:13 ` Ian Campbell
2013-02-20 11:48 ` [PATCH LINUX v5] " Ian Campbell
2013-02-21 17:11 ` Stefano Stabellini
2013-03-04 2:45 ` Rob Herring
2013-03-05 3:04 ` Will Deacon
2013-03-05 3:45 ` Ian Campbell
2013-03-05 6:55 ` Rob Herring
2013-03-05 7:03 ` Ian Campbell
2013-03-05 8:08 ` Will Deacon
2013-03-05 9:29 ` Ian Campbell
2013-03-07 3:17 ` Will Deacon
2013-03-07 7:17 ` Ian Campbell
2013-03-05 3:56 ` Ian Campbell
2013-03-05 14:04 ` Konrad Rzeszutek Wilk
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).