On Thu, Mar 14, 2013 at 05:13:54PM +0100, Paolo Bonzini wrote: > Il 14/03/2013 16:59, Gleb Natapov ha scritto: > > 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 :) > > Kind of... Windows will hide serial ports that return not-present for > _STA, for example. Linux will just hide the PNPxxxx path and present it > under /sys/bus/platform instead. > > >> 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. > > Hmm... can you do something like > > Name(PORT, 0xAAAA) > OperationRegion(PEOR, SystemIO, PORT, 0x01) > Field(PEOR, ByteAcc, NoLock, Preserve) { > PEPT, 8, > } > > ? i.e. use a Name inside an OperationRegion? > > If so, then we can patch 0xAAAA to zero for not-present and the port for > present and indeed patch a single place. > > If we have to patch 0x505 all over the place there's an advantage in > patching _STA only. But if we can do the above it's a bit cleaner to > use the port for the patched value, indeed. > And patching Name() is as simple as putting ACPI_EXTRACT_NAME_WORD_CONST above it.
> >> 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. > > We have no_user, but it's broken and not exactly a match for what you > want here. > > Paolo -- Gleb.