[PATCH 1/3] boot: cache HALT priority

[Gerd Hoffmann]

Call find_prio("HALT") only once, on first is_bootprio_strict() call.
Store the result in a variable and reuse it on subsequent calls.

Signed-off-by: Gerd Hoffmann 
---
 src/boot.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/boot.c b/src/boot.c
index 5182ab426b9f..afeb36a3319a 100644
--- a/src/boot.c
+++ b/src/boot.c
@@ -297,7 +297,11 @@ find_prio(const char *glob)

u8 is_bootprio_strict(void)
 {
-    return find_prio("HALT") >= 0;
+    static int prio_halt = -2;
+
+    if (prio_halt == -2)
+        prio_halt = find_prio("HALT");
+    return prio_halt >= 0;
 }

int bootprio_find_pci_device(struct pci_device *pci)

[PATCH 2/3] virtio-scsi: skip initializing non-bootable devices

[Gerd Hoffmann]

Check each disk attached to a virtio-scsi device whenever
it is bootable and skip initialization in case it isn't.

Signed-off-by: Gerd Hoffmann 
---
 src/hw/virtio-scsi.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/hw/virtio-scsi.c b/src/hw/virtio-scsi.c
index a27bdc1cfbb7..a5332848b8c8 100644
--- a/src/hw/virtio-scsi.c
+++ b/src/hw/virtio-scsi.c
@@ -111,8 +111,18 @@ virtio_scsi_init_lun(struct virtio_lun_s *vlun, struct pci_device *pci,
 static int
 virtio_scsi_add_lun(u32 lun, struct drive_s *tmpl_drv)
 {
+    u8 skip_nonbootable = is_bootprio_strict();
     struct virtio_lun_s *tmpl_vlun =
         container_of(tmpl_drv, struct virtio_lun_s, drive);
+    int prio = bootprio_find_scsi_device(tmpl_vlun->pci, tmpl_vlun->target, tmpl_vlun->lun);
+
+    if (skip_nonbootable && prio < 0) {
+        dprintf(1, "skipping init of a non-bootable virtio-scsi dev at %pP,"
+                " target %d, lun %d\n",
+                tmpl_vlun->pci, tmpl_vlun->target, tmpl_vlun->lun);
+        return -1;
+    }
+
     struct virtio_lun_s *vlun = malloc_low(sizeof(*vlun));
     if (!vlun) {
         warn_noalloc();
@@ -123,7 +133,6 @@ virtio_scsi_add_lun(u32 lun, struct drive_s *tmpl_drv)

boot_lchs_find_scsi_device(vlun->pci, vlun->target, vlun->lun,
                                &(vlun->drive.lchs));
-    int prio = bootprio_find_scsi_device(vlun->pci, vlun->target, vlun->lun);
     int ret = scsi_drive_setup(&vlun->drive, "virtio-scsi", prio);
     if (ret)
         goto fail;

[PATCH 2/3] virtio-scsi: skip initializing non-bootable devices

[Alexey Kirillov]

Hi,

14.01.2020, 12:25, "Gerd Hoffmann" :
> Check each disk attached to a virtio-scsi device whenever
> it is bootable and skip initialization in case it isn't.
>
> Signed-off-by: Gerd Hoffmann

Reviewed-by: Alexey Kirillov
> src/hw/virtio-scsi.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/src/hw/virtio-scsi.c b/src/hw/virtio-scsi.c
> index a27bdc1cfbb7..a5332848b8c8 100644
> --- a/src/hw/virtio-scsi.c
> +++ b/src/hw/virtio-scsi.c
> @@ -111,8 +111,18 @@ virtio_scsi_init_lun(struct virtio_lun_s *vlun, struct pci_device *pci,
>  static int
>  virtio_scsi_add_lun(u32 lun, struct drive_s *tmpl_drv)
>  {
> u8 skip_nonbootable = is_bootprio_strict();
>
> struct virtio_lun_s *tmpl_vlun =
>          container_of(tmpl_drv, struct virtio_lun_s, drive);
> int prio = bootprio_find_scsi_device(tmpl_vlun->pci, tmpl_vlun->target, tmpl_vlun->lun);
> if (skip_nonbootable && prio < 0) {
> dprintf(1, "skipping init of a non-bootable virtio-scsi dev at %pP,"
> " target %d, lun %d\n",
> tmpl_vlun->pci, tmpl_vlun->target, tmpl_vlun->lun);
> return -1;
> }
>
> struct virtio_lun_s *vlun = malloc_low(sizeof(*vlun));
>      if (!vlun) {
>          warn_noalloc();
> @@ -123,7 +133,6 @@ virtio_scsi_add_lun(u32 lun, struct drive_s *tmpl_drv)
>
> boot_lchs_find_scsi_device(vlun->pci, vlun->target, vlun->lun,
>                                 &(vlun->drive.lchs));
> int prio = bootprio_find_scsi_device(vlun->pci, vlun->target, vlun->lun);
>
> int ret = scsi_drive_setup(&vlun->drive, "virtio-scsi", prio);
>      if (ret)
>          goto fail;
> --
> 2.18.1

-- 
Alexey Kirillov
Yandex.Cloud

[PATCH 2/3] virtio-scsi: skip initializing non-bootable devices

[Philippe Mathieu-Daudé]

On 1/14/20 10:25 AM, Gerd Hoffmann wrote:
> Check each disk attached to a virtio-scsi device whenever
> it is bootable and skip initialization in case it isn't.
>
> Signed-off-by: Gerd Hoffmann

Reviewed-by: Philippe Mathieu-Daudé
> src/hw/virtio-scsi.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/src/hw/virtio-scsi.c b/src/hw/virtio-scsi.c
> index a27bdc1cfbb7..a5332848b8c8 100644
> --- a/src/hw/virtio-scsi.c
> +++ b/src/hw/virtio-scsi.c
> @@ -111,8 +111,18 @@ virtio_scsi_init_lun(struct virtio_lun_s *vlun, struct pci_device *pci,
>   static int
>   virtio_scsi_add_lun(u32 lun, struct drive_s *tmpl_drv)
>   {
> u8 skip_nonbootable = is_bootprio_strict();
>  struct virtio_lun_s *tmpl_vlun =
>      container_of(tmpl_drv, struct virtio_lun_s, drive);
> int prio = bootprio_find_scsi_device(tmpl_vlun->pci, tmpl_vlun->target, tmpl_vlun->lun);
> if (skip_nonbootable && prio < 0) {
>    dprintf(1, "skipping init of a non-bootable virtio-scsi dev at %pP,"
>            " target %d, lun %d\n",
>            tmpl_vlun->pci, tmpl_vlun->target, tmpl_vlun->lun);
>    return -1;
> }
> struct virtio_lun_s *vlun = malloc_low(sizeof(*vlun));
> if (!vlun) {
>     warn_noalloc();
>
> @@ -123,7 +133,6 @@ virtio_scsi_add_lun(u32 lun, struct drive_s *tmpl_drv)
>   boot_lchs_find_scsi_device(vlun->pci, vlun->target, vlun->lun,
>                              &(vlun->drive.lchs));
> int prio = bootprio_find_scsi_device(vlun->pci, vlun->target, vlun->lun);
>  int ret = scsi_drive_setup(&vlun->drive, "virtio-scsi", prio);
>  if (ret)
>      goto fail;

[PATCH 3/3] nvme: skip initializing non-bootable devices

[Gerd Hoffmann]

Check NVMe devices whenever they are bootable,
skip initialization in case they are not.

Signed-off-by: Gerd Hoffmann 
---
 src/hw/nvme.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/hw/nvme.c b/src/hw/nvme.c
index 2e3aa38682c4..41f3b768528e 100644
--- a/src/hw/nvme.c
+++ b/src/hw/nvme.c
@@ -586,8 +586,15 @@ nvme_controller_enable(struct nvme_ctrl *ctrl)
 static void
 nvme_controller_setup(void *opaque)
 {
+    u8 skip_nonbootable = is_bootprio_strict();
     struct pci_device *pci = opaque;

+    if (skip_nonbootable && bootprio_find_pci_device(pci) < 0) {
+        dprintf(1, "skipping init of a non-bootable NVMe at %pP\n",
+                pci);
+        goto err;
+    }
+
     struct nvme_reg volatile *reg = pci_enable_membar(pci, PCI_BASE_ADDRESS_0);
     if (!reg)
         return;

[PATCH 3/3] nvme: skip initializing non-bootable devices

[Alexey Kirillov]

Hi,

14.01.2020, 12:25, "Gerd Hoffmann" :
> Check NVMe devices whenever they are bootable,
> skip initialization in case they are not.
>
> Signed-off-by: Gerd Hoffmann

Reviewed-by: Alexey Kirillov
> src/hw/nvme.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/src/hw/nvme.c b/src/hw/nvme.c
> index 2e3aa38682c4..41f3b768528e 100644
> --- a/src/hw/nvme.c
> +++ b/src/hw/nvme.c
> @@ -586,8 +586,15 @@ nvme_controller_enable(struct nvme_ctrl *ctrl)
>  static void
>  nvme_controller_setup(void *opaque)
>  {
> u8 skip_nonbootable = is_bootprio_strict();
>
> struct pci_device *pci = opaque;
> if (skip_nonbootable && bootprio_find_pci_device(pci) < 0) {
> dprintf(1, "skipping init of a non-bootable NVMe at %pP\n",
> pci);
> goto err;
> }
>
> struct nvme_reg volatile *reg = pci_enable_membar(pci, PCI_BASE_ADDRESS_0);
>      if (!reg)
>          return;
> --
> 2.18.1

-- 
Alexey Kirillov
Yandex.Cloud

[PATCH 3/3] nvme: skip initializing non-bootable devices

[Philippe Mathieu-Daudé]

On 1/14/20 10:25 AM, Gerd Hoffmann wrote:
> Check NVMe devices whenever they are bootable,
> skip initialization in case they are not.
> Signed-off-by: Gerd Hoffmann
>
> src/hw/nvme.c | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/src/hw/nvme.c b/src/hw/nvme.c
> index 2e3aa38682c4..41f3b768528e 100644
> --- a/src/hw/nvme.c
> +++ b/src/hw/nvme.c
> @@ -586,8 +586,15 @@ nvme_controller_enable(struct nvme_ctrl *ctrl)
>   static void
>   nvme_controller_setup(void *opaque)
>   {
>
> u8 skip_nonbootable = is_bootprio_strict();
>  struct pci_device *pci = opaque;
>
> if (skip_nonbootable && bootprio_find_pci_device(pci) < 0) {
>    dprintf(1, "skipping init of a non-bootable NVMe at %pP\n",
>            pci);
>    goto err;
>
> }
> struct nvme_reg volatile *reg = pci_enable_membar(pci, PCI_BASE_ADDRESS_0);
> if (!reg)
>     return;

Reviewed-by: Philippe Mathieu-Daudé

[Alexey Kirillov]

Hi,

14.01.2020, 12:25, "Gerd Hoffmann" :
> Call find_prio("HALT") only once, on first is_bootprio_strict() call.
> Store the result in a variable and reuse it on subsequent calls.
>
> Signed-off-by: Gerd Hoffmann

Reviewed-by: Alexey Kirillov
> src/boot.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/src/boot.c b/src/boot.c
> index 5182ab426b9f..afeb36a3319a 100644
> --- a/src/boot.c
> +++ b/src/boot.c
> @@ -297,7 +297,11 @@ find_prio(const char *glob)
>
> u8 is_bootprio_strict(void)
>  {
> return find_prio("HALT") >= 0;
> static int prio_halt = -2;
> if (prio_halt == -2)
> prio_halt = find_prio("HALT");
> return prio_halt >= 0;
>
> }
> int bootprio_find_pci_device(struct pci_device *pci)
>
> 2.18.1

-- 
Alexey Kirillov
Yandex.Cloud

[Philippe Mathieu-Daudé]

On 1/14/20 10:25 AM, Gerd Hoffmann wrote:
> Call find_prio("HALT") only once, on first is_bootprio_strict() call.
> Store the result in a variable and reuse it on subsequent calls.
>
> Signed-off-by: Gerd Hoffmann

Reviewed-by: Philippe Mathieu-Daudé
> src/boot.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/src/boot.c b/src/boot.c
> index 5182ab426b9f..afeb36a3319a 100644
> --- a/src/boot.c
> +++ b/src/boot.c
> @@ -297,7 +297,11 @@ find_prio(const char *glob)
>
> u8 is_bootprio_strict(void)
>   {
> return find_prio("HALT") >= 0;
>
> static int prio_halt = -2;
>
> if (prio_halt == -2)
>    prio_halt = find_prio("HALT");
>
> return prio_halt >= 0;
> }
>
> int bootprio_find_pci_device(struct pci_device *pci)

[Laszlo Ersek]

On 01/14/20 10:25, Gerd Hoffmann wrote:
> Call find_prio("HALT") only once, on first is_bootprio_strict() call.
> Store the result in a variable and reuse it on subsequent calls.
> Signed-off-by: Gerd Hoffmann
>
> src/boot.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/src/boot.c b/src/boot.c
> index 5182ab426b9f..afeb36a3319a 100644
> --- a/src/boot.c
> +++ b/src/boot.c
> @@ -297,7 +297,11 @@ find_prio(const char *glob)
>
> u8 is_bootprio_strict(void)
>  {
> return find_prio("HALT") >= 0;
> static int prio_halt = -2;
> if (prio_halt == -2)
>    prio_halt = find_prio("HALT");
> return prio_halt >= 0;
>
> }
>
> int bootprio_find_pci_device(struct pci_device *pci)

General question. Is it safe to use static variables in this file with
initializers, i.e. without assigning their initial values through
statements?

What happens at reset? In particular, the "bootorder" fw_cfg file may
change across reset. (I'm not certain if specifically "HALT" can change
in "bootorder" across reset.)

I've found two static variables in this file:

static BootDevice *BootDevices VARVERIFY32INIT;
static int BootDeviceCount;

They seem to be set explicitly (in assignment statements) in the
loadBootDevices() function. "BootDeviceCount" seems to support my
concern, because

BootDeviceCount = 0;

is otherwise redundant, given:

static int BootDeviceCount;

I realize this is a very basic question to someone closely familiar with
the SeaBIOS architecture. Thanks for bearing with me!

Laszlo

[Kevin O'Connor]

On Tue, Jan 14, 2020 at 06:10:43PM +0100, Laszlo Ersek wrote:
> On 01/14/20 10:25, Gerd Hoffmann wrote:
> > Call find_prio("HALT") only once, on first is_bootprio_strict() call.
> > Store the result in a variable and reuse it on subsequent calls.
> > Signed-off-by: Gerd Hoffmann
> >
> > src/boot.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/boot.c b/src/boot.c
> > index 5182ab426b9f..afeb36a3319a 100644
> > --- a/src/boot.c
> > +++ b/src/boot.c
> > @@ -297,7 +297,11 @@ find_prio(const char *glob)
> >
> > u8 is_bootprio_strict(void)
> >  {
> > return find_prio("HALT") >= 0;
> > static int prio_halt = -2;
> > if (prio_halt == -2)
> >    prio_halt = find_prio("HALT");
> > return prio_halt >= 0;
> >
> > }
> >
> > int bootprio_find_pci_device(struct pci_device *pci)
>
> General question. Is it safe to use static variables in this file with
> initializers, i.e. without assigning their initial values through
> statements?

I'm not aware of any issues with that.  It should work.
> What happens at reset? In particular, the "bootorder" fw_cfg file may
> change across reset. (I'm not certain if specifically "HALT" can change
> in "bootorder" across reset.)

On a SeaBIOS reset it should get reset to -2 as far as I'm aware.

-Kevin