From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from esa6.hc3370-68.iphmx.com (esa6.hc3370-68.iphmx.com [216.71.155.175]) by mx.groups.io with SMTP id smtpd.web12.5364.1580375581333437608 for ; Thu, 30 Jan 2020 01:13:01 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@citrix.com header.s=securemail header.b=f0+OBKRs; spf=pass (domain: citrix.com, ip: 216.71.155.175, mailfrom: roger.pau@citrix.com) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=citrix.com; s=securemail; t=1580375581; h=date:from:to:cc:subject:message-id:references: mime-version:content-transfer-encoding:in-reply-to; bh=RedrHVl6xY/1L3lp/8+AlGn8Ra5bwj+zw4X1+RiuZlU=; b=f0+OBKRshBgntmy8do2HuoKKXmF+UtQc/lGW2P3UPuU1XrYqREh7b0oJ ahUSIEqUEOgf7jrwk6Beq/Mx0LkfK7M7HNj5gwxZuCiVh7T35pvnhM0rC cOtO2yMm86JGtsGyh9oHu65VvCF9VPH/6I5iQfnodVD10rQHN4f1dVypr o=; Authentication-Results: esa6.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none; spf=None smtp.pra=roger.pau@citrix.com; spf=Pass smtp.mailfrom=roger.pau@citrix.com; spf=None smtp.helo=postmaster@mail.citrix.com Received-SPF: None (esa6.hc3370-68.iphmx.com: no sender authenticity information available from domain of roger.pau@citrix.com) identity=pra; client-ip=162.221.158.21; receiver=esa6.hc3370-68.iphmx.com; envelope-from="roger.pau@citrix.com"; x-sender="roger.pau@citrix.com"; x-conformance=sidf_compatible Received-SPF: Pass (esa6.hc3370-68.iphmx.com: domain of roger.pau@citrix.com designates 162.221.158.21 as permitted sender) identity=mailfrom; client-ip=162.221.158.21; receiver=esa6.hc3370-68.iphmx.com; envelope-from="roger.pau@citrix.com"; x-sender="roger.pau@citrix.com"; x-conformance=sidf_compatible; x-record-type="v=spf1"; x-record-text="v=spf1 ip4:209.167.231.154 ip4:178.63.86.133 ip4:195.66.111.40/30 ip4:85.115.9.32/28 ip4:199.102.83.4 ip4:192.28.146.160 ip4:192.28.146.107 ip4:216.52.6.88 ip4:216.52.6.188 ip4:162.221.158.21 ip4:162.221.156.83 ip4:168.245.78.127 ~all" Received-SPF: None (esa6.hc3370-68.iphmx.com: no sender authenticity information available from domain of postmaster@mail.citrix.com) identity=helo; client-ip=162.221.158.21; receiver=esa6.hc3370-68.iphmx.com; envelope-from="roger.pau@citrix.com"; x-sender="postmaster@mail.citrix.com"; x-conformance=sidf_compatible IronPort-SDR: ZkQXAVoYTybVjPYrRBMcEBIH80qnhd6Ff2EFJdeI08TcnonJxnhlP7yUZiKQAxJjHpe6h+UYE/ dGFnhizCs06246vuScn4FrcivXsqWqq+xlH+fK/fChlPzz1Xu7ax1xdShk9ZmEvS0bIka74D86 nco3upEdyqxmipp7hPgOALhz84wGABPZwAKzL4T5CYvtGph8Iciqt4Iv+r7ZlXSxPzZuxkem6f WFURiIwexM8lEVPmEgt3rJk07m03aN2qWHmvQha0xrJAWy4c0sRqj+rHEMy1xYYbRjELi5z9eg LAc= X-SBRS: 2.7 X-MesageID: 12103615 X-Ironport-Server: esa6.hc3370-68.iphmx.com X-Remote-IP: 162.221.158.21 X-Policy: $RELAYED X-IronPort-AV: E=Sophos;i="5.70,381,1574139600"; d="scan'208";a="12103615" Date: Thu, 30 Jan 2020 10:12:51 +0100 From: =?UTF-8?B?Um9nZXIgUGF1IE1vbm7DqQ==?= To: Anthony PERARD CC: , Michael D Kinney , "Ard Biesheuvel" , , Laszlo Ersek , Liming Gao , "Jordan Justen" , Julien Grall Subject: Re: [PATCH 4/5] OvmfPkg/XenPlatformPei: Calibrate APIC timer frequency Message-ID: <20200130091251.GB4679@Air-de-Roger> References: <20200129121235.1814563-1-anthony.perard@citrix.com> <20200129121235.1814563-5-anthony.perard@citrix.com> MIME-Version: 1.0 In-Reply-To: <20200129121235.1814563-5-anthony.perard@citrix.com> Return-Path: roger.pau@citrix.com X-ClientProxiedBy: AMSPEX02CAS02.citrite.net (10.69.22.113) To AMSPEX02CL01.citrite.net (10.69.22.125) Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit On Wed, Jan 29, 2020 at 12:12:34PM +0000, Anthony PERARD wrote: > Calculate the frequency of the APIC timer that Xen provides. > > Even though the frequency is currently hard-coded, it isn't part of > the public ABI that Xen provides and thus may change at any time. OVMF > needs to determine the frequency by an other mean. > > Fortunately, Xen provides a way to determines the frequency of the > TSC, so we can use TSC to calibrate the frequency of the APIC timer. > That information is found in the shared_info page which we map and > unmap once done (XenBusDxe is going to map the page somewhere else). > > The calculated frequency is only logged in this patch, it will be used > in a following patch. > > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2490 > Signed-off-by: Anthony PERARD Thanks! Some comments below on the implementation. > --- > CC: Roger Pau Monné > --- > OvmfPkg/XenPlatformPei/XenPlatformPei.inf | 1 + > OvmfPkg/XenPlatformPei/Platform.h | 5 + > OvmfPkg/XenPlatformPei/Platform.c | 1 + > OvmfPkg/XenPlatformPei/Xen.c | 123 ++++++++++++++++++++++ > 4 files changed, 130 insertions(+) > > diff --git a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf > index 0ef77db92c03..335a442538c2 100644 > --- a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf > +++ b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf > @@ -52,6 +52,7 @@ [LibraryClasses] > DebugLib > HobLib > IoLib > + LocalApicLib > PciLib > ResourcePublicationLib > PeiServicesLib > diff --git a/OvmfPkg/XenPlatformPei/Platform.h b/OvmfPkg/XenPlatformPei/Platform.h > index 7661f4a8de0a..97e482a065f0 100644 > --- a/OvmfPkg/XenPlatformPei/Platform.h > +++ b/OvmfPkg/XenPlatformPei/Platform.h > @@ -127,6 +127,11 @@ XenGetE820Map ( > UINT32 *Count > ); > > +VOID > +CalibrateLapicTimer ( > + VOID > + ); > + > extern EFI_BOOT_MODE mBootMode; > > extern UINT8 mPhysMemAddressWidth; > diff --git a/OvmfPkg/XenPlatformPei/Platform.c b/OvmfPkg/XenPlatformPei/Platform.c > index 717fd0ab1a45..e9511eb40c62 100644 > --- a/OvmfPkg/XenPlatformPei/Platform.c > +++ b/OvmfPkg/XenPlatformPei/Platform.c > @@ -448,6 +448,7 @@ InitializeXenPlatform ( > InitializeRamRegions (); > > InitializeXen (); > + CalibrateLapicTimer (); > > if (mBootMode != BOOT_ON_S3_RESUME) { > ReserveEmuVariableNvStore (); > diff --git a/OvmfPkg/XenPlatformPei/Xen.c b/OvmfPkg/XenPlatformPei/Xen.c > index c41fecdc486e..d6cdc9a8e31c 100644 > --- a/OvmfPkg/XenPlatformPei/Xen.c > +++ b/OvmfPkg/XenPlatformPei/Xen.c > @@ -19,6 +19,7 @@ > // > #include > #include > +#include > #include > #include > #include > @@ -386,3 +387,125 @@ InitializeXen ( > > return EFI_SUCCESS; > } > + > + > +EFI_STATUS > +MapSharedInfoPage ( > + IN VOID *PagePtr > + ) > +{ > + xen_add_to_physmap_t Parameters; > + INTN ReturnCode; > + > + Parameters.domid = DOMID_SELF; > + Parameters.space = XENMAPSPACE_shared_info; > + Parameters.idx = 0; > + Parameters.gpfn = (UINTN) PagePtr >> EFI_PAGE_SHIFT; > + ReturnCode = XenHypercallMemoryOp (XENMEM_add_to_physmap, &Parameters); > + if (ReturnCode != 0) { > + return EFI_NO_MAPPING; > + } > + return EFI_SUCCESS; > +} > + > +VOID > +UnmapXenPage ( > + IN VOID *PagePtr > + ) > +{ > + xen_remove_from_physmap_t Parameters; > + INTN ReturnCode; > + > + Parameters.domid = DOMID_SELF; > + Parameters.gpfn = (UINTN) PagePtr >> EFI_PAGE_SHIFT; > + ReturnCode = XenHypercallMemoryOp (XENMEM_remove_from_physmap, &Parameters); I'm afraid this will leave a hole in the p2m, and hence freeing the page back to OVMF is wrong (I assume this is what FreePages does in CalibrateLapicTimer), as the physical address would be unpopulated after the call to XENMEM_remove_from_physmap. > + ASSERT (ReturnCode == 0); > +} > + > + > +STATIC > +UINT64 > +GetCPUFreq ( > + IN XEN_VCPU_TIME_INFO *VcpuTime > + ) > +{ > + UINT32 Version; > + UINT32 TSCToSystemMultiplier; > + INT8 TSCShift; > + UINT64 CPUFreq; > + > + do { > + Version = VcpuTime->Version; > + MemoryFence (); > + TSCToSystemMultiplier = VcpuTime->TSCToSystemMultiplier; > + TSCShift = VcpuTime->TSCShift; > + MemoryFence (); > + } while (((Version & 1) != 0) && (Version != VcpuTime->Version)); > + > + CPUFreq = (1000000000ULL << 32) / TSCToSystemMultiplier; > + if (TSCShift >= 0) { > + CPUFreq >>= VcpuTime->TSCShift; > + } else { > + CPUFreq <<= -VcpuTime->TSCShift; > + } > + return CPUFreq; > +} > + > +VOID > +XenDelay ( > + IN XEN_VCPU_TIME_INFO *VcpuTimeInfo, > + IN UINT64 DelayNs > + ) > +{ > + UINT64 Tick; > + > + Tick = AsmReadTsc (); > + Tick += (DelayNs * GetCPUFreq (VcpuTimeInfo)) / 1000000000ULL; > + while (AsmReadTsc() <= Tick) { > + CpuPause(); > + } > +} > + > + > +/** > + Calculate the frequency of the Local Apic Timer > +**/ > +VOID > +CalibrateLapicTimer ( > + VOID > + ) > +{ > + XEN_SHARED_INFO *SharedInfo; > + XEN_VCPU_TIME_INFO *VcpuTimeInfo; > + UINT32 TimerTick, TimerTick2; > + UINT64 TscTick, TscTick2; > + UINT64 Freq; > + EFI_STATUS Status; > + > + SharedInfo = AllocatePages (1); Hm, it's not the best approach to use a regular memory page to map the shared info: mapping it is very likely to cause superpage shattering, and once unmapped it leaves a hole in the p2m. As a reference you could map the shared info page at maximum physical address allowed (after checking it's not populated) like Wei is doing for the Xen on HyperV work. Roger.