| Message ID | 20190328133608.110805-1-marc.zyngier@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [GIT,PULL] KVM/ARM fixes for 5.1-rc3 | expand |
On 28/03/19 14:36, Marc Zyngier wrote: > Paolo, Radim, > > Here's a handful of KVM/ARM fixes for 5.1-rc3. We have a number of > stage-2 page table fixes, some srcu fixes the the ITS emulation, a > reset fix for the virtual PMU, a GICv4 regression fix, and some > cleanups. > > Please pull, > > M. > > The following changes since commit 9e98c678c2d6ae3a17cb2de55d17f69dddaa231b: > > Linux 5.1-rc1 (2019-03-17 14:22:26 -0700) > > are available in the Git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git tags/kvmarm-fixes-for-5.1 > > for you to fetch changes up to 8324c3d518cfd69f2a17866b52c13bf56d3042d8: > > KVM: arm/arm64: Comments cleanup in mmu.c (2019-03-28 13:17:17 +0000) > > ---------------------------------------------------------------- > KVM/ARM fixes for 5.1 > > - Fix THP handling in the presence of pre-existing PTEs > - Honor request for PTE mappings even when THPs are available > - GICv4 performance improvement > - Take the srcu lock when writing to guest-controlled ITS data structures > - Reset the virtual PMU in preemptible context > - Various cleanups > > ---------------------------------------------------------------- > Marc Zyngier (4): > KVM: arm64: Reset the PMU in preemptible context > arm64: KVM: Always set ICH_HCR_EL2.EN if GICv4 is enabled > KVM: arm/arm64: vgic-its: Take the srcu lock when writing to guest memory > KVM: arm/arm64: vgic-its: Take the srcu lock when parsing the memslots > > Suzuki K Poulose (2): > KVM: arm/arm64: Enforce PTE mappings at stage2 when needed > KVM: arm/arm64: Fix handling of stage2 huge mappings > > YueHaibing (1): > KVM: arm/arm64: vgic-its: Make attribute accessors static > > Zenghui Yu (1): > KVM: arm/arm64: Comments cleanup in mmu.c > > arch/arm/include/asm/kvm_mmu.h | 11 +++ > arch/arm/include/asm/stage2_pgtable.h | 2 + > arch/arm64/include/asm/kvm_mmu.h | 11 +++ > arch/arm64/kvm/reset.c | 6 +- > virt/kvm/arm/hyp/vgic-v3-sr.c | 4 +- > virt/kvm/arm/mmu.c | 125 ++++++++++++++++++++-------------- > virt/kvm/arm/vgic/vgic-its.c | 31 +++++---- > virt/kvm/arm/vgic/vgic-v3.c | 4 +- > virt/kvm/arm/vgic/vgic.c | 14 ++-- > 9 files changed, 133 insertions(+), 75 deletions(-) > Pulled, thanks. Will push together with the x86 stuff I'm testing now, and send it to Linus tomorrow. Paolo
Hi Suzuki, On 3/28/19 2:36 PM, Marc Zyngier wrote: > From: Suzuki K Poulose <suzuki.poulose@arm.com> > > commit 6794ad5443a2118 ("KVM: arm/arm64: Fix unintended stage 2 PMD mappings") > made the checks to skip huge mappings, stricter. However it introduced > a bug where we still use huge mappings, ignoring the flag to > use PTE mappings, by not reseting the vma_pagesize to PAGE_SIZE. > > Also, the checks do not cover the PUD huge pages, that was > under review during the same period. This patch fixes both > the issues. I face a regression with this patch. My guest gets stuck. I am running on AMD Seattle. Reverting the patch makes things work again for me. I run with qemu. In this scenario I don't use hugepages. I use 64kB page size for both the host and guest. Thanks Eric > > Fixes : 6794ad5443a2118 ("KVM: arm/arm64: Fix unintended stage 2 PMD mappings") > Reported-by: Zenghui Yu <yuzenghui@huawei.com> > Cc: Zenghui Yu <yuzenghui@huawei.com> > Cc: Christoffer Dall <christoffer.dall@arm.com> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > --- > virt/kvm/arm/mmu.c | 43 +++++++++++++++++++++---------------------- > 1 file changed, 21 insertions(+), 22 deletions(-) > > diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c > index ffd7acdceac7..bcdf978c0d1d 100644 > --- a/virt/kvm/arm/mmu.c > +++ b/virt/kvm/arm/mmu.c > @@ -1594,8 +1594,9 @@ static void kvm_send_hwpoison_signal(unsigned long address, > send_sig_mceerr(BUS_MCEERR_AR, (void __user *)address, lsb, current); > } > > -static bool fault_supports_stage2_pmd_mappings(struct kvm_memory_slot *memslot, > - unsigned long hva) > +static bool fault_supports_stage2_huge_mapping(struct kvm_memory_slot *memslot, > + unsigned long hva, > + unsigned long map_size) > { > gpa_t gpa_start; > hva_t uaddr_start, uaddr_end; > @@ -1610,34 +1611,34 @@ static bool fault_supports_stage2_pmd_mappings(struct kvm_memory_slot *memslot, > > /* > * Pages belonging to memslots that don't have the same alignment > - * within a PMD for userspace and IPA cannot be mapped with stage-2 > - * PMD entries, because we'll end up mapping the wrong pages. > + * within a PMD/PUD for userspace and IPA cannot be mapped with stage-2 > + * PMD/PUD entries, because we'll end up mapping the wrong pages. > * > * Consider a layout like the following: > * > * memslot->userspace_addr: > * +-----+--------------------+--------------------+---+ > - * |abcde|fgh Stage-1 PMD | Stage-1 PMD tv|xyz| > + * |abcde|fgh Stage-1 block | Stage-1 block tv|xyz| > * +-----+--------------------+--------------------+---+ > * > * memslot->base_gfn << PAGE_SIZE: > * +---+--------------------+--------------------+-----+ > - * |abc|def Stage-2 PMD | Stage-2 PMD |tvxyz| > + * |abc|def Stage-2 block | Stage-2 block |tvxyz| > * +---+--------------------+--------------------+-----+ > * > - * If we create those stage-2 PMDs, we'll end up with this incorrect > + * If we create those stage-2 blocks, we'll end up with this incorrect > * mapping: > * d -> f > * e -> g > * f -> h > */ > - if ((gpa_start & ~S2_PMD_MASK) != (uaddr_start & ~S2_PMD_MASK)) > + if ((gpa_start & (map_size - 1)) != (uaddr_start & (map_size - 1))) > return false; > > /* > * Next, let's make sure we're not trying to map anything not covered > - * by the memslot. This means we have to prohibit PMD size mappings > - * for the beginning and end of a non-PMD aligned and non-PMD sized > + * by the memslot. This means we have to prohibit block size mappings > + * for the beginning and end of a non-block aligned and non-block sized > * memory slot (illustrated by the head and tail parts of the > * userspace view above containing pages 'abcde' and 'xyz', > * respectively). > @@ -1646,8 +1647,8 @@ static bool fault_supports_stage2_pmd_mappings(struct kvm_memory_slot *memslot, > * userspace_addr or the base_gfn, as both are equally aligned (per > * the check above) and equally sized. > */ > - return (hva & S2_PMD_MASK) >= uaddr_start && > - (hva & S2_PMD_MASK) + S2_PMD_SIZE <= uaddr_end; > + return (hva & ~(map_size - 1)) >= uaddr_start && > + (hva & ~(map_size - 1)) + map_size <= uaddr_end; > } > > static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, > @@ -1676,12 +1677,6 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, > return -EFAULT; > } > > - if (!fault_supports_stage2_pmd_mappings(memslot, hva)) > - force_pte = true; > - > - if (logging_active) > - force_pte = true; > - > /* Let's check if we will get back a huge page backed by hugetlbfs */ > down_read(¤t->mm->mmap_sem); > vma = find_vma_intersection(current->mm, hva, hva + 1); > @@ -1692,6 +1687,12 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, > } > > vma_pagesize = vma_kernel_pagesize(vma); > + if (logging_active || > + !fault_supports_stage2_huge_mapping(memslot, hva, vma_pagesize)) { > + force_pte = true; > + vma_pagesize = PAGE_SIZE; > + } > + > /* > * The stage2 has a minimum of 2 level table (For arm64 see > * kvm_arm_setup_stage2()). Hence, we are guaranteed that we can > @@ -1699,11 +1700,9 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, > * As for PUD huge maps, we must make sure that we have at least > * 3 levels, i.e, PMD is not folded. > */ > - if ((vma_pagesize == PMD_SIZE || > - (vma_pagesize == PUD_SIZE && kvm_stage2_has_pmd(kvm))) && > - !force_pte) { > + if (vma_pagesize == PMD_SIZE || > + (vma_pagesize == PUD_SIZE && kvm_stage2_has_pmd(kvm))) > gfn = (fault_ipa & huge_page_mask(hstate_vma(vma))) >> PAGE_SHIFT; > - } > up_read(¤t->mm->mmap_sem); > > /* We need minimum second+third level pages */ >
Hi Suzuki, On 4/2/19 11:47 AM, Suzuki K Poulose wrote: > On Mon, Apr 01, 2019 at 07:10:37PM +0200, Auger Eric wrote: >> Hi Suzuki, >> >> On 3/28/19 2:36 PM, Marc Zyngier wrote: >>> From: Suzuki K Poulose <suzuki.poulose@arm.com> >>> >>> commit 6794ad5443a2118 ("KVM: arm/arm64: Fix unintended stage 2 PMD mappings") >>> made the checks to skip huge mappings, stricter. However it introduced >>> a bug where we still use huge mappings, ignoring the flag to >>> use PTE mappings, by not reseting the vma_pagesize to PAGE_SIZE. >>> >>> Also, the checks do not cover the PUD huge pages, that was >>> under review during the same period. This patch fixes both >>> the issues. >> >> I face a regression with this patch. My guest gets stuck. I am running >> on AMD Seattle. Reverting the patch makes things work again for me. I >> run with qemu. In this scenario I don't use hugepages. I use 64kB page >> size for both the host and guest. > > Hi Eric, > > Thanks for the testing. Does the following patch fix the issue for you ? Yes it does. Thanks Eric > > > ---8>--- > kvm: arm: Skip transparent huge pages in unaligned memslots > > We silently create stage2 huge mappings for a memslot with > unaligned IPA and user address. > > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > --- > virt/kvm/arm/mmu.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c > index 27c9583..4a22f5b 100644 > --- a/virt/kvm/arm/mmu.c > +++ b/virt/kvm/arm/mmu.c > @@ -1412,7 +1412,9 @@ static bool transparent_hugepage_adjust(kvm_pfn_t *pfnp, phys_addr_t *ipap) > * page accordingly. > */ > mask = PTRS_PER_PMD - 1; > - VM_BUG_ON((gfn & mask) != (pfn & mask)); > + /* Skip memslots with unaligned IPA and user address */ > + if ((gfn & mask) != (pfn & mask)) > + return false; > if (pfn & mask) { > *ipap &= PMD_MASK; > kvm_release_pfn_clean(pfn); >
On Tue, 02 Apr 2019 11:07:28 +0100, Auger Eric <eric.auger@redhat.com> wrote: > > Hi Suzuki, > > On 4/2/19 11:47 AM, Suzuki K Poulose wrote: > > On Mon, Apr 01, 2019 at 07:10:37PM +0200, Auger Eric wrote: > >> Hi Suzuki, > >> > >> On 3/28/19 2:36 PM, Marc Zyngier wrote: > >>> From: Suzuki K Poulose <suzuki.poulose@arm.com> > >>> > >>> commit 6794ad5443a2118 ("KVM: arm/arm64: Fix unintended stage 2 PMD mappings") > >>> made the checks to skip huge mappings, stricter. However it introduced > >>> a bug where we still use huge mappings, ignoring the flag to > >>> use PTE mappings, by not reseting the vma_pagesize to PAGE_SIZE. > >>> > >>> Also, the checks do not cover the PUD huge pages, that was > >>> under review during the same period. This patch fixes both > >>> the issues. > >> > >> I face a regression with this patch. My guest gets stuck. I am running > >> on AMD Seattle. Reverting the patch makes things work again for me. I > >> run with qemu. In this scenario I don't use hugepages. I use 64kB page > >> size for both the host and guest. > > > > Hi Eric, > > > > Thanks for the testing. Does the following patch fix the issue for you ? > > Yes it does. Thanks for testing this. Suzuki, can you please resend this with Eric's TB, and a Fixes: tag? I'll queue it right away. Thanks, M.
Paolo, Radim, Here's a handful of KVM/ARM fixes for 5.1-rc3. We have a number of stage-2 page table fixes, some srcu fixes the the ITS emulation, a reset fix for the virtual PMU, a GICv4 regression fix, and some cleanups. Please pull, M. The following changes since commit 9e98c678c2d6ae3a17cb2de55d17f69dddaa231b: Linux 5.1-rc1 (2019-03-17 14:22:26 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git tags/kvmarm-fixes-for-5.1 for you to fetch changes up to 8324c3d518cfd69f2a17866b52c13bf56d3042d8: KVM: arm/arm64: Comments cleanup in mmu.c (2019-03-28 13:17:17 +0000) ---------------------------------------------------------------- KVM/ARM fixes for 5.1 - Fix THP handling in the presence of pre-existing PTEs - Honor request for PTE mappings even when THPs are available - GICv4 performance improvement - Take the srcu lock when writing to guest-controlled ITS data structures - Reset the virtual PMU in preemptible context - Various cleanups ---------------------------------------------------------------- Marc Zyngier (4): KVM: arm64: Reset the PMU in preemptible context arm64: KVM: Always set ICH_HCR_EL2.EN if GICv4 is enabled KVM: arm/arm64: vgic-its: Take the srcu lock when writing to guest memory KVM: arm/arm64: vgic-its: Take the srcu lock when parsing the memslots Suzuki K Poulose (2): KVM: arm/arm64: Enforce PTE mappings at stage2 when needed KVM: arm/arm64: Fix handling of stage2 huge mappings YueHaibing (1): KVM: arm/arm64: vgic-its: Make attribute accessors static Zenghui Yu (1): KVM: arm/arm64: Comments cleanup in mmu.c arch/arm/include/asm/kvm_mmu.h | 11 +++ arch/arm/include/asm/stage2_pgtable.h | 2 + arch/arm64/include/asm/kvm_mmu.h | 11 +++ arch/arm64/kvm/reset.c | 6 +- virt/kvm/arm/hyp/vgic-v3-sr.c | 4 +- virt/kvm/arm/mmu.c | 125 ++++++++++++++++++++-------------- virt/kvm/arm/vgic/vgic-its.c | 31 +++++---- virt/kvm/arm/vgic/vgic-v3.c | 4 +- virt/kvm/arm/vgic/vgic.c | 14 ++-- 9 files changed, 133 insertions(+), 75 deletions(-)