Hi Michael, On 3/4/21 8:51 AM, Greg Kurz wrote: > On Wed, 3 Mar 2021 17:08:32 -0500 > "Michael S. Tsirkin" <m...@redhat.com> wrote: > >> On Wed, Mar 03, 2021 at 10:46:43PM +0100, Philippe Mathieu-Daudé wrote: >>> Introduce the cpu_virtio_is_big_endian() generic helper to avoid >>> calling CPUClass internal virtio_is_big_endian() one. >>> >>> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org> >> >> Using virtio in the name here probably because virtio wants this? >> That doesn't sound like a good naming strategy, name should >> tell us what function does not how it's used. >> > > I tend to agree but there was a consensus to deliberately put > virtio in the name when this was first introduced, so that > nobody else ever try to use it, as recorded in the commit log. > > commit bf7663c4bd8f8f619d6dbb5780025d92ace250a8 > Author: Greg Kurz <gr...@kaod.org> > Date: Tue Jun 24 19:33:21 2014 +0200 > > cpu: introduce CPUClass::virtio_is_big_endian() > > If we want to support targets that can change endianness (modern PPC and > ARM for the moment), we need to add a per-CPU class method to be called > from the virtio code. The virtio_ prefix in the name is a hint for people > to avoid misusage (aka. anywhere but from the virtio code). > > The default behaviour is to return the compile-time default target > endianness. > > Suggested-by: Peter Maydell <peter.mayd...@linaro.org> > Signed-off-by: Greg Kurz <gk...@linux.vnet.ibm.com> > Reviewed-by: Michael S. Tsirkin <m...@redhat.com> > Signed-off-by: Michael S. Tsirkin <m...@redhat.com> > > Is there something new on this front ? I'm not convinced that anything > but legacy virtio en POWER (or any other target that can change endian > at runtime) needs this. The next step I see for this is_big_endian() > stuff is deprecation and removal. In the meantime, I think we should > keep the virtio wording to prevent additional users for this.
On 3/3/21 11:15 PM, Michael S. Tsirkin wrote: > On a more concrete proposal, how about using this change > to rename the virtio_is_big_endian field to guest_is_big_endian(), > and put the wrapper somewhere in a virtio header instead? Due to Greg comment, I'll keep cpu_virtio_is_big_endian() in the v5 respin. This doesn't seem a real blocker for the rest of the changeset. We can settle the name and send a patch on top. Regards, Phil.
