From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.120]) by mx.groups.io with SMTP id smtpd.web10.5836.1587110856206372088 for ; Fri, 17 Apr 2020 01:07:36 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=dxKCQFUI; spf=pass (domain: redhat.com, ip: 205.139.110.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1587110855; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=s+6mb9RG5LV+mNfRKz2UGpdHv0BAoHl4KOjmUWIsiq4=; b=dxKCQFUILZj31OSWTdvZQq50IGWTJVP7rQN4lR5FnGFjNRvvJ2s0RekebrfmuqfTuRTfik jjmf3ZoxLwYJIuIogzsAk/btMsA8gYQ7Z8EgfZuxPSBdVsApw9LlFSc5MPzE9ingf3DU4R RC0cPfybrGiUVGBqriKFgmGy5fM6XxY= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-144-JMAhcc6-NK-OeHNV1WQO7g-1; Fri, 17 Apr 2020 04:07:31 -0400 X-MC-Unique: JMAhcc6-NK-OeHNV1WQO7g-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id CA8541005513; Fri, 17 Apr 2020 08:07:29 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-253.ams2.redhat.com [10.36.113.253]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8C64D60BE0; Fri, 17 Apr 2020 08:07:28 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH 01/13] OvmfPkg: Add bhyve support into AcpiTimerLib To: devel@edk2.groups.io, rebecca@bsdio.com Cc: Jordan Justen , Ard Biesheuvel References: <0b1d5644e175225839828e9a919152aff89491ba.1586991816.git.rebecca@bsdio.com> From: "Laszlo Ersek" Message-ID: <27750956-3b7d-5ef1-5a90-5f294bcb3cf8@redhat.com> Date: Fri, 17 Apr 2020 10:07:27 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <0b1d5644e175225839828e9a919152aff89491ba.1586991816.git.rebecca@bsdio.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 04/16/20 01:09, Rebecca Cran wrote: > On bhyve, the ACPI timer is located at a fixed IO address; it need > not be programmed into, nor fetched from, the PMBA -- power > management base address -- register of the PCI host bridge. > > Signed-off-by: Rebecca Cran > --- > OvmfPkg/Include/IndustryStandard/Bhyve.h | 21 ++++++++++++ > OvmfPkg/Include/OvmfPlatforms.h | 1 + > .../AcpiTimerLib/BaseAcpiTimerLibBhyve.c | 33 +++++++++++++++++++ > .../AcpiTimerLib/BaseAcpiTimerLibBhyve.inf | 29 ++++++++++++++++ > 4 files changed, 84 insertions(+) > create mode 100644 OvmfPkg/Include/IndustryStandard/Bhyve.h > create mode 100644 OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLibBhyve.c > create mode 100644 OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLibBhyve.inf > > diff --git a/OvmfPkg/Include/IndustryStandard/Bhyve.h b/OvmfPkg/Include/IndustryStandard/Bhyve.h > new file mode 100644 > index 0000000000..950f87048b > --- /dev/null > +++ b/OvmfPkg/Include/IndustryStandard/Bhyve.h > @@ -0,0 +1,21 @@ > +/** @file > + Various register numbers and value bits based on FreeBSD's bhyve > + at r359530. > + - https://svnweb.freebsd.org/base?view=revision&revision=359530 > + > + Copyright (C) 2020, Rebecca Cran > + > + SPDX-License-Identifier: BSD-2-Clause-Patent > +**/ > + > +#ifndef __BHYVE_H__ > +#define __BHYVE_H__ > + > +#include > +#include > +#include > +#include (1) These #include directives look unnecessary. > + > +#define BHYVE_ACPI_TIMER_IO_ADDR 0x408 > + > +#endif // __BHYVE_H__ > diff --git a/OvmfPkg/Include/OvmfPlatforms.h b/OvmfPkg/Include/OvmfPlatforms.h > index 59459231e8..77dd818e30 100644 > --- a/OvmfPkg/Include/OvmfPlatforms.h > +++ b/OvmfPkg/Include/OvmfPlatforms.h > @@ -14,6 +14,7 @@ > #include > #include > #include > +#include > > // > // OVMF Host Bridge DID Address > diff --git a/OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLibBhyve.c b/OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLibBhyve.c > new file mode 100644 > index 0000000000..58ebffac86 > --- /dev/null > +++ b/OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLibBhyve.c > @@ -0,0 +1,33 @@ > +/** @file > + Provide InternalAcpiGetTimerTick for the bhyve instance of the > + Base ACPI Timer Library > + > + Copyright (C) 2020, Rebecca Cran > + Copyright (C) 2014, Gabriel L. Somlo > + > + SPDX-License-Identifier: BSD-2-Clause-Patent > +**/ > + > +#include (2) We don't use DebugLib here, so please remove this #include. (... in retrospect, I think the same #include should be removed from the preexistent C source too, in this directory, but that's for another day.) > +#include > +#include > + > +/** > + Internal function to read the current tick counter of ACPI. > + > + Read the current ACPI tick counter using the counter address cached > + by this instance's constructor. > + > + @return The tick counter read. > + > +**/ > +UINT32 > +InternalAcpiGetTimerTick ( > + VOID > + ) > +{ > + // > + // Return the current ACPI timer value. (3) Please replace "// Return" with "// Return". > + // > + return IoRead32 (BHYVE_ACPI_TIMER_IO_ADDR); > +} > diff --git a/OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLibBhyve.inf b/OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLibBhyve.inf > new file mode 100644 > index 0000000000..1e48718f3d > --- /dev/null > +++ b/OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLibBhyve.inf > @@ -0,0 +1,29 @@ > +## @file > +# Base ACPI Timer Library Instance for Bhyve. > +# > +# Copyright (C) 2020, Rebecca Cran > +# Copyright (C) 2014, Gabriel L. Somlo > +# Copyright (c) 2008 - 2010, Intel Corporation. All rights reserved. > +# > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +## > + > +[Defines] > + INF_VERSION = 0x00010005 > + BASE_NAME = BaseAcpiTimerLibBhyve > + FILE_GUID = A5E3B247-7302-11EA-9C04-3CECEF0C1C08 > + MODULE_TYPE = BASE > + VERSION_STRING = 1.0 > + LIBRARY_CLASS = TimerLib > + > +[Sources] > + AcpiTimerLib.c > + BaseAcpiTimerLibBhyve.c (4) "AcpiTimerLib.h" should be listed here, as that is the file that declares the InternalAcpiGetTimerTick() function. > + > +[Packages] > + MdePkg/MdePkg.dec > + OvmfPkg/OvmfPkg.dec (5) I think you can drop "OvmfPkg/OvmfPkg.dec", as you don't seem to be consuming any interface (such as guid, protocol, pcd, lib class, ...) that's declared in "OvmfPkg.dec". > + > +[LibraryClasses] > + IoLib > With (1)-(5) fixed/considered: Reviewed-by: Laszlo Ersek Thanks! Laszlo