Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:8 as permitted sender) client-ip=2620:137:e000::3:8;
Date:   Thu, 21 Sep 2023 14:29:20 -0700
In-Reply-To: <363c4ac28af93aa96a52f897d2fe5c7ec013f746.1695327124.git.isaku.yamahata@intel.com>
Mime-Version: 1.0
References: <cover.1695327124.git.isaku.yamahata@intel.com> <363c4ac28af93aa96a52f897d2fe5c7ec013f746.1695327124.git.isaku.yamahata@intel.com>
Message-ID: <ZQy1sDOQAxPyZzjW@google.com>
Subject: Re: [RFC PATCH v2 4/6] KVM: gmem: Add ioctl to inject memory failure
 on guest memfd
From:   Sean Christopherson <seanjc@google.com>
To:     isaku.yamahata@intel.com
Cc:     kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
        isaku.yamahata@gmail.com, Michael Roth <michael.roth@amd.com>,
        Paolo Bonzini <pbonzini@redhat.com>, erdemaktas@google.com,
        Sagi Shahar <sagis@google.com>,
        David Matlack <dmatlack@google.com>,
        Kai Huang <kai.huang@intel.com>,
        Zhi Wang <zhi.wang.linux@gmail.com>, chen.bo@intel.com,
        linux-coco@lists.linux.dev,
        Chao Peng <chao.p.peng@linux.intel.com>,
        Ackerley Tng <ackerleytng@google.com>,
        Vishal Annapurve <vannapurve@google.com>,
        Yuan Yao <yuan.yao@linux.intel.com>,
        Jarkko Sakkinen <jarkko@kernel.org>,
        Xu Yilun <yilun.xu@intel.com>,
        Quentin Perret <qperret@google.com>, wei.w.wang@intel.com,
        Fuad Tabba <tabba@google.com>
Content-Type: text/plain; charset="us-ascii"
Precedence: bulk

On Thu, Sep 21, 2023, isaku.yamahata@intel.com wrote:
> From: Isaku Yamahata <isaku.yamahata@intel.com>
> 
> To test error_remove_page() method of KVM gmem, add a new ioctl to
> inject memory failure based on offset of guest memfd.
> 
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> ---
>  include/uapi/linux/kvm.h |  6 ++++
>  virt/kvm/guest_mem.c     | 68 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 74 insertions(+)
> 
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 65fc983af840..4160614bcc0f 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -2323,4 +2323,10 @@ struct kvm_create_guest_memfd {
>  	__u64 reserved[6];
>  };
>  
> +#define KVM_GUEST_MEMORY_FAILURE	_IOWR(KVMIO,  0xd5, struct kvm_guest_memory_failure)

If we're going to add a KVM ioctl(), my vote is to make it a generic ioctl(), not
something that's specific to guest_memfd().  IIUC, all we need is the PFN, so the
only downside is that it'd require valid memslots.  But the test isn't all that
interesting unless there are memslots, so I don't see that as a negative.

And if we add an ioctl(), it should be conditioned on CONFIG_HWPOISON_INJECT.

An alternative I think we should seriously consider is using the FAULT_INJECTION
framework to poison pages.  We (Google) have plans to utilize fault injection for
other things in KVM, e.g. to inject "failures" on CMPXCHG in atomic SPTE updates
to force KVM down unlikely slow paths.  I don't expect us to get patches posted
until early next year due to priorities, but hell or high water we will get patches
posted at some point.

The fault injection framework might be overkill for injecting memory errors, e.g.
a single ioctl() is definitely simpler to setup, but I suspect it would also be
much more powerful in the long run..