Re: [PATCH] scsi: virtio-scsi: Fix address translation failure of HighMem pages used by sg list

2012-07-25 Thread Sen Wang
2012/7/25 Paolo Bonzini pbonz...@redhat.com:
 Il 25/07/2012 10:29, Wang Sen ha scritto:
 When using the commands below to write some data to a virtio-scsi LUN of the
 QEMU guest(32-bit) with 1G physical memory(qemu -m 1024), the qemu will 
 crash.

   # sudo mkfs.ext4 /dev/sdb  (/dev/sdb is the virtio-scsi LUN.)
   # sudo mount /dev/sdb /mnt
   # dd if=/dev/zero of=/mnt/file bs=1M count=1024

 In current implementation, sg_set_buf is called to add buffers to sg list 
 which
 is put into the virtqueue eventually. But there are some HighMem pages in
 table-sgl can not get virtual address by sg_virt. So, sg_virt(sg_elem) may
 return NULL value. This will cause QEMU exit when virtqueue_map_sg is called
 in QEMU because an invalid GPA is passed by virtqueue.

 Heh, I was compiling (almost) the same patch as we speak. :)

Uh, what a coincidence! :)


 I've never seen QEMU crash; the VM would more likely just fail to boot
 with a panic.  But it's the same bug anyway.

I never met this before. How this situation happens?


 My solution is using sg_set_page instead of sg_set_buf.

 I have tested the patch on my workstation. QEMU would not crash any more.

 Signed-off-by: Wang Sen senw...@linux.vnet.ibm.com
 ---
  drivers/scsi/virtio_scsi.c |3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

 diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
 index 1b38431..fc5c88a 100644
 --- a/drivers/scsi/virtio_scsi.c
 +++ b/drivers/scsi/virtio_scsi.c
 @@ -198,7 +198,8 @@ static void virtscsi_map_sgl(struct scatterlist *sg, 
 unsigned int *p_idx,
   int i;

   for_each_sg(table-sgl, sg_elem, table-nents, i)
 - sg_set_buf(sg[idx++], sg_virt(sg_elem), sg_elem-length);
 + sg_set_page(sg[idx++], sg_page(sg_elem), sg_elem-length,
 + sg_elem-offset);

 This can simply be

sg[idx++] = *sg_elem;


Yes, I saw your another E-mail. I think you're right. Simply calling
sg_set_page can not handle
the flag bits correctly. So, I'll repost the patch soon. Thank you!

 Can you repost it with this change, and also add sta...@vger.kernel.org
 to the Cc?  Thanks very much!

 Paolo
 --
 To unsubscribe from this list: send the line unsubscribe linux-scsi in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
--
Wang Sen
Addr: XUPT,Xi'an,Shaanxi,China
Email: kelvin.x...@gmail.com
--
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] scsi: virtio-scsi: Fix address translation failure of HighMem pages used by sg list

2012-07-25 Thread Sen Wang
2012/7/25 Rolf Eike Beer eike-ker...@sf-tec.de:
 Am 25.07.2012 10:29, schrieb Wang Sen:

 When using the commands below to write some data to a virtio-scsi LUN of
 the
 QEMU guest(32-bit) with 1G physical memory(qemu -m 1024), the qemu
 will crash.

 # sudo mkfs.ext4 /dev/sdb  (/dev/sdb is the virtio-scsi LUN.)
 # sudo mount /dev/sdb /mnt
 # dd if=/dev/zero of=/mnt/file bs=1M count=1024

 In current implementation, sg_set_buf is called to add buffers to sg
 list which
 is put into the virtqueue eventually.


 The next sentence is somehow broken:


 But there are some HighMem pages in
 table-sgl can not get virtual address by sg_virt.


 Maybe something like But _if_ there are ... _you_ can not get ...?

Thanks, I'll pay attention in next post.


 Eike

 --
 To unsubscribe from this list: send the line unsubscribe linux-scsi in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
--
Wang Sen
Addr: XUPT,Xi'an,Shaanxi,China
Email: kelvin.x...@gmail.com
--
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] scsi: virtio-scsi: Fix address translation failure of HighMem pages used by sg list

2012-07-25 Thread Sen Wang
2012/7/25 Stefan Hajnoczi stefa...@linux.vnet.ibm.com:
 On Wed, Jul 25, 2012 at 10:44:14AM +0200, Paolo Bonzini wrote:
 Il 25/07/2012 10:29, Wang Sen ha scritto:
  When using the commands below to write some data to a virtio-scsi LUN of 
  the
  QEMU guest(32-bit) with 1G physical memory(qemu -m 1024), the qemu will 
  crash.
 
  # sudo mkfs.ext4 /dev/sdb  (/dev/sdb is the virtio-scsi LUN.)
  # sudo mount /dev/sdb /mnt
  # dd if=/dev/zero of=/mnt/file bs=1M count=1024
 
  In current implementation, sg_set_buf is called to add buffers to sg list 
  which
  is put into the virtqueue eventually. But there are some HighMem pages in
  table-sgl can not get virtual address by sg_virt. So, sg_virt(sg_elem) may
  return NULL value. This will cause QEMU exit when virtqueue_map_sg is 
  called
  in QEMU because an invalid GPA is passed by virtqueue.

 Heh, I was compiling (almost) the same patch as we speak. :)

 I've never seen QEMU crash; the VM would more likely just fail to boot
 with a panic.  But it's the same bug anyway.

 It's not a segfault crash, I think it hits an abort(3) in QEMU's
 virtio code when trying to map an invalid guest physical address.

How the guest boot fail? I never met this case.


 Stefan

 --
 To unsubscribe from this list: send the line unsubscribe linux-scsi in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
--
Wang Sen
Addr: XUPT,Xi'an,Shaanxi,China
Email: kelvin.x...@gmail.com
--
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html