Re: [Qemu-devel] [PATCH RFC 0/3] seabios: move acpi table formatting out of bios

[Gleb Natapov <gleb@redhat.com>]

On Wed, May 08, 2013 at 01:43:25PM +0300, Michael S. Tsirkin wrote:
> On Wed, May 08, 2013 at 01:34:59PM +0300, Gleb Natapov wrote:
> > On Wed, May 08, 2013 at 01:29:12PM +0300, Michael S. Tsirkin wrote:
> > > On Wed, May 08, 2013 at 12:31:50PM +0300, Gleb Natapov wrote:
> > > > On Tue, May 07, 2013 at 07:01:13PM -0400, Kevin O'Connor wrote:
> > > > > On Tue, May 07, 2013 at 09:00:48PM +0300, Michael S. Tsirkin wrote:
> > > > > > On Thu, Apr 25, 2013 at 12:02:20PM +0300, Michael S. Tsirkin wrote:
> > > > > > > Untested yet, but I thought I'd share the
> > > > > > > BIOS bits so we can agree on direction.
> > > > > > > 
> > > > > > > In particular check out ROM sizes:
> > > > > > > - Before patchset with DSDT enabled
> > > > > > >     Total size: 127880  Fixed: 59060  Free: 3192 (used 97.6% of 128KiB rom)
> > > > > > > - Before patchset with DSDT disabled
> > > > > > >     Total size: 122844  Fixed: 58884  Free: 8228 (used 93.7% of 128KiB rom)
> > > > > > > - After patchset:
> > > > > > >     Total size: 128776  Fixed: 59100  Free: 2296 (used 98.2% of 128KiB rom)
> > > > > > > - Legacy disabled at build time:
> > > > > > >     Total size: 119836  Fixed: 58996  Free: 11236 (used 91.4% of 128KiB rom)
> > > > > > > 
> > > > > > > As can be seen from this, most size savings come
> > > > > > > from dropping DSDT, but we do save a bit by removing
> > > > > > > other tables. Of course the real reason to move tables to QEMU
> > > > > > > is so that ACPI can better match hardware.
> > > > > > > 
> > > > > > > This patchset adds an option to move all code for formatting acpi tables
> > > > > > > out of BIOS. With this, QEMU has full control over the table layout.
> > > > > > > All tables are loaded from the new "/etc/acpi/" directory.
> > > > > > > Any entries in this directory cause BIOS to disable
> > > > > > > ACPI table generation completely.
> > > > > > > A generic linker script, controlled by QEMU, is
> > > > > > > loaded from "/etc/linker-script". It is used to
> > > > > > > patch in table pointers and checksums.
> > > > > > 
> > > > > > After some thought, there are two additional
> > > > > > options worth considering, in that they simplify
> > > > > > bios code somewhat:
> > > > > > 
> > > > > > - bios could get size from qemu, allocate a buffer
> > > > > >   (e.g. could be one buffer for all tables)
> > > > > >   and pass the address to qemu.
> > > > > >   qemu does all the patching
> > > > > > 
> > > > > > - further, qemu could do the copy of tables into
> > > > > >   that address directly
> > > > > 
> > > > > This seems more complex than necessary to me.
> > > > > 
> > > > > The important task is to get the tables generated in QEMU - I'd focus
> > > > > on getting the tables generated in QEMU (one table per fw_cfg "file").
> > > > > Once that is done, the SeaBIOS side can be easily implemented, and we
> > > > > can add any enhancements on top if we feel it is necessary.
> > > > > 
> > > > +1. This "copy of tables into that address directly" is just an ad-hoc PV
> > > > isa DMA device in disguise. Such device was refused when libguestfs
> > > > asked for it, and they wanted it for much better reason - performance.
> > > > There is existing mechanism to pass data into firmware. Use it please.
> > > 
> > > Yes I can code it up using FW_CFG for now.
> > > 
> > > One issue with QEMU_CFG_FILE_DIR is that it's broken wrt migration,
> > > unless we pass in very small bits of data which we
> > > can guarantee never changes across qemu versions.
> > > 
> > Shouldn't we guaranty that ACPI tables do not change for the same
> > machine type anyway?
> 
> That's not practical. They are too big to stay completely unchanged.
> 
I will not be surprised if this will cause us problem somehow. Guest
will see new tables only after reboot/resume from S4 so damage is
limited, but one thing that comes to mind is table's size change. If
they grow from one version to the other after resuming a guest from S4
on new QEMU version part of the tables may be corrupted.

> > > Off-list, I suggested fixing it and migrating file
> > > content, but Anthony thinks it's a bad idea.
> > > 
> > Why is this a bad idea to fix device migration?
> 
> You misunderstand I think.
> Question is whether we should be putting so much info in fw_cfg.
> If we keep fw_cfg for small things we don't need to
> migrate it. In that case ACPI tables have to be passed in
> using some other mechanism.
> 
Where this notion that fw_cfg is only for a small things is coming
from? I can assure you this was not the case when the device was
introduced. In fact it is used today for not so small things like
bootindex splash screen bitmaps, option rom loading and kernel/initrd
loading. Some of those are bigger then ACPI tables will ever be.
And they all should be migrated, so fw_cfg should be fixed anyway.

--
			Gleb.