On 6/30/20 3:00 AM, Marc Zyngier wrote:
On 2020-06-29 11:32, Mark Rutland wrote:
On Mon, Jun 29, 2020 at 07:18:40PM +1000, Gavin Shan wrote:
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 <gshan@xxxxxxxxxx>
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.
[...]
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.
[...]
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).
Yes, I was reluctant to make the changes regarding the uapi/tracepoint,
which is part of the ABI. I will drop the changes in v2.
Thanks,
Gavin
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
