* [PATCH 0/2] q800: fix and improve RTC/PRAM interface
@ 2019-12-19 20:14 Laurent Vivier
2019-12-19 20:14 ` [PATCH 1/2] q800: fix mac_via RTC PRAM commands Laurent Vivier
2019-12-19 20:14 ` [PATCH 2/2] q800: add a block backend to the PRAM Laurent Vivier
0 siblings, 2 replies; 4+ messages in thread
From: Laurent Vivier @ 2019-12-19 20:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Philippe Mathieu-Daudé, Mark Cave-Ayland, Laurent Vivier
A bug has been reported on launchpad about an error
reported by cc:
qemu-4.2.0/hw/misc/mac_via.c:467:27: style: Expression is
always false because 'else if' condition matches previous
condition at line 463. [multiCondition]
https://bugs.launchpad.net/qemu/+bug/1856549
In fact, the PRAM interface has not really been tested and
has several problems that needed to be fixed.
This series makes a cleanup in this part of code and
fix existing problems.
It also adds some trace-events that helped to debug the
numerous issues and a backend file to allow to save and
restore the content of the PRAM.
This has been tested by playing with the /dev/nvram interface
in the guest, checking we can read what we have written, and
we can read and write in the file on host and read and write
the same data in the guest /dev/nvram.
Laurent Vivier (2):
q800: fix mac_via RTC PRAM commands
q800: add a block backend to the PRAM
hw/m68k/q800.c | 6 +
hw/misc/mac_via.c | 339 ++++++++++++++++++++++++++++----------
hw/misc/trace-events | 19 +++
include/hw/misc/mac_via.h | 3 +
4 files changed, 284 insertions(+), 83 deletions(-)
--
2.24.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] q800: fix mac_via RTC PRAM commands
2019-12-19 20:14 [PATCH 0/2] q800: fix and improve RTC/PRAM interface Laurent Vivier
@ 2019-12-19 20:14 ` Laurent Vivier
2019-12-22 11:07 ` Mark Cave-Ayland
2019-12-19 20:14 ` [PATCH 2/2] q800: add a block backend to the PRAM Laurent Vivier
1 sibling, 1 reply; 4+ messages in thread
From: Laurent Vivier @ 2019-12-19 20:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Philippe Mathieu-Daudé, Mark Cave-Ayland, Laurent Vivier
The command byte is not decoded correctly.
This patch reworks the RTC/PRAM interface and fixes the problem.
It adds a comment before the function to explain how are encoded commands
and some trace-events to ease debugging.
Bug: https://bugs.launchpad.net/qemu/+bug/1856549
Fixes: 6dca62a000 ("hw/m68k: add VIA support")
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
hw/misc/mac_via.c | 274 ++++++++++++++++++++++++++++++-------------
hw/misc/trace-events | 19 +++
2 files changed, 210 insertions(+), 83 deletions(-)
diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index f3f130ad96..e5658af922 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -27,7 +27,7 @@
#include "sysemu/runstate.h"
#include "qapi/error.h"
#include "qemu/cutils.h"
-
+#include "trace.h"
/*
* VIAs: There are two in every machine,
@@ -278,6 +278,21 @@
/* VIA returns time offset from Jan 1, 1904, not 1970 */
#define RTC_OFFSET 2082844800
+enum {
+ REG_0,
+ REG_1,
+ REG_2,
+ REG_3,
+ REG_TEST,
+ REG_WPROTECT,
+ REG_PRAM_ADDR,
+ REG_PRAM_ADDR_LAST = REG_PRAM_ADDR + 19,
+ REG_PRAM_SECT,
+ REG_PRAM_SECT_LAST = REG_PRAM_SECT + 7,
+ REG_INVALID,
+ REG_EMPTY = 0xff,
+};
+
static void via1_VBL_update(MOS6522Q800VIA1State *v1s)
{
MOS6522State *s = MOS6522(v1s);
@@ -360,10 +375,62 @@ static void via2_irq_request(void *opaque, int irq, int level)
mdc->update_irq(s);
}
+/*
+ * RTC Commands
+ *
+ * Command byte Register addressed by the command
+ *
+ * z0000001 Seconds register 0 (lowest-order byte)
+ * z0000101 Seconds register 1
+ * z0001001 Seconds register 2
+ * z0001101 Seconds register 3 (highest-order byte)
+ * 00110001 Test register (write-only)
+ * 00110101 Write-Protect Register (write-only)
+ * z010aa01 RAM address 100aa ($10-$13) (first 20 bytes only)
+ * z1aaaa01 RAM address 0aaaa ($00-$0F) (first 20 bytes only)
+ * z0111aaa Extended memory designator and sector number
+ *
+ * For a read request, z=1, for a write z=0
+ * The letter a indicates bits whose value depend on what parameter
+ * RAM byte you want to address
+ */
+static int via1_rtc_compact_cmd(uint8_t value)
+{
+ uint8_t read = value & 0x80;
+
+ value &= 0x7f;
+
+ /* the last 2 bits of a command byte must always be 0b01 ... */
+ if ((value & 0x78) == 0x38) {
+ /* except for the extended memory designator */
+ return read | (REG_PRAM_SECT + (value & 0x07));
+ }
+ if ((value & 0x03) == 0x01) {
+ value >>= 2;
+ if ((value & 0x1c) == 0) {
+ /* seconds registers */
+ return read | (REG_0 + (value & 0x03));
+ } else if ((value == 0x0c) && !read) {
+ return REG_TEST;
+ } else if ((value == 0x0d) && !read) {
+ return REG_WPROTECT;
+ } else if ((value & 0x1c) == 0x08) {
+ /* RAM address 0x10 to 0x13 */
+ return read | (REG_PRAM_ADDR + 0x10 + (value & 0x03));
+ } else if ((value & 0x43) == 0x41) {
+ /* RAM address 0x00 to 0x0f */
+ return read | (REG_PRAM_ADDR + (value & 0x0f));
+ }
+ }
+ return REG_INVALID;
+}
+
static void via1_rtc_update(MacVIAState *m)
{
MOS6522Q800VIA1State *v1s = &m->mos6522_via1;
MOS6522State *s = MOS6522(v1s);
+ int cmd, sector, addr;
+ uint32_t time;
if (s->b & VIA1B_vRTCEnb) {
return;
@@ -376,7 +443,9 @@ static void via1_rtc_update(MacVIAState *m)
m->data_out |= s->b & VIA1B_vRTCData;
m->data_out_cnt++;
}
+ trace_via1_rtc_update_data_out(m->data_out_cnt, m->data_out);
} else {
+ trace_via1_rtc_update_data_in(m->data_in_cnt, m->data_in);
/* receive bits from the RTC */
if ((v1s->last_b & VIA1B_vRTCClk) &&
!(s->b & VIA1B_vRTCClk) &&
@@ -386,96 +455,132 @@ static void via1_rtc_update(MacVIAState *m)
m->data_in <<= 1;
m->data_in_cnt--;
}
+ return;
}
- if (m->data_out_cnt == 8) {
- m->data_out_cnt = 0;
-
- if (m->cmd == 0) {
- if (m->data_out & 0x80) {
- /* this is a read command */
- uint32_t time = m->tick_offset +
- (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) /
- NANOSECONDS_PER_SECOND);
- if (m->data_out == 0x81) { /* seconds register 0 */
- m->data_in = time & 0xff;
- m->data_in_cnt = 8;
- } else if (m->data_out == 0x85) { /* seconds register 1 */
- m->data_in = (time >> 8) & 0xff;
- m->data_in_cnt = 8;
- } else if (m->data_out == 0x89) { /* seconds register 2 */
- m->data_in = (time >> 16) & 0xff;
- m->data_in_cnt = 8;
- } else if (m->data_out == 0x8d) { /* seconds register 3 */
- m->data_in = (time >> 24) & 0xff;
- m->data_in_cnt = 8;
- } else if ((m->data_out & 0xf3) == 0xa1) {
- /* PRAM address 0x10 -> 0x13 */
- int addr = (m->data_out >> 2) & 0x03;
- m->data_in = v1s->PRAM[addr];
- m->data_in_cnt = 8;
- } else if ((m->data_out & 0xf3) == 0xa1) {
- /* PRAM address 0x00 -> 0x0f */
- int addr = (m->data_out >> 2) & 0x0f;
- m->data_in = v1s->PRAM[addr];
- m->data_in_cnt = 8;
- } else if ((m->data_out & 0xf8) == 0xb8) {
- /* extended memory designator and sector number */
- m->cmd = m->data_out;
- }
- } else {
- /* this is a write command */
- m->cmd = m->data_out;
+ if (m->data_out_cnt != 8) {
+ return;
+ }
+
+ m->data_out_cnt = 0;
+
+ trace_via1_rtc_internal_status(m->cmd, m->alt, m->data_out);
+ /* first byte: it's a command */
+ if (m->cmd == REG_EMPTY) {
+
+ cmd = via1_rtc_compact_cmd(m->data_out);
+ trace_via1_rtc_internal_cmd(cmd);
+
+ if (cmd == REG_INVALID) {
+ trace_via1_rtc_cmd_invalid(m->data_out);
+ return;
+ }
+
+ if (cmd & 0x80) { /* this is a read command */
+ switch (cmd & 0x7f) {
+ case REG_0...REG_3: /* seconds registers */
+ /*
+ * register 0 is lowest-order byte
+ * register 3 is highest-order byte
+ */
+
+ time = m->tick_offset + (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)
+ / NANOSECONDS_PER_SECOND);
+ trace_via1_rtc_internal_time(time);
+ m->data_in = (time >> ((cmd & 0x03) << 3)) & 0xff;
+ m->data_in_cnt = 8;
+ trace_via1_rtc_cmd_seconds_read((cmd & 0x7f) - REG_0,
+ m->data_in);
+ break;
+ case REG_PRAM_ADDR...REG_PRAM_ADDR_LAST:
+ /* PRAM address 0x00 -> 0x13 */
+ m->data_in = v1s->PRAM[(cmd & 0x7f) - REG_PRAM_ADDR];
+ m->data_in_cnt = 8;
+ trace_via1_rtc_cmd_pram_read((cmd & 0x7f) - REG_PRAM_ADDR,
+ m->data_in);
+ break;
+ case REG_PRAM_SECT...REG_PRAM_SECT_LAST:
+ /*
+ * extended memory designator and sector number
+ * the only two-byte read command
+ */
+ trace_via1_rtc_internal_set_cmd(cmd);
+ m->cmd = cmd;
+ break;
+ default:
+ g_assert_not_reached();
+ break;
}
+ return;
+ }
+
+ /* this is a write command, needs a parameter */
+ if (cmd == REG_WPROTECT || !m->wprotect) {
+ trace_via1_rtc_internal_set_cmd(cmd);
+ m->cmd = cmd;
} else {
+ trace_via1_rtc_internal_ignore_cmd(cmd);
+ }
+ return;
+ }
+
+ /* second byte: it's a parameter */
+ if (m->alt == REG_EMPTY) {
+ switch (m->cmd & 0x7f) {
+ case REG_0...REG_3: /* seconds register */
+ /* FIXME */
+ trace_via1_rtc_cmd_seconds_write(m->cmd - REG_0, m->data_out);
+ m->cmd = REG_EMPTY;
+ break;
+ case REG_TEST:
+ /* device control: nothing to do */
+ trace_via1_rtc_cmd_test_write(m->data_out);
+ m->cmd = REG_EMPTY;
+ break;
+ case REG_WPROTECT:
+ /* Write Protect register */
+ trace_via1_rtc_cmd_wprotect_write(m->data_out);
+ m->wprotect = !!(m->data_out & 0x80);
+ m->cmd = REG_EMPTY;
+ break;
+ case REG_PRAM_ADDR...REG_PRAM_ADDR_LAST:
+ /* PRAM address 0x00 -> 0x13 */
+ trace_via1_rtc_cmd_pram_write(m->cmd - REG_PRAM_ADDR, m->data_out);
+ v1s->PRAM[m->cmd - REG_PRAM_ADDR] = m->data_out;
+ m->cmd = REG_EMPTY;
+ break;
+ case REG_PRAM_SECT...REG_PRAM_SECT_LAST:
+ addr = (m->data_out >> 2) & 0x1f;
+ sector = (m->cmd & 0x7f) - REG_PRAM_SECT;
if (m->cmd & 0x80) {
- if ((m->cmd & 0xf8) == 0xb8) {
- /* extended memory designator and sector number */
- int sector = m->cmd & 0x07;
- int addr = (m->data_out >> 2) & 0x1f;
-
- m->data_in = v1s->PRAM[sector * 8 + addr];
- m->data_in_cnt = 8;
- }
- } else if (!m->wprotect) {
- /* this is a write command */
- if (m->alt != 0) {
- /* extended memory designator and sector number */
- int sector = m->cmd & 0x07;
- int addr = (m->alt >> 2) & 0x1f;
-
- v1s->PRAM[sector * 8 + addr] = m->data_out;
-
- m->alt = 0;
- } else if (m->cmd == 0x01) { /* seconds register 0 */
- /* FIXME */
- } else if (m->cmd == 0x05) { /* seconds register 1 */
- /* FIXME */
- } else if (m->cmd == 0x09) { /* seconds register 2 */
- /* FIXME */
- } else if (m->cmd == 0x0d) { /* seconds register 3 */
- /* FIXME */
- } else if (m->cmd == 0x31) {
- /* Test Register */
- } else if (m->cmd == 0x35) {
- /* Write Protect register */
- m->wprotect = m->data_out & 1;
- } else if ((m->cmd & 0xf3) == 0xa1) {
- /* PRAM address 0x10 -> 0x13 */
- int addr = (m->cmd >> 2) & 0x03;
- v1s->PRAM[addr] = m->data_out;
- } else if ((m->cmd & 0xf3) == 0xa1) {
- /* PRAM address 0x00 -> 0x0f */
- int addr = (m->cmd >> 2) & 0x0f;
- v1s->PRAM[addr] = m->data_out;
- } else if ((m->cmd & 0xf8) == 0xb8) {
- /* extended memory designator and sector number */
- m->alt = m->cmd;
- }
+ /* it's a read */
+ m->data_in = v1s->PRAM[sector * 32 + addr];
+ m->data_in_cnt = 8;
+ trace_via1_rtc_cmd_pram_sect_read(sector, addr,
+ sector * 32 + addr,
+ m->data_in);
+ m->cmd = REG_EMPTY;
+ } else {
+ /* it's a write, we need one more parameter */
+ trace_via1_rtc_internal_set_alt(addr, sector, addr);
+ m->alt = addr;
}
+ break;
+ default:
+ g_assert_not_reached();
+ break;
}
- m->data_out = 0;
+ return;
}
+
+ /* third byte: it's the data of a REG_PRAM_SECT write */
+ g_assert(REG_PRAM_SECT <= m->cmd && m->cmd <= REG_PRAM_SECT_LAST);
+ sector = m->cmd - REG_PRAM_SECT;
+ v1s->PRAM[sector * 32 + m->alt] = m->data_out;
+ trace_via1_rtc_cmd_pram_sect_write(sector, m->alt, sector * 32 + m->alt,
+ m->data_out);
+ m->alt = REG_EMPTY;
+ m->cmd = REG_EMPTY;
}
static int adb_via_poll(MacVIAState *s, int state, uint8_t *data)
@@ -742,6 +847,9 @@ static void mac_via_reset(DeviceState *dev)
v1s->next_VBL = 0;
timer_del(v1s->one_second_timer);
v1s->next_second = 0;
+
+ m->cmd = REG_EMPTY;
+ m->alt = REG_EMPTY;
}
static void mac_via_realize(DeviceState *dev, Error **errp)
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index 1deb1d08c1..2e0c820834 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -149,3 +149,22 @@ bcm2835_mbox_write(unsigned int size, uint64_t addr, uint64_t value) "mbox write
bcm2835_mbox_read(unsigned int size, uint64_t addr, uint64_t value) "mbox read sz:%u addr:0x%"PRIx64" data:0x%"PRIx64
bcm2835_mbox_irq(unsigned level) "mbox irq:ARM level:%u"
bcm2835_mbox_property(uint32_t tag, uint32_t bufsize, size_t resplen) "mbox property tag:0x%08x in_sz:%u out_sz:%zu"
+
+# mac_via.c
+via1_rtc_update_data_out(int count, int value) "count=%d value=0x%02x"
+via1_rtc_update_data_in(int count, int value) "count=%d value=0x%02x"
+via1_rtc_internal_status(int cmd, int alt, int value) "cmd=0x%02x alt=0x%02x value=0x%02x"
+via1_rtc_internal_cmd(int cmd) "cmd=0x%02x"
+via1_rtc_cmd_invalid(int value) "value=0x%02x"
+via1_rtc_internal_time(uint32_t time) "time=0x%08x"
+via1_rtc_internal_set_cmd(int cmd) "cmd=0x%02x"
+via1_rtc_internal_ignore_cmd(int cmd) "cmd=0x%02x"
+via1_rtc_internal_set_alt(int alt, int sector, int offset) "alt=0x%02x sector=%u offset=%u"
+via1_rtc_cmd_seconds_read(int reg, int value) "reg=%d value=0x%02x"
+via1_rtc_cmd_seconds_write(int reg, int value) "reg=%d value=0x%02x"
+via1_rtc_cmd_test_write(int value) "value=0x%02x"
+via1_rtc_cmd_wprotect_write(int value) "value=0x%02x"
+via1_rtc_cmd_pram_read(int addr, int value) "addr=%u value=0x%02x"
+via1_rtc_cmd_pram_write(int addr, int value) "addr=%u value=0x%02x"
+via1_rtc_cmd_pram_sect_read(int sector, int offset, int addr, int value) "sector=%u offset=%u addr=%d value=0x%02x"
+via1_rtc_cmd_pram_sect_write(int sector, int offset, int addr, int value) "sector=%u offset=%u addr=%d value=0x%02x"
--
2.24.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] q800: add a block backend to the PRAM
2019-12-19 20:14 [PATCH 0/2] q800: fix and improve RTC/PRAM interface Laurent Vivier
2019-12-19 20:14 ` [PATCH 1/2] q800: fix mac_via RTC PRAM commands Laurent Vivier
@ 2019-12-19 20:14 ` Laurent Vivier
1 sibling, 0 replies; 4+ messages in thread
From: Laurent Vivier @ 2019-12-19 20:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Philippe Mathieu-Daudé, Mark Cave-Ayland, Laurent Vivier
This allows to save and restore the content of the PRAM.
It may be useful if we want to check the configuration or to change it.
The backend is added using mtd interface, for instance:
... -drive file=pram.img,format=raw,if=mtd ...
where pram.img is the file where the data will be stored, its size must
be 256 bytes.
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
hw/m68k/q800.c | 6 ++++
hw/misc/mac_via.c | 65 +++++++++++++++++++++++++++++++++++++++
include/hw/misc/mac_via.h | 3 ++
3 files changed, 74 insertions(+)
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 4ca8678007..0c445c74bf 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -158,6 +158,7 @@ static void q800_init(MachineState *machine)
NubusBus *nubus;
GLUEState *irq;
qemu_irq *pic;
+ DriveInfo *dinfo;
linux_boot = (kernel_filename != NULL);
@@ -200,6 +201,11 @@ static void q800_init(MachineState *machine)
/* VIA */
via_dev = qdev_create(NULL, TYPE_MAC_VIA);
+ dinfo = drive_get(IF_MTD, 0, 0);
+ if (dinfo) {
+ qdev_prop_set_drive(via_dev, "drive", blk_by_legacy_dinfo(dinfo),
+ &error_abort);
+ }
qdev_init_nofail(via_dev);
sysbus = SYS_BUS_DEVICE(via_dev);
sysbus_mmio_map(sysbus, 0, VIA_BASE);
diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index e5658af922..e9e6a95eab 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -27,6 +27,8 @@
#include "sysemu/runstate.h"
#include "qapi/error.h"
#include "qemu/cutils.h"
+#include "hw/qdev-properties.h"
+#include "sysemu/block-backend.h"
#include "trace.h"
/*
@@ -375,6 +377,15 @@ static void via2_irq_request(void *opaque, int irq, int level)
mdc->update_irq(s);
}
+
+static void pram_update(MacVIAState *m)
+{
+ if (m->blk) {
+ blk_pwrite(m->blk, 0, m->mos6522_via1.PRAM,
+ sizeof(m->mos6522_via1.PRAM), 0);
+ }
+}
+
/*
* RTC Commands
*
@@ -547,6 +558,7 @@ static void via1_rtc_update(MacVIAState *m)
/* PRAM address 0x00 -> 0x13 */
trace_via1_rtc_cmd_pram_write(m->cmd - REG_PRAM_ADDR, m->data_out);
v1s->PRAM[m->cmd - REG_PRAM_ADDR] = m->data_out;
+ pram_update(m);
m->cmd = REG_EMPTY;
break;
case REG_PRAM_SECT...REG_PRAM_SECT_LAST:
@@ -577,6 +589,7 @@ static void via1_rtc_update(MacVIAState *m)
g_assert(REG_PRAM_SECT <= m->cmd && m->cmd <= REG_PRAM_SECT_LAST);
sector = m->cmd - REG_PRAM_SECT;
v1s->PRAM[sector * 32 + m->alt] = m->data_out;
+ pram_update(m);
trace_via1_rtc_cmd_pram_sect_write(sector, m->alt, sector * 32 + m->alt,
m->data_out);
m->alt = REG_EMPTY;
@@ -857,6 +870,7 @@ static void mac_via_realize(DeviceState *dev, Error **errp)
MacVIAState *m = MAC_VIA(dev);
MOS6522State *ms;
struct tm tm;
+ int ret;
/* Init VIAs 1 and 2 */
sysbus_init_child_obj(OBJECT(dev), "via1", &m->mos6522_via1,
@@ -890,6 +904,28 @@ static void mac_via_realize(DeviceState *dev, Error **errp)
m->adb_poll_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, via_adb_poll, m);
m->adb_data_ready = qdev_get_gpio_in_named(dev, "via1-irq",
VIA1_IRQ_ADB_READY_BIT);
+
+ if (m->blk) {
+ int64_t len = blk_getlength(m->blk);
+ if (len < 0) {
+ error_setg_errno(errp, -len,
+ "could not get length of backing image");
+ return;
+ }
+ ret = blk_set_perm(m->blk,
+ BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE,
+ BLK_PERM_ALL, errp);
+ if (ret < 0) {
+ return;
+ }
+
+ len = blk_pread(m->blk, 0, m->mos6522_via1.PRAM,
+ sizeof(m->mos6522_via1.PRAM));
+ if (len != sizeof(m->mos6522_via1.PRAM)) {
+ error_setg(errp, "can't read PRAM contents");
+ return;
+ }
+ }
}
static void mac_via_init(Object *obj)
@@ -914,10 +950,33 @@ static void mac_via_init(Object *obj)
TYPE_ADB_BUS, DEVICE(obj), "adb.0");
}
+static void postload_update_cb(void *opaque, int running, RunState state)
+{
+ MacVIAState *m = MAC_VIA(opaque);
+
+ qemu_del_vm_change_state_handler(m->vmstate);
+ m->vmstate = NULL;
+
+ pram_update(m);
+}
+
+static int mac_via_post_load(void *opaque, int version_id)
+{
+ MacVIAState *m = MAC_VIA(opaque);
+
+ if (m->blk) {
+ m->vmstate = qemu_add_vm_change_state_handler(postload_update_cb,
+ m);
+ }
+
+ return 0;
+}
+
static const VMStateDescription vmstate_mac_via = {
.name = "mac-via",
.version_id = 1,
.minimum_version_id = 1,
+ .post_load = mac_via_post_load,
.fields = (VMStateField[]) {
/* VIAs */
VMSTATE_STRUCT(mos6522_via1.parent_obj, MacVIAState, 0, vmstate_mos6522,
@@ -950,6 +1009,11 @@ static const VMStateDescription vmstate_mac_via = {
}
};
+static Property mac_via_properties[] = {
+ DEFINE_PROP_DRIVE("drive", MacVIAState, blk),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
static void mac_via_class_init(ObjectClass *oc, void *data)
{
DeviceClass *dc = DEVICE_CLASS(oc);
@@ -957,6 +1021,7 @@ static void mac_via_class_init(ObjectClass *oc, void *data)
dc->realize = mac_via_realize;
dc->reset = mac_via_reset;
dc->vmsd = &vmstate_mac_via;
+ dc->props = mac_via_properties;
}
static TypeInfo mac_via_info = {
diff --git a/include/hw/misc/mac_via.h b/include/hw/misc/mac_via.h
index 3f86fcb7e1..e74f85be0f 100644
--- a/include/hw/misc/mac_via.h
+++ b/include/hw/misc/mac_via.h
@@ -81,6 +81,8 @@ typedef struct MOS6522Q800VIA2State {
typedef struct MacVIAState {
SysBusDevice busdev;
+ VMChangeStateEntry *vmstate;
+
/* MMIO */
MemoryRegion mmio;
MemoryRegion via1mem;
@@ -100,6 +102,7 @@ typedef struct MacVIAState {
uint8_t cmd;
int wprotect;
int alt;
+ BlockBackend *blk;
/* ADB */
ADBBusState adb_bus;
--
2.24.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] q800: fix mac_via RTC PRAM commands
2019-12-19 20:14 ` [PATCH 1/2] q800: fix mac_via RTC PRAM commands Laurent Vivier
@ 2019-12-22 11:07 ` Mark Cave-Ayland
0 siblings, 0 replies; 4+ messages in thread
From: Mark Cave-Ayland @ 2019-12-22 11:07 UTC (permalink / raw)
To: Laurent Vivier, qemu-devel; +Cc: Philippe Mathieu-Daudé
On 19/12/2019 20:14, Laurent Vivier wrote:
> The command byte is not decoded correctly.
>
> This patch reworks the RTC/PRAM interface and fixes the problem.
> It adds a comment before the function to explain how are encoded commands
> and some trace-events to ease debugging.
>
> Bug: https://bugs.launchpad.net/qemu/+bug/1856549
> Fixes: 6dca62a000 ("hw/m68k: add VIA support")
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
> hw/misc/mac_via.c | 274 ++++++++++++++++++++++++++++++-------------
> hw/misc/trace-events | 19 +++
> 2 files changed, 210 insertions(+), 83 deletions(-)
>
> diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
> index f3f130ad96..e5658af922 100644
> --- a/hw/misc/mac_via.c
> +++ b/hw/misc/mac_via.c
> @@ -27,7 +27,7 @@
> #include "sysemu/runstate.h"
> #include "qapi/error.h"
> #include "qemu/cutils.h"
> -
> +#include "trace.h"
>
> /*
> * VIAs: There are two in every machine,
> @@ -278,6 +278,21 @@
> /* VIA returns time offset from Jan 1, 1904, not 1970 */
> #define RTC_OFFSET 2082844800
>
> +enum {
> + REG_0,
> + REG_1,
> + REG_2,
> + REG_3,
> + REG_TEST,
> + REG_WPROTECT,
> + REG_PRAM_ADDR,
> + REG_PRAM_ADDR_LAST = REG_PRAM_ADDR + 19,
> + REG_PRAM_SECT,
> + REG_PRAM_SECT_LAST = REG_PRAM_SECT + 7,
> + REG_INVALID,
> + REG_EMPTY = 0xff,
> +};
> +
> static void via1_VBL_update(MOS6522Q800VIA1State *v1s)
> {
> MOS6522State *s = MOS6522(v1s);
> @@ -360,10 +375,62 @@ static void via2_irq_request(void *opaque, int irq, int level)
> mdc->update_irq(s);
> }
>
> +/*
> + * RTC Commands
> + *
> + * Command byte Register addressed by the command
> + *
> + * z0000001 Seconds register 0 (lowest-order byte)
> + * z0000101 Seconds register 1
> + * z0001001 Seconds register 2
> + * z0001101 Seconds register 3 (highest-order byte)
> + * 00110001 Test register (write-only)
> + * 00110101 Write-Protect Register (write-only)
> + * z010aa01 RAM address 100aa ($10-$13) (first 20 bytes only)
> + * z1aaaa01 RAM address 0aaaa ($00-$0F) (first 20 bytes only)
> + * z0111aaa Extended memory designator and sector number
> + *
> + * For a read request, z=1, for a write z=0
> + * The letter a indicates bits whose value depend on what parameter
> + * RAM byte you want to address
> + */
> +static int via1_rtc_compact_cmd(uint8_t value)
> +{
> + uint8_t read = value & 0x80;
> +
> + value &= 0x7f;
> +
> + /* the last 2 bits of a command byte must always be 0b01 ... */
> + if ((value & 0x78) == 0x38) {
> + /* except for the extended memory designator */
> + return read | (REG_PRAM_SECT + (value & 0x07));
> + }
> + if ((value & 0x03) == 0x01) {
> + value >>= 2;
> + if ((value & 0x1c) == 0) {
> + /* seconds registers */
> + return read | (REG_0 + (value & 0x03));
> + } else if ((value == 0x0c) && !read) {
> + return REG_TEST;
> + } else if ((value == 0x0d) && !read) {
> + return REG_WPROTECT;
> + } else if ((value & 0x1c) == 0x08) {
> + /* RAM address 0x10 to 0x13 */
> + return read | (REG_PRAM_ADDR + 0x10 + (value & 0x03));
> + } else if ((value & 0x43) == 0x41) {
> + /* RAM address 0x00 to 0x0f */
> + return read | (REG_PRAM_ADDR + (value & 0x0f));
> + }
> + }
> + return REG_INVALID;
> +}
> +
> static void via1_rtc_update(MacVIAState *m)
> {
> MOS6522Q800VIA1State *v1s = &m->mos6522_via1;
> MOS6522State *s = MOS6522(v1s);
> + int cmd, sector, addr;
> + uint32_t time;
>
> if (s->b & VIA1B_vRTCEnb) {
> return;
> @@ -376,7 +443,9 @@ static void via1_rtc_update(MacVIAState *m)
> m->data_out |= s->b & VIA1B_vRTCData;
> m->data_out_cnt++;
> }
> + trace_via1_rtc_update_data_out(m->data_out_cnt, m->data_out);
> } else {
> + trace_via1_rtc_update_data_in(m->data_in_cnt, m->data_in);
> /* receive bits from the RTC */
> if ((v1s->last_b & VIA1B_vRTCClk) &&
> !(s->b & VIA1B_vRTCClk) &&
> @@ -386,96 +455,132 @@ static void via1_rtc_update(MacVIAState *m)
> m->data_in <<= 1;
> m->data_in_cnt--;
> }
> + return;
> }
>
> - if (m->data_out_cnt == 8) {
> - m->data_out_cnt = 0;
> -
> - if (m->cmd == 0) {
> - if (m->data_out & 0x80) {
> - /* this is a read command */
> - uint32_t time = m->tick_offset +
> - (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) /
> - NANOSECONDS_PER_SECOND);
> - if (m->data_out == 0x81) { /* seconds register 0 */
> - m->data_in = time & 0xff;
> - m->data_in_cnt = 8;
> - } else if (m->data_out == 0x85) { /* seconds register 1 */
> - m->data_in = (time >> 8) & 0xff;
> - m->data_in_cnt = 8;
> - } else if (m->data_out == 0x89) { /* seconds register 2 */
> - m->data_in = (time >> 16) & 0xff;
> - m->data_in_cnt = 8;
> - } else if (m->data_out == 0x8d) { /* seconds register 3 */
> - m->data_in = (time >> 24) & 0xff;
> - m->data_in_cnt = 8;
> - } else if ((m->data_out & 0xf3) == 0xa1) {
> - /* PRAM address 0x10 -> 0x13 */
> - int addr = (m->data_out >> 2) & 0x03;
> - m->data_in = v1s->PRAM[addr];
> - m->data_in_cnt = 8;
> - } else if ((m->data_out & 0xf3) == 0xa1) {
> - /* PRAM address 0x00 -> 0x0f */
> - int addr = (m->data_out >> 2) & 0x0f;
> - m->data_in = v1s->PRAM[addr];
> - m->data_in_cnt = 8;
> - } else if ((m->data_out & 0xf8) == 0xb8) {
> - /* extended memory designator and sector number */
> - m->cmd = m->data_out;
> - }
> - } else {
> - /* this is a write command */
> - m->cmd = m->data_out;
> + if (m->data_out_cnt != 8) {
> + return;
> + }
> +
> + m->data_out_cnt = 0;
> +
> + trace_via1_rtc_internal_status(m->cmd, m->alt, m->data_out);
> + /* first byte: it's a command */
> + if (m->cmd == REG_EMPTY) {
> +
> + cmd = via1_rtc_compact_cmd(m->data_out);
> + trace_via1_rtc_internal_cmd(cmd);
> +
> + if (cmd == REG_INVALID) {
> + trace_via1_rtc_cmd_invalid(m->data_out);
> + return;
> + }
> +
> + if (cmd & 0x80) { /* this is a read command */
> + switch (cmd & 0x7f) {
> + case REG_0...REG_3: /* seconds registers */
> + /*
> + * register 0 is lowest-order byte
> + * register 3 is highest-order byte
> + */
> +
> + time = m->tick_offset + (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)
> + / NANOSECONDS_PER_SECOND);
> + trace_via1_rtc_internal_time(time);
> + m->data_in = (time >> ((cmd & 0x03) << 3)) & 0xff;
> + m->data_in_cnt = 8;
> + trace_via1_rtc_cmd_seconds_read((cmd & 0x7f) - REG_0,
> + m->data_in);
> + break;
> + case REG_PRAM_ADDR...REG_PRAM_ADDR_LAST:
> + /* PRAM address 0x00 -> 0x13 */
> + m->data_in = v1s->PRAM[(cmd & 0x7f) - REG_PRAM_ADDR];
> + m->data_in_cnt = 8;
> + trace_via1_rtc_cmd_pram_read((cmd & 0x7f) - REG_PRAM_ADDR,
> + m->data_in);
> + break;
> + case REG_PRAM_SECT...REG_PRAM_SECT_LAST:
> + /*
> + * extended memory designator and sector number
> + * the only two-byte read command
> + */
> + trace_via1_rtc_internal_set_cmd(cmd);
> + m->cmd = cmd;
> + break;
> + default:
> + g_assert_not_reached();
> + break;
> }
> + return;
> + }
> +
> + /* this is a write command, needs a parameter */
> + if (cmd == REG_WPROTECT || !m->wprotect) {
> + trace_via1_rtc_internal_set_cmd(cmd);
> + m->cmd = cmd;
> } else {
> + trace_via1_rtc_internal_ignore_cmd(cmd);
> + }
> + return;
> + }
> +
> + /* second byte: it's a parameter */
> + if (m->alt == REG_EMPTY) {
> + switch (m->cmd & 0x7f) {
> + case REG_0...REG_3: /* seconds register */
> + /* FIXME */
> + trace_via1_rtc_cmd_seconds_write(m->cmd - REG_0, m->data_out);
> + m->cmd = REG_EMPTY;
> + break;
> + case REG_TEST:
> + /* device control: nothing to do */
> + trace_via1_rtc_cmd_test_write(m->data_out);
> + m->cmd = REG_EMPTY;
> + break;
> + case REG_WPROTECT:
> + /* Write Protect register */
> + trace_via1_rtc_cmd_wprotect_write(m->data_out);
> + m->wprotect = !!(m->data_out & 0x80);
> + m->cmd = REG_EMPTY;
> + break;
> + case REG_PRAM_ADDR...REG_PRAM_ADDR_LAST:
> + /* PRAM address 0x00 -> 0x13 */
> + trace_via1_rtc_cmd_pram_write(m->cmd - REG_PRAM_ADDR, m->data_out);
> + v1s->PRAM[m->cmd - REG_PRAM_ADDR] = m->data_out;
> + m->cmd = REG_EMPTY;
> + break;
> + case REG_PRAM_SECT...REG_PRAM_SECT_LAST:
> + addr = (m->data_out >> 2) & 0x1f;
> + sector = (m->cmd & 0x7f) - REG_PRAM_SECT;
> if (m->cmd & 0x80) {
> - if ((m->cmd & 0xf8) == 0xb8) {
> - /* extended memory designator and sector number */
> - int sector = m->cmd & 0x07;
> - int addr = (m->data_out >> 2) & 0x1f;
> -
> - m->data_in = v1s->PRAM[sector * 8 + addr];
> - m->data_in_cnt = 8;
> - }
> - } else if (!m->wprotect) {
> - /* this is a write command */
> - if (m->alt != 0) {
> - /* extended memory designator and sector number */
> - int sector = m->cmd & 0x07;
> - int addr = (m->alt >> 2) & 0x1f;
> -
> - v1s->PRAM[sector * 8 + addr] = m->data_out;
> -
> - m->alt = 0;
> - } else if (m->cmd == 0x01) { /* seconds register 0 */
> - /* FIXME */
> - } else if (m->cmd == 0x05) { /* seconds register 1 */
> - /* FIXME */
> - } else if (m->cmd == 0x09) { /* seconds register 2 */
> - /* FIXME */
> - } else if (m->cmd == 0x0d) { /* seconds register 3 */
> - /* FIXME */
> - } else if (m->cmd == 0x31) {
> - /* Test Register */
> - } else if (m->cmd == 0x35) {
> - /* Write Protect register */
> - m->wprotect = m->data_out & 1;
> - } else if ((m->cmd & 0xf3) == 0xa1) {
> - /* PRAM address 0x10 -> 0x13 */
> - int addr = (m->cmd >> 2) & 0x03;
> - v1s->PRAM[addr] = m->data_out;
> - } else if ((m->cmd & 0xf3) == 0xa1) {
> - /* PRAM address 0x00 -> 0x0f */
> - int addr = (m->cmd >> 2) & 0x0f;
> - v1s->PRAM[addr] = m->data_out;
> - } else if ((m->cmd & 0xf8) == 0xb8) {
> - /* extended memory designator and sector number */
> - m->alt = m->cmd;
> - }
> + /* it's a read */
> + m->data_in = v1s->PRAM[sector * 32 + addr];
> + m->data_in_cnt = 8;
> + trace_via1_rtc_cmd_pram_sect_read(sector, addr,
> + sector * 32 + addr,
> + m->data_in);
> + m->cmd = REG_EMPTY;
> + } else {
> + /* it's a write, we need one more parameter */
> + trace_via1_rtc_internal_set_alt(addr, sector, addr);
> + m->alt = addr;
> }
> + break;
> + default:
> + g_assert_not_reached();
> + break;
> }
> - m->data_out = 0;
> + return;
> }
> +
> + /* third byte: it's the data of a REG_PRAM_SECT write */
> + g_assert(REG_PRAM_SECT <= m->cmd && m->cmd <= REG_PRAM_SECT_LAST);
> + sector = m->cmd - REG_PRAM_SECT;
> + v1s->PRAM[sector * 32 + m->alt] = m->data_out;
> + trace_via1_rtc_cmd_pram_sect_write(sector, m->alt, sector * 32 + m->alt,
> + m->data_out);
> + m->alt = REG_EMPTY;
> + m->cmd = REG_EMPTY;
> }
>
> static int adb_via_poll(MacVIAState *s, int state, uint8_t *data)
> @@ -742,6 +847,9 @@ static void mac_via_reset(DeviceState *dev)
> v1s->next_VBL = 0;
> timer_del(v1s->one_second_timer);
> v1s->next_second = 0;
> +
> + m->cmd = REG_EMPTY;
> + m->alt = REG_EMPTY;
> }
>
> static void mac_via_realize(DeviceState *dev, Error **errp)
> diff --git a/hw/misc/trace-events b/hw/misc/trace-events
> index 1deb1d08c1..2e0c820834 100644
> --- a/hw/misc/trace-events
> +++ b/hw/misc/trace-events
> @@ -149,3 +149,22 @@ bcm2835_mbox_write(unsigned int size, uint64_t addr, uint64_t value) "mbox write
> bcm2835_mbox_read(unsigned int size, uint64_t addr, uint64_t value) "mbox read sz:%u addr:0x%"PRIx64" data:0x%"PRIx64
> bcm2835_mbox_irq(unsigned level) "mbox irq:ARM level:%u"
> bcm2835_mbox_property(uint32_t tag, uint32_t bufsize, size_t resplen) "mbox property tag:0x%08x in_sz:%u out_sz:%zu"
> +
> +# mac_via.c
> +via1_rtc_update_data_out(int count, int value) "count=%d value=0x%02x"
> +via1_rtc_update_data_in(int count, int value) "count=%d value=0x%02x"
> +via1_rtc_internal_status(int cmd, int alt, int value) "cmd=0x%02x alt=0x%02x value=0x%02x"
> +via1_rtc_internal_cmd(int cmd) "cmd=0x%02x"
> +via1_rtc_cmd_invalid(int value) "value=0x%02x"
> +via1_rtc_internal_time(uint32_t time) "time=0x%08x"
> +via1_rtc_internal_set_cmd(int cmd) "cmd=0x%02x"
> +via1_rtc_internal_ignore_cmd(int cmd) "cmd=0x%02x"
> +via1_rtc_internal_set_alt(int alt, int sector, int offset) "alt=0x%02x sector=%u offset=%u"
> +via1_rtc_cmd_seconds_read(int reg, int value) "reg=%d value=0x%02x"
> +via1_rtc_cmd_seconds_write(int reg, int value) "reg=%d value=0x%02x"
> +via1_rtc_cmd_test_write(int value) "value=0x%02x"
> +via1_rtc_cmd_wprotect_write(int value) "value=0x%02x"
> +via1_rtc_cmd_pram_read(int addr, int value) "addr=%u value=0x%02x"
> +via1_rtc_cmd_pram_write(int addr, int value) "addr=%u value=0x%02x"
> +via1_rtc_cmd_pram_sect_read(int sector, int offset, int addr, int value) "sector=%u offset=%u addr=%d value=0x%02x"
> +via1_rtc_cmd_pram_sect_write(int sector, int offset, int addr, int value) "sector=%u offset=%u addr=%d value=0x%02x"
Seems like a good improvement to me.
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
ATB,
Mark.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-12-22 11:11 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-12-19 20:14 [PATCH 0/2] q800: fix and improve RTC/PRAM interface Laurent Vivier
2019-12-19 20:14 ` [PATCH 1/2] q800: fix mac_via RTC PRAM commands Laurent Vivier
2019-12-22 11:07 ` Mark Cave-Ayland
2019-12-19 20:14 ` [PATCH 2/2] q800: add a block backend to the PRAM Laurent Vivier
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).