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

[BALATON Zoltan]

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.
> >
> >
> > 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.

Regards,
BALATON Zoltan

> >    >
> >    > Fixes: bafc90bdc594 ("q35: implement TSEG")
> >    > Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> >    > ---
> >    >  hw/pci-host/q35.c | 1 -
> >    >  1 file changed, 1 deletion(-)
> >    >
> >    > diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> >    > index fd18920e7f..859c197f25 100644
> >    > --- a/hw/pci-host/q35.c
> >    > +++ b/hw/pci-host/q35.c
> >    > @@ -290,7 +290,6 @@ static const MemoryRegionOps blackhole_ops = {
> >    >      .valid.max_access_size = 4,
> >    >      .impl.min_access_size = 4,
> >    >      .impl.max_access_size = 4,
> >    > -    .endianness = DEVICE_LITTLE_ENDIAN,
> >    >  };
> >    > 
> >    >  /* PCIe MMCFG */