Re: [PATCH 2/2] OvmfPkg/OvmfXen: Introduce XenDebugLibIoPort

["Laszlo Ersek" <lersek@redhat.com>]

On 04/20/20 18:29, Anthony PERARD wrote:
> Introduce XenDebugLibIoPort which is enabled with
> DEBUG_ON_HYPERVISOR_CONSOLE which send the debug output to Xen's
> console.
> 
> It's a copy PlatformDebugLibIoPort which always write to the IO port.
> It works with both Xen HVM guest and Xen PVH guest whereas the default
> PlatformDebugLibIoPort works only in HVM when QEMU is present.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
> 
> Notes:
>     Do you think it would be better to introduce a PCD to
>     PlatformDebugLibIoPort where we can disable the check
>     for the debug port and simply always write to it?
> 
>  OvmfPkg/OvmfXen.dsc                           | 13 ++++++++++++
>  .../PlatformDebugLibIoPort.inf                |  6 +++---
>  .../DebugLibDetect.h                          | 20 -------------------
>  .../DebugLib.c                                | 16 ---------------
>  .../DebugLibDetect.c                          | 20 ++-----------------
>  5 files changed, 18 insertions(+), 57 deletions(-)
>  copy OvmfPkg/Library/{PlatformDebugLibIoPort => XenDebugLibIoPort}/PlatformDebugLibIoPort.inf (76%)
>  copy OvmfPkg/Library/{PlatformDebugLibIoPort => XenDebugLibIoPort}/DebugLibDetect.h (57%)
>  copy OvmfPkg/Library/{PlatformDebugLibIoPort => XenDebugLibIoPort}/DebugLib.c (94%)
>  copy OvmfPkg/Library/{PlatformDebugLibIoPort => XenDebugLibIoPort}/DebugLibDetect.c (61%)

I think this approach is a good first approximation, but I think we can
refine it.

Rather than introducing a new directory, we should operate within
"OvmfPkg/Library/PlatformDebugLibIoPort".

(1) Please write a patch that replaces all instances of the string
"QEMU" with "hypervisor" in the following files:
- DebugLib.c
- DebugLibDetect.c
- DebugLibDetect.h
- DebugLibDetectRom.c

(2) Please write a patch for the following:

- move the PlatformDebugLibIoPortDetect() function definition to a
separate file called "DebugIoPortQemu.c",

- add the new C file to both existent INF files:
  - PlatformDebugLibIoPort.inf
  - PlatformRomDebugLibIoPort.inf
  (keep the source file lists alphabetically sorted)

- from "DebugLibDetect.h", move the BOCHS_DEBUG_PORT_MAGIC macro
definition to "DebugIoPortQemu.c".


I think this patch can be called:

  OvmfPkg/PlatformDebugLibIoPort: factor out debug port detection


After this patch, we well have two independent dimensions, for any
particular INF file:

- in PlatformDebugLibIoPortFound(), the library instance may or may not
cache the result of debug port checking

- in PlatformDebugLibIoPortDetect(), the library instance may abstract
the debug port detection method


(3) Please write another patch:
- introduce "DebugIoPortNocheck.c",
- implement PlatformDebugLibIoPortDetect() as "return TRUE",
- introduce a new INF file "PlatformRomDebugLibIoPortNocheck.inf", for
linking together:
  - DebugIoPortNocheck.c
  - DebugLib.c
  - DebugLibDetectRom.c

This library instance will not cache the result of the detection, and
the detection will return constant TRUE.

(4) Then, this library instance can be put to use in OvmfXen.dsc
(together with setting the proper PCD value). The library instance can
be used in all firmware phases / module types.

Thanks,
Laszlo