[PATCH 0/3] KVM: arm64: Some fixes and code adjustments for pvtime ST

During picking up pvtime LPT support for arm64, I do some trivial fixes for
pvtime ST.

Keqian Zhu (3):
  KVM: arm64: Some fixes of PV-time interface document
  KVM: uapi: Remove KVM_DEV_TYPE_ARM_PV_TIME in kvm_device_type
  KVM: arm64: Use kvm_write_guest_lock when init stolen time

 Documentation/virt/kvm/arm/pvtime.rst | 6 +++---
 arch/arm64/kvm/pvtime.c               | 6 +-----
 include/uapi/linux/kvm.h              | 2 --
 tools/include/uapi/linux/kvm.h        | 2 --
 4 files changed, 4 insertions(+), 12 deletions(-)

-- 
1.8.3.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

There is a lock version kvm_write_guest. Use it to simplify code.

Signed-off-by: Keqian Zhu <redacted>
---
 arch/arm64/kvm/pvtime.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/arch/arm64/kvm/pvtime.c b/arch/arm64/kvm/pvtime.c
index f7b52ce..2b24e7f 100644
--- a/arch/arm64/kvm/pvtime.c
+++ b/arch/arm64/kvm/pvtime.c

@@ -55,7 +55,6 @@ gpa_t kvm_init_stolen_time(struct kvm_vcpu *vcpu)
 	struct pvclock_vcpu_stolen_time init_values = {};
 	struct kvm *kvm = vcpu->kvm;
 	u64 base = vcpu->arch.steal.base;
-	int idx;
 
 	if (base == GPA_INVALID)
 		return base;

@@ -66,10 +65,7 @@ gpa_t kvm_init_stolen_time(struct kvm_vcpu *vcpu)
 	 */
 	vcpu->arch.steal.steal = 0;
 	vcpu->arch.steal.last_steal = current->sched_info.run_delay;
-
-	idx = srcu_read_lock(&kvm->srcu);
-	kvm_write_guest(kvm, base, &init_values, sizeof(init_values));
-	srcu_read_unlock(&kvm->srcu, idx);
+	kvm_write_guest_lock(kvm, base, &init_values, sizeof(init_values));
 
 	return base;
 }

-- 
1.8.3.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Rename PV_FEATURES tp PV_TIME_FEATURES

Signed-off-by: Keqian Zhu <redacted>
---
 Documentation/virt/kvm/arm/pvtime.rst | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/virt/kvm/arm/pvtime.rst b/Documentation/virt/kvm/arm/pvtime.rst
index 687b60d..94bffe2 100644
--- a/Documentation/virt/kvm/arm/pvtime.rst
+++ b/Documentation/virt/kvm/arm/pvtime.rst

@@ -3,7 +3,7 @@
 Paravirtualized time support for arm64
 ======================================
 
-Arm specification DEN0057/A defines a standard for paravirtualised time
+Arm specification DEN0057/A defines a standard for paravirtualized time
 support for AArch64 guests:
 
 https://developer.arm.com/docs/den0057/a

@@ -19,8 +19,8 @@ Two new SMCCC compatible hypercalls are defined:
 
 These are only available in the SMC64/HVC64 calling convention as
 paravirtualized time is not available to 32 bit Arm guests. The existence of
-the PV_FEATURES hypercall should be probed using the SMCCC 1.1 ARCH_FEATURES
-mechanism before calling it.
+the PV_TIME_FEATURES hypercall should be probed using the SMCCC 1.1
+ARCH_FEATURES mechanism before calling it.
 
 PV_TIME_FEATURES
     ============= ========    ==========

-- 
1.8.3.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

ARM64 PV-time ST is configured by userspace through vCPU attribute,
and KVM_DEV_TYPE_ARM_PV_TIME is unused.

Signed-off-by: Keqian Zhu <redacted>
---
 include/uapi/linux/kvm.h       | 2 --
 tools/include/uapi/linux/kvm.h | 2 --
 2 files changed, 4 deletions(-)

diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 4fdf303..9a6b97e 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h

@@ -1258,8 +1258,6 @@ enum kvm_device_type {
 #define KVM_DEV_TYPE_ARM_VGIC_ITS	KVM_DEV_TYPE_ARM_VGIC_ITS
 	KVM_DEV_TYPE_XIVE,
 #define KVM_DEV_TYPE_XIVE		KVM_DEV_TYPE_XIVE
-	KVM_DEV_TYPE_ARM_PV_TIME,
-#define KVM_DEV_TYPE_ARM_PV_TIME	KVM_DEV_TYPE_ARM_PV_TIME
 	KVM_DEV_TYPE_MAX,
 };
 

diff --git a/tools/include/uapi/linux/kvm.h b/tools/include/uapi/linux/kvm.h
index 4fdf303..9a6b97e 100644
--- a/tools/include/uapi/linux/kvm.h
+++ b/tools/include/uapi/linux/kvm.h

