Boot Source Override feature problems

* 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