March 2022 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: board_enable.c: Fix dead assignment
by Light (Code Review) 14 Mar '22

14 Mar '22

Attention is currently required from: Keith Hui, Angel Pons, Anastasia Klimchuk. Light has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62761 ) Change subject: board_enable.c: Fix dead assignment ...................................................................... Patch Set 4: (1 comment) Patchset: PS1: > Correct. […] Okay got it. Thanks:) -- To view, visit https://review.coreboot.org/c/flashrom/+/62761 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7458b416a69fd5e2aa300ca49d1352b6074ad0bc Gerrit-Change-Number: 62761 Gerrit-PatchSet: 4 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Keith Hui <buurin(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-CC: Keith Hui <buurin(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Keith Hui <buurin(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Keith Hui <buurin(a)gmail.com> Gerrit-Comment-Date: Mon, 14 Mar 2022 17:53:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Light <aarya.chaumal(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Keith Hui <buurin(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: hwaccess_x86_io: clean header concept
by Nico Huber (Code Review) 14 Mar '22

14 Mar '22

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/61276 ) Change subject: hwaccess_x86_io: clean header concept ...................................................................... hwaccess_x86_io: clean header concept Move all function implementations into the .c file TEST: `[g]make [WARNERROR=no]` on Linux, FreeBSD, NetBSD, OpenBSD, DragonflyBSD, OpenIndiana, Debian-GNU/Hurd Change-Id: I1400704e9ac5fed00c096796536108d5bfb875e3 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/61276 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M Makefile M hwaccess_x86_io.c M hwaccess_x86_io.h M meson.build 4 files changed, 315 insertions(+), 188 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved diff --git a/Makefile b/Makefile index 53531a4..f5ed409 100644 --- a/Makefile +++ b/Makefile @@ -365,25 +365,6 @@ $(call mark_unsupported,$(DEPENDS_ON_RAW_MEM_ACCESS)) endif -ifeq ($(TARGET_OS), $(filter $(TARGET_OS), Linux Darwin NetBSD OpenBSD)) -FEATURE_FLAGS += -D'USE_IOPL=1' -else -FEATURE_FLAGS += -D'USE_IOPL=0' -endif - -ifeq ($(TARGET_OS), $(filter $(TARGET_OS), FreeBSD FreeBSD-glibc DragonFlyBSD)) -FEATURE_FLAGS += -D'USE_DEV_IO=1' -else -FEATURE_FLAGS += -D'USE_DEV_IO=0' -endif - -ifeq ($(TARGET_OS), $(filter $(TARGET_OS), Hurd)) -FEATURE_FLAGS += -D'USE_IOPERM=1' -else -FEATURE_FLAGS += -D'USE_IOPERM=0' -endif - - ############################################################################### # Flash chip drivers and bus support infrastructure. diff --git a/hwaccess_x86_io.c b/hwaccess_x86_io.c index e891d98..c3ad313 100644 --- a/hwaccess_x86_io.c +++ b/hwaccess_x86_io.c @@ -17,12 +17,25 @@ */ /* - * This file implements x86 I/O port permission handling. + * This file implements x86 I/O port permission and access handling. * - * For this purpose the platform specific code is encapsuled in two functions - * and compiled only on the respective platform. `platform_get_io_perms()` is - * called to get the permissions and `platform_release_io_perms()` is called for - * releasing those. + * The first part of the file defines the platform dependend implementations to + * use on each platform. For getting I/O permissions set IO_PORT_PERMISSION to + * one of: + * - USE_DUMMY + * - USE_SUN + * - USE_DEVICE + * - USE_IOPERM + * - USE_IOPL + * For the IN[B/W/L] and OUT[B/W/L] functions set IO_PORT_FUNCTION to one of: + * - USE_LIBC_TARGET_LAST + * - USE_LIBC_TARGET_FIRST + * - USE_DOS + * - USE_ASM + * + * The platform specific code for getting I/O permissions consists of two + * functions. `platform_get_io_perms()` is called to get + * permissions and `platform_release_io_perms()` is called for releasing those. */ #include <errno.h> @@ -31,7 +44,126 @@ #include "flash.h" #include "hwaccess_x86_io.h" -#if defined(__DJGPP__) || defined(__LIBPAYLOAD) /* DJGPP, libpayload */ +/* IO_PORT_FUNCTION */ +#define USE_LIBC_TARGET_LAST 1 +#define USE_LIBC_TARGET_FIRST 2 +#define USE_ASM 3 +#define USE_DOS 4 + +/* IO_PORT_PERMISSION */ +#define USE_IOPL 5 +#define USE_DEVICE 6 +#define USE_DUMMY 7 +#define USE_SUN 8 +#define USE_IOPERM 9 + +#if defined(__ANDROID__) +#include <sys/glibc-syscalls.h> + +#define IO_PORT_PERMISSION USE_IOPL +#define IO_PORT_FUNCTION USE_LIBC_TARGET_LAST +#endif + +#if defined(__linux__) && !defined(__ANDROID__) +#include <sys/io.h> + +#define IO_PORT_PERMISSION USE_IOPL +#define IO_PORT_FUNCTION USE_LIBC_TARGET_LAST +#endif + +#if defined(__FreeBSD_kernel) && defined(__GLIBC__) +#include <sys/io.h> +#include <sys/types.h> +#include <fcntl.h> +#include <unistd.h> + +#define IO_PORT_PERMISSION USE_DEVICE +#define IO_PORT_FUNCTION USE_LIBC_TARGET_LAST +#endif + +#if defined(__FreeBSD__) || defined(__DragonFly__) +#include <sys/types.h> +#include <machine/cpufunc.h> +#include <fcntl.h> +#include <unistd.h> + +#define IO_PORT_PERMISSION USE_DEVICE +#define IO_PORT_FUNCTION USE_LIBC_TARGET_FIRST +#endif + +#if defined(__NetBSD__) +#include <sys/types.h> +#include <machine/sysarch.h> + +#if defined(__i386__) +#define iopl i386_iopl +#elif defined(__x86_64__) +#define iopl x86_64_iopl +#endif + +#define IO_PORT_PERMISSION USE_IOPL +#define IO_PORT_FUNCTION USE_ASM +#endif + +#if defined(__OpenBSD__) +#include <sys/types.h> +#include <machine/sysarch.h> + +#if defined(__i386__) +#define iopl i386_iopl +#elif defined(__amd64__) +#define iopl amd64_iopl +#endif + +#define IO_PORT_PERMISSION USE_IOPL +#define IO_PORT_FUNCTION USE_ASM +#endif + +#if defined(__MACH__) && defined(__APPLE__) +#include <DirectHW/DirectHW.h> + +#define IO_PORT_PERMISSION USE_IOPL +#define IO_PORT_FUNCTION USE_LIBC_TARGET_LAST +#endif + +#if defined(__DJGPP__) +#include <pc.h> + +#define IO_PORT_PERMISSION USE_DUMMY +#define IO_PORT_FUNCTION USE_DOS +#endif + +#if defined(__LIBPAYLOAD__) +#define IO_PORT_PERMISSION USE_DUMMY +#define IO_PORT_FUNCTION USE_LIBC_TARGET_LAST +#endif + +#if defined(__sun) +#include <sys/sysi86.h> +#include <sys/psw.h> +#include <asm/sunddi.h> + +#define IO_PORT_PERMISSION USE_SUN +#define IO_PORT_FUNCTION USE_LIBC_TARGET_FIRST +#endif + +#if defined(__gnu_hurd__) +#include <sys/io.h> + +#define IO_PORT_PERMISSION USE_IOPERM +#define IO_PORT_FUNCTION USE_LIBC_TARGET_LAST +#endif + +/* Make sure IO_PORT_PERMISSION and IO_PORT_FUNCTION are set */ +#if IO_PORT_PERMISSION == 0 || IO_PORT_FUNCTION == 0 +#error Unsupported or misconfigured platform. +#endif + +/* + * USE_DUMMY + * This is for platforms which have no privilege levels. + */ +#if IO_PORT_PERMISSION == USE_DUMMY static int platform_get_io_perms(void) { return 0; @@ -43,9 +175,11 @@ } #endif -#if defined(__sun) /* SunOS */ -#include <sys/sysi86.h> - +/* + * USE_SUN + * This implementation uses SunOS system call sysi86 to handle I/O port permissions. + */ +#if IO_PORT_PERMISSION == USE_SUN static int platform_get_io_perms(void) { return sysi86(SI86V86, V86SC_IOPL, PS_IOPL); @@ -58,10 +192,11 @@ } #endif -#if USE_DEV_IO /* FreeBSD, DragonFlyBSD */ -#include <fcntl.h> -#include <unistd.h> - +/* + * USE_DEVICE + * This implementation uses the virtual /dev/io file to handle I/O Port permissions. + */ +#if IO_PORT_PERMISSION == USE_DEVICE int io_fd; static int platform_get_io_perms(void) @@ -77,9 +212,11 @@ } #endif -#if USE_IOPERM /* GNU Hurd */ -#include <sys/io.h> - +/* + * USE_IOPERM + * This implementation uses the ioperm system call to handle I/O port permissions. + */ +#if IO_PORT_PERMISSION == USE_IOPERM static int platform_get_io_perms(void) { return ioperm(0, 65536, 1); @@ -92,7 +229,11 @@ } #endif -#if USE_IOPL /* Linux, Darwin, NetBSD, OpenBSD */ +/* + * USE_IOPL + * This implementation uses the iopl system call to handle I/O port permissions. + */ +#if IO_PORT_PERMISSION == USE_IOPL static int platform_get_io_perms(void) { return iopl(3); @@ -129,3 +270,145 @@ "that your kernel configuration has the option INSECURE enabled.\n"); return 1; } + +/* + * USE_LIBC_LAST + * This implementation wraps the glibc functions with the port as 2nd argument. + */ +#if IO_PORT_FUNCTION == USE_LIBC_TARGET_LAST +void OUTB(uint8_t value, uint16_t port) +{ + outb(value, port); +} + +void OUTW(uint16_t value, uint16_t port) +{ + outw(value, port); +} + +void OUTL(uint32_t value, uint16_t port) +{ + outl(value, port); +} +#endif + +/* + * USE_LIBC_FIRST + * This implementation uses libc based functions with the port as first argument + * like on BSDs. + */ +#if IO_PORT_FUNCTION == USE_LIBC_TARGET_FIRST +void OUTB(uint8_t value, uint16_t port) +{ + outb(port, value); +} + +void OUTW(uint16_t value, uint16_t port) +{ + outw(port, value); +} + +void OUTL(uint32_t value, uint16_t port) +{ + outl(port, value); +} +#endif + +/* + * LIBC_xxx + * INx functions can be shared between _LAST and _FIRST + */ +#if IO_PORT_FUNCTION == USE_LIBC_TARGET_LAST || IO_PORT_FUNCTION == USE_LIBC_TARGET_FIRST +uint8_t INB(uint16_t port) +{ + return inb(port); +} + +uint16_t INW(uint16_t port) +{ + return inw(port); +} + +uint32_t INL(uint16_t port) +{ + return inl(port); +} +#endif + +/* + * USE_DOS + * DOS provides the functions under a differnt name. + */ +#if IO_PORT_FUNCTION == USE_DOS +void OUTB(uint8_t value, uint16_t port) +{ + outportb(port, value); +} + +void OUTW(uint16_t value, uint16_t port) +{ + outportw(port, value); +} + +void OUTL(uint32_t value, uint16_t port) +{ + outportl(port, value); +} + +uint8_t INB(uint16_t port) +{ + return inportb(port); +} + +uint16_t INW(uint16_t port) +{ + return inportw(port); +} + +uint32_t INL(uint16_t port) +{ + return inportl(port); +} +#endif + +/* + * USE_ASM + * Pure assembly implementation. + */ +#if IO_PORT_FUNCTION == USE_ASM +void OUTB(uint8_t value, uint16_t port) +{ + __asm__ volatile ("outb %b0,%w1": :"a" (value), "Nd" (port)); +} + +void OUTW(uint16_t value, uint16_t port) +{ + __asm__ volatile ("outw %w0,%w1": :"a" (value), "Nd" (port)); +} + +void OUTL(uint32_t value, uint16_t port) +{ + __asm__ volatile ("outl %0,%w1": :"a" (value), "Nd" (port)); +} + +uint8_t INB(uint16_t port) +{ + uint8_t value; + __asm__ volatile ("inb %w1,%0":"=a" (value):"Nd" (port)); + return value; +} + +uint16_t INW(uint16_t port) +{ + uint16_t value; + __asm__ volatile ("inw %w1,%0":"=a" (value):"Nd" (port)); + return value; +} + +uint32_t INL(uint16_t port) +{ + uint32_t value; + __asm__ volatile ("inl %1,%0":"=a" (value):"Nd" (port)); + return value; +} +#endif diff --git a/hwaccess_x86_io.h b/hwaccess_x86_io.h index 0d16fdd..31d1e86 100644 --- a/hwaccess_x86_io.h +++ b/hwaccess_x86_io.h @@ -2,6 +2,8 @@ * This file is part of the flashrom project. * * Copyright (C) 2009 Carl-Daniel Hailfinger + * Copyright (C) 2022 secunet Security Networks AG + * (Written by Thomas Heijligen <thomas.heijligen(a)secunet.com) * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -13,144 +15,23 @@ * GNU General Public License for more details. */ +/* + * This file contains prototypes for x86 I/O Port access. + */ + #ifndef __HWACCESS_X86_IO_H__ #define __HWACCESS_X86_IO_H__ 1 -/* sys/io.h provides iopl(2) and x86 I/O port access functions (inb, outb etc). - * It is included in glibc (thus available also on debian/kFreeBSD) but also in other libcs that mimic glibc, - * e.g. musl and uclibc. Because we cannot detect the libc or existence of the header or of the instructions - * themselves safely in here we use some heuristic below: - * On Android we don't have the header file and no way for I/O port access at all. However, sys/glibc-syscalls.h - * refers to an iopl implementation and we therefore include at least that one for now. On non-Android we assume - * that a Linux system's libc has a suitable sys/io.h or (on non-Linux) we depend on glibc to offer it. */ -#if defined(__ANDROID__) -#include <sys/glibc-syscalls.h> -#elif defined(__linux__) || defined(__GLIBC__) -#include <sys/io.h> -#endif - -/* for iopl and outb under Solaris */ -#if defined (__sun) -#include <sys/sysi86.h> -#include <sys/psw.h> -#include <asm/sunddi.h> -#endif - +#include <stdint.h> +/** + */ int rget_io_perms(void); -/* Clarification about OUTB/OUTW/OUTL argument order: - * OUT[BWL](val, port) - */ - -#if defined(__FreeBSD__) || defined(__DragonFly__) - /* Note that Debian/kFreeBSD (FreeBSD kernel with glibc) has conflicting - * out[bwl] definitions in machine/cpufunc.h and sys/io.h at least in some - * versions. Use machine/cpufunc.h only for plain FreeBSD/DragonFlyBSD. - */ - #include <sys/types.h> - #include <machine/cpufunc.h> - #define OUTB(x, y) do { u_int outb_tmp = (y); outb(outb_tmp, (x)); } while (0) - #define OUTW(x, y) do { u_int outw_tmp = (y); outw(outw_tmp, (x)); } while (0) - #define OUTL(x, y) do { u_int outl_tmp = (y); outl(outl_tmp, (x)); } while (0) - #define INB(x) __extension__ ({ u_int inb_tmp = (x); inb(inb_tmp); }) - #define INW(x) __extension__ ({ u_int inw_tmp = (x); inw(inw_tmp); }) - #define INL(x) __extension__ ({ u_int inl_tmp = (x); inl(inl_tmp); }) -#else - -#if defined (__sun) - /* Note different order for outb */ - #define OUTB(x,y) outb(y, x) - #define OUTW(x,y) outw(y, x) - #define OUTL(x,y) outl(y, x) - #define INB inb - #define INW inw - #define INL inl -#else - -#ifdef __DJGPP__ - -#include <pc.h> - - #define OUTB(x,y) outportb(y, x) - #define OUTW(x,y) outportw(y, x) - #define OUTL(x,y) outportl(y, x) - - #define INB inportb - #define INW inportw - #define INL inportl - -#else - -#if defined(__MACH__) && defined(__APPLE__) - /* Header is part of the DirectHW library. */ - #include <DirectHW/DirectHW.h> -#endif - - /* This is the usual glibc interface. */ - #define OUTB outb - #define OUTW outw - #define OUTL outl - #define INB inb - #define INW inw - #define INL inl -#endif -#endif -#endif - -#if defined(__NetBSD__) || defined (__OpenBSD__) - #if defined(__i386__) || defined(__x86_64__) - #include <sys/types.h> - #include <machine/sysarch.h> - #if defined(__NetBSD__) - #if defined(__i386__) - #define iopl i386_iopl - #elif defined(__x86_64__) - #define iopl x86_64_iopl - #endif - #elif defined (__OpenBSD__) - #if defined(__i386__) - #define iopl i386_iopl - #elif defined(__amd64__) - #define iopl amd64_iopl - #endif - #endif - -static inline void outb(uint8_t value, uint16_t port) -{ - __asm__ volatile ("outb %b0,%w1": :"a" (value), "Nd" (port)); -} - -static inline uint8_t inb(uint16_t port) -{ - uint8_t value; - __asm__ volatile ("inb %w1,%0":"=a" (value):"Nd" (port)); - return value; -} - -static inline void outw(uint16_t value, uint16_t port) -{ - __asm__ volatile ("outw %w0,%w1": :"a" (value), "Nd" (port)); -} - -static inline uint16_t inw(uint16_t port) -{ - uint16_t value; - __asm__ volatile ("inw %w1,%0":"=a" (value):"Nd" (port)); - return value; -} - -static inline void outl(uint32_t value, uint16_t port) -{ - __asm__ volatile ("outl %0,%w1": :"a" (value), "Nd" (port)); -} - -static inline uint32_t inl(uint16_t port) -{ - uint32_t value; - __asm__ volatile ("inl %1,%0":"=a" (value):"Nd" (port)); - return value; -} - #endif -#endif +void OUTB(uint8_t value, uint16_t port); +void OUTW(uint16_t value, uint16_t port); +void OUTL(uint32_t value, uint16_t port); +uint8_t INB(uint16_t port); +uint16_t INW(uint16_t port); +uint32_t INL(uint16_t port); #endif /* __HWACCESS_X86_IO_H__ */ diff --git a/meson.build b/meson.build index cc393a7..b7fc25f 100644 --- a/meson.build +++ b/meson.build @@ -42,24 +42,6 @@ add_project_arguments('-DIS_WINDOWS=0', language : 'c') endif -if host_machine.system() in ['linux', 'darwin', 'netbsd', 'openbsd'] - add_project_arguments('-DUSE_IOPL=1', language : 'c') -else - add_project_arguments('-DUSE_IOPL=0', language : 'c') -endif - -if host_machine.system() in ['freebsd', 'dragonfly'] - add_project_arguments('-DUSE_DEV_IO=1', language : 'c') -else - add_project_arguments('-DUSE_DEV_IO=0', language : 'c') -endif - -if host_machine.system() in ['gnu'] - add_project_arguments('-DUSE_IOPERM=1', language : 'c') -else - add_project_arguments('-DUSE_IOPERM=0', language : 'c') -endif - # get defaults from configure config_atahpt = get_option('config_atahpt') config_atapromise = get_option('config_atapromise') -- To view, visit https://review.coreboot.org/c/flashrom/+/61276 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1400704e9ac5fed00c096796536108d5bfb875e3 Gerrit-Change-Number: 61276 Gerrit-PatchSet: 18 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: hwaccess_x86_io: clean header concept
by Nico Huber (Code Review) 14 Mar '22

