Re: [Qemu-devel] [PATCH v9 00/12] virtio-crypto: introduce framework and device emulation

[Halil Pasic <pasic@linux.vnet.ibm.com>]

On 10/31/2016 03:52 AM, Gonglei (Arei) wrote:
>> > Unfortunately I encountered a problem already on x86_64, more precisely
>> > the guest hangs in virtio_crypto_alg_ablkcipher_init_session after the
>> > kick. But before I start explaining in detail let me ask you: what are
>> > your plans for the linux kernel driver? I used the most current version
>> > from your github, but also tried to look for patches on mailing lists
>> > and I have found none. 
> I haven't submit the virtio-crypto driver to the mailing list yet cause
> I wanted to the patches on the QEMU side can be merged firstly,
> and then focus to the frontend driver.
> 

That is fine and the QEMU stuff does look good :). I'm still curious
about your road map that is what should I expect on the kernel side
and when? You could also cc me ;) when sending to the list.

>> > IMHO the problem causing my hang is in the kernel
>> > module. But I do not know here am I supposed to comment on it -- so I'm
>> > commenting here. Furthermore your kernel module currently do not seem to
>> > care about endianness which is bad for s390x.
>> > 
> That's true.
> 

For the sake of the experiment I have reworked 
virtio_crypto_alg_ablkcipher_init_session so that id does care
about the endianness of the guest. With that the functions seems
to work on s390x (but of course the next byte order problem warrants
that encryption/decryption does not work yet). It is not beautiful
but I can give you a patch if you want.

>> > The hangs  basically boils down to the following: after the kick in
>> > virtio_crypto_alg_ablkcipher_init_session we reach the point (in QEMU)
>> > where we want to translate the address for the third descriptor, the one
>> > which designates a buffer for the virtio_crypto_session_input. My
>> > backtrace looks like this:
>> > 
>> > 
>> > #0  address_space_translate (as=<optimized out>,
>> > as@entry=0x55555607d700 <address_space_memory>,
>> > addr=addr@entry=71468256835928, xlat=xlat@entry=0x7fffef4dc1a0,
>> > plen=plen@entry=0x7fffef4dc198, is_write=is_write@entry=true)
>> >     at /home/pasic/devel/qemu/exec.c:481
>> > #1  0x000055555575ef19 in address_space_map (as=0x55555607d700
>> > <address_space_memory>, addr=71468256835928, plen=<optimized out>,
>> > is_write=<optimized out>) at /home/pasic/devel/qemu/exec.c:2927
>> > #2  0x00005555557e5d00 in virtqueue_map_desc (vdev=0x555557d80de0,
>> > p_num_sg=0x7fffef4dc2bc, addr=0x7fffef4dc2f0, iov=0x7fffef4de300,
>> > max_num_sg=1022, is_write=true, pa=71468256835928, sz=16)
>> >     at /home/pasic/devel/qemu/hw/virtio/virtio.c:558
>> > #3  0x00005555557e5f86 in virtqueue_pop (vq=0x7fffef4dc2c0,
>> > sz=93825003936576) at /home/pasic/devel/qemu/hw/virtio/virtio.c:717
>> > #4  0x00005555557ed80b in virtio_crypto_handle_ctrl (vdev=<optimized out>,
>> > vq=0x7fffeec3e090) at /home/pasic/devel/qemu/hw/virtio/virtio-crypto.c:218
>> > 
>> > Now the first suspicious thing I see is that address_space_translate
>> > returns a pointer to the memory region io_mem_unassigned. Next thing
>> > happening is that we take the !memory_access_is_direct(mr, is_write)
>> > branch in address_space_map and return NULL at line 2932 which however
>> > makes  virtqueue_map_desc report "qemu-system-x86_64: virtio: bogus
>> > descriptor or out of resources" and return with false. Then pop returns
>> > null too and spins virtio_crypto_alg_ablkcipher_init_session till the
>> > end of time waiting for the answer.
>> > 
> It seems that the frontend driver passed a invalid gpa to the QEMU side,
> The Qemu side will invokes virtio_error() to notify the guest to reset the
> device.
> 

I did not say previously but I have checked and the gpa is good, that
is it has a correct offset in respect to ctrl's gpa (the next variable
on stack, which is the previous desc.addr used read only) which is good.
Furthermore the issue seems to be platform dependent as I do not see this
on s390x.

>> > Now if I change the code so that the virtio_crypto_session_input
>> > instance's backing memory is allocated with  kzalloc(sizeof(*input),
>> > GFP_DMA|GFP_NOIO) of allocating the on the stack things start working
>> > for me. I'm not too deep into this yet, so please forgive me if
>> > I'm a bit vague.
>> > 
> Good, maybe you find a bug of the frontend driver. Using stack memory
> might not be a good idea ;)
> 

Yeah, as I said, can't say (that is my understanding too shallow).

Cheers,
Halil