* Re: [PATCH] powerpc/4xx: Fix setup_kuep() on SMP
From: Michael Ellerman @ 2021-06-29 11:58 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <b9c2a9add0f11754539e24c6f421bd2009327c36.1624863323.git.christophe.leroy@csgroup.eu>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> On SMP, setup_kuep() is also called from start_secondary() since
> commit 86f46f343272 ("powerpc/32s: Initialise KUAP and KUEP in C").
>
> start_secondary() is not an __init function.
>
> Remove the __init marker from setup_kuep() and bail out when
> not caller on the first CPU as the work is already done.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Fixes: 10248dcba120 ("powerpc/44x: Implement Kernel Userspace Exec Protection (KUEP)")
> Fixes: 86f46f343272 ("powerpc/32s: Initialise KUAP and KUEP in C").
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/mm/nohash/44x.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/mm/nohash/44x.c b/arch/powerpc/mm/nohash/44x.c
> index 7da6d1e9fc9b..20c18bd5b9a0 100644
> --- a/arch/powerpc/mm/nohash/44x.c
> +++ b/arch/powerpc/mm/nohash/44x.c
> @@ -241,8 +241,11 @@ void __init mmu_init_secondary(int cpu)
> #endif /* CONFIG_SMP */
>
> #ifdef CONFIG_PPC_KUEP
> -void __init setup_kuep(bool disabled)
> +void setup_kuep(bool disabled)
> {
> + if (smp_processor_id() != boot_cpuid)
> + return;
> +
> if (disabled)
> patch_instruction_site(&patch__tlb_44x_kuep, ppc_inst(PPC_RAW_NOP()));
> else
Building ppc44x_defconfig gives me:
/linux/arch/powerpc/mm/nohash/44x.c: In function 'setup_kuep':
/linux/arch/powerpc/mm/nohash/44x.c:246:35: error: 'boot_cpuid' undeclared (first use in this function); did you mean 'boot_cpu_init'?
246 | if (smp_processor_id() != boot_cpuid)
| ^~~~~~~~~~
| boot_cpu_init
/linux/arch/powerpc/mm/nohash/44x.c:246:35: note: each undeclared identifier is reported only once for each function it appears in
cheers
^ permalink raw reply
* Re: [PATCH] fpga: dfl: fme: Fix cpu hotplug issue in performance reporting
From: kajoljain @ 2021-06-29 8:08 UTC (permalink / raw)
To: Greg KH
Cc: mark.rutland, maddy, rnsastry, trix, linux-fpga, linuxppc-dev,
linux-kernel, stable, linux-perf-users, atrajeev, mdf, will,
yilun.xu, hao.wu
In-Reply-To: <YNrLRLyyUeDemxTS@kroah.com>
On 6/29/21 12:57 PM, Greg KH wrote:
> On Tue, Jun 29, 2021 at 12:45:20PM +0530, kajoljain wrote:
>>
>>
>> On 6/28/21 3:47 PM, Kajol Jain wrote:
>>> The performance reporting driver added cpu hotplug
>>> feature but it didn't add pmu migration call in cpu
>>> offline function.
>>> This can create an issue incase the current designated
>>> cpu being used to collect fme pmu data got offline,
>>> as based on current code we are not migrating fme pmu to
>>> new target cpu. Because of that perf will still try to
>>> fetch data from that offline cpu and hence we will not
>>> get counter data.
>>>
>>> Patch fixed this issue by adding pmu_migrate_context call
>>> in fme_perf_offline_cpu function.
>>>
>>
>> Adding stable@vger.kernel.org in cc list as suggested by Moritz Fischer.
>
>
> <formletter>
>
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree. Please read:
> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> for how to do this properly.
Thanks Greg for pointing it, I will take care in my next version.
Thanks,
Kajol Jain
>
> </formletter>
>
^ permalink raw reply
* Re: [PATCH] fpga: dfl: fme: Fix cpu hotplug issue in performance reporting
From: Greg KH @ 2021-06-29 7:27 UTC (permalink / raw)
To: kajoljain
Cc: mark.rutland, maddy, rnsastry, trix, linux-fpga, linuxppc-dev,
linux-kernel, stable, linux-perf-users, atrajeev, mdf, will,
yilun.xu, hao.wu
In-Reply-To: <adc3b013-d39b-a183-dfce-86ca857949b8@linux.ibm.com>
On Tue, Jun 29, 2021 at 12:45:20PM +0530, kajoljain wrote:
>
>
> On 6/28/21 3:47 PM, Kajol Jain wrote:
> > The performance reporting driver added cpu hotplug
> > feature but it didn't add pmu migration call in cpu
> > offline function.
> > This can create an issue incase the current designated
> > cpu being used to collect fme pmu data got offline,
> > as based on current code we are not migrating fme pmu to
> > new target cpu. Because of that perf will still try to
> > fetch data from that offline cpu and hence we will not
> > get counter data.
> >
> > Patch fixed this issue by adding pmu_migrate_context call
> > in fme_perf_offline_cpu function.
> >
>
> Adding stable@vger.kernel.org in cc list as suggested by Moritz Fischer.
<formletter>
This is not the correct way to submit patches for inclusion in the
stable kernel tree. Please read:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.
</formletter>
^ permalink raw reply
* Re: [PATCH] fpga: dfl: fme: Fix cpu hotplug issue in performance reporting
From: kajoljain @ 2021-06-29 7:15 UTC (permalink / raw)
To: will, hao.wu, mark.rutland
Cc: maddy, rnsastry, trix, linux-fpga, linux-kernel, stable,
linux-perf-users, atrajeev, mdf, linuxppc-dev, yilun.xu
In-Reply-To: <20210628101721.188991-1-kjain@linux.ibm.com>
On 6/28/21 3:47 PM, Kajol Jain wrote:
> The performance reporting driver added cpu hotplug
> feature but it didn't add pmu migration call in cpu
> offline function.
> This can create an issue incase the current designated
> cpu being used to collect fme pmu data got offline,
> as based on current code we are not migrating fme pmu to
> new target cpu. Because of that perf will still try to
> fetch data from that offline cpu and hence we will not
> get counter data.
>
> Patch fixed this issue by adding pmu_migrate_context call
> in fme_perf_offline_cpu function.
>
Adding stable@vger.kernel.org in cc list as suggested by Moritz Fischer.
Thanks,
Kajol Jain
> Fixes: 724142f8c42a ("fpga: dfl: fme: add performance reporting support")
> Tested-by: Xu Yilun <yilun.xu@intel.com>
> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
> ---
> drivers/fpga/dfl-fme-perf.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> ---
> Changelog:
> - Remove RFC tag
> - Did nits changes on subject and commit message as suggested by Xu Yilun
> - Added Tested-by tag
> - Link to rfc patch: https://lkml.org/lkml/2021/6/28/112
> ---
> diff --git a/drivers/fpga/dfl-fme-perf.c b/drivers/fpga/dfl-fme-perf.c
> index 4299145ef347..b9a54583e505 100644
> --- a/drivers/fpga/dfl-fme-perf.c
> +++ b/drivers/fpga/dfl-fme-perf.c
> @@ -953,6 +953,10 @@ static int fme_perf_offline_cpu(unsigned int cpu, struct hlist_node *node)
> return 0;
>
> priv->cpu = target;
> +
> + /* Migrate fme_perf pmu events to the new target cpu */
> + perf_pmu_migrate_context(&priv->pmu, cpu, target);
> +
> return 0;
> }
>
>
^ permalink raw reply
* Re: [RFC] fpga: dfl: fme: Fix cpu hotplug code
From: kajoljain @ 2021-06-29 7:14 UTC (permalink / raw)
To: Moritz Fischer
Cc: mark.rutland, maddy, luwei.kang, rnsastry, trix, linux-fpga,
linuxppc-dev, linux-kernel, atrajeev, will, yilun.xu, hao.wu
In-Reply-To: <YNoXnHq+nlJhO8o6@epycbox.lan>
On 6/29/21 12:10 AM, Moritz Fischer wrote:
> On Mon, Jun 28, 2021 at 12:45:46PM +0530, Kajol Jain wrote:
>> Commit 724142f8c42a ("fpga: dfl: fme: add performance
>> reporting support") added performance reporting support
>> for FPGA management engine via perf.
>>
>> It also added cpu hotplug feature but it didn't add
>> pmu migration call in cpu offline function.
>> This can create an issue incase the current designated
>> cpu being used to collect fme pmu data got offline,
>> as based on current code we are not migrating fme pmu to
>> new target cpu. Because of that perf will still try to
>> fetch data from that offline cpu and hence we will not
>> get counter data.
>>
>> Patch fixed this issue by adding pmu_migrate_context call
>> in fme_perf_offline_cpu function.
>>
>> Fixes: 724142f8c42a ("fpga: dfl: fme: add performance reporting support")
>> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
>
> You might want to Cc: stable@vger.kernel.org if it fixes an actual bug.
Hi Moritz,
I already send patch out without RFC tag yesterday.
Link to the patch: https://lkml.org/lkml/2021/6/28/275
I will cc stable@vger.kernel.org there as suggested by you.
Thanks,
Kajol Jain
>> ---
>> drivers/fpga/dfl-fme-perf.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> ---
>> - This fix patch is not tested (as I don't have required environment).
>> But issue mentioned in the commit msg can be re-created, by starting any
>> fme_perf event and while its still running, offline current designated
>> cpu pointed by cpumask file. Since current code didn't migrating pmu,
>> perf gonna try getting counts from that offlined cpu and hence we will
>> not get event data.
>> ---
>> diff --git a/drivers/fpga/dfl-fme-perf.c b/drivers/fpga/dfl-fme-perf.c
>> index 4299145ef347..b9a54583e505 100644
>> --- a/drivers/fpga/dfl-fme-perf.c
>> +++ b/drivers/fpga/dfl-fme-perf.c
>> @@ -953,6 +953,10 @@ static int fme_perf_offline_cpu(unsigned int cpu, struct hlist_node *node)
>> return 0;
>>
>> priv->cpu = target;
>> +
>> + /* Migrate fme_perf pmu events to the new target cpu */
>> + perf_pmu_migrate_context(&priv->pmu, cpu, target);
>> +
>> return 0;
>> }
>>
>> --
>> 2.31.1
>>
> - Moritz
>
^ permalink raw reply
* Re: [PATCH] perf script python: Fix buffer size to report iregs in perf script
From: kajoljain @ 2021-06-29 7:09 UTC (permalink / raw)
To: Paul A. Clarke
Cc: ravi.bangoria, atrajeev, rnsastry, linuxppc-dev, linux-kernel,
acme, linux-perf-users, maddy, jolsa
In-Reply-To: <20210628144937.GE142768@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com>
On 6/28/21 8:19 PM, Paul A. Clarke wrote:
> On Mon, Jun 28, 2021 at 11:53:41AM +0530, Kajol Jain wrote:
>> Commit 48a1f565261d ("perf script python: Add more PMU fields
>> to event handler dict") added functionality to report fields like
>> weight, iregs, uregs etc via perf report.
>> That commit predefined buffer size to 512 bytes to print those fields.
>>
>> But incase of powerpc, since we added extended regs support
>> in commits:
>>
>> Commit 068aeea3773a ("perf powerpc: Support exposing Performance Monitor
>> Counter SPRs as part of extended regs")
>> Commit d735599a069f ("powerpc/perf: Add extended regs support for
>> power10 platform")
>>
>> Now iregs can carry more bytes of data and this predefined buffer size
>> can result to data loss in perf script output.
>>
>> Patch resolve this issue by making buffer size dynamic based on number
>> of registers needed to print. It also changed return type for function
>> "regs_map" from int to void, as the return value is not being used by
>> the caller function "set_regs_in_dict".
>>
>> Fixes: 068aeea3773a ("perf powerpc: Support exposing Performance Monitor
>> Counter SPRs as part of extended regs")
>> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
>> ---
>> .../util/scripting-engines/trace-event-python.c | 17 ++++++++++++-----
>> 1 file changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
>> index 4e4aa4c97ac5..c8c9706b4643 100644
>> --- a/tools/perf/util/scripting-engines/trace-event-python.c
>> +++ b/tools/perf/util/scripting-engines/trace-event-python.c
> [...]
>> @@ -713,7 +711,16 @@ static void set_regs_in_dict(PyObject *dict,
>> struct evsel *evsel)
>> {
>> struct perf_event_attr *attr = &evsel->core.attr;
>> - char bf[512];
>> +
>> + /*
>> + * Here value 28 is a constant size which can be used to print
>> + * one register value and its corresponds to:
>> + * 16 chars is to specify 64 bit register in hexadecimal.
>> + * 2 chars is for appending "0x" to the hexadecimal value and
>> + * 10 chars is for register name.
>> + */
>> + int size = __sw_hweight64(attr->sample_regs_intr) * 28;
>> + char bf[size];
>
> I propose using a template rather than a magic number here. Something like:
> const char reg_name_tmpl[] = "10 chars ";
> const char reg_value_tmpl[] = "0x0123456789abcdef";
> const int size = __sw_hweight64(attr->sample_regs_intr) +
> sizeof reg_name_tmpl + sizeof reg_value_tmpl;
>
Hi Paul,
Thanks for reviewing the patch. Yes these are
some standardization we can do by creating macros for different
fields.
The basic idea is, we want to provide significant buffer size
based on number of registers present in sample_regs_intr to accommodate
all data.
But before going to optimizing code, Arnaldo/Jiri, is this approach looks good to you?
> Pardon my ignorance, but is there no separation/whitespace between the name
> and the value?
This is how we will get data via perf script
r0:0xc000000000112008
r1:0xc000000023b37920
r2:0xc00000000144c900
r3:0xc0000000bc566120
r4:0xc0000000c5600000
r5:0x2606c6506ca
r6:0xc000000023b378f8
r7:0xfffffd9f93a48f0e
.....
And is there some significance to 10 characters for the
> register name, or is that a magic number?
Most of the register name are within 10 characters, basically we are giving this
magic number to make sure we have enough space in buffer to contain all registers
name with colon.
Thanks,
Kajol Jain
>
> PC
>
^ permalink raw reply
* Re: [PATCH] powerpc/64s: fix hash page fault interrupt handler
From: Sachin Sant @ 2021-06-29 4:56 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev
In-Reply-To: <20210629015057.1635519-1-npiggin@gmail.com>
> On 29-Jun-2021, at 7:20 AM, Nicholas Piggin <npiggin@gmail.com> wrote:
>
> The early bad fault or key fault test in do_hash_fault() ends up calling
> into ___do_page_fault without having gone through an interrupt handler
> wrapper (except the initial _RAW one). This can end up calling local irq
> functions while the interrupt has not been reconciled, which will likely
> cause crashes and it trips up on a later patch that adds more assertions.
>
> pkey_exec_prot from selftests causes this path to be executed.
>
> There is no real reason to run the in_nmi() test should be performed
> before the key fault check. In fact if a perf interrupt in the hash
> fault code did a stack walk that was made to take a key fault somehow
> then running ___do_page_fault could possibly cause another hash fault
> causing problems. Move the in_nmi() test first, and then do everything
> else inside the regular interrupt handler function.
>
> Fixes: 3a96570ffceb ("powerpc: convert interrupt handlers to use wrappers")
> Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
Thanks Nick.
Fixes the reported problem.
Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
-Sachin
^ permalink raw reply
* Re: [PATCH] Documentation: PCI: pci-error-recovery: rearrange the general sequence
From: Wesley Sheng @ 2021-06-29 3:34 UTC (permalink / raw)
To: Oliver O'Halloran, linasvepstas, ruscur, bhelgaas, corbet,
linux-pci, linuxppc-dev, linux-doc, linux-kernel
Cc: wesley.sheng, wesleyshenggit
In-Reply-To: <CAOSf1CHaLCAsnB42Je+ynJ6xv-M8qmScbfOLSHVze7D4fEh66Q@mail.gmail.com>
On Fri, Jun 18, 2021 at 05:21:32PM +1000, Oliver O'Halloran wrote:
> On Fri, Jun 18, 2021 at 4:05 PM Wesley Sheng <wesley.sheng@amd.com> wrote:
> >
> > Reset_link() callback function was called before mmio_enabled() in
> > pcie_do_recovery() function actually, so rearrange the general
> > sequence betwen step 2 and step 3 accordingly.
>
> I don't think this is true in all cases. If pcie_do_recovery() is
> called with state==pci_channel_io_normal (i.e. non-fatal AER) the link
> won't be reset. EEH (ppc PCI error recovery thing) also uses
> .mmio_enabled() as described.
Yes, in case of non-fatal AER, reset_link() callback (aer_root_reset() for
AER and dpc_reset_link() for DPC) will not be invoked. And if
.error_detected() return PCI_ERS_RESULT_CAN_RECOVER, .mmio_enabled() be
called followed.
But if pcie_do_recovery() is called with state == pci_channel_io_frozen,
reset_link() callback is called after .error_detected() but before
.mmio_enabled(). So I thought Step 2: MMIO Enabled and Step 3: Link Reset
should rearrange their sequence.
^ permalink raw reply
* [PATCH] powerpc/64s: fix hash page fault interrupt handler
From: Nicholas Piggin @ 2021-06-29 1:50 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Sachin Sant, Nicholas Piggin
The early bad fault or key fault test in do_hash_fault() ends up calling
into ___do_page_fault without having gone through an interrupt handler
wrapper (except the initial _RAW one). This can end up calling local irq
functions while the interrupt has not been reconciled, which will likely
cause crashes and it trips up on a later patch that adds more assertions.
pkey_exec_prot from selftests causes this path to be executed.
There is no real reason to run the in_nmi() test should be performed
before the key fault check. In fact if a perf interrupt in the hash
fault code did a stack walk that was made to take a key fault somehow
then running ___do_page_fault could possibly cause another hash fault
causing problems. Move the in_nmi() test first, and then do everything
else inside the regular interrupt handler function.
Fixes: 3a96570ffceb ("powerpc: convert interrupt handlers to use wrappers")
Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/mm/book3s64/hash_utils.c | 24 +++++++++++-------------
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/mm/book3s64/hash_utils.c b/arch/powerpc/mm/book3s64/hash_utils.c
index 96d9aa164007..ac5720371c0d 100644
--- a/arch/powerpc/mm/book3s64/hash_utils.c
+++ b/arch/powerpc/mm/book3s64/hash_utils.c
@@ -1522,8 +1522,8 @@ int hash_page(unsigned long ea, unsigned long access, unsigned long trap,
}
EXPORT_SYMBOL_GPL(hash_page);
-DECLARE_INTERRUPT_HANDLER_RET(__do_hash_fault);
-DEFINE_INTERRUPT_HANDLER_RET(__do_hash_fault)
+DECLARE_INTERRUPT_HANDLER(__do_hash_fault);
+DEFINE_INTERRUPT_HANDLER(__do_hash_fault)
{
unsigned long ea = regs->dar;
unsigned long dsisr = regs->dsisr;
@@ -1533,6 +1533,11 @@ DEFINE_INTERRUPT_HANDLER_RET(__do_hash_fault)
unsigned int region_id;
long err;
+ if (unlikely(dsisr & (DSISR_BAD_FAULT_64S | DSISR_KEYFAULT))) {
+ hash__do_page_fault(regs);
+ return;
+ }
+
region_id = get_region_id(ea);
if ((region_id == VMALLOC_REGION_ID) || (region_id == IO_REGION_ID))
mm = &init_mm;
@@ -1571,9 +1576,10 @@ DEFINE_INTERRUPT_HANDLER_RET(__do_hash_fault)
bad_page_fault(regs, SIGBUS);
}
err = 0;
- }
- return err;
+ } else if (err) {
+ hash__do_page_fault(regs);
+ }
}
/*
@@ -1582,13 +1588,6 @@ DEFINE_INTERRUPT_HANDLER_RET(__do_hash_fault)
*/
DEFINE_INTERRUPT_HANDLER_RAW(do_hash_fault)
{
- unsigned long dsisr = regs->dsisr;
-
- if (unlikely(dsisr & (DSISR_BAD_FAULT_64S | DSISR_KEYFAULT))) {
- hash__do_page_fault(regs);
- return 0;
- }
-
/*
* If we are in an "NMI" (e.g., an interrupt when soft-disabled), then
* don't call hash_page, just fail the fault. This is required to
@@ -1607,8 +1606,7 @@ DEFINE_INTERRUPT_HANDLER_RAW(do_hash_fault)
return 0;
}
- if (__do_hash_fault(regs))
- hash__do_page_fault(regs);
+ __do_hash_fault(regs);
return 0;
}
--
2.23.0
^ permalink raw reply related
* Re: [PATCH v5 4/6] powerpc/pseries: Consolidate different NUMA distance update code paths
From: kernel test robot @ 2021-06-28 20:40 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev, mpe
Cc: Nathan Lynch, Aneesh Kumar K.V, Daniel Henrique Barboza,
kbuild-all, David Gibson
In-Reply-To: <20210628151117.545935-5-aneesh.kumar@linux.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 6287 bytes --]
Hi "Aneesh,
I love your patch! Yet something to improve:
[auto build test ERROR on powerpc/next]
[also build test ERROR on v5.13 next-20210628]
[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/Aneesh-Kumar-K-V/Add-support-for-FORM2-associativity/20210628-231546
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-randconfig-r024-20210628 (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 9.3.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/fcbc8b19e99b1cf44fde904817f19616c6baecdb
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Aneesh-Kumar-K-V/Add-support-for-FORM2-associativity/20210628-231546
git checkout fcbc8b19e99b1cf44fde904817f19616c6baecdb
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=powerpc
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/mm/numa.c:298:6: warning: no previous prototype for 'update_numa_distance' [-Wmissing-prototypes]
298 | void update_numa_distance(struct device_node *node)
| ^~~~~~~~~~~~~~~~~~~~
arch/powerpc/mm/numa.c: In function 'parse_numa_properties':
>> arch/powerpc/mm/numa.c:809:7: error: implicit declaration of function '__vphn_get_associativity'; did you mean 'of_get_associativity'? [-Werror=implicit-function-declaration]
809 | if (__vphn_get_associativity(i, vphn_assoc) == 0) {
| ^~~~~~~~~~~~~~~~~~~~~~~~
| of_get_associativity
cc1: some warnings being treated as errors
vim +809 arch/powerpc/mm/numa.c
771
772 static int __init parse_numa_properties(void)
773 {
774 struct device_node *memory;
775 int default_nid = 0;
776 unsigned long i;
777 const __be32 *associativity;
778
779 if (numa_enabled == 0) {
780 printk(KERN_WARNING "NUMA disabled by user\n");
781 return -1;
782 }
783
784 primary_domain_index = find_primary_domain_index();
785
786 if (primary_domain_index < 0) {
787 /*
788 * if we fail to parse primary_domain_index from device tree
789 * mark the numa disabled, boot with numa disabled.
790 */
791 numa_enabled = false;
792 return primary_domain_index;
793 }
794
795 dbg("NUMA associativity depth for CPU/Memory: %d\n", primary_domain_index);
796
797 /*
798 * Even though we connect cpus to numa domains later in SMP
799 * init, we need to know the node ids now. This is because
800 * each node to be onlined must have NODE_DATA etc backing it.
801 */
802 for_each_present_cpu(i) {
803 __be32 vphn_assoc[VPHN_ASSOC_BUFSIZE];
804 struct device_node *cpu;
805 int nid = NUMA_NO_NODE;
806
807 memset(vphn_assoc, 0, VPHN_ASSOC_BUFSIZE * sizeof(__be32));
808
> 809 if (__vphn_get_associativity(i, vphn_assoc) == 0) {
810 nid = associativity_to_nid(vphn_assoc);
811 __initialize_form1_numa_distance(vphn_assoc);
812 } else {
813
814 /*
815 * Don't fall back to default_nid yet -- we will plug
816 * cpus into nodes once the memory scan has discovered
817 * the topology.
818 */
819 cpu = of_get_cpu_node(i, NULL);
820 BUG_ON(!cpu);
821
822 associativity = of_get_associativity(cpu);
823 if (associativity) {
824 nid = associativity_to_nid(associativity);
825 __initialize_form1_numa_distance(associativity);
826 }
827 of_node_put(cpu);
828 }
829
830 node_set_online(nid);
831 }
832
833 get_n_mem_cells(&n_mem_addr_cells, &n_mem_size_cells);
834
835 for_each_node_by_type(memory, "memory") {
836 unsigned long start;
837 unsigned long size;
838 int nid;
839 int ranges;
840 const __be32 *memcell_buf;
841 unsigned int len;
842
843 memcell_buf = of_get_property(memory,
844 "linux,usable-memory", &len);
845 if (!memcell_buf || len <= 0)
846 memcell_buf = of_get_property(memory, "reg", &len);
847 if (!memcell_buf || len <= 0)
848 continue;
849
850 /* ranges in cell */
851 ranges = (len >> 2) / (n_mem_addr_cells + n_mem_size_cells);
852 new_range:
853 /* these are order-sensitive, and modify the buffer pointer */
854 start = read_n_cells(n_mem_addr_cells, &memcell_buf);
855 size = read_n_cells(n_mem_size_cells, &memcell_buf);
856
857 /*
858 * Assumption: either all memory nodes or none will
859 * have associativity properties. If none, then
860 * everything goes to default_nid.
861 */
862 associativity = of_get_associativity(memory);
863 if (associativity) {
864 nid = associativity_to_nid(associativity);
865 __initialize_form1_numa_distance(associativity);
866 } else
867 nid = default_nid;
868
869 fake_numa_create_new_node(((start + size) >> PAGE_SHIFT), &nid);
870 node_set_online(nid);
871
872 size = numa_enforce_memory_limit(start, size);
873 if (size)
874 memblock_set_node(start, size, &memblock.memory, nid);
875
876 if (--ranges)
877 goto new_range;
878 }
879
880 /*
881 * Now do the same thing for each MEMBLOCK listed in the
882 * ibm,dynamic-memory property in the
883 * ibm,dynamic-reconfiguration-memory node.
884 */
885 memory = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
886 if (memory) {
887 walk_drmem_lmbs(memory, NULL, numa_setup_drmem_lmb);
888 of_node_put(memory);
889 }
890
891 return 0;
892 }
893
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 43062 bytes --]
^ permalink raw reply
* Re: [PATCH v5 4/6] powerpc/pseries: Consolidate different NUMA distance update code paths
From: kernel test robot @ 2021-06-28 20:21 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev, mpe
Cc: Nathan Lynch, Aneesh Kumar K.V, Daniel Henrique Barboza,
kbuild-all, David Gibson
In-Reply-To: <20210628151117.545935-5-aneesh.kumar@linux.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 2187 bytes --]
Hi "Aneesh,
I love your patch! Perhaps something to improve:
[auto build test WARNING on powerpc/next]
[also build test WARNING on v5.13 next-20210628]
[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/Aneesh-Kumar-K-V/Add-support-for-FORM2-associativity/20210628-231546
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-allyesconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 9.3.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/fcbc8b19e99b1cf44fde904817f19616c6baecdb
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Aneesh-Kumar-K-V/Add-support-for-FORM2-associativity/20210628-231546
git checkout fcbc8b19e99b1cf44fde904817f19616c6baecdb
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=powerpc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> arch/powerpc/mm/numa.c:298:6: warning: no previous prototype for 'update_numa_distance' [-Wmissing-prototypes]
298 | void update_numa_distance(struct device_node *node)
| ^~~~~~~~~~~~~~~~~~~~
vim +/update_numa_distance +298 arch/powerpc/mm/numa.c
294
295 /*
296 * Used to update distance information w.r.t newly added node.
297 */
> 298 void update_numa_distance(struct device_node *node)
299 {
300 if (affinity_form == FORM0_AFFINITY)
301 return;
302 else if (affinity_form == FORM1_AFFINITY) {
303 initialize_form1_numa_distance(node);
304 return;
305 }
306 }
307
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 73412 bytes --]
^ permalink raw reply
* Re: [PATCH 5/8] powerpc/64: enable MSR[EE] in irq replay pt_regs
From: Nicholas Piggin @ 2021-06-28 20:15 UTC (permalink / raw)
To: Sachin Sant; +Cc: linuxppc-dev
In-Reply-To: <2DCB27B7-04A8-461E-B6EA-1E6161CCDA3F@linux.vnet.ibm.com>
Excerpts from Sachin Sant's message of June 29, 2021 12:37 am:
>
>> On 28-Jun-2021, at 1:19 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
>>
>> Similar to 2b48e96be2f9f ("powerpc/64: fix irq replay pt_regs->softe
>> value"), enable MSR_EE in pt_regs->msr, which makes the regs look a
>> bit more normal and allows the extra debug checks to be added to
>> interrupt handler entry.
>>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>> arch/powerpc/include/asm/interrupt.h | 4 ++++
>> arch/powerpc/kernel/irq.c | 1 +
>> 2 files changed, 5 insertions(+)
>>
>> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
>> index 789311d1e283..d4bdf7d274ac 100644
>> --- a/arch/powerpc/include/asm/interrupt.h
>> +++ b/arch/powerpc/include/asm/interrupt.h
>> @@ -173,6 +173,8 @@ static inline void interrupt_enter_prepare(struct pt_regs *regs, struct interrup
>> BUG_ON(search_kernel_restart_table(regs->nip));
>> #endif
>> }
>> + if (IS_ENABLED(CONFIG_PPC_IRQ_SOFT_MASK_DEBUG))
>> + BUG_ON(!arch_irq_disabled_regs(regs) && !(regs->msr & MSR_EE));
>> #endif
>
> I think this BUG_ON was triggered while running selftests (powerpc/mm/pkey_exec_prot)
>
> [ 9741.254969] ------------[ cut here ]------------
> [ 9741.254978] kernel BUG at arch/powerpc/include/asm/interrupt.h:177!
> [ 9741.254985] Oops: Exception in kernel mode, sig: 5 [#1]
> [ 9741.254990] LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
> [ 9741.254995] Modules linked in: rpadlpar_io rpaphp uinput sha512_generic vmac n_gsm pps_ldisc pps_core ppp_synctty ppp_async ppp_generic slcan slip slhc snd_hrtimer snd_seq snd_seq_device snd_timer snd soundcore authenc pcrypt crypto_user n_hdlc dummy veth nfsv3 nfs_acl nfs lockd grace fscache netfs tun brd overlay vfat fat btrfs blake2b_generic xor zstd_compress raid6_pq xfs loop sctp ip6_udp_tunnel udp_tunnel dm_mod bonding nft_ct nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip_set rfkill nf_tables libcrc32c nfnetlink sunrpc pseries_rng xts vmx_crypto uio_pdrv_genirq uio sch_fq_codel ip_tables ext4 mbcache jbd2 sr_mod sd_mod cdrom t10_pi sg ibmvscsi ibmveth scsi_transport_srp fuse [last unloaded: test_cpuidle_latency]
> [ 9741.255097] CPU: 17 PID: 3278920 Comm: pkey_exec_prot Tainted: G W OE 5.13.0-rc7-next-20210625-dirty #4
> [ 9741.255106] NIP: c0000000000300d8 LR: c000000000009604 CTR: c000000000009330
> [ 9741.255111] REGS: c0000000347536f0 TRAP: 0700 Tainted: G W OE (5.13.0-rc7-next-20210625-dirty)
> [ 9741.255117] MSR: 8000000000021033 <SF,ME,IR,DR,RI,LE> CR: 22004282 XER: 20040000
> [ 9741.255130] CFAR: c00000000003007c IRQMASK: 3
> [ 9741.255130] GPR00: c000000000093cd0 c000000034753990 c0000000029bbe00 c000000034753a30
> [ 9741.255130] GPR04: 00007fff9ebb0000 0000000000200000 000000000000000a 000000000000002d
> [ 9741.255130] GPR08: 0000000000000000 0000000000000001 0000000000000000 7265677368657265
> [ 9741.255130] GPR12: 8000000000021033 c00000001ec27280 0000000000000000 0000000000000000
> [ 9741.255130] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> [ 9741.255130] GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000010003c40
> [ 9741.255130] GPR24: 0000000000000000 0000000000000000 0000000000200000 c00000005e89d200
> [ 9741.255130] GPR28: 0000000000000300 00007fff9ebb0000 c000000034753e80 c000000034753a30
> [ 9741.255191] NIP [c0000000000300d8] program_check_exception+0xe8/0x1c0
> [ 9741.255202] LR [c000000000009604] program_check_common_virt+0x2d4/0x320
> [ 9741.255209] Call Trace:
> [ 9741.255212] [c000000034753990] [0000000000000008] 0x8 (unreliable)
> [ 9741.255219] [c0000000347539c0] [c000000034753a80] 0xc000000034753a80
> [ 9741.255225] --- interrupt: 700 at arch_local_irq_restore+0x1d0/0x200
> [ 9741.255231] NIP: c000000000016790 LR: c000000000093388 CTR: c000000000008780
> [ 9741.255236] REGS: c000000034753a30 TRAP: 0700 Tainted: G W OE (5.13.0-rc7-next-20210625-dirty)
> [ 9741.255242] MSR: 8000000000021033 <SF,ME,IR,DR,RI,LE> CR: 24004288 XER: 20040000
> [ 9741.255253] CFAR: c0000000000165ec IRQMASK: 0
> [ 9741.255253] GPR00: c000000000093cd0 c000000034753cd0 c0000000029bbe00 0000000000000000
> [ 9741.255253] GPR04: 00007fff9ebb0000 0000000000200000 000000000000000a 000000000000002d
> [ 9741.255253] GPR08: 0000000000000000 0000000000000000 c0000000bd77d400 7265677368657265
> [ 9741.255253] GPR12: 0000000044000282 c00000001ec27280 0000000000000000 0000000000000000
> [ 9741.255253] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> [ 9741.255253] GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000010003c40
> [ 9741.255253] GPR24: 0000000000000000 0000000000000000 0000000000200000 c00000005e89d200
> [ 9741.255253] GPR28: 0000000000000300 00007fff9ebb0000 c000000034753e80 0000000000000001
> [ 9741.255313] NIP [c000000000016790] arch_local_irq_restore+0x1d0/0x200
> [ 9741.255319] LR [c000000000093388] ___do_page_fault+0x438/0xb80
> [ 9741.255325] --- interrupt: 700
> [ 9741.255328] [c000000034753cd0] [c00000000009be74] hash_page_mm+0x5e4/0x800 (unreliable)
> [ 9741.255335] [c000000034753d00] [000000000000002d] 0x2d
> [ 9741.255340] [c000000034753db0] [c000000000093cd0] hash__do_page_fault+0x30/0x70
> [ 9741.255348] [c000000034753de0] [c00000000009c438] do_hash_fault+0x78/0xb0
This looks like it's probably running un-reconciled due to the first
call to hash__do_page_fault not calling a real interrupt handler.
Without this patch, the test must be causing a warning due to the same
thing probably (the bug triggered in the program check interrupt handler).
I think this patch is probably the right fix for it.
Thanks,
Nick
---
diff --git a/arch/powerpc/mm/book3s64/hash_utils.c b/arch/powerpc/mm/book3s64/hash_utils.c
index 96d9aa164007..ac5720371c0d 100644
--- a/arch/powerpc/mm/book3s64/hash_utils.c
+++ b/arch/powerpc/mm/book3s64/hash_utils.c
@@ -1522,8 +1522,8 @@ int hash_page(unsigned long ea, unsigned long access, unsigned long trap,
}
EXPORT_SYMBOL_GPL(hash_page);
-DECLARE_INTERRUPT_HANDLER_RET(__do_hash_fault);
-DEFINE_INTERRUPT_HANDLER_RET(__do_hash_fault)
+DECLARE_INTERRUPT_HANDLER(__do_hash_fault);
+DEFINE_INTERRUPT_HANDLER(__do_hash_fault)
{
unsigned long ea = regs->dar;
unsigned long dsisr = regs->dsisr;
@@ -1533,6 +1533,11 @@ DEFINE_INTERRUPT_HANDLER_RET(__do_hash_fault)
unsigned int region_id;
long err;
+ if (unlikely(dsisr & (DSISR_BAD_FAULT_64S | DSISR_KEYFAULT))) {
+ hash__do_page_fault(regs);
+ return;
+ }
+
region_id = get_region_id(ea);
if ((region_id == VMALLOC_REGION_ID) || (region_id == IO_REGION_ID))
mm = &init_mm;
@@ -1571,9 +1576,10 @@ DEFINE_INTERRUPT_HANDLER_RET(__do_hash_fault)
bad_page_fault(regs, SIGBUS);
}
err = 0;
- }
- return err;
+ } else if (err) {
+ hash__do_page_fault(regs);
+ }
}
/*
@@ -1582,13 +1588,6 @@ DEFINE_INTERRUPT_HANDLER_RET(__do_hash_fault)
*/
DEFINE_INTERRUPT_HANDLER_RAW(do_hash_fault)
{
- unsigned long dsisr = regs->dsisr;
-
- if (unlikely(dsisr & (DSISR_BAD_FAULT_64S | DSISR_KEYFAULT))) {
- hash__do_page_fault(regs);
- return 0;
- }
-
/*
* If we are in an "NMI" (e.g., an interrupt when soft-disabled), then
* don't call hash_page, just fail the fault. This is required to
@@ -1607,8 +1606,7 @@ DEFINE_INTERRUPT_HANDLER_RAW(do_hash_fault)
return 0;
}
- if (__do_hash_fault(regs))
- hash__do_page_fault(regs);
+ __do_hash_fault(regs);
return 0;
}
^ permalink raw reply related
* Re: [RFC] fpga: dfl: fme: Fix cpu hotplug code
From: Moritz Fischer @ 2021-06-28 18:40 UTC (permalink / raw)
To: Kajol Jain
Cc: mark.rutland, maddy, luwei.kang, rnsastry, trix, linux-fpga,
linuxppc-dev, linux-kernel, atrajeev, mdf, will, yilun.xu, hao.wu
In-Reply-To: <20210628071546.167088-1-kjain@linux.ibm.com>
On Mon, Jun 28, 2021 at 12:45:46PM +0530, Kajol Jain wrote:
> Commit 724142f8c42a ("fpga: dfl: fme: add performance
> reporting support") added performance reporting support
> for FPGA management engine via perf.
>
> It also added cpu hotplug feature but it didn't add
> pmu migration call in cpu offline function.
> This can create an issue incase the current designated
> cpu being used to collect fme pmu data got offline,
> as based on current code we are not migrating fme pmu to
> new target cpu. Because of that perf will still try to
> fetch data from that offline cpu and hence we will not
> get counter data.
>
> Patch fixed this issue by adding pmu_migrate_context call
> in fme_perf_offline_cpu function.
>
> Fixes: 724142f8c42a ("fpga: dfl: fme: add performance reporting support")
> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
You might want to Cc: stable@vger.kernel.org if it fixes an actual bug.
> ---
> drivers/fpga/dfl-fme-perf.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> ---
> - This fix patch is not tested (as I don't have required environment).
> But issue mentioned in the commit msg can be re-created, by starting any
> fme_perf event and while its still running, offline current designated
> cpu pointed by cpumask file. Since current code didn't migrating pmu,
> perf gonna try getting counts from that offlined cpu and hence we will
> not get event data.
> ---
> diff --git a/drivers/fpga/dfl-fme-perf.c b/drivers/fpga/dfl-fme-perf.c
> index 4299145ef347..b9a54583e505 100644
> --- a/drivers/fpga/dfl-fme-perf.c
> +++ b/drivers/fpga/dfl-fme-perf.c
> @@ -953,6 +953,10 @@ static int fme_perf_offline_cpu(unsigned int cpu, struct hlist_node *node)
> return 0;
>
> priv->cpu = target;
> +
> + /* Migrate fme_perf pmu events to the new target cpu */
> + perf_pmu_migrate_context(&priv->pmu, cpu, target);
> +
> return 0;
> }
>
> --
> 2.31.1
>
- Moritz
^ permalink raw reply
* Re: [PATCH v4 7/7] powerpc/pseries: Add support for FORM2 associativity
From: Aneesh Kumar K.V @ 2021-06-28 15:04 UTC (permalink / raw)
To: David Gibson
Cc: Nathan Lynch, nvdimm, Daniel Henrique Barboza, dan.j.williams,
linuxppc-dev
In-Reply-To: <YNlpnOPu0ZLu/7nY@yekko>
David Gibson <david@gibson.dropbear.id.au> writes:
> On Thu, Jun 24, 2021 at 01:50:34PM +0530, Aneesh Kumar K.V wrote:
>> David Gibson <david@gibson.dropbear.id.au> writes:
>>
>> > On Thu, Jun 17, 2021 at 10:21:05PM +0530, Aneesh Kumar K.V wrote:
>> >> PAPR interface currently supports two different ways of communicating resource
>> >> grouping details to the OS. These are referred to as Form 0 and Form 1
>> >> associativity grouping. Form 0 is the older format and is now considered
>> >> deprecated. This patch adds another resource grouping named FORM2.
>> >>
>> >> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>> >> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> >> ---
>> >> Documentation/powerpc/associativity.rst | 135 ++++++++++++++++++++
>> >> arch/powerpc/include/asm/firmware.h | 3 +-
>> >> arch/powerpc/include/asm/prom.h | 1 +
>> >> arch/powerpc/kernel/prom_init.c | 3 +-
>> >> arch/powerpc/mm/numa.c | 149 +++++++++++++++++++++-
>> >> arch/powerpc/platforms/pseries/firmware.c | 1 +
>> >> 6 files changed, 286 insertions(+), 6 deletions(-)
>> >> create mode 100644 Documentation/powerpc/associativity.rst
>> >>
>> >> diff --git a/Documentation/powerpc/associativity.rst b/Documentation/powerpc/associativity.rst
>> >> new file mode 100644
>> >> index 000000000000..93be604ac54d
>> >> --- /dev/null
>> >> +++ b/Documentation/powerpc/associativity.rst
>> >> @@ -0,0 +1,135 @@
>> >> +============================
>> >> +NUMA resource associativity
>> >> +=============================
>> >> +
>> >> +Associativity represents the groupings of the various platform resources into
>> >> +domains of substantially similar mean performance relative to resources outside
>> >> +of that domain. Resources subsets of a given domain that exhibit better
>> >> +performance relative to each other than relative to other resources subsets
>> >> +are represented as being members of a sub-grouping domain. This performance
>> >> +characteristic is presented in terms of NUMA node distance within the Linux kernel.
>> >> +From the platform view, these groups are also referred to as domains.
>> >> +
>> >> +PAPR interface currently supports different ways of communicating these resource
>> >> +grouping details to the OS. These are referred to as Form 0, Form 1 and Form2
>> >> +associativity grouping. Form 0 is the older format and is now considered deprecated.
>> >> +
>> >> +Hypervisor indicates the type/form of associativity used via "ibm,arcitecture-vec-5 property".
>> >> +Bit 0 of byte 5 in the "ibm,architecture-vec-5" property indicates usage of Form 0 or Form 1.
>> >> +A value of 1 indicates the usage of Form 1 associativity. For Form 2 associativity
>> >> +bit 2 of byte 5 in the "ibm,architecture-vec-5" property is used.
>> >> +
>> >> +Form 0
>> >> +-----
>> >> +Form 0 associativity supports only two NUMA distance (LOCAL and REMOTE).
>> >> +
>> >> +Form 1
>> >> +-----
>> >> +With Form 1 a combination of ibm,associativity-reference-points and ibm,associativity
>> >> +device tree properties are used to determine the NUMA distance between resource groups/domains.
>> >> +
>> >> +The “ibm,associativity” property contains one or more lists of numbers (domainID)
>> >> +representing the resource’s platform grouping domains.
>> >> +
>> >> +The “ibm,associativity-reference-points” property contains one or more list of numbers
>> >> +(domainID index) that represents the 1 based ordinal in the associativity lists.
>> >> +The list of domainID index represnets increasing hierachy of
>> >> resource grouping.
>> >
>> > Typo "represnets". Also s/hierachy/hierarchy/
>> >
>> >> +
>> >> +ex:
>> >> +{ primary domainID index, secondary domainID index, tertiary domainID index.. }
>> >
>> >> +Linux kernel uses the domainID at the primary domainID index as the NUMA node id.
>> >> +Linux kernel computes NUMA distance between two domains by recursively comparing
>> >> +if they belong to the same higher-level domains. For mismatch at every higher
>> >> +level of the resource group, the kernel doubles the NUMA distance between the
>> >> +comparing domains.
>> >
>> > The Form1 description is still kinda confusing, but I don't really
>> > care. Form1 *is* confusing, it's Form2 that I hope will be clearer.
>> >
>> >> +
>> >> +Form 2
>> >> +-------
>> >> +Form 2 associativity format adds separate device tree properties representing NUMA node distance
>> >> +thereby making the node distance computation flexible. Form 2 also allows flexible primary
>> >> +domain numbering. With numa distance computation now detached from the index value of
>> >> +"ibm,associativity" property, Form 2 allows a large number of primary domain ids at the
>> >> +same domainID index representing resource groups of different performance/latency characteristics.
>> >
>> > So, see you've removed the special handling of secondary IDs for pmem
>> > - big improvement, thanks. IIUC, in this revised version, for Form2
>> > there's really no reason for ibm,associativity-reference-points to
>> > have >1 entry. Is that right?
>> >
>> > In Form2 everything revolves around the primary domain ID - so much
>> > that I suggest we come up with a short name for it. How about just
>> > "node id" since that's how Linux uses it.
>>
>> We don't really refer primary domain ID in rest of FORM2 details. We
>> do refer the entries as domainID.
>
> Right, which is unnecessarily confusing, because primary domain ID is
> going to be the only one that matters in practice.
>
>> Is renaming domainID to NUMA
>> node id better?
>>
>> >
>> >> +Hypervisor indicates the usage of FORM2 associativity using bit 2 of byte 5 in the
>> >> +"ibm,architecture-vec-5" property.
>> >> +
>> >> +"ibm,numa-lookup-index-table" property contains one or more list numbers representing
>> >> +the domainIDs present in the system. The offset of the domainID in this property is considered
>> >> +the domainID index.
>> >
>> > The lookup-index-table is all about compactifying the representation
>> > of the distance matrix. Because node ids are sparse, the matrix of
>> > distances is also effectively sparse. You don't want to have a huge
>> > matrix in the DT with mostly meaningless entries, so you use the
>> > lookup table to compact the node ids.
>> >
>> > It's a bit confusing though, because you now have two representations
>> > of exactly the same information the "full" node id (== primary
>> > domainID) and the "short" node id (==domainID lookup table offset).
>> >
>> > I can see three ways to clarify this:
>> >
>> > A) Give the short node id an explicit name rather than difficult to
>> > parse "domainID index" or "domainID offset" which gets easily
>> > muddled with other uses of index and offset. Use that name
>> > throughout.
>>
>> I dopped the domainID index and started referring to that as domain
>> distance offset.
>
> Better, since it's not ambiguous. Still kinda long and awkward.
>
>> > B) Eliminate the short ID entirely, and use an explicit sparse
>> > matrix representation for the distance table e.g. a list of
>> > (nodeA, nodeB, distance) tuples
>> >
>> > That does have the disadvantage that it's not structurally
>> > guaranteed that you have entries for every pair of "active" node
>> > ids.
>>
>> Other hypervisor would want to have a large possible domainID value and
>> mostly want to publish the full topology details during boot. Using the
>> above format makes a 16 node config have large "ibm,numa-distance-table"
>> property.
>
> It'd be about 3kiB, yes? That's fairly large, but not ludicrously
> large, so does that matter?
There is also the possibility that the 16 max node limit will get
updated in the future to a higher value.
>
>> > C) Eliminate the "long ID" entirely. In the Form2 world, is there
>> > actually any point allowing sparse node ids? In this case you'd
>> > have the node id read from associativity and use that directly to
>> > index the distance table
>>
>> Yes, other hypervisor would like to partition the domain ID space for
>> different resources.
>
> Is that really a hard requirement on the dt format though? The
> hypervisor could always have its own internal lookup table between
> long and short forms.
There is also the desire to keep new details closer to what values the
hypervisor already use for domainIDs with FORM1. From PAPR spec
point of view, it is desirable to avoid adding another level of
indirection at the hypervisor level.
>
>> >> +prop-encoded-array: The number N of the domainIDs encoded as with encode-int, followed by
>> >> +N domainID encoded as with encode-int
>> >> +
>> >> +For ex:
>> >> +ibm,numa-lookup-index-table = {4, 0, 8, 250, 252}, domainID index
>> >> for domainID 8 is 1.
>> >
>> > As noted "domainID index" is confusing here, because it's different
>> > from the "domainID index" values in reference-points.
>> >
>> >> +
>> >> +"ibm,numa-distance-table" property contains one or more list of numbers representing the NUMA
>> >> +distance between resource groups/domains present in the system.
>> >> +
>> >> +prop-encoded-array: The number N of the distance values encoded as with encode-int, followed by
>> >> +N distance values encoded as with encode-bytes. The max distance value we could encode is 255.
>> >
>> > The N here always has to be the square of the N in the
>> > lookup-index-table, yes? In which case do we actually need to include
>> > it?
>>
>> most of PAPR device tree property follows the pattern {N,....
>> Nelements}. This follows the same pattern.
>
> Many do, yes, but in many of those cases reading the N is actually
> useful. It's impossible to interpret this table if you don't already
> know the number of nodes and if the length of the distance doesn't
> match that, there's no reasonable fallback interpretation.
>
>> >> +For ex:
>> >> +ibm,numa-lookup-index-table = {3, 0, 8, 40}
>> >> +ibm,numa-distance-table = {9, 10, 20, 80, 20, 10, 160, 80, 160, 10}
>> >> +
>> >> + | 0 8 40
>> >> +--|------------
>> >> + |
>> >> +0 | 10 20 80
>> >> + |
>> >> +8 | 20 10 160
>> >> + |
>> >> +40| 80 160 10
>> >> +
>> >> +
>> >> +"ibm,associativity" property for resources in node 0, 8 and 40
>> >> +
>> >> +{ 3, 6, 7, 0 }
>> >> +{ 3, 6, 9, 8 }
>> >> +{ 3, 6, 7, 40}
>> >> +
>> >> +With "ibm,associativity-reference-points" { 0x3 }
>> >> +
>> >> +Each resource (drcIndex) now also supports additional optional device tree properties.
>> >> +These properties are marked optional because the platform can choose not to export
>> >> +them and provide the system topology details using the earlier defined device tree
>> >> +properties alone. The optional device tree properties are used when adding new resources
>> >> +(DLPAR) and when the platform didn't provide the topology details of the domain which
>> >> +contains the newly added resource during boot.
>> >
>> > In what circumstances would you envisage a hot-add creating a new NUMA
>> > node, as opposed to adding resources to an existing (possibly empty)
>> > node? Do you need any provision for removing NUMA nodes again?
>>
>> Both Qemu and PowerVM don't intend to use this at this point. Both will
>> provide the full possible NUMA node details during boot. But I was not
>> sure whether we really need to restrict the possibility of a new
>> resource being added. This can be true in case of new memory devices
>> that gets hotplugged in the latency of the device is such that we never
>> expressed that in the distance table during boot.
>
> Hm, ok. Can we just leave it out of the spec for now and add it in
> when/if a hypervisor needs it?
Sure. I will drop the DLPAR specific changes.
>
>> >> +"ibm,numa-lookup-index" property contains a number representing the domainID index to be used
>> >> +when building the NUMA distance of the numa node to which this resource belongs. This can
>> >> +be looked at as the index at which this new domainID would have appeared in
>> >> +"ibm,numa-lookup-index-table" if the domain was present during boot. The domainID
>> >
>> > Again, confusing use of "index" where it's used both for offsets in
>> > ibm,associativity properties and for offsets in ibm,numa-lookup-index-table
>>
>> I guess we can drop the ibm,numa-lookuup-index and state that the
>> number of distance element in "ibm,numa-distance" suggest the index at
>> which this NUMA node should appear for compuring the distance details.
>
> I don't really follow what you're saying there.
>
>> >> +of the new resource can be obtained from the existing "ibm,associativity" property. This
>> >> +can be used to build distance information of a newly onlined NUMA node via DLPAR operation.
>> >> +The value is 1 based array index value.
>> >> +
>> >> +prop-encoded-array: An integer encoded as with encode-int specifying the domainID index
>> >> +
>> >> +"ibm,numa-distance" property contains one or more list of numbers presenting the NUMA distance
>> >> +from this resource domain to other resources.
>> >> +
>> >> +prop-encoded-array: The number N of the distance values encoded as with encode-int, followed by
>> >> +N distance values encoded as with encode-bytes. The max distance value we could encode is 255.
>> >
>> > Again, does N have to equal 2 * (existing number of nodes + 1)? If so
>> > you should say so, if not you should specify how incomplete
>> > information is interpreted.
>> >
>> >> +For ex:
>> >> +ibm,associativity = { 4, 5, 10, 50}
>> >> +ibm,numa-lookup-index = { 4 }
>> >> +ibm,numa-distance = {8, 160, 255, 80, 10, 160, 255, 80, 10}
>> >> +
>> >> +resulting in a new toplogy as below.
>> >> + | 0 8 40 50
>> >> +--|------------------
>> >> + |
>> >> +0 | 10 20 80 160
>> >> + |
>> >> +8 | 20 10 160 255
>> >> + |
>> >> +40| 80 160 10 80
>> >> + |
>> >> +50| 160 255 80 10
>> >> +
>>
>> Here is the relevant updated part of the doc.
>>
>> Form 2 associativity format adds separate device tree properties representing NUMA node distance
>> thereby making the node distance computation flexible. Form 2 also allows flexible primary
>> domain numbering. With numa distance computation now detached from the index value in
>> "ibm,associativity-reference-points" property, Form 2 allows a large number of primary domain
>> ids at the same domainID index representing resource groups of different performance/latency
>> characteristics.
>>
>> Hypervisor indicates the usage of FORM2 associativity using bit 2 of byte 5 in the
>> "ibm,architecture-vec-5" property.
>>
>> "ibm,numa-lookup-index-table" property contains one or more list numbers representing
>
> s/one or more list numbers/a list of one or more numbers/
That was picked from PAPR reference. I updated the properties mentioned
in this file to be "a list of one or more numbers"
>
>> the domainIDs present in the system. The offset of the domainID in this property is
>> used as an index while computing numa distance information via "ibm,numa-distance-table".
>>
>> prop-encoded-array: The number N of the domainIDs encoded as with encode-int, followed by
>> N domainID encoded as with encode-int
>>
>> For ex:
>> "ibm,numa-lookup-index-table" = {4, 0, 8, 250, 252}. Offset of domainID 8 (2) is used when
>> computing the distance of domain 8 from other domains present in the system. For rest of
>> this document this offset will be referred to as domain distance offset.
>>
>> "ibm,numa-distance-table" property contains one or more list of numbers representing the NUMA
>> distance between resource groups/domains present in the system.
>>
>> prop-encoded-array: The number N of the distance values encoded as with encode-int, followed by
>> N distance values encoded as with encode-bytes. The max distance value we could encode is 255.
>
> If you insist on retaining the N here, at least explicitly state that
> it must be equal to m^2, where m is the number of nodes in numa-lookup-index-table.
updated
>
>> For ex:
>> ibm,numa-lookup-index-table = {3, 0, 8, 40}
>> ibm,numa-distance-table = {9, 10, 20, 80, 20, 10, 160, 80, 160, 10}
>>
>> | 0 8 40
>> --|------------
>> |
>> 0 | 10 20 80
>> |
>> 8 | 20 10 160
>> |
>> 40| 80 160 10
>>
>> A possible "ibm,associativity" property for resources in node 0, 8 and 40
>>
>> { 3, 6, 7, 0 }
>> { 3, 6, 9, 8 }
>> { 3, 6, 7, 40}
>>
>> With "ibm,associativity-reference-points" { 0x3 }
>>
>> "ibm,lookup-index-table" helps in having a compact representation of distance matrix.
>> Since domainID can be sparse, the matrix of distances can also be effectively sparse.
>> With "ibm,lookup-index-table" we are able to achieve a compact representation of
>> distance information.
>>
>> Each resource (drcIndex) now also supports additional optional device tree properties.
>> These properties are marked optional because the platform can choose not to export
>> them and provide the system topology details using the earlier defined device tree
>> properties alone. The optional device tree properties are used when adding new resources
>> (DLPAR) and when the platform didn't provide the topology details of the domain which
>> contains the newly added resource during boot.
>>
>> "ibm,numa-distance" property contains one or more list of numbers presenting the NUMA distance
>> from this resource domain to other resources.
>>
>> prop-encoded-array: The number N of the distance values encoded as with encode-int, followed by
>> N distance values encoded as with encode-bytes. The max distance value we could encode is 255.
>
>> If the system currently has 3 domains and a DLPAR operation is adding one additional
>> domain, "ibm,numa-distance" property should have 2 * (3 + 1) = 8 elements as shown below.
>> In other words the domain distance offset value for the newly added domain is number of
>> distance value elements in the "ibm,numa-distance" property divided by 2.
>
> You need to spell out the distinction between the two halves of the
> property - which one adds a row to numa-distance-table, and which one
> adds a column.
updated.
>
>> >> diff --git a/arch/powerpc/include/asm/firmware.h b/arch/powerpc/include/asm/firmware.h
>> >> index 60b631161360..97a3bd9ffeb9 100644
>> >> --- a/arch/powerpc/include/asm/firmware.h
>> >> +++ b/arch/powerpc/include/asm/firmware.h
>> >> @@ -53,6 +53,7 @@
>> >> #define FW_FEATURE_ULTRAVISOR ASM_CONST(0x0000004000000000)
>> >> #define FW_FEATURE_STUFF_TCE ASM_CONST(0x0000008000000000)
>> >> #define FW_FEATURE_RPT_INVALIDATE ASM_CONST(0x0000010000000000)
>> >> +#define FW_FEATURE_FORM2_AFFINITY ASM_CONST(0x0000020000000000)
>> >>
>> >> #ifndef __ASSEMBLY__
>> >>
>> >> @@ -73,7 +74,7 @@ enum {
>> >> FW_FEATURE_HPT_RESIZE | FW_FEATURE_DRMEM_V2 |
>> >> FW_FEATURE_DRC_INFO | FW_FEATURE_BLOCK_REMOVE |
>> >> FW_FEATURE_PAPR_SCM | FW_FEATURE_ULTRAVISOR |
>> >> - FW_FEATURE_RPT_INVALIDATE,
>> >> + FW_FEATURE_RPT_INVALIDATE | FW_FEATURE_FORM2_AFFINITY,
>> >> FW_FEATURE_PSERIES_ALWAYS = 0,
>> >> FW_FEATURE_POWERNV_POSSIBLE = FW_FEATURE_OPAL | FW_FEATURE_ULTRAVISOR,
>> >> FW_FEATURE_POWERNV_ALWAYS = 0,
>> >> diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
>> >> index df9fec9d232c..5c80152e8f18 100644
>> >> --- a/arch/powerpc/include/asm/prom.h
>> >> +++ b/arch/powerpc/include/asm/prom.h
>> >> @@ -149,6 +149,7 @@ extern int of_read_drc_info_cell(struct property **prop,
>> >> #define OV5_XCMO 0x0440 /* Page Coalescing */
>> >> #define OV5_FORM1_AFFINITY 0x0580 /* FORM1 NUMA affinity */
>> >> #define OV5_PRRN 0x0540 /* Platform Resource Reassignment */
>> >> +#define OV5_FORM2_AFFINITY 0x0520 /* Form2 NUMA affinity */
>> >> #define OV5_HP_EVT 0x0604 /* Hot Plug Event support */
>> >> #define OV5_RESIZE_HPT 0x0601 /* Hash Page Table resizing */
>> >> #define OV5_PFO_HW_RNG 0x1180 /* PFO Random Number Generator */
>> >> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
>> >> index 64b9593038a7..496fdac54c29 100644
>> >> --- a/arch/powerpc/kernel/prom_init.c
>> >> +++ b/arch/powerpc/kernel/prom_init.c
>> >> @@ -1070,7 +1070,8 @@ static const struct ibm_arch_vec ibm_architecture_vec_template __initconst = {
>> >> #else
>> >> 0,
>> >> #endif
>> >> - .associativity = OV5_FEAT(OV5_FORM1_AFFINITY) | OV5_FEAT(OV5_PRRN),
>> >> + .associativity = OV5_FEAT(OV5_FORM1_AFFINITY) | OV5_FEAT(OV5_PRRN) |
>> >> + OV5_FEAT(OV5_FORM2_AFFINITY),
>> >> .bin_opts = OV5_FEAT(OV5_RESIZE_HPT) | OV5_FEAT(OV5_HP_EVT),
>> >> .micro_checkpoint = 0,
>> >> .reserved0 = 0,
>> >> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
>> >> index d32729f235b8..5a7d94960fb7 100644
>> >> --- a/arch/powerpc/mm/numa.c
>> >> +++ b/arch/powerpc/mm/numa.c
>> >> @@ -56,12 +56,17 @@ static int n_mem_addr_cells, n_mem_size_cells;
>> >>
>> >> #define FORM0_AFFINITY 0
>> >> #define FORM1_AFFINITY 1
>> >> +#define FORM2_AFFINITY 2
>> >> static int affinity_form;
>> >>
>> >> #define MAX_DISTANCE_REF_POINTS 4
>> >> static int max_associativity_domain_index;
>> >> static const __be32 *distance_ref_points;
>> >> static int distance_lookup_table[MAX_NUMNODES][MAX_DISTANCE_REF_POINTS];
>> >> +static int numa_distance_table[MAX_NUMNODES][MAX_NUMNODES] = {
>> >> + [0 ... MAX_NUMNODES - 1] = { [0 ... MAX_NUMNODES - 1] = -1 }
>> >> +};
>> >> +static int numa_id_index_table[MAX_NUMNODES];
>> >>
>> >> /*
>> >> * Allocate node_to_cpumask_map based on number of available nodes
>> >> @@ -166,6 +171,27 @@ static void unmap_cpu_from_node(unsigned long cpu)
>> >> }
>> >> #endif /* CONFIG_HOTPLUG_CPU || CONFIG_PPC_SPLPAR */
>> >>
>> >> +/*
>> >> + * With FORM2 if we are not using logical domain ids, we will find
>> >> + * both primary and seconday domains with same value. Hence always
>> >> + * start comparison from secondary domains
>> >
>> >
>> > IIUC, in this new draft, secondary domains are no longer a meaningful
>> > thing in Form2, so this comment and code seem outdated.
>>
>> This needed fixup. With FORM2 our associativity array can be just one
>> element. I fixed up as below.
>>
>> static int __cpu_form2_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc)
>> {
>> int node1, node2;
>> int dist = 0;
>>
>> node1 = associativity_to_nid(cpu1_assoc);
>> node2 = associativity_to_nid(cpu2_assoc);
>>
>> dist = numa_distance_table[node1][node2];
>> if (dist == LOCAL_DISTANCE)
>> return 0;
>> else if (dist == REMOTE_DISTANCE)
>> return 1;
>> else
>> return 2;
>> }
>>
>> also renamed cpu_distance to cpu_relative_distance.
>>
>>
>> >> + */
>> >> +static int __cpu_form2_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc)
>> >> +{
>> >> + int dist = 0;
>> >> +
>> >> + int i, index;
>> >> +
>> >> + for (i = 1; i < max_associativity_domain_index; i++) {
>> >> + index = be32_to_cpu(distance_ref_points[i]);
>> >> + if (cpu1_assoc[index] == cpu2_assoc[index])
>> >> + break;
>> >> + dist++;
>> >> + }
>> >> +
>> >> + return dist;
>> >> +}
>> >> +
>> >> static int __cpu_form1_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc)
>> >> {
>> >> int dist = 0;
>> >> @@ -178,7 +204,6 @@ static int __cpu_form1_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc)
>> >> break;
>> >> dist++;
>> >> }
>> >> -
>> >> return dist;
>> >> }
>> >>
>> >> @@ -186,8 +211,9 @@ int cpu_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc)
>> >> {
>> >> /* We should not get called with FORM0 */
>> >> VM_WARN_ON(affinity_form == FORM0_AFFINITY);
>> >> -
>> >> - return __cpu_form1_distance(cpu1_assoc, cpu2_assoc);
>> >> + if (affinity_form == FORM1_AFFINITY)
>> >> + return __cpu_form1_distance(cpu1_assoc, cpu2_assoc);
>> >> + return __cpu_form2_distance(cpu1_assoc, cpu2_assoc);
>> >> }
>> >>
>> >> /* must hold reference to node during call */
>> >> @@ -201,7 +227,9 @@ int __node_distance(int a, int b)
>> >> int i;
>> >> int distance = LOCAL_DISTANCE;
>> >>
>> >> - if (affinity_form == FORM0_AFFINITY)
>> >> + if (affinity_form == FORM2_AFFINITY)
>> >> + return numa_distance_table[a][b];
>> >> + else if (affinity_form == FORM0_AFFINITY)
>> >> return ((a == b) ? LOCAL_DISTANCE : REMOTE_DISTANCE);
>> >>
>> >> for (i = 0; i < max_associativity_domain_index; i++) {
>> >> @@ -303,15 +331,116 @@ static void initialize_form1_numa_distance(struct device_node *node)
>> >>
>> >> /*
>> >> * Used to update distance information w.r.t newly added node.
>> >> + * ibm,numa-lookup-index -> 4
>> >> + * ibm,numa-distance -> {5, 20, 40, 60, 80, 10 }
>> >> */
>> >> void update_numa_distance(struct device_node *node)
>> >> {
>> >> + int i, nid, other_nid, other_nid_index = 0;
>> >> + const __be32 *numa_indexp;
>> >> + const __u8 *numa_distancep;
>> >> + int numa_index, max_numa_index, numa_distance;
>> >> +
>> >> if (affinity_form == FORM0_AFFINITY)
>> >> return;
>> >> else if (affinity_form == FORM1_AFFINITY) {
>> >> initialize_form1_numa_distance(node);
>> >> return;
>> >> }
>> >> + /* FORM2 affinity */
>> >> +
>> >> + nid = of_node_to_nid_single(node);
>> >> + if (nid == NUMA_NO_NODE)
>> >> + return;
>> >> +
>> >> + /* Already initialized */
>> >> + if (numa_distance_table[nid][nid] != -1)
>> >
>> > IIUC, this should exactly correspond to the case where the new
>> > resource lies in an existing NUMA node, yes?
>>
>> yes.
>>
>> >
>> >> + return;
>> >> + /*
>> >> + * update node distance if not already populated.
>> >> + */
>> >> + numa_distancep = of_get_property(node, "ibm,numa-distance", NULL);
>> >> + if (!numa_distancep)
>> >> + return;
>> >> +
>> >> + numa_indexp = of_get_property(node, "ibm,numa-lookup-index", NULL);
>> >> + if (!numa_indexp)
>> >> + return;
>> >> +
>> >> + numa_index = of_read_number(numa_indexp, 1);
>> >> + /*
>> >> + * update the numa_id_index_table. Device tree look at index table as
>> >> + * 1 based array indexing.
>> >> + */
>> >> + numa_id_index_table[numa_index - 1] = nid;
>> >> +
>> >> + max_numa_index = of_read_number((const __be32 *)numa_distancep, 1);
>> >> + VM_WARN_ON(max_numa_index != 2 * numa_index);
>> >
>> > You WARN_ON(), but you don't actually bail out to avoid indexing past
>> > the end of the property below.
>> >
>> > AFAICT none of this really works unless numa_index == (previous number
>> > of numa nodes + 1), which makes all the use of these different
>> > variables kind of confusing. If you had a variable that you just set
>> > equal to the new number of numa nodes (previous number plus the one
>> > being added), then I think you can replace all numa_index and
>> > max_numa_index references with that and it will be clearer.
>>
>> I rewrote that as below.
>>
>> numa_distancep = of_get_property(node, "ibm,numa-distance", NULL);
>> if (!numa_distancep)
>> return;
>>
>> max_dist_element = of_read_number((const __be32 *)numa_distancep, 1);
>>
>> numa_id_index = max_dist_element >> 1;
>
> You already know the existing number of NUMA nodes, so it seems
> clearer to me to alidate the new property against that, rather than
> trying to parse it out of the new property.
>
>> /*
>> * update the numa_id_index_table. Device tree look at index table as
>> * 1 based array indexing.
>> */
>> if (numa_id_index_table[numa_id_index - 1] != -1) {
>> WARN(1, "Wrong NUMA distance information\n");
>> return;
>> }
>
> I believe
> if (WARN_ON(...))
> return;
>
> is idiomatic.
>
>> numa_id_index_table[numa_id_index - 1] = nid;
>>
>> /* Skip the size which is encoded int */
>> numa_distancep += sizeof(__be32);
>
> Is numa_distancep a u32 pointer, or a char pointer? If it's a u32
> pointer then adding sizeof(__be32) rather than 1 is incorrect here.
>
const __u8 *numa_distancep;
>> /*
>> * First fill the distance information from other node to this node.
>> */
>> distance_index = 0;
>> for (i = 0; i < numa_id_index; i++) {
>> numa_distance = numa_distancep[distance_index++];
>
> .. but if it's a char pointer then this looks wrong.
We are reading the 8 bit distance value here.
>
>> other_nid = numa_id_index_table[i];
>> if (other_nid == NUMA_NO_NODE)
>> continue;
>> numa_distance_table[other_nid][nid] = numa_distance;
>> }
>>
>> for (i = 0; i < numa_id_index; i++) {
>> numa_distance = numa_distancep[distance_index++];
>
> I'd just compute the offset from i here, rather than having another
> variable to track.
>
>> other_nid = numa_id_index_table[i];
>> if (other_nid == NUMA_NO_NODE)
>> continue;
>> numa_distance_table[nid][other_nid] = numa_distance;
>> }
>> }
>>
>>
>> >
>> >
>> >> + /* Skip the size which is encoded int */
>> >> + numa_distancep += sizeof(__be32);
>> >> +
>> >> + /*
>> >> + * First fill the distance information from other node to this node.
>> >> + */
>> >> + other_nid_index = 0;
>> >> + for (i = 0; i < numa_index; i++) {
>> >> + numa_distance = numa_distancep[i];
>> >> + other_nid = numa_id_index_table[other_nid_index++];
>> >> + numa_distance_table[other_nid][nid] = numa_distance;
>> >> + }
>> >> +
>> >> + other_nid_index = 0;
>> >> + for (; i < max_numa_index; i++) {
>> >
>> > Again, splitting this loop and carrying i over seems a confusing way
>> > to code this. It's basically two loops of N, one writing a row of the
>> > distance matrix, one writing a column (other_nid will even go through
>> > the same values in each loop).
>>
>> Fixed
>>
>> >
>> >> + numa_distance = numa_distancep[i];
>> >> + other_nid = numa_id_index_table[other_nid_index++];
>> >> + numa_distance_table[nid][other_nid] = numa_distance;
>> >> + }
>> >> +}
>> >> +
>> >> +/*
>> >> + * ibm,numa-lookup-index-table= {N, domainid1, domainid2, ..... domainidN}
>> >> + * ibm,numa-distance-table = { N, 1, 2, 4, 5, 1, 6, .... N elements}
>> >> + */
>> >> +static void initialize_form2_numa_distance_lookup_table(struct device_node *root)
>> >> +{
>> >> + const __u8 *numa_dist_table;
>> >> + const __be32 *numa_lookup_index;
>> >> + int numa_dist_table_length;
>> >> + int max_numa_index, distance_index;
>> >> + int i, curr_row = 0, curr_column = 0;
>> >> +
>> >> + numa_lookup_index = of_get_property(root, "ibm,numa-lookup-index-table", NULL);
>> >> + max_numa_index = of_read_number(&numa_lookup_index[0], 1);
>> >
>> > max_numa_index here has a different meaning to max_numa_index in the
>> > previous function, which is pointlessly confusing.
>> >
>> >> + /* first element of the array is the size and is encode-int */
>> >> + numa_dist_table = of_get_property(root, "ibm,numa-distance-table", NULL);
>> >> + numa_dist_table_length = of_read_number((const __be32 *)&numa_dist_table[0], 1);
>> >> + /* Skip the size which is encoded int */
>> >> + numa_dist_table += sizeof(__be32);
>> >> +
>> >> + pr_debug("numa_dist_table_len = %d, numa_dist_indexes_len = %d \n",
>> >> + numa_dist_table_length, max_numa_index);
>> >> +
>> >> + for (i = 0; i < max_numa_index; i++)
>> >> + /* +1 skip the max_numa_index in the property */
>> >> + numa_id_index_table[i] = of_read_number(&numa_lookup_index[i + 1], 1);
>> >> +
>> >> +
>> >> + VM_WARN_ON(numa_dist_table_length != max_numa_index * max_numa_index);
>> >
>> > Again, you don't actually bail out in this case. And if it has to
>> > have this value, what's the point of encoding it into the property.
>> >
>> >> + for (distance_index = 0; distance_index < numa_dist_table_length; distance_index++) {
>> >> + int nodeA = numa_id_index_table[curr_row];
>> >> + int nodeB = numa_id_index_table[curr_column++];
>> >
>> > You've already (sort of) verified that the distance table has size
>> > N^2, in which case you can just to a simple two dimensional loop
>> > rather than having to do ugly calculations of row and column.
>>
>> Fixed all the above and updated as below.
>> if (numa_dist_table_length != max_numa_index * max_numa_index) {
>> WARN(1, "Wrong NUMA distnce information\n");
> ^^^^^^^
>
updated.
>> /* consider everybody else just remote. */
>>
>> for (i = 0; i < max_numa_index; i++) {
>> for (j = 0; j < max_numa_index; j++) {
>> int nodeA = numa_id_index_table[i];
>> int nodeB = numa_id_index_table[j];
>>
>> if (nodeA == nodeB)
>> numa_distance_table[nodeA][nodeB] = LOCAL_DISTANCE;
>> else
>> numa_distance_table[nodeA][nodeB] = REMOTE_DISTANCE;
>> }
>> }
>> }
>>
>> distance_index = 0;
>> for (i = 0; i < max_numa_index; i++) {
>> for (j = 0; j < max_numa_index; j++) {
>> int nodeA = numa_id_index_table[i];
>> int nodeB = numa_id_index_table[j];
>>
>> numa_distance_table[nodeA][nodeB] = numa_dist_table[distance_index++];
>>
>> pr_debug("dist[%d][%d]=%d ", nodeA, nodeB, numa_distance_table[nodeA][nodeB]);
>> }
>> }
>> }
>>
>>
>> -aneesh
>>
>
> --
> David Gibson | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
> | _way_ _around_!
> http://www.ozlabs.org/~dgibson
^ permalink raw reply
* [PATCH v5 6/6] powerpc/pseries: Add support for FORM2 associativity
From: Aneesh Kumar K.V @ 2021-06-28 15:11 UTC (permalink / raw)
To: linuxppc-dev, mpe
Cc: Nathan Lynch, Aneesh Kumar K.V, Daniel Henrique Barboza,
David Gibson
In-Reply-To: <20210628151117.545935-1-aneesh.kumar@linux.ibm.com>
PAPR interface currently supports two different ways of communicating resource
grouping details to the OS. These are referred to as Form 0 and Form 1
associativity grouping. Form 0 is the older format and is now considered
deprecated. This patch adds another resource grouping named FORM2.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
Documentation/powerpc/associativity.rst | 103 ++++++++++++++
arch/powerpc/include/asm/firmware.h | 3 +-
arch/powerpc/include/asm/prom.h | 1 +
arch/powerpc/kernel/prom_init.c | 3 +-
arch/powerpc/mm/numa.c | 157 ++++++++++++++++++----
arch/powerpc/platforms/pseries/firmware.c | 1 +
6 files changed, 242 insertions(+), 26 deletions(-)
create mode 100644 Documentation/powerpc/associativity.rst
diff --git a/Documentation/powerpc/associativity.rst b/Documentation/powerpc/associativity.rst
new file mode 100644
index 000000000000..31cc7da2c7a6
--- /dev/null
+++ b/Documentation/powerpc/associativity.rst
@@ -0,0 +1,103 @@
+============================
+NUMA resource associativity
+=============================
+
+Associativity represents the groupings of the various platform resources into
+domains of substantially similar mean performance relative to resources outside
+of that domain. Resources subsets of a given domain that exhibit better
+performance relative to each other than relative to other resources subsets
+are represented as being members of a sub-grouping domain. This performance
+characteristic is presented in terms of NUMA node distance within the Linux kernel.
+From the platform view, these groups are also referred to as domains.
+
+PAPR interface currently supports different ways of communicating these resource
+grouping details to the OS. These are referred to as Form 0, Form 1 and Form2
+associativity grouping. Form 0 is the older format and is now considered deprecated.
+
+Hypervisor indicates the type/form of associativity used via "ibm,architecture-vec-5 property".
+Bit 0 of byte 5 in the "ibm,architecture-vec-5" property indicates usage of Form 0 or Form 1.
+A value of 1 indicates the usage of Form 1 associativity. For Form 2 associativity
+bit 2 of byte 5 in the "ibm,architecture-vec-5" property is used.
+
+Form 0
+-----
+Form 0 associativity supports only two NUMA distances (LOCAL and REMOTE).
+
+Form 1
+-----
+With Form 1 a combination of ibm,associativity-reference-points, and ibm,associativity
+device tree properties are used to determine the NUMA distance between resource groups/domains.
+
+The “ibm,associativity” property contains a list of one or more numbers (domainID)
+representing the resource’s platform grouping domains.
+
+The “ibm,associativity-reference-points” property contains a list of one or more numbers
+(domainID index) that represents the 1 based ordinal in the associativity lists.
+The list of domainID indexes represents an increasing hierarchy of resource grouping.
+
+ex:
+{ primary domainID index, secondary domainID index, tertiary domainID index.. }
+
+Linux kernel uses the domainID at the primary domainID index as the NUMA node id.
+Linux kernel computes NUMA distance between two domains by recursively comparing
+if they belong to the same higher-level domains. For mismatch at every higher
+level of the resource group, the kernel doubles the NUMA distance between the
+comparing domains.
+
+Form 2
+-------
+Form 2 associativity format adds separate device tree properties representing NUMA node distance
+thereby making the node distance computation flexible. Form 2 also allows flexible primary
+domain numbering. With numa distance computation now detached from the index value in
+"ibm,associativity-reference-points" property, Form 2 allows a large number of primary domain
+ids at the same domainID index representing resource groups of different performance/latency
+characteristics.
+
+Hypervisor indicates the usage of FORM2 associativity using bit 2 of byte 5 in the
+"ibm,architecture-vec-5" property.
+
+"ibm,numa-lookup-index-table" property contains a list of one or more numbers representing
+the domainIDs present in the system. The offset of the domainID in this property is
+used as an index while computing numa distance information via "ibm,numa-distance-table".
+
+prop-encoded-array: The number N of the domainIDs encoded as with encode-int, followed by
+N domainID encoded as with encode-int
+
+For ex:
+"ibm,numa-lookup-index-table" = {4, 0, 8, 250, 252}. The offset of domainID 8 (2) is used when
+computing the distance of domain 8 from other domains present in the system. For the rest of
+this document, this offset will be referred to as domain distance offset.
+
+"ibm,numa-distance-table" property contains a list of one or more numbers representing the NUMA
+distance between resource groups/domains present in the system.
+
+prop-encoded-array: The number N of the distance values encoded as with encode-int, followed by
+N distance values encoded as with encode-bytes. The max distance value we could encode is 255.
+The number N must be equal to the square of m where m is the number of domainIDs in the
+numa-lookup-index-table.
+
+For ex:
+ibm,numa-lookup-index-table = {3, 0, 8, 40}
+ibm,numa-distance-table = {9, 10, 20, 80, 20, 10, 160, 80, 160, 10}
+
+ | 0 8 40
+--|------------
+ |
+0 | 10 20 80
+ |
+8 | 20 10 160
+ |
+40| 80 160 10
+
+A possible "ibm,associativity" property for resources in node 0, 8 and 40
+
+{ 3, 6, 7, 0 }
+{ 3, 6, 9, 8 }
+{ 3, 6, 7, 40}
+
+With "ibm,associativity-reference-points" { 0x3 }
+
+"ibm,lookup-index-table" helps in having a compact representation of distance matrix.
+Since domainID can be sparse, the matrix of distances can also be effectively sparse.
+With "ibm,lookup-index-table" we can achieve a compact representation of
+distance information.
diff --git a/arch/powerpc/include/asm/firmware.h b/arch/powerpc/include/asm/firmware.h
index 60b631161360..97a3bd9ffeb9 100644
--- a/arch/powerpc/include/asm/firmware.h
+++ b/arch/powerpc/include/asm/firmware.h
@@ -53,6 +53,7 @@
#define FW_FEATURE_ULTRAVISOR ASM_CONST(0x0000004000000000)
#define FW_FEATURE_STUFF_TCE ASM_CONST(0x0000008000000000)
#define FW_FEATURE_RPT_INVALIDATE ASM_CONST(0x0000010000000000)
+#define FW_FEATURE_FORM2_AFFINITY ASM_CONST(0x0000020000000000)
#ifndef __ASSEMBLY__
@@ -73,7 +74,7 @@ enum {
FW_FEATURE_HPT_RESIZE | FW_FEATURE_DRMEM_V2 |
FW_FEATURE_DRC_INFO | FW_FEATURE_BLOCK_REMOVE |
FW_FEATURE_PAPR_SCM | FW_FEATURE_ULTRAVISOR |
- FW_FEATURE_RPT_INVALIDATE,
+ FW_FEATURE_RPT_INVALIDATE | FW_FEATURE_FORM2_AFFINITY,
FW_FEATURE_PSERIES_ALWAYS = 0,
FW_FEATURE_POWERNV_POSSIBLE = FW_FEATURE_OPAL | FW_FEATURE_ULTRAVISOR,
FW_FEATURE_POWERNV_ALWAYS = 0,
diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
index df9fec9d232c..5c80152e8f18 100644
--- a/arch/powerpc/include/asm/prom.h
+++ b/arch/powerpc/include/asm/prom.h
@@ -149,6 +149,7 @@ extern int of_read_drc_info_cell(struct property **prop,
#define OV5_XCMO 0x0440 /* Page Coalescing */
#define OV5_FORM1_AFFINITY 0x0580 /* FORM1 NUMA affinity */
#define OV5_PRRN 0x0540 /* Platform Resource Reassignment */
+#define OV5_FORM2_AFFINITY 0x0520 /* Form2 NUMA affinity */
#define OV5_HP_EVT 0x0604 /* Hot Plug Event support */
#define OV5_RESIZE_HPT 0x0601 /* Hash Page Table resizing */
#define OV5_PFO_HW_RNG 0x1180 /* PFO Random Number Generator */
diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index 5d9ea059594f..c483df6c9393 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -1069,7 +1069,8 @@ static const struct ibm_arch_vec ibm_architecture_vec_template __initconst = {
#else
0,
#endif
- .associativity = OV5_FEAT(OV5_FORM1_AFFINITY) | OV5_FEAT(OV5_PRRN),
+ .associativity = OV5_FEAT(OV5_FORM1_AFFINITY) | OV5_FEAT(OV5_PRRN) |
+ OV5_FEAT(OV5_FORM2_AFFINITY),
.bin_opts = OV5_FEAT(OV5_RESIZE_HPT) | OV5_FEAT(OV5_HP_EVT),
.micro_checkpoint = 0,
.reserved0 = 0,
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index c6293037a103..c68846fc9550 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -56,12 +56,17 @@ static int n_mem_addr_cells, n_mem_size_cells;
#define FORM0_AFFINITY 0
#define FORM1_AFFINITY 1
+#define FORM2_AFFINITY 2
static int affinity_form;
#define MAX_DISTANCE_REF_POINTS 4
static int max_associativity_domain_index;
static const __be32 *distance_ref_points;
static int distance_lookup_table[MAX_NUMNODES][MAX_DISTANCE_REF_POINTS];
+static int numa_distance_table[MAX_NUMNODES][MAX_NUMNODES] = {
+ [0 ... MAX_NUMNODES - 1] = { [0 ... MAX_NUMNODES - 1] = -1 }
+};
+static int numa_id_index_table[MAX_NUMNODES] = { [0 ... MAX_NUMNODES - 1] = NUMA_NO_NODE };
/*
* Allocate node_to_cpumask_map based on number of available nodes
@@ -166,6 +171,44 @@ static void unmap_cpu_from_node(unsigned long cpu)
}
#endif /* CONFIG_HOTPLUG_CPU || CONFIG_PPC_SPLPAR */
+/*
+ * Returns nid in the range [0..nr_node_ids], or -1 if no useful NUMA
+ * info is found.
+ */
+static int associativity_to_nid(const __be32 *associativity)
+{
+ int nid = NUMA_NO_NODE;
+
+ if (!numa_enabled)
+ goto out;
+
+ if (of_read_number(associativity, 1) >= primary_domain_index)
+ nid = of_read_number(&associativity[primary_domain_index], 1);
+
+ /* POWER4 LPAR uses 0xffff as invalid node */
+ if (nid == 0xffff || nid >= nr_node_ids)
+ nid = NUMA_NO_NODE;
+out:
+ return nid;
+}
+
+static int __cpu_form2_relative_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc)
+{
+ int dist;
+ int node1, node2;
+
+ node1 = associativity_to_nid(cpu1_assoc);
+ node2 = associativity_to_nid(cpu2_assoc);
+
+ dist = numa_distance_table[node1][node2];
+ if (dist <= LOCAL_DISTANCE)
+ return 0;
+ else if (dist <= REMOTE_DISTANCE)
+ return 1;
+ else
+ return 2;
+}
+
static int __cpu_form1_relative_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc)
{
int dist = 0;
@@ -186,8 +229,9 @@ int cpu_relative_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc)
{
/* We should not get called with FORM0 */
VM_WARN_ON(affinity_form == FORM0_AFFINITY);
-
- return __cpu_form1_relative_distance(cpu1_assoc, cpu2_assoc);
+ if (affinity_form == FORM1_AFFINITY)
+ return __cpu_form1_relative_distance(cpu1_assoc, cpu2_assoc);
+ return __cpu_form2_relative_distance(cpu1_assoc, cpu2_assoc);
}
/* must hold reference to node during call */
@@ -201,7 +245,9 @@ int __node_distance(int a, int b)
int i;
int distance = LOCAL_DISTANCE;
- if (affinity_form == FORM0_AFFINITY)
+ if (affinity_form == FORM2_AFFINITY)
+ return numa_distance_table[a][b];
+ else if (affinity_form == FORM0_AFFINITY)
return ((a == b) ? LOCAL_DISTANCE : REMOTE_DISTANCE);
for (i = 0; i < max_associativity_domain_index; i++) {
@@ -216,27 +262,6 @@ int __node_distance(int a, int b)
}
EXPORT_SYMBOL(__node_distance);
-/*
- * Returns nid in the range [0..nr_node_ids], or -1 if no useful NUMA
- * info is found.
- */
-static int associativity_to_nid(const __be32 *associativity)
-{
- int nid = NUMA_NO_NODE;
-
- if (!numa_enabled)
- goto out;
-
- if (of_read_number(associativity, 1) >= primary_domain_index)
- nid = of_read_number(&associativity[primary_domain_index], 1);
-
- /* POWER4 LPAR uses 0xffff as invalid node */
- if (nid == 0xffff || nid >= nr_node_ids)
- nid = NUMA_NO_NODE;
-out:
- return nid;
-}
-
/* Returns the nid associated with the given device tree node,
* or -1 if not found.
*/
@@ -305,12 +330,84 @@ static void initialize_form1_numa_distance(struct device_node *node)
*/
void update_numa_distance(struct device_node *node)
{
+ int nid;
+
if (affinity_form == FORM0_AFFINITY)
return;
else if (affinity_form == FORM1_AFFINITY) {
initialize_form1_numa_distance(node);
return;
}
+
+ /* FORM2 affinity */
+ nid = of_node_to_nid_single(node);
+ if (nid == NUMA_NO_NODE)
+ return;
+
+ /*
+ * With FORM2 we expect NUMA distance of all possible NUMA
+ * nodes to be provided during boot.
+ */
+ WARN(numa_distance_table[nid][nid] == -1,
+ "NUMA distance details for node %d not provided\n", nid);
+}
+
+/*
+ * ibm,numa-lookup-index-table= {N, domainid1, domainid2, ..... domainidN}
+ * ibm,numa-distance-table = { N, 1, 2, 4, 5, 1, 6, .... N elements}
+ */
+static void initialize_form2_numa_distance_lookup_table(struct device_node *root)
+{
+ int i, j;
+ const __u8 *numa_dist_table;
+ const __be32 *numa_lookup_index;
+ int numa_dist_table_length;
+ int max_numa_index, distance_index;
+
+ numa_lookup_index = of_get_property(root, "ibm,numa-lookup-index-table", NULL);
+ max_numa_index = of_read_number(&numa_lookup_index[0], 1);
+
+ /* first element of the array is the size and is encode-int */
+ numa_dist_table = of_get_property(root, "ibm,numa-distance-table", NULL);
+ numa_dist_table_length = of_read_number((const __be32 *)&numa_dist_table[0], 1);
+ /* Skip the size which is encoded int */
+ numa_dist_table += sizeof(__be32);
+
+ pr_debug("numa_dist_table_len = %d, numa_dist_indexes_len = %d\n",
+ numa_dist_table_length, max_numa_index);
+
+ for (i = 0; i < max_numa_index; i++)
+ /* +1 skip the max_numa_index in the property */
+ numa_id_index_table[i] = of_read_number(&numa_lookup_index[i + 1], 1);
+
+
+ if (numa_dist_table_length != max_numa_index * max_numa_index) {
+
+ WARN(1, "Wrong NUMA distance information\n");
+ /* consider everybody else just remote. */
+ for (i = 0; i < max_numa_index; i++) {
+ for (j = 0; j < max_numa_index; j++) {
+ int nodeA = numa_id_index_table[i];
+ int nodeB = numa_id_index_table[j];
+
+ if (nodeA == nodeB)
+ numa_distance_table[nodeA][nodeB] = LOCAL_DISTANCE;
+ else
+ numa_distance_table[nodeA][nodeB] = REMOTE_DISTANCE;
+ }
+ }
+ }
+
+ distance_index = 0;
+ for (i = 0; i < max_numa_index; i++) {
+ for (j = 0; j < max_numa_index; j++) {
+ int nodeA = numa_id_index_table[i];
+ int nodeB = numa_id_index_table[j];
+
+ numa_distance_table[nodeA][nodeB] = numa_dist_table[distance_index++];
+ pr_debug("dist[%d][%d]=%d ", nodeA, nodeB, numa_distance_table[nodeA][nodeB]);
+ }
+ }
}
static int __init find_primary_domain_index(void)
@@ -323,6 +420,9 @@ static int __init find_primary_domain_index(void)
*/
if (firmware_has_feature(FW_FEATURE_OPAL)) {
affinity_form = FORM1_AFFINITY;
+ } else if (firmware_has_feature(FW_FEATURE_FORM2_AFFINITY)) {
+ dbg("Using form 2 affinity\n");
+ affinity_form = FORM2_AFFINITY;
} else if (firmware_has_feature(FW_FEATURE_FORM1_AFFINITY)) {
dbg("Using form 1 affinity\n");
affinity_form = FORM1_AFFINITY;
@@ -367,8 +467,17 @@ static int __init find_primary_domain_index(void)
index = of_read_number(&distance_ref_points[1], 1);
} else {
+ /*
+ * Both FORM1 and FORM2 affinity find the primary domain details
+ * at the same offset.
+ */
index = of_read_number(distance_ref_points, 1);
}
+ /*
+ * If it is FORM2 also initialize the distance table here.
+ */
+ if (affinity_form == FORM2_AFFINITY)
+ initialize_form2_numa_distance_lookup_table(root);
/*
* Warn and cap if the hardware supports more than
diff --git a/arch/powerpc/platforms/pseries/firmware.c b/arch/powerpc/platforms/pseries/firmware.c
index 5d4c2bc20bba..f162156b7b68 100644
--- a/arch/powerpc/platforms/pseries/firmware.c
+++ b/arch/powerpc/platforms/pseries/firmware.c
@@ -123,6 +123,7 @@ vec5_fw_features_table[] = {
{FW_FEATURE_PRRN, OV5_PRRN},
{FW_FEATURE_DRMEM_V2, OV5_DRMEM_V2},
{FW_FEATURE_DRC_INFO, OV5_DRC_INFO},
+ {FW_FEATURE_FORM2_AFFINITY, OV5_FORM2_AFFINITY},
};
static void __init fw_vec5_feature_init(const char *vec5, unsigned long len)
--
2.31.1
^ permalink raw reply related
* [PATCH v5 5/6] powerpc/pseries: Add a helper for form1 cpu distance
From: Aneesh Kumar K.V @ 2021-06-28 15:11 UTC (permalink / raw)
To: linuxppc-dev, mpe
Cc: Nathan Lynch, Aneesh Kumar K.V, Daniel Henrique Barboza,
David Gibson
In-Reply-To: <20210628151117.545935-1-aneesh.kumar@linux.ibm.com>
This helper is only used with the dispatch trace log collection.
A later patch will add Form2 affinity support and this change helps
in keeping that simpler. Also add a comment explaining we don't expect
the code to be called with FORM0
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/include/asm/topology.h | 4 ++--
arch/powerpc/mm/numa.c | 10 +++++++++-
arch/powerpc/platforms/pseries/lpar.c | 4 ++--
3 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
index e4db64c0e184..ac8b5ed79832 100644
--- a/arch/powerpc/include/asm/topology.h
+++ b/arch/powerpc/include/asm/topology.h
@@ -36,7 +36,7 @@ static inline int pcibus_to_node(struct pci_bus *bus)
cpu_all_mask : \
cpumask_of_node(pcibus_to_node(bus)))
-extern int cpu_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc);
+int cpu_relative_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc);
extern int __node_distance(int, int);
#define node_distance(a, b) __node_distance(a, b)
@@ -83,7 +83,7 @@ static inline void sysfs_remove_device_from_node(struct device *dev,
static inline void update_numa_cpu_lookup_table(unsigned int cpu, int node) {}
-static inline int cpu_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc)
+static inline int cpu_relative_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc)
{
return 0;
}
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 7b142f79d600..c6293037a103 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -166,7 +166,7 @@ static void unmap_cpu_from_node(unsigned long cpu)
}
#endif /* CONFIG_HOTPLUG_CPU || CONFIG_PPC_SPLPAR */
-int cpu_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc)
+static int __cpu_form1_relative_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc)
{
int dist = 0;
@@ -182,6 +182,14 @@ int cpu_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc)
return dist;
}
+int cpu_relative_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc)
+{
+ /* We should not get called with FORM0 */
+ VM_WARN_ON(affinity_form == FORM0_AFFINITY);
+
+ return __cpu_form1_relative_distance(cpu1_assoc, cpu2_assoc);
+}
+
/* must hold reference to node during call */
static const __be32 *of_get_associativity(struct device_node *dev)
{
diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
index dab356e3ff87..afefbdfe768d 100644
--- a/arch/powerpc/platforms/pseries/lpar.c
+++ b/arch/powerpc/platforms/pseries/lpar.c
@@ -261,7 +261,7 @@ static int cpu_relative_dispatch_distance(int last_disp_cpu, int cur_disp_cpu)
if (!last_disp_cpu_assoc || !cur_disp_cpu_assoc)
return -EIO;
- return cpu_distance(last_disp_cpu_assoc, cur_disp_cpu_assoc);
+ return cpu_relative_distance(last_disp_cpu_assoc, cur_disp_cpu_assoc);
}
static int cpu_home_node_dispatch_distance(int disp_cpu)
@@ -281,7 +281,7 @@ static int cpu_home_node_dispatch_distance(int disp_cpu)
if (!disp_cpu_assoc || !vcpu_assoc)
return -EIO;
- return cpu_distance(disp_cpu_assoc, vcpu_assoc);
+ return cpu_relative_distance(disp_cpu_assoc, vcpu_assoc);
}
static void update_vcpu_disp_stat(int disp_cpu)
--
2.31.1
^ permalink raw reply related
* [PATCH v5 4/6] powerpc/pseries: Consolidate different NUMA distance update code paths
From: Aneesh Kumar K.V @ 2021-06-28 15:11 UTC (permalink / raw)
To: linuxppc-dev, mpe
Cc: Nathan Lynch, Aneesh Kumar K.V, Daniel Henrique Barboza,
David Gibson
In-Reply-To: <20210628151117.545935-1-aneesh.kumar@linux.ibm.com>
The associativity details of the newly added resourced are collected from
the hypervisor via "ibm,configure-connector" rtas call. Update the numa
distance details of the newly added numa node after the above call.
Instead of updating NUMA distance every time we lookup a node id
from the associativity property, add helpers that can be used
during boot which does this only once. Also remove the distance
update from node id lookup helpers.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/mm/numa.c | 173 +++++++++++++-----
arch/powerpc/platforms/pseries/hotplug-cpu.c | 2 +
.../platforms/pseries/hotplug-memory.c | 2 +
arch/powerpc/platforms/pseries/pseries.h | 1 +
4 files changed, 132 insertions(+), 46 deletions(-)
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 0ec16999beef..7b142f79d600 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -208,22 +208,6 @@ int __node_distance(int a, int b)
}
EXPORT_SYMBOL(__node_distance);
-static void initialize_distance_lookup_table(int nid,
- const __be32 *associativity)
-{
- int i;
-
- if (affinity_form != FORM1_AFFINITY)
- return;
-
- for (i = 0; i < max_associativity_domain_index; i++) {
- const __be32 *entry;
-
- entry = &associativity[be32_to_cpu(distance_ref_points[i]) - 1];
- distance_lookup_table[nid][i] = of_read_number(entry, 1);
- }
-}
-
/*
* Returns nid in the range [0..nr_node_ids], or -1 if no useful NUMA
* info is found.
@@ -241,15 +225,6 @@ static int associativity_to_nid(const __be32 *associativity)
/* POWER4 LPAR uses 0xffff as invalid node */
if (nid == 0xffff || nid >= nr_node_ids)
nid = NUMA_NO_NODE;
-
- if (nid > 0 &&
- of_read_number(associativity, 1) >= max_associativity_domain_index) {
- /*
- * Skip the length field and send start of associativity array
- */
- initialize_distance_lookup_table(nid, associativity + 1);
- }
-
out:
return nid;
}
@@ -287,6 +262,49 @@ int of_node_to_nid(struct device_node *device)
}
EXPORT_SYMBOL(of_node_to_nid);
+static void __initialize_form1_numa_distance(const __be32 *associativity)
+{
+ int i, nid;
+
+ if (affinity_form != FORM1_AFFINITY)
+ return;
+
+ if (of_read_number(associativity, 1) >= primary_domain_index) {
+ nid = of_read_number(&associativity[primary_domain_index], 1);
+
+ for (i = 0; i < max_associativity_domain_index; i++) {
+ const __be32 *entry;
+
+ entry = &associativity[be32_to_cpu(distance_ref_points[i])];
+ distance_lookup_table[nid][i] = of_read_number(entry, 1);
+ }
+ }
+}
+
+static void initialize_form1_numa_distance(struct device_node *node)
+{
+ const __be32 *associativity;
+
+ associativity = of_get_associativity(node);
+ if (!associativity)
+ return;
+
+ __initialize_form1_numa_distance(associativity);
+}
+
+/*
+ * Used to update distance information w.r.t newly added node.
+ */
+void update_numa_distance(struct device_node *node)
+{
+ if (affinity_form == FORM0_AFFINITY)
+ return;
+ else if (affinity_form == FORM1_AFFINITY) {
+ initialize_form1_numa_distance(node);
+ return;
+ }
+}
+
static int __init find_primary_domain_index(void)
{
int index;
@@ -433,6 +451,48 @@ static int of_get_assoc_arrays(struct assoc_arrays *aa)
return 0;
}
+static int get_nid_and_numa_distance(struct drmem_lmb *lmb)
+{
+ struct assoc_arrays aa = { .arrays = NULL };
+ int default_nid = NUMA_NO_NODE;
+ int nid = default_nid;
+ int rc, index;
+
+ if ((primary_domain_index < 0) || !numa_enabled)
+ return default_nid;
+
+ rc = of_get_assoc_arrays(&aa);
+ if (rc)
+ return default_nid;
+
+ if (primary_domain_index <= aa.array_sz &&
+ !(lmb->flags & DRCONF_MEM_AI_INVALID) && lmb->aa_index < aa.n_arrays) {
+ index = lmb->aa_index * aa.array_sz + primary_domain_index - 1;
+ nid = of_read_number(&aa.arrays[index], 1);
+
+ if (nid == 0xffff || nid >= nr_node_ids)
+ nid = default_nid;
+ if (nid > 0 && affinity_form == FORM1_AFFINITY) {
+ int i;
+ const __be32 *associativity;
+
+ index = lmb->aa_index * aa.array_sz;
+ associativity = &aa.arrays[index];
+ /*
+ * lookup array associativity entries have different format
+ * There is no length of the array as the first element.
+ */
+ for (i = 0; i < max_associativity_domain_index; i++) {
+ const __be32 *entry;
+
+ entry = &associativity[be32_to_cpu(distance_ref_points[i]) - 1];
+ distance_lookup_table[nid][i] = of_read_number(entry, 1);
+ }
+ }
+ }
+ return nid;
+}
+
/*
* This is like of_node_to_nid_single() for memory represented in the
* ibm,dynamic-reconfiguration-memory node.
@@ -458,21 +518,14 @@ int of_drconf_to_nid_single(struct drmem_lmb *lmb)
if (nid == 0xffff || nid >= nr_node_ids)
nid = default_nid;
-
- if (nid > 0) {
- index = lmb->aa_index * aa.array_sz;
- initialize_distance_lookup_table(nid,
- &aa.arrays[index]);
- }
}
-
return nid;
}
#ifdef CONFIG_PPC_SPLPAR
-static int vphn_get_nid(long lcpu)
+
+static int __vphn_get_associativity(long lcpu, __be32 *associativity)
{
- __be32 associativity[VPHN_ASSOC_BUFSIZE] = {0};
long rc, hwid;
/*
@@ -492,10 +545,22 @@ static int vphn_get_nid(long lcpu)
rc = hcall_vphn(hwid, VPHN_FLAG_VCPU, associativity);
if (rc == H_SUCCESS)
- return associativity_to_nid(associativity);
+ return 0;
}
+ return -1;
+}
+
+static int vphn_get_nid(long lcpu)
+{
+ __be32 associativity[VPHN_ASSOC_BUFSIZE] = {0};
+
+
+ if (!__vphn_get_associativity(lcpu, associativity))
+ return associativity_to_nid(associativity);
+
return NUMA_NO_NODE;
+
}
#else
static int vphn_get_nid(long unused)
@@ -692,7 +757,7 @@ static int __init numa_setup_drmem_lmb(struct drmem_lmb *lmb,
size = read_n_cells(n_mem_size_cells, usm);
}
- nid = of_drconf_to_nid_single(lmb);
+ nid = get_nid_and_numa_distance(lmb);
fake_numa_create_new_node(((base + size) >> PAGE_SHIFT),
&nid);
node_set_online(nid);
@@ -709,6 +774,7 @@ static int __init parse_numa_properties(void)
struct device_node *memory;
int default_nid = 0;
unsigned long i;
+ const __be32 *associativity;
if (numa_enabled == 0) {
printk(KERN_WARNING "NUMA disabled by user\n");
@@ -734,18 +800,30 @@ static int __init parse_numa_properties(void)
* each node to be onlined must have NODE_DATA etc backing it.
*/
for_each_present_cpu(i) {
+ __be32 vphn_assoc[VPHN_ASSOC_BUFSIZE];
struct device_node *cpu;
- int nid = vphn_get_nid(i);
+ int nid = NUMA_NO_NODE;
- /*
- * Don't fall back to default_nid yet -- we will plug
- * cpus into nodes once the memory scan has discovered
- * the topology.
- */
- if (nid == NUMA_NO_NODE) {
+ memset(vphn_assoc, 0, VPHN_ASSOC_BUFSIZE * sizeof(__be32));
+
+ if (__vphn_get_associativity(i, vphn_assoc) == 0) {
+ nid = associativity_to_nid(vphn_assoc);
+ __initialize_form1_numa_distance(vphn_assoc);
+ } else {
+
+ /*
+ * Don't fall back to default_nid yet -- we will plug
+ * cpus into nodes once the memory scan has discovered
+ * the topology.
+ */
cpu = of_get_cpu_node(i, NULL);
BUG_ON(!cpu);
- nid = of_node_to_nid_single(cpu);
+
+ associativity = of_get_associativity(cpu);
+ if (associativity) {
+ nid = associativity_to_nid(associativity);
+ __initialize_form1_numa_distance(associativity);
+ }
of_node_put(cpu);
}
@@ -781,8 +859,11 @@ static int __init parse_numa_properties(void)
* have associativity properties. If none, then
* everything goes to default_nid.
*/
- nid = of_node_to_nid_single(memory);
- if (nid < 0)
+ associativity = of_get_associativity(memory);
+ if (associativity) {
+ nid = associativity_to_nid(associativity);
+ __initialize_form1_numa_distance(associativity);
+ } else
nid = default_nid;
fake_numa_create_new_node(((start + size) >> PAGE_SHIFT), &nid);
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index 7e970f81d8ff..778b6ab35f0d 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -498,6 +498,8 @@ static ssize_t dlpar_cpu_add(u32 drc_index)
return saved_rc;
}
+ update_numa_distance(dn);
+
rc = dlpar_online_cpu(dn);
if (rc) {
saved_rc = rc;
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 36f66556a7c6..40d350f31a34 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -180,6 +180,8 @@ static int update_lmb_associativity_index(struct drmem_lmb *lmb)
return -ENODEV;
}
+ update_numa_distance(lmb_node);
+
dr_node = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
if (!dr_node) {
dlpar_free_cc_nodes(lmb_node);
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index 1f051a786fb3..663a0859cf13 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -113,4 +113,5 @@ extern u32 pseries_security_flavor;
void pseries_setup_security_mitigations(void);
void pseries_lpar_read_hblkrm_characteristics(void);
+void update_numa_distance(struct device_node *node);
#endif /* _PSERIES_PSERIES_H */
--
2.31.1
^ permalink raw reply related
* [PATCH v5 3/6] powerpc/pseries: Rename TYPE1_AFFINITY to FORM1_AFFINITY
From: Aneesh Kumar K.V @ 2021-06-28 15:11 UTC (permalink / raw)
To: linuxppc-dev, mpe
Cc: Nathan Lynch, Aneesh Kumar K.V, Daniel Henrique Barboza,
David Gibson
In-Reply-To: <20210628151117.545935-1-aneesh.kumar@linux.ibm.com>
Also make related code cleanup that will allow adding FORM2_AFFINITY in
later patches. No functional change in this patch.
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/include/asm/firmware.h | 4 +--
arch/powerpc/include/asm/prom.h | 2 +-
arch/powerpc/kernel/prom_init.c | 2 +-
arch/powerpc/mm/numa.c | 35 ++++++++++++++---------
arch/powerpc/platforms/pseries/firmware.c | 2 +-
5 files changed, 26 insertions(+), 19 deletions(-)
diff --git a/arch/powerpc/include/asm/firmware.h b/arch/powerpc/include/asm/firmware.h
index 7604673787d6..60b631161360 100644
--- a/arch/powerpc/include/asm/firmware.h
+++ b/arch/powerpc/include/asm/firmware.h
@@ -44,7 +44,7 @@
#define FW_FEATURE_OPAL ASM_CONST(0x0000000010000000)
#define FW_FEATURE_SET_MODE ASM_CONST(0x0000000040000000)
#define FW_FEATURE_BEST_ENERGY ASM_CONST(0x0000000080000000)
-#define FW_FEATURE_TYPE1_AFFINITY ASM_CONST(0x0000000100000000)
+#define FW_FEATURE_FORM1_AFFINITY ASM_CONST(0x0000000100000000)
#define FW_FEATURE_PRRN ASM_CONST(0x0000000200000000)
#define FW_FEATURE_DRMEM_V2 ASM_CONST(0x0000000400000000)
#define FW_FEATURE_DRC_INFO ASM_CONST(0x0000000800000000)
@@ -69,7 +69,7 @@ enum {
FW_FEATURE_SPLPAR | FW_FEATURE_LPAR |
FW_FEATURE_CMO | FW_FEATURE_VPHN | FW_FEATURE_XCMO |
FW_FEATURE_SET_MODE | FW_FEATURE_BEST_ENERGY |
- FW_FEATURE_TYPE1_AFFINITY | FW_FEATURE_PRRN |
+ FW_FEATURE_FORM1_AFFINITY | FW_FEATURE_PRRN |
FW_FEATURE_HPT_RESIZE | FW_FEATURE_DRMEM_V2 |
FW_FEATURE_DRC_INFO | FW_FEATURE_BLOCK_REMOVE |
FW_FEATURE_PAPR_SCM | FW_FEATURE_ULTRAVISOR |
diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
index 324a13351749..df9fec9d232c 100644
--- a/arch/powerpc/include/asm/prom.h
+++ b/arch/powerpc/include/asm/prom.h
@@ -147,7 +147,7 @@ extern int of_read_drc_info_cell(struct property **prop,
#define OV5_MSI 0x0201 /* PCIe/MSI support */
#define OV5_CMO 0x0480 /* Cooperative Memory Overcommitment */
#define OV5_XCMO 0x0440 /* Page Coalescing */
-#define OV5_TYPE1_AFFINITY 0x0580 /* Type 1 NUMA affinity */
+#define OV5_FORM1_AFFINITY 0x0580 /* FORM1 NUMA affinity */
#define OV5_PRRN 0x0540 /* Platform Resource Reassignment */
#define OV5_HP_EVT 0x0604 /* Hot Plug Event support */
#define OV5_RESIZE_HPT 0x0601 /* Hash Page Table resizing */
diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index 523b31685c4c..5d9ea059594f 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -1069,7 +1069,7 @@ static const struct ibm_arch_vec ibm_architecture_vec_template __initconst = {
#else
0,
#endif
- .associativity = OV5_FEAT(OV5_TYPE1_AFFINITY) | OV5_FEAT(OV5_PRRN),
+ .associativity = OV5_FEAT(OV5_FORM1_AFFINITY) | OV5_FEAT(OV5_PRRN),
.bin_opts = OV5_FEAT(OV5_RESIZE_HPT) | OV5_FEAT(OV5_HP_EVT),
.micro_checkpoint = 0,
.reserved0 = 0,
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 132813dd1a6c..0ec16999beef 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -53,7 +53,10 @@ EXPORT_SYMBOL(node_data);
static int primary_domain_index;
static int n_mem_addr_cells, n_mem_size_cells;
-static int form1_affinity;
+
+#define FORM0_AFFINITY 0
+#define FORM1_AFFINITY 1
+static int affinity_form;
#define MAX_DISTANCE_REF_POINTS 4
static int max_associativity_domain_index;
@@ -190,7 +193,7 @@ int __node_distance(int a, int b)
int i;
int distance = LOCAL_DISTANCE;
- if (!form1_affinity)
+ if (affinity_form == FORM0_AFFINITY)
return ((a == b) ? LOCAL_DISTANCE : REMOTE_DISTANCE);
for (i = 0; i < max_associativity_domain_index; i++) {
@@ -210,7 +213,7 @@ static void initialize_distance_lookup_table(int nid,
{
int i;
- if (!form1_affinity)
+ if (affinity_form != FORM1_AFFINITY)
return;
for (i = 0; i < max_associativity_domain_index; i++) {
@@ -289,6 +292,17 @@ static int __init find_primary_domain_index(void)
int index;
struct device_node *root;
+ /*
+ * Check for which form of affinity.
+ */
+ if (firmware_has_feature(FW_FEATURE_OPAL)) {
+ affinity_form = FORM1_AFFINITY;
+ } else if (firmware_has_feature(FW_FEATURE_FORM1_AFFINITY)) {
+ dbg("Using form 1 affinity\n");
+ affinity_form = FORM1_AFFINITY;
+ } else
+ affinity_form = FORM0_AFFINITY;
+
if (firmware_has_feature(FW_FEATURE_OPAL))
root = of_find_node_by_path("/ibm,opal");
else
@@ -318,23 +332,16 @@ static int __init find_primary_domain_index(void)
}
max_associativity_domain_index /= sizeof(int);
-
- if (firmware_has_feature(FW_FEATURE_OPAL) ||
- firmware_has_feature(FW_FEATURE_TYPE1_AFFINITY)) {
- dbg("Using form 1 affinity\n");
- form1_affinity = 1;
- }
-
- if (form1_affinity) {
- index = of_read_number(distance_ref_points, 1);
- } else {
+ if (affinity_form == FORM0_AFFINITY) {
if (max_associativity_domain_index < 2) {
printk(KERN_WARNING "NUMA: "
- "short ibm,associativity-reference-points\n");
+ "short ibm,associativity-reference-points\n");
goto err;
}
index = of_read_number(&distance_ref_points[1], 1);
+ } else {
+ index = of_read_number(distance_ref_points, 1);
}
/*
diff --git a/arch/powerpc/platforms/pseries/firmware.c b/arch/powerpc/platforms/pseries/firmware.c
index 4c7b7f5a2ebc..5d4c2bc20bba 100644
--- a/arch/powerpc/platforms/pseries/firmware.c
+++ b/arch/powerpc/platforms/pseries/firmware.c
@@ -119,7 +119,7 @@ struct vec5_fw_feature {
static __initdata struct vec5_fw_feature
vec5_fw_features_table[] = {
- {FW_FEATURE_TYPE1_AFFINITY, OV5_TYPE1_AFFINITY},
+ {FW_FEATURE_FORM1_AFFINITY, OV5_FORM1_AFFINITY},
{FW_FEATURE_PRRN, OV5_PRRN},
{FW_FEATURE_DRMEM_V2, OV5_DRMEM_V2},
{FW_FEATURE_DRC_INFO, OV5_DRC_INFO},
--
2.31.1
^ permalink raw reply related
* [PATCH v5 2/6] powerpc/pseries: rename distance_ref_points_depth to max_associativity_domain_index
From: Aneesh Kumar K.V @ 2021-06-28 15:11 UTC (permalink / raw)
To: linuxppc-dev, mpe
Cc: Nathan Lynch, Aneesh Kumar K.V, Daniel Henrique Barboza,
David Gibson
In-Reply-To: <20210628151117.545935-1-aneesh.kumar@linux.ibm.com>
No functional change in this patch
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/mm/numa.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 8365b298ec48..132813dd1a6c 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -56,7 +56,7 @@ static int n_mem_addr_cells, n_mem_size_cells;
static int form1_affinity;
#define MAX_DISTANCE_REF_POINTS 4
-static int distance_ref_points_depth;
+static int max_associativity_domain_index;
static const __be32 *distance_ref_points;
static int distance_lookup_table[MAX_NUMNODES][MAX_DISTANCE_REF_POINTS];
@@ -169,7 +169,7 @@ int cpu_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc)
int i, index;
- for (i = 0; i < distance_ref_points_depth; i++) {
+ for (i = 0; i < max_associativity_domain_index; i++) {
index = be32_to_cpu(distance_ref_points[i]);
if (cpu1_assoc[index] == cpu2_assoc[index])
break;
@@ -193,7 +193,7 @@ int __node_distance(int a, int b)
if (!form1_affinity)
return ((a == b) ? LOCAL_DISTANCE : REMOTE_DISTANCE);
- for (i = 0; i < distance_ref_points_depth; i++) {
+ for (i = 0; i < max_associativity_domain_index; i++) {
if (distance_lookup_table[a][i] == distance_lookup_table[b][i])
break;
@@ -213,7 +213,7 @@ static void initialize_distance_lookup_table(int nid,
if (!form1_affinity)
return;
- for (i = 0; i < distance_ref_points_depth; i++) {
+ for (i = 0; i < max_associativity_domain_index; i++) {
const __be32 *entry;
entry = &associativity[be32_to_cpu(distance_ref_points[i]) - 1];
@@ -240,7 +240,7 @@ static int associativity_to_nid(const __be32 *associativity)
nid = NUMA_NO_NODE;
if (nid > 0 &&
- of_read_number(associativity, 1) >= distance_ref_points_depth) {
+ of_read_number(associativity, 1) >= max_associativity_domain_index) {
/*
* Skip the length field and send start of associativity array
*/
@@ -310,14 +310,14 @@ static int __init find_primary_domain_index(void)
*/
distance_ref_points = of_get_property(root,
"ibm,associativity-reference-points",
- &distance_ref_points_depth);
+ &max_associativity_domain_index);
if (!distance_ref_points) {
dbg("NUMA: ibm,associativity-reference-points not found.\n");
goto err;
}
- distance_ref_points_depth /= sizeof(int);
+ max_associativity_domain_index /= sizeof(int);
if (firmware_has_feature(FW_FEATURE_OPAL) ||
firmware_has_feature(FW_FEATURE_TYPE1_AFFINITY)) {
@@ -328,7 +328,7 @@ static int __init find_primary_domain_index(void)
if (form1_affinity) {
index = of_read_number(distance_ref_points, 1);
} else {
- if (distance_ref_points_depth < 2) {
+ if (max_associativity_domain_index < 2) {
printk(KERN_WARNING "NUMA: "
"short ibm,associativity-reference-points\n");
goto err;
@@ -341,10 +341,10 @@ static int __init find_primary_domain_index(void)
* Warn and cap if the hardware supports more than
* MAX_DISTANCE_REF_POINTS domains.
*/
- if (distance_ref_points_depth > MAX_DISTANCE_REF_POINTS) {
+ if (max_associativity_domain_index > MAX_DISTANCE_REF_POINTS) {
printk(KERN_WARNING "NUMA: distance array capped at "
"%d entries\n", MAX_DISTANCE_REF_POINTS);
- distance_ref_points_depth = MAX_DISTANCE_REF_POINTS;
+ max_associativity_domain_index = MAX_DISTANCE_REF_POINTS;
}
of_node_put(root);
--
2.31.1
^ permalink raw reply related
* [PATCH v5 1/6] powerpc/pseries: rename min_common_depth to primary_domain_index
From: Aneesh Kumar K.V @ 2021-06-28 15:11 UTC (permalink / raw)
To: linuxppc-dev, mpe
Cc: Nathan Lynch, Aneesh Kumar K.V, Daniel Henrique Barboza,
David Gibson
In-Reply-To: <20210628151117.545935-1-aneesh.kumar@linux.ibm.com>
No functional change in this patch.
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/mm/numa.c | 38 +++++++++++++++++++-------------------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index f2bf98bdcea2..8365b298ec48 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -51,7 +51,7 @@ EXPORT_SYMBOL(numa_cpu_lookup_table);
EXPORT_SYMBOL(node_to_cpumask_map);
EXPORT_SYMBOL(node_data);
-static int min_common_depth;
+static int primary_domain_index;
static int n_mem_addr_cells, n_mem_size_cells;
static int form1_affinity;
@@ -232,8 +232,8 @@ static int associativity_to_nid(const __be32 *associativity)
if (!numa_enabled)
goto out;
- if (of_read_number(associativity, 1) >= min_common_depth)
- nid = of_read_number(&associativity[min_common_depth], 1);
+ if (of_read_number(associativity, 1) >= primary_domain_index)
+ nid = of_read_number(&associativity[primary_domain_index], 1);
/* POWER4 LPAR uses 0xffff as invalid node */
if (nid == 0xffff || nid >= nr_node_ids)
@@ -284,9 +284,9 @@ int of_node_to_nid(struct device_node *device)
}
EXPORT_SYMBOL(of_node_to_nid);
-static int __init find_min_common_depth(void)
+static int __init find_primary_domain_index(void)
{
- int depth;
+ int index;
struct device_node *root;
if (firmware_has_feature(FW_FEATURE_OPAL))
@@ -326,7 +326,7 @@ static int __init find_min_common_depth(void)
}
if (form1_affinity) {
- depth = of_read_number(distance_ref_points, 1);
+ index = of_read_number(distance_ref_points, 1);
} else {
if (distance_ref_points_depth < 2) {
printk(KERN_WARNING "NUMA: "
@@ -334,7 +334,7 @@ static int __init find_min_common_depth(void)
goto err;
}
- depth = of_read_number(&distance_ref_points[1], 1);
+ index = of_read_number(&distance_ref_points[1], 1);
}
/*
@@ -348,7 +348,7 @@ static int __init find_min_common_depth(void)
}
of_node_put(root);
- return depth;
+ return index;
err:
of_node_put(root);
@@ -437,16 +437,16 @@ int of_drconf_to_nid_single(struct drmem_lmb *lmb)
int nid = default_nid;
int rc, index;
- if ((min_common_depth < 0) || !numa_enabled)
+ if ((primary_domain_index < 0) || !numa_enabled)
return default_nid;
rc = of_get_assoc_arrays(&aa);
if (rc)
return default_nid;
- if (min_common_depth <= aa.array_sz &&
+ if (primary_domain_index <= aa.array_sz &&
!(lmb->flags & DRCONF_MEM_AI_INVALID) && lmb->aa_index < aa.n_arrays) {
- index = lmb->aa_index * aa.array_sz + min_common_depth - 1;
+ index = lmb->aa_index * aa.array_sz + primary_domain_index - 1;
nid = of_read_number(&aa.arrays[index], 1);
if (nid == 0xffff || nid >= nr_node_ids)
@@ -708,18 +708,18 @@ static int __init parse_numa_properties(void)
return -1;
}
- min_common_depth = find_min_common_depth();
+ primary_domain_index = find_primary_domain_index();
- if (min_common_depth < 0) {
+ if (primary_domain_index < 0) {
/*
- * if we fail to parse min_common_depth from device tree
+ * if we fail to parse primary_domain_index from device tree
* mark the numa disabled, boot with numa disabled.
*/
numa_enabled = false;
- return min_common_depth;
+ return primary_domain_index;
}
- dbg("NUMA associativity depth for CPU/Memory: %d\n", min_common_depth);
+ dbg("NUMA associativity depth for CPU/Memory: %d\n", primary_domain_index);
/*
* Even though we connect cpus to numa domains later in SMP
@@ -919,14 +919,14 @@ static void __init find_possible_nodes(void)
goto out;
}
- max_nodes = of_read_number(&domains[min_common_depth], 1);
+ max_nodes = of_read_number(&domains[primary_domain_index], 1);
for (i = 0; i < max_nodes; i++) {
if (!node_possible(i))
node_set(i, node_possible_map);
}
prop_length /= sizeof(int);
- if (prop_length > min_common_depth + 2)
+ if (prop_length > primary_domain_index + 2)
coregroup_enabled = 1;
out:
@@ -1259,7 +1259,7 @@ int cpu_to_coregroup_id(int cpu)
goto out;
index = of_read_number(associativity, 1);
- if (index > min_common_depth + 1)
+ if (index > primary_domain_index + 1)
return of_read_number(&associativity[index - 1], 1);
out:
--
2.31.1
^ permalink raw reply related
* [PATCH v5 0/6] Add support for FORM2 associativity
From: Aneesh Kumar K.V @ 2021-06-28 15:11 UTC (permalink / raw)
To: linuxppc-dev, mpe
Cc: Nathan Lynch, Aneesh Kumar K.V, Daniel Henrique Barboza,
David Gibson
Form2 associativity adds a much more flexible NUMA topology layout
than what is provided by Form1. More details can be found in patch 7.
$ numactl -H
...
node distances:
node 0 1 2 3
0: 10 11 222 33
1: 44 10 55 66
2: 77 88 10 99
3: 101 121 132 10
$
After DAX kmem memory add
# numactl -H
available: 5 nodes (0-4)
...
node distances:
node 0 1 2 3 4
0: 10 11 222 33 240
1: 44 10 55 66 255
2: 77 88 10 99 255
3: 101 121 132 10 230
4: 255 255 255 230 10
PAPR SCM now use the numa distance details to find the numa_node and target_node
for the device.
kvaneesh@ubuntu-guest:~$ ndctl list -N -v
[
{
"dev":"namespace0.0",
"mode":"devdax",
"map":"dev",
"size":1071644672,
"uuid":"d333d867-3f57-44c8-b386-d4d3abdc2bf2",
"raw_uuid":"915361ad-fe6a-42dd-848f-d6dc9f5af362",
"daxregion":{
"id":0,
"size":1071644672,
"devices":[
{
"chardev":"dax0.0",
"size":1071644672,
"target_node":4,
"mode":"devdax"
}
]
},
"align":2097152,
"numa_node":3
}
]
kvaneesh@ubuntu-guest:~$
The above output is with a Qemu command line
-numa node,nodeid=4 \
-numa dist,src=0,dst=1,val=11 -numa dist,src=0,dst=2,val=222 -numa dist,src=0,dst=3,val=33 -numa dist,src=0,dst=4,val=240 \
-numa dist,src=1,dst=0,val=44 -numa dist,src=1,dst=2,val=55 -numa dist,src=1,dst=3,val=66 -numa dist,src=1,dst=4,val=255 \
-numa dist,src=2,dst=0,val=77 -numa dist,src=2,dst=1,val=88 -numa dist,src=2,dst=3,val=99 -numa dist,src=2,dst=4,val=255 \
-numa dist,src=3,dst=0,val=101 -numa dist,src=3,dst=1,val=121 -numa dist,src=3,dst=2,val=132 -numa dist,src=3,dst=4,val=230 \
-numa dist,src=4,dst=0,val=255 -numa dist,src=4,dst=1,val=255 -numa dist,src=4,dst=2,val=255 -numa dist,src=4,dst=3,val=230 \
-object memory-backend-file,id=memnvdimm1,prealloc=yes,mem-path=$PMEM_DISK,share=yes,size=${PMEM_SIZE} \
-device nvdimm,label-size=128K,memdev=memnvdimm1,id=nvdimm1,slot=4,uuid=72511b67-0b3b-42fd-8d1d-5be3cae8bcaa,node=4
Qemu changes can be found at https://lore.kernel.org/qemu-devel/20210616011944.2996399-1-danielhb413@gmail.com/
Changes from v4:
* Drop DLPAR related device tree property for now because both Qemu nor PowerVM
will provide the distance details of all possible NUMA nodes during boot.
* Rework numa distance code based on review feedback.
Changes from v3:
* Drop PAPR SCM specific changes and depend completely on NUMA distance information.
Changes from v2:
* Add nvdimm list to Cc:
* update PATCH 8 commit message.
Changes from v1:
* Update FORM2 documentation.
* rename max_domain_index to max_associativity_domain_index
Aneesh Kumar K.V (6):
powerpc/pseries: rename min_common_depth to primary_domain_index
powerpc/pseries: rename distance_ref_points_depth to
max_associativity_domain_index
powerpc/pseries: Rename TYPE1_AFFINITY to FORM1_AFFINITY
powerpc/pseries: Consolidate different NUMA distance update code paths
powerpc/pseries: Add a helper for form1 cpu distance
powerpc/pseries: Add support for FORM2 associativity
Documentation/powerpc/associativity.rst | 103 +++++
arch/powerpc/include/asm/firmware.h | 7 +-
arch/powerpc/include/asm/prom.h | 3 +-
arch/powerpc/include/asm/topology.h | 4 +-
arch/powerpc/kernel/prom_init.c | 3 +-
arch/powerpc/mm/numa.c | 415 +++++++++++++-----
arch/powerpc/platforms/pseries/firmware.c | 3 +-
arch/powerpc/platforms/pseries/hotplug-cpu.c | 2 +
.../platforms/pseries/hotplug-memory.c | 2 +
arch/powerpc/platforms/pseries/lpar.c | 4 +-
arch/powerpc/platforms/pseries/pseries.h | 1 +
11 files changed, 432 insertions(+), 115 deletions(-)
create mode 100644 Documentation/powerpc/associativity.rst
--
2.31.1
^ permalink raw reply
* Re: [PATCH] perf script python: Fix buffer size to report iregs in perf script
From: Paul A. Clarke @ 2021-06-28 14:49 UTC (permalink / raw)
To: Kajol Jain
Cc: ravi.bangoria, atrajeev, rnsastry, linuxppc-dev, linux-kernel,
acme, linux-perf-users, maddy, jolsa
In-Reply-To: <20210628062341.155839-1-kjain@linux.ibm.com>
On Mon, Jun 28, 2021 at 11:53:41AM +0530, Kajol Jain wrote:
> Commit 48a1f565261d ("perf script python: Add more PMU fields
> to event handler dict") added functionality to report fields like
> weight, iregs, uregs etc via perf report.
> That commit predefined buffer size to 512 bytes to print those fields.
>
> But incase of powerpc, since we added extended regs support
> in commits:
>
> Commit 068aeea3773a ("perf powerpc: Support exposing Performance Monitor
> Counter SPRs as part of extended regs")
> Commit d735599a069f ("powerpc/perf: Add extended regs support for
> power10 platform")
>
> Now iregs can carry more bytes of data and this predefined buffer size
> can result to data loss in perf script output.
>
> Patch resolve this issue by making buffer size dynamic based on number
> of registers needed to print. It also changed return type for function
> "regs_map" from int to void, as the return value is not being used by
> the caller function "set_regs_in_dict".
>
> Fixes: 068aeea3773a ("perf powerpc: Support exposing Performance Monitor
> Counter SPRs as part of extended regs")
> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
> ---
> .../util/scripting-engines/trace-event-python.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
> index 4e4aa4c97ac5..c8c9706b4643 100644
> --- a/tools/perf/util/scripting-engines/trace-event-python.c
> +++ b/tools/perf/util/scripting-engines/trace-event-python.c
[...]
> @@ -713,7 +711,16 @@ static void set_regs_in_dict(PyObject *dict,
> struct evsel *evsel)
> {
> struct perf_event_attr *attr = &evsel->core.attr;
> - char bf[512];
> +
> + /*
> + * Here value 28 is a constant size which can be used to print
> + * one register value and its corresponds to:
> + * 16 chars is to specify 64 bit register in hexadecimal.
> + * 2 chars is for appending "0x" to the hexadecimal value and
> + * 10 chars is for register name.
> + */
> + int size = __sw_hweight64(attr->sample_regs_intr) * 28;
> + char bf[size];
I propose using a template rather than a magic number here. Something like:
const char reg_name_tmpl[] = "10 chars ";
const char reg_value_tmpl[] = "0x0123456789abcdef";
const int size = __sw_hweight64(attr->sample_regs_intr) +
sizeof reg_name_tmpl + sizeof reg_value_tmpl;
Pardon my ignorance, but is there no separation/whitespace between the name
and the value? And is there some significance to 10 characters for the
register name, or is that a magic number?
PC
^ permalink raw reply
* Re: [PATCH 5/8] powerpc/64: enable MSR[EE] in irq replay pt_regs
From: Sachin Sant @ 2021-06-28 14:37 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev
In-Reply-To: <20210628074932.1499554-6-npiggin@gmail.com>
> On 28-Jun-2021, at 1:19 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
>
> Similar to 2b48e96be2f9f ("powerpc/64: fix irq replay pt_regs->softe
> value"), enable MSR_EE in pt_regs->msr, which makes the regs look a
> bit more normal and allows the extra debug checks to be added to
> interrupt handler entry.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/interrupt.h | 4 ++++
> arch/powerpc/kernel/irq.c | 1 +
> 2 files changed, 5 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
> index 789311d1e283..d4bdf7d274ac 100644
> --- a/arch/powerpc/include/asm/interrupt.h
> +++ b/arch/powerpc/include/asm/interrupt.h
> @@ -173,6 +173,8 @@ static inline void interrupt_enter_prepare(struct pt_regs *regs, struct interrup
> BUG_ON(search_kernel_restart_table(regs->nip));
> #endif
> }
> + if (IS_ENABLED(CONFIG_PPC_IRQ_SOFT_MASK_DEBUG))
> + BUG_ON(!arch_irq_disabled_regs(regs) && !(regs->msr & MSR_EE));
> #endif
I think this BUG_ON was triggered while running selftests (powerpc/mm/pkey_exec_prot)
[ 9741.254969] ------------[ cut here ]------------
[ 9741.254978] kernel BUG at arch/powerpc/include/asm/interrupt.h:177!
[ 9741.254985] Oops: Exception in kernel mode, sig: 5 [#1]
[ 9741.254990] LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
[ 9741.254995] Modules linked in: rpadlpar_io rpaphp uinput sha512_generic vmac n_gsm pps_ldisc pps_core ppp_synctty ppp_async ppp_generic slcan slip slhc snd_hrtimer snd_seq snd_seq_device snd_timer snd soundcore authenc pcrypt crypto_user n_hdlc dummy veth nfsv3 nfs_acl nfs lockd grace fscache netfs tun brd overlay vfat fat btrfs blake2b_generic xor zstd_compress raid6_pq xfs loop sctp ip6_udp_tunnel udp_tunnel dm_mod bonding nft_ct nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip_set rfkill nf_tables libcrc32c nfnetlink sunrpc pseries_rng xts vmx_crypto uio_pdrv_genirq uio sch_fq_codel ip_tables ext4 mbcache jbd2 sr_mod sd_mod cdrom t10_pi sg ibmvscsi ibmveth scsi_transport_srp fuse [last unloaded: test_cpuidle_latency]
[ 9741.255097] CPU: 17 PID: 3278920 Comm: pkey_exec_prot Tainted: G W OE 5.13.0-rc7-next-20210625-dirty #4
[ 9741.255106] NIP: c0000000000300d8 LR: c000000000009604 CTR: c000000000009330
[ 9741.255111] REGS: c0000000347536f0 TRAP: 0700 Tainted: G W OE (5.13.0-rc7-next-20210625-dirty)
[ 9741.255117] MSR: 8000000000021033 <SF,ME,IR,DR,RI,LE> CR: 22004282 XER: 20040000
[ 9741.255130] CFAR: c00000000003007c IRQMASK: 3
[ 9741.255130] GPR00: c000000000093cd0 c000000034753990 c0000000029bbe00 c000000034753a30
[ 9741.255130] GPR04: 00007fff9ebb0000 0000000000200000 000000000000000a 000000000000002d
[ 9741.255130] GPR08: 0000000000000000 0000000000000001 0000000000000000 7265677368657265
[ 9741.255130] GPR12: 8000000000021033 c00000001ec27280 0000000000000000 0000000000000000
[ 9741.255130] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 9741.255130] GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000010003c40
[ 9741.255130] GPR24: 0000000000000000 0000000000000000 0000000000200000 c00000005e89d200
[ 9741.255130] GPR28: 0000000000000300 00007fff9ebb0000 c000000034753e80 c000000034753a30
[ 9741.255191] NIP [c0000000000300d8] program_check_exception+0xe8/0x1c0
[ 9741.255202] LR [c000000000009604] program_check_common_virt+0x2d4/0x320
[ 9741.255209] Call Trace:
[ 9741.255212] [c000000034753990] [0000000000000008] 0x8 (unreliable)
[ 9741.255219] [c0000000347539c0] [c000000034753a80] 0xc000000034753a80
[ 9741.255225] --- interrupt: 700 at arch_local_irq_restore+0x1d0/0x200
[ 9741.255231] NIP: c000000000016790 LR: c000000000093388 CTR: c000000000008780
[ 9741.255236] REGS: c000000034753a30 TRAP: 0700 Tainted: G W OE (5.13.0-rc7-next-20210625-dirty)
[ 9741.255242] MSR: 8000000000021033 <SF,ME,IR,DR,RI,LE> CR: 24004288 XER: 20040000
[ 9741.255253] CFAR: c0000000000165ec IRQMASK: 0
[ 9741.255253] GPR00: c000000000093cd0 c000000034753cd0 c0000000029bbe00 0000000000000000
[ 9741.255253] GPR04: 00007fff9ebb0000 0000000000200000 000000000000000a 000000000000002d
[ 9741.255253] GPR08: 0000000000000000 0000000000000000 c0000000bd77d400 7265677368657265
[ 9741.255253] GPR12: 0000000044000282 c00000001ec27280 0000000000000000 0000000000000000
[ 9741.255253] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 9741.255253] GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000010003c40
[ 9741.255253] GPR24: 0000000000000000 0000000000000000 0000000000200000 c00000005e89d200
[ 9741.255253] GPR28: 0000000000000300 00007fff9ebb0000 c000000034753e80 0000000000000001
[ 9741.255313] NIP [c000000000016790] arch_local_irq_restore+0x1d0/0x200
[ 9741.255319] LR [c000000000093388] ___do_page_fault+0x438/0xb80
[ 9741.255325] --- interrupt: 700
[ 9741.255328] [c000000034753cd0] [c00000000009be74] hash_page_mm+0x5e4/0x800 (unreliable)
[ 9741.255335] [c000000034753d00] [000000000000002d] 0x2d
[ 9741.255340] [c000000034753db0] [c000000000093cd0] hash__do_page_fault+0x30/0x70
[ 9741.255348] [c000000034753de0] [c00000000009c438] do_hash_fault+0x78/0xb0
[ 9741.255354] [c000000034753e10] [c00000000000891c] data_access_common_virt+0x19c/0x1f0
[ 9741.255361] --- interrupt: 300 at 0x10001e8c
[ 9741.255365] NIP: 0000000010001e8c LR: 0000000010001e84 CTR: 00007fff9ea4eb60
[ 9741.255370] REGS: c000000034753e80 TRAP: 0300 Tainted: G W OE (5.13.0-rc7-next-20210625-dirty)
[ 9741.255376] MSR: 800000000280f033 <SF,VEC,VSX,EE,PR,FP,ME,IR,DR,RI,LE> CR: 28000282 XER: 20040000
[ 9741.255391] CFAR: c00000000000ca44 DAR: 00007fff9ebb0000 DSISR: 00200000 IRQMASK: 0
[ 9741.255391] GPR00: 0000000010001e84 00007fffd11fa7b0 0000000010027f00 0000000000000033
[ 9741.255391] GPR04: 0000000010003c3d 0000000000000001 000000000000000a 000000000000002d
[ 9741.255391] GPR08: 0000000000000000 00007fff9ebb0000 0000000000000000 000001002ab20337
[ 9741.255391] GPR12: 0000000000000000 00007fff9ec4a270 0000000000000000 0000000000000000
[ 9741.255391] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 9741.255391] GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000010003c40
[ 9741.255391] GPR24: 0000000000000001 0000000010003c18 000000000000002d 0000000010020160
[ 9741.255391] GPR28: 00c0000000000000 0000000010020230 0000000000000002 0000000000000004
[ 9741.255455] NIP [0000000010001e8c] 0x10001e8c
[ 9741.255459] LR [0000000010001e84] 0x10001e84
[ 9741.255463] --- interrupt: 300
[ 9741.255466] Instruction dump:
[ 9741.255470] 60000000 e8010040 7c6a1b78 7c0803a6 0b0a0000 e93f0138 71290001 4082004c
[ 9741.255481] e95f0108 39200001 714a8000 4082ff68 <0b090000> 38210030 7fe3fb78 ebe1fff8
[ 9741.255494] ---[ end trace c668c70ea0d5061f ]—
Thanks
-Sachin
>
> booke_restore_dbcr0();
> @@ -268,6 +270,8 @@ static inline void interrupt_nmi_enter_prepare(struct pt_regs *regs, struct inte
> // arch_irq_disabled_regs(regs) behaves as expected.
> regs->softe = IRQS_ALL_DISABLED;
> }
> + if (IS_ENABLED(CONFIG_PPC_IRQ_SOFT_MASK_DEBUG))
> + BUG_ON(!arch_irq_disabled_regs(regs) && !(regs->msr & MSR_EE));
>
> /* Don't do any per-CPU operations until interrupt state is fixed */
>
> diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
> index 8428caf3194e..91e63eac4e8f 100644
> --- a/arch/powerpc/kernel/irq.c
> +++ b/arch/powerpc/kernel/irq.c
> @@ -121,6 +121,7 @@ void replay_soft_interrupts(void)
>
> ppc_save_regs(®s);
> regs.softe = IRQS_ENABLED;
> + regs.msr |= MSR_EE;
>
> again:
> if (IS_ENABLED(CONFIG_PPC_IRQ_SOFT_MASK_DEBUG))
> --
> 2.23.0
>
^ permalink raw reply
* Re: [PATCH] perf vendor events power10: Adds 24x7 nest metric events for power10 platform
From: Paul A. Clarke @ 2021-06-28 14:30 UTC (permalink / raw)
To: kajoljain
Cc: ravi.bangoria, atrajeev, rnsastry, jolsa, linux-kernel, acme,
linux-perf-users, maddy, linuxppc-dev
In-Reply-To: <d2eaebb6-7cd6-d2e3-0bed-0c054e7d2207@linux.ibm.com>
On Mon, Jun 28, 2021 at 11:58:54AM +0530, kajoljain wrote:
>
>
> On 6/25/21 6:51 PM, Paul A. Clarke wrote:
> > On Fri, Jun 25, 2021 at 05:29:48PM +0530, Kajol Jain wrote:
> >> Patch adds 24x7 nest metric events for POWER10.
> >>
> >> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
> >> ---
> >> .../arch/powerpc/power10/nest_metrics.json | 491 ++++++++++++++++++
> >> 1 file changed, 491 insertions(+)
> >> create mode 100644 tools/perf/pmu-events/arch/powerpc/power10/nest_metrics.json
> >>
> >> diff --git a/tools/perf/pmu-events/arch/powerpc/power10/nest_metrics.json b/tools/perf/pmu-events/arch/powerpc/power10/nest_metrics.json
> >> new file mode 100644
> >> index 000000000000..b79046cd8b09
> >> --- /dev/null
> >> +++ b/tools/perf/pmu-events/arch/powerpc/power10/nest_metrics.json
> >> @@ -0,0 +1,491 @@
> >> +[
> >> + {
> >> + "MetricName": "VEC_GROUP_PUMP_RETRY_RATIO_P01",
> >> + "BriefDescription": "VEC_GROUP_PUMP_RETRY_RATIO_P01",
> >
> > Is it possible to get better descriptions than just a restatement of the
> > name, or no description at all?
> >
> > This comment obviously applies to almost all of the metrics herein.
>
> Hi Paul,
> Thanks for reviewing the patch. Sure I will remove description part for now.
My sentence didn't parse well, sorry...
What I really meant was more like "Is it possible to get better descriptions?
Having just a restatement of the name (or no description at all in some cases)
is not helpful."
So, can we provide better descriptions of the metrics?
PC
^ permalink raw reply
* Re: [PATCH 3/8] powerpc/64s: add a table of implicit soft-masked addresses
From: Sachin Sant @ 2021-06-28 14:26 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev
In-Reply-To: <20210628074932.1499554-4-npiggin@gmail.com>
> On 28-Jun-2021, at 1:19 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
>
> Commit 9d1988ca87dd ("powerpc/64: treat low kernel text as irqs
> soft-masked") ends up catching too much code, including ret_from_fork,
> and parts of interrupt and syscall return that do not expect to be
> interrupts to be soft-masked. If an interrupt gets marked pending,
> and then the code proceeds out of the implicit soft-masked region it
> will fail to deal with the pending interrupt.
>
> Fix this by adding a new table of addresses which explicitly marks
> the regions of code that are soft masked. This table is only checked
> for interrupts that below __end_soft_masked, so most kernel interrupts
> will not have the overhead of the table search.
>
> Fixes: 9d1988ca87dd ("powerpc/64: treat low kernel text as irqs soft-masked")
> Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Thanks Nick for the fix.
I was able to verify this patch.
Both kernel boot and test ran to completion without the reported warning.
Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
-Sachin
^ 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