[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230221121135.GA1595130@chaop.bj.intel.com>
Date: Tue, 21 Feb 2023 20:11:35 +0800
From: Chao Peng <chao.p.peng@...ux.intel.com>
To: Michael Roth <michael.roth@....com>
Cc: Sean Christopherson <seanjc@...gle.com>,
Isaku Yamahata <isaku.yamahata@...il.com>, kvm@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-mm@...ck.org,
linux-fsdevel@...r.kernel.org, linux-arch@...r.kernel.org,
linux-api@...r.kernel.org, linux-doc@...r.kernel.org,
qemu-devel@...gnu.org, Paolo Bonzini <pbonzini@...hat.com>,
Jonathan Corbet <corbet@....net>,
Vitaly Kuznetsov <vkuznets@...hat.com>,
Wanpeng Li <wanpengli@...cent.com>,
Jim Mattson <jmattson@...gle.com>,
Joerg Roedel <joro@...tes.org>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Arnd Bergmann <arnd@...db.de>,
Naoya Horiguchi <naoya.horiguchi@....com>,
Miaohe Lin <linmiaohe@...wei.com>, x86@...nel.org,
"H . Peter Anvin" <hpa@...or.com>, Hugh Dickins <hughd@...gle.com>,
Jeff Layton <jlayton@...nel.org>,
"J . Bruce Fields" <bfields@...ldses.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Shuah Khan <shuah@...nel.org>, Mike Rapoport <rppt@...nel.org>,
Steven Price <steven.price@....com>,
"Maciej S . Szmigiero" <mail@...iej.szmigiero.name>,
Vlastimil Babka <vbabka@...e.cz>,
Vishal Annapurve <vannapurve@...gle.com>,
Yu Zhang <yu.c.zhang@...ux.intel.com>,
"Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
luto@...nel.org, jun.nakajima@...el.com, dave.hansen@...el.com,
ak@...ux.intel.com, david@...hat.com, aarcange@...hat.com,
ddutile@...hat.com, dhildenb@...hat.com,
Quentin Perret <qperret@...gle.com>, tabba@...gle.com,
mhocko@...e.com, wei.w.wang@...el.com
Subject: Re: [PATCH v10 0/9] KVM: mm: fd-based approach for supporting KVM
> Hi Sean,
>
> We've rebased the SEV+SNP support onto your updated UPM base support
> tree and things seem to be working okay, but we needed some fixups on
> top of the base support get things working, along with 1 workaround
> for an issue that hasn't been root-caused yet:
>
> https://github.com/mdroth/linux/commits/upmv10b-host-snp-v8-wip
>
> *stash (upm_base_support): mm: restrictedmem: Kirill's pinning implementation
> *workaround (use_base_support): mm: restrictedmem: loosen exclusivity check
What I'm seeing is Slot#3 gets added first and then deleted. When it's
gets added, Slot#0 already has the same range bound to restrictedmem so
trigger the exclusive check. This check is exactly the current code for.
> *fixup (upm_base_support): KVM: use inclusive ranges for restrictedmem binding/unbinding
> *fixup (upm_base_support): mm: restrictedmem: use inclusive ranges for issuing invalidations
As many kernel APIs treat 'end' as exclusive, I would rather keep using
exclusive 'end' for these APIs(restrictedmem_bind/restrictedmem_unbind
and notifier callbacks) but fix it internally in the restrictedmem. E.g.
all the places where xarray API needs a 'last'/'max' we use 'end - 1'.
See below for the change.
> *fixup (upm_base_support): KVM: fix restrictedmem GFN range calculations
Subtracting slot->restrictedmem.index for start/end in
restrictedmem_get_gfn_range() is the correct fix.
> *fixup (upm_base_support): KVM: selftests: CoCo compilation fixes
>
> We plan to post an updated RFC for v8 soon, but also wanted to share
> the staging tree in case you end up looking at the UPM integration aspects
> before then.
>
> -Mike
This is the restrictedmem fix to solve 'end' being stored and checked in xarray:
--- a/mm/restrictedmem.c
+++ b/mm/restrictedmem.c
@@ -46,12 +46,12 @@ static long restrictedmem_punch_hole(struct restrictedmem *rm, int mode,
*/
down_read(&rm->lock);
- xa_for_each_range(&rm->bindings, index, notifier, start, end)
+ xa_for_each_range(&rm->bindings, index, notifier, start, end - 1)
notifier->ops->invalidate_start(notifier, start, end);
ret = memfd->f_op->fallocate(memfd, mode, offset, len);
- xa_for_each_range(&rm->bindings, index, notifier, start, end)
+ xa_for_each_range(&rm->bindings, index, notifier, start, end - 1)
notifier->ops->invalidate_end(notifier, start, end);
up_read(&rm->lock);
@@ -224,7 +224,7 @@ static int restricted_error_remove_page(struct address_space *mapping,
}
spin_unlock(&inode->i_lock);
- xa_for_each_range(&rm->bindings, index, notifier, start, end)
+ xa_for_each_range(&rm->bindings, index, notifier, start, end - 1)
notifier->ops->error(notifier, start, end);
break;
}
@@ -301,11 +301,12 @@ int restrictedmem_bind(struct file *file, pgoff_t start, pgoff_t end,
if (exclusive != rm->exclusive)
goto out_unlock;
- if (exclusive && xa_find(&rm->bindings, &start, end, XA_PRESENT))
+ if (exclusive &&
+ xa_find(&rm->bindings, &start, end - 1, XA_PRESENT))
goto out_unlock;
}
- xa_store_range(&rm->bindings, start, end, notifier, GFP_KERNEL);
+ xa_store_range(&rm->bindings, start, end - 1, notifier, GFP_KERNEL);
rm->exclusive = exclusive;
ret = 0;
out_unlock:
@@ -320,7 +321,7 @@ void restrictedmem_unbind(struct file *file, pgoff_t start, pgoff_t end,
struct restrictedmem *rm = file->f_mapping->private_data;
down_write(&rm->lock);
- xa_store_range(&rm->bindings, start, end, NULL, GFP_KERNEL);
+ xa_store_range(&rm->bindings, start, end - 1, NULL, GFP_KERNEL);
synchronize_rcu();
up_write(&rm->lock);
}
Powered by blists - more mailing lists