* Re: [GIT PULL 2/2] arm64: defconfig: updates for v5.4
From: Arnd Bergmann @ 2019-09-03 16:56 UTC (permalink / raw)
To: Dinh Nguyen; +Cc: SoC Team, arm-soc, Linux ARM
In-Reply-To: <20190819141659.26414-2-dinguyen@kernel.org>
On Mon, Aug 19, 2019 at 4:17 PM Dinh Nguyen <dinguyen@kernel.org> wrote:
> ----------------------------------------------------------------
> arm64 defconfig for v5.4
> - Add CONFIG_DW_WATCHDOG to support the Designware watchdog driver
Pulled into arm/defconfig, thanks!
Arnd
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [GIT PULL] STM32 defconfig changes for v5.4 #1
From: Arnd Bergmann @ 2019-09-03 16:56 UTC (permalink / raw)
To: Alexandre Torgue
Cc: Kevin Hilman, SoC Team, arm-soc, Maxime Coquelin, Olof Johansson,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <b164eaa8-4553-9c0f-0729-2ecc96fbae7a@st.com>
On Fri, Aug 2, 2019 at 4:49 PM Alexandre Torgue <alexandre.torgue@st.com> wrote:
> ----------------------------------------------------------------
> STM32 defconfig updates for v5.4, round 1
>
> Highlights:
> ----------
>
> -Enable FMC2 NAND (used for STM32MP socs)
> -Enable STM32 booster regulator as module
> -Enable STM32 QSPI as module
Fore reference, I had pulled these before my vacation,
but not sent out an email notification.
Arnd
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] [MT6660] Mediatek Smart Amplifier Driver
From: Mark Brown @ 2019-09-03 16:38 UTC (permalink / raw)
To: richtek.jeff.chang
Cc: alsa-devel, linux-kernel, tiwai, lgirdwood, linux-mediatek,
matthias.bgg, perex, linux-arm-kernel
In-Reply-To: <1567494501-3427-1-git-send-email-richtek.jeff.chang@gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 3976 bytes --]
On Tue, Sep 03, 2019 at 03:08:21PM +0800, richtek.jeff.chang@gmail.com wrote:
> --- /dev/null
> +++ b/sound/soc/codecs/mt6660.c
> @@ -0,0 +1,802 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019 MediaTek Inc.
> + */
Please make the entire comment block a C++ comment so things look more
consistent.
> +struct reg_size_table {
> + u32 addr;
> + u8 size;
> +};
> +static const struct reg_size_table mt6660_reg_size_table[] = {
> + { MT6660_REG_HPF1_COEF, 4 },
> + { MT6660_REG_HPF2_COEF, 4 },
> + { MT6660_REG_TDM_CFG3, 2 },
> + { MT6660_REG_RESV17, 2 },
> + { MT6660_REG_RESV23, 2 },
> + { MT6660_REG_SIGMAX, 2 },
> + { MT6660_REG_DEVID, 2},
> + { MT6660_REG_TDM_CFG3, 2},
> + { MT6660_REG_HCLIP_CTRL, 2},
> + { MT6660_REG_DA_GAIN, 2},
> +};
Please talk to your hardware designers about the benefits of consistent
register sizes :/
> +static int32_t mt6660_i2c_update_bits(struct mt6660_chip *chip,
> + uint32_t addr, uint32_t mask, uint32_t data)
> +{
It would be good to implement a regmap rather than open coding
*everything* - it'd give you things like this without needing to open
code them. Providing you don't use the cache code it should cope fine
with variable register sizes.
> +
> +static int mt6660_i2c_init_setting(struct mt6660_chip *chip)
> +{
> + int i, len, ret;
> + const struct codec_reg_val *init_table;
> +
> + init_table = e4_reg_inits;
> + len = ARRAY_SIZE(e4_reg_inits);
> +
> + for (i = 0; i < len; i++) {
> + ret = mt6660_i2c_update_bits(chip, init_table[i].addr,
> + init_table[i].mask, init_table[i].data);
> + if (ret < 0)
> + return ret;
Why are we not using the chip defaults here?
> +static int mt6660_chip_power_on(
> + struct snd_soc_component *component, int on_off)
> +{
> + struct mt6660_chip *chip = (struct mt6660_chip *)
> + snd_soc_component_get_drvdata(component);
> + int ret = 0;
> + unsigned int val;
> +
> + dev_dbg(component->dev, "%s: on_off = %d\n", __func__, on_off);
> + mutex_lock(&chip->var_lock);
> + if (on_off) {
> + if (chip->pwr_cnt == 0) {
> + ret = mt6660_i2c_update_bits(chip,
> + MT6660_REG_SYSTEM_CTRL, 0x01, 0x00);
> + val = mt6660_i2c_read(chip, MT6660_REG_IRQ_STATUS1);
> + dev_info(chip->dev,
> + "%s reg0x05 = 0x%x\n", __func__, val);
> + }
> + chip->pwr_cnt++;
This looks like you're open coding runtime PM stuff? AFAICT the issue
is that you need to write to this register to do any I/O. Just
implement this via the standard runtime PM framework, you'll need to do
something about the register I/O in the controls (ideally in the
framework, it'd be a lot easier if you did have a cache) but you could
cut out this bit.
> +static int mt6660_component_set_bias_level(struct snd_soc_component *component,
> + enum snd_soc_bias_level level)
> +{
> + struct snd_soc_dapm_context *dapm =
> + snd_soc_component_get_dapm(component);
> + int ret;
> + unsigned int val;
> + struct mt6660_chip *chip = snd_soc_component_get_drvdata(component);
> +
> + if (dapm->bias_level == level) {
> + dev_warn(component->dev, "%s: repeat level change\n", __func__);
This shouldn't be a warning.
> +static const struct snd_kcontrol_new mt6660_component_snd_controls[] = {
> + SOC_SINGLE_EXT_TLV("Volume_Ctrl", MT6660_REG_VOL_CTRL, 0, 255,
> + 1, snd_soc_get_volsw, mt6660_component_put_volsw,
> + vol_ctl_tlv),
> + SOC_SINGLE_EXT("WDT_Enable", MT6660_REG_WDT_CTRL, 7, 1, 0,
> + snd_soc_get_volsw, mt6660_component_put_volsw),
These control names should all use standard ALSA control names - on/off
switches should end in Switch, volume controls in Volume.
> + SOC_SINGLE_EXT("I2SLRS", MT6660_REG_DATAO_SEL, 6, 3, 0,
> + snd_soc_get_volsw, mt6660_component_put_volsw),
> + SOC_SINGLE_EXT("I2SDOLS", MT6660_REG_DATAO_SEL, 3, 7, 0,
> + snd_soc_get_volsw, mt6660_component_put_volsw),
> + SOC_SINGLE_EXT("I2SDORS", MT6660_REG_DATAO_SEL, 0, 7, 0,
> + snd_soc_get_volsw, mt6660_component_put_volsw),
What do these controls do?
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 03/10] arm64: atomics: avoid out-of-line ll/sc atomics
From: Will Deacon @ 2019-09-03 16:37 UTC (permalink / raw)
To: Andrew Murray
Cc: mark.rutland, peterz, catalin.marinas, ndesaulniers,
Ard.Biesheuvel, Nathan Chancellor, robin.murphy, linux-arm-kernel
In-Reply-To: <20190903153120.GT9720@e119886-lin.cambridge.arm.com>
On Tue, Sep 03, 2019 at 04:31:20PM +0100, Andrew Murray wrote:
> On Tue, Sep 03, 2019 at 04:15:44PM +0100, Andrew Murray wrote:
> > On Tue, Sep 03, 2019 at 03:45:34PM +0100, Will Deacon wrote:
> > > Does it work if the only thing you change is the toolchain, and use GCC
> > > instead?
> >
> > Yup.
>
> Also this is Clang generation:
>
> ffff8000100f2700 <__ptrace_link>:
> ffff8000100f2700: f9426009 ldr x9, [x0, #1216]
> ffff8000100f2704: 91130008 add x8, x0, #0x4c0
> ffff8000100f2708: eb09011f cmp x8, x9
> ffff8000100f270c: 540002a1 b.ne ffff8000100f2760 <__ptrace_link+0x60> // b.any
> ffff8000100f2710: f9425829 ldr x9, [x1, #1200]
> ffff8000100f2714: 9112c02a add x10, x1, #0x4b0
> ffff8000100f2718: f9000528 str x8, [x9, #8]
> ffff8000100f271c: f9026009 str x9, [x0, #1216]
> ffff8000100f2720: f902640a str x10, [x0, #1224]
> ffff8000100f2724: f9025828 str x8, [x1, #1200]
> ffff8000100f2728: f9024001 str x1, [x0, #1152]
> ffff8000100f272c: b4000162 cbz x2, ffff8000100f2758 <__ptrace_link+0x58>
> ffff8000100f2730: b900985f str wzr, [x2, #152]
> ffff8000100f2734: 14000004 b ffff8000100f2744 <__ptrace_link+0x44>
> ffff8000100f2738: 14000001 b ffff8000100f273c <__ptrace_link+0x3c>
> ffff8000100f273c: 14000006 b ffff8000100f2754 <__ptrace_link+0x54>
> ffff8000100f2740: 14000001 b ffff8000100f2744 <__ptrace_link+0x44>
> ffff8000100f2744: 52800028 mov w8, #0x1 // #1
> ffff8000100f2748: b828005f stadd w8, [x2]
> ffff8000100f274c: f9030002 str x2, [x0, #1536]
> ffff8000100f2750: d65f03c0 ret
> ffff8000100f2754: 140007fd b ffff8000100f4748 <ptrace_check_attach+0xf8>
> ...
>
> This looks like the default path (before we write over it) will take you to
> the LSE code (e.g. ffff8000100f2734). I'm pretty sure this is wrong, or at
> least not what we expected to see. Also why 4 branches?
So I reproduced this with a silly atomic_inc wrapper:
void will_atomic_inc(atomic_t *v)
{
atomic_inc(v);
}
Compiles to:
0000000000000018 <will_atomic_inc>:
18: 14000004 b 28 <will_atomic_inc+0x10>
1c: 14000001 b 20 <will_atomic_inc+0x8>
20: 14000005 b 34 <will_atomic_inc+0x1c>
24: 14000001 b 28 <will_atomic_inc+0x10>
28: 52800028 mov w8, #0x1 // #1
2c: b828001f stadd w8, [x0]
30: d65f03c0 ret
34: 14000027 b d0 <dump_kernel_offset+0x60>
38: d65f03c0 ret
which is going to explode.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH AUTOSEL 5.2 17/23] usb: chipidea: imx: fix EPROBE_DEFER support during driver probe
From: Sasha Levin @ 2019-09-03 16:24 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Peter Chen, Fabio Estevam, André Draszik, Sascha Hauer,
linux-usb, NXP Linux Team, Pengutronix Kernel Team,
Greg Kroah-Hartman, Shawn Guo, linux-arm-kernel
In-Reply-To: <20190903162424.6877-1-sashal@kernel.org>
From: André Draszik <git@andred.net>
If driver probe needs to be deferred, e.g. because ci_hdrc_add_device()
isn't ready yet, this driver currently misbehaves badly:
a) success is still reported to the driver core (meaning a 2nd
probe attempt will never be done), leaving the driver in
a dysfunctional state and the hardware unusable
b) driver remove / shutdown OOPSes:
[ 206.786916] Unable to handle kernel paging request at virtual address fffffdff
[ 206.794148] pgd = 880b9f82
[ 206.796890] [fffffdff] *pgd=abf5e861, *pte=00000000, *ppte=00000000
[ 206.803179] Internal error: Oops: 37 [#1] PREEMPT SMP ARM
[ 206.808581] Modules linked in: wl18xx evbug
[ 206.813308] CPU: 1 PID: 1 Comm: systemd-shutdow Not tainted 4.19.35+gf345c93b4195 #1
[ 206.821053] Hardware name: Freescale i.MX7 Dual (Device Tree)
[ 206.826813] PC is at ci_hdrc_remove_device+0x4/0x20
[ 206.831699] LR is at ci_hdrc_imx_remove+0x20/0xe8
[ 206.836407] pc : [<805cd4b0>] lr : [<805d62cc>] psr: 20000013
[ 206.842678] sp : a806be40 ip : 00000001 fp : 80adbd3c
[ 206.847906] r10: 80b1b794 r9 : 80d5dfe0 r8 : a8192c44
[ 206.853136] r7 : 80db93a0 r6 : a8192c10 r5 : a8192c00 r4 : a93a4a00
[ 206.859668] r3 : 00000000 r2 : a8192ce4 r1 : ffffffff r0 : fffffdfb
[ 206.866201] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
[ 206.873341] Control: 10c5387d Table: a9e0c06a DAC: 00000051
[ 206.879092] Process systemd-shutdow (pid: 1, stack limit = 0xb271353c)
[ 206.885624] Stack: (0xa806be40 to 0xa806c000)
[ 206.889992] be40: a93a4a00 805d62cc a8192c1c a8170e10 a8192c10 8049a490 80d04d08 00000000
[ 206.898179] be60: 00000000 80d0da2c fee1dead 00000000 a806a000 00000058 00000000 80148b08
[ 206.906366] be80: 01234567 80148d8c a9858600 00000000 00000000 00000000 00000000 80d04d08
[ 206.914553] bea0: 00000000 00000000 a82741e0 a9858600 00000024 00000002 a9858608 00000005
[ 206.922740] bec0: 0000001e 8022c058 00000000 00000000 a806bf14 a9858600 00000000 a806befc
[ 206.930927] bee0: a806bf78 00000000 7ee12c30 8022c18c a806bef8 a806befc 00000000 00000001
[ 206.939115] bf00: 00000000 00000024 a806bf14 00000005 7ee13b34 7ee12c68 00000004 7ee13f20
[ 206.947302] bf20: 00000010 7ee12c7c 00000005 7ee12d04 0000000a 76e7dc00 00000001 80d0f140
[ 206.955490] bf40: ab637880 a974de40 60000013 80d0f140 ab6378a0 80d04d08 a8080470 a9858600
[ 206.963677] bf60: a9858600 00000000 00000000 8022c24c 00000000 80144310 00000000 00000000
[ 206.971864] bf80: 80101204 80d04d08 00000000 80d04d08 00000000 00000000 00000003 00000058
[ 206.980051] bfa0: 80101204 80101000 00000000 00000000 fee1dead 28121969 01234567 00000000
[ 206.988237] bfc0: 00000000 00000000 00000003 00000058 00000000 00000000 00000000 00000000
[ 206.996425] bfe0: 0049ffb0 7ee13d58 0048a84b 76f245a6 60000030 fee1dead 00000000 00000000
[ 207.004622] [<805cd4b0>] (ci_hdrc_remove_device) from [<805d62cc>] (ci_hdrc_imx_remove+0x20/0xe8)
[ 207.013509] [<805d62cc>] (ci_hdrc_imx_remove) from [<8049a490>] (device_shutdown+0x16c/0x218)
[ 207.022050] [<8049a490>] (device_shutdown) from [<80148b08>] (kernel_restart+0xc/0x50)
[ 207.029980] [<80148b08>] (kernel_restart) from [<80148d8c>] (sys_reboot+0xf4/0x1f0)
[ 207.037648] [<80148d8c>] (sys_reboot) from [<80101000>] (ret_fast_syscall+0x0/0x54)
[ 207.045308] Exception stack(0xa806bfa8 to 0xa806bff0)
[ 207.050368] bfa0: 00000000 00000000 fee1dead 28121969 01234567 00000000
[ 207.058554] bfc0: 00000000 00000000 00000003 00000058 00000000 00000000 00000000 00000000
[ 207.066737] bfe0: 0049ffb0 7ee13d58 0048a84b 76f245a6
[ 207.071799] Code: ebffffa8 e3a00000 e8bd8010 e92d4010 (e5904004)
[ 207.078021] ---[ end trace be47424e3fd46e9f ]---
[ 207.082647] Kernel panic - not syncing: Fatal exception
[ 207.087894] ---[ end Kernel panic - not syncing: Fatal exception ]---
c) the error path in combination with driver removal causes
imbalanced calls to the clk_*() and pm_()* APIs
a) happens because the original intended return value is
overwritten (with 0) by the return code of
regulator_disable() in ci_hdrc_imx_probe()'s error path
b) happens because ci_pdev is -EPROBE_DEFER, which causes
ci_hdrc_remove_device() to OOPS
Fix a) by being more careful in ci_hdrc_imx_probe()'s error
path and not overwriting the real error code
Fix b) by calling the respective cleanup functions during
remove only when needed (when ci_pdev != NULL, i.e. when
everything was initialised correctly). This also has the
side effect of not causing imbalanced clk_*() and pm_*()
API calls as part of the error code path.
Fixes: 7c8e8909417e ("usb: chipidea: imx: add HSIC support")
Signed-off-by: André Draszik <git@andred.net>
Cc: stable <stable@vger.kernel.org>
CC: Peter Chen <Peter.Chen@nxp.com>
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: Shawn Guo <shawnguo@kernel.org>
CC: Sascha Hauer <s.hauer@pengutronix.de>
CC: Pengutronix Kernel Team <kernel@pengutronix.de>
CC: Fabio Estevam <festevam@gmail.com>
CC: NXP Linux Team <linux-imx@nxp.com>
CC: linux-usb@vger.kernel.org
CC: linux-arm-kernel@lists.infradead.org
CC: linux-kernel@vger.kernel.org
Link: https://lore.kernel.org/r/20190810150758.17694-1-git@andred.net
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/usb/chipidea/ci_hdrc_imx.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index a76708501236d..5faae96735e62 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -453,9 +453,11 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
imx_disable_unprepare_clks(dev);
disable_hsic_regulator:
if (data->hsic_pad_regulator)
- ret = regulator_disable(data->hsic_pad_regulator);
+ /* don't overwrite original ret (cf. EPROBE_DEFER) */
+ regulator_disable(data->hsic_pad_regulator);
if (pdata.flags & CI_HDRC_PMQOS)
pm_qos_remove_request(&data->pm_qos_req);
+ data->ci_pdev = NULL;
return ret;
}
@@ -468,14 +470,17 @@ static int ci_hdrc_imx_remove(struct platform_device *pdev)
pm_runtime_disable(&pdev->dev);
pm_runtime_put_noidle(&pdev->dev);
}
- ci_hdrc_remove_device(data->ci_pdev);
+ if (data->ci_pdev)
+ ci_hdrc_remove_device(data->ci_pdev);
if (data->override_phy_control)
usb_phy_shutdown(data->phy);
- imx_disable_unprepare_clks(&pdev->dev);
- if (data->plat_data->flags & CI_HDRC_PMQOS)
- pm_qos_remove_request(&data->pm_qos_req);
- if (data->hsic_pad_regulator)
- regulator_disable(data->hsic_pad_regulator);
+ if (data->ci_pdev) {
+ imx_disable_unprepare_clks(&pdev->dev);
+ if (data->plat_data->flags & CI_HDRC_PMQOS)
+ pm_qos_remove_request(&data->pm_qos_req);
+ if (data->hsic_pad_regulator)
+ regulator_disable(data->hsic_pad_regulator);
+ }
return 0;
}
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v3 02/11] kselftest: arm64: adds first test and common utils
From: Cristian Marussi @ 2019-09-03 16:08 UTC (permalink / raw)
To: Dave Martin; +Cc: andreyknvl, shuah, linux-kselftest, linux-arm-kernel
In-Reply-To: <20190903153456.GM27757@arm.com>
Hi
On 03/09/2019 16:34, Dave Martin wrote:
> Hi, responding to some non-trivial comments here where re-work isn't
> needed -- so we have the right context for the mail thread.
>
> For any remaining nits, I'll comment on the v5 patch.
>
ok
> On Wed, Aug 28, 2019 at 06:34:09PM +0100, Cristian Marussi wrote:
>> Hi
>>
>> On 13/08/2019 17:24, Dave Martin wrote:
>
> [...]
>
>>>> diff --git a/tools/testing/selftests/arm64/signal/Makefile b/tools/testing/selftests/arm64/signal/Makefile
>
> [...]
>
>>>> +# Guessing as best as we can where the Kernel headers
>>>> +# could have been installed depending on ENV config and
>>>> +# type of invocation.
>>>> +ifeq ($(KBUILD_OUTPUT),)
>>>> +khdr_dir = $(top_srcdir)/usr/include
>>>> +else
>>>> +ifeq (0,$(MAKELEVEL))
>>>> +khdr_dir = $(KBUILD_OUTPUT)/usr/include
>>>> +else
>>>> +# the KSFT preferred location when KBUILD_OUTPUT is set
>>>> +khdr_dir = $(KBUILD_OUTPUT)/kselftest/usr/include
>>>> +endif
>>>> +endif
>>>
>>> When is KBUILD_OUTPUT set / not set?
>>>
>>
>> Depending how the user/CI is configured KSFT installs the kernel
>> headers in different places....here I'm trying to guess where they
>> have been installed by KSFT.
>>
>>>> +
>>>> +CFLAGS += -I$(khdr_dir)
>>>
>>> Do we rely on any non-UAPI headers? If not, the default should probably
>>> be to rely on the system headers (or toolchain default headers) -- i.e.,
>>> add no -I option at all.
>>
>> I only need updated UAPI headers, but I cannot build without this specific -I..
>> that points to the installed kernel headers directory.
>>
>> As an example it fails with: undefined HWCAP_SSBS if I remove the -I
>>
>>>
>>> I'm wondering why none of the other kselftests need this header search
>>> logic.
>>>
>>
>> Well... a lot of KSFT tests has something related to headers search in their Makefiles:
>>
>> ../kcmp/Makefile:CFLAGS += -I../../../../usr/include/
>> ../networking/timestamping/Makefile:CFLAGS += -I../../../../../usr/include
>> ../ipc/Makefile:CFLAGS += -I../../../../usr/include/
>> ../memfd/Makefile:CFLAGS += -I../../../../include/uapi/
>> ../memfd/Makefile:CFLAGS += -I../../../../include/
>> ../memfd/Makefile:CFLAGS += -I../../../../usr/include/
>>
>> which seems aimed at doing the same thing, but it is a broken approach
>> as far as I can see since if KBUILD_OUTPUT is set, KSFT will install the
>> headers accordingly, so that the above static includes won't work anymore.
>>
>> Not sure if I'm missing something here, but my understanding was that
>>
>> - some KSFT requires arch specific bits, usually included within the dedicated kernel
>> headers provided with the source itself and installed with make headers_install.
>>
>> and that
>>
>> - such headers can be found naturally, being included from top level libc headers
>> only if:
>>
>> 1. a fully updated toolchain containing updated headers too is available at CROSS_COMPILE=
>>
>> or
>>
>> 2. proper -I options are specified to the compiler to specify where KSFT installed the
>> kernel headers related to this kernel and its related KSFT testcases
>>
>> or
>>
>> 3. updated kernel headers were installed on top of the available CROSS_COMPILE toolchain
>>
>> or
>>
>> 4. we are building and running natively, so you can install the kernel headers on
>> system default path and those will be searched
>>
>>
>> My 'feeling' would have been that in the KSFT scenario we should try to stick with option 2.,
>> in order to be able to run KSFT and run the related testcases, relying just on the shipped
>> Kernel/KSFT and possibly underlying hw features, but not having any dependencies
>> on the toolchain/libc.
>>
>> My question is: what happens on a CI-somewhere if suddenly there's the need to update
>> the toolchain somehow (fully or partially only the headers) to be able to simply
>> build/run the new KSFT included with this Kernel ?; even if we accept this need to update
>> the toochain, where this CI should get/scrap-from these minimum toolchain requirements ?
>> (in an automated manner)
>>
>> If instead we can agree to stick with 2., I wonder if this locate-headers mechanism which I introduced
>> here should be in charge of the KSFT framework or if there is something broken in my tests: but
>> in these regards similar issues seems to affect KSFT arm64 tags tests queued on arm64/for-next
>>
>> https://lkml.org/lkml/2019/8/23/721
>
> Ack, I think we should stick with option 2 for now, but I agree to keep
> it local to your tests for now to avoid breaking stuff elsewhere.
>
> In general I think that kselftest should always search the installed
> UAPI headers from the containing kernel tree first, since that's the
> best way to ensure the headers are 100% up to date.
>
> This may need wider discussion in order to be deployed more widely
> across kselftest though.
>
Yes I agree, in the meantime in V5 I moved such mechanism (2. add -I$(khdr_src)) into the toplevel
KSFT arm64 Makefile at least so that it transparently works for all arm64 KSFT test families...in fact
in this way now also KSFT tags tests from Andrey compile fine (without a custom -I ../../../)
...not sure if it is the proper fix anyway.
> [...]
>
>>>> diff --git a/tools/testing/selftests/arm64/signal/test_signals.h b/tools/testing/selftests/arm64/signal/test_signals.h
>
> [...]
>
>>>> + * "The infrastructure emulates only the following system register space:
>>>> + * Op0=3, Op1=0, CRn=0, CRm=0,4,5,6,7
>>>> + */
>>>> +#define get_regval(regname, out) \
>>>> + asm volatile("mrs %0, " __stringify(regname) : "=r" (out) :: "memory")
>>>> +
>>>> +/* Regs encoding and masks naming copied in from sysreg.h */
>>>> +#define SYS_ID_AA64MMFR1_EL1 S3_0_C0_C7_1 /* MRS Emulated */
>>>> +#define SYS_ID_AA64MMFR2_EL1 S3_0_C0_C7_2 /* MRS Emulated */
>>>
>>> These ID regs are part of armv8.0-a, so we don't need to use the magic
>>> syntax.
>>> mmm... why I found them in non UAPI headers defined as follows ?
>>
>> arch/arm64/include/asm/sysreg.h:#define SYS_ID_AA64MMFR1_EL1 sys_reg(3, 0, 0, 7, 1)
>>
>> anyway I tried to use nonS3 regular sysreg naming (with a reasonably new compiler:
>>
>> /opt/toolchains/gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu/bin/aarch64-linux-gnu-
>>
>> and it fails (only on id_aa64mmfr2_el1) as follows:
>> /tmp/ccqAyE8P.s: Assembler messages:
>> /tmp/ccoGrnGc.s:1085: Error: selected processor does not support system register name 'id_aa64mmfr2_el1'
>>
>> In fact this seems to remind me (not totally sure) that this was the reason to use such S3 syntax on this
>> sysregs too.
>
> Ah, it looks like ID_AA64MMFR2_EL1 was added from ARMv8.2-A only. My
> bad.
>
> To keep things consistent, I'm fine with keeping the S3_ syntax for
> everything here.
>
>>>> +#define ID_AA64MMFR1_PAN_SHIFT 20
>>>> +#define ID_AA64MMFR2_UAO_SHIFT 4
>
> [...]
>
Not sure if in v5 I fixed only the fixable or left everything as it was...I have to double check.
>>>> diff --git a/tools/testing/selftests/arm64/signal/test_signals_utils.c b/tools/testing/selftests/arm64/signal/test_signals_utils.c
>
> [...]
>
>>>> +static inline bool are_feats_ok(struct tdescr *td)
>>>> +{
>>>> + return td ? td->feats_required == td->feats_supported : 0;
>>>
>>> Should this be something like
>>> (td->feats_required & td->feats_supported) == td->feats_required ?
>>>
>>> Otherwise additional supported features that our test doesn't care about
>>> will cause this check to fail.
>>>
>> Yes better.
>>
>>>
>>> Do we really need to check td?
>>>
>>
>> Overly defensive
>>
>>> assert(foo); followed by dereferincing foo is usually a bit pointless
>>> because you'd get a SIGSEGV anyway.
>>>
>>> However, since the tests generate deliberate SIGSEGVs too this could
>>> be confusing -- in which case, having an explicit assert() here does
>>> no harm.
>>>
>> not sure about which assert you refer here
>
> I was persuading myself that my own comment was unnecessary, so don't
> worry about it. The code is fine as-is.
ok
>
>>>> +}
>>>> +
>>>> +static void default_handler(int signum, siginfo_t *si, void *uc)
>>>> +{
>>>> + if (current->sig_trig && signum == current->sig_trig) {
>>>> + fprintf(stderr, "Handling SIG_TRIG\n");
>>>> + current->triggered = 1;
>>>> + /* ->run was asserted NON-NULL in test_setup() already */
>>>> + current->run(current, si, uc);
>>>> + } else if (signum == SIGILL && !current->initialized) {
>>>> + /*
>>>> + * A SIGILL here while still not initialized means we failed
>>>> + * even to asses the existence of features during init
>>>> + */
>>>> + fprintf(stdout,
>>>> + "Got SIGILL test_init. Marking ALL features UNSUPPORTED.\n");
>>>> + current->feats_supported = 0;
>>>> + } else if (current->sig_ok && signum == current->sig_ok) {
>>>> + /* it's a bug in the test code when this assert fail */
>>>
>>> Why? Is this because sig_ok is considered acceptable only as an effect
>>> of the test -- i.e., we shouldn't see it if the test hasn't been
>>> triggered yet?
>>
>> This assert would like to ensure that when you receive a sig_ok signal,
>> if a sig_trig was defined != 0, the trigger have been in fact used and processed before
>> receiving this sig_ok here: so you didn't define a signal trigger at all, or, if defined
>> it has been fired to arrive here. I'll add some commenting about this.
>
> OK
>
>>>> + assert(!current->sig_trig || current->triggered);
>>>> + fprintf(stderr,
>>>> + "SIG_OK -- SP:%p si_addr@:0x%p si_code:%d token@:0x%p offset:%ld\n",
>>>> + ((ucontext_t *)uc)->uc_mcontext.sp,
>>>> + si->si_addr, si->si_code, current->token,
>>>> + current->token - si->si_addr);
>>>> + /*
>>>> + * fake_sigreturn tests, which have sanity_enabled=1, set, at
>>>> + * the very last time, the token field to the SP address used
>>>> + * to place the fake sigframe: so token==0 means we never made
>>>> + * it to the end, segfaulting well-before, and the test is
>>>> + * possibly broken.
>>>> + */
>>>> + if (!current->sanity_disabled && !current->token) {
>>>> + fprintf(stdout,
>>>> + "current->token ZEROED...test is probably broken!\n");
>>>> + assert(0);
>>>
>>> In case someone builds with -DNDEBUG, should we add abort()?
>>>
>> Well, in such a case all the test suite is mostly compromised anyway.
>> But you are right, I'll add an abort() at least here when broken tests
>> are detected.
>
> I guess you're right. The abort() does no harm, anyway.
> Gone with abort() in v5
> [...]
>
>>>> diff --git a/tools/testing/selftests/arm64/signal/testcases/testcases.c b/tools/testing/selftests/arm64/signal/testcases/testcases.c
>
> [...]
>
>>>> +bool validate_reserved(ucontext_t *uc, size_t resv_sz, char **err)
>>>> +{
>>>> + bool terminated = false;
>>>> + size_t offs = 0;
>>>> + int flags = 0;
>>>> + struct extra_context *extra = NULL;
>>>> + struct _aarch64_ctx *head =
>>>> + (struct _aarch64_ctx *)uc->uc_mcontext.__reserved;
>>>> +
>>>> + if (!err)
>>>> + return false;
>>>> + /* Walk till the end terminator verifying __reserved contents */
>>>> + while (head && !terminated && offs < resv_sz) {
>>>> + if ((uint64_t)head & 0x0fUL) {
>>>> + *err = "Misaligned HEAD";
>>>> + return false;
>>>> + }
>>>> +
>>>> + switch (head->magic) {
>>>> + case 0:
>>>> + if (head->size)
>>>> + *err = "Bad size for MAGIC0";
>>>
>>> Or "terminator". We don't have an actual symbolic name for magic number
>>> 0. (Arguably it would have been nice to have a name, but we managed
>>> without.)
>>
>> ok
>>>
>>>> + else
>>>> + terminated = true;
>>>> + break;
>>>> + case FPSIMD_MAGIC:
>>>> + if (flags & FPSIMD_CTX)
>>>> + *err = "Multiple FPSIMD_MAGIC";
>>>> + else if (head->size !=
>>>> + sizeof(struct fpsimd_context))
>>>> + *err = "Bad size for fpsimd_context";
>>>> + flags |= FPSIMD_CTX;
>>>> + break;
>>>> + case ESR_MAGIC:
>>>> + if (head->size != sizeof(struct esr_context))
>>>> + fprintf(stderr,
>>>> + "Bad size for esr_context is not an error...just ignore.\n");
>>>
>>> Why isn't this an error? Should the kernel ever write an esr_context
>>> with a different size?
>>
>> There is no check on Kernel side:
>>
>> case ESR_MAGIC:
>> /* ignore */
>> break;
>>
>> so I sticked with that, since this function can be used to validate a
>> Kernel originated sigframe or a crafted one which will be passed down
>> to the Kernel.
>
> I see where you're coming from: I'll comment on the v5 patch instead of
> here, to make it easier to track any rework.
>
ok
> [...]
>
>>>> + if (flags & EXTRA_CTX)
>>>> + if (!validate_extra_context(extra, err))
>>>> + return false;
>>>
>>> Can we validate the contents of the extra context too?
>>>
>>> Ideally we can use the same code to check __reserved[] and the extra
>>> context.
>>>
>> Do you mean the content pointed by extra->datap ?
>> This extra_context validation routine is generally under review and fixes in a further
>> arm64/signal SVE extensions patch still to be published (and cleaned up):
>> [kselftest: arm64: adds SVE-related signal test], given that EXTRA_CONTEXT can effectively
>> appear only when SVE related instruction are used properly.
>>
>> Should I introduce this and other extra-context related fixes here instead ?
>> (it is hard to test and debug without any triggering SVE instruction though...)
>
> No, it's fine to exclude it for now.
>
> If there's a plan to add it later, that's good enough for me.
ok
>
> [...]
>
> Cheers
> ---Dave
>
Thanks
Cheers
Cristian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] KVM: arm64: vgic-v4: Move the GICv4 residency flow to be driven by vcpu_load/put
From: Marc Zyngier @ 2019-09-03 15:57 UTC (permalink / raw)
To: linux-arm-kernel, kvmarm, kvm
Cc: Suzuki K Poulose, Andre Przywara, Eric Auger, James Morse,
Andrew Murray, Julien Thierry
When the VHE code was reworked, a lot of the vgic stuff was moved around,
but the GICv4 residency code did stay untouched, meaning that we come
in and out of residency on each flush/sync, which is obviously suboptimal.
To address this, let's move things around a bit:
- Residency entry (flush) moves to vcpu_load
- Residency exit (sync) moves to vcpu_put
- On blocking (entry to WFI), we "put"
- On unblocking (exit from WFI, we "load"
Because these can nest (load/block/put/load/unblock/put, for example),
we now have per-VPE tracking of the residency state.
Additionally, vgic_v4_put gains a "need doorbell" parameter, which only
gets set to true when blocking because of a WFI. This allows a finer
control of the doorbell, which now also gets disabled as soon as
it gets signaled.
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
drivers/irqchip/irq-gic-v4.c | 7 +++-
include/kvm/arm_vgic.h | 4 +--
include/linux/irqchip/arm-gic-v4.h | 2 ++
virt/kvm/arm/arm.c | 12 ++++---
virt/kvm/arm/vgic/vgic-v3.c | 4 +++
virt/kvm/arm/vgic/vgic-v4.c | 55 ++++++++++++++----------------
virt/kvm/arm/vgic/vgic.c | 4 ---
virt/kvm/arm/vgic/vgic.h | 2 --
8 files changed, 48 insertions(+), 42 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v4.c b/drivers/irqchip/irq-gic-v4.c
index 563e87ed0766..45969927cc81 100644
--- a/drivers/irqchip/irq-gic-v4.c
+++ b/drivers/irqchip/irq-gic-v4.c
@@ -141,12 +141,17 @@ static int its_send_vpe_cmd(struct its_vpe *vpe, struct its_cmd_info *info)
int its_schedule_vpe(struct its_vpe *vpe, bool on)
{
struct its_cmd_info info;
+ int ret;
WARN_ON(preemptible());
info.cmd_type = on ? SCHEDULE_VPE : DESCHEDULE_VPE;
- return its_send_vpe_cmd(vpe, &info);
+ ret = its_send_vpe_cmd(vpe, &info);
+ if (!ret)
+ vpe->resident = on;
+
+ return ret;
}
int its_invall_vpe(struct its_vpe *vpe)
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index af4f09c02bf1..4dc58d7a0010 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -396,7 +396,7 @@ int kvm_vgic_v4_set_forwarding(struct kvm *kvm, int irq,
int kvm_vgic_v4_unset_forwarding(struct kvm *kvm, int irq,
struct kvm_kernel_irq_routing_entry *irq_entry);
-void kvm_vgic_v4_enable_doorbell(struct kvm_vcpu *vcpu);
-void kvm_vgic_v4_disable_doorbell(struct kvm_vcpu *vcpu);
+int vgic_v4_load(struct kvm_vcpu *vcpu);
+int vgic_v4_put(struct kvm_vcpu *vcpu, bool need_db);
#endif /* __KVM_ARM_VGIC_H */
diff --git a/include/linux/irqchip/arm-gic-v4.h b/include/linux/irqchip/arm-gic-v4.h
index e6b155713b47..ab1396afe08a 100644
--- a/include/linux/irqchip/arm-gic-v4.h
+++ b/include/linux/irqchip/arm-gic-v4.h
@@ -35,6 +35,8 @@ struct its_vpe {
/* Doorbell interrupt */
int irq;
irq_hw_number_t vpe_db_lpi;
+ /* VPE resident */
+ bool resident;
/* VPE proxy mapping */
int vpe_proxy_event;
/*
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 35a069815baf..4e69268621b6 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -321,20 +321,24 @@ void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu)
/*
* If we're about to block (most likely because we've just hit a
* WFI), we need to sync back the state of the GIC CPU interface
- * so that we have the lastest PMR and group enables. This ensures
+ * so that we have the latest PMR and group enables. This ensures
* that kvm_arch_vcpu_runnable has up-to-date data to decide
* whether we have pending interrupts.
+ *
+ * For the same reason, we want to tell GICv4 that we need
+ * doorbells to be signalled, should an interrupt become pending.
*/
preempt_disable();
kvm_vgic_vmcr_sync(vcpu);
+ vgic_v4_put(vcpu, true);
preempt_enable();
-
- kvm_vgic_v4_enable_doorbell(vcpu);
}
void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu)
{
- kvm_vgic_v4_disable_doorbell(vcpu);
+ preempt_disable();
+ vgic_v4_load(vcpu);
+ preempt_enable();
}
int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
index 8d69f007dd0c..48307a9eb1d8 100644
--- a/virt/kvm/arm/vgic/vgic-v3.c
+++ b/virt/kvm/arm/vgic/vgic-v3.c
@@ -664,6 +664,8 @@ void vgic_v3_load(struct kvm_vcpu *vcpu)
if (has_vhe())
__vgic_v3_activate_traps(vcpu);
+
+ WARN_ON(vgic_v4_load(vcpu));
}
void vgic_v3_vmcr_sync(struct kvm_vcpu *vcpu)
@@ -676,6 +678,8 @@ void vgic_v3_vmcr_sync(struct kvm_vcpu *vcpu)
void vgic_v3_put(struct kvm_vcpu *vcpu)
{
+ WARN_ON(vgic_v4_put(vcpu, false));
+
vgic_v3_vmcr_sync(vcpu);
kvm_call_hyp(__vgic_v3_save_aprs, vcpu);
diff --git a/virt/kvm/arm/vgic/vgic-v4.c b/virt/kvm/arm/vgic/vgic-v4.c
index 477af6aebb97..3a8a28854b13 100644
--- a/virt/kvm/arm/vgic/vgic-v4.c
+++ b/virt/kvm/arm/vgic/vgic-v4.c
@@ -85,6 +85,10 @@ static irqreturn_t vgic_v4_doorbell_handler(int irq, void *info)
{
struct kvm_vcpu *vcpu = info;
+ /* We got the message, no need to fire again */
+ if (!irqd_irq_disabled(&irq_to_desc(irq)->irq_data))
+ disable_irq_nosync(irq);
+
vcpu->arch.vgic_cpu.vgic_v3.its_vpe.pending_last = true;
kvm_make_request(KVM_REQ_IRQ_PENDING, vcpu);
kvm_vcpu_kick(vcpu);
@@ -192,20 +196,30 @@ void vgic_v4_teardown(struct kvm *kvm)
its_vm->vpes = NULL;
}
-int vgic_v4_sync_hwstate(struct kvm_vcpu *vcpu)
+int vgic_v4_put(struct kvm_vcpu *vcpu, bool need_db)
{
- if (!vgic_supports_direct_msis(vcpu->kvm))
+ struct its_vpe *vpe = &vcpu->arch.vgic_cpu.vgic_v3.its_vpe;
+ struct irq_desc *desc = irq_to_desc(vpe->irq);
+
+ if (!vgic_supports_direct_msis(vcpu->kvm) || !vpe->resident)
return 0;
- return its_schedule_vpe(&vcpu->arch.vgic_cpu.vgic_v3.its_vpe, false);
+ /*
+ * If blocking, a doorbell is required. Undo the nested
+ * disable_irq() calls...
+ */
+ while (need_db && irqd_irq_disabled(&desc->irq_data))
+ enable_irq(vpe->irq);
+
+ return its_schedule_vpe(vpe, false);
}
-int vgic_v4_flush_hwstate(struct kvm_vcpu *vcpu)
+int vgic_v4_load(struct kvm_vcpu *vcpu)
{
- int irq = vcpu->arch.vgic_cpu.vgic_v3.its_vpe.irq;
+ struct its_vpe *vpe = &vcpu->arch.vgic_cpu.vgic_v3.its_vpe;
int err;
- if (!vgic_supports_direct_msis(vcpu->kvm))
+ if (!vgic_supports_direct_msis(vcpu->kvm) || vpe->resident)
return 0;
/*
@@ -214,11 +228,14 @@ int vgic_v4_flush_hwstate(struct kvm_vcpu *vcpu)
* doc in drivers/irqchip/irq-gic-v4.c to understand how this
* turns into a VMOVP command at the ITS level.
*/
- err = irq_set_affinity(irq, cpumask_of(smp_processor_id()));
+ err = irq_set_affinity(vpe->irq, cpumask_of(smp_processor_id()));
if (err)
return err;
- err = its_schedule_vpe(&vcpu->arch.vgic_cpu.vgic_v3.its_vpe, true);
+ /* Disabled the doorbell, as we're about to enter the guest */
+ disable_irq(vpe->irq);
+
+ err = its_schedule_vpe(vpe, true);
if (err)
return err;
@@ -226,9 +243,7 @@ int vgic_v4_flush_hwstate(struct kvm_vcpu *vcpu)
* Now that the VPE is resident, let's get rid of a potential
* doorbell interrupt that would still be pending.
*/
- err = irq_set_irqchip_state(irq, IRQCHIP_STATE_PENDING, false);
-
- return err;
+ return irq_set_irqchip_state(vpe->irq, IRQCHIP_STATE_PENDING, false);
}
static struct vgic_its *vgic_get_its(struct kvm *kvm,
@@ -335,21 +350,3 @@ int kvm_vgic_v4_unset_forwarding(struct kvm *kvm, int virq,
mutex_unlock(&its->its_lock);
return ret;
}
-
-void kvm_vgic_v4_enable_doorbell(struct kvm_vcpu *vcpu)
-{
- if (vgic_supports_direct_msis(vcpu->kvm)) {
- int irq = vcpu->arch.vgic_cpu.vgic_v3.its_vpe.irq;
- if (irq)
- enable_irq(irq);
- }
-}
-
-void kvm_vgic_v4_disable_doorbell(struct kvm_vcpu *vcpu)
-{
- if (vgic_supports_direct_msis(vcpu->kvm)) {
- int irq = vcpu->arch.vgic_cpu.vgic_v3.its_vpe.irq;
- if (irq)
- disable_irq(irq);
- }
-}
diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
index 45a870cb63f5..99b02ca730a8 100644
--- a/virt/kvm/arm/vgic/vgic.c
+++ b/virt/kvm/arm/vgic/vgic.c
@@ -857,8 +857,6 @@ void kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu)
{
struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
- WARN_ON(vgic_v4_sync_hwstate(vcpu));
-
/* An empty ap_list_head implies used_lrs == 0 */
if (list_empty(&vcpu->arch.vgic_cpu.ap_list_head))
return;
@@ -882,8 +880,6 @@ static inline void vgic_restore_state(struct kvm_vcpu *vcpu)
/* Flush our emulation state into the GIC hardware before entering the guest. */
void kvm_vgic_flush_hwstate(struct kvm_vcpu *vcpu)
{
- WARN_ON(vgic_v4_flush_hwstate(vcpu));
-
/*
* If there are no virtual interrupts active or pending for this
* VCPU, then there is no work to do and we can bail out without
diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
index 83066a81b16a..c7fefd6b1c80 100644
--- a/virt/kvm/arm/vgic/vgic.h
+++ b/virt/kvm/arm/vgic/vgic.h
@@ -316,7 +316,5 @@ void vgic_its_invalidate_cache(struct kvm *kvm);
bool vgic_supports_direct_msis(struct kvm *kvm);
int vgic_v4_init(struct kvm *kvm);
void vgic_v4_teardown(struct kvm *kvm);
-int vgic_v4_sync_hwstate(struct kvm_vcpu *vcpu);
-int vgic_v4_flush_hwstate(struct kvm_vcpu *vcpu);
#endif
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* PCIe DMA addressing issues on Raspberry Pi 4
From: Nicolas Saenz Julienne @ 2019-09-03 15:43 UTC (permalink / raw)
To: Rob Herring, robin.murphy
Cc: Stefan Wahren, devicetree@vger.kernel.org, mbrugger, iommu,
Christoph Hellwig, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 2489 bytes --]
Hi all, sorry for the long read, I kept it as short as possible.
So, the wrapper around the PCIe block available on the Raspberry Pi 4 has a bug
preventing it from accessing anything beyond the first 3G of ram [1]. I'm
trying to figure out the best way to integrate this upstream.
Note that the only thing behind the PCIe bus is an USB3 chip. The bus is not
exposed to users directly.
I see two options:
- Isolate the PCIe block on it's own interconnect. IMO this could be acceptable
as it's arguable that the bug is not really in the PCIe block. I set the
interconnect's dma-range size to 2GB instead of 3GB as dma masks don't play
well with non power of two DMA constraints:
buggy-scb {
compatible = "simple-bus";
dma-ranges = <0x0 0x00000000 0x0 0x00000000 0x800000000>;
pcie {
compatible = "brcm,bcm2711-pcie";
dma-ranges = <0x02000000 0x0 0x00000000 0x0 0x00000000
0x1 0x00000000>;
[...]
};
};
scb {
compatible = "simple-bus";
dma-ranges = <0x0 0x00000000 0x0 0x00000000 0xfc0000000>;
eth0 {
[...]
};
[...]
};
With this setup the PCIe devices should behave correctly as long as they
don't play with their own DMA masks.
- Configure PCIe's inbound view of memory taking into account the buggy
interconnect:
scb {
compatible = "simple-bus";
dma-ranges = <0x0 0x00000000 0x0 0x00000000 0xfc000000>;
pcie {
compatible = "brcm,bcm2711-pcie";
dma-ranges = <0x02000000 0x0 0x00000000 0x0 0x00000000
0x0 0x80000000>;
[...]
};
eth0 {
[...]
};
[...]
};
The downside of this is that of_dma_configure() doesn't play well with PCI
devices. of_dma_configure() expects a device's OF node, yet the PCI core
passes the bridge's OF node, as the device has none. The result is
of_dma_configure() ignores PCI's dma-ranges. Solving this is not trivial.
IMO the simplest solution would be to create a distinct OF node on PCI
bridges, child of the actual PCI root complex. FYI this was already an issue
some years ago [2].
This solution also suffers from devices setting their own DMA masks.
If you're curious abot the downstream kernel, they use their custom buffer
bouncing code, which AFAIK is something we're trying to get rid of.
Any comments? Alternative solutions?
Thanks,
Nicolas
[1] https://www.spinics.net/lists/arm-kernel/msg740693.html
[2] https://patchwork.kernel.org/patch/9650345/#20294961
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 02/11] kselftest: arm64: adds first test and common utils
From: Dave Martin @ 2019-09-03 15:34 UTC (permalink / raw)
To: Cristian Marussi; +Cc: andreyknvl, shuah, linux-kselftest, linux-arm-kernel
In-Reply-To: <a76adb28-00f9-3c32-60e5-bc746db6f2fd@arm.com>
Hi, responding to some non-trivial comments here where re-work isn't
needed -- so we have the right context for the mail thread.
For any remaining nits, I'll comment on the v5 patch.
On Wed, Aug 28, 2019 at 06:34:09PM +0100, Cristian Marussi wrote:
> Hi
>
> On 13/08/2019 17:24, Dave Martin wrote:
[...]
> >> diff --git a/tools/testing/selftests/arm64/signal/Makefile b/tools/testing/selftests/arm64/signal/Makefile
[...]
> >> +# Guessing as best as we can where the Kernel headers
> >> +# could have been installed depending on ENV config and
> >> +# type of invocation.
> >> +ifeq ($(KBUILD_OUTPUT),)
> >> +khdr_dir = $(top_srcdir)/usr/include
> >> +else
> >> +ifeq (0,$(MAKELEVEL))
> >> +khdr_dir = $(KBUILD_OUTPUT)/usr/include
> >> +else
> >> +# the KSFT preferred location when KBUILD_OUTPUT is set
> >> +khdr_dir = $(KBUILD_OUTPUT)/kselftest/usr/include
> >> +endif
> >> +endif
> >
> > When is KBUILD_OUTPUT set / not set?
> >
>
> Depending how the user/CI is configured KSFT installs the kernel
> headers in different places....here I'm trying to guess where they
> have been installed by KSFT.
>
> >> +
> >> +CFLAGS += -I$(khdr_dir)
> >
> > Do we rely on any non-UAPI headers? If not, the default should probably
> > be to rely on the system headers (or toolchain default headers) -- i.e.,
> > add no -I option at all.
>
> I only need updated UAPI headers, but I cannot build without this specific -I..
> that points to the installed kernel headers directory.
>
> As an example it fails with: undefined HWCAP_SSBS if I remove the -I
>
> >
> > I'm wondering why none of the other kselftests need this header search
> > logic.
> >
>
> Well... a lot of KSFT tests has something related to headers search in their Makefiles:
>
> ../kcmp/Makefile:CFLAGS += -I../../../../usr/include/
> ../networking/timestamping/Makefile:CFLAGS += -I../../../../../usr/include
> ../ipc/Makefile:CFLAGS += -I../../../../usr/include/
> ../memfd/Makefile:CFLAGS += -I../../../../include/uapi/
> ../memfd/Makefile:CFLAGS += -I../../../../include/
> ../memfd/Makefile:CFLAGS += -I../../../../usr/include/
>
> which seems aimed at doing the same thing, but it is a broken approach
> as far as I can see since if KBUILD_OUTPUT is set, KSFT will install the
> headers accordingly, so that the above static includes won't work anymore.
>
> Not sure if I'm missing something here, but my understanding was that
>
> - some KSFT requires arch specific bits, usually included within the dedicated kernel
> headers provided with the source itself and installed with make headers_install.
>
> and that
>
> - such headers can be found naturally, being included from top level libc headers
> only if:
>
> 1. a fully updated toolchain containing updated headers too is available at CROSS_COMPILE=
>
> or
>
> 2. proper -I options are specified to the compiler to specify where KSFT installed the
> kernel headers related to this kernel and its related KSFT testcases
>
> or
>
> 3. updated kernel headers were installed on top of the available CROSS_COMPILE toolchain
>
> or
>
> 4. we are building and running natively, so you can install the kernel headers on
> system default path and those will be searched
>
>
> My 'feeling' would have been that in the KSFT scenario we should try to stick with option 2.,
> in order to be able to run KSFT and run the related testcases, relying just on the shipped
> Kernel/KSFT and possibly underlying hw features, but not having any dependencies
> on the toolchain/libc.
>
> My question is: what happens on a CI-somewhere if suddenly there's the need to update
> the toolchain somehow (fully or partially only the headers) to be able to simply
> build/run the new KSFT included with this Kernel ?; even if we accept this need to update
> the toochain, where this CI should get/scrap-from these minimum toolchain requirements ?
> (in an automated manner)
>
> If instead we can agree to stick with 2., I wonder if this locate-headers mechanism which I introduced
> here should be in charge of the KSFT framework or if there is something broken in my tests: but
> in these regards similar issues seems to affect KSFT arm64 tags tests queued on arm64/for-next
>
> https://lkml.org/lkml/2019/8/23/721
Ack, I think we should stick with option 2 for now, but I agree to keep
it local to your tests for now to avoid breaking stuff elsewhere.
In general I think that kselftest should always search the installed
UAPI headers from the containing kernel tree first, since that's the
best way to ensure the headers are 100% up to date.
This may need wider discussion in order to be deployed more widely
across kselftest though.
[...]
> >> diff --git a/tools/testing/selftests/arm64/signal/test_signals.h b/tools/testing/selftests/arm64/signal/test_signals.h
[...]
> >> + * "The infrastructure emulates only the following system register space:
> >> + * Op0=3, Op1=0, CRn=0, CRm=0,4,5,6,7
> >> + */
> >> +#define get_regval(regname, out) \
> >> + asm volatile("mrs %0, " __stringify(regname) : "=r" (out) :: "memory")
> >> +
> >> +/* Regs encoding and masks naming copied in from sysreg.h */
> >> +#define SYS_ID_AA64MMFR1_EL1 S3_0_C0_C7_1 /* MRS Emulated */
> >> +#define SYS_ID_AA64MMFR2_EL1 S3_0_C0_C7_2 /* MRS Emulated */
> >
> > These ID regs are part of armv8.0-a, so we don't need to use the magic
> > syntax.
> > mmm... why I found them in non UAPI headers defined as follows ?
>
> arch/arm64/include/asm/sysreg.h:#define SYS_ID_AA64MMFR1_EL1 sys_reg(3, 0, 0, 7, 1)
>
> anyway I tried to use nonS3 regular sysreg naming (with a reasonably new compiler:
>
> /opt/toolchains/gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu/bin/aarch64-linux-gnu-
>
> and it fails (only on id_aa64mmfr2_el1) as follows:
> /tmp/ccqAyE8P.s: Assembler messages:
> /tmp/ccoGrnGc.s:1085: Error: selected processor does not support system register name 'id_aa64mmfr2_el1'
>
> In fact this seems to remind me (not totally sure) that this was the reason to use such S3 syntax on this
> sysregs too.
Ah, it looks like ID_AA64MMFR2_EL1 was added from ARMv8.2-A only. My
bad.
To keep things consistent, I'm fine with keeping the S3_ syntax for
everything here.
> >> +#define ID_AA64MMFR1_PAN_SHIFT 20
> >> +#define ID_AA64MMFR2_UAO_SHIFT 4
[...]
> >> diff --git a/tools/testing/selftests/arm64/signal/test_signals_utils.c b/tools/testing/selftests/arm64/signal/test_signals_utils.c
[...]
> >> +static inline bool are_feats_ok(struct tdescr *td)
> >> +{
> >> + return td ? td->feats_required == td->feats_supported : 0;
> >
> > Should this be something like
> > (td->feats_required & td->feats_supported) == td->feats_required ?
> >
> > Otherwise additional supported features that our test doesn't care about
> > will cause this check to fail.
> >
> Yes better.
>
> >
> > Do we really need to check td?
> >
>
> Overly defensive
>
> > assert(foo); followed by dereferincing foo is usually a bit pointless
> > because you'd get a SIGSEGV anyway.
> >
> > However, since the tests generate deliberate SIGSEGVs too this could
> > be confusing -- in which case, having an explicit assert() here does
> > no harm.
> >
> not sure about which assert you refer here
I was persuading myself that my own comment was unnecessary, so don't
worry about it. The code is fine as-is.
> >> +}
> >> +
> >> +static void default_handler(int signum, siginfo_t *si, void *uc)
> >> +{
> >> + if (current->sig_trig && signum == current->sig_trig) {
> >> + fprintf(stderr, "Handling SIG_TRIG\n");
> >> + current->triggered = 1;
> >> + /* ->run was asserted NON-NULL in test_setup() already */
> >> + current->run(current, si, uc);
> >> + } else if (signum == SIGILL && !current->initialized) {
> >> + /*
> >> + * A SIGILL here while still not initialized means we failed
> >> + * even to asses the existence of features during init
> >> + */
> >> + fprintf(stdout,
> >> + "Got SIGILL test_init. Marking ALL features UNSUPPORTED.\n");
> >> + current->feats_supported = 0;
> >> + } else if (current->sig_ok && signum == current->sig_ok) {
> >> + /* it's a bug in the test code when this assert fail */
> >
> > Why? Is this because sig_ok is considered acceptable only as an effect
> > of the test -- i.e., we shouldn't see it if the test hasn't been
> > triggered yet?
>
> This assert would like to ensure that when you receive a sig_ok signal,
> if a sig_trig was defined != 0, the trigger have been in fact used and processed before
> receiving this sig_ok here: so you didn't define a signal trigger at all, or, if defined
> it has been fired to arrive here. I'll add some commenting about this.
OK
> >> + assert(!current->sig_trig || current->triggered);
> >> + fprintf(stderr,
> >> + "SIG_OK -- SP:%p si_addr@:0x%p si_code:%d token@:0x%p offset:%ld\n",
> >> + ((ucontext_t *)uc)->uc_mcontext.sp,
> >> + si->si_addr, si->si_code, current->token,
> >> + current->token - si->si_addr);
> >> + /*
> >> + * fake_sigreturn tests, which have sanity_enabled=1, set, at
> >> + * the very last time, the token field to the SP address used
> >> + * to place the fake sigframe: so token==0 means we never made
> >> + * it to the end, segfaulting well-before, and the test is
> >> + * possibly broken.
> >> + */
> >> + if (!current->sanity_disabled && !current->token) {
> >> + fprintf(stdout,
> >> + "current->token ZEROED...test is probably broken!\n");
> >> + assert(0);
> >
> > In case someone builds with -DNDEBUG, should we add abort()?
> >
> Well, in such a case all the test suite is mostly compromised anyway.
> But you are right, I'll add an abort() at least here when broken tests
> are detected.
I guess you're right. The abort() does no harm, anyway.
[...]
> >> diff --git a/tools/testing/selftests/arm64/signal/testcases/testcases.c b/tools/testing/selftests/arm64/signal/testcases/testcases.c
[...]
> >> +bool validate_reserved(ucontext_t *uc, size_t resv_sz, char **err)
> >> +{
> >> + bool terminated = false;
> >> + size_t offs = 0;
> >> + int flags = 0;
> >> + struct extra_context *extra = NULL;
> >> + struct _aarch64_ctx *head =
> >> + (struct _aarch64_ctx *)uc->uc_mcontext.__reserved;
> >> +
> >> + if (!err)
> >> + return false;
> >> + /* Walk till the end terminator verifying __reserved contents */
> >> + while (head && !terminated && offs < resv_sz) {
> >> + if ((uint64_t)head & 0x0fUL) {
> >> + *err = "Misaligned HEAD";
> >> + return false;
> >> + }
> >> +
> >> + switch (head->magic) {
> >> + case 0:
> >> + if (head->size)
> >> + *err = "Bad size for MAGIC0";
> >
> > Or "terminator". We don't have an actual symbolic name for magic number
> > 0. (Arguably it would have been nice to have a name, but we managed
> > without.)
>
> ok
> >
> >> + else
> >> + terminated = true;
> >> + break;
> >> + case FPSIMD_MAGIC:
> >> + if (flags & FPSIMD_CTX)
> >> + *err = "Multiple FPSIMD_MAGIC";
> >> + else if (head->size !=
> >> + sizeof(struct fpsimd_context))
> >> + *err = "Bad size for fpsimd_context";
> >> + flags |= FPSIMD_CTX;
> >> + break;
> >> + case ESR_MAGIC:
> >> + if (head->size != sizeof(struct esr_context))
> >> + fprintf(stderr,
> >> + "Bad size for esr_context is not an error...just ignore.\n");
> >
> > Why isn't this an error? Should the kernel ever write an esr_context
> > with a different size?
>
> There is no check on Kernel side:
>
> case ESR_MAGIC:
> /* ignore */
> break;
>
> so I sticked with that, since this function can be used to validate a
> Kernel originated sigframe or a crafted one which will be passed down
> to the Kernel.
I see where you're coming from: I'll comment on the v5 patch instead of
here, to make it easier to track any rework.
[...]
> >> + if (flags & EXTRA_CTX)
> >> + if (!validate_extra_context(extra, err))
> >> + return false;
> >
> > Can we validate the contents of the extra context too?
> >
> > Ideally we can use the same code to check __reserved[] and the extra
> > context.
> >
> Do you mean the content pointed by extra->datap ?
> This extra_context validation routine is generally under review and fixes in a further
> arm64/signal SVE extensions patch still to be published (and cleaned up):
> [kselftest: arm64: adds SVE-related signal test], given that EXTRA_CONTEXT can effectively
> appear only when SVE related instruction are used properly.
>
> Should I introduce this and other extra-context related fixes here instead ?
> (it is hard to test and debug without any triggering SVE instruction though...)
No, it's fine to exclude it for now.
If there's a plan to add it later, that's good enough for me.
[...]
Cheers
---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 03/10] arm64: atomics: avoid out-of-line ll/sc atomics
From: Andrew Murray @ 2019-09-03 15:31 UTC (permalink / raw)
To: Will Deacon
Cc: mark.rutland, peterz, catalin.marinas, ndesaulniers,
Ard.Biesheuvel, Nathan Chancellor, robin.murphy, linux-arm-kernel
In-Reply-To: <20190903151544.GS9720@e119886-lin.cambridge.arm.com>
On Tue, Sep 03, 2019 at 04:15:44PM +0100, Andrew Murray wrote:
> On Tue, Sep 03, 2019 at 03:45:34PM +0100, Will Deacon wrote:
> > On Tue, Sep 03, 2019 at 03:31:19PM +0100, Andrew Murray wrote:
> > > On Mon, Sep 02, 2019 at 11:00:11PM -0700, Nathan Chancellor wrote:
> > > > On Thu, Aug 29, 2019 at 04:48:27PM +0100, Will Deacon wrote:
> > > > > From: Andrew Murray <andrew.murray@arm.com>
> > > > >
> > > > > When building for LSE atomics (CONFIG_ARM64_LSE_ATOMICS), if the hardware
> > > > > or toolchain doesn't support it the existing code will fallback to ll/sc
> > > > > atomics. It achieves this by branching from inline assembly to a function
> > > > > that is built with special compile flags. Further this results in the
> > > > > clobbering of registers even when the fallback isn't used increasing
> > > > > register pressure.
> > > > >
> > > > > Improve this by providing inline implementations of both LSE and
> > > > > ll/sc and use a static key to select between them, which allows for the
> > > > > compiler to generate better atomics code. Put the LL/SC fallback atomics
> > > > > in their own subsection to improve icache performance.
> > > > >
> > > > > Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> > > > > Signed-off-by: Will Deacon <will@kernel.org>
> > > >
> > > > For some reason, this causes a clang built kernel to fail to boot in
> > > > QEMU. There are no logs, it just never starts. I am off for the next two
> > > > days so I am going to try to look into this but you might have some
> > > > immediate ideas.
> > > >
> > > > https://github.com/ClangBuiltLinux/linux/issues/649
> > >
> > > I've been able to reproduce this - with clang 9.0.6 and qemu (without KVM)
> > > and only when ARM64_LSE_ATOMICS is enabled.
> > >
> > > This is slightly concerning...
> > >
> > > (gdb) b __lse__cmpxchg_case_acq_32
> > > Breakpoint 1 at 0xffff80001012b3cc: __lse__cmpxchg_case_acq_32. (19 locations)
> > > (gdb) continue
> > > Continuing.
> >
> > [...]
> >
> > > Any ideas?
> >
> > Does it work if the only thing you change is the toolchain, and use GCC
> > instead?
>
> Yup.
Also this is Clang generation:
ffff8000100f2700 <__ptrace_link>:
ffff8000100f2700: f9426009 ldr x9, [x0, #1216]
ffff8000100f2704: 91130008 add x8, x0, #0x4c0
ffff8000100f2708: eb09011f cmp x8, x9
ffff8000100f270c: 540002a1 b.ne ffff8000100f2760 <__ptrace_link+0x60> // b.any
ffff8000100f2710: f9425829 ldr x9, [x1, #1200]
ffff8000100f2714: 9112c02a add x10, x1, #0x4b0
ffff8000100f2718: f9000528 str x8, [x9, #8]
ffff8000100f271c: f9026009 str x9, [x0, #1216]
ffff8000100f2720: f902640a str x10, [x0, #1224]
ffff8000100f2724: f9025828 str x8, [x1, #1200]
ffff8000100f2728: f9024001 str x1, [x0, #1152]
ffff8000100f272c: b4000162 cbz x2, ffff8000100f2758 <__ptrace_link+0x58>
ffff8000100f2730: b900985f str wzr, [x2, #152]
ffff8000100f2734: 14000004 b ffff8000100f2744 <__ptrace_link+0x44>
ffff8000100f2738: 14000001 b ffff8000100f273c <__ptrace_link+0x3c>
ffff8000100f273c: 14000006 b ffff8000100f2754 <__ptrace_link+0x54>
ffff8000100f2740: 14000001 b ffff8000100f2744 <__ptrace_link+0x44>
ffff8000100f2744: 52800028 mov w8, #0x1 // #1
ffff8000100f2748: b828005f stadd w8, [x2]
ffff8000100f274c: f9030002 str x2, [x0, #1536]
ffff8000100f2750: d65f03c0 ret
ffff8000100f2754: 140007fd b ffff8000100f4748 <ptrace_check_attach+0xf8>
...
This looks like the default path (before we write over it) will take you to
the LSE code (e.g. ffff8000100f2734). I'm pretty sure this is wrong, or at
least not what we expected to see. Also why 4 branches?
And GCC:
ffff8000100ebc98 <__ptrace_link>:
ffff8000100ebc98: f9426003 ldr x3, [x0, #1216]
ffff8000100ebc9c: 91130004 add x4, x0, #0x4c0
ffff8000100ebca0: eb03009f cmp x4, x3
ffff8000100ebca4: 54000261 b.ne ffff8000100ebcf0 <__ptrace_link+0x58> // b.any
ffff8000100ebca8: f9425825 ldr x5, [x1, #1200]
ffff8000100ebcac: 9112c026 add x6, x1, #0x4b0
ffff8000100ebcb0: f90004a4 str x4, [x5, #8]
ffff8000100ebcb4: f9026005 str x5, [x0, #1216]
ffff8000100ebcb8: f9026406 str x6, [x0, #1224]
ffff8000100ebcbc: f9025824 str x4, [x1, #1200]
ffff8000100ebcc0: f9024001 str x1, [x0, #1152]
ffff8000100ebcc4: b4000122 cbz x2, ffff8000100ebce8 <__ptrace_link+0x50>
ffff8000100ebcc8: b900985f str wzr, [x2, #152]
ffff8000100ebccc: 14000006 b ffff8000100ebce4 <__ptrace_link+0x4c>
ffff8000100ebcd0: 14000005 b ffff8000100ebce4 <__ptrace_link+0x4c>
ffff8000100ebcd4: 52800021 mov w1, #0x1 // #1
ffff8000100ebcd8: b821005f stadd w1, [x2]
ffff8000100ebcdc: f9030002 str x2, [x0, #1536]
ffff8000100ebce0: d65f03c0 ret
ffff8000100ebce4: 14000599 b ffff8000100ed348 <__arm64_compat_sys_ptrace+0x180>
...
Thanks,
Andrew Murray
>
>
> > Could be some teething issues in the 'asm goto' support for clang?
>
> Thanks,
>
> Andrew Murray
>
> >
> > Will
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 03/10] arm64: atomics: avoid out-of-line ll/sc atomics
From: Andrew Murray @ 2019-09-03 15:15 UTC (permalink / raw)
To: Will Deacon
Cc: mark.rutland, peterz, catalin.marinas, ndesaulniers,
Ard.Biesheuvel, Nathan Chancellor, robin.murphy, linux-arm-kernel
In-Reply-To: <20190903144534.h2rp3cyd3ryohhgj@willie-the-truck>
On Tue, Sep 03, 2019 at 03:45:34PM +0100, Will Deacon wrote:
> On Tue, Sep 03, 2019 at 03:31:19PM +0100, Andrew Murray wrote:
> > On Mon, Sep 02, 2019 at 11:00:11PM -0700, Nathan Chancellor wrote:
> > > On Thu, Aug 29, 2019 at 04:48:27PM +0100, Will Deacon wrote:
> > > > From: Andrew Murray <andrew.murray@arm.com>
> > > >
> > > > When building for LSE atomics (CONFIG_ARM64_LSE_ATOMICS), if the hardware
> > > > or toolchain doesn't support it the existing code will fallback to ll/sc
> > > > atomics. It achieves this by branching from inline assembly to a function
> > > > that is built with special compile flags. Further this results in the
> > > > clobbering of registers even when the fallback isn't used increasing
> > > > register pressure.
> > > >
> > > > Improve this by providing inline implementations of both LSE and
> > > > ll/sc and use a static key to select between them, which allows for the
> > > > compiler to generate better atomics code. Put the LL/SC fallback atomics
> > > > in their own subsection to improve icache performance.
> > > >
> > > > Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> > > > Signed-off-by: Will Deacon <will@kernel.org>
> > >
> > > For some reason, this causes a clang built kernel to fail to boot in
> > > QEMU. There are no logs, it just never starts. I am off for the next two
> > > days so I am going to try to look into this but you might have some
> > > immediate ideas.
> > >
> > > https://github.com/ClangBuiltLinux/linux/issues/649
> >
> > I've been able to reproduce this - with clang 9.0.6 and qemu (without KVM)
> > and only when ARM64_LSE_ATOMICS is enabled.
> >
> > This is slightly concerning...
> >
> > (gdb) b __lse__cmpxchg_case_acq_32
> > Breakpoint 1 at 0xffff80001012b3cc: __lse__cmpxchg_case_acq_32. (19 locations)
> > (gdb) continue
> > Continuing.
>
> [...]
>
> > Any ideas?
>
> Does it work if the only thing you change is the toolchain, and use GCC
> instead?
Yup.
> Could be some teething issues in the 'asm goto' support for clang?
Thanks,
Andrew Murray
>
> Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [GIT PULL] arm64: defconfig: hisilicon config updates for v5.4
From: Arnd Bergmann @ 2019-09-03 15:11 UTC (permalink / raw)
To: Wei Xu
Cc: Salil Mehta, jinying, Tangkunshan, John Garry, SoC Team,
Shameerali Kolothum Thodi, Linuxarm, Wangzhou (B), arm@kernel.org,
huangdaode, xuwei (O), Jonathan Cameron, Olof Johansson,
Liguozhu (Kenneth), Zhangyi ac,
linux-arm-kernel@lists.infradead.org, Shiju Jose
In-Reply-To: <5D562573.5030604@hisilicon.com>
On Fri, Aug 16, 2019 at 5:39 AM Wei Xu <xuwei5@hisilicon.com> wrote:
> ----------------------------------------------------------------
> ARM64: hisilicon: defconfig updates for v5.4
>
> - Enable ACPI_APEI_PCIEAER for the hisilicon D06 board to
> support PCIe AER error report
Pulled into arm/defconfig, thanks
Arnd
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 0/4] mmc: sdhci-of-aspeed: Fixes for AST2600 eMMC support
From: Ulf Hansson @ 2019-09-03 15:10 UTC (permalink / raw)
To: Andrew Jeffery
Cc: linux-aspeed, OpenBMC Maillist, linux-mmc@vger.kernel.org,
Adrian Hunter, Linux Kernel Mailing List, Joel Stanley, Linux ARM
In-Reply-To: <20190902035842.2747-1-andrew@aj.id.au>
On Mon, 2 Sep 2019 at 05:58, Andrew Jeffery <andrew@aj.id.au> wrote:
>
> Hello,
>
> I've added a couple of patches since v1 of this series. The horizon has
> broadened slightly with a fix for SPARC builds as well in patch 1/4. Ulf
> suggested a minor cleanup on v1 with respect to handling of the current clock
> value, so that's now patch 2/4. Patches 3/4 and 4/4 are as they were in v1.
Applied patch 2->4 for next, thanks!
Kind regards
Uffe
>
> The v1 series can be found here:
>
> https://patchwork.ozlabs.org/cover/1155757/
>
> Please review!
>
> Andrew
>
> Andrew Jeffery (4):
> mmc: sdhci-of-aspeed: Fix link failure for SPARC
> mmc: sdhci-of-aspeed: Drop redundant assignment to host->clock
> mmc: sdhci-of-aspeed: Uphold clocks-on post-condition of set_clock()
> mmc: sdhci-of-aspeed: Allow max-frequency limitation of SDCLK
>
> drivers/mmc/host/sdhci-of-aspeed.c | 62 ++++++++++++++++++++----------
> 1 file changed, 42 insertions(+), 20 deletions(-)
>
> --
> 2.20.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCHv7] drivers/amba: add reset control to amba bus probe
From: Linus Walleij @ 2019-09-03 15:07 UTC (permalink / raw)
To: Dinh Nguyen
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Daniel Thompson, Tony Luck, Manivannan Sadhasivam, Kees Cook,
Rob Herring, Anton Vorontsov, Russell King,
linux-kernel@vger.kernel.org, Philipp Zabel, Colin Cross,
Frank Rowand, Linux ARM
In-Reply-To: <20190903141215.18283-1-dinguyen@kernel.org>
On Tue, Sep 3, 2019 at 4:20 PM Dinh Nguyen <dinguyen@kernel.org> wrote:
> The primecell controller on some SoCs, i.e. SoCFPGA, is held in reset by
> default. Until recently, the DMA controller was brought out of reset by the
> bootloader(i.e. U-Boot). But a recent change in U-Boot, the peripherals
> that are not used are held in reset and are left to Linux to bring them
> out of reset.
>
> Add a mechanism for getting the reset property and de-assert the primecell
> module from reset if found. This is a not a hard fail if the reset properti
> is not present in the device tree node, so the driver will continue to
> probe.
>
> Because there are different variants of the controller that may have
> multiple reset signals, the code will find all reset(s) specified and
> de-assert them.
>
> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Please put this patch into Russell's patch tracker.
Yours,
Linus Walleij
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 7/8] mips: vdso: Remove unused VDSO_HAS_32BIT_FALLBACK
From: Vincenzo Frascino @ 2019-09-03 14:51 UTC (permalink / raw)
To: Paul Burton
Cc: linux-arch@vger.kernel.org, luto@kernel.org,
catalin.marinas@arm.com, 0x7f454c46@gmail.com,
linux-kernel@vger.kernel.org, linux-mips@vger.kernel.org,
linux-kselftest@vger.kernel.org, tglx@linutronix.de,
salyzyn@android.com, will@kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190903143801.7upetfqe6upouzlh@pburton-laptop>
Hi Paul,
On 9/3/19 3:46 PM, Paul Burton wrote:
> Hi Vincenzo,
>
> On Fri, Aug 30, 2019 at 02:59:01PM +0100, Vincenzo Frascino wrote:
>> VDSO_HAS_32BIT_FALLBACK has been removed from the core since
>> the architectures that support the generic vDSO library have
>> been converted to support the 32 bit fallbacks.
>>
>> Remove unused VDSO_HAS_32BIT_FALLBACK from mips vdso.
>>
>> Cc: Paul Burton <paul.burton@mips.com>
>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
>
> Do you want this one in mips-next too, or applied somewhere else along
> with the rest of the series? If the latter:
>
> Acked-by: Paul Burton <paul.burton@mips.com>
>
This patch has a dependency on patch n5 hence can not be applied independently.
Thanks,
Vincenzo
> Thanks,
> Paul
>
>> ---
>> arch/mips/include/asm/vdso/gettimeofday.h | 2 --
>> 1 file changed, 2 deletions(-)
>>
>> diff --git a/arch/mips/include/asm/vdso/gettimeofday.h b/arch/mips/include/asm/vdso/gettimeofday.h
>> index e78462e8ca2e..5ad2b086626d 100644
>> --- a/arch/mips/include/asm/vdso/gettimeofday.h
>> +++ b/arch/mips/include/asm/vdso/gettimeofday.h
>> @@ -107,8 +107,6 @@ static __always_inline int clock_getres_fallback(
>>
>> #if _MIPS_SIM != _MIPS_SIM_ABI64
>>
>> -#define VDSO_HAS_32BIT_FALLBACK 1
>> -
>> static __always_inline long clock_gettime32_fallback(
>> clockid_t _clkid,
>> struct old_timespec32 *_ts)
>> --
>> 2.23.0
>>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 1/4] mmc: sdhci-of-aspeed: Fix link failure for SPARC
From: Ulf Hansson @ 2019-09-03 14:48 UTC (permalink / raw)
To: Andrew Jeffery
Cc: linux-aspeed, Arnd Bergmann, OpenBMC Maillist, linux-mmc,
Adrian Hunter, Linux Kernel Mailing List, Joel Stanley, Linux ARM,
kbuild test robot
In-Reply-To: <83570e25-b20a-4a17-85ea-15a9a53289bf@www.fastmail.com>
On Mon, 2 Sep 2019 at 07:26, Andrew Jeffery <andrew@aj.id.au> wrote:
>
>
>
> On Mon, 2 Sep 2019, at 13:42, Joel Stanley wrote:
> > On Mon, 2 Sep 2019 at 03:58, Andrew Jeffery <andrew@aj.id.au> wrote:
> > >
> > > Resolves the following build error reported by the 0-day bot:
> > >
> > > ERROR: "of_platform_device_create" [drivers/mmc/host/sdhci-of-aspeed.ko] undefined!
> > >
> > > SPARC does not set CONFIG_OF_ADDRESS so the symbol is missing. Guard the
> > > callsite to maintain build coverage for the rest of the driver.
> > >
> > > Reported-by: kbuild test robot <lkp@intel.com>
> > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > > ---
> > > drivers/mmc/host/sdhci-of-aspeed.c | 38 ++++++++++++++++++++----------
> > > 1 file changed, 25 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/mmc/host/sdhci-of-aspeed.c b/drivers/mmc/host/sdhci-of-aspeed.c
> > > index d5acb5afc50f..96ca494752c5 100644
> > > --- a/drivers/mmc/host/sdhci-of-aspeed.c
> > > +++ b/drivers/mmc/host/sdhci-of-aspeed.c
> > > @@ -224,10 +224,30 @@ static struct platform_driver aspeed_sdhci_driver = {
> > > .remove = aspeed_sdhci_remove,
> > > };
> > >
> > > -static int aspeed_sdc_probe(struct platform_device *pdev)
> > > -
> > > +static int aspeed_sdc_create_sdhcis(struct platform_device *pdev)
> > > {
> > > +#if defined(CONFIG_OF_ADDRESS)
> >
> > This is going to be untested code forever, as no one will be running
> > on a chip with this hardware present but OF_ADDRESS disabled.
> >
> > How about we make the driver depend on OF_ADDRESS instead?
>
> Testing is split into two pieces here: compile-time and run-time.
> Clearly the run-time behaviour is going to be broken on configurations
> without CONFIG_OF_ADDRESS (SPARC as mentioned), but I don't think
> that means we shouldn't allow it to be compiled in that case
> (e.g. CONFIG_COMPILE_TEST performs a similar role).
>
> With respect to compile-time it's possible to compile either path as
> demonstrated by the build failure report.
>
> Having said that there's no reason we couldn't do what you suggest,
> just it wasn't the existing solution pattern for the problem (there are
> several other drivers that suffered the same bug that were fixed in the
> style of this patch). Either way works, it's all somewhat academic.
> Your suggestion is more obvious in terms of correctness, but this
> patch is basically just code motion (the only addition is the `#if`/
> `#endif` lines over what was already there if we disregard the
> function declaration/invocation). I'll change it if there are further
> complaints and a reason to do a v3.
I am in favor of Joel's suggestion as I don't really like having
ifdefs bloating around in the driver (unless very good reasons).
Please re-spin a v3.
Another option is to implement stub function and to deal with error
codes, but that sounds more like a long term thingy, if even
applicable here.
Kind regards
Uffe
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: FYI: imx-sdma firmware is not compatible with SLUB slab allocator
From: Leonard Crestez @ 2019-09-03 14:48 UTC (permalink / raw)
To: Jurgen Lambrecht, Robin Gong
Cc: Aisheng Dong, Fabio Estevam, dl-linux-imx,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <dc06392b-8242-7d09-e0fe-49fb04849ebb@televic.com>
On 03.09.2019 17:32, Jurgen Lambrecht wrote:
> On 9/3/19 7:57 AM, Robin Gong wrote:
>
>>> And that are the last commits on drivers/dma/imx-sdma.c for my 4.19.x+fslc
>>> branch. But I have already tried 5.1.x+fslc, and it also got stuck.
>> Sorry, I can't reproduce your issue on Linux 5.3-rc6 with 'CONFIG_SLOB=y' and
>> SDMA firmware built in kernel, Could you have a try on our imx6ul-14x14-evk
>> board with Linux 5.3-rc6 directly(no any patch needed)?
>
> This works on our own board (with imx6ul)!
Something seems to be wrong with the fslc tree, using 5.1.x+fslc at
latest commit cd1d083333e76e03d16f015c23f1f1b8c8637381 I can reproduce
the issue on imx6ul-14x14-evk board.
Running without SLOB and builtin firmware it's fine.
I couldn't reproduce with latest 4.19.x+fslc (currently at commit
91d5756ab9096bbec256115d1d6b85f5d7139f85), maybe some additional SDMA
patches were applied which fixed this issue?
--
Regards,
Leonard
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 7/8] mips: vdso: Remove unused VDSO_HAS_32BIT_FALLBACK
From: Paul Burton @ 2019-09-03 14:46 UTC (permalink / raw)
To: Vincenzo Frascino
Cc: linux-arch@vger.kernel.org, luto@kernel.org,
catalin.marinas@arm.com, 0x7f454c46@gmail.com,
linux-kernel@vger.kernel.org, linux-mips@vger.kernel.org,
linux-kselftest@vger.kernel.org, tglx@linutronix.de,
salyzyn@android.com, will@kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190830135902.20861-8-vincenzo.frascino@arm.com>
Hi Vincenzo,
On Fri, Aug 30, 2019 at 02:59:01PM +0100, Vincenzo Frascino wrote:
> VDSO_HAS_32BIT_FALLBACK has been removed from the core since
> the architectures that support the generic vDSO library have
> been converted to support the 32 bit fallbacks.
>
> Remove unused VDSO_HAS_32BIT_FALLBACK from mips vdso.
>
> Cc: Paul Burton <paul.burton@mips.com>
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Do you want this one in mips-next too, or applied somewhere else along
with the rest of the series? If the latter:
Acked-by: Paul Burton <paul.burton@mips.com>
Thanks,
Paul
> ---
> arch/mips/include/asm/vdso/gettimeofday.h | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/arch/mips/include/asm/vdso/gettimeofday.h b/arch/mips/include/asm/vdso/gettimeofday.h
> index e78462e8ca2e..5ad2b086626d 100644
> --- a/arch/mips/include/asm/vdso/gettimeofday.h
> +++ b/arch/mips/include/asm/vdso/gettimeofday.h
> @@ -107,8 +107,6 @@ static __always_inline int clock_getres_fallback(
>
> #if _MIPS_SIM != _MIPS_SIM_ABI64
>
> -#define VDSO_HAS_32BIT_FALLBACK 1
> -
> static __always_inline long clock_gettime32_fallback(
> clockid_t _clkid,
> struct old_timespec32 *_ts)
> --
> 2.23.0
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 03/10] arm64: atomics: avoid out-of-line ll/sc atomics
From: Will Deacon @ 2019-09-03 14:45 UTC (permalink / raw)
To: Andrew Murray
Cc: mark.rutland, peterz, catalin.marinas, ndesaulniers,
Ard.Biesheuvel, Nathan Chancellor, robin.murphy, linux-arm-kernel
In-Reply-To: <20190903143117.GR9720@e119886-lin.cambridge.arm.com>
On Tue, Sep 03, 2019 at 03:31:19PM +0100, Andrew Murray wrote:
> On Mon, Sep 02, 2019 at 11:00:11PM -0700, Nathan Chancellor wrote:
> > On Thu, Aug 29, 2019 at 04:48:27PM +0100, Will Deacon wrote:
> > > From: Andrew Murray <andrew.murray@arm.com>
> > >
> > > When building for LSE atomics (CONFIG_ARM64_LSE_ATOMICS), if the hardware
> > > or toolchain doesn't support it the existing code will fallback to ll/sc
> > > atomics. It achieves this by branching from inline assembly to a function
> > > that is built with special compile flags. Further this results in the
> > > clobbering of registers even when the fallback isn't used increasing
> > > register pressure.
> > >
> > > Improve this by providing inline implementations of both LSE and
> > > ll/sc and use a static key to select between them, which allows for the
> > > compiler to generate better atomics code. Put the LL/SC fallback atomics
> > > in their own subsection to improve icache performance.
> > >
> > > Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> > > Signed-off-by: Will Deacon <will@kernel.org>
> >
> > For some reason, this causes a clang built kernel to fail to boot in
> > QEMU. There are no logs, it just never starts. I am off for the next two
> > days so I am going to try to look into this but you might have some
> > immediate ideas.
> >
> > https://github.com/ClangBuiltLinux/linux/issues/649
>
> I've been able to reproduce this - with clang 9.0.6 and qemu (without KVM)
> and only when ARM64_LSE_ATOMICS is enabled.
>
> This is slightly concerning...
>
> (gdb) b __lse__cmpxchg_case_acq_32
> Breakpoint 1 at 0xffff80001012b3cc: __lse__cmpxchg_case_acq_32. (19 locations)
> (gdb) continue
> Continuing.
[...]
> Any ideas?
Does it work if the only thing you change is the toolchain, and use GCC
instead? Could be some teething issues in the 'asm goto' support for clang?
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 1/8] arm64: compat: vdso: Expose BUILD_VDSO32
From: Will Deacon @ 2019-09-03 14:38 UTC (permalink / raw)
To: Vincenzo Frascino
Cc: linux-arch, luto, catalin.marinas, 0x7f454c46, linux-kernel,
linux-mips, paul.burton, linux-kselftest, tglx, salyzyn,
linux-arm-kernel
In-Reply-To: <c0b1673d-e37d-a526-0862-ad07f779f5bf@arm.com>
On Tue, Sep 03, 2019 at 03:36:16PM +0100, Vincenzo Frascino wrote:
> On 8/30/19 2:58 PM, Vincenzo Frascino wrote:
> > clock_gettime32 and clock_getres_time32 should be compiled only with the
> > 32 bit vdso library.
> >
> > Expose BUILD_VDSO32 when arm64 compat is compiled, to provide an
> > indication to the generic library to include these symbols.
> >
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will@kernel.org>
> > Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> > Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> > ---
> > arch/arm64/include/asm/vdso/compat_gettimeofday.h | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/arm64/include/asm/vdso/compat_gettimeofday.h b/arch/arm64/include/asm/vdso/compat_gettimeofday.h
> > index c50ee1b7d5cd..fe7afe0f1a3d 100644
> > --- a/arch/arm64/include/asm/vdso/compat_gettimeofday.h
> > +++ b/arch/arm64/include/asm/vdso/compat_gettimeofday.h
> > @@ -17,6 +17,7 @@
> > #define VDSO_HAS_CLOCK_GETRES 1
> >
> > #define VDSO_HAS_32BIT_FALLBACK 1
> > +#define BUILD_VDSO32 1
> >
> > static __always_inline
> > int gettimeofday_fallback(struct __kernel_old_timeval *_tv,
> >
>
> This patch is independent from the rest and touches only arch code. Can it go in
> via the arm64 tree?
Why not take it via -tip along with patch 6? Otherwise we'll get a silly
conflict. I'd assumed this series was going in as one thing.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2] drm/mcde: Some fixes to handling video mode
From: Stephan Gerhold @ 2019-09-03 14:37 UTC (permalink / raw)
To: Linus Walleij
Cc: Maxime Ripard, Sean Paul, Maarten Lankhorst, linux-arm-kernel,
dri-devel
In-Reply-To: <20190903091512.15780-1-linus.walleij@linaro.org>
On Tue, Sep 03, 2019 at 11:15:12AM +0200, Linus Walleij wrote:
> The video DSI mode had not really been tested. These fixes makes
> it more likely to work on real hardware:
> - Set the HS clock to something the video mode reported by the
> panel can handle rather than the max HS rate.
> - Put the active width (x width) in the right bits and the VSA
> (vertical sync active) in the right bits (those were swapped).
> - Calculate the packet sizes in bytes as in the vendor driver,
> rather than in bits.
> - Handle negative result in front/back/sync packages and fall
> back to zero like in the vendor driver.
>
> Cc: Stephan Gerhold <stephan@gerhold.net>
> Fixes: 5fc537bfd000 ("drm/mcde: Add new driver for ST-Ericsson MCDE")
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - Fix some more comments so we understand what is going on.
> - Set up the maximum line limit size in the right register
> instead of setting it in the burst size register portion.
> - Set some default wakeup time other than zero (still need
> fixing more).
> ---
> drivers/gpu/drm/mcde/mcde_dsi.c | 75 ++++++++++++++++++++++-----------
> 1 file changed, 50 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/mcde/mcde_dsi.c b/drivers/gpu/drm/mcde/mcde_dsi.c
> index cd261c266f35..5c65cd70fcd3 100644
> --- a/drivers/gpu/drm/mcde/mcde_dsi.c
> +++ b/drivers/gpu/drm/mcde/mcde_dsi.c
> @@ -365,11 +365,12 @@ void mcde_dsi_te_request(struct mipi_dsi_device *mdsi)
> static void mcde_dsi_setup_video_mode(struct mcde_dsi *d,
> const struct drm_display_mode *mode)
> {
> - u8 bpp = mipi_dsi_pixel_format_to_bpp(d->mdsi->format);
> + /* cpp, characters per pixel, number of bytes per pixel */
> + u8 cpp = mipi_dsi_pixel_format_to_bpp(d->mdsi->format) / 8;
> u64 bpl;
> - u32 hfp;
> - u32 hbp;
> - u32 hsa;
> + int hfp;
> + int hbp;
> + int hsa;
> u32 blkline_pck, line_duration;
> u32 blkeol_pck, blkeol_duration;
> u32 val;
> @@ -408,11 +409,11 @@ static void mcde_dsi_setup_video_mode(struct mcde_dsi *d,
> return;
> }
>
> - /* TODO: TVG could be enabled here */
> + /* TODO: TVG (test video generator) could be enabled here */
>
> - /* Send blanking packet */
> + /* During blanking: go to LP mode */
> val |= DSI_VID_MAIN_CTL_REG_BLKLINE_MODE_LP_0;
> - /* Send EOL packet */
> + /* During EOL: go to LP mode */
> val |= DSI_VID_MAIN_CTL_REG_BLKEOL_MODE_LP_0;
> /* Recovery mode 1 */
> val |= 1 << DSI_VID_MAIN_CTL_RECOVERY_MODE_SHIFT;
> @@ -420,13 +421,13 @@ static void mcde_dsi_setup_video_mode(struct mcde_dsi *d,
> writel(val, d->regs + DSI_VID_MAIN_CTL);
>
> /* Vertical frame parameters are pretty straight-forward */
> - val = mode->vdisplay << DSI_VID_VSIZE_VSA_LENGTH_SHIFT;
> + val = mode->vdisplay << DSI_VID_VSIZE_VACT_LENGTH_SHIFT;
> /* vertical front porch */
> val |= (mode->vsync_start - mode->vdisplay)
> << DSI_VID_VSIZE_VFP_LENGTH_SHIFT;
> /* vertical sync active */
> val |= (mode->vsync_end - mode->vsync_start)
> - << DSI_VID_VSIZE_VACT_LENGTH_SHIFT;
> + << DSI_VID_VSIZE_VSA_LENGTH_SHIFT;
> /* vertical back porch */
> val |= (mode->vtotal - mode->vsync_end)
> << DSI_VID_VSIZE_VBP_LENGTH_SHIFT;
> @@ -437,21 +438,25 @@ static void mcde_dsi_setup_video_mode(struct mcde_dsi *d,
> * horizontal resolution is given in pixels and must be re-calculated
> * into bytes since this is what the hardware expects.
> *
> + * hfp = horizontal front porch in bytes
> + * hbp = horizontal back porch in bytes
> + * hsa = horizontal sync active in bytes
> + *
> * 6 + 2 is HFP header + checksum
> */
> - hfp = (mode->hsync_start - mode->hdisplay) * bpp - 6 - 2;
> + hfp = (mode->hsync_start - mode->hdisplay) * cpp - 6 - 2;
> if (d->mdsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE) {
> /*
> * 6 is HBP header + checksum
> * 4 is RGB header + checksum
> */
> - hbp = (mode->htotal - mode->hsync_end) * bpp - 4 - 6;
> + hbp = (mode->htotal - mode->hsync_end) * cpp - 4 - 6;
> /*
> * 6 is HBP header + checksum
> * 4 is HSW packet bytes
> * 4 is RGB header + checksum
> */
> - hsa = (mode->hsync_end - mode->hsync_start) * bpp - 4 - 4 - 6;
> + hsa = (mode->hsync_end - mode->hsync_start) * cpp - 4 - 4 - 6;
> } else {
> /*
> * HBP includes both back porch and sync
> @@ -459,11 +464,23 @@ static void mcde_dsi_setup_video_mode(struct mcde_dsi *d,
> * 4 is HSW packet bytes
> * 4 is RGB header + checksum
> */
> - hbp = (mode->htotal - mode->hsync_start) * bpp - 4 - 4 - 6;
> - /* HSA is not considered in this mode and set to 0 */
> + hbp = (mode->htotal - mode->hsync_start) * cpp - 4 - 4 - 6;
> + /* HSA is not present in this mode and set to 0 */
> + hsa = 0;
> + }
> + if (hfp < 0) {
> + dev_info(d->dev, "hfp negative, set to 0\n");
> + hfp = 0;
> + }
> + if (hbp < 0) {
> + dev_info(d->dev, "hbp negative, set to 0\n");
> + hbp = 0;
> + }
> + if (hsa < 0) {
> + dev_info(d->dev, "hsa negative, set to 0\n");
> hsa = 0;
> }
> - dev_dbg(d->dev, "hfp: %u, hbp: %u, hsa: %u\n",
> + dev_dbg(d->dev, "hfp: %u, hbp: %u, hsa: %u bytes\n",
> hfp, hbp, hsa);
>
> /* Frame parameters: horizontal sync active */
> @@ -474,8 +491,8 @@ static void mcde_dsi_setup_video_mode(struct mcde_dsi *d,
> val |= hfp << DSI_VID_HSIZE1_HFP_LENGTH_SHIFT;
> writel(val, d->regs + DSI_VID_HSIZE1);
>
> - /* RGB data length (bytes on one scanline) */
> - val = mode->hdisplay * (bpp / 8);
> + /* RGB data length (visible bytes on one scanline) */
> + val = mode->hdisplay * cpp;
> writel(val, d->regs + DSI_VID_HSIZE2);
>
> /* TODO: further adjustments for TVG mode here */
> @@ -507,37 +524,43 @@ static void mcde_dsi_setup_video_mode(struct mcde_dsi *d,
> }
>
> line_duration = (blkline_pck + 6) / d->mdsi->lanes;
> - dev_dbg(d->dev, "line duration %u\n", line_duration);
> + dev_dbg(d->dev, "line duration %u bytes\n", line_duration);
> val = line_duration << DSI_VID_DPHY_TIME_REG_LINE_DURATION_SHIFT;
> /*
> * This is the time to perform LP->HS on D-PHY
> * FIXME: nowhere to get this from: DT property on the DSI?
> + * values like 48 and 72 seen in the vendor code.
> */
> - val |= 0 << DSI_VID_DPHY_TIME_REG_WAKEUP_TIME_SHIFT;
> + val |= 48 << DSI_VID_DPHY_TIME_REG_WAKEUP_TIME_SHIFT;
> writel(val, d->regs + DSI_VID_DPHY_TIME);
I just want to note that the Samsung S3 Mini (golden) seems to use 88
here. I suppose we will need to see how important this property is,
or if panels can also work with a slightly wrong value.
>
> /* Calculate block end of line */
> - blkeol_pck = bpl - mode->hdisplay * bpp - 6;
> + blkeol_pck = bpl - mode->hdisplay * cpp - 6;
> blkeol_duration = (blkeol_pck + 6) / d->mdsi->lanes;
> - dev_dbg(d->dev, "blkeol pck: %u, duration: %u\n",
> - blkeol_pck, blkeol_duration);
> + dev_dbg(d->dev, "blkeol pck: %u bytes, duration: %u bytes\n",
> + blkeol_pck, blkeol_duration);
>
> if (d->mdsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
> /* Set up EOL clock for burst mode */
> val = readl(d->regs + DSI_VID_BLKSIZE1);
> val |= blkeol_pck << DSI_VID_BLKSIZE1_BLKEOL_PCK_SHIFT;
> writel(val, d->regs + DSI_VID_BLKSIZE1);
> - writel(blkeol_pck, d->regs + DSI_VID_VCA_SETTING2);
> + writel((blkeol_pck & DSI_VID_VCA_SETTING2_EXACT_BURST_LIMIT_MASK)
> + << DSI_VID_VCA_SETTING2_EXACT_BURST_LIMIT_SHIFT,
> + d->regs + DSI_VID_VCA_SETTING2);
It does not make a difference in this case because SHIFT = 0,
but shouldn't you normally shift before applying the mask?
Otherwise, you would wipe out the lower bits before you shift them.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 1/8] arm64: compat: vdso: Expose BUILD_VDSO32
From: Vincenzo Frascino @ 2019-09-03 14:36 UTC (permalink / raw)
To: linux-arch, linux-arm-kernel, linux-kernel, linux-mips,
linux-kselftest
Cc: catalin.marinas, 0x7f454c46, salyzyn, paul.burton, luto, tglx,
will
In-Reply-To: <20190830135902.20861-2-vincenzo.frascino@arm.com>
Hi Catalin and Will,
On 8/30/19 2:58 PM, Vincenzo Frascino wrote:
> clock_gettime32 and clock_getres_time32 should be compiled only with the
> 32 bit vdso library.
>
> Expose BUILD_VDSO32 when arm64 compat is compiled, to provide an
> indication to the generic library to include these symbols.
>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
> arch/arm64/include/asm/vdso/compat_gettimeofday.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/include/asm/vdso/compat_gettimeofday.h b/arch/arm64/include/asm/vdso/compat_gettimeofday.h
> index c50ee1b7d5cd..fe7afe0f1a3d 100644
> --- a/arch/arm64/include/asm/vdso/compat_gettimeofday.h
> +++ b/arch/arm64/include/asm/vdso/compat_gettimeofday.h
> @@ -17,6 +17,7 @@
> #define VDSO_HAS_CLOCK_GETRES 1
>
> #define VDSO_HAS_32BIT_FALLBACK 1
> +#define BUILD_VDSO32 1
>
> static __always_inline
> int gettimeofday_fallback(struct __kernel_old_timeval *_tv,
>
This patch is independent from the rest and touches only arch code. Can it go in
via the arm64 tree?
--
Regards,
Vincenzo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: FYI: imx-sdma firmware is not compatible with SLUB slab allocator
From: Jurgen Lambrecht @ 2019-09-03 14:32 UTC (permalink / raw)
To: Robin Gong, Leonard Crestez
Cc: Aisheng Dong, dl-linux-imx, linux-arm-kernel@lists.infradead.org
In-Reply-To: <VE1PR04MB66380D06DD2619493904B38B89B90@VE1PR04MB6638.eurprd04.prod.outlook.com>
On 9/3/19 7:57 AM, Robin Gong wrote:
> CAUTION: This Email originated from outside Televic. Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
>
> On 2019-8-29 14:24, Jurgen Lambrecht wrote:
>> On 8/28/19 4:05 PM, Robin Gong wrote:
>>> Could you help check if below commit in your side?
>>> commit ebb853b1bd5f659b92c71dc6a9de44cfc37c78c0
>>> Author: Lucas Stach<l.stach@pengutronix.de>
>>> Date: Tue Nov 6 03:40:28 2018 +0000
>> yes, it's in.
>>
>> Also the 2 follow-up commits of Lucas Stach:
>> 9063f5a99ea76f85935e3e453422d15e7be89b9e and
>> 374f384bc66f7a928f11eb20c0518f0f3fc1ffd6.
I had also already cherry picked your commit
3f5de4c7e16164a344a905649f08e8a90a68ff9f "dmaengine: imx-sdma: remove
BD_INTR for channel0".
But also then kernel hangs at loading sdma FW.
(this looked the most interesting commit)
>>
>> And that are the last commits on drivers/dma/imx-sdma.c for my 4.19.x+fslc
>> branch. But I have already tried 5.1.x+fslc, and it also got stuck.
> Sorry, I can't reproduce your issue on Linux 5.3-rc6 with 'CONFIG_SLOB=y' and
> SDMA firmware built in kernel, Could you have a try on our imx6ul-14x14-evk
> board with Linux 5.3-rc6 directly(no any patch needed)?
This works on our own board (with imx6ul)!
Thanks,
Jürgen
> root@imx6ul7d:~# dmesg | grep dma
> [ 0.991928] mxs-dma 1804000.dma-apbh: initialized
> [ 4.162199] imx-sdma 20ec000.sdma: loaded firmware 3.5
>> Kind regards,
>>
>> Jürgen
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 03/10] arm64: atomics: avoid out-of-line ll/sc atomics
From: Andrew Murray @ 2019-09-03 14:31 UTC (permalink / raw)
To: Nathan Chancellor
Cc: mark.rutland, peterz, catalin.marinas, ndesaulniers, robin.murphy,
Ard.Biesheuvel, Will Deacon, linux-arm-kernel
In-Reply-To: <20190903060011.GA60737@archlinux-threadripper>
On Mon, Sep 02, 2019 at 11:00:11PM -0700, Nathan Chancellor wrote:
> On Thu, Aug 29, 2019 at 04:48:27PM +0100, Will Deacon wrote:
> > From: Andrew Murray <andrew.murray@arm.com>
> >
> > When building for LSE atomics (CONFIG_ARM64_LSE_ATOMICS), if the hardware
> > or toolchain doesn't support it the existing code will fallback to ll/sc
> > atomics. It achieves this by branching from inline assembly to a function
> > that is built with special compile flags. Further this results in the
> > clobbering of registers even when the fallback isn't used increasing
> > register pressure.
> >
> > Improve this by providing inline implementations of both LSE and
> > ll/sc and use a static key to select between them, which allows for the
> > compiler to generate better atomics code. Put the LL/SC fallback atomics
> > in their own subsection to improve icache performance.
> >
> > Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> > Signed-off-by: Will Deacon <will@kernel.org>
>
> For some reason, this causes a clang built kernel to fail to boot in
> QEMU. There are no logs, it just never starts. I am off for the next two
> days so I am going to try to look into this but you might have some
> immediate ideas.
>
> https://github.com/ClangBuiltLinux/linux/issues/649
I've been able to reproduce this - with clang 9.0.6 and qemu (without KVM)
and only when ARM64_LSE_ATOMICS is enabled.
This is slightly concerning...
(gdb) b __lse__cmpxchg_case_acq_32
Breakpoint 1 at 0xffff80001012b3cc: __lse__cmpxchg_case_acq_32. (19 locations)
(gdb) continue
Continuing.
Breakpoint 1, __cmpxchg_case_acq_32 (ptr=<optimized out>, old=0, new=1) at /home/amurray/linux/./arch/arm64/include/asm/cmpxchg.h:121
121 __CMPXCHG_CASE(acq_, 32)
(gdb) bt
#0 __cmpxchg_case_acq_32 (ptr=<optimized out>, old=0, new=1) at /home/amurray/linux/./arch/arm64/include/asm/cmpxchg.h:121
#1 __cmpxchg_acq (ptr=<optimized out>, old=<optimized out>, new=<optimized out>, size=4)
at /home/amurray/linux/./arch/arm64/include/asm/cmpxchg.h:173
#2 atomic_cmpxchg_acquire (v=<optimized out>, old=0, new=1) at /home/amurray/linux/./include/asm-generic/atomic-instrumented.h:664
#3 atomic_try_cmpxchg_acquire (v=<optimized out>, new=1, old=<optimized out>)
at /home/amurray/linux/./include/linux/atomic-fallback.h:931
#4 queued_spin_lock (lock=<optimized out>) at /home/amurray/linux/./include/asm-generic/qspinlock.h:78
#5 do_raw_spin_lock (lock=<optimized out>) at /home/amurray/linux/./include/linux/spinlock.h:181
#6 __raw_spin_lock (lock=0xffff8000119b15d4 <logbuf_lock>) at /home/amurray/linux/./include/linux/spinlock_api_smp.h:143
#7 _raw_spin_lock (lock=0xffff8000119b15d4 <logbuf_lock>) at kernel/locking/spinlock.c:151
#8 0xffff800010147028 in vprintk_emit (facility=0, level=-1, dict=0x0, dictlen=0,
fmt=0xffff800011103afe "\001\066Booting Linux on physical CPU 0x%010lx [0x%08x]\n", args=...) at kernel/printk/printk.c:1966
#9 0xffff800010147818 in vprintk_default (fmt=0xffff800011103afe "\001\066Booting Linux on physical CPU 0x%010lx [0x%08x]\n", args=...)
at kernel/printk/printk.c:2013
#10 0xffff800010149c94 in vprintk_func (fmt=0xffff800011103afe "\001\066Booting Linux on physical CPU 0x%010lx [0x%08x]\n", args=...)
at kernel/printk/printk_safe.c:386
#11 0xffff8000101461bc in printk (fmt=0xffff8000119b15d4 <logbuf_lock> "") at kernel/printk/printk.c:2046
#12 0xffff8000112d3238 in smp_setup_processor_id () at arch/arm64/kernel/setup.c:96
#13 0xffff8000112d06a4 in start_kernel () at init/main.c:581
#14 0x0000000000000000 in ?? ()
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
In other words system_uses_lse_atomics seems to give us the LSE variant when we
don't have LSE, thus resulting in an invalid instruction (we end up in
do_undefinstr).
Though I don't think system_uses_lse_atomics is at fault here, the behaviour
varies depending on subtle code changes to lse.h, for example:
- change system_uses_lse_atomics as follows, and the kernel boots as far as
"Loading compiled-in X.509 certificates" and it gets stuck.
--- a/arch/arm64/include/asm/lse.h
+++ b/arch/arm64/include/asm/lse.h
@@ -21,8 +21,11 @@ extern struct static_key_false arm64_const_caps_ready;
static inline bool system_uses_lse_atomics(void)
{
- return (static_branch_likely(&arm64_const_caps_ready)) &&
- static_branch_likely(&cpu_hwcap_keys[ARM64_HAS_LSE_ATOMICS]);
+ if ((static_branch_likely(&arm64_const_caps_ready)) &&
+ static_branch_likely(&cpu_hwcap_keys[ARM64_HAS_LSE_ATOMICS]))
+ return true;
+
+ return false;
}
- change is as follows, and we don't panic, but get stuck elsewhere in boot.
diff --git a/arch/arm64/include/asm/lse.h b/arch/arm64/include/asm/lse.h
index 80b388278149..7c1d51fa54b2 100644
--- a/arch/arm64/include/asm/lse.h
+++ b/arch/arm64/include/asm/lse.h
@@ -16,13 +16,17 @@
__asm__(".arch_extension lse");
+void panic(const char *fmt, ...) __noreturn __cold;
extern struct static_key_false cpu_hwcap_keys[ARM64_NCAPS];
extern struct static_key_false arm64_const_caps_ready;
static inline bool system_uses_lse_atomics(void)
{
- return (static_branch_likely(&arm64_const_caps_ready)) &&
- static_branch_likely(&cpu_hwcap_keys[ARM64_HAS_LSE_ATOMICS]);
+ if ((static_branch_likely(&arm64_const_caps_ready)) &&
+ static_branch_likely(&cpu_hwcap_keys[ARM64_HAS_LSE_ATOMICS]))
+ panic("ATOMICS");
+
+ return false;
}
- change system_uses_lse_atomics to return false and it always boots
Any ideas?
Thanks,
Andrew Murray
>
> There is another weird failure that might be somewhat related but I have
> no idea.
>
> https://github.com/ClangBuiltLinux/linux/issues/648
>
> Cheers,
> Nathan
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [GIT PULL] ARM: mvebu: dt for v5.4 (#1)
From: Arnd Bergmann @ 2019-09-03 14:29 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Andrew Lunn, Jason Cooper, linux-kernel@vger.kernel.org, SoC Team,
arm-soc, Olof Johansson, Linux ARM, Sebastian Hesselbarth
In-Reply-To: <878srdzjpj.fsf@FE-laptop>
On Wed, Aug 28, 2019 at 12:07 PM Gregory CLEMENT
<gregory.clement@bootlin.com> wrote:
> ----------------------------------------------------------------
> mvebu dt for 5.4 (part 1)
>
> - Disable the kirkwood RTC that doesn't work on the ts219 board
>
Pulled into arm/dt, thanks!
Arnd
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox