1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH] aspeed_scu: Implement RNG register

Joel Stanley posted 1 patch 7 years, 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180528124621.22977-1-joel@jms.id.au
Test checkpatch passed
Test docker-mingw@fedora passed
Test docker-quick@centos7 passed
Test s390x passed
There is a newer version of this series
hw/misc/aspeed_scu.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
[Qemu-devel] [PATCH] aspeed_scu: Implement RNG register
Posted by Joel Stanley 7 years, 5 months ago 
The ASPEED SoCs contain a single register that returns random data when
read. This models that register so that guests can use it.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 hw/misc/aspeed_scu.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
index 5e6d5744eeca..8fa0cecf0fa1 100644
--- a/hw/misc/aspeed_scu.c
+++ b/hw/misc/aspeed_scu.c
@@ -16,6 +16,7 @@
 #include "qapi/visitor.h"
 #include "qemu/bitops.h"
 #include "qemu/log.h"
+#include "crypto/random.h"
 #include "trace.h"
 
 #define TO_REG(offset) ((offset) >> 2)
@@ -154,6 +155,18 @@ static const uint32_t ast2500_a1_resets[ASPEED_SCU_NR_REGS] = {
      [BMC_DEV_ID]      = 0x00002402U
 };
 
+static uint32_t aspeed_scu_get_random(void)
+{
+    Error *err = NULL;
+    uint32_t num;
+
+    if (qcrypto_random_bytes((uint8_t *)&num, sizeof(num), &err)) {
+        error_report_err(err);
+    }
+
+    return num;
+}
+
 static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size)
 {
     AspeedSCUState *s = ASPEED_SCU(opaque);
@@ -167,6 +180,9 @@ static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size)
     }
 
     switch (reg) {
+    case RNG_DATA:
+        return aspeed_scu_get_random();
+        break;
     case WAKEUP_EN:
         qemu_log_mask(LOG_GUEST_ERROR,
                       "%s: Read of write-only offset 0x%" HWADDR_PRIx "\n",
@@ -287,6 +303,9 @@ static void aspeed_scu_realize(DeviceState *dev, Error **errp)
                           TYPE_ASPEED_SCU, SCU_IO_REGION_SIZE);
 
     sysbus_init_mmio(sbd, &s->iomem);
+
+    if (qcrypto_random_init(errp))
+        return;
 }
 
 static const VMStateDescription vmstate_aspeed_scu = {
-- 
2.17.0
Re: [Qemu-devel] [PATCH] aspeed_scu: Implement RNG register
Posted by Cédric Le Goater 7 years, 5 months ago 
Hello Joel, 

On 05/28/2018 02:46 PM, Joel Stanley wrote:
> The ASPEED SoCs contain a single register that returns random data when
> read. This models that register so that guests can use it.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  hw/misc/aspeed_scu.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
> index 5e6d5744eeca..8fa0cecf0fa1 100644
> --- a/hw/misc/aspeed_scu.c
> +++ b/hw/misc/aspeed_scu.c
> @@ -16,6 +16,7 @@
>  #include "qapi/visitor.h"
>  #include "qemu/bitops.h"
>  #include "qemu/log.h"
> +#include "crypto/random.h"
>  #include "trace.h"
>  
>  #define TO_REG(offset) ((offset) >> 2)
> @@ -154,6 +155,18 @@ static const uint32_t ast2500_a1_resets[ASPEED_SCU_NR_REGS] = {
>       [BMC_DEV_ID]      = 0x00002402U
>  };
>  
> +static uint32_t aspeed_scu_get_random(void)
> +{
> +    Error *err = NULL;
> +    uint32_t num;
> +
> +    if (qcrypto_random_bytes((uint8_t *)&num, sizeof(num), &err)) {
> +        error_report_err(err);
> +    }
> +
> +    return num;
> +}
> +
>  static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size)
>  {
>      AspeedSCUState *s = ASPEED_SCU(opaque);
> @@ -167,6 +180,9 @@ static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size)
>      }
>  
>      switch (reg) {
> +    case RNG_DATA:
> +        return aspeed_scu_get_random();

may be we could test bit 1 of RNG_CTRL to check if it is enabled or not.

> +        break;
>      case WAKEUP_EN:
>          qemu_log_mask(LOG_GUEST_ERROR,
>                        "%s: Read of write-only offset 0x%" HWADDR_PRIx "\n",
> @@ -287,6 +303,9 @@ static void aspeed_scu_realize(DeviceState *dev, Error **errp)
>                            TYPE_ASPEED_SCU, SCU_IO_REGION_SIZE);
>  
>      sysbus_init_mmio(sbd, &s->iomem);
> +
> +    if (qcrypto_random_init(errp))
> +        return;
>  }

