|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 1/2] xen: event channel arrays are xen_ulong_t and not unsigned long
On Thu, 14 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@xxxxxxxxxx>
> Cc: Jan Beulich <JBeulich@xxxxxxxx>
> Cc: Keir (Xen.org) <keir@xxxxxxx>
> Cc: Tim Deegan <tim@xxxxxxx>
> Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> ---
> arch/arm/include/asm/xen/events.h | 22 +++++++++++
> arch/x86/include/asm/xen/events.h | 2 +
> drivers/xen/events.c | 75 ++++++++++++++++++++----------------
> include/xen/interface/xen.h | 8 ++--
> 4 files changed, 70 insertions(+), 37 deletions(-)
>
> diff --git a/arch/arm/include/asm/xen/events.h
> b/arch/arm/include/asm/xen/events.h
> index 94b4e90..92dbb89 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 xen_read_evtchn_pending_sel(struct vcpu_info
> *vcpu_info)
> +{
> + xen_ulong_t val;
> + 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" (val), "=&r" (tmp)
> + : "r" (0), "r" (&vcpu_info->evtchn_pending_sel)
> + : "memory", "cc");
> + return val;
> +}
At this point we might as well introduce an xchg64, it is going to be
more useful to us in the future and to other non-Xen people too.
> #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..55edd58 100644
> --- a/arch/x86/include/asm/xen/events.h
> +++ b/arch/x86/include/asm/xen/events.h
> @@ -16,4 +16,6 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
> return raw_irqs_disabled_flags(regs->flags);
> }
>
> +#define xen_read_evtchn_pending_sel(v) xchg(&(v)->evtchn_pending_sel, 0)
> +
> #endif /* _ASM_X86_XEN_EVENTS_H */
> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> index 0be4df3..e90a440 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. */
> @@ -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]));
> }
>
Are you sure that the implementation of sync_test_bit, sync_set_bit,
etc, can cope with a bit > 32?
For example ____atomic_set_bit blatantly (bit & 31) at the beginning of
the function.
> @@ -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,25 +1222,27 @@ 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 " : " ");
>
> @@ -1247,16 +1257,16 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id)
>
> 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");
> }
> }
> @@ -1304,9 +1314,8 @@ static void __xen_evtchn_do_upcall(void)
>
> #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 = xen_read_evtchn_pending_sel(vcpu_info);
>
> start_word_idx = __this_cpu_read(current_word_idx);
> start_bit_idx = __this_cpu_read(current_bit_idx);
> @@ -1355,7 +1364,7 @@ static void __xen_evtchn_do_upcall(void)
> bit_idx = __ffs(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 +1373,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 +1386,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 +1496,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 +1545,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 886a5d8..53ec416 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
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |