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

["Michael S. Tsirkin" <mst@redhat.com>]

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


> 
>     >
>     > 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 */
> 
>