Comment 17 for bug 2024479

Hi Jo,

Thanks for the debdiffs.

Lunar and Jammy (both: 1 patch) look good, just removed '0001' from .patch.

Focal has 6 patches and required more attention/changes (which I adjusted),
and patches look good (some notes below for documentation/other reviewers).

The (updated) debdiffs built correctly on ppa:mfo/lp2024479 on supported
architectures (amd64, arm64, armhf, ppc64el, s390x) and I'm confident on
your testing to be performed during -proposed verification.

So, uploaded to F/J/L.

Thanks,
Mauricio

...

Focal:

1) all Origin: links had wrong commit IDs; fixed.

2) there are commits to fix 3 different things, IIUIC,
   all of which are needed for big arm64 systems:
   (patches 1-2) phys_offset on large memory systems;
   (patches 3-5) many memory regions in /proc/iomem;
   (patch 6) split / memory regions for crash kernel;

3) patch 1 is mostly a big code movement, which I followed,
   and the few small changes in existing functions are for
   the purpose described (and are additions, not changes).

4) patch 2 adds code and hooks it into existing code

5) patch 3 simplifies the function call path, which is OK,
   and does a more significant logic change, but it still
   should perform the same thing (parse /proc/iomem format,
   which didn't change), and has no fixup commits.

6) patch 4 adds helpers for patches 3 and 5.

7) btw, patches 3/4 order is swapped (3 deps on 4); fixed.

8) patch 5 uses the helpers to change some existing code
   from static to dynamic allocation.

9) patch 6 (added to J/L/M) too, similarly, for the number
   of crash kernel / usable memory regions.

Fix-up commits: none strictly required.
---

I checked for newer commits (after 1st) in the modified files:
(cat lp2024479_focal-v2.debdiff | grep '^+--- a/' | cut -d/ -f2- | sort -u)

$ git log --oneline f4ce0706d9574aecb7d4aa9af7208a1bc9b6afb4..origin/master -- \
  kexec/arch/arm64/{kexec,crashdump}-arm64.{c,h} \
  kexec/mem_regions.{c,h} \
  util_lib/Makefile \
  vmcore-dmesg/Makefile \
  vmcore-dmesg/vmcore-dmesg.c

Only 4 commits were applicable to these code changes, AFAICT.

There are 3 cleanup/style/optional, no functional impact:

- commit 545c811050a3 ("Cleanup: remove the read_elf_kcore()")
- commit 14ad054e7baa ("Fix an error definition about the variable 'fname'")
- commit a7c4cb8e9985 ("Cleanup: move it back from util_lib/elf_info.c")

There is 1 which is more serious, but does not impact Ubuntu kernels,
since do not use 52-bit (but 48-) virtual address space with M/L/J/F:

- commit 67ea2d99e135 ("arm64: make phys_offset signed")
  (... phys_offset can be negative if running 52-bits kernel on 48-bits hardware ...)

  mantic:
 $ git grep -r ARM64_VA_BITS origin/master-next -- debian.master/config | grep -e "'y'" -e ARM64_VA_BITS_52
 <...>:CONFIG_ARM64_VA_BITS_48 policy<{'arm64': 'y'}>
 <...>:CONFIG_ARM64_VA_BITS_52 policy<{'arm64-generic-64k': 'n'}>
  lunar:
 $ git grep -r ARM64_VA_BITS origin/master-next -- debian.master/config | grep -e "'y'" -e ARM64_VA_BITS_52
 <...>:CONFIG_ARM64_VA_BITS_48 policy<{'arm64': 'y'}>
 <...>:CONFIG_ARM64_VA_BITS_52 policy<{'arm64-generic-64k': 'n'}>
  jammy:
 $ git grep -r ARM64_VA_BITS origin/master-next -- debian.master/config | grep -e "'y'" -e ARM64_VA_BITS_52
 <...>:CONFIG_ARM64_VA_BITS_48 policy<{'arm64': 'y'}>
 <...>:CONFIG_ARM64_VA_BITS_52 policy<{'arm64-generic-64k': 'n', 'arm64-lowlatency-64k': 'n'}>
  focal:
 $ git grep -r ARM64_VA_BITS origin/master-next -- debian.master/config | grep -e "'y'" -e ARM64_VA_BITS_52
 <...>:CONFIG_ARM64_VA_BITS_48 policy<{'arm64': 'y'}>