From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=41412 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OONxY-0005zM-2S for qemu-devel@nongnu.org; Tue, 15 Jun 2010 00:41:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OONxW-0003qm-Vp for qemu-devel@nongnu.org; Tue, 15 Jun 2010 00:41:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:26341) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OONxW-0003qf-No for qemu-devel@nongnu.org; Tue, 15 Jun 2010 00:41:10 -0400 Message-ID: <4C17045E.6000203@redhat.com> Date: Tue, 15 Jun 2010 07:41:02 +0300 From: Avi Kivity MIME-Version: 1.0 References: <20100614083053.GC21797@redhat.com> <20100614135425.GA18002@morn.localdomain> <20100614140959.GI21797@redhat.com> <4C1641EF.9070001@redhat.com> <20100614182521.GA22454@morn.localdomain> In-Reply-To: <20100614182521.GA22454@morn.localdomain> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [SeaBIOS] [PATCHv2] load hpet info for HPET ACPI table from qemu List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin O'Connor Cc: seabios@seabios.org, qemu-devel@nongnu.org, Gleb Natapov On 06/14/2010 09:25 PM, Kevin O'Connor wrote: > On Mon, Jun 14, 2010 at 05:51:27PM +0300, Avi Kivity wrote: > >> On 06/14/2010 05:09 PM, Gleb Natapov wrote: >> >>>> Could we just have qemu build the hpet tables and pass them through to >>>> seabios? Perhaps using the qemu_cfg_acpi_additional_tables() method. >>>> >>>> >>> Possible, and I considered that. I personally prefer to pass minimum >>> information required for seabios to discover underlying HW and leave >>> ACPI table creation to seabios. That is how things done for HW that >>> seabios can actually detect. If we will go your way pretty soon we will >>> move creation of ACPI/SMBIOS/MP tables into qemu and IMHO this will be >>> step backworkds. >>> >> I agree. ACPI is a firmware/OS interface. If we move ACPI table >> generation into qemu, it becomes a mixed hardware/firmware/OS >> interface. >> > This seems to be a philosophical distinction. Lets go over the > practical implications. > In my experience, well-defined interfaces ("philosophical distinctions") are more important in the long term than practicalities. The practicalities change, but confusion over incorrect interfaces, or problems when wrong interfaces are used, are forever. > It seems there was a change in qemu to the hpet functionality. > Although the change is solely between qemu and the OS, it's necessary > to patch both qemu and seabios for the OS to see the change. This > means creating and reviewing patches for two separate repos. This > also requires release coordination - the seabios change has to be > committed and released, and then qemu needs to be released with the > new seabios. Additional changes in seabios tip will get merged into > qemu, which could complicate testing. > > If a table needs to refer to some other information which is in a table that is generated by seabios, we cannot generate this table from qemu. That's much worse that reviewing and applying two patches. >> Better keep those interfaces separate: hardware/firmware (fwcfg) and >> firmware/OS (acpi). >> > One could look at the current hpet patch as implementing: > qemu -> struct hpet_fw_entry -> seabios -> struct acpi_20_hpet -> OS. > > I'm suggesting that we do the following instead: > qemu -> struct acpi_20_hpet -> seabios -> struct acpi_20_hpet -> OS. > > I'm not suggesting a radical rethink of fwcfg, but I fail to see the > advantage in introducing the arbitrary "struct hpet_fw_entry" when > there is a perfectly good, well defined, "struct acpi_20_hpet" that > already exists. This new arbitrary intermediate format just > introduces "make work" for all of us. > Choosing an existing format is fine. But seabios blindly copying qemu provided data is wrong IMO. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.