From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f50.google.com (mail-wm1-f50.google.com [209.85.128.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1F6C130BB8A for ; Tue, 5 May 2026 08:43:39 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.50 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777970621; cv=none; b=YG4CCGcSj8MPf8q8hJTY9cVZ9mGGwtuhVqnOXHB7zNzszQRdMGhnxedYVn+SKjqHIJn0T0vg/nFrMT036jgBDsHH3NPbd6buNPVwt8R9brr+4aZY4r+IxcI90tgdKiXwPlLYsarFOLYXPjMYPc8aWKTp7MgdCHcN/eSiv0Kfwuo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777970621; c=relaxed/simple; bh=X8b8CRYor803YBDg9tNMJtvXmpGh4u88a+M2Dy9KEN0=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=uTMUX21NtoVfMNw379DMkh5hoOAdc9gPUXub2OTfyVceihOdPvdi/S/p8Juzpmk92GrC3EN2CNg1BZQFgTo8CVzjpm3TT1XKFH/4pOINiaoRim73OgLjhdAaTHXZEP2CceaV1RsMVASztPN6KH052DTaJTyqfItWCbjwzvhLxAs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com; spf=pass smtp.mailfrom=suse.com; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b=JdH9CQPd; arc=none smtp.client-ip=209.85.128.50 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=suse.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b="JdH9CQPd" Received: by mail-wm1-f50.google.com with SMTP id 5b1f17b1804b1-488e1a8ac40so47818335e9.2 for ; Tue, 05 May 2026 01:43:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1777970618; x=1778575418; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:autocrypt:from :content-language:references:cc:to:subject:user-agent:mime-version :date:message-id:from:to:cc:subject:date:message-id:reply-to; bh=0//ladv0CDpbweCjcqd+Fj+nnO8JuT9nkhnkBOafoa0=; b=JdH9CQPdWBTKYZsRpbnerDcb2Rjw7oJsF0RClYrhC5me7Xhj1ms66/fveFS2Ye4nGu 1LKCLhoefq5eYy2f7bkS3btDbRVdqns4Uohp0dHAOKq+IZ+jtzkbTrIH3f5t/KIg0mPC b321CgCVi8phiZx/v+/AAyOIvfz8BX/B5ptTfUvvaOOd4J2JjXPwTBZSCUpAmcUf6+ta jLJsC8HxMI47oljofOpyYiqHOIGOUVqmUGULS82d73hXwWqKL8E+psGNzDDRCNdq9sX6 Sy9qdxKQhhPtfM47HwSyIhL/hvxW4mR0764Ttv0uBJq5xZbZ1hFXANIiQeRLVMXUXIip PEDg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1777970618; x=1778575418; h=content-transfer-encoding:in-reply-to:autocrypt:from :content-language:references:cc:to:subject:user-agent:mime-version :date:message-id:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=0//ladv0CDpbweCjcqd+Fj+nnO8JuT9nkhnkBOafoa0=; b=e+XYMILcLTpMKiv2YPkDiLCzvI8lPpyFjMw1Jk+hJvTs07TGzrQPM+snYvJE95c9KB ixAonTWK2YF67BucEvCJ5qGKcPlDK/Qmn3+t8dKSaFdmP6KqGArjY3LxF00+6AKXTwwc tuQN63795O8MnpMA01mOuBFPTzLKymF7+lgHiLAGVeRIaiV8NGUGNiqAMfdfGE1UDWHe CS9rZs5I8Ssmx1Js8ICEekAtvggrJgPyaZtYK1173WmmU7Zw+4v7hyJh7b/wQVMKdi+B RLUatu0WZEGXPPoGwn4vfkC5h+tgJ3aoxJmIUg9rk5f0rAE6btvEoTC+bwnDWAe5cz2b v6jg== X-Forwarded-Encrypted: i=1; AFNElJ/p92noIegSe8871rQKjo7MzkcSyn5+oCnSFHBcJqGWyoWZtcygfg3fy791Wh8nduL/AwRC5vWPVkLpZsA=@vger.kernel.org X-Gm-Message-State: AOJu0Yw9sNWjaLaDoTKnJ7EGk0DEsMTryazLNsT8AH/qkrV1tW4N5ajL dKucfLRpHVmuFcwEwi6lPJWRCu5m2XmehJ/RVyznlqATDTgUSzhGFGxs9CXqIVP+Uw== X-Gm-Gg: AeBDieuIhfPzP6+Fbo+vxWfffO+JYlCYhCuZfGxkMG5Xt4YbUwYNRNRWOnYryZ4YAdU tTI9ewQDsbM6aMypZkzS0zRszugVCTVnU/dJXW83JtuOOpQSrKDQZKRuD/trS4+jDz3wFu8eDfM C+kFeFqp2FZpM5wzEEnWLrU1bxqVXx+xi8X3/hMxQ8RgJIsfG+3Ak81zy/1NqoDRx5Geej853ri Bv3WulkfOzCMZqCHqdI36pASopqyDg9A3BMrCVmXzqNoFSketJJQD5HQTaAxwE2fdfuXoU6QBtw OuDPdXQpIde5SBrMNKfytMR/rZ2vnpQQMygitGds4Ja2XVZBXU58ecNvD1sxGrj2hW/pTjUO9YX bF5s2s7xzlQ4U7FBViMq/LarexOYzcM/6iKj6/7BA2P4f/HoDHonk1veepyY/1BKtc8My93s+qS x6WzEoWBzGM944Qy+X99wPkRqJHcXgzA25KvM2ayKXA4JGPR1SayHJnajm7eDvHz3LzlSmBxK17 CDQAlvyTLkLhmlpCJF8P7yn+w== X-Received: by 2002:a05:600c:a30a:b0:485:30d4:6b9e with SMTP id 5b1f17b1804b1-48d18ce275fmr22165355e9.21.1777970618561; Tue, 05 May 2026 01:43:38 -0700 (PDT) Received: from [10.156.60.236] (ip-037-024-206-209.um08.pools.vodafone-ip.de. [37.24.206.209]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-48d1a98247fsm17639875e9.0.2026.05.05.01.43.37 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 05 May 2026 01:43:38 -0700 (PDT) Message-ID: <6b241236-ed6d-4ada-beac-8f660bd55185@suse.com> Date: Tue, 5 May 2026 10:43:37 +0200 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] x86/xen: Fix a potential problem in xen_e820_resolve_conflicts() To: Juergen Gross Cc: Boris Ostrovsky , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , "H. Peter Anvin" , xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org, x86@kernel.org References: <20260505080653.197775-1-jgross@suse.com> Content-Language: en-US From: Jan Beulich Autocrypt: addr=jbeulich@suse.com; keydata= xsDiBFk3nEQRBADAEaSw6zC/EJkiwGPXbWtPxl2xCdSoeepS07jW8UgcHNurfHvUzogEq5xk hu507c3BarVjyWCJOylMNR98Yd8VqD9UfmX0Hb8/BrA+Hl6/DB/eqGptrf4BSRwcZQM32aZK 7Pj2XbGWIUrZrd70x1eAP9QE3P79Y2oLrsCgbZJfEwCgvz9JjGmQqQkRiTVzlZVCJYcyGGsD /0tbFCzD2h20ahe8rC1gbb3K3qk+LpBtvjBu1RY9drYk0NymiGbJWZgab6t1jM7sk2vuf0Py O9Hf9XBmK0uE9IgMaiCpc32XV9oASz6UJebwkX+zF2jG5I1BfnO9g7KlotcA/v5ClMjgo6Gl MDY4HxoSRu3i1cqqSDtVlt+AOVBJBACrZcnHAUSuCXBPy0jOlBhxPqRWv6ND4c9PH1xjQ3NP nxJuMBS8rnNg22uyfAgmBKNLpLgAGVRMZGaGoJObGf72s6TeIqKJo/LtggAS9qAUiuKVnygo 3wjfkS9A3DRO+SpU7JqWdsveeIQyeyEJ/8PTowmSQLakF+3fote9ybzd880fSmFuIEJldWxp Y2ggPGpiZXVsaWNoQHN1c2UuY29tPsJgBBMRAgAgBQJZN5xEAhsDBgsJCAcDAgQVAggDBBYC AwECHgECF4AACgkQoDSui/t3IH4J+wCfQ5jHdEjCRHj23O/5ttg9r9OIruwAn3103WUITZee e7Sbg12UgcQ5lv7SzsFNBFk3nEQQCACCuTjCjFOUdi5Nm244F+78kLghRcin/awv+IrTcIWF hUpSs1Y91iQQ7KItirz5uwCPlwejSJDQJLIS+QtJHaXDXeV6NI0Uef1hP20+y8qydDiVkv6l IreXjTb7DvksRgJNvCkWtYnlS3mYvQ9NzS9PhyALWbXnH6sIJd2O9lKS1Mrfq+y0IXCP10eS FFGg+Av3IQeFatkJAyju0PPthyTqxSI4lZYuJVPknzgaeuJv/2NccrPvmeDg6Coe7ZIeQ8Yj t0ARxu2xytAkkLCel1Lz1WLmwLstV30g80nkgZf/wr+/BXJW/oIvRlonUkxv+IbBM3dX2OV8 AmRv1ySWPTP7AAMFB/9PQK/VtlNUJvg8GXj9ootzrteGfVZVVT4XBJkfwBcpC/XcPzldjv+3 HYudvpdNK3lLujXeA5fLOH+Z/G9WBc5pFVSMocI71I8bT8lIAzreg0WvkWg5V2WZsUMlnDL9 mpwIGFhlbM3gfDMs7MPMu8YQRFVdUvtSpaAs8OFfGQ0ia3LGZcjA6Ik2+xcqscEJzNH+qh8V m5jjp28yZgaqTaRbg3M/+MTbMpicpZuqF4rnB0AQD12/3BNWDR6bmh+EkYSMcEIpQmBM51qM EKYTQGybRCjpnKHGOxG0rfFY1085mBDZCH5Kx0cl0HVJuQKC+dV2ZY5AqjcKwAxpE75MLFkr wkkEGBECAAkFAlk3nEQCGwwACgkQoDSui/t3IH7nnwCfcJWUDUFKdCsBH/E5d+0ZnMQi+G0A nAuWpQkjM1ASeQwSHEeAWPgskBQL In-Reply-To: <20260505080653.197775-1-jgross@suse.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 05.05.2026 10:06, Juergen Gross wrote: > When fixing a conflict in xen_e820_resolve_conflicts(), the loop over > the E820 map entries needs to be restarted, as the E820 map will have > been modified by the fix. Otherwise entries might be skipped by > accident. > > Fixes: be35d91c8880 ("xen: tolerate ACPI NVS memory overlapping with Xen allocated memory") > Signed-off-by: Juergen Gross First, while trying to review this, isn't there another issue in xen_e820_swap_entry_with_ram(), in that entry->addr = entry_end - swap_size + swap_addr - swap_entry->addr; really means to be entry->addr = entry_end - swap_size + swap_entry->addr - swap_addr; (affecting non-page-aligned E820 entries)? Further, that function converts swap_entry to the page-aligned superset of the passed in range. How is it guaranteed that this new range won't overlap with the predecessor and/or successor one? Wouldn't that need to be conversion to the page-aligned subset instead? And then, is passing the page-aligned superset to xen_add_remap_nonram() really appropriate? Why would any leading or trailing space there be subject to remapping? > --- a/arch/x86/xen/setup.c > +++ b/arch/x86/xen/setup.c > @@ -695,17 +695,22 @@ static void __init xen_e820_resolve_conflicts(phys_addr_t start, > return; > > end = start + size; > - entry = xen_e820_table.entries; > + mapcnt = 0; > > - for (mapcnt = 0; mapcnt < xen_e820_table.nr_entries; mapcnt++) { > + while (mapcnt < xen_e820_table.nr_entries) { > + entry = xen_e820_table.entries + mapcnt; > if (entry->addr >= end) > return; > > if (entry->addr + entry->size > start && > - entry->type == E820_TYPE_NVS) > + entry->type == E820_TYPE_NVS) { > xen_e820_swap_entry_with_ram(entry); > + /* E820 map has been changed, restart loop! */ > + mapcnt = 0; > + continue; > + } > > - entry++; > + mapcnt++; > } > } Given what exactly xen_e820_swap_entry_with_ram() does, restarting from entry 0 looks to be needed only if the non-RAM entry ended up moving down (strictly speaking even there it wouldn't need to be entry 0). If it moved up, simply not incrementing mapcnt would look to suffice. Since the extra overhead is likely tolerable here (with simplicity of the code being more important), this may want mentioning in a code comment (or at least the description). Preferably with that: Reviewed-by: Jan Beulich Jan