On Thu, 11 Mar 2021 at 21:22, Laurent Vivier <laur...@vivier.eu> wrote:
>
> Implement the goldfish tty device as defined in
>
> https://android.googlesource.com/platform/external/qemu/+/master/docs/GOLDFISH-VIRTUAL-HARDWARE.TXT
>
> and based on the kernel driver code:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/goldfish.c
>
> Signed-off-by: Laurent Vivier <laur...@vivier.eu>
> Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org>
> Tested-by: Philippe Mathieu-Daudé <f4...@amsat.org>
> Message-Id: <20210309195941.763896-2-laur...@vivier.eu>
> Signed-off-by: Laurent Vivier <laur...@vivier.eu>
I didn't notice this earlier, but this looks odd:
> +static uint64_t goldfish_tty_read(void *opaque, hwaddr addr,
> + unsigned size)
> +{
> + GoldfishTTYState *s = opaque;
> + uint64_t value = 0;
> +
> + switch (addr) {
> + case REG_BYTES_READY:
> + value = fifo8_num_used(&s->rx_fifo);
> + break;
> + case REG_VERSION:
> + value = 0;
You report as a version 0 Goldfish TTY device.
This is the old kind that used guest virtual addresses,
unlike the more sensible version 1 ("ranchu") kind that uses
physical addresses.
You can see this in the kernel driver code:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/goldfish.c
where it looks at qtty->version.
> + case CMD_WRITE_BUFFER:
> + len = s->data_len;
> + ptr = s->data_ptr;
> + while (len) {
> + to_copy = MIN(GOLFISH_TTY_BUFFER_SIZE, len);
> +
> + address_space_rw(&address_space_memory, ptr,
> + MEMTXATTRS_UNSPECIFIED, data_out, to_copy, 0);
> + qemu_chr_fe_write_all(&s->chr, data_out, to_copy);
> +
> + len -= to_copy;
> + ptr += to_copy;
> + }
> + break;
> + case CMD_READ_BUFFER:
> + len = s->data_len;
> + ptr = s->data_ptr;
> + while (len && !fifo8_is_empty(&s->rx_fifo)) {
> + buf = (uint8_t *)fifo8_pop_buf(&s->rx_fifo, len, &to_copy);
> + address_space_rw(&address_space_memory, ptr,
> + MEMTXATTRS_UNSPECIFIED, buf, to_copy, 1);
> +
> + len -= to_copy;
> + ptr += to_copy;
> + }
...but here you're treating the data pointer value from the
guest like a physical address. I'm not sure how this works.
(This is one of the areas where you need to be really cautious about
using the goldfish devices -- "device model gets virtual addresses from
guest OS" is a really bad design.)
thanks
-- PMM
