From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.6 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 88E96C433E2 for ; Tue, 15 Sep 2020 01:36:33 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3918D212CC for ; Tue, 15 Sep 2020 01:36:33 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="lG2JqnYY" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726045AbgIOBg2 (ORCPT ); Mon, 14 Sep 2020 21:36:28 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56366 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726019AbgIOBg1 (ORCPT ); Mon, 14 Sep 2020 21:36:27 -0400 Received: from mail-il1-x144.google.com (mail-il1-x144.google.com [IPv6:2607:f8b0:4864:20::144]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C332CC06174A; Mon, 14 Sep 2020 18:36:26 -0700 (PDT) Received: by mail-il1-x144.google.com with SMTP id u18so1422600iln.13; Mon, 14 Sep 2020 18:36:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=1rBPwHwU2FkVKjAeAJcaXFgeOnP2PmSEpXOhpG8dYA8=; b=lG2JqnYY2TwYs7ftKsQ7Hj7XI/MeSctq2Gda3b7kK1Ln96SAzjPxmsLHJIy3s0NXWp QiBhExv43OxLhhMhbK73QDmHQnxtubR+YGPok+DyRX0RTH3CSQbMLSjKltyugzlhH7Rw Y6WJIO6QaDpWFVg/HuGPsqVttFMKb9UQNs/RUjerL4q4efKZ1LYLjMMsaHNuyTYFCmQf 1GzD4cEyjs8LuXRQSzp45yjaBlp+w5DvNrn7mhAqRi9rBdMYZ81f+BlcBkE3t9yUckmX jSKI1tYcIs4ljJKSRAkGNYFAdffhvPpvKGX5RmEVfmtIoGWUP2LSM+QBuPi2ZJb8JUyY Z6+A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=1rBPwHwU2FkVKjAeAJcaXFgeOnP2PmSEpXOhpG8dYA8=; b=WhKbEAPyfSpSFuc71rw6cbDI9theL6UWUDEqAAHp6k3W/U94yLlyTWqjyTlygDHRf6 vOGT1UNqhYLdAHy+yeFBy5mtosyEX539WL35OvexeZb9ULoSG73qnr6mZS4QsJYULck9 AzMscW5KXlX9ijPWrp0BBLLZmjxr/5oS1Fd1au8m+3CmVgfG6L5oJ4F+RXTaQScHt5pU l8cEqv2FW4tIfjnafO9e5RoclOdTcWRdDEnVFxSQAT9fji+Q21RiCEPEmQWZ+KlTRG4L vPFbQDPFdQsk1zI18Ltcq/08AXeYDw18BtyMGekMU/DFdq8BcfRjNEqPxqSbGgkbjMfu wtLA== X-Gm-Message-State: AOAM531pqZnTIg0drXDZDdudk7cr2p/K/QCBdJKfBxWLOujvcGLwjygk fB5wRSKbK4ucj/jOT+rlCcZxJlzjGf8Aryqg82U= X-Google-Smtp-Source: ABdhPJxtPrlM7Cw/cVdSUBwfIV+ugJcy/PfYbfSAsEaCdsTIT0DXPECiraeh9Oedh7CWgIoriAz5X6e0EOwIptU1OSI= X-Received: by 2002:a92:d1d0:: with SMTP id u16mr9570660ilg.171.1600133785312; Mon, 14 Sep 2020 18:36:25 -0700 (PDT) MIME-Version: 1.0 References: <1600070215-3901-1-git-send-email-yangtiezhu@loongson.cn> <45a3e3a0-3dc6-e60e-9381-b436a7d6889a@loongson.cn> In-Reply-To: From: Huacai Chen Date: Tue, 15 Sep 2020 09:36:13 +0800 Message-ID: Subject: Re: [RFC PATCH v3] PCI/portdrv: Only disable Bus Master on kexec reboot and connected PCI devices To: Tiezhu Yang Cc: Bjorn Helgaas , linux-pci , linux-kernel , "Rafael J. Wysocki" , Konstantin Khlebnikov , Khalid Aziz , Vivek Goyal , Lukas Wunner , "Oliver O'Halloran" , Jiaxun Yang , Xuefeng Li , zhouyanjie , git Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org Hi, Tiezhu, On Mon, Sep 14, 2020 at 7:25 PM Tiezhu Yang wrote: > > On 09/14/2020 05:46 PM, Huacai Chen wrote: > > Hi, Tiezhu, > > > > On Mon, Sep 14, 2020 at 5:30 PM Tiezhu Yang wr= ote: > >> On 09/14/2020 04:52 PM, Huacai Chen wrote: > >>> Hi, Tiezhu, > >>> > >>> How do you test kexec? kexec -e or systemctl kexec? Or both? > >> kexec -l vmlinux --append=3D"root=3D/dev/sda2 console=3DttyS0,115200" > >> kexec -e > > So you haven't tested "systemctl kexec"? > > Yes, the distro I used is Loongnix which has not kexec service now. > Is there any problem when use systemctl kexec? If you have more details, > please let me know. If you use systemctl kexec, the first part of kexec is the same as a normal reboot/poweroff. So, there is no reason that reboot/poweroff is bad but kexec is good. Then, Bjorn, what is the best solution now? It seems like my first version is OK (commit messages should be improved, of course). Huacai > > > > > Huacai > >>> P.S., Please also CC my gmail (chenhuacai@gmail.com) since lemote.com > >>> has some communication problems. > >> OK, no problem. > >> > >>> Huacai > >>> > >>>> =E9=99=88=E5=8D=8E=E6=89=8D=E6=B1=9F=E8=8B=8F=E8=88=AA=E5=A4=A9=E9= =BE=99=E6=A2=A6=E4=BF=A1=E6=81=AF=E6=8A=80=E6=9C=AF=E6=9C=89=E9=99=90=E5=85= =AC=E5=8F=B8/=E7=A0=94=E5=8F=91=E4=B8=AD=E5=BF=83/=E8=BD=AF=E4=BB=B6=E9=83= =A8 ------------------ Original ------------------From: "Tiezhu Yang";Date: Mon, Sep 14, 2020 03:57 PMTo: "Bjorn Helgaas"= ; Cc: "linux-pci"; "linux-= kernel"; "Rafael J. Wysocki"; "Konstantin Khlebnikov"; "Khalid Aziz"<= khalid.aziz@oracle.com>; "Vivek Goyal"; "Lukas Wunner"; "Oliver O'Halloran"; "Huacai Chen"; "Jiaxun Yang"; "Xuefeng Li"; Subject: [RFC PATCH v3] PCI/portdrv: Only disable Bus Mas= ter on kexec reboot and connected PCI devices After commit 745be2e700cd ("P= CIe: portdrv: call pci_disable_device > >>>> during remove") and commit cc27b735ad3a ("PCI/portdrv: Turn off PCIe > >>>> services during shutdown"), it also calls pci_disable_device() durin= g > >>>> shutdown, this leads to shutdown or reboot failure occasionally due = to > >>>> clear PCI_COMMAND_MASTER on the device in do_pci_disable_device(). > >>>> > >>>> drivers/pci/pci.c > >>>> static void do_pci_disable_device(struct pci_dev *dev) > >>>> { > >>>> u16 pci_command; > >>>> > >>>> pci_read_config_word(dev, PCI_COMMAND, &pci_command); > >>>> if (pci_command & PCI_COMMAND_MASTER) { > >>>> pci_command &=3D ~PCI_COMMAND_MASTER; > >>>> pci_write_config_word(dev, PCI_COMMAND, pci_comman= d); > >>>> } > >>>> > >>>> pcibios_disable_device(dev); > >>>> } > >>>> > >>>> When remove "pci_command &=3D ~PCI_COMMAND_MASTER;", it can work wel= l when > >>>> shutdown or reboot. > >>>> > >>>> As Oliver O'Halloran said, no need to call pci_disable_device() when > >>>> actually shutting down, but we should call pci_disable_device() befo= re > >>>> handing over to the new kernel on kexec reboot, so we can do some > >>>> condition checks which are already executed afterwards by the functi= on > >>>> pci_device_shutdown(), this is done by commit 4fc9bbf98fd6 ("PCI: Di= sable > >>>> Bus Master only on kexec reboot") and commit 6e0eda3c3898 ("PCI: Don= 't try > >>>> to disable Bus Master on disconnected PCI devices"). > >>>> > >>>> drivers/pci/pci-driver.c > >>>> static void pci_device_shutdown(struct device *dev) > >>>> { > >>>> ... > >>>> if (drv && drv->shutdown) > >>>> drv->shutdown(pci_dev); > >>>> > >>>> /* > >>>> * If this is a kexec reboot, turn off Bus Master bit on t= he > >>>> * device to tell it to not continue to do DMA. Don't touc= h > >>>> * devices in D3cold or unknown states. > >>>> * If it is not a kexec reboot, firmware will hit the PCI > >>>> * devices with big hammer and stop their DMA any way. > >>>> */ > >>>> if (kexec_in_progress && (pci_dev->current_state <=3D PCI_= D3hot)) > >>>> pci_clear_master(pci_dev); > >>>> } > >>>> > >>>> [ 36.159446] Call Trace: > >>>> [ 36.241688] [] show_stack+0x9c/0x130 > >>>> [ 36.326619] [] dump_stack+0xb0/0xf0 > >>>> [ 36.410403] [] pcie_portdrv_shutdown+0x18/0x78 > >>>> [ 36.495302] [] pci_device_shutdown+0x44/0x90 > >>>> [ 36.580027] [] device_shutdown+0x130/0x290 > >>>> [ 36.664486] [] kernel_power_off+0x38/0x80 > >>>> [ 36.748272] [] __do_sys_reboot+0x1a4/0x258 > >>>> [ 36.831985] [] syscall_common+0x34/0x58 > >>>> > >>>> Signed-off-by: Tiezhu Yang > >>>> --- > >>>> drivers/pci/pcie/portdrv_core.c | 1 - > >>>> drivers/pci/pcie/portdrv_pci.c | 14 +++++++++++++- > >>>> 2 files changed, 13 insertions(+), 2 deletions(-) > >>>> > >>>> diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/port= drv_core.c > >>>> index 50a9522..1991aca 100644 > >>>> --- a/drivers/pci/pcie/portdrv_core.c > >>>> +++ b/drivers/pci/pcie/portdrv_core.c > >>>> @@ -491,7 +491,6 @@ void pcie_port_device_remove(struct pci_dev *dev= ) > >>>> { > >>>> device_for_each_child(&dev->dev, NULL, remove_iter); > >>>> pci_free_irq_vectors(dev); > >>>> - pci_disable_device(dev); > >>>> } > >>>> > >>>> /** > >>>> diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portd= rv_pci.c > >>>> index 3a3ce40..cab37a8 100644 > >>>> --- a/drivers/pci/pcie/portdrv_pci.c > >>>> +++ b/drivers/pci/pcie/portdrv_pci.c > >>>> @@ -143,6 +143,18 @@ static void pcie_portdrv_remove(struct pci_dev = *dev) > >>>> } > >>>> > >>>> pcie_port_device_remove(dev); > >>>> + pci_disable_device(dev); > >>>> +} > >>>> + > >>>> +static void pcie_portdrv_shutdown(struct pci_dev *dev) > >>>> +{ > >>>> + if (pci_bridge_d3_possible(dev)) { > >>>> + pm_runtime_forbid(&dev->dev); > >>>> + pm_runtime_get_noresume(&dev->dev); > >>>> + pm_runtime_dont_use_autosuspend(&dev->dev); > >>>> + } > >>>> + > >>>> + pcie_port_device_remove(dev); > >>>> } > >>>> > >>>> static pci_ers_result_t pcie_portdrv_error_detected(struct pci_de= v *dev, > >>>> @@ -211,7 +223,7 @@ static struct pci_driver pcie_portdriver =3D { > >>>> > >>>> .probe =3D pcie_portdrv_probe, > >>>> .remove =3D pcie_portdrv_remove, > >>>> - .shutdown =3D pcie_portdrv_remove, > >>>> + .shutdown =3D pcie_portdrv_shutdown, > >>>> > >>>> .err_handler =3D &pcie_portdrv_err_handler, > >>>> > >>>> -- > >>>> 2.1.0 >