From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33466) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZTUwN-00006h-UD for qemu-devel@nongnu.org; Sun, 23 Aug 2015 09:04:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZTUwM-0006aA-Gw for qemu-devel@nongnu.org; Sun, 23 Aug 2015 09:04:03 -0400 Message-ID: <55D9C4B0.10402@ilande.co.uk> Date: Sun, 23 Aug 2015 14:03:44 +0100 From: Mark Cave-Ayland MIME-Version: 1.0 References: <1440257533-1504-1-git-send-email-cormac@c-obrien.org> <1440257533-1504-5-git-send-email-cormac@c-obrien.org> In-Reply-To: <1440257533-1504-5-git-send-email-cormac@c-obrien.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH RFC 4/4] PPC: fix CUDA packet header size List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cormac O'Brien , agraf@suse.de, qemu-devel@nongnu.org, qemu-ppc@nongnu.org On 22/08/15 16:32, Cormac O'Brien wrote: > Change the CUDA packet model to use a three-byte header as in real hardware. > Also add handlers for CUDA_COMBINED_FORMAT_IIC and CUDA_GET_SET_IIC. I think it would make sense to split this patch into 2 - one for the CUDA packet changes, and another to add the *_IIC handlers. > Signed-off-by: Cormac O'Brien > --- > hw/input/adb.c | 2 +- > hw/misc/macio/cuda.c | 54 ++++++++++++++++++++++++++++++---------------------- > 2 files changed, 32 insertions(+), 24 deletions(-) > > diff --git a/hw/input/adb.c b/hw/input/adb.c > index a18eea2..4834234 100644 > --- a/hw/input/adb.c > +++ b/hw/input/adb.c > @@ -84,7 +84,7 @@ int adb_request(ADBBusState *s, uint8_t *obuf, const uint8_t *buf, int len) > return adc->devreq(d, obuf, buf, len); > } > } > - return ADB_RET_NOTPRESENT; > + return 0; //ADB_RET_NOTPRESENT; Does this need another constant defined rather than just using 0? > } > > /* XXX: move that to cuda ? */ > diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c > index f3984e3..753b5f3 100644 > --- a/hw/misc/macio/cuda.c > +++ b/hw/misc/macio/cuda.c > @@ -480,7 +480,7 @@ static void cuda_adb_poll(void *opaque) > static void cuda_receive_packet(CUDAState *s, > const uint8_t *data, int len) > { > - uint8_t obuf[16]; > + uint8_t obuf[16] = { CUDA_PACKET, 0, data[0] }; > int autopoll; > uint32_t ti; > > @@ -497,23 +497,16 @@ static void cuda_receive_packet(CUDAState *s, > timer_del(s->adb_poll_timer); > } > } > - obuf[0] = CUDA_PACKET; > - obuf[1] = data[1]; > - cuda_send_packet_to_host(s, obuf, 2); > + //obuf[1] = data[1]; > + cuda_send_packet_to_host(s, obuf, 3); > break; > case CUDA_SET_TIME: > ti = (((uint32_t)data[1]) << 24) + (((uint32_t)data[2]) << 16) + (((uint32_t)data[3]) << 8) + data[4]; > s->tick_offset = ti - (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) / get_ticks_per_sec()); > - obuf[0] = CUDA_PACKET; > - obuf[1] = 0; > - obuf[2] = 0; > cuda_send_packet_to_host(s, obuf, 3); > break; > case CUDA_GET_TIME: > ti = s->tick_offset + (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) / get_ticks_per_sec()); > - obuf[0] = CUDA_PACKET; > - obuf[1] = 0; > - obuf[2] = 0; > obuf[3] = ti >> 24; > obuf[4] = ti >> 16; > obuf[5] = ti >> 8; > @@ -524,23 +517,36 @@ static void cuda_receive_packet(CUDAState *s, > case CUDA_SET_DEVICE_LIST: > case CUDA_SET_AUTO_RATE: > case CUDA_SET_POWER_MESSAGES: > - obuf[0] = CUDA_PACKET; > - obuf[1] = 0; > - cuda_send_packet_to_host(s, obuf, 2); > + cuda_send_packet_to_host(s, obuf, 3); > break; > case CUDA_POWERDOWN: > - obuf[0] = CUDA_PACKET; > - obuf[1] = 0; > - cuda_send_packet_to_host(s, obuf, 2); > + cuda_send_packet_to_host(s, obuf, 3); > qemu_system_shutdown_request(); > break; > case CUDA_RESET_SYSTEM: > - obuf[0] = CUDA_PACKET; > - obuf[1] = 0; > - cuda_send_packet_to_host(s, obuf, 2); > + cuda_send_packet_to_host(s, obuf, 3); > qemu_system_reset_request(); > break; > + case CUDA_COMBINED_FORMAT_IIC: > + obuf[0] = ERROR_PACKET; > + obuf[1] = 0x5; > + obuf[2] = CUDA_PACKET; > + obuf[3] = data[0]; > + cuda_send_packet_to_host(s, obuf, 4); > + break; > + case CUDA_GET_SET_IIC: > + if (len == 4) { > + cuda_send_packet_to_host(s, obuf, 3); > + } else { > + obuf[0] = ERROR_PACKET; > + obuf[1] = 0x2; > + obuf[2] = CUDA_PACKET; > + obuf[3] = data[0]; > + cuda_send_packet_to_host(s, obuf, 4); > + } > + break; > default: > + cuda_send_packet_to_host(s, obuf, 3); > break; > } > } > @@ -560,19 +566,21 @@ static void cuda_receive_packet_from_host(CUDAState *s, > switch(data[0]) { > case ADB_PACKET: > { > - uint8_t obuf[ADB_MAX_OUT_LEN + 2]; > + uint8_t obuf[ADB_MAX_OUT_LEN + 3]; > int olen; > - olen = adb_request(&s->adb_bus, obuf + 2, data + 1, len - 1); > - if (olen > 0) { > + olen = adb_request(&s->adb_bus, obuf + 3, data + 1, len - 1); > + if (olen >= 0) { > obuf[0] = ADB_PACKET; > obuf[1] = 0x00; > + obuf[2] = data[1]; > } else { > /* error */ > obuf[0] = ADB_PACKET; > obuf[1] = -olen; > + obuf[2] = data[1]; > olen = 0; > } > - cuda_send_packet_to_host(s, obuf, olen + 2); > + cuda_send_packet_to_host(s, obuf, olen + 3); > } > break; > case CUDA_PACKET: > With some further testing, it seems that this breaks darwinppc-602.iso and OS X 10.2 on -M g3beige causing it to panic somewhere in the AppleCuda module. I think splitting the patch as mentioned above would help narrow down whether its the CUDA packet or the *_IIC changes in particular which cause the issue. Note that the source code for the AppleCuda module can be found at http://www.opensource.apple.com/source/AppleCuda/AppleCuda-100.0.3/ which should further help too. ATB, Mark.