Messages in this thread | | | Subject | Re: [PATCH v5 19/22] KVM: arm64: Support SDEI ioctl commands on vCPU | From | Gavin Shan <> | Date | Fri, 25 Mar 2022 18:23:14 +0800 |
| |
Hi Oliver,
On 3/25/22 4:37 PM, Oliver Upton wrote: > On Fri, Mar 25, 2022 at 03:59:50PM +0800, Gavin Shan wrote: >> On 3/24/22 1:55 AM, Oliver Upton wrote: >>> 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. >>> >> >> Yes, KVM_{GET,SET}_ONE_REG is the ideal interface for migration. You're >> correct we're only concerned by software signaled event and the one for >> Async PF. >> >> I didn't look into Raghavendra's series deeply. Actually, a lump of >> registers can be avoid after 2048 byte size is specified in its >> encoding. I think 2048 bytes are enough for now since there are >> only two supported events. > > When I had said 'one giant pseudoregister' I actually meant 'one > pseudoregister per event', not all of SDEI into a single > structure. Since most of this is in flux now, it is hard to point > out what/how we should migrate from conversation alone. > > And on the topic of Raghavendra's series, I do not believe it is > required anymore here w/ the removal of shared events, which I'm > strongly in favor of. > > Let's delve deeper into migration on the next series :) >
Ok, Thanks for your clarification about 'one giant pseudoregister'. Lets have more discussion about the migration on next revision. To be more clear, I plan to implement the base functionality, where only the private event is supported. After it reaches into mergeable or merged, we can post the add-on series to support migration.
>> In the future, we probably have varied number of SDEI events to >> be migrated. In that case, we need to add a new bit to the encoding >> of the pseudo system register, so that VMM (QEMU) can support >> variable sized system register and keep reading and writing on >> these registers on migration: >> >> PSEUDO_SDEI_ADDR: 64-bits in width >> PSEUDO_SDEI_DATA: has varied size >> >> PSEUDO_SDEI_ADDR is used to (1) Indicate the size of PSEUDO_SDEI_DATA >> (2) The information to be read/written, for example the (shared/private) >> registered events on VM and vCPU, VCPU state. >> >> PSEUDO_SDEI_DATA is used to (1) Retrieved information or that to be >> written. (2) Flags to indicate current block of information is the >> last one or not. > > I don't think we have sufficient encoding space in the register ID > to allow for arbitrary length registers. Any new registers for SDEI will > need to fit into one of the predefined sizes. Note that we've already > conditioned userspace to handle registers this way and anything else is > an ABI change. >
Ok, I think we need padding to structures of the event, to make the event object aligned to 64-bytes aligned, and VCPU state to 512-bytes aligned. 64-bytes and 128-bytes registers have been supported.
Thanks, Gavin
| |