diff mbox series

[v5,3/3] hwrng: add mtk-sec-rng driver

Message ID 1574864578-467-4-git-send-email-neal.liu@mediatek.com (mailing list archive)
State Changes Requested
Delegated to: Herbert Xu
Headers show
Series MediaTek Security random number generator support | expand

Commit Message

Neal Liu Nov. 27, 2019, 2:22 p.m. UTC
For MediaTek SoCs on ARMv8 with TrustZone enabled, peripherals like
entropy sources is not accessible from normal world (linux) and
rather accessible from secure world (ATF/TEE) only. This driver aims
to provide a generic interface to ATF rng service.

Signed-off-by: Neal Liu <neal.liu@mediatek.com>
---
 drivers/char/hw_random/Kconfig       |   16 ++++++
 drivers/char/hw_random/Makefile      |    1 +
 drivers/char/hw_random/mtk-sec-rng.c |  103 ++++++++++++++++++++++++++++++++++
 3 files changed, 120 insertions(+)
 create mode 100644 drivers/char/hw_random/mtk-sec-rng.c

Comments

Ard Biesheuvel Nov. 27, 2019, 3:03 p.m. UTC | #1
On Wed, 27 Nov 2019 at 15:23, Neal Liu <neal.liu@mediatek.com> wrote:
>
> For MediaTek SoCs on ARMv8 with TrustZone enabled, peripherals like
> entropy sources is not accessible from normal world (linux) and
> rather accessible from secure world (ATF/TEE) only. This driver aims
> to provide a generic interface to ATF rng service.
>
> Signed-off-by: Neal Liu <neal.liu@mediatek.com>
> ---
>  drivers/char/hw_random/Kconfig       |   16 ++++++
>  drivers/char/hw_random/Makefile      |    1 +
>  drivers/char/hw_random/mtk-sec-rng.c |  103 ++++++++++++++++++++++++++++++++++
>  3 files changed, 120 insertions(+)
>  create mode 100644 drivers/char/hw_random/mtk-sec-rng.c
>
> diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
> index 25a7d8f..f08c852 100644
> --- a/drivers/char/hw_random/Kconfig
> +++ b/drivers/char/hw_random/Kconfig
> @@ -398,6 +398,22 @@ config HW_RANDOM_MTK
>
>           If unsure, say Y.
>
> +config HW_RANDOM_MTK_SEC
> +       tristate "MediaTek Security Random Number Generator support"
> +       depends on HW_RANDOM
> +       depends on ARCH_MEDIATEK || COMPILE_TEST
> +       default HW_RANDOM
> +         help
> +         This driver provides kernel-side support for the Random Number
> +         Generator hardware found on MediaTek SoCs. The difference with
> +         mtk-rng is the Random Number Generator hardware is secure
> +         access only.
> +
> +         To compile this driver as a module, choose M here. the
> +         module will be called mtk-sec-rng.
> +
> +         If unsure, say Y.
> +
>  config HW_RANDOM_S390
>         tristate "S390 True Random Number Generator support"
>         depends on S390
> diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
> index 7c9ef4a..bee5412 100644
> --- a/drivers/char/hw_random/Makefile
> +++ b/drivers/char/hw_random/Makefile
> @@ -36,6 +36,7 @@ obj-$(CONFIG_HW_RANDOM_PIC32) += pic32-rng.o
>  obj-$(CONFIG_HW_RANDOM_MESON) += meson-rng.o
>  obj-$(CONFIG_HW_RANDOM_CAVIUM) += cavium-rng.o cavium-rng-vf.o
>  obj-$(CONFIG_HW_RANDOM_MTK)    += mtk-rng.o
> +obj-$(CONFIG_HW_RANDOM_MTK_SEC)        += mtk-sec-rng.o
>  obj-$(CONFIG_HW_RANDOM_S390) += s390-trng.o
>  obj-$(CONFIG_HW_RANDOM_KEYSTONE) += ks-sa-rng.o
>  obj-$(CONFIG_HW_RANDOM_OPTEE) += optee-rng.o
> diff --git a/drivers/char/hw_random/mtk-sec-rng.c b/drivers/char/hw_random/mtk-sec-rng.c
> new file mode 100644
> index 0000000..69ddeca
> --- /dev/null
> +++ b/drivers/char/hw_random/mtk-sec-rng.c
> @@ -0,0 +1,103 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 MediaTek Inc.
> + */
> +
> +#include <linux/arm-smccc.h>
> +#include <linux/hw_random.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/soc/mediatek/mtk_sip_svc.h>
> +
> +#define MTK_SEC_RNG_MAGIC      0x74726e67
> +#define SMC_RET_NUM            4
> +#define MTK_SEC_RND_SIZE       (sizeof(u32) * SMC_RET_NUM)
> +
> +static void mtk_sec_get_rnd(uint32_t *val)
> +{
> +       struct arm_smccc_res res;
> +
> +       arm_smccc_smc(MTK_SIP_KERNEL_GET_RND,
> +                     MTK_SEC_RNG_MAGIC, 0, 0, 0, 0, 0, 0, &res);
> +

Can this call never fail? How does the firmware signal that something
is wrong with the underlying hardware?

> +       val[0] = res.a0;
> +       val[1] = res.a1;
> +       val[2] = res.a2;
> +       val[3] = res.a3;
> +}
> +
> +static int mtk_sec_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
> +{
> +       u32 val[4] = {0};
> +       int retval = 0;
> +       int i;
> +
> +       while (max >= MTK_SEC_RND_SIZE) {
> +               mtk_sec_get_rnd(val);
> +
> +               for (i = 0; i < SMC_RET_NUM; i++) {
> +                       *(u32 *)buf = val[i];
> +                       buf += sizeof(u32);
> +               }
> +
> +               retval += MTK_SEC_RND_SIZE;
> +               max -= MTK_SEC_RND_SIZE;
> +       }
> +
> +       return retval;
> +}
> +
> +static struct hwrng mtk_sec_rng = {
> +       .name = "mtk_sec_rng",
> +       .read = mtk_sec_rng_read,
> +       .quality = 900,
> +};
> +
> +static int mtk_sec_rng_probe(void)
> +{
> +       int ret;
> +
> +       ret = hwrng_register(&mtk_sec_rng);
> +       if (ret) {
> +               pr_err("Failed to register rng device: %d\n", ret);
> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +
> +static int __init mtk_sec_rng_driver_init(void)
> +{
> +       struct device_node *fw_np;
> +       struct device_node *np;
> +       const char *method;
> +
> +       fw_np = of_find_node_by_name(NULL, "firmware");
> +       if (!fw_np)
> +               return -ENODEV;
> +
> +       np = of_find_compatible_node(fw_np, NULL, "mediatek,mtk-sec-rng");
> +       if (!np)
> +               return -ENODEV;
> +
> +       if (of_property_read_string(np, "method", &method))
> +               return -ENXIO;
> +
> +       if (strncmp("smc", method, strlen("smc")))
> +               return -EINVAL;
> +
> +       return mtk_sec_rng_probe();
> +}
> +
> +static void __exit mtk_sec_rng_driver_exit(void)
> +{
> +       hwrng_unregister(&mtk_sec_rng);
> +}
> +
> +module_init(mtk_sec_rng_driver_init);
> +module_exit(mtk_sec_rng_driver_exit);
> +
> +MODULE_DESCRIPTION("MediaTek Security Random Number Generator Driver");
> +MODULE_AUTHOR("Neal Liu <neal.liu@mediatek.com>");
> +MODULE_LICENSE("GPL");
> --
> 1.7.9.5
Neal Liu Nov. 28, 2019, 3:02 p.m. UTC | #2
On Wed, 2019-11-27 at 23:03 +0800, Ard Biesheuvel wrote:
> On Wed, 27 Nov 2019 at 15:23, Neal Liu <neal.liu@mediatek.com> wrote:
> >
> > For MediaTek SoCs on ARMv8 with TrustZone enabled, peripherals like
> > entropy sources is not accessible from normal world (linux) and
> > rather accessible from secure world (ATF/TEE) only. This driver aims
> > to provide a generic interface to ATF rng service.
> >
> > Signed-off-by: Neal Liu <neal.liu@mediatek.com>
> > ---
> >  drivers/char/hw_random/Kconfig       |   16 ++++++
> >  drivers/char/hw_random/Makefile      |    1 +
> >  drivers/char/hw_random/mtk-sec-rng.c |  103 ++++++++++++++++++++++++++++++++++
> >  3 files changed, 120 insertions(+)
> >  create mode 100644 drivers/char/hw_random/mtk-sec-rng.c
> >
> > diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
> > index 25a7d8f..f08c852 100644
> > --- a/drivers/char/hw_random/Kconfig
> > +++ b/drivers/char/hw_random/Kconfig
> > @@ -398,6 +398,22 @@ config HW_RANDOM_MTK
> >
> >           If unsure, say Y.
> >
> > +config HW_RANDOM_MTK_SEC
> > +       tristate "MediaTek Security Random Number Generator support"
> > +       depends on HW_RANDOM
> > +       depends on ARCH_MEDIATEK || COMPILE_TEST
> > +       default HW_RANDOM
> > +         help
> > +         This driver provides kernel-side support for the Random Number
> > +         Generator hardware found on MediaTek SoCs. The difference with
> > +         mtk-rng is the Random Number Generator hardware is secure
> > +         access only.
> > +
> > +         To compile this driver as a module, choose M here. the
> > +         module will be called mtk-sec-rng.
> > +
> > +         If unsure, say Y.
> > +
> >  config HW_RANDOM_S390
> >         tristate "S390 True Random Number Generator support"
> >         depends on S390
> > diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
> > index 7c9ef4a..bee5412 100644
> > --- a/drivers/char/hw_random/Makefile
> > +++ b/drivers/char/hw_random/Makefile
> > @@ -36,6 +36,7 @@ obj-$(CONFIG_HW_RANDOM_PIC32) += pic32-rng.o
> >  obj-$(CONFIG_HW_RANDOM_MESON) += meson-rng.o
> >  obj-$(CONFIG_HW_RANDOM_CAVIUM) += cavium-rng.o cavium-rng-vf.o
> >  obj-$(CONFIG_HW_RANDOM_MTK)    += mtk-rng.o
> > +obj-$(CONFIG_HW_RANDOM_MTK_SEC)        += mtk-sec-rng.o
> >  obj-$(CONFIG_HW_RANDOM_S390) += s390-trng.o
> >  obj-$(CONFIG_HW_RANDOM_KEYSTONE) += ks-sa-rng.o
> >  obj-$(CONFIG_HW_RANDOM_OPTEE) += optee-rng.o
> > diff --git a/drivers/char/hw_random/mtk-sec-rng.c b/drivers/char/hw_random/mtk-sec-rng.c
> > new file mode 100644
> > index 0000000..69ddeca
> > --- /dev/null
> > +++ b/drivers/char/hw_random/mtk-sec-rng.c
> > @@ -0,0 +1,103 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2019 MediaTek Inc.
> > + */
> > +
> > +#include <linux/arm-smccc.h>
> > +#include <linux/hw_random.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/soc/mediatek/mtk_sip_svc.h>
> > +
> > +#define MTK_SEC_RNG_MAGIC      0x74726e67
> > +#define SMC_RET_NUM            4
> > +#define MTK_SEC_RND_SIZE       (sizeof(u32) * SMC_RET_NUM)
> > +
> > +static void mtk_sec_get_rnd(uint32_t *val)
> > +{
> > +       struct arm_smccc_res res;
> > +
> > +       arm_smccc_smc(MTK_SIP_KERNEL_GET_RND,
> > +                     MTK_SEC_RNG_MAGIC, 0, 0, 0, 0, 0, 0, &res);
> > +
> 
> Can this call never fail? How does the firmware signal that something
> is wrong with the underlying hardware?
> 

The smc call is supported in both ARMv7 & ARMv8 architectures.But yes,
it should check hardware status before assigning it.

We would like to check that if hardware is something wrong, all return
value will be zero. ex:

	if (!res.a0 && !res.a1 && !res.a2 && !res.a3)
		return false;

> > +       val[0] = res.a0;
> > +       val[1] = res.a1;
> > +       val[2] = res.a2;
> > +       val[3] = res.a3;

	return true;
> > +}
> > +
> > +static int mtk_sec_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
> > +{
> > +       u32 val[4] = {0};
> > +       int retval = 0;
> > +       int i;
> > +
> > +       while (max >= MTK_SEC_RND_SIZE) {
> > +               mtk_sec_get_rnd(val);
> > +
> > +               for (i = 0; i < SMC_RET_NUM; i++) {
> > +                       *(u32 *)buf = val[i];
> > +                       buf += sizeof(u32);
> > +               }
> > +
> > +               retval += MTK_SEC_RND_SIZE;
> > +               max -= MTK_SEC_RND_SIZE;
> > +       }
> > +
> > +       return retval;
> > +}
> > +
> > +static struct hwrng mtk_sec_rng = {
> > +       .name = "mtk_sec_rng",
> > +       .read = mtk_sec_rng_read,
> > +       .quality = 900,
> > +};
> > +
> > +static int mtk_sec_rng_probe(void)
> > +{
> > +       int ret;
> > +
> > +       ret = hwrng_register(&mtk_sec_rng);
> > +       if (ret) {
> > +               pr_err("Failed to register rng device: %d\n", ret);
> > +               return ret;
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +static int __init mtk_sec_rng_driver_init(void)
> > +{
> > +       struct device_node *fw_np;
> > +       struct device_node *np;
> > +       const char *method;
> > +
> > +       fw_np = of_find_node_by_name(NULL, "firmware");
> > +       if (!fw_np)
> > +               return -ENODEV;
> > +
> > +       np = of_find_compatible_node(fw_np, NULL, "mediatek,mtk-sec-rng");
> > +       if (!np)
> > +               return -ENODEV;
> > +
> > +       if (of_property_read_string(np, "method", &method))
> > +               return -ENXIO;
> > +
> > +       if (strncmp("smc", method, strlen("smc")))
> > +               return -EINVAL;
> > +
> > +       return mtk_sec_rng_probe();
> > +}
> > +
> > +static void __exit mtk_sec_rng_driver_exit(void)
> > +{
> > +       hwrng_unregister(&mtk_sec_rng);
> > +}
> > +
> > +module_init(mtk_sec_rng_driver_init);
> > +module_exit(mtk_sec_rng_driver_exit);
> > +
> > +MODULE_DESCRIPTION("MediaTek Security Random Number Generator Driver");
> > +MODULE_AUTHOR("Neal Liu <neal.liu@mediatek.com>");
> > +MODULE_LICENSE("GPL");
> > --
> > 1.7.9.5
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
Lars Persson Nov. 29, 2019, 10:02 a.m. UTC | #3
Hi Neal,

On Wed, Nov 27, 2019 at 3:23 PM Neal Liu <neal.liu@mediatek.com> wrote:
>
> For MediaTek SoCs on ARMv8 with TrustZone enabled, peripherals like
> entropy sources is not accessible from normal world (linux) and
> rather accessible from secure world (ATF/TEE) only. This driver aims
> to provide a generic interface to ATF rng service.
>

I am working on several SoCs that also will need this kind of driver
to get entropy from Arm trusted firmware.
If you intend to make this a generic interface, please clean up the
references to MediaTek and give it a more generic name. For example
"Arm Trusted Firmware random number driver".

It will also be helpful if the SMC call number is configurable.

- Lars
Neal Liu Nov. 29, 2019, 11:30 a.m. UTC | #4
On Fri, 2019-11-29 at 18:02 +0800, Lars Persson wrote:
> Hi Neal,
> 
> On Wed, Nov 27, 2019 at 3:23 PM Neal Liu <neal.liu@mediatek.com> wrote:
> >
> > For MediaTek SoCs on ARMv8 with TrustZone enabled, peripherals like
> > entropy sources is not accessible from normal world (linux) and
> > rather accessible from secure world (ATF/TEE) only. This driver aims
> > to provide a generic interface to ATF rng service.
> >
> 
> I am working on several SoCs that also will need this kind of driver
> to get entropy from Arm trusted firmware.
> If you intend to make this a generic interface, please clean up the
> references to MediaTek and give it a more generic name. For example
> "Arm Trusted Firmware random number driver".
> 
> It will also be helpful if the SMC call number is configurable.
> 
> - Lars

Yes, I'm trying to make this to a generic interface. I'll try to make
HW/platform related dependency to be configurable and let it more
generic.
Thanks for your suggestion.

> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
Ard Biesheuvel Dec. 2, 2019, 4:12 p.m. UTC | #5
(adding some more arm64 folks)

On Fri, 29 Nov 2019 at 11:30, Neal Liu <neal.liu@mediatek.com> wrote:
>
> On Fri, 2019-11-29 at 18:02 +0800, Lars Persson wrote:
> > Hi Neal,
> >
> > On Wed, Nov 27, 2019 at 3:23 PM Neal Liu <neal.liu@mediatek.com> wrote:
> > >
> > > For MediaTek SoCs on ARMv8 with TrustZone enabled, peripherals like
> > > entropy sources is not accessible from normal world (linux) and
> > > rather accessible from secure world (ATF/TEE) only. This driver aims
> > > to provide a generic interface to ATF rng service.
> > >
> >
> > I am working on several SoCs that also will need this kind of driver
> > to get entropy from Arm trusted firmware.
> > If you intend to make this a generic interface, please clean up the
> > references to MediaTek and give it a more generic name. For example
> > "Arm Trusted Firmware random number driver".
> >
> > It will also be helpful if the SMC call number is configurable.
> >
> > - Lars
>
> Yes, I'm trying to make this to a generic interface. I'll try to make
> HW/platform related dependency to be configurable and let it more
> generic.
> Thanks for your suggestion.
>

I don't think it makes sense for each arm64 platform to expose an
entropy source via SMC calls in a slightly different way, and model it
as a h/w driver. Instead, we should try to standardize this, and
perhaps expose it via the architectural helpers that already exist
(get_random_seed_long() and friends), so they get plugged into the
kernel random pool driver directly.

Note that in addition to drivers based on vendor SMC calls, we already
have a RNG h/w driver based on OP-TEE as well, where the driver
attaches to a standardized trusted OS interface identified by a UUID,
and which also gets invoked via SMC calls into secure firmware.
Marc Zyngier Dec. 2, 2019, 7:11 p.m. UTC | #6
On Mon, 2 Dec 2019 16:12:09 +0000
Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> (adding some more arm64 folks)
> 
> On Fri, 29 Nov 2019 at 11:30, Neal Liu <neal.liu@mediatek.com> wrote:
> >
> > On Fri, 2019-11-29 at 18:02 +0800, Lars Persson wrote:  
> > > Hi Neal,
> > >
> > > On Wed, Nov 27, 2019 at 3:23 PM Neal Liu <neal.liu@mediatek.com> wrote:  
> > > >
> > > > For MediaTek SoCs on ARMv8 with TrustZone enabled, peripherals like
> > > > entropy sources is not accessible from normal world (linux) and
> > > > rather accessible from secure world (ATF/TEE) only. This driver aims
> > > > to provide a generic interface to ATF rng service.
> > > >  
> > >
> > > I am working on several SoCs that also will need this kind of driver
> > > to get entropy from Arm trusted firmware.
> > > If you intend to make this a generic interface, please clean up the
> > > references to MediaTek and give it a more generic name. For example
> > > "Arm Trusted Firmware random number driver".
> > >
> > > It will also be helpful if the SMC call number is configurable.
> > >
> > > - Lars  
> >
> > Yes, I'm trying to make this to a generic interface. I'll try to make
> > HW/platform related dependency to be configurable and let it more
> > generic.
> > Thanks for your suggestion.
> >  
> 
> I don't think it makes sense for each arm64 platform to expose an
> entropy source via SMC calls in a slightly different way, and model it
> as a h/w driver. Instead, we should try to standardize this, and
> perhaps expose it via the architectural helpers that already exist
> (get_random_seed_long() and friends), so they get plugged into the
> kernel random pool driver directly.

Absolutely. I'd love to see a standard, ARM-specified, virtualizable
RNG that is abstracted from the HW.

> Note that in addition to drivers based on vendor SMC calls, we already
> have a RNG h/w driver based on OP-TEE as well, where the driver
> attaches to a standardized trusted OS interface identified by a UUID,
> and which also gets invoked via SMC calls into secure firmware.

... and probably an unhealthy number of hypervisor-specific hacks that
do the same thing. The sooner we plug this, the better.

Thanks,

	M.
Florian Fainelli Dec. 3, 2019, 4:16 a.m. UTC | #7
On 12/2/2019 11:11 AM, Marc Zyngier wrote:
> On Mon, 2 Dec 2019 16:12:09 +0000
> Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> 
>> (adding some more arm64 folks)
>>
>> On Fri, 29 Nov 2019 at 11:30, Neal Liu <neal.liu@mediatek.com> wrote:
>>>
>>> On Fri, 2019-11-29 at 18:02 +0800, Lars Persson wrote:  
>>>> Hi Neal,
>>>>
>>>> On Wed, Nov 27, 2019 at 3:23 PM Neal Liu <neal.liu@mediatek.com> wrote:  
>>>>>
>>>>> For MediaTek SoCs on ARMv8 with TrustZone enabled, peripherals like
>>>>> entropy sources is not accessible from normal world (linux) and
>>>>> rather accessible from secure world (ATF/TEE) only. This driver aims
>>>>> to provide a generic interface to ATF rng service.
>>>>>  
>>>>
>>>> I am working on several SoCs that also will need this kind of driver
>>>> to get entropy from Arm trusted firmware.
>>>> If you intend to make this a generic interface, please clean up the
>>>> references to MediaTek and give it a more generic name. For example
>>>> "Arm Trusted Firmware random number driver".
>>>>
>>>> It will also be helpful if the SMC call number is configurable.
>>>>
>>>> - Lars  
>>>
>>> Yes, I'm trying to make this to a generic interface. I'll try to make
>>> HW/platform related dependency to be configurable and let it more
>>> generic.
>>> Thanks for your suggestion.
>>>  
>>
>> I don't think it makes sense for each arm64 platform to expose an
>> entropy source via SMC calls in a slightly different way, and model it
>> as a h/w driver. Instead, we should try to standardize this, and
>> perhaps expose it via the architectural helpers that already exist
>> (get_random_seed_long() and friends), so they get plugged into the
>> kernel random pool driver directly.
> 
> Absolutely. I'd love to see a standard, ARM-specified, virtualizable
> RNG that is abstracted from the HW.

Do you think we could use virtio-rng on top of a modified virtio-mmio
which instead of being backed by a hardware mailbox, could use hvc/smc
calls to signal writes to shared memory and get notifications via an
interrupt? This would also open up the doors to other virtio uses cases
beyond just RNG (e.g.: console, block devices?). If this is completely
stupid, then please disregard this comment.
Will Deacon Dec. 3, 2019, 10:17 a.m. UTC | #8
On Mon, Dec 02, 2019 at 07:11:46PM +0000, Marc Zyngier wrote:
> On Mon, 2 Dec 2019 16:12:09 +0000
> Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> 
> > (adding some more arm64 folks)
> > 
> > On Fri, 29 Nov 2019 at 11:30, Neal Liu <neal.liu@mediatek.com> wrote:
> > >
> > > On Fri, 2019-11-29 at 18:02 +0800, Lars Persson wrote:  
> > > > Hi Neal,
> > > >
> > > > On Wed, Nov 27, 2019 at 3:23 PM Neal Liu <neal.liu@mediatek.com> wrote:  
> > > > >
> > > > > For MediaTek SoCs on ARMv8 with TrustZone enabled, peripherals like
> > > > > entropy sources is not accessible from normal world (linux) and
> > > > > rather accessible from secure world (ATF/TEE) only. This driver aims
> > > > > to provide a generic interface to ATF rng service.
> > > > >  
> > > >
> > > > I am working on several SoCs that also will need this kind of driver
> > > > to get entropy from Arm trusted firmware.
> > > > If you intend to make this a generic interface, please clean up the
> > > > references to MediaTek and give it a more generic name. For example
> > > > "Arm Trusted Firmware random number driver".
> > > >
> > > > It will also be helpful if the SMC call number is configurable.
> > > >
> > > > - Lars  
> > >
> > > Yes, I'm trying to make this to a generic interface. I'll try to make
> > > HW/platform related dependency to be configurable and let it more
> > > generic.
> > > Thanks for your suggestion.
> > >  
> > 
> > I don't think it makes sense for each arm64 platform to expose an
> > entropy source via SMC calls in a slightly different way, and model it
> > as a h/w driver. Instead, we should try to standardize this, and
> > perhaps expose it via the architectural helpers that already exist
> > (get_random_seed_long() and friends), so they get plugged into the
> > kernel random pool driver directly.
> 
> Absolutely. I'd love to see a standard, ARM-specified, virtualizable
> RNG that is abstracted from the HW.

Same here. I hacked up some initial code for doing this with KVM [1], but
I'd much rather it was part of a standard service specification so that
we could use the same interface for talking to the firmware and the
hypervisor.

Will

[1] https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/log/?h=kvm/hvc
Marc Zyngier Dec. 3, 2019, 11:17 a.m. UTC | #9
On 2019-12-03 04:16, Florian Fainelli wrote:
> On 12/2/2019 11:11 AM, Marc Zyngier wrote:
>> On Mon, 2 Dec 2019 16:12:09 +0000
>> Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>>
>>> (adding some more arm64 folks)
>>>
>>> On Fri, 29 Nov 2019 at 11:30, Neal Liu <neal.liu@mediatek.com> 
>>> wrote:
>>>>
>>>> On Fri, 2019-11-29 at 18:02 +0800, Lars Persson wrote:
>>>>> Hi Neal,
>>>>>
>>>>> On Wed, Nov 27, 2019 at 3:23 PM Neal Liu <neal.liu@mediatek.com> 
>>>>> wrote:
>>>>>>
>>>>>> For MediaTek SoCs on ARMv8 with TrustZone enabled, peripherals 
>>>>>> like
>>>>>> entropy sources is not accessible from normal world (linux) and
>>>>>> rather accessible from secure world (ATF/TEE) only. This driver 
>>>>>> aims
>>>>>> to provide a generic interface to ATF rng service.
>>>>>>
>>>>>
>>>>> I am working on several SoCs that also will need this kind of 
>>>>> driver
>>>>> to get entropy from Arm trusted firmware.
>>>>> If you intend to make this a generic interface, please clean up 
>>>>> the
>>>>> references to MediaTek and give it a more generic name. For 
>>>>> example
>>>>> "Arm Trusted Firmware random number driver".
>>>>>
>>>>> It will also be helpful if the SMC call number is configurable.
>>>>>
>>>>> - Lars
>>>>
>>>> Yes, I'm trying to make this to a generic interface. I'll try to 
>>>> make
>>>> HW/platform related dependency to be configurable and let it more
>>>> generic.
>>>> Thanks for your suggestion.
>>>>
>>>
>>> I don't think it makes sense for each arm64 platform to expose an
>>> entropy source via SMC calls in a slightly different way, and model 
>>> it
>>> as a h/w driver. Instead, we should try to standardize this, and
>>> perhaps expose it via the architectural helpers that already exist
>>> (get_random_seed_long() and friends), so they get plugged into the
>>> kernel random pool driver directly.
>>
>> Absolutely. I'd love to see a standard, ARM-specified, virtualizable
>> RNG that is abstracted from the HW.
>
> Do you think we could use virtio-rng on top of a modified virtio-mmio
> which instead of being backed by a hardware mailbox, could use 
> hvc/smc
> calls to signal writes to shared memory and get notifications via an
> interrupt? This would also open up the doors to other virtio uses 
> cases
> beyond just RNG (e.g.: console, block devices?). If this is 
> completely
> stupid, then please disregard this comment.

The problem with a virtio device is that it is a ... device. What we 
want
is to be able to have access to an entropy source extremely early in 
the
kernel life, and devices tend to be available pretty late in the game.
This means we cannot plug them in the architectural helpers that Ard
mentions above.

What you're suggesting looks more like a new kind of virtio transport,
which is interesting, in a remarkably twisted way... ;-)

Thanks,

         M.
Neal Liu Dec. 12, 2019, 5:13 a.m. UTC | #10
On Tue, 2019-12-03 at 11:17 +0000, Marc Zyngier wrote:
> On 2019-12-03 04:16, Florian Fainelli wrote:
> > On 12/2/2019 11:11 AM, Marc Zyngier wrote:
> >> On Mon, 2 Dec 2019 16:12:09 +0000
> >> Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >>
> >>> (adding some more arm64 folks)
> >>>
> >>> On Fri, 29 Nov 2019 at 11:30, Neal Liu <neal.liu@mediatek.com> 
> >>> wrote:
> >>>>
> >>>> On Fri, 2019-11-29 at 18:02 +0800, Lars Persson wrote:
> >>>>> Hi Neal,
> >>>>>
> >>>>> On Wed, Nov 27, 2019 at 3:23 PM Neal Liu <neal.liu@mediatek.com> 
> >>>>> wrote:
> >>>>>>
> >>>>>> For MediaTek SoCs on ARMv8 with TrustZone enabled, peripherals 
> >>>>>> like
> >>>>>> entropy sources is not accessible from normal world (linux) and
> >>>>>> rather accessible from secure world (ATF/TEE) only. This driver 
> >>>>>> aims
> >>>>>> to provide a generic interface to ATF rng service.
> >>>>>>
> >>>>>
> >>>>> I am working on several SoCs that also will need this kind of 
> >>>>> driver
> >>>>> to get entropy from Arm trusted firmware.
> >>>>> If you intend to make this a generic interface, please clean up 
> >>>>> the
> >>>>> references to MediaTek and give it a more generic name. For 
> >>>>> example
> >>>>> "Arm Trusted Firmware random number driver".
> >>>>>
> >>>>> It will also be helpful if the SMC call number is configurable.
> >>>>>
> >>>>> - Lars
> >>>>
> >>>> Yes, I'm trying to make this to a generic interface. I'll try to 
> >>>> make
> >>>> HW/platform related dependency to be configurable and let it more
> >>>> generic.
> >>>> Thanks for your suggestion.
> >>>>
> >>>
> >>> I don't think it makes sense for each arm64 platform to expose an
> >>> entropy source via SMC calls in a slightly different way, and model 
> >>> it
> >>> as a h/w driver. Instead, we should try to standardize this, and
> >>> perhaps expose it via the architectural helpers that already exist
> >>> (get_random_seed_long() and friends), so they get plugged into the
> >>> kernel random pool driver directly.
> >>
> >> Absolutely. I'd love to see a standard, ARM-specified, virtualizable
> >> RNG that is abstracted from the HW.
> >
> > Do you think we could use virtio-rng on top of a modified virtio-mmio
> > which instead of being backed by a hardware mailbox, could use 
> > hvc/smc
> > calls to signal writes to shared memory and get notifications via an
> > interrupt? This would also open up the doors to other virtio uses 
> > cases
> > beyond just RNG (e.g.: console, block devices?). If this is 
> > completely
> > stupid, then please disregard this comment.
> 
> The problem with a virtio device is that it is a ... device. What we 
> want
> is to be able to have access to an entropy source extremely early in 
> the
> kernel life, and devices tend to be available pretty late in the game.
> This means we cannot plug them in the architectural helpers that Ard
> mentions above.
> 
> What you're suggesting looks more like a new kind of virtio transport,
> which is interesting, in a remarkably twisted way... ;-)
> 
> Thanks,
> 
>          M.

In conclusion, is it helpful that hw_random has a generic interface to
add device randomness by talking to hwrng which is implemented in the
firmware or the hypervisor?
For most chip vendors, I think the answer is yes. We already prepared a
new patchset and need you agree with this idea.

Thanks

-Neal
Marc Zyngier Dec. 12, 2019, 11:45 a.m. UTC | #11
On 2019-12-12 05:13, Neal Liu wrote:
> On Tue, 2019-12-03 at 11:17 +0000, Marc Zyngier wrote:
>> On 2019-12-03 04:16, Florian Fainelli wrote:
>> > On 12/2/2019 11:11 AM, Marc Zyngier wrote:
>> >> On Mon, 2 Dec 2019 16:12:09 +0000
>> >> Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> >>
>> >>> (adding some more arm64 folks)
>> >>>
>> >>> On Fri, 29 Nov 2019 at 11:30, Neal Liu <neal.liu@mediatek.com>
>> >>> wrote:
>> >>>>
>> >>>> On Fri, 2019-11-29 at 18:02 +0800, Lars Persson wrote:
>> >>>>> Hi Neal,
>> >>>>>
>> >>>>> On Wed, Nov 27, 2019 at 3:23 PM Neal Liu 
>> <neal.liu@mediatek.com>
>> >>>>> wrote:
>> >>>>>>
>> >>>>>> For MediaTek SoCs on ARMv8 with TrustZone enabled, 
>> peripherals
>> >>>>>> like
>> >>>>>> entropy sources is not accessible from normal world (linux) 
>> and
>> >>>>>> rather accessible from secure world (ATF/TEE) only. This 
>> driver
>> >>>>>> aims
>> >>>>>> to provide a generic interface to ATF rng service.
>> >>>>>>
>> >>>>>
>> >>>>> I am working on several SoCs that also will need this kind of
>> >>>>> driver
>> >>>>> to get entropy from Arm trusted firmware.
>> >>>>> If you intend to make this a generic interface, please clean 
>> up
>> >>>>> the
>> >>>>> references to MediaTek and give it a more generic name. For
>> >>>>> example
>> >>>>> "Arm Trusted Firmware random number driver".
>> >>>>>
>> >>>>> It will also be helpful if the SMC call number is 
>> configurable.
>> >>>>>
>> >>>>> - Lars
>> >>>>
>> >>>> Yes, I'm trying to make this to a generic interface. I'll try 
>> to
>> >>>> make
>> >>>> HW/platform related dependency to be configurable and let it 
>> more
>> >>>> generic.
>> >>>> Thanks for your suggestion.
>> >>>>
>> >>>
>> >>> I don't think it makes sense for each arm64 platform to expose 
>> an
>> >>> entropy source via SMC calls in a slightly different way, and 
>> model
>> >>> it
>> >>> as a h/w driver. Instead, we should try to standardize this, and
>> >>> perhaps expose it via the architectural helpers that already 
>> exist
>> >>> (get_random_seed_long() and friends), so they get plugged into 
>> the
>> >>> kernel random pool driver directly.
>> >>
>> >> Absolutely. I'd love to see a standard, ARM-specified, 
>> virtualizable
>> >> RNG that is abstracted from the HW.
>> >
>> > Do you think we could use virtio-rng on top of a modified 
>> virtio-mmio
>> > which instead of being backed by a hardware mailbox, could use
>> > hvc/smc
>> > calls to signal writes to shared memory and get notifications via 
>> an
>> > interrupt? This would also open up the doors to other virtio uses
>> > cases
>> > beyond just RNG (e.g.: console, block devices?). If this is
>> > completely
>> > stupid, then please disregard this comment.
>>
>> The problem with a virtio device is that it is a ... device. What we
>> want
>> is to be able to have access to an entropy source extremely early in
>> the
>> kernel life, and devices tend to be available pretty late in the 
>> game.
>> This means we cannot plug them in the architectural helpers that Ard
>> mentions above.
>>
>> What you're suggesting looks more like a new kind of virtio 
>> transport,
>> which is interesting, in a remarkably twisted way... ;-)
>>
>> Thanks,
>>
>>          M.
>
> In conclusion, is it helpful that hw_random has a generic interface 
> to
> add device randomness by talking to hwrng which is implemented in the
> firmware or the hypervisor?
> For most chip vendors, I think the answer is yes. We already prepared 
> a
> new patchset and need you agree with this idea.

As long as it is a *unified* interface, I'm all for that.

Thanks,

         M.
Ard Biesheuvel Dec. 12, 2019, 2:03 p.m. UTC | #12
On Thu, 12 Dec 2019 at 12:45, Marc Zyngier <maz@kernel.org> wrote:
>
> On 2019-12-12 05:13, Neal Liu wrote:
> > On Tue, 2019-12-03 at 11:17 +0000, Marc Zyngier wrote:
> >> On 2019-12-03 04:16, Florian Fainelli wrote:
> >> > On 12/2/2019 11:11 AM, Marc Zyngier wrote:
> >> >> On Mon, 2 Dec 2019 16:12:09 +0000
> >> >> Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >> >>
> >> >>> (adding some more arm64 folks)
> >> >>>
> >> >>> On Fri, 29 Nov 2019 at 11:30, Neal Liu <neal.liu@mediatek.com>
> >> >>> wrote:
> >> >>>>
> >> >>>> On Fri, 2019-11-29 at 18:02 +0800, Lars Persson wrote:
> >> >>>>> Hi Neal,
> >> >>>>>
> >> >>>>> On Wed, Nov 27, 2019 at 3:23 PM Neal Liu
> >> <neal.liu@mediatek.com>
> >> >>>>> wrote:
> >> >>>>>>
> >> >>>>>> For MediaTek SoCs on ARMv8 with TrustZone enabled,
> >> peripherals
> >> >>>>>> like
> >> >>>>>> entropy sources is not accessible from normal world (linux)
> >> and
> >> >>>>>> rather accessible from secure world (ATF/TEE) only. This
> >> driver
> >> >>>>>> aims
> >> >>>>>> to provide a generic interface to ATF rng service.
> >> >>>>>>
> >> >>>>>
> >> >>>>> I am working on several SoCs that also will need this kind of
> >> >>>>> driver
> >> >>>>> to get entropy from Arm trusted firmware.
> >> >>>>> If you intend to make this a generic interface, please clean
> >> up
> >> >>>>> the
> >> >>>>> references to MediaTek and give it a more generic name. For
> >> >>>>> example
> >> >>>>> "Arm Trusted Firmware random number driver".
> >> >>>>>
> >> >>>>> It will also be helpful if the SMC call number is
> >> configurable.
> >> >>>>>
> >> >>>>> - Lars
> >> >>>>
> >> >>>> Yes, I'm trying to make this to a generic interface. I'll try
> >> to
> >> >>>> make
> >> >>>> HW/platform related dependency to be configurable and let it
> >> more
> >> >>>> generic.
> >> >>>> Thanks for your suggestion.
> >> >>>>
> >> >>>
> >> >>> I don't think it makes sense for each arm64 platform to expose
> >> an
> >> >>> entropy source via SMC calls in a slightly different way, and
> >> model
> >> >>> it
> >> >>> as a h/w driver. Instead, we should try to standardize this, and
> >> >>> perhaps expose it via the architectural helpers that already
> >> exist
> >> >>> (get_random_seed_long() and friends), so they get plugged into
> >> the
> >> >>> kernel random pool driver directly.
> >> >>
> >> >> Absolutely. I'd love to see a standard, ARM-specified,
> >> virtualizable
> >> >> RNG that is abstracted from the HW.
> >> >
> >> > Do you think we could use virtio-rng on top of a modified
> >> virtio-mmio
> >> > which instead of being backed by a hardware mailbox, could use
> >> > hvc/smc
> >> > calls to signal writes to shared memory and get notifications via
> >> an
> >> > interrupt? This would also open up the doors to other virtio uses
> >> > cases
> >> > beyond just RNG (e.g.: console, block devices?). If this is
> >> > completely
> >> > stupid, then please disregard this comment.
> >>
> >> The problem with a virtio device is that it is a ... device. What we
> >> want
> >> is to be able to have access to an entropy source extremely early in
> >> the
> >> kernel life, and devices tend to be available pretty late in the
> >> game.
> >> This means we cannot plug them in the architectural helpers that Ard
> >> mentions above.
> >>
> >> What you're suggesting looks more like a new kind of virtio
> >> transport,
> >> which is interesting, in a remarkably twisted way... ;-)
> >>
> >> Thanks,
> >>
> >>          M.
> >
> > In conclusion, is it helpful that hw_random has a generic interface
> > to
> > add device randomness by talking to hwrng which is implemented in the
> > firmware or the hypervisor?
> > For most chip vendors, I think the answer is yes. We already prepared
> > a
> > new patchset and need you agree with this idea.
>
> As long as it is a *unified* interface, I'm all for that.
>


