Re: [SeaBIOS] [PATCH v2 9/9] seabios: Build the dsdt separately

[Gerd Hoffmann]

On 10/09/12 05:35, Jason Baron wrote:
> From: Jason Baron <[e-mail]>
>
> This builds seabios such that the dsdt tables are no longer built into the
> seabios binary. They must be passed to the seabios via fw_cfg. This saves
> space in the seabios binary for unnecessary dsdt tables.
>
> I suspect that this will make other users of Seabios, besides qemu unhappy,
> but I'm not sure.

I think this should best be submitted / discussed separately from the
q35 patch series.  And dropping the dsdt from seabios will should
actually be the last step.  Loading via fw_cfg works even when seabios
has a builtin dsdt.  So we can start loading the dsdt via fwcfg
unconditionally in qemu without touching seabios.  Once everything is in
place in qemu we can finally zap the builtin dsdt in seabios (or make it
a config option so latest seabios continues to work with older qemu
versions).

cheers,
  Gerd

[PATCH v2 9/9] seabios: Build the dsdt separately

[Jason Baron]

On Tue, Oct 09, 2012 at 09:13:59AM +0200, Gerd Hoffmann wrote:
> On 10/09/12 05:35, Jason Baron wrote:
> > From: Jason Baron <[e-mail]>
> >
> > This builds seabios such that the dsdt tables are no longer built into the
> > seabios binary. They must be passed to the seabios via fw_cfg. This saves
> > space in the seabios binary for unnecessary dsdt tables.
> >
> > I suspect that this will make other users of Seabios, besides qemu unhappy,
> > but I'm not sure.
>
> I think this should best be submitted / discussed separately from the
> q35 patch series.  And dropping the dsdt from seabios will should
> actually be the last step.  Loading via fw_cfg works even when seabios
> has a builtin dsdt.  So we can start loading the dsdt via fwcfg
> unconditionally in qemu without touching seabios.  Once everything is in
> place in qemu we can finally zap the builtin dsdt in seabios (or make it
> a config option so latest seabios continues to work with older qemu
> versions).

Ok, I like this idea. My only hesitation would be that the dsdt table
might in the future include a dependency on being built with seabios, such as
the 'ACPI_EXTRACT_*' stuff. At the moment it can be cleanly separated
out. And this patch enforces that.

If we don't touch seabios here as you've suggested. What steps are
required on the qemu side? To copy the 2 .aml files to its bios dir (Not
too familiar with these details)?

Thanks,

-Jason

[PATCH v2 9/9] seabios: Build the dsdt separately

[Gerd Hoffmann]

Hi,
> Ok, I like this idea. My only hesitation would be that the dsdt table
> might in the future include a dependency on being built with seabios, such as
> the 'ACPI_EXTRACT_*' stuff. At the moment it can be cleanly separated
> out. And this patch enforces that.
> If we don't touch seabios here as you've suggested. What steps are
> required on the qemu side? To copy the 2 .aml files to its bios dir (Not
> too familiar with these details)?

