From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41498) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ui8jf-0005ds-Ls for qemu-devel@nongnu.org; Thu, 30 May 2013 15:42:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ui8jY-0005eN-NB for qemu-devel@nongnu.org; Thu, 30 May 2013 15:42:07 -0400 Received: from mail-qa0-x22f.google.com ([2607:f8b0:400d:c00::22f]:58759) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ui8jY-0005eJ-GT for qemu-devel@nongnu.org; Thu, 30 May 2013 15:42:00 -0400 Received: by mail-qa0-f47.google.com with SMTP id n20so32487qaj.20 for ; Thu, 30 May 2013 12:42:00 -0700 (PDT) From: Anthony Liguori In-Reply-To: References: <87ehcppt22.fsf@codemonkey.ws> Date: Thu, 30 May 2013 14:41:56 -0500 Message-ID: <87ehcocl17.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Subject: Re: [Qemu-devel] [PATCH v3 4/5] xilinx_devcfg: Zynq devcfg device model List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Crosthwaite Cc: peter.maydell@linaro.org, qemu-devel@nongnu.org, blauwirbel@gmail.com, kraxel@redhat.com, pbonzini@redhat.com, edgar.iglesias@gmail.com --=-=-= Peter Crosthwaite writes: > Hi Anthony, > > On Thu, May 30, 2013 at 3:57 AM, Anthony Liguori wrote: >> peter.crosthwaite@xilinx.com writes: >> >>> From: "Peter A. G. Crosthwaite" >>> >>> Minimal device model for devcfg module of Zynq. DMA capabilities and >>> interrupt generation supported. >>> >>> Signed-off-by: Peter A. G. Crosthwaite >>> --- >>> 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.crosthwaite@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 >> ... >> } >> } >> > > That's still possible using just the register API (Patch 2 content > only) and throwing away the memory API glue. I think its actually > quite similar to V1 of the patch series where I did not have the > callbacks, and use the switch-cases for register side-effects only. > This looks like the intention of your patch. Gerd made a push for the > callbacks in an earlier review and there was push to integrate to > Memory API . Is it reasonable to leave it up to the developer whether > they want to do full conversion (Patches 2 & 3) or half conversion > (Patch 2 only + your decode refactoring). V1 of this patch at: > > http://patchwork.ozlabs.org/patch/224534/ > > heres the relevant snippet (comments from me inline): > > +static void xilinx_devcfg_write(void *opaque, hwaddr addr, uint64_t value, > + unsigned size) > +{ > + XilinxDevcfg *s = opaque; > + int i; > + uint32_t aes_en; > + const char *prefix = ""; > + > + /* FIXME: use tracing to avoid these gymnastics */ > + if (XILINX_DEVCFG_ERR_DEBUG || qemu_get_log_flags() & LOG_GUEST_ERROR) { > + prefix = g_strdup_printf("%s:Addr %#08x", > + object_get_canonical_path(OBJECT(s)), > + (unsigned)addr); > + } > + addr >>= 2; > > This is the open coded decode logic but is trivial in this case. But this is invisible to the common layer. I think being able to have a common layer insight into "this value is being written to this device register" would be extremely useful. But my fear is that the current proposal will not work for a large set of devices. Let me provide another example (attached below) but highlighting the description: > static RegisterDecodeEntry decoder[] = { > /* addresses 0-1 must be open decoded due to latching */ > { .addr = 2, .regno = UART_FCR, .flags = REG_WRITE }, > { .addr = 2, .regno = UART_IIR, .flags = REG_READ }, > { .addr = 3, .regno = UART_LCR, .flags = REG_RW }, > { .addr = 4, .regno = UART_MCR, .flags = REG_RW }, > { .addr = 5, .regno = UART_LSR, .flags = REG_READ }, > { .addr = 6, .regno = UART_MSR, .flags = REG_READ }, > { .addr = 7, .regno = UART_SCR, .flags = REG_RW }, > }; > > static RegisterMapEntry regmap[] = { > DEFINE_REG_U8(SerialState, ier, UART_IER), > DEFINE_REG_U8(SerialState, iir, UART_IIR), > DEFINE_REG_U8(SerialState, lcr, UART_LCR), > DEFINE_REG_U8(SerialState, mcr, UART_MCR), > DEFINE_REG_U8(SerialState, lsr, UART_LSR), > DEFINE_REG_U8(SerialState, scr, UART_SCR), > DEFINE_REG_U8(SerialState, thr, UART_THR), > }; This is a clear separation between decode logic and load/store logic. They are very different things from a hardware point of view. > + assert(addr < R_MAX); > + > + if (s->lock && addr != R_UNLOCK) { > + qemu_log_mask(LOG_GUEST_ERROR, "%s:write to non-lock register while" > + " locked\n", prefix); > + return; > + } > + > > some pre write logic (I dropped it later as it was actually unimplemented). > > + uint32_write(&s->regs[addr], &xilinx_devcfg_regs_info[addr], value, prefix, > + XILINX_DEVCFG_ERR_DEBUG); > + > > this is the data-driven register_write() call under its old name. > > + /*side effects */ > + switch (addr) { > + case R_CTRL: > + for (i = 0; i < ARRAY_SIZE(lock_ctrl_map); ++i) { > + if (s->regs[R_LOCK] & 1 << i) { > + value &= ~lock_ctrl_map[i]; > + value |= lock_ctrl_map[i] & s->regs[R_CTRL]; > + } > + } > + aes_en = extract32(value, 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", > + prefix); > + } > + break; > + > + case R_DMA_DST_LEN: > + /* 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); > + break; > + > + case R_UNLOCK: > + if (value == R_UNLOCK_MAGIC) { > + s->lock = 0; > + DB_PRINT("successful unlock\n"); > + } else if (s->lock) {/* bad unlock attempt */ > + qemu_log_mask(LOG_GUEST_ERROR, "%s:failed unlock\n", prefix); > + s->regs[R_CTRL] &= ~PCAP_PR; > + s->regs[R_CTRL] &= ~PCFG_AES_EN_MASK; > + } > + break; > + } > > And the post-write logic. > > + xilinx_devcfg_update_ixr(s); > + > + if (*prefix) { > + g_free((void *)prefix); > + } > +} > + > >> 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. >> >> 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. >> > > My goal is to pretty much copy paste out the TRM for the clear GE > write. Some of my register fields in the TRM for this device say > things like "Reserved, always write as 1" which im trying to capture > in a data driven way without having to pollute my switch-cases with > this junk. It would be nice to autogenerate this as well. I think you're trying to solve too many problems at once. I don't think putting error messages in a register layout description is a good idea. >> 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. FWIW, I now think this is a bad idea :-) Here's the full example: --=-=-= Content-Type: application/octet-stream Content-Disposition: attachment; filename=foo Content-Transfer-Encoding: base64 Y29tbWl0IGNjZTE1YTFlZDZhYWM1MWY4YjdlMDVlYmE1YzkzNDY2YzNkOTZkZjcKQXV0aG9yOiBB bnRob255IExpZ3VvcmkgPGFsaWd1b3JpQHVzLmlibS5jb20+CkRhdGU6ICAgV2VkIE1heSAyOSAx Mzo0NDozNSAyMDEzIC0wNTAwCgogICAgRGVtb25zdHJhdGUgc2VwYXJhdGluZyByZWdpc3RlciBk ZWNvZGUgZnJvbSBnZXQvc2V0CgpkaWZmIC0tZ2l0IGEvaHcvY2hhci9zZXJpYWwuYyBiL2h3L2No YXIvc2VyaWFsLmMKaW5kZXggNjZiNjM0OC4uMzRjOGEyZSAxMDA2NDQKLS0tIGEvaHcvY2hhci9z ZXJpYWwuYworKysgYi9ody9jaGFyL3NlcmlhbC5jCkBAIC0yOTksNiArMjk5LDg5IEBAIHN0YXRp YyBnYm9vbGVhbiBzZXJpYWxfeG1pdChHSU9DaGFubmVsICpjaGFuLCBHSU9Db25kaXRpb24gY29u ZCwgdm9pZCAqb3BhcXVlKQogICAgIHJldHVybiBGQUxTRTsKIH0KIAorZW51bSB7CisgICAgVUFS VF9JTlZBTElEID0gMCwKKyAgICBVQVJUX0RJVklERVIsCisgICAgVUFSVF9SQlIsCisgICAgVUFS VF9USFIsCisgICAgVUFSVF9JRVIsCisgICAgVUFSVF9JSVIsCisgICAgVUFSVF9GQ1IsCisgICAg VUFSVF9MQ1IsCisgICAgVUFSVF9NQ1IsCisgICAgVUFSVF9MU1IsCisgICAgVUFSVF9NU1IsCisg ICAgVUFSVF9TQ1IsCit9OworCisjZGVmaW5lIFJFR19SRUFECTEKKyNkZWZpbmUgUkVHX1dSSVRF CTIKKyNkZWZpbmUgUkVHX1JXCQkoUkVHX1JFQUQgfCBSRUdfV1JJVEUpCisKK3R5cGVkZWYgc3Ry dWN0IFJlZ2lzdGVyRGVjb2RlRW50cnkKK3sKKyAgICB1aW50NjRfdCBhZGRyOworICAgIHVuc2ln bmVkIGFjY2Vzc19zaXplOworICAgIGludCByZWdubzsKKyAgICBpbnQgZmxhZ3M7Cit9IFJlZ2lz dGVyRGVjb2RlRW50cnk7CisKK3N0YXRpYyBSZWdpc3RlckRlY29kZUVudHJ5IGRlY29kZXJbXSA9 IHsKKyAgICAvKiBhZGRyZXNzZXMgMC0xIG11c3QgYmUgb3BlbiBkZWNvZGVkIGR1ZSB0byBsYXRj aGluZyAqLworICAgIHsgLmFkZHIgPSAyLCAucmVnbm8gPSBVQVJUX0ZDUiwgLmZsYWdzID0gUkVH X1dSSVRFIH0sCisgICAgeyAuYWRkciA9IDIsIC5yZWdubyA9IFVBUlRfSUlSLCAuZmxhZ3MgPSBS RUdfUkVBRCB9LAorICAgIHsgLmFkZHIgPSAzLCAucmVnbm8gPSBVQVJUX0xDUiwgLmZsYWdzID0g UkVHX1JXIH0sCisgICAgeyAuYWRkciA9IDQsIC5yZWdubyA9IFVBUlRfTUNSLCAuZmxhZ3MgPSBS RUdfUlcgfSwKKyAgICB7IC5hZGRyID0gNSwgLnJlZ25vID0gVUFSVF9MU1IsIC5mbGFncyA9IFJF R19SRUFEIH0sCisgICAgeyAuYWRkciA9IDYsIC5yZWdubyA9IFVBUlRfTVNSLCAuZmxhZ3MgPSBS RUdfUkVBRCB9LAorICAgIHsgLmFkZHIgPSA3LCAucmVnbm8gPSBVQVJUX1NDUiwgLmZsYWdzID0g UkVHX1JXIH0sCit9OworCit0eXBlZGVmIHN0cnVjdCBSZWdpc3Rlck1hcEVudHJ5Cit7CisgICAg c2l6ZV90IG9mZnNldDsKKyAgICBpbnQgcmVnbm87CisgICAgUmVnaXN0ZXJUeXBlIHR5cGU7Cisg ICAgdWludDY0X3QgbWFzazsKKyAgICBpbnQgcnNoaWZ0OworICAgIGludCBmbGFnczsKK30gUmVn aXN0ZXJNYXBFbnRyeTsKKworc3RhdGljIGludCBzZXJpYWxfZGVjb2RlKHZvaWQgKm9wYXF1ZSwg aHdhZGRyIGFkZHIsIHVuc2lnbmVkIHNpemUsCisgICAgICAgICAgICAgICAgICAgICAgICAgYm9v bCBpc193cml0ZSkKK3sKKyAgICBTZXJpYWxTdGF0ZSAqcyA9IG9wYXF1ZTsKKworICAgIHN3aXRj aCAoYWRkcikgeworICAgIGNhc2UgMDoKKyAgICAgICAgaWYgKHMtPmxjciAmIFVBUlRfTENSX0RM QUIpIHsKKyAgICAgICAgICAgIHJldHVybiBVQVJUX0RJVklERVI7CisgICAgICAgIH0KKyAgICAg ICAgaWYgKGlzX3dyaXRlKSB7CisgICAgICAgICAgICByZXR1cm4gVUFSVF9USFI7CisgICAgICAg IH0KKyAgICAgICAgcmV0dXJuIFVBUlRfUkJSOworICAgIGNhc2UgMToKKyAgICAgICAgaWYgKHMt PmxjciAmIFVBUlRfTENSX0RMQUIpIHsKKyAgICAgICAgICAgIHJldHVybiBVQVJUX0RJVklERVI7 CisgICAgICAgIH0KKyAgICAgICAgcmV0dXJuIFVBUlRfSUVSOworICAgIGRlZmF1bHQ6CisgICAg ICAgIHJldHVybiByZWdpc3Rlcl9kZWNvZGUoZGVjb2RlciwKKyAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICBBUlJBWV9TSVpFKGRlY29kZXIpLAorICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgIGFkZHIsIHNpemUsIGlzX3dyaXRlKTsKKyAgICB9Cit9CisKK3N0YXRpYyBSZWdpc3Rl ck1hcEVudHJ5IHJlZ21hcFtdID0geworICAgIERFRklORV9SRUdfVTgoU2VyaWFsU3RhdGUsIGll ciwgVUFSVF9JRVIpLAorICAgIERFRklORV9SRUdfVTgoU2VyaWFsU3RhdGUsIGlpciwgVUFSVF9J SVIpLAorICAgIERFRklORV9SRUdfVTgoU2VyaWFsU3RhdGUsIGxjciwgVUFSVF9MQ1IpLAorICAg IERFRklORV9SRUdfVTgoU2VyaWFsU3RhdGUsIG1jciwgVUFSVF9NQ1IpLAorICAgIERFRklORV9S RUdfVTgoU2VyaWFsU3RhdGUsIGxzciwgVUFSVF9MU1IpLAorICAgIERFRklORV9SRUdfVTgoU2Vy aWFsU3RhdGUsIHNjciwgVUFSVF9TQ1IpLAorICAgIERFRklORV9SRUdfVTgoU2VyaWFsU3RhdGUs IHRociwgVUFSVF9USFIpLAorfTsKIAogc3RhdGljIHZvaWQgc2VyaWFsX2lvcG9ydF93cml0ZSh2 b2lkICpvcGFxdWUsIGh3YWRkciBhZGRyLCB1aW50NjRfdCB2YWwsCiAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgIHVuc2lnbmVkIHNpemUpCkBAIC0zMDcsNTIgKzM5MCw0OCBAQCBzdGF0 aWMgdm9pZCBzZXJpYWxfaW9wb3J0X3dyaXRlKHZvaWQgKm9wYXF1ZSwgaHdhZGRyIGFkZHIsIHVp bnQ2NF90IHZhbCwKIAogICAgIGFkZHIgJj0gNzsKICAgICBEUFJJTlRGKCJ3cml0ZSBhZGRyPTB4 JSIgSFdBRERSX1BSSXggIiB2YWw9MHglIiBQUkl4NjQgIlxuIiwgYWRkciwgdmFsKTsKLSAgICBz d2l0Y2goYWRkcikgeworICAgIHN3aXRjaChyZWdpc3Rlcl9zdG9yZShyZWdtYXAsIHMsIHNlcmlh bF9kZWNvZGUocywgYWRkciwgc2l6ZSwgdHJ1ZSksIHNpemUsIHZhbCkpIHsKICAgICBkZWZhdWx0 OgotICAgIGNhc2UgMDoKLSAgICAgICAgaWYgKHMtPmxjciAmIFVBUlRfTENSX0RMQUIpIHsKKyAg ICBjYXNlIFVBUlRfRElWSURFUjoKKyAgICAgICAgaWYgKGFkZHIgPT0gMCkgewogICAgICAgICAg ICAgcy0+ZGl2aWRlciA9IChzLT5kaXZpZGVyICYgMHhmZjAwKSB8IHZhbDsKLSAgICAgICAgICAg IHNlcmlhbF91cGRhdGVfcGFyYW1ldGVycyhzKTsKICAgICAgICAgfSBlbHNlIHsKLSAgICAgICAg ICAgIHMtPnRociA9ICh1aW50OF90KSB2YWw7Ci0gICAgICAgICAgICBpZihzLT5mY3IgJiBVQVJU X0ZDUl9GRSkgewotICAgICAgICAgICAgICAgIGZpZm9fcHV0KHMsIFhNSVRfRklGTywgcy0+dGhy KTsKLSAgICAgICAgICAgICAgICBzLT50aHJfaXBlbmRpbmcgPSAwOwotICAgICAgICAgICAgICAg IHMtPmxzciAmPSB+VUFSVF9MU1JfVEVNVDsKLSAgICAgICAgICAgICAgICBzLT5sc3IgJj0gflVB UlRfTFNSX1RIUkU7Ci0gICAgICAgICAgICAgICAgc2VyaWFsX3VwZGF0ZV9pcnEocyk7Ci0gICAg ICAgICAgICB9IGVsc2UgewotICAgICAgICAgICAgICAgIHMtPnRocl9pcGVuZGluZyA9IDA7Ci0g ICAgICAgICAgICAgICAgcy0+bHNyICY9IH5VQVJUX0xTUl9USFJFOwotICAgICAgICAgICAgICAg IHNlcmlhbF91cGRhdGVfaXJxKHMpOwotICAgICAgICAgICAgfQotICAgICAgICAgICAgc2VyaWFs X3htaXQoTlVMTCwgR19JT19PVVQsIHMpOworICAgICAgICAgICAgcy0+ZGl2aWRlciA9IChzLT5k aXZpZGVyICYgMHgwMGZmKSB8ICh2YWwgPDwgOCk7CiAgICAgICAgIH0KKyAgICAgICAgc2VyaWFs X3VwZGF0ZV9wYXJhbWV0ZXJzKHMpOwogICAgICAgICBicmVhazsKLSAgICBjYXNlIDE6Ci0gICAg ICAgIGlmIChzLT5sY3IgJiBVQVJUX0xDUl9ETEFCKSB7Ci0gICAgICAgICAgICBzLT5kaXZpZGVy ID0gKHMtPmRpdmlkZXIgJiAweDAwZmYpIHwgKHZhbCA8PCA4KTsKLSAgICAgICAgICAgIHNlcmlh bF91cGRhdGVfcGFyYW1ldGVycyhzKTsKKyAgICBjYXNlIFVBUlRfVEhSOgorICAgICAgICBpZihz LT5mY3IgJiBVQVJUX0ZDUl9GRSkgeworICAgICAgICAgICAgZmlmb19wdXQocywgWE1JVF9GSUZP LCBzLT50aHIpOworICAgICAgICAgICAgcy0+dGhyX2lwZW5kaW5nID0gMDsKKyAgICAgICAgICAg IHMtPmxzciAmPSB+VUFSVF9MU1JfVEVNVDsKKyAgICAgICAgICAgIHMtPmxzciAmPSB+VUFSVF9M U1JfVEhSRTsKKyAgICAgICAgICAgIHNlcmlhbF91cGRhdGVfaXJxKHMpOwogICAgICAgICB9IGVs c2UgewotICAgICAgICAgICAgcy0+aWVyID0gdmFsICYgMHgwZjsKLSAgICAgICAgICAgIC8qIElm IHRoZSBiYWNrZW5kIGRldmljZSBpcyBhIHJlYWwgc2VyaWFsIHBvcnQsIHR1cm4gcG9sbGluZyBv ZiB0aGUgbW9kZW0KLSAgICAgICAgICAgICAgIHN0YXR1cyBsaW5lcyBvbiBwaHlzaWNhbCBwb3J0 IG9uIG9yIG9mZiBkZXBlbmRpbmcgb24gVUFSVF9JRVJfTVNJIHN0YXRlICovCi0gICAgICAgICAg ICBpZiAocy0+cG9sbF9tc2wgPj0gMCkgewotICAgICAgICAgICAgICAgIGlmIChzLT5pZXIgJiBV QVJUX0lFUl9NU0kpIHsKLSAgICAgICAgICAgICAgICAgICAgIHMtPnBvbGxfbXNsID0gMTsKLSAg ICAgICAgICAgICAgICAgICAgIHNlcmlhbF91cGRhdGVfbXNsKHMpOwotICAgICAgICAgICAgICAg IH0gZWxzZSB7Ci0gICAgICAgICAgICAgICAgICAgICBxZW11X2RlbF90aW1lcihzLT5tb2RlbV9z dGF0dXNfcG9sbCk7Ci0gICAgICAgICAgICAgICAgICAgICBzLT5wb2xsX21zbCA9IDA7Ci0gICAg ICAgICAgICAgICAgfQotICAgICAgICAgICAgfQotICAgICAgICAgICAgaWYgKHMtPmxzciAmIFVB UlRfTFNSX1RIUkUpIHsKLSAgICAgICAgICAgICAgICBzLT50aHJfaXBlbmRpbmcgPSAxOwotICAg ICAgICAgICAgICAgIHNlcmlhbF91cGRhdGVfaXJxKHMpOworICAgICAgICAgICAgcy0+dGhyX2lw ZW5kaW5nID0gMDsKKyAgICAgICAgICAgIHMtPmxzciAmPSB+VUFSVF9MU1JfVEhSRTsKKyAgICAg ICAgICAgIHNlcmlhbF91cGRhdGVfaXJxKHMpOworICAgICAgICB9CisgICAgICAgIHNlcmlhbF94 bWl0KE5VTEwsIEdfSU9fT1VULCBzKTsKKyAgICAgICAgYnJlYWs7CisgICAgY2FzZSBVQVJUX0lF UjoKKyAgICAgICAgLyogSWYgdGhlIGJhY2tlbmQgZGV2aWNlIGlzIGEgcmVhbCBzZXJpYWwgcG9y dCwgdHVybiBwb2xsaW5nIG9mIHRoZSBtb2RlbQorICAgICAgICAgICBzdGF0dXMgbGluZXMgb24g cGh5c2ljYWwgcG9ydCBvbiBvciBvZmYgZGVwZW5kaW5nIG9uIFVBUlRfSUVSX01TSSBzdGF0ZSAq LworICAgICAgICBpZiAocy0+cG9sbF9tc2wgPj0gMCkgeworICAgICAgICAgICAgaWYgKHMtPmll ciAmIFVBUlRfSUVSX01TSSkgeworICAgICAgICAgICAgICAgIHMtPnBvbGxfbXNsID0gMTsKKyAg ICAgICAgICAgICAgICBzZXJpYWxfdXBkYXRlX21zbChzKTsKKyAgICAgICAgICAgIH0gZWxzZSB7 CisgICAgICAgICAgICAgICAgcWVtdV9kZWxfdGltZXIocy0+bW9kZW1fc3RhdHVzX3BvbGwpOwor ICAgICAgICAgICAgICAgIHMtPnBvbGxfbXNsID0gMDsKICAgICAgICAgICAgIH0KICAgICAgICAg fQorICAgICAgICBpZiAocy0+bHNyICYgVUFSVF9MU1JfVEhSRSkgeworICAgICAgICAgICAgcy0+ dGhyX2lwZW5kaW5nID0gMTsKKyAgICAgICAgICAgIHNlcmlhbF91cGRhdGVfaXJxKHMpOworICAg ICAgICB9CiAgICAgICAgIGJyZWFrOwotICAgIGNhc2UgMjoKKyAgICBjYXNlIFVBUlRfRkNSOgog ICAgICAgICB2YWwgPSB2YWwgJiAweEZGOwogCiAgICAgICAgIGlmIChzLT5mY3IgPT0gdmFsKQpA QCAtMzk4LDEwICs0NzcsOSBAQCBzdGF0aWMgdm9pZCBzZXJpYWxfaW9wb3J0X3dyaXRlKHZvaWQg Km9wYXF1ZSwgaHdhZGRyIGFkZHIsIHVpbnQ2NF90IHZhbCwKICAgICAgICAgcy0+ZmNyID0gdmFs ICYgMHhDOTsKICAgICAgICAgc2VyaWFsX3VwZGF0ZV9pcnEocyk7CiAgICAgICAgIGJyZWFrOwot ICAgIGNhc2UgMzoKKyAgICBjYXNlIFVBUlRfTENSOgogICAgICAgICB7CiAgICAgICAgICAgICBp bnQgYnJlYWtfZW5hYmxlOwotICAgICAgICAgICAgcy0+bGNyID0gdmFsOwogICAgICAgICAgICAg c2VyaWFsX3VwZGF0ZV9wYXJhbWV0ZXJzKHMpOwogICAgICAgICAgICAgYnJlYWtfZW5hYmxlID0g KHZhbCA+PiA2KSAmIDE7CiAgICAgICAgICAgICBpZiAoYnJlYWtfZW5hYmxlICE9IHMtPmxhc3Rf YnJlYWtfZW5hYmxlKSB7CkBAIC00MTEsNyArNDg5LDcgQEAgc3RhdGljIHZvaWQgc2VyaWFsX2lv cG9ydF93cml0ZSh2b2lkICpvcGFxdWUsIGh3YWRkciBhZGRyLCB1aW50NjRfdCB2YWwsCiAgICAg ICAgICAgICB9CiAgICAgICAgIH0KICAgICAgICAgYnJlYWs7Ci0gICAgY2FzZSA0OgorICAgIGNh c2UgVUFSVF9NQ1I6CiAgICAgICAgIHsKICAgICAgICAgICAgIGludCBmbGFnczsKICAgICAgICAg ICAgIGludCBvbGRfbWNyID0gcy0+bWNyOwpAQCAtNDM3LDc1ICs1MTUsNTcgQEAgc3RhdGljIHZv aWQgc2VyaWFsX2lvcG9ydF93cml0ZSh2b2lkICpvcGFxdWUsIGh3YWRkciBhZGRyLCB1aW50NjRf dCB2YWwsCiAgICAgICAgICAgICB9CiAgICAgICAgIH0KICAgICAgICAgYnJlYWs7Ci0gICAgY2Fz ZSA1OgotICAgICAgICBicmVhazsKLSAgICBjYXNlIDY6Ci0gICAgICAgIGJyZWFrOwotICAgIGNh c2UgNzoKLSAgICAgICAgcy0+c2NyID0gdmFsOwotICAgICAgICBicmVhazsKICAgICB9CiB9CiAK IHN0YXRpYyB1aW50NjRfdCBzZXJpYWxfaW9wb3J0X3JlYWQodm9pZCAqb3BhcXVlLCBod2FkZHIg YWRkciwgdW5zaWduZWQgc2l6ZSkKIHsKICAgICBTZXJpYWxTdGF0ZSAqcyA9IG9wYXF1ZTsKLSAg ICB1aW50MzJfdCByZXQ7CisgICAgdWludDY0X3QgcmV0ID0gMHhGRjsKIAogICAgIGFkZHIgJj0g NzsKLSAgICBzd2l0Y2goYWRkcikgeworICAgIHN3aXRjaChyZWdpc3Rlcl9sb2FkKHJlZ21hcCwg cywgc2VyaWFsX2RlY29kZShzLCBhZGRyLCBzaXplLCBmYWxzZSksIHNpemUsICZyZXQpKSB7CiAg ICAgZGVmYXVsdDoKLSAgICBjYXNlIDA6Ci0gICAgICAgIGlmIChzLT5sY3IgJiBVQVJUX0xDUl9E TEFCKSB7CisgICAgICAgIGJyZWFrOworICAgIGNhc2UgVUFSVF9ESVZJREVSOgorICAgICAgICBp ZiAoYWRkciA9PSAwKSB7CiAgICAgICAgICAgICByZXQgPSBzLT5kaXZpZGVyICYgMHhmZjsKICAg ICAgICAgfSBlbHNlIHsKLSAgICAgICAgICAgIGlmKHMtPmZjciAmIFVBUlRfRkNSX0ZFKSB7Ci0g ICAgICAgICAgICAgICAgcmV0ID0gZmlmb19nZXQocyxSRUNWX0ZJRk8pOwotICAgICAgICAgICAg ICAgIGlmIChzLT5yZWN2X2ZpZm8uY291bnQgPT0gMCkKLSAgICAgICAgICAgICAgICAgICAgcy0+ bHNyICY9IH4oVUFSVF9MU1JfRFIgfCBVQVJUX0xTUl9CSSk7Ci0gICAgICAgICAgICAgICAgZWxz ZQotICAgICAgICAgICAgICAgICAgICBxZW11X21vZF90aW1lcihzLT5maWZvX3RpbWVvdXRfdGlt ZXIsIHFlbXVfZ2V0X2Nsb2NrX25zICh2bV9jbG9jaykgKyBzLT5jaGFyX3RyYW5zbWl0X3RpbWUg KiA0KTsKLSAgICAgICAgICAgICAgICBzLT50aW1lb3V0X2lwZW5kaW5nID0gMDsKLSAgICAgICAg ICAgIH0gZWxzZSB7Ci0gICAgICAgICAgICAgICAgcmV0ID0gcy0+cmJyOwotICAgICAgICAgICAg ICAgIHMtPmxzciAmPSB+KFVBUlRfTFNSX0RSIHwgVUFSVF9MU1JfQkkpOwotICAgICAgICAgICAg fQotICAgICAgICAgICAgc2VyaWFsX3VwZGF0ZV9pcnEocyk7Ci0gICAgICAgICAgICBpZiAoIShz LT5tY3IgJiBVQVJUX01DUl9MT09QKSkgewotICAgICAgICAgICAgICAgIC8qIGluIGxvb3BiYWNr IG1vZGUsIGRvbid0IHJlY2VpdmUgYW55IGRhdGEgKi8KLSAgICAgICAgICAgICAgICBxZW11X2No cl9hY2NlcHRfaW5wdXQocy0+Y2hyKTsKLSAgICAgICAgICAgIH0KKyAgICAgICAgICAgIHJldCA9 IChzLT5kaXZpZGVyID4+IDgpICYgMHhmZjsKICAgICAgICAgfQogICAgICAgICBicmVhazsKLSAg ICBjYXNlIDE6Ci0gICAgICAgIGlmIChzLT5sY3IgJiBVQVJUX0xDUl9ETEFCKSB7Ci0gICAgICAg ICAgICByZXQgPSAocy0+ZGl2aWRlciA+PiA4KSAmIDB4ZmY7CisgICAgY2FzZSBVQVJUX1JCUjoK KyAgICAgICAgaWYocy0+ZmNyICYgVUFSVF9GQ1JfRkUpIHsKKyAgICAgICAgICAgIHJldCA9IGZp Zm9fZ2V0KHMsUkVDVl9GSUZPKTsKKyAgICAgICAgICAgIGlmIChzLT5yZWN2X2ZpZm8uY291bnQg PT0gMCkKKyAgICAgICAgICAgICAgICBzLT5sc3IgJj0gfihVQVJUX0xTUl9EUiB8IFVBUlRfTFNS X0JJKTsKKyAgICAgICAgICAgIGVsc2UKKyAgICAgICAgICAgICAgICBxZW11X21vZF90aW1lcihz LT5maWZvX3RpbWVvdXRfdGltZXIsIHFlbXVfZ2V0X2Nsb2NrX25zICh2bV9jbG9jaykgKyBzLT5j aGFyX3RyYW5zbWl0X3RpbWUgKiA0KTsKKyAgICAgICAgICAgIHMtPnRpbWVvdXRfaXBlbmRpbmcg PSAwOwogICAgICAgICB9IGVsc2UgewotICAgICAgICAgICAgcmV0ID0gcy0+aWVyOworICAgICAg ICAgICAgcmV0ID0gcy0+cmJyOworICAgICAgICAgICAgcy0+bHNyICY9IH4oVUFSVF9MU1JfRFIg fCBVQVJUX0xTUl9CSSk7CisgICAgICAgIH0KKyAgICAgICAgc2VyaWFsX3VwZGF0ZV9pcnEocyk7 CisgICAgICAgIGlmICghKHMtPm1jciAmIFVBUlRfTUNSX0xPT1ApKSB7CisgICAgICAgICAgICAv KiBpbiBsb29wYmFjayBtb2RlLCBkb24ndCByZWNlaXZlIGFueSBkYXRhICovCisgICAgICAgICAg ICBxZW11X2Nocl9hY2NlcHRfaW5wdXQocy0+Y2hyKTsKICAgICAgICAgfQogICAgICAgICBicmVh azsKLSAgICBjYXNlIDI6Ci0gICAgICAgIHJldCA9IHMtPmlpcjsKLSAgICAgICAgaWYgKChyZXQg JiBVQVJUX0lJUl9JRCkgPT0gVUFSVF9JSVJfVEhSSSkgeworICAgIGNhc2UgVUFSVF9JSVI6Cisg ICAgICAgIGlmICgocy0+aWlyICYgVUFSVF9JSVJfSUQpID09IFVBUlRfSUlSX1RIUkkpIHsKICAg ICAgICAgICAgIHMtPnRocl9pcGVuZGluZyA9IDA7CiAgICAgICAgICAgICBzZXJpYWxfdXBkYXRl X2lycShzKTsKICAgICAgICAgfQogICAgICAgICBicmVhazsKLSAgICBjYXNlIDM6Ci0gICAgICAg IHJldCA9IHMtPmxjcjsKLSAgICAgICAgYnJlYWs7Ci0gICAgY2FzZSA0OgotICAgICAgICByZXQg PSBzLT5tY3I7Ci0gICAgICAgIGJyZWFrOwotICAgIGNhc2UgNToKLSAgICAgICAgcmV0ID0gcy0+ bHNyOworICAgIGNhc2UgVUFSVF9MU1I6CiAgICAgICAgIC8qIENsZWFyIGJyZWFrIGFuZCBvdmVy cnVuIGludGVycnVwdHMgKi8KICAgICAgICAgaWYgKHMtPmxzciAmIChVQVJUX0xTUl9CSXxVQVJU X0xTUl9PRSkpIHsKICAgICAgICAgICAgIHMtPmxzciAmPSB+KFVBUlRfTFNSX0JJfFVBUlRfTFNS X09FKTsKICAgICAgICAgICAgIHNlcmlhbF91cGRhdGVfaXJxKHMpOwogICAgICAgICB9CiAgICAg ICAgIGJyZWFrOwotICAgIGNhc2UgNjoKKyAgICBjYXNlIFVBUlRfTVNSOgogICAgICAgICBpZiAo cy0+bWNyICYgVUFSVF9NQ1JfTE9PUCkgewogICAgICAgICAgICAgLyogaW4gbG9vcGJhY2ssIHRo ZSBtb2RlbSBvdXRwdXQgcGlucyBhcmUgY29ubmVjdGVkIHRvIHRoZQogICAgICAgICAgICAgICAg aW5wdXRzICovCkBAIC01MjMsOSArNTgzLDYgQEAgc3RhdGljIHVpbnQ2NF90IHNlcmlhbF9pb3Bv cnRfcmVhZCh2b2lkICpvcGFxdWUsIGh3YWRkciBhZGRyLCB1bnNpZ25lZCBzaXplKQogICAgICAg ICAgICAgfQogICAgICAgICB9CiAgICAgICAgIGJyZWFrOwotICAgIGNhc2UgNzoKLSAgICAgICAg cmV0ID0gcy0+c2NyOwotICAgICAgICBicmVhazsKICAgICB9CiAgICAgRFBSSU5URigicmVhZCBh ZGRyPTB4JSIgSFdBRERSX1BSSXggIiB2YWw9MHglMDJ4XG4iLCBhZGRyLCByZXQpOwogICAgIHJl dHVybiByZXQ7Cg== --=-=-= Regards, Anthony Liguori >> > > Hmm ill have to think on this one. > > Regards, > Peter > >> 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 >> --=-=-=--