Patchwork prep: Fix software reset

login
register
mail settings
Submitter Julio Guerra
Date Feb. 16, 2013, 3:08 p.m.
Message ID <1361027311-23437-1-git-send-email-guerr@julio.in>
Download mbox | patch
Permalink /patch/220962/
State New
Headers show

Comments

Julio Guerra - Feb. 16, 2013, 3:08 p.m.
The software reset of a PReP machine should reset the entire system
and not only the processor. It occurs when changing the 7th bit of
port 0092 from 0 to 1.

Adding a new variable in PReP's sysctrl_t to store the soft reset bit
makes possible to be compliant with PReP specification :
* reset the system when changing soft reset bit from 0 to 1.
* the soft reset bit value is 1 after a soft reset.
* Port 0092 is read/write.

qemu_system_reset_request() does the required job (calling the reset
handlers) when the software reset is needed.

reset_irq is no longer needed, the CPU reset (calling ppc_prep_reset)
is called when qemu_system_reset calls every reset handlers.

Signed-off-by: Julio Guerra <guerr@julio.in>
---
 hw/ppc/prep.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

--
1.8.1.2
Alexander Graf - Feb. 25, 2013, 11:20 a.m.
On 16.02.2013, at 16:08, Julio Guerra wrote:

> The software reset of a PReP machine should reset the entire system
> and not only the processor. It occurs when changing the 7th bit of
> port 0092 from 0 to 1.
> 
> Adding a new variable in PReP's sysctrl_t to store the soft reset bit
> makes possible to be compliant with PReP specification :
> * reset the system when changing soft reset bit from 0 to 1.
> * the soft reset bit value is 1 after a soft reset.
> * Port 0092 is read/write.
> 
> qemu_system_reset_request() does the required job (calling the reset
> handlers) when the software reset is needed.
> 
> reset_irq is no longer needed, the CPU reset (calling ppc_prep_reset)
> is called when qemu_system_reset calls every reset handlers.
> 
> Signed-off-by: Julio Guerra <guerr@julio.in>

Andreas, could you take this one through the prep queue please?


Alex