roms/Makefile builds seabios for qemu and copyes bios.bin to pc-bios/.
Needs to be updated to also copy the acpi tables.  acpi tables must be
committed (like pc-bios/bios.bin, this is done for non-x86 hosts which
can't easily build x86 bios btw).  "make install" must be updated to
also install the acpi tables.  pc_piix.c should load them from disk and
stick into fw_cfg.

cheers,
  Gerd

[PATCH v2 9/9] seabios: Build the dsdt separately

[Jason Baron]

On Wed, Oct 10, 2012 at 08:14:46AM +0200, Gerd Hoffmann wrote:
> Hi,
> > Ok, I like this idea. My only hesitation would be that the dsdt table
> > might in the future include a dependency on being built with seabios, such as
> > the 'ACPI_EXTRACT_*' stuff. At the moment it can be cleanly separated
> > out. And this patch enforces that.
> > If we don't touch seabios here as you've suggested. What steps are
> > required on the qemu side? To copy the 2 .aml files to its bios dir (Not
> > too familiar with these details)?
>
> roms/Makefile builds seabios for qemu and copyes bios.bin to pc-bios/.
> Needs to be updated to also copy the acpi tables.  acpi tables must be
> committed (like pc-bios/bios.bin, this is done for non-x86 hosts which
> can't easily build x86 bios btw).  "make install" must be updated to
> also install the acpi tables.  pc_piix.c should load them from disk and
> stick into fw_cfg.
>
> cheers,
>   Gerd

Right, I implemented the patch below for this:

--- a/Makefile
+++ b/Makefile
@@ -275,7 +275,8 @@ bamboo.dtb petalogix-s3adsp1800.dtb petalogix-ml605.dtb \
 multiboot.bin linuxboot.bin kvmvapic.bin \
 s390-zipl.rom \
 spapr-rtas.bin slof.bin \
-palcode-clipper
+palcode-clipper \
+acpi-dsdt.aml q35-acpi-dsdt.aml \
 else
 BLOBS=
 endif
--- a/roms/Makefile
+++ b/roms/Makefile
@@ -12,6 +12,7 @@ bios: config.seabios
 	sh configure-seabios.sh $<
 	make -C seabios out/bios.bin
 	cp seabios/out/bios.bin ../pc-bios/bios.bin
+	cp seabios/out/*dsdt.aml ../pc-bios/
 
 seavgabios: $(patsubst %,seavgabios-%,$(vgabios_variants))
 

Another point of discussion is the ordering of the commits to seabios
vs. qemu. We could commit the above patch now (without the q35 aml, and
with the acpi-dsdt.aml file).

For q35, we could wait for the SeaBIOS bits to go in first, and then add
the q35-acpi-dsdt.aml to pci-bios/. In that way, no special command line
option will ever be required for q35 (except maybe -M pc_q35).

The alternative would be to commit both trees in parallel, and have
users of q35 specify -L <bios dir with q35 dsdt table>, until the
q35-acpi-dsdt.aml is added to qemu.

I could definitely see a strong case for the former option (waiting on
SeaBIOS bits), but wanted to have an agreed on plan for this.

Thanks,

-Jason

[PATCH v2 9/9] seabios: Build the dsdt separately

[Paolo Bonzini]

Il 10/10/2012 16:26, Jason Baron ha scritto:
> Another point of discussion is the ordering of the commits to seabios
> vs. qemu. We could commit the above patch now (without the q35 aml, and
> with the acpi-dsdt.aml file).
>
> For q35, we could wait for the SeaBIOS bits to go in first, and then add
> the q35-acpi-dsdt.aml to pci-bios/. In that way, no special command line
> option will ever be required for q35 (except maybe -M pc_q35).
>
> The alternative would be to commit both trees in parallel, and have
> users of q35 specify -L <bios dir with q35 dsdt table>, until the
> q35-acpi-dsdt.aml is added to qemu.
>
> I could definitely see a strong case for the former option (waiting on
> SeaBIOS bits), but wanted to have an agreed on plan for this.

I don't think it's that important.

Paolo

[PATCH v2 9/9] seabios: Build the dsdt separately

[Michael S. Tsirkin]

On Wed, Oct 10, 2012 at 10:26:51AM -0400, Jason Baron wrote:
> On Wed, Oct 10, 2012 at 08:14:46AM +0200, Gerd Hoffmann wrote:
> > Hi,
> > > Ok, I like this idea. My only hesitation would be that the dsdt table
> > > might in the future include a dependency on being built with seabios, such as
> > > the 'ACPI_EXTRACT_*' stuff. At the moment it can be cleanly separated
> > > out. And this patch enforces that.
> > > If we don't touch seabios here as you've suggested. What steps are
> > > required on the qemu side? To copy the 2 .aml files to its bios dir (Not
> > > too familiar with these details)?
> >
> > roms/Makefile builds seabios for qemu and copyes bios.bin to pc-bios/.
> > Needs to be updated to also copy the acpi tables.  acpi tables must be
> > committed (like pc-bios/bios.bin, this is done for non-x86 hosts which
> > can't easily build x86 bios btw).  "make install" must be updated to
> > also install the acpi tables.  pc_piix.c should load them from disk and
> > stick into fw_cfg.
> >
> > cheers,
> >   Gerd
>
> Right, I implemented the patch below for this:
>
> --- a/Makefile
> +++ b/Makefile
> @@ -275,7 +275,8 @@ bamboo.dtb petalogix-s3adsp1800.dtb petalogix-ml605.dtb \
>  multiboot.bin linuxboot.bin kvmvapic.bin \
>  s390-zipl.rom \
>  spapr-rtas.bin slof.bin \
> -palcode-clipper
> +palcode-clipper \
> +acpi-dsdt.aml q35-acpi-dsdt.aml \
>  else
>  BLOBS=
>  endif
> --- a/roms/Makefile
> +++ b/roms/Makefile
> @@ -12,6 +12,7 @@ bios: config.seabios
>   sh configure-seabios.sh $<
>   make -C seabios out/bios.bin
>   cp seabios/out/bios.bin ../pc-bios/bios.bin
> +	cp seabios/out/*dsdt.aml ../pc-bios/
>
> seavgabios: $(patsubst %,seavgabios-%,$(vgabios_variants))
>
> Another point of discussion is the ordering of the commits to seabios
> vs. qemu. We could commit the above patch now (without the q35 aml, and
> with the acpi-dsdt.aml file).
>
> For q35, we could wait for the SeaBIOS bits to go in first, and then add
> the q35-acpi-dsdt.aml to pci-bios/. In that way, no special command line
> option will ever be required for q35 (except maybe -M pc_q35).
>
> The alternative would be to commit both trees in parallel, and have
> users of q35 specify -L <bios dir with q35 dsdt table>, until the
> q35-acpi-dsdt.aml is added to qemu.
>
> I could definitely see a strong case for the former option (waiting on
> SeaBIOS bits), but wanted to have an agreed on plan for this.
>
> Thanks,
>
> -Jason

I think -L is OK too. q35 is still new, we are likely to
need some revisions while bugs are ironed out anyway.

-- 
MST

[PATCH v2 9/9] seabios: Build the dsdt separately

[Gerd Hoffmann]

Hi,
> Another point of discussion is the ordering of the commits to seabios
> vs. qemu. We could commit the above patch now (without the q35 aml, and
> with the acpi-dsdt.aml file).

Yes, that can go in separately.  Also the load-dsdt patch for piix.  And
run "make -C roms bios" once and "git add pc-bios/acpi-dsdt.aml".
> For q35, we could wait for the SeaBIOS bits to go in first, and then add
> the q35-acpi-dsdt.aml to pci-bios/. In that way, no special command line
> option will ever be required for q35 (except maybe -M pc_q35).

Once all q35 bits are in seabios we should update the seabios submodule
in qemu, again run "make -C roms bios" and commit the bios update
(including the new q35-acpi-dsdt.aml file).

commit 01afdadc92e71e29700e64f3a5f42c1c543e3cf9 is a bios update if you
wanna have an example to look at.

Then the need to use '-L' goes away.  That would be the final step once
all other q35 bits on both qemu + seabios are committed.
> The alternative would be to commit both trees in parallel, and have
> users of q35 specify -L <bios dir with q35 dsdt table>, until the
> q35-acpi-dsdt.aml is added to qemu.

Kevin tends to wait for the qemu bits being committed before applying
the corresponding patches to seabios.  So I think this would be the best
plan:

  (1) piix dsdt load patches (the stuff discussed above).
  (2) merge q35 series for qemu.
  (3) merge q35 series for seabios.
  (4) seabios update for qemu.

The q35 qemu series seems to be near ready for merge though,
so maybe it isn't worth splitting (1) + (2).

cheers,
  Gerd

[PATCH v2 9/9] seabios: Build the dsdt separately

[Michael S. Tsirkin]

On Tue, Oct 09, 2012 at 11:02:52AM -0400, Jason Baron wrote:
> On Tue, Oct 09, 2012 at 09:13:59AM +0200, Gerd Hoffmann wrote:
> > On 10/09/12 05:35, Jason Baron wrote:
> > > From: Jason Baron <[e-mail]>
> > >
> > > This builds seabios such that the dsdt tables are no longer built into the
> > > seabios binary. They must be passed to the seabios via fw_cfg. This saves
> > > space in the seabios binary for unnecessary dsdt tables.
> > >
> > > I suspect that this will make other users of Seabios, besides qemu unhappy,
> > > but I'm not sure.
> >
> > I think this should best be submitted / discussed separately from the
> > q35 patch series.  And dropping the dsdt from seabios will should
> > actually be the last step.  Loading via fw_cfg works even when seabios
> > has a builtin dsdt.  So we can start loading the dsdt via fwcfg
> > unconditionally in qemu without touching seabios.  Once everything is in
> > place in qemu we can finally zap the builtin dsdt in seabios (or make it
> > a config option so latest seabios continues to work with older qemu
> > versions).
>
> Ok, I like this idea. My only hesitation would be that the dsdt table
> might in the future include a dependency on being built with seabios, such as
> the 'ACPI_EXTRACT_*' stuff. At the moment it can be cleanly separated
> out. And this patch enforces that.
>
> If we don't touch seabios here as you've suggested. What steps are
> required on the qemu side? To copy the 2 .aml files to its bios dir (Not
> too familiar with these details)?
>
> Thanks,
>
> -Jason

I'm guessing if all of acpi is moved out of seabios we'll need
to move ACPI_EXTRACT_ stuff as well as the acpi parsing
out of there as well.

-- 
MST