[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v14 3/4] introduce pvevent device to deal with p
From: |
Gleb Natapov |
Subject: |
Re: [Qemu-devel] [PATCH v14 3/4] introduce pvevent device to deal with panicked event |
Date: |
Thu, 14 Mar 2013 17:59:10 +0200 |
On Thu, Mar 14, 2013 at 04:50:40PM +0100, Paolo Bonzini wrote:
> 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).
>
We are not there yet :)
> 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.
>
Yes, if you do not allow changing IO port patching _STA is enough, but
if you already patching it is easy to patch both.
> >>>> I think it's a nice compromise.
>
> ^^^ This still holds. :)
If we would have found a reasonable way to go without patching at all
then it would have been worthwhile to consider compromises, but if
patching is inevitable I honestly do not see big difference between
patching one place or two.
>
> >>>>>> * 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.
>
Fine with me. That was just a suggestion. I thought we had singleton
qdev flag.
--
Gleb.
- Re: [Qemu-devel] [PATCH v14 3/4] introduce pvevent device to deal with panicked event, (continued)
- Re: [Qemu-devel] [PATCH v14 3/4] introduce pvevent device to deal with panicked event, Markus Armbruster, 2013/03/20
- Re: [Qemu-devel] [PATCH v14 3/4] introduce pvevent device to deal with panicked event, Gleb Natapov, 2013/03/14
- Re: [Qemu-devel] [PATCH v14 3/4] introduce pvevent device to deal with panicked event, Paolo Bonzini, 2013/03/14
- Re: [Qemu-devel] [PATCH v14 3/4] introduce pvevent device to deal with panicked event, Gleb Natapov, 2013/03/14
- Re: [Qemu-devel] [PATCH v14 3/4] introduce pvevent device to deal with panicked event, Paolo Bonzini, 2013/03/14
- Re: [Qemu-devel] [PATCH v14 3/4] introduce pvevent device to deal with panicked event, Gleb Natapov, 2013/03/14
- Re: [Qemu-devel] [PATCH v14 3/4] introduce pvevent device to deal with panicked event, Paolo Bonzini, 2013/03/14
- Re: [Qemu-devel] [PATCH v14 3/4] introduce pvevent device to deal with panicked event,
Gleb Natapov <=
- Re: [Qemu-devel] [PATCH v14 3/4] introduce pvevent device to deal with panicked event, Paolo Bonzini, 2013/03/14
- Re: [Qemu-devel] [PATCH v14 3/4] introduce pvevent device to deal with panicked event, Gleb Natapov, 2013/03/15
- Re: [Qemu-devel] [PATCH v14 3/4] introduce pvevent device to deal with panicked event, Markus Armbruster, 2013/03/20
- Re: [Qemu-devel] [PATCH v14 3/4] introduce pvevent device to deal with panicked event, Hu Tao, 2013/03/14
Re: [Qemu-devel] [PATCH v14 3/4] introduce pvevent device to deal with panicked event, Markus Armbruster, 2013/03/20
[Qemu-devel] [PATCH v14 4/4] pvevent: add document to describe the usage, Hu Tao, 2013/03/14
Re: [Qemu-devel] [PATCH v14 0/4] pvevent device to deal with guest panic event, Gleb Natapov, 2013/03/14