14 Mar '22

Attention is currently required from: Thomas Heijligen. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61276 ) Change subject: hwaccess_x86_io: clean header concept ...................................................................... Patch Set 17: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/61276 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1400704e9ac5fed00c096796536108d5bfb875e3 Gerrit-Change-Number: 61276 Gerrit-PatchSet: 17 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Mon, 14 Mar 2022 11:28:31 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Makefile: use HAS_ USE_ pattern for serial support
by Nico Huber (Code Review) 14 Mar '22

14 Mar '22

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/62196 ) Change subject: Makefile: use HAS_ USE_ pattern for serial support ...................................................................... Makefile: use HAS_ USE_ pattern for serial support Align the usage of serial function with the selection of other dependencies. Change-Id: Ica951e76d6362b01f09d23a729a2a6049e7f0b66 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/62196 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M Makefile 1 file changed, 12 insertions(+), 10 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved diff --git a/Makefile b/Makefile index eb478dd..53531a4 100644 --- a/Makefile +++ b/Makefile @@ -98,6 +98,9 @@ CONFIG_PONY_SPI \ CONFIG_SERPROG \ +DEPENDS_ON_SOCKETS := \ + CONFIG_SERPROG \ + DEPENDS_ON_BITBANG_SPI := \ CONFIG_INTERNAL_X86 \ CONFIG_NICINTEL_SPI \ @@ -247,14 +250,13 @@ HAS_LINUX_MTD := $(call c_compile_test, Makefile.d/linux_mtd_test.c) HAS_LINUX_SPI := $(call c_compile_test, Makefile.d/linux_spi_test.c) HAS_LINUX_I2C := $(call c_compile_test, Makefile.d/linux_i2c_test.c) +HAS_SERIAL := $(strip $(if $(filter $(TARGET_OS), DOS libpayload), no, yes)) EXEC_SUFFIX := $(strip $(if $(filter $(TARGET_OS), DOS MinGW), .exe)) ifeq ($(TARGET_OS), DOS) # DJGPP has odd uint*_t definitions which cause lots of format string warnings. override CFLAGS += -Wno-format override LDFLAGS += -lgetopt -# Missing serial support. -$(call mark_unsupported,$(DEPENDS_ON_SERIAL)) endif ifeq ($(TARGET_OS), $(filter $(TARGET_OS), MinGW Cygwin)) @@ -292,8 +294,6 @@ $(call mark_unsupported,CONFIG_DUMMY) # libpayload does not provide the romsize field in struct pci_dev that the atapromise code requires. $(call mark_unsupported,CONFIG_ATAPROMISE) -# Bus Pirate, Serprog and PonyProg are not supported with libpayload (missing serial support). -$(call mark_unsupported,$(DEPENDS_ON_SERIAL)) # Dediprog, Developerbox, USB-Blaster, PICkit2, CH341A and FT2232 are not supported with libpayload (missing libusb support). $(call mark_unsupported,$(DEPENDS_ON_LIBUSB1) $(DEPENDS_ON_LIBFTDI) $(DEPENDS_ON_LIBJAYLINK)) endif @@ -340,6 +340,10 @@ $(call mark_unsupported,$(DEPENDS_ON_LIBUSB1)) endif +ifeq ($(HAS_SERIAL), no) +$(call mark_unsupported, $(DEPENDS_ON_SERIAL)) +endif + ifeq ($(ENDIAN), little) FEATURE_FLAGS += -D'__FLASHROM_LITTLE_ENDIAN__=1' endif @@ -611,8 +615,6 @@ ifeq ($(CONFIG_SERPROG), yes) FEATURE_FLAGS += -D'CONFIG_SERPROG=1' PROGRAMMER_OBJS += serprog.o -NEED_SERIAL += CONFIG_SERPROG -NEED_POSIX_SOCKETS += CONFIG_SERPROG endif ifeq ($(CONFIG_RAYER_SPI), yes) @@ -628,7 +630,6 @@ ifeq ($(CONFIG_PONY_SPI), yes) FEATURE_FLAGS += -D'CONFIG_PONY_SPI=1' PROGRAMMER_OBJS += pony_spi.o -NEED_SERIAL += CONFIG_PONY_SPI endif ifeq ($(CONFIG_BITBANG_SPI), yes) @@ -749,7 +750,6 @@ ifeq ($(CONFIG_BUSPIRATE_SPI), yes) FEATURE_FLAGS += -D'CONFIG_BUSPIRATE_SPI=1' PROGRAMMER_OBJS += buspirate_spi.o -NEED_SERIAL += CONFIG_BUSPIRATE_SPI endif ifeq ($(CONFIG_DEDIPROG), yes) @@ -807,7 +807,8 @@ LIB_OBJS += i2c_helper_linux.o endif -ifneq ($(NEED_SERIAL), ) +USE_SERIAL := $(if $(call filter_deps,$(DEPENDS_ON_SERIAL)),yes,no) +ifeq ($(USE_SERIAL), yes) LIB_OBJS += serial.o ifeq ($(TARGET_OS), Linux) LIB_OBJS += custom_baud_linux.o @@ -816,7 +817,8 @@ endif endif -ifneq ($(NEED_POSIX_SOCKETS), ) +USE_SOCKETS := $(if $(call filter_deps,$(DEPENDS_ON_SOCKETS)),yes,no) +ifeq ($(USE_SOCKETS), yes) ifeq ($(TARGET_OS), SunOS) override LDFLAGS += -lsocket -lnsl endif -- To view, visit https://review.coreboot.org/c/flashrom/+/62196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ica951e76d6362b01f09d23a729a2a6049e7f0b66 Gerrit-Change-Number: 62196 Gerrit-PatchSet: 5 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: board_enable.c: Fix dead assignment
by Keith Hui (Code Review) 14 Mar '22

