| Message ID | 20241024094012.29452-2-shameerali.kolothum.thodi@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: Errata management for VM Live migration | expand |
Hi, On Thu, Oct 24, 2024 at 10:40:10AM +0100, Shameer Kolothum wrote: > +``ARM_SMCCC_VENDOR_HYP_KVM_MIGRN_TARGET_NUM_FUNC_ID`` > +------------------------------------------------------ > + > +Query total number of migration target CPUs the Guest VM will be running during its > +lifetime and version information applicable to the data format used for > +``ARM_SMCCC_VENDOR_HYP_KVM_MIGRN_TARGET_CPUS_FUNC_ID``. The maximum number of targets > +supported is 64. Also the version number supported currently is 1.0. This hypercall > +must be handled by the userspace VMM. > + > ++---------------------+-------------------------------------------------------------+ > +| Presence: | Optional; KVM/ARM64 Guests only | > ++---------------------+-------------------------------------------------------------+ > +| Calling convention: | HVC64 | > ++---------------------+----------+--------------------------------------------------+ > +| Function ID: | (uint32) | 0xC600007D | > ++---------------------+----------+--------------------------------------------------+ > +| Arguments: | None | > ++---------------------+----------+----+---------------------------------------------+ > +| Return Values: | (int64) | R0 | ``NOT_SUPPORTED (-1)`` on error, else | > +| | | | [0-31] total migration targets | > +| | | | [32-63] version number | > ++---------------------+----------+----+---------------------------------------------+ We can't treat a single register as both a signed quantity *and* a full 64 bits of bitfields. Maybe just scrap the version and have this thing either return a negative error or positive quantity of implementations. > +``ARM_SMCCC_VENDOR_HYP_KVM_MIGRN_TARGET_CPUS_FUNC_ID`` > +------------------------------------------------------- > + > +Request migration target CPU information for the Guest VM. The information must be > +provided as per the format described by the version info in > +``ARM_SMCCC_VENDOR_HYP_KVM_MIGRN_TARGET_NUM_FUNC_ID``. At present, we only support > +the below format which corresponds to version 1.0. This hypercall will always be > +preceded by ``ARM_SMCCC_VENDOR_HYP_KVM_MIGRN_TARGET_NUM_FUNC_ID`` and may be > +invoked multiple times to retrieve the total number of target CPUs information > +advertised. This hypercall must be handled by the userspace VMM. > + > +A typical userspace usage scenario will be like below: > + > +1. Receives ``ARM_SMCCC_VENDOR_HYP_KVM_MIGRN_TARGET_NUM_FUNC_ID`` > + > + * Returns total number of migration targets and version number > + * Reset current target index to zero > +2. Receives ``ARM_SMCCC_VENDOR_HYP_KVM_MIGRN_TARGET_CPUS_FUNC_ID`` > + > + * Returns MIDR/REVIDR info in return register fields. Can return up to 4 > + * Update current target index based on returned target info > + * If there are remaining register fields, return zero to indicate the end > + * Repeat step 2 until current target index == total number of migration targets Hmm... I'd rather we make the guest supply the target index of the implementation it wants to discover. Otherwise, the VMM implementation of this hypercall interface is *stateful* and needs to remember to migrate that... > ++---------------------+-------------------------------------------------------------+ > +| Presence: | Optional; KVM/ARM64 Guests only | > ++---------------------+-------------------------------------------------------------+ > +| Calling convention: | HVC64 | > ++---------------------+----------+--------------------------------------------------+ > +| Function ID: | (uint32) | 0xC600007E | > ++---------------------+----------+----+---------------------------------------------+ > +| Arguments: | None | > ++---------------------+----------+----+---------------------------------------------+ > +| Return Values: | (int64) | R0 | ``NOT_SUPPORTED (-1)`` on error, else | > +| | | | [0-31] MIDR, [31-63] REVIDR, else | > +| | | | [0-63] Zero to mark end. | > ++---------------------+----------+----+---------------------------------------------+ Same deal here, x0 needs to always be treated as a signed quantity. How about -1 on error, 0 on success? Then, in the remaining registers: > +| | (uint64) | R1 | [0-31] MIDR, [32-63] REVIDR, else | > +| | | | [0-63] Zero to mark end. | > ++---------------------+----------+----+---------------------------------------------+ Both MIDR_EL1 and REVIDR_EL1 are 64 bit registers in AArch64. So we need to transfer a full 64 bits, even if the top half is _presently_ RES0 in MIDR_EL1. > +| | (uint64) | R2 | [0-31] MIDR, [32-63] REVIDR, else | > +| | | | [0-63] Zero to mark end. | > ++---------------------+----------+----+---------------------------------------------+ > +| | (uint64) | R3 | [0-31] MIDR, [32-63] REVIDR, else | > +| | | | [0-63] Zero to mark end. | > ++---------------------+----------+----+---------------------------------------------+ I think it's fine to have this hypercall return a single MIDR/REVIDR pair at a time. This is not a performance sensitive interface and will only be called a few times at boot. Actually -- what if we crammed everything into a single hypercall? DISCOVER_IMPLEMENTATION_FUNC_ID - Arguments: - arg0: selected implementation index - Return value: - r0: -1 on error, otherwise the maximum possible implementation index - r1: MIDR_EL1 of the selected implementation - r2: REVIDR_EL1 of the selected implementation We're guaranteed at least one CPU implementation of course, so the guest can just start w/ index 0 and iterate from there.
Hi Oliver, > -----Original Message----- > From: Oliver Upton <oliver.upton@linux.dev> > Sent: Friday, October 25, 2024 2:25 AM > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> > Cc: kvmarm@lists.linux.dev; maz@kernel.org; catalin.marinas@arm.com; > will@kernel.org; mark.rutland@arm.com; cohuck@redhat.com; > eric.auger@redhat.com; yuzenghui <yuzenghui@huawei.com>; Wangzhou > (B) <wangzhou1@hisilicon.com>; jiangkunkun <jiangkunkun@huawei.com>; > Jonathan Cameron <jonathan.cameron@huawei.com>; Anthony Jebson > <anthony.jebson@huawei.com>; linux-arm-kernel@lists.infradead.org; > Linuxarm <linuxarm@huawei.com> > Subject: Re: [RFC PATCH v2 1/3] KVM: arm64: Add hypercall support for > retrieving migration targets > > Hi, > > On Thu, Oct 24, 2024 at 10:40:10AM +0100, Shameer Kolothum wrote: > > +``ARM_SMCCC_VENDOR_HYP_KVM_MIGRN_TARGET_NUM_FUNC_ID`` > > +------------------------------------------------------ > > + > > +Query total number of migration target CPUs the Guest VM will be > running during its > > +lifetime and version information applicable to the data format used for > > +``ARM_SMCCC_VENDOR_HYP_KVM_MIGRN_TARGET_CPUS_FUNC_ID``. > The maximum number of targets > > +supported is 64. Also the version number supported currently is 1.0. This > hypercall > > +must be handled by the userspace VMM. > > + > > ++---------------------+-------------------------------------------------------------+ > > +| Presence: | Optional; KVM/ARM64 Guests only | > > ++---------------------+-------------------------------------------------------------+ > > +| Calling convention: | HVC64 | > > ++---------------------+----------+--------------------------------------------------+ > > +| Function ID: | (uint32) | 0xC600007D | > > ++---------------------+----------+--------------------------------------------------+ > > +| Arguments: | None | > > ++---------------------+----------+----+---------------------------------------------+ > > +| Return Values: | (int64) | R0 | ``NOT_SUPPORTED (-1)`` on error, > else | > > +| | | | [0-31] total migration targets | > > +| | | | [32-63] version number | > > ++---------------------+----------+----+---------------------------------------------+ > > We can't treat a single register as both a signed quantity *and* a full > 64 bits of bitfields. Maybe just scrap the version and have this thing > either return a negative error or positive quantity of implementations. Ok. I had a look at PV_TIME_ST/ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID and got that idea. Separate registers make sense though. Do we really need to skip the version number? The idea was to use that as a future proof for data format in case we realize that MIDR/REVIDR is not good enough for errata later. > > > +``ARM_SMCCC_VENDOR_HYP_KVM_MIGRN_TARGET_CPUS_FUNC_ID`` > > +------------------------------------------------------- > > + > > +Request migration target CPU information for the Guest VM. The > information must be > > +provided as per the format described by the version info in > > +``ARM_SMCCC_VENDOR_HYP_KVM_MIGRN_TARGET_NUM_FUNC_ID``. > At present, we only support > > +the below format which corresponds to version 1.0. This hypercall will > always be > > +preceded by > ``ARM_SMCCC_VENDOR_HYP_KVM_MIGRN_TARGET_NUM_FUNC_ID`` and > may be > > +invoked multiple times to retrieve the total number of target CPUs > information > > +advertised. This hypercall must be handled by the userspace VMM. > > + > > +A typical userspace usage scenario will be like below: > > + > > +1. Receives > ``ARM_SMCCC_VENDOR_HYP_KVM_MIGRN_TARGET_NUM_FUNC_ID`` > > + > > + * Returns total number of migration targets and version number > > + * Reset current target index to zero > > +2. Receives > ``ARM_SMCCC_VENDOR_HYP_KVM_MIGRN_TARGET_CPUS_FUNC_ID`` > > + > > + * Returns MIDR/REVIDR info in return register fields. Can return up to > 4 > > + * Update current target index based on returned target info > > + * If there are remaining register fields, return zero to indicate the end > > + * Repeat step 2 until current target index == total number of migration > targets > > Hmm... I'd rather we make the guest supply the target index of the > implementation it wants to discover. Otherwise, the VMM implementation > of this hypercall interface is *stateful* and needs to remember to > migrate that... Ok. I had the same feedback from Jonathan in an internal discussion as well and on re-reading Marc's comments on v1, he also had the same suggestion that I missed. Will change that. > > ++---------------------+-------------------------------------------------------------+ > > +| Presence: | Optional; KVM/ARM64 Guests only | > > ++---------------------+-------------------------------------------------------------+ > > +| Calling convention: | HVC64 | > > ++---------------------+----------+--------------------------------------------------+ > > +| Function ID: | (uint32) | 0xC600007E | > > ++---------------------+----------+----+---------------------------------------------+ > > +| Arguments: | None | > > ++---------------------+----------+----+---------------------------------------------+ > > +| Return Values: | (int64) | R0 | ``NOT_SUPPORTED (-1)`` on error, > else | > > +| | | | [0-31] MIDR, [31-63] REVIDR, else | > > +| | | | [0-63] Zero to mark end. | > > ++---------------------+----------+----+---------------------------------------------+ > > Same deal here, x0 needs to always be treated as a signed quantity. > > How about -1 on error, 0 on success? > > Then, in the remaining registers: > > > +| | (uint64) | R1 | [0-31] MIDR, [32-63] REVIDR, else | > > +| | | | [0-63] Zero to mark end. | > > ++---------------------+----------+----+---------------------------------------------+ > > Both MIDR_EL1 and REVIDR_EL1 are 64 bit registers in AArch64. So we need > to transfer a full 64 bits, even if the top half is _presently_ RES0 in > MIDR_EL1. Ok. > > > +| | (uint64) | R2 | [0-31] MIDR, [32-63] REVIDR, else | > > +| | | | [0-63] Zero to mark end. | > > ++---------------------+----------+----+---------------------------------------------+ > > +| | (uint64) | R3 | [0-31] MIDR, [32-63] REVIDR, else | > > +| | | | [0-63] Zero to mark end. | > > ++---------------------+----------+----+---------------------------------------------+ > > I think it's fine to have this hypercall return a single MIDR/REVIDR > pair at a time. This is not a performance sensitive interface and will > only be called a few times at boot. > > Actually -- what if we crammed everything into a single hypercall? > > DISCOVER_IMPLEMENTATION_FUNC_ID > > - Arguments: > - arg0: selected implementation index > > - Return value: > - r0: -1 on error, otherwise the maximum possible implementation index > - r1: MIDR_EL1 of the selected implementation > - r2: REVIDR_EL1 of the selected implementation > > We're guaranteed at least one CPU implementation of course, so the guest > can just start w/ index 0 and iterate from there. Ok. I will rework based on these and will sent out something soon. Thanks, Shameer
On Tue, Oct 29, 2024 at 04:00:39PM +0000, Shameerali Kolothum Thodi wrote: > > We can't treat a single register as both a signed quantity *and* a full > > 64 bits of bitfields. Maybe just scrap the version and have this thing > > either return a negative error or positive quantity of implementations. > > Ok. I had a look at PV_TIME_ST/ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID > and got that idea. Separate registers make sense though. > > Do we really need to skip the version number? The idea was to use that as a > future proof for data format in case we realize that MIDR/REVIDR is not good > enough for errata later. That is definitely an approach we can take. The alternative I had in mind was that we'd allocate a new function ID if we needed to break ABI to correct shortcomings of the original interface.
diff --git a/Documentation/virt/kvm/arm/hypercalls.rst b/Documentation/virt/kvm/arm/hypercalls.rst index af7bc2c2e0cb..ebd59fb72567 100644 --- a/Documentation/virt/kvm/arm/hypercalls.rst +++ b/Documentation/virt/kvm/arm/hypercalls.rst @@ -142,3 +142,80 @@ region is equal to the memory protection granule advertised by | | | +---------------------------------------------+ | | | | ``INVALID_PARAMETER (-3)`` | +---------------------+----------+----+---------------------------------------------+ + +``ARM_SMCCC_VENDOR_HYP_KVM_MIGRN_TARGET_NUM_FUNC_ID`` +------------------------------------------------------ + +Query total number of migration target CPUs the Guest VM will be running during its +lifetime and version information applicable to the data format used for +``ARM_SMCCC_VENDOR_HYP_KVM_MIGRN_TARGET_CPUS_FUNC_ID``. The maximum number of targets +supported is 64. Also the version number supported currently is 1.0. This hypercall +must be handled by the userspace VMM. + ++---------------------+-------------------------------------------------------------+ +| Presence: | Optional; KVM/ARM64 Guests only | ++---------------------+-------------------------------------------------------------+ +| Calling convention: | HVC64 | ++---------------------+----------+--------------------------------------------------+ +| Function ID: | (uint32) | 0xC600007D | ++---------------------+----------+--------------------------------------------------+ +| Arguments: | None | ++---------------------+----------+----+---------------------------------------------+ +| Return Values: | (int64) | R0 | ``NOT_SUPPORTED (-1)`` on error, else | +| | | | [0-31] total migration targets | +| | | | [32-63] version number | ++---------------------+----------+----+---------------------------------------------+ +| | (uint64) | R1 | Reserved / Must be zero | +| +----------+----+---------------------------------------------+ +| | (uint64) | R2 | Reserved / Must be zero | ++---------------------+----------+----+---------------------------------------------+ +| | (uint64) | R3 | Reserved / Must be zero | ++---------------------+----------+----+---------------------------------------------+ + + +``ARM_SMCCC_VENDOR_HYP_KVM_MIGRN_TARGET_CPUS_FUNC_ID`` +------------------------------------------------------- + +Request migration target CPU information for the Guest VM. The information must be +provided as per the format described by the version info in +``ARM_SMCCC_VENDOR_HYP_KVM_MIGRN_TARGET_NUM_FUNC_ID``. At present, we only support +the below format which corresponds to version 1.0. This hypercall will always be +preceded by ``ARM_SMCCC_VENDOR_HYP_KVM_MIGRN_TARGET_NUM_FUNC_ID`` and may be +invoked multiple times to retrieve the total number of target CPUs information +advertised. This hypercall must be handled by the userspace VMM. + +A typical userspace usage scenario will be like below: + +1. Receives ``ARM_SMCCC_VENDOR_HYP_KVM_MIGRN_TARGET_NUM_FUNC_ID`` + + * Returns total number of migration targets and version number + * Reset current target index to zero +2. Receives ``ARM_SMCCC_VENDOR_HYP_KVM_MIGRN_TARGET_CPUS_FUNC_ID`` + + * Returns MIDR/REVIDR info in return register fields. Can return up to 4 + * Update current target index based on returned target info + * If there are remaining register fields, return zero to indicate the end + * Repeat step 2 until current target index == total number of migration targets + ++---------------------+-------------------------------------------------------------+ +| Presence: | Optional; KVM/ARM64 Guests only | ++---------------------+-------------------------------------------------------------+ +| Calling convention: | HVC64 | ++---------------------+----------+--------------------------------------------------+ +| Function ID: | (uint32) | 0xC600007E | ++---------------------+----------+----+---------------------------------------------+ +| Arguments: | None | ++---------------------+----------+----+---------------------------------------------+ +| Return Values: | (int64) | R0 | ``NOT_SUPPORTED (-1)`` on error, else | +| | | | [0-31] MIDR, [31-63] REVIDR, else | +| | | | [0-63] Zero to mark end. | ++---------------------+----------+----+---------------------------------------------+ +| | (uint64) | R1 | [0-31] MIDR, [32-63] REVIDR, else | +| | | | [0-63] Zero to mark end. | ++---------------------+----------+----+---------------------------------------------+ +| | (uint64) | R2 | [0-31] MIDR, [32-63] REVIDR, else | +| | | | [0-63] Zero to mark end. | ++---------------------+----------+----+---------------------------------------------+ +| | (uint64) | R3 | [0-31] MIDR, [32-63] REVIDR, else | +| | | | [0-63] Zero to mark end. | ++---------------------+----------+----+---------------------------------------------+ diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h index f59099a213d0..df87ce48ae9e 100644 --- a/include/linux/arm-smccc.h +++ b/include/linux/arm-smccc.h @@ -179,6 +179,8 @@ #define ARM_SMCCC_KVM_FUNC_PKVM_RESV_62 62 #define ARM_SMCCC_KVM_FUNC_PKVM_RESV_63 63 /* End of pKVM hypercall range */ +#define ARM_SMCCC_KVM_FUNC_MIGRN_TARGET_NUM 125 +#define ARM_SMCCC_KVM_FUNC_MIGRN_TARGET_CPUS 126 #define ARM_SMCCC_KVM_FUNC_FEATURES_2 127 #define ARM_SMCCC_KVM_NUM_FUNCS 128 @@ -225,6 +227,17 @@ ARM_SMCCC_OWNER_VENDOR_HYP, \ ARM_SMCCC_KVM_FUNC_MMIO_GUARD) +#define ARM_SMCCC_VENDOR_HYP_KVM_MIGRN_TARGET_NUM_FUNC_ID \ + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ + ARM_SMCCC_SMC_64, \ + ARM_SMCCC_OWNER_VENDOR_HYP, \ + ARM_SMCCC_KVM_FUNC_MIGRN_TARGET_NUM) + +#define ARM_SMCCC_VENDOR_HYP_KVM_MIGRN_TARGET_CPUS_FUNC_ID \ + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ + ARM_SMCCC_SMC_64, \ + ARM_SMCCC_OWNER_VENDOR_HYP, \ + ARM_SMCCC_KVM_FUNC_MIGRN_TARGET_CPUS) /* ptp_kvm counter type ID */ #define KVM_PTP_VIRT_COUNTER 0 #define KVM_PTP_PHYS_COUNTER 1
If the VM requires migration to multiple targets, these hypercalls will provide a way to inform the Guest kernel about those targets. These hypercalls must be handled by userspace VMM to provide the required details. In subsequent patches Guest kernel will use these hypercalls to retrieve the target CPU information. Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> --- Documentation/virt/kvm/arm/hypercalls.rst | 77 +++++++++++++++++++++++ include/linux/arm-smccc.h | 13 ++++ 2 files changed, 90 insertions(+)