Re: [Xen-devel] [PATCH v4 3/9] x86/physdev: enable PHYSDEVOP_pci_mmcfg_reserved for PVH Dom0

["Jan Beulich" <jbeulich@xxxxxxxx>]

>>> Roger Pau Monne <roger.pau@xxxxxxxxxx> 07/20/17 12:24 PM >>>
>On Fri, Jul 14, 2017 at 04:32:19AM -0600, Jan Beulich wrote:
>> >>> On 30.06.17 at 17:01, <roger.pau@xxxxxxxxxx> wrote:
>> > So that hotplug (or MMCFG regions not present in the MCFG ACPI table)
>> > can be added at run time by the hardware domain.
>> 
>> I think the emphasis should be the other way around. I'm rather certain
>> hotplug of bridges doesn't really work right now anyway; at least
>> IO-APIC hotplug code is completely missing.
>
>IO-APICs can also be hot-plugged? Didn't even know about that...

Think of hot adding an entire node.

>> > When a new MMCFG area is added to a PVH Dom0, Xen will scan it and add
>> > the devices to the hardware domain.
>> 
>> Adding the MMIO regions is certainly necessary, but what's the point of
>> also scanning the bus and adding the devices?
>
>It's not strictly necessary, the same can be accomplished by Dom0
>calling PHYSDEVOP_manage_pci_add on each device.
>
>Just thought it wouldn't hurt to do it here, but given your comment
>below I'm not sure. I will wait for your reply before deciding what to
>do.
>
>> We expect Dom0 to tell us
>> anyway, and not doing the scan in Xen avoids complications we presently
>> have in the segment 0 case when Dom0 decides to re-number busses (e.g.
>> in order to fit in SR-IOV VFs).
>
>Is this renumbering performed by changing the
>Primary/Secondary/Subordinate bus number registers in the bridge?

Yes.

>If so we could detect such accesses (by adding traps to type 01h
>headers) and react accordingly.

Yes.

>What if Dom0 re-numbers the bus after having already registered the
>devices with Xen?

The expectation would be for Dom0 to first unregister all devices in the
sub-topology, to the re-numbering, and then re-add them. That doesn't
happen in Linux though, afair.

>> > @@ -1110,6 +1110,37 @@ void __hwdom_init setup_hwdom_pci_devices(
>> >      pcidevs_unlock();
>> >  }
>> >  
>> > +static int add_device(uint8_t devfn, struct pci_dev *pdev)
>> > +{
>> > +    return iommu_add_device(pdev);
>> > +}
>> 
>> You're discarding devfn here, just for iommu_add_device() to re-do the
>> phantom function handling. At the very least this is wasteful. Perhaps
>> you minimally want to call iommu_add_device() only when
>> devfn == pdev->devfn (if all of this code stays in the first place)?
>
>Doesn't the IOMMU also need to know about the phantom functions in
>order to add translations for them too?

Yes, that's why iommu_add_device() and others have a respective loop.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel