Re: [PATCH] arm64: KVM: debug: Remove spurious inline attributes
Marc Zyngier <marc.zyng...@arm.com> writes: > The debug trapping code is pretty heavy on the "inline" attribute, > but most functions are actually referenced in the sysreg tables, > making the inlining imposible. > > Removing the useless inline qualifier seems the right thing to do, > having verified that the output code is similar. > > Cc: Alex Bennée <alex.ben...@linaro.org> > Signed-off-by: Marc Zyngier <marc.zyng...@arm.com> > --- > arch/arm64/kvm/sys_regs.c | 58 > +++ > 1 file changed, 29 insertions(+), 29 deletions(-) > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index 88adebf..eec3598 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -220,9 +220,9 @@ static bool trap_debug_regs(struct kvm_vcpu *vcpu, > * All writes will set the KVM_ARM64_DEBUG_DIRTY flag to ensure the > * hyp.S code switches between host and guest values in future. > */ > -static inline void reg_to_dbg(struct kvm_vcpu *vcpu, > - struct sys_reg_params *p, > - u64 *dbg_reg) > +static void reg_to_dbg(struct kvm_vcpu *vcpu, > +struct sys_reg_params *p, > +u64 *dbg_reg) > { > u64 val = p->regval; > > @@ -235,18 +235,18 @@ static inline void reg_to_dbg(struct kvm_vcpu *vcpu, > vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY; > } > > -static inline void dbg_to_reg(struct kvm_vcpu *vcpu, > - struct sys_reg_params *p, > - u64 *dbg_reg) > +static void dbg_to_reg(struct kvm_vcpu *vcpu, > +struct sys_reg_params *p, > +u64 *dbg_reg) > { > p->regval = *dbg_reg; > if (p->is_32bit) > p->regval &= 0xUL; > } Christoffer's "register keyword" comments not-withstanding I'd prefer to keep the reg_to_dbg/dbg_to_reg functions as inline because they really are just boilerplate helpers I didn't want to repeat in the actual access functions - although if you've looked at the code I assume that means GCC has been smart about it. > > -static inline bool trap_bvr(struct kvm_vcpu *vcpu, > - struct sys_reg_params *p, > - const struct sys_reg_desc *rd) > +static bool trap_bvr(struct kvm_vcpu *vcpu, > + struct sys_reg_params *p, > + const struct sys_reg_desc *rd) > { > u64 *dbg_reg = >arch.vcpu_debug_state.dbg_bvr[rd->reg]; > > @@ -280,15 +280,15 @@ static int get_bvr(struct kvm_vcpu *vcpu, const struct > sys_reg_desc *rd, > return 0; > } > > -static inline void reset_bvr(struct kvm_vcpu *vcpu, > - const struct sys_reg_desc *rd) > +static void reset_bvr(struct kvm_vcpu *vcpu, > + const struct sys_reg_desc *rd) > { > vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg] = rd->val; > } > > -static inline bool trap_bcr(struct kvm_vcpu *vcpu, > - struct sys_reg_params *p, > - const struct sys_reg_desc *rd) > +static bool trap_bcr(struct kvm_vcpu *vcpu, > + struct sys_reg_params *p, > + const struct sys_reg_desc *rd) > { > u64 *dbg_reg = >arch.vcpu_debug_state.dbg_bcr[rd->reg]; > > @@ -323,15 +323,15 @@ static int get_bcr(struct kvm_vcpu *vcpu, const struct > sys_reg_desc *rd, > return 0; > } > > -static inline void reset_bcr(struct kvm_vcpu *vcpu, > - const struct sys_reg_desc *rd) > +static void reset_bcr(struct kvm_vcpu *vcpu, > + const struct sys_reg_desc *rd) > { > vcpu->arch.vcpu_debug_state.dbg_bcr[rd->reg] = rd->val; > } > > -static inline bool trap_wvr(struct kvm_vcpu *vcpu, > - struct sys_reg_params *p, > - const struct sys_reg_desc *rd) > +static bool trap_wvr(struct kvm_vcpu *vcpu, > + struct sys_reg_params *p, > + const struct sys_reg_desc *rd) > { > u64 *dbg_reg = >arch.vcpu_debug_state.dbg_wvr[rd->reg]; > > @@ -366,15 +366,15 @@ static int get_wvr(struct kvm_vcpu *vcpu, const struct > sys_reg_desc *rd, > return 0; > } > > -static inline void reset_wvr(struct kvm_vcpu *vcpu, > - const struct sys_reg_desc *rd) > +static void reset_wvr(struct kvm_vcpu *vcpu, > + const struct sys_reg_desc *rd) > { > vcpu->arch.vcpu_debug_state.dbg_wvr[rd->reg] = rd->val; >
Re: [PATCH] arm64: KVM: debug: Remove spurious inline attributes
Marc Zyngier <marc.zyng...@arm.com> writes: > On 17/12/15 16:28, Alex Bennée wrote: >> >> Marc Zyngier <marc.zyng...@arm.com> writes: >> >>> The debug trapping code is pretty heavy on the "inline" attribute, >>> but most functions are actually referenced in the sysreg tables, >>> making the inlining imposible. >>> >>> Removing the useless inline qualifier seems the right thing to do, >>> having verified that the output code is similar. >>> >>> Cc: Alex Bennée <alex.ben...@linaro.org> >>> Signed-off-by: Marc Zyngier <marc.zyng...@arm.com> >>> --- >>> arch/arm64/kvm/sys_regs.c | 58 >>> +++ >>> 1 file changed, 29 insertions(+), 29 deletions(-) >>> >>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c >>> index 88adebf..eec3598 100644 >>> --- a/arch/arm64/kvm/sys_regs.c >>> +++ b/arch/arm64/kvm/sys_regs.c >>> @@ -220,9 +220,9 @@ static bool trap_debug_regs(struct kvm_vcpu *vcpu, >>> * All writes will set the KVM_ARM64_DEBUG_DIRTY flag to ensure the >>> * hyp.S code switches between host and guest values in future. >>> */ >>> -static inline void reg_to_dbg(struct kvm_vcpu *vcpu, >>> - struct sys_reg_params *p, >>> - u64 *dbg_reg) >>> +static void reg_to_dbg(struct kvm_vcpu *vcpu, >>> + struct sys_reg_params *p, >>> + u64 *dbg_reg) >>> { >>> u64 val = p->regval; >>> >>> @@ -235,18 +235,18 @@ static inline void reg_to_dbg(struct kvm_vcpu *vcpu, >>> vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY; >>> } >>> >>> -static inline void dbg_to_reg(struct kvm_vcpu *vcpu, >>> - struct sys_reg_params *p, >>> - u64 *dbg_reg) >>> +static void dbg_to_reg(struct kvm_vcpu *vcpu, >>> + struct sys_reg_params *p, >>> + u64 *dbg_reg) >>> { >>> p->regval = *dbg_reg; >>> if (p->is_32bit) >>> p->regval &= 0xUL; >>> } >> >> Christoffer's "register keyword" comments not-withstanding I'd prefer to >> keep the reg_to_dbg/dbg_to_reg functions as inline because they really >> are just boilerplate helpers I didn't want to repeat in the actual >> access functions - although if you've looked at the code I assume that >> means GCC has been smart about it. > > Indeed, GCC is smart enough to directly inline it. In general, GCC is > doing a pretty good job at inlining static functions that are small > enough not to be worth jumping to. These days, "static inline" only > really makes sense in an include file. Fair enough. > >> The rest all make sense. I wonder why I was being so inline happy? >> >> Reviewed-by: Alex Bennée <alex.ben...@linaro.org> > > Thanks, > > M. -- Alex Bennée -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/4] VSOCK: Introduce vhost-vsock.ko
ock(>mutex); > + } > + mutex_unlock(>dev.mutex); > + return 0; > +} > + > +static long vhost_vsock_dev_ioctl(struct file *f, unsigned int ioctl, > + unsigned long arg) > +{ > + struct vhost_vsock *vsock = f->private_data; > + void __user *argp = (void __user *)arg; > + u64 __user *featurep = argp; > + u32 __user *cidp = argp; > + u32 guest_cid; > + u64 features; > + int r; > + > + switch (ioctl) { > + case VHOST_VSOCK_SET_GUEST_CID: > + if (get_user(guest_cid, cidp)) > + return -EFAULT; > + return vhost_vsock_set_cid(vsock, guest_cid); > + case VHOST_GET_FEATURES: > + features = VHOST_VSOCK_FEATURES; > + if (copy_to_user(featurep, , sizeof(features))) > + return -EFAULT; > + return 0; > + case VHOST_SET_FEATURES: > + if (copy_from_user(, featurep, sizeof(features))) > + return -EFAULT; > + return vhost_vsock_set_features(vsock, features); > + default: > + mutex_lock(>dev.mutex); > + r = vhost_dev_ioctl(>dev, ioctl, argp); > + if (r == -ENOIOCTLCMD) > + r = vhost_vring_ioctl(>dev, ioctl, argp); > + else > + vhost_vsock_flush(vsock); > + mutex_unlock(>dev.mutex); > + return r; > + } > +} > + > +static const struct file_operations vhost_vsock_fops = { > + .owner = THIS_MODULE, > + .open = vhost_vsock_dev_open, > + .release= vhost_vsock_dev_release, > + .llseek = noop_llseek, > + .unlocked_ioctl = vhost_vsock_dev_ioctl, > +}; > + > +static struct miscdevice vhost_vsock_misc = { > + .minor = MISC_DYNAMIC_MINOR, > + .name = "vhost-vsock", > + .fops = _vsock_fops, > +}; > + > +static int > +vhost_transport_socket_init(struct vsock_sock *vsk, struct vsock_sock *psk) > +{ > + struct virtio_transport *trans; > + int ret; > + > + ret = virtio_transport_do_socket_init(vsk, psk); > + if (ret) > + return ret; > + > + trans = vsk->trans; > + trans->ops = _ops; > + > + return ret; > +} > + > +static struct vsock_transport vhost_transport = { > + .get_local_cid= vhost_transport_get_local_cid, > + > + .init = vhost_transport_socket_init, > + .destruct = virtio_transport_destruct, > + .release = virtio_transport_release, > + .connect = virtio_transport_connect, > + .shutdown = virtio_transport_shutdown, > + > + .dgram_enqueue= virtio_transport_dgram_enqueue, > + .dgram_dequeue= virtio_transport_dgram_dequeue, > + .dgram_bind = virtio_transport_dgram_bind, > + .dgram_allow = virtio_transport_dgram_allow, > + > + .stream_enqueue = virtio_transport_stream_enqueue, > + .stream_dequeue = virtio_transport_stream_dequeue, > + .stream_has_data = virtio_transport_stream_has_data, > + .stream_has_space = virtio_transport_stream_has_space, > + .stream_rcvhiwat = virtio_transport_stream_rcvhiwat, > + .stream_is_active = virtio_transport_stream_is_active, > + .stream_allow = virtio_transport_stream_allow, > + > + .notify_poll_in = virtio_transport_notify_poll_in, > + .notify_poll_out = virtio_transport_notify_poll_out, > + .notify_recv_init = virtio_transport_notify_recv_init, > + .notify_recv_pre_block= virtio_transport_notify_recv_pre_block, > + .notify_recv_pre_dequeue = virtio_transport_notify_recv_pre_dequeue, > + .notify_recv_post_dequeue = virtio_transport_notify_recv_post_dequeue, > + .notify_send_init = virtio_transport_notify_send_init, > + .notify_send_pre_block= virtio_transport_notify_send_pre_block, > + .notify_send_pre_enqueue = virtio_transport_notify_send_pre_enqueue, > + .notify_send_post_enqueue = virtio_transport_notify_send_post_enqueue, > + > + .set_buffer_size = virtio_transport_set_buffer_size, > + .set_min_buffer_size = virtio_transport_set_min_buffer_size, > + .set_max_buffer_size = virtio_transport_set_max_buffer_size, > + .get_buffer_size = virtio_transport_get_buffer_size, > + .get_min_buffer_size = virtio_transport_get_min_buffer_size, > + .get_max_buffer_size = virtio_transport_get_max_buffer_size, > +}; > + > +static int __init vhost_vsock_init(void) > +{ > + int ret; > + > + ret = vsock_core_init(_transport); > + if (ret < 0) > + return ret; > + return misc_register(_vsock_misc); > +}; > + > +static void __exit vhost_vsock_exit(void) > +{ > + misc_deregister(_vsock_misc); > + vsock_core_exit(); > +}; > + > +module_init(vhost_vsock_init); > +module_exit(vhost_vsock_exit); > +MODULE_LICENSE("GPL v2"); > +MODULE_AUTHOR("Asias He"); > +MODULE_DESCRIPTION("vhost transport for vsock "); > diff --git a/drivers/vhost/vsock.h b/drivers/vhost/vsock.h > new file mode 100644 > index 000..0ddb107 > --- /dev/null > +++ b/drivers/vhost/vsock.h > @@ -0,0 +1,4 @@ > +#ifndef VHOST_VSOCK_H > +#define VHOST_VSOCK_H > +#define VHOST_VSOCK_SET_GUEST_CID _IOW(VHOST_VIRTIO, 0x60, __u32) > +#endif > -- > 2.5.0 -- Alex Bennée -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/4] VSOCK: Add Makefile and Kconfig
Stefan Hajnoczi <stefa...@redhat.com> writes: > From: Asias He <as...@redhat.com> > > Enable virtio-vsock and vhost-vsock. > > Signed-off-by: Asias He <as...@redhat.com> > Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com> > --- > v3: > * Don't put vhost vsock driver into staging > * Add missing Kconfig dependencies (Arnd Bergmann <a...@arndb.de>) > --- > drivers/vhost/Kconfig | 10 ++ > drivers/vhost/Makefile | 4 > net/vmw_vsock/Kconfig | 18 ++ > net/vmw_vsock/Makefile | 2 ++ > 4 files changed, 34 insertions(+) > > diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig > index 533eaf0..a1bb4c2 100644 > --- a/drivers/vhost/Kconfig > +++ b/drivers/vhost/Kconfig > @@ -21,6 +21,16 @@ config VHOST_SCSI > Say M here to enable the vhost_scsi TCM fabric module > for use with virtio-scsi guests > > +config VHOST_VSOCK > + tristate "vhost virtio-vsock driver" > + depends on VSOCKETS && EVENTFD > + select VIRTIO_VSOCKETS_COMMON > + select VHOST > + select VHOST_RING > + default n > + ---help--- > + Say M here to enable the vhost-vsock for virtio-vsock guests I think checkpatch prefers a few more words for the feature but I'm happy with it. > + > config VHOST_RING > tristate > ---help--- > diff --git a/drivers/vhost/Makefile b/drivers/vhost/Makefile > index e0441c3..6b012b9 100644 > --- a/drivers/vhost/Makefile > +++ b/drivers/vhost/Makefile > @@ -4,5 +4,9 @@ vhost_net-y := net.o > obj-$(CONFIG_VHOST_SCSI) += vhost_scsi.o > vhost_scsi-y := scsi.o > > +obj-$(CONFIG_VHOST_VSOCK) += vhost_vsock.o > +vhost_vsock-y := vsock.o > + > obj-$(CONFIG_VHOST_RING) += vringh.o > + > obj-$(CONFIG_VHOST) += vhost.o > diff --git a/net/vmw_vsock/Kconfig b/net/vmw_vsock/Kconfig > index 14810ab..74e0bc8 100644 > --- a/net/vmw_vsock/Kconfig > +++ b/net/vmw_vsock/Kconfig > @@ -26,3 +26,21 @@ config VMWARE_VMCI_VSOCKETS > > To compile this driver as a module, choose M here: the module > will be called vmw_vsock_vmci_transport. If unsure, say N. > + > +config VIRTIO_VSOCKETS > + tristate "virtio transport for Virtual Sockets" > + depends on VSOCKETS && VIRTIO > + select VIRTIO_VSOCKETS_COMMON > + help > + This module implements a virtio transport for Virtual Sockets. > + > + Enable this transport if your Virtual Machine runs on > Qemu/KVM. Is this better worded as: "Enable this transport if your Virtual Machine host supports vsockets over virtio." ? Otherwise: Reviewed-by: Alex Bennée <alex.ben...@linaro.org> > + > + To compile this driver as a module, choose M here: the module > + will be called virtio_vsock_transport. If unsure, say N. > + > +config VIRTIO_VSOCKETS_COMMON > + tristate > + ---help--- > + This option is selected by any driver which needs to access > + the virtio_vsock. > diff --git a/net/vmw_vsock/Makefile b/net/vmw_vsock/Makefile > index 2ce52d7..cf4c294 100644 > --- a/net/vmw_vsock/Makefile > +++ b/net/vmw_vsock/Makefile > @@ -1,5 +1,7 @@ > obj-$(CONFIG_VSOCKETS) += vsock.o > obj-$(CONFIG_VMWARE_VMCI_VSOCKETS) += vmw_vsock_vmci_transport.o > +obj-$(CONFIG_VIRTIO_VSOCKETS) += virtio_transport.o > +obj-$(CONFIG_VIRTIO_VSOCKETS_COMMON) += virtio_transport_common.o > > vsock-y += af_vsock.o vsock_addr.o > > -- > 2.5.0 -- Alex Bennée -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/4] VSOCK: Introduce virtio-vsock.ko
in_buffer_size, > + .get_max_buffer_size = virtio_transport_get_max_buffer_size, > +}; > + > +static int virtio_vsock_probe(struct virtio_device *vdev) > +{ > + vq_callback_t *callbacks[] = { > + virtio_vsock_ctrl_done, > + virtio_vsock_rx_done, > + virtio_vsock_tx_done, > + }; > + const char *names[] = { > + "ctrl", > + "rx", > + "tx", > + }; > + struct virtio_vsock *vsock = NULL; > + u32 guest_cid; > + int ret; > + > + ret = mutex_lock_interruptible(_virtio_vsock_mutex); > + if (ret) > + return ret; > + > + /* Only one virtio-vsock device per guest is supported */ > + if (the_virtio_vsock) { > + ret = -EBUSY; > + goto out; > + } > + > + vsock = kzalloc(sizeof(*vsock), GFP_KERNEL); > + if (!vsock) { > + ret = -ENOMEM; > + goto out; Won't this attempt to kfree a NULL vsock? > + } > + > + vsock->vdev = vdev; > + > + ret = vsock->vdev->config->find_vqs(vsock->vdev, VSOCK_VQ_MAX, > + vsock->vqs, callbacks, names); > + if (ret < 0) > + goto out; > + > + vdev->config->get(vdev, offsetof(struct virtio_vsock_config, guest_cid), > + _cid, sizeof(guest_cid)); > + vsock->guest_cid = le32_to_cpu(guest_cid); > + pr_debug("%s:guest_cid=%d\n", __func__, vsock->guest_cid); > + > + ret = vsock_core_init(_transport); > + if (ret < 0) > + goto out_vqs; > + > + vsock->rx_buf_nr = 0; > + vsock->rx_buf_max_nr = 0; > + > + vdev->priv = the_virtio_vsock = vsock; > + init_waitqueue_head(>queue_wait); > + mutex_init(>tx_lock); > + mutex_init(>rx_lock); > + INIT_WORK(>rx_work, virtio_transport_recv_pkt_work); > + INIT_WORK(>tx_work, virtio_transport_send_pkt_work); > + > + mutex_lock(>rx_lock); > + virtio_vsock_rx_fill(vsock); > + mutex_unlock(>rx_lock); > + > + mutex_unlock(_virtio_vsock_mutex); > + return 0; > + > +out_vqs: > + vsock->vdev->config->del_vqs(vsock->vdev); > +out: > + kfree(vsock); > + mutex_unlock(_virtio_vsock_mutex); > + return ret; > +} > + > +static void virtio_vsock_remove(struct virtio_device *vdev) > +{ > + struct virtio_vsock *vsock = vdev->priv; > + > + mutex_lock(_virtio_vsock_mutex); > + the_virtio_vsock = NULL; > + vsock_core_exit(); > + mutex_unlock(_virtio_vsock_mutex); > + > + kfree(vsock); > +} > + > +static struct virtio_device_id id_table[] = { > + { VIRTIO_ID_VSOCK, VIRTIO_DEV_ANY_ID }, > + { 0 }, > +}; > + > +static unsigned int features[] = { > +}; > + > +static struct virtio_driver virtio_vsock_driver = { > + .feature_table = features, > + .feature_table_size = ARRAY_SIZE(features), > + .driver.name = KBUILD_MODNAME, > + .driver.owner = THIS_MODULE, > + .id_table = id_table, > + .probe = virtio_vsock_probe, > + .remove = virtio_vsock_remove, > +}; > + > +static int __init virtio_vsock_init(void) > +{ > + int ret; > + > + virtio_vsock_workqueue = alloc_workqueue("virtio_vsock", 0, 0); > + if (!virtio_vsock_workqueue) > + return -ENOMEM; > + ret = register_virtio_driver(_vsock_driver); > + if (ret) > + destroy_workqueue(virtio_vsock_workqueue); > + return ret; > +} > + > +static void __exit virtio_vsock_exit(void) > +{ > + unregister_virtio_driver(_vsock_driver); > + destroy_workqueue(virtio_vsock_workqueue); > +} > + > +module_init(virtio_vsock_init); > +module_exit(virtio_vsock_exit); > +MODULE_LICENSE("GPL v2"); > +MODULE_AUTHOR("Asias He"); > +MODULE_DESCRIPTION("virtio transport for vsock"); > +MODULE_DEVICE_TABLE(virtio, id_table); -- Alex Bennée -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/4] VSOCK: Introduce virtio-vsock-common.ko
k_sk(sk); > + struct virtio_transport *trans = vsk->trans; > + int err = 0; > + > + switch (le16_to_cpu(pkt->hdr.op)) { > + case VIRTIO_VSOCK_OP_RW: > + pkt->len = le32_to_cpu(pkt->hdr.len); > + pkt->off = 0; > + pkt->trans = trans; > + > + mutex_lock(>rx_lock); > + virtio_transport_inc_rx_pkt(pkt); > + list_add_tail(>list, >rx_queue); > + mutex_unlock(>rx_lock); > + > + sk->sk_data_ready(sk); > + return err; > + case VIRTIO_VSOCK_OP_CREDIT_UPDATE: > + sk->sk_write_space(sk); > + break; > + case VIRTIO_VSOCK_OP_SHUTDOWN: > + pr_debug("%s: got shutdown\n", __func__); > + if (le32_to_cpu(pkt->hdr.flags) & VIRTIO_VSOCK_SHUTDOWN_RCV) > + vsk->peer_shutdown |= RCV_SHUTDOWN; > + if (le32_to_cpu(pkt->hdr.flags) & VIRTIO_VSOCK_SHUTDOWN_SEND) > + vsk->peer_shutdown |= SEND_SHUTDOWN; > + if (le32_to_cpu(pkt->hdr.flags)) > + sk->sk_state_change(sk); > + break; > + case VIRTIO_VSOCK_OP_RST: > + pr_debug("%s: got rst\n", __func__); > + sock_set_flag(sk, SOCK_DONE); > + vsk->peer_shutdown = SHUTDOWN_MASK; > + if (vsock_stream_has_data(vsk) <= 0) > + sk->sk_state = SS_DISCONNECTING; > + sk->sk_state_change(sk); > + break; > + default: > + err = -EINVAL; > + break; > + } > + > + virtio_transport_free_pkt(pkt); > + return err; > +} > + > +static int > +virtio_transport_send_response(struct vsock_sock *vsk, > +struct virtio_vsock_pkt *pkt) > +{ > + struct virtio_transport *trans = vsk->trans; > + struct virtio_vsock_pkt_info info = { > + .op = VIRTIO_VSOCK_OP_RESPONSE, > + .type = VIRTIO_VSOCK_TYPE_STREAM, > + .remote_cid = le32_to_cpu(pkt->hdr.src_cid), > + .remote_port = le32_to_cpu(pkt->hdr.src_port), > + }; > + > + pr_debug("%s: send_response\n", __func__); > + > + return trans->ops->send_pkt(vsk, ); > +} > + > +/* Handle server socket */ > +static int > +virtio_transport_recv_listen(struct sock *sk, struct virtio_vsock_pkt *pkt) > +{ > + struct vsock_sock *vsk = vsock_sk(sk); > + struct vsock_sock *vchild; > + struct sock *child; > + > + if (le16_to_cpu(pkt->hdr.op) != VIRTIO_VSOCK_OP_REQUEST) { > + virtio_transport_send_reset(vsk, pkt); > + return -EINVAL; > + } > + > + if (sk_acceptq_is_full(sk)) { > + virtio_transport_send_reset(vsk, pkt); > + return -ENOMEM; > + } > + > + pr_debug("%s: create pending\n", __func__); > + child = __vsock_create(sock_net(sk), NULL, sk, GFP_KERNEL, > +sk->sk_type, 0); > + if (!child) { > + virtio_transport_send_reset(vsk, pkt); > + return -ENOMEM; > + } > + > + sk->sk_ack_backlog++; > + > + lock_sock(child); > + > + child->sk_state = SS_CONNECTED; > + > + vchild = vsock_sk(child); > + vsock_addr_init(>local_addr, le32_to_cpu(pkt->hdr.dst_cid), > + le32_to_cpu(pkt->hdr.dst_port)); > + vsock_addr_init(>remote_addr, le32_to_cpu(pkt->hdr.src_cid), > + le32_to_cpu(pkt->hdr.src_port)); > + > + vsock_insert_connected(vchild); > + vsock_enqueue_accept(sk, child); > + virtio_transport_send_response(vchild, pkt); > + > + release_sock(child); > + > + sk->sk_data_ready(sk); > + return 0; > +} > + > +static void virtio_transport_space_update(struct sock *sk, > + struct virtio_vsock_pkt *pkt) > +{ > + struct vsock_sock *vsk = vsock_sk(sk); > + struct virtio_transport *trans = vsk->trans; > + bool space_available; > + > + /* buf_alloc and fwd_cnt is always included in the hdr */ > + mutex_lock(>tx_lock); > + trans->peer_buf_alloc = le32_to_cpu(pkt->hdr.buf_alloc); > + trans->peer_fwd_cnt = le32_to_cpu(pkt->hdr.fwd_cnt); > + space_available = virtio_transport_has_space(vsk); > + mutex_unlock(>tx_lock); > + > + if (space_available) > + sk->sk_write_space(sk); > +} > + > +/* We are under the virtio-vsock's vsock->rx_lock or > + * vhost-vsock's vq->mutex lock */ > +void virtio_transport_recv_pkt(struct virtio_vsock_pkt *pkt) > +{ > + struct virtio_transport *trans; > + struct sockaddr_vm src, dst; > + struct vsock_sock *vsk; > + struct sock *sk; > + > + vsock_addr_init(, le32_to_cpu(pkt->hdr.src_cid), > le32_to_cpu(pkt->hdr.src_port)); > + vsock_addr_init(, le32_to_cpu(pkt->hdr.dst_cid), > le32_to_cpu(pkt->hdr.dst_port)); > + > + virtio_vsock_dumppkt(__func__, pkt); > + > + if (le16_to_cpu(pkt->hdr.type) != VIRTIO_VSOCK_TYPE_STREAM) { > + /* TODO send RST */ TODO's shouldn't make it into final submissions. > + goto free_pkt; > + } > + > + /* The socket must be in connected or bound table > + * otherwise send reset back > + */ > + sk = vsock_find_connected_socket(, ); > + if (!sk) { > + sk = vsock_find_bound_socket(); > + if (!sk) { > + pr_debug("%s: can not find bound_socket\n", __func__); > + virtio_vsock_dumppkt(__func__, pkt); > + /* Ignore this pkt instead of sending reset back */ > + /* TODO send a RST unless this packet is a RST > (to avoid infinite loops) */ Ditto. > + goto free_pkt; > + } > + } > + > + vsk = vsock_sk(sk); > + trans = vsk->trans; > + BUG_ON(!trans); See above re: BUG_ON > + > + virtio_transport_space_update(sk, pkt); > + > + lock_sock(sk); > + switch (sk->sk_state) { > + case VSOCK_SS_LISTEN: > + virtio_transport_recv_listen(sk, pkt); > + virtio_transport_free_pkt(pkt); > + break; > + case SS_CONNECTING: > + virtio_transport_recv_connecting(sk, pkt); > + virtio_transport_free_pkt(pkt); > + break; > + case SS_CONNECTED: > + virtio_transport_recv_connected(sk, pkt); > + break; > + default: > + virtio_transport_free_pkt(pkt); > + break; > + } > + release_sock(sk); > + > + /* Release refcnt obtained when we fetched this socket out of the > + * bound or connected list. > + */ > + sock_put(sk); > + return; > + > +free_pkt: > + virtio_transport_free_pkt(pkt); > +} > +EXPORT_SYMBOL_GPL(virtio_transport_recv_pkt); > + > +void virtio_transport_free_pkt(struct virtio_vsock_pkt *pkt) > +{ > + kfree(pkt->buf); > + kfree(pkt); > +} > +EXPORT_SYMBOL_GPL(virtio_transport_free_pkt); > + > +MODULE_LICENSE("GPL v2"); > +MODULE_AUTHOR("Asias He"); > +MODULE_DESCRIPTION("common code for virtio vsock"); > -- > 2.5.0 -- Alex Bennée -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 3/6] target-arm: kvm - support for single step
Peter Maydell <peter.mayd...@linaro.org> writes: > On 12 November 2015 at 16:20, Alex Bennée <alex.ben...@linaro.org> wrote: >> This adds support for single-step. There isn't much to do on the QEMU >> side as after we set-up the request for single step via the debug ioctl >> it is all handled within the kernel. >> >> Signed-off-by: Alex Bennée <alex.ben...@linaro.org> >> >> --- >> v2 >> - convert to using HSR_EC >> v3 >> - use internals.h definitions >> --- >> target-arm/kvm.c | 10 ++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/target-arm/kvm.c b/target-arm/kvm.c >> index 50f70ef..d505a7e 100644 >> --- a/target-arm/kvm.c >> +++ b/target-arm/kvm.c >> @@ -535,6 +535,13 @@ static int kvm_handle_debug(CPUState *cs, struct >> kvm_run *run) >> kvm_cpu_synchronize_state(cs); >> >> switch (hsr_ec) { >> +case EC_SOFTWARESTEP: >> +if (cs->singlestep_enabled) { >> +return true; >> +} else { >> +error_report("Came out of SINGLE STEP when not enabled"); >> +} >> +break; >> case EC_AA64_BKPT: >> if (kvm_find_sw_breakpoint(cs, env->pc)) { >> return true; >> @@ -595,6 +602,9 @@ int kvm_arch_on_sigbus(int code, void *addr) >> >> void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg) >> { >> +if (cs->singlestep_enabled) { >> +dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_SINGLESTEP; >> +} > > Doesn't kvm_update_guest_debug() already set these bits, or am > I misreading it? Yeah. This raises an interesting problem about what to do when we don't have the capability. I could suppress those bits in the update function but that seems a bit hacky. Looking at the GDB capability code there doesn't seem to report breakpoint capability short of just failing when you try to set one. > >> if (kvm_sw_breakpoints_active(cs)) { >> dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP; >> } >> -- >> 2.6.3 > > thanks > -- PMM -- Alex Bennée -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 6/6] tests/guest-debug: introduce basic gdbstub tests
Peter Maydell <peter.mayd...@linaro.org> writes: > On 12 November 2015 at 16:20, Alex Bennée <alex.ben...@linaro.org> wrote: >> From: Alex Bennée <a...@bennee.com> >> >> The aim of these tests is to combine with an appropriate kernel >> image (with symbol-file vmlinux) and check it behaves as it should. >> Given a kernel it checks: >> >> - single step >> - software breakpoint >> - hardware breakpoint >> - access, read and write watchpoints >> >> On success it returns 0 to the calling process. >> >> I've not plumbed this into the "make check" logic though as we need a >> solution for providing non-host binaries to the tests. However the test >> is structured to work with pretty much any Linux kernel image as it >> uses the basic kernel_init code which is common across architectures. > > Do these tests pass if you run them on the TCG QEMU, just out > of interest? You'll be glad to know they do. > I'm not a great fan of tests that aren't in 'make check' > because IME they just bitrot, but as you say we have no > sensible approach for handling tests that need to run real > guest code :-( I was pondering if a git sub-project with large file support would work. We could add pre-built binaries to the tree with appropriate meta-data (src tree, version, config) to rebuild if required. There would be some degree of trust implied in the original builder though. Maybe a signed commit? > > thanks > -- PMM -- Alex Bennée -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 0/6] QEMU support for KVM Guest Debug on arm64
Hi, Here is the latest patch set to support debugging of KVM guests on arm64. The main changes are fixing arm32 compiles (mostly with stubs for the upcomming arm32 debug) and the usual bunch of minor tweaks and clarifications following review. I've kept the GDB Python based test in tests/guest-debug and cleaned it up so it will work with python2/3 linked GDBs. It still isn't plumbed it in to the "make check" so can be dropped until we have a solution for testing against non-host binaries. So in summary the changes are: - Fixed arm32 compile - Use results of debug capability checks - Whitespace and comment cleanups - Py2/3 cleanliness for test script More detailed changelogs are attached to each patch. GIT Repo: The patch series is based off a recent master and can be found at: https://github.com/stsquad/qemu branch: kvm/guest-debug-v10 Alex Bennée (6): target-arm: kvm64 - introduce kvm_arm_init_debug() target-arm: kvm - implement software breakpoints target-arm: kvm - support for single step target-arm: kvm - add support for HW assisted debug target-arm: kvm - re-inject guest debug exceptions tests/guest-debug: introduce basic gdbstub tests target-arm/helper-a64.c | 12 +- target-arm/kvm.c | 65 +++--- target-arm/kvm32.c| 47 target-arm/kvm64.c| 464 ++ target-arm/kvm_arm.h | 30 +++ tests/guest-debug/test-gdbstub.py | 176 +++ 6 files changed, 757 insertions(+), 37 deletions(-) create mode 100644 tests/guest-debug/test-gdbstub.py -- 2.6.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 5/6] target-arm: kvm - re-inject guest debug exceptions
If we can't find details for the debug exception in our debug state then we can assume the exception is due to debugging inside the guest. To inject the exception into the guest state we re-use the TCG exception code (do_interrupt). However while guest debugging is in effect we currently can't handle the guest using single step as we will keep trapping to back to userspace. GDB makes heavy use of single-step behind the scenes which effectively means the guests ability to debug itself is disabled while it is being debugged. Signed-off-by: Alex Bennée <alex.ben...@linaro.org> --- v5: - new for v5 v10: - fix arm32 compile - add full stop at end of sentance - attempted to expand on limitations in commit msg --- target-arm/helper-a64.c | 12 ++-- target-arm/kvm64.c | 24 +--- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c index deb8dbe..fc3ccdf 100644 --- a/target-arm/helper-a64.c +++ b/target-arm/helper-a64.c @@ -25,6 +25,7 @@ #include "qemu/bitops.h" #include "internals.h" #include "qemu/crc32c.h" +#include "sysemu/kvm.h" #include /* For crc32 */ /* C2.4.7 Multiply and divide */ @@ -469,7 +470,8 @@ void aarch64_cpu_do_interrupt(CPUState *cs) new_el); if (qemu_loglevel_mask(CPU_LOG_INT) && !excp_is_internal(cs->exception_index)) { -qemu_log_mask(CPU_LOG_INT, "...with ESR 0x%" PRIx32 "\n", +qemu_log_mask(CPU_LOG_INT, "...with ESR %x/0x%" PRIx32 "\n", + env->exception.syndrome >> ARM_EL_EC_SHIFT, env->exception.syndrome); } @@ -535,6 +537,12 @@ void aarch64_cpu_do_interrupt(CPUState *cs) aarch64_restore_sp(env, new_el); env->pc = addr; -cs->interrupt_request |= CPU_INTERRUPT_EXITTB; + +qemu_log_mask(CPU_LOG_INT, "...to EL%d PC 0x%" PRIx64 " PSTATE 0x%x\n", + new_el, env->pc, pstate_read(env)); + +if (!kvm_enabled()) { +cs->interrupt_request |= CPU_INTERRUPT_EXITTB; +} } #endif diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c index 771ecdb..8e6d044 100644 --- a/target-arm/kvm64.c +++ b/target-arm/kvm64.c @@ -871,6 +871,7 @@ bool kvm_arm_handle_debug(CPUState *cs, struct kvm_debug_exit_arch *debug_exit) { int hsr_ec = debug_exit->hsr >> ARM_EL_EC_SHIFT; ARMCPU *cpu = ARM_CPU(cs); +CPUClass *cc = CPU_GET_CLASS(cs); CPUARMState *env = >env; /* Ensure PC is synchronised */ @@ -881,7 +882,14 @@ bool kvm_arm_handle_debug(CPUState *cs, struct kvm_debug_exit_arch *debug_exit) if (cs->singlestep_enabled) { return true; } else { -error_report("Came out of SINGLE STEP when not enabled"); +/* + * The kernel should have supressed the guests ability to + * single step at this point so something has gone wrong. + */ +error_report("%s: guest single-step while debugging unsupported" + " (%"PRIx64", %"PRIx32")\n", + __func__, env->pc, debug_exit->hsr); +return false; } break; case EC_AA64_BKPT: @@ -908,12 +916,14 @@ bool kvm_arm_handle_debug(CPUState *cs, struct kvm_debug_exit_arch *debug_exit) __func__, debug_exit->hsr, env->pc); } -/* If we don't handle this it could be it really is for the - guest to handle */ -qemu_log_mask(LOG_UNIMP, - "%s: re-injecting exception not yet implemented" - " (0x%"PRIx32", %"PRIx64")\n", - __func__, hsr_ec, env->pc); +/* If we are not handling the debug exception it must belong to + * the guest. Let's re-use the existing TCG interrupt code to set + * everything up properly. + */ +cs->exception_index = EXCP_BKPT; +env->exception.syndrome = debug_exit->hsr; +env->exception.vaddress = debug_exit->far; +cc->do_interrupt(cs); return false; } -- 2.6.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 1/6] target-arm: kvm64 - introduce kvm_arm_init_debug()
As we haven't always had guest debug support we need to probe for it. Additionally we don't do this in the start-up capability code so we don't fall over on old kernels. Signed-off-by: Alex Bennée <alex.ben...@linaro.org> --- target-arm/kvm64.c | 18 ++ 1 file changed, 18 insertions(+) diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c index ceebfeb..d087794 100644 --- a/target-arm/kvm64.c +++ b/target-arm/kvm64.c @@ -25,6 +25,22 @@ #include "internals.h" #include "hw/arm/arm.h" +static bool have_guest_debug; + +/** + * kvm_arm_init_debug() + * @cs: CPUState + * + * Check for guest debug capabilities. + * + */ +static void kvm_arm_init_debug(CPUState *cs) +{ +have_guest_debug = kvm_check_extension(cs->kvm_state, + KVM_CAP_SET_GUEST_DEBUG); +return; +} + static inline void set_feature(uint64_t *features, int feature) { *features |= 1ULL << feature; @@ -121,6 +137,8 @@ int kvm_arch_init_vcpu(CPUState *cs) } cpu->mp_affinity = mpidr & ARM64_AFFINITY_MASK; +kvm_arm_init_debug(cs); + return kvm_arm_init_cpreg_list(cpu); } -- 2.6.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 6/6] tests/guest-debug: introduce basic gdbstub tests
The aim of these tests is to combine with an appropriate kernel image (with symbol-file vmlinux) and check it behaves as it should. Given a kernel it checks: - single step - software breakpoint - hardware breakpoint - access, read and write watchpoints On success it returns 0 to the calling process. I've not plumbed this into the "make check" logic though as we need a solution for providing non-host binaries to the tests. However the test is structured to work with pretty much any Linux kernel image as it uses the basic kernel_init code which is common across architectures. Signed-off-by: Alex Bennée <alex.ben...@linaro.org> --- v10: - fixup for Py2/3 cleanliness - drop to shell on exception --- tests/guest-debug/test-gdbstub.py | 176 ++ 1 file changed, 176 insertions(+) create mode 100644 tests/guest-debug/test-gdbstub.py diff --git a/tests/guest-debug/test-gdbstub.py b/tests/guest-debug/test-gdbstub.py new file mode 100644 index 000..31ba6c9 --- /dev/null +++ b/tests/guest-debug/test-gdbstub.py @@ -0,0 +1,176 @@ +# +# This script needs to be run on startup +# qemu -kernel ${KERNEL} -s -S +# and then: +# gdb ${KERNEL}.vmlinux -x ${QEMU_SRC}/tests/guest-debug/test-gdbstub.py + +import gdb + +failcount = 0 + + +def report(cond, msg): +"Report success/fail of test" +if cond: +print ("PASS: %s" % (msg)) +else: +print ("FAIL: %s" % (msg)) +failcount += 1 + + +def check_step(): +"Step an instruction, check it moved." +start_pc = gdb.parse_and_eval('$pc') +gdb.execute("si") +end_pc = gdb.parse_and_eval('$pc') + +return not (start_pc == end_pc) + + +def check_break(sym_name): +"Setup breakpoint, continue and check we stopped." +sym, ok = gdb.lookup_symbol(sym_name) +bp = gdb.Breakpoint(sym_name) + +gdb.execute("c") + +# hopefully we came back +end_pc = gdb.parse_and_eval('$pc') +print ("%s == %s %d" % (end_pc, sym.value(), bp.hit_count)) +bp.delete() + +# can we test we hit bp? +return end_pc == sym.value() + + +# We need to do hbreak manually as the python interface doesn't export it +def check_hbreak(sym_name): +"Setup hardware breakpoint, continue and check we stopped." +sym, ok = gdb.lookup_symbol(sym_name) +gdb.execute("hbreak %s" % (sym_name)) +gdb.execute("c") + +# hopefully we came back +end_pc = gdb.parse_and_eval('$pc') +print ("%s == %s" % (end_pc, sym.value())) + +if end_pc == sym.value(): +gdb.execute("d 1") +return True +else: +return False + + +class WatchPoint(gdb.Breakpoint): + +def get_wpstr(self, sym_name): +"Setup sym and wp_str for given symbol." +self.sym, ok = gdb.lookup_symbol(sym_name) +wp_addr = gdb.parse_and_eval(sym_name).address +self.wp_str = '*(%(type)s)(&%(address)s)' % dict( +type = wp_addr.type, address = sym_name) + +return(self.wp_str) + +def __init__(self, sym_name, type): +wp_str = self.get_wpstr(sym_name) +super(WatchPoint, self).__init__(wp_str, gdb.BP_WATCHPOINT, type) + +def stop(self): +end_pc = gdb.parse_and_eval('$pc') +print ("HIT WP @ %s" % (end_pc)) +return True + + +def do_one_watch(sym, wtype, text): + +wp = WatchPoint(sym, wtype) +gdb.execute("c") +report_str = "%s for %s (%s)" % (text, sym, wp.sym.value()) + +if wp.hit_count > 0: +report(True, report_str) +wp.delete() +else: +report(False, report_str) + + +def check_watches(sym_name): +"Watch a symbol for any access." + +# Should hit for any read +do_one_watch(sym_name, gdb.WP_ACCESS, "awatch") + +# Again should hit for reads +do_one_watch(sym_name, gdb.WP_READ, "rwatch") + +# Finally when it is written +do_one_watch(sym_name, gdb.WP_WRITE, "watch") + + +class CatchBreakpoint(gdb.Breakpoint): +def __init__(self, sym_name): +super(CatchBreakpoint, self).__init__(sym_name) +self.sym, ok = gdb.lookup_symbol(sym_name) + +def stop(self): +end_pc = gdb.parse_and_eval('$pc') +print ("CB: %s == %s" % (end_pc, self.sym.value())) +if end_pc == self.sym.value(): +report(False, "Hit final catchpoint") + + +def run_test(): +"Run throught the tests one by one" + +print ("Checking we can step the first few instructions") +step_ok = 0 +for i in range(3): +if check_step(): +step_ok += 1 + +report(step_ok == 3, "single step in boot code") + +print ("Checking HW breakpoint works") +break_ok = check_hbreak("kernel_init") +report(break_ok,
[PATCH v10 3/6] target-arm: kvm - support for single step
This adds support for single-step. There isn't much to do on the QEMU side as after we set-up the request for single step via the debug ioctl it is all handled within the kernel. The actual setting of the KVM_GUESTDBG_SINGLESTEP flag is already in the common code. If the kernel doesn't support guest debug the ioctl will simply error. Signed-off-by: Alex Bennée <alex.ben...@linaro.org> --- v2 - convert to using HSR_EC v3 - use internals.h definitions v10 - fix arm32 build - remove redundent flag setting (done in main kvm.c) - more words on fail case --- target-arm/kvm64.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c index 3b3929d..5f96cde 100644 --- a/target-arm/kvm64.c +++ b/target-arm/kvm64.c @@ -534,6 +534,13 @@ bool kvm_arm_handle_debug(CPUState *cs, struct kvm_debug_exit_arch *debug_exit) kvm_cpu_synchronize_state(cs); switch (hsr_ec) { +case EC_SOFTWARESTEP: +if (cs->singlestep_enabled) { +return true; +} else { +error_report("Came out of SINGLE STEP when not enabled"); +} +break; case EC_AA64_BKPT: if (kvm_find_sw_breakpoint(cs, env->pc)) { return true; -- 2.6.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 2/6] target-arm: kvm - implement software breakpoints
These don't involve messing around with debug registers, just setting the breakpoint instruction in memory. GDB will not use this mechanism if it can't access the memory to write the breakpoint. All the kernel has to do is ensure the hypervisor traps the breakpoint exceptions and returns to userspace. Signed-off-by: Alex Bennée <alex.ben...@linaro.org> -- v2 - handle debug exit with new hsr exception info - add verbosity to UNIMP message v3 - sync with kvm_cpu_synchronize_state() before checking PC. - use internals.h defines - use env->pc - use proper format types v9 - add include for error_report - define a brk_insn constant v10 - fix up for arm32 compile - move sw_bp_code to kvm32 (stubs)/64 (working) - move kvm_handle_debug to kvm32/64 as kvm_arm_handle_debug - don't enable SW_BP unless the define is there - wrap in have_guest_debug check --- target-arm/kvm.c | 39 +--- target-arm/kvm32.c | 18 + target-arm/kvm64.c | 72 target-arm/kvm_arm.h | 9 +++ 4 files changed, 123 insertions(+), 15 deletions(-) diff --git a/target-arm/kvm.c b/target-arm/kvm.c index 79ef4c6..7f44e22 100644 --- a/target-arm/kvm.c +++ b/target-arm/kvm.c @@ -17,6 +17,7 @@ #include "qemu-common.h" #include "qemu/timer.h" +#include "qemu/error-report.h" #include "sysemu/sysemu.h" #include "sysemu/kvm.h" #include "kvm_arm.h" @@ -516,9 +517,23 @@ MemTxAttrs kvm_arch_post_run(CPUState *cs, struct kvm_run *run) return MEMTXATTRS_UNSPECIFIED; } + int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run) { -return 0; +int ret = 0; + +switch (run->exit_reason) { +case KVM_EXIT_DEBUG: +if (kvm_arm_handle_debug(cs, >debug.arch)) { +ret = EXCP_DEBUG; +} /* otherwise return to guest */ +break; +default: +qemu_log_mask(LOG_UNIMP, "%s: un-handled exit reason %d\n", + __func__, run->exit_reason); +break; +} +return ret; } bool kvm_arch_stop_on_emulation_error(CPUState *cs) @@ -541,16 +556,16 @@ int kvm_arch_on_sigbus(int code, void *addr) return 1; } +/* The #ifdef protections are until 32bit headers are imported and can + * be removed once both 32 and 64 bit reach feature parity. + */ void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg) { -qemu_log_mask(LOG_UNIMP, "%s: not implemented\n", __func__); -} - -int kvm_arch_insert_sw_breakpoint(CPUState *cs, - struct kvm_sw_breakpoint *bp) -{ -qemu_log_mask(LOG_UNIMP, "%s: not implemented\n", __func__); -return -EINVAL; +#ifdef KVM_GUESTDBG_USE_SW_BP +if (kvm_sw_breakpoints_active(cs)) { +dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP; +} +#endif } int kvm_arch_insert_hw_breakpoint(target_ulong addr, @@ -567,12 +582,6 @@ int kvm_arch_remove_hw_breakpoint(target_ulong addr, return -EINVAL; } -int kvm_arch_remove_sw_breakpoint(CPUState *cs, - struct kvm_sw_breakpoint *bp) -{ -qemu_log_mask(LOG_UNIMP, "%s: not implemented\n", __func__); -return -EINVAL; -} void kvm_arch_remove_all_hw_breakpoints(void) { diff --git a/target-arm/kvm32.c b/target-arm/kvm32.c index df1e2b0..5ce969f 100644 --- a/target-arm/kvm32.c +++ b/target-arm/kvm32.c @@ -475,3 +475,21 @@ int kvm_arch_get_registers(CPUState *cs) return 0; } + +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp) +{ +qemu_log_mask(LOG_UNIMP, "%s: guest debug not yet implemented\n", __func__); +return -EINVAL; +} + +int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp) +{ +qemu_log_mask(LOG_UNIMP, "%s: guest debug not yet implemented\n", __func__); +return -EINVAL; +} + +bool kvm_arm_handle_debug(CPUState *cs, struct kvm_debug_exit_arch *debug_exit) +{ +qemu_log_mask(LOG_UNIMP, "%s: guest debug not yet implemented\n", __func__); +return false; +} diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c index d087794..3b3929d 100644 --- a/target-arm/kvm64.c +++ b/target-arm/kvm64.c @@ -18,6 +18,7 @@ #include "config-host.h" #include "qemu-common.h" #include "qemu/timer.h" +#include "qemu/error-report.h" #include "sysemu/sysemu.h" #include "sysemu/kvm.h" #include "kvm_arm.h" @@ -481,3 +482,74 @@ int kvm_arch_get_registers(CPUState *cs) /* TODO: other registers */ return ret; } + +/* C6.6.29 BRK instruction */ +static const uint32_t brk_insn = 0xd420; + +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp) +{ +if (have_guest_debug) { +if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)>sav
[PATCH v10 4/6] target-arm: kvm - add support for HW assisted debug
This adds basic support for HW assisted debug. The ioctl interface to KVM allows us to pass an implementation defined number of break and watch point registers. When KVM_GUESTDBG_USE_HW is specified these debug registers will be installed in place on the world switch into the guest. The hardware is actually capable of more advanced matching but it is unclear if this expressiveness is available via the gdbstub protocol. Signed-off-by: Alex Bennée <alex.ben...@linaro.org> --- v2 - correct setting of PMC/BAS/MASK - improved commentary - added helper function to check watchpoint in range - fix find/deletion of watchpoints v3 - use internals.h definitions v6 - KVM_GUESTDBG_USE_HW_BP->KVM_GUESTDBG_USE_HW - renamed some helper functions to avoid confusion v9 - fix merge conflicts on re-base - rm asm/ptrace.h include - add additional commentry for hw breakpoints - explain gdb's model for HW bkpts - fix up spacing, formatting as per checkpatch - better PAC values - use is_power_of_2 - use _arm_ fn naming and add docs - add a CPUWatchpoint structure for reporting - replace manual array manipulation with g_array abstraction v10 - fix compilation for arm32/split imps between kvm32/64 - make find_hw_watchpoint/breakpoint local static functions - cleaned up comment grammar - fixed up missing spaces - removed pointless ?: booleanisation - removed pointless kvm_arm_hw_debug_active wrappers - s/is/if/ --- target-arm/kvm.c | 26 +--- target-arm/kvm32.c | 29 + target-arm/kvm64.c | 361 ++- target-arm/kvm_arm.h | 21 +++ 4 files changed, 415 insertions(+), 22 deletions(-) diff --git a/target-arm/kvm.c b/target-arm/kvm.c index 7f44e22..84974bb 100644 --- a/target-arm/kvm.c +++ b/target-arm/kvm.c @@ -566,26 +566,12 @@ void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg) dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP; } #endif -} - -int kvm_arch_insert_hw_breakpoint(target_ulong addr, - target_ulong len, int type) -{ -qemu_log_mask(LOG_UNIMP, "%s: not implemented\n", __func__); -return -EINVAL; -} - -int kvm_arch_remove_hw_breakpoint(target_ulong addr, - target_ulong len, int type) -{ -qemu_log_mask(LOG_UNIMP, "%s: not implemented\n", __func__); -return -EINVAL; -} - - -void kvm_arch_remove_all_hw_breakpoints(void) -{ -qemu_log_mask(LOG_UNIMP, "%s: not implemented\n", __func__); +#ifdef KVM_GUESTDBG_USE_HW +if (kvm_arm_hw_debug_active(cs)) { +dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_HW; +kvm_arm_copy_hw_debug_data(>arch); +} +#endif } void kvm_arch_init_irq_routing(KVMState *s) diff --git a/target-arm/kvm32.c b/target-arm/kvm32.c index 5ce969f..ff83ce6 100644 --- a/target-arm/kvm32.c +++ b/target-arm/kvm32.c @@ -493,3 +493,32 @@ bool kvm_arm_handle_debug(CPUState *cs, struct kvm_debug_exit_arch *debug_exit) qemu_log_mask(LOG_UNIMP, "%s: guest debug not yet implemented\n", __func__); return false; } + +int kvm_arch_insert_hw_breakpoint(target_ulong addr, + target_ulong len, int type) +{ +qemu_log_mask(LOG_UNIMP, "%s: not implemented\n", __func__); +return -EINVAL; +} + +int kvm_arch_remove_hw_breakpoint(target_ulong addr, + target_ulong len, int type) +{ +qemu_log_mask(LOG_UNIMP, "%s: not implemented\n", __func__); +return -EINVAL; +} + +void kvm_arch_remove_all_hw_breakpoints(void) +{ +qemu_log_mask(LOG_UNIMP, "%s: not implemented\n", __func__); +} + +void kvm_arm_copy_hw_debug_data(struct kvm_guest_debug_arch *ptr) +{ +qemu_log_mask(LOG_UNIMP, "%s: not implemented\n", __func__); +} + +bool kvm_arm_hw_debug_active(CPUState *cs) +{ +return false; +} diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c index 5f96cde..771ecdb 100644 --- a/target-arm/kvm64.c +++ b/target-arm/kvm64.c @@ -2,6 +2,7 @@ * ARM implementation of KVM hooks, 64 bit specific code * * Copyright Mian-M. Hamayun 2013, Virtual Open Systems + * Copyright Alex Bennée 2014, Linaro * * This work is licensed under the terms of the GNU GPL, version 2 or later. * See the COPYING file in the top-level directory. @@ -12,13 +13,17 @@ #include #include #include +#include +#include #include #include "config-host.h" #include "qemu-common.h" #include "qemu/timer.h" #include "qemu/error-report.h" +#include "qemu/host-utils.h" +#include "exec/gdbstub.h" #include "sysemu/sysemu.h" #include "sysemu/kvm.h" #include "kvm_arm.h" @@ -28,20 +33,358 @@ static bool have_guest_debug; +/* + * Although the ARM implementation of hardware assisted debugging + * allows
Re: [PATCH v2 08/21] arm64: KVM: Implement debug save/restore
Marc Zyngier <marc.zyng...@arm.com> writes: > On 01/12/15 12:56, Christoffer Dall wrote: >> On Fri, Nov 27, 2015 at 06:50:02PM +, Marc Zyngier wrote: >>> Implement the debug save restore as a direct translation of >>> the assembly code version. >>> >>> Signed-off-by: Marc Zyngier <marc.zyng...@arm.com> >>> --- >>> arch/arm64/kvm/hyp/Makefile | 1 + >>> arch/arm64/kvm/hyp/debug-sr.c | 130 >>> ++ >>> arch/arm64/kvm/hyp/hyp.h | 9 +++ >>> 3 files changed, 140 insertions(+) >>> create mode 100644 arch/arm64/kvm/hyp/debug-sr.c >>> +void __hyp_text __debug_cond_save_host_state(struct kvm_vcpu *vcpu) >>> +{ >>> + if ((vcpu->arch.ctxt.sys_regs[MDSCR_EL1] & DBG_MDSCR_KDE) || >>> + (vcpu->arch.ctxt.sys_regs[MDSCR_EL1] & DBG_MDSCR_KDE)) I've just noticed I'm seeing double here. Did a DBG_MDSCR_MDE can transliterated here? >>> + vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY; >>> + >>> + __debug_save_state(vcpu, >arch.host_debug_state, >>> + kern_hyp_va(vcpu->arch.host_cpu_context)); >> >> doesn't the assmebly code jump across saving this state neither bits are >> set where this always saves the state? > > It doesn't. The save/restore functions are guarded by tests on > KVM_ARM64_DEBUG_DIRTY, just like we have skip_debug_state on all actions > involving the save/restore in the assembly version. > >> in any case, I feel some context is lost when this is moved away from >> assembly and understanding this patch would be easier if the semantics >> of these two _cond functions were documented. > > I can migrate the existing comments if you think that helps. > > Thanks, > > M. -- Alex Bennée -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 04/21] arm64: KVM: Implement vgic-v3 save/restore
t; + cpu_if->vgic_lr[LR_OFFSET(11)] = read_gicreg(ICH_LR11_EL2); > + case 10: > + cpu_if->vgic_lr[LR_OFFSET(10)] = read_gicreg(ICH_LR10_EL2); > + case 9: > + cpu_if->vgic_lr[LR_OFFSET(9)] = read_gicreg(ICH_LR9_EL2); > + case 8: > + cpu_if->vgic_lr[LR_OFFSET(8)] = read_gicreg(ICH_LR8_EL2); > + case 7: > + cpu_if->vgic_lr[LR_OFFSET(7)] = read_gicreg(ICH_LR7_EL2); > + case 6: > + cpu_if->vgic_lr[LR_OFFSET(6)] = read_gicreg(ICH_LR6_EL2); > + case 5: > + cpu_if->vgic_lr[LR_OFFSET(5)] = read_gicreg(ICH_LR5_EL2); > + case 4: > + cpu_if->vgic_lr[LR_OFFSET(4)] = read_gicreg(ICH_LR4_EL2); > + case 3: > + cpu_if->vgic_lr[LR_OFFSET(3)] = read_gicreg(ICH_LR3_EL2); > + case 2: > + cpu_if->vgic_lr[LR_OFFSET(2)] = read_gicreg(ICH_LR2_EL2); > + case 1: > + cpu_if->vgic_lr[LR_OFFSET(1)] = read_gicreg(ICH_LR1_EL2); > + case 0: > + cpu_if->vgic_lr[LR_OFFSET(0)] = read_gicreg(ICH_LR0_EL2); > + } > + > + switch (nr_pri) { > + case 7: > + cpu_if->vgic_ap0r[3] = read_gicreg(ICH_AP0R3_EL2); > + cpu_if->vgic_ap0r[2] = read_gicreg(ICH_AP0R2_EL2); > + case 6: > + cpu_if->vgic_ap0r[1] = read_gicreg(ICH_AP0R1_EL2); > + default: > + cpu_if->vgic_ap0r[0] = read_gicreg(ICH_AP0R0_EL2); > + } > + > + switch (nr_pri) { > + case 7: > + cpu_if->vgic_ap1r[3] = read_gicreg(ICH_AP1R3_EL2); > + cpu_if->vgic_ap1r[2] = read_gicreg(ICH_AP1R2_EL2); > + case 6: > + cpu_if->vgic_ap1r[1] = read_gicreg(ICH_AP1R1_EL2); > + default: > + cpu_if->vgic_ap1r[0] = read_gicreg(ICH_AP1R0_EL2); > + } > + > + write_gicreg(read_gicreg(ICC_SRE_EL2) | ICC_SRE_EL2_ENABLE, > + ICC_SRE_EL2); > + isb(); > + write_gicreg(1, ICC_SRE_EL1); > +} > + > +void __hyp_text __vgic_v3_restore_state(struct kvm_vcpu *vcpu) > +{ > + struct vgic_v3_cpu_if *cpu_if = >arch.vgic_cpu.vgic_v3; > + u64 val; > + u32 nr_lr, nr_pri; > + > + /* Make sure SRE is valid before writing the other registers */ > + write_gicreg(cpu_if->vgic_sre, ICC_SRE_EL1); > + isb(); > + > + write_gicreg(cpu_if->vgic_hcr, ICH_HCR_EL2); > + write_gicreg(cpu_if->vgic_vmcr, ICH_VMCR_EL2); > + > + val = read_gicreg(ICH_VTR_EL2); > + nr_lr = val & 0xf; > + nr_pri = ((u32)val >> 29) + 1; > + > + switch (nr_pri) { > + case 7: > + write_gicreg(cpu_if->vgic_ap1r[3], ICH_AP1R3_EL2); > + write_gicreg(cpu_if->vgic_ap1r[2], ICH_AP1R2_EL2); > + case 6: > + write_gicreg(cpu_if->vgic_ap1r[1], ICH_AP1R1_EL2); > + default: > + write_gicreg(cpu_if->vgic_ap1r[0], ICH_AP1R0_EL2); > + } > + > + switch (nr_pri) { > + case 7: > + write_gicreg(cpu_if->vgic_ap0r[3], ICH_AP0R3_EL2); > + write_gicreg(cpu_if->vgic_ap0r[2], ICH_AP0R2_EL2); > + case 6: > + write_gicreg(cpu_if->vgic_ap0r[1], ICH_AP0R1_EL2); > + default: > + write_gicreg(cpu_if->vgic_ap0r[0], ICH_AP0R0_EL2); > + } > + > + switch (nr_lr) { > + case 15: > + write_gicreg(cpu_if->vgic_lr[LR_OFFSET(15)], ICH_LR15_EL2); > + case 14: > + write_gicreg(cpu_if->vgic_lr[LR_OFFSET(14)], ICH_LR14_EL2); > + case 13: > + write_gicreg(cpu_if->vgic_lr[LR_OFFSET(13)], ICH_LR13_EL2); > + case 12: > + write_gicreg(cpu_if->vgic_lr[LR_OFFSET(12)], ICH_LR12_EL2); > + case 11: > + write_gicreg(cpu_if->vgic_lr[LR_OFFSET(11)], ICH_LR11_EL2); > + case 10: > + write_gicreg(cpu_if->vgic_lr[LR_OFFSET(10)], ICH_LR10_EL2); > + case 9: > + write_gicreg(cpu_if->vgic_lr[LR_OFFSET(9)], ICH_LR9_EL2); > + case 8: > + write_gicreg(cpu_if->vgic_lr[LR_OFFSET(8)], ICH_LR8_EL2); > + case 7: > + write_gicreg(cpu_if->vgic_lr[LR_OFFSET(7)], ICH_LR7_EL2); > + case 6: > + write_gicreg(cpu_if->vgic_lr[LR_OFFSET(6)], ICH_LR6_EL2); > + case 5: > + write_gicreg(cpu_if->vgic_lr[LR_OFFSET(5)], ICH_LR5_EL2); > + case 4: > + write_gicreg(cpu_if->vgic_lr[LR_OFFSET(4)], ICH_LR4_EL2); > + case 3: > + write_gicreg(cpu_if->vgic_lr[LR_OFFSET(3)], ICH_LR3_EL2); > + case 2: > + write_gicreg(cpu_if->vgic_lr[
Re: [PATCH 2/2] KVM: Create debugfs dir and stat files for each VM
Janosch Frank <fran...@linux.vnet.ibm.com> writes: > On 11/27/2015 09:42 PM, Tyler Baker wrote: >> On 27 November 2015 at 10:53, Tyler Baker <tyler.ba...@linaro.org> wrote: >>> On 27 November 2015 at 09:08, Tyler Baker <tyler.ba...@linaro.org> wrote: >>>> On 27 November 2015 at 00:54, Christian Borntraeger >>>> <borntrae...@de.ibm.com> wrote: >>>>> On 11/26/2015 09:47 PM, Christian Borntraeger wrote: >>>>>> On 11/26/2015 05:17 PM, Tyler Baker wrote: >>>>>>> Hi Christian, >>>>>>> >>>>>>> The kernelci.org bot recently has been reporting kvm guest boot >>>>>>> failures[1] on various arm64 platforms in next-20151126. The bot >>>>>>> bisected[2] the failures to the commit in -next titled "KVM: Create >>>>>>> debugfs dir and stat files for each VM". I confirmed by reverting this >>>>>>> commit on top of next-20151126 it resolves the boot issue. >>>>>>> >> > After a quick look into qemu I guess I've found the problem: > kvm_init creates a vm, does checking and self initialization and > then calls kvm_arch_init. The arch initialization indirectly > calls kvm_arm_create_scratch_host_vcpu and that's where the > trouble begins, as it also creates a VM. > > My assumption was, that nobody would create multiple VMs under > the same PID. Christian and I are working on a solution on kernel > side. Yeah ARM is a little weird in that respect as the scratch VM is used to probe capabilities. There is nothing in the API that says you can't have multiple VMs per PID so I guess a better unique identifier is needed. -- Alex Bennée -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 08/21] arm64: KVM: Implement debug save/restore
\ > + } > + > +void __hyp_text __debug_save_state(struct kvm_vcpu *vcpu, > +struct kvm_guest_debug_arch *dbg, > +struct kvm_cpu_context *ctxt) > +{ > + if (vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY) { > + u64 aa64dfr0 = read_sysreg(id_aa64dfr0_el1); > + int brps, wrps; > + > + brps = (aa64dfr0 >> 12) & 0xf; > + wrps = (aa64dfr0 >> 20) & 0xf; I'm sure we have that defined - get_num_brps/wrps() - but I don't know if the compiler would do a better job with it. > + > + save_debug(dbg->dbg_bcr, dbgbcr, brps); > + save_debug(dbg->dbg_bvr, dbgbvr, brps); > + save_debug(dbg->dbg_wcr, dbgwcr, wrps); > + save_debug(dbg->dbg_wvr, dbgwvr, wrps); > + > + ctxt->sys_regs[MDCCINT_EL1] = read_sysreg(mdccint_el1); > + } > +} > + > +void __hyp_text __debug_restore_state(struct kvm_vcpu *vcpu, > + struct kvm_guest_debug_arch *dbg, > + struct kvm_cpu_context *ctxt) > +{ > + if (vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY) { > + u64 aa64dfr0 = read_sysreg(id_aa64dfr0_el1); > + int brps, wrps; > + > + brps = (aa64dfr0 >> 12) & 0xf; > + wrps = (aa64dfr0 >> 20) & 0xf; > + > + restore_debug(dbg->dbg_bcr, dbgbcr, brps); > + restore_debug(dbg->dbg_bvr, dbgbvr, brps); > + restore_debug(dbg->dbg_wcr, dbgwcr, wrps); > + restore_debug(dbg->dbg_wvr, dbgwvr, wrps); > + > + write_sysreg(ctxt->sys_regs[MDCCINT_EL1], mdccint_el1); > + } > +} > + > +void __hyp_text __debug_cond_save_host_state(struct kvm_vcpu *vcpu) > +{ > + if ((vcpu->arch.ctxt.sys_regs[MDSCR_EL1] & DBG_MDSCR_KDE) || > + (vcpu->arch.ctxt.sys_regs[MDSCR_EL1] & DBG_MDSCR_KDE)) > + vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY; > + > + __debug_save_state(vcpu, >arch.host_debug_state, > +kern_hyp_va(vcpu->arch.host_cpu_context)); > +} > + > +void __hyp_text __debug_cond_restore_host_state(struct kvm_vcpu *vcpu) > +{ > + if (vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY) { > + __debug_restore_state(vcpu, >arch.host_debug_state, > + kern_hyp_va(vcpu->arch.host_cpu_context)); > + vcpu->arch.debug_flags &= ~KVM_ARM64_DEBUG_DIRTY; > + } > +} > + > +u32 __hyp_text __debug_read_mdcr_el2(void) > +{ > + return read_sysreg(mdcr_el2); > +} > diff --git a/arch/arm64/kvm/hyp/hyp.h b/arch/arm64/kvm/hyp/hyp.h > index 4639330..2581232 100644 > --- a/arch/arm64/kvm/hyp/hyp.h > +++ b/arch/arm64/kvm/hyp/hyp.h > @@ -41,5 +41,14 @@ void __sysreg_restore_state(struct kvm_cpu_context *ctxt); > void __sysreg32_save_state(struct kvm_vcpu *vcpu); > void __sysreg32_restore_state(struct kvm_vcpu *vcpu); > > +void __debug_save_state(struct kvm_vcpu *vcpu, > + struct kvm_guest_debug_arch *dbg, > + struct kvm_cpu_context *ctxt); > +void __debug_restore_state(struct kvm_vcpu *vcpu, > +struct kvm_guest_debug_arch *dbg, > +struct kvm_cpu_context *ctxt); > +void __debug_cond_save_host_state(struct kvm_vcpu *vcpu); > +void __debug_cond_restore_host_state(struct kvm_vcpu *vcpu); > + > #endif /* __ARM64_KVM_HYP_H__ */ I spun it up against my KVM debug tests and everything was fine. Have a: Tested-by: Alex Bennée <alex.ben...@linaro.org> Reviewed-by: Alex Bennée <alex.ben...@linaro.org> -- Alex Bennée -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 11/21] arm64: KVM: Implement the core world switch
__debug_cond_save_state(vcpu, >arch.host_debug_state, host_ctxt); > + > + __activate_traps(vcpu); > + __activate_vm(vcpu); > + > + __vgic_restore_state(vcpu); > + __timer_restore_state(vcpu); > + > + /* > + * We must restore the 32-bit state before the sysregs, thanks > + * to Cortex-A57 erratum #852523. > + */ > + __sysreg32_restore_state(vcpu); > + __sysreg_restore_state(guest_ctxt); > + __debug_restore_state(vcpu, >arch.vcpu_debug_state, > guest_ctxt); Did you right this before guest debug was merged? Now vcpu.debug_ptr/VCPU_DEBUG_PTR selects which set of debug registers we are going to use before entering the guest. > + > + /* Jump in the fire! */ > + exit_code = __guest_enter(vcpu, host_ctxt); > + /* And we're baaack! */ > + > + __sysreg_save_state(guest_ctxt); > + __sysreg32_save_state(vcpu); > + __timer_save_state(vcpu); > + __vgic_save_state(vcpu); > + > + __deactivate_traps(vcpu); > + __deactivate_vm(vcpu); > + > + __sysreg_restore_state(host_ctxt); > + > + __debug_save_state(vcpu, >arch.vcpu_debug_state, guest_ctxt); > + __debug_clear_restore_state(vcpu, >arch.host_debug_state, > host_ctxt); > + > + return exit_code; > +} -- Alex Bennée -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 1/6] target-arm: kvm64 - introduce kvm_arm_init_debug()
Peter Maydell <peter.mayd...@linaro.org> writes: > On 20 November 2015 at 15:05, Peter Maydell <peter.mayd...@linaro.org> wrote: >> On 12 November 2015 at 16:20, Alex Bennée <alex.ben...@linaro.org> wrote: >>> As we haven't always had guest debug support we need to probe for it. >>> Additionally we don't do this in the start-up capability code so we >>> don't fall over on old kernels. >>> >>> Signed-off-by: Alex Bennée <alex.ben...@linaro.org> >>> --- >>> target-arm/kvm64.c | 18 ++ >>> 1 file changed, 18 insertions(+) >>> >>> diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c >>> index ceebfeb..d087794 100644 >>> --- a/target-arm/kvm64.c >>> +++ b/target-arm/kvm64.c >>> @@ -25,6 +25,22 @@ >>> #include "internals.h" >>> #include "hw/arm/arm.h" >>> >>> +static bool have_guest_debug; >>> + >>> +/** >>> + * kvm_arm_init_debug() >>> + * @cs: CPUState >>> + * >>> + * Check for guest debug capabilities. >>> + * >>> + */ >>> +static void kvm_arm_init_debug(CPUState *cs) >>> +{ >>> +have_guest_debug = kvm_check_extension(cs->kvm_state, >>> + KVM_CAP_SET_GUEST_DEBUG); >>> +return; >>> +} >>> + >>> static inline void set_feature(uint64_t *features, int feature) >>> { >>> *features |= 1ULL << feature; >>> @@ -121,6 +137,8 @@ int kvm_arch_init_vcpu(CPUState *cs) >>> } >>> cpu->mp_affinity = mpidr & ARM64_AFFINITY_MASK; >>> >>> +kvm_arm_init_debug(cs); >>> + >>> return kvm_arm_init_cpreg_list(cpu); >>> } >> >> I assume in practice the kernel guarantees that either all >> CPUs have the SET_GUEST_DEBUG cap, or none do :-) >> >> Reviewed-by: Peter Maydell <peter.mayd...@linaro.org> > > ...except I've just noticed that nothing else in this patchset > ever reads the have_guest_debug bool we just set, so what is > the purpose of this patch? Oops, maybe to point out my stupidity ;-) But yes the SET_GUEST_DEBUG cap is kernel wide. > > thanks > -- PMM -- Alex Bennée -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v9 6/6] tests/guest-debug: introduce basic gdbstub tests
From: Alex Bennée <a...@bennee.com> The aim of these tests is to combine with an appropriate kernel image (with symbol-file vmlinux) and check it behaves as it should. Given a kernel it checks: - single step - software breakpoint - hardware breakpoint - access, read and write watchpoints On success it returns 0 to the calling process. I've not plumbed this into the "make check" logic though as we need a solution for providing non-host binaries to the tests. However the test is structured to work with pretty much any Linux kernel image as it uses the basic kernel_init code which is common across architectures. Signed-off-by: Alex Bennée <alex.ben...@linaro.org> --- tests/guest-debug/test-gdbstub.py | 171 ++ 1 file changed, 171 insertions(+) create mode 100644 tests/guest-debug/test-gdbstub.py diff --git a/tests/guest-debug/test-gdbstub.py b/tests/guest-debug/test-gdbstub.py new file mode 100644 index 000..aa53567 --- /dev/null +++ b/tests/guest-debug/test-gdbstub.py @@ -0,0 +1,171 @@ +# +# This script needs to be run on startup +# qemu -kernel ${KERNEL} -s -S +# and then: +# gdb ${KERNEL}.vmlinux -x ${QEMU_SRC}/tests/guest-debug/test-gdbstub.py + +import gdb + +failcount = 0 + +def report(cond, msg): +"Report success/fail of test" +if cond: +print "PASS: %s" % (msg) +else: +print "FAIL: %s" % (msg) +failcount += 1 + +def check_step(): +"Step an instruction, check it moved." +start_pc = gdb.parse_and_eval('$pc') +gdb.execute("si") +end_pc = gdb.parse_and_eval('$pc') + +return not (start_pc == end_pc) + + +def check_break(sym_name): +"Setup breakpoint, continue and check we stopped." +sym, ok = gdb.lookup_symbol(sym_name) +bp = gdb.Breakpoint(sym_name) + +gdb.execute("c") + +# hopefully we came back +end_pc = gdb.parse_and_eval('$pc') +print "%s == %s %d" % (end_pc, sym.value(), bp.hit_count) +bp.delete() + +# can we test we hit bp? +return end_pc == sym.value() + + +# We need to do hbreak manually as the python interface doesn't export it +def check_hbreak(sym_name): +"Setup hardware breakpoint, continue and check we stopped." +sym, ok = gdb.lookup_symbol(sym_name) +gdb.execute("hbreak %s" % (sym_name)) +gdb.execute("c") + +# hopefully we came back +end_pc = gdb.parse_and_eval('$pc') +print "%s == %s" % (end_pc, sym.value()) + +if end_pc == sym.value(): +gdb.execute("d 1") +return True +else: +return False + + +class WatchPoint(gdb.Breakpoint): + +def get_wpstr(self, sym_name): +"Setup sym and wp_str for given symbol." +self.sym, ok = gdb.lookup_symbol(sym_name) +wp_addr = gdb.parse_and_eval(sym_name).address +self.wp_str = '*(%(type)s)(&%(address)s)' % dict( +type = wp_addr.type, address = sym_name) + +return(self.wp_str) + +def __init__(self, sym_name, type): +wp_str = self.get_wpstr(sym_name) +super(WatchPoint, self).__init__(wp_str, gdb.BP_WATCHPOINT, type) + +def stop(self): +end_pc = gdb.parse_and_eval('$pc') +print "HIT WP @ %s" % (end_pc) +return True + + +def do_one_watch(sym, wtype, text): + +wp = WatchPoint(sym, wtype) +gdb.execute("c") +report_str = "%s for %s (%s)" % (text, sym, wp.sym.value()) + +if wp.hit_count > 0: +report(True, report_str) +wp.delete() +else: +report(False, report_str) + + +def check_watches(sym_name): +"Watch a symbol for any access." + +# Should hit for any read +do_one_watch(sym_name, gdb.WP_ACCESS, "awatch") + +# Again should hit for reads +do_one_watch(sym_name, gdb.WP_READ, "rwatch") + +# Finally when it is written +do_one_watch(sym_name, gdb.WP_WRITE, "watch") + + +class CatchBreakpoint(gdb.Breakpoint): +def __init__(self, sym_name): +super(CatchBreakpoint, self).__init__(sym_name) +self.sym, ok = gdb.lookup_symbol(sym_name) + +def stop(self): +end_pc = gdb.parse_and_eval ('$pc') +print "CB: %s == %s" % (end_pc, self.sym.value()) +if end_pc == sym.value(): +report(False, "Hit final catchpoint") + + +def run_test(): +"Run throught the tests one by one" + +print "Checking we can step the first few instructions" +step_ok = 0 +for i in xrange(3): +if check_step(): +step_ok += 1 + +report(step_ok == 3, "single step in boot code") + +print "Checking HW breakpoint works" +break_ok = check_hbreak("kernel_init") +report(break_ok, "hbreak @ kernel_init") + +# Can't
[PATCH v9 4/6] target-arm: kvm - add support for HW assisted debug
This adds basic support for HW assisted debug. The ioctl interface to KVM allows us to pass an implementation defined number of break and watch point registers. When KVM_GUESTDBG_USE_HW is specified these debug registers will be installed in place on the world switch into the guest. The hardware is actually capable of more advanced matching but it is unclear if this expressiveness is available via the gdbstub protocol. Signed-off-by: Alex Bennée <alex.ben...@linaro.org> --- v2 - correct setting of PMC/BAS/MASK - improved commentary - added helper function to check watchpoint in range - fix find/deletion of watchpoints v3 - use internals.h definitions v6 - KVM_GUESTDBG_USE_HW_BP->KVM_GUESTDBG_USE_HW - renamed some helper functions to avoid confusion v9 - fix merge conflicts on re-base - rm asm/ptrace.h include - add additional commentry for hw breakpoints - explain gdb's model for HW bkpts - fix up spacing, formatting as per checkpatch - better PAC values - use is_power_of_2 - use _arm_ fn naming and add docs - add a CPUWatchpoint structure for reporting - replace manual array manipulation with g_array abstraction --- target-arm/kvm.c | 38 +++--- target-arm/kvm64.c | 352 ++- target-arm/kvm_arm.h | 38 ++ 3 files changed, 406 insertions(+), 22 deletions(-) diff --git a/target-arm/kvm.c b/target-arm/kvm.c index d505a7e..1f57e92 100644 --- a/target-arm/kvm.c +++ b/target-arm/kvm.c @@ -547,6 +547,20 @@ static int kvm_handle_debug(CPUState *cs, struct kvm_run *run) return true; } break; +case EC_BREAKPOINT: +if (kvm_arm_find_hw_breakpoint(cs, env->pc)) { +return true; +} +break; +case EC_WATCHPOINT: +{ +CPUWatchpoint *wp = kvm_arm_find_hw_watchpoint(cs, arch_info->far); +if (wp) { +cs->watchpoint_hit = wp; +return true; +} +break; +} default: error_report("%s: unhandled debug exit (%"PRIx32", %"PRIx64")\n", __func__, arch_info->hsr, env->pc); @@ -608,6 +622,10 @@ void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg) if (kvm_sw_breakpoints_active(cs)) { dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP; } +if (kvm_arm_hw_debug_active(cs)) { +dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_HW; +kvm_arm_copy_hw_debug_data(>arch); +} } /* C6.6.29 BRK instruction */ @@ -635,26 +653,6 @@ int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp) return 0; } -int kvm_arch_insert_hw_breakpoint(target_ulong addr, - target_ulong len, int type) -{ -qemu_log_mask(LOG_UNIMP, "%s: not implemented\n", __func__); -return -EINVAL; -} - -int kvm_arch_remove_hw_breakpoint(target_ulong addr, - target_ulong len, int type) -{ -qemu_log_mask(LOG_UNIMP, "%s: not implemented\n", __func__); -return -EINVAL; -} - - -void kvm_arch_remove_all_hw_breakpoints(void) -{ -qemu_log_mask(LOG_UNIMP, "%s: not implemented\n", __func__); -} - void kvm_arch_init_irq_routing(KVMState *s) { } diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c index d087794..c468324 100644 --- a/target-arm/kvm64.c +++ b/target-arm/kvm64.c @@ -2,6 +2,7 @@ * ARM implementation of KVM hooks, 64 bit specific code * * Copyright Mian-M. Hamayun 2013, Virtual Open Systems + * Copyright Alex Bennée 2014, Linaro * * This work is licensed under the terms of the GNU GPL, version 2 or later. * See the COPYING file in the top-level directory. @@ -12,12 +13,17 @@ #include #include #include +#include +#include #include #include "config-host.h" #include "qemu-common.h" #include "qemu/timer.h" +#include "qemu/host-utils.h" +#include "qemu/error-report.h" +#include "exec/gdbstub.h" #include "sysemu/sysemu.h" #include "sysemu/kvm.h" #include "kvm_arm.h" @@ -27,20 +33,362 @@ static bool have_guest_debug; +/* + * Although the ARM implementation of hardware assisted debugging + * allows for different breakpoints per-core the current GDB interface + * treats them as a global pool of registers (which seems to be the + * case for x86, ppc and s390). As a result we store one copy of + * registers which is used for all active cores. + * + * Write access is serialised by virtue of the GDB protocol which + * updates things. Read access (i.e. when the values are copied to the + * vCPU) is also gated by GDBs run control. + * + * This is not unreasonable as most of the time debugging kernels you + * never know which core will eventually execute your function. + */ + +typedef struct { +uint64_t bcr; +ui
[PATCH v9 3/6] target-arm: kvm - support for single step
This adds support for single-step. There isn't much to do on the QEMU side as after we set-up the request for single step via the debug ioctl it is all handled within the kernel. Signed-off-by: Alex Bennée <alex.ben...@linaro.org> --- v2 - convert to using HSR_EC v3 - use internals.h definitions --- target-arm/kvm.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/target-arm/kvm.c b/target-arm/kvm.c index 50f70ef..d505a7e 100644 --- a/target-arm/kvm.c +++ b/target-arm/kvm.c @@ -535,6 +535,13 @@ static int kvm_handle_debug(CPUState *cs, struct kvm_run *run) kvm_cpu_synchronize_state(cs); switch (hsr_ec) { +case EC_SOFTWARESTEP: +if (cs->singlestep_enabled) { +return true; +} else { +error_report("Came out of SINGLE STEP when not enabled"); +} +break; case EC_AA64_BKPT: if (kvm_find_sw_breakpoint(cs, env->pc)) { return true; @@ -595,6 +602,9 @@ int kvm_arch_on_sigbus(int code, void *addr) void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg) { +if (cs->singlestep_enabled) { +dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_SINGLESTEP; +} if (kvm_sw_breakpoints_active(cs)) { dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP; } -- 2.6.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v9 2/6] target-arm: kvm - implement software breakpoints
These don't involve messing around with debug registers, just setting the breakpoint instruction in memory. GDB will not use this mechanism if it can't access the memory to write the breakpoint. All the kernel has to do is ensure the hypervisor traps the breakpoint exceptions and returns to userspace. Signed-off-by: Alex Bennée <alex.ben...@linaro.org> -- v2 - handle debug exit with new hsr exception info - add verbosity to UNIMP message v3 - sync with kvm_cpu_synchronize_state() before checking PC. - use internals.h defines - use env->pc - use proper format types v9 - add include for error_report - define a brk_insn constant --- target-arm/kvm.c | 90 1 file changed, 78 insertions(+), 12 deletions(-) diff --git a/target-arm/kvm.c b/target-arm/kvm.c index 79ef4c6..50f70ef 100644 --- a/target-arm/kvm.c +++ b/target-arm/kvm.c @@ -17,6 +17,7 @@ #include "qemu-common.h" #include "qemu/timer.h" +#include "qemu/error-report.h" #include "sysemu/sysemu.h" #include "sysemu/kvm.h" #include "kvm_arm.h" @@ -516,9 +517,60 @@ MemTxAttrs kvm_arch_post_run(CPUState *cs, struct kvm_run *run) return MEMTXATTRS_UNSPECIFIED; } +/* See v8 ARM ARM D7.2.27 ESR_ELx, Exception Syndrome Register + * + * To minimise translating between kernel and user-space the kernel + * ABI just provides user-space with the full exception syndrome + * register value to be decoded in QEMU. + */ + +static int kvm_handle_debug(CPUState *cs, struct kvm_run *run) +{ +struct kvm_debug_exit_arch *arch_info = >debug.arch; +int hsr_ec = arch_info->hsr >> ARM_EL_EC_SHIFT; +ARMCPU *cpu = ARM_CPU(cs); +CPUARMState *env = >env; + +/* Ensure PC is synchronised */ +kvm_cpu_synchronize_state(cs); + +switch (hsr_ec) { +case EC_AA64_BKPT: +if (kvm_find_sw_breakpoint(cs, env->pc)) { +return true; +} +break; +default: +error_report("%s: unhandled debug exit (%"PRIx32", %"PRIx64")\n", + __func__, arch_info->hsr, env->pc); +} + +/* If we don't handle this it could be it really is for the + guest to handle */ +qemu_log_mask(LOG_UNIMP, + "%s: re-injecting exception not yet implemented" + " (0x%"PRIx32", %"PRIx64")\n", + __func__, hsr_ec, env->pc); + +return false; +} + int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run) { -return 0; +int ret = 0; + +switch (run->exit_reason) { +case KVM_EXIT_DEBUG: +if (kvm_handle_debug(cs, run)) { +ret = EXCP_DEBUG; +} /* otherwise return to guest */ +break; +default: +qemu_log_mask(LOG_UNIMP, "%s: un-handled exit reason %d\n", + __func__, run->exit_reason); +break; +} +return ret; } bool kvm_arch_stop_on_emulation_error(CPUState *cs) @@ -543,14 +595,34 @@ int kvm_arch_on_sigbus(int code, void *addr) void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg) { -qemu_log_mask(LOG_UNIMP, "%s: not implemented\n", __func__); +if (kvm_sw_breakpoints_active(cs)) { +dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP; +} } -int kvm_arch_insert_sw_breakpoint(CPUState *cs, - struct kvm_sw_breakpoint *bp) +/* C6.6.29 BRK instruction */ +static const uint32_t brk_insn = 0xd420; + +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp) { -qemu_log_mask(LOG_UNIMP, "%s: not implemented\n", __func__); -return -EINVAL; + +if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)>saved_insn, 4, 0) || +cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)_insn, 4, 1)) { +return -EINVAL; +} +return 0; +} + +int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp) +{ +static uint32_t brk; + +if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *), 4, 0) || +brk != brk_insn || +cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)>saved_insn, 4, 1)) { +return -EINVAL; +} +return 0; } int kvm_arch_insert_hw_breakpoint(target_ulong addr, @@ -567,12 +639,6 @@ int kvm_arch_remove_hw_breakpoint(target_ulong addr, return -EINVAL; } -int kvm_arch_remove_sw_breakpoint(CPUState *cs, - struct kvm_sw_breakpoint *bp) -{ -qemu_log_mask(LOG_UNIMP, "%s: not implemented\n", __func__); -return -EINVAL; -} void kvm_arch_remove_all_hw_breakpoints(void) { -- 2.6.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v9 5/6] target-arm: kvm - re-inject guest debug exceptions
From: Alex Bennée <a...@bennee.com> If we can't find details for the debug exception in our debug state then we can assume the exception is due to debugging inside the guest. To inject the exception into the guest state we re-use the TCG exception code (do_interupt). However while guest debugging is in effect we currently can't handle the guest using single step which is heavily used by GDB. Signed-off-by: Alex Bennée <alex.ben...@linaro.org> --- v5: - new for v5 --- target-arm/helper-a64.c | 12 ++-- target-arm/kvm.c| 27 +++ 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c index deb8dbe..fc3ccdf 100644 --- a/target-arm/helper-a64.c +++ b/target-arm/helper-a64.c @@ -25,6 +25,7 @@ #include "qemu/bitops.h" #include "internals.h" #include "qemu/crc32c.h" +#include "sysemu/kvm.h" #include /* For crc32 */ /* C2.4.7 Multiply and divide */ @@ -469,7 +470,8 @@ void aarch64_cpu_do_interrupt(CPUState *cs) new_el); if (qemu_loglevel_mask(CPU_LOG_INT) && !excp_is_internal(cs->exception_index)) { -qemu_log_mask(CPU_LOG_INT, "...with ESR 0x%" PRIx32 "\n", +qemu_log_mask(CPU_LOG_INT, "...with ESR %x/0x%" PRIx32 "\n", + env->exception.syndrome >> ARM_EL_EC_SHIFT, env->exception.syndrome); } @@ -535,6 +537,12 @@ void aarch64_cpu_do_interrupt(CPUState *cs) aarch64_restore_sp(env, new_el); env->pc = addr; -cs->interrupt_request |= CPU_INTERRUPT_EXITTB; + +qemu_log_mask(CPU_LOG_INT, "...to EL%d PC 0x%" PRIx64 " PSTATE 0x%x\n", + new_el, env->pc, pstate_read(env)); + +if (!kvm_enabled()) { +cs->interrupt_request |= CPU_INTERRUPT_EXITTB; +} } #endif diff --git a/target-arm/kvm.c b/target-arm/kvm.c index 1f57e92..4ac177a 100644 --- a/target-arm/kvm.c +++ b/target-arm/kvm.c @@ -529,9 +529,10 @@ static int kvm_handle_debug(CPUState *cs, struct kvm_run *run) struct kvm_debug_exit_arch *arch_info = >debug.arch; int hsr_ec = arch_info->hsr >> ARM_EL_EC_SHIFT; ARMCPU *cpu = ARM_CPU(cs); +CPUClass *cc = CPU_GET_CLASS(cs); CPUARMState *env = >env; -/* Ensure PC is synchronised */ +/* Ensure all state is synchronised */ kvm_cpu_synchronize_state(cs); switch (hsr_ec) { @@ -539,7 +540,14 @@ static int kvm_handle_debug(CPUState *cs, struct kvm_run *run) if (cs->singlestep_enabled) { return true; } else { -error_report("Came out of SINGLE STEP when not enabled"); +/* + * The kernel should have supressed the guests ability to + * single step at this point so something has gone wrong. + */ +error_report("%s: guest single-step while debugging unsupported" + " (%"PRIx64", %"PRIx32")\n", + __func__, env->pc, arch_info->hsr); +return false; } break; case EC_AA64_BKPT: @@ -564,14 +572,17 @@ static int kvm_handle_debug(CPUState *cs, struct kvm_run *run) default: error_report("%s: unhandled debug exit (%"PRIx32", %"PRIx64")\n", __func__, arch_info->hsr, env->pc); +return false; } -/* If we don't handle this it could be it really is for the - guest to handle */ -qemu_log_mask(LOG_UNIMP, - "%s: re-injecting exception not yet implemented" - " (0x%"PRIx32", %"PRIx64")\n", - __func__, hsr_ec, env->pc); +/* If we are not handling the debug exception it must belong to + * the guest. Let's re-use the existing TCG interrupt code to set + * everything up properly + */ +cs->exception_index = EXCP_BKPT; +env->exception.syndrome = arch_info->hsr; +env->exception.vaddress = arch_info->far; +cc->do_interrupt(cs); return false; } -- 2.6.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v9 1/6] target-arm: kvm64 - introduce kvm_arm_init_debug()
As we haven't always had guest debug support we need to probe for it. Additionally we don't do this in the start-up capability code so we don't fall over on old kernels. Signed-off-by: Alex Bennée <alex.ben...@linaro.org> --- target-arm/kvm64.c | 18 ++ 1 file changed, 18 insertions(+) diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c index ceebfeb..d087794 100644 --- a/target-arm/kvm64.c +++ b/target-arm/kvm64.c @@ -25,6 +25,22 @@ #include "internals.h" #include "hw/arm/arm.h" +static bool have_guest_debug; + +/** + * kvm_arm_init_debug() + * @cs: CPUState + * + * Check for guest debug capabilities. + * + */ +static void kvm_arm_init_debug(CPUState *cs) +{ +have_guest_debug = kvm_check_extension(cs->kvm_state, + KVM_CAP_SET_GUEST_DEBUG); +return; +} + static inline void set_feature(uint64_t *features, int feature) { *features |= 1ULL << feature; @@ -121,6 +137,8 @@ int kvm_arch_init_vcpu(CPUState *cs) } cpu->mp_affinity = mpidr & ARM64_AFFINITY_MASK; +kvm_arm_init_debug(cs); + return kvm_arm_init_cpreg_list(cpu); } -- 2.6.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v9 0/6] QEMU support for KVM Guest Debug on arm64
Hi, Here is the latest patch set to support debugging of KVM guests on arm64. The main changes since the last post are that the kernel patches have now been mainlined. As the recent master has synced up headers they are no longer needed for this series. I've add a GDB Python based test into tests/guest-debug. However I haven't plumbed it in to the "make check" machinery as we don't really have a good solution for adding binaries for testing (especially in TCG cases). This patch can be dropped if wanted and I'll just keep it locally. So in summary the changes are: - Kernel ABI now finalised and mainlined - Fixed the handling of watchpoints - Removed manual array fiddling in favour of g_array - Addressed most of Peter Maydell's review comments - The patch series is now checkpatch.pl clean More detailed changelogs are attached to each patch. GIT Repo: The patch series is based off a recent master and can be found at: https://github.com/stsquad/qemu branch: kvm/guest-debug-v9 Alex Bennée (6): target-arm: kvm64 - introduce kvm_arm_init_debug() target-arm: kvm - implement software breakpoints target-arm: kvm - support for single step target-arm: kvm - add support for HW assisted debug target-arm: kvm - re-inject guest debug exceptions tests/guest-debug: introduce basic gdbstub tests target-arm/helper-a64.c | 12 +- target-arm/kvm.c | 137 +++--- target-arm/kvm64.c| 366 ++ target-arm/kvm_arm.h | 38 tests/guest-debug/test-gdbstub.py | 171 ++ 5 files changed, 696 insertions(+), 28 deletions(-) create mode 100644 tests/guest-debug/test-gdbstub.py -- 2.6.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [RFC PATCH v2] os-android: Add support to android platform
diff --git a/default-configs/pci.mak b/default-configs/pci.mak > index 7e10903..e76dd41 100644 > --- a/default-configs/pci.mak > +++ b/default-configs/pci.mak > @@ -35,5 +35,5 @@ CONFIG_SDHCI=y > CONFIG_EDU=y > CONFIG_VGA=y > CONFIG_VGA_PCI=y > -CONFIG_IVSHMEM=$(CONFIG_KVM) > +CONFIG_IVSHMEM=$(call land,$(call lnot,$(CONFIG_ANDROID)),$(CONFIG_KVM)) > CONFIG_ROCKER=y > diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c > index b1beaa6..44beee3 100644 > --- a/hw/i386/kvm/pci-assign.c > +++ b/hw/i386/kvm/pci-assign.c > @@ -22,7 +22,6 @@ > */ > #include > #include > -#include > #include > #include > #include > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h > index ab3c876..f5be51c 100644 > --- a/include/qemu/osdep.h > +++ b/include/qemu/osdep.h > @@ -59,6 +59,10 @@ > #define WEXITSTATUS(x) (x) > #endif > > +#ifdef CONFIG_ANDROID > +#include "sysemu/os-android.h" > +#endif > + > #ifdef _WIN32 > #include "sysemu/os-win32.h" > #endif > @@ -284,4 +288,7 @@ void os_mem_prealloc(int fd, char *area, size_t sz); > > int qemu_read_password(char *buf, int buf_size); > > +int qemu_getdtablesize(void); > + > + > #endif > diff --git a/include/sysemu/os-android.h b/include/sysemu/os-android.h > new file mode 100644 > index 000..4877403 > --- /dev/null > +++ b/include/sysemu/os-android.h > @@ -0,0 +1,12 @@ > +#ifndef QEMU_OS_ANDROID_H > +#define QEMU_OS_ANDROID_H > + > +/* > + * For include the basename prototyping in android. > + */ > +#include > + > +#define getdtablesize qemu_getdtablesize > +#define IOV_MAX 1024 > + > +#endif > diff --git a/kvm-all.c b/kvm-all.c > index c6f5128..f7bb9c7 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -45,7 +45,11 @@ > #endif > > /* KVM uses PAGE_SIZE in its definition of COALESCED_MMIO_MAX */ > +#ifdef PAGE_SIZE > +QEMU_BUILD_BUG_ON(PAGE_SIZE != TARGET_PAGE_SIZE); > +#else > #define PAGE_SIZE TARGET_PAGE_SIZE > +#endif Is this an Android limitation or a fix for KVM in general? If it is the later it should be split into a separate patch. > > //#define DEBUG_KVM > > diff --git a/tests/Makefile b/tests/Makefile > index 34c6136..99faf1f 100644 > --- a/tests/Makefile > +++ b/tests/Makefile > @@ -418,8 +418,10 @@ tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o > libqemuutil.a libqemustub. > tests/test-write-threshold$(EXESUF): tests/test-write-threshold.o > $(block-obj-y) libqemuutil.a libqemustub.a > > ifeq ($(CONFIG_POSIX),y) > +ifneq ($(CONFIG_ANDROID),y) > LIBS += -lutil > endif > +endif > > # QTest rules > > diff --git a/util/oslib-posix.c b/util/oslib-posix.c > index 3ae4987..80d92bc 100644 > --- a/util/oslib-posix.c > +++ b/util/oslib-posix.c > @@ -62,6 +62,8 @@ extern int daemon(int, int); > #include > #include > #include > +#include > +#include > > #ifdef CONFIG_LINUX > #include > @@ -482,3 +484,13 @@ int qemu_read_password(char *buf, int buf_size) > printf("\n"); > return ret; > } > + > +int qemu_getdtablesize(void) > +{ > +struct rlimit r; > + > +if (getrlimit(RLIMIT_NOFILE, ) < 0) { > +return sysconf(_SC_OPEN_MAX); > +} > +return r.rlim_cur; > +} > diff --git a/util/qemu-openpty.c b/util/qemu-openpty.c > index 4c53211..6ef9604 100644 > --- a/util/qemu-openpty.c > +++ b/util/qemu-openpty.c > @@ -51,11 +51,16 @@ > # include > #endif > > -#ifdef __sun__ > +#if defined(__sun__) || defined(CONFIG_ANDROID) > + > /* Once Solaris has openpty(), this is going to be removed. */ > static int openpty(int *amaster, int *aslave, char *name, > struct termios *termp, struct winsize *winp) > { > +#if defined(CONFIG_ANDROID) > +char pty_buf[PATH_MAX]; > +#define ptsname(fd) pty_buf > +#endif Indentation not consistent with the rest of the function. > const char *slave; > int mfd = -1, sfd = -1; > > @@ -67,17 +72,20 @@ static int openpty(int *amaster, int *aslave, char *name, > > if (grantpt(mfd) == -1 || unlockpt(mfd) == -1) > goto err; > - > +#if defined(CONFIG_ANDROID) > +ptsname_r(mfd, pty_buf, PATH_MAX); > +#endif Should we be checking for errors here? > if ((slave = ptsname(mfd)) == NULL) > goto err; > > if ((sfd = open(slave, O_RDONLY | O_NOCTTY)) == -1) > goto err; > > +#ifndef CONFIG_ANDROID > if (ioctl(sfd, I_PUSH, "ptem") == -1 || > (termp != NULL && tcgetattr(sfd, termp) < 0)) > goto err; > - > +#endif > if (amaster) > *amaster = mfd; > if (aslave) > @@ -93,7 +101,8 @@ err: > close(mfd); > return -1; > } > - > +#endif > +#ifdef __sun__ > static void cfmakeraw (struct termios *termios_p) > { > termios_p->c_iflag &= > @@ -112,7 +121,7 @@ int qemu_openpty_raw(int *aslave, char *pty_name) > { > int amaster; > struct termios tty; > -#if defined(__OpenBSD__) || defined(__DragonFly__) > +#if defined(__OpenBSD__) || defined(__DragonFly__) || defined(CONFIG_ANDROID) > char pty_buf[PATH_MAX]; > #define q_ptsname(x) pty_buf > #else -- Alex Bennée -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64: KVM: Fix user access for debug registers
Marc Zyngier <marc.zyng...@arm.com> writes: > When setting the debug register from userspace, make sure that > copy_from_user() is called with its parameters in the expected > order. It otherwise doesn't do what you think. Oops. Well that exposes a big hole in my testing. While I tested debugging inside the guest worked before and after being guest debugged I think GDBs tendency to reload all the debug registers between each step may have masked this. Debugging GDB in action or some sort of migration event would of course screw this up but I'm afraid my testing wasn't evil enough. Anyway have a: Reviewed-by: Alex Bennée <alex.ben...@linaro.org> > > Reported-by: Peter Maydell <peter.mayd...@linaro.org> > Cc: Alex Bennée <alex.ben...@linaro.org> > Fixes: 84e690bfbed1 ("KVM: arm64: introduce vcpu->arch.debug_ptr") > Signed-off-by: Marc Zyngier <marc.zyng...@arm.com> > --- > arch/arm64/kvm/sys_regs.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index b41607d..1d0463e 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -272,7 +272,7 @@ static int set_bvr(struct kvm_vcpu *vcpu, const struct > sys_reg_desc *rd, > { > __u64 *r = >arch.vcpu_debug_state.dbg_bvr[rd->reg]; > > - if (copy_from_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0) > + if (copy_from_user(r, uaddr, KVM_REG_SIZE(reg->id)) != 0) > return -EFAULT; > return 0; > } > @@ -314,7 +314,7 @@ static int set_bcr(struct kvm_vcpu *vcpu, const struct > sys_reg_desc *rd, > { > __u64 *r = >arch.vcpu_debug_state.dbg_bcr[rd->reg]; > > - if (copy_from_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0) > + if (copy_from_user(r, uaddr, KVM_REG_SIZE(reg->id)) != 0) > return -EFAULT; > > return 0; > @@ -358,7 +358,7 @@ static int set_wvr(struct kvm_vcpu *vcpu, const struct > sys_reg_desc *rd, > { > __u64 *r = >arch.vcpu_debug_state.dbg_wvr[rd->reg]; > > - if (copy_from_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0) > + if (copy_from_user(r, uaddr, KVM_REG_SIZE(reg->id)) != 0) > return -EFAULT; > return 0; > } > @@ -400,7 +400,7 @@ static int set_wcr(struct kvm_vcpu *vcpu, const struct > sys_reg_desc *rd, > { > __u64 *r = >arch.vcpu_debug_state.dbg_wcr[rd->reg]; > > - if (copy_from_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0) > + if (copy_from_user(r, uaddr, KVM_REG_SIZE(reg->id)) != 0) > return -EFAULT; > return 0; > } -- Alex Bennée -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64: KVM: Fix user access for debug registers
Christoffer Dall <christoffer.d...@linaro.org> writes: > On Wed, Sep 16, 2015 at 11:41:10AM +0100, Marc Zyngier wrote: >> When setting the debug register from userspace, make sure that >> copy_from_user() is called with its parameters in the expected >> order. It otherwise doesn't do what you think. >> >> Reported-by: Peter Maydell <peter.mayd...@linaro.org> >> Cc: Alex Bennée <alex.ben...@linaro.org> >> Fixes: 84e690bfbed1 ("KVM: arm64: introduce vcpu->arch.debug_ptr") >> Signed-off-by: Marc Zyngier <marc.zyng...@arm.com> > > yikes! OK I'm now muchly confused as to how it could have worked... > > Reviewed-by: Christoffer Dall <christoffer.d...@linaro.org> -- Alex Bennée -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] Help debugging a regression in KVM Module
Peter Lieven p...@kamp.de writes: Hi, some time a go I stumbled across a regression in the KVM Module that has been introduced somewhere between 3.17 and 3.19. I have a rather old openSUSE guest with an XFS filesystem which realiably crashes after some live migrations. I originally believed that the issue might be related to my setup with a 3.12 host kernel and kvm-kmod 3.19, but I now found that it is also still present with a 3.19 host kernel with included 3.19 kvm module. My idea was to continue testing on a 3.12 host kernel and then bisect all commits to the kvm related parts. Now my question is how to best bisect only kvm related changes (those that go into kvm-kmod)? In general I don't bother. As it is a bisection you eliminate half the commits at a time you get their fairly quickly anyway. However you can tell bisect which parts of the tree you car about: git bisect start -- arch/arm64/kvm include/linux/kvm* include/uapi/linux/kvm* virt/kvm/ Thanks, Peter -- Alex Bennée -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-unit-tests PATCH 3/3] arm/run: use ACCEL to choose between kvm and tcg
Andrew Jones drjo...@redhat.com writes: Inspired by a patch by Alex Bennée. This version uses a new unittests.cfg variable and includes support for DRYRUN. Signed-off-by: Andrew Jones drjo...@redhat.com --- Another difference with Alex's patch is we no longer output 'Running with TCG', as I don't think it's necessary. The command line captures that, and the whole point of the patch is to silence the 'kvm accelerator not found.' messages anyway. Yeah that makes sense. arm/run| 36 ++-- arm/unittests.cfg | 4 +++- run_tests.sh | 3 ++- scripts/functions.bash | 8 ++-- 4 files changed, 41 insertions(+), 10 deletions(-) snip I see your re-posting so I'll do a full review then. Is there a branch I can re-base my stuff on top of for now? -- Alex Bennée -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-unit-tests PATCH v5 11/11] new: arm/barrier-test for memory barriers
alvise rigo a.r...@virtualopensystems.com writes: On Mon, Aug 3, 2015 at 6:06 PM, Alex Bennée alex.ben...@linaro.org wrote: alvise rigo a.r...@virtualopensystems.com writes: On Mon, Aug 3, 2015 at 12:30 PM, Alex Bennée alex.ben...@linaro.org wrote: alvise rigo a.r...@virtualopensystems.com writes: Hi Alex, Nice set of tests, they are proving to be helpful. One question below. snip Why are we calling these last two instructions with the 'eq' suffix? Shouldn't we just strex r1, r0, [sptr] and then cmp r1, #0? Possibly, my armv7 is a little rusty. I'm just looking at tweaking this test now so I'll try and clean that up. Please find the updated test attached. I've also included some new test modes. In theory the barrier test by itself should still fail but it Thanks, I will check them out. passes on real ARMv7 as well as TCG. I'm trying to run the test on a heavier core-ed ARMv7 to check. I suspect we get away with it on ARMv7-on-x86_64 due to the strong ordering of the x86. The excl and acqrel tests now run without issue (although again plain acqrel semantics shouldn't stop a race corrupting shared_value). I suppose that, in order to have some race conditions due to a lack of a proper emulation of barriers and acqrel instructions, we need a test that does not involve atomic instructions at all, to reduce the emulation overhead as much as possible. Does this sound reasonable? I'm writing a lockless test now which uses just barriers and a postbox style signal. But as I say I need to understand why the pure barrier tests still works when it really shouldn't. I'll tweak the v8 versions of the test tomorrow. -- Alex Bennée -- Alex Bennée -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-unit-tests PATCH v5 11/11] new: arm/barrier-test for memory barriers
alvise rigo a.r...@virtualopensystems.com writes: On Mon, Aug 3, 2015 at 12:30 PM, Alex Bennée alex.ben...@linaro.org wrote: alvise rigo a.r...@virtualopensystems.com writes: Hi Alex, Nice set of tests, they are proving to be helpful. One question below. snip Why are we calling these last two instructions with the 'eq' suffix? Shouldn't we just strex r1, r0, [sptr] and then cmp r1, #0? Possibly, my armv7 is a little rusty. I'm just looking at tweaking this test now so I'll try and clean that up. Please find the updated test attached. I've also included some new test modes. In theory the barrier test by itself should still fail but it passes on real ARMv7 as well as TCG. I'm trying to run the test on a heavier core-ed ARMv7 to check. I suspect we get away with it on ARMv7-on-x86_64 due to the strong ordering of the x86. The excl and acqrel tests now run without issue (although again plain acqrel semantics shouldn't stop a race corrupting shared_value). I'll tweak the v8 versions of the test tomorrow. -- Alex Bennée From 0953549985134268bf9079a7a01b2631d8a4fdee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= a...@bennee.com Date: Thu, 30 Jul 2015 15:13:33 + Subject: [kvm-unit-tests PATCH] arm/barrier-test: add memory barrier tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This test has been written mainly to stress multi-threaded TCG behaviour but will demonstrate failure by default on real hardware. The test takes the following parameters: - lock use GCC's locking semantics - atomic use GCC's __atomic primitives - barrier use plain dmb() barriers - wfelock use WaitForEvent sleep - excl use load/store exclusive semantics - acqrel use acquire/release semantics Also two more options allow the test to be tweaked - noshuffle disables the memory shuffling - count=%ld set your own per-CPU increment count Signed-off-by: Alex Bennée alex.ben...@linaro.org --- v2 - Don't use thumb style strexeq stuff - Add atomic, barrier and wfelock tests - Add count/noshuffle test controls --- arm/barrier-test.c | 284 +++ config/config-arm-common.mak | 2 + 2 files changed, 286 insertions(+) create mode 100644 arm/barrier-test.c diff --git a/arm/barrier-test.c b/arm/barrier-test.c new file mode 100644 index 000..765f8f6 --- /dev/null +++ b/arm/barrier-test.c @@ -0,0 +1,284 @@ +#include libcflat.h +#include asm/smp.h +#include asm/cpumask.h +#include asm/barrier.h +#include asm/mmu.h + +#include prng.h + +#define MAX_CPUS 8 + +/* How many increments to do */ +static int increment_count = 1000; +static int do_shuffle = 1; + + +/* shared value all the tests attempt to safely increment */ +static unsigned int shared_value; + +/* PAGE_SIZE * uint32_t means we span several pages */ +static uint32_t memory_array[PAGE_SIZE]; + +/* We use the alignment of the following to ensure accesses to locking + * and synchronisation primatives don't interfere with the page of the + * shared value + */ +__attribute__((aligned(PAGE_SIZE))) static unsigned int per_cpu_value[MAX_CPUS]; +__attribute__((aligned(PAGE_SIZE))) static cpumask_t smp_test_complete; +__attribute__((aligned(PAGE_SIZE))) static int global_lock; + +struct isaac_ctx prng_context[MAX_CPUS]; + +void (*inc_fn)(void); + + +/* In any SMP setting this *should* fail due to cores stepping on + * each other updating the shared variable + */ +static void increment_shared(void) +{ + shared_value++; +} + +/* GCC __sync primitives are deprecated in favour of __atomic */ +static void increment_shared_with_lock(void) +{ + while (__sync_lock_test_and_set(global_lock, 1)); + shared_value++; + __sync_lock_release(global_lock); +} + +/* In practice even __ATOMIC_RELAXED uses ARM's ldxr/stex exclusive + * semantics */ +static void increment_shared_with_atomic(void) +{ + __atomic_add_fetch(shared_value, 1, __ATOMIC_SEQ_CST); +} + + +/* By themselves barriers do not gaurentee atomicity */ +static void increment_shared_with_barrier(void) +{ +#if defined (__LP64__) || defined (_LP64) +#else + asm volatile( + ldr r0, [%[sptr]]\n + dmb\n + add r0, r0, #0x1\n + str r1, r0, [%[sptr]]\n + dmb\n + : /* out */ + : [sptr] r (shared_value) /* in */ + : r0, r1, cc); +#endif +} + +/* + * Load/store exclusive with WFE (wait-for-event) + */ + +static void increment_shared_with_wfelock(void) +{ +#if defined (__LP64__) || defined (_LP64) +#else + asm volatile( + mov r1, #1\n + 1: ldrex r0, [%[lock]]\n + cmp r0, #0\n + wfene\n + strexeq r0, r1, [%[lock]]\n + cmpeq r0, #0\n + bne 1b\n + dmb\n + /* lock held */ + ldr r0, [%[sptr]]\n + add r0, r0, #0x1\n + str r0, [%[sptr]]\n + /* now release */ + mov r0, #0\n + dmb\n + str r0, [%[lock]]\n + dsb\n + sev\n + : /* out */ + : [lock] r (global_lock), [sptr] r (shared_value) /* in */ + : r0, r1, cc); +#endif +} + + +/* + * Hand-written version of the load/store exclusive
Re: [kvm-unit-tests PATCH v5 11/11] new: arm/barrier-test for memory barriers
alvise rigo a.r...@virtualopensystems.com writes: Hi Alex, Nice set of tests, they are proving to be helpful. One question below. On Fri, Jul 31, 2015 at 5:54 PM, Alex Bennée alex.ben...@linaro.org wrote: From: Alex Bennée a...@bennee.com This test has been written mainly to stress multi-threaded TCG behaviour but will demonstrate failure by default on real hardware. The test takes the following parameters: - lock use GCC's locking semantics - excl use load/store exclusive semantics - acqrel use acquire/release semantics Currently excl/acqrel lock up on MTTCG Signed-off-by: Alex Bennée alex.ben...@linaro.org --- arm/barrier-test.c | 206 +++ config/config-arm-common.mak | 2 + 2 files changed, 208 insertions(+) create mode 100644 arm/barrier-test.c diff --git a/arm/barrier-test.c b/arm/barrier-test.c new file mode 100644 index 000..53d690b --- /dev/null +++ b/arm/barrier-test.c @@ -0,0 +1,206 @@ +#include libcflat.h +#include asm/smp.h +#include asm/cpumask.h +#include asm/barrier.h +#include asm/mmu.h + +#include prng.h + +#define MAX_CPUS 4 + +/* How many increments to do */ +static int increment_count = 1000; + + +/* shared value, we use the alignment to ensure the global_lock value + * doesn't share a page */ +static unsigned int shared_value; + +/* PAGE_SIZE * uint32_t means we span several pages */ +static uint32_t memory_array[PAGE_SIZE]; + +__attribute__((aligned(PAGE_SIZE))) static unsigned int per_cpu_value[MAX_CPUS]; +__attribute__((aligned(PAGE_SIZE))) static cpumask_t smp_test_complete; +__attribute__((aligned(PAGE_SIZE))) static int global_lock; + +struct isaac_ctx prng_context[MAX_CPUS]; + +void (*inc_fn)(void); + +static void lock(int *lock_var) +{ + while (__sync_lock_test_and_set(lock_var, 1)); +} +static void unlock(int *lock_var) +{ + __sync_lock_release(lock_var); +} + +static void increment_shared(void) +{ + shared_value++; +} + +static void increment_shared_with_lock(void) +{ + lock(global_lock); + shared_value++; + unlock(global_lock); +} + +static void increment_shared_with_excl(void) +{ +#if defined (__LP64__) || defined (_LP64) + asm volatile( + 1: ldxrw0, [%[sptr]]\n + add w0, w0, #0x1\n + stxrw1, w0, [%[sptr]]\n + cbnzw1, 1b\n + : /* out */ + : [sptr] r (shared_value) /* in */ + : w0, w1, cc); +#else + asm volatile( + 1: ldrex r0, [%[sptr]]\n + add r0, r0, #0x1\n + strexeq r1, r0, [%[sptr]]\n + cmpeq r1, #0\n Why are we calling these last two instructions with the 'eq' suffix? Shouldn't we just strex r1, r0, [sptr] and then cmp r1, #0? Possibly, my armv7 is a little rusty. I'm just looking at tweaking this test now so I'll try and clean that up. Thank you, alvise + bne 1b\n + : /* out */ + : [sptr] r (shared_value) /* in */ + : r0, r1, cc); +#endif +} + +static void increment_shared_with_acqrel(void) +{ +#if defined (__LP64__) || defined (_LP64) + asm volatile( + ldarw0, [%[sptr]]\n + add w0, w0, #0x1\n + str w0, [%[sptr]]\n + : /* out */ + : [sptr] r (shared_value) /* in */ + : w0); +#else + /* ARMv7 has no acquire/release semantics but we +* can ensure the results of the write are propagated +* with the use of barriers. +*/ + asm volatile( + 1: ldrex r0, [%[sptr]]\n + add r0, r0, #0x1\n + strexeq r1, r0, [%[sptr]]\n + cmpeq r1, #0\n + bne 1b\n + dmb\n + : /* out */ + : [sptr] r (shared_value) /* in */ + : r0, r1, cc); +#endif + +} + +/* The idea of this is just to generate some random load/store + * activity which may or may not race with an un-barried incremented + * of the shared counter + */ +static void shuffle_memory(int cpu) +{ + int i; + uint32_t lspat = isaac_next_uint32(prng_context[cpu]); + uint32_t seq = isaac_next_uint32(prng_context[cpu]); + int count = seq 0x1f; + uint32_t val=0; + + seq = 5; + + for (i=0; icount; i++) { + int index = seq ~PAGE_MASK; + if (lspat 1) { + val ^= memory_array[index]; + } else { + memory_array[index] = val; + } + seq = PAGE_SHIFT; + seq ^= lspat; + lspat = 1; + } + +} + +static void do_increment(void) +{ + int i; + int cpu = smp_processor_id(); + + printf(CPU%d online\n, cpu); + + for (i=0; i increment_count; i
[kvm-unit-tests PATCH v5 07/11] new arm/tlbflush-test: TLB torture test
This adds a fairly brain dead torture test for TLB flushes intended for stressing the MTTCG QEMU build. It takes the usual -smp option for multiple CPUs. By default it CPU0 will do a TLBIALL flush after each cycle. You can pass options via -append to control additional aspects of the test: - page flush each page in turn (one per function) - self do the flush after each computation cycle - verbose report progress on each computation cycle Signed-off-by: Alex Bennée alex.ben...@linaro.org --- v2 - rename to tlbflush-test - made makefile changes cleaner - added self/other flush mode - create specific prefix - whitespace fixes --- arm/tlbflush-test.c | 194 +++ config/config-arm-common.mak | 7 +- 2 files changed, 198 insertions(+), 3 deletions(-) create mode 100644 arm/tlbflush-test.c diff --git a/arm/tlbflush-test.c b/arm/tlbflush-test.c new file mode 100644 index 000..0375ad9 --- /dev/null +++ b/arm/tlbflush-test.c @@ -0,0 +1,194 @@ +#include libcflat.h +#include asm/smp.h +#include asm/cpumask.h +#include asm/barrier.h +#include asm/mmu.h + +#define SEQ_LENGTH 10 +#define SEQ_HASH 0x7cd707fe + +static cpumask_t smp_test_complete; +static int flush_count = 100; +static int flush_self = 0; +static int flush_page = 0; +static int flush_verbose = 0; + +/* Work functions + * + * These work functions need to be: + * + * - page aligned, so we can flush one function at a time + * - have branches, so QEMU TCG generates multiple basic blocks + * - call across pages, so we exercise the TCG basic block slow path + */ + +/* Adler32 */ +__attribute__((aligned(PAGE_SIZE))) uint32_t hash_array(const void *buf, + size_t buflen) +{ + const uint8_t *data = (uint8_t *) buf; + uint32_t s1 = 1; + uint32_t s2 = 0; + + for (size_t n = 0; n buflen; n++) { + s1 = (s1 + data[n]) % 65521; + s2 = (s2 + s1) % 65521; + } + return (s2 16) | s1; +} + +__attribute__((aligned(PAGE_SIZE))) void create_fib_sequence(int length, + unsigned int *array) +{ + int i; + + /* first two values */ + array[0] = 0; + array[1] = 1; + for (i=2; ilength; i++) { + array[i] = array[i-2] + array[i-1]; + } +} + +__attribute__((aligned(PAGE_SIZE))) unsigned long long factorial(unsigned int n) +{ + unsigned int i; + unsigned long long fac = 1; + for (i=1; i=n; i++) + { + fac = fac * i; + } + return fac; +} + +__attribute__((aligned(PAGE_SIZE))) void factorial_array +(unsigned int n, unsigned int *input, unsigned long long *output) +{ + unsigned int i; + for (i=0; in; i++) { + output[i] = factorial(input[i]); + } +} + +__attribute__((aligned(PAGE_SIZE))) unsigned int do_computation(void) +{ + unsigned int fib_array[SEQ_LENGTH]; + unsigned long long facfib_array[SEQ_LENGTH]; + uint32_t fib_hash, facfib_hash; + + create_fib_sequence(SEQ_LENGTH, fib_array[0]); + fib_hash = hash_array(fib_array[0], sizeof(fib_array)); + factorial_array(SEQ_LENGTH, fib_array[0], facfib_array[0]); + facfib_hash = hash_array(facfib_array[0], sizeof(facfib_array)); + + return (fib_hash ^ facfib_hash); +} + +/* This provides a table of the work functions so we can flush each + * page individually + */ +static void * pages[] = {hash_array, create_fib_sequence, factorial, +factorial_array, do_computation}; + +static void do_flush(int i) +{ + if (flush_page) { + flush_tlb_page((unsigned long)pages[i % ARRAY_SIZE(pages)]); + } else { + flush_tlb_all(); + } +} + + +static void just_compute(void) +{ + int i, errors = 0; + int cpu = smp_processor_id(); + + uint32_t result; + + printf(CPU%d online\n, cpu); + + for (i=0; i flush_count; i++) { + result = do_computation(); + + if (result != SEQ_HASH) { + errors++; + printf(CPU%d: seq%d 0x%x!=0x%x\n, + cpu, i, result, SEQ_HASH); + } + + if (flush_verbose (i % 1000) == 0) { + printf(CPU%d: seq%d\n, cpu, i); + } + + if (flush_self) { + do_flush(i); + } + } + + report(CPU%d: Done - Errors: %d\n, errors == 0, cpu, errors); + + cpumask_set_cpu(cpu, smp_test_complete); + if (cpu != 0) + halt(); +} + +static void just_flush(void) +{ + int cpu = smp_processor_id(); + int i = 0; + + /* set our CPU as done, keep flushing until everyone else + finished */ + cpumask_set_cpu(cpu, smp_test_complete); + + while (!cpumask_full(smp_test_complete
[kvm-unit-tests PATCH v5 03/11] configure: emit HOST=$host to config.mak
This is useful information for the run scripts to know, especially if they want to drop to using TCG. Signed-off-by: Alex Bennée alex.ben...@linaro.org Reviewed-by: Andrew Jones drjo...@redhat.com --- v3 - add r-b tag --- configure | 2 ++ 1 file changed, 2 insertions(+) diff --git a/configure b/configure index b2ad32a..078b70c 100755 --- a/configure +++ b/configure @@ -7,6 +7,7 @@ ld=ld objcopy=objcopy ar=ar arch=`uname -m | sed -e s/i.86/i386/ | sed -e 's/arm.*/arm/'` +host=$arch cross_prefix= usage() { @@ -122,6 +123,7 @@ ln -s $asm lib/asm cat EOF config.mak PREFIX=$prefix KERNELDIR=$(readlink -f $kerneldir) +HOST=$host ARCH=$arch ARCH_NAME=$arch_name PROCESSOR=$processor -- 2.5.0 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH v5 06/11] lib/arm: add flush_tlb_page mmu function
This introduces a new flush_tlb_page function which does exactly what you expect. It's going to be useful for the future TLB torture test. Signed-off-by: Alex Bennée alex.ben...@linaro.org --- lib/arm/asm/mmu.h | 11 +++ lib/arm64/asm/mmu.h | 8 2 files changed, 19 insertions(+) diff --git a/lib/arm/asm/mmu.h b/lib/arm/asm/mmu.h index c1bd01c..2bb0cde 100644 --- a/lib/arm/asm/mmu.h +++ b/lib/arm/asm/mmu.h @@ -14,8 +14,11 @@ #define PTE_AF PTE_EXT_AF #define PTE_WBWA L_PTE_MT_WRITEALLOC +/* See B3.18.7 TLB maintenance operations */ + static inline void local_flush_tlb_all(void) { + /* TLBIALL */ asm volatile(mcr p15, 0, %0, c8, c7, 0 :: r (0)); dsb(); isb(); @@ -27,6 +30,14 @@ static inline void flush_tlb_all(void) local_flush_tlb_all(); } +static inline void flush_tlb_page(unsigned long vaddr) +{ + /* TLBIMVAA */ + asm volatile(mcr p15, 0, %0, c8, c7, 3 :: r (vaddr)); + dsb(); + isb(); +} + #include asm/mmu-api.h #endif /* __ASMARM_MMU_H_ */ diff --git a/lib/arm64/asm/mmu.h b/lib/arm64/asm/mmu.h index 18b4d6b..3bc31c9 100644 --- a/lib/arm64/asm/mmu.h +++ b/lib/arm64/asm/mmu.h @@ -19,6 +19,14 @@ static inline void flush_tlb_all(void) isb(); } +static inline void flush_tlb_page(unsigned long vaddr) +{ + unsigned long page = vaddr 12; + dsb(ishst); + asm(tlbi vaae1is, %0 :: r (page)); + dsb(ish); +} + #include asm/mmu-api.h #endif /* __ASMARM64_MMU_H_ */ -- 2.5.0 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH v5 00/11] My current MTTCG tests
Hi, This is the current state of my MTTCG tests based on the KVM's unit testing framework. The earlier patches in the series have already been reviewed and will (with the exception of the emacs patch) be making their way upstream. There are a couple of addition to library functions: - printf %u suppport - flush_tlb_page for arm and arm64 - a generic prng from CCAN The two actual tests are: - tlbflush-test - barrier-test The latter barrier test hangs the current -v6 MTTCG patch set in both excl and acqrel modes and will make a good torture test for Alvise's atomic patch set. I suspect the load/store ordering issues will show up better once tested on a weak-ordered backend. I'm open to suggestions for other tests worth adding to show up the issues. The github tree can be found at: https://github.com/stsquad/kvm-unit-tests/tree/current-mttcg-tests Alex Bennée (11): arm/run: set indentation defaults for emacs README: add some CONTRIBUTING notes configure: emit HOST=$host to config.mak arm/run: introduce usingkvm var and use it lib/printf: support the %u unsigned fmt field lib/arm: add flush_tlb_page mmu function new arm/tlbflush-test: TLB torture test arm/unittests.cfg: add the tlbflush tests arm: query /dev/kvm for maximum vcpus new: add isaac prng library from CCAN new: arm/barrier-test for memory barriers README | 26 ++ arm/barrier-test.c | 206 +++ arm/run | 19 +++- arm/tlbflush-test.c | 194 arm/unittests.cfg| 26 +- arm/utils/kvm-query.c| 41 + config/config-arm-common.mak | 18 +++- configure| 2 + lib/arm/asm/mmu.h| 11 +++ lib/arm64/asm/mmu.h | 8 ++ lib/printf.c | 13 +++ lib/prng.c | 162 ++ lib/prng.h | 82 + 13 files changed, 801 insertions(+), 7 deletions(-) create mode 100644 arm/barrier-test.c create mode 100644 arm/tlbflush-test.c create mode 100644 arm/utils/kvm-query.c create mode 100644 lib/prng.c create mode 100644 lib/prng.h -- 2.5.0 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH v5 08/11] arm/unittests.cfg: add the tlbflush tests
--- arm/unittests.cfg | 24 1 file changed, 24 insertions(+) diff --git a/arm/unittests.cfg b/arm/unittests.cfg index ee655b2..19d72ad 100644 --- a/arm/unittests.cfg +++ b/arm/unittests.cfg @@ -35,3 +35,27 @@ file = selftest.flat smp = $(getconf _NPROCESSORS_CONF) extra_params = -append 'smp' groups = selftest + +# TLB Torture Tests +[tlbflush::all_other] +file = tlbflush-test.flat +smp = $(getconf _NPROCESSORS_CONF) +groups = tlbflush + +[tlbflush::page_other] +file = tlbflush-test.flat +smp = $(getconf _NPROCESSORS_CONF) +extra_params = -append 'page' +groups = tlbflush + +[tlbflush::all_self] +file = tlbflush-test.flat +smp = $(getconf _NPROCESSORS_CONF) +extra_params = -append 'self' +groups = tlbflush + +[tlbflush::page_self] +file = tlbflush-test.flat +smp = $(getconf _NPROCESSORS_CONF) +extra_params = -append 'page self' +groups = tlbflush -- 2.5.0 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH v5 09/11] arm: query /dev/kvm for maximum vcpus
From: Alex Bennée a...@bennee.com The previous $(getconf _NPROCESSORS_CONF) isn't correct as the default maximum VCPU configuration is 4 on arm64 machines which typically have more actual cores. This introduces a simple utility program to query the KVM capabilities and use the correct maximum number of vcpus. [FOR DISCUSSION: this fails on TCG which could use _NPROCESSORS_CONF] Signed-off-by: Alex Bennée alex.ben...@linaro.org --- arm/unittests.cfg| 10 +- arm/utils/kvm-query.c| 41 + config/config-arm-common.mak | 8 +++- 3 files changed, 53 insertions(+), 6 deletions(-) create mode 100644 arm/utils/kvm-query.c diff --git a/arm/unittests.cfg b/arm/unittests.cfg index 19d72ad..7a3a32b 100644 --- a/arm/unittests.cfg +++ b/arm/unittests.cfg @@ -32,30 +32,30 @@ groups = selftest # Test SMP support [selftest::smp] file = selftest.flat -smp = $(getconf _NPROCESSORS_CONF) +smp = $(./arm/utils/kvm-query max_vcpu) extra_params = -append 'smp' groups = selftest # TLB Torture Tests [tlbflush::all_other] file = tlbflush-test.flat -smp = $(getconf _NPROCESSORS_CONF) +smp = $(./arm/utils/kvm-query max_vcpu) groups = tlbflush [tlbflush::page_other] file = tlbflush-test.flat -smp = $(getconf _NPROCESSORS_CONF) +smp = $(./arm/utils/kvm-query max_vcpu) extra_params = -append 'page' groups = tlbflush [tlbflush::all_self] file = tlbflush-test.flat -smp = $(getconf _NPROCESSORS_CONF) +smp = $(./arm/utils/kvm-query max_vcpu) extra_params = -append 'self' groups = tlbflush [tlbflush::page_self] file = tlbflush-test.flat -smp = $(getconf _NPROCESSORS_CONF) +smp = $(./arm/utils/kvm-query max_vcpu) extra_params = -append 'page self' groups = tlbflush diff --git a/arm/utils/kvm-query.c b/arm/utils/kvm-query.c new file mode 100644 index 000..4f979b1 --- /dev/null +++ b/arm/utils/kvm-query.c @@ -0,0 +1,41 @@ +/* + * kvm-query.c + * + * A simple utility to query KVM capabilities. + */ + +#include sys/ioctl.h +#include sys/types.h +#include sys/stat.h +#include fcntl.h +#include unistd.h + +#include string.h +#include stdio.h + +#include linux/kvm.h + +int get_max_vcpu(void) +{ + int fd = open(/dev/kvm, O_RDWR); + if (fd0) { + int ret = ioctl(fd, KVM_CHECK_EXTENSION, KVM_CAP_MAX_VCPUS); + printf(%d\n, ret 0 ? ret : 0); + close(fd); + return 0; + } else { + return -1; + } +} + +int main(int argc, char **argv) +{ + for (int i=0; iargc; i++) { + char *arg = argv[i]; + if (strcmp(arg, max_vcpu) == 0) { + return get_max_vcpu(); + } + } + + return -1; +} diff --git a/config/config-arm-common.mak b/config/config-arm-common.mak index 164199b..9c7607b 100644 --- a/config/config-arm-common.mak +++ b/config/config-arm-common.mak @@ -13,7 +13,9 @@ tests-common = $(TEST_DIR)/selftest.flat tests-common += $(TEST_DIR)/spinlock-test.flat tests-common += $(TEST_DIR)/tlbflush-test.flat -all: test_cases +utils-common = $(TEST_DIR)/utils/kvm-query + +all: test_cases utils ## phys_base = $(LOADADDR) @@ -58,6 +60,9 @@ FLATLIBS = $(libcflat) $(LIBFDT_archive) $(libgcc) $(libeabi) $(libeabi): $(eabiobjs) $(AR) rcs $@ $^ +$(TEST_DIR)/utils/%: $(TEST_DIR)/utils/%.c + $(CC) -std=gnu99 -o $@ $^ + arm_clean: libfdt_clean asm_offsets_clean $(RM) $(TEST_DIR)/*.{o,flat,elf} $(libeabi) $(eabiobjs) \ $(TEST_DIR)/.*.d lib/arm/.*.d @@ -69,6 +74,7 @@ tests_and_config = $(TEST_DIR)/*.flat $(TEST_DIR)/unittests.cfg generated_files = $(asm-offsets) test_cases: $(generated_files) $(tests-common) $(tests) +utils: $(utils-common) $(TEST_DIR)/selftest.elf: $(cstart.o) $(TEST_DIR)/selftest.o $(TEST_DIR)/spinlock-test.elf: $(cstart.o) $(TEST_DIR)/spinlock-test.o -- 2.5.0 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH v5 04/11] arm/run: introduce usingkvm var and use it
This makes the script a little cleaner by only checking for KVM support in one place. If KVM isn't available we can fall back to TCG emulation and echo the fact to the screen rather than let QEMU complain. Signed-off-by: Alex Bennée alex.ben...@linaro.org Reviewed-by: Andrew Jones drjo...@redhat.com --- v2 - rm redundant M= statement v3 - make usingkvm use yes - merge patches 3/4 into one v4 - use single quotes consistently - add r-b tag --- arm/run | 18 -- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/arm/run b/arm/run index 6b42a2e..6b3d558 100755 --- a/arm/run +++ b/arm/run @@ -8,6 +8,15 @@ fi source config.mak processor=$PROCESSOR +# Default to using KVM if available and on the right ARM host +if [ -c /dev/kvm ]; then + if [ $HOST = arm ] [ $ARCH = arm ]; then + usingkvm=yes + elif [ $HOST = aarch64 ]; then + usingkvm=yes + fi +fi + qemu=${QEMU:-qemu-system-$ARCH_NAME} qpath=$(which $qemu 2/dev/null) @@ -22,6 +31,12 @@ if ! $qemu -machine '?' 21 | grep 'ARM Virtual Machine' /dev/null; then fi M='-machine virt' +if [ $usingkvm = yes ]; then + M+=',accel=kvm' +else + echo Running with TCG + M+=',accel=tcg' +fi if ! $qemu $M -device '?' 21 | grep virtconsole /dev/null; then echo $qpath doesn't support virtio-console for chr-testdev. Exiting. @@ -34,12 +49,11 @@ if $qemu $M -chardev testdev,id=id -initrd . 21 \ exit 2 fi -M='-machine virt,accel=kvm:tcg' chr_testdev='-device virtio-serial-device' chr_testdev+=' -device virtconsole,chardev=ctd -chardev testdev,id=ctd' # arm64 must use '-cpu host' with kvm -if [ $(arch) = aarch64 ] [ $ARCH = arm64 ] [ -c /dev/kvm ]; then +if [ $usingkvm = yes ] [ $ARCH = arm64 ]; then processor=host fi -- 2.5.0 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH v5 11/11] new: arm/barrier-test for memory barriers
From: Alex Bennée a...@bennee.com This test has been written mainly to stress multi-threaded TCG behaviour but will demonstrate failure by default on real hardware. The test takes the following parameters: - lock use GCC's locking semantics - excl use load/store exclusive semantics - acqrel use acquire/release semantics Currently excl/acqrel lock up on MTTCG Signed-off-by: Alex Bennée alex.ben...@linaro.org --- arm/barrier-test.c | 206 +++ config/config-arm-common.mak | 2 + 2 files changed, 208 insertions(+) create mode 100644 arm/barrier-test.c diff --git a/arm/barrier-test.c b/arm/barrier-test.c new file mode 100644 index 000..53d690b --- /dev/null +++ b/arm/barrier-test.c @@ -0,0 +1,206 @@ +#include libcflat.h +#include asm/smp.h +#include asm/cpumask.h +#include asm/barrier.h +#include asm/mmu.h + +#include prng.h + +#define MAX_CPUS 4 + +/* How many increments to do */ +static int increment_count = 1000; + + +/* shared value, we use the alignment to ensure the global_lock value + * doesn't share a page */ +static unsigned int shared_value; + +/* PAGE_SIZE * uint32_t means we span several pages */ +static uint32_t memory_array[PAGE_SIZE]; + +__attribute__((aligned(PAGE_SIZE))) static unsigned int per_cpu_value[MAX_CPUS]; +__attribute__((aligned(PAGE_SIZE))) static cpumask_t smp_test_complete; +__attribute__((aligned(PAGE_SIZE))) static int global_lock; + +struct isaac_ctx prng_context[MAX_CPUS]; + +void (*inc_fn)(void); + +static void lock(int *lock_var) +{ + while (__sync_lock_test_and_set(lock_var, 1)); +} +static void unlock(int *lock_var) +{ + __sync_lock_release(lock_var); +} + +static void increment_shared(void) +{ + shared_value++; +} + +static void increment_shared_with_lock(void) +{ + lock(global_lock); + shared_value++; + unlock(global_lock); +} + +static void increment_shared_with_excl(void) +{ +#if defined (__LP64__) || defined (_LP64) + asm volatile( + 1: ldxrw0, [%[sptr]]\n + add w0, w0, #0x1\n + stxrw1, w0, [%[sptr]]\n + cbnzw1, 1b\n + : /* out */ + : [sptr] r (shared_value) /* in */ + : w0, w1, cc); +#else + asm volatile( + 1: ldrex r0, [%[sptr]]\n + add r0, r0, #0x1\n + strexeq r1, r0, [%[sptr]]\n + cmpeq r1, #0\n + bne 1b\n + : /* out */ + : [sptr] r (shared_value) /* in */ + : r0, r1, cc); +#endif +} + +static void increment_shared_with_acqrel(void) +{ +#if defined (__LP64__) || defined (_LP64) + asm volatile( + ldarw0, [%[sptr]]\n + add w0, w0, #0x1\n + str w0, [%[sptr]]\n + : /* out */ + : [sptr] r (shared_value) /* in */ + : w0); +#else + /* ARMv7 has no acquire/release semantics but we +* can ensure the results of the write are propagated +* with the use of barriers. +*/ + asm volatile( + 1: ldrex r0, [%[sptr]]\n + add r0, r0, #0x1\n + strexeq r1, r0, [%[sptr]]\n + cmpeq r1, #0\n + bne 1b\n + dmb\n + : /* out */ + : [sptr] r (shared_value) /* in */ + : r0, r1, cc); +#endif + +} + +/* The idea of this is just to generate some random load/store + * activity which may or may not race with an un-barried incremented + * of the shared counter + */ +static void shuffle_memory(int cpu) +{ + int i; + uint32_t lspat = isaac_next_uint32(prng_context[cpu]); + uint32_t seq = isaac_next_uint32(prng_context[cpu]); + int count = seq 0x1f; + uint32_t val=0; + + seq = 5; + + for (i=0; icount; i++) { + int index = seq ~PAGE_MASK; + if (lspat 1) { + val ^= memory_array[index]; + } else { + memory_array[index] = val; + } + seq = PAGE_SHIFT; + seq ^= lspat; + lspat = 1; + } + +} + +static void do_increment(void) +{ + int i; + int cpu = smp_processor_id(); + + printf(CPU%d online\n, cpu); + + for (i=0; i increment_count; i++) { + per_cpu_value[cpu]++; + inc_fn(); + + shuffle_memory(cpu); + } + + printf(CPU%d: Done, %d incs\n, cpu, per_cpu_value[cpu]); + + cpumask_set_cpu(cpu, smp_test_complete); + if (cpu != 0) + halt(); +} + +int main(int argc, char **argv) +{ + int cpu; + unsigned int i, sum = 0; + static const unsigned char seed[] = myseed; + + inc_fn = increment_shared; + + isaac_init(prng_context[0], seed[0], sizeof(seed)); + + for (i=0; iargc; i++) { + char *arg = argv[i]; + + if (strcmp(arg, lock) == 0
[kvm-unit-tests PATCH v5 02/11] README: add some CONTRIBUTING notes
Signed-off-by: Alex Bennée alex.ben...@linaro.org Reviewed-by: Andrew Jones drjo...@redhat.com --- v2 - mention consistency v3 - add r-b tag --- README | 26 ++ 1 file changed, 26 insertions(+) diff --git a/README b/README index e9869d1..9389a26 100644 --- a/README +++ b/README @@ -25,3 +25,29 @@ Directory structure: ./ARCH: the sources of the tests and the created objects/images See ARCH/README for architecture specific documentation. + +CONTRIBUTING: += + +Style +- + +Currently there is a mix of indentation styles so any changes to +existing files should be consistent with the existing style. For new +files: + + - C: please use standard linux-with-tabs + - Shell: use TABs for indentation + +Patches +--- + +Patches are welcome at the KVM mailing list kvm@vger.kernel.org. + +Please prefix messages with: [kvm-unit-tests PATCH] + +You can add the following to .git/config to do this automatically for you: + +[format] + subjectprefix = kvm-unit-tests PATCH + -- 2.5.0 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH v5 05/11] lib/printf: support the %u unsigned fmt field
--- lib/printf.c | 13 + 1 file changed, 13 insertions(+) diff --git a/lib/printf.c b/lib/printf.c index 89308fb..5d83605 100644 --- a/lib/printf.c +++ b/lib/printf.c @@ -180,6 +180,19 @@ int vsnprintf(char *buf, int size, const char *fmt, va_list va) break; } break; + case 'u': + switch (nlong) { + case 0: + print_unsigned(s, va_arg(va, unsigned), 10, props); + break; + case 1: + print_unsigned(s, va_arg(va, unsigned long), 10, props); + break; + default: + print_unsigned(s, va_arg(va, unsigned long long), 10, props); + break; + } + break; case 'x': switch (nlong) { case 0: -- 2.5.0 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH v5 01/11] arm/run: set indentation defaults for emacs
Signed-off-by: Alex Bennée alex.ben...@linaro.org --- arm/run | 1 + 1 file changed, 1 insertion(+) diff --git a/arm/run b/arm/run index 662a856..6b42a2e 100755 --- a/arm/run +++ b/arm/run @@ -1,4 +1,5 @@ #!/bin/bash +# -*- sh-basic-offset:8 indent-tabs-mode: t -*- if [ ! -f config.mak ]; then echo run ./configure first. See ./configure -h -- 2.5.0 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-unit-tests RFC PATCH] Add MAINTAINERS file
Paolo Bonzini pbonz...@redhat.com writes: Applied as follows: +Maintainers +--- +M: Paolo Bonzini pbonz...@redhat.com +M: Marcelo Tosatti mtosa...@redhat.com +L: kvm@vger.kernel.org +T: git://git.kernel.org/pub/scm/virt/kvm/kvm-unit-tests.git + +Architecture Specific Code: +--- + +ARM +M: Drew Jones drjo...@redhat.com +L: kvm@vger.kernel.org +L: kvm...@lists.cs.columbia.edu +F: arm/* +F: lib/arm/* +F: lib/arm64/* + +x86 +M: Paolo Bonzini pbonz...@redhat.com +M: Marcelo Tosatti mtosa...@redhat.com +L: kvm@vger.kernel.org +F: x86/* +F: lib/x86/* I haven't seen this upstream yet. Has this been pushed yet? -- Alex Bennée -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-unit-tests RFC PATCH] Add MAINTAINERS file
Christoffer Dall christoffer.d...@linaro.org writes: On Tue, Jul 28, 2015 at 2:51 PM, Marc Zyngier marc.zyng...@arm.com wrote: On 28/07/15 13:44, Andrew Jones wrote: On Tue, Jul 28, 2015 at 12:41:38PM +0100, Alex Bennée wrote: Paolo Bonzini pbonz...@redhat.com writes: On 28/07/2015 10:56, Alex Bennée wrote: +Architecture Specific Code: +--- + +ARM +M: Christoffer Dall christoffer.d...@linaro.org +L: linux-arm-ker...@lists.infradead.org (moderated for non-subscribers) +L: kvm...@lists.cs.columbia.edu +F: arm/* + Hmm, shouldn't the maintainer be Drew here (or Drew + Christoffer)? I was going by his demur deferral earlier ;-) Also, for arm patches I usually CC Christoffer too... But sure the more the merrier! Yeah, I wasn't running away from the role. I was just stating that it's nice to get Christoffer's input on low-level arm bits. And, particularly as we start adding real kvm tests, it'd be good to get his and/or Marc's input to make sure the tests are complete. Anyway, feel free to add me as a maintainer for kvm-unit-tests/arm. I'm happy to do it. I'm always happy to be CCed on architecture related questions. Just don't advertise me as a maintainer for the unit tests! ;-) I'm with Marc here, don't put me in the maintainers file, I don't have the bandwidth for this at the moment and I'm quite happy seeing Drew taking care of this. I shall add suggested CC's then and leave Drew as maintainer? -Christoffer -- Alex Bennée -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests RFC PATCH] Add MAINTAINERS file
This may be overkill for a project as small as the unit tests now but perhaps it pays to be explicit? Signed-off-by: Alex Bennée alex.ben...@linaro.org --- MAINTAINERS | 67 + 1 file changed, 67 insertions(+) create mode 100644 MAINTAINERS diff --git a/MAINTAINERS b/MAINTAINERS new file mode 100644 index 000..89b0260 --- /dev/null +++ b/MAINTAINERS @@ -0,0 +1,67 @@ +KVM Unit Tests Maintainers +== + +The intention of this file is not to establish who owns what portions of the +code base, but to provide a set of names that developers can consult when they +have a question about a particular subset and also to provide a set of names +to be CC'd when submitting a patch to obtain appropriate review. + +In general, if you have a question about inclusion of a patch, you +should consult the KVM mailing list kvm@vger.kernel.org and not any +specific individual privately. + +Descriptions of section entries: + + M: Mail patches to: FullName address@domain + L: Mailing list that is relevant to this area + W: Web-page with status/info + Q: Patchwork web based patch tracking system site + T: SCM tree type and location. Type is one of: git, hg, quilt, stgit. + S: Status, one of the following: + Supported: Someone is actually paid to look after this. + Maintained: Someone actually looks after it. + Odd Fixes: It has a maintainer but they don't have time to do + much other than throw the odd patch in. See below. + Orphan: No current maintainer [but maybe you could take the + role as you write your new code]. + Obsolete:Old code. Something tagged obsolete generally means + it has been replaced by a better system and you + should be using that. + F: Files and directories with wildcard patterns. + A trailing slash includes all files and subdirectory files. + F: drivers/net/all files in and below drivers/net + F: drivers/net/* all files in drivers/net, but not below + F: */net/* all files in any top level directory/net + One pattern per line. Multiple F: lines acceptable. + X: Files and directories that are NOT maintained, same rules as F: + Files exclusions are tested before file matches. + Can be useful for excluding a specific subdirectory, for instance: + F: net/ + X: net/ipv6/ + matches all files in and below net excluding net/ipv6/ + K: Keyword perl extended regex pattern to match content in a + patch or file. For instance: + K: of_get_profile + matches patches or files that contain of_get_profile + K: \b(printk|pr_(info|err))\b + matches patches or files that contain one or more of the words + printk, pr_info or pr_err + One regex pattern per line. Multiple K: lines acceptable. + + +Maintainers +--- +L: kvm@vger.kernel.org +M: Paolo Bonzini pbonz...@redhat.com +M: Marcelo Tosatti mtosa...@redhat.com +T: git://git.kernel.org/pub/scm/virt/kvm/kvm-unit-tests.git + +Architecture Specific Code: +--- + +ARM +M: Christoffer Dall christoffer.d...@linaro.org +L: linux-arm-ker...@lists.infradead.org (moderated for non-subscribers) +L: kvm...@lists.cs.columbia.edu +F: arm/* + -- 2.4.6 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-unit-tests RFC PATCH] Add MAINTAINERS file
Paolo Bonzini pbonz...@redhat.com writes: On 28/07/2015 10:56, Alex Bennée wrote: +Architecture Specific Code: +--- + +ARM +M: Christoffer Dall christoffer.d...@linaro.org +L: linux-arm-ker...@lists.infradead.org (moderated for non-subscribers) +L: kvm...@lists.cs.columbia.edu +F: arm/* + Hmm, shouldn't the maintainer be Drew here (or Drew + Christoffer)? I was going by his demur deferral earlier ;-) Also, for arm patches I usually CC Christoffer too... But sure the more the merrier! -- Alex Bennée -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-unit-tests PATCH v4 0/3] A couple of small fixes
Alex Bennée alex.ben...@linaro.org writes: One minor fix for quote usage and added a final r-b tag from Drew. Please apply Ping. Do I need to get anyone in particular to commit this. I notice we don't list any particular maintainers with commit rights. Alex Bennée (3): README: add some CONTRIBUTING notes configure: emit HOST=$host to config.mak arm/run: introduce usingkvm var and use it README| 26 ++ arm/run | 18 -- configure | 2 ++ 3 files changed, 44 insertions(+), 2 deletions(-) -- Alex Bennée -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-unit-tests RFC PATCH] arm/tlbflush.c: TLB flushing torture test [DEV]
Andrew Jones drjo...@redhat.com writes: On Fri, Jul 24, 2015 at 02:25:06PM +0100, Alex Bennée wrote: This adds a fairly brain dead torture test for TLB flushes intended for stressing the MTTCG QEMU build. It takes the usual -smp option for multiple CPUs. By default it will do a TLBIALL flush after each cycle. If you pass -append page to the kernel it will take it in turns to flush each of the computation functions. At the moment it doesn't do any re-mapping of pages but maybe that is something that could be done in the future. [DEV VERSION FOR COMMENT] Signed-off-by: Alex Bennée alex.ben...@linaro.org --- arm/tlbflush.c | 163 +++ config/config-arm-common.mak | 4 +- lib/arm/asm/mmu.h| 11 +++ 3 files changed, 177 insertions(+), 1 deletion(-) create mode 100644 arm/tlbflush.c diff --git a/arm/tlbflush.c b/arm/tlbflush.c new file mode 100644 index 000..6eeff18 --- /dev/null +++ b/arm/tlbflush.c @@ -0,0 +1,163 @@ +#include libcflat.h +#include asm/smp.h +#include asm/cpumask.h +#include asm/barrier.h +#include asm/mmu.h + +#define SEQ_LENGTH 10 + +static cpumask_t smp_test_complete; +static int flush_count = 10; +static int flush_self = 1; +static int flush_page = 0; + +__attribute__((aligned(0x1000))) unsigned int hash_array(int length, unsigned int *array) You should use PAGE_SIZE instead of 0x1000 in these attributes, allowing the test to also work for aarch64, as we're using 64k pages on aarch64. Good point. +{ +int i; +unsigned int sum=0; +for (i=0; ilength; i++) +{ +unsigned int val = *array++; +sum ^= val; +sum ^= (val (val % 16)); +sum ^= (val (val % 32)); +} + +return sum; +} + +__attribute__((aligned(0x1000))) void create_fib_sequence(int length, unsigned int *array) +{ +int i; + +/* first two values */ +array[0] = 0; +array[1] = 1; +for (i=2; ilength; i++) +{ +array[i] = array[i-2] + array[i-1]; +} +} + +__attribute__((aligned(0x1000))) unsigned long long factorial(unsigned int n) +{ +unsigned int i; +unsigned long long fac = 1; +for (i=1; i=n; i++) +{ +fac = fac * i; +} +return fac; +} + +/* do some computationally expensive stuff, return a checksum of the + * results */ +__attribute__((aligned(0x1000))) unsigned int do_computation(void) +{ +unsigned int fib_array[SEQ_LENGTH]; +unsigned long long facfib_array[SEQ_LENGTH]; +unsigned int fib_hash, facfib_hash; +int cpu = smp_processor_id(); +int i, j; + +create_fib_sequence(SEQ_LENGTH, fib_array[0]); +fib_hash = hash_array(SEQ_LENGTH, fib_array[0]); +for (i=0; iSEQ_LENGTH; i++) { +for (j=0; jfib_array[i]; j++) { +facfib_array[i] = factorial(fib_array[i]+j); +} +} +facfib_hash = 0; +for (i=0; iSEQ_LENGTH; i++) { +for (j=0; jfib_array[i]; j++) { +facfib_hash ^= hash_array(sizeof(facfib_array)/sizeof(unsigned int), (unsigned int *)facfib_array[0]); +} +} + +#if 0 +printf(CPU:%d FIBSEQ , cpu); +for (i=0; iSEQ_LENGTH; i++) +printf(%u,, fib_array[i]); +printf(\n); + +printf(CPU:%d FACFIB , cpu); +for (i=0; iSEQ_LENGTH; i++) +printf(%llu,, facfib_array[i]); +printf(\n); +#endif + +return (fib_hash ^ facfib_hash); +} + +static void * pages[] = {hash_array, create_fib_sequence, factorial, do_computation}; I can't comment on whether or not the complexity of do_computation is necessary for your test, but it seems like overkill. Comments explaining why it's necessary would be good. OK. From QEMUs TCG point of view I just want to ensure I have more than two basic blocks per-page region so I can check the block-chaining in-page and jump caching intra-page which are both affected on flushes. A computationally complex routine with a known answer would be nicer though I guess. + +static void test_flush(void) +{ +int i, errors = 0; +int cpu = smp_processor_id(); + +unsigned int ref; + +printf(CPU%d online\n, cpu); + +ref = do_computation(); What makes you sure that the first time you do the computation per cpu is correct? I think computing it externally, and saving the result, i.e. #define EXPECTED_RESULT 0x12345678 would be more reliable. OK. + +for (i=0; i flush_count; i++) { +unsigned int this_ref = do_computation(); + +if (this_ref != ref) { +errors++; +printf(CPU%d: seq%d 0x%x!=0x%x\n, +cpu, i, ref, this_ref); +} + +if ((i % 1000) == 0) { +printf(CPU%d: seq%d\n, cpu, i
[kvm-unit-tests RFC PATCH] arm/tlbflush.c: TLB flushing torture test [DEV]
This adds a fairly brain dead torture test for TLB flushes intended for stressing the MTTCG QEMU build. It takes the usual -smp option for multiple CPUs. By default it will do a TLBIALL flush after each cycle. If you pass -append page to the kernel it will take it in turns to flush each of the computation functions. At the moment it doesn't do any re-mapping of pages but maybe that is something that could be done in the future. [DEV VERSION FOR COMMENT] Signed-off-by: Alex Bennée alex.ben...@linaro.org --- arm/tlbflush.c | 163 +++ config/config-arm-common.mak | 4 +- lib/arm/asm/mmu.h| 11 +++ 3 files changed, 177 insertions(+), 1 deletion(-) create mode 100644 arm/tlbflush.c diff --git a/arm/tlbflush.c b/arm/tlbflush.c new file mode 100644 index 000..6eeff18 --- /dev/null +++ b/arm/tlbflush.c @@ -0,0 +1,163 @@ +#include libcflat.h +#include asm/smp.h +#include asm/cpumask.h +#include asm/barrier.h +#include asm/mmu.h + +#define SEQ_LENGTH 10 + +static cpumask_t smp_test_complete; +static int flush_count = 10; +static int flush_self = 1; +static int flush_page = 0; + +__attribute__((aligned(0x1000))) unsigned int hash_array(int length, unsigned int *array) +{ + int i; + unsigned int sum=0; + for (i=0; ilength; i++) + { + unsigned int val = *array++; + sum ^= val; + sum ^= (val (val % 16)); + sum ^= (val (val % 32)); + } + + return sum; +} + +__attribute__((aligned(0x1000))) void create_fib_sequence(int length, unsigned int *array) +{ + int i; + + /* first two values */ + array[0] = 0; + array[1] = 1; + for (i=2; ilength; i++) + { + array[i] = array[i-2] + array[i-1]; + } +} + +__attribute__((aligned(0x1000))) unsigned long long factorial(unsigned int n) +{ + unsigned int i; + unsigned long long fac = 1; + for (i=1; i=n; i++) + { + fac = fac * i; + } + return fac; +} + +/* do some computationally expensive stuff, return a checksum of the + * results */ +__attribute__((aligned(0x1000))) unsigned int do_computation(void) +{ + unsigned int fib_array[SEQ_LENGTH]; + unsigned long long facfib_array[SEQ_LENGTH]; + unsigned int fib_hash, facfib_hash; + int cpu = smp_processor_id(); + int i, j; + + create_fib_sequence(SEQ_LENGTH, fib_array[0]); + fib_hash = hash_array(SEQ_LENGTH, fib_array[0]); + for (i=0; iSEQ_LENGTH; i++) { + for (j=0; jfib_array[i]; j++) { + facfib_array[i] = factorial(fib_array[i]+j); + } + } + facfib_hash = 0; + for (i=0; iSEQ_LENGTH; i++) { + for (j=0; jfib_array[i]; j++) { + facfib_hash ^= hash_array(sizeof(facfib_array)/sizeof(unsigned int), (unsigned int *)facfib_array[0]); + } + } + +#if 0 + printf(CPU:%d FIBSEQ , cpu); + for (i=0; iSEQ_LENGTH; i++) + printf(%u,, fib_array[i]); + printf(\n); + + printf(CPU:%d FACFIB , cpu); + for (i=0; iSEQ_LENGTH; i++) + printf(%llu,, facfib_array[i]); + printf(\n); +#endif + + return (fib_hash ^ facfib_hash); +} + +static void * pages[] = {hash_array, create_fib_sequence, factorial, do_computation}; + +static void test_flush(void) +{ + int i, errors = 0; + int cpu = smp_processor_id(); + + unsigned int ref; + + printf(CPU%d online\n, cpu); + + ref = do_computation(); + + for (i=0; i flush_count; i++) { + unsigned int this_ref = do_computation(); + + if (this_ref != ref) { + errors++; + printf(CPU%d: seq%d 0x%x!=0x%x\n, + cpu, i, ref, this_ref); + } + + if ((i % 1000) == 0) { + printf(CPU%d: seq%d\n, cpu, i); + } + + if (flush_self) { + if (flush_page) { + int j = (i % (sizeof(pages)/sizeof(void *))); + flush_tlb_page((unsigned long)pages[j]); + } else { + flush_tlb_all(); + } + } + } + + report(CPU%d: Done - Errors: %d\n, errors == 0, cpu, errors); + + cpumask_set_cpu(cpu, smp_test_complete); + if (cpu != 0) + halt(); +} + +int main(int argc, char **argv) +{ + int cpu, i; + + report_prefix_push(tlbflush); + + for (i=0; iargc; i++) { + char *arg = argv[i]; +/* printf(arg:%d:%s\n, i, arg); */ + + if (strcmp(arg, page) == 0) { + report_prefix_push(page); + flush_page = 1
[kvm-unit-tests PATCH v4 1/3] README: add some CONTRIBUTING notes
Signed-off-by: Alex Bennée alex.ben...@linaro.org Reviewed-by: Andrew Jones drjo...@redhat.com --- v2 - mention consistency v3 - add r-b tag --- README | 26 ++ 1 file changed, 26 insertions(+) diff --git a/README b/README index e9869d1..9389a26 100644 --- a/README +++ b/README @@ -25,3 +25,29 @@ Directory structure: ./ARCH: the sources of the tests and the created objects/images See ARCH/README for architecture specific documentation. + +CONTRIBUTING: += + +Style +- + +Currently there is a mix of indentation styles so any changes to +existing files should be consistent with the existing style. For new +files: + + - C: please use standard linux-with-tabs + - Shell: use TABs for indentation + +Patches +--- + +Patches are welcome at the KVM mailing list kvm@vger.kernel.org. + +Please prefix messages with: [kvm-unit-tests PATCH] + +You can add the following to .git/config to do this automatically for you: + +[format] + subjectprefix = kvm-unit-tests PATCH + -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH v4 0/3] A couple of small fixes
One minor fix for quote usage and added a final r-b tag from Drew. Please apply Alex Bennée (3): README: add some CONTRIBUTING notes configure: emit HOST=$host to config.mak arm/run: introduce usingkvm var and use it README| 26 ++ arm/run | 18 -- configure | 2 ++ 3 files changed, 44 insertions(+), 2 deletions(-) -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH v4 3/3] arm/run: introduce usingkvm var and use it
This makes the script a little cleaner by only checking for KVM support in one place. If KVM isn't available we can fall back to TCG emulation and echo the fact to the screen rather than let QEMU complain. Signed-off-by: Alex Bennée alex.ben...@linaro.org Reviewed-by: Andrew Jones drjo...@redhat.com --- v2 - rm redundant M= statement v3 - make usingkvm use yes - merge patches 3/4 into one v4 - use single quotes consistently - add r-b tag --- arm/run | 18 -- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/arm/run b/arm/run index 6b42a2e..6b3d558 100755 --- a/arm/run +++ b/arm/run @@ -8,6 +8,15 @@ fi source config.mak processor=$PROCESSOR +# Default to using KVM if available and on the right ARM host +if [ -c /dev/kvm ]; then + if [ $HOST = arm ] [ $ARCH = arm ]; then + usingkvm=yes + elif [ $HOST = aarch64 ]; then + usingkvm=yes + fi +fi + qemu=${QEMU:-qemu-system-$ARCH_NAME} qpath=$(which $qemu 2/dev/null) @@ -22,6 +31,12 @@ if ! $qemu -machine '?' 21 | grep 'ARM Virtual Machine' /dev/null; then fi M='-machine virt' +if [ $usingkvm = yes ]; then + M+=',accel=kvm' +else + echo Running with TCG + M+=',accel=tcg' +fi if ! $qemu $M -device '?' 21 | grep virtconsole /dev/null; then echo $qpath doesn't support virtio-console for chr-testdev. Exiting. @@ -34,12 +49,11 @@ if $qemu $M -chardev testdev,id=id -initrd . 21 \ exit 2 fi -M='-machine virt,accel=kvm:tcg' chr_testdev='-device virtio-serial-device' chr_testdev+=' -device virtconsole,chardev=ctd -chardev testdev,id=ctd' # arm64 must use '-cpu host' with kvm -if [ $(arch) = aarch64 ] [ $ARCH = arm64 ] [ -c /dev/kvm ]; then +if [ $usingkvm = yes ] [ $ARCH = arm64 ]; then processor=host fi -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH v4 2/3] configure: emit HOST=$host to config.mak
This is useful information for the run scripts to know, especially if they want to drop to using TCG. Signed-off-by: Alex Bennée alex.ben...@linaro.org Reviewed-by: Andrew Jones drjo...@redhat.com --- v3 - add r-b tag --- configure | 2 ++ 1 file changed, 2 insertions(+) diff --git a/configure b/configure index b2ad32a..078b70c 100755 --- a/configure +++ b/configure @@ -7,6 +7,7 @@ ld=ld objcopy=objcopy ar=ar arch=`uname -m | sed -e s/i.86/i386/ | sed -e 's/arm.*/arm/'` +host=$arch cross_prefix= usage() { @@ -122,6 +123,7 @@ ln -s $asm lib/asm cat EOF config.mak PREFIX=$prefix KERNELDIR=$(readlink -f $kerneldir) +HOST=$host ARCH=$arch ARCH_NAME=$arch_name PROCESSOR=$processor -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH v3 0/3] A couple of small fixes
A few more tweaks following review comments. I've added the r-b tags where given and squashed patches 3/4 together. Alex Bennée (3): README: add some CONTRIBUTING notes configure: emit HOST=$host to config.mak arm/run: introduce usingkvm var and use it README| 26 ++ arm/run | 18 -- configure | 2 ++ 3 files changed, 44 insertions(+), 2 deletions(-) -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH v3 3/3] arm/run: introduce usingkvm var and use it
This makes the script a little cleaner by only checking for KVM support in one place. If KVM isn't available we can fall back to TCG emulation and echo the fact to the screen rather than let QEMU complain. Signed-off-by: Alex Bennée alex.ben...@linaro.org --- v2 - rm redundant M= statement v3 - make usingkvm use yes - merge patches 3/4 into one --- arm/run | 18 -- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/arm/run b/arm/run index 6b42a2e..cf6e902 100755 --- a/arm/run +++ b/arm/run @@ -8,6 +8,15 @@ fi source config.mak processor=$PROCESSOR +# Default to using KVM if available and on the right ARM host +if [ -c /dev/kvm ]; then + if [ $HOST = arm ] [ $ARCH = arm ]; then + usingkvm=yes + elif [ $HOST = aarch64 ]; then + usingkvm=yes + fi +fi + qemu=${QEMU:-qemu-system-$ARCH_NAME} qpath=$(which $qemu 2/dev/null) @@ -22,6 +31,12 @@ if ! $qemu -machine '?' 21 | grep 'ARM Virtual Machine' /dev/null; then fi M='-machine virt' +if [ $usingkvm = yes ]; then + M+=,accel=kvm +else + echo Running with TCG + M+=',accel=tcg' +fi if ! $qemu $M -device '?' 21 | grep virtconsole /dev/null; then echo $qpath doesn't support virtio-console for chr-testdev. Exiting. @@ -34,12 +49,11 @@ if $qemu $M -chardev testdev,id=id -initrd . 21 \ exit 2 fi -M='-machine virt,accel=kvm:tcg' chr_testdev='-device virtio-serial-device' chr_testdev+=' -device virtconsole,chardev=ctd -chardev testdev,id=ctd' # arm64 must use '-cpu host' with kvm -if [ $(arch) = aarch64 ] [ $ARCH = arm64 ] [ -c /dev/kvm ]; then +if [ $usingkvm = yes ] [ $ARCH = arm64 ]; then processor=host fi -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH v3 2/3] configure: emit HOST=$host to config.mak
This is useful information for the run scripts to know, especially if they want to drop to using TCG. Signed-off-by: Alex Bennée alex.ben...@linaro.org Reviewed-by: Andrew Jones drjo...@redhat.com --- v3 - add r-b tag --- configure | 2 ++ 1 file changed, 2 insertions(+) diff --git a/configure b/configure index b2ad32a..078b70c 100755 --- a/configure +++ b/configure @@ -7,6 +7,7 @@ ld=ld objcopy=objcopy ar=ar arch=`uname -m | sed -e s/i.86/i386/ | sed -e 's/arm.*/arm/'` +host=$arch cross_prefix= usage() { @@ -122,6 +123,7 @@ ln -s $asm lib/asm cat EOF config.mak PREFIX=$prefix KERNELDIR=$(readlink -f $kerneldir) +HOST=$host ARCH=$arch ARCH_NAME=$arch_name PROCESSOR=$processor -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH v3 1/3] README: add some CONTRIBUTING notes
Signed-off-by: Alex Bennée alex.ben...@linaro.org Reviewed-by: Andrew Jones drjo...@redhat.com --- v2 - mention consistency v3 - add r-b tag --- README | 26 ++ 1 file changed, 26 insertions(+) diff --git a/README b/README index e9869d1..9389a26 100644 --- a/README +++ b/README @@ -25,3 +25,29 @@ Directory structure: ./ARCH: the sources of the tests and the created objects/images See ARCH/README for architecture specific documentation. + +CONTRIBUTING: += + +Style +- + +Currently there is a mix of indentation styles so any changes to +existing files should be consistent with the existing style. For new +files: + + - C: please use standard linux-with-tabs + - Shell: use TABs for indentation + +Patches +--- + +Patches are welcome at the KVM mailing list kvm@vger.kernel.org. + +Please prefix messages with: [kvm-unit-tests PATCH] + +You can add the following to .git/config to do this automatically for you: + +[format] + subjectprefix = kvm-unit-tests PATCH + -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v8 03/11] KVM: arm: guest debug, add stub KVM_SET_GUEST_DEBUG ioctl
This commit adds a stub function to support the KVM_SET_GUEST_DEBUG ioctl. Any unsupported flag will return -EINVAL. For now, only KVM_GUESTDBG_ENABLE is supported, although it won't have any effects. Signed-off-by: Alex Bennée alex.ben...@linaro.org. Reviewed-by: Christoffer Dall christoffer.d...@linaro.org --- v2 - simplified form of the ioctl (stuff will go into setup_debug) v3 - KVM_GUESTDBG_VALID-KVM_GUESTDBG_VALID_MASK - move mask check to the top of function - add ioctl doc header - split capability into separate patch - tweaked commit wording w.r.t return of -EINVAL v4 - add r-b-tag v7 - moved ioctl to arm64/kvm/guest.c, stubbed arm/kvm/guest.c --- Documentation/virtual/kvm/api.txt | 2 +- arch/arm/kvm/arm.c| 7 --- arch/arm/kvm/guest.c | 6 ++ arch/arm64/kvm/guest.c| 27 +++ 4 files changed, 34 insertions(+), 8 deletions(-) diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt index c34c32d..ba635c7 100644 --- a/Documentation/virtual/kvm/api.txt +++ b/Documentation/virtual/kvm/api.txt @@ -2645,7 +2645,7 @@ handled. 4.87 KVM_SET_GUEST_DEBUG Capability: KVM_CAP_SET_GUEST_DEBUG -Architectures: x86, s390, ppc +Architectures: x86, s390, ppc, arm64 Type: vcpu ioctl Parameters: struct kvm_guest_debug (in) Returns: 0 on success; -1 on error diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c index d9631ec..92b80bc 100644 --- a/arch/arm/kvm/arm.c +++ b/arch/arm/kvm/arm.c @@ -302,13 +302,6 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu) kvm_arm_set_running_vcpu(NULL); } -int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu, - struct kvm_guest_debug *dbg) -{ - return -EINVAL; -} - - int kvm_arch_vcpu_ioctl_get_mpstate(struct kvm_vcpu *vcpu, struct kvm_mp_state *mp_state) { diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c index d503fbb..96e935b 100644 --- a/arch/arm/kvm/guest.c +++ b/arch/arm/kvm/guest.c @@ -290,3 +290,9 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu, { return -EINVAL; } + +int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu, + struct kvm_guest_debug *dbg) +{ + return -EINVAL; +} diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c index 9535bd5..0ba8677 100644 --- a/arch/arm64/kvm/guest.c +++ b/arch/arm64/kvm/guest.c @@ -331,3 +331,30 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu, { return -EINVAL; } + +#define KVM_GUESTDBG_VALID_MASK (KVM_GUESTDBG_ENABLE) + +/** + * kvm_arch_vcpu_ioctl_set_guest_debug - set up guest debugging + * @kvm: pointer to the KVM struct + * @kvm_guest_debug: the ioctl data buffer + * + * This sets up and enables the VM for guest debugging. Userspace + * passes in a control flag to enable different debug types and + * potentially other architecture specific information in the rest of + * the structure. + */ +int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu, + struct kvm_guest_debug *dbg) +{ + if (dbg-control ~KVM_GUESTDBG_VALID_MASK) + return -EINVAL; + + if (dbg-control KVM_GUESTDBG_ENABLE) { + vcpu-guest_debug = dbg-control; + } else { + /* If not enabled clear all flags */ + vcpu-guest_debug = 0; + } + return 0; +} -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v8 02/11] KVM: arm64: guest debug, define API headers
This commit defines the API headers for guest debugging. There are two architecture specific debug structures: - kvm_guest_debug_arch, allows us to pass in HW debug registers - kvm_debug_exit_arch, signals exception and possible faulting address The type of debugging being used is controlled by the architecture specific control bits of the kvm_guest_debug-control flags in the ioctl structure. Signed-off-by: Alex Bennée alex.ben...@linaro.org Reviewed-by: David Hildenbrand d...@linux.vnet.ibm.com Reviewed-by: Andrew Jones drjo...@redhat.com Acked-by: Christoffer Dall christoffer.d...@linaro.org --- v2 - expose hsr and pc directly to user-space v3 - s/control/controlled/ in commit message - add v8 to ARM ARM comment (ARM Architecture Reference Manual) - add rb tag - rm pc, add far - re-word comments on alignment - rename KVM_ARM_NDBG_REGS - KVM_ARM_MAX_DBG_REGS v4 - now uses common HW/SW BP define - add a-b-tag - use u32 for control regs v5 - revert to have arch specific KVM_GUESTDBG_USE_SW/HW_BP - rm stale comments dbgctrl was stored as u64 v6 - mv far comment from later patch - KVM_GUESTDBG_USE_HW_BP - KVM_GUESTDBG_USE_HW - revert control regs to u64 (parity with GET/SET_ONE_REG) --- arch/arm64/include/uapi/asm/kvm.h | 27 +++ 1 file changed, 27 insertions(+) diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h index d268320..d82f3f3 100644 --- a/arch/arm64/include/uapi/asm/kvm.h +++ b/arch/arm64/include/uapi/asm/kvm.h @@ -100,12 +100,39 @@ struct kvm_sregs { struct kvm_fpu { }; +/* + * See v8 ARM ARM D7.3: Debug Registers + * + * The architectural limit is 16 debug registers of each type although + * in practice there are usually less (see ID_AA64DFR0_EL1). + * + * Although the control registers are architecturally defined as 32 + * bits wide we use a 64 bit structure here to keep parity with + * KVM_GET/SET_ONE_REG behaviour which treats all system registers as + * 64 bit values. It also allows for the possibility of the + * architecture expanding the control registers without having to + * change the userspace ABI. + */ +#define KVM_ARM_MAX_DBG_REGS 16 struct kvm_guest_debug_arch { + __u64 dbg_bcr[KVM_ARM_MAX_DBG_REGS]; + __u64 dbg_bvr[KVM_ARM_MAX_DBG_REGS]; + __u64 dbg_wcr[KVM_ARM_MAX_DBG_REGS]; + __u64 dbg_wvr[KVM_ARM_MAX_DBG_REGS]; }; struct kvm_debug_exit_arch { + __u32 hsr; + __u64 far; /* used for watchpoints */ }; +/* + * Architecture specific defines for kvm_guest_debug-control + */ + +#define KVM_GUESTDBG_USE_SW_BP (1 16) +#define KVM_GUESTDBG_USE_HW(1 17) + struct kvm_sync_regs { }; -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v8 08/11] KVM: arm64: introduce vcpu-arch.debug_ptr
This introduces a level of indirection for the debug registers. Instead of using the sys_regs[] directly we store registers in a structure in the vcpu. The new kvm_arm_reset_debug_ptr() sets the debug ptr to the guest context. Because we no longer give the sys_regs offset for the sys_reg_desc-reg field, but instead the index into a debug-specific struct we need to add a number of additional trap functions for each register. Also as the generic generic user-space access code no longer works we have introduced a new pair of function pointers to the sys_reg_desc structure to override the generic code when needed. Signed-off-by: Alex Bennée alex.ben...@linaro.org --- v6: - fix up some ws issues - correct clobber info - re-word commentary in kvm_host.h - fix endian access issues for aarch32 fields - revert all KVM_GET/SET_ONE_REG to 64bit (also see ABI update) v7 - new fn kvm_arm_reset_debug_ptr(), stubbed for arm - split trap fns into bcr,bvr,bcr,wvr and wxvr - add set/get fns to sys_regs_desc - reg_to_dbg/dbg_to_reg helpers for 32bit support v8 - rm #if 0 left over from re-factor - re-word reset_ptr comment - fix inadvertent whitespace changes - rename get/set to get_user/set_user - update commit wording - re-factor dbg_to_reg/reg_to_dbg - add newlines to keep checkpatch happy - ensure 63:32 of debug registers untouched by aarch32 --- arch/arm/include/asm/kvm_host.h | 1 + arch/arm/kvm/arm.c| 2 + arch/arm64/include/asm/kvm_asm.h | 24 ++-- arch/arm64/include/asm/kvm_host.h | 17 ++- arch/arm64/kernel/asm-offsets.c | 6 + arch/arm64/kvm/debug.c| 9 ++ arch/arm64/kvm/hyp.S | 24 ++-- arch/arm64/kvm/sys_regs.c | 274 +++--- arch/arm64/kvm/sys_regs.h | 6 + 9 files changed, 316 insertions(+), 47 deletions(-) diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h index 746c0c69..825c337 100644 --- a/arch/arm/include/asm/kvm_host.h +++ b/arch/arm/include/asm/kvm_host.h @@ -239,5 +239,6 @@ static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {} static inline void kvm_arm_init_debug(void) {} static inline void kvm_arm_setup_debug(struct kvm_vcpu *vcpu) {} static inline void kvm_arm_clear_debug(struct kvm_vcpu *vcpu) {} +static inline void kvm_arm_reset_debug_ptr(struct kvm_vcpu *vcpu) {} #endif /* __ARM_KVM_HOST_H__ */ diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c index af60e6f..525473f 100644 --- a/arch/arm/kvm/arm.c +++ b/arch/arm/kvm/arm.c @@ -279,6 +279,8 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu) /* Set up the timer */ kvm_timer_vcpu_init(vcpu); + kvm_arm_reset_debug_ptr(vcpu); + return 0; } diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h index d6b507e..e997404 100644 --- a/arch/arm64/include/asm/kvm_asm.h +++ b/arch/arm64/include/asm/kvm_asm.h @@ -46,24 +46,16 @@ #defineCNTKCTL_EL1 20 /* Timer Control Register (EL1) */ #definePAR_EL1 21 /* Physical Address Register */ #define MDSCR_EL1 22 /* Monitor Debug System Control Register */ -#define DBGBCR0_EL123 /* Debug Breakpoint Control Registers (0-15) */ -#define DBGBCR15_EL1 38 -#define DBGBVR0_EL139 /* Debug Breakpoint Value Registers (0-15) */ -#define DBGBVR15_EL1 54 -#define DBGWCR0_EL155 /* Debug Watchpoint Control Registers (0-15) */ -#define DBGWCR15_EL1 70 -#define DBGWVR0_EL171 /* Debug Watchpoint Value Registers (0-15) */ -#define DBGWVR15_EL1 86 -#define MDCCINT_EL187 /* Monitor Debug Comms Channel Interrupt Enable Reg */ +#define MDCCINT_EL123 /* Monitor Debug Comms Channel Interrupt Enable Reg */ /* 32bit specific registers. Keep them at the end of the range */ -#defineDACR32_EL2 88 /* Domain Access Control Register */ -#defineIFSR32_EL2 89 /* Instruction Fault Status Register */ -#defineFPEXC32_EL2 90 /* Floating-Point Exception Control Register */ -#defineDBGVCR32_EL291 /* Debug Vector Catch Register */ -#defineTEECR32_EL1 92 /* ThumbEE Configuration Register */ -#defineTEEHBR32_EL193 /* ThumbEE Handler Base Register */ -#defineNR_SYS_REGS 94 +#defineDACR32_EL2 24 /* Domain Access Control Register */ +#defineIFSR32_EL2 25 /* Instruction Fault Status Register */ +#defineFPEXC32_EL2 26 /* Floating-Point Exception Control Register */ +#defineDBGVCR32_EL227 /* Debug Vector Catch Register */ +#defineTEECR32_EL1 28 /* ThumbEE Configuration Register */ +#defineTEEHBR32_EL129 /* ThumbEE Handler Base Register */ +#defineNR_SYS_REGS 30 /* 32bit mapping */ #define c0_MPIDR (MPIDR_EL1 * 2) /* MultiProcessor ID Register */ diff --git
[PATCH v8 00/11] KVM Guest Debug support for arm64
Here is V8 of the KVM Guest Debug support for arm64. The diffstat between v7 and v8 is getting pretty small and as I haven't re-based you can run: git diff -u guest-debug/4.1-v7..guest-debug/4.1-v8 And the kernelci report is at: http://kernelci.org/build/alex/kernel/v4.1-11-g182a5fa64600/ The only real changes apart from comments and white space are to sys_regs which sees another minor re-factoring. The 32 bit handling explicitly preserves the top 32 bits of the AArch64 registers although I'm not convinced it matter too much as for a booting AArch32 guest kernel they should start as 0 and never change. For full details see the changelog on each of the patches. GIT Repos: The patches for this series are based off v4.1 and can be found at: Kernel: https://git.linaro.org/people/alex.bennee/linux.git branch: guest-debug/4.1-v8 describe: v4.1-11-g182a5faB QEMU: https://github.com/stsquad/qemu branch: kvm/guest-debug-v6*** BLURB HERE *** Alex Bennée (11): KVM: add comments for kvm_debug_exit_arch struct KVM: arm64: guest debug, define API headers KVM: arm: guest debug, add stub KVM_SET_GUEST_DEBUG ioctl KVM: arm: introduce kvm_arm_init/setup/clear_debug KVM: arm64: guest debug, add SW break point support KVM: arm64: guest debug, add support for single-step KVM: arm64: re-factor hyp.S debug register code KVM: arm64: introduce vcpu-arch.debug_ptr KVM: arm64: guest debug, HW assisted debug support KVM: arm64: enable KVM_CAP_SET_GUEST_DEBUG KVM: arm64: add trace points for guest_debug debug Documentation/virtual/kvm/api.txt | 15 +- arch/arm/include/asm/kvm_host.h| 5 + arch/arm/kvm/arm.c | 18 +- arch/arm/kvm/guest.c | 6 + arch/arm64/include/asm/hw_breakpoint.h | 14 + arch/arm64/include/asm/kvm_asm.h | 26 +- arch/arm64/include/asm/kvm_host.h | 37 ++- arch/arm64/include/uapi/asm/kvm.h | 27 ++ arch/arm64/kernel/asm-offsets.c| 7 + arch/arm64/kernel/hw_breakpoint.c | 12 - arch/arm64/kvm/Makefile| 2 +- arch/arm64/kvm/debug.c | 217 + arch/arm64/kvm/guest.c | 40 +++ arch/arm64/kvm/handle_exit.c | 44 +++ arch/arm64/kvm/hyp.S | 544 ++--- arch/arm64/kvm/reset.c | 16 + arch/arm64/kvm/sys_regs.c | 291 -- arch/arm64/kvm/sys_regs.h | 6 + arch/arm64/kvm/trace.h | 123 include/uapi/linux/kvm.h | 5 + 20 files changed, 1001 insertions(+), 454 deletions(-) create mode 100644 arch/arm64/kvm/debug.c -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v8 01/11] KVM: add comments for kvm_debug_exit_arch struct
Bring into line with the comments for the other structures and their KVM_EXIT_* cases. Also update api.txt to reflect use in kvm_run documentation. Signed-off-by: Alex Bennée alex.ben...@linaro.org Reviewed-by: David Hildenbrand d...@linux.vnet.ibm.com Reviewed-by: Andrew Jones drjo...@redhat.com Acked-by: Christoffer Dall christoffer.d...@linaro.org --- v2 - add comments for other exit types v3 - s/commentary/comments/ - add rb tags - update api.txt kvm_run to include KVM_EXIT_DEBUG desc v4 - sp fixes - add a-b --- Documentation/virtual/kvm/api.txt | 4 +++- include/uapi/linux/kvm.h | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt index 9fa2bf8..c34c32d 100644 --- a/Documentation/virtual/kvm/api.txt +++ b/Documentation/virtual/kvm/api.txt @@ -3070,11 +3070,13 @@ data_offset describes where the data is located (KVM_EXIT_IO_OUT) or where kvm expects application code to place the data for the next KVM_RUN invocation (KVM_EXIT_IO_IN). Data format is a packed array. + /* KVM_EXIT_DEBUG */ struct { struct kvm_debug_exit_arch arch; } debug; -Unused. +If the exit_reason is KVM_EXIT_DEBUG, then a vcpu is processing a debug event +for which architecture specific information is returned. /* KVM_EXIT_MMIO */ struct { diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index 4b60056..70ac641 100644 --- a/include/uapi/linux/kvm.h +++ b/include/uapi/linux/kvm.h @@ -237,6 +237,7 @@ struct kvm_run { __u32 count; __u64 data_offset; /* relative to kvm_run start */ } io; + /* KVM_EXIT_DEBUG */ struct { struct kvm_debug_exit_arch arch; } debug; @@ -285,6 +286,7 @@ struct kvm_run { __u32 data; __u8 is_write; } dcr; + /* KVM_EXIT_INTERNAL_ERROR */ struct { __u32 suberror; /* Available with KVM_CAP_INTERNAL_ERROR_DATA: */ @@ -295,6 +297,7 @@ struct kvm_run { struct { __u64 gprs[32]; } osi; + /* KVM_EXIT_PAPR_HCALL */ struct { __u64 nr; __u64 ret; -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v8 05/11] KVM: arm64: guest debug, add SW break point support
This adds support for SW breakpoints inserted by userspace. We do this by trapping all guest software debug exceptions to the hypervisor (MDCR_EL2.TDE). The exit handler sets an exit reason of KVM_EXIT_DEBUG with the kvm_debug_exit_arch structure holding the exception syndrome information. It will be up to userspace to extract the PC (via GET_ONE_REG) and determine if the debug event was for a breakpoint it inserted. If not userspace will need to re-inject the correct exception restart the hypervisor to deliver the debug exception to the guest. Any other guest software debug exception (e.g. single step or HW assisted breakpoints) will cause an error and the VM to be killed. This is addressed by later patches which add support for the other debug types. Signed-off-by: Alex Bennée alex.ben...@linaro.org Reviewed-by: Christoffer Dall christoffer.d...@linaro.org --- v2 - update to use new exit struct - tweak for C setup - do our setup in debug_setup/clear code - fixed up comments v3: - fix spacing in KVM_GUESTDBG_VALID_MASK - fix and clarify wording on kvm_handle_guest_debug - handle error case in kvm_handle_guest_debug - re-word the commit message v4 - rm else leg - add r-b-tag v7 - moved ioctl to guest --- Documentation/virtual/kvm/api.txt | 2 +- arch/arm64/kvm/debug.c| 3 +++ arch/arm64/kvm/guest.c| 2 +- arch/arm64/kvm/handle_exit.c | 36 4 files changed, 41 insertions(+), 2 deletions(-) diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt index ba635c7..33c8143 100644 --- a/Documentation/virtual/kvm/api.txt +++ b/Documentation/virtual/kvm/api.txt @@ -2667,7 +2667,7 @@ when running. Common control bits are: The top 16 bits of the control field are architecture specific control flags which can include the following: - - KVM_GUESTDBG_USE_SW_BP: using software breakpoints [x86] + - KVM_GUESTDBG_USE_SW_BP: using software breakpoints [x86, arm64] - KVM_GUESTDBG_USE_HW_BP: using hardware breakpoints [x86, s390] - KVM_GUESTDBG_INJECT_DB: inject DB type exception [x86] - KVM_GUESTDBG_INJECT_BP: inject BP type exception [x86] diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c index faf0e1f..8d1bfa4 100644 --- a/arch/arm64/kvm/debug.c +++ b/arch/arm64/kvm/debug.c @@ -73,6 +73,9 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu) if (trap_debug) vcpu-arch.mdcr_el2 |= MDCR_EL2_TDA; + /* Trap breakpoints? */ + if (vcpu-guest_debug KVM_GUESTDBG_USE_SW_BP) + vcpu-arch.mdcr_el2 |= MDCR_EL2_TDE; } void kvm_arm_clear_debug(struct kvm_vcpu *vcpu) diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c index 0ba8677..22d22c5 100644 --- a/arch/arm64/kvm/guest.c +++ b/arch/arm64/kvm/guest.c @@ -332,7 +332,7 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu, return -EINVAL; } -#define KVM_GUESTDBG_VALID_MASK (KVM_GUESTDBG_ENABLE) +#define KVM_GUESTDBG_VALID_MASK (KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP) /** * kvm_arch_vcpu_ioctl_set_guest_debug - set up guest debugging diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c index 524fa25..27f38a9 100644 --- a/arch/arm64/kvm/handle_exit.c +++ b/arch/arm64/kvm/handle_exit.c @@ -82,6 +82,40 @@ static int kvm_handle_wfx(struct kvm_vcpu *vcpu, struct kvm_run *run) return 1; } +/** + * kvm_handle_guest_debug - handle a debug exception instruction + * + * @vcpu: the vcpu pointer + * @run: access to the kvm_run structure for results + * + * We route all debug exceptions through the same handler. If both the + * guest and host are using the same debug facilities it will be up to + * userspace to re-inject the correct exception for guest delivery. + * + * @return: 0 (while setting run-exit_reason), -1 for error + */ +static int kvm_handle_guest_debug(struct kvm_vcpu *vcpu, struct kvm_run *run) +{ + u32 hsr = kvm_vcpu_get_hsr(vcpu); + int ret = 0; + + run-exit_reason = KVM_EXIT_DEBUG; + run-debug.arch.hsr = hsr; + + switch (hsr ESR_ELx_EC_SHIFT) { + case ESR_ELx_EC_BKPT32: + case ESR_ELx_EC_BRK64: + break; + default: + kvm_err(%s: un-handled case hsr: %#08x\n, + __func__, (unsigned int) hsr); + ret = -1; + break; + } + + return ret; +} + static exit_handle_fn arm_exit_handlers[] = { [ESR_ELx_EC_WFx]= kvm_handle_wfx, [ESR_ELx_EC_CP15_32]= kvm_handle_cp15_32, @@ -96,6 +130,8 @@ static exit_handle_fn arm_exit_handlers[] = { [ESR_ELx_EC_SYS64] = kvm_handle_sys_reg, [ESR_ELx_EC_IABT_LOW] = kvm_handle_guest_abort, [ESR_ELx_EC_DABT_LOW] = kvm_handle_guest_abort, + [ESR_ELx_EC_BKPT32] = kvm_handle_guest_debug, + [ESR_ELx_EC_BRK64] = kvm_handle_guest_debug, }; static
[PATCH v8 04/11] KVM: arm: introduce kvm_arm_init/setup/clear_debug
This is a precursor for later patches which will need to do more to setup debug state before entering the hyp.S switch code. The existing functionality for setting mdcr_el2 has been moved out of hyp.S and now uses the value kept in vcpu-arch.mdcr_el2. As the assembler used to previously mask and preserve MDCR_EL2.HPMN I've had to add a mechanism to save the value of mdcr_el2 as a per-cpu variable during the initialisation code. The kernel never sets this number so we are assuming the bootcode has set up the correct value here. This also moves the conditional setting of the TDA bit from the hyp code into the C code which is currently used for the lazy debug register context switch code. Signed-off-by: Alex Bennée alex.ben...@linaro.org Reviewed-by: Christoffer Dall christoffer.d...@linaro.org --- v3 - rename fns from arch-arm - preserve MDCR_EL2.HPMN setting - re-word some of the comments - fix some minor grammar nits - merge setting of mdcr_el2 - introduce trap_debug flag - move setup/clear within the irq lock section v4 - fix TDOSA desc - rm un-needed else leg - s/arch/arm/ v6 - add s-o-b tag --- arch/arm/include/asm/kvm_host.h | 4 ++ arch/arm/kvm/arm.c| 9 - arch/arm64/include/asm/kvm_asm.h | 2 + arch/arm64/include/asm/kvm_host.h | 5 +++ arch/arm64/kernel/asm-offsets.c | 1 + arch/arm64/kvm/Makefile | 2 +- arch/arm64/kvm/debug.c| 81 +++ arch/arm64/kvm/hyp.S | 19 - 8 files changed, 110 insertions(+), 13 deletions(-) create mode 100644 arch/arm64/kvm/debug.c diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h index d71607c..746c0c69 100644 --- a/arch/arm/include/asm/kvm_host.h +++ b/arch/arm/include/asm/kvm_host.h @@ -236,4 +236,8 @@ static inline void kvm_arch_sync_events(struct kvm *kvm) {} static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {} static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {} +static inline void kvm_arm_init_debug(void) {} +static inline void kvm_arm_setup_debug(struct kvm_vcpu *vcpu) {} +static inline void kvm_arm_clear_debug(struct kvm_vcpu *vcpu) {} + #endif /* __ARM_KVM_HOST_H__ */ diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c index 92b80bc..af60e6f 100644 --- a/arch/arm/kvm/arm.c +++ b/arch/arm/kvm/arm.c @@ -542,6 +542,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run) continue; } + kvm_arm_setup_debug(vcpu); + /** * Enter the guest */ @@ -554,7 +556,10 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run) vcpu-mode = OUTSIDE_GUEST_MODE; kvm_guest_exit(); trace_kvm_exit(kvm_vcpu_trap_get_class(vcpu), *vcpu_pc(vcpu)); - /* + + kvm_arm_clear_debug(vcpu); + +/* * We may have taken a host interrupt in HYP mode (ie * while executing the guest). This interrupt is still * pending, as we haven't serviced it yet! @@ -902,6 +907,8 @@ static void cpu_init_hyp_mode(void *dummy) vector_ptr = (unsigned long)__kvm_hyp_vector; __cpu_init_hyp_mode(boot_pgd_ptr, pgd_ptr, hyp_stack_ptr, vector_ptr); + + kvm_arm_init_debug(); } static int hyp_init_cpu_notify(struct notifier_block *self, diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h index 4f7310f..d6b507e 100644 --- a/arch/arm64/include/asm/kvm_asm.h +++ b/arch/arm64/include/asm/kvm_asm.h @@ -137,6 +137,8 @@ extern char __restore_vgic_v2_state[]; extern char __save_vgic_v3_state[]; extern char __restore_vgic_v3_state[]; +extern u32 __kvm_get_mdcr_el2(void); + #endif #endif /* __ARM_KVM_ASM_H__ */ diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index f0f58c9..7cb99b5 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -103,6 +103,7 @@ struct kvm_vcpu_arch { /* HYP configuration */ u64 hcr_el2; + u32 mdcr_el2; /* Exception Information */ struct kvm_vcpu_fault_info fault; @@ -250,4 +251,8 @@ static inline void kvm_arch_sync_events(struct kvm *kvm) {} static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {} static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {} +void kvm_arm_init_debug(void); +void kvm_arm_setup_debug(struct kvm_vcpu *vcpu); +void kvm_arm_clear_debug(struct kvm_vcpu *vcpu); + #endif /* __ARM64_KVM_HOST_H__ */ diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c index da675cc..dfb25a2 100644 --- a/arch/arm64/kernel/asm-offsets.c +++ b/arch/arm64/kernel/asm-offsets.c @@ -117,6 +117,7 @@ int main(void) DEFINE(VCPU_HPFAR_EL2
[PATCH v8 06/11] KVM: arm64: guest debug, add support for single-step
This adds support for single-stepping the guest. To do this we need to manipulate the guests PSTATE.SS and MDSCR_EL1.SS bits to trigger stepping. We take care to preserve MDSCR_EL1 and trap access to it to ensure we don't affect the apparent state of the guest. As we have to enable trapping of all software debug exceptions we suppress the ability of the guest to single-step itself. If we didn't we would have to deal with the exception arriving while the guest was in kernelspace when the guest is expecting to single-step userspace. This is something we don't want to unwind in the kernel. Once the host is no longer debugging the guest its ability to single-step userspace is restored. Signed-off-by: Alex Bennée alex.ben...@linaro.org Reviewed-by: Christoffer Dall christoffer.d...@linaro.org --- v2 - Move pstate/mdscr manipulation into C - don't export guest_debug to assembly - add accessor for saved_debug regs - tweak save/restore of mdscr_el1 v3 - don't save PC in debug information struct - rename debug_saved_regs-guest_debug_state - save whole value, only use bits in restore - add save/restore_guest-debug_regs helper functions - simplify commit message for clarity - rm vcpu_debug_saved_reg access fn v4 - added more comments based on suggestions - guest_debug_state-guest_debug_preserved - no point masking restore, we will trap out v5 - more comments - don't bother preserving pstate.ss (guest never sees change) v6 - reword comments on guest SS suppression - simplify comment for save regs, SS explained in detail later on - add r-b-t (code) - expanded commit description v7 - merge fix for ioctl move to guest.c --- arch/arm64/include/asm/kvm_host.h | 11 +++ arch/arm64/kvm/debug.c| 68 --- arch/arm64/kvm/guest.c| 4 ++- arch/arm64/kvm/handle_exit.c | 2 ++ 4 files changed, 80 insertions(+), 5 deletions(-) diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 7cb99b5..e2db6a6 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -123,6 +123,17 @@ struct kvm_vcpu_arch { * here. */ + /* +* Guest registers we preserve during guest debugging. +* +* These shadow registers are updated by the kvm_handle_sys_reg +* trap handler if the guest accesses or updates them while we +* are using guest debug. +*/ + struct { + u32 mdscr_el1; + } guest_debug_preserved; + /* Don't run the guest */ bool pause; diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c index 8d1bfa4..d439eb8 100644 --- a/arch/arm64/kvm/debug.c +++ b/arch/arm64/kvm/debug.c @@ -19,11 +19,39 @@ #include linux/kvm_host.h +#include asm/debug-monitors.h +#include asm/kvm_asm.h #include asm/kvm_arm.h +#include asm/kvm_emulate.h + +/* These are the bits of MDSCR_EL1 we may manipulate */ +#define MDSCR_EL1_DEBUG_MASK (DBG_MDSCR_SS | \ + DBG_MDSCR_KDE | \ + DBG_MDSCR_MDE) static DEFINE_PER_CPU(u32, mdcr_el2); /** + * save/restore_guest_debug_regs + * + * For some debug operations we need to tweak some guest registers. As + * a result we need to save the state of those registers before we + * make those modifications. + * + * Guest access to MDSCR_EL1 is trapped by the hypervisor and handled + * after we have restored the preserved value to the main context. + */ +static void save_guest_debug_regs(struct kvm_vcpu *vcpu) +{ + vcpu-arch.guest_debug_preserved.mdscr_el1 = vcpu_sys_reg(vcpu, MDSCR_EL1); +} + +static void restore_guest_debug_regs(struct kvm_vcpu *vcpu) +{ + vcpu_sys_reg(vcpu, MDSCR_EL1) = vcpu-arch.guest_debug_preserved.mdscr_el1; +} + +/** * kvm_arm_init_debug - grab what we need for debug * * Currently the sole task of this function is to retrieve the initial @@ -38,7 +66,6 @@ void kvm_arm_init_debug(void) __this_cpu_write(mdcr_el2, kvm_call_hyp(__kvm_get_mdcr_el2)); } - /** * kvm_arm_setup_debug - set up debug related stuff * @@ -73,12 +100,45 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu) if (trap_debug) vcpu-arch.mdcr_el2 |= MDCR_EL2_TDA; - /* Trap breakpoints? */ - if (vcpu-guest_debug KVM_GUESTDBG_USE_SW_BP) + /* Is Guest debugging in effect? */ + if (vcpu-guest_debug) { + /* Route all software debug exceptions to EL2 */ vcpu-arch.mdcr_el2 |= MDCR_EL2_TDE; + + /* Save guest debug state */ + save_guest_debug_regs(vcpu); + + /* +* Single Step (ARM ARM D2.12.3 The software step state +* machine) +* +* If we are doing Single Step we need to manipulate +* the guest's MDSCR_EL1.SS and PSTATE.SS. Once the +* step has
Re: [kvm-unit-tests PATCH 7/7] arm/run: add --debug option
Andrew Jones drjo...@redhat.com writes: On Fri, Jul 03, 2015 at 02:48:47PM +0100, Alex Bennée wrote: This allows you to pass debug options through to the QEMU command line. e.g.: ./arm/run --debug -name debug-threads=on -- arm/vos-spinlock-test.flat -smp 4 doesn't ./arm/run arm/vos-spinlock-test.flat -smp 4 -name debug-threads=on do the same thing? You are right of course, I forgot you needed an explict -append for the args to the test anyway. Signed-off-by: Alex Bennée alex.ben...@linaro.org --- arm/run | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/arm/run b/arm/run index 43d7508..871e35e 100755 --- a/arm/run +++ b/arm/run @@ -18,10 +18,15 @@ if [ -c /dev/kvm ]; then fi fi +debugopts= while :; do case $1 in --force-tcg) usingkvm=0 +;; +--debug) +shift +debugopts=$1 shift ;; --) @@ -78,7 +83,7 @@ if [ $usingkvm = 1 ] [ $ARCH = arm64 ]; then fi command=$qemu $M -cpu $processor $chr_testdev -command+= -display none -serial stdio -kernel +command+= -display none -serial stdio $debugopts -kernel echo $command $@ $command $@ -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- Alex Bennée -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH v2 0/4] A couple of small fixes
I dropped the option parsing stuff which can be achieved by just appending QEMU args to the end of the command line. I've also addressed comments as suggested. Alex Bennée (4): README: add some CONTRIBUTING notes configure: emit HOST=$host to config.mak arm/run: introduce usingkvm var and use it arm/run: clean-up setting of accel options README| 26 ++ arm/run | 19 +-- configure | 2 ++ 3 files changed, 45 insertions(+), 2 deletions(-) -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH v2 1/4] README: add some CONTRIBUTING notes
Signed-off-by: Alex Bennée alex.ben...@linaro.org --- v2 - mention consistency --- README | 26 ++ 1 file changed, 26 insertions(+) diff --git a/README b/README index e9869d1..9389a26 100644 --- a/README +++ b/README @@ -25,3 +25,29 @@ Directory structure: ./ARCH: the sources of the tests and the created objects/images See ARCH/README for architecture specific documentation. + +CONTRIBUTING: += + +Style +- + +Currently there is a mix of indentation styles so any changes to +existing files should be consistent with the existing style. For new +files: + + - C: please use standard linux-with-tabs + - Shell: use TABs for indentation + +Patches +--- + +Patches are welcome at the KVM mailing list kvm@vger.kernel.org. + +Please prefix messages with: [kvm-unit-tests PATCH] + +You can add the following to .git/config to do this automatically for you: + +[format] + subjectprefix = kvm-unit-tests PATCH + -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH v2 4/4] arm/run: clean-up setting of accel options
It would be nice to use --no-kvm but that flags a warning on pure-TCG builds. We echo the fact we are using TCG for the benefit of interactive use. Signed-off-by: Alex Bennée alex.ben...@linaro.org --- v2 - rm redundant M= statement --- arm/run | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/arm/run b/arm/run index 493ce0d..7266004 100755 --- a/arm/run +++ b/arm/run @@ -32,6 +32,12 @@ if ! $qemu -machine '?' 21 | grep 'ARM Virtual Machine' /dev/null; then fi M='-machine virt' +if [ $usingkvm = 1 ]; then + M+=,accel=kvm +else + echo Running with TCG + M+=',accel=tcg' +fi if ! $qemu $M -device '?' 21 | grep virtconsole /dev/null; then echo $qpath doesn't support virtio-console for chr-testdev. Exiting. @@ -44,7 +50,6 @@ if $qemu $M -chardev testdev,id=id -initrd . 21 \ exit 2 fi -M='-machine virt,accel=kvm:tcg' chr_testdev='-device virtio-serial-device' chr_testdev+=' -device virtconsole,chardev=ctd -chardev testdev,id=ctd' -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH v2 3/4] arm/run: introduce usingkvm var and use it
This makes the script a little cleaner by only checking for KVM support in one place. Signed-off-by: Alex Bennée alex.ben...@linaro.org --- arm/run | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/arm/run b/arm/run index 6b42a2e..493ce0d 100755 --- a/arm/run +++ b/arm/run @@ -8,6 +8,16 @@ fi source config.mak processor=$PROCESSOR +# Default to using KVM if available and on the right ARM host +usingkvm=0 +if [ -c /dev/kvm ]; then + if [ $HOST = arm ] [ $ARCH = arm ]; then + usingkvm=1 + elif [ $HOST = aarch64 ]; then + usingkvm=1 + fi +fi + qemu=${QEMU:-qemu-system-$ARCH_NAME} qpath=$(which $qemu 2/dev/null) @@ -39,7 +49,7 @@ chr_testdev='-device virtio-serial-device' chr_testdev+=' -device virtconsole,chardev=ctd -chardev testdev,id=ctd' # arm64 must use '-cpu host' with kvm -if [ $(arch) = aarch64 ] [ $ARCH = arm64 ] [ -c /dev/kvm ]; then +if [ $usingkvm = 1 ] [ $ARCH = arm64 ]; then processor=host fi -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH v2 2/4] configure: emit HOST=$host to config.mak
This is useful information for the run scripts to know, especially if they want to drop to using TCG. Signed-off-by: Alex Bennée alex.ben...@linaro.org --- configure | 2 ++ 1 file changed, 2 insertions(+) diff --git a/configure b/configure index b2ad32a..078b70c 100755 --- a/configure +++ b/configure @@ -7,6 +7,7 @@ ld=ld objcopy=objcopy ar=ar arch=`uname -m | sed -e s/i.86/i386/ | sed -e 's/arm.*/arm/'` +host=$arch cross_prefix= usage() { @@ -122,6 +123,7 @@ ln -s $asm lib/asm cat EOF config.mak PREFIX=$prefix KERNELDIR=$(readlink -f $kerneldir) +HOST=$host ARCH=$arch ARCH_NAME=$arch_name PROCESSOR=$processor -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v8 07/11] KVM: arm64: re-factor hyp.S debug register code
This is a pre-cursor to sharing the code with the guest debug support. This replaces the big macro that fishes data out of a fixed location with a more general helper macro to restore a set of debug registers. It uses macro substitution so it can be re-used for debug control and value registers. It does however rely on the debug registers being 64 bit aligned (as they happen to be in the hyp ABI). Signed-off-by: Alex Bennée alex.ben...@linaro.org Reviewed-by: Christoffer Dall christoffer.d...@linaro.org --- v3: - return to the patch series - add save and restore targets - change register use and document v4: - keep original setup/restore names - don't use split u32/u64 structure yet v6: - fix ws and clobber info in hyp.S v7: - fix whitespace - add r-b-tag --- arch/arm64/kvm/hyp.S | 517 ++- 1 file changed, 138 insertions(+), 379 deletions(-) diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S index 2c67a14..77c08df 100644 --- a/arch/arm64/kvm/hyp.S +++ b/arch/arm64/kvm/hyp.S @@ -228,199 +228,52 @@ stp x24, x25, [x3, #160] .endm -.macro save_debug - // x2: base address for cpu context - // x3: tmp register - - mrs x26, id_aa64dfr0_el1 - ubfxx24, x26, #12, #4 // Extract BRPs - ubfxx25, x26, #20, #4 // Extract WRPs - mov w26, #15 - sub w24, w26, w24 // How many BPs to skip - sub w25, w26, w25 // How many WPs to skip - - add x3, x2, #CPU_SYSREG_OFFSET(DBGBCR0_EL1) - - adr x26, 1f - add x26, x26, x24, lsl #2 - br x26 -1: - mrs x20, dbgbcr15_el1 - mrs x19, dbgbcr14_el1 - mrs x18, dbgbcr13_el1 - mrs x17, dbgbcr12_el1 - mrs x16, dbgbcr11_el1 - mrs x15, dbgbcr10_el1 - mrs x14, dbgbcr9_el1 - mrs x13, dbgbcr8_el1 - mrs x12, dbgbcr7_el1 - mrs x11, dbgbcr6_el1 - mrs x10, dbgbcr5_el1 - mrs x9, dbgbcr4_el1 - mrs x8, dbgbcr3_el1 - mrs x7, dbgbcr2_el1 - mrs x6, dbgbcr1_el1 - mrs x5, dbgbcr0_el1 - - adr x26, 1f - add x26, x26, x24, lsl #2 - br x26 - -1: - str x20, [x3, #(15 * 8)] - str x19, [x3, #(14 * 8)] - str x18, [x3, #(13 * 8)] - str x17, [x3, #(12 * 8)] - str x16, [x3, #(11 * 8)] - str x15, [x3, #(10 * 8)] - str x14, [x3, #(9 * 8)] - str x13, [x3, #(8 * 8)] - str x12, [x3, #(7 * 8)] - str x11, [x3, #(6 * 8)] - str x10, [x3, #(5 * 8)] - str x9, [x3, #(4 * 8)] - str x8, [x3, #(3 * 8)] - str x7, [x3, #(2 * 8)] - str x6, [x3, #(1 * 8)] - str x5, [x3, #(0 * 8)] - - add x3, x2, #CPU_SYSREG_OFFSET(DBGBVR0_EL1) - - adr x26, 1f - add x26, x26, x24, lsl #2 - br x26 -1: - mrs x20, dbgbvr15_el1 - mrs x19, dbgbvr14_el1 - mrs x18, dbgbvr13_el1 - mrs x17, dbgbvr12_el1 - mrs x16, dbgbvr11_el1 - mrs x15, dbgbvr10_el1 - mrs x14, dbgbvr9_el1 - mrs x13, dbgbvr8_el1 - mrs x12, dbgbvr7_el1 - mrs x11, dbgbvr6_el1 - mrs x10, dbgbvr5_el1 - mrs x9, dbgbvr4_el1 - mrs x8, dbgbvr3_el1 - mrs x7, dbgbvr2_el1 - mrs x6, dbgbvr1_el1 - mrs x5, dbgbvr0_el1 - - adr x26, 1f - add x26, x26, x24, lsl #2 - br x26 - -1: - str x20, [x3, #(15 * 8)] - str x19, [x3, #(14 * 8)] - str x18, [x3, #(13 * 8)] - str x17, [x3, #(12 * 8)] - str x16, [x3, #(11 * 8)] - str x15, [x3, #(10 * 8)] - str x14, [x3, #(9 * 8)] - str x13, [x3, #(8 * 8)] - str x12, [x3, #(7 * 8)] - str x11, [x3, #(6 * 8)] - str x10, [x3, #(5 * 8)] - str x9, [x3, #(4 * 8)] - str x8, [x3, #(3 * 8)] - str x7, [x3, #(2 * 8)] - str x6, [x3, #(1 * 8)] - str x5, [x3, #(0 * 8)] - - add x3, x2, #CPU_SYSREG_OFFSET(DBGWCR0_EL1) - - adr x26, 1f - add x26, x26, x25, lsl #2 - br x26 +.macro save_debug type + // x4: pointer to register set + // x5: number of registers to skip + // x6..x22 trashed + + adr x22, 1f + add x22, x22, x5, lsl #2 + br x22 1: - mrs x20, dbgwcr15_el1 - mrs x19, dbgwcr14_el1 - mrs x18, dbgwcr13_el1 - mrs x17, dbgwcr12_el1 - mrs x16, dbgwcr11_el1 - mrs x15, dbgwcr10_el1 - mrs x14, dbgwcr9_el1 - mrs x13, dbgwcr8_el1 - mrs x12, dbgwcr7_el1 - mrs x11, dbgwcr6_el1 - mrs x10, dbgwcr5_el1 - mrs x9, dbgwcr4_el1 - mrs x8
[PATCH v8 10/11] KVM: arm64: enable KVM_CAP_SET_GUEST_DEBUG
Finally advertise the KVM capability for SET_GUEST_DEBUG. Once arm support is added this check can be moved to the common kvm_vm_ioctl_check_extension() code. Signed-off-by: Alex Bennée alex.ben...@linaro.org Acked-by: Christoffer Dall christoffer.d...@linaro.org --- v3: - separated capability check from previous patches - moved into arm64 specific ioctl handler. v4: - add a-b-tag --- arch/arm64/kvm/reset.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c index 8620926..b4af618 100644 --- a/arch/arm64/kvm/reset.c +++ b/arch/arm64/kvm/reset.c @@ -77,6 +77,9 @@ int kvm_arch_dev_ioctl_check_extension(long ext) case KVM_CAP_GUEST_DEBUG_HW_WPS: r = get_num_wrps(); break; + case KVM_CAP_SET_GUEST_DEBUG: + r = 1; + break; default: r = 0; } -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v8 09/11] KVM: arm64: guest debug, HW assisted debug support
This adds support for userspace to control the HW debug registers for guest debug. In the debug ioctl we copy an IMPDEF registers into a new register set called host_debug_state. We use the recently introduced vcpu parameter debug_ptr to select which register set is copied into the real registers when world switch occurs. I've made some helper functions from hw_breakpoint.c more widely available for re-use. As with single step we need to tweak the guest registers to enable the exceptions so we need to save and restore those bits. Two new capabilities have been added to the KVM_EXTENSION ioctl to allow userspace to query the number of hardware break and watch points available on the host hardware. Signed-off-by: Alex Bennée alex.ben...@linaro.org Reviewed-by: Christoffer Dall christoffer.d...@linaro.org --- v2 - switched to C setup - replace host debug registers directly into context - minor tweak to api docs - setup right register for debug - add FAR_EL2 to debug exit structure - add support for trapping debug register access v3 - remove stray trace statement - fix spacing around operators (various) - clean-up usage of trap_debug - introduce debug_ptr, replace excessive memcpy stuff - don't use memcpy in ioctl, just assign - update cap ioctl documentation - reword a number comments - rename host_debug_state-external_debug_state v4 - use the new u32/u64 split debug_ptr approach - fix some wording/comments v5 - don't set MDSCR_EL1.KDE (not needed) v6 - update wording given change in commentary - KVM_GUESTDBG_USE_HW_BP-KVM_GUESTDBG_USE_HW v7 - fix merge conflicts from ioctl move to guest.c - use kvm_arm_reset_debug_ptr to reset ptr - a BUG_ON() test has been added to trap failure to reset debug_ptr - debugging-debug in kvm_host.h comment - s/defined// s/to// in commit msg - rm ref to introducing debug_ptr in commit msg - add r-b tag v8 - add #include asm/cputype.h to hw_breakpoint.h --- Documentation/virtual/kvm/api.txt | 7 +- arch/arm64/include/asm/hw_breakpoint.h | 14 arch/arm64/include/asm/kvm_host.h | 6 - arch/arm64/kernel/hw_breakpoint.c | 12 -- arch/arm64/kvm/debug.c | 40 +- arch/arm64/kvm/guest.c | 7 ++ arch/arm64/kvm/handle_exit.c | 6 + arch/arm64/kvm/reset.c | 13 +++ include/uapi/linux/kvm.h | 2 ++ 9 files changed, 88 insertions(+), 19 deletions(-) diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt index 33c8143..ada57df 100644 --- a/Documentation/virtual/kvm/api.txt +++ b/Documentation/virtual/kvm/api.txt @@ -2668,7 +2668,7 @@ The top 16 bits of the control field are architecture specific control flags which can include the following: - KVM_GUESTDBG_USE_SW_BP: using software breakpoints [x86, arm64] - - KVM_GUESTDBG_USE_HW_BP: using hardware breakpoints [x86, s390] + - KVM_GUESTDBG_USE_HW_BP: using hardware breakpoints [x86, s390, arm64] - KVM_GUESTDBG_INJECT_DB: inject DB type exception [x86] - KVM_GUESTDBG_INJECT_BP: inject BP type exception [x86] - KVM_GUESTDBG_EXIT_PENDING: trigger an immediate guest exit [s390] @@ -2683,6 +2683,11 @@ updated to the correct (supplied) values. The second part of the structure is architecture specific and typically contains a set of debug registers. +For arm64 the number of debug registers is implementation defined and +can be determined by querying the KVM_CAP_GUEST_DEBUG_HW_BPS and +KVM_CAP_GUEST_DEBUG_HW_WPS capabilities which return a positive number +indicating the number of supported registers. + When debug events exit the main run loop with the reason KVM_EXIT_DEBUG with the kvm_debug_exit_arch part of the kvm_run structure containing architecture specific debug information. diff --git a/arch/arm64/include/asm/hw_breakpoint.h b/arch/arm64/include/asm/hw_breakpoint.h index 52b484b..4c47cb2 100644 --- a/arch/arm64/include/asm/hw_breakpoint.h +++ b/arch/arm64/include/asm/hw_breakpoint.h @@ -16,6 +16,8 @@ #ifndef __ASM_HW_BREAKPOINT_H #define __ASM_HW_BREAKPOINT_H +#include asm/cputype.h + #ifdef __KERNEL__ struct arch_hw_breakpoint_ctrl { @@ -132,5 +134,17 @@ static inline void ptrace_hw_copy_thread(struct task_struct *task) extern struct pmu perf_ops_bp; +/* Determine number of BRP registers available. */ +static inline int get_num_brps(void) +{ + return ((read_cpuid(ID_AA64DFR0_EL1) 12) 0xf) + 1; +} + +/* Determine number of WRP registers available. */ +static inline int get_num_wrps(void) +{ + return ((read_cpuid(ID_AA64DFR0_EL1) 20) 0xf) + 1; +} + #endif /* __KERNEL__ */ #endif /* __ASM_BREAKPOINT_H */ diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 461d288..6c745e0 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm
[PATCH v8 11/11] KVM: arm64: add trace points for guest_debug debug
This includes trace points for: kvm_arch_setup_guest_debug kvm_arch_clear_guest_debug I've also added some generic register setting trace events and also a trace point to dump the array of hardware registers. Signed-off-by: Alex Bennée alex.ben...@linaro.org --- v3 - add trace event for debug access. - remove short trace #define, rename trace events - use __print_array with fixed array instead of own func - rationalise trace points (only one per register changed) - add vcpu ptr to the debug_setup trace - remove :: in prints v4 - u32/u64 split on debug registers - fix for renames - add tracing of traps/set_guest_debug - remove handle_guest_debug trace v5 - minor print fmt fix - rm pstate traces v6 - fix merge conflicts - update control reg tracking to u64 (abi change) v7 - fix merge conflicts from ioctl move - fix other minor merge conflicts - fixes for the re-factored sys_regs code v8 - updates for sys_regs re-factor --- arch/arm64/kvm/debug.c| 36 +- arch/arm64/kvm/guest.c| 4 ++ arch/arm64/kvm/sys_regs.c | 17 +++ arch/arm64/kvm/trace.h| 123 ++ 4 files changed, 179 insertions(+), 1 deletion(-) diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c index 4a99e54..47e5f0f 100644 --- a/arch/arm64/kvm/debug.c +++ b/arch/arm64/kvm/debug.c @@ -18,12 +18,15 @@ */ #include linux/kvm_host.h +#include linux/hw_breakpoint.h #include asm/debug-monitors.h #include asm/kvm_asm.h #include asm/kvm_arm.h #include asm/kvm_emulate.h +#include trace.h + /* These are the bits of MDSCR_EL1 we may manipulate */ #define MDSCR_EL1_DEBUG_MASK (DBG_MDSCR_SS | \ DBG_MDSCR_KDE | \ @@ -44,11 +47,17 @@ static DEFINE_PER_CPU(u32, mdcr_el2); static void save_guest_debug_regs(struct kvm_vcpu *vcpu) { vcpu-arch.guest_debug_preserved.mdscr_el1 = vcpu_sys_reg(vcpu, MDSCR_EL1); + + trace_kvm_arm_set_dreg32(Saved MDSCR_EL1, + vcpu-arch.guest_debug_preserved.mdscr_el1); } static void restore_guest_debug_regs(struct kvm_vcpu *vcpu) { vcpu_sys_reg(vcpu, MDSCR_EL1) = vcpu-arch.guest_debug_preserved.mdscr_el1; + + trace_kvm_arm_set_dreg32(Restored MDSCR_EL1, + vcpu_sys_reg(vcpu, MDSCR_EL1)); } /** @@ -99,6 +108,8 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu) { bool trap_debug = !(vcpu-arch.debug_flags KVM_ARM64_DEBUG_DIRTY); + trace_kvm_arm_setup_debug(vcpu, vcpu-guest_debug); + vcpu-arch.mdcr_el2 = __this_cpu_read(mdcr_el2) MDCR_EL2_HPMN_MASK; vcpu-arch.mdcr_el2 |= (MDCR_EL2_TPM | MDCR_EL2_TPMCR | @@ -140,6 +151,8 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu) vcpu_sys_reg(vcpu, MDSCR_EL1) = ~DBG_MDSCR_SS; } + trace_kvm_arm_set_dreg32(SPSR_EL2, *vcpu_cpsr(vcpu)); + /* * HW Breakpoints and watchpoints * @@ -156,6 +169,14 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu) vcpu-arch.debug_ptr = vcpu-arch.external_debug_state; vcpu-arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY; trap_debug = true; + + trace_kvm_arm_set_regset(BKPTS, get_num_brps(), + vcpu-arch.debug_ptr-dbg_bcr[0], + vcpu-arch.debug_ptr-dbg_bvr[0]); + + trace_kvm_arm_set_regset(WAPTS, get_num_wrps(), + vcpu-arch.debug_ptr-dbg_wcr[0], + vcpu-arch.debug_ptr-dbg_wvr[0]); } } @@ -165,10 +186,15 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu) /* Trap debug register access */ if (trap_debug) vcpu-arch.mdcr_el2 |= MDCR_EL2_TDA; + + trace_kvm_arm_set_dreg32(MDCR_EL2, vcpu-arch.mdcr_el2); + trace_kvm_arm_set_dreg32(MDSCR_EL1, vcpu_sys_reg(vcpu, MDSCR_EL1)); } void kvm_arm_clear_debug(struct kvm_vcpu *vcpu) { + trace_kvm_arm_clear_debug(vcpu-guest_debug); + if (vcpu-guest_debug) { restore_guest_debug_regs(vcpu); @@ -176,8 +202,16 @@ void kvm_arm_clear_debug(struct kvm_vcpu *vcpu) * If we were using HW debug we need to restore the * debug_ptr to the guest debug state. */ - if (vcpu-guest_debug KVM_GUESTDBG_USE_HW) + if (vcpu-guest_debug KVM_GUESTDBG_USE_HW) { kvm_arm_reset_debug_ptr(vcpu); + trace_kvm_arm_set_regset(BKPTS, get_num_brps(), + vcpu-arch.debug_ptr-dbg_bcr[0], + vcpu-arch.debug_ptr-dbg_bvr[0
Re: [PATCH v7 09/11] KVM: arm64: guest debug, HW assisted debug support
Will Deacon will.dea...@arm.com writes: On Fri, Jul 03, 2015 at 05:07:41PM +0100, Alex Bennée wrote: Will Deacon will.dea...@arm.com writes: On Thu, Jul 02, 2015 at 02:50:33PM +0100, Alex Bennée wrote: Are you happy with this?: [...] +/** + * kvm_arch_dev_ioctl_check_extension + * + * We currently assume that the number of HW registers is uniform + * across all CPUs (see cpuinfo_sanity_check). + */ int kvm_arch_dev_ioctl_check_extension(long ext) { int r; @@ -64,6 +71,12 @@ int kvm_arch_dev_ioctl_check_extension(long ext) case KVM_CAP_ARM_EL1_32BIT: r = cpu_has_32bit_el1(); break; + case KVM_CAP_GUEST_DEBUG_HW_BPS: + r = hw_breakpoint_slots(TYPE_INST); + break; + case KVM_CAP_GUEST_DEBUG_HW_WPS: + r = hw_breakpoint_slots(TYPE_DATA); + break; Whilst I much prefer this code, it actually adds an unwanted dependency on PERF_EVENTS that I didn't think about to start with. Sorry to keep messing you about -- I guess your original patch is the best thing after all. Everything looks to be in hw_breakpoint.[ch] which does depend on CONFIG_HAVE_HW_BREAKPOINT which depends on PERF_EVENTS to be built. However the previous code depended on this behaviour as well. I think your original approach (of sticking stuff in the header file) works regardless of the CONFIG option, no? Ahh yeah I reverted that to an extern due to random compile breakage: http://storage.kernelci.org/alex/v4.1-12-gd38574dba3ec/arm64-allmodconfig/build.log I'll see if I can fix that up. It would seem weird to enable guest debug using HW debug registers to debug the guest yet not allowing the host kernel to use them? Of course this is the only code they would share as all the magic of guest debugging is already mostly there for dirty guest handling. I'm not familiar with Kconfig but it looks like this is all part of arm64 defconfig. Are people really going to want to disable PERF_EVENTS but still debug their guests with HW support? Then it's your call. I just find the host dependency on perf a bit weird to get guest debug working (especially as the dependency is completely fake because we don't use any perf infrastructure at all). Will -- Alex Bennée -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 7/7] arm/run: add --debug option
This allows you to pass debug options through to the QEMU command line. e.g.: ./arm/run --debug -name debug-threads=on -- arm/vos-spinlock-test.flat -smp 4 Signed-off-by: Alex Bennée alex.ben...@linaro.org --- arm/run | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/arm/run b/arm/run index 43d7508..871e35e 100755 --- a/arm/run +++ b/arm/run @@ -18,10 +18,15 @@ if [ -c /dev/kvm ]; then fi fi +debugopts= while :; do case $1 in --force-tcg) usingkvm=0 + ;; + --debug) + shift + debugopts=$1 shift ;; --) @@ -78,7 +83,7 @@ if [ $usingkvm = 1 ] [ $ARCH = arm64 ]; then fi command=$qemu $M -cpu $processor $chr_testdev -command+= -display none -serial stdio -kernel +command+= -display none -serial stdio $debugopts -kernel echo $command $@ $command $@ -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 6/7] arm/run: introduce basic option parsing
So far this simple option parsing loop allows us to --force-tcg even when running on ARM hardware. Signed-off-by: Alex Bennée alex.ben...@linaro.org --- arm/run | 17 + 1 file changed, 17 insertions(+) diff --git a/arm/run b/arm/run index a3a33b3..43d7508 100755 --- a/arm/run +++ b/arm/run @@ -18,6 +18,23 @@ if [ -c /dev/kvm ]; then fi fi +while :; do + case $1 in + --force-tcg) + usingkvm=0 + shift + ;; + --) + # End of all options. + shift + break + ;; + *) + break + ;; + esac +done + qemu=${QEMU:-qemu-system-$ARCH_NAME} qpath=$(which $qemu 2/dev/null) -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 4/7] run/arm: introduce usingkvm var and use it
This makes it easier to force KVM off in later patches. Signed-off-by: Alex Bennée alex.ben...@linaro.org --- arm/run | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/arm/run b/arm/run index 6b42a2e..493ce0d 100755 --- a/arm/run +++ b/arm/run @@ -8,6 +8,16 @@ fi source config.mak processor=$PROCESSOR +# Default to using KVM if available and on the right ARM host +usingkvm=0 +if [ -c /dev/kvm ]; then + if [ $HOST = arm ] [ $ARCH = arm ]; then + usingkvm=1 + elif [ $HOST = aarch64 ]; then + usingkvm=1 + fi +fi + qemu=${QEMU:-qemu-system-$ARCH_NAME} qpath=$(which $qemu 2/dev/null) @@ -39,7 +49,7 @@ chr_testdev='-device virtio-serial-device' chr_testdev+=' -device virtconsole,chardev=ctd -chardev testdev,id=ctd' # arm64 must use '-cpu host' with kvm -if [ $(arch) = aarch64 ] [ $ARCH = arm64 ] [ -c /dev/kvm ]; then +if [ $usingkvm = 1 ] [ $ARCH = arm64 ]; then processor=host fi -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 1/7] READ: add some CONTRIBUTING notes
Signed-off-by: Alex Bennée alex.ben...@linaro.org --- README | 22 ++ 1 file changed, 22 insertions(+) diff --git a/README b/README index e9869d1..4001456 100644 --- a/README +++ b/README @@ -25,3 +25,25 @@ Directory structure: ./ARCH: the sources of the tests and the created objects/images See ARCH/README for architecture specific documentation. + +CONTRIBUTING: += + +Style +- + + - C: please use standard linux-with-tabs + - Shell: use TABs for indentation + +Patches +--- + +Patches are welcome at the KVM mailing list kvm@vger.kernel.org. + +Please prefix messages with: [kvm-unit-tests PATCH] + +You can add the following to .git/config to do this automatically for you: + +[format] + subjectprefix = kvm-unit-tests PATCH + -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 5/7] arm/run: clean-up setting of accel options
It would be nice to use --no-kvm but that flags a warning on pure-TCG builds. We echo the fact we are using TCG for the benefit of interactive use. Signed-off-by: Alex Bennée alex.ben...@linaro.org --- arm/run | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/arm/run b/arm/run index 493ce0d..a3a33b3 100755 --- a/arm/run +++ b/arm/run @@ -44,7 +44,14 @@ if $qemu $M -chardev testdev,id=id -initrd . 21 \ exit 2 fi -M='-machine virt,accel=kvm:tcg' +M='-machine virt' +if [ $usingkvm = 1 ]; then + M+=,accel=kvm +else + echo Running with TCG + M+=',accel=tcg' +fi + chr_testdev='-device virtio-serial-device' chr_testdev+=' -device virtconsole,chardev=ctd -chardev testdev,id=ctd' -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 2/7] configure: emit HOST=$host to config.mak
This is useful information for the run scripts to know, especially if they want to drop to using TCG. Signed-off-by: Alex Bennée alex.ben...@linaro.org --- configure | 2 ++ 1 file changed, 2 insertions(+) diff --git a/configure b/configure index b2ad32a..078b70c 100755 --- a/configure +++ b/configure @@ -7,6 +7,7 @@ ld=ld objcopy=objcopy ar=ar arch=`uname -m | sed -e s/i.86/i386/ | sed -e 's/arm.*/arm/'` +host=$arch cross_prefix= usage() { @@ -122,6 +123,7 @@ ln -s $asm lib/asm cat EOF config.mak PREFIX=$prefix KERNELDIR=$(readlink -f $kerneldir) +HOST=$host ARCH=$arch ARCH_NAME=$arch_name PROCESSOR=$processor -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 3/7] arm/run: set indentation defaults for emacs
Signed-off-by: Alex Bennée alex.ben...@linaro.org --- arm/run | 1 + 1 file changed, 1 insertion(+) diff --git a/arm/run b/arm/run index 662a856..6b42a2e 100755 --- a/arm/run +++ b/arm/run @@ -1,4 +1,5 @@ #!/bin/bash +# -*- sh-basic-offset:8 indent-tabs-mode: t -*- if [ ! -f config.mak ]; then echo run ./configure first. See ./configure -h -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 0/7] A number of small arm/run fixes
Following on from yesterdays comments I've re-spun the changes as suggested. I've also added a very simple opt parsing for arm/run to allow us to --force-tcg and --debug more qemu cmdline. As a bonus I've codified some notes for contributing into the README. Alex Bennée (7): READ: add some CONTRIBUTING notes configure: emit HOST=$host to config.mak arm/run: set indentation defaults for emacs run/arm: introduce usingkvm var and use it arm/run: clean-up setting of accel options arm/run: introduce basic option parsing arm/run: add --debug option README| 22 ++ arm/run | 46 +++--- configure | 2 ++ 3 files changed, 67 insertions(+), 3 deletions(-) -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm/run: don't enable KVM if system can't do it
Andrew Jones drjo...@redhat.com writes: On Thu, Jul 02, 2015 at 03:45:17PM +0200, Paolo Bonzini wrote: On 02/07/2015 13:51, Andrew Jones wrote: 4) I recently mentioned[*] it might be nice to add a '-force-tcg' type of arm/run command line option, allowing tcg to be used even if it's possible to use kvm. Adding that at the same time would be nice. Can you just use --no-kvm? It is equivalent to -machine accel=tcg, Sounds perfect. Thanks! and it overrides previous -machine accel=foo options. For a TCG only build it complains: Option no-kvm not supported for this target :-/ Paolo ps: I also share the yay feeling, of course! -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- Alex Bennée -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 09/11] KVM: arm64: guest debug, HW assisted debug support
Will Deacon will.dea...@arm.com writes: Hi Alex, On Thu, Jul 02, 2015 at 02:50:33PM +0100, Alex Bennée wrote: Are you happy with this?: [...] +/** + * kvm_arch_dev_ioctl_check_extension + * + * We currently assume that the number of HW registers is uniform + * across all CPUs (see cpuinfo_sanity_check). + */ int kvm_arch_dev_ioctl_check_extension(long ext) { int r; @@ -64,6 +71,12 @@ int kvm_arch_dev_ioctl_check_extension(long ext) case KVM_CAP_ARM_EL1_32BIT: r = cpu_has_32bit_el1(); break; + case KVM_CAP_GUEST_DEBUG_HW_BPS: + r = hw_breakpoint_slots(TYPE_INST); + break; + case KVM_CAP_GUEST_DEBUG_HW_WPS: + r = hw_breakpoint_slots(TYPE_DATA); + break; Whilst I much prefer this code, it actually adds an unwanted dependency on PERF_EVENTS that I didn't think about to start with. Sorry to keep messing you about -- I guess your original patch is the best thing after all. Everything looks to be in hw_breakpoint.[ch] which does depend on CONFIG_HAVE_HW_BREAKPOINT which depends on PERF_EVENTS to be built. However the previous code depended on this behaviour as well. It would seem weird to enable guest debug using HW debug registers to debug the guest yet not allowing the host kernel to use them? Of course this is the only code they would share as all the magic of guest debugging is already mostly there for dirty guest handling. I'm not familiar with Kconfig but it looks like this is all part of arm64 defconfig. Are people really going to want to disable PERF_EVENTS but still debug their guests with HW support? Will -- Alex Bennée -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 08/11] KVM: arm64: introduce vcpu-arch.debug_ptr
Christoffer Dall christoffer.d...@linaro.org writes: On Wed, Jul 01, 2015 at 07:29:00PM +0100, Alex Bennée wrote: This introduces a level of indirection for the debug registers. Instead of using the sys_regs[] directly we store registers in a structure in the vcpu. The new kvm_arm_reset_debug_ptr() sets the debug ptr to the guest context. This also entails updating the sys_regs code to access this new structure. New access function have been added for each set of debug registers. The generic functions are still used for the few registers stored in the main context. New access function pointers have been added to the sys_reg_desc structure to support the GET/SET_ONE_REG ioctl operations. Why is this needed? Previously I had a hacky: if (r-access == trap_debug64) return debug_get64(vcpu, r, reg, uaddr); Which used the same offset information. Now we have a cleaner: if (r-set) return (r-set)(vcpu, r, reg, uaddr); Which accesses the structure directly, as the trap functions do: __u64 *r = vcpu-arch.vcpu_debug_state.dbg_bvr[rd-reg]; if (copy_from_user(uaddr, r, KVM_REG_SIZE(reg-id)) != 0) return -EFAULT; return 0; snip +#if 0 +static int debug_set64(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd, +const struct kvm_one_reg *reg, void __user *uaddr) +{ +__u64 *r = (__u64 *) ((void * )vcpu-arch.vcpu_debug_state + rd-reg); +if (copy_from_user(uaddr, r, KVM_REG_SIZE(reg-id)) != 0) +return -EFAULT; +return 0; +} + +static int debug_get64(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd, +const struct kvm_one_reg *reg, void __user *uaddr) +{ +__u64 *r = (__u64 *) ((void * )vcpu-arch.vcpu_debug_state + rd-reg); +if (copy_to_user(uaddr, r, KVM_REG_SIZE(reg-id)) != 0) +return -EFAULT; +return 0; +} +#endif + what is this ifdef'ed block of code doing here? Oops. Yeah looks like I missed removing that after I finished the re-factor. These where the old get/set functions I used. int kvm_arm_sys_reg_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) { const struct sys_reg_desc *r; @@ -1303,6 +1530,9 @@ int kvm_arm_sys_reg_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg if (!r) return get_invariant_sys_reg(reg-id, uaddr); +if (r-get) +return (r-get)(vcpu, r, reg, uaddr); + return reg_to_user(uaddr, vcpu_sys_reg(vcpu, r-reg), reg-id); } @@ -1321,6 +1551,9 @@ int kvm_arm_sys_reg_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg if (!r) return set_invariant_sys_reg(reg-id, uaddr); +if (r-set) +return (r-set)(vcpu, r, reg, uaddr); + return reg_from_user(vcpu_sys_reg(vcpu, r-reg), uaddr, reg-id); } diff --git a/arch/arm64/kvm/sys_regs.h b/arch/arm64/kvm/sys_regs.h index d411e25..9265e7d 100644 --- a/arch/arm64/kvm/sys_regs.h +++ b/arch/arm64/kvm/sys_regs.h @@ -55,6 +55,12 @@ struct sys_reg_desc { /* Value (usually reset value) */ u64 val; + +/* Get/Set functions, fallback if NULL */ Is this only meant for usersapce access or when should one use these? Yes for GET/SET +int (*get)(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd, + const struct kvm_one_reg *reg, void __user *uaddr); +int (*set)(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd, + const struct kvm_one_reg *reg, void __user *uaddr); }; static inline void print_sys_reg_instr(const struct sys_reg_params *p) -- 2.4.5 -- Alex Bennée -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] arm/run: don't enable KVM if system can't do it
As ARM (and no doubt other systems) can also run tests in pure TCG mode we might as well not bother enabling accel=kvm if we aren't on a real ARM based system. This prevents us seeing ugly warning messages when testing TCG. Signed-off-by: Alex Bennée alex.ben...@linaro.org --- arm/run | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/arm/run b/arm/run index 662a856..2bdb4be 100755 --- a/arm/run +++ b/arm/run @@ -33,7 +33,13 @@ if $qemu $M -chardev testdev,id=id -initrd . 21 \ exit 2 fi -M='-machine virt,accel=kvm:tcg' +host=`uname -m | sed -e 's/arm.*/arm/'` +if [ ${host} = arm ] || [ ${host} = aarch64 ]; then +M='-machine virt,accel=kvm:tcg' +else +M='-machine virt,accel=tcg' +fi + chr_testdev='-device virtio-serial-device' chr_testdev+=' -device virtconsole,chardev=ctd -chardev testdev,id=ctd' -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm/run: don't enable KVM if system can't do it
Andrew Jones drjo...@redhat.com writes: On Thu, Jul 02, 2015 at 12:05:31PM +0100, Alex Bennée wrote: As ARM (and no doubt other systems) can also run tests in pure TCG mode we might as well not bother enabling accel=kvm if we aren't on a real ARM based system. This prevents us seeing ugly warning messages when testing TCG. First, YAY! We're getting contributions to kvm-unit-tests/arm! :-) well so far I've been noodling about looking at it for KVM Guest Debug testing. I've a hideous branch on github that attempts to test exercise the debug register trapping code. However that falls down as I really need to find an easy way of attaching GDB to the qemu-gdb stub while the test is running. However with the TCG multi-thread work coming up I certainly see the need to exercise QEMU in a way that the internal TCG test code might have trouble with. Signed-off-by: Alex Bennée alex.ben...@linaro.org --- arm/run | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/arm/run b/arm/run index 662a856..2bdb4be 100755 --- a/arm/run +++ b/arm/run @@ -33,7 +33,13 @@ if $qemu $M -chardev testdev,id=id -initrd . 21 \ exit 2 fi -M='-machine virt,accel=kvm:tcg' +host=`uname -m | sed -e 's/arm.*/arm/'` +if [ ${host} = arm ] || [ ${host} = aarch64 ]; then +M='-machine virt,accel=kvm:tcg' +else +M='-machine virt,accel=tcg' +fi I think this is a good idea, although I had actually left that warning on purpose. Originally, the plan was for these unit tests to be kvm specific. If they could be developed with the aid of tcg, and even used to test tcg, then fine, but running them on tcg should always complain, in order to make sure that the test output clearly showed that it had not been running on kvm. Developing unit tests for tcg is also a good idea though, and there's really no reason not to share this framework. So, for this patch I'd prefer we do a few things differently; 1) we should be able to integrate this new condition with the arm64 must use '-cpu host' with kvm condition that is lower down. And, let's just make this $HOST variable one that ./configure prepares, allowing that arm64 condition to s/$(arch)/$HOST/ and avoiding the need to duplicate the sed -e 's/arm.*/arm/' Yeah makes sense. 2) we might as well do something like M='-machine virt' if using-kvm M+=',accel=kvm' else M+=',accel=tcg' fi now, since we don't want to use the accel fallback feature anymore 3) outputting which one we're using might still be nice, otherwise one must inspect the qemu command line in the logs to find out 4) I recently mentioned[*] it might be nice to add a '-force-tcg' type of arm/run command line option, allowing tcg to be used even if it's possible to use kvm. Adding that at the same time would be nice. Would it also be useful for other arches? Does run-tests.sh pass 5) we use tabs for indentation in arm/run, and only bother with the variable's {}, if necessary My shell quoting was rusty. I think $(host) was calling the host command for some reason. 6) we should post patches with [kvm-unit-tests PATCH] to avoid confusion with other kvm postings. (I screwed that up on my last two postings...). /me ponders if he can just config git for that. I'll patch the readme ;-) Thanks! drew [*] https://lists.gnu.org/archive/html/qemu-devel/2015-06/msg07514.html + chr_testdev='-device virtio-serial-device' chr_testdev+=' -device virtconsole,chardev=ctd -chardev testdev,id=ctd' -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- Alex Bennée -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 09/11] KVM: arm64: guest debug, HW assisted debug support
Will Deacon will.dea...@arm.com writes: Are you happy with this?: Subject: [PATCH v8 09/11] KVM: arm64: guest debug, HW assisted debug support This adds support for userspace to control the HW debug registers for guest debug. In the debug ioctl we copy an IMPDEF registers into a new register set called host_debug_state. We use the recently introduced vcpu parameter debug_ptr to select which register set is copied into the real registers when world switch occurs. I've made some helper functions from hw_breakpoint.c more widely available for re-use. As with single step we need to tweak the guest registers to enable the exceptions so we need to save and restore those bits. Two new capabilities have been added to the KVM_EXTENSION ioctl to allow userspace to query the number of hardware break and watch points available on the host hardware. Signed-off-by: Alex Bennée alex.ben...@linaro.org Reviewed-by: Christoffer Dall christoffer.d...@linaro.org --- v2 - switched to C setup - replace host debug registers directly into context - minor tweak to api docs - setup right register for debug - add FAR_EL2 to debug exit structure - add support for trapping debug register access v3 - remove stray trace statement - fix spacing around operators (various) - clean-up usage of trap_debug - introduce debug_ptr, replace excessive memcpy stuff - don't use memcpy in ioctl, just assign - update cap ioctl documentation - reword a number comments - rename host_debug_state-external_debug_state v4 - use the new u32/u64 split debug_ptr approach - fix some wording/comments v5 - don't set MDSCR_EL1.KDE (not needed) v6 - update wording given change in commentary - KVM_GUESTDBG_USE_HW_BP-KVM_GUESTDBG_USE_HW v7 - fix merge conflicts from ioctl move to guest.c - use kvm_arm_reset_debug_ptr to reset ptr - a BUG_ON() test has been added to trap failure to reset debug_ptr - debugging-debug in kvm_host.h comment - s/defined// s/to// in commit msg - rm ref to introducing debug_ptr in commit msg - add r-b tag v8 - use hw_breakpoint_slots() instead --- Documentation/virtual/kvm/api.txt | 7 ++- arch/arm64/include/asm/kvm_host.h | 6 +- arch/arm64/kvm/debug.c| 40 ++- arch/arm64/kvm/guest.c| 7 +++ arch/arm64/kvm/handle_exit.c | 6 ++ arch/arm64/kvm/reset.c| 13 + arch/arm64/kvm/sys_regs.c | 3 --- include/uapi/linux/kvm.h | 2 ++ 8 files changed, 74 insertions(+), 10 deletions(-) diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt index 33c8143..ada57df 100644 --- a/Documentation/virtual/kvm/api.txt +++ b/Documentation/virtual/kvm/api.txt @@ -2668,7 +2668,7 @@ The top 16 bits of the control field are architecture specific control flags which can include the following: - KVM_GUESTDBG_USE_SW_BP: using software breakpoints [x86, arm64] - - KVM_GUESTDBG_USE_HW_BP: using hardware breakpoints [x86, s390] + - KVM_GUESTDBG_USE_HW_BP: using hardware breakpoints [x86, s390, arm64] - KVM_GUESTDBG_INJECT_DB: inject DB type exception [x86] - KVM_GUESTDBG_INJECT_BP: inject BP type exception [x86] - KVM_GUESTDBG_EXIT_PENDING: trigger an immediate guest exit [s390] @@ -2683,6 +2683,11 @@ updated to the correct (supplied) values. The second part of the structure is architecture specific and typically contains a set of debug registers. +For arm64 the number of debug registers is implementation defined and +can be determined by querying the KVM_CAP_GUEST_DEBUG_HW_BPS and +KVM_CAP_GUEST_DEBUG_HW_WPS capabilities which return a positive number +indicating the number of supported registers. + When debug events exit the main run loop with the reason KVM_EXIT_DEBUG with the kvm_debug_exit_arch part of the kvm_run structure containing architecture specific debug information. diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 461d288..6c745e0 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -116,13 +116,17 @@ struct kvm_vcpu_arch { * debugging the guest from the host and to maintain separate host and * guest state during world switches. vcpu_debug_state are the debug * registers of the vcpu as the guest sees them. host_debug_state are -* the host registers which are saved and restored during world switches. +* the host registers which are saved and restored during +* world switches. external_debug_state contains the debug +* values we want to debug the guest. This is set via the +* KVM_SET_GUEST_DEBUG ioctl. * * debug_ptr points to the set of debug registers that should be loaded * onto the hardware when running the guest. */ struct kvm_guest_debug_arch *debug_ptr; struct
[PATCH v7 05/11] KVM: arm64: guest debug, add SW break point support
This adds support for SW breakpoints inserted by userspace. We do this by trapping all guest software debug exceptions to the hypervisor (MDCR_EL2.TDE). The exit handler sets an exit reason of KVM_EXIT_DEBUG with the kvm_debug_exit_arch structure holding the exception syndrome information. It will be up to userspace to extract the PC (via GET_ONE_REG) and determine if the debug event was for a breakpoint it inserted. If not userspace will need to re-inject the correct exception restart the hypervisor to deliver the debug exception to the guest. Any other guest software debug exception (e.g. single step or HW assisted breakpoints) will cause an error and the VM to be killed. This is addressed by later patches which add support for the other debug types. Signed-off-by: Alex Bennée alex.ben...@linaro.org Reviewed-by: Christoffer Dall christoffer.d...@linaro.org --- v2 - update to use new exit struct - tweak for C setup - do our setup in debug_setup/clear code - fixed up comments v3: - fix spacing in KVM_GUESTDBG_VALID_MASK - fix and clarify wording on kvm_handle_guest_debug - handle error case in kvm_handle_guest_debug - re-word the commit message v4 - rm else leg - add r-b-tag v7 - moved ioctl to guest --- Documentation/virtual/kvm/api.txt | 2 +- arch/arm64/kvm/debug.c| 3 +++ arch/arm64/kvm/guest.c| 2 +- arch/arm64/kvm/handle_exit.c | 36 4 files changed, 41 insertions(+), 2 deletions(-) diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt index ba635c7..33c8143 100644 --- a/Documentation/virtual/kvm/api.txt +++ b/Documentation/virtual/kvm/api.txt @@ -2667,7 +2667,7 @@ when running. Common control bits are: The top 16 bits of the control field are architecture specific control flags which can include the following: - - KVM_GUESTDBG_USE_SW_BP: using software breakpoints [x86] + - KVM_GUESTDBG_USE_SW_BP: using software breakpoints [x86, arm64] - KVM_GUESTDBG_USE_HW_BP: using hardware breakpoints [x86, s390] - KVM_GUESTDBG_INJECT_DB: inject DB type exception [x86] - KVM_GUESTDBG_INJECT_BP: inject BP type exception [x86] diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c index faf0e1f..8d1bfa4 100644 --- a/arch/arm64/kvm/debug.c +++ b/arch/arm64/kvm/debug.c @@ -73,6 +73,9 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu) if (trap_debug) vcpu-arch.mdcr_el2 |= MDCR_EL2_TDA; + /* Trap breakpoints? */ + if (vcpu-guest_debug KVM_GUESTDBG_USE_SW_BP) + vcpu-arch.mdcr_el2 |= MDCR_EL2_TDE; } void kvm_arm_clear_debug(struct kvm_vcpu *vcpu) diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c index 0ba8677..22d22c5 100644 --- a/arch/arm64/kvm/guest.c +++ b/arch/arm64/kvm/guest.c @@ -332,7 +332,7 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu, return -EINVAL; } -#define KVM_GUESTDBG_VALID_MASK (KVM_GUESTDBG_ENABLE) +#define KVM_GUESTDBG_VALID_MASK (KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP) /** * kvm_arch_vcpu_ioctl_set_guest_debug - set up guest debugging diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c index 524fa25..27f38a9 100644 --- a/arch/arm64/kvm/handle_exit.c +++ b/arch/arm64/kvm/handle_exit.c @@ -82,6 +82,40 @@ static int kvm_handle_wfx(struct kvm_vcpu *vcpu, struct kvm_run *run) return 1; } +/** + * kvm_handle_guest_debug - handle a debug exception instruction + * + * @vcpu: the vcpu pointer + * @run: access to the kvm_run structure for results + * + * We route all debug exceptions through the same handler. If both the + * guest and host are using the same debug facilities it will be up to + * userspace to re-inject the correct exception for guest delivery. + * + * @return: 0 (while setting run-exit_reason), -1 for error + */ +static int kvm_handle_guest_debug(struct kvm_vcpu *vcpu, struct kvm_run *run) +{ + u32 hsr = kvm_vcpu_get_hsr(vcpu); + int ret = 0; + + run-exit_reason = KVM_EXIT_DEBUG; + run-debug.arch.hsr = hsr; + + switch (hsr ESR_ELx_EC_SHIFT) { + case ESR_ELx_EC_BKPT32: + case ESR_ELx_EC_BRK64: + break; + default: + kvm_err(%s: un-handled case hsr: %#08x\n, + __func__, (unsigned int) hsr); + ret = -1; + break; + } + + return ret; +} + static exit_handle_fn arm_exit_handlers[] = { [ESR_ELx_EC_WFx]= kvm_handle_wfx, [ESR_ELx_EC_CP15_32]= kvm_handle_cp15_32, @@ -96,6 +130,8 @@ static exit_handle_fn arm_exit_handlers[] = { [ESR_ELx_EC_SYS64] = kvm_handle_sys_reg, [ESR_ELx_EC_IABT_LOW] = kvm_handle_guest_abort, [ESR_ELx_EC_DABT_LOW] = kvm_handle_guest_abort, + [ESR_ELx_EC_BKPT32] = kvm_handle_guest_debug, + [ESR_ELx_EC_BRK64] = kvm_handle_guest_debug, }; static
[PATCH v7 04/11] KVM: arm: introduce kvm_arm_init/setup/clear_debug
This is a precursor for later patches which will need to do more to setup debug state before entering the hyp.S switch code. The existing functionality for setting mdcr_el2 has been moved out of hyp.S and now uses the value kept in vcpu-arch.mdcr_el2. As the assembler used to previously mask and preserve MDCR_EL2.HPMN I've had to add a mechanism to save the value of mdcr_el2 as a per-cpu variable during the initialisation code. The kernel never sets this number so we are assuming the bootcode has set up the correct value here. This also moves the conditional setting of the TDA bit from the hyp code into the C code which is currently used for the lazy debug register context switch code. Signed-off-by: Alex Bennée alex.ben...@linaro.org Reviewed-by: Christoffer Dall christoffer.d...@linaro.org --- v3 - rename fns from arch-arm - preserve MDCR_EL2.HPMN setting - re-word some of the comments - fix some minor grammar nits - merge setting of mdcr_el2 - introduce trap_debug flag - move setup/clear within the irq lock section v4 - fix TDOSA desc - rm un-needed else leg - s/arch/arm/ v6 - add s-o-b tag --- arch/arm/include/asm/kvm_host.h | 4 ++ arch/arm/kvm/arm.c| 9 - arch/arm64/include/asm/kvm_asm.h | 2 + arch/arm64/include/asm/kvm_host.h | 5 +++ arch/arm64/kernel/asm-offsets.c | 1 + arch/arm64/kvm/Makefile | 2 +- arch/arm64/kvm/debug.c| 81 +++ arch/arm64/kvm/hyp.S | 19 - 8 files changed, 110 insertions(+), 13 deletions(-) create mode 100644 arch/arm64/kvm/debug.c diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h index d71607c..746c0c69 100644 --- a/arch/arm/include/asm/kvm_host.h +++ b/arch/arm/include/asm/kvm_host.h @@ -236,4 +236,8 @@ static inline void kvm_arch_sync_events(struct kvm *kvm) {} static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {} static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {} +static inline void kvm_arm_init_debug(void) {} +static inline void kvm_arm_setup_debug(struct kvm_vcpu *vcpu) {} +static inline void kvm_arm_clear_debug(struct kvm_vcpu *vcpu) {} + #endif /* __ARM_KVM_HOST_H__ */ diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c index 92b80bc..af60e6f 100644 --- a/arch/arm/kvm/arm.c +++ b/arch/arm/kvm/arm.c @@ -542,6 +542,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run) continue; } + kvm_arm_setup_debug(vcpu); + /** * Enter the guest */ @@ -554,7 +556,10 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run) vcpu-mode = OUTSIDE_GUEST_MODE; kvm_guest_exit(); trace_kvm_exit(kvm_vcpu_trap_get_class(vcpu), *vcpu_pc(vcpu)); - /* + + kvm_arm_clear_debug(vcpu); + +/* * We may have taken a host interrupt in HYP mode (ie * while executing the guest). This interrupt is still * pending, as we haven't serviced it yet! @@ -902,6 +907,8 @@ static void cpu_init_hyp_mode(void *dummy) vector_ptr = (unsigned long)__kvm_hyp_vector; __cpu_init_hyp_mode(boot_pgd_ptr, pgd_ptr, hyp_stack_ptr, vector_ptr); + + kvm_arm_init_debug(); } static int hyp_init_cpu_notify(struct notifier_block *self, diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h index 4f7310f..d6b507e 100644 --- a/arch/arm64/include/asm/kvm_asm.h +++ b/arch/arm64/include/asm/kvm_asm.h @@ -137,6 +137,8 @@ extern char __restore_vgic_v2_state[]; extern char __save_vgic_v3_state[]; extern char __restore_vgic_v3_state[]; +extern u32 __kvm_get_mdcr_el2(void); + #endif #endif /* __ARM_KVM_ASM_H__ */ diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index f0f58c9..7cb99b5 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -103,6 +103,7 @@ struct kvm_vcpu_arch { /* HYP configuration */ u64 hcr_el2; + u32 mdcr_el2; /* Exception Information */ struct kvm_vcpu_fault_info fault; @@ -250,4 +251,8 @@ static inline void kvm_arch_sync_events(struct kvm *kvm) {} static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {} static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {} +void kvm_arm_init_debug(void); +void kvm_arm_setup_debug(struct kvm_vcpu *vcpu); +void kvm_arm_clear_debug(struct kvm_vcpu *vcpu); + #endif /* __ARM64_KVM_HOST_H__ */ diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c index da675cc..dfb25a2 100644 --- a/arch/arm64/kernel/asm-offsets.c +++ b/arch/arm64/kernel/asm-offsets.c @@ -117,6 +117,7 @@ int main(void) DEFINE(VCPU_HPFAR_EL2
[PATCH v7 01/11] KVM: add comments for kvm_debug_exit_arch struct
Bring into line with the comments for the other structures and their KVM_EXIT_* cases. Also update api.txt to reflect use in kvm_run documentation. Signed-off-by: Alex Bennée alex.ben...@linaro.org Reviewed-by: David Hildenbrand d...@linux.vnet.ibm.com Reviewed-by: Andrew Jones drjo...@redhat.com Acked-by: Christoffer Dall christoffer.d...@linaro.org --- v2 - add comments for other exit types v3 - s/commentary/comments/ - add rb tags - update api.txt kvm_run to include KVM_EXIT_DEBUG desc v4 - sp fixes - add a-b --- Documentation/virtual/kvm/api.txt | 4 +++- include/uapi/linux/kvm.h | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt index 9fa2bf8..c34c32d 100644 --- a/Documentation/virtual/kvm/api.txt +++ b/Documentation/virtual/kvm/api.txt @@ -3070,11 +3070,13 @@ data_offset describes where the data is located (KVM_EXIT_IO_OUT) or where kvm expects application code to place the data for the next KVM_RUN invocation (KVM_EXIT_IO_IN). Data format is a packed array. + /* KVM_EXIT_DEBUG */ struct { struct kvm_debug_exit_arch arch; } debug; -Unused. +If the exit_reason is KVM_EXIT_DEBUG, then a vcpu is processing a debug event +for which architecture specific information is returned. /* KVM_EXIT_MMIO */ struct { diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index 4b60056..70ac641 100644 --- a/include/uapi/linux/kvm.h +++ b/include/uapi/linux/kvm.h @@ -237,6 +237,7 @@ struct kvm_run { __u32 count; __u64 data_offset; /* relative to kvm_run start */ } io; + /* KVM_EXIT_DEBUG */ struct { struct kvm_debug_exit_arch arch; } debug; @@ -285,6 +286,7 @@ struct kvm_run { __u32 data; __u8 is_write; } dcr; + /* KVM_EXIT_INTERNAL_ERROR */ struct { __u32 suberror; /* Available with KVM_CAP_INTERNAL_ERROR_DATA: */ @@ -295,6 +297,7 @@ struct kvm_run { struct { __u64 gprs[32]; } osi; + /* KVM_EXIT_PAPR_HCALL */ struct { __u64 nr; __u64 ret; -- 2.4.5 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 06/11] KVM: arm64: guest debug, add support for single-step
This adds support for single-stepping the guest. To do this we need to manipulate the guests PSTATE.SS and MDSCR_EL1.SS bits to trigger stepping. We take care to preserve MDSCR_EL1 and trap access to it to ensure we don't affect the apparent state of the guest. As we have to enable trapping of all software debug exceptions we suppress the ability of the guest to single-step itself. If we didn't we would have to deal with the exception arriving while the guest was in kernelspace when the guest is expecting to single-step userspace. This is something we don't want to unwind in the kernel. Once the host is no longer debugging the guest its ability to single-step userspace is restored. Signed-off-by: Alex Bennée alex.ben...@linaro.org Reviewed-by: Christoffer Dall christoffer.d...@linaro.org --- v2 - Move pstate/mdscr manipulation into C - don't export guest_debug to assembly - add accessor for saved_debug regs - tweak save/restore of mdscr_el1 v3 - don't save PC in debug information struct - rename debug_saved_regs-guest_debug_state - save whole value, only use bits in restore - add save/restore_guest-debug_regs helper functions - simplify commit message for clarity - rm vcpu_debug_saved_reg access fn v4 - added more comments based on suggestions - guest_debug_state-guest_debug_preserved - no point masking restore, we will trap out v5 - more comments - don't bother preserving pstate.ss (guest never sees change) v6 - reword comments on guest SS suppression - simplify comment for save regs, SS explained in detail later on - add r-b-t (code) - expanded commit description v7 - merge fix for ioctl move to guest.c --- arch/arm64/include/asm/kvm_host.h | 11 +++ arch/arm64/kvm/debug.c| 68 --- arch/arm64/kvm/guest.c| 4 ++- arch/arm64/kvm/handle_exit.c | 2 ++ 4 files changed, 80 insertions(+), 5 deletions(-) diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 7cb99b5..e2db6a6 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -123,6 +123,17 @@ struct kvm_vcpu_arch { * here. */ + /* +* Guest registers we preserve during guest debugging. +* +* These shadow registers are updated by the kvm_handle_sys_reg +* trap handler if the guest accesses or updates them while we +* are using guest debug. +*/ + struct { + u32 mdscr_el1; + } guest_debug_preserved; + /* Don't run the guest */ bool pause; diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c index 8d1bfa4..d439eb8 100644 --- a/arch/arm64/kvm/debug.c +++ b/arch/arm64/kvm/debug.c @@ -19,11 +19,39 @@ #include linux/kvm_host.h +#include asm/debug-monitors.h +#include asm/kvm_asm.h #include asm/kvm_arm.h +#include asm/kvm_emulate.h + +/* These are the bits of MDSCR_EL1 we may manipulate */ +#define MDSCR_EL1_DEBUG_MASK (DBG_MDSCR_SS | \ + DBG_MDSCR_KDE | \ + DBG_MDSCR_MDE) static DEFINE_PER_CPU(u32, mdcr_el2); /** + * save/restore_guest_debug_regs + * + * For some debug operations we need to tweak some guest registers. As + * a result we need to save the state of those registers before we + * make those modifications. + * + * Guest access to MDSCR_EL1 is trapped by the hypervisor and handled + * after we have restored the preserved value to the main context. + */ +static void save_guest_debug_regs(struct kvm_vcpu *vcpu) +{ + vcpu-arch.guest_debug_preserved.mdscr_el1 = vcpu_sys_reg(vcpu, MDSCR_EL1); +} + +static void restore_guest_debug_regs(struct kvm_vcpu *vcpu) +{ + vcpu_sys_reg(vcpu, MDSCR_EL1) = vcpu-arch.guest_debug_preserved.mdscr_el1; +} + +/** * kvm_arm_init_debug - grab what we need for debug * * Currently the sole task of this function is to retrieve the initial @@ -38,7 +66,6 @@ void kvm_arm_init_debug(void) __this_cpu_write(mdcr_el2, kvm_call_hyp(__kvm_get_mdcr_el2)); } - /** * kvm_arm_setup_debug - set up debug related stuff * @@ -73,12 +100,45 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu) if (trap_debug) vcpu-arch.mdcr_el2 |= MDCR_EL2_TDA; - /* Trap breakpoints? */ - if (vcpu-guest_debug KVM_GUESTDBG_USE_SW_BP) + /* Is Guest debugging in effect? */ + if (vcpu-guest_debug) { + /* Route all software debug exceptions to EL2 */ vcpu-arch.mdcr_el2 |= MDCR_EL2_TDE; + + /* Save guest debug state */ + save_guest_debug_regs(vcpu); + + /* +* Single Step (ARM ARM D2.12.3 The software step state +* machine) +* +* If we are doing Single Step we need to manipulate +* the guest's MDSCR_EL1.SS and PSTATE.SS. Once the +* step has
[PATCH v7 11/11] KVM: arm64: add trace points for guest_debug debug
This includes trace points for: kvm_arch_setup_guest_debug kvm_arch_clear_guest_debug I've also added some generic register setting trace events and also a trace point to dump the array of hardware registers. Signed-off-by: Alex Bennée alex.ben...@linaro.org --- v3 - add trace event for debug access. - remove short trace #define, rename trace events - use __print_array with fixed array instead of own func - rationalise trace points (only one per register changed) - add vcpu ptr to the debug_setup trace - remove :: in prints v4 - u32/u64 split on debug registers - fix for renames - add tracing of traps/set_guest_debug - remove handle_guest_debug trace v5 - minor print fmt fix - rm pstate traces v6 - fix merge conflicts - update control reg tracking to u64 (abi change) v7 - fix merge conflicts from ioctl move - fix other minor merge conflicts - fixes for the re-factored sys_regs code --- arch/arm64/kvm/debug.c| 35 - arch/arm64/kvm/guest.c| 4 ++ arch/arm64/kvm/sys_regs.c | 21 arch/arm64/kvm/trace.h| 123 ++ 4 files changed, 182 insertions(+), 1 deletion(-) diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c index 46b73d7..119107f 100644 --- a/arch/arm64/kvm/debug.c +++ b/arch/arm64/kvm/debug.c @@ -24,6 +24,8 @@ #include asm/kvm_arm.h #include asm/kvm_emulate.h +#include trace.h + /* These are the bits of MDSCR_EL1 we may manipulate */ #define MDSCR_EL1_DEBUG_MASK (DBG_MDSCR_SS | \ DBG_MDSCR_KDE | \ @@ -44,11 +46,17 @@ static DEFINE_PER_CPU(u32, mdcr_el2); static void save_guest_debug_regs(struct kvm_vcpu *vcpu) { vcpu-arch.guest_debug_preserved.mdscr_el1 = vcpu_sys_reg(vcpu, MDSCR_EL1); + + trace_kvm_arm_set_dreg32(Saved MDSCR_EL1, + vcpu-arch.guest_debug_preserved.mdscr_el1); } static void restore_guest_debug_regs(struct kvm_vcpu *vcpu) { vcpu_sys_reg(vcpu, MDSCR_EL1) = vcpu-arch.guest_debug_preserved.mdscr_el1; + + trace_kvm_arm_set_dreg32(Restored MDSCR_EL1, + vcpu_sys_reg(vcpu, MDSCR_EL1)); } /** @@ -99,6 +107,8 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu) { bool trap_debug = !(vcpu-arch.debug_flags KVM_ARM64_DEBUG_DIRTY); + trace_kvm_arm_setup_debug(vcpu, vcpu-guest_debug); + vcpu-arch.mdcr_el2 = __this_cpu_read(mdcr_el2) MDCR_EL2_HPMN_MASK; vcpu-arch.mdcr_el2 |= (MDCR_EL2_TPM | MDCR_EL2_TPMCR | @@ -140,6 +150,8 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu) vcpu_sys_reg(vcpu, MDSCR_EL1) = ~DBG_MDSCR_SS; } + trace_kvm_arm_set_dreg32(SPSR_EL2, *vcpu_cpsr(vcpu)); + /* * HW Breakpoints and watchpoints * @@ -156,6 +168,14 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu) vcpu-arch.debug_ptr = vcpu-arch.external_debug_state; vcpu-arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY; trap_debug = true; + + trace_kvm_arm_set_regset(BKPTS, get_num_brps(), + vcpu-arch.debug_ptr-dbg_bcr[0], + vcpu-arch.debug_ptr-dbg_bvr[0]); + + trace_kvm_arm_set_regset(WAPTS, get_num_wrps(), + vcpu-arch.debug_ptr-dbg_wcr[0], + vcpu-arch.debug_ptr-dbg_wvr[0]); } } @@ -165,10 +185,15 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu) /* Trap debug register access */ if (trap_debug) vcpu-arch.mdcr_el2 |= MDCR_EL2_TDA; + + trace_kvm_arm_set_dreg32(MDCR_EL2, vcpu-arch.mdcr_el2); + trace_kvm_arm_set_dreg32(MDSCR_EL1, vcpu_sys_reg(vcpu, MDSCR_EL1)); } void kvm_arm_clear_debug(struct kvm_vcpu *vcpu) { + trace_kvm_arm_clear_debug(vcpu-guest_debug); + if (vcpu-guest_debug) { restore_guest_debug_regs(vcpu); @@ -176,8 +201,16 @@ void kvm_arm_clear_debug(struct kvm_vcpu *vcpu) * If we were using HW debug we need to restore the * debug_ptr to the guest debug state. */ - if (vcpu-guest_debug KVM_GUESTDBG_USE_HW) + if (vcpu-guest_debug KVM_GUESTDBG_USE_HW) { kvm_arm_reset_debug_ptr(vcpu); + trace_kvm_arm_set_regset(BKPTS, get_num_brps(), + vcpu-arch.debug_ptr-dbg_bcr[0], + vcpu-arch.debug_ptr-dbg_bvr[0]); + + trace_kvm_arm_set_regset(WAPTS, get_num_wrps(), + vcpu-arch.debug_ptr-dbg_wcr[0