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=-6.5 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 EFF23C43461 for ; Wed, 9 Sep 2020 18:32:16 +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 2A1C720C09 for ; Wed, 9 Sep 2020 18:32:16 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=anisinha-ca.20150623.gappssmtp.com header.i=@anisinha-ca.20150623.gappssmtp.com header.b="zlRg5wEH" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2A1C720C09 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=anisinha.ca Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:34430 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kG4tD-0000zX-7m for qemu-devel@archiver.kernel.org; Wed, 09 Sep 2020 14:32:15 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:53218) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kG4sJ-0000Xq-58 for qemu-devel@nongnu.org; Wed, 09 Sep 2020 14:31:19 -0400 Received: from mail-pf1-x444.google.com ([2607:f8b0:4864:20::444]:44571) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kG4sG-0005uw-EQ for qemu-devel@nongnu.org; Wed, 09 Sep 2020 14:31:18 -0400 Received: by mail-pf1-x444.google.com with SMTP id o20so2892772pfp.11 for ; Wed, 09 Sep 2020 11:31:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=anisinha-ca.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:message-id:in-reply-to:references:subject :mime-version; bh=WPEDgy3gwK94AG6B/UduphjaOI+yXvAfd2+D93+SmwY=; b=zlRg5wEHxictInYw848aTDOIoZSeCpzlW9V3W9drj/EJSD8uj29mhPN4bEKFD8ygQa asI8qTvryHNlX+YHjKJhJfnqYT//b4En8O5mRV2VJH+lKtu+DLtsQsXnjefgf3QWReLp 5zljlMwbhc83cSvkQicNoJLv14rcZlOk1C6Looc8zl+t9ui50sXC5usMY1VeWI6RAadO 7rFpPTxj/E4D0NjqGkIIdO1URWkFpp+pb2vtCAz7baLABbNrp8t0TWE6SMr7v7gPzNHX 3pdgQCOiyDENU+m6CSMqUYbEsHdUSyy6gbyDmjNbjCFnGPZKROlJhWeBXLwdEzYEWQQD ObFw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:message-id:in-reply-to :references:subject:mime-version; bh=WPEDgy3gwK94AG6B/UduphjaOI+yXvAfd2+D93+SmwY=; b=bSHU1iluoKv9QqREzWb0gtf6hoCJJFweG6EV7U6WJF78sO/ZRnHHdfNeaU7jBh4cPT /7ZyQbMbxPco/UPHFzGfot15rAi8BxIykSZWIMAZLAULpndwSf6eCK9ERYGjVOVbYCd+ 1tmAOw9T3fCXbzn1cJu7ZU7b/5AvdqkN9utjLoRgqHNc5omrejImJ6IeRxv6I/96v6Fq ZcX4NCqy0LQHHdBZnr5A+A86LGzZFAXVtBZ8EOmtJ+lG5Mmlaa0NDCuQDYPQH/aU4LZk COtBUY80FNSDwEf3wpnsi91ue5KLZ7czyeAGsToa/2ZlDhS9yJNt3Ymav8+8C2XDR4rM yjyA== X-Gm-Message-State: AOAM530rm7a6kGEv/3wGeVKFzvA2WCcivpnZWbuwIpyDQYM6X1yAa9Ob Ba1HcNGgNeRVQ8TF2taXGiRbeQ== X-Google-Smtp-Source: ABdhPJwQb8p3xW/zEiIpp+FLX6LLxuGySApWvcoCzjDk3bK5CDMhGq/Il/7MAEOhhaIHYVC7F5DEAA== X-Received: by 2002:a17:902:c192:: with SMTP id d18mr2115921pld.36.1599676274467; Wed, 09 Sep 2020 11:31:14 -0700 (PDT) Received: from [192.168.1.4] ([115.96.128.213]) by smtp.gmail.com with ESMTPSA id gk14sm2599959pjb.41.2020.09.09.11.31.10 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Wed, 09 Sep 2020 11:31:13 -0700 (PDT) Date: Thu, 10 Sep 2020 00:00:54 +0530 From: Ani Sinha To: Julia Suvorova Message-ID: <98b26cfa-3ff7-4302-9fc2-0e6672cfbd08@Spark> In-Reply-To: References: <20200904161000.12115-1-ani@anisinha.ca> Subject: Re: [PATCH v3] i440fx/acpi: do not add hotplug related amls for cold plugged bridges X-Readdle-Message-ID: 98b26cfa-3ff7-4302-9fc2-0e6672cfbd08@Spark MIME-Version: 1.0 Content-Type: multipart/alternative; boundary="5f591f6b_327b23c6_1586" Received-SPF: none client-ip=2607:f8b0:4864:20::444; envelope-from=ani@anisinha.ca; helo=mail-pf1-x444.google.com X-detected-operating-system: by eggs.gnu.org: No matching host in p0f cache. That's all we know. X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_NONE=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: Eduardo Habkost , "Michael S. Tsirkin" , QEMU Developers , Paolo Bonzini , Igor Mammedov , Richard Henderson Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --5f591f6b_327b23c6_1586 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Content-Disposition: inline On Sep 9, 2020, 23:20 +0530, Julia Suvorova , wrote: > On Fri, Sep 4, 2020 at 6:10 PM Ani Sinha wrote: > > > > Cold plugged bridges are not hot unpluggable, even when their hotplug > > property (acpi-pci-hotplug-with-bridge-support) is turned off. Please see > > the function acpi_pcihp_pc_no_hotplug() (thanks Julia). However, with > > the current implementaton, windows would try to hot-unplug a pci bridge when > > it's hotplug switch is off. This is regardless of whether there are devices > > attached to the bridge. This is because we add amls like _EJ0 etc for the > > pci slot where the bridge is cold plugged. We have a demo video here: > > https://youtu.be/pME2sjyQweo > > > > In this fix, we identify a cold plugged bridge and for cold plugged bridges, > > we do not add the appropriate amls and acpi methods that are used by the OS > > to identify a hot-pluggable/unpluggable pci device. After this change, Windows > > does not show an option to eject the PCI bridge. A demo video is here: > > https://youtu.be/kbgej5B9Hgs > > > > While at it, I have also updated a stale comment. > > > > This change is tested with a Windows 2012R2 guest image and Windows 2019 server > > guest image running on Ubuntu 18.04 host. This change is based off of upstream > > qemu master branch tag v5.1.0. > > > > Signed-off-by: Ani Sinha > > Reviewed-by: Julia Suvorova > > BTW, aren't all bridges handled in build_append_pci_bus_devices() cold-plugged? Yes they are. > > > --- > > hw/i386/acpi-build.c | 12 ++++++------ > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > changelog: > > v3: commit log updates providing more accurate information as received from Julia. > > v2: cosmetic commit log updates with patch testing information. > > v1: initial patch. > > > > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > index b7bcbbbb2a..90b863f4ec 100644 > > --- a/hw/i386/acpi-build.c > > +++ b/hw/i386/acpi-build.c > > @@ -359,6 +359,7 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus, > > int slot = PCI_SLOT(i); > > bool hotplug_enabled_dev; > > bool bridge_in_acpi; > > + bool cold_plugged_bridge; > > > > if (!pdev) { > > if (bsel) { /* add hotplug slots for non present devices */ > > @@ -380,15 +381,14 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus, > > pc = PCI_DEVICE_GET_CLASS(pdev); > > dc = DEVICE_GET_CLASS(pdev); > > > > - /* When hotplug for bridges is enabled, bridges are > > - * described in ACPI separately (see build_pci_bus_end). > > - * In this case they aren't themselves hot-pluggable. > > + /* > > + * Cold plugged bridges aren't themselves hot-pluggable. > > * Hotplugged bridges *are* hot-pluggable. > > */ > > - bridge_in_acpi = pc->is_bridge && pcihp_bridge_en && > > - !DEVICE(pdev)->hotplugged; > > + cold_plugged_bridge = pc->is_bridge && !DEVICE(pdev)->hotplugged; > > + bridge_in_acpi = cold_plugged_bridge && pcihp_bridge_en; > > > > - hotplug_enabled_dev = bsel && dc->hotpluggable && !bridge_in_acpi; > > + hotplug_enabled_dev = bsel && dc->hotpluggable && !cold_plugged_bridge; > > > > if (pc->class_id == PCI_CLASS_BRIDGE_ISA) { > > continue; > > -- > > 2.17.1 > > > --5f591f6b_327b23c6_1586 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline
On Sep 9, 2020, 23:20 +0530, Julia Suvorova <jus= ual=40redhat.com>, wrote:
On =46ri, Sep 4, 2020 at 6:10 PM= Ani Sinha <ani=40anisinha.ca> wrote:

