| Message ID | 20231030133240.116758-1-xenia.ragiadakou@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] x86/hvm/dom0: fix PVH initrd and metadata placement | expand |
On Mon, Oct 30, 2023 at 03:32:40PM +0200, Xenia Ragiadakou wrote: > 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 three cases below: > (1) start > kernel_start && end > kernel_end > (2) start < kernel_start && end < kernel_end > (3) 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. > In case (3), the evaluation will result in either end = kernel_start > or start = kernel_end but in both cases will be end < start, and > will either load initrd in the middle of the image, or arbitrarily > at kernel_end. > > This patch reorganizes the conditionals to include so far unconsidered > cases as well, uniformly returning the lowest available address. It would be good to mention that this was discovered because Zephyr has multiple loaded segments in non-contiguous ranges, so that we know what triggered the change and how we could test further improvements (like the usage of a rangeset). > > Fixes: 73b47eea2104 ('x86/dom0: improve PVH initrd and metadata placement') > Signed-off-by: Xenia Ragiadakou <xenia.ragiadakou@amd.com> > Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> With the commit message adjusted preferably. Thanks, Roger.
diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c index c7d47d0d4c..62debc7415 100644 --- 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;
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 three cases below: (1) start > kernel_start && end > kernel_end (2) start < kernel_start && end < kernel_end (3) 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. In case (3), the evaluation will result in either end = kernel_start or start = kernel_end but in both cases will be end < start, and will either load initrd in the middle of the image, or arbitrarily at kernel_end. 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@amd.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> --- Changes in v2: - cover further cases of overlap (Jan) - mention with an in-code comment that a proper, more fine-grained solution can be implemented using a rangeset (Roger) xen/arch/x86/hvm/dom0_build.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)