From patchwork Tue Feb 9 22:59:56 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Wei Huang X-Patchwork-Id: 8266761 Return-Path: X-Original-To: patchwork-qemu-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 14D479F1C1 for ; Tue, 9 Feb 2016 23:00:19 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 4DC0B2020F for ; Tue, 9 Feb 2016 23:00:18 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 3C14D201F2 for ; Tue, 9 Feb 2016 23:00:17 +0000 (UTC) Received: from localhost ([::1]:33321 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTHGa-00049H-Ba for patchwork-qemu-devel@patchwork.kernel.org; Tue, 09 Feb 2016 18:00:16 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43993) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTHGQ-00046Y-Gt for qemu-devel@nongnu.org; Tue, 09 Feb 2016 18:00:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aTHGL-0007mU-DW for qemu-devel@nongnu.org; Tue, 09 Feb 2016 18:00:06 -0500 Received: from mx1.redhat.com ([209.132.183.28]:44635) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTHGL-0007ll-5q; Tue, 09 Feb 2016 18:00:01 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id 4F8BC155DE; Tue, 9 Feb 2016 22:59:59 +0000 (UTC) Received: from [10.10.51.89] (unused [10.10.51.89] (may be forged)) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u19MxvtD031449 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 9 Feb 2016 17:59:57 -0500 To: Shannon Zhao , Peter Maydell , Michael Tokarev References: <1454005340-15682-1-git-send-email-wei@redhat.com> <56B1A90E.3000506@msgid.tls.msk.ru> <56B22469.7040308@redhat.com> <56B2AD13.6030504@huawei.com> <56B2EB3E.2000908@redhat.com> <56B2F4E3.6010807@huawei.com> From: Wei Huang Message-ID: <56BA6F6C.5000301@redhat.com> Date: Tue, 9 Feb 2016 16:59:56 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 MIME-Version: 1.0 In-Reply-To: <56B2F4E3.6010807@huawei.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: QEMU Trivial , QEMU Developers , Shannon Zhao Subject: Re: [Qemu-devel] [PATCH 1/1] arm: virt: change GPIO trigger interrupt to pulse X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 02/04/2016 12:51 AM, Shannon Zhao wrote: > > > On 2016/2/4 14:10, Wei Huang wrote: >> >> On 02/03/2016 07:44 PM, Shannon Zhao wrote: >> I reversed the order of edge pulling. The state is 1 according to printk >> inside gpio_keys driver. However the reboot still failed with two >> reboots (1 very early, 1 later). >> > Because to make the input work, it should call input_event twice I think. > > input_event(input, type, button->code, 1) means the button pressed > input_event(input, type, button->code, 0) means the button released > > But We only see guest entering gpio_keys_gpio_report_event once. > > My original purpose is like below: > > call qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1) to make guest > execute input_event(input, type, button->code, 1) > call qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 0) to make guest > execute input_event(input, type, button->code, 0). > > But even though it calls qemu_set_irq twice, it only calls pl061_update > once in qemu. Hi Shannon, Assuming that we are talking about the special case you found (i.e. send reboot very early and then send another one after guest VM fully booted). Dug into the code further, here are my findings: === Why ACPI case failed? === QEMU pl061.c checks the current request against the new request (see s->old_in_data ^ s->data in pl061.c file). If no change, nothing will happen. So two consecutive reboots will cause the following state change; apparently the second request won't trigger VM reboot because pl01_update() decides _not_ to inject IRQ into guest VM. (PL061State fields) data old_in_data istate VM boot 0 0 0 1st ACPI reboot request 8 0 0 After VM PL061 driver ACK 8 8 0 2nd ACPI reboot request 8 no-change, no IRQ <== To solve this problem, we have to invert PL061State->data at least once to trigger IRQ inside VM. Two solutions: S1: "Pulse" static void virt_powerdown_req(Notifier *n, void *opaque) { qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 0); qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1); } S2: "Invert" static int old_gpio_level = 0; static void virt_powerdown_req(Notifier *n, void *opaque) { /* use gpio Pin 3 for power button event */ qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), !old_gpio_level); old_gpio_level = !old_gpio_level; } Both S1 and S2 works for ACPI. But S1 has problem with DT, which is explained below. === Why DT fails with S1 === DT approach requires both PL061 and gpio-keys to work together. Looking into guest kernel gpio-keys/input code, you will find that it only reacts to both LEVEL-HI and input changes. Here is the code snip from drivers/input/input.c file: /* auto-repeat bypasses state updates */ if (value == 2) { disposition = INPUT_PASS_TO_HANDLERS; break; } if (!!test_bit(code, dev->key) != !!value) { __change_bit(code, dev->key); disposition = INPUT_PASS_TO_HANDLERS; } Unless we adds gpio-keys DT property to "autorepeat", the "!!test_bit(code, dev->key) != !!value" is always FALSE with S1. Thus systemd won't receive any input event; and no power-switch will be triggered. In comparison S2 will work because value is changed very time. === Summary === 1. Cleaning PL061 state is required. That means "[PATCH V2 1/2] ARM: PL061: Clear PL061 device state after reset" is necessary. 2. S2 is better. To support S2, we needs to change GPIO IRQ polarity to AML_ACTIVE_BOTH in ACPI description. I attach a small patch in this email. It should work for your DT case. Please test it and let me know your comments. Thanks, -Wei > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 7b124f6..7b1dc5e 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -326,7 +326,7 @@ static void acpi_dsdt_add_gpio(Aml *scope, const MemMapEntry *gpio_memmap, Aml *aei = aml_resource_template(); /* Pin 3 for power button */ const uint32_t pin_list[1] = {3}; - aml_append(aei, aml_gpio_int(AML_CONSUMER, AML_EDGE, AML_ACTIVE_HIGH, + aml_append(aei, aml_gpio_int(AML_CONSUMER, AML_EDGE, 3, AML_EXCLUSIVE, AML_PULL_UP, 0, pin_list, 1, "GPO0", NULL, 0)); aml_append(dev, aml_name_decl("_AEI", aei)); diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 5695e72..ed02a6c 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -547,10 +547,12 @@ static void create_rtc(const VirtBoardInfo *vbi, qemu_irq *pic) } static DeviceState *pl061_dev; +static int old_gpio_level = 0; static void virt_powerdown_req(Notifier *n, void *opaque) { /* use gpio Pin 3 for power button event */ - qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1); + qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), !old_gpio_level); + old_gpio_level = !old_gpio_level; } static Notifier virt_system_powerdown_notifier = {