[SeaBIOS] Re: [PATCH 1/4] pci: Allow scanning pci bus number up to 255 in CSM mode
> On Thu, Dec 31, 2020 at 07:08:23PM -0800, Bin Gao wrote:
> > On real hardware especially server platforms, there might be multiple
> > root buses, thus pci bus number could run up to 255. This patch fixed
> > pci_probe_devices() by allowing to scan all 256 pci bus numbers(0-255).
> >
> > Signed-off-by: Bin Gao
> > ---
> > src/hw/pcidevice.c | 8
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/src/hw/pcidevice.c b/src/hw/pcidevice.c
> > index 8853cf7..8d9c401 100644
> > --- a/src/hw/pcidevice.c
> > +++ b/src/hw/pcidevice.c
> > @@ -26,6 +26,14 @@ pci_probe_devices(void)
> > struct hlist_node **pprev =
> > int extraroots = romfile_loadint("etc/extra-pci-roots", 0);
> > int bus = -1, lastbus = 0, rootbuses = 0, count=0;
> > +
> > +// There might be multiple PCI root buses on physical hardware
> > especially
> > +// server platforms, thus bus number could run up to the top value,
> > +// i.e. 0xff. Setting extraroots to 0xff to ensure we can enumerate all
> > +// PCI bus numbers.
> > +if (CONFIG_CSM)
> > +extraroots = 0xff;
>
> This doesn't look correct to me. It would take a noticeable amount of
> time to scan every possible PCI root bus. It would introduce a
> regression for any current users of CONFIG_CSM.
On real hardware scanning every possible PCI root bus wouldn't be slow so it's
not a problem. Would it be reasonable to scan all pci buses only on physical
hardware?
if (CONFIG_CSM && ! CONFIG_QEMU_HARDWARE)
extraroots = 0xff;
>
> The "etc/extra-pci-roots" config is intended to allow users to make
> these choices at runtime. If more configurability is desired on
> CONFIG_CSM, then it will be needed to figure out a way to pass options
> to SeaBIOS when in that mode.
So far I only need this extraroots config on CONFIG_CSM.
I didn't see an easy way to pass options from UEFI BIOS to SeaBIOS, unless we
extend the CSM protocol defined by Intel.
>
> -Kevin
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org
[SeaBIOS] Re: [PATCH 1/4] pci: Allow scanning pci bus number up to 255 in CSM mode
On Thu, Dec 31, 2020 at 07:08:23PM -0800, Bin Gao wrote:
> On real hardware especially server platforms, there might be multiple
> root buses, thus pci bus number could run up to 255. This patch fixed
> pci_probe_devices() by allowing to scan all 256 pci bus numbers(0-255).
>
> Signed-off-by: Bin Gao
> ---
> src/hw/pcidevice.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/src/hw/pcidevice.c b/src/hw/pcidevice.c
> index 8853cf7..8d9c401 100644
> --- a/src/hw/pcidevice.c
> +++ b/src/hw/pcidevice.c
> @@ -26,6 +26,14 @@ pci_probe_devices(void)
> struct hlist_node **pprev =
> int extraroots = romfile_loadint("etc/extra-pci-roots", 0);
> int bus = -1, lastbus = 0, rootbuses = 0, count=0;
> +
> +// There might be multiple PCI root buses on physical hardware especially
> +// server platforms, thus bus number could run up to the top value,
> +// i.e. 0xff. Setting extraroots to 0xff to ensure we can enumerate all
> +// PCI bus numbers.
> +if (CONFIG_CSM)
> +extraroots = 0xff;
This doesn't look correct to me. It would take a noticeable amount of
time to scan every possible PCI root bus. It would introduce a
regression for any current users of CONFIG_CSM.
The "etc/extra-pci-roots" config is intended to allow users to make
these choices at runtime. If more configurability is desired on
CONFIG_CSM, then it will be needed to figure out a way to pass options
to SeaBIOS when in that mode.
-Kevin
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org
[SeaBIOS] Re: [PATCH 1/4] pci: Allow scanning pci bus number up to 255 in CSM mode
>
>Dear Bin,
>
>
>Thank you very much for your patches.
>
>Am 01.01.21 um 04:08 schrieb Bin Gao:
>> On real hardware especially server platforms, there might be multiple
>
>Please give a concrete example of the hardware.
All Intel server processors have multiple PCI roots, called IIO.
>
>> root buses, thus pci bus number could run up to 255. This patch fixed
>
>Nit: I’d use present tense in commit messages: This patch fixes ….
>
>> pci_probe_devices() by allowing to scan all 256 pci bus numbers(0-255).
>
>Nit: Space before (.
Will fix this (and tense) in next revision.
>
>> Signed-off-by: Bin Gao
>> ---
>> src/hw/pcidevice.c | 8
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/src/hw/pcidevice.c b/src/hw/pcidevice.c
>> index 8853cf7..8d9c401 100644
>> --- a/src/hw/pcidevice.c
>> +++ b/src/hw/pcidevice.c
>> @@ -26,6 +26,14 @@ pci_probe_devices(void)
>> struct hlist_node **pprev =
>> int extraroots = romfile_loadint("etc/extra-pci-roots", 0);
>> int bus = -1, lastbus = 0, rootbuses = 0, count=0;
>> +
>> +// There might be multiple PCI root buses on physical hardware
>> especially
>> +// server platforms, thus bus number could run up to the top value,
>> +// i.e. 0xff. Setting extraroots to 0xff to ensure we can enumerate all
>> +// PCI bus numbers.
>> +if (CONFIG_CSM)
>> +extraroots = 0xff;
>> +
>
>Can there be a romfile with CSM? If yes, the configured value would be
>overwritten. Maybe add a log message in that case?
There is no romfile with CSM.
I do think the correct fix is to scan all the PCI buses, instead of to get a
default max bus number. The fact is,
on physical hardware, we can't assume the max bus number. On typical server
design, it easily consumes
up all the bus numbers, e.g. some PCIe bridges could pad up with many buses. On
Intel server processors, for instance,
we may want to split the 256 bus number to all IIOs so it's almost sure that we
can reach up the maximal possible
bus number (in a single PC segment).
Yes, I can add a log message in that case.
>
>> while (bus < 0xff && (bus < MaxPCIBus || rootbuses < extraroots)) {
>> bus++;
>> int bdf;
>>
>
>
>Kind regards,
>
>Paul
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org
[SeaBIOS] Re: [PATCH 1/4] pci: Allow scanning pci bus number up to 255 in CSM mode
Dear Bin,
Thank you very much for your patches.
Am 01.01.21 um 04:08 schrieb Bin Gao:
On real hardware especially server platforms, there might be multiple
Please give a concrete example of the hardware.
root buses, thus pci bus number could run up to 255. This patch fixed
Nit: I’d use present tense in commit messages: This patch fixes ….
pci_probe_devices() by allowing to scan all 256 pci bus numbers(0-255).
Nit: Space before (.
Signed-off-by: Bin Gao
---
src/hw/pcidevice.c | 8
1 file changed, 8 insertions(+)
diff --git a/src/hw/pcidevice.c b/src/hw/pcidevice.c
index 8853cf7..8d9c401 100644
--- a/src/hw/pcidevice.c
+++ b/src/hw/pcidevice.c
@@ -26,6 +26,14 @@ pci_probe_devices(void)
struct hlist_node **pprev =
int extraroots = romfile_loadint("etc/extra-pci-roots", 0);
int bus = -1, lastbus = 0, rootbuses = 0, count=0;
+
+// There might be multiple PCI root buses on physical hardware especially
+// server platforms, thus bus number could run up to the top value,
+// i.e. 0xff. Setting extraroots to 0xff to ensure we can enumerate all
+// PCI bus numbers.
+if (CONFIG_CSM)
+extraroots = 0xff;
+
Can there be a romfile with CSM? If yes, the configured value would be
overwritten. Maybe add a log message in that case?
while (bus < 0xff && (bus < MaxPCIBus || rootbuses < extraroots)) {
bus++;
int bdf;
Kind regards,
Paul
___
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org
