Re: [PATCH 1/1] OvmfPkg/BhyveBhf: install bhyve's ACPI tables

["Rebecca Cran" <rebecca@bsdio.com>]

I applied the patch and tested it. The UEFI Shell command "acpiview" 
reports an error. I enabled the "acpiview" command with the patch:


diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
index d0d2712c5662..0c04e4834cf7 100644
--- a/OvmfPkg/Bhyve/BhyveX64.dsc
+++ b/OvmfPkg/Bhyve/BhyveX64.dsc
@@ -785,6 +785,7 @@ [Components]
NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
+ 
NULL|ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf
  !if $(NETWORK_IP6_ENABLE) == TRUE
NULL|ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.inf
  !endif


The bhyve command I used is:

bhyve -AHP -s 0:0,hostbridge -s 1:0,lpc -s 2:0,virtio-net,tap1 -s 
3:0,virtio-blk,./guest.img -s 
4:0,ahci-cd,/home/bcran/FreeBSD-14.0-CURRENT-amd64-20230427-60167184abd5-262599-disc1.iso 
-c 4 -m 1G -s 29,fbuf,tcp=0.0.0.0:5900,w=1280,h=1024,wait -s 
30,xhci,tablet -l 
bootrom,/home/bcran/src/uefi/edk2/Build/BhyveX64/DEBUG_GCC5/FV/BHYVE_CODE.fd,/home/bcran/src/uefi/edk2/Build/BhyveX64/DEBUG_GCC5/FV/BHYVE_VARS.fd 
-l com1,stdio guest


MADT                                 :
   Signature                          : APIC
   Length                             : 114
   Revision                           : 1
   Checksum                           : 0x66
   Oem ID                             : BHYVE
   Oem Table ID                       : BVAPIC
   Oem Revision                       : 0x1
   Creator ID                         : BASL
   Creator Revision                   : 0x20220504
   Local Interrupt Controller Address : 0xFEE00000
   Flags                              : 0x1
   PROCESSOR LOCAL APIC               :
     Type                             : 0x0
     Length                           : 8
     ACPI Processor UID               : 0x0
     APIC ID                          : 0x0
     Flags                            : 0x1
       Enabled                        : 1
       Online Capable                 : 0
       Reserved                       : 0

   PROCESSOR LOCAL APIC               :
     Type                             : 0x0
     Length                           : 8
     ACPI Processor UID               : 0x1
     APIC ID                          : 0x1
     Flags                            : 0x1
       Enabled                        : 1
       Online Capable                 : 0
       Reserved                       : 0

   PROCESSOR LOCAL APIC               :
     Type                             : 0x0
     Length                           : 8
     ACPI Processor UID               : 0x2
     APIC ID                          : 0x2
     Flags                            : 0x1
       Enabled                        : 1
       Online Capable                 : 0
       Reserved                       : 0

   PROCESSOR LOCAL APIC               :
     Type                             : 0x0
     Length                           : 8
     ACPI Processor UID               : 0x3
     APIC ID                          : 0x3
     Flags                            : 0x1
       Enabled                        : 1
       Online Capable                 : 0
       Reserved                       : 0

   IO APIC                            :
     Type                             : 0x1
     Length                           : 12
     I/O APIC ID                      : 0x0
     Reserved                         : 0x0
     I/O APIC Address                 : 0xFEC00000
     Global System Interrupt Base     : 0x0
   INTERRUPT SOURCE OVERRIDE          :
     Type                             : 0x2
     Length                           : 10
     Bus                              : 0x0
     Source                           : 0x0
     Global System Interrupt          : 0x2
     Flags                            : 0x5
   INTERRUPT SOURCE OVERRIDE          :
     Type                             : 0x2
     Length                           : 10
     Bus                              : 0x0
     Source                           : 0x9
     Global System Interrupt          : 0x9
     Flags                            : 0xF
ERROR: Unknown Interrupt Controller Structure, Type = 4, Length = 6


-- 

Rebecca Cran


On 4/17/23 05:53, Rebecca Cran wrote:
> On 4/17/23 5:46 AM, Corvin Köhne wrote:
>> +
>> +/**
>> +  Get the address of bhyve's ACPI Root System Description Pointer 
>> (RSDP).
>> +
>> +  @param  RsdpPtr             Return pointer to RSDP.
>> +
>> +  @return EFI_SUCCESS         Bhyve's RSDP successfully found.
>> +  @return EFI_NOT_FOUND       Couldn't find bhyve's RSDP.
>> +  @return EFI_UNSUPPORTED     Revision is lower than 2.
>> +  @return EFI_PROTOCOL_ERROR  Invalid RSDP found.
>
> I think these should be @retval instead of @return ?
>
>> +    if (Rsdp->Revision < 2) {
>> +      DEBUG ((DEBUG_INFO, "%a: unsupported RSDP found\n", 
>> __FUNCTION__));
>
> You missed converting this __FUNCTION__ to __func__.
>
>> +    //
>> +    // For ACPI 1.0/2.0/3.0 the checksum of first 20 bytes should be 0.
>> +    // For ACPI 2.0/3.0 the checksum of the entire table should be 0.
>> +    //
>> +    UINT8  Sum = CalculateCheckSum8 (
>> +                   (CONST UINT8 *)Rsdp,
>> +                   sizeof 
>> (EFI_ACPI_1_0_ROOT_SYSTEM_DESCRIPTION_POINTER)
>> +                   );
>
> Variables should be declared at the top of the function, and 
> initialized separately.
>
>> +    if (Sum != 0) {
>> +      DEBUG ((
>> +        DEBUG_INFO,
>> +        "%a: RSDP header checksum not valid: 0x%02x\n",
>> +        __func__,
>> +        Sum
>> +        ));
>> +      return EFI_PROTOCOL_ERROR;
>> +    }
>> +
>> +    Sum = CalculateCheckSum8 (
>> +            (CONST UINT8 *)Rsdp,
>> +            sizeof (EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER)
>> +            );
>> +    if (Sum != 0) {
>> +      DEBUG ((
>> +        DEBUG_INFO,
>> +        "%a: RSDP table checksum not valid: 0x%02x\n",
>> +        __func__,
>> +        Sum
>> +        ));
>> +      return EFI_PROTOCOL_ERROR;
>> +    }
>> +
>> +    //
>> +    // RSDP was found and is valid
>> +    //
>> +    *RsdpPtr = Rsdp;
>> +
>> +    return EFI_SUCCESS;
>> +  }
>> +
>> +  DEBUG ((DEBUG_INFO, "%a: RSDP not found\n", __func__));
> Should these DEBUG_INFO messages which appear to be warnings/errors 
> use DEBUG_WARN or DEBUG_ERROR, instead?
>
>