Yeah, but I'm not sure it makes sense to model it as a device like
this. It would be nice if we could tie this into the ARM SMCCC
discovery, and use the SMC calls to back arch_get_random_seed_long()
[provided we fix the braindead way in which that is being used today
in the interrupt code]
Marc Zyngier Dec. 12, 2019, 2:30 p.m. UTC | #13
On 2019-12-12 14:03, Ard Biesheuvel wrote:
> On Thu, 12 Dec 2019 at 12:45, Marc Zyngier <maz@kernel.org> wrote:
>>
>> On 2019-12-12 05:13, Neal Liu wrote:
>> > On Tue, 2019-12-03 at 11:17 +0000, Marc Zyngier wrote:
>> >> On 2019-12-03 04:16, Florian Fainelli wrote:
>> >> > On 12/2/2019 11:11 AM, Marc Zyngier wrote:
>> >> >> On Mon, 2 Dec 2019 16:12:09 +0000
>> >> >> Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> >> >>
>> >> >>> (adding some more arm64 folks)
>> >> >>>
>> >> >>> On Fri, 29 Nov 2019 at 11:30, Neal Liu 
>> <neal.liu@mediatek.com>
>> >> >>> wrote:
>> >> >>>>
>> >> >>>> On Fri, 2019-11-29 at 18:02 +0800, Lars Persson wrote:
>> >> >>>>> Hi Neal,
>> >> >>>>>
>> >> >>>>> On Wed, Nov 27, 2019 at 3:23 PM Neal Liu
>> >> <neal.liu@mediatek.com>
>> >> >>>>> wrote:
>> >> >>>>>>
>> >> >>>>>> For MediaTek SoCs on ARMv8 with TrustZone enabled,
>> >> peripherals
>> >> >>>>>> like
>> >> >>>>>> entropy sources is not accessible from normal world 
>> (linux)
>> >> and
>> >> >>>>>> rather accessible from secure world (ATF/TEE) only. This
>> >> driver
>> >> >>>>>> aims
>> >> >>>>>> to provide a generic interface to ATF rng service.
>> >> >>>>>>
>> >> >>>>>
>> >> >>>>> I am working on several SoCs that also will need this kind 
>> of
>> >> >>>>> driver
>> >> >>>>> to get entropy from Arm trusted firmware.
>> >> >>>>> If you intend to make this a generic interface, please 
>> clean
>> >> up
>> >> >>>>> the
>> >> >>>>> references to MediaTek and give it a more generic name. For
>> >> >>>>> example
>> >> >>>>> "Arm Trusted Firmware random number driver".
>> >> >>>>>
>> >> >>>>> It will also be helpful if the SMC call number is
>> >> configurable.
>> >> >>>>>
>> >> >>>>> - Lars
>> >> >>>>
>> >> >>>> Yes, I'm trying to make this to a generic interface. I'll 
>> try
>> >> to
>> >> >>>> make
>> >> >>>> HW/platform related dependency to be configurable and let it
>> >> more
>> >> >>>> generic.
>> >> >>>> Thanks for your suggestion.
>> >> >>>>
>> >> >>>
>> >> >>> I don't think it makes sense for each arm64 platform to 
>> expose
>> >> an
>> >> >>> entropy source via SMC calls in a slightly different way, and
>> >> model
>> >> >>> it
>> >> >>> as a h/w driver. Instead, we should try to standardize this, 
>> and
>> >> >>> perhaps expose it via the architectural helpers that already
>> >> exist
>> >> >>> (get_random_seed_long() and friends), so they get plugged 
>> into
>> >> the
>> >> >>> kernel random pool driver directly.
>> >> >>
>> >> >> Absolutely. I'd love to see a standard, ARM-specified,
>> >> virtualizable
>> >> >> RNG that is abstracted from the HW.
>> >> >
>> >> > Do you think we could use virtio-rng on top of a modified
>> >> virtio-mmio
>> >> > which instead of being backed by a hardware mailbox, could use
>> >> > hvc/smc
>> >> > calls to signal writes to shared memory and get notifications 
>> via
>> >> an
>> >> > interrupt? This would also open up the doors to other virtio 
>> uses
>> >> > cases
>> >> > beyond just RNG (e.g.: console, block devices?). If this is
>> >> > completely
>> >> > stupid, then please disregard this comment.
>> >>
>> >> The problem with a virtio device is that it is a ... device. What 
>> we
>> >> want
>> >> is to be able to have access to an entropy source extremely early 
>> in
>> >> the
>> >> kernel life, and devices tend to be available pretty late in the
>> >> game.
>> >> This means we cannot plug them in the architectural helpers that 
>> Ard
>> >> mentions above.
>> >>
>> >> What you're suggesting looks more like a new kind of virtio
>> >> transport,
>> >> which is interesting, in a remarkably twisted way... ;-)
>> >>
>> >> Thanks,
>> >>
>> >>          M.
>> >
>> > In conclusion, is it helpful that hw_random has a generic 
>> interface
>> > to
>> > add device randomness by talking to hwrng which is implemented in 
>> the
>> > firmware or the hypervisor?
>> > For most chip vendors, I think the answer is yes. We already 
>> prepared
>> > a
>> > new patchset and need you agree with this idea.
>>
>> As long as it is a *unified* interface, I'm all for that.
>>
>
>
> Yeah, but I'm not sure it makes sense to model it as a device like
> this. It would be nice if we could tie this into the ARM SMCCC
> discovery, and use the SMC calls to back arch_get_random_seed_long()

