Re: [PATCH v5 19/22] KVM: arm64: Support SDEI ioctl commands on vCPU

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm



[Index of Archives]     [Linux KVM]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux