* Re: [PATCH v3 1/2] powerpc/perf/hv-24x7: Add cpu hotplug support
From: Madhavan Srinivasan @ 2020-07-07 4:15 UTC (permalink / raw)
To: Michael Ellerman, Kajol Jain, linuxppc-dev
Cc: nathanl, ego, suka, maddy, anju
In-Reply-To: <87zh8d5oab.fsf@mpe.ellerman.id.au>
On 7/6/20 8:43 AM, Michael Ellerman wrote:
> Kajol Jain <kjain@linux.ibm.com> writes:
>> Patch here adds cpu hotplug functions to hv_24x7 pmu.
>> A new cpuhp_state "CPUHP_AP_PERF_POWERPC_HV_24x7_ONLINE" enum
>> is added.
>>
>> The online callback function updates the cpumask only if its
>> empty. As the primary intention of adding hotplug support
>> is to designate a CPU to make HCALL to collect the
>> counter data.
>>
>> The offline function test and clear corresponding cpu in a cpumask
>> and update cpumask to any other active cpu.
>>
>> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
>> Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
>> ---
>> arch/powerpc/perf/hv-24x7.c | 45 +++++++++++++++++++++++++++++++++++++
>> include/linux/cpuhotplug.h | 1 +
>> 2 files changed, 46 insertions(+)
>>
>> diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
>> index db213eb7cb02..ce4739e2b407 100644
>> --- a/arch/powerpc/perf/hv-24x7.c
>> +++ b/arch/powerpc/perf/hv-24x7.c
>> @@ -31,6 +31,8 @@ static int interface_version;
>> /* Whether we have to aggregate result data for some domains. */
>> static bool aggregate_result_elements;
>>
>> +static cpumask_t hv_24x7_cpumask;
>> +
>> static bool domain_is_valid(unsigned domain)
>> {
>> switch (domain) {
>> @@ -1641,6 +1643,44 @@ static struct pmu h_24x7_pmu = {
>> .capabilities = PERF_PMU_CAP_NO_EXCLUDE,
>> };
>>
>> +static int ppc_hv_24x7_cpu_online(unsigned int cpu)
>> +{
>> + /* Make this CPU the designated target for counter collection */
> The comment implies every newly onlined CPU will become the target, but
> actually it's only the first onlined CPU.
>
> So I think the comment needs updating, or you could just drop the
> comment, I think the code is fairly clear by itself.
>
>> + if (cpumask_empty(&hv_24x7_cpumask))
>> + cpumask_set_cpu(cpu, &hv_24x7_cpumask);
>> +
>> + return 0;
>> +}
>> +
>> +static int ppc_hv_24x7_cpu_offline(unsigned int cpu)
>> +{
>> + int target = -1;
> No need to initialise target, you assign to it unconditionally below.
>
>> + /* Check if exiting cpu is used for collecting 24x7 events */
>> + if (!cpumask_test_and_clear_cpu(cpu, &hv_24x7_cpumask))
>> + return 0;
>> +
>> + /* Find a new cpu to collect 24x7 events */
>> + target = cpumask_last(cpu_active_mask);
> Any reason to use cpumask_last() vs cpumask_first(), or a randomly
> chosen CPU?
>
>> + if (target < 0 || target >= nr_cpu_ids)
>> + return -1;
>> +
>> + /* Migrate 24x7 events to the new target */
>> + cpumask_set_cpu(target, &hv_24x7_cpumask);
>> + perf_pmu_migrate_context(&h_24x7_pmu, cpu, target);
>> +
>> + return 0;
>> +}
>> +
>> +static int hv_24x7_cpu_hotplug_init(void)
>> +{
>> + return cpuhp_setup_state(CPUHP_AP_PERF_POWERPC_HV_24x7_ONLINE,
>> + "perf/powerpc/hv_24x7:online",
>> + ppc_hv_24x7_cpu_online,
>> + ppc_hv_24x7_cpu_offline);
>> +}
>> +
>> static int hv_24x7_init(void)
>> {
>> int r;
>> @@ -1685,6 +1725,11 @@ static int hv_24x7_init(void)
>> if (r)
>> return r;
>>
>> + /* init cpuhotplug */
>> + r = hv_24x7_cpu_hotplug_init();
>> + if (r)
>> + pr_err("hv_24x7: CPU hotplug init failed\n");
>> +
> The hotplug initialisation shouldn't fail unless something is badly
> wrong. I think you should just fail initialisation of the entire PMU if
> that happens, which will make the error handling in the next patch much
> simpler.
We did fail the PMU registration on failure of the hotplug
code (and yes error handling is much simpler), but on internal
review/discussion,
what came up was that, hv_24x7 PMU will still be usable without
the hotplug code (with "-C" option to perf tool command line).
Maddy
>
> cheers
>
>> r = perf_pmu_register(&h_24x7_pmu, h_24x7_pmu.name, -1);
>> if (r)
>> return r;
^ permalink raw reply
* Re: [PATCH] powerpc/numa: Restrict possible nodes based on platform
From: Srikar Dronamraju @ 2020-07-07 2:53 UTC (permalink / raw)
To: Nathan Lynch; +Cc: Tyrel Datwyler, linuxppc-dev, Bharata B Rao
In-Reply-To: <874kqkf91s.fsf@linux.ibm.com>
* Nathan Lynch <nathanl@linux.ibm.com> [2020-07-06 19:44:31]:
> Tyrel Datwyler <tyreld@linux.ibm.com> writes:
> >> --- a/arch/powerpc/mm/numa.c
> >> +++ b/arch/powerpc/mm/numa.c
> >> @@ -897,7 +897,7 @@ static void __init find_possible_nodes(void)
> >> return;
> >>
> >> if (of_property_read_u32_index(rtas,
> >> - "ibm,max-associativity-domains",
> >> + "ibm,current-associativity-domains",
> >
> > I'm not sure ibm,current-associativity-domains is guaranteed to exist on older
> > firmware. You may need check that it exists and fall back to
> > ibm,max-associativity-domains in the event it doesn't
>
> Yes. Looks like it's a PowerVM-specific property.
Right, this is just a PowerVM specific property and doesn't affect PowerNV.
On PowerNV thought we have sparse nodes, the max possible nodes is set
correctly.
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply
* Re: [PATCH] powerpc/pseries: detect secure and trusted boot state of the system.
From: Daniel Axtens @ 2020-07-07 2:53 UTC (permalink / raw)
To: Nayna Jain, linuxppc-dev; +Cc: Nayna Jain, linux-kernel, Mimi Zohar
In-Reply-To: <87a70c3wpj.fsf@dja-thinkpad.axtens.net>
As a final extra note,
https://github.com/daxtens/qemu/tree/pseries-secboot
is a qemu repository that fakes out the relevant properties if anyone
else wants to test this.
Also,
> 3-9 - enabled, OS-defined behaviour. In this patch we map all these
> values to enabled and enforcing. Again I think this is the
> appropriate thing to do.
this should read "enabled and enforcing, requirements are at the
discretion of the operating system". Apologies.
Regards,
Daniel
> ibm,trusted-boot isn't published by a current PowerVM LPAR but will be
> published in future. (Currently, trusted boot state is inferred by the
> presence or absense of a vTPM.) It's simply 1 = enabled, 0 = disabled.
>
> As for this patch specifically, with the very small nits below,
>
> Reviewed-by: Daniel Axtens <dja@axtens.net>
>
>> - node = get_ppc_fw_sb_node();
>> - enabled = of_property_read_bool(node, "os-secureboot-enforcing");
>> + if (machine_is(powernv)) {
>> + node = get_ppc_fw_sb_node();
>> + enabled =
>> + of_property_read_bool(node, "os-secureboot-enforcing");
>> + of_node_put(node);
>> + }
>>
>> - of_node_put(node);
>> + if (machine_is(pseries)) {
> Maybe this should be an else if?
>
>> + secureboot = of_get_property(of_root, "ibm,secure-boot", NULL);
>> + if (secureboot)
>> + enabled = (*secureboot > 1) ? true : false;
>> + }
>>
>> pr_info("Secure boot mode %s\n", enabled ? "enabled" : "disabled");
>>
>> @@ -38,11 +48,20 @@ bool is_ppc_trustedboot_enabled(void)
>> {
>> struct device_node *node;
>> bool enabled = false;
>> + const u32 *trustedboot;
>>
>> - node = get_ppc_fw_sb_node();
>> - enabled = of_property_read_bool(node, "trusted-enabled");
>> + if (machine_is(powernv)) {
>> + node = get_ppc_fw_sb_node();
>> + enabled = of_property_read_bool(node, "trusted-enabled");
>> + of_node_put(node);
>> + }
>>
>> - of_node_put(node);
>> + if (machine_is(pseries)) {
> Likewise.
>> + trustedboot =
>> + of_get_property(of_root, "ibm,trusted-boot", NULL);
>> + if (trustedboot)
>> + enabled = (*trustedboot > 0) ? true : false;
>
> Regards,
> Daniel
^ permalink raw reply
* Re: [PATCH] powerpc/numa: Restrict possible nodes based on platform
From: Srikar Dronamraju @ 2020-07-07 2:50 UTC (permalink / raw)
To: Tyrel Datwyler; +Cc: Nathan Lynch, linuxppc-dev, Bharata B Rao
In-Reply-To: <00388e11-9025-e273-137d-c23f8795457a@linux.ibm.com>
* Tyrel Datwyler <tyreld@linux.ibm.com> [2020-07-06 13:58:42]:
> On 7/5/20 11:40 PM, Srikar Dronamraju wrote:
> > diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> > index 9fcf2d195830..3d55cef1a2dc 100644
> > --- a/arch/powerpc/mm/numa.c
> > +++ b/arch/powerpc/mm/numa.c
> > @@ -897,7 +897,7 @@ static void __init find_possible_nodes(void)
> > return;
> >
> > if (of_property_read_u32_index(rtas,
> > - "ibm,max-associativity-domains",
> > + "ibm,current-associativity-domains",
>
> I'm not sure ibm,current-associativity-domains is guaranteed to exist on older
> firmware. You may need check that it exists and fall back to
> ibm,max-associativity-domains in the event it doesn't
>
> -Tyrel
>
Oh,
thanks Tyrel thats a good observation.
Will fallback on max-associativity.
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply
* RE: [PATCH v6 00/11] Add the multiple PF support for DWC and Layerscape
From: Z.q. Hou @ 2020-07-07 2:10 UTC (permalink / raw)
To: Lorenzo Pieralisi, Xiaowei Bao
Cc: Roy Zang, devicetree@vger.kernel.org, jingoohan1@gmail.com,
linuxppc-dev@lists.ozlabs.org, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, Leo Li, M.h. Lian,
robh+dt@kernel.org, linux-arm-kernel@lists.infradead.org,
gustavo.pimentel@synopsys.com, bhelgaas@google.com,
andrew.murray@arm.com, kishon@ti.com, shawnguo@kernel.org,
Mingkai Hu, amurray@thegoodpenguin.co.uk
In-Reply-To: <20200706104639.GC26377@e121166-lin.cambridge.arm.com>
Hi Lorenzo,
> -----Original Message-----
> From: Lorenzo Pieralisi [mailto:lorenzo.pieralisi@arm.com]
> Sent: 2020年7月6日 18:47
> To: Xiaowei Bao <xiaowei.bao@nxp.com>
> Cc: Z.q. Hou <zhiqiang.hou@nxp.com>; M.h. Lian <minghuan.lian@nxp.com>;
> Mingkai Hu <mingkai.hu@nxp.com>; bhelgaas@google.com;
> robh+dt@kernel.org; shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>;
> kishon@ti.com; Roy Zang <roy.zang@nxp.com>;
> amurray@thegoodpenguin.co.uk; jingoohan1@gmail.com;
> gustavo.pimentel@synopsys.com; andrew.murray@arm.com;
> linux-pci@vger.kernel.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH v6 00/11] Add the multiple PF support for DWC and
> Layerscape
>
> On Sat, Mar 14, 2020 at 11:30:27AM +0800, Xiaowei Bao wrote:
> > Add the PCIe EP multiple PF support for DWC and Layerscape, add the
> > doorbell MSIX function for DWC, use list to manage the PF of one PCIe
> > controller, and refactor the Layerscape EP driver due to some
> > platforms difference.
> >
> > Xiaowei Bao (11):
> > PCI: designware-ep: Add multiple PFs support for DWC
> > PCI: designware-ep: Add the doorbell mode of MSI-X in EP mode
> > PCI: designware-ep: Move the function of getting MSI capability
> > forward
> > PCI: designware-ep: Modify MSI and MSIX CAP way of finding
> > dt-bindings: pci: layerscape-pci: Add compatible strings for ls1088a
> > and ls2088a
> > PCI: layerscape: Fix some format issue of the code
> > PCI: layerscape: Modify the way of getting capability with different
> > PEX
> > PCI: layerscape: Modify the MSIX to the doorbell mode
> > PCI: layerscape: Add EP mode support for ls1088a and ls2088a
> > arm64: dts: layerscape: Add PCIe EP node for ls1088a
> > misc: pci_endpoint_test: Add LS1088a in pci_device_id table
> >
> > .../devicetree/bindings/pci/layerscape-pci.txt | 2 +
> > arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 31 +++
> > drivers/misc/pci_endpoint_test.c | 2 +
> > drivers/pci/controller/dwc/pci-layerscape-ep.c | 100 ++++++--
> > drivers/pci/controller/dwc/pcie-designware-ep.c | 255
> +++++++++++++++++----
> > drivers/pci/controller/dwc/pcie-designware.c | 59 +++--
> > drivers/pci/controller/dwc/pcie-designware.h | 48 +++-
> > 7 files changed, 404 insertions(+), 93 deletions(-)
>
> Can you rebase it against v5.8-rc1 please ?
Yes, I will help to rebase.
Thanks,
Zhiqiang
>
> Thanks,
> Lorenzo
^ permalink raw reply
* Re: [PATCH] powerpc/pseries: detect secure and trusted boot state of the system.
From: Daniel Axtens @ 2020-07-07 2:06 UTC (permalink / raw)
To: Nayna Jain, linuxppc-dev; +Cc: Nayna Jain, linux-kernel, Mimi Zohar
In-Reply-To: <1593882535-21368-1-git-send-email-nayna@linux.ibm.com>
Thanks Nayna!
I'm hoping to get better public documentation for this soon as it's not
documented in a public PAPR yet.
Until then:
The values of ibm,secure-boot under PowerVM are:
0 - disabled
1 - audit mode only. This patch ignores this value for Linux, which I
think is the appropriate thing to do.
2 - enabled and enforcing
3-9 - enabled, OS-defined behaviour. In this patch we map all these
values to enabled and enforcing. Again I think this is the
appropriate thing to do.
ibm,trusted-boot isn't published by a current PowerVM LPAR but will be
published in future. (Currently, trusted boot state is inferred by the
presence or absense of a vTPM.) It's simply 1 = enabled, 0 = disabled.
As for this patch specifically, with the very small nits below,
Reviewed-by: Daniel Axtens <dja@axtens.net>
> - node = get_ppc_fw_sb_node();
> - enabled = of_property_read_bool(node, "os-secureboot-enforcing");
> + if (machine_is(powernv)) {
> + node = get_ppc_fw_sb_node();
> + enabled =
> + of_property_read_bool(node, "os-secureboot-enforcing");
> + of_node_put(node);
> + }
>
> - of_node_put(node);
> + if (machine_is(pseries)) {
Maybe this should be an else if?
> + secureboot = of_get_property(of_root, "ibm,secure-boot", NULL);
> + if (secureboot)
> + enabled = (*secureboot > 1) ? true : false;
> + }
>
> pr_info("Secure boot mode %s\n", enabled ? "enabled" : "disabled");
>
> @@ -38,11 +48,20 @@ bool is_ppc_trustedboot_enabled(void)
> {
> struct device_node *node;
> bool enabled = false;
> + const u32 *trustedboot;
>
> - node = get_ppc_fw_sb_node();
> - enabled = of_property_read_bool(node, "trusted-enabled");
> + if (machine_is(powernv)) {
> + node = get_ppc_fw_sb_node();
> + enabled = of_property_read_bool(node, "trusted-enabled");
> + of_node_put(node);
> + }
>
> - of_node_put(node);
> + if (machine_is(pseries)) {
Likewise.
> + trustedboot =
> + of_get_property(of_root, "ibm,trusted-boot", NULL);
> + if (trustedboot)
> + enabled = (*trustedboot > 0) ? true : false;
Regards,
Daniel
^ permalink raw reply
* Re: [PATCH] MAINTAINERS: Add Shengjiu to reviewer list of sound/soc/fsl
From: Shengjiu Wang @ 2020-07-07 1:51 UTC (permalink / raw)
To: Nicolin Chen
Cc: Linux-ALSA, Li.Xiubo, Shengjiu Wang, linuxppc-dev, timur,
linux-kernel, Mark Brown, Fabio Estevam
In-Reply-To: <20200702193102.25282-1-nicoleotsuka@gmail.com>
On Fri, Jul 3, 2020 at 3:33 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
>
> Add Shengjiu who's actively working on the latest fsl/nxp audio drivers.
>
> Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
> Cc: Shengjiu Wang <shengjiu.wang@nxp.com>
> ---
> To Shengjiu, please ack if you feel okay with this (your email address too).
Thanks Nicolin for nominating me as a reviewer.
I'd like to use my gmail address "shengjiu.wang@gmail.com".
with this then
Acked-by: Shengjiu Wang <shengjiu.wang@gmail.com>
best regards
wang shengjiu
>
> MAINTAINERS | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 496fd4eafb68..54aab083bb88 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6956,6 +6956,7 @@ M: Timur Tabi <timur@kernel.org>
> M: Nicolin Chen <nicoleotsuka@gmail.com>
> M: Xiubo Li <Xiubo.Lee@gmail.com>
> R: Fabio Estevam <festevam@gmail.com>
> +R: Shengjiu Wang <shengjiu.wang@nxp.com>
> L: alsa-devel@alsa-project.org (moderated for non-subscribers)
> L: linuxppc-dev@lists.ozlabs.org
> S: Maintained
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH v5 10/26] powerpc/book3s64/pkeys: Convert pkey_total to max_pkey
From: Michael Ellerman @ 2020-07-07 1:26 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev; +Cc: linuxram, bauerman
In-Reply-To: <15527f2f-1b50-2ff2-3e05-b03dec985391@linux.ibm.com>
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> On 7/6/20 12:34 PM, Michael Ellerman wrote:
>> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>>> max_pkey now represents max key value that userspace can allocate.
>>>
>
> I guess commit message is confusing.
And the name.
If it's called max_pkey then it should be the maximum allowed pkey
value.
cheers
>>> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
>>> index 75d2a2c19c04..652bad7334f3 100644
>>> --- a/arch/powerpc/include/asm/pkeys.h
>>> +++ b/arch/powerpc/include/asm/pkeys.h
>>> @@ -12,7 +12,7 @@
>>> #include <asm/firmware.h>
>>>
>>> DECLARE_STATIC_KEY_FALSE(pkey_disabled);
>>> -extern int pkeys_total; /* total pkeys as per device tree */
>>> +extern int max_pkey;
>>> extern u32 initial_allocation_mask; /* bits set for the initially allocated keys */
>>> extern u32 reserved_allocation_mask; /* bits set for reserved keys */
>>>
>>> @@ -44,7 +44,10 @@ static inline int vma_pkey(struct vm_area_struct *vma)
>>> return (vma->vm_flags & ARCH_VM_PKEY_FLAGS) >> VM_PKEY_SHIFT;
>>> }
>>>
>>> -#define arch_max_pkey() pkeys_total
>>> +static inline int arch_max_pkey(void)
>>> +{
>>> + return max_pkey;
>>> +}
>>
>> If pkeys_total = 32 then max_pkey = 31.
>
> we have
>
> #ifdef CONFIG_PPC_4K_PAGES
> /*
> * The OS can manage only 8 pkeys due to its inability to represent them
> * in the Linux 4K PTE. Mark all other keys reserved.
> */
> max_pkey = min(8, pkeys_total);
> #else
> max_pkey = pkeys_total;
> #endif
>
> so it is 32.
>
>>
>> So we can't just substitute one for the other. ie. arch_max_pkey() must
>> have been wrong, or it is wrong now.
>>
>>> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
>>> index 87d882a9aaf2..a4d7287082a8 100644
>>> --- a/arch/powerpc/mm/book3s64/pkeys.c
>>> +++ b/arch/powerpc/mm/book3s64/pkeys.c
>>> @@ -14,7 +14,7 @@
>>>
>>> DEFINE_STATIC_KEY_FALSE(pkey_disabled);
>>> DEFINE_STATIC_KEY_FALSE(execute_pkey_disabled);
>>> -int pkeys_total; /* Total pkeys as per device tree */
>>> +int max_pkey; /* Maximum key value supported */
>>> u32 initial_allocation_mask; /* Bits set for the initially allocated keys */
>>> /*
>>> * Keys marked in the reservation list cannot be allocated by userspace
>>> @@ -84,7 +84,7 @@ static int scan_pkey_feature(void)
>>>
>>> static int pkey_initialize(void)
>>> {
>>> - int os_reserved, i;
>>> + int pkeys_total, i;
>>>
>>> /*
>>> * We define PKEY_DISABLE_EXECUTE in addition to the arch-neutral
>>> @@ -122,12 +122,12 @@ static int pkey_initialize(void)
>>> * The OS can manage only 8 pkeys due to its inability to represent them
>>> * in the Linux 4K PTE. Mark all other keys reserved.
>>> */
>>> - os_reserved = pkeys_total - 8;
>>> + max_pkey = min(8, pkeys_total);
>>
>> Shouldn't it be 7 ?
>>
>>> #else
>>> - os_reserved = 0;
>>> + max_pkey = pkeys_total;
>>> #endif
>>>
>>> - if (unlikely((pkeys_total - os_reserved) <= execute_only_key)) {
>>> + if (unlikely(max_pkey <= execute_only_key)) {
>>
>> Isn't that an off-by-one now?
>>
>> This is one-off boot time code, there's no need to clutter it with
>> unlikely.
>>
>>> /*
>>> * Insufficient number of keys to support
>>> * execute only key. Mark it unavailable.
>>> @@ -174,10 +174,10 @@ static int pkey_initialize(void)
>>> default_uamor &= ~(0x3ul << pkeyshift(1));
>>>
>>> /*
>>> - * Prevent the usage of OS reserved the keys. Update UAMOR
>>> + * Prevent the usage of OS reserved keys. Update UAMOR
>>> * for those keys.
>>> */
>>> - for (i = (pkeys_total - os_reserved); i < pkeys_total; i++) {
>>> + for (i = max_pkey; i < pkeys_total; i++) {
>>
>> Another off-by-one? Shouldn't we start from max_pkey + 1 ?
>>
>>> reserved_allocation_mask |= (0x1 << i);
>>> default_uamor &= ~(0x3ul << pkeyshift(i));
>>> }
>>
>
> It is the commit message. It is max pkey such that userspace can
> allocate 0 - maxp_pkey -1.
>
> -aneesh
^ permalink raw reply
* Re: [PATCH v5 19/26] powerpc/book3s64/kuap: Move KUAP related function outside radix
From: Michael Ellerman @ 2020-07-07 1:22 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev; +Cc: linuxram, bauerman
In-Reply-To: <d09d0150-860a-1e6a-1d4a-01ae8d7e97b9@linux.ibm.com>
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> On 7/6/20 6:11 PM, Michael Ellerman wrote:
>> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>>> The next set of patches adds support for kuap with hash translation.
>>
>> That's no longer true of this series.
>>
>>> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
>>> index 0d72c0246052..e93b65a0e6e7 100644
>>> --- a/arch/powerpc/mm/book3s64/pkeys.c
>>> +++ b/arch/powerpc/mm/book3s64/pkeys.c
>>> @@ -199,6 +200,24 @@ void __init pkey_early_init_devtree(void)
>>> return;
>>> }
>>>
>>> +#ifdef CONFIG_PPC_KUAP
>>> +void __init setup_kuap(bool disabled)
>>> +{
>>> + if (disabled || !early_radix_enabled())
>>> + return;
>>> +
>>> + if (smp_processor_id() == boot_cpuid) {
>>> + pr_info("Activating Kernel Userspace Access Prevention\n");
>>> + cur_cpu_spec->mmu_features |= MMU_FTR_RADIX_KUAP;
>>> + }
>>> +
>>> + /* Make sure userspace can't change the AMR */
>>> + mtspr(SPRN_UAMOR, 0);
>>> + mtspr(SPRN_AMR, AMR_KUAP_BLOCKED);
>>> + isync();
>>> +}
>>> +#endif
>>
>> This makes this code depend on CONFIG_PPC_MEM_KEYS=y, which it didn't
>> used to.
>>
>> That risks breaking people's existing .configs, if they have
>> PPC_MEM_KEYS=n they will now lose KUAP.
>>
>> And I'm not convinced the two features should be tied together, at least
>> at the user-visible Kconfig level.
>
> That simplifies the addition of hash kuap a lot. Especially in the
> exception entry and return paths. I did try to consider them as
> independent options. But then the feature fixup in asm code gets
> unnecessarily complicated. Also the UAMOR handling also get complicated.
Yep. I'm OK if most of the code is enabled for either/both options, but
I think the user-visible options should not depend on each other.
So something like:
config PPC_PKEY
def_bool y
depends on PPC_MEM_KEYS || PPC_KUAP
And then the low-level code is guarded by PPC_PKEY.
Or we just say that making MEM_KEYS configurable is not worth the
added complexity and turn it on always.
cheers
^ permalink raw reply
* Re: [PATCH v5 18/26] powerpc/book3s64/keys/kuap: Reset AMR/IAMR values on kexec
From: Michael Ellerman @ 2020-07-07 1:07 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev; +Cc: linuxram, bauerman
In-Reply-To: <2739323b-51c0-c713-0986-aa0bdaab3184@linux.ibm.com>
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> On 7/6/20 5:59 PM, Michael Ellerman wrote:
>> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>>> As we kexec across kernels that use AMR/IAMR for different purposes
>>> we need to ensure that new kernels get kexec'd with a reset value
>>> of AMR/IAMR. For ex: the new kernel can use key 0 for kernel mapping and the old
>>> AMR value prevents access to key 0.
>>>
>>> This patch also removes reset if IAMR and AMOR in kexec_sequence. Reset of AMOR
>>> is not needed and the IAMR reset is partial (it doesn't do the reset
>>> on secondary cpus) and is redundant with this patch.
>>
>> I like the idea of cleaning this stuff up.
>>
>> But I think tying it into kup/kuep/kuap and the MMU features and ifdefs
>> and so on is overly complicated.
>>
>> We should just have eg:
>>
>> void reset_sprs(void)
>> {
>> if (early_cpu_has_feature(CPU_FTR_ARCH_206)) {
>> mtspr(SPRN_AMR, 0);
>> mtspr(SPRN_UAMOR, 0);
>> }
>>
>> if (early_cpu_has_feature(CPU_FTR_ARCH_207S)) {
>> mtspr(SPRN_IAMR, 0);
>> }
>> }
>>
>> And call that from the kexec paths.
>
> Will fix. Should that be early_cpu_has_feature()? cpu_has_feature()
> should work there right?
Yeah I guess. I was thinking if we crashed before code patching was
done, but if that happens we can't kdump anyway. So I'm probably being
over paranoid.
cheers
^ permalink raw reply
* Re: [PATCH v5 13/26] powerpc/book3s64/pkeys: Enable MMU_FTR_PKEY
From: Michael Ellerman @ 2020-07-07 1:02 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev; +Cc: linuxram, bauerman
In-Reply-To: <6fa2a91d-c11c-2be4-2057-15f86d4dd39c@linux.ibm.com>
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>>>
>>>> /*
>>>> * Let's assume 32 pkeys on P8 bare metal, if its not
>>>> defined by device
>>>> * tree. We make this exception since skiboot forgot to
>>>> expose this
>>>> * property on power8.
>>>> */
>>>> if (!firmware_has_feature(FW_FEATURE_LPAR) &&
>>>> - cpu_has_feature(CPU_FTRS_POWER8))
>>>> + early_cpu_has_feature(CPU_FTRS_POWER8))
>>>> pkeys_total = 32;
>>>
>>> That's not how cpu_has_feature() works, we'll need to fix that.
>>>
>>> cheers
>>>
>>
>> I did a separate patch to handle that which switch the above to
>>
>> /*
>> * Let's assume 32 pkeys on P8/P9 bare metal, if its not
>> defined by device
>> * tree. We make this exception since skiboot forgot to expose
>> this
>> * property on power8/9.
>> */
>> if (!firmware_has_feature(FW_FEATURE_LPAR) &&
>> (early_cpu_has_feature(CPU_FTR_ARCH_207S) ||
>> early_cpu_has_feature(CPU_FTR_ARCH_300)))
>> pkeys_total = 32;
>>
>
> We should do a PVR check here i guess.
Yes, the ARCH features don't work because P10 will have both of those
enabled.
> ret = of_scan_flat_dt(dt_scan_storage_keys, &pkeys_total);
> if (ret == 0) {
>
> /*
> * Let's assume 32 pkeys on P8/P9 bare metal, if its not defined by device
> * tree. We make this exception since skiboot forgot to expose this
> * property on power8/9.
Well, it does expose it on Power9 after v6.6, but most P9 systems have
an older firmware than that.
And also the kernel has been enabling that on Power9 because of the
CPU_FTRS_POWER8 bug, so this is not actually a behaviour change.
> */
> if (!firmware_has_feature(FW_FEATURE_LPAR) &&
> (pvr_version_is(PVR_POWER8) || pvr_version_is(PVR_POWER9)))
> pkeys_total = 32;
> }
You need PVR_POWER8E and PVR_POWER8NVL as well.
cheers
^ permalink raw reply
* [PATCH 1/1] KVM/PPC: Fix typo on H_DISABLE_AND_GET hcall
From: Leonardo Bras @ 2020-07-07 0:48 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Bharata B Rao, Leonardo Bras, Vaibhav Jain, Sukadev Bhattiprolu
Cc: linuxppc-dev, linux-kernel, kvm-ppc
On PAPR+ the hcall() on 0x1B0 is called H_DISABLE_AND_GET, but got
defined as H_DISABLE_AND_GETC instead.
This define was introduced with a typo in commit <b13a96cfb055>
("[PATCH] powerpc: Extends HCALL interface for InfiniBand usage"), and was
later used without having the typo noticed.
Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
---
arch/powerpc/include/asm/hvcall.h | 2 +-
arch/powerpc/kvm/trace_hv.h | 2 +-
tools/perf/arch/powerpc/util/book3s_hcalls.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index e90c073e437e..d8ada9c7ec78 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -237,7 +237,7 @@
#define H_CREATE_RPT 0x1A4
#define H_REMOVE_RPT 0x1A8
#define H_REGISTER_RPAGES 0x1AC
-#define H_DISABLE_AND_GETC 0x1B0
+#define H_DISABLE_AND_GET 0x1B0
#define H_ERROR_DATA 0x1B4
#define H_GET_HCA_INFO 0x1B8
#define H_GET_PERF_COUNT 0x1BC
diff --git a/arch/powerpc/kvm/trace_hv.h b/arch/powerpc/kvm/trace_hv.h
index 4a61a971c34e..830a126e095d 100644
--- a/arch/powerpc/kvm/trace_hv.h
+++ b/arch/powerpc/kvm/trace_hv.h
@@ -89,7 +89,7 @@
{H_CREATE_RPT, "H_CREATE_RPT"}, \
{H_REMOVE_RPT, "H_REMOVE_RPT"}, \
{H_REGISTER_RPAGES, "H_REGISTER_RPAGES"}, \
- {H_DISABLE_AND_GETC, "H_DISABLE_AND_GETC"}, \
+ {H_DISABLE_AND_GET, "H_DISABLE_AND_GET"}, \
{H_ERROR_DATA, "H_ERROR_DATA"}, \
{H_GET_HCA_INFO, "H_GET_HCA_INFO"}, \
{H_GET_PERF_COUNT, "H_GET_PERF_COUNT"}, \
diff --git a/tools/perf/arch/powerpc/util/book3s_hcalls.h b/tools/perf/arch/powerpc/util/book3s_hcalls.h
index 54cfa0530e86..488f4339b83c 100644
--- a/tools/perf/arch/powerpc/util/book3s_hcalls.h
+++ b/tools/perf/arch/powerpc/util/book3s_hcalls.h
@@ -84,7 +84,7 @@
{0x1a4, "H_CREATE_RPT"}, \
{0x1a8, "H_REMOVE_RPT"}, \
{0x1ac, "H_REGISTER_RPAGES"}, \
- {0x1b0, "H_DISABLE_AND_GETC"}, \
+ {0x1b0, "H_DISABLE_AND_GET"}, \
{0x1b4, "H_ERROR_DATA"}, \
{0x1b8, "H_GET_HCA_INFO"}, \
{0x1bc, "H_GET_PERF_COUNT"}, \
--
2.25.4
^ permalink raw reply related
* Re: [PATCH] powerpc/numa: Restrict possible nodes based on platform
From: Nathan Lynch @ 2020-07-07 0:44 UTC (permalink / raw)
To: Tyrel Datwyler; +Cc: linuxppc-dev, Srikar Dronamraju, Bharata B Rao
In-Reply-To: <00388e11-9025-e273-137d-c23f8795457a@linux.ibm.com>
Tyrel Datwyler <tyreld@linux.ibm.com> writes:
>> --- a/arch/powerpc/mm/numa.c
>> +++ b/arch/powerpc/mm/numa.c
>> @@ -897,7 +897,7 @@ static void __init find_possible_nodes(void)
>> return;
>>
>> if (of_property_read_u32_index(rtas,
>> - "ibm,max-associativity-domains",
>> + "ibm,current-associativity-domains",
>
> I'm not sure ibm,current-associativity-domains is guaranteed to exist on older
> firmware. You may need check that it exists and fall back to
> ibm,max-associativity-domains in the event it doesn't
Yes. Looks like it's a PowerVM-specific property.
^ permalink raw reply
* Re: [PATCH v2 00/12] ppc64: enable kdump support for kexec_file_load syscall
From: piliu @ 2020-07-07 0:43 UTC (permalink / raw)
To: Hari Bathini, Michael Ellerman, Andrew Morton
Cc: Kexec-ml, Petr Tesarik, Mahesh J Salgaonkar, Sourabh Jain, lkml,
linuxppc-dev, Mimi Zohar, Vivek Goyal, Dave Young,
Thiago Jung Bauermann, Eric Biederman
In-Reply-To: <159371956443.21555.18251597651350106920.stgit@hbathini.in.ibm.com>
On 07/03/2020 03:53 AM, Hari Bathini wrote:
> This patch series enables kdump support for kexec_file_load system
> call (kexec -s -p) on PPC64. The changes are inspired from kexec-tools
> code but heavily modified for kernel consumption. There is scope to
> expand purgatory to verify sha256 digest along with other improvements
> in purgatory code. Will deal with those changes in a separate patch
> series later.
>
> The first patch adds a weak arch_kexec_locate_mem_hole() function to
> override locate memory hole logic suiting arch needs. There are some
> special regions in ppc64 which should be avoided while loading buffer
> & there are multiple callers to kexec_add_buffer making it complicated
> to maintain range sanity and using generic lookup at the same time.
>
> The second patch marks ppc64 specific code within arch/powerpc/kexec
> and arch/powerpc/purgatory to make the subsequent code changes easy
> to understand.
>
> The next patch adds helper function to setup different memory ranges
> needed for loading kdump kernel, booting into it and exporting the
> crashing kernel's elfcore.
>
> The fourth patch overrides arch_kexec_locate_mem_hole() function to
> locate memory hole for kdump segments by accounting for the special
> memory regions, referred to as excluded memory ranges, and sets
> kbuf->mem when a suitable memory region is found.
>
> The fifth patch moves walk_drmem_lmbs() out of .init section with
> a few changes to reuse it for setting up kdump kernel's usable memory
> ranges. The next patch uses walk_drmem_lmbs() to look up the LMBs
> and set linux,drconf-usable-memory & linux,usable-memory properties
> in order to restrict kdump kernel's memory usage.
>
> The seventh patch adds relocation support for the purgatory. Patch 8
> helps setup the stack for the purgatory. The next patch setups up
> backup region as a segment while loading kdump kernel and teaches
> purgatory to copy it from source to destination.
>
> Patch 10 builds the elfcore header for the running kernel & passes
> the info to kdump kernel via "elfcorehdr=" parameter to export as
> /proc/vmcore file. The next patch sets up the memory reserve map
> for the kexec kernel and also claims kdump support for kdump as
> all the necessary changes are added.
>
> The last patch fixes a lookup issue for `kexec -l -s` case when
> memory is reserved for crashkernel.
>
> Tested the changes successfully on P8, P9 lpars, couple of OpenPOWER
> boxes and a simulator.
>
> Changes in v2:
> * Introduced arch_kexec_locate_mem_hole() for override and dropped
> weak arch_kexec_add_buffer().
> * Addressed warnings reported by lkp.
> * Added patch to address kexec load issue when memory is reserved
> for crashkernel.
> * Used the appropriate license header for the new files added.
> * Added an option to merge ranges to minimize reallocations while
> adding memory ranges.
> * Dropped within_crashkernel parameter for add_opal_mem_range() &
> add_rtas_mem_range() functions as it is not really needed.
>
> ---
>
> Hari Bathini (12):
> kexec_file: allow archs to handle special regions while locating memory hole
> powerpc/kexec_file: mark PPC64 specific code
> powerpc/kexec_file: add helper functions for getting memory ranges
> ppc64/kexec_file: avoid stomping memory used by special regions
> powerpc/drmem: make lmb walk a bit more flexible
> ppc64/kexec_file: restrict memory usage of kdump kernel
> ppc64/kexec_file: add support to relocate purgatory
> ppc64/kexec_file: setup the stack for purgatory
> ppc64/kexec_file: setup backup region for kdump kernel
> ppc64/kexec_file: prepare elfcore header for crashing kernel
> ppc64/kexec_file: add appropriate regions for memory reserve map
> ppc64/kexec_file: fix kexec load failure with lack of memory hole
>
>
> arch/powerpc/include/asm/crashdump-ppc64.h | 15
> arch/powerpc/include/asm/drmem.h | 9
> arch/powerpc/include/asm/kexec.h | 35 +
> arch/powerpc/include/asm/kexec_ranges.h | 18
> arch/powerpc/include/asm/purgatory.h | 11
> arch/powerpc/kernel/prom.c | 13
> arch/powerpc/kexec/Makefile | 2
> arch/powerpc/kexec/elf_64.c | 35 +
> arch/powerpc/kexec/file_load.c | 78 +
> arch/powerpc/kexec/file_load_64.c | 1509 ++++++++++++++++++++++++++++
> arch/powerpc/kexec/ranges.c | 397 +++++++
> arch/powerpc/mm/drmem.c | 87 +-
> arch/powerpc/mm/numa.c | 13
> arch/powerpc/purgatory/Makefile | 28 -
> arch/powerpc/purgatory/purgatory_64.c | 36 +
> arch/powerpc/purgatory/trampoline.S | 117 --
> arch/powerpc/purgatory/trampoline_64.S | 175 +++
> include/linux/kexec.h | 29 -
> kernel/kexec_file.c | 16
> 19 files changed, 2413 insertions(+), 210 deletions(-)
> create mode 100644 arch/powerpc/include/asm/crashdump-ppc64.h
> create mode 100644 arch/powerpc/include/asm/kexec_ranges.h
> create mode 100644 arch/powerpc/include/asm/purgatory.h
> create mode 100644 arch/powerpc/kexec/file_load_64.c
> create mode 100644 arch/powerpc/kexec/ranges.c
> create mode 100644 arch/powerpc/purgatory/purgatory_64.c
> delete mode 100644 arch/powerpc/purgatory/trampoline.S
> create mode 100644 arch/powerpc/purgatory/trampoline_64.S
>
Tested-by: Pingfan Liu <piliu@redhat.com>
^ permalink raw reply
* Re: [PATCH 1/4] dma-mapping: move the remaining DMA API calls out of line
From: Alexey Kardashevskiy @ 2020-07-07 0:43 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Greg Kroah-Hartman, Joerg Roedel, linuxppc-dev, linux-kernel,
iommu, Robin Murphy, Lu Baolu
In-Reply-To: <95c46afb-bb40-3cb3-bf32-3e510fcd474d@ozlabs.ru>
On 03/06/2020 14:13, Alexey Kardashevskiy wrote:
>
>
> On 09/05/2020 18:19, Christoph Hellwig wrote:
>> On Tue, May 05, 2020 at 02:18:37PM +1000, Alexey Kardashevskiy wrote:
>>>
>>>
>>> On 17/04/2020 17:58, Christoph Hellwig wrote:
>>>> On Wed, Apr 15, 2020 at 09:21:37PM +1000, Alexey Kardashevskiy wrote:
>>>>> And the fact they were exported leaves possibility that there is a
>>>>> driver somewhere relying on these symbols or distro kernel won't build
>>>>> because the symbol disappeared from exports (I do not know what KABI
>>>>> guarantees or if mainline kernel cares).
>>>>
>>>> We absolutely do not care. In fact for abuses of APIs that drivers
>>>> should not use we almost care to make them private and break people
>>>> abusing them.
>>>
>>> ok :)
>>>
>>>>> I do not care in particular but
>>>>> some might, a line separated with empty lines in the commit log would do.
>>>>
>>>> I'll add a blurb for the next version.
>>>
>>>
>>> Has it gone anywhere? Thanks,
>>
>> I've been hoping for the sg_buf helpers to land first, as they need
>> backporting and would conflict. Do you urgently need the series?
>
> Any progress with sg_buf helpers stuff? Thanks,
Any luck there? I'd really like to cross this off my todo list :) Thanks,
--
Alexey
^ permalink raw reply
* Re: [PATCH] powerpc/spufs: add CONFIG_COREDUMP dependency
From: Jeremy Kerr @ 2020-07-07 0:09 UTC (permalink / raw)
To: Arnd Bergmann, Michael Ellerman
Cc: linuxppc-dev, Paul Mackerras, Christoph Hellwig, linux-kernel
In-Reply-To: <20200706132302.3885935-1-arnd@arndb.de>
Hi Arnd,
> The kernel test robot pointed out a slightly different error message
> after recent commit 5456ffdee666 ("powerpc/spufs: simplify spufs core
> dumping") to spufs for a configuration that never worked:
>
> powerpc64-linux-ld: arch/powerpc/platforms/cell/spufs/file.o: in
> function `.spufs_proxydma_info_dump':
> > > file.c:(.text+0x4c68): undefined reference to `.dump_emit'
> powerpc64-linux-ld: arch/powerpc/platforms/cell/spufs/file.o: in
> function `.spufs_dma_info_dump':
> file.c:(.text+0x4d70): undefined reference to `.dump_emit'
> powerpc64-linux-ld: arch/powerpc/platforms/cell/spufs/file.o: in
> function `.spufs_wbox_info_dump':
> file.c:(.text+0x4df4): undefined reference to `.dump_emit'
>
> Add a Kconfig dependency to prevent this from happening again.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Looks good to me, thanks.
Acked-by: Jeremy Kerr <jk@ozlabs.org>
Cheers,
Jeremy
^ permalink raw reply
* Re: [PATCH] powerpc/numa: Restrict possible nodes based on platform
From: Nathan Lynch @ 2020-07-06 23:19 UTC (permalink / raw)
To: Srikar Dronamraju; +Cc: linuxppc-dev, Bharata B Rao
In-Reply-To: <20200706064002.14848-1-srikar@linux.vnet.ibm.com>
Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> index 9fcf2d195830..3d55cef1a2dc 100644
> --- a/arch/powerpc/mm/numa.c
> +++ b/arch/powerpc/mm/numa.c
> @@ -897,7 +897,7 @@ static void __init find_possible_nodes(void)
> return;
>
> if (of_property_read_u32_index(rtas,
> - "ibm,max-associativity-domains",
> + "ibm,current-associativity-domains",
> min_common_depth, &numnodes))
Looks good if ibm,current-associativity-domains[min_common_depth]
actually denotes the range of possible values, i.e. a value of 2 implies
node numbers 0 and 1. PAPR+ says it's the "number of unique values",
which isn't how I would specify the property if it's supposed to express
a range. But it's probably OK...
^ permalink raw reply
* Re: [PATCH] powerpc/numa: Restrict possible nodes based on platform
From: Tyrel Datwyler @ 2020-07-06 20:58 UTC (permalink / raw)
To: Srikar Dronamraju, Michael Ellerman
Cc: Nathan Lynch, linuxppc-dev, Bharata B Rao
In-Reply-To: <20200706064002.14848-1-srikar@linux.vnet.ibm.com>
On 7/5/20 11:40 PM, Srikar Dronamraju wrote:
> As per PAPR, there are 2 device tree property
> ibm,max-associativity-domains (which defines the maximum number of
> domains that the firmware i.e PowerVM can support) and
> ibm,current-associativity-domains (which defines the maximum number of
> domains that the platform can support). Value of
> ibm,max-associativity-domains property is always greater than or equal
> to ibm,current-associativity-domains property.
>
> Powerpc currently uses ibm,max-associativity-domains property while
> setting the possible number of nodes. This is currently set at 32.
> However the possible number of nodes for a platform may be significantly
> less. Hence set the possible number of nodes based on
> ibm,current-associativity-domains property.
>
> $ lsprop /proc/device-tree/rtas/ibm,*associ*-domains
> /proc/device-tree/rtas/ibm,current-associativity-domains
> 00000005 00000001 00000002 00000002 00000002 00000010
> /proc/device-tree/rtas/ibm,max-associativity-domains
> 00000005 00000001 00000008 00000020 00000020 00000100
>
> $ cat /sys/devices/system/node/possible ##Before patch
> 0-31
>
> $ cat /sys/devices/system/node/possible ##After patch
> 0-1
>
> Note the maximum nodes this platform can support is only 2 but the
> possible nodes is set to 32.
>
> This is important because lot of kernel and user space code allocate
> structures for all possible nodes leading to a lot of memory that is
> allocated but not used.
>
> I ran a simple experiment to create and destroy 100 memory cgroups on
> boot on a 8 node machine (Power8 Alpine).
>
> Before patch
> free -k at boot
> total used free shared buff/cache available
> Mem: 523498176 4106816 518820608 22272 570752 516606720
> Swap: 4194240 0 4194240
>
> free -k after creating 100 memory cgroups
> total used free shared buff/cache available
> Mem: 523498176 4628416 518246464 22336 623296 516058688
> Swap: 4194240 0 4194240
>
> free -k after destroying 100 memory cgroups
> total used free shared buff/cache available
> Mem: 523498176 4697408 518173760 22400 627008 515987904
> Swap: 4194240 0 4194240
>
> After patch
> free -k at boot
> total used free shared buff/cache available
> Mem: 523498176 3969472 518933888 22272 594816 516731776
> Swap: 4194240 0 4194240
>
> free -k after creating 100 memory cgroups
> total used free shared buff/cache available
> Mem: 523498176 4181888 518676096 22208 640192 516496448
> Swap: 4194240 0 4194240
>
> free -k after destroying 100 memory cgroups
> total used free shared buff/cache available
> Mem: 523498176 4232320 518619904 22272 645952 516443264
> Swap: 4194240 0 4194240
>
> Observations:
> Fixed kernel takes 137344 kb (4106816-3969472) less to boot.
> Fixed kernel takes 309184 kb (4628416-4181888-137344) less to create 100 memcgs.
>
> Cc: Nathan Lynch <nathanl@linux.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: Anton Blanchard <anton@ozlabs.org>
> Cc: Bharata B Rao <bharata@linux.ibm.com>
> Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> arch/powerpc/mm/numa.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> index 9fcf2d195830..3d55cef1a2dc 100644
> --- a/arch/powerpc/mm/numa.c
> +++ b/arch/powerpc/mm/numa.c
> @@ -897,7 +897,7 @@ static void __init find_possible_nodes(void)
> return;
>
> if (of_property_read_u32_index(rtas,
> - "ibm,max-associativity-domains",
> + "ibm,current-associativity-domains",
I'm not sure ibm,current-associativity-domains is guaranteed to exist on older
firmware. You may need check that it exists and fall back to
ibm,max-associativity-domains in the event it doesn't
-Tyrel
> min_common_depth, &numnodes))
> goto out;
>
^ permalink raw reply
* Re: [PATCH v3 0/6] powerpc: queued spinlocks and rwlocks
From: Waiman Long @ 2020-07-06 18:39 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
Cc: linux-arch, Peter Zijlstra, Boqun Feng, linux-kernel, kvm-ppc,
virtualization, Ingo Molnar, Will Deacon
In-Reply-To: <20200706043540.1563616-1-npiggin@gmail.com>
On 7/6/20 12:35 AM, Nicholas Piggin wrote:
> v3 is updated to use __pv_queued_spin_unlock, noticed by Waiman (thank you).
>
> Thanks,
> Nick
>
> Nicholas Piggin (6):
> powerpc/powernv: must include hvcall.h to get PAPR defines
> powerpc/pseries: move some PAPR paravirt functions to their own file
> powerpc: move spinlock implementation to simple_spinlock
> powerpc/64s: implement queued spinlocks and rwlocks
> powerpc/pseries: implement paravirt qspinlocks for SPLPAR
> powerpc/qspinlock: optimised atomic_try_cmpxchg_lock that adds the
> lock hint
>
> arch/powerpc/Kconfig | 13 +
> arch/powerpc/include/asm/Kbuild | 2 +
> arch/powerpc/include/asm/atomic.h | 28 ++
> arch/powerpc/include/asm/paravirt.h | 89 +++++
> arch/powerpc/include/asm/qspinlock.h | 91 ++++++
> arch/powerpc/include/asm/qspinlock_paravirt.h | 7 +
> arch/powerpc/include/asm/simple_spinlock.h | 292 +++++++++++++++++
> .../include/asm/simple_spinlock_types.h | 21 ++
> arch/powerpc/include/asm/spinlock.h | 308 +-----------------
> arch/powerpc/include/asm/spinlock_types.h | 17 +-
> arch/powerpc/lib/Makefile | 3 +
> arch/powerpc/lib/locks.c | 12 +-
> arch/powerpc/platforms/powernv/pci-ioda-tce.c | 1 +
> arch/powerpc/platforms/pseries/Kconfig | 5 +
> arch/powerpc/platforms/pseries/setup.c | 6 +-
> include/asm-generic/qspinlock.h | 4 +
> 16 files changed, 577 insertions(+), 322 deletions(-)
> create mode 100644 arch/powerpc/include/asm/paravirt.h
> create mode 100644 arch/powerpc/include/asm/qspinlock.h
> create mode 100644 arch/powerpc/include/asm/qspinlock_paravirt.h
> create mode 100644 arch/powerpc/include/asm/simple_spinlock.h
> create mode 100644 arch/powerpc/include/asm/simple_spinlock_types.h
>
This patch looks OK to me.
I had run some microbenchmark on powerpc system with or w/o the patch.
On a 2-socket 160-thread SMT4 POWER9 system (not virtualized):
5.8.0-rc4
=========
Running locktest with spinlock [runtime = 10s, load = 1]
Threads = 160, Min/Mean/Max = 77,665/90,153/106,895
Threads = 160, Total Rate = 1,441,759 op/s; Percpu Rate = 9,011 op/s
Running locktest with rwlock [runtime = 10s, r% = 50%, load = 1]
Threads = 160, Min/Mean/Max = 47,879/53,807/63,689
Threads = 160, Total Rate = 860,192 op/s; Percpu Rate = 5,376 op/s
Running locktest with spinlock [runtime = 10s, load = 1]
Threads = 80, Min/Mean/Max = 242,907/319,514/463,161
Threads = 80, Total Rate = 2,555 kop/s; Percpu Rate = 32 kop/s
Running locktest with rwlock [runtime = 10s, r% = 50%, load = 1]
Threads = 80, Min/Mean/Max = 146,161/187,474/259,270
Threads = 80, Total Rate = 1,498 kop/s; Percpu Rate = 19 kop/s
Running locktest with spinlock [runtime = 10s, load = 1]
Threads = 40, Min/Mean/Max = 646,639/1,000,817/1,455,205
Threads = 40, Total Rate = 4,001 kop/s; Percpu Rate = 100 kop/s
Running locktest with rwlock [runtime = 10s, r% = 50%, load = 1]
Threads = 40, Min/Mean/Max = 402,165/597,132/814,555
Threads = 40, Total Rate = 2,388 kop/s; Percpu Rate = 60 kop/s
5.8.0-rc4-qlock+
================
Running locktest with spinlock [runtime = 10s, load = 1]
Threads = 160, Min/Mean/Max = 123,835/124,580/124,587
Threads = 160, Total Rate = 1,992 kop/s; Percpu Rate = 12 kop/s
Running locktest with rwlock [runtime = 10s, r% = 50%, load = 1]
Threads = 160, Min/Mean/Max = 254,210/264,714/276,784
Threads = 160, Total Rate = 4,231 kop/s; Percpu Rate = 26 kop/s
Running locktest with spinlock [runtime = 10s, load = 1]
Threads = 80, Min/Mean/Max = 599,715/603,397/603,450
Threads = 80, Total Rate = 4,825 kop/s; Percpu Rate = 60 kop/s
Running locktest with rwlock [runtime = 10s, r% = 50%, load = 1]
Threads = 80, Min/Mean/Max = 492,687/525,224/567,456
Threads = 80, Total Rate = 4,199 kop/s; Percpu Rate = 52 kop/s
Running locktest with spinlock [runtime = 10s, load = 1]
Threads = 40, Min/Mean/Max = 1,325,623/1,325,628/1,325,636
Threads = 40, Total Rate = 5,299 kop/s; Percpu Rate = 132 kop/s
Running locktest with rwlock [runtime = 10s, r% = 50%, load = 1]
Threads = 40, Min/Mean/Max = 1,249,731/1,292,977/1,342,815
Threads = 40, Total Rate = 5,168 kop/s; Percpu Rate = 129 kop/s
On systems on large number of cpus, qspinlock lock is faster and more fair.
With some tuning, we may be able to squeeze out more performance.
Cheers,
Longman
^ permalink raw reply
* Re: [PATCH] kbuild: introduce ccflags-remove-y and asflags-remove-y
From: Masahiro Yamada @ 2020-07-06 18:14 UTC (permalink / raw)
To: Anders Roxell
Cc: Michal Marek, Yoshinori Sato, Linux Kbuild mailing list,
Linux-sh list, Linux Kernel Mailing List, Steven Rostedt,
Russell King, linuxppc-dev, Ingo Molnar, Paul Mackerras,
Sami Tolvanen, Rich Felker, linux-arm-kernel
In-Reply-To: <CADYN=9JnzPC6Ja9s3_01k-CDTSuxKBMRdrqU5rqp0xw1r9XpRw@mail.gmail.com>
Hi Anders,
On Mon, Jul 6, 2020 at 8:24 PM Anders Roxell <anders.roxell@linaro.org> wrote:
>
> The full log can be found here [1].
>
> Without this patch for 'trace_selftest_dynamic' for instance, CC_FLAGS_FTRACE
> was removed from kernel/trace/*, and then added back to
> kernel/trace/trace_selftest_dynamic.
> While with this patch it looks like we add the flag (even though it is
> already there), and then
> removes the flag for all files in kernel/trace/* .
You are right.
I will drop this patch,
and send v2.
Thank you.
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH v5 13/26] powerpc/book3s64/pkeys: Enable MMU_FTR_PKEY
From: Aneesh Kumar K.V @ 2020-07-06 17:17 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: linuxram, bauerman
In-Reply-To: <cddb4987-860f-f4be-43b0-f164031f9f6a@linux.ibm.com>
>>
>>> /*
>>> * Let's assume 32 pkeys on P8 bare metal, if its not
>>> defined by device
>>> * tree. We make this exception since skiboot forgot to
>>> expose this
>>> * property on power8.
>>> */
>>> if (!firmware_has_feature(FW_FEATURE_LPAR) &&
>>> - cpu_has_feature(CPU_FTRS_POWER8))
>>> + early_cpu_has_feature(CPU_FTRS_POWER8))
>>> pkeys_total = 32;
>>
>> That's not how cpu_has_feature() works, we'll need to fix that.
>>
>> cheers
>>
>
> I did a separate patch to handle that which switch the above to
>
> /*
> * Let's assume 32 pkeys on P8/P9 bare metal, if its not
> defined by device
> * tree. We make this exception since skiboot forgot to expose
> this
> * property on power8/9.
> */
> if (!firmware_has_feature(FW_FEATURE_LPAR) &&
> (early_cpu_has_feature(CPU_FTR_ARCH_207S) ||
> early_cpu_has_feature(CPU_FTR_ARCH_300)))
> pkeys_total = 32;
>
We should do a PVR check here i guess.
ret = of_scan_flat_dt(dt_scan_storage_keys, &pkeys_total);
if (ret == 0) {
/*
* Let's assume 32 pkeys on P8/P9 bare metal, if its not defined by device
* tree. We make this exception since skiboot forgot to expose this
* property on power8/9.
*/
if (!firmware_has_feature(FW_FEATURE_LPAR) &&
(pvr_version_is(PVR_POWER8) || pvr_version_is(PVR_POWER9)))
pkeys_total = 32;
}
-aneesh
^ permalink raw reply
* Re: [PATCH v5 3/3] mm/page_alloc: Keep memoryless cpuless node 0 offline
From: Andi Kleen @ 2020-07-06 16:08 UTC (permalink / raw)
To: Michal Hocko
Cc: Gautham R Shenoy, Srikar Dronamraju, David Hildenbrand,
linuxppc-dev, linux-kernel, linux-mm, Satheesh Rajendran,
Mel Gorman, Kirill A. Shutemov, Andrew Morton,
Michal Suchánek, Linus Torvalds, Christopher Lameter,
Vlastimil Babka
In-Reply-To: <20200703092414.GR18446@dhcp22.suse.cz>
> > What's the point of this indirection other than another way of avoiding
> > empty node 0?
>
> Honestly, I do not have any idea. I've traced it down to
> Author: Andi Kleen <ak@suse.de>
> Date: Tue Jan 11 15:35:48 2005 -0800
I don't remember all the details, and I can't even find the commit
(is it in linux-historic?).
But AFAIK there's no guarantee PXMs are small and continuous, so it
seemed better to have a clean zero based space.
Back then we had a lot of problems with buggy SRAT tables in BIOS,
so we really tried to avoid trusting the BIOS as much as possible.
-Andi
^ permalink raw reply
* Re: [PATCH v5 19/26] powerpc/book3s64/kuap: Move KUAP related function outside radix
From: Aneesh Kumar K.V @ 2020-07-06 14:41 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: linuxram, bauerman
In-Reply-To: <87eepo6cjm.fsf@mpe.ellerman.id.au>
On 7/6/20 6:11 PM, Michael Ellerman wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>> The next set of patches adds support for kuap with hash translation.
>
> That's no longer true of this series.
>
>> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
>> index 0d72c0246052..e93b65a0e6e7 100644
>> --- a/arch/powerpc/mm/book3s64/pkeys.c
>> +++ b/arch/powerpc/mm/book3s64/pkeys.c
>> @@ -199,6 +200,24 @@ void __init pkey_early_init_devtree(void)
>> return;
>> }
>>
>> +#ifdef CONFIG_PPC_KUAP
>> +void __init setup_kuap(bool disabled)
>> +{
>> + if (disabled || !early_radix_enabled())
>> + return;
>> +
>> + if (smp_processor_id() == boot_cpuid) {
>> + pr_info("Activating Kernel Userspace Access Prevention\n");
>> + cur_cpu_spec->mmu_features |= MMU_FTR_RADIX_KUAP;
>> + }
>> +
>> + /* Make sure userspace can't change the AMR */
>> + mtspr(SPRN_UAMOR, 0);
>> + mtspr(SPRN_AMR, AMR_KUAP_BLOCKED);
>> + isync();
>> +}
>> +#endif
>
> This makes this code depend on CONFIG_PPC_MEM_KEYS=y, which it didn't
> used to.
>
> That risks breaking people's existing .configs, if they have
> PPC_MEM_KEYS=n they will now lose KUAP.
>
> And I'm not convinced the two features should be tied together, at least
> at the user-visible Kconfig level.
>
That simplifies the addition of hash kuap a lot. Especially in the
exception entry and return paths. I did try to consider them as
independent options. But then the feature fixup in asm code gets
unnecessarily complicated. Also the UAMOR handling also get complicated.
-aneesh
^ permalink raw reply
* Re: [PATCH v5 18/26] powerpc/book3s64/keys/kuap: Reset AMR/IAMR values on kexec
From: Aneesh Kumar K.V @ 2020-07-06 14:39 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: linuxram, bauerman
In-Reply-To: <87h7uk6d3m.fsf@mpe.ellerman.id.au>
On 7/6/20 5:59 PM, Michael Ellerman wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>> As we kexec across kernels that use AMR/IAMR for different purposes
>> we need to ensure that new kernels get kexec'd with a reset value
>> of AMR/IAMR. For ex: the new kernel can use key 0 for kernel mapping and the old
>> AMR value prevents access to key 0.
>>
>> This patch also removes reset if IAMR and AMOR in kexec_sequence. Reset of AMOR
>> is not needed and the IAMR reset is partial (it doesn't do the reset
>> on secondary cpus) and is redundant with this patch.
>
> I like the idea of cleaning this stuff up.
>
> But I think tying it into kup/kuep/kuap and the MMU features and ifdefs
> and so on is overly complicated.
>
> We should just have eg:
>
> void reset_sprs(void)
> {
> if (early_cpu_has_feature(CPU_FTR_ARCH_206)) {
> mtspr(SPRN_AMR, 0);
> mtspr(SPRN_UAMOR, 0);
> }
>
> if (early_cpu_has_feature(CPU_FTR_ARCH_207S)) {
> mtspr(SPRN_IAMR, 0);
> }
> }
>
> And call that from the kexec paths.
>
Will fix. Should that be early_cpu_has_feature()? cpu_has_feature()
should work there right?
-aneesh
^ permalink raw reply
* Re: [PATCH v5 13/26] powerpc/book3s64/pkeys: Enable MMU_FTR_PKEY
From: Aneesh Kumar K.V @ 2020-07-06 14:09 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: linuxram, bauerman
In-Reply-To: <878sfw6b7v.fsf@mpe.ellerman.id.au>
On 7/6/20 6:40 PM, Michael Ellerman wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>> Parse storage keys related device tree entry in early_init_devtree
>> and enable MMU feature MMU_FTR_PKEY if pkeys are supported.
>>
>> MMU feature is used instead of CPU feature because this enables us
>> to group MMU_FTR_KUAP and MMU_FTR_PKEY in asm feature fixup code.
>
> Subject should probably be "Add MMU_FTR_PKEY".
>
>> diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h
>> index f4ac25d4df05..72966d3d8f64 100644
>> --- a/arch/powerpc/include/asm/mmu.h
>> +++ b/arch/powerpc/include/asm/mmu.h
>> @@ -23,6 +23,7 @@
>>
>> /* Radix page table supported and enabled */
>> #define MMU_FTR_TYPE_RADIX ASM_CONST(0x00000040)
>> +#define MMU_FTR_PKEY ASM_CONST(0x00000080)
>
> It's not a type, so it should be with the individual feature bits below:
>
We don't have free bit in the other group. For now i will move this to
modified arch/powerpc/include/asm/mmu.h
@@ -23,12 +23,15 @@
/* Radix page table supported and enabled */
#define MMU_FTR_TYPE_RADIX ASM_CONST(0x00000040)
-#define MMU_FTR_PKEY ASM_CONST(0x00000080)
/*
* Individual features below.
*/
+/*
+ * Support for memory protection keys.
+ */
+#define MMU_FTR_PKEY ASM_CONST(0x00001000)
/*
* Support for 68 bit VA space. We added that from ISA 2.05
*/
>> /*
>> * Individual features below.
>
>> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
>> index 6a3bac357e24..6d70797352d8 100644
>> --- a/arch/powerpc/kernel/prom.c
>> +++ b/arch/powerpc/kernel/prom.c
>> @@ -815,6 +815,11 @@ void __init early_init_devtree(void *params)
>> /* Now try to figure out if we are running on LPAR and so on */
>> pseries_probe_fw_features();
>>
>> + /*
>> + * Initialize pkey features and default AMR/IAMR values
>> + */
>> + pkey_early_init_devtree();
>
> Not your fault, but the fact that we're having to do more and more
> initialisation this early, based on the flat device tree, makes me
> wonder if we need to rethink how we're doing the CPU/MMU feature setup.
>
>> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
>> index 0ff59acdbb84..bbba9c601e14 100644
>> --- a/arch/powerpc/mm/book3s64/pkeys.c
>> +++ b/arch/powerpc/mm/book3s64/pkeys.c
>> @@ -38,38 +39,45 @@ static int execute_only_key = 2;
>> #define PKEY_REG_BITS (sizeof(u64) * 8)
>> #define pkeyshift(pkey) (PKEY_REG_BITS - ((pkey+1) * AMR_BITS_PER_PKEY))
>>
>> +static int __init dt_scan_storage_keys(unsigned long node,
>> + const char *uname, int depth,
>> + void *data)
>> +{
>> + const char *type = of_get_flat_dt_prop(node, "device_type", NULL);
>> + const __be32 *prop;
>> + int pkeys_total;
>> +
>> + /* We are scanning "cpu" nodes only */
>> + if (type == NULL || strcmp(type, "cpu") != 0)
>> + return 0;
>> +
>> + prop = of_get_flat_dt_prop(node, "ibm,processor-storage-keys", NULL);
>> + if (!prop)
>> + return 0;
>> + pkeys_total = be32_to_cpu(prop[0]);
>> + return pkeys_total;
>
> That's not really the way the return value is meant to be used for these
> scanning functions.
>
> The usual return values are 0 to keep scanning and !0 means we've found
> the node we're looking for and we should stop scanning.
>
> Doing it this way means if you find 0 pkeys it will keep scanning.
>
> Instead you should pass &pkeys_total as the data pointer and set that.
>
>> +}
>> +
done
modified arch/powerpc/mm/book3s64/pkeys.c
@@ -52,7 +52,7 @@ static int __init dt_scan_storage_keys(unsigned long node,
{
const char *type = of_get_flat_dt_prop(node, "device_type", NULL);
const __be32 *prop;
- int pkeys_total;
+ int *pkeys_total = (int *) data;
/* We are scanning "cpu" nodes only */
if (type == NULL || strcmp(type, "cpu") != 0)
@@ -61,12 +61,13 @@ static int __init dt_scan_storage_keys(unsigned long
node,
prop = of_get_flat_dt_prop(node, "ibm,processor-storage-keys", NULL);
if (!prop)
return 0;
- pkeys_total = be32_to_cpu(prop[0]);
- return pkeys_total;
+ *pkeys_total = be32_to_cpu(prop[0]);
+ return 1;
}
static int scan_pkey_feature(void)
{
+ int ret;
int pkeys_total;
/*
@@ -75,8 +76,8 @@ static int scan_pkey_feature(void)
if (early_radix_enabled())
return 0;
- pkeys_total = of_scan_flat_dt(dt_scan_storage_keys, NULL);
- if (pkeys_total == 0) {
+ ret = of_scan_flat_dt(dt_scan_storage_keys, &pkeys_total);
+ if (ret == 0) {
/*
* Let's assume 32 pkeys on P8/P9 bare metal, if its not defined by
device
@@ -87,7 +88,6 @@ static int scan_pkey_feature(void)
(early_cpu_has_feature(CPU_FTR_ARCH_207S) ||
early_cpu_has_feature(CPU_FTR_ARCH_300)))
pkeys_total = 32;
-
}
/*
>> static int scan_pkey_feature(void)
>> {
>> - u32 vals[2];
>> - int pkeys_total = 0;
>> - struct device_node *cpu;
>> + int pkeys_total;
>>
>> /*
>> * Pkey is not supported with Radix translation.
>> */
>> - if (radix_enabled())
>> + if (early_radix_enabled())
>> return 0;
>>
>> - cpu = of_find_node_by_type(NULL, "cpu");
>> - if (!cpu)
>> - return 0;
>> + pkeys_total = of_scan_flat_dt(dt_scan_storage_keys, NULL);
>> + if (pkeys_total == 0) {
>>
>> - if (of_property_read_u32_array(cpu,
>> - "ibm,processor-storage-keys", vals, 2) == 0) {
>> - /*
>> - * Since any pkey can be used for data or execute, we will
>> - * just treat all keys as equal and track them as one entity.
>> - */
>> - pkeys_total = vals[0];
>> - /* Should we check for IAMR support FIXME!! */
>
> ???
The device tree allows us to have different count for both AMR and IAMR.
The current code skip that. I guess i added a comment in earlier patch
to check that whether we need to handle different AMR and IAMR counts.
The same comment get dropped here.
>
>> - } else {
>
> This loses the ability to differentiate between no storage-keys property
> at all vs a property that specifies zero keys, which I don't think is a
> good change.
>
>> /*
>> * Let's assume 32 pkeys on P8 bare metal, if its not defined by device
>> * tree. We make this exception since skiboot forgot to expose this
>> * property on power8.
>> */
>> if (!firmware_has_feature(FW_FEATURE_LPAR) &&
>> - cpu_has_feature(CPU_FTRS_POWER8))
>> + early_cpu_has_feature(CPU_FTRS_POWER8))
>> pkeys_total = 32;
>
> That's not how cpu_has_feature() works, we'll need to fix that.
>
> cheers
>
I did a separate patch to handle that which switch the above to
/*
* Let's assume 32 pkeys on P8/P9 bare metal, if its not defined by device
* tree. We make this exception since skiboot forgot to expose this
* property on power8/9.
*/
if (!firmware_has_feature(FW_FEATURE_LPAR) &&
(early_cpu_has_feature(CPU_FTR_ARCH_207S) ||
early_cpu_has_feature(CPU_FTR_ARCH_300)))
pkeys_total = 32;
^ 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