Creating new target for Cloud Hypervisor

Creating new target for Cloud Hypervisor

[Boeuf, Sebastien]

Hi all,

So far I've been able to patch the OvmfPkgX64 target to make it work for both
QEMU and Cloud Hypervisor, but as I try to enable more features (EFI shell for
instance) the gap is getting bigger and harder to keep them working together.

That's why I'm thinking about creating an OvmfCh target that would be a simple
copy of OvmfX64 at first, and then we could keep improving from there. There are
multiple things that are not needed by Cloud Hypervisor, which might help reduce
the complexity of the firmware, eventually leading to faster boot.

I'd like some confirmation from the community that it's okay to go down this road
before I proceed and send the patches.

Thanks,
Sebastien
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: Creating new target for Cloud Hypervisor

[Yao, Jiewen]

Looking at current OvmfPkg today. We have:
OvmfPkg\OvmfPkgIa32.fdf
OvmfPkg\OvmfPkgIa32X64.fdf
OvmfPkg\OvmfPkgX64.fdf
OvmfPkg\OvmfXen.fdf
OvmfPkg\AmdSev\AmdSevX64.fdf
OvmfPkg\Bhyve\BhyveX64.fdf
OvmfPkg\Microvm\MicrovmX64.fdf

And we will have OvmfPkg\IntelTdx\IntelTdxX64.fdf soon.

I think it is OK to create:
OvmfPkg\CloudHv\CloudHvX64.fdf

Thank you
Yao Jiewen

> -----Original Message-----
> From: Boeuf, Sebastien <sebastien.boeuf@intel.com>
> Sent: Monday, January 10, 2022 5:14 PM
> To: devel@edk2.groups.io; kraxel@redhat.com; Yao, Jiewen
> <jiewen.yao@intel.com>
> Subject: Creating new target for Cloud Hypervisor
> 
> Hi all,
> 
> So far I've been able to patch the OvmfPkgX64 target to make it work for both
> QEMU and Cloud Hypervisor, but as I try to enable more features (EFI shell for
> instance) the gap is getting bigger and harder to keep them working together.
> 
> That's why I'm thinking about creating an OvmfCh target that would be a simple
> copy of OvmfX64 at first, and then we could keep improving from there. There
> are
> multiple things that are not needed by Cloud Hypervisor, which might help
> reduce
> the complexity of the firmware, eventually leading to faster boot.
> 
> I'd like some confirmation from the community that it's okay to go down this
> road
> before I proceed and send the patches.
> 
> Thanks,
> Sebastien

Re: Creating new target for Cloud Hypervisor

[Gerd Hoffmann]

On Mon, Jan 10, 2022 at 09:13:44AM +0000, Boeuf, Sebastien wrote:
> Hi all,
> 
> So far I've been able to patch the OvmfPkgX64 target to make it work for both
> QEMU and Cloud Hypervisor, but as I try to enable more features (EFI shell for
> instance) the gap is getting bigger and harder to keep them working together.
> 
> That's why I'm thinking about creating an OvmfCh target that would be a simple
> copy of OvmfX64 at first, and then we could keep improving from there. There are
> multiple things that are not needed by Cloud Hypervisor, which might help reduce
> the complexity of the firmware, eventually leading to faster boot.
> 
> I'd like some confirmation from the community that it's okay to go down this road
> before I proceed and send the patches.

Well, depends.  A separate target is extra maintainance effort.  But
having to write code for runtime-switching where compile-time switching
would work without additional code is extra maintainance effort too ...

For microvm pci support (not yet merged) tipped things towards a
separate target.  pcie in microvm works completely different when
compared to pc/q35.  Using mmconfig for pci config space access is
mandatory, port 0xcf8 is not supported.  So fitting that with a runtime
switch into OvmfPkg/Library/DxePciLibI440FxQ35 (and probably some other
places) would have been quite messy, with a separate target is is *alot*
easier.

Quite a few places use a runtime switch nevertheless to avoid code
duplication.  PlatformPei for example is identical for both OvmfPkgX64
and MicrovmX86 targets, with case: branches for microvm in switch
statements.

So, what problem you are facing which makes you think a separate target
would work better?  The timer thing should be a non-issue as we plan to
switch over OvmfPkgX64 to use apic timer anyway.

take care,
  Gerd

Re: Creating new target for Cloud Hypervisor

[Boeuf, Sebastien]

On Mon, 2022-01-10 at 10:35 +0000, Yao, Jiewen wrote:
> Looking at current OvmfPkg today. We have:
> OvmfPkg\OvmfPkgIa32.fdf
> OvmfPkg\OvmfPkgIa32X64.fdf
> OvmfPkg\OvmfPkgX64.fdf
> OvmfPkg\OvmfXen.fdf
> OvmfPkg\AmdSev\AmdSevX64.fdf
> OvmfPkg\Bhyve\BhyveX64.fdf
> OvmfPkg\Microvm\MicrovmX64.fdf
> 
> And we will have OvmfPkg\IntelTdx\IntelTdxX64.fdf soon.
> 
> I think it is OK to create:
> OvmfPkg\CloudHv\CloudHvX64.fdf

Sounds good, I'll start working on this.

