Re: [Qemu-devel] [PATCH 04/13] cuda.c: fix CUDA ADB error packet format

[Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>]

On 04/11/15 03:12, David Gibson wrote:

> On Fri, Oct 23, 2015 at 02:56:29PM +0100, Mark Cave-Ayland wrote:
>> ADB error packets should be of the form (type, status, cmd) rather than just
>> (type, status). This fixes ADB device detection under MacOS 9.
> 
> Hmm.. are there any public doc on these ADB / CUDA things that we can
> look up to compare to?

Sadly no. The main process for working during GSoC was to add logging
into various areas to see the point at which execution stopped, then
compare with MOL, code up the difference and see if this helps boot
progress.

Pretty much every other OS will read in the entire CUDA packet based
upon the length and check the status for errors, whereas it seems MacOS
9 additionally checks the right number of bytes are received in the
reply (even if they are never used).

>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>>  hw/misc/macio/cuda.c |    6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
>> index 5d7043e..9ec19af 100644
>> --- a/hw/misc/macio/cuda.c
>> +++ b/hw/misc/macio/cuda.c
>> @@ -560,19 +560,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) {
>>                  obuf[0] = ADB_PACKET;
>>                  obuf[1] = 0x00;
>> +                cuda_send_packet_to_host(s, obuf, olen + 2);
>>              } else {
>>                  /* error */
>>                  obuf[0] = ADB_PACKET;
>>                  obuf[1] = -olen;
>> +                obuf[2] = data[1];
>>                  olen = 0;
>> +                cuda_send_packet_to_host(s, obuf, olen + 3);
> 
> Using 'olen + 3' seems confusing here, rather than just '3', since you
> just set olen = 0.

I do prefer the way it is at the moment, because all the calls to
cuda_send_packet_to_host() use a length of the form "olen + X" which
tells you the payload length (0) + the header length (X). So from this
we know that there are just the 3 standard header bytes and no
additional payload being returned by this particular transaction.

>>              }
>> -            cuda_send_packet_to_host(s, obuf, olen + 2);
>>          }
>>          break;
>>      case CUDA_PACKET:
> 

ATB,

Mark.