Re: [Qemu-devel] [PATCH v2 8/8] x86: correctly implement soft reset

[Michael S. Tsirkin]

On Fri, May 02, 2014 at 04:33:22PM +0200, Paolo Bonzini wrote:
> Do not do a hard reset for port 92h, keyboard controller, or cf9h soft reset.
> These only reset the CPU.
> 
> Reviewed-by: Anthony Liguori <address@hidden>
> Signed-off-by: Paolo Bonzini <address@hidden>

I note that we allow setting reset type in the same write
with reset itself.
i440fx spec says
    2
    Reset CPU (RCPU). RCPU is used to initiate a hard or soft reset to the
    CPU. During hard reset, the
    PMC asserts CRESET# for 2 msec and PCIRST# for 1 msec. During soft
    reset, the PMC asserts INIT#.
    BISTE and SHRE must be set up prior to writing a 1 to this bit. Two
    operations are required to initiate a
    reset using this register. The first write operation programs BISTE and
    SHRE to the appropriate state
    while setting RCPU to 0. The second write operation keeps the BISTE and
    SHRE at their programmed
    state while setting RCPU to 1.

it does not actually say what happens if you try to change reset type
in the same access as the reset so I think we are ok
with this simple logic that you implemented.



> ---
>  hw/i386/pc.c       |  3 ++-
>  hw/input/pckbd.c   |  5 +++--
>  hw/isa/lpc_ich9.c  | 12 ++++++++++--
>  hw/pci-host/piix.c |  8 ++++++--
>  4 files changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index a59e958..8716864 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -45,6 +45,7 @@
>  #include "kvm_i386.h"
>  #include "hw/xen/xen.h"
>  #include "sysemu/blockdev.h"
> +#include "sysemu/cpus.h"
>  #include "hw/block/block.h"
>  #include "ui/qemu-spice.h"
>  #include "exec/memory.h"
> @@ -477,7 +478,7 @@ static void port92_write(void *opaque, hwaddr addr, 
> uint64_t val,
>      s->outport = val;
>      qemu_set_irq(*s->a20_out, (val >> 1) & 1);
>      if ((val & 1) && !(oldval & 1)) {
> -        qemu_system_reset_request();
> +        cpu_soft_reset();
>      }
>  }
>  
> diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
> index 29af3d7..fd87776 100644
> --- a/hw/input/pckbd.c
> +++ b/hw/input/pckbd.c
> @@ -26,6 +26,7 @@
>  #include "hw/i386/pc.h"
>  #include "hw/input/ps2.h"
>  #include "sysemu/sysemu.h"
> +#include "sysemu/cpus.h"
>  
>  /* debug PC keyboard */
>  //#define DEBUG_KBD
> @@ -220,7 +221,7 @@ static void outport_write(KBDState *s, uint32_t val)
>          qemu_set_irq(*s->a20_out, (val >> 1) & 1);
>      }
>      if (!(val & 1)) {
> -        qemu_system_reset_request();
> +        cpu_soft_reset();
>      }
>  }
>  
> @@ -299,7 +300,7 @@ static void kbd_write_command(void *opaque, hwaddr addr,
>          s->outport &= ~KBD_OUT_A20;
>          break;
>      case KBD_CCMD_RESET:
> -        qemu_system_reset_request();
> +        cpu_soft_reset();
>          break;
>      case KBD_CCMD_NO_OP:
>          /* ignore that */
> diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
> index 51ce12d..e9fde85 100644
> --- a/hw/isa/lpc_ich9.c
> +++ b/hw/isa/lpc_ich9.c
> @@ -45,6 +45,7 @@
>  #include "hw/pci/pci_bus.h"
>  #include "exec/address-spaces.h"
>  #include "sysemu/sysemu.h"
> +#include "sysemu/cpus.h"
>  
>  static int ich9_lpc_sci_irq(ICH9LPCState *lpc);
>  
> @@ -507,8 +508,15 @@ static void ich9_rst_cnt_write(void *opaque, hwaddr 
> addr, uint64_t val,
>      ICH9LPCState *lpc = opaque;
>  
>      if (val & 4) {
> -        qemu_system_reset_request();
> -        return;
> +        /* In a real ICH9, FULL_RST affects whether the hardware goes to S5
> +         * for 3-5 seconds, but is not enough alone; you need to set SYS_RST
> +         * too.
> +         */
> +        if (val & 2) {
> +            qemu_system_reset_request();
> +        } else {
> +            cpu_soft_reset();
> +        }
>      }
>      lpc->rst_cnt = val & 0xA; /* keep FULL_RST (bit 3) and SYS_RST (bit 1) */
>  }
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index ffdc853..1e60172 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -34,6 +34,7 @@
>  #include "sysemu/sysemu.h"
>  #include "hw/i386/ioapic.h"
>  #include "qapi/visitor.h"
> +#include "sysemu/cpus.h"
>  
>  /*
>   * I440FX chipset data sheet.
> @@ -587,8 +588,11 @@ static void rcr_write(void *opaque, hwaddr addr, 
> uint64_t val, unsigned len)
>      PIIX3State *d = opaque;
>  
>      if (val & 4) {
> -        qemu_system_reset_request();
> -        return;
> +        if (val & 2) {
> +            qemu_system_reset_request();
> +        } else {
> +            cpu_soft_reset();
> +        }
>      }
>      d->rcr = val & 2; /* keep System Reset type only */
>  }
> -- 
> 1.8.3.1
>