From 5a96c41b250293318621965db1c2439cc2ce5efa Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Fri, 10 Apr 2020 05:14:06 +0000 Subject: [PATCH 1/4] Revert "- Make the case that width < 8 behave as the same as before. Pointed out by" This reverts commit 65836e0b02151f9cf5d8be5028ec2a11adcf9f86. --- sys/dev/acpi/acpi_ec.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/sys/dev/acpi/acpi_ec.c b/sys/dev/acpi/acpi_ec.c index ad7dd9ac3f19..789142e4abbb 100644 --- a/sys/dev/acpi/acpi_ec.c +++ b/sys/dev/acpi/acpi_ec.c @@ -1,4 +1,4 @@ -/* $NetBSD: acpi_ec.c,v 1.77 2019/08/06 01:53:47 msaitoh Exp $ */ +/* $NetBSD: acpi_ec.c,v 1.76 2019/08/05 10:12:04 msaitoh Exp $ */ /*- * Copyright (c) 2007 Joerg Sonnenberger . @@ -59,7 +59,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: acpi_ec.c,v 1.77 2019/08/06 01:53:47 msaitoh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: acpi_ec.c,v 1.76 2019/08/05 10:12:04 msaitoh Exp $"); #include #include @@ -679,15 +679,20 @@ acpiec_space_handler(uint32_t func, ACPI_PHYSICAL_ADDRESS paddr, if (func == ACPI_READ) *value = 0; - for (addr = paddr; addr < (paddr + width / 8); addr++, reg++) { - if (func == ACPI_READ) + do { + switch (func) { + case ACPI_READ: rv = acpiec_read(dv, addr, reg); - else + break; + case ACPI_WRITE: rv = acpiec_write(dv, addr, *reg); - + break; + } if (rv != AE_OK) break; - } + addr++; + reg++; + } while (addr < (paddr + width / 8)); return rv; } From 7aac1bb1e01fa1f3ba050e42994a452e676b0377 Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Fri, 10 Apr 2020 05:14:56 +0000 Subject: [PATCH 2/4] Revert "- Fix a bug that acpiec_space_handler() doesn't access more than 64bit" This reverts commit 4f1eb604cabce6f57600cf000e6130af4254ab44. --- sys/dev/acpi/acpi_ec.c | 79 +++++++++++++++++++----------------------- 1 file changed, 35 insertions(+), 44 deletions(-) diff --git a/sys/dev/acpi/acpi_ec.c b/sys/dev/acpi/acpi_ec.c index 789142e4abbb..d4f9de4d551f 100644 --- a/sys/dev/acpi/acpi_ec.c +++ b/sys/dev/acpi/acpi_ec.c @@ -1,4 +1,4 @@ -/* $NetBSD: acpi_ec.c,v 1.76 2019/08/05 10:12:04 msaitoh Exp $ */ +/* $NetBSD: acpi_ec.c,v 1.75 2017/03/11 08:26:23 tsutsui Exp $ */ /*- * Copyright (c) 2007 Joerg Sonnenberger . @@ -59,7 +59,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: acpi_ec.c,v 1.76 2019/08/05 10:12:04 msaitoh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: acpi_ec.c,v 1.75 2017/03/11 08:26:23 tsutsui Exp $"); #include #include @@ -404,7 +404,6 @@ post_data_map: static bool acpiec_suspend(device_t dv, const pmf_qual_t *qual) { - acpiec_cold = true; return true; @@ -413,7 +412,6 @@ acpiec_suspend(device_t dv, const pmf_qual_t *qual) static bool acpiec_resume(device_t dv, const pmf_qual_t *qual) { - acpiec_cold = false; return true; @@ -456,10 +454,9 @@ acpiec_parse_gpe_package(device_t self, ACPI_HANDLE ec_handle, ACPI_FREE(p); return false; } - + if (p->Package.Count != 2) { - aprint_error_dev(self, - "_GPE package does not contain 2 elements\n"); + aprint_error_dev(self, "_GPE package does not contain 2 elements\n"); ACPI_FREE(p); return false; } @@ -514,7 +511,6 @@ static ACPI_STATUS acpiec_space_setup(ACPI_HANDLE region, uint32_t func, void *arg, void **region_arg) { - if (func == ACPI_REGION_DEACTIVATE) *region_arg = NULL; else @@ -532,11 +528,9 @@ acpiec_lock(device_t dv) mutex_enter(&sc->sc_access_mtx); if (sc->sc_need_global_lock) { - rv = AcpiAcquireGlobalLock(EC_LOCK_TIMEOUT, - &sc->sc_global_lock); + rv = AcpiAcquireGlobalLock(EC_LOCK_TIMEOUT, &sc->sc_global_lock); if (rv != AE_OK) { - aprint_error_dev(dv, - "failed to acquire global lock: %s\n", + aprint_error_dev(dv, "failed to acquire global lock: %s\n", AcpiFormatException(rv)); return; } @@ -552,8 +546,7 @@ acpiec_unlock(device_t dv) if (sc->sc_need_global_lock) { rv = AcpiReleaseGlobalLock(sc->sc_global_lock); if (rv != AE_OK) { - aprint_error_dev(dv, - "failed to release global lock: %s\n", + aprint_error_dev(dv, "failed to release global lock: %s\n", AcpiFormatException(rv)); } } @@ -594,8 +587,7 @@ acpiec_read(device_t dv, uint8_t addr, uint8_t *val) } else if (cv_timedwait(&sc->sc_cv, &sc->sc_mtx, EC_CMD_TIMEOUT * hz)) { mutex_exit(&sc->sc_mtx); acpiec_unlock(dv); - aprint_error_dev(dv, - "command takes over %d sec...\n", EC_CMD_TIMEOUT); + aprint_error_dev(dv, "command takes over %d sec...\n", EC_CMD_TIMEOUT); return AE_ERROR; } @@ -642,8 +634,7 @@ acpiec_write(device_t dv, uint8_t addr, uint8_t val) } else if (cv_timedwait(&sc->sc_cv, &sc->sc_mtx, EC_CMD_TIMEOUT * hz)) { mutex_exit(&sc->sc_mtx); acpiec_unlock(dv); - aprint_error_dev(dv, - "command takes over %d sec...\n", EC_CMD_TIMEOUT); + aprint_error_dev(dv, "command takes over %d sec...\n", EC_CMD_TIMEOUT); return AE_ERROR; } @@ -657,42 +648,43 @@ static ACPI_STATUS acpiec_space_handler(uint32_t func, ACPI_PHYSICAL_ADDRESS paddr, uint32_t width, ACPI_INTEGER *value, void *arg, void *region_arg) { - device_t dv = arg; + device_t dv; ACPI_STATUS rv; - uint8_t addr; - uint8_t *reg; + uint8_t addr, reg; + unsigned int i; - if ((func != ACPI_READ) && (func != ACPI_WRITE)) { - aprint_error("%s: invalid Address Space function called: %x\n", - device_xname(dv), (unsigned int)func); - return AE_BAD_PARAMETER; - } if (paddr > 0xff || width % 8 != 0 || value == NULL || arg == NULL || paddr + width / 8 > 0x100) return AE_BAD_PARAMETER; addr = paddr; - reg = (uint8_t *)value; + dv = arg; rv = AE_OK; - if (func == ACPI_READ) + switch (func) { + case ACPI_READ: *value = 0; - - do { - switch (func) { - case ACPI_READ: - rv = acpiec_read(dv, addr, reg); - break; - case ACPI_WRITE: - rv = acpiec_write(dv, addr, *reg); - break; + for (i = 0; i < width; i += 8, ++addr) { + rv = acpiec_read(dv, addr, ®); + if (rv != AE_OK) + break; + *value |= (ACPI_INTEGER)reg << i; + } + break; + case ACPI_WRITE: + for (i = 0; i < width; i += 8, ++addr) { + reg = (*value >>i) & 0xff; + rv = acpiec_write(dv, addr, reg); + if (rv != AE_OK) + break; } - if (rv != AE_OK) - break; - addr++; - reg++; - } while (addr < (paddr + width / 8)); + break; + default: + aprint_error("%s: invalid Address Space function called: %x\n", + device_xname(dv), (unsigned int)func); + return AE_BAD_PARAMETER; + } return rv; } @@ -875,8 +867,7 @@ acpiec_bus_read(device_t dv, u_int addr, ACPI_INTEGER *val, int width) ACPI_STATUS acpiec_bus_write(device_t dv, u_int addr, ACPI_INTEGER val, int width) { - return acpiec_space_handler(ACPI_WRITE, addr, width * 8, &val, dv, - NULL); + return acpiec_space_handler(ACPI_WRITE, addr, width * 8, &val, dv, NULL); } ACPI_HANDLE From 69c0ba9f314925f5a70ad0a665a7d7b8aebc3f4e Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Fri, 10 Apr 2020 05:44:47 +0000 Subject: [PATCH 3/4] KNF --- sys/dev/acpi/acpi_ec.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/sys/dev/acpi/acpi_ec.c b/sys/dev/acpi/acpi_ec.c index d4f9de4d551f..687ff620103e 100644 --- a/sys/dev/acpi/acpi_ec.c +++ b/sys/dev/acpi/acpi_ec.c @@ -404,6 +404,7 @@ post_data_map: static bool acpiec_suspend(device_t dv, const pmf_qual_t *qual) { + acpiec_cold = true; return true; @@ -412,6 +413,7 @@ acpiec_suspend(device_t dv, const pmf_qual_t *qual) static bool acpiec_resume(device_t dv, const pmf_qual_t *qual) { + acpiec_cold = false; return true; @@ -454,9 +456,10 @@ acpiec_parse_gpe_package(device_t self, ACPI_HANDLE ec_handle, ACPI_FREE(p); return false; } - + if (p->Package.Count != 2) { - aprint_error_dev(self, "_GPE package does not contain 2 elements\n"); + aprint_error_dev(self, + "_GPE package does not contain 2 elements\n"); ACPI_FREE(p); return false; } @@ -511,6 +514,7 @@ static ACPI_STATUS acpiec_space_setup(ACPI_HANDLE region, uint32_t func, void *arg, void **region_arg) { + if (func == ACPI_REGION_DEACTIVATE) *region_arg = NULL; else @@ -528,9 +532,11 @@ acpiec_lock(device_t dv) mutex_enter(&sc->sc_access_mtx); if (sc->sc_need_global_lock) { - rv = AcpiAcquireGlobalLock(EC_LOCK_TIMEOUT, &sc->sc_global_lock); + rv = AcpiAcquireGlobalLock(EC_LOCK_TIMEOUT, + &sc->sc_global_lock); if (rv != AE_OK) { - aprint_error_dev(dv, "failed to acquire global lock: %s\n", + aprint_error_dev(dv, + "failed to acquire global lock: %s\n", AcpiFormatException(rv)); return; } @@ -546,7 +552,8 @@ acpiec_unlock(device_t dv) if (sc->sc_need_global_lock) { rv = AcpiReleaseGlobalLock(sc->sc_global_lock); if (rv != AE_OK) { - aprint_error_dev(dv, "failed to release global lock: %s\n", + aprint_error_dev(dv, + "failed to release global lock: %s\n", AcpiFormatException(rv)); } } @@ -587,7 +594,8 @@ acpiec_read(device_t dv, uint8_t addr, uint8_t *val) } else if (cv_timedwait(&sc->sc_cv, &sc->sc_mtx, EC_CMD_TIMEOUT * hz)) { mutex_exit(&sc->sc_mtx); acpiec_unlock(dv); - aprint_error_dev(dv, "command takes over %d sec...\n", EC_CMD_TIMEOUT); + aprint_error_dev(dv, + "command takes over %d sec...\n", EC_CMD_TIMEOUT); return AE_ERROR; } @@ -634,7 +642,8 @@ acpiec_write(device_t dv, uint8_t addr, uint8_t val) } else if (cv_timedwait(&sc->sc_cv, &sc->sc_mtx, EC_CMD_TIMEOUT * hz)) { mutex_exit(&sc->sc_mtx); acpiec_unlock(dv); - aprint_error_dev(dv, "command takes over %d sec...\n", EC_CMD_TIMEOUT); + aprint_error_dev(dv, + "command takes over %d sec...\n", EC_CMD_TIMEOUT); return AE_ERROR; } @@ -674,7 +683,7 @@ acpiec_space_handler(uint32_t func, ACPI_PHYSICAL_ADDRESS paddr, break; case ACPI_WRITE: for (i = 0; i < width; i += 8, ++addr) { - reg = (*value >>i) & 0xff; + reg = (*value >> i) & 0xff; rv = acpiec_write(dv, addr, reg); if (rv != AE_OK) break; @@ -867,7 +876,8 @@ acpiec_bus_read(device_t dv, u_int addr, ACPI_INTEGER *val, int width) ACPI_STATUS acpiec_bus_write(device_t dv, u_int addr, ACPI_INTEGER val, int width) { - return acpiec_space_handler(ACPI_WRITE, addr, width * 8, &val, dv, NULL); + return acpiec_space_handler(ACPI_WRITE, addr, width * 8, &val, dv, + NULL); } ACPI_HANDLE From 705279ff55cbd52f288b786f15a25c86bd1385d2 Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Fri, 10 Apr 2020 05:45:23 +0000 Subject: [PATCH 4/4] Reject overly large widths, from mlelstv. --- sys/dev/acpi/acpi_ec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sys/dev/acpi/acpi_ec.c b/sys/dev/acpi/acpi_ec.c index 687ff620103e..e4defac4a91b 100644 --- a/sys/dev/acpi/acpi_ec.c +++ b/sys/dev/acpi/acpi_ec.c @@ -662,8 +662,8 @@ acpiec_space_handler(uint32_t func, ACPI_PHYSICAL_ADDRESS paddr, uint8_t addr, reg; unsigned int i; - if (paddr > 0xff || width % 8 != 0 || value == NULL || arg == NULL || - paddr + width / 8 > 0x100) + if (paddr > 0xff || width % 8 != 0 || width > sizeof(ACPI_INTEGER)*8 || + value == NULL || arg == NULL || paddr + width / 8 > 0x100) return AE_BAD_PARAMETER; addr = paddr;