Re: [PATCH v2 2/7] char: add goldfish-tty

2021-01-23 Thread Philippe Mathieu-Daudé
On 12/20/20 12:26 PM, Laurent Vivier wrote:
> Signed-off-by: Laurent Vivier 
> ---
>  include/hw/char/goldfish_tty.h |  36 +
>  hw/char/goldfish_tty.c | 266 +
>  hw/char/Kconfig|   3 +
>  hw/char/meson.build|   2 +
>  hw/char/trace-events   |   9 ++
>  5 files changed, 316 insertions(+)
>  create mode 100644 include/hw/char/goldfish_tty.h
>  create mode 100644 hw/char/goldfish_tty.c
...

> +static void goldfish_tty_cmd(GoldfishTTYState *s, uint32_t cmd)
> +{
> +int to_copy;
> +
> +switch (cmd) {
> +case CMD_INT_DISABLE:
> +if (s->int_enabled) {
> +if (s->data_in_count) {
> +qemu_set_irq(s->irq, 0);
> +}
> +s->int_enabled = false;
> +}
> +break;
> +case CMD_INT_ENABLE:
> +if (!s->int_enabled) {
> +if (s->data_in_count) {
> +qemu_set_irq(s->irq, 1);
> +}
> +s->int_enabled = true;
> +}
> +break;
> +case CMD_WRITE_BUFFER:
> +to_copy = s->data_len;
> +while (to_copy) {
> +int len;
> +
> +len = MIN(GOLFISH_TTY_BUFFER_SIZE, to_copy);
> +
> +address_space_rw(_space_memory, s->data_ptr,
> + MEMTXATTRS_UNSPECIFIED, s->data_out, len, 0);

Could this fail, no need to check return value?

> +to_copy -= len;
> +qemu_chr_fe_write_all(>chr, s->data_out, len);
> +}
> +break;
> +case CMD_READ_BUFFER:
> +to_copy = MIN(s->data_len, s->data_in_count);
> +address_space_rw(_space_memory, s->data_ptr,
> + MEMTXATTRS_UNSPECIFIED, s->data_in, to_copy, 1);

Ditto.

> +s->data_in_count -= to_copy;
> +memmove(s->data_in, s->data_in + to_copy, s->data_in_count);
> +if (s->int_enabled && !s->data_in_count) {
> +qemu_set_irq(s->irq, 0);
> +}
> +break;
> +}
> +}
> +
> +static void goldfish_tty_write(void *opaque, hwaddr addr,
> +   uint64_t value, unsigned size)
> +{
> +GoldfishTTYState *s = opaque;
> +unsigned char c;
> +
> +trace_goldfish_tty_write(s, addr, size, value);
> +
> +switch (addr) {
> +case REG_PUT_CHAR:
> +c = value;
> +qemu_chr_fe_write_all(>chr, , sizeof(c));
> +break;
> +case REG_CMD:
> +goldfish_tty_cmd(s, value);
> +break;
> +case REG_DATA_PTR:
> +s->data_ptr = value;
> +break;
> +case REG_DATA_PTR_HIGH:
> +s->data_ptr = (value << 32) | (uint32_t)s->data_ptr;
> +break;
> +case REG_DATA_LEN:
> +s->data_len = value;
> +break;
> +default:
> +qemu_log_mask(LOG_UNIMP,
> +  "%s: unimplemented register write 
> 0x%02"HWADDR_PRIx"\n",
> +  __func__, addr);
> +break;
> +}
> +}
> +
> +static const MemoryRegionOps goldfish_tty_ops = {
> +.read = goldfish_tty_read,
> +.write = goldfish_tty_write,
> +.endianness = DEVICE_NATIVE_ENDIAN,
> +.valid.max_access_size = 4,
> +.impl.max_access_size = 4,

Missing:

  .impl.min = 4,

Otherwise:
Reviewed-by: Philippe Mathieu-Daudé 

> +};
> +



[PATCH v2 2/7] char: add goldfish-tty

2020-12-20 Thread Laurent Vivier
Signed-off-by: Laurent Vivier 
---
 include/hw/char/goldfish_tty.h |  36 +
 hw/char/goldfish_tty.c | 266 +
 hw/char/Kconfig|   3 +
 hw/char/meson.build|   2 +
 hw/char/trace-events   |   9 ++
 5 files changed, 316 insertions(+)
 create mode 100644 include/hw/char/goldfish_tty.h
 create mode 100644 hw/char/goldfish_tty.c

