Hi, On Mon, Nov 04, 2013 at 01:17:10PM +0100, Gerd Hoffmann wrote: > Unlike the existing FW_CFG_E820_TABLE entry which carries reservations > only the new etc/e820 file also has entries for RAM.
Acked, it looks the best the way to go if the objective is to keep backwards compatibility with older seabios protocol. I have to trust you on why we need to stay backwards compatible at all times and why we can't commit an updated bios.bin before a new seabios stable release happened. Otherwise we could have just fixed FW_CFG_E820_TABLE breaking backwards compatibility without introducing a partially overlapping fw_cfg. About the file, I wonder what happens if too many people starts to use files and we'll run out of FW_CFG_FILE_SLOTS at runtime (assert(index < FW_CFG_FILE_SLOTS);). To me seeing the list of all fw_cfg IDs in a header file to define all possible paravirt APIs seabios need to know about, looked not so bad, but then grepping for fw_cfg_add_file is equivalent. The main issue is that if we use files from now, it would be nicer not to limit those to FW_CFG_FILE_SLOTS but to allocate them a bit more dynamically without asserts but this is offtopic (I just happened to read how files are created to review this patch). Probably one patch could be added to s/FW_CFG_E820_TABLE/FW_CFG_E820_TABLE_OLD/, otherwise if somebody read fw_cfg.h, it won't be apparent that the grepping shouldn't stop there to reach the real e820 map. Thanks! Andrea
