[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [PATCH 2/2] Add Enhanced Three-Speed Etherne
From: |
Fabien Chouteau |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [PATCH 2/2] Add Enhanced Three-Speed Ethernet Controller (eTSEC) |
Date: |
Wed, 17 Jul 2013 12:17:06 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/20130623 Thunderbird/17.0.7 |
On 07/16/2013 07:50 PM, Scott Wood wrote:
> On 07/16/2013 10:28:28 AM, Fabien Chouteau wrote:
>> On 07/16/2013 04:06 AM, Scott Wood wrote:
>> > On 07/10/2013 12:10:02 PM, Fabien Chouteau wrote:
>> >> +obj-$(CONFIG_ETSEC) += etsec.o etsec_registers.o etsec_rings.o
>> >> etsec_miim.o
>> >
>> > Maybe an fsl_etsec directory?
>> >
>>
>> Is it really necessary?
>
> Not strictly necessary, but it would help keep things tidy.
>
I will try :)
>> >> +DeviceState *etsec_create(hwaddr base,
>> >> + MemoryRegion * mr,
>> >> + NICInfo * nd,
>> >> + qemu_irq tx_irq,
>> >> + qemu_irq rx_irq,
>> >> + qemu_irq err_irq)
>> >>
>> > Do you plan to update hw/ppc/e500.c (or maybe some other platform?) to
>> > call this?
>> >
>>
>> No I don't, not for the moment. I use it in one of our machine (that is not
>> in mainstream).
>
> Someone should, so we can test this without your out-of-tree code.
>
>> e500.c would require PCI support I think.
>
> e500.c has PCI support, but how is that relevant to eTSEC?
I guess it's not. We would have to remove:
if (pci_bus) {
/* Register network interfaces. */
for (i = 0; i < nb_nics; i++) {
pci_nic_init_nofail(&nd_table[i], pci_bus, "virtio", NULL);
}
}
>
>> >> + if (*size == etsec->rx_padding) {
>> >> + /* The remaining bytes are for padding which is not actually
>> >> allocated
>> >> + in the buffer */
>> >> +
>> >> + rem = MIN(etsec->regs[MRBLR].value - bd->length,
>> >> etsec->rx_padding);
>> >> +
>> >> + if (rem > 0) {
>> >> + memset(padd, 0x0, sizeof(padd));
>> >> + etsec->rx_padding -= rem;
>> >> + *size -= rem;
>> >> + bd->length += rem;
>> >> + cpu_physical_memory_write(bufptr, padd, rem);
>> >> + }
>> >> + }
>> >
>> > What if *size > 0 && *size < etsec->rx_padding?
>>
>> I don't think it's possible...
>
> Maybe throw in an assertion, then?
>
> I can see how it might not be possible if rx_padding is being used for
> padding a short frame, since MRBLR must be a multiple of 64, but what if it's
> 4 bytes for CRC?
>
Can you explain a possible error scenario?
>> >> +static void rx_init_frame(eTSEC *etsec, const uint8_t *buf, size_t size)
>> >> +{
>> >> + uint32_t fcb_size = 0;
>> >> + uint8_t prsdep = (etsec->regs[RCTRL].value >> RCTRL_PRSDEP_OFFSET)
>> >> + & RCTRL_PRSDEP_MASK;
>> >> +
>> >> + if (prsdep != 0) {
>> >> + /* Prepend FCB */
>> >> + fcb_size = 8 + 2; /* FCB size + align */
>> >> + /* I can't find this 2 bytes alignement in fsl documentation but
>> >> VxWorks
>> >> + expects them */
>> >
>> > Could these 2 bytes be from RCTRL[PAD], which you seem to ignore?
>>
>> Did you mean RCTRL[PAL]? It could be that.
>
> Yes, sorry.
>
Thanks for pointing it ;)
>> >> + etsec->rx_padding = 4;
>>
>> CRC.
>>
>> >> + if (size < 60) {
>> >> + etsec->rx_padding += 60 - size;
>> >> + }
>> >
>> > Could you explain what you're doing with rx_padding?
>>
>> Avoiding short frames. I'll add a comments.
>
> This is for when RCTRL[RSF] is set, to accept short frames? Is it documented
> anywhere that these frames are zero-padded to 64 bytes on rx? If not, is
> this the observed behavior of real hardware?
>
> In any case, please add some comments.
>
Actually, this might be obsolete. At the time QEMU was sending short
frames, but vxWorks drop them, so I made this change to always have 64B
frames. Now I have another patch to fix this and I will remove this hack
and implement RCTRL[RSF].
>> >> + /* ring_base = (etsec->regs[RBASEH].value & 0xF) << 32; */
>> >> + ring_base += etsec->regs[RBASE0 + ring_nbr].value & ~0x7;
>> >> + start_bd_addr = bd_addr = etsec->regs[RBPTR0 + ring_nbr].value &
>> >> ~0x7;
>> >
>> > What about RBDBPH (upper bits of physical address)? Likewise for TX.
>> >
>>
>> I'm only interested in 32bits address spaces, so RBASEH, TBASEH, RBDBPH or
>> TBDBPH.
>
> When adding code to mainline, it's about more than what you're personally
> interested in...
>
If I'm not "personally" interested, I will not have time to develop or
test a feature. If someone wants to do 36bit addressing, feel free to do
it, that's why I send the patch on QEMU-devel, but I won't implement
full support of eTSEC myself...
> Could you at least have a way to diagnose when the guest OS tries to
> use some functionality that you don't support, rather than silently
> doing the wrong thing?
>
This device is so complex, detecting unsupported features would take too
much work.
Regards,
--
Fabien Chouteau
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 2/2] Add Enhanced Three-Speed Ethernet Controller (eTSEC), (continued)
- Re: [Qemu-ppc] [PATCH 2/2] Add Enhanced Three-Speed Ethernet Controller (eTSEC), Scott Wood, 2013/07/15
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 2/2] Add Enhanced Three-Speed Ethernet Controller (eTSEC), Fabien Chouteau, 2013/07/16
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 2/2] Add Enhanced Three-Speed Ethernet Controller (eTSEC), Alexander Graf, 2013/07/16
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 2/2] Add Enhanced Three-Speed Ethernet Controller (eTSEC), Fabien Chouteau, 2013/07/16
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 2/2] Add Enhanced Three-Speed Ethernet Controller (eTSEC), Scott Wood, 2013/07/16
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 2/2] Add Enhanced Three-Speed Ethernet Controller (eTSEC), Fabien Chouteau, 2013/07/17
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 2/2] Add Enhanced Three-Speed Ethernet Controller (eTSEC), Alexander Graf, 2013/07/17
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 2/2] Add Enhanced Three-Speed Ethernet Controller (eTSEC), Fabien Chouteau, 2013/07/17
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 2/2] Add Enhanced Three-Speed Ethernet Controller (eTSEC), Scott Wood, 2013/07/16
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 2/2] Add Enhanced Three-Speed Ethernet Controller (eTSEC),
Fabien Chouteau <=
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 2/2] Add Enhanced Three-Speed Ethernet Controller (eTSEC), Alexander Graf, 2013/07/17
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 2/2] Add Enhanced Three-Speed Ethernet Controller (eTSEC), Fabien Chouteau, 2013/07/17
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 2/2] Add Enhanced Three-Speed Ethernet Controller (eTSEC), Scott Wood, 2013/07/17
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 2/2] Add Enhanced Three-Speed Ethernet Controller (eTSEC), Fabien Chouteau, 2013/07/18
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 2/2] Add Enhanced Three-Speed Ethernet Controller (eTSEC), Scott Wood, 2013/07/18
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 2/2] Add Enhanced Three-Speed Ethernet Controller (eTSEC), Fabien Chouteau, 2013/07/19
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 2/2] Add Enhanced Three-Speed Ethernet Controller (eTSEC), Scott Wood, 2013/07/19
- Re: [Qemu-ppc] [Qemu-devel] [PATCH 2/2] Add Enhanced Three-Speed Ethernet Controller (eTSEC), Fabien Chouteau, 2013/07/22