14 Mar '22

Attention is currently required from: Keith Hui, Angel Pons, Light, Anastasia Klimchuk. Keith Hui has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62761 ) Change subject: board_enable.c: Fix dead assignment ...................................................................... Patch Set 4: (1 comment) Patchset: PS1: > `INB(0x80)` reads a byte from I/O port 0x80. […] Correct. The read is used as a small delay between polling the SMBus controller for when the command is done. I probably copied it from other similar functions within flashrom. The assignment (which this patch tries to remove) is to make sure compiler doesn't warn; we don't care about what is read from 0x80. If it builds still afterwards, it's fine by me. -- To view, visit https://review.coreboot.org/c/flashrom/+/62761 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7458b416a69fd5e2aa300ca49d1352b6074ad0bc Gerrit-Change-Number: 62761 Gerrit-PatchSet: 4 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Keith Hui <buurin(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-CC: Keith Hui <buurin(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Keith Hui <buurin(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 14 Mar 2022 11:19:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Light <aarya.chaumal(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Add support for Meteor Lake
by Subrata Banik (Code Review) 14 Mar '22

14 Mar '22

Attention is currently required from: Wonkyu Kim, Anastasia Klimchuk. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62783 ) Change subject: ichspi: Add support for Meteor Lake ...................................................................... Patch Set 4: (2 comments) File util/ich_descriptors_tool/ich_descriptors_tool.c: https://review.coreboot.org/c/flashrom/+/62783/comment/802f9b39_1ecd7180 PS3, Line 130: "\t- \"gemini\" for Intel's Gemini Lake SoC.\n" > Seems to make sense to group named SoC's together over the other topology of x00 series? Lets see wh […] Ack https://review.coreboot.org/c/flashrom/+/62783/comment/71a09015_75297978 PS3, Line 241: else if (strcmp(csn, "meteor") == 0) : cs = CHIPSET_METEOR_LAKE; > > should this be moved down a bit to be ordered in the same way the others are to be consistent? […] Ack -- To view, visit https://review.coreboot.org/c/flashrom/+/62783 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0a2ffe2ba8d96c90d89b77e0d8583d179ff02a75 Gerrit-Change-Number: 62783 Gerrit-PatchSet: 4 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Wonkyu Kim Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Wonkyu Kim Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 14 Mar 2022 11:13:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Add support for Meteor Lake
by Edward O'Callaghan (Code Review) 14 Mar '22

