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=-7.5 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no 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 8A367C4743C for ; Wed, 23 Jun 2021 11:14:59 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id DD78461076 for ; Wed, 23 Jun 2021 11:14:58 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DD78461076 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:52990 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lw0qP-0007Mg-LM for qemu-devel@archiver.kernel.org; Wed, 23 Jun 2021 07:14:57 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:36164) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lw0pX-0006Oh-Eu for qemu-devel@nongnu.org; Wed, 23 Jun 2021 07:14:03 -0400 Received: from mail-oi1-x232.google.com ([2607:f8b0:4864:20::232]:33321) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1lw0pU-00056P-So for qemu-devel@nongnu.org; Wed, 23 Jun 2021 07:14:02 -0400 Received: by mail-oi1-x232.google.com with SMTP id s17so2981021oij.0 for ; Wed, 23 Jun 2021 04:14:00 -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; bh=wxN4AilJ6fMpO6BDzvIKWDBl3QjYFr055bALHsXhvWQ=; b=ATPeS+VorGr3C+fOwgos3VLcX7BMec+XP8pbyl2rmR6nEWyMf4jGfMyUgIJX0Xm+Cu jnYyIU4tPvWwSAQelkZfYfAOa9JHEXSMP8Szox3nAisLyqfE/9DuJ/QDUFhV+wo81evm Tmiew6ZtaFWrHuo922lXdISBhBBllBhM6jkeCwzNX2TDs2+LyNYcI52jkN4gRrlWyc9x 1VhPHtIcj4yc+JbRYdOe5JcHOkBJhqkg6Fi7hJjaa/0swjyf1KCEhvo9Sj9yYIFHv7ZF NwGQc25MSVEsgCTFAhW2pN55sFZlzooOaV8ub3UhdE3SSzSzLoZg2UxKEzbIxgXst+HV 0IkQ== 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; bh=wxN4AilJ6fMpO6BDzvIKWDBl3QjYFr055bALHsXhvWQ=; b=XUVswmfls6YyPfi2mfwGkjIAUMNF52rdAJM9e6AU4r4Z+S8hKKmCdcJjWZZWdTDZFL deanfS62xbzoEaMd52+bPu4+WHebpM0hp0X7ygnE1yqY9Erbvy9mRG6kju8QWTz+KBAY /f9Nkocp9DWEHpXfd+K9+nSD8jSJ60X3848CjdgcznAuLJaKsJKe2x8hs4LnJvt3Mqfv WW9+5Op5UR0cuKcFVpYQHtFyeb5qaaIWZq7qRWcp0n64A5XfuVQPdtPFk55l5bTawrdK a8rE7BCUOzyXwQrKjZvt7YUPx23aqY9JnkwTKCHLmxW3yDkEVYmr+cvOXPslI2imdk8T JHwQ== X-Gm-Message-State: AOAM5305S1BHe+QXrvNeDXlshKiRas8h9qM6Wxz85HasYX7AVNObHfFV 8CTa+ezpmQoRpbO2lCib1M0pX0g+KR2VK8XB+is= X-Google-Smtp-Source: ABdhPJyD3uTo4ih+SFvTX0OYH7qZynS1jzOlDtbHhZZWvuB815ZeHVS159Qbb8D0zrqege/KvPlSodC59wTBHGgWcqk= X-Received: by 2002:aca:c453:: with SMTP id u80mr2646863oif.93.1624446839711; Wed, 23 Jun 2021 04:13:59 -0700 (PDT) MIME-Version: 1.0 References: <20210617190739.3673064-1-jusual@redhat.com> <20210617190739.3673064-7-jusual@redhat.com> In-Reply-To: <20210617190739.3673064-7-jusual@redhat.com> From: Marcel Apfelbaum Date: Wed, 23 Jun 2021 14:13:48 +0300 Message-ID: Subject: Re: [PATCH v5 6/7] hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35 To: Julia Suvorova Content-Type: multipart/alternative; boundary="000000000000d66b4705c56d005a" Received-SPF: pass client-ip=2607:f8b0:4864:20::232; envelope-from=marcel.apfelbaum@gmail.com; helo=mail-oi1-x232.google.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Igor Mammedov , qemu devel list , Eduardo Habkost , "Michael S. Tsirkin" Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --000000000000d66b4705c56d005a Content-Type: text/plain; charset="UTF-8" On Thu, Jun 17, 2021 at 10:08 PM Julia Suvorova wrote: > Q35 has three different types of PCI devices hot-plug: PCIe Native, > SHPC Native and ACPI hot-plug. This patch changes the default choice > for cold-plugged bridges from PCIe Native to ACPI Hot-plug with > ability to use SHPC and PCIe Native for hot-plugged bridges. > > I personally think the best of all worlds is to have as default: - Native PCIe hotplug for pcie-root-ports - ACPI hotplug for devices behind PCI bridges (pcie-pci bridge or pci-pci bridge). Having ACPI hotplug *as default* for pcie-root-ports seems weird even if it solves existing bugs (we should solve them...). I am not against having a property to force ACPI hotplug even for pcie root-ports, just not being the default. It doesn't have to be part of this series, it can be a patch on top. I would leave APCI hotplug as an option and not as the default, giving us time to test it, come up with a fix for the pcie-hotplug race and a patch for allowing ACPI hotplug only for the bridges. Thanks, Marcel > This is a list of the PCIe Native hot-plug issues that led to this > change: > * no racy behavior during boot (see 110c477c2ed) > * no delay during deleting - after the actual power off software > must wait at least 1 second before indicating about it. This case > is quite important for users, it even has its own bug: > https://bugzilla.redhat.com/show_bug.cgi?id=1594168 > * no timer-based behavior - in addition to the previous example, > the attention button has a 5-second waiting period, during which > the operation can be canceled with a second press. While this > looks fine for manual button control, automation will result in > the need to queue or drop events, and the software receiving > events in all sort of unspecified combinations of attention/power > indicator states, which is racy and uppredictable. > * fixes: > * https://bugzilla.redhat.com/show_bug.cgi?id=1752465 > * https://bugzilla.redhat.com/show_bug.cgi?id=1690256 > > To return to PCIe Native hot-plug: > -global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off > > Signed-off-by: Julia Suvorova > Reviewed-by: Igor Mammedov > --- > hw/acpi/ich9.c | 2 +- > hw/i386/pc.c | 1 + > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c > index bcbd567cb0..daa3cd44b1 100644 > --- a/hw/acpi/ich9.c > +++ b/hw/acpi/ich9.c > @@ -424,7 +424,7 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs > *pm) > pm->disable_s3 = 0; > pm->disable_s4 = 0; > pm->s4_val = 2; > - pm->use_acpi_hotplug_bridge = false; > + pm->use_acpi_hotplug_bridge = true; > > object_property_add_uint32_ptr(obj, ACPI_PM_PROP_PM_IO_BASE, > &pm->pm_io_base, OBJ_PROP_FLAG_READ); > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index c6d8d0d84d..cc10589552 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -98,6 +98,7 @@ GlobalProperty pc_compat_6_0[] = { > { "qemu64" "-" TYPE_X86_CPU, "family", "6" }, > { "qemu64" "-" TYPE_X86_CPU, "model", "6" }, > { "qemu64" "-" TYPE_X86_CPU, "stepping", "3" }, > + { "ICH9-LPC", "acpi-pci-hotplug-with-bridge-support", "off" }, > }; > const size_t pc_compat_6_0_len = G_N_ELEMENTS(pc_compat_6_0); > > -- > 2.30.2 > > --000000000000d66b4705c56d005a Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Thu, Jun 17, 2021 at 10:08 PM Juli= a Suvorova <jusual@redhat.com&g= t; wrote:
Q35 ha= s three different types of PCI devices hot-plug: PCIe Native,
SHPC Native and ACPI hot-plug. This patch changes the default choice
for cold-plugged bridges from PCIe Native to ACPI Hot-plug with
ability to use SHPC and PCIe Native for hot-plugged bridges.


