On 22 January 2018 at 16:52, Ard Biesheuvel <ard.biesheu...@linaro.org> wrote:
> On 22 January 2018 at 16:39, Peter Maydell <peter.mayd...@linaro.org> wrote:
>> On 19 January 2018 at 18:22, Ard Biesheuvel <ard.biesheu...@linaro.org>
>> wrote:
>>> This implements emulation of the new SM3 instructions that have
>>> been added as an optional extension to the ARMv8 Crypto Extensions
>>> in ARM v8.2.
>>>
>>> Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
>>
>>
>>> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
>>> index 787b94047286..1e3ff9a6152f 100644
>>> --- a/target/arm/translate-a64.c
>>> +++ b/target/arm/translate-a64.c
>>> @@ -11148,28 +11148,39 @@ static void
>>> disas_crypto_three_reg_sha512(DisasContext *s, uint32_t insn)
>>> CryptoThreeOpFn *genfn;
>>> int feature;
>>>
>>> - if (o != 0) {
>>> - unallocated_encoding(s);
>>> - return;
>>> - }
>>
>> If you wrote this code in the first patch in the
>> if (o == 0) {
>> stuff;
>> } else {
>> unallocated_encoding(s);
>> return;
>> }
>>
>> form, it would make this patch easier to read as it wouldn't
>> need to change the lines for the SHA512 cases.
>>
>
> ok
>
>>> -
>>> - switch (opcode) {
>>> - case 0: /* SHA512H */
>>> - feature = ARM_FEATURE_V8_SHA512;
>>> - genfn = gen_helper_crypto_sha512h;
>>> - break;
>>> - case 1: /* SHA512H2 */
>>> - feature = ARM_FEATURE_V8_SHA512;
>>> - genfn = gen_helper_crypto_sha512h2;
>>> - break;
>>> - case 2: /* SHA512SU1 */
>>> - feature = ARM_FEATURE_V8_SHA512;
>>> - genfn = gen_helper_crypto_sha512su1;
>>> - break;
>>> - case 3: /* RAX1 */
>>> - feature = ARM_FEATURE_V8_SHA3;
>>> - genfn = NULL;
>>> - break;
>>> + if (o == 0) {
>>> + switch (opcode) {
>>> + case 0: /* SHA512H */
>>> + feature = ARM_FEATURE_V8_SHA512;
>>> + genfn = gen_helper_crypto_sha512h;
>>> + break;
>>> + case 1: /* SHA512H2 */
>>> + feature = ARM_FEATURE_V8_SHA512;
>>> + genfn = gen_helper_crypto_sha512h2;
>>> + break;
>>> + case 2: /* SHA512SU1 */
>>> + feature = ARM_FEATURE_V8_SHA512;
>>> + genfn = gen_helper_crypto_sha512su1;
>>> + break;
>>> + case 3: /* RAX1 */
>>> + feature = ARM_FEATURE_V8_SHA3;
>>> + genfn = NULL;
>>> + break;
>>> + }
>>> + } else {
>>> + switch (opcode) {
>>> + case 0: /* SM3PARTW1 */
>>> + feature = ARM_FEATURE_V8_SM3;
>>> + genfn = gen_helper_crypto_sm3partw1;
>>> + break;
>>> + case 1: /* SM3PARTW2 */
>>> + feature = ARM_FEATURE_V8_SM3;
>>> + genfn = gen_helper_crypto_sm3partw2;
>>> + break;
>>> + default:
>>> + unallocated_encoding(s);
>>> + return;
>>> + }
>>
>> This seems to be missing support for SM4EKEY (which is O==1
>> opcode == 0b10 and also part of the v8.2 SM feature) ?
>>
>
> It is part of the v8.2 SM extension, which consists of SM3 secure hash
> and SM4 encryption, which are two different things (and AA64ISAR0 has
> separate feature bits for each). The ARM ARM does stipulate that both
> should be set if either one is set, but still provides two separate
> bits, and so one can be enabled without the other.
>
Same for ELF_HWCAPs btw
