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: <ZkIdqoELmQ3tUJ8M@arm.com>
Date: Mon, 13 May 2024 15:03:22 +0100
From: Catalin Marinas <catalin.marinas@....com>
To: Steven Price <steven.price@....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 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?

>  #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).

> +
> +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);
>  }

-- 
Catalin