| Message ID | 1418054888-11310-3-git-send-email-imammedo@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Dec 08, 2014 at 04:08:01PM +0000, Igor Mammedov wrote: > when bridge hotplug is disabled, i.e. for machine > types less then 2.0, bridge device was created as > hotpluggable by mistake since commit 133a2da. > > Fix it by just creating it as a present device. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> What exactly is the problem here? It seems that such bridge is hotpluggable, even though e.g. windows guests lacks drivers to support this. > --- > hw/i386/acpi-build.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index b37a397..1fb92e5 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -913,7 +913,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) > } > } > > - if (!dc->hotpluggable || bridge_in_acpi) { > + if (!dc->hotpluggable || pc->is_bridge) { > clear_bit(slot, slot_hotplug_enable); > } > } > -- > 1.8.3.1
On Mon, 8 Dec 2014 21:13:25 +0200 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Mon, Dec 08, 2014 at 04:08:01PM +0000, Igor Mammedov wrote: > > when bridge hotplug is disabled, i.e. for machine > > types less then 2.0, bridge device was created as > > hotpluggable by mistake since commit 133a2da. > > > > Fix it by just creating it as a present device. > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > What exactly is the problem here? > It seems that such bridge is hotpluggable, even though > e.g. windows guests lacks drivers to support this. before 133a2da slot with existing at startup bridge weren't marked as hotpluggable nor described in SSDT. But after 133a2da it's described as hotpluggable slot for compat machines (2.0 and lower) which doesn't match with original behavior. Also Marcel mentioned that bridges could be hotpluggable but that they should not be hot-UNpluggable. > > > > --- > > hw/i386/acpi-build.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > index b37a397..1fb92e5 100644 > > --- a/hw/i386/acpi-build.c > > +++ b/hw/i386/acpi-build.c > > @@ -913,7 +913,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) > > } > > } > > > > - if (!dc->hotpluggable || bridge_in_acpi) { > > + if (!dc->hotpluggable || pc->is_bridge) { > > clear_bit(slot, slot_hotplug_enable); > > } > > } > > -- > > 1.8.3.1
On Tue, Dec 09, 2014 at 11:27:16AM +0100, Igor Mammedov wrote: > On Mon, 8 Dec 2014 21:13:25 +0200 > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > On Mon, Dec 08, 2014 at 04:08:01PM +0000, Igor Mammedov wrote: > > > when bridge hotplug is disabled, i.e. for machine > > > types less then 2.0, bridge device was created as > > > hotpluggable by mistake since commit 133a2da. > > > > > > Fix it by just creating it as a present device. > > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > > What exactly is the problem here? > > It seems that such bridge is hotpluggable, even though > > e.g. windows guests lacks drivers to support this. > before 133a2da slot with existing at startup bridge weren't > marked as hotpluggable nor described in SSDT. But after > 133a2da it's described as hotpluggable slot for compat > machines (2.0 and lower) which doesn't match with original > behavior. > > Also Marcel mentioned that bridges could be hotpluggable > but that they should not be hot-UNpluggable. OK so is there some guest that's confused? What's the bug? > > > > > > > --- > > > hw/i386/acpi-build.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > > index b37a397..1fb92e5 100644 > > > --- a/hw/i386/acpi-build.c > > > +++ b/hw/i386/acpi-build.c > > > @@ -913,7 +913,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) > > > } > > > } > > > > > > - if (!dc->hotpluggable || bridge_in_acpi) { > > > + if (!dc->hotpluggable || pc->is_bridge) { > > > clear_bit(slot, slot_hotplug_enable); > > > } > > > } > > > -- > > > 1.8.3.1
On Tue, 9 Dec 2014 12:34:02 +0200 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Tue, Dec 09, 2014 at 11:27:16AM +0100, Igor Mammedov wrote: > > On Mon, 8 Dec 2014 21:13:25 +0200 > > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > On Mon, Dec 08, 2014 at 04:08:01PM +0000, Igor Mammedov wrote: > > > > when bridge hotplug is disabled, i.e. for machine > > > > types less then 2.0, bridge device was created as > > > > hotpluggable by mistake since commit 133a2da. > > > > > > > > Fix it by just creating it as a present device. > > > > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > > > > What exactly is the problem here? > > > It seems that such bridge is hotpluggable, even though > > > e.g. windows guests lacks drivers to support this. > > before 133a2da slot with existing at startup bridge weren't > > marked as hotpluggable nor described in SSDT. But after > > 133a2da it's described as hotpluggable slot for compat > > machines (2.0 and lower) which doesn't match with original > > behavior. > > > > Also Marcel mentioned that bridges could be hotpluggable > > but that they should not be hot-UNpluggable. > > OK so is there some guest that's confused? > What's the bug? It allows to do eject on bridge when it shouldn't. I don't know about consequences of it, it's better not to allow invalid action in the first place. > > > > > > > > > > > --- > > > > hw/i386/acpi-build.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > > > index b37a397..1fb92e5 100644 > > > > --- a/hw/i386/acpi-build.c > > > > +++ b/hw/i386/acpi-build.c > > > > @@ -913,7 +913,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) > > > > } > > > > } > > > > > > > > - if (!dc->hotpluggable || bridge_in_acpi) { > > > > + if (!dc->hotpluggable || pc->is_bridge) { > > > > clear_bit(slot, slot_hotplug_enable); > > > > } > > > > } > > > > -- > > > > 1.8.3.1
On Tue, Dec 09, 2014 at 12:45:27PM +0100, Igor Mammedov wrote: > On Tue, 9 Dec 2014 12:34:02 +0200 > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > On Tue, Dec 09, 2014 at 11:27:16AM +0100, Igor Mammedov wrote: > > > On Mon, 8 Dec 2014 21:13:25 +0200 > > > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > > > On Mon, Dec 08, 2014 at 04:08:01PM +0000, Igor Mammedov wrote: > > > > > when bridge hotplug is disabled, i.e. for machine > > > > > types less then 2.0, bridge device was created as > > > > > hotpluggable by mistake since commit 133a2da. > > > > > > > > > > Fix it by just creating it as a present device. > > > > > > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > > > > > > What exactly is the problem here? > > > > It seems that such bridge is hotpluggable, even though > > > > e.g. windows guests lacks drivers to support this. > > > before 133a2da slot with existing at startup bridge weren't > > > marked as hotpluggable nor described in SSDT. But after > > > 133a2da it's described as hotpluggable slot for compat > > > machines (2.0 and lower) which doesn't match with original > > > behavior. > > > > > > Also Marcel mentioned that bridges could be hotpluggable > > > but that they should not be hot-UNpluggable. > > > > OK so is there some guest that's confused? > > What's the bug? > It allows to do eject on bridge when it shouldn't. I'm not sure it shouldn't. Assuming guest supports SHPC, eject on such a bridge might well work correctly. > I don't know about consequences of it, > it's better not to allow invalid action in the first place. > > > > > > > > > > > > > > > > > --- > > > > > hw/i386/acpi-build.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > > > > index b37a397..1fb92e5 100644 > > > > > --- a/hw/i386/acpi-build.c > > > > > +++ b/hw/i386/acpi-build.c > > > > > @@ -913,7 +913,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) > > > > > } > > > > > } > > > > > > > > > > - if (!dc->hotpluggable || bridge_in_acpi) { > > > > > + if (!dc->hotpluggable || pc->is_bridge) { > > > > > clear_bit(slot, slot_hotplug_enable); > > > > > } > > > > > } > > > > > -- > > > > > 1.8.3.1
On Tue, 9 Dec 2014 14:51:59 +0200 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Tue, Dec 09, 2014 at 12:45:27PM +0100, Igor Mammedov wrote: > > On Tue, 9 Dec 2014 12:34:02 +0200 > > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > On Tue, Dec 09, 2014 at 11:27:16AM +0100, Igor Mammedov wrote: > > > > On Mon, 8 Dec 2014 21:13:25 +0200 > > > > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > > > > > On Mon, Dec 08, 2014 at 04:08:01PM +0000, Igor Mammedov wrote: > > > > > > when bridge hotplug is disabled, i.e. for machine > > > > > > types less then 2.0, bridge device was created as > > > > > > hotpluggable by mistake since commit 133a2da. > > > > > > > > > > > > Fix it by just creating it as a present device. > > > > > > > > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > > > > > > > > What exactly is the problem here? > > > > > It seems that such bridge is hotpluggable, even though > > > > > e.g. windows guests lacks drivers to support this. > > > > before 133a2da slot with existing at startup bridge weren't > > > > marked as hotpluggable nor described in SSDT. But after > > > > 133a2da it's described as hotpluggable slot for compat > > > > machines (2.0 and lower) which doesn't match with original > > > > behavior. > > > > > > > > Also Marcel mentioned that bridges could be hotpluggable > > > > but that they should not be hot-UNpluggable. > > > > > > OK so is there some guest that's confused? > > > What's the bug? > > It allows to do eject on bridge when it shouldn't. > > I'm not sure it shouldn't. Assuming guest supports SHPC, eject on such a > bridge might well work correctly. If I remember correctly SHPC hotplug doesn't need ASL parts at all. > > > > I don't know about consequences of it, > > it's better not to allow invalid action in the first place. > > > > > > > > > > > > > > > > > > > > > > > --- > > > > > > hw/i386/acpi-build.c | 2 +- > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > > > > > index b37a397..1fb92e5 100644 > > > > > > --- a/hw/i386/acpi-build.c > > > > > > +++ b/hw/i386/acpi-build.c > > > > > > @@ -913,7 +913,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) > > > > > > } > > > > > > } > > > > > > > > > > > > - if (!dc->hotpluggable || bridge_in_acpi) { > > > > > > + if (!dc->hotpluggable || pc->is_bridge) { > > > > > > clear_bit(slot, slot_hotplug_enable); > > > > > > } > > > > > > } > > > > > > -- > > > > > > 1.8.3.1
On Tue, 9 Dec 2014 14:51:59 +0200 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Tue, Dec 09, 2014 at 12:45:27PM +0100, Igor Mammedov wrote: > > On Tue, 9 Dec 2014 12:34:02 +0200 > > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > On Tue, Dec 09, 2014 at 11:27:16AM +0100, Igor Mammedov wrote: > > > > On Mon, 8 Dec 2014 21:13:25 +0200 > > > > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > > > > > On Mon, Dec 08, 2014 at 04:08:01PM +0000, Igor Mammedov wrote: > > > > > > when bridge hotplug is disabled, i.e. for machine > > > > > > types less then 2.0, bridge device was created as > > > > > > hotpluggable by mistake since commit 133a2da. > > > > > > > > > > > > Fix it by just creating it as a present device. > > > > > > > > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > > > > > > > > What exactly is the problem here? > > > > > It seems that such bridge is hotpluggable, even though > > > > > e.g. windows guests lacks drivers to support this. > > > > before 133a2da slot with existing at startup bridge weren't > > > > marked as hotpluggable nor described in SSDT. But after > > > > 133a2da it's described as hotpluggable slot for compat > > > > machines (2.0 and lower) which doesn't match with original > > > > behavior. > > > > > > > > Also Marcel mentioned that bridges could be hotpluggable > > > > but that they should not be hot-UNpluggable. > > > > > > OK so is there some guest that's confused? > > > What's the bug? > > It allows to do eject on bridge when it shouldn't. > > I'm not sure it shouldn't. Assuming guest supports SHPC, eject on such a > bridge might well work correctly. Question here is should we keep compat mode as it was for old machine types (i.e. not hot-removable), for new machines bridge is not hot-removable now or just ignore silent change made by 133a2da? > > > > I don't know about consequences of it, > > it's better not to allow invalid action in the first place. > > > > > > > > > > > > > > > > > > > > > > > --- > > > > > > hw/i386/acpi-build.c | 2 +- > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > > > > > index b37a397..1fb92e5 100644 > > > > > > --- a/hw/i386/acpi-build.c > > > > > > +++ b/hw/i386/acpi-build.c > > > > > > @@ -913,7 +913,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) > > > > > > } > > > > > > } > > > > > > > > > > > > - if (!dc->hotpluggable || bridge_in_acpi) { > > > > > > + if (!dc->hotpluggable || pc->is_bridge) { > > > > > > clear_bit(slot, slot_hotplug_enable); > > > > > > } > > > > > > } > > > > > > -- > > > > > > 1.8.3.1
On Tue, Dec 09, 2014 at 01:57:51PM +0100, Igor Mammedov wrote: > On Tue, 9 Dec 2014 14:51:59 +0200 > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > On Tue, Dec 09, 2014 at 12:45:27PM +0100, Igor Mammedov wrote: > > > On Tue, 9 Dec 2014 12:34:02 +0200 > > > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > > > On Tue, Dec 09, 2014 at 11:27:16AM +0100, Igor Mammedov wrote: > > > > > On Mon, 8 Dec 2014 21:13:25 +0200 > > > > > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > > > > > > > On Mon, Dec 08, 2014 at 04:08:01PM +0000, Igor Mammedov wrote: > > > > > > > when bridge hotplug is disabled, i.e. for machine > > > > > > > types less then 2.0, bridge device was created as > > > > > > > hotpluggable by mistake since commit 133a2da. > > > > > > > > > > > > > > Fix it by just creating it as a present device. > > > > > > > > > > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > > > > > > > > > > What exactly is the problem here? > > > > > > It seems that such bridge is hotpluggable, even though > > > > > > e.g. windows guests lacks drivers to support this. > > > > > before 133a2da slot with existing at startup bridge weren't > > > > > marked as hotpluggable nor described in SSDT. But after > > > > > 133a2da it's described as hotpluggable slot for compat > > > > > machines (2.0 and lower) which doesn't match with original > > > > > behavior. > > > > > > > > > > Also Marcel mentioned that bridges could be hotpluggable > > > > > but that they should not be hot-UNpluggable. > > > > > > > > OK so is there some guest that's confused? > > > > What's the bug? > > > It allows to do eject on bridge when it shouldn't. > > > > I'm not sure it shouldn't. Assuming guest supports SHPC, eject on such a > > bridge might well work correctly. > If I remember correctly SHPC hotplug doesn't need ASL parts at all. Right. And then presumably bridge itself can be removed. > > > > > > > I don't know about consequences of it, > > > it's better not to allow invalid action in the first place. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > --- > > > > > > > hw/i386/acpi-build.c | 2 +- > > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > > > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > > > > > > index b37a397..1fb92e5 100644 > > > > > > > --- a/hw/i386/acpi-build.c > > > > > > > +++ b/hw/i386/acpi-build.c > > > > > > > @@ -913,7 +913,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) > > > > > > > } > > > > > > > } > > > > > > > > > > > > > > - if (!dc->hotpluggable || bridge_in_acpi) { > > > > > > > + if (!dc->hotpluggable || pc->is_bridge) { > > > > > > > clear_bit(slot, slot_hotplug_enable); > > > > > > > } > > > > > > > } > > > > > > > -- > > > > > > > 1.8.3.1
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index b37a397..1fb92e5 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -913,7 +913,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) } } - if (!dc->hotpluggable || bridge_in_acpi) { + if (!dc->hotpluggable || pc->is_bridge) { clear_bit(slot, slot_hotplug_enable); } }
when bridge hotplug is disabled, i.e. for machine types less then 2.0, bridge device was created as hotpluggable by mistake since commit 133a2da. Fix it by just creating it as a present device. Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- hw/i386/acpi-build.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)