linux-kernel - Re: [PATCH] Input: i8042: Quiet down probe failure messages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <abc96443-ed27-4021-afa9-280be027e355@amd.com>
Date:   Wed, 6 Dec 2023 13:22:25 -0600
From:   Mario Limonciello <mario.limonciello@....com>
To:     Rahul Rameshbabu <rrameshbabu@...dia.com>
Cc:     dmitry.torokhov@...il.com, linux-input@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Input: i8042: Quiet down probe failure messages

On 12/6/2023 12:46, Rahul Rameshbabu wrote:
> On Wed, 06 Dec, 2023 11:58:18 -0600 Mario Limonciello <mario.limonciello@....com> wrote:
>> The Framework 16" laptop doesn't have a PS/2 keyboard. At bootup the
>> following messages are emitted:
>>
>> i8042: PNP: No PS/2 controller found.
>> i8042: PNP: Probing ports directly.
>> i8042: Can't read CTR while initializing i8042
>> i8042: probe of i8042 failed with error -5
>>
>> The last two messages are ERR and WARN respectively.  These messages
>> might be useful for one boot while diagnosing a problem for someone
>> but as there is no PS/2 controller in PNP or on the machine they're
>> needlessly noisy to emit every boot.
>>
>> Downgrade the CTR message to debug and change the error code for the
>> failure so that the base device code doesn't emit a warning.
>>
>> If someone has problems with i8042 and they need this information,
>> they can turn on dynamic debugging to get these messages.
>>
>> Signed-off-by: Mario Limonciello <mario.limonciello@....com>
>> ---
> 
> For the Framework 16, I think the following should be done.
> 
> Use SERIO_QUIRK_NOPNP for the device to avoid the PS/2 controller
> probing. You can find examples in drivers/input/serio/i8042-acpipnpio.h
> under the i8042_dmi_quirk_table. This will prevent emitting the first
> two messages in the shared snippet.

I had tried this initially, and yes those first two messages are 
removed.  But TBH I'm not too worried about those as they're INFO. 
Adding a quirk just switches them over to a new INFO message.

But the ERR and WARN messages still come up.  The 3 messages that show 
up are:

i8042: PNP detection disabled
i8042: Can't read CTR while initializing i8042
i8042: probe of i8042 failed with error -5

I'm more concerned that ERR and WARN messages show up making you think 
there is a problem to look into.

> 
> 
>>   drivers/input/serio/i8042.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
>> index 9fbb8d31575a..95dd585fdc1a 100644
>> --- a/drivers/input/serio/i8042.c
>> +++ b/drivers/input/serio/i8042.c
>> @@ -1008,8 +1008,8 @@ static int i8042_controller_init(void)
>>   			udelay(50);
>>   
>>   		if (i8042_command(&ctr[n++ % 2], I8042_CMD_CTL_RCTR)) {
>> -			pr_err("Can't read CTR while initializing i8042\n");
>> -			return i8042_probe_defer ? -EPROBE_DEFER : -EIO;
>> +			pr_debug("Can't read CTR while initializing\n");
> 
> I also think this error message should be pr_err in the situation that
> the SERIO_QUIRK_PROBE_DEFER quirk is not used. I think what you are
> likely looking for is avoiding emitting this message when the
> SERIO_QUIRK_PROBE_DEFER quirk is used for noise reduction purposes.

SERIO_QUIRK_PROBE_DEFER isn't set on this machine.

> 
>> +			return i8042_probe_defer ? -EPROBE_DEFER : -ENXIO;
> 
> I do not think this change makes sense to me personally. It is indeed an
> I/O issue with the i8042 controller on the Framework motherboard, so the
> error should be -EIO when i8042_probe_defer is not set.

With i8042.debug enabled I can see that the error is a wait read 
timeout.  So I had thought -ENXIO ("No such device or address") made 
sense here.

If you feel strongly that the errors and warnings should stay as is I I 
wonder if this should be looked at from i8042_pnp_init().

In the no PNP device declared case it still probes, why isn't PNP trusted?

> 
>>   		}
>>   
>>   	} while (n < 2 || ctr[0] != ctr[1]);
> 
> --
> Thanks,
> 
> Rahul Rameshbabu