Re: [PATCH v2] arm64: lse: fix LSE atomics with LLVM's integrated assembler
On Tue, Oct 8, 2019 at 4:31 PM Andrew Murray wrote: > This looks good to me. I can build and boot in a model with both Clang > (9.0.6) and GCC (7.3.1) and boot a guest without anything going bang. Great, thank you for testing this! > Though when I build with AS=clang, e.g. > > make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- CC=clang AS=clang Image Note that this patch only fixes issues with inline assembly, which should at some point allow us to drop -no-integrated-as from clang builds. I believe there are still other fixes needed before AS=clang works. > I get errors like this: > > CC init/main.o > In file included from init/main.c:17: > In file included from ./include/linux/module.h:9: > In file included from ./include/linux/list.h:9: > In file included from ./include/linux/kernel.h:12: > In file included from ./include/linux/bitops.h:26: > In file included from ./arch/arm64/include/asm/bitops.h:26: > In file included from ./include/asm-generic/bitops/atomic.h:5: > In file included from ./include/linux/atomic.h:7: > In file included from ./arch/arm64/include/asm/atomic.h:16: > In file included from ./arch/arm64/include/asm/cmpxchg.h:14: > In file included from ./arch/arm64/include/asm/lse.h:13: > In file included from ./include/linux/jump_label.h:117: > ./arch/arm64/include/asm/jump_label.h:24:20: error: expected a symbol > reference in '.long' directive > " .align 3 \n\t" > ^ > :4:21: note: instantiated into assembly here > .long 1b - ., "" - . >^ > > I'm assuming that I'm doing something wrong? No, this particular issue will be fixed in clang 10: https://github.com/ClangBuiltLinux/linux/issues/500 Sami
Re: [PATCH v2 0/2] Avoid regmap debugfs collisions in qcom llcc driver
On Tue 08 Oct 16:45 PDT 2019, Stephen Boyd wrote:
> Now a two part series. These patches fix a debugfs name collision for
> the llcc regmaps and moves the config to a local variable to save on
> image size.
>
> Changes from v1
> (https://lkml.kernel.org/r/20191004233132.194336-1-swb...@chromium.org):
> * New second patch
> * Dropped static
> * See range-diff below!
>
> Stephen Boyd (2):
> soc: qcom: llcc: Name regmaps to avoid collisions
> soc: qcom: llcc: Move regmap config to local variable
>
> drivers/soc/qcom/llcc-slice.c | 14 +++---
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> Cc: Venkata Narendra Kumar Gutta
> Cc: Evan Green
>
> Range-diff against v1:
> -: > 1: 07bc0e8bdb6e soc: qcom: llcc: Name regmaps to avoid
> collisions
> 1: 0c54fc8a7ed6 ! 2: 5c4446e36783 soc: qcom: llcc: Name regmaps to avoid
> collisions
> @@ Metadata
> Author: Stephen Boyd
>
> ## Commit message ##
> -soc: qcom: llcc: Name regmaps to avoid collisions
> +soc: qcom: llcc: Move regmap config to local variable
>
> -We'll end up with debugfs collisions if we don't give names to the
> -regmaps created inside this driver. Copy the template config over
> into
> -this function and give the regmap the same name as the resource name.
> +This is now a global variable that we're modifying to fix the name.
> +That isn't terribly thread safe and it's not necessary to be a
> global so
> +let's just move this to a local variable instead. This saves space in
> +the symtab and actually reduces kernel image size because the regmap
> +config is large and we can replace the initialization of that
> structure
> +with a memset and a few member assignments.
>
> -Fixes: 7f9c136216c7 ("soc: qcom: Add broadcast base for Last Level
> Cache Controller (LLCC)")
> -Cc: Venkata Narendra Kumar Gutta
> -Cc: Evan Green
> Signed-off-by: Stephen Boyd
>
> ## drivers/soc/qcom/llcc-slice.c ##
> @@ drivers/soc/qcom/llcc-slice.c
>
> static struct llcc_drv_data *drv_data = (void *) -EPROBE_DEFER;
>
> --static const struct regmap_config llcc_regmap_config = {
> +-static struct regmap_config llcc_regmap_config = {
> -.reg_bits = 32,
> -.reg_stride = 4,
> -.val_bits = 32,
> @@ drivers/soc/qcom/llcc-slice.c: static struct regmap
> *qcom_llcc_init_mmio(struct
> {
> struct resource *res;
> void __iomem *base;
> -+static struct regmap_config llcc_regmap_config = {
> ++struct regmap_config llcc_regmap_config = {
Now that this isn't static I like the end result better. Not sure about
the need for splitting it in two patches, but if Evan is happy I'll take
it.
Regards,
Bjorn
> +.reg_bits = 32,
> +.reg_stride = 4,
> +.val_bits = 32,
> @@ drivers/soc/qcom/llcc-slice.c: static struct regmap
> *qcom_llcc_init_mmio(struct
>
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, name);
> if (!res)
> -@@ drivers/soc/qcom/llcc-slice.c: static struct regmap
> *qcom_llcc_init_mmio(struct platform_device *pdev,
> - if (IS_ERR(base))
> - return ERR_CAST(base);
> -
> -+llcc_regmap_config.name = name;
> - return devm_regmap_init_mmio(&pdev->dev, base,
> &llcc_regmap_config);
> - }
> -
>
> base-commit: 8b0eed9f6e36a5488967b0acc51444d658dd711b
> --
> Sent by a computer through tubes
>
[PATCH] ASoC: jz4740: Remove unused match variable
After commit 67ad656bdd70 ("ASoC: jz4740: Use
of_device_get_match_data()"), the match local variable is unused and the
compiler rightly warns.
sound/soc/jz4740/jz4740-i2s.c: In function 'jz4740_i2s_dev_probe':
sound/soc/jz4740/jz4740-i2s.c:500:29: warning: unused variable 'match'
[-Wunused-variable]
500 | const struct of_device_id *match;
Drop it.
Reported-by: Stephen Rothwell
Cc: Arnd Bergmann
Cc: Geert Uytterhoeven
Cc: Paul Cercueil
Cc: Liam Girdwood
Cc: Mark Brown
Cc: Jaroslav Kysela
Cc: Takashi Iwai
Cc: Rob Herring
Cc: Frank Rowand
Cc:
Fixes: 67ad656bdd70 ("ASoC: jz4740: Use of_device_get_match_data()")
Signed-off-by: Stephen Boyd
---
Can also be squashed. Sorry I missed this warning in the compile log.
sound/soc/jz4740/jz4740-i2s.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/sound/soc/jz4740/jz4740-i2s.c b/sound/soc/jz4740/jz4740-i2s.c
index d2dab4d24b87..38d48d101783 100644
--- a/sound/soc/jz4740/jz4740-i2s.c
+++ b/sound/soc/jz4740/jz4740-i2s.c
@@ -497,7 +497,6 @@ static int jz4740_i2s_dev_probe(struct platform_device
*pdev)
struct jz4740_i2s *i2s;
struct resource *mem;
int ret;
- const struct of_device_id *match;
i2s = devm_kzalloc(&pdev->dev, sizeof(*i2s), GFP_KERNEL);
if (!i2s)
base-commit: 442630f691a1537b7e0cc35e3d580222077549cb
--
Sent by a computer through tubes
Re: [PATCH v2 3/3] phy: qcom-qmp: Add SM8150 QMP UFS PHY support
On Tue 08 Oct 00:46 PDT 2019, Jack Pham wrote: > On Fri, Sep 06, 2019 at 10:40:17AM +0530, Vinod Koul wrote: [..] > I was thinking of taking a stab at USB if I get time, not sure if that's > already on your or somebody's (Bjorn?) radar. > We only have remote access to the hardware, making it rather tedious to do USB. So it's all yours! :) Regards, Bjorn
[PATCH v2 1/2] soc: qcom: llcc: Name regmaps to avoid collisions
We'll end up with debugfs collisions if we don't give names to the
regmaps created by this driver. Change the name of the config before
registering it so we don't collide in debugfs.
Fixes: 7f9c136216c7 ("soc: qcom: Add broadcast base for Last Level Cache
Controller (LLCC)")
Cc: Venkata Narendra Kumar Gutta
Cc: Evan Green
Signed-off-by: Stephen Boyd
---
drivers/soc/qcom/llcc-slice.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/qcom/llcc-slice.c b/drivers/soc/qcom/llcc-slice.c
index 9090ea12eaf3..4a6111635f82 100644
--- a/drivers/soc/qcom/llcc-slice.c
+++ b/drivers/soc/qcom/llcc-slice.c
@@ -48,7 +48,7 @@
static struct llcc_drv_data *drv_data = (void *) -EPROBE_DEFER;
-static const struct regmap_config llcc_regmap_config = {
+static struct regmap_config llcc_regmap_config = {
.reg_bits = 32,
.reg_stride = 4,
.val_bits = 32,
@@ -323,6 +323,7 @@ static struct regmap *qcom_llcc_init_mmio(struct
platform_device *pdev,
if (IS_ERR(base))
return ERR_CAST(base);
+ llcc_regmap_config.name = name;
return devm_regmap_init_mmio(&pdev->dev, base, &llcc_regmap_config);
}
--
Sent by a computer through tubes
[PATCH v2 2/2] soc: qcom: llcc: Move regmap config to local variable
This is now a global variable that we're modifying to fix the name.
That isn't terribly thread safe and it's not necessary to be a global so
let's just move this to a local variable instead. This saves space in
the symtab and actually reduces kernel image size because the regmap
config is large and we can replace the initialization of that structure
with a memset and a few member assignments.
Cc: Venkata Narendra Kumar Gutta
Cc: Evan Green
Signed-off-by: Stephen Boyd
---
drivers/soc/qcom/llcc-slice.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/soc/qcom/llcc-slice.c b/drivers/soc/qcom/llcc-slice.c
index 4a6111635f82..50aea3f0be41 100644
--- a/drivers/soc/qcom/llcc-slice.c
+++ b/drivers/soc/qcom/llcc-slice.c
@@ -48,13 +48,6 @@
static struct llcc_drv_data *drv_data = (void *) -EPROBE_DEFER;
-static struct regmap_config llcc_regmap_config = {
- .reg_bits = 32,
- .reg_stride = 4,
- .val_bits = 32,
- .fast_io = true,
-};
-
/**
* llcc_slice_getd - get llcc slice descriptor
* @uid: usecase_id for the client
@@ -314,6 +307,12 @@ static struct regmap *qcom_llcc_init_mmio(struct
platform_device *pdev,
{
struct resource *res;
void __iomem *base;
+ struct regmap_config llcc_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .fast_io = true,
+ };
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, name);
if (!res)
--
Sent by a computer through tubes
Re: [PATCH 4.19 001/106] tpm: use tpm_try_get_ops() in tpm-sysfs.c.
On Tue, Oct 08, 2019 at 02:51:20PM +0200, Pavel Machek wrote: > For example this did not have any locking, and is now protected by > > get_device(&chip->dev); > > down_read(&chip->ops_sem); > > . Is that intended? Is this known to fix any bugs? It is, sysfs code can otherwise race when ops is set NULL in class shutdown. /Jarkko
[PATCH v2 0/2] Avoid regmap debugfs collisions in qcom llcc driver
Now a two part series. These patches fix a debugfs name collision for
the llcc regmaps and moves the config to a local variable to save on
image size.
Changes from v1
(https://lkml.kernel.org/r/20191004233132.194336-1-swb...@chromium.org):
* New second patch
* Dropped static
* See range-diff below!
Stephen Boyd (2):
soc: qcom: llcc: Name regmaps to avoid collisions
soc: qcom: llcc: Move regmap config to local variable
drivers/soc/qcom/llcc-slice.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
Cc: Venkata Narendra Kumar Gutta
Cc: Evan Green
Range-diff against v1:
-: > 1: 07bc0e8bdb6e soc: qcom: llcc: Name regmaps to avoid
collisions
1: 0c54fc8a7ed6 ! 2: 5c4446e36783 soc: qcom: llcc: Name regmaps to avoid
collisions
@@ Metadata
Author: Stephen Boyd
## Commit message ##
-soc: qcom: llcc: Name regmaps to avoid collisions
+soc: qcom: llcc: Move regmap config to local variable
-We'll end up with debugfs collisions if we don't give names to the
-regmaps created inside this driver. Copy the template config over into
-this function and give the regmap the same name as the resource name.
+This is now a global variable that we're modifying to fix the name.
+That isn't terribly thread safe and it's not necessary to be a global
so
+let's just move this to a local variable instead. This saves space in
+the symtab and actually reduces kernel image size because the regmap
+config is large and we can replace the initialization of that structure
+with a memset and a few member assignments.
-Fixes: 7f9c136216c7 ("soc: qcom: Add broadcast base for Last Level
Cache Controller (LLCC)")
-Cc: Venkata Narendra Kumar Gutta
-Cc: Evan Green
Signed-off-by: Stephen Boyd
## drivers/soc/qcom/llcc-slice.c ##
@@ drivers/soc/qcom/llcc-slice.c
static struct llcc_drv_data *drv_data = (void *) -EPROBE_DEFER;
--static const struct regmap_config llcc_regmap_config = {
+-static struct regmap_config llcc_regmap_config = {
- .reg_bits = 32,
- .reg_stride = 4,
- .val_bits = 32,
@@ drivers/soc/qcom/llcc-slice.c: static struct regmap
*qcom_llcc_init_mmio(struct
{
struct resource *res;
void __iomem *base;
-+ static struct regmap_config llcc_regmap_config = {
++ struct regmap_config llcc_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
@@ drivers/soc/qcom/llcc-slice.c: static struct regmap
*qcom_llcc_init_mmio(struct
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, name);
if (!res)
-@@ drivers/soc/qcom/llcc-slice.c: static struct regmap
*qcom_llcc_init_mmio(struct platform_device *pdev,
- if (IS_ERR(base))
- return ERR_CAST(base);
-
-+ llcc_regmap_config.name = name;
- return devm_regmap_init_mmio(&pdev->dev, base, &llcc_regmap_config);
- }
-
base-commit: 8b0eed9f6e36a5488967b0acc51444d658dd711b
--
Sent by a computer through tubes
Re: [PATCH 5.4 regression fix] Input: soc_button_array - partial revert of support for newer surface devices
On Sat, Oct 05, 2019 at 12:55:51PM +0200, Hans de Goede wrote:
> Commit c394159310d0 ("Input: soc_button_array - add support for newer
> surface devices") not only added support for the MSHW0040 ACPI HID,
> but for some reason it also makes changes to the error handling of the
> soc_button_lookup_gpio() call in soc_button_device_create(). Note ideally
> this seamingly unrelated change would have been made in a separate commit,
> with a message explaining the what and why of this change.
>
> I guess this change may have been added to deal with -EPROBE_DEFER errors,
> but in case of the existing support for PNP0C40 devices, treating
> -EPROBE_DEFER as any other error is deliberate, see the comment this
> commit adds for why.
>
> The actual returning of -EPROBE_DEFER to the caller of soc_button_probe()
> introduced by the new error checking causes a serious regression:
>
> On devices with so called virtual GPIOs soc_button_lookup_gpio() will
> always return -EPROBE_DEFER for these fake GPIOs, when this happens
> during the second call of soc_button_device_create() we already have
> successfully registered our first child. This causes the kernel to think
> we are making progress with probing things even though we unregister the
> child before again before we return the -EPROBE_DEFER. Since we are making
> progress the kernel will retry deferred-probes again immediately ending
> up stuck in a loop with the following showing in dmesg:
>
> [ 124.022697] input: gpio-keys as
> /devices/platform/INTCFD9:00/gpio-keys.0.auto/input/input6537
> [ 124.040764] input: gpio-keys as
> /devices/platform/INTCFD9:00/gpio-keys.0.auto/input/input6538
> [ 124.056967] input: gpio-keys as
> /devices/platform/INTCFD9:00/gpio-keys.0.auto/input/input6539
> [ 124.072143] input: gpio-keys as
> /devices/platform/INTCFD9:00/gpio-keys.0.auto/input/input6540
> [ 124.092373] input: gpio-keys as
> /devices/platform/INTCFD9:00/gpio-keys.0.auto/input/input6541
> [ 124.108065] input: gpio-keys as
> /devices/platform/INTCFD9:00/gpio-keys.0.auto/input/input6542
> [ 124.128483] input: gpio-keys as
> /devices/platform/INTCFD9:00/gpio-keys.0.auto/input/input6543
> [ 124.147141] input: gpio-keys as
> /devices/platform/INTCFD9:00/gpio-keys.0.auto/input/input6544
> [ 124.165070] input: gpio-keys as
> /devices/platform/INTCFD9:00/gpio-keys.0.auto/input/input6545
> [ 124.179775] input: gpio-keys as
> /devices/platform/INTCFD9:00/gpio-keys.0.auto/input/input6546
> [ 124.202726] input: gpio-keys as
> /devices/platform/INTCFD9:00/gpio-keys.0.auto/input/input6547
>
>
> And 1 CPU core being stuck at 100% and udev hanging since it is waiting
> for the modprobe of soc_button_array to return.
>
> This patch reverts the soc_button_lookup_gpio() error handling changes,
> fixing this regression.
>
> Fixes: c394159310d0 ("Input: soc_button_array - add support for newer surface
> devices")
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=205031
> Cc: Maximilian Luz
> Signed-off-by: Hans de Goede
Applied, thank you.
> ---
> drivers/input/misc/soc_button_array.c | 17 -
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/input/misc/soc_button_array.c
> b/drivers/input/misc/soc_button_array.c
> index 97e3639e99d0..97761421d6dd 100644
> --- a/drivers/input/misc/soc_button_array.c
> +++ b/drivers/input/misc/soc_button_array.c
> @@ -92,11 +92,18 @@ soc_button_device_create(struct platform_device *pdev,
> continue;
>
> gpio = soc_button_lookup_gpio(&pdev->dev, info->acpi_index);
> - if (gpio < 0 && gpio != -ENOENT) {
> - error = gpio;
> - goto err_free_mem;
> - } else if (!gpio_is_valid(gpio)) {
> - /* Skip GPIO if not present */
> + if (!gpio_is_valid(gpio)) {
> + /*
> + * Skip GPIO if not present. Note we deliberately
> + * ignore -EPROBE_DEFER errors here. On some devices
> + * Intel is using so called virtual GPIOs which are not
> + * GPIOs at all but some way for AML code to check some
> + * random status bits without need a custom opregion.
> + * In some cases the resources table we parse points to
> + * such a virtual GPIO, since these are not real GPIOs
> + * we do not have a driver for these so they will never
> + * show up, therefor we ignore -EPROBE_DEFER.
> + */
> continue;
> }
>
> --
> 2.23.0
>
--
Dmitry
Re: [PATCH] ARC: mm: remove __ARCH_USE_5LEVEL_HACK
On Tue, Oct 08, 2019 at 09:38:36PM +, Vineet Gupta wrote: > Add the intermediate p4d accessors to make it 5 level compliant. > > Thi sis non-functional change anyways since ARC has software page walker ^ Typo. > with 2 lookup levels (pgd -> pte) > > Signed-off-by: Vineet Gupta > --- > arch/arc/include/asm/pgtable.h | 1 - > arch/arc/mm/fault.c| 10 -- > 2 files changed, 8 insertions(+), 3 deletions(-) Have you tested it with CONFIG_HIGHMEM=y? alloc_kmap_pgtable() has to be converted too. -- Kirill A. Shutemov
Re: [PATCH 4.19 002/106] tpm: Fix TPM 1.2 Shutdown sequence to prevent future TPM operations
> > --- a/drivers/char/tpm/tpm-chip.c
> > +++ b/drivers/char/tpm/tpm-chip.c
> > @@ -187,12 +187,13 @@ static int tpm_class_shutdown(struct device *dev)
> > {
> > struct tpm_chip *chip = container_of(dev, struct tpm_chip, dev);
> >
> > + down_write(&chip->ops_sem);
> > if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> > - down_write(&chip->ops_sem);
> > tpm2_shutdown(chip, TPM2_SU_CLEAR);
> > chip->ops = NULL;
> > - up_write(&chip->ops_sem);
> > }
> > + chip->ops = NULL;
> > + up_write(&chip->ops_sem);
> >
> > return 0;
> > }
>
> Still can be improved -- chip->ops = NULL; is done twice, copy inside
> the if {} is redundant...
Thanks. I can update this.
/Jarkko
RE: [BUGFIX PATCH] selftests: Use real temporary working directory for archiving
> -Original Message-
> From: Masami Hiramatsu on Thursday, October 03, 2019 7:13 PM
>
> Use real temporary working directory for generating kselftest
> archive.
>
> tools/testing/selftests/kselftest directory has been used for
> the temporary working directory for making a tar archive from
> gen_kselftest_tar.sh, and it removes the directory for cleanup.
>
> However, since the kselftest directory became a part of the
> repository, it must not be used as a working dir.
I'm not objecting to the test, but I would like to understand this
point better. Under normal circumstances (i.e. when not using O= or
KBUILD_OUTPUT)
the rest of the kernel directory structure holds generated files.
What is the issue with using kselftest to hold generated files?
-- Tim
>
> Introduce mktemp to prepare a temporary working directory
> for archiving kselftests.
>
> Signed-off-by: Masami Hiramatsu
> ---
> tools/testing/selftests/gen_kselftest_tar.sh |8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/gen_kselftest_tar.sh
> b/tools/testing/selftests/gen_kselftest_tar.sh
> index a27e2eec3586..eba1e9987ffc 100755
> --- a/tools/testing/selftests/gen_kselftest_tar.sh
> +++ b/tools/testing/selftests/gen_kselftest_tar.sh
> @@ -38,16 +38,16 @@ main()
> esac
> fi
>
> - install_dir=./kselftest
> + tmpdir=`mktemp -d ./install-XX` || exit 1
>
> # Run install using INSTALL_KSFT_PATH override to generate install
> # directory
> -./kselftest_install.sh
> -tar $copts kselftest${ext} $install_dir
> +./kselftest_install.sh $tmpdir
> +tar $copts kselftest${ext} -C $tmpdir kselftest
> echo "Kselftest archive kselftest${ext} created!"
>
> # clean up install directory
> -rm -rf kselftest
> +rm -rf $tmpdir
> }
>
> main "$@"
Re: [PATCH] riscv: Fix memblock reservation for device tree blob
On Tue, 08 Oct 2019 16:31:17 PDT (-0700), a...@eecs.berkeley.edu wrote:
On 2019-10-08 15:38:15 -0700, Palmer Dabbelt wrote:
On Fri, 20 Sep 2019 21:34:57 PDT (-0700), a...@brainfault.org wrote:
> On Sat, Sep 21, 2019 at 6:30 AM Albert Ou wrote:
>>
>> This fixes an error with how the FDT blob is reserved in memblock.
>> An incorrect physical address calculation exposed the FDT header to
>> unintended corruption, which typically manifested with of_fdt_raw_init()
>> faulting during late boot after fdt_totalsize() returned a wrong value.
>> Systems with smaller physical memory sizes more frequently trigger this
>> issue, as the kernel is more likely to allocate from the DMA32 zone
>> where bbl places the DTB after the kernel image.
>>
>> Commit 671f9a3e2e24 ("RISC-V: Setup initial page tables in two stages")
>> changed the mapping of the DTB to reside in the fixmap area.
>> Consequently, early_init_fdt_reserve_self() cannot be used anymore in
>> setup_bootmem() since it relies on __pa() to derive a physical address,
>> which does not work with dtb_early_va that is no longer a valid kernel
>> logical address.
>>
>> The reserved[0x1] region shows the effect of the pointer underflow
>> resulting from the __pa(initial_boot_params) offset subtraction:
>>
>> [0.00] MEMBLOCK configuration:
>> [0.00] memory size = 0x1fe0 reserved size =
0x00a2e514
>> [0.00] memory.cnt = 0x1
>> [0.00] memory[0x0] [0x8020-0x9fff],
0x1fe0 bytes flags: 0x0
>> [0.00] reserved.cnt = 0x2
>> [0.00] reserved[0x0] [0x8020-0x80c2dfeb],
0x00a2dfec bytes flags: 0x0
>> [0.00] reserved[0x1] [0xfff08010-0xfff080100527],
0x0528 bytes flags: 0x0
>>
>> With the fix applied:
>>
>> [0.00] MEMBLOCK configuration:
>> [0.00] memory size = 0x1fe0 reserved size =
0x00a2e514
>> [0.00] memory.cnt = 0x1
>> [0.00] memory[0x0] [0x8020-0x9fff],
0x1fe0 bytes flags: 0x0
>> [0.00] reserved.cnt = 0x2
>> [0.00] reserved[0x0] [0x8020-0x80c2dfeb],
0x00a2dfec bytes flags: 0x0
>> [0.00] reserved[0x1] [0x80e0-0x80e00527],
0x0528 bytes flags: 0x0
>
> Thanks for catching this issue.
>
> Most of us did not notice this issue most likely because:
> 1. We generally have good enough RAM on QEMU and SiFive Unleashed
> 2. Most of people use OpenSBI FW_JUMP on QEMU and U-Boot on
> SiFive Unleashed to boot in Linux which places FDT quite far away
> from Linux kernel end
>
> Linux ARM64 kernel also uses FIXMAP to access FDT and over there
> as well early_init_fdt_reserve_self() is not used.
>
>>
>> Fixes: 671f9a3e2e24 ("RISC-V: Setup initial page tables in two stages")
>> Signed-off-by: Albert Ou
>> ---
>> arch/riscv/mm/init.c | 13 -
>> 1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
>> index f0ba713..52d007c 100644
>> --- a/arch/riscv/mm/init.c
>> +++ b/arch/riscv/mm/init.c
>> @@ -11,6 +11,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include
>> #include
>> @@ -82,6 +83,8 @@ static void __init setup_initrd(void)
>> }
>> #endif /* CONFIG_BLK_DEV_INITRD */
>>
>> +static phys_addr_t __dtb_pa __initdata;
>
> May be dtb_early_pa will be more consistent name
> instead of __dtb_pa because it matches dtb_early_va
> used below.
>
>> +
>> void __init setup_bootmem(void)
>> {
>> struct memblock_region *reg;
>> @@ -117,7 +120,12 @@ void __init setup_bootmem(void)
>> setup_initrd();
>> #endif /* CONFIG_BLK_DEV_INITRD */
>>
>> - early_init_fdt_reserve_self();
>> + /*
>> +* Avoid using early_init_fdt_reserve_self() since __pa() does
>> +* not work for DTB pointers that are fixmap addresses
>> +*/
>> + memblock_reserve(__dtb_pa, fdt_totalsize(dtb_early_va));
>> +
>> early_init_fdt_scan_reserved_mem();
>> memblock_allow_resize();
>> memblock_dump_all();
>> @@ -333,6 +341,7 @@ static uintptr_t __init best_map_size(phys_addr_t base,
phys_addr_t size)
>> "not use absolute addressing."
>> #endif
>>
>> +
>
> Please remove this newline addition.
>
>> asmlinkage void __init setup_vm(uintptr_t dtb_pa)
>> {
>> uintptr_t va, end_va;
>> @@ -393,6 +402,8 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
>>
>> /* Save pointer to DTB for early FDT parsing */
>> dtb_early_va = (void *)fix_to_virt(FIX_FDT) + (dtb_pa & ~PAGE_MASK);
>> + /* Save physical address for memblock reservation */
>> + __dtb_pa = dtb_pa;
>> }
>>
>> static void __init setup_vm_final(void)
>> --
>> 2.7.4
>>
>>
>> ___
>> linux-riscv mailing list
>> linux-ri...@lists.infradead.org
>> h
Re: [PATCH v2] arm64: lse: fix LSE atomics with LLVM's integrated assembler
On Tue, Oct 08, 2019 at 02:27:30PM -0700, Sami Tolvanen wrote:
> Unlike gcc, clang considers each inline assembly block to be independent
> and therefore, when using the integrated assembler for inline assembly,
> any preambles that enable features must be repeated in each block.
>
> This change defines __LSE_PREAMBLE and adds it to each inline assembly
> block that has LSE instructions, which allows them to be compiled also
> with clang's assembler.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/671
> Signed-off-by: Sami Tolvanen
This looks good to me. I can build and boot in a model with both Clang
(9.0.6) and GCC (7.3.1) and boot a guest without anything going bang.
Though when I build with AS=clang, e.g.
make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- CC=clang AS=clang Image
I get errors like this:
CC init/main.o
In file included from init/main.c:17:
In file included from ./include/linux/module.h:9:
In file included from ./include/linux/list.h:9:
In file included from ./include/linux/kernel.h:12:
In file included from ./include/linux/bitops.h:26:
In file included from ./arch/arm64/include/asm/bitops.h:26:
In file included from ./include/asm-generic/bitops/atomic.h:5:
In file included from ./include/linux/atomic.h:7:
In file included from ./arch/arm64/include/asm/atomic.h:16:
In file included from ./arch/arm64/include/asm/cmpxchg.h:14:
In file included from ./arch/arm64/include/asm/lse.h:13:
In file included from ./include/linux/jump_label.h:117:
./arch/arm64/include/asm/jump_label.h:24:20: error: expected a symbol reference
in '.long' directive
" .align 3 \n\t"
^
:4:21: note: instantiated into assembly here
.long 1b - ., "" - .
^
I'm assuming that I'm doing something wrong?
Thanks,
Andrew Murray
> ---
> v2:
> - Add a preamble to inline assembly blocks that use LSE instead
>of allowing the compiler to emit LSE instructions everywhere.
>
> ---
> arch/arm64/include/asm/atomic_lse.h | 19 +++
> arch/arm64/include/asm/lse.h| 6 +++---
> 2 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/include/asm/atomic_lse.h
> b/arch/arm64/include/asm/atomic_lse.h
> index c6bd87d2915b..3ee600043042 100644
> --- a/arch/arm64/include/asm/atomic_lse.h
> +++ b/arch/arm64/include/asm/atomic_lse.h
> @@ -14,6 +14,7 @@
> static inline void __lse_atomic_##op(int i, atomic_t *v)
> \
> {\
> asm volatile( \
> + __LSE_PREAMBLE \
> "" #asm_op " %w[i], %[v]\n" \
> : [i] "+r" (i), [v] "+Q" (v->counter) \
> : "r" (v)); \
> @@ -30,6 +31,7 @@ ATOMIC_OP(add, stadd)
> static inline int __lse_atomic_fetch_##op##name(int i, atomic_t *v) \
> {\
> asm volatile( \
> + __LSE_PREAMBLE \
> "" #asm_op #mb " %w[i], %w[i], %[v]" \
> : [i] "+r" (i), [v] "+Q" (v->counter) \
> : "r" (v) \
> @@ -58,6 +60,7 @@ static inline int __lse_atomic_add_return##name(int i,
> atomic_t *v) \
> u32 tmp;\
> \
> asm volatile( \
> + __LSE_PREAMBLE \
> " ldadd" #mb "%w[i], %w[tmp], %[v]\n" \
> " add %w[i], %w[i], %w[tmp]" \
> : [i] "+r" (i), [v] "+Q" (v->counter), [tmp] "=&r" (tmp)\
> @@ -77,6 +80,7 @@ ATOMIC_OP_ADD_RETURN(, al, "memory")
> static inline void __lse_atomic_and(int i, atomic_t *v)
> {
> asm volatile(
> + __LSE_PREAMBLE
> " mvn %w[i], %w[i]\n"
> " stclr %w[i], %[v]"
> : [i] "+&r" (i), [v] "+Q" (v->counter)
> @@ -87,6 +91,7 @@ static inline void __lse_atomic_and(int i, atomic_t *v)
> static inline int __lse_atomic_fetch_and##name(int i, atomic_t *v) \
> {\
> asm volatile( \
> + __LSE_PREAMBLE \
> " mvn %w[i], %w[i]\n" \
> " ldclr" #mb "%w[i
Re: [PATCH] riscv: Fix memblock reservation for device tree blob
On 2019-10-08 15:38:15 -0700, Palmer Dabbelt wrote:
> On Fri, 20 Sep 2019 21:34:57 PDT (-0700), a...@brainfault.org wrote:
> > On Sat, Sep 21, 2019 at 6:30 AM Albert Ou wrote:
> >>
> >> This fixes an error with how the FDT blob is reserved in memblock.
> >> An incorrect physical address calculation exposed the FDT header to
> >> unintended corruption, which typically manifested with of_fdt_raw_init()
> >> faulting during late boot after fdt_totalsize() returned a wrong value.
> >> Systems with smaller physical memory sizes more frequently trigger this
> >> issue, as the kernel is more likely to allocate from the DMA32 zone
> >> where bbl places the DTB after the kernel image.
> >>
> >> Commit 671f9a3e2e24 ("RISC-V: Setup initial page tables in two stages")
> >> changed the mapping of the DTB to reside in the fixmap area.
> >> Consequently, early_init_fdt_reserve_self() cannot be used anymore in
> >> setup_bootmem() since it relies on __pa() to derive a physical address,
> >> which does not work with dtb_early_va that is no longer a valid kernel
> >> logical address.
> >>
> >> The reserved[0x1] region shows the effect of the pointer underflow
> >> resulting from the __pa(initial_boot_params) offset subtraction:
> >>
> >> [0.00] MEMBLOCK configuration:
> >> [0.00] memory size = 0x1fe0 reserved size =
> >> 0x00a2e514
> >> [0.00] memory.cnt = 0x1
> >> [0.00] memory[0x0] [0x8020-0x9fff],
> >> 0x1fe0 bytes flags: 0x0
> >> [0.00] reserved.cnt = 0x2
> >> [0.00] reserved[0x0] [0x8020-0x80c2dfeb],
> >> 0x00a2dfec bytes flags: 0x0
> >> [0.00] reserved[0x1] [0xfff08010-0xfff080100527],
> >> 0x0528 bytes flags: 0x0
> >>
> >> With the fix applied:
> >>
> >> [0.00] MEMBLOCK configuration:
> >> [0.00] memory size = 0x1fe0 reserved size =
> >> 0x00a2e514
> >> [0.00] memory.cnt = 0x1
> >> [0.00] memory[0x0] [0x8020-0x9fff],
> >> 0x1fe0 bytes flags: 0x0
> >> [0.00] reserved.cnt = 0x2
> >> [0.00] reserved[0x0] [0x8020-0x80c2dfeb],
> >> 0x00a2dfec bytes flags: 0x0
> >> [0.00] reserved[0x1] [0x80e0-0x80e00527],
> >> 0x0528 bytes flags: 0x0
> >
> > Thanks for catching this issue.
> >
> > Most of us did not notice this issue most likely because:
> > 1. We generally have good enough RAM on QEMU and SiFive Unleashed
> > 2. Most of people use OpenSBI FW_JUMP on QEMU and U-Boot on
> > SiFive Unleashed to boot in Linux which places FDT quite far away
> > from Linux kernel end
> >
> > Linux ARM64 kernel also uses FIXMAP to access FDT and over there
> > as well early_init_fdt_reserve_self() is not used.
> >
> >>
> >> Fixes: 671f9a3e2e24 ("RISC-V: Setup initial page tables in two stages")
> >> Signed-off-by: Albert Ou
> >> ---
> >> arch/riscv/mm/init.c | 13 -
> >> 1 file changed, 12 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> >> index f0ba713..52d007c 100644
> >> --- a/arch/riscv/mm/init.c
> >> +++ b/arch/riscv/mm/init.c
> >> @@ -11,6 +11,7 @@
> >> #include
> >> #include
> >> #include
> >> +#include
> >>
> >> #include
> >> #include
> >> @@ -82,6 +83,8 @@ static void __init setup_initrd(void)
> >> }
> >> #endif /* CONFIG_BLK_DEV_INITRD */
> >>
> >> +static phys_addr_t __dtb_pa __initdata;
> >
> > May be dtb_early_pa will be more consistent name
> > instead of __dtb_pa because it matches dtb_early_va
> > used below.
> >
> >> +
> >> void __init setup_bootmem(void)
> >> {
> >> struct memblock_region *reg;
> >> @@ -117,7 +120,12 @@ void __init setup_bootmem(void)
> >> setup_initrd();
> >> #endif /* CONFIG_BLK_DEV_INITRD */
> >>
> >> - early_init_fdt_reserve_self();
> >> + /*
> >> +* Avoid using early_init_fdt_reserve_self() since __pa() does
> >> +* not work for DTB pointers that are fixmap addresses
> >> +*/
> >> + memblock_reserve(__dtb_pa, fdt_totalsize(dtb_early_va));
> >> +
> >> early_init_fdt_scan_reserved_mem();
> >> memblock_allow_resize();
> >> memblock_dump_all();
> >> @@ -333,6 +341,7 @@ static uintptr_t __init best_map_size(phys_addr_t
> >> base, phys_addr_t size)
> >> "not use absolute addressing."
> >> #endif
> >>
> >> +
> >
> > Please remove this newline addition.
> >
> >> asmlinkage void __init setup_vm(uintptr_t dtb_pa)
> >> {
> >> uintptr_t va, end_va;
> >> @@ -393,6 +402,8 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
> >>
> >> /* Save pointer to DTB for early FDT parsing */
> >> dtb_early_va = (void *)fix_to_virt(FIX_FDT) + (dtb_pa &
> >> ~PAGE_MASK);
> >> + /* Save physical address for memblock reservation */
> >> + __dtb_p
Re: [PATCH] lib/list-test: add a test for the 'list' doubly linked list
On Mon, Oct 7, 2019 at 6:03 PM Kees Cook wrote:
(...)
> > +
> > +static void list_init_test(struct kunit *test)
> > +{
> > + /* Test the different ways of initialising a list. */
> > + struct list_head list1 = LIST_HEAD_INIT(list1);
> > + struct list_head list2;
> > + LIST_HEAD(list3);
> > +
> > + INIT_LIST_HEAD(&list2);
>
> Can you also add different storage locations and initial contents tests?
> For example:
>
> struct list_head *list4;
> struct list_head *list5;
>
> list4 = kzalloc(sizeof(*list4));
> INIT_LIST_HEAD(list4);
>
> list5 = kmalloc(sizeof(*list5));
> memset(list4, 0xff, sizeof(*list5));
> INIT_LIST_HEAD(list5);
>
>
> KUNIT_EXPECT_TRUE(test, list_empty_careful(&list4));
> KUNIT_EXPECT_TRUE(test, list_empty_careful(&list5));
>
> kfree(list4);
> kfree(list5);
>
> Then we know it's not an accident that INIT_LIST_HEAD() works when both
> all-zeros and all-0xFF initial contents are there. :)
Will do.
> > +static void list_entry_test(struct kunit *test)
> > +{
> > + struct list_test_struct test_struct;
>
> I guess this is not a missing initialization here because this is just
> testing the container_of() wrapper macro?
>
Yeah: we shouldn't be doing any memory accesses here, just the pointer
manipulation, so it shouldn't matter.
I can add a comment pointing this out, or just initialise it anyway.
> > +
> > + KUNIT_EXPECT_PTR_EQ(test, &test_struct,
> > list_entry(&(test_struct.list), struct list_test_struct, list));
> > +}
> > +
> > +static void list_first_entry_test(struct kunit *test)
> > +{
> > + struct list_test_struct test_struct1, test_struct2;
> > + static LIST_HEAD(list);
>
> This is this static?
>
Oops, this doesn't need to be static. I'll fix this (and the others)
for the next version.
> > +static void list_for_each_test(struct kunit *test)
> > +{
> > + struct list_head entries[3], *cur;
> > + LIST_HEAD(list);
> > + int i = 0;
> > +
> > + list_add_tail(&entries[0], &list);
> > + list_add_tail(&entries[1], &list);
> > + list_add_tail(&entries[2], &list);
> > +
> > + list_for_each(cur, &list) {
> > + KUNIT_EXPECT_PTR_EQ(test, cur, &entries[i]);
> > + i++;
> > + }
> > +
> > + KUNIT_EXPECT_EQ(test, i, 3);
> > +}
> > +
> > +static void list_for_each_prev_test(struct kunit *test)
> > +{
> > + struct list_head entries[3], *cur;
> > + LIST_HEAD(list);
> > + int i = 2;
> > +
> > + list_add_tail(&entries[0], &list);
> > + list_add_tail(&entries[1], &list);
> > + list_add_tail(&entries[2], &list);
> > +
> > + list_for_each_prev(cur, &list) {
> > + KUNIT_EXPECT_PTR_EQ(test, cur, &entries[i]);
> > + i--;
> > + }
> > +
> > + KUNIT_EXPECT_EQ(test, i, -1);
>
> Both of these tests test that the list contained the correct number of
> entries, not that anything about ordering was established. I would load
> values into these with the list_test_struct and test that the counting
> matches the expected ordering.
>
These tests do check the order of the entries (the order of the list
should match the order of the entries array, and KUNIT_EXPECT_PTR_EQ()
is verifying that the entries[i] is correct).
It would be possible to actually use list_test_struct like with the
list_for_each_entry tests, but since list_for_each just returns the
list_head, it didn't seem useful, so long as the list_head pointers
match.
> > +}
> > +
> > +static void list_for_each_safe_test(struct kunit *test)
> > +{
> > + struct list_head entries[3], *cur, *n;
> > + LIST_HEAD(list);
> > + int i = 0;
> > +
> > +
> > + list_add_tail(&entries[0], &list);
> > + list_add_tail(&entries[1], &list);
> > + list_add_tail(&entries[2], &list);
> > +
> > + list_for_each_safe(cur, n, &list) {
> > + KUNIT_EXPECT_PTR_EQ(test, cur, &entries[i]);
> > + list_del(&entries[i]);
> > + i++;
> > + }
> > +
> > + KUNIT_EXPECT_EQ(test, i, 3);
>
> I would expect an is_empty() test here too.
>
Will do.
> > +}
> > +
> > +static void list_for_each_prev_safe_test(struct kunit *test)
> > +{
> > + struct list_head entries[3], *cur, *n;
> > + LIST_HEAD(list);
> > + int i = 2;
> > +
> > + list_add_tail(&entries[0], &list);
> > + list_add_tail(&entries[1], &list);
> > + list_add_tail(&entries[2], &list);
> > +
> > + list_for_each_prev_safe(cur, n, &list) {
> > + KUNIT_EXPECT_PTR_EQ(test, cur, &entries[i]);
> > + list_del(&entries[i]);
> > + i--;
> > + }
>
> Same thing here.
>
Will do.
> > +static void list_for_each_entry_test(struct kunit *test)
> > +{
> > + struct list_test_struct entries[5], *cur;
> > + static LIST_HEAD(list);
> > + int i = 0;
> > +
> > + for (i = 0; i < 5; ++i) {
> > + entries[i].data = i;
> > + list_add_tail(&entries[i].list, &list
[PATCH] cpufreq: Avoid cpufreq_suspend() deadlock on system shutdown
From: Rafael J. Wysocki
It is incorrect to set the cpufreq syscore shutdown callback pointer
to cpufreq_suspend(), because that function cannot be run in the
syscore stage of system shutdown for two reasons: (a) it may attempt
to carry out actions depending on devices that have already been shut
down at that point and (b) the RCU synchronization carried out by it
may not be able to make progress then.
The latter issue has been present since commit 45975c7d21a1 ("rcu:
Define RCU-sched API in terms of RCU for Tree RCU PREEMPT builds"),
but the former one has always been there regardless.
Fix that by dropping cpufreq_syscore_ops altogether and making
device_shutdown() call cpufreq_suspend() directly before shutting
down devices, which is along the lines of what system-wide power
management does.
Fixes: 45975c7d21a1 ("rcu: Define RCU-sched API in terms of RCU for Tree RCU
PREEMPT builds")
Reported-by: Ville Syrjälä
Tested-by: Ville Syrjälä
Signed-off-by: Rafael J. Wysocki
---
drivers/base/core.c |3 +++
drivers/cpufreq/cpufreq.c | 10 --
2 files changed, 3 insertions(+), 10 deletions(-)
Index: linux-pm/drivers/cpufreq/cpufreq.c
===
--- linux-pm.orig/drivers/cpufreq/cpufreq.c
+++ linux-pm/drivers/cpufreq/cpufreq.c
@@ -2737,14 +2737,6 @@ int cpufreq_unregister_driver(struct cpu
}
EXPORT_SYMBOL_GPL(cpufreq_unregister_driver);
-/*
- * Stop cpufreq at shutdown to make sure it isn't holding any locks
- * or mutexes when secondary CPUs are halted.
- */
-static struct syscore_ops cpufreq_syscore_ops = {
- .shutdown = cpufreq_suspend,
-};
-
struct kobject *cpufreq_global_kobject;
EXPORT_SYMBOL(cpufreq_global_kobject);
@@ -2756,8 +2748,6 @@ static int __init cpufreq_core_init(void
cpufreq_global_kobject = kobject_create_and_add("cpufreq",
&cpu_subsys.dev_root->kobj);
BUG_ON(!cpufreq_global_kobject);
- register_syscore_ops(&cpufreq_syscore_ops);
-
return 0;
}
module_param(off, int, 0444);
Index: linux-pm/drivers/base/core.c
===
--- linux-pm.orig/drivers/base/core.c
+++ linux-pm/drivers/base/core.c
@@ -9,6 +9,7 @@
*/
#include
+#include
#include
#include
#include
@@ -3179,6 +3180,8 @@ void device_shutdown(void)
wait_for_device_probe();
device_block_probing();
+ cpufreq_suspend();
+
spin_lock(&devices_kset->list_lock);
/*
* Walk the devices list backward, shutting down each in turn.
Re: [PATCH] tpm: add check after commands attribs tab allocation
On Mon, Oct 07, 2019 at 02:46:37PM -0700, Tadeusz Struk wrote:
> devm_kcalloc() can fail and return NULL so we need to check for that.
>
> Fixes: 58472f5cd4f6f ("tpm: validate TPM 2.0 commands")
> Signed-off-by: Tadeusz Struk
Thank you.
Cc: sta...@vger.kernel.org
Reviewed-by: Jarkko Sakkinen
/Jarkko
Re: [PATCH v4 0/2] mm/memory-failure: Poison read receives SIGKILL instead of SIGBUS issue
Hi Jane, I think that this patchset is good enough and ready to be merged. Andrew, could you consider queuing this series into your tree? Thanks, Naoya Horiguchi On Tue, Oct 08, 2019 at 11:13:23AM -0700, Jane Chu wrote: > Hi, Naoya, > > What is the status of the patches? > Is there anything I need to do from my end ? > > Regards, > -jane > > On 8/6/2019 10:25 AM, Jane Chu wrote: > > Change in v4: > > - remove trailing white space > > > > Changes in v3: > > - move **tk cleanup to its own patch > > > > Changes in v2: > > - move 'tk' allocations internal to add_to_kill(), suggested by Dan; > > - ran checkpatch.pl check, pointed out by Matthew; > > - Noaya pointed out that v1 would have missed the SIGKILL > > if "tk->addr == -EFAULT", since the code returns early. > > Incorporated Noaya's suggestion, also, skip VMAs where > > "tk->size_shift == 0" for zone device page, and deliver SIGBUS > > when "tk->size_shift != 0" so the payload is helpful; > > - added Suggested-by: Naoya Horiguchi > > > > Jane Chu (2): > >mm/memory-failure.c clean up around tk pre-allocation > >mm/memory-failure: Poison read receives SIGKILL instead of SIGBUS if > > mmaped more than once > > > > mm/memory-failure.c | 62 > > ++--- > > 1 file changed, 26 insertions(+), 36 deletions(-) > > >
Re: [RFC/RFT][PATCH v8] cpuidle: New timer events oriented governor for tickless systems
On Tuesday, October 8, 2019 12:49:01 PM CEST Rafael J. Wysocki wrote:
> On Tue, Oct 8, 2019 at 11:51 AM Rafael J. Wysocki wrote:
> >
> > On Tue, Oct 8, 2019 at 8:20 AM Doug Smythies wrote:
> > >
> > > On 2019.10.06 08:34 Rafael J. Wysocki wrote:
> > > > On Sun, Oct 6, 2019 at 4:46 PM Doug Smythies
> > > > wrote:
> > > >> On 2019.10.01 02:32 Rafael J. Wysocki wrote:
> > > >>> On Sun, Sep 29, 2019 at 6:05 PM Doug Smythies
> > > >>> wrote:
> > > On 2019.09.26 09:32 Doug Smythies wrote:
> > >
> > > > If the deepest idle state is disabled, the system
> > > > can become somewhat unstable, with anywhere between no problem
> > > > at all, to the occasional temporary jump using a lot more
> > > > power for a few seconds, to a permanent jump using a lot more
> > > > power continuously. I have been unable to isolate the exact
> > > > test load conditions under which this will occur. However,
> > > > temporarily disabling and then enabling other idle states
> > > > seems to make for a somewhat repeatable test. It is important
> > > > to note that the issue occurs with only ever disabling the deepest
> > > > idle state, just not reliably.
> > > >
> > > > I want to know how you want to proceed before I do a bunch of
> > > > regression testing.
> > >
> > > >> I do not think I stated it clearly before: The problem here is that
> > > >> some CPUs
> > > >> seem to get stuck in idle state 0, and when they do power consumption
> > > >> spikes,
> > > >> often by several hundred % and often indefinitely.
> > > >
> > > > That indeed has not been clear to me, thanks for the clarification!
> > >
> > > >
> > > >> I made a hack job automated test:
> > > >> Kernel tests fail rate
> > > >> 5.4-rc16616 13.45%
> > > >> 5.3 23764.50%
> > > >> 5.3-teov7 121360.00% <<< teo.c reverted and teov7
> > > >> put in its place.
> > > >> 5.4-rc1-ds 111680.00% <<< [old] proposed patch (> 7
> > > >> hours test time)
> > >
> > >
> > >5.4-rc1-ds12 4224 0.005 <<< new proposed patch
> > >
> > > >>
> > > >> [old] Proposed patch (on top of kernel 5.4-rc1): [deleted]
> > >
> > > > This change may cause the deepest state to be selected even if its
> > > > "hits" metric is less than the "misses" one AFAICS, in which case the
> > > > max_early_index state should be selected instead.
> > > >
> > > > It looks like the max_early_index computation is broken when the
> > > > deepest state is disabled.
> > >
> > > O.K. Thanks for your quick reply, and insight.
> > >
> > > I think long durations always need to be counted, but currently if
> > > the deepest idle state is disabled, they are not.
> > > How about this?:
> > > (test results added above, more tests pending if this might be a path
> > > forward.)
> > >
> > > diff --git a/drivers/cpuidle/governors/teo.c
> > > b/drivers/cpuidle/governors/teo.c
> > > index b5a0e49..a970d2c 100644
> > > --- a/drivers/cpuidle/governors/teo.c
> > > +++ b/drivers/cpuidle/governors/teo.c
> > > @@ -155,10 +155,12 @@ static void teo_update(struct cpuidle_driver *drv,
> > > struct cpuidle_device *dev)
> > >
> > > cpu_data->states[i].early_hits -= early_hits >>
> > > DECAY_SHIFT;
> > >
> > > - if (drv->states[i].target_residency <= sleep_length_us) {
> > > - idx_timer = i;
> > > - if (drv->states[i].target_residency <=
> > > measured_us)
> > > - idx_hit = i;
> > > + if (!(drv->states[i].disabled ||
> > > dev->states_usage[i].disable)){
> > > + if (drv->states[i].target_residency <=
> > > sleep_length_us) {
> > > + idx_timer = i;
> > > + if (drv->states[i].target_residency <=
> > > measured_us)
> > > + idx_hit = i;
> > > + }
> >
> > What if the state is enabled again after some time?
>
> Actually, the states are treated as "bins" here, so for the metrics it
> doesn't matter whether or not they are enabled at the moment.
>
> > > }
> > > }
> > >
> > > @@ -256,39 +258,25 @@ static int teo_select(struct cpuidle_driver *drv,
> > > struct cpuidle_device *dev,
> > > struct cpuidle_state *s = &drv->states[i];
> > > struct cpuidle_state_usage *su = &dev->states_usage[i];
> > >
> > > - if (s->disabled || su->disable) {
> > > - /*
> > > -* If the "early hits" metric of a disabled state
> > > is
> > > -* greater than the current maximum, it should be
> > > taken
> > > -* into account, because it would be a mistake to
> > > select
> > > -* a deeper state with lower "early hits" metric.
> > > The
> > > -
Re: [PATCH] mm: thp: move deferred split queue to memcg's nodeinfo
On 10/8/19 7:44 AM, Kirill A. Shutemov wrote:
On Mon, Oct 07, 2019 at 04:30:30PM +0200, Michal Hocko wrote:
On Mon 07-10-19 16:19:59, Vlastimil Babka wrote:
On 10/2/19 10:43 AM, Michal Hocko wrote:
On Wed 02-10-19 06:16:43, Yang Shi wrote:
The commit 87eaceb3faa59b9b4d940ec9554ce251325d83fe ("mm: thp: make
deferred split shrinker memcg aware") makes deferred split queue per
memcg to resolve memcg pre-mature OOM problem. But, all nodes end up
sharing the same queue instead of one queue per-node before the commit.
It is not a big deal for memcg limit reclaim, but it may cause global
kswapd shrink THPs from a different node.
And, 0-day testing reported -19.6% regression of stress-ng's madvise
test [1]. I didn't see that much regression on my test box (24 threads,
48GB memory, 2 nodes), with the same test (stress-ng --timeout 1
--metrics-brief --sequential 72 --class vm --exclude spawn,exec), I saw
average -3% (run the same test 10 times then calculate the average since
the test itself may have most 15% variation according to my test)
regression sometimes (not every time, sometimes I didn't see regression
at all).
This might be caused by deferred split queue lock contention. With some
configuration (i.e. just one root memcg) the lock contention my be worse
than before (given 2 nodes, two locks are reduced to one lock).
So, moving deferred split queue to memcg's nodeinfo to make it NUMA
aware again.
With this change stress-ng's madvise test shows average 4% improvement
sometimes and I didn't see degradation anymore.
My concern about this getting more and more complex
(http://lkml.kernel.org/r/20191002084014.gh15...@dhcp22.suse.cz) holds
here even more. Can we step back and reconsider the whole thing please?
What about freeing immediately after split via workqueue and also have a
synchronous version called before going oom? Maybe there would be also
other things that would benefit from this scheme instead of traditional
reclaim and shrinkers?
That is exactly what we have discussed some time ago.
Yes, I've posted the patch:
http://lkml.kernel.org/r/20190827125911.boya23eowxhqmopa@box
But I still not sure that the approach is right. I expect it to trigger
performance regressions. For system with pleanty of free memory, we will
just pay split cost for nothing in many cases.
This is exactly what I'm concerned about as well.
Re: [PATCH net,v2 1/2] net: ethernet: mediatek: Fix MT7629 missing GMII mode support
On Mon, 7 Oct 2019 15:08:43 +0800, MarkLee wrote:
> Add missing configuration for mt7629 gmii mode support
>
> Fixes: 7e538372694b ("net: ethernet: mediatek: Re-add support SGMII")
Thank you for adding the Fixes tag. It seem, however, that the patch in
question did not change the ge_mode setting. Is it because GMII now
makes a call to mtk_gmac_gephy_path_setup() that the different setting
is required? The Fixes tag should point to the commit which introduced
the wrong behaviour, it may be the initial commit of the driver if the
behaviour was always there.
Could you add more information to the patch description and perhaps
update Fixes tag if 7e538372694b didn't introduce the problem?
Re: [PATCH net,v2 0/2] Update MT7629 to support PHYLINK API
On Mon, 7 Oct 2019 15:08:42 +0800, MarkLee wrote: > This patch target to update mt7629 eth driver and dts to support PHYLINK Thanks for the patches Mark. The description of the set should probably say that it _fixes_ some issues. Right now it sounds a little bit like you were adding a new feature. Could you rewrite the cover letter to give us a better idea what issues this patch set is fixing and why those issues occur?
Re: [GIT PULL] LED fixes for 5.4-rc3.
The pull request you sent on Tue, 8 Oct 2019 22:42:58 +0200: > git://git.kernel.org/pub/scm/linux/kernel/git/j.anaszewski/linux-leds.git > tags/led-fixes-for-5.4-rc3 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/e3280b54afed870d531571212f1fc375df39b7d2 Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
Re: [PATCH] lib/list-test: add a test for the 'list' doubly linked list
On Tue, Oct 8, 2019 at 11:15 AM Kees Cook wrote:
>
> On Tue, Oct 08, 2019 at 10:48:37AM -0700, Brendan Higgins wrote:
> > On Mon, Oct 07, 2019 at 02:36:33PM -0700, David Gow wrote:
> > > This change adds a KUnit test for the kernel doubly linked list
> > > implementation in include/linux/list.h
> > >
> > > Note that, at present, it only tests the list_ types (not the
> > > singly-linked hlist_), and does not yet test all of the
> > > list_for_each_entry* macros (and some related things like
> > > list_prepare_entry).
> > >
> > > This change depends on KUnit, so should be merged via the 'test' branch:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/log/?h=test
> > >
> > > Signed-off-by: David Gow
> > > ---
> > > lib/Kconfig.debug | 12 +
> > > lib/Makefile | 3 +
> > > lib/list-test.c | 711 ++
> > > 3 files changed, 726 insertions(+)
> > > create mode 100644 lib/list-test.c
> >
> > Also, I think it might be good to make a MAINTAINERs entry for this
> > test.
>
> Another thought, though maybe this is already covered and I missed the
> "best practices" notes on naming conventions.
>
> As the "one-off" tests are already named "foo_test.c" it seems like
> KUnit tests should be named distinctly. Should this be lib/kunit-list.c,
> lib/list-kunit.c, or something else?
So we already had a discussion on this here:
https://patchwork.kernel.org/patch/10925861/
(Sounds like I should have probably documented that :-))
However, I am sympathetic to your argument. I was thinking that it
might be good to make get_maintainer suggest CC'ing kunit-dev@ and
linux-kselftest@ for all new tests and this would be hard with the
*-test.c naming scheme.
If we are going to change it, now is probably the time to do it :-)
> For internal naming of structs and tests, should things be
> named "kunit_foo"? Examples here would be kunit_list_struct and
> kunit_list_test_...
I had generally been following the pattern:
foo.c
struct foo_bar {};
int foo_baz() {}
foo-test.c
struct foo_test_buzz {};
void foo_test_does_foo_baz_buzz(struct kunit *test) {}
However, now that you point that out I am realizing there is a bunch
of stuff here that is not consistent with that (whoops, sorry for not
catching that earlier, David).
Nevertheless, I think the list_test_struct is fine and conforms to the pattern.
> When testing other stuff, should only exposed interfaces be tested?
> Many things have their API exposed via registration of a static structure
> of function pointers to static functions. What's the proposed best way
> to get at that? Should the KUnit tests is IN the .c file that declares
> all the static functions?
Yeah, that is a good point, but I don't think entirely relevant to
this code review.
Fundamentally it boils down to figuring out what your API is, and
coming up with a way to expose it. For drivers, that means finding a
way to give KUnit access to the generated driver objects, in some
cases it means using more dependency injection, in other cases it may
mean making something that is static, not static.
I know those answers sound pretty unsatisfying, and I have some
features planned which alleviate some of those issues, but I think the
most important thing is making examples of how to deal with some of
the broad cases, getting agreement on them, documenting them, finding
exceptions and iterating.
Nevertheless, if you want to start enumerating cases and proposed
solutions, I would be more than happy to have that conversation now,
but we might want to fork the discussion.
Cheers!
Re: [PATCH] soc: qcom: llcc: Name regmaps to avoid collisions
Quoting Evan Green (2019-10-07 14:20:47)
> On Fri, Oct 4, 2019 at 4:31 PM Stephen Boyd wrote:
> >
> > We'll end up with debugfs collisions if we don't give names to the
> > regmaps created inside this driver. Copy the template config over into
> > this function and give the regmap the same name as the resource name.
> >
> > Fixes: 7f9c136216c7 ("soc: qcom: Add broadcast base for Last Level Cache
> > Controller (LLCC)")
> > Cc: Venkata Narendra Kumar Gutta
> > Cc: Evan Green
> > Signed-off-by: Stephen Boyd
> > ---
> > drivers/soc/qcom/llcc-slice.c | 14 +++---
> > 1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/soc/qcom/llcc-slice.c b/drivers/soc/qcom/llcc-slice.c
> > index 9090ea12eaf3..aa342938c403 100644
> > --- a/drivers/soc/qcom/llcc-slice.c
> > +++ b/drivers/soc/qcom/llcc-slice.c
> > @@ -48,13 +48,6 @@
> >
> > static struct llcc_drv_data *drv_data = (void *) -EPROBE_DEFER;
> >
> > -static const struct regmap_config llcc_regmap_config = {
> > - .reg_bits = 32,
> > - .reg_stride = 4,
> > - .val_bits = 32,
> > - .fast_io = true,
> > -};
> > -
> > /**
> > * llcc_slice_getd - get llcc slice descriptor
> > * @uid: usecase_id for the client
> > @@ -314,6 +307,12 @@ static struct regmap *qcom_llcc_init_mmio(struct
> > platform_device *pdev,
> > {
> > struct resource *res;
> > void __iomem *base;
> > + static struct regmap_config llcc_regmap_config = {
> > + .reg_bits = 32,
> > + .reg_stride = 4,
> > + .val_bits = 32,
> > + .fast_io = true,
> > + };
>
> Why did you move this to be a static local? I think it works, but it
> makes it look like this is a local variable that's possibly used out
> of scope. Maybe leave it as a global?
And have a followup patch to move it to a static local? Sounds ok to me
if you prefer.
Re: [PATCH v3 1/2] PCI: PCIe: ASPM: Introduce pcie_aspm_enabled()
On Tue, Oct 8, 2019 at 11:16 PM Bjorn Helgaas wrote:
>
> On Tue, Oct 08, 2019 at 11:27:51AM +0200, Rafael J. Wysocki wrote:
> > On Tue, Oct 8, 2019 at 12:34 AM Bjorn Helgaas wrote:
> > > On Thu, Aug 08, 2019 at 11:55:07PM +0200, Rafael J. Wysocki wrote:
> > > > From: Rafael J. Wysocki
> > > >
> > > > Add a function checking whether or not PCIe ASPM has been enabled for
> > > > a given device.
> > > >
> > > > It will be used by the NVMe driver to decide how to handle the
> > > > device during system suspend.
> > > >
> > > > Signed-off-by: Rafael J. Wysocki
> > > > ---
> > > >
> > > > v2 -> v3:
> > > > * Make the new function return bool.
> > > > * Change its name back to pcie_aspm_enabled().
> > > > * Fix kerneldoc comment formatting.
> > > >
> > > > -> v2:
> > > > * Move the PCI/PCIe ASPM changes to a separate patch.
> > > > * Add the _mask suffix to the new function name.
> > > > * Add EXPORT_SYMBOL_GPL() to the new function.
> > > > * Avoid adding an unnecessary blank line.
> > > >
> > > > ---
> > > > drivers/pci/pcie/aspm.c | 20
> > > > include/linux/pci.h |3 +++
> > > > 2 files changed, 23 insertions(+)
> > > >
> > > > Index: linux-pm/drivers/pci/pcie/aspm.c
> > > > ===
> > > > --- linux-pm.orig/drivers/pci/pcie/aspm.c
> > > > +++ linux-pm/drivers/pci/pcie/aspm.c
> > > > @@ -1170,6 +1170,26 @@ static int pcie_aspm_get_policy(char *bu
> > > > module_param_call(policy, pcie_aspm_set_policy, pcie_aspm_get_policy,
> > > > NULL, 0644);
> > > >
> > > > +/**
> > > > + * pcie_aspm_enabled - Check if PCIe ASPM has been enabled for a
> > > > device.
> > > > + * @pci_device: Target device.
> > > > + */
> > > > +bool pcie_aspm_enabled(struct pci_dev *pci_device)
> > > > +{
> > > > + struct pci_dev *bridge = pci_upstream_bridge(pci_device);
> > > > + bool ret;
> > > > +
> > > > + if (!bridge)
> > > > + return false;
> > > > +
> > > > + mutex_lock(&aspm_lock);
> > > > + ret = bridge->link_state ? !!bridge->link_state->aspm_enabled :
> > > > false;
> > > > + mutex_unlock(&aspm_lock);
> > >
> > > Why do we need to acquire aspm_lock here? We aren't modifying
> > > anything, and I don't think we're preventing a race. If this races
> > > with another thread that changes aspm_enabled, we'll return either the
> > > old state or the new one, and I think that's still the case even if we
> > > don't acquire aspm_lock.
> >
> > Well, if we can guarantee that pci_remove_bus_device() will never be
> > called in parallel with this helper, then I agree, but can we
> > guarantee that?
>
> Hmm, yeah, I guess that's the question. It's not a race with another
> thread changing aspm_enabled; the potential race is with another
> thread removing the last child of "bridge", which will free the
> link_state and set bridge->link_state = NULL.
>
> I think it should be safe to call device-related PCI interfaces if
> you're holding a reference to the device, e.g., from a driver bound to
> the device or a sysfs accessor. Since we call pcie_aspm_enabled(dev)
> from a driver bound to "dev", another thread should not be able to
> remove "dev" while we're using it.
>
> I know that's a little hand-wavey, but if it weren't true, I think
> we'd have a lot more locking sprinkled everywhere in the PCI core than
> we do.
>
> This has implications for Heiner's ASPM sysfs patches because we're
> currently doing this in sysfs accessors:
>
> static ssize_t aspm_attr_show_common(struct device *dev, ...)
> {
> ...
> link = pcie_aspm_get_link(pdev);
>
> mutex_lock(&aspm_lock);
> enabled = link->aspm_enabled & state;
> mutex_unlock(&aspm_lock);
> ...
> }
>
> I assume sysfs must be holding a reference that guarantees "dev" is
> valid througout this code, and therefore we should not need to hold
> aspm_lock.
In principle, pcie_aspm_enabled() need not be called via sysfs.
In the particular NVMe use case, it is called from the driver's own PM
callback, so it would be safe without the locking AFAICS.
I guess it is safe to drop the locking from there, but then it would
be good to mention in the kerneldoc that calling it is only safe under
the assumption that the link_state object cannot go away while it is
running.
Re: [PATCH v3 1/2] tpm: Use GFP_KERNEL for allocating struct tpm_buf
On Mon, Oct 07, 2019 at 12:12:08PM -0700, James Bottomley wrote: > From: James Bottomley > Subject: [PATCH] tpm: use GFP kernel for tpm_buf allocations > > The current code uses GFP_HIGHMEM, which is wrong because GFP_HIGHMEM > (on 32 bit systems) is memory ordinarily inaccessible to the kernel > and should only be used for allocations affecting userspace. In order > to make highmem visible to the kernel on 32 bit it has to be kmapped, > which consumes valuable entries in the kmap region. Since the tpm_buf > is only ever used in the kernel, switch to using a GFP_KERNEL > allocation so as not to waste kmap space on 32 bits. > > Fixes: a74f8b36352e (tpm: introduce tpm_buf) > Signed-off-by: James Bottomley Thanks a lot. Makes a lot more sense than the patch that I sent. Reviewed-by: Jarkko Sakkinen /Jarkko
Re: [PATCH] sunrpc: fix crash when cache_head become valid before update
On Tue, Oct 08 2019, J . Bruce Fields wrote: > On Tue, Oct 08, 2019 at 10:02:53AM +, Pavel Tikhomirov wrote: >> Add Neil to CC, sorry, had lost it somehow... > > Always happy when we can fix a bug by deleting code, and your > explanation makes sense to me, but I'll give Neil a chance to look it > over if he wants. Yes, it makes sense to me. But I'm not sure that is worth much. The original fix got a Reviewed-by from me but was wrong. Acked-by: NeilBrown 'Acked' is weaker than 'reviewed' - isn't it? :-) NeilBrown signature.asc Description: PGP signature
Re: [PATCH 1/2] firmware: coreboot: Export the binary FMAP
Quoting patrick.rudo...@9elements.com (2019-10-08 04:53:25)
> From: Patrick Rudolph
>
> Expose coreboot's binary FMAP[1] to /sys/firmware/fmap.
>
> coreboot copies the FMAP to a CBMEM buffer at boot since CB:35377[2],
> allowing an architecture independ way of exposing the FMAP to userspace.
>
> Will be used by fwupd[3] to determine the current firmware layout.
>
> [1]: https://doc.coreboot.org/lib/flashmap.html
> [2]: https://review.coreboot.org/c/coreboot/+/35377
> [3]: https://fwupd.org/
>
> Signed-off-by: Patrick Rudolph
> ---
> drivers/firmware/google/Kconfig | 8 ++
> drivers/firmware/google/Makefile | 1 +
> drivers/firmware/google/fmap-coreboot.c | 156 ++
> drivers/firmware/google/fmap-coreboot.h | 13 ++
> drivers/firmware/google/fmap_serialized.h | 59
> 5 files changed, 237 insertions(+)
> create mode 100644 drivers/firmware/google/fmap-coreboot.c
> create mode 100644 drivers/firmware/google/fmap-coreboot.h
> create mode 100644 drivers/firmware/google/fmap_serialized.h
>
> diff --git a/drivers/firmware/google/Kconfig b/drivers/firmware/google/Kconfig
> index a3a6ca659ffa..5fbbd7b8fef6 100644
> --- a/drivers/firmware/google/Kconfig
> +++ b/drivers/firmware/google/Kconfig
> @@ -74,4 +74,12 @@ config GOOGLE_VPD
> This option enables the kernel to expose the content of Google VPD
> under /sys/firmware/vpd.
>
> +config GOOGLE_FMAP
> + tristate "Coreboot FMAP access"
> + depends on GOOGLE_COREBOOT_TABLE
> + help
> + This option enables the kernel to search for a Google FMAP in
> + the coreboot table. If found, this binary file is exported to
> userland
> + in the file /sys/firmware/fmap.
> +
> endif # GOOGLE_FIRMWARE
> diff --git a/drivers/firmware/google/Makefile
> b/drivers/firmware/google/Makefile
> index d17caded5d88..6d31fe167700 100644
> --- a/drivers/firmware/google/Makefile
> +++ b/drivers/firmware/google/Makefile
> @@ -6,6 +6,7 @@ obj-$(CONFIG_GOOGLE_FRAMEBUFFER_COREBOOT) +=
> framebuffer-coreboot.o
> obj-$(CONFIG_GOOGLE_MEMCONSOLE)+= memconsole.o
> obj-$(CONFIG_GOOGLE_MEMCONSOLE_COREBOOT) += memconsole-coreboot.o
> obj-$(CONFIG_GOOGLE_MEMCONSOLE_X86_LEGACY) += memconsole-x86-legacy.o
> +obj-$(CONFIG_GOOGLE_FMAP) += fmap-coreboot.o
>
> vpd-sysfs-y := vpd.o vpd_decode.o
> obj-$(CONFIG_GOOGLE_VPD) += vpd-sysfs.o
> diff --git a/drivers/firmware/google/fmap-coreboot.c
> b/drivers/firmware/google/fmap-coreboot.c
> new file mode 100644
> index ..14050030ebc6
> --- /dev/null
> +++ b/drivers/firmware/google/fmap-coreboot.c
> @@ -0,0 +1,156 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * fmap-coreboot.c
> + *
> + * Exports the binary FMAP through coreboot table.
> + *
> + * Copyright 2012-2013 David Herrmann
> + * Copyright 2017 Google Inc.
> + * Copyright 2019 9elements Agency GmbH
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "coreboot_table.h"
> +#include "fmap_serialized.h"
> +
> +#define CB_TAG_FMAP 0x37
> +
> +static void *fmap;
> +static u32 fmap_size;
> +
> +/*
> + * Convert FMAP region to name.
> + * The caller has to free the string.
> + * Return NULL if no containing region was found.
> + */
> +const char *coreboot_fmap_region_to_name(const u32 start, const u32 size)
> +{
> + const char *name = NULL;
> + struct fmap *iter;
> + u32 size_old = ~0;
> + int i;
> +
> + iter = fmap;
> + /* Find smallest containing region */
> + for (i = 0; i < iter->nareas && fmap; i++) {
> + if (iter->areas[i].offset <= start &&
> + iter->areas[i].size >= size &&
> + iter->areas[i].size <= size_old) {
> + size_old = iter->areas[i].size;
> + name = iter->areas[i].name;
> + }
> + }
> +
> + if (name)
> + return kstrdup(name, GFP_KERNEL);
> + return NULL;
> +}
> +EXPORT_SYMBOL(coreboot_fmap_region_to_name);
> +
> +static ssize_t fmap_read(struct file *filp, struct kobject *kobp,
> +struct bin_attribute *bin_attr, char *buf,
> +loff_t pos, size_t count)
> +{
> + if (!fmap)
> + return -ENODEV;
> +
> + return memory_read_from_buffer(buf, count, &pos, fmap, fmap_size);
> +}
> +
> +static int fmap_probe(struct coreboot_device *dev)
> +{
> + struct lb_cbmem_ref *cbmem_ref = &dev->cbmem_ref;
> + struct fmap *header;
> +
> + if (!cbmem_ref)
> + return -ENODEV;
This is impossible.
> +
> + header = memremap(cbmem_ref->cbmem_addr, sizeof(*header),
> MEMREMAP_WB);
> + if (!header) {
> + pr_warn("coreboot: Failed to remap FMAP\n");
> + return -ENOMEM;
> + }
> +
> + /* Validate FMAP signature */
> + if
linux-next: manual merge of the bpf-next tree with the bpf tree
Hi all,
Today's linux-next merge of the bpf-next tree got a conflict in:
tools/lib/bpf/Makefile
between commit:
1bd63524593b ("libbpf: handle symbol versioning properly for libbpf.a")
from the bpf tree and commit:
e01a75c15969 ("libbpf: Move bpf_{helpers, helper_defs, endian, tracing}.h
into libbpf")
from the bpf-next tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc tools/lib/bpf/Makefile
index 56ce6292071b,1270955e4845..
--- a/tools/lib/bpf/Makefile
+++ b/tools/lib/bpf/Makefile
@@@ -143,7 -133,9 +143,9 @@@ LIB_TARGET := $(addprefix $(OUTPUT),$(L
LIB_FILE := $(addprefix $(OUTPUT),$(LIB_FILE))
PC_FILE := $(addprefix $(OUTPUT),$(PC_FILE))
+ TAGS_PROG := $(if $(shell which etags 2>/dev/null),etags,ctags)
+
-GLOBAL_SYM_COUNT = $(shell readelf -s --wide $(BPF_IN) | \
+GLOBAL_SYM_COUNT = $(shell readelf -s --wide $(BPF_IN_SHARED) | \
cut -d "@" -f1 | sed 's/_v[0-9]_[0-9]_[0-9].*//' | \
awk '/GLOBAL/ && /DEFAULT/ && !/UND/ {print $$8}' | \
sort -u | wc -l)
@@@ -165,7 -157,7 +167,7 @@@ all: fixde
all_cmd: $(CMD_TARGETS) check
- $(BPF_IN_SHARED): force elfdep bpfdep
-$(BPF_IN): force elfdep bpfdep bpf_helper_defs.h
++$(BPF_IN_SHARED): force elfdep bpfdep bpf_helper_defs.h
@(test -f ../../include/uapi/linux/bpf.h -a -f
../../../include/uapi/linux/bpf.h && ( \
(diff -B ../../include/uapi/linux/bpf.h
../../../include/uapi/linux/bpf.h >/dev/null) || \
echo "Warning: Kernel ABI header at 'tools/include/uapi/linux/bpf.h'
differs from latest version at 'include/uapi/linux/bpf.h'" >&2 )) || true
@@@ -181,14 -173,15 +183,18 @@@
@(test -f ../../include/uapi/linux/if_xdp.h -a -f
../../../include/uapi/linux/if_xdp.h && ( \
(diff -B ../../include/uapi/linux/if_xdp.h
../../../include/uapi/linux/if_xdp.h >/dev/null) || \
echo "Warning: Kernel ABI header at 'tools/include/uapi/linux/if_xdp.h'
differs from latest version at 'include/uapi/linux/if_xdp.h'" >&2 )) || true
- $(Q)$(MAKE) $(build)=libbpf
+ $(Q)$(MAKE) $(build)=libbpf OUTPUT=$(SHARED_OBJDIR) CFLAGS="$(CFLAGS)
$(SHLIB_FLAGS)"
+
+$(BPF_IN_STATIC): force elfdep bpfdep
+ $(Q)$(MAKE) $(build)=libbpf OUTPUT=$(STATIC_OBJDIR)
+ bpf_helper_defs.h: $(srctree)/include/uapi/linux/bpf.h
+ $(Q)$(srctree)/scripts/bpf_helpers_doc.py --header \
+ --file $(srctree)/include/uapi/linux/bpf.h > bpf_helper_defs.h
+
$(OUTPUT)libbpf.so: $(OUTPUT)libbpf.so.$(LIBBPF_VERSION)
-$(OUTPUT)libbpf.so.$(LIBBPF_VERSION): $(BPF_IN)
+$(OUTPUT)libbpf.so.$(LIBBPF_VERSION): $(BPF_IN_SHARED)
$(QUIET_LINK)$(CC) --shared
-Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
-Wl,--version-script=$(VERSION_SCRIPT) $^
-lelf -o $@
@ln -sf $(@F) $(OUTPUT)libbpf.so
@@@ -268,9 -266,9 +279,10 @@@ config-clean
$(Q)$(MAKE) -C $(srctree)/tools/build/feature/ clean >/dev/null
clean:
- $(call QUIET_CLEAN, libbpf) $(RM) $(TARGETS) $(CXX_TEST_TARGET) \
+ $(call QUIET_CLEAN, libbpf) $(RM) -rf $(TARGETS) $(CXX_TEST_TARGET) \
*.o *~ *.a *.so *.so.$(LIBBPF_MAJOR_VERSION) .*.d .*.cmd \
- *.pc LIBBPF-CFLAGS $(SHARED_OBJDIR) $(STATIC_OBJDIR)
- *.pc LIBBPF-CFLAGS bpf_helper_defs.h
++ *.pc LIBBPF-CFLAGS $(SHARED_OBJDIR) $(STATIC_OBJDIR) \
++ bpf_helper_defs.h
$(call QUIET_CLEAN, core-gen) $(RM) $(OUTPUT)FEATURE-DUMP.libbpf
pgp5edpwKrpEL.pgp
Description: OpenPGP digital signature
Re: [PATCH 2/2] firmware: coreboot: Export active CBFS partition
Quoting patrick.rudo...@9elements.com (2019-10-08 04:53:26)
> From: Patrick Rudolph
>
> Expose the name of the active CBFS partition under
> /sys/firmware/cbfs_active_partition
Somehow we've gotten /sys/firmware/log to be the coreboot log, and quite
frankly that blows my mind that this path was accepted upstream.
Userspace has to know it's running on coreboot firmware to know that
/sys/firmware/log is actually the coreboot log. But it is what it is so
we're not going to change it.
Why don't we expose fmap "areas" under some coreboot fmap class that has
sysfs nodes for the properties of that fmap_area? Basically
/sys/class/coreboot_fmap/section0/version
/sys/class/coreboot_fmap/section0//{size,offset,flags}
/sys/class/coreboot_fmap/section1//{size,offset,flags}
I made 'section' an incrementing number because I'm not sure that the
fmap header, struct fmap::name, is unique for all fmaps. Is it? If it is
unique then we can have
/sys/class/coreboot_fmap///{size,flags}
And we may want to make the fmap area a class too, so it would be
/sys/class/coreboot_fmap//coreboot_fmap_area//{size,offset,flags}
But I also wonder why this is being exposed by the kernel at all? Why
can't userspace read the fmap table from the flash chip itself and then
parse it to understand how to update the firmware on the chip? Isn't
that better than relying on coreboot to populate something for us in
memory that describes the flash chip so we can seek around it? I guess
this is faster this way because reading flash chips may be slow?
>
> In case of Google's VBOOT[1] that currently can be one of:
> "FW_MAIN_A", "FW_MAIN_B" or "COREBOOT"
>
> Will be used by fwupd[2] to determinde the active partition in the
s/determinde/determine/ ?
> VBOOT A/B partition update scheme.
>
> [1]: https://doc.coreboot.org/security/vboot/index.html
> [2]: https://fwupd.org/
Can you link to the code in fwupd that is using this stuff from the
kernel?
>
> Signed-off-by: Patrick Rudolph
> ---
> drivers/firmware/google/Kconfig | 10 ++
> drivers/firmware/google/Makefile | 1 +
> drivers/firmware/google/bootmedia-coreboot.c | 115 +++
> drivers/firmware/google/coreboot_table.h | 13 +++
> 4 files changed, 139 insertions(+)
> create mode 100644 drivers/firmware/google/bootmedia-coreboot.c
>
> diff --git a/drivers/firmware/google/Kconfig b/drivers/firmware/google/Kconfig
> index 5fbbd7b8fef6..f58b723d77de 100644
> --- a/drivers/firmware/google/Kconfig
> +++ b/drivers/firmware/google/Kconfig
> @@ -82,4 +82,14 @@ config GOOGLE_FMAP
> the coreboot table. If found, this binary file is exported to
> userland
> in the file /sys/firmware/fmap.
>
> +config GOOGLE_BOOTMEDIA
Please try to add this Kconfig somewhere alphabetically. Otherwise
people keep adding to the end of the list and it becomes sort of hard to
manage merges.
> + tristate "Coreboot bootmedia params access"
> + depends on GOOGLE_COREBOOT_TABLE
> + depends on GOOGLE_FMAP
> + help
> + This option enables the kernel to search for a boot media params
> + table, providing the active CBFS partition. If found, the name of
> + the partition is exported to userland in the file
> + /sys/firmware/cbfs_active_partition.
> +
> endif # GOOGLE_FIRMWARE
> diff --git a/drivers/firmware/google/Makefile
> b/drivers/firmware/google/Makefile
> index 6d31fe167700..e47c59376566 100644
> --- a/drivers/firmware/google/Makefile
> +++ b/drivers/firmware/google/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_GOOGLE_MEMCONSOLE)+= memconsole.o
> obj-$(CONFIG_GOOGLE_MEMCONSOLE_COREBOOT) += memconsole-coreboot.o
> obj-$(CONFIG_GOOGLE_MEMCONSOLE_X86_LEGACY) += memconsole-x86-legacy.o
> obj-$(CONFIG_GOOGLE_FMAP) += fmap-coreboot.o
> +obj-$(CONFIG_GOOGLE_BOOTMEDIA) += bootmedia-coreboot.o
Same here, although it looks like FMAP would need to move in the
previous patch to be sorted by Kconfig name.
>
> vpd-sysfs-y := vpd.o vpd_decode.o
> obj-$(CONFIG_GOOGLE_VPD) += vpd-sysfs.o
> diff --git a/drivers/firmware/google/bootmedia-coreboot.c
> b/drivers/firmware/google/bootmedia-coreboot.c
> new file mode 100644
> index ..09c0caedaf05
> --- /dev/null
> +++ b/drivers/firmware/google/bootmedia-coreboot.c
> @@ -0,0 +1,115 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * bootmedia-coreboot.c
> + *
> + * Exports the active VBOOT partition name through boot media params.
> + *
> + * Copyright 2012-2013 David Herrmann
> + * Copyright 2017 Google Inc.
> + * Copyright 2019 Patrick Rudolph
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "coreboot_table.h"
> +#include "fmap-coreboot.h"
> +
> +#define CB_TAG_BOOT_MEDIA_PARAMS 0x30
> +
> +/* The current CBFS partition */
> +static char *name;
Can this be passed through some
Re: [PATCH v7 10/21] RISC-V: KVM: Handle MMIO exits for VCPU
On Mon, 23 Sep 2019 04:12:17 PDT (-0700), pbonz...@redhat.com wrote:
On 04/09/19 18:15, Anup Patel wrote:
+ unsigned long guest_sstatus =
+ vcpu->arch.guest_context.sstatus | SR_MXR;
+ unsigned long guest_hstatus =
+ vcpu->arch.guest_context.hstatus | HSTATUS_SPRV;
+ unsigned long guest_vsstatus, old_stvec, tmp;
+
+ guest_sstatus = csr_swap(CSR_SSTATUS, guest_sstatus);
+ old_stvec = csr_swap(CSR_STVEC, (ulong)&__kvm_riscv_unpriv_trap);
+
+ if (read_insn) {
+ guest_vsstatus = csr_read_set(CSR_VSSTATUS, SR_MXR);
Is this needed? IIUC SSTATUS.MXR encompasses a wider set of permissions:
The HS-level MXR bit makes any executable page readable. {\tt
vsstatus}.MXR makes readable those pages marked executable at the VS
translation level, but only if readable at the guest-physical
translation level.
So it should be enough to set SSTATUS.MXR=1 I think. But you also
shouldn't set SSTATUS.MXR=1 in the !read_insn case.
Also, you can drop the irq save/restore (which is already a save/restore
of SSTATUS) since you already write 0 to SSTATUS.SIE in your csr_swap.
Perhaps add a BUG_ON(guest_sstatus & SR_SIE) before the csr_swap?
+ asm volatile ("\n"
+ "csrrw %[hstatus], " STR(CSR_HSTATUS) ", %[hstatus]\n"
+ "li %[tilen], 4\n"
+ "li %[tscause], 0\n"
+ "lhu %[val], (%[addr])\n"
+ "andi %[tmp], %[val], 3\n"
+ "addi %[tmp], %[tmp], -3\n"
+ "bne %[tmp], zero, 2f\n"
+ "lhu %[tmp], 2(%[addr])\n"
+ "sll %[tmp], %[tmp], 16\n"
+ "add %[val], %[val], %[tmp]\n"
+ "2: csrw " STR(CSR_HSTATUS) ", %[hstatus]"
+ : [hstatus] "+&r"(guest_hstatus), [val] "=&r" (val),
+ [tmp] "=&r" (tmp), [tilen] "+&r" (tilen),
+ [tscause] "+&r" (tscause)
+ : [addr] "r" (addr));
+ csr_write(CSR_VSSTATUS, guest_vsstatus);
+#ifndef CONFIG_RISCV_ISA_C
+ "li %[tilen], 4\n"
+#else
+ "li %[tilen], 2\n"
+#endif
Can you use an assembler directive to force using a non-compressed
format for ld and lw? This would get rid of tilen, which is costing 6
bytes (if I did the RVC math right) in order to save two. :)
Paolo
+ "li %[tscause], 0\n"
+#ifdef CONFIG_64BIT
+ "ld %[val], (%[addr])\n"
+#else
+ "lw %[val], (%[addr])\n"
+#endif
To: a...@brainfault.org
CC: pbonz...@redhat.com
CC: Anup Patel
CC: Paul Walmsley
CC: rkrc...@redhat.com
CC: daniel.lezc...@linaro.org
CC: t...@linutronix.de
CC: g...@amazon.com
CC: Atish Patra
CC: Alistair Francis
CC: Damien Le Moal
CC: Christoph Hellwig
CC: k...@vger.kernel.org
CC: linux-ri...@lists.infradead.org
CC: linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7 10/21] RISC-V: KVM: Handle MMIO exits for VCPU
In-Reply-To:
On Mon, 23 Sep 2019 06:09:43 PDT (-0700), a...@brainfault.org wrote:
On Mon, Sep 23, 2019 at 4:42 PM Paolo Bonzini wrote:
On 04/09/19 18:15, Anup Patel wrote:
> + unsigned long guest_sstatus =
> + vcpu->arch.guest_context.sstatus | SR_MXR;
> + unsigned long guest_hstatus =
> + vcpu->arch.guest_context.hstatus | HSTATUS_SPRV;
> + unsigned long guest_vsstatus, old_stvec, tmp;
> +
> + guest_sstatus = csr_swap(CSR_SSTATUS, guest_sstatus);
> + old_stvec = csr_swap(CSR_STVEC, (ulong)&__kvm_riscv_unpriv_trap);
> +
> + if (read_insn) {
> + guest_vsstatus = csr_read_set(CSR_VSSTATUS, SR_MXR);
Is this needed? IIUC SSTATUS.MXR encompasses a wider set of permissions:
The HS-level MXR bit makes any executable page readable. {\tt
vsstatus}.MXR makes readable those pages marked executable at the VS
translation level, but only if readable at the guest-physical
translation level.
So it should be enough to set SSTATUS.MXR=1 I think. But you also
shouldn't set SSTATUS.MXR=1 in the !read_insn case.
I was being overly cautious here. Initially, I thought SSTATUS.MXR
applies only to Stage2 and VSSTATUS.MXR applies only to Stage1.
I agree with you. The HS-mode should only need to set SSTATUS.MXR.
Also, you can drop the irq save/restore (which is already a save/restore
of SSTATUS) since you already write 0 to SSTATUS.SIE in your csr_swap.
Perhaps add a BUG_ON(guest_sstatus & SR_SIE) before the csr_swap?
I had already dropped irq save/restore in v7 series and having BUG_ON()
on guest_sstatus here would be better.
> + asm volatile ("\n"
> + "csrrw %[hstatus], " STR(CSR_HSTATUS) ", %[hstatus]\n"
> +
Re: [PATCH v4 05/10] mm: Return faster for non-fatal signals in user mode faults
On Sun, 22 Sep 2019 21:25:18 PDT (-0700), pet...@redhat.com wrote:
The idea comes from the upstream discussion between Linus and Andrea:
https://lore.kernel.org/lkml/20171102193644.gb22...@redhat.com/
A summary to the issue: there was a special path in handle_userfault()
in the past that we'll return a VM_FAULT_NOPAGE when we detected
non-fatal signals when waiting for userfault handling. We did that by
reacquiring the mmap_sem before returning. However that brings a risk
in that the vmas might have changed when we retake the mmap_sem and
even we could be holding an invalid vma structure.
This patch is a preparation of removing that special path by allowing
the page fault to return even faster if we were interrupted by a
non-fatal signal during a user-mode page fault handling routine.
Suggested-by: Linus Torvalds
Suggested-by: Andrea Arcangeli
Signed-off-by: Peter Xu
---
arch/alpha/mm/fault.c| 3 ++-
arch/arc/mm/fault.c | 5 +
arch/arm/mm/fault.c | 9 +
arch/arm64/mm/fault.c| 9 +
arch/hexagon/mm/vm_fault.c | 3 ++-
arch/ia64/mm/fault.c | 3 ++-
arch/m68k/mm/fault.c | 5 +++--
arch/microblaze/mm/fault.c | 3 ++-
arch/mips/mm/fault.c | 3 ++-
arch/nds32/mm/fault.c| 9 +
arch/nios2/mm/fault.c| 3 ++-
arch/openrisc/mm/fault.c | 3 ++-
arch/parisc/mm/fault.c | 3 ++-
arch/powerpc/mm/fault.c | 2 ++
arch/riscv/mm/fault.c| 5 +++--
arch/s390/mm/fault.c | 4 ++--
arch/sh/mm/fault.c | 4
arch/sparc/mm/fault_32.c | 2 +-
arch/sparc/mm/fault_64.c | 3 ++-
arch/um/kernel/trap.c| 4 +++-
arch/unicore32/mm/fault.c| 5 +++--
arch/x86/mm/fault.c | 2 ++
arch/xtensa/mm/fault.c | 3 ++-
include/linux/sched/signal.h | 12
24 files changed, 75 insertions(+), 32 deletions(-)
diff --git a/arch/alpha/mm/fault.c b/arch/alpha/mm/fault.c
index de4cc6936391..ab1d4212d658 100644
--- a/arch/alpha/mm/fault.c
+++ b/arch/alpha/mm/fault.c
@@ -150,7 +150,8 @@ do_page_fault(unsigned long address, unsigned long mmcsr,
the fault. */
fault = handle_mm_fault(vma, address, flags);
- if ((fault & VM_FAULT_RETRY) && fatal_signal_pending(current))
+ if ((fault & VM_FAULT_RETRY) &&
+ fault_should_check_signal(user_mode(regs)))
return;
if (unlikely(fault & VM_FAULT_ERROR)) {
diff --git a/arch/arc/mm/fault.c b/arch/arc/mm/fault.c
index 61919e4e4eec..27adf4e608e4 100644
--- a/arch/arc/mm/fault.c
+++ b/arch/arc/mm/fault.c
@@ -142,6 +142,11 @@ void do_page_fault(unsigned long address, struct pt_regs
*regs)
goto no_context;
return;
}
+
+ /* Allow user to handle non-fatal signals first */
+ if (signal_pending(current) && user_mode(regs))
+ return;
+
/*
* retry state machine
*/
diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index 2ae28ffec622..f00fb4eafe54 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -291,14 +291,15 @@ do_page_fault(unsigned long addr, unsigned int fsr,
struct pt_regs *regs)
fault = __do_page_fault(mm, addr, fsr, flags, tsk);
- /* If we need to retry but a fatal signal is pending, handle the
+ /* If we need to retry but a signal is pending, try to handle the
* signal first. We do not need to release the mmap_sem because
* it would already be released in __lock_page_or_retry in
* mm/filemap.c. */
- if ((fault & VM_FAULT_RETRY) && fatal_signal_pending(current)) {
- if (!user_mode(regs))
+ if (unlikely(fault & VM_FAULT_RETRY && signal_pending(current))) {
+ if (fatal_signal_pending(current) && !user_mode(regs))
goto no_context;
- return 0;
+ if (user_mode(regs))
+ return 0;
}
/*
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 613e7434c208..0d3fe0ea6a70 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -479,15 +479,16 @@ static int __kprobes do_page_fault(unsigned long addr,
unsigned int esr,
if (fault & VM_FAULT_RETRY) {
/*
-* If we need to retry but a fatal signal is pending,
+* If we need to retry but a signal is pending, try to
* handle the signal first. We do not need to release
* the mmap_sem because it would already be released
* in __lock_page_or_retry in mm/filemap.c.
*/
- if (fatal_signal_pending(current)) {
- if (!user_mode(regs))
+ if (signal_pending(current)) {
+ if (fatal_signal_pending(current) && !user_mode(regs))
[RESEND PATCH v2 4/5] x86: use the correct function type for sys_ni_syscall
Use the correct function type for sys_ni_syscall in system
call tables to fix indirect call mismatches with Control-Flow
Integrity (CFI) checking.
Signed-off-by: Sami Tolvanen
---
arch/x86/entry/syscall_32.c| 8 +++-
arch/x86/entry/syscall_64.c| 14 ++
arch/x86/entry/syscalls/syscall_32.tbl | 4 ++--
3 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/arch/x86/entry/syscall_32.c b/arch/x86/entry/syscall_32.c
index aa3336a7cb15..7d17b3addbbb 100644
--- a/arch/x86/entry/syscall_32.c
+++ b/arch/x86/entry/syscall_32.c
@@ -10,13 +10,11 @@
#ifdef CONFIG_IA32_EMULATION
/* On X86_64, we use struct pt_regs * to pass parameters to syscalls */
#define __SYSCALL_I386(nr, sym, qual) extern asmlinkage long sym(const struct
pt_regs *);
-
-/* this is a lie, but it does not hurt as sys_ni_syscall just returns -EINVAL
*/
-extern asmlinkage long sys_ni_syscall(const struct pt_regs *);
-
+#define __sys_ni_syscall __ia32_sys_ni_syscall
#else /* CONFIG_IA32_EMULATION */
#define __SYSCALL_I386(nr, sym, qual) extern asmlinkage long sym(unsigned
long, unsigned long, unsigned long, unsigned long, unsigned long, unsigned
long);
extern asmlinkage long sys_ni_syscall(unsigned long, unsigned long, unsigned
long, unsigned long, unsigned long, unsigned long);
+#define __sys_ni_syscall sys_ni_syscall
#endif /* CONFIG_IA32_EMULATION */
#include
@@ -29,6 +27,6 @@ __visible const sys_call_ptr_t
ia32_sys_call_table[__NR_syscall_compat_max+1] =
* Smells like a compiler bug -- it doesn't work
* when the & below is removed.
*/
- [0 ... __NR_syscall_compat_max] = &sys_ni_syscall,
+ [0 ... __NR_syscall_compat_max] = &__sys_ni_syscall,
#include
};
diff --git a/arch/x86/entry/syscall_64.c b/arch/x86/entry/syscall_64.c
index b1bf31713374..adf619a856e8 100644
--- a/arch/x86/entry/syscall_64.c
+++ b/arch/x86/entry/syscall_64.c
@@ -4,11 +4,17 @@
#include
#include
#include
+#include
#include
#include
-/* this is a lie, but it does not hurt as sys_ni_syscall just returns -EINVAL
*/
-extern asmlinkage long sys_ni_syscall(const struct pt_regs *);
+extern asmlinkage long sys_ni_syscall(void);
+
+SYSCALL_DEFINE0(ni_syscall)
+{
+ return sys_ni_syscall();
+}
+
#define __SYSCALL_64(nr, sym, qual) extern asmlinkage long sym(const struct
pt_regs *);
#define __SYSCALL_X32(nr, sym, qual) __SYSCALL_64(nr, sym, qual)
#include
@@ -23,7 +29,7 @@ asmlinkage const sys_call_ptr_t
sys_call_table[__NR_syscall_max+1] = {
* Smells like a compiler bug -- it doesn't work
* when the & below is removed.
*/
- [0 ... __NR_syscall_max] = &sys_ni_syscall,
+ [0 ... __NR_syscall_max] = &__x64_sys_ni_syscall,
#include
};
@@ -40,7 +46,7 @@ asmlinkage const sys_call_ptr_t
x32_sys_call_table[__NR_syscall_x32_max+1] = {
* Smells like a compiler bug -- it doesn't work
* when the & below is removed.
*/
- [0 ... __NR_syscall_x32_max] = &sys_ni_syscall,
+ [0 ... __NR_syscall_x32_max] = &__x64_sys_ni_syscall,
#include
};
diff --git a/arch/x86/entry/syscalls/syscall_32.tbl
b/arch/x86/entry/syscalls/syscall_32.tbl
index 2de75fda1d20..15908eb9b17e 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -124,7 +124,7 @@
110i386ioplsys_iopl
__ia32_sys_iopl
111i386vhangup sys_vhangup
__ia32_sys_vhangup
112i386idle
-113i386vm86old sys_vm86old
sys_ni_syscall
+113i386vm86old sys_vm86old
__ia32_sys_ni_syscall
114i386wait4 sys_wait4
__ia32_compat_sys_wait4
115i386swapoff sys_swapoff
__ia32_sys_swapoff
116i386sysinfo sys_sysinfo
__ia32_compat_sys_sysinfo
@@ -177,7 +177,7 @@
163i386mremap sys_mremap
__ia32_sys_mremap
164i386setresuid sys_setresuid16
__ia32_sys_setresuid16
165i386getresuid sys_getresuid16
__ia32_sys_getresuid16
-166i386vm86sys_vm86
sys_ni_syscall
+166i386vm86sys_vm86
__ia32_sys_ni_syscall
167i386query_module
168i386pollsys_poll
__ia32_sys_poll
169i386nfsservctl
--
2.23.0.581.g78d2f28ef7-goog
RE: [PATCH 1/2] Drivers: hv: vmbus: Introduce table of VMBus protocol versions
> From: Vitaly Kuznetsov
> Sent: Tuesday, October 8, 2019 6:00 AM
> ...
> > Looking at the uses of VERSION_INVAL, I find one remaining occurrence
> > of this macro in vmbus_bus_resume(), which does:
> >
> > if (vmbus_proto_version == VERSION_INVAL ||
> > vmbus_proto_version == 0) {
> > ...
> > }
> >
> > TBH I'm looking at vmbus_bus_resume() and vmbus_bus_suspend() for the
> > first time and I'm not sure about how to change such check yet... any
> > suggestions?
>
> Hm, I don't think vmbus_proto_version can ever become == VERSION_INVAL
> if we rewrite the code the way you suggest, right? So you'll reduce this
> to 'if (!vmbus_proto_version)' meaning we haven't negotiated any version
> (yet).
Yeah, Vitaly is correct. The check may be a little paranoid as I believe
"vmbus_proto_version" must be a negotiated value in vmbus_bus_resume()
and vmbus_bus_suspend(). I added the check just in case.
> > Mmh, I see that vmbus_bus_resume() and vmbus_bus_suspend() can access
> > vmbus_connection.conn_state: can such accesses race with a concurrent
> > vmbus_connect()?
>
> Let's summon Dexuan who's the author! :-)
There should not be an issue:
vmbus_connect() is called in the early subsys_initcall(hv_acpi_init).
vmbus_bus_suspend() is called late in the PM code after the kernel boots up,
e.g.
in the hibernation function hibernation_snapshot() -> dpm_suspend().
vmbus_bus_resume() is also called later in late_initcall_sync(software_resume).
In the hibernatin process, vmbus_bus_suspend()/resume() can also be called a
few times, and vmbus_bus_resume() calls vmbus_negotiate_version(). As I
checked, there is no issue, either.
Thanks,
Dexuan
[RESEND PATCH v2 0/5] x86: fix syscall function type mismatches
This patch set changes x86 syscall wrappers and related functions to use function types that match sys_call_ptr_t. This fixes indirect call mismatches with Control-Flow Integrity (CFI) checking. Changes since v1: - Use SYSCALL_DEFINE0 for __x64_sys_ni_syscall. - Include Andy's COMPAT_SYSCALL_DEFINE0 patch and use the macro for (rt_)sigreturn. Andy Lutomirski (1): x86/syscalls: Wire up COMPAT_SYSCALL_DEFINE0 Sami Tolvanen (4): x86: use the correct function type in SYSCALL_DEFINE0 x86: use COMPAT_SYSCALL_DEFINE0 for IA32 (rt_)sigreturn x86: use the correct function type for sys_ni_syscall x86: fix function types in COND_SYSCALL arch/x86/entry/syscall_32.c| 8 +-- arch/x86/entry/syscall_64.c| 14 +++-- arch/x86/entry/syscalls/syscall_32.tbl | 8 +-- arch/x86/ia32/ia32_signal.c| 5 +- arch/x86/include/asm/syscall_wrapper.h | 76 -- 5 files changed, 78 insertions(+), 33 deletions(-) -- 2.23.0.581.g78d2f28ef7-goog
[RESEND PATCH v2 2/5] x86/syscalls: Wire up COMPAT_SYSCALL_DEFINE0
From: Andy Lutomirski
x86 has special handling for COMPAT_SYSCALL_DEFINEx, but there was
no override for COMPAT_SYSCALL_DEFINE0. Wire it up so that we can
use it for rt_sigreturn.
Signed-off-by: Andy Lutomirski
Signed-off-by: Sami Tolvanen
---
arch/x86/include/asm/syscall_wrapper.h | 32 --
1 file changed, 30 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/syscall_wrapper.h
b/arch/x86/include/asm/syscall_wrapper.h
index 90eb70df0b18..3dab04841494 100644
--- a/arch/x86/include/asm/syscall_wrapper.h
+++ b/arch/x86/include/asm/syscall_wrapper.h
@@ -28,13 +28,21 @@
* kernel/sys_ni.c and SYS_NI in kernel/time/posix-stubs.c to cover this
* case as well.
*/
+#define __IA32_COMPAT_SYS_STUB0(x, name) \
+ asmlinkage long __ia32_compat_sys_##name(const struct pt_regs *regs);\
+ ALLOW_ERROR_INJECTION(__ia32_compat_sys_##name, ERRNO); \
+ asmlinkage long __ia32_compat_sys_##name(const struct pt_regs *regs)\
+ { \
+ return __se_compat_sys_##name();\
+ }
+
#define __IA32_COMPAT_SYS_STUBx(x, name, ...) \
asmlinkage long __ia32_compat_sys##name(const struct pt_regs *regs);\
ALLOW_ERROR_INJECTION(__ia32_compat_sys##name, ERRNO); \
asmlinkage long __ia32_compat_sys##name(const struct pt_regs *regs)\
{ \
return
__se_compat_sys##name(SC_IA32_REGS_TO_ARGS(x,__VA_ARGS__));\
- } \
+ }
#define __IA32_SYS_STUBx(x, name, ...) \
asmlinkage long __ia32_sys##name(const struct pt_regs *regs); \
@@ -76,15 +84,24 @@
* of the x86-64-style parameter ordering of x32 syscalls. The syscalls common
* with x86_64 obviously do not need such care.
*/
+#define __X32_COMPAT_SYS_STUB0(x, name, ...) \
+ asmlinkage long __x32_compat_sys_##name(const struct pt_regs *regs);\
+ ALLOW_ERROR_INJECTION(__x32_compat_sys_##name, ERRNO); \
+ asmlinkage long __x32_compat_sys_##name(const struct pt_regs *regs)\
+ { \
+ return __se_compat_sys_##name();\
+ }
+
#define __X32_COMPAT_SYS_STUBx(x, name, ...) \
asmlinkage long __x32_compat_sys##name(const struct pt_regs *regs);\
ALLOW_ERROR_INJECTION(__x32_compat_sys##name, ERRNO); \
asmlinkage long __x32_compat_sys##name(const struct pt_regs *regs)\
{ \
return
__se_compat_sys##name(SC_X86_64_REGS_TO_ARGS(x,__VA_ARGS__));\
- } \
+ }
#else /* CONFIG_X86_X32 */
+#define __X32_COMPAT_SYS_STUB0(x, name)
#define __X32_COMPAT_SYS_STUBx(x, name, ...)
#endif /* CONFIG_X86_X32 */
@@ -95,6 +112,17 @@
* mapping of registers to parameters, we need to generate stubs for each
* of them.
*/
+#define COMPAT_SYSCALL_DEFINE0(name) \
+ static long __se_compat_sys_##name(void); \
+ static inline long __do_compat_sys_##name(void);\
+ __IA32_COMPAT_SYS_STUB0(x, name)\
+ __X32_COMPAT_SYS_STUB0(x, name) \
+ static long __se_compat_sys_##name(void)\
+ { \
+ return __do_compat_sys_##name();\
+ } \
+ static inline long __do_compat_sys_##name(void)
+
#define COMPAT_SYSCALL_DEFINEx(x, name, ...)
\
static long __se_compat_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__));
\
static inline long
__do_compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__));\
--
2.23.0.581.g78d2f28ef7-goog
[RESEND PATCH v2 5/5] x86: fix function types in COND_SYSCALL
Define a weak function in COND_SYSCALL instead of a weak alias to
sys_ni_syscall, which has an incompatible type. This fixes indirect
call mismatches with Control-Flow Integrity (CFI) checking.
Signed-off-by: Sami Tolvanen
---
arch/x86/include/asm/syscall_wrapper.h | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/arch/x86/include/asm/syscall_wrapper.h
b/arch/x86/include/asm/syscall_wrapper.h
index 3dab04841494..e2389ce9bf58 100644
--- a/arch/x86/include/asm/syscall_wrapper.h
+++ b/arch/x86/include/asm/syscall_wrapper.h
@@ -6,6 +6,8 @@
#ifndef _ASM_X86_SYSCALL_WRAPPER_H
#define _ASM_X86_SYSCALL_WRAPPER_H
+struct pt_regs;
+
/* Mapping of registers to parameters for syscalls on x86-64 and x32 */
#define SC_X86_64_REGS_TO_ARGS(x, ...) \
__MAP(x,__SC_ARGS \
@@ -64,9 +66,15 @@
SYSCALL_ALIAS(__ia32_sys_##sname, __x64_sys_##sname); \
asmlinkage long __x64_sys_##sname(const struct pt_regs *__unused)
-#define COND_SYSCALL(name) \
- cond_syscall(__x64_sys_##name); \
- cond_syscall(__ia32_sys_##name)
+#define COND_SYSCALL(name)
\
+ asmlinkage __weak long __x64_sys_##name(const struct pt_regs *__unused)
\
+ {
\
+ return sys_ni_syscall();
\
+ }
\
+ asmlinkage __weak long __ia32_sys_##name(const struct pt_regs
*__unused)\
+ {
\
+ return sys_ni_syscall();
\
+ }
#define SYS_NI(name) \
SYSCALL_ALIAS(__x64_sys_##name, sys_ni_posix_timers); \
@@ -218,7 +226,11 @@
#endif
#ifndef COND_SYSCALL
-#define COND_SYSCALL(name) cond_syscall(__x64_sys_##name)
+#define COND_SYSCALL(name)
\
+ asmlinkage __weak long __x64_sys_##name(const struct pt_regs *__unused)
\
+ {
\
+ return sys_ni_syscall();
\
+ }
#endif
#ifndef SYS_NI
@@ -230,7 +242,6 @@
* For VSYSCALLS, we need to declare these three syscalls with the new
* pt_regs-based calling convention for in-kernel use.
*/
-struct pt_regs;
asmlinkage long __x64_sys_getcpu(const struct pt_regs *regs);
asmlinkage long __x64_sys_gettimeofday(const struct pt_regs *regs);
asmlinkage long __x64_sys_time(const struct pt_regs *regs);
--
2.23.0.581.g78d2f28ef7-goog
[RESEND PATCH v2 1/5] x86: use the correct function type in SYSCALL_DEFINE0
Although a syscall defined using SYSCALL_DEFINE0 doesn't accept parameters, use the correct function type to avoid type mismatches with Control-Flow Integrity (CFI) checking. Signed-off-by: Sami Tolvanen Acked-by: Andy Lutomirski --- arch/x86/include/asm/syscall_wrapper.h | 23 --- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/arch/x86/include/asm/syscall_wrapper.h b/arch/x86/include/asm/syscall_wrapper.h index e046a405743d..90eb70df0b18 100644 --- a/arch/x86/include/asm/syscall_wrapper.h +++ b/arch/x86/include/asm/syscall_wrapper.h @@ -48,12 +48,13 @@ * To keep the naming coherent, re-define SYSCALL_DEFINE0 to create an alias * named __ia32_sys_*() */ -#define SYSCALL_DEFINE0(sname) \ - SYSCALL_METADATA(_##sname, 0); \ - asmlinkage long __x64_sys_##sname(void);\ - ALLOW_ERROR_INJECTION(__x64_sys_##sname, ERRNO);\ - SYSCALL_ALIAS(__ia32_sys_##sname, __x64_sys_##sname); \ - asmlinkage long __x64_sys_##sname(void) + +#define SYSCALL_DEFINE0(sname) \ + SYSCALL_METADATA(_##sname, 0); \ + asmlinkage long __x64_sys_##sname(const struct pt_regs *__unused);\ + ALLOW_ERROR_INJECTION(__x64_sys_##sname, ERRNO);\ + SYSCALL_ALIAS(__ia32_sys_##sname, __x64_sys_##sname); \ + asmlinkage long __x64_sys_##sname(const struct pt_regs *__unused) #define COND_SYSCALL(name) \ cond_syscall(__x64_sys_##name); \ @@ -181,11 +182,11 @@ * macros to work correctly. */ #ifndef SYSCALL_DEFINE0 -#define SYSCALL_DEFINE0(sname) \ - SYSCALL_METADATA(_##sname, 0); \ - asmlinkage long __x64_sys_##sname(void);\ - ALLOW_ERROR_INJECTION(__x64_sys_##sname, ERRNO);\ - asmlinkage long __x64_sys_##sname(void) +#define SYSCALL_DEFINE0(sname) \ + SYSCALL_METADATA(_##sname, 0); \ + asmlinkage long __x64_sys_##sname(const struct pt_regs *__unused);\ + ALLOW_ERROR_INJECTION(__x64_sys_##sname, ERRNO);\ + asmlinkage long __x64_sys_##sname(const struct pt_regs *__unused) #endif #ifndef COND_SYSCALL -- 2.23.0.581.g78d2f28ef7-goog
[RESEND PATCH v2 3/5] x86: use COMPAT_SYSCALL_DEFINE0 for IA32 (rt_)sigreturn
Use COMPAT_SYSCALL_DEFINE0 to define (rt_)sigreturn syscalls to
replace sys32_sigreturn and sys32_rt_sigreturn. This fixes indirect
call mismatches with Control-Flow Integrity (CFI) checking.
Signed-off-by: Sami Tolvanen
---
arch/x86/entry/syscalls/syscall_32.tbl | 4 ++--
arch/x86/ia32/ia32_signal.c| 5 +++--
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/x86/entry/syscalls/syscall_32.tbl
b/arch/x86/entry/syscalls/syscall_32.tbl
index 3fe02546aed3..2de75fda1d20 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -130,7 +130,7 @@
116i386sysinfo sys_sysinfo
__ia32_compat_sys_sysinfo
117i386ipc sys_ipc
__ia32_compat_sys_ipc
118i386fsync sys_fsync
__ia32_sys_fsync
-119i386sigreturn sys_sigreturn
sys32_sigreturn
+119i386sigreturn sys_sigreturn
__ia32_compat_sys_sigreturn
120i386clone sys_clone
__ia32_compat_sys_x86_clone
121i386setdomainname sys_setdomainname
__ia32_sys_setdomainname
122i386uname sys_newuname
__ia32_sys_newuname
@@ -184,7 +184,7 @@
170i386setresgid sys_setresgid16
__ia32_sys_setresgid16
171i386getresgid sys_getresgid16
__ia32_sys_getresgid16
172i386prctl sys_prctl
__ia32_sys_prctl
-173i386rt_sigreturnsys_rt_sigreturn
sys32_rt_sigreturn
+173i386rt_sigreturnsys_rt_sigreturn
__ia32_compat_sys_rt_sigreturn
174i386rt_sigactionsys_rt_sigaction
__ia32_compat_sys_rt_sigaction
175i386rt_sigprocmask sys_rt_sigprocmask
__ia32_compat_sys_rt_sigprocmask
176i386rt_sigpending sys_rt_sigpending
__ia32_compat_sys_rt_sigpending
diff --git a/arch/x86/ia32/ia32_signal.c b/arch/x86/ia32/ia32_signal.c
index 1cee10091b9f..30416d7f19d4 100644
--- a/arch/x86/ia32/ia32_signal.c
+++ b/arch/x86/ia32/ia32_signal.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -118,7 +119,7 @@ static int ia32_restore_sigcontext(struct pt_regs *regs,
return err;
}
-asmlinkage long sys32_sigreturn(void)
+COMPAT_SYSCALL_DEFINE0(sigreturn)
{
struct pt_regs *regs = current_pt_regs();
struct sigframe_ia32 __user *frame = (struct sigframe_ia32 __user
*)(regs->sp-8);
@@ -144,7 +145,7 @@ asmlinkage long sys32_sigreturn(void)
return 0;
}
-asmlinkage long sys32_rt_sigreturn(void)
+COMPAT_SYSCALL_DEFINE0(rt_sigreturn)
{
struct pt_regs *regs = current_pt_regs();
struct rt_sigframe_ia32 __user *frame;
--
2.23.0.581.g78d2f28ef7-goog
Re: [PATCH] riscv: Fix memblock reservation for device tree blob
On Fri, 20 Sep 2019 21:34:57 PDT (-0700), a...@brainfault.org wrote:
On Sat, Sep 21, 2019 at 6:30 AM Albert Ou wrote:
This fixes an error with how the FDT blob is reserved in memblock.
An incorrect physical address calculation exposed the FDT header to
unintended corruption, which typically manifested with of_fdt_raw_init()
faulting during late boot after fdt_totalsize() returned a wrong value.
Systems with smaller physical memory sizes more frequently trigger this
issue, as the kernel is more likely to allocate from the DMA32 zone
where bbl places the DTB after the kernel image.
Commit 671f9a3e2e24 ("RISC-V: Setup initial page tables in two stages")
changed the mapping of the DTB to reside in the fixmap area.
Consequently, early_init_fdt_reserve_self() cannot be used anymore in
setup_bootmem() since it relies on __pa() to derive a physical address,
which does not work with dtb_early_va that is no longer a valid kernel
logical address.
The reserved[0x1] region shows the effect of the pointer underflow
resulting from the __pa(initial_boot_params) offset subtraction:
[0.00] MEMBLOCK configuration:
[0.00] memory size = 0x1fe0 reserved size =
0x00a2e514
[0.00] memory.cnt = 0x1
[0.00] memory[0x0] [0x8020-0x9fff],
0x1fe0 bytes flags: 0x0
[0.00] reserved.cnt = 0x2
[0.00] reserved[0x0] [0x8020-0x80c2dfeb],
0x00a2dfec bytes flags: 0x0
[0.00] reserved[0x1] [0xfff08010-0xfff080100527],
0x0528 bytes flags: 0x0
With the fix applied:
[0.00] MEMBLOCK configuration:
[0.00] memory size = 0x1fe0 reserved size =
0x00a2e514
[0.00] memory.cnt = 0x1
[0.00] memory[0x0] [0x8020-0x9fff],
0x1fe0 bytes flags: 0x0
[0.00] reserved.cnt = 0x2
[0.00] reserved[0x0] [0x8020-0x80c2dfeb],
0x00a2dfec bytes flags: 0x0
[0.00] reserved[0x1] [0x80e0-0x80e00527],
0x0528 bytes flags: 0x0
Thanks for catching this issue.
Most of us did not notice this issue most likely because:
1. We generally have good enough RAM on QEMU and SiFive Unleashed
2. Most of people use OpenSBI FW_JUMP on QEMU and U-Boot on
SiFive Unleashed to boot in Linux which places FDT quite far away
from Linux kernel end
Linux ARM64 kernel also uses FIXMAP to access FDT and over there
as well early_init_fdt_reserve_self() is not used.
Fixes: 671f9a3e2e24 ("RISC-V: Setup initial page tables in two stages")
Signed-off-by: Albert Ou
---
arch/riscv/mm/init.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index f0ba713..52d007c 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include
#include
@@ -82,6 +83,8 @@ static void __init setup_initrd(void)
}
#endif /* CONFIG_BLK_DEV_INITRD */
+static phys_addr_t __dtb_pa __initdata;
May be dtb_early_pa will be more consistent name
instead of __dtb_pa because it matches dtb_early_va
used below.
+
void __init setup_bootmem(void)
{
struct memblock_region *reg;
@@ -117,7 +120,12 @@ void __init setup_bootmem(void)
setup_initrd();
#endif /* CONFIG_BLK_DEV_INITRD */
- early_init_fdt_reserve_self();
+ /*
+* Avoid using early_init_fdt_reserve_self() since __pa() does
+* not work for DTB pointers that are fixmap addresses
+*/
+ memblock_reserve(__dtb_pa, fdt_totalsize(dtb_early_va));
+
early_init_fdt_scan_reserved_mem();
memblock_allow_resize();
memblock_dump_all();
@@ -333,6 +341,7 @@ static uintptr_t __init best_map_size(phys_addr_t base,
phys_addr_t size)
"not use absolute addressing."
#endif
+
Please remove this newline addition.
asmlinkage void __init setup_vm(uintptr_t dtb_pa)
{
uintptr_t va, end_va;
@@ -393,6 +402,8 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
/* Save pointer to DTB for early FDT parsing */
dtb_early_va = (void *)fix_to_virt(FIX_FDT) + (dtb_pa & ~PAGE_MASK);
+ /* Save physical address for memblock reservation */
+ __dtb_pa = dtb_pa;
}
static void __init setup_vm_final(void)
--
2.7.4
___
linux-riscv mailing list
linux-ri...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
This deserves to be stable kernel fix as well.
You should add:
Cc: sta...@vger.kernel.org
in your commit description.
Apart from minor nits above.
Reviewed-by: Anup Patel
I tried this patch for both RV64 and RV32 on QEMU with
Yocto rootfs.
Tested-by: Anup Patel
Regards,
Anup
Albert: Do you plan on spinning a v2 of the patch set?
[PATCH] Input: synaptics-rmi4 - Avoid processing unknown IRQs
rmi_process_interrupt_requests() calls handle_nested_irq() for each interrupt status bit it finds. If the irq domain mapping for this bit had not yet been set up, then it ends up calling handle_nested_irq(0), which causes a NULL pointer dereference. There's already code that masks the irq_status bits coming out of the hardware with current_irq_mask, presumably to avoid this situation. However current_irq_mask seems to more reflect the actual mask set in the hardware rather than the IRQs software has set up and registered for. For example, in rmi_driver_reset_handler(), the current_irq_mask is initialized based on what is read from the hardware. If the reset value of this mask enables IRQs that Linux has not set up yet, then we end up in this situation. There appears to be a third unused bitmask that used to serve this purpose, fn_irq_bits. Use that bitmask instead of current_irq_mask to avoid calling handle_nested_irq() on IRQs that have not yet been set up. Signed-off-by: Evan Green --- drivers/input/rmi4/rmi_driver.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c index 772493b1f665..190b9974526b 100644 --- a/drivers/input/rmi4/rmi_driver.c +++ b/drivers/input/rmi4/rmi_driver.c @@ -146,7 +146,7 @@ static int rmi_process_interrupt_requests(struct rmi_device *rmi_dev) } mutex_lock(&data->irq_mutex); - bitmap_and(data->irq_status, data->irq_status, data->current_irq_mask, + bitmap_and(data->irq_status, data->irq_status, data->fn_irq_bits, data->irq_count); /* * At this point, irq_status has all bits that are set in the @@ -385,6 +385,8 @@ static int rmi_driver_set_irq_bits(struct rmi_device *rmi_dev, bitmap_copy(data->current_irq_mask, data->new_irq_mask, data->num_of_irq_regs); + bitmap_or(data->fn_irq_bits, data->fn_irq_bits, mask, data->irq_count); + error_unlock: mutex_unlock(&data->irq_mutex); return error; @@ -398,6 +400,8 @@ static int rmi_driver_clear_irq_bits(struct rmi_device *rmi_dev, struct device *dev = &rmi_dev->dev; mutex_lock(&data->irq_mutex); + bitmap_andnot(data->fn_irq_bits, + data->fn_irq_bits, mask, data->irq_count); bitmap_andnot(data->new_irq_mask, data->current_irq_mask, mask, data->irq_count); -- 2.21.0
KMSAN: uninit-value in bsearch
Hello, syzbot found the following crash on: HEAD commit:1e76a3e5 kmsan: replace __GFP_NO_KMSAN_SHADOW with kmsan_i.. git tree: https://github.com/google/kmsan.git master console output: https://syzkaller.appspot.com/x/log.txt?x=130b16fd60 kernel config: https://syzkaller.appspot.com/x/.config?x=f03c659d0830ab8d dashboard link: https://syzkaller.appspot.com/bug?extid=427ce10b72235583ef69 compiler: clang version 9.0.0 (/home/glider/llvm/clang 80fee25776c2fb61e74c1ecb1a523375c2500b69) Unfortunately, I don't have any reproducer for this crash yet. IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+427ce10b72235583e...@syzkaller.appspotmail.com dvb-usb: bulk message failed: -22 (1/-30591) = BUG: KMSAN: uninit-value in ir_lookup_by_scancode drivers/media/rc/rc-main.c:494 [inline] BUG: KMSAN: uninit-value in rc_g_keycode_from_table drivers/media/rc/rc-main.c:582 [inline] BUG: KMSAN: uninit-value in rc_keydown+0x1a6/0x6f0 drivers/media/rc/rc-main.c:816 CPU: 1 PID: 11436 Comm: kworker/1:2 Not tainted 5.3.0-rc7+ #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Workqueue: events dvb_usb_read_remote_control Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x191/0x1f0 lib/dump_stack.c:113 kmsan_report+0x13a/0x2b0 mm/kmsan/kmsan_report.c:108 __msan_warning+0x73/0xe0 mm/kmsan/kmsan_instr.c:250 bsearch+0x1dd/0x250 lib/bsearch.c:41 ir_lookup_by_scancode drivers/media/rc/rc-main.c:494 [inline] rc_g_keycode_from_table drivers/media/rc/rc-main.c:582 [inline] rc_keydown+0x1a6/0x6f0 drivers/media/rc/rc-main.c:816 cxusb_rc_query+0x2e1/0x360 drivers/media/usb/dvb-usb/cxusb.c:548 dvb_usb_read_remote_control+0xf9/0x290 drivers/media/usb/dvb-usb/dvb-usb-remote.c:261 process_one_work+0x1572/0x1ef0 kernel/workqueue.c:2269 worker_thread+0x111b/0x2460 kernel/workqueue.c:2415 kthread+0x4b5/0x4f0 kernel/kthread.c:256 ret_from_fork+0x35/0x40 arch/x86/entry/entry_64.S:355 Uninit was stored to memory at: kmsan_save_stack_with_flags mm/kmsan/kmsan.c:150 [inline] kmsan_internal_chain_origin+0xd2/0x170 mm/kmsan/kmsan.c:314 __msan_chain_origin+0x6b/0xe0 mm/kmsan/kmsan_instr.c:184 rc_g_keycode_from_table drivers/media/rc/rc-main.c:583 [inline] rc_keydown+0x2c4/0x6f0 drivers/media/rc/rc-main.c:816 cxusb_rc_query+0x2e1/0x360 drivers/media/usb/dvb-usb/cxusb.c:548 dvb_usb_read_remote_control+0xf9/0x290 drivers/media/usb/dvb-usb/dvb-usb-remote.c:261 process_one_work+0x1572/0x1ef0 kernel/workqueue.c:2269 worker_thread+0x111b/0x2460 kernel/workqueue.c:2415 kthread+0x4b5/0x4f0 kernel/kthread.c:256 ret_from_fork+0x35/0x40 arch/x86/entry/entry_64.S:355 Local variable description: ircode@cxusb_rc_query Variable was created at: cxusb_rc_query+0x4d/0x360 drivers/media/usb/dvb-usb/cxusb.c:543 dvb_usb_read_remote_control+0xf9/0x290 drivers/media/usb/dvb-usb/dvb-usb-remote.c:261 = --- This bug is generated by a bot. It may contain errors. See https://goo.gl/tpsmEJ for more information about syzbot. syzbot engineers can be reached at syzkal...@googlegroups.com. syzbot will keep track of this bug report. See: https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
Re: [PATCH v4] rtc: wilco-ec: Handle reading invalid times
LGTM. Reviewed-by: Daniel Campello
[PATCH] usb: iowarrior: fix access to freed data structure
struct iowarrior gets freed prematurely in iowarrior_release while
it is still being referenced from usb_interface, so let only
iowarrior_disconnect call iowarrior_delete.
Fixes: KMSAN: uninit-value in iowarrior_disconnect
Reported-by: syzbot+0761012cebf7bdb38...@syzkaller.appspotmail.com
Signed-off-by: Valentin Vidic
---
drivers/usb/misc/iowarrior.c | 35 +++
1 file changed, 15 insertions(+), 20 deletions(-)
diff --git a/drivers/usb/misc/iowarrior.c b/drivers/usb/misc/iowarrior.c
index f5bed9f29e56..0492ea76c4bf 100644
--- a/drivers/usb/misc/iowarrior.c
+++ b/drivers/usb/misc/iowarrior.c
@@ -638,7 +638,6 @@ static int iowarrior_open(struct inode *inode, struct file
*file)
static int iowarrior_release(struct inode *inode, struct file *file)
{
struct iowarrior *dev;
- int retval = 0;
dev = file->private_data;
if (!dev)
@@ -650,27 +649,23 @@ static int iowarrior_release(struct inode *inode, struct
file *file)
mutex_lock(&dev->mutex);
if (dev->opened <= 0) {
- retval = -ENODEV; /* close called more than once */
mutex_unlock(&dev->mutex);
- } else {
- dev->opened = 0;/* we're closing now */
- retval = 0;
- if (dev->present) {
- /*
- The device is still connected so we only shutdown
- pending read-/write-ops.
-*/
- usb_kill_urb(dev->int_in_urb);
- wake_up_interruptible(&dev->read_wait);
- wake_up_interruptible(&dev->write_wait);
- mutex_unlock(&dev->mutex);
- } else {
- /* The device was unplugged, cleanup resources */
- mutex_unlock(&dev->mutex);
- iowarrior_delete(dev);
- }
+ return -ENODEV; /* close called more than once */
}
- return retval;
+
+ dev->opened = 0;/* we're closing now */
+ if (dev->present) {
+ /*
+* The device is still connected so we only shutdown
+* pending read/write ops.
+*/
+ usb_kill_urb(dev->int_in_urb);
+ wake_up_interruptible(&dev->read_wait);
+ wake_up_interruptible(&dev->write_wait);
+ }
+
+ mutex_unlock(&dev->mutex);
+ return 0;
}
static __poll_t iowarrior_poll(struct file *file, poll_table * wait)
--
2.20.1
Re: [PATCH v3 5/5] livepatch: Selftests of the API for tracking system state changes
Hi Petr,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[cannot apply to v5.4-rc2 next-20191008]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url:
https://github.com/0day-ci/linux/commits/Petr-Mladek/livepatch-Keep-replaced-patches-until-post_patch-callback-is-called/20191003-171833
config: x86_64-randconfig-e004-201940 (attached as .config)
compiler: gcc-7 (Debian 7.4.0-13) 7.4.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot
All error/warnings (new ones prefixed by >>):
lib/livepatch/test_klp_state.c: In function 'allocate_loglevel_state':
>> lib/livepatch/test_klp_state.c:41:25: error: implicit declaration of
>> function 'kzalloc'; did you mean 'kvzalloc'?
>> [-Werror=implicit-function-declaration]
loglevel_state->data = kzalloc(sizeof(console_loglevel), GFP_KERNEL);
^~~
kvzalloc
>> lib/livepatch/test_klp_state.c:41:23: warning: assignment makes pointer from
>> integer without a cast [-Wint-conversion]
loglevel_state->data = kzalloc(sizeof(console_loglevel), GFP_KERNEL);
^
lib/livepatch/test_klp_state.c: In function 'free_loglevel_state':
>> lib/livepatch/test_klp_state.c:85:2: error: implicit declaration of function
>> 'kfree'; did you mean 'kvfree'? [-Werror=implicit-function-declaration]
kfree(loglevel_state->data);
^
kvfree
cc1: some warnings being treated as errors
--
lib/livepatch/test_klp_state2.c: In function 'allocate_loglevel_state':
>> lib/livepatch/test_klp_state2.c:48:25: error: implicit declaration of
>> function 'kzalloc'; did you mean 'kvzalloc'?
>> [-Werror=implicit-function-declaration]
loglevel_state->data = kzalloc(sizeof(console_loglevel), GFP_KERNEL);
^~~
kvzalloc
>> lib/livepatch/test_klp_state2.c:48:23: warning: assignment makes pointer
>> from integer without a cast [-Wint-conversion]
loglevel_state->data = kzalloc(sizeof(console_loglevel), GFP_KERNEL);
^
lib/livepatch/test_klp_state2.c: In function 'free_loglevel_state':
>> lib/livepatch/test_klp_state2.c:114:2: error: implicit declaration of
>> function 'kfree'; did you mean 'kvfree'?
>> [-Werror=implicit-function-declaration]
kfree(loglevel_state->data);
^
kvfree
cc1: some warnings being treated as errors
vim +41 lib/livepatch/test_klp_state.c
32
33 static int allocate_loglevel_state(void)
34 {
35 struct klp_state *loglevel_state;
36
37 loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
38 if (!loglevel_state)
39 return -EINVAL;
40
> 41 loglevel_state->data = kzalloc(sizeof(console_loglevel),
GFP_KERNEL);
42 if (!loglevel_state->data)
43 return -ENOMEM;
44
45 pr_info("%s: allocating space to store console_loglevel\n",
46 __func__);
47 return 0;
48 }
49
50 static void fix_console_loglevel(void)
51 {
52 struct klp_state *loglevel_state;
53
54 loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
55 if (!loglevel_state)
56 return;
57
58 pr_info("%s: fixing console_loglevel\n", __func__);
59 *(int *)loglevel_state->data = console_loglevel;
60 console_loglevel = CONSOLE_LOGLEVEL_MOTORMOUTH;
61 }
62
63 static void restore_console_loglevel(void)
64 {
65 struct klp_state *loglevel_state;
66
67 loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
68 if (!loglevel_state)
69 return;
70
71 pr_info("%s: restoring console_loglevel\n", __func__);
72 console_loglevel = *(int *)loglevel_state->data;
73 }
74
75 static void free_loglevel_state(void)
76 {
77 struct klp_state *loglevel_state;
78
79 loglevel_state = klp_get_state(&patch, CONSOLE_LOGLEVEL_STATE);
80 if (!loglevel_state)
81 return;
82
83 pr_info("%s: freeing space for the stored console_loglevel\n",
84 __func__);
> 85 kfree(loglevel_state->data);
86 }
87
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
[PATCH 0/3] AMD IOMMU Changes for NTB
Hi,
Please find the following patches which help support
Non-Transparent-Bridge (NTB) devices on AMD platforms with the IOMMU
enabled.
The first patch implements dma_map_resource() correctly with the AMD
IOMMU. This is required for correct functioning of ntb_transport which
uses that interface.
The second two patches add support for multiple PCI aliases. NTB
hardware will normally send TLPs from a range of requestor IDs to
facilitate routing the responses back to the correct requestor on the
other side of the bridge. To support this, NTB hardware registers a
number of PCI aliases. Currently the AMD IOMMU only allows for one
PCI alias so TLPs from the other aliases get rejected.
See commit ad281ecf1c7d ("PCI: Add DMA alias quirk for Microsemi
Switchtec NTB") for more information on this.
Similar patches were upstreamed for Intel hardware earlier this year:
commit 21d5d27c042d ("iommu/vt-d: Implement dma_[un]map_resource()")
commit 3f0c625c6ae7 ("iommu/vt-d: Allow interrupts from the entire bus
for aliased devices")
Thanks,
Logan
--
Kit Chow (1):
iommu/amd: Implement dma_[un]map_resource()
Logan Gunthorpe (2):
iommu/amd: Support multiple PCI DMA aliases in device table
iommu/amd: Support multiple PCI DMA aliases in IRQ Remapping
drivers/iommu/amd_iommu.c | 198 +++-
drivers/iommu/amd_iommu_types.h | 2 +-
2 files changed, 120 insertions(+), 80 deletions(-)
--
2.20.1
Re: [PATCH] x86: use the correct function type for native_set_fixmap
On Tue, Sep 24, 2019 at 12:51 PM Sami Tolvanen wrote: > Someone else probably knows better, but yes, we could also fix this by > changing set_fixmap to accept enum fixed_addresses as the first > parameter, and changing the type of xen_set_fixmap instead. This approach is actually more problematic since we cannot forward declare an enum in C and including here results in a ton of other missing dependencies. I assume this is why the callback function accepts unsigned in the first place. Sami
[PATCH v4] wilco_ec: Add Dell's USB PowerShare Policy control
USB PowerShare is a policy which affects charging via the special
USB PowerShare port (marked with a small lightning bolt or battery icon)
when in low power states:
- In S0, the port will always provide power.
- In S0ix, if usb_charge is enabled, then power will be supplied to
the port when on AC or if battery is > 50%. Else no power is supplied.
- In S5, if usb_charge is enabled, then power will be supplied to
the port when on AC. Else no power is supplied.
Signed-off-by: Daniel Campello
Signed-off-by: Nick Crews
---
v4 changes:
- Renamed from usb_power_share to usb_charge to match existing feature
in other platforms in the kernel (i.e., sony-laptop, samsung-laptop,
lg-laptop)
v3 changes:
- Drop a silly blank line
- Use val > 1 instead of val != 0 && val != 1
v2 changes:
- Move documentation to Documentation/ABI/testing/sysfs-platform-wilco-ec
- Zero out reserved bytes in requests.
.../ABI/testing/sysfs-platform-wilco-ec | 17
drivers/platform/chrome/wilco_ec/sysfs.c | 91 +++
2 files changed, 108 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-platform-wilco-ec
b/Documentation/ABI/testing/sysfs-platform-wilco-ec
index 8827a734f933..bb7ba67cae97 100644
--- a/Documentation/ABI/testing/sysfs-platform-wilco-ec
+++ b/Documentation/ABI/testing/sysfs-platform-wilco-ec
@@ -31,6 +31,23 @@ Description:
Output will a version string be similar to the example below:
08B6
+What: /sys/bus/platform/devices/GOOG000C\:00/usb_charge
+Date: October 2019
+KernelVersion: 5.5
+Description:
+ Control the USB PowerShare Policy. USB PowerShare is a policy
+ which affects charging via the special USB PowerShare port
+ (marked with a small lightning bolt or battery icon) when in
+ low power states:
+ - In S0, the port will always provide power.
+ - In S0ix, if usb_charge is enabled, then power will be
+ supplied to the port when on AC or if battery is > 50%.
+ Else no power is supplied.
+ - In S5, if usb_charge is enabled, then power will be supplied
+ to the port when on AC. Else no power is supplied.
+
+ Input should be either "0" or "1".
+
What: /sys/bus/platform/devices/GOOG000C\:00/version
Date: May 2019
KernelVersion: 5.3
diff --git a/drivers/platform/chrome/wilco_ec/sysfs.c
b/drivers/platform/chrome/wilco_ec/sysfs.c
index 3b86a21005d3..f0d174b6bb21 100644
--- a/drivers/platform/chrome/wilco_ec/sysfs.c
+++ b/drivers/platform/chrome/wilco_ec/sysfs.c
@@ -23,6 +23,26 @@ struct boot_on_ac_request {
u8 reserved7;
} __packed;
+#define CMD_USB_CHARGE 0x39
+
+enum usb_charge_op {
+ USB_CHARGE_GET = 0,
+ USB_CHARGE_SET = 1,
+};
+
+struct usb_charge_request {
+ u8 cmd; /* Always CMD_USB_CHARGE */
+ u8 reserved;
+ u8 op; /* One of enum usb_charge_op */
+ u8 val; /* When setting, either 0 or 1 */
+} __packed;
+
+struct usb_charge_response {
+ u8 reserved;
+ u8 status; /* Set by EC to 0 on success, other value on failure */
+ u8 val; /* When getting, set by EC to either 0 or 1 */
+} __packed;
+
#define CMD_EC_INFO0x38
enum get_ec_info_op {
CMD_GET_EC_LABEL= 0,
@@ -131,12 +151,83 @@ static ssize_t model_number_show(struct device *dev,
static DEVICE_ATTR_RO(model_number);
+static int send_usb_charge(struct wilco_ec_device *ec,
+ struct usb_charge_request *rq,
+ struct usb_charge_response *rs)
+{
+ struct wilco_ec_message msg;
+ int ret;
+
+ memset(&msg, 0, sizeof(msg));
+ msg.type = WILCO_EC_MSG_LEGACY;
+ msg.request_data = rq;
+ msg.request_size = sizeof(*rq);
+ msg.response_data = rs;
+ msg.response_size = sizeof(*rs);
+ ret = wilco_ec_mailbox(ec, &msg);
+ if (ret < 0)
+ return ret;
+ if (rs->status)
+ return -EIO;
+
+ return 0;
+}
+
+static ssize_t usb_charge_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct wilco_ec_device *ec = dev_get_drvdata(dev);
+ struct usb_charge_request rq;
+ struct usb_charge_response rs;
+ int ret;
+
+ memset(&rq, 0, sizeof(rq));
+ rq.cmd = CMD_USB_CHARGE;
+ rq.op = USB_CHARGE_GET;
+
+ ret = send_usb_charge(ec, &rq, &rs);
+ if (ret < 0)
+ return ret;
+
+ return sprintf(buf, "%d\n", rs.val);
+}
+
+static ssize_t usb_charge_store(struct device *dev,
+struct device_attribute *attr,
+const char *buf, size_t count)
+{
+ struct wilco_ec_device *ec = dev_get_drvdata(dev);
+ struct usb_charge_request rq;
+ struct
Re: pivot_root(".", ".") and the fchdir() dance
"Michael Kerrisk (man-pages)" writes:
> On 10/8/19 9:40 PM, Eric W. Biederman wrote:
>> "Michael Kerrisk (man-pages)" writes:
>>
>>> Hello Eric,
>>>
>> Creating of a mount namespace in a user namespace automatically does
>> 'mount("", "/", MS_SLAVE | MS_REC, NULL);' if the starting mount
>> namespace was not created in that user namespace. AKA creating
>> a mount namespace in a user namespace does the unshare for you.
>
> Oh -- I had forgotten that detail. But it is documented
> (by you, I think) in mount_namespaces(7):
>
>* A mount namespace has an owner user namespace. A
> mount namespace whose owner user namespace is differ‐
> ent from the owner user namespace of its parent mount
> namespace is considered a less privileged mount names‐
> pace.
>
>* When creating a less privileged mount namespace,
> shared mounts are reduced to slave mounts. (Shared
> and slave mounts are discussed below.) This ensures
> that mappings performed in less privileged mount
> namespaces will not propagate to more privileged mount
> namespaces.
>
> There's one point that description that troubles me. There is a
> reference to "parent mount namespace", but as I understand things
> there is no parental relationship among mount namespaces instances
> (or am I wrong?). Should that wording not be rather something
> like "the mount namespace of the process that created this mount
> namespace"?
How about "the mount namespace this mount namespace started as a copy of"
You are absolutely correct there is no relationship between mount
namespaces. There is just the propagation tree between mounts. (Which
acts similarly to a parent/child relationship but is not at all the same
thing).
>>>
>>> Thanks. I made the text as follows:
>>>
>>>* Each mount namespace has an owner user namespace. As noted
>>> above, when a new mount namespace is created, it inherits a
>>> copy of the mount points from the mount namespace of the
>>> process that created the new mount namespace. If the two mount
>>> namespaces are owned by different user namespaces, then the new
>>> mount namespace is considered less privileged.
>>
>> I hate to nitpick,
>
> I love it when you nitpick. Thanks for your attention to the details
> of my wording.
>
>> but I am going to say that when I read the text above
>> the phrase "mount namespace of the process that created the new mount
>> namespace" feels wrong.
>>
>> Either you use unshare(2) and the mount namespace of the process that
>> created the mount namespace changes.
>>
>> Or you use clone(2) and you could argue it is the new child that created
>> the mount namespace.
>>
>> Having a different mount namespace at the end of the creation operation
>> feels like it makes your phrase confusing about what the starting
>> mount namespace is. I hate to use references that are ambiguous when
>> things are changing.
>>
>> I agree that the term parent is also wrong.
>
> I see what you mean. My wording is imprecise.
>
> So, I tweaked text earlier in the page so that it now reads
> as follows:
>
>A new mount namespace is created using either clone(2) or
>unshare(2) with the CLONE_NEWNS flag. When a new mount namespace
>is created, its mount point list is initialized as follows:
>
>* If the namespace is created using clone(2), the mount point
> list of the child's namespace is a copy of the mount point list
> in the parent's namespace.
>
>* If the namespace is created using unshare(2), the mount point
> list of the new namespace is a copy of the mount point list in
> the caller's previous mount namespace.
>
> And then I tweaked the text that we are currently discussing to read:
>
>* Each mount namespace has an owner user namespace. As explained
> above, when a new mount namespace is created, its mount point
> list is initialized as a copy of the mount point list of
> another mount namespace. If the new namespaces and the names‐
> pace from which the mount point list was copied are owned by
> different user namespaces, then the new mount namespace is con‐
> sidered less privileged.
>
> How does this look to you now?
Much better thank you.
Eric
Re: [PATCH v2] arm64: lse: fix LSE atomics with LLVM's integrated assembler
On Tue, Oct 8, 2019 at 2:27 PM 'Sami Tolvanen' via Clang Built Linux
wrote:
>
> Unlike gcc, clang considers each inline assembly block to be independent
> and therefore, when using the integrated assembler for inline assembly,
> any preambles that enable features must be repeated in each block.
>
> This change defines __LSE_PREAMBLE and adds it to each inline assembly
> block that has LSE instructions, which allows them to be compiled also
> with clang's assembler.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/671
> Signed-off-by: Sami Tolvanen
Thanks, I think this will better limit the assembler to use of these
instructions, while preventing the C compiler from emitting them.
$ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make CC=clang AS=clang -j71 clean
$ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make CC=clang AS=clang
-j71 defconfig
$ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make CC=clang AS=clang
-j71 fs/ext4/balloc.o
$ git am
$ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make CC=clang AS=clang
-j71 fs/ext4/balloc.o
...
$ echo $?
0
$ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make CC=clang AS=clang -j71 clean
$ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make CC=clang -j71 defconfig
$ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- make CC=clang -j71
$ qemu-system-aarch64 -kernel arch/arm64/boot/Image.gz -machine virt
-cpu cortex-a72 -nographic --append "console=ttyAMA0" -m 2048 -initrd
/android1/buildroot/output/images/rootfs.cpio
Reviewed-by: Nick Desaulniers
Tested-by: Nick Desaulniers
> ---
> v2:
> - Add a preamble to inline assembly blocks that use LSE instead
>of allowing the compiler to emit LSE instructions everywhere.
>
> ---
> arch/arm64/include/asm/atomic_lse.h | 19 +++
> arch/arm64/include/asm/lse.h| 6 +++---
> 2 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/include/asm/atomic_lse.h
> b/arch/arm64/include/asm/atomic_lse.h
> index c6bd87d2915b..3ee600043042 100644
> --- a/arch/arm64/include/asm/atomic_lse.h
> +++ b/arch/arm64/include/asm/atomic_lse.h
> @@ -14,6 +14,7 @@
> static inline void __lse_atomic_##op(int i, atomic_t *v)
> \
> { \
> asm volatile( \
> + __LSE_PREAMBLE \
> " " #asm_op " %w[i], %[v]\n" \
> : [i] "+r" (i), [v] "+Q" (v->counter) \
> : "r" (v)); \
> @@ -30,6 +31,7 @@ ATOMIC_OP(add, stadd)
> static inline int __lse_atomic_fetch_##op##name(int i, atomic_t *v)\
> { \
> asm volatile( \
> + __LSE_PREAMBLE \
> " " #asm_op #mb " %w[i], %w[i], %[v]" \
> : [i] "+r" (i), [v] "+Q" (v->counter) \
> : "r" (v) \
> @@ -58,6 +60,7 @@ static inline int __lse_atomic_add_return##name(int i,
> atomic_t *v) \
> u32 tmp;\
> \
> asm volatile( \
> + __LSE_PREAMBLE \
> " ldadd" #mb "%w[i], %w[tmp], %[v]\n" \
> " add %w[i], %w[i], %w[tmp]" \
> : [i] "+r" (i), [v] "+Q" (v->counter), [tmp] "=&r" (tmp)\
> @@ -77,6 +80,7 @@ ATOMIC_OP_ADD_RETURN(, al, "memory")
> static inline void __lse_atomic_and(int i, atomic_t *v)
> {
> asm volatile(
> + __LSE_PREAMBLE
> " mvn %w[i], %w[i]\n"
> " stclr %w[i], %[v]"
> : [i] "+&r" (i), [v] "+Q" (v->counter)
> @@ -87,6 +91,7 @@ static inline void __lse_atomic_and(int i, atomic_t *v)
> static inline int __lse_atomic_fetch_and##name(int i, atomic_t *v) \
> { \
> asm volatile( \
> + __LSE_PREAMBLE \
> " mvn %w[i], %w[i]\n" \
> " ldclr" #mb "%w[i], %w[i], %[v]" \
> : [i] "+&r" (i), [v] "+Q" (v->counter) \
> @@ -106,6 +111,7 @@ ATOMIC_FETCH_OP_AND(, al, "memory")
> static inline void __lse_atomic_sub(int i, atomic_t *v)
> {
> asm volatile(
> + __LSE_PREAMBLE
> " neg %w[i], %w[i]\n"
>
[PATCH v8 4/4] ftrace: Add an option for tracing console latencies
This new trace option "console-latency" will enable the latency
tracers to trace the console latencies. Previously this has always been
implicitely disabled. I guess this is because they are considered
to be well known and unavoidable.
However, for some organizations it may nevertheless be desirable to
trace them. Basically, we want to be able to tell that there are
latencies in the system under test because someone has incorrectly
enabled the serial console.
Signed-off-by: Viktor Rosendahl (BMW)
---
include/linux/irqflags.h | 22 ++
kernel/printk/printk.c | 6 --
kernel/trace/trace.h | 1 +
kernel/trace/trace_irqsoff.c | 18 ++
4 files changed, 45 insertions(+), 2 deletions(-)
diff --git a/include/linux/irqflags.h b/include/linux/irqflags.h
index 21619c92c377..3de891723331 100644
--- a/include/linux/irqflags.h
+++ b/include/linux/irqflags.h
@@ -13,6 +13,7 @@
#define _LINUX_TRACE_IRQFLAGS_H
#include
+#include
#include
/* Currently trace_softirqs_on/off is used only by lockdep */
@@ -68,9 +69,30 @@ do { \
defined(CONFIG_PREEMPT_TRACER)
extern void stop_critical_timings(void);
extern void start_critical_timings(void);
+ extern bool console_tracing_disabled(void);
+
+# define console_stop_critical_timings(flag) \
+ do {\
+ typecheck(bool, flag); \
+ flag = console_tracing_disabled(); \
+ if (flag) \
+ stop_critical_timings();\
+ } while (0)
+
+# define console_start_critical_timings(flag) \
+ do { \
+ typecheck(bool, flag); \
+ if (flag)\
+ start_critical_timings();\
+ } while (0)
+
#else
# define stop_critical_timings() do { } while (0)
# define start_critical_timings() do { } while (0)
+# define console_stop_critical_timings(flag) \
+ do { typecheck(bool, flag); } while (0)
+# define console_start_critical_timings(flag) \
+ do { typecheck(bool, flag); } while (0)
#endif
/*
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index ca65327a6de8..f27e96273453 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2369,6 +2369,7 @@ void console_unlock(void)
static char ext_text[CONSOLE_EXT_LOG_MAX];
static char text[LOG_LINE_MAX + PREFIX_MAX];
unsigned long flags;
+ bool cflag;
bool do_cond_resched, retry;
if (console_suspended) {
@@ -2469,9 +2470,10 @@ void console_unlock(void)
*/
console_lock_spinning_enable();
- stop_critical_timings();/* don't trace print latency */
+ /* don't trace print latency if it's disabled */
+ console_stop_critical_timings(cflag);
call_console_drivers(ext_text, ext_len, text, len);
- start_critical_timings();
+ console_start_critical_timings(cflag);
if (console_lock_spinning_disable_and_check()) {
printk_safe_exit_irqrestore(flags);
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 591c7a873235..10d12c8f7f77 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1261,6 +1261,7 @@ extern int trace_get_user(struct trace_parser *parser,
const char __user *ubuf,
C(PRINTK_MSGONLY, "printk-msg-only"), \
C(CONTEXT_INFO, "context-info"), /* Print
pid/cpu/time */ \
C(LATENCY_FMT, "latency-format"), \
+ C(CONSOLE_LATENCY, "console-latency"), \
C(RECORD_CMD, "record-cmd"), \
C(RECORD_TGID, "record-tgid"), \
C(OVERWRITE,"overwrite"), \
diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
index a745b0cee5d3..576e2162114e 100644
--- a/kernel/trace/trace_irqsoff.c
+++ b/kernel/trace/trace_irqsoff.c
@@ -456,6 +456,24 @@ void stop_critical_timings(void)
EXPORT_SYMBOL_GPL(stop_critical_timings);
NOKPROBE_SYMBOL(stop_critical_timings);
+bool console_tracing_disabled(void)
+{
+ struct trace_array *tr = irqsoff_trace;
+ int pc = preempt_count();
+
+ /*
+* If tracing is disabled, then the question of whether to trace console
+* latencies is moot. By always returning false here we save the caller
+* the calls to start/stop_critical_timings(). These calls would not do
+* anything anyway.
+*/
+ if (!preempt_trace(pc) && !irq_trace())
+ return false;
+
+ return !(tr->trace_flags & TRACE_ITER_CONSOLE_LATENCY);
+}
+EXPORT_SYMBOL_GP
[PATCH v8 2/4] preemptirq_delay_test: Add the burst feature and a sysfs trigger
This burst feature enables the user to generate a burst of
preempt/irqsoff latencies. This makes it possible to test whether we
are able to detect latencies that systematically occur very close to
each other.
The maximum burst size is 10. We also create 10 identical test
functions, so that we get 10 different backtraces; this is useful
when we want to test whether we can detect all the latencies in a
burst. Otherwise, there would be no easy way of differentiating
between which latency in a burst was captured by the tracer.
In addition, there is a sysfs trigger, so that it's not necessary to
reload the module to repeat the test. The trigger will appear as
/sys/kernel/preemptirq_delay_test/trigger in sysfs.
Signed-off-by: Viktor Rosendahl (BMW)
Reviewed-by: Joel Fernandes (Google)
---
kernel/trace/Kconfig | 6 +-
kernel/trace/preemptirq_delay_test.c | 144 +++
2 files changed, 128 insertions(+), 22 deletions(-)
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index e08527f50d2a..2a58380ea310 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -752,9 +752,9 @@ config PREEMPTIRQ_DELAY_TEST
configurable delay. The module busy waits for the duration of the
critical section.
- For example, the following invocation forces a one-time irq-disabled
- critical section for 500us:
- modprobe preemptirq_delay_test test_mode=irq delay=50
+ For example, the following invocation generates a burst of three
+ irq-disabled critical sections for 500us:
+ modprobe preemptirq_delay_test test_mode=irq delay=500 burst_size=3
If unsure, say N
diff --git a/kernel/trace/preemptirq_delay_test.c
b/kernel/trace/preemptirq_delay_test.c
index d8765c952fab..31c0fad4cb9e 100644
--- a/kernel/trace/preemptirq_delay_test.c
+++ b/kernel/trace/preemptirq_delay_test.c
@@ -10,18 +10,25 @@
#include
#include
#include
+#include
#include
#include
#include
#include
+#include
static ulong delay = 100;
-static char test_mode[10] = "irq";
+static char test_mode[12] = "irq";
+static uint burst_size = 1;
-module_param_named(delay, delay, ulong, S_IRUGO);
-module_param_string(test_mode, test_mode, 10, S_IRUGO);
-MODULE_PARM_DESC(delay, "Period in microseconds (100 uS default)");
-MODULE_PARM_DESC(test_mode, "Mode of the test such as preempt or irq (default
irq)");
+module_param_named(delay, delay, ulong, 0444);
+module_param_string(test_mode, test_mode, 12, 0444);
+module_param_named(burst_size, burst_size, uint, 0444);
+MODULE_PARM_DESC(delay, "Period in microseconds (100 us default)");
+MODULE_PARM_DESC(test_mode, "Mode of the test such as preempt, irq, or
alternate (default irq)");
+MODULE_PARM_DESC(burst_size, "The size of a burst (default 1)");
+
+#define MIN(x, y) ((x) < (y) ? (x) : (y))
static void busy_wait(ulong time)
{
@@ -34,37 +41,136 @@ static void busy_wait(ulong time)
} while ((end - start) < (time * 1000));
}
-static int preemptirq_delay_run(void *data)
+static __always_inline void irqoff_test(void)
{
unsigned long flags;
+ local_irq_save(flags);
+ busy_wait(delay);
+ local_irq_restore(flags);
+}
- if (!strcmp(test_mode, "irq")) {
- local_irq_save(flags);
- busy_wait(delay);
- local_irq_restore(flags);
- } else if (!strcmp(test_mode, "preempt")) {
- preempt_disable();
- busy_wait(delay);
- preempt_enable();
+static __always_inline void preemptoff_test(void)
+{
+ preempt_disable();
+ busy_wait(delay);
+ preempt_enable();
+}
+
+static void execute_preemptirqtest(int idx)
+{
+ if (!strcmp(test_mode, "irq"))
+ irqoff_test();
+ else if (!strcmp(test_mode, "preempt"))
+ preemptoff_test();
+ else if (!strcmp(test_mode, "alternate")) {
+ if (idx % 2 == 0)
+ irqoff_test();
+ else
+ preemptoff_test();
}
+}
+
+#define DECLARE_TESTFN(POSTFIX)\
+ static void preemptirqtest_##POSTFIX(int idx) \
+ { \
+ execute_preemptirqtest(idx);\
+ } \
+/*
+ * We create 10 different functions, so that we can get 10 different
+ * backtraces.
+ */
+DECLARE_TESTFN(0)
+DECLARE_TESTFN(1)
+DECLARE_TESTFN(2)
+DECLARE_TESTFN(3)
+DECLARE_TESTFN(4)
+DECLARE_TESTFN(5)
+DECLARE_TESTFN(6)
+DECLARE_TESTFN(7)
+DECLARE_TESTFN(8)
+DECLARE_TESTFN(9)
+
+static void (*testfuncs[])(int) = {
+ preemptirqtest_0,
+ preemptirqtest_1,
+ preemptirqtest_2,
+ preemptirqtest_3,
+ preemptirqtest_4,
+ preemptirqtest_5,
+ preemptirqtest_6,
+ preemptirqtest_7,
+ preemptirqtest_8,
+ preemptirqtest_9,
+};
+
+#define NR_TES
[PATCH v8 3/4] Add the latency-collector to tools
This is a tool that is intended to work around the fact that the preemptoff, irqsoff, and preemptirqsoff tracers only work in overwrite mode. The idea is to act randomly in such a way that we do not systematically lose any latencies, so that if enough testing is done, all latencies will be captured. If the same burst of latencies is repeated, then sooner or later we will have captured all the latencies. It also works with the wakeup_dl, wakeup_rt, wakeup, and hwlat tracers. However, in that case it is probably not useful to use the random sleep functionality. The reason why it may be desirable to catch all latencies with a long test campaign is that for some organizations, it's necessary to test the kernel in the field and not practical for developers to work iteratively with field testers. Because of cost and project schedules it is not possible to start a new test campaign every time a latency problem has been fixed. It uses inotify to detect changes to /sys/kernel/debug/tracing/trace. When a latency is detected, it will either sleep or print immediately, depending on a function that act as an unfair coin toss. If immediate print is chosen, it means that we open /sys/kernel/debug/tracing/trace and thereby cause a blackout period that will hide any subsequent latencies. If sleep is chosen, it means that we wait before opening /sys/kernel/debug/tracing/trace, by default for 1000 ms, to see if there is another latency during this period. If there is, then we will lose the previous latency. The coin will be tossed again with a different probability, and we will either print the new latency, or possibly a subsequent one. The probability for the unfair coin toss is chosen so that there is equal probability to obtain any of the latencies in a burst. However, this assumes that we make an assumption of how many latencies there can be. By default the program assumes that there are no more than 2 latencies in a burst, the probability of immediate printout will be: 1/2 and 1 Thus, the probability of getting each of the two latencies will be 1/2. If we ever find that there is more than one latency in a series, meaning that we reach the probability of 1, then the table will be expanded to: 1/3, 1/2, and 1 Thus, we assume that there are no more than three latencies and each with a probability of 1/3 of being captured. If the probability of 1 is reached in the new table, that is we see more than two closely occurring latencies, then the table will again be extended, and so on. On my systems, it seems like this scheme works fairly well, as long as the latencies we trace are long enough, 300 us seems to be enough. This userspace program receive the inotify event at the end of a latency, and it has time until the end of the next latency to react, that is to open /sys/kernel/debug/tracing/trace. Thus, if we trace latencies that are >300 us, then we have at least 300 us to react. The minimum latency will of course not be 300 us on all systems, it will depend on the hardware, kernel version, workload and configuration. Example output: root@myhost:~# echo 100 > /sys/kernel/debug/tracing/tracing_thresh root@myhost:~# echo preemptirqsoff > /sys/kernel/debug/tracing/current_tracer root@myhost:~# latency-collector -rvv -a 11 & [1] 4915 root@myhost:~# Running with policy rr and priority 99. Using 3 print threads. /sys/kernel/debug/tracing/trace will be printed with random delay Start size of the probability table:11 Print a message when prob. table table changes size:true Print a warning when an event has been lost:true Sleep time is: 1000 ms Initializing probability table to 11 Thread 4916 runs with policy rr and priority 99 Thread 4917 runs with policy rr and priority 99 Thread 4918 runs with policy rr and priority 99 Thread 4919 runs with policy rr and priority 99 root@myhost:~# modprobe preemptirq_delay_test delay=105 test_mode=alternate burst_size=10 2850.638308 Latency 1 printout skipped due to random delay 2850.638383 Latency 2 printout skipped due to random delay 2850.638497 Latency 3 immediate print BEGIN < alternat-49226d...0us!: execute_preemptirqtest <-preemptirqtest_2 alternat-49226d... 106us : execute_preemptirqtest <-preemptirqtest_2 alternat-49226d... 106us : tracer_hardirqs_on <-preemptirqtest_2 alternat-49226d... 108us : => preemptirqtest_2 => preemptirq_delay_run => kthread => ret_from_fork >> END < In the example above, we randomly, happened to get the third latency in a burst burst of 10. If the experiment is repeated enough times, we will get all 10. Sometimes, there will be lost latencies, so that we get: 3054.078294 Latency 44 printout skipped due to inotify loop ..or: 3211.957685 Latency 112 printout skipped due to print loop In my experie
[PATCH v8 0/4] Some new features for the preempt/irqsoff tracers
Hello all, I have retained the fourth patch, although it was suggested that is becoming obsolete soon. I have retained it only because I do not know the status of the code that will make it obsolete. It's the last patch of the series and if there indeed is some code that will remove the latency issues from the printk code, then of course it makes sense to drop it. The first three patches will work without it. Changes in v8: - [PATCH 1/4]: * Moved a forward declaration in order to make code a bit more robust. * Converted a macro NOP to a static function so that the type is checked. - [PATCH 2/4]: * No change. - [PATCH 3/4]: * No change - [PACTH 4/4]: * Added a comment to explain an optimization in console_tracing_disabled(). This series is meant to address two issues with the latency tracing. The first three patches provide a method to trace latencies that always occurs very close to each other and to differentiate between them, in spite of the fact that the latency tracers work in overwrite mode. [PATCH 1/4] This implement fs notification for tracing_max_latency. It makes it possible for userspace to detect when a new latency has been detected. [PATCH 2/4] This extends the preemptirq_delay_test module so that it can be used to generate a burst of closely occurring latencies. [PATCH 3/4] This adds a user space program to the tools directory that utilizes the fs notification feature and a randomized algorithm to print out any of the latencies in a burst with approximately equal probability. The last patch is not directly connected but earlier it didn't apply cleanly on its own. However, now it does, so in principle it could be applied separately from the others. [PATCH 4/4] This adds the option console-latency to the trace options. This makes it possible to enable tracing of console latencies. best regards, Viktor Viktor Rosendahl (BMW) (4): ftrace: Implement fs notification for tracing_max_latency preemptirq_delay_test: Add the burst feature and a sysfs trigger Add the latency-collector to tools ftrace: Add an option for tracing console latencies include/linux/irqflags.h | 22 + kernel/printk/printk.c |6 +- kernel/trace/Kconfig |6 +- kernel/trace/preemptirq_delay_test.c | 144 ++- kernel/trace/trace.c | 75 +- kernel/trace/trace.h | 19 + kernel/trace/trace_hwlat.c |4 +- kernel/trace/trace_irqsoff.c | 18 + tools/Makefile | 14 +- tools/trace/Makefile | 20 + tools/trace/latency-collector.c | 1212 ++ 11 files changed, 1507 insertions(+), 33 deletions(-) create mode 100644 tools/trace/Makefile create mode 100644 tools/trace/latency-collector.c -- 2.17.1
[PATCH v8 1/4] ftrace: Implement fs notification for tracing_max_latency
This patch implements the feature that the tracing_max_latency file,
e.g. /sys/kernel/debug/tracing/tracing_max_latency will receive
notifications through the fsnotify framework when a new latency is
available.
One particularly interesting use of this facility is when enabling
threshold tracing, through /sys/kernel/debug/tracing/tracing_thresh,
together with the preempt/irqsoff tracers. This makes it possible to
implement a user space program that can, with equal probability,
obtain traces of latencies that occur immediately after each other in
spite of the fact that the preempt/irqsoff tracers operate in overwrite
mode.
This facility works with the hwlat, preempt/irqsoff, and wakeup
tracers.
The tracers may call the latency_fsnotify() from places such as
__schedule() or do_idle(); this makes it impossible to call
queue_work() directly without risking a deadlock. The same would
happen with a softirq, kernel thread or tasklet. For this reason we
use the irq_work mechanism to call queue_work().
This patch creates a new workqueue. The reason for doing this is that
I wanted to use the WQ_UNBOUND and WQ_HIGHPRI flags. My thinking was
that WQ_UNBOUND might help with the latency in some important cases.
If we use:
queue_work(system_highpri_wq, &tr->fsnotify_work);
then the work will (almost) always execute on the same CPU but if we are
unlucky that CPU could be too busy while there could be another CPU in
the system that would be able to process the work soon enough.
queue_work_on() could be used to queue the work on another CPU but it
seems difficult to select the right CPU.
Signed-off-by: Viktor Rosendahl (BMW)
Reviewed-by: Joel Fernandes (Google)
---
kernel/trace/trace.c | 75 +-
kernel/trace/trace.h | 18 +
kernel/trace/trace_hwlat.c | 4 +-
3 files changed, 94 insertions(+), 3 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 252f79c435f8..f8ca307f66cf 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -44,6 +44,9 @@
#include
#include
#include
+#include
+#include
+#include
#include "trace.h"
#include "trace_output.h"
@@ -1479,6 +1482,74 @@ static ssize_t trace_seq_to_buffer(struct trace_seq *s,
void *buf, size_t cnt)
}
unsigned long __read_mostlytracing_thresh;
+static const struct file_operations tracing_max_lat_fops;
+
+#if (defined(CONFIG_TRACER_MAX_TRACE) || defined(CONFIG_HWLAT_TRACER)) && \
+ defined(CONFIG_FSNOTIFY)
+
+static struct workqueue_struct *fsnotify_wq;
+
+static void latency_fsnotify_workfn(struct work_struct *work)
+{
+ struct trace_array *tr = container_of(work, struct trace_array,
+ fsnotify_work);
+ fsnotify(tr->d_max_latency->d_inode, FS_MODIFY,
+tr->d_max_latency->d_inode, FSNOTIFY_EVENT_INODE, NULL, 0);
+}
+
+static void latency_fsnotify_workfn_irq(struct irq_work *iwork)
+{
+ struct trace_array *tr = container_of(iwork, struct trace_array,
+ fsnotify_irqwork);
+ queue_work(fsnotify_wq, &tr->fsnotify_work);
+}
+
+static void trace_create_maxlat_file(struct trace_array *tr,
+struct dentry *d_tracer)
+{
+ INIT_WORK(&tr->fsnotify_work, latency_fsnotify_workfn);
+ init_irq_work(&tr->fsnotify_irqwork, latency_fsnotify_workfn_irq);
+ tr->d_max_latency = trace_create_file("tracing_max_latency", 0644,
+ d_tracer, &tr->max_latency,
+ &tracing_max_lat_fops);
+}
+
+__init static int latency_fsnotify_init(void)
+{
+ fsnotify_wq = alloc_workqueue("tr_max_lat_wq",
+ WQ_UNBOUND | WQ_HIGHPRI, 0);
+ if (!fsnotify_wq) {
+ pr_err("Unable to allocate tr_max_lat_wq\n");
+ return -ENOMEM;
+ }
+ return 0;
+}
+
+late_initcall_sync(latency_fsnotify_init);
+
+void latency_fsnotify(struct trace_array *tr)
+{
+ if (!fsnotify_wq)
+ return;
+ /*
+* We cannot call queue_work(&tr->fsnotify_work) from here because it's
+* possible that we are called from __schedule() or do_idle(), which
+* could cause a deadlock.
+*/
+ irq_work_queue(&tr->fsnotify_irqwork);
+}
+
+/*
+ * (defined(CONFIG_TRACER_MAX_TRACE) || defined(CONFIG_HWLAT_TRACER)) && \
+ * defined(CONFIG_FSNOTIFY)
+ */
+#else
+
+#define trace_create_maxlat_file(tr, d_tracer) \
+ trace_create_file("tracing_max_latency", 0644, d_tracer,\
+ &tr->max_latency, &tracing_max_lat_fops)
+
+#endif
#ifdef CONFIG_TRACER_MAX_TRACE
/*
@@ -1518,6 +1589,7 @@ __update_max_tr(struct trace_array *tr, struct
task_struct *tsk, int cpu)
/* record this tasks comm */
tracing_record_cmdline(tsk);
+ latency_fsnotify(tr);
}
/**
@@ -8550,8 +862
Re: [PATCH] PCI/IOV: update num_VFs earlier
On 10/08/2019 05:38 PM, Bjorn Helgaas wrote:
On Thu, Oct 03, 2019 at 05:10:07PM -0500, Bjorn Helgaas wrote:
On Thu, Oct 03, 2019 at 11:04:45AM +0200, CREGUT Pierre IMT/OLN wrote:
...
NIC drivers send netlink events when their state change, but it is
the core that changes the value of num_vfs. So I would think it is
the core responsibility to make sure the exposed value makes sense
and it would be better to ignore the details of the driver
implementation.
Yes, I think you're right. And I like your previous suggestion of
just locking the device in the reader. I'm not enough of a sysfs
expert to know if there's a good reason to avoid a lock there. Does
the following look reasonable to you?
I applied the patch below to pci/virtualization for v5.5, thanks for
I hope not... see below
your great patience!
commit 0940fc95da45
Author: Pierre Crégut
Date: Wed Sep 11 09:27:36 2019 +0200
PCI/IOV: Serialize sysfs sriov_numvfs reads vs writes
When sriov_numvfs is being updated, drivers may notify about new devices
before they are reflected in sriov->num_VFs, so concurrent sysfs reads
previously returned stale values.
Serialize the sysfs read vs the write so the read returns the correct
num_VFs value.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=202991
Link:
https://lore.kernel.org/r/20190911072736.32091-1-pierre.cre...@orange.com
Signed-off-by: Pierre Crégut
Signed-off-by: Bjorn Helgaas
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index b3f972e8cfed..e77562aabbae 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -254,8 +254,14 @@ static ssize_t sriov_numvfs_show(struct device *dev,
char *buf)
{
struct pci_dev *pdev = to_pci_dev(dev);
+ u16 num_vfs;
+
+ /* Serialize vs sriov_numvfs_store() so readers see valid num_VFs */
+ device_lock(&pdev->dev);
^ lock
+ num_vfs = pdev->sriov->num_VFs;
+ device_lock(&pdev->dev);
and lock again!
- return sprintf(buf, "%u\n", pdev->sriov->num_VFs);
+ return sprintf(buf, "%u\n", num_vfs);
}
/*
Re: [PATCH 3/3] Input: mms114 - add support for mms345l
On Mon, Oct 07, 2019 at 10:50:21PM +0200, Stephan Gerhold wrote: > MMS345L is another first generation touch screen from Melfas, > which uses the same registers as MMS152. > > However, using I2C_M_NOSTART for it causes errors when reading: > > i2c i2c-0: sendbytes: NAK bailout. > mms114 0-0048: __mms114_read_reg: i2c transfer failed (-5) > > The driver works fine as soon as I2C_M_NOSTART is removed. > > Add a separate melfas,mms345l binding, and make use of I2C_M_NOSTART > only for MMS114 and MMS152. > > Signed-off-by: Stephan Gerhold > --- > Note: I was not able to find a datasheet for any of the models, > so this change is merely based on testing and comparison with > the downstream driver [1]. > > There was a related patch [2] that removes I2C_M_NOSTART for all models, > but it seems abandoned and I do not have any other model for testing. > Therefore, this patch implements the least instrusive solution > and only removes I2C_M_NOSTART for MMS345L. Hmm, at this point I am inclined to pick up Andi's patch since it seems to work for you and him and it looks like Android drivers are not using I2C_M_NOSTART. I wonder if this was some quirk/big on the platform where it was originally developed. Any objections? Thanks. -- Dmitry
Re: [PATCH 1/3] Input: mms114 - use device_get_match_data
On Tue, Oct 08, 2019 at 02:44:26PM +0300, Andi Shyti wrote: > Hi Stephan, > > > device_get_match_data is available now, so we can replace the call > > to of_device_get_match_data and remove the FIXME comment. > > > > Signed-off-by: Stephan Gerhold > > Reviewed-by: Andi Shyti Applied, thank you. -- Dmitry
[PATCH v7] tpm_crb: fix fTPM on AMD Zen+ CPUs
From: Ivan Lazeev
Bug link: https://bugzilla.kernel.org/show_bug.cgi?id=195657
cmd/rsp buffers are expected to be in the same ACPI region.
For Zen+ CPUs BIOS's might report two different regions, some of
them also report region sizes inconsistent with values from TPM
registers.
Memory configuration on ASRock x470 ITX:
db0a-dc59efff : Reserved
dc57e000-dc57efff : MSFT0101:00
dc582000-dc582fff : MSFT0101:00
Work around the issue by storing ACPI regions declared for the
device in a fixed array and adding an array for pointers to
corresponding possibly allocated resources in crb_map_io function.
This data was previously held for a single resource
in struct crb_priv (iobase field) and local variable io_res in
crb_map_io function. ACPI resources array is used to find index of
corresponding region for each buffer and make the buffer size
consistent with region's length. Array of pointers to allocated
resources is used to map the region at most once.
Signed-off-by: Ivan Lazeev
---
Changes in v7:
- use terminator entry in iores_array
drivers/char/tpm/tpm_crb.c | 123 +++--
1 file changed, 90 insertions(+), 33 deletions(-)
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index e59f1f91d7f3..49d142721b11 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -22,6 +22,7 @@
#include "tpm.h"
#define ACPI_SIG_TPM2 "TPM2"
+#define TPM_CRB_MAX_RESOURCES 3
static const guid_t crb_acpi_start_guid =
GUID_INIT(0x6BBF6CAB, 0x5463, 0x4714,
@@ -91,7 +92,6 @@ enum crb_status {
struct crb_priv {
u32 sm;
const char *hid;
- void __iomem *iobase;
struct crb_regs_head __iomem *regs_h;
struct crb_regs_tail __iomem *regs_t;
u8 __iomem *cmd;
@@ -434,21 +434,27 @@ static const struct tpm_class_ops tpm_crb = {
static int crb_check_resource(struct acpi_resource *ares, void *data)
{
- struct resource *io_res = data;
+ struct resource *iores_array = data;
struct resource_win win;
struct resource *res = &(win.res);
+ int i;
if (acpi_dev_resource_memory(ares, res) ||
acpi_dev_resource_address_space(ares, &win)) {
- *io_res = *res;
- io_res->name = NULL;
+ for (i = 0; i < TPM_CRB_MAX_RESOURCES + 1; ++i) {
+ if (resource_type(iores_array + i) != IORESOURCE_MEM) {
+ iores_array[i] = *res;
+ iores_array[i].name = NULL;
+ break;
+ }
+ }
}
return 1;
}
-static void __iomem *crb_map_res(struct device *dev, struct crb_priv *priv,
-struct resource *io_res, u64 start, u32 size)
+static void __iomem *crb_map_res(struct device *dev, struct resource *iores,
+void __iomem **iobase_ptr, u64 start, u32 size)
{
struct resource new_res = {
.start = start,
@@ -460,10 +466,16 @@ static void __iomem *crb_map_res(struct device *dev,
struct crb_priv *priv,
if (start != new_res.start)
return (void __iomem *) ERR_PTR(-EINVAL);
- if (!resource_contains(io_res, &new_res))
+ if (!iores)
return devm_ioremap_resource(dev, &new_res);
- return priv->iobase + (new_res.start - io_res->start);
+ if (!*iobase_ptr) {
+ *iobase_ptr = devm_ioremap_resource(dev, iores);
+ if (IS_ERR(*iobase_ptr))
+ return *iobase_ptr;
+ }
+
+ return *iobase_ptr + (new_res.start - iores->start);
}
/*
@@ -490,9 +502,13 @@ static u64 crb_fixup_cmd_size(struct device *dev, struct
resource *io_res,
static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
struct acpi_table_tpm2 *buf)
{
- struct list_head resources;
- struct resource io_res;
+ struct list_head acpi_resource_list;
+ struct resource iores_array[TPM_CRB_MAX_RESOURCES + 1] = { {0} };
+ void __iomem *iobase_array[TPM_CRB_MAX_RESOURCES] = {0};
struct device *dev = &device->dev;
+ struct resource *iores;
+ void __iomem **iobase_ptr;
+ int i;
u32 pa_high, pa_low;
u64 cmd_pa;
u32 cmd_size;
@@ -501,21 +517,41 @@ static int crb_map_io(struct acpi_device *device, struct
crb_priv *priv,
u32 rsp_size;
int ret;
- INIT_LIST_HEAD(&resources);
- ret = acpi_dev_get_resources(device, &resources, crb_check_resource,
-&io_res);
+ INIT_LIST_HEAD(&acpi_resource_list);
+ ret = acpi_dev_get_resources(device, &acpi_resource_list,
+crb_check_resource, iores_array);
if (ret < 0)
return ret;
- acpi_dev_free_resource_list(&resources);
+ acpi_dev_fr
Re: [PATCH v8 0/3] HEVC/H.265 stateless support for V4L2 and Cedrus
Dne petek, 27. september 2019 ob 16:34:08 CEST je Paul Kocialkowski napisal(a): > HEVC/H.265 stateless support for V4L2 and Cedrus > > This is early support for HEVC/H.265 stateless decoding in V4L2, > including both definitions and driver support for the Cedrus VPU > driver, which concerns Allwinner devices. > > A specific pixel format is introduced for the HEVC slice format and > controls are provided to pass the bitstream metadata to the decoder. > Some bitstream extensions are intentionally not supported at this point. > > Since this is the first proposal for stateless HEVC/H.265 support in > V4L2, reviews and comments about the controls definitions are > particularly welcome. > > On the Cedrus side, the H.265 implementation covers frame pictures > with both uni-directional and bi-direction prediction modes (P/B > slices). Field pictures (interleaved), scaling lists and 10-bit output > are not supported at this point. Whole series is: Reviewed-by: Jernej Skrabec Hopefully this can be merged soon. Best regards, Jernej > > Changes since v7: > * Rebased on latest media tree; > * Fixed holes in structures for cacheline alignment; > * Added decode mode and start code controls > (only per-slice and no start code is supported at this point). > > Changes since v6: > * Rebased on latest media tree from Hans; > * Reordered some fields to avoid holes and multi-padding; > * Updated the documentation. > > Changes since v5: > * Rebased atop latest next media tree; > * Moved to flags instead of u8 fields; > * Added padding to ensure 64-bit alignment > (tested with GDB on 32 and 64-bit architectures); > * Reworked cedrus H.265 driver support a bit for flags; > * Split off codec-specific control validation and init; > * Added HEVC controls fields cleanup at std_validate to allow reliable > control comparison with memcmp; > * Fixed various misc reported mistakes. > > Changes since v4: > * Rebased atop latest H.254 series. > > Changes since v3: > * Updated commit messages; > * Updated CID base to avoid conflicts; > * Used cpu_to_le32 for packed le32 data; > * Fixed misc minor issues in the drive code; > * Made it clear in the docs that the API will evolve; > * Made the pixfmt private and split commits about it. > > Changes since v2: > * Moved headers to non-public API; > * Added H265 capability for A64 and H5; > * Moved docs to ext-ctrls-codec.rst; > * Mentionned sections of the spec in the docs; > * Added padding to control structures for 32-bit alignment; > * Made write function use void/size in bytes; > * Reduced the number of arguments to helpers when possible; > * Removed PHYS_OFFSET since we already set PFN_OFFSET; > * Added comments where suggested; > * Moved to timestamp for references instead of index; > * Fixed some style issues reported by checkpatch. > > Changes since v1: > * Added a H.265 capability to whitelist relevant platforms; > * Switched over to tags instead of buffer indices in the DPB > * Declared variable in their reduced scope as suggested; > * Added the H.265/HEVC spec to the biblio; > * Used in-doc references to the spec and the required APIs; > * Removed debugging leftovers. > > Cheers! > > Paul Kocialkowski (3): > media: v4l: Add definitions for HEVC stateless decoding > media: pixfmt: Document the HEVC slice pixel format > media: cedrus: Add HEVC/H.265 decoding support > > Documentation/media/uapi/v4l/biblio.rst | 9 + > .../media/uapi/v4l/ext-ctrls-codec.rst| 553 +++- > .../media/uapi/v4l/pixfmt-compressed.rst | 23 + > .../media/uapi/v4l/vidioc-queryctrl.rst | 18 + > .../media/videodev2.h.rst.exceptions | 3 + > drivers/media/v4l2-core/v4l2-ctrls.c | 108 ++- > drivers/media/v4l2-core/v4l2-ioctl.c | 1 + > drivers/staging/media/sunxi/cedrus/Makefile | 2 +- > drivers/staging/media/sunxi/cedrus/cedrus.c | 52 +- > drivers/staging/media/sunxi/cedrus/cedrus.h | 18 + > .../staging/media/sunxi/cedrus/cedrus_dec.c | 9 + > .../staging/media/sunxi/cedrus/cedrus_h265.c | 616 ++ > .../staging/media/sunxi/cedrus/cedrus_hw.c| 4 + > .../staging/media/sunxi/cedrus/cedrus_regs.h | 271 > .../staging/media/sunxi/cedrus/cedrus_video.c | 10 + > include/media/hevc-ctrls.h| 212 ++ > include/media/v4l2-ctrls.h| 7 + > 17 files changed, 1907 insertions(+), 9 deletions(-) > create mode 100644 drivers/staging/media/sunxi/cedrus/cedrus_h265.c > create mode 100644 include/media/hevc-ctrls.h
Re: [PATCH v2 linux-kselftest-test 3/3] kunit: update documentation to describe module-based build
On Tue, Oct 08, 2019 at 03:55:46PM +0100, Alan Maguire wrote: > Documentation should describe how to build kunit and tests as > modules. > > Signed-off-by: Alan Maguire > Signed-off-by: Knut Omang > > --- > Documentation/dev-tools/kunit/faq.rst | 3 ++- > Documentation/dev-tools/kunit/index.rst | 3 +++ > Documentation/dev-tools/kunit/usage.rst | 16 > 3 files changed, 21 insertions(+), 1 deletion(-) [...] > diff --git a/Documentation/dev-tools/kunit/usage.rst > b/Documentation/dev-tools/kunit/usage.rst > index c6e6963..fa0f03f 100644 > --- a/Documentation/dev-tools/kunit/usage.rst > +++ b/Documentation/dev-tools/kunit/usage.rst > @@ -539,6 +539,22 @@ Interspersed in the kernel logs you might see the > following: > > Congratulations, you just ran a KUnit test on the x86 architecture! > > +In a similar manner, kunit and kunit tests can also be built as modules, > +so if you wanted to run tests in this way you might add the following config > +options to your ``.config``: > + > +.. code-block:: none > + > +CONFIG_KUNIT=m > +CONFIG_KUNIT_EXAMPLE_TEST=m This doesn't appear to be properly tabbed. > +Once the kernel is built and installed, a simple > + > +.. code-block:: bash > + modprobe example-test > + > +...will run the tests. > + > Writing new tests for other architectures > - > > -- > 1.8.3.1 >
Re: "Shrink zones before removing memory" causes kernel panic with kpagecount
On 08.10.19 22:18, Qian Cai wrote: > The linux-next series "mm/memory_hotplug: Shrink zones before removing memory" > [1] causes a kernel panic while reading /proc/kpagecount after offlining a > memory section. It was reproduced on both x86 and powerpc. Reverted the whole > series fixed the problem. > > [1] https://lore.kernel.org/linux-mm/20191006085646.5768-1-da...@redhat.com/ > > # echo offline > /sys/devices/system/memory/memory124/state > # cat /proc/kpagecount > > [ 133.268032][ T8809] remove from free list 7c000 256 7d000 > [ 133.268134][ T8809] remove from free list 7c100 256 7d000 > [ 133.268153][ T8809] remove from free list 7c200 256 7d000 > [ 133.268182][ T8809] remove from free list 7c300 256 7d000 > [ 133.268212][ T8809] remove from free list 7c400 256 7d000 > [ 133.268241][ T8809] remove from free list 7c500 256 7d000 > [ 133.268260][ T8809] remove from free list 7c600 256 7d000 > [ 133.268289][ T8809] remove from free list 7c700 256 7d000 > [ 133.268329][ T8809] remove from free list 7c800 256 7d000 > [ 133.268359][ T8809] remove from free list 7c900 256 7d000 > [ 133.268399][ T8809] remove from free list 7ca00 256 7d000 > [ 133.268429][ T8809] remove from free list 7cb00 256 7d000 > [ 133.268458][ T8809] remove from free list 7cc00 256 7d000 > [ 133.268488][ T8809] remove from free list 7cd00 256 7d000 > [ 133.268517][ T8809] remove from free list 7ce00 256 7d000 > [ 133.268546][ T8809] remove from free list 7cf00 256 7d000 > [ 133.268580][ T8809] Offlined Pages 4096 > [ 144.038732][ T8944] BUG: Unable to handle kernel data access at > 0xfffe > [ 144.038769][ T8944] Faulting instruction address: 0xc0590c08 > [ 144.038794][ T8944] Oops: Kernel access of bad area, sig: 11 [#1] > [ 144.038807][ T8944] LE PAGE_SIZE=64K MMU=Radix MMU=Hash SMP NR_CPUS=256 > DEBUG_PAGEALLOC NUMA PowerNV > [ 144.038822][ T8944] Modules linked in: ip_tables x_tables xfs sd_mod bnx2x > mdio ahci libahci tg3 libata libphy firmware_class dm_mirror dm_region_hash > dm_log dm_mod > [ 144.038864][ T8944] CPU: 116 PID: 8944 Comm: cat Not tainted 5.4.0-rc2+ #6 > [ 144.038898][ T8944] NIP: c0590c08 LR: c0577330 CTR: > c05909d0 > [ 144.038945][ T8944] REGS: c00020196bd6fa30 TRAP: 0380 Not tainted > (5.4.0- > rc2+) > [ 144.038989][ T8944] MSR: 90009033 CR: > 48022428 XER: 2004 > [ 144.039028][ T8944] CFAR: c0590ad0 IRQMASK: 0 > [ 144.039028][ T8944] GPR00: c0577330 c00020196bd6fcc0 > c1122d00 > c0002009d3d4a880 > [ 144.039028][ T8944] GPR04: 7fffb687 0002 > fffe > c00c > [ 144.039028][ T8944] GPR08: 01f0 c00c01f0 > 0001 > c09413d0 > [ 144.039028][ T8944] GPR12: c05909d0 c000201fff677000 > > > [ 144.039028][ T8944] GPR16: 0002 7fffca34cfa8 > > > [ 144.039028][ T8944] GPR20: > c000 > c00020196bd6fdf0 > [ 144.039028][ T8944] GPR24: 7fffb687 07ff > > c0aa6c20 > [ 144.039028][ T8944] GPR28: 7fffb689 0008 > 0007c000 > 7fffb687 > [ 144.039240][ T8944] NIP [c0590c08] kpagecount_read+0x238/0x3f0 > [ 144.039263][ T8944] LR [c0577330] proc_reg_read+0x90/0x130 > [ 144.039274][ T8944] Call Trace: > [ 144.039304][ T8944] [c00020196bd6fd30] [c0577330] > proc_reg_read+0x90/0x130 > [ 144.039342][ T8944] [c00020196bd6fd60] [c04978bc] > __vfs_read+0x3c/0x70 > [ 144.039377][ T8944] [c00020196bd6fd80] [c049799c] > vfs_read+0xac/0x170 > [ 144.039423][ T8944] [c00020196bd6fdd0] [c0497dfc] > ksys_read+0x7c/0x140 > [ 144.039472][ T8944] [c00020196bd6fe20] [c000b378] > system_call+0x5c/0x68 > [ 144.039495][ T8944] Instruction dump: > [ 144.039513][ T8944] 4e800020 6000 3d22000d 3929c098 7bc83664 e8e9 > 7d274215 418200ac > [ 144.039540][ T8944] e9490008 38ca 714a0001 7cc9309e 2faa > e9490008 419e00fc > [ 144.039580][ T8944] ---[ end trace 96fb2ea2d503fda9 ]--- > [ 144.492072][ T8944] > [ 145.492172][ T8944] Kernel panic - not syncing: Fatal exception > Thanks, that's somewhat expected as I taint pages more aggressively. It's a pre-existing issue. You can trigger the exact same BUG by 1. Hotplugging a DIMM but not onlining it 2. cat /proc/kpagecount The right fix is to add a pgn_to_online_page() to the PFN walker and skip all PFNs that are not online. This was already discussed in the context of ZONE_DEVICE and I am yet waiting for a fix. I can prepare and send a fix for that PFN walker tomorrow. -- Thanks, David / dhildenb
Re: pivot_root(".", ".") and the fchdir() dance
On 10/8/19 9:40 PM, Eric W. Biederman wrote:
> "Michael Kerrisk (man-pages)" writes:
>
>> Hello Eric,
>>
> Creating of a mount namespace in a user namespace automatically does
> 'mount("", "/", MS_SLAVE | MS_REC, NULL);' if the starting mount
> namespace was not created in that user namespace. AKA creating
> a mount namespace in a user namespace does the unshare for you.
Oh -- I had forgotten that detail. But it is documented
(by you, I think) in mount_namespaces(7):
* A mount namespace has an owner user namespace. A
mount namespace whose owner user namespace is differ‐
ent from the owner user namespace of its parent mount
namespace is considered a less privileged mount names‐
pace.
* When creating a less privileged mount namespace,
shared mounts are reduced to slave mounts. (Shared
and slave mounts are discussed below.) This ensures
that mappings performed in less privileged mount
namespaces will not propagate to more privileged mount
namespaces.
There's one point that description that troubles me. There is a
reference to "parent mount namespace", but as I understand things
there is no parental relationship among mount namespaces instances
(or am I wrong?). Should that wording not be rather something
like "the mount namespace of the process that created this mount
namespace"?
>>>
>>> How about "the mount namespace this mount namespace started as a copy of"
>>>
>>> You are absolutely correct there is no relationship between mount
>>> namespaces. There is just the propagation tree between mounts. (Which
>>> acts similarly to a parent/child relationship but is not at all the same
>>> thing).
>>
>> Thanks. I made the text as follows:
>>
>>* Each mount namespace has an owner user namespace. As noted
>> above, when a new mount namespace is created, it inherits a
>> copy of the mount points from the mount namespace of the
>> process that created the new mount namespace. If the two mount
>> namespaces are owned by different user namespaces, then the new
>> mount namespace is considered less privileged.
>
> I hate to nitpick,
I love it when you nitpick. Thanks for your attention to the details
of my wording.
> but I am going to say that when I read the text above
> the phrase "mount namespace of the process that created the new mount
> namespace" feels wrong.
>
> Either you use unshare(2) and the mount namespace of the process that
> created the mount namespace changes.
>
> Or you use clone(2) and you could argue it is the new child that created
> the mount namespace.
>
> Having a different mount namespace at the end of the creation operation
> feels like it makes your phrase confusing about what the starting
> mount namespace is. I hate to use references that are ambiguous when
> things are changing.
>
> I agree that the term parent is also wrong.
I see what you mean. My wording is imprecise.
So, I tweaked text earlier in the page so that it now reads
as follows:
A new mount namespace is created using either clone(2) or
unshare(2) with the CLONE_NEWNS flag. When a new mount namespace
is created, its mount point list is initialized as follows:
* If the namespace is created using clone(2), the mount point
list of the child's namespace is a copy of the mount point list
in the parent's namespace.
* If the namespace is created using unshare(2), the mount point
list of the new namespace is a copy of the mount point list in
the caller's previous mount namespace.
And then I tweaked the text that we are currently discussing to read:
* Each mount namespace has an owner user namespace. As explained
above, when a new mount namespace is created, its mount point
list is initialized as a copy of the mount point list of
another mount namespace. If the new namespaces and the names‐
pace from which the mount point list was copied are owned by
different user namespaces, then the new mount namespace is con‐
sidered less privileged.
How does this look to you now?
Thanks,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
[PATCH] ARC: mm: remove __ARCH_USE_5LEVEL_HACK
Add the intermediate p4d accessors to make it 5 level compliant. Thi sis non-functional change anyways since ARC has software page walker with 2 lookup levels (pgd -> pte) Signed-off-by: Vineet Gupta --- arch/arc/include/asm/pgtable.h | 1 - arch/arc/mm/fault.c| 10 -- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/arch/arc/include/asm/pgtable.h b/arch/arc/include/asm/pgtable.h index 976b5931372e..902d45428cea 100644 --- a/arch/arc/include/asm/pgtable.h +++ b/arch/arc/include/asm/pgtable.h @@ -33,7 +33,6 @@ #define _ASM_ARC_PGTABLE_H #include -#define __ARCH_USE_5LEVEL_HACK #include #include #include/* to propagate CONFIG_ARC_MMU_VER */ diff --git a/arch/arc/mm/fault.c b/arch/arc/mm/fault.c index 3861543b66a0..fb86bc3e9b35 100644 --- a/arch/arc/mm/fault.c +++ b/arch/arc/mm/fault.c @@ -30,6 +30,7 @@ noinline static int handle_kernel_vaddr_fault(unsigned long address) * with the 'reference' page table. */ pgd_t *pgd, *pgd_k; + p4d_t *p4d, *p4d_k; pud_t *pud, *pud_k; pmd_t *pmd, *pmd_k; @@ -39,8 +40,13 @@ noinline static int handle_kernel_vaddr_fault(unsigned long address) if (!pgd_present(*pgd_k)) goto bad_area; - pud = pud_offset(pgd, address); - pud_k = pud_offset(pgd_k, address); + p4d = p4d_offset(pgd, address); + p4d_k = p4d_offset(pgd_k, address); + if (!p4d_present(*p4d_k)) + goto bad_area; + + pud = pud_offset(p4d, address); + pud_k = pud_offset(p4d_k, address); if (!pud_present(*pud_k)) goto bad_area; -- 2.20.1
Re: [PATCH] PCI/IOV: update num_VFs earlier
On Thu, Oct 03, 2019 at 05:10:07PM -0500, Bjorn Helgaas wrote:
> On Thu, Oct 03, 2019 at 11:04:45AM +0200, CREGUT Pierre IMT/OLN wrote:
> > ...
> > NIC drivers send netlink events when their state change, but it is
> > the core that changes the value of num_vfs. So I would think it is
> > the core responsibility to make sure the exposed value makes sense
> > and it would be better to ignore the details of the driver
> > implementation.
>
> Yes, I think you're right. And I like your previous suggestion of
> just locking the device in the reader. I'm not enough of a sysfs
> expert to know if there's a good reason to avoid a lock there. Does
> the following look reasonable to you?
I applied the patch below to pci/virtualization for v5.5, thanks for
your great patience!
> commit 0940fc95da45
> Author: Pierre Crégut
> Date: Wed Sep 11 09:27:36 2019 +0200
>
> PCI/IOV: Serialize sysfs sriov_numvfs reads vs writes
>
> When sriov_numvfs is being updated, drivers may notify about new devices
> before they are reflected in sriov->num_VFs, so concurrent sysfs reads
> previously returned stale values.
>
> Serialize the sysfs read vs the write so the read returns the correct
> num_VFs value.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=202991
> Link:
> https://lore.kernel.org/r/20190911072736.32091-1-pierre.cre...@orange.com
> Signed-off-by: Pierre Crégut
> Signed-off-by: Bjorn Helgaas
>
> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index b3f972e8cfed..e77562aabbae 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -254,8 +254,14 @@ static ssize_t sriov_numvfs_show(struct device *dev,
>char *buf)
> {
> struct pci_dev *pdev = to_pci_dev(dev);
> + u16 num_vfs;
> +
> + /* Serialize vs sriov_numvfs_store() so readers see valid num_VFs */
> + device_lock(&pdev->dev);
> + num_vfs = pdev->sriov->num_VFs;
> + device_lock(&pdev->dev);
>
> - return sprintf(buf, "%u\n", pdev->sriov->num_VFs);
> + return sprintf(buf, "%u\n", num_vfs);
> }
>
> /*
Re: [PATCH v2 linux-kselftest-test 1/3] kunit: allow kunit tests to be loaded as a module
On Tue, Oct 08, 2019 at 03:55:44PM +0100, Alan Maguire wrote:
> as tests are added to kunit, it will become less feasible to execute
> all built tests together. By supporting modular tests we provide
> a simple way to do selective execution on a running system; specifying
>
> CONFIG_KUNIT=y
> CONFIG_KUNIT_EXAMPLE_TEST=m
>
> ...means we can simply "insmod example-test.ko" to run the tests.
>
> To achieve this we need to
>
> o export the required symbols in kunit
> o support a new way of declaring test suites. Because a module cannot
> do multiple late_initcall()s, we provide a kunit_test_suites() macro
> to declare multiple suites within the same module at once.
>
> Signed-off-by: Alan Maguire
> Signed-off-by: Knut Omang
>
> ---
> include/kunit/test.h | 30 +++---
> kernel/sysctl-test.c | 6 +-
> lib/Kconfig.debug | 4 ++--
> lib/kunit/Kconfig | 4 ++--
> lib/kunit/assert.c | 8
> lib/kunit/example-test.c | 6 +-
> lib/kunit/string-stream-test.c | 9 +++--
> lib/kunit/string-stream.c | 7 +++
> lib/kunit/test-test.c | 8 ++--
> lib/kunit/test.c | 8
> lib/kunit/try-catch.c | 8 ++--
> 11 files changed, 79 insertions(+), 19 deletions(-)
>
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index dba4830..9fc6c1b 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -12,6 +12,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -204,24 +205,39 @@ struct kunit {
> * Registers @suite with the test framework. See &struct kunit_suite for
> * more information.
> *
> - * NOTE: Currently KUnit tests are all run as late_initcalls; this means
> + * When builtin, KUnit tests are all run as late_initcalls; this means
> * that they cannot test anything where tests must run at a different init
> * phase. One significant restriction resulting from this is that KUnit
> * cannot reliably test anything that is initialize in the late_init phase;
> * another is that KUnit is useless to test things that need to be run in
> * an earlier init phase.
> *
> + * An alternative is to build the tests as a module. Because modules
> + * do not support multiple late_initcall()s, we need to initialize an
> + * array of suites for a module.
> + *
> * TODO(brendanhigg...@google.com): Don't run all KUnit tests as
> * late_initcalls. I have some future work planned to dispatch all KUnit
> * tests from the same place, and at the very least to do so after
> * everything else is definitely initialized.
> */
> -#define kunit_test_suite(suite)
>\
> - static int kunit_suite_init##suite(void) \
> - { \
> - return kunit_run_tests(&suite);\
> - } \
> - late_initcall(kunit_suite_init##suite)
> +#define kunit_test_suites(...)
> \
> + static struct kunit_suite *suites[] = { __VA_ARGS__, NULL}; \
> + static int kunit_test_suites_init(void) \
> + { \
> + unsigned int i; \
> + for (i = 0; suites[i] != NULL; i++) \
> + kunit_run_tests(suites[i]); \
> + return 0; \
> + } \
> + late_initcall(kunit_test_suites_init); \
> + static void __exit kunit_test_suites_exit(void) \
> + { \
> + return; \
> + } \
> + module_exit(kunit_test_suites_exit)
> +
> +#define kunit_test_suite(suite) kunit_test_suites(suite)
I think it is fine to just rename this kunit_test_suites.
> /*
> * Like kunit_alloc_resource() below, but returns the struct kunit_resource
> diff --git a/kernel/sysctl-test.c b/kernel/sysctl-test.c
> index 2a63241..15161c5 100644
> --- a/kernel/sysctl-test.c
> +++ b/kernel/sysctl-test.c
> @@ -389,4 +389,8 @@ static void
> sysctl_test_api_dointvec_write_single_greater_int_max(
> .test_cases = sysctl_test_cases,
> };
>
> -kunit_test_suite(sysctl_test_suite);
> +kunit_test_suite(&sysctl_test_suite);
> +
> +#ifdef MODULE
> +MODULE_LICENSE("GPL");
> +#endif /* MODULE */
Here and elsewhere: the "ifdef/endif MODULE" should not be necessary.
> diff --git a/lib
Re: [PATCH v11 07/22] riscv: mm: Add p?d_leaf() definitions
On Mon, 7 Oct 2019, Steven Price wrote:
> walk_page_range() is going to be allowed to walk page tables other than
> those of user space. For this it needs to know when it has reached a
> 'leaf' entry in the page tables. This information is provided by the
> p?d_leaf() functions/macros.
>
> For riscv a page is a leaf page when it has a read, write or execute bit
> set on it.
>
> CC: Palmer Dabbelt
> CC: Albert Ou
> CC: linux-ri...@lists.infradead.org
> Signed-off-by: Steven Price
Acked-by: Paul Walmsley # for arch/riscv
Alex has a good point, but probably the right thing to do is to replace
the contents of the arch/riscv/mm/hugetlbpage.c p{u,m}d_huge() functions
with calls to Steven's new static inline functions.
- Paul
Re: [PATCH] PCI: Enhance the ACS quirk for Cavium devices
On Tue, Oct 08, 2019 at 08:25:23AM +, Robert Richter wrote:
> On 04.10.19 14:48:13, Bjorn Helgaas wrote:
> > commit 37b22fbfec2d
> > Author: George Cherian
> > Date: Thu Sep 19 02:43:34 2019 +
> >
> > PCI: Apply Cavium ACS quirk to CN99xx and CN11xxx Root Ports
> >
> > Add an array of Cavium Root Port device IDs and apply the quirk only to
> > the
> > listed devices.
> >
> > Instead of applying the quirk to all Root Ports where
> > "(dev->device & 0xf800) == 0xa000", apply it only to CN88xx 0xa180 and
> > 0xa170 Root Ports.
>
> No, this can't be removed. It is a match all for all CN8xxx variants
> (note the 3 'x', all TX1 cores). So all device ids from 0xa000 to
> 0xa7FF are affected here and need the quirk.
OK, I'll drop the patch and wait for a new one. Maybe what was needed
was to keep the "(dev->device & 0xf800) == 0xa000" part and add the
pci_quirk_cavium_acs_ids[] array in addition?
> > Also apply the quirk to CN99xx (0xaf84) and CN11xxx (0xb884) Root Ports.
>
> I thought the quirk is CN8xxx specific, but I could be wrong here.
>
> -Robert
>
> >
> > Link:
> > https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_r_20190919024319.GA8792-40dc5-2Deodlnx05.marvell.com&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=8vKOpC26NZGzQPAMiIlimxyEGCRSJiq-j8yyjPJ6VZ4&m=Vmml-rx3t63ZbbXZ0XaESAM9yAlexE29R-giTbcj4Qk&s=57jKIj8BAydbLpftLt5Ssva7vD6GuoCaIpjTi-sB5kU&e=
> >
> > Fixes: f2ddaf8dfd4a ("PCI: Apply Cavium ThunderX ACS quirk to more Root
> > Ports")
> > Fixes: b404bcfbf035 ("PCI: Add ACS quirk for all Cavium devices")
> > Signed-off-by: George Cherian
> > Signed-off-by: Bjorn Helgaas
> > Cc: sta...@vger.kernel.org # v4.12+
> >
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > index 320255e5e8f8..4e5048cb5ec6 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -4311,17 +4311,24 @@ static int pci_quirk_amd_sb_acs(struct pci_dev
> > *dev, u16 acs_flags)
> > #endif
> > }
> >
> > +static const u16 pci_quirk_cavium_acs_ids[] = {
> > + 0xa180, 0xa170, /* CN88xx family of devices */
> > + 0xaf84, /* CN99xx family of devices */
> > + 0xb884, /* CN11xxx family of devices */
> > +};
> > +
> > static bool pci_quirk_cavium_acs_match(struct pci_dev *dev)
> > {
> > - /*
> > -* Effectively selects all downstream ports for whole ThunderX 1
> > -* family by 0xf800 mask (which represents 8 SoCs), while the lower
> > -* bits of device ID are used to indicate which subdevice is used
> > -* within the SoC.
> > -*/
> > - return (pci_is_pcie(dev) &&
> > - (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) &&
> > - ((dev->device & 0xf800) == 0xa000));
> > + int i;
> > +
> > + if (!pci_is_pcie(dev) || pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT)
> > + return false;
> > +
> > + for (i = 0; i < ARRAY_SIZE(pci_quirk_cavium_acs_ids); i++)
> > + if (pci_quirk_cavium_acs_ids[i] == dev->device)
> > + return true;
> > +
> > + return false;
> > }
> >
> > static int pci_quirk_cavium_acs(struct pci_dev *dev, u16 acs_flags)
[PATCH v2] arm64: lse: fix LSE atomics with LLVM's integrated assembler
Unlike gcc, clang considers each inline assembly block to be independent
and therefore, when using the integrated assembler for inline assembly,
any preambles that enable features must be repeated in each block.
This change defines __LSE_PREAMBLE and adds it to each inline assembly
block that has LSE instructions, which allows them to be compiled also
with clang's assembler.
Link: https://github.com/ClangBuiltLinux/linux/issues/671
Signed-off-by: Sami Tolvanen
---
v2:
- Add a preamble to inline assembly blocks that use LSE instead
of allowing the compiler to emit LSE instructions everywhere.
---
arch/arm64/include/asm/atomic_lse.h | 19 +++
arch/arm64/include/asm/lse.h| 6 +++---
2 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/include/asm/atomic_lse.h
b/arch/arm64/include/asm/atomic_lse.h
index c6bd87d2915b..3ee600043042 100644
--- a/arch/arm64/include/asm/atomic_lse.h
+++ b/arch/arm64/include/asm/atomic_lse.h
@@ -14,6 +14,7 @@
static inline void __lse_atomic_##op(int i, atomic_t *v)
\
{ \
asm volatile( \
+ __LSE_PREAMBLE \
" " #asm_op " %w[i], %[v]\n" \
: [i] "+r" (i), [v] "+Q" (v->counter) \
: "r" (v)); \
@@ -30,6 +31,7 @@ ATOMIC_OP(add, stadd)
static inline int __lse_atomic_fetch_##op##name(int i, atomic_t *v)\
{ \
asm volatile( \
+ __LSE_PREAMBLE \
" " #asm_op #mb " %w[i], %w[i], %[v]" \
: [i] "+r" (i), [v] "+Q" (v->counter) \
: "r" (v) \
@@ -58,6 +60,7 @@ static inline int __lse_atomic_add_return##name(int i,
atomic_t *v) \
u32 tmp;\
\
asm volatile( \
+ __LSE_PREAMBLE \
" ldadd" #mb "%w[i], %w[tmp], %[v]\n" \
" add %w[i], %w[i], %w[tmp]" \
: [i] "+r" (i), [v] "+Q" (v->counter), [tmp] "=&r" (tmp)\
@@ -77,6 +80,7 @@ ATOMIC_OP_ADD_RETURN(, al, "memory")
static inline void __lse_atomic_and(int i, atomic_t *v)
{
asm volatile(
+ __LSE_PREAMBLE
" mvn %w[i], %w[i]\n"
" stclr %w[i], %[v]"
: [i] "+&r" (i), [v] "+Q" (v->counter)
@@ -87,6 +91,7 @@ static inline void __lse_atomic_and(int i, atomic_t *v)
static inline int __lse_atomic_fetch_and##name(int i, atomic_t *v) \
{ \
asm volatile( \
+ __LSE_PREAMBLE \
" mvn %w[i], %w[i]\n" \
" ldclr" #mb "%w[i], %w[i], %[v]" \
: [i] "+&r" (i), [v] "+Q" (v->counter) \
@@ -106,6 +111,7 @@ ATOMIC_FETCH_OP_AND(, al, "memory")
static inline void __lse_atomic_sub(int i, atomic_t *v)
{
asm volatile(
+ __LSE_PREAMBLE
" neg %w[i], %w[i]\n"
" stadd %w[i], %[v]"
: [i] "+&r" (i), [v] "+Q" (v->counter)
@@ -118,6 +124,7 @@ static inline int __lse_atomic_sub_return##name(int i,
atomic_t *v) \
u32 tmp;\
\
asm volatile( \
+ __LSE_PREAMBLE \
" neg %w[i], %w[i]\n" \
" ldadd" #mb "%w[i], %w[tmp], %[v]\n" \
" add %w[i], %w[i], %w[tmp]" \
@@ -139,6 +146,7 @@ ATOMIC_OP_SUB_RETURN(, al, "memory")
static inline int __lse_atomic_fetch_sub##name(int i, atomic_t *v) \
{ \
asm volatile( \
+ __LSE_PREAMBLE \
" neg %w[i], %w[i]\n" \
" ldadd" #mb "%w[i], %w[i], %[v]" \
Re: [PATCH 2/2] media: meson: vdec: add H.264 decoding support
On Tue, Oct 8, 2019 at 10:44 PM Nicolas Dufresne wrote:
>
> Le mardi 08 octobre 2019 à 16:27 -0400, Nicolas Dufresne a écrit :
> > Le lundi 07 octobre 2019 à 16:59 +0200, Maxime Jourdan a écrit :
> > > Add support for the H264 compressed format (V4L2_PIX_FMT_H264).
> > >
> > > Signed-off-by: Maxime Jourdan
> > > ---
> > > drivers/staging/media/meson/vdec/Makefile | 2 +-
> > > drivers/staging/media/meson/vdec/codec_h264.c | 482 ++
> > > drivers/staging/media/meson/vdec/codec_h264.h | 14 +
> > > .../staging/media/meson/vdec/vdec_platform.c | 37 ++
> > > 4 files changed, 534 insertions(+), 1 deletion(-)
> > > create mode 100644 drivers/staging/media/meson/vdec/codec_h264.c
> > > create mode 100644 drivers/staging/media/meson/vdec/codec_h264.h
> > >
> > > diff --git a/drivers/staging/media/meson/vdec/Makefile
> > > b/drivers/staging/media/meson/vdec/Makefile
> > > index 6bea129084b7..711d990c760e 100644
> > > --- a/drivers/staging/media/meson/vdec/Makefile
> > > +++ b/drivers/staging/media/meson/vdec/Makefile
> > > @@ -3,6 +3,6 @@
> > >
> > > meson-vdec-objs = esparser.o vdec.o vdec_helpers.o vdec_platform.o
> > > meson-vdec-objs += vdec_1.o
> > > -meson-vdec-objs += codec_mpeg12.o
> > > +meson-vdec-objs += codec_mpeg12.o codec_h264.o
> > >
> > > obj-$(CONFIG_VIDEO_MESON_VDEC) += meson-vdec.o
> > > diff --git a/drivers/staging/media/meson/vdec/codec_h264.c
> > > b/drivers/staging/media/meson/vdec/codec_h264.c
> > > new file mode 100644
> > > index ..4528a6a01c3d
> > > --- /dev/null
> > > +++ b/drivers/staging/media/meson/vdec/codec_h264.c
> > > @@ -0,0 +1,482 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright (C) 2019 BayLibre, SAS
> > > + * Author: Maxime Jourdan
> > > + */
> > > +
> > > +#include
> > > +#include
> > > +
> > > +#include "vdec_helpers.h"
> > > +#include "dos_regs.h"
> > > +
> > > +#define SIZE_EXT_FW(20 * SZ_1K)
> > > +#define SIZE_WORKSPACE 0x1ee000
> > > +#define SIZE_SEI (8 * SZ_1K)
> > > +
> > > +/*
> > > + * Offset added by the firmware which must be substracted
> > > + * from the workspace phyaddr
> > > + */
> > > +#define WORKSPACE_BUF_OFFSET 0x100
> > > +
> > > +/* ISR status */
> > > +#define CMD_MASK GENMASK(7, 0)
> > > +#define CMD_SRC_CHANGE 1
> > > +#define CMD_FRAMES_READY 2
> > > +#define CMD_FATAL_ERROR6
> > > +#define CMD_BAD_WIDTH 7
> > > +#define CMD_BAD_HEIGHT 8
> > > +
> > > +#define SEI_DATA_READY BIT(15)
> > > +
> > > +/* Picture type */
> > > +#define PIC_TOP_BOT5
> > > +#define PIC_BOT_TOP6
> > > +
> > > +/* Size of Motion Vector per macroblock */
> > > +#define MB_MV_SIZE 96
> > > +
> > > +/* Frame status data */
> > > +#define PIC_STRUCT_BIT 5
> > > +#define PIC_STRUCT_MASKGENMASK(2, 0)
> > > +#define BUF_IDX_MASK GENMASK(4, 0)
> > > +#define ERROR_FLAG BIT(9)
> > > +#define OFFSET_BIT 16
> > > +#define OFFSET_MASKGENMASK(15, 0)
> > > +
> > > +/* Bitstream parsed data */
> > > +#define MB_TOTAL_BIT 8
> > > +#define MB_TOTAL_MASK GENMASK(15, 0)
> > > +#define MB_WIDTH_MASK GENMASK(7, 0)
> > > +#define MAX_REF_BIT24
> > > +#define MAX_REF_MASK GENMASK(6, 0)
> > > +#define AR_IDC_BIT 16
> > > +#define AR_IDC_MASKGENMASK(7, 0)
> > > +#define AR_PRESENT_FLAGBIT(0)
> > > +#define AR_EXTEND 0xff
> > > +
> > > +/*
> > > + * Buffer to send to the ESPARSER to signal End Of Stream for H.264.
> > > + * This is a 16x16 encoded picture that will trigger drain firmware-side.
> > > + * There is no known alternative.
> > > + */
> > > +static const u8 eos_sequence[SZ_4K] = {
> > > + 0x00, 0x00, 0x00, 0x01, 0x06, 0x05, 0xff, 0xe4, 0xdc, 0x45, 0xe9,
> > > 0xbd,
> > > + 0xe6, 0xd9, 0x48, 0xb7, 0x96, 0x2c, 0xd8, 0x20, 0xd9, 0x23, 0xee,
> > > 0xef,
> > > + 0x78, 0x32, 0x36, 0x34, 0x20, 0x2d, 0x20, 0x63, 0x6f, 0x72, 0x65,
> > > 0x20,
> > > + 0x36, 0x37, 0x20, 0x72, 0x31, 0x31, 0x33, 0x30, 0x20, 0x38, 0x34,
> > > 0x37,
> > > + 0x35, 0x39, 0x37, 0x37, 0x20, 0x2d, 0x20, 0x48, 0x2e, 0x32, 0x36,
> > > 0x34,
> > > + 0x2f, 0x4d, 0x50, 0x45, 0x47, 0x2d, 0x34, 0x20, 0x41, 0x56, 0x43,
> > > 0x20,
> > > + 0x63, 0x6f, 0x64, 0x65, 0x63, 0x20, 0x2d, 0x20, 0x43, 0x6f, 0x70,
> > > 0x79,
> > > + 0x6c, 0x65, 0x66, 0x74, 0x20, 0x32, 0x30, 0x30, 0x33, 0x2d, 0x32,
> > > 0x30,
> > > + 0x30, 0x39, 0x20, 0x2d, 0x20, 0x68, 0x74, 0x74, 0x70, 0x3a, 0x2f,
> > > 0x2f,
> > > + 0x77, 0x77, 0x77, 0x2e, 0x76, 0x69, 0x64, 0x65, 0x6f, 0x6c, 0x61,
> > > 0x6e,
> > > + 0x2e, 0x6f, 0x72, 0x67, 0x2f, 0x78, 0x32, 0x36, 0x34, 0x2e, 0x68,
> > > 0x74,
> > > + 0x6d, 0x6c, 0x20, 0x2d, 0x20, 0x6f, 0x70, 0x74, 0x69, 0x6f, 0x6e,
> > > 0x73,
> > > + 0x3a, 0x20, 0x63, 0x61, 0x62, 0x61, 0x63, 0x3d, 0x31, 0x20, 0x72,
> > > 0x65,
> > > + 0x66, 0x3d, 0x31, 0x20, 0x64, 0x65, 0x62, 0x6c, 0x6f, 0x63, 0x6b,
> > > 0x3d,
> > > + 0x31, 0x3a, 0x30, 0x3a, 0x30, 0x2
Potential uninitialized variables in cfg80211
Hi All: net/wireless/chan.c: Inside function cfg80211_chandef_compatible(), variable "c1_pri40", " c2_pri40", "c1_pri80" and "c2_pri80" could be uninitialized if chandef_primary_freqs() fails. However, they are used later in the if statement to decide the control flow, which is potentially unsafe. The patch is hard since we do not know the correct value to initialize them. -- Kind Regards, Yizhuo Zhai Computer Science, Graduate Student University of California, Riverside
Re: [PATCH v3 1/2] PCI: PCIe: ASPM: Introduce pcie_aspm_enabled()
On Tue, Oct 08, 2019 at 11:27:51AM +0200, Rafael J. Wysocki wrote:
> On Tue, Oct 8, 2019 at 12:34 AM Bjorn Helgaas wrote:
> > On Thu, Aug 08, 2019 at 11:55:07PM +0200, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki
> > >
> > > Add a function checking whether or not PCIe ASPM has been enabled for
> > > a given device.
> > >
> > > It will be used by the NVMe driver to decide how to handle the
> > > device during system suspend.
> > >
> > > Signed-off-by: Rafael J. Wysocki
> > > ---
> > >
> > > v2 -> v3:
> > > * Make the new function return bool.
> > > * Change its name back to pcie_aspm_enabled().
> > > * Fix kerneldoc comment formatting.
> > >
> > > -> v2:
> > > * Move the PCI/PCIe ASPM changes to a separate patch.
> > > * Add the _mask suffix to the new function name.
> > > * Add EXPORT_SYMBOL_GPL() to the new function.
> > > * Avoid adding an unnecessary blank line.
> > >
> > > ---
> > > drivers/pci/pcie/aspm.c | 20
> > > include/linux/pci.h |3 +++
> > > 2 files changed, 23 insertions(+)
> > >
> > > Index: linux-pm/drivers/pci/pcie/aspm.c
> > > ===
> > > --- linux-pm.orig/drivers/pci/pcie/aspm.c
> > > +++ linux-pm/drivers/pci/pcie/aspm.c
> > > @@ -1170,6 +1170,26 @@ static int pcie_aspm_get_policy(char *bu
> > > module_param_call(policy, pcie_aspm_set_policy, pcie_aspm_get_policy,
> > > NULL, 0644);
> > >
> > > +/**
> > > + * pcie_aspm_enabled - Check if PCIe ASPM has been enabled for a device.
> > > + * @pci_device: Target device.
> > > + */
> > > +bool pcie_aspm_enabled(struct pci_dev *pci_device)
> > > +{
> > > + struct pci_dev *bridge = pci_upstream_bridge(pci_device);
> > > + bool ret;
> > > +
> > > + if (!bridge)
> > > + return false;
> > > +
> > > + mutex_lock(&aspm_lock);
> > > + ret = bridge->link_state ? !!bridge->link_state->aspm_enabled :
> > > false;
> > > + mutex_unlock(&aspm_lock);
> >
> > Why do we need to acquire aspm_lock here? We aren't modifying
> > anything, and I don't think we're preventing a race. If this races
> > with another thread that changes aspm_enabled, we'll return either the
> > old state or the new one, and I think that's still the case even if we
> > don't acquire aspm_lock.
>
> Well, if we can guarantee that pci_remove_bus_device() will never be
> called in parallel with this helper, then I agree, but can we
> guarantee that?
Hmm, yeah, I guess that's the question. It's not a race with another
thread changing aspm_enabled; the potential race is with another
thread removing the last child of "bridge", which will free the
link_state and set bridge->link_state = NULL.
I think it should be safe to call device-related PCI interfaces if
you're holding a reference to the device, e.g., from a driver bound to
the device or a sysfs accessor. Since we call pcie_aspm_enabled(dev)
from a driver bound to "dev", another thread should not be able to
remove "dev" while we're using it.
I know that's a little hand-wavey, but if it weren't true, I think
we'd have a lot more locking sprinkled everywhere in the PCI core than
we do.
This has implications for Heiner's ASPM sysfs patches because we're
currently doing this in sysfs accessors:
static ssize_t aspm_attr_show_common(struct device *dev, ...)
{
...
link = pcie_aspm_get_link(pdev);
mutex_lock(&aspm_lock);
enabled = link->aspm_enabled & state;
mutex_unlock(&aspm_lock);
...
}
I assume sysfs must be holding a reference that guarantees "dev" is
valid througout this code, and therefore we should not need to hold
aspm_lock.
Bjorn
Re: [PATCH v2] mfd: intel-lpss: use devm_ioremap_uc for MMIO
I think that's a fair point. Sorry for not noticing it earlier. > On Tue, Oct 8, 2019, 6:12 PM Andy Shevchenko < > andriy.shevche...@linux.intel.com> wrote: > > Maybe we can even split this to two patches? I assume splitting means one to add devm_ioremap_uc and one to use it for intel-lpss-pci. Tuowen
Re: KASAN: use-after-free Read in tipc_udp_nl_dump_remoteip
syzbot has bisected this bug to:
commit 057af70713445fad2459aa348c9c2c4ecf7db938
Author: Jiri Pirko
Date: Sat Oct 5 18:04:39 2019 +
net: tipc: have genetlink code to parse the attrs during dumpit
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=11675620e0
start commit: 056ddc38 Merge branch 'stmmac-next'
git tree: net-next
final crash:https://syzkaller.appspot.com/x/report.txt?x=13675620e0
console output: https://syzkaller.appspot.com/x/log.txt?x=15675620e0
kernel config: https://syzkaller.appspot.com/x/.config?x=d9be300620399522
dashboard link: https://syzkaller.appspot.com/bug?extid=dbe02e13bcce52bcf182
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=137ecdfb60
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=15dd0d0b60
Reported-by: syzbot+dbe02e13bcce52bcf...@syzkaller.appspotmail.com
Fixes: 057af7071344 ("net: tipc: have genetlink code to parse the attrs
during dumpit")
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
Re: [PATCH] arm64: lse: fix LSE atomics with LLVM's integrated assembler
On Tue, 8 Oct 2019 at 18:19, Robin Murphy wrote:
>
> On 08/10/2019 16:22, Sami Tolvanen wrote:
> > On Mon, Oct 7, 2019 at 2:46 PM 'Nick Desaulniers' via Clang Built
> > Linux wrote:
> >> I'm worried that one of these might lower to LSE atomics without
> >> ALTERNATIVE guards by blanketing all C code with `-march=armv8-a+lse`.
> >
> > True, that's a valid concern. I think adding the directive to each
> > assembly block is the way forward then, assuming the maintainers are
> > fine with that.
>
> It's definitely a valid concern in principle, but in practice note that
> lse.h ends up included in ~99% of C files, so the extension is enabled
> more or less everywhere already.
>
lse.h currently does
__asm__(".arch_extensionlse");
which instructs the assembler to permit the use of LSE opcodes, but it
does not instruct the compiler to emit them, so this is not quite the
same thing.
Re: [PATCH v2 linux-kselftest-test 0/3] kunit: support building core/tests as modules
On Tue, Oct 08, 2019 at 03:55:43PM +0100, Alan Maguire wrote: > The current kunit execution model is to provide base kunit functionality > and tests built-in to the kernel. The aim of this series is to allow > building kunit itself and tests as modules. This in turn allows a Cool! I had plans for supporting this eventually, so I am more than happy to accept support for this! > simple form of selective execution; load the module you wish to test. > In doing so, kunit itself (if also built as a module) will be loaded as > an implicit dependency. Seems like a reasonable initial approach. I had some plans for a centralized test executor, but I don't think that this should be a problem. > Because this requires a core API modification - if a module delivers > multiple suites, they must be declared with the kunit_test_suites() > macro - we're proposing this patch as a candidate to be applied to the > test tree before too many kunit consumers appear. We attempt to deal > with existing consumers in patch 1. Makese sense.
RE: [PATCH v2] x86/hyperv: make vapic support x2apic mode
From: Michael Kelley Sent: Friday, October 4, 2019 3:33 PM > > From: Roman Kagan Sent: Friday, October 4, 2019 2:19 AM > > > > On Fri, Oct 04, 2019 at 03:01:51AM +, Michael Kelley wrote: > > > From: Roman Kagan Sent: Thursday, October 3, 2019 > > > 5:53 AM > > > > > > > > > > AFAIU you're trying to mirror native_x2apic_icr_write() here but this > > > > > is > > > > > different from what hv_apic_icr_write() does > > > > > (SET_APIC_DEST_FIELD(id)). > > > > > > > > Right. In xapic mode the ICR2 aka the high 4 bytes of ICR is programmed > > > > with the destination id in the highest byte; in x2apic mode the whole > > > > ICR2 is set to the 32bit destination id. > > > > > > > > > Is it actually correct? (I think you've tested this and it is but) > > > > > > > > As I wrote in the commit log, I haven't tested it in the sense that I > > > > ran a Linux guest in a Hyper-V VM exposing x2apic to the guest, because > > > > I didn't manage to configure it to do so. OTOH I did run a Windows > > > > guest in QEMU/KVM with hv_apic and x2apic enabled and saw it write > > > > destination ids unshifted to the ICR2 part of ICR, so I assume it's > > > > correct. > > > > > > > > > Michael, could you please shed some light here? > > > > > > > > Would be appreciated, indeed. > > > > > > > > > > The newest version of Hyper-V provides an x2apic in a guest VM when the > > > number of vCPUs in the VM is > 240. This version of Hyper-V is beginning > > > to be deployed in Azure to enable the M416v2 VM size, but the > > > functionality > > > is not yet available for the on-premises version of Hyper-V. However, I > > > can > > > test this configuration internally with the above patch -- give me a few > > > days. > > > > > > An additional complication is that when running on Intel processors that > > > offer > > > vAPIC functionality, the Hyper-V "hints" value does *not* recommend using > > > the > > > MSR-based APIC accesses. In this case, memory-mapped access to the x2apic > > > registers is faster than the synthetic MSRs. > > > > I guess you mean "using regular x2apic MSRs compared to the synthetic > > MSRs". > > Yes, of course you are correct. > > > Indeed they do essentially the same thing, and there's no reason > > for one set of MSRs to be significantly faster than the other. However, > > hv_apic_eoi_write makes use of "apic assists" aka lazy EOI which is > > certainly a win, and I'm not sure if it works without hv_apic. > > > > I've checked with the Hyper-V people and the presence of vAPIC makes > a difference. If vAPIC is present in the hardware: > 1) Hyper-V does not set the HV_X64_APIC_ACCESS_RECOMMENDED flag > 2) The architectural MSRs should be used instead of the Hyper-V > synthetic MSRs, as they are significantly faster. The architectural > MSRs do not cause a VMEXIT because they are handled entirely by > the vAPIC microcode in the CPU. The synthetic MSRs do cause a VMEXIT. > 3) The lazy EOI functionality should not be used > > If vAPIC is not present in the hardware: > 1) Hyper-V will set HV_X64_APIC_ACCESS_RECOMMENDED > 2) Either set of MSRs has about the same performance, but we > should use the synthetic MSRs. > 3) The lazy EOI functionality has some value and should be used > > The same will apply to the AMD AVIC in some Hyper-V updates that > are coming soon. > > So I think your code makes sense given the above information. By > Monday I'll try to test it on a Hyper-V guest VM with x2APIC. > I've smoke tested your code with a Hyper-V guest VM with x2APIC and 1024 vCPUs and HV_X64_APIC_ACCESS_RECOMMENDED enabled. The new x2apic functions you have added appear to work. No issues were seen. However, based on further discussion with the Hyper-V team, the architectural MSRs and the synthetic MSRs really are interchangeable with an x2apic. There's no perf difference like there is with the memory-mapped registers in the classic APIC. So your new x2apic functions aren't really needed -- all that's needed is to skip plugging in the hv_apic functions when an x2apic is present. The native x2apic functions will work just fine. Note that even with x2apic, hv_apic_eoi_write() should still be used to take advantage of the lazy EOI functionality. It's OK to use the synthetic EOI MSR with x2apic for this case, so again no additional code is needed. I quickly changed the code to do the above so that the architectural MSRs are used, and those changes successfully smoke test on the same 1024 vCPU VM with no problems. I tested with vAPIC enabled and with vAPIC disabled, and all looks good. Michael
Re: [PATCH v2] of: Make of_dma_get_range() work on bus nodes
Hi Rob/Robin,
On Tue, 2019-10-08 at 14:52 -0500, Rob Herring wrote:
> From: Robin Murphy
>
> Since the "dma-ranges" property is only valid for a node representing a
> bus, of_dma_get_range() currently assumes the node passed in is a leaf
> representing a device, and starts the walk from its parent. In cases
> like PCI host controllers on typical FDT systems, however, where the PCI
> endpoints are probed dynamically the initial leaf node represents the
> 'bus' itself, and this logic means we fail to consider any "dma-ranges"
> describing the host bridge itself. Rework the logic such that
> of_dma_get_range() also works correctly starting from a bus node
> containing "dma-ranges".
>
> While this does mean "dma-ranges" could incorrectly be in a device leaf
> node, there isn't really any way in this function to ensure that a leaf
> node is or isn't a bus node.
Sorry, I'm not totally sure if this is what you're pointing out with the last
sentence. But, what about the case of a bus configuring a device which also
happens to be a memory mapped bus (say a PCI platform device). It'll get it's
dma config based on its own dma-ranges which is not what we want.
> Signed-off-by: Robin Murphy
> [robh: Allow for the bus child node to still be passed in]
> Signed-off-by: Rob Herring
> ---
> Resending, hit send too quickly.
>
> v2:
> - Ensure once we find dma-ranges, every parent has it.
I like this new approach.
Regards,
Nicolas
> - Only get the #{size,address}-cells after we find non-empty dma-ranges
> - Add a check on the 'dma-ranges' length
>
> This is all that remains of the dma-ranges series. I've applied the rest
> of the series prep and fixes. I dropped "of: Ratify of_dma_configure()
> interface" as the assertions that the node pointer being the parent only
> when struct device doesn't have a DT node pointer is not always
> true.
>
> I didn't include any tested-bys as this has changed a bit. A git branch
> is here[1].
>
> Rob
>
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git dma-masks-v2
>
> drivers/of/address.c | 44 ++--
> 1 file changed, 18 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index 5ce69d026584..99c1b8058559 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -930,47 +930,39 @@ int of_dma_get_range(struct device_node *np, u64
> *dma_addr, u64 *paddr, u64 *siz
> const __be32 *ranges = NULL;
> int len, naddr, nsize, pna;
> int ret = 0;
> + bool found_dma_ranges = false;
> u64 dmaaddr;
>
> - if (!node)
> - return -EINVAL;
> -
> - while (1) {
> - struct device_node *parent;
> -
> - naddr = of_n_addr_cells(node);
> - nsize = of_n_size_cells(node);
> -
> - parent = __of_get_dma_parent(node);
> - of_node_put(node);
> -
> - node = parent;
> - if (!node)
> - break;
> -
> + while (node) {
> ranges = of_get_property(node, "dma-ranges", &len);
>
> /* Ignore empty ranges, they imply no translation required */
> if (ranges && len > 0)
> break;
>
> - /*
> - * At least empty ranges has to be defined for parent node if
> - * DMA is supported
> - */
> - if (!ranges)
> - break;
> + /* Once we find 'dma-ranges', then a missing one is an error */
> + if (found_dma_ranges && !ranges) {
> + ret = -ENODEV;
> + goto out;
> + }
> + found_dma_ranges = true;
> +
> + node = of_get_next_dma_parent(node);
> }
>
> - if (!ranges) {
> + if (!node || !ranges) {
> pr_debug("no dma-ranges found for node(%pOF)\n", np);
> ret = -ENODEV;
> goto out;
> }
>
> - len /= sizeof(u32);
> -
> + naddr = of_bus_n_addr_cells(node);
> + nsize = of_bus_n_size_cells(node);
> pna = of_n_addr_cells(node);
> + if ((len / sizeof(__be32)) % (pna + naddr + nsize)) {
> + ret = -EINVAL;
> + goto out;
> + }
>
> /* dma-ranges format:
>* DMA addr : naddr cells
> @@ -978,7 +970,7 @@ int of_dma_get_range(struct device_node *np, u64
> *dma_addr, u64 *paddr, u64 *siz
>* size : nsize cells
>*/
> dmaaddr = of_read_number(ranges, naddr);
> - *paddr = of_translate_dma_address(np, ranges);
> + *paddr = of_translate_dma_address(node, ranges + naddr);
> if (*paddr == OF_BAD_ADDR) {
> pr_err("translation of DMA address(%llx) to CPU address failed
> node(%pOF)\n",
> dmaaddr, np);
signature.asc
Description: This is a digitally signed message part
Re: linux-next: Fixes tags need some work in the mvebu tree
On Wed, 9 Oct 2019 07:38:03 +1100
Stephen Rothwell wrote:
> Hi all,
>
> In commit
>
> 69eea31a26da ("arm64: dts: armada-3720-turris-mox: convert usb-phy to
> phy-supply")
>
> Fixes tag
>
> Fixes: eb6c2eb6c7fb ("usb: host: xhci-plat: Prevent an abnormally
>
This is weird, in the patch I sent the tag ends there with ...")
Marek
Re: [RFC PATCH v2] e1000e: Use rtnl_lock to prevent race conditions between net and pci/pm
On Mon, 2019-10-07 at 10:27 -0700, Alexander Duyck wrote:
> From: Alexander Duyck
>
> This patch is meant to address possible race conditions that can exist
> between network configuration and power management. A similar issue was
> fixed for igb in commit 9474933caf21 ("igb: close/suspend race in
> netif_device_detach").
>
> In addition it consolidates the code so that the PCI error handling code
> will essentially perform the power management freeze on the device prior to
> attempting a reset, and will thaw the device afterwards if that is what it
> is planning to do. Otherwise when we call close on the interface it should
> see it is detached and not attempt to call the logic to down the interface
> and free the IRQs again.
>
> >From what I can tell the check that was adding the check for __E1000_DOWN
> in e1000e_close was added when runtime power management was added. However
> it should not be relevant for us as we perform a call to
> pm_runtime_get_sync before we call e1000_down/free_irq so it should always
> be back up before we call into this anyway.
>
> Signed-off-by: Alexander Duyck
> ---
>
> RFC v2: Dropped some unused variables
> Added logic to check for device present before removing to pm_freeze
> Fixed misplaced err_irq to before rtnl_unlock()
>
> drivers/net/ethernet/intel/e1000e/netdev.c | 40
> +++-
> 1 file changed, 21 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c
> b/drivers/net/ethernet/intel/e1000e/netdev.c
> index d7d56e42a6aa..8b4e589aca36 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -4715,12 +4715,12 @@ int e1000e_close(struct net_device *netdev)
>
> pm_runtime_get_sync(&pdev->dev);
>
> - if (!test_bit(__E1000_DOWN, &adapter->state)) {
> + if (netif_device_present(netdev)) {
> e1000e_down(adapter, true);
> e1000_free_irq(adapter);
>
> /* Link status message must follow this format */
> - pr_info("%s NIC Link is Down\n", adapter->netdev->name);
> + pr_info("%s NIC Link is Down\n", netdev->name);
> }
>
> napi_disable(&adapter->napi);
> @@ -6298,10 +6298,14 @@ static int e1000e_pm_freeze(struct device *dev)
> {
> struct net_device *netdev = dev_get_drvdata(dev);
> struct e1000_adapter *adapter = netdev_priv(netdev);
> + bool present;
>
> + rtnl_lock();
> +
> + present = netif_device_present(netdev);
> netif_device_detach(netdev);
>
> - if (netif_running(netdev)) {
> + if (present && netif_running(netdev)) {
> int count = E1000_CHECK_RESET_COUNT;
>
> while (test_bit(__E1000_RESETTING, &adapter->state) && count--)
> @@ -6313,6 +6317,8 @@ static int e1000e_pm_freeze(struct device *dev)
> e1000e_down(adapter, false);
> e1000_free_irq(adapter);
> }
> + rtnl_unlock();
> +
> e1000e_reset_interrupt_capability(adapter);
>
> /* Allow time for pending master requests to run */
> @@ -6626,27 +6632,31 @@ static int __e1000_resume(struct pci_dev *pdev)
> return 0;
> }
>
> -#ifdef CONFIG_PM_SLEEP
> static int e1000e_pm_thaw(struct device *dev)
> {
> struct net_device *netdev = dev_get_drvdata(dev);
> struct e1000_adapter *adapter = netdev_priv(netdev);
> + int rc = 0;
>
> e1000e_set_interrupt_capability(adapter);
> - if (netif_running(netdev)) {
> - u32 err = e1000_request_irq(adapter);
>
> - if (err)
> - return err;
> + rtnl_lock();
> + if (netif_running(netdev)) {
> + rc = e1000_request_irq(adapter);
> + if (rc)
> + goto err_irq;
>
> e1000e_up(adapter);
> }
>
> netif_device_attach(netdev);
> +err_irq:
> + rtnl_unlock();
>
> - return 0;
> + return rc;
> }
>
> +#ifdef CONFIG_PM_SLEEP
> static int e1000e_pm_suspend(struct device *dev)
> {
> struct pci_dev *pdev = to_pci_dev(dev);
> @@ -6818,16 +6828,11 @@ static void e1000_netpoll(struct net_device *netdev)
> static pci_ers_result_t e1000_io_error_detected(struct pci_dev *pdev,
> pci_channel_state_t state)
> {
> - struct net_device *netdev = pci_get_drvdata(pdev);
> - struct e1000_adapter *adapter = netdev_priv(netdev);
> -
> - netif_device_detach(netdev);
> + e1000e_pm_freeze(&pdev->dev);
>
> if (state == pci_channel_io_perm_failure)
> return PCI_ERS_RESULT_DISCONNECT;
>
> - if (netif_running(netdev))
> - e1000e_down(adapter, true);
> pci_disable_device(pdev);
>
> /* Request a slot slot reset. */
> @@ -6893,10 +6898,7 @@ static void e1000_io_resume(struct pci_dev *pdev)
>
> e1000_init_manageability_pt(adapter);
>
> - if (netif_running(netdev))
> - e1000e_up(
[PATCH v11 07/16] dt: bindings: lp55xx: Be consistent in the document with LED acronym
Update the document to be consistent in case when using "LED". This acronym should be capital throughout the document. Signed-off-by: Dan Murphy --- Documentation/devicetree/bindings/leds/leds-lp55xx.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/leds/leds-lp55xx.txt b/Documentation/devicetree/bindings/leds/leds-lp55xx.txt index 1b66a413fb9d..bfe2805c5534 100644 --- a/Documentation/devicetree/bindings/leds/leds-lp55xx.txt +++ b/Documentation/devicetree/bindings/leds/leds-lp55xx.txt @@ -1,4 +1,4 @@ -Binding for TI/National Semiconductor LP55xx Led Drivers +Binding for TI/National Semiconductor LP55xx LED Drivers Required properties: - compatible: one of @@ -12,8 +12,8 @@ Required properties: - clock-mode: Input clock mode, (0: automode, 1: internal, 2: external) Each child has own specific current settings -- led-cur: Current setting at each led channel (mA x10, 0 if led is not connected) -- max-cur: Maximun current at each led channel. +- led-cur: Current setting at each LED channel (mA x10, 0 if LED is not connected) +- max-cur: Maximun current at each LED channel. Optional properties: - enable-gpio: GPIO attached to the chip's enable pin -- 2.22.0.214.g8dca754b1e
[PATCH v11 01/16] dt: bindings: Add multicolor class dt bindings documention
Add DT bindings for the LEDs multicolor class framework.
Signed-off-by: Dan Murphy
---
.../bindings/leds/leds-class-multicolor.txt | 98 +++
1 file changed, 98 insertions(+)
create mode 100644
Documentation/devicetree/bindings/leds/leds-class-multicolor.txt
diff --git a/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt
b/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt
new file mode 100644
index ..8619c9bf1811
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt
@@ -0,0 +1,98 @@
+* Multicolor LED properties
+
+Bindings for multi color LEDs show how to describe current outputs of
+either integrated multi-color LED elements (like RGB, RGBW, RGBWA-UV
+etc.) or standalone LEDs, to achieve logically grouped multi-color LED
+modules. This is achieved by adding multi-led nodes layer to the
+monochrome LED bindings.
+
+The nodes and properties defined in this document are unique to the multicolor
+LED class. Common LED nodes and properties are inherited from the common.txt
+within this documentation directory.
+
+Required LED Child properties:
+ - color : For multicolor LED support this property should be defined as
+ LED_COLOR_ID_MULTI and further definition can be found in
+ include/linux/leds/common.h.
+
+led-controller@30 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "ti,lp5024";
+ reg = <0x29>;
+
+ multi-led@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+ color = ;
+ function = LED_FUNCTION_STATUS;
+
+
+ led@3 {
+ reg = <3>;
+ color = ;
+ };
+
+ led@4 {
+ reg = <4>;
+ color = ;
+ };
+
+ led@5 {
+ reg = <5>;
+ color = ;
+ };
+ };
+
+ multi-led@2 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ color = ;
+ function = LED_FUNCTION_ACTIVITY;
+ reg = <2>;
+ ti,led-bank = <2 3 5>;
+
+ led@6 {
+ reg = <0x6>;
+ color = ;
+ led-sources = <6 9 15>;
+ };
+
+ led@7 {
+ reg = <0x7>;
+ color = ;
+ led-sources = <7 10 16>;
+ };
+
+ led@8 {
+ reg = <0x8>;
+ color = ;
+ led-sources = <8 11 17>;
+ };
+ };
+
+ multi-led@4 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <4>;
+ color = ;
+ function = LED_FUNCTION_ACTIVITY;
+
+ led@12 {
+ reg = <12>;
+ color = ;
+ };
+
+ led@13 {
+ reg = <13>;
+ color = ;
+ };
+
+ led@14 {
+ reg = <14>;
+ color = ;
+ };
+ };
+
+};
--
2.22.0.214.g8dca754b1e
[PATCH v11 06/16] leds: lp50xx: Add the LP50XX family of the RGB LED driver
Introduce the LP5036/30/24/18/12/9 RGB LED driver. The difference in these parts are the number of LED outputs where the: LP5036 can control 36 LEDs LP5030 can control 30 LEDs LP5024 can control 24 LEDs LP5018 can control 18 LEDs LP5012 can control 12 LEDs LP5009 can control 9 LEDs The device has the ability to group LED output into control banks so that multiple LED banks can be controlled with the same mixing and brightness. Inversely the LEDs can also be controlled independently. Signed-off-by: Dan Murphy --- drivers/leds/Kconfig | 11 + drivers/leds/Makefile | 1 + drivers/leds/leds-lp50xx.c | 799 + 3 files changed, 811 insertions(+) create mode 100644 drivers/leds/leds-lp50xx.c diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index a1ede89afc9e..fb614a6b9afa 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -363,6 +363,17 @@ config LEDS_LP3952 To compile this driver as a module, choose M here: the module will be called leds-lp3952. +config LEDS_LP50XX + tristate "LED Support for TI LP5036/30/24/18/12/9 LED driver chip" + depends on LEDS_CLASS && REGMAP_I2C + depends on LEDS_CLASS_MULTI_COLOR + help + If you say yes here you get support for the Texas Instruments + LP5036, LP5030, LP5024, LP5018, LP5012 and LP5009 LED driver. + + To compile this driver as a module, choose M here: the + module will be called leds-lp50xx. + config LEDS_LP55XX_COMMON tristate "Common Driver for TI/National LP5521/5523/55231/5562/8501" depends on LEDS_LP5521 || LEDS_LP5523 || LEDS_LP5562 || LEDS_LP8501 diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index 841038cfe35b..7a208a0f7b84 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -34,6 +34,7 @@ obj-$(CONFIG_LEDS_GPIO_REGISTER) += leds-gpio-register.o obj-$(CONFIG_LEDS_GPIO)+= leds-gpio.o obj-$(CONFIG_LEDS_LP3944) += leds-lp3944.o obj-$(CONFIG_LEDS_LP3952) += leds-lp3952.o +obj-$(CONFIG_LEDS_LP50XX) += leds-lp50xx.o obj-$(CONFIG_LEDS_LP55XX_COMMON) += leds-lp55xx-common.o obj-$(CONFIG_LEDS_LP5521) += leds-lp5521.o obj-$(CONFIG_LEDS_LP5523) += leds-lp5523.o diff --git a/drivers/leds/leds-lp50xx.c b/drivers/leds/leds-lp50xx.c new file mode 100644 index ..6a042f973a83 --- /dev/null +++ b/drivers/leds/leds-lp50xx.c @@ -0,0 +1,799 @@ +// SPDX-License-Identifier: GPL-2.0 +// TI LP50XX LED chip family driver +// Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#define LP50XX_DEV_CFG00x00 +#define LP50XX_DEV_CFG10x01 +#define LP50XX_LED_CFG00x02 + +/* LP5009 and LP5012 registers */ +#define LP5012_BNK_BRT 0x03 +#define LP5012_BNKA_CLR0x04 +#define LP5012_BNKB_CLR0x05 +#define LP5012_BNKC_CLR0x06 +#define LP5012_LED0_BRT0x07 +#define LP5012_LED1_BRT0x08 +#define LP5012_LED2_BRT0x09 +#define LP5012_LED3_BRT0x0a +#define LP5012_OUT0_CLR0x0b +#define LP5012_OUT1_CLR0x0c +#define LP5012_OUT2_CLR0x0d +#define LP5012_OUT3_CLR0x0e +#define LP5012_OUT4_CLR0x0f +#define LP5012_OUT5_CLR0x10 +#define LP5012_OUT6_CLR0x11 +#define LP5012_OUT7_CLR0x12 +#define LP5012_OUT8_CLR0x13 +#define LP5012_OUT9_CLR0x14 +#define LP5012_OUT10_CLR 0x15 +#define LP5012_OUT11_CLR 0x16 +#define LP5012_RESET 0x17 + +/* LP5018 and LP5024 registers */ +#define LP5024_BNK_BRT 0x03 +#define LP5024_BNKA_CLR0x04 +#define LP5024_BNKB_CLR0x05 +#define LP5024_BNKC_CLR0x06 +#define LP5024_LED0_BRT0x07 +#define LP5024_LED1_BRT0x08 +#define LP5024_LED2_BRT0x09 +#define LP5024_LED3_BRT0x0a +#define LP5024_LED4_BRT0x0b +#define LP5024_LED5_BRT0x0c +#define LP5024_LED6_BRT0x0d +#define LP5024_LED7_BRT0x0e + +#define LP5024_OUT0_CLR0x0f +#define LP5024_OUT1_CLR0x10 +#define LP5024_OUT2_CLR0x11 +#define LP5024_OUT3_CLR0x12 +#define LP5024_OUT4_CLR0x13 +#define LP5024_OUT5_CLR0x14 +#define LP5024_OUT6_CLR0x15 +#define LP5024_OUT7_CLR0x16 +#define LP5024_OUT8_CLR0x17 +#define LP5024_OUT9_CLR0x18 +#define LP5024_OUT10_CLR 0x19 +#define LP5024_OUT11_CLR 0x1a
[PATCH v11 08/16] dt: bindings: lp55xx: Update binding for Multicolor Framework
Update the DT binding to include the properties to use the
multicolor framework for the devices that use the LP55xx
framework.
Signed-off-by: Dan Murphy
CC: Tony Lindgren
CC: "Benoît Cousson"
CC: Linus Walleij
CC: Shawn Guo
CC: Sascha Hauer
CC: Pengutronix Kernel Team
CC: Fabio Estevam
CC: NXP Linux Team
---
.../devicetree/bindings/leds/leds-lp55xx.txt | 149 +++---
1 file changed, 124 insertions(+), 25 deletions(-)
diff --git a/Documentation/devicetree/bindings/leds/leds-lp55xx.txt
b/Documentation/devicetree/bindings/leds/leds-lp55xx.txt
index bfe2805c5534..736a2e1538be 100644
--- a/Documentation/devicetree/bindings/leds/leds-lp55xx.txt
+++ b/Documentation/devicetree/bindings/leds/leds-lp55xx.txt
@@ -1,6 +1,8 @@
Binding for TI/National Semiconductor LP55xx LED Drivers
Required properties:
+- #address-cells: 1
+- #size-cells: 0
- compatible: one of
national,lp5521
national,lp5523
@@ -14,6 +16,18 @@ Required properties:
Each child has own specific current settings
- led-cur: Current setting at each LED channel (mA x10, 0 if LED is not
connected)
- max-cur: Maximun current at each LED channel.
+- reg: Output channel for the LED. This is zero based channel identifier and
+ the data sheet is a one based channel identifier.
+ reg value to output to LED output number
+ D1 = reg value is 0
+ D2 = reg value is 1
+ D3 = reg value is 2
+ D4 = reg value is 3
+ D5 = reg value is 4
+ D6 = reg value is 5
+ D7 = reg value is 6
+ D8 = reg value is 7
+ D9 = reg value is 8
Optional properties:
- enable-gpio: GPIO attached to the chip's enable pin
@@ -35,23 +49,28 @@ example 1) LP5521
on channel 0.
lp5521@32 {
+ #address-cells = <1>;
+ #size-cells = <0>;
compatible = "national,lp5521";
reg = <0x32>;
label = "lp5521_pri";
clock-mode = /bits/ 8 <2>;
- chan0 {
+ chan@0 {
+ reg = <0>;
led-cur = /bits/ 8 <0x2f>;
max-cur = /bits/ 8 <0x5f>;
linux,default-trigger = "heartbeat";
};
- chan1 {
+ chan@1 {
+ reg = <1>;
led-cur = /bits/ 8 <0x2f>;
max-cur = /bits/ 8 <0x5f>;
};
- chan2 {
+ chan@2 {
+ reg = <2>;
led-cur = /bits/ 8 <0x2f>;
max-cur = /bits/ 8 <0x5f>;
};
@@ -70,59 +89,70 @@ ASEL1ASEL0Address
VEN VEN 35h
lp5523@32 {
+ #address-cells = <1>;
+ #size-cells = <0>;
compatible = "national,lp5523";
reg = <0x32>;
clock-mode = /bits/ 8 <1>;
- chan0 {
+ chan@0 {
+ reg = <0>;
chan-name = "d1";
led-cur = /bits/ 8 <0x14>;
max-cur = /bits/ 8 <0x20>;
};
- chan1 {
+ chan@1 {
+ reg = <1>;
chan-name = "d2";
led-cur = /bits/ 8 <0x14>;
max-cur = /bits/ 8 <0x20>;
};
- chan2 {
+ chan@2 {
+ reg = <2>;
chan-name = "d3";
led-cur = /bits/ 8 <0x14>;
max-cur = /bits/ 8 <0x20>;
};
- chan3 {
+ chan@3 {
+ reg = <3>;
chan-name = "d4";
led-cur = /bits/ 8 <0x14>;
max-cur = /bits/ 8 <0x20>;
};
- chan4 {
+ chan@4 {
+ reg = <4>;
chan-name = "d5";
led-cur = /bits/ 8 <0x14>;
max-cur = /bits/ 8 <0x20>;
};
- chan5 {
+ chan@5 {
+ reg = <5>;
chan-name = "d6";
led-cur = /bits/ 8 <0x14>;
max-cur = /bits/ 8 <0x20>;
};
- chan6 {
+ chan@6 {
+ reg = <6>;
chan-name = "d7";
led-cur = /bits/ 8 <0x14>;
max-cur = /bits/ 8 <0x20>;
};
- chan7 {
+ chan@7 {
+ reg = <7>;
chan-name = "d8";
led-cur = /bits/ 8 <0x14>;
max-cur = /bits/ 8 <0x20>;
};
- chan8 {
+ chan@8 {
+ reg = <8>;
chan-name = "d9";
led-cur = /bits/ 8 <0x14>;
max-cur = /bits/ 8 <0x20>;
@@ -133,29 +163,35 @@ example 3) LP5562
4 channels are defined.
lp5562@30 {
+ #address-cells = <1>;
+ #size-cells = <0>;
compatible = "ti,lp5562";
reg = <0x30>;
clock-mode = /bits/8 <2>;
- chan0 {
+ chan@0 {
+ reg = <0>;
chan-name = "R";
led-cur = /bits/ 8 <0x20>;
max-cur = /bits/ 8 <0x60>;
};
- chan1 {
+ chan@1 {
+ reg = <1>;
chan-name = "G";
led-cur = /bits/ 8 <0x20>;
m
[PATCH v11 13/16] leds: lp5523: Update the lp5523 code to add intensity function
Add the intensity function call back to support the multicolor
framework. This function allows setting a specific brightness on
a specific channel.
Signed-off-by: Dan Murphy
---
drivers/leds/leds-lp5523.c | 20
1 file changed, 20 insertions(+)
diff --git a/drivers/leds/leds-lp5523.c b/drivers/leds/leds-lp5523.c
index d0b931a136b9..5d6002e4d657 100644
--- a/drivers/leds/leds-lp5523.c
+++ b/drivers/leds/leds-lp5523.c
@@ -791,6 +791,25 @@ static ssize_t store_master_fader_leds(struct device *dev,
return ret;
}
+static int lp5523_led_intensity(struct lp55xx_led *led)
+{
+ struct lp55xx_chip *chip = led->chip;
+ int ret;
+ int i;
+
+ mutex_lock(&chip->lock);
+ for (i = 0; i < led->mc_cdev.num_leds; i++) {
+ ret = lp55xx_write(chip,
+ LP5523_REG_LED_PWM_BASE +
+ led->color_component[i].output_num,
+ led->color_component[i].brightness);
+ if (ret)
+ break;
+ }
+ mutex_unlock(&chip->lock);
+ return ret;
+}
+
static int lp5523_led_brightness(struct lp55xx_led *led)
{
struct lp55xx_chip *chip = led->chip;
@@ -857,6 +876,7 @@ static struct lp55xx_device_config lp5523_cfg = {
.max_channel = LP5523_MAX_LEDS,
.post_init_device = lp5523_post_init_device,
.brightness_fn = lp5523_led_brightness,
+ .color_intensity_fn = lp5523_led_intensity,
.set_led_current= lp5523_set_led_current,
.firmware_cb= lp5523_firmware_loaded,
.run_engine = lp5523_run_engine,
--
2.22.0.214.g8dca754b1e
[PATCH v11 14/16] leds: lp5521: Add multicolor framework intensity support
Add the intensity function call back to support the multicolor
framework. This function allows setting a specific brightness on
a specific channel.
Signed-off-by: Dan Murphy
---
drivers/leds/leds-lp5521.c | 20
1 file changed, 20 insertions(+)
diff --git a/drivers/leds/leds-lp5521.c b/drivers/leds/leds-lp5521.c
index 6f0272249dc8..05731f405c9a 100644
--- a/drivers/leds/leds-lp5521.c
+++ b/drivers/leds/leds-lp5521.c
@@ -349,6 +349,25 @@ static int lp5521_run_selftest(struct lp55xx_chip *chip,
char *buf)
return 0;
}
+static int lp5521_led_intensity(struct lp55xx_led *led)
+{
+ struct lp55xx_chip *chip = led->chip;
+ int ret;
+ int i;
+
+ mutex_lock(&chip->lock);
+ for (i = 0; i < led->mc_cdev.num_leds; i++) {
+ ret = lp55xx_write(chip,
+ LP5521_REG_LED_PWM_BASE +
+ led->color_component[i].output_num,
+ led->color_component[i].brightness);
+ if (ret)
+ break;
+ }
+ mutex_unlock(&chip->lock);
+ return ret;
+}
+
static int lp5521_led_brightness(struct lp55xx_led *led)
{
struct lp55xx_chip *chip = led->chip;
@@ -490,6 +509,7 @@ static struct lp55xx_device_config lp5521_cfg = {
.max_channel = LP5521_MAX_LEDS,
.post_init_device = lp5521_post_init_device,
.brightness_fn = lp5521_led_brightness,
+ .color_intensity_fn = lp5521_led_intensity,
.set_led_current= lp5521_set_led_current,
.firmware_cb= lp5521_firmware_loaded,
.run_engine = lp5521_run_engine,
--
2.22.0.214.g8dca754b1e
[PATCH v11 09/16] ARM: dts: n900: Add reg property to the LP5523 channel node
Add the reg property to each channel node. This update is
to accomodate the multicolor framework. In addition to the
accomodation this allows the LEDs to be placed on any channel
and allow designs to skip channels as opposed to requiring
sequential order.
Signed-off-by: Dan Murphy
CC: Tony Lindgren
CC: "Benoît Cousson"
k# interactive rebase in progress; onto ae89cc6d4a8c
---
arch/arm/boot/dts/omap3-n900.dts | 29 -
1 file changed, 20 insertions(+), 9 deletions(-)
diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
index 84a5ade1e865..643f35619246 100644
--- a/arch/arm/boot/dts/omap3-n900.dts
+++ b/arch/arm/boot/dts/omap3-n900.dts
@@ -607,63 +607,74 @@
};
lp5523: lp5523@32 {
+ #address-cells = <1>;
+ #size-cells = <0>;
compatible = "national,lp5523";
reg = <0x32>;
clock-mode = /bits/ 8 <0>; /* LP55XX_CLOCK_AUTO */
enable-gpio = <&gpio2 9 GPIO_ACTIVE_HIGH>; /* 41 */
- chan0 {
+ chan@0 {
chan-name = "lp5523:kb1";
led-cur = /bits/ 8 <50>;
max-cur = /bits/ 8 <100>;
+ reg = <0>;
};
- chan1 {
+ chan@1 {
chan-name = "lp5523:kb2";
led-cur = /bits/ 8 <50>;
max-cur = /bits/ 8 <100>;
+ reg = <1>;
};
- chan2 {
+ chan@2 {
chan-name = "lp5523:kb3";
led-cur = /bits/ 8 <50>;
max-cur = /bits/ 8 <100>;
+ reg = <2>;
};
- chan3 {
+ chan@3 {
chan-name = "lp5523:kb4";
led-cur = /bits/ 8 <50>;
max-cur = /bits/ 8 <100>;
+ reg = <3>;
};
- chan4 {
+ chan@4 {
chan-name = "lp5523:b";
led-cur = /bits/ 8 <50>;
max-cur = /bits/ 8 <100>;
+ reg = <4>;
};
- chan5 {
+ chan@5 {
chan-name = "lp5523:g";
led-cur = /bits/ 8 <50>;
max-cur = /bits/ 8 <100>;
+ reg = <5>;
};
- chan6 {
+ chan@6 {
chan-name = "lp5523:r";
led-cur = /bits/ 8 <50>;
max-cur = /bits/ 8 <100>;
+ reg = <6>;
};
- chan7 {
+ chan@7 {
chan-name = "lp5523:kb5";
led-cur = /bits/ 8 <50>;
max-cur = /bits/ 8 <100>;
+ reg = <7>;
};
- chan8 {
+ chan@8 {
chan-name = "lp5523:kb6";
led-cur = /bits/ 8 <50>;
max-cur = /bits/ 8 <100>;
+ reg = <8>;
};
};
--
2.22.0.214.g8dca754b1e
[PATCH v11 05/16] dt: bindings: lp50xx: Introduce the lp50xx family of RGB drivers
Introduce the bindings for the Texas Instruments LP5036, LP5030, LP5024,
LP5018, LP5012 and LP5009 RGB LED device driver. The LP5036/30/24/18/12/9
can control RGB LEDs individually or as part of a control bank group.
These devices have the ability to adjust the mixing control for the RGB
LEDs to obtain different colors independent of the overall brightness of
the LED grouping.
Datasheet:
http://www.ti.com/lit/ds/symlink/lp5012.pdf
http://www.ti.com/lit/ds/symlink/lp5024.pdf
http://www.ti.com/lit/ds/symlink/lp5036.pdf
Signed-off-by: Dan Murphy
---
.../devicetree/bindings/leds/leds-lp50xx.txt | 148 ++
1 file changed, 148 insertions(+)
create mode 100644 Documentation/devicetree/bindings/leds/leds-lp50xx.txt
diff --git a/Documentation/devicetree/bindings/leds/leds-lp50xx.txt
b/Documentation/devicetree/bindings/leds/leds-lp50xx.txt
new file mode 100644
index ..8a0a21f1056c
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-lp50xx.txt
@@ -0,0 +1,148 @@
+* Texas Instruments - LP5009/12/18/24/30/36 RGB LED driver
+
+The LP50XX is multi-channel, I2C RGB LED Drivers that can group RGB LEDs into
+a LED group or control them individually.
+
+The difference in these RGB LED drivers is the number of supported RGB modules.
+
+Required parent properties:
+ - compatible:
+ "ti,lp5009"
+ "ti,lp5012"
+ "ti,lp5018"
+ "ti,lp5024"
+ "ti,lp5030"
+ "ti,lp5036"
+ - reg : I2C slave address
+ lp5009/12 - 0x14, 0x15, 0x16, 0x17
+ lp5018/24 - 0x28, 0x29, 0x2a, 0x2b
+ lp5030/36 - 0x30, 0x31, 0x32, 0x33
+ - #address-cells : 1
+ - #size-cells : 0
+
+Optional parent properties:
+ - enable-gpios : gpio pin to enable/disable the device.
+ - vled-supply : LED supply
+
+Required child properties:
+ - #address-cells : 1
+ - #size-cells : 0
+ - reg : This is the LED module number.
+ - color : Must be LED_COLOR_ID_MULTI
+ - function : see Documentation/devicetree/bindings/leds/common.txt
+
+Required child properties only is LED modules will be banked:
+ - ti,led-bank : This property denotes the LED module numbers that will
+ be controlled as a single RGB cluster. Each LED module
+ number will be controlled by a single LED class
instance.
+ There can only be one instance of the ti,led-bank
+ property for each device node.
+
+Required grandchildren properties:
+ - reg : A single entry denoting the LED output that controls
+ the monochrome LED.
+ - color : see Documentation/devicetree/bindings/leds/common.txt
+ - led-sources : see Documentation/devicetree/bindings/leds/common.txt
+
+The LED outputs associated with the LED modules are defined in Table 1 of the
+corresponding data sheets.
+
+LP5009 - 3 Total RGB cluster LED outputs 0-2
+LP5012 - 4 Total RGB cluster LED outputs 0-3
+LP5018 - 6 Total RGB cluster LED outputs 0-5
+LP5024 - 8 Total RGB cluster LED outputs 0-7
+LP5030 - 10 Total RGB cluster LED outputs 0-9
+LP5036 - 12 Total RGB cluster LED outputs 0-11
+
+Optional child properties:
+ - label : see Documentation/devicetree/bindings/leds/common.txt
+ - linux,default-trigger :
+ see Documentation/devicetree/bindings/leds/common.txt
+
+Examples:
+led-controller@29 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "ti,lp5024";
+ reg = <0x29>;
+ enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
+ vled-supply = <&vmmcsd_fixed>;
+
+ multi-led@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+ color = ;
+ function = LED_FUNCTION_STATUS;
+
+ led@3 {
+ reg = <3>;
+ color = ;
+ };
+
+ led@4 {
+ reg = <4>;
+ color = ;
+ };
+
+ led@5 {
+ reg = <5>;
+ color = ;
+ };
+ };
+
+ multi-led@2 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <2>;
+ color = ;
+ function = LED_FUNCTION_STANDBY;
+ ti,led-bank = <2 3 5>;
+
+ led@6 {
+ reg = <0x6>;
+ color = ;
+ led-sources = <6 9 15>;
+ };
+
+ led@7 {
+ reg = <0x7>;
+ color = ;
+ led-sources = <7 10 16>;
+ };
+
+ led@8 {
+ reg = <0x8>;
+ color = ;
+ led-sources = <8 11 17>;
+ };
+ };
+
+ multi-led@
[PATCH v11 03/16] leds: Add multicolor ID to the color ID list
Add a new color ID that is declared as MULTICOLOR as with the
multicolor framework declaring a definitive color is not accurate
as the node can contain multiple colors.
Signed-off-by: Dan Murphy
---
drivers/leds/led-core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index f1f718dbe0f8..846248a0693d 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -34,6 +34,7 @@ const char * const led_colors[LED_COLOR_ID_MAX] = {
[LED_COLOR_ID_VIOLET] = "violet",
[LED_COLOR_ID_YELLOW] = "yellow",
[LED_COLOR_ID_IR] = "ir",
+ [LED_COLOR_ID_MULTI] = "multicolor",
};
EXPORT_SYMBOL_GPL(led_colors);
--
2.22.0.214.g8dca754b1e
[PATCH v11 11/16] ARM: dts: ste-href: Add reg property to the LP5521 channel nodes
Add the reg property to each channel node. This update is
to accomodate the multicolor framework. In addition to the
accomodation this allows the LEDs to be placed on any channel
and allow designs to skip channels as opposed to requiring
sequential order.
Signed-off-by: Dan Murphy
CC: Linus Walleij
---
arch/arm/boot/dts/ste-href.dtsi | 22 --
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/arch/arm/boot/dts/ste-href.dtsi b/arch/arm/boot/dts/ste-href.dtsi
index 4f6acbd8c040..8a873da102d3 100644
--- a/arch/arm/boot/dts/ste-href.dtsi
+++ b/arch/arm/boot/dts/ste-href.dtsi
@@ -56,16 +56,21 @@
reg = <0x33>;
label = "lp5521_pri";
clock-mode = /bits/ 8 <2>;
- chan0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ chan@0 {
+ reg = <0>;
led-cur = /bits/ 8 <0x2f>;
max-cur = /bits/ 8 <0x5f>;
linux,default-trigger = "heartbeat";
};
- chan1 {
+ chan@1 {
+ reg = <1>;
led-cur = /bits/ 8 <0x2f>;
max-cur = /bits/ 8 <0x5f>;
};
- chan2 {
+ chan@2 {
+ reg = <2>;
led-cur = /bits/ 8 <0x2f>;
max-cur = /bits/ 8 <0x5f>;
};
@@ -75,15 +80,20 @@
reg = <0x34>;
label = "lp5521_sec";
clock-mode = /bits/ 8 <2>;
- chan0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ chan@0 {
+ reg = <0>;
led-cur = /bits/ 8 <0x2f>;
max-cur = /bits/ 8 <0x5f>;
};
- chan1 {
+ chan@1 {
+ reg = <1>;
led-cur = /bits/ 8 <0x2f>;
max-cur = /bits/ 8 <0x5f>;
};
- chan2 {
+ chan@2 {
+ reg = <2>;
led-cur = /bits/ 8 <0x2f>;
max-cur = /bits/ 8 <0x5f>;
};
--
2.22.0.214.g8dca754b1e
[PATCH v11 04/16] leds: multicolor: Introduce a multicolor class definition
Introduce a multicolor class that groups colored LEDs within a LED node. The multi color class groups monochrome LEDs and allows controlling two aspects of the final combined color: hue and lightness. The former is controlled via _intensity files and the latter is controlled via brightness file. Signed-off-by: Dan Murphy --- .../ABI/testing/sysfs-class-led-multicolor| 35 +++ Documentation/leds/index.rst | 1 + Documentation/leds/leds-class-multicolor.rst | 96 +++ drivers/leds/Kconfig | 10 + drivers/leds/Makefile | 1 + drivers/leds/led-class-multicolor.c | 271 ++ include/linux/led-class-multicolor.h | 143 + 7 files changed, 557 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-class-led-multicolor create mode 100644 Documentation/leds/leds-class-multicolor.rst create mode 100644 drivers/leds/led-class-multicolor.c create mode 100644 include/linux/led-class-multicolor.h diff --git a/Documentation/ABI/testing/sysfs-class-led-multicolor b/Documentation/ABI/testing/sysfs-class-led-multicolor new file mode 100644 index ..65cb43de26e6 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-class-led-multicolor @@ -0,0 +1,35 @@ +What: /sys/class/leds//brightness +Date: Sept 2019 +KernelVersion: 5.5 +Contact: Dan Murphy +Description: read/write + Writing to this file will update all LEDs within the group to a + calculated percentage of what each color LED intensity is set + to. The percentage is calculated via the equation below: + + led_brightness = brightness * _intensity/_max_intensity + + For additional details please refer to + Documentation/leds/leds-class-multicolor.rst. + + The value of the color is from 0 to + /sys/class/leds//max_brightness. + +What: /sys/class/leds//colors/_intensity +Date: Sept 2019 +KernelVersion: 5.5 +Contact: Dan Murphy +Description: read/write + The _intensity file is created based on the color + defined by the registrar of the class. + There is one file per color presented. + + The value of the color is from 0 to + /sys/class/leds//colors/_max_intensity. + +What: /sys/class/leds//colors/_max_intensity +Date: Sept 2019 +KernelVersion: 5.5 +Contact: Dan Murphy +Description: read only + Maximum intensity level for the LED color. diff --git a/Documentation/leds/index.rst b/Documentation/leds/index.rst index 060f4e485897..bc70c6aa7138 100644 --- a/Documentation/leds/index.rst +++ b/Documentation/leds/index.rst @@ -9,6 +9,7 @@ LEDs leds-class leds-class-flash + leds-class-multicolor ledtrig-oneshot ledtrig-transient ledtrig-usbport diff --git a/Documentation/leds/leds-class-multicolor.rst b/Documentation/leds/leds-class-multicolor.rst new file mode 100644 index ..7a695a29377e --- /dev/null +++ b/Documentation/leds/leds-class-multicolor.rst @@ -0,0 +1,96 @@ + +Multi Color LED handling under Linux + + +Description +=== +The multi color class groups monochrome LEDs and allows controlling two +aspects of the final combined color: hue and lightness. The former is +controlled via _intensity files and the latter is controlled +via brightness file. + +For more details on hue and lightness notions please refer to +https://en.wikipedia.org/wiki/CIECAM02. + +Note that intensity files only cache the written value and the actual +change of hardware state occurs upon writing brightness file. This +allows for changing many factors of the perceived color in a virtually +unnoticeable way for the human observer. + +Multicolor Class Control + +The multicolor class presents the LED groups under a directory called "colors". +This directory is a child under the LED parent node created by the led_class +framework. The led_class framework is documented in led-class.rst within this +documentation directory. + +Each colored LED will have two files created under the colors directory +_intensity and _max_intensity. These files will contain +one of LED_COLOR_ID_* definitions from the header +include/dt-bindings/leds/common.h. + +Directory Layout Example + +root:/sys/class/leds/rgb:grouped_leds# ls -lR colors/ +-rw-r--r--1 root root 4096 Jul 7 03:10 blue_intensity +-r--r--r--1 root root 4096 Jul 7 03:10 blue_max_intensity +-rw-r--r--1 root root 4096 Jul 7 03:10 green_intensity +-r--r--r--1 root root 4096 Jul 7 03:10 green_max_intensity +-rw-r--r--1 root root 4096 Jul 7 03:10 red_intensity +-r--r--r--1 root root 4096
[PATCH v11 12/16] leds: lp55xx: Add multicolor framework support to lp55xx
Add multicolor framework support for the lp55xx family.
Signed-off-by: Dan Murphy
---
drivers/leds/Kconfig | 1 +
drivers/leds/leds-lp55xx-common.c | 176 +++---
drivers/leds/leds-lp55xx-common.h | 11 ++
include/linux/platform_data/leds-lp55xx.h | 5 +
4 files changed, 169 insertions(+), 24 deletions(-)
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index fb614a6b9afa..5706bf8d8bd1 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -377,6 +377,7 @@ config LEDS_LP50XX
config LEDS_LP55XX_COMMON
tristate "Common Driver for TI/National LP5521/5523/55231/5562/8501"
depends on LEDS_LP5521 || LEDS_LP5523 || LEDS_LP5562 || LEDS_LP8501
+ depends on OF
select FW_LOADER
select FW_LOADER_USER_HELPER
help
diff --git a/drivers/leds/leds-lp55xx-common.c
b/drivers/leds/leds-lp55xx-common.c
index 44ced02b49f9..85629f836082 100644
--- a/drivers/leds/leds-lp55xx-common.c
+++ b/drivers/leds/leds-lp55xx-common.c
@@ -131,14 +131,54 @@ static struct attribute *lp55xx_led_attrs[] = {
};
ATTRIBUTE_GROUPS(lp55xx_led);
+static int lp55xx_map_channel(struct lp55xx_led *led, int color_id,
+ enum led_brightness brightness)
+{
+ int i;
+
+ for (i = 0; i < led->mc_cdev.num_leds; i++) {
+ if (led->color_component[i].color_id == color_id) {
+ led->color_component[i].brightness = brightness;
+ return 0;
+ }
+ }
+
+ return -EINVAL;
+}
+
static int lp55xx_set_brightness(struct led_classdev *cdev,
enum led_brightness brightness)
{
+ struct led_mc_color_conversion color_component[LP55XX_MAX_GROUPED_CHAN];
struct lp55xx_led *led = cdev_to_lp55xx_led(cdev);
struct lp55xx_device_config *cfg = led->chip->cfg;
+ int ret;
+ int i;
- led->brightness = (u8)brightness;
- return cfg->brightness_fn(led);
+ if (led->mc_cdev.num_leds > 1) {
+ if (!cfg->color_intensity_fn)
+ return -EINVAL;
+
+ led_mc_calc_brightness(&led->mc_cdev, brightness,
+ color_component);
+
+ for (i = 0; i < led->mc_cdev.num_leds; i++) {
+ ret = lp55xx_map_channel(led,
+ color_component[i].color_id,
+ color_component[i].brightness);
+ if (ret)
+ return ret;
+ }
+
+ ret = cfg->color_intensity_fn(led);
+ if (ret)
+ return ret;
+ } else {
+ led->brightness = (u8)brightness;
+ ret = cfg->brightness_fn(led);
+ }
+
+ return ret;
}
static int lp55xx_init_led(struct lp55xx_led *led,
@@ -147,9 +187,9 @@ static int lp55xx_init_led(struct lp55xx_led *led,
struct lp55xx_platform_data *pdata = chip->pdata;
struct lp55xx_device_config *cfg = chip->cfg;
struct device *dev = &chip->cl->dev;
+ int max_channel = cfg->max_channel;
char name[32];
int ret;
- int max_channel = cfg->max_channel;
if (chan >= max_channel) {
dev_err(dev, "invalid channel: %d / %d\n", chan, max_channel);
@@ -159,10 +199,34 @@ static int lp55xx_init_led(struct lp55xx_led *led,
if (pdata->led_config[chan].led_current == 0)
return 0;
+ if (pdata->led_config[chan].name) {
+ led->cdev.name = pdata->led_config[chan].name;
+ } else {
+ snprintf(name, sizeof(name), "%s:channel%d",
+ pdata->label ? : chip->cl->name, chan);
+ led->cdev.name = name;
+ }
+
+ if (pdata->led_config[chan].num_colors > 1) {
+ led->mc_cdev.led_cdev = &led->cdev;
+ led->cdev.brightness_set_blocking = lp55xx_set_brightness;
+ led->cdev.groups = lp55xx_led_groups;
+ led->mc_cdev.num_leds = pdata->led_config[chan].num_colors;
+ led->mc_cdev.available_colors =
+ pdata->led_config[chan].available_colors;
+ memcpy(led->color_component,
+ pdata->led_config[chan].color_component,
+ sizeof(led->color_component));
+ } else {
+
+ led->cdev.default_trigger =
+ pdata->led_config[chan].default_trigger;
+ led->cdev.brightness_set_blocking = lp55xx_set_brightness;
+ } led->cdev.groups = lp55xx_led_groups;
+
led->led_current = pdata->led_config[chan].led_current;
led->max_current = pdata->led_config[chan].max_current;
led->chan_nr = pdata->led_config[chan].chan_nr;
- led->cdev.default_trigger = pdata->led_config[chan].default_trigger;
[PATCH v11 16/16] leds: lp5523: Fix checkpatch issues in the code
Fix checkpatch errors and warnings for the LP5523.c device
driver.
Signed-off-by: Dan Murphy
---
drivers/leds/leds-lp5523.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/leds/leds-lp5523.c b/drivers/leds/leds-lp5523.c
index 5d6002e4d657..3c7c2e68e137 100644
--- a/drivers/leds/leds-lp5523.c
+++ b/drivers/leds/leds-lp5523.c
@@ -23,13 +23,13 @@
#define LP5523_PROGRAM_LENGTH 32 /* bytes */
/* Memory is used like this:
- 0x00 engine 1 program
- 0x10 engine 2 program
- 0x20 engine 3 program
- 0x30 engine 1 muxing info
- 0x40 engine 2 muxing info
- 0x50 engine 3 muxing info
-*/
+ * 0x00 engine 1 program
+ * 0x10 engine 2 program
+ * 0x20 engine 3 program
+ * 0x30 engine 1 muxing info
+ * 0x40 engine 2 muxing info
+ * 0x50 engine 3 muxing info
+ */
#define LP5523_MAX_LEDS9
/* Registers */
@@ -326,7 +326,7 @@ static int lp5523_update_program_memory(struct lp55xx_chip
*chip,
const u8 *data, size_t size)
{
u8 pattern[LP5523_PROGRAM_LENGTH] = {0};
- unsigned cmd;
+ unsigned int cmd;
char c[3];
int nrchars;
int ret;
@@ -468,6 +468,7 @@ static int lp5523_mux_parse(const char *buf, u16 *mux,
size_t len)
static void lp5523_mux_to_array(u16 led_mux, char *array)
{
int i, pos = 0;
+
for (i = 0; i < LP5523_MAX_LEDS; i++)
pos += sprintf(array + pos, "%x", LED_ACTIVE(led_mux, i));
@@ -506,7 +507,7 @@ static int lp5523_load_mux(struct lp55xx_chip *chip, u16
mux, int nr)
if (ret)
return ret;
- ret = lp55xx_write(chip, LP5523_REG_PROG_MEM , (u8)(mux >> 8));
+ ret = lp55xx_write(chip, LP5523_REG_PROG_MEM, (u8)(mux >> 8));
if (ret)
return ret;
--
2.22.0.214.g8dca754b1e
[PATCH v11 02/16] dt-bindings: leds: Add multicolor ID to the color ID list
Add a new color ID that is declared as MULTICOLOR as with the multicolor framework declaring a definitive color is not accurate as the node can contain multiple colors. Signed-off-by: Dan Murphy --- include/dt-bindings/leds/common.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h index 9e1256a7c1bf..7006d15f71e8 100644 --- a/include/dt-bindings/leds/common.h +++ b/include/dt-bindings/leds/common.h @@ -29,7 +29,8 @@ #define LED_COLOR_ID_VIOLET5 #define LED_COLOR_ID_YELLOW6 #define LED_COLOR_ID_IR7 -#define LED_COLOR_ID_MAX 8 +#define LED_COLOR_ID_MULTI 8 +#define LED_COLOR_ID_MAX 9 /* Standard LED functions */ #define LED_FUNCTION_ACTIVITY "activity" -- 2.22.0.214.g8dca754b1e
[PATCH v11 00/16] Multicolor Framework v11
Hello This is v11 of the multicolor framework patchset. This patchet has been tested on LP5523, LP5521, and the LP5012,24 and 36 evms connected to a beagle bone black. For the LP5523/5521 I have tested with both MC and non-MC DT as well as an intermixed DT with color and white LEDs. Changes made based on this series: https://lore.kernel.org/patchwork/project/lkml/list/?series=412400 MC framework: - Added inline functions for when CONFIG_LEDS_CLASS_MULTI_COLOR is not set - Added the output number to the color conversion struct - Fixed devm_* structure function - Fixed permissions for MC files. Also the document. LP50xx: - Added range checking for banked LEDs LP55xx: - Removed grouped_channels and channel_color arrays in favor of the new color coversion structure. - Changed the intensity function to perform all the writing to the device within the childs intensity fn so now the intensity and brightness functions have a similar design but function different. Dan Murphy (16): dt: bindings: Add multicolor class dt bindings documention dt-bindings: leds: Add multicolor ID to the color ID list leds: Add multicolor ID to the color ID list leds: multicolor: Introduce a multicolor class definition dt: bindings: lp50xx: Introduce the lp50xx family of RGB drivers leds: lp50xx: Add the LP50XX family of the RGB LED driver dt: bindings: lp55xx: Be consistent in the document with LED acronym dt: bindings: lp55xx: Update binding for Multicolor Framework ARM: dts: n900: Add reg property to the LP5523 channel node ARM: dts: imx6dl-yapp4: Add reg property to the lp5562 channel node ARM: dts: ste-href: Add reg property to the LP5521 channel nodes leds: lp55xx: Add multicolor framework support to lp55xx leds: lp5523: Update the lp5523 code to add intensity function leds: lp5521: Add multicolor framework intensity support leds: lp55xx: Fix checkpatch file permissions issues leds: lp5523: Fix checkpatch issues in the code .../ABI/testing/sysfs-class-led-multicolor| 35 + .../bindings/leds/leds-class-multicolor.txt | 98 +++ .../devicetree/bindings/leds/leds-lp50xx.txt | 148 .../devicetree/bindings/leds/leds-lp55xx.txt | 155 +++- Documentation/leds/index.rst | 1 + Documentation/leds/leds-class-multicolor.rst | 96 +++ arch/arm/boot/dts/imx6dl-yapp4-common.dtsi| 14 +- arch/arm/boot/dts/omap3-n900.dts | 29 +- arch/arm/boot/dts/ste-href.dtsi | 22 +- drivers/leds/Kconfig | 22 + drivers/leds/Makefile | 2 + drivers/leds/led-class-multicolor.c | 271 ++ drivers/leds/led-core.c | 1 + drivers/leds/leds-lp50xx.c| 799 ++ drivers/leds/leds-lp5521.c| 20 + drivers/leds/leds-lp5523.c| 39 +- drivers/leds/leds-lp55xx-common.c | 198 - drivers/leds/leds-lp55xx-common.h | 11 + include/dt-bindings/leds/common.h | 3 +- include/linux/led-class-multicolor.h | 143 include/linux/platform_data/leds-lp55xx.h | 5 + 21 files changed, 2019 insertions(+), 93 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-class-led-multicolor create mode 100644 Documentation/devicetree/bindings/leds/leds-class-multicolor.txt create mode 100644 Documentation/devicetree/bindings/leds/leds-lp50xx.txt create mode 100644 Documentation/leds/leds-class-multicolor.rst create mode 100644 drivers/leds/led-class-multicolor.c create mode 100644 drivers/leds/leds-lp50xx.c create mode 100644 include/linux/led-class-multicolor.h -- 2.22.0.214.g8dca754b1e
[PATCH v11 15/16] leds: lp55xx: Fix checkpatch file permissions issues
Fix the checkpatch warnings for the use of the file permission macros.
In converting the file permissions to the DEVICE_ATTR_XX macros the
call back function names needed to be updated within the code.
This means that the lp55xx_ needed to be dropped in the name to keep in
harmony with the ABI documentation.
Signed-off-by: Dan Murphy
---
drivers/leds/leds-lp55xx-common.c | 22 ++
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/leds/leds-lp55xx-common.c
b/drivers/leds/leds-lp55xx-common.c
index 85629f836082..fc5224108c64 100644
--- a/drivers/leds/leds-lp55xx-common.c
+++ b/drivers/leds/leds-lp55xx-common.c
@@ -78,7 +78,7 @@ static int lp55xx_post_init_device(struct lp55xx_chip *chip)
return cfg->post_init_device(chip);
}
-static ssize_t lp55xx_show_current(struct device *dev,
+static ssize_t led_current_show(struct device *dev,
struct device_attribute *attr,
char *buf)
{
@@ -87,7 +87,7 @@ static ssize_t lp55xx_show_current(struct device *dev,
return scnprintf(buf, PAGE_SIZE, "%d\n", led->led_current);
}
-static ssize_t lp55xx_store_current(struct device *dev,
+static ssize_t led_current_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t len)
{
@@ -111,7 +111,7 @@ static ssize_t lp55xx_store_current(struct device *dev,
return len;
}
-static ssize_t lp55xx_show_max_current(struct device *dev,
+static ssize_t max_current_show(struct device *dev,
struct device_attribute *attr,
char *buf)
{
@@ -120,9 +120,8 @@ static ssize_t lp55xx_show_max_current(struct device *dev,
return scnprintf(buf, PAGE_SIZE, "%d\n", led->max_current);
}
-static DEVICE_ATTR(led_current, S_IRUGO | S_IWUSR, lp55xx_show_current,
- lp55xx_store_current);
-static DEVICE_ATTR(max_current, S_IRUGO , lp55xx_show_max_current, NULL);
+static DEVICE_ATTR_RW(led_current);
+static DEVICE_ATTR_RO(max_current);
static struct attribute *lp55xx_led_attrs[] = {
&dev_attr_led_current.attr,
@@ -282,7 +281,7 @@ static int lp55xx_request_firmware(struct lp55xx_chip *chip)
GFP_KERNEL, chip, lp55xx_firmware_loaded);
}
-static ssize_t lp55xx_show_engine_select(struct device *dev,
+static ssize_t select_engine_show(struct device *dev,
struct device_attribute *attr,
char *buf)
{
@@ -292,7 +291,7 @@ static ssize_t lp55xx_show_engine_select(struct device *dev,
return sprintf(buf, "%d\n", chip->engine_idx);
}
-static ssize_t lp55xx_store_engine_select(struct device *dev,
+static ssize_t select_engine_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t len)
{
@@ -334,7 +333,7 @@ static inline void lp55xx_run_engine(struct lp55xx_chip
*chip, bool start)
chip->cfg->run_engine(chip, start);
}
-static ssize_t lp55xx_store_engine_run(struct device *dev,
+static ssize_t run_engine_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t len)
{
@@ -359,9 +358,8 @@ static ssize_t lp55xx_store_engine_run(struct device *dev,
return len;
}
-static DEVICE_ATTR(select_engine, S_IRUGO | S_IWUSR,
- lp55xx_show_engine_select, lp55xx_store_engine_select);
-static DEVICE_ATTR(run_engine, S_IWUSR, NULL, lp55xx_store_engine_run);
+static DEVICE_ATTR_RW(select_engine);
+static DEVICE_ATTR_WO(run_engine);
static struct attribute *lp55xx_engine_attributes[] = {
&dev_attr_select_engine.attr,
--
2.22.0.214.g8dca754b1e
[PATCH v11 10/16] ARM: dts: imx6dl-yapp4: Add reg property to the lp5562 channel node
Add the reg property to each channel node. This update is
to accomodate the multicolor framework. In addition to the
accomodation this allows the LEDs to be placed on any channel
and allow designs to skip channels as opposed to requiring
sequential order.
Signed-off-by: Dan Murphy
CC: Shawn Guo
CC: Sascha Hauer
CC: Pengutronix Kernel Team
CC: Fabio Estevam
CC: NXP Linux Team
---
arch/arm/boot/dts/imx6dl-yapp4-common.dtsi | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/imx6dl-yapp4-common.dtsi
b/arch/arm/boot/dts/imx6dl-yapp4-common.dtsi
index e8d800fec637..efc466ed1fea 100644
--- a/arch/arm/boot/dts/imx6dl-yapp4-common.dtsi
+++ b/arch/arm/boot/dts/imx6dl-yapp4-common.dtsi
@@ -257,29 +257,35 @@
reg = <0x30>;
clock-mode = /bits/ 8 <1>;
status = "disabled";
+ #address-cells = <1>;
+ #size-cells = <0>;
- chan0 {
+ chan@0 {
chan-name = "R";
led-cur = /bits/ 8 <0x20>;
max-cur = /bits/ 8 <0x60>;
+ reg = <0>;
};
- chan1 {
+ chan@1 {
chan-name = "G";
led-cur = /bits/ 8 <0x20>;
max-cur = /bits/ 8 <0x60>;
+ reg = <1>;
};
- chan2 {
+ chan@2 {
chan-name = "B";
led-cur = /bits/ 8 <0x20>;
max-cur = /bits/ 8 <0x60>;
+ reg = <2>;
};
- chan3 {
+ chan@3 {
chan-name = "W";
led-cur = /bits/ 8 <0x0>;
max-cur = /bits/ 8 <0x0>;
+ reg = <3>;
};
};
--
2.22.0.214.g8dca754b1e
[PATCH] x86/hpet: Fix uninitialized use in hpet_msi_resume()
Inside function hpet_msi_resume(), variable "msg" could be uninitialized if irq_chip_compose_msi_msg() returns -ENOSYS. However, it is directly used in hpet_msi_write(), which is potentially unsafe. Signed-off-by: Yizhuo --- arch/x86/kernel/hpet.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c index c6f791bc481e..5bc76819dd32 100644 --- a/arch/x86/kernel/hpet.c +++ b/arch/x86/kernel/hpet.c @@ -501,6 +501,7 @@ static int hpet_clkevt_msi_resume(struct clock_event_device *evt) struct irq_data *data = irq_get_irq_data(hc->irq); struct msi_msg msg; + memset(&msg, 0, sizeof(struct msi_msg)); /* Restore the MSI msg and unmask the interrupt */ irq_chip_compose_msi_msg(data, &msg); hpet_msi_write(hc, &msg); -- 2.17.1
