Re: [PATCH v5 04/17] KVM: Add KVM_CAP_MEMORY_FAULT_INFO

From: David Matlack <dmatlack@google.com>
To: Sean Christopherson <seanjc@google.com>
Cc: Anish Moorthy <amoorthy@google.com>,
	oliver.upton@linux.dev, kvm@vger.kernel.org,
	kvmarm@lists.linux.dev, pbonzini@redhat.com, maz@kernel.org,
	robert.hoo.linux@gmail.com, jthoughton@google.com,
	ricarkol@google.com, axelrasmussen@google.com, peterx@redhat.com,
	nadav.amit@gmail.com, isaku.yamahata@gmail.com,
	kconsul@linux.vnet.ibm.com
Subject: Re: [PATCH v5 04/17] KVM: Add KVM_CAP_MEMORY_FAULT_INFO
Date: Mon, 16 Oct 2023 10:07:12 -0700	[thread overview]
Message-ID: <CALzav=fX+cCXQBXhxvRx0KZvHP=GdbP88Kvk9pnx=Ndsf9awEw@mail.gmail.com> (raw)
In-Reply-To: <ZSXg2CjvVb0ugikT@google.com>

On Tue, Oct 10, 2023 at 4:40 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Tue, Oct 10, 2023, David Matlack wrote:
> > On Fri, Sep 8, 2023 at 3:30 PM Anish Moorthy <amoorthy@google.com> wrote:
> > >
> > > +::
> > > +       union {
> > > +               /* KVM_SPEC_EXIT_MEMORY_FAULT */
> > > +               struct {
> > > +                       __u64 flags;
> > > +                       __u64 gpa;
> > > +                       __u64 len; /* in bytes */
> >
> > I wonder if `gpa` and `len` should just be replaced with `gfn`.
> >
> > - We don't seem to care about returning an exact `gpa` out to
> > userspace since this series just returns gpa = gfn * PAGE_SIZE out to
> > userspace.
> > - The len we return seems kind of arbitrary. PAGE_SIZE on x86 and
> > vma_pagesize on ARM64. But at the end of the day we're not asking the
> > kernel to fault in any specific length of mapping. We're just asking
> > for gfn-to-pfn for a specific gfn.
> > - I'm not sure userspace will want to do anything with this information.
>
> Extending ABI is tricky.  E.g. if a use case comes along that needs/wants to
> return a range, then we'd need to add a flag and also update userspace to actually
> do the right thing.
>
> The page fault path doesn't need such information because hardware gives a very
> precise faulting address.  But if we ever get to a point where KVM provides info
> for uaccess failures, then we'll likely want to provide the range.  E.g. if a
> uaccess splits a page, on x86, we'd either need to register our own exception
> fixup and use custom uaccess macros (eww), or convice the world that extending
> ex_handler_uaccess() and all of the uaccess macros that they need to provide the
> exact address that failed.

I wonder if userspace might need a precise fault address in some
situations? e.g. If KVM returns -HWPOISON for an access that spans a
page boundary, userspace won't know which is poisoned. Maybe SNP/TDX
need precise fault addresses as well? I don't know enough about how
SNP and TDX plan to use this UAPI.

>
> And for SNP and TDX, I believe the range will be used when the guest uses a
> hardware-vendor-defined hypercall to request conversions between private and
> shared.  Or maybe the plan is to funnel those into KVM_HC_MAP_GPA_RANGE?