From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38515) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vdkjc-0000Oz-Lr for qemu-devel@nongnu.org; Tue, 05 Nov 2013 12:48:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VdkjW-0006QW-Fm for qemu-devel@nongnu.org; Tue, 05 Nov 2013 12:48:12 -0500 Received: from mx1.redhat.com ([209.132.183.28]:10210) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VdkjW-0006QM-7w for qemu-devel@nongnu.org; Tue, 05 Nov 2013 12:48:06 -0500 Date: Tue, 5 Nov 2013 18:48:01 +0100 From: Andrea Arcangeli Message-ID: <20131105174801.GG3835@redhat.com> References: <1383567431-13540-1-git-send-email-kraxel@redhat.com> <1383567431-13540-2-git-send-email-kraxel@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1383567431-13540-2-git-send-email-kraxel@redhat.com> Subject: Re: [Qemu-devel] [PATCH 1/2] pc: add etc/e820 fw_cfg file List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: qemu-devel@nongnu.org, Anthony Liguori 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