* Re: [PATCH v4 3/7] mm: page_isolation: check specified range for unmovable pages
From: Oscar Salvador @ 2022-02-02 12:18 UTC (permalink / raw)
To: Zi Yan
Cc: Mel Gorman, David Hildenbrand, linuxppc-dev, linux-kernel,
virtualization, linux-mm, iommu, Eric Ren, Robin Murphy,
Christoph Hellwig, Vlastimil Babka, Marek Szyprowski
In-Reply-To: <20220119190623.1029355-4-zi.yan@sent.com>
On Wed, Jan 19, 2022 at 02:06:19PM -0500, Zi Yan wrote:
> From: Zi Yan <ziy@nvidia.com>
>
> Enable set_migratetype_isolate() to check specified sub-range for
> unmovable pages during isolation. Page isolation is done
> at max(MAX_ORDER_NR_PAEGS, pageblock_nr_pages) granularity, but not all
> pages within that granularity are intended to be isolated. For example,
> alloc_contig_range(), which uses page isolation, allows ranges without
> alignment. This commit makes unmovable page check only look for
> interesting pages, so that page isolation can succeed for any
> non-overlapping ranges.
Another thing that came to my mind.
Prior to this patch, has_unmovable_pages() was checking on pageblock
granularity, starting at pfn#0 of the pageblock.
With this patch, you no longer check on pageblock granularity, which
means you might isolate a pageblock, but some pages that sneaked in
might actually be unmovable.
E.g:
Let's say you have a pageblock that spans (pfn#512,pfn#1024),
and you pass alloc_contig_range() (pfn#514,pfn#1024).
has_unmovable_pages() will start checking the pageblock at pfn#514,
and so it will mis pfn#512 and pfn#513. Isn't that a problem, if those
pfn turn out to be actually unmovable?
--
Oscar Salvador
SUSE Labs
^ permalink raw reply
* Re: [PATCH v4 3/7] mm: page_isolation: check specified range for unmovable pages
From: David Hildenbrand @ 2022-02-02 12:25 UTC (permalink / raw)
To: Oscar Salvador, Zi Yan
Cc: Mel Gorman, linuxppc-dev, linux-kernel, virtualization, linux-mm,
iommu, Eric Ren, Robin Murphy, Christoph Hellwig, Vlastimil Babka,
Marek Szyprowski
In-Reply-To: <Yfp2rv0K6d3cNmwg@localhost.localdomain>
On 02.02.22 13:18, Oscar Salvador wrote:
> On Wed, Jan 19, 2022 at 02:06:19PM -0500, Zi Yan wrote:
>> From: Zi Yan <ziy@nvidia.com>
>>
>> Enable set_migratetype_isolate() to check specified sub-range for
>> unmovable pages during isolation. Page isolation is done
>> at max(MAX_ORDER_NR_PAEGS, pageblock_nr_pages) granularity, but not all
>> pages within that granularity are intended to be isolated. For example,
>> alloc_contig_range(), which uses page isolation, allows ranges without
>> alignment. This commit makes unmovable page check only look for
>> interesting pages, so that page isolation can succeed for any
>> non-overlapping ranges.
>
> Another thing that came to my mind.
> Prior to this patch, has_unmovable_pages() was checking on pageblock
> granularity, starting at pfn#0 of the pageblock.
> With this patch, you no longer check on pageblock granularity, which
> means you might isolate a pageblock, but some pages that sneaked in
> might actually be unmovable.
>
> E.g:
>
> Let's say you have a pageblock that spans (pfn#512,pfn#1024),
> and you pass alloc_contig_range() (pfn#514,pfn#1024).
> has_unmovable_pages() will start checking the pageblock at pfn#514,
> and so it will mis pfn#512 and pfn#513. Isn't that a problem, if those
> pfn turn out to be actually unmovable?
That's the whole idea for being able to allocate parts of an unmovable
pageblock that are movable.
If the first part is unmovable but the second part is movable, nothing
should stop us from trying to allocate the second part.
Of course, we want to remember the original migratetype of the
pageblock, to restore that after isolation -- otherwise we'll end up
converting all such pageblocks to MIGRATE_MOVABLE. The next patch does
that IIRC.
However, devil is in the detail, and I still have to review those parts
of this series.
Note that there are no current users of alloc_contig_range() that
allocate < MAX_ORDER - 1 -- except CMA, but for CMA we immediately exit
has_unmovable_pages() either way.
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v2 1/2] KVM: PPC: Book3S PR: Disable SCV when AIL could be disabled
From: kernel test robot @ 2022-02-02 13:09 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: kbuild-all, Nicholas Piggin
In-Reply-To: <20220129072511.105523-2-npiggin@gmail.com>
Hi Nicholas,
I love your patch! Yet something to improve:
[auto build test ERROR on powerpc/topic/ppc-kvm]
[also build test ERROR on powerpc/next v5.17-rc2 next-20220202]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Nicholas-Piggin/KVM-PPC-Book3S-PR-Fixes-for-AIL-and-SCV/20220129-152655
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git topic/ppc-kvm
config: powerpc-randconfig-r005-20220131 (https://download.01.org/0day-ci/archive/20220202/202202022141.phJ3zWBF-lkp@intel.com/config)
compiler: powerpc-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/a751cf4de8f5854152b969afed947b0640ab0c33
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Nicholas-Piggin/KVM-PPC-Book3S-PR-Fixes-for-AIL-and-SCV/20220129-152655
git checkout a751cf4de8f5854152b969afed947b0640ab0c33
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=powerpc SHELL=/bin/bash arch/powerpc/kvm/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
arch/powerpc/kvm/book3s_pr.c: In function 'kvmppc_core_vcpu_load_pr':
>> arch/powerpc/kvm/book3s_pr.c:146:74: error: 'struct thread_struct' has no member named 'fscr'
146 | if (cpu_has_feature(CPU_FTR_ARCH_300) && (current->thread.fscr & FSCR_SCV))
| ^
arch/powerpc/kvm/book3s_pr.c: In function 'kvmppc_core_vcpu_put_pr':
arch/powerpc/kvm/book3s_pr.c:184:74: error: 'struct thread_struct' has no member named 'fscr'
184 | if (cpu_has_feature(CPU_FTR_ARCH_300) && (current->thread.fscr & FSCR_SCV))
| ^
vim +146 arch/powerpc/kvm/book3s_pr.c
141
142 /* Disable AIL if supported */
143 if (cpu_has_feature(CPU_FTR_HVMODE)) {
144 if (cpu_has_feature(CPU_FTR_ARCH_207S))
145 mtspr(SPRN_LPCR, mfspr(SPRN_LPCR) & ~LPCR_AIL);
> 146 if (cpu_has_feature(CPU_FTR_ARCH_300) && (current->thread.fscr & FSCR_SCV))
147 mtspr(SPRN_FSCR, mfspr(SPRN_FSCR) & ~FSCR_SCV);
148 }
149
150 vcpu->cpu = smp_processor_id();
151 #ifdef CONFIG_PPC_BOOK3S_32
152 current->thread.kvm_shadow_vcpu = vcpu->arch.shadow_vcpu;
153 #endif
154
155 if (kvmppc_is_split_real(vcpu))
156 kvmppc_fixup_split_real(vcpu);
157
158 kvmppc_restore_tm_pr(vcpu);
159 }
160
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* Re: [PATCH RFC v1] drivers/base/node: consolidate node device subsystem initialization in node_dev_init()
From: David Hildenbrand @ 2022-02-02 13:45 UTC (permalink / raw)
To: linux-kernel
Cc: Michal Hocko, linux-ia64, Rafael J. Wysocki, Dave Hansen,
linux-mm, Rich Felker, Paul Mackerras, sparclinux, linux-riscv,
Will Deacon, linux-s390, Yoshinori Sato, linux-sh, x86,
Ingo Molnar, Catalin Marinas, Albert Ou, Vasily Gorbik,
Heiko Carstens, Borislav Petkov, Paul Walmsley, Thomas Gleixner,
linux-arm-kernel, Oscar Salvador, Thomas Bogendoerfer,
Greg Kroah-Hartman, linux-mips, Palmer Dabbelt, Andrew Morton,
linuxppc-dev, David S. Miller
In-Reply-To: <20220128151540.164759-1-david@redhat.com>
On 28.01.22 16:15, David Hildenbrand wrote:
> ... and call node_dev_init() after memory_dev_init() from driver_init(),
> so before any of the existing arch/subsys calls. All online nodes should
> be known at that point.
>
> This is in line with memory_dev_init(), which initializes the memory
> device subsystem and creates all memory block devices.
>
> Similar to memory_dev_init(), panic() if anything goes wrong, we don't
> want to continue with such basic initialization errors.
>
> The important part is that node_dev_init() gets called after
> memory_dev_init() and after cpu_dev_init(), but before any of the
> relevant archs call register_cpu() to register the new cpu device under
> the node device. The latter should be the case for the current users
> of topology_init().
>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Oscar Salvador <osalvador@suse.de>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Paul Walmsley <paul.walmsley@sifive.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Albert Ou <aou@eecs.berkeley.edu>
> Cc: Heiko Carstens <hca@linux.ibm.com>
> Cc: Vasily Gorbik <gor@linux.ibm.com>
> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> Cc: Rich Felker <dalias@libc.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: x86@kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-ia64@vger.kernel.org
> Cc: linux-mips@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-riscv@lists.infradead.org
> Cc: linux-s390@vger.kernel.org
> Cc: linux-sh@vger.kernel.org
> Cc: sparclinux@vger.kernel.org
> Cc: linux-mm@kvack.org
> Signed-off-by: David Hildenbrand <david@redhat.com>
If there are no further comments, then I'll resend as !RFC with a
slightly extended patch description, testing at least under arm64 and
ppc64 as they are relatively easy to test for me.
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: powerpc: Set crashkernel offset to mid of RMA region
From: Sourabh Jain @ 2022-02-02 15:08 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: mahesh, hbathini, Abdul haleem
In-Reply-To: <87r18mn74r.fsf@mpe.ellerman.id.au>
On 01/02/22 17:14, Michael Ellerman wrote:
> Sourabh Jain <sourabhjain@linux.ibm.com> writes:
>> On large config LPARs (having 192 and more cores), Linux fails to boot
>> due to insufficient memory in the first memblock. It is due to the
>> memory reservation for the crash kernel which starts at 128MB offset of
>> the first memblock. This memory reservation for the crash kernel doesn't
>> leave enough space in the first memblock to accommodate other essential
>> system resources.
>>
>> The crash kernel start address was set to 128MB offset by default to
>> ensure that the crash kernel get some memory below the RMA region which
>> is used to be of size 256MB. But given that the RMA region size can be
>> 512MB or more, setting the crash kernel offset to mid of RMA size will
>> leave enough space for kernel to allocate memory for other system
>> resources.
>>
>> Since the above crash kernel offset change is only applicable to the LPAR
>> platform, the LPAR feature detection is pushed before the crash kernel
>> reservation. The rest of LPAR specific initialization will still
>> be done during pseries_probe_fw_features as usual.
>>
>> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>> Reported-and-tested-by: Abdul haleem <abdhalee@linux.vnet.ibm.com>
>>
>> ---
>> arch/powerpc/kernel/rtas.c | 4 ++++
>> arch/powerpc/kexec/core.c | 15 +++++++++++----
>> 2 files changed, 15 insertions(+), 4 deletions(-)
>>
>> ---
>> Change in v3:
>> Dropped 1st and 2nd patch from v2. 1st and 2nd patch from v2 patch
>> series [1] try to discover 1T segment MMU feature support
>> BEFORE boot CPU paca allocation ([1] describes why it is needed).
>> MPE has posted a patch [2] that archives a similar objective by moving
>> boot CPU paca allocation after mmu_early_init_devtree().
>>
>> NOTE: This patch is dependent on the patch [2].
>>
>> [1] https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20211018084434.217772-3-sourabhjain@linux.ibm.com/
>> [2] https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-January/239175.html
>> ---
>>
>> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
>> index 733e6ef36758..06df7464fb57 100644
>> --- a/arch/powerpc/kernel/rtas.c
>> +++ b/arch/powerpc/kernel/rtas.c
>> @@ -1313,6 +1313,10 @@ int __init early_init_dt_scan_rtas(unsigned long node,
>> entryp = of_get_flat_dt_prop(node, "linux,rtas-entry", NULL);
>> sizep = of_get_flat_dt_prop(node, "rtas-size", NULL);
>>
>> + /* need this feature to decide the crashkernel offset */
>> + if (of_get_flat_dt_prop(node, "ibm,hypertas-functions", NULL))
>> + powerpc_firmware_features |= FW_FEATURE_LPAR;
>> +
> As you'd have seen this breaks the 32-bit build. It will need an #ifdef
> CONFIG_PPC64 around it.
>
>> if (basep && entryp && sizep) {
>> rtas.base = *basep;
>> rtas.entry = *entryp;
>> diff --git a/arch/powerpc/kexec/core.c b/arch/powerpc/kexec/core.c
>> index 8b68d9f91a03..abf5897ae88c 100644
>> --- a/arch/powerpc/kexec/core.c
>> +++ b/arch/powerpc/kexec/core.c
>> @@ -134,11 +134,18 @@ void __init reserve_crashkernel(void)
>> if (!crashk_res.start) {
>> #ifdef CONFIG_PPC64
>> /*
>> - * On 64bit we split the RMO in half but cap it at half of
>> - * a small SLB (128MB) since the crash kernel needs to place
>> - * itself and some stacks to be in the first segment.
>> + * On the LPAR platform place the crash kernel to mid of
>> + * RMA size (512MB or more) to ensure the crash kernel
>> + * gets enough space to place itself and some stack to be
>> + * in the first segment. At the same time normal kernel
>> + * also get enough space to allocate memory for essential
>> + * system resource in the first segment. Keep the crash
>> + * kernel starts at 128MB offset on other platforms.
>> */
>> - crashk_res.start = min(0x8000000ULL, (ppc64_rma_size / 2));
>> + if (firmware_has_feature(FW_FEATURE_LPAR))
>> + crashk_res.start = ppc64_rma_size / 2;
>> + else
>> + crashk_res.start = min(0x8000000ULL, (ppc64_rma_size / 2));
> I think this will break on machines using Radix won't it? At this point
> in boot ppc64_rma_size will be == 0. Because we won't call into
> hash__setup_initial_memory_limit().
>
> That's not changed by your patch, but seems like this code needs to be
> more careful/clever.
Interesting, but in my testing, I found that ppc64_rma_size
did get initialized before reserve_crashkernel() using radix on LPAR.
I am not sure why but hash__setup_initial_memory_limit() function is
gets called
regardless of radix or hash. Not sure whether it is by design but here
is the flow:
setup_initial_memory_limit()
static inline void setup_initial_memory_limit()
(arch/powerpc/include/asm/book3s/64/mmu.h)
if (!early_radix_enabled()) // FALSE regardless of radix
is enabled or not
hash__setup_initial_memory_limit() // initialize
ppc64_rma_size
reserve_crashkernel() // initialize crashkernel offset to mid of
RMA size.
For the sack of understanding even if we restrict crashkernel offset
setting to mid RMA (i.e. ppc64_rma_size/2) for
only hash it may not save radix because even today we are assigning
crashkernel offset using
ppc64_rma_size variable.
Is the current flow of initializing ppc64_rma_size variable before
reserve_crashkernel() for radix expected?
Please provide your input.
Thanks,
Sourabh Jain
^ permalink raw reply
* Re: [PATCH v4 3/7] mm: page_isolation: check specified range for unmovable pages
From: Zi Yan @ 2022-02-02 16:25 UTC (permalink / raw)
To: David Hildenbrand
Cc: Mel Gorman, linuxppc-dev, linux-kernel, virtualization, linux-mm,
iommu, Eric Ren, Oscar Salvador, Robin Murphy, Christoph Hellwig,
Vlastimil Babka, Marek Szyprowski
In-Reply-To: <21c196f8-18ca-d720-4241-00c9461854d3@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 2996 bytes --]
On 2 Feb 2022, at 7:25, David Hildenbrand wrote:
> On 02.02.22 13:18, Oscar Salvador wrote:
>> On Wed, Jan 19, 2022 at 02:06:19PM -0500, Zi Yan wrote:
>>> From: Zi Yan <ziy@nvidia.com>
>>>
>>> Enable set_migratetype_isolate() to check specified sub-range for
>>> unmovable pages during isolation. Page isolation is done
>>> at max(MAX_ORDER_NR_PAEGS, pageblock_nr_pages) granularity, but not all
>>> pages within that granularity are intended to be isolated. For example,
>>> alloc_contig_range(), which uses page isolation, allows ranges without
>>> alignment. This commit makes unmovable page check only look for
>>> interesting pages, so that page isolation can succeed for any
>>> non-overlapping ranges.
>>
>> Another thing that came to my mind.
>> Prior to this patch, has_unmovable_pages() was checking on pageblock
>> granularity, starting at pfn#0 of the pageblock.
>> With this patch, you no longer check on pageblock granularity, which
>> means you might isolate a pageblock, but some pages that sneaked in
>> might actually be unmovable.
>>
>> E.g:
>>
>> Let's say you have a pageblock that spans (pfn#512,pfn#1024),
>> and you pass alloc_contig_range() (pfn#514,pfn#1024).
>> has_unmovable_pages() will start checking the pageblock at pfn#514,
>> and so it will mis pfn#512 and pfn#513. Isn't that a problem, if those
>> pfn turn out to be actually unmovable?
>
> That's the whole idea for being able to allocate parts of an unmovable
> pageblock that are movable.
>
> If the first part is unmovable but the second part is movable, nothing
> should stop us from trying to allocate the second part.
>
> Of course, we want to remember the original migratetype of the
> pageblock, to restore that after isolation -- otherwise we'll end up
> converting all such pageblocks to MIGRATE_MOVABLE. The next patch does
> that IIRC.
Yes. A desirable optimization is to make MIGRATE_ISOLATE a standalone bit,
so isolating a pageblock will not remove its original migratetype. It is
on my todo list.
>
> However, devil is in the detail, and I still have to review those parts
> of this series.
Thanks. You can wait for my next version. I am planning to make
start_isolate_page_range() accept any address range and move migratetype
save and restore into it, so that the caller do not need to worry about
alignment.
Basically, start_isolate_page_range() will need to migrate compound pages
at the beginning and/or the end of the given range [start_pfn, end_pfn) if
start_pfn and/or end_pfn-1 is in the middle of a compound page.
If start_pfn and/or end_pfn-1 is in the middle of a free page, the free
page will need to be split and put into separate migratetype lists.
>
>
> Note that there are no current users of alloc_contig_range() that
> allocate < MAX_ORDER - 1 -- except CMA, but for CMA we immediately exit
> has_unmovable_pages() either way.
>
> --
> Thanks,
>
> David / dhildenb
--
Best Regards,
Yan, Zi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 854 bytes --]
^ permalink raw reply
* Re: [PATCH v4 3/7] mm: page_isolation: check specified range for unmovable pages
From: Oscar Salvador @ 2022-02-02 16:35 UTC (permalink / raw)
To: David Hildenbrand
Cc: Mel Gorman, Eric Ren, linuxppc-dev, linux-kernel, virtualization,
linux-mm, iommu, Zi Yan, Robin Murphy, Christoph Hellwig,
Vlastimil Babka, Marek Szyprowski
In-Reply-To: <21c196f8-18ca-d720-4241-00c9461854d3@redhat.com>
On Wed, Feb 02, 2022 at 01:25:28PM +0100, David Hildenbrand wrote:
> That's the whole idea for being able to allocate parts of an unmovable
> pageblock that are movable.
>
> If the first part is unmovable but the second part is movable, nothing
> should stop us from trying to allocate the second part.
Yeah, I see, I was a bit slow there, but I see the point now.
Thanks David
--
Oscar Salvador
SUSE Labs
^ permalink raw reply
* Re: [PATCH] spi: mpc512x-psc: Fix compile errors
From: Mark Brown @ 2022-02-02 18:16 UTC (permalink / raw)
To: linux-spi, Linus Walleij
Cc: Anatolij Gustschin, linuxppc-dev, kernel test robot,
Uwe Kleine-König
In-Reply-To: <20220201234535.569973-1-linus.walleij@linaro.org>
On Wed, 2 Feb 2022 00:45:35 +0100, Linus Walleij wrote:
> My patch created compilation bugs in the MPC512x-PSC driver.
> Fix them up.
>
>
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
Thanks!
[1/1] spi: mpc512x-psc: Fix compile errors
commit: 8d37f2710f022837635d9f97db3ac8c853e86979
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply
* Re: [PATCH 2/2] KVM: selftests: Add support for ppc64le
From: Shuah Khan @ 2022-02-02 19:24 UTC (permalink / raw)
To: Fabiano Rosas, kvm
Cc: aik, linuxppc-dev, Shuah Khan, npiggin, paulus, linux-kselftest,
pbonzini, shuah
In-Reply-To: <20220120170109.948681-3-farosas@linux.ibm.com>
On 1/20/22 10:01 AM, Fabiano Rosas wrote:
> This adds the infrastructure for writing tests for the powerpc
> platform (Only Radix MMU for now).
>
> This patch also enables two tests:
>
> - a dummy sample test that creates a guest with one vcpu, issues
> hypercalls and reads/writes test values from memory.
>
Since this test does something - rename it to something meaningful
as opposed to sample.
> - the kvm_page_table test, although at this point I'm not using it to
> test KVM, but mostly as a way to stress test this code.
>
> $ make -C tools/testing/selftests TARGETS=kvm
> $ make -C tools/testing/selftests TARGETS=kvm run_tests
>
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
> MAINTAINERS | 3 +
> tools/testing/selftests/kvm/.gitignore | 1 +
> tools/testing/selftests/kvm/Makefile | 14 +-
> .../selftests/kvm/include/kvm_util_base.h | 7 +
> .../selftests/kvm/include/ppc64le/processor.h | 43 +++
> tools/testing/selftests/kvm/lib/kvm_util.c | 5 +
> .../testing/selftests/kvm/lib/powerpc/hcall.S | 6 +
> .../selftests/kvm/lib/powerpc/processor.c | 343 ++++++++++++++++++
> .../testing/selftests/kvm/lib/powerpc/ucall.c | 67 ++++
> .../selftests/kvm/powerpc/sample_test.c | 144 ++++++++
> 10 files changed, 630 insertions(+), 3 deletions(-)
> create mode 100644 tools/testing/selftests/kvm/include/ppc64le/processor.h
> create mode 100644 tools/testing/selftests/kvm/lib/powerpc/hcall.S
> create mode 100644 tools/testing/selftests/kvm/lib/powerpc/processor.c
> create mode 100644 tools/testing/selftests/kvm/lib/powerpc/ucall.c
> create mode 100644 tools/testing/selftests/kvm/powerpc/sample_test.c
>
It will be easier to review this if split into a separate patch for
each test.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a76e7558b151..15c89d33d584 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10537,6 +10537,9 @@ F: arch/powerpc/include/asm/kvm*
> F: arch/powerpc/include/uapi/asm/kvm*
> F: arch/powerpc/kernel/kvm*
> F: arch/powerpc/kvm/
> +F: tools/testing/selftests/kvm/include/ppc64le/
> +F: tools/testing/selftests/kvm/lib/powerpc/
> +F: tools/testing/selftests/kvm/powerpc/
>
> KERNEL VIRTUAL MACHINE FOR RISC-V (KVM/riscv)
> M: Anup Patel <anup@brainfault.org>
> diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
> index 8c129961accf..45ab993e2845 100644
> --- a/tools/testing/selftests/kvm/.gitignore
> +++ b/tools/testing/selftests/kvm/.gitignore
> @@ -46,6 +46,7 @@
> /x86_64/xen_vmcall_test
> /x86_64/xss_msr_test
> /x86_64/vmx_pmu_msrs_test
> +/powerpc/sample_test
> /access_tracking_perf_test
> /demand_paging_test
> /dirty_log_test
> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> index 556da71c33b8..5ae27109e9b9 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -17,9 +17,9 @@ KSFT_KHDR_INSTALL := 1
> # LINUX_TOOL_ARCH_INCLUDE is set using ARCH variable.
> #
> # x86_64 targets are named to include x86_64 as a suffix and directories
> -# for includes are in x86_64 sub-directory. s390x and aarch64 follow the
> -# same convention. "uname -m" doesn't result in the correct mapping for
> -# s390x and aarch64.
> +# for includes are in x86_64 sub-directory. s390x, aarch64 and ppc64le
> +# follow the same convention. "uname -m" doesn't result in the correct
> +# mapping for s390x, aarch64 and ppc64le.
> #
> # No change necessary for x86_64
> UNAME_M := $(shell uname -m)
> @@ -36,12 +36,17 @@ endif
> ifeq ($(ARCH),riscv)
> UNAME_M := riscv
> endif
> +# Set UNAME_M for ppc64le compile/install to work
> +ifeq ($(ARCH),powerpc)
> + UNAME_M := ppc64le
> +endif
>
> LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/rbtree.c lib/sparsebit.c lib/test_util.c lib/guest_modes.c lib/perf_test_util.c
> LIBKVM_x86_64 = lib/x86_64/apic.c lib/x86_64/processor.c lib/x86_64/vmx.c lib/x86_64/svm.c lib/x86_64/ucall.c lib/x86_64/handlers.S
> LIBKVM_aarch64 = lib/aarch64/processor.c lib/aarch64/ucall.c lib/aarch64/handlers.S lib/aarch64/spinlock.c lib/aarch64/gic.c lib/aarch64/gic_v3.c lib/aarch64/vgic.c
> LIBKVM_s390x = lib/s390x/processor.c lib/s390x/ucall.c lib/s390x/diag318_test_handler.c
> LIBKVM_riscv = lib/riscv/processor.c lib/riscv/ucall.c
> +LIBKVM_ppc64le = lib/powerpc/processor.c lib/powerpc/ucall.c lib/powerpc/hcall.S
>
> TEST_GEN_PROGS_x86_64 = x86_64/cr4_cpuid_sync_test
> TEST_GEN_PROGS_x86_64 += x86_64/get_msr_index_features
> @@ -133,6 +138,9 @@ TEST_GEN_PROGS_riscv += kvm_page_table_test
> TEST_GEN_PROGS_riscv += set_memory_region_test
> TEST_GEN_PROGS_riscv += kvm_binary_stats_test
>
> +TEST_GEN_PROGS_ppc64le += powerpc/sample_test
Same comment as above. Find a name that desribes what this does?
> +TEST_GEN_PROGS_ppc64le += kvm_page_table_test
> +
> TEST_GEN_PROGS += $(TEST_GEN_PROGS_$(UNAME_M))
> LIBKVM += $(LIBKVM_$(UNAME_M))
>
> diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
> index 66775de26952..a930d663fe67 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util_base.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
> @@ -54,6 +54,7 @@ enum vm_guest_mode {
> VM_MODE_P36V48_16K,
> VM_MODE_P36V48_64K,
> VM_MODE_P36V47_16K,
> + VM_MODE_P51V52_64K,
> NUM_VM_MODES,
> };
>
> @@ -87,6 +88,12 @@ extern enum vm_guest_mode vm_mode_default;
> #define MIN_PAGE_SHIFT 12U
> #define ptes_per_page(page_size) ((page_size) / 8)
>
> +#elif defined(__powerpc__)
> +
> +#define VM_MODE_DEFAULT VM_MODE_P51V52_64K
> +#define MIN_PAGE_SHIFT 16U
> +#define ptes_per_page(page_size) ((page_size) / 8)
> +
> #endif
>
> #define MIN_PAGE_SIZE (1U << MIN_PAGE_SHIFT)
> diff --git a/tools/testing/selftests/kvm/include/ppc64le/processor.h b/tools/testing/selftests/kvm/include/ppc64le/processor.h
> new file mode 100644
> index 000000000000..fbc1332b2b80
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/include/ppc64le/processor.h
> @@ -0,0 +1,43 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * powerpc processor specific defines
> + */
> +#ifndef SELFTEST_KVM_PROCESSOR_H
> +#define SELFTEST_KVM_PROCESSOR_H
Let's make this SELFTEST_KVM_PPC_PROCESSOR_H - it will be
a scpefici to ppc
> +
> +#define PPC_BIT(x) (1ULL << (63 - x))
> +
> +#define MSR_SF PPC_BIT(0)
> +#define MSR_IR PPC_BIT(58)
> +#define MSR_DR PPC_BIT(59)
> +#define MSR_LE PPC_BIT(63)
> +
> +#define LPCR_UPRT PPC_BIT(41)
> +#define LPCR_EVIRT PPC_BIT(42)
> +#define LPCR_HR PPC_BIT(43)
> +#define LPCR_GTSE PPC_BIT(53)
> +
> +#define PATB_GR PPC_BIT(0)
> +
> +#define PTE_VALID PPC_BIT(0)
> +#define PTE_LEAF PPC_BIT(1)
> +#define PTE_R PPC_BIT(55)
> +#define PTE_C PPC_BIT(56)
> +#define PTE_RC (PTE_R | PTE_C)
> +#define PTE_READ 0x4
> +#define PTE_WRITE 0x2
> +#define PTE_EXEC 0x1
> +#define PTE_RWX (PTE_READ|PTE_WRITE|PTE_EXEC)
> +
> +extern uint64_t hcall(uint64_t nr, ...);
> +
> +static inline uint32_t mfpvr(void)
> +{
> + uint32_t pvr;
> +
> + asm ("mfpvr %0"
> + : "=r"(pvr));
Let's match the asm() style with the rest of the asm code in this
test. Same line.
asm ("mfpvr %0" : "=r"(pvr));
> + return pvr;
> +}
> +
> +#endif
thanks,
-- Shuah
^ permalink raw reply
* Re: [PATCH v7 0/5] Allow guest access to EFI confidential computing secret area
From: Matthew Garrett @ 2022-02-02 4:01 UTC (permalink / raw)
To: James Bottomley
Cc: linux-efi, Brijesh Singh, Lenny Szubowicz, Gerd Hoffmann,
gcwilson, Ard Biesheuvel, Daniele Buono, Andi Kleen, Nayna Jain,
James Morris, Dov Murik, Jim Cadden, Peter Gonda, Borislav Petkov,
Serge E. Hallyn, Tom Lendacky, Ashish Kalra, dougmill,
Dr. David Alan Gilbert, Tobin Feldman-Fitzthum, linux-coco,
gjoyce, dja, Dave Hansen, Greg KH, linux-kernel,
linux-security-module, linuxppc-dev, Andrew Scull
In-Reply-To: <37779659ca96ac9c1f11bcc0ac0665895c795b54.camel@linux.ibm.com>
On Tue, Feb 01, 2022 at 09:24:50AM -0500, James Bottomley wrote:
> On Tue, 2022-02-01 at 14:50 +0100, Greg KH wrote:
> > You all need to work together to come up with a unified place for
> > this and stop making it platform-specific.
We're talking about things that have massively different semantics. How
do we expose that without an unwieldy API that has to try to be a
superset of everything implemented, which then has to be extended when
yet another implementation shows up with another behavioural quirk? EFI
variables already need extremely careful handling to avoid rm -rf /sys
bricking the system - should we impose that on everything, or should we
allow the underlying implementation to leak through in some ways?
^ permalink raw reply
* [PATCH] powerpc/module_64: use module_init_section instead of patching names
From: Wedson Almeida Filho @ 2022-02-02 5:51 UTC (permalink / raw)
To: mpe; +Cc: Wedson Almeida Filho, paulus, linuxppc-dev, linux-kernel
Without this patch, module init sections are disabled by patching their
names in arch-specific code when they're loaded (which prevents code in
layout_sections from finding init sections). This patch uses the new
arch-specific module_init_section instead.
This allows modules that have .init_array sections to have the
initialisers properly called (on load, before init). Without this patch,
the initialisers are not called because .init_array is renamed to
_init_array, and thus isn't found by code in find_module_sections().
Signed-off-by: Wedson Almeida Filho <wedsonaf@google.com>
---
arch/powerpc/kernel/module_64.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 5d77d3f5fbb5..6a45e6ddbe58 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -277,6 +277,12 @@ static Elf64_Sym *find_dot_toc(Elf64_Shdr *sechdrs,
return NULL;
}
+bool module_init_section(const char *name)
+{
+ /* We don't handle .init for the moment: always return false. */
+ return false;
+}
+
int module_frob_arch_sections(Elf64_Ehdr *hdr,
Elf64_Shdr *sechdrs,
char *secstrings,
@@ -286,7 +292,6 @@ int module_frob_arch_sections(Elf64_Ehdr *hdr,
/* Find .toc and .stubs sections, symtab and strtab */
for (i = 1; i < hdr->e_shnum; i++) {
- char *p;
if (strcmp(secstrings + sechdrs[i].sh_name, ".stubs") == 0)
me->arch.stubs_section = i;
else if (strcmp(secstrings + sechdrs[i].sh_name, ".toc") == 0) {
@@ -298,10 +303,6 @@ int module_frob_arch_sections(Elf64_Ehdr *hdr,
dedotify_versions((void *)hdr + sechdrs[i].sh_offset,
sechdrs[i].sh_size);
- /* We don't handle .init for the moment: rename to _init */
- while ((p = strstr(secstrings + sechdrs[i].sh_name, ".init")))
- p[0] = '_';
-
if (sechdrs[i].sh_type == SHT_SYMTAB)
dedotify((void *)hdr + sechdrs[i].sh_offset,
sechdrs[i].sh_size / sizeof(Elf64_Sym),
--
2.35.0.rc2.247.g8bbb082509-goog
^ permalink raw reply related
* Re: [PATCH v7 0/5] Allow guest access to EFI confidential computing secret area
From: Matthew Garrett @ 2022-02-02 6:54 UTC (permalink / raw)
To: Greg KH
Cc: linux-efi, Brijesh Singh, Lenny Szubowicz, Gerd Hoffmann,
gcwilson, Ard Biesheuvel, Daniele Buono, Andi Kleen, Nayna Jain,
James Morris, Dov Murik, Jim Cadden, Peter Gonda, Borislav Petkov,
Serge E. Hallyn, Tom Lendacky, Ashish Kalra, dougmill,
James Bottomley, Dr. David Alan Gilbert, Tobin Feldman-Fitzthum,
linux-coco, gjoyce, dja, Dave Hansen, linux-kernel,
linux-security-module, linuxppc-dev, Andrew Scull
In-Reply-To: <YfogOurPZb7+Yelo@kroah.com>
On Wed, Feb 02, 2022 at 07:10:02AM +0100, Greg KH wrote:
> On Wed, Feb 02, 2022 at 04:01:57AM +0000, Matthew Garrett wrote:
> > We're talking about things that have massively different semantics.
>
> I see lots of different platforms trying to provide access to their
> "secure" firmware data to userspace in different ways. That feels to me
> like they are the same thing that userspace would care about in a
> unified way.
EFI variables are largely for the OS to provide information to the
firmware, while this patchset is to provide information from the
firmware to the OS. I don't see why we'd expect to use the same userland
tooling for both.
In the broader case - I don't think we *can* use the same userland
tooling for everything. For example, the patches to add support for
manipulating the Power secure boot keys originally attempted to make it
look like efivars, but the underlying firmware semantics are
sufficiently different that even exposing the same kernel interface
wouldn't be a sufficient abstraction and userland would still need to
behave differently. Exposing an interface that looks consistent but
isn't is arguably worse for userland than exposing explicitly distinct
interfaces.
^ permalink raw reply
* Re: [PATCH v7 0/5] Allow guest access to EFI confidential computing secret area
From: Matthew Garrett @ 2022-02-02 7:10 UTC (permalink / raw)
To: Greg KH
Cc: linux-efi, Brijesh Singh, Lenny Szubowicz, Gerd Hoffmann,
gcwilson, Ard Biesheuvel, Daniele Buono, Andi Kleen, Nayna Jain,
James Morris, Dov Murik, Jim Cadden, Peter Gonda, Borislav Petkov,
Serge E. Hallyn, Tom Lendacky, Ashish Kalra, dougmill,
James Bottomley, Dr. David Alan Gilbert, Tobin Feldman-Fitzthum,
linux-coco, gjoyce, dja, Dave Hansen, linux-kernel,
linux-security-module, linuxppc-dev, Andrew Scull
In-Reply-To: <YfotMyQiQ66xfCOQ@kroah.com>
On Wed, Feb 02, 2022 at 08:05:23AM +0100, Greg KH wrote:
> I see different platform patches trying to stick these blobs in
> different locations and ways to access (securityfs, sysfs, char device
> node), which seems crazy to me. Why can't we at least pick one way to
> access these to start with, and then have the filesystem layout be
> platform-specific as needed, which will give the correct hints to
> userspace as to what it needs to do here?
Which other examples are you thinking of? I think this conversation may
have accidentally become conflated with a different prior one and now
we're talking at cross purposes.
^ permalink raw reply
* Re: [PATCH v7 0/5] Allow guest access to EFI confidential computing secret area
From: Matthew Garrett @ 2022-02-02 8:04 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-efi, Brijesh Singh, Lenny Szubowicz, Gerd Hoffmann,
gcwilson, Daniele Buono, Andi Kleen, Nayna Jain, James Morris,
Dov Murik, Jim Cadden, Peter Gonda, Borislav Petkov,
Serge E. Hallyn, Tom Lendacky, Ashish Kalra, dougmill,
James Bottomley, Dr. David Alan Gilbert, Tobin Feldman-Fitzthum,
linux-coco, gjoyce, Daniel Axtens, Dave Hansen, Greg KH,
Linux Kernel Mailing List, linux-security-module,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), Andrew Scull
In-Reply-To: <CAMj1kXFTyc9KnMsnvs+mt80DbJL8VGKKcQ0J=4NrGYGSAG8sRw@mail.gmail.com>
On Wed, Feb 02, 2022 at 08:22:03AM +0100, Ard Biesheuvel wrote:
> On Wed, 2 Feb 2022 at 08:10, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> > Which other examples are you thinking of? I think this conversation may
> > have accidentally become conflated with a different prior one and now
> > we're talking at cross purposes.
>
> This came up a while ago during review of one of the earlier revisions
> of this patch set.
>
> https://lore.kernel.org/linux-efi/YRZuIIVIzMfgjtEl@google.com/
>
> which describes another two variations on the theme, for pKVM guests
> as well as Android bare metal.
Oh, I see! That makes much more sense - sorry, I wasn't Cc:ed on that,
so thought this was related to the efivars/Power secure boot. My
apologies, sorry for the noise. In that case, given the apparent
agreement between the patch owners that a consistent interface would
work for them, I think I agree with Greg that we should strive for that.
Given the described behaviour of the Google implementation, it feels
like the semantics in this implementation would be sufficient for them
as well, but having confirmation of that would be helpful.
On the other hand, I also agree that a new filesystem for this is
overkill. I did that for efivarfs and I think the primary lesson from
that is that people who aren't familiar with the vfs shouldn't be
writing filesystems. Securityfs seems entirely reasonable, and it's
consistent with other cases where we expose firmware-provided data
that's security relevant.
The only thing I personally struggle with here is whether "coco" is the
best name for it, and whether there are reasonable use cases that
wouldn't be directly related to confidential computing (eg, if the
firmware on a bare-metal platform had a mechanism for exposing secrets
to the OS based on some specific platform security state, it would seem
reasonable to expose it via this mechanism but it may not be what we'd
normally think of as Confidential Computing).
But I'd also say that while we only have one implementation currently
sending patches, it's fine for the code to live in that implementation
and then be abstracted out once we have another.
^ permalink raw reply
* Re: [PATCH v7 0/5] Allow guest access to EFI confidential computing secret area
From: Matthew Garrett @ 2022-02-02 8:45 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: linux-efi, Brijesh Singh, Lenny Szubowicz, gcwilson,
Ard Biesheuvel, Daniele Buono, Andi Kleen, Nayna Jain,
James Morris, Dov Murik, Jim Cadden, Peter Gonda, Borislav Petkov,
Serge E. Hallyn, Tom Lendacky, Ashish Kalra, dougmill,
James Bottomley, Dr. David Alan Gilbert, Tobin Feldman-Fitzthum,
linux-coco, gjoyce, Daniel Axtens, Dave Hansen, Greg KH,
Linux Kernel Mailing List, linux-security-module,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), Andrew Scull
In-Reply-To: <20220202083653.p3cb4w3qdud4e33t@sirius.home.kraxel.org>
On Wed, Feb 02, 2022 at 09:36:53AM +0100, Gerd Hoffmann wrote:
> Having a "secrets/" directory looks good to me. Then the individual
> implementations can either add files to the directory, i.e. efi_secrets
> would create "secrets/<guid>" files. Or each implementation creates a
> subdirectory with the secrets, i.e. "secrets/coco/" and
> "secrets/coco/<guid>".
I prefer a subdirectory, on the basis that we could conceivably end up
with more than one implementation on a single device at some point, and
also because it makes it trivial for userland to determine what the
source is which may make a semantic difference under certain
circumstances.
> Longer-term (i.e once we have more than one implementation) we probably
> need a separate module which owns and manages the "secrets/" directory,
> and possibly provides some common helper functions too.
Agree.
^ permalink raw reply
* Re: [PATCH 6/7] modules: Add CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
From: Luis Chamberlain @ 2022-02-02 23:34 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-arch@vger.kernel.org, Daniel Thompson,
kgdb-bugreport@lists.sourceforge.net, Jason Wessel,
linux-kernel@vger.kernel.org, Douglas Anderson,
linux-mm@kvack.org, Jessica Yu, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <b49235ef-1fc3-7b8f-7e23-79f461d83e18@csgroup.eu>
On Wed, Jan 26, 2022 at 06:38:30AM +0000, Christophe Leroy wrote:
>
>
> Le 25/01/2022 à 22:10, Luis Chamberlain a écrit :
> > On Mon, Jan 24, 2022 at 09:22:34AM +0000, Christophe Leroy wrote:
> >> This can also be useful on other powerpc/32 in order to maximize the
> >> chance of code being close enough to kernel core to avoid branch
> >> trampolines.
> >
> > Curious about all this branch trampoline talk. Do you have data to show
> > negative impact with things as-is?
>
> See
> https://github.com/linuxppc/linux/commit/2ec13df167040cd153c25c4d96d0ffc573ac4c40
>
> Or
> https://github.com/linuxppc/linux/commit/7d485f647c1f4a6976264c90447fb0dbf07b111d
This was useful and fun to read, thanks.
> > Also, was powerpc/32 broken then without this? The commit log seems to
> > suggest so, but I don't think that's the case. How was this issue noticed?
>
>
> Your question is related to the trampoline topic or the exec/noexec
> flagging ?
>
> Regarding trampoline, everything is working OK. That's just cherry on
> the cake, when putting data away you can have more code closer to the
> kernel. But that would not have been a reason in itself for this series.
>
> Regarding the exec/noexec discussion, it's a real issue. powerpc/32
> doesn't honor page exec flag, so when you select STRICT_MODULES_RWX and
> flag module data as no-exec, it remains executable. That's because
> powerpc/32 MMU doesn't have a per page exec flag but only a per
> 256Mbytes segment exec flag.
>
> Typical PPC32 layount:
> 0xf0000000-0xffffffff : VMALLOC AREA ==> NO EXEC
> 0xc0000000-0xefffffff : Linear kernel memory mapping
> 0xb0000000-0xbfffffff : MODULES AREA ==> EXEC
> 0x00000000-0xafffffff : User space ==> EXEC
>
> So STRICT_MODULES_RWX is broken on some powerpc/32
You know, this is the sort of information that I think would be
very useful for the commit log. Can you ammend?
> >
> > Are there other future CPU families being planned where this is all true for
> > as well? Are they goin to be 32-bit as well?
>
> Future I don't know.
>
> Regarding the trampoline stuff, I see at least the following existing
> architectures with a similar constraint:
>
> ARM:
>
> https://elixir.bootlin.com/linux/v5.16/source/arch/arm/include/asm/memory.h#L55
>
> ARM even has a config item to allow trampolines or not. I might add the
> same to powerpc to reduce number of pages used by modules.
>
> https://elixir.bootlin.com/linux/v5.16/source/arch/arm/Kconfig#L1514
>
> NDS32 has the constraint
>
> https://elixir.bootlin.com/linux/v5.16/source/arch/nds32/include/asm/memory.h#L41
>
> NIOS2 has the constraint, allthough they handled it in a different way:
>
> https://elixir.bootlin.com/linux/v5.16/source/arch/nios2/kernel/module.c#L30
>
>
>
> Even ARM64 benefits from modules closer to kernel:
>
> https://elixir.bootlin.com/linux/v5.16/source/arch/arm64/Kconfig#L1848
>
>
> Another future opportunity with the ability to allocate module parts
> separately is the possibility to then use huge vmalloc mappings.
>
> Today huge vmalloc mappings cannot be used for modules, see recent
> discussion at
> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20211227145903.187152-4-wangkefeng.wang@huawei.com/
Alrighty, this is sufficient information, thanks!
Luis
^ permalink raw reply
* Re: [PATCH v2 5/5] powerpc: Select ARCH_WANTS_MODULES_DATA_IN_VMALLOC on book3s/32 and 8xx
From: Luis Chamberlain @ 2022-02-02 23:36 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-arch@vger.kernel.org, kgdb-bugreport@lists.sourceforge.net,
linux-kernel@vger.kernel.org, linux-mm@kvack.org, Paul Mackerras,
Jessica Yu, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <a20285472ad0a0a13a1d93c4707180be5b4fa092.1643282353.git.christophe.leroy@csgroup.eu>
On Thu, Jan 27, 2022 at 11:28:12AM +0000, Christophe Leroy wrote:
> book3s/32 and 8xx have a separate area for allocating modules,
> defined by MODULES_VADDR / MODULES_END.
>
> On book3s/32, it is not possible to protect against execution
> on a page basis. A full 256M segment is either Exec or NoExec.
> The module area is in an Exec segment while vmalloc area is
> in a NoExec segment.
>
> In order to protect module data against execution, select
> ARCH_WANTS_MODULES_DATA_IN_VMALLOC.
>
> For the 8xx (and possibly other 32 bits platform in the future),
> there is no such constraint on Exec/NoExec protection, however
> there is a critical distance between kernel functions and callers
> that needs to remain below 32Mbytes in order to avoid costly
> trampolines. By allocating data outside of module area, we
> increase the chance for module text to remain within acceptable
> distance from kernel core text.
>
> So select ARCH_WANTS_MODULES_DATA_IN_VMALLOC for 8xx as well.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
Cc list first and then the SOB.
Luis
^ permalink raw reply
* Re: [PATCH] mm: Merge pte_mkhuge() call into arch_make_huge_pte()
From: Mike Kravetz @ 2022-02-02 23:36 UTC (permalink / raw)
To: Anshuman Khandual, linux-mm
Cc: Catalin Marinas, linuxppc-dev, linux-kernel, Paul Mackerras,
sparclinux, Andrew Morton, Will Deacon, David S. Miller,
linux-arm-kernel
In-Reply-To: <1643780286-18798-1-git-send-email-anshuman.khandual@arm.com>
On 2/1/22 21:38, Anshuman Khandual wrote:
> Each call into pte_mkhuge() is invariably followed by arch_make_huge_pte().
> Instead arch_make_huge_pte() can accommodate pte_mkhuge() at the beginning.
> This updates generic fallback stub for arch_make_huge_pte() and available
> platforms definitions. This makes huge pte creation much cleaner and easier
> to follow.
>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Mike Kravetz <mike.kravetz@oracle.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: sparclinux@vger.kernel.org
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> arch/arm64/mm/hugetlbpage.c | 1 +
> arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h | 1 +
> arch/sparc/mm/hugetlbpage.c | 1 +
> include/linux/hugetlb.h | 2 +-
> mm/hugetlb.c | 3 +--
> mm/vmalloc.c | 1 -
> 6 files changed, 5 insertions(+), 4 deletions(-)
Seems like a reasonable cleanup/simplification to me.
Acked-by: Mike Kravetz <mike.kravetz@oracle.com>
--
Mike Kravetz
^ permalink raw reply
* Re: [PATCH v3 3/6] modules: Introduce data_layout
From: Luis Chamberlain @ 2022-02-02 23:48 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-arch@vger.kernel.org, kgdb-bugreport@lists.sourceforge.net,
linux-kernel@vger.kernel.org, linux-mm@kvack.org, Jessica Yu,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <230bfd896f24ca7a9281783aaa8c0ebfebd0bc7e.1643475473.git.christophe.leroy@csgroup.eu>
On Sat, Jan 29, 2022 at 05:02:07PM +0000, Christophe Leroy wrote:
> diff --git a/kernel/module.c b/kernel/module.c
> index 163e32e39064..11f51e17fb9f 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -81,6 +81,8 @@
> /* If this is set, the section belongs in the init part of the module */
> #define INIT_OFFSET_MASK (1UL << (BITS_PER_LONG-1))
>
> +#define data_layout core_layout
> +
> /*
> * Mutex protects:
> * 1) List of modules (also safely readable with preempt_disable),
> @@ -2451,7 +2454,10 @@ static void layout_sections(struct module *mod, struct load_info *info)
> || s->sh_entsize != ~0UL
> || module_init_layout_section(sname))
> continue;
> - s->sh_entsize = get_offset(mod, &mod->core_layout.size, s, i);
> + if (m)
> + s->sh_entsize = get_offset(mod, &mod->data_layout.size, s, i);
> + else
> + s->sh_entsize = get_offset(mod, &mod->core_layout.size, s, i);
> pr_debug("\t%s\n", sname);
Huh why is this branching here, given you just used mod->data_layout in
all other areas?
> @@ -3468,6 +3474,8 @@ static int move_module(struct module *mod, struct load_info *info)
> if (shdr->sh_entsize & INIT_OFFSET_MASK)
> dest = mod->init_layout.base
> + (shdr->sh_entsize & ~INIT_OFFSET_MASK);
> + else if (!(shdr->sh_flags & SHF_EXECINSTR))
> + dest = mod->data_layout.base + shdr->sh_entsize;
> else
> dest = mod->core_layout.base + shdr->sh_entsize;
>
Likewise here.
Luis
^ permalink raw reply
* Re: [PATCH v3 4/6] modules: Add CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
From: Luis Chamberlain @ 2022-02-03 0:01 UTC (permalink / raw)
To: Christophe Leroy, Aaron Tomlin
Cc: linux-arch@vger.kernel.org, Daniel Thompson,
kgdb-bugreport@lists.sourceforge.net, Jason Wessel,
linux-kernel@vger.kernel.org, Douglas Anderson,
linux-mm@kvack.org, Jessica Yu, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <b59ed8781ef9af995c5bfa762de1f42fdfc57c74.1643475473.git.christophe.leroy@csgroup.eu>
On Sat, Jan 29, 2022 at 05:02:09PM +0000, Christophe Leroy wrote:
> diff --git a/kernel/module.c b/kernel/module.c
> index 11f51e17fb9f..f3758115ebaa 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -81,7 +81,9 @@
> /* If this is set, the section belongs in the init part of the module */
> #define INIT_OFFSET_MASK (1UL << (BITS_PER_LONG-1))
>
> +#ifndef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
> #define data_layout core_layout
> +#endif
>
> /*
> * Mutex protects:
> @@ -111,6 +113,12 @@ static struct mod_tree_root {
> #define module_addr_min mod_tree.addr_min
> #define module_addr_max mod_tree.addr_max
>
> +#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
> +static struct mod_tree_root mod_data_tree __cacheline_aligned = {
> + .addr_min = -1UL,
> +};
> +#endif
> +
> #ifdef CONFIG_MODULES_TREE_LOOKUP
>
> /*
> @@ -186,6 +194,11 @@ static void mod_tree_insert(struct module *mod)
> __mod_tree_insert(&mod->core_layout.mtn, &mod_tree);
> if (mod->init_layout.size)
> __mod_tree_insert(&mod->init_layout.mtn, &mod_tree);
> +
> +#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
> + mod->data_layout.mtn.mod = mod;
> + __mod_tree_insert(&mod->data_layout.mtn, &mod_data_tree);
> +#endif
kernel/ directory has quite a few files, module.c is the second to
largest file, and it has tons of stuff. Aaron is doing work to
split things out to make code easier to read and so that its easier
to review changes. See:
https://lkml.kernel.org/r/20220130213214.1042497-1-atomlin@redhat.com
I think this is a good patch example which could benefit from that work.
So I'd much prefer to see that work go in first than this, so to see if
we can make the below changes more compartamentalized.
Curious, how much testing has been put into this series?
Luis
^ permalink raw reply
* Re: [PATCH v3 0/6] Allocate module text and data separately
From: Luis Chamberlain @ 2022-02-03 0:05 UTC (permalink / raw)
To: Christophe Leroy, Aaron Tomlin
Cc: linux-arch@vger.kernel.org, kgdb-bugreport@lists.sourceforge.net,
linux-kernel@vger.kernel.org, linux-mm@kvack.org, Jessica Yu,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <cover.1643475473.git.christophe.leroy@csgroup.eu>
On Sat, Jan 29, 2022 at 05:02:03PM +0000, Christophe Leroy wrote:
> This series allow architectures to request having modules data in
> vmalloc area instead of module area.
>
> This is required on powerpc book3s/32 in order to set data non
> executable, because it is not possible to set executability on page
> basis, this is done per 256 Mbytes segments. The module area has exec
> right, vmalloc area has noexec. Without this change module data
> remains executable regardless of CONFIG_STRICT_MODULES_RWX.
>
> This can also be useful on other powerpc/32 in order to maximize the
> chance of code being close enough to kernel core to avoid branch
> trampolines.
>
This looks good, however I'd like to see Aaron's changes go in first,
and then yours. Aaron's changes still need to be tested by 0-day and I
need to finish review, but that's the order of how I'd prefer to see
changes merged / tested. I'll try to review his changes, dump them to
modules-next and then I'd like to trouble you to rebase ontop of that.
We should get all this tested early for the next release.
Luis
^ permalink raw reply
* Re: [PATCH V5 15/21] riscv: compat: Add hw capability check for elf
From: Guo Ren @ 2022-02-03 2:44 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-arch, linux-s390, Guo Ren, Parisc List, Arnd Bergmann,
Greg Kroah-Hartman, Drew Fustini, Anup Patel, Wang Junqiang,
the arch/x86 maintainers, Linux Kernel Mailing List, linux-csky,
open list:BROADCOM NVRAM DRIVER, Palmer Dabbelt, liush,
sparclinux, linux-riscv, linuxppc-dev, Linux ARM, Wei Fu
In-Reply-To: <20220202075159.GB18398@lst.de>
On Wed, Feb 2, 2022 at 3:52 PM Christoph Hellwig <hch@lst.de> wrote:
>
> On Tue, Feb 01, 2022 at 11:05:39PM +0800, guoren@kernel.org wrote:
> > +bool compat_elf_check_arch(Elf32_Ehdr *hdr)
> > +{
> > + if (compat_mode_support && (hdr->e_machine == EM_RISCV))
> > + return true;
> > + else
> > + return false;
> > +}
>
> This can be simplified to:
>
> return compat_mode_support && hdr->e_machine == EM_RISCV;
Good point.
>
> I'd also rename compat_mode_support to compat_mode_supported
Okay
>
> > +
> > +static int compat_mode_detect(void)
> > +{
> > + unsigned long tmp = csr_read(CSR_STATUS);
> > +
> > + csr_write(CSR_STATUS, (tmp & ~SR_UXL) | SR_UXL_32);
> > +
> > + if ((csr_read(CSR_STATUS) & SR_UXL) != SR_UXL_32) {
> > + pr_info("riscv: 32bit compat mode detect failed\n");
> > + compat_mode_support = false;
> > + } else {
> > + compat_mode_support = true;
> > + pr_info("riscv: 32bit compat mode detected\n");
> > + }
>
> I don't think we need these printks here.
Okay
>
> Also this could be simplified to:
>
> compat_mode_supported = (csr_read(CSR_STATUS) & SR_UXL) == SR_UXL_32;
Okay
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
^ permalink raw reply
* [PATCH V2] mm: Merge pte_mkhuge() call into arch_make_huge_pte()
From: Anshuman Khandual @ 2022-02-03 3:57 UTC (permalink / raw)
To: linux-mm
Cc: Anshuman Khandual, Catalin Marinas, linuxppc-dev, linux-kernel,
Paul Mackerras, sparclinux, Andrew Morton, Will Deacon,
David S. Miller, linux-arm-kernel, Mike Kravetz
Each call into pte_mkhuge() is invariably followed by arch_make_huge_pte().
Instead arch_make_huge_pte() can accommodate pte_mkhuge() at the beginning.
This updates generic fallback stub for arch_make_huge_pte() and available
platforms definitions. This makes huge pte creation much cleaner and easier
to follow.
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Paul Mackerras <paulus@samba.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Mike Kravetz <mike.kravetz@oracle.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: sparclinux@vger.kernel.org
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Acked-by: Mike Kravetz <mike.kravetz@oracle.com>
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
This applies on v5.17-rc2
Changes in V2:
- Direct PTE encode in arch_make_huge_pte() on powerpc platform per Christophe
Changes in V1:
https://lore.kernel.org/all/1643780286-18798-1-git-send-email-anshuman.khandual@arm.com/
arch/arm64/mm/hugetlbpage.c | 1 +
arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h | 4 ++--
arch/sparc/mm/hugetlbpage.c | 1 +
include/linux/hugetlb.h | 2 +-
mm/hugetlb.c | 3 +--
mm/vmalloc.c | 1 -
6 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
index ffb9c229610a..228226c5fa80 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -347,6 +347,7 @@ pte_t arch_make_huge_pte(pte_t entry, unsigned int shift, vm_flags_t flags)
{
size_t pagesize = 1UL << shift;
+ entry = pte_mkhuge(entry);
if (pagesize == CONT_PTE_SIZE) {
entry = pte_mkcont(entry);
} else if (pagesize == CONT_PMD_SIZE) {
diff --git a/arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h b/arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h
index 64b6c608eca4..de092b04ee1a 100644
--- a/arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h
+++ b/arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h
@@ -71,9 +71,9 @@ static inline pte_t arch_make_huge_pte(pte_t entry, unsigned int shift, vm_flags
size_t size = 1UL << shift;
if (size == SZ_16K)
- return __pte(pte_val(entry) & ~_PAGE_HUGE);
+ return __pte(pte_val(entry) | _PAGE_SPS);
else
- return entry;
+ return __pte(pte_val(entry) | _PAGE_SPS | _PAGE_HUGE);
}
#define arch_make_huge_pte arch_make_huge_pte
#endif
diff --git a/arch/sparc/mm/hugetlbpage.c b/arch/sparc/mm/hugetlbpage.c
index 0f49fada2093..d8e0e3c7038d 100644
--- a/arch/sparc/mm/hugetlbpage.c
+++ b/arch/sparc/mm/hugetlbpage.c
@@ -181,6 +181,7 @@ pte_t arch_make_huge_pte(pte_t entry, unsigned int shift, vm_flags_t flags)
{
pte_t pte;
+ entry = pte_mkhuge(entry);
pte = hugepage_shift_to_tte(entry, shift);
#ifdef CONFIG_SPARC64
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index d1897a69c540..52c462390aee 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -754,7 +754,7 @@ static inline void arch_clear_hugepage_flags(struct page *page) { }
static inline pte_t arch_make_huge_pte(pte_t entry, unsigned int shift,
vm_flags_t flags)
{
- return entry;
+ return pte_mkhuge(entry);
}
#endif
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 61895cc01d09..5ca253c1b4e4 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -4637,7 +4637,6 @@ static pte_t make_huge_pte(struct vm_area_struct *vma, struct page *page,
vma->vm_page_prot));
}
entry = pte_mkyoung(entry);
- entry = pte_mkhuge(entry);
entry = arch_make_huge_pte(entry, shift, vma->vm_flags);
return entry;
@@ -6172,7 +6171,7 @@ unsigned long hugetlb_change_protection(struct vm_area_struct *vma,
unsigned int shift = huge_page_shift(hstate_vma(vma));
old_pte = huge_ptep_modify_prot_start(vma, address, ptep);
- pte = pte_mkhuge(huge_pte_modify(old_pte, newprot));
+ pte = huge_pte_modify(old_pte, newprot);
pte = arch_make_huge_pte(pte, shift, vma->vm_flags);
huge_ptep_modify_prot_commit(vma, address, ptep, old_pte, pte);
pages++;
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 4165304d3547..d0b14dd73adc 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -118,7 +118,6 @@ static int vmap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
if (size != PAGE_SIZE) {
pte_t entry = pfn_pte(pfn, prot);
- entry = pte_mkhuge(entry);
entry = arch_make_huge_pte(entry, ilog2(size), 0);
set_huge_pte_at(&init_mm, addr, pte, entry);
pfn += PFN_DOWN(size);
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v2 5/5] powerpc: Select ARCH_WANTS_MODULES_DATA_IN_VMALLOC on book3s/32 and 8xx
From: Michael Ellerman @ 2022-02-03 5:39 UTC (permalink / raw)
To: Luis Chamberlain, Christophe Leroy
Cc: linux-arch@vger.kernel.org, kgdb-bugreport@lists.sourceforge.net,
linux-kernel@vger.kernel.org, linux-mm@kvack.org, Paul Mackerras,
Jessica Yu, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <YfsVhcpVTW0+YCl5@bombadil.infradead.org>
Luis Chamberlain <mcgrof@kernel.org> writes:
> On Thu, Jan 27, 2022 at 11:28:12AM +0000, Christophe Leroy wrote:
>> book3s/32 and 8xx have a separate area for allocating modules,
>> defined by MODULES_VADDR / MODULES_END.
>>
>> On book3s/32, it is not possible to protect against execution
>> on a page basis. A full 256M segment is either Exec or NoExec.
>> The module area is in an Exec segment while vmalloc area is
>> in a NoExec segment.
>>
>> In order to protect module data against execution, select
>> ARCH_WANTS_MODULES_DATA_IN_VMALLOC.
>>
>> For the 8xx (and possibly other 32 bits platform in the future),
>> there is no such constraint on Exec/NoExec protection, however
>> there is a critical distance between kernel functions and callers
>> that needs to remain below 32Mbytes in order to avoid costly
>> trampolines. By allocating data outside of module area, we
>> increase the chance for module text to remain within acceptable
>> distance from kernel core text.
>>
>> So select ARCH_WANTS_MODULES_DATA_IN_VMALLOC for 8xx as well.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Paul Mackerras <paulus@samba.org>
>
> Cc list first and then the SOB.
Just delete the Cc: list, it's meaningless.
cheers
^ permalink raw reply
* Re: [PATCH v3 3/6] modules: Introduce data_layout
From: Christophe Leroy @ 2022-02-03 6:58 UTC (permalink / raw)
To: Luis Chamberlain
Cc: linux-arch@vger.kernel.org, kgdb-bugreport@lists.sourceforge.net,
linux-kernel@vger.kernel.org, linux-mm@kvack.org, Jessica Yu,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <YfsYaDyqrFyVypkv@bombadil.infradead.org>
Le 03/02/2022 à 00:48, Luis Chamberlain a écrit :
> On Sat, Jan 29, 2022 at 05:02:07PM +0000, Christophe Leroy wrote:
>> diff --git a/kernel/module.c b/kernel/module.c
>> index 163e32e39064..11f51e17fb9f 100644
>> --- a/kernel/module.c
>> +++ b/kernel/module.c
>> @@ -81,6 +81,8 @@
>> /* If this is set, the section belongs in the init part of the module */
>> #define INIT_OFFSET_MASK (1UL << (BITS_PER_LONG-1))
>>
>> +#define data_layout core_layout
>> +
>> /*
>> * Mutex protects:
>> * 1) List of modules (also safely readable with preempt_disable),
>> @@ -2451,7 +2454,10 @@ static void layout_sections(struct module *mod, struct load_info *info)
>> || s->sh_entsize != ~0UL
>> || module_init_layout_section(sname))
>> continue;
>> - s->sh_entsize = get_offset(mod, &mod->core_layout.size, s, i);
>> + if (m)
>> + s->sh_entsize = get_offset(mod, &mod->data_layout.size, s, i);
>> + else
>> + s->sh_entsize = get_offset(mod, &mod->core_layout.size, s, i);
>> pr_debug("\t%s\n", sname);
>
> Huh why is this branching here, given you just used mod->data_layout in
> all other areas?
The module text remains in core_layout, so the text section still needs
core_layout. In the masks[][] table, it corresponds to the first line,
which has flag SHF_EXECINSTR. In the loop that's when 'm' is 0.
In the following switch/case, case 0 still uses core_layout.
>
>> @@ -3468,6 +3474,8 @@ static int move_module(struct module *mod, struct load_info *info)
>> if (shdr->sh_entsize & INIT_OFFSET_MASK)
>> dest = mod->init_layout.base
>> + (shdr->sh_entsize & ~INIT_OFFSET_MASK);
>> + else if (!(shdr->sh_flags & SHF_EXECINSTR))
>> + dest = mod->data_layout.base + shdr->sh_entsize;
>> else
>> dest = mod->core_layout.base + shdr->sh_entsize;
>>
>
> Likewise here.
Same here, the section with flag SHF_EXECINSTR is a text section, it
stays in core_layout.
Christophe
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox