Re: [RFC PATCH v2 5/6] KVM: Add flags to struct kvm_gfn_range

[Michael Roth]

On Thu, Jun 22, 2023 at 04:16:29PM -0700, isaku.yamahata@xxxxxxxxx wrote:
> From: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
> 
> 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 <isaku.yamahata@xxxxxxxxx>
> 
> ---
> 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.

-Mike