Cold plugged bridges are not hot= unpluggable, even when their hotplug
property (acpi-pci-hotplug-with-= bridge-support) is turned off. Please see
the function acpi=5Fpcihp=5Fpc=5F= no=5Fhotplug() (thanks Julia). However, with
the current implementaton, windo= ws would try to hot-unplug a pci bridge when
it's hotplug switch is off. This= is regardless of whether there are devices
attached to the bridge. This is = because we add amls like =5FEJ0 etc for the
pci slot where the bridge is col= d plugged. We have a demo video here:
https://youtu.be/pME2sjyQweo

In this fix, we identify a cold = plugged bridge and for cold plugged bridges,
we do not add the appropriate am= ls and acpi methods that are used by the OS
to identify a hot-pluggable/unpl= uggable pci device. After this change, Windows
does not show an option to eject= the PCI bridge. A demo video is here:
https://youtu.be/kbgej5B9Hgs

While at it, I have also updated= a stale comment.

This change is tested with a Win= dows 2012R2 guest image and Windows 2019 server
guest image running on Ubuntu 18= .04 host. This change is based off of upstream
qemu master branch tag v5.1.0.

Signed-off-by: Ani Sinha <ani= =40anisinha.ca>

Reviewed-by: Julia Suvorova <= jusual=40redhat.com>

