ron minnich has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Add configurable ramstage support for minimal PCI scanning
This CL has changes that allow us to enable a configurable ramstage, and one change that allows us to minimize PCI scanning. Minimal scanning is a frequently requested feature.
To enable it, we add two new variables to src/Kconfig CONFIGURABLE_RAMSTAGE is the overall variable controlling other options for minimizing the ramstage.
MINIMAL_PCI_SCANNING is how we indicate we wish to enable minimal PCI scanning.
Some devices must be scanned in all cases, such as 0:0.0.
To indicate which devices we must scan, we add a new mandatory keyword to sconfig
It is used in place of on, off, or hidden, and indicates a device is enabled and mandatory. Mandatory devices are always scanned. When MINIMAL_PCI_SCANNING is enabled, ONLY mandatory devices are scanned.
We further add support in src/device/pci_device.c to manage both MINIMAL_PCI_SCANNING and mandatory devices.
Finally, to show how this works in practice, we add mandatory keywords to 3 devices on the qemu-q35.
This is tested and working on the qemu-q35 target.
Change-Id: I2073d9f8e9297c2b02530821ebb634ea2a5c758e Signed-off-by: Ronald G. Minnich rminnich@gmail.com --- M src/Kconfig M src/device/pci_device.c M src/include/device/device.h M src/mainboard/emulation/qemu-q35/devicetree.cb M util/sconfig/lex.yy.c_shipped M util/sconfig/main.c M util/sconfig/sconfig.h M util/sconfig/sconfig.l M util/sconfig/sconfig.tab.c_shipped M util/sconfig/sconfig.tab.h_shipped M util/sconfig/sconfig.y 11 files changed, 394 insertions(+), 354 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/36221/1
diff --git a/src/Kconfig b/src/Kconfig index 4c71f28..8fd3d78 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -335,6 +335,21 @@ Skip PCI enumeration logic and only allocate BAR for fixed devices (bootable devices, TPM over GSPI).
+config CONFIGURABLE_RAMSTAGE + bool "Enable a configurable ramstage." + default y if ARCH_X86 + help + A configurable ramstage allows you to select which parts of the ramstage + to run. Currently, we can only select a minimal PCI scanning step. + The minimal PCI scanning will only check those parts that are enabled + in the devicetree.cb. By convention none of those devices should be bridges. + +config MINIMAL_PCI_SCANNING + bool "Enable minimal PCI scanning" + depends on CONFIGURABLE_RAMSTAGE + help + If this option is enabled, coreboot will scan only devices + marked as mandatory in devicetree.cb endmenu
menu "Mainboard" diff --git a/src/device/pci_device.c b/src/device/pci_device.c index 191c846..4cd0401 100644 --- a/src/device/pci_device.c +++ b/src/device/pci_device.c @@ -1187,6 +1187,20 @@ * non-existence and single function devices. */ for (devfn = min_devfn; devfn <= max_devfn; devfn++) { + /* When we are not configuring anything, we don't look through + * bridges. Hence, only devices on bus 0 matter. Since we don't + * look through bridges, we don't need to worry about getting here + * for any bus that is non-zero. */ + if (!CONFIG(MINIMAL_PCI_SCANNING)) { + dev = dev_find_slot(0, devfn); + if (!dev) { + printk(BIOS_WARNING, "%#x is NOT static, skipping it\n", devfn); + continue; + } + if (!dev->mandatory) + continue; + } + /* First thing setup the device structure. */ dev = pci_scan_get_dev(bus, devfn);
diff --git a/src/include/device/device.h b/src/include/device/device.h index 405d816e..db36ed3 100644 --- a/src/include/device/device.h +++ b/src/include/device/device.h @@ -129,6 +129,7 @@ unsigned int on_mainboard : 1; unsigned int disable_pcie_aspm : 1; unsigned int hidden : 1; /* set if we should hide from UI */ + unsigned int mandatory : 1; /* set if this device is used even in minimum PCI cases */ u8 command;
/* Base registers for this device. I/O, MEM and Expansion ROM */ diff --git a/src/mainboard/emulation/qemu-q35/devicetree.cb b/src/mainboard/emulation/qemu-q35/devicetree.cb index 671a2d6..fb42e8a 100644 --- a/src/mainboard/emulation/qemu-q35/devicetree.cb +++ b/src/mainboard/emulation/qemu-q35/devicetree.cb @@ -5,10 +5,10 @@ end end device domain 0 on - device pci 0.0 on end # northbridge (q35) + device pci 0.0 mandatory end # northbridge (q35) chip southbridge/intel/i82801ix # present unconditionally - device pci 1f.0 on end # LPC + device pci 1f.0 mandatory end # LPC device pci 1f.2 on end # SATA device pci 1f.3 on end # SMBus
@@ -19,7 +19,7 @@ device pci 1a.2 on end # UHCI #6 device pci 1a.7 on end # EHCI #2 device pci 1b.0 on end # HD Audio - device pci 1c.0 on end # PCIe Port #1 + device pci 1c.0 mandatory end # PCIe Port #1 device pci 1c.1 on end # PCIe Port #2 device pci 1c.2 on end # PCIe Port #3 device pci 1c.3 on end # PCIe Port #4 diff --git a/util/sconfig/lex.yy.c_shipped b/util/sconfig/lex.yy.c_shipped index 14ffeff..c8cc44d 100644 --- a/util/sconfig/lex.yy.c_shipped +++ b/util/sconfig/lex.yy.c_shipped @@ -6,7 +6,7 @@ #define FLEX_SCANNER #define YY_FLEX_MAJOR_VERSION 2 #define YY_FLEX_MINOR_VERSION 6 -#define YY_FLEX_SUBMINOR_VERSION 1 +#define YY_FLEX_SUBMINOR_VERSION 4 #if YY_FLEX_SUBMINOR_VERSION > 0 #define FLEX_BETA #endif @@ -81,10 +81,16 @@ #define UINT32_MAX (4294967295U) #endif
+#ifndef SIZE_MAX +#define SIZE_MAX (~(size_t)0) +#endif + #endif /* ! C99 */
#endif /* ! FLEXINT_H */
+/* begin standard C++ headers. */ + /* TODO: this is always defined, so inline it */ #define yyconst const
@@ -97,32 +103,26 @@ /* Returned upon end-of-file. */ #define YY_NULL 0
-/* Promotes a possibly negative, possibly signed char to an unsigned - * integer for use as an array index. If the signed char is negative, - * we want to instead treat it as an 8-bit unsigned char, hence the - * double cast. +/* Promotes a possibly negative, possibly signed char to an + * integer in range [0..255] for use as an array index. */ -#define YY_SC_TO_UI(c) ((unsigned int) (unsigned char) c) +#define YY_SC_TO_UI(c) ((YY_CHAR) (c))
/* Enter a start condition. This macro really ought to take a parameter, * but we do it the disgusting crufty way forced on us by the ()-less * definition of BEGIN. */ #define BEGIN (yy_start) = 1 + 2 * - /* Translate the current start state into a value that can be later handed * to BEGIN to return to the state. The YYSTATE alias is for lex * compatibility. */ #define YY_START (((yy_start) - 1) / 2) #define YYSTATE YY_START - /* Action number for EOF rule of a given start state. */ #define YY_STATE_EOF(state) (YY_END_OF_BUFFER + state + 1) - /* Special action meaning "start processing a new file". */ -#define YY_NEW_FILE yyrestart(yyin ) - +#define YY_NEW_FILE yyrestart( yyin ) #define YY_END_OF_BUFFER_CHAR 0
/* Size of default input buffer. */ @@ -159,7 +159,7 @@ #define EOB_ACT_CONTINUE_SCAN 0 #define EOB_ACT_END_OF_FILE 1 #define EOB_ACT_LAST_MATCH 2 - + #define YY_LESS_LINENO(n) #define YY_LINENO_REWIND_TO(ptr)
@@ -168,7 +168,7 @@ do \ { \ /* Undo effects of setting up yytext. */ \ - yy_size_t yyless_macro_arg = (n); \ + int yyless_macro_arg = (n); \ YY_LESS_LINENO(yyless_macro_arg);\ *yy_cp = (yy_hold_char); \ YY_RESTORE_YY_MORE_OFFSET \ @@ -176,7 +176,6 @@ YY_DO_BEFORE_ACTION; /* set up yytext again */ \ } \ while ( 0 ) - #define unput(c) yyunput( c, (yytext_ptr) )
#ifndef YY_STRUCT_YY_BUFFER_STATE @@ -258,7 +257,6 @@ #define YY_CURRENT_BUFFER ( (yy_buffer_stack) \ ? (yy_buffer_stack)[(yy_buffer_stack_top)] \ : NULL) - /* Same as previous macro, but useful when we know that the buffer stack is not * NULL or when we need an lvalue. For internal use only. */ @@ -279,62 +277,56 @@ */ static int yy_did_buffer_switch_on_eof;
-void yyrestart (FILE *input_file ); -void yy_switch_to_buffer (YY_BUFFER_STATE new_buffer ); -YY_BUFFER_STATE yy_create_buffer (FILE *file,int size ); -void yy_delete_buffer (YY_BUFFER_STATE b ); -void yy_flush_buffer (YY_BUFFER_STATE b ); -void yypush_buffer_state (YY_BUFFER_STATE new_buffer ); -void yypop_buffer_state (void ); +void yyrestart ( FILE *input_file ); +void yy_switch_to_buffer ( YY_BUFFER_STATE new_buffer ); +YY_BUFFER_STATE yy_create_buffer ( FILE *file, int size ); +void yy_delete_buffer ( YY_BUFFER_STATE b ); +void yy_flush_buffer ( YY_BUFFER_STATE b ); +void yypush_buffer_state ( YY_BUFFER_STATE new_buffer ); +void yypop_buffer_state ( void );
-static void yyensure_buffer_stack (void ); -static void yy_load_buffer_state (void ); -static void yy_init_buffer (YY_BUFFER_STATE b,FILE *file ); +static void yyensure_buffer_stack ( void ); +static void yy_load_buffer_state ( void ); +static void yy_init_buffer ( YY_BUFFER_STATE b, FILE *file ); +#define YY_FLUSH_BUFFER yy_flush_buffer( YY_CURRENT_BUFFER )
-#define YY_FLUSH_BUFFER yy_flush_buffer(YY_CURRENT_BUFFER ) +YY_BUFFER_STATE yy_scan_buffer ( char *base, yy_size_t size ); +YY_BUFFER_STATE yy_scan_string ( const char *yy_str ); +YY_BUFFER_STATE yy_scan_bytes ( const char *bytes, int len );
-YY_BUFFER_STATE yy_scan_buffer (char *base,yy_size_t size ); -YY_BUFFER_STATE yy_scan_string (yyconst char *yy_str ); -YY_BUFFER_STATE yy_scan_bytes (yyconst char *bytes,int len ); - -void *yyalloc (yy_size_t ); -void *yyrealloc (void *,yy_size_t ); -void yyfree (void * ); +void *yyalloc ( yy_size_t ); +void *yyrealloc ( void *, yy_size_t ); +void yyfree ( void * );
#define yy_new_buffer yy_create_buffer - #define yy_set_interactive(is_interactive) \ { \ if ( ! YY_CURRENT_BUFFER ){ \ yyensure_buffer_stack (); \ YY_CURRENT_BUFFER_LVALUE = \ - yy_create_buffer(yyin,YY_BUF_SIZE ); \ + yy_create_buffer( yyin, YY_BUF_SIZE ); \ } \ YY_CURRENT_BUFFER_LVALUE->yy_is_interactive = is_interactive; \ } - #define yy_set_bol(at_bol) \ { \ if ( ! YY_CURRENT_BUFFER ){\ yyensure_buffer_stack (); \ YY_CURRENT_BUFFER_LVALUE = \ - yy_create_buffer(yyin,YY_BUF_SIZE ); \ + yy_create_buffer( yyin, YY_BUF_SIZE ); \ } \ YY_CURRENT_BUFFER_LVALUE->yy_at_bol = at_bol; \ } - #define YY_AT_BOL() (YY_CURRENT_BUFFER_LVALUE->yy_at_bol)
/* Begin user sect3 */ - -typedef unsigned char YY_CHAR; +typedef flex_uint8_t YY_CHAR;
FILE *yyin = NULL, *yyout = NULL;
typedef int yy_state_type;
extern int yylineno; - int yylineno = 1;
extern char *yytext; @@ -343,10 +335,10 @@ #endif #define yytext_ptr yytext
-static yy_state_type yy_get_previous_state (void ); -static yy_state_type yy_try_NUL_trans (yy_state_type current_state ); -static int yy_get_next_buffer (void ); -static void yynoreturn yy_fatal_error (yyconst char* msg ); +static yy_state_type yy_get_previous_state ( void ); +static yy_state_type yy_try_NUL_trans ( yy_state_type current_state ); +static int yy_get_next_buffer ( void ); +static void yynoreturn yy_fatal_error ( const char* msg );
/* Done after the current pattern has been matched and before the * corresponding action - sets up yytext. @@ -357,9 +349,8 @@ (yy_hold_char) = *yy_cp; \ *yy_cp = '\0'; \ (yy_c_buf_p) = yy_cp; - -#define YY_NUM_RULES 38 -#define YY_END_OF_BUFFER 39 +#define YY_NUM_RULES 39 +#define YY_END_OF_BUFFER 40 /* This struct is not used in this scanner, but its presence is necessary. */ struct yy_trans_info @@ -367,28 +358,29 @@ flex_int32_t yy_verify; flex_int32_t yy_nxt; }; -static yyconst flex_int16_t yy_accept[160] = +static const flex_int16_t yy_accept[168] = { 0, - 0, 0, 39, 37, 1, 3, 37, 37, 37, 32, - 32, 30, 33, 37, 33, 33, 33, 37, 37, 37, - 37, 37, 37, 37, 37, 37, 37, 37, 1, 3, - 37, 0, 37, 37, 0, 2, 32, 33, 37, 37, - 37, 37, 33, 37, 37, 37, 37, 37, 37, 37, - 24, 37, 37, 37, 37, 7, 37, 37, 37, 37, - 37, 37, 37, 36, 36, 37, 0, 31, 37, 37, - 16, 37, 37, 23, 28, 37, 37, 13, 37, 37, - 22, 37, 37, 8, 10, 12, 37, 37, 20, 37, - 21, 37, 0, 34, 4, 37, 37, 37, 37, 37, + 0, 0, 40, 38, 1, 3, 38, 38, 38, 33, + 33, 31, 34, 38, 34, 34, 34, 38, 38, 38, + 38, 38, 38, 38, 38, 38, 38, 38, 1, 3, + 38, 0, 38, 38, 0, 2, 33, 34, 38, 38, + 38, 38, 34, 38, 38, 38, 38, 38, 38, 38, + 25, 38, 38, 38, 38, 38, 7, 38, 38, 38, + 38, 38, 38, 38, 37, 37, 38, 0, 32, 38, + 38, 17, 38, 38, 24, 29, 38, 38, 14, 38, + 38, 23, 38, 38, 38, 8, 11, 13, 38, 38, + 21, 38, 22, 38, 0, 35, 4, 38, 38, 38,
- 37, 37, 37, 19, 37, 37, 37, 35, 35, 37, - 37, 37, 37, 37, 37, 37, 14, 37, 37, 37, - 37, 5, 17, 37, 9, 37, 11, 37, 37, 37, - 37, 18, 26, 37, 37, 37, 37, 37, 37, 6, - 37, 37, 37, 37, 37, 37, 37, 25, 37, 37, - 15, 37, 27, 37, 37, 37, 37, 29, 0 + 38, 38, 38, 38, 38, 38, 20, 38, 38, 38, + 36, 36, 38, 38, 38, 38, 38, 38, 38, 15, + 38, 38, 38, 38, 38, 5, 18, 38, 9, 38, + 12, 38, 38, 38, 38, 38, 19, 27, 38, 38, + 38, 38, 38, 38, 38, 38, 6, 38, 38, 38, + 38, 10, 38, 38, 38, 26, 38, 38, 16, 38, + 28, 38, 38, 38, 38, 30, 0 } ;
-static yyconst YY_CHAR yy_ec[256] = +static const YY_CHAR yy_ec[256] = { 0, 1, 1, 1, 1, 1, 1, 1, 1, 2, 3, 1, 1, 4, 1, 1, 1, 1, 1, 1, 1, @@ -420,7 +412,7 @@ 1, 1, 1, 1, 1 } ;
-static yyconst YY_CHAR yy_meta[39] = +static const YY_CHAR yy_meta[39] = { 0, 1, 2, 2, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, @@ -428,114 +420,118 @@ 1, 1, 1, 1, 1, 1, 1, 1 } ;
-static yyconst flex_uint16_t yy_base[167] = +static const flex_int16_t yy_base[175] = { 0, - 0, 0, 227, 0, 224, 228, 222, 37, 41, 38, - 187, 0, 44, 209, 54, 78, 60, 201, 196, 45, - 203, 192, 42, 47, 197, 62, 184, 0, 214, 228, - 77, 210, 88, 69, 211, 228, 0, 87, 104, 198, - 187, 176, 93, 183, 178, 188, 179, 186, 186, 180, - 186, 171, 171, 175, 177, 0, 173, 167, 173, 177, - 169, 175, 174, 0, 228, 101, 186, 0, 179, 159, - 172, 162, 169, 0, 0, 164, 164, 0, 162, 152, - 0, 156, 151, 0, 0, 0, 154, 153, 0, 144, - 0, 171, 170, 0, 0, 155, 154, 147, 139, 149, + 0, 0, 235, 0, 232, 236, 230, 37, 41, 38, + 195, 0, 44, 217, 54, 78, 60, 209, 204, 45, + 211, 48, 42, 52, 206, 62, 193, 0, 223, 236, + 88, 219, 93, 79, 220, 236, 0, 93, 104, 207, + 196, 185, 96, 192, 187, 197, 188, 195, 195, 189, + 195, 180, 180, 181, 183, 185, 0, 181, 175, 181, + 185, 177, 183, 182, 0, 236, 115, 194, 0, 187, + 167, 180, 170, 177, 0, 0, 172, 172, 0, 170, + 160, 0, 164, 168, 158, 0, 0, 0, 161, 160, + 0, 151, 0, 178, 177, 0, 0, 162, 161, 154,
- 137, 143, 148, 0, 133, 136, 126, 0, 228, 137, - 141, 133, 135, 131, 133, 138, 0, 122, 122, 121, - 118, 0, 0, 133, 0, 117, 134, 128, 132, 113, - 113, 0, 0, 120, 112, 110, 121, 94, 95, 0, - 94, 92, 97, 86, 85, 84, 76, 0, 71, 78, - 0, 67, 0, 61, 55, 32, 29, 0, 228, 40, - 129, 131, 133, 135, 137, 139 + 146, 156, 144, 150, 155, 156, 0, 139, 142, 132, + 0, 236, 143, 147, 139, 141, 137, 139, 144, 0, + 128, 127, 127, 126, 123, 0, 0, 138, 0, 122, + 139, 125, 132, 136, 117, 117, 0, 0, 124, 116, + 115, 113, 124, 97, 98, 91, 0, 102, 100, 98, + 83, 0, 80, 83, 74, 0, 60, 63, 0, 63, + 0, 56, 51, 33, 29, 0, 236, 40, 132, 134, + 136, 138, 140, 142 } ;
-static yyconst flex_int16_t yy_def[167] = +static const flex_int16_t yy_def[175] = { 0, - 159, 1, 159, 160, 159, 159, 160, 161, 162, 160, - 10, 160, 10, 160, 10, 10, 10, 160, 160, 160, - 160, 160, 160, 160, 160, 160, 160, 160, 159, 159, - 161, 163, 164, 162, 165, 159, 10, 10, 10, 160, - 160, 160, 10, 160, 160, 160, 160, 160, 160, 160, - 160, 160, 160, 160, 160, 160, 160, 160, 160, 160, - 160, 160, 160, 160, 159, 164, 166, 39, 160, 160, - 160, 160, 160, 160, 160, 160, 160, 160, 160, 160, - 160, 160, 160, 160, 160, 160, 160, 160, 160, 160, - 160, 160, 159, 160, 160, 160, 160, 160, 160, 160, + 167, 1, 167, 168, 167, 167, 168, 169, 170, 168, + 10, 168, 10, 168, 10, 10, 10, 168, 168, 168, + 168, 168, 168, 168, 168, 168, 168, 168, 167, 167, + 169, 171, 172, 170, 173, 167, 10, 10, 10, 168, + 168, 168, 10, 168, 168, 168, 168, 168, 168, 168, + 168, 168, 168, 168, 168, 168, 168, 168, 168, 168, + 168, 168, 168, 168, 168, 167, 172, 174, 39, 168, + 168, 168, 168, 168, 168, 168, 168, 168, 168, 168, + 168, 168, 168, 168, 168, 168, 168, 168, 168, 168, + 168, 168, 168, 168, 167, 168, 168, 168, 168, 168,
- 160, 160, 160, 160, 160, 160, 160, 160, 159, 160, - 160, 160, 160, 160, 160, 160, 160, 160, 160, 160, - 160, 160, 160, 160, 160, 160, 160, 160, 160, 160, - 160, 160, 160, 160, 160, 160, 160, 160, 160, 160, - 160, 160, 160, 160, 160, 160, 160, 160, 160, 160, - 160, 160, 160, 160, 160, 160, 160, 160, 0, 159, - 159, 159, 159, 159, 159, 159 + 168, 168, 168, 168, 168, 168, 168, 168, 168, 168, + 168, 167, 168, 168, 168, 168, 168, 168, 168, 168, + 168, 168, 168, 168, 168, 168, 168, 168, 168, 168, + 168, 168, 168, 168, 168, 168, 168, 168, 168, 168, + 168, 168, 168, 168, 168, 168, 168, 168, 168, 168, + 168, 168, 168, 168, 168, 168, 168, 168, 168, 168, + 168, 168, 168, 168, 168, 168, 0, 167, 167, 167, + 167, 167, 167, 167 } ;
-static yyconst flex_uint16_t yy_nxt[267] = +static const flex_int16_t yy_nxt[275] = { 0, 4, 5, 6, 7, 8, 9, 10, 11, 10, 12, 13, 13, 14, 4, 4, 4, 13, 13, 15, 16, 17, 13, 18, 19, 20, 21, 22, 4, 23, 24, 4, 25, 26, 4, 27, 4, 4, 4, 32, 32, - 28, 33, 35, 36, 37, 37, 37, 158, 38, 38, + 28, 33, 35, 36, 37, 37, 37, 166, 38, 38, 38, 38, 38, 49, 38, 38, 38, 38, 38, 38, - 38, 38, 38, 55, 157, 57, 38, 38, 38, 56, - 35, 36, 50, 51, 58, 156, 52, 41, 32, 32, - 155, 64, 154, 42, 38, 38, 38, 46, 60, 67, - 67, 61, 28, 38, 38, 38, 62, 153, 43, 38, + 38, 38, 38, 56, 54, 165, 38, 38, 38, 57, + 58, 164, 50, 51, 55, 163, 52, 41, 162, 59, + 35, 36, 161, 42, 38, 38, 38, 46, 61, 32, + 32, 62, 65, 160, 68, 68, 63, 28, 43, 38,
- 38, 38, 67, 67, 152, 92, 44, 151, 150, 45, - 68, 68, 68, 149, 68, 68, 148, 147, 146, 145, - 68, 68, 68, 68, 68, 68, 144, 143, 72, 31, - 31, 34, 34, 32, 32, 66, 66, 35, 35, 67, - 67, 142, 141, 140, 139, 138, 137, 136, 135, 134, + 38, 38, 38, 38, 38, 159, 44, 158, 157, 45, + 69, 69, 69, 156, 69, 69, 68, 68, 155, 94, + 69, 69, 69, 69, 69, 69, 154, 153, 152, 151, + 150, 73, 31, 31, 34, 34, 32, 32, 67, 67, + 35, 35, 68, 68, 149, 148, 147, 146, 145, 144, + 143, 142, 141, 140, 139, 138, 137, 136, 135, 134, 133, 132, 131, 130, 129, 128, 127, 126, 125, 124, 123, 122, 121, 120, 119, 118, 117, 116, 115, 114, 113, 112, 111, 110, 109, 108, 107, 106, 105, 104, - 103, 102, 101, 100, 99, 98, 97, 96, 95, 94, - 93, 91, 90, 89, 88, 87, 86, 85, 84, 83, + 103, 102, 101, 100, 99, 98, 97, 96, 95, 93,
- 82, 81, 80, 79, 78, 77, 76, 75, 74, 73, - 71, 70, 69, 36, 65, 29, 63, 59, 54, 53, - 48, 47, 40, 39, 30, 29, 159, 3, 159, 159, - 159, 159, 159, 159, 159, 159, 159, 159, 159, 159, - 159, 159, 159, 159, 159, 159, 159, 159, 159, 159, - 159, 159, 159, 159, 159, 159, 159, 159, 159, 159, - 159, 159, 159, 159, 159, 159 + 92, 91, 90, 89, 88, 87, 86, 85, 84, 83, + 82, 81, 80, 79, 78, 77, 76, 75, 74, 72, + 71, 70, 36, 66, 29, 64, 60, 53, 48, 47, + 40, 39, 30, 29, 167, 3, 167, 167, 167, 167, + 167, 167, 167, 167, 167, 167, 167, 167, 167, 167, + 167, 167, 167, 167, 167, 167, 167, 167, 167, 167, + 167, 167, 167, 167, 167, 167, 167, 167, 167, 167, + 167, 167, 167, 167 } ;
-static yyconst flex_int16_t yy_chk[267] = +static const flex_int16_t yy_chk[275] = { 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 8, 8, - 160, 8, 9, 9, 10, 10, 10, 157, 10, 10, + 168, 8, 9, 9, 10, 10, 10, 165, 10, 10, 13, 13, 13, 20, 10, 10, 10, 10, 10, 10, - 15, 15, 15, 23, 156, 24, 17, 17, 17, 23, - 34, 34, 20, 20, 24, 155, 20, 15, 31, 31, - 154, 31, 152, 15, 16, 16, 16, 17, 26, 33, - 33, 26, 33, 38, 38, 38, 26, 150, 16, 43, + 15, 15, 15, 23, 22, 164, 17, 17, 17, 23, + 24, 163, 20, 20, 22, 162, 20, 15, 160, 24, + 34, 34, 158, 15, 16, 16, 16, 17, 26, 31, + 31, 26, 31, 157, 33, 33, 26, 33, 16, 38,
- 43, 43, 66, 66, 149, 66, 16, 147, 146, 16, - 39, 39, 39, 145, 39, 39, 144, 143, 142, 141, - 39, 39, 39, 39, 39, 39, 139, 138, 43, 161, - 161, 162, 162, 163, 163, 164, 164, 165, 165, 166, - 166, 137, 136, 135, 134, 131, 130, 129, 128, 127, - 126, 124, 121, 120, 119, 118, 116, 115, 114, 113, - 112, 111, 110, 107, 106, 105, 103, 102, 101, 100, - 99, 98, 97, 96, 93, 92, 90, 88, 87, 83, - 82, 80, 79, 77, 76, 73, 72, 71, 70, 69, - 67, 63, 62, 61, 60, 59, 58, 57, 55, 54, + 38, 38, 43, 43, 43, 155, 16, 154, 153, 16, + 39, 39, 39, 151, 39, 39, 67, 67, 150, 67, + 39, 39, 39, 39, 39, 39, 149, 148, 146, 145, + 144, 43, 169, 169, 170, 170, 171, 171, 172, 172, + 173, 173, 174, 174, 143, 142, 141, 140, 139, 136, + 135, 134, 133, 132, 131, 130, 128, 125, 124, 123, + 122, 121, 119, 118, 117, 116, 115, 114, 113, 110, + 109, 108, 106, 105, 104, 103, 102, 101, 100, 99, + 98, 95, 94, 92, 90, 89, 85, 84, 83, 81, + 80, 78, 77, 74, 73, 72, 71, 70, 68, 64,
- 53, 52, 51, 50, 49, 48, 47, 46, 45, 44, - 42, 41, 40, 35, 32, 29, 27, 25, 22, 21, - 19, 18, 14, 11, 7, 5, 3, 159, 159, 159, - 159, 159, 159, 159, 159, 159, 159, 159, 159, 159, - 159, 159, 159, 159, 159, 159, 159, 159, 159, 159, - 159, 159, 159, 159, 159, 159, 159, 159, 159, 159, - 159, 159, 159, 159, 159, 159 + 63, 62, 61, 60, 59, 58, 56, 55, 54, 53, + 52, 51, 50, 49, 48, 47, 46, 45, 44, 42, + 41, 40, 35, 32, 29, 27, 25, 21, 19, 18, + 14, 11, 7, 5, 3, 167, 167, 167, 167, 167, + 167, 167, 167, 167, 167, 167, 167, 167, 167, 167, + 167, 167, 167, 167, 167, 167, 167, 167, 167, 167, + 167, 167, 167, 167, 167, 167, 167, 167, 167, 167, + 167, 167, 167, 167 } ;
static yy_state_type yy_last_accepting_state; @@ -586,36 +582,36 @@ #define YY_EXTRA_TYPE void * #endif
-static int yy_init_globals (void ); +static int yy_init_globals ( void );
/* Accessor methods to globals. These are made visible to non-reentrant scanners for convenience. */
-int yylex_destroy (void ); +int yylex_destroy ( void );
-int yyget_debug (void ); +int yyget_debug ( void );
-void yyset_debug (int debug_flag ); +void yyset_debug ( int debug_flag );
-YY_EXTRA_TYPE yyget_extra (void ); +YY_EXTRA_TYPE yyget_extra ( void );
-void yyset_extra (YY_EXTRA_TYPE user_defined ); +void yyset_extra ( YY_EXTRA_TYPE user_defined );
-FILE *yyget_in (void ); +FILE *yyget_in ( void );
-void yyset_in (FILE * _in_str ); +void yyset_in ( FILE * _in_str );
-FILE *yyget_out (void ); +FILE *yyget_out ( void );
-void yyset_out (FILE * _out_str ); +void yyset_out ( FILE * _out_str );
- int yyget_leng (void ); + int yyget_leng ( void );
-char *yyget_text (void ); +char *yyget_text ( void );
-int yyget_lineno (void ); +int yyget_lineno ( void );
-void yyset_lineno (int _line_number ); +void yyset_lineno ( int _line_number );
/* Macros after this point can all be overridden by user definitions in * section 1. @@ -623,32 +619,31 @@
#ifndef YY_SKIP_YYWRAP #ifdef __cplusplus -extern "C" int yywrap (void ); +extern "C" int yywrap ( void ); #else -extern int yywrap (void ); +extern int yywrap ( void ); #endif #endif
#ifndef YY_NO_UNPUT
- static void yyunput (int c,char *buf_ptr ); + static void yyunput ( int c, char *buf_ptr );
#endif
#ifndef yytext_ptr -static void yy_flex_strncpy (char *,yyconst char *,int ); +static void yy_flex_strncpy ( char *, const char *, int ); #endif
#ifdef YY_NEED_STRLEN -static int yy_flex_strlen (yyconst char * ); +static int yy_flex_strlen ( const char * ); #endif
#ifndef YY_NO_INPUT - #ifdef __cplusplus -static int yyinput (void ); +static int yyinput ( void ); #else -static int input (void ); +static int input ( void ); #endif
#endif @@ -781,10 +776,10 @@ if ( ! YY_CURRENT_BUFFER ) { yyensure_buffer_stack (); YY_CURRENT_BUFFER_LVALUE = - yy_create_buffer(yyin,YY_BUF_SIZE ); + yy_create_buffer( yyin, YY_BUF_SIZE ); }
- yy_load_buffer_state( ); + yy_load_buffer_state( ); }
{ @@ -814,13 +809,13 @@ while ( yy_chk[yy_base[yy_current_state] + yy_c] != yy_current_state ) { yy_current_state = (int) yy_def[yy_current_state]; - if ( yy_current_state >= 160 ) - yy_c = yy_meta[(unsigned int) yy_c]; + if ( yy_current_state >= 168 ) + yy_c = yy_meta[yy_c]; } - yy_current_state = yy_nxt[yy_base[yy_current_state] + (flex_int16_t) yy_c]; + yy_current_state = yy_nxt[yy_base[yy_current_state] + yy_c]; ++yy_cp; } - while ( yy_base[yy_current_state] != 228 ); + while ( yy_base[yy_current_state] != 236 );
yy_find_action: yy_act = yy_accept[yy_current_state]; @@ -880,95 +875,95 @@ YY_BREAK case 9: YY_RULE_SETUP -{yylval.number=3; return(HIDDEN);} +{yylval.number=3; return(STATUS);} YY_BREAK case 10: YY_RULE_SETUP -{yylval.number=PCI; return(BUS);} +{yylval.number=5; return(STATUS);} YY_BREAK case 11: YY_RULE_SETUP -{yylval.number=IOAPIC; return(BUS);} +{yylval.number=PCI; return(BUS);} YY_BREAK case 12: YY_RULE_SETUP -{yylval.number=PNP; return(BUS);} +{yylval.number=IOAPIC; return(BUS);} YY_BREAK case 13: YY_RULE_SETUP -{yylval.number=I2C; return(BUS);} +{yylval.number=PNP; return(BUS);} YY_BREAK case 14: YY_RULE_SETUP -{yylval.number=APIC; return(BUS);} +{yylval.number=I2C; return(BUS);} YY_BREAK case 15: YY_RULE_SETUP -{yylval.number=CPU_CLUSTER; return(BUS);} +{yylval.number=APIC; return(BUS);} YY_BREAK case 16: YY_RULE_SETUP -{yylval.number=CPU; return(BUS);} +{yylval.number=CPU_CLUSTER; return(BUS);} YY_BREAK case 17: YY_RULE_SETUP -{yylval.number=DOMAIN; return(BUS);} +{yylval.number=CPU; return(BUS);} YY_BREAK case 18: YY_RULE_SETUP -{yylval.number=GENERIC; return(BUS);} +{yylval.number=DOMAIN; return(BUS);} YY_BREAK case 19: YY_RULE_SETUP -{yylval.number=MMIO; return(BUS);} +{yylval.number=GENERIC; return(BUS);} YY_BREAK case 20: YY_RULE_SETUP -{yylval.number=SPI; return(BUS);} +{yylval.number=MMIO; return(BUS);} YY_BREAK case 21: YY_RULE_SETUP -{yylval.number=USB; return(BUS);} +{yylval.number=SPI; return(BUS);} YY_BREAK case 22: YY_RULE_SETUP -{yylval.number=IRQ; return(RESOURCE);} +{yylval.number=USB; return(BUS);} YY_BREAK case 23: YY_RULE_SETUP -{yylval.number=DRQ; return(RESOURCE);} +{yylval.number=IRQ; return(RESOURCE);} YY_BREAK case 24: YY_RULE_SETUP -{yylval.number=IO; return(RESOURCE);} +{yylval.number=DRQ; return(RESOURCE);} YY_BREAK case 25: YY_RULE_SETUP -{return(IOAPIC_IRQ);} +{yylval.number=IO; return(RESOURCE);} YY_BREAK case 26: YY_RULE_SETUP -{return(INHERIT);} +{return(IOAPIC_IRQ);} YY_BREAK case 27: YY_RULE_SETUP -{return(SUBSYSTEMID);} +{return(INHERIT);} YY_BREAK case 28: YY_RULE_SETUP -{return(END);} +{return(SUBSYSTEMID);} YY_BREAK case 29: YY_RULE_SETUP -{return(SLOT_DESC);} +{return(END);} YY_BREAK case 30: YY_RULE_SETUP -{return(EQUALS);} +{return(SLOT_DESC);} YY_BREAK case 31: YY_RULE_SETUP -{yylval.string = malloc(yyleng+1); strncpy(yylval.string, yytext, yyleng); yylval.string[yyleng]='\0'; return(NUMBER);} +{return(EQUALS);} YY_BREAK case 32: YY_RULE_SETUP @@ -980,12 +975,11 @@ YY_BREAK case 34: YY_RULE_SETUP -{yylval.string = malloc(yyleng+1); strncpy(yylval.string, yytext, yyleng); yylval.string[yyleng]='\0'; return(PCIINT);} +{yylval.string = malloc(yyleng+1); strncpy(yylval.string, yytext, yyleng); yylval.string[yyleng]='\0'; return(NUMBER);} YY_BREAK case 35: -/* rule 35 can match eol */ YY_RULE_SETUP -{yylval.string = malloc(yyleng-1); strncpy(yylval.string, yytext+1, yyleng-2); yylval.string[yyleng-2]='\0'; return(STRING);} +{yylval.string = malloc(yyleng+1); strncpy(yylval.string, yytext, yyleng); yylval.string[yyleng]='\0'; return(PCIINT);} YY_BREAK case 36: /* rule 36 can match eol */ @@ -993,10 +987,15 @@ {yylval.string = malloc(yyleng-1); strncpy(yylval.string, yytext+1, yyleng-2); yylval.string[yyleng-2]='\0'; return(STRING);} YY_BREAK case 37: +/* rule 37 can match eol */ +YY_RULE_SETUP +{yylval.string = malloc(yyleng-1); strncpy(yylval.string, yytext+1, yyleng-2); yylval.string[yyleng-2]='\0'; return(STRING);} + YY_BREAK +case 38: YY_RULE_SETUP {yylval.string = malloc(yyleng+1); strncpy(yylval.string, yytext, yyleng); yylval.string[yyleng]='\0'; return(STRING);} YY_BREAK -case 38: +case 39: YY_RULE_SETUP ECHO; YY_BREAK @@ -1077,7 +1076,7 @@ { (yy_did_buffer_switch_on_eof) = 0;
- if ( yywrap( ) ) + if ( yywrap( ) ) { /* Note: because we've taken care in * yy_get_next_buffer() to have set up @@ -1144,7 +1143,7 @@ { char *dest = YY_CURRENT_BUFFER_LVALUE->yy_ch_buf; char *source = (yytext_ptr); - yy_size_t number_to_move, i; + int number_to_move, i; int ret_val;
if ( (yy_c_buf_p) > &YY_CURRENT_BUFFER_LVALUE->yy_ch_buf[(yy_n_chars) + 1] ) @@ -1173,7 +1172,7 @@ /* Try to read more data. */
/* First move last chars to start of buffer. */ - number_to_move = (yy_size_t) ((yy_c_buf_p) - (yytext_ptr)) - 1; + number_to_move = (int) ((yy_c_buf_p) - (yytext_ptr) - 1);
for ( i = 0; i < number_to_move; ++i ) *(dest++) = *(source++); @@ -1209,7 +1208,8 @@
b->yy_ch_buf = (char *) /* Include room in for 2 EOB chars. */ - yyrealloc((void *) b->yy_ch_buf,(yy_size_t) (b->yy_buf_size + 2) ); + yyrealloc( (void *) b->yy_ch_buf, + (yy_size_t) (b->yy_buf_size + 2) ); } else /* Can't grow it, we don't own it. */ @@ -1241,7 +1241,7 @@ if ( number_to_move == YY_MORE_ADJ ) { ret_val = EOB_ACT_END_OF_FILE; - yyrestart(yyin ); + yyrestart( yyin ); }
else @@ -1255,12 +1255,15 @@ else ret_val = EOB_ACT_CONTINUE_SCAN;
- if ((int) ((yy_n_chars) + number_to_move) > YY_CURRENT_BUFFER_LVALUE->yy_buf_size) { + if (((yy_n_chars) + number_to_move) > YY_CURRENT_BUFFER_LVALUE->yy_buf_size) { /* Extend the array by 50%, plus the number we really need. */ int new_size = (yy_n_chars) + number_to_move + ((yy_n_chars) >> 1); - YY_CURRENT_BUFFER_LVALUE->yy_ch_buf = (char *) yyrealloc((void *) YY_CURRENT_BUFFER_LVALUE->yy_ch_buf,(yy_size_t) new_size ); + YY_CURRENT_BUFFER_LVALUE->yy_ch_buf = (char *) yyrealloc( + (void *) YY_CURRENT_BUFFER_LVALUE->yy_ch_buf, (yy_size_t) new_size ); if ( ! YY_CURRENT_BUFFER_LVALUE->yy_ch_buf ) YY_FATAL_ERROR( "out of dynamic memory in yy_get_next_buffer()" ); + /* "- 2" to take care of EOB's */ + YY_CURRENT_BUFFER_LVALUE->yy_buf_size = (int) (new_size - 2); }
(yy_n_chars) += number_to_move; @@ -1292,10 +1295,10 @@ while ( yy_chk[yy_base[yy_current_state] + yy_c] != yy_current_state ) { yy_current_state = (int) yy_def[yy_current_state]; - if ( yy_current_state >= 160 ) - yy_c = yy_meta[(unsigned int) yy_c]; + if ( yy_current_state >= 168 ) + yy_c = yy_meta[yy_c]; } - yy_current_state = yy_nxt[yy_base[yy_current_state] + (flex_int16_t) yy_c]; + yy_current_state = yy_nxt[yy_base[yy_current_state] + yy_c]; }
return yy_current_state; @@ -1320,11 +1323,11 @@ while ( yy_chk[yy_base[yy_current_state] + yy_c] != yy_current_state ) { yy_current_state = (int) yy_def[yy_current_state]; - if ( yy_current_state >= 160 ) - yy_c = yy_meta[(unsigned int) yy_c]; + if ( yy_current_state >= 168 ) + yy_c = yy_meta[yy_c]; } - yy_current_state = yy_nxt[yy_base[yy_current_state] + (flex_int16_t) yy_c]; - yy_is_jam = (yy_current_state == 159); + yy_current_state = yy_nxt[yy_base[yy_current_state] + yy_c]; + yy_is_jam = (yy_current_state == 167);
return yy_is_jam ? 0 : yy_current_state; } @@ -1394,7 +1397,7 @@
else { /* need more input */ - int offset = (yy_c_buf_p) - (yytext_ptr); + int offset = (int) ((yy_c_buf_p) - (yytext_ptr)); ++(yy_c_buf_p);
switch ( yy_get_next_buffer( ) ) @@ -1411,13 +1414,13 @@ */
/* Reset buffer status. */ - yyrestart(yyin ); + yyrestart( yyin );
/*FALLTHROUGH*/
case EOB_ACT_END_OF_FILE: { - if ( yywrap( ) ) + if ( yywrap( ) ) return 0;
if ( ! (yy_did_buffer_switch_on_eof) ) @@ -1455,11 +1458,11 @@ if ( ! YY_CURRENT_BUFFER ){ yyensure_buffer_stack (); YY_CURRENT_BUFFER_LVALUE = - yy_create_buffer(yyin,YY_BUF_SIZE ); + yy_create_buffer( yyin, YY_BUF_SIZE ); }
- yy_init_buffer(YY_CURRENT_BUFFER,input_file ); - yy_load_buffer_state( ); + yy_init_buffer( YY_CURRENT_BUFFER, input_file ); + yy_load_buffer_state( ); }
/** Switch to a different input buffer. @@ -1487,7 +1490,7 @@ }
YY_CURRENT_BUFFER_LVALUE = new_buffer; - yy_load_buffer_state( ); + yy_load_buffer_state( );
/* We don't actually know whether we did this switch during * EOF (yywrap()) processing, but the only time this flag @@ -1515,7 +1518,7 @@ { YY_BUFFER_STATE b;
- b = (YY_BUFFER_STATE) yyalloc(sizeof( struct yy_buffer_state ) ); + b = (YY_BUFFER_STATE) yyalloc( sizeof( struct yy_buffer_state ) ); if ( ! b ) YY_FATAL_ERROR( "out of dynamic memory in yy_create_buffer()" );
@@ -1524,13 +1527,13 @@ /* yy_ch_buf has to be 2 characters longer than the size given because * we need to put in 2 end-of-buffer characters. */ - b->yy_ch_buf = (char *) yyalloc((yy_size_t) (b->yy_buf_size + 2) ); + b->yy_ch_buf = (char *) yyalloc( (yy_size_t) (b->yy_buf_size + 2) ); if ( ! b->yy_ch_buf ) YY_FATAL_ERROR( "out of dynamic memory in yy_create_buffer()" );
b->yy_is_our_buffer = 1;
- yy_init_buffer(b,file ); + yy_init_buffer( b, file );
return b; } @@ -1549,9 +1552,9 @@ YY_CURRENT_BUFFER_LVALUE = (YY_BUFFER_STATE) 0;
if ( b->yy_is_our_buffer ) - yyfree((void *) b->yy_ch_buf ); + yyfree( (void *) b->yy_ch_buf );
- yyfree((void *) b ); + yyfree( (void *) b ); }
/* Initializes or reinitializes a buffer. @@ -1563,7 +1566,7 @@ { int oerrno = errno;
- yy_flush_buffer(b ); + yy_flush_buffer( b );
b->yy_input_file = file; b->yy_fill_buffer = 1; @@ -1606,7 +1609,7 @@ b->yy_buffer_status = YY_BUFFER_NEW;
if ( b == YY_CURRENT_BUFFER ) - yy_load_buffer_state( ); + yy_load_buffer_state( ); }
/** Pushes the new state onto the stack. The new state becomes @@ -1637,7 +1640,7 @@ YY_CURRENT_BUFFER_LVALUE = new_buffer;
/* copied from yy_switch_to_buffer. */ - yy_load_buffer_state( ); + yy_load_buffer_state( ); (yy_did_buffer_switch_on_eof) = 1; }
@@ -1656,7 +1659,7 @@ --(yy_buffer_stack_top);
if (YY_CURRENT_BUFFER) { - yy_load_buffer_state( ); + yy_load_buffer_state( ); (yy_did_buffer_switch_on_eof) = 1; } } @@ -1666,7 +1669,7 @@ */ static void yyensure_buffer_stack (void) { - int num_to_alloc; + yy_size_t num_to_alloc;
if (!(yy_buffer_stack)) {
@@ -1723,7 +1726,7 @@ /* They forgot to leave room for the EOB's. */ return NULL;
- b = (YY_BUFFER_STATE) yyalloc(sizeof( struct yy_buffer_state ) ); + b = (YY_BUFFER_STATE) yyalloc( sizeof( struct yy_buffer_state ) ); if ( ! b ) YY_FATAL_ERROR( "out of dynamic memory in yy_scan_buffer()" );
@@ -1737,7 +1740,7 @@ b->yy_fill_buffer = 0; b->yy_buffer_status = YY_BUFFER_NEW;
- yy_switch_to_buffer(b ); + yy_switch_to_buffer( b );
return b; } @@ -1750,10 +1753,10 @@ * @note If you want to scan bytes that may contain NUL values, then use * yy_scan_bytes() instead. */ -YY_BUFFER_STATE yy_scan_string (yyconst char * yystr ) +YY_BUFFER_STATE yy_scan_string (const char * yystr ) {
- return yy_scan_bytes(yystr,(int) strlen(yystr) ); + return yy_scan_bytes( yystr, (int) strlen(yystr) ); }
/** Setup the input buffer state to scan the given bytes. The next call to yylex() will @@ -1763,7 +1766,7 @@ * * @return the newly allocated buffer state object. */ -YY_BUFFER_STATE yy_scan_bytes (yyconst char * yybytes, int _yybytes_len ) +YY_BUFFER_STATE yy_scan_bytes (const char * yybytes, int _yybytes_len ) { YY_BUFFER_STATE b; char *buf; @@ -1772,7 +1775,7 @@
/* Get memory for full buffer, including space for trailing EOB's. */ n = (yy_size_t) (_yybytes_len + 2); - buf = (char *) yyalloc(n ); + buf = (char *) yyalloc( n ); if ( ! buf ) YY_FATAL_ERROR( "out of dynamic memory in yy_scan_bytes()" );
@@ -1781,7 +1784,7 @@
buf[_yybytes_len] = buf[_yybytes_len+1] = YY_END_OF_BUFFER_CHAR;
- b = yy_scan_buffer(buf,n ); + b = yy_scan_buffer( buf, n ); if ( ! b ) YY_FATAL_ERROR( "bad buffer in yy_scan_bytes()" );
@@ -1797,9 +1800,9 @@ #define YY_EXIT_FAILURE 2 #endif
-static void yynoreturn yy_fatal_error (yyconst char* msg ) +static void yynoreturn yy_fatal_error (const char* msg ) { - (void) fprintf( stderr, "%s\n", msg ); + fprintf( stderr, "%s\n", msg ); exit( YY_EXIT_FAILURE ); }
@@ -1810,7 +1813,7 @@ do \ { \ /* Undo effects of setting up yytext. */ \ - yy_size_t yyless_macro_arg = (n); \ + int yyless_macro_arg = (n); \ YY_LESS_LINENO(yyless_macro_arg);\ yytext[yyleng] = (yy_hold_char); \ (yy_c_buf_p) = yytext + yyless_macro_arg; \ @@ -1934,7 +1937,7 @@
/* Pop the buffer stack, destroying each element. */ while(YY_CURRENT_BUFFER){ - yy_delete_buffer(YY_CURRENT_BUFFER ); + yy_delete_buffer( YY_CURRENT_BUFFER ); YY_CURRENT_BUFFER_LVALUE = NULL; yypop_buffer_state(); } @@ -1955,7 +1958,7 @@ */
#ifndef yytext_ptr -static void yy_flex_strncpy (char* s1, yyconst char * s2, int n ) +static void yy_flex_strncpy (char* s1, const char * s2, int n ) { int i; @@ -1965,7 +1968,7 @@ #endif
#ifdef YY_NEED_STRLEN -static int yy_flex_strlen (yyconst char * s ) +static int yy_flex_strlen (const char * s ) { int n; for ( n = 0; s[n]; ++n ) diff --git a/util/sconfig/main.c b/util/sconfig/main.c index 5c23333..7844055 100644 --- a/util/sconfig/main.c +++ b/util/sconfig/main.c @@ -516,6 +516,7 @@
new_d->enabled = status & 0x01; new_d->hidden = (status >> 1) & 0x01; + new_d->mandatory = (status >> 2) & 0x01; new_d->chip_instance = chip_instance; chip_instance->ref_count++;
@@ -810,6 +811,7 @@ fprintf(fil, "},\n"); fprintf(fil, "\t.enabled = %d,\n", ptr->enabled); fprintf(fil, "\t.hidden = %d,\n", ptr->hidden); + fprintf(fil, "\t.mandatory = %d,\n", ptr->mandatory); fprintf(fil, "\t.on_mainboard = 1,\n"); if (ptr->subsystem_vendor > 0) fprintf(fil, "\t.subsystem_vendor = 0x%04x,\n", diff --git a/util/sconfig/sconfig.h b/util/sconfig/sconfig.h index eea2a14..60842f1 100644 --- a/util/sconfig/sconfig.h +++ b/util/sconfig/sconfig.h @@ -104,6 +104,8 @@ /* Indicates device status (enabled / hidden or not). */ int enabled; int hidden; + /* non-zero if the device should be included in all cases */ + int mandatory;
/* Subsystem IDs for the device. */ int subsystem_vendor; diff --git a/util/sconfig/sconfig.l b/util/sconfig/sconfig.l index 87de6e2..14eb965 100755 --- a/util/sconfig/sconfig.l +++ b/util/sconfig/sconfig.l @@ -29,7 +29,8 @@ register {return(REGISTER);} on {yylval.number=1; return(BOOL);} off {yylval.number=0; return(BOOL);} -hidden {yylval.number=3; return(HIDDEN);} +hidden {yylval.number=3; return(STATUS);} +mandatory {yylval.number=5; return(STATUS);} pci {yylval.number=PCI; return(BUS);} ioapic {yylval.number=IOAPIC; return(BUS);} pnp {yylval.number=PNP; return(BUS);} diff --git a/util/sconfig/sconfig.tab.c_shipped b/util/sconfig/sconfig.tab.c_shipped index 8e1e57d..f4335c7 100644 --- a/util/sconfig/sconfig.tab.c_shipped +++ b/util/sconfig/sconfig.tab.c_shipped @@ -1,8 +1,8 @@ -/* A Bison parser, made by GNU Bison 3.0.5. */ +/* A Bison parser, made by GNU Bison 3.0.4. */
/* Bison implementation for Yacc-like parsers in C
- Copyright (C) 1984, 1989-1990, 2000-2015, 2018 Free Software Foundation, Inc. + Copyright (C) 1984, 1989-1990, 2000-2015 Free Software Foundation, Inc.
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 @@ -44,7 +44,7 @@ #define YYBISON 1
/* Bison version. */ -#define YYBISON_VERSION "3.0.5" +#define YYBISON_VERSION "3.0.4"
/* Skeleton name. */ #define YYSKELETON_NAME "yacc.c" @@ -109,8 +109,8 @@
/* In a future release of Bison, this section will be replaced by #include "sconfig.tab.h_shipped". */ -#ifndef YY_YY_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED -# define YY_YY_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED +#ifndef YY_YY_HOME_RMINNICH_PROJECTS_LINUXBOOT_COREBOOTNERF_GITHUBCOREBOOT_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED +# define YY_YY_HOME_RMINNICH_PROJECTS_LINUXBOOT_COREBOOTNERF_GITHUBCOREBOOT_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED /* Debug traces. */ #ifndef YYDEBUG # define YYDEBUG 0 @@ -128,34 +128,35 @@ DEVICE = 259, REGISTER = 260, BOOL = 261, - HIDDEN = 262, - BUS = 263, - RESOURCE = 264, - END = 265, - EQUALS = 266, - HEX = 267, - STRING = 268, - PCI = 269, - PNP = 270, - I2C = 271, - APIC = 272, - CPU_CLUSTER = 273, - CPU = 274, - DOMAIN = 275, - IRQ = 276, - DRQ = 277, - SLOT_DESC = 278, - IO = 279, - NUMBER = 280, - SUBSYSTEMID = 281, - INHERIT = 282, - IOAPIC_IRQ = 283, - IOAPIC = 284, - PCIINT = 285, - GENERIC = 286, - SPI = 287, - USB = 288, - MMIO = 289 + STATUS = 262, + MANDATORY = 263, + BUS = 264, + RESOURCE = 265, + END = 266, + EQUALS = 267, + HEX = 268, + STRING = 269, + PCI = 270, + PNP = 271, + I2C = 272, + APIC = 273, + CPU_CLUSTER = 274, + CPU = 275, + DOMAIN = 276, + IRQ = 277, + DRQ = 278, + SLOT_DESC = 279, + IO = 280, + NUMBER = 281, + SUBSYSTEMID = 282, + INHERIT = 283, + IOAPIC_IRQ = 284, + IOAPIC = 285, + PCIINT = 286, + GENERIC = 287, + SPI = 288, + USB = 289, + MMIO = 290 }; #endif
@@ -184,7 +185,7 @@
int yyparse (void);
-#endif /* !YY_YY_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED */ +#endif /* !YY_YY_HOME_RMINNICH_PROJECTS_LINUXBOOT_COREBOOTNERF_GITHUBCOREBOOT_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED */
/* Copy the second part of user declarations. */
@@ -430,10 +431,10 @@ /* YYFINAL -- State number of the termination state. */ #define YYFINAL 3 /* YYLAST -- Last index in YYTABLE. */ -#define YYLAST 43 +#define YYLAST 40
/* YYNTOKENS -- Number of terminals. */ -#define YYNTOKENS 35 +#define YYNTOKENS 36 /* YYNNTS -- Number of nonterminals. */ #define YYNNTS 15 /* YYNRULES -- Number of rules. */ @@ -444,7 +445,7 @@ /* YYTRANSLATE[YYX] -- Symbol number corresponding to YYX as returned by yylex, with out-of-bounds checking. */ #define YYUNDEFTOK 2 -#define YYMAXUTOK 289 +#define YYMAXUTOK 290
#define YYTRANSLATE(YYX) \ ((unsigned int) (YYX) <= YYMAXUTOK ? yytranslate[YYX] : YYUNDEFTOK) @@ -481,7 +482,8 @@ 2, 2, 2, 2, 2, 2, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, - 25, 26, 27, 28, 29, 30, 31, 32, 33, 34 + 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, + 35 };
#if YYDEBUG @@ -500,12 +502,12 @@ static const char *const yytname[] = { "$end", "error", "$undefined", "CHIP", "DEVICE", "REGISTER", "BOOL", - "HIDDEN", "BUS", "RESOURCE", "END", "EQUALS", "HEX", "STRING", "PCI", - "PNP", "I2C", "APIC", "CPU_CLUSTER", "CPU", "DOMAIN", "IRQ", "DRQ", - "SLOT_DESC", "IO", "NUMBER", "SUBSYSTEMID", "INHERIT", "IOAPIC_IRQ", - "IOAPIC", "PCIINT", "GENERIC", "SPI", "USB", "MMIO", "$accept", - "devtree", "$@1", "chipchildren", "devicechildren", "chip", "@2", - "device", "@3", "status", "resource", "registers", "subsystemid", + "STATUS", "MANDATORY", "BUS", "RESOURCE", "END", "EQUALS", "HEX", + "STRING", "PCI", "PNP", "I2C", "APIC", "CPU_CLUSTER", "CPU", "DOMAIN", + "IRQ", "DRQ", "SLOT_DESC", "IO", "NUMBER", "SUBSYSTEMID", "INHERIT", + "IOAPIC_IRQ", "IOAPIC", "PCIINT", "GENERIC", "SPI", "USB", "MMIO", + "$accept", "devtree", "$@1", "chipchildren", "devicechildren", "chip", + "@2", "device", "@3", "status", "resource", "registers", "subsystemid", "ioapic_irq", "smbios_slot_desc", YY_NULLPTR }; #endif @@ -518,7 +520,7 @@ 0, 256, 257, 258, 259, 260, 261, 262, 263, 264, 265, 266, 267, 268, 269, 270, 271, 272, 273, 274, 275, 276, 277, 278, 279, 280, 281, 282, 283, 284, - 285, 286, 287, 288, 289 + 285, 286, 287, 288, 289, 290 }; # endif
@@ -536,11 +538,11 @@ STATE-NUM. */ static const yytype_int8 yypact[] = { - -12, 11, 9, -12, 1, -12, -12, -12, 0, 5, - 3, -12, -12, -12, -12, -10, 6, 2, 8, -12, - -12, -12, -12, -12, -3, -1, -12, 13, 4, 7, - -12, -12, -12, -12, -12, -12, 16, 15, 10, -11, - 12, 17, -5, 14, -12, 18, -12, -12, -12 + -12, 6, 9, -12, -1, -12, -12, -12, 0, 5, + 1, -12, -12, -12, -12, -10, 7, 3, 8, -12, + -12, -12, -12, -12, -3, -9, -12, 11, 2, 4, + -12, -12, -12, -12, -12, -12, 15, 17, 10, -11, + 12, 18, -5, 13, -12, 19, -12, -12, -12 };
/* YYDEFACT[STATE-NUM] -- Default reduction number in state STATE-NUM. @@ -558,7 +560,7 @@ /* YYPGOTO[NTERM-NUM]. */ static const yytype_int8 yypgoto[] = { - -12, -12, -12, -12, -12, -6, -12, 19, -12, -12, + -12, -12, -12, -12, -12, -6, -12, 16, -12, -12, -12, -12, -12, -12, -12 };
@@ -574,39 +576,39 @@ number is the opposite. If YYTABLE_NINF, syntax error. */ static const yytype_uint8 yytable[] = { - 4, 9, 12, 4, 9, 10, 25, 26, 19, 20, - 11, 3, 4, 15, 6, 17, 16, 18, 30, 43, - 27, 22, 46, 28, 36, 29, 37, 40, 41, 38, - 45, 48, 39, 0, 0, 42, 0, 44, 0, 47, - 0, 0, 0, 31 + 4, 9, 12, 4, 9, 10, 3, 25, 26, 19, + 20, 11, 4, 6, 15, 16, 17, 36, 30, 18, + 43, 27, 22, 46, 28, 37, 29, 40, 38, 0, + 39, 41, 45, 48, 0, 0, 42, 0, 44, 47, + 31 };
static const yytype_int8 yycheck[] = { - 3, 4, 8, 3, 4, 5, 9, 10, 6, 7, - 10, 0, 3, 8, 13, 25, 13, 11, 24, 30, - 23, 13, 27, 26, 25, 28, 13, 11, 13, 25, - 13, 13, 25, -1, -1, 25, -1, 25, -1, 25, - -1, -1, -1, 24 + 3, 4, 8, 3, 4, 5, 0, 10, 11, 6, + 7, 11, 3, 14, 9, 14, 26, 26, 24, 12, + 31, 24, 14, 28, 27, 14, 29, 12, 26, -1, + 26, 14, 14, 14, -1, -1, 26, -1, 26, 26, + 24 };
/* YYSTOS[STATE-NUM] -- The (internal number of the) accessing symbol of state STATE-NUM. */ static const yytype_uint8 yystos[] = { - 0, 36, 37, 0, 3, 40, 13, 41, 38, 4, - 5, 10, 40, 42, 46, 8, 13, 25, 11, 6, - 7, 44, 13, 43, 39, 9, 10, 23, 26, 28, - 40, 42, 45, 47, 48, 49, 25, 13, 25, 25, - 11, 13, 25, 30, 25, 13, 27, 25, 13 + 0, 37, 38, 0, 3, 41, 14, 42, 39, 4, + 5, 11, 41, 43, 47, 9, 14, 26, 12, 6, + 7, 45, 14, 44, 40, 10, 11, 24, 27, 29, + 41, 43, 46, 48, 49, 50, 26, 14, 26, 26, + 12, 14, 26, 31, 26, 14, 28, 26, 14 };
/* YYR1[YYN] -- Symbol number of symbol that rule YYN derives. */ static const yytype_uint8 yyr1[] = { - 0, 35, 37, 36, 38, 38, 38, 38, 39, 39, - 39, 39, 39, 39, 39, 41, 40, 43, 42, 44, - 44, 45, 46, 47, 47, 48, 49, 49, 49 + 0, 36, 38, 37, 39, 39, 39, 39, 40, 40, + 40, 40, 40, 40, 40, 42, 41, 44, 43, 45, + 45, 46, 47, 48, 48, 49, 50, 50, 50 };
/* YYR2[YYN] -- Number of symbols on the right hand side of rule YYN. */ @@ -975,7 +977,6 @@ case N: \ yyformat = S; \ break - default: /* Avoid compiler warnings. */ YYCASE_(0, YY_("syntax error")); YYCASE_(1, YY_("syntax error, unexpected %s")); YYCASE_(2, YY_("syntax error, unexpected %s, expecting %s")); diff --git a/util/sconfig/sconfig.tab.h_shipped b/util/sconfig/sconfig.tab.h_shipped index bcbd644..272f651 100644 --- a/util/sconfig/sconfig.tab.h_shipped +++ b/util/sconfig/sconfig.tab.h_shipped @@ -1,8 +1,8 @@ -/* A Bison parser, made by GNU Bison 3.0.5. */ +/* A Bison parser, made by GNU Bison 3.0.4. */
/* Bison interface for Yacc-like parsers in C
- Copyright (C) 1984, 1989-1990, 2000-2015, 2018 Free Software Foundation, Inc. + Copyright (C) 1984, 1989-1990, 2000-2015 Free Software Foundation, Inc.
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 @@ -30,8 +30,8 @@ This special exception was added by the Free Software Foundation in version 2.2 of Bison. */
-#ifndef YY_YY_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED -# define YY_YY_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED +#ifndef YY_YY_HOME_RMINNICH_PROJECTS_LINUXBOOT_COREBOOTNERF_GITHUBCOREBOOT_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED +# define YY_YY_HOME_RMINNICH_PROJECTS_LINUXBOOT_COREBOOTNERF_GITHUBCOREBOOT_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED /* Debug traces. */ #ifndef YYDEBUG # define YYDEBUG 0 @@ -49,34 +49,35 @@ DEVICE = 259, REGISTER = 260, BOOL = 261, - HIDDEN = 262, - BUS = 263, - RESOURCE = 264, - END = 265, - EQUALS = 266, - HEX = 267, - STRING = 268, - PCI = 269, - PNP = 270, - I2C = 271, - APIC = 272, - CPU_CLUSTER = 273, - CPU = 274, - DOMAIN = 275, - IRQ = 276, - DRQ = 277, - SLOT_DESC = 278, - IO = 279, - NUMBER = 280, - SUBSYSTEMID = 281, - INHERIT = 282, - IOAPIC_IRQ = 283, - IOAPIC = 284, - PCIINT = 285, - GENERIC = 286, - SPI = 287, - USB = 288, - MMIO = 289 + STATUS = 262, + MANDATORY = 263, + BUS = 264, + RESOURCE = 265, + END = 266, + EQUALS = 267, + HEX = 268, + STRING = 269, + PCI = 270, + PNP = 271, + I2C = 272, + APIC = 273, + CPU_CLUSTER = 274, + CPU = 275, + DOMAIN = 276, + IRQ = 277, + DRQ = 278, + SLOT_DESC = 279, + IO = 280, + NUMBER = 281, + SUBSYSTEMID = 282, + INHERIT = 283, + IOAPIC_IRQ = 284, + IOAPIC = 285, + PCIINT = 286, + GENERIC = 287, + SPI = 288, + USB = 289, + MMIO = 290 }; #endif
@@ -105,4 +106,4 @@
int yyparse (void);
-#endif /* !YY_YY_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED */ +#endif /* !YY_YY_HOME_RMINNICH_PROJECTS_LINUXBOOT_COREBOOTNERF_GITHUBCOREBOOT_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED */ diff --git a/util/sconfig/sconfig.y b/util/sconfig/sconfig.y index 0d894a9..d55b18b 100755 --- a/util/sconfig/sconfig.y +++ b/util/sconfig/sconfig.y @@ -31,7 +31,7 @@ int number; }
-%token CHIP DEVICE REGISTER BOOL HIDDEN BUS RESOURCE END EQUALS HEX STRING PCI PNP I2C APIC CPU_CLUSTER CPU DOMAIN IRQ DRQ SLOT_DESC IO NUMBER SUBSYSTEMID INHERIT IOAPIC_IRQ IOAPIC PCIINT GENERIC SPI USB MMIO +%token CHIP DEVICE REGISTER BOOL STATUS MANDATORY BUS RESOURCE END EQUALS HEX STRING PCI PNP I2C APIC CPU_CLUSTER CPU DOMAIN IRQ DRQ SLOT_DESC IO NUMBER SUBSYSTEMID INHERIT IOAPIC_IRQ IOAPIC PCIINT GENERIC SPI USB MMIO %% devtree: { cur_parent = root_parent; } chip;
@@ -56,7 +56,7 @@ cur_parent = $<dev>5->parent; };
-status: BOOL | HIDDEN; +status: BOOL | STATUS ;
resource: RESOURCE NUMBER /* == resnum */ EQUALS NUMBER /* == resval */ { add_resource(cur_parent, $<number>1, strtol($<string>2, NULL, 0), strtol($<string>4, NULL, 0)); } ;
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36221/1/src/include/device/device.h File src/include/device/device.h:
https://review.coreboot.org/c/coreboot/+/36221/1/src/include/device/device.h... PS1, Line 132: unsigned int mandatory : 1; /* set if this device is used even in minimum PCI cases */ line over 96 characters
https://review.coreboot.org/c/coreboot/+/36221/1/util/sconfig/lex.yy.c_shipp... File util/sconfig/lex.yy.c_shipped:
https://review.coreboot.org/c/coreboot/+/36221/1/util/sconfig/lex.yy.c_shipp... PS1, Line 162: trailing whitespace
Hello Subrata Banik, Lean Sheng Tan, Jeremy Soller, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36221
to look at the new patch set (#2).
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Add configurable ramstage support for minimal PCI scanning
This CL has changes that allow us to enable a configurable ramstage, and one change that allows us to minimize PCI scanning. Minimal scanning is a frequently requested feature.
To enable it, we add two new variables to src/Kconfig CONFIGURABLE_RAMSTAGE is the overall variable controlling other options for minimizing the ramstage.
MINIMAL_PCI_SCANNING is how we indicate we wish to enable minimal PCI scanning.
Some devices must be scanned in all cases, such as 0:0.0.
To indicate which devices we must scan, we add a new mandatory keyword to sconfig
It is used in place of on, off, or hidden, and indicates a device is enabled and mandatory. Mandatory devices are always scanned. When MINIMAL_PCI_SCANNING is enabled, ONLY mandatory devices are scanned.
We further add support in src/device/pci_device.c to manage both MINIMAL_PCI_SCANNING and mandatory devices.
Finally, to show how this works in practice, we add mandatory keywords to 3 devices on the qemu-q35.
This is tested and working on the qemu-q35 target with a linux kernel payload and u-root initramfs.
Change-Id: I2073d9f8e9297c2b02530821ebb634ea2a5c758e Signed-off-by: Ronald G. Minnich rminnich@gmail.com --- M src/Kconfig M src/device/pci_device.c M src/include/device/device.h M src/mainboard/emulation/qemu-q35/devicetree.cb M util/sconfig/lex.yy.c_shipped M util/sconfig/main.c M util/sconfig/sconfig.h M util/sconfig/sconfig.l M util/sconfig/sconfig.tab.c_shipped M util/sconfig/sconfig.tab.h_shipped M util/sconfig/sconfig.y 11 files changed, 394 insertions(+), 354 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/36221/2
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36221/2/src/include/device/device.h File src/include/device/device.h:
https://review.coreboot.org/c/coreboot/+/36221/2/src/include/device/device.h... PS2, Line 132: unsigned int mandatory : 1; /* set if this device is used even in minimum PCI cases */ line over 96 characters
https://review.coreboot.org/c/coreboot/+/36221/2/util/sconfig/lex.yy.c_shipp... File util/sconfig/lex.yy.c_shipped:
https://review.coreboot.org/c/coreboot/+/36221/2/util/sconfig/lex.yy.c_shipp... PS2, Line 162: trailing whitespace
Hello Subrata Banik, Lean Sheng Tan, Jeremy Soller, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36221
to look at the new patch set (#3).
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Add configurable ramstage support for minimal PCI scanning
This CL has changes that allow us to enable a configurable ramstage, and one change that allows us to minimize PCI scanning. Minimal scanning is a frequently requested feature.
To enable it, we add two new variables to src/Kconfig CONFIGURABLE_RAMSTAGE is the overall variable controlling other options for minimizing the ramstage.
MINIMAL_PCI_SCANNING is how we indicate we wish to enable minimal PCI scanning.
Some devices must be scanned in all cases, such as 0:0.0.
To indicate which devices we must scan, we add a new mandatory keyword to sconfig
It is used in place of on, off, or hidden, and indicates a device is enabled and mandatory. Mandatory devices are always scanned. When MINIMAL_PCI_SCANNING is enabled, ONLY mandatory devices are scanned.
We further add support in src/device/pci_device.c to manage both MINIMAL_PCI_SCANNING and mandatory devices.
Finally, to show how this works in practice, we add mandatory keywords to 3 devices on the qemu-q35.
This is tested and working on the qemu-q35 target with a linux kernel payload and u-root initramfs.
Change-Id: I2073d9f8e9297c2b02530821ebb634ea2a5c758e Signed-off-by: Ronald G. Minnich rminnich@gmail.com --- M src/Kconfig M src/device/pci_device.c M src/include/device/device.h M src/mainboard/emulation/qemu-q35/devicetree.cb M util/sconfig/lex.yy.c_shipped M util/sconfig/main.c M util/sconfig/sconfig.h M util/sconfig/sconfig.l M util/sconfig/sconfig.tab.c_shipped M util/sconfig/sconfig.tab.h_shipped M util/sconfig/sconfig.y 11 files changed, 395 insertions(+), 355 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/36221/3
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36221/3/util/sconfig/lex.yy.c_shipp... File util/sconfig/lex.yy.c_shipped:
https://review.coreboot.org/c/coreboot/+/36221/3/util/sconfig/lex.yy.c_shipp... PS3, Line 162: trailing whitespace
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 3: Code-Review+1
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36221/3/util/sconfig/lex.yy.c_shipp... File util/sconfig/lex.yy.c_shipped:
https://review.coreboot.org/c/coreboot/+/36221/3/util/sconfig/lex.yy.c_shipp... PS3, Line 162: is this valid or existing junk whitespace ?
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36221/3/src/include/device/device.h File src/include/device/device.h:
https://review.coreboot.org/c/coreboot/+/36221/3/src/include/device/device.h... PS3, Line 132: unsigned int mandatory : 1; /* set if this device is used in minimum PCI cases */ What does 'used' mean w.r.t. the semantics of the pci subsystem in coreboot?
Hello Aaron Durbin, Subrata Banik, Lean Sheng Tan, Jeremy Soller, Philipp Deppenwiese, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36221
to look at the new patch set (#4).
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Add configurable ramstage support for minimal PCI scanning
This CL has changes that allow us to enable a configurable ramstage, and one change that allows us to minimize PCI scanning. Minimal scanning is a frequently requested feature.
To enable it, we add two new variables to src/Kconfig CONFIGURABLE_RAMSTAGE is the overall variable controlling other options for minimizing the ramstage.
MINIMAL_PCI_SCANNING is how we indicate we wish to enable minimal PCI scanning.
Some devices must be scanned in all cases, such as 0:0.0.
To indicate which devices we must scan, we add a new mandatory keyword to sconfig
It is used in place of on, off, or hidden, and indicates a device is enabled and mandatory. Mandatory devices are always scanned. When MINIMAL_PCI_SCANNING is enabled, ONLY mandatory devices are scanned.
We further add support in src/device/pci_device.c to manage both MINIMAL_PCI_SCANNING and mandatory devices.
Finally, to show how this works in practice, we add mandatory keywords to 3 devices on the qemu-q35.
This is tested and working on the qemu-q35 target with a linux kernel payload and u-root initramfs.
Change-Id: I2073d9f8e9297c2b02530821ebb634ea2a5c758e Signed-off-by: Ronald G. Minnich rminnich@gmail.com --- M src/Kconfig M src/device/pci_device.c M src/include/device/device.h M src/mainboard/emulation/qemu-q35/devicetree.cb M util/sconfig/lex.yy.c_shipped M util/sconfig/main.c M util/sconfig/sconfig.h M util/sconfig/sconfig.l M util/sconfig/sconfig.tab.c_shipped M util/sconfig/sconfig.tab.h_shipped M util/sconfig/sconfig.y 11 files changed, 397 insertions(+), 355 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/36221/4
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36221/4/util/sconfig/lex.yy.c_shipp... File util/sconfig/lex.yy.c_shipped:
https://review.coreboot.org/c/coreboot/+/36221/4/util/sconfig/lex.yy.c_shipp... PS4, Line 162: trailing whitespace
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36221/3/src/include/device/device.h File src/include/device/device.h:
https://review.coreboot.org/c/coreboot/+/36221/3/src/include/device/device.h... PS3, Line 132: unsigned int mandatory : 1; /* set if this device is used in minimum PCI cases */
What does 'used' mean w.r.t. […]
I changed the comment, see what you think.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36221/3/src/include/device/device.h File src/include/device/device.h:
https://review.coreboot.org/c/coreboot/+/36221/3/src/include/device/device.h... PS3, Line 132: unsigned int mandatory : 1; /* set if this device is used in minimum PCI cases */
I changed the comment, see what you think.
looks better, thanks. Am I right that we do not have devicetree.cb documentation?
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36221/3/src/include/device/device.h File src/include/device/device.h:
https://review.coreboot.org/c/coreboot/+/36221/3/src/include/device/device.h... PS3, Line 132: unsigned int mandatory : 1; /* set if this device is used in minimum PCI cases */
looks better, thanks. Am I right that we do not have devicetree. […]
yeah, I wrote one long ago and it's quite obsolete. Needs to be done.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 4: Code-Review+2
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 4:
Is this work related or replacement to ENV_PAYLOAD_LOADER introduction?
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36221/4/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/36221/4/src/device/pci_device.c@119... PS4, Line 1195: dev = dev_find_slot(0, devfn); Sigh.. dev_find_slot() should be banned. Would following work for you:
dev = pcidev_path_behind(bus, devfn);
Out of scope, but I don't like how the for-loop hardcodes dev.fn. Aaron might have some related work on that.
https://review.coreboot.org/c/coreboot/+/36221/4/src/device/pci_device.c@119... PS4, Line 1197: printk(BIOS_WARNING, "%#x is NOT static, skipping it\n", devfn); We prefer B:D.f notation for PCI devices on console output.
Hello Aaron Durbin, Subrata Banik, Lean Sheng Tan, Jeremy Soller, Shelley Chen, Philipp Deppenwiese, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36221
to look at the new patch set (#5).
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Add configurable ramstage support for minimal PCI scanning
This CL has changes that allow us to enable a configurable ramstage, and one change that allows us to minimize PCI scanning. Minimal scanning is a frequently requested feature.
To enable it, we add two new variables to src/Kconfig CONFIGURABLE_RAMSTAGE is the overall variable controlling other options for minimizing the ramstage.
MINIMAL_PCI_SCANNING is how we indicate we wish to enable minimal PCI scanning.
Some devices must be scanned in all cases, such as 0:0.0.
To indicate which devices we must scan, we add a new mandatory keyword to sconfig
It is used in place of on, off, or hidden, and indicates a device is enabled and mandatory. Mandatory devices are always scanned. When MINIMAL_PCI_SCANNING is enabled, ONLY mandatory devices are scanned.
We further add support in src/device/pci_device.c to manage both MINIMAL_PCI_SCANNING and mandatory devices.
Finally, to show how this works in practice, we add mandatory keywords to 3 devices on the qemu-q35.
This is tested and working on the qemu-q35 target.
Change-Id: I2073d9f8e9297c2b02530821ebb634ea2a5c758e Signed-off-by: Ronald G. Minnich rminnich@gmail.com --- M 3rdparty/fsp M src/Kconfig M src/device/pci_device.c M src/include/device/device.h M src/mainboard/emulation/qemu-q35/devicetree.cb M util/sconfig/lex.yy.c_shipped M util/sconfig/main.c M util/sconfig/sconfig.h M util/sconfig/sconfig.l M util/sconfig/sconfig.tab.c_shipped M util/sconfig/sconfig.tab.h_shipped M util/sconfig/sconfig.y 12 files changed, 390 insertions(+), 354 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/36221/5
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36221/5/src/include/device/device.h File src/include/device/device.h:
https://review.coreboot.org/c/coreboot/+/36221/5/src/include/device/device.h... PS5, Line 132: unsigned int mandatory : 1; /* set if this device is used even in minimum PCI cases */ line over 96 characters
https://review.coreboot.org/c/coreboot/+/36221/5/util/sconfig/lex.yy.c_shipp... File util/sconfig/lex.yy.c_shipped:
https://review.coreboot.org/c/coreboot/+/36221/5/util/sconfig/lex.yy.c_shipp... PS5, Line 162: trailing whitespace
Hello Aaron Durbin, Subrata Banik, Lean Sheng Tan, Jeremy Soller, Shelley Chen, Philipp Deppenwiese, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36221
to look at the new patch set (#6).
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Add configurable ramstage support for minimal PCI scanning
This CL has changes that allow us to enable a configurable ramstage, and one change that allows us to minimize PCI scanning. Minimal scanning is a frequently requested feature.
To enable it, we add two new variables to src/Kconfig CONFIGURABLE_RAMSTAGE is the overall variable controlling other options for minimizing the ramstage.
MINIMAL_PCI_SCANNING is how we indicate we wish to enable minimal PCI scanning.
Some devices must be scanned in all cases, such as 0:0.0.
To indicate which devices we must scan, we add a new mandatory keyword to sconfig
It is used in place of on, off, or hidden, and indicates a device is enabled and mandatory. Mandatory devices are always scanned. When MINIMAL_PCI_SCANNING is enabled, ONLY mandatory devices are scanned.
We further add support in src/device/pci_device.c to manage both MINIMAL_PCI_SCANNING and mandatory devices.
Finally, to show how this works in practice, we add mandatory keywords to 3 devices on the qemu-q35.
This is tested and working on the qemu-q35 target.
Change-Id: I2073d9f8e9297c2b02530821ebb634ea2a5c758e Signed-off-by: Ronald G. Minnich rminnich@gmail.com --- M src/Kconfig M src/device/pci_device.c M src/include/device/device.h M src/mainboard/emulation/qemu-q35/devicetree.cb M util/sconfig/lex.yy.c_shipped M util/sconfig/main.c M util/sconfig/sconfig.h M util/sconfig/sconfig.l M util/sconfig/sconfig.tab.c_shipped M util/sconfig/sconfig.tab.h_shipped M util/sconfig/sconfig.y 11 files changed, 389 insertions(+), 353 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/36221/6
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36221/6/src/include/device/device.h File src/include/device/device.h:
https://review.coreboot.org/c/coreboot/+/36221/6/src/include/device/device.h... PS6, Line 132: unsigned int mandatory : 1; /* set if this device is used even in minimum PCI cases */ line over 96 characters
https://review.coreboot.org/c/coreboot/+/36221/6/util/sconfig/lex.yy.c_shipp... File util/sconfig/lex.yy.c_shipped:
https://review.coreboot.org/c/coreboot/+/36221/6/util/sconfig/lex.yy.c_shipp... PS6, Line 162: trailing whitespace
Hello Aaron Durbin, Subrata Banik, Lean Sheng Tan, Jeremy Soller, Shelley Chen, Philipp Deppenwiese, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36221
to look at the new patch set (#7).
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Add configurable ramstage support for minimal PCI scanning
This CL has changes that allow us to enable a configurable ramstage, and one change that allows us to minimize PCI scanning. Minimal scanning is a frequently requested feature.
To enable it, we add two new variables to src/Kconfig CONFIGURABLE_RAMSTAGE is the overall variable controlling other options for minimizing the ramstage.
MINIMAL_PCI_SCANNING is how we indicate we wish to enable minimal PCI scanning.
Some devices must be scanned in all cases, such as 0:0.0.
To indicate which devices we must scan, we add a new mandatory keyword to sconfig
It is used in place of on, off, or hidden, and indicates a device is enabled and mandatory. Mandatory devices are always scanned. When MINIMAL_PCI_SCANNING is enabled, ONLY mandatory devices are scanned.
We further add support in src/device/pci_device.c to manage both MINIMAL_PCI_SCANNING and mandatory devices.
Finally, to show how this works in practice, we add mandatory keywords to 3 devices on the qemu-q35.
This is tested and working on the qemu-q35 target.
Change-Id: I2073d9f8e9297c2b02530821ebb634ea2a5c758e Signed-off-by: Ronald G. Minnich rminnich@gmail.com --- M src/Kconfig M src/device/pci_device.c M src/include/device/device.h M src/mainboard/emulation/qemu-q35/devicetree.cb M util/sconfig/lex.yy.c_shipped M util/sconfig/main.c M util/sconfig/sconfig.h M util/sconfig/sconfig.l M util/sconfig/sconfig.tab.c_shipped M util/sconfig/sconfig.tab.h_shipped M util/sconfig/sconfig.y 11 files changed, 385 insertions(+), 353 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/36221/7
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36221/7/src/include/device/device.h File src/include/device/device.h:
https://review.coreboot.org/c/coreboot/+/36221/7/src/include/device/device.h... PS7, Line 132: unsigned int mandatory : 1; /* set if this device is used even in minimum PCI cases */ line over 96 characters
https://review.coreboot.org/c/coreboot/+/36221/7/util/sconfig/lex.yy.c_shipp... File util/sconfig/lex.yy.c_shipped:
https://review.coreboot.org/c/coreboot/+/36221/7/util/sconfig/lex.yy.c_shipp... PS7, Line 162: trailing whitespace
Hello Aaron Durbin, Subrata Banik, Lean Sheng Tan, Jeremy Soller, Shelley Chen, Philipp Deppenwiese, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36221
to look at the new patch set (#8).
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Add configurable ramstage support for minimal PCI scanning
This CL has changes that allow us to enable a configurable ramstage, and one change that allows us to minimize PCI scanning. Minimal scanning is a frequently requested feature.
To enable it, we add two new variables to src/Kconfig CONFIGURABLE_RAMSTAGE is the overall variable controlling other options for minimizing the ramstage.
MINIMAL_PCI_SCANNING is how we indicate we wish to enable minimal PCI scanning.
Some devices must be scanned in all cases, such as 0:0.0.
To indicate which devices we must scan, we add a new mandatory keyword to sconfig
It is used in place of on, off, or hidden, and indicates a device is enabled and mandatory. Mandatory devices are always scanned. When MINIMAL_PCI_SCANNING is enabled, ONLY mandatory devices are scanned.
We further add support in src/device/pci_device.c to manage both MINIMAL_PCI_SCANNING and mandatory devices.
Finally, to show how this works in practice, we add mandatory keywords to 3 devices on the qemu-q35.
This is tested and working on the qemu-q35 target.
Change-Id: I2073d9f8e9297c2b02530821ebb634ea2a5c758e Signed-off-by: Ronald G. Minnich rminnich@gmail.com --- M src/Kconfig M src/device/pci_device.c M src/include/device/device.h M src/mainboard/emulation/qemu-q35/devicetree.cb M util/sconfig/lex.yy.c_shipped M util/sconfig/main.c M util/sconfig/sconfig.h M util/sconfig/sconfig.l M util/sconfig/sconfig.tab.c_shipped M util/sconfig/sconfig.tab.h_shipped M util/sconfig/sconfig.y 11 files changed, 387 insertions(+), 353 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/36221/8
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36221/8/util/sconfig/lex.yy.c_shipp... File util/sconfig/lex.yy.c_shipped:
https://review.coreboot.org/c/coreboot/+/36221/8/util/sconfig/lex.yy.c_shipp... PS8, Line 162: trailing whitespace
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 8:
(7 comments)
Nice work. Can the QEMU Q35 change be factored out?
https://review.coreboot.org/c/coreboot/+/36221/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36221/8//COMMIT_MSG@25 PS8, Line 25: keyword to sconfig Strange line break. Could you reformat the whole message for 75 characters, please?
https://review.coreboot.org/c/coreboot/+/36221/8/src/Kconfig File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/36221/8/src/Kconfig@339 PS8, Line 339: bool "Enable a configurable ramstage." Please remove the dot at the end.
https://review.coreboot.org/c/coreboot/+/36221/8/src/Kconfig@344 PS8, Line 344: The minimal PCI scanning will only check those parts that are enabled : in the devicetree.cb. By convention none of those devices should be bridges. Move this to the description of the option below?
https://review.coreboot.org/c/coreboot/+/36221/8/src/include/device/device.h File src/include/device/device.h:
https://review.coreboot.org/c/coreboot/+/36221/8/src/include/device/device.h... PS8, Line 133: * Remove for concise multi-line comments.
https://review.coreboot.org/c/coreboot/+/36221/8/util/sconfig/lex.yy.c_shipp... File util/sconfig/lex.yy.c_shipped:
https://review.coreboot.org/c/coreboot/+/36221/8/util/sconfig/lex.yy.c_shipp... PS8, Line 9: #define YY_FLEX_SUBMINOR_VERSION 4 Why increment it by 3?
https://review.coreboot.org/c/coreboot/+/36221/8/util/sconfig/sconfig.tab.c_... File util/sconfig/sconfig.tab.c_shipped:
https://review.coreboot.org/c/coreboot/+/36221/8/util/sconfig/sconfig.tab.c_... PS8, Line 112: #ifndef YY_YY_HOME_RMINNICH_PROJECTS_LINUXBOOT_COREBOOTNERF_GITHUBCOREBOOT_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED Is this wanted?
https://review.coreboot.org/c/coreboot/+/36221/8/util/sconfig/sconfig.tab.h_... File util/sconfig/sconfig.tab.h_shipped:
https://review.coreboot.org/c/coreboot/+/36221/8/util/sconfig/sconfig.tab.h_... PS8, Line 33: #ifndef YY_YY_HOME_RMINNICH_PROJECTS_LINUXBOOT_COREBOOTNERF_GITHUBCOREBOOT_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED Ditto.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 8: Code-Review-1
(2 comments)
Please answer and address previous questions about ENV_PAYLOAD_LOADER. Intel and PCI specs and devicetree are already convoluted enough with stale implementations and ideas, further complicated by poor HyperTransport integration.
Also, you state (PCI) minimal scanning is a frequently requested feature. Please forward such communications early-on to the mailing list so others can see the design requirements.
https://review.coreboot.org/c/coreboot/+/36221/8/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/36221/8/src/device/pci_device.c@119... PS8, Line 1191: dev = pcidev_path_behind(bus, devfn); I did not see this on first review, but this probably evaluates to same 'dev' as pci_scan_get_dev() call below.
This MINIMAL_PCI_SCANNING should just directly iterate the list of bus->children instead of attempting to search for each dev.fn separately.
https://review.coreboot.org/c/coreboot/+/36221/8/src/device/pci_device.c@122... PS8, Line 1229: *prev = dev->sibling; It is pci_probe_dev() that filled vendor field. So you are effectively blocking PCI configuration access to all but PCI functions listed as mandatory? Intentional or not?
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 8: Code-Review-1
-1 just because I saw a premature +2 along the lines.
About the implementation: If we need special semantics (if not, see alternatives below), I would prefer a clean, second implementation of pci_scan_bus() instead of making the current one even harder to follow.
About the devicetree notation: * `mandatory` seems orthogonal to the last addition `hidden`. How would I specify a device that is mandatory and should be used in (hidden) ACPI mode? * What is the point of specifying a device in the devicetree if it isn't mandatory? iow, do we need another flag at all? wouldn't the existing .on_mainboard suffice? If the point is to avoid scanning addresses that might return nothing, it should.
About alternatives: For most platforms, we know that bus 0 doesn't (shouldn't?) contain surprises. We had a little discussion about handling chipset embedded devices better lately. If we would specify bus 0 devices exactly, per platform, the mainboard devicetree would only have to mention details (e.g. disable that, add downstream devices there...). And for PCIe root ports, we already know that we only have to scan for downstream device 0 (and not 1..1f). I'm not very familiar with PCIe capabilities, but maybe there is even a hint in downstream bridges if we have to scan all 32 device numbers.
So how about, instead of adding a feature on top, for anonymous folks that frequently request minimal scanning, we look for a generic way first to implement it for everyone?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 8:
Patch Set 4:
Is this work related to ENV_PAYLOAD_LOADER introduction?
YES
Is this work replacement to ENV_PAYLOAD_LOADER introduction?
NO
We had introduced ENV_PAYLOAD_LOADER with an idea that ramstage might not be mandatory stage for all platform hence we could also just boot to payload from postcar (if exist after romstage) directly.
What it saves ?
1. Entire boot state machine programming with more than what might required to boot a platform 2. Minimum programming hence save boot time and spi size 3. We have created 3 major bucket out of that exercise A Minimum programming to have system pick payload and boot to OS B Required ASL/ACPI table to make platform device functional C Entire PCI enumeration because all payload are not self sufficient like linuxboot. Incase depthcharge, we expected to provide PCI devices with required resources for depthcharge to use
Now, while coming to solution space, we have seen than 3-C might be equally frustrating/opportunistic for !ENV_PAYLOAD_LOADER case as well, where why BIOS/coreboot to do entire PCI enumeration rather we should handle "minimum/mandatory" PCI device enumeration required to boot the platform via payload.
Aaron has an idea to pause ENV_PAYLOAD_LOADER activity and spent some time to create configurable ramstage where we could achieve all 3-A/B/C goal.
This CL is intended to achieve 3-C where we can decide to mark "mandatory" PCI devices for platform so that we can save boot time and eventually the idea would be create new API rather using PCI_SCAN_BUS to do minimum stuffs as well (TODO)
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 8:
Patch Set 8:
Patch Set 4:
Is this work related to ENV_PAYLOAD_LOADER introduction?
YES
Is this work replacement to ENV_PAYLOAD_LOADER introduction?
NO
Aaron has an idea to pause ENV_PAYLOAD_LOADER activity and spent some time to create configurable ramstage where we could achieve all 3-A/B/C goal.
Addition of ENV_PAYLOAD_LOADER was IMHO submitted prematurely anyways and I'd rather see it reverted. One of the bigger assumptions it made was that one could easily move 'some' code from ramstage-y to postcar-y which is not true. Latter uses __SIMPLE_DEVICE__ and lacks PCI drivers entirely.
Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 8: Code-Review+1
I can just speak for myself/9elements. I would really like to see that we have a configurable ramstage approach for LinuxBoot payloads if we remove the previous one. Most people don't seem to know but the Linux kernel supports PCI device enumeration and some other stuff. That's why multiple companies seeking such a solution.
BTW this CL has nothing to do with the ENV_PAYLOAD_LOADER stuff...
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 8:
BTW this CL has nothing to do with the ENV_PAYLOAD_LOADER stuff...
Different approaches to same request; Either do less in ramstage, or move the little stuff you need from ramstage to postcar.
ron minnich has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Abandoned
It is perhaps best I find a different project for this kind of more advanced work.
ron minnich has restored this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Restored
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 8:
(10 comments)
no sooner did I abandon it than two different companies asked me not to.
https://review.coreboot.org/c/coreboot/+/36221/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36221/8//COMMIT_MSG@25 PS8, Line 25: keyword to sconfig
Strange line break. […]
it's intentional
https://review.coreboot.org/c/coreboot/+/36221/8/src/Kconfig File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/36221/8/src/Kconfig@339 PS8, Line 339: bool "Enable a configurable ramstage."
Please remove the dot at the end.
Done
https://review.coreboot.org/c/coreboot/+/36221/8/src/Kconfig@344 PS8, Line 344: The minimal PCI scanning will only check those parts that are enabled : in the devicetree.cb. By convention none of those devices should be bridges.
Move this to the description of the option below?
I just blew it away, I did not see a need for this text.
https://review.coreboot.org/c/coreboot/+/36221/4/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/36221/4/src/device/pci_device.c@119... PS4, Line 1195: dev = dev_find_slot(0, devfn);
Sigh.. dev_find_slot() should be banned. Would following work for you: […]
Done
https://review.coreboot.org/c/coreboot/+/36221/4/src/device/pci_device.c@119... PS4, Line 1197: printk(BIOS_WARNING, "%#x is NOT static, skipping it\n", devfn);
We prefer B:D.f notation for PCI devices on console output.
I blew away the print, it's a time waster.
https://review.coreboot.org/c/coreboot/+/36221/8/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/36221/8/src/device/pci_device.c@119... PS8, Line 1191: dev = pcidev_path_behind(bus, devfn);
I did not see this on first review, but this probably evaluates to same 'dev' as pci_scan_get_dev() […]
yes it should. And it will. What we're showing here is the minimal set of changes to make this work at all, b/c every pass we've taken at this has been bikeshedded into oblivion.
https://review.coreboot.org/c/coreboot/+/36221/8/src/device/pci_device.c@122... PS8, Line 1229: *prev = dev->sibling;
It is pci_probe_dev() that filled vendor field. […]
yes we are.
https://review.coreboot.org/c/coreboot/+/36221/8/src/include/device/device.h File src/include/device/device.h:
https://review.coreboot.org/c/coreboot/+/36221/8/src/include/device/device.h... PS8, Line 133: *
Remove for concise multi-line comments.
Done
https://review.coreboot.org/c/coreboot/+/36221/8/util/sconfig/sconfig.tab.c_... File util/sconfig/sconfig.tab.c_shipped:
https://review.coreboot.org/c/coreboot/+/36221/8/util/sconfig/sconfig.tab.c_... PS8, Line 112: #ifndef YY_YY_HOME_RMINNICH_PROJECTS_LINUXBOOT_COREBOOTNERF_GITHUBCOREBOOT_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED
Is this wanted?
this is a generated file. Not much to be done I think.
https://review.coreboot.org/c/coreboot/+/36221/8/util/sconfig/sconfig.tab.h_... File util/sconfig/sconfig.tab.h_shipped:
https://review.coreboot.org/c/coreboot/+/36221/8/util/sconfig/sconfig.tab.h_... PS8, Line 33: #ifndef YY_YY_HOME_RMINNICH_PROJECTS_LINUXBOOT_COREBOOTNERF_GITHUBCOREBOOT_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED
Ditto.
ditto :-)
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 8:
so far 4 companies have indicated an interest in this CL System76 9elements intel google
no anonymity there.
It's a trivial CL. We did it this way because of the objections to just removing the ramstage in the last 12 months of discussion; we were asked to look at a configurable ramstage. This is the first step on the road. It shows the variable to enable it and the variable to use it.
This has nothing to do with ENV_PAYLOAD_LOADER in my view. It is concerned only with turning off coreboot enumeration of PCI space, in part because coreboot is still 32-bit firmware on x86 and that's causing trouble for some companies; in part because it's not needed anyway for things like Linux: Linux has a much better idea of how it needs configuration space laid out than coreboot does.
We're hoping on subsequent CLs to make further changes in how this is achieved, but the goal was to present a very minimal change that people could evaluate for their needs to get some experience with it. The need is real, however, and the need has yet to be accommodated. This simple change accommodates it.
Nico, I understand your point, but that looks to me like a far larger effort. I'd like to get some evaluation if this is taking us at least the right direction (from a user point of view) before we start making too many far-reaching changes.
as for hidden vs. mandatory: I can find no use of the hidden keyword in any Kconfig; am I missing something? If it is in fact never used, should it be in the parser at all?
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36221/8/src/Kconfig File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/36221/8/src/Kconfig@339 PS8, Line 339: bool "Enable a configurable ramstage."
Done
Can you push that updated change please? :-)
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 8:
Nico, I understand your point, but that looks to me like a far larger effort. I'd like to get some evaluation if this is taking us at least the right direction (from a user point of view) before we start making too many far-reaching changes.
I don't think what I suggested is far-reaching. It's rather overdue maintenance, that will be pushed even further into the future if we keep accepting "evaluation" changes.
as for hidden vs. mandatory: I can find no use of the hidden keyword in any Kconfig; am I missing something? If it is in fact never used, should it be in the parser at all?
You should ask its author, it wasn't merged too long ago, IIRC. Still my question remains, what is wrong with `.on_mainboard`?
Hello Kyösti Mälkki, Aaron Durbin, Subrata Banik, Jeremy Soller, Shelley Chen, build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Lean Sheng Tan, Philipp Deppenwiese, Nico Huber, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36221
to look at the new patch set (#9).
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Add configurable ramstage support for minimal PCI scanning
This CL has changes that allow us to enable a configurable ramstage, and one change that allows us to minimize PCI scanning. Minimal scanning is a frequently requested feature.
To enable it, we add two new variables to src/Kconfig CONFIGURABLE_RAMSTAGE is the overall variable controlling other options for minimizing the ramstage.
MINIMAL_PCI_SCANNING is how we indicate we wish to enable minimal PCI scanning.
Some devices must be scanned in all cases, such as 0:0.0.
To indicate which devices we must scan, we add a new mandatory keyword to sconfig
It is used in place of on, off, or hidden, and indicates a device is enabled and mandatory. Mandatory devices are always scanned. When MINIMAL_PCI_SCANNING is enabled, ONLY mandatory devices are scanned.
We further add support in src/device/pci_device.c to manage both MINIMAL_PCI_SCANNING and mandatory devices.
Finally, to show how this works in practice, we add mandatory keywords to 3 devices on the qemu-q35.
This is tested and working on the qemu-q35 target.
Change-Id: I2073d9f8e9297c2b02530821ebb634ea2a5c758e Signed-off-by: Ronald G. Minnich rminnich@gmail.com --- M src/Kconfig M src/device/pci_device.c M src/include/device/device.h M src/mainboard/emulation/qemu-q35/devicetree.cb M util/sconfig/lex.yy.c_shipped M util/sconfig/main.c M util/sconfig/sconfig.h M util/sconfig/sconfig.l M util/sconfig/sconfig.tab.c_shipped M util/sconfig/sconfig.tab.h_shipped M util/sconfig/sconfig.y 11 files changed, 385 insertions(+), 353 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/36221/9
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36221/8/src/Kconfig File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/36221/8/src/Kconfig@339 PS8, Line 339: bool "Enable a configurable ramstage."
Can you push that updated change please? :-)
Done
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36221/9/util/sconfig/lex.yy.c_shipp... File util/sconfig/lex.yy.c_shipped:
https://review.coreboot.org/c/coreboot/+/36221/9/util/sconfig/lex.yy.c_shipp... PS9, Line 162: trailing whitespace
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 9:
You should ask its author, it wasn't merged too long ago, IIRC. Still my question remains, what is wrong with `.on_mainboard`?
To me, that's not the same as mandatory. on_mainboard means just that -- it's a non-removable device of some sort. We originally added it for devices soldered on to the board that could not be removed.
Mandatory means "present and must be configured". Just because a device is on_mainboard does not mean we want to scan it in coreboot. I view those as different things.
The idea of mandatory is that it is hardware that must be enabled or we can't load a payload. We learned on the q35 that 0:00.0 was mandatory, and also that 0:1f.0 was mandatory. No other devices need be scanned.
The reason I like this way of changing the pci scan code, for a first pass, is that the footprint and impact of the change is tiny. This slight change to behavior doesn't involve a lot of code, and it makes it look like only those devices marked mandatory were found. I believe this lets us test the idea -- that a Linux can function correctly with minimal PCI setup -- across a wide range of boards to make sure nothing gets broken. It's a very slight change that lets us easily test the idea. The default is that it doesn't work.
So, small code space impact, small binary size impact, defaults to off -- I'm comfortable with that as opposed to the earlier, more far-reaching changes.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 9:
(4 comments)
I agree resource allocator to be very limited (MEM and PREFMEM particularly), yet I have not seen serious attemps of previously mentioned companies fixing it themselves or contracting anyone familiar with the subsystem to work on the topic.
Please list your design requirements rather than saying "this is a requested feature by 4 companies". Is it only about the 32bit MMIO space or maybe also the some dozen milliseconds lost in ramstage probing for PCI devices that could never ever be present?
I am not at all convinced completely bypassing PCI probing and removing nodes from devicetree is the way to proceed, whatever the design requirements are.
https://review.coreboot.org/c/coreboot/+/36221/9/src/Kconfig File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/36221/9/src/Kconfig@340 PS9, Line 340: default y if ARCH_X86 Why the arch dependency?
https://review.coreboot.org/c/coreboot/+/36221/9/src/Kconfig@348 PS9, Line 348: help This option should only be visible for boards with devicetree.cb files modified to have 'mandatory' entries.
Also:
depends on PCI
https://review.coreboot.org/c/coreboot/+/36221/8/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/36221/8/src/device/pci_device.c@119... PS8, Line 1191: dev = pcidev_path_behind(bus, devfn);
yes it should. And it will. […]
Can you give links to reviews of these past "bikeshedded passes" because I don't remember reviewing much PCI related changes on this topic. And both Subrata and You are saying this is not related to that "move payload loader into postcar" change that possibly was bikeshedded for a good reason.
https://review.coreboot.org/c/coreboot/+/36221/8/src/device/pci_device.c@122... PS8, Line 1229: *prev = dev->sibling;
yes we are.
To skip scanning of devices also affects ACPI namespace. You would probably still need some way of communicating certain platform details (power management events come to mind) from the firmware. You possibly end up creating more static .asl files and that might get very ugly with PCI bifurcation/coalesce.
I think ACPI does not reference PCI buses by secondary numbers, but you probably should not create (obsolete) MP tables at all if PCI busses are not enumerated.
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 9:
Kyösti,
your concern seems to be mostly about "why do this at all?" rather than "is this the right approach to do and integrate it?", so that's what I'm responding to here:
Patch Set 9: I agree resource allocator to be very limited (MEM and PREFMEM particularly), yet I have not seen serious attemps of previously mentioned companies fixing it themselves or
I like to ask UEFI folks why they think they can build a better operating system than folks who do nothing but build operating systems. The same is true for us: Linux has a battle-proven resource allocator, and if you end up using that anyway, why not skip ours instead of doing everything twice?
Doesn't mean we should remove our allocator because it's still useful in situations where Linux is prohibitive (that's the difference between coreboot and UEFI: they demand you take the whole enchilada, no questions asked).
contracting anyone familiar with the subsystem to work on the topic.
Last I heard, Rudolf, who designed and wrote the latest instance of our allocator, is happy with his job and tied up with personal projects, so I don't think he's available for contract.
Please list your design requirements rather than saying "this is a requested feature by 4 companies". Is it only about the 32bit MMIO space or maybe also the some dozen milliseconds lost in ramstage probing for PCI devices that could never ever be present?
The requirement is to pursue the original vision of having a minimal piece of code that gets the hardware into a state that Linux can take over. Ron never went away from that goal, and he pursued it on and off over time.
We do all kind of nerdy things in our code base that satisfy some rather peculiar requirements (for example, we added the splashscreen code + jpeg decoder for a one-off demo to sell the idea of coreboot to stakeholders), and I fail to see why we should stop doing so.
Of course, any such hack still needs to fit in the system as a whole (most prominently: stay out of the way of users who don't use it), but that doesn't seem to be what you're arguing.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 9:
Patch Set 9:
Kyösti,
your concern seems to be mostly about "why do this at all?" rather than "is this the right approach to do and integrate it?", so that's what I'm responding to here:
Well I literally wrote this: "I am not at all convinced completely bypassing PCI probing and removing nodes from devicetree is the way to proceed, whatever the design requirements are."
The overhead of probing PCI device ID and keeping the node in devicetree is pretty minimal. I had suggested to only probe for PCI BDF already present in static devicetree instead of doing the full scan of 32 devices(potentially times 8 functioncs), if that couple milliseconds is of importance there.
One can probe the device IDs, keep the device PCI configuration space accessible, and still leave all standard PCI BARs unprogrammed and bypass all device init.
The development here is not orthogonal to 64-bit MMIO resource support; for payloads other than kernel one needs to make decisions what BARs are boot-critical and must be located in MMIO window below 4 GiB. That would serve/solve environments other than linux-as-a-payload too. I wonder if majority of those requesting better PCI resource allocations have just fundamentally stated that large (>256MiB) prefetch frame buffers don't work well with coreboot... IMHO development efforts should go here instead.
The requirement is to pursue the original vision of having a minimal piece of code that gets the hardware into a state that Linux can take over. Ron never went away from that goal, and he pursued it on and off over time.
And I am questioning if that original vision is viable with contemporary hardware. My impression is that there is an increasing amount of platform customisation present in devicetree.cb files, but maybe a lot of that is just passed to blobs instead of having coreboot proper do the actual register writes. I would not be surprised if those blobs temporarily opened up some BARs. I would rather not see a solution of MINIMAL_PCI_SCANNING that only works with FSP blobs and fails for those who still have open-source chipset configuration and need some resources assigned.
I also popped the question about dynamically created PCI methods in ACPI namespace. Is there anything critical or important there? Like PIRQ/MSI-X/GSI/IOAPIC routing information that OS drivers could not resolve without firmware assistance? I imagine it could be tricky for static platform .asl to assign properties (PME,hotplug?) to PCI devices whose generation is delayed to OS. ASL is not really my thing, this is just one more aspect where proof-of-concept runs on overly-simplified qemu-q35 might not fault or highlight issues.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 9:
I share Kyösti's concerns how this could be compatible to our code for real hardware targets. Existing platform code relies heavily on the VID/DID of PCI devices, that are never read with this approach. So what should it be, should we hide the minimal scanning option for all boards but those that are confirmed to be fully compatible? Should we add a big fat warning, that this is experimental and will break 90% of our ramstage code (and hence, nobody should complain or even try to fix it)? Should we remodel all of coreboot so we rely less on PCI and do things more in a blob-style manner?
Without knowing even a coarse plan how to adapt this to exis- ting hardware support, I really don't see how this is a step forward. At least a hardware PoC with extensive Linux tests (not just, it boots) would be nice. Also, if this requires changes to the OS, do we expect upstream Linux, for instance, to follow?
Lean Sheng Tan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 9: Code-Review+1
Patch Set 9:
I share Kyösti's concerns how this could be compatible to our code for real hardware targets. Existing platform code relies heavily on the VID/DID of PCI devices, that are never read with this approach. So what should it be, should we hide the minimal scanning option for all boards but those that are confirmed to be fully compatible? Should we add a big fat warning, that this is experimental and will break 90% of our ramstage code (and hence, nobody should complain or even try to fix it)? Should we remodel all of coreboot so we rely less on PCI and do things more in a blob-style manner?
Without knowing even a coarse plan how to adapt this to exis- ting hardware support, I really don't see how this is a step forward. At least a hardware PoC with extensive Linux tests (not just, it boots) would be nice. Also, if this requires changes to the OS, do we expect upstream Linux, for instance, to follow?
Hi Nico, While I respect your technical expertise, I cannot fathom your sentiments on this case. Do you mean that every changes in open source should be vigorously validated with most platforms, and require a coarse plan for implementation? From my understanding, the success and sustainability of open source relies on its openness to encourage and embrace changes no matter how big or small. You should be even more understanding on this as I saw your countless changes everywhere, still we happily accept your inputs even it breaks our codes some times. With that being said, this patch is providing a forefront idea for us to further improve the flexibility of coreboot. Of course, we can't expect it to be perfect, but anyone can take it and experiment with it and further improving it. I can foresee how it will make our codes to run more dynamically, and it will also eventually push changes to the linux side as well.
I can't make you to change your mind, but i am sure you know what needs to be done if we want coreboot to continue evolves into much greater form.
Jeremy Soller has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 9:
coreboot's current resource allocation does not work at all for modern hardware. This is not just advanced server platforms, but also mobile platforms will be affected soon.
In https://review.coreboot.org/c/coreboot/+/35946, I describe the need to pre-allocate resources for hotpluggable PCIe bridges. On some server systems, there are 8 U.2 ports, each supporting hot pluggable drives, and with the usage of adapter cards, potentially hot pluggable devices of any kind. A conservative allocation for each port would be around 256 MiB of prefetch memory. If this is done in 32-bit space, we have on such a system, 2 GiB of prefetch memory allocated.
This problem is not unique to server platforms, with lots of PCIe buses. Intel introduced integrated Thunderbolt with Ice Lake, and will be providing it across all mobile platforms with Tiger Lake. The common U-class platform will have up to four Thunderbolt ports, each one providing daisy chained PCIe support. USB 4.0 will standardize such a setup, so it will be something that will be encountered on most new hardware soon, not just these new Intel mobile platforms.
coreboot's allocator could be worked on to provide 64-bit allocation of these resources, which would be one way of addressing the problem. However, the difficulty of doing so compared to the benefit is questionable. Right now, the Linux and Tianocore payloads are fully capable of doing their own PCIe resource allocation. However, with devices being initialized by coreboot, there is no easy way to allow the devices to be re-initialized by the payload.
What this patch does is provide a quick and easy to understand way for an advanced platform to do PCIe resource allocation in the payload. No massive rewrite of the coreboot allocator has to take place. The payloads that do not support this can continue using the simple allocator coreboot has. coreboot pushes advanced initialization to the payload where it probably should be.
At System76, we are shipping products with Thunderbolt using coreboot next week. We are doing this by using a patched version of coreboot with PCIe hotplug resource allocation support. We are lucky to only have one Thunderbolt port, so we can continue using the 32-bit allocator. We will not be so lucky with our coming products. We are also working with Intel on MinPlatform, which provides an alternative to boot initialization with coreboot. The ability to do PCIe resource allocation in the payload will soon become a deciding factor in our ability to continue using and contributing to coreboot.
Jeremy Soller has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 9: Code-Review+1
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 9:
Patch Set 9:
coreboot's current resource allocation does not work at all for modern hardware. This is not just advanced server platforms, but also mobile platforms will be affected soon.
And it is exactly those platforms where I would expect that power-management related hot-plug events will require firmware to fill/assign properties to PCIe rootports or bridges in ACPI tables. Can you give me a definitive answer that is not the case or that you have already considered that and have some solution in mind?
I don't remember Ron committing significant work to PCI subsystem or devicetree for quite some time, so I don't know if is aware how devicetree interconnects with ACPI since 2014 or so.
coreboot's allocator could be worked on to provide 64-bit allocation of these resources, which would be one way of addressing the problem. However, the difficulty of doing so compared to the benefit is questionable. Right now, the Linux and Tianocore payloads are fully capable of doing their own PCIe resource allocation. However, with devices being initialized by coreboot, there is no easy way to allow the devices to be re-initialized by the payload.
Benefit: Both linux-as-a-payload and TianoCore users are in the minority of installations nowadays, while everyone would want those large MMIO resources pushed above 4 GiB allocation solved.
What this patch does is provide a quick and easy to understand way for an advanced platform to do PCIe resource allocation in the payload. No massive rewrite of the coreboot allocator has to take place. The payloads that do not support this can continue using the simple allocator coreboot has. coreboot pushes advanced initialization to the payload where it probably should be.
Please comment on and review my previous arguments. One can complete PCI enumeration without assigning the resources, that would avoid the ACPI namespace pitfalls.
Also development guidelines have certain and somewhat strong suggestion against squashing patches together, This one is borderline when the commit message already starts with this does a) and b) and c). Also we nowadays require unresolved comments to be addressed before submit is allowed, and in general rubber-stamp reviews have reduced a lot since 2014.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 9:
I'm a little nonplussed by the comments.
I think the biggest problem is that people don't stay on topic. Some say it's about ENV_PAYLOAD_LOADER, some say it isn't. Some even say this is about resource allocation, which is actually orthogonal.
That shows that there is a lot interest in related things. I guess the mailing list would be a much better place because one can follow threads on the individual topics more easily there. And maybe we could even find the problem to your solution :)
Multiple vendors have shown a need for what this CL does.
I'd assume they work with hardware, while this only works in Qemu?
It's simple. It's off by default. It fixes a problem.
What's the issue here?
We're trying to help. But if you don't want to explain the problem you're trying to solve, we can't. End of discussion?
Nobody has given a -2 here, that means you can go on. Not every patch has to make everybody happy. If you want to evaluate this further and rather have the big bang discussion on the first change that affects more than a few lines, that's ok too. But don't blame anyone but your- self then, if this turns out as a dead end.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 9:
I share Kyösti's concerns how this could be compatible to our code for real hardware targets. Existing platform code relies heavily on the VID/DID of PCI devices, that are never read with this approach. So what should it be, should we hide the minimal scanning option for all boards but those that are confirmed to be fully compatible? Should we add a big fat warning, that this is experimental and will break 90% of our ramstage code (and hence, nobody should complain or even try to fix it)? Should we remodel all of coreboot so we rely less on PCI and do things more in a blob-style manner?
Without knowing even a coarse plan how to adapt this to exis- ting hardware support, I really don't see how this is a step forward. At least a hardware PoC with extensive Linux tests (not just, it boots) would be nice. Also, if this requires changes to the OS, do we expect upstream Linux, for instance, to follow?
Hi Nico,
Hello Lean,
While I respect your technical expertise, I cannot fathom your sentiments on this case.
it's not about sentiment, it's about unanswered questions.
Do you mean that every changes in open source should be vigorously validated with most platforms,
No.
and require a coarse plan for implementation?
Yes.
From my understanding, the success and sustainability of open source relies on its openness to encourage and embrace changes no matter how big or small.
This statements makes little sense here because you project it on the size of a change? So yes, the size of a change doesn't matter. What is your point?
You should be even more understanding on this as I saw your countless changes everywhere, still we happily accept your inputs even it breaks our codes some times. With that being said, this patch is providing a forefront idea for us to further improve the flexibility of coreboot. Of course, we can't expect it to be perfect, but anyone can take it and experiment with it and further improving it. I can foresee how it will make our codes to run more dynamically, and it will also eventually push changes to the linux side as well.
It also has the potential to divide coreboot development. If we don't plan ahead how the platform code should work both with this option enabled and disabled, we might end up implementing a lot of things twice... maybe it's just me again, failing to see that other players have unlimited resources.
I can't make you to change your mind, but i am sure you know what needs to be done if we want coreboot to continue evolves into much greater form.
Of course you can change my mind. You could start by answering my questions instead of hijacking a technical discussion.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 9:
Patch Set 9:
I'm a little nonplussed by the comments. Multiple vendors have shown a need for what this CL does. It's simple. It's off by default. It fixes a problem.
What's the issue here?
Ron, I enjoyed Jeremy's thorough answer. Yours, not so much.
If we are seeking the minimal change approach to resolve resource allocation problem (which appears to be *the* vendor request based on the feedback we received so far) changes to ACPI namespace and/or AML seem largely out-of-scope and undesirable.
The keyword 'mandatory' sounds very much like a platform property that will end up being copied for each individual mainboard. Well... PCI drivers would be the perfect place to have that instead, that approach leaves all devicetree.cb files unaffected and no util/sconfig changes necessary either. One more reason why you would want to probe the device ID and bind it to a driver.
If my hunch about ACPI is correct here, and we eventually want to keep PCI enumeration without resource allocation, we are looking at 99% revert of this commit to fix it. Cases where vendors (and/or original authors) return to their code and willingly accept reverts are unfortunately rare. I did not fix the previous regressions I caused related to hidden Intel PCI devices either, I believe ultimately Nico committed the fixes(?).
The other issue that triggers -2 from me 99% of the time; Authors not respecting the time others put into reviews with good intentions. The attitude is there somewhere in the previous replies, 'Lets just merge our code now so others can fix it afterwards'. For a moment I thought I had mistakenly written my question in Finnish, but no, seemed to be fairly plain English.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 9:
Patch Set 9:
coreboot's current resource allocation does not work at all for modern hardware. This is not just advanced server platforms, but also mobile platforms will be affected soon.
In https://review.coreboot.org/c/coreboot/+/35946, I describe the need to pre-allocate resources for hotpluggable PCIe bridges. On some server systems, there are 8 U.2 ports, each supporting hot pluggable drives, and with the usage of adapter cards, potentially hot pluggable devices of any kind. A conservative allocation for each port would be around 256 MiB of prefetch memory. If this is done in 32-bit space, we have on such a system, 2 GiB of prefetch memory allocated.
This problem is not unique to server platforms, with lots of PCIe buses. Intel introduced integrated Thunderbolt with Ice Lake, and will be providing it across all mobile platforms with Tiger Lake. The common U-class platform will have up to four Thunderbolt ports, each one providing daisy chained PCIe support. USB 4.0 will standardize such a setup, so it will be something that will be encountered on most new hardware soon, not just these new Intel mobile platforms.
coreboot's allocator could be worked on to provide 64-bit allocation of these resources, which would be one way of addressing the problem. However, the difficulty of doing so compared to the benefit is questionable. Right now, the Linux and Tianocore payloads are fully capable of doing their own PCIe resource allocation. However, with devices being initialized by coreboot, there is no easy way to allow the devices to be re-initialized by the payload.
Could you please elaborate on your assessment of difficulty of adding 64-bit resource allocation? I don't think it's that hard in practice, but there are some current gotchas:
coreboot on x86 arch doesn't utilize x86-64 isa by default. If we didn't target the devices that we want to hoist to 64-bit resource allocation we could run into an issue that coreboot drivers wouldn't work or even the payloads. I think your summary is fairly accurate, but it's also assuming 64-bit being pervasive. That's certainly a great desire, but I think there are use-cases where wanting 64-bit allocation may not be warranted. In short, we just need a way to turn it off and/or steer devices to fall within the 32-bit or 64-bit address space.
For any resource intensive setup, yes, the IO hole is problematic. Instead of performing multiple reboots to settle on a layout we've typically split 32-bit address space in 2GiB/2GiB for RAM vs mmio. And, yes, that is constraining if you are assuming people need and demand 256MiB of mmio space for large number of devices (and assume the OS can't correctly reallocate).
What this patch does is provide a quick and easy to understand way for an advanced platform to do PCIe resource allocation in the payload. No massive rewrite of the coreboot allocator has to take place. The payloads that do not support this can continue using the simple allocator coreboot has. coreboot pushes advanced initialization to the payload where it probably should be.
For my own curiosity, what's your definition of advanced? Doing full USB4/thunderbolt enumeration in firmware?
At System76, we are shipping products with Thunderbolt using coreboot next week. We are doing this by using a patched version of coreboot with PCIe hotplug resource allocation support. We are lucky to only have one Thunderbolt port, so we can continue using the 32-bit allocator. We will not be so lucky with our coming products. We are also working with Intel on MinPlatform, which provides an alternative to boot initialization with coreboot. The ability to do PCIe resource allocation in the payload will soon become a deciding factor in our ability to continue using and contributing to coreboot.
I think we'll be working on/looking for solutions to these problems shortly on our end because we are pursuing usb4 on our products. I don't think the desire is controversial. While acknowledging we can improve the resource allocator, our current plans are to assume we *only* want USB4 enumeration and topology in the OS once it's booted, and I'm taking the assumption that Linux will be able to accomplish the enumeration of devices and resource assignment within its environment.
FWIW, if one believes the payload/OS is capable of enumerating and assigning resources, one can also accomplish this fairly easily w/o even touching device tree. Make a pci driver in coreboot that has an empty read_resources() callback that matches on the pci dids. The allocation is skipped and it's straight forward.
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 9:
Patch Set 9:
What's the issue here?
Ron, I enjoyed Jeremy's thorough answer. Yours, not so much.
I'm not trying to be unpleasant. I literally do not understand the objections.
I am talking to companies that don't see a path forward in coreboot without this change, e.g. Jeremy's comment: "The ability to do PCIe resource allocation in the payload will soon become a deciding factor in our ability to continue using and contributing to coreboot."
The question is not changing how coreboot does this function; the question is whether we want coreboot to do it at all. We have found a few cases in a few mainboards where we have to scan a few devices; the mandatory keyword is designed to communicate that fact.
Further, there is interest in seeing how the payload can do some of these functions, as opposed to having the ramstage do it. This is in part because the ramstage is a general purpose stage that accomodates a broad range of payloads, and what it does is not always needed.
If we are seeking the minimal change approach to resolve resource allocation problem (which appears to be *the* vendor request based on the feedback we received so far) changes to ACPI namespace and/or AML seem largely out-of-scope and undesirable.
I'm not sure you are entirely correct on some details, e.g. I've never needed ACPI information when implementing MSI-X in a kernel, just for MSI (which is why we did not bother with MSI in Akaros and Harvey, we went right to MSI-X).
This request is our first attempt at a configurable ramstage. It is a first step along a path. Based on earlier experience, we figure it is best to take this one step at a time, rather than all at once. The minimal PCI is not the only thing we are seeking, it is one of the things we are seeking. In the work of the previous year is was made clear to us that removing the ramstage was not acceptable, and that a configurable ramstage made more sense. So we are trying that.
The keyword 'mandatory' sounds very much like a platform property that will end up being copied for each individual mainboard. Well... PCI drivers would be the perfect place to have that instead,
I am not sure we know that. We chose this path as we feel that a device being mandatory is likely to be mainboard-dependent, i.e. how a mainboard uses devices, than a property of the device itself.
All that said, I respect you and Nico very highly; I don't intend to submit this until I have some feeling I'm not misunderstanding what you are telling me. I regret any bad feelings I might have caused.
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 9:
Patch Set 9:
If my hunch about ACPI is correct here, and we eventually want to keep PCI enumeration without resource allocation, we are looking at 99% revert of this commit to fix it. Cases where vendors (and/or original authors) return to their code and willingly accept reverts are unfortunately rare. I did not fix the previous regressions I caused related to hidden Intel PCI devices either, I believe ultimately Nico committed the fixes(?).
So, kyosti, were we to add this mandatory struct member to the device struct (pick a name if you don't like that one) and in the MINIMAL scanning case, have the loop skip those devices not marked mandatory, is that approach one you would find workable?
I'm fine with that assuming I understand you correctly.
For the q35 target, it would be 0 lines of sconfig change, 2 lines added to two devices, and 2 lines added to the scan bus. Would this be something that makes more sense?
thanks.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 9:
If my hunch about ACPI is correct here, and we eventually want to keep PCI enumeration without resource allocation, we are looking at 99% revert of this commit to fix it. Cases where vendors (and/or original authors) return to their code and willingly accept reverts are unfortunately rare. I did not fix the previous regressions I caused related to hidden Intel PCI devices either, I believe ultimately Nico committed the fixes(?).
I think some people may be talking past one another. I apologize if I am off. In server world hotplug devices have a system design component that does require some ACPI code for sequencing the power, etc. In the USB4/thunderbolt world peripherals are truly hot-pluggable w/o having that specific design component. The former needs per platform handling while the latter doesn't have to.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 9:
All that said, I respect you and Nico very highly; I don't intend to submit this until I have some feeling I'm not misunderstanding what you are telling me. I regret any bad feelings I might have caused.
Don't worry about the feelings. Let me try to explain what I see here. This change disables enumeration of all non-mandatory PCI devices. And you are saying this will be most of the devices in the `devicetree.cb`. So all later phases of coreboot (which I estimated as 90% of ramstage) won't know these devices exist. All code that was written to bring these devices into a state usable by the OS won't be executed, in the non blob case. And in the FSP case, we'll violate our contract which says we have assigned resources to the devices, which we haven't.
These are the most obvious effects, let's not get into details. Your commit message says, you want to minimize scanning, not mentioning any of the effects above. So looking at the imple- mentation, it seems to me, you brought a battle axe to slice cheese. And while you are already at the third slice, other people tell me, you are cutting cubes oO
That's why I would like to hear from you, Ron, what is the goal of this change? Is it about handing over control to the payload with less resources assigned (what Jeremy asks for)? or is it about scanning less to reduce boot times? or about doing less in general? or something entirely else that I miss?
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 9:
Patch Set 9:
That's why I would like to hear from you, Ron, what is the goal of this change? Is it about handing over control to the payload with less resources assigned (what Jeremy asks for)? or is it about scanning less to reduce boot times? or about doing less in general? or something entirely else that I miss?
1) yes, it is about handing over control to the payload with less resources assigned. Why?
Because, in some cases, for some uses, coreboot is doing the wrong thing in the name of supporting all possible payloads.
The coreboot allocator does a very good job of setting things up so any payload works. To make that work, it has to assume that it may boot a 32-bit payload (Aaron pointed this out I believe). In some cases, that leads to restrictions that are causing trouble for system76 and, IIUC, 9elements. In my discussions with jeremy it became pretty clear that it would be better for them were coreboot not to do this allocation; further, it's no longer clear it is needed.
64-bit systems have always had problems, going back to the early 2000s and opteron. At Los Alamos we had issues when interfaces (like Quadrics) demanded a large fraction of the low 4G. It would have been simpler for us had coreboot not tried to continue supporting 32-bit constraints.
This is not a new problem. But it has not been an issue for anyone but me until the last 2 years or so.
2) Yes, it is about reducing boot times.
Intel got some pretty good numbers by reducing what coreboot is doing. I can let them speak to that but the performance was compelling.
3) yes, it is about doing less in general
I like systems that do as little as possible. If we can remove work, I think we should.
From an esthetic standpoint, on systems I know will boot 64-bit linux kernels in flash (i.e. what Google and facebook are rolling into their data centers today -- 64-bit linux kernels in flash) running the coreboot allocator no longer seems needed. The kernel nowadays has its own preferences about how to configure PCI, and it doesn't make a lot of sense for coreboot to do a bunch of work the kernel either will undo or would like to undo.
So that's it. I would guess there are more reasons, but these are the ones that I think answer your questions.
This discussion then splits into 2 questions:
1) do we ever see coreboot allowing this kind of change? On this answer hinges the future use of coreboot by those using it now. I.e., they might well decided they need to use something else.
2) If the change in this CL is not acceptable, what change can we make that mostly eliminates coreboot PCI scanning? Is the alternative Kyosti mentioned acceptable, for example? If not, what is?
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 9:
Patch Set 9:
Patch Set 9:
If my hunch about ACPI is correct here, and we eventually want to keep PCI enumeration without resource allocation, we are looking at 99% revert of this commit to fix it. Cases where vendors (and/or original authors) return to their code and willingly accept reverts are unfortunately rare. I did not fix the previous regressions I caused related to hidden Intel PCI devices either, I believe ultimately Nico committed the fixes(?).
So, kyosti, were we to add this mandatory struct member to the device struct (pick a name if you don't like that one) and in the MINIMAL scanning case, have the loop skip those devices not marked
mandatory, is that approach one you would find workable?
There is my promise somewhere not to -2 the work you do here. Commits that I feel take us further from supporting memory resources in 64bit MMIO space will not receive my +2 either.
I was after a solution that involves _not_ modifying the devicetree.cb files at all. You would probe the PCI device ID, bind it to a PCI driver, and have the driver tell you the device is mandatory.
I'm fine with that assuming I understand you correctly.
For the q35 target, it would be 0 lines of sconfig change, 2 lines added to two devices, and 2 lines added to the scan bus. Would this be something that makes more sense?
I was after solution were you would not modify devicetree.cb files at all. For each PCI device that is bound to driver, you would need to take care callbacks that require assigned resources are skipped, so it is more code changes.
The net result (for non-blobbed PCI device init) may be that most on-board/integrated PCI devices end up being marked as mandatory. That's why I am trying to approach the problem from the other end and only leave the problematic large MMIO resources unassigned.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 9:
Patch Set 9: 2) Yes, it is about reducing boot times.
Intel got some pretty good numbers by reducing what coreboot is doing. I can let them speak to that but the performance was compelling.
I am keen to see those numbers. In the past it was often overlooked that for ramstage, and resource allocation aspecially, having serial console enabled skews those numbers and makes results heavily dependent of the loglevel one has selected.
So let's make sure we get data from some practical setupruns, i.e. make sure serial console does not spew out everything.
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 9:
Patch Set 9:
So let's make sure we get data from some practical setupruns, i.e. make sure serial console does not spew out everything.
agreed.
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 9:
Patch Set 9:
I was after a solution that involves _not_ modifying the devicetree.cb files at all. You would probe the PCI device ID, bind it to a PCI driver, and have the driver tell you the device is mandatory.
I think I was agreeing with you that we can try your proposed approach :-)
No devicetree.cb changes. No sconfig changes. I'll take a look.
I'll take a run at it in the coming days, time allowing.
The net result (for non-blobbed PCI device init) may be that most on-board/integrated PCI devices end up being marked as mandatory. That's why I am trying to approach the problem from the other end and only leave the problematic large MMIO resources unassigned.
Point taken. OK, we'll do some further work on this and let you know how it goes.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 9:
That's why I would like to hear from you, Ron, what is the goal of this change? Is it about handing over control to the payload with less resources assigned (what Jeremy asks for)? or is it about scanning less to reduce boot times? or about doing less in general? or something entirely else that I miss?
- yes, it is about handing over control to the payload with less resources assigned. Why?
...
- Yes, it is about reducing boot times.
Intel got some pretty good numbers by reducing what coreboot is doing. I can let them speak to that but the performance was compelling.
That's interesting. Though, I have to say, I've also seen numbers from Intel folks and they never came from somebody with a view on the whole firmware topic. What they suggest seems doable, though, with an OS tailored to the firmware; i.e. ignoring common inter- faces like ACPI that make PCs compatible to generic OS's.
Also, please, whenever somebody from Intel tells you about boot times, remind them (not matter who and at least once per day) that Intel chose to integrate very slow binary blobs into coreboot. coreboot: 600ms to the payload, blobboot (on much faster CPUs): 1.2s (Just random numbers from the top of my head, but I think it reflects reality pretty well. Also biased in favor of blobboot: the 600ms on GM45 include some DRAM training, the 1.2s don't.)
Actually, not about FSP runtime effects, but I've just started to document problems with its integration and why FSP-S causes more trouble than it solves: CB:36328. Feel free to add boot times as an incentive for more open source.
- yes, it is about doing less in general
I like systems that do as little as possible. If we can remove work, I think we should.
From an esthetic standpoint, on systems I know will boot 64-bit linux kernels in flash (i.e. what Google and facebook are rolling into their data centers today -- 64-bit linux kernels in flash) running the coreboot allocator no longer seems needed. The kernel nowadays has its own preferences about how to configure PCI, and it doesn't make a lot of sense for coreboot to do a bunch of work the kernel either will undo or would like to undo.
So that's it. I would guess there are more reasons, but these are the ones that I think answer your questions.
Thanks for the elaboration, actually didn't ask for reasons, but it's also very useful information :)
This discussion then splits into 2 questions:
- do we ever see coreboot allowing this kind of change? On this answer hinges the future use of coreboot by those using it now. I.e., they might well decided they need to use something else.
I'd say, yes, of course!
- If the change in this CL is not acceptable, what change can we make that mostly eliminates coreboot PCI scanning? Is the alternative Kyosti mentioned acceptable, for example? If not, what is?
I didn't follow Kyösti's suggestions in every details, but it seems he has the right idea.
PCI scanning and resource allocation are two separate problems. It is very hard to keep compatibility with the current per-platform code and get rid of the former. Resource allocation, OTOH, is only needed if coreboot itself uses the resources. Which is hard to say for SoC integrated devices (FSP blackbox issues) unless the code is open-source. It is quite possible, for instance, that if you don't assign resources to the SATA device, that FSP fails to set correct drive-strengths on the PHYs. Things like this won't be noticed without intensive testing (or reading vendor code...). So I don't think this is a problem that should be trialed-by-error.
For the resource problem, this might also be a point to start experimenting: Stub out pci_dev_read_resources() for the devices that shouldn't have resources assigned. It's still a little unclear to me which devices would need that and why, though (what can coreboot do that Linux can't undo?).
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 9:
Patch Set 9:
Patch Set 9:
I was after a solution that involves _not_ modifying the devicetree.cb files at all. You would probe the PCI device ID, bind it to a PCI driver, and have the driver tell you the device is mandatory.
I think I was agreeing with you that we can try your proposed approach :-)
No devicetree.cb changes. No sconfig changes. I'll take a look.
I'll take a run at it in the coming days, time allowing.
The net result (for non-blobbed PCI device init) may be that most on-board/integrated PCI devices end up being marked as mandatory. That's why I am trying to approach the problem from the other end and only leave the problematic large MMIO resources unassigned.
Point taken. OK, we'll do some further work on this and let you know how it goes.
I don't know who 'we' refers to there. Anyways; my opinion should be pretty clear from all the previous comments, I would rather see development efforts put directly into utilising 64-bit MMIO window for the viable (and problematic) large memory resources.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 9:
Patch Set 9:
That's why I would like to hear from you, Ron, what is the goal of this change? Is it about handing over control to the payload with less resources assigned (what Jeremy asks for)? or is it about scanning less to reduce boot times? or about doing less in general? or something entirely else that I miss?
- yes, it is about handing over control to the payload with less resources assigned. Why?
...
- Yes, it is about reducing boot times.
Intel got some pretty good numbers by reducing what coreboot is doing. I can let them speak to that but the performance was compelling.
That's interesting. Though, I have to say, I've also seen numbers from Intel folks and they never came from somebody with a view on the whole firmware topic. What they suggest seems doable, though, with an OS tailored to the firmware; i.e. ignoring common inter- faces like ACPI that make PCs compatible to generic OS's.
Also, please, whenever somebody from Intel tells you about boot times, remind them (not matter who and at least once per day) that Intel chose to integrate very slow binary blobs into coreboot. coreboot: 600ms to the payload, blobboot (on much faster CPUs): 1.2s (Just random numbers from the top of my head, but I think it reflects reality pretty well. Also biased in favor of blobboot: the 600ms on GM45 include some DRAM training, the 1.2s don't.)
Subrata, can you give an update about the experiments of completely bypassing postcar-stage on FSP2.0 platforms? From what I remember, it started with a claim of ~15ms (<1%) improvement on entry to payload. Then, after some MTRR fix in coreboot proper, the experiment ended up being a ~7ms regression. During that painful and long review period, FSP-S (?) blob was updated and it was now 100ms slower? Is my post-mortem of the situation honest and accurate? Was the speed regression finally fixed or explained?
Hello Kyösti Mälkki, Aaron Durbin, Subrata Banik, Jeremy Soller, Shelley Chen, build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Lean Sheng Tan, Philipp Deppenwiese, Nico Huber, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36221
to look at the new patch set (#10).
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
WIP: Add configurable ramstage support for minimal PCI scanning
This CL has changes that allow us to enable a configurable ramstage, and one change that allows us to minimize PCI scanning.
We add two new variables to src/Kconfig: CONFIGURABLE_RAMSTAGE is the overall variable controlling other options for minimizing the ramstage.
MINIMAL_PCI_SCANNING is how we indicate we wish to enable minimal PCI scanning.
To indicate which devices we must scan, we add a new always_scan struct member to the pci driver. Drivers which must always be scanned can set this to 1.
It is not clear that this approach is close to correct, so comments are welcome.
Change-Id: I2073d9f8e9297c2b02530821ebb634ea2a5c758e Signed-off-by: Ronald G. Minnich rminnich@gmail.com --- M src/Kconfig M src/device/pci_device.c M src/include/device/pci.h 3 files changed, 36 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/36221/10
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 10:
(7 comments)
https://review.coreboot.org/c/coreboot/+/36221/10/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/36221/10/src/device/pci_device.c@94... PS10, Line 949: if (!dev->ops && driver) code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/36221/10/src/device/pci_device.c@94... PS10, Line 949: if (!dev->ops && driver) please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/36221/10/src/device/pci_device.c@94... PS10, Line 949: if (!dev->ops && driver) suspect code indent for conditional statements (8, 10)
https://review.coreboot.org/c/coreboot/+/36221/10/src/device/pci_device.c@95... PS10, Line 950: dev->ops = (struct device_operations *)driver->ops; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/36221/10/src/device/pci_device.c@95... PS10, Line 950: dev->ops = (struct device_operations *)driver->ops; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/36221/10/src/include/device/pci.h File src/include/device/pci.h:
https://review.coreboot.org/c/coreboot/+/36221/10/src/include/device/pci.h@4... PS10, Line 42: unsigned int always_scan : 1; /* scan even if MINIMAL_PCI is set */ code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/36221/10/src/include/device/pci.h@4... PS10, Line 42: unsigned int always_scan : 1; /* scan even if MINIMAL_PCI is set */ please, no spaces at the start of a line
Hello Kyösti Mälkki, Aaron Durbin, Subrata Banik, Jeremy Soller, Shelley Chen, build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Lean Sheng Tan, Philipp Deppenwiese, Nico Huber, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36221
to look at the new patch set (#11).
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
WIP: Add configurable ramstage support for minimal PCI scanning
This CL has changes that allow us to enable a configurable ramstage, and one change that allows us to minimize PCI scanning.
We add two new variables to src/Kconfig: CONFIGURABLE_RAMSTAGE is the overall variable controlling other options for minimizing the ramstage.
MINIMAL_PCI_SCANNING is how we indicate we wish to enable minimal PCI scanning.
To indicate which devices we must scan, we add a new always_scan struct member to the pci driver. Drivers which must always be scanned can set this to 1.
It is not clear that this approach is close to correct, so comments are welcome.
Change-Id: I2073d9f8e9297c2b02530821ebb634ea2a5c758e Signed-off-by: Ronald G. Minnich rminnich@gmail.com --- M src/Kconfig M src/device/pci_device.c M src/include/device/pci.h 3 files changed, 36 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/36221/11
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36221/11/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/36221/11/src/device/pci_device.c@94... PS11, Line 947: dev->enabled = 0; You probably should never clear dev->enabled in connection with MINIMAL_PCI_SCANNING=y. This is connected to mechanisms of completely disabling said PCI functions in the hardware, i.e. they will become inaccessible even for payloads.
This approach may have the same symptomps of not creating ACPI namespace objects and not programming PCI subsystem IDs as the approach of skipping the PCI device ID probe completely.
I may not have said or even realised this before, but approach here requires that any PCI device ID with problematic resources has to be explicitly added as a dummy driver. We cannot roll firmware updates every time a new PCIe graphics card hits the shelfs, so using device ID (alone at least) as a criteria for scanning seems like a doomed idea...
In some of the comments, Aaron pointed out hooking into .read_resources. That might be the best suggestion so far.
Hello Kyösti Mälkki, Aaron Durbin, Subrata Banik, Jeremy Soller, Shelley Chen, build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Lean Sheng Tan, Philipp Deppenwiese, Nico Huber, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36221
to look at the new patch set (#12).
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
WIP: Add configurable ramstage support for minimal PCI scanning
This CL has changes that allow us to enable a configurable ramstage, and one change that allows us to minimize PCI scanning.
We add two new variables to src/Kconfig: CONFIGURABLE_RAMSTAGE is the overall variable controlling other options for minimizing the ramstage.
MINIMAL_PCI_SCANNING is how we indicate we wish to enable minimal PCI scanning.
To indicate which devices we must scan, we add a new always_scan struct member to the pci driver. Drivers which must always be scanned can set this to 1.
It is not clear that this approach is close to correct, so comments are welcome.
Change-Id: I2073d9f8e9297c2b02530821ebb634ea2a5c758e Signed-off-by: Ronald G. Minnich rminnich@gmail.com --- M src/Kconfig M src/device/pci_device.c M src/include/device/pci.h 3 files changed, 36 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/36221/12
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 12:
(1 comment)
Patch Set 11:
(1 comment)
for the read_resources approach, could I do this; in the standard read_resources, if CONFIG_MINIMAL_PCI_SCANNING is set, I just return and then add a new function read_resources_always which does not test CONFIG_MINIMAL_PCI_SCANNING?
It would be nice to do this without changing the whole tree, at least until we know if it works.
https://review.coreboot.org/c/coreboot/+/36221/11/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/36221/11/src/device/pci_device.c@94... PS11, Line 943: if (CONFIG(MINIMAL_PCI_SCANNING) && driver && !driver->always_scan) { I think we also want if (CONFIG(MINIMAL_PCI_SCANNING) && ! driver dev->enabled = 0
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 12:
Patch Set 12:
(1 comment)
Patch Set 11:
(1 comment)
for the read_resources approach, could I do this; in the standard read_resources, if CONFIG_MINIMAL_PCI_SCANNING is set, I just return and then add a new function read_resources_always which does not test CONFIG_MINIMAL_PCI_SCANNING?
It would be nice to do this without changing the whole tree, at least until we know if it works.
I would want to see a solution that does not break when payload is other than kernel or TianoCore. That means a complete PCI tree enumeration and only leaving the problematic resources unassigned. To maintain the boot diversity and leave a choice of payload to the 'user', I would hate to see SoC vendors telling their platforms now require MINIMAL_PCI_SCANNING=y to boot at all.
Those problematic resources can often be classified as PREFMEM and >= 64MiB in size. Often, they would also be graphics, so they might not be boot-critical for common payloads like SeaBIOS and GRUB. Does it matter when payload screen does not appear on 4 screens?
Currently, there should be few of these problematic resources within integrated PCI devices. The number of drivers where .read_resources needs to change is small, maybe even limited to that USB4/ThunderBolt. But Jeremy investigated even that could have generic solution by checking for some PCIe capability blocks? To handle all this ill-resource detection in pci_dev_read_resources() would take care of anonymous add-on PCIe cards too.
Hello Kyösti Mälkki, Aaron Durbin, Subrata Banik, Jeremy Soller, Shelley Chen, build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Lean Sheng Tan, Philipp Deppenwiese, Nico Huber, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36221
to look at the new patch set (#13).
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
WIP: Add configurable ramstage support for minimal PCI scanning
This CL has changes that allow us to enable a configurable ramstage, and one change that allows us to minimize PCI scanning.
We add two new variables to src/Kconfig: CONFIGURABLE_RAMSTAGE is the overall variable controlling other options for minimizing the ramstage.
MINIMAL_PCI_SCANNING is how we indicate we wish to enable minimal PCI scanning.
We modify pci_{bus,dev}_read_resources to return immediately if CONFIG(MINIMAL_PCI_SCANNING) is true. Otherwise they call pci_{bus,dev}_read_resources_always.
We provide one example where we had to scan the device, namely, the SATA device (0:1f.2) on the qemu q35 target.
Change-Id: I2073d9f8e9297c2b02530821ebb634ea2a5c758e Signed-off-by: Ronald G. Minnich rminnich@gmail.com --- M src/Kconfig M src/device/pci_device.c M src/include/device/pci.h M src/southbridge/intel/i82801ix/sata.c 4 files changed, 32 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/36221/13
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 13:
While I think Kyösti is right, that should handle problematic resources more directly, I don't see this would lead to a "minimal scanning" requirement. And if it does, we'll give them time until the next release to fix it.
Ron, if you want to pursue the course of patch set 13, there still need to be some safeguards added: * As MINIMAL_PCI_SCANNING (can we find a better name? MINIMAL_PCI_RESOURCES?) likely breaks any platform that isn't prepared for it, how about adding another Kconfig, selected by prepared platforms and let the former depend on it. * Unless we want to review every driver with a `.init` function if it can work without resources or runs into undefined behaviour, we should only call `.init` if `.read_resources` wasn't blocked. I guess we'll have to add some tag to `struct device`. This still means we'd have to review platform code that already avoids our devicetree model and accesses devices directly, e.g. with pci_dev_on_root(). Hmmm, same for `.final`.
Here's another thought, how we could avoid trouble: Instead of opting out of minimal-scanning per device, we could opt in. For instance, if MINIMAL_PCI_SCANNING is enabled, use a pci_dev_optional_read_resource() by default, and every driver that supports it, could use it too, but would have to do so explicitly.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 13:
- Unless we want to review every driver with a `.init` function if it can work without resources or runs into undefined behaviour, we should only call `.init` if `.read_resources` wasn't blocked. I guess we'll have to add some tag to `struct device`. This still means we'd have to review platform code that already avoids our devicetree model and accesses devices directly, e.g. with pci_dev_on_root(). Hmmm, same for `.final`.
I believe .init that only needs to access PCI configuration space should be allowed, even if resource BARs were left unassigned?
Any .init with a need to access registers behind BARs should already be using find_resource(), although it seems like we have cases where this is not followed by res == NULL check, though.
I started to wonder about that (coverity should have complained), and noticed there is die() in find_resource() already. Maybe address that instead of blocking .init calls?
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36221/13/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/36221/13/src/device/pci_device.c@44... PS13, Line 446: pci_bus_read_resources_always(dev); This is the generic function used for PCI bridges and PCIe rootports. Now, if none of the secondary side devices will have resources assigned, existing code should already leave the respective window resources closed (does it mean unassigned?). If any of the secondary side devices needs their resources assigned, the upstream bridge window resources must be assigned too.
I believe pci_bus_read_resources() would work without adding the conditional MINIMAL_PCI_SCANNING and I am pretty sure having it breaks some scenarios.
Eg. Intel i210 and Realtek R8168, PCIe GbE devices, require MMIO or IO resources assigned in ramstage to update ethernet MAC.
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 13:
Patch Set 13:
- As MINIMAL_PCI_SCANNING (can we find a better name? MINIMAL_PCI_RESOURCES?) likely breaks any platform that isn't prepared for it, how about adding another Kconfig, selected by prepared platforms and let the former depend on it.
I just made this one change, how's it look.
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 13:
Patch Set 13:
Patch Set 13:
- As MINIMAL_PCI_SCANNING (can we find a better name? MINIMAL_PCI_RESOURCES?) likely breaks any platform that isn't prepared for it, how about adding another Kconfig, selected by prepared platforms and let the former depend on it.
I just made this one change, how's it look.
will redo the names once this one thing looks ok, I have not forgotten.
Hello Kyösti Mälkki, Patrick Rudolph, Aaron Durbin, Subrata Banik, Jeremy Soller, Shelley Chen, build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Lean Sheng Tan, Philipp Deppenwiese, Nico Huber, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36221
to look at the new patch set (#14).
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
WIP: Add configurable ramstage support for minimal PCI scanning
This CL has changes that allow us to enable a configurable ramstage, and one change that allows us to minimize PCI scanning.
We add two new variables to src/Kconfig: CONFIGURABLE_RAMSTAGE is the overall variable controlling other options for minimizing the ramstage.
MINIMAL_PCI_SCANNING is how we indicate we wish to enable minimal PCI scanning.
We modify pci_{bus,dev}_read_resources to return immediately if CONFIG(MINIMAL_PCI_SCANNING) is true. Otherwise they call pci_{bus,dev}_read_resources_always.
We provide one example where we had to scan the device, namely, the SATA device (0:1f.2) on the qemu q35 target.
Change-Id: I2073d9f8e9297c2b02530821ebb634ea2a5c758e Signed-off-by: Ronald G. Minnich rminnich@gmail.com --- M src/Kconfig M src/device/pci_device.c M src/include/device/pci.h M src/mainboard/emulation/qemu-q35/Kconfig M src/southbridge/intel/i82801ix/sata.c 5 files changed, 38 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/36221/14
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36221/13/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/36221/13/src/device/pci_device.c@44... PS13, Line 446: pci_bus_read_resources_always(dev);
This is the generic function used for PCI bridges and PCIe rootports. […]
I think in this case I like Nico's idea, where we only mark certain boards as supporting minimal scanning. Hence, for most boards, minimal scanning is not going to be available (will not show up in Kconfig dialog).
Would that address your concern for now. I'd rather not scan through bridges if I can avoid it.
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 14:
Patch Set 13:
Here's another thought, how we could avoid trouble: Instead of opting out of minimal-scanning per device, we could opt in. For instance, if MINIMAL_PCI_SCANNING is enabled, use a pci_dev_optional_read_resource() by default, and every driver that supports it, could use it too, but would have to do so explicitly.
I'm not sure. I think I'd rather have people indicate that they really need to be scanned, with the ..._always function. It will be a bit too easy for people to avoid the opt int.
But I'm not sure I know.
Jeremy Soller has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 14: Code-Review+1
The latest patch looks like a nice way of achieving the goals I mentioned earlier
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 14:
Here's another thought, how we could avoid trouble: Instead of opting out of minimal-scanning per device, we could opt in. For instance, if MINIMAL_PCI_SCANNING is enabled, use a pci_dev_optional_read_resource() by default, and every driver that supports it, could use it too, but would have to do so explicitly.
I'm not sure. I think I'd rather have people indicate that they really need to be scanned, with the ..._always function. It will be a bit too easy for people to avoid the opt int.
But I'm not sure I know.
I think it boils down to the question if you want the author of a patch that enables minial scanning or the reviewer to do the work: * opt-in: Author has to look into each driver and decide if it can be enabled. * opt-out: Reviewer gets to check every affected driver if it doesn't run into undefined behaviour.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 14: Code-Review+1
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 14:
Patch Set 14:
Here's another thought, how we could avoid trouble: Instead of opting out of minimal-scanning per device, we could opt in. For instance, if MINIMAL_PCI_SCANNING is enabled, use a pci_dev_optional_read_resource() by default, and every driver that supports it, could use it too, but would have to do so explicitly.
I'm not sure. I think I'd rather have people indicate that they really need to be scanned, with the ..._always function. It will be a bit too easy for people to avoid the opt int.
But I'm not sure I know.
I think it boils down to the question if you want the author of a patch that enables minial scanning or the reviewer to do the work:
- opt-in: Author has to look into each driver and decide if it can be enabled.
- opt-out: Reviewer gets to check every affected driver if it doesn't run into undefined behaviour.
ok, I get the point. Overall this will make it a bit more work for people who want to try minimal scanning out, but at the same time, there are very few people, I guess, who want to try minimal scanning out :-)
I'll look at that tomorrow, it will basically flip the sense of the code but should not be a big deal and should do what you suggest.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 14:
Patch Set 14: Code-Review+1
Are we heading towards creating all PCI APIs with and without _always like been done for read_resources()?
I believe our original goal is to block probe function for all other devices except the one marked as "mandatory" in devicetree.cb right ? @Ron
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36221/13/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/36221/13/src/device/pci_device.c@44... PS13, Line 446: pci_bus_read_resources_always(dev);
I think in this case I like Nico's idea, where we only mark certain boards as supporting minimal sca […]
Let's try to avoid the use of obscure term 'scanning'.
The part I like about #13 is that it does complete PCI enumeration (probes device ID and attempts to bind that with built-in drivers). This avoids the complications of altering ACPI namespace. I did found some other places where PCI device bus numbers are injected to some tables that are forwarded to OS. Like before, I consider partial PCI enumeration harmful. Like Nico, I would replace _SCANNING in the KConfig name.
You will get my support for "restricted PCI resources allocation", the details how to accomplish this shall be polished. The semantics of .read_resources become a bit complicated if you leave PCI bridge windows unassigned, I think it is too strict a requirement to not be able to opt-in PCI devices that are situated behind PCIe bridges.
I am mostly ignoring the "boot-time" arguments for both restricted PCI enumeration and resource allocation for the time being. Performance numbers have not been produced for review and I suspect these PCI bridge window resource probes to be very lightweight.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 14:
ok, I get the point. Overall this will make it a bit more work for people who want to try minimal scanning out, but at the same time, there are very few people, I guess, who want to try minimal scanning out :-)
I'll look at that tomorrow, it will basically flip the sense of the code but should not be a big deal and should do what you suggest.
I am in the favour of opting-out, and let's not forget that PCI subsystem code is unaware of the structure of register banks behind PCI device BARs, so we could make a ruling that only the PCI driver context is allowed to access the PCI device resources.
How do you feel about the statements below:
1. PCI devices that do not bind to PCI drivers can always implicitly opt-out, because nobody is going to access those BARs. 2. Most external/add-on PCIe devices located behind PCIe bridges would hit rule #1. This includes those troublsome PCIe graphics with large PREFMEM resources. 3. With the recursive nature of how .read_resources already works, upstream PCIe bridges would implicitly opt-out in the case all the secondary side devices have opted out. 4. Within PCI drivers, .init is the only callback where resource assignment may be required. 5. If .init is not implemented at all, we can implicitly opt-out. 6. If .init does not call any find_resource(), we can explicitly opt-out by using alternative .read_resources() implementation with a conditional MINIMAL_PCI_RESOURCES check in it. 7. If .init does call find_resource(), it's likely for the reason that does configurations that are dependent of either some Kconfig or devicetree.cb. Or it could be a static chipset configuration that is not easy to delay to payload. These devices can rarely opt-out, but it could be done explicitly using custom .read_resources() like rule #6. 8. Platforms with blobbed chipset initialisation would currently likely hit rule #6, but it might be possible to have them hit #5 instead by removing .init callbacks. I have not looked into them really. 9. If choice of payload is capable of completing the resource allocation, possibly extending to 64-bit MMIO space, it may be desirable to leave as many resources as possible unallocated and potentially even unallocate the ones that had been assigned.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 14:
Patch Set 14:
Patch Set 14: Code-Review+1
Are we heading towards creating all PCI APIs with and without _always like been done for read_resources()?
I believe our original goal is to block probe function for all other devices except the one marked as "mandatory" in devicetree.cb right ? @Ron
Some of us (Aaron, Nico, me) seem to be much against (that original goal or approach) of partial PCI probing/enumeration due the undesirable side-effects mentioned along the review comments here. Furthermore, some of us seem to agree the real problem is with the resource allocation and not the PCI enumeration part.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 14:
Patch Set 14:
Patch Set 14:
Patch Set 14: Code-Review+1
Are we heading towards creating all PCI APIs with and without _always like been done for read_resources()?
I believe our original goal is to block probe function for all other devices except the one marked as "mandatory" in devicetree.cb right ? @Ron
Some of us (Aaron, Nico, me) seem to be much against (that original goal or approach) of partial PCI probing/enumeration due the undesirable side-effects mentioned along the review comments here. Furthermore, some of us seem to agree the real problem is with the resource allocation and not the PCI enumeration part.
if we divide the original definition of PCI enumeration then it combination of discovering the device, marking devices numbering and allocate the resource, enabling resource. Not in static world we really don't have to do PCI device discover unless its on PCI-E root ports so resource allocation might be only significant things we have in hand right now. isn't it?
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 14:
NOTE: I pushed CB:36493 containing the same statemens I had made in my previous comment. Better comment the concerns and/or validity there inline, so the documentation side of things would not drag so much behind.
Existence of that document does not mean this commit would have to fulfill all the bullets mentioned there, before we can merge something.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 14:
Patch Set 14:
Patch Set 14:
Patch Set 14:
Patch Set 14: Code-Review+1
Are we heading towards creating all PCI APIs with and without _always like been done for read_resources()?
I believe our original goal is to block probe function for all other devices except the one marked as "mandatory" in devicetree.cb right ? @Ron
Some of us (Aaron, Nico, me) seem to be much against (that original goal or approach) of partial PCI probing/enumeration due the undesirable side-effects mentioned along the review comments here. Furthermore, some of us seem to agree the real problem is with the resource allocation and not the PCI enumeration part.
if we divide the original definition of PCI enumeration then it combination of discovering the device, marking devices numbering and allocate the resource, enabling resource. Not in static world we really don't have to do PCI device discover unless its on PCI-E root ports so resource allocation might be only significant things we have in hand right now. isn't it?
The PCI subsystem has always had separation of enumeration and resource allocation. It is only this development of "MINIMAL_PCI_SCANNING" where that was forgotten.
I think with #13 we reached some agreement that complete PCI enumeration is not harmful. I hope from now on this commit could concentrate only on addressing the somewhat urgent problem of PCI resource allocation.
As for any arguments about boot-time enhancements that could be achieved by partial PCI enumeration, let's just delay that discussion and development to another commit.
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 14:
Patch Set 14: The PCI subsystem has always had separation of enumeration and resource allocation. It is only this development of "MINIMAL_PCI_SCANNING" where that was forgotten.
As the author of the original version of that code, I think I can safely say it was not forgotten.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 14:
So have we gone from wanting to not assign resources for certain devices to not assigning resources for almost all devices unless someone explicitly changes the code?
1. I don't think that was our original problem. At least for USB4/thunderbolt there's a finite set of viddids we can purposefully not allocate resources for.
2. Then that leaves people plugging in GPU(s) with large amount of resources. That's a different problem, and I assume we can solve that by pushing those into 64-bit address space. I'm hand waving some details that we may need to add, but I think that solves that problem.
I think I can envision a path for 1 and 2 and seems fairly straight forward. But if one wants coreboot *not* to assign any resources I think that just complicates matters w.r.t. __pci_drivers and other implicit expectations. Is one just going to knock down all the pci command registers for every visible pci device prior to jumping to payload? That seems cleaner from an implementation perspective, but I think that's going to lead to unintended issues. Should we tweak such a notion and do it only for devces (and their parent) on secondary buses?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 14:
Patch Set 14:
So have we gone from wanting to not assign resources for certain devices to not assigning resources for almost all devices unless someone explicitly changes the code?
i still believe our goal should be "wanting to not assign resources for certain devices" and that we should control using some static keyword what Ron has made like "mandatory". like in my experiment earlier. In order to boot to payload we need only below devices
1. Host Bridge 0:0:0 to create basic memory mark as RAM 2. IGD if we are in !NORMAL_MODE 3. Block Device like eMMC/NVME (incase of NVME we might need to look for PCIE root port, but that is also fixed should we look for 0:1c:0 or 0:1d:0 and in case of eMMC we anyway have dedicated PCI B:D:F)
Just by allocating resources dynamically to these 3 devices we could boot to OS happily. I believe that was the agenda when we say configurable ramstage option enable. Isn't it ?
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 14:
Patch Set 14:
So have we gone from wanting to not assign resources for certain devices to not assigning resources for almost all devices unless someone explicitly changes the code?
- I don't think that was our original problem. At least for USB4/thunderbolt there's a finite set of viddids we can purposefully not allocate resources for.
I.e. a blacklist? That actually looks more complex to me than this current fairly simple solution.
What if I express what you just said a little differently?
We are saying that for certain select mainboards, where such a capability is of interest, we can enable letting the payload do almost all the work of PCI configuration.
In that case, for those mainboards, yes, we can skip just about all devices, and this is something several companies would like to have. They've expressed interest at OSFC, on the coreboot telecon, on the mailing list, to me personally, and in this CL discussion.
We're not claiming this approach works for everything. Then again, we know the current system is not working for everything either, and as Jeremy points out, there are problems coming our way; a general solution that works for all payloads seems increasingly unlikely; and, even if likely, starts to look pretty complicated.
The jury is still out on this current approach; people are supposed to be reporting back to me with some results on real hardware but I have not heard anything yet.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 14:
Patch Set 14:
So have we gone from wanting to not assign resources for certain devices to not assigning resources for almost all devices unless someone explicitly changes the code?
- I don't think that was our original problem. At least for USB4/thunderbolt there's a finite set of viddids we can purposefully not allocate resources for.
If we have a pattern, I would like to avoid VID/DID matching. I believe Jeremy investigated already there are some PCI class or capabilities that would allow generic USB4/thunderbolt detection.
- Then that leaves people plugging in GPU(s) with large amount of resources. That's a different problem, and I assume we can solve that by pushing those into 64-bit address space. I'm hand waving some details that we may need to add, but I think that solves that problem.
That is the approach I would want vendor to really put the effort into. But it turns out to be more work than I plan to volunteer for at least. For the review of this commit I am mostly in "damage management mode" to overlook that an unnecessary complex bandage does not land.
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 14:
Patch Set 14:
That is the approach I would want vendor to really put the effort into. But it turns out to be more work than I plan to volunteer for at least. For the review of this commit I am mostly in "damage management mode" to overlook that an unnecessary complex bandage does not land.
Kyosti, how does the damage look at present. I'm working on one more approach to try to bring in Nico's idea but not done yet.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 14:
Patch Set 14:
Patch Set 14:
That is the approach I would want vendor to really put the effort into. But it turns out to be more work than I plan to volunteer for at least. For the review of this commit I am mostly in "damage management mode" to overlook that an unnecessary complex bandage does not land.
Kyosti, how does the damage look at present. I'm working on one more approach to try to bring in Nico's idea but not done yet.
It's much less intrusive than before :). Specially if we take into account that there would be few cases where you need to explicitly opt-out.
However, looking at some logged times created from CB:36574 and CB:36584, I am a bit concerned that majority of advertised performance boost of partial enumeration / allocation would be due to console. Still waiting for Intel to provide us with some data.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 14:
Patch Set 14:
Patch Set 14:
So have we gone from wanting to not assign resources for certain devices to not assigning resources for almost all devices unless someone explicitly changes the code?
- I don't think that was our original problem. At least for USB4/thunderbolt there's a finite set of viddids we can purposefully not allocate resources for.
I.e. a blacklist? That actually looks more complex to me than this current fairly simple solution.
What if I express what you just said a little differently?
We are saying that for certain select mainboards, where such a capability is of interest, we can enable letting the payload do almost all the work of PCI configuration.
In that case, for those mainboards, yes, we can skip just about all devices, and this is something several companies would like to have. They've expressed interest at OSFC, on the coreboot telecon, on the mailing list, to me personally, and in this CL discussion.
We're not claiming this approach works for everything. Then again, we know the current system is not working for everything either, and as Jeremy points out, there are problems coming our way; a general solution that works for all payloads seems increasingly unlikely; and, even if likely, starts to look pretty complicated.
The jury is still out on this current approach; people are supposed to be reporting back to me with some results on real hardware but I have not heard anything yet.
I would say what you describe, Ron, is requirement #3: Opt out all resource assignment. I am fairly certain that with some of the existing chipsets and supporting code this won't work without adding additional hacks/changes.
The addition of #3 definitely doesn't solve the cases Chrome OS would need. I'm trying to understand the superset of people's desires. Jeremy's description of his usage seemed to fall into #1 and #2, but your request is unique from what I have read.
FWIW, I think it's important that the post coreboot boundary is effectively an OS. Getting all the hardware initialized does take needing to access registers, etc, even if things don't look like pci standard. So are we going to undo that programming so the payload/OS can reprogram BARs? It would be the simplest from a coding perspective since it's apply this policy to every found pci device.
ron minnich has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Abandoned
This has been a useful discussion, with the result that we're exploring some other approaches.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36221/6/src/include/device/device.h File src/include/device/device.h:
https://review.coreboot.org/c/coreboot/+/36221/6/src/include/device/device.h... PS6, Line 132: unsigned int mandatory : 1; /* set if this device is used even in minimum PCI cases */
line over 96 characters
Ack
ron minnich has restored this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: WIP: Add configurable ramstage support for minimal PCI scanning ......................................................................
Restored
Subrata Banik has uploaded a new patch set (#15) to the change originally created by ron minnich. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Add configurable ramstage support for minimal PCI scanning
This CL has changes that allow us to enable a configurable ramstage, and one change that allows us to minimize PCI scanning. Minimal scanning is a frequently requested feature.
To enable it, we add two new variables to src/Kconfig CONFIGURABLE_RAMSTAGE is the overall variable controlling other options for minimizing the ramstage.
MINIMAL_PCI_SCANNING is how we indicate we wish to enable minimal PCI scanning.
Some devices must be scanned in all cases, such as 0:0.0.
To indicate which devices we must scan, we add a new mandatory keyword to sconfig
It is used in place of on, off, or hidden, and indicates a device is enabled and mandatory. Mandatory devices are always scanned. When MINIMAL_PCI_SCANNING is enabled, ONLY mandatory devices are scanned.
We further add support in src/device/pci_device.c to manage both MINIMAL_PCI_SCANNING and mandatory devices.
Finally, to show how this works in practice, we add mandatory keywords to 3 devices on the qemu-q35.
TEST= 1. This is tested and working on the qemu-q35 target. 2. On CML-Hatch
Before CL: Total Boot time: ~685ms
After CL: Total Boot time: ~615ms
Change-Id: I2073d9f8e9297c2b02530821ebb634ea2a5c758e Signed-off-by: Ronald G. Minnich rminnich@gmail.com --- M src/Kconfig M src/device/pci_device.c M src/include/device/device.h M src/mainboard/emulation/qemu-q35/devicetree.cb M util/sconfig/lex.yy.c_shipped M util/sconfig/main.c M util/sconfig/sconfig.h M util/sconfig/sconfig.l M util/sconfig/sconfig.tab.c_shipped M util/sconfig/sconfig.tab.h_shipped M util/sconfig/sconfig.y 11 files changed, 387 insertions(+), 355 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/36221/15
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 15: Code-Review+2
Jeremy Soller has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 15: Code-Review+2
This looks good to me
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 15: Code-Review+2
I guess I can +2 since this was brought back by subrata?
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 15:
(1 comment)
Subrata, did you rebase patchset #7 and push it as #15?
https://review.coreboot.org/c/coreboot/+/36221/15/util/sconfig/sconfig.tab.h... File util/sconfig/sconfig.tab.h_shipped:
https://review.coreboot.org/c/coreboot/+/36221/15/util/sconfig/sconfig.tab.h... PS15, Line 34: # define YY_YY_HOME_RMINNICH_PROJECTS_LINUXBOOT_COREBOOTNERF_GITHUBCOREBOOT_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED Are you sure you want these or did you not review the work?
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 15:
(1 comment)
Patch Set 15:
(1 comment)
Subrata, did you rebase patchset #7 and push it as #15?
https://review.coreboot.org/c/coreboot/+/36221/15/util/sconfig/sconfig.tab.h... File util/sconfig/sconfig.tab.h_shipped:
https://review.coreboot.org/c/coreboot/+/36221/15/util/sconfig/sconfig.tab.h... PS15, Line 34: # define YY_YY_HOME_RMINNICH_PROJECTS_LINUXBOOT_COREBOOTNERF_GITHUBCOREBOOT_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED
Are you sure you want these or did you not review the work?
It's just a guard. It's a moral hazard of shipping generated source code artifacts.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 16:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36221/11/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/36221/11/src/device/pci_device.c@94... PS11, Line 947: dev->enabled = 0;
You probably should never clear dev->enabled in connection with MINIMAL_PCI_SCANNING=y. […]
Ack
https://review.coreboot.org/c/coreboot/+/36221/13/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/36221/13/src/device/pci_device.c@44... PS13, Line 446: pci_bus_read_resources_always(dev);
Let's try to avoid the use of obscure term 'scanning'. […]
Ack
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 16:
(1 comment)
Patch Set 15:
(1 comment)
Subrata, did you rebase patchset #7 and push it as #15?
yes, patchset #7 looks more logic and very minimum to configure from mainboard
https://review.coreboot.org/c/coreboot/+/36221/15/util/sconfig/sconfig.tab.h... File util/sconfig/sconfig.tab.h_shipped:
https://review.coreboot.org/c/coreboot/+/36221/15/util/sconfig/sconfig.tab.h... PS15, Line 34: # define YY_YY_HOME_RMINNICH_PROJECTS_LINUXBOOT_COREBOOTNERF_GITHUBCOREBOOT_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED
It's just a guard. It's a moral hazard of shipping generated source code artifacts.
Ack
Subrata Banik has uploaded a new patch set (#17) to the change originally created by ron minnich. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Add configurable ramstage support for minimal PCI scanning
This CL has changes that allow us to enable a configurable ramstage, and one change that allows us to minimize PCI scanning. Minimal scanning is a frequently requested feature.
To enable it, we add two new variables to src/Kconfig CONFIGURABLE_RAMSTAGE is the overall variable controlling other options for minimizing the ramstage.
MINIMAL_PCI_SCANNING is how we indicate we wish to enable minimal PCI scanning.
Some devices must be scanned in all cases, such as 0:0.0.
To indicate which devices we must scan, we add a new mandatory keyword to sconfig
It is used in place of on, off, or hidden, and indicates a device is enabled and mandatory. Mandatory devices are always scanned. When MINIMAL_PCI_SCANNING is enabled, ONLY mandatory devices are scanned.
We further add support in src/device/pci_device.c to manage both MINIMAL_PCI_SCANNING and mandatory devices.
Finally, to show how this works in practice, we add mandatory keywords to 3 devices on the qemu-q35.
TEST= 1. This is tested and working on the qemu-q35 target. 2. On CML-Hatch
Before CL: Total Boot time: ~685ms
After CL: Total Boot time: ~615ms
Change-Id: I2073d9f8e9297c2b02530821ebb634ea2a5c758e Signed-off-by: Ronald G. Minnich rminnich@gmail.com --- M src/Kconfig M src/device/pci_device.c M src/include/device/device.h M src/mainboard/emulation/qemu-q35/devicetree.cb M util/sconfig/lex.yy.c_shipped M util/sconfig/main.c M util/sconfig/sconfig.h M util/sconfig/sconfig.l M util/sconfig/sconfig.tab.c_shipped M util/sconfig/sconfig.tab.h_shipped M util/sconfig/sconfig.y 11 files changed, 387 insertions(+), 355 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/36221/17
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 17:
(3 comments)
https://review.coreboot.org/c/coreboot/+/36221/9/src/Kconfig File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/36221/9/src/Kconfig@340 PS9, Line 340: default y if ARCH_X86
Why the arch dependency?
i can clean this up later
https://review.coreboot.org/c/coreboot/+/36221/9/src/Kconfig@348 PS9, Line 348: help
This option should only be visible for boards with devicetree. […]
i can clean this up later
https://review.coreboot.org/c/coreboot/+/36221/11/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/36221/11/src/device/pci_device.c@94... PS11, Line 943: if (CONFIG(MINIMAL_PCI_SCANNING) && driver && !driver->always_scan) {
I think we also want […]
Ack
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 17:
(6 comments)
https://review.coreboot.org/c/coreboot/+/36221/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36221/8//COMMIT_MSG@25 PS8, Line 25: keyword to sconfig
it's intentional
Ack
https://review.coreboot.org/c/coreboot/+/36221/8/src/Kconfig File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/36221/8/src/Kconfig@344 PS8, Line 344: The minimal PCI scanning will only check those parts that are enabled : in the devicetree.cb. By convention none of those devices should be bridges.
I just blew it away, I did not see a need for this text.
Ack
https://review.coreboot.org/c/coreboot/+/36221/8/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/36221/8/src/device/pci_device.c@119... PS8, Line 1191: dev = pcidev_path_behind(bus, devfn);
Can you give links to reviews of these past "bikeshedded passes" because I don't remember reviewing […]
Ack
https://review.coreboot.org/c/coreboot/+/36221/8/util/sconfig/lex.yy.c_shipp... File util/sconfig/lex.yy.c_shipped:
https://review.coreboot.org/c/coreboot/+/36221/8/util/sconfig/lex.yy.c_shipp... PS8, Line 9: #define YY_FLEX_SUBMINOR_VERSION 4
Why increment it by 3?
Ack
https://review.coreboot.org/c/coreboot/+/36221/8/util/sconfig/sconfig.tab.c_... File util/sconfig/sconfig.tab.c_shipped:
https://review.coreboot.org/c/coreboot/+/36221/8/util/sconfig/sconfig.tab.c_... PS8, Line 112: #ifndef YY_YY_HOME_RMINNICH_PROJECTS_LINUXBOOT_COREBOOTNERF_GITHUBCOREBOOT_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED
this is a generated file. Not much to be done I think.
Ack
https://review.coreboot.org/c/coreboot/+/36221/8/util/sconfig/sconfig.tab.h_... File util/sconfig/sconfig.tab.h_shipped:
https://review.coreboot.org/c/coreboot/+/36221/8/util/sconfig/sconfig.tab.h_... PS8, Line 33: #ifndef YY_YY_HOME_RMINNICH_PROJECTS_LINUXBOOT_COREBOOTNERF_GITHUBCOREBOOT_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED
ditto :-)
Ack
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 17:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36221/4/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/36221/4/src/device/pci_device.c@119... PS4, Line 1197: printk(BIOS_WARNING, "%#x is NOT static, skipping it\n", devfn);
I blew away the print, it's a time waster.
Ack
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 17:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36221/8/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/36221/8/src/device/pci_device.c@122... PS8, Line 1229: *prev = dev->sibling;
To skip scanning of devices also affects ACPI namespace. […]
Ack
https://review.coreboot.org/c/coreboot/+/36221/3/util/sconfig/lex.yy.c_shipp... File util/sconfig/lex.yy.c_shipped:
https://review.coreboot.org/c/coreboot/+/36221/3/util/sconfig/lex.yy.c_shipp... PS3, Line 162:
is this valid or existing junk whitespace ?
Ack
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 17: Code-Review+2
Jeremy Soller has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 17: Code-Review+2
ron minnich has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Add configurable ramstage support for minimal PCI scanning
This CL has changes that allow us to enable a configurable ramstage, and one change that allows us to minimize PCI scanning. Minimal scanning is a frequently requested feature.
To enable it, we add two new variables to src/Kconfig CONFIGURABLE_RAMSTAGE is the overall variable controlling other options for minimizing the ramstage.
MINIMAL_PCI_SCANNING is how we indicate we wish to enable minimal PCI scanning.
Some devices must be scanned in all cases, such as 0:0.0.
To indicate which devices we must scan, we add a new mandatory keyword to sconfig
It is used in place of on, off, or hidden, and indicates a device is enabled and mandatory. Mandatory devices are always scanned. When MINIMAL_PCI_SCANNING is enabled, ONLY mandatory devices are scanned.
We further add support in src/device/pci_device.c to manage both MINIMAL_PCI_SCANNING and mandatory devices.
Finally, to show how this works in practice, we add mandatory keywords to 3 devices on the qemu-q35.
TEST= 1. This is tested and working on the qemu-q35 target. 2. On CML-Hatch
Before CL: Total Boot time: ~685ms
After CL: Total Boot time: ~615ms
Change-Id: I2073d9f8e9297c2b02530821ebb634ea2a5c758e Signed-off-by: Ronald G. Minnich rminnich@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/36221 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Jeremy Soller jeremy@system76.com --- M src/Kconfig M src/device/pci_device.c M src/include/device/device.h M src/mainboard/emulation/qemu-q35/devicetree.cb M util/sconfig/lex.yy.c_shipped M util/sconfig/main.c M util/sconfig/sconfig.h M util/sconfig/sconfig.l M util/sconfig/sconfig.tab.c_shipped M util/sconfig/sconfig.tab.h_shipped M util/sconfig/sconfig.y 11 files changed, 387 insertions(+), 355 deletions(-)
Approvals: build bot (Jenkins): Verified ron minnich: Looks good to me, approved Jeremy Soller: Looks good to me, approved
diff --git a/src/Kconfig b/src/Kconfig index f538a1c..3742c04 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -354,6 +354,21 @@ Skip PCI enumeration logic and only allocate BAR for fixed devices (bootable devices, TPM over GSPI).
+config CONFIGURABLE_RAMSTAGE + bool "Enable a configurable ramstage." + default y if ARCH_X86 + help + A configurable ramstage allows you to select which parts of the ramstage + to run. Currently, we can only select a minimal PCI scanning step. + The minimal PCI scanning will only check those parts that are enabled + in the devicetree.cb. By convention none of those devices should be bridges. + +config MINIMAL_PCI_SCANNING + bool "Enable minimal PCI scanning" + depends on CONFIGURABLE_RAMSTAGE + help + If this option is enabled, coreboot will scan only devices + marked as mandatory in devicetree.cb endmenu
menu "Mainboard" diff --git a/src/device/pci_device.c b/src/device/pci_device.c index 47c0e9f..b1e88a6 100644 --- a/src/device/pci_device.c +++ b/src/device/pci_device.c @@ -1195,6 +1195,12 @@ * non-existence and single function devices. */ for (devfn = min_devfn; devfn <= max_devfn; devfn++) { + if (CONFIG(MINIMAL_PCI_SCANNING)) { + dev = pcidev_path_behind(bus, devfn); + if (!dev || !dev->mandatory) + continue; + } + /* First thing setup the device structure. */ dev = pci_scan_get_dev(bus, devfn);
diff --git a/src/include/device/device.h b/src/include/device/device.h index c3a1106..333ac5d 100644 --- a/src/include/device/device.h +++ b/src/include/device/device.h @@ -119,7 +119,10 @@ unsigned int initialized : 1; /* 1 if we have initialized the device */ unsigned int on_mainboard : 1; unsigned int disable_pcie_aspm : 1; - unsigned int hidden : 1; /* set if we should hide from UI */ + /* set if we should hide from UI */ + unsigned int hidden : 1; + /* set if this device is used even in minimum PCI cases */ + unsigned int mandatory : 1; u8 command; uint16_t hotplug_buses; /* Number of hotplug buses to allocate */
diff --git a/src/mainboard/emulation/qemu-q35/devicetree.cb b/src/mainboard/emulation/qemu-q35/devicetree.cb index 671a2d6..c032606 100644 --- a/src/mainboard/emulation/qemu-q35/devicetree.cb +++ b/src/mainboard/emulation/qemu-q35/devicetree.cb @@ -5,10 +5,10 @@ end end device domain 0 on - device pci 0.0 on end # northbridge (q35) + device pci 0.0 mandatory end # northbridge (q35) chip southbridge/intel/i82801ix # present unconditionally - device pci 1f.0 on end # LPC + device pci 1f.0 mandatory end # LPC device pci 1f.2 on end # SATA device pci 1f.3 on end # SMBus
diff --git a/util/sconfig/lex.yy.c_shipped b/util/sconfig/lex.yy.c_shipped index 14ffeff..6329773 100644 --- a/util/sconfig/lex.yy.c_shipped +++ b/util/sconfig/lex.yy.c_shipped @@ -6,7 +6,7 @@ #define FLEX_SCANNER #define YY_FLEX_MAJOR_VERSION 2 #define YY_FLEX_MINOR_VERSION 6 -#define YY_FLEX_SUBMINOR_VERSION 1 +#define YY_FLEX_SUBMINOR_VERSION 4 #if YY_FLEX_SUBMINOR_VERSION > 0 #define FLEX_BETA #endif @@ -81,10 +81,16 @@ #define UINT32_MAX (4294967295U) #endif
+#ifndef SIZE_MAX +#define SIZE_MAX (~(size_t)0) +#endif + #endif /* ! C99 */
#endif /* ! FLEXINT_H */
+/* begin standard C++ headers. */ + /* TODO: this is always defined, so inline it */ #define yyconst const
@@ -97,32 +103,26 @@ /* Returned upon end-of-file. */ #define YY_NULL 0
-/* Promotes a possibly negative, possibly signed char to an unsigned - * integer for use as an array index. If the signed char is negative, - * we want to instead treat it as an 8-bit unsigned char, hence the - * double cast. +/* Promotes a possibly negative, possibly signed char to an + * integer in range [0..255] for use as an array index. */ -#define YY_SC_TO_UI(c) ((unsigned int) (unsigned char) c) +#define YY_SC_TO_UI(c) ((YY_CHAR) (c))
/* Enter a start condition. This macro really ought to take a parameter, * but we do it the disgusting crufty way forced on us by the ()-less * definition of BEGIN. */ #define BEGIN (yy_start) = 1 + 2 * - /* Translate the current start state into a value that can be later handed * to BEGIN to return to the state. The YYSTATE alias is for lex * compatibility. */ #define YY_START (((yy_start) - 1) / 2) #define YYSTATE YY_START - /* Action number for EOF rule of a given start state. */ #define YY_STATE_EOF(state) (YY_END_OF_BUFFER + state + 1) - /* Special action meaning "start processing a new file". */ -#define YY_NEW_FILE yyrestart(yyin ) - +#define YY_NEW_FILE yyrestart( yyin ) #define YY_END_OF_BUFFER_CHAR 0
/* Size of default input buffer. */ @@ -159,16 +159,14 @@ #define EOB_ACT_CONTINUE_SCAN 0 #define EOB_ACT_END_OF_FILE 1 #define EOB_ACT_LAST_MATCH 2 - #define YY_LESS_LINENO(n) #define YY_LINENO_REWIND_TO(ptr) - /* Return all but the first "n" matched characters back to the input stream. */ #define yyless(n) \ do \ { \ /* Undo effects of setting up yytext. */ \ - yy_size_t yyless_macro_arg = (n); \ + int yyless_macro_arg = (n); \ YY_LESS_LINENO(yyless_macro_arg);\ *yy_cp = (yy_hold_char); \ YY_RESTORE_YY_MORE_OFFSET \ @@ -176,7 +174,6 @@ YY_DO_BEFORE_ACTION; /* set up yytext again */ \ } \ while ( 0 ) - #define unput(c) yyunput( c, (yytext_ptr) )
#ifndef YY_STRUCT_YY_BUFFER_STATE @@ -258,7 +255,6 @@ #define YY_CURRENT_BUFFER ( (yy_buffer_stack) \ ? (yy_buffer_stack)[(yy_buffer_stack_top)] \ : NULL) - /* Same as previous macro, but useful when we know that the buffer stack is not * NULL or when we need an lvalue. For internal use only. */ @@ -279,62 +275,56 @@ */ static int yy_did_buffer_switch_on_eof;
-void yyrestart (FILE *input_file ); -void yy_switch_to_buffer (YY_BUFFER_STATE new_buffer ); -YY_BUFFER_STATE yy_create_buffer (FILE *file,int size ); -void yy_delete_buffer (YY_BUFFER_STATE b ); -void yy_flush_buffer (YY_BUFFER_STATE b ); -void yypush_buffer_state (YY_BUFFER_STATE new_buffer ); -void yypop_buffer_state (void ); +void yyrestart ( FILE *input_file ); +void yy_switch_to_buffer ( YY_BUFFER_STATE new_buffer ); +YY_BUFFER_STATE yy_create_buffer ( FILE *file, int size ); +void yy_delete_buffer ( YY_BUFFER_STATE b ); +void yy_flush_buffer ( YY_BUFFER_STATE b ); +void yypush_buffer_state ( YY_BUFFER_STATE new_buffer ); +void yypop_buffer_state ( void );
-static void yyensure_buffer_stack (void ); -static void yy_load_buffer_state (void ); -static void yy_init_buffer (YY_BUFFER_STATE b,FILE *file ); +static void yyensure_buffer_stack ( void ); +static void yy_load_buffer_state ( void ); +static void yy_init_buffer ( YY_BUFFER_STATE b, FILE *file ); +#define YY_FLUSH_BUFFER yy_flush_buffer( YY_CURRENT_BUFFER )
-#define YY_FLUSH_BUFFER yy_flush_buffer(YY_CURRENT_BUFFER ) +YY_BUFFER_STATE yy_scan_buffer ( char *base, yy_size_t size ); +YY_BUFFER_STATE yy_scan_string ( const char *yy_str ); +YY_BUFFER_STATE yy_scan_bytes ( const char *bytes, int len );
-YY_BUFFER_STATE yy_scan_buffer (char *base,yy_size_t size ); -YY_BUFFER_STATE yy_scan_string (yyconst char *yy_str ); -YY_BUFFER_STATE yy_scan_bytes (yyconst char *bytes,int len ); - -void *yyalloc (yy_size_t ); -void *yyrealloc (void *,yy_size_t ); -void yyfree (void * ); +void *yyalloc ( yy_size_t ); +void *yyrealloc ( void *, yy_size_t ); +void yyfree ( void * );
#define yy_new_buffer yy_create_buffer - #define yy_set_interactive(is_interactive) \ { \ if ( ! YY_CURRENT_BUFFER ){ \ yyensure_buffer_stack (); \ YY_CURRENT_BUFFER_LVALUE = \ - yy_create_buffer(yyin,YY_BUF_SIZE ); \ + yy_create_buffer( yyin, YY_BUF_SIZE ); \ } \ YY_CURRENT_BUFFER_LVALUE->yy_is_interactive = is_interactive; \ } - #define yy_set_bol(at_bol) \ { \ if ( ! YY_CURRENT_BUFFER ){\ yyensure_buffer_stack (); \ YY_CURRENT_BUFFER_LVALUE = \ - yy_create_buffer(yyin,YY_BUF_SIZE ); \ + yy_create_buffer( yyin, YY_BUF_SIZE ); \ } \ YY_CURRENT_BUFFER_LVALUE->yy_at_bol = at_bol; \ } - #define YY_AT_BOL() (YY_CURRENT_BUFFER_LVALUE->yy_at_bol)
/* Begin user sect3 */ - -typedef unsigned char YY_CHAR; +typedef flex_uint8_t YY_CHAR;
FILE *yyin = NULL, *yyout = NULL;
typedef int yy_state_type;
extern int yylineno; - int yylineno = 1;
extern char *yytext; @@ -343,10 +333,10 @@ #endif #define yytext_ptr yytext
-static yy_state_type yy_get_previous_state (void ); -static yy_state_type yy_try_NUL_trans (yy_state_type current_state ); -static int yy_get_next_buffer (void ); -static void yynoreturn yy_fatal_error (yyconst char* msg ); +static yy_state_type yy_get_previous_state ( void ); +static yy_state_type yy_try_NUL_trans ( yy_state_type current_state ); +static int yy_get_next_buffer ( void ); +static void yynoreturn yy_fatal_error ( const char* msg );
/* Done after the current pattern has been matched and before the * corresponding action - sets up yytext. @@ -357,9 +347,8 @@ (yy_hold_char) = *yy_cp; \ *yy_cp = '\0'; \ (yy_c_buf_p) = yy_cp; - -#define YY_NUM_RULES 38 -#define YY_END_OF_BUFFER 39 +#define YY_NUM_RULES 39 +#define YY_END_OF_BUFFER 40 /* This struct is not used in this scanner, but its presence is necessary. */ struct yy_trans_info @@ -367,28 +356,29 @@ flex_int32_t yy_verify; flex_int32_t yy_nxt; }; -static yyconst flex_int16_t yy_accept[160] = +static const flex_int16_t yy_accept[168] = { 0, - 0, 0, 39, 37, 1, 3, 37, 37, 37, 32, - 32, 30, 33, 37, 33, 33, 33, 37, 37, 37, - 37, 37, 37, 37, 37, 37, 37, 37, 1, 3, - 37, 0, 37, 37, 0, 2, 32, 33, 37, 37, - 37, 37, 33, 37, 37, 37, 37, 37, 37, 37, - 24, 37, 37, 37, 37, 7, 37, 37, 37, 37, - 37, 37, 37, 36, 36, 37, 0, 31, 37, 37, - 16, 37, 37, 23, 28, 37, 37, 13, 37, 37, - 22, 37, 37, 8, 10, 12, 37, 37, 20, 37, - 21, 37, 0, 34, 4, 37, 37, 37, 37, 37, + 0, 0, 40, 38, 1, 3, 38, 38, 38, 33, + 33, 31, 34, 38, 34, 34, 34, 38, 38, 38, + 38, 38, 38, 38, 38, 38, 38, 38, 1, 3, + 38, 0, 38, 38, 0, 2, 33, 34, 38, 38, + 38, 38, 34, 38, 38, 38, 38, 38, 38, 38, + 25, 38, 38, 38, 38, 38, 7, 38, 38, 38, + 38, 38, 38, 38, 37, 37, 38, 0, 32, 38, + 38, 17, 38, 38, 24, 29, 38, 38, 14, 38, + 38, 23, 38, 38, 38, 8, 11, 13, 38, 38, + 21, 38, 22, 38, 0, 35, 4, 38, 38, 38,
- 37, 37, 37, 19, 37, 37, 37, 35, 35, 37, - 37, 37, 37, 37, 37, 37, 14, 37, 37, 37, - 37, 5, 17, 37, 9, 37, 11, 37, 37, 37, - 37, 18, 26, 37, 37, 37, 37, 37, 37, 6, - 37, 37, 37, 37, 37, 37, 37, 25, 37, 37, - 15, 37, 27, 37, 37, 37, 37, 29, 0 + 38, 38, 38, 38, 38, 38, 20, 38, 38, 38, + 36, 36, 38, 38, 38, 38, 38, 38, 38, 15, + 38, 38, 38, 38, 38, 5, 18, 38, 9, 38, + 12, 38, 38, 38, 38, 38, 19, 27, 38, 38, + 38, 38, 38, 38, 38, 38, 6, 38, 38, 38, + 38, 10, 38, 38, 38, 26, 38, 38, 16, 38, + 28, 38, 38, 38, 38, 30, 0 } ;
-static yyconst YY_CHAR yy_ec[256] = +static const YY_CHAR yy_ec[256] = { 0, 1, 1, 1, 1, 1, 1, 1, 1, 2, 3, 1, 1, 4, 1, 1, 1, 1, 1, 1, 1, @@ -420,7 +410,7 @@ 1, 1, 1, 1, 1 } ;
-static yyconst YY_CHAR yy_meta[39] = +static const YY_CHAR yy_meta[39] = { 0, 1, 2, 2, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, @@ -428,114 +418,118 @@ 1, 1, 1, 1, 1, 1, 1, 1 } ;
-static yyconst flex_uint16_t yy_base[167] = +static const flex_int16_t yy_base[175] = { 0, - 0, 0, 227, 0, 224, 228, 222, 37, 41, 38, - 187, 0, 44, 209, 54, 78, 60, 201, 196, 45, - 203, 192, 42, 47, 197, 62, 184, 0, 214, 228, - 77, 210, 88, 69, 211, 228, 0, 87, 104, 198, - 187, 176, 93, 183, 178, 188, 179, 186, 186, 180, - 186, 171, 171, 175, 177, 0, 173, 167, 173, 177, - 169, 175, 174, 0, 228, 101, 186, 0, 179, 159, - 172, 162, 169, 0, 0, 164, 164, 0, 162, 152, - 0, 156, 151, 0, 0, 0, 154, 153, 0, 144, - 0, 171, 170, 0, 0, 155, 154, 147, 139, 149, + 0, 0, 235, 0, 232, 236, 230, 37, 41, 38, + 195, 0, 44, 217, 54, 78, 60, 209, 204, 45, + 211, 48, 42, 52, 206, 62, 193, 0, 223, 236, + 88, 219, 93, 79, 220, 236, 0, 93, 104, 207, + 196, 185, 96, 192, 187, 197, 188, 195, 195, 189, + 195, 180, 180, 181, 183, 185, 0, 181, 175, 181, + 185, 177, 183, 182, 0, 236, 115, 194, 0, 187, + 167, 180, 170, 177, 0, 0, 172, 172, 0, 170, + 160, 0, 164, 168, 158, 0, 0, 0, 161, 160, + 0, 151, 0, 178, 177, 0, 0, 162, 161, 154,
- 137, 143, 148, 0, 133, 136, 126, 0, 228, 137, - 141, 133, 135, 131, 133, 138, 0, 122, 122, 121, - 118, 0, 0, 133, 0, 117, 134, 128, 132, 113, - 113, 0, 0, 120, 112, 110, 121, 94, 95, 0, - 94, 92, 97, 86, 85, 84, 76, 0, 71, 78, - 0, 67, 0, 61, 55, 32, 29, 0, 228, 40, - 129, 131, 133, 135, 137, 139 + 146, 156, 144, 150, 155, 156, 0, 139, 142, 132, + 0, 236, 143, 147, 139, 141, 137, 139, 144, 0, + 128, 127, 127, 126, 123, 0, 0, 138, 0, 122, + 139, 125, 132, 136, 117, 117, 0, 0, 124, 116, + 115, 113, 124, 97, 98, 91, 0, 102, 100, 98, + 83, 0, 80, 83, 74, 0, 60, 63, 0, 63, + 0, 56, 51, 33, 29, 0, 236, 40, 132, 134, + 136, 138, 140, 142 } ;
-static yyconst flex_int16_t yy_def[167] = +static const flex_int16_t yy_def[175] = { 0, - 159, 1, 159, 160, 159, 159, 160, 161, 162, 160, - 10, 160, 10, 160, 10, 10, 10, 160, 160, 160, - 160, 160, 160, 160, 160, 160, 160, 160, 159, 159, - 161, 163, 164, 162, 165, 159, 10, 10, 10, 160, - 160, 160, 10, 160, 160, 160, 160, 160, 160, 160, - 160, 160, 160, 160, 160, 160, 160, 160, 160, 160, - 160, 160, 160, 160, 159, 164, 166, 39, 160, 160, - 160, 160, 160, 160, 160, 160, 160, 160, 160, 160, - 160, 160, 160, 160, 160, 160, 160, 160, 160, 160, - 160, 160, 159, 160, 160, 160, 160, 160, 160, 160, + 167, 1, 167, 168, 167, 167, 168, 169, 170, 168, + 10, 168, 10, 168, 10, 10, 10, 168, 168, 168, + 168, 168, 168, 168, 168, 168, 168, 168, 167, 167, + 169, 171, 172, 170, 173, 167, 10, 10, 10, 168, + 168, 168, 10, 168, 168, 168, 168, 168, 168, 168, + 168, 168, 168, 168, 168, 168, 168, 168, 168, 168, + 168, 168, 168, 168, 168, 167, 172, 174, 39, 168, + 168, 168, 168, 168, 168, 168, 168, 168, 168, 168, + 168, 168, 168, 168, 168, 168, 168, 168, 168, 168, + 168, 168, 168, 168, 167, 168, 168, 168, 168, 168,
- 160, 160, 160, 160, 160, 160, 160, 160, 159, 160, - 160, 160, 160, 160, 160, 160, 160, 160, 160, 160, - 160, 160, 160, 160, 160, 160, 160, 160, 160, 160, - 160, 160, 160, 160, 160, 160, 160, 160, 160, 160, - 160, 160, 160, 160, 160, 160, 160, 160, 160, 160, - 160, 160, 160, 160, 160, 160, 160, 160, 0, 159, - 159, 159, 159, 159, 159, 159 + 168, 168, 168, 168, 168, 168, 168, 168, 168, 168, + 168, 167, 168, 168, 168, 168, 168, 168, 168, 168, + 168, 168, 168, 168, 168, 168, 168, 168, 168, 168, + 168, 168, 168, 168, 168, 168, 168, 168, 168, 168, + 168, 168, 168, 168, 168, 168, 168, 168, 168, 168, + 168, 168, 168, 168, 168, 168, 168, 168, 168, 168, + 168, 168, 168, 168, 168, 168, 0, 167, 167, 167, + 167, 167, 167, 167 } ;
-static yyconst flex_uint16_t yy_nxt[267] = +static const flex_int16_t yy_nxt[275] = { 0, 4, 5, 6, 7, 8, 9, 10, 11, 10, 12, 13, 13, 14, 4, 4, 4, 13, 13, 15, 16, 17, 13, 18, 19, 20, 21, 22, 4, 23, 24, 4, 25, 26, 4, 27, 4, 4, 4, 32, 32, - 28, 33, 35, 36, 37, 37, 37, 158, 38, 38, + 28, 33, 35, 36, 37, 37, 37, 166, 38, 38, 38, 38, 38, 49, 38, 38, 38, 38, 38, 38, - 38, 38, 38, 55, 157, 57, 38, 38, 38, 56, - 35, 36, 50, 51, 58, 156, 52, 41, 32, 32, - 155, 64, 154, 42, 38, 38, 38, 46, 60, 67, - 67, 61, 28, 38, 38, 38, 62, 153, 43, 38, + 38, 38, 38, 56, 54, 165, 38, 38, 38, 57, + 58, 164, 50, 51, 55, 163, 52, 41, 162, 59, + 35, 36, 161, 42, 38, 38, 38, 46, 61, 32, + 32, 62, 65, 160, 68, 68, 63, 28, 43, 38,
- 38, 38, 67, 67, 152, 92, 44, 151, 150, 45, - 68, 68, 68, 149, 68, 68, 148, 147, 146, 145, - 68, 68, 68, 68, 68, 68, 144, 143, 72, 31, - 31, 34, 34, 32, 32, 66, 66, 35, 35, 67, - 67, 142, 141, 140, 139, 138, 137, 136, 135, 134, + 38, 38, 38, 38, 38, 159, 44, 158, 157, 45, + 69, 69, 69, 156, 69, 69, 68, 68, 155, 94, + 69, 69, 69, 69, 69, 69, 154, 153, 152, 151, + 150, 73, 31, 31, 34, 34, 32, 32, 67, 67, + 35, 35, 68, 68, 149, 148, 147, 146, 145, 144, + 143, 142, 141, 140, 139, 138, 137, 136, 135, 134, 133, 132, 131, 130, 129, 128, 127, 126, 125, 124, 123, 122, 121, 120, 119, 118, 117, 116, 115, 114, 113, 112, 111, 110, 109, 108, 107, 106, 105, 104, - 103, 102, 101, 100, 99, 98, 97, 96, 95, 94, - 93, 91, 90, 89, 88, 87, 86, 85, 84, 83, + 103, 102, 101, 100, 99, 98, 97, 96, 95, 93,
- 82, 81, 80, 79, 78, 77, 76, 75, 74, 73, - 71, 70, 69, 36, 65, 29, 63, 59, 54, 53, - 48, 47, 40, 39, 30, 29, 159, 3, 159, 159, - 159, 159, 159, 159, 159, 159, 159, 159, 159, 159, - 159, 159, 159, 159, 159, 159, 159, 159, 159, 159, - 159, 159, 159, 159, 159, 159, 159, 159, 159, 159, - 159, 159, 159, 159, 159, 159 + 92, 91, 90, 89, 88, 87, 86, 85, 84, 83, + 82, 81, 80, 79, 78, 77, 76, 75, 74, 72, + 71, 70, 36, 66, 29, 64, 60, 53, 48, 47, + 40, 39, 30, 29, 167, 3, 167, 167, 167, 167, + 167, 167, 167, 167, 167, 167, 167, 167, 167, 167, + 167, 167, 167, 167, 167, 167, 167, 167, 167, 167, + 167, 167, 167, 167, 167, 167, 167, 167, 167, 167, + 167, 167, 167, 167 } ;
-static yyconst flex_int16_t yy_chk[267] = +static const flex_int16_t yy_chk[275] = { 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 8, 8, - 160, 8, 9, 9, 10, 10, 10, 157, 10, 10, + 168, 8, 9, 9, 10, 10, 10, 165, 10, 10, 13, 13, 13, 20, 10, 10, 10, 10, 10, 10, - 15, 15, 15, 23, 156, 24, 17, 17, 17, 23, - 34, 34, 20, 20, 24, 155, 20, 15, 31, 31, - 154, 31, 152, 15, 16, 16, 16, 17, 26, 33, - 33, 26, 33, 38, 38, 38, 26, 150, 16, 43, + 15, 15, 15, 23, 22, 164, 17, 17, 17, 23, + 24, 163, 20, 20, 22, 162, 20, 15, 160, 24, + 34, 34, 158, 15, 16, 16, 16, 17, 26, 31, + 31, 26, 31, 157, 33, 33, 26, 33, 16, 38,
- 43, 43, 66, 66, 149, 66, 16, 147, 146, 16, - 39, 39, 39, 145, 39, 39, 144, 143, 142, 141, - 39, 39, 39, 39, 39, 39, 139, 138, 43, 161, - 161, 162, 162, 163, 163, 164, 164, 165, 165, 166, - 166, 137, 136, 135, 134, 131, 130, 129, 128, 127, - 126, 124, 121, 120, 119, 118, 116, 115, 114, 113, - 112, 111, 110, 107, 106, 105, 103, 102, 101, 100, - 99, 98, 97, 96, 93, 92, 90, 88, 87, 83, - 82, 80, 79, 77, 76, 73, 72, 71, 70, 69, - 67, 63, 62, 61, 60, 59, 58, 57, 55, 54, + 38, 38, 43, 43, 43, 155, 16, 154, 153, 16, + 39, 39, 39, 151, 39, 39, 67, 67, 150, 67, + 39, 39, 39, 39, 39, 39, 149, 148, 146, 145, + 144, 43, 169, 169, 170, 170, 171, 171, 172, 172, + 173, 173, 174, 174, 143, 142, 141, 140, 139, 136, + 135, 134, 133, 132, 131, 130, 128, 125, 124, 123, + 122, 121, 119, 118, 117, 116, 115, 114, 113, 110, + 109, 108, 106, 105, 104, 103, 102, 101, 100, 99, + 98, 95, 94, 92, 90, 89, 85, 84, 83, 81, + 80, 78, 77, 74, 73, 72, 71, 70, 68, 64,
- 53, 52, 51, 50, 49, 48, 47, 46, 45, 44, - 42, 41, 40, 35, 32, 29, 27, 25, 22, 21, - 19, 18, 14, 11, 7, 5, 3, 159, 159, 159, - 159, 159, 159, 159, 159, 159, 159, 159, 159, 159, - 159, 159, 159, 159, 159, 159, 159, 159, 159, 159, - 159, 159, 159, 159, 159, 159, 159, 159, 159, 159, - 159, 159, 159, 159, 159, 159 + 63, 62, 61, 60, 59, 58, 56, 55, 54, 53, + 52, 51, 50, 49, 48, 47, 46, 45, 44, 42, + 41, 40, 35, 32, 29, 27, 25, 21, 19, 18, + 14, 11, 7, 5, 3, 167, 167, 167, 167, 167, + 167, 167, 167, 167, 167, 167, 167, 167, 167, 167, + 167, 167, 167, 167, 167, 167, 167, 167, 167, 167, + 167, 167, 167, 167, 167, 167, 167, 167, 167, 167, + 167, 167, 167, 167 } ;
static yy_state_type yy_last_accepting_state; @@ -586,36 +580,36 @@ #define YY_EXTRA_TYPE void * #endif
-static int yy_init_globals (void ); +static int yy_init_globals ( void );
/* Accessor methods to globals. These are made visible to non-reentrant scanners for convenience. */
-int yylex_destroy (void ); +int yylex_destroy ( void );
-int yyget_debug (void ); +int yyget_debug ( void );
-void yyset_debug (int debug_flag ); +void yyset_debug ( int debug_flag );
-YY_EXTRA_TYPE yyget_extra (void ); +YY_EXTRA_TYPE yyget_extra ( void );
-void yyset_extra (YY_EXTRA_TYPE user_defined ); +void yyset_extra ( YY_EXTRA_TYPE user_defined );
-FILE *yyget_in (void ); +FILE *yyget_in ( void );
-void yyset_in (FILE * _in_str ); +void yyset_in ( FILE * _in_str );
-FILE *yyget_out (void ); +FILE *yyget_out ( void );
-void yyset_out (FILE * _out_str ); +void yyset_out ( FILE * _out_str );
- int yyget_leng (void ); + int yyget_leng ( void );
-char *yyget_text (void ); +char *yyget_text ( void );
-int yyget_lineno (void ); +int yyget_lineno ( void );
-void yyset_lineno (int _line_number ); +void yyset_lineno ( int _line_number );
/* Macros after this point can all be overridden by user definitions in * section 1. @@ -623,32 +617,31 @@
#ifndef YY_SKIP_YYWRAP #ifdef __cplusplus -extern "C" int yywrap (void ); +extern "C" int yywrap ( void ); #else -extern int yywrap (void ); +extern int yywrap ( void ); #endif #endif
#ifndef YY_NO_UNPUT
- static void yyunput (int c,char *buf_ptr ); + static void yyunput ( int c, char *buf_ptr );
#endif
#ifndef yytext_ptr -static void yy_flex_strncpy (char *,yyconst char *,int ); +static void yy_flex_strncpy ( char *, const char *, int ); #endif
#ifdef YY_NEED_STRLEN -static int yy_flex_strlen (yyconst char * ); +static int yy_flex_strlen ( const char * ); #endif
#ifndef YY_NO_INPUT - #ifdef __cplusplus -static int yyinput (void ); +static int yyinput ( void ); #else -static int input (void ); +static int input ( void ); #endif
#endif @@ -781,10 +774,10 @@ if ( ! YY_CURRENT_BUFFER ) { yyensure_buffer_stack (); YY_CURRENT_BUFFER_LVALUE = - yy_create_buffer(yyin,YY_BUF_SIZE ); + yy_create_buffer( yyin, YY_BUF_SIZE ); }
- yy_load_buffer_state( ); + yy_load_buffer_state( ); }
{ @@ -814,13 +807,13 @@ while ( yy_chk[yy_base[yy_current_state] + yy_c] != yy_current_state ) { yy_current_state = (int) yy_def[yy_current_state]; - if ( yy_current_state >= 160 ) - yy_c = yy_meta[(unsigned int) yy_c]; + if ( yy_current_state >= 168 ) + yy_c = yy_meta[yy_c]; } - yy_current_state = yy_nxt[yy_base[yy_current_state] + (flex_int16_t) yy_c]; + yy_current_state = yy_nxt[yy_base[yy_current_state] + yy_c]; ++yy_cp; } - while ( yy_base[yy_current_state] != 228 ); + while ( yy_base[yy_current_state] != 236 );
yy_find_action: yy_act = yy_accept[yy_current_state]; @@ -880,95 +873,95 @@ YY_BREAK case 9: YY_RULE_SETUP -{yylval.number=3; return(HIDDEN);} +{yylval.number=3; return(STATUS);} YY_BREAK case 10: YY_RULE_SETUP -{yylval.number=PCI; return(BUS);} +{yylval.number=5; return(STATUS);} YY_BREAK case 11: YY_RULE_SETUP -{yylval.number=IOAPIC; return(BUS);} +{yylval.number=PCI; return(BUS);} YY_BREAK case 12: YY_RULE_SETUP -{yylval.number=PNP; return(BUS);} +{yylval.number=IOAPIC; return(BUS);} YY_BREAK case 13: YY_RULE_SETUP -{yylval.number=I2C; return(BUS);} +{yylval.number=PNP; return(BUS);} YY_BREAK case 14: YY_RULE_SETUP -{yylval.number=APIC; return(BUS);} +{yylval.number=I2C; return(BUS);} YY_BREAK case 15: YY_RULE_SETUP -{yylval.number=CPU_CLUSTER; return(BUS);} +{yylval.number=APIC; return(BUS);} YY_BREAK case 16: YY_RULE_SETUP -{yylval.number=CPU; return(BUS);} +{yylval.number=CPU_CLUSTER; return(BUS);} YY_BREAK case 17: YY_RULE_SETUP -{yylval.number=DOMAIN; return(BUS);} +{yylval.number=CPU; return(BUS);} YY_BREAK case 18: YY_RULE_SETUP -{yylval.number=GENERIC; return(BUS);} +{yylval.number=DOMAIN; return(BUS);} YY_BREAK case 19: YY_RULE_SETUP -{yylval.number=MMIO; return(BUS);} +{yylval.number=GENERIC; return(BUS);} YY_BREAK case 20: YY_RULE_SETUP -{yylval.number=SPI; return(BUS);} +{yylval.number=MMIO; return(BUS);} YY_BREAK case 21: YY_RULE_SETUP -{yylval.number=USB; return(BUS);} +{yylval.number=SPI; return(BUS);} YY_BREAK case 22: YY_RULE_SETUP -{yylval.number=IRQ; return(RESOURCE);} +{yylval.number=USB; return(BUS);} YY_BREAK case 23: YY_RULE_SETUP -{yylval.number=DRQ; return(RESOURCE);} +{yylval.number=IRQ; return(RESOURCE);} YY_BREAK case 24: YY_RULE_SETUP -{yylval.number=IO; return(RESOURCE);} +{yylval.number=DRQ; return(RESOURCE);} YY_BREAK case 25: YY_RULE_SETUP -{return(IOAPIC_IRQ);} +{yylval.number=IO; return(RESOURCE);} YY_BREAK case 26: YY_RULE_SETUP -{return(INHERIT);} +{return(IOAPIC_IRQ);} YY_BREAK case 27: YY_RULE_SETUP -{return(SUBSYSTEMID);} +{return(INHERIT);} YY_BREAK case 28: YY_RULE_SETUP -{return(END);} +{return(SUBSYSTEMID);} YY_BREAK case 29: YY_RULE_SETUP -{return(SLOT_DESC);} +{return(END);} YY_BREAK case 30: YY_RULE_SETUP -{return(EQUALS);} +{return(SLOT_DESC);} YY_BREAK case 31: YY_RULE_SETUP -{yylval.string = malloc(yyleng+1); strncpy(yylval.string, yytext, yyleng); yylval.string[yyleng]='\0'; return(NUMBER);} +{return(EQUALS);} YY_BREAK case 32: YY_RULE_SETUP @@ -980,12 +973,11 @@ YY_BREAK case 34: YY_RULE_SETUP -{yylval.string = malloc(yyleng+1); strncpy(yylval.string, yytext, yyleng); yylval.string[yyleng]='\0'; return(PCIINT);} +{yylval.string = malloc(yyleng+1); strncpy(yylval.string, yytext, yyleng); yylval.string[yyleng]='\0'; return(NUMBER);} YY_BREAK case 35: -/* rule 35 can match eol */ YY_RULE_SETUP -{yylval.string = malloc(yyleng-1); strncpy(yylval.string, yytext+1, yyleng-2); yylval.string[yyleng-2]='\0'; return(STRING);} +{yylval.string = malloc(yyleng+1); strncpy(yylval.string, yytext, yyleng); yylval.string[yyleng]='\0'; return(PCIINT);} YY_BREAK case 36: /* rule 36 can match eol */ @@ -993,10 +985,15 @@ {yylval.string = malloc(yyleng-1); strncpy(yylval.string, yytext+1, yyleng-2); yylval.string[yyleng-2]='\0'; return(STRING);} YY_BREAK case 37: +/* rule 37 can match eol */ +YY_RULE_SETUP +{yylval.string = malloc(yyleng-1); strncpy(yylval.string, yytext+1, yyleng-2); yylval.string[yyleng-2]='\0'; return(STRING);} + YY_BREAK +case 38: YY_RULE_SETUP {yylval.string = malloc(yyleng+1); strncpy(yylval.string, yytext, yyleng); yylval.string[yyleng]='\0'; return(STRING);} YY_BREAK -case 38: +case 39: YY_RULE_SETUP ECHO; YY_BREAK @@ -1077,7 +1074,7 @@ { (yy_did_buffer_switch_on_eof) = 0;
- if ( yywrap( ) ) + if ( yywrap( ) ) { /* Note: because we've taken care in * yy_get_next_buffer() to have set up @@ -1144,7 +1141,7 @@ { char *dest = YY_CURRENT_BUFFER_LVALUE->yy_ch_buf; char *source = (yytext_ptr); - yy_size_t number_to_move, i; + int number_to_move, i; int ret_val;
if ( (yy_c_buf_p) > &YY_CURRENT_BUFFER_LVALUE->yy_ch_buf[(yy_n_chars) + 1] ) @@ -1173,7 +1170,7 @@ /* Try to read more data. */
/* First move last chars to start of buffer. */ - number_to_move = (yy_size_t) ((yy_c_buf_p) - (yytext_ptr)) - 1; + number_to_move = (int) ((yy_c_buf_p) - (yytext_ptr) - 1);
for ( i = 0; i < number_to_move; ++i ) *(dest++) = *(source++); @@ -1209,7 +1206,8 @@
b->yy_ch_buf = (char *) /* Include room in for 2 EOB chars. */ - yyrealloc((void *) b->yy_ch_buf,(yy_size_t) (b->yy_buf_size + 2) ); + yyrealloc( (void *) b->yy_ch_buf, + (yy_size_t) (b->yy_buf_size + 2) ); } else /* Can't grow it, we don't own it. */ @@ -1241,7 +1239,7 @@ if ( number_to_move == YY_MORE_ADJ ) { ret_val = EOB_ACT_END_OF_FILE; - yyrestart(yyin ); + yyrestart( yyin ); }
else @@ -1255,12 +1253,15 @@ else ret_val = EOB_ACT_CONTINUE_SCAN;
- if ((int) ((yy_n_chars) + number_to_move) > YY_CURRENT_BUFFER_LVALUE->yy_buf_size) { + if (((yy_n_chars) + number_to_move) > YY_CURRENT_BUFFER_LVALUE->yy_buf_size) { /* Extend the array by 50%, plus the number we really need. */ int new_size = (yy_n_chars) + number_to_move + ((yy_n_chars) >> 1); - YY_CURRENT_BUFFER_LVALUE->yy_ch_buf = (char *) yyrealloc((void *) YY_CURRENT_BUFFER_LVALUE->yy_ch_buf,(yy_size_t) new_size ); + YY_CURRENT_BUFFER_LVALUE->yy_ch_buf = (char *) yyrealloc( + (void *) YY_CURRENT_BUFFER_LVALUE->yy_ch_buf, (yy_size_t) new_size ); if ( ! YY_CURRENT_BUFFER_LVALUE->yy_ch_buf ) YY_FATAL_ERROR( "out of dynamic memory in yy_get_next_buffer()" ); + /* "- 2" to take care of EOB's */ + YY_CURRENT_BUFFER_LVALUE->yy_buf_size = (int) (new_size - 2); }
(yy_n_chars) += number_to_move; @@ -1292,10 +1293,10 @@ while ( yy_chk[yy_base[yy_current_state] + yy_c] != yy_current_state ) { yy_current_state = (int) yy_def[yy_current_state]; - if ( yy_current_state >= 160 ) - yy_c = yy_meta[(unsigned int) yy_c]; + if ( yy_current_state >= 168 ) + yy_c = yy_meta[yy_c]; } - yy_current_state = yy_nxt[yy_base[yy_current_state] + (flex_int16_t) yy_c]; + yy_current_state = yy_nxt[yy_base[yy_current_state] + yy_c]; }
return yy_current_state; @@ -1320,11 +1321,11 @@ while ( yy_chk[yy_base[yy_current_state] + yy_c] != yy_current_state ) { yy_current_state = (int) yy_def[yy_current_state]; - if ( yy_current_state >= 160 ) - yy_c = yy_meta[(unsigned int) yy_c]; + if ( yy_current_state >= 168 ) + yy_c = yy_meta[yy_c]; } - yy_current_state = yy_nxt[yy_base[yy_current_state] + (flex_int16_t) yy_c]; - yy_is_jam = (yy_current_state == 159); + yy_current_state = yy_nxt[yy_base[yy_current_state] + yy_c]; + yy_is_jam = (yy_current_state == 167);
return yy_is_jam ? 0 : yy_current_state; } @@ -1394,7 +1395,7 @@
else { /* need more input */ - int offset = (yy_c_buf_p) - (yytext_ptr); + int offset = (int) ((yy_c_buf_p) - (yytext_ptr)); ++(yy_c_buf_p);
switch ( yy_get_next_buffer( ) ) @@ -1411,13 +1412,13 @@ */
/* Reset buffer status. */ - yyrestart(yyin ); + yyrestart( yyin );
/*FALLTHROUGH*/
case EOB_ACT_END_OF_FILE: { - if ( yywrap( ) ) + if ( yywrap( ) ) return 0;
if ( ! (yy_did_buffer_switch_on_eof) ) @@ -1455,11 +1456,11 @@ if ( ! YY_CURRENT_BUFFER ){ yyensure_buffer_stack (); YY_CURRENT_BUFFER_LVALUE = - yy_create_buffer(yyin,YY_BUF_SIZE ); + yy_create_buffer( yyin, YY_BUF_SIZE ); }
- yy_init_buffer(YY_CURRENT_BUFFER,input_file ); - yy_load_buffer_state( ); + yy_init_buffer( YY_CURRENT_BUFFER, input_file ); + yy_load_buffer_state( ); }
/** Switch to a different input buffer. @@ -1487,7 +1488,7 @@ }
YY_CURRENT_BUFFER_LVALUE = new_buffer; - yy_load_buffer_state( ); + yy_load_buffer_state( );
/* We don't actually know whether we did this switch during * EOF (yywrap()) processing, but the only time this flag @@ -1515,7 +1516,7 @@ { YY_BUFFER_STATE b;
- b = (YY_BUFFER_STATE) yyalloc(sizeof( struct yy_buffer_state ) ); + b = (YY_BUFFER_STATE) yyalloc( sizeof( struct yy_buffer_state ) ); if ( ! b ) YY_FATAL_ERROR( "out of dynamic memory in yy_create_buffer()" );
@@ -1524,13 +1525,13 @@ /* yy_ch_buf has to be 2 characters longer than the size given because * we need to put in 2 end-of-buffer characters. */ - b->yy_ch_buf = (char *) yyalloc((yy_size_t) (b->yy_buf_size + 2) ); + b->yy_ch_buf = (char *) yyalloc( (yy_size_t) (b->yy_buf_size + 2) ); if ( ! b->yy_ch_buf ) YY_FATAL_ERROR( "out of dynamic memory in yy_create_buffer()" );
b->yy_is_our_buffer = 1;
- yy_init_buffer(b,file ); + yy_init_buffer( b, file );
return b; } @@ -1549,9 +1550,9 @@ YY_CURRENT_BUFFER_LVALUE = (YY_BUFFER_STATE) 0;
if ( b->yy_is_our_buffer ) - yyfree((void *) b->yy_ch_buf ); + yyfree( (void *) b->yy_ch_buf );
- yyfree((void *) b ); + yyfree( (void *) b ); }
/* Initializes or reinitializes a buffer. @@ -1563,7 +1564,7 @@ { int oerrno = errno;
- yy_flush_buffer(b ); + yy_flush_buffer( b );
b->yy_input_file = file; b->yy_fill_buffer = 1; @@ -1606,7 +1607,7 @@ b->yy_buffer_status = YY_BUFFER_NEW;
if ( b == YY_CURRENT_BUFFER ) - yy_load_buffer_state( ); + yy_load_buffer_state( ); }
/** Pushes the new state onto the stack. The new state becomes @@ -1637,7 +1638,7 @@ YY_CURRENT_BUFFER_LVALUE = new_buffer;
/* copied from yy_switch_to_buffer. */ - yy_load_buffer_state( ); + yy_load_buffer_state( ); (yy_did_buffer_switch_on_eof) = 1; }
@@ -1656,7 +1657,7 @@ --(yy_buffer_stack_top);
if (YY_CURRENT_BUFFER) { - yy_load_buffer_state( ); + yy_load_buffer_state( ); (yy_did_buffer_switch_on_eof) = 1; } } @@ -1666,7 +1667,7 @@ */ static void yyensure_buffer_stack (void) { - int num_to_alloc; + yy_size_t num_to_alloc;
if (!(yy_buffer_stack)) {
@@ -1723,7 +1724,7 @@ /* They forgot to leave room for the EOB's. */ return NULL;
- b = (YY_BUFFER_STATE) yyalloc(sizeof( struct yy_buffer_state ) ); + b = (YY_BUFFER_STATE) yyalloc( sizeof( struct yy_buffer_state ) ); if ( ! b ) YY_FATAL_ERROR( "out of dynamic memory in yy_scan_buffer()" );
@@ -1737,7 +1738,7 @@ b->yy_fill_buffer = 0; b->yy_buffer_status = YY_BUFFER_NEW;
- yy_switch_to_buffer(b ); + yy_switch_to_buffer( b );
return b; } @@ -1750,10 +1751,10 @@ * @note If you want to scan bytes that may contain NUL values, then use * yy_scan_bytes() instead. */ -YY_BUFFER_STATE yy_scan_string (yyconst char * yystr ) +YY_BUFFER_STATE yy_scan_string (const char * yystr ) {
- return yy_scan_bytes(yystr,(int) strlen(yystr) ); + return yy_scan_bytes( yystr, (int) strlen(yystr) ); }
/** Setup the input buffer state to scan the given bytes. The next call to yylex() will @@ -1763,7 +1764,7 @@ * * @return the newly allocated buffer state object. */ -YY_BUFFER_STATE yy_scan_bytes (yyconst char * yybytes, int _yybytes_len ) +YY_BUFFER_STATE yy_scan_bytes (const char * yybytes, int _yybytes_len ) { YY_BUFFER_STATE b; char *buf; @@ -1772,7 +1773,7 @@
/* Get memory for full buffer, including space for trailing EOB's. */ n = (yy_size_t) (_yybytes_len + 2); - buf = (char *) yyalloc(n ); + buf = (char *) yyalloc( n ); if ( ! buf ) YY_FATAL_ERROR( "out of dynamic memory in yy_scan_bytes()" );
@@ -1781,7 +1782,7 @@
buf[_yybytes_len] = buf[_yybytes_len+1] = YY_END_OF_BUFFER_CHAR;
- b = yy_scan_buffer(buf,n ); + b = yy_scan_buffer( buf, n ); if ( ! b ) YY_FATAL_ERROR( "bad buffer in yy_scan_bytes()" );
@@ -1797,9 +1798,9 @@ #define YY_EXIT_FAILURE 2 #endif
-static void yynoreturn yy_fatal_error (yyconst char* msg ) +static void yynoreturn yy_fatal_error (const char* msg ) { - (void) fprintf( stderr, "%s\n", msg ); + fprintf( stderr, "%s\n", msg ); exit( YY_EXIT_FAILURE ); }
@@ -1810,7 +1811,7 @@ do \ { \ /* Undo effects of setting up yytext. */ \ - yy_size_t yyless_macro_arg = (n); \ + int yyless_macro_arg = (n); \ YY_LESS_LINENO(yyless_macro_arg);\ yytext[yyleng] = (yy_hold_char); \ (yy_c_buf_p) = yytext + yyless_macro_arg; \ @@ -1934,7 +1935,7 @@
/* Pop the buffer stack, destroying each element. */ while(YY_CURRENT_BUFFER){ - yy_delete_buffer(YY_CURRENT_BUFFER ); + yy_delete_buffer( YY_CURRENT_BUFFER ); YY_CURRENT_BUFFER_LVALUE = NULL; yypop_buffer_state(); } @@ -1955,7 +1956,7 @@ */
#ifndef yytext_ptr -static void yy_flex_strncpy (char* s1, yyconst char * s2, int n ) +static void yy_flex_strncpy (char* s1, const char * s2, int n ) { int i; @@ -1965,7 +1966,7 @@ #endif
#ifdef YY_NEED_STRLEN -static int yy_flex_strlen (yyconst char * s ) +static int yy_flex_strlen (const char * s ) { int n; for ( n = 0; s[n]; ++n ) diff --git a/util/sconfig/main.c b/util/sconfig/main.c index 3b60e2a..d784642 100644 --- a/util/sconfig/main.c +++ b/util/sconfig/main.c @@ -516,6 +516,7 @@
new_d->enabled = status & 0x01; new_d->hidden = (status >> 1) & 0x01; + new_d->mandatory = (status >> 2) & 0x01; new_d->chip_instance = chip_instance; chip_instance->ref_count++;
@@ -810,6 +811,7 @@ fprintf(fil, "},\n"); fprintf(fil, "\t.enabled = %d,\n", ptr->enabled); fprintf(fil, "\t.hidden = %d,\n", ptr->hidden); + fprintf(fil, "\t.mandatory = %d,\n", ptr->mandatory); fprintf(fil, "\t.on_mainboard = 1,\n"); if (ptr->subsystem_vendor > 0) fprintf(fil, "\t.subsystem_vendor = 0x%04x,\n", diff --git a/util/sconfig/sconfig.h b/util/sconfig/sconfig.h index eea2a14..60842f1 100644 --- a/util/sconfig/sconfig.h +++ b/util/sconfig/sconfig.h @@ -104,6 +104,8 @@ /* Indicates device status (enabled / hidden or not). */ int enabled; int hidden; + /* non-zero if the device should be included in all cases */ + int mandatory;
/* Subsystem IDs for the device. */ int subsystem_vendor; diff --git a/util/sconfig/sconfig.l b/util/sconfig/sconfig.l index 87de6e2..14eb965 100755 --- a/util/sconfig/sconfig.l +++ b/util/sconfig/sconfig.l @@ -29,7 +29,8 @@ register {return(REGISTER);} on {yylval.number=1; return(BOOL);} off {yylval.number=0; return(BOOL);} -hidden {yylval.number=3; return(HIDDEN);} +hidden {yylval.number=3; return(STATUS);} +mandatory {yylval.number=5; return(STATUS);} pci {yylval.number=PCI; return(BUS);} ioapic {yylval.number=IOAPIC; return(BUS);} pnp {yylval.number=PNP; return(BUS);} diff --git a/util/sconfig/sconfig.tab.c_shipped b/util/sconfig/sconfig.tab.c_shipped index 8e1e57d..f4335c7 100644 --- a/util/sconfig/sconfig.tab.c_shipped +++ b/util/sconfig/sconfig.tab.c_shipped @@ -1,8 +1,8 @@ -/* A Bison parser, made by GNU Bison 3.0.5. */ +/* A Bison parser, made by GNU Bison 3.0.4. */
/* Bison implementation for Yacc-like parsers in C
- Copyright (C) 1984, 1989-1990, 2000-2015, 2018 Free Software Foundation, Inc. + Copyright (C) 1984, 1989-1990, 2000-2015 Free Software Foundation, Inc.
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 @@ -44,7 +44,7 @@ #define YYBISON 1
/* Bison version. */ -#define YYBISON_VERSION "3.0.5" +#define YYBISON_VERSION "3.0.4"
/* Skeleton name. */ #define YYSKELETON_NAME "yacc.c" @@ -109,8 +109,8 @@
/* In a future release of Bison, this section will be replaced by #include "sconfig.tab.h_shipped". */ -#ifndef YY_YY_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED -# define YY_YY_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED +#ifndef YY_YY_HOME_RMINNICH_PROJECTS_LINUXBOOT_COREBOOTNERF_GITHUBCOREBOOT_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED +# define YY_YY_HOME_RMINNICH_PROJECTS_LINUXBOOT_COREBOOTNERF_GITHUBCOREBOOT_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED /* Debug traces. */ #ifndef YYDEBUG # define YYDEBUG 0 @@ -128,34 +128,35 @@ DEVICE = 259, REGISTER = 260, BOOL = 261, - HIDDEN = 262, - BUS = 263, - RESOURCE = 264, - END = 265, - EQUALS = 266, - HEX = 267, - STRING = 268, - PCI = 269, - PNP = 270, - I2C = 271, - APIC = 272, - CPU_CLUSTER = 273, - CPU = 274, - DOMAIN = 275, - IRQ = 276, - DRQ = 277, - SLOT_DESC = 278, - IO = 279, - NUMBER = 280, - SUBSYSTEMID = 281, - INHERIT = 282, - IOAPIC_IRQ = 283, - IOAPIC = 284, - PCIINT = 285, - GENERIC = 286, - SPI = 287, - USB = 288, - MMIO = 289 + STATUS = 262, + MANDATORY = 263, + BUS = 264, + RESOURCE = 265, + END = 266, + EQUALS = 267, + HEX = 268, + STRING = 269, + PCI = 270, + PNP = 271, + I2C = 272, + APIC = 273, + CPU_CLUSTER = 274, + CPU = 275, + DOMAIN = 276, + IRQ = 277, + DRQ = 278, + SLOT_DESC = 279, + IO = 280, + NUMBER = 281, + SUBSYSTEMID = 282, + INHERIT = 283, + IOAPIC_IRQ = 284, + IOAPIC = 285, + PCIINT = 286, + GENERIC = 287, + SPI = 288, + USB = 289, + MMIO = 290 }; #endif
@@ -184,7 +185,7 @@
int yyparse (void);
-#endif /* !YY_YY_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED */ +#endif /* !YY_YY_HOME_RMINNICH_PROJECTS_LINUXBOOT_COREBOOTNERF_GITHUBCOREBOOT_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED */
/* Copy the second part of user declarations. */
@@ -430,10 +431,10 @@ /* YYFINAL -- State number of the termination state. */ #define YYFINAL 3 /* YYLAST -- Last index in YYTABLE. */ -#define YYLAST 43 +#define YYLAST 40
/* YYNTOKENS -- Number of terminals. */ -#define YYNTOKENS 35 +#define YYNTOKENS 36 /* YYNNTS -- Number of nonterminals. */ #define YYNNTS 15 /* YYNRULES -- Number of rules. */ @@ -444,7 +445,7 @@ /* YYTRANSLATE[YYX] -- Symbol number corresponding to YYX as returned by yylex, with out-of-bounds checking. */ #define YYUNDEFTOK 2 -#define YYMAXUTOK 289 +#define YYMAXUTOK 290
#define YYTRANSLATE(YYX) \ ((unsigned int) (YYX) <= YYMAXUTOK ? yytranslate[YYX] : YYUNDEFTOK) @@ -481,7 +482,8 @@ 2, 2, 2, 2, 2, 2, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, - 25, 26, 27, 28, 29, 30, 31, 32, 33, 34 + 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, + 35 };
#if YYDEBUG @@ -500,12 +502,12 @@ static const char *const yytname[] = { "$end", "error", "$undefined", "CHIP", "DEVICE", "REGISTER", "BOOL", - "HIDDEN", "BUS", "RESOURCE", "END", "EQUALS", "HEX", "STRING", "PCI", - "PNP", "I2C", "APIC", "CPU_CLUSTER", "CPU", "DOMAIN", "IRQ", "DRQ", - "SLOT_DESC", "IO", "NUMBER", "SUBSYSTEMID", "INHERIT", "IOAPIC_IRQ", - "IOAPIC", "PCIINT", "GENERIC", "SPI", "USB", "MMIO", "$accept", - "devtree", "$@1", "chipchildren", "devicechildren", "chip", "@2", - "device", "@3", "status", "resource", "registers", "subsystemid", + "STATUS", "MANDATORY", "BUS", "RESOURCE", "END", "EQUALS", "HEX", + "STRING", "PCI", "PNP", "I2C", "APIC", "CPU_CLUSTER", "CPU", "DOMAIN", + "IRQ", "DRQ", "SLOT_DESC", "IO", "NUMBER", "SUBSYSTEMID", "INHERIT", + "IOAPIC_IRQ", "IOAPIC", "PCIINT", "GENERIC", "SPI", "USB", "MMIO", + "$accept", "devtree", "$@1", "chipchildren", "devicechildren", "chip", + "@2", "device", "@3", "status", "resource", "registers", "subsystemid", "ioapic_irq", "smbios_slot_desc", YY_NULLPTR }; #endif @@ -518,7 +520,7 @@ 0, 256, 257, 258, 259, 260, 261, 262, 263, 264, 265, 266, 267, 268, 269, 270, 271, 272, 273, 274, 275, 276, 277, 278, 279, 280, 281, 282, 283, 284, - 285, 286, 287, 288, 289 + 285, 286, 287, 288, 289, 290 }; # endif
@@ -536,11 +538,11 @@ STATE-NUM. */ static const yytype_int8 yypact[] = { - -12, 11, 9, -12, 1, -12, -12, -12, 0, 5, - 3, -12, -12, -12, -12, -10, 6, 2, 8, -12, - -12, -12, -12, -12, -3, -1, -12, 13, 4, 7, - -12, -12, -12, -12, -12, -12, 16, 15, 10, -11, - 12, 17, -5, 14, -12, 18, -12, -12, -12 + -12, 6, 9, -12, -1, -12, -12, -12, 0, 5, + 1, -12, -12, -12, -12, -10, 7, 3, 8, -12, + -12, -12, -12, -12, -3, -9, -12, 11, 2, 4, + -12, -12, -12, -12, -12, -12, 15, 17, 10, -11, + 12, 18, -5, 13, -12, 19, -12, -12, -12 };
/* YYDEFACT[STATE-NUM] -- Default reduction number in state STATE-NUM. @@ -558,7 +560,7 @@ /* YYPGOTO[NTERM-NUM]. */ static const yytype_int8 yypgoto[] = { - -12, -12, -12, -12, -12, -6, -12, 19, -12, -12, + -12, -12, -12, -12, -12, -6, -12, 16, -12, -12, -12, -12, -12, -12, -12 };
@@ -574,39 +576,39 @@ number is the opposite. If YYTABLE_NINF, syntax error. */ static const yytype_uint8 yytable[] = { - 4, 9, 12, 4, 9, 10, 25, 26, 19, 20, - 11, 3, 4, 15, 6, 17, 16, 18, 30, 43, - 27, 22, 46, 28, 36, 29, 37, 40, 41, 38, - 45, 48, 39, 0, 0, 42, 0, 44, 0, 47, - 0, 0, 0, 31 + 4, 9, 12, 4, 9, 10, 3, 25, 26, 19, + 20, 11, 4, 6, 15, 16, 17, 36, 30, 18, + 43, 27, 22, 46, 28, 37, 29, 40, 38, 0, + 39, 41, 45, 48, 0, 0, 42, 0, 44, 47, + 31 };
static const yytype_int8 yycheck[] = { - 3, 4, 8, 3, 4, 5, 9, 10, 6, 7, - 10, 0, 3, 8, 13, 25, 13, 11, 24, 30, - 23, 13, 27, 26, 25, 28, 13, 11, 13, 25, - 13, 13, 25, -1, -1, 25, -1, 25, -1, 25, - -1, -1, -1, 24 + 3, 4, 8, 3, 4, 5, 0, 10, 11, 6, + 7, 11, 3, 14, 9, 14, 26, 26, 24, 12, + 31, 24, 14, 28, 27, 14, 29, 12, 26, -1, + 26, 14, 14, 14, -1, -1, 26, -1, 26, 26, + 24 };
/* YYSTOS[STATE-NUM] -- The (internal number of the) accessing symbol of state STATE-NUM. */ static const yytype_uint8 yystos[] = { - 0, 36, 37, 0, 3, 40, 13, 41, 38, 4, - 5, 10, 40, 42, 46, 8, 13, 25, 11, 6, - 7, 44, 13, 43, 39, 9, 10, 23, 26, 28, - 40, 42, 45, 47, 48, 49, 25, 13, 25, 25, - 11, 13, 25, 30, 25, 13, 27, 25, 13 + 0, 37, 38, 0, 3, 41, 14, 42, 39, 4, + 5, 11, 41, 43, 47, 9, 14, 26, 12, 6, + 7, 45, 14, 44, 40, 10, 11, 24, 27, 29, + 41, 43, 46, 48, 49, 50, 26, 14, 26, 26, + 12, 14, 26, 31, 26, 14, 28, 26, 14 };
/* YYR1[YYN] -- Symbol number of symbol that rule YYN derives. */ static const yytype_uint8 yyr1[] = { - 0, 35, 37, 36, 38, 38, 38, 38, 39, 39, - 39, 39, 39, 39, 39, 41, 40, 43, 42, 44, - 44, 45, 46, 47, 47, 48, 49, 49, 49 + 0, 36, 38, 37, 39, 39, 39, 39, 40, 40, + 40, 40, 40, 40, 40, 42, 41, 44, 43, 45, + 45, 46, 47, 48, 48, 49, 50, 50, 50 };
/* YYR2[YYN] -- Number of symbols on the right hand side of rule YYN. */ @@ -975,7 +977,6 @@ case N: \ yyformat = S; \ break - default: /* Avoid compiler warnings. */ YYCASE_(0, YY_("syntax error")); YYCASE_(1, YY_("syntax error, unexpected %s")); YYCASE_(2, YY_("syntax error, unexpected %s, expecting %s")); diff --git a/util/sconfig/sconfig.tab.h_shipped b/util/sconfig/sconfig.tab.h_shipped index bcbd644..272f651 100644 --- a/util/sconfig/sconfig.tab.h_shipped +++ b/util/sconfig/sconfig.tab.h_shipped @@ -1,8 +1,8 @@ -/* A Bison parser, made by GNU Bison 3.0.5. */ +/* A Bison parser, made by GNU Bison 3.0.4. */
/* Bison interface for Yacc-like parsers in C
- Copyright (C) 1984, 1989-1990, 2000-2015, 2018 Free Software Foundation, Inc. + Copyright (C) 1984, 1989-1990, 2000-2015 Free Software Foundation, Inc.
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 @@ -30,8 +30,8 @@ This special exception was added by the Free Software Foundation in version 2.2 of Bison. */
-#ifndef YY_YY_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED -# define YY_YY_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED +#ifndef YY_YY_HOME_RMINNICH_PROJECTS_LINUXBOOT_COREBOOTNERF_GITHUBCOREBOOT_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED +# define YY_YY_HOME_RMINNICH_PROJECTS_LINUXBOOT_COREBOOTNERF_GITHUBCOREBOOT_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED /* Debug traces. */ #ifndef YYDEBUG # define YYDEBUG 0 @@ -49,34 +49,35 @@ DEVICE = 259, REGISTER = 260, BOOL = 261, - HIDDEN = 262, - BUS = 263, - RESOURCE = 264, - END = 265, - EQUALS = 266, - HEX = 267, - STRING = 268, - PCI = 269, - PNP = 270, - I2C = 271, - APIC = 272, - CPU_CLUSTER = 273, - CPU = 274, - DOMAIN = 275, - IRQ = 276, - DRQ = 277, - SLOT_DESC = 278, - IO = 279, - NUMBER = 280, - SUBSYSTEMID = 281, - INHERIT = 282, - IOAPIC_IRQ = 283, - IOAPIC = 284, - PCIINT = 285, - GENERIC = 286, - SPI = 287, - USB = 288, - MMIO = 289 + STATUS = 262, + MANDATORY = 263, + BUS = 264, + RESOURCE = 265, + END = 266, + EQUALS = 267, + HEX = 268, + STRING = 269, + PCI = 270, + PNP = 271, + I2C = 272, + APIC = 273, + CPU_CLUSTER = 274, + CPU = 275, + DOMAIN = 276, + IRQ = 277, + DRQ = 278, + SLOT_DESC = 279, + IO = 280, + NUMBER = 281, + SUBSYSTEMID = 282, + INHERIT = 283, + IOAPIC_IRQ = 284, + IOAPIC = 285, + PCIINT = 286, + GENERIC = 287, + SPI = 288, + USB = 289, + MMIO = 290 }; #endif
@@ -105,4 +106,4 @@
int yyparse (void);
-#endif /* !YY_YY_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED */ +#endif /* !YY_YY_HOME_RMINNICH_PROJECTS_LINUXBOOT_COREBOOTNERF_GITHUBCOREBOOT_UTIL_SCONFIG_SCONFIG_TAB_H_SHIPPED_INCLUDED */ diff --git a/util/sconfig/sconfig.y b/util/sconfig/sconfig.y index 0d894a9..d55b18b 100755 --- a/util/sconfig/sconfig.y +++ b/util/sconfig/sconfig.y @@ -31,7 +31,7 @@ int number; }
-%token CHIP DEVICE REGISTER BOOL HIDDEN BUS RESOURCE END EQUALS HEX STRING PCI PNP I2C APIC CPU_CLUSTER CPU DOMAIN IRQ DRQ SLOT_DESC IO NUMBER SUBSYSTEMID INHERIT IOAPIC_IRQ IOAPIC PCIINT GENERIC SPI USB MMIO +%token CHIP DEVICE REGISTER BOOL STATUS MANDATORY BUS RESOURCE END EQUALS HEX STRING PCI PNP I2C APIC CPU_CLUSTER CPU DOMAIN IRQ DRQ SLOT_DESC IO NUMBER SUBSYSTEMID INHERIT IOAPIC_IRQ IOAPIC PCIINT GENERIC SPI USB MMIO %% devtree: { cur_parent = root_parent; } chip;
@@ -56,7 +56,7 @@ cur_parent = $<dev>5->parent; };
-status: BOOL | HIDDEN; +status: BOOL | STATUS ;
resource: RESOURCE NUMBER /* == resnum */ EQUALS NUMBER /* == resval */ { add_resource(cur_parent, $<number>1, strtol($<string>2, NULL, 0), strtol($<string>4, NULL, 0)); } ;
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 18:
Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3 Emulation targets: EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/505 EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/504 EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/503
Please note: This test is under development and might not be accurate at all!
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 18:
After reading the review comments, I am quite confused. Wasn’t a change of the Kconfig option requested, and patch set #13 preferred?
Could you please update the commit message the next time with the reasoning against the other comments?
Also, it’d be great, if this new feature and plans would be announced on the mailing list, and the 4.12 release notes be updated.
-
9elements QA (Code Review) -
Aaron Durbin (Code Review)
-
build bot (Jenkins) (Code Review)
-
Jeremy Soller (Code Review)
-
Kyösti Mälkki (Code Review)
-
Lean Sheng Tan (Code Review)
-
Martin Roth (Code Review)
-
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Philipp Deppenwiese (Code Review)
-
ron minnich (Code Review)
-
Subrata Banik (Code Review)