[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 15/16] spapr_pci: enable basic hotplug operat
From: |
David Gibson |
Subject: |
Re: [Qemu-devel] [PATCH v5 15/16] spapr_pci: enable basic hotplug operations |
Date: |
Fri, 27 Feb 2015 16:14:57 +1100 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Thu, Feb 26, 2015 at 02:06:04PM -0600, Michael Roth wrote:
> Quoting Michael Roth (2015-02-25 00:40:26)
> > Quoting Michael Roth (2015-02-24 23:17:24)
> > > Quoting David Gibson (2015-02-24 21:11:29)
> > > > On Mon, Feb 16, 2015 at 08:27:51AM -0600, Michael Roth wrote:
[snip]
> > > > > + }
> > > > > +
> > > > > + assigned = &rp->assigned.fields[assigned_idx++];
> > > > > + assigned->phys_hi = cpu_to_be32(reg->phys_hi | b_n(1));
> > > > > + assigned->phys_mid = cpu_to_be32(d->io_regions[i].addr >>
> > > > > 32);
> > > > > + assigned->phys_lo = cpu_to_be32(d->io_regions[i].addr);
> > > >
> > > > Not sure if I understood the comment above properly; should these
> > > > addresses actually be translated through an mappings above the PHB?
> > > > Not that there are any such translations on sPAPR, but...
> > >
> > > The io_regions[i].addr values are relative to the IO/MEM address spaces
> > > for
> > > the device, which correspond to IO/MEM windows for the PHB. So I think the
> > > memory API should handle any translation above that...
> > >
> > > Or do you mean because of the n=0/relocatable IO regions described by
> > > 'reg'?
> > >
> > > IIUC, when n=0, reg->phys_{mid,lo} can be used to encode a starting
> > > offset,
> > > so a guest can re-assign/re-program an IO region that's already been
> > > assigned
> > > and described via the correspond fields in 'assigned-addresses', so long
> > > as
> > > uses an addr above reg->phys{mid,lo}.
> > >
> > > So, we could use those reg->phys_{mid,lo} values as an alternative to the
> > > PHBs IO/MEM windows (I guess for platforms that don't provide these
> > > windows
> > > and just expose the full address space?).
> > >
> > > But since we have those windows, we end up not needing this, so we always
> > > do
> > > n=0, reg->phys_mid = reg->phys_lo = 0, so the values in
> > > assigned->phys_{mid,lo} end up just being offsets into the IO/MEM windows,
> > > which correspond directly to io_regions[n].addr.
> >
> > Argh, not sure what I was thinking. io_regions[n].addr is relative to the
> > IO/MEM window, so from the guest perspective .addr is indeed a different
> > value...
> >
> > So maybe reg->phys_mid/reg->phys_lo do in fact need to reflect the window
> > offsets so that the relocatable region assignments are offset
> > accordingly...
> >
> > Will look into it more.
>
> So, maybe my first response wasn't as misguided as I thought. I haven't
> found any documentation that lays out the specifics, but the kernel code makes
> it fairly clear that the IO/MEM addresses in assigned-addresses are relative
> to PHB's IO/MEM windows.
>
> The code basically does this:
>
> arch/powerpc/kernel/pci_of_scan.c: of_pci_parse_addrs()
> addrs = of_get('assigned-addresses')
> config_offset = addrs[0] && 0xff (phys.hi)
> pci_bar_num = (config_offset - PCI_BASE_ADDRESS_0) / 4
> base = &addrs[1:2] (phys.mid/phys.lo)
> size = &addrs[3:4] (size.hi, size.lo)
> region.start = base
> region.end = base + size -1
> res = &dev->resource[pci_bar_num];
> pcibios_bus_to_resource(dev->bus, res, ®ion)
> drivers/pci/host-bridge.c:pcibios_bus_to_resource(bus, bar_res, bar_region)
> bridge = pci_host_bridge(bus)
> for window in bridge->windows:
> # skip if window type (IO/MEM), doesn't match device BAR's res)
> bus_region.start = window->res->start - window->offset
> bus_region.end = window->res->end - window->offset
> if bus_region contains device BAR region:
> offset = window->offset
> break
> //take the region, add the window offset, and use that as the resource
> addr
> res->start = region->start + offset
> res->end = region->end + offset
>
> I tested this with the rpaphp hotplug module, which honors
> assigned-addresses (current guest hotplug uses generic pci rescan instead
> due to a long-standing issue with rpaphp only being able to handle
> 1 slot per PHB if there are devices present at module-load time), by adding
> some test code to pre-assign the BARs during hotplug (with addresses relative
> to PHB's IO/MEM windows) to make sure assigned-addresses values get properly
> translated to the correct offset in guest's io/system address space.
>
> Will add a note in the comments.
Ok, sounds like the only problem was a confusing description in the
comment that made it seem like the assigned-address values were GPAs
rather than relative to the PHB windows.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
pgpdR7WZTXGuN.pgp
Description: PGP signature
- Re: [Qemu-devel] [PATCH v5 10/16] spapr_drc: add spapr_drc_populate_dt(), (continued)
[Qemu-devel] [PATCH v5 01/16] docs: add sPAPR hotplug/dynamic-reconfiguration documentation, Michael Roth, 2015/02/16
[Qemu-devel] [PATCH v5 16/16] spapr_pci: emit hotplug add/remove events during hotplug, Michael Roth, 2015/02/16
[Qemu-devel] [PATCH v5 13/16] spapr_pci: populate DRC dt entries for PHBs, Michael Roth, 2015/02/16
[Qemu-devel] [PATCH v5 11/16] spapr_pci: add dynamic-reconfiguration option for spapr-pci-host-bridge, Michael Roth, 2015/02/16
[Qemu-devel] [PATCH v5 12/16] spapr_pci: create DRConnectors for each PCI slot during PHB realize, Michael Roth, 2015/02/16
[Qemu-devel] [PATCH v5 04/16] spapr_rtas: add set-indicator RTAS interface, Michael Roth, 2015/02/16