[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] hw/pci: disable pci-bridge's shpc by default
From: |
Andrew Jones |
Subject: |
Re: [Qemu-devel] [PATCH] hw/pci: disable pci-bridge's shpc by default |
Date: |
Wed, 16 Nov 2016 17:44:42 +0100 |
User-agent: |
Mutt/1.6.0.1 (2016-04-01) |
On Sat, Nov 05, 2016 at 06:46:34PM +0200, Marcel Apfelbaum wrote:
> On 11/03/2016 09:40 PM, Michael S. Tsirkin wrote:
> > On Thu, Nov 03, 2016 at 01:05:44PM +0200, Marcel Apfelbaum wrote:
> > > On 11/03/2016 06:18 AM, Michael S. Tsirkin wrote:
> > > > On Wed, Nov 02, 2016 at 05:16:42PM +0200, Marcel Apfelbaum wrote:
> > > > > The shpc component is optional while ACPI hotplug is used
> > > > > for hot-plugging PCI devices into a PCI-PCI bridge.
> > > > > Disabling the shpc by default will make slot 0 usable at boot time
> > >
> > > Hi Michael
> > >
> > > >
> > > > at the cost of breaking all hotplug for all non-acpi users.
> > > >
> > >
> > > Do we have a non-acpi user that is able to use the shpc component as-is
> > > today?
> >
> > power and some arm systems I guess?
> >
>
> Adding Andrew , maybe he can give us an answer.
Not really :-) My lack of PCI knowledge makes that difficult. I'd be happy
to help with an experiment though. Can you give me command line arguments,
qmp commands, etc. that I should use to try it out? I imagine I should
just boot an ARM guest using DT (instead of ACPI) and then attempt to
hotplug a PCI device. I'm not sure, however, what, if any, special
configuration I need in order to ensure I'm testing what you're
interested in.
Thanks,
drew
>
> Anybody else can help answering this?
>
> > > I remember we need to even tweak QEMU before it can be used, but I might
> > > be wrong.
> > >
> > > And we don't touch the current machines < 2.8 .
> > >
> > > > > and not only for hot-plug, without loosing any functionality.
> > > > > Older machines will have shpc enabled for compatibility reasons.
> > > > >
> > > > > Signed-off-by: Marcel Apfelbaum <address@hidden>
> > > >
> > > > Is an extra slot such a big deal? You can always add more bridges ...
> > > >
> > >
> > > It is not only about the slot itself, but more about the usage model.
> > > The PCIe Upstream ports/DMI-PCI devices are also pci-bridges,
> > > but for them slot 0 is allowed.
> >
> > The reason is that these devices are not themselves
> > hotpluggable. Isn't there a flag that allows adding
> > a non hotpluggable device? Allowing these would be one solution.
> >
> > > And what about the hotplug? Slot 0 is not usable at boot, but then is
> > > usable again (for ACPI users) making people wondering:
> > > https://bugzilla.redhat.com/show_bug.cgi?id=1175113
> >
> > Let's just disallow that then for consistency?
> >
>
> I suppose we can do that... not sure if it worth it.
>
> Thanks,
> Marcel
>
> >
> > > My point is - can shpc be used as-is today? Even so, I suspect there are
> > > much (much)
> > > less users using SHPC than ACPI based hotplug. If this is the case, why
> > > bother the
> > > majority of the users? And for the shpc users, they can keep the prev
> > > machines
> > > or change the command line, I think changes like this happens over the
> > > time.
> > >
> > > Adding Markus for his opinion on command line changes.
> > >
> > > Thanks,
> > > Marcel
> > > > > ---
> > > > > hw/pci-bridge/pci_bridge_dev.c | 2 +-
> > > > > include/hw/compat.h | 4 ++++
> > > > > 2 files changed, 5 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/hw/pci-bridge/pci_bridge_dev.c
> > > > > b/hw/pci-bridge/pci_bridge_dev.c
> > > > > index 5dbd933..647ad80 100644
> > > > > --- a/hw/pci-bridge/pci_bridge_dev.c
> > > > > +++ b/hw/pci-bridge/pci_bridge_dev.c
> > > > > @@ -163,7 +163,7 @@ static Property pci_bridge_dev_properties[] = {
> > > > > DEFINE_PROP_ON_OFF_AUTO(PCI_BRIDGE_DEV_PROP_MSI, PCIBridgeDev,
> > > > > msi,
> > > > > ON_OFF_AUTO_AUTO),
> > > > > DEFINE_PROP_BIT(PCI_BRIDGE_DEV_PROP_SHPC, PCIBridgeDev, flags,
> > > > > - PCI_BRIDGE_DEV_F_SHPC_REQ, true),
> > > > > + PCI_BRIDGE_DEV_F_SHPC_REQ, false),
> > > > > DEFINE_PROP_END_OF_LIST(),
> > > > > };
> > > > >
> > > > > diff --git a/include/hw/compat.h b/include/hw/compat.h
> > > > > index 0f06e11..388b7ec 100644
> > > > > --- a/include/hw/compat.h
> > > > > +++ b/include/hw/compat.h
> > > > > @@ -18,6 +18,10 @@
> > > > > .driver = "intel-iommu",\
> > > > > .property = "x-buggy-eim",\
> > > > > .value = "true",\
> > > > > + },{\
> > > > > + .driver = "pci-bridge",\
> > > > > + .property = "shpc",\
> > > > > + .value = "on",\
> > > > > },
> > > > >
> > > > > #define HW_COMPAT_2_6 \
> > > > > --
> > > > > 2.5.5
>
>
Re: [Qemu-devel] [PATCH] hw/pci: disable pci-bridge's shpc by default, Michael S. Tsirkin, 2016/11/03
- Re: [Qemu-devel] [PATCH] hw/pci: disable pci-bridge's shpc by default, Marcel Apfelbaum, 2016/11/03
- Re: [Qemu-devel] [PATCH] hw/pci: disable pci-bridge's shpc by default, Michael S. Tsirkin, 2016/11/03
- Re: [Qemu-devel] [PATCH] hw/pci: disable pci-bridge's shpc by default, Marcel Apfelbaum, 2016/11/05
- Re: [Qemu-devel] [PATCH] hw/pci: disable pci-bridge's shpc by default,
Andrew Jones <=
- Re: [Qemu-devel] [PATCH] hw/pci: disable pci-bridge's shpc by default, Marcel Apfelbaum, 2016/11/16
- Re: [Qemu-devel] [PATCH] hw/pci: disable pci-bridge's shpc by default, Andrew Jones, 2016/11/18
- Re: [Qemu-devel] [PATCH] hw/pci: disable pci-bridge's shpc by default, Michael S. Tsirkin, 2016/11/18
- Re: [Qemu-devel] [PATCH] hw/pci: disable pci-bridge's shpc by default, Marcel Apfelbaum, 2016/11/22
- Re: [Qemu-devel] [PATCH] hw/pci: disable pci-bridge's shpc by default, Laurent Vivier, 2016/11/22
- Re: [Qemu-devel] [PATCH] hw/pci: disable pci-bridge's shpc by default, Marcel Apfelbaum, 2016/11/23
- Re: [Qemu-devel] [PATCH] hw/pci: disable pci-bridge's shpc by default, David Gibson, 2016/11/24
- Re: [Qemu-devel] [PATCH] hw/pci: disable pci-bridge's shpc by default, Marcel Apfelbaum, 2016/11/24
- Re: [Qemu-devel] [PATCH] hw/pci: disable pci-bridge's shpc by default, David Gibson, 2016/11/24
Re: [Qemu-devel] [PATCH] hw/pci: disable pci-bridge's shpc by default, Michael S. Tsirkin, 2016/11/22