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?)
~Andrew