Probably I wasn't clear enough, but that's really what I meant by
a unified interface (implemented by the firmware or the hypervisor).

> [provided we fix the braindead way in which that is being used today
> in the interrupt code]

Ah, I said I'd look into it. Thanks for the reminder...

Thanks,

         M.
Sudeep Holla Dec. 12, 2019, 2:43 p.m. UTC | #14
On Mon, Dec 02, 2019 at 04:12:09PM +0000, Ard Biesheuvel wrote:
> (adding some more arm64 folks)
> 
> On Fri, 29 Nov 2019 at 11:30, Neal Liu <neal.liu@mediatek.com> wrote:
> >
> > On Fri, 2019-11-29 at 18:02 +0800, Lars Persson wrote:
> > > Hi Neal,
> > >
> > > On Wed, Nov 27, 2019 at 3:23 PM Neal Liu <neal.liu@mediatek.com> wrote:
> > > >
> > > > For MediaTek SoCs on ARMv8 with TrustZone enabled, peripherals like
> > > > entropy sources is not accessible from normal world (linux) and
> > > > rather accessible from secure world (ATF/TEE) only. This driver aims
> > > > to provide a generic interface to ATF rng service.
> > > >
> > >
> > > I am working on several SoCs that also will need this kind of driver
> > > to get entropy from Arm trusted firmware.
> > > If you intend to make this a generic interface, please clean up the
> > > references to MediaTek and give it a more generic name. For example
> > > "Arm Trusted Firmware random number driver".
> > >
> > > It will also be helpful if the SMC call number is configurable.
> > >
> > > - Lars
> >
> > Yes, I'm trying to make this to a generic interface. I'll try to make
> > HW/platform related dependency to be configurable and let it more
> > generic.
> > Thanks for your suggestion.
> >
> 
> I don't think it makes sense for each arm64 platform to expose an
> entropy source via SMC calls in a slightly different way, and model it
> as a h/w driver. Instead, we should try to standardize this, and
> perhaps expose it via the architectural helpers that already exist
> (get_random_seed_long() and friends), so they get plugged into the
> kernel random pool driver directly.
> 
> Note that in addition to drivers based on vendor SMC calls, we already
> have a RNG h/w driver based on OP-TEE as well, where the driver
> attaches to a standardized trusted OS interface identified by a UUID,
> and which also gets invoked via SMC calls into secure firmware.

