* [Qemu-devel] [PATCH] Exit on reset for armv7-m
@ 2015-10-08 15:40 Michael Davidsaver
2015-10-08 15:40 ` [Qemu-devel] [PATCH] armv7-m: exit on external reset request Michael Davidsaver
2015-10-08 20:09 ` [Qemu-devel] [PATCH] Exit on reset for armv7-m Peter Crosthwaite
0 siblings, 2 replies; 20+ messages in thread
From: Michael Davidsaver @ 2015-10-08 15:40 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Michael Davidsaver
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH] armv7-m: exit on external reset request
2015-10-08 15:40 [Qemu-devel] [PATCH] Exit on reset for armv7-m Michael Davidsaver
@ 2015-10-08 15:40 ` Michael Davidsaver
2015-10-09 16:59 ` Peter Maydell
2015-10-08 20:09 ` [Qemu-devel] [PATCH] Exit on reset for armv7-m Peter Crosthwaite
1 sibling, 1 reply; 20+ messages in thread
From: Michael Davidsaver @ 2015-10-08 15:40 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Michael Davidsaver
Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH] Exit on reset for armv7-m
2015-10-08 15:40 [Qemu-devel] [PATCH] Exit on reset for armv7-m Michael Davidsaver
2015-10-08 15:40 ` [Qemu-devel] [PATCH] armv7-m: exit on external reset request Michael Davidsaver
@ 2015-10-08 20:09 ` Peter Crosthwaite
2015-10-09 1:24 ` Michael Davidsaver
1 sibling, 1 reply; 20+ messages in thread
From: Peter Crosthwaite @ 2015-10-08 20:09 UTC (permalink / raw)
To: Michael Davidsaver; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
On Thu, Oct 8, 2015 at 8:40 AM, Michael Davidsaver
<mdavidsaver@gmail.com> 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).
> 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.
> 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.
Regards,
Peter
> armv7-m: exit on external reset request
>
> hw/intc/armv7m_nvic.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> --
> 2.1.4
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH] Exit on reset for armv7-m
2015-10-08 20:09 ` [Qemu-devel] [PATCH] Exit on reset for armv7-m Peter Crosthwaite
@ 2015-10-09 1:24 ` Michael Davidsaver
2015-10-09 4:21 ` Peter Crosthwaite
0 siblings, 1 reply; 20+ messages in thread
From: Michael Davidsaver @ 2015-10-09 1:24 UTC (permalink / raw)
To: Peter Crosthwaite; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
On 10/08/2015 04:09 PM, Peter Crosthwaite wrote:
> On Thu, Oct 8, 2015 at 8:40 AM, Michael Davidsaver
> <mdavidsaver@gmail.com> 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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH] Exit on reset for armv7-m
2015-10-09 1:24 ` Michael Davidsaver
@ 2015-10-09 4:21 ` Peter Crosthwaite
0 siblings, 0 replies; 20+ messages in thread
From: Peter Crosthwaite @ 2015-10-09 4:21 UTC (permalink / raw)
To: Michael Davidsaver; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
On Thu, Oct 8, 2015 at 6:24 PM, Michael Davidsaver
<mdavidsaver@gmail.com> wrote:
> On 10/08/2015 04:09 PM, Peter Crosthwaite wrote:
>> On Thu, Oct 8, 2015 at 8:40 AM, Michael Davidsaver
>> <mdavidsaver@gmail.com> 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.
>
I would resend as a single. But I am yet to get around to a proper
review of the patch proper, as I need to dig out relavent docs. So if
you want a day or two I might have more comments.
Regards,
Peter
>>> 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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH] armv7-m: exit on external reset request
2015-10-08 15:40 ` [Qemu-devel] [PATCH] armv7-m: exit on external reset request Michael Davidsaver
@ 2015-10-09 16:59 ` Peter Maydell
2015-10-09 17:25 ` Michael Davidsaver
0 siblings, 1 reply; 20+ messages in thread
From: Peter Maydell @ 2015-10-09 16:59 UTC (permalink / raw)
To: Michael Davidsaver; +Cc: QEMU Developers
On 8 October 2015 at 16:40, Michael Davidsaver <mdavidsaver@gmail.com> wrote:
> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
> ---
> 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) {
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?
thanks
-- PMM
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH] armv7-m: exit on external reset request
2015-10-09 16:59 ` Peter Maydell
@ 2015-10-09 17:25 ` Michael Davidsaver
2015-10-09 18:18 ` Peter Crosthwaite
0 siblings, 1 reply; 20+ messages in thread
From: Michael Davidsaver @ 2015-10-09 17:25 UTC (permalink / raw)
To: Peter Maydell; +Cc: QEMU Developers
On 10/09/2015 12:59 PM, Peter Maydell wrote:
> On 8 October 2015 at 16:40, Michael Davidsaver <mdavidsaver@gmail.com> 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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH] armv7-m: exit on external reset request
2015-10-09 17:25 ` Michael Davidsaver
@ 2015-10-09 18:18 ` Peter Crosthwaite
2015-10-09 18:51 ` Michael Davidsaver
0 siblings, 1 reply; 20+ messages in thread
From: Peter Crosthwaite @ 2015-10-09 18:18 UTC (permalink / raw)
To: Michael Davidsaver; +Cc: Peter Maydell, QEMU Developers
On Fri, Oct 9, 2015 at 10:25 AM, Michael Davidsaver
<mdavidsaver@gmail.com> wrote:
>
>
> On 10/09/2015 12:59 PM, Peter Maydell wrote:
>> On 8 October 2015 at 16:40, Michael Davidsaver <mdavidsaver@gmail.com> 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.
>
We should check the lm3s docs to see what the implementation of this
signal is. 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.
> 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)
>
Nice!
Regards,
Peter
>
> 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
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH] armv7-m: exit on external reset request
2015-10-09 18:18 ` Peter Crosthwaite
@ 2015-10-09 18:51 ` Michael Davidsaver
2015-10-10 14:35 ` Michael Davidsaver
0 siblings, 1 reply; 20+ messages in thread
From: Michael Davidsaver @ 2015-10-09 18:51 UTC (permalink / raw)
To: Peter Crosthwaite; +Cc: Peter Maydell, QEMU Developers
On 10/09/2015 02:18 PM, Peter Crosthwaite wrote:
> On Fri, Oct 9, 2015 at 10:25 AM, Michael Davidsaver
> <mdavidsaver@gmail.com> wrote:
>>
>>
>> On 10/09/2015 12:59 PM, Peter Maydell wrote:
>>> On 8 October 2015 at 16:40, Michael Davidsaver <mdavidsaver@gmail.com> 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...)
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH] armv7-m: exit on external reset request
2015-10-09 18:51 ` Michael Davidsaver
@ 2015-10-10 14:35 ` Michael Davidsaver
2015-10-10 18:54 ` [Qemu-devel] [PATCH v2] " Michael Davidsaver
0 siblings, 1 reply; 20+ messages in thread
From: Michael Davidsaver @ 2015-10-10 14:35 UTC (permalink / raw)
To: Peter Crosthwaite; +Cc: Peter Maydell, QEMU Developers
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
>> <mdavidsaver@gmail.com> wrote:
>>>
>>>
>>> On 10/09/2015 12:59 PM, Peter Maydell wrote:
>>>> On 8 October 2015 at 16:40, Michael Davidsaver <mdavidsaver@gmail.com> 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*?
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v2] armv7-m: exit on external reset request
2015-10-10 14:35 ` Michael Davidsaver
@ 2015-10-10 18:54 ` Michael Davidsaver
2015-10-11 15:06 ` Peter Crosthwaite
0 siblings, 1 reply; 20+ messages in thread
From: Michael Davidsaver @ 2015-10-10 18:54 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Peter Crosthwaite, Michael Davidsaver
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, 0xfffff000, 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, 0x20000000, 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 & (0x10000 << i)) {
dev = sysbus_create_simple(TYPE_STELLARIS_GPTM,
0x40030000 + 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 +1290,7 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model,
}
if (board->dc2 & (1 << 12)) {
- dev = sysbus_create_simple(TYPE_STELLARIS_I2C, 0x40020000, pic[8]);
+ dev = sysbus_create_simple(TYPE_STELLARIS_I2C, 0x40020000, qdev_get_gpio_in(nvic, 8));
i2c = (I2CBus *)qdev_get_child_bus(dev, "i2c");
if (board->peripherals & BP_OLED_I2C) {
i2c_create_slave(i2c, "ssd0303", 0x3d);
@@ -1286,11 +1300,11 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model,
for (i = 0; i < 4; i++) {
if (board->dc2 & (1 << i)) {
sysbus_create_simple("pl011_luminary", 0x4000c000 + i * 0x1000,
- pic[uart_irq[i]]);
+ qdev_get_gpio_in(nvic, uart_irq[i]));
}
}
if (board->dc2 & (1 << 4)) {
- dev = sysbus_create_simple("pl022", 0x40008000, pic[7]);
+ dev = sysbus_create_simple("pl022", 0x40008000, qdev_get_gpio_in(nvic, 7));
if (board->peripherals & BP_OLED_SSI) {
void *bus;
DeviceState *sddev;
@@ -1326,7 +1340,7 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model,
qdev_set_nic_properties(enet, &nd_table[0]);
qdev_init_nofail(enet);
sysbus_mmio_map(SYS_BUS_DEVICE(enet), 0, 0x40048000);
- sysbus_connect_irq(SYS_BUS_DEVICE(enet), 0, pic[42]);
+ sysbus_connect_irq(SYS_BUS_DEVICE(enet), 0, qdev_get_gpio_in(nvic, 42));
}
if (board->peripherals & BP_GAMEPAD) {
qemu_irq gpad_irq[5];
diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
index 4d26a7e..b3e4c31 100644
--- a/hw/arm/stm32f205_soc.c
+++ b/hw/arm/stm32f205_soc.c
@@ -59,9 +59,8 @@ static void stm32f205_soc_initfn(Object *obj)
static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
{
STM32F205State *s = STM32F205_SOC(dev_soc);
- DeviceState *syscfgdev, *usartdev, *timerdev;
+ DeviceState *syscfgdev, *usartdev, *timerdev, *nvic;
SysBusDevice *syscfgbusdev, *usartbusdev, *timerbusdev;
- qemu_irq *pic;
Error *err = NULL;
int i;
@@ -88,8 +87,8 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
vmstate_register_ram_global(sram);
memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, sram);
- pic = armv7m_init(get_system_memory(), FLASH_SIZE, 96,
- s->kernel_filename, s->cpu_model);
+ nvic = armv7m_init(get_system_memory(), FLASH_SIZE, 96,
+ s->kernel_filename, s->cpu_model);
/* System configuration controller */
syscfgdev = DEVICE(&s->syscfg);
@@ -100,7 +99,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
}
syscfgbusdev = SYS_BUS_DEVICE(syscfgdev);
sysbus_mmio_map(syscfgbusdev, 0, 0x40013800);
- sysbus_connect_irq(syscfgbusdev, 0, pic[71]);
+ sysbus_connect_irq(syscfgbusdev, 0, qdev_get_gpio_in(nvic, 71));
/* Attach UART (uses USART registers) and USART controllers */
for (i = 0; i < STM_NUM_USARTS; i++) {
@@ -112,7 +111,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
}
usartbusdev = SYS_BUS_DEVICE(usartdev);
sysbus_mmio_map(usartbusdev, 0, usart_addr[i]);
- sysbus_connect_irq(usartbusdev, 0, pic[usart_irq[i]]);
+ sysbus_connect_irq(usartbusdev, 0, qdev_get_gpio_in(nvic, usart_irq[i]));
}
/* Timer 2 to 5 */
@@ -126,7 +125,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
}
timerbusdev = SYS_BUS_DEVICE(timerdev);
sysbus_mmio_map(timerbusdev, 0, timer_addr[i]);
- sysbus_connect_irq(timerbusdev, 0, pic[timer_irq[i]]);
+ sysbus_connect_irq(timerbusdev, 0, qdev_get_gpio_in(nvic, timer_irq[i]));
}
}
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 3ec8408..52990e9 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) {
@@ -540,6 +544,7 @@ static void armv7m_nvic_instance_init(Object *obj)
* set the num-irq property appropriately.
*/
s->num_irq = 64;
+ qdev_init_gpio_out_named(DEVICE(obj), &NVIC(obj)->sysresetreq, "SYSRESETREQ", 1);
}
static void armv7m_nvic_class_init(ObjectClass *klass, void *data)
diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h
index 4dcd4f9..7916b6b 100644
--- a/include/hw/arm/arm.h
+++ b/include/hw/arm/arm.h
@@ -17,7 +17,7 @@
#include "cpu.h"
/* armv7m.c */
-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);
/*
--
2.1.4
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v2] armv7-m: exit on external reset request
2015-10-10 18:54 ` [Qemu-devel] [PATCH v2] " Michael Davidsaver
@ 2015-10-11 15:06 ` Peter Crosthwaite
2015-10-12 3:36 ` [Qemu-devel] [PATCH v3 1/3] armv7-m: Return DeviceState* from armv7m_init() Michael Davidsaver
` (2 more replies)
0 siblings, 3 replies; 20+ messages in thread
From: Peter Crosthwaite @ 2015-10-11 15:06 UTC (permalink / raw)
To: Michael Davidsaver; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
Looks great. But I think you want to split this into staged patches.
Use git reset HEAD^ to undo the git commit (but keep the file changes)
then git add -p to select hunks to stage. Then commit the logically
sequential groups of changes as indiv. patches. A general rule is you
should try and avoid commiting refactorings along with new features. I
see 3 patches here.
On Sat, Oct 10, 2015 at 11:54 AM, Michael Davidsaver
<mdavidsaver@gmail.com> wrote:
> Implement the SYSRESETREQ bit of the AIRCR register
> for armv7-m (ie. cortex-m3) to trigger a GPIO out.
>
This is probably the second patch (your new self-contained feature).
> 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.
>
This is the first patch (refactorings).
Ideally we do this in a more QOM-correct way like with the A-class
MPCores but this rework actually gets us closer to that, so I see the
value in taking this as the first step. The function that now returns
a DeviceState is easily reworked to be a QOM object construction.
> Add GPIO in for the stellaris board which calls
> qemu_system_reset_request() on reset request.
Third patch (connect it all up).
Regards,
Peter
> ---
> 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(-)
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v3 1/3] armv7-m: Return DeviceState* from armv7m_init()
2015-10-11 15:06 ` Peter Crosthwaite
@ 2015-10-12 3:36 ` Michael Davidsaver
2015-10-30 21:15 ` Peter Crosthwaite
2015-10-12 3:36 ` [Qemu-devel] [PATCH v3 2/3] armv7-m: Implement SYSRESETREQ Michael Davidsaver
2015-10-12 3:36 ` [Qemu-devel] [PATCH v3 3/3] stellaris: exit on external reset request Michael Davidsaver
2 siblings, 1 reply; 20+ messages in thread
From: Michael Davidsaver @ 2015-10-12 3:36 UTC (permalink / raw)
To: Peter Crosthwaite; +Cc: Peter Maydell, Michael Davidsaver, qemu-devel
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 <mdavidsaver@gmail.com>
---
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, 0xfffff000, 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, 0x20000000, 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 & (0x10000 << i)) {
dev = sysbus_create_simple(TYPE_STELLARIS_GPTM,
0x40030000 + 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, 0x40020000, pic[8]);
+ dev = sysbus_create_simple(TYPE_STELLARIS_I2C, 0x40020000,
+ qdev_get_gpio_in(nvic, 8));
i2c = (I2CBus *)qdev_get_child_bus(dev, "i2c");
if (board->peripherals & BP_OLED_I2C) {
i2c_create_slave(i2c, "ssd0303", 0x3d);
@@ -1286,11 +1292,12 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model,
for (i = 0; i < 4; i++) {
if (board->dc2 & (1 << i)) {
sysbus_create_simple("pl011_luminary", 0x4000c000 + i * 0x1000,
- pic[uart_irq[i]]);
+ qdev_get_gpio_in(nvic, uart_irq[i]));
}
}
if (board->dc2 & (1 << 4)) {
- dev = sysbus_create_simple("pl022", 0x40008000, pic[7]);
+ dev = sysbus_create_simple("pl022", 0x40008000,
+ qdev_get_gpio_in(nvic, 7));
if (board->peripherals & BP_OLED_SSI) {
void *bus;
DeviceState *sddev;
@@ -1326,7 +1333,7 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model,
qdev_set_nic_properties(enet, &nd_table[0]);
qdev_init_nofail(enet);
sysbus_mmio_map(SYS_BUS_DEVICE(enet), 0, 0x40048000);
- sysbus_connect_irq(SYS_BUS_DEVICE(enet), 0, pic[42]);
+ sysbus_connect_irq(SYS_BUS_DEVICE(enet), 0, qdev_get_gpio_in(nvic, 42));
}
if (board->peripherals & BP_GAMEPAD) {
qemu_irq gpad_irq[5];
diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
index 4d26a7e..3f99340 100644
--- a/hw/arm/stm32f205_soc.c
+++ b/hw/arm/stm32f205_soc.c
@@ -59,9 +59,8 @@ static void stm32f205_soc_initfn(Object *obj)
static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
{
STM32F205State *s = STM32F205_SOC(dev_soc);
- DeviceState *syscfgdev, *usartdev, *timerdev;
+ DeviceState *syscfgdev, *usartdev, *timerdev, *nvic;
SysBusDevice *syscfgbusdev, *usartbusdev, *timerbusdev;
- qemu_irq *pic;
Error *err = NULL;
int i;
@@ -88,8 +87,8 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
vmstate_register_ram_global(sram);
memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, sram);
- pic = armv7m_init(get_system_memory(), FLASH_SIZE, 96,
- s->kernel_filename, s->cpu_model);
+ nvic = armv7m_init(get_system_memory(), FLASH_SIZE, 96,
+ s->kernel_filename, s->cpu_model);
/* System configuration controller */
syscfgdev = DEVICE(&s->syscfg);
@@ -100,7 +99,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
}
syscfgbusdev = SYS_BUS_DEVICE(syscfgdev);
sysbus_mmio_map(syscfgbusdev, 0, 0x40013800);
- sysbus_connect_irq(syscfgbusdev, 0, pic[71]);
+ sysbus_connect_irq(syscfgbusdev, 0, qdev_get_gpio_in(nvic, 71));
/* Attach UART (uses USART registers) and USART controllers */
for (i = 0; i < STM_NUM_USARTS; i++) {
@@ -112,7 +111,8 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
}
usartbusdev = SYS_BUS_DEVICE(usartdev);
sysbus_mmio_map(usartbusdev, 0, usart_addr[i]);
- sysbus_connect_irq(usartbusdev, 0, pic[usart_irq[i]]);
+ sysbus_connect_irq(usartbusdev, 0,
+ qdev_get_gpio_in(nvic, usart_irq[i]));
}
/* Timer 2 to 5 */
@@ -126,7 +126,8 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
}
timerbusdev = SYS_BUS_DEVICE(timerdev);
sysbus_mmio_map(timerbusdev, 0, timer_addr[i]);
- sysbus_connect_irq(timerbusdev, 0, pic[timer_irq[i]]);
+ sysbus_connect_irq(timerbusdev, 0,
+ qdev_get_gpio_in(nvic, timer_irq[i]));
}
}
diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h
index 4dcd4f9..7916b6b 100644
--- a/include/hw/arm/arm.h
+++ b/include/hw/arm/arm.h
@@ -17,7 +17,7 @@
#include "cpu.h"
/* armv7m.c */
-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);
/*
--
2.1.4
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v3 2/3] armv7-m: Implement SYSRESETREQ
2015-10-11 15:06 ` Peter Crosthwaite
2015-10-12 3:36 ` [Qemu-devel] [PATCH v3 1/3] armv7-m: Return DeviceState* from armv7m_init() Michael Davidsaver
@ 2015-10-12 3:36 ` Michael Davidsaver
2015-10-30 21:16 ` Peter Crosthwaite
2015-10-12 3:36 ` [Qemu-devel] [PATCH v3 3/3] stellaris: exit on external reset request Michael Davidsaver
2 siblings, 1 reply; 20+ messages in thread
From: Michael Davidsaver @ 2015-10-12 3:36 UTC (permalink / raw)
To: Peter Crosthwaite; +Cc: Peter Maydell, Michael Davidsaver, qemu-devel
Implement the SYSRESETREQ bit of the AIRCR register
for armv7-m (ie. cortex-m3) to trigger a GPIO out.
Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v3 3/3] stellaris: exit on external reset request
2015-10-11 15:06 ` Peter Crosthwaite
2015-10-12 3:36 ` [Qemu-devel] [PATCH v3 1/3] armv7-m: Return DeviceState* from armv7m_init() Michael Davidsaver
2015-10-12 3:36 ` [Qemu-devel] [PATCH v3 2/3] armv7-m: Implement SYSRESETREQ Michael Davidsaver
@ 2015-10-12 3:36 ` Michael Davidsaver
2015-10-30 21:13 ` Peter Crosthwaite
2 siblings, 1 reply; 20+ messages in thread
From: Michael Davidsaver @ 2015-10-12 3:36 UTC (permalink / raw)
To: Peter Crosthwaite; +Cc: Peter Maydell, Michael Davidsaver, qemu-devel
Add GPIO in for the stellaris board which calls
qemu_system_reset_request() on reset request.
Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v3 3/3] stellaris: exit on external reset request
2015-10-12 3:36 ` [Qemu-devel] [PATCH v3 3/3] stellaris: exit on external reset request Michael Davidsaver
@ 2015-10-30 21:13 ` Peter Crosthwaite
0 siblings, 0 replies; 20+ messages in thread
From: Peter Crosthwaite @ 2015-10-30 21:13 UTC (permalink / raw)
To: Michael Davidsaver; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
"arm: stellaris: " prefix in subject line.
On Sun, Oct 11, 2015 at 8:36 PM, Michael Davidsaver
<mdavidsaver@gmail.com> wrote:
> Add GPIO in for the stellaris board which calls
> qemu_system_reset_request() on reset request.
>
> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
Otherwise,
Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
Regards,
Peter
> ---
> 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
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/3] armv7-m: Return DeviceState* from armv7m_init()
2015-10-12 3:36 ` [Qemu-devel] [PATCH v3 1/3] armv7-m: Return DeviceState* from armv7m_init() Michael Davidsaver
@ 2015-10-30 21:15 ` Peter Crosthwaite
2015-10-30 21:20 ` Peter Crosthwaite
0 siblings, 1 reply; 20+ messages in thread
From: Peter Crosthwaite @ 2015-10-30 21:15 UTC (permalink / raw)
To: Michael Davidsaver
Cc: Peter Maydell, qemu-devel@nongnu.org Developers, Alistair Francis
Missing CC of Alistair for STM32F205.
On Sun, Oct 11, 2015 at 8:36 PM, Michael Davidsaver
<mdavidsaver@gmail.com> wrote:
> 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 <mdavidsaver@gmail.com>
This is a good step towards QOMification, and I like it in its own right.
Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
Regards,
Peter
> ---
> 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, 0xfffff000, 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, 0x20000000, 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 & (0x10000 << i)) {
> dev = sysbus_create_simple(TYPE_STELLARIS_GPTM,
> 0x40030000 + 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, 0x40020000, pic[8]);
> + dev = sysbus_create_simple(TYPE_STELLARIS_I2C, 0x40020000,
> + qdev_get_gpio_in(nvic, 8));
> i2c = (I2CBus *)qdev_get_child_bus(dev, "i2c");
> if (board->peripherals & BP_OLED_I2C) {
> i2c_create_slave(i2c, "ssd0303", 0x3d);
> @@ -1286,11 +1292,12 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model,
> for (i = 0; i < 4; i++) {
> if (board->dc2 & (1 << i)) {
> sysbus_create_simple("pl011_luminary", 0x4000c000 + i * 0x1000,
> - pic[uart_irq[i]]);
> + qdev_get_gpio_in(nvic, uart_irq[i]));
> }
> }
> if (board->dc2 & (1 << 4)) {
> - dev = sysbus_create_simple("pl022", 0x40008000, pic[7]);
> + dev = sysbus_create_simple("pl022", 0x40008000,
> + qdev_get_gpio_in(nvic, 7));
> if (board->peripherals & BP_OLED_SSI) {
> void *bus;
> DeviceState *sddev;
> @@ -1326,7 +1333,7 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model,
> qdev_set_nic_properties(enet, &nd_table[0]);
> qdev_init_nofail(enet);
> sysbus_mmio_map(SYS_BUS_DEVICE(enet), 0, 0x40048000);
> - sysbus_connect_irq(SYS_BUS_DEVICE(enet), 0, pic[42]);
> + sysbus_connect_irq(SYS_BUS_DEVICE(enet), 0, qdev_get_gpio_in(nvic, 42));
> }
> if (board->peripherals & BP_GAMEPAD) {
> qemu_irq gpad_irq[5];
> diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
> index 4d26a7e..3f99340 100644
> --- a/hw/arm/stm32f205_soc.c
> +++ b/hw/arm/stm32f205_soc.c
> @@ -59,9 +59,8 @@ static void stm32f205_soc_initfn(Object *obj)
> static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
> {
> STM32F205State *s = STM32F205_SOC(dev_soc);
> - DeviceState *syscfgdev, *usartdev, *timerdev;
> + DeviceState *syscfgdev, *usartdev, *timerdev, *nvic;
> SysBusDevice *syscfgbusdev, *usartbusdev, *timerbusdev;
> - qemu_irq *pic;
> Error *err = NULL;
> int i;
>
> @@ -88,8 +87,8 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
> vmstate_register_ram_global(sram);
> memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, sram);
>
> - pic = armv7m_init(get_system_memory(), FLASH_SIZE, 96,
> - s->kernel_filename, s->cpu_model);
> + nvic = armv7m_init(get_system_memory(), FLASH_SIZE, 96,
> + s->kernel_filename, s->cpu_model);
>
> /* System configuration controller */
> syscfgdev = DEVICE(&s->syscfg);
> @@ -100,7 +99,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
> }
> syscfgbusdev = SYS_BUS_DEVICE(syscfgdev);
> sysbus_mmio_map(syscfgbusdev, 0, 0x40013800);
> - sysbus_connect_irq(syscfgbusdev, 0, pic[71]);
> + sysbus_connect_irq(syscfgbusdev, 0, qdev_get_gpio_in(nvic, 71));
>
> /* Attach UART (uses USART registers) and USART controllers */
> for (i = 0; i < STM_NUM_USARTS; i++) {
> @@ -112,7 +111,8 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
> }
> usartbusdev = SYS_BUS_DEVICE(usartdev);
> sysbus_mmio_map(usartbusdev, 0, usart_addr[i]);
> - sysbus_connect_irq(usartbusdev, 0, pic[usart_irq[i]]);
> + sysbus_connect_irq(usartbusdev, 0,
> + qdev_get_gpio_in(nvic, usart_irq[i]));
> }
>
> /* Timer 2 to 5 */
> @@ -126,7 +126,8 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
> }
> timerbusdev = SYS_BUS_DEVICE(timerdev);
> sysbus_mmio_map(timerbusdev, 0, timer_addr[i]);
> - sysbus_connect_irq(timerbusdev, 0, pic[timer_irq[i]]);
> + sysbus_connect_irq(timerbusdev, 0,
> + qdev_get_gpio_in(nvic, timer_irq[i]));
> }
> }
>
> diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h
> index 4dcd4f9..7916b6b 100644
> --- a/include/hw/arm/arm.h
> +++ b/include/hw/arm/arm.h
> @@ -17,7 +17,7 @@
> #include "cpu.h"
>
> /* armv7m.c */
> -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);
>
> /*
> --
> 2.1.4
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/3] armv7-m: Implement SYSRESETREQ
2015-10-12 3:36 ` [Qemu-devel] [PATCH v3 2/3] armv7-m: Implement SYSRESETREQ Michael Davidsaver
@ 2015-10-30 21:16 ` Peter Crosthwaite
0 siblings, 0 replies; 20+ messages in thread
From: Peter Crosthwaite @ 2015-10-30 21:16 UTC (permalink / raw)
To: Michael Davidsaver; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
On Sun, Oct 11, 2015 at 8:36 PM, Michael Davidsaver
<mdavidsaver@gmail.com> wrote:
> Implement the SYSRESETREQ bit of the AIRCR register
> for armv7-m (ie. cortex-m3) to trigger a GPIO out.
>
> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> ---
> 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
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/3] armv7-m: Return DeviceState* from armv7m_init()
2015-10-30 21:15 ` Peter Crosthwaite
@ 2015-10-30 21:20 ` Peter Crosthwaite
2015-10-30 21:35 ` Peter Maydell
0 siblings, 1 reply; 20+ messages in thread
From: Peter Crosthwaite @ 2015-10-30 21:20 UTC (permalink / raw)
To: Michael Davidsaver
Cc: Peter Maydell, qemu-devel@nongnu.org Developers, Alistair Francis
This series looks good, up to Peter if is qualifies for 2.5. It was
clearly on list well before soft freeze and really my tardiness as to
why it is late with review. So I'll make a case for inclusion.
It does seem to be missing a cover, and/or the in-reply-to looks a bit
strange. Maybe patches missed it? Can you try an all-in-one resend
with git-generated cover?
Regards,
Peter
On Fri, Oct 30, 2015 at 2:15 PM, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> Missing CC of Alistair for STM32F205.
>
> On Sun, Oct 11, 2015 at 8:36 PM, Michael Davidsaver
> <mdavidsaver@gmail.com> wrote:
>> 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 <mdavidsaver@gmail.com>
>
> This is a good step towards QOMification, and I like it in its own right.
>
> Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
>
> Regards,
> Peter
>
>> ---
>> 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, 0xfffff000, 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, 0x20000000, 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 & (0x10000 << i)) {
>> dev = sysbus_create_simple(TYPE_STELLARIS_GPTM,
>> 0x40030000 + 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, 0x40020000, pic[8]);
>> + dev = sysbus_create_simple(TYPE_STELLARIS_I2C, 0x40020000,
>> + qdev_get_gpio_in(nvic, 8));
>> i2c = (I2CBus *)qdev_get_child_bus(dev, "i2c");
>> if (board->peripherals & BP_OLED_I2C) {
>> i2c_create_slave(i2c, "ssd0303", 0x3d);
>> @@ -1286,11 +1292,12 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model,
>> for (i = 0; i < 4; i++) {
>> if (board->dc2 & (1 << i)) {
>> sysbus_create_simple("pl011_luminary", 0x4000c000 + i * 0x1000,
>> - pic[uart_irq[i]]);
>> + qdev_get_gpio_in(nvic, uart_irq[i]));
>> }
>> }
>> if (board->dc2 & (1 << 4)) {
>> - dev = sysbus_create_simple("pl022", 0x40008000, pic[7]);
>> + dev = sysbus_create_simple("pl022", 0x40008000,
>> + qdev_get_gpio_in(nvic, 7));
>> if (board->peripherals & BP_OLED_SSI) {
>> void *bus;
>> DeviceState *sddev;
>> @@ -1326,7 +1333,7 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model,
>> qdev_set_nic_properties(enet, &nd_table[0]);
>> qdev_init_nofail(enet);
>> sysbus_mmio_map(SYS_BUS_DEVICE(enet), 0, 0x40048000);
>> - sysbus_connect_irq(SYS_BUS_DEVICE(enet), 0, pic[42]);
>> + sysbus_connect_irq(SYS_BUS_DEVICE(enet), 0, qdev_get_gpio_in(nvic, 42));
>> }
>> if (board->peripherals & BP_GAMEPAD) {
>> qemu_irq gpad_irq[5];
>> diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
>> index 4d26a7e..3f99340 100644
>> --- a/hw/arm/stm32f205_soc.c
>> +++ b/hw/arm/stm32f205_soc.c
>> @@ -59,9 +59,8 @@ static void stm32f205_soc_initfn(Object *obj)
>> static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>> {
>> STM32F205State *s = STM32F205_SOC(dev_soc);
>> - DeviceState *syscfgdev, *usartdev, *timerdev;
>> + DeviceState *syscfgdev, *usartdev, *timerdev, *nvic;
>> SysBusDevice *syscfgbusdev, *usartbusdev, *timerbusdev;
>> - qemu_irq *pic;
>> Error *err = NULL;
>> int i;
>>
>> @@ -88,8 +87,8 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>> vmstate_register_ram_global(sram);
>> memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, sram);
>>
>> - pic = armv7m_init(get_system_memory(), FLASH_SIZE, 96,
>> - s->kernel_filename, s->cpu_model);
>> + nvic = armv7m_init(get_system_memory(), FLASH_SIZE, 96,
>> + s->kernel_filename, s->cpu_model);
>>
>> /* System configuration controller */
>> syscfgdev = DEVICE(&s->syscfg);
>> @@ -100,7 +99,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>> }
>> syscfgbusdev = SYS_BUS_DEVICE(syscfgdev);
>> sysbus_mmio_map(syscfgbusdev, 0, 0x40013800);
>> - sysbus_connect_irq(syscfgbusdev, 0, pic[71]);
>> + sysbus_connect_irq(syscfgbusdev, 0, qdev_get_gpio_in(nvic, 71));
>>
>> /* Attach UART (uses USART registers) and USART controllers */
>> for (i = 0; i < STM_NUM_USARTS; i++) {
>> @@ -112,7 +111,8 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>> }
>> usartbusdev = SYS_BUS_DEVICE(usartdev);
>> sysbus_mmio_map(usartbusdev, 0, usart_addr[i]);
>> - sysbus_connect_irq(usartbusdev, 0, pic[usart_irq[i]]);
>> + sysbus_connect_irq(usartbusdev, 0,
>> + qdev_get_gpio_in(nvic, usart_irq[i]));
>> }
>>
>> /* Timer 2 to 5 */
>> @@ -126,7 +126,8 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>> }
>> timerbusdev = SYS_BUS_DEVICE(timerdev);
>> sysbus_mmio_map(timerbusdev, 0, timer_addr[i]);
>> - sysbus_connect_irq(timerbusdev, 0, pic[timer_irq[i]]);
>> + sysbus_connect_irq(timerbusdev, 0,
>> + qdev_get_gpio_in(nvic, timer_irq[i]));
>> }
>> }
>>
>> diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h
>> index 4dcd4f9..7916b6b 100644
>> --- a/include/hw/arm/arm.h
>> +++ b/include/hw/arm/arm.h
>> @@ -17,7 +17,7 @@
>> #include "cpu.h"
>>
>> /* armv7m.c */
>> -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);
>>
>> /*
>> --
>> 2.1.4
>>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/3] armv7-m: Return DeviceState* from armv7m_init()
2015-10-30 21:20 ` Peter Crosthwaite
@ 2015-10-30 21:35 ` Peter Maydell
0 siblings, 0 replies; 20+ messages in thread
From: Peter Maydell @ 2015-10-30 21:35 UTC (permalink / raw)
To: Peter Crosthwaite
Cc: Alistair Francis, Michael Davidsaver,
qemu-devel@nongnu.org Developers
On 30 October 2015 at 21:20, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> This series looks good, up to Peter if is qualifies for 2.5. It was
> clearly on list well before soft freeze and really my tardiness as to
> why it is late with review. So I'll make a case for inclusion.
>
> It does seem to be missing a cover, and/or the in-reply-to looks a bit
> strange. Maybe patches missed it? Can you try an all-in-one resend
> with git-generated cover?
There's no cover letter in my mail archive either (which meant
that I didn't look at this earlier, since I thought it was in
the same set as the "add MPU" patch, which definitely is not
2.3 material). Anyway, I think these patches look good and
we may as well put them in for 2.5 (it only affects M profile).
I've made the tweak to the commit subject you suggest for 3/3.
thanks
-- PMM
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2015-10-30 21:36 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-08 15:40 [Qemu-devel] [PATCH] Exit on reset for armv7-m Michael Davidsaver
2015-10-08 15:40 ` [Qemu-devel] [PATCH] armv7-m: exit on external reset request Michael Davidsaver
2015-10-09 16:59 ` Peter Maydell
2015-10-09 17:25 ` Michael Davidsaver
2015-10-09 18:18 ` Peter Crosthwaite
2015-10-09 18:51 ` Michael Davidsaver
2015-10-10 14:35 ` Michael Davidsaver
2015-10-10 18:54 ` [Qemu-devel] [PATCH v2] " Michael Davidsaver
2015-10-11 15:06 ` Peter Crosthwaite
2015-10-12 3:36 ` [Qemu-devel] [PATCH v3 1/3] armv7-m: Return DeviceState* from armv7m_init() Michael Davidsaver
2015-10-30 21:15 ` Peter Crosthwaite
2015-10-30 21:20 ` Peter Crosthwaite
2015-10-30 21:35 ` Peter Maydell
2015-10-12 3:36 ` [Qemu-devel] [PATCH v3 2/3] armv7-m: Implement SYSRESETREQ Michael Davidsaver
2015-10-30 21:16 ` Peter Crosthwaite
2015-10-12 3:36 ` [Qemu-devel] [PATCH v3 3/3] stellaris: exit on external reset request Michael Davidsaver
2015-10-30 21:13 ` Peter Crosthwaite
2015-10-08 20:09 ` [Qemu-devel] [PATCH] Exit on reset for armv7-m Peter Crosthwaite
2015-10-09 1:24 ` Michael Davidsaver
2015-10-09 4:21 ` Peter Crosthwaite
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).