On Tue, Mar 20, 2018 at 02:01:07AM +0000, Zhoujian (jay) wrote:
>
>
> > -----Original Message-----
> > From: Michael S. Tsirkin [mailto:m...@redhat.com]
> > Sent: Saturday, March 17, 2018 2:20 AM
> > To: qemu-devel@nongnu.org
> > Cc: Zhoujian (jay) <jianjay.z...@huawei.com>
> > Subject: [PATCH] vhost-user: avoid misaligned access
> >
> > We can't pass a pointer to memory field directly since it's within a packed
> > structure, so isn't aligned.
> > Pass a pointer on stack and copy.
> >
> > Fixes: 30c4cc7 ("vhost: used_memslots refactoring")
> > Cc: Jay Zhou <jianjay.z...@huawei.com>
> > Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
> > ---
> >
> > I had to apply this to fix make check errors with clang.
> > Pls review, test and ack.
> >
> > Thanks!
> >
> > hw/virtio/vhost-user.c | 14 +++++++++++---
> > 1 file changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index
> > c12fdd9..a44ee7f 100644
> > --- a/hw/virtio/vhost-user.c
> > +++ b/hw/virtio/vhost-user.c
> > @@ -396,6 +396,7 @@ static int vhost_user_set_mem_table_postcopy(struct
> > vhost_dev *dev,
> > bool reply_supported = virtio_has_feature(dev->protocol_features,
> >
> > VHOST_USER_PROTOCOL_F_REPLY_ACK);
> > VhostUserMsg msg_reply;
> > + VhostUserMemory memory = {};
> > int region_i, msg_i;
> >
> > VhostUserMsg msg = {
> > @@ -407,10 +408,11 @@ static int vhost_user_set_mem_table_postcopy(struct
> > vhost_dev *dev,
> > msg.hdr.flags |= VHOST_USER_NEED_REPLY_MASK;
> > }
> >
> > - if (vhost_user_prepare_msg(dev, &msg.payload.memory, fds) < 0) {
> > + if (vhost_user_prepare_msg(dev, &memory, fds) < 0) {
> > error_report("Failed preparing vhost-user memory table msg");
> > return -1;
> > }
> > + msg.payload.memory = memory;
> >
> > fd_num = msg.payload.memory.nregions;
> >
> > @@ -549,16 +551,19 @@ static int vhost_user_set_mem_table(struct vhost_dev
> > *dev,
> > .hdr.request = VHOST_USER_SET_MEM_TABLE,
> > .hdr.flags = VHOST_USER_VERSION,
> > };
> > + VhostUserMemory memory = {};
> >
> > if (reply_supported) {
> > msg.hdr.flags |= VHOST_USER_NEED_REPLY_MASK;
> > }
> >
> > - if (vhost_user_prepare_msg(dev, &msg.payload.memory, fds) < 0) {
> > + if (vhost_user_prepare_msg(dev, &memory, fds) < 0) {
> > error_report("Failed preparing vhost-user memory table msg");
> > return -1;
> > }
> >
> > + msg.payload.memory = memory;
> > +
> > fd_num = msg.payload.memory.nregions;
> >
> > if (!fd_num) {
> > @@ -1575,8 +1580,11 @@ static void vhost_user_set_used_memslots(struct
> > vhost_dev *dev) {
> > int fds[VHOST_MEMORY_MAX_NREGIONS];
> > VhostUserMsg msg;
> > + VhostUserMemory memory = {};
> > +
> > + vhost_user_prepare_msg(dev, &memory, fds);
> >
> > - vhost_user_prepare_msg(dev, &msg.payload.memory, fds);
> > + msg.payload.memory = memory;
> > }
FYI I think it's better to pass pointer to msg to
avoid alignment issues.
> Hi Michael, here should be like this:
>
>
> static void vhost_user_set_used_memslots(struct vhost_dev *dev)
> {
> int fds[VHOST_MEMORY_MAX_NREGIONS];
> - VhostUserMsg msg;
> + VhostUserMemory memory = {};
>
> - vhost_user_prepare_msg(dev, &msg.payload.memory, fds);
> + vhost_user_prepare_msg(dev, &memory, fds);
> }
>
>
> Regards,
> Jay
But what's the point of all this? The structure is discarded
after being initialized. Doesn't look right to me.
> >
> > const VhostOps user_ops = {
> > --
> > MST
