Re: [Qemu-devel] [PATCH v14 5/9] target-arm: kvm64: inject synchronous External Abort
Hi Peter, On 2018/1/13 13:24, gengdongjiu wrote: >>> + >>> +/* For the AArch64, instruction length is 32-bit */ >>> +esr |= ARM_EL_IL; >>> +env->exception.syndrome = esr; >>> + >>> +cc->do_interrupt(c); >>> + >>> +/* set ESR_EL1 */ >>> +ret = kvm_arm_cpreg_value(cpu, offsetof(CPUARMState, cp15.esr_el[1])); >> Breakpoint injection doesn't need to do this. Neither should this code. > As my above explanation, in the KVM mode, it needs to set the ESR_ELx in > extra method. > the cc->do_interrupt(c) does not set ESR_ELx. so I use kvm_arm_cpreg_value() > to set it. whether you have better method to set the ESR_Elx except for my > method? Thanks. If QEMU changes the KVM's registers, it needs to call write_list_to_kvmstate() to write the cpu->cpreg_values[] list to KVM through KVM_SET_ONE_REG IOCTL[1]. In Qemu, now it should not have software path to change the cpu->cpreg_values[] list except write_cpustate_to_list(). Here I can also call write_cpustate_to_list() instead of kvm_arm_cpreg_value() to change cpu->cpreg_values[] list, but the write_cpustate_to_list() will write all the coprocessor state to the cpu->cpreg_values[] list, we can not sure all the coprocessor states are right, so here I only change corresponding index value in this list using kvm_arm_cpreg_value(). Breakpoint injection that you mentioned should not change KVM register or not in the KVM mode. [1]: kvm_arch_put_registers() -> write_list_to_kvmstate() -> write cpu->cpreg_values[] to the kernel KVM through KVM_SET_ONE_REG > > >>> +if (ret) { >>> +fprintf(stderr, "<%s> failed to set esr_el1\n", __func__); >>> +abort(); >>> +} >>> +} >>> + >>> #define AARCH64_CORE_REG(x) (KVM_REG_ARM64 | KVM_REG_SIZE_U64 | \ >>> KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(x)) >>> >>> -- >>> 1.8.3.1
[Qemu-devel] [PATCH v3 5/7] target/m68k: add moves
and introduce SFC and DFC control registers. Signed-off-by: Laurent Vivier --- v2: copy bit 2 of SFC and DFC to tb->flags to inline memory access in moves decoder. target/m68k/cpu.h | 10 -- target/m68k/helper.c| 10 ++ target/m68k/monitor.c | 2 ++ target/m68k/op_helper.c | 4 +-- target/m68k/qregs.def | 2 ++ target/m68k/translate.c | 88 +++-- 6 files changed, 109 insertions(+), 7 deletions(-) diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h index 5e48103d3f..af24f061f5 100644 --- a/target/m68k/cpu.h +++ b/target/m68k/cpu.h @@ -138,6 +138,8 @@ typedef struct CPUM68KState { uint32_t mbar; uint32_t rambar0; uint32_t cacr; +uint32_t sfc; +uint32_t dfc; int pending_vector; int pending_level; @@ -547,8 +549,12 @@ static inline void cpu_get_tb_cpu_state(CPUM68KState *env, target_ulong *pc, { *pc = env->pc; *cs_base = 0; -*flags = (env->sr & SR_S) /* Bit 13 */ -| ((env->macsr >> 4) & 0xf);/* Bits 0-3 */ +*flags = (env->macsr >> 4) & 0xf; /* Bits 0-3 */ +if (env->sr & SR_S) { +*flags |= SR_S; /* Bit 13 */ +*flags |= (env->sfc & 4) << 12; /* Bit 14 */ +*flags |= (env->dfc & 4) << 13; /* Bit 15 */ +} } #endif diff --git a/target/m68k/helper.c b/target/m68k/helper.c index f1d50e54b1..c1bd0e9681 100644 --- a/target/m68k/helper.c +++ b/target/m68k/helper.c @@ -203,6 +203,12 @@ void HELPER(m68k_movec_to)(CPUM68KState *env, uint32_t reg, uint32_t val) switch (reg) { /* MC680[1234]0 */ +case M68K_CR_SFC: +env->sfc = val & 7; +return; +case M68K_CR_DFC: +env->dfc = val & 7; +return; case M68K_CR_VBR: env->vbr = val; return; @@ -254,6 +260,10 @@ uint32_t HELPER(m68k_movec_from)(CPUM68KState *env, uint32_t reg) switch (reg) { /* MC680[1234]0 */ +case M68K_CR_SFC: +return env->sfc; +case M68K_CR_DFC: +return env->dfc; case M68K_CR_VBR: return env->vbr; /* MC680[234]0 */ diff --git a/target/m68k/monitor.c b/target/m68k/monitor.c index a20af6b09c..c31feb4b02 100644 --- a/target/m68k/monitor.c +++ b/target/m68k/monitor.c @@ -31,6 +31,8 @@ static const MonitorDef monitor_defs[] = { { "ssp", offsetof(CPUM68KState, sp[0]) }, { "usp", offsetof(CPUM68KState, sp[1]) }, { "isp", offsetof(CPUM68KState, sp[2]) }, +{ "sfc", offsetof(CPUM68KState, sfc) }, +{ "dfc", offsetof(CPUM68KState, dfc) }, { "urp", offsetof(CPUM68KState, mmu.urp) }, { "srp", offsetof(CPUM68KState, mmu.srp) }, { "dttr0", offsetof(CPUM68KState, mmu.ttr[M68K_DTTR0]) }, diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c index f023901061..4609caa546 100644 --- a/target/m68k/op_helper.c +++ b/target/m68k/op_helper.c @@ -399,8 +399,8 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw) env->mmu.fault = false; if (qemu_loglevel_mask(CPU_LOG_INT)) { qemu_log("" - "ssw: %08x ea: %08x\n", - env->mmu.ssw, env->mmu.ar); + "ssw: %08x ea: %08x sfc: %ddfc: %d\n", + env->mmu.ssw, env->mmu.ar, env->sfc, env->dfc); } } else if (cs->exception_index == EXCP_ADDRESS) { do_stack_frame(env, &sp, 2, oldsr, 0, retaddr); diff --git a/target/m68k/qregs.def b/target/m68k/qregs.def index 1aadc622db..efe2bf90ee 100644 --- a/target/m68k/qregs.def +++ b/target/m68k/qregs.def @@ -1,5 +1,7 @@ DEFO32(PC, pc) DEFO32(SR, sr) +DEFO32(DFC, dfc) +DEFO32(SFC, sfc) DEFO32(CC_OP, cc_op) DEFO32(CC_X, cc_x) DEFO32(CC_C, cc_c) diff --git a/target/m68k/translate.c b/target/m68k/translate.c index 00a5bce6a3..694de43106 100644 --- a/target/m68k/translate.c +++ b/target/m68k/translate.c @@ -48,6 +48,8 @@ static char cpu_reg_names[2 * 8 * 3 + 5 * 4]; static TCGv cpu_dregs[8]; static TCGv cpu_aregs[8]; static TCGv_i64 cpu_macc[4]; +static TCGv QEMU_DFC; +static TCGv QEMU_SFC; #define REG(insn, pos) (((insn) >> (pos)) & 7) #define DREG(insn, pos) cpu_dregs[REG(insn, pos)] @@ -103,6 +105,10 @@ void m68k_tcg_init(void) p += 5; } +QEMU_DFC = tcg_global_mem_new(cpu_env, offsetof(CPUM68KState, dfc), + "DFC"); +QEMU_SFC = tcg_global_mem_new(cpu_env, offsetof(CPUM68KState, sfc), + "SFC"); NULL_QREG = tcg_global_mem_new(cpu_env, -4, "NULL"); store_dummy = tcg_global_mem_new(cpu_env, -8, "NULL"); } @@ -115,7 +121,9 @@ typedef struct DisasContext { int is_jmp; CCOp cc_op; /* Current CC operation */ int cc_op_synced; -int user; +#if defined(CONFIG_SOFTMMU) +uint32_t user; +#endif struct TranslationBlock *tb; int singlestep_enabled; TCGv_i64 mactmp; @@ -178,7 +186,15 @@ static v
[Qemu-devel] [PATCH v3 2/7] target/m68k: add MC68040 MMU
Only add MC68040 MMU page table processing and related registers (Special Status Word, Translation Control Register, User Root Pointer and Supervisor Root Pointer). Transparent Translation Registers, DFC/SFC and pflush/ptest will be added later. Signed-off-by: Laurent Vivier --- v3: s/smaller/smallest/ v2: move mmu_fault to CPUM68KState set TARGET_PAGE_BITS to 12 to avoid tlb_add_large_page() path use -page_size to mask address instead of TARGET_PAGE_MASK add ACCESS_DEBUG to not update page table USED/MODIFIED bits on gdb access rename ACCESS_INT to ACCESS_DATA target/m68k/cpu.c | 4 +- target/m68k/cpu.h | 113 ++-- target/m68k/helper.c| 223 ++-- target/m68k/monitor.c | 2 + target/m68k/op_helper.c | 94 +++- target/m68k/translate.c | 2 + 6 files changed, 422 insertions(+), 16 deletions(-) diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c index 03126ba543..98919b358b 100644 --- a/target/m68k/cpu.c +++ b/target/m68k/cpu.c @@ -269,9 +269,9 @@ static void m68k_cpu_class_init(ObjectClass *c, void *data) cc->set_pc = m68k_cpu_set_pc; cc->gdb_read_register = m68k_cpu_gdb_read_register; cc->gdb_write_register = m68k_cpu_gdb_write_register; -#ifdef CONFIG_USER_ONLY cc->handle_mmu_fault = m68k_cpu_handle_mmu_fault; -#else +#if defined(CONFIG_SOFTMMU) +cc->do_unassigned_access = m68k_cpu_unassigned_access; cc->get_phys_page_debug = m68k_cpu_get_phys_page_debug; #endif cc->disas_set_info = m68k_cpu_disas_set_info; diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h index c60564a047..63e5ba3dad 100644 --- a/target/m68k/cpu.h +++ b/target/m68k/cpu.h @@ -116,6 +116,12 @@ typedef struct CPUM68KState { /* MMU status. */ struct { uint32_t ar; +uint32_t ssw; +/* 68040 */ +uint16_t tcr; +uint32_t urp; +uint32_t srp; +bool fault; } mmu; /* Control registers. */ @@ -226,6 +232,90 @@ typedef enum { #define M68K_USP1 #define M68K_ISP2 +/* bits for 68040 special status word */ +#define M68K_CP_040 0x8000 +#define M68K_CU_040 0x4000 +#define M68K_CT_040 0x2000 +#define M68K_CM_040 0x1000 +#define M68K_MA_040 0x0800 +#define M68K_ATC_040 0x0400 +#define M68K_LK_040 0x0200 +#define M68K_RW_040 0x0100 +#define M68K_SIZ_040 0x0060 +#define M68K_TT_040 0x0018 +#define M68K_TM_040 0x0007 + +#define M68K_TM_040_DATA 0x0001 +#define M68K_TM_040_CODE 0x0002 +#define M68K_TM_040_SUPER 0x0004 + +/* bits for 68040 write back status word */ +#define M68K_WBV_040 0x80 +#define M68K_WBSIZ_040 0x60 +#define M68K_WBBYT_040 0x20 +#define M68K_WBWRD_040 0x40 +#define M68K_WBLNG_040 0x00 +#define M68K_WBTT_040 0x18 +#define M68K_WBTM_040 0x07 + +/* bus access size codes */ +#define M68K_BA_SIZE_MASK0x60 +#define M68K_BA_SIZE_BYTE0x20 +#define M68K_BA_SIZE_WORD0x40 +#define M68K_BA_SIZE_LONG0x00 +#define M68K_BA_SIZE_LINE0x60 + +/* bus access transfer type codes */ +#define M68K_BA_TT_MOVE160x08 + +/* bits for 68040 MMU status register (mmusr) */ +#define M68K_MMU_B_040 0x0800 +#define M68K_MMU_G_040 0x0400 +#define M68K_MMU_U1_040 0x0200 +#define M68K_MMU_U0_040 0x0100 +#define M68K_MMU_S_040 0x0080 +#define M68K_MMU_CM_040 0x0060 +#define M68K_MMU_M_040 0x0010 +#define M68K_MMU_WP_040 0x0004 +#define M68K_MMU_T_040 0x0002 +#define M68K_MMU_R_040 0x0001 + +#define M68K_MMU_SR_MASK_040 (M68K_MMU_G_040 | M68K_MMU_U1_040 | \ + M68K_MMU_U0_040 | M68K_MMU_S_040 | \ + M68K_MMU_CM_040 | M68K_MMU_M_040 | \ + M68K_MMU_WP_040) + +/* bits for 68040 MMU Translation Control Register */ +#define M68K_TCR_ENABLED 0x8000 +#define M68K_TCR_PAGE_8K 0x4000 + +/* bits for 68040 MMU Table Descriptor / Page Descriptor / TTR */ +#define M68K_DESC_WRITEPROT 0x0004 +#define M68K_DESC_USED 0x0008 +#define M68K_DESC_MODIFIED 0x0010 +#define M68K_DESC_CACHEMODE 0x0060 +#define M68K_DESC_CM_WRTHRU 0x +#define M68K_DESC_CM_COPYBK 0x0020 +#define M68K_DESC_CM_SERIAL 0x0040 +#define M68K_DESC_CM_NCACHE 0x0060 +#define M68K_DESC_SUPERONLY 0x0080 +#define M68K_DESC_USERATTR 0x0300 +#define M68K_DESC_USERATTR_SHIFT 8 +#define M68K_DESC_GLOBAL0x0400 +#define M68K_DESC_URESERVED 0x0800 + +#define M68K_POINTER_BASE(entry)(entry & ~0x1ff) +#define M68K_ROOT_INDEX(addr) ((address >> 23) & 0x1fc) +#define M68K_POINTER_INDEX(addr)((address >> 16) & 0x1fc) +#define M68K_4K_PAGE_BASE(entry)(next & ~0xff) +#define M68K_4K_PAGE_INDEX(addr)((address >> 10) & 0xfc) +#define M68K_8K_PAGE_BASE(entry)(next & ~0x7f) +#define M68K_8K_PAGE_INDEX(addr)((address >> 11) & 0x7c) +#define M68K_UDT_VALID(entry) (entry & 2) +#define M68K_PDT_VALID(entry) (entry & 3) +#define M68K_PDT_INDIRECT(entry)(
[Qemu-devel] [PATCH v3 0/7] target/m68k: supervisor mode (part 2)
This series introduces the MC68040 MMU. But first of all, we need to modify the prototype of tlb_fill() and handle_mmu_fault handler to pass the size of the access. MC68040 stores this value in the exception stack frame. Following patches add: - MMU page table and fault handlers, - Transparent Translation Registers - instruction "moves" to move data between user and kernel space - instructions pflush/ptest, to flush TLB and convert virtual address to physical address - "info tlb" HMP command I have tested it doesn't break QEMU linux-user mode emulation and coldfire softmmu machine. With the help of these patches I'm able to start a debian etch-m68k, but the following patches are still missing in master: - m68k softfloat series - Quadra 800 machine emulation series (VIA emulation, Nubus emulation, ESCC control/data address bit selector, video card emulation, ESP Pseudo-DMA, SWIM floppy controller, Apple Sound Chip emulation, and some big-endian fixes for dp8393x) v3: fix checkpatch.pl errors/warnings s/smaller/smallest/ v2: use "0" for the access size when we don't know the size of the access move mmu_fault to CPUM68KState set TARGET_PAGE_BITS to 12 to avoid tlb_add_large_page() path use -page_size to mask address instead of TARGET_PAGE_MASK add ACCESS_DEBUG to not update page table USED/MODIFIED bits on gdb access rename ACCESS_INT to ACCESS_DATA add index parameter to gen_load()/gen_store() copy bit 2 of SFC and DFC to tb->flags to inline memory access in moves decoder change ACCESS_PTEST value because of new ACCESS_DEBUG Laurent Vivier (7): accel/tcg: add size paremeter in tlb_fill() target/m68k: add MC68040 MMU target/m68k: add Transparent Translation target/m68k: add index parameter to gen_load()/gen_store() and Co. target/m68k: add moves target/m68k: add pflush/ptest target/m68k: add HMP command "info tlb" accel/tcg/cputlb.c| 13 +- accel/tcg/softmmu_template.h | 14 +- accel/tcg/user-exec.c | 2 +- hmp-commands-info.hx | 2 +- include/exec/exec-all.h | 6 +- include/qom/cpu.h | 2 +- target/alpha/cpu.h| 2 +- target/alpha/helper.c | 4 +- target/alpha/mem_helper.c | 6 +- target/arm/cpu.c | 4 +- target/arm/op_helper.c| 4 +- target/cris/cpu.h | 2 +- target/cris/helper.c | 4 +- target/cris/op_helper.c | 6 +- target/hppa/cpu.h | 3 +- target/hppa/helper.c | 2 +- target/hppa/op_helper.c | 2 +- target/i386/cpu.h | 2 +- target/i386/excp_helper.c | 4 +- target/i386/mem_helper.c | 6 +- target/lm32/cpu.h | 2 +- target/lm32/helper.c | 2 +- target/lm32/op_helper.c | 6 +- target/m68k/cpu.c | 4 +- target/m68k/cpu.h | 147 +- target/m68k/helper.c | 604 +- target/m68k/helper.h | 2 + target/m68k/monitor.c | 22 ++ target/m68k/op_helper.c | 101 ++- target/m68k/qregs.def | 2 + target/m68k/translate.c | 251 +- target/microblaze/cpu.h | 2 +- target/microblaze/helper.c| 4 +- target/microblaze/op_helper.c | 6 +- target/mips/helper.c | 2 +- target/mips/internal.h| 2 +- target/mips/op_helper.c | 10 +- target/moxie/cpu.h| 2 +- target/moxie/helper.c | 10 +- target/nios2/cpu.h| 2 +- target/nios2/helper.c | 6 +- target/nios2/mmu.c| 6 +- target/openrisc/cpu.h | 2 +- target/openrisc/mmu.c | 8 +- target/openrisc/mmu_helper.c | 6 +- target/ppc/cpu.h | 2 +- target/ppc/mmu_helper.c | 4 +- target/ppc/user_only_helper.c | 2 +- target/s390x/excp_helper.c| 4 +- target/s390x/internal.h | 2 +- target/s390x/mem_helper.c | 8 +- target/sh4/cpu.h | 2 +- target/sh4/helper.c | 4 +- target/sh4/op_helper.c| 6 +- target/sparc/cpu.h| 2 +- target/sparc/ldst_helper.c| 6 +- target/sparc/mmu_helper.c | 6 +- target/tilegx/cpu.c | 4 +- target/tricore/op_helper.c| 4 +- target/unicore32/cpu.h| 2 +- target/unicore32/helper.c | 2 +- target/unicore32/op_helper.c | 6 +- target/unicore32/softmmu.c| 2 +- target/xtensa/op_helper.c | 4 +- 64 files changed, 1171 insertions(+), 202 deletions(-) -- 2.14.3
[Qemu-devel] [PATCH v3 7/7] target/m68k: add HMP command "info tlb"
Dump MMU state and address mappings. Signed-off-by: Laurent Vivier Reviewed-by: Richard Henderson --- CC: Dr. David Alan Gilbert hmp-commands-info.hx | 2 +- target/m68k/cpu.h | 1 + target/m68k/helper.c | 216 ++ target/m68k/monitor.c | 13 +++ 4 files changed, 231 insertions(+), 1 deletion(-) diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx index 54c3e5eac6..ad590a4ffb 100644 --- a/hmp-commands-info.hx +++ b/hmp-commands-info.hx @@ -216,7 +216,7 @@ Show PCI information. ETEXI #if defined(TARGET_I386) || defined(TARGET_SH4) || defined(TARGET_SPARC) || \ -defined(TARGET_PPC) || defined(TARGET_XTENSA) +defined(TARGET_PPC) || defined(TARGET_XTENSA) || defined(TARGET_M68K) { .name = "tlb", .args_type = "", diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h index 1655875dbb..23e5aa6578 100644 --- a/target/m68k/cpu.h +++ b/target/m68k/cpu.h @@ -560,4 +560,5 @@ static inline void cpu_get_tb_cpu_state(CPUM68KState *env, target_ulong *pc, } } +void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUM68KState *env); #endif diff --git a/target/m68k/helper.c b/target/m68k/helper.c index 6950c03ada..6e79cd8621 100644 --- a/target/m68k/helper.c +++ b/target/m68k/helper.c @@ -374,6 +374,222 @@ int m68k_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int size, int rw, /* MMU: 68040 only */ +static void print_address_zone(FILE *f, fprintf_function cpu_fprintf, + uint32_t logical, uint32_t physical, + uint32_t size, int attr) +{ +cpu_fprintf(f, "%08x - %08x -> %08x - %08x %c ", +logical, logical + size - 1, +physical, physical + size - 1, +attr & 4 ? 'W' : '-'); +size >>= 10; +if (size < 1024) { +cpu_fprintf(f, "(%d KiB)\n", size); +} else { +size >>= 10; +if (size < 1024) { +cpu_fprintf(f, "(%d MiB)\n", size); +} else { +size >>= 10; +cpu_fprintf(f, "(%d GiB)\n", size); +} +} +} + +static void dump_address_map(FILE *f, fprintf_function cpu_fprintf, + CPUM68KState *env, uint32_t root_pointer) +{ +int i, j, k; +int tic_size, tic_shift; +uint32_t tib_mask; +uint32_t tia, tib, tic; +uint32_t logical = 0x, physical = 0x; +uint32_t first_logical = 0x, first_physical = 0x; +uint32_t last_logical, last_physical; +int32_t size; +int last_attr = -1, attr = -1; +M68kCPU *cpu = m68k_env_get_cpu(env); +CPUState *cs = CPU(cpu); + +if (env->mmu.tcr & M68K_TCR_PAGE_8K) { +/* 8k page */ +tic_size = 32; +tic_shift = 13; +tib_mask = 0xff80; +} else { +/* 4k page */ +tic_size = 64; +tic_shift = 12; +tib_mask = 0xff00; +} +for (i = 0; i < 128; i++) { +tia = ldl_phys(cs->as, M68K_POINTER_BASE(root_pointer) + i * 4); +if (!M68K_UDT_VALID(tia)) { +continue; +} +for (j = 0; j < 128; j++) { +tib = ldl_phys(cs->as, M68K_POINTER_BASE(tia) + j * 4); +if (!M68K_UDT_VALID(tia)) { +continue; +} +for (k = 0; k < tic_size; k++) { +tic = ldl_phys(cs->as, (tib & tib_mask) + k * 4); +if (!M68K_PDT_VALID(tia)) { +continue; +} +if (M68K_PDT_INDIRECT(tic)) { +tic = ldl_phys(cs->as, M68K_INDIRECT_POINTER(tic)); +} + +last_logical = logical; +logical = (i << 25) | (j << 18) | (k << tic_shift); + +last_physical = physical; +physical = tic & ~((1 << tic_shift) - 1); + +last_attr = attr; +attr = tic & ((1 << tic_shift) - 1); + +if ((logical != (last_logical + (1 << tic_shift))) || +(physical != (last_physical + (1 << tic_shift))) || +(attr & 4) != (last_attr & 4)) { + +if (first_logical != 0x) { +size = last_logical + (1 << tic_shift) - + first_logical; +print_address_zone(f, cpu_fprintf, first_logical, + first_physical, size, last_attr); +} +first_logical = logical; +first_physical = physical; +} +} +} +} +if (first_logical != logical || (attr & 4) != (last_attr & 4)) { +size = logical + (1 << tic_shift) - first_logical; +print_address_zone(f, cpu_fprintf, first_logical, first_physical, size, + last_attr); +} +} + +#define DUMP_CACHEFLAGS(a) \ +switch (a & M68K_DESC_CACHEMODE)
[Qemu-devel] [PATCH v3 3/7] target/m68k: add Transparent Translation
Add ittr0, ittr1, dttr0, dttr1 and manage Transparent Translations Signed-off-by: Laurent Vivier Reviewed-by: Richard Henderson --- target/m68k/cpu.h | 18 +++ target/m68k/helper.c| 79 + target/m68k/monitor.c | 4 +++ target/m68k/translate.c | 3 ++ 4 files changed, 104 insertions(+) diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h index 63e5ba3dad..5e48103d3f 100644 --- a/target/m68k/cpu.h +++ b/target/m68k/cpu.h @@ -76,6 +76,14 @@ #define EXCP_RTE0x100 #define EXCP_HALT_INSN 0x101 +#define M68K_DTTR0 0 +#define M68K_DTTR1 1 +#define M68K_ITTR0 2 +#define M68K_ITTR1 3 + +#define M68K_MAX_TTR 2 +#define TTR(type, index) ttr[((type & ACCESS_CODE) == ACCESS_CODE) * 2 + index] + #define NB_MMU_MODES 2 #define TARGET_INSN_START_EXTRA_WORDS 1 @@ -122,6 +130,7 @@ typedef struct CPUM68KState { uint32_t urp; uint32_t srp; bool fault; +uint32_t ttr[4]; } mmu; /* Control registers. */ @@ -316,6 +325,15 @@ typedef enum { #define M68K_PDT_INDIRECT(entry)((entry & 3) == 2) #define M68K_INDIRECT_POINTER(addr) (addr & ~3) +/* bits for 68040 MMU Transparent Translation Registers */ +#define M68K_TTR_ADDR_BASE 0xff00 +#define M68K_TTR_ADDR_MASK 0x00ff +#define M68K_TTR_ADDR_MASK_SHIFT8 +#define M68K_TTR_ENABLED 0x8000 +#define M68K_TTR_SFIELD0x6000 +#define M68K_TTR_SFIELD_USER 0x +#define M68K_TTR_SFIELD_SUPER 0x2000 + /* m68k Control Registers */ /* ColdFire */ diff --git a/target/m68k/helper.c b/target/m68k/helper.c index 13c6bb3d25..f1d50e54b1 100644 --- a/target/m68k/helper.c +++ b/target/m68k/helper.c @@ -230,6 +230,19 @@ void HELPER(m68k_movec_to)(CPUM68KState *env, uint32_t reg, uint32_t val) case M68K_CR_ISP: env->sp[M68K_ISP] = val; return; +/* MC68040/MC68LC040 */ +case M68K_CR_ITT0: +env->mmu.ttr[M68K_ITTR0] = val; +return; +case M68K_CR_ITT1: + env->mmu.ttr[M68K_ITTR1] = val; +return; +case M68K_CR_DTT0: +env->mmu.ttr[M68K_DTTR0] = val; +return; +case M68K_CR_DTT1: +env->mmu.ttr[M68K_DTTR1] = val; +return; } cpu_abort(CPU(cpu), "Unimplemented control register write 0x%x = 0x%x\n", reg, val); @@ -260,6 +273,14 @@ uint32_t HELPER(m68k_movec_from)(CPUM68KState *env, uint32_t reg) /* MC68040/MC68LC040 */ case M68K_CR_URP: return env->mmu.urp; +case M68K_CR_ITT0: +return env->mmu.ttr[M68K_ITTR0]; +case M68K_CR_ITT1: +return env->mmu.ttr[M68K_ITTR1]; +case M68K_CR_DTT0: +return env->mmu.ttr[M68K_DTTR0]; +case M68K_CR_DTT1: +return env->mmu.ttr[M68K_DTTR1]; } cpu_abort(CPU(cpu), "Unimplemented control register read 0x%x\n", reg); @@ -338,6 +359,53 @@ int m68k_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int size, int rw, /* MMU: 68040 only */ +static int check_TTR(uint32_t ttr, int *prot, target_ulong addr, + int access_type) +{ +uint32_t base, mask; + +/* check if transparent translation is enabled */ +if ((ttr & M68K_TTR_ENABLED) == 0) { +return 0; +} + +/* check mode access */ +switch (ttr & M68K_TTR_SFIELD) { +case M68K_TTR_SFIELD_USER: +/* match only if user */ +if ((access_type & ACCESS_SUPER) != 0) { +return 0; +} +break; +case M68K_TTR_SFIELD_SUPER: +/* match only if supervisor */ +if ((access_type & ACCESS_SUPER) == 0) { +return 0; +} +break; +default: +/* all other values disable mode matching (FC2) */ +break; +} + +/* check address matching */ + +base = ttr & M68K_TTR_ADDR_BASE; +mask = (ttr & M68K_TTR_ADDR_MASK) ^ M68K_TTR_ADDR_MASK; +mask <<= M68K_TTR_ADDR_MASK_SHIFT; + +if ((addr & mask) != (base & mask)) { +return 0; +} + +*prot = PAGE_READ | PAGE_EXEC; +if ((ttr & M68K_DESC_WRITEPROT) == 0) { +*prot |= PAGE_WRITE; +} + +return 1; +} + static int get_physical_address(CPUM68KState *env, hwaddr *physical, int *prot, target_ulong address, int access_type, target_ulong *page_size) @@ -350,6 +418,17 @@ static int get_physical_address(CPUM68KState *env, hwaddr *physical, target_ulong page_mask; bool debug = access_type & ACCESS_DEBUG; int page_bits; +int i; + +/* Transparent Translation (physical = logical) */ +for (i = 0; i < M68K_MAX_TTR; i++) { +if (check_TTR(env->mmu.TTR(access_type, i), + prot, address, access_type)) { +*physical = address; +*page_size = TARGET_PAGE_SIZE; +return 0; +} +} /* Page Table Root Pointer */ *prot = PAGE_READ | PAGE_WRITE; diff --git a/targ
[Qemu-devel] [PATCH v3 6/7] target/m68k: add pflush/ptest
Signed-off-by: Laurent Vivier Reviewed-by: Richard Henderson --- v2: change ACCESS_PTEST value because of new ACCESS_DEBUG use -page_size to mask address instead of TARGET_PAGE_MASK target/m68k/cpu.h | 3 +++ target/m68k/helper.c| 72 + target/m68k/helper.h| 2 ++ target/m68k/monitor.c | 1 + target/m68k/op_helper.c | 1 + target/m68k/translate.c | 33 +++ 6 files changed, 112 insertions(+) diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h index af24f061f5..1655875dbb 100644 --- a/target/m68k/cpu.h +++ b/target/m68k/cpu.h @@ -131,6 +131,7 @@ typedef struct CPUM68KState { uint32_t srp; bool fault; uint32_t ttr[4]; +uint32_t mmusr; } mmu; /* Control registers. */ @@ -510,6 +511,8 @@ enum { ACCESS_STORE = 0x02, /* 1 bit to indicate debug access */ ACCESS_DEBUG = 0x04, +/* PTEST instruction */ +ACCESS_PTEST = 0x08, /* Type of instruction that generated the access */ ACCESS_CODE = 0x10, /* Code fetch access*/ ACCESS_DATA = 0x20, /* Data load/store access*/ diff --git a/target/m68k/helper.c b/target/m68k/helper.c index c1bd0e9681..6950c03ada 100644 --- a/target/m68k/helper.c +++ b/target/m68k/helper.c @@ -221,6 +221,9 @@ void HELPER(m68k_movec_to)(CPUM68KState *env, uint32_t reg, uint32_t val) case M68K_CR_TC: env->mmu.tcr = val; return; +case M68K_CR_MMUSR: +env->mmu.mmusr = val; +return; case M68K_CR_SRP: env->mmu.srp = val; return; @@ -272,6 +275,8 @@ uint32_t HELPER(m68k_movec_from)(CPUM68KState *env, uint32_t reg) /* MC680[34]0 */ case M68K_CR_TC: return env->mmu.tcr; +case M68K_CR_MMUSR: +return env->mmu.mmusr; case M68K_CR_SRP: return env->mmu.srp; case M68K_CR_USP: @@ -434,6 +439,10 @@ static int get_physical_address(CPUM68KState *env, hwaddr *physical, for (i = 0; i < M68K_MAX_TTR; i++) { if (check_TTR(env->mmu.TTR(access_type, i), prot, address, access_type)) { +if (access_type & ACCESS_PTEST) { +/* Transparent Translation Register bit */ +env->mmu.mmusr = M68K_MMU_T_040 | M68K_MMU_R_040; +} *physical = address; *page_size = TARGET_PAGE_SIZE; return 0; @@ -462,6 +471,9 @@ static int get_physical_address(CPUM68KState *env, hwaddr *physical, stl_phys(cs->as, entry, next | M68K_DESC_USED); } if (next & M68K_DESC_WRITEPROT) { +if (access_type & ACCESS_PTEST) { +env->mmu.mmusr |= M68K_MMU_WP_040; +} *prot &= ~PAGE_WRITE; if (access_type & ACCESS_STORE) { return -1; @@ -479,6 +491,9 @@ static int get_physical_address(CPUM68KState *env, hwaddr *physical, stl_phys(cs->as, entry, next | M68K_DESC_USED); } if (next & M68K_DESC_WRITEPROT) { +if (access_type & ACCESS_PTEST) { +env->mmu.mmusr |= M68K_MMU_WP_040; +} *prot &= ~PAGE_WRITE; if (access_type & ACCESS_STORE) { return -1; @@ -526,6 +541,12 @@ static int get_physical_address(CPUM68KState *env, hwaddr *physical, page_offset = address & ~page_mask; *physical = (next & page_mask) + page_offset; +if (access_type & ACCESS_PTEST) { +env->mmu.mmusr |= next & M68K_MMU_SR_MASK_040; +env->mmu.mmusr |= *physical & 0xf000; +env->mmu.mmusr |= M68K_MMU_R_040; +} + if (next & M68K_DESC_WRITEPROT) { *prot &= ~PAGE_WRITE; if (access_type & ACCESS_STORE) { @@ -1079,6 +1100,57 @@ void HELPER(set_mac_extu)(CPUM68KState *env, uint32_t val, uint32_t acc) } #if defined(CONFIG_SOFTMMU) +void HELPER(ptest)(CPUM68KState *env, uint32_t addr, uint32_t is_read) +{ +M68kCPU *cpu = m68k_env_get_cpu(env); +CPUState *cs = CPU(cpu); +hwaddr physical; +int access_type; +int prot; +int ret; +target_ulong page_size; + +access_type = ACCESS_PTEST; +if (env->dfc & 4) { +access_type |= ACCESS_SUPER; +} +if ((env->dfc & 3) == 2) { +access_type |= ACCESS_CODE; +} +if (!is_read) { +access_type |= ACCESS_STORE; +} + +env->mmu.mmusr = 0; +env->mmu.ssw = 0; +ret = get_physical_address(env, &physical, &prot, addr, + access_type, &page_size); +if (ret == 0) { +tlb_set_page(cs, addr & -page_size, + physical & -page_size, + prot, access_type & ACCESS_SUPER ? + MMU_KERNEL_IDX : MMU_USER_IDX, page_size); +} +} + +void HELPER(pflush)(CPUM68KState *env, uint32_t addr, uint32_t opmode) +{ +M68kCPU *cpu = m68k_env_get_cpu(env); + +switch (opmode) { +case 0: /* Flush page entry if not global */ +case 1: /* Flush page ent
[Qemu-devel] [PATCH v3 1/7] accel/tcg: add size paremeter in tlb_fill()
The MC68040 MMU provides the size of the access that triggers the page fault. This size is set in the Special Status Word which is written in the stack frame of the access fault exception. So we need the size in m68k_cpu_unassigned_access() and m68k_cpu_handle_mmu_fault(). To be able to do that, this patch modifies the prototype of handle_mmu_fault handler, tlb_fill() and probe_write(). do_unassigned_access() already includes a size parameter. This patch also updates handle_mmu_fault handlers and tlb_fill() of all targets (only parameter, no code change). Signed-off-by: Laurent Vivier --- v2: use "0" for the access size when we don't know the size of the access CC: Paolo Bonzini CC: Peter Maydell CC: Edgar E. Iglesias CC: Eduardo Habkost CC: Michael Walle CC: Aurelien Jarno CC: Anthony Green CC: Chris Wulff CC: Stafford Horne CC: Alexander Graf CC: Artyom Tarasenko CC: Bastian Koppelmann CC: Guan Xuetao CC: Max Filippov CC: qemu-...@nongnu.org CC: qemu-...@nongnu.org CC: qemu-s3...@nongnu.org accel/tcg/cputlb.c| 13 - accel/tcg/softmmu_template.h | 14 -- accel/tcg/user-exec.c | 2 +- include/exec/exec-all.h | 6 +++--- include/qom/cpu.h | 2 +- target/alpha/cpu.h| 2 +- target/alpha/helper.c | 4 ++-- target/alpha/mem_helper.c | 6 +++--- target/arm/cpu.c | 4 ++-- target/arm/op_helper.c| 4 ++-- target/cris/cpu.h | 2 +- target/cris/helper.c | 4 ++-- target/cris/op_helper.c | 6 +++--- target/hppa/cpu.h | 3 ++- target/hppa/helper.c | 2 +- target/hppa/op_helper.c | 2 +- target/i386/cpu.h | 2 +- target/i386/excp_helper.c | 4 ++-- target/i386/mem_helper.c | 6 +++--- target/lm32/cpu.h | 2 +- target/lm32/helper.c | 2 +- target/lm32/op_helper.c | 6 +++--- target/m68k/cpu.h | 2 +- target/m68k/helper.c | 4 ++-- target/m68k/op_helper.c | 6 +++--- target/microblaze/cpu.h | 2 +- target/microblaze/helper.c| 4 ++-- target/microblaze/op_helper.c | 6 +++--- target/mips/helper.c | 2 +- target/mips/internal.h| 2 +- target/mips/op_helper.c | 10 +- target/moxie/cpu.h| 2 +- target/moxie/helper.c | 10 +- target/nios2/cpu.h| 2 +- target/nios2/helper.c | 6 -- target/nios2/mmu.c| 6 +++--- target/openrisc/cpu.h | 2 +- target/openrisc/mmu.c | 8 target/openrisc/mmu_helper.c | 6 +++--- target/ppc/cpu.h | 2 +- target/ppc/mmu_helper.c | 4 ++-- target/ppc/user_only_helper.c | 2 +- target/s390x/excp_helper.c| 4 ++-- target/s390x/internal.h | 2 +- target/s390x/mem_helper.c | 8 target/sh4/cpu.h | 2 +- target/sh4/helper.c | 4 ++-- target/sh4/op_helper.c| 6 +++--- target/sparc/cpu.h| 2 +- target/sparc/ldst_helper.c| 6 +++--- target/sparc/mmu_helper.c | 6 +++--- target/tilegx/cpu.c | 4 ++-- target/tricore/op_helper.c| 4 ++-- target/unicore32/cpu.h| 2 +- target/unicore32/helper.c | 2 +- target/unicore32/op_helper.c | 6 +++--- target/unicore32/softmmu.c| 2 +- target/xtensa/op_helper.c | 4 ++-- 58 files changed, 129 insertions(+), 121 deletions(-) diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c index 8fd84209df..05439039e9 100644 --- a/accel/tcg/cputlb.c +++ b/accel/tcg/cputlb.c @@ -880,7 +880,7 @@ tb_page_addr_t get_page_addr_code(CPUArchState *env, target_ulong addr) if (unlikely(env->tlb_table[mmu_idx][index].addr_code != (addr & (TARGET_PAGE_MASK | TLB_INVALID_MASK { if (!VICTIM_TLB_HIT(addr_read, addr)) { -tlb_fill(ENV_GET_CPU(env), addr, MMU_INST_FETCH, mmu_idx, 0); +tlb_fill(ENV_GET_CPU(env), addr, 0, MMU_INST_FETCH, mmu_idx, 0); } } iotlbentry = &env->iotlb[mmu_idx][index]; @@ -928,7 +928,7 @@ tb_page_addr_t get_page_addr_code(CPUArchState *env, target_ulong addr) * Otherwise the function will return, and there will be a valid * entry in the TLB for this access. */ -void probe_write(CPUArchState *env, target_ulong addr, int mmu_idx, +void probe_write(CPUArchState *env, target_ulong addr, int size, int mmu_idx, uintptr_t retaddr) { int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); @@ -938,7 +938,8 @@ void probe_write(CPUArchState *env, target_ulong addr, int mmu_idx, != (tlb_addr & (TARGET_PAGE_MASK | TLB_INVALID_MASK))) { /* TLB entry is for a different page */ if (!VICTIM_TLB_HIT(addr_write, addr)) { -tlb_fill(ENV_GET_CPU(env), addr, MMU_DATA_STORE, mmu_idx, retaddr); +tlb_fill(ENV_GET_CPU(env), addr, size, MMU_DATA_STORE, + mm
[Qemu-devel] [PATCH v3 4/7] target/m68k: add index parameter to gen_load()/gen_store() and Co.
The instruction "moves" can select source and destination address space (user or kernel). This patch modifies all the load/store functions to be able to provide the address space the caller wants to use instead of using the current one. All the callers are modified to provide the default address space to these functions. Signed-off-by: Laurent Vivier --- v3: fix checkpatch.pl errors/warnings v2: new patch in the series needed by "add moves" to pass the MMU index to gen_load()/gen_store(). target/m68k/translate.c | 125 +--- 1 file changed, 66 insertions(+), 59 deletions(-) diff --git a/target/m68k/translate.c b/target/m68k/translate.c index af70825480..00a5bce6a3 100644 --- a/target/m68k/translate.c +++ b/target/m68k/translate.c @@ -281,10 +281,10 @@ static inline void gen_addr_fault(DisasContext *s) /* Generate a load from the specified address. Narrow values are sign extended to full register width. */ -static inline TCGv gen_load(DisasContext * s, int opsize, TCGv addr, int sign) +static inline TCGv gen_load(DisasContext *s, int opsize, TCGv addr, +int sign, int index) { TCGv tmp; -int index = IS_USER(s); tmp = tcg_temp_new_i32(); switch(opsize) { case OS_BYTE: @@ -309,9 +309,9 @@ static inline TCGv gen_load(DisasContext * s, int opsize, TCGv addr, int sign) } /* Generate a store. */ -static inline void gen_store(DisasContext *s, int opsize, TCGv addr, TCGv val) +static inline void gen_store(DisasContext *s, int opsize, TCGv addr, TCGv val, + int index) { -int index = IS_USER(s); switch(opsize) { case OS_BYTE: tcg_gen_qemu_st8(val, addr, index); @@ -336,13 +336,13 @@ typedef enum { /* Generate an unsigned load if VAL is 0 a signed load if val is -1, otherwise generate a store. */ static TCGv gen_ldst(DisasContext *s, int opsize, TCGv addr, TCGv val, - ea_what what) + ea_what what, int index) { if (what == EA_STORE) { -gen_store(s, opsize, addr, val); +gen_store(s, opsize, addr, val, index); return store_dummy; } else { -return gen_load(s, opsize, addr, what == EA_LOADS); +return gen_load(s, opsize, addr, what == EA_LOADS, index); } } @@ -464,7 +464,7 @@ static TCGv gen_lea_indexed(CPUM68KState *env, DisasContext *s, TCGv base) } if ((ext & 3) != 0) { /* memory indirect */ -base = gen_load(s, OS_LONG, add, 0); +base = gen_load(s, OS_LONG, add, 0, IS_USER(s)); if ((ext & 0x44) == 4) { add = gen_addr_index(s, ext, tmp); tcg_gen_add_i32(tmp, add, base); @@ -793,7 +793,8 @@ static TCGv gen_lea(CPUM68KState *env, DisasContext *s, uint16_t insn, a write otherwise it is a read (0 == sign extend, -1 == zero extend). ADDRP is non-null for readwrite operands. */ static TCGv gen_ea_mode(CPUM68KState *env, DisasContext *s, int mode, int reg0, -int opsize, TCGv val, TCGv *addrp, ea_what what) +int opsize, TCGv val, TCGv *addrp, ea_what what, +int index) { TCGv reg, tmp, result; int32_t offset; @@ -817,10 +818,10 @@ static TCGv gen_ea_mode(CPUM68KState *env, DisasContext *s, int mode, int reg0, } case 2: /* Indirect register */ reg = get_areg(s, reg0); -return gen_ldst(s, opsize, reg, val, what); +return gen_ldst(s, opsize, reg, val, what, index); case 3: /* Indirect postincrement. */ reg = get_areg(s, reg0); -result = gen_ldst(s, opsize, reg, val, what); +result = gen_ldst(s, opsize, reg, val, what, index); if (what == EA_STORE || !addrp) { TCGv tmp = tcg_temp_new(); if (reg0 == 7 && opsize == OS_BYTE && @@ -844,7 +845,7 @@ static TCGv gen_ea_mode(CPUM68KState *env, DisasContext *s, int mode, int reg0, *addrp = tmp; } } -result = gen_ldst(s, opsize, tmp, val, what); +result = gen_ldst(s, opsize, tmp, val, what, index); if (what == EA_STORE || !addrp) { delay_set_areg(s, reg0, tmp, false); } @@ -863,7 +864,7 @@ static TCGv gen_ea_mode(CPUM68KState *env, DisasContext *s, int mode, int reg0, *addrp = tmp; } } -return gen_ldst(s, opsize, tmp, val, what); +return gen_ldst(s, opsize, tmp, val, what, index); case 7: /* Other */ switch (reg0) { case 0: /* Absolute short. */ @@ -904,11 +905,11 @@ static TCGv gen_ea_mode(CPUM68KState *env, DisasContext *s, int mode, int reg0, } static TCGv gen_ea(CPUM68KState *env, DisasContext *s, uint16_t insn, - int opsize, TCGv val, TCGv *addrp, ea_what what) + int opsize, TCGv val, TCGv *addrp, ea_what what,
Re: [Qemu-devel] [PATCH 0/7] CAN bus support for QEMU (SJA1000 PCI so far)
Hello Konrad, thanks for review. On Friday 12 of January 2018 11:43:18 KONRAD Frederic wrote: > You should add that to the title as well: > > git format-patch ... --subject-prefix="PATCH V3" ... > > to avoid any confusion. OK, I add V4. > You need to run the ./scripts/checkpatch.pl on your patches to > check the coding-style before submitting. I have run ./scripts/checkpatch.pl on patches but I have ignored warnings for comments overflowing column 80, because of better readability, same for //#define DEBUG_CAN which seems to be used quite often in the rest of the QEMU code. But I (try hard) switch to the strict mode for next version. I follow all your comments. I have send e-mail to Deniz Eren to provide his signed-off. I have received plain patches from him without this line. I trust that they have been intended to be integrated/released under GPL but I am not sure if rules allows to add the line myself. As for the inline can_bus_filter_match > > +static inline > > +int can_bus_filter_match(struct qemu_can_filter *filter, qemu_canid_t > > can_id) +{ > > +int m; > > +if (((can_id | filter->can_mask) & QEMU_CAN_ERR_FLAG)) { > > +return (filter->can_mask & QEMU_CAN_ERR_FLAG) != 0; > > +} > > +m = (can_id & filter->can_mask) == (filter->can_id & > > filter->can_mask); +return filter->can_id & QEMU_CAN_INV_FILTER ? !m > > : m; > > +} > Is it really required to inline this? I agree that it size is on upper size for inline. My initial version has next form which would be better to be inlined int can_bus_filter_match(struct qemu_can_filter *filter, qemu_canid_t can_id) { return (can_id & filter->can_mask) == (filter->can_id & filter->can_mask); } but I decided then to add more SocketCAN like behavior to can use it for CAN bus clients filters list in the future. Function call cost is not so high today. The function can be sometimes optimized to basic form when it is inlined and actual masks computation eliminates some cases, but again branch predictor should take care about that. > > +void can_sja_hardware_reset(CanSJA1000State *s) > Is something able to reset the chip outside? I have found and supported more SJA1000 based CAN cards with reset option during my LinCAN driver development in the past. The card have special register or address which allows to activate RESET pin of SJA1000. So even that reset cannot be activated externally on currently supported cards, I would keep that function available for future boards emulation if there is no objection. Best wishes, Pavel
[Qemu-devel] [PATCH v2] i.MX: Fix FEC/ENET receive funtions
The actual imx_eth_enable_rx() function is buggy. It updates s->regs[ENET_RDAR] after calling qemu_flush_queued_packets(). qemu_flush_queued_packets() is going to call imx_XXX_receive() which itself is going to call imx_eth_enable_rx(). By updating s->regs[ENET_RDAR] after calling qemu_flush_queued_packets() we end up updating the register with an outdated value which might lead to disabling the receive function in the i.MX FEC/ENET device. This patch change the place where the register update is done so that the register value stays up to date and the receive function can keep running. Reported-by: Fyleo Tested-by: Fyleo Signed-off-by: Jean-Christophe Dubois --- Change since v1: 1. Rebase the patch on the updated master branch hw/net/imx_fec.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c index 4fb48f62ba..9506f9b69f 100644 --- a/hw/net/imx_fec.c +++ b/hw/net/imx_fec.c @@ -595,19 +595,16 @@ static void imx_eth_do_tx(IMXFECState *s, uint32_t index) static void imx_eth_enable_rx(IMXFECState *s, bool flush) { IMXFECBufDesc bd; -bool rx_ring_full; imx_fec_read_bd(&bd, s->rx_descriptor); -rx_ring_full = !(bd.flags & ENET_BD_E); +s->regs[ENET_RDAR] = (bd.flags & ENET_BD_E) ? ENET_RDAR_RDAR : 0; -if (rx_ring_full) { +if (!s->regs[ENET_RDAR]) { FEC_PRINTF("RX buffer full\n"); } else if (flush) { qemu_flush_queued_packets(qemu_get_queue(s->nic)); } - -s->regs[ENET_RDAR] = rx_ring_full ? 0 : ENET_RDAR_RDAR; } static void imx_eth_reset(DeviceState *d) @@ -866,7 +863,6 @@ static void imx_eth_write(void *opaque, hwaddr offset, uint64_t value, case ENET_RDAR: if (s->regs[ENET_ECR] & ENET_ECR_ETHEREN) { if (!s->regs[index]) { -s->regs[index] = ENET_RDAR_RDAR; imx_eth_enable_rx(s, true); } } else { -- 2.14.1
Re: [Qemu-devel] [PATCH v9 07/13] xilinx_spips: Add support for RX discard and RX drain
On 13 January 2018 at 01:04, francisco iglesias wrote: > CID 1383841 (#2 of 4): Uninitialized scalar variable (UNINIT)29. > uninit_use_in_call: Using uninitialized value (uint32_t)tx_rx[0] when > calling > ssi_transfer. > > This is correct, tx_rx is used uninitialized but since we are transmitting > dummy cycles the transmitted value (tx_rx[0] in this case) is not used (by > the flashes), this the reason the code looks like that. Would you like me to > create a patch for quieting coverity here anyway? tx_rx[0] = ssi_transfer(s->spi[bus], (uint32_t)tx_rx[0]); is effectively using the value (since you don't know what the thing on the other end of that function is going to do with the value you pass it. You really don't want to make this code's correctness depend on the called function never looking at its argument. If you want to pass a dummy value why not just use constant 0 or something rather than an uninitialized variable ? I think quieting coverity is probably a good idea anyway for the other false positives (it sounds like it hasn't been able to determine that paths in the rest of the loop can only occur if the top of the loop went through a path initializing the variable. Code like that is also hard for humans to comprehend, though...) thanks -- PMM
Re: [Qemu-devel] [PATCH 6/8] vl.c: disallow command line fw cfg without opt/
On 01/12/2018 11:06 AM, Marc-André Lureau wrote: Hi On Thu, Dec 7, 2017 at 10:30 PM, wrote: From: "Michael S. Tsirkin" Allowing arbitary file names on command line is setting us up for failure: future guests will look for a specific QEMU-specified name and will get confused finding a user file there. Why is this part of a IPMI series? It's been so long I don't remember any more. I think I suggested a simpler fix when Michael posted his, and it ended up in my queue, because at one point the IPMI code was messing around in fw_cfg. But you are right, it doesn't belong here. There was a lengthy discussion on this patch earlier, could you summarize the conclusion? I think there was no conclusion. This patch should just be dropped. Thanks for this and your other comments. -corey thanks Signed-off-by: Michael S. Tsirkin [Change "warning" to "error" in the error report.] Signed-off-by: Corey Minyard --- vl.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/vl.c b/vl.c index 1ad1c04..fae9300 100644 --- a/vl.c +++ b/vl.c @@ -2400,8 +2400,9 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, Error **errp) return -1; } if (strncmp(name, "opt/", 4) != 0) { -warn_report("externally provided fw_cfg item names " -"should be prefixed with \"opt/\""); +error_report("error: externally provided fw_cfg item names " + "should be prefixed with \"opt/\""); +return -1; } if (nonempty_str(str)) { size = strlen(str); /* NUL terminator NOT included in fw_cfg blob */ -- 2.7.4
[Qemu-devel] [PATCH 2/3] linux-user, m68k: select CPU according to ELF header values
M680x0 doesn't support the same set of instructions as ColdFire, so we can't use "any" CPU type to execute m68020 instructions. We select CPU type ("m68020" or "any" for ColdFire) according to the ELF header. If we can't, we use by default the value used until now: "any". Signed-off-by: Laurent Vivier --- linux-user/main.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/linux-user/main.c b/linux-user/main.c index 9ce90ae634..2fc2267fd4 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -4267,6 +4267,16 @@ static const char *get_cpu_model(int fd) #elif defined(TARGET_UNICORE32) return "any"; #elif defined(TARGET_M68K) +int ret; +uint32_t eflags; + +ret = get_elf_eflags(fd, &eflags); +if (ret == 0 && eflags == 0) { +/* 680x0 */ +return "m68020"; +} + +/* Coldfire */ return "any"; #elif defined(TARGET_SPARC) #ifdef TARGET_SPARC64 -- 2.14.3
[Qemu-devel] [PATCH 0/3] linux-user: select CPU type according ELF header values
This idea has been suggested to me before by Philippe Mathieu-Daudé, and recently YunQiang Su has proposed a patch to manage the MIPS r6 case. Based on this, this series tries to clean-up the original patch, and introduces the use for m68k architecture and port the patch from YunQiang Su. Laurent Vivier (1): linux-user,m68k: select CPU according to ELF header values YunQiang Su (2): linux-user: introduce functions to detect CPU type linux-user: MIPS set cpu to r6 CPU if binary is R6 include/elf.h| 4 ++ linux-user/elfload.c | 37 linux-user/main.c| 121 +++ linux-user/qemu.h| 1 + 4 files changed, 115 insertions(+), 48 deletions(-) -- 2.14.3
[Qemu-devel] [PATCH 3/3] linux-user: MIPS set cpu to r6 CPU if binary is R6
From: YunQiang Su So here we need to detect the version of binaries and set cpu_model for it. [lv: original patch modified to move code into get_cpu_model()] Signed-off-by: Laurent Vivier --- Notes: YunQiang Su, please add your Signed-off-by that was missing in your original patch. include/elf.h | 4 linux-user/main.c | 10 ++ 2 files changed, 14 insertions(+) diff --git a/include/elf.h b/include/elf.h index e8a515ce3d..f2104809b1 100644 --- a/include/elf.h +++ b/include/elf.h @@ -40,6 +40,10 @@ typedef int64_t Elf64_Sxword; #define EF_MIPS_ARCH_5 0x4000 /* -mips5 code. */ #define EF_MIPS_ARCH_320x5000 /* MIPS32 code. */ #define EF_MIPS_ARCH_640x6000 /* MIPS64 code. */ +#define EF_MIPS_ARCH_32R2 0x7000 /* MIPS32r2 code. */ +#define EF_MIPS_ARCH_64R2 0x8000 /* MIPS64r2 code. */ +#define EF_MIPS_ARCH_32R6 0x9000 /* MIPS32r6 code. */ +#define EF_MIPS_ARCH_64R6 0xa000 /* MIPS64r6 code. */ /* The ABI of a file. */ #define EF_MIPS_ABI_O320x1000 /* O32 ABI. */ diff --git a/linux-user/main.c b/linux-user/main.c index 2fc2267fd4..3229ef079e 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -4285,9 +4285,19 @@ static const char *get_cpu_model(int fd) return "Fujitsu MB86904"; #endif #elif defined(TARGET_MIPS) +int ret; +uint32_t eflags; + +ret = get_elf_eflags(fd, &eflags); #if defined(TARGET_ABI_MIPSN32) || defined(TARGET_ABI_MIPSN64) +if (ret == 0 && (eflags & EF_MIPS_ARCH_64R6) != 0) { +return "I6400"; +} return "5KEf"; #else +if (ret == 0 && (eflags & EF_MIPS_ARCH_32R6) != 0) { +return "mips32r6-generic"; +} return "24Kf"; #endif #elif defined TARGET_OPENRISC -- 2.14.3
[Qemu-devel] [PATCH 1/3] linux-user: introduce functions to detect CPU type
From: YunQiang Su Move CPU type name selection to a function, and add a function to return ELF e_flags. [lv: splitted the patch and some cleanup in get_elf_eflags()] Signed-off-by: Laurent Vivier --- Notes: YunQiang Su, please add your Signed-off-by that was missing in your original patch. linux-user/elfload.c | 37 +++ linux-user/main.c| 101 +++ linux-user/qemu.h| 1 + 3 files changed, 91 insertions(+), 48 deletions(-) diff --git a/linux-user/elfload.c b/linux-user/elfload.c index 20f3d8c2c3..03244f5e97 100644 --- a/linux-user/elfload.c +++ b/linux-user/elfload.c @@ -2376,6 +2376,43 @@ give_up: g_free(syms); } +int get_elf_eflags(int fd, uint32_t *eflags) +{ +struct elfhdr ehdr; +off_t offset; +int ret; + +/* Read ELF header */ +offset = lseek(fd, 0, SEEK_SET); +if (offset == (off_t) -1) { +return -1; +} +ret = read(fd, &ehdr, sizeof(ehdr)); +if (ret < sizeof(ehdr)) { +return -1; +} +offset = lseek(fd, offset, SEEK_SET); +if (offset == (off_t) -1) { +return -1; +} + +/* Check ELF signature */ +if (!elf_check_ident(&ehdr)) { +return -1; +} + +/* check header */ +bswap_ehdr(&ehdr); +if (!elf_check_ehdr(&ehdr)) { +return -1; +} + +/* return architecture id */ +*eflags = ehdr.e_flags; + +return 0; +} + int load_elf_binary(struct linux_binprm *bprm, struct image_info *info) { struct image_info interp_info; diff --git a/linux-user/main.c b/linux-user/main.c index 450eb3ce65..9ce90ae634 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -4254,6 +4254,49 @@ static int parse_args(int argc, char **argv) return optind; } +static const char *get_cpu_model(int fd) +{ +#if defined(TARGET_I386) +#ifdef TARGET_X86_64 +return "qemu64"; +#else +return "qemu32"; +#endif +#elif defined(TARGET_ARM) +return "any"; +#elif defined(TARGET_UNICORE32) +return "any"; +#elif defined(TARGET_M68K) +return "any"; +#elif defined(TARGET_SPARC) +#ifdef TARGET_SPARC64 +return "TI UltraSparc II"; +#else +return "Fujitsu MB86904"; +#endif +#elif defined(TARGET_MIPS) +#if defined(TARGET_ABI_MIPSN32) || defined(TARGET_ABI_MIPSN64) +return "5KEf"; +#else +return "24Kf"; +#endif +#elif defined TARGET_OPENRISC +return "or1200"; +#elif defined(TARGET_PPC) +# ifdef TARGET_PPC64 +return "POWER8"; +# else +return "750"; +# endif +#elif defined TARGET_SH4 +return "sh7785"; +#elif defined TARGET_S390X +return "qemu"; +#else +return "any"; +#endif +} + int main(int argc, char **argv, char **envp) { struct target_pt_regs regs1, *regs = ®s1; @@ -4318,46 +4361,17 @@ int main(int argc, char **argv, char **envp) init_qemu_uname_release(); +execfd = qemu_getauxval(AT_EXECFD); +if (execfd == 0) { +execfd = open(filename, O_RDONLY); +if (execfd < 0) { +printf("Error while loading %s: %s\n", filename, strerror(errno)); +_exit(EXIT_FAILURE); +} +} + if (cpu_model == NULL) { -#if defined(TARGET_I386) -#ifdef TARGET_X86_64 -cpu_model = "qemu64"; -#else -cpu_model = "qemu32"; -#endif -#elif defined(TARGET_ARM) -cpu_model = "any"; -#elif defined(TARGET_UNICORE32) -cpu_model = "any"; -#elif defined(TARGET_M68K) -cpu_model = "any"; -#elif defined(TARGET_SPARC) -#ifdef TARGET_SPARC64 -cpu_model = "TI UltraSparc II"; -#else -cpu_model = "Fujitsu MB86904"; -#endif -#elif defined(TARGET_MIPS) -#if defined(TARGET_ABI_MIPSN32) || defined(TARGET_ABI_MIPSN64) -cpu_model = "5KEf"; -#else -cpu_model = "24Kf"; -#endif -#elif defined TARGET_OPENRISC -cpu_model = "or1200"; -#elif defined(TARGET_PPC) -# ifdef TARGET_PPC64 -cpu_model = "POWER8"; -# else -cpu_model = "750"; -# endif -#elif defined TARGET_SH4 -cpu_model = "sh7785"; -#elif defined TARGET_S390X -cpu_model = "qemu"; -#else -cpu_model = "any"; -#endif +cpu_model = get_cpu_model(execfd); } tcg_exec_init(0); /* NOTE: we need to init the CPU at this stage to get @@ -4450,15 +4464,6 @@ int main(int argc, char **argv, char **envp) cpu->opaque = ts; task_settid(ts); -execfd = qemu_getauxval(AT_EXECFD); -if (execfd == 0) { -execfd = open(filename, O_RDONLY); -if (execfd < 0) { -printf("Error while loading %s: %s\n", filename, strerror(errno)); -_exit(EXIT_FAILURE); -} -} - ret = loader_exec(execfd, filename, target_argv, target_environ, regs, info, &bprm); if (ret != 0) { diff --git a/linux-user/qemu.h b/linux-user/qemu.h index 4edd7d0c08..c2a701163c 100644 --- a/linux-user/qemu.h +++ b/linux-user/qemu.h @@ -188,6 +188,7 @@ int loader_exec(int fdexec, const char *filename, char **argv, char **envp,
Re: [Qemu-devel] [RFC 01/23] scripts: Add decodetree.py
Hi Richard, +# Field examples: +# +# %disp 0:s16 -- sextract(i, 0, 16) +# %imm9 16:6 10:3 -- extract(i, 16, 6) << 3 | extract(i, 10, 3) startindex:endindex for unnamed_field is more intuitive. As any ISA manual would specify those. +# +# It is recommended, but not required, that all field_ref and args_ref +# appear at the end of the line, not interleaving with fixedbit_elf or +# field_elt. This seems not very intuitive to me. Why specify the field_refs at the end and fill the space they occupy with '.'? I'd prefer interleaving the reference to make it more readable (which doesn't work so far) +# Pattern examples: +# +# addl_r 01 . . 000 . @opr +# addl_i 01 . . 000 . @opi We should note in the documentation where the LSB is. Got that the wrong way in the first time I implemented RISC-V . +def str_indent(c): +"""Return a string with C spaces""" +r = '' +for i in range(0, c): +r += ' ' +return r return ' ' * c +def popcount(b): +b = (b & 0x) + ((b >> 1) & 0x) +b = (b & 0x) + ((b >> 2) & 0x) +b = (b & 0x0f0f0f0f) + ((b >> 4) & 0x0f0f0f0f) +b = (b + (b >> 8)) & 0x00ff00ff +b = (b + (b >> 16)) & 0x +return b We really don't need to look for efficiency here? return bin(b).count('1') +def parse_arguments(lineno, name, toks): +"""Parse one argument set from TOKS at LINENO""" +global arguments +global re_ident + +flds = [] arg_fields is a better name. Likewise for other places containing flds. Such that one knows where this fields belongs to. +elif name[0] == '&': +parse_arguments(lineno, name[1:], toks) +elif name[0] == '@': +parse_generic(lineno, True, name[1:], toks) +else: +parse_generic(lineno, False, name, toks) +toks = [] Why do we have a generic function for pat and fields? Rather make it two parse functions and split subparsings that they share into generic functions, instead of doing the distinction in parse_generic(). +for p in pats: +pnames.append(p.name) +#pdb.set_trace() Guess you forgot that ;) +long_opts = [ 'decode=', 'translate=', 'header=', 'output=' ] +try: +(opts, args) = getopt.getopt(sys.argv[1:], 'h:o:', long_opts) +except getopt.GetoptError as err: +error(0, err) +for o, a in opts: +if o in ('-h', '--header'): +h_file = a +elif o in ('-o', '--output'): +c_file = a +elif o == '--decode': +decode_function = a +elif o == '--translate': +translate_prefix = a +else: +assert False, 'unhandled option' Please use argsparse On a more general note. Do you really need to invent your own description format + parser? Why not reuse an existing markup language? Can you give me a rationale? That said, your description language looks very readable, even though I'm not a big fan of symbol prefixes ;). Thanks for improving my original approach. Cheers, Bastian
Re: [Qemu-devel] [RFC 01/23] scripts: Add decodetree.py
On 13 January 2018 at 17:14, Bastian Koppelmann wrote: > Hi Richard, > > +# Field examples: > +# > +# %disp 0:s16 -- sextract(i, 0, 16) > +# %imm9 16:6 10:3 -- extract(i, 16, 6) << 3 | extract(i, 10, 3) > > startindex:endindex for unnamed_field is more intuitive. As any ISA > manual would specify those. I find start:len easier to work with personally (and it matches what we use in QEMU for the extract/deposit APIs, and for what little it's worth, what RISU data file syntax does). You're probably right that ISA manuals will tend to give bit positions though. thanks -- PMM
Re: [Qemu-devel] [PATCH v9 07/13] xilinx_spips: Add support for RX discard and RX drain
On Saturday, 13 January 2018, Peter Maydell wrote: > On 13 January 2018 at 01:04, francisco iglesias > wrote: > > CID 1383841 (#2 of 4): Uninitialized scalar variable (UNINIT)29. > > uninit_use_in_call: Using uninitialized value (uint32_t)tx_rx[0] when > > calling > > ssi_transfer. > > > > This is correct, tx_rx is used uninitialized but since we are > transmitting > > dummy cycles the transmitted value (tx_rx[0] in this case) is not used > (by > > the flashes), this the reason the code looks like that. Would you like > me to > > create a patch for quieting coverity here anyway? > > tx_rx[0] = ssi_transfer(s->spi[bus], (uint32_t)tx_rx[0]); > > is effectively using the value (since you don't know what the > thing on the other end of that function is going to do with the > value you pass it. You really don't want to make this code's > correctness depend on the called function never looking at > its argument. If you want to pass a dummy value why not > just use constant 0 or something rather than an uninitialized > variable ? > > I think quieting coverity is probably a good idea anyway for > the other false positives (it sounds like it hasn't been able > to determine that paths in the rest of the loop can only occur > if the top of the loop went through a path initializing the > variable. Code like that is also hard for humans to comprehend, > though...) > > thanks > -- PMM > Hi Peter, I'll create a patch attempting to correct the coverity complains and comeback! Bets regards, Francisco Iglesias
Re: [Qemu-devel] [PATCH 5/7] CAN bus PCM-3680I PCI (dual SJA1000 channel) emulation added.
Signed-off-by: Deniz Eren I’ve tested and used this work actively at for developing driverless Straddle and AGV embedded software with QNX and Linux hosts with Advantech CAN-bus cards. Sent from my iPhone Deniz Eren +61 400 307 762 > On 7 Jan 2018, at 7:47 am, p...@cmp.felk.cvut.cz wrote: > > From: Deniz Eren > > Signed-off-by: Pavel Pisa > --- > hw/can/Makefile.objs | 1 + > hw/can/can_pcm3680_pci.c | 335 +++ > 2 files changed, 336 insertions(+) > create mode 100644 hw/can/can_pcm3680_pci.c > > diff --git a/hw/can/Makefile.objs b/hw/can/Makefile.objs > index c9d07b9b16..6a328f0c3a 100644 > --- a/hw/can/Makefile.objs > +++ b/hw/can/Makefile.objs > @@ -9,4 +9,5 @@ common-obj-y += can_host_stub.o > endif > common-obj-$(CONFIG_CAN_SJA1000) += can_sja1000.o > common-obj-$(CONFIG_CAN_PCI) += can_kvaser_pci.o > +common-obj-$(CONFIG_CAN_PCI) += can_pcm3680_pci.o > endif > diff --git a/hw/can/can_pcm3680_pci.c b/hw/can/can_pcm3680_pci.c > new file mode 100644 > index 00..692aab6ab8 > --- /dev/null > +++ b/hw/can/can_pcm3680_pci.c > @@ -0,0 +1,335 @@ > +/* > + * PCM-3680i PCI CAN device (SJA1000 based) emulation > + * > + * Copyright (c) 2016 Deniz Eren (deniz.e...@icloud.com) > + * > + * Based on Kvaser PCI CAN device (SJA1000 based) emulation implemented by > + * Jin Yang and Pavel Pisa > + * > + * 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 "qemu/osdep.h" > +#include "qemu/event_notifier.h" > +#include "qemu/thread.h" > +#include "qemu/sockets.h" > +#include "qemu/error-report.h" > +#include "chardev/char.h" > +#include "hw/hw.h" > +#include "hw/pci/pci.h" > +#include "can/can_emu.h" > + > +#include "can_sja1000.h" > + > +#define TYPE_CAN_PCI_DEV "pcm3680_pci" > + > +#define PCM3680i_PCI_DEV(obj) \ > +OBJECT_CHECK(Pcm3680iPCIState, (obj), TYPE_CAN_PCI_DEV) > + > +#ifndef PCM3680i_PCI_VENDOR_ID1 > +#define PCM3680i_PCI_VENDOR_ID1 0x13fe/* the PCI device and vendor > IDs */ > +#endif > + > +#ifndef PCM3680i_PCI_DEVICE_ID1 > +#define PCM3680i_PCI_DEVICE_ID1 0xc002 > +#endif > + > +#define PCM3680i_PCI_SJA_RANGE 0x200 > + > +#define PCM3680i_PCI_BYTES_PER_SJA 0x20 > + > +typedef struct Pcm3680iPCIState { > +/*< private >*/ > +PCIDevice dev; > +/*< public >*/ > +MemoryRegionsja_io[2]; > + > +CanSJA1000State sja_state[2]; > +qemu_irqirq; > + > +char*model; /* The model that support, only SJA1000 now. */ > +char*canbus[2]; > +char*host[2]; > +} Pcm3680iPCIState; > + > +static void pcm3680i_pci_irq_raise(void *opaque) > +{ > +Pcm3680iPCIState *d = (Pcm3680iPCIState *)opaque; > + > +qemu_irq_raise(d->irq); > +} > + > +static void pcm3680i_pci_irq_lower(void *opaque) > +{ > +Pcm3680iPCIState *d = (Pcm3680iPCIState *)opaque; > + > +qemu_irq_lower(d->irq); > +} > + > +static void > +pcm3680i_pci_reset(void *opaque) > +{ > +Pcm3680iPCIState *d = (Pcm3680iPCIState *)opaque; > +CanSJA1000State *s1 = &d->sja_state[0]; > +CanSJA1000State *s2 = &d->sja_state[1]; > + > +can_sja_hardware_reset(s1); > +can_sja_hardware_reset(s2); > +} > + > +static uint64_t pcm3680i_pci_sja1_io_read(void *opaque, hwaddr addr, > + unsigned size) > +{ > +Pcm3680iPCIState *d = opaque; > +CanSJA1000State *s = &d->sja_state[0]; > + > +if (addr >= PCM3680i_PCI_BYTES_PER_SJA) { > +return 0; > +} > + > +return can_sja_mem_read(s, addr, size); > +} > + > +static void pcm3680i_pci_sja1_io_write(void *opaque, hwaddr addr, > + uint64_t data, unsigned size) > +{ > +Pcm3680iPCIState *d = opaque; > +CanSJA1000State *s = &d->sja_state[0]; > + > +if (addr >= PCM3680i_PCI_BYTES_PER_SJA) { > +return; > +} > + > +can_sja_mem_write(s, addr,
Re: [Qemu-devel] [PATCH 6/7] CAN bus MIOe-3680 PCI (dual SJA1000 channel) emulation added.
Signed-off-by: Deniz Eren Sent from my iPhone Deniz Eren +61 400 307 762 > On 7 Jan 2018, at 7:47 am, p...@cmp.felk.cvut.cz wrote: > > From: Deniz Eren > > Signed-off-by: Pavel Pisa > --- > hw/can/Makefile.objs | 1 + > hw/can/can_mioe3680_pci.c | 335 ++ > 2 files changed, 336 insertions(+) > create mode 100644 hw/can/can_mioe3680_pci.c > > diff --git a/hw/can/Makefile.objs b/hw/can/Makefile.objs > index 6a328f0c3a..8fcc455800 100644 > --- a/hw/can/Makefile.objs > +++ b/hw/can/Makefile.objs > @@ -10,4 +10,5 @@ endif > common-obj-$(CONFIG_CAN_SJA1000) += can_sja1000.o > common-obj-$(CONFIG_CAN_PCI) += can_kvaser_pci.o > common-obj-$(CONFIG_CAN_PCI) += can_pcm3680_pci.o > +common-obj-$(CONFIG_CAN_PCI) += can_mioe3680_pci.o > endif > diff --git a/hw/can/can_mioe3680_pci.c b/hw/can/can_mioe3680_pci.c > new file mode 100644 > index 00..799e74a7ac > --- /dev/null > +++ b/hw/can/can_mioe3680_pci.c > @@ -0,0 +1,335 @@ > +/* > + * MIOe-3680 PCI CAN device (SJA1000 based) emulation > + * > + * Copyright (c) 2016 Deniz Eren (deniz.e...@icloud.com) > + * > + * Based on Kvaser PCI CAN device (SJA1000 based) emulation implemented by > + * Jin Yang and Pavel Pisa > + * > + * 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 "qemu/osdep.h" > +#include "qemu/event_notifier.h" > +#include "qemu/thread.h" > +#include "qemu/sockets.h" > +#include "qemu/error-report.h" > +#include "chardev/char.h" > +#include "hw/hw.h" > +#include "hw/pci/pci.h" > +#include "can/can_emu.h" > + > +#include "can_sja1000.h" > + > +#define TYPE_CAN_PCI_DEV "mioe3680_pci" > + > +#define MIOe3680_PCI_DEV(obj) \ > +OBJECT_CHECK(Mioe3680PCIState, (obj), TYPE_CAN_PCI_DEV) > + > +#ifndef MIOe3680_PCI_VENDOR_ID1 > +#define MIOe3680_PCI_VENDOR_ID1 0x13fe/* the PCI device and vendor > IDs */ > +#endif > + > +#ifndef MIOe3680_PCI_DEVICE_ID1 > +#define MIOe3680_PCI_DEVICE_ID1 0xc302 > +#endif > + > +#define MIOe3680_PCI_SJA_RANGE 0x800 > + > +#define MIOe3680_PCI_BYTES_PER_SJA 0x80 > + > +typedef struct Mioe3680PCIState { > +/*< private >*/ > +PCIDevice dev; > +/*< public >*/ > +MemoryRegionsja_io[2]; > + > +CanSJA1000State sja_state[2]; > +qemu_irqirq; > + > +char*model; /* The model that support, only SJA1000 now. */ > +char*canbus[2]; > +char*host[2]; > +} Mioe3680PCIState; > + > +static void mioe3680_pci_irq_raise(void *opaque) > +{ > +Mioe3680PCIState *d = (Mioe3680PCIState *)opaque; > + > +qemu_irq_raise(d->irq); > +} > + > +static void mioe3680_pci_irq_lower(void *opaque) > +{ > +Mioe3680PCIState *d = (Mioe3680PCIState *)opaque; > + > +qemu_irq_lower(d->irq); > +} > + > +static void > +mioe3680_pci_reset(void *opaque) > +{ > +Mioe3680PCIState *d = (Mioe3680PCIState *)opaque; > +CanSJA1000State *s1 = &d->sja_state[0]; > +CanSJA1000State *s2 = &d->sja_state[1]; > + > +can_sja_hardware_reset(s1); > +can_sja_hardware_reset(s2); > +} > + > +static uint64_t mioe3680_pci_sja1_io_read(void *opaque, hwaddr addr, > + unsigned size) > +{ > +Mioe3680PCIState *d = opaque; > +CanSJA1000State *s = &d->sja_state[0]; > + > +if (addr >= MIOe3680_PCI_BYTES_PER_SJA) { > +return 0; > +} > + > +return can_sja_mem_read(s, addr >> 2, size); > +} > + > +static void mioe3680_pci_sja1_io_write(void *opaque, hwaddr addr, uint64_t > data, > + unsigned size) > +{ > +Mioe3680PCIState *d = opaque; > +CanSJA1000State *s = &d->sja_state[0]; > + > +if (addr >= MIOe3680_PCI_BYTES_PER_SJA) { > +return; > +} > + > +can_sja_mem_write(s, addr >> 2, data, size); > +} > + > +static uint64_t mioe3680_pci_sja2_io_read(void *opaque, hwaddr addr, > +
[Qemu-devel] [RFC PATCH 0/3] add helpers to be more explicit when using QOM abstract parent hooks
Hi, Learning how to implement QOM devices I found the pattern changing parent hooks when the parent is abstract not trivial to understand. This series add few helpers to have this pattern more explicit. Those functions deserve some comments, but before spending more time I'd like to get some feedbacks if this might be useful or not. If so, I can split the 3rd patch in many patches for each subsystem maintainers. This can also be use for the CPUClass->reset() method, dunno if useful but maybe to be consistent. Regards, Phil. Philippe Mathieu-Daudé (3): qdev: rename typedef qdev_resetfn() -> DeviceReset() qdev: add helpers to be more explicit when using abstract QOM parent functions qdev: use device_class_set_parent_realize/unrealize/reset() include/hw/qdev-core.h | 14 -- hw/core/qdev.c | 24 hw/i386/kvm/i8254.c| 4 ++-- hw/i386/kvm/i8259.c| 3 +-- hw/input/adb-kbd.c | 4 ++-- hw/input/adb-mouse.c | 4 ++-- hw/intc/arm_gic.c | 3 +-- hw/intc/arm_gic_kvm.c | 7 +++ hw/intc/arm_gicv3.c| 3 +-- hw/intc/arm_gicv3_its_kvm.c| 3 +-- hw/intc/arm_gicv3_kvm.c| 7 +++ hw/intc/i8259.c| 3 +-- hw/net/vmxnet3.c | 4 ++-- hw/pci-bridge/gen_pcie_root_port.c | 3 +-- hw/scsi/vmw_pvscsi.c | 4 ++-- hw/timer/i8254.c | 3 +-- hw/vfio/amd-xgbe.c | 4 ++-- hw/vfio/calxeda-xgmac.c| 4 ++-- hw/virtio/virtio-pci.c | 4 ++-- target/alpha/cpu.c | 4 ++-- target/arm/cpu.c | 4 ++-- target/cris/cpu.c | 4 ++-- target/hppa/cpu.c | 4 ++-- target/i386/cpu.c | 8 target/lm32/cpu.c | 5 ++--- target/m68k/cpu.c | 5 ++--- target/microblaze/cpu.c| 5 ++--- target/mips/cpu.c | 5 ++--- target/moxie/cpu.c | 5 ++--- target/nios2/cpu.c | 4 ++-- target/openrisc/cpu.c | 5 ++--- target/ppc/translate_init.c| 8 target/s390x/cpu.c | 4 ++-- target/sh4/cpu.c | 4 ++-- target/sparc/cpu.c | 4 ++-- target/tilegx/cpu.c| 4 ++-- target/tricore/cpu.c | 4 ++-- target/unicore32/cpu.c | 4 ++-- target/xtensa/cpu.c| 4 ++-- 39 files changed, 109 insertions(+), 90 deletions(-) -- 2.15.1
[Qemu-devel] [RFC PATCH 1/3] qdev: rename typedef qdev_resetfn() -> DeviceReset()
following the DeviceRealize and DeviceUnrealize typedefs, this unify a bit the new QOM API. Signed-off-by: Philippe Mathieu-Daudé --- include/hw/qdev-core.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index 0a71bf83f0..83db53b3f5 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -32,9 +32,9 @@ typedef enum DeviceCategory { typedef int (*qdev_initfn)(DeviceState *dev); typedef int (*qdev_event)(DeviceState *dev); -typedef void (*qdev_resetfn)(DeviceState *dev); typedef void (*DeviceRealize)(DeviceState *dev, Error **errp); typedef void (*DeviceUnrealize)(DeviceState *dev, Error **errp); +typedef void (*DeviceReset)(DeviceState *dev); typedef void (*BusRealize)(BusState *bus, Error **errp); typedef void (*BusUnrealize)(BusState *bus, Error **errp); @@ -117,7 +117,7 @@ typedef struct DeviceClass { bool hotpluggable; /* callbacks */ -void (*reset)(DeviceState *dev); +DeviceReset reset; DeviceRealize realize; DeviceUnrealize unrealize; -- 2.15.1
[Qemu-devel] [RFC PATCH 3/3] qdev: use device_class_set_parent_realize/unrealize/reset()
changes generated using the following Coccinelle patch: @@ type DeviceParentClass; DeviceParentClass *pc; DeviceClass *dc; identifier parent_fn; identifier child_fn; @@ ( +device_class_set_parent_realize(dc, child_fn, &pc->parent_fn); -pc->parent_fn = dc->realize; ... -dc->realize = child_fn; | +device_class_set_parent_unrealize(dc, child_fn, &pc->parent_fn); -pc->parent_fn = dc->unrealize; ... -dc->unrealize = child_fn; | +device_class_set_parent_reset(dc, child_fn, &pc->parent_fn); -pc->parent_fn = dc->reset; ... -dc->reset = child_fn; ) Signed-off-by: Philippe Mathieu-Daudé --- hw/i386/kvm/i8254.c| 4 ++-- hw/i386/kvm/i8259.c| 3 +-- hw/input/adb-kbd.c | 4 ++-- hw/input/adb-mouse.c | 4 ++-- hw/intc/arm_gic.c | 3 +-- hw/intc/arm_gic_kvm.c | 7 +++ hw/intc/arm_gicv3.c| 3 +-- hw/intc/arm_gicv3_its_kvm.c| 3 +-- hw/intc/arm_gicv3_kvm.c| 7 +++ hw/intc/i8259.c| 3 +-- hw/net/vmxnet3.c | 4 ++-- hw/pci-bridge/gen_pcie_root_port.c | 3 +-- hw/scsi/vmw_pvscsi.c | 4 ++-- hw/timer/i8254.c | 3 +-- hw/vfio/amd-xgbe.c | 4 ++-- hw/vfio/calxeda-xgmac.c| 4 ++-- hw/virtio/virtio-pci.c | 4 ++-- target/alpha/cpu.c | 4 ++-- target/arm/cpu.c | 4 ++-- target/cris/cpu.c | 4 ++-- target/hppa/cpu.c | 4 ++-- target/i386/cpu.c | 8 target/lm32/cpu.c | 5 ++--- target/m68k/cpu.c | 5 ++--- target/microblaze/cpu.c| 5 ++--- target/mips/cpu.c | 5 ++--- target/moxie/cpu.c | 5 ++--- target/nios2/cpu.c | 4 ++-- target/openrisc/cpu.c | 5 ++--- target/ppc/translate_init.c| 8 target/s390x/cpu.c | 4 ++-- target/sh4/cpu.c | 4 ++-- target/sparc/cpu.c | 4 ++-- target/tilegx/cpu.c| 4 ++-- target/tricore/cpu.c | 4 ++-- target/unicore32/cpu.c | 4 ++-- target/xtensa/cpu.c| 4 ++-- 37 files changed, 73 insertions(+), 88 deletions(-) diff --git a/hw/i386/kvm/i8254.c b/hw/i386/kvm/i8254.c index 521a58498a..13f20f47d9 100644 --- a/hw/i386/kvm/i8254.c +++ b/hw/i386/kvm/i8254.c @@ -315,8 +315,8 @@ static void kvm_pit_class_init(ObjectClass *klass, void *data) PITCommonClass *k = PIT_COMMON_CLASS(klass); DeviceClass *dc = DEVICE_CLASS(klass); -kpc->parent_realize = dc->realize; -dc->realize = kvm_pit_realizefn; +device_class_set_parent_realize(dc, kvm_pit_realizefn, +&kpc->parent_realize); k->set_channel_gate = kvm_pit_set_gate; k->get_channel_info = kvm_pit_get_channel_info; dc->reset = kvm_pit_reset; diff --git a/hw/i386/kvm/i8259.c b/hw/i386/kvm/i8259.c index b91e98074e..05394cdb7b 100644 --- a/hw/i386/kvm/i8259.c +++ b/hw/i386/kvm/i8259.c @@ -142,8 +142,7 @@ static void kvm_i8259_class_init(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); dc->reset = kvm_pic_reset; -kpc->parent_realize = dc->realize; -dc->realize = kvm_pic_realize; +device_class_set_parent_realize(dc, kvm_pic_realize, &kpc->parent_realize); k->pre_save = kvm_pic_get; k->post_load = kvm_pic_put; } diff --git a/hw/input/adb-kbd.c b/hw/input/adb-kbd.c index 354f56e41e..266aed1b7b 100644 --- a/hw/input/adb-kbd.c +++ b/hw/input/adb-kbd.c @@ -374,8 +374,8 @@ static void adb_kbd_class_init(ObjectClass *oc, void *data) ADBDeviceClass *adc = ADB_DEVICE_CLASS(oc); ADBKeyboardClass *akc = ADB_KEYBOARD_CLASS(oc); -akc->parent_realize = dc->realize; -dc->realize = adb_kbd_realizefn; +device_class_set_parent_realize(dc, adb_kbd_realizefn, +&akc->parent_realize); set_bit(DEVICE_CATEGORY_INPUT, dc->categories); adc->devreq = adb_kbd_request; diff --git a/hw/input/adb-mouse.c b/hw/input/adb-mouse.c index c9004233b8..47e88faf25 100644 --- a/hw/input/adb-mouse.c +++ b/hw/input/adb-mouse.c @@ -228,8 +228,8 @@ static void adb_mouse_class_init(ObjectClass *oc, void *data) ADBDeviceClass *adc = ADB_DEVICE_CLASS(oc); ADBMouseClass *amc = ADB_MOUSE_CLASS(oc); -amc->parent_realize = dc->realize; -dc->realize = adb_mouse_realizefn; +device_class_set_parent_realize(dc, adb_mouse_realizefn, +&amc->parent_realize); set_bit(DEVICE_CATEGORY_INPUT, dc->categories); adc->devreq = adb_mouse_request; diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index d701e49ff9..9e0a5ea725 100644 --- a/hw/intc/arm_gic.c +++ b/hw/intc/arm_gic.c @@ -1444,8 +1444,7 @@ static void arm_gic_class_init(ObjectClass *kl
[Qemu-devel] [RFC PATCH 2/3] qdev: add helpers to be more explicit when using abstract QOM parent functions
QOM API learning curve is quite hard, in particular when devices inherit from abstract parent. To be more explicit about when a device class change the parent hooks, add few helpers hoping a device class_init() will be easier to understand. Signed-off-by: Philippe Mathieu-Daudé --- include/hw/qdev-core.h | 10 ++ hw/core/qdev.c | 24 2 files changed, 34 insertions(+) diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index 83db53b3f5..0fc53b33d0 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -381,6 +381,16 @@ void qdev_machine_init(void); */ void device_reset(DeviceState *dev); +void device_class_set_parent_reset(DeviceClass *dc, + DeviceReset dev_reset, + DeviceReset *parent_reset); +void device_class_set_parent_realize(DeviceClass *dc, + DeviceRealize dev_realize, + DeviceRealize *parent_realize); +void device_class_set_parent_unrealize(DeviceClass *dc, + DeviceUnrealize dev_unrealize, + DeviceUnrealize *parent_unrealize); + const struct VMStateDescription *qdev_get_vmsd(DeviceState *dev); const char *qdev_fw_name(DeviceState *dev); diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 2951a5..a71cd264e2 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -1140,6 +1140,30 @@ static void device_class_init(ObjectClass *class, void *data) dc->user_creatable = true; } +void device_class_set_parent_reset(DeviceClass *dc, + DeviceReset dev_reset, + DeviceReset *parent_reset) +{ +*parent_reset = dc->reset; +dc->reset = dev_reset; +} + +void device_class_set_parent_realize(DeviceClass *dc, + DeviceRealize dev_realize, + DeviceRealize *parent_realize) +{ +*parent_realize = dc->realize; +dc->realize = dev_realize; +} + +void device_class_set_parent_unrealize(DeviceClass *dc, + DeviceUnrealize dev_unrealize, + DeviceUnrealize *parent_unrealize) +{ +*parent_unrealize = dc->unrealize; +dc->unrealize = dev_unrealize; +} + void device_reset(DeviceState *dev) { DeviceClass *klass = DEVICE_GET_CLASS(dev); -- 2.15.1
[Qemu-devel] [PATCH] usb-ccid: QOM'ify
Signed-off-by: Philippe Mathieu-Daudé --- hw/usb/ccid.h | 9 +-- hw/usb/ccid-card-emulated.c | 48 +++- hw/usb/ccid-card-passthru.c | 13 +- hw/usb/dev-smartcard-reader.c | 57 +++ 4 files changed, 60 insertions(+), 67 deletions(-) diff --git a/hw/usb/ccid.h b/hw/usb/ccid.h index 1f070116d6..d9a1cbd09f 100644 --- a/hw/usb/ccid.h +++ b/hw/usb/ccid.h @@ -28,20 +28,25 @@ typedef struct CCIDCardInfo CCIDCardInfo; * into the smartcard device (hw/ccid-card-*.c) */ typedef struct CCIDCardClass { +/*< private >*/ DeviceClass parent_class; +/*< public >*/ +DeviceRealize parent_realize; +DeviceUnrealize parent_unrealize; + const uint8_t *(*get_atr)(CCIDCardState *card, uint32_t *len); void (*apdu_from_guest)(CCIDCardState *card, const uint8_t *apdu, uint32_t len); -void (*exitfn)(CCIDCardState *card); -int (*initfn)(CCIDCardState *card); } CCIDCardClass; /* * state of the CCID Card device (i.e. hw/ccid-card-*.c) */ struct CCIDCardState { +/*< private >*/ DeviceState qdev; +/*< public >*/ uint32_tslot; /* For future use with multiple slot reader. */ }; diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c index e646eb243b..db2ed53773 100644 --- a/hw/usb/ccid-card-emulated.c +++ b/hw/usb/ccid-card-emulated.c @@ -27,6 +27,7 @@ */ #include "qemu/osdep.h" +#include "qapi/error.h" #include #include #include @@ -480,9 +481,9 @@ static uint32_t parse_enumeration(char *str, return ret; } -static int emulated_initfn(CCIDCardState *base) +static void emulated_realize(DeviceState *dev, Error **errp) { -EmulatedState *card = EMULATED_CCID_CARD(base); +EmulatedState *card = EMULATED_CCID_CARD(dev); VCardEmulError ret; const EnumTable *ptable; @@ -495,7 +496,8 @@ static int emulated_initfn(CCIDCardState *base) card->reader = NULL; card->quit_apdu_thread = 0; if (init_event_notifier(card) < 0) { -return -1; +error_setg(errp, TYPE_EMULATED_CCID ": event notifier creation failed"); +return; } card->backend = 0; @@ -505,11 +507,12 @@ static int emulated_initfn(CCIDCardState *base) } if (card->backend == 0) { -printf("backend must be one of:\n"); +error_setg(errp, TYPE_EMULATED_CCID ": no backend specified."); +printf("backend must be one of:\n"); /* TODO remove */ for (ptable = backend_enum_table; ptable->name != NULL; ++ptable) { printf("%s\n", ptable->name); } -return -1; +return; } /* TODO: a passthru backened that works on local machine. third card type?*/ @@ -517,39 +520,38 @@ static int emulated_initfn(CCIDCardState *base) if (card->cert1 != NULL && card->cert2 != NULL && card->cert3 != NULL) { ret = emulated_initialize_vcard_from_certificates(card); } else { -printf("%s: you must provide all three certs for" - " certificates backend\n", TYPE_EMULATED_CCID); -return -1; +error_setg(errp, TYPE_EMULATED_CCID ": you must provide all three " + "certs for certificates backend"); +return; } } else { if (card->backend != BACKEND_NSS_EMULATED) { -printf("%s: bad backend specified. The options are:\n%s (default)," -" %s.\n", TYPE_EMULATED_CCID, BACKEND_NSS_EMULATED_NAME, -BACKEND_CERTIFICATES_NAME); -return -1; +error_setg(errp, TYPE_EMULATED_CCID ": bad backend specified. " + "The options are: %s (default), %s.", + BACKEND_NSS_EMULATED_NAME, BACKEND_CERTIFICATES_NAME); +return; } if (card->cert1 != NULL || card->cert2 != NULL || card->cert3 != NULL) { -printf("%s: unexpected cert parameters to nss emulated backend\n", - TYPE_EMULATED_CCID); -return -1; +error_setg(errp, TYPE_EMULATED_CCID ": unexpected cert parameters " + "to nss emulated backend"); +return; } /* default to mirroring the local hardware readers */ ret = wrap_vcard_emul_init(NULL); } if (ret != VCARD_EMUL_OK) { -printf("%s: failed to initialize vcard\n", TYPE_EMULATED_CCID); -return -1; +error_setg(errp, TYPE_EMULATED_CCID ": failed to initialize vcard"); +return; } qemu_thread_create(&card->event_thread_id, "ccid/event", event_thread, card, QEMU_THREAD_JOINABLE); qemu_thread_create(&card->apdu_thread_id, "ccid/apdu", handle_apdu_thread, card, QEMU_THREAD_JOINABLE); -return 0; } -static void emulated_exitfn(CCI
Re: [Qemu-devel] [PATCH] usb-ccid: QOM'ify
Cc'ing Andreas Färber On 01/13/2018 11:35 PM, Philippe Mathieu-Daudé wrote: > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/usb/ccid.h | 9 +-- > hw/usb/ccid-card-emulated.c | 48 +++- > hw/usb/ccid-card-passthru.c | 13 +- > hw/usb/dev-smartcard-reader.c | 57 > +++ > 4 files changed, 60 insertions(+), 67 deletions(-) > > diff --git a/hw/usb/ccid.h b/hw/usb/ccid.h > index 1f070116d6..d9a1cbd09f 100644 > --- a/hw/usb/ccid.h > +++ b/hw/usb/ccid.h > @@ -28,20 +28,25 @@ typedef struct CCIDCardInfo CCIDCardInfo; > * into the smartcard device (hw/ccid-card-*.c) > */ > typedef struct CCIDCardClass { > +/*< private >*/ > DeviceClass parent_class; > +/*< public >*/ > +DeviceRealize parent_realize; > +DeviceUnrealize parent_unrealize; > + > const uint8_t *(*get_atr)(CCIDCardState *card, uint32_t *len); > void (*apdu_from_guest)(CCIDCardState *card, > const uint8_t *apdu, > uint32_t len); > -void (*exitfn)(CCIDCardState *card); > -int (*initfn)(CCIDCardState *card); > } CCIDCardClass; > > /* > * state of the CCID Card device (i.e. hw/ccid-card-*.c) > */ > struct CCIDCardState { > +/*< private >*/ > DeviceState qdev; > +/*< public >*/ > uint32_tslot; /* For future use with multiple slot reader. */ > }; > > diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c > index e646eb243b..db2ed53773 100644 > --- a/hw/usb/ccid-card-emulated.c > +++ b/hw/usb/ccid-card-emulated.c > @@ -27,6 +27,7 @@ > */ > > #include "qemu/osdep.h" > +#include "qapi/error.h" > #include > #include > #include > @@ -480,9 +481,9 @@ static uint32_t parse_enumeration(char *str, > return ret; > } > > -static int emulated_initfn(CCIDCardState *base) > +static void emulated_realize(DeviceState *dev, Error **errp) > { > -EmulatedState *card = EMULATED_CCID_CARD(base); > +EmulatedState *card = EMULATED_CCID_CARD(dev); > VCardEmulError ret; > const EnumTable *ptable; > > @@ -495,7 +496,8 @@ static int emulated_initfn(CCIDCardState *base) > card->reader = NULL; > card->quit_apdu_thread = 0; > if (init_event_notifier(card) < 0) { > -return -1; > +error_setg(errp, TYPE_EMULATED_CCID ": event notifier creation > failed"); > +return; > } > > card->backend = 0; > @@ -505,11 +507,12 @@ static int emulated_initfn(CCIDCardState *base) > } > > if (card->backend == 0) { > -printf("backend must be one of:\n"); > +error_setg(errp, TYPE_EMULATED_CCID ": no backend specified."); > +printf("backend must be one of:\n"); /* TODO remove */ > for (ptable = backend_enum_table; ptable->name != NULL; ++ptable) { > printf("%s\n", ptable->name); > } > -return -1; > +return; > } > > /* TODO: a passthru backened that works on local machine. third card > type?*/ > @@ -517,39 +520,38 @@ static int emulated_initfn(CCIDCardState *base) > if (card->cert1 != NULL && card->cert2 != NULL && card->cert3 != > NULL) { > ret = emulated_initialize_vcard_from_certificates(card); > } else { > -printf("%s: you must provide all three certs for" > - " certificates backend\n", TYPE_EMULATED_CCID); > -return -1; > +error_setg(errp, TYPE_EMULATED_CCID ": you must provide all > three " > + "certs for certificates backend"); > +return; > } > } else { > if (card->backend != BACKEND_NSS_EMULATED) { > -printf("%s: bad backend specified. The options are:\n%s > (default)," > -" %s.\n", TYPE_EMULATED_CCID, BACKEND_NSS_EMULATED_NAME, > -BACKEND_CERTIFICATES_NAME); > -return -1; > +error_setg(errp, TYPE_EMULATED_CCID ": bad backend specified. " > + "The options are: %s (default), %s.", > + BACKEND_NSS_EMULATED_NAME, BACKEND_CERTIFICATES_NAME); > +return; > } > if (card->cert1 != NULL || card->cert2 != NULL || card->cert3 != > NULL) { > -printf("%s: unexpected cert parameters to nss emulated > backend\n", > - TYPE_EMULATED_CCID); > -return -1; > +error_setg(errp, TYPE_EMULATED_CCID ": unexpected cert > parameters " > + "to nss emulated backend"); > +return; > } > /* default to mirroring the local hardware readers */ > ret = wrap_vcard_emul_init(NULL); > } > if (ret != VCARD_EMUL_OK) { > -printf("%s: failed to initialize vcard\n", TYPE_EMULATED_CCID); > -return -1; > +error_setg(errp, TYPE_EMULATED_CCID ": failed to initia
[Qemu-devel] [PATCH] hw/i2c: QOM'ify i2c slave
Signed-off-by: Philippe Mathieu-Daudé --- hw/i2c/core.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/i2c/core.c b/hw/i2c/core.c index 59068f157e..c84dbfb884 100644 --- a/hw/i2c/core.c +++ b/hw/i2c/core.c @@ -8,6 +8,7 @@ */ #include "qemu/osdep.h" +#include "qapi/error.h" #include "hw/i2c/i2c.h" typedef struct I2CNode I2CNode; @@ -276,16 +277,15 @@ const VMStateDescription vmstate_i2c_slave = { } }; -static int i2c_slave_qdev_init(DeviceState *dev) +static void i2c_slave_realize(DeviceState *dev, Error **errp) { I2CSlave *s = I2C_SLAVE(dev); I2CSlaveClass *sc = I2C_SLAVE_GET_CLASS(s); -if (sc->init) { -return sc->init(s); +if (sc->init && sc->init(s)) { +error_setg(errp, "i2c slave initialization failed"); +return; } - -return 0; } DeviceState *i2c_create_slave(I2CBus *bus, const char *name, uint8_t addr) @@ -301,7 +301,7 @@ DeviceState *i2c_create_slave(I2CBus *bus, const char *name, uint8_t addr) static void i2c_slave_class_init(ObjectClass *klass, void *data) { DeviceClass *k = DEVICE_CLASS(klass); -k->init = i2c_slave_qdev_init; +k->realize = i2c_slave_realize; set_bit(DEVICE_CATEGORY_MISC, k->categories); k->bus_type = TYPE_I2C_BUS; k->props = i2c_props; -- 2.15.1
[Qemu-devel] [PATCH] sysbus: convert init() to realize()
Signed-off-by: Philippe Mathieu-Daudé --- hw/core/sysbus.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c index 5d0887f499..0531eb60ce 100644 --- a/hw/core/sysbus.c +++ b/hw/core/sysbus.c @@ -18,6 +18,7 @@ */ #include "qemu/osdep.h" +#include "qapi/error.h" #include "hw/sysbus.h" #include "monitor/monitor.h" #include "exec/address-spaces.h" @@ -200,15 +201,15 @@ void sysbus_init_ioports(SysBusDevice *dev, uint32_t ioport, uint32_t size) } } -static int sysbus_device_init(DeviceState *dev) +static void sysbus_device_realize(DeviceState *dev, Error **errp) { SysBusDevice *sd = SYS_BUS_DEVICE(dev); SysBusDeviceClass *sbc = SYS_BUS_DEVICE_GET_CLASS(sd); -if (!sbc->init) { -return 0; +if (sbc->init && sbc->init(sd)) { +error_setg(errp, "sysbus device initialization failed"); +return; } -return sbc->init(sd); } DeviceState *sysbus_create_varargs(const char *name, @@ -324,7 +325,7 @@ MemoryRegion *sysbus_address_space(SysBusDevice *dev) static void sysbus_device_class_init(ObjectClass *klass, void *data) { DeviceClass *k = DEVICE_CLASS(klass); -k->init = sysbus_device_init; +k->realize = sysbus_device_realize; k->bus_type = TYPE_SYSTEM_BUS; /* * device_add plugs devices into a suitable bus. For "real" buses, -- 2.15.1
Re: [Qemu-devel] [PATCH] hw/i2c: QOM'ify i2c slave
This description is more appropriate: "hw/i2c: convert i2c slave init() to realize()" On 01/13/2018 11:45 PM, Philippe Mathieu-Daudé wrote: > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/i2c/core.c | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/hw/i2c/core.c b/hw/i2c/core.c > index 59068f157e..c84dbfb884 100644 > --- a/hw/i2c/core.c > +++ b/hw/i2c/core.c > @@ -8,6 +8,7 @@ > */ > > #include "qemu/osdep.h" > +#include "qapi/error.h" > #include "hw/i2c/i2c.h" > > typedef struct I2CNode I2CNode; > @@ -276,16 +277,15 @@ const VMStateDescription vmstate_i2c_slave = { > } > }; > > -static int i2c_slave_qdev_init(DeviceState *dev) > +static void i2c_slave_realize(DeviceState *dev, Error **errp) > { > I2CSlave *s = I2C_SLAVE(dev); > I2CSlaveClass *sc = I2C_SLAVE_GET_CLASS(s); > > -if (sc->init) { > -return sc->init(s); > +if (sc->init && sc->init(s)) { > +error_setg(errp, "i2c slave initialization failed"); > +return; > } > - > -return 0; > } > > DeviceState *i2c_create_slave(I2CBus *bus, const char *name, uint8_t addr) > @@ -301,7 +301,7 @@ DeviceState *i2c_create_slave(I2CBus *bus, const char > *name, uint8_t addr) > static void i2c_slave_class_init(ObjectClass *klass, void *data) > { > DeviceClass *k = DEVICE_CLASS(klass); > -k->init = i2c_slave_qdev_init; > +k->realize = i2c_slave_realize; > set_bit(DEVICE_CATEGORY_MISC, k->categories); > k->bus_type = TYPE_I2C_BUS; > k->props = i2c_props; >
Re: [Qemu-devel] vhost-pci and virtio-vhost-user
On Friday, January 12, 2018 6:38 PM, Stefan Hajnoczi wrote: > On Fri, Jan 12, 2018 at 02:44:00PM +0800, Wei Wang wrote: > > On 01/11/2018 05:56 PM, Stefan Hajnoczi wrote: > > > On Thu, Jan 11, 2018 at 6:31 AM, Wei Wang > wrote: > > > > On 01/11/2018 12:14 AM, Stefan Hajnoczi wrote: > > > I expect vhost-pci to require fewer code changes. If you judge "simpler" just > by the patch count or size, then vhost-pci will win. > > The reason for that is virtio-vhost-user integrates with librte_vhost. > This requires refactoring librte_vhost to support multiple transports. > I think the driver having more code is fine. The device part is what will go into the spec, and people have the freedom to implement their own driver (either the dpdk or kernel driver) in the future, if they don't want the librte_vhost based one. So, for the first version, I think it would be better to not make the device full-featured (e.g. pass through as little messages as possible), this would give users a chance to get a device and driver with the least lines of code. > I think the virtio-vhost-user end result is worth it though: vhost devices > like > examples/vhost/ and examples/vhost/scsi/ will work with both AF_UNIX and > virtio-vhost-user. This makes it simpler for users and vhost device > developers - you only have one implementation of net, scsi, blk, etc devices. For the driver part, isn't it that net, scsi, blk will have their own separate implementation on top of the common libret_vhost? Best, Wei