Re: [PATCH v6 24/26] powerpc/pseries: Implement secvars for dynamic secure boot

[Michael Ellerman <michaele@au1.ibm.com>]

Andrew Donnellan <ajd@linux.ibm.com> writes:
> On Mon, 2023-02-13 at 22:32 +1100, Michael Ellerman wrote:
>> > > +       memcpy(&flags, data, sizeof(flags));
>> > 
>> > conversion from bytestream to integer: I think in this case it
>> > would be better to use
>> > 
>> > flags = cpu_to_be64p((__u64*)data);
>> > 
>> > so that the flags always in hypervisor/big endian format
>> 
>> I don't think it's correct to byte swap the flags here. They must be
>> in big endian format, but that's up to the caller.
>
> That was what I initially thought, until I went and tested it properly
> and found it was indeed broken (at least in our qemu environment, this
> is slightly tricky for me to test right now on real hardware with real
> PowerVM) depending on kernel endianness.
>
> - Userspace writes the flags into the buffer in BE order
>
> - The first 8 bytes of the buffer are memcpy()ed, in BE order, into
> flags (a u64)
>
> - plpar_hcall9() is called with flags as an argument, loaded into r9
>
> - r9 is moved to r8 before jumping into the hypervisor
>
> On a BE system, this works fine. On an LE system, this results in the
> bytes in the flags variable being loaded into the register in LE order,
> so the conversion is necessary.

Ah yep of course. So although the flags are written by userspace as part
of the data as a stream of bytes, they're passed to the HV via a
register.

I've had this patch in next for a few days and don't want to rebase it.
So can you send a follow-up patch to fix the flags endianess, with a
nice changelog and comment :)

cheers