2013-03-04 02:45:41

by Rob Herring

[permalink] [raw]
Subject: 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 <[email protected]>
> Cc: Jan Beulich <[email protected]>
> Cc: Keir (Xen.org) <[email protected]>
> Cc: Tim Deegan <[email protected]>
> Cc: Stefano Stabellini <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: Konrad Rzeszutek Wilk <[email protected]>
> ---
> 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;
> +}


2013-03-05 03:05:18

by Will Deacon

[permalink] [raw]
Subject: 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 <[email protected]>
> > Cc: Jan Beulich <[email protected]>
> > Cc: Keir (Xen.org) <[email protected]>
> > Cc: Tim Deegan <[email protected]>
> > Cc: Stefano Stabellini <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: Konrad Rzeszutek Wilk <[email protected]>

[...]

> 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

2013-03-05 03:48:18

by Ian Campbell

[permalink] [raw]
Subject: 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 <[email protected]>
> > > Cc: Jan Beulich <[email protected]>
> > > Cc: Keir (Xen.org) <[email protected]>
> > > Cc: Tim Deegan <[email protected]>
> > > Cc: Stefano Stabellini <[email protected]>
> > > Cc: [email protected]
> > > Cc: [email protected]
> > > Cc: Konrad Rzeszutek Wilk <[email protected]>
>
> [...]
>
> > 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.

2013-03-05 03:59:37

by Ian Campbell

[permalink] [raw]
Subject: 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 <[email protected]>
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 <[email protected]>
---
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


2013-03-05 06:55:55

by Rob Herring

[permalink] [raw]
Subject: 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 <[email protected]>
>>> Cc: Jan Beulich <[email protected]>
>>> Cc: Keir (Xen.org) <[email protected]>
>>> Cc: Tim Deegan <[email protected]>
>>> Cc: Stefano Stabellini <[email protected]>
>>> Cc: [email protected]
>>> Cc: [email protected]
>>> Cc: Konrad Rzeszutek Wilk <[email protected]>
>
> [...]
>
>> 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

2013-03-05 07:06:39

by Ian Campbell

[permalink] [raw]
Subject: 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.

2013-03-05 08:09:14

by Will Deacon

[permalink] [raw]
Subject: 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

2013-03-05 09:32:45

by Ian Campbell

[permalink] [raw]
Subject: 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 */

2013-03-05 14:06:19

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: 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 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.

2013-03-07 03:17:47

by Will Deacon

[permalink] [raw]
Subject: 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

2013-03-07 07:20:31

by Ian Campbell

[permalink] [raw]
Subject: 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.