Yes, I agree. I had raised the issue internally and forgot to follow up.
I raised this few months back after I read a blog[1]

--
Regards,
Sudeep

[1] https://community.arm.com/developer/ip-products/processors/f/cortex-a-forum/43679/arm-really-should-standardize-an-smc-interface-for-hardware-random-number-generators
diff mbox series

Patch

diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index 25a7d8f..f08c852 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig
@@ -398,6 +398,22 @@  config HW_RANDOM_MTK
 
 	  If unsure, say Y.
 
+config HW_RANDOM_MTK_SEC
+	tristate "MediaTek Security Random Number Generator support"
+	depends on HW_RANDOM
+	depends on ARCH_MEDIATEK || COMPILE_TEST
+	default HW_RANDOM
+	  help
+	  This driver provides kernel-side support for the Random Number
+	  Generator hardware found on MediaTek SoCs. The difference with
+	  mtk-rng is the Random Number Generator hardware is secure
+	  access only.
+
+	  To compile this driver as a module, choose M here. the
+	  module will be called mtk-sec-rng.
+
+	  If unsure, say Y.
+
 config HW_RANDOM_S390
 	tristate "S390 True Random Number Generator support"
 	depends on S390
diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
index 7c9ef4a..bee5412 100644
--- a/drivers/char/hw_random/Makefile
+++ b/drivers/char/hw_random/Makefile
@@ -36,6 +36,7 @@  obj-$(CONFIG_HW_RANDOM_PIC32) += pic32-rng.o
 obj-$(CONFIG_HW_RANDOM_MESON) += meson-rng.o
 obj-$(CONFIG_HW_RANDOM_CAVIUM) += cavium-rng.o cavium-rng-vf.o
 obj-$(CONFIG_HW_RANDOM_MTK)	+= mtk-rng.o
