RE: [PATCH] KVM: arm64: Disable OS double lock visibility by default and ignore VMM writes

[Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>]

Hi Oliver,

> -----Original Message-----
> From: Oliver Upton <oliver.upton@linux.dev>
> Sent: Thursday, August 8, 2024 6:40 PM
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Cc: kvmarm@lists.linux.dev; linux-arm-kernel@lists.infradead.org;
> maz@kernel.org; will@kernel.org; catalin.marinas@arm.com;
> james.morse@arm.com; suzuki.poulose@arm.com; yuzenghui
> <yuzenghui@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
> Linuxarm <linuxarm@huawei.com>
> Subject: Re: [PATCH] KVM: arm64: Disable OS double lock visibility by default
> and ignore VMM writes
> 
> Hi Shameer,
> 
> I find myself asking *why* we need this, could you share some details
> on the issue you're encountering?

Sorry, I missed the why part. Mainly for VM migration purposes as we have systems
with DoubleLock implemented and not implemented(with DebugVer 8.2).

> 
> Indeed, RAZ/WI is not a faithful implementation of FEAT_DoubleLock, but
> I wouldn't expect it to be used in a VM in the first place.
> 
> On Thu, Aug 08, 2024 at 01:57:11PM +0100, Shameer Kolothum wrote:
> > KVM exposes the OS double lock feature bit to Guests but returns
> > RAZ/WI on Guest OSDLR_EL1 access. Make sure we are hiding OS double
> > lock from Guests now. However we can't hide DoubleLock if the reported
> > DebugVer is < 8.2. So report a minimum DebugVer of 8.2 to Guests.
> 
> What if a user wanted to virtualize an exact CPU model that only
> implemented v8.0?

Yeah. I was a bit concerned as mentioned below of bumping up DebugVer to 8.2.
But then I found a similar attempt you made a while back,
https://lore.kernel.org/linux-arm-kernel/20211029003202.158161-1-oupton@google.com/T/#meee94d87db3f8042156557dbf9743bb03cf0aaa9

> 
> > All this may break migration from the older kernels. Take care of
> > that by ignoring VMM writes for these values.
> 
> Ignoring userspace writes is a pretty big hammer. In situations where
> KVM had advertised a feature that was outright not supported (e.g. IMP DEF
> PMUs) it _might_ make sense. But with this change we're messing with a
> CPU feature we *do* support.

The concern here is for the DebugVer I guess. But if VMs are not making use of
any 8.0 specific features(as I understand it, only external debugger support is the
difference), then is that an issue?
 
> Would allowing userspace to downgrade ID_AA664DFR0_EL1.DoubleLock to
> 0b1111 be enough?

Yeah. Could I guess. But then we need to check the DebugVer matches to 8.2 or not
as well.

Idea was, is there any point in exposing  features that are not supported or used
by VMs in the first place.

Thanks,
Shameer