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
