Received-SPF: pass (google.com: domain of linux-kernel+bounces-206499-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: Michael Kelley <mhklinux@outlook.com>
To: Catalin Marinas <catalin.marinas@arm.com>
CC: Steven Price <steven.price@arm.com>, "kvm@vger.kernel.org"
	<kvm@vger.kernel.org>, "kvmarm@lists.linux.dev" <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-arm-kernel@lists.infradead.org>, "linux-kernel@vger.kernel.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" <linux-coco@lists.linux.dev>, Ganapatrao
 Kulkarni <gankulkarni@os.amperecomputing.com>
Subject: RE: [PATCH v3 00/14] arm64: Support for running as a guest in Arm CCA
Thread-Topic: [PATCH v3 00/14] arm64: Support for running as a guest in Arm
 CCA
Thread-Index: AQHatysPRYaUedD8SEOOWptuNOqVvrG7hVGQgADmoQCAAA8XwA==
Date: Fri, 7 Jun 2024 16:36:18 +0000
Message-ID:
 <SN6PR02MB41571B5C2C9C59B0DF5F4E7ED4FB2@SN6PR02MB4157.namprd02.prod.outlook.com>
References: <20240605093006.145492-1-steven.price@arm.com>
 <SN6PR02MB415739D48B10C26D2673F3FED4FB2@SN6PR02MB4157.namprd02.prod.outlook.com>
 <ZmMjam3-L807AFR-@arm.com>
In-Reply-To: <ZmMjam3-L807AFR-@arm.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0:
 =?us-ascii?Q?65E8VSnH6qy0vcHpQwnWEnDcgJnlLb5BPylM9W5H4W/V9SfDYtfh804jJn7i?=
 =?us-ascii?Q?Sxk4fpMLjYXavzW7jYi0KyX3TN2tiV2iu56TNA2ZZf7MX6O/kalT+lg7JzFR?=
 =?us-ascii?Q?MbDyp7060nNwn1sIpI0ql2mSZ776eq5c9VlqPz7PignaBwJrgg6o8s6bg8gW?=
 =?us-ascii?Q?G+OllutsuRE2BRu7HqZVQQksgrE/gOFcW0Z4O53Q25B4PrvgSw46u8LqvXWy?=
 =?us-ascii?Q?ist1BtOi0Hd0uEVmE1zGwkGtt4Tee91vi0kcOWBL5JIIzJm0S1/jjSTLuQ2Z?=
 =?us-ascii?Q?imVzt5UukP2y2bE8gmjLIjuAzZExqSwrUSJ+H4RcF1clT26Y5X6mRxllGlOQ?=
 =?us-ascii?Q?I4ki0wFshG39Mdc93F0FxdO3tomxMIi/1IcX33oOko/2o2RcBHWgEMOWbdqL?=
 =?us-ascii?Q?GBKH01awiJebDIws6KqlD3K/V8jSRhWIZbrRyg2SzKHgRTK3sNiLPY4vG2aq?=
 =?us-ascii?Q?XCcP+I2zXRsnU4sxluYsr61E6GTL+fMYoVEAH0bhH64A63Ufd0VhBHIqEWcR?=
 =?us-ascii?Q?f9g4ijSrY0gs79aD7vzPBug9Qzz7MkMa9UIIpP8EUuvkJywlCHOqFDrsBqKz?=
 =?us-ascii?Q?OGubbboz0EXElp8pT2I2se5dwY56BeYwYVdCt5zCQnqNnCb0prZ6qMIpBnxD?=
 =?us-ascii?Q?i9bDVlw9RvrQsfwbB/yPKWzHYVCpuVX8VRV3//kqPaoN+usZYPme8N0cB8BL?=
 =?us-ascii?Q?Pkm1ZwVKB5IjVfwOLiRguK26OVnqB1Z9l/fuCn7C32mJAqIpY0noXPy48isp?=
 =?us-ascii?Q?KxrMhDsnBk2474L1JmE/L2wb/ROCuam2AXLPBcjedncn8eynmXcwubZCMxX+?=
 =?us-ascii?Q?wVbJN1W4zmgEJeiT3R078JMzj4DMt3bQV3Tbd03ATtzQhN4KzMpF534MYVjN?=
 =?us-ascii?Q?dt4+cQCHMxAagIwBe/vxIkdLpdY7xlEagLI1YZUK2gnNsGqR6QpGeBiu1+F4?=
 =?us-ascii?Q?J4v08YgONvyhw8kQZF5H6nQLXdVySJyRzetXkluIO5cAmZWtvJB0BbcDVjC/?=
 =?us-ascii?Q?jSyKtwzf8ryHvLUItA1V4dfgOM4gvbaw7CWnnXi7NbwyVicKHTnZ21lLzH0o?=
 =?us-ascii?Q?F9KNcXBLXRK2pBUCKqZVYZfJj9NTKKZ0stHLcA+cTYhJtNWw1IQq4WsWTcbr?=
 =?us-ascii?Q?FoYfPksndmBDsayjFJYH2HR8A/XYV64Nz808eO47kx+hIGdDn3DMO4iulXST?=
 =?us-ascii?Q?Pq29XMAYmfYSMRLBf0zU+eHz+GmFnMQP4YrzXz0VzjVRMxwdXVGwEHU/u/k?=
 =?us-ascii?Q?=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SN6PR02MB4157.namprd02.prod.outlook.com
X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-CrossTenant-Network-Message-Id: 5fb1d624-d10d-4061-6f59-08dc870ff4db
X-MS-Exchange-CrossTenant-rms-persistedconsumerorg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-CrossTenant-originalarrivaltime: 07 Jun 2024 16:36:18.1701
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR02MB9117

From: Catalin Marinas <catalin.marinas@arm.com> Sent: Friday, June 7, 2024 =
8:13 AM
>=20
> On Fri, Jun 07, 2024 at 01:38:15AM +0000, Michael Kelley wrote:
> > From: Steven Price <steven.price@arm.com> Sent: Wednesday, June 5, 2024=
 2:30 AM
> > > This series adds support for running Linux in a protected VM under th=
e
> > > Arm Confidential Compute Architecture (CCA). This has been updated
> > > following the feedback from the v2 posting[1]. Thanks for the feedbac=
k!
> > > Individual patches have a change log for v3.
> > >
> > > The biggest change from v2 is fixing set_memory_{en,de}crypted() to
> > > perform a break-before-make sequence. Note that only the virtual addr=
ess
> > > supplied is flipped between shared and protected, so if e.g. a vmallo=
c()
> > > address is passed the linear map will still point to the (now invalid=
)
> > > previous IPA. Attempts to access the wrong address may trigger a
> > > Synchronous External Abort. However any code which attempts to access
> > > the 'encrypted' alias after set_memory_decrypted() is already likely =
to
> > > be broken on platforms that implement memory encryption, so I don't
> > > expect problems.
> >
> > In the case of a vmalloc() address, load_unaligned_zeropad() could stil=
l
> > make an access to the underlying pages through the linear address. In
> > CoCo guests on x86, both the vmalloc PTE and the linear map PTE are
> > flipped, so the load_unaligned_zeropad() problem can occur only during
> > the transition between decrypted and encrypted. But even then, the
> > exception handlers have code to fixup this case and allow everything to
> > proceed normally.
> >
> > I haven't looked at the code in your patches, but do you handle that ca=
se,
> > or somehow prevent it?
>=20
> If we can guarantee that only full a vm_struct area is changed at a
> time, the vmap guard page would prevent this issue (not sure we can
> though). Otherwise I think we either change the set_memory_*() code to
> deal with the other mappings or we handle the exception.

I don't think the vmap guard pages help. The vmalloc() memory consists
of individual pages that are scattered throughout the direct map. The stray
reference from load_unaligned_zeropad() will originate in a kmalloc'ed
memory page that precedes one of these scattered individual pages, and
will use a direct map kernel vaddr.  So the guard page in vmalloc space don=
't
come into play. At least in the Hyper-V use case, an entire vmalloc allocat=
ion
*is* flipped as a unit, so the guard pages do prevent a stray reference fro=
m
load_unaligned_zeropad() that originates in vmalloc space. At one
point I looked to see if load_unaligned_zeropad() is ever used on vmalloc
addresses.  I think the answer was "no",  making the guard page question
moot, but I'm not sure. :-(

Another thought: The use of load_unaligned_zeropad() is conditional on
CONFIG_DCACHE_WORD_ACCESS. There are #ifdef'ed alternate
implementations that don't use load_unaligned_zeropad() if it is not
enabled. I looked at just disabling it in CoCo VMs, but I don't know the
performance impact. I speculated that the benefits were more noticeable
in processors from a decade or more ago, and perhaps less so now, but
never did any measurements. There was also a snag in that x86-only
code has a usage of load_unaligned_zeropad() without an alternate
implementation, so I never went fully down that path. But arm64 would
probably "just work" if it were disabled.

>=20
> We also have potential user mappings, do we need to do anything about
> them?

I'm unclear on the scenario here.  Would memory with a user mapping
ever be flipped between decrypted and encrypted while the user mapping
existed?  I don't recall being concerned about user mappings, so maybe
had ruled out that scenario. On x86, flipping between decrypted and
encrypted may effectively change the contents of the memory, so doing
a flip while mapped into user space seems problematic. But maybe I'm
missing something.

Michael