[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v4 03/28] cpu: Introduce cpu_virtio_is_big_endian()

From: Philippe Mathieu-Daudé
Subject: Re: [PATCH v4 03/28] cpu: Introduce cpu_virtio_is_big_endian()
Date: Thu, 22 Apr 2021 12:33:30 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1

Hi Michael,

On 3/4/21 8:51 AM, Greg Kurz wrote:
> On Wed, 3 Mar 2021 17:08:32 -0500
> "Michael S. Tsirkin" <mst@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é <f4bug@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 <groug@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.maydell@linaro.org>
>     Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
>     Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>     Signed-off-by: Michael S. Tsirkin <mst@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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]