From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: citrix.com, ip: 216.71.145.153, mailfrom: roger.pau@citrix.com) Received: from esa2.hc3370-68.iphmx.com (esa2.hc3370-68.iphmx.com [216.71.145.153]) by groups.io with SMTP; Thu, 25 Jul 2019 02:08:43 -0700 Authentication-Results: esa2.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 (esa2.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=esa2.hc3370-68.iphmx.com; envelope-from="roger.pau@citrix.com"; x-sender="roger.pau@citrix.com"; x-conformance=sidf_compatible Received-SPF: Pass (esa2.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=esa2.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 ~all" Received-SPF: None (esa2.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=esa2.hc3370-68.iphmx.com; envelope-from="roger.pau@citrix.com"; x-sender="postmaster@mail.citrix.com"; x-conformance=sidf_compatible IronPort-SDR: i6YADqt/X5Z7NpyR1p8yyv0X/fN3HmCV8sGRf3ScfS944ZcMYpOFnlYwrzfOhW2+eBx7/ilfM7 DSp2c0lpbvUEL42b76X2qnwkPFP/rJTNHGVrtnwYCjbVLevtua5uFmFw8vuh51rbnJb5UmyAKa EaKijstuW02fde+YOLmq7r9QS6tekW8sZeH/nxnQuJg+6Zan0TaE4qYKjUE2pS8BZuP4hFJYBo ZdmhrxZ8uzXf5iXcs8mWlbHPRb2spQKRxZBywt0Sg+jP6IrekYVpXgHibvxN4jfXrziXHfcOny ZpQ= X-SBRS: 2.7 X-MesageID: 3410189 X-Ironport-Server: esa2.hc3370-68.iphmx.com X-Remote-IP: 162.221.158.21 X-Policy: $RELAYED X-IronPort-AV: E=Sophos;i="5.64,306,1559534400"; d="scan'208";a="3410189" Date: Thu, 25 Jul 2019 11:08:29 +0200 From: =?UTF-8?B?Um9nZXIgUGF1IE1vbm7DqQ==?= To: Anthony PERARD CC: , , Ard Biesheuvel , Jordan Justen , Laszlo Ersek , Julien Grall , "Andrew Cooper" Subject: Re: [PATCH v3 24/35] OvmfPkg/XenPlatformPei: Rework memory detection Message-ID: <20190725090829.yiq2wpszksp5n2ab@Air-de-Roger> References: <20190704144233.27968-1-anthony.perard@citrix.com> <20190704144233.27968-25-anthony.perard@citrix.com> <20190715141521.aqmpchgzyleoergc@MacBook-Air-de-Roger.local> <20190722145319.GG1208@perard.uk.xensource.com> <20190723094207.ccnzyzuma4ydpugi@Air-de-Roger> <20190724161759.GB1242@perard.uk.xensource.com> MIME-Version: 1.0 In-Reply-To: <20190724161759.GB1242@perard.uk.xensource.com> User-Agent: NeoMutt/20180716 Return-Path: roger.pau@citrix.com X-ClientProxiedBy: AMSPEX02CAS02.citrite.net (10.69.22.113) To AMSPEX02CL02.citrite.net (10.69.22.126) Content-Type: text/plain; charset="iso-8859-1" Content-Disposition: inline Content-Transfer-Encoding: 8bit On Wed, Jul 24, 2019 at 05:17:59PM +0100, Anthony PERARD wrote: > On Tue, Jul 23, 2019 at 11:42:07AM +0200, Roger Pau Monné wrote: > > On Mon, Jul 22, 2019 at 03:53:19PM +0100, Anthony PERARD wrote: > > > On Mon, Jul 15, 2019 at 04:15:21PM +0200, Roger Pau Monné wrote: > > > > On Thu, Jul 04, 2019 at 03:42:22PM +0100, Anthony PERARD wrote: > > > > > + // error message: CpuDxe: IntersectMemoryDescriptor: > > > > > + // desc [FC000000, 100000000) type 1 cap 8700000000026001 > > > > > + // conflicts with aperture [FEE00000, FEE01000) cap 1 > > > > > // > > > > > - if (Entry->Type != EfiAcpiAddressRangeMemory) { > > > > > - continue; > > > > > + if (!XenHvmloaderDetected ()) { > > > > > + AddReservedMemoryBaseSizeHob (Base, End - Base, FALSE); > > > > > > > > This special casing for PVH looks weird, ideally we would like to use > > > > the same code path, or else it should be explicitly mentioned why PVH > > > > has diverging behaviour. > > > > > > I think hvmloader is the issue rather than PVH. Here is part of the > > > "memory map" as found in hvmloader/config.h: > > > > > > /* Special BIOS mappings, etc. are allocated from here upwards... */ > > > #define RESERVED_MEMBASE 0xFC000000 > > > /* NB. ACPI_INFO_PHYSICAL_ADDRESS *MUST* match definition in acpi/dsdt.asl! */ > > > #define ACPI_INFO_PHYSICAL_ADDRESS 0xFC000000 > > > #define RESERVED_MEMORY_DYNAMIC_START 0xFC001000 > > > #define RESERVED_MEMORY_DYNAMIC_END 0xFE000000 > > > > > > and hvmloader simply creates a single e820 reserved entry, from > > > RESERVED_MEMBASE to the top of 4GB. It's probably too much. > > > > But isn't this kind of dangerous? How can you assure future versions > > of hvmloader won't use this space? > > > > > If hvmloader only reserved > > > ACPI_INFO_PHYSICAL_ADDRESS-RESERVED_MEMORY_DYNAMIC_END, I might not have > > > to special case hvmloader. > > > > Could we look into getting this fixed in hvmloader then? > > > > I think it's dangerous for OVMF to play such tricks with the memory > > map. > > > > > As far as I know 0xfee00000 isn't a special > > > bios mapping, but something the hardware provides. > > > > Yes, that's used by the lapic, so it's not specific to hvmloader. > > Right, I've got a closer look at that CpuDxe module, it wants the local > APIC memory mapped space to be "mapped IO", and that different than > "reserved". > > So while parsing the e820 from hvmloader, instead of ignoring all > reserved region, I'm going to avoid adding the local apic memory mapped > space. > > something like: > if (hvmloaderDetected()) I don't think you need to gate this on hvmloader being used, while it's true that PVH memory map doesn't contain such reserved memory region ATM I don't see any harm in doing this for PVH also. > Base = $(start of the e820 entry); > End = $(start of the e820 entry + size); > LocalApic = 0xfee00000; > if (Base < LocalApic && LocalApic < End) { > AddReservedMemoryRangeHob (Base, LocalApic, FALSE); > if (End > (LocalApic + SIZE_4KB)) { The range is actually from 0xfee00000 to 0xfeefffff (2MB), so you likely want to make sure non of this is added as reserved? Thanks, Roger.