| Message ID | 20220720122926.415659-4-sughosh.ganu@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tpm: rng: Move TPM RNG functionality to driver model | expand |
On Wed, 20 Jul 2022 at 15:30, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > The TPM device comes with the random number generator(RNG) > functionality which is built into the TPM device. Add logic to add the > RNG child device in the TPM uclass post probe callback. > > The RNG device can then be used to pass a set of random bytes to the > linux kernel, need for address space randomisation through the > EFI_RNG_PROTOCOL interface. > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > --- > Changes since V6: None > > drivers/tpm/tpm-uclass.c | 37 +++++++++++++++++++++++++++++++++---- > 1 file changed, 33 insertions(+), 4 deletions(-) > > diff --git a/drivers/tpm/tpm-uclass.c b/drivers/tpm/tpm-uclass.c > index f67fe1019b..e1f1ef01e1 100644 > --- a/drivers/tpm/tpm-uclass.c > +++ b/drivers/tpm/tpm-uclass.c > @@ -11,10 +11,15 @@ > #include <log.h> > #include <linux/delay.h> > #include <linux/unaligned/be_byteshift.h> > +#include <tpm_api.h> > #include <tpm-v1.h> > #include <tpm-v2.h> > #include "tpm_internal.h" > > +#include <dm/lists.h> > + > +#define TPM_RNG_DRV_NAME "tpm-rng" > + > int tpm_open(struct udevice *dev) > { > struct tpm_ops *ops = tpm_get_ops(dev); > @@ -136,12 +141,36 @@ int tpm_xfer(struct udevice *dev, const uint8_t *sendbuf, size_t send_size, > return 0; > } > > +static int tpm_uclass_post_probe(struct udevice *dev) > +{ > + int ret; > + const char *drv = TPM_RNG_DRV_NAME; > + struct udevice *child; > + > + if (CONFIG_IS_ENABLED(TPM_RNG)) { > + ret = device_find_first_child_by_uclass(dev, UCLASS_RNG, > + &child); > + > + if (ret != -ENODEV) { > + log_debug("RNG child already added to the TPM device\n"); > + return ret; > + } > + > + ret = device_bind_driver(dev, drv, "tpm-rng0", &child); > + if (ret) > + return log_msg_ret("bind", ret); > + } > + > + return 0; > +} > + > UCLASS_DRIVER(tpm) = { > - .id = UCLASS_TPM, > - .name = "tpm", > - .flags = DM_UC_FLAG_SEQ_ALIAS, > + .id = UCLASS_TPM, > + .name = "tpm", > + .flags = DM_UC_FLAG_SEQ_ALIAS, > #if CONFIG_IS_ENABLED(OF_REAL) > - .post_bind = dm_scan_fdt_dev, > + .post_bind = dm_scan_fdt_dev, > #endif > + .post_probe = tpm_uclass_post_probe, > .per_device_auto = sizeof(struct tpm_chip_priv), > }; > -- > 2.34.1 > Simon can you please ACK this ? Looks good to me Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Hi Sughosh, On Wed, 20 Jul 2022 at 06:30, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > The TPM device comes with the random number generator(RNG) > functionality which is built into the TPM device. Add logic to add the > RNG child device in the TPM uclass post probe callback. > > The RNG device can then be used to pass a set of random bytes to the > linux kernel, need for address space randomisation through the > EFI_RNG_PROTOCOL interface. Ilias has asked me to review this patch again. I don't want my review tag on it since it is not correct, so far as driver model / device tree go. But I have no objection to it going in since my understanding is we can disable TPM_RNG later as needed. I prefer to have some acknowledgement of the previous discussion, e.g.: No compatible string is provided because this is not available in the binding defined by Linux. If multiple rand devices are in the system, then some method of selecting them (other than device tree) will need to be used, or a binding will need to be added. since I cannot imagine people remembering to look up the previous version in patchwork. But if you don't want that, then that's fine. > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > --- > Changes since V6: None > > drivers/tpm/tpm-uclass.c | 37 +++++++++++++++++++++++++++++++++---- > 1 file changed, 33 insertions(+), 4 deletions(-) > > diff --git a/drivers/tpm/tpm-uclass.c b/drivers/tpm/tpm-uclass.c > index f67fe1019b..e1f1ef01e1 100644 > --- a/drivers/tpm/tpm-uclass.c > +++ b/drivers/tpm/tpm-uclass.c > @@ -11,10 +11,15 @@ > #include <log.h> > #include <linux/delay.h> > #include <linux/unaligned/be_byteshift.h> > +#include <tpm_api.h> > #include <tpm-v1.h> > #include <tpm-v2.h> > #include "tpm_internal.h" > > +#include <dm/lists.h> BTW this should be above the previous header. > + > +#define TPM_RNG_DRV_NAME "tpm-rng" > + > int tpm_open(struct udevice *dev) > { > struct tpm_ops *ops = tpm_get_ops(dev); > @@ -136,12 +141,36 @@ int tpm_xfer(struct udevice *dev, const uint8_t *sendbuf, size_t send_size, > return 0; > } > > +static int tpm_uclass_post_probe(struct udevice *dev) > +{ > + int ret; > + const char *drv = TPM_RNG_DRV_NAME; > + struct udevice *child; > + > + if (CONFIG_IS_ENABLED(TPM_RNG)) { > + ret = device_find_first_child_by_uclass(dev, UCLASS_RNG, > + &child); > + > + if (ret != -ENODEV) { > + log_debug("RNG child already added to the TPM device\n"); > + return ret; > + } > + > + ret = device_bind_driver(dev, drv, "tpm-rng0", &child); If this does get re-issued, I think this could just use TPM_RNG_DRV_NAME directory. > + if (ret) > + return log_msg_ret("bind", ret); > + } > + > + return 0; > +} > + > UCLASS_DRIVER(tpm) = { > - .id = UCLASS_TPM, > - .name = "tpm", > - .flags = DM_UC_FLAG_SEQ_ALIAS, > + .id = UCLASS_TPM, > + .name = "tpm", > + .flags = DM_UC_FLAG_SEQ_ALIAS, > #if CONFIG_IS_ENABLED(OF_REAL) > - .post_bind = dm_scan_fdt_dev, > + .post_bind = dm_scan_fdt_dev, > #endif > + .post_probe = tpm_uclass_post_probe, > .per_device_auto = sizeof(struct tpm_chip_priv), > }; > -- > 2.34.1 > Regards, Simon
Hi Simon, On Fri, 22 Jul 2022 at 12:00, Simon Glass <sjg@chromium.org> wrote: > > Hi Sughosh, > > On Wed, 20 Jul 2022 at 06:30, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > The TPM device comes with the random number generator(RNG) > > functionality which is built into the TPM device. Add logic to add the > > RNG child device in the TPM uclass post probe callback. > > > > The RNG device can then be used to pass a set of random bytes to the > > linux kernel, need for address space randomisation through the > > EFI_RNG_PROTOCOL interface. > > Ilias has asked me to review this patch again. > > I don't want my review tag on it since it is not correct, so far as > driver model / device tree go. But I have no objection to it going in > since my understanding is we can disable TPM_RNG later as needed. Yes you can > > I prefer to have some acknowledgement of the previous discussion, e.g.: Sure > > No compatible string is provided because this is not available in > the binding defined by Linux. If multiple rand devices are in the > system, then some method of selecting them (other than device tree) > will need to be used, or a binding will need to be added. > > since I cannot imagine people remembering to look up the previous > version in patchwork. Comment in the code or commit message? I'd prefer adding it as a comment > > But if you don't want that, then that's fine. No I am fine that makes sense. What I was looking for was basically your acknowledgement that we are going to pick this up. Shall I carry it on the TPM tree? Thanks /Ilias > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > --- > > Changes since V6: None > > > > drivers/tpm/tpm-uclass.c | 37 +++++++++++++++++++++++++++++++++---- > > 1 file changed, 33 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/tpm/tpm-uclass.c b/drivers/tpm/tpm-uclass.c > > index f67fe1019b..e1f1ef01e1 100644 > > --- a/drivers/tpm/tpm-uclass.c > > +++ b/drivers/tpm/tpm-uclass.c > > @@ -11,10 +11,15 @@ > > #include <log.h> > > #include <linux/delay.h> > > #include <linux/unaligned/be_byteshift.h> > > +#include <tpm_api.h> > > #include <tpm-v1.h> > > #include <tpm-v2.h> > > #include "tpm_internal.h" > > > > +#include <dm/lists.h> > > BTW this should be above the previous header. > > > + > > +#define TPM_RNG_DRV_NAME "tpm-rng" > > + > > int tpm_open(struct udevice *dev) > > { > > struct tpm_ops *ops = tpm_get_ops(dev); > > @@ -136,12 +141,36 @@ int tpm_xfer(struct udevice *dev, const uint8_t *sendbuf, size_t send_size, > > return 0; > > } > > > > +static int tpm_uclass_post_probe(struct udevice *dev) > > +{ > > + int ret; > > + const char *drv = TPM_RNG_DRV_NAME; > > + struct udevice *child; > > + > > + if (CONFIG_IS_ENABLED(TPM_RNG)) { > > + ret = device_find_first_child_by_uclass(dev, UCLASS_RNG, > > + &child); > > + > > + if (ret != -ENODEV) { > > + log_debug("RNG child already added to the TPM device\n"); > > + return ret; > > + } > > + > > + ret = device_bind_driver(dev, drv, "tpm-rng0", &child); > > If this does get re-issued, I think this could just use > TPM_RNG_DRV_NAME directory. > > > + if (ret) > > + return log_msg_ret("bind", ret); > > + } > > + > > + return 0; > > +} > > + > > UCLASS_DRIVER(tpm) = { > > - .id = UCLASS_TPM, > > - .name = "tpm", > > - .flags = DM_UC_FLAG_SEQ_ALIAS, > > + .id = UCLASS_TPM, > > + .name = "tpm", > > + .flags = DM_UC_FLAG_SEQ_ALIAS, > > #if CONFIG_IS_ENABLED(OF_REAL) > > - .post_bind = dm_scan_fdt_dev, > > + .post_bind = dm_scan_fdt_dev, > > #endif > > + .post_probe = tpm_uclass_post_probe, > > .per_device_auto = sizeof(struct tpm_chip_priv), > > }; > > -- > > 2.34.1 > > > > Regards, > Simon
Hi Ilias, On Fri, 22 Jul 2022 at 03:05, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > Hi Simon, > > On Fri, 22 Jul 2022 at 12:00, Simon Glass <sjg@chromium.org> wrote: > > > > Hi Sughosh, > > > > On Wed, 20 Jul 2022 at 06:30, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > > > The TPM device comes with the random number generator(RNG) > > > functionality which is built into the TPM device. Add logic to add the > > > RNG child device in the TPM uclass post probe callback. > > > > > > The RNG device can then be used to pass a set of random bytes to the > > > linux kernel, need for address space randomisation through the > > > EFI_RNG_PROTOCOL interface. > > > > Ilias has asked me to review this patch again. > > > > I don't want my review tag on it since it is not correct, so far as > > driver model / device tree go. But I have no objection to it going in > > since my understanding is we can disable TPM_RNG later as needed. > > Yes you can > > > > > I prefer to have some acknowledgement of the previous discussion, e.g.: > > Sure > > > > > No compatible string is provided because this is not available in > > the binding defined by Linux. If multiple rand devices are in the > > system, then some method of selecting them (other than device tree) > > will need to be used, or a binding will need to be added. > > > > since I cannot imagine people remembering to look up the previous > > version in patchwork. > > Comment in the code or commit message? I'd prefer adding it as a comment Comment is fine too > > > > > But if you don't want that, then that's fine. > > No I am fine that makes sense. What I was looking for was basically > your acknowledgement that we are going to pick this up. > > Shall I carry it on the TPM tree? It's up to you. I am due to collect some more patches in a day or two so if you'd like me to grab it, just assign it to me in patchwork. Regards, Simon > > Thanks > /Ilias > > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > > --- > > > Changes since V6: None > > > > > > drivers/tpm/tpm-uclass.c | 37 +++++++++++++++++++++++++++++++++---- > > > 1 file changed, 33 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/tpm/tpm-uclass.c b/drivers/tpm/tpm-uclass.c > > > index f67fe1019b..e1f1ef01e1 100644 > > > --- a/drivers/tpm/tpm-uclass.c > > > +++ b/drivers/tpm/tpm-uclass.c > > > @@ -11,10 +11,15 @@ > > > #include <log.h> > > > #include <linux/delay.h> > > > #include <linux/unaligned/be_byteshift.h> > > > +#include <tpm_api.h> > > > #include <tpm-v1.h> > > > #include <tpm-v2.h> > > > #include "tpm_internal.h" > > > > > > +#include <dm/lists.h> > > > > BTW this should be above the previous header. > > > > > + > > > +#define TPM_RNG_DRV_NAME "tpm-rng" > > > + > > > int tpm_open(struct udevice *dev) > > > { > > > struct tpm_ops *ops = tpm_get_ops(dev); > > > @@ -136,12 +141,36 @@ int tpm_xfer(struct udevice *dev, const uint8_t *sendbuf, size_t send_size, > > > return 0; > > > } > > > > > > +static int tpm_uclass_post_probe(struct udevice *dev) > > > +{ > > > + int ret; > > > + const char *drv = TPM_RNG_DRV_NAME; > > > + struct udevice *child; > > > + > > > + if (CONFIG_IS_ENABLED(TPM_RNG)) { > > > + ret = device_find_first_child_by_uclass(dev, UCLASS_RNG, > > > + &child); > > > + > > > + if (ret != -ENODEV) { > > > + log_debug("RNG child already added to the TPM device\n"); > > > + return ret; > > > + } > > > + > > > + ret = device_bind_driver(dev, drv, "tpm-rng0", &child); > > > > If this does get re-issued, I think this could just use > > TPM_RNG_DRV_NAME directory. > > > > > + if (ret) > > > + return log_msg_ret("bind", ret); > > > + } > > > + > > > + return 0; > > > +} > > > + > > > UCLASS_DRIVER(tpm) = { > > > - .id = UCLASS_TPM, > > > - .name = "tpm", > > > - .flags = DM_UC_FLAG_SEQ_ALIAS, > > > + .id = UCLASS_TPM, > > > + .name = "tpm", > > > + .flags = DM_UC_FLAG_SEQ_ALIAS, > > > #if CONFIG_IS_ENABLED(OF_REAL) > > > - .post_bind = dm_scan_fdt_dev, > > > + .post_bind = dm_scan_fdt_dev, > > > #endif > > > + .post_probe = tpm_uclass_post_probe, > > > .per_device_auto = sizeof(struct tpm_chip_priv), > > > }; > > > -- > > > 2.34.1 > > > > > > > Regards, > > Simon
diff --git a/drivers/tpm/tpm-uclass.c b/drivers/tpm/tpm-uclass.c index f67fe1019b..e1f1ef01e1 100644 --- a/drivers/tpm/tpm-uclass.c +++ b/drivers/tpm/tpm-uclass.c @@ -11,10 +11,15 @@ #include <log.h> #include <linux/delay.h> #include <linux/unaligned/be_byteshift.h> +#include <tpm_api.h> #include <tpm-v1.h> #include <tpm-v2.h> #include "tpm_internal.h" +#include <dm/lists.h> + +#define TPM_RNG_DRV_NAME "tpm-rng" + int tpm_open(struct udevice *dev) { struct tpm_ops *ops = tpm_get_ops(dev); @@ -136,12 +141,36 @@ int tpm_xfer(struct udevice *dev, const uint8_t *sendbuf, size_t send_size, return 0; } +static int tpm_uclass_post_probe(struct udevice *dev) +{ + int ret; + const char *drv = TPM_RNG_DRV_NAME; + struct udevice *child; + + if (CONFIG_IS_ENABLED(TPM_RNG)) { + ret = device_find_first_child_by_uclass(dev, UCLASS_RNG, + &child); + + if (ret != -ENODEV) { + log_debug("RNG child already added to the TPM device\n"); + return ret; + } + + ret = device_bind_driver(dev, drv, "tpm-rng0", &child); + if (ret) + return log_msg_ret("bind", ret); + } + + return 0; +} + UCLASS_DRIVER(tpm) = { - .id = UCLASS_TPM, - .name = "tpm", - .flags = DM_UC_FLAG_SEQ_ALIAS, + .id = UCLASS_TPM, + .name = "tpm", + .flags = DM_UC_FLAG_SEQ_ALIAS, #if CONFIG_IS_ENABLED(OF_REAL) - .post_bind = dm_scan_fdt_dev, + .post_bind = dm_scan_fdt_dev, #endif + .post_probe = tpm_uclass_post_probe, .per_device_auto = sizeof(struct tpm_chip_priv), };
The TPM device comes with the random number generator(RNG) functionality which is built into the TPM device. Add logic to add the RNG child device in the TPM uclass post probe callback. The RNG device can then be used to pass a set of random bytes to the linux kernel, need for address space randomisation through the EFI_RNG_PROTOCOL interface. Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> --- Changes since V6: None drivers/tpm/tpm-uclass.c | 37 +++++++++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-)