qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 5/5] crypto: support multiple threads accessi


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [Qemu-devel] [PATCH v2 5/5] crypto: support multiple threads accessing one QCryptoBlock
Date: Thu, 6 Dec 2018 17:42:54 +0000

06.12.2018 13:54, Daniel P. Berrangé wrote:
> On Wed, Dec 05, 2018 at 05:47:00PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> The two thing that should be handled are cipher and ivgen. For ivgen
>> the solution is just mutex, as iv calculations should not be long in
>> comparison with encryption/decryption. And for cipher let's just keep
>> per-thread ciphers.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>> ---

[..]

>> --- a/crypto/block.c
>> +++ b/crypto/block.c

[..]

>>   static int do_qcrypto_cipher_encrypt(QCryptoCipher *cipher,
>>                                        size_t niv,
>>                                        QCryptoIVGen *ivgen,
>> +                                     QemuMutex *ivgen_mutex,
>>                                        int sectorsize,
>>                                        uint64_t offset,
>>                                        uint8_t *buf,
>> @@ -218,10 +307,15 @@ static int do_qcrypto_cipher_encrypt(QCryptoCipher 
>> *cipher,
>>       while (len > 0) {
>>           size_t nbytes;
>>           if (niv) {
>> -            if (qcrypto_ivgen_calculate(ivgen,
>> -                                        startsector,
>> -                                        iv, niv,
>> -                                        errp) < 0) {
>> +            if (ivgen_mutex) {
>> +                qemu_mutex_lock(ivgen_mutex);
>> +            }
>> +            ret = qcrypto_ivgen_calculate(ivgen, startsector, iv, niv, 
>> errp);
>> +            if (ivgen_mutex) {
>> +                qemu_mutex_unlock(ivgen_mutex);
>> +            }
> 
> I think we should just make  ivgen_mutex compulsory....
> 
>> +
>> +            if (ret < 0) {
>>                   goto cleanup;
>>               }
>>   
>> @@ -258,8 +352,9 @@ int qcrypto_cipher_decrypt_helper(QCryptoCipher *cipher,
>>                                     size_t len,
>>                                     Error **errp)
>>   {
>> -    return do_qcrypto_cipher_encrypt(cipher, niv, ivgen, sectorsize, offset,
>> -                                     buf, len, qcrypto_cipher_decrypt, 
>> errp);
>> +    return do_qcrypto_cipher_encrypt(cipher, niv, ivgen, NULL, sectorsize,
>> +                                     offset, buf, len, 
>> qcrypto_cipher_decrypt,
>> +                                     errp);
>>   }
>>   
>>   
>> @@ -272,11 +367,11 @@ int qcrypto_cipher_encrypt_helper(QCryptoCipher 
>> *cipher,
>>                                     size_t len,
>>                                     Error **errp)
>>   {
>> -    return do_qcrypto_cipher_encrypt(cipher, niv, ivgen, sectorsize, offset,
>> -                                     buf, len, qcrypto_cipher_encrypt, 
>> errp);
>> +    return do_qcrypto_cipher_encrypt(cipher, niv, ivgen, NULL, sectorsize,
>> +                                     offset, buf, len, 
>> qcrypto_cipher_encrypt,
>> +                                     errp);
>>   }
> 
> ...and get the mutex passed into these functions, as its easier to just
> know the ivgen is always protected, and not have to trace back the callpath
> to see if the usage is safe.

but there places, where these helpers called without any QCryptoBlock, when we 
just
have local cipher and ivgen, so there is no mutex and it's not needed.

-- 
Best regards,
Vladimir

reply via email to

[Prev in Thread] Current Thread [Next in Thread]