On Wed, Jun 16, 2010 at 08:55:05PM -0400, Kevin O'Connor wrote:
On Tue, Jun 15, 2010 at 07:41:02AM +0300, Avi Kivity wrote:
On 06/14/2010 09:25 PM, Kevin O'Connor wrote:
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.
I agree with the importance of clean interfaces. However, I feel the current approach to acpi table handling between qemu and seabios is not ideal. I'm proposing an interface which I believe is an improvement.
What is it? If you mean passing ACPI tables, then this is huge step backwards. Qemu started like that and then Fabrice moved everything to BIOS where it should be. Check these commit in qemu git: 2146e8389f267a5fb751106b0dfc6421808ccbd0 362ee297a6a977801758302c68b4ef5d1af76ab3 a0910fa4a5371c2cc70cb9f63ccc65b66decbb22 71b300df920a39db4368723765eed533f5fc209b
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.
I understand. Such tables would not make sense to generate in qemu.
So now we generate part of the tables in qemu and part in seabios. This is not sane.
I also understand and appreciate the desire for qemu to not touch guest memory. This means rsdp and rsdt are the domain of seabios. The only other table that directly addresses the memory location of another table (that I know of) is fadt - which is also tied to seabios' smi handler - so this too is in seabios' domain.
You see now you are starting to rationalize about which tables should be generated by bios and which are not and the fact is that ACPI was designed with assumption that firmware generates tables, so how can you be sure that you will not find flaws in your rationalization later on?
I'm not aware of any dependencies to seabios in any of the other tables (eg, madt, srat, hpet, ssdt, dsdt).
With certain HW configs firmware may need to configure hpet to function in legacy mode (replacing RTC/PIT) so if seabios is unaware of hpet no doesn't mean it will stay that way.
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.
Okay. For "struct acpi_20_hpet", what transformations or checks do you think seabios needs to perform?
Lets concentrate on principles and not hpet in particular. Just because proposed interface is similar to how HPET ACPI table looks doesn't mean we should build ACPI tables in qemu. I could have been lazy and pass only "has hpet" flag to the seabios, but I tried to make interface general to cover future qemu enhancements.
But lets get back to principles. Since qemu is not one platform but very dynamic system and firmware is tightly coupled with the platform it runs on and we do not want to have separate firmware for each possible configuration the only solution is to make firmware to be dynamically adoptable to platform it runs on. For that every bit of information about underlying HW should be discoverable by firmware. Not all HW was designed to be discoverable by software, so we need to create a channel between qemu and firmware to pass information about otherwise undiscoverable devices. Hpet is only one of such devices, so we need a way to pass platform device tree into firmware. We can find generic way to do this for all devices, or we may introduce separate interface for each device when needed like propose patch does.
-- Gleb.