On 2020-06-29 11:32, Mark Rutland wrote:

On Mon, Jun 29, 2020 at 07:18:40PM +1000, Gavin Shan wrote:

quoted

kvm/arm32 isn't supported since commit 541ad0150ca4 ("arm: Remove
32bit KVM host support"). So HSR isn't meaningful since then. This
renames HSR to ESR accordingly. This shouldn't cause any functional
changes:

   * Rename kvm_vcpu_get_hsr() to kvm_vcpu_get_esr() to make the
     function names self-explanatory.
   * Rename variables from @hsr to @esr to make them self-explanatory.

Signed-off-by: Gavin Shan <redacted>

At a high-level, I agree that we should move to the `esr` naming to
match the architecture and minimize surprise. However, I think there 
are
some ABI changes here, which *are* funcitonal changes, and those need 
to
be avoided.

[...]

quoted

diff --git a/arch/arm64/include/uapi/asm/kvm.h 

b/arch/arm64/include/uapi/asm/kvm.h
index ba85bb23f060..d54345573a88 100644

--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h

@@ -140,7 +140,7 @@ struct kvm_guest_debug_arch {
 };

 struct kvm_debug_exit_arch {
-	__u32 hsr;
+	__u32 esr;
 	__u64 far;	/* used for watchpoints */
 };

This is userspace ABI, and changing this *will* break userspace. This
*is* a functional change.

NAK to this specifically. At best these should be a comment here that
this is naming is legacym but must stay for ABI reasons.

[...]

quoted

diff --git a/arch/arm64/kvm/trace_arm.h b/arch/arm64/kvm/trace_arm.h
index 4c71270cc097..ee4f691b16ff 100644
--- a/arch/arm64/kvm/trace_arm.h
+++ b/arch/arm64/kvm/trace_arm.h

@@ -42,7 +42,7 @@ TRACE_EVENT(kvm_exit,
 		__entry->vcpu_pc		= vcpu_pc;
 	),

-	TP_printk("%s: HSR_EC: 0x%04x (%s), PC: 0x%08lx",
+	TP_printk("%s: ESR_EC: 0x%04x (%s), PC: 0x%08lx",
 		  __print_symbolic(__entry->ret, kvm_arm_exception_type),
 		  __entry->esr_ec,
 		  __print_symbolic(__entry->esr_ec, kvm_arm_exception_class),

Likewise, isn't all the tracepoint format stuff ABI? I'm not 
comfortable
that we can change this.

Tracepoints are ABI, and they cannot change. As it is, this patch
isn't acceptable (the worse offender being the uapi change though).

         M.
-- 
Who you jivin' with that Cosmik Debris?

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