Re: [PATCH 02/15] hw/pci-host/q35: Fix double, contradicting .endianness assignment

[Philippe Mathieu-Daudé]

On 13/6/23 15:05, Igor Mammedov wrote:
> On Tue, 13 Jun 2023 13:07:17 +0200 (CEST)
> BALATON Zoltan <balaton@eik.bme.hu> wrote:
>
> > On Tue, 13 Jun 2023, Michael S. Tsirkin wrote:
> > > On Tue, Jun 13, 2023 at 09:46:53AM +0200, Bernhard Beschow wrote:
> > > > On Mon, Jun 12, 2023 at 3:01 PM Igor Mammedov <imammedo@redhat.com> wrote:
> > > >
> > > >      On Sun, 11 Jun 2023 12:33:59 +0200
> > > >      Bernhard Beschow <shentey@gmail.com> wrote:
> > > >   
> > > >     > Fixes the following clangd warning (-Winitializer-overrides):
> > > >     >
> > > >     >   q35.c:297:19: Initializer overrides prior initialization of this
> > > >      subobject
> > > >     >   q35.c:292:19: previous initialization is here
> > > >     >
> > > >     > Settle on native endian which causes the least overhead.
> > > >      indeed it doesn't matter which way we read all ones, so that should work.
> > > >      but does it really matter (I mean the overhead/what workload)?
> > > >      If not, I'd prefer explicit LE as it's now to be consistent
> > > >      the the rest of memops on Q35.

Meaning trying to optimize this single MR on big-endian is irrelevant :)

> > > > I got a comment from Michael about this in [1], so I've changed it. I don't
> > > > mind changing it either way.
> > > >
> > > > Best regards,
> > > > Bernhard
> > > >
> > > > [1] https://patchew.org/QEMU/20230214131441.101760-1-shentey@gmail.com/
> > > > 20230214131441.101760-3-shentey@gmail.com/#
> > > > 20230301164339-mutt-send-email-mst@kernel.org
> > >
> > > Hmm it's not terribly important, and the optimization is trivial,
> > > but yes people tend to copy code, good point. Maybe add a comment?
> > > /*
> > > * Note: don't copy this!  normally use DEVICE_LITTLE_ENDIAN. This only
> > > * works because we don't allow writes and always read all-ones.
> > > */
> >
> > Why don't you leave DEVICE_LITTLE_ENDIAN and remove DEVICE_NATIVE_ENDIAN
> > instead? If this only returns all 1s then it does not matter and also
> > DEVICE_LITTLE_ENDIAN was the last assignment so likely that was effective
> > so far anyway.
>
> I'm in favor of this as well,
> the 'optimization' and then piling comments on top to clarify confusion
> should be justified by usefulness of it (no perf numbers/usecase were present 
> so far).
> In absence of above, I'd prefer real hw behavior (LE in this case).