From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38757) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UzYsO-000659-5o for qemu-devel@nongnu.org; Wed, 17 Jul 2013 17:03:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UzYsL-0000HU-51 for qemu-devel@nongnu.org; Wed, 17 Jul 2013 17:03:08 -0400 Date: Wed, 17 Jul 2013 16:02:50 -0500 From: Scott Wood In-Reply-To: <51E66F22.6030202@adacore.com> (from chouteau@adacore.com on Wed Jul 17 05:17:06 2013) Message-ID: <1374094970.8183.365@snotra> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; delsp=Yes; format=Flowed Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH 2/2] Add Enhanced Three-Speed Ethernet Controller (eTSEC) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fabien Chouteau Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org On 07/17/2013 05:17:06 AM, Fabien Chouteau wrote: > 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: > >> >> + if (*size =3D=3D etsec->rx_padding) { > >> >> + /* The remaining bytes are for padding which is not =20 > actually allocated > >> >> + in the buffer */ > >> >> + > >> >> + rem =3D MIN(etsec->regs[MRBLR].value - bd->length, =20 > etsec->rx_padding); > >> >> + > >> >> + if (rem > 0) { > >> >> + memset(padd, 0x0, sizeof(padd)); > >> >> + etsec->rx_padding -=3D rem; > >> >> + *size -=3D rem; > >> >> + bd->length +=3D 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 =20 > for padding a short frame, since MRBLR must be a multiple of 64, but =20 > what if it's 4 bytes for CRC? > > >=20 > Can you explain a possible error scenario? 126 byte packet, no fcb. rx_padding is 4 for CRC. Suppose MRBLR is =20 128. Wouldn't *size be 2 here? > > 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? > > >=20 > This device is so complex, detecting unsupported features would take =20 > too > much work. I was thinking along the lines of marking registers and bits within =20 registers as supported (or which are properly no-ops in QEMU) -- and =20 warning the first time you see a non-default-value write to an =20 unsupported field or register. It could save a lot of debugging. -Scott=