From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:51213) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UcvIm-0002Pa-Lh for qemu-devel@nongnu.org; Thu, 16 May 2013 06:20:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UcvIj-00011G-6q for qemu-devel@nongnu.org; Thu, 16 May 2013 06:20:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:64822) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UcvIi-000119-Sv for qemu-devel@nongnu.org; Thu, 16 May 2013 06:20:45 -0400 Message-ID: <5194B383.9060409@redhat.com> Date: Thu, 16 May 2013 12:22:59 +0200 From: Laszlo Ersek MIME-Version: 1.0 References: <1368590499-11707-1-git-send-email-lig.fnst@cn.fujitsu.com> <1368590499-11707-2-git-send-email-lig.fnst@cn.fujitsu.com> <51934995.4010000@redhat.com> <1368608071.5142.55.camel@liguang.fnst.cn.fujitsu.com> <5193913A.8070701@redhat.com> <1368663276.5142.61.camel@liguang.fnst.cn.fujitsu.com> In-Reply-To: <1368663276.5142.61.camel@liguang.fnst.cn.fujitsu.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 2/2] pc: reject do pc_acpi_init if acpi_enabled is false List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: li guang Cc: Anthony Liguori , Gleb Natapov , mst@redhat.com, qemu-devel@nongnu.org, Bruce Rogers , Gerd Hoffmann , Igor Mammedov , Paolo Bonzini On 05/16/13 02:14, li guang wrote: > =E5=9C=A8 2013-05-15=E4=B8=89=E7=9A=84 15:44 +0200=EF=BC=8CLaszlo Ersek= =E5=86=99=E9=81=93=EF=BC=9A >> On 05/15/13 10:54, li guang wrote: >>> =E5=9C=A8 2013-05-15=E4=B8=89=E7=9A=84 10:38 +0200=EF=BC=8CPaolo Bonz= ini=E5=86=99=E9=81=93=EF=BC=9A >>>> Il 15/05/2013 06:01, liguang ha scritto: >>>>> Signed-off-by: liguang >>>> >>>> --verbose, please. >>>> >>>> What problem does this patch fix? >>> >>> Oh, sorry to be lazy ... >>> QEMU's option '-no-acpi' seems does not play >>> a correct role, even started with this,=20 >>> ACPI tables will also be embedded into BIOS,=20 >>> and there's no different between with or without it >>> for q35, as i can see. >>> >>> here, I'm assuming '-no-acpi' is to disable ACPI. >> >> -no-acpi disables a block of code in pc_init1() [hw/i386/pc_piix.c], >> namely piix4_pm_init() and smbus_eeprom_init(). >> >> pc_acpi_init() loads / exports a default DSDT for the boot firmware. I= f >> the -acpitable switch is passed, then that code doesn't run. >=20 > Yes, if '-acpitable' & '-no-acpi' passed, I think QEMU is valid to > override '-no-acpi' by '-acpitable'. I think these flags are orthogonal. As far as I can see, the former turns off a piece of hardware emulation (like support for suspend / hibernate and the System Control Interrupt, and more). The latter enables the user to provide custom ACPI tables for SeaBIOS (or other boot firmware) in place of the default DSDT. It seems that originally "acpi_enabled" (-no-acpi) used to control both purposes, see commit 6515b20: the call to acpi_bios_init() used to depend on acpi_enabled !=3D 0. However the two goals got separated with commit a5954d5 apparently. >> I think disabling PM but keeping the default DSDT from SeaBIOS is a >> valid use case; the DSDT seems to contain a bunch of non-PM >> functionality (see src/acpi-dsdt*.dsl in SeaBIOS). The "-no-acpi" swit= ch >> is likely a misnomer (it should say "-no-acpi-pm" or some such), but i= n >> any case I believe it should not prevent exporting the DSDT. >=20 > what's the purpose of only disable PM by '-no-acpi'? I mentioned the src/acpi-dsdt*.dsl files in SeaBIOS that get built into the default DSDT. With "-no-acpi" only, qemu keeps at least the following working: - the \DBUG ACPI method [src/acpi-dsdt-dbug.dsl], - definition of the \_SB\HPET ("PNP0103") device [src/acpi-dsdt-hpet.dsl]= , - descriptions (interrupts, io-ports) of ISA devices (RTC, keyboard, mouse, floppy disk controller, parport, serial port) [src/acpi-dsdt-isa.dsl], - description of the PCI IO windows (PCI host bridge windows) [src/acpi-dsdt-pci-crs.dsl], whose lack can mess up VGA display for example (see the "pci=3Dnocrs" kernel option), - the PCI interrupt routing table [acpi-dsdt.dsl]. Again, I believe "-no-acpi" has probably become a misnomer by now and it means "-no-acpi-pm" in reality. > can we use -no-acpi to disable whole ACPI? I cannot *prove* it would be wrong, but I feel insecure about it, considering the commit history and the non-PM-related functionality in the default DSDT. Currently a user may expect the above contents to be present in ACPI tables, and only PM (plus maybe hotplug) not to work, when passing -no-acpi. Anyway I'm no authority on this -- anyone, feel free to chime in. I'm certainly not NACKing the patch, I just have concerns. Thanks, Laszlo