linux-kernel - Re: [PATCH 2/2] platform/x86: Add Intel Galileo platform specific setup

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: <54AF38EA.6090005@nexus-software.ie>
Date:	Fri, 09 Jan 2015 02:11:54 +0000
From:	Bryan O'Donoghue <pure.logic@...us-software.ie>
To:	"Ong, Boon Leong" <boon.leong.ong@...el.com>
CC:	"tglx@...utronix.de" <tglx@...utronix.de>,
	"mingo@...hat.com" <mingo@...hat.com>,
	"hpa@...or.com" <hpa@...or.com>, "x86@...nel.org" <x86@...nel.org>,
	"dvhart@...radead.org" <dvhart@...radead.org>,
	"platform-driver-x86@...r.kernel.org" 
	<platform-driver-x86@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"andy.shevchenko@...il.com" <andy.shevchenko@...il.com>
Subject: Re: [PATCH 2/2] platform/x86: Add Intel Galileo platform specific
 setup

On 09/01/15 01:00, Ong, Boon Leong wrote:

>> +static void __init
>> +intel_galileo_imr_sanity(unsigned long base, unsigned long size) {
>> +	/* Test zero zero */
>> +	if (imr_add(0, 0, 0, 0, true) == 0)
>> +		pr_err(SANITY "zero sized IMR @ 0x00000000\n");
>
> A side-discussion on imr_add():
> I would think that we should allow 1KiB IMR setting. Current imr_add() logic
> is prohibiting it.

Hi Boon Leong. Ermm, it does allow 1 KiB IMR regions. The following code 
works on the unmodifed V1 driver.

/* Test 1 KiB works */
idx = imr_add(0, IMR_ALIGN, IMR_READ_ACCESS_ALL,
               IMR_WRITE_ACCESS_ALL,false);
if (idx < 0)
	pr_err(SANITY "Couldn't add an IMR @ 0x%04x bytes\n", IMR_ALIGN);

Note IMR_ALIGN is 0x400

I'll add that test to the set of sanity tests in V2 just to put your 
mind at ease though.

 > So, the 'size' input should be at least 1KiB and imr_add()
 > internal logic will adjust 'hi' by -1KiB. Please consider ..

Hmm.

Actually I had a response all typed out for you why I didn't want an API 
to presume to modify the size of my input from the user's POV but, 
thinking about it twice - I agree with you.

V2 will subtract IMR_ALIGN (0x400) bytes from the size.

It's stupid to have to subtract IMR_ALIGN bytes on the input - and 
assumes the user of the API understands how the hardware works - but, of 
course the point of an API is so that the user of it doesn't *have* to 
understand that.

Good call.

--
BOD
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/