linux-kernel - Re: [PATCH v2 06/18] firmware: qcom_scm-64: Move svc/cmd/owner into qcom_scm

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <5dcf376f.1c69fb81.6fa51.4fa7@mx.google.com>
Date:   Fri, 15 Nov 2019 15:40:30 -0800
From:   Stephen Boyd <swboyd@...omium.org>
To:     Elliot Berman <eberman@...eaurora.org>, agross@...nel.org,
        bjorn.andersson@...aro.org, saiprakash.ranjan@...eaurora.org
Cc:     Elliot Berman <eberman@...eaurora.org>, tsoni@...eaurora.org,
        sidgup@...eaurora.org, psodagud@...eaurora.org,
        linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 06/18] firmware: qcom_scm-64: Move svc/cmd/owner into qcom_scm_desc

Quoting Elliot Berman (2019-11-12 13:22:42)
> diff --git a/drivers/firmware/qcom_scm-64.c b/drivers/firmware/qcom_scm-64.c
> index 7de6022..e81fb6e 100644
> --- a/drivers/firmware/qcom_scm-64.c
> +++ b/drivers/firmware/qcom_scm-64.c
> @@ -46,8 +46,11 @@ enum qcom_scm_arg_types {
>   * @res:       The values returned by the secure syscall
>   */
>  struct qcom_scm_desc {
> +       u32 svc;
> +       u32 cmd;
>         u32 arginfo;
>         u64 args[MAX_QCOM_SCM_ARGS];
> +       u32 owner;

Owner is always the same. Why move it to the qcom_scm_desc structure? I
can understand svc and cmd but there's no justification for this struct
member.

>  };
>  
>  static u64 qcom_smccc_convention = -1;
[...]
> @@ -542,26 +584,32 @@ int __qcom_scm_io_readl(struct device *dev, phys_addr_t addr,
>  
>  int __qcom_scm_io_writel(struct device *dev, phys_addr_t addr, unsigned int val)
>  {
> -       struct qcom_scm_desc desc = {0};
> +       struct qcom_scm_desc desc = {
> +               .svc = QCOM_SCM_SVC_IO,
> +               .cmd = QCOM_SCM_IO_WRITE,
> +               .owner = ARM_SMCCC_OWNER_SIP,
> +       };
>         struct arm_smccc_res res;
>  
>         desc.args[0] = addr;
>         desc.args[1] = val;
>         desc.arginfo = QCOM_SCM_ARGS(2);

The style is weird here. Half of the struct is initialized during
declaration and the arguments are set here with individual statements.
Can we have one way or the other and not both?

>  
> -       return qcom_scm_call(dev, QCOM_SCM_SVC_IO, QCOM_SCM_IO_WRITE,
> -                            &desc, &res);
> +       return qcom_scm_call(dev, &desc, &res);
>  }
>