14 Mar '22

Attention is currently required from: Wonkyu Kim, Subrata Banik, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62783 ) Change subject: ichspi: Add support for Meteor Lake ...................................................................... Patch Set 3: Code-Review+2 (1 comment) File util/ich_descriptors_tool/ich_descriptors_tool.c: https://review.coreboot.org/c/flashrom/+/62783/comment/d1ac5be4_c335c154 PS3, Line 130: "\t- \"gemini\" for Intel's Gemini Lake SoC.\n" > > maybe the SoC are grouped around here though. […] Seems to make sense to group named SoC's together over the other topology of x00 series? Lets see what the community would like here? -- To view, visit https://review.coreboot.org/c/flashrom/+/62783 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0a2ffe2ba8d96c90d89b77e0d8583d179ff02a75 Gerrit-Change-Number: 62783 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Wonkyu Kim Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Wonkyu Kim Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 14 Mar 2022 11:10:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Add support for Meteor Lake
by Subrata Banik (Code Review) 14 Mar '22

14 Mar '22

Attention is currently required from: Wonkyu Kim, Edward O'Callaghan, Anastasia Klimchuk. Hello build bot (Jenkins), Wonkyu Kim, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/62783 to look at the new patch set (#4). Change subject: ichspi: Add support for Meteor Lake ...................................................................... ichspi: Add support for Meteor Lake This patch adds Meteor Lake support into flashrom. Additionally, utilize CSSO (CPU Soft Strap Offset) to uniquely detect the chipset when the CSSL (CPU Soft Strap Length) field default value (0x03) on Meteor Lake is the same as Elkhart Lake. BUG=b:224325352 TEST=Flashrom is able to detect MTL SPI DID and show chipset name as below: > flashrom --flash-name .... Found chipset "Intel Meteor Lake-P/M". .... > flashrom - internal --ifd -i fd -i bios -r /tmp/bios.rom .... Reading ich_descriptor... done. Assuming chipset 'Meteor Lake'. Using regions: "bios", "fd". Reading flash... done. SUCCESS Signed-off-by: Subrata Banik <subratabanik(a)google.com> Change-Id: I0a2ffe2ba8d96c90d89b77e0d8583d179ff02a75 --- M chipset_enable.c M ich_descriptors.c M ichspi.c M programmer.h M util/ich_descriptors_tool/ich_descriptors_tool.c 5 files changed, 38 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/83/62783/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/62783 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0a2ffe2ba8d96c90d89b77e0d8583d179ff02a75 Gerrit-Change-Number: 62783 Gerrit-PatchSet: 4 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Wonkyu Kim Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Wonkyu Kim Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: ichspi: Add support for Meteor Lake
by Subrata Banik (Code Review) 14 Mar '22

