openbmc.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* Boot Source Override feature problems
@ 2021-05-26 20:50 Konstantin Aladyshev
  2021-06-11 12:17 ` Deepak Kodihalli
  0 siblings, 1 reply; 6+ messages in thread
From: Konstantin Aladyshev @ 2021-05-26 20:50 UTC (permalink / raw)
  To: openbmc

Hello!
I've merged almost all of my patchsets for the EFI/Legacy support in
the Boot Override feature (only bmcweb patchset is left
https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/40970) and I
want to return to the discussion about the current implementation of
the Boot Override feature in OpenBMC.

First, here are implementation details from IPMI and Redfish specs for
this feature:

IPMI specification (Document Revision 1.1 October 1, 2013)
```
IPMI:
1b - enabled/disabled
1b - one-time/permanent
1b - EFI/Legacy
4b - BDS (boot device selector)
  0000b = No override
  0001b = Force PXE
  0010b = Force boot from default Hard-drive
  0011b = Force boot from default Hard-drive, request Safe Mode
  0100b = Force boot from default Diagnostic Partition
  0101b = Force boot from default CD/DVD
  0110b = Force boot into BIOS Setup
  0111b = Force boot from remotely connected (redirected)
Floppy/primary removable media
  1001b = Force boot from primary remote media
  1000b = Force boot from remotely connected (redirected) CD/DVD
  1010b = reserved
  1011b = Force boot from remotely connected (redirected) Hard Drive
  1100-1110b = Reserved
  1111b = Force boot from Floppy/primary removable media
```
Redfish specification (DSP2046 2021.1 Redfish Resource and Schema
Guide 18 May 2021)
```
BootSourceOverrideEnabled - Continuous/Disabled/Once
BootSourceOverrideMode - Legacy/UEFI
BootSourceOverrideTarget -
  BiosSetup = Boot to the BIOS setup utility.
  Cd = Boot from the CD or DVD.
  Diags = Boot to the manufacturer's diagnostics program.
  Floppy = Boot from the floppy disk drive.
  Hdd = Boot from a hard drive.
  None = Boot from the normal boot device.
  Pxe = Boot from the Pre-Boot EXecution (PXE) environment.
  RemoteDrive (v1.2+) = Boot from a remote drive, such as an iSCSI target.
  SDCard (v1.1+) = Boot from an SD card.
  UefiBootNext (v1.5+) = Boot to the UEFI device that the BootNext
property specifies.
  UefiHttp (v1.1+) = Boot from a UEFI HTTP network location.
  UefiShell = Boot to the UEFI Shell.
  UefiTarget = Boot to the UEFI device specified in the
UefiTargetBootSourceOverride property.
  Usb = Boot from a system BIOS-specified USB device.
  Utilities = Boot to the manufacturer's utilities program or programs
```

In the OpenBMC the current Dbus interfaces for the Boot Override feature are:
```
/xyz/openbmc_project/control/host0/boot:
    - Interface: xyz.openbmc_project.Control.Boot.Source
    - Interface: xyz.openbmc_project.Control.Boot.Mode
    - Interface: xyz.openbmc_project.Control.Boot.Type
/xyz/openbmc_project/control/host0/boot/one_time:
    - Interface: xyz.openbmc_project.Control.Boot.Source
    - Interface: xyz.openbmc_project.Control.Boot.Mode
    - Interface: xyz.openbmc_project.Control.Boot.Type
    - Interface: xyz.openbmc_project.Object.Enable
