Re: [SeaBIOS] [PATCH 1/2] nvme: add option to toggle probing only boot devices

[Paul Menzel]

Dear Julian,

On 07/06/18 11:32, Julian Stecklina wrote:
> Some systems have a large number of NVMe controllers. Probing all of
> them requires an excessive amount of memory and with out-of-order
> probing this also means that the important ones will not be probed,
> because SeaBIOS runs out of memory.
>
> Add an option to skip probing of NVMe devices, if there is no boot
> priority for them.
> Signed-off-by: Julian Stecklina
>
> src/Kconfig   |  8 ++++++++
>  src/hw/nvme.c | 14 +++++++++++++-
>  2 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/src/Kconfig b/src/Kconfig
> index 55a87cb..ef8f458 100644
> --- a/src/Kconfig
> +++ b/src/Kconfig
> @@ -233,6 +233,14 @@ menu "Hardware support"
>          default y
>          help
>              Support for NVMe disk code.
>
> config NVME_PROBE_BOOTDEVICES_ONLY
>    depends on NVME
>    bool "Only probe NVMe controllers with boot priority"
>    default n
>    help
>        Only enables NVMe controllers that have a boot priority
>        configured. This saves memory on systems with many non-bootable
>        NVMe devices.
>
> config PS2PORT
>        depends on KEYBOARD || MOUSE
>
> diff --git a/src/hw/nvme.c b/src/hw/nvme.c
> index e6d739d..5525f68 100644
> --- a/src/hw/nvme.c
> +++ b/src/hw/nvme.c
> @@ -625,6 +625,16 @@ nvme_controller_setup(void *opaque)
>      dprintf(2, "Failed to enable NVMe controller.\n");
>  }
>
> +static int should_setup_controller(struct pci_device *pci)
> +{
> +#ifdef NVME_PROBE_BOOTDEVICES_ONLY
>    return bootprio_find_pci_device(pci) >= 0;
>
> +#else
>    return 1;
>
> +#endif
> +}
>
> // Locate and init NVMe controllers
>  static void
>  nvme_scan(void)
> @@ -640,7 +650,9 @@ nvme_scan(void)
>              continue;
>          }
>    run_thread(nvme_controller_setup, pci);
>    if (should_setup_controller(pci)) {
>        run_thread(nvme_controller_setup, pci);
>    }
> }
>
> }

Thank you for the patch. Would it be better to make that a
run-time option?

Kind regards,

Paul