[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YC1RUGv5eL1+dZDs@google.com>
Date: Wed, 17 Feb 2021 17:24:32 +0000
From: Quentin Perret <qperret@...gle.com>
To: Mate Toth-Pal <mate.toth-pal@....com>, r@...gle.com
Cc: Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>, Marc Zyngier <maz@...nel.org>,
James Morse <james.morse@....com>,
Julien Thierry <julien.thierry.kdev@...il.com>,
Suzuki K Poulose <suzuki.poulose@....com>,
Rob Herring <robh+dt@...nel.org>,
Frank Rowand <frowand.list@...il.com>,
devicetree@...r.kernel.org, android-kvm@...gle.com,
linux-kernel@...r.kernel.org, kernel-team@...roid.com,
kvmarm@...ts.cs.columbia.edu, linux-arm-kernel@...ts.infradead.org,
Fuad Tabba <tabba@...gle.com>,
Mark Rutland <mark.rutland@....com>,
David Brazdil <dbrazdil@...gle.com>, nd@....com
Subject: Re: [RFC PATCH v2 00/26] KVM/arm64: A stage 2 for the host
Hi Mate,
On Wednesday 17 Feb 2021 at 17:27:07 (+0100), Mate Toth-Pal wrote:
> We tested the pKVM changes pulled from here:
>
>
> > https://android-kvm.googlesource.com/linux qperret/host-stage2-v2
>
>
> We were using a target with Arm architecture with FEAT_S2FWB, and found that
> there is a bug in the patch.
>
>
> It turned out that the Kernel checks for the extension, and sets up the
> stage 2 translation so that it forces the host memory type to write-through.
> However it seems that the code doesn't turn on the feature in the HCR_EL2
> register.
>
>
> We were able to fix the issue by applying the following patch:
>
>
> diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> index 0cd3eb178f3b..e8521a072ea6 100644
> --- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> +++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> @@ -105,6 +105,8 @@ int kvm_host_prepare_stage2(void *mem_pgt_pool, void
> *dev_pgt_pool)
> params->vttbr = kvm_get_vttbr(mmu);
> params->vtcr = host_kvm.arch.vtcr;
> params->hcr_el2 |= HCR_VM;
> + if (cpus_have_const_cap(ARM64_HAS_STAGE2_FWB))
> + params->hcr_el2 |= HCR_FWB;
> __flush_dcache_area(params, sizeof(*params));
> }
Aha, indeed, this looks right. I'll double check HCR_EL2 to see if I'm
missing any other, and I'll add this to v3.
Thanks for testing, and the for the report.
Quentin
Powered by blists - more mailing lists