linux-kernel - Re: [PATCH v2 03/14] arm64: realm: Query IPA size from the RMM

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: <c3672ee2-ae50-4750-87e0-5001953a5371@arm.com>
Date: Thu, 16 May 2024 16:13:43 +0100
From: Steven Price <steven.price@....com>
To: Catalin Marinas <catalin.marinas@....com>
Cc: kvm@...r.kernel.org, kvmarm@...ts.linux.dev, Marc Zyngier
 <maz@...nel.org>, Will Deacon <will@...nel.org>,
 James Morse <james.morse@....com>, Oliver Upton <oliver.upton@...ux.dev>,
 Suzuki K Poulose <suzuki.poulose@....com>, Zenghui Yu
 <yuzenghui@...wei.com>, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org, Joey Gouly <joey.gouly@....com>,
 Alexandru Elisei <alexandru.elisei@....com>,
 Christoffer Dall <christoffer.dall@....com>, Fuad Tabba <tabba@...gle.com>,
 linux-coco@...ts.linux.dev,
 Ganapatrao Kulkarni <gankulkarni@...amperecomputing.com>
Subject: Re: [PATCH v2 03/14] arm64: realm: Query IPA size from the RMM

On 13/05/2024 15:03, Catalin Marinas wrote:
> On Fri, Apr 12, 2024 at 09:42:02AM +0100, Steven Price wrote:
>> diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h
>> index dd9ee67d1d87..15d8f0133af8 100644
>> --- a/arch/arm64/include/asm/pgtable-prot.h
>> +++ b/arch/arm64/include/asm/pgtable-prot.h
>> @@ -63,6 +63,9 @@
>>  #include <asm/pgtable-types.h>
>>  
>>  extern bool arm64_use_ng_mappings;
>> +extern unsigned long prot_ns_shared;
>> +
>> +#define PROT_NS_SHARED		((prot_ns_shared))
> 
> Nit: what's with the double parenthesis here?

No idea - I'll remove a set!

>>  #define PTE_MAYBE_NG		(arm64_use_ng_mappings ? PTE_NG : 0)
>>  #define PMD_MAYBE_NG		(arm64_use_ng_mappings ? PMD_SECT_NG : 0)
>> diff --git a/arch/arm64/kernel/rsi.c b/arch/arm64/kernel/rsi.c
>> index 1076649ac082..b93252ed6fc5 100644
>> --- a/arch/arm64/kernel/rsi.c
>> +++ b/arch/arm64/kernel/rsi.c
>> @@ -7,6 +7,11 @@
>>  #include <linux/memblock.h>
>>  #include <asm/rsi.h>
>>  
>> +struct realm_config __attribute((aligned(PAGE_SIZE))) config;
> 
> Another nit: use __aligned(PAGE_SIZE).
> 
> However, does the spec require this to be page-size aligned? The spec
> says aligned to 0x1000 and that's not necessarily the kernel page size.
> It also states that the RsiRealmConfig structure is 4096 and I did not
> see this in the first patch, you only have 8 bytes in this structure.
> Some future spec may write more data here overriding your other
> variables in the data section.
> 
> So that's the wrong place to force the alignment. Just do this when you
> define the actual structure in the first patch:
> 
> struct realm_config {
> 	union {
> 		unsigned long ipa_bits; /* Width of IPA in bits */
> 		u8 __pad[4096];
> 	};
> } __aligned(4096);
> 
> and maybe with a comment on why the alignment and padding. You could
> also have an unnamed struct around ipa_bits in case you want to add more
> fields in the future (siginfo follows this pattern).

Good suggestion - I'll update this. It turns out struct realm_config is
already missing "hash_algo" (not that we use it yet), so I'll add that too.

Thanks,

Steve

>> +
>> +unsigned long prot_ns_shared;
>> +EXPORT_SYMBOL(prot_ns_shared);
>> +
>>  DEFINE_STATIC_KEY_FALSE_RO(rsi_present);
>>  EXPORT_SYMBOL(rsi_present);
>>  
>> @@ -53,6 +58,9 @@ void __init arm64_rsi_init(void)
>>  {
>>  	if (!rsi_version_matches())
>>  		return;
>> +	if (rsi_get_realm_config(&config))
>> +		return;
>> +	prot_ns_shared = BIT(config.ipa_bits - 1);
>>  
>>  	static_branch_enable(&rsi_present);
>>  }
>