From: Yinghai Lu on
Kenji Kaneshige wrote:
> Yinghai Lu wrote:
>> Kenji Kaneshige wrote:
>>> Yinghai Lu wrote:
>>>> Kenji Kaneshige wrote:
>>>>> Yinghai Lu wrote:
>>>>>> move out bus_size_bridges and assign resources out of
>>>>>> pciehp_add_bridge()
>>>>>> and at last do them all together one time including slot bridge, to
>>>>>> avoid to call
>>>>>> assign resources several times, when there are several bridges under
>>>>>> the slot bridge.
>>>>>>
>>>>>> need to introduce pci_bridge_assign_resources there.
>>>>>>
>>>>>> handle the case the first bridge that doesn't get pre-allocated big
>>>>>> enough res from FW.
>>>>>> for example pcie devices need 256M, but the bridge only get
>>>>>> preallocated 2M...
>>>>>>
>>>>> Though I have not looked at the patch deeply yet (sorry), I have
>>>>> some questions and concerns about this change. Please correct me
>>>>> if my understanding is not correct.
>>>>>
>>>>> - Your patch doesn't seems to have the code to free resources.
>>>>> If we need to expand the resource range, don't we need to free
>>>>> preallocated resource before allocating the new one?
>>>> that is done with pci_bus_size_bridges ==> pbus_size_io/pbus_size_mem
>>>> ==> find_free_bus_resource ==> release_resource.
>>>>
>>> I didn't noticed that find_free_bus_resource() was changed to call
>>> release_resource() recently...
>>>
>>> By the way, does this (release_resource() by find_bus_resource())
>>> change the resource assignment by BIOS also for bridges other than
>>> the ports with hotplug slot (switch upstreamport, for example)?
>>
>> yes.
>>
>> BIOS preallocate small range for the bridge, and leave the BAR for the
>> device under that bridge uninitialized.
>>
>
> Does this happen at the boot time regardless of hot-plug?

yes

>
>
>>>>> - Your patch seems to update BARs for bridge itself. I think it
>>>>> would break the bridge's driver (port service driver) that if
>>>>> it controls the device's capability by using IO/Mem, though I
>>>>> don't know if such driver or capabilities exists now.
>>>> port service driver will be AER and pciehotplug.
>>>> it seems those driver are not use those BAR...
>>>> those BAR are supposed for the devices under the pcie bridge.
>>>>
>>> I understand that there are only two port service drivers (AER and
>>> PCIe hotplug) and both doesn't use BAR. But I still have a concern
>>> that changing bridge's BARs might cause problems in the future. In
>>> my understanding, what you need is expanding IO/Mem base and limit
>>> of root or switch downstream ports. If so, I think we should only
>>> touch IO/Mem base/limit, and should not touch bridge's BARs.
>>
>> those bridge BAR are for devices under that bridge. the port device is
>> not supposed to use them.
>>
>
> Do you mean you touch only BARs of the devices under the bridge?

no. the BAR of 0x1c, 0x20, and 0x28

>
>> if we don't touch the bridge's BAR, the hw will not forward the io for
>> those devices under it.
>>
>
> I understand you need to touch I/O base/limit and Mem base/limit. But
> I don't understand why you also need to update bridge's BARs. Could
> you please explain a little more about it?
>
> Just in case, my terminology "bridge's BARs" is Base Address Register
> 0 (offset 0x10) and Base Address Register 1 (offset 0x14) in the
> (type 1) configuration space header of the bridge.

i mean 0x1c, 0x20, 0x28

did not notice that bridge device's 0x10, 0x14 are used...
if port service need to use 0x10, 0x14, and the device is enabled, we should touch 0x10, and 0x14.


YH
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Yinghai Lu on
Yinghai Lu wrote:
> Kenji Kaneshige wrote:
>> I understand you need to touch I/O base/limit and Mem base/limit. But
>> I don't understand why you also need to update bridge's BARs. Could
>> you please explain a little more about it?
>>
>> Just in case, my terminology "bridge's BARs" is Base Address Register
>> 0 (offset 0x10) and Base Address Register 1 (offset 0x14) in the
>> (type 1) configuration space header of the bridge.
>
> i mean 0x1c, 0x20, 0x28
>
> did not notice that bridge device's 0x10, 0x14 are used...
> if port service need to use 0x10, 0x14, and the device is enabled, we should touch 0x10, and 0x14.

after check the code, if

pci_bridge_assign_resources ==> pdev_assign_resources_sorted ==> pdev_sort_resources

will not touch 0x10 and 0x14, if those resource is claimed by port service.

