Received-SPF: pass (google.com: domain of linux-kernel+bounces-177665-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Mon, 13 May 2024 15:03:22 +0100
From: Catalin Marinas <catalin.marinas@arm.com>
To: Steven Price <steven.price@arm.com>
Cc: kvm@vger.kernel.org, kvmarm@lists.linux.dev,
	Marc Zyngier <maz@kernel.org>, Will Deacon <will@kernel.org>,
	James Morse <james.morse@arm.com>,
	Oliver Upton <oliver.upton@linux.dev>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Zenghui Yu <yuzenghui@huawei.com>,
	linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
	Joey Gouly <joey.gouly@arm.com>,
	Alexandru Elisei <alexandru.elisei@arm.com>,
	Christoffer Dall <christoffer.dall@arm.com>,
	Fuad Tabba <tabba@google.com>, linux-coco@lists.linux.dev,
	Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com>
Subject: Re: [PATCH v2 03/14] arm64: realm: Query IPA size from the RMM
Message-ID: <ZkIdqoELmQ3tUJ8M@arm.com>
References: <20240412084213.1733764-1-steven.price@arm.com>
 <20240412084213.1733764-4-steven.price@arm.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240412084213.1733764-4-steven.price@arm.com>

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