by Michael Roth

[permalink] [raw]

On Wed, Jun 28, 2023 at 10:21:32AM -0500,
Michael Roth <[email protected]> wrote:

> On Thu, Jun 22, 2023 at 04:16:29PM -0700, [email protected] wrote:
> > From: Isaku Yamahata <[email protected]>
> >
> > Add flags to strut kvm_gfn_range to indicate who triggered the callback
> > and new memory attributes.
> >
> > TDX needs to know the reason for a callback by kvm_unmap_gfn_range(). mmu
> > notifier, set memory attributes ioctl or KVM gmem callback. With TDX,
> > zapping a private page from the encrypted page table and adding the page
> > back to the same private GPA results in zeroing the page, and the guest has
> > to accept the page again. On the change of memory attribute from private
> > to shared, zapping the GPA range irrespective to private-or-shared and
> > expecting the fault doesn't work for TDX. Instead, zap shared pages only
> > and keep the private pages. Concretely
> > - If it's from mmu notifier, zap shared pages.
> > - If it's from KVM gmem, zap private pages.
> > - If setting memory attributes to private, zap shared pages.
> > - If setting memory attributes to shared, zap private pages.
> >
> > Signed-off-by: Isaku Yamahata <[email protected]>
> >
> > ---
> > Changes v1 -> v2:
> > - consolidate KVM_GFN_RANGE_FLAGS_GMEM_{PUNCH_HOLE, RELEASE} into
> > KVM_GFN_RANGE_FLAGS_GMEM.
> > - Update the commit message to describe TDX more. Drop SEV_SNP.
> > ---
> > include/linux/kvm_host.h | 10 +++++++++-
> > virt/kvm/guest_mem.c | 9 ++++++---
> > virt/kvm/kvm_main.c | 4 +++-
> > 3 files changed, 18 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > index 1a47cedae8a1..1fe0516fcddf 100644
> > --- a/include/linux/kvm_host.h
> > +++ b/include/linux/kvm_host.h
> > @@ -256,12 +256,20 @@ int kvm_async_pf_wakeup_all(struct kvm_vcpu *vcpu);
> > #endif
> >
> > #ifdef CONFIG_KVM_GENERIC_MMU_NOTIFIER
> > +
> > +#define KVM_GFN_RANGE_FLAGS_SET_MEM_ATTR BIT(0)
> > +#define KVM_GFN_RANGE_FLAGS_GMEM BIT(1)
> > +
> > struct kvm_gfn_range {
> > struct kvm_memory_slot *slot;
> > gfn_t start;
> > gfn_t end;
> > - pte_t pte;
> > + union {
> > + pte_t pte;
> > + u64 attrs;
> > + };
> > bool may_block;
> > + unsigned int flags;
> > };
> > bool kvm_unmap_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range);
> > bool kvm_age_gfn(struct kvm *kvm, struct kvm_gfn_range *range);
> > diff --git a/virt/kvm/guest_mem.c b/virt/kvm/guest_mem.c
> > index cdf2d84683c8..387226136960 100644
> > --- a/virt/kvm/guest_mem.c
> > +++ b/virt/kvm/guest_mem.c
> > @@ -99,7 +99,8 @@ static struct folio *kvm_gmem_get_folio(struct file *file, pgoff_t index)
> > }
> >
> > static void kvm_gmem_invalidate_begin(struct kvm *kvm, struct kvm_gmem *gmem,
> > - pgoff_t start, pgoff_t end)
> > + pgoff_t start, pgoff_t end,
> > + unsigned int flags)
> > {
> > struct kvm_memory_slot *slot;
> > unsigned long index;
> > @@ -118,6 +119,7 @@ static void kvm_gmem_invalidate_begin(struct kvm *kvm, struct kvm_gmem *gmem,
> > .slot = slot,
> > .pte = __pte(0),
> > .may_block = true,
> > + .flags = flags,
> > };
> >
> > kvm_mmu_invalidate_range_add(kvm, gfn_range.start, gfn_range.end);
> > @@ -156,7 +158,8 @@ static long kvm_gmem_punch_hole(struct file *file, loff_t offset, loff_t len)
> > */
> > filemap_invalidate_lock(file->f_mapping);
> >
> > - kvm_gmem_invalidate_begin(kvm, gmem, start, end);
> > + kvm_gmem_invalidate_begin(kvm, gmem, start, end,
> > + KVM_GFN_RANGE_FLAGS_GMEM);
>
> Do you anticipate ever needing to pass a different flag via
> kvm_gmem_invalidate_begin()? If not it might make sense to just
> hard-code it rather than passing as a parameter.

I'll update this patch following the suggestion by Sean. [1]
I'll drop the flag. the struct kvm_gfn_range will be

struct kvm_gfn_range {
struct kvm_memory_slot *slot;
gfn_t start;
gfn_t end;
union {
struct test_clear_young_metadata *metadata;
unsigned long attributes;
pte_t pte;
unsigned long callback_arg; /* needs a better name */
};
bool only_private;
bool only_shared;
bool may_block;
};

[1] https://lore.kernel.org/all/[email protected]/
--
Isaku Yamahata <[email protected]>