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

Re: [PATCH 09/12] x86/boot: add start and size fields to struct boot_module


  • To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, "Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Jason Andryuk <jason.andryuk@xxxxxxx>
  • Date: Sun, 10 Nov 2024 19:41:36 -0500
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=citrix.com smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none (0)
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; 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=ABJUv5E9o58HhKmF0WeZnKV+R/ZPY+Ymo+vs/dthYmI=; b=Xkf9ukrnK3kBh6rvU8tKl7Do7zE0xZtrMZhAMCUu25Sx1++9RBZWRAhgfo1lD1q7Xrk1m1cwne1PV1qnWOMQ9g5jmKc1tJFVNMV+FVRtOHl3+DoftV5T23jqs+ttOunAakCrZBO/V6WWXoAy5BW4E/oDQn1KY3CAMlwRvNhT+F+VzRAelP1Cje7sdk+Fuw+qJ33ajwsjZ2fSWQockfzgN/DO2YdLdGIb2p1dUkgZbgLYzZvPvbt9ke1AFwSO6Lzhu4hMY971AdKmBnKa+q/0/BckvVK+e4WvleZbtcvaKt5BMnuG5laEgfLn71gtM9nH3U0Yjvs9boPF5Evr6zOrQA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=HUdlEl4wR+DX8e/bGLQUwK7cT5baBT2Vlw6DR8dMjNvZvfU41mYsFAsOXwR3knCOyx55voKYsVg5Dq5/auoPD92lxdO8ludI9JG9XXJjWGdja86P/0rGPNSWa34kneI4LZIr/G+243cLObPm5N6sm1hSzsCTt+4879tsy2t40P1EueoWOfU8NE1o55gQv5bIib/2i0Y+g7AoTPvxwqW7CXPDoFcJcBZflh3jNguOhvTr0RiewYXJkw888cOJywGv1tW09Qx720o/whv8kxqB97VsJNKkOkmm6ANaKv0ABqank3BAmuJm4aAOFF/OzOqc9v9OK+8PVWONRGr4wCWQDw==
  • Cc: <christopher.w.clark@xxxxxxxxx>, <stefano.stabellini@xxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Delivery-date: Mon, 11 Nov 2024 00:42:14 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 2024-11-08 16:10, Andrew Cooper wrote:
On 08/11/2024 8:48 pm, Jason Andryuk wrote:
On 2024-11-08 14:17, Daniel P. Smith wrote:
On 11/7/24 15:47, Jason Andryuk wrote:
On 2024-11-02 13:25, Daniel P. Smith wrote:
@@ -1745,13 +1733,11 @@ void asmlinkage __init noreturn
__start_xen(void)
       for ( i = 0; i < bi->nr_modules; ++i )
       {
-        const struct boot_module *bm = &bi->mods[i];
+        unsigned long s = bi->mods[i].start, l = bi->mods[i].size;
-        set_pdx_range(bm->mod->mod_start,
-                      bm->mod->mod_start + PFN_UP(bm->mod->mod_end));
-        map_pages_to_xen((unsigned long)mfn_to_virt(bm->mod-
mod_start),
-                         _mfn(bm->mod->mod_start),
-                         PFN_UP(bm->mod->mod_end), PAGE_HYPERVISOR);
+        set_pdx_range(paddr_to_pfn(s), paddr_to_pfn(s) + PFN_UP(l));

This is fine today since s (.start) is checked for page alignment.
The other option would be `paddr_to_pfn(s + l) + 1`, but I'm not
sure that is an improvement.

Out of curiosity, why are you thinking that module start would never
be paged aligned?

I think you have an extra negation - the module start is always page
aligned as checked elsewhere.

While reviewing, I was just noting that this code starts rounding
addresses when it previously operated on pfns.  Non page-aligned s + l
could then cross a page boundary.

It's worth saying that until this patch, Xen critically depends on
modules having 4k alignment.

This patch finally breaks that dependency, so we can e.g. load microcode
actually-early on boot.

While the modules are currently aligned, lets try and write code which
doesn't assume it.

So yes, probably best to have

     set_pdx_range(paddr_to_pfn(s), PFN_UP(s + l));

here.  (I think?)

I think PFN_UP(s + l) is wrong when it ends on an address ending in 0x1000 since it will not be rounded up.

s=0x1000
l=0x1000
PFN_UP(0x2000) = 2, but I think we want to pass 3 set_pdx_range.

So I think we want:
    paddr_to_pfn(s + l) + 1

I was wondering if we needed s + l - 1.  But size is set as
mod_end - mod_start, so s + l is correct.

Regards,
Jason



 


Rackspace

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