Andrew Donnellan <ajd@xxxxxxxxxxxxx> 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
