Ian Campbell
2013-Feb-20 11:48 UTC
[PATCH LINUX v5] xen: event channel arrays are xen_ulong_t and not unsigned long
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@lists.infradead.org
Cc: xen-devel@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
Stefano Stabellini
2013-Feb-21 17:11 UTC
Re: [PATCH LINUX v5] xen: event channel arrays are xen_ulong_t and not unsigned long
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@lists.infradead.org > Cc: xen-devel@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 >
Rob Herring
2013-Mar-04 02:45 UTC
Re: [PATCH LINUX v5] xen: event channel arrays are xen_ulong_t and not unsigned long
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@lists.infradead.org > Cc: xen-devel@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; > +}
Will Deacon
2013-Mar-05 03:04 UTC
Re: [PATCH LINUX v5] xen: event channel arrays are xen_ulong_t and not unsigned long
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@lists.infradead.org > > Cc: xen-devel@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
Ian Campbell
2013-Mar-05 03:45 UTC
Re: [PATCH LINUX v5] xen: event channel arrays are xen_ulong_t and not unsigned long
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@lists.infradead.org > > > Cc: xen-devel@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.
Ian Campbell
2013-Mar-05 03:56 UTC
Re: [PATCH LINUX v5] xen: event channel arrays are xen_ulong_t and not unsigned long
> > 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
Rob Herring
2013-Mar-05 06:55 UTC
Re: [PATCH LINUX v5] xen: event channel arrays are xen_ulong_t and not unsigned long
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@lists.infradead.org >>> Cc: xen-devel@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
Ian Campbell
2013-Mar-05 07:03 UTC
Re: [PATCH LINUX v5] xen: event channel arrays are xen_ulong_t and not unsigned long
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.
Will Deacon
2013-Mar-05 08:08 UTC
Re: [PATCH LINUX v5] xen: event channel arrays are xen_ulong_t and not unsigned long
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
Ian Campbell
2013-Mar-05 09:29 UTC
Re: [PATCH LINUX v5] xen: event channel arrays are xen_ulong_t and not unsigned long
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 */
Konrad Rzeszutek Wilk
2013-Mar-05 14:04 UTC
Re: [PATCH LINUX v5] xen: event channel arrays are xen_ulong_t and not unsigned long
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 anYes.> 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.
Will Deacon
2013-Mar-07 03:17 UTC
Re: [PATCH LINUX v5] xen: event channel arrays are xen_ulong_t and not unsigned long
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
Ian Campbell
2013-Mar-07 07:17 UTC
Re: [PATCH LINUX v5] xen: event channel arrays are xen_ulong_t and not unsigned long
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.