[coreboot-gerrit] Change in coreboot[master]: driver/thunderbolt: Driver for allocating hotplug resources

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35946 )

Change subject: driver/thunderbolt: Driver for allocating hotplug resources
......................................................................


Patch Set 1:

(3 comments)

https://review.coreboot.org/c/coreboot/+/35946/1/src/drivers/thunderbolt/thu... 
File src/drivers/thunderbolt/thunderbolt.c:

https://review.coreboot.org/c/coreboot/+/35946/1/src/drivers/thunderbolt/thu... 
PS1, Line 51: 	resource->flags |= IORESOURCE_IO;
> In order for the hot-plugging of devices and busses to work, we need to reserve MMIO and IO space fo […]
> I understand the requirements in order to set up a pci topology. However, my question was bout the assumptions that are being made about the OS or program running after coreboot. This patch is assuming a static allocation is sufficient when it's definitely not going to be correct 100% of the time.

https://review.coreboot.org/c/coreboot/+/35946/1/src/drivers/thunderbolt/thu... 
PS1, Line 73: 	pciexp_scan_bridge(dev);
> haven't looked very closely at the code, but what does happen if there are devices already connected […]
> That's assuming thunderbolt links are already provisioned and pcie tunneling is happening. Otherwise one couldn't see the devices on the other side of the link.

https://review.coreboot.org/c/coreboot/+/35946/1/src/drivers/thunderbolt/thu... 
PS1, Line 80: 		slot = alloc_dev(dev->link_list, &slot_path);
> IIRC the 256MiB was what the vendor firmware does; not sure if this is mandated by the TB spec. 256MiB prefetchable MMIO space sounds reasonable to me; 256MiB non-prefetchable MMIO sounds rather big to me though (and the non-prefetchable can't be mapped above 4G)

What are you basing your assumptions on? Video cards and small BARs on a nic?



-- 
To view, visit https://review.coreboot.org/c/coreboot/+/35946
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I500191626584b83e6a8ae38417fd324b5e803afc
Gerrit-Change-Number: 35946
Gerrit-PatchSet: 1
Gerrit-Owner: Jeremy Soller <[e-mail]>
Gerrit-Reviewer: Jeremy Soller <[e-mail]>
Gerrit-Reviewer: Martin Roth <[e-mail]>
Gerrit-Reviewer: Patrick Georgi <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Aaron Durbin <[e-mail]>
Gerrit-CC: Brandon Breitenstein <[e-mail]>
Gerrit-CC: Divya S Sasidharan <[e-mail]>
Gerrit-CC: Duncan Laurie <[e-mail]>
Gerrit-CC: Felix Held <[e-mail]>
Gerrit-CC: Furquan Shaikh <[e-mail]>
Gerrit-CC: Kyösti Mälkki <[e-mail]>
Gerrit-CC: Matt DeVillier <[e-mail]>
Gerrit-CC: Mimoja <[e-mail]>
Gerrit-CC: Nathaniel L Desimone <[e-mail]>
Gerrit-CC: Nico Huber <[e-mail]>
Gerrit-CC: Patrick Rudolph <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-CC: Subrata Banik <[e-mail]>
Gerrit-Comment-Date: Fri, 11 Oct 2019 14:08:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Aaron Durbin <[e-mail]>
Comment-In-Reply-To: Felix Held <[e-mail]>
Comment-In-Reply-To: Patrick Rudolph <[e-mail]>
Comment-In-Reply-To: Nico Huber <[e-mail]>
Gerrit-MessageType: comment