linux-kernel - Re: [PATCHv3 08/11] arm64: Check for selected granule support

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]

Date:	Thu, 15 Oct 2015 13:58:57 +0100
From:	"Suzuki K. Poulose" <Suzuki.Poulose@....com>
To:	Mark Rutland <mark.rutland@....com>
Cc:	Jeremy Linton <jeremy.linton@....com>,
	linux-arm-kernel@...ts.infradead.org, steve.capper@...aro.org,
	ard.biesheuvel@...aro.org, marc.zyngier@....com,
	catalin.marinas@....com, will.deacon@....com,
	linux-kernel@...r.kernel.org, christoffer.dall@...aro.org
Subject: Re: [PATCHv3 08/11] arm64: Check for selected granule support

On 15/10/15 13:37, Mark Rutland wrote:
> On Thu, Oct 15, 2015 at 12:25:33PM +0100, Suzuki K. Poulose wrote:
>> On Thu, Oct 15, 2015 at 11:45:15AM +0100, Mark Rutland wrote:
>>> On Wed, Oct 14, 2015 at 04:13:47PM -0500, Jeremy Linton wrote:
>>>> On 10/14/2015 06:20 AM, Suzuki K. Poulose wrote:


>>
>> ----8>----
>>
>> Author: Suzuki K. Poulose <suzuki.poulose@....com>
>> Date:   Wed Oct 14 11:25:16 2015 +0100
>>
>>      arm64: Check for selected granule support
>>
>>      Ensure that the selected page size is supported by the CPU(s). If it isn't
>>      park the CPU. A check is added to the EFI stub to detect if the boot CPU
>>      supports the page size, failing which, we fail the boot gracefully, with
>>      an error message.
>>
>>      Signed-off-by: Suzuki K. Poulose <suzuki.poulose@....com>
>>      [ Added a check to EFI stub ]
>>      Signed-off-by: Jeremy Linton <jeremy.linton@....com>
>
> Your sign-off should be last, given you are taking resposibility for
> Jeremy's patch.

OK, I was a bit confused about how it should look like. I kept it on top
so that I could add [ ] for Jeremy's contribution.

>
> However, I would prefer that the EFI stub addition were a separate/later
> patch.

OK, that makes sense.

>
>> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
>> index a7f3d4b..72d814c 100644
>> --- a/arch/arm64/include/asm/sysreg.h
>> +++ b/arch/arm64/include/asm/sysreg.h

>> +#define ID_AA64MMFR0_TGRAN4_NI		0xf
>> +#define ID_AA64MMFR0_TGRAN4_ON		0x0
>> +#define ID_AA64MMFR0_TGRAN64_NI		0xf
>> +#define ID_AA64MMFR0_TGRAN64_ON		0x0
>> +#define ID_AA64MMFR0_TGRAN16_NI		0x0
>> +#define ID_AA64MMFR0_TGRAN16_ON		0x1
>
> I still don't like "ON" here -- I thought these would also be changed
> s/ON/SUPPORTED/.

I know and I expected that. I have "_ON" in my 'CPU feature' series, which
will/can be reused here. Hence kept it _ON. I can change it everywhere to
_SUPPORTED, since I may need to spin another version for that.


>>   #include <linux/efi.h>
>>   #include <asm/efi.h>
>> +#include <asm/sysreg.h>
>>   #include <asm/sections.h>
>
> Nit: include order.

OK

>
>>
>> +#if defined(CONFIG_ARM64_4K_PAGES)
>> +#define PAGE_SIZE_STR	"4K"
>> +#elif defined(CONFIG_ARM64_64K_PAGES)
>> +#define PAGE_SIZE_STR	"64K"
>> +#endif
>> +
>>   efi_status_t __init handle_kernel_image(efi_system_table_t *sys_table_arg,
>>   					unsigned long *image_addr,
>>   					unsigned long *image_size,
>> @@ -25,6 +32,17 @@ efi_status_t __init handle_kernel_image(efi_system_table_t *sys_table_arg,
>>   	unsigned long kernel_size, kernel_memsize = 0;
>>   	unsigned long nr_pages;
>>   	void *old_image_addr = (void *)*image_addr;
>> +	u64 aa64mmfr0_el1;
>> +
>> +	/*
>> +	 * Check to see if the CPU supports the requested pagesize
>> +	 */
>> +	asm volatile("mrs %0, ID_AA64MMFR0_EL1" : "=r" (aa64mmfr0_el1));
>
> Can we not use read_cpuid() or similar here?

Yes, I will try that out. I didn't want to include additional header-files
in efi-stub.c.


>> +	aa64mmfr0_el1 >>= ID_AA64MMFR0_TGRAN_SHIFT;
>
> ... and can we not do the shift and mask in one go?
>

>> +	if ((aa64mmfr0_el1 & 0xf) != ID_AA64MMFR0_TGRAN_SUPPORTED) {
>> +		pr_efi_err(sys_table_arg, PAGE_SIZE_STR" granule not supported by the CPU\n");
>
> Nit: space before the first quote, please.

Will do

Thanks
Suzuki

--
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/