[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
top.
Regards,
Phil.
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [PATCH v4 03/28] cpu: Introduce cpu_virtio_is_big_endian(),
Philippe Mathieu-Daudé <=