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: Wed, 26 Jan 2022 14:24:10 +0800 From: Tzung-Bi Shih <tzungbi@...gle.com> To: Dustin Howett <dustin@...ett.net> Cc: linux-kernel@...r.kernel.org, Benson Leung <bleung@...omium.org>, Guenter Roeck <groeck@...omium.org> Subject: Re: [PATCH 2/2] platform/chrome: reserve only the I/O ports required for the MEC EC On Tue, Jan 25, 2022 at 09:15:45AM -0600, Dustin Howett wrote: > On Mon, Jan 24, 2022 at 10:17 PM Tzung-Bi Shih <tzungbi@...gle.com> wrote: > > > > > > The original code: > > - devm_request_region(dev, EC_LPC_ADDR_MEMMAP, ...) and then > > - cros_ec_lpc_ops.read(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID, ...). > > > > After the patch: > > - devm_request_region(dev, EC_HOST_CMD_REGION0, ...) and then > > - cros_ec_lpc_ops.read(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID, ...). > > > > Does it work if it reads out of request_region range? > > > > > > The 2 request_region are now guarded by the first "if (buf[0] != 'E' || buf[1] != 'C')". That is, only non-MEC will request the 2 regions. > > > > Doesn't other MECs (e.g. non-Framework Laptop) need the 2 regions? > > So, in both cases this should be legal. Here's why: > > The MEC protocol multiplexes memory-mapped reads through the same 8 > I/O ports (0x800 - 0x807) > as it does host commands. It works by adjusting the base address, > EC_LPC_ADDR_MEMMAP (0x900), > to 0x100 before it initiates a standard MEC LPC xfer. > See cros_ec_lpc_mec_read_bytes line ~101 (as of 881007522c8fcc3785). > > Therefore, the adjusted flow in the patch is: > > 0. Default cros_ec_lpc_ops to the MEC version of read/xfer [unchanged in patch] > 1. Request 0x800 - 0x807 (MEC region) > 2. read() using the MEC read function (using only the above ports) > 3. if it succeeds, great! we have a MEC EC. > --- if it failed --- > 4. Map the non-MEC port range 0x900 - 0x9FF for memory-mapped reads > 5. read() using the NON-MEC read function (using ports 0x900 - 0x9FF) > 6. if it succeeds, map the remaining host command ports 0x808 - 0x8FF > > In short, only non-MEC needs the 0x900 - 0x9FF mapping for "mapped > memory". Therefore we can defer the > port allocation until after we've failed to read mapped memory the MEC way. :) > > Based on my understanding of the MEC protocol, non-Framework Laptop > MECs hold this invariant true as well. > They should only need ports 0x800 - 0x807. Thanks for the detail explanation. After reading cros_ec_lpc_mec_read_bytes() carefully, I guess I got it. The patch actually fixes 2 issues: 1. The original code accesses the 8 IO ports (i.e. 0x800 - 0x807) via cros_ec_lpc_mec_read_bytes(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID, ...) without requesting the region in advance. 2. MEC variants only need to request the 8 IO ports. However, the rest of ports (i.e. 0x808 - 0x9ff) are for non-MECs. > Want me to send a v2 with updated commit messages? Yes, that would be helpful.
Powered by blists - more mailing lists