I personally=C2=A0think the best of al= l worlds is to have as default:
=C2=A0- Native PCIe hotplug for p= cie-root-ports
=C2=A0- ACPI hotplug for devices behind PCI bridge= s (pcie-pci bridge or pci-pci bridge).

Having ACPI= hotplug *as default* for=C2=A0pcie-root-ports seems weird even
i= f it solves existing=C2=A0bugs (we should solve them...).

I am not against having a property to force ACPI hotplug even for p= cie root-ports,
just not being the default. It doesn't have t= o=C2=A0 be part of this series, it can be a patch on
top.

I would leave APCI hotplug as an option and not as the de= fault, giving us time to test it,
come up with a fix for the pcie= -hotplug race and a patch for allowing ACPI hotplug
only for the = bridges.

Thanks,
Marcel

=C2=A0
This is a list of the PCIe Native hot-plug issues that led to this
change:
=C2=A0 =C2=A0 * no racy behavior during boot (see 110c477c2ed)
=C2=A0 =C2=A0 * no delay during deleting - after the actual power off softw= are
=C2=A0 =C2=A0 =C2=A0 must wait at least 1 second before indicating about it= . This case
=C2=A0 =C2=A0 =C2=A0 is quite important for users, it even has its own bug:=
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 https://bugz= illa.redhat.com/show_bug.cgi?id=3D1594168
=C2=A0 =C2=A0 * no timer-based behavior - in addition to the previous examp= le,
=C2=A0 =C2=A0 =C2=A0 the attention button has a 5-second waiting period, du= ring which
=C2=A0 =C2=A0 =C2=A0 the operation can be canceled with a second press. Whi= le this
=C2=A0 =C2=A0 =C2=A0 looks fine for manual button control, automation will = result in
=C2=A0 =C2=A0 =C2=A0 the need to queue or drop events, and the software rec= eiving
=C2=A0 =C2=A0 =C2=A0 events in all sort of unspecified combinations of atte= ntion/power
=C2=A0 =C2=A0 =C2=A0 indicator states, which is racy and uppredictable.
=C2=A0 =C2=A0 * fixes:
=C2=A0 =C2=A0 =C2=A0 =C2=A0 * https://bugzilla.= redhat.com/show_bug.cgi?id=3D1752465
=C2=A0 =C2=A0 =C2=A0 =C2=A0 * https://bugzilla.= redhat.com/show_bug.cgi?id=3D1690256