```
It works this way:
- when `xyz.openbmc_project.Object.Enable` property under
`/xyz/openbmc_project/control/host0/boot/one_time` is set to `true` we
use Boot.Source/Mode/Type under
`/xyz/openbmc_project/control/host0/boot/one_time` for the override
feature.
- when `xyz.openbmc_project.Object.Enable` property under
`/xyz/openbmc_project/control/host0/boot/one_time` is set to `false`
we use Boot.Source/Mode/Type under
`/xyz/openbmc_project/control/host0/boot` for the override feature.

I don't really get why we split Override Source to `Boot.Source` and
`Boot.Mode`, but this is the question for another time.

Right now I want to discuss the main problem with this approach... it
is that OVERRIDE IS ALWAYS ENABLED. This
`xyz.openbmc_project.Object.Enable` property only indicates if we
should use permanent or one-time override.

I guess no one have noticed it since but by default override target
(`Boot.Source`) is set to something like "None". So no one have
experienced any difficulties. Override is enabled, but it says boot
default.

Proof that IPMI valid flag is always enabled:
```uint1_t validFlag = 1;```
https://github.com/openbmc/phosphor-host-ipmid/blob/e76a61a2f7c19ed07e2bfe98533d82bc23692fc1/chassishandler.cpp#L1861

`bmcweb` deals with this issue a little bit different (hello
inconsistency!), it performs weird logic to decide if it should set
`BootSourceOverrideEnabled` to `Disabled`.
https://github.com/openbmc/bmcweb/blob/master/redfish-core/lib/systems.hpp#L951
Which would get even weirder when support for EFI/Legacy selector
would be merged:
https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/40970/10/redfish-core/lib/systems.hpp#929

So as you can see, the current approach is kinda buggy, ipmid always
reports override as valid, bmcweb reports override as disabled when
you write `BootSourceOverrideTarget = None`.

This all is already a problem, but when we add Legacy/EFI selector
support, things are getting really messy.
ipmid can no longer always say that override is valid, since it would
be overriding boot either to EFI or Legacy.
bmcweb now can report that override is disabled only when
`BootSourceOverrideTarget = None` and `BootSourceOverrideMode = EFI`.
Weird, yeah? We write that we want override to `None/EFI`, but read
that override is `Disabled`. Weird and obviously wrong.

How to overcome all of this?
To be honest I don't see any use in splitting Boot Override feature in
two Dbus objects under `/xyz/openbmc_project/control/host0/boot` and
`/xyz/openbmc_project/control/host0/boot/one_time`, since we don't
need to fallback to permanent override after one-time override.

So I think the problem can be solved if we would have something like
this on Dbus to represent Boot Override feature:
```
/xyz/openbmc_project/control/host0/boot:
    - Interface: xyz.openbmc_project.Control.Boot.Source
    - Interface: xyz.openbmc_project.Control.Boot.Mode
    - Interface: xyz.openbmc_project.Control.Boot.Type
    - Interface: xyz.openbmc_project.Control.Boot.Permanent # true/false
    - Interface: xyz.openbmc_project.Object.Enable
```
I don't know if we can reuse any of the current interfaces for the
`xyz.openbmc_project.Control.Boot.Permanent` feature, but I think
something like these interfaces are what we need. With
`Boot.Permanent` we can drop `one-time` object, and with
`Object.Enable` we can solve all the aforementioned problems.

This is an architecture change, so please, I want some feedback from
the community for this change before I'll start working on the
patchsets.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: Boot Source Override feature problems
@ 2021-06-23 16:36 Konstantin Aladyshev
  0 siblings, 0 replies; 6+ messages in thread
From: Konstantin Aladyshev @ 2021-06-23 16:36 UTC (permalink / raw)
  To: openbmc; +Cc: a.kartashev, Deepak Kodihalli

We need to have two `xyz.openbmc_project.Object.Enable` interfaces:
1) one that stores information whether the BootSourceOverride feature
is enabled or not,
2) another one that stores information whether the BootSourceOverride
feature is permanent or one_time.

The current implementation has only 1), proposed design has both 1) and 2):
```
 /xyz/openbmc_project/control/host0/boot:
      - Interface: xyz.openbmc_project.Control.Boot.Source
      - Interface: xyz.openbmc_project.Control.Boot.Mode
      - Interface: xyz.openbmc_project.Control.Boot.Type
      - Interface: xyz.openbmc_project.Object.Enable              <---------  1)
 /xyz/openbmc_project/control/host0/boot/one_time:
      - Interface: xyz.openbmc_project.Object.Enable              <---------  2)