@@ -1258,8 +1258,6 @@ enum kvm_device_type {
 #define KVM_DEV_TYPE_ARM_VGIC_ITS	KVM_DEV_TYPE_ARM_VGIC_ITS
 	KVM_DEV_TYPE_XIVE,
 #define KVM_DEV_TYPE_XIVE		KVM_DEV_TYPE_XIVE
-	KVM_DEV_TYPE_ARM_PV_TIME,
-#define KVM_DEV_TYPE_ARM_PV_TIME	KVM_DEV_TYPE_ARM_PV_TIME
 	KVM_DEV_TYPE_MAX,
 };
 

-- 
1.8.3.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On 2020-08-17 04:37, Keqian Zhu wrote:

No. You can't drop anything from UAPI, used or not. Doing so will
break the compilation of any userspace that, for any reason, references
this value. We cannot reuse this value in the future either, as it would
create a we wouldn't know which device to create.

It is pretty unfortunate that PV time has turned into such a train 
wreck,
but that's what we have now, and it has to stay.

         M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Hi Marc,

On 2020/8/17 15:39, Marc Zyngier wrote:

On 2020-08-17 04:37, Keqian Zhu wrote:

quoted

ARM64 PV-time ST is configured by userspace through vCPU attribute,
and KVM_DEV_TYPE_ARM_PV_TIME is unused.

Signed-off-by: Keqian Zhu <redacted>
---
 include/uapi/linux/kvm.h       | 2 --
 tools/include/uapi/linux/kvm.h | 2 --
 2 files changed, 4 deletions(-)

diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 4fdf303..9a6b97e 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h

@@ -1258,8 +1258,6 @@ enum kvm_device_type {
 #define KVM_DEV_TYPE_ARM_VGIC_ITS    KVM_DEV_TYPE_ARM_VGIC_ITS
     KVM_DEV_TYPE_XIVE,
 #define KVM_DEV_TYPE_XIVE        KVM_DEV_TYPE_XIVE
-    KVM_DEV_TYPE_ARM_PV_TIME,
-#define KVM_DEV_TYPE_ARM_PV_TIME    KVM_DEV_TYPE_ARM_PV_TIME
     KVM_DEV_TYPE_MAX,
 };

diff --git a/tools/include/uapi/linux/kvm.h b/tools/include/uapi/linux/kvm.h
index 4fdf303..9a6b97e 100644
--- a/tools/include/uapi/linux/kvm.h
+++ b/tools/include/uapi/linux/kvm.h

@@ -1258,8 +1258,6 @@ enum kvm_device_type {
 #define KVM_DEV_TYPE_ARM_VGIC_ITS    KVM_DEV_TYPE_ARM_VGIC_ITS
     KVM_DEV_TYPE_XIVE,
 #define KVM_DEV_TYPE_XIVE        KVM_DEV_TYPE_XIVE
-    KVM_DEV_TYPE_ARM_PV_TIME,
-#define KVM_DEV_TYPE_ARM_PV_TIME    KVM_DEV_TYPE_ARM_PV_TIME
     KVM_DEV_TYPE_MAX,
 };

No. You can't drop anything from UAPI, used or not. Doing so will
break the compilation of any userspace that, for any reason, references
this value. We cannot reuse this value in the future either, as it would
create a we wouldn't know which device to create.

It is pretty unfortunate that PV time has turned into such a train wreck,
but that's what we have now, and it has to stay.

Well, I see. It is a sad thing indeed.

Thanks,
Keqian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On Mon, Aug 17, 2020 at 11:37:27AM +0800, Keqian Zhu wrote:

Reviewed-by: Andrew Jones <redacted>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On Mon, Aug 17, 2020 at 11:37:29AM +0800, Keqian Zhu wrote:

Reviewed-by: Andrew Jones <redacted>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Hi Andrew,

On 2020/8/17 16:47, Andrew Jones wrote:

On Mon, Aug 17, 2020 at 11:37:27AM +0800, Keqian Zhu wrote:

quoted

Rename PV_FEATURES tp PV_TIME_FEATURES

Signed-off-by: Keqian Zhu <redacted>
---
 Documentation/virt/kvm/arm/pvtime.rst | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/virt/kvm/arm/pvtime.rst b/Documentation/virt/kvm/arm/pvtime.rst
index 687b60d..94bffe2 100644
--- a/Documentation/virt/kvm/arm/pvtime.rst
+++ b/Documentation/virt/kvm/arm/pvtime.rst

@@ -3,7 +3,7 @@
 Paravirtualized time support for arm64
 ======================================
 
-Arm specification DEN0057/A defines a standard for paravirtualised time
+Arm specification DEN0057/A defines a standard for paravirtualized time
 support for AArch64 guests:
 
 https://developer.arm.com/docs/den0057/a

