linux-kernel - Re: [PATCH 3/3] soc: xilinx: zynqmp: Add firmware interface

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: <3809b6a0-1cb9-37af-a524-8bc1986268ec@xilinx.com>
Date:   Wed, 16 Aug 2017 16:34:44 +0200
From:   Michal Simek <michal.simek@...inx.com>
To:     Arnd Bergmann <arnd@...db.de>,
        Michal Simek <michal.simek@...inx.com>
CC:     Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Sören Brinkmann <soren.brinkmann@...inx.com>,
        Lucas Stach <l.stach@...gutronix.de>,
        Michal Simek <monstr@...str.eu>, yangbo lu <yangbo.lu@....com>,
        Andreas Färber <afaerber@...e.de>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Alexandre Belloni <alexandre.belloni@...e-electrons.com>,
        Baoyou Xie <baoyou.xie@...aro.org>,
        Shawn Guo <shawnguo@...nel.org>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Nicolas Ferre <nicolas.ferre@...rochip.com>,
        Simon Horman <horms+renesas@...ge.net.au>
Subject: Re: [PATCH 3/3] soc: xilinx: zynqmp: Add firmware interface

On 16.8.2017 16:00, Michal Simek wrote:
> On 16.8.2017 14:41, Arnd Bergmann wrote:
>> On Wed, Aug 16, 2017 at 1:51 PM, Michal Simek <michal.simek@...inx.com> wrote:
>>> On 14.8.2017 17:06, Arnd Bergmann wrote:
>>>> On Fri, Aug 4, 2017 at 3:45 PM, Michal Simek <michal.simek@...inx.com> wrote:
>>>>> +static noinline int do_fw_call_smc(u64 arg0, u64 arg1, u64 arg2,
>>>>> +                                  u32 *ret_payload)
>>>>> +{
>>>>> +       struct arm_smccc_res res;
>>>>> +
>>>>> +       arm_smccc_smc(arg0, arg1, arg2, 0, 0, 0, 0, 0, &res);
>>>>> +
>>>>> +       if (ret_payload) {
>>>>> +               ret_payload[0] = (u32)res.a0;
>>>>> +               ret_payload[1] = (u32)(res.a0 >> 32);
>>>>> +               ret_payload[2] = (u32)res.a1;
>>>>> +               ret_payload[3] = (u32)(res.a1 >> 32);
>>>>> +               ret_payload[4] = (u32)res.a2;
>>>>> +       }
>>>>> +
>>>>> +       return zynqmp_pm_ret_code((enum pm_ret_status)res.a0);
>>>>> +}
>>>>
>>>> It looks like you forgot to add the cpu_to_le32/le32_to_cpu conversions
>>>> here to make this work on big-endian kernels.
>>>
>>> We have discussed support for big endian kernels in past and discussion
>>> end up with that there is no customer for this. It means I can change
>>> this but none will use this.
>>
>> Ok, thanks. As a general rule, I prefer kernel code to be written
>> in a portable way even when you assume that is not necessary.
>>
>> Besides the obvious problem of users that end up wanting to do
>> something you don't expect, there is the more general issue of
>> copying code into another driver that may need to be more portable.
> 
> 
> I fully understand this. Let me play with it but I expect there will be
> different issues then just this.
> 

What do you think?
		ret_payload[0] = lower_32_bits(le64_to_cpu(res.a0));
		ret_payload[1] = upper_32_bits(le64_to_cpu(res.a0));
		ret_payload[2] = lower_32_bits(le64_to_cpu(res.a1));
		ret_payload[3] = upper_32_bits(le64_to_cpu(res.a1));
		ret_payload[4] = lower_32_bits(le64_to_cpu(res.a2));

There should be probably also change in invoke_pm_fn to do conversion
from cpu to le64.

int invoke_pm_fn(u32 pm_api_id, u32 arg0, u32 arg1, u32 arg2, u32 arg3,
		 u32 *ret_payload)
{
	/*
	 * Added SIP service call Function Identifier
	 * Make sure to stay in x0 register
	 */
	u64 smc_arg[4];

	smc_arg[0] = cpu_to_le64(PM_SIP_SVC | pm_api_id);
	smc_arg[1] = cpu_to_le64(((u64)arg1 << 32) | arg0);
	smc_arg[2] = cpu_to_le64(((u64)arg3 << 32) | arg2);

	return do_fw_call(smc_arg[0], smc_arg[1], smc_arg[2], ret_payload);
}

This is not tested on BE just on LE.

Thanks,
Michal