> ---
> hw/ppc/prep.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index e06dded..64dab8b 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -178,12 +178,12 @@ static const MemoryRegionOps PPC_XCSR_ops = {
> 
> /* Fake super-io ports for PREP platform (Intel 82378ZB) */
> typedef struct sysctrl_t {
> -    qemu_irq reset_irq;
>     M48t59State *nvram;
>     uint8_t state;
>     uint8_t syscontrol;
>     int contiguous_map;
>     int endian;
> +    uint8_t sreset;
> } sysctrl_t;
> 
> enum {
> @@ -203,9 +203,11 @@ static void PREP_io_800_writeb (void *opaque, uint32_t addr, uint32_t val)
>         /* Special port 92 */
>         /* Check soft reset asked */
>         if (val & 0x01) {
> -            qemu_irq_raise(sysctrl->reset_irq);
> +	    if (!sysctrl->sreset)
> +                qemu_system_reset_request();
> +            sysctrl->sreset = 1;
>         } else {
> -            qemu_irq_lower(sysctrl->reset_irq);
> +            sysctrl->sreset = 0;
>         }
>         /* Check LE mode */
>         if (val & 0x02) {
> @@ -267,7 +269,7 @@ static uint32_t PREP_io_800_readb (void *opaque, uint32_t addr)
>     switch (addr) {
>     case 0x0092:
>         /* Special port 92 */
> -        retval = 0x00;
> +        retval = (sysctrl->endian << 1) | sysctrl->sreset;
>         break;
>     case 0x0800:
>         /* Motorola CPU configuration register */
> @@ -624,7 +626,8 @@ static void ppc_prep_init(QEMUMachineInitArgs *args)
>     }
>     isa_create_simple(isa_bus, "i8042");
> 
> -    sysctrl->reset_irq = first_cpu->irq_inputs[PPC6xx_INPUT_HRESET];
> +    sysctrl->sreset = 0;
> +    sysctrl->endian = 0;
>     /* System control ports */
>     register_ioport_read(0x0092, 0x01, 1, &PREP_io_800_readb, sysctrl);
>     register_ioport_write(0x0092, 0x01, 1, &PREP_io_800_writeb, sysctrl);
> --
> 1.8.1.2
>
Andreas Färber - Feb. 25, 2013, 12:25 p.m.
Am 25.02.2013 12:20, schrieb Alexander Graf:
> 
> On 16.02.2013, at 16:08, Julio Guerra wrote:
> 
>> The software reset of a PReP machine should reset the entire system
>> and not only the processor. It occurs when changing the 7th bit of
>> port 0092 from 0 to 1.
>>
>> Adding a new variable in PReP's sysctrl_t to store the soft reset bit
>> makes possible to be compliant with PReP specification :
>> * reset the system when changing soft reset bit from 0 to 1.
>> * the soft reset bit value is 1 after a soft reset.
>> * Port 0092 is read/write.
>>
>> qemu_system_reset_request() does the required job (calling the reset
>> handlers) when the software reset is needed.
>>
>> reset_irq is no longer needed, the CPU reset (calling ppc_prep_reset)
>> is called when qemu_system_reset calls every reset handlers.
>>
>> Signed-off-by: Julio Guerra <guerr@julio.in>
> 
> Andreas, could you take this one through the prep queue please?

It's PReP only, so I intend to handle it. But apart from checkpatch.pl
style problems (that I could fix up myself) this is touching on the same
soft reset topic that I am awaiting the outcome for x86.

The issue of returning endianness bit for 0x0092 is independent of that
and could be split out. I want a qtest case though, therefore my
interest in Markus' series. Could become a separate prep-test though.

Andreas
Julio Guerra - April 3, 2013, 1:59 p.m.
2013/2/25 Andreas Färber <andreas.faerber@web.de>:
> Am 25.02.2013 12:20, schrieb Alexander Graf:
>>
>> On 16.02.2013, at 16:08, Julio Guerra wrote:
>>
>>> The software reset of a PReP machine should reset the entire system
>>> and not only the processor. It occurs when changing the 7th bit of
>>> port 0092 from 0 to 1.
>>>
>>> Adding a new variable in PReP's sysctrl_t to store the soft reset bit
>>> makes possible to be compliant with PReP specification :
>>> * reset the system when changing soft reset bit from 0 to 1.
>>> * the soft reset bit value is 1 after a soft reset.
>>> * Port 0092 is read/write.
>>>
>>> qemu_system_reset_request() does the required job (calling the reset
>>> handlers) when the software reset is needed.
>>>
>>> reset_irq is no longer needed, the CPU reset (calling ppc_prep_reset)
>>> is called when qemu_system_reset calls every reset handlers.
>>>
>>> Signed-off-by: Julio Guerra <guerr@julio.in>
>>
>> Andreas, could you take this one through the prep queue please?
>
> It's PReP only, so I intend to handle it. But apart from checkpatch.pl
> style problems (that I could fix up myself) this is touching on the same
> soft reset topic that I am awaiting the outcome for x86.
>
> The issue of returning endianness bit for 0x0092 is independent of that
> and could be split out. I want a qtest case though, therefore my
> interest in Markus' series. Could become a separate prep-test though.
>

ping
Julio Guerra - April 17, 2013, 6:29 a.m.
2013/2/25 Andreas Färber <andreas.faerber@web.de>:
> Am 25.02.2013 12:20, schrieb Alexander Graf:
>>
>> On 16.02.2013, at 16:08, Julio Guerra wrote:
>>
>>> The software reset of a PReP machine should reset the entire system
>>> and not only the processor. It occurs when changing the 7th bit of
>>> port 0092 from 0 to 1.
>>>
>>> Adding a new variable in PReP's sysctrl_t to store the soft reset bit
>>> makes possible to be compliant with PReP specification :
>>> * reset the system when changing soft reset bit from 0 to 1.
>>> * the soft reset bit value is 1 after a soft reset.
>>> * Port 0092 is read/write.
>>>
>>> qemu_system_reset_request() does the required job (calling the reset
>>> handlers) when the software reset is needed.
>>>
>>> reset_irq is no longer needed, the CPU reset (calling ppc_prep_reset)
>>> is called when qemu_system_reset calls every reset handlers.
>>>
>>> Signed-off-by: Julio Guerra <guerr@julio.in>
>>
>> Andreas, could you take this one through the prep queue please?
>
> It's PReP only, so I intend to handle it. But apart from checkpatch.pl
> style problems (that I could fix up myself) this is touching on the same
> soft reset topic that I am awaiting the outcome for x86.
>
> The issue of returning endianness bit for 0x0092 is independent of that
> and could be split out. I want a qtest case though, therefore my
> interest in Markus' series. Could become a separate prep-test though.
>

Sorry to insist but is there something wrong with this patch ?

--
Julio Guerra
Andreas Färber - April 17, 2013, 10:45 a.m.
Am 17.04.2013 08:29, schrieb Julio Guerra:
> 2013/2/25 Andreas Färber <andreas.faerber@web.de>:
>> Am 25.02.2013 12:20, schrieb Alexander Graf:
>>>
>>> On 16.02.2013, at 16:08, Julio Guerra wrote:
>>>
>>>> The software reset of a PReP machine should reset the entire system
>>>> and not only the processor. It occurs when changing the 7th bit of
>>>> port 0092 from 0 to 1.
>>>>
>>>> Adding a new variable in PReP's sysctrl_t to store the soft reset bit
>>>> makes possible to be compliant with PReP specification :
>>>> * reset the system when changing soft reset bit from 0 to 1.
>>>> * the soft reset bit value is 1 after a soft reset.
>>>> * Port 0092 is read/write.
>>>>
>>>> qemu_system_reset_request() does the required job (calling the reset
>>>> handlers) when the software reset is needed.
>>>>
>>>> reset_irq is no longer needed, the CPU reset (calling ppc_prep_reset)
>>>> is called when qemu_system_reset calls every reset handlers.
>>>>
>>>> Signed-off-by: Julio Guerra <guerr@julio.in>
>>>
>>> Andreas, could you take this one through the prep queue please?
>>
>> It's PReP only, so I intend to handle it. But apart from checkpatch.pl
>> style problems (that I could fix up myself) this is touching on the same
>> soft reset topic that I am awaiting the outcome for x86.
>>
>> The issue of returning endianness bit for 0x0092 is independent of that
>> and could be split out. I want a qtest case though, therefore my
>> interest in Markus' series. Could become a separate prep-test though.
>>
> 
> Sorry to insist but is there something wrong with this patch ?

Yes, quite frankly I already indicated that it is inacceptable as-is:

* Coding Style issues to be fixed
* No agreed solution for Soft Reset yet that I am aware of
* Conflicting RFC patchsets by Hervé wrt systemio

If you could take a look at the latter yourself and provide a
trimmed-down patch, that would speed things up.

Regards,
Andreas
Andreas Färber - May 6, 2013, 12:35 a.m.
Am 16.02.2013 16:08, schrieb Julio Guerra:
> The software reset of a PReP machine should reset the entire system
> and not only the processor. It occurs when changing the 7th bit of
> port 0092 from 0 to 1.
> 
> Adding a new variable in PReP's sysctrl_t to store the soft reset bit
> makes possible to be compliant with PReP specification :
> * reset the system when changing soft reset bit from 0 to 1.
> * the soft reset bit value is 1 after a soft reset.
> * Port 0092 is read/write.
> 
> qemu_system_reset_request() does the required job (calling the reset
> handlers) when the software reset is needed.
> 
> reset_irq is no longer needed, the CPU reset (calling ppc_prep_reset)
> is called when qemu_system_reset calls every reset handlers.
> 
> Signed-off-by: Julio Guerra <guerr@julio.in>
> ---
>  hw/ppc/prep.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index e06dded..64dab8b 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -178,12 +178,12 @@ static const MemoryRegionOps PPC_XCSR_ops = {
> 
>  /* Fake super-io ports for PREP platform (Intel 82378ZB) */
>  typedef struct sysctrl_t {
> -    qemu_irq reset_irq;
>      M48t59State *nvram;
>      uint8_t state;
>      uint8_t syscontrol;
>      int contiguous_map;
>      int endian;
> +    uint8_t sreset;
>  } sysctrl_t;
> 
>  enum {
> @@ -203,9 +203,11 @@ static void PREP_io_800_writeb (void *opaque, uint32_t addr, uint32_t val)
>          /* Special port 92 */
>          /* Check soft reset asked */
>          if (val & 0x01) {
> -            qemu_irq_raise(sysctrl->reset_irq);
> +	    if (!sysctrl->sreset)
> +                qemu_system_reset_request();
> +            sysctrl->sreset = 1;
>          } else {
> -            qemu_irq_lower(sysctrl->reset_irq);
> +            sysctrl->sreset = 0;
>          }
>          /* Check LE mode */
>          if (val & 0x02) {

> @@ -267,7 +269,7 @@ static uint32_t PREP_io_800_readb (void *opaque, uint32_t addr)
>      switch (addr) {
>      case 0x0092:
>          /* Special port 92 */
> -        retval = 0x00;
> +        retval = (sysctrl->endian << 1)

Thanks, applied this bit to prep-up:
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/prep-up

Andreas

> | sysctrl->sreset;
>          break;
>      case 0x0800:
>          /* Motorola CPU configuration register */
> @@ -624,7 +626,8 @@ static void ppc_prep_init(QEMUMachineInitArgs *args)
>      }
>      isa_create_simple(isa_bus, "i8042");
> 
> -    sysctrl->reset_irq = first_cpu->irq_inputs[PPC6xx_INPUT_HRESET];
> +    sysctrl->sreset = 0;
> +    sysctrl->endian = 0;
>      /* System control ports */
>      register_ioport_read(0x0092, 0x01, 1, &PREP_io_800_readb, sysctrl);
>      register_ioport_write(0x0092, 0x01, 1, &PREP_io_800_writeb, sysctrl);
> --
> 1.8.1.2
>
Julio Guerra - May 6, 2013, 11:14 a.m.
2013/5/6 Andreas Färber <andreas.faerber@web.de>:
>
> Thanks, applied this bit to prep-up:
> http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/prep-up
>

Ok. I just saw Hervé's work on system I/O and your short discussion.
So should I just wait for a soft-reset-method agreement and solution
now ?

--
Julio Guerra

Patch

diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index e06dded..64dab8b 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -178,12 +178,12 @@  static const MemoryRegionOps PPC_XCSR_ops = {

 /* Fake super-io ports for PREP platform (Intel 82378ZB) */
 typedef struct sysctrl_t {
-    qemu_irq reset_irq;
     M48t59State *nvram;
     uint8_t state;
     uint8_t syscontrol;
     int contiguous_map;
     int endian;
+    uint8_t sreset;
 } sysctrl_t;

 enum {
@@ -203,9 +203,11 @@  static void PREP_io_800_writeb (void *opaque, uint32_t addr, uint32_t val)
         /* Special port 92 */
         /* Check soft reset asked */
         if (val & 0x01) {
-            qemu_irq_raise(sysctrl->reset_irq);
+	    if (!sysctrl->sreset)
+                qemu_system_reset_request();
+            sysctrl->sreset = 1;
         } else {
-            qemu_irq_lower(sysctrl->reset_irq);
+            sysctrl->sreset = 0;
         }
         /* Check LE mode */
         if (val & 0x02) {
@@ -267,7 +269,7 @@  static uint32_t PREP_io_800_readb (void *opaque, uint32_t addr)
     switch (addr) {
     case 0x0092:
         /* Special port 92 */
-        retval = 0x00;
+        retval = (sysctrl->endian << 1) | sysctrl->sreset;
         break;
     case 0x0800:
         /* Motorola CPU configuration register */
@@ -624,7 +626,8 @@  static void ppc_prep_init(QEMUMachineInitArgs *args)
     }
     isa_create_simple(isa_bus, "i8042");

-    sysctrl->reset_irq = first_cpu->irq_inputs[PPC6xx_INPUT_HRESET];
+    sysctrl->sreset = 0;
+    sysctrl->endian = 0;
     /* System control ports */
     register_ioport_read(0x0092, 0x01, 1, &PREP_io_800_readb, sysctrl);
     register_ioport_write(0x0092, 0x01, 1, &PREP_io_800_writeb, sysctrl);