Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Wed, 23 Mar 2022 17:55:10 +0000
From:   Oliver Upton <oupton@google.com>
To:     Gavin Shan <gshan@redhat.com>
Cc:     kvmarm@lists.cs.columbia.edu, maz@kernel.org,
        linux-kernel@vger.kernel.org, eauger@redhat.com,
        shan.gavin@gmail.com, Jonathan.Cameron@huawei.com,
        pbonzini@redhat.com, vkuznets@redhat.com, will@kernel.org
Subject: Re: [PATCH v5 19/22] KVM: arm64: Support SDEI ioctl commands on vCPU
Message-ID: <Yjte/g4yG+gw7vnt@google.com>
References: <20220322080710.51727-1-gshan@redhat.com>
 <20220322080710.51727-20-gshan@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220322080710.51727-20-gshan@redhat.com>
Precedence: bulk

On Tue, Mar 22, 2022 at 04:07:07PM +0800, Gavin Shan wrote:
> This supports ioctl commands on vCPU to manage the various object.
> It's primarily used by VMM to accomplish migration. The ioctl
> commands introduced by this are highlighted as below:
> 
>    * KVM_SDEI_CMD_GET_VCPU_EVENT_COUNT
>      Return the total count of vCPU events, which have been queued
>      on the target vCPU.
> 
>    * KVM_SDEI_CMD_GET_VCPU_EVENT
>    * KVM_SDEI_CMD_SET_VCPU_EVENT
>      Get or set vCPU events.
> 
>    * KVM_SDEI_CMD_GET_VCPU_STATE
>    * KVM_SDEI_CMD_SET_VCPU_STATE
>      Get or set vCPU state.

All of this GET/SET stuff can probably be added to KVM_{GET,SET}_ONE_REG
immediately. Just introduce new registers any time a new event comes
along. The only event we have at the end of this series is the
software-signaled event, with async PF coming later right?

Some special consideration is likely necessary to avoid adding a
register for every u64 chunk of data. I don't think we need to afford
userspace any illusion of granularity with these, and can probably lump
it all under one giant pseudoregister.

>    * KVM_SDEI_CMD_INJECT_EVENT
>      Inject SDEI event.

What events are we going to allow userspace to inject? IIUC, the
software-signaled event is an IPI and really under the control of the
guest. Async PF is entriely under KVM control.

I do agree that having some form of event injection would be great. VM
providers have found it useful to allow users to NMI their VMs when they
get wedged. I just believe that userspace should not be able to trigger
events that have a defined meaning and are under full KVM ownership.

IMO, unless the async PF changes need to go out to userspace, you could
probably skip event injection for now and only worry about SDEI within a
VM.

--
Thanks,
Oliver