/* Sort resources by alignment */
void pdev_sort_resources(struct pci_dev *dev, struct resource_list *head)
{
int i;

for (i = 0; i < PCI_NUM_RESOURCES; i++) {
struct resource *r;
struct resource_list *list, *tmp;
resource_size_t r_align;

r = &dev->resource[i];

if (r->flags & IORESOURCE_PCI_FIXED)
continue;

if (!(r->flags) || r->parent)
continue;


r->parent != NULL, will make it skip those two.

So -v3 should be safe.

Thanks

Yinghai Lu


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Kenji Kaneshige on
Yinghai Lu wrote:
> Yinghai Lu wrote:
>> Kenji Kaneshige wrote:
>>> I understand you need to touch I/O base/limit and Mem base/limit. But
>>> I don't understand why you also need to update bridge's BARs. Could
>>> you please explain a little more about it?
>>>
>>> Just in case, my terminology "bridge's BARs" is Base Address Register
>>> 0 (offset 0x10) and Base Address Register 1 (offset 0x14) in the
>>> (type 1) configuration space header of the bridge.
>> i mean 0x1c, 0x20, 0x28
>>
>> did not notice that bridge device's 0x10, 0x14 are used...
>> if port service need to use 0x10, 0x14, and the device is enabled, we should touch 0x10, and 0x14.
>
> after check the code, if
>
> pci_bridge_assign_resources ==> pdev_assign_resources_sorted ==> pdev_sort_resources
>
> will not touch 0x10 and 0x14, if those resource is claimed by port service.
>
> /* Sort resources by alignment */
> void pdev_sort_resources(struct pci_dev *dev, struct resource_list *head)
> {
> int i;
>
> for (i = 0; i < PCI_NUM_RESOURCES; i++) {
> struct resource *r;
> struct resource_list *list, *tmp;
> resource_size_t r_align;
>
> r = &dev->resource[i];
>
> if (r->flags & IORESOURCE_PCI_FIXED)
> continue;
>
> if (!(r->flags) || r->parent)
> continue;
>
>
> r->parent != NULL, will make it skip those two.
>
> So -v3 should be safe.
>

Thank you for the clarification.

But I still don't understand the whole picture of your set of
changes. Let me ask some questions.

In my understanding of your set of changes, if there is a PCIe
switch with some hot-plug slots and all of those slots are empty,
I/O and Memory resources assigned by BIOS are all released at
the boot time. For example, suppose the following case.

bridge(A)
|
-----------------------
| |
bridge(B) bridge(C)
| |
slot(1) slot(2)
(empty) (empty)

bridge(A): P2P bridge for switch upstream port
bridge(B): P2P bridge for switch downstream port
bridge(C): P2P bridge for switch downstream port

In the above example, I/O and Mem resource assigned to bridge(A),
bridge(B) and bridge(C) are all released at the boot time. Correct?

Then, when a adapter card is hot-added to slot(1), I/O and Mem
resources enough for enabling the hot-added adapter card is assigned
to bridge(A), bridge(B) and the adapter card. Correct?

Then, when an another adpater card is hot-added to slot(2), we
need to assign enough resource to bridge(C) and the new card.
But bridge(A) doesn't have enough resource for bridge(C) and
the new card. In addition, all bridge(A) and bridge(B) and the
adapter card on slot(1) are already working. How do you assign
resource to bridge(C) and the card on slot(2)?

Thanks,
Kenji Kaneshige




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Yinghai Lu on
Kenji Kaneshige wrote:
> Yinghai Lu wrote:
>> Yinghai Lu wrote:
>>> Kenji Kaneshige wrote:
>>>> I understand you need to touch I/O base/limit and Mem base/limit. But
>>>> I don't understand why you also need to update bridge's BARs. Could
>>>> you please explain a little more about it?
>>>>
>>>> Just in case, my terminology "bridge's BARs" is Base Address Register
>>>> 0 (offset 0x10) and Base Address Register 1 (offset 0x14) in the
>>>> (type 1) configuration space header of the bridge.
>>> i mean 0x1c, 0x20, 0x28
>>>
>>> did not notice that bridge device's 0x10, 0x14 are used...
>>> if port service need to use 0x10, 0x14, and the device is enabled, we
>>> should touch 0x10, and 0x14.
>>
>> after check the code, if
>> pci_bridge_assign_resources ==> pdev_assign_resources_sorted ==>
>> pdev_sort_resources
>>
>> will not touch 0x10 and 0x14, if those resource is claimed by port
>> service.
>>
>> /* Sort resources by alignment */
>> void pdev_sort_resources(struct pci_dev *dev, struct resource_list *head)
>> { int i;
>> for (i = 0; i < PCI_NUM_RESOURCES; i++) {
>> struct resource *r;
>> struct resource_list *list, *tmp;
>> resource_size_t r_align;
>> r = &dev->resource[i];
>> if (r->flags &
>> IORESOURCE_PCI_FIXED)
>> continue;
>> if (!(r->flags) || r->parent)
>> continue;
>>
>> r->parent != NULL, will make it skip those two.
>>
>> So -v3 should be safe.
>>
>
> Thank you for the clarification.
>
> But I still don't understand the whole picture of your set of
> changes. Let me ask some questions.
>
> In my understanding of your set of changes, if there is a PCIe
> switch with some hot-plug slots and all of those slots are empty,
> I/O and Memory resources assigned by BIOS are all released at
> the boot time. For example, suppose the following case.
>
> bridge(A)
> |
> -----------------------
> | |
> bridge(B) bridge(C)
> | |
> slot(1) slot(2)
> (empty) (empty)
>
> bridge(A): P2P bridge for switch upstream port
> bridge(B): P2P bridge for switch downstream port
> bridge(C): P2P bridge for switch downstream port
>
> In the above example, I/O and Mem resource assigned to bridge(A),
> bridge(B) and bridge(C) are all released at the boot time. Correct?
>
> Then, when a adapter card is hot-added to slot(1), I/O and Mem
> resources enough for enabling the hot-added adapter card is assigned
> to bridge(A), bridge(B) and the adapter card. Correct?
>
> Then, when an another adpater card is hot-added to slot(2), we
> need to assign enough resource to bridge(C) and the new card.
> But bridge(A) doesn't have enough resource for bridge(C) and
> the new card. In addition, all bridge(A) and bridge(B) and the
> adapter card on slot(1) are already working. How do you assign
> resource to bridge(C) and the card on slot(2)?
>

thanks, will update the patches to only handle leaf bridge, and don't touch min_size etc.

YH
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Yinghai Lu on
Eric W. Biederman wrote:
> Yinghai Lu <yinghai(a)kernel.org> writes:
>
>> Kenji Kaneshige wrote:
>>> Yinghai Lu wrote:
>>>> Yinghai Lu wrote:
>>>>> Kenji Kaneshige wrote:
>>>>>> I understand you need to touch I/O base/limit and Mem base/limit. But
>>>>>> I don't understand why you also need to update bridge's BARs. Could
>>>>>> you please explain a little more about it?
>>>>>>
>>>>>> Just in case, my terminology "bridge's BARs" is Base Address Register
>>>>>> 0 (offset 0x10) and Base Address Register 1 (offset 0x14) in the
>>>>>> (type 1) configuration space header of the bridge.
>>>>> i mean 0x1c, 0x20, 0x28
>>>>>
>>>>> did not notice that bridge device's 0x10, 0x14 are used...
>>>>> if port service need to use 0x10, 0x14, and the device is enabled, we
>>>>> should touch 0x10, and 0x14.
>>>> after check the code, if
>>>> pci_bridge_assign_resources ==> pdev_assign_resources_sorted ==>
>>>> pdev_sort_resources
>>>>
>>>> will not touch 0x10 and 0x14, if those resource is claimed by port
>>>> service.
>>>>
>>>> /* Sort resources by alignment */
>>>> void pdev_sort_resources(struct pci_dev *dev, struct resource_list *head)
>>>> { int i;
>>>> for (i = 0; i < PCI_NUM_RESOURCES; i++) {
>>>> struct resource *r;
>>>> struct resource_list *list, *tmp;
>>>> resource_size_t r_align;
>>>> r = &dev->resource[i];
>>>> if (r->flags &
>>>> IORESOURCE_PCI_FIXED)
>>>> continue;
>>>> if (!(r->flags) || r->parent)
>>>> continue;
>>>>
>>>> r->parent != NULL, will make it skip those two.
>>>>
>>>> So -v3 should be safe.
>>>>
>>> Thank you for the clarification.
>>>
>>> But I still don't understand the whole picture of your set of
>>> changes. Let me ask some questions.
>>>
>>> In my understanding of your set of changes, if there is a PCIe
>>> switch with some hot-plug slots and all of those slots are empty,
>>> I/O and Memory resources assigned by BIOS are all released at
>>> the boot time. For example, suppose the following case.
>>>
>>> bridge(A)
>>> |
>>> -----------------------
>>> | |
>>> bridge(B) bridge(C)
>>> | |
>>> slot(1) slot(2)
>>> (empty) (empty)
>>>
>>> bridge(A): P2P bridge for switch upstream port
>>> bridge(B): P2P bridge for switch downstream port
>>> bridge(C): P2P bridge for switch downstream port
>>>
>>> In the above example, I/O and Mem resource assigned to bridge(A),
>>> bridge(B) and bridge(C) are all released at the boot time. Correct?
>>>
>>> Then, when a adapter card is hot-added to slot(1), I/O and Mem
>>> resources enough for enabling the hot-added adapter card is assigned
>>> to bridge(A), bridge(B) and the adapter card. Correct?
>>>
>>> Then, when an another adpater card is hot-added to slot(2), we
>>> need to assign enough resource to bridge(C) and the new card.
>>> But bridge(A) doesn't have enough resource for bridge(C) and
>>> the new card. In addition, all bridge(A) and bridge(B) and the
>>> adapter card on slot(1) are already working. How do you assign
>>> resource to bridge(C) and the card on slot(2)?
>>>
>> thanks, will update the patches to only handle leaf bridge, and don't touch min_size etc.
>
> Tell me what is your expected behavior if I plug a bridge with hotplug
> slots into a leaf hotplug slot? Will you assign me enough resources so
> that I can plug in additional devices?

no.

you need to plug device in those slots and then insert it into a leaf hotplug slot.

>
> Today I make plugging in a hotplug bridge work by having the firmware
> reserve at one level and having the kernel reserve at the next level.
>
> Windows handles the issue in theory by performing some kind of
> hot-unplugging of drivers that already have assigned resources and
> then replugging them. Which allows a full renumbering of busses.
> We don't have the infrastructure to do that safely today.

that will take some drivers offline at first ?

YH
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/