Isn't this routine called from main() already ? 

C. 

>  
>  static const VMStateDescription vmstate_aspeed_scu = {
>
Re: [Qemu-devel] [PATCH] aspeed_scu: Implement RNG register
Posted by Joel Stanley 7 years, 5 months ago 
On 28 May 2018 at 23:17, Cédric Le Goater <clg@kaod.org> wrote:
> Hello Joel,
>
> On 05/28/2018 02:46 PM, Joel Stanley wrote:
>> The ASPEED SoCs contain a single register that returns random data when
>> read. This models that register so that guests can use it.
>>
>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>> ---
>>  hw/misc/aspeed_scu.c | 19 +++++++++++++++++++
>>  1 file changed, 19 insertions(+)
>>
>> diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
>> index 5e6d5744eeca..8fa0cecf0fa1 100644
>> --- a/hw/misc/aspeed_scu.c
>> +++ b/hw/misc/aspeed_scu.c
>> @@ -16,6 +16,7 @@
>>  #include "qapi/visitor.h"
>>  #include "qemu/bitops.h"
>>  #include "qemu/log.h"
>> +#include "crypto/random.h"
>>  #include "trace.h"
>>
>>  #define TO_REG(offset) ((offset) >> 2)
>> @@ -154,6 +155,18 @@ static const uint32_t ast2500_a1_resets[ASPEED_SCU_NR_REGS] = {
>>       [BMC_DEV_ID]      = 0x00002402U
>>  };
>>
>> +static uint32_t aspeed_scu_get_random(void)
>> +{
>> +    Error *err = NULL;
>> +    uint32_t num;
>> +
>> +    if (qcrypto_random_bytes((uint8_t *)&num, sizeof(num), &err)) {
>> +        error_report_err(err);
>> +    }
>> +
>> +    return num;
>> +}
>> +
>>  static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size)
>>  {
>>      AspeedSCUState *s = ASPEED_SCU(opaque);
>> @@ -167,6 +180,9 @@ static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size)
>>      }
>>
>>      switch (reg) {
>> +    case RNG_DATA:
>> +        return aspeed_scu_get_random();
>
> may be we could test bit 1 of RNG_CTRL to check if it is enabled or not.

The RNG is enabled by default, and I didn't find any software that
disables it, but it can't hurt to have that check.

I'll send a v2 with that check.

>
>> +        break;
>>      case WAKEUP_EN:
>>          qemu_log_mask(LOG_GUEST_ERROR,
>>                        "%s: Read of write-only offset 0x%" HWADDR_PRIx "\n",
>> @@ -287,6 +303,9 @@ static void aspeed_scu_realize(DeviceState *dev, Error **errp)
>>                            TYPE_ASPEED_SCU, SCU_IO_REGION_SIZE);
>>
>>      sysbus_init_mmio(sbd, &s->iomem);
>> +
>> +    if (qcrypto_random_init(errp))
>> +        return;
>>  }
>
> Isn't this routine called from main() already ?

It is indirectly called, yes. I'll remove this.

Cheers,

