linux-kernel - Re: [RFC PATCH 0/8] KVM: Prepopulate guest memory API

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240312013208.GD935089@ls.amr.corp.intel.com>
Date: Mon, 11 Mar 2024 18:32:08 -0700
From: Isaku Yamahata <isaku.yamahata@...el.com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Michael Roth <michael.roth@....com>, isaku.yamahata@...el.com,
	kvm@...r.kernel.org, isaku.yamahata@...il.com,
	linux-kernel@...r.kernel.org, Paolo Bonzini <pbonzini@...hat.com>,
	David Matlack <dmatlack@...gle.com>,
	Federico Parola <federico.parola@...ito.it>,
	isaku.yamahata@...ux.intel.com
Subject: Re: [RFC PATCH 0/8] KVM: Prepopulate guest memory API

On Mon, Mar 11, 2024 at 04:44:27PM -0700,
Sean Christopherson <seanjc@...gle.com> wrote:

> On Sun, Mar 10, 2024, Michael Roth wrote:
> > On Fri, Mar 01, 2024 at 09:28:42AM -0800, isaku.yamahata@...el.com wrote:
> > >   struct kvm_sev_launch_update_data {
> > >         __u64 uaddr;
> > >         __u32 len;
> > >   };
> > > 
> > > - TDX and measurement
> > >   The TDX correspondence is TDH.MEM.PAGE.ADD and TDH.MR.EXTEND.  TDH.MEM.EXTEND
> > >   extends its measurement by the page contents.
> > >   Option 1. Add an additional flag like KVM_MEMORY_MAPPING_FLAG_EXTEND to issue
> > >             TDH.MEM.EXTEND
> > >   Option 2. Don't handle extend. Let TDX vendor specific API
> > >             KVM_EMMORY_ENCRYPT_OP to handle it with the subcommand like
> > >             KVM_TDX_EXTEND_MEMORY.
> > 
> > For SNP this happens unconditionally via SNP_LAUNCH_UPDATE, and with some
> > additional measurements via SNP_LAUNCH_FINISH, and down the road when live
> > migration support is added that flow will be a bit different. So
> > personally I think it's better to leave separate for now.
> 
> +1.  The only reason to do EXTEND at the same time as PAGE.ADD would be to
> optimize setups that want the measurement to be extended with the contents of a
> page immediately after the measurement is extended with the mapping metadata for
> said page.  And AFAIK, the only reason to prefer that approach is for backwards
> compatibility, which is not a concern for KVM.  I suppose maaaybe some memory
> locality performance benefits, but that seems like a stretch.
> 
> <time passes>
> 
> And I think this whole conversation is moot, because I don't think there's a need
> to do PAGE.ADD during KVM_MAP_MEMORY[*].  If KVM_MAP_MEMORY does only the SEPT.ADD
> side of things, then both @source (PAGE.ADD) and the EXTEND flag go away.
> 
> > But I'd be hesitant to bake more requirements into this pre-mapping
> > interface, it feels like we're already overloading it as is.
> 
> Agreed.  After being able to think more about this ioctl(), I think KVM_MAP_MEMORY
> should be as "pure" of a mapping operation as we can make it.  It'd be a little
> weird that using KVM_MAP_MEMORY is required for TDX VMs, but not other VMs.  But
> that's really just a reflection of S-EPT, so it's arguably not even a bad thing.
> 
> [*] https://lore.kernel.org/all/Ze-TJh0BBOWm9spT@google.com

Let me give it a try to remove source from struct kvm_memory_mapping. With the
unit in byte instead of page, it will be
  struct kvm_memory_mapping {
        __u64 base_address;
	__u64 size;
	__u64 flags;
  };

SNP won't have any changes.  Always error for KVM_MAP_MEMORY for SNP?
(I'll leave it to Roth.)
TDX will have TDX_INIT_MEM_REGION with new implementation.
-- 
Isaku Yamahata <isaku.yamahata@...el.com>