Le 11/03/2021 à 22:57, Peter Maydell a écrit : > 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.)
Thank you Peter. I will resend the pull request without the virt m68k machine part. Laurent
