[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: |
Laurent Vivier |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH v8 03/10] dp8393x: manage big endian bus |
Date: |
Wed, 26 Jun 2019 12:11:56 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.0 |
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.
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 )
Thanks,
Laurent
- [Qemu-block] [PATCH v8 00/10] hw/m68k: add Apple Machintosh Quadra 800 machine, Laurent Vivier, 2019/06/19
- [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