[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH v8 03/10] dp8393x: manage big endia
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH v8 03/10] dp8393x: manage big endian bus |
Date: |
Wed, 26 Jun 2019 19:49:51 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.0 |
On 6/26/19 12:11 PM, Laurent Vivier wrote:
> Le 26/06/2019 à 10:57, Philippe Mathieu-Daudé a écrit :
>> On 6/25/19 7:09 PM, Laurent Vivier wrote:
>>> Le 25/06/2019 à 17:57, Philippe Mathieu-Daudé a écrit :
>>>> On 6/24/19 10:07 PM, Laurent Vivier wrote:
>>>>> Hi,
>>>>>
>>>>> Jason, Can I have an Acked-by from you (as network devices maintainer)?
>>>>
>>>> Hmm something seems odd here indeed...
>>>>
>>>> What a stable model! This file has no logical modification since its
>>>> introduction, a65f56eeba "Implement sonic netcard (MIPS Jazz)"
>>>>
>>>> Here we had:
>>>>
>>>> static void dp8393x_writeb(void *opaque, hwaddr addr, uint32_t val)
>>>> {
>>>> uint16_t old_val = dp8393x_readw(opaque, addr & ~0x1);
>>>>
>>>> switch (addr & 3) {
>>>> case 0:
>>>> val = val | (old_val & 0xff00);
>>>> break;
>>>> case 1:
>>>> val = (val << 8) | (old_val & 0x00ff);
>>>> break;
>>>> }
>>>> dp8393x_writew(opaque, addr & ~0x1, val);
>>>> }
>>>>
>>>> So we had 16-bit endian shifting there.
>>>>
>>>> And few lines below:
>>>>
>>>> /* XXX: Check byte ordering */
>>>> ...
>>>> /* Calculate the ethernet checksum */
>>>> #ifdef SONIC_CALCULATE_RXCRC
>>>> checksum = cpu_to_le32(crc32(0, buf, rx_len));
>>>> #else
>>>> checksum = 0;
>>>> #endif
>>>>
>>>> After various housekeeping, we get:
>>>>
>>>> 84689cbb97 "net/dp8393x: do not use old_mmio accesses"
>>>>
>>>> The MIPS Jazz is known to run in both endianess, but I haven't checked
>>>> if at that time both were available.
>>>>
>>>> Have you tried this patch?
>>>>
>>>> -- >8 --
>>>> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
>>>> index bdb0b3b2c2..646e11206f 100644
>>>> @@ -651,7 +651,7 @@ static const MemoryRegionOps dp8393x_ops = {
>>>> .write = dp8393x_write,
>>>> .impl.min_access_size = 2,
>>>> .impl.max_access_size = 2,
>>>> - .endianness = DEVICE_NATIVE_ENDIAN,
>>>> + .endianness = DEVICE_LITTLE_ENDIAN,
>>>> };
>>>> ---
>>>>
>>>> (but then mips64-softmmu Jazz would have networking broken).
>>>>
>>>
>>> I doesn't help, the endianness is a MemoryRegion property (see
>>> memory_region_wrong_endianness()) so it is used when the CPU writes to
>>> the device MMIO, not when the device accesses the other memory.
>>> In this case, it reads from system_memory. Perhaps we can create the
>>> address_space with a system_memory in big endian mode?
>>
>> Ah I missed that...
>>
>> What about not using address_space_rw(data) but directly use
>> address_space_lduw_le() and address_space_stw_le() instead?
>>
>
> It's more complicated than that, because access size depends on a
> register value:
>
> static uint16_t dp8393x_get(dp8393xState *s, int width, uint16_t *base,
> int offset)
> {
> uint16_t val;
>
> if (s->big_endian) {
> val = be16_to_cpu(base[offset * width + width - 1]);
> } else {
> val = le16_to_cpu(base[offset * width]);
> }
> return val;
> }
>
> and width is:
>
> width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1;
>
> So in the end we always need the big_endian flag to know how to read the
> memory. I think it's simpler to read/write the memory (like a real DMA
> access), and then to swap data internally.
Fair enough. My R-b tag stands anyway :)
> Moreover, the big-endian/little-endian is a real feature of the
> controller (see 1.3 DATA WIDTH AND BYTE ORDERING,
> http://pccomponents.com/datasheets/NSC83932.PDF )
Can you (or the maintainer taking this series) amend this information to
your commit?
Thanks for the info provided in this thread,
Phil.
- [Qemu-block] [PATCH v8 07/10] hw/m68k: add Nubus support, (continued)
- [Qemu-block] [PATCH v8 07/10] hw/m68k: add Nubus support, Laurent Vivier, 2019/06/19
- [Qemu-block] [PATCH v8 08/10] hw/m68k: add Nubus support for macfb video card, Laurent Vivier, 2019/06/19
- [Qemu-block] [PATCH v8 06/10] hw/m68k: add macfb video card, Laurent Vivier, 2019/06/19
- [Qemu-block] [PATCH v8 05/10] hw/m68k: implement ADB bus support for via, Laurent Vivier, 2019/06/19
- [Qemu-block] [PATCH v8 03/10] dp8393x: manage big endian bus, Laurent Vivier, 2019/06/19
[Qemu-block] [PATCH v8 09/10] hw/m68k: add a dummy SWIM floppy controller, Laurent Vivier, 2019/06/19
[Qemu-block] [PATCH v8 04/10] hw/m68k: add via support, Laurent Vivier, 2019/06/19
[Qemu-block] [PATCH v8 10/10] hw/m68k: define Macintosh Quadra 800, Laurent Vivier, 2019/06/19
[Qemu-block] [PATCH v8 02/10] esp: add pseudo-DMA as used by Macintosh, Laurent Vivier, 2019/06/19
[Qemu-block] [PATCH v8 01/10] escc: introduce a selector for the register bit, Laurent Vivier, 2019/06/19
Re: [Qemu-block] [Qemu-devel] [PATCH v8 00/10] hw/m68k: add Apple Machintosh Quadra 800 machine, no-reply, 2019/06/19
Re: [Qemu-block] [Qemu-devel] [PATCH v8 00/10] hw/m68k: add Apple Machintosh Quadra 800 machine, Mark Cave-Ayland, 2019/06/22