* [PATCH 0/1] Add IMR driver for Keem Bay @ 2020-04-21 16:36 Daniele Alessandrelli 2020-04-21 16:36 ` [PATCH 1/1] soc: keembay: Add Keem Bay IMR driver Daniele Alessandrelli 2020-04-30 19:49 ` [PATCH 0/1] Add IMR driver for Keem Bay Alessandrelli, Daniele 0 siblings, 2 replies; 19+ messages in thread From: Daniele Alessandrelli @ 2020-04-21 16:36 UTC (permalink / raw) To: linux-kernel Cc: Greg Kroah-Hartman, Rob Herring, Andy Shevchenko, Daniele Alessandrelli, Paul J Murphy The following is a patch for a new Intel Movidius SoC, code-named Keem Bay. Keem Bay needs a driver to disable the Isolated Memory Region (IMR) set up by the SoC bootloader during early boot. If such an IMR is not disabled and some device tries to access it, the system will reboot. Since this driver is SoC-specific and Keem Bay is a new SoC, I was unsure of where to put this driver. In the end I decided to create a new 'keembay' directory in 'drivers/soc'. I hope that's reasonable, if not, just let me know. Daniele Alessandrelli (1): soc: keembay: Add Keem Bay IMR driver MAINTAINERS | 5 ++++ drivers/soc/Kconfig | 1 + drivers/soc/Makefile | 1 + drivers/soc/keembay/Kconfig | 22 +++++++++++++++++ drivers/soc/keembay/Makefile | 5 ++++ drivers/soc/keembay/keembay-imr.c | 40 +++++++++++++++++++++++++++++++ 6 files changed, 74 insertions(+) create mode 100644 drivers/soc/keembay/Kconfig create mode 100644 drivers/soc/keembay/Makefile create mode 100644 drivers/soc/keembay/keembay-imr.c -- 2.21.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/1] soc: keembay: Add Keem Bay IMR driver 2020-04-21 16:36 [PATCH 0/1] Add IMR driver for Keem Bay Daniele Alessandrelli @ 2020-04-21 16:36 ` Daniele Alessandrelli 2020-05-01 8:10 ` Greg Kroah-Hartman 2020-05-07 17:44 ` Pavel Machek 2020-04-30 19:49 ` [PATCH 0/1] Add IMR driver for Keem Bay Alessandrelli, Daniele 1 sibling, 2 replies; 19+ messages in thread From: Daniele Alessandrelli @ 2020-04-21 16:36 UTC (permalink / raw) To: linux-kernel Cc: Greg Kroah-Hartman, Rob Herring, Andy Shevchenko, Daniele Alessandrelli, Paul J Murphy From: Daniele Alessandrelli <daniele.alessandrelli@intel.com> Keem Bay bootloader sets up a temporary Isolated Memory Region (IMR) to protect itself during pre-Linux boot. This temporary IMR remains active even when control is passed to the Linux Kernel. It is Kernel responsibility to remove such an IMR during initialization. This driver adds such functionality. The driver is loaded during `early_init`, which should ensure that the IMR is removed before devices that may try to access the IMR are initialized. Signed-off-by: Daniele Alessandrelli <daniele.alessandrelli@intel.com> --- MAINTAINERS | 5 ++++ drivers/soc/Kconfig | 1 + drivers/soc/Makefile | 1 + drivers/soc/keembay/Kconfig | 22 +++++++++++++++++ drivers/soc/keembay/Makefile | 5 ++++ drivers/soc/keembay/keembay-imr.c | 40 +++++++++++++++++++++++++++++++ 6 files changed, 74 insertions(+) create mode 100644 drivers/soc/keembay/Kconfig create mode 100644 drivers/soc/keembay/Makefile create mode 100644 drivers/soc/keembay/keembay-imr.c diff --git a/MAINTAINERS b/MAINTAINERS index b816a453b10e..59f1923a0f25 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9194,6 +9194,11 @@ S: Maintained W: http://lse.sourceforge.net/kdump/ F: Documentation/admin-guide/kdump/ +KEEMBAY IMR +M: Daniele Alessandrelli <daniele.alessandrelli@intel.com> +S: Maintained +F: drivers/soc/keembay/keembay-imr.c + KEENE FM RADIO TRANSMITTER DRIVER M: Hans Verkuil <hverkuil@xs4all.nl> L: linux-media@vger.kernel.org diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig index 425ab6f7e375..eeeba3ef7338 100644 --- a/drivers/soc/Kconfig +++ b/drivers/soc/Kconfig @@ -9,6 +9,7 @@ source "drivers/soc/bcm/Kconfig" source "drivers/soc/fsl/Kconfig" source "drivers/soc/imx/Kconfig" source "drivers/soc/ixp4xx/Kconfig" +source "drivers/soc/keembay/Kconfig" source "drivers/soc/mediatek/Kconfig" source "drivers/soc/qcom/Kconfig" source "drivers/soc/renesas/Kconfig" diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile index 36452bed86ef..65c981207283 100644 --- a/drivers/soc/Makefile +++ b/drivers/soc/Makefile @@ -13,6 +13,7 @@ obj-y += fsl/ obj-$(CONFIG_ARCH_GEMINI) += gemini/ obj-y += imx/ obj-$(CONFIG_ARCH_IXP4XX) += ixp4xx/ +obj-y += keembay/ obj-$(CONFIG_SOC_XWAY) += lantiq/ obj-y += mediatek/ obj-y += amlogic/ diff --git a/drivers/soc/keembay/Kconfig b/drivers/soc/keembay/Kconfig new file mode 100644 index 000000000000..2161bce131b3 --- /dev/null +++ b/drivers/soc/keembay/Kconfig @@ -0,0 +1,22 @@ +# SPDX-License-Identifier: GPL-2.0-only +# +# Keem Bay SoC drivers. +# + +menu "Keem Bay SoC drivers" + +config KEEMBAY_IMR + bool "Clean-up Keem Bay bootloader IMR at boot" + depends on ARM64 + help + This option makes the Kernel clean up the Isolated Memory Region + (IMR) set up by Keem Bay bootloader (U-boot) to protect itself during + early boot. + + The IMR number to be cleaned up is taken from the device tree + (property 'u-boot-imr' of the 'chosen' node). + + If you are compiling the Kernel for a Keem Bay SoC select Y, + otherwise select N. + +endmenu diff --git a/drivers/soc/keembay/Makefile b/drivers/soc/keembay/Makefile new file mode 100644 index 000000000000..dacfdb9f5fc1 --- /dev/null +++ b/drivers/soc/keembay/Makefile @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0-only +# +# Makefile for Keem Bay SoC drivers. +# +obj-$(CONFIG_KEEMBAY_IMR) += keembay-imr.o diff --git a/drivers/soc/keembay/keembay-imr.c b/drivers/soc/keembay/keembay-imr.c new file mode 100644 index 000000000000..eabbdd6e69a7 --- /dev/null +++ b/drivers/soc/keembay/keembay-imr.c @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2019-2020 Intel Corporation + */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include <linux/arm-smccc.h> +#include <linux/init.h> +#include <linux/of.h> +#include <linux/printk.h> +#include <linux/types.h> + +/* Keem Bay SiP SVC for clearing an IMR. */ +#define KMB_SIP_SVC_IMR_CLEAR 0x8200ff13 + +static int __init clear_imr(u64 imr) +{ + struct arm_smccc_res res = { 0 }; + + arm_smccc_smc(KMB_SIP_SVC_IMR_CLEAR, imr, 0, 0, 0, 0, 0, 0, &res); + + return res.a0; +} + +static int __init kmb_imr_init(void) +{ + u32 imr; + int rc; + + rc = of_property_read_u32(of_chosen, "u-boot-imr", &imr); + if (rc) { + pr_warn("Skipping IMR clean-up: No U-Boot IMR defined in device tree\n"); + return 0; + } + pr_info("Disabling Keem Bay U-boot IMR: %u\n", imr); + + return clear_imr(imr); +} +early_initcall(kmb_imr_init); -- 2.21.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 1/1] soc: keembay: Add Keem Bay IMR driver 2020-04-21 16:36 ` [PATCH 1/1] soc: keembay: Add Keem Bay IMR driver Daniele Alessandrelli @ 2020-05-01 8:10 ` Greg Kroah-Hartman 2020-05-01 11:50 ` Daniele Alessandrelli 2020-05-07 17:44 ` Pavel Machek 1 sibling, 1 reply; 19+ messages in thread From: Greg Kroah-Hartman @ 2020-05-01 8:10 UTC (permalink / raw) To: Daniele Alessandrelli Cc: linux-kernel, Rob Herring, Andy Shevchenko, Daniele Alessandrelli, Paul J Murphy On Tue, Apr 21, 2020 at 05:36:18PM +0100, Daniele Alessandrelli wrote: > From: Daniele Alessandrelli <daniele.alessandrelli@intel.com> > > Keem Bay bootloader sets up a temporary Isolated Memory Region (IMR) to > protect itself during pre-Linux boot. > > This temporary IMR remains active even when control is passed to the > Linux Kernel. It is Kernel responsibility to remove such an IMR during > initialization. > > This driver adds such functionality. > > The driver is loaded during `early_init`, which should ensure that the > IMR is removed before devices that may try to access the IMR are > initialized. > > Signed-off-by: Daniele Alessandrelli <daniele.alessandrelli@intel.com> First off, there is a "proper" way to send patches to the kernel community that Intel has that I do not think you are following. Please work with the Intel "Linux group" to do that first, as odds are they would have caught all of these issues beforehand (hint, which is why that process is in place...) > --- > MAINTAINERS | 5 ++++ > drivers/soc/Kconfig | 1 + > drivers/soc/Makefile | 1 + > drivers/soc/keembay/Kconfig | 22 +++++++++++++++++ > drivers/soc/keembay/Makefile | 5 ++++ > drivers/soc/keembay/keembay-imr.c | 40 +++++++++++++++++++++++++++++++ > 6 files changed, 74 insertions(+) > create mode 100644 drivers/soc/keembay/Kconfig > create mode 100644 drivers/soc/keembay/Makefile > create mode 100644 drivers/soc/keembay/keembay-imr.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index b816a453b10e..59f1923a0f25 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -9194,6 +9194,11 @@ S: Maintained > W: http://lse.sourceforge.net/kdump/ > F: Documentation/admin-guide/kdump/ > > +KEEMBAY IMR > +M: Daniele Alessandrelli <daniele.alessandrelli@intel.com> > +S: Maintained > +F: drivers/soc/keembay/keembay-imr.c > + > KEENE FM RADIO TRANSMITTER DRIVER > M: Hans Verkuil <hverkuil@xs4all.nl> > L: linux-media@vger.kernel.org > diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig > index 425ab6f7e375..eeeba3ef7338 100644 > --- a/drivers/soc/Kconfig > +++ b/drivers/soc/Kconfig > @@ -9,6 +9,7 @@ source "drivers/soc/bcm/Kconfig" > source "drivers/soc/fsl/Kconfig" > source "drivers/soc/imx/Kconfig" > source "drivers/soc/ixp4xx/Kconfig" > +source "drivers/soc/keembay/Kconfig" For a single 40 line driver, do not make a new directory. If you end up with lots in the future, then just move the files. Don't over-engineer we have no idea what will actually happen in the future. > source "drivers/soc/mediatek/Kconfig" > source "drivers/soc/qcom/Kconfig" > source "drivers/soc/renesas/Kconfig" > diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile > index 36452bed86ef..65c981207283 100644 > --- a/drivers/soc/Makefile > +++ b/drivers/soc/Makefile > @@ -13,6 +13,7 @@ obj-y += fsl/ > obj-$(CONFIG_ARCH_GEMINI) += gemini/ > obj-y += imx/ > obj-$(CONFIG_ARCH_IXP4XX) += ixp4xx/ > +obj-y += keembay/ > obj-$(CONFIG_SOC_XWAY) += lantiq/ > obj-y += mediatek/ > obj-y += amlogic/ > diff --git a/drivers/soc/keembay/Kconfig b/drivers/soc/keembay/Kconfig > new file mode 100644 > index 000000000000..2161bce131b3 > --- /dev/null > +++ b/drivers/soc/keembay/Kconfig > @@ -0,0 +1,22 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +# > +# Keem Bay SoC drivers. > +# > + > +menu "Keem Bay SoC drivers" > + > +config KEEMBAY_IMR > + bool "Clean-up Keem Bay bootloader IMR at boot" > + depends on ARM64 > + help > + This option makes the Kernel clean up the Isolated Memory Region > + (IMR) set up by Keem Bay bootloader (U-boot) to protect itself during > + early boot. > + > + The IMR number to be cleaned up is taken from the device tree > + (property 'u-boot-imr' of the 'chosen' node). > + > + If you are compiling the Kernel for a Keem Bay SoC select Y, > + otherwise select N. No module support? What about kernels that support more than one ARM device in the same kernel, you just broke that :( > + > +endmenu > diff --git a/drivers/soc/keembay/Makefile b/drivers/soc/keembay/Makefile > new file mode 100644 > index 000000000000..dacfdb9f5fc1 > --- /dev/null > +++ b/drivers/soc/keembay/Makefile > @@ -0,0 +1,5 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +# > +# Makefile for Keem Bay SoC drivers. > +# > +obj-$(CONFIG_KEEMBAY_IMR) += keembay-imr.o > diff --git a/drivers/soc/keembay/keembay-imr.c b/drivers/soc/keembay/keembay-imr.c > new file mode 100644 > index 000000000000..eabbdd6e69a7 > --- /dev/null > +++ b/drivers/soc/keembay/keembay-imr.c > @@ -0,0 +1,40 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (C) 2019-2020 Intel Corporation > + */ > + > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > + > +#include <linux/arm-smccc.h> > +#include <linux/init.h> > +#include <linux/of.h> > +#include <linux/printk.h> > +#include <linux/types.h> > + > +/* Keem Bay SiP SVC for clearing an IMR. */ > +#define KMB_SIP_SVC_IMR_CLEAR 0x8200ff13 > + > +static int __init clear_imr(u64 imr) > +{ > + struct arm_smccc_res res = { 0 }; > + > + arm_smccc_smc(KMB_SIP_SVC_IMR_CLEAR, imr, 0, 0, 0, 0, 0, 0, &res); > + > + return res.a0; > +} > + > +static int __init kmb_imr_init(void) > +{ > + u32 imr; > + int rc; > + > + rc = of_property_read_u32(of_chosen, "u-boot-imr", &imr); Is there a device tree documetnation for this attribute? You have to have that approved before you can get your code merged. > + if (rc) { > + pr_warn("Skipping IMR clean-up: No U-Boot IMR defined in device tree\n"); > + return 0; > + } > + pr_info("Disabling Keem Bay U-boot IMR: %u\n", imr); Drivers are quiet if all is good, don't be noisy for no reason. > + > + return clear_imr(imr); Did that really need to be a separate function? > +} > +early_initcall(kmb_imr_init); Like I said above, you just broke multi-system kernels by always trying to do this. Trigger off of a hardware device that only your platform has in order to properly load and run. As-is, you don't want to do this. Anyway, Intel owes me more liquor for this review as the resources you had to get review of this were obviously not taken advantage of. greg k-h ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/1] soc: keembay: Add Keem Bay IMR driver 2020-05-01 8:10 ` Greg Kroah-Hartman @ 2020-05-01 11:50 ` Daniele Alessandrelli 2020-05-01 11:59 ` Greg Kroah-Hartman 2020-05-24 21:28 ` Pavel Machek 0 siblings, 2 replies; 19+ messages in thread From: Daniele Alessandrelli @ 2020-05-01 11:50 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: linux-kernel, Rob Herring, Andy Shevchenko, Paul J Murphy, Arnd Bergmann Thanks for your feedback. > > First off, there is a "proper" way to send patches to the kernel > community that Intel has that I do not think you are > following. Please > work with the Intel "Linux group" to do that first, as odds are they > would have caught all of these issues beforehand (hint, which is why > that process is in place...) > I actually followed that process and got the OK to try to upstream. I think the issues you identified went uncaught mainly due to the limited Linux ARM expertise within that group (and Intel in general). Anyway, I'll keep working with our Linux Kernel people to try to reduce the burden on you and the other kernel maintainers. > > diff --git a/drivers/soc/keembay/Kconfig > > b/drivers/soc/keembay/Kconfig > > new file mode 100644 > > index 000000000000..2161bce131b3 > > --- /dev/null > > +++ b/drivers/soc/keembay/Kconfig > > @@ -0,0 +1,22 @@ > > +# SPDX-License-Identifier: GPL-2.0-only > > +# > > +# Keem Bay SoC drivers. > > +# > > + > > +menu "Keem Bay SoC drivers" > > + > > +config KEEMBAY_IMR > > + bool "Clean-up Keem Bay bootloader IMR at boot" > > + depends on ARM64 > > + help > > + This option makes the Kernel clean up the Isolated Memory > > Region > > + (IMR) set up by Keem Bay bootloader (U-boot) to protect > > itself during > > + early boot. > > + > > + The IMR number to be cleaned up is taken from the device tree > > + (property 'u-boot-imr' of the 'chosen' node). > > + > > + If you are compiling the Kernel for a Keem Bay SoC select Y, > > + otherwise select N. > > No module support? > > What about kernels that support more than one ARM device in the same > kernel, you just broke that :( > <cut> > > > +} > > +early_initcall(kmb_imr_init); > > Like I said above, you just broke multi-system kernels by always > trying > to do this. Trigger off of a hardware device that only your platform > has in order to properly load and run. As-is, you don't want to do > this. My bad, I didn't consider the issue of multi-platform ARM kernels. The problem is that I need this code to be run early at boot, so I don't think I can make this a module. But I'm sure there is a proper way to achieve what I need without breaking multi-system kernels. I'll do my homework and try to find it. > > Anyway, Intel owes me more liquor for this review as the resources > you > had to get review of this were obviously not taken advantage of. > > greg k-h ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/1] soc: keembay: Add Keem Bay IMR driver 2020-05-01 11:50 ` Daniele Alessandrelli @ 2020-05-01 11:59 ` Greg Kroah-Hartman 2020-05-24 21:28 ` Pavel Machek 1 sibling, 0 replies; 19+ messages in thread From: Greg Kroah-Hartman @ 2020-05-01 11:59 UTC (permalink / raw) To: Daniele Alessandrelli Cc: linux-kernel, Rob Herring, Andy Shevchenko, Paul J Murphy, Arnd Bergmann On Fri, May 01, 2020 at 12:50:36PM +0100, Daniele Alessandrelli wrote: > Thanks for your feedback. > > > > > First off, there is a "proper" way to send patches to the kernel > > community that Intel has that I do not think you are > > following. Please > > work with the Intel "Linux group" to do that first, as odds are they > > would have caught all of these issues beforehand (hint, which is why > > that process is in place...) > > > > I actually followed that process and got the OK to try to upstream. Then someone seriously steered you wrong as I know of one specific thing you did incorrectly (hint, you needed to get them to sign off on the patch...) > I think the issues you identified went uncaught mainly due to the > limited Linux ARM expertise within that group (and Intel in general). No, they should know better, if not, there are bigger problems there :( greg k-h ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/1] soc: keembay: Add Keem Bay IMR driver 2020-05-01 11:50 ` Daniele Alessandrelli 2020-05-01 11:59 ` Greg Kroah-Hartman @ 2020-05-24 21:28 ` Pavel Machek 2020-05-25 8:18 ` Arnd Bergmann 1 sibling, 1 reply; 19+ messages in thread From: Pavel Machek @ 2020-05-24 21:28 UTC (permalink / raw) To: Daniele Alessandrelli Cc: Greg Kroah-Hartman, linux-kernel, Rob Herring, Andy Shevchenko, Paul J Murphy, Arnd Bergmann Hi! > > Like I said above, you just broke multi-system kernels by always > > trying > > to do this. Trigger off of a hardware device that only your platform > > has in order to properly load and run. As-is, you don't want to do > > this. > > My bad, I didn't consider the issue of multi-platform ARM kernels. > > The problem is that I need this code to be run early at boot, so I > don't think I can make this a module. How early is early enough? What bootloader are you using? I believe you should simply fix your bootloader not to pass locked memory to the kernel. Alternatively, take that memory off the "memory available" maps, and only re-add it once it is usable. Anything else is dangerous. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/1] soc: keembay: Add Keem Bay IMR driver 2020-05-24 21:28 ` Pavel Machek @ 2020-05-25 8:18 ` Arnd Bergmann 2020-05-27 13:31 ` Alessandrelli, Daniele 0 siblings, 1 reply; 19+ messages in thread From: Arnd Bergmann @ 2020-05-25 8:18 UTC (permalink / raw) To: Pavel Machek Cc: Daniele Alessandrelli, Greg Kroah-Hartman, linux-kernel, Rob Herring, Andy Shevchenko, Paul J Murphy On Sun, May 24, 2020 at 11:28 PM Pavel Machek <pavel@ucw.cz> wrote: > > Hi! > > > > Like I said above, you just broke multi-system kernels by always > > > trying > > > to do this. Trigger off of a hardware device that only your platform > > > has in order to properly load and run. As-is, you don't want to do > > > this. > > > > My bad, I didn't consider the issue of multi-platform ARM kernels. > > > > The problem is that I need this code to be run early at boot, so I > > don't think I can make this a module. > > How early is early enough? > > What bootloader are you using? > > I believe you should simply fix your bootloader not to pass locked memory to the kernel. > > Alternatively, take that memory off the "memory available" maps, and only re-add it once > it is usable. > > Anything else is dangerous. Agreed, this sounds like an incompatible extension of the boot protocol that we should otherwise not merge. However, there is also a lot of missing information here, and it is always possible they are trying to something for a good reason. As long as the problem that the bootloader is trying to solve is explained well enough in the changelog, we can discuss it to see how it should be done properly. Arnd ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/1] soc: keembay: Add Keem Bay IMR driver 2020-05-25 8:18 ` Arnd Bergmann @ 2020-05-27 13:31 ` Alessandrelli, Daniele 2020-05-27 14:33 ` Arnd Bergmann 2020-05-28 11:22 ` Pavel Machek 0 siblings, 2 replies; 19+ messages in thread From: Alessandrelli, Daniele @ 2020-05-27 13:31 UTC (permalink / raw) To: pavel, arnd Cc: robh, Murphy, Paul J, gregkh, Shevchenko, Andriy, linux-kernel, daniele.alessandrelli Hi Arnd, Pavel, Thanks for your feedback. > > > > > > The problem is that I need this code to be run early at boot, so > > > I > > > don't think I can make this a module. > > > > How early is early enough? Basically, before any device with direct memory access is activated (because if anybody, except the ARM CPU, tries to access that memory, the memory protection mechanism will be triggered and a reboot will happen). > > > > What bootloader are you using? U-Boot > > > > I believe you should simply fix your bootloader not to pass locked > > memory to the kernel. The bootloader is behaving like that for security reasons, so we'd like to avoid modifying it. I'll provide more information below. > > > > Alternatively, take that memory off the "memory available" maps, > > and only re-add it once > > it is usable. > > > > Anything else is dangerous. That sounds like an interesting solution, thanks! Do you know any code that I can use as a reference? Since, the protected memory is just a few megabytes large, I guess that in theory we could just have U-Boot mark it as reserved memory and forget about it; but if there's a way to re-add it back once in Linux that would be great. > > Agreed, this sounds like an incompatible extension of the boot > protocol > that we should otherwise not merge. > > However, there is also a lot of missing information here, and it is > always > possible they are trying to something for a good reason. As long as > the > problem that the bootloader is trying to solve is explained well > enough > in the changelog, we can discuss it to see how it should be done > properly. Apologies, I should have provided more information. Here it is :) Basically, at boot time U-Boot code and core memory (.text, .data, .bss, etc.) is protected by this Isolated Memory Region (IMR) which prevents any device or processing units other than the ARM CPU to access/modify the memory. This is done for security reasons, to reduce the risks that a potential attacker can use "hijacked" HW devices to interfere with the boot process (and break the secure boot flow in place). Before booting the Kernel, U-Boot sets up a new IMR to protect the Kernel image (so that the kernel can benefit of a similar protection) and then starts the kernel, delegating to the kernel the task of switching off the U-Boot IMR. U-Boot doesn't turn off its own IMR because doing so would leave a (tiny) window in which the boot execution flow is not protected. If you have any additional questions or feedback, just let me know. Regards, Daniele -------------------------------------------------------------- Intel Research and Development Ireland Limited Registered in Ireland Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/1] soc: keembay: Add Keem Bay IMR driver 2020-05-27 13:31 ` Alessandrelli, Daniele @ 2020-05-27 14:33 ` Arnd Bergmann 2020-05-27 17:43 ` Daniele Alessandrelli 2020-05-28 11:22 ` Pavel Machek 1 sibling, 1 reply; 19+ messages in thread From: Arnd Bergmann @ 2020-05-27 14:33 UTC (permalink / raw) To: Alessandrelli, Daniele Cc: pavel, robh, Murphy, Paul J, gregkh, Shevchenko, Andriy, linux-kernel, daniele.alessandrelli On Wed, May 27, 2020 at 3:31 PM Alessandrelli, Daniele <daniele.alessandrelli@intel.com> wrote: > > > Alternatively, take that memory off the "memory available" maps, > > > and only re-add it once > > > it is usable. > > > > > > Anything else is dangerous. > > That sounds like an interesting solution, thanks! > > Do you know any code that I can use as a reference? > > Since, the protected memory is just a few megabytes large, I guess that > in theory we could just have U-Boot mark it as reserved memory and > forget about it; but if there's a way to re-add it back once in Linux > that would be great. Adding it back later on with a loadable device driver should not be a problem, as this is not a violation of the boot protocol. > > Agreed, this sounds like an incompatible extension of the boot > > protocol that we should otherwise not merge. > > > > However, there is also a lot of missing information here, and it is > > always possible they are trying to something for a good reason. > > As long as the problem that the bootloader is trying to solve is > > explained well enough in the changelog, we can discuss it to see > > how it should be done properly. > > Apologies, I should have provided more information. Here it is :) > > Basically, at boot time U-Boot code and core memory (.text, .data, > .bss, etc.) is protected by this Isolated Memory Region (IMR) which > prevents any device or processing units other than the ARM CPU to > access/modify the memory. > > This is done for security reasons, to reduce the risks that a potential > attacker can use "hijacked" HW devices to interfere with the boot > process (and break the secure boot flow in place). > > Before booting the Kernel, U-Boot sets up a new IMR to protect the > Kernel image (so that the kernel can benefit of a similar protection) > and then starts the kernel, delegating to the kernel the task of > switching off the U-Boot IMR. > > U-Boot doesn't turn off its own IMR because doing so would leave a > (tiny) window in which the boot execution flow is not protected. > > If you have any additional questions or feedback, just let me know. Thank you for the detailed explanation. I've never seen this done in the Linux boot flow, but I can see how it helps prevent certain kinds of attacks. It seems that just reserving the u-boot area and optionally freeing it later from a driver solves most of your problem. I have one related question though: if the kernel itself is protected, does that mean that any driver that does a DMA to statically allocated memory inside of the kernel is broken as well? I think this is something that a couple of USB drivers do, though it is fairly rare. Does u-boot protect both only the executable sections of the kernel or also data, and does the hardware block both read and write on the IMR, or just writes? Arnd ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/1] soc: keembay: Add Keem Bay IMR driver 2020-05-27 14:33 ` Arnd Bergmann @ 2020-05-27 17:43 ` Daniele Alessandrelli 2020-05-27 18:59 ` Arnd Bergmann 0 siblings, 1 reply; 19+ messages in thread From: Daniele Alessandrelli @ 2020-05-27 17:43 UTC (permalink / raw) To: Arnd Bergmann Cc: pavel, robh, Murphy, Paul J, gregkh, Shevchenko, Andriy, linux-kernel On Wed, 2020-05-27 at 16:33 +0200, Arnd Bergmann wrote: > On Wed, May 27, 2020 at 3:31 PM Alessandrelli, Daniele > <daniele.alessandrelli@intel.com> wrote: > > > > Alternatively, take that memory off the "memory available" > > > > maps, > > > > and only re-add it once > > > > it is usable. > > > > > > > > Anything else is dangerous. > > > > That sounds like an interesting solution, thanks! > > > > Do you know any code that I can use as a reference? > > > > Since, the protected memory is just a few megabytes large, I guess > > that > > in theory we could just have U-Boot mark it as reserved memory and > > forget about it; but if there's a way to re-add it back once in > > Linux > > that would be great. > > Adding it back later on with a loadable device driver should > not be a problem, as this is not a violation of the boot protocol. Cool, I'll try to do that then, thanks! I see two ways to do that though: 1. Create a device driver that gets a reference to the memory region from its DT node and then re-adds the memory to the pool of available memory. 2. Use a special "compatible" string for my memory region and create a driver to handle it. However, I think that in the second case the driver must be builtin. Would that be okay? Also, from a quick look, it seems that I can re-add that memory back by calling memblock_free() (or a similar memblock_* function). Am I on the right track? > > > > Agreed, this sounds like an incompatible extension of the boot > > > protocol that we should otherwise not merge. > > > > > > However, there is also a lot of missing information here, and it > > > is > > > always possible they are trying to something for a good reason. > > > As long as the problem that the bootloader is trying to solve is > > > explained well enough in the changelog, we can discuss it to see > > > how it should be done properly. > > > > Apologies, I should have provided more information. Here it is :) > > > > Basically, at boot time U-Boot code and core memory (.text, .data, > > .bss, etc.) is protected by this Isolated Memory Region (IMR) which > > prevents any device or processing units other than the ARM CPU to > > access/modify the memory. > > > > This is done for security reasons, to reduce the risks that a > > potential > > attacker can use "hijacked" HW devices to interfere with the boot > > process (and break the secure boot flow in place). > > > > Before booting the Kernel, U-Boot sets up a new IMR to protect the > > Kernel image (so that the kernel can benefit of a similar > > protection) > > and then starts the kernel, delegating to the kernel the task of > > switching off the U-Boot IMR. > > > > U-Boot doesn't turn off its own IMR because doing so would leave a > > (tiny) window in which the boot execution flow is not protected. > > > > If you have any additional questions or feedback, just let me know. > > Thank you for the detailed explanation. I've never seen this done > in the Linux boot flow, but I can see how it helps prevent certain > kinds of attacks. > > It seems that just reserving the u-boot area and optionally > freeing it later from a driver solves most of your problem. > I have one related question though: if the kernel itself is > protected, does that mean that any driver that does a DMA > to statically allocated memory inside of the kernel is broken > as well? I think this is something that a couple of USB drivers > do, though it is fairly rare. Does u-boot protect both only > the executable sections of the kernel or also data, and does > the hardware block both read and write on the IMR, or just > writes? Yes, you're very right. Drivers that do DMA transfers to statically allocated memory inside the kernel will be broken. We are currently seeing this with our eMMC driver. Current solution is to add the eMMC controller to the list of allowed "agents" for the IMR. This will reduce the level of protection, but since we expect to have only a few of these exceptions (since, as you pointed out, drivers that do DMA to static kernel memory seem to be quite rare), we think that there is still value in having the Kernel IMR. Do you see any issue with this? Regards, Daniele ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/1] soc: keembay: Add Keem Bay IMR driver 2020-05-27 17:43 ` Daniele Alessandrelli @ 2020-05-27 18:59 ` Arnd Bergmann 2020-05-28 12:27 ` Daniele Alessandrelli 0 siblings, 1 reply; 19+ messages in thread From: Arnd Bergmann @ 2020-05-27 18:59 UTC (permalink / raw) To: Daniele Alessandrelli Cc: pavel, robh, Murphy, Paul J, gregkh, Shevchenko, Andriy, linux-kernel On Wed, May 27, 2020 at 7:43 PM Daniele Alessandrelli <daniele.alessandrelli@linux.intel.com> wrote: > On Wed, 2020-05-27 at 16:33 +0200, Arnd Bergmann wrote: > > On Wed, May 27, 2020 at 3:31 PM Alessandrelli, Daniele <daniele.alessandrelli@intel.com> wrote: > > > > Adding it back later on with a loadable device driver should > > not be a problem, as this is not a violation of the boot protocol. > > Cool, I'll try to do that then, thanks! > > I see two ways to do that though: > > 1. Create a device driver that gets a reference to the memory region > from its DT node and then re-adds the memory to the pool of available > memory. > > 2. Use a special "compatible" string for my memory region and create a > driver to handle it. I think the first approach is more common. > However, I think that in the second case the driver must be builtin. > Would that be okay? It's better to avoid that. > Also, from a quick look, it seems that I can re-add that memory back by > calling memblock_free() (or a similar memblock_* function). Am I on the > right track? I'm not sure if memblock_free() works after early memory initialization is complete, but I think there is some way to do it later. Maybe try memblock_free() first, and then look for something else if it doesn't work. > > It seems that just reserving the u-boot area and optionally > > freeing it later from a driver solves most of your problem. > > I have one related question though: if the kernel itself is > > protected, does that mean that any driver that does a DMA > > to statically allocated memory inside of the kernel is broken > > as well? I think this is something that a couple of USB drivers > > do, though it is fairly rare. Does u-boot protect both only > > the executable sections of the kernel or also data, and does > > the hardware block both read and write on the IMR, or just > > writes? > > Yes, you're very right. Drivers that do DMA transfers to statically > allocated memory inside the kernel will be broken. > > We are currently seeing this with our eMMC driver. > > Current solution is to add the eMMC controller to the list of allowed > "agents" for the IMR. This will reduce the level of protection, but > since we expect to have only a few of these exceptions (since, as you > pointed out, drivers that do DMA to static kernel memory seem to be > quite rare), we think that there is still value in having the Kernel > IMR. > > Do you see any issue with this? I think you should try to fix the driver rather than making an exception for it. Hot-pluggable drivers are a much more interesting case I think, because on the one hand you have no idea what users might want to plug in legitimately, but on the other hand those are also the most likely attack vectors (driver bugs for random USB drivers overwriting kernel memory when faced with malicious hardware) that this feature is trying to prevent. I also wonder whether we should do something in the normal iommu code that prevents one from mapping a page that the kernel would consider as protected (kernel .text, freed memory, ...) into the iommu in the first place. Arnd ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/1] soc: keembay: Add Keem Bay IMR driver 2020-05-27 18:59 ` Arnd Bergmann @ 2020-05-28 12:27 ` Daniele Alessandrelli 0 siblings, 0 replies; 19+ messages in thread From: Daniele Alessandrelli @ 2020-05-28 12:27 UTC (permalink / raw) To: Arnd Bergmann Cc: pavel, robh, Murphy, Paul J, gregkh, Shevchenko, Andriy, linux-kernel On Wed, 2020-05-27 at 20:59 +0200, Arnd Bergmann wrote: > On Wed, May 27, 2020 at 7:43 PM Daniele Alessandrelli > <daniele.alessandrelli@linux.intel.com> wrote: > > On Wed, 2020-05-27 at 16:33 +0200, Arnd Bergmann wrote: > > > On Wed, May 27, 2020 at 3:31 PM Alessandrelli, Daniele < > > > daniele.alessandrelli@intel.com> wrote: > > > > > > Adding it back later on with a loadable device driver should > > > not be a problem, as this is not a violation of the boot > > > protocol. > > > > Cool, I'll try to do that then, thanks! > > > > I see two ways to do that though: > > > > 1. Create a device driver that gets a reference to the memory > > region > > from its DT node and then re-adds the memory to the pool of > > available > > memory. > > > > 2. Use a special "compatible" string for my memory region and > > create a > > driver to handle it. > > I think the first approach is more common. > > > However, I think that in the second case the driver must be > > builtin. > > Would that be okay? > > It's better to avoid that. > > > Also, from a quick look, it seems that I can re-add that memory > > back by > > calling memblock_free() (or a similar memblock_* function). Am I on > > the > > right track? > > I'm not sure if memblock_free() works after early memory > initialization > is complete, but I think there is some way to do it later. Maybe try > memblock_free() first, and then look for something else if it doesn't > work. > Brilliant. I will create a new patch using the 1st approach and see if memblock_free() works; if not, I will look for something else. Thanks a lot for your valuable feedback and advice. > > > It seems that just reserving the u-boot area and optionally > > > freeing it later from a driver solves most of your problem. > > > I have one related question though: if the kernel itself is > > > protected, does that mean that any driver that does a DMA > > > to statically allocated memory inside of the kernel is broken > > > as well? I think this is something that a couple of USB drivers > > > do, though it is fairly rare. Does u-boot protect both only > > > the executable sections of the kernel or also data, and does > > > the hardware block both read and write on the IMR, or just > > > writes? > > > > Yes, you're very right. Drivers that do DMA transfers to statically > > allocated memory inside the kernel will be broken. > > > > We are currently seeing this with our eMMC driver. > > > > Current solution is to add the eMMC controller to the list of > > allowed > > "agents" for the IMR. This will reduce the level of protection, but > > since we expect to have only a few of these exceptions (since, as > > you > > pointed out, drivers that do DMA to static kernel memory seem to be > > quite rare), we think that there is still value in having the > > Kernel > > IMR. > > > > Do you see any issue with this? > > I think you should try to fix the driver rather than making an > exception for it. Yes, we'll look into that. > Hot-pluggable drivers are a much more interesting > case I think, because on the one hand you have no idea what > users might want to plug in legitimately, but on the other hand > those are also the most likely attack vectors (driver bugs for > random USB drivers overwriting kernel memory when faced with > malicious hardware) that this feature is trying to prevent. > > I also wonder whether we should do something in the normal > iommu code that prevents one from mapping a page that the > kernel would consider as protected (kernel .text, freed memory, > ...) into the iommu in the first place. Sounds like an iteresting security feature; expecially because it would apply to any hardware. > > Arnd ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/1] soc: keembay: Add Keem Bay IMR driver 2020-05-27 13:31 ` Alessandrelli, Daniele 2020-05-27 14:33 ` Arnd Bergmann @ 2020-05-28 11:22 ` Pavel Machek 2020-05-28 13:00 ` Daniele Alessandrelli 1 sibling, 1 reply; 19+ messages in thread From: Pavel Machek @ 2020-05-28 11:22 UTC (permalink / raw) To: Alessandrelli, Daniele Cc: arnd, robh, Murphy, Paul J, gregkh, Shevchenko, Andriy, linux-kernel, daniele.alessandrelli [-- Attachment #1: Type: text/plain, Size: 1587 bytes --] Hi! > > Agreed, this sounds like an incompatible extension of the boot > > protocol > > that we should otherwise not merge. > > > > However, there is also a lot of missing information here, and it is > > always > > possible they are trying to something for a good reason. As long as > > the > > problem that the bootloader is trying to solve is explained well > > enough > > in the changelog, we can discuss it to see how it should be done > > properly. > > > Apologies, I should have provided more information. Here it is :) > > Basically, at boot time U-Boot code and core memory (.text, .data, > .bss, etc.) is protected by this Isolated Memory Region (IMR) which > prevents any device or processing units other than the ARM CPU to > access/modify the memory. > > This is done for security reasons, to reduce the risks that a potential > attacker can use "hijacked" HW devices to interfere with the boot > process (and break the secure boot flow in place). Dunno. You disable that after boot anyway. Whether it is disabled just before starting kernel or just after it makes very little difference. Plus, I'm not sure if this has much security value at all. If I can corrupt data u-boot works _with_ (such as kernel, dtb), I'll take over the system anyway. IOW I believe the best/simplest way is to simply disable this in u-boot before jumping to kernel entrypoint. Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 195 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/1] soc: keembay: Add Keem Bay IMR driver 2020-05-28 11:22 ` Pavel Machek @ 2020-05-28 13:00 ` Daniele Alessandrelli 0 siblings, 0 replies; 19+ messages in thread From: Daniele Alessandrelli @ 2020-05-28 13:00 UTC (permalink / raw) To: Pavel Machek Cc: arnd, robh, Murphy, Paul J, gregkh, Shevchenko, Andriy, linux-kernel On Thu, 2020-05-28 at 13:22 +0200, Pavel Machek wrote: > Hi! > > > > Agreed, this sounds like an incompatible extension of the boot > > > protocol > > > that we should otherwise not merge. > > > > > > However, there is also a lot of missing information here, and it > > > is > > > always > > > possible they are trying to something for a good reason. As long > > > as > > > the > > > problem that the bootloader is trying to solve is explained well > > > enough > > > in the changelog, we can discuss it to see how it should be done > > > properly. > > > > Apologies, I should have provided more information. Here it is :) > > > > Basically, at boot time U-Boot code and core memory (.text, .data, > > .bss, etc.) is protected by this Isolated Memory Region (IMR) which > > prevents any device or processing units other than the ARM CPU to > > access/modify the memory. > > > > This is done for security reasons, to reduce the risks that a > > potential > > attacker can use "hijacked" HW devices to interfere with the boot > > process (and break the secure boot flow in place). > > Dunno. You disable that after boot anyway. Whether it is disabled > just before starting kernel or just after it makes very little > difference. Not sure I get your point. Disabling it while U-Boot is still running poses a security risk (even if arguably tiny), while doing it once the the Kernel is running is totally safe. So, I'd prefer to do it in the Kernel, unless practical reasons prevent it. > > Plus, I'm not sure if this has much security value at all. If I can > corrupt data u-boot works _with_ (such as kernel, dtb), I'll take > over the system anyway. True, U-Boot data needs to be protected too and, in fact, we're trying to do that as well. Other IMRs are used to protect the kernel, dtb, and other critical memory sections. > > IOW I believe the best/simplest way is to simply disable this in > u-boot before jumping to kernel entrypoint. Yes, that's definitely the simplest solution, but, IMO, not the safest one. So, I'd prefer to build on your initial suggestion and Arnd's advice and create a new device driver to disable the IMR once Linux is running. But, yes, if that eventually proves unfeasible, I might just have the bootloader disable the protection right before booting the OS. > > Best regards, > > Pavel > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/1] soc: keembay: Add Keem Bay IMR driver 2020-04-21 16:36 ` [PATCH 1/1] soc: keembay: Add Keem Bay IMR driver Daniele Alessandrelli 2020-05-01 8:10 ` Greg Kroah-Hartman @ 2020-05-07 17:44 ` Pavel Machek 1 sibling, 0 replies; 19+ messages in thread From: Pavel Machek @ 2020-05-07 17:44 UTC (permalink / raw) To: Daniele Alessandrelli Cc: linux-kernel, Greg Kroah-Hartman, Rob Herring, Andy Shevchenko, Daniele Alessandrelli, Paul J Murphy On Tue 2020-04-21 17:36:18, Daniele Alessandrelli wrote: > From: Daniele Alessandrelli <daniele.alessandrelli@intel.com> > > Keem Bay bootloader sets up a temporary Isolated Memory Region (IMR) to > protect itself during pre-Linux boot. What kind of bootloader is the SoC using? Sounds like bootloader responsibility to me... Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/1] Add IMR driver for Keem Bay 2020-04-21 16:36 [PATCH 0/1] Add IMR driver for Keem Bay Daniele Alessandrelli 2020-04-21 16:36 ` [PATCH 1/1] soc: keembay: Add Keem Bay IMR driver Daniele Alessandrelli @ 2020-04-30 19:49 ` Alessandrelli, Daniele 2020-05-01 7:09 ` gregkh 1 sibling, 1 reply; 19+ messages in thread From: Alessandrelli, Daniele @ 2020-04-30 19:49 UTC (permalink / raw) To: linux-kernel Cc: robh, Murphy, Paul J, gregkh, Shevchenko, Andriy, arnd, linux-arm-kernel On Tue, 2020-04-21 at 17:36 +0100, Daniele Alessandrelli wrote: > The following is a patch for a new Intel Movidius SoC, code-named > Keem > Bay. > > Keem Bay needs a driver to disable the Isolated Memory Region (IMR) > set up by the SoC bootloader during early boot. > > If such an IMR is not disabled and some device tries to access it, > the system will reboot. > > Since this driver is SoC-specific and Keem Bay is a new SoC, I was > unsure of where to put this driver. In the end I decided to create a > new 'keembay' directory in 'drivers/soc'. I hope that's reasonable, > if > not, just let me know. > > > Daniele Alessandrelli (1): > soc: keembay: Add Keem Bay IMR driver > > MAINTAINERS | 5 ++++ > drivers/soc/Kconfig | 1 + > drivers/soc/Makefile | 1 + > drivers/soc/keembay/Kconfig | 22 +++++++++++++++++ > drivers/soc/keembay/Makefile | 5 ++++ > drivers/soc/keembay/keembay-imr.c | 40 > +++++++++++++++++++++++++++++++ > 6 files changed, 74 insertions(+) > create mode 100644 drivers/soc/keembay/Kconfig > create mode 100644 drivers/soc/keembay/Makefile > create mode 100644 drivers/soc/keembay/keembay-imr.c > Adding some more people (Arnd and linux-arm-kernel ML) in CC in the hope of getting some guidance on how to have this patch merged. I'm a novice, so I wonder if the lack of feedback is because I'm doing something wrong or if I just sent the initial email to the wrong people / ML. I'd appreciate any help you can provide. -------------------------------------------------------------- Intel Research and Development Ireland Limited Registered in Ireland Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/1] Add IMR driver for Keem Bay 2020-04-30 19:49 ` [PATCH 0/1] Add IMR driver for Keem Bay Alessandrelli, Daniele @ 2020-05-01 7:09 ` gregkh 2020-05-01 7:53 ` Daniele Alessandrelli 0 siblings, 1 reply; 19+ messages in thread From: gregkh @ 2020-05-01 7:09 UTC (permalink / raw) To: Alessandrelli, Daniele Cc: linux-kernel, robh, Murphy, Paul J, Shevchenko, Andriy, arnd, linux-arm-kernel On Thu, Apr 30, 2020 at 07:49:36PM +0000, Alessandrelli, Daniele wrote: > This e-mail and any attachments may contain confidential material for the sole > use of the intended recipient(s). Any review or distribution by others is > strictly prohibited. If you are not the intended recipient, please contact the > sender and delete all copies. This footer means I delete all of your emails... ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/1] Add IMR driver for Keem Bay 2020-05-01 7:09 ` gregkh @ 2020-05-01 7:53 ` Daniele Alessandrelli 2020-05-01 8:04 ` gregkh 0 siblings, 1 reply; 19+ messages in thread From: Daniele Alessandrelli @ 2020-05-01 7:53 UTC (permalink / raw) To: gregkh Cc: linux-kernel, robh, Murphy, Paul J, Shevchenko, Andriy, arnd, linux-arm-kernel On Fri, 2020-05-01 at 09:09 +0200, gregkh@linuxfoundation.org wrote: > On Thu, Apr 30, 2020 at 07:49:36PM +0000, Alessandrelli, Daniele > wrote: > > This e-mail and any attachments may contain confidential material > > for the sole > > use of the intended recipient(s). Any review or distribution by > > others is > > strictly prohibited. If you are not the intended recipient, please > > contact the > > sender and delete all copies. > > This footer means I delete all of your emails... My bad, I replied using the wrong email address (i.e., the one that adds the footer automatically). Sorry about that :/ I'll be more careful the next time. The original emails (the ones with the cover letter and the patch) were fine though (unless I did something else wrong). Any advice on how to have the patch reviewed and eventually merged? ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/1] Add IMR driver for Keem Bay 2020-05-01 7:53 ` Daniele Alessandrelli @ 2020-05-01 8:04 ` gregkh 0 siblings, 0 replies; 19+ messages in thread From: gregkh @ 2020-05-01 8:04 UTC (permalink / raw) To: Daniele Alessandrelli Cc: linux-kernel, robh, Murphy, Paul J, Shevchenko, Andriy, arnd, linux-arm-kernel On Fri, May 01, 2020 at 08:53:34AM +0100, Daniele Alessandrelli wrote: > On Fri, 2020-05-01 at 09:09 +0200, gregkh@linuxfoundation.org wrote: > > On Thu, Apr 30, 2020 at 07:49:36PM +0000, Alessandrelli, Daniele > > wrote: > > > This e-mail and any attachments may contain confidential material > > > for the sole > > > use of the intended recipient(s). Any review or distribution by > > > others is > > > strictly prohibited. If you are not the intended recipient, please > > > contact the > > > sender and delete all copies. > > > > This footer means I delete all of your emails... > > My bad, I replied using the wrong email address (i.e., the one that > adds the footer automatically). Sorry about that :/ > I'll be more careful the next time. > > The original emails (the ones with the cover letter and the patch) were > fine though (unless I did something else wrong). Any advice on how to > have the patch reviewed and eventually merged? > > > > > > > Ok, let me go look at the code... ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2020-05-28 13:00 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-04-21 16:36 [PATCH 0/1] Add IMR driver for Keem Bay Daniele Alessandrelli 2020-04-21 16:36 ` [PATCH 1/1] soc: keembay: Add Keem Bay IMR driver Daniele Alessandrelli 2020-05-01 8:10 ` Greg Kroah-Hartman 2020-05-01 11:50 ` Daniele Alessandrelli 2020-05-01 11:59 ` Greg Kroah-Hartman 2020-05-24 21:28 ` Pavel Machek 2020-05-25 8:18 ` Arnd Bergmann 2020-05-27 13:31 ` Alessandrelli, Daniele 2020-05-27 14:33 ` Arnd Bergmann 2020-05-27 17:43 ` Daniele Alessandrelli 2020-05-27 18:59 ` Arnd Bergmann 2020-05-28 12:27 ` Daniele Alessandrelli 2020-05-28 11:22 ` Pavel Machek 2020-05-28 13:00 ` Daniele Alessandrelli 2020-05-07 17:44 ` Pavel Machek 2020-04-30 19:49 ` [PATCH 0/1] Add IMR driver for Keem Bay Alessandrelli, Daniele 2020-05-01 7:09 ` gregkh 2020-05-01 7:53 ` Daniele Alessandrelli 2020-05-01 8:04 ` gregkh
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).