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

[Thomas Huth]

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 <address@hidden>
>>> ---
>>>  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?

>> 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?)

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

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

What about the "isa_create_simple(isa_bus, TYPE_I8042)" in mips_r4k.c ?

>>> diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
>>> index 47a606f5e3..af393818fc 100644
>>> --- a/hw/input/pckbd.c
>>> +++ b/hw/input/pckbd.c
>>> @@ -568,6 +568,8 @@ static void i8042_class_initfn(ObjectClass *klass, void 
>>> *data)
>>>      dc->realize = i8042_realizefn;
>>>      dc->vmsd = &vmstate_kbd_isa;
>>>      set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
>>> +    /* i8042 is a device on the motherboard, and not pluggable by the user 
>>> */
> 
> I'm not sure the comment is accurate, maybe "ISA i8042 are provided by
> Super I/O devices"?

Fine for me, too ... but what about mips_r4k in that case?

 Thomas