Hi, thanks for the patch! I have some questions:

On Wed, Nov 27, 2019 at 3:46 PM 'YuChen Qian' via OSv Development <
[email protected]> wrote:

> The original keyboard reset does not wait a for short period, so we see
> the triple fault being triggered even if the keyboard reset is
> successful sometimes. Added a wait there to make sure the keyboard reset
> completes before triggering the triple fault.
>

Out of curiosity, what is the downside of not waiting, and trying another
reboot method even if the previous one will have eventually worked?
Isn't the end-result the same - that it will reboot?
Is the risk that the machine will reboot for a second time right after the
first one?


> Added PCI reset, and enabled ACPI reset. ACPI reset does not work
> currently on kvm and qemu, but nevertheless we should try it first.
>
> Signed-off-by: YuChen Qian <[email protected]>
> Reviewed-by: Hendrik Borghorst <[email protected]>
> Reviewed-by: Marius Hillenbrand <[email protected]>
> Cc-Team: kaos-brimstone <[email protected]>
>
> CR: https://code.amazon.com/reviews/CR-14659421
> ---
>  arch/x64/power.cc | 30 +++++++++++++++++++++++-------
>  1 file changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x64/power.cc b/arch/x64/power.cc
> index 61d56bcc..d62ed84e 100644
> --- a/arch/x64/power.cc
> +++ b/arch/x64/power.cc
> @@ -54,16 +54,32 @@ void poweroff(void)
>      halt();
>  }
>
> +void pci_reboot(void) {
>

Please make these functions "static", so they are not visible from outside
this source file (there is no reason why they should be).

+    u8 v = processor::inb(0x0cf9) & ~6;
> +    processor::outb(v|2, 0x0cf9); // request hard reset
> +    usleep(50);
> +    processor::outb(v|6, 0x0cf9); // actually do the reset
>
+    usleep(50);
>

Nice, I wasn't familiar with this method before.

Is it really necessary to write 0x2 once, wait, and then write 0x2 | 0x4
("6") in a second write?
Won't a single write of 6 - once - work, as the (virtual) hardware realizes
that both 0x4 (do a reset) and 0x2 (make it a hardware reset) bits are
turned on?

+}
> +
> +void kbd_reboot(void) {
> +    while (processor::inb(0x64) & 0x02); // clear all keyboard buffers
> +    processor::outb(0xfe, 0x64);
> +    usleep(50);
> +}
> +
>  void reboot(void)
>  {
> -    // It would be nice if AcpiReset() worked, but it doesn't seem to work
> -    // (on qemu & kvm), so let's resort to other techniques, which appear
> -    // to work. Hopefully one of them will work on any hypervisor.
> -    // Method 1: "fast reset" via System Control Port A (port 0x92)
> -    processor::outb(1, 0x92);
> +    // Method 1: AcpiReset, does not work on qemu or kvm now because the
> reset
> +    // register is not supported. Nevertheless, we should try it first
> +    AcpiReset();
>      // Method 2: Reset using the 8042 PS/2 Controller ("keyboard
> controller")
> -    processor::outb(0xfe, 0x64);
> -    // Method 3: Cause triple fault by loading a broken IDT and
> triggering an
> +    kbd_reboot();
> +    // Method 3: PCI reboot
> +    pci_reboot();
> +    // Method 4: "fast reset" via System Control Port A (port 0x92)
> +    processor::outb(1, 0x92);
>

I'm curious why you moved this, which was the first method we tried until
now, to be the fourth. What's
the downside of doing it first?

+    // Method 5: Cause triple fault by loading a broken IDT and triggering
> an
>      // interrupt.
>      processor::lidt(processor::desc_ptr(0, 0));
>      __asm__ __volatile__("int3");
> --
> 2.17.1
>
>
>
>
> Amazon Development Center Germany GmbH
> Krausenstr. 38
> 10117 Berlin
> Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
> Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
> Sitz: Berlin
> Ust-ID: DE 289 237 879
>
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "OSv Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to [email protected].
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/osv-dev/20191127134607.15293-1-yuchenq%40amazon.de
> .
>

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/osv-dev/CANEVyjtjhQv%3DUcxFLhLPB3%3Dxw5Zq7jQ3Fgzc-r6oJ1K8%3D7BuWA%40mail.gmail.com.

Reply via email to