linux-kernel - Re: [PATCH v5 3/6] ACPI: parse SPCR and enable matching console

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: <56F17F2F.7050305@hurleysoftware.com>
Date:	Tue, 22 Mar 2016 10:21:51 -0700
From:	Peter Hurley <peter@...leysoftware.com>
To:	Aleksey Makarov <aleksey.makarov@...aro.org>,
	Andy Shevchenko <andy.shevchenko@...il.com>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"Rafael J . Wysocki" <rjw@...ysocki.net>,
	Len Brown <lenb@...nel.org>,
	"linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
	"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	linux-arm Mailing List <linux-arm-kernel@...ts.infradead.org>,
	Russell King <linux@....linux.org.uk>,
	Leif Lindholm <leif.lindholm@...aro.org>,
	Graeme Gregory <graeme.gregory@...aro.org>,
	Al Stone <ahs3@...hat.com>,
	Christopher Covington <cov@...eaurora.org>,
	Yury Norov <ynorov@...iumnetworks.com>,
	"Zheng, Lv" <lv.zheng@...el.com>, Jiri Slaby <jslaby@...e.com>
Subject: Re: [PATCH v5 3/6] ACPI: parse SPCR and enable matching console

On 03/22/2016 10:04 AM, Aleksey Makarov wrote:
> 
> 
> On 03/22/2016 07:07 PM, Peter Hurley wrote:
>> On 03/22/2016 04:09 AM, Andy Shevchenko wrote:
>>> On Tue, Mar 22, 2016 at 12:46 PM, Aleksey Makarov
>>> <aleksey.makarov@...aro.org> wrote:
> 
>>>> +       sprintf(opts, "%s,%s,0x%llx,%d", uart, iotype,
>>>> +               table->serial_port.address, baud_rate);
>>>
>>> You may use snprintf(), though my question here what would happen on
>>> 32-bit kernel when you supply 64-bit address as an option?
>>
>> Yeah this should probably use %pa for the printf specifier.
>>
>> But note this exposes underlying bug in the earlycon support, because
>> that was originally written without 32/64-mixed bitness in mind; ie.,
>> the address is parsed and handled as unsigned long in most places.
> 
> I don't quite follow this.  table->serial_port.address is explicitly u64,
> not pointer, so, according to printk-formats.txt %llx is ok here, %pa is wrong.
> Am I missing something?

No, you're right.
There is still the underlying bug I noted; I'll fix that in this release cycle.


>>>>         /*
>>>> -        * Just 'earlycon' is a valid param for devicetree earlycons;
>>>> -        * don't generate a warning from parse_early_params() in that case
>>>> +        * Just 'earlycon' is a valid param for devicetree and ACPI SPCR
>>>> +        * earlycons; don't generate a warning from parse_early_params()
>>>> +        * in that case
>>>>          */
>>>> -       if (!buf || !buf[0])
>>>> -               return early_init_dt_scan_chosen_serial();
>>>> +       if (!buf || !buf[0]) {
>>>> +               init_spcr_earlycon();
>>>
>>>> +               early_init_dt_scan_chosen_serial();
>>>> +               return 0;
>>>
>>> And you hide an error?
>>
>> Well, this is a little bit tricky because "earlycon" early parameter with
>> missing /chosen/stdout-path node is no longer an error, since ACPI may be
>> specifying the earlycon instead.
> 
> Agree, but note the email by Rob Herring.  The code should be like this:
> 
> if (!buf || !buf[0]) {
> 	if (acpi_disabled) {
> 		return early_init_dt_scan_chosen_serial();
> 	} else {
> 		init_spcr_earlycon();
> 		return 0;
> 	}
> }

Ok.


> But that requires to have made ACPI/DT decision at this point.

It's an unfortunate command-line option dependency, but I think it's ok
for the moment.  The same problem would exist if DT could be disabled
via the command line.


> 
> Thank you
> Aleksey
>