| Message ID | 20200113111323.10463-1-rppt@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | arm/arm64: add support for folded p4d page tables | expand |
On Mon, Jan 13, 2020 at 01:13:22PM +0200, Mike Rapoport wrote: > From: Mike Rapoport <rppt@linux.ibm.com> > > This is a part of clean up of the page table manipulation code that aims to > remove asm-generic/5level-fixup.h and asm-generic/pgtable-nop4d-hack.h > > There is a single patch for both arm and arm64 because doing the conversion > separately would mean breaking the shared mmu bits in virt/kvm/arm. Unfortunately, that's going to be really hard to merge, as the two architectures are maintained in different trees and the breadth of this patch series is likely to lead to conflicts in both. Will
On 2020-01-22 18:50, Will Deacon wrote: > On Mon, Jan 13, 2020 at 01:13:22PM +0200, Mike Rapoport wrote: >> From: Mike Rapoport <rppt@linux.ibm.com> >> >> This is a part of clean up of the page table manipulation code that >> aims to >> remove asm-generic/5level-fixup.h and asm-generic/pgtable-nop4d-hack.h >> >> There is a single patch for both arm and arm64 because doing the >> conversion >> separately would mean breaking the shared mmu bits in virt/kvm/arm. > > Unfortunately, that's going to be really hard to merge, as the two > architectures are maintained in different trees and the breadth of this > patch series is likely to lead to conflicts in both. But maybe this is the reason we've all been waiting for, for which we sacrifice 32bit KVM host on the altar of progress, and finally move along. Will and I are the only known users, and that'd be a good incentive to experience some if this 64bit goodness... ;-) M.
On Wed, Jan 22, 2020 at 06:50:17PM +0000, Will Deacon wrote: > On Mon, Jan 13, 2020 at 01:13:22PM +0200, Mike Rapoport wrote: > > From: Mike Rapoport <rppt@linux.ibm.com> > > > > This is a part of clean up of the page table manipulation code that aims to > > remove asm-generic/5level-fixup.h and asm-generic/pgtable-nop4d-hack.h > > > > There is a single patch for both arm and arm64 because doing the conversion > > separately would mean breaking the shared mmu bits in virt/kvm/arm. > > Unfortunately, that's going to be really hard to merge, as the two > architectures are maintained in different trees and the breadth of this > patch series is likely to lead to conflicts in both. I anyway realized that sending these changes arch-by-arch was not so bright idea, so my intention is to make "v2" include all the changes required to drop asm-generic/5level-fixup.h and merge it via the -mm tree. > Will
Hi Marc, On Wednesday 22 Jan 2020 at 18:56:38 (+0000), Marc Zyngier wrote: > But maybe this is the reason we've all been waiting for, for which we > sacrifice 32bit KVM host on the altar of progress, and finally move along. > > Will and I are the only known users, and that'd be a good incentive to > experience some if this 64bit goodness... ;-) Jumping in this discussion a bit randomly, but I just wanted to share some thoughts that hopefully are relevant to this discussion and can be of interest to the community. Context: we have a use-case where guests would need some degree of memory protection from the host for confidentiality reasons. We're currently looking at extending KVM to support this feature by enabling the stage 2 translation for the host (in the NVHE case) so we can prevent it from accessing private guest memory, in addition to many other changes required to make this work properly. We're currently at the prototyping stage, but hopefully we'll be able to share patches soon. I'm bringing this up now because this particular use-case doesn't seem relevant in the arm32 world -- all our potential users are on arm64. However, because of the current structure of the arm/arm64 KVM host code, making significant arm64-specific changes turns out to be really hard. We're currently left with three options: 1. move code from virt/kvm/arm and duplicate it in the arch/arm and arch/arm64 folders so the arm64 version can diverge. I can imagine this duplication isn't exactly an appealing solution from a maintainer's perspective ... 2. do changes in the virt/kvm/arm folder directly, but these must be met with matching changes in the respective arch/ folders. The code added to arch/arm, however, would be practically dead code, largely un-used and un-tested as there will be no real arm32 users of this feature. 3. have lots of kvm_arm_* callbacks stubbed for arm32, but this tends to be really hard to apply to this use-case as some of the changes are really quite intrusive. Obviously, details matter for all of this, and lots of discussions will be needed once the patches are on the list. But the point I'm trying to make here is the following: regardless of the option we end up choosing (most likely a mix of all three), the sole fact that we have to deal with this is clearly slowing down development of the feature. This would a be perfectly reasonable and acceptable overhead if this had to be done to keep 32bit KVM host support for a real user community, but since it doesn't seem to exist (?), fighting with the above options feels like a lot of wasted efforts. (Note: I am not implying that Will and you are not real persons, but well, you see what I mean ;-)). So, this is the end of my daily rant. But hopefully this other example of a real-world feature that's being held back by the 32bit KVM host code will be useful background when/if we go ahead and finally decide stop supporting it. Thanks, Quentin
Hi Quentin, On 2020-01-24 12:20, Quentin Perret wrote: > Hi Marc, > > On Wednesday 22 Jan 2020 at 18:56:38 (+0000), Marc Zyngier wrote: >> But maybe this is the reason we've all been waiting for, for which we >> sacrifice 32bit KVM host on the altar of progress, and finally move >> along. >> >> Will and I are the only known users, and that'd be a good incentive to >> experience some if this 64bit goodness... ;-) [future work for which 32bit support gets in the way] > This would a be perfectly reasonable and acceptable overhead if this > had > to be done to keep 32bit KVM host support for a real user community, > but > since it doesn't seem to exist (?), fighting with the above options > feels like a lot of wasted efforts. (Note: I am not implying that Will > and you are not real persons, but well, you see what I mean ;-)). I don't disagree at all. To be honest, I've been on the cusp of getting rid of it for a while, for multiple reasons: - It has no users (as you noticed) - It is hardly tested (a consequence of the above) - It isn't feature complete (no debug, no PMU) - It doesn't follow any of the evolution of the architecture (a more generic feature of the 32bit port, probably because people run their 64bit-capable cores in 64bit mode) - It is becoming a mess of empty stubs The maintenance aspect hasn't been a real problem so far. Even the NV support is only about 200 lines of stubs. But what you have in mind is going to be much more invasive, and I wouldn't want an unused feature to get in the way. What I may end-up doing is to send a RFC series to remove the 32bit host support from the tree during in the 5.6 cycle, targeting 5.7. If someone shouts loudly during that time frame, we keep it and you'll have to work around it. If nobody cares, then dropping it is the right thing to do. Would that be OK with you? M.
On Friday 24 Jan 2020 at 13:34:35 (+0000), Marc Zyngier wrote: > I don't disagree at all. To be honest, I've been on the cusp of getting > rid of it for a while, for multiple reasons: > > - It has no users (as you noticed) > - It is hardly tested (a consequence of the above) > - It isn't feature complete (no debug, no PMU) > - It doesn't follow any of the evolution of the architecture (a more > generic feature of the 32bit port, probably because people run their > 64bit-capable cores in 64bit mode) > - It is becoming a mess of empty stubs > > The maintenance aspect hasn't been a real problem so far. Even the NV > support is only about 200 lines of stubs. But what you have in mind is > going to be much more invasive, and I wouldn't want an unused feature to > get in the way. > > What I may end-up doing is to send a RFC series to remove the 32bit host > support from the tree during in the 5.6 cycle, targeting 5.7. If someone > shouts loudly during that time frame, we keep it and you'll have to work > around it. If nobody cares, then dropping it is the right thing to do. > > Would that be OK with you? Absolutely. And yes, if there are users of the 32 bits port, it'll be on us to work around in a clean way, but I think this is perfectly fair. I'll be happy to try and test your RFC series when it goes on the list if that can help. Thanks! Quentin
From: Mike Rapoport <rppt@linux.ibm.com> Hi, This is a part of clean up of the page table manipulation code that aims to remove asm-generic/5level-fixup.h and asm-generic/pgtable-nop4d-hack.h There is a single patch for both arm and arm64 because doing the conversion separately would mean breaking the shared mmu bits in virt/kvm/arm. v2: * fix build error for arch/arm64/mm/dump.c * simplify p4d unfolding in arch/arm64/mm/kasan_init.c Mike Rapoport (1): arm/arm64: add support for folded p4d page tables arch/arm/include/asm/kvm_mmu.h | 5 +- arch/arm/include/asm/pgtable.h | 1 - arch/arm/include/asm/stage2_pgtable.h | 15 +- arch/arm/lib/uaccess_with_memcpy.c | 9 +- arch/arm/mach-sa1100/assabet.c | 2 +- arch/arm/mm/dump.c | 29 +++- arch/arm/mm/fault-armv.c | 7 +- arch/arm/mm/fault.c | 28 +++- arch/arm/mm/idmap.c | 3 +- arch/arm/mm/init.c | 2 +- arch/arm/mm/ioremap.c | 12 +- arch/arm/mm/mm.h | 2 +- arch/arm/mm/mmu.c | 35 +++- arch/arm/mm/pgd.c | 40 ++++- arch/arm64/include/asm/kvm_mmu.h | 10 +- arch/arm64/include/asm/pgalloc.h | 10 +- arch/arm64/include/asm/pgtable-types.h | 5 +- arch/arm64/include/asm/pgtable.h | 37 +++-- arch/arm64/include/asm/stage2_pgtable.h | 48 ++++-- arch/arm64/kernel/hibernate.c | 46 +++++- arch/arm64/mm/dump.c | 29 +++- arch/arm64/mm/fault.c | 9 +- arch/arm64/mm/hugetlbpage.c | 15 +- arch/arm64/mm/kasan_init.c | 26 ++- arch/arm64/mm/mmu.c | 52 ++++-- arch/arm64/mm/pageattr.c | 7 +- virt/kvm/arm/mmu.c | 209 ++++++++++++++++++++---- 27 files changed, 550 insertions(+), 143 deletions(-)