@@ -19,8 +19,8 @@ Two new SMCCC compatible hypercalls are defined:
 
 These are only available in the SMC64/HVC64 calling convention as
 paravirtualized time is not available to 32 bit Arm guests. The existence of
-the PV_FEATURES hypercall should be probed using the SMCCC 1.1 ARCH_FEATURES
-mechanism before calling it.
+the PV_TIME_FEATURES hypercall should be probed using the SMCCC 1.1
+ARCH_FEATURES mechanism before calling it.
 
 PV_TIME_FEATURES
     ============= ========    ==========

-- 
1.8.3.1

Reviewed-by: Andrew Jones <redacted>

Thanks for your review.

Also It will be very nice if you have time to review the patch series
supporting pvtime LPT.

Thanks,
Keqian

.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On 17/08/2020 04:37, Keqian Zhu wrote:

Rename PV_FEATURES tp PV_TIME_FEATURES

                      ^^
Typos are sadly far too easy in documentation...

Signed-off-by: Keqian Zhu <redacted>

Reviewed-by: Steven Price <steven.price@arm.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On 17/08/2020 09:43, zhukeqian wrote:

Hi Marc,

On 2020/8/17 15:39, Marc Zyngier wrote:

quoted

On 2020-08-17 04:37, Keqian Zhu wrote:

quoted

ARM64 PV-time ST is configured by userspace through vCPU attribute,
and KVM_DEV_TYPE_ARM_PV_TIME is unused.

Signed-off-by: Keqian Zhu <redacted>
---
  include/uapi/linux/kvm.h       | 2 --
  tools/include/uapi/linux/kvm.h | 2 --
  2 files changed, 4 deletions(-)

diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 4fdf303..9a6b97e 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h

@@ -1258,8 +1258,6 @@ enum kvm_device_type {
  #define KVM_DEV_TYPE_ARM_VGIC_ITS    KVM_DEV_TYPE_ARM_VGIC_ITS
      KVM_DEV_TYPE_XIVE,
  #define KVM_DEV_TYPE_XIVE        KVM_DEV_TYPE_XIVE
-    KVM_DEV_TYPE_ARM_PV_TIME,
-#define KVM_DEV_TYPE_ARM_PV_TIME    KVM_DEV_TYPE_ARM_PV_TIME
      KVM_DEV_TYPE_MAX,
  };

diff --git a/tools/include/uapi/linux/kvm.h b/tools/include/uapi/linux/kvm.h
index 4fdf303..9a6b97e 100644
--- a/tools/include/uapi/linux/kvm.h
+++ b/tools/include/uapi/linux/kvm.h

@@ -1258,8 +1258,6 @@ enum kvm_device_type {
  #define KVM_DEV_TYPE_ARM_VGIC_ITS    KVM_DEV_TYPE_ARM_VGIC_ITS
      KVM_DEV_TYPE_XIVE,
  #define KVM_DEV_TYPE_XIVE        KVM_DEV_TYPE_XIVE
-    KVM_DEV_TYPE_ARM_PV_TIME,
-#define KVM_DEV_TYPE_ARM_PV_TIME    KVM_DEV_TYPE_ARM_PV_TIME
      KVM_DEV_TYPE_MAX,
  };

No. You can't drop anything from UAPI, used or not. Doing so will
break the compilation of any userspace that, for any reason, references
this value. We cannot reuse this value in the future either, as it would
create a we wouldn't know which device to create.

It is pretty unfortunate that PV time has turned into such a train wreck,
but that's what we have now, and it has to stay.

Well, I see. It is a sad thing indeed.

Sorry about that, this got refactored so many times I guess I lost track 
of what was actually needed and this hunk remained when it should have 
been removed.

I would hope that I'm the only one who has any userspace code which uses 
this, but I guess we should still be cautious since this has been in 
several releases now.

Steve

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On 17/08/2020 04:37, Keqian Zhu wrote:

There is a lock version kvm_write_guest. Use it to simplify code.

Signed-off-by: Keqian Zhu <redacted>

Reviewed-by: Steven Price <steven.price@arm.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Hi Steven,

On 2020/8/17 17:49, Steven Price wrote:

On 17/08/2020 09:43, zhukeqian wrote:

quoted

Hi Marc,

[...]

quoted

quoted

It is pretty unfortunate that PV time has turned into such a train wreck,
but that's what we have now, and it has to stay.

Well, I see. It is a sad thing indeed.

Sorry about that, this got refactored so many times I guess I lost track of what was actually needed and this hunk remained when it should have been removed.

It's fine :-) , not a serious problem.

I would hope that I'm the only one who has any userspace code which uses this, but I guess we should still be cautious since this has been in several releases now.

OK. For insurance purposes, we ought to ignore this patch to avoid breaking any user-space program.

Steve
.

Thanks,
Keqian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel