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.155.175, mailfrom: roger.pau@citrix.com) Received: from esa6.hc3370-68.iphmx.com (esa6.hc3370-68.iphmx.com [216.71.155.175]) by groups.io with SMTP; Thu, 25 Jul 2019 04:03:26 -0700 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 ~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: fyjYNnGPqj5nyRzSN9nQSD9nyNq78atAUuffswnMBsnPdGeuqiALL86XOmO+Bki3ZVkcOwpqqu rETvLk5h8/jBAG2ekZoQYzCll0gvCrjC0aO7FYmtG9p2IsOpjlWNGHlknjGEvoHn4L6n2cW3Xh PLBJ6BC5oy4eD89BdnJfVg92pzwhk4Eh2Ek2394MDD0v7Lr9bYW9q7TJJILhJWQe/EXgF+YHpd hg70c1729nruJxMrp3mbAf8qbnBVLFrw8Qb7zHeoJ3X9raJt7nS57Do1XgeoKGuBi8rI/O9G8d iVo= X-SBRS: 2.7 X-MesageID: 3525494 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.64,306,1559534400"; d="scan'208";a="3525494" Date: Thu, 25 Jul 2019 13:03:10 +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: <20190725110310.2egzbmy6kdpp5fep@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> <20190725090829.yiq2wpszksp5n2ab@Air-de-Roger> <20190725100534.GC1242@perard.uk.xensource.com> MIME-Version: 1.0 In-Reply-To: <20190725100534.GC1242@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 Thu, Jul 25, 2019 at 11:05:34AM +0100, Anthony PERARD wrote: > On Thu, Jul 25, 2019 at 11:08:29AM +0200, Roger Pau Monné wrote: > > 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. > > Ok. > > > > 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? > > You mean 1MB, right ? :-). D'oh, yes :). > I've try to find out in the Intel manual why > it would be 1MB and couldn't find that, but on the other hand the > initialisation code for OVMF running on QEMU does also reserve 1MB for > the local apic. So I'll change to 1MB. It's the Interrupt Address Range, which contains the mmio lapic registers and the mmio region where devices write in order to signal interrupts to the apic (used as the address field for MSI(-X)). Thanks, Roger.