> 
> Thank you
> Yao Jiewen
> 
> > -----Original Message-----
> > From: Boeuf, Sebastien <sebastien.boeuf@intel.com>
> > Sent: Monday, January 10, 2022 5:14 PM
> > To: devel@edk2.groups.io; kraxel@redhat.com; Yao, Jiewen
> > <jiewen.yao@intel.com>
> > Subject: Creating new target for Cloud Hypervisor
> > 
> > Hi all,
> > 
> > So far I've been able to patch the OvmfPkgX64 target to make it
> > work for both
> > QEMU and Cloud Hypervisor, but as I try to enable more features
> > (EFI shell for
> > instance) the gap is getting bigger and harder to keep them working
> > together.
> > 
> > That's why I'm thinking about creating an OvmfCh target that would
> > be a simple
> > copy of OvmfX64 at first, and then we could keep improving from
> > there. There
> > are
> > multiple things that are not needed by Cloud Hypervisor, which
> > might help
> > reduce
> > the complexity of the firmware, eventually leading to faster boot.
> > 
> > I'd like some confirmation from the community that it's okay to go
> > down this
> > road
> > before I proceed and send the patches.
> > 
> > Thanks,
> > Sebastien

---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: Creating new target for Cloud Hypervisor

[Boeuf, Sebastien]

On Mon, 2022-01-10 at 11:45 +0100, kraxel@redhat.com wrote:
> On Mon, Jan 10, 2022 at 09:13:44AM +0000, Boeuf, Sebastien wrote:
> > Hi all,
> > 
> > So far I've been able to patch the OvmfPkgX64 target to make it
> > work for both
> > QEMU and Cloud Hypervisor, but as I try to enable more features
> > (EFI shell for
> > instance) the gap is getting bigger and harder to keep them working
> > together.
> > 
> > That's why I'm thinking about creating an OvmfCh target that would
> > be a simple
> > copy of OvmfX64 at first, and then we could keep improving from
> > there. There are
> > multiple things that are not needed by Cloud Hypervisor, which
> > might help reduce
> > the complexity of the firmware, eventually leading to faster boot.
> > 
> > I'd like some confirmation from the community that it's okay to go
> > down this road
> > before I proceed and send the patches.
> 
> Well, depends.  A separate target is extra maintainance effort.  But
> having to write code for runtime-switching where compile-time
> switching
> would work without additional code is extra maintainance effort too
> ...
> 
> For microvm pci support (not yet merged) tipped things towards a
> separate target.  pcie in microvm works completely different when
> compared to pc/q35.  Using mmconfig for pci config space access is
> mandatory, port 0xcf8 is not supported.  So fitting that with a
> runtime
> switch into OvmfPkg/Library/DxePciLibI440FxQ35 (and probably some
> other
> places) would have been quite messy, with a separate target is is
> *alot*
> easier.
> 
> Quite a few places use a runtime switch nevertheless to avoid code
> duplication.  PlatformPei for example is identical for both
> OvmfPkgX64
> and MicrovmX86 targets, with case: branches for microvm in switch
> statements.
> 
> So, what problem you are facing which makes you think a separate
> target
> would work better?  The timer thing should be a non-issue as we plan
> to
> switch over OvmfPkgX64 to use apic timer anyway.

Well I have a problem regarding SerialDxe because it breaks a bit QEMU
since adding it without removing the PciSerial registers two ways of
reading from serial. From microvm, you simply removed PciSerial since
you know it doesn't support LPC bridge, but I can't do the same here.
Can you think of any other way of properly handling this with a runtime
switch?

But more generally, things like the 8259 PIC, or PS2 keyboard are not
things that we try to support in Cloud Hypervisor, as well as Q35
specific bits being present in the target, meaning there's room for
simplification.

> 
> take care,
>   Gerd
> 

---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: Creating new target for Cloud Hypervisor

[Gerd Hoffmann]

  Hi,

> > So, what problem you are facing which makes you think a separate
> > target would work better?  The timer thing should be a non-issue as
> > we plan to switch over OvmfPkgX64 to use apic timer anyway.

> Well I have a problem regarding SerialDxe because it breaks a bit QEMU
> since adding it without removing the PciSerial registers two ways of
> reading from serial.

Gotcha.

> From microvm, you simply removed PciSerial since you know it doesn't
> support LPC bridge, but I can't do the same here.  Can you think of
> any other way of properly handling this with a runtime switch?

Well, tianocore isn't really designed for this.  Typically image builds
have to handle one specific platform only, so that kind of runtime
switches is not needed and support for it is not really present in
tianocore.

Virtualization is kind of special here as we have a single build
supporting multiple platforms (pc & q35 qemu machine types with various
config variants like sev/tdx on/off) to avoid the number of builds for
qemu explode and to make things less confusing for users.

So the ovmf runtime checks are open-coded in many places (all those
switch (mHostBridgeDevId) statements for example).  There is
ovmf-specific code for PCI where alot of the code only exists to
allow for runtime-switching between PCI (pc) and PCIe (q35).

So, yes, I guess it makes sense to have a separate target.  Avoids the
Serial issue, you can drop drivers, you can probably also simplify PCI
as I suspect you don't need the PCI/PCIe runtime switching, ...

take care,
  Gerd