[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] x86/hvm/dom0: fix PVH initrd and metadata placement


  • To: Xenia Ragiadakou <xenia.ragiadakou@xxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Mon, 30 Oct 2023 13:12:21 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=DHxJBS9g4VPvxUGnG/f4KLJN1mHF0ajstqPxa/WEU30=; b=a7yRk26TXDCsCB6KppsIhvSjRiRgwVR7zgg+rOx+Xh3qotQ540R3xb5h0x3UucBKXgBtlvzvXomIgN0iqsaTiRzoOm22JeXG7UV94ITf4r3dFx5vQ9QboyZ2Zjdq0nnzCxhTMh+KPqiS04vsMJUTmcCNHpic5oheRRAI7qHOKmUxhngZ9Rgi8Z3HcwL73bpBR/5BSIgwTMSDRuLhnLgNWFUBGVT0nLgfBhIJEmkhp6Hu2FQTLOIK6Xecf+kjnKHTWmr1aKmrMSq00Af9ZAuMxzORRARm7tx0aAOQ4eEecCQuAR/dlKhSlZWaH1I5iyoYpEhXsnQ3QLLxFr5Se/TIdg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=UQUzbq941PQiyaaw/9yRR/NICJ8+B1/68fAklNEb/K3JOXlMdLTtgZxMTvbfhBOdYL+hkAWWy8pChaqt+VhB7vDS3xmS61jNLSL5gWL6zjHXCeu4HEEdUrh5EFrUPUfgTYlWJAsjVGjzXZva34120OB6axwVlEs+iVVJcGot9gd8fZAiGSrdqKRvXY+OuB7IG3Z+gGNEIkRFH0NK3bKP8GjFZdwfTaSn4mOKTcdYikJxrKA1VYw/gV/M8Dxsaadgj4QpH7K5Efn8bnhtiqt3sonqY4fqj8eGkBwDNmyshcO0v3O9vmX9hICKptjnolVhyywhcRo1w7K78r65olPcFw==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx, Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Delivery-date: Mon, 30 Oct 2023 12:12:35 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 30.10.2023 08:37, Xenia Ragiadakou wrote:
> Jan would it be possible to sketch a patch of your suggested solution 
> because I 'm afraid I have not fully understood it yet and I won't be 
> able to implement it properly for a v2?

While what Roger sent looks to be sufficient, I still thought I'd send
my variant, which I think yields more overall consistent results. Like
Roger's this isn't really tested (beyond making sure it builds).

Jan

From: Xenia Ragiadakou <xenia.ragiadakou@xxxxxxx>
Subject: x86/hvm/dom0: fix PVH initrd and metadata placement

Given that start < kernel_end and end > kernel_start, the logic that
determines the best placement for dom0 initrd and metadata, does not
take into account the two cases below:
(1) start > kernel_start && end > kernel_end
(2) start < kernel_start && end < kernel_end

In case (1), the evaluation will result in end = kernel_start
i.e. end < start, and will load initrd in the middle of the kernel.
In case (2), the evaluation will result in start = kernel_end
i.e. end < start, and will load initrd at kernel_end, that is out
of the memory region under evaluation.

This patch reorganizes the conditionals to include so far unconsidered
cases as well, uniformly returning the lowest available address.

Fixes: 73b47eea2104 ('x86/dom0: improve PVH initrd and metadata placement')
Signed-off-by: Xenia Ragiadakou <xenia.ragiadakou@xxxxxxx>
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
Contrary to my original intentions, with the function preferring lower
addresses (by walking the E820 table forwards), the new cases also
return lowest-possible addresses.
---
v2: Cover further cases of overlap.

--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -515,16 +515,23 @@ static paddr_t __init find_memory(
 
         ASSERT(IS_ALIGNED(start, PAGE_SIZE) && IS_ALIGNED(end, PAGE_SIZE));
 
+        /*
+         * NB: Even better would be to use rangesets to determine a suitable
+         * range, in particular in case a kernel requests multiple heavily
+         * discontiguous regions (which right now we fold all into one big
+         * region).
+         */
         if ( end <= kernel_start || start >= kernel_end )
-            ; /* No overlap, nothing to do. */
+        {
+            /* No overlap, just check whether the region is large enough. */
+            if ( end - start >= size )
+                return start;
+        }
         /* Deal with the kernel already being loaded in the region. */
-        else if ( kernel_start - start > end - kernel_end )
-            end = kernel_start;
-        else
-            start = kernel_end;
-
-        if ( end - start >= size )
+        else if ( kernel_start > start && kernel_start - start >= size )
             return start;
+        else if ( kernel_end < end && end - kernel_end >= size )
+            return kernel_end;
     }
 
     return INVALID_PADDR;




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.