| Message ID | 20220810193033.1090251-7-pcc@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: permit MAP_SHARED mappings with MTE enabled | expand |
On Wed, Aug 10, 2022 at 12:30:32PM -0700, Peter Collingbourne wrote: > Certain VMMs such as crosvm have features (e.g. sandboxing) that depend > on being able to map guest memory as MAP_SHARED. The current restriction > on sharing MAP_SHARED pages with the guest is preventing the use of > those features with MTE. Now that the races between tasks concurrently > clearing tags on the same page have been fixed, remove this restriction. > > Signed-off-by: Peter Collingbourne <pcc@google.com> > --- > arch/arm64/kvm/mmu.c | 8 -------- > 1 file changed, 8 deletions(-) > > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c > index d54be80e31dd..fc65dc20655d 100644 > --- a/arch/arm64/kvm/mmu.c > +++ b/arch/arm64/kvm/mmu.c > @@ -1075,14 +1075,6 @@ static void sanitise_mte_tags(struct kvm *kvm, kvm_pfn_t pfn, > > static bool kvm_vma_mte_allowed(struct vm_area_struct *vma) > { > - /* > - * VM_SHARED mappings are not allowed with MTE to avoid races > - * when updating the PG_mte_tagged page flag, see > - * sanitise_mte_tags for more details. > - */ > - if (vma->vm_flags & VM_SHARED) > - return false; I think this is fine with the locking in place (BTW, it may be worth mentioning in the commit message that it's a relaxation of the ABI). I'd like Steven to have a look as well when he gets the time, in case we missed anything on the KVM+MTE side. Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
On 02/09/2022 14:45, Catalin Marinas wrote: > On Wed, Aug 10, 2022 at 12:30:32PM -0700, Peter Collingbourne wrote: >> Certain VMMs such as crosvm have features (e.g. sandboxing) that depend >> on being able to map guest memory as MAP_SHARED. The current restriction >> on sharing MAP_SHARED pages with the guest is preventing the use of >> those features with MTE. Now that the races between tasks concurrently >> clearing tags on the same page have been fixed, remove this restriction. >> >> Signed-off-by: Peter Collingbourne <pcc@google.com> >> --- >> arch/arm64/kvm/mmu.c | 8 -------- >> 1 file changed, 8 deletions(-) >> >> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c >> index d54be80e31dd..fc65dc20655d 100644 >> --- a/arch/arm64/kvm/mmu.c >> +++ b/arch/arm64/kvm/mmu.c >> @@ -1075,14 +1075,6 @@ static void sanitise_mte_tags(struct kvm *kvm, kvm_pfn_t pfn, >> >> static bool kvm_vma_mte_allowed(struct vm_area_struct *vma) >> { >> - /* >> - * VM_SHARED mappings are not allowed with MTE to avoid races >> - * when updating the PG_mte_tagged page flag, see >> - * sanitise_mte_tags for more details. >> - */ >> - if (vma->vm_flags & VM_SHARED) >> - return false; > > I think this is fine with the locking in place (BTW, it may be worth > mentioning in the commit message that it's a relaxation of the ABI). I'd > like Steven to have a look as well when he gets the time, in case we > missed anything on the KVM+MTE side. > > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com> Looks fine to me, and thanks for doing the work: I was never very pleased with the !VM_SHARED restriction, but I couldn't figure a good way of getting the locking to work. Reviewed-by: Steven Price <steven.price@arm.com>
On Wed, 10 Aug 2022 20:30:32 +0100, Peter Collingbourne <pcc@google.com> wrote: > > Certain VMMs such as crosvm have features (e.g. sandboxing) that depend > on being able to map guest memory as MAP_SHARED. The current restriction > on sharing MAP_SHARED pages with the guest is preventing the use of > those features with MTE. Now that the races between tasks concurrently > clearing tags on the same page have been fixed, remove this restriction. > > Signed-off-by: Peter Collingbourne <pcc@google.com> > --- > arch/arm64/kvm/mmu.c | 8 -------- > 1 file changed, 8 deletions(-) > > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c > index d54be80e31dd..fc65dc20655d 100644 > --- a/arch/arm64/kvm/mmu.c > +++ b/arch/arm64/kvm/mmu.c > @@ -1075,14 +1075,6 @@ static void sanitise_mte_tags(struct kvm *kvm, kvm_pfn_t pfn, > > static bool kvm_vma_mte_allowed(struct vm_area_struct *vma) > { > - /* > - * VM_SHARED mappings are not allowed with MTE to avoid races > - * when updating the PG_mte_tagged page flag, see > - * sanitise_mte_tags for more details. > - */ > - if (vma->vm_flags & VM_SHARED) > - return false; > - > return vma->vm_flags & VM_MTE_ALLOWED; > } > Can you provide a pointer to some VMM making use of this functionality and enabling MTE? A set of crosvm patches (for example) would be useful to evaluate this series. Thanks, M.
On Mon, Sep 12, 2022 at 9:23 AM Marc Zyngier <maz@kernel.org> wrote: > > On Wed, 10 Aug 2022 20:30:32 +0100, > Peter Collingbourne <pcc@google.com> wrote: > > > > Certain VMMs such as crosvm have features (e.g. sandboxing) that depend > > on being able to map guest memory as MAP_SHARED. The current restriction > > on sharing MAP_SHARED pages with the guest is preventing the use of > > those features with MTE. Now that the races between tasks concurrently > > clearing tags on the same page have been fixed, remove this restriction. > > > > Signed-off-by: Peter Collingbourne <pcc@google.com> > > --- > > arch/arm64/kvm/mmu.c | 8 -------- > > 1 file changed, 8 deletions(-) > > > > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c > > index d54be80e31dd..fc65dc20655d 100644 > > --- a/arch/arm64/kvm/mmu.c > > +++ b/arch/arm64/kvm/mmu.c > > @@ -1075,14 +1075,6 @@ static void sanitise_mte_tags(struct kvm *kvm, kvm_pfn_t pfn, > > > > static bool kvm_vma_mte_allowed(struct vm_area_struct *vma) > > { > > - /* > > - * VM_SHARED mappings are not allowed with MTE to avoid races > > - * when updating the PG_mte_tagged page flag, see > > - * sanitise_mte_tags for more details. > > - */ > > - if (vma->vm_flags & VM_SHARED) > > - return false; > > - > > return vma->vm_flags & VM_MTE_ALLOWED; > > } > > > > Can you provide a pointer to some VMM making use of this functionality > and enabling MTE? A set of crosvm patches (for example) would be > useful to evaluate this series. Hi Marc, I've been using a modified crosvm to test this series. Please find below a link to the proposed crosvm patches which make use of the series: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3892141 Peter
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c index d54be80e31dd..fc65dc20655d 100644 --- a/arch/arm64/kvm/mmu.c +++ b/arch/arm64/kvm/mmu.c @@ -1075,14 +1075,6 @@ static void sanitise_mte_tags(struct kvm *kvm, kvm_pfn_t pfn, static bool kvm_vma_mte_allowed(struct vm_area_struct *vma) { - /* - * VM_SHARED mappings are not allowed with MTE to avoid races - * when updating the PG_mte_tagged page flag, see - * sanitise_mte_tags for more details. - */ - if (vma->vm_flags & VM_SHARED) - return false; - return vma->vm_flags & VM_MTE_ALLOWED; }
Certain VMMs such as crosvm have features (e.g. sandboxing) that depend on being able to map guest memory as MAP_SHARED. The current restriction on sharing MAP_SHARED pages with the guest is preventing the use of those features with MTE. Now that the races between tasks concurrently clearing tags on the same page have been fixed, remove this restriction. Signed-off-by: Peter Collingbourne <pcc@google.com> --- arch/arm64/kvm/mmu.c | 8 -------- 1 file changed, 8 deletions(-)