Re: [Qemu-ppc] [PATCH qemu 1/2] spapr_pci_vfio: Remove redundant spapr-pci-vfio-host-bridge

[David Gibson]

On Thu, Sep 03, 2015 at 01:19:01PM +1000, Alexey Kardashevskiy wrote:
> On 09/03/2015 12:05 PM, David Gibson wrote:
> >On Wed, Sep 02, 2015 at 06:16:02PM +1000, Alexey Kardashevskiy wrote:
> >>sPAPRTCETable is handling 2 TCE tables already:
> >>
> >>1) guest view of the TCE table - emulated devices use only this table;
> >>
> >>2) hardware IOMMU table - VFIO PCI devices use it for actual work but
> >>it does not replace 1) and it is not visible to the guest.
> >>The initialization of this table is driven by vfio-pci device,
> >>DMA map/unmap requests are handled via MemoryListener so there is very
> >>little to do in spapr-pci-vfio-host-bridge.
> >>
> >>This moves VFIO bits to the generic spapr-pci-host-bridge which allows
> >>putting emulated and VFIO devices on the same PHB. It is still possible
> >>to create multiple PHBs and avoid sharing PHB resouces for emulated and
> >>VFIO devices.
> >>
> >>If there is no VFIO-PCI device attaches, no special ioctls will be called.
> >>If there are some VFIO-PCI devices attached, PHB may refuse to attach
> >>another VFIO-PCI device if a VFIO container on the host kernel side
> >>does not support container sharing.
> >>
> >>This makes spapr-pci-vfio-host-bridge type equal to spapr-pci-host-bridge
> >>except it has an additional "iommu" property so spapr-pci-vfio-host-bridge
> >>still should be used for VFIO devices. The next patch will remove IOMMU ID
> >>property and allow putting VFIO-PCI devices onto spapr-pci-host-bridge.
> >>
> >>This adds a number of VFIO-PCI devices currently attached to a PHB as
> >>PHB needs to know whether to do DMA setup for VFIO or not. Since
> >>at the moment of the PHB's realize() invocation we cannot tell yet
> >>how many VFIO-PCI devices are there (they are not attached yet),
> >>this moves DMA setup to the reset handler.
> >>
> >>This moves PCI device lookup from spapr_phb_vfio_eeh_set_option() to
> >>rtas_ibm_set_eeh_option() as we need to know if the device is "vfio-pci"
> >>and decide whether to call spapr_phb_vfio_eeh_set_option() or not.
> >>
> >>This should cause no behavioural change.
> >>
> >>Signed-off-by: Alexey Kardashevskiy <address@hidden>
> >
> >[snip]
> >>  static int spapr_phb_children_reset(Object *child, void *opaque)
> >>  {
> >>      DeviceState *dev = (DeviceState *) object_dynamic_cast(child, 
> >> TYPE_DEVICE);
> >>@@ -1413,8 +1401,42 @@ static int spapr_phb_children_reset(Object *child, 
> >>void *opaque)
> >>
> >>  static void spapr_phb_reset(DeviceState *qdev)
> >>  {
> >>+    sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(qdev);
> >>+    sPAPRTCETable *tcet;
> >>+
> >>      /* Reset the IOMMU state */
> >>      object_child_foreach(OBJECT(qdev), spapr_phb_children_reset, NULL);
> >>+
> >>+    if (spapr_phb_dma_capabilities_update(sphb)) {
> >>+        return;
> >>+    }
> >>+
> >>+    /* Register default 32bit DMA window */
> >>+    tcet = spapr_tce_find_by_liobn(sphb->dma_liobn);
> >>+    if (!tcet) {
> >>+        const unsigned nb = sphb->dma32_window_size >> 
> >>SPAPR_TCE_PAGE_SHIFT;
> >>+        tcet = spapr_tce_new_table(DEVICE(sphb), sphb->dma_liobn,
> >>+                                   sphb->dma32_window_start,
> >>+                                   SPAPR_TCE_PAGE_SHIFT, nb,
> >>+                                   sphb->vfio_num > 0);
> >
> >Could delaying the construction of the TCE table object until reset
> >time cause problems with migration?  i.e. can you be sure that the
> >destination will have the TCE table object present and in a suitable
> >state to accept the incoming table information from the source?
> 
> 
> This is a valid concern but the PHB reset handler is called (just checked)
> when QEMU is started with "-incoming tcp:vpl2:33333" so yes, I am sure :)

Ok.  The rest of these patches look fine, so if you fix the bus offset
problem, I'll merge into spapr-next.

-- 
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

pgprYTMwmjWzT.pgp
Description: PGP signature