[OpenBIOS] [PATCH RFC] ppc: fix CUDA ADB packet header format

[Mark Cave-Ayland]

On 10/07/15 18:49, Cormac O'Brien wrote:

> Previous versions of QEMU use a 2-byte header for CUDA ADB packets where it
> should have used a 3-byte one, so this commit allows cuda_adb_req() to
> differentiate between the two formats and act accordingly.
> 
> Signed-off-by: Cormac O'Brien <cormac at c-obrien.org>
> ---
>  drivers/cuda.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cuda.c b/drivers/cuda.c
> index 9555dea..deceacf 100644
> --- a/drivers/cuda.c
> +++ b/drivers/cuda.c
> @@ -144,8 +144,18 @@ static int cuda_adb_req (void *host, const uint8_t *snd_buf, int len,
>   //   CUDA_DPRINTF("len: %d %02x\n", len, snd_buf[0]);
>      len = cuda_request(host, ADB_PACKET, snd_buf, len, buffer);
>      if (len > 1 && buffer[0] == ADB_PACKET) {
> -        pos = buffer + 2;
> -        len -= 2;
> +        /* QEMU's previous model used a 2-byte header where it should use a
> +         * 3-byte one, so we check to see what kind of header we ought to use.
> +         */
> +        if (len > 2 && buffer[2] == snd_buf[0]) {
> +            /* Correct 3-byte header */
> +            pos = buffer + 3;
> +            len -= 3;
> +        } else {
> +            /* Old 2-byte header */
> +            pos = buffer + 2;
> +            len -= 2;
> +        }
>      } else {
>          pos = buffer + 1;
>          len = -1;
> 

Technically the patch looks good to me - due to the nature of the
change, can you confirm that this patch doesn't cause regressions in any
other OS images during testing? Even better, is there a reference to a
3-byte header in any of the open OS sources to confirm that this is
definitely the correct solution?

The only change I would like to see is a reference to specific QEMU
versions in the comments and commit log, so instead of referencing
"QEMU's previous model" it would be better to state "QEMU versions <=
2.4" instead. This just makes it a bit easier to understand to
implications of removing the hack at a later date.


ATB,

Mark.