Il 14/03/2013 15:23, Gleb Natapov ha scritto: > 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?
I'm just following the model of other ISA devices, I don't think there's any difference in this respect between well-known and pv devices (also because in the end all modern guests will use ACPI to discover even well-known devices). The board hardcodes 0x505 for pvpanic just like it hardcodes 0x3f8 for serial ports. >>> 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. Yes, but it's enough to patch _STA. Easier in both QEMU and the BIOS. >>>> I think it's a nice compromise. ^^^ This still holds. :) >>>>>> * 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. We don't fail machine creation if someone wants to place a serial port at 0x5678. With ISA it's basically garbage-in, garbage-out, I don't see a reason to make pvpanic special in this respect. Paolo