Joel
Re: [Qemu-devel] [PATCH] aspeed_scu: Implement RNG register
Posted by Joel Stanley 7 years, 5 months ago 
On 28 May 2018 at 23:33, Joel Stanley <joel@jms.id.au> wrote:
> On 28 May 2018 at 23:17, Cédric Le Goater <clg@kaod.org> wrote:
>> Hello Joel,
>>
>> On 05/28/2018 02:46 PM, Joel Stanley wrote:
>>> The ASPEED SoCs contain a single register that returns random data when
>>> read. This models that register so that guests can use it.
>>>
>>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>>> ---
>>>  hw/misc/aspeed_scu.c | 19 +++++++++++++++++++
>>>  1 file changed, 19 insertions(+)
>>>
>>> diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
>>> index 5e6d5744eeca..8fa0cecf0fa1 100644
>>> --- a/hw/misc/aspeed_scu.c
>>> +++ b/hw/misc/aspeed_scu.c
>>> @@ -16,6 +16,7 @@
>>>  #include "qapi/visitor.h"
>>>  #include "qemu/bitops.h"
>>>  #include "qemu/log.h"
>>> +#include "crypto/random.h"
>>>  #include "trace.h"
>>>
>>>  #define TO_REG(offset) ((offset) >> 2)
>>> @@ -154,6 +155,18 @@ static const uint32_t ast2500_a1_resets[ASPEED_SCU_NR_REGS] = {
>>>       [BMC_DEV_ID]      = 0x00002402U
>>>  };
>>>
>>> +static uint32_t aspeed_scu_get_random(void)
>>> +{
>>> +    Error *err = NULL;
>>> +    uint32_t num;
>>> +
>>> +    if (qcrypto_random_bytes((uint8_t *)&num, sizeof(num), &err)) {
>>> +        error_report_err(err);
>>> +    }
>>> +
>>> +    return num;
>>> +}
>>> +
>>>  static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size)
>>>  {
>>>      AspeedSCUState *s = ASPEED_SCU(opaque);
>>> @@ -167,6 +180,9 @@ static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size)
>>>      }
>>>
>>>      switch (reg) {
>>> +    case RNG_DATA:
>>> +        return aspeed_scu_get_random();
>>
>> may be we could test bit 1 of RNG_CTRL to check if it is enabled or not.
>
> The RNG is enabled by default, and I didn't find any software that
> disables it, but it can't hurt to have that check.

I did some testing on hardware, and the rng still outputs a different
number each time I ask for one regardless of the state of the enabled
bit.

How should we model that?
Re: [Qemu-devel] [PATCH] aspeed_scu: Implement RNG register
Posted by Cédric Le Goater 7 years, 5 months ago 
On 05/28/2018 04:29 PM, Joel Stanley wrote:
> On 28 May 2018 at 23:33, Joel Stanley <joel@jms.id.au> wrote:
>> On 28 May 2018 at 23:17, Cédric Le Goater <clg@kaod.org> wrote:
>>> Hello Joel,
>>>
>>> On 05/28/2018 02:46 PM, Joel Stanley wrote:
>>>> The ASPEED SoCs contain a single register that returns random data when
>>>> read. This models that register so that guests can use it.
>>>>
>>>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>>>> ---
>>>>  hw/misc/aspeed_scu.c | 19 +++++++++++++++++++
>>>>  1 file changed, 19 insertions(+)
>>>>
>>>> diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
>>>> index 5e6d5744eeca..8fa0cecf0fa1 100644
>>>> --- a/hw/misc/aspeed_scu.c
>>>> +++ b/hw/misc/aspeed_scu.c
>>>> @@ -16,6 +16,7 @@
>>>>  #include "qapi/visitor.h"
>>>>  #include "qemu/bitops.h"
>>>>  #include "qemu/log.h"
>>>> +#include "crypto/random.h"
>>>>  #include "trace.h"
>>>>
>>>>  #define TO_REG(offset) ((offset) >> 2)
>>>> @@ -154,6 +155,18 @@ static const uint32_t ast2500_a1_resets[ASPEED_SCU_NR_REGS] = {
>>>>       [BMC_DEV_ID]      = 0x00002402U
>>>>  };
>>>>
>>>> +static uint32_t aspeed_scu_get_random(void)
>>>> +{
>>>> +    Error *err = NULL;
>>>> +    uint32_t num;
>>>> +
>>>> +    if (qcrypto_random_bytes((uint8_t *)&num, sizeof(num), &err)) {
>>>> +        error_report_err(err);
>>>> +    }
>>>> +
>>>> +    return num;
>>>> +}
>>>> +
>>>>  static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size)
>>>>  {
>>>>      AspeedSCUState *s = ASPEED_SCU(opaque);
>>>> @@ -167,6 +180,9 @@ static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size)
>>>>      }
>>>>
>>>>      switch (reg) {
>>>> +    case RNG_DATA:
>>>> +        return aspeed_scu_get_random();
>>>
>>> may be we could test bit 1 of RNG_CTRL to check if it is enabled or not.
>>
>> The RNG is enabled by default, and I didn't find any software that
>> disables it, but it can't hurt to have that check.
> 
> I did some testing on hardware, and the rng still outputs a different
> number each time I ask for one regardless of the state of the enabled
> bit.
> 
> How should we model that?
> 

I confirm that the HW doesn't really care about the enabled bit.
Let's ignore it then ?

C.

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.