lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAN5uoS9JqV7vGsf3630+AHOCOfsGj84xwDTUjxw4fUZ4coDqvw@mail.gmail.com>
Date:   Fri, 17 Mar 2023 18:07:27 +0100
From:   Etienne Carriere <etienne.carriere@...aro.org>
To:     Jens Wiklander <jens.wiklander@...aro.org>
Cc:     linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        Marc Zyngier <maz@...nel.org>,
        Sumit Garg <sumit.garg@...aro.org>
Subject: Re: [PATCH v3 3/3] optee: add enable/disable/set_wake handlers to
 optee irqs

On Thu, 16 Mar 2023 at 14:45, Jens Wiklander <jens.wiklander@...aro.org> wrote:
>
> On Wed, Mar 15, 2023 at 12:32 PM Etienne Carriere
> <etienne.carriere@...aro.org> wrote:
> >
> > Implements OP-TEE's It Notif PTA API to deactivate and activate an
> > interrupt notifier and to configure the wakeup capability
> > of that interrupt. These controls are useful for efficient power
> > management of the device when an interrupt controller resources is
> > hosted in OP-TEE world.
> >
> > When OP-TEE does not implement the It Notif PTA, the related handlers
> > simply return with success. If OP-TEE exposes the PTA services, they
> > are invoked on enable, disable and set_wake irqchip operations.
> >
> > Cc: Jens Wiklander <jens.wiklander@...aro.org>
> > Cc: Marc Zyngier <maz@...nel.org>
> > Cc: Sumit Garg <sumit.garg@...aro.org>
> >
> > Signed-off-by: Etienne Carriere <etienne.carriere@...aro.org>
> > ---
> > Changes since v3:
> > - Renamed it_notif/IT_NOTIF to itr_notif/ITR_NOTIF.
> >
> > Changes since v2:
> > - Patch added in v2 series for power-up/down and wakeup configuration
> >   of the irq chip.
> >
> > Patch not present in v1
> > - Patch added for power-up/down and wakeup configuration of the irq chip.
> > ---
> >  drivers/tee/optee/optee_private.h |   2 +
> >  drivers/tee/optee/smc_abi.c       | 157 ++++++++++++++++++++++++++++++
> >  2 files changed, 159 insertions(+)
> >
> > diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
> > index f467409e02e9..4b4002d81315 100644
> > --- a/drivers/tee/optee/optee_private.h
> > +++ b/drivers/tee/optee/optee_private.h
> > @@ -166,6 +166,7 @@ struct optee_ops {
> >   * @scan_bus_done      flag if device registation was already done.
> >   * @scan_bus_wq                workqueue to scan optee bus and register optee drivers
> >   * @scan_bus_work      workq to scan optee bus and register optee drivers
> > + * @notif_itr_pta_ctx   TEE context for invoking interrupt configuration services
> >   */
> >  struct optee {
> >         struct tee_device *supp_teedev;
> > @@ -185,6 +186,7 @@ struct optee {
> >         bool   scan_bus_done;
> >         struct workqueue_struct *scan_bus_wq;
> >         struct work_struct scan_bus_work;
> > +       struct tee_context *notif_itr_pta_ctx;
>
> Please use the other *ctx above instead.

I will discard this 3th patch and replace this PTA services with SMC
function IDs as suggested.

BR,
Etienne

>
> Thanks,
> Jens
>
> >  };
> >
> >  struct optee_session {
> > diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c
> > index 6e6c6979abfd..046aa242b2ef 100644
> > --- a/drivers/tee/optee/smc_abi.c
> > +++ b/drivers/tee/optee/smc_abi.c
> > @@ -52,6 +52,43 @@
> >   */
> >  #define OPTEE_MIN_STATIC_POOL_ALIGN    9 /* 512 bytes aligned */
> >
> > +/*
> > + * Interrupt notification can be configured using Notif-IT PTA services.
> > + * Below are the PTA UUID and its API commands.
> > + */
> > +#define PTA_ITR_NOTIF_UUID \
> > +       UUID_INIT(0x4461e5c7, 0xb523, 0x4b73, 0xac, 0xed, 0x75, 0xad, \
> > +                 0x2b, 0x9b, 0x59, 0xa1)
> > +
> > +/*
> > + * PTA_ITR_NOTIF_ACTIVATE_DETECTION - Enable a interrupt notification
> > + *
> > + * [in]  params[0].value.a     Interrupt ID
> > + */
> > +#define PTA_ITR_NOTIF_ACTIVATE_DETECTION       0
> > +
> > +/*
> > + * PTA_ITR_NOTIF_DEACTIVATE_DETECTION - Disable a interrupt notification
> > + *
> > + * [in]  params[0].value.a     Interrupt ID
> > + */
> > +#define PTA_ITR_NOTIF_DEACTIVATE_DETECTION     1
> > +
> > +/*
> > + * PTA_ITR_NOTIF_ENABLE_WAKEUP_SOURCE - Enable an interrupt wakeup source
> > + *
> > + * [in]  params[0].value.a     Interrupt ID
> > + */
> > +#define PTA_ITR_NOTIF_ENABLE_WAKEUP_SOURCE     2
> > +
> > +/*
> > + * PTA_ITR_NOTIF_ENABLE_WAKEUP_SOURCE - Disable an interrupt wakeup source
> > + *
> > + * [in]  params[0].value.a     Interrupt ID
> > + */
> > +#define PTA_ITR_NOTIF_DISABLE_WAKEUP_SOURCE    3
> > +
> > +
> >  /*
> >   * 1. Convert between struct tee_param and struct optee_msg_param
> >   *
> > @@ -977,6 +1014,92 @@ static int optee_smc_stop_async_notif(struct tee_context *ctx)
> >   * 5. Asynchronous notification
> >   */
> >
> > +static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data)
> > +{
> > +       return ver->impl_id == TEE_IMPL_ID_OPTEE;
> > +}
> > +
> > +static void init_optee_pta_context(struct optee *optee)
> > +{
> > +       struct tee_context *ctx = NULL;
> > +       const uuid_t pta_uuid = PTA_ITR_NOTIF_UUID;
> > +       struct tee_ioctl_open_session_arg sess_arg;
> > +       int ret;
> > +
> > +       ctx = tee_client_open_context(NULL, optee_ctx_match, NULL, NULL);
> > +       if (IS_ERR(ctx))
> > +               return;
> > +
> > +       memset(&sess_arg, 0, sizeof(sess_arg));
> > +       export_uuid(sess_arg.uuid, &pta_uuid);
> > +       sess_arg.clnt_login = TEE_IOCTL_LOGIN_REE_KERNEL;
> > +
> > +       ret = tee_client_open_session(ctx, &sess_arg, NULL);
> > +       if ((ret < 0) || (sess_arg.ret != 0)) {
> > +               pr_err("Can't open ITR_NOTIF PTA session: %#x\n", sess_arg.ret);
> > +               tee_client_close_context(ctx);
> > +               return;
> > +       }
> > +
> > +       tee_client_close_session(ctx, sess_arg.session);
> > +
> > +       optee->notif_itr_pta_ctx = ctx;
> > +}
> > +
> > +static void release_optee_pta_context(struct optee *optee)
> > +{
> > +       if (optee->notif_itr_pta_ctx) {
> > +               tee_client_close_context(optee->notif_itr_pta_ctx);
> > +               optee->notif_itr_pta_ctx = NULL;
> > +       }
> > +}
> > +
> > +static int invoke_optee_pta(struct optee *optee, unsigned int command,
> > +                           unsigned int irq_id)
> > +{
> > +       const uuid_t pta_uuid = PTA_ITR_NOTIF_UUID;
> > +       struct tee_ioctl_open_session_arg sess_arg;
> > +       struct tee_ioctl_invoke_arg inv_arg;
> > +       struct tee_param param[1];
> > +       int ret;
> > +
> > +       if (!optee->notif_itr_pta_ctx)
> > +               return -ENOENT;
> > +
> > +       memset(&sess_arg, 0, sizeof(sess_arg));
> > +       export_uuid(sess_arg.uuid, &pta_uuid);
> > +       sess_arg.clnt_login = TEE_IOCTL_LOGIN_REE_KERNEL;
> > +
> > +       ret = tee_client_open_session(optee->notif_itr_pta_ctx, &sess_arg, NULL);
> > +       if ((ret < 0) || (sess_arg.ret != 0)) {
> > +               pr_err("tee_client_open_session failed, err: %#x\n", sess_arg.ret);
> > +               if (!ret)
> > +                       ret = -EINVAL;
> > +               return ret;
> > +       }
> > +
> > +       memset(&inv_arg, 0, sizeof(inv_arg));
> > +       inv_arg.session = sess_arg.session;
> > +       inv_arg.func = command;
> > +       inv_arg.num_params = 1;
> > +
> > +       memset(&param, 0, sizeof(param));
> > +       param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT;
> > +       param[0].u.value.a = irq_id;
> > +
> > +       ret = tee_client_invoke_func(optee->notif_itr_pta_ctx, &inv_arg, param);
> > +       if ((ret < 0) || (inv_arg.ret != 0)) {
> > +               pr_err("tee_client_invoke_func failed, ret: %d, err: %#x\n",
> > +                      ret, inv_arg.ret);
> > +               if (!ret)
> > +                       ret = -EINVAL;
> > +       }
> > +
> > +       tee_client_close_session(optee->notif_itr_pta_ctx, sess_arg.session);
> > +
> > +       return ret;
> > +}
> > +
> >  static void optee_itr_notif_mask(struct irq_data *d)
> >  {
> >         struct optee *optee = d->domain->host_data;
> > @@ -995,10 +1118,40 @@ static void optee_itr_notif_unmask(struct irq_data *d)
> >                              0, 0, 0, 0, 0, &res);
> >  }
> >
> > +static void optee_itr_notif_disable(struct irq_data *d)
> > +{
> > +       struct optee *optee = d->domain->host_data;
> > +
> > +       (void)invoke_optee_pta(optee, PTA_ITR_NOTIF_DEACTIVATE_DETECTION, d->hwirq);
> > +}
> > +
> > +static void optee_itr_notif_enable(struct irq_data *d)
> > +{
> > +       struct optee *optee = d->domain->host_data;
> > +
> > +       (void)invoke_optee_pta(optee, PTA_ITR_NOTIF_ACTIVATE_DETECTION, d->hwirq);
> > +}
> > +
> > +static int optee_itr_notif_set_wake(struct irq_data *d, unsigned int on)
> > +{
> > +       struct optee *optee = d->domain->host_data;
> > +       u32 command;
> > +
> > +       if (on)
> > +               command = PTA_ITR_NOTIF_ENABLE_WAKEUP_SOURCE;
> > +       else
> > +               command = PTA_ITR_NOTIF_DISABLE_WAKEUP_SOURCE;
> > +
> > +       return invoke_optee_pta(optee, command, d->hwirq);
> > +}
> > +
> >  static struct irq_chip optee_irq_chip = {
> >         .name = "optee-it",
> >         .irq_mask = optee_itr_notif_mask,
> >         .irq_unmask = optee_itr_notif_unmask,
> > +       .irq_disable = optee_itr_notif_disable,
> > +       .irq_enable = optee_itr_notif_enable,
> > +       .irq_set_wake = optee_itr_notif_set_wake,
> >  };
> >
> >  static int optee_itr_alloc(struct irq_domain *d, unsigned int virq,
> > @@ -1459,6 +1612,7 @@ static int optee_smc_remove(struct platform_device *pdev)
> >                 optee_disable_shm_cache(optee);
> >
> >         optee_smc_notif_uninit_irq(optee);
> > +       release_optee_pta_context(optee);
> >
> >         optee_remove_common(optee);
> >
> > @@ -1646,6 +1800,8 @@ static int optee_probe(struct platform_device *pdev)
> >                                 irq_dispose_mapping(irq);
> >                                 goto err_notif_uninit;
> >                         }
> > +
> > +                       init_optee_pta_context(optee);
> >                 }
> >
> >                 enable_async_notif(optee->smc.invoke_fn);
> > @@ -1683,6 +1839,7 @@ static int optee_probe(struct platform_device *pdev)
> >                 optee_disable_shm_cache(optee);
> >         optee_smc_notif_uninit_irq(optee);
> >         optee_unregister_devices();
> > +       release_optee_pta_context(optee);
> >  err_notif_uninit:
> >         optee_notif_uninit(optee);
> >  err_close_ctx:
> > --
> > 2.25.1
> >

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