On Tue, 2011-05-24 at 01:17 +0100, Kevin O'Connor wrote:
On Mon, May 23, 2011 at 11:44:38AM +0100, Ian Campbell wrote:
On Tue, 2011-05-17 at 16:59 +0100, Ian Campbell wrote:
The following implements all your feedback (I hope). I have squashed it down into a single commit which supports direct boot.
BTW, in general, the above patch looks okay to me.
I've gotten all the PCI setup and ACPI stuff etc etc working but, frankly, the patch to SeaBIOS is getting to be pretty enormous and intrusive.
Is that due to incompleteness / innacuracies in the current SeaBIOS code, or due to requirements specific to Xen?
A mixture of both but mainly requirements specific to Xen, I think.
PCI setup is one of the main things, the interrupt routing in particular is different which has knock on effects on chipset setup (e.g. legacy PCI ISA IRQ routing) and the BIOS tables (e.g. ACPI _PRT entries). This strikes me as being highly "mainboard" specific, IOW the stuff I would expect to find in coreboot rather than SeaBIOS.
Another difference is that the existing hvmloader tables are ACPI 2.0 while SeaBIOS only creates 1.0 tables, I'm not sure that strictly speaking counts as a incompleteness in SeaBIOS since it's not obvious that Xen actually needs/uses any of the functionality of 2.0. Also it might be something SeaBIOS would like to grow in any case.
If you have test code, I'd be curious to see a patch on the mailing list - it may help later to understand the use cases for SeaBIOS.
It's in a pretty embarrassing state at the moment, but OK. Please note that I was nowhere near suggesting this patch was in any way suitable to be applied or even near to becoming an RFC!
I initially took a rather gung-ho approach in the interests of matching as closely as possible how hvmloader+rombios sets things up, rather than evaluating the actual useful meaning of the differences. Having made things match my intention was to go back and evaluate each change and attempt to get rid of it unless there was an extremely pressing reason not to, but I didn't reach that phase yet. So I think this is an upper bound (possibly by a large-ish order of magnitude) on the level of change.
I hardcoded a few things here and there too (e.g. numbers of CPUs), in the interests of getting something working before going back and cleaning it up.
The diff (vs. the patches I already posted) is attached, it's diffstat is:
src/Kconfig | 34 + src/acpi-dsdt.dsl | 9482 +++++++++++++++++++++++++++++++++++++++++++++----- src/acpi-dsdt.hex | 9331 +++++++++++++++++++++++++++++++++++++++++++------- src/acpi.c | 152 +- src/acpi.h | 93 + src/config.h | 6 +- src/dev-i440fx.c | 16 +- src/pciinit.c | 6 +- src/pmm.c | 5 +- src/post.c | 5 + src/util.h | 5 + src/xen.c | 128 +- src/xen.h | 3 + src/xen/hvm/hvm_op.h | 245 ++ src/xen/hvm/params.h | 145 + src/xen/version.h | 94 + src/xen/xen.h | 739 ++++ src/xen_hypercall.h | 187 + 18 files changed, 18591 insertions(+), 2085 deletions(-)
The DSDT change rather dominates, dropping it gives a more sensible: 11 files changed, 414 insertions(+), 39 deletions(-)
I just ripped the DSDT out of hvmloader, I'm sure with analysis the changes could be reduced to something much more sane, although I have a feeling it would still end up as a subtly different variant of the table (_PRT and such).
The more I look at it the more I am coming to the conclusion that it would be better to have hvmloader setup all this platform level stuff and pass details onto SeaBIOS as necessary, following the model used with coreboot->SeaBIOS rather than the emulator's way of doing things. hvmloader basically already fulfils the same role for Xen HVM guests as coreboot does for physical hardware so I think that makes a certain amount of sense.
I'm okay with that approach as well
The SeaBIOS patch for this approach seems likely to be an awful lot smaller. Basically most "if (COREBOOT)" become effectively "if (COREBOOT||XEN)" + a Xen equivalent to coreboot_copy_biostable and coreboot_fill_map.
- there are pros and cons to each method.
Apart from avoiding bloating SeaBIOS with a load of Xen specific stuff (which would be duplicated in hvmloader and also needed by the tianocore firmware variant etc) I think the big thing pulling me towards this approach is the idea that the BIOS tables describe the hardware, and since the "hardware" in this case is defined by Xen the tables ought to be supplied by Xen. I don't know if that's a distinction which people who really know about things at this level actually make though.
(At various points, it's been discussed whether SeaBIOS should generate ACPI tables for coreboot,
Would that involve pulling a bunch of mainboard specific stuff from coreboot into SeaBIOS?
and it has also been discussed if QEmu should just pass in ACPI tables to SeaBIOS..)
That sounds plausible to me, at least based on my current understanding of what I think is going on ;-)
Ian.