From: "Michael S. Tsirkin" <mst@redhat.com>
To: Isaku Yamahata <yamahata@valinux.co.jp>
Cc: Blue Swirl <blauwirbel@gmail.com>,
Alex Williamson <alex.williamson@redhat.com>,
qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] Re: [PATCH] pci: disable migration of p2p bridge
Date: Wed, 22 Dec 2010 12:54:37 +0200 [thread overview]
Message-ID: <20101222105437.GA10771@redhat.com> (raw)
In-Reply-To: <20101222080045.GA7603@valinux.co.jp>
On Wed, Dec 22, 2010 at 05:00:45PM +0900, Isaku Yamahata wrote:
> On Wed, Dec 22, 2010 at 08:27:17AM +0200, Michael S. Tsirkin wrote:
> > On Wed, Dec 22, 2010 at 12:13:43PM +0900, Isaku Yamahata wrote:
> > > Right now pcibus_get_dev_path() isn't migration save because
> > > bus number/secondary bus number are set by guest OS.
> > > So it can't be used reliably for qemu internal id.
> > >
> > > For 0.14 release, disable p2p bridge migration at the moment.
> > > Once pcibus_get_dev_path() is fixed, this patch should be reverted.
> > > It will be addressed for 0.15 release.
> > >
> > > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > > Cc: Alex Williamson <alex.williamson@redhat.com>
> > > Cc: Blue Swirl <blauwirbel@gmail.com>
> > > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> >
> >
> > Hmm, haven't looked into this deeply - can we do this in one place
> > when the bridge is created?
>
> Unfortunately it's not easy. It requires revising
> register_device_unmigratable(). I have to admit this patch is ugly.
>
> This patch is temporal work around and should be reverted eventually.
> So I think it is better to address the original issue (allowing migration
> of p2p bridge) instead of addressing register_device_unmigratable().
>
> thanks
Hmm. Let's discuss how we will fix pcibus_get_dev_path?
It does not seem to matter how exactly we build it up.
Any way to describe the topology will do I think,
it is slighly ugly to have multiple routines to
build up the path.
For migration, it does not even matter how we describe the domain
as it can't be hot-polugged, we can just use some kind of number.
However, at the moment we don't really support multi-domain
systems, in that there's no way to create such, correct?
So we could use the domain:00:dev.fn:dev.fn:dev.fn syntax as the
natural and backwards compatible way.
What does matter is the commands we give the users for hotplug, these
really are visible. However, it's not clear that we must support a full
hierarchical path for this. Maybe the parent bus qdev id is sufficient
for this?
If so the multiple routines to build up the path issue is
a non-issue and we can easily just fix migration...
Sorry about rambling, to summarise let's just apply the below?
---->
pci: fix migration device path for devices behind nested bridges
We were using bus number in the device path, which is clearly
broken as this number is guest-assigned for all devices
except the root.
Fix by using hierarchical list of slot/function numbers, walking
the path from root down to device, instead. Add :00 as bus number
so that if there are no nested bridges, this is compatible
with what we have now.
Note: as pointed out by Gleb, using openfirmware paths
might be cleaner, doing this would break compatibility though.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/pci.c | 44 ++++++++++++++++++++++++++++++++++++--------
1 files changed, 36 insertions(+), 8 deletions(-)
diff --git a/hw/pci.c b/hw/pci.c
index 8f6fcf8..aed2d42 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1826,15 +1826,41 @@ static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent)
static char *pcibus_get_dev_path(DeviceState *dev)
{
- PCIDevice *d = (PCIDevice *)dev;
- char path[16];
-
- snprintf(path, sizeof(path), "%04x:%02x:%02x.%x",
- pci_find_domain(d->bus),
- 0 /* TODO: need a persistent path for nested buses.
- * Note: pci_bus_num(d->bus) is not right as it's guest
- * assigned. */,
- PCI_SLOT(d->devfn), PCI_FUNC(d->devfn));
-
- return strdup(path);
+ PCIDevice *d = container_of(dev, PCIDevice, qdev);
+ PCIDevice *t;
+ int slot_depth;
+ /* Path format: Domain:00:Slot.Function:Slot.Function....:Slot.Function.
+ * 00 is added here to make this format compatible with
+ * domain:Bus:Slot.Func for systems without nested PCI bridges.
+ * Slot.Function list specifies the slot and function numbers for all
+ * devices on the path from root to the specific device. */
+ int domain_len = strlen("DDDD:00");
+ int slot_len = strlen(":SS.F");
+ int path_len;
+ char *path, *p;
+
+ /* Calculate # of slots on path between device and root. */;
+ slot_depth = 0;
+ for (t = d; t; t = t->bus->parent_dev)
+ ++slot_depth;
+
+ path_len = domain_len + slot_len * slot_depth;
+
+ /* Allocate memory, fill in the terminating null byte. */
+ path = malloc(path_len + 1 /* For '\0' */);
+ path[path_len] = '\0';
+
+ /* First field is the domain. */
+ snprintf(path, domain_len, "%04x:00", pci_find_domain(d->bus));
+
+ /* Fill in slot numbers. We walk up from device to root, so need to print
+ * them in the reverse order, last to first. */
+ p = path + path_len;
+ for (t = d; t; t = t->bus->parent_dev) {
+ p -= slot_len;
+ snprintf(p, slot_len, ":%02x.%x", PCI_SLOT(t->devfn), PCI_FUNC(d->devfn));
+ }
+
+ return path;
}
--
1.7.3.2.91.g446ac
prev parent reply other threads:[~2010-12-22 10:55 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-12-22 3:13 [Qemu-devel] [PATCH] pci: disable migration of p2p bridge Isaku Yamahata
2010-12-22 6:27 ` [Qemu-devel] " Michael S. Tsirkin
2010-12-22 8:00 ` Isaku Yamahata
2010-12-22 10:54 ` Michael S. Tsirkin [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20101222105437.GA10771@redhat.com \
--to=mst@redhat.com \
--cc=alex.williamson@redhat.com \
--cc=blauwirbel@gmail.com \
--cc=qemu-devel@nongnu.org \
--cc=yamahata@valinux.co.jp \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).