[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 3/3] hw: correctly implement soft reset
From: |
li guang |
Subject: |
Re: [Qemu-devel] [PATCH v2 3/3] hw: correctly implement soft reset |
Date: |
Wed, 06 Mar 2013 17:06:23 +0800 |
在 2013-03-06三的 09:36 +0100,Paolo Bonzini写道:
> Il 06/03/2013 03:02, li guang ha scritto:
> > 在 2013-03-05二的 20:00 +0100,Paolo Bonzini写道:
> >> 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>
> >> ---
> >> hw/lpc_ich9.c | 7 ++++++-
> >> hw/pc.c | 3 ++-
> >> hw/pckbd.c | 5 +++--
> >> hw/piix_pci.c | 8 ++++++--
> >> 4 files changed, 17 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/hw/lpc_ich9.c b/hw/lpc_ich9.c
> >> index e473758..5540f61 100644
> >> --- a/hw/lpc_ich9.c
> >> +++ b/hw/lpc_ich9.c
> >> @@ -45,6 +45,7 @@
> >> #include "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);
> >>
> >> @@ -506,7 +507,11 @@ static void ich9_rst_cnt_write(void *opaque, hwaddr
> >> addr, uint64_t val,
> >> ICH9LPCState *lpc = opaque;
> >>
> >> if (val & 4) {
> >> - qemu_system_reset_request();
> >> + if (val & 0xA) {
> >> + qemu_system_reset_request();
> >> + } else {
> >> + cpu_reset_all_async();
> >> + }
> >> return;
> >
> > in fact, soft reset is cpu reset, hard reset is platform reset,
> > it is too harsh to require both bit 3 & 1 to do a system reset,
> > they are independent, either of them can trigger that.
>
> This is "full reset if (val & 0xA) != 0". You interpreted it as (val &
> 0xA) = 0xA, I think.
No, IMHO, full reset should be done only by bit 4.
that is bit 1 & 2 are combined to determined reset cpu/system.
and system reset does not always be the same with full reset(
mostly be different in hardware signal of power cycle),
it's up to the board hardware designer.
>
> >> }
> >> lpc->rst_cnt = val & 0xA; /* keep FULL_RST (bit 3) and SYS_RST (bit
> >> 1) */
> >> diff --git a/hw/pc.c b/hw/pc.c
> >> index 3e1cf2e..54f5b72 100644
> >> --- a/hw/pc.c
> >> +++ b/hw/pc.c
> >> @@ -45,6 +45,7 @@
> >> #include "kvm_i386.h"
> >> #include "xen.h"
> >> #include "sysemu/blockdev.h"
> >> +#include "sysemu/cpus.h"
> >> #include "hw/block-common.h"
> >> #include "ui/qemu-spice.h"
> >> #include "exec/memory.h"
> >> @@ -443,7 +444,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_reset_all_async();
> >> }
> >> }
> >>
> >> diff --git a/hw/pckbd.c b/hw/pckbd.c
> >> index 3bad09b..fd66788 100644
> >> --- a/hw/pckbd.c
> >> +++ b/hw/pckbd.c
> >> @@ -26,6 +26,7 @@
> >> #include "pc.h"
> >> #include "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_reset_all_async();
> >> }
> >> }
> >>
> >> @@ -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_reset_all_async();
> >
> > Oh, no, system reset is correct.
>
> No, the keyboard controller is connected to the CPU reset signal.
Not always, AFAIK,
traditionally, this should do system reset,
though system reset may be triggered by INIT# which
I guess is you said "CPU reset signal".
>
> > in the real world, system and cpu reset are quite different,
> > cpu reset only reset processor power, while system reset
> > will reset platform power.
>
> Separating the two is exactly the purpose of this patch. :)
>
> Paolo
>
> >> break;
> >> case KBD_CCMD_NO_OP:
> >> /* ignore that */
> >> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> >> index 6c77e49..785e0a7 100644
> >> --- a/hw/piix_pci.c
> >> +++ b/hw/piix_pci.c
> >> @@ -32,6 +32,7 @@
> >> #include "xen.h"
> >> #include "pam.h"
> >> #include "sysemu/sysemu.h"
> >> +#include "sysemu/cpus.h"
> >>
> >> /*
> >> * I440FX chipset data sheet.
> >> @@ -521,8 +522,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_reset_all_async();
> >> + }
> >> }
> >> d->rcr = val & 2; /* keep System Reset type only */
> >> }
> >
> >
> >
> >
>
- Re: [Qemu-devel] [PATCH v2 1/3] cpu: make CPU_INTERRUPT_RESET available on all targets, (continued)
[Qemu-devel] [PATCH v2 3/3] hw: correctly implement soft reset, Paolo Bonzini, 2013/03/05
Re: [Qemu-devel] [PATCH v2 0/3] Implement x86 soft reset, Laszlo Ersek, 2013/03/05