Andreas Färber <afaer...@suse.de> writes: > Am 08.08.2013 17:40, schrieb Anthony Liguori: >> Andreas Färber <afaer...@suse.de> writes: >>> Am 08.08.2013 15:31, schrieb Anthony Liguori: >>>> We have a mechanism to do weak functions via stubs/. I think it would >>>> be better to do cpu_get_byteswap() as a stub function and then overload >>>> it in the ppc64 code. >>> >>> If this as your name indicates is a per-CPU function then it should go >>> into CPUClass. Interesting question is, what is virtio supposed to do if >>> we have two ppc CPUs, one is Big Endian, the other is Little Endian. >> >> PPC64 is big endian. AFAIK, there is no such thing as a little endian >> PPC64 processor. >> >> This is just a processor mode where loads/stores are byte lane swapped. >> Hence the name 'cpu_get_byteswap'. It's just asking whether the >> load/stores are being swapped or not. > > Exactly, just read it as "is in ... Endian mode". On the CPUs I am more > familiar with (e.g., 970), this used to be controlled via an MSR bit, > which as CPUPPCState::msr exists per CPUState. I did not check on real > hardware, but from the QEMU code this would allow for the mixed-endian > scenario described above. > >> At least for PPC64, it's not possible to enable/disable byte lane >> swapping for individual CPUs. It's done through a system-wide hcall. > > What is offending me is only the following: If we name it > cpu_get_byteswap() as proposed by you, then its first argument should be > a CPUState *cpu. Its value would be read from the derived type's state, > such as the MSR bit in the code path that you wanted duplicated. The > function implementing that register-reading would be a hook in CPUClass, > with a default implementation in qom/cpu.c rather than a fallback in > stubs/. To access CPUClass, CPUState cannot be NULL, as brought up by > Stefano for cpu_do_unassigned_access(); not following that pattern > prevents mixing CPU architectures, which my large refactorings have > partially been about. Cf. my guest-memory-dump refactoring. > > If it is just some random global value, then please don't call it > cpu_*(). Since sPAPR is not a target of its own, I don't see how/where > you want to implement that hcall query as per-target function either, > that might rather call for a QEMUMachine hook? > > I don't care or argue about byte lanes here, I am just trying to keep > API design consistent and not regressing on the way to heterogeneous > emulation.
That's a lot of replumbing and indirect function calls for a fairly obscure case. We certainly don't have a nice CPUState lying around in virtio at the moment, for example. I can try to plumb this in if there's consensus, but I suspect it's making the job 10x harder. (The next logical step would be for st* and ld* to take the cpu to query its endianness, Anthony's weird ideas notwithstanding). Cheers, Rusty.
