Re: [Qemu-devel] [PATCH v14 3/4] introduce pvevent device to deal with panicked event

[Paolo Bonzini <pbonzini@redhat.com>]

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.

> 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.

>> 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.

Paolo