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.144, mailfrom: anthony.perard@citrix.com) Received: from esa4.hc3370-68.iphmx.com (esa4.hc3370-68.iphmx.com [216.71.155.144]) by groups.io with SMTP; Tue, 23 Jul 2019 09:06:58 -0700 Authentication-Results: esa4.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none; spf=None smtp.pra=anthony.perard@citrix.com; spf=Pass smtp.mailfrom=anthony.perard@citrix.com; spf=None smtp.helo=postmaster@mail.citrix.com Received-SPF: None (esa4.hc3370-68.iphmx.com: no sender authenticity information available from domain of anthony.perard@citrix.com) identity=pra; client-ip=162.221.158.21; receiver=esa4.hc3370-68.iphmx.com; envelope-from="anthony.perard@citrix.com"; x-sender="anthony.perard@citrix.com"; x-conformance=sidf_compatible Received-SPF: Pass (esa4.hc3370-68.iphmx.com: domain of anthony.perard@citrix.com designates 162.221.158.21 as permitted sender) identity=mailfrom; client-ip=162.221.158.21; receiver=esa4.hc3370-68.iphmx.com; envelope-from="anthony.perard@citrix.com"; x-sender="anthony.perard@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 (esa4.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=esa4.hc3370-68.iphmx.com; envelope-from="anthony.perard@citrix.com"; x-sender="postmaster@mail.citrix.com"; x-conformance=sidf_compatible IronPort-SDR: V1jFv8jIlD/VJKv15wDVlubMHDY0rxS4PfnI6lh/v6sf+wyoFoo4vLkQZXSUsH7HeIyWz1M8mk tIvTwoA+kv+EwMvov2JhMz9u+3u+77wzc7cFQCTy7iUoRxkYdGrizW0qc8UFECY0o4tc0LeBuP jlAi3l19MiBm9lQrtjk4B0OKcU91EX/UcEaw2OXWhkelfk5fNxmmRc077DjOM10s8vaEIIXgGY /H9sULBIW2G7z0u50GCI6McSuR3FEZ4R6/qwQ0k58XrK3fwoTCjcHa8TO+gncBWIep6t5gAKQn uR8= X-SBRS: 2.7 X-MesageID: 3444515 X-Ironport-Server: esa4.hc3370-68.iphmx.com X-Remote-IP: 162.221.158.21 X-Policy: $RELAYED X-IronPort-AV: E=Sophos;i="5.64,299,1559534400"; d="scan'208";a="3444515" Date: Tue, 23 Jul 2019 17:06:38 +0100 From: "Anthony PERARD" To: Roger Pau =?iso-8859-1?Q?Monn=E9?= CC: , , Ard Biesheuvel , Jordan Justen , Laszlo Ersek , Julien Grall , "Andrew Cooper" Subject: Re: [PATCH v3 24/35] OvmfPkg/XenPlatformPei: Rework memory detection Message-ID: <20190723160638.GA1242@perard.uk.xensource.com> 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> MIME-Version: 1.0 In-Reply-To: <20190723094207.ccnzyzuma4ydpugi@Air-de-Roger> User-Agent: Mutt/1.12.1 (2019-06-15) Return-Path: anthony.perard@citrix.com Content-Type: text/plain; charset="iso-8859-1" Content-Disposition: inline Content-Transfer-Encoding: 8bit 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: > > > You could maybe initialize this as a global to avoid having to issue > > > a hypercall each time you need to get something from the memory map. > > > > That function does that, it only make the hypercall once. (The hypercall > > can only be made once anyway, the second time Xen doesn't return the > > map.) > > Why? I'm looking at the implementation in Xen of XENMEM_memory_map and > I'm not sure I see how/why the hypercall can only be made once. AFAICT > you should be able to call XENMEM_memory_map multiple times without > issues, or else it's a bug somewhere. :-(, I probably made a mistake when testing that. I tried again and calling the hypercall serveral time gave the same result. Sorry for the noise. > > > > +} > > > > > > > > UINT32 > > > > GetSystemMemorySizeBelow4gb ( > > > > @@ -105,6 +146,19 @@ GetSystemMemorySizeBelow4gb ( > > > > UINT8 Cmos0x34; > > > > UINT8 Cmos0x35; > > > > > > > > + // > > > > + // In PVH case, there is no CMOS, we have to calculate the memory size > > > > + // from parsing the E820 > > > > + // > > > > + if (XenPvhDetected ()) { > > > > > > IIRC on HVM you can also get the memory map from the hypercall, in > > > which case you could use the same code path for both HVM and PVH. > > > > I think that wouldn't work because in my experiment, the hypercall would > > only return the map the first time (at least on PVH). hvmloader already > > make the hypercall so OVMF can't. > > OK, I'm not sure the reason for this, as I said above I think this is > a bug somewhere. You should be able to call XENMEM_memory_map multiple > times. > > > On the other hand, XenGetE820Map() return an E820 map, it doesn't matter > > if it's the one passed by hvmloader, or the one we've got directly from > > Xen. So I guess we could ignore what hvmloader have written in the CMOS > > Hm, I'm not sure hvmloader uploads a new memory map to Xen (using > XENMEM_set_memory_map) if it does any modifications to it. It should > certainly do it, so that the guest OS gets the same memory map from > the hypercall or from the firmware. hvmloader doesn't call XENMEM_set_memory_map (I don't find that string in the source code), also, I've tested again calling the get memory_map hypercall in HVM guests and the e820 from hvmloader is different from the one from the hypercall: from hvmloader: Type Mem - 00000000 -> 000A0000 Type Res - 000F0000 -> 00100000 Type Mem - 00100000 -> 3F6B3000 Type Res - FC000000 -> 100000000 from Xen: Type Mem - 00100000 -> 3F800000 > > > > + switch (Entry->Type) { > > > > + case EfiAcpiAddressRangeMemory: > > > > + AddMemoryRangeHob (Base, End); > > > > + break; > > > > + case EfiAcpiAddressRangeACPI: > > > > + // > > > > + // Ignore, OVMF should read the ACPI tables and provide them to linux > > > > + // from a different location. > > > > > > Will OVMF also parse dynamic tables to check for references there? > > > > I haven't looked at what OVMF does with the ACPI tables, but Linux seems > > fine. I've compared the boot output of linux running as PVH vs booted > > via OVMF. Beside the location of the table been different, the number of > > table where the same, I don't remember other difference. > > OK, what I find weird is that you seem to discard quite a lot of stuff > from the original memory map, and then reconstruct it afterwards I > assume? > > It would seem safer to not discard regions from the memory map > provided to OVMF, and instead just build on top of it. I would expect OK, I'll add back the EfiAcpiAddressRangeACPI into the reserved regions. > for example that OVMF will use some of the RAM regions on the memory > map, and it should likely turn those areas from RAM into reserved > regions. > > > > > + // 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. It's not worse than before :), there's a lot more to cleanup :(. I've left an hvmloader specific call AddReservedMemoryBaseSizeHob (0xFC000000, 0x1000000, FALSE); which is outside of that XenPublishRamRegions() function. See: https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg00360.html I probably need to remove that now and see if I can simply use hvmloader's e820. -- Anthony PERARD