Re: [SeaBIOS] [PATCHv3] acpi: make default DSDT optional

[Paul Menzel]

Am Montag, den 18.03.2013, 08:57 -0400 schrieb Kevin O'Connor:
> On Mon, Mar 18, 2013 at 01:00:42PM +0200, Michael S. Tsirkin wrote:
> > Since commit f7e4dd6c18ccfbaf6cd2f5eaaed2b77cabc8a406 QEMU loads its

Michael, thank you for putting this in. As commit hashes are hard to
memorize adding the summary to is useful in my opinion. For example for
this commit like this.

Since commit »acpi: autoload dsdt« (f7e4dd6c) [1] …

[…]

[1] http://git.qemu.org/?p=qemu.git;a=commit;h=f7e4dd6c18ccfbaf6cd2f5eaaed2b77ca...

[…]
> > --- a/src/Kconfig
> > +++ b/src/Kconfig
> > @@ -387,6 +387,12 @@ menu "BIOS Tables"
> >          default y
> >          help
> >              Support generation of ACPI tables.
> > config ACPI_DSDT
> >    bool "Include default ACPI DSDT"
> >    default y
> >    depends on ACPI
> >    help
> >        Include default DSDT ACPI table in BIOS.
>
> As Paul points out, it would really help if the help stated which
> released version of QEMU is needed to turn this off.

I just meant the commit message, but it is useful in Kconfig too. Kevin,
good suggestion.
> > --- a/src/acpi.c
> > +++ b/src/acpi.c
> > @@ -202,7 +202,11 @@ struct srat_memory_affinity
> >      u32    reserved3[2];
> >  } PACKED;
> >
> > +#ifdef CONFIG_ACPI_DSDT
> >  #include "acpi-dsdt.hex"
> > +#else
> > +static u8 AmlCode[1];
> > +#endif
>
> As Paul points out, the #ifdef doesn't make sense as the symbol is
> always defined.  However, lets avoid #ifs and let the build weed this
> out.

That was Laszlo and not me. ;-)
> > if (fadt && !fadt->dsdt) {
> > if (fadt && !fadt->dsdt && CONFIG_ACPI_DSDT) {
>
> It would be preferable to make the CONFIG_X be first in the if.

Thanks,

Paul