| 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
| ||
|
Message-ID: <88bdcca8-4df4-15ff-2e88-c53255b1fe30@daynix.com>
Date: Sun, 11 Dec 2022 14:25:31 +0900
From: Akihiko Odaki <akihiko.odaki@...nix.com>
To: Marc Zyngier <maz@...nel.org>,
Akihiko Odaki <akihiko.odaki@...il.com>
Cc: linux-kernel@...r.kernel.org, kvmarm@...ts.linux.dev,
kvmarm@...ts.cs.columbia.edu, linux-arm-kernel@...ts.infradead.org,
Mathieu Poirier <mathieu.poirier@...aro.org>,
Oliver Upton <oliver.upton@...ux.dev>,
Suzuki K Poulose <suzuki.poulose@....com>,
Alexandru Elisei <alexandru.elisei@....com>,
James Morse <james.morse@....com>,
Will Deacon <will@...nel.org>,
Catalin Marinas <catalin.marinas@....com>,
asahi@...ts.linux.dev, Alyssa Rosenzweig <alyssa@...enzweig.io>,
Sven Peter <sven@...npeter.dev>,
Hector Martin <marcan@...can.st>
Subject: Re: [PATCH 0/3] KVM: arm64: Handle CCSIDR associativity mismatches
On 2022/12/04 23:57, Marc Zyngier wrote:
> On Fri, 02 Dec 2022 09:55:24 +0000,
> Akihiko Odaki <akihiko.odaki@...il.com> wrote:
>>
>> On 2022/12/02 18:40, Marc Zyngier wrote:
>>> On Fri, 02 Dec 2022 05:17:12 +0000,
>>> Akihiko Odaki <akihiko.odaki@...nix.com> wrote:
>>>>
>>>>>> On M2 MacBook Air, I have seen no other difference in standard ID
>>>>>> registers and CCSIDRs are exceptions. Perhaps Apple designed this way
>>>>>> so that macOS's Hypervisor can freely migrate vCPU, but I can't assure
>>>>>> that without more analysis. This is still enough to migrate vCPU
>>>>>> running Linux at least.
>>>>>
>>>>> I guess that MacOS hides more of the underlying HW than KVM does. And
>>>>> KVM definitely doesn't hide the MIDR_EL1 registers, which *are*
>>>>> different between the two clusters.
>>>>
>>>> It seems KVM stores a MIDR value of a CPU and reuse it as "invariant"
>>>> value for ioctls while it exposes the MIDR value each physical CPU
>>>> owns to vCPU.
>>>
>>> This only affects the VMM though, and not the guest which sees the
>>> MIDR of the CPU it runs on. The problem is that at short of pinning
>>> the vcpus, you don't know where they will run. So any value is fair
>>> game.
>>
>> Yes, my concern is that VMM can be confused if it sees something
>> different from what the guest on the vCPU sees.
>
> Well, this has been part of the ABI for about 10 years, since Rusty
> introduced this notion of invariant, so userspace is already working
> around it if that's an actual issue.
In that case, I think it is better to document that the interface is not
working properly and deprecated.
>
> This would be easily addressed though, and shouldn't result in any
> issue. The following should do the trick (only lightly tested on an
> M1).
This can be problematic when restoring vcpu state saved with the old
kernel. A possible solution is to allow the userspace to overwrite
MIDR_EL1 as proposed for CCSIDR_EL1.
Regards,
Akihiko Odaki
>
> Thanks,
>
> M.
>
> From f1caacb89eb8ae40dc38669160a2f081f87f4b15 Mon Sep 17 00:00:00 2001
> From: Marc Zyngier <maz@...nel.org>
> Date: Sun, 4 Dec 2022 14:22:22 +0000
> Subject: [PATCH] KVM: arm64: Return MIDR_EL1 to userspace as seen on the vcpu
> thread
>
> When booting, KVM sample the MIDR of the CPU it initialises on,
> and keep this as the value that will forever be exposed to userspace.
>
> However, this has nothing to do with the value that the guest will
> see. On an asymetric system, this can result in userspace observing
> weird things, specially if it has pinned the vcpus on a *different*
> set of CPUs.
>
> Instead, return the MIDR value for the vpcu we're currently on and
> that the vcpu will observe if it has been pinned onto that CPU.
>
> For symmetric systems, this changes nothing. For asymmetric machines,
> they will observe the correct MIDR value at the point of the call.
>
> Reported-by: Akihiko Odaki <akihiko.odaki@...il.com>
> Signed-off-by: Marc Zyngier <maz@...nel.org>
> ---
> arch/arm64/kvm/sys_regs.c | 19 +++++++++++++++++--
> 1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index f4a7c5abcbca..f6bcf8ba9b2e 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1246,6 +1246,22 @@ static int set_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> return 0;
> }
>
> +static int get_midr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + u64 *val)
> +{
> + *val = read_sysreg(midr_el1);
> + return 0;
> +}
> +
> +static int set_midr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + u64 val)
> +{
> + if (val != read_sysreg(midr_el1))
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> static int get_raz_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> u64 *val)
> {
> @@ -1432,6 +1448,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
>
> { SYS_DESC(SYS_DBGVCR32_EL2), NULL, reset_val, DBGVCR32_EL2, 0 },
>
> + { SYS_DESC(SYS_MIDR_EL1), .get_user = get_midr, .set_user = set_midr },
> { SYS_DESC(SYS_MPIDR_EL1), NULL, reset_mpidr, MPIDR_EL1 },
>
> /*
> @@ -2609,7 +2626,6 @@ id_to_sys_reg_desc(struct kvm_vcpu *vcpu, u64 id,
> ((struct sys_reg_desc *)r)->val = read_sysreg(reg); \
> }
>
> -FUNCTION_INVARIANT(midr_el1)
> FUNCTION_INVARIANT(revidr_el1)
> FUNCTION_INVARIANT(clidr_el1)
> FUNCTION_INVARIANT(aidr_el1)
> @@ -2621,7 +2637,6 @@ static void get_ctr_el0(struct kvm_vcpu *v, const struct sys_reg_desc *r)
>
> /* ->val is filled in by kvm_sys_reg_table_init() */
> static struct sys_reg_desc invariant_sys_regs[] = {
> - { SYS_DESC(SYS_MIDR_EL1), NULL, get_midr_el1 },
> { SYS_DESC(SYS_REVIDR_EL1), NULL, get_revidr_el1 },
> { SYS_DESC(SYS_CLIDR_EL1), NULL, get_clidr_el1 },
> { SYS_DESC(SYS_AIDR_EL1), NULL, get_aidr_el1 },
Powered by blists - more mailing lists