[Qemu-devel] [PATCH] Exit on reset for armv7-m
Implement the SYSRESETREQ bit of the AIRCR register for armv7-m (ie. cortex-m3). A small patch to see if I have the submission process figured out. Michael Davidsaver (1): armv7-m: exit on external reset request hw/intc/armv7m_nvic.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) -- 2.1.4
[Qemu-devel] [PATCH] armv7-m: exit on external reset request
Signed-off-by: Michael Davidsaver --- hw/intc/armv7m_nvic.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 3ec8408..a671d84 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -15,6 +15,7 @@ #include "hw/arm/arm.h" #include "exec/address-spaces.h" #include "gic_internal.h" +#include "sysemu/sysemu.h" typedef struct { GICState gic; @@ -348,10 +349,13 @@ static void nvic_writel(nvic_state *s, uint32_t offset, uint32_t value) break; case 0xd0c: /* Application Interrupt/Reset Control. */ if ((value >> 16) == 0x05fa) { +if (value & 4) { +qemu_system_reset_request(); +} if (value & 2) { qemu_log_mask(LOG_UNIMP, "VECTCLRACTIVE unimplemented\n"); } -if (value & 5) { +if (value & 1) { qemu_log_mask(LOG_UNIMP, "AIRCR system reset unimplemented\n"); } if (value & 0x700) { -- 2.1.4
Re: [Qemu-devel] [PATCH] Exit on reset for armv7-m
On 10/08/2015 04:09 PM, Peter Crosthwaite wrote: > On Thu, Oct 8, 2015 at 8:40 AM, Michael Davidsaver > wrote: >> Implement the SYSRESETREQ bit of the AIRCR register >> for armv7-m (ie. cortex-m3). >> > > This would serve better as the commit message to the patch (which I > notice is missing a commit blurb). Does this warrant sending an updated patch? I would have no objection to whomever applies the patch simply changing the commit message. >> A small patch to see if I have the submission process figured out. >> > > It is usual for the cover and patches to be enumerated together with > the cover being patch 0, e.g. the subject would read [PATCH 0/1]. the > --cover-letter switch of git format-patch does this for you. Alas in this case it did not. Probably because I omitted '--numbered'. >> Michael Davidsaver (1): > > When sending a single, a cover is not required. You could just send > the patch on it's own. This will all be useful when you come to send > multi-patch series however. Noted. Thanks for the feedback Peter. Michael
[Qemu-devel] [PATCH 1/3] armv7-m: exit on external reset request
Implement the SYSRESETREQ bit of the AIRCR register for armv7-m (ie. cortex-m3). --- hw/intc/armv7m_nvic.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 3ec8408..a671d84 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -15,6 +15,7 @@ #include "hw/arm/arm.h" #include "exec/address-spaces.h" #include "gic_internal.h" +#include "sysemu/sysemu.h" typedef struct { GICState gic; @@ -348,10 +349,13 @@ static void nvic_writel(nvic_state *s, uint32_t offset, uint32_t value) break; case 0xd0c: /* Application Interrupt/Reset Control. */ if ((value >> 16) == 0x05fa) { +if (value & 4) { +qemu_system_reset_request(); +} if (value & 2) { qemu_log_mask(LOG_UNIMP, "VECTCLRACTIVE unimplemented\n"); } -if (value & 5) { +if (value & 1) { qemu_log_mask(LOG_UNIMP, "AIRCR system reset unimplemented\n"); } if (value & 0x700) { -- 2.1.4
[Qemu-devel] [PATCH 0/3] armv7-m exit, exception fixes and add MPU
Please be aware that this work is based on my reading of ARM documentation, and hasn't yet been cross-checked with real hardware. Patch #1 is (I think) fairly straightforward. I'd like to get some comments on patches #2 and #3, which I don't consider ready for merge. The first patch was previously sent on the thread "armv7-m: exit on external reset request", and included here with only a changed commit message. I'm trying to add support for the MPU (PMSAv7) for cortex-m3 and cortex-m4. In the process I found an issue with handling of exceptions other than IRQ. In arm_v7m_cpu_do_interrupt() 'v7m.exception' is not being updated for UsageFault, SVC, and MemManage cases. In the UsageFault case, the handler isn't invoked, and an invalid instruction is perpetually re-executed, resulting in a stuck state. Patch #2 relaxes handling of mis-aligned handler functions (ie. missing '.thumb_func') to mirror what is already done on exception return. While a guest_errors message will be helpful for people (like myself) who make this mistake, I'm not sure if automatically correcting the error is appropriate. I'm having some difficulty in getting my test code loaded on a real cortex-m4 for a cross-check. It may be some time before I succeed (might have to get a different board). If anyone is interested in trying to do this test, please let me know as I'm happy to assist. Michael Davidsaver (3): armv7-m: exit on external reset request armv7-m: fix non-IRQ exceptions armv7-m: add MPU to cortex-m3 and cortex-m4 hw/arm/armv7m.c | 8 --- hw/intc/armv7m_nvic.c | 160 -- target-arm/cpu-qom.h | 4 ++ target-arm/cpu.c | 14 + target-arm/helper.c | 34 +-- 5 files changed, 200 insertions(+), 20 deletions(-) -- 2.1.4
Re: [Qemu-devel] [PATCH] armv7-m: exit on external reset request
On 10/09/2015 12:59 PM, Peter Maydell wrote: > On 8 October 2015 at 16:40, Michael Davidsaver wrote: >> ... >> case 0xd0c: /* Application Interrupt/Reset Control. */ >> if ((value >> 16) == 0x05fa) { >> +if (value & 4) { >> +qemu_system_reset_request(); >> +} ... > > Strictly speaking what this bit does is assert a signal out of > the CPU to some external power management or similar device > in the SoC, which then is responsible for doing the reset. > So just calling qemu_system_reset_request() here is a bit of > a shortcut. But maybe it's a pragmatic shortcut? I'm not sure what you mean by shortcut? Most targets have some way to trigger qemu to exit. I actually compiled a list recently (see below). I couldn't find one for the lm3s6965evb machine, thus this patch. FYI, one of my interests in QEMU is to automate testing of low level code. Being able to cause the qemu process to exit when the tests complete is quite helpful (simplifies the test runner) qemu_system_reset_request() highbank.c -> register write integratorcp.c -> register write musicpal.c -> register write (musicpal board) omap1.c -> register write omap2.c -> register write pc.c -> register write (port 92) pckbd.c -> register write (two different registers) lpc_ich9.c -> register write arm_sysctl.c -> register write (vexpress only or board_ids 0x100, 0x178, 0x182 only, ) cuda.c -> register write slavio_misc.c -> register write zynq_slcr.c -> register write (xilinx-zynq-a9 board) apb.c -> register write bonito.c -> register write piix.c -> register write mpc8544_guts.c -> register write ppc.c -> e500 only ppce500_irq_init() w/ PPCE500_INPUT_MCK ppc405_uc.c -> store_40x_dbcr0() spapr_hcall.c -> KVM only spapr_rtas.c -> register RTAS_SYSTEM_REBOOT etraxfs_timer.c -> watchdog timer expire m48t59.c -> watchdog timer expire milkymist-sysctl.c -> register write pxa2xx_timer.c -> watchdog timer expire (bsp option) watchdog.c -> generic watchdog timer expire xtfpga.c -> register write cpu_exit() pc.c -> DMA_init prep.c -> DMA_init (broken) spapr_rtas.c -> register RTAS_STOP_SELF
Re: [Qemu-devel] [PATCH] armv7-m: exit on external reset request
On 10/09/2015 02:18 PM, Peter Crosthwaite wrote: > On Fri, Oct 9, 2015 at 10:25 AM, Michael Davidsaver > wrote: >> >> >> On 10/09/2015 12:59 PM, Peter Maydell wrote: >>> On 8 October 2015 at 16:40, Michael Davidsaver >>> wrote: >>>> ... >>>> case 0xd0c: /* Application Interrupt/Reset Control. */ >>>> if ((value >> 16) == 0x05fa) { >>>> +if (value & 4) { >>>> +qemu_system_reset_request(); >>>> +} >> ... >>> >>> Strictly speaking what this bit does is assert a signal out of >>> the CPU to some external power management or similar device >>> in the SoC, which then is responsible for doing the reset. >>> So just calling qemu_system_reset_request() here is a bit of >>> a shortcut. But maybe it's a pragmatic shortcut? >> >> I'm not sure what you mean by shortcut? Most targets have some way to >> trigger qemu to exit. I actually compiled a list recently (see below). I >> couldn't find one for the lm3s6965evb machine, thus this patch. >> > ... > I think it would be better for SoC (or board) level to > install the GPIO handler to do the reset. As far as target-arm is > concerned this is then just a GPIO. I don't think I'm well versed enough in qemu lingo to parse this sentence :) Are you concerned about adding this function to AIRCR (which would effect every armv7-m board), or about directly calling qemu_system_reset_request() in armv7m_nvic.c? > We should check the lm3s docs to see what the implementation of this > signal is. To quote page 129 of http://www.ti.com/lit/ds/symlink/lm3s6965.pdf > System Reset Request > Value Description > 0 > No effect. > 1 > Resets the core and all on-chip peripherals except the Debug > interface. > This bit is automatically cleared during the reset of the core and reads > as 0. There is also mention of a board specific watchdog timer. I also looked at the TI MSP432P4xx (I have one), which give a similar definition for this bit (along with a dozen board specific ways to do different reset levels...)
[Qemu-devel] [PATCH 2/3] armv7-m: fix non-IRQ exceptions
Handlers will not be entered unless v7m.exception is updated. For example, an invalid instruction won't invoke UsageError, but rather re-executes the invalid instruction forever. Add warn and fix of mis-aligned handlers. Ensure exception return "addresses" always fault, and trap them just before the EXCP_DATA_ABORT handler would be invoked and execute return instead of MemManage. This removes the need for the "armv7m.hack" MemoryRegion. --- hw/arm/armv7m.c | 8 target-arm/helper.c | 27 +-- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c index eb214db..0fc95de 100644 --- a/hw/arm/armv7m.c +++ b/hw/arm/armv7m.c @@ -178,7 +178,6 @@ qemu_irq *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq, uint64_t lowaddr; int i; int big_endian; -MemoryRegion *hack = g_new(MemoryRegion, 1); if (cpu_model == NULL) { cpu_model = "cortex-m3"; @@ -226,13 +225,6 @@ qemu_irq *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq, } } -/* Hack to map an additional page of ram at the top of the address - space. This stops qemu complaining about executing code outside RAM - when returning from an exception. */ -memory_region_init_ram(hack, NULL, "armv7m.hack", 0x1000, &error_fatal); -vmstate_register_ram_global(hack); -memory_region_add_subregion(system_memory, 0xf000, hack); - qemu_register_reset(armv7m_reset, cpu); return pic; } diff --git a/target-arm/helper.c b/target-arm/helper.c index 8367997..56b238f 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -5346,18 +5346,23 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) switch (cs->exception_index) { case EXCP_UDEF: armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE); -return; +env->v7m.exception = ARMV7M_EXCP_USAGE; +break; case EXCP_SWI: /* The PC already points to the next instruction. */ armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_SVC); -return; +env->v7m.exception = ARMV7M_EXCP_SVC; +break; case EXCP_PREFETCH_ABORT: case EXCP_DATA_ABORT: -/* TODO: if we implemented the MPU registers, this is where we - * should set the MMFAR, etc from exception.fsr and exception.vaddress. - */ +if(env->v7m.exception!=0 && env->exception.vaddress>=0xfff0) { +/* this isn't a real fault, but rather a result of return from interrupt */ +do_v7m_exception_exit(env); +return; +} armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_MEM); -return; +env->v7m.exception = ARMV7M_EXCP_MEM; +break; case EXCP_BKPT: if (semihosting_enabled()) { int nr; @@ -5407,6 +5412,12 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) addr = ldl_phys(cs->as, env->v7m.vecbase + env->v7m.exception * 4); env->regs[15] = addr & 0xfffe; env->thumb = addr & 1; +if(!env->thumb) { +qemu_log_mask(LOG_GUEST_ERROR, + "M profile interrupt handler with misaligned " + "PC is UNPREDICTABLE\n"); +env->thumb = 1; +} } /* Function used to synchronize QEMU's AArch64 register set with AArch32 @@ -6682,6 +6693,10 @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, *phys_ptr = address; *prot = 0; +/* ensure exception returns take precidence */ +if(env->v7m.exception!=0 && env->exception.vaddress>=0xfff0) +return true; + if (regime_translation_disabled(env, mmu_idx)) { /* MPU disabled */ get_phys_addr_pmsav7_default(env, mmu_idx, address, prot); } else { /* MPU enabled */ -- 2.1.4
[Qemu-devel] [PATCH 3/3] armv7-m: add MPU to cortex-m3 and cortex-m4
The M series MPU is almost the same as the already implemented R series MPU. So use the M series and translate as best we can. The HFNMIENA bit in MPU_CTRL is not implemented. Implement CFSR and MMFAR to report fault address to MemManage handler. Add MPU feature flag to cortex-m3 and -m4. --- hw/intc/armv7m_nvic.c | 154 -- target-arm/cpu-qom.h | 4 ++ target-arm/cpu.c | 14 + target-arm/helper.c | 7 +++ 4 files changed, 174 insertions(+), 5 deletions(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index a671d84..94011cf 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -245,12 +245,11 @@ static uint32_t nvic_readl(nvic_state *s, uint32_t offset) if (s->gic.irq_state[ARMV7M_EXCP_USAGE].enabled) val |= (1 << 18); return val; case 0xd28: /* Configurable Fault Status. */ -/* TODO: Implement Fault Status. */ -qemu_log_mask(LOG_UNIMP, "Configurable Fault Status unimplemented\n"); -return 0; +return ARM_CPU(current_cpu)->pmsav7_cfsr; +case 0xd34: /* MMFAR MemManage Fault Address */ +return ARM_CPU(current_cpu)->pmsav7_mmfar; case 0xd2c: /* Hard Fault Status. */ case 0xd30: /* Debug Fault Status. */ -case 0xd34: /* Mem Manage Address. */ case 0xd38: /* Bus Fault Address. */ case 0xd3c: /* Aux Fault Status. */ /* TODO: Implement fault status registers. */ @@ -283,6 +282,55 @@ static uint32_t nvic_readl(nvic_state *s, uint32_t offset) case 0xd70: /* ISAR4. */ return 0x01310102; /* TODO: Implement debug registers. */ +case 0xd90: /* MPU_TYPE */ +cpu = ARM_CPU(current_cpu); +return cpu->has_mpu ? (cpu->pmsav7_dregion<<8) : 0; +break; +case 0xd94: /* MPU_CTRL */ +val = 0; +cpu = ARM_CPU(current_cpu); +if(cpu->env.cp15.sctlr_el[0] & SCTLR_M) +val |= 1; /* ENABLE */ +/* HFNMIENA not implemented, see nvic_writel() */ +if(cpu->env.cp15.sctlr_el[0] & SCTLR_BR) +val |= 4; /* PRIVDEFENA */ +return val; +case 0xd98: /* MPU_RNR */ +return ARM_CPU(current_cpu)->env.cp15.c6_rgnr; +case 0xd9c: /* MPU_RBAR */ +case 0xda4: /* MPU_RBAR_A1 */ +case 0xdaC: /* MPU_RBAR_A2 */ +case 0xdb4: /* MPU_RBAR_A3 */ +{ +uint32_t range; +cpu = ARM_CPU(current_cpu); +if(offset==0xd9c) +range = cpu->env.cp15.c6_rgnr; +else +range = (offset-0xda4)/8; + +if(range>=cpu->pmsav7_dregion) return 0; + +return (cpu->env.pmsav7.drbar[range]&(0x1f)) | (range&0xf); +} +case 0xda0: /* MPU_RASR */ +case 0xda8: /* MPU_RASR_A1 */ +case 0xdb0: /* MPU_RASR_A2 */ +case 0xdb8: /* MPU_RASR_A3 */ +{ +uint32_t range; +cpu = ARM_CPU(current_cpu); + +if(offset==0xda0) +range = cpu->env.cp15.c6_rgnr; +else +range = (offset-0xda8)/8; + +if(range>=cpu->pmsav7_dregion) return 0; + +return ((cpu->env.pmsav7.dracr[range]&0x)<<16) +| (cpu->env.pmsav7.drsr[range]&0x); +} default: qemu_log_mask(LOG_GUEST_ERROR, "NVIC: Bad read offset 0x%x\n", offset); return 0; @@ -376,14 +424,110 @@ static void nvic_writel(nvic_state *s, uint32_t offset, uint32_t value) s->gic.irq_state[ARMV7M_EXCP_USAGE].enabled = (value & (1 << 18)) != 0; break; case 0xd28: /* Configurable Fault Status. */ +case 0xd34: /* Mem Manage Address. */ +return; case 0xd2c: /* Hard Fault Status. */ case 0xd30: /* Debug Fault Status. */ -case 0xd34: /* Mem Manage Address. */ case 0xd38: /* Bus Fault Address. */ case 0xd3c: /* Aux Fault Status. */ qemu_log_mask(LOG_UNIMP, "NVIC: fault status registers unimplemented\n"); break; +case 0xd90: /* MPU_TYPE (0xe000ed90) */ +return; /* RO */ +case 0xd94: /* MPU_CTRL */ +{ +unsigned q; +uint32_t tset=0, tclear=0; +if(value&1) { +tset |= SCTLR_M; +} else { +tclear |= SCTLR_M; +} +if(value&2) { +/* The M series MPU is almost functionally the same + * as the R series MPU, so we translate to the R series + * register format. + * + * One difference is that the R series MPU doesn't implement + * HFNMIENA (bypass MPU in some exception handlers). + */ +qemu_log_mask(LOG_UNIMP, "M profile does not implement HFNMIENA bit in MPU_CTRL\n"); +} +if(value&4) { +tset |= SCTLR_BR; +} else { +tclear |= SCTLR_BR; +} +cpu = ARM_CPU(current_cpu); +/* TODO, which sctlr(s) really need to be set? */ +for(q=0; q<4; q++) { +cpu->env.cp15.sctlr_el[q]
Re: [Qemu-devel] [PATCH] armv7-m: exit on external reset request
On 10/09/2015 02:51 PM, Michael Davidsaver wrote: > On 10/09/2015 02:18 PM, Peter Crosthwaite wrote: >> On Fri, Oct 9, 2015 at 10:25 AM, Michael Davidsaver >> wrote: >>> >>> >>> On 10/09/2015 12:59 PM, Peter Maydell wrote: >>>> On 8 October 2015 at 16:40, Michael Davidsaver >>>> wrote: >>>>> ... >>>>> case 0xd0c: /* Application Interrupt/Reset Control. */ >>>>> if ((value >> 16) == 0x05fa) { >>>>> +if (value & 4) { >>>>> +qemu_system_reset_request(); >>>>> +} >>> ... >>>> >>>> Strictly speaking what this bit does is assert a signal out of >>>> the CPU to some external power management or similar device >>>> in the SoC, which then is responsible for doing the reset. >>>> So just calling qemu_system_reset_request() here is a bit of >>>> a shortcut. But maybe it's a pragmatic shortcut? >>> >>> I'm not sure what you mean by shortcut? Most targets have some way to >>> trigger qemu to exit. I actually compiled a list recently (see below). I >>> couldn't find one for the lm3s6965evb machine, thus this patch. >>> >> > ... >> I think it would be better for SoC (or board) level to >> install the GPIO handler to do the reset. As far as target-arm is >> concerned this is then just a GPIO. > > I don't think I'm well versed enough in qemu lingo to parse this sentence :) > Are you concerned about adding this function to AIRCR (which would effect > every armv7-m board), > or about directly calling qemu_system_reset_request() in armv7m_nvic.c? I think I've answered my own question. You mean to replace the call to qemu_system_reset_request() with something like qemu_irq_pulse()? I think I see how to do this. The simplest (adding another named GPIO to the NVIC devices) is complicated by the fact that the armv7m_init() doesn't return the nvic object, but rather an array of qemu_irq. Is it reasonable to change armv7m_init() to return DeviceState*?
[Qemu-devel] [PATCH v2] armv7-m: exit on external reset request
Implement the SYSRESETREQ bit of the AIRCR register for armv7-m (ie. cortex-m3) to trigger a GPIO out. Change armv7m_init to return the DeviceState* for the NVIC. This allows access to all GPIO blocks, not just the IRQ inputs. Move qdev_get_gpio_in() calls out of armv7m_init() into board code for stellaris and stm32f205 boards. Add GPIO in for the stellaris board which calls qemu_system_reset_request() on reset request. --- hw/arm/armv7m.c| 9 ++--- hw/arm/stellaris.c | 36 +--- hw/arm/stm32f205_soc.c | 13 ++--- hw/intc/armv7m_nvic.c | 7 ++- include/hw/arm/arm.h | 2 +- 5 files changed, 40 insertions(+), 27 deletions(-) diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c index eb214db..a80d2ad 100644 --- a/hw/arm/armv7m.c +++ b/hw/arm/armv7m.c @@ -166,17 +166,15 @@ static void armv7m_reset(void *opaque) mem_size is in bytes. Returns the NVIC array. */ -qemu_irq *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq, +DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq, const char *kernel_filename, const char *cpu_model) { ARMCPU *cpu; CPUARMState *env; DeviceState *nvic; -qemu_irq *pic = g_new(qemu_irq, num_irq); int image_size; uint64_t entry; uint64_t lowaddr; -int i; int big_endian; MemoryRegion *hack = g_new(MemoryRegion, 1); @@ -198,9 +196,6 @@ qemu_irq *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq, qdev_init_nofail(nvic); sysbus_connect_irq(SYS_BUS_DEVICE(nvic), 0, qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ)); -for (i = 0; i < num_irq; i++) { -pic[i] = qdev_get_gpio_in(nvic, i); -} #ifdef TARGET_WORDS_BIGENDIAN big_endian = 1; @@ -234,7 +229,7 @@ qemu_irq *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq, memory_region_add_subregion(system_memory, 0xf000, hack); qemu_register_reset(armv7m_reset, cpu); -return pic; +return nvic; } static Property bitband_properties[] = { diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c index 3d6486f..e3b19f3 100644 --- a/hw/arm/stellaris.c +++ b/hw/arm/stellaris.c @@ -16,6 +16,7 @@ #include "net/net.h" #include "hw/boards.h" #include "exec/address-spaces.h" +#include "sysemu/sysemu.h" #define GPIO_A 0 #define GPIO_B 1 @@ -1176,6 +1177,13 @@ static int stellaris_adc_init(SysBusDevice *sbd) return 0; } +static +void do_sys_reset(void *opaque, int n, int level) +{ +if(level) +qemu_system_reset_request(); +} + /* Board init. */ static stellaris_board_info stellaris_boards[] = { { "LM3S811EVB", @@ -1210,8 +1218,7 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model, 0x40024000, 0x40025000, 0x40026000}; static const int gpio_irq[7] = {0, 1, 2, 3, 4, 30, 31}; -qemu_irq *pic; -DeviceState *gpio_dev[7]; +DeviceState *gpio_dev[7], *nvic; qemu_irq gpio_in[7][8]; qemu_irq gpio_out[7][8]; qemu_irq adc; @@ -1241,12 +1248,19 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model, vmstate_register_ram_global(sram); memory_region_add_subregion(system_memory, 0x2000, sram); -pic = armv7m_init(system_memory, flash_size, NUM_IRQ_LINES, +nvic = armv7m_init(system_memory, flash_size, NUM_IRQ_LINES, kernel_filename, cpu_model); +qdev_connect_gpio_out_named(nvic, "SYSRESETREQ", 0, +qemu_allocate_irq(&do_sys_reset, NULL, 0)); + if (board->dc1 & (1 << 16)) { dev = sysbus_create_varargs(TYPE_STELLARIS_ADC, 0x40038000, -pic[14], pic[15], pic[16], pic[17], NULL); +qdev_get_gpio_in(nvic, 14), +qdev_get_gpio_in(nvic, 15), +qdev_get_gpio_in(nvic, 16), +qdev_get_gpio_in(nvic, 17), +NULL); adc = qdev_get_gpio_in(dev, 0); } else { adc = NULL; @@ -1255,19 +1269,19 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model, if (board->dc2 & (0x1 << i)) { dev = sysbus_create_simple(TYPE_STELLARIS_GPTM, 0x4003 + i * 0x1000, - pic[timer_irq[i]]); + qdev_get_gpio_in(nvic, timer_irq[i])); /* TODO: This is incorrect, but we get away with it because the ADC output is only ever pulsed. */ qdev_connect_gpio_out(dev, 0, adc); } } -stellaris_sys_init(0x400fe000, pic[28], board, nd_table[0].macaddr.a); +stellaris_sys_init(0x400fe000, qdev_get_gpio_in(nvic, 28), board, nd_table[0].macaddr.a); for (
Re: [Qemu-devel] [PATCH 2/3] armv7-m: fix non-IRQ exceptions
I'm starting to doubt my diagnosis. The bug may be in my understanding of the interrupt priorities. I'll have to do another test program. On Oct 11, 2015 11:25 AM, "Peter Crosthwaite" wrote: > On Fri, Oct 9, 2015 at 6:28 AM, Michael Davidsaver > wrote: > > Handlers will not be entered unless v7m.exception is updated. > > For example, an invalid instruction won't invoke UsageError, > > but rather re-executes the invalid instruction forever. > > > > Add warn and fix of mis-aligned handlers. > > > > Ensure exception return "addresses" always fault, > > and trap them just before the EXCP_DATA_ABORT > > handler would be invoked and execute return instead > > of MemManage. > > This removes the need for the "armv7m.hack" MemoryRegion. > > --- > > hw/arm/armv7m.c | 8 > > target-arm/helper.c | 27 +-- > > 2 files changed, 21 insertions(+), 14 deletions(-) > > > > diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c > > index eb214db..0fc95de 100644 > > --- a/hw/arm/armv7m.c > > +++ b/hw/arm/armv7m.c > > @@ -178,7 +178,6 @@ qemu_irq *armv7m_init(MemoryRegion *system_memory, > int mem_size, int num_irq, > > uint64_t lowaddr; > > int i; > > int big_endian; > > -MemoryRegion *hack = g_new(MemoryRegion, 1); > > > > if (cpu_model == NULL) { > > cpu_model = "cortex-m3"; > > @@ -226,13 +225,6 @@ qemu_irq *armv7m_init(MemoryRegion *system_memory, > int mem_size, int num_irq, > > } > > } > > > > -/* Hack to map an additional page of ram at the top of the address > > - space. This stops qemu complaining about executing code outside > RAM > > - when returning from an exception. */ > > -memory_region_init_ram(hack, NULL, "armv7m.hack", 0x1000, > &error_fatal); > > -vmstate_register_ram_global(hack); > > -memory_region_add_subregion(system_memory, 0xf000, hack); > > - > > CC PMM, Alistair and Marcin. They were discussing this recently. > > Regards, > Peter > > > qemu_register_reset(armv7m_reset, cpu); > > return pic; > > } > > diff --git a/target-arm/helper.c b/target-arm/helper.c > > index 8367997..56b238f 100644 > > --- a/target-arm/helper.c > > +++ b/target-arm/helper.c > > @@ -5346,18 +5346,23 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) > > switch (cs->exception_index) { > > case EXCP_UDEF: > > armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE); > > -return; > > +env->v7m.exception = ARMV7M_EXCP_USAGE; > > +break; > > case EXCP_SWI: > > /* The PC already points to the next instruction. */ > > armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_SVC); > > -return; > > +env->v7m.exception = ARMV7M_EXCP_SVC; > > +break; > > case EXCP_PREFETCH_ABORT: > > case EXCP_DATA_ABORT: > > -/* TODO: if we implemented the MPU registers, this is where we > > - * should set the MMFAR, etc from exception.fsr and > exception.vaddress. > > - */ > > +if(env->v7m.exception!=0 && > env->exception.vaddress>=0xfff0) { > > +/* this isn't a real fault, but rather a result of return > from interrupt */ > > +do_v7m_exception_exit(env); > > +return; > > +} > > armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_MEM); > > -return; > > +env->v7m.exception = ARMV7M_EXCP_MEM; > > +break; > > case EXCP_BKPT: > > if (semihosting_enabled()) { > > int nr; > > @@ -5407,6 +5412,12 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) > > addr = ldl_phys(cs->as, env->v7m.vecbase + env->v7m.exception * 4); > > env->regs[15] = addr & 0xfffe; > > env->thumb = addr & 1; > > +if(!env->thumb) { > > +qemu_log_mask(LOG_GUEST_ERROR, > > + "M profile interrupt handler with misaligned " > > + "PC is UNPREDICTABLE\n"); > > +env->thumb = 1; > > +} > > } > > > > /* Function used to synchronize QEMU's AArch64 register set with AArch32 > > @@ -6682,6 +6693,10 @@ static bool get_phys_addr_pmsav7(CPUARMState > *env, uint32_t address, > > *phys_ptr = address; > > *prot = 0; > > > > +/* ensure exception returns take precidence */ > > +if(env->v7m.exception!=0 && env->exception.vaddress>=0xfff0) > > +return true; > > + > > if (regime_translation_disabled(env, mmu_idx)) { /* MPU disabled */ > > get_phys_addr_pmsav7_default(env, mmu_idx, address, prot); > > } else { /* MPU enabled */ > > -- > > 2.1.4 > > >
[Qemu-devel] [PATCH v3 2/3] armv7-m: Implement SYSRESETREQ
Implement the SYSRESETREQ bit of the AIRCR register for armv7-m (ie. cortex-m3) to trigger a GPIO out. Signed-off-by: Michael Davidsaver --- hw/intc/armv7m_nvic.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 3ec8408..6fc167e 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -28,6 +28,7 @@ typedef struct { MemoryRegion gic_iomem_alias; MemoryRegion container; uint32_t num_irq; +qemu_irq sysresetreq; } nvic_state; #define TYPE_NVIC "armv7m_nvic" @@ -348,10 +349,13 @@ static void nvic_writel(nvic_state *s, uint32_t offset, uint32_t value) break; case 0xd0c: /* Application Interrupt/Reset Control. */ if ((value >> 16) == 0x05fa) { +if (value & 4) { +qemu_irq_pulse(s->sysresetreq); +} if (value & 2) { qemu_log_mask(LOG_UNIMP, "VECTCLRACTIVE unimplemented\n"); } -if (value & 5) { +if (value & 1) { qemu_log_mask(LOG_UNIMP, "AIRCR system reset unimplemented\n"); } if (value & 0x700) { @@ -535,11 +539,14 @@ static void armv7m_nvic_instance_init(Object *obj) * value in the GICState struct. */ GICState *s = ARM_GIC_COMMON(obj); +DeviceState *dev = DEVICE(obj); +nvic_state *nvic = NVIC(obj); /* The ARM v7m may have anything from 0 to 496 external interrupt * IRQ lines. We default to 64. Other boards may differ and should * set the num-irq property appropriately. */ s->num_irq = 64; +qdev_init_gpio_out_named(dev, &nvic->sysresetreq, "SYSRESETREQ", 1); } static void armv7m_nvic_class_init(ObjectClass *klass, void *data) -- 2.1.4
[Qemu-devel] [PATCH v3 3/3] stellaris: exit on external reset request
Add GPIO in for the stellaris board which calls qemu_system_reset_request() on reset request. Signed-off-by: Michael Davidsaver --- hw/arm/stellaris.c | 12 1 file changed, 12 insertions(+) diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c index 82a4ad5..0114e0a 100644 --- a/hw/arm/stellaris.c +++ b/hw/arm/stellaris.c @@ -16,6 +16,7 @@ #include "net/net.h" #include "hw/boards.h" #include "exec/address-spaces.h" +#include "sysemu/sysemu.h" #define GPIO_A 0 #define GPIO_B 1 @@ -1176,6 +1177,14 @@ static int stellaris_adc_init(SysBusDevice *sbd) return 0; } +static +void do_sys_reset(void *opaque, int n, int level) +{ +if (level) { +qemu_system_reset_request(); +} +} + /* Board init. */ static stellaris_board_info stellaris_boards[] = { { "LM3S811EVB", @@ -1243,6 +1252,9 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model, nvic = armv7m_init(system_memory, flash_size, NUM_IRQ_LINES, kernel_filename, cpu_model); +qdev_connect_gpio_out_named(nvic, "SYSRESETREQ", 0, +qemu_allocate_irq(&do_sys_reset, NULL, 0)); + if (board->dc1 & (1 << 16)) { dev = sysbus_create_varargs(TYPE_STELLARIS_ADC, 0x40038000, qdev_get_gpio_in(nvic, 14), -- 2.1.4
[Qemu-devel] [PATCH v3 1/3] armv7-m: Return DeviceState* from armv7m_init()
Change armv7m_init to return the DeviceState* for the NVIC. This allows access to all GPIO blocks, not just the IRQ inputs. Move qdev_get_gpio_in() calls out of armv7m_init() into board code for stellaris and stm32f205 boards. Signed-off-by: Michael Davidsaver --- hw/arm/armv7m.c| 9 ++--- hw/arm/stellaris.c | 29 ++--- hw/arm/stm32f205_soc.c | 15 --- include/hw/arm/arm.h | 2 +- 4 files changed, 29 insertions(+), 26 deletions(-) diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c index eb214db..a80d2ad 100644 --- a/hw/arm/armv7m.c +++ b/hw/arm/armv7m.c @@ -166,17 +166,15 @@ static void armv7m_reset(void *opaque) mem_size is in bytes. Returns the NVIC array. */ -qemu_irq *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq, +DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq, const char *kernel_filename, const char *cpu_model) { ARMCPU *cpu; CPUARMState *env; DeviceState *nvic; -qemu_irq *pic = g_new(qemu_irq, num_irq); int image_size; uint64_t entry; uint64_t lowaddr; -int i; int big_endian; MemoryRegion *hack = g_new(MemoryRegion, 1); @@ -198,9 +196,6 @@ qemu_irq *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq, qdev_init_nofail(nvic); sysbus_connect_irq(SYS_BUS_DEVICE(nvic), 0, qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ)); -for (i = 0; i < num_irq; i++) { -pic[i] = qdev_get_gpio_in(nvic, i); -} #ifdef TARGET_WORDS_BIGENDIAN big_endian = 1; @@ -234,7 +229,7 @@ qemu_irq *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq, memory_region_add_subregion(system_memory, 0xf000, hack); qemu_register_reset(armv7m_reset, cpu); -return pic; +return nvic; } static Property bitband_properties[] = { diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c index 3d6486f..82a4ad5 100644 --- a/hw/arm/stellaris.c +++ b/hw/arm/stellaris.c @@ -1210,8 +1210,7 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model, 0x40024000, 0x40025000, 0x40026000}; static const int gpio_irq[7] = {0, 1, 2, 3, 4, 30, 31}; -qemu_irq *pic; -DeviceState *gpio_dev[7]; +DeviceState *gpio_dev[7], *nvic; qemu_irq gpio_in[7][8]; qemu_irq gpio_out[7][8]; qemu_irq adc; @@ -1241,12 +1240,16 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model, vmstate_register_ram_global(sram); memory_region_add_subregion(system_memory, 0x2000, sram); -pic = armv7m_init(system_memory, flash_size, NUM_IRQ_LINES, +nvic = armv7m_init(system_memory, flash_size, NUM_IRQ_LINES, kernel_filename, cpu_model); if (board->dc1 & (1 << 16)) { dev = sysbus_create_varargs(TYPE_STELLARIS_ADC, 0x40038000, -pic[14], pic[15], pic[16], pic[17], NULL); +qdev_get_gpio_in(nvic, 14), +qdev_get_gpio_in(nvic, 15), +qdev_get_gpio_in(nvic, 16), +qdev_get_gpio_in(nvic, 17), +NULL); adc = qdev_get_gpio_in(dev, 0); } else { adc = NULL; @@ -1255,19 +1258,21 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model, if (board->dc2 & (0x1 << i)) { dev = sysbus_create_simple(TYPE_STELLARIS_GPTM, 0x4003 + i * 0x1000, - pic[timer_irq[i]]); + qdev_get_gpio_in(nvic, timer_irq[i])); /* TODO: This is incorrect, but we get away with it because the ADC output is only ever pulsed. */ qdev_connect_gpio_out(dev, 0, adc); } } -stellaris_sys_init(0x400fe000, pic[28], board, nd_table[0].macaddr.a); +stellaris_sys_init(0x400fe000, qdev_get_gpio_in(nvic, 28), + board, nd_table[0].macaddr.a); for (i = 0; i < 7; i++) { if (board->dc4 & (1 << i)) { gpio_dev[i] = sysbus_create_simple("pl061_luminary", gpio_addr[i], - pic[gpio_irq[i]]); + qdev_get_gpio_in(nvic, +gpio_irq[i])); for (j = 0; j < 8; j++) { gpio_in[i][j] = qdev_get_gpio_in(gpio_dev[i], j); gpio_out[i][j] = NULL; @@ -1276,7 +1281,8 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model, } if (board->dc2 & (1 << 12)) { -dev = sysbus_create_simple(TYPE_STELLARIS_I2C, 0x40
Re: [Qemu-devel] [PATCH 3/3] armv7-m: add MPU to cortex-m3 and cortex-m4
On 10/11/2015 11:23 AM, Peter Crosthwaite wrote: > On Fri, Oct 9, 2015 at 6:28 AM, Michael Davidsaver > wrote: >> The M series MPU is almost the same as the already >> implemented R series MPU. So use the M series >> and translate as best we can. >> > There is some work on list for this that never got a respin: > > https://lists.gnu.org/archive/html/qemu-devel/2015-07/msg01945.html Well, I totally missed that. I'll have look. > ... >> +case 0xd34: /* MMFAR MemManage Fault Address */ >> +return ARM_CPU(current_cpu)->pmsav7_mmfar; > Why reorder the addresses in the switch? I was thinking to avoid duplicating the qemu_log_mask() for the unimplemented registers. I take it that this to you is not the lesser evil :) > ... If you run your patch through scripts/checkpatch.pl it will detect > some of these conventions. Will do. > ... More style nits here All noted. > Regards, Peter Michael
[Qemu-devel] [PATCH v4 1/3] armv7-m: Return DeviceState* from armv7m_init()
Change armv7m_init to return the DeviceState* for the NVIC. This allows access to all GPIO blocks, not just the IRQ inputs. Move qdev_get_gpio_in() calls out of armv7m_init() into board code for stellaris and stm32f205 boards. --- hw/arm/armv7m.c| 9 ++--- hw/arm/stellaris.c | 29 ++--- hw/arm/stm32f205_soc.c | 15 --- include/hw/arm/arm.h | 2 +- 4 files changed, 29 insertions(+), 26 deletions(-) diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c index eb214db..a80d2ad 100644 --- a/hw/arm/armv7m.c +++ b/hw/arm/armv7m.c @@ -166,17 +166,15 @@ static void armv7m_reset(void *opaque) mem_size is in bytes. Returns the NVIC array. */ -qemu_irq *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq, +DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq, const char *kernel_filename, const char *cpu_model) { ARMCPU *cpu; CPUARMState *env; DeviceState *nvic; -qemu_irq *pic = g_new(qemu_irq, num_irq); int image_size; uint64_t entry; uint64_t lowaddr; -int i; int big_endian; MemoryRegion *hack = g_new(MemoryRegion, 1); @@ -198,9 +196,6 @@ qemu_irq *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq, qdev_init_nofail(nvic); sysbus_connect_irq(SYS_BUS_DEVICE(nvic), 0, qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ)); -for (i = 0; i < num_irq; i++) { -pic[i] = qdev_get_gpio_in(nvic, i); -} #ifdef TARGET_WORDS_BIGENDIAN big_endian = 1; @@ -234,7 +229,7 @@ qemu_irq *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq, memory_region_add_subregion(system_memory, 0xf000, hack); qemu_register_reset(armv7m_reset, cpu); -return pic; +return nvic; } static Property bitband_properties[] = { diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c index 3d6486f..82a4ad5 100644 --- a/hw/arm/stellaris.c +++ b/hw/arm/stellaris.c @@ -1210,8 +1210,7 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model, 0x40024000, 0x40025000, 0x40026000}; static const int gpio_irq[7] = {0, 1, 2, 3, 4, 30, 31}; -qemu_irq *pic; -DeviceState *gpio_dev[7]; +DeviceState *gpio_dev[7], *nvic; qemu_irq gpio_in[7][8]; qemu_irq gpio_out[7][8]; qemu_irq adc; @@ -1241,12 +1240,16 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model, vmstate_register_ram_global(sram); memory_region_add_subregion(system_memory, 0x2000, sram); -pic = armv7m_init(system_memory, flash_size, NUM_IRQ_LINES, +nvic = armv7m_init(system_memory, flash_size, NUM_IRQ_LINES, kernel_filename, cpu_model); if (board->dc1 & (1 << 16)) { dev = sysbus_create_varargs(TYPE_STELLARIS_ADC, 0x40038000, -pic[14], pic[15], pic[16], pic[17], NULL); +qdev_get_gpio_in(nvic, 14), +qdev_get_gpio_in(nvic, 15), +qdev_get_gpio_in(nvic, 16), +qdev_get_gpio_in(nvic, 17), +NULL); adc = qdev_get_gpio_in(dev, 0); } else { adc = NULL; @@ -1255,19 +1258,21 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model, if (board->dc2 & (0x1 << i)) { dev = sysbus_create_simple(TYPE_STELLARIS_GPTM, 0x4003 + i * 0x1000, - pic[timer_irq[i]]); + qdev_get_gpio_in(nvic, timer_irq[i])); /* TODO: This is incorrect, but we get away with it because the ADC output is only ever pulsed. */ qdev_connect_gpio_out(dev, 0, adc); } } -stellaris_sys_init(0x400fe000, pic[28], board, nd_table[0].macaddr.a); +stellaris_sys_init(0x400fe000, qdev_get_gpio_in(nvic, 28), + board, nd_table[0].macaddr.a); for (i = 0; i < 7; i++) { if (board->dc4 & (1 << i)) { gpio_dev[i] = sysbus_create_simple("pl061_luminary", gpio_addr[i], - pic[gpio_irq[i]]); + qdev_get_gpio_in(nvic, +gpio_irq[i])); for (j = 0; j < 8; j++) { gpio_in[i][j] = qdev_get_gpio_in(gpio_dev[i], j); gpio_out[i][j] = NULL; @@ -1276,7 +1281,8 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model, } if (board->dc2 & (1 << 12)) { -dev = sysbus_create_simple(TYPE_STELLARIS_I2C, 0x4002, pic[8]); +dev = sysbus_create_simple(TYPE_STELLARIS_I2C, 0x4002, +
[Qemu-devel] [PATCH v4 3/3] arm: stellaris: exit on external reset request
Add GPIO in for the stellaris board which calls qemu_system_reset_request() on reset request. --- hw/arm/stellaris.c | 12 1 file changed, 12 insertions(+) diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c index 82a4ad5..0114e0a 100644 --- a/hw/arm/stellaris.c +++ b/hw/arm/stellaris.c @@ -16,6 +16,7 @@ #include "net/net.h" #include "hw/boards.h" #include "exec/address-spaces.h" +#include "sysemu/sysemu.h" #define GPIO_A 0 #define GPIO_B 1 @@ -1176,6 +1177,14 @@ static int stellaris_adc_init(SysBusDevice *sbd) return 0; } +static +void do_sys_reset(void *opaque, int n, int level) +{ +if (level) { +qemu_system_reset_request(); +} +} + /* Board init. */ static stellaris_board_info stellaris_boards[] = { { "LM3S811EVB", @@ -1243,6 +1252,9 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model, nvic = armv7m_init(system_memory, flash_size, NUM_IRQ_LINES, kernel_filename, cpu_model); +qdev_connect_gpio_out_named(nvic, "SYSRESETREQ", 0, +qemu_allocate_irq(&do_sys_reset, NULL, 0)); + if (board->dc1 & (1 << 16)) { dev = sysbus_create_varargs(TYPE_STELLARIS_ADC, 0x40038000, qdev_get_gpio_in(nvic, 14), -- 2.1.4
[Qemu-devel] [PATCH v4 0/3] armv7-m: exit on external reset request
Resending patch set. The only change from v3 is to the message of #3. Changes armv7-m to implement the SYSRESETREQ register as GPIO. This necessitates some refactoring of armv7m_init() and the board code which calls it to allow the additional named GPIO to be used in board code. The Stellaris board installs a handler which calls qemu_system_reset_request(). Michael Davidsaver (3): armv7-m: Return DeviceState* from armv7m_init() armv7-m: Implement SYSRESETREQ arm: stellaris: exit on external reset request hw/arm/armv7m.c| 9 ++--- hw/arm/stellaris.c | 41 ++--- hw/arm/stm32f205_soc.c | 15 --- hw/intc/armv7m_nvic.c | 9 - include/hw/arm/arm.h | 2 +- 5 files changed, 49 insertions(+), 27 deletions(-) -- 2.1.4
[Qemu-devel] [PATCH v4 2/3] armv7-m: Implement SYSRESETREQ
Implement the SYSRESETREQ bit of the AIRCR register for armv7-m (ie. cortex-m3) to trigger a GPIO out. --- hw/intc/armv7m_nvic.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 3ec8408..6fc167e 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -28,6 +28,7 @@ typedef struct { MemoryRegion gic_iomem_alias; MemoryRegion container; uint32_t num_irq; +qemu_irq sysresetreq; } nvic_state; #define TYPE_NVIC "armv7m_nvic" @@ -348,10 +349,13 @@ static void nvic_writel(nvic_state *s, uint32_t offset, uint32_t value) break; case 0xd0c: /* Application Interrupt/Reset Control. */ if ((value >> 16) == 0x05fa) { +if (value & 4) { +qemu_irq_pulse(s->sysresetreq); +} if (value & 2) { qemu_log_mask(LOG_UNIMP, "VECTCLRACTIVE unimplemented\n"); } -if (value & 5) { +if (value & 1) { qemu_log_mask(LOG_UNIMP, "AIRCR system reset unimplemented\n"); } if (value & 0x700) { @@ -535,11 +539,14 @@ static void armv7m_nvic_instance_init(Object *obj) * value in the GICState struct. */ GICState *s = ARM_GIC_COMMON(obj); +DeviceState *dev = DEVICE(obj); +nvic_state *nvic = NVIC(obj); /* The ARM v7m may have anything from 0 to 496 external interrupt * IRQ lines. We default to 64. Other boards may differ and should * set the num-irq property appropriately. */ s->num_irq = 64; +qdev_init_gpio_out_named(dev, &nvic->sysresetreq, "SYSRESETREQ", 1); } static void armv7m_nvic_class_init(ObjectClass *klass, void *data) -- 2.1.4
[Qemu-devel] [PATCH 04/18] armv7m: Explicit error for bad vector table
Give an explicit error and abort when a load from VECBASE fails. Otherwise would likely jump to 0, which for v7-m holds the reset stack pointer address. Signed-off-by: Michael Davidsaver --- target-arm/helper.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/target-arm/helper.c b/target-arm/helper.c index 4178400..1d7ac43 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -5496,7 +5496,17 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) /* Clear IT bits */ env->condexec_bits = 0; env->regs[14] = lr; -addr = ldl_phys(cs->as, env->v7m.vecbase + env->v7m.exception * 4); +{ +MemTxResult result; +addr = address_space_ldl(cs->as, + env->v7m.vecbase + env->v7m.exception * 4, + MEMTXATTRS_UNSPECIFIED, &result); +if (result != MEMTX_OK) { +cpu_abort(cs, "Failed to read from exception vector table " + "entry %08x\n", + env->v7m.vecbase + env->v7m.exception * 4); +} +} env->regs[15] = addr & 0xfffe; env->thumb = addr & 1; if (!env->thumb) { -- 2.1.4
[Qemu-devel] [PATCH 03/18] armv7m: Complain about incorrect exception table entries.
For -M These should always be thumb mode. Log a message if this is seen. Signed-off-by: Michael Davidsaver --- target-arm/helper.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/target-arm/helper.c b/target-arm/helper.c index 4408100..4178400 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -5396,7 +5396,8 @@ static void do_v7m_exception_exit(CPUARMState *env) qemu_log_mask(LOG_GUEST_ERROR, "M profile return from interrupt with misaligned " "PC is UNPREDICTABLE\n"); -/* Actual hardware seems to ignore the lsbit, and there are several +/* The ARM calls for UsageFault when the T bit isn't set, but + * actual hardware seems to ignore the lsbit, and there are several * RTOSes out there which incorrectly assume the r15 in the stack * frame should be a Thumb-style "lsbit indicates ARM/Thumb" value. */ @@ -5498,6 +5499,12 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) addr = ldl_phys(cs->as, env->v7m.vecbase + env->v7m.exception * 4); env->regs[15] = addr & 0xfffe; env->thumb = addr & 1; +if (!env->thumb) { +qemu_log_mask(LOG_GUEST_ERROR, + "M profile interrupt handler %d with incorrect " + "instruction mode in PC is UNPREDICTABLE\n", + env->v7m.exception); +} } /* Function used to synchronize QEMU's AArch64 register set with AArch32 -- 2.1.4
[Qemu-devel] [PATCH 02/18] armv7m: Undo armv7m.hack
Add CPU unassigned access handler in place of special MemoryRegion to catch exception returns. Signed-off-by: Michael Davidsaver --- hw/arm/armv7m.c | 8 target-arm/cpu.c | 18 ++ 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c index a80d2ad..68146de 100644 --- a/hw/arm/armv7m.c +++ b/hw/arm/armv7m.c @@ -176,7 +176,6 @@ DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq, uint64_t entry; uint64_t lowaddr; int big_endian; -MemoryRegion *hack = g_new(MemoryRegion, 1); if (cpu_model == NULL) { cpu_model = "cortex-m3"; @@ -221,13 +220,6 @@ DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq, } } -/* Hack to map an additional page of ram at the top of the address - space. This stops qemu complaining about executing code outside RAM - when returning from an exception. */ -memory_region_init_ram(hack, NULL, "armv7m.hack", 0x1000, &error_fatal); -vmstate_register_ram_global(hack); -memory_region_add_subregion(system_memory, 0xf000, hack); - qemu_register_reset(armv7m_reset, cpu); return nvic; } diff --git a/target-arm/cpu.c b/target-arm/cpu.c index 30739fc..be026bc 100644 --- a/target-arm/cpu.c +++ b/target-arm/cpu.c @@ -280,6 +280,23 @@ bool arm_cpu_exec_interrupt(CPUState *cs, int interrupt_request) } #if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64) +static void arm_v7m_unassigned_access(CPUState *cpu, hwaddr addr, + bool is_write, bool is_exec, int opaque, + unsigned size) +{ +ARMCPU *arm = ARM_CPU(cpu); +CPUARMState *env = &arm->env; + +if (env->v7m.exception != 0 && addr >= 0xfff0 && !is_write) { +cpu->exception_index = EXCP_EXCEPTION_EXIT; +cpu_loop_exit(cpu); +} else { +/* TODO, signal some *Fault? */ +cpu_abort(cpu, "Trying to access outside RAM or ROM at 0x" + TARGET_FMT_plx "\n", addr); +} +} + static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int interrupt_request) { CPUClass *cc = CPU_GET_CLASS(cs); @@ -909,6 +926,7 @@ static void arm_v7m_class_init(ObjectClass *oc, void *data) cc->do_interrupt = arm_v7m_cpu_do_interrupt; #endif +cc->do_unassigned_access = arm_v7m_unassigned_access; cc->cpu_exec_interrupt = arm_v7m_cpu_exec_interrupt; } -- 2.1.4
[Qemu-devel] [PATCH 05/18] armv7m: expand NVIC state
Expand the NVIC to fully support -M priorities and masking. Doesn't use GIC code. Move some state to ARMCPU to allow calculation of exception masking. Add storage for PRIGROUP to configure group/sub-group split. Track group and sub-group in separate fields for quick comparison. Mix in vector # with sub-group as per tie breaking rules. NVIC now derives directly from SysBusDevice, and struct NVICClass is eliminated. Also add DPRINTF() macro. Signed-off-by: Michael Davidsaver --- hw/intc/armv7m_nvic.c | 74 ++- target-arm/cpu.h | 3 +++ 2 files changed, 52 insertions(+), 25 deletions(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 6fc167e..487a09a 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -13,43 +13,67 @@ #include "hw/sysbus.h" #include "qemu/timer.h" #include "hw/arm/arm.h" +#include "target-arm/cpu.h" #include "exec/address-spaces.h" -#include "gic_internal.h" -typedef struct { -GICState gic; +/*#define DEBUG_NVIC 0 + */ +#ifdef DEBUG_NVIC +#define DPRINTF(LVL, fmt, ...) \ +do { if ((LVL) <= DEBUG_NVIC) { \ +fprintf(stderr, "armv7m_nvic: " fmt , ## __VA_ARGS__); \ +} } while (0) +#else +#define DPRINTF(LVL, fmt, ...) do {} while (0) +#endif + +/* the number of IRQ lines in addition to the 16 internal + * exception vectors. + */ +#define NVIC_MAX_IRQ 496 + +#define NVIC_MAX_VECTORS 512 + +struct vec_info { +uint16_t prio_sub; /* sub-group priority*512 + exception# */ +int8_t prio_group; /* group priority [-2, 0x7f] */ +uint8_t raw_prio; /* value writen by guest */ +uint8_t enabled; +uint8_t pending; +uint8_t active; +uint8_t level; +/* exceptions <=15 never set level */ +}; +typedef struct vec_info vec_info; + +struct nvic_state { +/*< private >*/ +SysBusDevice parent_obj; +/*< public >*/ + +ARMCPU *cpu; /* NVIC is so closely tied to the CPU, just keep a ref */ + +vec_info vectors[NVIC_MAX_VECTORS]; + +uint8_t prigroup; + struct { uint32_t control; uint32_t reload; int64_t tick; QEMUTimer *timer; } systick; -MemoryRegion sysregmem; -MemoryRegion gic_iomem_alias; -MemoryRegion container; + +MemoryRegion iomem; /* system control space and NVIC */ + uint32_t num_irq; +qemu_irq excpout; qemu_irq sysresetreq; -} nvic_state; +}; +typedef struct nvic_state nvic_state; #define TYPE_NVIC "armv7m_nvic" -/** - * NVICClass: - * @parent_reset: the parent class' reset handler. - * - * A model of the v7M NVIC and System Controller - */ -typedef struct NVICClass { -/*< private >*/ -ARMGICClass parent_class; -/*< public >*/ -DeviceRealize parent_realize; -void (*parent_reset)(DeviceState *dev); -} NVICClass; - -#define NVIC_CLASS(klass) \ -OBJECT_CLASS_CHECK(NVICClass, (klass), TYPE_NVIC) -#define NVIC_GET_CLASS(obj) \ -OBJECT_GET_CLASS(NVICClass, (obj), TYPE_NVIC) + #define NVIC(obj) \ OBJECT_CHECK(nvic_state, (obj), TYPE_NVIC) diff --git a/target-arm/cpu.h b/target-arm/cpu.h index 815fef8..c193fbb 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -398,6 +398,9 @@ typedef struct CPUARMState { uint32_t control; int current_sp; int exception; +int exception_prio; +unsigned pending; +int pending_prio; } v7m; /* Information associated with an exception about to be taken: -- 2.1.4
[Qemu-devel] [PATCH 00/18] Fix exception handling and msr/mrs access
This series grew from a previous incorrect patch attempting to fix some incorrect behavior. After spending some time going through the arch. ref. manual for v7-M I think I understand better how this should work and have made a number of changes which actually improve the situation. These changes have not yet been cross checked against real hardware, and I therefore don't consider them mergeable. It's gotten big enough though that I'd like to get some feedback. I think the changes in this series effect only ARMv7-M specific code with the exception of removing references to NVIC from the GIC code. * Add unprivileged access case for MRS/MSR instructions * Priority based exception masking with PRIMASK, FAULTMASK, and BASEPRI. * Auto-clear FAULTMASK on exception return (except NMI) * Validation and consistency checking on exception return * Exception priorities using PRIGROUP * Exception escalation to HardFault when priority permits * Escalation to unrecoverable exception otherwise (though the action is not correct, see below) * Correct calculation of the RETTOBASE field of ICSR * Remove the need for the armv7m.hack MemoryRegion to catch exception returns * Fill in previously unimplemented HFSR, CFSR, and CCR registers This series removes the dependence of the NVIC code on the GIC. The GIC doesn't have the concept of PRIGROUP to change the size of the group priority field. Also, there are a lot of cases in this code which I don't understand and worry about breaking. Now that I have things working (I think), I could look at recombining them if this is desired. Some additional state is also added to v7m in struct CPUARMState so that all the information needed in arm_v7m_cpu_exec_interrupt() is found in one place. I started by having this state split between CPU and struct nvic_state, but found this confusing. Some guidance would be helpful. I add a pointer to ARMCPU* in struct nvic_state which is populated in armv7m_nvic_realize(). I think this is reasonable given the tight coupling between NVIC and CPU, but it does look ugly. At the moment I've left the action of an unrecoverable exception to call cpu_abort(). I'm not sure of the value of implementing the actual defined behavior in the context of QEMU. I've tried to add VMState as appropriate, but have not tested it. I looked briefly at qtest, but can't quite see how to use it given the need to execute code to test most of the exception behavior. Is something like this feasible at present? Regards, Michael Michael Davidsaver (18): armv7m: MRS/MSR handle unprivileged access armv7m: Undo armv7m.hack armv7m: Complain about incorrect exception table entries. armv7m: Explicit error for bad vector table armv7m: expand NVIC state armv7m: new NVIC utility functions armv7m: Update NVIC registers armv7m: fix RETTOBASE armv7m: NVIC update vmstate armv7m: NVIC initialization armv7m: fix I and F flag handling armv7m: simpler/faster exception start armv7m: implement CFSR and HFSR armv7m: auto-clear FAULTMASK arm: gic: Remove references to NVIC armv7m: check exception return consistency armv7m: implement CCR armv7m: prevent unprivileged write to STIR hw/arm/armv7m.c | 8 - hw/intc/arm_gic.c| 14 +- hw/intc/arm_gic_common.c | 23 +- hw/intc/armv7m_nvic.c| 777 --- hw/intc/gic_internal.h | 7 +- target-arm/cpu.c | 44 +-- target-arm/cpu.h | 35 ++- target-arm/helper.c | 222 ++ target-arm/machine.c | 7 +- 9 files changed, 843 insertions(+), 294 deletions(-) -- 2.1.4
[Qemu-devel] [PATCH 01/18] armv7m: MRS/MSR handle unprivileged access
The MRS and MSR instruction handling isn't checking the current permission level. Signed-off-by: Michael Davidsaver --- target-arm/helper.c | 79 + 1 file changed, 37 insertions(+), 42 deletions(-) diff --git a/target-arm/helper.c b/target-arm/helper.c index 4ecae61..4408100 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -7365,23 +7365,32 @@ uint32_t HELPER(get_r13_banked)(CPUARMState *env, uint32_t mode) uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg) { -ARMCPU *cpu = arm_env_get_cpu(env); +uint32_t mask; +unsigned el = arm_current_el(env); + +/* First handle registers which unprivileged can read */ + +switch (reg) { +case 0 ... 7: /* xPSR sub-fields */ +mask = 0; +if ((reg&1) && el) { +mask |= 0x01ff; /* IPSR (unpriv. reads as zero) */ +} +if (!(reg&4)) { +mask |= 0xf800; /* APSR */ +} +/* EPSR reads as zero */ +return xpsr_read(env) & mask; +break; +case 20: /* CONTROL */ +return env->v7m.control; +} + +if (el == 0) { +return 0; /* unprivileged reads others as zero */ +} switch (reg) { -case 0: /* APSR */ -return xpsr_read(env) & 0xf800; -case 1: /* IAPSR */ -return xpsr_read(env) & 0xf80001ff; -case 2: /* EAPSR */ -return xpsr_read(env) & 0xff00fc00; -case 3: /* xPSR */ -return xpsr_read(env) & 0xff00fdff; -case 5: /* IPSR */ -return xpsr_read(env) & 0x01ff; -case 6: /* EPSR */ -return xpsr_read(env) & 0x0700fc00; -case 7: /* IEPSR */ -return xpsr_read(env) & 0x0700edff; case 8: /* MSP */ return env->v7m.current_sp ? env->v7m.other_sp : env->regs[13]; case 9: /* PSP */ @@ -7393,40 +7402,26 @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg) return env->v7m.basepri; case 19: /* FAULTMASK */ return (env->daif & PSTATE_F) != 0; -case 20: /* CONTROL */ -return env->v7m.control; default: -/* ??? For debugging only. */ -cpu_abort(CPU(cpu), "Unimplemented system register read (%d)\n", reg); +qemu_log_mask(LOG_GUEST_ERROR, "Attempt to read unknown special" + " register %d\n", reg); return 0; } } void HELPER(v7m_msr)(CPUARMState *env, uint32_t reg, uint32_t val) { -ARMCPU *cpu = arm_env_get_cpu(env); +if (arm_current_el(env) == 0 && reg > 7) { +/* only xPSR sub-fields may be written by unprivileged */ +return; +} switch (reg) { -case 0: /* APSR */ -xpsr_write(env, val, 0xf800); -break; -case 1: /* IAPSR */ -xpsr_write(env, val, 0xf800); -break; -case 2: /* EAPSR */ -xpsr_write(env, val, 0xfe00fc00); -break; -case 3: /* xPSR */ -xpsr_write(env, val, 0xfe00fc00); -break; -case 5: /* IPSR */ -/* IPSR bits are readonly. */ -break; -case 6: /* EPSR */ -xpsr_write(env, val, 0x0600fc00); -break; -case 7: /* IEPSR */ -xpsr_write(env, val, 0x0600fc00); +case 0 ... 7: /* xPSR sub-fields */ +/* only APSR is actually writable */ +if (reg&4) { +xpsr_write(env, val, 0xf800); /* APSR */ +} break; case 8: /* MSP */ if (env->v7m.current_sp) @@ -7467,8 +7462,8 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t reg, uint32_t val) switch_v7m_sp(env, (val & 2) != 0); break; default: -/* ??? For debugging only. */ -cpu_abort(CPU(cpu), "Unimplemented system register write (%d)\n", reg); +qemu_log_mask(LOG_GUEST_ERROR, "Attempt to write unknown special" + " register %d\n", reg); return; } } -- 2.1.4
[Qemu-devel] [PATCH 08/18] armv7m: fix RETTOBASE
The polarity is reversed, and it should include internal exceptions. Should be set when # of active exceptions <= 1. Signed-off-by: Michael Davidsaver --- hw/intc/armv7m_nvic.c | 17 + 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 30e349e..3b10dee 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -432,16 +432,20 @@ static uint32_t nvic_readl(nvic_state *s, uint32_t offset) val = cpu->env.v7m.exception; /* VECTPENDING */ val |= (cpu->env.v7m.pending << 12)&0x1ff; -/* ISRPENDING and RETTOBASE */ +/* ISRPENDING - Set it any externel IRQ pending (vector>=16) */ for (irq = 16; irq < s->num_irq; irq++) { if (s->vectors[irq].pending) { val |= (1 << 22); break; } +} +/* RETTOBASE - Set if no (other) handler is active */ +for (irq = 1; irq < s->num_irq; irq++) { if (irq != cpu->env.v7m.exception && s->vectors[irq].active) { -val |= (1 << 11); +val |= (1 << 11); /* some other handler is active */ } } +val ^= (1<<11); /* invert */ /* PENDSTSET */ if (s->vectors[ARMV7M_EXCP_SYSTICK].pending) { val |= (1 << 26); @@ -454,6 +458,7 @@ static uint32_t nvic_readl(nvic_state *s, uint32_t offset) if (s->vectors[ARMV7M_EXCP_NMI].pending) { val |= (1 << 31); } +/* ISRPREEMPT not implemented */ return val; case 0xd08: /* Vector Table Offset. */ return cpu->env.v7m.vecbase; @@ -588,10 +593,14 @@ static void nvic_writel(nvic_state *s, uint32_t offset, uint32_t value) qemu_irq_pulse(s->sysresetreq); } if (value & 2) { -qemu_log_mask(LOG_UNIMP, "VECTCLRACTIVE unimplemented\n"); +qemu_log_mask(LOG_GUEST_ERROR, + "Setting VECTCLRACTIVE when not in DEBUG mode " + "is UNPREDICTABLE\n"); } if (value & 1) { -qemu_log_mask(LOG_UNIMP, "AIRCR system reset unimplemented\n"); +qemu_log_mask(LOG_GUEST_ERROR, + "Setting VECTRESET when not in DEBUG mode " + "is UNPREDICTABLE\n"); } if (value & 0x700) { unsigned i; -- 2.1.4
[Qemu-devel] [PATCH 15/18] arm: gic: Remove references to NVIC
armv7m_nvic.c no longer relies on the GIC. Remove REV_NVIC and conditionals which use it. Signed-off-by: Michael Davidsaver --- hw/intc/arm_gic.c| 14 +++--- hw/intc/arm_gic_common.c | 23 --- hw/intc/gic_internal.h | 7 ++- 3 files changed, 17 insertions(+), 27 deletions(-) diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index 8bad132..d543a93 100644 --- a/hw/intc/arm_gic.c +++ b/hw/intc/arm_gic.c @@ -184,7 +184,7 @@ static void gic_set_irq(void *opaque, int irq, int level) return; } -if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) { +if (s->revision == REV_11MPCORE) { gic_set_irq_11mpcore(s, irq, level, cm, target); } else { gic_set_irq_generic(s, irq, level, cm, target); @@ -335,7 +335,7 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu, MemTxAttrs attrs) return 1023; } -if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) { +if (s->revision == REV_11MPCORE) { /* Clear pending flags for both level and edge triggered interrupts. * Level triggered IRQs will be reasserted once they become inactive. */ @@ -514,7 +514,7 @@ void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs) return; /* No active IRQ. */ } -if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) { +if (s->revision == REV_11MPCORE) { /* Mark level triggered interrupts as pending if they are still raised. */ if (!GIC_TEST_EDGE_TRIGGER(irq) && GIC_TEST_ENABLED(irq, cm) @@ -672,7 +672,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs) } else if (offset < 0xf10) { goto bad_reg; } else if (offset < 0xf30) { -if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) { +if (s->revision == REV_11MPCORE) { goto bad_reg; } @@ -883,7 +883,7 @@ static void gic_dist_writeb(void *opaque, hwaddr offset, if (irq < GIC_NR_SGIS) value |= 0xaa; for (i = 0; i < 4; i++) { -if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) { +if (s->revision == REV_11MPCORE) { if (value & (1 << (i * 2))) { GIC_SET_MODEL(irq + i); } else { @@ -901,7 +901,7 @@ static void gic_dist_writeb(void *opaque, hwaddr offset, goto bad_reg; } else if (offset < 0xf20) { /* GICD_CPENDSGIRn */ -if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) { +if (s->revision == REV_11MPCORE) { goto bad_reg; } irq = (offset - 0xf10); @@ -912,7 +912,7 @@ static void gic_dist_writeb(void *opaque, hwaddr offset, } } else if (offset < 0xf30) { /* GICD_SPENDSGIRn */ -if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) { +if (s->revision == REV_11MPCORE) { goto bad_reg; } irq = (offset - 0xf20); diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c index 9c82b97..4987047 100644 --- a/hw/intc/arm_gic_common.c +++ b/hw/intc/arm_gic_common.c @@ -97,9 +97,7 @@ void gic_init_irqs_and_mmio(GICState *s, qemu_irq_handler handler, * [N+32..N+63] PPIs for CPU 1 * ... */ -if (s->revision != REV_NVIC) { -i += (GIC_INTERNAL * s->num_cpu); -} +i += (GIC_INTERNAL * s->num_cpu); qdev_init_gpio_in(DEVICE(s), handler, i); for (i = 0; i < s->num_cpu; i++) { @@ -113,16 +111,12 @@ void gic_init_irqs_and_mmio(GICState *s, qemu_irq_handler handler, memory_region_init_io(&s->iomem, OBJECT(s), ops, s, "gic_dist", 0x1000); sysbus_init_mmio(sbd, &s->iomem); -if (s->revision != REV_NVIC) { -/* This is the main CPU interface "for this core". It is always - * present because it is required by both software emulation and KVM. - * NVIC is not handled here because its CPU interface is different, - * neither it can use KVM. - */ -memory_region_init_io(&s->cpuiomem[0], OBJECT(s), ops ? &ops[1] : NULL, - s, "gic_cpu", s->revision == 2 ? 0x1000 : 0x100); -sysbus_init_mmio(sbd, &s->cpuiomem[0]); -} +/* This is the main CPU interface "for this core". It is always + * present because it is required by both software emulation and KVM. + */ +memory_region_init_io(&s->cpuiomem[0], OBJECT(s), ops ? &ops[1] : NULL, + s, "gic_cpu", s->revision == 2 ? 0x1000 : 0x100); +sysbus_init_mmio(sbd, &s->cpuiomem[0]); } static void arm_gic_common_realize(DeviceState *dev, Error **errp) @@ -154,7 +148,7 @@ static void arm_gi
[Qemu-devel] [PATCH 06/18] armv7m: new NVIC utility functions
Internal functions for operations previously done by GIC internals. nvic_irq_update() recalculates highest pending/active exceptions. armv7m_nvic_set_pending() include exception escalation logic. armv7m_nvic_acknowledge_irq() and nvic_irq_update() update ARMCPU fields. Signed-off-by: Michael Davidsaver --- hw/intc/armv7m_nvic.c | 250 +++--- 1 file changed, 235 insertions(+), 15 deletions(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 487a09a..ebb4d4e 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -140,36 +140,256 @@ static void systick_reset(nvic_state *s) timer_del(s->systick.timer); } -/* The external routines use the hardware vector numbering, ie. the first - IRQ is #16. The internal GIC routines use #32 as the first IRQ. */ +/* caller must call nvic_irq_update() after this */ +static +void set_prio(nvic_state *s, unsigned irq, uint8_t prio) +{ +unsigned submask = (1<<(s->prigroup+1))-1; + +assert(irq > 3); /* only use for configurable prios */ +assert(irq < NVIC_MAX_VECTORS); + +s->vectors[irq].raw_prio = prio; +s->vectors[irq].prio_group = (prio>>(s->prigroup+1)); +s->vectors[irq].prio_sub = irq + (prio&submask)*NVIC_MAX_VECTORS; + +DPRINTF(0, "Set %u priority grp %d sub %u\n", irq, +s->vectors[irq].prio_group, s->vectors[irq].prio_sub); +} + +/* recompute highest pending */ +static +void nvic_irq_update(nvic_state *s, int update_active) +{ +unsigned i; +int lvl; +CPUARMState *env = &s->cpu->env; +int16_t act_group = 0x100, pend_group = 0x100; +uint16_t act_sub = 0, pend_sub = 0; +uint16_t act_irq = 0, pend_irq = 0; + +/* find highest priority */ +for (i = 1; i < s->num_irq; i++) { +vec_info *I = &s->vectors[i]; + +DPRINTF(2, " VECT %d %d:%u\n", i, I->prio_group, I->prio_sub); + +if (I->active && ((I->prio_group < act_group) +|| (I->prio_group == act_group && I->prio_sub < act_sub))) +{ +act_group = I->prio_group; +act_sub = I->prio_sub; +act_irq = i; +} + +if (I->enabled && I->pending && ((I->prio_group < pend_group) +|| (I->prio_group == pend_group && I->prio_sub < pend_sub))) +{ +pend_group = I->prio_group; +pend_sub = I->prio_sub; +pend_irq = i; +} +} + +env->v7m.pending = pend_irq; +env->v7m.pending_prio = pend_group; + +if (update_active) { +env->v7m.exception = act_irq; +env->v7m.exception_prio = act_group; +} + +/* Raise NVIC output even if pend_group is masked. + * This is necessary as we get no notification + * when PRIMASK et al. are changed. + * As long as our output is high cpu_exec() will call + * into arm_v7m_cpu_exec_interrupt() frequently, which + * then tests to see if the pending exception + * is permitted. + */ +lvl = pend_irq > 0; +DPRINTF(1, "highest pending %d %d:%u\n", pend_irq, pend_group, pend_sub); +DPRINTF(1, "highest active %d %d:%u\n", act_irq, act_group, act_sub); + +DPRINTF(0, "IRQ %c highest act %d pend %d\n", +lvl ? 'X' : '_', act_irq, pend_irq); + +qemu_set_irq(s->excpout, lvl); +} + +static +void armv7m_nvic_clear_pending(void *opaque, int irq) +{ +nvic_state *s = (nvic_state *)opaque; +vec_info *I; + +assert(irq >= 0); +assert(irq < NVIC_MAX_VECTORS); + +I = &s->vectors[irq]; +if (I->pending) { +I->pending = 0; +nvic_irq_update(s, 0); +} +} + void armv7m_nvic_set_pending(void *opaque, int irq) { nvic_state *s = (nvic_state *)opaque; -if (irq >= 16) -irq += 16; -gic_set_pending_private(&s->gic, 0, irq); +CPUARMState *env = &s->cpu->env; +vec_info *I; +int active = s->cpu->env.v7m.exception; + +assert(irq > 0); +assert(irq < NVIC_MAX_VECTORS); + +I = &s->vectors[irq]; + +if (irq < ARMV7M_EXCP_SYSTICK && irq != ARMV7M_EXCP_DEBUG) { +int runnable = armv7m_excp_unmasked(s->cpu); +/* test for exception escalation for vectors other than: + * Debug (12), SysTick (15), and all external IRQs (>=16) + */ +unsigned escalate = 0; +if (I->active) { +/* trying to pend an active fault (possibly nested). + * eg. UsageFault in UsageFault handler + */ +escalate = 1; +DPRINTF(0, " Escalate, active\n"); +} else if (!I->enabled) { +/* trying to pend a disabled fault + * eg. UsageF
[Qemu-devel] [PATCH 09/18] armv7m: NVIC update vmstate
Signed-off-by: Michael Davidsaver --- hw/intc/armv7m_nvic.c | 64 +-- 1 file changed, 62 insertions(+), 2 deletions(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 3b10dee..c860b36 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -810,15 +810,75 @@ static const MemoryRegionOps nvic_sysreg_ops = { .endianness = DEVICE_NATIVE_ENDIAN, }; +static +int nvic_post_load(void *opaque, int version_id) +{ +nvic_state *s = opaque; +unsigned i; + +/* evil hack to get ARMCPU* ahead of time */ +assert(cpus.tqh_first); +assert(!CPU_NEXT(cpus.tqh_first)); +s->cpu = ARM_CPU(cpus.tqh_first); +assert(s->cpu); + +/* recalculate priorities */ +for (i = 4; i < s->num_irq; i++) { +set_prio(s, i, s->vectors[i].raw_prio); +} + +nvic_irq_update(s, highest_runnable_prio(s->cpu)); + +return 0; +} + +static +int vec_info_get(QEMUFile *f, void *pv, size_t size) +{ +vec_info *I = pv; +I->prio_sub = qemu_get_be16(f); +I->prio_group = qemu_get_byte(f); +I->raw_prio = qemu_get_ubyte(f); +I->enabled = qemu_get_ubyte(f); +I->pending = qemu_get_ubyte(f); +I->active = qemu_get_ubyte(f); +I->level = qemu_get_ubyte(f); +return 0; +} + +static +void vec_info_put(QEMUFile *f, void *pv, size_t size) +{ +vec_info *I = pv; +qemu_put_be16(f, I->prio_sub); +qemu_put_byte(f, I->prio_group); +qemu_put_ubyte(f, I->raw_prio); +qemu_put_ubyte(f, I->enabled); +qemu_put_ubyte(f, I->pending); +qemu_put_ubyte(f, I->active); +qemu_put_ubyte(f, I->level); +} + +static const VMStateInfo vmstate_info = { +.name = "armv7m_nvic_info", +.get = vec_info_get, +.put = vec_info_put, +}; + static const VMStateDescription vmstate_nvic = { .name = "armv7m_nvic", -.version_id = 1, -.minimum_version_id = 1, +.version_id = 2, +.minimum_version_id = 2, +.post_load = &nvic_post_load, .fields = (VMStateField[]) { +VMSTATE_ARRAY(vectors, nvic_state, NVIC_MAX_VECTORS, 0, + vmstate_info, vec_info), +VMSTATE_UINT8(prigroup, nvic_state), VMSTATE_UINT32(systick.control, nvic_state), VMSTATE_UINT32(systick.reload, nvic_state), VMSTATE_INT64(systick.tick, nvic_state), VMSTATE_TIMER_PTR(systick.timer, nvic_state), +VMSTATE_UINT32(num_irq, nvic_state), VMSTATE_END_OF_LIST() } }; -- 2.1.4
[Qemu-devel] [PATCH 12/18] armv7m: simpler/faster exception start
No need to bounce through EXCP_IRQ handling for non-IRQ exceptions. just update CPU state directly. Signed-off-by: Michael Davidsaver --- target-arm/helper.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/target-arm/helper.c b/target-arm/helper.c index 1d7ac43..2541890 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -5433,23 +5433,21 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) /* For exceptions we just mark as pending on the NVIC, and let that handle it. */ -/* TODO: Need to escalate if the current priority is higher than the - one we're raising. */ switch (cs->exception_index) { case EXCP_UDEF: armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE); -return; +break; case EXCP_SWI: /* The PC already points to the next instruction. */ armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_SVC); -return; +break; case EXCP_PREFETCH_ABORT: case EXCP_DATA_ABORT: /* TODO: if we implemented the MPU registers, this is where we * should set the MMFAR, etc from exception.fsr and exception.vaddress. */ armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_MEM); -return; +break; case EXCP_BKPT: if (semihosting_enabled()) { int nr; @@ -5466,7 +5464,6 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_DEBUG); return; case EXCP_IRQ: -env->v7m.exception = armv7m_nvic_acknowledge_irq(env->nvic); break; case EXCP_EXCEPTION_EXIT: do_v7m_exception_exit(env); @@ -5476,6 +5473,10 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) return; /* Never happens. Keep compiler happy. */ } +armv7m_nvic_acknowledge_irq(env->nvic); + +qemu_log_mask(CPU_LOG_INT, "... as %d\n", env->v7m.exception); + /* Align stack pointer. */ /* ??? Should only do this if Configuration Control Register STACKALIGN bit is set. */ -- 2.1.4
[Qemu-devel] [PATCH 18/18] armv7m: prevent unprivileged write to STIR
Prevent unprivileged from writing to the Software Triggered Interrupt register Signed-off-by: Michael Davidsaver --- hw/intc/armv7m_nvic.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index ca8c93c..b744cd5 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -654,7 +654,9 @@ static void nvic_writel(nvic_state *s, uint32_t offset, uint32_t value) "NVIC: fault status registers unimplemented\n"); break; case 0xf00: /* Software Triggered Interrupt Register */ -if ((value & 0x1ff) < NVIC_MAX_IRQ) { +/* STIR write allowed if privlaged or USERSETMPEND set */ +if ((arm_current_el(&cpu->env) || (cpu->env.v7m.ccr&2)) +&& ((value & 0x1ff) < NVIC_MAX_IRQ)) { armv7m_nvic_set_pending(s, (value&0x1ff)+16); } break; -- 2.1.4
[Qemu-devel] [PATCH 07/18] armv7m: Update NVIC registers
Replace use of GIC state/functions with new NVIC. Signed-off-by: Michael Davidsaver --- hw/intc/armv7m_nvic.c | 233 -- 1 file changed, 168 insertions(+), 65 deletions(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index ebb4d4e..30e349e 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -394,13 +394,13 @@ void set_irq_level(void *opaque, int n, int level) static uint32_t nvic_readl(nvic_state *s, uint32_t offset) { -ARMCPU *cpu; +ARMCPU *cpu = s->cpu; uint32_t val; int irq; switch (offset) { case 4: /* Interrupt Control Type. */ -return (s->num_irq / 32) - 1; +return ((s->num_irq - 16) / 32) - 1; case 0x10: /* SysTick Control and Status. */ val = s->systick.control; s->systick.control &= ~SYSTICK_COUNTFLAG; @@ -426,45 +426,39 @@ static uint32_t nvic_readl(nvic_state *s, uint32_t offset) case 0x1c: /* SysTick Calibration Value. */ return 1; case 0xd00: /* CPUID Base. */ -cpu = ARM_CPU(current_cpu); return cpu->midr; case 0xd04: /* Interrupt Control State. */ /* VECTACTIVE */ -cpu = ARM_CPU(current_cpu); val = cpu->env.v7m.exception; -if (val == 1023) { -val = 0; -} else if (val >= 32) { -val -= 16; -} /* VECTPENDING */ -if (s->gic.current_pending[0] != 1023) -val |= (s->gic.current_pending[0] << 12); +val |= (cpu->env.v7m.pending << 12)&0x1ff; /* ISRPENDING and RETTOBASE */ -for (irq = 32; irq < s->num_irq; irq++) { -if (s->gic.irq_state[irq].pending) { +for (irq = 16; irq < s->num_irq; irq++) { +if (s->vectors[irq].pending) { val |= (1 << 22); break; } -if (irq != cpu->env.v7m.exception && s->gic.irq_state[irq].active) { +if (irq != cpu->env.v7m.exception && s->vectors[irq].active) { val |= (1 << 11); } } /* PENDSTSET */ -if (s->gic.irq_state[ARMV7M_EXCP_SYSTICK].pending) +if (s->vectors[ARMV7M_EXCP_SYSTICK].pending) { val |= (1 << 26); +} /* PENDSVSET */ -if (s->gic.irq_state[ARMV7M_EXCP_PENDSV].pending) +if (s->vectors[ARMV7M_EXCP_PENDSV].pending) { val |= (1 << 28); +} /* NMIPENDSET */ -if (s->gic.irq_state[ARMV7M_EXCP_NMI].pending) +if (s->vectors[ARMV7M_EXCP_NMI].pending) { val |= (1 << 31); +} return val; case 0xd08: /* Vector Table Offset. */ -cpu = ARM_CPU(current_cpu); return cpu->env.v7m.vecbase; case 0xd0c: /* Application Interrupt/Reset Control. */ -return 0xfa05; +return 0xfa05 | (s->prigroup<<8); case 0xd10: /* System Control. */ /* TODO: Implement SLEEPONEXIT. */ return 0; @@ -473,20 +467,20 @@ static uint32_t nvic_readl(nvic_state *s, uint32_t offset) return 0; case 0xd24: /* System Handler Status. */ val = 0; -if (s->gic.irq_state[ARMV7M_EXCP_MEM].active) val |= (1 << 0); -if (s->gic.irq_state[ARMV7M_EXCP_BUS].active) val |= (1 << 1); -if (s->gic.irq_state[ARMV7M_EXCP_USAGE].active) val |= (1 << 3); -if (s->gic.irq_state[ARMV7M_EXCP_SVC].active) val |= (1 << 7); -if (s->gic.irq_state[ARMV7M_EXCP_DEBUG].active) val |= (1 << 8); -if (s->gic.irq_state[ARMV7M_EXCP_PENDSV].active) val |= (1 << 10); -if (s->gic.irq_state[ARMV7M_EXCP_SYSTICK].active) val |= (1 << 11); -if (s->gic.irq_state[ARMV7M_EXCP_USAGE].pending) val |= (1 << 12); -if (s->gic.irq_state[ARMV7M_EXCP_MEM].pending) val |= (1 << 13); -if (s->gic.irq_state[ARMV7M_EXCP_BUS].pending) val |= (1 << 14); -if (s->gic.irq_state[ARMV7M_EXCP_SVC].pending) val |= (1 << 15); -if (s->gic.irq_state[ARMV7M_EXCP_MEM].enabled) val |= (1 << 16); -if (s->gic.irq_state[ARMV7M_EXCP_BUS].enabled) val |= (1 << 17); -if (s->gic.irq_state[ARMV7M_EXCP_USAGE].enabled) val |= (1 << 18); +if (s->vectors[ARMV7M_EXCP_MEM].active) val |= (1 << 0); +if (s->vectors[ARMV7M_EXCP_BUS].active) val |= (1 << 1); +if (s->vectors[ARMV7M_EXCP_USAGE].active) val |= (1 << 3); +if (s->vectors[ARMV7M_EXCP_SVC].active) val |= (1 << 7); +if (s->vectors[ARMV7M_EXCP_DEBUG].active) val |= (1 << 8); +if (s->vectors[ARMV7M_EXCP_PENDSV].active) val |= (1 << 10); +if (s->v
[Qemu-devel] [PATCH 11/18] armv7m: fix I and F flag handling
Despite having the same notation, these bits have completely different meaning than -AR. Add armv7m_excp_unmasked() to calculate the currently runable exception priority taking into account masks and active handlers. Use this in conjunction with the pending exception priority to determine if the pending exception can interrupt execution. Signed-off-by: Michael Davidsaver --- target-arm/cpu.c | 26 +++--- target-arm/cpu.h | 27 ++- 2 files changed, 33 insertions(+), 20 deletions(-) diff --git a/target-arm/cpu.c b/target-arm/cpu.c index be026bc..5d03117 100644 --- a/target-arm/cpu.c +++ b/target-arm/cpu.c @@ -173,6 +173,8 @@ static void arm_cpu_reset(CPUState *s) uint32_t initial_pc; /* Loaded from 0x4 */ uint8_t *rom; +env->v7m.exception_prio = env->v7m.pending_prio = 0x100; + env->daif &= ~PSTATE_I; rom = rom_ptr(0); if (rom) { @@ -301,29 +303,15 @@ static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int interrupt_request) { CPUClass *cc = CPU_GET_CLASS(cs); ARMCPU *cpu = ARM_CPU(cs); -CPUARMState *env = &cpu->env; bool ret = false; - -if (interrupt_request & CPU_INTERRUPT_FIQ -&& !(env->daif & PSTATE_F)) { -cs->exception_index = EXCP_FIQ; -cc->do_interrupt(cs); -ret = true; -} -/* ARMv7-M interrupt return works by loading a magic value - * into the PC. On real hardware the load causes the - * return to occur. The qemu implementation performs the - * jump normally, then does the exception return when the - * CPU tries to execute code at the magic address. - * This will cause the magic PC value to be pushed to - * the stack if an interrupt occurred at the wrong time. - * We avoid this by disabling interrupts when - * pc contains a magic address. +/* ARMv7-M interrupt masking works differently than -A or -R. + * There is no FIQ/IRQ distinction. + * Instead of masking interrupt sources, the I and F bits + * (along with basepri) mask certain exception priority levels. */ if (interrupt_request & CPU_INTERRUPT_HARD -&& !(env->daif & PSTATE_I) -&& (env->regs[15] < 0xfff0)) { +&& (armv7m_excp_unmasked(cpu) >= cpu->env.v7m.pending_prio)) { cs->exception_index = EXCP_IRQ; cc->do_interrupt(cs); ret = true; diff --git a/target-arm/cpu.h b/target-arm/cpu.h index c193fbb..29d89ce 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -1033,9 +1033,34 @@ void arm_cpu_list(FILE *f, fprintf_function cpu_fprintf); uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t excp_idx, uint32_t cur_el, bool secure); +/* @returns highest (numerically lowest) unmasked exception priority + */ +static inline +int armv7m_excp_unmasked(ARMCPU *cpu) +{ +CPUARMState *env = &cpu->env; +int runnable; + +/* find highest (numerically lowest) priority which could + * run based on masks + */ +if (env->daif&PSTATE_F) { /* FAULTMASK */ +runnable = -2; +} else if (env->daif&PSTATE_I) { /* PRIMASK */ +runnable = -1; +} else if (env->v7m.basepri > 0) { +/* BASEPRI==1 -> runnable==-1 (same as PRIMASK==1) */ +runnable = env->v7m.basepri-2; +} else { +runnable = 0x100; /* lower than any possible priority */ +} +/* consider priority of active handler */ +return MIN(runnable, env->v7m.exception_prio-1); +} + /* Interface between CPU and Interrupt controller. */ void armv7m_nvic_set_pending(void *opaque, int irq); -int armv7m_nvic_acknowledge_irq(void *opaque); +void armv7m_nvic_acknowledge_irq(void *opaque); void armv7m_nvic_complete_irq(void *opaque, int irq); /* Interface for defining coprocessor registers. -- 2.1.4
[Qemu-devel] [PATCH 14/18] armv7m: auto-clear FAULTMASK
on return from all exceptions other than NMI Signed-off-by: Michael Davidsaver --- target-arm/helper.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/target-arm/helper.c b/target-arm/helper.c index 5be09b8..83af528 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -5379,8 +5379,13 @@ static void do_v7m_exception_exit(CPUARMState *env) uint32_t xpsr; type = env->regs[15]; -if (env->v7m.exception != 0) +if (env->v7m.exception != ARMV7M_EXCP_NMI) { +/* Auto-clear FAULTMASK on return from other than NMI */ +env->daif &= ~PSTATE_F; +} +if (env->v7m.exception != 0) { armv7m_nvic_complete_irq(env->nvic, env->v7m.exception); +} /* Switch to the target stack. */ switch_v7m_sp(env, (type & 4) != 0); -- 2.1.4
[Qemu-devel] [PATCH 10/18] armv7m: NVIC initialization
Signed-off-by: Michael Davidsaver --- hw/intc/armv7m_nvic.c | 107 -- 1 file changed, 51 insertions(+), 56 deletions(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index c860b36..8eaf677 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -827,7 +827,7 @@ int nvic_post_load(void *opaque, int version_id) set_prio(s, i, s->vectors[i].raw_prio); } -nvic_irq_update(s, highest_runnable_prio(s->cpu)); +nvic_irq_update(s, 0); return 0; } @@ -883,66 +883,66 @@ static const VMStateDescription vmstate_nvic = { } }; +static Property props_nvic[] = { +DEFINE_PROP_UINT32("num-irq", nvic_state, num_irq, 64), +DEFINE_PROP_END_OF_LIST() +}; + static void armv7m_nvic_reset(DeviceState *dev) { nvic_state *s = NVIC(dev); -NVICClass *nc = NVIC_GET_CLASS(s); -nc->parent_reset(dev); -/* Common GIC reset resets to disabled; the NVIC doesn't have - * per-CPU interfaces so mark our non-existent CPU interface - * as enabled by default, and with a priority mask which allows - * all interrupts through. - */ -s->gic.cpu_ctlr[0] = GICC_CTLR_EN_GRP0; -s->gic.priority_mask[0] = 0x100; -/* The NVIC as a whole is always enabled. */ -s->gic.ctlr = 1; + +s->vectors[ARMV7M_EXCP_RESET].enabled = 1; +s->vectors[ARMV7M_EXCP_NMI].enabled = 1; +s->vectors[ARMV7M_EXCP_HARD].enabled = 1; +s->vectors[ARMV7M_EXCP_SVC].enabled = 1; +s->vectors[ARMV7M_EXCP_DEBUG].enabled = 1; +s->vectors[ARMV7M_EXCP_PENDSV].enabled = 1; + +s->vectors[ARMV7M_EXCP_RESET].prio_group = -3; +s->vectors[ARMV7M_EXCP_NMI].prio_group = -2; +s->vectors[ARMV7M_EXCP_HARD].prio_group = -1; + systick_reset(s); } static void armv7m_nvic_realize(DeviceState *dev, Error **errp) { nvic_state *s = NVIC(dev); -NVICClass *nc = NVIC_GET_CLASS(s); -Error *local_err = NULL; - -/* The NVIC always has only one CPU */ -s->gic.num_cpu = 1; -/* Tell the common code we're an NVIC */ -s->gic.revision = 0x; -s->num_irq = s->gic.num_irq; -nc->parent_realize(dev, &local_err); -if (local_err) { -error_propagate(errp, local_err); + +/* evil hack to get ARMCPU* ahead of time */ +assert(cpus.tqh_first); +assert(!CPU_NEXT(cpus.tqh_first)); +s->cpu = ARM_CPU(cpus.tqh_first); +assert(s->cpu); + +if (s->num_irq > NVIC_MAX_IRQ) { +error_setg(errp, TYPE_NVIC " num_irq too large"); return; } -gic_init_irqs_and_distributor(&s->gic); -/* The NVIC and system controller register area looks like this: - * 0..0xff : system control registers, including systick - * 0x100..0xcff : GIC-like registers - * 0xd00..0xfff : system control registers - * We use overlaying to put the GIC like registers - * over the top of the system control register region. - */ -memory_region_init(&s->container, OBJECT(s), "nvic", 0x1000); -/* The system register region goes at the bottom of the priority - * stack as it covers the whole page. + +qdev_init_gpio_in(dev, set_irq_level, s->num_irq); + +s->num_irq += 16; /* include space for internal exception vectors */ + +/* The NVIC and system controller register area starts at 0xe000e000 + * and looks like this: + * 0x004 - ICTR + * 0x010 - 0x1c - systick + * 0x100..0x7ec - NVIC + * 0x7f0..0xcff - Reserved + * 0xd00..0xd3c - SCS registers + * 0xd40..0xeff - Reserved or Not implemented + * 0xf00 - STIR */ -memory_region_init_io(&s->sysregmem, OBJECT(s), &nvic_sysreg_ops, s, + +memory_region_init_io(&s->iomem, OBJECT(s), &nvic_sysreg_ops, s, "nvic_sysregs", 0x1000); -memory_region_add_subregion(&s->container, 0, &s->sysregmem); -/* Alias the GIC region so we can get only the section of it - * we need, and layer it on top of the system register region. - */ -memory_region_init_alias(&s->gic_iomem_alias, OBJECT(s), - "nvic-gic", &s->gic.iomem, - 0x100, 0xc00); -memory_region_add_subregion_overlap(&s->container, 0x100, -&s->gic_iomem_alias, 1); + /* Map the whole thing into system memory at the location required * by the v7M architecture. */ -memory_region_add_subregion(get_system_memory(), 0xe000e000, &s->container); +memory_region_add_subregion(get_system_memory(), 0xe000e000, &s->iomem); s->systick.timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, systick_timer_tick, s); } @@ -954,36 +954,31 @@ static void armv7m_nvic_instance_init(Object *obj) * any us
[Qemu-devel] [PATCH 17/18] armv7m: implement CCR
Implement Configuration and Control register. Handle STACKALIGN and USERSETMPEND bits. Signed-off-by: Michael Davidsaver --- hw/intc/armv7m_nvic.c | 15 +++ target-arm/cpu.h | 1 + target-arm/helper.c | 8 +++- target-arm/machine.c | 1 + 4 files changed, 16 insertions(+), 9 deletions(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index a75dd3c..ca8c93c 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -474,8 +474,7 @@ static uint32_t nvic_readl(nvic_state *s, uint32_t offset) /* TODO: Implement SLEEPONEXIT. */ return 0; case 0xd14: /* Configuration Control. */ -/* TODO: Implement Configuration Control bits. */ -return 0; +return cpu->env.v7m.ccr; case 0xd24: /* System Handler Status. */ val = 0; if (s->vectors[ARMV7M_EXCP_MEM].active) val |= (1 << 0); @@ -619,9 +618,17 @@ static void nvic_writel(nvic_state *s, uint32_t offset, uint32_t value) } break; case 0xd10: /* System Control. */ -case 0xd14: /* Configuration Control. */ /* TODO: Implement control registers. */ -qemu_log_mask(LOG_UNIMP, "NVIC: SCR and CCR unimplemented\n"); +qemu_log_mask(LOG_UNIMP, "NVIC: SCR unimplemented\n"); +break; +case 0xd14: /* Configuration Control. */ +value &= 0x31b; +if (value&0x118) { +qemu_log_mask(LOG_UNIMP, "CCR unimplemented bits" + " BFHFNMIGN, DIV_0_TRP, UNALIGN_TRP"); +value &= ~0x118; +} +cpu->env.v7m.ccr = value; break; case 0xd24: /* System Handler Control. */ /* TODO: Real hardware allows you to set/clear the active bits diff --git a/target-arm/cpu.h b/target-arm/cpu.h index 72b0b32..90ccdcd 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -396,6 +396,7 @@ typedef struct CPUARMState { uint32_t vecbase; uint32_t basepri; uint32_t control; +uint32_t ccr; /* Configuration and Control */ uint32_t cfsr; /* Configurable Fault Status */ uint32_t hfsr; /* HardFault Status */ int current_sp; diff --git a/target-arm/helper.c b/target-arm/helper.c index 3993f77..402bfc5 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -5412,7 +5412,7 @@ static void do_v7m_exception_exit(CPUARMState *env) break; case 0x9: /* Return to Thread mode w/ Main stack */ case 0xd: /* Return to Thread mode w/ Process stack */ -if (env->v7m.exception != 0) { +if ((env->v7m.exception != 0) && !(env->v7m.ccr&1)) { /* Attempt to return to Thread mode * from nested handler while NONBASETHRDENA not set. */ @@ -5564,10 +5564,8 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) qemu_log_mask(CPU_LOG_INT, "... as %d\n", env->v7m.exception); -/* Align stack pointer. */ -/* ??? Should only do this if Configuration Control Register - STACKALIGN bit is set. */ -if (env->regs[13] & 4) { +/* Align stack pointer (STACKALIGN) */ +if (env->v7m.ccr&(1<<9)) { env->regs[13] -= 4; xpsr |= 0x200; } diff --git a/target-arm/machine.c b/target-arm/machine.c index d7c2034..e8b710d 100644 --- a/target-arm/machine.c +++ b/target-arm/machine.c @@ -100,6 +100,7 @@ static const VMStateDescription vmstate_m = { VMSTATE_UINT32(env.v7m.vecbase, ARMCPU), VMSTATE_UINT32(env.v7m.basepri, ARMCPU), VMSTATE_UINT32(env.v7m.control, ARMCPU), +VMSTATE_UINT32(env.v7m.ccr, ARMCPU), VMSTATE_UINT32(env.v7m.cfsr, ARMCPU), VMSTATE_UINT32(env.v7m.hfsr, ARMCPU), VMSTATE_INT32(env.v7m.current_sp, ARMCPU), -- 2.1.4
[Qemu-devel] [PATCH 13/18] armv7m: implement CFSR and HFSR
Add the Configurable and Hard Fault Status registers. Note undefined instructions and escalations Signed-off-by: Michael Davidsaver --- hw/intc/armv7m_nvic.c | 10 +++--- target-arm/cpu.h | 2 ++ target-arm/helper.c | 1 + target-arm/machine.c | 6 -- 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 8eaf677..734f6f8 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -287,6 +287,7 @@ void armv7m_nvic_set_pending(void *opaque, int irq) } I = &s->vectors[irq]; +s->cpu->env.v7m.hfsr |= 1<<30; /* FORCED */ DPRINTF(0, "Escalate %d to %d\n", oldirq, irq); } } @@ -488,10 +489,9 @@ static uint32_t nvic_readl(nvic_state *s, uint32_t offset) if (s->vectors[ARMV7M_EXCP_USAGE].enabled) val |= (1 << 18); return val; case 0xd28: /* Configurable Fault Status. */ -/* TODO: Implement Fault Status. */ -qemu_log_mask(LOG_UNIMP, "Configurable Fault Status unimplemented\n"); -return 0; +return cpu->env.v7m.cfsr; case 0xd2c: /* Hard Fault Status. */ +return cpu->env.v7m.hfsr; case 0xd30: /* Debug Fault Status. */ case 0xd34: /* Mem Manage Address. */ case 0xd38: /* Bus Fault Address. */ @@ -629,7 +629,11 @@ static void nvic_writel(nvic_state *s, uint32_t offset, uint32_t value) */ break; case 0xd28: /* Configurable Fault Status. */ +cpu->env.v7m.cfsr &= ~value; /* W1C */ +break; case 0xd2c: /* Hard Fault Status. */ +cpu->env.v7m.hfsr &= ~value; /* W1C */ +break; case 0xd30: /* Debug Fault Status. */ case 0xd34: /* Mem Manage Address. */ case 0xd38: /* Bus Fault Address. */ diff --git a/target-arm/cpu.h b/target-arm/cpu.h index 29d89ce..e98bca0 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -396,6 +396,8 @@ typedef struct CPUARMState { uint32_t vecbase; uint32_t basepri; uint32_t control; +uint32_t cfsr; /* Configurable Fault Status */ +uint32_t hfsr; /* HardFault Status */ int current_sp; int exception; int exception_prio; diff --git a/target-arm/helper.c b/target-arm/helper.c index 2541890..5be09b8 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -5436,6 +5436,7 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) switch (cs->exception_index) { case EXCP_UDEF: armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE); +env->v7m.cfsr |= 1<<16; /* UNDEFINSTR */ break; case EXCP_SWI: /* The PC already points to the next instruction. */ diff --git a/target-arm/machine.c b/target-arm/machine.c index 36a0d15..d7c2034 100644 --- a/target-arm/machine.c +++ b/target-arm/machine.c @@ -92,14 +92,16 @@ static bool m_needed(void *opaque) static const VMStateDescription vmstate_m = { .name = "cpu/m", -.version_id = 1, -.minimum_version_id = 1, +.version_id = 2, +.minimum_version_id = 2, .needed = m_needed, .fields = (VMStateField[]) { VMSTATE_UINT32(env.v7m.other_sp, ARMCPU), VMSTATE_UINT32(env.v7m.vecbase, ARMCPU), VMSTATE_UINT32(env.v7m.basepri, ARMCPU), VMSTATE_UINT32(env.v7m.control, ARMCPU), +VMSTATE_UINT32(env.v7m.cfsr, ARMCPU), +VMSTATE_UINT32(env.v7m.hfsr, ARMCPU), VMSTATE_INT32(env.v7m.current_sp, ARMCPU), VMSTATE_INT32(env.v7m.exception, ARMCPU), VMSTATE_END_OF_LIST() -- 2.1.4
[Qemu-devel] [PATCH 16/18] armv7m: check exception return consistency
Detect use of reserved exception return codes and return to thread mode from nested exception handler. Also check consistency between NVIC and CPU wrt. the active exception. Signed-off-by: Michael Davidsaver --- hw/intc/armv7m_nvic.c | 7 +++- target-arm/cpu.h | 2 +- target-arm/helper.c | 97 ++- 3 files changed, 96 insertions(+), 10 deletions(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 734f6f8..a75dd3c 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -342,7 +342,7 @@ void armv7m_nvic_acknowledge_irq(void *opaque) assert(env->v7m.exception > 0); /* spurious exception? */ } -void armv7m_nvic_complete_irq(void *opaque, int irq) +bool armv7m_nvic_complete_irq(void *opaque, int irq) { nvic_state *s = (nvic_state *)opaque; vec_info *I; @@ -352,12 +352,17 @@ void armv7m_nvic_complete_irq(void *opaque, int irq) I = &s->vectors[irq]; +if (!I->active) { +return true; +} + I->active = 0; I->pending = I->level; assert(!I->level || irq >= 16); nvic_irq_update(s, 1); DPRINTF(0, "EOI %d\n", irq); +return false; } /* Only called for external interrupt (vector>=16) */ diff --git a/target-arm/cpu.h b/target-arm/cpu.h index e98bca0..72b0b32 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -1063,7 +1063,7 @@ int armv7m_excp_unmasked(ARMCPU *cpu) /* Interface between CPU and Interrupt controller. */ void armv7m_nvic_set_pending(void *opaque, int irq); void armv7m_nvic_acknowledge_irq(void *opaque); -void armv7m_nvic_complete_irq(void *opaque, int irq); +bool armv7m_nvic_complete_irq(void *opaque, int irq); /* Interface for defining coprocessor registers. * Registers are defined in tables of arm_cp_reginfo structs diff --git a/target-arm/helper.c b/target-arm/helper.c index 83af528..3993f77 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -5375,18 +5375,65 @@ static void switch_v7m_sp(CPUARMState *env, int process) static void do_v7m_exception_exit(CPUARMState *env) { +unsigned ufault = 0; uint32_t type; uint32_t xpsr; +uint32_t nvic_active; + +if (env->v7m.exception == 0) { +hw_error("Return from exception w/o active exception. Logic error."); +} -type = env->regs[15]; if (env->v7m.exception != ARMV7M_EXCP_NMI) { /* Auto-clear FAULTMASK on return from other than NMI */ env->daif &= ~PSTATE_F; } -if (env->v7m.exception != 0) { -armv7m_nvic_complete_irq(env->nvic, env->v7m.exception); + +if (armv7m_nvic_complete_irq(env->nvic, env->v7m.exception)) { +qemu_log_mask(LOG_GUEST_ERROR, "Requesting return from exception " + "from inactive exception %d\n", + env->v7m.exception); +ufault = 1; +} +/* env->v7m.exception and friends updated by armv7m_nvic_complete_irq() */ + +type = env->regs[15]&0xf; +/* QEMU seems to clear the LSB at some point. */ +type |= 1; + +switch (type) { +case 0x1: /* Return to Handler mode */ +if (env->v7m.exception == 0) { +qemu_log_mask(LOG_GUEST_ERROR, "Requesting return from exception " + "to Handler mode not allowed at base level of " + "activation"); +ufault = 1; +} +break; +case 0x9: /* Return to Thread mode w/ Main stack */ +case 0xd: /* Return to Thread mode w/ Process stack */ +if (env->v7m.exception != 0) { +/* Attempt to return to Thread mode + * from nested handler while NONBASETHRDENA not set. + */ +qemu_log_mask(LOG_GUEST_ERROR, "Nested exception return to %d w/" + " Thread mode while NONBASETHRDENA not set\n", + env->v7m.exception); +ufault = 1; +} +break; +default: +qemu_log_mask(LOG_GUEST_ERROR, "Exception return w/ reserved code" + " %02x\n", (unsigned)type); +ufault = 1; } +/* TODO? if ufault==1 ARM calls for entering exception handler + * directly w/o popping stack. + * We pop anyway since the active UsageFault will push on entry + * which should happen before execution resumes? + */ + /* Switch to the target stack. */ switch_v7m_sp(env, (type & 4) != 0); /* Pop registers. */ @@ -5409,15 +5456,49 @@ static void do_v7m_exception_exit(CPUARMState *env) env->regs[15] &= ~1U; } xpsr = v7m_pop(env); +nvic_active = env->v7m.exception; xpsr_write(env, xpsr, 0xfdff); + /* Undo stack alignment. */ if (xpsr &am
Re: [Qemu-devel] [Qemu-arm] [PATCH] target-arm: Priority masking with basepri on v7m
On 11/18/2015 02:41 PM, Peter Maydell wrote: > For a big patchset like this the easiest thing is to just ask > Michael if he has a public git repo with the patches in (I've > cc'd him). https://github.com/mdavidsaver/qemu/tree/fixirq The posted patches were https://github.com/qemu/qemu/compare/c4a7bf54e588ff2de9fba0898b686156251b2063...dd7fc89e3c70b1e0074efa41e3ca89f5cee43054 This branch also has more recent changesets which weren't posted. Some of this will be posted in future (MPU stuff), and some should just be ignored (qtest fumbling).
Re: [Qemu-devel] [PATCH 00/18] Fix exception handling and msr/mrs access
On 11/20/2015 08:59 AM, Peter Maydell wrote: > I think I've now done that at least for the earlier patches. > There are probably some other finer details that I'll get to > in a later round of patch review but hopefully you have enough > to do some of the fixes and restructuring of this patchset for v2 Thanks for the comments. I'll be addressing them individually and follow that with another patch set.
Re: [Qemu-devel] [PATCH 01/18] armv7m: MRS/MSR handle unprivileged access
On 11/17/2015 12:09 PM, Peter Maydell wrote: > On 9 November 2015 at 01:11, Michael Davidsaver wrote: >> The MRS and MSR instruction handling isn't checking >> the current permission level. >> >> Signed-off-by: Michael Davidsaver >> --- >> target-arm/helper.c | 79 >> + >> 1 file changed, 37 insertions(+), 42 deletions(-) > > This patch looks good overall, but there's one style nit: > >> +case 0 ... 7: /* xPSR sub-fields */ >> +mask = 0; >> +if ((reg&1) && el) { > > you want spaces around operators, so "reg & 1" here and elsewhere. Would be nice if checkpatch.pl caught these, but I understand that this would be quite difficult to do well. I've tried to catch this with grep and sort through the false positives. I think I got them all. > It would also be good to mention in the commit message the > other things this patch is fixing: > * privileged attempts to write EPSR should do nothing > * accessing an unknown special register now triggers a >guest-error warning rather than aborting QEMU Will do.
Re: [Qemu-devel] [PATCH 03/18] armv7m: Complain about incorrect exception table entries.
On 11/17/2015 12:20 PM, Peter Maydell wrote: > This one's not really correct, I'm afraid (though the spec-mandated > behaviour is a bit subtle). I've dropped this patch.
Re: [Qemu-devel] [PATCH 04/18] armv7m: Explicit error for bad vector table
On 11/17/2015 12:33 PM, Peter Maydell wrote: > On 9 November 2015 at 01:11, Michael Davidsaver wrote: >> Give an explicit error and abort when a load >> from VECBASE fails. Otherwise would likely >> jump to 0, which for v7-m holds the reset stack >> pointer address. >> >> Signed-off-by: Michael Davidsaver >> --- >> target-arm/helper.c | 12 +++- >> 1 file changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/target-arm/helper.c b/target-arm/helper.c >> index 4178400..1d7ac43 100644 >> --- a/target-arm/helper.c >> +++ b/target-arm/helper.c >> @@ -5496,7 +5496,17 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) >> /* Clear IT bits */ >> env->condexec_bits = 0; >> env->regs[14] = lr; >> -addr = ldl_phys(cs->as, env->v7m.vecbase + env->v7m.exception * 4); >> +{ >> +MemTxResult result; >> +addr = address_space_ldl(cs->as, >> + env->v7m.vecbase + env->v7m.exception * 4, >> + MEMTXATTRS_UNSPECIFIED, &result); >> +if (result != MEMTX_OK) { >> +cpu_abort(cs, "Failed to read from exception vector table " >> + "entry %08x\n", >> + env->v7m.vecbase + env->v7m.exception * 4); >> +} >> +} > > The behaviour on a failed vector table read is actually architecturally > specified: we should take a nested exception (escalated to HardFault). > If it happens while we're trying to take a HardFault in the first place > then we go into Lockup (where the CPU sits around repeatedly trying > to execute an instruction at 0xFFFE; it is technically possible > to get back out of Lockup by taking an NMI or a system reset). > > That said, trying to get nested exceptions and priority escalation > right is fairly involved, and implementing lockup is both involved > and an exercise in pointlessness. So I think this code is an > improvement overall. This is my thinking as well. One point against it is that abort() is inconvenient when using '-gdb'. I'm not sure if there is something else which could be done (cpu halt?). > I would suggest some small changes, though: > > (1) factor this out into its own function, something like: > static uint32_t v7m_read_vector(CPUARMState *env, int excnum) > so the calling code can just do >addr = v7m_read_vector(env, env->v7m.exception); > (2) use a local variable for "env->v7m.vecbase + excnum * 4" > rather than calculating it twice Done.
Re: [Qemu-devel] [PATCH 05/18] armv7m: expand NVIC state
On 11/17/2015 01:10 PM, Peter Maydell wrote: > On 9 November 2015 at 01:11, Michael Davidsaver wrote: >> Expand the NVIC to fully support -M priorities and masking. >> Doesn't use GIC code. >> >> Move some state to ARMCPU to allow calculation of exception masking. >> >> Add storage for PRIGROUP to configure group/sub-group split. >> Track group and sub-group in separate fields for quick comparison. >> Mix in vector # with sub-group as per tie breaking rules. >> >> NVIC now derives directly from SysBusDevice, and >> struct NVICClass is eliminated. >> >> Also add DPRINTF() macro. >> >> Signed-off-by: Michael Davidsaver > > This patch doesn't compile, because you've removed the definition of > NVICClass, NVIC_CLASS, etc, but not their uses. A patchset needs to > compile after every patch in it, not just at the end when all patches > are applied. You'll need to rearrange your changes between patches > a bit. In the next rev. I've rearranged things so that each patches compiles. At least according to 'git rebase -i -x make', so not a full rebuilt. This does mean that the big block of changes to the NVIC are now almost entirely in one patch as I couldn't see how to split them up given that the nvic_state structure is changed so much.
Re: [Qemu-devel] [PATCH 06/18] armv7m: new NVIC utility functions
On 11/20/2015 08:25 AM, Peter Maydell wrote: > On 9 November 2015 at 01:11, Michael Davidsaver wrote: >> Internal functions for operations previously done >> by GIC internals. >> >> nvic_irq_update() recalculates highest pending/active >> exceptions. >> >> armv7m_nvic_set_pending() include exception escalation >> logic. >> >> armv7m_nvic_acknowledge_irq() and nvic_irq_update() >> update ARMCPU fields. > > Hi; I have a lot of review comments on this patch set, but that's > really because v7M exception logic is pretty complicated and > our current code is a long way away from correct. You might > find it helpful to separate out "restructure the NVIC code > into its own device" into separate patches from "and now add > functionality like exception escalation", perhaps. I'd certainly find this helpful :) I'm just not sure how to accomplish this and still make every patch compile. >> Signed-off-by: Michael Davidsaver >> --- >> hw/intc/armv7m_nvic.c | 250 >> +++--- >> 1 file changed, 235 insertions(+), 15 deletions(-) >> >> diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c >> index 487a09a..ebb4d4e 100644 >> --- a/hw/intc/armv7m_nvic.c >> +++ b/hw/intc/armv7m_nvic.c >> @@ -140,36 +140,256 @@ static void systick_reset(nvic_state *s) >> timer_del(s->systick.timer); >> } >> >> -/* The external routines use the hardware vector numbering, ie. the first >> - IRQ is #16. The internal GIC routines use #32 as the first IRQ. */ >> +/* caller must call nvic_irq_update() after this */ >> +static >> +void set_prio(nvic_state *s, unsigned irq, uint8_t prio) >> +{ >> +unsigned submask = (1<<(s->prigroup+1))-1; >> + >> +assert(irq > 3); /* only use for configurable prios */ >> +assert(irq < NVIC_MAX_VECTORS); >> + >> +s->vectors[irq].raw_prio = prio; >> +s->vectors[irq].prio_group = (prio>>(s->prigroup+1)); >> +s->vectors[irq].prio_sub = irq + (prio&submask)*NVIC_MAX_VECTORS; > > Precalculating the group priority seems a bit odd, since it > will change later anyway if the guest writes to PRIGROUP. I've kept the pre-calculation as the alternative comparison code is no simpler when tie breaking with exception number is done. >> + >> +DPRINTF(0, "Set %u priority grp %d sub %u\n", irq, >> +s->vectors[irq].prio_group, s->vectors[irq].prio_sub); >> +} >> + >> +/* recompute highest pending */ >> +static >> +void nvic_irq_update(nvic_state *s, int update_active) >> +{ >> +unsigned i; >> +int lvl; >> +CPUARMState *env = &s->cpu->env; >> +int16_t act_group = 0x100, pend_group = 0x100; >> +uint16_t act_sub = 0, pend_sub = 0; >> +uint16_t act_irq = 0, pend_irq = 0; >> + >> +/* find highest priority */ >> +for (i = 1; i < s->num_irq; i++) { >> +vec_info *I = &s->vectors[i]; > > Minor style issue: we prefer lower case for variable names, > and CamelCase for structure type names (see CODING_STYLE). > So this would be VecInfo *vec; or something similar. Done. >> + >> +DPRINTF(2, " VECT %d %d:%u\n", i, I->prio_group, I->prio_sub); >> + >> +if (I->active && ((I->prio_group < act_group) >> +|| (I->prio_group == act_group && I->prio_sub < act_sub))) > > Because the group priority and sub priority are just two contiguous > parts of the raw priority, you get the same effect here by just > doing a comparison on the raw value: "I->raw_prio < act_raw_prio". Not quite since it's possible that I->raw_prio == act_raw_prio. As I read the ARM this situation should be avoided by using the exception number to break the tie. This way no two priorities are ever equal. This is the reason that act_sub is "irq + (prio&submask)*NVIC_MAX_VECTORS". > Note however that the running priority is actually only the > group priority part (see the pseudocode ExecutionPriority function) > and so you want something more like: > > highestpri = 0x100; > for (each vector) { > if (vector->active && vector->raw_prio < highestpri) { > highestpri = vector->raw_prio & prigroup_mask; > act_sub = vector->raw_prio & ~prigroup_mask; // if you need it > } > } > > (this is why it's not worth precalculating the group and >
Re: [Qemu-devel] [PATCH 09/18] armv7m: NVIC update vmstate
On 11/17/2015 12:58 PM, Peter Maydell wrote: > On 9 November 2015 at 01:11, Michael Davidsaver wrote: >> Signed-off-by: Michael Davidsaver >> --- >> hw/intc/armv7m_nvic.c | 64 >> +-- >> 1 file changed, 62 insertions(+), 2 deletions(-) >> >> diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c >> index 3b10dee..c860b36 100644 >> --- a/hw/intc/armv7m_nvic.c >> +++ b/hw/intc/armv7m_nvic.c >> @@ -810,15 +810,75 @@ static const MemoryRegionOps nvic_sysreg_ops = { >> .endianness = DEVICE_NATIVE_ENDIAN, >> }; >> >> +static >> +int nvic_post_load(void *opaque, int version_id) >> +{ >> +nvic_state *s = opaque; >> +unsigned i; >> + >> +/* evil hack to get ARMCPU* ahead of time */ >> +assert(cpus.tqh_first); >> +assert(!CPU_NEXT(cpus.tqh_first)); >> +s->cpu = ARM_CPU(cpus.tqh_first); >> +assert(s->cpu); > > Why do we need to do this? By the time we get to loading > state into the system, s->cpu should already have been set. Just so, removed. >> + >> +/* recalculate priorities */ >> +for (i = 4; i < s->num_irq; i++) { >> +set_prio(s, i, s->vectors[i].raw_prio); >> +} >> + >> +nvic_irq_update(s, highest_runnable_prio(s->cpu)); >> + >> +return 0; >> +} >> + >> +static >> +int vec_info_get(QEMUFile *f, void *pv, size_t size) >> +{ >> +vec_info *I = pv; >> +I->prio_sub = qemu_get_be16(f); >> +I->prio_group = qemu_get_byte(f); >> +I->raw_prio = qemu_get_ubyte(f); >> +I->enabled = qemu_get_ubyte(f); >> +I->pending = qemu_get_ubyte(f); >> +I->active = qemu_get_ubyte(f); >> +I->level = qemu_get_ubyte(f); >> +return 0; >> +} >> + >> +static >> +void vec_info_put(QEMUFile *f, void *pv, size_t size) >> +{ >> +vec_info *I = pv; >> +qemu_put_be16(f, I->prio_sub); >> +qemu_put_byte(f, I->prio_group); >> +qemu_put_ubyte(f, I->raw_prio); >> +qemu_put_ubyte(f, I->enabled); >> +qemu_put_ubyte(f, I->pending); >> +qemu_put_ubyte(f, I->active); >> +qemu_put_ubyte(f, I->level); >> +} >> + >> +static const VMStateInfo vmstate_info = { >> +.name = "armv7m_nvic_info", >> +.get = vec_info_get, >> +.put = vec_info_put, >> +}; > > I don't think there's any need to use get and put functions here: > better to define a VMStateDescripton for the struct vec_info, > and then you can just refer to that from the main vmstate_nvic > description. (hw/audio/pl041.c has some examples of this.) Fixed >> + >> static const VMStateDescription vmstate_nvic = { >> .name = "armv7m_nvic", >> -.version_id = 1, >> -.minimum_version_id = 1, >> +.version_id = 2, >> +.minimum_version_id = 2, >> +.post_load = &nvic_post_load, >> .fields = (VMStateField[]) { >> +VMSTATE_ARRAY(vectors, nvic_state, NVIC_MAX_VECTORS, 0, >> + vmstate_info, vec_info), >> +VMSTATE_UINT8(prigroup, nvic_state), >> VMSTATE_UINT32(systick.control, nvic_state), >> VMSTATE_UINT32(systick.reload, nvic_state), >> VMSTATE_INT64(systick.tick, nvic_state), >> VMSTATE_TIMER_PTR(systick.timer, nvic_state), >> +VMSTATE_UINT32(num_irq, nvic_state), >> VMSTATE_END_OF_LIST() >> } >> }; > > Ideally the VMState should be added in the same patches that > add new fields or structures, rather than in its own patch > at the end. I'll try to do this.
Re: [Qemu-devel] [PATCH 11/18] armv7m: fix I and F flag handling
On 11/20/2015 08:47 AM, Peter Maydell wrote: > On 9 November 2015 at 01:11, Michael Davidsaver wrote: >> Despite having the same notation, these bits >> have completely different meaning than -AR. >> >> Add armv7m_excp_unmasked() >> to calculate the currently runable exception priority >> taking into account masks and active handlers. >> Use this in conjunction with the pending exception >> priority to determine if the pending exception >> can interrupt execution. > > This function is used by code added in earlier patches in > this series, so this patch needs to be moved earlier in the > series, or those patches won't compile. Should be fixed. >> Signed-off-by: Michael Davidsaver >> --- >> target-arm/cpu.c | 26 +++--- >> target-arm/cpu.h | 27 ++- >> 2 files changed, 33 insertions(+), 20 deletions(-) >> >> diff --git a/target-arm/cpu.c b/target-arm/cpu.c >> index be026bc..5d03117 100644 >> --- a/target-arm/cpu.c >> +++ b/target-arm/cpu.c >> @@ -173,6 +173,8 @@ static void arm_cpu_reset(CPUState *s) >> uint32_t initial_pc; /* Loaded from 0x4 */ >> uint8_t *rom; >> >> +env->v7m.exception_prio = env->v7m.pending_prio = 0x100; >> + >> env->daif &= ~PSTATE_I; >> rom = rom_ptr(0); >> if (rom) { >> @@ -301,29 +303,15 @@ static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, >> int interrupt_request) >> { >> CPUClass *cc = CPU_GET_CLASS(cs); >> ARMCPU *cpu = ARM_CPU(cs); >> -CPUARMState *env = &cpu->env; >> bool ret = false; >> >> - >> -if (interrupt_request & CPU_INTERRUPT_FIQ >> -&& !(env->daif & PSTATE_F)) { >> -cs->exception_index = EXCP_FIQ; >> -cc->do_interrupt(cs); >> -ret = true; >> -} >> -/* ARMv7-M interrupt return works by loading a magic value >> - * into the PC. On real hardware the load causes the >> - * return to occur. The qemu implementation performs the >> - * jump normally, then does the exception return when the >> - * CPU tries to execute code at the magic address. >> - * This will cause the magic PC value to be pushed to >> - * the stack if an interrupt occurred at the wrong time. >> - * We avoid this by disabling interrupts when >> - * pc contains a magic address. > > This (removing this comment and the checks for the magic address) > seem to be part of a separate change [probably the one in > "armv7m: Undo armv7m.hack"] and shouldn't be in this patch. Relocated. >> +/* ARMv7-M interrupt masking works differently than -A or -R. >> + * There is no FIQ/IRQ distinction. >> + * Instead of masking interrupt sources, the I and F bits >> + * (along with basepri) mask certain exception priority levels. >> */ >> if (interrupt_request & CPU_INTERRUPT_HARD >> -&& !(env->daif & PSTATE_I) >> -&& (env->regs[15] < 0xfff0)) { >> +&& (armv7m_excp_unmasked(cpu) >= cpu->env.v7m.pending_prio)) { >> cs->exception_index = EXCP_IRQ; >> cc->do_interrupt(cs); >> ret = true; >> diff --git a/target-arm/cpu.h b/target-arm/cpu.h >> index c193fbb..29d89ce 100644 >> --- a/target-arm/cpu.h >> +++ b/target-arm/cpu.h >> @@ -1033,9 +1033,34 @@ void arm_cpu_list(FILE *f, fprintf_function >> cpu_fprintf); >> uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t excp_idx, >> uint32_t cur_el, bool secure); >> >> +/* @returns highest (numerically lowest) unmasked exception priority >> + */ >> +static inline >> +int armv7m_excp_unmasked(ARMCPU *cpu) > > What this is really calculating is the current execution > priority (running priority) of the CPU, so I think a better > name would be armv7m_current_exec_priority() or > armv7m_current_priority() or armv7m_running_priority() or similar. Now armv7m_excp_running_prio() >> +{ >> +CPUARMState *env = &cpu->env; >> +int runnable; >> + >> +/* find highest (numerically lowest) priority which could >> + * run based on masks >> + */ >> +if (env->daif&PSTATE_F) { /* FAULTMASK */ > > Style issue -- operands should have spaces around them. > >> +runnable = -2; > > These all seem to be off by one: FAULTMASK sets the > runn
[Qemu-devel] [PATCH v2 04/26] armv7m: additional cpu state for exception handling
Track priorities and highest active and pending exception. Also the highest pending exception for faster exception handler entry. The pending exception information will be re-calculated on load, so no additional vmstate tracking is needed. --- target-arm/cpu.c | 2 ++ target-arm/cpu.h | 3 +++ 2 files changed, 5 insertions(+) diff --git a/target-arm/cpu.c b/target-arm/cpu.c index 728854f..5348028 100644 --- a/target-arm/cpu.c +++ b/target-arm/cpu.c @@ -173,6 +173,8 @@ static void arm_cpu_reset(CPUState *s) uint32_t initial_pc; /* Loaded from 0x4 */ uint8_t *rom; +env->v7m.exception_prio = env->v7m.pending_prio = 0x100; + env->daif &= ~PSTATE_I; rom = rom_ptr(0); if (rom) { diff --git a/target-arm/cpu.h b/target-arm/cpu.h index 815fef8..c193fbb 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -398,6 +398,9 @@ typedef struct CPUARMState { uint32_t control; int current_sp; int exception; +int exception_prio; +unsigned pending; +int pending_prio; } v7m; /* Information associated with an exception about to be taken: -- 2.1.4
[Qemu-devel] [PATCH v2 02/26] armv7m: Undo armv7m.hack
Add CPU unassigned access handler in place of special MemoryRegion to catch exception returns. The unassigned handler will signal other faults as either prefetch or data exceptions, with the FSR code 0x8 to distinguish them from memory translation faults (0xd). Future code will make use of this distinction when deciding to raise BusFault or MemManage exceptions. --- hw/arm/armv7m.c | 8 target-arm/cpu.c | 32 +--- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c index a80d2ad..68146de 100644 --- a/hw/arm/armv7m.c +++ b/hw/arm/armv7m.c @@ -176,7 +176,6 @@ DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq, uint64_t entry; uint64_t lowaddr; int big_endian; -MemoryRegion *hack = g_new(MemoryRegion, 1); if (cpu_model == NULL) { cpu_model = "cortex-m3"; @@ -221,13 +220,6 @@ DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq, } } -/* Hack to map an additional page of ram at the top of the address - space. This stops qemu complaining about executing code outside RAM - when returning from an exception. */ -memory_region_init_ram(hack, NULL, "armv7m.hack", 0x1000, &error_fatal); -vmstate_register_ram_global(hack); -memory_region_add_subregion(system_memory, 0xf000, hack); - qemu_register_reset(armv7m_reset, cpu); return nvic; } diff --git a/target-arm/cpu.c b/target-arm/cpu.c index 30739fc..728854f 100644 --- a/target-arm/cpu.c +++ b/target-arm/cpu.c @@ -280,6 +280,25 @@ bool arm_cpu_exec_interrupt(CPUState *cs, int interrupt_request) } #if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64) +static void arm_v7m_unassigned_access(CPUState *cpu, hwaddr addr, + bool is_write, bool is_exec, int opaque, + unsigned size) +{ +ARMCPU *arm = ARM_CPU(cpu); +CPUARMState *env = &arm->env; + +env->exception.vaddress = addr; +env->exception.fsr = is_write ? 0x808 : 0x8; /* Precise External Abort */ +if (env->v7m.exception != 0 && addr >= 0xfff0 && !is_write) { +cpu->exception_index = EXCP_EXCEPTION_EXIT; +} else if (is_exec) { +cpu->exception_index = EXCP_PREFETCH_ABORT; +} else { +cpu->exception_index = EXCP_DATA_ABORT; +} +cpu_loop_exit(cpu); +} + static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int interrupt_request) { CPUClass *cc = CPU_GET_CLASS(cs); @@ -294,19 +313,9 @@ static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int interrupt_request) cc->do_interrupt(cs); ret = true; } -/* ARMv7-M interrupt return works by loading a magic value - * into the PC. On real hardware the load causes the - * return to occur. The qemu implementation performs the - * jump normally, then does the exception return when the - * CPU tries to execute code at the magic address. - * This will cause the magic PC value to be pushed to - * the stack if an interrupt occurred at the wrong time. - * We avoid this by disabling interrupts when - * pc contains a magic address. - */ if (interrupt_request & CPU_INTERRUPT_HARD && !(env->daif & PSTATE_I) -&& (env->regs[15] < 0xfff0)) { +) { cs->exception_index = EXCP_IRQ; cc->do_interrupt(cs); ret = true; @@ -909,6 +918,7 @@ static void arm_v7m_class_init(ObjectClass *oc, void *data) cc->do_interrupt = arm_v7m_cpu_do_interrupt; #endif +cc->do_unassigned_access = arm_v7m_unassigned_access; cc->cpu_exec_interrupt = arm_v7m_cpu_exec_interrupt; } -- 2.1.4
[Qemu-devel] [PATCH v2 07/26] armv7m: simpler/faster exception start
No need to bounce through EXCP_IRQ handling for non-IRQ exceptions. just update CPU state directly. --- target-arm/helper.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/target-arm/helper.c b/target-arm/helper.c index 7b76f32..4490b74 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -5451,23 +5451,21 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) /* For exceptions we just mark as pending on the NVIC, and let that handle it. */ -/* TODO: Need to escalate if the current priority is higher than the - one we're raising. */ switch (cs->exception_index) { case EXCP_UDEF: armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE); -return; +break; case EXCP_SWI: /* The PC already points to the next instruction. */ armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_SVC); -return; +break; case EXCP_PREFETCH_ABORT: case EXCP_DATA_ABORT: /* TODO: if we implemented the MPU registers, this is where we * should set the MMFAR, etc from exception.fsr and exception.vaddress. */ armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_MEM); -return; +break; case EXCP_BKPT: if (semihosting_enabled()) { int nr; @@ -5484,7 +5482,6 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_DEBUG); return; case EXCP_IRQ: -env->v7m.exception = armv7m_nvic_acknowledge_irq(env->nvic); break; case EXCP_EXCEPTION_EXIT: do_v7m_exception_exit(env); @@ -5494,6 +5491,10 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) return; /* Never happens. Keep compiler happy. */ } +armv7m_nvic_acknowledge_irq(env->nvic); + +qemu_log_mask(CPU_LOG_INT, "... as %d\n", env->v7m.exception); + /* Align stack pointer. */ /* ??? Should only do this if Configuration Control Register STACKALIGN bit is set. */ -- 2.1.4
[Qemu-devel] [PATCH v2 01/26] armv7m: MRS/MSR handle unprivileged access
The MRS and MSR instruction handling isn't checking the current permission level. Prevent privlaged from changing writing EPSR fields. Access to unknown/undefined special registers not fatal (read 0, write ignored) w/ guest error message. --- target-arm/helper.c | 79 + 1 file changed, 37 insertions(+), 42 deletions(-) diff --git a/target-arm/helper.c b/target-arm/helper.c index afc4163..2c631e3 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -7375,23 +7375,32 @@ uint32_t HELPER(get_r13_banked)(CPUARMState *env, uint32_t mode) uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg) { -ARMCPU *cpu = arm_env_get_cpu(env); +uint32_t mask; +unsigned el = arm_current_el(env); + +/* First handle registers which unprivileged can read */ + +switch (reg) { +case 0 ... 7: /* xPSR sub-fields */ +mask = 0; +if ((reg & 1) && el) { +mask |= 0x01ff; /* IPSR (unpriv. reads as zero) */ +} +if (!(reg & 4)) { +mask |= 0xf800; /* APSR */ +} +/* EPSR reads as zero */ +return xpsr_read(env) & mask; +break; +case 20: /* CONTROL */ +return env->v7m.control; +} + +if (el == 0) { +return 0; /* unprivileged reads others as zero */ +} switch (reg) { -case 0: /* APSR */ -return xpsr_read(env) & 0xf800; -case 1: /* IAPSR */ -return xpsr_read(env) & 0xf80001ff; -case 2: /* EAPSR */ -return xpsr_read(env) & 0xff00fc00; -case 3: /* xPSR */ -return xpsr_read(env) & 0xff00fdff; -case 5: /* IPSR */ -return xpsr_read(env) & 0x01ff; -case 6: /* EPSR */ -return xpsr_read(env) & 0x0700fc00; -case 7: /* IEPSR */ -return xpsr_read(env) & 0x0700edff; case 8: /* MSP */ return env->v7m.current_sp ? env->v7m.other_sp : env->regs[13]; case 9: /* PSP */ @@ -7403,40 +7412,26 @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg) return env->v7m.basepri; case 19: /* FAULTMASK */ return (env->daif & PSTATE_F) != 0; -case 20: /* CONTROL */ -return env->v7m.control; default: -/* ??? For debugging only. */ -cpu_abort(CPU(cpu), "Unimplemented system register read (%d)\n", reg); +qemu_log_mask(LOG_GUEST_ERROR, "Attempt to read unknown special" + " register %d\n", reg); return 0; } } void HELPER(v7m_msr)(CPUARMState *env, uint32_t reg, uint32_t val) { -ARMCPU *cpu = arm_env_get_cpu(env); +if (arm_current_el(env) == 0 && reg > 7) { +/* only xPSR sub-fields may be written by unprivileged */ +return; +} switch (reg) { -case 0: /* APSR */ -xpsr_write(env, val, 0xf800); -break; -case 1: /* IAPSR */ -xpsr_write(env, val, 0xf800); -break; -case 2: /* EAPSR */ -xpsr_write(env, val, 0xfe00fc00); -break; -case 3: /* xPSR */ -xpsr_write(env, val, 0xfe00fc00); -break; -case 5: /* IPSR */ -/* IPSR bits are readonly. */ -break; -case 6: /* EPSR */ -xpsr_write(env, val, 0x0600fc00); -break; -case 7: /* IEPSR */ -xpsr_write(env, val, 0x0600fc00); +case 0 ... 7: /* xPSR sub-fields */ +/* only APSR is actually writable */ +if (reg & 4) { +xpsr_write(env, val, 0xf800); /* APSR */ +} break; case 8: /* MSP */ if (env->v7m.current_sp) @@ -7477,8 +7472,8 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t reg, uint32_t val) switch_v7m_sp(env, (val & 2) != 0); break; default: -/* ??? For debugging only. */ -cpu_abort(CPU(cpu), "Unimplemented system register write (%d)\n", reg); +qemu_log_mask(LOG_GUEST_ERROR, "Attempt to write unknown special" + " register %d\n", reg); return; } } -- 2.1.4
[Qemu-devel] [PATCH v2 03/26] armv7m: Explicit error for bad vector table
Give an explicit error and abort when a load from VECBASE fails. Otherwise would likely jump to 0, which for v7-m holds the reset stack pointer address. --- target-arm/helper.c | 21 - 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/target-arm/helper.c b/target-arm/helper.c index 2c631e3..7b76f32 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -5414,6 +5414,25 @@ static void do_v7m_exception_exit(CPUARMState *env) pointer. */ } +static +uint32_t arm_v7m_load_vector(ARMCPU *cpu) + +{ +CPUState *cs = &cpu->parent_obj; +CPUARMState *env = &cpu->env; +MemTxResult result; +hwaddr vec = env->v7m.vecbase + env->v7m.exception * 4; +uint32_t addr; + +addr = address_space_ldl(cs->as, vec, + MEMTXATTRS_UNSPECIFIED, &result); +if (result != MEMTX_OK) { +cpu_abort(cs, "Failed to read from exception vector table " + "entry %08x\n", (unsigned)vec); +} +return addr; +} + void arm_v7m_cpu_do_interrupt(CPUState *cs) { ARMCPU *cpu = ARM_CPU(cs); @@ -5495,7 +5514,7 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) /* Clear IT bits */ env->condexec_bits = 0; env->regs[14] = lr; -addr = ldl_phys(cs->as, env->v7m.vecbase + env->v7m.exception * 4); +addr = arm_v7m_load_vector(cpu); env->regs[15] = addr & 0xfffe; env->thumb = addr & 1; } -- 2.1.4
[Qemu-devel] [PATCH v2 09/26] armv7m: implement CFSR, HFSR, BFAR, and MMFAR
Add the Configurable, HardFault, BusFault and MemManage Status registers. Note undefined instructions, violations, and escalations. No BusFaults are raised at this point. --- hw/intc/armv7m_nvic.c | 28 ++-- target-arm/cpu.h | 4 target-arm/helper.c | 3 +++ target-arm/machine.c | 8 ++-- 4 files changed, 35 insertions(+), 8 deletions(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index ca9bd4c..619c320 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -333,6 +333,7 @@ void armv7m_nvic_set_pending(void *opaque, int irq) irq = ARMV7M_EXCP_HARD; vec = &s->vectors[irq]; +s->cpu->env.v7m.hfsr |= 1<<30; /* FORCED */ DPRINTF(0, "Escalate %d to HardFault\n", oldirq); } } @@ -548,16 +549,20 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset) if (s->vectors[ARMV7M_EXCP_USAGE].enabled) val |= (1 << 18); return val; case 0xd28: /* Configurable Fault Status. */ -/* TODO: Implement Fault Status. */ -qemu_log_mask(LOG_UNIMP, "Configurable Fault Status unimplemented\n"); -return 0; +return cpu->env.v7m.cfsr; case 0xd2c: /* Hard Fault Status. */ +return cpu->env.v7m.hfsr; case 0xd30: /* Debug Fault Status. */ -case 0xd34: /* Mem Manage Address. */ +qemu_log_mask(LOG_UNIMP, "Debug Fault status register unimplemented\n"); +return 0; +case 0xd34: /* MMFAR MemManage Fault Address */ +return cpu->env.v7m.mmfar; case 0xd38: /* Bus Fault Address. */ +return cpu->env.v7m.bfar; case 0xd3c: /* Aux Fault Status. */ /* TODO: Implement fault status registers. */ -qemu_log_mask(LOG_UNIMP, "Fault status registers unimplemented\n"); +qemu_log_mask(LOG_UNIMP, + "Aux Fault status registers unimplemented\n"); return 0; case 0xd40: /* PFR0. */ return 0x0030; @@ -690,13 +695,24 @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value) */ break; case 0xd28: /* Configurable Fault Status. */ +cpu->env.v7m.cfsr &= ~value; /* W1C */ +break; case 0xd2c: /* Hard Fault Status. */ +cpu->env.v7m.hfsr &= ~value; /* W1C */ +break; case 0xd30: /* Debug Fault Status. */ +qemu_log_mask(LOG_UNIMP, + "NVIC: debug fault status register unimplemented\n"); +break; case 0xd34: /* Mem Manage Address. */ +cpu->env.v7m.mmfar = value; +return; case 0xd38: /* Bus Fault Address. */ +cpu->env.v7m.bfar = value; +return; case 0xd3c: /* Aux Fault Status. */ qemu_log_mask(LOG_UNIMP, - "NVIC: fault status registers unimplemented\n"); + "NVIC: Aux fault status registers unimplemented\n"); break; case 0xf00: /* Software Triggered Interrupt Register */ if ((value & 0x1ff) < NVIC_MAX_IRQ) { diff --git a/target-arm/cpu.h b/target-arm/cpu.h index 4b7f78e..4262efc 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -396,6 +396,10 @@ typedef struct CPUARMState { uint32_t vecbase; uint32_t basepri; uint32_t control; +uint32_t cfsr; /* Configurable Fault Status */ +uint32_t hfsr; /* HardFault Status */ +uint32_t mmfar; /* MemManage Fault Address */ +uint32_t bfar; /* BusFault Address */ int current_sp; int exception; int exception_prio; diff --git a/target-arm/helper.c b/target-arm/helper.c index 4490b74..d1ca011 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -5454,6 +5454,7 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) switch (cs->exception_index) { case EXCP_UDEF: armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE); +env->v7m.cfsr |= 1<<16; /* UNDEFINSTR */ break; case EXCP_SWI: /* The PC already points to the next instruction. */ @@ -5465,6 +5466,8 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) * should set the MMFAR, etc from exception.fsr and exception.vaddress. */ armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_MEM); +env->v7m.mmfar = env->exception.vaddress; +env->v7m.cfsr = (1<<1)|(1<<7); /* DACCVIOL and MMARVALID */ break; case EXCP_BKPT: if (semihosting_enabled()) { diff --git a/target-arm/machine.c b/target-arm/machine.c index 36a0d15..14a4882 100644 --- a/target-arm/machine.c +++ b/target-arm/machine.c @@ -92,14 +92,18 @@ static bool m_needed(void *opaque) static const VMStateDescription vmstate_m = { .name = "cpu/m", -.version_id = 1, -.minimum_version_id = 1, +.version_id = 2, +.minimum_version_id = 2, .needed = m_needed, .fields = (VMStateField[]) { VMSTATE_UINT32(env.v7m.other_sp, ARMCPU), VMSTATE_UIN
[Qemu-devel] [PATCH v2 00/26] armv7m: exception handling, MPU, and more
All, Second revision of ARMv7-M exception handling patchset, which now adds MPU support (as well as can be done). Parts of this series are informed by the previous work of Alex Zuepke. This time I have access to a EK-TM4C1294XL eval board (cortex-m4f), and have done some cross-checks using test programs of my own[1]. There are differences in some corner cases, but mostly agreement (certainly better than before). This series attempts to fix the exception masking and priority escalation behavour, and in the process removes the dependence of the NVIC on the shared GIC code. It also fills out some of the missing fault reporting registers. The CCR register is also added, mainly to implement the STACKALIGN bit. I've also made an attempt on USERSETMPEND. BFHFNMIGN, DIV_0_TRP, UNALIGN_TRP remain unimplemnted. Additional checks are added to do_v7m_exception_exit() to capture the faults which should arise when the guest stack gets out of sync with the CPU's state. In cross-checking with a real device I also found that the behavour of CONTROL bit 1 wasn't as spec'd in exception handlers. I also "discovered" that many v7-M targets don't implement all 8 bits of the priority registers (the TM4C1294 only has 3). So I've added a Property to allow this to be set appropriately. This led me to make some changes to allow board code to actually set this Property, which entailed splitting armv7m_init() in two parts, and associated changes to the stellaris and stm32f205 boards. I'm not completely happy with the level of compatibility with the MPU. For my own purposes it's good enough as I can make special builds. However, the additional alignment and size restrictions imposed by the common TLB and 1024 page size will probably break most unmodified guests using the MPU. I can't see any way around this short of changes to the TLB code, or a seperate build with TARGET_PAGE_BITS==5. I'm not inclined to undertake either. Should this part of the series be dropped? Michael [1] https://github.com/mdavidsaver/baremetal/tree/qemutest Michael Davidsaver (26): armv7m: MRS/MSR handle unprivileged access armv7m: Undo armv7m.hack armv7m: Explicit error for bad vector table armv7m: additional cpu state for exception handling armv7m: add armv7m_excp_running_prio() armv7m: fix I and F flag handling armv7m: simpler/faster exception start armv7m: rewrite NVIC armv7m: implement CFSR, HFSR, BFAR, and MMFAR armv7m: auto-clear FAULTMASK arm: gic: Remove references to NVIC armv7m: check exception return consistency armv7m: implement CCR armv7m: prevent unprivileged write to STIR armv7m: add MPU to cortex-m3 and cortex-m4 armv7m: add some mpu debugging prints armv7m: mpu background miss is perm fault armv7m: update base region policy armv7m: mpu not allowed to map exception return codes armv7m: observable initial register state armv7m: CONTROL<1> handling armv7m: priority field mask qom: add cpu_generic_init_unrealized() armv7m: split armv7m_init in two parts armv7m: remove extra cpu_reset() armv7m: decide whether faults are MemManage or BusFault hw/arm/armv7m.c | 60 ++- hw/arm/stellaris.c |7 +- hw/arm/stm32f205_soc.c |6 +- hw/intc/arm_gic.c| 14 +- hw/intc/arm_gic_common.c | 23 +- hw/intc/armv7m_nvic.c| 1037 +- hw/intc/gic_internal.h |7 +- include/hw/arm/arm.h |4 +- include/qom/cpu.h| 12 + qom/cpu.c| 23 +- target-arm/cpu-qom.h |6 + target-arm/cpu.c | 61 ++- target-arm/cpu.h | 17 +- target-arm/helper.c | 346 target-arm/machine.c | 11 +- 15 files changed, 1252 insertions(+), 382 deletions(-) -- 2.1.4
[Qemu-devel] [PATCH v2 06/26] armv7m: fix I and F flag handling
Despite having the same notation, these bits have completely different meaning than -AR. Use armv7m_excp_running_prio() and the highest pending exception priority to determine if the pending exception can interrupt preempt. --- target-arm/cpu.c | 16 ++-- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/target-arm/cpu.c b/target-arm/cpu.c index 5348028..6e3b251 100644 --- a/target-arm/cpu.c +++ b/target-arm/cpu.c @@ -305,19 +305,15 @@ static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int interrupt_request) { CPUClass *cc = CPU_GET_CLASS(cs); ARMCPU *cpu = ARM_CPU(cs); -CPUARMState *env = &cpu->env; bool ret = false; - -if (interrupt_request & CPU_INTERRUPT_FIQ -&& !(env->daif & PSTATE_F)) { -cs->exception_index = EXCP_FIQ; -cc->do_interrupt(cs); -ret = true; -} +/* ARMv7-M interrupt masking works differently than -A or -R. + * There is no FIQ/IRQ distinction. + * Instead of masking interrupt sources, the I and F bits + * (along with basepri) mask certain exception priority levels. + */ if (interrupt_request & CPU_INTERRUPT_HARD -&& !(env->daif & PSTATE_I) -) { +&& (armv7m_excp_running_prio(cpu) > cpu->env.v7m.pending_prio)) { cs->exception_index = EXCP_IRQ; cc->do_interrupt(cs); ret = true; -- 2.1.4
[Qemu-devel] [PATCH v2 05/26] armv7m: add armv7m_excp_running_prio()
Implements v7m exception priority algorithm using FAULTMASK, PRIMASK, BASEPRI, and the highest priority active exception. The number returned is the current execution priority which may be in the range [-2,0x7f] when an exception is active or 0x100 when no exception is active. --- hw/intc/armv7m_nvic.c | 25 + target-arm/cpu.h | 1 + 2 files changed, 26 insertions(+) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 6fc167e..0145ca7 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -18,6 +18,8 @@ typedef struct { GICState gic; +uint8_t prigroup; + struct { uint32_t control; uint32_t reload; @@ -116,6 +118,29 @@ static void systick_reset(nvic_state *s) timer_del(s->systick.timer); } +/* @returns the active (running) exception priority. + *only a higher (numerically lower) priority can preempt. + */ +int armv7m_excp_running_prio(ARMCPU *cpu) +{ +CPUARMState *env = &cpu->env; +nvic_state *s = env->nvic; +int running; + +if (env->daif & PSTATE_F) { /* FAULTMASK */ +running = -1; +} else if (env->daif & PSTATE_I) { /* PRIMASK */ +running = 0; +} else if (env->v7m.basepri > 0) { +/* BASEPRI==1 -> masks [1,255] (not same as PRIMASK==1) */ +running = env->v7m.basepri >> (s->prigroup+1); +} else { +running = 0x100; /* lower than any possible priority */ +} +/* consider priority of active handler */ +return MIN(running, env->v7m.exception_prio); +} + /* The external routines use the hardware vector numbering, ie. the first IRQ is #16. The internal GIC routines use #32 as the first IRQ. */ void armv7m_nvic_set_pending(void *opaque, int irq) diff --git a/target-arm/cpu.h b/target-arm/cpu.h index c193fbb..e2d9e75 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -1034,6 +1034,7 @@ uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t excp_idx, uint32_t cur_el, bool secure); /* Interface between CPU and Interrupt controller. */ +int armv7m_excp_running_prio(ARMCPU *cpu); void armv7m_nvic_set_pending(void *opaque, int irq); int armv7m_nvic_acknowledge_irq(void *opaque); void armv7m_nvic_complete_irq(void *opaque, int irq); -- 2.1.4
[Qemu-devel] [PATCH v2 12/26] armv7m: check exception return consistency
Detect use of reserved exception return codes and return to thread mode from nested exception handler. Also check consistency between NVIC and CPU wrt. the active exception. --- hw/intc/armv7m_nvic.c | 7 +++- target-arm/cpu.h | 2 +- target-arm/helper.c | 95 ++- 3 files changed, 94 insertions(+), 10 deletions(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 619c320..7d261ae 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -396,7 +396,7 @@ void armv7m_nvic_acknowledge_irq(void *opaque) assert(env->v7m.exception > 0); /* spurious exception? */ } -void armv7m_nvic_complete_irq(void *opaque, int irq) +bool armv7m_nvic_complete_irq(void *opaque, int irq) { NVICState *s = (NVICState *)opaque; VecInfo *vec; @@ -406,12 +406,17 @@ void armv7m_nvic_complete_irq(void *opaque, int irq) vec = &s->vectors[irq]; +if (!vec->active) { +return true; +} + vec->active = 0; vec->pending = vec->level; assert(!vec->level || irq >= 16); nvic_irq_update(s); DPRINTF(0, "EOI %d\n", irq); +return false; } /* Only called for external interrupt (vector>=16) */ diff --git a/target-arm/cpu.h b/target-arm/cpu.h index 4262efc..b98ef89 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -1043,7 +1043,7 @@ void armv7m_nvic_set_pending(void *opaque, int irq); bool armv7m_nvic_is_active(void *opaque, int irq); int armv7m_nvic_get_active_prio(void *opaque); void armv7m_nvic_acknowledge_irq(void *opaque); -void armv7m_nvic_complete_irq(void *opaque, int irq); +bool armv7m_nvic_complete_irq(void *opaque, int irq); /* Interface for defining coprocessor registers. * Registers are defined in tables of arm_cp_reginfo structs diff --git a/target-arm/helper.c b/target-arm/helper.c index b6ec761..f7e496d 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -5375,18 +5375,65 @@ static void switch_v7m_sp(CPUARMState *env, int process) static void do_v7m_exception_exit(CPUARMState *env) { +unsigned ufault = 0; uint32_t type; uint32_t xpsr; -type = env->regs[15]; +if (env->v7m.exception == 0) { +hw_error("Return from exception w/o active exception. Logic error."); +} + if (env->v7m.exception != ARMV7M_EXCP_NMI) { /* Auto-clear FAULTMASK on return from other than NMI */ env->daif &= ~PSTATE_F; } -if (env->v7m.exception != 0) { -armv7m_nvic_complete_irq(env->nvic, env->v7m.exception); + +if (armv7m_nvic_complete_irq(env->nvic, env->v7m.exception)) { +qemu_log_mask(LOG_GUEST_ERROR, "Requesting return from exception " + "from inactive exception %d\n", + env->v7m.exception); +ufault = 1; +} +env->v7m.exception = -42; /* spoil, will be unstacked below */ +env->v7m.exception_prio = armv7m_nvic_get_active_prio(env->nvic); + +type = env->regs[15] & 0xf; +/* QEMU seems to clear the LSB at some point. */ +type |= 1; + +switch (type) { +case 0x1: /* Return to Handler mode */ +if (env->v7m.exception_prio == 0x100) { +qemu_log_mask(LOG_GUEST_ERROR, "Requesting return from exception " + "to Handler mode not allowed at base level of " + "activation"); +ufault = 1; +} +break; +case 0x9: /* Return to Thread mode w/ Main stack */ +case 0xd: /* Return to Thread mode w/ Process stack */ +if (env->v7m.exception_prio != 0x100) { +/* Attempt to return to Thread mode + * from nested handler while NONBASETHRDENA not set. + */ +qemu_log_mask(LOG_GUEST_ERROR, "Nested exception return to %d w/" + " Thread mode while NONBASETHRDENA not set\n", + env->v7m.exception); +ufault = 1; +} +break; +default: +qemu_log_mask(LOG_GUEST_ERROR, "Exception return w/ reserved code" + " %02x\n", (unsigned)type); +ufault = 1; } +/* TODO? if ufault==1 ARM calls for entering exception handler + * directly w/o popping stack. + * We pop anyway since the active UsageFault will push on entry + * which should happen before execution resumes? + */ + /* Switch to the target stack. */ switch_v7m_sp(env, (type & 4) != 0); /* Pop registers. */ @@ -5409,14 +5456,46 @@ static void do_v7m_exception_exit(CPUARMState *env) } xpsr = v7m_pop(env); xpsr_write(env, xpsr, 0xfdff); + +assert(env->v7m.exception!=-42); + /* Undo stack alignment. */ if (xpsr & 0x200) env->regs[13] |= 4; -/* ??? The exception return type specifies Thread/Handler mode. However - this is also implied by the xPSR value. Not sure what to do - if there is a mismatch. */ -/* ??? Likewise
[Qemu-devel] [PATCH v2 11/26] arm: gic: Remove references to NVIC
armv7m_nvic.c no longer relies on the GIC. Remove REV_NVIC and conditionals which use it. --- hw/intc/arm_gic.c| 14 +++--- hw/intc/arm_gic_common.c | 23 --- hw/intc/gic_internal.h | 7 ++- 3 files changed, 17 insertions(+), 27 deletions(-) diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index 13e297d..2b09cd9 100644 --- a/hw/intc/arm_gic.c +++ b/hw/intc/arm_gic.c @@ -182,7 +182,7 @@ static void gic_set_irq(void *opaque, int irq, int level) return; } -if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) { +if (s->revision == REV_11MPCORE) { gic_set_irq_11mpcore(s, irq, level, cm, target); } else { gic_set_irq_generic(s, irq, level, cm, target); @@ -333,7 +333,7 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu, MemTxAttrs attrs) return 1023; } -if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) { +if (s->revision == REV_11MPCORE) { /* Clear pending flags for both level and edge triggered interrupts. * Level triggered IRQs will be reasserted once they become inactive. */ @@ -512,7 +512,7 @@ void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs) return; /* No active IRQ. */ } -if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) { +if (s->revision == REV_11MPCORE) { /* Mark level triggered interrupts as pending if they are still raised. */ if (!GIC_TEST_EDGE_TRIGGER(irq) && GIC_TEST_ENABLED(irq, cm) @@ -670,7 +670,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs) } else if (offset < 0xf10) { goto bad_reg; } else if (offset < 0xf30) { -if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) { +if (s->revision == REV_11MPCORE) { goto bad_reg; } @@ -881,7 +881,7 @@ static void gic_dist_writeb(void *opaque, hwaddr offset, if (irq < GIC_NR_SGIS) value |= 0xaa; for (i = 0; i < 4; i++) { -if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) { +if (s->revision == REV_11MPCORE) { if (value & (1 << (i * 2))) { GIC_SET_MODEL(irq + i); } else { @@ -899,7 +899,7 @@ static void gic_dist_writeb(void *opaque, hwaddr offset, goto bad_reg; } else if (offset < 0xf20) { /* GICD_CPENDSGIRn */ -if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) { +if (s->revision == REV_11MPCORE) { goto bad_reg; } irq = (offset - 0xf10); @@ -910,7 +910,7 @@ static void gic_dist_writeb(void *opaque, hwaddr offset, } } else if (offset < 0xf30) { /* GICD_SPENDSGIRn */ -if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) { +if (s->revision == REV_11MPCORE) { goto bad_reg; } irq = (offset - 0xf20); diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c index 9c82b97..4987047 100644 --- a/hw/intc/arm_gic_common.c +++ b/hw/intc/arm_gic_common.c @@ -97,9 +97,7 @@ void gic_init_irqs_and_mmio(GICState *s, qemu_irq_handler handler, * [N+32..N+63] PPIs for CPU 1 * ... */ -if (s->revision != REV_NVIC) { -i += (GIC_INTERNAL * s->num_cpu); -} +i += (GIC_INTERNAL * s->num_cpu); qdev_init_gpio_in(DEVICE(s), handler, i); for (i = 0; i < s->num_cpu; i++) { @@ -113,16 +111,12 @@ void gic_init_irqs_and_mmio(GICState *s, qemu_irq_handler handler, memory_region_init_io(&s->iomem, OBJECT(s), ops, s, "gic_dist", 0x1000); sysbus_init_mmio(sbd, &s->iomem); -if (s->revision != REV_NVIC) { -/* This is the main CPU interface "for this core". It is always - * present because it is required by both software emulation and KVM. - * NVIC is not handled here because its CPU interface is different, - * neither it can use KVM. - */ -memory_region_init_io(&s->cpuiomem[0], OBJECT(s), ops ? &ops[1] : NULL, - s, "gic_cpu", s->revision == 2 ? 0x1000 : 0x100); -sysbus_init_mmio(sbd, &s->cpuiomem[0]); -} +/* This is the main CPU interface "for this core". It is always + * present because it is required by both software emulation and KVM. + */ +memory_region_init_io(&s->cpuiomem[0], OBJECT(s), ops ? &ops[1] : NULL, + s, "gic_cpu", s->revision == 2 ? 0x1000 : 0x100); +sysbus_init_mmio(sbd, &s->cpuiomem[0]); } static void arm_gic_common_realize(DeviceState *dev, Error **errp) @@ -154,7 +148,7 @@ static void arm_gic_common_realize(DeviceState *dev, Error **errp) } if (s->security_extn && -(s->revision == REV_11MPCORE || s->revision == REV_NVIC)) { +(s->revision == REV_11MPCORE)) { error_setg(errp, "this GIC revision does not implement
[Qemu-devel] [PATCH v2 10/26] armv7m: auto-clear FAULTMASK
on return from all exceptions other than NMI --- target-arm/helper.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/target-arm/helper.c b/target-arm/helper.c index d1ca011..b6ec761 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -5379,8 +5379,13 @@ static void do_v7m_exception_exit(CPUARMState *env) uint32_t xpsr; type = env->regs[15]; -if (env->v7m.exception != 0) +if (env->v7m.exception != ARMV7M_EXCP_NMI) { +/* Auto-clear FAULTMASK on return from other than NMI */ +env->daif &= ~PSTATE_F; +} +if (env->v7m.exception != 0) { armv7m_nvic_complete_irq(env->nvic, env->v7m.exception); +} /* Switch to the target stack. */ switch_v7m_sp(env, (type & 4) != 0); -- 2.1.4
[Qemu-devel] [PATCH v2 20/26] armv7m: observable initial register state
At least for TI TM4C1294. LR==-1 XPSR==0 PRIMASK, FAULTMASK, and BASEPRI all cleared so exception handlers are unmasked. STKALIGN set. --- target-arm/cpu.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/target-arm/cpu.c b/target-arm/cpu.c index 1fa1f96..8b85888 100644 --- a/target-arm/cpu.c +++ b/target-arm/cpu.c @@ -175,7 +175,10 @@ static void arm_cpu_reset(CPUState *s) env->v7m.exception_prio = env->v7m.pending_prio = 0x100; -env->daif &= ~PSTATE_I; +env->v7m.ccr = 1<<9; /* STKALIGN */ + +env->daif &= ~(PSTATE_I|PSTATE_F); +env->ZF = 1; rom = rom_ptr(0); if (rom) { /* Address zero is covered by ROM which hasn't yet been @@ -194,6 +197,7 @@ static void arm_cpu_reset(CPUState *s) } env->regs[13] = initial_msp & 0xFFFC; +env->regs[14] = 0x; env->regs[15] = initial_pc & ~1; env->thumb = initial_pc & 1; } -- 2.1.4
[Qemu-devel] [PATCH v2 18/26] armv7m: update base region policy
Update MPU background policy as per ARM. Main changes are preventing writes to ROM and no-exec for device regions. --- target-arm/helper.c | 35 +++ 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/target-arm/helper.c b/target-arm/helper.c index e73f7a6..e42f6d0 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -7062,16 +7062,35 @@ static inline void get_phys_addr_pmsav7_default(CPUARMState *env, ARMMMUIdx mmu_idx, int32_t address, int *prot) { -*prot = PAGE_READ | PAGE_WRITE; -switch (address) { -case 0xF000 ... 0x: -if (regime_sctlr(env, mmu_idx) & SCTLR_V) { /* hivecs execing is ok */ +if (!IS_M(env)) { +*prot = PAGE_READ | PAGE_WRITE; +switch (address) { +case 0xF000 ... 0x: +if (regime_sctlr(env, mmu_idx) & SCTLR_V) { +/* hivecs execing is ok */ +*prot |= PAGE_EXEC; +} +break; +case 0x ... 0x7FFF: *prot |= PAGE_EXEC; +break; +} +} else { +/* ARM specfies XN (PAGE_EXEC) but leaves R/W to implementation. + * Mark ROM as read only since writes would otherwise be ignored. + */ +switch (address) { +case 0 ... 0x1fff: /* ROM */ +*prot = PAGE_READ | PAGE_EXEC; +break; +case 0x2000 ... 0x3fff: /* SRAM */ +case 0x6000 ... 0x7fff: /* RAM */ +case 0x8000 ... 0x9fff: /* RAM */ +*prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; +break; +default: /* Peripheral, 2x Device, and System */ +*prot = PAGE_READ | PAGE_WRITE; } -break; -case 0x ... 0x7FFF: -*prot |= PAGE_EXEC; -break; } } -- 2.1.4
[Qemu-devel] [PATCH v2 14/26] armv7m: prevent unprivileged write to STIR
Prevent unprivileged from writing to the Software Triggered Interrupt register --- hw/intc/armv7m_nvic.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 0f9ca6a..5731146 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -727,7 +727,9 @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value) "NVIC: Aux fault status registers unimplemented\n"); break; case 0xf00: /* Software Triggered Interrupt Register */ -if ((value & 0x1ff) < NVIC_MAX_IRQ) { +/* STIR write allowed if privlaged or USERSETMPEND set */ +if ((arm_current_el(&cpu->env) || (cpu->env.v7m.ccr & 2)) +&& ((value & 0x1ff) < NVIC_MAX_IRQ)) { armv7m_nvic_set_pending(s, (value&0x1ff)+16); } break; -- 2.1.4
[Qemu-devel] [PATCH v2 08/26] armv7m: rewrite NVIC
Expand the NVIC to fully support -M priorities and masking. Doesn't use GIC code. Use PRIGROUP to configure group/sub-group split. Track group and sub-group in separate fields for quick comparison. Mix in vector # with sub-group as per tie breaking rules. NVIC now derives directly from SysBusDevice, and struct NVICClass is eliminated. Also add DPRINTF() macro. Internal functions for operations previously done by GIC internals. nvic_irq_update() recalculates highest pending exception. Update ARMCPU state. armv7m_nvic_set_pending() include exception escalation logic. Replace use of GIC state/functions with new NVIC. Fix RETTOBASE. The polarity is reversed, and it should include internal exceptions. Should be set when # of active exceptions == 1. --- hw/intc/armv7m_nvic.c | 805 ++ target-arm/cpu.h | 4 +- 2 files changed, 622 insertions(+), 187 deletions(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 0145ca7..ca9bd4c 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -13,11 +13,48 @@ #include "hw/sysbus.h" #include "qemu/timer.h" #include "hw/arm/arm.h" +#include "target-arm/cpu.h" #include "exec/address-spaces.h" -#include "gic_internal.h" -typedef struct { -GICState gic; +/*#define DEBUG_NVIC 0 + */ +#ifdef DEBUG_NVIC +#define DPRINTF(LVL, fmt, ...) \ +do { if ((LVL) <= DEBUG_NVIC) { \ +fprintf(stderr, "armv7m_nvic: " fmt , ## __VA_ARGS__); \ +} } while (0) +#else +#define DPRINTF(LVL, fmt, ...) do {} while (0) +#endif + +/* the number of IRQ lines in addition to the 16 internal + * exception vectors. + */ +#define NVIC_MAX_IRQ 496 + +#define NVIC_MAX_VECTORS 512 + +struct VecInfo { +uint16_t prio_sub; /* sub-group priority*512 + exception# */ +int8_t prio_group; /* group priority [-2, 0x7f] */ +uint8_t raw_prio; /* value writen by guest */ +uint8_t enabled; +uint8_t pending; +uint8_t active; +uint8_t level; +/* exceptions <=15 never set level */ +}; +typedef struct VecInfo VecInfo; + +struct NVICState { +/*< private >*/ +SysBusDevice parent_obj; +/*< public >*/ + +ARMCPU *cpu; /* NVIC is so closely tied to the CPU, just keep a ref */ + +VecInfo vectors[NVIC_MAX_VECTORS]; + uint8_t prigroup; struct { @@ -26,34 +63,19 @@ typedef struct { int64_t tick; QEMUTimer *timer; } systick; -MemoryRegion sysregmem; -MemoryRegion gic_iomem_alias; -MemoryRegion container; + +MemoryRegion iomem; /* system control space and NVIC */ + uint32_t num_irq; +qemu_irq excpout; qemu_irq sysresetreq; -} nvic_state; +}; +typedef struct NVICState NVICState; #define TYPE_NVIC "armv7m_nvic" -/** - * NVICClass: - * @parent_reset: the parent class' reset handler. - * - * A model of the v7M NVIC and System Controller - */ -typedef struct NVICClass { -/*< private >*/ -ARMGICClass parent_class; -/*< public >*/ -DeviceRealize parent_realize; -void (*parent_reset)(DeviceState *dev); -} NVICClass; - -#define NVIC_CLASS(klass) \ -OBJECT_CLASS_CHECK(NVICClass, (klass), TYPE_NVIC) -#define NVIC_GET_CLASS(obj) \ -OBJECT_GET_CLASS(NVICClass, (obj), TYPE_NVIC) + #define NVIC(obj) \ -OBJECT_CHECK(nvic_state, (obj), TYPE_NVIC) +OBJECT_CHECK(NVICState, (obj), TYPE_NVIC) static const uint8_t nvic_id[] = { 0x00, 0xb0, 0x1b, 0x00, 0x0d, 0xe0, 0x05, 0xb1 @@ -70,7 +92,7 @@ static const uint8_t nvic_id[] = { int system_clock_scale; /* Conversion factor from qemu timer to SysTick frequencies. */ -static inline int64_t systick_scale(nvic_state *s) +static inline int64_t systick_scale(NVICState *s) { if (s->systick.control & SYSTICK_CLKSOURCE) return system_clock_scale; @@ -78,7 +100,7 @@ static inline int64_t systick_scale(nvic_state *s) return 1000; } -static void systick_reload(nvic_state *s, int reset) +static void systick_reload(NVICState *s, int reset) { /* The Cortex-M3 Devices Generic User Guide says that "When the * ENABLE bit is set to 1, the counter loads the RELOAD value from the @@ -97,7 +119,7 @@ static void systick_reload(nvic_state *s, int reset) static void systick_timer_tick(void * opaque) { -nvic_state *s = (nvic_state *)opaque; +NVICState *s = (NVICState *)opaque; s->systick.control |= SYSTICK_COUNTFLAG; if (s->systick.control & SYSTICK_TICKINT) { /* Trigger the interrupt. */ @@ -110,7 +132,7 @@ static void systick_timer_tick(void * opaque) } } -static void systick_reset(nvic_state *s) +static void systick_reset(NVICState *s) { s->systick.control = 0; s->systick.reload = 0; @@ -118,13 +140,120 @@ static void systick_reset(nvic_state *s) timer_del(s->systick.timer); } +/* caller must call nvic_irq_update() after this */ +static +void set_prio(NVICState *s, unsigned irq, uint8_t prio) +{ +unsigned submask = (1<<(s->prigroup+1))-1; + +assert(irq > 3); /* onl
[Qemu-devel] [PATCH v2 19/26] armv7m: mpu not allowed to map exception return codes
Always pass these through to be caught be by the unassigned handler. --- target-arm/helper.c | 9 + 1 file changed, 9 insertions(+) diff --git a/target-arm/helper.c b/target-arm/helper.c index e42f6d0..a5adf2d 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -7106,6 +7106,15 @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, *phys_ptr = address; *prot = 0; +/* Magic exception codes returns always pass through the MPU + * to be trapped later in arm_v7m_unassigned_access() + */ +if (IS_M(env) && env->v7m.exception != 0 && address >= 0xfff0) { +*prot = PAGE_EXEC; +*fsr = 0; +return false; +} + if (regime_translation_disabled(env, mmu_idx)) { /* MPU disabled */ get_phys_addr_pmsav7_default(env, mmu_idx, address, prot); } else { /* MPU enabled */ -- 2.1.4
[Qemu-devel] [PATCH v2 23/26] qom: add cpu_generic_init_unrealized()
cpu_generic_init() without realized=true. Gives board code an opportunity to change CPU properties. --- include/qom/cpu.h | 12 qom/cpu.c | 23 +-- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/include/qom/cpu.h b/include/qom/cpu.h index 51a1323..9093500 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -489,6 +489,18 @@ ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model); CPUState *cpu_generic_init(const char *typename, const char *cpu_model); /** + * cpu_generic_init_unrealized: + * @typename: The CPU base type. + * @cpu_model: The model string including optional parameters. + * + * Instantiates a CPU, processes optional parameters but does not realize it. + * + * Returns: A #CPUState or %NULL if an error occurred. + */ +CPUState *cpu_generic_init_unrealized(const char *typename, + const char *cpu_model); + +/** * cpu_has_work: * @cpu: The vCPU to check. * diff --git a/qom/cpu.c b/qom/cpu.c index fb80d13..f622fc2 100644 --- a/qom/cpu.c +++ b/qom/cpu.c @@ -42,6 +42,23 @@ bool cpu_exists(int64_t id) CPUState *cpu_generic_init(const char *typename, const char *cpu_model) { +CPUState *cpu = cpu_generic_init_unrealized(typename, cpu_model); +if (cpu) { +Error *err = NULL; +object_property_set_bool(OBJECT(cpu), true, "realized", &err); + +if (err != NULL) { +error_report_err(err); +object_unref(OBJECT(cpu)); +return NULL; +} +} +return cpu; +} + +CPUState *cpu_generic_init_unrealized(const char *typename, + const char *cpu_model) +{ char *str, *name, *featurestr; CPUState *cpu; ObjectClass *oc; @@ -63,13 +80,7 @@ CPUState *cpu_generic_init(const char *typename, const char *cpu_model) featurestr = strtok(NULL, ","); cc->parse_features(cpu, featurestr, &err); g_free(str); -if (err != NULL) { -goto out; -} - -object_property_set_bool(OBJECT(cpu), true, "realized", &err); -out: if (err != NULL) { error_report_err(err); object_unref(OBJECT(cpu)); -- 2.1.4
[Qemu-devel] [PATCH v2 21/26] armv7m: CONTROL<1> handling
The ARM states that this bit indicates the stack being used, which in handler mode is always MSP. Further CONTROL<1>==1 in handler mode is reserved. With the TM4C always CONTROL<1>==0 in handler mode which inconveniently prevents the handler from knowing which stack thread mode was using... This bit is a direct indication of which stack pointer register is "aliased" to r13, so easiest to eliminate the now redundant current_sp field. --- target-arm/cpu.h | 1 - target-arm/helper.c | 20 ++-- target-arm/machine.c | 1 - 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/target-arm/cpu.h b/target-arm/cpu.h index b93f8ae..01c9cdb 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -402,7 +402,6 @@ typedef struct CPUARMState { uint32_t mmfar; /* MemManage Fault Address */ uint32_t bfar; /* BusFault Address */ unsigned mpu_hfnmiena; /* MPU_CTRL not mappable into SCTLR */ -int current_sp; int exception; int exception_prio; unsigned pending; diff --git a/target-arm/helper.c b/target-arm/helper.c index a5adf2d..2661da4 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -5362,14 +5362,14 @@ static uint32_t v7m_pop(CPUARMState *env) } /* Switch to V7M main or process stack pointer. */ -static void switch_v7m_sp(CPUARMState *env, int process) +static void switch_v7m_sp(CPUARMState *env, bool process) { uint32_t tmp; -if (env->v7m.current_sp != process) { +if (!!(env->v7m.control & 2) != process) { tmp = env->v7m.other_sp; env->v7m.other_sp = env->regs[13]; env->regs[13] = tmp; -env->v7m.current_sp = process; +env->v7m.control = (env->v7m.control & ~2) | (process ? 2 : 0); } } @@ -5457,7 +5457,7 @@ static void do_v7m_exception_exit(CPUARMState *env) xpsr = v7m_pop(env); xpsr_write(env, xpsr, 0xfdff); -assert(env->v7m.exception!=-42); +assert(env->v7m.exception != -42); /* Undo stack alignment. */ if (xpsr & 0x200) @@ -5528,7 +5528,7 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) arm_log_exception(cs->exception_index); lr = 0xfff1; -if (env->v7m.current_sp) +if (env->v7m.control & 2) lr |= 4; if (env->v7m.exception == 0) lr |= 8; @@ -7550,9 +7550,9 @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg) switch (reg) { case 8: /* MSP */ -return env->v7m.current_sp ? env->v7m.other_sp : env->regs[13]; +return env->v7m.control & 2 ? env->v7m.other_sp : env->regs[13]; case 9: /* PSP */ -return env->v7m.current_sp ? env->regs[13] : env->v7m.other_sp; +return env->v7m.control & 2 ? env->regs[13] : env->v7m.other_sp; case 16: /* PRIMASK */ return (env->daif & PSTATE_I) != 0; case 17: /* BASEPRI */ @@ -7582,13 +7582,13 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t reg, uint32_t val) } break; case 8: /* MSP */ -if (env->v7m.current_sp) +if (env->v7m.control & 2) env->v7m.other_sp = val; else env->regs[13] = val; break; case 9: /* PSP */ -if (env->v7m.current_sp) +if (env->v7m.control & 2) env->regs[13] = val; else env->v7m.other_sp = val; @@ -7616,8 +7616,8 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t reg, uint32_t val) } break; case 20: /* CONTROL */ -env->v7m.control = val & 3; switch_v7m_sp(env, (val & 2) != 0); +env->v7m.control = (env->v7m.control & ~1) | (val & 1); break; default: qemu_log_mask(LOG_GUEST_ERROR, "Attempt to write unknown special" diff --git a/target-arm/machine.c b/target-arm/machine.c index 8852410..dab1626 100644 --- a/target-arm/machine.c +++ b/target-arm/machine.c @@ -105,7 +105,6 @@ static const VMStateDescription vmstate_m = { VMSTATE_UINT32(env.v7m.hfsr, ARMCPU), VMSTATE_UINT32(env.v7m.mmfar, ARMCPU), VMSTATE_UINT32(env.v7m.bfar, ARMCPU), -VMSTATE_INT32(env.v7m.current_sp, ARMCPU), VMSTATE_UINT32(env.v7m.mpu_hfnmiena, ARMCPU), VMSTATE_INT32(env.v7m.exception, ARMCPU), VMSTATE_END_OF_LIST() -- 2.1.4
[Qemu-devel] [PATCH v2 13/26] armv7m: implement CCR
Implement Configuration and Control register. Handle STACKALIGN and USERSETMPEND bits. --- hw/intc/armv7m_nvic.c | 15 +++ target-arm/cpu.h | 1 + target-arm/helper.c | 8 +++- target-arm/machine.c | 1 + 4 files changed, 16 insertions(+), 9 deletions(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 7d261ae..0f9ca6a 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -534,8 +534,7 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset) /* TODO: Implement SLEEPONEXIT. */ return 0; case 0xd14: /* Configuration Control. */ -/* TODO: Implement Configuration Control bits. */ -return 0; +return cpu->env.v7m.ccr; case 0xd24: /* System Handler Status. */ val = 0; if (s->vectors[ARMV7M_EXCP_MEM].active) val |= (1 << 0); @@ -685,9 +684,17 @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value) } break; case 0xd10: /* System Control. */ -case 0xd14: /* Configuration Control. */ /* TODO: Implement control registers. */ -qemu_log_mask(LOG_UNIMP, "NVIC: SCR and CCR unimplemented\n"); +qemu_log_mask(LOG_UNIMP, "NVIC: SCR unimplemented\n"); +break; +case 0xd14: /* Configuration Control. */ +value &= 0x31b; +if (value & 0x118) { +qemu_log_mask(LOG_UNIMP, "CCR unimplemented bits" + " BFHFNMIGN, DIV_0_TRP, UNALIGN_TRP"); +value &= ~0x118; +} +cpu->env.v7m.ccr = value; break; case 0xd24: /* System Handler Control. */ /* TODO: Real hardware allows you to set/clear the active bits diff --git a/target-arm/cpu.h b/target-arm/cpu.h index b98ef89..4e1b8cf 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -396,6 +396,7 @@ typedef struct CPUARMState { uint32_t vecbase; uint32_t basepri; uint32_t control; +uint32_t ccr; /* Configuration and Control */ uint32_t cfsr; /* Configurable Fault Status */ uint32_t hfsr; /* HardFault Status */ uint32_t mmfar; /* MemManage Fault Address */ diff --git a/target-arm/helper.c b/target-arm/helper.c index f7e496d..17d1ca0 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -5412,7 +5412,7 @@ static void do_v7m_exception_exit(CPUARMState *env) break; case 0x9: /* Return to Thread mode w/ Main stack */ case 0xd: /* Return to Thread mode w/ Process stack */ -if (env->v7m.exception_prio != 0x100) { +if ((env->v7m.exception_prio != 0x100) && !(env->v7m.ccr & 1)) { /* Attempt to return to Thread mode * from nested handler while NONBASETHRDENA not set. */ @@ -5582,10 +5582,8 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) qemu_log_mask(CPU_LOG_INT, "... as %d\n", env->v7m.exception); -/* Align stack pointer. */ -/* ??? Should only do this if Configuration Control Register - STACKALIGN bit is set. */ -if (env->regs[13] & 4) { +/* Align stack pointer (STACKALIGN) */ +if ((env->regs[13] & 4) && (env->v7m.ccr & (1<<9))) { env->regs[13] -= 4; xpsr |= 0x200; } diff --git a/target-arm/machine.c b/target-arm/machine.c index 14a4882..7aee41e 100644 --- a/target-arm/machine.c +++ b/target-arm/machine.c @@ -100,6 +100,7 @@ static const VMStateDescription vmstate_m = { VMSTATE_UINT32(env.v7m.vecbase, ARMCPU), VMSTATE_UINT32(env.v7m.basepri, ARMCPU), VMSTATE_UINT32(env.v7m.control, ARMCPU), +VMSTATE_UINT32(env.v7m.ccr, ARMCPU), VMSTATE_UINT32(env.v7m.cfsr, ARMCPU), VMSTATE_UINT32(env.v7m.hfsr, ARMCPU), VMSTATE_UINT32(env.v7m.mmfar, ARMCPU), -- 2.1.4
[Qemu-devel] [PATCH v2 25/26] armv7m: remove extra cpu_reset()
cpu_reset() is called as a side-effect of realizing the CPU. arm_cpu_reset() calls rom_ptr(0), which expects to find the image mapped. This was happening way before load_*() and was worked around with a second call to cpu_reset(). Now wait to realize until after the image is mapped. --- hw/arm/armv7m.c | 16 +--- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c index fb805fe..41b9596 100644 --- a/hw/arm/armv7m.c +++ b/hw/arm/armv7m.c @@ -159,13 +159,6 @@ static void armv7m_bitband_init(void) /* Board init. */ -static void armv7m_reset(void *opaque) -{ -ARMCPU *cpu = opaque; - -cpu_reset(CPU(cpu)); -} - void armv7m_init(const char *cpu_model) { ARMCPU *cpu; @@ -206,9 +199,6 @@ void armv7m_realize(int mem_size, const char *kernel_filename) uint64_t lowaddr; int big_endian; -qdev_init_nofail(DEVICE(cpu)); -qdev_init_nofail(nvic); - #ifdef TARGET_WORDS_BIGENDIAN big_endian = 1; #else @@ -233,7 +223,11 @@ void armv7m_realize(int mem_size, const char *kernel_filename) } } -qemu_register_reset(armv7m_reset, cpu); +/* Realizing cpu calls cpu_reset(), which must have rom image + * already mapped to find the correct entry point. + */ +qdev_init_nofail(DEVICE(cpu)); +qdev_init_nofail(nvic); } static Property bitband_properties[] = { -- 2.1.4
[Qemu-devel] [PATCH v2 22/26] armv7m: priority field mask
Many v7m CPUs don't implement all of the 8 bits of the priority fields. Typically, only the top N bits are available. Existing practice implies that writes to unimplemented bits will be ignore, and read as zero. This allows a guest to discover the implemented bits by writing 0xff to (eg. basepri). The value read back will have only the available bits set. --- hw/intc/armv7m_nvic.c | 2 ++ target-arm/cpu-qom.h | 6 ++ target-arm/cpu.c | 7 +++ target-arm/helper.c | 6 -- 4 files changed, 19 insertions(+), 2 deletions(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 2b42d9d..e2410a3 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -149,6 +149,8 @@ void set_prio(NVICState *s, unsigned irq, uint8_t prio) assert(irq > 3); /* only use for configurable prios */ assert(irq < NVIC_MAX_VECTORS); +prio &= s->cpu->v7m_priority_mask; + s->vectors[irq].raw_prio = prio; s->vectors[irq].prio_group = (prio>>(s->prigroup+1)); s->vectors[irq].prio_sub = irq + (prio & submask) * NVIC_MAX_VECTORS; diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h index 25fb1ce..79cf591 100644 --- a/target-arm/cpu-qom.h +++ b/target-arm/cpu-qom.h @@ -108,6 +108,12 @@ typedef struct ARMCPU { /* PMSAv7 MPU number of supported regions */ uint32_t pmsav7_dregion; +/* Some v7-M targets don't impliment the full 8 bits + * of the priority fields. Writes to unimplimented + * bits are treated as zero (guest can discover mask). + */ +uint8_t v7m_priority_mask; + /* PSCI conduit used to invoke PSCI methods * 0 - disabled, 1 - smc, 2 - hvc */ diff --git a/target-arm/cpu.c b/target-arm/cpu.c index 8b85888..27cf482 100644 --- a/target-arm/cpu.c +++ b/target-arm/cpu.c @@ -527,6 +527,9 @@ static Property arm_cpu_has_mpu_property = static Property arm_cpu_pmsav7_dregion_property = DEFINE_PROP_UINT32("pmsav7-dregion", ARMCPU, pmsav7_dregion, 16); +static Property armv7m_priority_mask_property = +DEFINE_PROP_UINT8("priority-mask", ARMCPU, v7m_priority_mask, 0xff); + static void arm_cpu_post_init(Object *obj) { ARMCPU *cpu = ARM_CPU(obj); @@ -565,6 +568,10 @@ static void arm_cpu_post_init(Object *obj) } } +if (IS_M(&cpu->env) && arm_feature(&cpu->env, ARM_FEATURE_V7)) { +qdev_property_add_static(DEVICE(obj), &armv7m_priority_mask_property, + &error_abort); +} } static void arm_cpu_finalizefn(Object *obj) diff --git a/target-arm/helper.c b/target-arm/helper.c index 2661da4..c890b3a 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -7569,6 +7569,8 @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg) void HELPER(v7m_msr)(CPUARMState *env, uint32_t reg, uint32_t val) { +ARMCPU *cpu = arm_env_get_cpu(env); + if (arm_current_el(env) == 0 && reg > 7) { /* only xPSR sub-fields may be written by unprivileged */ return; @@ -7601,10 +7603,10 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t reg, uint32_t val) } break; case 17: /* BASEPRI */ -env->v7m.basepri = val & 0xff; +env->v7m.basepri = val & cpu->v7m_priority_mask; break; case 18: /* BASEPRI_MAX */ -val &= 0xff; +val &= cpu->v7m_priority_mask; if (val != 0 && (val < env->v7m.basepri || env->v7m.basepri == 0)) env->v7m.basepri = val; break; -- 2.1.4
[Qemu-devel] [PATCH v2 17/26] armv7m: mpu background miss is perm fault
Set an appropriate FSR code when an access does not match any MPU region, including the background/default. --- target-arm/helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target-arm/helper.c b/target-arm/helper.c index da99825..e73f7a6 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -7163,7 +7163,7 @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, if (cpu->pmsav7_dregion && (is_user || !(regime_sctlr(env, mmu_idx) & SCTLR_BR))) { /* background fault */ -*fsr = 0; +*fsr = 0x00d; /* Permission fault */ qemu_log_mask(CPU_LOG_MMU, "Miss MPU\n"); return true; -- 2.1.4
[Qemu-devel] [PATCH v2 15/26] armv7m: add MPU to cortex-m3 and cortex-m4
The M series MPU is almost the same as the already implemented R series MPU. So use the M series and translate. Primary difference is that a real v7-M MPU is has much relaxed alignment and size requirements for MPU regions (32 bytes) compared with the 1K page size of the QEMU TLB which is shared with all ARM targets. Add MPU feature flag to cortex-m3 and -m4. The v7-R MPU registers don't have a place for HFNMIENA, so add another v7m. field. --- hw/arm/stellaris.c| 11 hw/intc/armv7m_nvic.c | 163 -- target-arm/cpu.c | 2 + target-arm/cpu.h | 1 + target-arm/helper.c | 6 ++ target-arm/machine.c | 1 + 6 files changed, 179 insertions(+), 5 deletions(-) diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c index 0114e0a..7e56f02 100644 --- a/hw/arm/stellaris.c +++ b/hw/arm/stellaris.c @@ -1255,6 +1255,17 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model, qdev_connect_gpio_out_named(nvic, "SYSRESETREQ", 0, qemu_allocate_irq(&do_sys_reset, NULL, 0)); +{ +/* hack to change the number of MPU regions. + * Less of a hack than messing with cpu_model string. + * Safe as long as the number is being reduced. + */ +ARMCPU *cpu = ARM_CPU(qemu_get_cpu(0)); +assert(cpu->pmsav7_dregion>=8); +cpu->pmsav7_dregion = 8; +} + + if (board->dc1 & (1 << 16)) { dev = sysbus_create_varargs(TYPE_STELLARIS_ADC, 0x40038000, qdev_get_gpio_in(nvic, 14), diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 5731146..2b42d9d 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -595,8 +595,67 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset) case 0xd70: /* ISAR4. */ return 0x01310102; /* TODO: Implement debug registers. */ +case 0xd90: /* MPU_TYPE */ +return cpu->has_mpu ? (cpu->pmsav7_dregion<<8) : 0; +break; +case 0xd94: /* MPU_CTRL */ +val = 0; +/* We only use sctlr_el[1] since v7m has only two ELs unpriv. (0) + * and priv. (1). The "controlling" EL is always priv. + */ +if (cpu->env.cp15.sctlr_el[1] & SCTLR_M) { +val |= 1; /* ENABLE */ +} +if (cpu->env.v7m.mpu_hfnmiena) { +val |= 2; /* HFNMIENA */ +} +if (cpu->env.cp15.sctlr_el[1] & SCTLR_BR) { +val |= 4; /* PRIVDEFENA */ +} +return val; +case 0xd98: /* MPU_RNR */ +return cpu->env.cp15.c6_rgnr; +case 0xd9c: /* MPU_RBAR */ +case 0xda4: /* MPU_RBAR_A1 */ +case 0xdac: /* MPU_RBAR_A2 */ +case 0xdb4: /* MPU_RBAR_A3 */ +{ +uint32_t range; +if (offset == 0xd9c) { +range = cpu->env.cp15.c6_rgnr; +} else { +range = (offset - 0xda4)/8; +} + +if (range >= cpu->pmsav7_dregion) { +return 0; +} else { +return (cpu->env.pmsav7.drbar[range] & (0x1f)) | (range & 0xf); +} +} +case 0xda0: /* MPU_RASR */ +case 0xda8: /* MPU_RASR_A1 */ +case 0xdb0: /* MPU_RASR_A2 */ +case 0xdb8: /* MPU_RASR_A3 */ +{ +uint32_t range; + +if (offset == 0xda0) { +range = cpu->env.cp15.c6_rgnr; +} else { +range = (offset - 0xda8)/8; +} + +if (range >= cpu->pmsav7_dregion) { +return 0; +} else { +return ((cpu->env.pmsav7.dracr[range] & 0x)<<16) +| (cpu->env.pmsav7.drsr[range] & 0x); +} +} default: -qemu_log_mask(LOG_GUEST_ERROR, "NVIC: Bad read offset 0x%x\n", offset); +qemu_log_mask(LOG_GUEST_ERROR, + "NVIC: Bad read offset 0x%x\n", offset); return 0; } } @@ -726,11 +785,105 @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value) qemu_log_mask(LOG_UNIMP, "NVIC: Aux fault status registers unimplemented\n"); break; +case 0xd90: /* MPU_TYPE (0xe000ed90) */ +return; /* RO */ +case 0xd94: /* MPU_CTRL */ +{ +if ((value & 3) == 2) { +qemu_log_mask(LOG_GUEST_ERROR, "MPU_CTRL: HFNMIENA and !ENABLE is " + "UNPREDICTABLE\n"); +/* we choice to ignore HFNMIENA when the MPU + * is not enabled. + */ +value &= ~2; +} +if (value & 1) { +cpu->env.cp15.sctlr_el[1] |= SCTLR_M; +} else { +cpu->env.cp15.sctlr_el[1] &= ~SCTLR_M; +} +cpu->env.v7m.mpu_hfnmiena = !!(value & 2); +if (value & 4) { +cpu->env.cp15.sctlr_el[1] |= SCTLR_BR; +} else { +cpu->env.cp15.sctlr_el[1] &= ~SCTLR_BR; +} +tlb_flush(CPU(cpu), 1); +} +break; +case 0xd98: /*
[Qemu-devel] [PATCH v2 16/26] armv7m: add some mpu debugging prints
Provide some more "-d mmu" related to the MPU translation process as an aid in debugging guest MPU configurations. Helpful since our MPU resolution is limited to the ARM7-AR page size. --- target-arm/helper.c | 19 ++- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/target-arm/helper.c b/target-arm/helper.c index 589aa54..da99825 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -7110,7 +7110,7 @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, if (base & rmask) { qemu_log_mask(LOG_GUEST_ERROR, "DRBAR %" PRIx32 " misaligned " - "to DRSR region size, mask = %" PRIx32, + "to DRSR region size, mask = %" PRIx32 "\n", base, rmask); continue; } @@ -7148,9 +7148,9 @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, } } if (rsize < TARGET_PAGE_BITS) { -qemu_log_mask(LOG_UNIMP, "No support for MPU (sub)region" +qemu_log_mask(LOG_UNIMP, "No support for MPU[%u] (sub)region " "alignment of %" PRIu32 " bits. Minimum is %d\n", - rsize, TARGET_PAGE_BITS); + n, rsize, TARGET_PAGE_BITS); continue; } if (srdis) { @@ -7164,11 +7164,14 @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, (is_user || !(regime_sctlr(env, mmu_idx) & SCTLR_BR))) { /* background fault */ *fsr = 0; + +qemu_log_mask(CPU_LOG_MMU, "Miss MPU\n"); return true; } get_phys_addr_pmsav7_default(env, mmu_idx, address, prot); } else { /* a MPU hit! */ uint32_t ap = extract32(env->pmsav7.dracr[n], 8, 3); +qemu_log_mask(CPU_LOG_MMU, "Hit MPU %u AP %08x\n", n, (unsigned)ap); if (is_user) { /* User mode AP bit decoding */ switch (ap) { @@ -7382,9 +7385,15 @@ static bool get_phys_addr(CPUARMState *env, target_ulong address, */ if (arm_feature(env, ARM_FEATURE_MPU) && arm_feature(env, ARM_FEATURE_V7)) { +bool ret; *page_size = TARGET_PAGE_SIZE; -return get_phys_addr_pmsav7(env, address, access_type, mmu_idx, -phys_ptr, prot, fsr); +ret = get_phys_addr_pmsav7(env, address, access_type, mmu_idx, + phys_ptr, prot, fsr); +qemu_log_mask(CPU_LOG_MMU, "TLB %08x mmu_idx=%u AC %u -> AC %u %s\n", + (unsigned)address, mmu_idx, 1<
[Qemu-devel] [PATCH v2 26/26] armv7m: decide whether faults are MemManage or BusFault
General logic is that operations stopped by the MPU are MemManage, and those which go through the MPU and are caught by the unassigned handle are BusFault. --- target-arm/helper.c | 35 +-- 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/target-arm/helper.c b/target-arm/helper.c index c890b3a..630d5c9 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -5546,12 +5546,35 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) break; case EXCP_PREFETCH_ABORT: case EXCP_DATA_ABORT: -/* TODO: if we implemented the MPU registers, this is where we - * should set the MMFAR, etc from exception.fsr and exception.vaddress. - */ -armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_MEM); -env->v7m.mmfar = env->exception.vaddress; -env->v7m.cfsr = (1<<1)|(1<<7); /* DACCVIOL and MMARVALID */ +switch (env->exception.fsr & 0xf) { +case 0x8: /* External Abort */ +switch (cs->exception_index) { +case EXCP_PREFETCH_ABORT: +env->v7m.cfsr |= (1<<(8+1)); /* PRECISERR */ +break; +case EXCP_DATA_ABORT: +env->v7m.cfsr |= (1<<(8+0)); /* IBUSERR */ +break; +} +armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_BUS); +env->v7m.bfar = env->exception.vaddress; +env->v7m.cfsr |= (1<<(8+7)); /* BFARVALID */ +break; +case 0xd: /* Permission fault */ +default: +switch (cs->exception_index) { +case EXCP_PREFETCH_ABORT: +env->v7m.cfsr |= (1<<0); /* IACCVIOL */ +break; +case EXCP_DATA_ABORT: +env->v7m.cfsr |= (1<<1); /* DACCVIOL */ +break; +} +armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_MEM); +env->v7m.mmfar = env->exception.vaddress; +env->v7m.cfsr |= (1<<7); /* MMARVALID */ +break; +} break; case EXCP_BKPT: if (semihosting_enabled()) { -- 2.1.4
[Qemu-devel] [PATCH v2 24/26] armv7m: split armv7m_init in two parts
Separate init and realize phases to allow board code the opportunity to set properties on the cpu and nvic. Assign names for cpu, nvic, and bitband regions. update stellaris and stm32 board code accordingly. --- hw/arm/armv7m.c| 42 +++--- hw/arm/stellaris.c | 18 +- hw/arm/stm32f205_soc.c | 6 -- include/hw/arm/arm.h | 4 ++-- 4 files changed, 38 insertions(+), 32 deletions(-) diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c index 68146de..fb805fe 100644 --- a/hw/arm/armv7m.c +++ b/hw/arm/armv7m.c @@ -144,11 +144,15 @@ static void armv7m_bitband_init(void) dev = qdev_create(NULL, TYPE_BITBAND); qdev_prop_set_uint32(dev, "base", 0x2000); +object_property_add_child(qdev_get_machine(), "bitband22", + &dev->parent_obj, &error_fatal); qdev_init_nofail(dev); sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0x2200); dev = qdev_create(NULL, TYPE_BITBAND); qdev_prop_set_uint32(dev, "base", 0x4000); +object_property_add_child(qdev_get_machine(), "bitband42", + &dev->parent_obj, &error_fatal); qdev_init_nofail(dev); sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0x4200); } @@ -162,39 +166,48 @@ static void armv7m_reset(void *opaque) cpu_reset(CPU(cpu)); } -/* Init CPU and memory for a v7-M based board. - mem_size is in bytes. - Returns the NVIC array. */ - -DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq, - const char *kernel_filename, const char *cpu_model) +void armv7m_init(const char *cpu_model) { ARMCPU *cpu; CPUARMState *env; DeviceState *nvic; -int image_size; -uint64_t entry; -uint64_t lowaddr; -int big_endian; if (cpu_model == NULL) { - cpu_model = "cortex-m3"; +cpu_model = "cortex-m3"; } -cpu = cpu_arm_init(cpu_model); +cpu = ARM_CPU(cpu_generic_init_unrealized(TYPE_ARM_CPU, cpu_model)); if (cpu == NULL) { fprintf(stderr, "Unable to find CPU definition\n"); exit(1); } env = &cpu->env; +object_property_add_child(qdev_get_machine(), "cpu[*]", OBJECT(cpu), + &error_fatal); + armv7m_bitband_init(); nvic = qdev_create(NULL, "armv7m_nvic"); -qdev_prop_set_uint32(nvic, "num-irq", num_irq); +object_property_add_child(qdev_get_machine(), "nvic", &nvic->parent_obj, + &error_fatal); env->nvic = nvic; -qdev_init_nofail(nvic); + sysbus_connect_irq(SYS_BUS_DEVICE(nvic), 0, qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ)); +} + + +void armv7m_realize(int mem_size, const char *kernel_filename) +{ +ARMCPU *cpu = ARM_CPU(first_cpu); +DeviceState *nvic = DEVICE(object_resolve_path("/machine/nvic", NULL)); +int image_size; +uint64_t entry; +uint64_t lowaddr; +int big_endian; + +qdev_init_nofail(DEVICE(cpu)); +qdev_init_nofail(nvic); #ifdef TARGET_WORDS_BIGENDIAN big_endian = 1; @@ -221,7 +234,6 @@ DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq, } qemu_register_reset(armv7m_reset, cpu); -return nvic; } static Property bitband_properties[] = { diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c index 7e56f02..3f12b57 100644 --- a/hw/arm/stellaris.c +++ b/hw/arm/stellaris.c @@ -1249,23 +1249,15 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model, vmstate_register_ram_global(sram); memory_region_add_subregion(system_memory, 0x2000, sram); -nvic = armv7m_init(system_memory, flash_size, NUM_IRQ_LINES, - kernel_filename, cpu_model); +armv7m_init(cpu_model); +qdev_prop_set_uint32(&first_cpu->parent_obj, "pmsav7-dregion", 8); +nvic = DEVICE(object_resolve_path("/machine/nvic", NULL)); +qdev_prop_set_uint32(nvic, "num-irq", NUM_IRQ_LINES); +armv7m_realize(flash_size, kernel_filename); qdev_connect_gpio_out_named(nvic, "SYSRESETREQ", 0, qemu_allocate_irq(&do_sys_reset, NULL, 0)); -{ -/* hack to change the number of MPU regions. - * Less of a hack than messing with cpu_model string. - * Safe as long as the number is being reduced. - */ -ARMCPU *cpu = ARM_CPU(qemu_get_cpu(0)); -assert(cpu->pmsav7_dregion>=8); -cpu->pmsav7_dregion = 8; -} - - if (board->dc1 & (1 << 16)) { dev = sysbus_create_varargs(TYPE_STELLARIS_ADC, 0x40038000, qdev_get_gpio_in(nvic, 14), diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c index 3f99340..01ae1e7 100644 --- a/hw/arm/stm32f205_soc.c +++ b/hw/arm/stm32f205_soc.c @@ -87,8 +87,10 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp) vmstate_register_ram_global(sram); memory_r
Re: [Qemu-devel] [PATCH v2 02/26] armv7m: Undo armv7m.hack
On 12/17/2015 10:38 AM, Peter Maydell wrote: > On 3 December 2015 at 00:18, Michael Davidsaver wrote: >> Add CPU unassigned access handler in place of special >> MemoryRegion to catch exception returns. >> >> The unassigned handler will signal other faults as either >> prefetch or data exceptions, with the FSR code 0x8 to >> distinguish them from memory translation faults (0xd). >> Future code will make use of this distinction when >> deciding to raise BusFault or MemManage exceptions. > This patch breaks my Stellaris test image -- instead of starting > it just sits there with a black screen. > > I've put a copy of that test image up at > http://people.linaro.org/~peter.maydell/stellaris.tgz > You can run it with path/to/stellaris/runme path/to/qemu-system-arm . There were several issues. Two bugs (wrong IRQ enabled and systick not enabled) and a "feature" (access to unimplemented registers for a PWM controller is now a BusFault). As a workaround for the "feature" I add a low priority MemoryRegion from 0x4000 -> 0x40ff which completes all reads with zero and logs. Please advise on how this should be handled. With these changes both test programs appear to run correctly, although the http server example has painfully slow load times and seems to hit an out of memory condition if I look at it wrong. Is this expected? (and the blub on the buttons page about "xml technology" is priceless) I can see that the http server example spends some time attempting MII operations on the NIC. As these aren't modeled it spins and eventually gives up. > ... > We could use a comment here (a) explaining what we're doing and (b) > mentioning that this isn't architecturally correct -- ideally we > should catch these exception exits on execution of the jump insn, not > by letting the jump execute and then trapping when we actually try to > execute at the magic addresses. Will do. >> ... >> @@ -294,19 +313,9 @@ static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, >> int interrupt_request) >> cc->do_interrupt(cs); >> ret = true; >> } >> -/* ARMv7-M interrupt return works by loading a magic value >> - * into the PC. On real hardware the load causes the >> - * return to occur. The qemu implementation performs the >> - * jump normally, then does the exception return when the >> - * CPU tries to execute code at the magic address. >> - * This will cause the magic PC value to be pushed to >> - * the stack if an interrupt occurred at the wrong time. >> - * We avoid this by disabling interrupts when >> - * pc contains a magic address. >> - */ >> if (interrupt_request & CPU_INTERRUPT_HARD >> && !(env->daif & PSTATE_I) >> -&& (env->regs[15] < 0xfff0)) { >> +) { > Can we really drop this change? The thing it's guarding against > (interrupt comes in while the PC is this not-really-an-address > value) can still happen whether we catch the attempt to execute > in translate.c or via the unassigned-access hook. I don't think the M-profile case in gen_intermediate_code() in translate.c can ever be reached without first hitting the unassigned memory handler. Before the code can be translated, the page containing it must be loaded. Such loads no longer succeed. Put more literally, gen_intermediate_code() is only called from tb_gen_code() where it comes after a call to get_page_addr_code(), wherein the unassigned handler calls cpu_loop_exit(). I've replaced the M case for EXCP_EXCEPTION_EXIT in gen_intermediate_code() with an assert. So far it hasn't failed.
Re: [Qemu-devel] [PATCH v2 03/26] armv7m: Explicit error for bad vector table
On 12/17/2015 08:25 AM, Peter Maydell wrote: > On 3 December 2015 at 00:18, Michael Davidsaver wrote: >> ... >> +static >> +uint32_t arm_v7m_load_vector(ARMCPU *cpu) >> + >> +{ >> +CPUState *cs = &cpu->parent_obj; > This isn't the right way to cast to the base class of a QOM object. > You want: >CPUState *cs = CPU(cpu); from cpu.h > /* Since this macro is used a lot in hot code paths and in conjunction > with > * FooCPU *foo_env_get_cpu(), we deviate from usual QOM practice by using > * an unchecked cast. > */ > #define CPU(obj) ((CPUState *)(obj)) Given the present definition of CPU() this change seems like a step backwards in terms of safety as mis-use won't be caught at compile or runtime. I'll change it anyway. > >> +CPUARMState *env = &cpu->env; >> +MemTxResult result; >> +hwaddr vec = env->v7m.vecbase + env->v7m.exception * 4; >> +uint32_t addr; >> + >> +addr = address_space_ldl(cs->as, vec, >> + MEMTXATTRS_UNSPECIFIED, &result); >> +if (result != MEMTX_OK) { > We could use a comment here: >/* Architecturally this should cause a HardFault setting HSFR.VECTTBL, > * which would then be immediately followed by our failing to load > * the entry vector for that HardFault, which is a Lockup case. > * Since we don't model Lockup, we just report this guest error > * via cpu_abort(). > */ Added.
Re: [Qemu-devel] [PATCH v2 05/26] armv7m: add armv7m_excp_running_prio()
On 12/17/2015 09:36 AM, Peter Maydell wrote: > On 3 December 2015 at 00:18, Michael Davidsaver wrote: >> Implements v7m exception priority algorithm >> using FAULTMASK, PRIMASK, BASEPRI, and the highest >> priority active exception. >> >> The number returned is the current execution priority >> which may be in the range [-2,0x7f] when an exception is active >> or 0x100 when no exception is active. >> --- >> hw/intc/armv7m_nvic.c | 25 + >> target-arm/cpu.h | 1 + >> 2 files changed, 26 insertions(+) >> >> diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c >> index 6fc167e..0145ca7 100644 >> --- a/hw/intc/armv7m_nvic.c >> +++ b/hw/intc/armv7m_nvic.c >> @@ -18,6 +18,8 @@ >> >> typedef struct { >> GICState gic; >> +uint8_t prigroup; >> + >> struct { >> uint32_t control; >> uint32_t reload; >> @@ -116,6 +118,29 @@ static void systick_reset(nvic_state *s) >> timer_del(s->systick.timer); >> } >> >> +/* @returns the active (running) exception priority. >> + *only a higher (numerically lower) priority can preempt. >> + */ >> +int armv7m_excp_running_prio(ARMCPU *cpu) >> +{ >> +CPUARMState *env = &cpu->env; >> +nvic_state *s = env->nvic; >> +int running; >> + >> +if (env->daif & PSTATE_F) { /* FAULTMASK */ >> +running = -1; >> +} else if (env->daif & PSTATE_I) { /* PRIMASK */ >> +running = 0; >> +} else if (env->v7m.basepri > 0) { >> +/* BASEPRI==1 -> masks [1,255] (not same as PRIMASK==1) */ >> +running = env->v7m.basepri >> (s->prigroup+1); > This isn't right -- the effect of PRIGROUP is that we mask > out the lower (subgroup) bits, but the upper group bits stay > where they are rather than shifting down. > > So you want env->v7m.basepri & ~((1 << (s->prigroup + 1)) - 1); > > (the same mask you need to get the group priority for > an interrupt). I don't know about "right", but it is consistent with how the .prio_group field is now handled in the nvic. So I think the final behavior is as specified. There is no functional reason that I do this. I just think it makes the DPRINTF messages easier to interpret. If you feel strongly I can change this.
Re: [Qemu-devel] [PATCH v2 02/26] armv7m: Undo armv7m.hack
On 12/17/2015 10:38 AM, Peter Maydell wrote: > We could use a comment here (a) explaining what we're doing and (b) > mentioning that this isn't architecturally correct -- ideally we should > catch these exception exits on execution of the jump insn, not by > letting the jump execute and then trapping when we actually try to > execute at the magic addresses. I had an instructive little digression to investigate doing things the "right way" (in tcg). I can see how it would be done by adding a conditional every time the PC could be updated. To me the unassigned handler trick/hack seems simpler (less likely to add a bug) and avoids emitting more code for every ldm/pop instruction.
Re: [Qemu-devel] [PATCH v2 06/26] armv7m: fix I and F flag handling
On 12/17/2015 10:18 AM, Peter Maydell wrote: > On 17 December 2015 at 14:39, Peter Maydell wrote: >> On 3 December 2015 at 00:18, Michael Davidsaver >> wrote: >>> Despite having the same notation, these bits >>> have completely different meaning than -AR. >>> >>> Use armv7m_excp_running_prio() and the highest >>> pending exception priority to determine >>> if the pending exception can interrupt preempt. >>> --- >>> target-arm/cpu.c | 16 ++-- >>> 1 file changed, 6 insertions(+), 10 deletions(-) >> Reviewed-by: Peter Maydell > ...except this breaks the build for linux-user: > > LINK arm-linux-user/qemu-arm > target-arm/cpu.o: In function `arm_v7m_cpu_exec_interrupt': > /home/petmay01/linaro/qemu-from-laptop/qemu/target-arm/cpu.c:316: > undefined reference to `armv7m_excp_running_prio' > > because the function you're calling here is in armv7m_nvic.c, > which isn't compiled into the linux-user binary. Is there any reason to include the armv7m code in linux-user at all?
Re: [Qemu-devel] Any progress with the Cortex-M4 emulation?
On 04/06/2016 06:23 PM, Liviu Ionescu wrote: > >> On 07 Apr 2016, at 01:04, Peter Maydell wrote: >> >> ... Somebody needs to do the necessary work to fix the >> code review issues. ... > > in this case I'll probably wait for this process to be completed and > reevaluate the situation by then. Liviu, I haven't had time to complete a revision of my patch set so far this year. I've been busy with other work (good for me, bad for qemu), and don't see this situation changing for an least the next two months. It sounds like you have time and inclination to do part of what I've started. I hope my having done half of a too big job won't keep you from finishing part of it. If you decide to work on this problem, please don't hesitate to appropriate, or ignore, what I've done so far. For what it's worth, I did start another revision back in February. It does include the change Peter requested to the storage of priorities wrt. prigroup, but doesn't break up the big "rewrite NVIC" patch. https://github.com/mdavidsaver/qemu/tree/fixirq2 Separately, I have a set of target test programs which I can run both with qemu and a real board. They mostly agree. Aside from test8.c (MPU) they might be of interest. https://github.com/mdavidsaver/baremetal/tree/qemutest Michael
Re: [Qemu-devel] [PATCH 3/9] armv7m: Rewrite NVIC to not use any GIC code
On 02/16/2017 09:11 AM, Peter Maydell wrote: > I haven't actually checked real hardware behaviour, but I think > we can fairly safely implement this as not checking the IPSR > exception field. (We might as well go with the "reads 1 in > handler mode" choice of UNKNOWN that the M3 documents, though.) For what it's worth, I dug up my TI TM4C1294 eval board and re-ran test10.c [1] which is designed to probe this behavior by nesting exceptions PendSV within SVC. RETTOBASE is 0x800 in ICSR. > 1..12 > # BASEPRI mask 00e0 > # DEBUG prio 00e0 > ok 1 - == ICSR > ok 2 - == SHCSR > # Call SVC > # In SVC > ok 3 - 080b == 080b ICSR > ok 4 - 0080 == 0080 SHCSR > # In PendSV > ok 5 - 000e == 000e ICSR > ok 6 - 0480 == 0480 SHCSR > # Back in SVC > ok 7 - 0003 == 0003 Back in SVC > ok 8 - 080b == 080b ICSR > ok 9 - 0080 == 0080 SHCSR > # Back in main > ok 10 - 0004 == 0004 Back in main > ok 11 - == ICSR > ok 12 - == SHCSR > # Done [1] https://github.com/mdavidsaver/baremetal/blob/qemutest/test10.c
Re: [Qemu-devel] [PATCH 3/9] armv7m: Rewrite NVIC to not use any GIC code
On 02/18/2017 01:38 PM, Peter Maydell wrote: > On 18 February 2017 at 17:45, Michael Davidsaver > wrote: >> On 02/16/2017 09:11 AM, Peter Maydell wrote: >>> I haven't actually checked real hardware behaviour, but I think >>> we can fairly safely implement this as not checking the IPSR >>> exception field. (We might as well go with the "reads 1 in >>> handler mode" choice of UNKNOWN that the M3 documents, though.) >> >> For what it's worth, I dug up my TI TM4C1294 eval board and re-ran >> test10.c [1] which is designed to probe this behavior by nesting >> exceptions PendSV within SVC. RETTOBASE is 0x800 in ICSR. > > That's a Cortex-M4. From the test it looks like it > has a different choice of UNKNOWN behaviour for > the value in Handler mode, so real code in the field > isn't going to be relying on that and it doesn't > matter what we choose. I've been away from arm/m for too long to claim any detailed knowledge of the documentation. My intent here is only to provide a data point w/ real hardware, not to interpret it. > I don't think the test looks at the "what happens if the > exception in the IPSR exception field isn't actually > active" case, right? Correct.
Re: [Qemu-devel] [PATCH 05/17] timer: generalize Dallas/Maxim RTC i2c devices
On 11/29/2017 11:13 PM, David Gibson wrote: > On Sun, Nov 26, 2017 at 03:59:03PM -0600, Michael Davidsaver wrote: >> Support for: ds1307, ds1337, ds1338, ds1339, >> ds1340, ds1375, ds1388, and ds3231. >> >> Tested with ds1338 and ds1375. >> >> Signed-off-by: Michael Davidsaver > > I certainly like the idea of consolidating this code, but reviewing to > see that the new code really is a generalization of the old is > something I won't have time for for a while. > > Also, hw/timer is not within my purview so it'll probably need to go > another path to merge. Could you be a bit more explicit about what, if anything, I need to do to move this forward? >> --- >> default-configs/arm-softmmu.mak | 2 +- >> hw/timer/Makefile.objs | 2 +- >> hw/timer/ds-rtc-i2c.c | 461 >> >> hw/timer/ds1338.c | 239 - >> 4 files changed, 463 insertions(+), 241 deletions(-) >> create mode 100644 hw/timer/ds-rtc-i2c.c >> delete mode 100644 hw/timer/ds1338.c >> >> diff --git a/default-configs/arm-softmmu.mak >> b/default-configs/arm-softmmu.mak >> index d37edc4312..b857823681 100644 >> --- a/default-configs/arm-softmmu.mak >> +++ b/default-configs/arm-softmmu.mak >> @@ -31,7 +31,7 @@ CONFIG_SMC91C111=y >> CONFIG_ALLWINNER_EMAC=y >> CONFIG_IMX_FEC=y >> CONFIG_FTGMAC100=y >> -CONFIG_DS1338=y >> +CONFIG_DSRTCI2C=y >> CONFIG_PFLASH_CFI01=y >> CONFIG_PFLASH_CFI02=y >> CONFIG_MICRODRIVE=y >> diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs >> index 8c19eac3b6..290015ebec 100644 >> --- a/hw/timer/Makefile.objs >> +++ b/hw/timer/Makefile.objs >> @@ -3,7 +3,7 @@ common-obj-$(CONFIG_ARM_MPTIMER) += arm_mptimer.o >> common-obj-$(CONFIG_ARM_V7M) += armv7m_systick.o >> common-obj-$(CONFIG_A9_GTIMER) += a9gtimer.o >> common-obj-$(CONFIG_CADENCE) += cadence_ttc.o >> -common-obj-$(CONFIG_DS1338) += ds1338.o >> +common-obj-$(CONFIG_DSRTCI2C) += ds-rtc-i2c.o >> common-obj-$(CONFIG_HPET) += hpet.o >> common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o >> common-obj-$(CONFIG_M48T59) += m48t59.o >> diff --git a/hw/timer/ds-rtc-i2c.c b/hw/timer/ds-rtc-i2c.c >> new file mode 100644 >> index 00..ad2f8f2a68 >> --- /dev/null >> +++ b/hw/timer/ds-rtc-i2c.c >> @@ -0,0 +1,461 @@ >> +/* Emulation of various Dallas/Maxim RTCs accessed via I2C bus >> + * >> + * Copyright (c) 2017 Michael Davidsaver >> + * Copyright (c) 2009 CodeSourcery >> + * >> + * Authors: Michael Davidsaver >> + * Paul Brook >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2. See >> + * the LICENSE file in the top-level directory. >> + * >> + * Models real time read/set and NVRAM. >> + * Does not model alarms, or control/status registers. >> + * >> + * Generalized register map is: >> + * [Current time] >> + * [Alarm settings] (optional) >> + * [Control/Status] (optional) >> + * [Non-volatile memory] (optional) >> + * >> + * The current time registers are almost always the same, >> + * with the exception being that some have a CENTURY bit >> + * in the month register. >> + */ >> +#include "qemu/osdep.h" >> +#include "qemu/log.h" >> +#include "qemu/timer.h" >> +#include "qemu/bcd.h" >> +#include "hw/hw.h" >> +#include "hw/registerfields.h" >> +#include "hw/i2c/i2c.h" >> +#include "sysemu/qtest.h" >> +#include "qemu/error-report.h" >> + >> +/* #define DEBUG_DSRTC */ >> + >> +#ifdef DEBUG_DSRTC >> +#define DPRINTK(FMT, ...) info_report(TYPE_DSRTC " : " FMT, ## __VA_ARGS__) >> +#else >> +#define DPRINTK(FMT, ...) do {} while (0) >> +#endif >> + >> +#define LOG(MSK, FMT, ...) qemu_log_mask(MSK, TYPE_DSRTC " : " FMT "\n", \ >> +## __VA_ARGS__) >> + >> +#define DSRTC_REGSIZE (0x40) >> + >> +/* values stored in BCD */ >> +/* 00-59 */ >> +#define R_SEC (0x0) >> +/* 00-59 */ >> +#define R_MIN (0x1) >> +#define R_HOUR (0x2) >> +/* 1-7 */ >> +#define R_WDAY (0x3) >> +/* 0-31 */ >> +#define R_DATE (0x4) >> +#define R_MONTH (0x5) >> +/* 0-99 */ >> +#define R_YEAR (0x6) >> + >> +/* use 12 hour mode when set */ >> +FIELD(HOUR, SET12, 6, 1) >> +/* 00-23 */ >> +FI
Re: [Qemu-devel] [PATCH 02/12] e500: consolidate mpc8540 guts with e500-ccsr
On 12/05/2017 10:12 PM, David Gibson wrote: > On Wed, Nov 22, 2017 at 02:36:43PM +1100, David Gibson wrote: >> On Sun, Nov 19, 2017 at 09:24:10PM -0600, Michael Davidsaver wrote: >>> Preparation for adding more MPC control >>> registers. >>> >>> Use e500 SVR to enable part specific registers. >>> Only the mpc8544 reset register at present. >>> >>> Expose CCSR as SysBusDevice region to eliminate >>> e500-ccsr.h. >>> >>> Track CCSR base address within device, and map on reset, >>> in preparation for CCSRBAR. >>> >>> Signed-off-by: Michael Davidsaver >> >> Applied to ppc-for-2.12. > > Sorry. I've now pulled this from ppc-for-2.12, because it caused a > breakage of make check (specifically the boot-serial-test with ppc64). Oops, I'll begin running this test myself. So far I've only been running tests for i386, amd64, arm, and ppc.
Re: [Qemu-devel] [PATCH 12/17] e500: add i2c controller to CCSR
On 12/05/2017 01:49 AM, David Gibson wrote: > On Sun, Nov 26, 2017 at 03:59:10PM -0600, Michael Davidsaver wrote: >> Add i2c controller found on mpc8540, >> mpc8544, and P2010 (newer ppc, unmodeled). > > This adds it unconditionally. Are there any E500 models where it > doesn't exist? Not in the devices I've looked at, though I certainly haven't looked at them all. In fact the mpc8544 has two i2c controllers. I keep mentioning the P2010 as it is about a decade newer than the mpc85xx chips. My _assumption_ is that the commonalities between these are a reasonable least common denominator. >> >> Signed-off-by: Michael Davidsaver >> --- >> hw/ppc/e500_ccsr.c | 15 +++ >> 1 file changed, 15 insertions(+) >> >> diff --git a/hw/ppc/e500_ccsr.c b/hw/ppc/e500_ccsr.c >> index c479ed91ee..cd8216daaf 100644 >> --- a/hw/ppc/e500_ccsr.c >> +++ b/hw/ppc/e500_ccsr.c >> @@ -46,6 +46,8 @@ >> #define E500_ERR_DETECT (0x2e40) >> #define E500_ERR_DISABLE (0x2e44) >> >> +#define E500_I2C_OFFSET (0x3000) >> + >> #define E500_DUART_OFFSET(N) (0x4500 + (N) * 0x100) >> >> #define E500_PORPLLSR(0xE) >> @@ -72,6 +74,7 @@ typedef struct { >> uint32_t ccb_freq; >> >> DeviceState *pic; >> +DeviceState *i2c; >> } CCSRState; >> >> #define TYPE_E500_CCSR "e500-ccsr" >> @@ -272,6 +275,18 @@ static void e500_ccsr_realize(DeviceState *dev, Error >> **errp) >> sysbus_mmio_get_region(pic, 0)); >> /* Note: MPIC internal interrupts are offset by 16 */ >> >> +/* attach I2C controller */ >> +ccsr->i2c = qdev_create(NULL, "mpc8540-i2c"); > > Ah.. so I think I missed this on many earlier patches. I believe > you're not generally supposed to create new subdevices at realize() > time. Instead the device should be created at CCSR instance_init() > time (but the sub device's "realized" property will need to be set to > 1 from CCSR realize()). > >> +object_property_add_child(qdev_get_machine(), "i2c[*]", >> + OBJECT(ccsr->i2c), NULL); >> +qdev_init_nofail(ccsr->i2c); >> +memory_region_add_subregion(&ccsr->iomem, E500_I2C_OFFSET, >> +sysbus_mmio_get_region( >> +SYS_BUS_DEVICE(ccsr->i2c), 0)); >> +sysbus_connect_irq(SYS_BUS_DEVICE(ccsr->i2c), 0, >> + qdev_get_gpio_in(ccsr->pic, 16 + 27)); >> + >> + >> /* DUARTS */ >> /* for mpc8540, mpc8544, and P2010 (unmodeled), the DUART reference >> clock >> * is the CCB clock divided by 16. > signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 13/17] e500: move PCI host bridge into CCSR
On 12/05/2017 01:53 AM, David Gibson wrote: > On Sun, Nov 26, 2017 at 03:59:11PM -0600, Michael Davidsaver wrote: >> Signed-off-by: Michael Davidsaver > > Hmm. Is there anything you're *not* planning to move under the CCSR. Well, the decrementer/timebase initialization for one as this has nothing to do with the CCSR registers. I haven't added the TSEC/eTSEC instances either. Partly this is because the existing boards, for reasons I don't understand, use virtio NICs. Further, the mpc8540 has TSEC instances 1 and 2, while the mpc8544 has instances 1 and 3. So I decided to leave NIC setup to the Machine rather then add the extra code to parameterize this under the CCSR device. > If not, I'm really wondering if the CCSR ought to be a device in its > own right, rather than just a container memory region used within the > machine. I don't think I follow what you mean by "device" in this context? The CCSR object is a SysBusDevice in the qom tree ("/machine/e500-ccsr"). What device-like characteristics could it have? >> --- >> hw/ppc/e500.c | 13 - >> hw/ppc/e500_ccsr.c | 27 +++ >> 2 files changed, 31 insertions(+), 9 deletions(-) >> >> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c >> index cfd5ed0152..b0c8495aef 100644 >> --- a/hw/ppc/e500.c >> +++ b/hw/ppc/e500.c >> @@ -769,6 +769,8 @@ void ppce500_init(MachineState *machine, PPCE500Params >> *params) >> qdev_prop_set_uint32(dev, "mpic-model", params->mpic_version); >> qdev_prop_set_uint32(dev, "base", params->ccsrbar_base); >> qdev_prop_set_uint32(dev, "ram-size", ram_size); >> +qdev_prop_set_uint32(dev, "pci_first_slot", params->pci_first_slot); >> +qdev_prop_set_uint32(dev, "pci_first_pin_irq", pci_irq_nrs[0]); >> qdev_init_nofail(dev); >> ccsr_addr_space = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0); >> >> @@ -778,20 +780,13 @@ void ppce500_init(MachineState *machine, PPCE500Params >> *params) >> >> >> /* PCI */ >> -dev = qdev_create(NULL, "e500-pcihost"); >> -object_property_add_child(qdev_get_machine(), "pci-host", OBJECT(dev), >> - &error_abort); >> -qdev_prop_set_uint32(dev, "first_slot", params->pci_first_slot); >> -qdev_prop_set_uint32(dev, "first_pin_irq", pci_irq_nrs[0]); >> -qdev_init_nofail(dev); >> +dev = DEVICE(object_resolve_path("/machine/pci-host", 0)); >> +assert(dev); >> s = SYS_BUS_DEVICE(dev); >> for (i = 0; i < PCI_NUM_PINS; i++) { >> sysbus_connect_irq(s, i, qdev_get_gpio_in(mpicdev, pci_irq_nrs[i])); >> } >> >> -memory_region_add_subregion(ccsr_addr_space, MPC8544_PCI_REGS_OFFSET, >> -sysbus_mmio_get_region(s, 0)); >> - >> pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0"); >> if (!pci_bus) >> printf("couldn't create PCI controller!\n"); >> diff --git a/hw/ppc/e500_ccsr.c b/hw/ppc/e500_ccsr.c >> index cd8216daaf..4ec8f7524d 100644 >> --- a/hw/ppc/e500_ccsr.c >> +++ b/hw/ppc/e500_ccsr.c >> @@ -50,6 +50,8 @@ >> >> #define E500_DUART_OFFSET(N) (0x4500 + (N) * 0x100) >> >> +#define E500_PCI_OFFSET (0x8000ULL) >> + >> #define E500_PORPLLSR(0xE) >> #define E500_PVR (0xE00A0) >> #define E500_SVR (0xE00A4) >> @@ -75,6 +77,7 @@ typedef struct { >> >> DeviceState *pic; >> DeviceState *i2c; >> +DeviceState *pcihost; >> } CCSRState; >> >> #define TYPE_E500_CCSR "e500-ccsr" >> @@ -201,6 +204,7 @@ static void e500_ccsr_init(Object *obj) >> DeviceState *dev = DEVICE(obj); >> CCSRState *ccsr = E500_CCSR(dev); >> >> +/* prepare MPIC */ >> assert(current_machine); >> if (kvm_enabled()) { >> >> @@ -228,6 +232,18 @@ static void e500_ccsr_init(Object *obj) >> object_property_add_alias(obj, "mpic-model", >>OBJECT(ccsr->pic), "model", >>&error_fatal); >> + >> +/* prepare PCI host bridge */ >> +ccsr->pcihost = qdev_create(NULL, "e500-pcihost"); >> +object_property_add_child(qdev_get_machine(), "pci-host", >> OBJECT(ccsr->pcihost), >> + &error_abort); >> + >> +obj
[Qemu-devel] [PATCH 01/12] e500: add board config options
allow board code to skip common NIC and guest image setup and configure decrementor frequency. Existing boards unchanged. Signed-off-by: Michael Davidsaver --- hw/ppc/e500.c | 8 ++-- hw/ppc/e500.h | 3 +++ hw/ppc/e500plat.c | 1 + hw/ppc/mpc8544ds.c | 1 + 4 files changed, 11 insertions(+), 2 deletions(-) diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c index 5cf0dabef3..9e7e1b29c4 100644 --- a/hw/ppc/e500.c +++ b/hw/ppc/e500.c @@ -826,7 +826,7 @@ void ppce500_init(MachineState *machine, PPCE500Params *params) env->mpic_iack = params->ccsrbar_base + MPC8544_MPIC_REGS_OFFSET + 0xa0; -ppc_booke_timers_init(cpu, 4, PPC_TIMER_E500); +ppc_booke_timers_init(cpu, params->decrementor_freq, PPC_TIMER_E500); /* Register reset handler */ if (!i) { @@ -899,7 +899,7 @@ void ppce500_init(MachineState *machine, PPCE500Params *params) if (!pci_bus) printf("couldn't create PCI controller!\n"); -if (pci_bus) { +if (pci_bus && !params->tsec_nic) { /* Register network interfaces. */ for (i = 0; i < nb_nics; i++) { pci_nic_init_nofail(&nd_table[i], pci_bus, "virtio", NULL); @@ -948,6 +948,10 @@ void ppce500_init(MachineState *machine, PPCE500Params *params) sysbus_mmio_get_region(s, 0)); } +if (params->skip_load) { +return; +} + /* Load kernel. */ if (machine->kernel_filename) { kernel_base = cur_base; diff --git a/hw/ppc/e500.h b/hw/ppc/e500.h index 70ba1d8f4f..40f72f2de2 100644 --- a/hw/ppc/e500.h +++ b/hw/ppc/e500.h @@ -22,6 +22,9 @@ typedef struct PPCE500Params { hwaddr pci_mmio_base; hwaddr pci_mmio_bus_base; hwaddr spin_base; +uint32_t decrementor_freq; /* in Hz */ +bool skip_load; +bool tsec_nic; } PPCE500Params; void ppce500_init(MachineState *machine, PPCE500Params *params); diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c index e59e80fb9e..3d07987bd1 100644 --- a/hw/ppc/e500plat.c +++ b/hw/ppc/e500plat.c @@ -47,6 +47,7 @@ static void e500plat_init(MachineState *machine) .pci_mmio_base = 0xCULL, .pci_mmio_bus_base = 0xE000ULL, .spin_base = 0xFEF00ULL, +.decrementor_freq = 4, }; /* Older KVM versions don't support EPR which breaks guests when we announce diff --git a/hw/ppc/mpc8544ds.c b/hw/ppc/mpc8544ds.c index 1717953ec7..6d9931c475 100644 --- a/hw/ppc/mpc8544ds.c +++ b/hw/ppc/mpc8544ds.c @@ -40,6 +40,7 @@ static void mpc8544ds_init(MachineState *machine) .pci_mmio_bus_base = 0xC000ULL, .pci_pio_base = 0xE100ULL, .spin_base = 0xEF00ULL, +.decrementor_freq = 4, }; if (machine->ram_size > 0xc000) { -- 2.11.0
[Qemu-devel] [PATCH 08/12] e500: add mpc8540 i2c controller to ccsr
Signed-off-by: Michael Davidsaver --- hw/ppc/e500.c | 8 1 file changed, 8 insertions(+) diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c index 6f77844303..bef7d313d4 100644 --- a/hw/ppc/e500.c +++ b/hw/ppc/e500.c @@ -861,6 +861,14 @@ void ppce500_init(MachineState *machine, PPCE500Params *params) qdev_init_nofail(dev); ccsr_addr_space = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0); +dev = qdev_create(NULL, "mpc8540-i2c"); +object_property_add_child(qdev_get_machine(), "i2c[*]", + OBJECT(dev), NULL); +qdev_init_nofail(dev); +s = SYS_BUS_DEVICE(dev); +memory_region_add_subregion(ccsr_addr_space, 0x3000, +sysbus_mmio_get_region(s, 0)); + mpicdev = ppce500_init_mpic(machine, params, ccsr_addr_space, irqs); /* Serial */ -- 2.11.0
[Qemu-devel] [PATCH 03/12] e500: note possible bug with host bridge
Signed-off-by: Michael Davidsaver --- hw/pci-host/ppce500.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c index f2d108bc8a..0e2833bd98 100644 --- a/hw/pci-host/ppce500.c +++ b/hw/pci-host/ppce500.c @@ -424,6 +424,9 @@ static void e500_pcihost_bridge_realize(PCIDevice *d, Error **errp) MemoryRegion *ccsr_mr = sysbus_mmio_get_region(ccsr, 0); pci_config_set_class(d->config, PCI_CLASS_BRIDGE_PCI); +/* BUG? identifies as PCI_HEADER_TYPE_BRIDGE but uses + * standard device config read/write + */ d->config[PCI_HEADER_TYPE] = (d->config[PCI_HEADER_TYPE] & PCI_HEADER_TYPE_MULTI_FUNCTION) | PCI_HEADER_TYPE_BRIDGE; -- 2.11.0
[Qemu-devel] [PATCH 04/12] e500: additional CCSR registers
Add CCSRBAR to allow CCSR region to be relocated. Guest memory size introspection. Dummy RAM error controls. Guest clock introspection. Signed-off-by: Michael Davidsaver --- hw/ppc/e500.c | 2 ++ hw/ppc/e500.h | 1 + hw/ppc/e500_ccsr.c | 72 -- hw/ppc/e500plat.c | 1 + hw/ppc/mpc8544ds.c | 1 + 5 files changed, 75 insertions(+), 2 deletions(-) diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c index 474a46a985..057be1751b 100644 --- a/hw/ppc/e500.c +++ b/hw/ppc/e500.c @@ -853,7 +853,9 @@ void ppce500_init(MachineState *machine, PPCE500Params *params) dev = qdev_create(NULL, "e500-ccsr"); object_property_add_child(qdev_get_machine(), "e500-ccsr", OBJECT(dev), NULL); +qdev_prop_set_uint32(dev, "porpllsr", params->porpllsr); qdev_prop_set_uint32(dev, "base", params->ccsrbar_base); +qdev_prop_set_uint32(dev, "ram-size", ram_size); qdev_init_nofail(dev); ccsr_addr_space = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0); diff --git a/hw/ppc/e500.h b/hw/ppc/e500.h index 40f72f2de2..1f39095dfa 100644 --- a/hw/ppc/e500.h +++ b/hw/ppc/e500.h @@ -22,6 +22,7 @@ typedef struct PPCE500Params { hwaddr pci_mmio_base; hwaddr pci_mmio_bus_base; hwaddr spin_base; +uint32_t porpllsr; /* value of PORPLLSR register */ uint32_t decrementor_freq; /* in Hz */ bool skip_load; bool tsec_nic; diff --git a/hw/ppc/e500_ccsr.c b/hw/ppc/e500_ccsr.c index 1b586c3f42..c58b17f06b 100644 --- a/hw/ppc/e500_ccsr.c +++ b/hw/ppc/e500_ccsr.c @@ -31,6 +31,16 @@ /* E500_ denotes registers common to all */ +#define E500_CCSRBAR (0) + +#define E500_CS0_BNDS(0x2000) + +#define E500_CS0_CONFIG (0x2080) + +#define E500_ERR_DETECT (0x2e40) +#define E500_ERR_DISABLE (0x2e44) + +#define E500_PORPLLSR(0xE) #define E500_PVR (0xE00A0) #define E500_SVR (0xE00A4) @@ -44,7 +54,11 @@ typedef struct { MemoryRegion iomem; -uint32_t defbase; +uint32_t defbase, base; +uint32_t ram_size; +uint32_t merrd; + +uint32_t porpllsr; } CCSRState; #define TYPE_E500_CCSR "e500-ccsr" @@ -53,10 +67,28 @@ typedef struct { static uint64_t e500_ccsr_read(void *opaque, hwaddr addr, unsigned size) { +CCSRState *ccsr = opaque; PowerPCCPU *cpu = POWERPC_CPU(current_cpu); CPUPPCState *env = &cpu->env; switch (addr) { +case E500_CCSRBAR: +return ccsr->base >> 12; +case E500_CS0_BNDS: +/* we model all RAM in a single chip with addresses [0, ram_size) */ +return (ccsr->ram_size - 1) >> 24; +case E500_CS0_CONFIG: +return 1 << 31; +case E500_ERR_DETECT: +return 0; /* (errors not modeled) */ +case E500_ERR_DISABLE: +return ccsr->merrd; +case E500_PORPLLSR: +if (!ccsr->porpllsr) { +qemu_log_mask(LOG_UNIMP, + "Machine does not provide valid PORPLLSR\n"); +} +return ccsr->porpllsr; case E500_PVR: return env->spr[SPR_PVR]; case E500_SVR: @@ -72,10 +104,22 @@ static uint64_t e500_ccsr_read(void *opaque, hwaddr addr, static void e500_ccsr_write(void *opaque, hwaddr addr, uint64_t value, unsigned size) { +CCSRState *ccsr = opaque; PowerPCCPU *cpu = POWERPC_CPU(current_cpu); CPUPPCState *env = &cpu->env; uint32_t svr = env->spr[SPR_E500_SVR] >> 16; +switch (addr) { +case E500_CCSRBAR: +value &= 0x000fff00; +ccsr->base = value << 12; +sysbus_mmio_map(SYS_BUS_DEVICE(ccsr), 0, ccsr->base); +return; +case E500_ERR_DISABLE: +ccsr->merrd = value & 0xd; +return; +} + switch (svr) { case 0: /* generic. assumed to be mpc8544ds or e500plat board */ case 0x8034: /* mpc8544 */ @@ -104,11 +148,20 @@ static const MemoryRegionOps e500_ccsr_ops = { } }; +static int e500_ccsr_post_load(void *opaque, int version_id) +{ +CCSRState *ccsr = opaque; + +sysbus_mmio_map(SYS_BUS_DEVICE(ccsr), 0, ccsr->base); +return 0; +} + static void e500_ccsr_reset(DeviceState *dev) { CCSRState *ccsr = E500_CCSR(dev); -sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, ccsr->defbase); +ccsr->base = ccsr->defbase; +e500_ccsr_post_load(ccsr, 1); } static void e500_ccsr_initfn(Object *obj) @@ -123,15 +176,30 @@ static void e500_ccsr_initfn(Object *obj) static Property e500_ccsr_props[] = { DEFINE_PROP_UINT32("base", CCSRState, defbase, 0xff70), +DEFINE_PROP_UINT32("ram-size", CCSRState, ram_size, 0), +DEFINE_PROP_UINT32("porpllsr", CCSRState, porpllsr, 0), DEFINE_PROP_END_OF_LIST() }; +st
[Qemu-devel] [PATCH 06/12] i2c: add mpc8540 i2c controller
Signed-off-by: Michael Davidsaver --- hw/i2c/Makefile.objs | 1 + hw/i2c/mpc8540_i2c.c | 287 +++ 2 files changed, 288 insertions(+) create mode 100644 hw/i2c/mpc8540_i2c.c diff --git a/hw/i2c/Makefile.objs b/hw/i2c/Makefile.objs index 0594dea3ae..79af1dd901 100644 --- a/hw/i2c/Makefile.objs +++ b/hw/i2c/Makefile.objs @@ -9,3 +9,4 @@ common-obj-$(CONFIG_IMX_I2C) += imx_i2c.o common-obj-$(CONFIG_ASPEED_SOC) += aspeed_i2c.o obj-$(CONFIG_OMAP) += omap_i2c.o obj-$(CONFIG_PPC4XX) += ppc4xx_i2c.o +obj-$(CONFIG_E500) += mpc8540_i2c.o diff --git a/hw/i2c/mpc8540_i2c.c b/hw/i2c/mpc8540_i2c.c new file mode 100644 index 00..884052cc9b --- /dev/null +++ b/hw/i2c/mpc8540_i2c.c @@ -0,0 +1,287 @@ +/* + * MPC8540 I2C bus interface + * As described in + * MPC8540 PowerQUICC III Integrated Host Processor Reference Manual, Rev. 1 + * Part 2 chapter 11 + * + * Copyright (c) 2015 Michael Davidsaver + * + * This work is licensed under the terms of the GNU GPL, version 2. See + * the LICENSE file in the top-level directory. + */ +#include "qemu/osdep.h" +#include "qemu/log.h" +#include "hw/hw.h" +#include "hw/registerfields.h" +#include "hw/i2c/i2c.h" +#include "hw/sysbus.h" + +/* #define DEBUG_LVL 0 */ + +#ifdef DEBUG_LVL +#define DPRINTK(LVL, FMT, ...) do { if ((LVL) <= DEBUG_LVL) { \ +printf(TYPE_MPC8540_I2C " : " FMT, ## __VA_ARGS__); } } while (0) +#else +#define DPRINTK(LVL, FMT, ...) do {} while (0) +#endif + +#define LOG(MSK, FMT, ...) qemu_log_mask(MSK, TYPE_MPC8540_I2C \ +" : " FMT, ## __VA_ARGS__) + +#define TYPE_MPC8540_I2C "mpc8540-i2c" +#define MPC8540_I2C(obj) OBJECT_CHECK(I2CState, (obj), TYPE_MPC8540_I2C) + +/* offsets relative to CCSR offset 0x3000 */ +#define R_I2CADR (0) +#define R_I2CFDR (4) +#define R_I2CCR (8) +#define R_I2CSR (0xc) +#define R_I2CDR (0x10) +#define R_I2CDFSRR (0x14) + +FIELD(I2CCR, MEN, 7, 1) +FIELD(I2CCR, MIEN, 6, 1) +FIELD(I2CCR, MSTA, 5, 1) +FIELD(I2CCR, MTX, 4, 1) +FIELD(I2CCR, TXAK, 3, 1) +FIELD(I2CCR, RSTA, 2, 1) +FIELD(I2CCR, BCST, 0, 1) + +FIELD(I2CSR, MCF, 7, 1) +FIELD(I2CSR, MAAS, 6, 1) +FIELD(I2CSR, MBB, 5, 1) +FIELD(I2CSR, MAL, 4, 1) +FIELD(I2CSR, BCSTM, 3, 1) +FIELD(I2CSR, SRW, 2, 1) +FIELD(I2CSR, MIF, 1, 1) +FIELD(I2CSR, RXAK, 0, 1) + +typedef struct I2CState { +SysBusDevice parent_obj; + +I2CBus *bus; + +uint8_t ctrl, sts; +uint8_t freq, filt; +/* Reads are pipelined, this is the next data value */ +uint8_t dbuf; + +qemu_irq irq; + +MemoryRegion mmio; +} I2CState; + +#define I2CCR(BIT) FIELD_EX32(i2c->ctrl, I2CCR, BIT) +#define I2CSR(BIT) FIELD_EX32(i2c->sts, I2CSR, BIT) + +#define I2CSR_SET(BIT, VAL) do {\ +i2c->sts = FIELD_DP32(i2c->sts, I2CSR, BIT, VAL);\ +} while (0) + +static +void mpc8540_update_irq(I2CState *i2c) +{ +int ena = i2c->ctrl & 0x40, +sts = i2c->sts & 0x02, +act = !!(ena && sts); + +DPRINTK(1, "IRQ %c ena %c sts %c\n", +act ? 'X' : '_', +ena ? 'X' : '_', +sts ? 'X' : '_'); + +qemu_set_irq(i2c->irq, act); +} + +static +uint64_t mpc8540_i2c_read(void *opaque, hwaddr addr, unsigned size) +{ +I2CState *i2c = opaque; +uint32_t val, offset = addr; + +switch (offset) { +case R_I2CADR: /* ADDR */ +val = 0; +break; +case R_I2CFDR: /* Freq Div. */ +val = i2c->freq; +break; +case R_I2CCR: /* CONTROL */ +val = i2c->ctrl & ~0x06; +break; +case R_I2CSR: /* STATUS */ +val = i2c->sts; +break; +case R_I2CDR: /* DATA */ +/* Reads are "pipelined" and so return the previous value of the + * register + */ +val = i2c->dbuf; +if (I2CCR(MEN) && I2CSR(MBB)) { /* enabled and busy */ +if (!i2c_bus_busy(i2c->bus) || I2CCR(MTX)) { +LOG(LOG_GUEST_ERROR, "Read during addr or tx\n"); +i2c->dbuf = 0xff; +} else { +int ret = i2c_recv(i2c->bus); +i2c->dbuf = (uint8_t)ret; +DPRINTK(0, "READ %02x ('%c')\n", i2c->dbuf, (char)i2c->dbuf); +I2CSR_SET(MIF, 1); +I2CSR_SET(RXAK, 0); +mpc8540_update_irq(i2c); +} +} else { +i2c->dbuf = 0xff; +LOG(LOG_GUEST_ERROR, "Read when not enabled or busy\n"); +} +break; +case R_I2CDFSRR: /* FILTER */ +val = i2c->filt; +break; +default: +val = 0xff; +} + +DPRINTK(offset == 0xc ? 2 : 1, " read %08x -> %08x\n", +(unsigned)offset, (unsigned)val); +return val; +} + +static +void mpc8
[Qemu-devel] [PATCH 00/12] Add MVME3100 PPC SBC
This series adds simulation of MVME3100 powerpc SBCs, originally from Motorola, and now sold by Artesyn[1]. There are two variants differing in CPU speed and memory size. I've been working on this sporadically for the past 2 year. Recently I've finished all the features which I have in mind. If this series is accepted there is a continuation which adds VME bus. I've found it useful in software compatibility testing. I wonder if there is any interest at large? There are two main parts of this series. 1-5 are changing code common with the "ppce500" and "mpc8544ds" boards, with the remainder being additions. The changes are to how the CCSR region is handled in order to support the CCSRBAR register which allows the whole region to be relocated. Also added are a couple of memory and clock configuration registers which RTEMS guests read. #3 is actually a minor issue I found recently with the mpc8544 PCI host bridge, which I'm uncertain how to address. The host bridge device 0:0 identifies itself as a bridge, but doesn't properly implement the bridge config registers. This confuses Linux, which then does a full re-enumeration (successfully). The rest are additions of an I2C controller, an I2C eeprom, an I2C RTC, and new board code. My testing has been almost exclusively with an RTEMS guest[2]. Though I have recently done a little with Linux. RTEMS guests (and Linux too for now) require a stub bootloader[3] to put the system in the same state as the real bootloader. RTEMS has an unfortunately strong dependence on bootloader provided configuration (eg. it doesn't re-enumerate the PCI bus). [1] https://www.artesyn.com/computing/products/product/mvme3100-vme-board-with-freescale-mpc8540-system-on-chip-processor [2] https://www.rtems.org/ [3] https://github.com/mdavidsaver/qemu/wiki Michael Davidsaver (12): e500: add board config options e500: consolidate mpc8540 guts with e500-ccsr e500: note possible bug with host bridge e500: additional CCSR registers e500: name openpic and pci host bridge i2c: add mpc8540 i2c controller qtest: add e500_i2c_create() e500: add mpc8540 i2c controller to ccsr nvram: add AT24Cx i2c eeprom timer: add ds1375 RTC ppc: add mvme3100 machine tests: add mvme3100-test default-configs/ppc-softmmu.mak | 1 + hw/i2c/Makefile.objs| 1 + hw/i2c/mpc8540_i2c.c| 287 + hw/nvram/Makefile.objs | 1 + hw/nvram/eeprom_at24c.c | 205 hw/pci-host/ppce500.c | 13 +- hw/ppc/Makefile.objs| 4 +- hw/ppc/e500-ccsr.h | 17 - hw/ppc/e500.c | 59 ++-- hw/ppc/e500.h | 4 + hw/ppc/e500_ccsr.c | 220 + hw/ppc/e500plat.c | 2 + hw/ppc/mpc8544_guts.c | 143 - hw/ppc/mpc8544ds.c | 2 + hw/ppc/mvme3100.c | 688 hw/ppc/mvme3100_cpld.c | 192 +++ hw/timer/Makefile.objs | 1 + hw/timer/ds1375-i2c.c | 293 + tests/Makefile.include | 4 + tests/libqos/i2c-e500.c | 66 tests/libqos/i2c.h | 3 + tests/mvme3100-test.c | 79 + 22 files changed, 2083 insertions(+), 202 deletions(-) create mode 100644 hw/i2c/mpc8540_i2c.c create mode 100644 hw/nvram/eeprom_at24c.c delete mode 100644 hw/ppc/e500-ccsr.h create mode 100644 hw/ppc/e500_ccsr.c delete mode 100644 hw/ppc/mpc8544_guts.c create mode 100644 hw/ppc/mvme3100.c create mode 100644 hw/ppc/mvme3100_cpld.c create mode 100644 hw/timer/ds1375-i2c.c create mode 100644 tests/libqos/i2c-e500.c create mode 100644 tests/mvme3100-test.c -- 2.11.0
[Qemu-devel] [PATCH 05/12] e500: name openpic and pci host bridge
Signed-off-by: Michael Davidsaver --- hw/ppc/e500.c | 4 1 file changed, 4 insertions(+) diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c index 057be1751b..6f77844303 100644 --- a/hw/ppc/e500.c +++ b/hw/ppc/e500.c @@ -685,6 +685,8 @@ static DeviceState *ppce500_init_mpic_qemu(PPCE500Params *params, int i, j, k; dev = qdev_create(NULL, TYPE_OPENPIC); +object_property_add_child(qdev_get_machine(), "pic", OBJECT(dev), + &error_fatal); qdev_prop_set_uint32(dev, "model", params->mpic_version); qdev_prop_set_uint32(dev, "nb_cpus", smp_cpus); @@ -876,6 +878,8 @@ void ppce500_init(MachineState *machine, PPCE500Params *params) /* PCI */ dev = qdev_create(NULL, "e500-pcihost"); +object_property_add_child(qdev_get_machine(), "pci-host", OBJECT(dev), + &error_abort); qdev_prop_set_uint32(dev, "first_slot", params->pci_first_slot); qdev_prop_set_uint32(dev, "first_pin_irq", pci_irq_nrs[0]); qdev_init_nofail(dev); -- 2.11.0
[Qemu-devel] [PATCH 07/12] qtest: add e500_i2c_create()
Add interface for testing i2c devices with PPC e500. Signed-off-by: Michael Davidsaver --- tests/Makefile.include | 1 + tests/libqos/i2c-e500.c | 66 + tests/libqos/i2c.h | 3 +++ 3 files changed, 70 insertions(+) create mode 100644 tests/libqos/i2c-e500.c diff --git a/tests/Makefile.include b/tests/Makefile.include index c002352134..ad1c219423 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -721,6 +721,7 @@ libqos-pc-obj-y += tests/libqos/malloc-pc.o tests/libqos/libqos-pc.o libqos-pc-obj-y += tests/libqos/ahci.o libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o libqos-imx-obj-y = $(libqos-obj-y) tests/libqos/i2c-imx.o +libqos-e500-obj-y = $(libqos-obj-y) tests/libqos/i2c-e500.o libqos-usb-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/usb.o libqos-virtio-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o tests/libqos/virtio-mmio.o tests/libqos/malloc-generic.o diff --git a/tests/libqos/i2c-e500.c b/tests/libqos/i2c-e500.c new file mode 100644 index 00..4272ada0a5 --- /dev/null +++ b/tests/libqos/i2c-e500.c @@ -0,0 +1,66 @@ +/* + * QTest I2C driver + * + * Copyright (c) 2016 Michael Davidsaver + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ +#include "qemu/osdep.h" +#include "libqos/i2c.h" + + +#include "qemu/bswap.h" +#include "libqtest.h" + +typedef struct E500I2C { +I2CAdapter parent; + +uint64_t addr; +} E500I2C; + +static void e500_i2c_send(I2CAdapter *i2c, uint8_t addr, + const uint8_t *buf, uint16_t len) +{ +E500I2C *s = (E500I2C *)i2c; + +writeb(s->addr + 0x8, 0xb0); /* Enable and START a write */ +writeb(s->addr + 0x10, addr & 0xfe); /* Send address for write */ + +while (len--) { +writeb(s->addr + 0x10, *buf++); +} + +writeb(s->addr + 0x8, 0x80); /* STOP but leave enabled */ +} + +static void e500_i2c_recv(I2CAdapter *i2c, uint8_t addr, + uint8_t *buf, uint16_t len) +{ +E500I2C *s = (E500I2C *)i2c; + +writeb(s->addr + 0x8, 0xa0); /* Enable and START a read */ +writeb(s->addr + 0x10, addr | 1); /* Send address for read */ + +/* reads are "pipelined" so the initial value is junk */ +readb(s->addr + 0x10); + +while (len--) { +*buf++ = readb(s->addr + 0x10); +} + +writeb(s->addr + 0x8, 0x80); /* STOP but leave enabled */ +} + +I2CAdapter *e500_i2c_create(uint64_t ccsr_base) +{ +E500I2C *s = g_malloc0(sizeof(*s)); +I2CAdapter *i2c = (I2CAdapter *)s; + +s->addr = ccsr_base + 0x3000; + +i2c->send = e500_i2c_send; +i2c->recv = e500_i2c_recv; + +return i2c; +} diff --git a/tests/libqos/i2c.h b/tests/libqos/i2c.h index 6e648f922a..40c59a7997 100644 --- a/tests/libqos/i2c.h +++ b/tests/libqos/i2c.h @@ -29,4 +29,7 @@ I2CAdapter *omap_i2c_create(uint64_t addr); /* libi2c-imx.c */ I2CAdapter *imx_i2c_create(uint64_t addr); +/* i2c-e500.c */ +I2CAdapter *e500_i2c_create(uint64_t ccsr_base); + #endif -- 2.11.0
[Qemu-devel] [PATCH 12/12] tests: add mvme3100-test
Exercise some features of the mvme3100 CPLD logic and read from the eeprom w/ VPD. Signed-off-by: Michael Davidsaver --- tests/Makefile.include | 3 ++ tests/mvme3100-test.c | 79 ++ 2 files changed, 82 insertions(+) create mode 100644 tests/mvme3100-test.c diff --git a/tests/Makefile.include b/tests/Makefile.include index ad1c219423..7ea041a885 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -372,6 +372,8 @@ check-qtest-s390x-y += tests/virtio-balloon-test$(EXESUF) check-qtest-s390x-y += tests/virtio-console-test$(EXESUF) check-qtest-s390x-y += tests/virtio-serial-test$(EXESUF) +check-qtest-ppc-$(CONFIG_E500) += tests/mvme3100-test$(EXESUF) + check-qtest-generic-y += tests/qom-test$(EXESUF) check-qtest-generic-y += tests/test-hmp$(EXESUF) @@ -781,6 +783,7 @@ tests/i82801b11-test$(EXESUF): tests/i82801b11-test.o tests/ac97-test$(EXESUF): tests/ac97-test.o tests/es1370-test$(EXESUF): tests/es1370-test.o tests/intel-hda-test$(EXESUF): tests/intel-hda-test.o +tests/mvme3100-test$(EXESUF): tests/mvme3100-test.o $(libqos-e500-obj-y) tests/ioh3420-test$(EXESUF): tests/ioh3420-test.o tests/usb-hcd-ohci-test$(EXESUF): tests/usb-hcd-ohci-test.o $(libqos-usb-obj-y) tests/usb-hcd-uhci-test$(EXESUF): tests/usb-hcd-uhci-test.o $(libqos-usb-obj-y) diff --git a/tests/mvme3100-test.c b/tests/mvme3100-test.c new file mode 100644 index 00..6dde8d1d29 --- /dev/null +++ b/tests/mvme3100-test.c @@ -0,0 +1,79 @@ +#include + +#include "qemu/osdep.h" +#include "libqtest.h" +#include "libqos/libqos.h" +#include "libqos/i2c.h" + +#define assert_equal(A, B) g_assert_cmphex((A), ==, (B)) + +static +I2CAdapter *i2c; + +static +void test_ccsr(void) +{ +/* CCSRBAR is self referential */ +assert_equal(readl(0xff70), 0x000ff700); + +/* introspect memory size */ +assert_equal(readl(0xff702080), 0x8000); +/* value is (ram_size-1)>>24 */ +assert_equal(readl(0xff702000), 15); +} + +static +void test_cpld(void) +{ +/* read/write to test register */ +assert_equal(readl(0xe210), 0x); +assert_equal(readl(0xe214), 0x); + +writel(0xe210, 0x12345678); + +assert_equal(readl(0xe210), 0x12345678); +assert_equal(readl(0xe214), 0x12345678 ^ 0x); +} + +static +void test_eeprom(void) +{ +char buf[] = "\x00\x00MOTOROLA"; + +/* 1. zero address pointer + * 2. write 8 bytes, + * 3. re-zero address pointer + */ +i2c_send(i2c, 0xa8, (uint8_t *)buf, 10); +i2c_send(i2c, 0xa8, (uint8_t *)buf, 2); + +/* read 8 bytes */ +i2c_recv(i2c, 0xa8, (uint8_t *)buf, 8); +buf[8] = '\0'; + +/* Read header for Motorola VPD info */ +g_assert_cmpstr(buf, ==, "MOTOROLA"); +} + +int main(int argc, char *argv[]) +{ +int ret; +g_test_init(&argc, &argv, NULL); + +qtest_start("-machine mvme3100-1152"); + +i2c = e500_i2c_create(0xff70); + +qtest_add_func("/mvme3100/ccsr", test_ccsr); +qtest_add_func("/mvme3100/cpld", test_cpld); +qtest_add_func("/mvme3100/eeprom", test_eeprom); + +ret = g_test_run(); + +printf("Tests done\n"); + +qtest_end(); +printf("Tests end\n"); + +return ret; +} -- 2.11.0
[Qemu-devel] [PATCH 09/12] nvram: add AT24Cx i2c eeprom
Signed-off-by: Michael Davidsaver --- hw/nvram/Makefile.objs | 1 + hw/nvram/eeprom_at24c.c | 205 2 files changed, 206 insertions(+) create mode 100644 hw/nvram/eeprom_at24c.c diff --git a/hw/nvram/Makefile.objs b/hw/nvram/Makefile.objs index c018f6b2ff..0f4ee71dcb 100644 --- a/hw/nvram/Makefile.objs +++ b/hw/nvram/Makefile.objs @@ -1,5 +1,6 @@ common-obj-$(CONFIG_DS1225Y) += ds1225y.o common-obj-y += eeprom93xx.o +common-obj-y += eeprom_at24c.o common-obj-y += fw_cfg.o common-obj-y += chrp_nvram.o common-obj-$(CONFIG_MAC_NVRAM) += mac_nvram.o diff --git a/hw/nvram/eeprom_at24c.c b/hw/nvram/eeprom_at24c.c new file mode 100644 index 00..efa3621ac6 --- /dev/null +++ b/hw/nvram/eeprom_at24c.c @@ -0,0 +1,205 @@ +/* + * *AT24C* series I2C EEPROM + * + * Copyright (c) 2015 Michael Davidsaver + * + * This work is licensed under the terms of the GNU GPL, version 2. See + * the LICENSE file in the top-level directory. + */ + +#include + +#include "qemu/osdep.h" +#include "qapi/error.h" +#include "hw/hw.h" +#include "hw/i2c/i2c.h" +#include "sysemu/block-backend.h" + +/* #define DEBUG_AT24C */ + +#ifdef DEBUG_AT24C +#define DPRINTK(FMT, ...) printf(TYPE_AT24C_EE " : " FMT, ## __VA_ARGS__) +#else +#define DPRINTK(FMT, ...) do {} while (0) +#endif + +#define ERR(FMT, ...) fprintf(stderr, TYPE_AT24C_EE " : " FMT, \ +## __VA_ARGS__) + +#define TYPE_AT24C_EE "at24c-eeprom" +#define AT24C_EE(obj) OBJECT_CHECK(EEPROMState, (obj), TYPE_AT24C_EE) + +typedef struct EEPROMState { +I2CSlave parent_obj; + +/* address counter */ +uint16_t cur; +/* total size in bytes */ +uint32_t rsize; +bool writable; +/* cells changed since last START? */ +bool changed; +/* during WRITE, # of address bytes transfered */ +uint8_t haveaddr; + +uint8_t *mem; + +BlockBackend *blk; +} EEPROMState; + +static +int at24c_eeprom_event(I2CSlave *s, enum i2c_event event) +{ +EEPROMState *ee = container_of(s, EEPROMState, parent_obj); + +switch (event) { +case I2C_START_SEND: +case I2C_START_RECV: +case I2C_FINISH: +ee->haveaddr = 0; +DPRINTK("clear\n"); +if (ee->blk && ee->changed) { +int len = blk_pwrite(ee->blk, 0, ee->mem, ee->rsize, 0); +if (len != ee->rsize) { +ERR(TYPE_AT24C_EE +" : failed to write backing file\n"); +} +DPRINTK("Wrote to backing file\n"); +} +ee->changed = false; +break; +case I2C_NACK: +break; +} +return 0; +} + +static +int at24c_eeprom_recv(I2CSlave *s) +{ +EEPROMState *ee = AT24C_EE(s); +int ret; + +ret = ee->mem[ee->cur]; + +ee->cur = (ee->cur + 1u) % ee->rsize; +DPRINTK("Recv %02x %c\n", ret, ret); + +return ret; +} + +static +int at24c_eeprom_send(I2CSlave *s, uint8_t data) +{ +EEPROMState *ee = AT24C_EE(s); + +if (ee->haveaddr < 2) { +ee->cur <<= 8; +ee->cur |= data; +ee->haveaddr++; +if (ee->haveaddr == 2) { +ee->cur %= ee->rsize; +DPRINTK("Set pointer %04x\n", ee->cur); +} + +} else { +if (ee->writable) { +DPRINTK("Send %02x\n", data); +ee->mem[ee->cur] = data; +ee->changed = true; +} else { +DPRINTK("Send error %02x read-only\n", data); +} +ee->cur = (ee->cur + 1u) % ee->rsize; + +} + +return 0; +} + +static +int at24c_eeprom_init(I2CSlave *i2c) +{ +EEPROMState *ee = AT24C_EE(i2c); + +ee->mem = g_malloc0(ee->rsize); + +if (ee->blk) { +int64_t len = blk_getlength(ee->blk); + +if (len != ee->rsize) { +ERR(TYPE_AT24C_EE " : Backing file size %lu != %u\n", +(unsigned long)len, (unsigned)ee->rsize); +exit(1); +} + +if (blk_set_perm(ee->blk, BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE, + BLK_PERM_ALL, &error_fatal) < 0) +{ +ERR(TYPE_AT24C_EE +" : Backing file incorrect permission\n"); +exit(1); +} +} +return 0; +} + +static +void at24c_eeprom_reset(DeviceState *state) +{ +EEPROMState *ee = AT24C_EE(state); + +ee->changed = false; +ee->cur = 0; +ee->haveaddr = 0; + +memset(ee->mem, 0, ee->rsize); + +if (ee->blk) { +int len = blk_pread(ee->blk, 0, ee->mem, ee->rsize); + +if (len != ee->rsize) { +ERR(TYPE_AT24C_EE +" : Failed initial sync with
[Qemu-devel] [PATCH 11/12] ppc: add mvme3100 machine
Signed-off-by: Michael Davidsaver --- hw/ppc/Makefile.objs | 1 + hw/ppc/mvme3100.c | 688 + hw/ppc/mvme3100_cpld.c | 192 ++ 3 files changed, 881 insertions(+) create mode 100644 hw/ppc/mvme3100.c create mode 100644 hw/ppc/mvme3100_cpld.c diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs index c1a63d0c39..c1118aaa42 100644 --- a/hw/ppc/Makefile.objs +++ b/hw/ppc/Makefile.objs @@ -26,5 +26,6 @@ obj-$(CONFIG_MAC) += mac_newworld.o obj-$(CONFIG_E500) += e500.o mpc8544ds.o e500plat.o obj-$(CONFIG_E500) += ppce500_spin.o obj-$(CONFIG_E500) += e500_ccsr.o +obj-$(CONFIG_E500) += mvme3100.o mvme3100_cpld.o # PowerPC 440 Xilinx ML507 reference board. obj-$(CONFIG_XILINX) += virtex_ml507.o diff --git a/hw/ppc/mvme3100.c b/hw/ppc/mvme3100.c new file mode 100644 index 00..2e6d428533 --- /dev/null +++ b/hw/ppc/mvme3100.c @@ -0,0 +1,688 @@ +/* + * MVME3100 board emulation + * + * Copyright (c) 2015 Michael Davidsaver + * + * This work is licensed under the terms of the GNU GPL, version 2. See + * the LICENSE file in the top-level directory. + * + * This model was developed according to the + * MVME3100 Single Board Computer Programmer's Reference + * P/N: 6806800G37B + * July 2014 + * + * mvme3100-1152 + * 677MHz core, 256MB ram, 64MB flash + * mvme3100-1263 + * 833MHz core, 512MB ram, 128MB flash + * + * MOTLoad on mvme3100-1152 says: + * MPU-Type =MPC8540 + * MPU-Int Clock Speed =666MHz + * MPU-CCB Clock Speed =333MHz + * MPU-DDR Clock Speed =166MHz + * MPU-PCI Clock Speed =66MHz, PCI, 64-bit + * MPU-Int Cache(L2) Enabled, 256KB, L2CTL =A8000300 + * Reset/Boot Vector=Flash0 + * Local Memory Found =1000 (&268435456) + * + * MOTLoad on mvme3100-1263 says: + * MPU-Type =MPC8540 + * MPU-Int Clock Speed =833MHz + * MPU-CCB Clock Speed =333MHz + * MPU-DDR Clock Speed =166MHz + * MPU-PCI Clock Speed =66MHz, PCI, 64-bit + * MPU-Int Cache(L2) Enabled, 256KB, L2CTL =A8000300 + * Reset/Boot Vector=Flash0 + * Local Memory Found =2000 (&536870912) + * + * Clock ratios + * CCB/PCI -> 5/1 + * core/CCB -> 2/1 (-1152) + *-> 5/2 (-1263) + * + * The overall memory map is determined by the Local Address Windows. + * We do not model the LAWs explicitly. + * + * MOTLoad configures as follows (a super set of table 1-4) + * (MOTLoad RTOS Version 2.0, PAL Version 1.2 RM04) + * LAW 0, 7 - disabled + * LAW 1 - 0x -> 0x7fff - RAM 2G + * LAW 2 - 0x8000 -> 0xbfff - PCI 1G + * LAW 3 - 0xc000 -> 0xdfff - PCI 512MB + * LAW 4 - 0xe000 -> 0xe0ff - PCI 16MB + * gap - 0xe100 -> 0xbfff - CCSR @ 0xe100 + * LAW 5 - 0xe200 -> 0xe2ff - LBC 16MB + * gap - 0xe300 -> 0xefff + * LAW 6 - 0xf000 -> 0x - LBC 256MB + * + * And validated against the RTEMS 4.9.6 mvme3100 BSP + */ +#include "qemu/osdep.h" +#include "qemu/log.h" +#include "qapi/error.h" +#include "qapi/visitor.h" +#include "e500.h" +#include "cpu.h" +#include "qemu-common.h" +#include "cpu-qom.h" +#include "sysemu/sysemu.h" +#include "sysemu/dma.h" +#include "sysemu/block-backend.h" +#include "hw/loader.h" +#include "hw/pci/pci.h" +#include "hw/boards.h" +#include "hw/ppc/ppc.h" +#include "hw/net/fsl_etsec/etsec.h" +#include "sysemu/device_tree.h" +#include "sysemu/qtest.h" +#include "hw/ppc/openpic.h" +#include "qemu/error-report.h" + +/* Same as prep.c and other PPC boards */ +#define CFG_ADDR 0xf510 + +#define TYPE_MVME3100 MACHINE_TYPE_NAME("mvme3100") +#define MVME3100(obj) OBJECT_CHECK(MVME3100State, (obj), TYPE_MVME3100) +#define MVME3100_GET_CLASS(obj) \ +OBJECT_GET_CLASS(MVME3100Class, (obj), TYPE_MVME3100) +#define MVME3100_CLASS(klass) \ +OBJECT_CLASS_CHECK(MVME3100Class, (klass), TYPE_MVME3100) + + +typedef struct mvme3100_info { +const char *desc; +uint32_t cpu_freq; +uint32_t porpllsr; +uint32_t ram_size; +} mvme3100_info; + +typedef struct MVME3100Class { +/*< private >*/ +MachineClass parent_class; +/*< public >*/ + +const mvme3100_info *info; +} MVME3100Class; + +typedef struct MVME3100State { +/*< private >*/ +MachineState parent_obj; +/*< public >*/ + +uint32_t load_address, + entry_address; +} MVME3100State; + + +/* motload "global environment" variables */ +static +const char *gev[] = { +/* TODO: somehow snoop in slirp_instances to pick up IP config? */ +"mot-/dev/enet0-cipa=10.0.2.15", +"mot-/dev/enet0-gipa=10.0.2.2", +"mot-/dev/enet0-snma=255.255.255.0", +"mot-/dev
[Qemu-devel] [PATCH 10/12] timer: add ds1375 RTC
only basic functionality implemented (read time and sram). no set time or alarms. Signed-off-by: Michael Davidsaver --- default-configs/ppc-softmmu.mak | 1 + hw/timer/Makefile.objs | 1 + hw/timer/ds1375-i2c.c | 293 3 files changed, 295 insertions(+) create mode 100644 hw/timer/ds1375-i2c.c diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak index bb225c6e46..04bfa79154 100644 --- a/default-configs/ppc-softmmu.mak +++ b/default-configs/ppc-softmmu.mak @@ -52,3 +52,4 @@ CONFIG_SERIAL_ISA=y CONFIG_MC146818RTC=y CONFIG_ISA_TESTDEV=y CONFIG_RS6000_MC=y +CONFIG_DS1375=y diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs index 8c19eac3b6..6521d47367 100644 --- a/hw/timer/Makefile.objs +++ b/hw/timer/Makefile.objs @@ -4,6 +4,7 @@ common-obj-$(CONFIG_ARM_V7M) += armv7m_systick.o common-obj-$(CONFIG_A9_GTIMER) += a9gtimer.o common-obj-$(CONFIG_CADENCE) += cadence_ttc.o common-obj-$(CONFIG_DS1338) += ds1338.o +common-obj-$(CONFIG_DS1375) += ds1375-i2c.o common-obj-$(CONFIG_HPET) += hpet.o common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o common-obj-$(CONFIG_M48T59) += m48t59.o diff --git a/hw/timer/ds1375-i2c.c b/hw/timer/ds1375-i2c.c new file mode 100644 index 00..dba9cc05c4 --- /dev/null +++ b/hw/timer/ds1375-i2c.c @@ -0,0 +1,293 @@ +/* + * Dallas/Maxim ds1375 I2C RTC w/ SRAM + * + * Copyright (c) 2017 Michael Davidsaver + * + * This work is licensed under the terms of the GNU GPL, version 2. See + * the LICENSE file in the top-level directory. + * + * Only basic functionality is modeled (time and user SRAM). + * Alarms not modeled. + */ +#include "qemu/osdep.h" +#include "qemu-common.h" +#include "qemu/log.h" +#include "qemu/timer.h" +#include "qemu/bcd.h" +#include "hw/hw.h" +#include "hw/registerfields.h" +#include "hw/i2c/i2c.h" + +#define DEBUG_DS1375 + +#ifdef DEBUG_DS1375 +#define DPRINTK(FMT, ...) printf(TYPE_DS1375 " : " FMT, ## __VA_ARGS__) +#else +#define DPRINTK(FMT, ...) do {} while (0) +#endif + +#define LOG(MSK, FMT, ...) qemu_log_mask(MSK, TYPE_DS1375 " : " FMT, \ +## __VA_ARGS__) + +#define TYPE_DS1375 "ds1375" +#define DS1375(obj) OBJECT_CHECK(DS1375State, (obj), TYPE_DS1375) + +#define DS1375_REGSIZE 0x20 + +#define R_SEC (0x0) +#define R_MIN (0x1) +#define R_HOUR (0x2) +#define R_WDAY (0x3) +#define R_DATE (0x4) +#define R_MONTH (0x5) +#define R_YEAR (0x6) +#define R_A1SEC (0x7) +#define R_A1MIN (0x8) +#define R_A1HOUR (0x9) +#define R_A1DAY (0xa) +#define R_A2SEC (0xb) +#define R_A2MIN (0xc) +#define R_A2HOUR (0xd) +#define R_CTRL (0xe) +#define R_STS (0xf) + +FIELD(HOUR, SET12, 6, 1) +FIELD(HOUR, HOUR24, 0, 6) +FIELD(HOUR, AMPM, 5, 1) +FIELD(HOUR, HOUR12, 0, 5) + +FIELD(MONTH, MONTH, 0, 5) +FIELD(MONTH, CENTURY, 7, 1) + +FIELD(CTRL, ECLK, 7, 1) +FIELD(CTRL, CLKSEL, 5, 2) +FIELD(CTRL, RS, 3, 2) +FIELD(CTRL, INTCN, 2, 1) +FIELD(CTRL, A2IE, 1, 1) +FIELD(CTRL, A1IE, 0, 1) + +typedef struct DS1375State { +I2CSlave parent_obj; + +/* register address counter */ +uint8_t addr; +/* when writing, whether the address has been sent */ +bool addrd; + +int time_offset; + +uint8_t regs[DS1375_REGSIZE]; +} DS1375State; + +/* update current time register if clock enabled */ +static +void ds1375_latch(DS1375State *ds) +{ +struct tm now; + +if (!ARRAY_FIELD_EX32(ds->regs, CTRL, ECLK)) { +return; +} + +qemu_get_timedate(&now, ds->time_offset); + +DPRINTK("Current Time %3u/%2u/%u %2u:%2u:%2u (wday %u)\n", +now.tm_year, now.tm_mon, now.tm_mday, +now.tm_hour, now.tm_min, now.tm_sec, +now.tm_wday); + +/* ensure unused bits are zero */ +memset(ds->regs, 0, R_YEAR + 1); + +ds->regs[R_SEC] = to_bcd(now.tm_sec); +ds->regs[R_MIN] = to_bcd(now.tm_min); + +if (ARRAY_FIELD_EX32(ds->regs, HOUR, SET12) == 0) { +/* 24 hour */ +ARRAY_FIELD_DP32(ds->regs, HOUR, HOUR24, to_bcd(now.tm_hour)); +} else { +/* 12 hour am/pm */ +ARRAY_FIELD_DP32(ds->regs, HOUR, AMPM, now.tm_hour >= 12); +ARRAY_FIELD_DP32(ds->regs, HOUR, HOUR12, to_bcd(now.tm_hour % 12u)); +} + +ds->regs[R_WDAY] = now.tm_wday; /* day of the week */ +ds->regs[R_DATE] = to_bcd(now.tm_mday); + +ARRAY_FIELD_DP32(ds->regs, MONTH, MONTH, to_bcd(now.tm_mon + 1)); +ARRAY_FIELD_DP32(ds->regs, MONTH, CENTURY, now.tm_year > 99); + +ds->regs[R_YEAR] = to_bcd(now.tm_year % 100u); + +DPRINTK("Latched time\n"); +} + +static +void ds1375_update(DS1375State *ds) +{ +struct tm now; + +now.tm_sec = from_bcd(ds->regs[R_SEC]); +now.tm_min = from_bcd(ds->regs[R_MIN]); + +if (ARRAY_FIELD_EX32(ds->regs, HOUR, SET12
Re: [Qemu-devel] [PATCH 03/12] e500: note possible bug with host bridge
On 11/21/2017 09:46 PM, David Gibson wrote: > On Sun, Nov 19, 2017 at 09:24:11PM -0600, Michael Davidsaver wrote: >> Signed-off-by: Michael Davidsaver > > I'm not sure if you're saying you think there is a hardware bug which > we're faithfully emulating, or a software bug. I mean that the emulation is incorrect in that it just sets config[PCI_HEADER_TYPE]==PCI_HEADER_TYPE_BRIDGE but does none of the other initialization of the base-pci-bridge class. I specifically observed Linux being confused by the fact that the primary, secondary, and subordinate bus registers don't work right because they're actually the BAR2 address register. Further, it seems odd that a host bridge would identify itself as a pci-to-pci bridge. The mpc8540 doesn't. The mpc8544 docs aren't clear, and I don't have a real one to test. My inclination is to remove the line changing PCI_HEADER_TYPE, but I'm hesitant about breaking things. Especially since this doesn't trigger mis-behavior in Linux or RTEMS. >> --- >> hw/pci-host/ppce500.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c >> index f2d108bc8a..0e2833bd98 100644 >> --- a/hw/pci-host/ppce500.c >> +++ b/hw/pci-host/ppce500.c >> @@ -424,6 +424,9 @@ static void e500_pcihost_bridge_realize(PCIDevice *d, >> Error **errp) >> MemoryRegion *ccsr_mr = sysbus_mmio_get_region(ccsr, 0); >> >> pci_config_set_class(d->config, PCI_CLASS_BRIDGE_PCI); >> +/* BUG? identifies as PCI_HEADER_TYPE_BRIDGE but uses >> + * standard device config read/write >> + */ >> d->config[PCI_HEADER_TYPE] = >> (d->config[PCI_HEADER_TYPE] & PCI_HEADER_TYPE_MULTI_FUNCTION) | >> PCI_HEADER_TYPE_BRIDGE; > signature.asc Description: OpenPGP digital signature