From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:36394) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UG94t-0002SR-3v for qemu-devel@nongnu.org; Thu, 14 Mar 2013 10:24:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UG94n-0003yW-9b for qemu-devel@nongnu.org; Thu, 14 Mar 2013 10:24:19 -0400 Received: from mx1.redhat.com ([209.132.183.28]:63545) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UG94n-0003yR-1a for qemu-devel@nongnu.org; Thu, 14 Mar 2013 10:24:13 -0400 Date: Thu, 14 Mar 2013 16:23:53 +0200 From: Gleb Natapov Message-ID: <20130314142353.GU11223@redhat.com> References: <5d85837fd58683b7e13ac78d6d468af6e152a512.1363243596.git.hutao@cn.fujitsu.com> <514194E4.3040208@redhat.com> <20130314091919.GD11223@redhat.com> <51419BCF.8030003@redhat.com> <20130314123440.GP11223@redhat.com> <5141D57C.7000303@redhat.com> <20130314135658.GA13274@redhat.com> <5141D922.1080508@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5141D922.1080508@redhat.com> Subject: Re: [Qemu-devel] [PATCH v14 3/4] introduce pvevent device to deal with panicked event List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Peter Maydell , "Michael S. Tsirkin" , Hu Tao , qemu-devel , Markus Armbruster , Blue Swirl , Orit Wasserman , Juan Quintela , Alexander Graf , Christian Borntraeger , Jan Kiszka , Andrew Jones , Alex Williamson , Sasha Levin , Stefan Hajnoczi , Luiz Capitulino , KAMEZAWA Hiroyuki , Kevin Wolf , Anthony Liguori , Marcelo Tosatti On Thu, Mar 14, 2013 at 03:05:22PM +0100, Paolo Bonzini wrote: > Il 14/03/2013 14:56, Gleb Natapov ha scritto: > > On Thu, Mar 14, 2013 at 02:49:48PM +0100, Paolo Bonzini wrote: > >> Il 14/03/2013 13:34, Gleb Natapov ha scritto: > >>>> * it can be an ISA device; the interface is the I/O port and ACPI > >>>> support is provided just for convenience of the OSPM. In this case, > >>>> "-device pvevent" should just add handlers for the port. The ACPI > >>>> support is similar to what we do for other on-board ISA devices, for > >>>> example serial ports (the serial ports use PIIX PCI configuration > >>>> instead of fw-cfg, but that's a minor detail). It only needs to work > >>>> for port 0x505, so the fw-cfg data can be a single yes/no value and only > >>>> the _STA method needs patching. See piix4_pm_machine_ready in > >>>> hw/acpi_piix4.c. > >>> > >>> Again I think there is a big difference between well knows device and > >>> PV devices that we add at random location. And if we make the later > >>> configurable i.e it may or may not be present and location where it is > >>> present can be changed then we better not make a guest to do guesses. > >> > >> No guesses here on part of the guest, and no probing in the firmware > >> two. The same number is hard-coded in QEMU and the DSDT, which go in > >> pairs anyway, but _not_ in the guest kernel (also thanks to Hu's nice > >> trick with the methods). > > > > That's the problem. The number is not hard coded in QEMU only DSDT. > > It is hard-coded where the board creates it, or at least as the default > value of the qdev property. > Default value that can be changes is not hard coded. Why do you allow change in one place, but not the other? > > If you hard code it in QEMU (make it non configurable) and make device mandatory > > static DSDT make sense if provided by QEMU. > > You cannot make it mandatory due to versioned machine types, but my plan > would be to make it mandatory on "pc" and "pc-1.5". For that plan it > makes sense to have a static DSDT. Sorry if it was unclear. And then you will have to have different DSDT for pre pc-1.5. Dynamic patching solves exactly that problem. > > >> I think it's a nice compromise. > >> > >>>> * ACPI support is a first-class part of the device. Each instance of > >>>> the device should be there in the ACPI tables. In this case the fw-cfg > >>>> data needs to be a list of ports, and it is probably simpler to combine > >>>> all the definitions in an SSDT that is dynamically-built (similar to > >>>> what we do for PCI hotplug slots). Or even provide a separate SSDT for > >>>> each instance of the device. > >>>> > >>>> I prefer the first, the second seems to be over-engineered. > >>>> > >>> Second is over-engineering indeed. The device should be singleton and > >>> fail if second instance is created. Do we have such capability in qdev? > >> > >> No, but why should it fail? > >> > > Why should it not? Guest cannot use more than on of them, why allow to > > create insane configs? > > Who cares? Insane ISA device configs anyway are not discoverable by > guests, you need to teach the guest about the device manually. > With proper ACPI they are discoverable. Since writing ACPI support for multiple pvpanic devices is clear case of over-engineering it is a courtesy to QEMU users to fail machine creation that cannot be properly described by ACPI. -- Gleb.