Anthony Liguori <anth...@codemonkey.ws> writes: > peter.crosthwa...@xilinx.com writes: > >> From: "Peter A. G. Crosthwaite" <peter.crosthwa...@petalogix.com> >> >> Minimal device model for devcfg module of Zynq. DMA capabilities and >> interrupt generation supported. >> >> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwa...@petalogix.com> >> --- >> Changed since v2: >> Some QOM styling updates. >> Re-implemented nw0 for lock register as pre_write >> Changed since v1: >> Rebased against new version of Register API. >> Use action callbacks for side effects rather than switch. >> Documented reasons for ge0, ge1 (Verbatim from TRM) >> Added ui1 definitions for unimplemented major features >> Removed dead lock code >> >> default-configs/arm-softmmu.mak | 1 + >> hw/dma/Makefile.objs | 1 + >> hw/dma/xilinx_devcfg.c | 495 >> ++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 497 insertions(+) >> create mode 100644 hw/dma/xilinx_devcfg.c >> >> diff --git a/default-configs/arm-softmmu.mak >> b/default-configs/arm-softmmu.mak >> index 27cbe3d..5a17f64 100644 >> --- a/default-configs/arm-softmmu.mak >> +++ b/default-configs/arm-softmmu.mak >> @@ -61,6 +61,7 @@ CONFIG_PXA2XX=y >> CONFIG_BITBANG_I2C=y >> CONFIG_FRAMEBUFFER=y >> CONFIG_XILINX_SPIPS=y >> +CONFIG_ZYNQ_DEVCFG=y >> >> CONFIG_A9SCU=y >> CONFIG_MARVELL_88W8618=y >> diff --git a/hw/dma/Makefile.objs b/hw/dma/Makefile.objs >> index 0e65ed0..96025cf 100644 >> --- a/hw/dma/Makefile.objs >> +++ b/hw/dma/Makefile.objs >> @@ -5,6 +5,7 @@ common-obj-$(CONFIG_PL330) += pl330.o >> common-obj-$(CONFIG_I82374) += i82374.o >> common-obj-$(CONFIG_I8257) += i8257.o >> common-obj-$(CONFIG_XILINX_AXI) += xilinx_axidma.o >> +common-obj-$(CONFIG_ZYNQ_DEVCFG) += xilinx_devcfg.o >> common-obj-$(CONFIG_ETRAXFS) += etraxfs_dma.o >> common-obj-$(CONFIG_STP2000) += sparc32_dma.o >> common-obj-$(CONFIG_SUN4M) += sun4m_iommu.o >> diff --git a/hw/dma/xilinx_devcfg.c b/hw/dma/xilinx_devcfg.c >> new file mode 100644 >> index 0000000..b82b7cc >> --- /dev/null >> +++ b/hw/dma/xilinx_devcfg.c >> @@ -0,0 +1,495 @@ >> +/* >> + * QEMU model of the Xilinx Devcfg Interface >> + * >> + * Copyright (c) 2011 Peter A.G. Crosthwaite >> (peter.crosthwa...@petalogix.com) >> + * >> + * Permission is hereby granted, free of charge, to any person obtaining a >> copy >> + * of this software and associated documentation files (the "Software"), to >> deal >> + * in the Software without restriction, including without limitation the >> rights >> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell >> + * copies of the Software, and to permit persons to whom the Software is >> + * furnished to do so, subject to the following conditions: >> + * >> + * The above copyright notice and this permission notice shall be included >> in >> + * all copies or substantial portions of the Software. >> + * >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS >> OR >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL >> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR >> OTHER >> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING >> FROM, >> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN >> + * THE SOFTWARE. >> + */ >> + >> +#include "sysemu/sysemu.h" >> +#include "sysemu/dma.h" >> +#include "hw/sysbus.h" >> +#include "hw/ptimer.h" >> +#include "qemu/bitops.h" >> +#include "hw/register.h" >> +#include "qemu/bitops.h" >> + >> +#define TYPE_XILINX_DEVCFG "xlnx.ps7-dev-cfg" >> + >> +#define XILINX_DEVCFG(obj) \ >> + OBJECT_CHECK(XilinxDevcfg, (obj), TYPE_XILINX_DEVCFG) >> + >> +/* FIXME: get rid of hardcoded nastiness */ >> + >> +#define FREQ_HZ 900000000 >> + >> +#define BTT_MAX 0x400 /* bytes to transfer per delay inteval */ >> +#define CYCLES_BTT_MAX 10000 /*approximate 10k cycles per delay interval */ >> + >> +#ifndef XILINX_DEVCFG_ERR_DEBUG >> +#define XILINX_DEVCFG_ERR_DEBUG 0 >> +#endif >> +#define DB_PRINT(...) do { \ >> + if (XILINX_DEVCFG_ERR_DEBUG) { \ >> + fprintf(stderr, ": %s: ", __func__); \ >> + fprintf(stderr, ## __VA_ARGS__); \ >> + } \ >> +} while (0); >> + >> +#define R_CTRL (0x00/4) >> + #define FORCE_RST (1 << 31) /* Not supported, writes ignored >> */ >> + #define PCAP_PR (1 << 27) /* Forced to 0 on bad unlock */ >> + #define PCAP_MODE (1 << 26) >> + #define MULTIBOOT_EN (1 << 24) >> + #define USER_MODE (1 << 15) >> + #define PCFG_AES_FUSE (1 << 12) /* locked by AES_FUSE_LOCK */ >> + #define PCFG_AES_EN_SHIFT 9 /* locked by AES_EN_LOCK */ >> + #define PCFG_AES_EN_LEN 3 /* locked by AES_EN_LOCK */ >> + #define PCFG_AES_EN_MASK (((1 << PCFG_AES_EN_LEN) - 1) \ >> + << PCFG_AES_EN_SHIFT) >> + #define SEU_EN (1 << 8) /* locked by SEU_LOCK */ >> + #define SEC_EN (1 << 7) /* locked by SEC_LOCK */ >> + #define SPNIDEN (1 << 6) /* locked by DBG_LOCK */ >> + #define SPIDEN (1 << 5) /* locked by DBG_LOCK */ >> + #define NIDEN (1 << 4) /* locked by DBG_LOCK */ >> + #define DBGEN (1 << 3) /* locked by DBG_LOCK */ >> + #define DAP_EN (7 << 0) /* locked by DBG_LOCK */ >> + >> +#define R_LOCK (0x04/4) >> + #define AES_FUSE_LOCK 4 >> + #define AES_EN_LOCK 3 >> + #define SEU_LOCK 2 >> + #define SEC_LOCK 1 >> + #define DBG_LOCK 0 >> + >> +/* mapping bits in R_LOCK to what they lock in R_CTRL */ >> +static const uint32_t lock_ctrl_map[] = { >> + [AES_FUSE_LOCK] = PCFG_AES_FUSE, >> + [AES_EN_LOCK] = PCFG_AES_EN_MASK, >> + [SEU_LOCK] = SEU_LOCK, >> + [SEC_LOCK] = SEC_LOCK, >> + [DBG_LOCK] = SPNIDEN | SPIDEN | NIDEN | DBGEN | DAP_EN, >> +}; >> + >> +#define R_CFG (0x08/4) >> + #define RFIFO_TH_SHIFT 10 >> + #define RFIFO_TH_LEN 2 >> + #define WFIFO_TH_SHIFT 8 >> + #define WFIFO_TH_LEN 2 >> + #define DISABLE_SRC_INC (1 << 5) >> + #define DISABLE_DST_INC (1 << 4) >> +#define R_CFG_RO 0xFFFFF800 >> +#define R_CFG_RESET 0x50B >> + >> +#define R_INT_STS (0x0C/4) >> + #define PSS_GTS_USR_B_INT (1 << 31) >> + #define PSS_FST_CFG_B_INT (1 << 30) >> + #define PSS_CFG_RESET_B_INT (1 << 27) >> + #define RX_FIFO_OV_INT (1 << 18) >> + #define WR_FIFO_LVL_INT (1 << 17) >> + #define RD_FIFO_LVL_INT (1 << 16) >> + #define DMA_CMD_ERR_INT (1 << 15) >> + #define DMA_Q_OV_INT (1 << 14) >> + #define DMA_DONE_INT (1 << 13) >> + #define DMA_P_DONE_INT (1 << 12) >> + #define P2D_LEN_ERR_INT (1 << 11) >> + #define PCFG_DONE_INT (1 << 2) >> + #define R_INT_STS_RSVD ((0x7 << 24) | (0x1 << 19) | (0xF < 7)) >> + >> +#define R_INT_MASK (0x10/4) >> + >> +#define R_STATUS (0x14/4) >> + #define DMA_CMD_Q_F (1 << 31) >> + #define DMA_CMD_Q_E (1 << 30) >> + #define DMA_DONE_CNT_SHIFT 28 >> + #define DMA_DONE_CNT_LEN 2 >> + #define RX_FIFO_LVL_SHIFT 20 >> + #define RX_FIFO_LVL_LEN 5 >> + #define TX_FIFO_LVL_SHIFT 12 >> + #define TX_FIFO_LVL_LEN 7 >> + #define TX_FIFO_LVL (0x7f << 12) >> + #define PSS_GTS_USR_B (1 << 11) >> + #define PSS_FST_CFG_B (1 << 10) >> + #define PSS_CFG_RESET_B (1 << 5) >> + >> +#define R_DMA_SRC_ADDR (0x18/4) >> +#define R_DMA_DST_ADDR (0x1C/4) >> +#define R_DMA_SRC_LEN (0x20/4) >> +#define R_DMA_DST_LEN (0x24/4) >> +#define R_ROM_SHADOW (0x28/4) >> +#define R_SW_ID (0x30/4) >> +#define R_UNLOCK (0x34/4) >> + >> +#define R_UNLOCK_MAGIC 0x757BDF0D >> + >> +#define R_MCTRL (0x80/4) >> + #define PS_VERSION_SHIFT 28 >> + #define PS_VERSION_MASK (0xf << PS_VERSION_SHIFT) >> + #define PCFG_POR_B (1 << 8) >> + #define INT_PCAP_LPBK (1 << 4) >> + >> +#define R_MAX (0x118/4+1) >> + >> +#define RX_FIFO_LEN 32 >> +#define TX_FIFO_LEN 128 >> + >> +#define DMA_COMMAND_FIFO_LEN 10 >> + >> +typedef struct XilinxDevcfgDMACommand { >> + uint32_t src_addr; >> + uint32_t dest_addr; >> + uint32_t src_len; >> + uint32_t dest_len; >> +} XilinxDevcfgDMACommand; >> + >> +static const VMStateDescription vmstate_xilinx_devcfg_dma_command = { >> + .name = "xilinx_devcfg_dma_command", >> + .version_id = 1, >> + .minimum_version_id = 1, >> + .minimum_version_id_old = 1, >> + .fields = (VMStateField[]) { >> + VMSTATE_UINT32(src_addr, XilinxDevcfgDMACommand), >> + VMSTATE_UINT32(dest_addr, XilinxDevcfgDMACommand), >> + VMSTATE_UINT32(src_len, XilinxDevcfgDMACommand), >> + VMSTATE_UINT32(dest_len, XilinxDevcfgDMACommand), >> + VMSTATE_END_OF_LIST() >> + } >> +}; >> + >> +typedef struct XilinxDevcfg { >> + SysBusDevice parent_obj; >> + >> + MemoryRegion iomem; >> + qemu_irq irq; >> + >> + QEMUBH *timer_bh; >> + ptimer_state *timer; >> + >> + XilinxDevcfgDMACommand dma_command_fifo[DMA_COMMAND_FIFO_LEN]; >> + uint8_t dma_command_fifo_num; >> + >> + uint32_t regs[R_MAX]; >> + RegisterInfo regs_info[R_MAX]; >> +} XilinxDevcfg; >> + >> +static const VMStateDescription vmstate_xilinx_devcfg = { >> + .name = "xilinx_devcfg", >> + .version_id = 1, >> + .minimum_version_id = 1, >> + .minimum_version_id_old = 1, >> + .fields = (VMStateField[]) { >> + VMSTATE_PTIMER(timer, XilinxDevcfg), >> + VMSTATE_STRUCT_ARRAY(dma_command_fifo, XilinxDevcfg, >> + DMA_COMMAND_FIFO_LEN, 0, >> + vmstate_xilinx_devcfg_dma_command, >> + XilinxDevcfgDMACommand), >> + VMSTATE_UINT8(dma_command_fifo_num, XilinxDevcfg), >> + VMSTATE_UINT32_ARRAY(regs, XilinxDevcfg, R_MAX), >> + VMSTATE_END_OF_LIST() >> + } >> +}; >> + >> +static void xilinx_devcfg_update_ixr(XilinxDevcfg *s) >> +{ >> + qemu_set_irq(s->irq, !!(~s->regs[R_INT_MASK] & s->regs[R_INT_STS])); >> +} >> + >> +static void xilinx_devcfg_reset(DeviceState *dev) >> +{ >> + XilinxDevcfg *s = XILINX_DEVCFG(dev); >> + int i; >> + >> + for (i = 0; i < R_MAX; ++i) { >> + register_reset(&s->regs_info[i]); >> + } >> +} >> + >> +#define min(a, b) ((a) < (b) ? (a) : (b)) >> + >> +static void xilinx_devcfg_dma_go(void *opaque) >> +{ >> + XilinxDevcfg *s = XILINX_DEVCFG(opaque); >> + uint8_t buf[BTT_MAX]; >> + XilinxDevcfgDMACommand *dmah = s->dma_command_fifo; >> + uint32_t btt = BTT_MAX; >> + >> + btt = min(btt, dmah->src_len); >> + if (s->regs[R_MCTRL] & INT_PCAP_LPBK) { >> + btt = min(btt, dmah->dest_len); >> + } >> + DB_PRINT("reading %x bytes from %x\n", btt, dmah->src_addr); >> + dma_memory_read(&dma_context_memory, dmah->src_addr, buf, btt); >> + dmah->src_len -= btt; >> + dmah->src_addr += btt; >> + if (s->regs[R_MCTRL] & INT_PCAP_LPBK) { >> + DB_PRINT("writing %x bytes from %x\n", btt, dmah->dest_addr); >> + dma_memory_write(&dma_context_memory, dmah->dest_addr, buf, btt); >> + dmah->dest_len -= btt; >> + dmah->dest_addr += btt; >> + } >> + if (!dmah->src_len && !dmah->dest_len) { >> + DB_PRINT("dma operation finished\n"); >> + s->regs[R_INT_STS] |= DMA_DONE_INT | DMA_P_DONE_INT; >> + s->dma_command_fifo_num = s->dma_command_fifo_num - 1; >> + memcpy(s->dma_command_fifo, &s->dma_command_fifo[1], >> + sizeof(*s->dma_command_fifo) * DMA_COMMAND_FIFO_LEN - 1); >> + } >> + xilinx_devcfg_update_ixr(s); >> + if (s->dma_command_fifo_num) { /* there is still work to do */ >> + DB_PRINT("dma work remains, setting up callback ptimer\n"); >> + ptimer_set_freq(s->timer, FREQ_HZ); >> + ptimer_set_count(s->timer, CYCLES_BTT_MAX); >> + ptimer_run(s->timer, 1); >> + } >> +} >> + >> +static void r_ixr_post_write(RegisterInfo *reg, uint64_t val) >> +{ >> + XilinxDevcfg *s = XILINX_DEVCFG(reg->opaque); >> + >> + xilinx_devcfg_update_ixr(s); >> +} >> + >> +static uint64_t r_ctrl_pre_write(RegisterInfo *reg, uint64_t val) >> +{ >> + XilinxDevcfg *s = XILINX_DEVCFG(reg->opaque); >> + int i; >> + >> + for (i = 0; i < ARRAY_SIZE(lock_ctrl_map); ++i) { >> + if (s->regs[R_LOCK] & 1 << i) { >> + val &= ~lock_ctrl_map[i]; >> + val |= lock_ctrl_map[i] & s->regs[R_CTRL]; >> + } >> + } >> + return val; >> +} >> + >> +static void r_ctrl_post_write(RegisterInfo *reg, uint64_t val) >> +{ >> + uint32_t aes_en = extract32(val, PCFG_AES_EN_SHIFT, PCFG_AES_EN_LEN); >> + >> + if (aes_en != 0 && aes_en != 7) { >> + qemu_log_mask(LOG_UNIMP, "%s: warning, aes-en bits inconsistent," >> + "unimplemeneted security reset should happen!\n", >> + reg->prefix); >> + } >> +} >> + >> +static void r_unlock_post_write(RegisterInfo *reg, uint64_t val) >> +{ >> + XilinxDevcfg *s = XILINX_DEVCFG(reg->opaque); >> + >> + if (val == R_UNLOCK_MAGIC) { >> + DB_PRINT("successful unlock\n"); >> + } else {/* bad unlock attempt */ >> + qemu_log_mask(LOG_GUEST_ERROR, "%s: failed unlock\n", reg->prefix); >> + qemu_log_mask(LOG_UNIMP, "%s: failed unlock, unimplmeneted crash of" >> + "device should happen\n", reg->prefix); >> + s->regs[R_CTRL] &= ~PCAP_PR; >> + s->regs[R_CTRL] &= ~PCFG_AES_EN_MASK; >> + } >> +} >> + >> +static uint64_t r_lock_pre_write(RegisterInfo *reg, uint64_t val) >> +{ >> + XilinxDevcfg *s = XILINX_DEVCFG(reg->opaque); >> + >> + /* once bits are locked they stay locked */ >> + return s->regs[R_LOCK] | val; >> +} >> + >> +static void r_dma_dst_len_post_write(RegisterInfo *reg, uint64_t val) >> +{ >> + XilinxDevcfg *s = XILINX_DEVCFG(reg->opaque); >> + >> + /* TODO: implement command queue overflow check and interrupt */ >> + s->dma_command_fifo[s->dma_command_fifo_num].src_addr = >> + s->regs[R_DMA_SRC_ADDR] & ~0x3UL; >> + s->dma_command_fifo[s->dma_command_fifo_num].dest_addr = >> + s->regs[R_DMA_DST_ADDR] & ~0x3UL; >> + s->dma_command_fifo[s->dma_command_fifo_num].src_len = >> + s->regs[R_DMA_SRC_LEN] << 2; >> + s->dma_command_fifo[s->dma_command_fifo_num].dest_len = >> + s->regs[R_DMA_DST_LEN] << 2; >> + s->dma_command_fifo_num++; >> + DB_PRINT("dma transfer started; %d total transfers pending\n", >> + s->dma_command_fifo_num); >> + xilinx_devcfg_dma_go(s); >> +} >> + >> +static const RegisterAccessInfo xilinx_devcfg_regs_info[] = { >> + [R_CTRL] = { .name = "CTRL", >> + .reset = PCAP_PR | PCAP_MODE | 0x3 << 13, >> + .ro = 0x107f6000, >> + .ge1 = (RegisterAccessError[]) { >> + { .mask = 0x1 << 15, .reason = "Reserved - do not modify" }, >> + {}, >> + }, >> + .ge0 = (RegisterAccessError[]) { >> + { .mask = 0x3 << 13, .reason = "Reserved - always write with 1" >> }, >> + {}, >> + }, >> + .ui1 = (RegisterAccessError[]) { >> + { .mask = FORCE_RST, .reason = "PS reset not implemented" }, >> + { .mask = PCAP_MODE, .reason = "FPGA Fabric doesnt exist" }, >> + { .mask = PCFG_AES_EN_MASK, .reason = "AES not implmented" }, >> + {}, >> + }, >> + .pre_write = r_ctrl_pre_write, >> + .post_write = r_ctrl_post_write, > > I dislike that this mechanism decentralizes register access. What about > a slightly different style so you could do something like: > > static void my_device_io_write(...) > { > switch (register_decode(&my_device_reginfo, s, &info)) { > case R_CTRL: > // handle pre-write bits here > ... > } > > switch (register_write(&my_device_reginfo, s)) { > case R_CTRL: > //handle post write bits > ... > } > } > > It makes it much easier to convert existing code. We can then leave > most of the switch statements intact and just introduce register > descriptions. > > I think splitting decode from data processing is useful too because it > makes it easier to support latching.
Here's a more illustrated example. I didn't try to make anything data driven but I hope you can see how that would fit in. I think there's a lot of value in splitting decode vs. load/store. I think it's important to allow certain things to remain open coded and if you see how I did it, there are a few registers that are decoded and load/stored in an open coded fashion remaining. By separating decode, we can have universal tracing of specific register access (even if we're open coding it). I think reset is also out of scope for an API like this. Particularly when talking about latching, there isn't a 1-1 relationship anymore between things with initial values and then the visibility within the address space.
>From 8825ec58285957b8586e9439a84bbd5eca773fbb Mon Sep 17 00:00:00 2001 From: Anthony Liguori <aligu...@us.ibm.com> Date: Wed, 29 May 2013 13:44:35 -0500 Subject: [PATCH] Demonstrate separating register decode from get/set --- hw/char/serial.c | 253 ++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 167 insertions(+), 86 deletions(-) diff --git a/hw/char/serial.c b/hw/char/serial.c index 66b6348..be637f5 100644 --- a/hw/char/serial.c +++ b/hw/char/serial.c @@ -299,6 +299,113 @@ static gboolean serial_xmit(GIOChannel *chan, GIOCondition cond, void *opaque) return FALSE; } +enum { + UART_INVALID = 0, + UART_DIVIDER, + UART_RBR, + UART_THR, + UART_IER, + UART_IIR, + UART_FCR, + UART_LCR, + UART_MCR, + UART_LSR, + UART_MSR, + UART_SCR, +}; + +static int serial_decode(void *opaque, hwaddr addr, unsigned size, bool is_write) +{ + SerialState *s = opaque; + + switch (addr) { + case 0: + if (s->lcr & UART_LCR_DLAB) { + return UART_DIVIDER; + } + if (is_write) { + return UART_THR; + } + return UART_RBR; + case 1: + if (s->lcr & UART_LCR_DLAB) { + return UART_DIVIDER; + } + return UART_IER; + case 2: + if (is_write) { + return UART_FCR; + } + return UART_IIR; + case 3: + return UART_LCR; + case 4: + return UART_MCR; + case 5: + if (is_write) { + return UART_INVALID; + } + return UART_LSR; + case 6: + if (is_write) { + return UART_INVALID; + } + return UART_MSR; + case 7: + return UART_SCR; + default: + return UART_INVALID; + } +} + +static int serial_load(void *opaque, int regno, unsigned size, uint64_t *ret) +{ + SerialState *s = opaque; + + switch (regno) { + case UART_IER: + *ret = s->ier; + break; + case UART_IIR: + *ret = s->iir; + break; + case UART_LCR: + *ret = s->lcr; + break; + case UART_MCR: + *ret = s->mcr; + break; + case UART_LSR: + *ret = s->lsr; + break; + case UART_SCR: + *ret = s->scr; + break; + default: + break; + } + + return regno; +} + +static int serial_store(void *opaque, int regno, unsigned size, uint64_t val) +{ + SerialState *s = opaque; + + switch (regno) { + case UART_THR: + s->thr = val; + break; + case UART_IER: + s->ier = val & 0x0f; + break; + case UART_LCR: + s->lcr = val; + break; + } + + return regno; +} static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val, unsigned size) @@ -307,52 +414,48 @@ static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val, addr &= 7; DPRINTF("write addr=0x%" HWADDR_PRIx " val=0x%" PRIx64 "\n", addr, val); - switch(addr) { + switch(serial_store(s, serial_decode(s, addr, size, true), size, val)) { default: - case 0: - if (s->lcr & UART_LCR_DLAB) { + case UART_DIVIDER: + if (addr == 0) { s->divider = (s->divider & 0xff00) | val; - serial_update_parameters(s); } else { - s->thr = (uint8_t) val; - if(s->fcr & UART_FCR_FE) { - fifo_put(s, XMIT_FIFO, s->thr); - s->thr_ipending = 0; - s->lsr &= ~UART_LSR_TEMT; - s->lsr &= ~UART_LSR_THRE; - serial_update_irq(s); - } else { - s->thr_ipending = 0; - s->lsr &= ~UART_LSR_THRE; - serial_update_irq(s); - } - serial_xmit(NULL, G_IO_OUT, s); + s->divider = (s->divider & 0x00ff) | (val << 8); } + serial_update_parameters(s); break; - case 1: - if (s->lcr & UART_LCR_DLAB) { - s->divider = (s->divider & 0x00ff) | (val << 8); - serial_update_parameters(s); + case UART_THR: + if(s->fcr & UART_FCR_FE) { + fifo_put(s, XMIT_FIFO, s->thr); + s->thr_ipending = 0; + s->lsr &= ~UART_LSR_TEMT; + s->lsr &= ~UART_LSR_THRE; + serial_update_irq(s); } else { - s->ier = val & 0x0f; - /* If the backend device is a real serial port, turn polling of the modem - status lines on physical port on or off depending on UART_IER_MSI state */ - if (s->poll_msl >= 0) { - if (s->ier & UART_IER_MSI) { - s->poll_msl = 1; - serial_update_msl(s); - } else { - qemu_del_timer(s->modem_status_poll); - s->poll_msl = 0; - } - } - if (s->lsr & UART_LSR_THRE) { - s->thr_ipending = 1; - serial_update_irq(s); + s->thr_ipending = 0; + s->lsr &= ~UART_LSR_THRE; + serial_update_irq(s); + } + serial_xmit(NULL, G_IO_OUT, s); + break; + case UART_IER: + /* If the backend device is a real serial port, turn polling of the modem + status lines on physical port on or off depending on UART_IER_MSI state */ + if (s->poll_msl >= 0) { + if (s->ier & UART_IER_MSI) { + s->poll_msl = 1; + serial_update_msl(s); + } else { + qemu_del_timer(s->modem_status_poll); + s->poll_msl = 0; } } + if (s->lsr & UART_LSR_THRE) { + s->thr_ipending = 1; + serial_update_irq(s); + } break; - case 2: + case UART_FCR: val = val & 0xFF; if (s->fcr == val) @@ -398,10 +501,9 @@ static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val, s->fcr = val & 0xC9; serial_update_irq(s); break; - case 3: + case UART_LCR: { int break_enable; - s->lcr = val; serial_update_parameters(s); break_enable = (val >> 6) & 1; if (break_enable != s->last_break_enable) { @@ -411,7 +513,7 @@ static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val, } } break; - case 4: + case UART_MCR: { int flags; int old_mcr = s->mcr; @@ -437,75 +539,57 @@ static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val, } } break; - case 5: - break; - case 6: - break; - case 7: - s->scr = val; - break; } } static uint64_t serial_ioport_read(void *opaque, hwaddr addr, unsigned size) { SerialState *s = opaque; - uint32_t ret; + uint64_t ret = 0xFF; addr &= 7; - switch(addr) { + switch(serial_load(s, serial_decode(s, addr, size, false), size, &ret)) { default: - case 0: - if (s->lcr & UART_LCR_DLAB) { + break; + case UART_DIVIDER: + if (addr == 0) { ret = s->divider & 0xff; } else { - if(s->fcr & UART_FCR_FE) { - ret = fifo_get(s,RECV_FIFO); - if (s->recv_fifo.count == 0) - s->lsr &= ~(UART_LSR_DR | UART_LSR_BI); - else - qemu_mod_timer(s->fifo_timeout_timer, qemu_get_clock_ns (vm_clock) + s->char_transmit_time * 4); - s->timeout_ipending = 0; - } else { - ret = s->rbr; - s->lsr &= ~(UART_LSR_DR | UART_LSR_BI); - } - serial_update_irq(s); - if (!(s->mcr & UART_MCR_LOOP)) { - /* in loopback mode, don't receive any data */ - qemu_chr_accept_input(s->chr); - } + ret = (s->divider >> 8) & 0xff; } break; - case 1: - if (s->lcr & UART_LCR_DLAB) { - ret = (s->divider >> 8) & 0xff; + case UART_RBR: + if(s->fcr & UART_FCR_FE) { + ret = fifo_get(s,RECV_FIFO); + if (s->recv_fifo.count == 0) + s->lsr &= ~(UART_LSR_DR | UART_LSR_BI); + else + qemu_mod_timer(s->fifo_timeout_timer, qemu_get_clock_ns (vm_clock) + s->char_transmit_time * 4); + s->timeout_ipending = 0; } else { - ret = s->ier; + ret = s->rbr; + s->lsr &= ~(UART_LSR_DR | UART_LSR_BI); + } + serial_update_irq(s); + if (!(s->mcr & UART_MCR_LOOP)) { + /* in loopback mode, don't receive any data */ + qemu_chr_accept_input(s->chr); } break; - case 2: - ret = s->iir; - if ((ret & UART_IIR_ID) == UART_IIR_THRI) { + case UART_IIR: + if ((s->iir & UART_IIR_ID) == UART_IIR_THRI) { s->thr_ipending = 0; serial_update_irq(s); } break; - case 3: - ret = s->lcr; - break; - case 4: - ret = s->mcr; - break; - case 5: - ret = s->lsr; + case UART_LSR: /* Clear break and overrun interrupts */ if (s->lsr & (UART_LSR_BI|UART_LSR_OE)) { s->lsr &= ~(UART_LSR_BI|UART_LSR_OE); serial_update_irq(s); } break; - case 6: + case UART_MSR: if (s->mcr & UART_MCR_LOOP) { /* in loopback, the modem output pins are connected to the inputs */ @@ -523,9 +607,6 @@ static uint64_t serial_ioport_read(void *opaque, hwaddr addr, unsigned size) } } break; - case 7: - ret = s->scr; - break; } DPRINTF("read addr=0x%" HWADDR_PRIx " val=0x%02x\n", addr, ret); return ret; -- 1.8.0
Regards, Anthony Liguori > > I think the current spin is probably over generalizing too. I don't > think supporting ge0/ge1 makes a lot of sense from the start. Decisions > aren't always that simple and it's weird to have sanity checking split > across two places. > > BTW: I think it's also a good idea to model this as a QOM object so that > device state can be access through the QOM tree. > > Regards, > > Anthony Liguori > >> + }, >> + [R_LOCK] = { .name = "LOCK", >> + .ro = ~ONES(5), >> + .pre_write = r_lock_pre_write, >> + }, >> + [R_CFG] = { .name = "CFG", >> + .reset = 1 << RFIFO_TH_SHIFT | 1 << WFIFO_TH_SHIFT | 0x8, >> + .ge1 = (RegisterAccessError[]) { >> + { .mask = 0x7, .reason = "Reserved - do not modify" }, >> + {}, >> + }, >> + .ge0 = (RegisterAccessError[]) { >> + { .mask = 0x8, .reason = "Reserved - do not modify" }, >> + {}, >> + }, >> + .ro = 0x00f | ~ONES(12), >> + }, >> + [R_INT_STS] = { .name = "INT_STS", >> + .w1c = ~R_INT_STS_RSVD, >> + .reset = PSS_GTS_USR_B_INT | PSS_CFG_RESET_B_INT | WR_FIFO_LVL_INT, >> + .ro = R_INT_STS_RSVD, >> + .post_write = r_ixr_post_write, >> + }, >> + [R_INT_MASK] = { .name = "INT_MASK", >> + .reset = ~0, >> + .ro = R_INT_STS_RSVD, >> + .post_write = r_ixr_post_write, >> + }, >> + [R_STATUS] = { .name = "STATUS", >> + .reset = DMA_CMD_Q_E | PSS_GTS_USR_B | PSS_CFG_RESET_B, >> + .ro = ~0, >> + .ge1 = (RegisterAccessError[]) { >> + {.mask = 0x1, .reason = "Reserved - do not modify" }, >> + {}, >> + }, >> + }, >> + [R_DMA_SRC_ADDR] = { .name = "DMA_SRC_ADDR" }, >> + [R_DMA_DST_ADDR] = { .name = "DMA_DST_ADDR" }, >> + [R_DMA_SRC_LEN] = { .name = "DMA_SRC_LEN", .ro = ~ONES(27) }, >> + [R_DMA_DST_LEN] = { .name = "DMA_DST_LEN", >> + .ro = ~ONES(27), >> + .post_write = r_dma_dst_len_post_write, >> + }, >> + [R_ROM_SHADOW] = { .name = "ROM_SHADOW", >> + .ge1 = (RegisterAccessError[]) { >> + {.mask = ~0, .reason = "Reserved - do not modify" }, >> + {}, >> + }, >> + }, >> + [R_SW_ID] = { .name = "SW_ID" }, >> + [R_UNLOCK] = { .name = "UNLOCK", >> + .post_write = r_unlock_post_write, >> + }, >> + [R_MCTRL] = { .name = "MCTRL", >> + /* Silicon 3.0 for version field, and the mysterious reserved bit >> 23 */ >> + .reset = 0x2 << PS_VERSION_SHIFT | 1 << 23, >> + /* some reserved bits are rw while others are ro */ >> + .ro = ~INT_PCAP_LPBK, >> + .ge1 = (RegisterAccessError[]) { >> + { .mask = 0x00F00300, .reason = "Reserved - do not modify" }, >> + { .mask = 0x00000003, .reason = "Reserved - always write with >> 0" }, >> + {} >> + }, >> + .ge0 = (RegisterAccessError[]) { >> + { .mask = 1 << 23, .reason = "Reserved - always write with 1" }, >> + {} >> + }, >> + }, >> + [R_MAX] = {} >> +}; >> + >> +static const MemoryRegionOps devcfg_reg_ops = { >> + .read = register_read_memory_le, >> + .write = register_write_memory_le, >> + .endianness = DEVICE_LITTLE_ENDIAN, >> + .valid = { >> + .min_access_size = 4, >> + .max_access_size = 4, >> + } >> +}; >> + >> +static void xilinx_devcfg_realize(DeviceState *dev, Error **errp) >> +{ >> + XilinxDevcfg *s = XILINX_DEVCFG(dev); >> + const char *prefix = object_get_canonical_path(OBJECT(dev)); >> + int i; >> + >> + for (i = 0; i < R_MAX; ++i) { >> + RegisterInfo *r = &s->regs_info[i]; >> + >> + *r = (RegisterInfo) { >> + .data = &s->regs[i], >> + .data_size = sizeof(uint32_t), >> + .access = &xilinx_devcfg_regs_info[i], >> + .debug = XILINX_DEVCFG_ERR_DEBUG, >> + .prefix = prefix, >> + .opaque = s, >> + }; >> + memory_region_init_io(&r->mem, &devcfg_reg_ops, r, "devcfg-regs", >> 4); >> + memory_region_add_subregion(&s->iomem, i * 4, &r->mem); >> + } >> +} >> + >> +static void xilinx_devcfg_init(Object *obj) >> +{ >> + SysBusDevice *sbd = SYS_BUS_DEVICE(obj); >> + XilinxDevcfg *s = XILINX_DEVCFG(obj); >> + >> + s->timer_bh = qemu_bh_new(xilinx_devcfg_dma_go, s); >> + s->timer = ptimer_init(s->timer_bh); >> + >> + sysbus_init_irq(sbd, &s->irq); >> + >> + memory_region_init(&s->iomem, "devcfg", R_MAX*4); >> + sysbus_init_mmio(sbd, &s->iomem); >> +} >> + >> +static void xilinx_devcfg_class_init(ObjectClass *klass, void *data) >> +{ >> + DeviceClass *dc = DEVICE_CLASS(klass); >> + >> + dc->reset = xilinx_devcfg_reset; >> + dc->vmsd = &vmstate_xilinx_devcfg; >> + dc->realize = xilinx_devcfg_realize; >> +} >> + >> +static const TypeInfo xilinx_devcfg_info = { >> + .name = TYPE_XILINX_DEVCFG, >> + .parent = TYPE_SYS_BUS_DEVICE, >> + .instance_size = sizeof(XilinxDevcfg), >> + .instance_init = xilinx_devcfg_init, >> + .class_init = xilinx_devcfg_class_init, >> +}; >> + >> +static void xilinx_devcfg_register_types(void) >> +{ >> + type_register_static(&xilinx_devcfg_info); >> +} >> + >> +type_init(xilinx_devcfg_register_types) >> -- >> 1.8.3.rc1.44.gb387c77.dirty