Re: [Qemu-devel] [PATCH] hw/input/pckbd: The i8042 device should not be user_creatable

[Markus Armbruster <armbru@redhat.com>]

Thomas Huth <thuth@redhat.com> writes:

> On 04/04/2019 15.29, Philippe Mathieu-Daudé wrote:
>> On 4/4/19 12:07 PM, Paolo Bonzini wrote:
>>> On 04/04/19 09:14, Thomas Huth wrote:
>>>> The i8042 PS/2 controller is part of the chipset on the motherboard.
>>>> It is instantiated by the machine init code, and it does not make sense
>>>> to allow the user to plug an additional i8042 in any of the free ISA slots.
>>>> Thus let's mark the device with user_creatable = false.
>>>>
>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>> ---
>>>>  hw/input/pckbd.c | 2 ++
>>>>  1 file changed, 2 insertions(+)
>>>
>>> user_creatable is not for devices that are not pluggable in real life;
>>> it is for devices that crash QEMU (!) or always fail if plugged by the user.
>
> ... hmm, but presenting devices to the user that are clearly not
> intended for direct use is also not very nice, is it?

Maybe, but hiding them should be separate from marking devices that
still defeat device_add.

In the ideal world, we'd be able to start with an empty board, then
build a machine with by wiring together devices.  Say like use
device_add to create and plug into parent bus, qom-set link properties
to create additional wires.

Plenty of devices fail at the device_add stage, or require additional
wiring by code.  These are marked not user_creatable.

See also commit e90f2a8c3e0e677eeea46a9b401c3f98425ffa37.

>>> So the question to ask is: would it make sense, and especially work, to
>>> add an i8042 to machines that do have an ISA bridge (for example the Alpha?)

Scratch the "would it make sense" part, keep the "would it work" part.

> I don't think so. It is a device that is supposed to be part of the
> chipset on the motherboard, so operating systems certainly don't know
> how to use this device on other machines.
>
> And at least some part of the device have to be set up in source code
> (see e.g. i8042_setup_a20_line() ...).

If (and only if) the parts that need code are essential to the
functioning the device, it should be marked not user_creatable.

>> Correct me if I'm wrong but it seems no machine directly use a 8042 on a
>> ISA bus, it is always part of a SuperIO chipset. It is not reflected in
>> the code (in particular the X86 machines, but I'm working on cleaning this).

Known issue: we model a bunch of x86 devices as ISA devices, even though
they're actually part of a super i/o device connected via LPC bus.

[...]