Introduce the firmware register to hold the vendor specific
hypervisor service calls (owner value 6) as a bitmap. The
bitmap represents the features that'll be enabled for the
guest, as configured by the user-space. Currently, this
includes support for KVM-vendor features along with
reading the UID, represented by bit-0, and Precision Time
Protocol (PTP), represented by bit-1.
Signed-off-by: Raghavendra Rao Ananta <[email protected]>
---
arch/arm64/include/asm/kvm_host.h | 2 ++
arch/arm64/include/uapi/asm/kvm.h | 4 ++++
arch/arm64/kvm/hypercalls.c | 23 ++++++++++++++++++-----
include/kvm/arm_hypercalls.h | 2 ++
4 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 27d4b2a7970e..a025c2ba012a 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -106,10 +106,12 @@ struct kvm_arch_memory_slot {
*
* @std_bmap: Bitmap of standard secure service calls
* @std_hyp_bmap: Bitmap of standard hypervisor service calls
+ * @vendor_hyp_bmap: Bitmap of vendor specific hypervisor service calls
*/
struct kvm_smccc_features {
unsigned long std_bmap;
unsigned long std_hyp_bmap;
+ unsigned long vendor_hyp_bmap;
};
struct kvm_arch {
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index 9eecc7ee8c14..e7d5ae222684 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -344,6 +344,10 @@ struct kvm_arm_copy_mte_tags {
#define KVM_REG_ARM_STD_HYP_BMAP KVM_REG_ARM_FW_FEAT_BMAP_REG(1)
#define KVM_REG_ARM_STD_HYP_BIT_PV_TIME 0
+#define KVM_REG_ARM_VENDOR_HYP_BMAP KVM_REG_ARM_FW_FEAT_BMAP_REG(2)
+#define KVM_REG_ARM_VENDOR_HYP_BIT_FUNC_FEAT 0
+#define KVM_REG_ARM_VENDOR_HYP_BIT_PTP 1
+
/* Device Control API: ARM VGIC */
#define KVM_DEV_ARM_VGIC_GRP_ADDR 0
#define KVM_DEV_ARM_VGIC_GRP_DIST_REGS 1
diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
index f097bebdad39..76e626d0e699 100644
--- a/arch/arm64/kvm/hypercalls.c
+++ b/arch/arm64/kvm/hypercalls.c
@@ -72,9 +72,6 @@ static bool kvm_hvc_call_default_allowed(struct kvm_vcpu *vcpu, u32 func_id)
*/
case ARM_SMCCC_VERSION_FUNC_ID:
case ARM_SMCCC_ARCH_FEATURES_FUNC_ID:
- case ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID:
- case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID:
- case ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID:
return true;
default:
return kvm_psci_func_id_is_valid(vcpu, func_id);
@@ -97,6 +94,13 @@ static bool kvm_hvc_call_allowed(struct kvm_vcpu *vcpu, u32 func_id)
case ARM_SMCCC_HV_PV_TIME_ST:
return kvm_arm_fw_reg_feat_enabled(&smccc_feat->std_hyp_bmap,
KVM_REG_ARM_STD_HYP_BIT_PV_TIME);
+ case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID:
+ case ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID:
+ return kvm_arm_fw_reg_feat_enabled(&smccc_feat->vendor_hyp_bmap,
+ KVM_REG_ARM_VENDOR_HYP_BIT_FUNC_FEAT);
+ case ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID:
+ return kvm_arm_fw_reg_feat_enabled(&smccc_feat->vendor_hyp_bmap,
+ KVM_REG_ARM_VENDOR_HYP_BIT_PTP);
default:
return kvm_hvc_call_default_allowed(vcpu, func_id);
}
@@ -189,8 +193,7 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
val[3] = ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_3;
break;
case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID:
- val[0] = BIT(ARM_SMCCC_KVM_FUNC_FEATURES);
- val[0] |= BIT(ARM_SMCCC_KVM_FUNC_PTP);
+ val[0] = smccc_feat->vendor_hyp_bmap;
break;
case ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID:
kvm_ptp_get_time(vcpu, val);
@@ -217,6 +220,7 @@ static const u64 kvm_arm_fw_reg_ids[] = {
KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_3,
KVM_REG_ARM_STD_BMAP,
KVM_REG_ARM_STD_HYP_BMAP,
+ KVM_REG_ARM_VENDOR_HYP_BMAP,
};
void kvm_arm_init_hypercalls(struct kvm *kvm)
@@ -225,6 +229,7 @@ void kvm_arm_init_hypercalls(struct kvm *kvm)
smccc_feat->std_bmap = KVM_ARM_SMCCC_STD_FEATURES;
smccc_feat->std_hyp_bmap = KVM_ARM_SMCCC_STD_HYP_FEATURES;
+ smccc_feat->vendor_hyp_bmap = KVM_ARM_SMCCC_VENDOR_HYP_FEATURES;
}
int kvm_arm_get_fw_num_regs(struct kvm_vcpu *vcpu)
@@ -317,6 +322,9 @@ int kvm_arm_get_fw_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
case KVM_REG_ARM_STD_HYP_BMAP:
val = READ_ONCE(smccc_feat->std_hyp_bmap);
break;
+ case KVM_REG_ARM_VENDOR_HYP_BMAP:
+ val = READ_ONCE(smccc_feat->vendor_hyp_bmap);
+ break;
default:
return -ENOENT;
}
@@ -343,6 +351,10 @@ static int kvm_arm_set_fw_reg_bmap(struct kvm_vcpu *vcpu, u64 reg_id, u64 val)
fw_reg_bmap = &smccc_feat->std_hyp_bmap;
fw_reg_features = KVM_ARM_SMCCC_STD_HYP_FEATURES;
break;
+ case KVM_REG_ARM_VENDOR_HYP_BMAP:
+ fw_reg_bmap = &smccc_feat->vendor_hyp_bmap;
+ fw_reg_features = KVM_ARM_SMCCC_VENDOR_HYP_FEATURES;
+ break;
default:
return -ENOENT;
}
@@ -445,6 +457,7 @@ int kvm_arm_set_fw_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
return 0;
case KVM_REG_ARM_STD_BMAP:
case KVM_REG_ARM_STD_HYP_BMAP:
+ case KVM_REG_ARM_VENDOR_HYP_BMAP:
return kvm_arm_set_fw_reg_bmap(vcpu, reg->id, val);
default:
return -ENOENT;
diff --git a/include/kvm/arm_hypercalls.h b/include/kvm/arm_hypercalls.h
index aadd6ae3ab72..4ebfdd26e486 100644
--- a/include/kvm/arm_hypercalls.h
+++ b/include/kvm/arm_hypercalls.h
@@ -9,9 +9,11 @@
/* Last valid bits of the bitmapped firmware registers */
#define KVM_REG_ARM_STD_BMAP_BIT_MAX 0
#define KVM_REG_ARM_STD_HYP_BMAP_BIT_MAX 0
+#define KVM_REG_ARM_VENDOR_HYP_BMAP_BIT_MAX 1
#define KVM_ARM_SMCCC_STD_FEATURES GENMASK(KVM_REG_ARM_STD_BMAP_BIT_MAX, 0)
#define KVM_ARM_SMCCC_STD_HYP_FEATURES GENMASK(KVM_REG_ARM_STD_HYP_BMAP_BIT_MAX, 0)
+#define KVM_ARM_SMCCC_VENDOR_HYP_FEATURES GENMASK(KVM_REG_ARM_VENDOR_HYP_BMAP_BIT_MAX, 0)
int kvm_hvc_call_handler(struct kvm_vcpu *vcpu);
--
2.36.0.rc2.479.g8af0fa9b8e-goog
Hi Raghu,
On Fri, Apr 22, 2022 at 5:03 PM Raghavendra Rao Ananta
<[email protected]> wrote:
>
> Introduce the firmware register to hold the vendor specific
> hypervisor service calls (owner value 6) as a bitmap. The
> bitmap represents the features that'll be enabled for the
> guest, as configured by the user-space. Currently, this
> includes support for KVM-vendor features along with
> reading the UID, represented by bit-0, and Precision Time
> Protocol (PTP), represented by bit-1.
>
> Signed-off-by: Raghavendra Rao Ananta <[email protected]>
> ---
> arch/arm64/include/asm/kvm_host.h | 2 ++
> arch/arm64/include/uapi/asm/kvm.h | 4 ++++
> arch/arm64/kvm/hypercalls.c | 23 ++++++++++++++++++-----
> include/kvm/arm_hypercalls.h | 2 ++
> 4 files changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 27d4b2a7970e..a025c2ba012a 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -106,10 +106,12 @@ struct kvm_arch_memory_slot {
> *
> * @std_bmap: Bitmap of standard secure service calls
> * @std_hyp_bmap: Bitmap of standard hypervisor service calls
> + * @vendor_hyp_bmap: Bitmap of vendor specific hypervisor service calls
> */
> struct kvm_smccc_features {
> unsigned long std_bmap;
> unsigned long std_hyp_bmap;
> + unsigned long vendor_hyp_bmap;
> };
>
> struct kvm_arch {
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 9eecc7ee8c14..e7d5ae222684 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -344,6 +344,10 @@ struct kvm_arm_copy_mte_tags {
> #define KVM_REG_ARM_STD_HYP_BMAP KVM_REG_ARM_FW_FEAT_BMAP_REG(1)
> #define KVM_REG_ARM_STD_HYP_BIT_PV_TIME 0
>
> +#define KVM_REG_ARM_VENDOR_HYP_BMAP KVM_REG_ARM_FW_FEAT_BMAP_REG(2)
> +#define KVM_REG_ARM_VENDOR_HYP_BIT_FUNC_FEAT 0
> +#define KVM_REG_ARM_VENDOR_HYP_BIT_PTP 1
> +
> /* Device Control API: ARM VGIC */
> #define KVM_DEV_ARM_VGIC_GRP_ADDR 0
> #define KVM_DEV_ARM_VGIC_GRP_DIST_REGS 1
> diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
> index f097bebdad39..76e626d0e699 100644
> --- a/arch/arm64/kvm/hypercalls.c
> +++ b/arch/arm64/kvm/hypercalls.c
> @@ -72,9 +72,6 @@ static bool kvm_hvc_call_default_allowed(struct kvm_vcpu *vcpu, u32 func_id)
> */
> case ARM_SMCCC_VERSION_FUNC_ID:
> case ARM_SMCCC_ARCH_FEATURES_FUNC_ID:
> - case ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID:
> - case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID:
> - case ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID:
> return true;
> default:
> return kvm_psci_func_id_is_valid(vcpu, func_id);
> @@ -97,6 +94,13 @@ static bool kvm_hvc_call_allowed(struct kvm_vcpu *vcpu, u32 func_id)
> case ARM_SMCCC_HV_PV_TIME_ST:
> return kvm_arm_fw_reg_feat_enabled(&smccc_feat->std_hyp_bmap,
> KVM_REG_ARM_STD_HYP_BIT_PV_TIME);
> + case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID:
> + case ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID:
> + return kvm_arm_fw_reg_feat_enabled(&smccc_feat->vendor_hyp_bmap,
> + KVM_REG_ARM_VENDOR_HYP_BIT_FUNC_FEAT);
> + case ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID:
> + return kvm_arm_fw_reg_feat_enabled(&smccc_feat->vendor_hyp_bmap,
> + KVM_REG_ARM_VENDOR_HYP_BIT_PTP);
> default:
> return kvm_hvc_call_default_allowed(vcpu, func_id);
> }
> @@ -189,8 +193,7 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
> val[3] = ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_3;
> break;
> case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID:
> - val[0] = BIT(ARM_SMCCC_KVM_FUNC_FEATURES);
> - val[0] |= BIT(ARM_SMCCC_KVM_FUNC_PTP);
> + val[0] = smccc_feat->vendor_hyp_bmap;
> break;
> case ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID:
> kvm_ptp_get_time(vcpu, val);
> @@ -217,6 +220,7 @@ static const u64 kvm_arm_fw_reg_ids[] = {
> KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_3,
> KVM_REG_ARM_STD_BMAP,
> KVM_REG_ARM_STD_HYP_BMAP,
> + KVM_REG_ARM_VENDOR_HYP_BMAP,
> };
>
> void kvm_arm_init_hypercalls(struct kvm *kvm)
> @@ -225,6 +229,7 @@ void kvm_arm_init_hypercalls(struct kvm *kvm)
>
> smccc_feat->std_bmap = KVM_ARM_SMCCC_STD_FEATURES;
> smccc_feat->std_hyp_bmap = KVM_ARM_SMCCC_STD_HYP_FEATURES;
> + smccc_feat->vendor_hyp_bmap = KVM_ARM_SMCCC_VENDOR_HYP_FEATURES;
> }
>
> int kvm_arm_get_fw_num_regs(struct kvm_vcpu *vcpu)
> @@ -317,6 +322,9 @@ int kvm_arm_get_fw_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
> case KVM_REG_ARM_STD_HYP_BMAP:
> val = READ_ONCE(smccc_feat->std_hyp_bmap);
> break;
> + case KVM_REG_ARM_VENDOR_HYP_BMAP:
> + val = READ_ONCE(smccc_feat->vendor_hyp_bmap);
> + break;
> default:
> return -ENOENT;
> }
> @@ -343,6 +351,10 @@ static int kvm_arm_set_fw_reg_bmap(struct kvm_vcpu *vcpu, u64 reg_id, u64 val)
> fw_reg_bmap = &smccc_feat->std_hyp_bmap;
> fw_reg_features = KVM_ARM_SMCCC_STD_HYP_FEATURES;
> break;
> + case KVM_REG_ARM_VENDOR_HYP_BMAP:
> + fw_reg_bmap = &smccc_feat->vendor_hyp_bmap;
> + fw_reg_features = KVM_ARM_SMCCC_VENDOR_HYP_FEATURES;
> + break;
> default:
> return -ENOENT;
> }
> @@ -445,6 +457,7 @@ int kvm_arm_set_fw_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
> return 0;
> case KVM_REG_ARM_STD_BMAP:
> case KVM_REG_ARM_STD_HYP_BMAP:
> + case KVM_REG_ARM_VENDOR_HYP_BMAP:
> return kvm_arm_set_fw_reg_bmap(vcpu, reg->id, val);
> default:
> return -ENOENT;
> diff --git a/include/kvm/arm_hypercalls.h b/include/kvm/arm_hypercalls.h
> index aadd6ae3ab72..4ebfdd26e486 100644
> --- a/include/kvm/arm_hypercalls.h
> +++ b/include/kvm/arm_hypercalls.h
> @@ -9,9 +9,11 @@
> /* Last valid bits of the bitmapped firmware registers */
> #define KVM_REG_ARM_STD_BMAP_BIT_MAX 0
> #define KVM_REG_ARM_STD_HYP_BMAP_BIT_MAX 0
> +#define KVM_REG_ARM_VENDOR_HYP_BMAP_BIT_MAX 1
Nit: IMHO perhaps it might be more convenient to define the MAX macro
in arch/arm64/include/uapi/asm/kvm.h like below for maintenance ?
(The same comments are applied to other KVM_REG_ARM_*_BMAP_BIT_MAX)
#define KVM_REG_ARM_VENDOR_HYP_BIT_MAX KVM_REG_ARM_VENDOR_HYP_BIT_PTP
Thanks,
Reiji
>
> #define KVM_ARM_SMCCC_STD_FEATURES GENMASK(KVM_REG_ARM_STD_BMAP_BIT_MAX, 0)
> #define KVM_ARM_SMCCC_STD_HYP_FEATURES GENMASK(KVM_REG_ARM_STD_HYP_BMAP_BIT_MAX, 0)
> +#define KVM_ARM_SMCCC_VENDOR_HYP_FEATURES GENMASK(KVM_REG_ARM_VENDOR_HYP_BMAP_BIT_MAX, 0)
>
> int kvm_hvc_call_handler(struct kvm_vcpu *vcpu);
>
> --
> 2.36.0.rc2.479.g8af0fa9b8e-goog
>
Hi Reiji,
On Sun, Apr 24, 2022 at 11:22 PM Reiji Watanabe <[email protected]> wrote:
>
> Hi Raghu,
>
> On Fri, Apr 22, 2022 at 5:03 PM Raghavendra Rao Ananta
> <[email protected]> wrote:
> >
> > Introduce the firmware register to hold the vendor specific
> > hypervisor service calls (owner value 6) as a bitmap. The
> > bitmap represents the features that'll be enabled for the
> > guest, as configured by the user-space. Currently, this
> > includes support for KVM-vendor features along with
> > reading the UID, represented by bit-0, and Precision Time
> > Protocol (PTP), represented by bit-1.
> >
> > Signed-off-by: Raghavendra Rao Ananta <[email protected]>
> > ---
> > arch/arm64/include/asm/kvm_host.h | 2 ++
> > arch/arm64/include/uapi/asm/kvm.h | 4 ++++
> > arch/arm64/kvm/hypercalls.c | 23 ++++++++++++++++++-----
> > include/kvm/arm_hypercalls.h | 2 ++
> > 4 files changed, 26 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > index 27d4b2a7970e..a025c2ba012a 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -106,10 +106,12 @@ struct kvm_arch_memory_slot {
> > *
> > * @std_bmap: Bitmap of standard secure service calls
> > * @std_hyp_bmap: Bitmap of standard hypervisor service calls
> > + * @vendor_hyp_bmap: Bitmap of vendor specific hypervisor service calls
> > */
> > struct kvm_smccc_features {
> > unsigned long std_bmap;
> > unsigned long std_hyp_bmap;
> > + unsigned long vendor_hyp_bmap;
> > };
> >
> > struct kvm_arch {
> > diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> > index 9eecc7ee8c14..e7d5ae222684 100644
> > --- a/arch/arm64/include/uapi/asm/kvm.h
> > +++ b/arch/arm64/include/uapi/asm/kvm.h
> > @@ -344,6 +344,10 @@ struct kvm_arm_copy_mte_tags {
> > #define KVM_REG_ARM_STD_HYP_BMAP KVM_REG_ARM_FW_FEAT_BMAP_REG(1)
> > #define KVM_REG_ARM_STD_HYP_BIT_PV_TIME 0
> >
> > +#define KVM_REG_ARM_VENDOR_HYP_BMAP KVM_REG_ARM_FW_FEAT_BMAP_REG(2)
> > +#define KVM_REG_ARM_VENDOR_HYP_BIT_FUNC_FEAT 0
> > +#define KVM_REG_ARM_VENDOR_HYP_BIT_PTP 1
> > +
> > /* Device Control API: ARM VGIC */
> > #define KVM_DEV_ARM_VGIC_GRP_ADDR 0
> > #define KVM_DEV_ARM_VGIC_GRP_DIST_REGS 1
> > diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
> > index f097bebdad39..76e626d0e699 100644
> > --- a/arch/arm64/kvm/hypercalls.c
> > +++ b/arch/arm64/kvm/hypercalls.c
> > @@ -72,9 +72,6 @@ static bool kvm_hvc_call_default_allowed(struct kvm_vcpu *vcpu, u32 func_id)
> > */
> > case ARM_SMCCC_VERSION_FUNC_ID:
> > case ARM_SMCCC_ARCH_FEATURES_FUNC_ID:
> > - case ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID:
> > - case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID:
> > - case ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID:
> > return true;
> > default:
> > return kvm_psci_func_id_is_valid(vcpu, func_id);
> > @@ -97,6 +94,13 @@ static bool kvm_hvc_call_allowed(struct kvm_vcpu *vcpu, u32 func_id)
> > case ARM_SMCCC_HV_PV_TIME_ST:
> > return kvm_arm_fw_reg_feat_enabled(&smccc_feat->std_hyp_bmap,
> > KVM_REG_ARM_STD_HYP_BIT_PV_TIME);
> > + case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID:
> > + case ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID:
> > + return kvm_arm_fw_reg_feat_enabled(&smccc_feat->vendor_hyp_bmap,
> > + KVM_REG_ARM_VENDOR_HYP_BIT_FUNC_FEAT);
> > + case ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID:
> > + return kvm_arm_fw_reg_feat_enabled(&smccc_feat->vendor_hyp_bmap,
> > + KVM_REG_ARM_VENDOR_HYP_BIT_PTP);
> > default:
> > return kvm_hvc_call_default_allowed(vcpu, func_id);
> > }
> > @@ -189,8 +193,7 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
> > val[3] = ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_3;
> > break;
> > case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID:
> > - val[0] = BIT(ARM_SMCCC_KVM_FUNC_FEATURES);
> > - val[0] |= BIT(ARM_SMCCC_KVM_FUNC_PTP);
> > + val[0] = smccc_feat->vendor_hyp_bmap;
> > break;
> > case ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID:
> > kvm_ptp_get_time(vcpu, val);
> > @@ -217,6 +220,7 @@ static const u64 kvm_arm_fw_reg_ids[] = {
> > KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_3,
> > KVM_REG_ARM_STD_BMAP,
> > KVM_REG_ARM_STD_HYP_BMAP,
> > + KVM_REG_ARM_VENDOR_HYP_BMAP,
> > };
> >
> > void kvm_arm_init_hypercalls(struct kvm *kvm)
> > @@ -225,6 +229,7 @@ void kvm_arm_init_hypercalls(struct kvm *kvm)
> >
> > smccc_feat->std_bmap = KVM_ARM_SMCCC_STD_FEATURES;
> > smccc_feat->std_hyp_bmap = KVM_ARM_SMCCC_STD_HYP_FEATURES;
> > + smccc_feat->vendor_hyp_bmap = KVM_ARM_SMCCC_VENDOR_HYP_FEATURES;
> > }
> >
> > int kvm_arm_get_fw_num_regs(struct kvm_vcpu *vcpu)
> > @@ -317,6 +322,9 @@ int kvm_arm_get_fw_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
> > case KVM_REG_ARM_STD_HYP_BMAP:
> > val = READ_ONCE(smccc_feat->std_hyp_bmap);
> > break;
> > + case KVM_REG_ARM_VENDOR_HYP_BMAP:
> > + val = READ_ONCE(smccc_feat->vendor_hyp_bmap);
> > + break;
> > default:
> > return -ENOENT;
> > }
> > @@ -343,6 +351,10 @@ static int kvm_arm_set_fw_reg_bmap(struct kvm_vcpu *vcpu, u64 reg_id, u64 val)
> > fw_reg_bmap = &smccc_feat->std_hyp_bmap;
> > fw_reg_features = KVM_ARM_SMCCC_STD_HYP_FEATURES;
> > break;
> > + case KVM_REG_ARM_VENDOR_HYP_BMAP:
> > + fw_reg_bmap = &smccc_feat->vendor_hyp_bmap;
> > + fw_reg_features = KVM_ARM_SMCCC_VENDOR_HYP_FEATURES;
> > + break;
> > default:
> > return -ENOENT;
> > }
> > @@ -445,6 +457,7 @@ int kvm_arm_set_fw_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
> > return 0;
> > case KVM_REG_ARM_STD_BMAP:
> > case KVM_REG_ARM_STD_HYP_BMAP:
> > + case KVM_REG_ARM_VENDOR_HYP_BMAP:
> > return kvm_arm_set_fw_reg_bmap(vcpu, reg->id, val);
> > default:
> > return -ENOENT;
> > diff --git a/include/kvm/arm_hypercalls.h b/include/kvm/arm_hypercalls.h
> > index aadd6ae3ab72..4ebfdd26e486 100644
> > --- a/include/kvm/arm_hypercalls.h
> > +++ b/include/kvm/arm_hypercalls.h
> > @@ -9,9 +9,11 @@
> > /* Last valid bits of the bitmapped firmware registers */
> > #define KVM_REG_ARM_STD_BMAP_BIT_MAX 0
> > #define KVM_REG_ARM_STD_HYP_BMAP_BIT_MAX 0
> > +#define KVM_REG_ARM_VENDOR_HYP_BMAP_BIT_MAX 1
>
> Nit: IMHO perhaps it might be more convenient to define the MAX macro
> in arch/arm64/include/uapi/asm/kvm.h like below for maintenance ?
> (The same comments are applied to other KVM_REG_ARM_*_BMAP_BIT_MAX)
>
> #define KVM_REG_ARM_VENDOR_HYP_BIT_MAX KVM_REG_ARM_VENDOR_HYP_BIT_PTP
>
We have been going back and forth on this :)
It made sense for me to keep it in uapi as well, but I took Oliver's
suggestion of keeping it outside of uapi since this is something that
could be constantly changing [1].
Thank you.
Raghavendra
[1]: https://lore.kernel.org/lkml/CAJHc60wz5WsZWTn66i41+G4-dsjCFuFkthXU_Vf6QeXHkgzrZg@mail.gmail.com/
> Thanks,
> Reiji
>
>
> >
> > #define KVM_ARM_SMCCC_STD_FEATURES GENMASK(KVM_REG_ARM_STD_BMAP_BIT_MAX, 0)
> > #define KVM_ARM_SMCCC_STD_HYP_FEATURES GENMASK(KVM_REG_ARM_STD_HYP_BMAP_BIT_MAX, 0)
> > +#define KVM_ARM_SMCCC_VENDOR_HYP_FEATURES GENMASK(KVM_REG_ARM_VENDOR_HYP_BMAP_BIT_MAX, 0)
> >
> > int kvm_hvc_call_handler(struct kvm_vcpu *vcpu);
> >
> > --
> > 2.36.0.rc2.479.g8af0fa9b8e-goog
> >
On Mon, Apr 25, 2022 at 9:52 AM Raghavendra Rao Ananta
<[email protected]> wrote:
[...]
> > > +#define KVM_REG_ARM_VENDOR_HYP_BMAP_BIT_MAX 1
> >
> > Nit: IMHO perhaps it might be more convenient to define the MAX macro
> > in arch/arm64/include/uapi/asm/kvm.h like below for maintenance ?
> > (The same comments are applied to other KVM_REG_ARM_*_BMAP_BIT_MAX)
> >
> > #define KVM_REG_ARM_VENDOR_HYP_BIT_MAX KVM_REG_ARM_VENDOR_HYP_BIT_PTP
> >
> We have been going back and forth on this :)
> It made sense for me to keep it in uapi as well, but I took Oliver's
> suggestion of keeping it outside of uapi since this is something that
> could be constantly changing [1].
The maximum set of features in a given bitmap register is a property
of the running system, not the headers chosen at compile time. There
is an illusion of ABI breakage when adding new bits to the registers
if we've declared the max bit in UAPI. We also define
KVM_VCPU_MAX_FEATURES outside of UAPI, even though it is related to
the KVM_ARM_VCPU_INIT ioctl.
--
Thanks,
Oliver
On 4/23/22 8:03 AM, Raghavendra Rao Ananta wrote:
> Introduce the firmware register to hold the vendor specific
> hypervisor service calls (owner value 6) as a bitmap. The
> bitmap represents the features that'll be enabled for the
> guest, as configured by the user-space. Currently, this
> includes support for KVM-vendor features along with
> reading the UID, represented by bit-0, and Precision Time
> Protocol (PTP), represented by bit-1.
>
> Signed-off-by: Raghavendra Rao Ananta <[email protected]>
> ---
> arch/arm64/include/asm/kvm_host.h | 2 ++
> arch/arm64/include/uapi/asm/kvm.h | 4 ++++
> arch/arm64/kvm/hypercalls.c | 23 ++++++++++++++++++-----
> include/kvm/arm_hypercalls.h | 2 ++
> 4 files changed, 26 insertions(+), 5 deletions(-)
>
Reviewed-by: Gavin Shan <[email protected]>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 27d4b2a7970e..a025c2ba012a 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -106,10 +106,12 @@ struct kvm_arch_memory_slot {
> *
> * @std_bmap: Bitmap of standard secure service calls
> * @std_hyp_bmap: Bitmap of standard hypervisor service calls
> + * @vendor_hyp_bmap: Bitmap of vendor specific hypervisor service calls
> */
> struct kvm_smccc_features {
> unsigned long std_bmap;
> unsigned long std_hyp_bmap;
> + unsigned long vendor_hyp_bmap;
> };
>
> struct kvm_arch {
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 9eecc7ee8c14..e7d5ae222684 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -344,6 +344,10 @@ struct kvm_arm_copy_mte_tags {
> #define KVM_REG_ARM_STD_HYP_BMAP KVM_REG_ARM_FW_FEAT_BMAP_REG(1)
> #define KVM_REG_ARM_STD_HYP_BIT_PV_TIME 0
>
> +#define KVM_REG_ARM_VENDOR_HYP_BMAP KVM_REG_ARM_FW_FEAT_BMAP_REG(2)
> +#define KVM_REG_ARM_VENDOR_HYP_BIT_FUNC_FEAT 0
> +#define KVM_REG_ARM_VENDOR_HYP_BIT_PTP 1
> +
> /* Device Control API: ARM VGIC */
> #define KVM_DEV_ARM_VGIC_GRP_ADDR 0
> #define KVM_DEV_ARM_VGIC_GRP_DIST_REGS 1
> diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
> index f097bebdad39..76e626d0e699 100644
> --- a/arch/arm64/kvm/hypercalls.c
> +++ b/arch/arm64/kvm/hypercalls.c
> @@ -72,9 +72,6 @@ static bool kvm_hvc_call_default_allowed(struct kvm_vcpu *vcpu, u32 func_id)
> */
> case ARM_SMCCC_VERSION_FUNC_ID:
> case ARM_SMCCC_ARCH_FEATURES_FUNC_ID:
> - case ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID:
> - case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID:
> - case ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID:
> return true;
> default:
> return kvm_psci_func_id_is_valid(vcpu, func_id);
> @@ -97,6 +94,13 @@ static bool kvm_hvc_call_allowed(struct kvm_vcpu *vcpu, u32 func_id)
> case ARM_SMCCC_HV_PV_TIME_ST:
> return kvm_arm_fw_reg_feat_enabled(&smccc_feat->std_hyp_bmap,
> KVM_REG_ARM_STD_HYP_BIT_PV_TIME);
> + case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID:
> + case ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID:
> + return kvm_arm_fw_reg_feat_enabled(&smccc_feat->vendor_hyp_bmap,
> + KVM_REG_ARM_VENDOR_HYP_BIT_FUNC_FEAT);
> + case ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID:
> + return kvm_arm_fw_reg_feat_enabled(&smccc_feat->vendor_hyp_bmap,
> + KVM_REG_ARM_VENDOR_HYP_BIT_PTP);
> default:
> return kvm_hvc_call_default_allowed(vcpu, func_id);
> }
> @@ -189,8 +193,7 @@ int kvm_hvc_call_handler(struct kvm_vcpu *vcpu)
> val[3] = ARM_SMCCC_VENDOR_HYP_UID_KVM_REG_3;
> break;
> case ARM_SMCCC_VENDOR_HYP_KVM_FEATURES_FUNC_ID:
> - val[0] = BIT(ARM_SMCCC_KVM_FUNC_FEATURES);
> - val[0] |= BIT(ARM_SMCCC_KVM_FUNC_PTP);
> + val[0] = smccc_feat->vendor_hyp_bmap;
> break;
> case ARM_SMCCC_VENDOR_HYP_KVM_PTP_FUNC_ID:
> kvm_ptp_get_time(vcpu, val);
> @@ -217,6 +220,7 @@ static const u64 kvm_arm_fw_reg_ids[] = {
> KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_3,
> KVM_REG_ARM_STD_BMAP,
> KVM_REG_ARM_STD_HYP_BMAP,
> + KVM_REG_ARM_VENDOR_HYP_BMAP,
> };
>
> void kvm_arm_init_hypercalls(struct kvm *kvm)
> @@ -225,6 +229,7 @@ void kvm_arm_init_hypercalls(struct kvm *kvm)
>
> smccc_feat->std_bmap = KVM_ARM_SMCCC_STD_FEATURES;
> smccc_feat->std_hyp_bmap = KVM_ARM_SMCCC_STD_HYP_FEATURES;
> + smccc_feat->vendor_hyp_bmap = KVM_ARM_SMCCC_VENDOR_HYP_FEATURES;
> }
>
> int kvm_arm_get_fw_num_regs(struct kvm_vcpu *vcpu)
> @@ -317,6 +322,9 @@ int kvm_arm_get_fw_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
> case KVM_REG_ARM_STD_HYP_BMAP:
> val = READ_ONCE(smccc_feat->std_hyp_bmap);
> break;
> + case KVM_REG_ARM_VENDOR_HYP_BMAP:
> + val = READ_ONCE(smccc_feat->vendor_hyp_bmap);
> + break;
> default:
> return -ENOENT;
> }
> @@ -343,6 +351,10 @@ static int kvm_arm_set_fw_reg_bmap(struct kvm_vcpu *vcpu, u64 reg_id, u64 val)
> fw_reg_bmap = &smccc_feat->std_hyp_bmap;
> fw_reg_features = KVM_ARM_SMCCC_STD_HYP_FEATURES;
> break;
> + case KVM_REG_ARM_VENDOR_HYP_BMAP:
> + fw_reg_bmap = &smccc_feat->vendor_hyp_bmap;
> + fw_reg_features = KVM_ARM_SMCCC_VENDOR_HYP_FEATURES;
> + break;
> default:
> return -ENOENT;
> }
> @@ -445,6 +457,7 @@ int kvm_arm_set_fw_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
> return 0;
> case KVM_REG_ARM_STD_BMAP:
> case KVM_REG_ARM_STD_HYP_BMAP:
> + case KVM_REG_ARM_VENDOR_HYP_BMAP:
> return kvm_arm_set_fw_reg_bmap(vcpu, reg->id, val);
> default:
> return -ENOENT;
> diff --git a/include/kvm/arm_hypercalls.h b/include/kvm/arm_hypercalls.h
> index aadd6ae3ab72..4ebfdd26e486 100644
> --- a/include/kvm/arm_hypercalls.h
> +++ b/include/kvm/arm_hypercalls.h
> @@ -9,9 +9,11 @@
> /* Last valid bits of the bitmapped firmware registers */
> #define KVM_REG_ARM_STD_BMAP_BIT_MAX 0
> #define KVM_REG_ARM_STD_HYP_BMAP_BIT_MAX 0
> +#define KVM_REG_ARM_VENDOR_HYP_BMAP_BIT_MAX 1
>
> #define KVM_ARM_SMCCC_STD_FEATURES GENMASK(KVM_REG_ARM_STD_BMAP_BIT_MAX, 0)
> #define KVM_ARM_SMCCC_STD_HYP_FEATURES GENMASK(KVM_REG_ARM_STD_HYP_BMAP_BIT_MAX, 0)
> +#define KVM_ARM_SMCCC_VENDOR_HYP_FEATURES GENMASK(KVM_REG_ARM_VENDOR_HYP_BMAP_BIT_MAX, 0)
>
> int kvm_hvc_call_handler(struct kvm_vcpu *vcpu);
>
>