linux-kernel - Re: [PATCH v5 05/19] arm64: Detect if in a realm and set RIPAS RAM

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2e8caa91-bf66-4555-87b3-52f469b2c7ef@arm.com>
Date: Fri, 30 Aug 2024 16:54: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>,
 Gavin Shan <gshan@...hat.com>, Shanker Donthineni <sdonthineni@...dia.com>,
 Alper Gun <alpergun@...gle.com>
Subject: Re: [PATCH v5 05/19] arm64: Detect if in a realm and set RIPAS RAM

On 26/08/2024 11:03, Catalin Marinas wrote:
> On Mon, Aug 19, 2024 at 02:19:10PM +0100, Steven Price wrote:
>> +static bool rsi_version_matches(void)
>> +{
>> +	unsigned long ver_lower, ver_higher;
>> +	unsigned long ret = rsi_request_version(RSI_ABI_VERSION,
>> +						&ver_lower,
>> +						&ver_higher);
>> +
>> +	if (ret == SMCCC_RET_NOT_SUPPORTED)
>> +		return false;
>> +
>> +	if (ret != RSI_SUCCESS) {
>> +		pr_err("RME: RMM doesn't support RSI version %lu.%lu. Supported range: %lu.%lu-%lu.%lu\n",
>> +		       RSI_ABI_VERSION_MAJOR, RSI_ABI_VERSION_MINOR,
>> +		       RSI_ABI_VERSION_GET_MAJOR(ver_lower),
>> +		       RSI_ABI_VERSION_GET_MINOR(ver_lower),
>> +		       RSI_ABI_VERSION_GET_MAJOR(ver_higher),
>> +		       RSI_ABI_VERSION_GET_MINOR(ver_higher));
>> +		return false;
>> +	}
>> +
>> +	pr_info("RME: Using RSI version %lu.%lu\n",
>> +		RSI_ABI_VERSION_GET_MAJOR(ver_lower),
>> +		RSI_ABI_VERSION_GET_MINOR(ver_lower));
>> +
>> +	return true;
>> +}
> 
> I don't have the spec at hand now (on a plane) but given the possibility
> of a 1.0 guest regressing on later RMM versions, I wonder whether we
> should simply bail out if it's not an exact version match. I forgot what
> the spec says about returned ranges (they were pretty confusing last
> time I checked).

Well the idea at least is that the RMM can tell us that it is providing
a 1.0 compatible interface. So it might be supporting 1.x but it's
promising that what it's providing is 1.0 compatible.

Indeed the spec allows the RMM to emulate 1.0 while supporting a higher
(incompatible) interface as well - which is where the version ranges
come in. So in the future we might negotiate versions with the RMM, or
opportunistically use newer features if the RMM provides them. But
obviously for now the guest is only 1.0.

I'd prefer not to add an exact version match because then upgrading the
RMM would break existing guests and would probably lead to pressure for
the RMM to simply lie to guests to avoid them breaking on upgrade.

>> +
>> +void __init arm64_rsi_setup_memory(void)
>> +{
>> +	u64 i;
>> +	phys_addr_t start, end;
>> +
>> +	if (!is_realm_world())
>> +		return;
>> +
>> +	/*
>> +	 * Iterate over the available memory ranges and convert the state to
>> +	 * protected memory. We should take extra care to ensure that we DO NOT
>> +	 * permit any "DESTROYED" pages to be converted to "RAM".
>> +	 *
>> +	 * BUG_ON is used because if the attempt to switch the memory to
>> +	 * protected has failed here, then future accesses to the memory are
>> +	 * simply going to be reflected as a SEA (Synchronous External Abort)
>> +	 * which we can't handle.  Bailing out early prevents the guest limping
>> +	 * on and dying later.
>> +	 */
>> +	for_each_mem_range(i, &start, &end) {
>> +		BUG_ON(rsi_set_memory_range_protected_safe(start, end));
>> +	}
> 
> Would it help debugging if we print the memory ranges as well rather
> than just a BUG_ON()?
> 

Yes that would probably be useful - I'll fix that.

Thanks,
Steve