lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 03 Jun 2019 17:56:18 -0300 From: Ezequiel Garcia <ezequiel@...labora.com> To: Enric Balletbo i Serra <enric.balletbo@...labora.com>, linux-kernel@...r.kernel.org Cc: Nick Crews <ncrews@...omium.org>, Guenter Roeck <groeck@...omium.org>, kernel@...labora.com, Duncan Laurie <dlaurie@...gle.com>, Benson Leung <bleung@...omium.org>, Arnd Bergmann <arnd@...db.de>, Thomas Gleixner <tglx@...utronix.de>, Greg Kroah-Hartman <gregkh@...uxfoundation.org> Subject: Re: [PATCH] platform/chrome: cros_ec_lpc: Choose Microchip EC at runtime Hi Enric, On Thu, 2019-05-30 at 19:11 +0200, Enric Balletbo i Serra wrote: > On many boards, communication between the kernel and the Embedded > Controller happens over an LPC bus. In these cases, the kernel config > CONFIG_CROS_EC_LPC is enabled. Some of these LPC boards contain a > Microchip Embedded Controller (MEC) that is different from the regular > EC. On these devices, the same LPC bus is used, but the protocol is > a little different. In these cases, the CONFIG_CROS_EC_LPC_MEC kernel > config is enabled. Currently, the kernel decides at compile-time whether > or not to use the MEC variant, and, when that kernel option is selected > it breaks the other boards. We would like a kind of runtime detection to > avoid this. > > This patch adds that detection mechanism by probing the protocol at > runtime, first we assume that a MEC variant is connected, and if the > protocol fails it fallbacks to the regular EC. This adds a bit of > overload because we try to read twice on those LPC boards that doesn't > contain a MEC variant, but is a better solution than having to select the > EC variant at compile-time. > > While here also fix the alignment in Kconfig file for this config option > replacing the spaces by tabs. > > Signed-off-by: Enric Balletbo i Serra <enric.balletbo@...labora.com> > --- > Hi, > > This is the first attempt to solve the issue to be able to select at > runtime the CrOS MEC variant. My first thought was check for a device ID, > the MEC1322 has a register that contains the device ID, however I am not > sure if we can read that register from the host without modifying the > firmware. Also, I am not sure if the MEC1322 is the only device used > that supports that LPC protocol variant, so I ended with a more easy > solution, check if the protocol fails or not. Some background on this > issue can be found [1] and [2] > > The patch has been tested on: > - Acer Chromebook R11 (Cyan - MEC variant) > - Pixel Chromebook 2015 (Samus - non-MEC variant) > - Dell Chromebook 11 (Wolf - non-MEC variant) > - Toshiba Chromebook (Leon - non-MEC variant) > > Nick, could you test the patch for Wilco? > > Best regards, > Enric > > [1] https://bugs.chromium.org/p/chromium/issues/detail?id=932626 > [2] https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/1474254 > > drivers/platform/chrome/Kconfig | 29 +++++----------- > drivers/platform/chrome/Makefile | 3 +- > drivers/platform/chrome/cros_ec_lpc.c | 15 ++++++-- > drivers/platform/chrome/cros_ec_lpc_reg.c | 42 +++++++++-------------- > drivers/platform/chrome/cros_ec_lpc_reg.h | 3 ++ > drivers/platform/chrome/wilco_ec/Kconfig | 2 +- > 6 files changed, 43 insertions(+), 51 deletions(-) > > diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig > index 2826f7136f65..453e69733842 100644 > --- a/drivers/platform/chrome/Kconfig > +++ b/drivers/platform/chrome/Kconfig > @@ -83,28 +83,17 @@ config CROS_EC_SPI > 'pre-amble' bytes before the response actually starts. > > config CROS_EC_LPC > - tristate "ChromeOS Embedded Controller (LPC)" > - depends on MFD_CROS_EC && ACPI && (X86 || COMPILE_TEST) > - help > - If you say Y here, you get support for talking to the ChromeOS EC > - over an LPC bus. This uses a simple byte-level protocol with a > - checksum. This is used for userspace access only. The kernel > - typically has its own communication methods. > - > - To compile this driver as a module, choose M here: the > - module will be called cros_ec_lpc. > - > -config CROS_EC_LPC_MEC > - bool "ChromeOS Embedded Controller LPC Microchip EC (MEC) variant" > - depends on CROS_EC_LPC > - default n > + tristate "ChromeOS Embedded Controller (LPC)" > + depends on MFD_CROS_EC && ACPI && (X86 || COMPILE_TEST) > help > - If you say Y here, a variant LPC protocol for the Microchip EC > - will be used. Note that this variant is not backward compatible > - with non-Microchip ECs. > + If you say Y here, you get support for talking to the ChromeOS EC > + over an LPC bus, including the LPC Microchip EC (MEC) variant. > + This uses a simple byte-level protocol with a checksum. This is > + used for userspace access only. The kernel typically has its own > + communication methods. > > - If you have a ChromeOS Embedded Controller Microchip EC variant > - choose Y here. > + To compile this driver as a module, choose M here: the > + module will be called cros_ec_lpcs. > > config CROS_EC_PROTO > bool > diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile > index 1b2f1dcfcd5c..d6416411888f 100644 > --- a/drivers/platform/chrome/Makefile > +++ b/drivers/platform/chrome/Makefile > @@ -9,8 +9,7 @@ obj-$(CONFIG_CHROMEOS_TBMC) += chromeos_tbmc.o > obj-$(CONFIG_CROS_EC_I2C) += cros_ec_i2c.o > obj-$(CONFIG_CROS_EC_RPMSG) += cros_ec_rpmsg.o > obj-$(CONFIG_CROS_EC_SPI) += cros_ec_spi.o > -cros_ec_lpcs-objs := cros_ec_lpc.o cros_ec_lpc_reg.o > -cros_ec_lpcs-$(CONFIG_CROS_EC_LPC_MEC) += cros_ec_lpc_mec.o > +cros_ec_lpcs-objs := cros_ec_lpc.o cros_ec_lpc_reg.o cros_ec_lpc_mec.o > obj-$(CONFIG_CROS_EC_LPC) += cros_ec_lpcs.o > obj-$(CONFIG_CROS_EC_PROTO) += cros_ec_proto.o cros_ec_trace.o > obj-$(CONFIG_CROS_KBD_LED_BACKLIGHT) += cros_kbd_led_backlight.o > diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c > index c9c240fbe7c6..2cbc71c8edba 100644 > --- a/drivers/platform/chrome/cros_ec_lpc.c > +++ b/drivers/platform/chrome/cros_ec_lpc.c > @@ -248,10 +248,21 @@ static int cros_ec_lpc_probe(struct platform_device *pdev) > return -EBUSY; > } > > + /* > + * Read the mapped ID twice, the first one is assuming the > + * EC is a Microchip Embedded Controller (MEC) variant, if the > + * protocol fails, fallback to the non MEC variant and try to > + * read again the ID. > + */ > cros_ec_lpc_read_bytes(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID, 2, buf); > if (buf[0] != 'E' || buf[1] != 'C') { > - dev_err(dev, "EC ID not detected\n"); > - return -ENODEV; > + cros_ec_is_microchip = false; > + cros_ec_lpc_read_bytes(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID, > + 2, buf); > + if (buf[0] != 'E' || buf[1] != 'C') { > + dev_err(dev, "EC ID not detected\n"); > + return -ENODEV; > + } > } > > if (!devm_request_region(dev, EC_HOST_CMD_REGION0, > diff --git a/drivers/platform/chrome/cros_ec_lpc_reg.c b/drivers/platform/chrome/cros_ec_lpc_reg.c > index 0f5cd0ac8b49..953580ac207e 100644 > --- a/drivers/platform/chrome/cros_ec_lpc_reg.c > +++ b/drivers/platform/chrome/cros_ec_lpc_reg.c > @@ -9,6 +9,12 @@ > > #include "cros_ec_lpc_mec.h" > > +/* > + * Assume that the Embedded Controller is the Microhcip variant, we will > + * mark as false if that's not the case. > + */ > +bool cros_ec_is_microchip = true; > + I'm reluctant to add global state to the kernel, specially when it's something that's not per-kernel but rather a property of the cros ec device. Maybe this can be represented with a priv data inside a union: struct cros_ec_lpc_hw { u8 (*read_bytes)(unsigned int offset, unsigned int length, u8 *dest); ... }; struct cros_ec_device { ... union { struct cros_ec_lpc_hw lpc_priv; } }; You can then detect the protocol at probe time, and assign the I/O hooks (read_bytes, etc). Should work the same way, but with a different design. Thanks, Eze
Powered by blists - more mailing lists