To return to PCIe Native hot-plug:
=C2=A0 =C2=A0 -global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=3Doff
Signed-off-by: Julia Suvorova <jusual@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
=C2=A0hw/acpi/ich9.c | 2 +-
=C2=A0hw/i386/pc.c=C2=A0 =C2=A0| 1 +
=C2=A02 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index bcbd567cb0..daa3cd44b1 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -424,7 +424,7 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs = *pm)
=C2=A0 =C2=A0 =C2=A0pm->disable_s3 =3D 0;
=C2=A0 =C2=A0 =C2=A0pm->disable_s4 =3D 0;
=C2=A0 =C2=A0 =C2=A0pm->s4_val =3D 2;
-=C2=A0 =C2=A0 pm->use_acpi_hotplug_bridge =3D false;
+=C2=A0 =C2=A0 pm->use_acpi_hotplug_bridge =3D true;

=C2=A0 =C2=A0 =C2=A0object_property_add_uint32_ptr(obj, ACPI_PM_PROP_PM_IO_= BASE,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 &pm->pm_io_base= , OBJ_PROP_FLAG_READ);
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index c6d8d0d84d..cc10589552 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -98,6 +98,7 @@ GlobalProperty pc_compat_6_0[] =3D {
=C2=A0 =C2=A0 =C2=A0{ "qemu64" "-" TYPE_X86_CPU, "= family", "6" },
=C2=A0 =C2=A0 =C2=A0{ "qemu64" "-" TYPE_X86_CPU, "= model", "6" },
=C2=A0 =C2=A0 =C2=A0{ "qemu64" "-" TYPE_X86_CPU, "= stepping", "3" },
+=C2=A0 =C2=A0 { "ICH9-LPC", "acpi-pci-hotplug-with-bridge-s= upport", "off" },
=C2=A0};
=C2=A0const size_t pc_compat_6_0_len =3D G_N_ELEMENTS(pc_compat_6_0);

--
2.30.2

--000000000000d66b4705c56d005a--