14 Mar '22

Attention is currently required from: Wonkyu Kim, Edward O'Callaghan, Anastasia Klimchuk. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62783 ) Change subject: ichspi: Add support for Meteor Lake ...................................................................... Patch Set 3: (2 comments) File util/ich_descriptors_tool/ich_descriptors_tool.c: https://review.coreboot.org/c/flashrom/+/62783/comment/878f35c1_c37a51c6 PS3, Line 130: "\t- \"gemini\" for Intel's Gemini Lake SoC.\n" > maybe the SoC are grouped around here though. ah, I can do that, just that MTL is next to ADL hence kept at line 141. If you want, I can move it here as well. https://review.coreboot.org/c/flashrom/+/62783/comment/64ae1680_ad605932 PS3, Line 241: else if (strcmp(csn, "meteor") == 0) : cs = CHIPSET_METEOR_LAKE; > should this be moved down a bit to be ordered in the same way the others are to be consistent? same reason as other file. -- To view, visit https://review.coreboot.org/c/flashrom/+/62783 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0a2ffe2ba8d96c90d89b77e0d8583d179ff02a75 Gerrit-Change-Number: 62783 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Wonkyu Kim Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Wonkyu Kim Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 14 Mar 2022 10:51:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Add support for Meteor Lake
by Edward O'Callaghan (Code Review) 14 Mar '22

14 Mar '22

Attention is currently required from: Wonkyu Kim, Subrata Banik, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62783 ) Change subject: ichspi: Add support for Meteor Lake ...................................................................... Patch Set 3: (2 comments) File util/ich_descriptors_tool/ich_descriptors_tool.c: https://review.coreboot.org/c/flashrom/+/62783/comment/ddf6e9d2_d0e7b4fd PS3, Line 130: "\t- \"gemini\" for Intel's Gemini Lake SoC.\n" maybe the SoC are grouped around here though. https://review.coreboot.org/c/flashrom/+/62783/comment/9dddc792_6b04e437 PS3, Line 241: else if (strcmp(csn, "meteor") == 0) : cs = CHIPSET_METEOR_LAKE; should this be moved down a bit to be ordered in the same way the others are to be consistent? -- To view, visit https://review.coreboot.org/c/flashrom/+/62783 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0a2ffe2ba8d96c90d89b77e0d8583d179ff02a75 Gerrit-Change-Number: 62783 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Wonkyu Kim Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Wonkyu Kim Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 14 Mar 2022 10:48:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2022