Re: [PATCH v3] i440fx/acpi: do not add hotplug related amls for cold plugged bridges
On Fri, Sep 11, 2020 at 9:39 PM Michael S. Tsirkin wrote:
>
> On Fri, Sep 11, 2020 at 09:35:42PM +0530, Ani Sinha wrote:
> > On Fri, Sep 11, 2020 at 9:22 PM Michael S. Tsirkin wrote:
> > >
> > > On Thu, Sep 10, 2020 at 12:19:19AM +0530, Ani Sinha wrote:
> > > > On Sep 10, 2020, 00:00 +0530, Ani Sinha , wrote:
> > > >
> > > > 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.
> > > >
> > > >
> > > > Maybe as an improvement we can simply identify a bridge instead of a
> > > > cold
> > > > plugged bridge. However let’s have that improvement as a separate patch
> > > > on top
> > > > of this. Also let’s see what Igor thinks.
> > >
> > > Well this changes acpi tables so I don't see how this will pass
> > > the unit tests. Did you test this change?
> >
> > There were no unit tests to cover this bridge flag. I have added the
> > unit tests. It's the same one you have queued.
>
>
> Hmm. somehow the new tests fail for me ...
Ok I was working off v5.1.0 tag. Did not realize. I rebased all my
patches to the latest master and reworked the unit tests.
I have sent you the entire patch set as exists in my workspace. It
passes unit tests.
>
> > >
> > >
> > > >
> > > >
> > > >
> > > > ---
> > > >
> > > > 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 b7bc2a..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
Re: [PATCH v3] i440fx/acpi: do not add hotplug related amls for cold plugged bridges
On Fri, Sep 11, 2020 at 9:45 PM Ani Sinha wrote: > > On Fri, Sep 11, 2020 at 9:39 PM Michael S. Tsirkin wrote: > > > > On Fri, Sep 11, 2020 at 09:35:42PM +0530, Ani Sinha wrote: > > > On Fri, Sep 11, 2020 at 9:22 PM Michael S. Tsirkin > > > wrote: > > > > > > > > On Thu, Sep 10, 2020 at 12:19:19AM +0530, Ani Sinha wrote: > > > > > On Sep 10, 2020, 00:00 +0530, Ani Sinha , wrote: > > > > > > > > > > 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. > > > > > > > > > > > > > > > Maybe as an improvement we can simply identify a bridge instead of a > > > > > cold > > > > > plugged bridge. However let’s have that improvement as a separate > > > > > patch on top > > > > > of this. Also let’s see what Igor thinks. > > > > > > > > Well this changes acpi tables so I don't see how this will pass > > > > the unit tests. Did you test this change? > > > > > > There were no unit tests to cover this bridge flag. I have added the > > > unit tests. It's the same one you have queued. > > > > > > Hmm. somehow the new tests fail for me ... > > Maybe you need to pull this change out, make sure the tests passes and > then apply it again: > > commit b26b2241baefc51fe2069b83cea9cfa654e75a43 (HEAD -> v5.1.0) > Author: Ani Sinha > Date: Tue Sep 1 15:19:22 2020 +0530 > > piix4: don't reserve hw resources when hotplug is off globally > > When acpi hotplug is turned off for both root pci bus as well as for pci > bridges, we should not generate the related amls for DSDT table or > initialize > related hw ports or reserve hw resources. This change makes sure all those > operations are turned off in the case acpi pci hotplug is off globally. > > Signed-off-by: Ani Sinha For this patch, I have not sent any updated unit tests. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > --- > > > > > > > > > > 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
Re: [PATCH v3] i440fx/acpi: do not add hotplug related amls for cold plugged bridges
On Fri, Sep 11, 2020 at 9:39 PM Michael S. Tsirkin wrote: > > On Fri, Sep 11, 2020 at 09:35:42PM +0530, Ani Sinha wrote: > > On Fri, Sep 11, 2020 at 9:22 PM Michael S. Tsirkin wrote: > > > > > > On Thu, Sep 10, 2020 at 12:19:19AM +0530, Ani Sinha wrote: > > > > On Sep 10, 2020, 00:00 +0530, Ani Sinha , wrote: > > > > > > > > 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. > > > > > > > > > > > > Maybe as an improvement we can simply identify a bridge instead of a > > > > cold > > > > plugged bridge. However let’s have that improvement as a separate patch > > > > on top > > > > of this. Also let’s see what Igor thinks. > > > > > > Well this changes acpi tables so I don't see how this will pass > > > the unit tests. Did you test this change? > > > > There were no unit tests to cover this bridge flag. I have added the > > unit tests. It's the same one you have queued. > > > Hmm. somehow the new tests fail for me ... Maybe you need to pull this change out, make sure the tests passes and then apply it again: commit b26b2241baefc51fe2069b83cea9cfa654e75a43 (HEAD -> v5.1.0) Author: Ani Sinha Date: Tue Sep 1 15:19:22 2020 +0530 piix4: don't reserve hw resources when hotplug is off globally When acpi hotplug is turned off for both root pci bus as well as for pci bridges, we should not generate the related amls for DSDT table or initialize related hw ports or reserve hw resources. This change makes sure all those operations are turned off in the case acpi pci hotplug is off globally. Signed-off-by: Ani Sinha > > > > > > > > > > > > > > > > > > > > > > > --- > > > > > > > > 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 b7bc2a..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 > > > >
Re: [PATCH v3] i440fx/acpi: do not add hotplug related amls for cold plugged bridges
On Fri, Sep 11, 2020 at 09:35:42PM +0530, Ani Sinha wrote:
> On Fri, Sep 11, 2020 at 9:22 PM Michael S. Tsirkin wrote:
> >
> > On Thu, Sep 10, 2020 at 12:19:19AM +0530, Ani Sinha wrote:
> > > On Sep 10, 2020, 00:00 +0530, Ani Sinha , wrote:
> > >
> > > 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.
> > >
> > >
> > > Maybe as an improvement we can simply identify a bridge instead of a cold
> > > plugged bridge. However let’s have that improvement as a separate patch
> > > on top
> > > of this. Also let’s see what Igor thinks.
> >
> > Well this changes acpi tables so I don't see how this will pass
> > the unit tests. Did you test this change?
>
> There were no unit tests to cover this bridge flag. I have added the
> unit tests. It's the same one you have queued.
Hmm. somehow the new tests fail for me ...
> >
> >
> > >
> > >
> > >
> > > ---
> > >
> > > 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 b7bc2a..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.
> > >
> > >
Re: [PATCH v3] i440fx/acpi: do not add hotplug related amls for cold plugged bridges
On Fri, Sep 11, 2020 at 9:22 PM Michael S. Tsirkin wrote:
>
> On Thu, Sep 10, 2020 at 12:19:19AM +0530, Ani Sinha wrote:
> > On Sep 10, 2020, 00:00 +0530, Ani Sinha , wrote:
> >
> > 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.
> >
> >
> > Maybe as an improvement we can simply identify a bridge instead of a cold
> > plugged bridge. However let’s have that improvement as a separate patch on
> > top
> > of this. Also let’s see what Igor thinks.
>
> Well this changes acpi tables so I don't see how this will pass
> the unit tests. Did you test this change?
There were no unit tests to cover this bridge flag. I have added the
unit tests. It's the same one you have queued.
>
>
> >
> >
> >
> > ---
> >
> > 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 b7bc2a..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 && !
> >
Re: [PATCH v3] i440fx/acpi: do not add hotplug related amls for cold plugged bridges
On Thu, Sep 10, 2020 at 12:19:19AM +0530, Ani Sinha wrote:
> On Sep 10, 2020, 00:00 +0530, Ani Sinha , wrote:
>
> 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.
>
>
> Maybe as an improvement we can simply identify a bridge instead of a cold
> plugged bridge. However let’s have that improvement as a separate patch on top
> of this. Also let’s see what Igor thinks.
Well this changes acpi tables so I don't see how this will pass
the unit tests. Did you test this change?
>
>
>
> ---
>
> 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 b7bc2a..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
>
>
>
>
>
Re: [PATCH v3] i440fx/acpi: do not add hotplug related amls for cold plugged bridges
On Sep 10, 2020, 00:00 +0530, Ani Sinha , wrote:
> 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.
Maybe as an improvement we can simply identify a bridge instead of a cold
plugged bridge. However let’s have that improvement as a separate patch on top
of this. Also let’s see what Igor thinks.
> >
> > > ---
> > > 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 b7bc2a..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
> > >
> >
Re: [PATCH v3] i440fx/acpi: do not add hotplug related amls for cold plugged bridges
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 b7bc2a..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
> >
>
Re: [PATCH v3] i440fx/acpi: do not add hotplug related amls for cold plugged bridges
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?
> ---
> 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 b7bc2a..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
>
Re: [PATCH v3] i440fx/acpi: do not add hotplug related amls for cold plugged bridges
Gentle reminder to kindly review this patch :-)
@Julia Suvorova @Michael S. Tsirkin
On Mon, Sep 7, 2020 at 6:47 PM Ani Sinha wrote:
>
> Julia Michael,
>
> Can you please provide your inputs on this one and the corresponding unit
> tests?
>
> On Fri, Sep 4, 2020 at 9:40 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
>>
>> ---
>>
>> 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 b7bc2a..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
>>
>>
>>
Re: [PATCH v3] i440fx/acpi: do not add hotplug related amls for cold plugged bridges
Julia Michael,
Can you please provide your inputs on this one and the corresponding unit
tests?
On Fri, Sep 4, 2020 at 9:40 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
>
> ---
>
> 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 b7bc2a..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
>
>
>
>
[PATCH v3] i440fx/acpi: do not add hotplug related amls for cold plugged bridges
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
---
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 b7bc2a..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