```

Also earlier there were two sets of Boot.Source/Boot.Mode/Boot.Type
settings. But the second one isn't really necessary as the
BootSourceOverride feature doesn't fallback to permanent override
after one-time override. So we need to keep only one set of
Boot.Source/Boot.Mode/Boot.Type settings.

Best regards,
Konstantin Aladyshev

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: Boot Source Override feature problems
@ 2021-06-23 18:08 Konstantin Aladyshev
  0 siblings, 0 replies; 6+ messages in thread
From: Konstantin Aladyshev @ 2021-06-23 18:08 UTC (permalink / raw)
  To: openbmc, a.kartashev, Deepak Kodihalli

On Wed, Jun 23, 2021 at 8:09 PM Andrei Kartashev <a.kartashev@yadro.com> wrote:
>
> On Wed, 2021-06-23 at 19:36 +0300, Konstantin Aladyshev wrote:
> > We need to have two `xyz.openbmc_project.Object.Enable` interfaces:
> > 1) one that stores information whether the BootSourceOverride feature
> > is enabled or not,
> > 2) another one that stores information whether the BootSourceOverride
> > feature is permanent or one_time.
> >
> > The current implementation has only 1), proposed design has both 1)
> > and 2):
> > ```
> >  /xyz/openbmc_project/control/host0/boot:
> >       - Interface: xyz.openbmc_project.Control.Boot.Source
> >       - Interface: xyz.openbmc_project.Control.Boot.Mode
> >       - Interface: xyz.openbmc_project.Control.Boot.Type
> >       - Interface: xyz.openbmc_project.Object.Enable              <--
> > -------  1)
> >  /xyz/openbmc_project/control/host0/boot/one_time:
> >       - Interface: xyz.openbmc_project.Object.Enable              <--
> > -------  2)
> > ```
> >
>
> Right, but your initial proposal was to use "Permanent" flag instead of
> second one:
>     - Interface: xyz.openbmc_project.Control.Boot.Permanent #
> true/false

To use something like  `xyz.openbmc_project.Control.Boot.Permanent` we
need to create another interface in the `phosphor-dbus-interfaces`
repository, which is something the OpenBMC community want to avoid
without necessities.
As the same functionality can be achieved with the
`xyz.openbmc_project.Object.Enable` interface under
`/xyz/openbmc_project/control/host0/boot` I've decided to take this
approach instead.

>
> BTW, can anyone explain me, why do we have all this as separate
> interfaces with only one properties?
> As for me, it should be one interface
> "xyz.openbmc_project.Control.Boot" with several properties.
>

I don't know.

> > Also earlier there were two sets of Boot.Source/Boot.Mode/Boot.Type
> > settings. But the second one isn't really necessary as the
> > BootSourceOverride feature doesn't fallback to permanent override
> > after one-time override. So we need to keep only one set of
> > Boot.Source/Boot.Mode/Boot.Type settings.
> >
>
> The fact that it was broken doesn't automatically mean that this need
> to be removed. May be it worth to fix this...
>

I didn't mean that something was broken here. I just meant that the
typical BootSourceOverride functionality in a BMC is not intended to
work this way.

> But here I tend to agree that
> "/xyz/openbmc_project/control/host0/boot/one_time" can be wasted. I
> just think it can be wasted completely. Then you can use GetAll on
> /xyz/openbmc_project/control/host0/boot to get all required data.
>

Currently "/xyz/openbmc_project/control/host0/boot/one_time" is used
to keep two separate `xyz.openbmc_project.Object.Enable` objects, so
we can't waste it.


> > Best regards,
> > Konstantin Aladyshev
>
> --
> Best regards,
> Andrei Kartashev
>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-06-23 18:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-26 20:50 Boot Source Override feature problems Konstantin Aladyshev
2021-06-11 12:17 ` Deepak Kodihalli
2021-06-21 15:59   ` Konstantin Aladyshev
2021-06-22 16:49     ` Andrei Kartashev
2021-06-23 16:36 Konstantin Aladyshev
2021-06-23 18:08 Konstantin Aladyshev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).