2023-10-06 14:00:36

by Greg Kroah-Hartman

[permalink] [raw]
Subject: [PATCH] tee: make tee_class constant

Now that the driver core allows for struct class to be in read-only
memory, we should make all 'class' structures declared at build time
placing them into read-only memory, instead of having to be dynamically
allocated at runtime.

Cc: Jens Wiklander <[email protected]>
Cc: Sumit Garg <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/tee/tee_core.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
index 0eb342de0b00..5ddfd5d9ac7f 100644
--- a/drivers/tee/tee_core.c
+++ b/drivers/tee/tee_core.c
@@ -40,7 +40,10 @@ static const uuid_t tee_client_uuid_ns = UUID_INIT(0x58ac9ca0, 0x2086, 0x4683,
static DECLARE_BITMAP(dev_mask, TEE_NUM_DEVICES);
static DEFINE_SPINLOCK(driver_lock);

-static struct class *tee_class;
+static const struct class tee_class = {
+ .name = "tee",
+};
+
static dev_t tee_devt;

struct tee_context *teedev_open(struct tee_device *teedev)
@@ -919,7 +922,7 @@ struct tee_device *tee_device_alloc(const struct tee_desc *teedesc,
teedesc->flags & TEE_DESC_PRIVILEGED ? "priv" : "",
teedev->id - offs);

- teedev->dev.class = tee_class;
+ teedev->dev.class = &tee_class;
teedev->dev.release = tee_release_device;
teedev->dev.parent = dev;

@@ -1112,7 +1115,7 @@ tee_client_open_context(struct tee_context *start,
dev = &start->teedev->dev;

do {
- dev = class_find_device(tee_class, dev, &match_data, match_dev);
+ dev = class_find_device(&tee_class, dev, &match_data, match_dev);
if (!dev) {
ctx = ERR_PTR(-ENOENT);
break;
@@ -1226,10 +1229,10 @@ static int __init tee_init(void)
{
int rc;

- tee_class = class_create("tee");
- if (IS_ERR(tee_class)) {
+ rc = class_register(&tee_class);
+ if (rc) {
pr_err("couldn't create class\n");
- return PTR_ERR(tee_class);
+ return rc;
}

rc = alloc_chrdev_region(&tee_devt, 0, TEE_NUM_DEVICES, "tee");
@@ -1249,8 +1252,7 @@ static int __init tee_init(void)
out_unreg_chrdev:
unregister_chrdev_region(tee_devt, TEE_NUM_DEVICES);
out_unreg_class:
- class_destroy(tee_class);
- tee_class = NULL;
+ class_unregister(&tee_class);

return rc;
}
@@ -1259,8 +1261,7 @@ static void __exit tee_exit(void)
{
bus_unregister(&tee_bus_type);
unregister_chrdev_region(tee_devt, TEE_NUM_DEVICES);
- class_destroy(tee_class);
- tee_class = NULL;
+ class_unregister(&tee_class);
}

subsys_initcall(tee_init);
--
2.42.0


2023-10-09 05:36:54

by Sumit Garg

[permalink] [raw]
Subject: Re: [PATCH] tee: make tee_class constant

On Fri, 6 Oct 2023 at 19:30, Greg Kroah-Hartman
<[email protected]> wrote:
>
> Now that the driver core allows for struct class to be in read-only
> memory, we should make all 'class' structures declared at build time
> placing them into read-only memory, instead of having to be dynamically
> allocated at runtime.
>
> Cc: Jens Wiklander <[email protected]>
> Cc: Sumit Garg <[email protected]>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
> ---
> drivers/tee/tee_core.c | 21 +++++++++++----------
> 1 file changed, 11 insertions(+), 10 deletions(-)
>

Apart from nit below, feel free to add:

Reviewed-by: Sumit Garg <[email protected]>

> diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> index 0eb342de0b00..5ddfd5d9ac7f 100644
> --- a/drivers/tee/tee_core.c
> +++ b/drivers/tee/tee_core.c
> @@ -40,7 +40,10 @@ static const uuid_t tee_client_uuid_ns = UUID_INIT(0x58ac9ca0, 0x2086, 0x4683,
> static DECLARE_BITMAP(dev_mask, TEE_NUM_DEVICES);
> static DEFINE_SPINLOCK(driver_lock);
>
> -static struct class *tee_class;
> +static const struct class tee_class = {
> + .name = "tee",
> +};
> +
> static dev_t tee_devt;
>
> struct tee_context *teedev_open(struct tee_device *teedev)
> @@ -919,7 +922,7 @@ struct tee_device *tee_device_alloc(const struct tee_desc *teedesc,
> teedesc->flags & TEE_DESC_PRIVILEGED ? "priv" : "",
> teedev->id - offs);
>
> - teedev->dev.class = tee_class;
> + teedev->dev.class = &tee_class;
> teedev->dev.release = tee_release_device;
> teedev->dev.parent = dev;
>
> @@ -1112,7 +1115,7 @@ tee_client_open_context(struct tee_context *start,
> dev = &start->teedev->dev;
>
> do {
> - dev = class_find_device(tee_class, dev, &match_data, match_dev);
> + dev = class_find_device(&tee_class, dev, &match_data, match_dev);
> if (!dev) {
> ctx = ERR_PTR(-ENOENT);
> break;
> @@ -1226,10 +1229,10 @@ static int __init tee_init(void)
> {
> int rc;
>
> - tee_class = class_create("tee");
> - if (IS_ERR(tee_class)) {
> + rc = class_register(&tee_class);
> + if (rc) {
> pr_err("couldn't create class\n");

nit: this error message should be updated as well.

-Sumit

> - return PTR_ERR(tee_class);
> + return rc;
> }
>
> rc = alloc_chrdev_region(&tee_devt, 0, TEE_NUM_DEVICES, "tee");
> @@ -1249,8 +1252,7 @@ static int __init tee_init(void)
> out_unreg_chrdev:
> unregister_chrdev_region(tee_devt, TEE_NUM_DEVICES);
> out_unreg_class:
> - class_destroy(tee_class);
> - tee_class = NULL;
> + class_unregister(&tee_class);
>
> return rc;
> }
> @@ -1259,8 +1261,7 @@ static void __exit tee_exit(void)
> {
> bus_unregister(&tee_bus_type);
> unregister_chrdev_region(tee_devt, TEE_NUM_DEVICES);
> - class_destroy(tee_class);
> - tee_class = NULL;
> + class_unregister(&tee_class);
> }
>
> subsys_initcall(tee_init);
> --
> 2.42.0
>

2023-10-09 10:13:42

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] tee: make tee_class constant

On Mon, Oct 09, 2023 at 11:04:33AM +0530, Sumit Garg wrote:
> On Fri, 6 Oct 2023 at 19:30, Greg Kroah-Hartman
> <[email protected]> wrote:
> >
> > Now that the driver core allows for struct class to be in read-only
> > memory, we should make all 'class' structures declared at build time
> > placing them into read-only memory, instead of having to be dynamically
> > allocated at runtime.
> >
> > Cc: Jens Wiklander <[email protected]>
> > Cc: Sumit Garg <[email protected]>
> > Signed-off-by: Greg Kroah-Hartman <[email protected]>
> > ---
> > drivers/tee/tee_core.c | 21 +++++++++++----------
> > 1 file changed, 11 insertions(+), 10 deletions(-)
> >
>
> Apart from nit below, feel free to add:
>
> Reviewed-by: Sumit Garg <[email protected]>
>
> > diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> > index 0eb342de0b00..5ddfd5d9ac7f 100644
> > --- a/drivers/tee/tee_core.c
> > +++ b/drivers/tee/tee_core.c
> > @@ -40,7 +40,10 @@ static const uuid_t tee_client_uuid_ns = UUID_INIT(0x58ac9ca0, 0x2086, 0x4683,
> > static DECLARE_BITMAP(dev_mask, TEE_NUM_DEVICES);
> > static DEFINE_SPINLOCK(driver_lock);
> >
> > -static struct class *tee_class;
> > +static const struct class tee_class = {
> > + .name = "tee",
> > +};
> > +
> > static dev_t tee_devt;
> >
> > struct tee_context *teedev_open(struct tee_device *teedev)
> > @@ -919,7 +922,7 @@ struct tee_device *tee_device_alloc(const struct tee_desc *teedesc,
> > teedesc->flags & TEE_DESC_PRIVILEGED ? "priv" : "",
> > teedev->id - offs);
> >
> > - teedev->dev.class = tee_class;
> > + teedev->dev.class = &tee_class;
> > teedev->dev.release = tee_release_device;
> > teedev->dev.parent = dev;
> >
> > @@ -1112,7 +1115,7 @@ tee_client_open_context(struct tee_context *start,
> > dev = &start->teedev->dev;
> >
> > do {
> > - dev = class_find_device(tee_class, dev, &match_data, match_dev);
> > + dev = class_find_device(&tee_class, dev, &match_data, match_dev);
> > if (!dev) {
> > ctx = ERR_PTR(-ENOENT);
> > break;
> > @@ -1226,10 +1229,10 @@ static int __init tee_init(void)
> > {
> > int rc;
> >
> > - tee_class = class_create("tee");
> > - if (IS_ERR(tee_class)) {
> > + rc = class_register(&tee_class);
> > + if (rc) {
> > pr_err("couldn't create class\n");
>
> nit: this error message should be updated as well.

Ah, missed that, thanks! Will send a follow-up patch for that.

greg k-h