+obj-$(CONFIG_HW_RANDOM_MTK_SEC)	+= mtk-sec-rng.o
 obj-$(CONFIG_HW_RANDOM_S390) += s390-trng.o
 obj-$(CONFIG_HW_RANDOM_KEYSTONE) += ks-sa-rng.o
 obj-$(CONFIG_HW_RANDOM_OPTEE) += optee-rng.o
diff --git a/drivers/char/hw_random/mtk-sec-rng.c b/drivers/char/hw_random/mtk-sec-rng.c
new file mode 100644
index 0000000..69ddeca
--- /dev/null
+++ b/drivers/char/hw_random/mtk-sec-rng.c
@@ -0,0 +1,103 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 MediaTek Inc.
+ */
+
+#include <linux/arm-smccc.h>
+#include <linux/hw_random.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/soc/mediatek/mtk_sip_svc.h>
+
+#define MTK_SEC_RNG_MAGIC	0x74726e67
+#define SMC_RET_NUM		4
+#define MTK_SEC_RND_SIZE	(sizeof(u32) * SMC_RET_NUM)
+
+static void mtk_sec_get_rnd(uint32_t *val)
+{
+	struct arm_smccc_res res;
+
+	arm_smccc_smc(MTK_SIP_KERNEL_GET_RND,
+		      MTK_SEC_RNG_MAGIC, 0, 0, 0, 0, 0, 0, &res);
+
+	val[0] = res.a0;
+	val[1] = res.a1;
+	val[2] = res.a2;
+	val[3] = res.a3;
+}
+
+static int mtk_sec_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)
+{
+	u32 val[4] = {0};
+	int retval = 0;
+	int i;
+
+	while (max >= MTK_SEC_RND_SIZE) {
+		mtk_sec_get_rnd(val);
+
+		for (i = 0; i < SMC_RET_NUM; i++) {
+			*(u32 *)buf = val[i];
+			buf += sizeof(u32);
+		}
+
+		retval += MTK_SEC_RND_SIZE;
+		max -= MTK_SEC_RND_SIZE;
+	}
+
+	return retval;
+}
+
+static struct hwrng mtk_sec_rng = {
+	.name = "mtk_sec_rng",
+	.read = mtk_sec_rng_read,
+	.quality = 900,
+};
+
+static int mtk_sec_rng_probe(void)
+{
+	int ret;
+
+	ret = hwrng_register(&mtk_sec_rng);
+	if (ret) {
+		pr_err("Failed to register rng device: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int __init mtk_sec_rng_driver_init(void)
+{
+	struct device_node *fw_np;
+	struct device_node *np;
+	const char *method;
+
+	fw_np = of_find_node_by_name(NULL, "firmware");
+	if (!fw_np)
+		return -ENODEV;
+
+	np = of_find_compatible_node(fw_np, NULL, "mediatek,mtk-sec-rng");
+	if (!np)
+		return -ENODEV;
+
+	if (of_property_read_string(np, "method", &method))
+		return -ENXIO;
+
+	if (strncmp("smc", method, strlen("smc")))
+		return -EINVAL;
+
+	return mtk_sec_rng_probe();
+}
+
+static void __exit mtk_sec_rng_driver_exit(void)
+{
+	hwrng_unregister(&mtk_sec_rng);
+}
+
+module_init(mtk_sec_rng_driver_init);
+module_exit(mtk_sec_rng_driver_exit);
+
+MODULE_DESCRIPTION("MediaTek Security Random Number Generator Driver");
+MODULE_AUTHOR("Neal Liu <neal.liu@mediatek.com>");
+MODULE_LICENSE("GPL");