Re: [PATCH] KVM: PPC: fix ONE_REG AltiVec support
Happy new year ! On Wed, 16 Dec 2015 18:24:03 +0100 Greg Kurz <gk...@linux.vnet.ibm.com> wrote: > The get and set operations got exchanged by mistake when moving the > code from book3s.c to powerpc.c. > > Fixes: 3840edc8033ad5b86deee309c1c321ca54257452 > Signed-off-by: Greg Kurz <gk...@linux.vnet.ibm.com> > --- > Ping ? > It's been there for over a year but I guess we want that in 4.4, even > if doesn't break the host kernel. > > arch/powerpc/kvm/powerpc.c | 20 ++-- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > index 6fd2405c7f4a..a3b182dcb823 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -919,21 +919,17 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, > struct kvm_one_reg *reg) > r = -ENXIO; > break; > } > - vcpu->arch.vr.vr[reg->id - KVM_REG_PPC_VR0] = val.vval; > + val.vval = vcpu->arch.vr.vr[reg->id - KVM_REG_PPC_VR0]; > break; > case KVM_REG_PPC_VSCR: > if (!cpu_has_feature(CPU_FTR_ALTIVEC)) { > r = -ENXIO; > break; > } > - vcpu->arch.vr.vscr.u[3] = set_reg_val(reg->id, val); > + val = get_reg_val(reg->id, vcpu->arch.vr.vscr.u[3]); > break; > case KVM_REG_PPC_VRSAVE: > - if (!cpu_has_feature(CPU_FTR_ALTIVEC)) { > - r = -ENXIO; > - break; > - } > - vcpu->arch.vrsave = set_reg_val(reg->id, val); > + val = get_reg_val(reg->id, vcpu->arch.vrsave); > break; > #endif /* CONFIG_ALTIVEC */ > default: > @@ -974,17 +970,21 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, > struct kvm_one_reg *reg) > r = -ENXIO; > break; > } > - val.vval = vcpu->arch.vr.vr[reg->id - KVM_REG_PPC_VR0]; > + vcpu->arch.vr.vr[reg->id - KVM_REG_PPC_VR0] = val.vval; > break; > case KVM_REG_PPC_VSCR: > if (!cpu_has_feature(CPU_FTR_ALTIVEC)) { > r = -ENXIO; > break; > } > - val = get_reg_val(reg->id, vcpu->arch.vr.vscr.u[3]); > + vcpu->arch.vr.vscr.u[3] = set_reg_val(reg->id, val); > break; > case KVM_REG_PPC_VRSAVE: > - val = get_reg_val(reg->id, vcpu->arch.vrsave); > + if (!cpu_has_feature(CPU_FTR_ALTIVEC)) { > + r = -ENXIO; > + break; > + } > + vcpu->arch.vrsave = set_reg_val(reg->id, val); > break; > #endif /* CONFIG_ALTIVEC */ > default: > > -- > 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 > -- 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] KVM: PPC: fix ONE_REG AltiVec support
Happy new year ! On Wed, 16 Dec 2015 18:24:03 +0100 Greg Kurz <gk...@linux.vnet.ibm.com> wrote: > The get and set operations got exchanged by mistake when moving the > code from book3s.c to powerpc.c. > > Fixes: 3840edc8033ad5b86deee309c1c321ca54257452 > Signed-off-by: Greg Kurz <gk...@linux.vnet.ibm.com> > --- > Ping ? > It's been there for over a year but I guess we want that in 4.4, even > if doesn't break the host kernel. > > arch/powerpc/kvm/powerpc.c | 20 ++-- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > index 6fd2405c7f4a..a3b182dcb823 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -919,21 +919,17 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, > struct kvm_one_reg *reg) > r = -ENXIO; > break; > } > - vcpu->arch.vr.vr[reg->id - KVM_REG_PPC_VR0] = val.vval; > + val.vval = vcpu->arch.vr.vr[reg->id - KVM_REG_PPC_VR0]; > break; > case KVM_REG_PPC_VSCR: > if (!cpu_has_feature(CPU_FTR_ALTIVEC)) { > r = -ENXIO; > break; > } > - vcpu->arch.vr.vscr.u[3] = set_reg_val(reg->id, val); > + val = get_reg_val(reg->id, vcpu->arch.vr.vscr.u[3]); > break; > case KVM_REG_PPC_VRSAVE: > - if (!cpu_has_feature(CPU_FTR_ALTIVEC)) { > - r = -ENXIO; > - break; > - } > - vcpu->arch.vrsave = set_reg_val(reg->id, val); > + val = get_reg_val(reg->id, vcpu->arch.vrsave); > break; > #endif /* CONFIG_ALTIVEC */ > default: > @@ -974,17 +970,21 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, > struct kvm_one_reg *reg) > r = -ENXIO; > break; > } > - val.vval = vcpu->arch.vr.vr[reg->id - KVM_REG_PPC_VR0]; > + vcpu->arch.vr.vr[reg->id - KVM_REG_PPC_VR0] = val.vval; > break; > case KVM_REG_PPC_VSCR: > if (!cpu_has_feature(CPU_FTR_ALTIVEC)) { > r = -ENXIO; > break; > } > - val = get_reg_val(reg->id, vcpu->arch.vr.vscr.u[3]); > + vcpu->arch.vr.vscr.u[3] = set_reg_val(reg->id, val); > break; > case KVM_REG_PPC_VRSAVE: > - val = get_reg_val(reg->id, vcpu->arch.vrsave); > + if (!cpu_has_feature(CPU_FTR_ALTIVEC)) { > + r = -ENXIO; > + break; > + } > + vcpu->arch.vrsave = set_reg_val(reg->id, val); > break; > #endif /* CONFIG_ALTIVEC */ > default: > > -- > 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 > -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] KVM: PPC: fix ONE_REG AltiVec support
The get and set operations got exchanged by mistake when moving the code from book3s.c to powerpc.c. Fixes: 3840edc8033ad5b86deee309c1c321ca54257452 Signed-off-by: Greg Kurz <gk...@linux.vnet.ibm.com> --- It's been there for over a year but I guess we want that in 4.4, even if doesn't break the host kernel. arch/powerpc/kvm/powerpc.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index 6fd2405c7f4a..a3b182dcb823 100644 --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -919,21 +919,17 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg) r = -ENXIO; break; } - vcpu->arch.vr.vr[reg->id - KVM_REG_PPC_VR0] = val.vval; + val.vval = vcpu->arch.vr.vr[reg->id - KVM_REG_PPC_VR0]; break; case KVM_REG_PPC_VSCR: if (!cpu_has_feature(CPU_FTR_ALTIVEC)) { r = -ENXIO; break; } - vcpu->arch.vr.vscr.u[3] = set_reg_val(reg->id, val); + val = get_reg_val(reg->id, vcpu->arch.vr.vscr.u[3]); break; case KVM_REG_PPC_VRSAVE: - if (!cpu_has_feature(CPU_FTR_ALTIVEC)) { - r = -ENXIO; - break; - } - vcpu->arch.vrsave = set_reg_val(reg->id, val); + val = get_reg_val(reg->id, vcpu->arch.vrsave); break; #endif /* CONFIG_ALTIVEC */ default: @@ -974,17 +970,21 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg) r = -ENXIO; break; } - val.vval = vcpu->arch.vr.vr[reg->id - KVM_REG_PPC_VR0]; + vcpu->arch.vr.vr[reg->id - KVM_REG_PPC_VR0] = val.vval; break; case KVM_REG_PPC_VSCR: if (!cpu_has_feature(CPU_FTR_ALTIVEC)) { r = -ENXIO; break; } - val = get_reg_val(reg->id, vcpu->arch.vr.vscr.u[3]); + vcpu->arch.vr.vscr.u[3] = set_reg_val(reg->id, val); break; case KVM_REG_PPC_VRSAVE: - val = get_reg_val(reg->id, vcpu->arch.vrsave); + if (!cpu_has_feature(CPU_FTR_ALTIVEC)) { + r = -ENXIO; + break; + } + vcpu->arch.vrsave = set_reg_val(reg->id, val); break; #endif /* CONFIG_ALTIVEC */ default: -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] KVM: PPC: fix ONE_REG AltiVec support
The get and set operations got exchanged by mistake when moving the code from book3s.c to powerpc.c. Fixes: 3840edc8033ad5b86deee309c1c321ca54257452 Signed-off-by: Greg Kurz <gk...@linux.vnet.ibm.com> --- It's been there for over a year but I guess we want that in 4.4, even if doesn't break the host kernel. arch/powerpc/kvm/powerpc.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index 6fd2405c7f4a..a3b182dcb823 100644 --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -919,21 +919,17 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg) r = -ENXIO; break; } - vcpu->arch.vr.vr[reg->id - KVM_REG_PPC_VR0] = val.vval; + val.vval = vcpu->arch.vr.vr[reg->id - KVM_REG_PPC_VR0]; break; case KVM_REG_PPC_VSCR: if (!cpu_has_feature(CPU_FTR_ALTIVEC)) { r = -ENXIO; break; } - vcpu->arch.vr.vscr.u[3] = set_reg_val(reg->id, val); + val = get_reg_val(reg->id, vcpu->arch.vr.vscr.u[3]); break; case KVM_REG_PPC_VRSAVE: - if (!cpu_has_feature(CPU_FTR_ALTIVEC)) { - r = -ENXIO; - break; - } - vcpu->arch.vrsave = set_reg_val(reg->id, val); + val = get_reg_val(reg->id, vcpu->arch.vrsave); break; #endif /* CONFIG_ALTIVEC */ default: @@ -974,17 +970,21 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg) r = -ENXIO; break; } - val.vval = vcpu->arch.vr.vr[reg->id - KVM_REG_PPC_VR0]; + vcpu->arch.vr.vr[reg->id - KVM_REG_PPC_VR0] = val.vval; break; case KVM_REG_PPC_VSCR: if (!cpu_has_feature(CPU_FTR_ALTIVEC)) { r = -ENXIO; break; } - val = get_reg_val(reg->id, vcpu->arch.vr.vscr.u[3]); + vcpu->arch.vr.vscr.u[3] = set_reg_val(reg->id, val); break; case KVM_REG_PPC_VRSAVE: - val = get_reg_val(reg->id, vcpu->arch.vrsave); + if (!cpu_has_feature(CPU_FTR_ALTIVEC)) { + r = -ENXIO; + break; + } + vcpu->arch.vrsave = set_reg_val(reg->id, val); break; #endif /* CONFIG_ALTIVEC */ default: -- 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] vhost: move is_le setup to the backend
On Thu, 12 Nov 2015 15:28:19 +0100 Greg Kurz <gk...@linux.vnet.ibm.com> wrote: > On Thu, 12 Nov 2015 15:46:30 +0200 > "Michael S. Tsirkin" <m...@redhat.com> wrote: > > > On Fri, Oct 30, 2015 at 12:42:35PM +0100, Greg Kurz wrote: > > > The vq->is_le field is used to fix endianness when accessing the vring via > > > the cpu_to_vhost16() and vhost16_to_cpu() helpers in the following cases: > > > > > > 1) host is big endian and device is modern virtio > > > > > > 2) host has cross-endian support and device is legacy virtio with a > > > different > > >endianness than the host > > > > > > Both cases rely on the VHOST_SET_FEATURES ioctl, but 2) also needs the > > > VHOST_SET_VRING_ENDIAN ioctl to be called by userspace. Since vq->is_le > > > is only needed when the backend is active, it was decided to set it at > > > backend start. > > > > > > This is currently done in vhost_init_used()->vhost_init_is_le() but it > > > obfuscates the core vhost code. This patch moves the is_le setup to a > > > dedicated function that is called from the backend code. > > > > > > Note vhost_net is the only backend that can pass vq->private_data == NULL > > > to > > > vhost_init_used(), hence the "if (sock)" branch. > > > > > > No behaviour change. > > > > > > Signed-off-by: Greg Kurz <gk...@linux.vnet.ibm.com> > > > > I plan to look at this next week, busy with QEMU 2.5 now. > > > > I don't have any deadline for this since this is only a cleanup tentative. > > Thanks. > ... but ping anyway since it was a month ago :) Cheers. -- Greg > > > --- > > > drivers/vhost/net.c |6 ++ > > > drivers/vhost/scsi.c |3 +++ > > > drivers/vhost/test.c |2 ++ > > > drivers/vhost/vhost.c | 12 +++- > > > drivers/vhost/vhost.h |1 + > > > 5 files changed, 19 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c > > > index 9eda69e40678..d6319cb2664c 100644 > > > --- a/drivers/vhost/net.c > > > +++ b/drivers/vhost/net.c > > > @@ -917,6 +917,12 @@ static long vhost_net_set_backend(struct vhost_net > > > *n, unsigned index, int fd) > > > > > > vhost_net_disable_vq(n, vq); > > > vq->private_data = sock; > > > + > > > + if (sock) > > > + vhost_set_is_le(vq); > > > + else > > > + vq->is_le = virtio_legacy_is_little_endian(); > > > + > > > r = vhost_init_used(vq); > > > if (r) > > > goto err_used; > > > diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c > > > index e25a23692822..e2644a301fa5 100644 > > > --- a/drivers/vhost/scsi.c > > > +++ b/drivers/vhost/scsi.c > > > @@ -1276,6 +1276,9 @@ vhost_scsi_set_endpoint(struct vhost_scsi *vs, > > > vq = >vqs[i].vq; > > > mutex_lock(>mutex); > > > vq->private_data = vs_tpg; > > > + > > > + vhost_set_is_le(vq); > > > + > > > vhost_init_used(vq); > > > mutex_unlock(>mutex); > > > } > > > diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c > > > index f2882ac98726..b1c7df502211 100644 > > > --- a/drivers/vhost/test.c > > > +++ b/drivers/vhost/test.c > > > @@ -196,6 +196,8 @@ static long vhost_test_run(struct vhost_test *n, int > > > test) > > > oldpriv = vq->private_data; > > > vq->private_data = priv; > > > > > > + vhost_set_is_le(vq); > > > + > > > r = vhost_init_used(>vqs[index]); > > > > > > mutex_unlock(>mutex); > > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > > > index eec2f11809ff..6be863dcbd13 100644 > > > --- a/drivers/vhost/vhost.c > > > +++ b/drivers/vhost/vhost.c > > > @@ -113,6 +113,12 @@ static void vhost_init_is_le(struct vhost_virtqueue > > > *vq) > > > } > > > #endif /* CONFIG_VHOST_CROSS_ENDIAN_LEGACY */ > > > > > > +void vhost_set_is_le(struct vhost_virtqueue *vq) > > > +{ > > > + vhost_init_is_le(vq); > > > +} > > > +EXPORT_SYMBOL_GPL(vhost_
Re: [PATCH] vhost: move is_le setup to the backend
On Fri, 30 Oct 2015 12:42:35 +0100 Greg Kurz <gk...@linux.vnet.ibm.com> wrote: > The vq->is_le field is used to fix endianness when accessing the vring via > the cpu_to_vhost16() and vhost16_to_cpu() helpers in the following cases: > > 1) host is big endian and device is modern virtio > > 2) host has cross-endian support and device is legacy virtio with a different >endianness than the host > > Both cases rely on the VHOST_SET_FEATURES ioctl, but 2) also needs the > VHOST_SET_VRING_ENDIAN ioctl to be called by userspace. Since vq->is_le > is only needed when the backend is active, it was decided to set it at > backend start. > > This is currently done in vhost_init_used()->vhost_init_is_le() but it > obfuscates the core vhost code. This patch moves the is_le setup to a > dedicated function that is called from the backend code. > > Note vhost_net is the only backend that can pass vq->private_data == NULL to > vhost_init_used(), hence the "if (sock)" branch. > > No behaviour change. > > Signed-off-by: Greg Kurz <gk...@linux.vnet.ibm.com> > --- Ping ? > drivers/vhost/net.c |6 ++ > drivers/vhost/scsi.c |3 +++ > drivers/vhost/test.c |2 ++ > drivers/vhost/vhost.c | 12 +++- > drivers/vhost/vhost.h |1 + > 5 files changed, 19 insertions(+), 5 deletions(-) > > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c > index 9eda69e40678..d6319cb2664c 100644 > --- a/drivers/vhost/net.c > +++ b/drivers/vhost/net.c > @@ -917,6 +917,12 @@ static long vhost_net_set_backend(struct vhost_net *n, > unsigned index, int fd) > > vhost_net_disable_vq(n, vq); > vq->private_data = sock; > + > + if (sock) > + vhost_set_is_le(vq); > + else > + vq->is_le = virtio_legacy_is_little_endian(); > + > r = vhost_init_used(vq); > if (r) > goto err_used; > diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c > index e25a23692822..e2644a301fa5 100644 > --- a/drivers/vhost/scsi.c > +++ b/drivers/vhost/scsi.c > @@ -1276,6 +1276,9 @@ vhost_scsi_set_endpoint(struct vhost_scsi *vs, > vq = >vqs[i].vq; > mutex_lock(>mutex); > vq->private_data = vs_tpg; > + > + vhost_set_is_le(vq); > + > vhost_init_used(vq); > mutex_unlock(>mutex); > } > diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c > index f2882ac98726..b1c7df502211 100644 > --- a/drivers/vhost/test.c > +++ b/drivers/vhost/test.c > @@ -196,6 +196,8 @@ static long vhost_test_run(struct vhost_test *n, int test) > oldpriv = vq->private_data; > vq->private_data = priv; > > + vhost_set_is_le(vq); > + > r = vhost_init_used(>vqs[index]); > > mutex_unlock(>mutex); > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > index eec2f11809ff..6be863dcbd13 100644 > --- a/drivers/vhost/vhost.c > +++ b/drivers/vhost/vhost.c > @@ -113,6 +113,12 @@ static void vhost_init_is_le(struct vhost_virtqueue *vq) > } > #endif /* CONFIG_VHOST_CROSS_ENDIAN_LEGACY */ > > +void vhost_set_is_le(struct vhost_virtqueue *vq) > +{ > + vhost_init_is_le(vq); > +} > +EXPORT_SYMBOL_GPL(vhost_set_is_le); > + > static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh, > poll_table *pt) > { > @@ -1156,12 +1162,8 @@ int vhost_init_used(struct vhost_virtqueue *vq) > { > __virtio16 last_used_idx; > int r; > - if (!vq->private_data) { > - vq->is_le = virtio_legacy_is_little_endian(); > + if (!vq->private_data) > return 0; > - } > - > - vhost_init_is_le(vq); > > r = vhost_update_used_flags(vq); > if (r) > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h > index 4772862b71a7..8a62041959fe 100644 > --- a/drivers/vhost/vhost.h > +++ b/drivers/vhost/vhost.h > @@ -162,6 +162,7 @@ bool vhost_enable_notify(struct vhost_dev *, struct > vhost_virtqueue *); > > int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log, > unsigned int log_num, u64 len); > +void vhost_set_is_le(struct vhost_virtqueue *vq); > > #define vq_err(vq, fmt, ...) do { \ > pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \ > > -- > 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 > -- 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] vhost: move is_le setup to the backend
On Thu, 12 Nov 2015 15:46:30 +0200 "Michael S. Tsirkin" <m...@redhat.com> wrote: > On Fri, Oct 30, 2015 at 12:42:35PM +0100, Greg Kurz wrote: > > The vq->is_le field is used to fix endianness when accessing the vring via > > the cpu_to_vhost16() and vhost16_to_cpu() helpers in the following cases: > > > > 1) host is big endian and device is modern virtio > > > > 2) host has cross-endian support and device is legacy virtio with a > > different > >endianness than the host > > > > Both cases rely on the VHOST_SET_FEATURES ioctl, but 2) also needs the > > VHOST_SET_VRING_ENDIAN ioctl to be called by userspace. Since vq->is_le > > is only needed when the backend is active, it was decided to set it at > > backend start. > > > > This is currently done in vhost_init_used()->vhost_init_is_le() but it > > obfuscates the core vhost code. This patch moves the is_le setup to a > > dedicated function that is called from the backend code. > > > > Note vhost_net is the only backend that can pass vq->private_data == NULL to > > vhost_init_used(), hence the "if (sock)" branch. > > > > No behaviour change. > > > > Signed-off-by: Greg Kurz <gk...@linux.vnet.ibm.com> > > I plan to look at this next week, busy with QEMU 2.5 now. > I don't have any deadline for this since this is only a cleanup tentative. Thanks. > > --- > > drivers/vhost/net.c |6 ++ > > drivers/vhost/scsi.c |3 +++ > > drivers/vhost/test.c |2 ++ > > drivers/vhost/vhost.c | 12 +++- > > drivers/vhost/vhost.h |1 + > > 5 files changed, 19 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c > > index 9eda69e40678..d6319cb2664c 100644 > > --- a/drivers/vhost/net.c > > +++ b/drivers/vhost/net.c > > @@ -917,6 +917,12 @@ static long vhost_net_set_backend(struct vhost_net *n, > > unsigned index, int fd) > > > > vhost_net_disable_vq(n, vq); > > vq->private_data = sock; > > + > > + if (sock) > > + vhost_set_is_le(vq); > > + else > > + vq->is_le = virtio_legacy_is_little_endian(); > > + > > r = vhost_init_used(vq); > > if (r) > > goto err_used; > > diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c > > index e25a23692822..e2644a301fa5 100644 > > --- a/drivers/vhost/scsi.c > > +++ b/drivers/vhost/scsi.c > > @@ -1276,6 +1276,9 @@ vhost_scsi_set_endpoint(struct vhost_scsi *vs, > > vq = >vqs[i].vq; > > mutex_lock(>mutex); > > vq->private_data = vs_tpg; > > + > > + vhost_set_is_le(vq); > > + > > vhost_init_used(vq); > > mutex_unlock(>mutex); > > } > > diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c > > index f2882ac98726..b1c7df502211 100644 > > --- a/drivers/vhost/test.c > > +++ b/drivers/vhost/test.c > > @@ -196,6 +196,8 @@ static long vhost_test_run(struct vhost_test *n, int > > test) > > oldpriv = vq->private_data; > > vq->private_data = priv; > > > > + vhost_set_is_le(vq); > > + > > r = vhost_init_used(>vqs[index]); > > > > mutex_unlock(>mutex); > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > > index eec2f11809ff..6be863dcbd13 100644 > > --- a/drivers/vhost/vhost.c > > +++ b/drivers/vhost/vhost.c > > @@ -113,6 +113,12 @@ static void vhost_init_is_le(struct vhost_virtqueue > > *vq) > > } > > #endif /* CONFIG_VHOST_CROSS_ENDIAN_LEGACY */ > > > > +void vhost_set_is_le(struct vhost_virtqueue *vq) > > +{ > > + vhost_init_is_le(vq); > > +} > > +EXPORT_SYMBOL_GPL(vhost_set_is_le); > > + > > static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh, > > poll_table *pt) > > { > > @@ -1156,12 +1162,8 @@ int vhost_init_used(struct vhost_virtqueue *vq) > > { > > __virtio16 last_used_idx; > > int r; > > - if (!vq->private_data) { > > - vq->is_le = virtio_legacy_is_little_endian(); > > + if (!vq->private_data) > > return 0; > > - } > > - > > - vhost_init_is_le(vq); > > > > r = vhost_update_used_flags(vq); > >
[PATCH] vhost: move is_le setup to the backend
The vq->is_le field is used to fix endianness when accessing the vring via the cpu_to_vhost16() and vhost16_to_cpu() helpers in the following cases: 1) host is big endian and device is modern virtio 2) host has cross-endian support and device is legacy virtio with a different endianness than the host Both cases rely on the VHOST_SET_FEATURES ioctl, but 2) also needs the VHOST_SET_VRING_ENDIAN ioctl to be called by userspace. Since vq->is_le is only needed when the backend is active, it was decided to set it at backend start. This is currently done in vhost_init_used()->vhost_init_is_le() but it obfuscates the core vhost code. This patch moves the is_le setup to a dedicated function that is called from the backend code. Note vhost_net is the only backend that can pass vq->private_data == NULL to vhost_init_used(), hence the "if (sock)" branch. No behaviour change. Signed-off-by: Greg Kurz <gk...@linux.vnet.ibm.com> --- drivers/vhost/net.c |6 ++ drivers/vhost/scsi.c |3 +++ drivers/vhost/test.c |2 ++ drivers/vhost/vhost.c | 12 +++- drivers/vhost/vhost.h |1 + 5 files changed, 19 insertions(+), 5 deletions(-) diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c index 9eda69e40678..d6319cb2664c 100644 --- a/drivers/vhost/net.c +++ b/drivers/vhost/net.c @@ -917,6 +917,12 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd) vhost_net_disable_vq(n, vq); vq->private_data = sock; + + if (sock) + vhost_set_is_le(vq); + else + vq->is_le = virtio_legacy_is_little_endian(); + r = vhost_init_used(vq); if (r) goto err_used; diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c index e25a23692822..e2644a301fa5 100644 --- a/drivers/vhost/scsi.c +++ b/drivers/vhost/scsi.c @@ -1276,6 +1276,9 @@ vhost_scsi_set_endpoint(struct vhost_scsi *vs, vq = >vqs[i].vq; mutex_lock(>mutex); vq->private_data = vs_tpg; + + vhost_set_is_le(vq); + vhost_init_used(vq); mutex_unlock(>mutex); } diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c index f2882ac98726..b1c7df502211 100644 --- a/drivers/vhost/test.c +++ b/drivers/vhost/test.c @@ -196,6 +196,8 @@ static long vhost_test_run(struct vhost_test *n, int test) oldpriv = vq->private_data; vq->private_data = priv; + vhost_set_is_le(vq); + r = vhost_init_used(>vqs[index]); mutex_unlock(>mutex); diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index eec2f11809ff..6be863dcbd13 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -113,6 +113,12 @@ static void vhost_init_is_le(struct vhost_virtqueue *vq) } #endif /* CONFIG_VHOST_CROSS_ENDIAN_LEGACY */ +void vhost_set_is_le(struct vhost_virtqueue *vq) +{ + vhost_init_is_le(vq); +} +EXPORT_SYMBOL_GPL(vhost_set_is_le); + static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh, poll_table *pt) { @@ -1156,12 +1162,8 @@ int vhost_init_used(struct vhost_virtqueue *vq) { __virtio16 last_used_idx; int r; - if (!vq->private_data) { - vq->is_le = virtio_legacy_is_little_endian(); + if (!vq->private_data) return 0; - } - - vhost_init_is_le(vq); r = vhost_update_used_flags(vq); if (r) diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h index 4772862b71a7..8a62041959fe 100644 --- a/drivers/vhost/vhost.h +++ b/drivers/vhost/vhost.h @@ -162,6 +162,7 @@ bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *); int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log, unsigned int log_num, u64 len); +void vhost_set_is_le(struct vhost_virtqueue *vq); #define vq_err(vq, fmt, ...) do { \ pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \ -- 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] vhost: fix performance on LE hosts
On Tue, 27 Oct 2015 11:37:39 +0200 "Michael S. Tsirkin" <m...@redhat.com> wrote: > commit 2751c9882b947292fcfb084c4f604e01724af804 ("vhost: cross-endian > support for legacy devices") introduced a minor regression: even with > cross-endian disabled, and even on LE host, vhost_is_little_endian is > checking is_le flag so there's always a branch. > > To fix, simply check virtio_legacy_is_little_endian first. > > Cc: Greg Kurz <gk...@linux.vnet.ibm.com> > Signed-off-by: Michael S. Tsirkin <m...@redhat.com> Oops my bad for this oversight... Reviewed-by: Greg Kurz <gk...@linux.vnet.ibm.com> > --- > drivers/vhost/vhost.h | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h > index 4772862..d3f7674 100644 > --- a/drivers/vhost/vhost.h > +++ b/drivers/vhost/vhost.h > @@ -183,10 +183,17 @@ static inline bool vhost_has_feature(struct > vhost_virtqueue *vq, int bit) > return vq->acked_features & (1ULL << bit); > } > > +#ifdef CONFIG_VHOST_CROSS_ENDIAN_LEGACY > static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq) > { > return vq->is_le; > } > +#else > +static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq) > +{ > + return virtio_legacy_is_little_endian() || vq->is_le; > +} > +#endif > > /* Memory accessors */ > static inline u16 vhost16_to_cpu(struct vhost_virtqueue *vq, __virtio16 val) -- 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] Fix AF_PACKET ABI breakage in 4.2
On Thu, 24 Sep 2015 12:50:59 +0300 "Michael S. Tsirkin" <m...@redhat.com> wrote: > On Thu, Sep 24, 2015 at 09:25:45AM +0200, Greg Kurz wrote: > > On Wed, 23 Sep 2015 19:45:08 +0100 > > David Woodhouse <dw...@infradead.org> wrote: > > > > > Commit 7d82410950aa ("virtio: add explicit big-endian support to memory > > > accessors") accidentally changed the virtio_net header used by > > > AF_PACKET with PACKET_VNET_HDR from host-endian to big-endian. > > > > > > > Hi David, > > > > Oops my bad... I obviously overlooked this one when adding cross-endian > > support. > > > > > Since virtio_legacy_is_little_endian() is a very long identifier, > > > define a VIO_LE macro and use that throughout the code instead of the > > > > VIO usually refers to virtual IO adapters for the PowerPC pSeries platform. > > > > > hard-coded 'false' for little-endian. > > > > > > > What about introducing dedicated accessors as it is done in many other > > locations where we do virtio byteswap ? Something like: > > > > static inline bool packet_is_little_endian(void) > > { > > return virtio_legacy_is_little_endian(); > > } > > > > static inline u16 packet16_to_cpu(__virtio16 val) > > { > > return __virtio16_to_cpu(packet_is_little_endian(), val); > > } > > > > static inline __virtio16 cpu_to_packet16(u16 val) > > { > > return __cpu_to_virtio16(packet_is_little_endian(), val); > > } > > > > It results in prettier code IMHO. Have a look at drivers/net/tun.c or > > drivers/vhost/vhost.c. > > > > > This restores the ABI to match 4.1 and earlier kernels, and makes my > > > test program work again. > > > > > > > BTW, there is still work to do if we want to support cross-endian legacy or > > virtio 1 on a big endian arch... > > > > Cheers. > > > > -- > > Greg > > It seems the API that we have is a confusing one. > > virtio endian-ness is either native or little, depending on a flag, so > __virtio16_to_cpu seems to mean "either native to cpu or little to cpu > depending on flag". > > It used to be like that, but not anymore. > > This leads to all kind of bugs. > > For example, I have only now realized vhost_is_little_endian isn't a > constant on LE hosts if cross-endian support is not compiled. > > I think we need to fix it, but also think about a better API. > Your original API is good and works well for all the callers that don't care for cross-endian support. I guess we'd rather move the cross-endian burden to the few callers who need it, i.e. tun, macvtap and vhost when cross-endian is compiled. More or less like this: /* Original API : either little-endian or native */ static inline u16 __virtio16_to_cpu(bool little_endian, __virtio16 val) { if (little_endian) return le16_to_cpu((__force __le16)val); else return (__force u16)val; } #ifdef CONFIG_VHOST_CROSS_ENDIAN_LEGACY static inline u16 vhost16_to_cpu(struct vhost_virtqueue *vq, __virtio16 val) { /* little-endian because of virtio 1 */ if (vhost_has_feature(vq, VIRTIO_F_VERSION_1)) return __virtio16_to_cpu(true, val); #ifdef __LITTLE_ENDIAN /* native little-endian */ return (__force u16)val; #else /* native big-endian */ return be16_to_cpu((__force __be16)val); #endif } #else static inline u16 vhost16_to_cpu(struct vhost_virtqueue *vq, __virtio16 val) { #ifdef __LITTLE_ENDIAN /* fast path for little-endian host */ return __virtio16_to_cpu(true, val); #else /* slow path for big-endian host */ return __virtio16_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); #endif } #endif Does it make sense ? > > > > Signed-off-by: David Woodhouse <david.woodho...@intel.com> > > > --- > > > On Wed, 2015-09-23 at 11:09 -0700, David Miller wrote: > > > > > +#define VIO_LE virtio_legacy_is_little_endian() > > > > > > > > When you define a shorthand macro, the defines to a function call, > > > > make the macro have parenthesis too. > > > > > > In which case I suppose it also wants to be lower-case. Although > > > "function call" is a bit strong since it's effectively just a constant. > > > I'm still wondering if it'd be nicer just to use (__force u16) instead. > > > > > > diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c > > > index 7b8e39a..aa4b15c 100644 > > > --- a
Re: [PATCH v2] Fix AF_PACKET ABI breakage in 4.2
On Wed, 23 Sep 2015 19:45:08 +0100 David Woodhousewrote: > Commit 7d82410950aa ("virtio: add explicit big-endian support to memory > accessors") accidentally changed the virtio_net header used by > AF_PACKET with PACKET_VNET_HDR from host-endian to big-endian. > Hi David, Oops my bad... I obviously overlooked this one when adding cross-endian support. > Since virtio_legacy_is_little_endian() is a very long identifier, > define a VIO_LE macro and use that throughout the code instead of the VIO usually refers to virtual IO adapters for the PowerPC pSeries platform. > hard-coded 'false' for little-endian. > What about introducing dedicated accessors as it is done in many other locations where we do virtio byteswap ? Something like: static inline bool packet_is_little_endian(void) { return virtio_legacy_is_little_endian(); } static inline u16 packet16_to_cpu(__virtio16 val) { return __virtio16_to_cpu(packet_is_little_endian(), val); } static inline __virtio16 cpu_to_packet16(u16 val) { return __cpu_to_virtio16(packet_is_little_endian(), val); } It results in prettier code IMHO. Have a look at drivers/net/tun.c or drivers/vhost/vhost.c. > This restores the ABI to match 4.1 and earlier kernels, and makes my > test program work again. > BTW, there is still work to do if we want to support cross-endian legacy or virtio 1 on a big endian arch... Cheers. -- Greg > Signed-off-by: David Woodhouse > --- > On Wed, 2015-09-23 at 11:09 -0700, David Miller wrote: > > > +#define VIO_LE virtio_legacy_is_little_endian() > > > > When you define a shorthand macro, the defines to a function call, > > make the macro have parenthesis too. > > In which case I suppose it also wants to be lower-case. Although > "function call" is a bit strong since it's effectively just a constant. > I'm still wondering if it'd be nicer just to use (__force u16) instead. > > diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c > index 7b8e39a..aa4b15c 100644 > --- a/net/packet/af_packet.c > +++ b/net/packet/af_packet.c > @@ -230,6 +230,8 @@ struct packet_skb_cb { > } sa; > }; > > +#define vio_le() virtio_legacy_is_little_endian() > + > #define PACKET_SKB_CB(__skb) ((struct packet_skb_cb *)((__skb)->cb)) > > #define GET_PBDQC_FROM_RB(x) ((struct tpacket_kbdq_core *)(&(x)->prb_bdqc)) > @@ -2680,15 +2682,15 @@ static int packet_snd(struct socket *sock, struct > msghdr *msg, size_t len) > goto out_unlock; > > if ((vnet_hdr.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) && > - (__virtio16_to_cpu(false, vnet_hdr.csum_start) + > - __virtio16_to_cpu(false, vnet_hdr.csum_offset) + 2 > > - __virtio16_to_cpu(false, vnet_hdr.hdr_len))) > - vnet_hdr.hdr_len = __cpu_to_virtio16(false, > - __virtio16_to_cpu(false, vnet_hdr.csum_start) + > - __virtio16_to_cpu(false, vnet_hdr.csum_offset) > + 2); > + (__virtio16_to_cpu(vio_le(), vnet_hdr.csum_start) + > + __virtio16_to_cpu(vio_le(), vnet_hdr.csum_offset) + 2 > > + __virtio16_to_cpu(vio_le(), vnet_hdr.hdr_len))) > + vnet_hdr.hdr_len = __cpu_to_virtio16(vio_le(), > + __virtio16_to_cpu(vio_le(), > vnet_hdr.csum_start) + > + __virtio16_to_cpu(vio_le(), > vnet_hdr.csum_offset) + 2); > > err = -EINVAL; > - if (__virtio16_to_cpu(false, vnet_hdr.hdr_len) > len) > + if (__virtio16_to_cpu(vio_le(), vnet_hdr.hdr_len) > len) > goto out_unlock; > > if (vnet_hdr.gso_type != VIRTIO_NET_HDR_GSO_NONE) { > @@ -2731,7 +2733,7 @@ static int packet_snd(struct socket *sock, struct > msghdr *msg, size_t len) > hlen = LL_RESERVED_SPACE(dev); > tlen = dev->needed_tailroom; > skb = packet_alloc_skb(sk, hlen + tlen, hlen, len, > -__virtio16_to_cpu(false, vnet_hdr.hdr_len), > +__virtio16_to_cpu(vio_le(), vnet_hdr.hdr_len), > msg->msg_flags & MSG_DONTWAIT, ); > if (skb == NULL) > goto out_unlock; > @@ -2778,8 +2780,8 @@ static int packet_snd(struct socket *sock, struct > msghdr *msg, size_t len) > > if (po->has_vnet_hdr) { > if (vnet_hdr.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) { > - u16 s = __virtio16_to_cpu(false, vnet_hdr.csum_start); > - u16 o = __virtio16_to_cpu(false, vnet_hdr.csum_offset); > + u16 s = __virtio16_to_cpu(vio_le(), > vnet_hdr.csum_start); > + u16 o = __virtio16_to_cpu(vio_le(), > vnet_hdr.csum_offset); > if (!skb_partial_csum_set(skb, s, o)) { > err = -EINVAL; >
Re: [PATCH v4] ppc/spapr: Implement H_RANDOM hypercall in QEMU
On Mon, 21 Sep 2015 12:10:00 +1000 David Gibson <da...@gibson.dropbear.id.au> wrote: > On Fri, Sep 18, 2015 at 11:05:52AM +0200, Greg Kurz wrote: > > On Thu, 17 Sep 2015 10:49:41 +0200 > > Thomas Huth <th...@redhat.com> wrote: > > > > > The PAPR interface defines a hypercall to pass high-quality > > > hardware generated random numbers to guests. Recent kernels can > > > already provide this hypercall to the guest if the right hardware > > > random number generator is available. But in case the user wants > > > to use another source like EGD, or QEMU is running with an older > > > kernel, we should also have this call in QEMU, so that guests that > > > do not support virtio-rng yet can get good random numbers, too. > > > > > > This patch now adds a new pseudo-device to QEMU that either > > > directly provides this hypercall to the guest or is able to > > > enable the in-kernel hypercall if available. The in-kernel > > > hypercall can be enabled with the use-kvm property, e.g.: > > > > > > qemu-system-ppc64 -device spapr-rng,use-kvm=true > > > > > > For handling the hypercall in QEMU instead, a "RngBackend" is > > > required since the hypercall should provide "good" random data > > > instead of pseudo-random (like from a "simple" library function > > > like rand() or g_random_int()). Since there are multiple RngBackends > > > available, the user must select an appropriate back-end via the > > > "rng" property of the device, e.g.: > > > > > > qemu-system-ppc64 -object rng-random,filename=/dev/hwrng,id=gid0 \ > > >-device spapr-rng,rng=gid0 ... > > > > > > See http://wiki.qemu-project.org/Features-Done/VirtIORNG for > > > other example of specifying RngBackends. > > > > > > Signed-off-by: Thomas Huth <th...@redhat.com> > > > --- > > > > It is a good thing that the user can choose between in-kernel and backend, > > and this patch does the work. > > > > This being said, I am not sure about the use case where a user has a hwrng > > capable platform and wants to run guests without any hwrng support at all is > > an appropriate default behavior... I guess we will find more users that want > > in-kernel being the default if it is available. > > > > The patch below modifies yours to do just this: the pseudo-device is only > > created if hwrng is present and not already created. > > I have mixed feelings about this. On the one hand, I agree that it > would be nice to allow H_RANDOM support by default. On the other hand > the patch below leaves no way to turn it off for testing purposes. It This could be handled with a new spapr property, say 'use-hwrng', defaulting to true. > also adds another place where the guest hardware depends on the host > configuration, which adds to the already substantial mess of ensuring > that source and destination hardware configuration matches for > migration. > Yeah, describing the guest hw is really essential for migration... this is best addressed at the libvirt level with a full XML description of the machine... but FWIW if we are talking about running pseries on a POWER8 or newer host, I am not aware about "hwrng-less" boards... but I am probably missing something :) Back to Thomas' patch, it does the job and brings H_RANDOM, which is currently missing. Acked-by: Greg Kurz <gk...@linux.vnet.ibm.com> I could test both use-kvm and backend flavors (including migration). Tested-by: Greg Kurz <gk...@linux.vnet.ibm.com> Thanks. -- Greg pgpUFFPbPHOji.pgp Description: OpenPGP digital signature
Re: [PATCH v4] ppc/spapr: Implement H_RANDOM hypercall in QEMU
On Thu, 17 Sep 2015 10:49:41 +0200 Thomas Huthwrote: > The PAPR interface defines a hypercall to pass high-quality > hardware generated random numbers to guests. Recent kernels can > already provide this hypercall to the guest if the right hardware > random number generator is available. But in case the user wants > to use another source like EGD, or QEMU is running with an older > kernel, we should also have this call in QEMU, so that guests that > do not support virtio-rng yet can get good random numbers, too. > > This patch now adds a new pseudo-device to QEMU that either > directly provides this hypercall to the guest or is able to > enable the in-kernel hypercall if available. The in-kernel > hypercall can be enabled with the use-kvm property, e.g.: > > qemu-system-ppc64 -device spapr-rng,use-kvm=true > > For handling the hypercall in QEMU instead, a "RngBackend" is > required since the hypercall should provide "good" random data > instead of pseudo-random (like from a "simple" library function > like rand() or g_random_int()). Since there are multiple RngBackends > available, the user must select an appropriate back-end via the > "rng" property of the device, e.g.: > > qemu-system-ppc64 -object rng-random,filename=/dev/hwrng,id=gid0 \ >-device spapr-rng,rng=gid0 ... > > See http://wiki.qemu-project.org/Features-Done/VirtIORNG for > other example of specifying RngBackends. > > Signed-off-by: Thomas Huth > --- It is a good thing that the user can choose between in-kernel and backend, and this patch does the work. This being said, I am not sure about the use case where a user has a hwrng capable platform and wants to run guests without any hwrng support at all is an appropriate default behavior... I guess we will find more users that want in-kernel being the default if it is available. The patch below modifies yours to do just this: the pseudo-device is only created if hwrng is present and not already created. diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 240fab72e7af..4b92efed2ef3 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -939,6 +939,14 @@ static int spapr_check_htab_fd(sPAPRMachineState *spapr) return rc; } +static void spapr_rng_create(void) +{ +Object *rng = object_new(TYPE_SPAPR_RNG); + +object_property_set_bool(rng, true, "use-kvm", _abort); +object_property_set_bool(rng, true, "realized", _abort); +} + static void ppc_spapr_reset(void) { sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine()); @@ -962,6 +970,14 @@ static void ppc_spapr_reset(void) spapr->rtas_addr = rtas_limit - RTAS_MAX_SIZE; spapr->fdt_addr = spapr->rtas_addr - FDT_MAX_SIZE; +/* Create a rng device if the user did not provide it already and + * KVM has hwrng support. + */ +if (kvmppc_hwrng_present() && +!object_resolve_path_type("", TYPE_SPAPR_RNG, NULL)) { +spapr_rng_create(); +} + /* Load the fdt */ spapr_finalize_fdt(spapr, spapr->fdt_addr, spapr->rtas_addr, spapr->rtas_size); diff --git a/hw/ppc/spapr_rng.c b/hw/ppc/spapr_rng.c index ed43d5e04221..ee5af302bd4d 100644 --- a/hw/ppc/spapr_rng.c +++ b/hw/ppc/spapr_rng.c @@ -114,7 +114,7 @@ static void spapr_rng_realize(DeviceState *dev, Error **errp) sPAPRRngState *rngstate = SPAPR_RNG(dev); if (rngstate->use_kvm) { -if (kvmppc_enable_hwrng() == 0) { +if (kvmppc_hwrng_present() && kvmppc_enable_hwrng() == 0) { return; } /* diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c index 42f66fea23e9..008f8a26ab17 100644 --- a/target-ppc/kvm.c +++ b/target-ppc/kvm.c @@ -2485,11 +2485,12 @@ int kvm_arch_msi_data_to_gsi(uint32_t data) return data & 0x; } -int kvmppc_enable_hwrng(void) +bool kvmppc_hwrng_present(void) { -if (!kvm_enabled() || !kvm_check_extension(kvm_state, KVM_CAP_PPC_HWRNG)) { -return -1; -} +return kvm_enabled() && kvm_check_extension(kvm_state, KVM_CAP_PPC_HWRNG); +} +int kvmppc_enable_hwrng(void) +{ return kvmppc_enable_hcall(kvm_state, H_RANDOM); } diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h index 68836b401105..4b78bfe5224a 100644 --- a/target-ppc/kvm_ppc.h +++ b/target-ppc/kvm_ppc.h @@ -55,6 +55,7 @@ void kvmppc_hash64_free_pteg(uint64_t token); void kvmppc_hash64_write_pte(CPUPPCState *env, target_ulong pte_index, target_ulong pte0, target_ulong pte1); bool kvmppc_has_cap_fixup_hcalls(void); +bool kvmppc_hwrng_present(void); int kvmppc_enable_hwrng(void); #else @@ -249,6 +250,11 @@ static inline bool kvmppc_has_cap_fixup_hcalls(void) abort(); } +static inline bool kvmppc_hwrng_present(void) +{ +return false; +} + static inline int kvmppc_enable_hwrng(void) { return -1; > hw/ppc/Makefile.objs | 2 +- > hw/ppc/spapr.c | 8 +++ > hw/ppc/spapr_rng.c | 186 >
[PATCH] KVM: PPC: fix typo in top comment about locking
Signed-off-by: Greg Kurz <gk...@linux.vnet.ibm.com> --- arch/powerpc/kvm/book3s_xics.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/kvm/book3s_xics.c b/arch/powerpc/kvm/book3s_xics.c index c6ca7db64673..905e94a1370f 100644 --- a/arch/powerpc/kvm/book3s_xics.c +++ b/arch/powerpc/kvm/book3s_xics.c @@ -41,7 +41,7 @@ * === * * Each ICS has a spin lock protecting the information about the IRQ - * sources and avoiding simultaneous deliveries if the same interrupt. + * sources and avoiding simultaneous deliveries of the same interrupt. * * ICP operations are done via a single compare & swap transaction * (most ICP state fits in the union kvmppc_icp_state) -- 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] KVM: PPC: fix typo in top comment about locking
Signed-off-by: Greg Kurz <gk...@linux.vnet.ibm.com> --- arch/powerpc/kvm/book3s_xics.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/kvm/book3s_xics.c b/arch/powerpc/kvm/book3s_xics.c index c6ca7db64673..905e94a1370f 100644 --- a/arch/powerpc/kvm/book3s_xics.c +++ b/arch/powerpc/kvm/book3s_xics.c @@ -41,7 +41,7 @@ * === * * Each ICS has a spin lock protecting the information about the IRQ - * sources and avoiding simultaneous deliveries if the same interrupt. + * sources and avoiding simultaneous deliveries of the same interrupt. * * ICP operations are done via a single compare & swap transaction * (most ICP state fits in the union kvmppc_icp_state) -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] KVM: Add Kconfig option to signal cross-endian guests
On Thu, 9 Jul 2015 09:49:05 +0200 Thomas Huth th...@redhat.com wrote: The option for supporting cross-endianness legacy guests in the vhost and tun code should only be available on systems that support cross-endian guests. Signed-off-by: Thomas Huth th...@redhat.com Acked-by: Greg Kurz gk...@linux.vnet.ibm.com -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] KVM: Add Kconfig option to signal cross-endian guests
On Thu, 9 Jul 2015 09:49:05 +0200 Thomas Huth th...@redhat.com wrote: The option for supporting cross-endianness legacy guests in the vhost and tun code should only be available on systems that support cross-endian guests. Signed-off-by: Thomas Huth th...@redhat.com Acked-by: Greg Kurz gk...@linux.vnet.ibm.com -- 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: [PULL] virtio/vhost: cross endian support
On Thu, 2 Jul 2015 08:01:28 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Wed, Jul 01, 2015 at 12:02:50PM -0700, Linus Torvalds wrote: On Wed, Jul 1, 2015 at 2:31 AM, Michael S. Tsirkin m...@redhat.com wrote: virtio/vhost: cross endian support Ugh. Does this really have to be dynamic? Can't virtio do the sane thing, and just use a _fixed_ endianness? Doing a unconditional byte swap is faster and simpler than the crazy conditionals. That's true regardless of endianness, but gets to be even more so if the fixed endianness is little-endian, since BE is not-so-slowly fading from the world. Linus Yea, well - support for legacy BE guests on the new LE hosts is exactly the motivation for this. I dislike it too, but there are two redeeming properties that made me merge this: 1. It's a trivial amount of code: since we wrap host/guest accesses anyway, almost all of it is well hidden from drivers. 2. Sane platforms would never set flags like VHOST_CROSS_ENDIAN_LEGACY - and when it's clear, there's zero overhead (as some point it was tested by compiling with and without the patches, got the same stripped binary). Maybe we could create a Kconfig symbol to enforce point (2): prevent people from enabling it e.g. on x86. I will look into this - but it can be done by a patch on top, so I think this can be merged as is. This cross-endian *oddity* is targeting PowerPC book3s_64 processors... I am not aware of any other users. Maybe create a symbol that would be only selected by PPC_BOOK3S_64 ? Or do you know of someone using kernel with all config options enabled undiscriminately? Thanks, -- 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 v6 0/8] vhost: support for cross endian guests
On Tue, 12 May 2015 12:52:55 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Tue, May 12, 2015 at 12:44:26PM +0200, Greg Kurz wrote: On Fri, 24 Apr 2015 15:31:54 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Fri, Apr 24, 2015 at 02:24:15PM +0200, Greg Kurz wrote: Only cosmetic and documentation changes since v5. --- Looks sane to me. I plan to review and apply next week. Hi Michael, I realize you just got back and have tons of things to do... Do you still plan to apply shortly ? Would you also have time to comment the QEMU part ? Thanks. -- Greg Yes, sorry about the delay - I also got virtio upstream landed on my lap. I'll do my best to prioritize. Hi Michael, This is just a friendly reminder. Are there chances you pick up this kernel series before virtio 1.0 is in upstream QEMU ? Cheers. -- Greg Greg Kurz (8): virtio: introduce virtio_is_little_endian() helper tun: add tun_is_little_endian() helper macvtap: introduce macvtap_is_little_endian() helper vringh: introduce vringh_is_little_endian() helper vhost: introduce vhost_is_little_endian() helper virtio: add explicit big-endian support to memory accessors vhost: cross-endian support for legacy devices macvtap/tun: cross-endian support for little-endian hosts drivers/net/Kconfig | 14 ++ drivers/net/macvtap.c| 66 +- drivers/net/tun.c| 68 ++ drivers/vhost/Kconfig| 15 +++ drivers/vhost/vhost.c| 85 ++ drivers/vhost/vhost.h| 25 --- include/linux/virtio_byteorder.h | 24 ++- include/linux/virtio_config.h| 18 +--- include/linux/vringh.h | 18 +--- include/uapi/linux/if_tun.h |6 +++ include/uapi/linux/vhost.h | 14 ++ 11 files changed, 320 insertions(+), 33 deletions(-) -- Greg -- 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 v5 6/8] virtio: add explicit big-endian support to memory accessors
On Fri, 24 Apr 2015 09:04:21 +0200 Cornelia Huck cornelia.h...@de.ibm.com wrote: On Thu, 23 Apr 2015 21:27:19 +0200 Thomas Huth th...@redhat.com wrote: Thomas's e-mail did not make it to my mailbox... weird. :-\ On Thu, 23 Apr 2015 17:29:06 +0200 Greg Kurz gk...@linux.vnet.ibm.com wrote: The current memory accessors logic is: - little endian if little_endian - native endian (i.e. no byteswap) if !little_endian If we want to fully support cross-endian vhost, we also need to be able to convert to big endian. Instead of changing the little_endian argument to some 3-value enum, this patch changes the logic to: - little endian if little_endian - big endian if !little_endian The native endian case is handled by all users with a trivial helper. This patch doesn't change any functionality, nor it does add overhead. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- Changes since v4: - style fixes (I have chosen if ... else in most places to stay below 80 columns, with the notable exception of the vhost helper which gets shorten in a later patch) drivers/net/macvtap.c|5 - drivers/net/tun.c|5 - drivers/vhost/vhost.h|2 +- include/linux/virtio_byteorder.h | 24 ++-- include/linux/virtio_config.h|5 - include/linux/vringh.h |2 +- 6 files changed, 28 insertions(+), 15 deletions(-) diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c index a2f2958..6cf6b3e 100644 --- a/drivers/net/macvtap.c +++ b/drivers/net/macvtap.c @@ -51,7 +51,10 @@ struct macvtap_queue { static inline bool macvtap_is_little_endian(struct macvtap_queue *q) { - return q-flags MACVTAP_VNET_LE; + if (q-flags MACVTAP_VNET_LE) + return true; + else + return virtio_legacy_is_little_endian(); simply: return (q-flags MACVTAP_VNET_LE) || virtio_legacy_is_little_endian(); ? ISTR that MST preferred the current notation, but I like either your way or ?: (with linebreak) even better. MST did not like the initial notation I had used actually. FWIW I like the simplicity of Thomas's suggestion... even better than ?: which is: some_LE_check() ? true : some_default_endianness() } (...) diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h index 6a49960..954c657 100644 --- a/drivers/vhost/vhost.h +++ b/drivers/vhost/vhost.h @@ -175,7 +175,7 @@ static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit) static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq) { - return vhost_has_feature(vq, VIRTIO_F_VERSION_1); + return vhost_has_feature(vq, VIRTIO_F_VERSION_1) ? true : virtio_legacy_is_little_endian(); } That line is way longer than 80 characters ... may I suggest to switch at least here to: return vhost_has_feature(vq, VIRTIO_F_VERSION_1) || virtio_legacy_is_little_endian(); I think the line will collapse in a further patch anyway. Yes, as mentionned in the changelog :) -- 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 -- 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 v5 7/8] vhost: cross-endian support for legacy devices
On Fri, 24 Apr 2015 09:19:26 +0200 Cornelia Huck cornelia.h...@de.ibm.com wrote: On Thu, 23 Apr 2015 17:29:42 +0200 Greg Kurz gk...@linux.vnet.ibm.com wrote: This patch brings cross-endian support to vhost when used to implement legacy virtio devices. Since it is a relatively rare situation, the feature availability is controlled by a kernel config option (not set by default). The vq-is_le boolean field is added to cache the endianness to be used for ring accesses. It defaults to native endian, as expected by legacy virtio devices. When the ring gets active, we force little endian if the device is modern. When the ring is deactivated, we revert to the native endian default. If cross-endian was compiled in, a vq-user_be boolean field is added so that userspace may request a specific endianness. This field is used to override the default when activating the ring of a legacy device. It has no effect on modern devices. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- Changes since v4: - rewrote patch title to mention cross-endian - renamed config to VHOST_CROSS_ENDIAN_LEGACY - rewrote config description and help - moved ifdefery to top of vhost.c - added a detailed comment about the lifecycle of vq-user_be in vhost_init_is_le() - renamed ioctls to VHOST_[GS]ET_VRING_ENDIAN - added LE/BE defines to the ioctl API - rewrote ioctl sanity check with the LE/BE defines - updated comment in uapi/linux/vhost.h to mention that the availibility of both SET and GET ioctls depends on the kernel config drivers/vhost/Kconfig | 15 drivers/vhost/vhost.c | 86 +++- drivers/vhost/vhost.h | 10 + include/uapi/linux/vhost.h | 12 ++ 4 files changed, 121 insertions(+), 2 deletions(-) diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig index 017a1e8..74d7380 100644 --- a/drivers/vhost/Kconfig +++ b/drivers/vhost/Kconfig @@ -32,3 +32,18 @@ config VHOST ---help--- This option is selected by any driver which needs to access the core of vhost. + +config VHOST_CROSS_ENDIAN_LEGACY + bool Cross-endian support for vhost + default n + ---help--- + This option allows vhost to support guests with a different byte + ordering from host. ...while using legacy virtio. Might help to explain the LEGACY in the config option ;) Makes sense indeed ! + + Userspace programs can control the feature using the + VHOST_SET_VRING_ENDIAN and VHOST_GET_VRING_ENDIAN ioctls. + + This is only useful on a few platforms (ppc64 and arm64). Since it + adds some overhead, it is disabled default. s/default/by default/ Ok. + + If unsure, say N. diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index 2ee2826..8c4390d 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -36,6 +36,78 @@ enum { #define vhost_used_event(vq) ((__virtio16 __user *)vq-avail-ring[vq-num]) #define vhost_avail_event(vq) ((__virtio16 __user *)vq-used-ring[vq-num]) +#ifdef CONFIG_VHOST_CROSS_ENDIAN_LEGACY +static void vhost_vq_reset_user_be(struct vhost_virtqueue *vq) +{ + vq-user_be = !virtio_legacy_is_little_endian(); +} + +static long vhost_set_vring_endian(struct vhost_virtqueue *vq, int __user *argp) +{ + struct vhost_vring_state s; + + if (vq-private_data) + return -EBUSY; + + if (copy_from_user(s, argp, sizeof(s))) + return -EFAULT; + + if (s.num != VHOST_VRING_LITTLE_ENDIAN + s.num != VHOST_VRING_BIG_ENDIAN) + return -EINVAL; + + vq-user_be = s.num; + + return 0; +} + +static long vhost_get_vring_endian(struct vhost_virtqueue *vq, u32 idx, + int __user *argp) +{ + struct vhost_vring_state s = { + .index = idx, + .num = vq-user_be + }; + + if (copy_to_user(argp, s, sizeof(s))) + return -EFAULT; + + return 0; +} + +static void vhost_init_is_le(struct vhost_virtqueue *vq) +{ + /* Note for legacy virtio: user_be is initialized at reset time +* according to the host endianness. If userspace does not set an +* explicit endianness, the default behavior is native endian, as +* expected by legacy virtio. +*/ + vq-is_le = vhost_has_feature(vq, VIRTIO_F_VERSION_1) || !vq-user_be; +} +#else +static void vhost_vq_reset_user_be(struct vhost_virtqueue *vq) +{ + ; Just leave the function body empty? Ok. +} + +static long vhost_set_vring_endian(struct vhost_virtqueue *vq, int __user *argp) +{ + return -ENOIOCTLCMD; +} + +static long vhost_get_vring_endian(struct vhost_virtqueue *vq, u32 idx, + int __user *argp) +{ + return -ENOIOCTLCMD; +} + +static void
[PATCH v6 5/8] vhost: introduce vhost_is_little_endian() helper
Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/vhost/vhost.h | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h index 8c1c792..6a49960 100644 --- a/drivers/vhost/vhost.h +++ b/drivers/vhost/vhost.h @@ -173,34 +173,39 @@ static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit) return vq-acked_features (1ULL bit); } +static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq) +{ + return vhost_has_feature(vq, VIRTIO_F_VERSION_1); +} + /* Memory accessors */ static inline u16 vhost16_to_cpu(struct vhost_virtqueue *vq, __virtio16 val) { - return __virtio16_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __virtio16_to_cpu(vhost_is_little_endian(vq), val); } static inline __virtio16 cpu_to_vhost16(struct vhost_virtqueue *vq, u16 val) { - return __cpu_to_virtio16(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __cpu_to_virtio16(vhost_is_little_endian(vq), val); } static inline u32 vhost32_to_cpu(struct vhost_virtqueue *vq, __virtio32 val) { - return __virtio32_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __virtio32_to_cpu(vhost_is_little_endian(vq), val); } static inline __virtio32 cpu_to_vhost32(struct vhost_virtqueue *vq, u32 val) { - return __cpu_to_virtio32(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __cpu_to_virtio32(vhost_is_little_endian(vq), val); } static inline u64 vhost64_to_cpu(struct vhost_virtqueue *vq, __virtio64 val) { - return __virtio64_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __virtio64_to_cpu(vhost_is_little_endian(vq), val); } static inline __virtio64 cpu_to_vhost64(struct vhost_virtqueue *vq, u64 val) { - return __cpu_to_virtio64(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __cpu_to_virtio64(vhost_is_little_endian(vq), val); } #endif -- 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 v6 6/8] virtio: add explicit big-endian support to memory accessors
The current memory accessors logic is: - little endian if little_endian - native endian (i.e. no byteswap) if !little_endian If we want to fully support cross-endian vhost, we also need to be able to convert to big endian. Instead of changing the little_endian argument to some 3-value enum, this patch changes the logic to: - little endian if little_endian - big endian if !little_endian The native endian case is handled by all users with a trivial helper. This patch doesn't change any functionality, nor it does add overhead. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- Changes since v5: - changed endian checking helpers as suggested by Thomas (use || and line breaker) drivers/net/macvtap.c|3 ++- drivers/net/tun.c|3 ++- drivers/vhost/vhost.h|3 ++- include/linux/virtio_byteorder.h | 24 ++-- include/linux/virtio_config.h|3 ++- include/linux/vringh.h |3 ++- 6 files changed, 24 insertions(+), 15 deletions(-) diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c index a2f2958..0327d9d 100644 --- a/drivers/net/macvtap.c +++ b/drivers/net/macvtap.c @@ -51,7 +51,8 @@ struct macvtap_queue { static inline bool macvtap_is_little_endian(struct macvtap_queue *q) { - return q-flags MACVTAP_VNET_LE; + return q-flags MACVTAP_VNET_LE || + virtio_legacy_is_little_endian(); } static inline u16 macvtap16_to_cpu(struct macvtap_queue *q, __virtio16 val) diff --git a/drivers/net/tun.c b/drivers/net/tun.c index 3c3d6c0..7c4f6b6 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -208,7 +208,8 @@ struct tun_struct { static inline bool tun_is_little_endian(struct tun_struct *tun) { - return tun-flags TUN_VNET_LE; + return tun-flags TUN_VNET_LE || + virtio_legacy_is_little_endian(); } static inline u16 tun16_to_cpu(struct tun_struct *tun, __virtio16 val) diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h index 6a49960..a4fa33a 100644 --- a/drivers/vhost/vhost.h +++ b/drivers/vhost/vhost.h @@ -175,7 +175,8 @@ static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit) static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq) { - return vhost_has_feature(vq, VIRTIO_F_VERSION_1); + return vhost_has_feature(vq, VIRTIO_F_VERSION_1) || + virtio_legacy_is_little_endian(); } /* Memory accessors */ diff --git a/include/linux/virtio_byteorder.h b/include/linux/virtio_byteorder.h index 51865d0..ce63a2c 100644 --- a/include/linux/virtio_byteorder.h +++ b/include/linux/virtio_byteorder.h @@ -3,17 +3,21 @@ #include linux/types.h #include uapi/linux/virtio_types.h -/* - * Low-level memory accessors for handling virtio in modern little endian and in - * compatibility native endian format. - */ +static inline bool virtio_legacy_is_little_endian(void) +{ +#ifdef __LITTLE_ENDIAN + return true; +#else + return false; +#endif +} static inline u16 __virtio16_to_cpu(bool little_endian, __virtio16 val) { if (little_endian) return le16_to_cpu((__force __le16)val); else - return (__force u16)val; + return be16_to_cpu((__force __be16)val); } static inline __virtio16 __cpu_to_virtio16(bool little_endian, u16 val) @@ -21,7 +25,7 @@ static inline __virtio16 __cpu_to_virtio16(bool little_endian, u16 val) if (little_endian) return (__force __virtio16)cpu_to_le16(val); else - return (__force __virtio16)val; + return (__force __virtio16)cpu_to_be16(val); } static inline u32 __virtio32_to_cpu(bool little_endian, __virtio32 val) @@ -29,7 +33,7 @@ static inline u32 __virtio32_to_cpu(bool little_endian, __virtio32 val) if (little_endian) return le32_to_cpu((__force __le32)val); else - return (__force u32)val; + return be32_to_cpu((__force __be32)val); } static inline __virtio32 __cpu_to_virtio32(bool little_endian, u32 val) @@ -37,7 +41,7 @@ static inline __virtio32 __cpu_to_virtio32(bool little_endian, u32 val) if (little_endian) return (__force __virtio32)cpu_to_le32(val); else - return (__force __virtio32)val; + return (__force __virtio32)cpu_to_be32(val); } static inline u64 __virtio64_to_cpu(bool little_endian, __virtio64 val) @@ -45,7 +49,7 @@ static inline u64 __virtio64_to_cpu(bool little_endian, __virtio64 val) if (little_endian) return le64_to_cpu((__force __le64)val); else - return (__force u64)val; + return be64_to_cpu((__force __be64)val); } static inline __virtio64 __cpu_to_virtio64(bool little_endian, u64 val) @@ -53,7 +57,7 @@ static inline __virtio64 __cpu_to_virtio64(bool little_endian, u64 val) if (little_endian) return
[PATCH v6 3/8] macvtap: introduce macvtap_is_little_endian() helper
Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/net/macvtap.c |9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c index 27ecc5c..a2f2958 100644 --- a/drivers/net/macvtap.c +++ b/drivers/net/macvtap.c @@ -49,14 +49,19 @@ struct macvtap_queue { #define MACVTAP_VNET_LE 0x8000 +static inline bool macvtap_is_little_endian(struct macvtap_queue *q) +{ + return q-flags MACVTAP_VNET_LE; +} + static inline u16 macvtap16_to_cpu(struct macvtap_queue *q, __virtio16 val) { - return __virtio16_to_cpu(q-flags MACVTAP_VNET_LE, val); + return __virtio16_to_cpu(macvtap_is_little_endian(q), val); } static inline __virtio16 cpu_to_macvtap16(struct macvtap_queue *q, u16 val) { - return __cpu_to_virtio16(q-flags MACVTAP_VNET_LE, val); + return __cpu_to_virtio16(macvtap_is_little_endian(q), val); } static struct proto macvtap_proto = { -- 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 v6 4/8] vringh: introduce vringh_is_little_endian() helper
Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- include/linux/vringh.h | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/include/linux/vringh.h b/include/linux/vringh.h index a3fa537..3ed62ef 100644 --- a/include/linux/vringh.h +++ b/include/linux/vringh.h @@ -226,33 +226,38 @@ static inline void vringh_notify(struct vringh *vrh) vrh-notify(vrh); } +static inline bool vringh_is_little_endian(const struct vringh *vrh) +{ + return vrh-little_endian; +} + static inline u16 vringh16_to_cpu(const struct vringh *vrh, __virtio16 val) { - return __virtio16_to_cpu(vrh-little_endian, val); + return __virtio16_to_cpu(vringh_is_little_endian(vrh), val); } static inline __virtio16 cpu_to_vringh16(const struct vringh *vrh, u16 val) { - return __cpu_to_virtio16(vrh-little_endian, val); + return __cpu_to_virtio16(vringh_is_little_endian(vrh), val); } static inline u32 vringh32_to_cpu(const struct vringh *vrh, __virtio32 val) { - return __virtio32_to_cpu(vrh-little_endian, val); + return __virtio32_to_cpu(vringh_is_little_endian(vrh), val); } static inline __virtio32 cpu_to_vringh32(const struct vringh *vrh, u32 val) { - return __cpu_to_virtio32(vrh-little_endian, val); + return __cpu_to_virtio32(vringh_is_little_endian(vrh), val); } static inline u64 vringh64_to_cpu(const struct vringh *vrh, __virtio64 val) { - return __virtio64_to_cpu(vrh-little_endian, val); + return __virtio64_to_cpu(vringh_is_little_endian(vrh), val); } static inline __virtio64 cpu_to_vringh64(const struct vringh *vrh, u64 val) { - return __cpu_to_virtio64(vrh-little_endian, val); + return __cpu_to_virtio64(vringh_is_little_endian(vrh), val); } #endif /* _LINUX_VRINGH_H */ -- 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 v6 1/8] virtio: introduce virtio_is_little_endian() helper
Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- include/linux/virtio_config.h | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h index ca3ed78..bd1a582 100644 --- a/include/linux/virtio_config.h +++ b/include/linux/virtio_config.h @@ -205,35 +205,40 @@ int virtqueue_set_affinity(struct virtqueue *vq, int cpu) return 0; } +static inline bool virtio_is_little_endian(struct virtio_device *vdev) +{ + return virtio_has_feature(vdev, VIRTIO_F_VERSION_1); +} + /* Memory accessors */ static inline u16 virtio16_to_cpu(struct virtio_device *vdev, __virtio16 val) { - return __virtio16_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val); + return __virtio16_to_cpu(virtio_is_little_endian(vdev), val); } static inline __virtio16 cpu_to_virtio16(struct virtio_device *vdev, u16 val) { - return __cpu_to_virtio16(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val); + return __cpu_to_virtio16(virtio_is_little_endian(vdev), val); } static inline u32 virtio32_to_cpu(struct virtio_device *vdev, __virtio32 val) { - return __virtio32_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val); + return __virtio32_to_cpu(virtio_is_little_endian(vdev), val); } static inline __virtio32 cpu_to_virtio32(struct virtio_device *vdev, u32 val) { - return __cpu_to_virtio32(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val); + return __cpu_to_virtio32(virtio_is_little_endian(vdev), val); } static inline u64 virtio64_to_cpu(struct virtio_device *vdev, __virtio64 val) { - return __virtio64_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val); + return __virtio64_to_cpu(virtio_is_little_endian(vdev), val); } static inline __virtio64 cpu_to_virtio64(struct virtio_device *vdev, u64 val) { - return __cpu_to_virtio64(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val); + return __cpu_to_virtio64(virtio_is_little_endian(vdev), val); } /* Config space accessors. */ -- 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 v6 8/8] macvtap/tun: cross-endian support for little-endian hosts
The VNET_LE flag was introduced to fix accesses to virtio 1.0 headers that are always little-endian. It can also be used to handle the special case of a legacy little-endian device implemented by a big-endian host. Let's add a flag and ioctls for big-endian devices as well. If both flags are set, little-endian wins. Since this is isn't a common usecase, the feature is controlled by a kernel config option (not set by default). Both macvtap and tun are covered by this patch since they share the same API with userland. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- Changes since v5: - changed {macvtapi,tun}_legacy_is_little_endian() to use ?: drivers/net/Kconfig | 14 ++ drivers/net/macvtap.c | 57 +- drivers/net/tun.c | 59 ++- include/uapi/linux/if_tun.h |6 4 files changed, 134 insertions(+), 2 deletions(-) diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index df51d60..71ac0ec 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -244,6 +244,20 @@ config TUN If you don't know what to use this for, you don't need it. +config TUN_VNET_CROSS_LE + bool Support for cross-endian vnet headers on little-endian kernels + default n + ---help--- + This option allows TUN/TAP and MACVTAP device drivers in a + little-endian kernel to parse vnet headers that come from a + big-endian legacy virtio device. + + Userspace programs can control the feature using the TUNSETVNETBE + and TUNGETVNETBE ioctls. + + Unless you have a little-endian system hosting a big-endian virtual + machine with a legacy virtio NIC, you should say N. + config VETH tristate Virtual ethernet pair device ---help--- diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c index 0327d9d..dc0a47c 100644 --- a/drivers/net/macvtap.c +++ b/drivers/net/macvtap.c @@ -48,11 +48,60 @@ struct macvtap_queue { #define MACVTAP_FEATURES (IFF_VNET_HDR | IFF_MULTI_QUEUE) #define MACVTAP_VNET_LE 0x8000 +#define MACVTAP_VNET_BE 0x4000 + +#ifdef CONFIG_TUN_VNET_CROSS_LE +static inline bool macvtap_legacy_is_little_endian(struct macvtap_queue *q) +{ + return q-flags MACVTAP_VNET_BE ? false : + virtio_legacy_is_little_endian(); +} + +static long macvtap_get_vnet_be(struct macvtap_queue *q, int __user *sp) +{ + int s = !!(q-flags MACVTAP_VNET_BE); + + if (put_user(s, sp)) + return -EFAULT; + + return 0; +} + +static long macvtap_set_vnet_be(struct macvtap_queue *q, int __user *sp) +{ + int s; + + if (get_user(s, sp)) + return -EFAULT; + + if (s) + q-flags |= MACVTAP_VNET_BE; + else + q-flags = ~MACVTAP_VNET_BE; + + return 0; +} +#else +static inline bool macvtap_legacy_is_little_endian(struct macvtap_queue *q) +{ + return virtio_legacy_is_little_endian(); +} + +static long macvtap_get_vnet_be(struct macvtap_queue *q, int __user *argp) +{ + return -EINVAL; +} + +static long macvtap_set_vnet_be(struct macvtap_queue *q, int __user *argp) +{ + return -EINVAL; +} +#endif /* CONFIG_TUN_VNET_CROSS_LE */ static inline bool macvtap_is_little_endian(struct macvtap_queue *q) { return q-flags MACVTAP_VNET_LE || - virtio_legacy_is_little_endian(); + macvtap_legacy_is_little_endian(q); } static inline u16 macvtap16_to_cpu(struct macvtap_queue *q, __virtio16 val) @@ -1096,6 +1145,12 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd, q-flags = ~MACVTAP_VNET_LE; return 0; + case TUNGETVNETBE: + return macvtap_get_vnet_be(q, sp); + + case TUNSETVNETBE: + return macvtap_set_vnet_be(q, sp); + case TUNSETOFFLOAD: /* let the user check for future flags */ if (arg ~(TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 | diff --git a/drivers/net/tun.c b/drivers/net/tun.c index 7c4f6b6..9fa05d6 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -111,6 +111,7 @@ do { \ #define TUN_FASYNC IFF_ATTACH_QUEUE /* High bits in flags field are unused. */ #define TUN_VNET_LE 0x8000 +#define TUN_VNET_BE 0x4000 #define TUN_FEATURES (IFF_NO_PI | IFF_ONE_QUEUE | IFF_VNET_HDR | \ IFF_MULTI_QUEUE) @@ -206,10 +207,58 @@ struct tun_struct { u32 flow_count; }; +#ifdef CONFIG_TUN_VNET_CROSS_LE +static inline bool tun_legacy_is_little_endian(struct tun_struct *tun) +{ + return tun-flags TUN_VNET_BE ? false : + virtio_legacy_is_little_endian(); +} + +static long tun_get_vnet_be(struct tun_struct *tun, int __user *argp) +{ + int be = !!(tun-flags TUN_VNET_BE); + + if (put_user
[PATCH v6 2/8] tun: add tun_is_little_endian() helper
Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/net/tun.c |9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/net/tun.c b/drivers/net/tun.c index 857dca4..3c3d6c0 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -206,14 +206,19 @@ struct tun_struct { u32 flow_count; }; +static inline bool tun_is_little_endian(struct tun_struct *tun) +{ + return tun-flags TUN_VNET_LE; +} + static inline u16 tun16_to_cpu(struct tun_struct *tun, __virtio16 val) { - return __virtio16_to_cpu(tun-flags TUN_VNET_LE, val); + return __virtio16_to_cpu(tun_is_little_endian(tun), val); } static inline __virtio16 cpu_to_tun16(struct tun_struct *tun, u16 val) { - return __cpu_to_virtio16(tun-flags TUN_VNET_LE, val); + return __cpu_to_virtio16(tun_is_little_endian(tun), val); } static inline u32 tun_hashfn(u32 rxhash) -- 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 v6 7/8] vhost: cross-endian support for legacy devices
This patch brings cross-endian support to vhost when used to implement legacy virtio devices. Since it is a relatively rare situation, the feature availability is controlled by a kernel config option (not set by default). The vq-is_le boolean field is added to cache the endianness to be used for ring accesses. It defaults to native endian, as expected by legacy virtio devices. When the ring gets active, we force little endian if the device is modern. When the ring is deactivated, we revert to the native endian default. If cross-endian was compiled in, a vq-user_be boolean field is added so that userspace may request a specific endianness. This field is used to override the default when activating the ring of a legacy device. It has no effect on modern devices. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- Changes since v5: - fixed description in Kconfig - fixed error description in uapi header - dropped useless semi-colon in the vhost_vq_reset_user_be() stub drivers/vhost/Kconfig | 15 drivers/vhost/vhost.c | 85 +++- drivers/vhost/vhost.h | 11 +- include/uapi/linux/vhost.h | 14 +++ 4 files changed, 122 insertions(+), 3 deletions(-) diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig index 017a1e8..533eaf0 100644 --- a/drivers/vhost/Kconfig +++ b/drivers/vhost/Kconfig @@ -32,3 +32,18 @@ config VHOST ---help--- This option is selected by any driver which needs to access the core of vhost. + +config VHOST_CROSS_ENDIAN_LEGACY + bool Cross-endian support for vhost + default n + ---help--- + This option allows vhost to support guests with a different byte + ordering from host while using legacy virtio. + + Userspace programs can control the feature using the + VHOST_SET_VRING_ENDIAN and VHOST_GET_VRING_ENDIAN ioctls. + + This is only useful on a few platforms (ppc64 and arm64). Since it + adds some overhead, it is disabled by default. + + If unsure, say N. diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index 2ee2826..9e8e004 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -36,6 +36,77 @@ enum { #define vhost_used_event(vq) ((__virtio16 __user *)vq-avail-ring[vq-num]) #define vhost_avail_event(vq) ((__virtio16 __user *)vq-used-ring[vq-num]) +#ifdef CONFIG_VHOST_CROSS_ENDIAN_LEGACY +static void vhost_vq_reset_user_be(struct vhost_virtqueue *vq) +{ + vq-user_be = !virtio_legacy_is_little_endian(); +} + +static long vhost_set_vring_endian(struct vhost_virtqueue *vq, int __user *argp) +{ + struct vhost_vring_state s; + + if (vq-private_data) + return -EBUSY; + + if (copy_from_user(s, argp, sizeof(s))) + return -EFAULT; + + if (s.num != VHOST_VRING_LITTLE_ENDIAN + s.num != VHOST_VRING_BIG_ENDIAN) + return -EINVAL; + + vq-user_be = s.num; + + return 0; +} + +static long vhost_get_vring_endian(struct vhost_virtqueue *vq, u32 idx, + int __user *argp) +{ + struct vhost_vring_state s = { + .index = idx, + .num = vq-user_be + }; + + if (copy_to_user(argp, s, sizeof(s))) + return -EFAULT; + + return 0; +} + +static void vhost_init_is_le(struct vhost_virtqueue *vq) +{ + /* Note for legacy virtio: user_be is initialized at reset time +* according to the host endianness. If userspace does not set an +* explicit endianness, the default behavior is native endian, as +* expected by legacy virtio. +*/ + vq-is_le = vhost_has_feature(vq, VIRTIO_F_VERSION_1) || !vq-user_be; +} +#else +static void vhost_vq_reset_user_be(struct vhost_virtqueue *vq) +{ +} + +static long vhost_set_vring_endian(struct vhost_virtqueue *vq, int __user *argp) +{ + return -ENOIOCTLCMD; +} + +static long vhost_get_vring_endian(struct vhost_virtqueue *vq, u32 idx, + int __user *argp) +{ + return -ENOIOCTLCMD; +} + +static void vhost_init_is_le(struct vhost_virtqueue *vq) +{ + if (vhost_has_feature(vq, VIRTIO_F_VERSION_1)) + vq-is_le = true; +} +#endif /* CONFIG_VHOST_CROSS_ENDIAN_LEGACY */ + static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh, poll_table *pt) { @@ -199,6 +270,8 @@ static void vhost_vq_reset(struct vhost_dev *dev, vq-call = NULL; vq-log_ctx = NULL; vq-memory = NULL; + vq-is_le = virtio_legacy_is_little_endian(); + vhost_vq_reset_user_be(vq); } static int vhost_worker(void *data) @@ -806,6 +879,12 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp) } else filep = eventfp; break; + case VHOST_SET_VRING_ENDIAN: + r
[PATCH v6 0/8] vhost: support for cross endian guests
Only cosmetic and documentation changes since v5. --- Greg Kurz (8): virtio: introduce virtio_is_little_endian() helper tun: add tun_is_little_endian() helper macvtap: introduce macvtap_is_little_endian() helper vringh: introduce vringh_is_little_endian() helper vhost: introduce vhost_is_little_endian() helper virtio: add explicit big-endian support to memory accessors vhost: cross-endian support for legacy devices macvtap/tun: cross-endian support for little-endian hosts drivers/net/Kconfig | 14 ++ drivers/net/macvtap.c| 66 +- drivers/net/tun.c| 68 ++ drivers/vhost/Kconfig| 15 +++ drivers/vhost/vhost.c| 85 ++ drivers/vhost/vhost.h| 25 --- include/linux/virtio_byteorder.h | 24 ++- include/linux/virtio_config.h| 18 +--- include/linux/vringh.h | 18 +--- include/uapi/linux/if_tun.h |6 +++ include/uapi/linux/vhost.h | 14 ++ 11 files changed, 320 insertions(+), 33 deletions(-) -- Greg -- 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 v5 2/8] tun: add tun_is_little_endian() helper
Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/net/tun.c |9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/net/tun.c b/drivers/net/tun.c index 857dca4..3c3d6c0 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -206,14 +206,19 @@ struct tun_struct { u32 flow_count; }; +static inline bool tun_is_little_endian(struct tun_struct *tun) +{ + return tun-flags TUN_VNET_LE; +} + static inline u16 tun16_to_cpu(struct tun_struct *tun, __virtio16 val) { - return __virtio16_to_cpu(tun-flags TUN_VNET_LE, val); + return __virtio16_to_cpu(tun_is_little_endian(tun), val); } static inline __virtio16 cpu_to_tun16(struct tun_struct *tun, u16 val) { - return __cpu_to_virtio16(tun-flags TUN_VNET_LE, val); + return __cpu_to_virtio16(tun_is_little_endian(tun), val); } static inline u32 tun_hashfn(u32 rxhash) -- 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 v5 3/8] macvtap: introduce macvtap_is_little_endian() helper
Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/net/macvtap.c |9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c index 27ecc5c..a2f2958 100644 --- a/drivers/net/macvtap.c +++ b/drivers/net/macvtap.c @@ -49,14 +49,19 @@ struct macvtap_queue { #define MACVTAP_VNET_LE 0x8000 +static inline bool macvtap_is_little_endian(struct macvtap_queue *q) +{ + return q-flags MACVTAP_VNET_LE; +} + static inline u16 macvtap16_to_cpu(struct macvtap_queue *q, __virtio16 val) { - return __virtio16_to_cpu(q-flags MACVTAP_VNET_LE, val); + return __virtio16_to_cpu(macvtap_is_little_endian(q), val); } static inline __virtio16 cpu_to_macvtap16(struct macvtap_queue *q, u16 val) { - return __cpu_to_virtio16(q-flags MACVTAP_VNET_LE, val); + return __cpu_to_virtio16(macvtap_is_little_endian(q), val); } static struct proto macvtap_proto = { -- 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 v5 4/8] vringh: introduce vringh_is_little_endian() helper
Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- include/linux/vringh.h | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/include/linux/vringh.h b/include/linux/vringh.h index a3fa537..3ed62ef 100644 --- a/include/linux/vringh.h +++ b/include/linux/vringh.h @@ -226,33 +226,38 @@ static inline void vringh_notify(struct vringh *vrh) vrh-notify(vrh); } +static inline bool vringh_is_little_endian(const struct vringh *vrh) +{ + return vrh-little_endian; +} + static inline u16 vringh16_to_cpu(const struct vringh *vrh, __virtio16 val) { - return __virtio16_to_cpu(vrh-little_endian, val); + return __virtio16_to_cpu(vringh_is_little_endian(vrh), val); } static inline __virtio16 cpu_to_vringh16(const struct vringh *vrh, u16 val) { - return __cpu_to_virtio16(vrh-little_endian, val); + return __cpu_to_virtio16(vringh_is_little_endian(vrh), val); } static inline u32 vringh32_to_cpu(const struct vringh *vrh, __virtio32 val) { - return __virtio32_to_cpu(vrh-little_endian, val); + return __virtio32_to_cpu(vringh_is_little_endian(vrh), val); } static inline __virtio32 cpu_to_vringh32(const struct vringh *vrh, u32 val) { - return __cpu_to_virtio32(vrh-little_endian, val); + return __cpu_to_virtio32(vringh_is_little_endian(vrh), val); } static inline u64 vringh64_to_cpu(const struct vringh *vrh, __virtio64 val) { - return __virtio64_to_cpu(vrh-little_endian, val); + return __virtio64_to_cpu(vringh_is_little_endian(vrh), val); } static inline __virtio64 cpu_to_vringh64(const struct vringh *vrh, u64 val) { - return __cpu_to_virtio64(vrh-little_endian, val); + return __cpu_to_virtio64(vringh_is_little_endian(vrh), val); } #endif /* _LINUX_VRINGH_H */ -- 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 v5 5/8] vhost: introduce vhost_is_little_endian() helper
Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/vhost/vhost.h | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h index 8c1c792..6a49960 100644 --- a/drivers/vhost/vhost.h +++ b/drivers/vhost/vhost.h @@ -173,34 +173,39 @@ static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit) return vq-acked_features (1ULL bit); } +static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq) +{ + return vhost_has_feature(vq, VIRTIO_F_VERSION_1); +} + /* Memory accessors */ static inline u16 vhost16_to_cpu(struct vhost_virtqueue *vq, __virtio16 val) { - return __virtio16_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __virtio16_to_cpu(vhost_is_little_endian(vq), val); } static inline __virtio16 cpu_to_vhost16(struct vhost_virtqueue *vq, u16 val) { - return __cpu_to_virtio16(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __cpu_to_virtio16(vhost_is_little_endian(vq), val); } static inline u32 vhost32_to_cpu(struct vhost_virtqueue *vq, __virtio32 val) { - return __virtio32_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __virtio32_to_cpu(vhost_is_little_endian(vq), val); } static inline __virtio32 cpu_to_vhost32(struct vhost_virtqueue *vq, u32 val) { - return __cpu_to_virtio32(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __cpu_to_virtio32(vhost_is_little_endian(vq), val); } static inline u64 vhost64_to_cpu(struct vhost_virtqueue *vq, __virtio64 val) { - return __virtio64_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __virtio64_to_cpu(vhost_is_little_endian(vq), val); } static inline __virtio64 cpu_to_vhost64(struct vhost_virtqueue *vq, u64 val) { - return __cpu_to_virtio64(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __cpu_to_virtio64(vhost_is_little_endian(vq), val); } #endif -- 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 v5 1/8] virtio: introduce virtio_is_little_endian() helper
Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- include/linux/virtio_config.h | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h index ca3ed78..bd1a582 100644 --- a/include/linux/virtio_config.h +++ b/include/linux/virtio_config.h @@ -205,35 +205,40 @@ int virtqueue_set_affinity(struct virtqueue *vq, int cpu) return 0; } +static inline bool virtio_is_little_endian(struct virtio_device *vdev) +{ + return virtio_has_feature(vdev, VIRTIO_F_VERSION_1); +} + /* Memory accessors */ static inline u16 virtio16_to_cpu(struct virtio_device *vdev, __virtio16 val) { - return __virtio16_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val); + return __virtio16_to_cpu(virtio_is_little_endian(vdev), val); } static inline __virtio16 cpu_to_virtio16(struct virtio_device *vdev, u16 val) { - return __cpu_to_virtio16(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val); + return __cpu_to_virtio16(virtio_is_little_endian(vdev), val); } static inline u32 virtio32_to_cpu(struct virtio_device *vdev, __virtio32 val) { - return __virtio32_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val); + return __virtio32_to_cpu(virtio_is_little_endian(vdev), val); } static inline __virtio32 cpu_to_virtio32(struct virtio_device *vdev, u32 val) { - return __cpu_to_virtio32(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val); + return __cpu_to_virtio32(virtio_is_little_endian(vdev), val); } static inline u64 virtio64_to_cpu(struct virtio_device *vdev, __virtio64 val) { - return __virtio64_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val); + return __virtio64_to_cpu(virtio_is_little_endian(vdev), val); } static inline __virtio64 cpu_to_virtio64(struct virtio_device *vdev, u64 val) { - return __cpu_to_virtio64(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val); + return __cpu_to_virtio64(virtio_is_little_endian(vdev), val); } /* Config space accessors. */ -- 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 v5 0/8] vhost: support for cross endian guests
Hi, This patchset allows vhost to be used with legacy virtio when guest and host have a different endianness. It is compatible with modern virtio and can be fully compiled out through kernel config. FWIW, I could flawlessly kexec/reboot guests from ppc64 to ppc64le and back. I could also migrate from a ppc64 to a ppc64le host and back. No regressions on x86 as expected. My experimental QEMU tree is here: https://github.com/gkurz/qemu.git vhost/cross-endian I'd be glad if this series could make it to 4.1. Cheers. --- Greg Kurz (8): virtio: introduce virtio_is_little_endian() helper tun: add tun_is_little_endian() helper macvtap: introduce macvtap_is_little_endian() helper vringh: introduce vringh_is_little_endian() helper vhost: introduce vhost_is_little_endian() helper virtio: add explicit big-endian support to memory accessors vhost: cross-endian support for legacy devices macvtap/tun: cross-endian support for little-endian hosts drivers/net/Kconfig | 14 ++ drivers/net/macvtap.c| 68 +- drivers/net/tun.c| 70 ++- drivers/vhost/Kconfig| 15 +++ drivers/vhost/vhost.c| 86 ++ drivers/vhost/vhost.h| 25 --- include/linux/virtio_byteorder.h | 24 ++- include/linux/virtio_config.h| 20 ++--- include/linux/vringh.h | 17 +--- include/uapi/linux/if_tun.h |6 +++ include/uapi/linux/vhost.h | 12 + 11 files changed, 324 insertions(+), 33 deletions(-) -- Greg -- 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 v5 6/8] virtio: add explicit big-endian support to memory accessors
The current memory accessors logic is: - little endian if little_endian - native endian (i.e. no byteswap) if !little_endian If we want to fully support cross-endian vhost, we also need to be able to convert to big endian. Instead of changing the little_endian argument to some 3-value enum, this patch changes the logic to: - little endian if little_endian - big endian if !little_endian The native endian case is handled by all users with a trivial helper. This patch doesn't change any functionality, nor it does add overhead. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- Changes since v4: - style fixes (I have chosen if ... else in most places to stay below 80 columns, with the notable exception of the vhost helper which gets shorten in a later patch) drivers/net/macvtap.c|5 - drivers/net/tun.c|5 - drivers/vhost/vhost.h|2 +- include/linux/virtio_byteorder.h | 24 ++-- include/linux/virtio_config.h|5 - include/linux/vringh.h |2 +- 6 files changed, 28 insertions(+), 15 deletions(-) diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c index a2f2958..6cf6b3e 100644 --- a/drivers/net/macvtap.c +++ b/drivers/net/macvtap.c @@ -51,7 +51,10 @@ struct macvtap_queue { static inline bool macvtap_is_little_endian(struct macvtap_queue *q) { - return q-flags MACVTAP_VNET_LE; + if (q-flags MACVTAP_VNET_LE) + return true; + else + return virtio_legacy_is_little_endian(); } static inline u16 macvtap16_to_cpu(struct macvtap_queue *q, __virtio16 val) diff --git a/drivers/net/tun.c b/drivers/net/tun.c index 3c3d6c0..5b044d4 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -208,7 +208,10 @@ struct tun_struct { static inline bool tun_is_little_endian(struct tun_struct *tun) { - return tun-flags TUN_VNET_LE; + if (tun-flags TUN_VNET_LE) + return true; + else + return virtio_legacy_is_little_endian(); } static inline u16 tun16_to_cpu(struct tun_struct *tun, __virtio16 val) diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h index 6a49960..954c657 100644 --- a/drivers/vhost/vhost.h +++ b/drivers/vhost/vhost.h @@ -175,7 +175,7 @@ static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit) static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq) { - return vhost_has_feature(vq, VIRTIO_F_VERSION_1); + return vhost_has_feature(vq, VIRTIO_F_VERSION_1) ? true : virtio_legacy_is_little_endian(); } /* Memory accessors */ diff --git a/include/linux/virtio_byteorder.h b/include/linux/virtio_byteorder.h index 51865d0..ce63a2c 100644 --- a/include/linux/virtio_byteorder.h +++ b/include/linux/virtio_byteorder.h @@ -3,17 +3,21 @@ #include linux/types.h #include uapi/linux/virtio_types.h -/* - * Low-level memory accessors for handling virtio in modern little endian and in - * compatibility native endian format. - */ +static inline bool virtio_legacy_is_little_endian(void) +{ +#ifdef __LITTLE_ENDIAN + return true; +#else + return false; +#endif +} static inline u16 __virtio16_to_cpu(bool little_endian, __virtio16 val) { if (little_endian) return le16_to_cpu((__force __le16)val); else - return (__force u16)val; + return be16_to_cpu((__force __be16)val); } static inline __virtio16 __cpu_to_virtio16(bool little_endian, u16 val) @@ -21,7 +25,7 @@ static inline __virtio16 __cpu_to_virtio16(bool little_endian, u16 val) if (little_endian) return (__force __virtio16)cpu_to_le16(val); else - return (__force __virtio16)val; + return (__force __virtio16)cpu_to_be16(val); } static inline u32 __virtio32_to_cpu(bool little_endian, __virtio32 val) @@ -29,7 +33,7 @@ static inline u32 __virtio32_to_cpu(bool little_endian, __virtio32 val) if (little_endian) return le32_to_cpu((__force __le32)val); else - return (__force u32)val; + return be32_to_cpu((__force __be32)val); } static inline __virtio32 __cpu_to_virtio32(bool little_endian, u32 val) @@ -37,7 +41,7 @@ static inline __virtio32 __cpu_to_virtio32(bool little_endian, u32 val) if (little_endian) return (__force __virtio32)cpu_to_le32(val); else - return (__force __virtio32)val; + return (__force __virtio32)cpu_to_be32(val); } static inline u64 __virtio64_to_cpu(bool little_endian, __virtio64 val) @@ -45,7 +49,7 @@ static inline u64 __virtio64_to_cpu(bool little_endian, __virtio64 val) if (little_endian) return le64_to_cpu((__force __le64)val); else - return (__force u64)val; + return be64_to_cpu((__force __be64)val); } static inline __virtio64 __cpu_to_virtio64
[PATCH v5 8/8] macvtap/tun: cross-endian support for little-endian hosts
The VNET_LE flag was introduced to fix accesses to virtio 1.0 headers that are always little-endian. It can also be used to handle the special case of a legacy little-endian device implemented by a big-endian host. Let's add a flag and ioctls for big-endian devices as well. If both flags are set, little-endian wins. Since this is isn't a common usecase, the feature is controlled by a kernel config option (not set by default). Both macvtap and tun are covered by this patch since they share the same API with userland. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- Changes since v4: - rewrote patch title to mention cross-endian - renamed config to TUN_VNET_CROSS_LE - rewrote config description and help - moved ifdefery to top of tun.c and macvtap.c - updated comment in uapi/linux/if_tun.h to mention that the availibility of both SET and GET ioctls depends on the kernel config drivers/net/Kconfig | 14 ++ drivers/net/macvtap.c | 58 +- drivers/net/tun.c | 60 ++- include/uapi/linux/if_tun.h |6 4 files changed, 136 insertions(+), 2 deletions(-) diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index df51d60..71ac0ec 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -244,6 +244,20 @@ config TUN If you don't know what to use this for, you don't need it. +config TUN_VNET_CROSS_LE + bool Support for cross-endian vnet headers on little-endian kernels + default n + ---help--- + This option allows TUN/TAP and MACVTAP device drivers in a + little-endian kernel to parse vnet headers that come from a + big-endian legacy virtio device. + + Userspace programs can control the feature using the TUNSETVNETBE + and TUNGETVNETBE ioctls. + + Unless you have a little-endian system hosting a big-endian virtual + machine with a legacy virtio NIC, you should say N. + config VETH tristate Virtual ethernet pair device ---help--- diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c index 6cf6b3e..460ed9f 100644 --- a/drivers/net/macvtap.c +++ b/drivers/net/macvtap.c @@ -48,13 +48,63 @@ struct macvtap_queue { #define MACVTAP_FEATURES (IFF_VNET_HDR | IFF_MULTI_QUEUE) #define MACVTAP_VNET_LE 0x8000 +#define MACVTAP_VNET_BE 0x4000 + +#ifdef CONFIG_TUN_VNET_CROSS_LE +static inline bool macvtap_legacy_is_little_endian(struct macvtap_queue *q) +{ + if (q-flags MACVTAP_VNET_BE) + return false; + return virtio_legacy_is_little_endian(); +} + +static long macvtap_get_vnet_be(struct macvtap_queue *q, int __user *sp) +{ + int s = !!(q-flags MACVTAP_VNET_BE); + + if (put_user(s, sp)) + return -EFAULT; + + return 0; +} + +static long macvtap_set_vnet_be(struct macvtap_queue *q, int __user *sp) +{ + int s; + + if (get_user(s, sp)) + return -EFAULT; + + if (s) + q-flags |= MACVTAP_VNET_BE; + else + q-flags = ~MACVTAP_VNET_BE; + + return 0; +} +#else +static inline bool macvtap_legacy_is_little_endian(struct macvtap_queue *q) +{ + return virtio_legacy_is_little_endian(); +} + +static long macvtap_get_vnet_be(struct macvtap_queue *q, int __user *argp) +{ + return -EINVAL; +} + +static long macvtap_set_vnet_be(struct macvtap_queue *q, int __user *argp) +{ + return -EINVAL; +} +#endif /* CONFIG_TUN_VNET_CROSS_LE */ static inline bool macvtap_is_little_endian(struct macvtap_queue *q) { if (q-flags MACVTAP_VNET_LE) return true; else - return virtio_legacy_is_little_endian(); + return macvtap_legacy_is_little_endian(q); } static inline u16 macvtap16_to_cpu(struct macvtap_queue *q, __virtio16 val) @@ -1098,6 +1148,12 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd, q-flags = ~MACVTAP_VNET_LE; return 0; + case TUNGETVNETBE: + return macvtap_get_vnet_be(q, sp); + + case TUNSETVNETBE: + return macvtap_set_vnet_be(q, sp); + case TUNSETOFFLOAD: /* let the user check for future flags */ if (arg ~(TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 | diff --git a/drivers/net/tun.c b/drivers/net/tun.c index 5b044d4..1b0afa9 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -111,6 +111,7 @@ do { \ #define TUN_FASYNC IFF_ATTACH_QUEUE /* High bits in flags field are unused. */ #define TUN_VNET_LE 0x8000 +#define TUN_VNET_BE 0x4000 #define TUN_FEATURES (IFF_NO_PI | IFF_ONE_QUEUE | IFF_VNET_HDR | \ IFF_MULTI_QUEUE) @@ -206,12 +207,61 @@ struct tun_struct { u32 flow_count; }; +#ifdef CONFIG_TUN_VNET_CROSS_LE +static inline
[PATCH v5 7/8] vhost: cross-endian support for legacy devices
This patch brings cross-endian support to vhost when used to implement legacy virtio devices. Since it is a relatively rare situation, the feature availability is controlled by a kernel config option (not set by default). The vq-is_le boolean field is added to cache the endianness to be used for ring accesses. It defaults to native endian, as expected by legacy virtio devices. When the ring gets active, we force little endian if the device is modern. When the ring is deactivated, we revert to the native endian default. If cross-endian was compiled in, a vq-user_be boolean field is added so that userspace may request a specific endianness. This field is used to override the default when activating the ring of a legacy device. It has no effect on modern devices. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- Changes since v4: - rewrote patch title to mention cross-endian - renamed config to VHOST_CROSS_ENDIAN_LEGACY - rewrote config description and help - moved ifdefery to top of vhost.c - added a detailed comment about the lifecycle of vq-user_be in vhost_init_is_le() - renamed ioctls to VHOST_[GS]ET_VRING_ENDIAN - added LE/BE defines to the ioctl API - rewrote ioctl sanity check with the LE/BE defines - updated comment in uapi/linux/vhost.h to mention that the availibility of both SET and GET ioctls depends on the kernel config drivers/vhost/Kconfig | 15 drivers/vhost/vhost.c | 86 +++- drivers/vhost/vhost.h | 10 + include/uapi/linux/vhost.h | 12 ++ 4 files changed, 121 insertions(+), 2 deletions(-) diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig index 017a1e8..74d7380 100644 --- a/drivers/vhost/Kconfig +++ b/drivers/vhost/Kconfig @@ -32,3 +32,18 @@ config VHOST ---help--- This option is selected by any driver which needs to access the core of vhost. + +config VHOST_CROSS_ENDIAN_LEGACY + bool Cross-endian support for vhost + default n + ---help--- + This option allows vhost to support guests with a different byte + ordering from host. + + Userspace programs can control the feature using the + VHOST_SET_VRING_ENDIAN and VHOST_GET_VRING_ENDIAN ioctls. + + This is only useful on a few platforms (ppc64 and arm64). Since it + adds some overhead, it is disabled default. + + If unsure, say N. diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index 2ee2826..8c4390d 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -36,6 +36,78 @@ enum { #define vhost_used_event(vq) ((__virtio16 __user *)vq-avail-ring[vq-num]) #define vhost_avail_event(vq) ((__virtio16 __user *)vq-used-ring[vq-num]) +#ifdef CONFIG_VHOST_CROSS_ENDIAN_LEGACY +static void vhost_vq_reset_user_be(struct vhost_virtqueue *vq) +{ + vq-user_be = !virtio_legacy_is_little_endian(); +} + +static long vhost_set_vring_endian(struct vhost_virtqueue *vq, int __user *argp) +{ + struct vhost_vring_state s; + + if (vq-private_data) + return -EBUSY; + + if (copy_from_user(s, argp, sizeof(s))) + return -EFAULT; + + if (s.num != VHOST_VRING_LITTLE_ENDIAN + s.num != VHOST_VRING_BIG_ENDIAN) + return -EINVAL; + + vq-user_be = s.num; + + return 0; +} + +static long vhost_get_vring_endian(struct vhost_virtqueue *vq, u32 idx, + int __user *argp) +{ + struct vhost_vring_state s = { + .index = idx, + .num = vq-user_be + }; + + if (copy_to_user(argp, s, sizeof(s))) + return -EFAULT; + + return 0; +} + +static void vhost_init_is_le(struct vhost_virtqueue *vq) +{ + /* Note for legacy virtio: user_be is initialized at reset time +* according to the host endianness. If userspace does not set an +* explicit endianness, the default behavior is native endian, as +* expected by legacy virtio. +*/ + vq-is_le = vhost_has_feature(vq, VIRTIO_F_VERSION_1) || !vq-user_be; +} +#else +static void vhost_vq_reset_user_be(struct vhost_virtqueue *vq) +{ + ; +} + +static long vhost_set_vring_endian(struct vhost_virtqueue *vq, int __user *argp) +{ + return -ENOIOCTLCMD; +} + +static long vhost_get_vring_endian(struct vhost_virtqueue *vq, u32 idx, + int __user *argp) +{ + return -ENOIOCTLCMD; +} + +static void vhost_init_is_le(struct vhost_virtqueue *vq) +{ + if (vhost_has_feature(vq, VIRTIO_F_VERSION_1)) + vq-is_le = true; +} +#endif /* CONFIG_VHOST_CROSS_ENDIAN_LEGACY */ + static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh, poll_table *pt) { @@ -199,6 +271,8 @@ static void vhost_vq_reset(struct vhost_dev *dev, vq-call = NULL; vq-log_ctx = NULL; vq-memory = NULL; + vq-is_le
Re: [PATCH v4 8/8] macvtap/tun: add VNET_BE flag
On Tue, 21 Apr 2015 20:30:23 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Tue, Apr 21, 2015 at 06:22:20PM +0200, Greg Kurz wrote: On Tue, 21 Apr 2015 16:06:33 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Fri, Apr 10, 2015 at 12:20:21PM +0200, Greg Kurz wrote: The VNET_LE flag was introduced to fix accesses to virtio 1.0 headers that are always little-endian. It can also be used to handle the special case of a legacy little-endian device implemented by a big-endian host. Let's add a flag and ioctls for big-endian devices as well. If both flags are set, little-endian wins. Since this is isn't a common usecase, the feature is controlled by a kernel config option (not set by default). Both macvtap and tun are covered by this patch since they share the same API with userland. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/net/Kconfig | 12 drivers/net/macvtap.c | 60 +- drivers/net/tun.c | 62 ++- include/uapi/linux/if_tun.h |2 + 4 files changed, 134 insertions(+), 2 deletions(-) diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index df51d60..f0e23a0 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -244,6 +244,18 @@ config TUN If you don't know what to use this for, you don't need it. +config TUN_VNET_BE + bool Support for big-endian vnet headers + default n + ---help--- + This option allows TUN/TAP and MACVTAP device drivers to parse + vnet headers that are in big-endian byte order. It is useful + when the headers come from a big-endian legacy virtio driver and + the host is little-endian. + + Unless you have a little-endian system hosting a big-endian virtual + machine with a virtio NIC, you should say N. + should mention cross-endian, not big-endian, right? The current TUN_VNET_LE related code is already doing cross-endian: without this patch, one can already run a LE guest on a BE host... wouldn't it be confusing to mention cross-endian only when the guest is BE ? Hmm I think no - LE is also useful for virtio 1 - this is what it was intended for after all. What about having a completely distinct implementation for cross-endian that don't reuse the existing code and defines then ? I think implementation and interface are fine, just the documentation can be improved a bit. How about: Support for cross-endian vnet headers on little-endian kernels. Accordingly CONFIG_TUN_VNET_CROSS_LE ? Sure. And what about also renaming the ioctl to TUNSETVNETCROSSLE then ? -- Greg -- 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 v4 7/8] vhost: feature to set the vring endianness
On Tue, 21 Apr 2015 20:25:03 +0200 Michael S. Tsirkin m...@redhat.com wrote: [ ... ] @@ -630,6 +634,53 @@ static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m) return 0; } +#ifdef CONFIG_VHOST_SET_ENDIAN_LEGACY +static long vhost_set_vring_big_endian(struct vhost_virtqueue *vq, + int __user *argp) +{ + struct vhost_vring_state s; + + if (vq-private_data) + return -EBUSY; + + if (copy_from_user(s, argp, sizeof(s))) + return -EFAULT; + + if (s.num s.num != 1) s.num ~0x1 Since s.num is unsigned and I assume this won't change, what about s.num 1 as suggested by Cornelia ? I just tried and gcc optimizes s.num != 0 s.num != 1 to s.num 1 The former will be more readable once we replace 0 and 1 with defines. So ignore my advice, keep code as is but use defines. Ok. [ ... ] --- a/include/uapi/linux/vhost.h +++ b/include/uapi/linux/vhost.h @@ -103,6 +103,15 @@ struct vhost_memory { /* Get accessor: reads index, writes value in num */ #define VHOST_GET_VRING_BASE _IOWR(VHOST_VIRTIO, 0x12, struct vhost_vring_state) +/* Set the vring byte order in num. This is a legacy only API that is simply + * ignored when VIRTIO_F_VERSION_1 is set. + * 0 to set to little-endian + * 1 to set to big-endian How about defines for these? Ok. I'll put the defines here so that all the cross-endian stuff lies in the same hunk. Is it ok for you ? Fine. + * other values return EINVAL. Pls also add a note saying that not all kernel configurations support this ioctl, but all configurations that support SET also support GET. Ok. + */ +#define VHOST_SET_VRING_BIG_ENDIAN _IOW(VHOST_VIRTIO, 0x13, struct vhost_vring_state) +#define VHOST_GET_VRING_BIG_ENDIAN _IOW(VHOST_VIRTIO, 0x14, struct vhost_vring_state) + /* The following ioctls use eventfd file descriptors to signal and poll * for events. */ I'm inclined to think VHOST_SET_VRING_ENDIAN is a slightly better name. What do you think? Or VHOST_SET_VRING_CROSS_ENDIAN ? I like the idea to keep a hint that this API is for cross-endian only... like the rest of this series. -- Greg -- 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 v4 0/8] vhost: support for cross endian guests
On Fri, 17 Apr 2015 11:18:13 +0200 Greg Kurz gk...@linux.vnet.ibm.com wrote: On Fri, 10 Apr 2015 12:15:00 +0200 Greg Kurz gk...@linux.vnet.ibm.com wrote: Hi, This patchset allows vhost to be used with legacy virtio when guest and host have a different endianness. Patch 7 got rewritten according to Cornelia's and Michael's comments. I have also introduced patch 8 that brings BE vnet headers support to tun/macvtap. This series is enough to have vhost_net working flawlessly. I could succesfully reboot guests from ppc64 to ppc64le and vice-versa on ppc64 and ppc64le hosts. Michael, I am ready to post v5 with the changes suggested by Cornelia... Do you have any extra comments on this v4 ? Ping ? -- Greg --- Greg Kurz (8): virtio: introduce virtio_is_little_endian() helper tun: add tun_is_little_endian() helper macvtap: introduce macvtap_is_little_endian() helper vringh: introduce vringh_is_little_endian() helper vhost: introduce vhost_is_little_endian() helper virtio: add explicit big-endian support to memory accessors vhost: feature to set the vring endianness macvtap/tun: add VNET_BE flag drivers/net/Kconfig | 12 ++ drivers/net/macvtap.c| 69 ++- drivers/net/tun.c| 71 +++- drivers/vhost/Kconfig| 10 + drivers/vhost/vhost.c| 76 ++ drivers/vhost/vhost.h| 25 ++--- include/linux/virtio_byteorder.h | 24 +++- include/linux/virtio_config.h| 19 +++--- include/linux/vringh.h | 19 +++--- include/uapi/linux/if_tun.h |2 + include/uapi/linux/vhost.h |9 + 11 files changed, 303 insertions(+), 33 deletions(-) -- Greg ___ Virtualization mailing list virtualizat...@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization -- 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 6/7] virtio: add explicit big-endian support to memory accessors
On Tue, 7 Apr 2015 17:56:25 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Tue, Apr 07, 2015 at 02:15:52PM +0200, Greg Kurz wrote: The current memory accessors logic is: - little endian if little_endian - native endian (i.e. no byteswap) if !little_endian If we want to fully support cross-endian vhost, we also need to be able to convert to big endian. Instead of changing the little_endian argument to some 3-value enum, this patch changes the logic to: - little endian if little_endian - big endian if !little_endian The native endian case is handled by all users with a trivial helper. This patch doesn't change any functionality, nor it does add overhead. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/net/macvtap.c|4 +++- drivers/net/tun.c|4 +++- drivers/vhost/vhost.h|4 +++- include/linux/virtio_byteorder.h | 24 ++-- include/linux/virtio_config.h|4 +++- include/linux/vringh.h |4 +++- 6 files changed, 29 insertions(+), 15 deletions(-) diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c index a2f2958..0a03a66 100644 --- a/drivers/net/macvtap.c +++ b/drivers/net/macvtap.c @@ -51,7 +51,9 @@ struct macvtap_queue { static inline bool macvtap_is_little_endian(struct macvtap_queue *q) { - return q-flags MACVTAP_VNET_LE; + if (q-flags MACVTAP_VNET_LE) + return true; + return virtio_legacy_is_little_endian(); } static inline u16 macvtap16_to_cpu(struct macvtap_queue *q, __virtio16 val) Hmm I'm not sure how well this will work once you actually make it dynamic. Remains to be seen. Oops I overlooked this mail... FWIW I could reboot/kexec from a ppc64 guest to ppc64le and back with the following QEMU: https://github.com/gkurz/qemu/commits/vhost/cross-endian diff --git a/drivers/net/tun.c b/drivers/net/tun.c index 3c3d6c0..053f9b6 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -208,7 +208,9 @@ struct tun_struct { static inline bool tun_is_little_endian(struct tun_struct *tun) { - return tun-flags TUN_VNET_LE; + if (tun-flags TUN_VNET_LE) + return true; + return virtio_legacy_is_little_endian(); } static inline u16 tun16_to_cpu(struct tun_struct *tun, __virtio16 val) diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h index 6a49960..4e9a186 100644 --- a/drivers/vhost/vhost.h +++ b/drivers/vhost/vhost.h @@ -175,7 +175,9 @@ static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit) static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq) { - return vhost_has_feature(vq, VIRTIO_F_VERSION_1); + if (vhost_has_feature(vq, VIRTIO_F_VERSION_1)) + return true; + return virtio_legacy_is_little_endian(); } /* Memory accessors */ diff --git a/include/linux/virtio_byteorder.h b/include/linux/virtio_byteorder.h index 51865d0..ce63a2c 100644 --- a/include/linux/virtio_byteorder.h +++ b/include/linux/virtio_byteorder.h @@ -3,17 +3,21 @@ #include linux/types.h #include uapi/linux/virtio_types.h -/* - * Low-level memory accessors for handling virtio in modern little endian and in - * compatibility native endian format. - */ +static inline bool virtio_legacy_is_little_endian(void) +{ +#ifdef __LITTLE_ENDIAN + return true; +#else + return false; +#endif +} static inline u16 __virtio16_to_cpu(bool little_endian, __virtio16 val) { if (little_endian) return le16_to_cpu((__force __le16)val); else - return (__force u16)val; + return be16_to_cpu((__force __be16)val); } static inline __virtio16 __cpu_to_virtio16(bool little_endian, u16 val) @@ -21,7 +25,7 @@ static inline __virtio16 __cpu_to_virtio16(bool little_endian, u16 val) if (little_endian) return (__force __virtio16)cpu_to_le16(val); else - return (__force __virtio16)val; + return (__force __virtio16)cpu_to_be16(val); } static inline u32 __virtio32_to_cpu(bool little_endian, __virtio32 val) @@ -29,7 +33,7 @@ static inline u32 __virtio32_to_cpu(bool little_endian, __virtio32 val) if (little_endian) return le32_to_cpu((__force __le32)val); else - return (__force u32)val; + return be32_to_cpu((__force __be32)val); } static inline __virtio32 __cpu_to_virtio32(bool little_endian, u32 val) @@ -37,7 +41,7 @@ static inline __virtio32 __cpu_to_virtio32(bool little_endian, u32 val) if (little_endian) return (__force __virtio32)cpu_to_le32(val); else - return (__force __virtio32)val; + return (__force __virtio32)cpu_to_be32(val); } static inline u64 __virtio64_to_cpu(bool little_endian
Re: [PATCH v4 7/8] vhost: feature to set the vring endianness
On Tue, 21 Apr 2015 16:04:23 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Fri, Apr 10, 2015 at 12:19:16PM +0200, Greg Kurz wrote: This patch brings cross-endian support to vhost when used to implement legacy virtio devices. Since it is a relatively rare situation, the feature availability is controlled by a kernel config option (not set by default). The vq-is_le boolean field is added to cache the endianness to be used for ring accesses. It defaults to native endian, as expected by legacy virtio devices. When the ring gets active, we force little endian if the device is modern. When the ring is deactivated, we revert to the native endian default. If cross-endian was compiled in, a vq-user_be boolean field is added so that userspace may request a specific endianness. This field is used to override the default when activating the ring of a legacy device. It has no effect on modern devices. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/vhost/Kconfig | 10 ++the drivers/vhost/vhost.c | 76 +++- drivers/vhost/vhost.h | 12 +-- include/uapi/linux/vhost.h |9 + 4 files changed, 103 insertions(+), 4 deletions(-) Changes since v3: - VHOST_SET_VRING_ENDIAN_LEGACY ioctl renamed to VHOST_SET_VRING_BIG_ENDIAN - ioctl API is now: 0 for le, 1 for be, other values are EINVAL - ioctl doesn't filter out modern devices - ioctl stubs return ENOIOCTLCMD - forbid endianness changes when vring is active - logic now handled with vq-is_le and vq-user_be according to device start/stop as suggested by Michael diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig index 017a1e8..0aec88c 100644 --- a/drivers/vhost/Kconfig +++ b/drivers/vhost/Kconfig @@ -32,3 +32,13 @@ config VHOST ---help--- This option is selected by any driver which needs to access the core of vhost. + +config VHOST_SET_ENDIAN_LEGACY I'd prefer namin this VHOST_CROSS_ENDIAN_LEGACY Yes, makes sense. I'll make sure most of the cross-endian changes are easy to grep. + bool Cross-endian support for host kernel accelerator + default n + ---help--- + This option allows vhost to support guests with a different byte + ordering from host. It is disabled by default since it adds overhead + and it is only needed by a few platforms (powerpc and arm). and is only useful on a few platforms (powerpc and arm). it seems to refer to overhead, which is rarely needed. needed is a bit too strong, you can always e.g. run virtio in userspace. My poor English again... it seems you understood what I wanted to write though :) + + If unsure, say N. diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index 2ee2826..3eb756b 100644the --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -199,6 +199,10 @@ static void vhost_vq_reset(struct vhost_dev *dev, vq-call = NULL; vq-log_ctx = NULL; vq-memory = NULL; + vq-is_le = virtio_legacy_is_little_endian(); +#ifdef CONFIG_VHOST_SET_ENDIAN_LEGACY + vq-user_be = !vq-is_le; +#endif add a wrapper for this too? Will do. } static int vhost_worker(void *data) @@ -630,6 +634,53 @@ static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m) return 0; } +#ifdef CONFIG_VHOST_SET_ENDIAN_LEGACY +static long vhost_set_vring_big_endian(struct vhost_virtqueue *vq, + int __user *argp) +{ + struct vhost_vring_state s; + + if (vq-private_data) + return -EBUSY; + + if (copy_from_user(s, argp, sizeof(s))) + return -EFAULT; + + if (s.num s.num != 1) s.num ~0x1 Since s.num is unsigned and I assume this won't change, what about s.num 1 as suggested by Cornelia ? + return -EINVAL; + + vq-user_be = s.num; + + return 0; +} + +static long vhost_get_vring_big_endian(struct vhost_virtqueue *vq, u32 idx, + int __user *argp) +{ + struct vhost_vring_state s = { + .index = idx, + .num = vq-user_be + }; + + if (copy_to_user(argp, s, sizeof(s))) + return -EFAULT; + + return 0; +} +#else +static long vhost_set_vring_big_endian(struct vhost_virtqueue *vq, + int __user *argp) +{ + return -ENOIOCTLCMD; +} + +static long vhost_get_vring_big_endian(struct vhost_virtqueue *vq, u32 idx, + int __user *argp) +{ + return -ENOIOCTLCMD; +} +#endif /* CONFIG_VHOST_SET_ENDIAN_LEGACY */ + long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp) { struct file *eventfp, *filep = NULL; @@ -806,6 +857,12 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp
Re: [PATCH v4 6/8] virtio: add explicit big-endian support to memory accessors
On Tue, 21 Apr 2015 16:09:44 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Fri, Apr 10, 2015 at 12:16:20PM +0200, Greg Kurz wrote: The current memory accessors logic is: - little endian if little_endian - native endian (i.e. no byteswap) if !little_endian If we want to fully support cross-endian vhost, we also need to be able to convert to big endian. Instead of changing the little_endian argument to some 3-value enum, this patch changes the logic to: - little endian if little_endian - big endian if !little_endian The native endian case is handled by all users with a trivial helper. This patch doesn't change any functionality, nor it does add overhead. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- OK overall. Style comment: drivers/net/macvtap.c|4 +++- drivers/net/tun.c|4 +++- drivers/vhost/vhost.h|4 +++- include/linux/virtio_byteorder.h | 24 ++-- include/linux/virtio_config.h|4 +++- include/linux/vringh.h |4 +++- 6 files changed, 29 insertions(+), 15 deletions(-) diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c index a2f2958..0a03a66 100644 --- a/drivers/net/macvtap.c +++ b/drivers/net/macvtap.c @@ -51,7 +51,9 @@ struct macvtap_queue { static inline bool macvtap_is_little_endian(struct macvtap_queue *q) { - return q-flags MACVTAP_VNET_LE; + if (q-flags MACVTAP_VNET_LE) + return true; + return virtio_legacy_is_little_endian(); } I'd prefer a bit more symmetry: + if (q-flags MACVTAP_VNET_LE) + return true; + else + return virtio_legacy_is_little_endian(); Or better just: return (q-flags MACVTAP_VNET_LE) ? true : virtio_legacy_is_little_endian(); might make line long, but your follow-up patch makes it short again, so that's ok. Will do. static inline u16 macvtap16_to_cpu(struct macvtap_queue *q, __virtio16 val) diff --git a/drivers/net/tun.c b/drivers/net/tun.c index 3c3d6c0..053f9b6 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -208,7 +208,9 @@ struct tun_struct { static inline bool tun_is_little_endian(struct tun_struct *tun) { - return tun-flags TUN_VNET_LE; + if (tun-flags TUN_VNET_LE) + return true; + return virtio_legacy_is_little_endian(); } static inline u16 tun16_to_cpu(struct tun_struct *tun, __virtio16 val) diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h index 6a49960..4e9a186 100644 --- a/drivers/vhost/vhost.h +++ b/drivers/vhost/vhost.h @@ -175,7 +175,9 @@ static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit) static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq) { - return vhost_has_feature(vq, VIRTIO_F_VERSION_1); + if (vhost_has_feature(vq, VIRTIO_F_VERSION_1)) + return true; + return virtio_legacy_is_little_endian(); } /* Memory accessors */ diff --git a/include/linux/virtio_byteorder.h b/include/linux/virtio_byteorder.h index 51865d0..ce63a2c 100644 --- a/include/linux/virtio_byteorder.h +++ b/include/linux/virtio_byteorder.h @@ -3,17 +3,21 @@ #include linux/types.h #include uapi/linux/virtio_types.h -/* - * Low-level memory accessors for handling virtio in modern little endian and in - * compatibility native endian format. - */ +static inline bool virtio_legacy_is_little_endian(void) +{ +#ifdef __LITTLE_ENDIAN + return true; +#else + return false; +#endif +} static inline u16 __virtio16_to_cpu(bool little_endian, __virtio16 val) { if (little_endian) return le16_to_cpu((__force __le16)val); else - return (__force u16)val; + return be16_to_cpu((__force __be16)val); } static inline __virtio16 __cpu_to_virtio16(bool little_endian, u16 val) @@ -21,7 +25,7 @@ static inline __virtio16 __cpu_to_virtio16(bool little_endian, u16 val) if (little_endian) return (__force __virtio16)cpu_to_le16(val); else - return (__force __virtio16)val; + return (__force __virtio16)cpu_to_be16(val); } static inline u32 __virtio32_to_cpu(bool little_endian, __virtio32 val) @@ -29,7 +33,7 @@ static inline u32 __virtio32_to_cpu(bool little_endian, __virtio32 val) if (little_endian) return le32_to_cpu((__force __le32)val); else - return (__force u32)val; + return be32_to_cpu((__force __be32)val); } static inline __virtio32 __cpu_to_virtio32(bool little_endian, u32 val) @@ -37,7 +41,7 @@ static inline __virtio32 __cpu_to_virtio32(bool little_endian, u32 val) if (little_endian) return (__force __virtio32)cpu_to_le32(val); else - return (__force __virtio32)val
Re: [PATCH v4 8/8] macvtap/tun: add VNET_BE flag
On Tue, 21 Apr 2015 16:06:33 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Fri, Apr 10, 2015 at 12:20:21PM +0200, Greg Kurz wrote: The VNET_LE flag was introduced to fix accesses to virtio 1.0 headers that are always little-endian. It can also be used to handle the special case of a legacy little-endian device implemented by a big-endian host. Let's add a flag and ioctls for big-endian devices as well. If both flags are set, little-endian wins. Since this is isn't a common usecase, the feature is controlled by a kernel config option (not set by default). Both macvtap and tun are covered by this patch since they share the same API with userland. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/net/Kconfig | 12 drivers/net/macvtap.c | 60 +- drivers/net/tun.c | 62 ++- include/uapi/linux/if_tun.h |2 + 4 files changed, 134 insertions(+), 2 deletions(-) diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index df51d60..f0e23a0 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -244,6 +244,18 @@ config TUN If you don't know what to use this for, you don't need it. +config TUN_VNET_BE + bool Support for big-endian vnet headers + default n + ---help--- + This option allows TUN/TAP and MACVTAP device drivers to parse + vnet headers that are in big-endian byte order. It is useful + when the headers come from a big-endian legacy virtio driver and + the host is little-endian. + + Unless you have a little-endian system hosting a big-endian virtual + machine with a virtio NIC, you should say N. + should mention cross-endian, not big-endian, right? The current TUN_VNET_LE related code is already doing cross-endian: without this patch, one can already run a LE guest on a BE host... wouldn't it be confusing to mention cross-endian only when the guest is BE ? What about having a completely distinct implementation for cross-endian that don't reuse the existing code and defines then ? config VETH tristate Virtual ethernet pair device ---help--- diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c index 0a03a66..e0ab1b7 100644 --- a/drivers/net/macvtap.c +++ b/drivers/net/macvtap.c @@ -48,12 +48,27 @@ struct macvtap_queue { #define MACVTAP_FEATURES (IFF_VNET_HDR | IFF_MULTI_QUEUE) #define MACVTAP_VNET_LE 0x8000 +#define MACVTAP_VNET_BE 0x4000 + +#ifdef CONFIG_TUN_VNET_BE +static inline bool macvtap_legacy_is_little_endian(struct macvtap_queue *q) +{ + if (q-flags MACVTAP_VNET_BE) + return false; + return virtio_legacy_is_little_endian(); +} +#else +static inline bool macvtap_legacy_is_little_endian(struct macvtap_queue *q) +{ + return virtio_legacy_is_little_endian(); +} +#endif static inline bool macvtap_is_little_endian(struct macvtap_queue *q) { if (q-flags MACVTAP_VNET_LE) return true; - return virtio_legacy_is_little_endian(); + return macvtap_legacy_is_little_endian(q); } static inline u16 macvtap16_to_cpu(struct macvtap_queue *q, __virtio16 val) @@ -1000,6 +1015,43 @@ static int set_offload(struct macvtap_queue *q, unsigned long arg) return 0; } +#ifdef CONFIG_TUN_VNET_BE +static long macvtap_get_vnet_be(struct macvtap_queue *q, int __user *sp) +{ + int s = !!(q-flags MACVTAP_VNET_BE); + + if (put_user(s, sp)) + return -EFAULT; + + return 0; +} + +static long macvtap_set_vnet_be(struct macvtap_queue *q, int __user *sp) +{ + int s; + + if (get_user(s, sp)) + return -EFAULT; + + if (s) + q-flags |= MACVTAP_VNET_BE; + else + q-flags = ~MACVTAP_VNET_BE; + + return 0; +} +#else +static long macvtap_get_vnet_be(struct macvtap_queue *q, int __user *argp) +{ + return -EINVAL; +} + +static long macvtap_set_vnet_be(struct macvtap_queue *q, int __user *argp) +{ + return -EINVAL; +} +#endif /* CONFIG_TUN_VNET_BE */ + /* * provide compatibility with generic tun/tap interface */ @@ -1097,6 +1149,12 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd, q-flags = ~MACVTAP_VNET_LE; return 0; + case TUNGETVNETBE: + return macvtap_get_vnet_be(q, sp); + + case TUNSETVNETBE: + return macvtap_set_vnet_be(q, sp); + case TUNSETOFFLOAD: /* let the user check for future flags */ if (arg ~(TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 | diff --git a/drivers/net/tun.c b/drivers/net/tun.c index 053f9b6..4e12488 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -111,6 +111,7 @@ do
Re: [PATCH v4 0/8] vhost: support for cross endian guests
On Tue, 21 Apr 2015 16:10:18 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Fri, Apr 10, 2015 at 12:15:00PM +0200, Greg Kurz wrote: Hi, This patchset allows vhost to be used with legacy virtio when guest and host have a different endianness. Patch 7 got rewritten according to Cornelia's and Michael's comments. I have also introduced patch 8 that brings BE vnet headers support to tun/macvtap. This series is enough to have vhost_net working flawlessly. I could succesfully reboot guests from ppc64 to ppc64le and vice-versa on ppc64 and ppc64le hosts. Looks good overall. A couple of style comments. Thanks! Thanks for your time Michael. --- Greg Kurz (8): virtio: introduce virtio_is_little_endian() helper tun: add tun_is_little_endian() helper macvtap: introduce macvtap_is_little_endian() helper vringh: introduce vringh_is_little_endian() helper vhost: introduce vhost_is_little_endian() helper virtio: add explicit big-endian support to memory accessors vhost: feature to set the vring endianness macvtap/tun: add VNET_BE flag drivers/net/Kconfig | 12 ++ drivers/net/macvtap.c| 69 ++- drivers/net/tun.c| 71 +++- drivers/vhost/Kconfig| 10 + drivers/vhost/vhost.c| 76 ++ drivers/vhost/vhost.h| 25 ++--- include/linux/virtio_byteorder.h | 24 +++- include/linux/virtio_config.h| 19 +++--- include/linux/vringh.h | 19 +++--- include/uapi/linux/if_tun.h |2 + include/uapi/linux/vhost.h |9 + 11 files changed, 303 insertions(+), 33 deletions(-) -- Greg -- 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 v4 0/8] vhost: support for cross endian guests
On Fri, 10 Apr 2015 12:15:00 +0200 Greg Kurz gk...@linux.vnet.ibm.com wrote: Hi, This patchset allows vhost to be used with legacy virtio when guest and host have a different endianness. Patch 7 got rewritten according to Cornelia's and Michael's comments. I have also introduced patch 8 that brings BE vnet headers support to tun/macvtap. This series is enough to have vhost_net working flawlessly. I could succesfully reboot guests from ppc64 to ppc64le and vice-versa on ppc64 and ppc64le hosts. Michael, I am ready to post v5 with the changes suggested by Cornelia... Do you have any extra comments on this v4 ? -- Greg --- Greg Kurz (8): virtio: introduce virtio_is_little_endian() helper tun: add tun_is_little_endian() helper macvtap: introduce macvtap_is_little_endian() helper vringh: introduce vringh_is_little_endian() helper vhost: introduce vhost_is_little_endian() helper virtio: add explicit big-endian support to memory accessors vhost: feature to set the vring endianness macvtap/tun: add VNET_BE flag drivers/net/Kconfig | 12 ++ drivers/net/macvtap.c| 69 ++- drivers/net/tun.c| 71 +++- drivers/vhost/Kconfig| 10 + drivers/vhost/vhost.c| 76 ++ drivers/vhost/vhost.h| 25 ++--- include/linux/virtio_byteorder.h | 24 +++- include/linux/virtio_config.h| 19 +++--- include/linux/vringh.h | 19 +++--- include/uapi/linux/if_tun.h |2 + include/uapi/linux/vhost.h |9 + 11 files changed, 303 insertions(+), 33 deletions(-) -- Greg ___ Virtualization mailing list virtualizat...@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization -- 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 v4 7/8] vhost: feature to set the vring endianness
On Tue, 14 Apr 2015 16:20:23 +0200 Cornelia Huck cornelia.h...@de.ibm.com wrote: On Fri, 10 Apr 2015 12:19:16 +0200 Greg Kurz gk...@linux.vnet.ibm.com wrote: This patch brings cross-endian support to vhost when used to implement legacy virtio devices. Since it is a relatively rare situation, the feature availability is controlled by a kernel config option (not set by default). The vq-is_le boolean field is added to cache the endianness to be used for ring accesses. It defaults to native endian, as expected by legacy virtio devices. When the ring gets active, we force little endian if the device is modern. When the ring is deactivated, we revert to the native endian default. If cross-endian was compiled in, a vq-user_be boolean field is added so that userspace may request a specific endianness. This field is used to override the default when activating the ring of a legacy device. It has no effect on modern devices. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/vhost/Kconfig | 10 ++ drivers/vhost/vhost.c | 76 +++- drivers/vhost/vhost.h | 12 +-- include/uapi/linux/vhost.h |9 + 4 files changed, 103 insertions(+), 4 deletions(-) +#ifdef CONFIG_VHOST_SET_ENDIAN_LEGACY +static long vhost_set_vring_big_endian(struct vhost_virtqueue *vq, + int __user *argp) +{ + struct vhost_vring_state s; + + if (vq-private_data) + return -EBUSY; + + if (copy_from_user(s, argp, sizeof(s))) + return -EFAULT; + + if (s.num s.num != 1) Checking for s.num 1 might be more obvious at a glance? Sure since s.num is unsigned. + return -EINVAL; + + vq-user_be = s.num; + + return 0; +} + (...) +#ifdef CONFIG_VHOST_SET_ENDIAN_LEGACY +static void vhost_init_is_le(struct vhost_virtqueue *vq) +{ + vq-is_le = vhost_has_feature(vq, VIRTIO_F_VERSION_1) || !vq-user_be; So if the endianness is not explicitly set to BE, it will be forced to LE (instead of native endian)? Won't that break userspace that does not yet use the new interface when CONFIG_VHOST_SET_ENDIAN_LEGACY is set? If userspace doesn't use the new interface, then vq-user_be will retain its default value that was set in vhost_vq_reset(), i.e. : vq-user_be = !virtio_legacy_is_little_endian(); This means default is native endian. What about adding this comment ? static void vhost_init_is_le(struct vhost_virtqueue *vq) { /* Note for legacy virtio: user_be is initialized in vhost_vq_reset() * according to the host endianness. If userspace does not set an * explicit endianness, the default behavior is native endian, as * expected by legacy virtio. */ vq-is_le = vhost_has_feature(vq, VIRTIO_F_VERSION_1) || !vq-user_be; } +} +#else +static void vhost_init_is_le(struct vhost_virtqueue *vq) +{ + if (vhost_has_feature(vq, VIRTIO_F_VERSION_1)) + vq-is_le = true; +} +#endif + int vhost_init_used(struct vhost_virtqueue *vq) { __virtio16 last_used_idx; int r; - if (!vq-private_data) + if (!vq-private_data) { + vq-is_le = virtio_legacy_is_little_endian(); return 0; + } + + vhost_init_is_le(vq); r = vhost_update_used_flags(vq); if (r) -- 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 7/7] vhost: feature to set the vring endianness
On Tue, 7 Apr 2015 17:52:28 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Tue, Apr 07, 2015 at 02:19:31PM +0200, Greg Kurz wrote: This patch brings cross-endian support to vhost when used to implement legacy virtio devices. Since it is a relatively rare situation, the feature availability is controlled by a kernel config option (not set by default). The ioctls introduced by this patch are for legacy only: virtio 1.0 devices are returned EPERM. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com EINVAL probably makes more sense? I had choosen EPERM because the error isn't related to the arguments being passed by userspace. It simply does not make sense to set the vring endianness for a virtio 1.0 device. That being said, I am perfectly fine with EINVAL. :) --- drivers/vhost/Kconfig | 10 drivers/vhost/vhost.c | 55 drivers/vhost/vhost.h | 17 +- include/uapi/linux/vhost.h |5 4 files changed, 86 insertions(+), 1 deletion(-) Changes since v2: - fixed typos in Kconfig description - renamed vq-legacy_big_endian to vq-legacy_is_little_endian - vq-legacy_is_little_endian reset to default in vhost_vq_reset() - dropped VHOST_F_SET_ENDIAN_LEGACY feature - dropped struct vhost_vring_endian from the user API (re-use struct vhost_vring_state instead) - added VHOST_GET_VRING_ENDIAN_LEGACY ioctl - introduced more helpers and stubs to avoid polluting the code with ifdefs diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig index 017a1e8..0aec88c 100644 --- a/drivers/vhost/Kconfig +++ b/drivers/vhost/Kconfig @@ -32,3 +32,13 @@ config VHOST ---help--- This option is selected by any driver which needs to access the core of vhost. + +config VHOST_SET_ENDIAN_LEGACY + bool Cross-endian support for host kernel accelerator + default n + ---help--- + This option allows vhost to support guests with a different byte + ordering from host. It is disabled by default since it adds overhead + and it is only needed by a few platforms (powerpc and arm). + + If unsure, say N. diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index 2ee2826..3529a3c 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -199,6 +199,7 @@ static void vhost_vq_reset(struct vhost_dev *dev, vq-call = NULL; vq-log_ctx = NULL; vq-memory = NULL; + vq-legacy_is_little_endian = virtio_legacy_is_little_endian(); } static int vhost_worker(void *data) @@ -630,6 +631,54 @@ static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m) return 0; } +#ifdef CONFIG_VHOST_SET_ENDIAN_LEGACY +static long vhost_set_vring_endian_legacy(struct vhost_virtqueue *vq, + void __user *argp) +{ + struct vhost_vring_state s; + + if (vhost_has_feature(vq, VIRTIO_F_VERSION_1)) + return -EPERM; EINVAL probably makes more sense? But I'm not sure this is helpful: one can set VIRTIO_F_VERSION_1 afterwards, and your patch does not seem to detect this. Yeah, when I dropped the *bogus* feature from v2, I forgot to patch VHOST_SET_FEATURES accordingly... But thinking about it now, the choice to error out when setting VIRTIO_F_VERSION_1 because cross-endian legacy was set before looks terrible... :-\ + + if (copy_from_user(s, argp, sizeof(s))) + return -EFAULT; + + vq-legacy_is_little_endian = !!s.num; + return 0; +} + +static long vhost_get_vring_endian_legacy(struct vhost_virtqueue *vq, + u32 idx, + void __user *argp) +{ + struct vhost_vring_state s = { + .index = idx, + .num = vq-legacy_is_little_endian + }; + + if (vhost_has_feature(vq, VIRTIO_F_VERSION_1)) + return -EPERM; + + if (copy_to_user(argp, s, sizeof(s))) + return -EFAULT; + + return 0; +} +#else +static long vhost_set_vring_endian_legacy(struct vhost_virtqueue *vq, + void __user *argp) +{ + return 0; +} + +static long vhost_get_vring_endian_legacy(struct vhost_virtqueue *vq, + u32 idx, + void __user *argp) +{ + return 0; +} Should be -ENOIOCTLCMD? Sure. +#endif /* CONFIG_VHOST_SET_ENDIAN_LEGACY */ + long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp) { struct file *eventfp, *filep = NULL; @@ -806,6 +855,12 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp) } else filep = eventfp; break; + case VHOST_SET_VRING_ENDIAN_LEGACY: + r
Re: [PATCH v3 7/7] vhost: feature to set the vring endianness
On Tue, 7 Apr 2015 17:01:31 +0200 Cornelia Huck cornelia.h...@de.ibm.com wrote: On Tue, 07 Apr 2015 14:19:31 +0200 Greg Kurz gk...@linux.vnet.ibm.com wrote: This patch brings cross-endian support to vhost when used to implement legacy virtio devices. Since it is a relatively rare situation, the feature availability is controlled by a kernel config option (not set by default). The ioctls introduced by this patch are for legacy only: virtio 1.0 devices are returned EPERM. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/vhost/Kconfig | 10 drivers/vhost/vhost.c | 55 drivers/vhost/vhost.h | 17 +- include/uapi/linux/vhost.h |5 4 files changed, 86 insertions(+), 1 deletion(-) +#ifdef CONFIG_VHOST_SET_ENDIAN_LEGACY +static long vhost_set_vring_endian_legacy(struct vhost_virtqueue *vq, + void __user *argp) +{ + struct vhost_vring_state s; + + if (vhost_has_feature(vq, VIRTIO_F_VERSION_1)) + return -EPERM; + + if (copy_from_user(s, argp, sizeof(s))) + return -EFAULT; + + vq-legacy_is_little_endian = !!s.num; + return 0; +} + +static long vhost_get_vring_endian_legacy(struct vhost_virtqueue *vq, + u32 idx, + void __user *argp) +{ + struct vhost_vring_state s = { + .index = idx, + .num = vq-legacy_is_little_endian + }; + + if (vhost_has_feature(vq, VIRTIO_F_VERSION_1)) + return -EPERM; + + if (copy_to_user(argp, s, sizeof(s))) + return -EFAULT; + + return 0; +} +#else +static long vhost_set_vring_endian_legacy(struct vhost_virtqueue *vq, + void __user *argp) +{ + return 0; I'm wondering whether this handler should return an error if the feature is not configured for the kernel? How can the userspace caller find out whether it has successfully prompted the kernel to handle the endianness correctly? Yes you're right. I think -ENOIOCTLCMD as suggested by Michael is a good candidate. Thanks. +} + +static long vhost_get_vring_endian_legacy(struct vhost_virtqueue *vq, + u32 idx, + void __user *argp) +{ + return 0; +} +#endif /* CONFIG_VHOST_SET_ENDIAN_LEGACY */ -- 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 7/7] vhost: feature to set the vring endianness
On Tue, 7 Apr 2015 18:11:29 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Tue, Apr 07, 2015 at 02:19:31PM +0200, Greg Kurz wrote: This patch brings cross-endian support to vhost when used to implement legacy virtio devices. Since it is a relatively rare situation, the feature availability is controlled by a kernel config option (not set by default). The ioctls introduced by this patch are for legacy only: virtio 1.0 devices are returned EPERM. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/vhost/Kconfig | 10 drivers/vhost/vhost.c | 55 drivers/vhost/vhost.h | 17 +- include/uapi/linux/vhost.h |5 4 files changed, 86 insertions(+), 1 deletion(-) Changes since v2: - fixed typos in Kconfig description - renamed vq-legacy_big_endian to vq-legacy_is_little_endian - vq-legacy_is_little_endian reset to default in vhost_vq_reset() - dropped VHOST_F_SET_ENDIAN_LEGACY feature - dropped struct vhost_vring_endian from the user API (re-use struct vhost_vring_state instead) - added VHOST_GET_VRING_ENDIAN_LEGACY ioctl - introduced more helpers and stubs to avoid polluting the code with ifdefs diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig index 017a1e8..0aec88c 100644 --- a/drivers/vhost/Kconfig +++ b/drivers/vhost/Kconfig @@ -32,3 +32,13 @@ config VHOST ---help--- This option is selected by any driver which needs to access the core of vhost. + +config VHOST_SET_ENDIAN_LEGACY + bool Cross-endian support for host kernel accelerator + default n + ---help--- + This option allows vhost to support guests with a different byte + ordering from host. It is disabled by default since it adds overhead + and it is only needed by a few platforms (powerpc and arm). + + If unsure, say N. diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index 2ee2826..3529a3c 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -199,6 +199,7 @@ static void vhost_vq_reset(struct vhost_dev *dev, vq-call = NULL; vq-log_ctx = NULL; vq-memory = NULL; + vq-legacy_is_little_endian = virtio_legacy_is_little_endian(); } static int vhost_worker(void *data) @@ -630,6 +631,54 @@ static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m) return 0; } +#ifdef CONFIG_VHOST_SET_ENDIAN_LEGACY +static long vhost_set_vring_endian_legacy(struct vhost_virtqueue *vq, + void __user *argp) +{ + struct vhost_vring_state s; + + if (vhost_has_feature(vq, VIRTIO_F_VERSION_1)) + return -EPERM; + + if (copy_from_user(s, argp, sizeof(s))) + return -EFAULT; + + vq-legacy_is_little_endian = !!s.num; + return 0; +} + +static long vhost_get_vring_endian_legacy(struct vhost_virtqueue *vq, + u32 idx, + void __user *argp) +{ + struct vhost_vring_state s = { + .index = idx, + .num = vq-legacy_is_little_endian + }; + + if (vhost_has_feature(vq, VIRTIO_F_VERSION_1)) + return -EPERM; + + if (copy_to_user(argp, s, sizeof(s))) + return -EFAULT; + + return 0; +} +#else +static long vhost_set_vring_endian_legacy(struct vhost_virtqueue *vq, + void __user *argp) +{ + return 0; +} + +static long vhost_get_vring_endian_legacy(struct vhost_virtqueue *vq, + u32 idx, + void __user *argp) +{ + return 0; +} +#endif /* CONFIG_VHOST_SET_ENDIAN_LEGACY */ + long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp) { struct file *eventfp, *filep = NULL; @@ -806,6 +855,12 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp) } else filep = eventfp; break; + case VHOST_SET_VRING_ENDIAN_LEGACY: + r = vhost_set_vring_endian_legacy(vq, argp); + break; + case VHOST_GET_VRING_ENDIAN_LEGACY: + r = vhost_get_vring_endian_legacy(vq, idx, argp); + break; default: r = -ENOIOCTLCMD; } diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h index 4e9a186..981ba06 100644 --- a/drivers/vhost/vhost.h +++ b/drivers/vhost/vhost.h @@ -106,6 +106,9 @@ struct vhost_virtqueue { /* Log write descriptors */ void __user *log_base; struct vhost_log *log; + + /* We need to know the device endianness with legacy virtio. */ + bool legacy_is_little_endian; }; struct vhost_dev { @@ -173,11 +176,23 @@ static inline bool vhost_has_feature(struct
[PATCH v3 0/7] vhost: support for cross endian guests
Hi, This patchset allows vhost to be used with legacy virtio when guest and host have a different endianness. Patches 1-6 remain the same as the previous post. Patch 7 was heavily changed according to MST's comments. --- Greg Kurz (7): virtio: introduce virtio_is_little_endian() helper tun: add tun_is_little_endian() helper macvtap: introduce macvtap_is_little_endian() helper vringh: introduce vringh_is_little_endian() helper vhost: introduce vhost_is_little_endian() helper virtio: add explicit big-endian support to memory accessors vhost: feature to set the vring endianness drivers/net/macvtap.c| 11 ++-- drivers/net/tun.c| 11 ++-- drivers/vhost/Kconfig| 10 +++ drivers/vhost/vhost.c| 55 ++ drivers/vhost/vhost.h| 34 +++ include/linux/virtio_byteorder.h | 24 ++--- include/linux/virtio_config.h| 19 + include/linux/vringh.h | 19 + include/uapi/linux/vhost.h |5 +++ 9 files changed, 156 insertions(+), 32 deletions(-) -- Greg -- 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 v3 3/7] macvtap: introduce macvtap_is_little_endian() helper
Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/net/macvtap.c |9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c index 27ecc5c..a2f2958 100644 --- a/drivers/net/macvtap.c +++ b/drivers/net/macvtap.c @@ -49,14 +49,19 @@ struct macvtap_queue { #define MACVTAP_VNET_LE 0x8000 +static inline bool macvtap_is_little_endian(struct macvtap_queue *q) +{ + return q-flags MACVTAP_VNET_LE; +} + static inline u16 macvtap16_to_cpu(struct macvtap_queue *q, __virtio16 val) { - return __virtio16_to_cpu(q-flags MACVTAP_VNET_LE, val); + return __virtio16_to_cpu(macvtap_is_little_endian(q), val); } static inline __virtio16 cpu_to_macvtap16(struct macvtap_queue *q, u16 val) { - return __cpu_to_virtio16(q-flags MACVTAP_VNET_LE, val); + return __cpu_to_virtio16(macvtap_is_little_endian(q), val); } static struct proto macvtap_proto = { -- 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 v3 2/7] tun: add tun_is_little_endian() helper
Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/net/tun.c |9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/net/tun.c b/drivers/net/tun.c index 857dca4..3c3d6c0 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -206,14 +206,19 @@ struct tun_struct { u32 flow_count; }; +static inline bool tun_is_little_endian(struct tun_struct *tun) +{ + return tun-flags TUN_VNET_LE; +} + static inline u16 tun16_to_cpu(struct tun_struct *tun, __virtio16 val) { - return __virtio16_to_cpu(tun-flags TUN_VNET_LE, val); + return __virtio16_to_cpu(tun_is_little_endian(tun), val); } static inline __virtio16 cpu_to_tun16(struct tun_struct *tun, u16 val) { - return __cpu_to_virtio16(tun-flags TUN_VNET_LE, val); + return __cpu_to_virtio16(tun_is_little_endian(tun), val); } static inline u32 tun_hashfn(u32 rxhash) -- 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 v3 1/7] virtio: introduce virtio_is_little_endian() helper
Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- include/linux/virtio_config.h | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h index ca3ed78..bd1a582 100644 --- a/include/linux/virtio_config.h +++ b/include/linux/virtio_config.h @@ -205,35 +205,40 @@ int virtqueue_set_affinity(struct virtqueue *vq, int cpu) return 0; } +static inline bool virtio_is_little_endian(struct virtio_device *vdev) +{ + return virtio_has_feature(vdev, VIRTIO_F_VERSION_1); +} + /* Memory accessors */ static inline u16 virtio16_to_cpu(struct virtio_device *vdev, __virtio16 val) { - return __virtio16_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val); + return __virtio16_to_cpu(virtio_is_little_endian(vdev), val); } static inline __virtio16 cpu_to_virtio16(struct virtio_device *vdev, u16 val) { - return __cpu_to_virtio16(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val); + return __cpu_to_virtio16(virtio_is_little_endian(vdev), val); } static inline u32 virtio32_to_cpu(struct virtio_device *vdev, __virtio32 val) { - return __virtio32_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val); + return __virtio32_to_cpu(virtio_is_little_endian(vdev), val); } static inline __virtio32 cpu_to_virtio32(struct virtio_device *vdev, u32 val) { - return __cpu_to_virtio32(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val); + return __cpu_to_virtio32(virtio_is_little_endian(vdev), val); } static inline u64 virtio64_to_cpu(struct virtio_device *vdev, __virtio64 val) { - return __virtio64_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val); + return __virtio64_to_cpu(virtio_is_little_endian(vdev), val); } static inline __virtio64 cpu_to_virtio64(struct virtio_device *vdev, u64 val) { - return __cpu_to_virtio64(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val); + return __cpu_to_virtio64(virtio_is_little_endian(vdev), val); } /* Config space accessors. */ -- 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 v3 6/7] virtio: add explicit big-endian support to memory accessors
The current memory accessors logic is: - little endian if little_endian - native endian (i.e. no byteswap) if !little_endian If we want to fully support cross-endian vhost, we also need to be able to convert to big endian. Instead of changing the little_endian argument to some 3-value enum, this patch changes the logic to: - little endian if little_endian - big endian if !little_endian The native endian case is handled by all users with a trivial helper. This patch doesn't change any functionality, nor it does add overhead. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/net/macvtap.c|4 +++- drivers/net/tun.c|4 +++- drivers/vhost/vhost.h|4 +++- include/linux/virtio_byteorder.h | 24 ++-- include/linux/virtio_config.h|4 +++- include/linux/vringh.h |4 +++- 6 files changed, 29 insertions(+), 15 deletions(-) diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c index a2f2958..0a03a66 100644 --- a/drivers/net/macvtap.c +++ b/drivers/net/macvtap.c @@ -51,7 +51,9 @@ struct macvtap_queue { static inline bool macvtap_is_little_endian(struct macvtap_queue *q) { - return q-flags MACVTAP_VNET_LE; + if (q-flags MACVTAP_VNET_LE) + return true; + return virtio_legacy_is_little_endian(); } static inline u16 macvtap16_to_cpu(struct macvtap_queue *q, __virtio16 val) diff --git a/drivers/net/tun.c b/drivers/net/tun.c index 3c3d6c0..053f9b6 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -208,7 +208,9 @@ struct tun_struct { static inline bool tun_is_little_endian(struct tun_struct *tun) { - return tun-flags TUN_VNET_LE; + if (tun-flags TUN_VNET_LE) + return true; + return virtio_legacy_is_little_endian(); } static inline u16 tun16_to_cpu(struct tun_struct *tun, __virtio16 val) diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h index 6a49960..4e9a186 100644 --- a/drivers/vhost/vhost.h +++ b/drivers/vhost/vhost.h @@ -175,7 +175,9 @@ static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit) static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq) { - return vhost_has_feature(vq, VIRTIO_F_VERSION_1); + if (vhost_has_feature(vq, VIRTIO_F_VERSION_1)) + return true; + return virtio_legacy_is_little_endian(); } /* Memory accessors */ diff --git a/include/linux/virtio_byteorder.h b/include/linux/virtio_byteorder.h index 51865d0..ce63a2c 100644 --- a/include/linux/virtio_byteorder.h +++ b/include/linux/virtio_byteorder.h @@ -3,17 +3,21 @@ #include linux/types.h #include uapi/linux/virtio_types.h -/* - * Low-level memory accessors for handling virtio in modern little endian and in - * compatibility native endian format. - */ +static inline bool virtio_legacy_is_little_endian(void) +{ +#ifdef __LITTLE_ENDIAN + return true; +#else + return false; +#endif +} static inline u16 __virtio16_to_cpu(bool little_endian, __virtio16 val) { if (little_endian) return le16_to_cpu((__force __le16)val); else - return (__force u16)val; + return be16_to_cpu((__force __be16)val); } static inline __virtio16 __cpu_to_virtio16(bool little_endian, u16 val) @@ -21,7 +25,7 @@ static inline __virtio16 __cpu_to_virtio16(bool little_endian, u16 val) if (little_endian) return (__force __virtio16)cpu_to_le16(val); else - return (__force __virtio16)val; + return (__force __virtio16)cpu_to_be16(val); } static inline u32 __virtio32_to_cpu(bool little_endian, __virtio32 val) @@ -29,7 +33,7 @@ static inline u32 __virtio32_to_cpu(bool little_endian, __virtio32 val) if (little_endian) return le32_to_cpu((__force __le32)val); else - return (__force u32)val; + return be32_to_cpu((__force __be32)val); } static inline __virtio32 __cpu_to_virtio32(bool little_endian, u32 val) @@ -37,7 +41,7 @@ static inline __virtio32 __cpu_to_virtio32(bool little_endian, u32 val) if (little_endian) return (__force __virtio32)cpu_to_le32(val); else - return (__force __virtio32)val; + return (__force __virtio32)cpu_to_be32(val); } static inline u64 __virtio64_to_cpu(bool little_endian, __virtio64 val) @@ -45,7 +49,7 @@ static inline u64 __virtio64_to_cpu(bool little_endian, __virtio64 val) if (little_endian) return le64_to_cpu((__force __le64)val); else - return (__force u64)val; + return be64_to_cpu((__force __be64)val); } static inline __virtio64 __cpu_to_virtio64(bool little_endian, u64 val) @@ -53,7 +57,7 @@ static inline __virtio64 __cpu_to_virtio64(bool little_endian, u64 val) if (little_endian) return (__force __virtio64)cpu_to_le64
[PATCH v3 5/7] vhost: introduce vhost_is_little_endian() helper
Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/vhost/vhost.h | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h index 8c1c792..6a49960 100644 --- a/drivers/vhost/vhost.h +++ b/drivers/vhost/vhost.h @@ -173,34 +173,39 @@ static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit) return vq-acked_features (1ULL bit); } +static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq) +{ + return vhost_has_feature(vq, VIRTIO_F_VERSION_1); +} + /* Memory accessors */ static inline u16 vhost16_to_cpu(struct vhost_virtqueue *vq, __virtio16 val) { - return __virtio16_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __virtio16_to_cpu(vhost_is_little_endian(vq), val); } static inline __virtio16 cpu_to_vhost16(struct vhost_virtqueue *vq, u16 val) { - return __cpu_to_virtio16(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __cpu_to_virtio16(vhost_is_little_endian(vq), val); } static inline u32 vhost32_to_cpu(struct vhost_virtqueue *vq, __virtio32 val) { - return __virtio32_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __virtio32_to_cpu(vhost_is_little_endian(vq), val); } static inline __virtio32 cpu_to_vhost32(struct vhost_virtqueue *vq, u32 val) { - return __cpu_to_virtio32(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __cpu_to_virtio32(vhost_is_little_endian(vq), val); } static inline u64 vhost64_to_cpu(struct vhost_virtqueue *vq, __virtio64 val) { - return __virtio64_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __virtio64_to_cpu(vhost_is_little_endian(vq), val); } static inline __virtio64 cpu_to_vhost64(struct vhost_virtqueue *vq, u64 val) { - return __cpu_to_virtio64(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __cpu_to_virtio64(vhost_is_little_endian(vq), val); } #endif -- 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 v3 7/7] vhost: feature to set the vring endianness
This patch brings cross-endian support to vhost when used to implement legacy virtio devices. Since it is a relatively rare situation, the feature availability is controlled by a kernel config option (not set by default). The ioctls introduced by this patch are for legacy only: virtio 1.0 devices are returned EPERM. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/vhost/Kconfig | 10 drivers/vhost/vhost.c | 55 drivers/vhost/vhost.h | 17 +- include/uapi/linux/vhost.h |5 4 files changed, 86 insertions(+), 1 deletion(-) Changes since v2: - fixed typos in Kconfig description - renamed vq-legacy_big_endian to vq-legacy_is_little_endian - vq-legacy_is_little_endian reset to default in vhost_vq_reset() - dropped VHOST_F_SET_ENDIAN_LEGACY feature - dropped struct vhost_vring_endian from the user API (re-use struct vhost_vring_state instead) - added VHOST_GET_VRING_ENDIAN_LEGACY ioctl - introduced more helpers and stubs to avoid polluting the code with ifdefs diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig index 017a1e8..0aec88c 100644 --- a/drivers/vhost/Kconfig +++ b/drivers/vhost/Kconfig @@ -32,3 +32,13 @@ config VHOST ---help--- This option is selected by any driver which needs to access the core of vhost. + +config VHOST_SET_ENDIAN_LEGACY + bool Cross-endian support for host kernel accelerator + default n + ---help--- + This option allows vhost to support guests with a different byte + ordering from host. It is disabled by default since it adds overhead + and it is only needed by a few platforms (powerpc and arm). + + If unsure, say N. diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index 2ee2826..3529a3c 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -199,6 +199,7 @@ static void vhost_vq_reset(struct vhost_dev *dev, vq-call = NULL; vq-log_ctx = NULL; vq-memory = NULL; + vq-legacy_is_little_endian = virtio_legacy_is_little_endian(); } static int vhost_worker(void *data) @@ -630,6 +631,54 @@ static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m) return 0; } +#ifdef CONFIG_VHOST_SET_ENDIAN_LEGACY +static long vhost_set_vring_endian_legacy(struct vhost_virtqueue *vq, + void __user *argp) +{ + struct vhost_vring_state s; + + if (vhost_has_feature(vq, VIRTIO_F_VERSION_1)) + return -EPERM; + + if (copy_from_user(s, argp, sizeof(s))) + return -EFAULT; + + vq-legacy_is_little_endian = !!s.num; + return 0; +} + +static long vhost_get_vring_endian_legacy(struct vhost_virtqueue *vq, + u32 idx, + void __user *argp) +{ + struct vhost_vring_state s = { + .index = idx, + .num = vq-legacy_is_little_endian + }; + + if (vhost_has_feature(vq, VIRTIO_F_VERSION_1)) + return -EPERM; + + if (copy_to_user(argp, s, sizeof(s))) + return -EFAULT; + + return 0; +} +#else +static long vhost_set_vring_endian_legacy(struct vhost_virtqueue *vq, + void __user *argp) +{ + return 0; +} + +static long vhost_get_vring_endian_legacy(struct vhost_virtqueue *vq, + u32 idx, + void __user *argp) +{ + return 0; +} +#endif /* CONFIG_VHOST_SET_ENDIAN_LEGACY */ + long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp) { struct file *eventfp, *filep = NULL; @@ -806,6 +855,12 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp) } else filep = eventfp; break; + case VHOST_SET_VRING_ENDIAN_LEGACY: + r = vhost_set_vring_endian_legacy(vq, argp); + break; + case VHOST_GET_VRING_ENDIAN_LEGACY: + r = vhost_get_vring_endian_legacy(vq, idx, argp); + break; default: r = -ENOIOCTLCMD; } diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h index 4e9a186..981ba06 100644 --- a/drivers/vhost/vhost.h +++ b/drivers/vhost/vhost.h @@ -106,6 +106,9 @@ struct vhost_virtqueue { /* Log write descriptors */ void __user *log_base; struct vhost_log *log; + + /* We need to know the device endianness with legacy virtio. */ + bool legacy_is_little_endian; }; struct vhost_dev { @@ -173,11 +176,23 @@ static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit) return vq-acked_features (1ULL bit); } +#ifdef CONFIG_VHOST_SET_ENDIAN_LEGACY +static inline bool vhost_legacy_is_little_endian(struct vhost_virtqueue *vq
[PATCH v3 4/7] vringh: introduce vringh_is_little_endian() helper
Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- include/linux/vringh.h | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/include/linux/vringh.h b/include/linux/vringh.h index a3fa537..3ed62ef 100644 --- a/include/linux/vringh.h +++ b/include/linux/vringh.h @@ -226,33 +226,38 @@ static inline void vringh_notify(struct vringh *vrh) vrh-notify(vrh); } +static inline bool vringh_is_little_endian(const struct vringh *vrh) +{ + return vrh-little_endian; +} + static inline u16 vringh16_to_cpu(const struct vringh *vrh, __virtio16 val) { - return __virtio16_to_cpu(vrh-little_endian, val); + return __virtio16_to_cpu(vringh_is_little_endian(vrh), val); } static inline __virtio16 cpu_to_vringh16(const struct vringh *vrh, u16 val) { - return __cpu_to_virtio16(vrh-little_endian, val); + return __cpu_to_virtio16(vringh_is_little_endian(vrh), val); } static inline u32 vringh32_to_cpu(const struct vringh *vrh, __virtio32 val) { - return __virtio32_to_cpu(vrh-little_endian, val); + return __virtio32_to_cpu(vringh_is_little_endian(vrh), val); } static inline __virtio32 cpu_to_vringh32(const struct vringh *vrh, u32 val) { - return __cpu_to_virtio32(vrh-little_endian, val); + return __cpu_to_virtio32(vringh_is_little_endian(vrh), val); } static inline u64 vringh64_to_cpu(const struct vringh *vrh, __virtio64 val) { - return __virtio64_to_cpu(vrh-little_endian, val); + return __virtio64_to_cpu(vringh_is_little_endian(vrh), val); } static inline __virtio64 cpu_to_vringh64(const struct vringh *vrh, u64 val) { - return __cpu_to_virtio64(vrh-little_endian, val); + return __cpu_to_virtio64(vringh_is_little_endian(vrh), val); } #endif /* _LINUX_VRINGH_H */ -- 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 0/7] vhost: support for cross endian guests
On Tue, 7 Apr 2015 17:55:08 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Tue, Apr 07, 2015 at 02:09:29PM +0200, Greg Kurz wrote: Hi, This patchset allows vhost to be used with legacy virtio when guest and host have a different endianness. Patches 1-6 remain the same as the previous post. Patch 7 was heavily changed according to MST's comments. This still doesn't actually work, right? tun and macvtap need new ioctls too ... Yes they do. I already have a patch but I wasn't sure if I should send it along this series... Since it looks like there will be a v4, I'll add the tun/macvtap patch. Thanks. -- Greg --- Greg Kurz (7): virtio: introduce virtio_is_little_endian() helper tun: add tun_is_little_endian() helper macvtap: introduce macvtap_is_little_endian() helper vringh: introduce vringh_is_little_endian() helper vhost: introduce vhost_is_little_endian() helper virtio: add explicit big-endian support to memory accessors vhost: feature to set the vring endianness drivers/net/macvtap.c| 11 ++-- drivers/net/tun.c| 11 ++-- drivers/vhost/Kconfig| 10 +++ drivers/vhost/vhost.c| 55 ++ drivers/vhost/vhost.h| 34 +++ include/linux/virtio_byteorder.h | 24 ++--- include/linux/virtio_config.h| 19 + include/linux/vringh.h | 19 + include/uapi/linux/vhost.h |5 +++ 9 files changed, 156 insertions(+), 32 deletions(-) -- Greg -- 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 v2 3/7] macvtap: introduce macvtap_is_little_endian() helper
Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/net/macvtap.c |9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c index 27ecc5c..a2f2958 100644 --- a/drivers/net/macvtap.c +++ b/drivers/net/macvtap.c @@ -49,14 +49,19 @@ struct macvtap_queue { #define MACVTAP_VNET_LE 0x8000 +static inline bool macvtap_is_little_endian(struct macvtap_queue *q) +{ + return q-flags MACVTAP_VNET_LE; +} + static inline u16 macvtap16_to_cpu(struct macvtap_queue *q, __virtio16 val) { - return __virtio16_to_cpu(q-flags MACVTAP_VNET_LE, val); + return __virtio16_to_cpu(macvtap_is_little_endian(q), val); } static inline __virtio16 cpu_to_macvtap16(struct macvtap_queue *q, u16 val) { - return __cpu_to_virtio16(q-flags MACVTAP_VNET_LE, val); + return __cpu_to_virtio16(macvtap_is_little_endian(q), val); } static struct proto macvtap_proto = { -- 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 v2 4/7] vringh: introduce vringh_is_little_endian() helper
Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- include/linux/vringh.h | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/include/linux/vringh.h b/include/linux/vringh.h index a3fa537..3ed62ef 100644 --- a/include/linux/vringh.h +++ b/include/linux/vringh.h @@ -226,33 +226,38 @@ static inline void vringh_notify(struct vringh *vrh) vrh-notify(vrh); } +static inline bool vringh_is_little_endian(const struct vringh *vrh) +{ + return vrh-little_endian; +} + static inline u16 vringh16_to_cpu(const struct vringh *vrh, __virtio16 val) { - return __virtio16_to_cpu(vrh-little_endian, val); + return __virtio16_to_cpu(vringh_is_little_endian(vrh), val); } static inline __virtio16 cpu_to_vringh16(const struct vringh *vrh, u16 val) { - return __cpu_to_virtio16(vrh-little_endian, val); + return __cpu_to_virtio16(vringh_is_little_endian(vrh), val); } static inline u32 vringh32_to_cpu(const struct vringh *vrh, __virtio32 val) { - return __virtio32_to_cpu(vrh-little_endian, val); + return __virtio32_to_cpu(vringh_is_little_endian(vrh), val); } static inline __virtio32 cpu_to_vringh32(const struct vringh *vrh, u32 val) { - return __cpu_to_virtio32(vrh-little_endian, val); + return __cpu_to_virtio32(vringh_is_little_endian(vrh), val); } static inline u64 vringh64_to_cpu(const struct vringh *vrh, __virtio64 val) { - return __virtio64_to_cpu(vrh-little_endian, val); + return __virtio64_to_cpu(vringh_is_little_endian(vrh), val); } static inline __virtio64 cpu_to_vringh64(const struct vringh *vrh, u64 val) { - return __cpu_to_virtio64(vrh-little_endian, val); + return __cpu_to_virtio64(vringh_is_little_endian(vrh), val); } #endif /* _LINUX_VRINGH_H */ -- 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 v2 1/7] virtio: introduce virtio_is_little_endian() helper
Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- include/linux/virtio_config.h | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h index ca3ed78..bd1a582 100644 --- a/include/linux/virtio_config.h +++ b/include/linux/virtio_config.h @@ -205,35 +205,40 @@ int virtqueue_set_affinity(struct virtqueue *vq, int cpu) return 0; } +static inline bool virtio_is_little_endian(struct virtio_device *vdev) +{ + return virtio_has_feature(vdev, VIRTIO_F_VERSION_1); +} + /* Memory accessors */ static inline u16 virtio16_to_cpu(struct virtio_device *vdev, __virtio16 val) { - return __virtio16_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val); + return __virtio16_to_cpu(virtio_is_little_endian(vdev), val); } static inline __virtio16 cpu_to_virtio16(struct virtio_device *vdev, u16 val) { - return __cpu_to_virtio16(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val); + return __cpu_to_virtio16(virtio_is_little_endian(vdev), val); } static inline u32 virtio32_to_cpu(struct virtio_device *vdev, __virtio32 val) { - return __virtio32_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val); + return __virtio32_to_cpu(virtio_is_little_endian(vdev), val); } static inline __virtio32 cpu_to_virtio32(struct virtio_device *vdev, u32 val) { - return __cpu_to_virtio32(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val); + return __cpu_to_virtio32(virtio_is_little_endian(vdev), val); } static inline u64 virtio64_to_cpu(struct virtio_device *vdev, __virtio64 val) { - return __virtio64_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val); + return __virtio64_to_cpu(virtio_is_little_endian(vdev), val); } static inline __virtio64 cpu_to_virtio64(struct virtio_device *vdev, u64 val) { - return __cpu_to_virtio64(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val); + return __cpu_to_virtio64(virtio_is_little_endian(vdev), val); } /* Config space accessors. */ -- 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 v2 5/7] vhost: introduce vhost_is_little_endian() helper
Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/vhost/vhost.h | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h index 8c1c792..6a49960 100644 --- a/drivers/vhost/vhost.h +++ b/drivers/vhost/vhost.h @@ -173,34 +173,39 @@ static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit) return vq-acked_features (1ULL bit); } +static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq) +{ + return vhost_has_feature(vq, VIRTIO_F_VERSION_1); +} + /* Memory accessors */ static inline u16 vhost16_to_cpu(struct vhost_virtqueue *vq, __virtio16 val) { - return __virtio16_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __virtio16_to_cpu(vhost_is_little_endian(vq), val); } static inline __virtio16 cpu_to_vhost16(struct vhost_virtqueue *vq, u16 val) { - return __cpu_to_virtio16(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __cpu_to_virtio16(vhost_is_little_endian(vq), val); } static inline u32 vhost32_to_cpu(struct vhost_virtqueue *vq, __virtio32 val) { - return __virtio32_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __virtio32_to_cpu(vhost_is_little_endian(vq), val); } static inline __virtio32 cpu_to_vhost32(struct vhost_virtqueue *vq, u32 val) { - return __cpu_to_virtio32(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __cpu_to_virtio32(vhost_is_little_endian(vq), val); } static inline u64 vhost64_to_cpu(struct vhost_virtqueue *vq, __virtio64 val) { - return __virtio64_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __virtio64_to_cpu(vhost_is_little_endian(vq), val); } static inline __virtio64 cpu_to_vhost64(struct vhost_virtqueue *vq, u64 val) { - return __cpu_to_virtio64(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __cpu_to_virtio64(vhost_is_little_endian(vq), val); } #endif -- 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 v2 2/7] tun: add tun_is_little_endian() helper
Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/net/tun.c |9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/net/tun.c b/drivers/net/tun.c index 857dca4..3c3d6c0 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -206,14 +206,19 @@ struct tun_struct { u32 flow_count; }; +static inline bool tun_is_little_endian(struct tun_struct *tun) +{ + return tun-flags TUN_VNET_LE; +} + static inline u16 tun16_to_cpu(struct tun_struct *tun, __virtio16 val) { - return __virtio16_to_cpu(tun-flags TUN_VNET_LE, val); + return __virtio16_to_cpu(tun_is_little_endian(tun), val); } static inline __virtio16 cpu_to_tun16(struct tun_struct *tun, u16 val) { - return __cpu_to_virtio16(tun-flags TUN_VNET_LE, val); + return __cpu_to_virtio16(tun_is_little_endian(tun), val); } static inline u32 tun_hashfn(u32 rxhash) -- 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 v2 0/7] vhost: support for cross endian guests
Hi, This patchset allows vhost to be used with legacy virtio when guest and host have a different endianness. It is a complete rework of my initial post. Patches 1 to 5 are preliminary work: we move the endianness check out of all memory accessors to separate functions. Patch 6 changes the semantics of the accessors so that they have explicit big endian support. Patch 7 brings the cross-endian feature, with the following changes since v1: - conditionnal enablement through a kernel config - introduction of a new vhost feature to advertise cross-endian to userspace The tentative to fix vnet headers was dropped for the moment. As a consequnce, vhost_net still fails to work with cross-endian. It will be fixed in another patchset I'm currently working on. --- Greg Kurz (7): virtio: introduce virtio_is_little_endian() helper tun: add tun_is_little_endian() helper macvtap: introduce macvtap_is_little_endian() helper vringh: introduce vringh_is_little_endian() helper vhost: introduce vhost_is_little_endian() helper virtio: add explicit big-endian support to memory accessors vhost: feature to set the vring endianness drivers/net/macvtap.c| 11 +-- drivers/net/tun.c| 11 +-- drivers/vhost/Kconfig| 10 ++ drivers/vhost/net.c |5 + drivers/vhost/scsi.c |4 drivers/vhost/test.c |4 drivers/vhost/vhost.c| 19 +++ drivers/vhost/vhost.h| 32 +--- include/linux/virtio_byteorder.h | 24 ++-- include/linux/virtio_config.h| 19 +-- include/linux/vringh.h | 19 +-- include/uapi/linux/vhost.h | 10 ++ 12 files changed, 135 insertions(+), 33 deletions(-) -- Greg -- 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 v2 6/7] virtio: add explicit big-endian support to memory accessors
The current memory accessors logic is: - little endian if little_endian - native endian (i.e. no byteswap) if !little_endian If we want to fully support cross-endian vhost, we also need to be able to convert to big endian. Instead of changing the little_endian argument to some 3-value enum, this patch changes the logic to: - little endian if little_endian - big endian if !little_endian The native endian case is handled by all users with a trivial helper. This patch doesn't change any functionality, nor it does add overhead. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/net/macvtap.c|4 +++- drivers/net/tun.c|4 +++- drivers/vhost/vhost.h|4 +++- include/linux/virtio_byteorder.h | 24 ++-- include/linux/virtio_config.h|4 +++- include/linux/vringh.h |4 +++- 6 files changed, 29 insertions(+), 15 deletions(-) diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c index a2f2958..0a03a66 100644 --- a/drivers/net/macvtap.c +++ b/drivers/net/macvtap.c @@ -51,7 +51,9 @@ struct macvtap_queue { static inline bool macvtap_is_little_endian(struct macvtap_queue *q) { - return q-flags MACVTAP_VNET_LE; + if (q-flags MACVTAP_VNET_LE) + return true; + return virtio_legacy_is_little_endian(); } static inline u16 macvtap16_to_cpu(struct macvtap_queue *q, __virtio16 val) diff --git a/drivers/net/tun.c b/drivers/net/tun.c index 3c3d6c0..053f9b6 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -208,7 +208,9 @@ struct tun_struct { static inline bool tun_is_little_endian(struct tun_struct *tun) { - return tun-flags TUN_VNET_LE; + if (tun-flags TUN_VNET_LE) + return true; + return virtio_legacy_is_little_endian(); } static inline u16 tun16_to_cpu(struct tun_struct *tun, __virtio16 val) diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h index 6a49960..4e9a186 100644 --- a/drivers/vhost/vhost.h +++ b/drivers/vhost/vhost.h @@ -175,7 +175,9 @@ static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit) static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq) { - return vhost_has_feature(vq, VIRTIO_F_VERSION_1); + if (vhost_has_feature(vq, VIRTIO_F_VERSION_1)) + return true; + return virtio_legacy_is_little_endian(); } /* Memory accessors */ diff --git a/include/linux/virtio_byteorder.h b/include/linux/virtio_byteorder.h index 51865d0..ce63a2c 100644 --- a/include/linux/virtio_byteorder.h +++ b/include/linux/virtio_byteorder.h @@ -3,17 +3,21 @@ #include linux/types.h #include uapi/linux/virtio_types.h -/* - * Low-level memory accessors for handling virtio in modern little endian and in - * compatibility native endian format. - */ +static inline bool virtio_legacy_is_little_endian(void) +{ +#ifdef __LITTLE_ENDIAN + return true; +#else + return false; +#endif +} static inline u16 __virtio16_to_cpu(bool little_endian, __virtio16 val) { if (little_endian) return le16_to_cpu((__force __le16)val); else - return (__force u16)val; + return be16_to_cpu((__force __be16)val); } static inline __virtio16 __cpu_to_virtio16(bool little_endian, u16 val) @@ -21,7 +25,7 @@ static inline __virtio16 __cpu_to_virtio16(bool little_endian, u16 val) if (little_endian) return (__force __virtio16)cpu_to_le16(val); else - return (__force __virtio16)val; + return (__force __virtio16)cpu_to_be16(val); } static inline u32 __virtio32_to_cpu(bool little_endian, __virtio32 val) @@ -29,7 +33,7 @@ static inline u32 __virtio32_to_cpu(bool little_endian, __virtio32 val) if (little_endian) return le32_to_cpu((__force __le32)val); else - return (__force u32)val; + return be32_to_cpu((__force __be32)val); } static inline __virtio32 __cpu_to_virtio32(bool little_endian, u32 val) @@ -37,7 +41,7 @@ static inline __virtio32 __cpu_to_virtio32(bool little_endian, u32 val) if (little_endian) return (__force __virtio32)cpu_to_le32(val); else - return (__force __virtio32)val; + return (__force __virtio32)cpu_to_be32(val); } static inline u64 __virtio64_to_cpu(bool little_endian, __virtio64 val) @@ -45,7 +49,7 @@ static inline u64 __virtio64_to_cpu(bool little_endian, __virtio64 val) if (little_endian) return le64_to_cpu((__force __le64)val); else - return (__force u64)val; + return be64_to_cpu((__force __be64)val); } static inline __virtio64 __cpu_to_virtio64(bool little_endian, u64 val) @@ -53,7 +57,7 @@ static inline __virtio64 __cpu_to_virtio64(bool little_endian, u64 val) if (little_endian) return (__force __virtio64)cpu_to_le64
[PATCH v2 7/7] vhost: feature to set the vring endianness
This patch brings cross-endian support to vhost when used to implement legacy virtio devices. Since it is a relatively rare situation, the feature availability is controlled by a kernel config option (not set by default). If cross-endian support is compiled in, vhost abvertises a new feature to be negotiated with userspace. If userspace acknowledges the feature, it can inform vhost about the endianness to use with a new ioctl. This feature is mutually exclusive with virtio 1.0. Even if the vhost device advertises virtio 1.0 and legacy cross-endian features, it cannot receive aknowledgement for both at the same time. Hot paths are being preserved from any penalty when the config option is disabled or when virtio 1.0 is being used. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/vhost/Kconfig | 10 ++ drivers/vhost/net.c|5 + drivers/vhost/scsi.c |4 drivers/vhost/test.c |4 drivers/vhost/vhost.c | 19 +++ drivers/vhost/vhost.h | 13 - include/uapi/linux/vhost.h | 10 ++ 7 files changed, 64 insertions(+), 1 deletion(-) diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig index 017a1e8..5bb8da9 100644 --- a/drivers/vhost/Kconfig +++ b/drivers/vhost/Kconfig @@ -32,3 +32,13 @@ config VHOST ---help--- This option is selected by any driver which needs to access the core of vhost. + +config VHOST_SET_ENDIAN_LEGACY + bool Cross-endian support for host kernel accelerator + default n + ---help--- + This option allows vhost to support guests with a different byte + ordering. It is disabled by default since it adds overhead and it + is only needed by a few platforms (powerpc and arm). + + If unsure, say n. diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c index 2bbfc25..5274a44 100644 --- a/drivers/vhost/net.c +++ b/drivers/vhost/net.c @@ -1011,6 +1011,11 @@ static int vhost_net_set_features(struct vhost_net *n, u64 features) vhost_hlen = 0; sock_hlen = hdr_len; } + + if (features ((1ULL VHOST_F_SET_ENDIAN_LEGACY) | + (1ULL VIRTIO_F_VERSION_1))) + return -EINVAL; + mutex_lock(n-dev.mutex); if ((features (1 VHOST_F_LOG_ALL)) !vhost_log_access_ok(n-dev)) { diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c index 71df240..b53e9c2 100644 --- a/drivers/vhost/scsi.c +++ b/drivers/vhost/scsi.c @@ -1544,6 +1544,10 @@ static int vhost_scsi_set_features(struct vhost_scsi *vs, u64 features) if (features ~VHOST_SCSI_FEATURES) return -EOPNOTSUPP; + if (features ((1ULL VHOST_F_SET_ENDIAN_LEGACY) | + (1ULL VIRTIO_F_VERSION_1))) + return -EINVAL; + mutex_lock(vs-dev.mutex); if ((features (1 VHOST_F_LOG_ALL)) !vhost_log_access_ok(vs-dev)) { diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c index d9c501e..cfefdad 100644 --- a/drivers/vhost/test.c +++ b/drivers/vhost/test.c @@ -243,6 +243,10 @@ static int vhost_test_set_features(struct vhost_test *n, u64 features) { struct vhost_virtqueue *vq; + if (features ((1ULL VHOST_F_SET_ENDIAN_LEGACY) | + (1ULL VIRTIO_F_VERSION_1))) + return -EINVAL; + mutex_lock(n-dev.mutex); if ((features (1 VHOST_F_LOG_ALL)) !vhost_log_access_ok(n-dev)) { diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index 2ee2826..60a0f32 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -199,6 +199,7 @@ static void vhost_vq_reset(struct vhost_dev *dev, vq-call = NULL; vq-log_ctx = NULL; vq-memory = NULL; + vq-legacy_big_endian = false; } static int vhost_worker(void *data) @@ -806,6 +807,24 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp) } else filep = eventfp; break; +#ifdef CONFIG_VHOST_SET_ENDIAN_LEGACY + case VHOST_SET_VRING_ENDIAN_LEGACY: + { + struct vhost_vring_endian e; + + if (!vhost_has_feature(vq, VHOST_F_SET_ENDIAN_LEGACY)) { + r = -EINVAL; + break; + } + + if (copy_from_user(e, argp, sizeof(e))) { + r = -EFAULT; + break; + } + vq-legacy_big_endian = e.is_big_endian; + break; + } +#endif default: r = -ENOIOCTLCMD; } diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h index 4e9a186..d50881d 100644 --- a/drivers/vhost/vhost.h +++ b/drivers/vhost/vhost.h @@ -106,6 +106,9 @@ struct vhost_virtqueue { /* Log write descriptors */ void __user *log_base; struct vhost_log *log
Re: [PATCH v2 7/7] vhost: feature to set the vring endianness
On Thu, 2 Apr 2015 16:20:46 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Thu, Apr 02, 2015 at 03:17:13PM +0200, Greg Kurz wrote: This patch brings cross-endian support to vhost when used to implement legacy virtio devices. Since it is a relatively rare situation, the feature availability is controlled by a kernel config option (not set by default). If cross-endian support is compiled in, vhost abvertises a new feature to be negotiated with userspace. If userspace acknowledges the feature, it can inform vhost about the endianness to use with a new ioctl. This feature is mutually exclusive with virtio 1.0. Even if the vhost device advertises virtio 1.0 and legacy cross-endian features, it cannot receive aknowledgement for both at the same time. Hot paths are being preserved from any penalty when the config option is disabled or when virtio 1.0 is being used. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/vhost/Kconfig | 10 ++ drivers/vhost/net.c|5 + drivers/vhost/scsi.c |4 drivers/vhost/test.c |4 drivers/vhost/vhost.c | 19 +++ drivers/vhost/vhost.h | 13 - include/uapi/linux/vhost.h | 10 ++ 7 files changed, 64 insertions(+), 1 deletion(-) diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig index 017a1e8..5bb8da9 100644 --- a/drivers/vhost/Kconfig +++ b/drivers/vhost/Kconfig @@ -32,3 +32,13 @@ config VHOST ---help--- This option is selected by any driver which needs to access the core of vhost. + +config VHOST_SET_ENDIAN_LEGACY + bool Cross-endian support for host kernel accelerator + default n + ---help--- + This option allows vhost to support guests with a different byte + ordering from host . It is disabled by default since it adds overhead and it + is only needed by a few platforms (powerpc and arm). + + If unsure, say n. N diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c index 2bbfc25..5274a44 100644 --- a/drivers/vhost/net.c +++ b/drivers/vhost/net.c @@ -1011,6 +1011,11 @@ static int vhost_net_set_features(struct vhost_net *n, u64 features) vhost_hlen = 0; sock_hlen = hdr_len; } + + if (features ((1ULL VHOST_F_SET_ENDIAN_LEGACY) | + (1ULL VIRTIO_F_VERSION_1))) + return -EINVAL; + mutex_lock(n-dev.mutex); if ((features (1 VHOST_F_LOG_ALL)) !vhost_log_access_ok(n-dev)) { diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c index 71df240..b53e9c2 100644 --- a/drivers/vhost/scsi.c +++ b/drivers/vhost/scsi.c @@ -1544,6 +1544,10 @@ static int vhost_scsi_set_features(struct vhost_scsi *vs, u64 features) if (features ~VHOST_SCSI_FEATURES) return -EOPNOTSUPP; + if (features ((1ULL VHOST_F_SET_ENDIAN_LEGACY) | + (1ULL VIRTIO_F_VERSION_1))) + return -EINVAL; + mutex_lock(vs-dev.mutex); if ((features (1 VHOST_F_LOG_ALL)) !vhost_log_access_ok(vs-dev)) { diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c index d9c501e..cfefdad 100644 --- a/drivers/vhost/test.c +++ b/drivers/vhost/test.c @@ -243,6 +243,10 @@ static int vhost_test_set_features(struct vhost_test *n, u64 features) { struct vhost_virtqueue *vq; + if (features ((1ULL VHOST_F_SET_ENDIAN_LEGACY) | + (1ULL VIRTIO_F_VERSION_1))) + return -EINVAL; + mutex_lock(n-dev.mutex); if ((features (1 VHOST_F_LOG_ALL)) !vhost_log_access_ok(n-dev)) { Above seems to prevent users from specifying either VIRTIO_F_VERSION_1 or VHOST_F_SET_ENDIAN_LEGACY. Does it actually work? Arggg no it doesn't *of course*... I've added these bogus checks lately and didn't re-test :( diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index 2ee2826..60a0f32 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -199,6 +199,7 @@ static void vhost_vq_reset(struct vhost_dev *dev, vq-call = NULL; vq-log_ctx = NULL; vq-memory = NULL; + vq-legacy_big_endian = false; } static int vhost_worker(void *data) @@ -806,6 +807,24 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp) } else filep = eventfp; break; +#ifdef CONFIG_VHOST_SET_ENDIAN_LEGACY + case VHOST_SET_VRING_ENDIAN_LEGACY: + { + struct vhost_vring_endian e; + + if (!vhost_has_feature(vq, VHOST_F_SET_ENDIAN_LEGACY)) { + r = -EINVAL; + break; + } + + if (copy_from_user(e, argp, sizeof(e))) { + r = -EFAULT; + break; + } + vq
Re: [PATCH] kvm: move advertising of KVM_CAP_IRQFD to common code
On Thu, 5 Mar 2015 11:54:46 +0100 Paolo Bonzini pbonz...@redhat.com wrote: POWER supports irqfds but forgot to advertise them. Some userspace does not check for the capability, but others check it---thus they work on x86 and s390 but not POWER. To avoid that other architectures in the future make the same mistake, let common code handle KVM_CAP_IRQFD the same way as KVM_CAP_IRQFD_RESAMPLE. Reported-by: Greg Kurz gk...@linux.vnet.ibm.com Cc: sta...@vger.kernel.org Fixes: 297e21053a52f060944e9f0de4c64fad9bcd72fc Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- Thanks ! Tested-by: Greg Kurz gk...@linux.vnet.ibm.com Marcelo, please apply this for 4.0. --- arch/s390/kvm/kvm-s390.c | 1 - arch/x86/kvm/x86.c | 1 - virt/kvm/kvm_main.c | 1 + 3 files changed, 1 insertion(+), 2 deletions(-) diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index 0c3623927563..17277968e50a 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -165,7 +165,6 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) case KVM_CAP_ONE_REG: case KVM_CAP_ENABLE_CAP: case KVM_CAP_S390_CSS_SUPPORT: - case KVM_CAP_IRQFD: case KVM_CAP_IOEVENTFD: case KVM_CAP_DEVICE_CTRL: case KVM_CAP_ENABLE_CAP_VM: diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index bd7a70be41b3..32bf19ef3115 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -2744,7 +2744,6 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) case KVM_CAP_USER_NMI: case KVM_CAP_REINJECT_CONTROL: case KVM_CAP_IRQ_INJECT_STATUS: - case KVM_CAP_IRQFD: case KVM_CAP_IOEVENTFD: case KVM_CAP_IOEVENTFD_NO_LENGTH: case KVM_CAP_PIT2: diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index a1093700f3a4..a2214d9609bd 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -2492,6 +2492,7 @@ static long kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg) case KVM_CAP_SIGNAL_MSI: #endif #ifdef CONFIG_HAVE_KVM_IRQFD + case KVM_CAP_IRQFD: case KVM_CAP_IRQFD_RESAMPLE: #endif case KVM_CAP_CHECK_EXTENSION_VM: -- 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 0/3] vhost_net: support for cross endian guests
On Sun, 22 Feb 2015 10:53:51 +0100 Michael S. Tsirkin m...@redhat.com wrote: On Fri, Feb 20, 2015 at 11:07:24AM +0100, Greg Kurz wrote: Hi, This patchset allows vhost_net to be used with legacy virtio when guest and host have a different endianness. It is based on previous work by Cédric Le Goater: https://www.mail-archive.com/kvm-ppc@vger.kernel.org/msg09848.html As suggested by MST: - the API now asks for a specific format (big endian) instead of the hint whether byteswap is needed or not (patch 1) - rebased on top of the virtio-1 accessors (patch 2) Patch 3 is a separate fix: I think it is also valid for virtio-1. I don't think so. See e.g. this code in tun: gso.csum_offset = cpu_to_tun16(tun, skb-csum_offset); looks like it has the correct endian-ness for virtio-1. Indeed. I will fix tun/macvtap as you suggested. Thanks for the review. -- Greg Please comment. --- Greg Kurz (3): vhost: add VHOST_VRING_F_LEGACY_BIG_ENDIAN flag vhost: add support for legacy virtio vhost_net: fix virtio_net header endianness drivers/vhost/net.c| 32 ++-- drivers/vhost/vhost.c |6 +- drivers/vhost/vhost.h | 23 +-- include/uapi/linux/vhost.h |2 ++ 4 files changed, 50 insertions(+), 13 deletions(-) -- Greg -- 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 3/3] vhost_net: fix virtio_net header endianness
Without this patch, packets are being silently dropped by the tap backend. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/vhost/net.c | 32 ++-- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c index afa06d2..2923eee 100644 --- a/drivers/vhost/net.c +++ b/drivers/vhost/net.c @@ -288,6 +288,16 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success) rcu_read_unlock_bh(); } +static void fix_virtio_net_hdr(struct vhost_virtqueue *vq) +{ + struct virtio_net_hdr *hdr = vq-iov[0].iov_base; + + hdr-hdr_len = vhost16_to_cpu(vq, hdr-hdr_len); + hdr-gso_size = vhost16_to_cpu(vq, hdr-gso_size); + hdr-csum_start = vhost16_to_cpu(vq, hdr-csum_start); + hdr-csum_offset = vhost16_to_cpu(vq, hdr-csum_offset); +} + /* Expects to be always run from workqueue - which acts as * read-size critical section for our kind of RCU. */ static void handle_tx(struct vhost_net *net) @@ -352,6 +362,10 @@ static void handle_tx(struct vhost_net *net) out %d, int %d\n, out, in); break; } + + if (!hdr_size) + fix_virtio_net_hdr(vq); + /* Skip header. TODO: support TSO. */ len = iov_length(vq-iov, out); iov_iter_init(msg.msg_iter, WRITE, vq-iov, out, len); @@ -609,12 +623,18 @@ static void handle_rx(struct vhost_net *net) continue; } /* Supply virtio_net_hdr if VHOST_NET_F_VIRTIO_NET_HDR */ - if (unlikely(vhost_hlen) - copy_to_iter(hdr, sizeof(hdr), fixup) != sizeof(hdr)) { - vq_err(vq, Unable to write vnet_hdr at addr %p\n, - vq-iov-iov_base); - break; - } + if (unlikely(vhost_hlen)) { + size_t len = copy_to_iter(hdr, sizeof(hdr), fixup); + + if (len != sizeof(hdr)) { + vq_err(vq, + Unable to write vnet_hdr at addr %p\n, + vq-iov-iov_base); + break; + } + } else + fix_virtio_net_hdr(vq); + /* TODO: Should check and handle checksum. */ num_buffers = cpu_to_vhost16(vq, headcount); -- 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 2/3] vhost: add support for legacy virtio
Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/vhost/vhost.h | 20 ++-- 1 file changed, 14 insertions(+), 6 deletions(-) Michael, The vhost_is_little_endian() helper adds unconditionnal overhead to fixed endian architectures: that is all architectures except arm and ppc64. This was addressed in QEMU with a TARGET_IS_BIENDIAN macro. Please give an advice about how to address this in the vhost code. Thanks. diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h index ce2c68e..21e7d6a 100644 --- a/drivers/vhost/vhost.h +++ b/drivers/vhost/vhost.h @@ -176,34 +176,42 @@ static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit) return vq-acked_features (1ULL bit); } +static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq) +{ + if (vhost_has_feature(vq, VIRTIO_F_VERSION_1)) + return true; + else + return !vq-legacy_big_endian; +} + /* Memory accessors */ static inline u16 vhost16_to_cpu(struct vhost_virtqueue *vq, __virtio16 val) { - return __virtio16_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __virtio16_to_cpu(vhost_is_little_endian(vq), val); } static inline __virtio16 cpu_to_vhost16(struct vhost_virtqueue *vq, u16 val) { - return __cpu_to_virtio16(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __cpu_to_virtio16(vhost_is_little_endian(vq), val); } static inline u32 vhost32_to_cpu(struct vhost_virtqueue *vq, __virtio32 val) { - return __virtio32_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __virtio32_to_cpu(vhost_is_little_endian(vq), val); } static inline __virtio32 cpu_to_vhost32(struct vhost_virtqueue *vq, u32 val) { - return __cpu_to_virtio32(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __cpu_to_virtio32(vhost_is_little_endian(vq), val); } static inline u64 vhost64_to_cpu(struct vhost_virtqueue *vq, __virtio64 val) { - return __virtio64_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __virtio64_to_cpu(vhost_is_little_endian(vq), val); } static inline __virtio64 cpu_to_vhost64(struct vhost_virtqueue *vq, u64 val) { - return __cpu_to_virtio64(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val); + return __cpu_to_virtio64(vhost_is_little_endian(vq), val); } #endif -- 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 1/3] vhost: add VHOST_VRING_F_LEGACY_BIG_ENDIAN flag
The VHOST_VRING_F_LEGACY_BIG_ENDIAN flag informs the kernel that the associated device is big endian. Of course, this only makes sense for legacy virtio devices since modern virtio devices are always little endian. It will be used by the vhost memory accessors to byteswap vring data when we have a legacy device, in case host and guest endianness differ. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- drivers/vhost/vhost.c |6 +- drivers/vhost/vhost.h |3 +++ include/uapi/linux/vhost.h |2 ++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index 2ee2826..dad3c37 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -199,6 +199,7 @@ static void vhost_vq_reset(struct vhost_dev *dev, vq-call = NULL; vq-log_ctx = NULL; vq-memory = NULL; + vq-legacy_big_endian = false; } static int vhost_worker(void *data) @@ -701,7 +702,8 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp) r = -EFAULT; break; } - if (a.flags ~(0x1 VHOST_VRING_F_LOG)) { + if (a.flags ~(0x1 VHOST_VRING_F_LOG| + 0x1 VHOST_VRING_F_LEGACY_BIG_ENDIAN)) { r = -EOPNOTSUPP; break; } @@ -751,6 +753,8 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp) vq-avail = (void __user *)(unsigned long)a.avail_user_addr; vq-log_addr = a.log_guest_addr; vq-used = (void __user *)(unsigned long)a.used_user_addr; + vq-legacy_big_endian = + !!(a.flags (0x1 VHOST_VRING_F_LEGACY_BIG_ENDIAN)); break; case VHOST_SET_VRING_KICK: if (copy_from_user(f, argp, sizeof f)) { diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h index 8c1c792..ce2c68e 100644 --- a/drivers/vhost/vhost.h +++ b/drivers/vhost/vhost.h @@ -106,6 +106,9 @@ struct vhost_virtqueue { /* Log write descriptors */ void __user *log_base; struct vhost_log *log; + + /* We need to know the device endianness with legacy virtio. */ + bool legacy_big_endian; }; struct vhost_dev { diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h index bb6a5b4..0bf4491 100644 --- a/include/uapi/linux/vhost.h +++ b/include/uapi/linux/vhost.h @@ -34,6 +34,8 @@ struct vhost_vring_addr { /* Flag values: */ /* Whether log address is valid. If set enables logging. */ #define VHOST_VRING_F_LOG 0 + /* Whether we have a big-endian legacy virtio device. */ +#define VHOST_VRING_F_LEGACY_BIG_ENDIAN 1 /* Start of array of descriptors (virtually contiguous) */ __u64 desc_user_addr; -- 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 0/3] vhost_net: support for cross endian guests
Hi, This patchset allows vhost_net to be used with legacy virtio when guest and host have a different endianness. It is based on previous work by Cédric Le Goater: https://www.mail-archive.com/kvm-ppc@vger.kernel.org/msg09848.html As suggested by MST: - the API now asks for a specific format (big endian) instead of the hint whether byteswap is needed or not (patch 1) - rebased on top of the virtio-1 accessors (patch 2) Patch 3 is a separate fix: I think it is also valid for virtio-1. Please comment. --- Greg Kurz (3): vhost: add VHOST_VRING_F_LEGACY_BIG_ENDIAN flag vhost: add support for legacy virtio vhost_net: fix virtio_net header endianness drivers/vhost/net.c| 32 ++-- drivers/vhost/vhost.c |6 +- drivers/vhost/vhost.h | 23 +-- include/uapi/linux/vhost.h |2 ++ 4 files changed, 50 insertions(+), 13 deletions(-) -- Greg -- 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 v8 34/50] vhost/net: virtio 1.0 byte swap
On Tue, 06 Jan 2015 16:55:30 -0700 Alex Williamson alex.william...@redhat.com wrote: On Mon, 2014-12-01 at 18:05 +0200, Michael S. Tsirkin wrote: I had to add an explicit tag to suppress compiler warning: gcc isn't smart enough to notice that len is always initialized since function is called with size 0. I'm getting a panic inside a guest when this change is applied on the host. I identified this patch via bisect and confirmed by reverting it from v3.19-rc2. Guest is centos6. Thanks, Alex commit 8b38694a2dc8b18374310df50174f1e4376d6824 Author: Michael S. Tsirkin m...@redhat.com Date: Fri Oct 24 14:19:48 2014 +0300 vhost/net: virtio 1.0 byte swap I had to add an explicit tag to suppress compiler warning: gcc isn't smart enough to notice that len is always initialized since function is called with size 0. Signed-off-by: Michael S. Tsirkin m...@redhat.com Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com This chunk looks suspicious: - heads[headcount - 1].len += datalen; + heads[headcount - 1].len = cpu_to_vhost32(vq, len - datalen); s/len - datalen/len + datalen/ ? XML chunk: interface type='direct' mac address='52:54:00:64:f3:34'/ source dev='iscsinet0' mode='bridge'/ model type='virtio'/ address type='pci' domain='0x' bus='0x00' slot='0x03' function='0x0'/ /interface Panic log: 1BUG: unable to handle kernel NULL pointer dereference at 0010 1IP: [a0079469] virtnet_poll+0x4f9/0x910 [virtio_net] 4PGD 1aa2f4067 PUD 1aa2f5067 PMD 0 4Oops: [#1] SMP 4last sysfs file: /sys/devices/pci:00/:00:03.0/virtio0/net/eth9/ifindex 4CPU 0 4Modules linked in: 8021q garp stp llc ipt_REJECT nf_conntrack_ipv4 nf_defrag_ipv4 iptable_filter ip_tables ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 xt_state nf_conntrack ip6table_filter ip6_tables ipv6 uinput microcode snd_hda_codec_realtek snd_hda_intel snd_hda_codec snd_hwdep snd_seq snd_seq_device snd_pcm snd_timer snd soundcore snd_page_alloc igbvf nvidia(P)(U) i2c_core tg3 ptp pps_core virtio_balloon virtio_net virtio_console ext4 jbd2 mbcache virtio_blk virtio_pci virtio_ring virtio pata_acpi ata_generic ata_piix dm_mirror dm_region_hash dm_log dm_mod [last unloaded: speedstep_lib] 4 4Pid: 1374, comm: NetworkManager Tainted: P --- 2.6.32-431.23.3.el6.centos.plus.x86_64 #1 QEMU Standard PC (i440FX + PIIX, 1996) 4RIP: 0010:[a0079469] [a0079469] virtnet_poll+0x4f9/0x910 [virtio_net] 4RSP: 0018:880028203e48 EFLAGS: 00010246 4RAX: 8801a3383d00 RBX: 8801a6aaf480 RCX: 8801aa20b6e0 4RDX: 00c0 RSI: 8801a3383c00 RDI: 8801a3383cc0 4RBP: 880028203ed8 R08: 009e R09: 8801aa1d800c 4R10: 0218 R11: R12: 8801aa20b6e0 4R13: R14: R15: 4FS: 7febf114d800() GS:88002820() knlGS: 4CS: 0010 DS: ES: CR0: 80050033 4CR2: 0010 CR3: 0001aa793000 CR4: 06f0 4DR0: DR1: DR2: 4DR3: DR6: 0ff0 DR7: 0400 4Process NetworkManager (pid: 1374, threadinfo 8801a74ba000, task 8801a8d56040) 4Stack: 4 8801aa1d8000 009e 8801aa20b6e0 8801aa20b718 4d 8801aa20b780 8801aa1d800c 8801a6aaf4b8 8801aa20b020 4d 0080 8801aa20b708 0001 1f5981a830c8 4Call Trace: 4 IRQ 4 [8146ae33] net_rx_action+0x103/0x2f0 4 [8107a5f1] __do_softirq+0xc1/0x1e0 4 [8100c30c] ? call_softirq+0x1c/0x30 4 [8100c30c] call_softirq+0x1c/0x30 4 EOI 4 [8100fa75] ? do_softirq+0x65/0xa0 4 [8107b2ea] local_bh_enable+0x9a/0xb0 4 [a007813a] virtnet_napi_enable+0x4a/0x60 [virtio_net] 4 [a0078ebf] virtnet_open+0x4f/0x60 [virtio_net] 4 [81467691] dev_open+0xa1/0x100 4 [81466751] dev_change_flags+0xa1/0x1d0 4 [81474a59] do_setlink+0x169/0x8b0 4 [814770b6] ? rtnl_fill_ifinfo+0x946/0xcb0 4 [812a3d24] ? nla_parse+0x34/0x110 4 [8147659e] rtnl_setlink+0xee/0x130 4 [81475b67] rtnetlink_rcv_msg+0x2d7/0x340 4 [81231e14] ? socket_has_perm+0x74/0x90 4 [81475890] ? rtnetlink_rcv_msg+0x0/0x340 4 [814910a9] netlink_rcv_skb+0xa9/0xd0 4 [81475875] rtnetlink_rcv+0x25/0x40 4 [81490cdb] netlink_unicast+0x2db/0x320 4 [81491750] netlink_sendmsg+0x2c0/0x3d0 4 [814520c3] sock_sendmsg+0x123/0x150 4 [81453d73] ? sock_recvmsg+0x133/0x160 4 [8109afa0] ? autoremove_wake_function+0x0/0x40 4 [81136941] ? lru_cache_add_lru+0x21/0x40 4 [8115522d] ? page_add_new_anon_rmap+0x9d/0xf0 4 [8114aeef] ?
Re: [PATCH RFC v3 07/12] dataplane: allow virtio-1 devices
On Wed, 26 Nov 2014 18:28:38 +0100 Cornelia Huck cornelia.h...@de.ibm.com wrote: Handle endianness conversion for virtio-1 virtqueues correctly. Note that dataplane now needs to be built per-target. Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com Build is ok now. Acked-by: Greg Kurz gk...@linux.vnet.ibm.com --- hw/block/dataplane/virtio-blk.c |4 +- hw/scsi/virtio-scsi-dataplane.c |2 +- hw/virtio/Makefile.objs |2 +- hw/virtio/dataplane/Makefile.objs |2 +- hw/virtio/dataplane/vring.c | 86 ++--- include/hw/virtio/dataplane/vring-accessors.h | 75 + include/hw/virtio/dataplane/vring.h | 14 +--- 7 files changed, 131 insertions(+), 54 deletions(-) create mode 100644 include/hw/virtio/dataplane/vring-accessors.h diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index 1222a37..2d8cc15 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -16,7 +16,9 @@ #include qemu/iov.h #include qemu/thread.h #include qemu/error-report.h +#include hw/virtio/virtio-access.h #include hw/virtio/dataplane/vring.h +#include hw/virtio/dataplane/vring-accessors.h #include sysemu/block-backend.h #include hw/virtio/virtio-blk.h #include virtio-blk.h @@ -75,7 +77,7 @@ static void complete_request_vring(VirtIOBlockReq *req, unsigned char status) VirtIOBlockDataPlane *s = req-dev-dataplane; stb_p(req-in-status, status); -vring_push(req-dev-dataplane-vring, req-elem, +vring_push(s-vdev, req-dev-dataplane-vring, req-elem, req-qiov.size + sizeof(*req-in)); /* Suppress notification to guest by BH and its scheduled diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c index 03a1e8c..418d73b 100644 --- a/hw/scsi/virtio-scsi-dataplane.c +++ b/hw/scsi/virtio-scsi-dataplane.c @@ -94,7 +94,7 @@ void virtio_scsi_vring_push_notify(VirtIOSCSIReq *req) { VirtIODevice *vdev = VIRTIO_DEVICE(req-vring-parent); -vring_push(req-vring-vring, req-elem, +vring_push(vdev, req-vring-vring, req-elem, req-qsgl.size + req-resp_iov.size); if (vring_should_notify(vdev, req-vring-vring)) { diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs index d21c397..19b224a 100644 --- a/hw/virtio/Makefile.objs +++ b/hw/virtio/Makefile.objs @@ -2,7 +2,7 @@ common-obj-y += virtio-rng.o common-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o common-obj-y += virtio-bus.o common-obj-y += virtio-mmio.o -common-obj-$(CONFIG_VIRTIO) += dataplane/ +obj-$(CONFIG_VIRTIO) += dataplane/ obj-y += virtio.o virtio-balloon.o obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o diff --git a/hw/virtio/dataplane/Makefile.objs b/hw/virtio/dataplane/Makefile.objs index 9a8cfc0..753a9ca 100644 --- a/hw/virtio/dataplane/Makefile.objs +++ b/hw/virtio/dataplane/Makefile.objs @@ -1 +1 @@ -common-obj-y += vring.o +obj-y += vring.o diff --git a/hw/virtio/dataplane/vring.c b/hw/virtio/dataplane/vring.c index a051775..0da8d6b 100644 --- a/hw/virtio/dataplane/vring.c +++ b/hw/virtio/dataplane/vring.c @@ -18,7 +18,9 @@ #include hw/hw.h #include exec/memory.h #include exec/address-spaces.h +#include hw/virtio/virtio-access.h #include hw/virtio/dataplane/vring.h +#include hw/virtio/dataplane/vring-accessors.h #include qemu/error-report.h /* vring_map can be coupled with vring_unmap or (if you still have the @@ -83,7 +85,7 @@ bool vring_setup(Vring *vring, VirtIODevice *vdev, int n) vring_init(vring-vr, virtio_queue_get_num(vdev, n), vring_ptr, 4096); vring-last_avail_idx = virtio_queue_get_last_avail_idx(vdev, n); -vring-last_used_idx = vring-vr.used-idx; +vring-last_used_idx = vring_get_used_idx(vdev, vring); vring-signalled_used = 0; vring-signalled_used_valid = false; @@ -104,7 +106,7 @@ void vring_teardown(Vring *vring, VirtIODevice *vdev, int n) void vring_disable_notification(VirtIODevice *vdev, Vring *vring) { if (!(vdev-guest_features[0] (1 VIRTIO_RING_F_EVENT_IDX))) { -vring-vr.used-flags |= VRING_USED_F_NO_NOTIFY; +vring_set_used_flags(vdev, vring, VRING_USED_F_NO_NOTIFY); } } @@ -117,10 +119,10 @@ bool vring_enable_notification(VirtIODevice *vdev, Vring *vring) if (vdev-guest_features[0] (1 VIRTIO_RING_F_EVENT_IDX)) { vring_avail_event(vring-vr) = vring-vr.avail-idx; } else { -vring-vr.used-flags = ~VRING_USED_F_NO_NOTIFY; +vring_clear_used_flags(vdev, vring, VRING_USED_F_NO_NOTIFY); } smp_mb(); /* ensure update is seen before reading avail_idx */ -return !vring_more_avail(vring); +return !vring_more_avail(vdev, vring); } /* This is stolen from linux/drivers/vhost/vhost.c:vhost_notify
Re: [Qemu-devel] [PATCH RFC v3 05/12] virtio: introduce legacy virtio devices
On Wed, 26 Nov 2014 18:28:36 +0100 Cornelia Huck cornelia.h...@de.ibm.com wrote: Introduce a helper function to indicate whether a virtio device is operating in legacy or virtio standard mode. It may be used to make decisions about the endianess of virtio accesses and other virtio-1 specific changes, enabling us to support transitional devices. Reviewed-by: Thomas Huth th...@linux.vnet.ibm.com Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- hw/virtio/virtio.c|6 +- include/hw/virtio/virtio-access.h |4 include/hw/virtio/virtio.h| 13 +++-- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 2eb5d3c..4149f45 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -883,7 +883,11 @@ static bool virtio_device_endian_needed(void *opaque) VirtIODevice *vdev = opaque; assert(vdev-device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN); -return vdev-device_endian != virtio_default_endian(); +if (virtio_device_is_legacy(vdev)) { +return vdev-device_endian != virtio_default_endian(); +} +/* Devices conforming to VIRTIO 1.0 or later are always LE. */ +return vdev-device_endian != VIRTIO_DEVICE_ENDIAN_LITTLE; } Sorry but I still don't understand why we need to stream the device_endian subsection if we have a virtio-1 device... this field is only used on legacy device paths. Can you share an example where it is needed ? static const VMStateDescription vmstate_virtio_device_endian = { diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h index 46456fd..c123ee0 100644 --- a/include/hw/virtio/virtio-access.h +++ b/include/hw/virtio/virtio-access.h @@ -19,6 +19,10 @@ static inline bool virtio_access_is_big_endian(VirtIODevice *vdev) { +if (!virtio_device_is_legacy(vdev)) { +/* Devices conforming to VIRTIO 1.0 or later are always LE. */ +return false; +} #if defined(TARGET_IS_BIENDIAN) return virtio_is_big_endian(vdev); #elif defined(TARGET_WORDS_BIGENDIAN) diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index b408166..40e567c 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -275,9 +275,18 @@ void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign, void virtio_queue_notify_vq(VirtQueue *vq); void virtio_irq(VirtQueue *vq); +static inline bool virtio_device_is_legacy(VirtIODevice *vdev) +{ +return !(vdev-guest_features[1] (1 (VIRTIO_F_VERSION_1 - 32))); +} + static inline bool virtio_is_big_endian(VirtIODevice *vdev) { -assert(vdev-device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN); -return vdev-device_endian == VIRTIO_DEVICE_ENDIAN_BIG; +if (virtio_device_is_legacy(vdev)) { +assert(vdev-device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN); +return vdev-device_endian == VIRTIO_DEVICE_ENDIAN_BIG; +} +/* Devices conforming to VIRTIO 1.0 or later are always LE. */ +return false; } #endif -- 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 RFC v2 07/12] dataplane: allow virtio-1 devices
On Tue, 25 Nov 2014 14:24:18 +0100 Cornelia Huck cornelia.h...@de.ibm.com wrote: Handle endianness conversion for virtio-1 virtqueues correctly. Note that dataplane now needs to be built per-target. Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- We still have the same error as in your previous post... In file included from include/hw/virtio/dataplane/vring.h:23:0, from include/hw/virtio/virtio-scsi.h:21, from hw/virtio/virtio-pci.c:24: include/hw/virtio/virtio-access.h: In function ‘virtio_access_is_big_endian’: include/hw/virtio/virtio-access.h:28:15: error: attempt to use poisoned TARGET_WORDS_BIGENDIAN #elif defined(TARGET_WORDS_BIGENDIAN) Either build virtio-pci per-target or probably better move the target tainted accessors to another file. hw/block/dataplane/virtio-blk.c |3 +- hw/scsi/virtio-scsi-dataplane.c |2 +- hw/virtio/Makefile.objs |2 +- hw/virtio/dataplane/Makefile.objs |2 +- hw/virtio/dataplane/vring.c | 85 +++ include/hw/virtio/dataplane/vring.h | 64 -- 6 files changed, 113 insertions(+), 45 deletions(-) diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index 1222a37..c25878c 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -16,6 +16,7 @@ #include qemu/iov.h #include qemu/thread.h #include qemu/error-report.h +#include hw/virtio/virtio-access.h #include hw/virtio/dataplane/vring.h #include sysemu/block-backend.h #include hw/virtio/virtio-blk.h @@ -75,7 +76,7 @@ static void complete_request_vring(VirtIOBlockReq *req, unsigned char status) VirtIOBlockDataPlane *s = req-dev-dataplane; stb_p(req-in-status, status); -vring_push(req-dev-dataplane-vring, req-elem, +vring_push(s-vdev, req-dev-dataplane-vring, req-elem, req-qiov.size + sizeof(*req-in)); /* Suppress notification to guest by BH and its scheduled diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c index 03a1e8c..418d73b 100644 --- a/hw/scsi/virtio-scsi-dataplane.c +++ b/hw/scsi/virtio-scsi-dataplane.c @@ -94,7 +94,7 @@ void virtio_scsi_vring_push_notify(VirtIOSCSIReq *req) { VirtIODevice *vdev = VIRTIO_DEVICE(req-vring-parent); -vring_push(req-vring-vring, req-elem, +vring_push(vdev, req-vring-vring, req-elem, req-qsgl.size + req-resp_iov.size); if (vring_should_notify(vdev, req-vring-vring)) { diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs index d21c397..19b224a 100644 --- a/hw/virtio/Makefile.objs +++ b/hw/virtio/Makefile.objs @@ -2,7 +2,7 @@ common-obj-y += virtio-rng.o common-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o common-obj-y += virtio-bus.o common-obj-y += virtio-mmio.o -common-obj-$(CONFIG_VIRTIO) += dataplane/ +obj-$(CONFIG_VIRTIO) += dataplane/ obj-y += virtio.o virtio-balloon.o obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o diff --git a/hw/virtio/dataplane/Makefile.objs b/hw/virtio/dataplane/Makefile.objs index 9a8cfc0..753a9ca 100644 --- a/hw/virtio/dataplane/Makefile.objs +++ b/hw/virtio/dataplane/Makefile.objs @@ -1 +1 @@ -common-obj-y += vring.o +obj-y += vring.o diff --git a/hw/virtio/dataplane/vring.c b/hw/virtio/dataplane/vring.c index a051775..69809ff 100644 --- a/hw/virtio/dataplane/vring.c +++ b/hw/virtio/dataplane/vring.c @@ -18,6 +18,7 @@ #include hw/hw.h #include exec/memory.h #include exec/address-spaces.h +#include hw/virtio/virtio-access.h #include hw/virtio/dataplane/vring.h #include qemu/error-report.h @@ -83,7 +84,7 @@ bool vring_setup(Vring *vring, VirtIODevice *vdev, int n) vring_init(vring-vr, virtio_queue_get_num(vdev, n), vring_ptr, 4096); vring-last_avail_idx = virtio_queue_get_last_avail_idx(vdev, n); -vring-last_used_idx = vring-vr.used-idx; +vring-last_used_idx = vring_get_used_idx(vdev, vring); vring-signalled_used = 0; vring-signalled_used_valid = false; @@ -104,7 +105,7 @@ void vring_teardown(Vring *vring, VirtIODevice *vdev, int n) void vring_disable_notification(VirtIODevice *vdev, Vring *vring) { if (!(vdev-guest_features[0] (1 VIRTIO_RING_F_EVENT_IDX))) { -vring-vr.used-flags |= VRING_USED_F_NO_NOTIFY; +vring_set_used_flags(vdev, vring, VRING_USED_F_NO_NOTIFY); } } @@ -117,10 +118,10 @@ bool vring_enable_notification(VirtIODevice *vdev, Vring *vring) if (vdev-guest_features[0] (1 VIRTIO_RING_F_EVENT_IDX)) { vring_avail_event(vring-vr) = vring-vr.avail-idx; } else { -vring-vr.used-flags = ~VRING_USED_F_NO_NOTIFY; +vring_clear_used_flags(vdev, vring, VRING_USED_F_NO_NOTIFY); } smp_mb(); /* ensure update is seen before reading avail_idx */ -return !vring_more_avail(vring); +return
Re: [Qemu-devel] [PATCH RFC 05/11] virtio: introduce legacy virtio devices
On Thu, 30 Oct 2014 19:02:01 +0100 Cornelia Huck cornelia.h...@de.ibm.com wrote: On Tue, 28 Oct 2014 16:40:18 +0100 Greg Kurz gk...@linux.vnet.ibm.com wrote: On Tue, 7 Oct 2014 16:40:01 +0200 Cornelia Huck cornelia.h...@de.ibm.com wrote: Introduce a helper function to indicate whether a virtio device is operating in legacy or virtio standard mode. It may be used to make decisions about the endianess of virtio accesses and other virtio-1 specific changes, enabling us to support transitional devices. Reviewed-by: Thomas Huth th...@linux.vnet.ibm.com Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- hw/virtio/virtio.c|6 +- include/hw/virtio/virtio-access.h |4 include/hw/virtio/virtio.h| 13 +++-- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 7aaa953..e6ae3a0 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -883,7 +883,11 @@ static bool virtio_device_endian_needed(void *opaque) VirtIODevice *vdev = opaque; assert(vdev-device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN); -return vdev-device_endian != virtio_default_endian(); +if (virtio_device_is_legacy(vdev)) { +return vdev-device_endian != virtio_default_endian(); +} +/* Devices conforming to VIRTIO 1.0 or later are always LE. */ +return vdev-device_endian != VIRTIO_DEVICE_ENDIAN_LITTLE; } Shouldn't we have some code doing the following somewhere ? if (!virtio_device_is_legacy(vdev)) { vdev-device_endian = VIRTIO_DEVICE_ENDIAN_LITTLE; } also, since virtio-1 is LE only, do we expect device_endian to be different from VIRTIO_DEVICE_ENDIAN_LITTLE ? device_endian should not depend on whether the device is legacy or not. virtio_is_big_endian always returns false for virtio-1 devices, though. Sorry, I had missed the virtio_is_big_endian() change: it that makes device_endian a legacy virtio only matter. So why would we care to migrate the endian subsection when we have a virtio-1 device ? Shouldn't virtio_device_endian_needed() return false for virtio-1 ? -- Greg -- 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] [PATCH RFC 07/11] dataplane: allow virtio-1 devices
On Tue, 7 Oct 2014 16:40:03 +0200 Cornelia Huck cornelia.h...@de.ibm.com wrote: Handle endianness conversion for virtio-1 virtqueues correctly. Note that dataplane now needs to be built per-target. It also affects hw/virtio/virtio-pci.c: In file included from include/hw/virtio/dataplane/vring.h:23:0, from include/hw/virtio/virtio-scsi.h:21, from hw/virtio/virtio-pci.c:24: include/hw/virtio/virtio-access.h: In function ‘virtio_access_is_big_endian’: include/hw/virtio/virtio-access.h:28:15: error: attempt to use poisoned TARGET_WORDS_BIGENDIAN #elif defined(TARGET_WORDS_BIGENDIAN) ^ FWIW when I added endian ambivalent support to virtio, I remember *some people* getting angry at the idea of turning common code into per-target... :) See comment below. Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- hw/block/dataplane/virtio-blk.c |3 +- hw/scsi/virtio-scsi-dataplane.c |2 +- hw/virtio/Makefile.objs |2 +- hw/virtio/dataplane/Makefile.objs |2 +- hw/virtio/dataplane/vring.c | 85 +++ include/hw/virtio/dataplane/vring.h | 64 -- 6 files changed, 113 insertions(+), 45 deletions(-) diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index 5458f9d..eb45a3d 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -16,6 +16,7 @@ #include qemu/iov.h #include qemu/thread.h #include qemu/error-report.h +#include hw/virtio/virtio-access.h #include hw/virtio/dataplane/vring.h #include block/block.h #include hw/virtio/virtio-blk.h @@ -75,7 +76,7 @@ static void complete_request_vring(VirtIOBlockReq *req, unsigned char status) VirtIOBlockDataPlane *s = req-dev-dataplane; stb_p(req-in-status, status); -vring_push(req-dev-dataplane-vring, req-elem, +vring_push(s-vdev, req-dev-dataplane-vring, req-elem, req-qiov.size + sizeof(*req-in)); /* Suppress notification to guest by BH and its scheduled diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c index b778e05..3e2b706 100644 --- a/hw/scsi/virtio-scsi-dataplane.c +++ b/hw/scsi/virtio-scsi-dataplane.c @@ -81,7 +81,7 @@ VirtIOSCSIReq *virtio_scsi_pop_req_vring(VirtIOSCSI *s, void virtio_scsi_vring_push_notify(VirtIOSCSIReq *req) { -vring_push(req-vring-vring, req-elem, +vring_push((VirtIODevice *)req-dev, req-vring-vring, req-elem, req-qsgl.size + req-resp_iov.size); event_notifier_set(req-vring-guest_notifier); } diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs index d21c397..19b224a 100644 --- a/hw/virtio/Makefile.objs +++ b/hw/virtio/Makefile.objs @@ -2,7 +2,7 @@ common-obj-y += virtio-rng.o common-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o common-obj-y += virtio-bus.o common-obj-y += virtio-mmio.o -common-obj-$(CONFIG_VIRTIO) += dataplane/ +obj-$(CONFIG_VIRTIO) += dataplane/ obj-y += virtio.o virtio-balloon.o obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o diff --git a/hw/virtio/dataplane/Makefile.objs b/hw/virtio/dataplane/Makefile.objs index 9a8cfc0..753a9ca 100644 --- a/hw/virtio/dataplane/Makefile.objs +++ b/hw/virtio/dataplane/Makefile.objs @@ -1 +1 @@ -common-obj-y += vring.o +obj-y += vring.o diff --git a/hw/virtio/dataplane/vring.c b/hw/virtio/dataplane/vring.c index b84957f..4624521 100644 --- a/hw/virtio/dataplane/vring.c +++ b/hw/virtio/dataplane/vring.c @@ -18,6 +18,7 @@ #include hw/hw.h #include exec/memory.h #include exec/address-spaces.h +#include hw/virtio/virtio-access.h #include hw/virtio/dataplane/vring.h #include qemu/error-report.h @@ -83,7 +84,7 @@ bool vring_setup(Vring *vring, VirtIODevice *vdev, int n) vring_init(vring-vr, virtio_queue_get_num(vdev, n), vring_ptr, 4096); vring-last_avail_idx = virtio_queue_get_last_avail_idx(vdev, n); -vring-last_used_idx = vring-vr.used-idx; +vring-last_used_idx = vring_get_used_idx(vdev, vring); vring-signalled_used = 0; vring-signalled_used_valid = false; @@ -104,7 +105,7 @@ void vring_teardown(Vring *vring, VirtIODevice *vdev, int n) void vring_disable_notification(VirtIODevice *vdev, Vring *vring) { if (!(vdev-guest_features[0] (1 VIRTIO_RING_F_EVENT_IDX))) { -vring-vr.used-flags |= VRING_USED_F_NO_NOTIFY; +vring_set_used_flags(vdev, vring, VRING_USED_F_NO_NOTIFY); } } @@ -117,10 +118,10 @@ bool vring_enable_notification(VirtIODevice *vdev, Vring *vring) if (vdev-guest_features[0] (1 VIRTIO_RING_F_EVENT_IDX)) { vring_avail_event(vring-vr) = vring-vr.avail-idx; } else { -vring-vr.used-flags = ~VRING_USED_F_NO_NOTIFY; +vring_clear_used_flags(vdev, vring, VRING_USED_F_NO_NOTIFY); } smp_mb(); /* ensure update is seen before
Re: [Qemu-devel] [PATCH RFC 05/11] virtio: introduce legacy virtio devices
On Tue, 7 Oct 2014 16:40:01 +0200 Cornelia Huck cornelia.h...@de.ibm.com wrote: Introduce a helper function to indicate whether a virtio device is operating in legacy or virtio standard mode. It may be used to make decisions about the endianess of virtio accesses and other virtio-1 specific changes, enabling us to support transitional devices. Reviewed-by: Thomas Huth th...@linux.vnet.ibm.com Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- hw/virtio/virtio.c|6 +- include/hw/virtio/virtio-access.h |4 include/hw/virtio/virtio.h| 13 +++-- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 7aaa953..e6ae3a0 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -883,7 +883,11 @@ static bool virtio_device_endian_needed(void *opaque) VirtIODevice *vdev = opaque; assert(vdev-device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN); -return vdev-device_endian != virtio_default_endian(); +if (virtio_device_is_legacy(vdev)) { +return vdev-device_endian != virtio_default_endian(); +} +/* Devices conforming to VIRTIO 1.0 or later are always LE. */ +return vdev-device_endian != VIRTIO_DEVICE_ENDIAN_LITTLE; } Shouldn't we have some code doing the following somewhere ? if (!virtio_device_is_legacy(vdev)) { vdev-device_endian = VIRTIO_DEVICE_ENDIAN_LITTLE; } also, since virtio-1 is LE only, do we expect device_endian to be different from VIRTIO_DEVICE_ENDIAN_LITTLE ? Cheers. -- Greg static const VMStateDescription vmstate_virtio_device_endian = { diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h index 46456fd..c123ee0 100644 --- a/include/hw/virtio/virtio-access.h +++ b/include/hw/virtio/virtio-access.h @@ -19,6 +19,10 @@ static inline bool virtio_access_is_big_endian(VirtIODevice *vdev) { +if (!virtio_device_is_legacy(vdev)) { +/* Devices conforming to VIRTIO 1.0 or later are always LE. */ +return false; +} #if defined(TARGET_IS_BIENDIAN) return virtio_is_big_endian(vdev); #elif defined(TARGET_WORDS_BIGENDIAN) diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index b408166..40e567c 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -275,9 +275,18 @@ void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign, void virtio_queue_notify_vq(VirtQueue *vq); void virtio_irq(VirtQueue *vq); +static inline bool virtio_device_is_legacy(VirtIODevice *vdev) +{ +return !(vdev-guest_features[1] (1 (VIRTIO_F_VERSION_1 - 32))); +} + static inline bool virtio_is_big_endian(VirtIODevice *vdev) { -assert(vdev-device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN); -return vdev-device_endian == VIRTIO_DEVICE_ENDIAN_BIG; +if (virtio_device_is_legacy(vdev)) { +assert(vdev-device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN); +return vdev-device_endian == VIRTIO_DEVICE_ENDIAN_BIG; +} +/* Devices conforming to VIRTIO 1.0 or later are always LE. */ +return false; } #endif -- 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 9/9] kvmtool: virtio: enable arm/arm64 support for bi-endianness
On Wed, 07 May 2014 10:52:01 +0100 Marc Zyngier marc.zyng...@arm.com wrote: On Wed, May 07 2014 at 10:34:30 am BST, Peter Maydell peter.mayd...@linaro.org wrote: On 6 May 2014 19:38, Peter Maydell peter.mayd...@linaro.org wrote: On 6 May 2014 18:25, Marc Zyngier marc.zyng...@arm.com wrote: On Tue, May 06 2014 at 3:28:07 pm BST, Will Deacon will.dea...@arm.com wrote: On Thu, Apr 24, 2014 at 07:17:23PM +0100, Marc Zyngier wrote: +reg.addr = (u64)data; +if (ioctl(vcpu-vcpu_fd, KVM_GET_ONE_REG, reg) 0) +die(KVM_GET_ONE_REG failed (SCTLR_EL1)); + +return (data SCTLR_EL1_EE_MASK) ? VIRTIO_ENDIAN_BE : VIRTIO_ENDIAN_LE; This rules out guests where userspace and kernelspace can run with different endinness. Whilst Linux doesn't currently do this, can we support it here? It all gets a bit hairy if the guest is using a stage-1 SMMU to let userspace play with a virtio device... Yeah, I suppose we could check either EE or E0 depending on the mode when the access was made. We already have all the information, just need to handle the case. I'll respin the series. How virtio implementations should determine their endianness is a spec question, I think; at any rate QEMU and kvmtool ought to agree on how it's done. I think the most recent suggestion on the QEMU mailing list (for PPC) is that we should care about the guest kernel endianness, but I don't know if anybody thought of the pass-through-to-userspace usecase... Current opinion on the qemu-devel thread seems to be that we should just define that the endianness of the virtio device is the endianness of the guest kernel at the point where the guest triggers a reset of the virtio device by writing zero the QueuePFN or Status registers. On AArch32, we only have the CPSR.E bit to select the endiannes. Are we going to simply explode if the access comes from userspace? On AArch64, we can either select the kernel endianness, or userspace endianness. Are we going to go a different route just for the sake of enforcing kernel access? I'm inclined to think of userspace access as a valid use case. M. All the fuzz is not really about enforcing kernel access... PPC also has a current endianness selector (MSR_LE) but it only makes sense if you are in the cpu context. Initial versions of the virtio biendian support for QEMU PPC64 used an arbitrary cpu: in this case, the only sensible thing to look at to support kernel based virtio is the interrupt endianness selector (LPCR_ILE), because if gives a safe hint of the kernel endianness. The patch set has evolved and now uses current_cpu at device reset time. As a consequence, we are not necessarily tied to the kernel LPCR_ILE selector I guess. Cheers. -- Gregory Kurz kurzg...@fr.ibm.com gk...@linux.vnet.ibm.com Software Engineer @ IBM/Meiosys http://www.ibm.com Tel +33 (0)562 165 496 Anarchy is about taking complete responsibility for yourself. Alan Moore. -- 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 9/9] kvmtool: virtio: enable arm/arm64 support for bi-endianness
On Wed, 7 May 2014 13:17:51 +0100 Peter Maydell peter.mayd...@linaro.org wrote: On 7 May 2014 12:04, Marc Zyngier marc.zyng...@arm.com wrote: On Wed, May 07 2014 at 11:40:54 am BST, Greg Kurz gk...@linux.vnet.ibm.com wrote: All the fuzz is not really about enforcing kernel access... PPC also has a current endianness selector (MSR_LE) but it only makes sense if you are in the cpu context. Initial versions of the virtio biendian support for QEMU PPC64 used an arbitrary cpu: in this case, the only sensible thing to look at to support kernel based virtio is the interrupt endianness selector (LPCR_ILE), because if gives a safe hint of the kernel endianness. The patch set has evolved and now uses current_cpu at device reset time. As a consequence, we are not necessarily tied to the kernel LPCR_ILE selector I guess. Ah yes, I'd forgotten the history behind why we ended up looking at interrupt endianness. That makes a lot of sense, thanks for explaining that. You're basically doing the exact same thing we do with kvmtool on ARM. So if we have similar architectural features on both sides, why don't we support both kernel and userspace access? I don't think that we really need to get into whether userspace access is or is not a good idea -- endianness of the CPU which does the virtio reset at the point when it does that reset is a nice simple rule that should generalise across architectures, so why make it more complicated than that? thanks -- PMM I am convinced... and feeling a bit guilty for all the noise ;) I'll come with a new virtio patch set for QEMU that does just what you say. -- Gregory Kurz kurzg...@fr.ibm.com gk...@linux.vnet.ibm.com Software Engineer @ IBM/Meiosys http://www.ibm.com Tel +33 (0)562 165 496 Anarchy is about taking complete responsibility for yourself. Alan Moore. -- 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: Preparing kvm/next for first pull request of 3.15 merge window
On Wed, 26 Mar 2014 10:19:52 +0100 Paolo Bonzini pbonz...@redhat.com wrote: Il 26/03/2014 04:51, Paul Mackerras ha scritto: I would like to know from ARM and PPC maintainers *now* (before the merge window opens) what will be in 3.15. Also, PPC guys, please make sure the pull requests will be based on commit e724f080f5dd (KVM: PPC: Book3S HV: Fix register usage when loading/saving VRSAVE, 2014-03-13). Alex has 5 commits in his kvm-ppc-queue branch that should go in. That tree is based on your kvm-3.14-2 tag, not on e724f080f5dd. Would you accept a pull request for that from me on Alex's behalf since he's away? Yes, of course. Do you need me to rebase it at all? These are: 1ac4484979 PPC: KVM: Introduce hypervisor call H_GET_TCE e0a7be38c9 PPC: KVM: fix RESUME_GUEST check before returning from kvmppc_run_core() 013e98e657 PPC: KVM: fix RESUME_GUEST check before ending CEDE in kvmppc_run_core() e8a7f11fe0 PPC: KVM: fix VCPU run for HV KVM (v2) 26d96ec97c PPC: KVM: introduce helper to check RESUME_GUEST and related I would consider rebasing; but you know better than me the effect of the two host-crash-fixing patches and how the testability/bisectability of kvm-ppc-queue is affected. In particular, how likely is it that reverse-bisection (which commit fixed the bug?) would end up on e724f080f5dd rather than one of the four RESUME_GUEST commits? Paul, It is safe to merge e0a7be38c9, 013e98e657, e8a7f11fe0 and 26d96ec97c as a single patch... thing I should I've done from the beginning I guess. :) Cheers. -- Greg There is also the set of 8 patches that I just posted and Scott acked. I can make a branch based on e724f080f5dd and send you a pull request. Yes, thanks! Paolo Let me know what you would prefer regarding Alex's kvm-ppc-queue branch. -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- Gregory Kurz kurzg...@fr.ibm.com gk...@linux.vnet.ibm.com Software Engineer @ IBM/Meiosys http://www.ibm.com Tel +33 (0)562 165 496 Anarchy is about taking complete responsibility for yourself. Alan Moore. -- 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 0/4] PPC: KVM: fix RESUME_GUEST checks
On Thu, 6 Feb 2014 18:39:30 +0100 Alexander Graf ag...@suse.de wrote: On 06.02.2014, at 17:36, Greg Kurz gk...@linux.vnet.ibm.com wrote: As discussed in this thread: http://patchwork.ozlabs.org/patch/309166/ We need some consistency in the way we check whether the guest should resume or not because: - new RESUME_GUEST_XXX values may show up - more locations in KVM may need to perform a similar check This serie introduces a helper and patches the locations where it should be called. There is yet another location in __kvmppc_vcpu_run, but it is assembly and cannot call a C inlined function. Thanks, applied all to kvm-ppc-queue. I think the splitting on this one is quite excessive - a single patch would've done :). Alex Heh... I paranoid ! :) Thanks Alex ! -- Greg -- 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 1/4] PPC: KVM: introduce helper to check RESUME_GUEST and related
There are some locations where we check if the return value of a service indicates that we should resume the guest. This patch introduces a helper to handle the two possible values we currently have: - RESUME_GUEST - RESUME_GUEST_NV Since it only affects Book3S HV for now, the helper is added to the kvm_book3s.h header file. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- arch/powerpc/include/asm/kvm_book3s.h |5 + 1 file changed, 5 insertions(+) diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h index 83851aa..bb1e38a 100644 --- a/arch/powerpc/include/asm/kvm_book3s.h +++ b/arch/powerpc/include/asm/kvm_book3s.h @@ -304,6 +304,11 @@ static inline ulong kvmppc_get_fault_dar(struct kvm_vcpu *vcpu) return vcpu-arch.fault_dar; } +static inline bool is_kvmppc_resume_guest(int r) +{ + return (r == RESUME_GUEST || r == RESUME_GUEST_NV); +} + /* Magic register values loaded into r3 and r4 before the 'sc' assembly * instruction for the OSI hypercalls */ #define OSI_SC_MAGIC_R30x113724FA -- 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 2/4] PPC: KVM: fix VCPU run for HV KVM (v2)
From: Alexey Kardashevskiy a...@ozlabs.ru When write to MMIO happens and there is an ioeventfd for that and is handled successfully, ioeventfd_write() returns 0 (success) and kvmppc_handle_store() returns EMULATE_DONE. Then kvmppc_emulate_mmio() converts EMULATE_DONE to RESUME_GUEST_NV and this broke from the loop. This adds handling of RESUME_GUEST_NV in kvmppc_vcpu_run_hv(). changes in v2: - as suggesed by Alex Graf on kvm-ppc@, do it with a helper Cc: Michael S. Tsirkin m...@redhat.com Suggested-by: Paul Mackerras pau...@samba.org Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- arch/powerpc/kvm/book3s_hv.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c index 17fc949..d62dc6c 100644 --- a/arch/powerpc/kvm/book3s_hv.c +++ b/arch/powerpc/kvm/book3s_hv.c @@ -1731,7 +1731,7 @@ static int kvmppc_vcpu_run_hv(struct kvm_run *run, struct kvm_vcpu *vcpu) vcpu-arch.fault_dar, vcpu-arch.fault_dsisr); srcu_read_unlock(vcpu-kvm-srcu, srcu_idx); } - } while (r == RESUME_GUEST); + } while (is_kvmppc_resume_guest(r)); out: vcpu-arch.state = KVMPPC_VCPU_NOTREADY; -- 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 0/4] PPC: KVM: fix RESUME_GUEST checks
As discussed in this thread: http://patchwork.ozlabs.org/patch/309166/ We need some consistency in the way we check whether the guest should resume or not because: - new RESUME_GUEST_XXX values may show up - more locations in KVM may need to perform a similar check This serie introduces a helper and patches the locations where it should be called. There is yet another location in __kvmppc_vcpu_run, but it is assembly and cannot call a C inlined function. --- Alexey Kardashevskiy (1): PPC: KVM: fix VCPU run for HV KVM (v2) Greg Kurz (3): PPC: KVM: introduce helper to check RESUME_GUEST and related PPC: KVM: fix RESUME_GUEST check before ending CEDE in kvmppc_run_core() PPC: KVM: fix RESUME_GUEST check before returning from kvmppc_run_core() arch/powerpc/include/asm/kvm_book3s.h |5 + arch/powerpc/kvm/book3s_hv.c |6 +++--- 2 files changed, 8 insertions(+), 3 deletions(-) -- Greg Kurz -- 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 4/4] PPC: KVM: fix RESUME_GUEST check before returning from kvmppc_run_core()
Let's use a helper for this, in case new RESUME_GUEST_XXX values are introduced. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- arch/powerpc/kvm/book3s_hv.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c index 85df6a4..3b498d9 100644 --- a/arch/powerpc/kvm/book3s_hv.c +++ b/arch/powerpc/kvm/book3s_hv.c @@ -1541,7 +1541,7 @@ static void kvmppc_run_core(struct kvmppc_vcore *vc) vc-vcore_state = VCORE_INACTIVE; list_for_each_entry_safe(vcpu, vnext, vc-runnable_threads, arch.run_list) { - if (vcpu-arch.ret != RESUME_GUEST) { + if (!is_kvmppc_resume_guest(vcpu-arch.ret)) { kvmppc_remove_runnable(vc, vcpu); wake_up(vcpu-arch.cpu_run); } -- 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 3/4] PPC: KVM: fix RESUME_GUEST check before ending CEDE in kvmppc_run_core()
Let's use a helper for this, in case new RESUME_GUEST_XXX values are introduced. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- arch/powerpc/kvm/book3s_hv.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c index d62dc6c..85df6a4 100644 --- a/arch/powerpc/kvm/book3s_hv.c +++ b/arch/powerpc/kvm/book3s_hv.c @@ -1530,7 +1530,7 @@ static void kvmppc_run_core(struct kvmppc_vcore *vc) vcpu-arch.trap = 0; if (vcpu-arch.ceded) { - if (ret != RESUME_GUEST) + if (!is_kvmppc_resume_guest(ret)) kvmppc_end_cede(vcpu); else kvmppc_set_timer(vcpu); -- 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 3/4] PPC: KVM: fix RESUME_GUEST check before ending CEDE in kvmppc_run_core()
Let's use a helper for this, in case new RESUME_GUEST_XXX values are introduced. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- arch/powerpc/kvm/book3s_hv.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c index d62dc6c..85df6a4 100644 --- a/arch/powerpc/kvm/book3s_hv.c +++ b/arch/powerpc/kvm/book3s_hv.c @@ -1530,7 +1530,7 @@ static void kvmppc_run_core(struct kvmppc_vcore *vc) vcpu-arch.trap = 0; if (vcpu-arch.ceded) { - if (ret != RESUME_GUEST) + if (!is_kvmppc_resume_guest(ret)) kvmppc_end_cede(vcpu); else kvmppc_set_timer(vcpu); -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] PPC: KVM: introduce helper to check RESUME_GUEST and related
There are some locations where we check if the return value of a service indicates that we should resume the guest. This patch introduces a helper to handle the two possible values we currently have: - RESUME_GUEST - RESUME_GUEST_NV Since it only affects Book3S HV for now, the helper is added to the kvm_book3s.h header file. Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- arch/powerpc/include/asm/kvm_book3s.h |5 + 1 file changed, 5 insertions(+) diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h index 83851aa..bb1e38a 100644 --- a/arch/powerpc/include/asm/kvm_book3s.h +++ b/arch/powerpc/include/asm/kvm_book3s.h @@ -304,6 +304,11 @@ static inline ulong kvmppc_get_fault_dar(struct kvm_vcpu *vcpu) return vcpu-arch.fault_dar; } +static inline bool is_kvmppc_resume_guest(int r) +{ + return (r == RESUME_GUEST || r == RESUME_GUEST_NV); +} + /* Magic register values loaded into r3 and r4 before the 'sc' assembly * instruction for the OSI hypercalls */ #define OSI_SC_MAGIC_R30x113724FA -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/4] PPC: KVM: fix RESUME_GUEST checks
As discussed in this thread: http://patchwork.ozlabs.org/patch/309166/ We need some consistency in the way we check whether the guest should resume or not because: - new RESUME_GUEST_XXX values may show up - more locations in KVM may need to perform a similar check This serie introduces a helper and patches the locations where it should be called. There is yet another location in __kvmppc_vcpu_run, but it is assembly and cannot call a C inlined function. --- Alexey Kardashevskiy (1): PPC: KVM: fix VCPU run for HV KVM (v2) Greg Kurz (3): PPC: KVM: introduce helper to check RESUME_GUEST and related PPC: KVM: fix RESUME_GUEST check before ending CEDE in kvmppc_run_core() PPC: KVM: fix RESUME_GUEST check before returning from kvmppc_run_core() arch/powerpc/include/asm/kvm_book3s.h |5 + arch/powerpc/kvm/book3s_hv.c |6 +++--- 2 files changed, 8 insertions(+), 3 deletions(-) -- Greg Kurz -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-ppc] KVM and variable-endianness guest CPUs
On Wed, 22 Jan 2014 20:25:05 -0800 Victor Kamensky victor.kamen...@linaro.org wrote: Hi Alex, Sorry, for delayed reply, I was focusing on discussion with Peter. Hope you and other folks may get something out of it :). Please see responses inline On 22 January 2014 02:52, Alexander Graf ag...@suse.de wrote: On 22.01.2014, at 08:26, Victor Kamensky victor.kamen...@linaro.org wrote: On 21 January 2014 22:41, Alexander Graf ag...@suse.de wrote: Native endian really is just a shortcut for target endian which is LE for ARM and BE for PPC. There shouldn't be a qemu-system-armeb or qemu-system-ppc64le. I disagree. Fully functional ARM BE system is what we've been working on for last few months. 'We' is Linaro Networking Group, Endian subteam and some other guys in ARM and across community. Why we do that is a bit beyond of this discussion. ARM BE patches for both V7 and V8 are already in mainline kernel. But ARM BE KVM host is broken now. It is known deficiency that I am trying to fix. Please look at [1]. Patches for V7 BE KVM were proposed and currently under active discussion. Currently I work on ARM V8 BE KVM changes. So native endian in ARM is value of CPSR register E bit. If it is off native endian is LE, if it is on it is BE. Once and if we agree on ARM BE KVM host changes, the next step would be patches in qemu one of which introduces qemu-system-armeb. Please see [2]. I think we're facing an ideology conflict here. Yes, there should be a qemu-system-arm that is BE capable. Maybe it is not ideology conflict but rather terminology clarity issue :). I am not sure what do you mean by qemu-system-arm that is BE capable. In qemu build system there is just target name 'arm', which is ARM V7 cpu in LE mode, and 'armeb' target which is ARM V7 cpu in BE mode. That is true for a lot of open source packages. You could check [1] patch that introduces armeb target into qemu. Build for arm target produces qemu-system-arm executable that is marked 'ELF 32-bit LSB executable' and it could run on LE traditional ARM Linux. Build for armeb target produces qemu-system-armeb executable that is marked 'ELF 32-bit MSB executable' that can run on BE ARM Linux. armbe is nothing special here, just build option for qemu that should run on BE ARM Linux. Hmmm... it looks like there is a confusion about the qemu command naming. The -target suffix in qemu-system-target has nothing to do with the ELF information of the command itself. [greg@bahia ~]$ file `which qemu-system-arm` /bin/qemu-system-arm: ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically linked (uses shared libs), for GNU/Linux 2.6.32, BuildID[sha1]=0xbcb974847daa8159c17ed74906cd5351387d4097, stripped It is valid to create a new target if it is substantially different from existing ones (ppc64 versus ppc for example). This is not the case with ARM since it is the very same CPU that can switch endianess with the 'setend' instruction (which needs anyway to be emulated when running in TCG mode). qemu-system-arm is THE command that should be able to emulate an ARM cpu, whether the guest does 'setend le' or 'setend be'. Both qemu-system-arm and qemu-system-armeb should be BE/LE capable. I.e either of them along with KVM could either run LE or BE guest. MarcZ demonstrated that this is possible. I've tested both LE and BE guests with qemu-system-arm running on traditional LE ARM Linux, effectively repeating Marc's setup but with qemu. And I did test with my patches both BE and LE guests with qemu-system-armeb running on BE ARM Linux. There should also be a qemu-system-ppc64 that is LE capable. But there is no point in changing the default endiannes for the virtual CPUs that we plug in there. Both CPUs are perfectly capable of running in LE or BE mode, the question is just what we declare the default. I am not sure, what you mean by default? Is it initial setting of CPSR E bit and 'cp15 c1, c0, 0' EE bit? Yes, the way it is currently implemented by committed qemu-system-arm, and proposed qemu-system-armeb patches, they are both off. I.e even qemu-system-armeb starts running vcpu in LE mode, exactly by very similar reason as desribed in your next paragraph qemu-system-armeb has tiny bootloader that starts in LE mode, jumps to kernel kernel switches cpu to run in BE mode 'setend be' and EE bit is set just before mmu is enabled. Think about the PPC bootstrap. We start off with a BE firmware, then boot into the Linux kernel which calls a hypercall to set the LE bit on every interrupt. We have very similar situation with BE ARM Linux. When we run ARM BE Linux we start with bootloader which is LE and then CPU issues 'setend be' very soon as it starts executing kernel code, all secondary CPUs issue 'setend be' when they go out of reset pen or bootmonitor sleep. But there's no reason this little endian kernel couldn't
Re: [Qemu-ppc] [RFC] QEMU/KVM PowerPC: virtio and guest endianness
On Fri, 4 Oct 2013 13:43:38 +0200 Alexander Graf ag...@suse.de wrote: CC'ing qemu-devel - please use qemu-ppc@ only as a tag, every mail needs to go to qemu-devel as well. Sure I will. On 03.10.2013, at 16:29, Greg Kurz wrote: [...] I have searched for an appropriate place to add the polling and I must admit I did not find any... I am no QEMU expert but I suspect we would need some kind of arch specific hook to be called from the virtio code to do this... :-\ I hope I am wrong, please correct me if so. Just put it into the normal register sync function and call cpu_synchronize_state() on virtio reset. Thanks, I will look into that. We have to decide which scheme to follow. There are 2 way we can / should handle registers usually: a) owned by QEMU b) owned by KVM If they're owned by QEMU, every hypercall needs to go into QEMU which then propagates that change through an ioctl back into KVM. If they're owned by KVM, QEMU needs to fetch them whenever it needs to As a general rule of thumb path b is easier to hack up, path a is easier to maintain long term. Which is pretty much what you're seeing here. Agreed. I have a better feeling for (2) because: - 2-liner patch in KVM - no extra code change in QEMU - already *partially* tested I don't understand. QEMU would get triggered, then have to propagate things back into KVM. We definitely do _not_ want KVM to do magic, then tell QEMU to handle a hypercall again. My idea was to have KVM and QEMU be like *co-owners* of the LPCR register... now I admit it was a bad idea ! :) Also, I understood Rusty is working on the next virtio specification which should address the endian issue: probably not worth to add too many temporary lines in the QEMU code... Does 3.13 support LE mode? Does 3.13 support the new and shiny virtio spec? There's a good chance we'd have to deal with guest kernels that can do LE, but not sane virtio. I don't know any details about 3.13 but I guess you are right, it is unlikely the guests will have it. Of course, I probably lack some essential knowledge that would be more favorable to (1)... so please comment and argue ! :) I think a 100% QEMU implementation that just goes through all vcpus and does a simple SET_ONE_REG for LPCR to set ILE would be the best. Anton's patch isn't in Linus' tree yet, right? So all it takes is a partial revert of that one to not handle the actual hypercall in KVM. And some code in kvmppc_set_lpcr() to also set intr_msr (not changing it is a bug today already). Alex Thanks for your indications Alex. Cheers. -- Greg -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-ppc] [RFC] QEMU/KVM PowerPC: virtio and guest endianness
Answering to both Paul and Alex. On Fri, 4 Oct 2013 13:54:25 +0200 Alexander Graf ag...@suse.de wrote: On 04.10.2013, at 13:53, Paul Mackerras wrote: I don't mind particularly whether H_SET_MODE for the endianness setting gets handled in the kernel or in QEMU, but I don't think it should be handled in both. If you want QEMU to know about the endianness setting immediately, make the kernel version do nothing and get QEMU to handle it -- which if KVM is enabled will mean iterating over all vcpus and getting them all to send the new LPCR setting to the kernel via the SET_ONE_REG ioctl. However, I want the setting of breakpoint registers (CIABR and DAWR/X) via H_SET_MODE to happen in the kernel, preferably in real mode, since that can happen on context switch and thus needs to be quick. Paul, As far as virtio is concerned, QEMU only needs to know about the guest endiannes if a virtio device shows up. The virtio reset flow is a good candiadate for that. I don't want to see a single hypercall be split across the QEMU/KVM barrier. So if there's a reasonable incentive to handle H_SET_MODE in KVM, we should handle all of it in KVM. Alex, The appropriate solution would be then to let KVM implement the whole H_SET_MODE hcall and own LPCR. QEMU will poll it with cpu_synchronize_state(). It seems to preserve all the requirements. Alex -- Thanks for your guidance. Cheers. -- Greg -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] QEMU/KVM PowerPC: virtio and guest endianness
Hi, There have been some work on the topic lately but no agreement has been reached yet. I want to consolidate the facts in a single thread of mail and re-start the discussion. Please find below a recap of what we have as of today: From a virtio POV, guest endianness is reflected by the endianness of the interrupt vectors (ILE bit in the LPCR register). The guest kernel relies on the H_SET_MODE_RESOURCE_LE hcall to set this bit, early in the boot process. Rusty sent a patchset on qemu-devel@ to provide the necessary bits to perform byteswap in the QEMU: http://patchwork.ozlabs.org/patch/266451/ http://patchwork.ozlabs.org/patch/266452/ http://patchwork.ozlabs.org/patch/266450/ (plus other enablement patches for virtio drivers, not essential for the discussion). In non-KVM mode, QEMU implements the H_SET_MODE_RESOURCE_LE and updates its internal value for LPCR when the guest requests it. Rusty's patchset works out-of-the-box in this mode: I could successfully setup and use a 9p share over virtio transport (broader virtio testing still to be done though). When using KVM, the story is different : QEMU is not on this endianness change flow anymore, providing KVM has the following patch from Anton: http://patchwork.ozlabs.org/patch/277079/ There are *at least* two approaches to bring back endianness knowledge to QEMU: polling (1) and propagation (2). (1) QEMU must retrieve LPCR from the kernel using the following API: http://patchwork.ozlabs.org/patch/273029/ (2) KVM can resume execution to the host and thus propagating H_SET_MODE_RESOURCE_LE to QEMU. Laurent came up with a patch on linuxppc-dev@ to do this: http://patchwork.ozlabs.org/patch/278590/ I would say (1) is a standard and sane way of addressing the issue: since the LPCR register value is held by KVM, it makes sense to introduce an API to get/set it. Then, it is up to QEMU to use this API. We can dumbly do the polling in all the places where byteswapping matters: it is clearly sub-optimized, especially since the LPCR_ILE bit doesn't change so often. Rusty suggested we can retrieve it at virtio device reset time and cache it, since an endianness change after the devices have started to be used is non-sensical. I have searched for an appropriate place to add the polling and I must admit I did not find any... I am no QEMU expert but I suspect we would need some kind of arch specific hook to be called from the virtio code to do this... :-\ I hope I am wrong, please correct me if so. On the other hand, (2) looks a bit hacky: KVM usually returns to the host when it cannot fully handle the h_call. Propagating may look like a useless path to follow from a KVM POV. From a QEMU POV, things are different: propagation will trig the fallback code in QEMU, already working in non-KVM mode. Nothing more to be done. I have a better feeling for (2) because: - 2-liner patch in KVM - no extra code change in QEMU - already *partially* tested Also, I understood Rusty is working on the next virtio specification which should address the endian issue: probably not worth to add too many temporary lines in the QEMU code... Of course, I probably lack some essential knowledge that would be more favorable to (1)... so please comment and argue ! :) Thanks. -- Greg Kurz -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html