Merged - https://github.com/tianocore/edk2/pull/3317
From: Boeuf, Sebastien <sebastien.bo...@intel.com> Sent: Wednesday, September 7, 2022 11:25 PM To: Yao, Jiewen <jiewen....@intel.com> Cc: kra...@redhat.com; Justen, Jordan L <jordan.l.jus...@intel.com>; devel@edk2.groups.io Subject: Re: [PATCH v2] OvmfPkg: Update I/O port related to ACPI devices for CloudHv Ok sounds good. Thanks, Sebastien ________________________________ From: Yao, Jiewen <jiewen....@intel.com<mailto:jiewen....@intel.com>> Sent: Wednesday, September 7, 2022 5:23 PM To: Boeuf, Sebastien <sebastien.bo...@intel.com<mailto:sebastien.bo...@intel.com>> Cc: kra...@redhat.com<mailto:kra...@redhat.com> <kra...@redhat.com<mailto:kra...@redhat.com>>; Justen, Jordan L <jordan.l.jus...@intel.com<mailto:jordan.l.jus...@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> Subject: RE: [PATCH v2] OvmfPkg: Update I/O port related to ACPI devices for CloudHv I see. The is hard to let a base lib access the HOB. I think we can integrate this patch at first to make it work, with known limitation. > -----Original Message----- > From: Boeuf, Sebastien > <sebastien.bo...@intel.com<mailto:sebastien.bo...@intel.com>> > Sent: Wednesday, September 7, 2022 11:21 PM > To: Yao, Jiewen <jiewen....@intel.com<mailto:jiewen....@intel.com>> > Cc: kra...@redhat.com<mailto:kra...@redhat.com>; Justen, Jordan L > <jordan.l.jus...@intel.com<mailto:jordan.l.jus...@intel.com>>; > devel@edk2.groups.io<mailto:devel@edk2.groups.io> > Subject: Re: [PATCH v2] OvmfPkg: Update I/O port related to ACPI devices > for CloudHv > > I had a quick try and I've realized > OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.c didn't have access > to the EFI_HOB_PLATFORM_INFO. Is there an alternative? > > Thanks, > Sebastien > > On Wed, 2022-09-07 at 16:23 +0200, Sebastien Boeuf wrote: > > Hi Jiewen, > > > > After I looked into the UefiPayload example, I have a few questions > > on > > how to implement things in OvmfPkg: > > > > - Do you expect EFI_HOB_PLATFORM_INFO to be extended with two > > additional fields AcpiTimerAddress and AcpiShutdownAddress? Or do you > > think the ACPI_BOARD_INFO should be copied over from the the > > UefiPayload package? > > > > - Is InitializePlatform() from OvmfPkg/PlatformPei/Platform.c the > > correct place where the FADT parsing should happen? I would need the > > platform info HOB to be accessible from > > OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c, > > OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.c, > > OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c, > > OvmfPkg/Library/ResetSystemLib/DxeResetShutdown.c, and > > OvmfPkg/Library/ResetSystemLib/BaseResetShutdown.c. > > > > Thanks, > > Sebastien > > > > On Tue, 2022-09-06 at 15:57 +0000, Yao, Jiewen wrote: > > > One good example is in UefiPayloadPkg. > > > > > > 1. At entrypoint, the UefiPayload parses the ACPI table and build > > > gUefiAcpiBoardInfoGuid.https://github.com/tianocore/edk2/blob/mas > > > ter/UefiPayloadPkg/UefiPayloadEntry/AcpiTable.c#L23 > > > > > > 1. Later, AcpiTimer driver uses the ACPI data in > > > gUefiAcpiBoardInfoGuid.https://github.com/tianocore/edk2/blob/mas > > > ter/UefiPayloadPkg/Library/AcpiTimerLib/AcpiTimerLib.c#L49 > > > > > > 1. Other driver may also use the ACPI data, such as > > > PciExpressAddress. (from > > > MCFG).https://github.com/tianocore/edk2/blob/master/UefiPayloadPk > > > g/Library/PciSegmentInfoLibAcpiBoardInfo/PciSegmentInfoLibAcpiBoa > > > rdInfo.c#L55 > > > > > > > > > Thank you > > > Yao Jiewen > > > > > > > > > From: Boeuf, Sebastien > > > <sebastien.bo...@intel.com<mailto:sebastien.bo...@intel.com>> > > > Sent: Tuesday, September 6, 2022 11:41 PM > > > To: Yao, Jiewen <jiewen....@intel.com<mailto:jiewen....@intel.com>> > > > Cc: Justen, Jordan L > > > <jordan.l.jus...@intel.com<mailto:jordan.l.jus...@intel.com>>; > > > kra...@redhat.com<mailto:kra...@redhat.com>; > > > devel@edk2.groups.io<mailto:devel@edk2.groups.io> > > > Subject: Re: [PATCH v2] OvmfPkg: Update I/O port related to ACPI > > > devices for CloudHv > > > > > > Hi Jiewen, > > > > > > We patched Cloud Hypervisor to support both I/O ports for at least > > > two versions. And of course at some point users will have to rely > > > on > > > latest CloudHv binary (which we always build). > > > > > > One improvement could be to retrieve the I/O ports addresses from > > > the > > > FADT table. Do you know if there's some code already doing that in > > > OVMF? > > > > > > Thanks, > > > Sebastien > > > From: Yao, Jiewen <jiewen....@intel.com<mailto:jiewen....@intel.com>> > > > Sent: Tuesday, September 6, 2022 4:27 PM > > > To: Boeuf, Sebastien > > > <sebastien.bo...@intel.com<mailto:sebastien.bo...@intel.com>> > > > Cc: Justen, Jordan L > > > <jordan.l.jus...@intel.com<mailto:jordan.l.jus...@intel.com>>;kra...@redhat.com > > > <kra...@redhat.com<mailto:kra...@redhat.com>>;devel@edk2.groups.io > > > <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> > > > Subject: RE: [PATCH v2] OvmfPkg: Update I/O port related to ACPI > > > devices for CloudHv > > > > > > This seems a big incompatible change. > > > > > > I feel this is weird to hardcode the configuration here. > > > > > > How the OVMF binary knows it runs on a new CloudHv or old CloudHv? > > > > > > Can we have a mechanism to detect the data at runtime? E.g. read > > > some > > > PCI register ? > > > > > > > > > > > > > > > From: Boeuf, Sebastien > > > <sebastien.bo...@intel.com<mailto:sebastien.bo...@intel.com>> > > > Sent: Tuesday, September 6, 2022 10:08 PM > > > To: Yao, Jiewen <jiewen....@intel.com<mailto:jiewen....@intel.com>> > > > Cc: Justen, Jordan L > > > <jordan.l.jus...@intel.com<mailto:jordan.l.jus...@intel.com>>;kra...@redhat.com; > > > devel@edk2.groups.io<mailto:devel@edk2.groups.io> > > > Subject: Re: [PATCH v2] OvmfPkg: Update I/O port related to ACPI > > > devices for CloudHv > > > > > > Hi Jiewen, > > > > > > Do you think this could be merged? > > > > > > Thanks, > > > Sebastien > > > From: Boeuf, Sebastien > > > <sebastien.bo...@intel.com<mailto:sebastien.bo...@intel.com>> > > > Sent: Friday, August 19, 2022 11:59 AM > > > To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> > > > <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> > > > Cc: Yao, Jiewen <jiewen....@intel.com<mailto:jiewen....@intel.com>>; > > > Justen, Jordan L > > > <jordan.l.jus...@intel.com<mailto:jordan.l.jus...@intel.com>>;kra...@redhat.com > > > <kra...@redhat.com<mailto:kra...@redhat.com>>; > > > Boeuf, Sebastien > > > <sebastien.bo...@intel.com<mailto:sebastien.bo...@intel.com>> > > > Subject: [PATCH v2] OvmfPkg: Update I/O port related to ACPI > > > devices > > > for CloudHv > > > > > > From: Sebastien Boeuf > > > <sebastien.bo...@intel.com<mailto:sebastien.bo...@intel.com>> > > > > > > Both ACPI shutdown and ACPI PM timer devices has been moved to > > > different > > > port addresses in the latest version of Cloud Hypervisor. These > > > changes > > > need to be reflected on the OVMF firmware. > > > > > > Acked-by: Gerd Hoffmann <kra...@redhat.com<mailto:kra...@redhat.com>> > > > Signed-off-by: Sebastien Boeuf > > > <sebastien.bo...@intel.com<mailto:sebastien.bo...@intel.com>> > > > --- > > > OvmfPkg/Include/IndustryStandard/CloudHv.h | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/OvmfPkg/Include/IndustryStandard/CloudHv.h > > > b/OvmfPkg/Include/IndustryStandard/CloudHv.h > > > index d31ecc9eec..527c236f48 100644 > > > --- a/OvmfPkg/Include/IndustryStandard/CloudHv.h > > > +++ b/OvmfPkg/Include/IndustryStandard/CloudHv.h > > > @@ -16,12 +16,12 @@ > > > // > > > > > > // ACPI timer address > > > > > > // > > > > > > -#define CLOUDHV_ACPI_TIMER_IO_ADDRESS 0xb008 > > > > > > +#define CLOUDHV_ACPI_TIMER_IO_ADDRESS 0x0608 > > > > > > > > > > > > // > > > > > > // ACPI shutdown device address > > > > > > // > > > > > > -#define CLOUDHV_ACPI_SHUTDOWN_IO_ADDRESS 0x03c0 > > > > > > +#define CLOUDHV_ACPI_SHUTDOWN_IO_ADDRESS 0x0600 > > > > > > > > > > > > // > > > > > > // 32-bit MMIO memory hole base address > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#93472): https://edk2.groups.io/g/devel/message/93472 Mute This Topic: https://groups.io/mt/93121228/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