BTW, aren't all bridges handled = in build=5Fappend=5Fpci=5Fbus=5Fdevices() cold-plugged=3F

Yes they are.

---
hw/i386/acpi-build.c =7C 12 ++++= ++------
1 file changed, 6 insertions(+),= 6 deletions(-)

changelog:
v3: commit log updates providing= more accurate information as received from Julia.
v2: cosmetic commit log updates = with patch testing information.
v1: initial patch.


diff --git a/hw/i386/acpi-build.= c b/hw/i386/acpi-build.c
index b7bcbbbb2a..90b863f4ec 100= 644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
=40=40 -359,6 +359,7 =40=40 stat= ic void build=5Fappend=5Fpci=5Fbus=5Fdevices(Aml *parent=5Fscope, PCIBus = *bus,
int slot =3D PCI=5FSLOT(i);
bool hotplug=5Fenabled=5Fdev;
bool bridge=5Fin=5Facpi;
+ bool cold=5Fplugged=5Fbridge;<= /blockquote>

if (=21pdev) =7B
if (bsel) =7B /* add hotplug slo= ts for non present devices */
=40=40 -380,15 +381,14 =40=40 st= atic void build=5Fappend=5Fpci=5Fbus=5Fdevices(Aml *parent=5Fscope, PCIBu= s *bus,
pc =3D PCI=5FDEVICE=5FGET=5FCLAS= S(pdev);
dc =3D DEVICE=5FGET=5FCLASS(pdev= );

- /* When hotplug for bridges is= enabled, bridges are
- * described in ACPI separately= (see build=5Fpci=5Fbus=5Fend).
- * In this case they aren't the= mselves hot-pluggable.
+ /*
+ * Cold plugged bridges aren't = themselves hot-pluggable.
* Hotplugged bridges *are* hot-p= luggable.
*/
- bridge=5Fin=5Facpi =3D pc->= is=5Fbridge && pcihp=5Fbridge=5Fen &&
- =21DEVICE(pdev)->hotplugged= ;
+ cold=5Fplugged=5Fbridge =3D pc= ->is=5Fbridge && =21DEVICE(pdev)->hotplugged;
+ bridge=5Fin=5Facpi =3D cold=5F= plugged=5Fbridge && pcihp=5Fbridge=5Fen;

- hotplug=5Fenabled=5Fdev =3D bs= el && dc->hotpluggable && =21bridge=5Fin=5Facpi;
+ hotplug=5Fenabled=5Fdev =3D bs= el && dc->hotpluggable && =21cold=5Fplugged=5Fbridge;<= /blockquote>

if (pc->class=5Fid =3D=3D PCI= =5FCLASS=5FBRIDGE=5FISA) =7B
continue;
--
2.17.1


--5f591f6b_327b23c6_1586--