diff --git a/include/hw/char/goldfish_tty.h b/include/hw/char/goldfish_tty.h
new file mode 100644
index ..84d78f8cff54
--- /dev/null
+++ b/include/hw/char/goldfish_tty.h
@@ -0,0 +1,36 @@
+/*
+ * SPDX-License-Identifer: GPL-2.0-or-later
+ *
+ * Goldfish TTY
+ *
+ * (c) 2020 Laurent Vivier 
+ *
+ */
+
+#ifndef HW_CHAR_GOLDFISH_TTY_H
+#define HW_CHAR_GOLDFISH_TTY_H
+
+#include "chardev/char-fe.h"
+
+#define TYPE_GOLDFISH_TTY "goldfish_tty"
+OBJECT_DECLARE_SIMPLE_TYPE(GoldfishTTYState, GOLDFISH_TTY)
+
+#define GOLFISH_TTY_BUFFER_SIZE 128
+
+struct GoldfishTTYState {
+SysBusDevice parent_obj;
+
+MemoryRegion iomem;
+qemu_irq irq;
+CharBackend chr;
+
+uint32_t data_len;
+uint64_t data_ptr;
+bool int_enabled;
+
+uint32_t data_in_count;
+uint8_t data_in[GOLFISH_TTY_BUFFER_SIZE];
+uint8_t data_out[GOLFISH_TTY_BUFFER_SIZE];
+};
+
+#endif
diff --git a/hw/char/goldfish_tty.c b/hw/char/goldfish_tty.c
new file mode 100644
index ..7780940a8817
--- /dev/null
+++ b/hw/char/goldfish_tty.c
@@ -0,0 +1,266 @@
+/*
+ * SPDX-License-Identifer: GPL-2.0-or-later
+ *
+ * Goldfish TTY
+ *
+ * (c) 2020 Laurent Vivier 
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "hw/irq.h"
+#include "hw/qdev-properties.h"
+#include "hw/sysbus.h"
+#include "migration/vmstate.h"
+#include "chardev/char-fe.h"
+#include "qemu/log.h"
+#include "trace.h"
+#include "exec/address-spaces.h"
+#include "hw/char/goldfish_tty.h"
+
+/* registers */
+
+enum {
+REG_PUT_CHAR  = 0x00,
+REG_BYTES_READY   = 0x04,
+REG_CMD   = 0x08,
+REG_DATA_PTR  = 0x10,
+REG_DATA_LEN  = 0x14,
+REG_DATA_PTR_HIGH = 0x18,
+REG_VERSION   = 0x20,
+};
+
+/* commands */
+
+enum {
+CMD_INT_DISABLE   = 0x00,
+CMD_INT_ENABLE= 0x01,
+CMD_WRITE_BUFFER  = 0x02,
+CMD_READ_BUFFER   = 0x03,
+};
+
+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 = s->data_in_count;
+break;
+case REG_VERSION:
+value = 0;
+break;
+default:
+qemu_log_mask(LOG_UNIMP,
+  "%s: unimplemented register read 0x%02"HWADDR_PRIx"\n",
+  __func__, addr);
+break;
+}
+
+trace_goldfish_tty_read(s, addr, size, value);
+
+return value;
+}
+
+static void goldfish_tty_cmd(GoldfishTTYState *s, uint32_t cmd)
+{
+int to_copy;
+
+switch (cmd) {
+case CMD_INT_DISABLE:
+if (s->int_enabled) {
+if (s->data_in_count) {
+qemu_set_irq(s->irq, 0);
+}
+s->int_enabled = false;
+}
+break;
+case CMD_INT_ENABLE:
+if (!s->int_enabled) {
+if (s->data_in_count) {
+qemu_set_irq(s->irq, 1);
+}
+s->int_enabled = true;
+}
+break;
+case CMD_WRITE_BUFFER:
+to_copy = s->data_len;
+while (to_copy) {
+int len;
+
+len = MIN(GOLFISH_TTY_BUFFER_SIZE, to_copy);
+
+address_space_rw(_space_memory, s->data_ptr,
+ MEMTXATTRS_UNSPECIFIED, s->data_out, len, 0);
+to_copy -= len;
+qemu_chr_fe_write_all(>chr, s->data_out, len);
+}
+break;
+case CMD_READ_BUFFER:
+to_copy = MIN(s->data_len, s->data_in_count);
+address_space_rw(_space_memory, s->data_ptr,
+ MEMTXATTRS_UNSPECIFIED, s->data_in, to_copy, 1);
+s->data_in_count -= to_copy;
+memmove(s->data_in, s->data_in + to_copy, s->data_in_count);
+if (s->int_enabled && !s->data_in_count) {
+qemu_set_irq(s->irq, 0);
+}
+break;
+}
+}
+
+static void goldfish_tty_write(void *opaque, hwaddr addr,
+   uint64_t value, unsigned size)
+{
+GoldfishTTYState *s = opaque;
+unsigned char c;
+
+trace_goldfish_tty_write(s, addr, size, value);
+
+switch (addr) {
+case REG_PUT_CHAR:
+c = value;
+qemu_chr_fe_write_all(>chr, , sizeof(c));
+break;
+case REG_CMD:
+goldfish_tty_cmd(s, value);
+break;
+case REG_DATA_PTR:
+s->data_ptr = value;
+break;
+case REG_DATA_PTR_HIGH:
+s->data_ptr = (value << 32) | (uint32_t)s->data_ptr;
+break;
+case REG_DATA_LEN:
+s->data_len = value;
+break;
+default:
+