Re: [PATCH] powerpc/64: Drop ppc_inst_as_str()
On Thu, Jun 2, 2022 at 6:49 PM Segher Boessenkool wrote: > > On Thu, Jun 02, 2022 at 01:01:04PM +1000, Jordan Niethe wrote: > > > What about the more fundamental thing? Have the order of the two halves > > > of a prefixed insn as ulong not depend on endianness? It really is two > > > opcodes, and the prefixed one is first, always, even in LE. > > The reason would be the value of as ulong is then used to write a > > prefixed instruction to > > memory with std. > > If both endiannesses had the halves the same one of them would store > > the suffix in front of the prefix. > > You cannot do such a (possibly) unaligned access from C though, not > without invoking undefined behaviour. The compiler usually lets you get > away with it, but there are no guarantees. You can make sure you only > ever do such an access from assembler code of course. Would using inline assembly to do it be ok? > > Swapping the two halves of a register costs at most one insn. It is > harmful premature optimisation to make this single cycle advantage > override more important consideration (almost everything else :-) ) I'm not sure I follow. We are not doing this as an optimisation, but out of the necessity of writing the prefixed instruction to memory in a single instruction so that we don't end up with half an instruction in the kernel image. > > > Segher
Re: [PATCH 2/6] powerpc: Provide syscall wrapper
Thanks for your comment. I comment briefly here on how files like arch/powerpc/kernel/syscalls.c implement ppc specialisations of various syscalls, with implementations that themselves call syscall symbols. >> For platforms supporting this syscall wrapper, emit symbols with usual >> in-register parameters (`sys...`) to support calls to syscall handlers >> from within the kernel. The implementation of ppc_personality can be immediately reworked to call ksys_personality, but I can’t do the same for sys_old_select for example, which is required to implement ppc_select. As such we emit both symbols (sys_... and __powerpc...) so that ppc implementations can efficiently call other implementations without needing to allocate another pt_regs onto the stack. This does admittedly introduce some code bloat into the binary. I see a couple ways of fixing this: 1. Remove all calls to syscall implementations via sys_... symbols from within the kernel. 2. Restrict which syscall implementations receive a sys_... symbol to limit the number of duplicated functions. Do you have any suggestions on how I might fix this? > On 2 Jun 2022, at 12:33 am, Christophe Leroy > wrote: > > > > Le 01/06/2022 à 07:48, Rohan McLure a écrit : >> [Vous ne recevez pas souvent de courriers de la part de >> rmcl...@linux.ibm.com. Découvrez pourquoi cela peut être important à >> l'adresse https://aka.ms/LearnAboutSenderIdentification.] >> >> Syscall wrapper implemented as per s390, x86, arm64, providing the >> option for gprs to be cleared on entry to the kernel, reducing caller >> influence influence on speculation within syscall routine. The wrapper >> is a macro that emits syscall handler implementations with parameters >> passed by stack pointer. >> >> For platforms supporting this syscall wrapper, emit symbols with usual >> in-register parameters (`sys...`) to support calls to syscall handlers >> from within the kernel. >> >> Syscalls are wrapped on all platforms except Cell processor. SPUs require >> access syscall prototypes which are omitted with ARCH_HAS_SYSCALL_WRAPPER >> enabled. >> > > Also wondering why I get duplicated syscall functions. Shouldn't the > sys_ ones go away once we implement the __powerpc_sys_ ones ? > > c001e9a0 : > c001e9a0: 94 21 ff b0 stwur1,-80(r1) > c001e9a4: 7c 08 02 a6 mflrr0 > c001e9a8: 38 a0 00 40 li r5,64 > c001e9ac: 38 80 00 00 li r4,0 > c001e9b0: 38 61 00 08 addir3,r1,8 > c001e9b4: 90 01 00 54 stw r0,84(r1) > c001e9b8: 4b ff 6d 55 bl c001570c > c001e9bc: 38 61 00 08 addir3,r1,8 > c001e9c0: 39 20 00 11 li r9,17 > c001e9c4: 91 21 00 1c stw r9,28(r1) > c001e9c8: 4b ff fb 31 bl c001e4f8 > c001e9cc: 80 01 00 54 lwz r0,84(r1) > c001e9d0: 38 21 00 50 addir1,r1,80 > c001e9d4: 7c 08 03 a6 mtlrr0 > c001e9d8: 4e 80 00 20 blr > > c001e9dc <__powerpc_sys_fork>: > c001e9dc: 94 21 ff b0 stwur1,-80(r1) > c001e9e0: 7c 08 02 a6 mflrr0 > c001e9e4: 38 a0 00 40 li r5,64 > c001e9e8: 38 80 00 00 li r4,0 > c001e9ec: 38 61 00 08 addir3,r1,8 > c001e9f0: 90 01 00 54 stw r0,84(r1) > c001e9f4: 4b ff 6d 19 bl c001570c > c001e9f8: 38 61 00 08 addir3,r1,8 > c001e9fc: 39 20 00 11 li r9,17 > c001ea00: 91 21 00 1c stw r9,28(r1) > c001ea04: 4b ff fa f5 bl c001e4f8 > c001ea08: 80 01 00 54 lwz r0,84(r1) > c001ea0c: 38 21 00 50 addir1,r1,80 > c001ea10: 7c 08 03 a6 mtlrr0 > c001ea14: 4e 80 00 20 blr > > c001ea18 : > c001ea18: 94 21 ff b0 stwur1,-80(r1) > c001ea1c: 7c 08 02 a6 mflrr0 > c001ea20: 38 a0 00 38 li r5,56 > c001ea24: 38 80 00 00 li r4,0 > c001ea28: 38 61 00 10 addir3,r1,16 > c001ea2c: 90 01 00 54 stw r0,84(r1) > c001ea30: 4b ff 6c dd bl c001570c > c001ea34: 38 61 00 08 addir3,r1,8 > c001ea38: 39 40 00 00 li r10,0 > c001ea3c: 39 60 41 00 li r11,16640 > c001ea40: 39 20 00 11 li r9,17 > c001ea44: 91 41 00 08 stw r10,8(r1) > c001ea48: 91 61 00 0c stw r11,12(r1) > c001ea4c: 91 21 00 1c stw r9,28(r1) > c001ea50: 4b ff fa a9 bl c001e4f8 > c001ea54: 80 01 00 54 lwz r0,84(r1) > c001ea58: 38 21 00 50 addir1,r1,80 > c001ea5c: 7c 08 03 a6 mtlrr0 > c001ea60: 4e 80 00 20 blr > > c001ea64 <__powerpc_sys_vfork>: > c001ea64: 94 21 ff b0 stwur1,-80(r1) > c001ea68: 7c 08 02 a6 mflrr0 > c001ea6c: 38 a0 00 38 li r5,56 > c001ea70: 38 80 00 00 li r4,0 > c001ea74: 38 61 00 10 addir3,r1,16 > c001ea78: 90 01 00 54 stw r0,84(r1) > c001ea7c: 4b ff 6c 91 bl c0
[powerpc:fixes-test] BUILD SUCCESS 3e8635fb2e072672cbc650989ffedf8300ad67fb
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git fixes-test branch HEAD: 3e8635fb2e072672cbc650989ffedf8300ad67fb powerpc/kasan: Force thread size increase with KASAN elapsed time: 723m configs tested: 86 configs skipped: 109 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: arm64 defconfig arm64allyesconfig arm allmodconfig arm defconfig arm allyesconfig arc axs101_defconfig arm gemini_defconfig arm vf610m4_defconfig arm viper_defconfig nios2 defconfig m68k apollo_defconfig powerpc ep8248e_defconfig sh se7780_defconfig sh kfr2r09_defconfig armmini2440_defconfig sh se7343_defconfig x86_64randconfig-c001 i386 randconfig-c001 arm randconfig-c002-20220531 ia64defconfig m68k allyesconfig m68k allmodconfig m68kdefconfig arc allyesconfig cskydefconfig nios2allyesconfig alpha defconfig alphaallyesconfig h8300allyesconfig xtensa allyesconfig arc defconfig sh allmodconfig s390defconfig s390 allmodconfig parisc defconfig parisc64defconfig parisc allyesconfig s390 allyesconfig sparc defconfig i386 allyesconfig sparcallyesconfig i386defconfig i386 debian-10.3-kselftests i386 debian-10.3 mips allyesconfig mips allmodconfig powerpc allyesconfig powerpc allnoconfig powerpc allmodconfig x86_64randconfig-a006 x86_64randconfig-a004 x86_64randconfig-a002 i386 randconfig-a012 i386 randconfig-a014 i386 randconfig-a016 arc randconfig-r043-20220531 s390 randconfig-r044-20220531 riscvrandconfig-r042-20220531 riscv defconfig riscvnommu_virt_defconfig riscv rv32_defconfig riscvnommu_k210_defconfig riscv allnoconfig riscvallmodconfig riscvallyesconfig x86_64rhel-8.3-kselftests um x86_64_defconfig um i386_defconfig x86_64 rhel-8.3-func x86_64 rhel-8.3-syz x86_64 rhel-8.3-kunit clang tested configs: mips randconfig-c004-20220531 x86_64randconfig-c007 i386 randconfig-c001 s390 randconfig-c005-20220531 arm randconfig-c002-20220531 powerpc randconfig-c003-20220531 riscvrandconfig-c006-20220531 powerpc tqm5200_defconfig riscvnommu_virt_defconfig powerpc acadia_defconfig arm netwinder_defconfig arm lpc32xx_defconfig i386 randconfig-a002 i386 randconfig-a006 i386 randconfig-a004 x86_64randconfig-a012 x86_64randconfig-a014 x86_64randconfig-a016 -- 0-DAY CI Kernel Test Service https://01.org/lkp
Re: [PATCH v2 2/4] of: dynamic: add of_property_alloc() and of_property_free()
On 6/1/22 23:58, Clément Léger wrote:
> Le Wed, 1 Jun 2022 15:32:29 -0700,
> Tyrel Datwyler a écrit :
>
>>> /**
>>> - * __of_prop_dup - Copy a property dynamically.
>>> - * @prop: Property to copy
>>> + * of_property_free - Free a property allocated dynamically.
>>> + * @prop: Property to be freed
>>> + */
>>> +void of_property_free(const struct property *prop)
>>> +{
>>> + if (!of_property_check_flag(prop, OF_DYNAMIC))
>>> + return;
>>> +
>>
>> This looks wrong to me. From what I understand the value data is allocated as
>> trailing memory that is part of the property allocation itself. (ie. prop =
>> kzalloc(sizeof(*prop) + len, allocflags)). So, kfree(prop) should also take
>> care
>> of the trailing value data. Calling kfree(prop->value) is bogus since
>> prop->value wasn't dynamically allocated on its own.
>
> kfree(prop->value) is only called if the value is not the trailing data
> of the property so I don't see what is wrong there. In that case, only
> kfree(prop) is called.
Right, Rob clarified for me in the v1 patch.
>
>>
>> Also, this condition will always fail. You explicitly set prop->value = prop
>> + 1
>> in alloc.
>
> The user that did allocated the property might want to provide its own
> "value". In that case, prop->value would be overwritten by the user
> allocated value and thus the check would be true, hence calling
> kfree(prop->value).
So, that was the part I was missing. I think a comment would be helpful so its
clear value can be either trailing or user assigned.
-Tyrel
>
>>
>> Maybe I need to go back and look at v1 again.
>>
>> -Tyrel
>>
>>> + if (prop->value != prop + 1)
>>> + kfree(prop->value);
>>> +
>>> + kfree(prop->name);
>>> + kfree(prop);
>>> +}
>>> +EXPORT_SYMBOL(of_property_free);
>>> +
>
>
Re: [PATCH 1/3] of: dynamic: add of_property_alloc() and of_property_free()
On 6/2/22 07:06, Rob Herring wrote:
> On Wed, Jun 1, 2022 at 5:31 PM Tyrel Datwyler wrote:
>>
>> On 5/5/22 12:37, Rob Herring wrote:
>>> On Wed, May 04, 2022 at 05:40:31PM +0200, Clément Léger wrote:
Add function which allows to dynamically allocate and free properties.
Use this function internally for all code that used the same logic
(mainly __of_prop_dup()).
Signed-off-by: Clément Léger
---
drivers/of/dynamic.c | 101 ++-
include/linux/of.h | 16 +++
2 files changed, 88 insertions(+), 29 deletions(-)
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index cd3821a6444f..e8700e509d2e 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -313,9 +313,7 @@ static void property_list_free(struct property
*prop_list)
for (prop = prop_list; prop != NULL; prop = next) {
next = prop->next;
-kfree(prop->name);
-kfree(prop->value);
-kfree(prop);
+of_property_free(prop);
}
}
@@ -367,48 +365,95 @@ void of_node_release(struct kobject *kobj)
}
/**
- * __of_prop_dup - Copy a property dynamically.
- * @prop: Property to copy
+ * of_property_free - Free a property allocated dynamically.
+ * @prop: Property to be freed
+ */
+void of_property_free(const struct property *prop)
+{
+kfree(prop->value);
+kfree(prop->name);
+kfree(prop);
+}
+EXPORT_SYMBOL(of_property_free);
+
+/**
+ * of_property_alloc - Allocate a property dynamically.
+ * @name: Name of the new property
+ * @value: Value that will be copied into the new property value
+ * @value_len: length of @value to be copied into the new property
value
+ * @len:Length of new property value, must be greater than @value_len
>>>
>>> What's the usecase for the lengths being different? That doesn't seem
>>> like a common case, so perhaps handle it with a NULL value and
>>> non-zero length. Then the caller has to deal with populating
>>> prop->value.
>>>
* @allocflags: Allocation flags (typically pass GFP_KERNEL)
*
- * Copy a property by dynamically allocating the memory of both the
+ * Create a property by dynamically allocating the memory of both the
* property structure and the property name & contents. The property's
* flags have the OF_DYNAMIC bit set so that we can differentiate between
* dynamically allocated properties and not.
*
* Return: The newly allocated property or NULL on out of memory error.
*/
-struct property *__of_prop_dup(const struct property *prop, gfp_t
allocflags)
+struct property *of_property_alloc(const char *name, const void *value,
+ int value_len, int len, gfp_t allocflags)
{
-struct property *new;
+int alloc_len = len;
+struct property *prop;
+
+if (len < value_len)
+return NULL;
-new = kzalloc(sizeof(*new), allocflags);
-if (!new)
+prop = kzalloc(sizeof(*prop), allocflags);
+if (!prop)
return NULL;
+prop->name = kstrdup(name, allocflags);
+if (!prop->name)
+goto out_err;
+
/*
- * NOTE: There is no check for zero length value.
- * In case of a boolean property, this will allocate a value
- * of zero bytes. We do this to work around the use
- * of of_get_property() calls on boolean values.
+ * Even if the property has no value, it must be set to a
+ * non-null value since of_get_property() is used to check
+ * some values that might or not have a values (ranges for
+ * instance). Moreover, when the node is released, prop->value
+ * is kfreed so the memory must come from kmalloc.
>>>
>>> Allowing for NULL value didn't turn out well...
>>>
>>> We know that we can do the kfree because OF_DYNAMIC is set IIRC...
>>>
>>> If we do 1 allocation for prop and value, then we can test
>>> for "prop->value == prop + 1" to determine if we need to free or not.
>>
>> If its a single allocation do we even need a test? Doesn't kfree(prop) take
>> care
>> of the property and the trailing memory allocated for the value?
>
> Yes, it does when it's a single alloc, but it's testing for when
> prop->value is not a single allocation because we could have either.
>
Ok, that is the part I was missing. Thanks for the clarification.
-Tyrel
Re: [PATCH 0/2] Disabling NMI watchdog during LPM's memory transfer
Laurent Dufour writes: > When a partition is transferred, once it arrives at the destination node, > the partition is active but much of its memory must be transferred from the > start node. > > It depends on the activity in the partition, but the more CPU the partition > has, the more memory to be transferred is likely to be. This causes latency > when accessing pages that need to be transferred, and often, for large > partitions, it triggers the NMI watchdog. It also triggers warnings from other watchdogs and subsystems that have soft latency requirements - softlockup, RCU, workqueue. The issue is more general than the NMI watchdog. > The NMI watchdog causes the CPU stack to dump where it appears to be > stuck. In this case, it does not bring much information since it can happen > during any memory access of the kernel. When the site of a watchdog backtrace shows a thread stuck on a routine memory access as opposed to something like a lock acquisition, that is actually useful information that shouldn't be discarded. It tells us the platform is failing to adequately virtualize partition memory. This isn't a benign situation and it's likely to unacceptably affect real workloads. The kernel is ideally situated to detect and warn about this. > In addition, the NMI interrupt mechanism is not secure and can generate a > dump system in the event that the interruption is taken while > MSR[RI]=0. This sounds like a general problem with that facility that isn't specific to partition migration? Maybe it should be disabled altogether until that can be fixed? > Given how often hard lockups are detected when transferring large > partitions, it seems best to disable the watchdog NMI until the memory > transfer from the start node is complete. At this time, I'm far from convinced. Disabling the watchdog is going to make the underlying problems in the platform and/or network harder to understand.
[PATCH v2 3/4] powerpc/pseries: register pseries-wdt device with platform bus
PAPR v2.12 defines a new hypercall, H_WATCHDOG. The hypercall permits
guest control of one or more virtual watchdog timers.
These timers do not conform to PowerPC device conventions. They are
not affixed to any extant bus, nor do they have full representation in
the device tree.
As a workaround we represent them as platform devices.
This patch registers a single platform device, "pseries-wdt", with the
platform bus if the FW_FEATURE_WATCHDOG flag is set.
A driver for this device, "pseries-wdt", will be introduced in a
subsequent patch.
Signed-off-by: Scott Cheloha
---
arch/powerpc/platforms/pseries/setup.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/arch/powerpc/platforms/pseries/setup.c
b/arch/powerpc/platforms/pseries/setup.c
index afb074269b42..233c64f59815 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -14,6 +14,7 @@
#include
#include
+#include
#include
#include
#include
@@ -169,6 +170,22 @@ static void __init fwnmi_init(void)
#endif
}
+/*
+ * Affix a device for the first timer to the platform bus if
+ * we have firmware support for the H_WATCHDOG hypercall.
+ */
+static struct platform_device *pseries_wdt_pdev;
+
+static __init int pseries_wdt_init(void)
+{
+ if (!firmware_has_feature(FW_FEATURE_WATCHDOG))
+ return 0;
+ pseries_wdt_pdev = platform_device_register_simple("pseries-wdt",
+ 0, NULL, 0);
+ return 0;
+}
+machine_subsys_initcall(pseries, pseries_wdt_init);
+
static void pseries_8259_cascade(struct irq_desc *desc)
{
struct irq_chip *chip = irq_desc_get_chip(desc);
--
2.27.0
[PATCH v2 4/4] watchdog/pseries-wdt: initial support for H_WATCHDOG-based watchdog timers
PAPR v2.12 defines a new hypercall, H_WATCHDOG. The hypercall permits guest control of one or more virtual watchdog timers. The timers have millisecond granularity. The guest is terminated when a timer expires. This patch adds a watchdog driver for these timers, "pseries-wdt". pseries_wdt_probe() currently assumes the existence of only one platform device and always assigns it watchdogNumber 1. If we ever expose more than one timer to userspace we will need to devise a way to assign a distinct watchdogNumber to each platform device at device registration time. Signed-off-by: Scott Cheloha --- .../watchdog/watchdog-parameters.rst | 12 + drivers/watchdog/Kconfig | 8 + drivers/watchdog/Makefile | 1 + drivers/watchdog/pseries-wdt.c| 264 ++ 4 files changed, 285 insertions(+) create mode 100644 drivers/watchdog/pseries-wdt.c diff --git a/Documentation/watchdog/watchdog-parameters.rst b/Documentation/watchdog/watchdog-parameters.rst index 223c99361a30..29153eed6689 100644 --- a/Documentation/watchdog/watchdog-parameters.rst +++ b/Documentation/watchdog/watchdog-parameters.rst @@ -425,6 +425,18 @@ pnx833x_wdt: - +pseries-wdt: +action: + Action taken when watchdog expires: 0 (power off), 1 (restart), + 2 (dump and restart). (default=1) +timeout: + Initial watchdog timeout in seconds. (default=60) +nowayout: + Watchdog cannot be stopped once started. + (default=kernel config parameter) + +- + rc32434_wdt: timeout: Watchdog timeout value, in seconds (default=20) diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index c4e82a8d863f..06b412603f3e 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -1932,6 +1932,14 @@ config MEN_A21_WDT # PPC64 Architecture +config PSERIES_WDT + tristate "POWER Architecture Platform Watchdog Timer" + depends on PPC_PSERIES + select WATCHDOG_CORE + help + Driver for virtual watchdog timers provided by PAPR + hypervisors (e.g. PowerVM, KVM). + config WATCHDOG_RTAS tristate "RTAS watchdog" depends on PPC_RTAS diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index f7da867e8782..f35660409f17 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -184,6 +184,7 @@ obj-$(CONFIG_BOOKE_WDT) += booke_wdt.o obj-$(CONFIG_MEN_A21_WDT) += mena21_wdt.o # PPC64 Architecture +obj-$(CONFIG_PSERIES_WDT) += pseries-wdt.o obj-$(CONFIG_WATCHDOG_RTAS) += wdrtas.o # S390 Architecture diff --git a/drivers/watchdog/pseries-wdt.c b/drivers/watchdog/pseries-wdt.c new file mode 100644 index ..cfe53587457d --- /dev/null +++ b/drivers/watchdog/pseries-wdt.c @@ -0,0 +1,264 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) 2022 International Business Machines, Inc. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define DRV_NAME "pseries-wdt" + +/* + * The PAPR's MSB->LSB bit ordering is 0->63. These macros simplify + * defining bitfields as described in the PAPR without needing to + * transpose values to the more C-like 63->0 ordering. + */ +#define SETFIELD(_v, _b, _e) \ + (((unsigned long)(_v) << PPC_BITLSHIFT(_e)) & PPC_BITMASK((_b), (_e))) +#define GETFIELD(_v, _b, _e) \ + (((unsigned long)(_v) & PPC_BITMASK((_b), (_e))) >> PPC_BITLSHIFT(_e)) + +/* + * The H_WATCHDOG hypercall first appears in PAPR v2.12 and is + * described fully in sections 14.5 and 14.15.6. + * + * + * H_WATCHDOG Input + * + * R4: "flags": + * + * Bits 48-55: "operation" + * + * 0x01 Start Watchdog + * 0x02 Stop Watchdog + * 0x03 Query Watchdog Capabilities + */ +#define PSERIES_WDTF_OP(op)SETFIELD((op), 48, 55) +#define PSERIES_WDTF_OP_START PSERIES_WDTF_OP(0x1) +#define PSERIES_WDTF_OP_STOP PSERIES_WDTF_OP(0x2) +#define PSERIES_WDTF_OP_QUERY PSERIES_WDTF_OP(0x3) + +/* + * Bits 56-63: "timeoutAction" (for "Start Watchdog" only) + * + * 0x01 Hard poweroff + * 0x02 Hard restart + * 0x03 Dump restart + */ +#define PSERIES_WDTF_ACTION(ac)SETFIELD(ac, 56, 63) +#define PSERIES_WDTF_ACTION_HARD_POWEROFF PSERIES_WDTF_ACTION(0x1) +#define PSERIES_WDTF_ACTION_HARD_RESTART PSERIES_WDTF_ACTION(0x2) +#define PSERIES_WDTF_ACTION_DUMP_RESTART PSERIES_WDTF_ACTION(0x3) + +/* + * H_WATCHDOG Output + * + * R3: Return code + * + * H_SUCCESSThe operation completed. + * + * H_BUSY The hypervisor is too busy; retry the operation. + * + * H_PARAMETER The given "flags" are somehow invalid. Either the + * "operation" or "timeoutAction" is invalid, or a + *
[PATCH v2 2/4] powerpc/pseries: add FW_FEATURE_WATCHDOG flag
PAPR v2.12 specifies a new optional function set, "hcall-watchdog",
for the /rtas/ibm,hypertas-functions property. The presence of this
function set indicates support for the H_WATCHDOG hypercall.
Check for this function set and, if present, set the new
FW_FEATURE_WATCHDOG flag.
Signed-off-by: Scott Cheloha
---
arch/powerpc/include/asm/firmware.h | 3 ++-
arch/powerpc/platforms/pseries/firmware.c | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/firmware.h
b/arch/powerpc/include/asm/firmware.h
index 834b8ecf..398e0b5e485f 100644
--- a/arch/powerpc/include/asm/firmware.h
+++ b/arch/powerpc/include/asm/firmware.h
@@ -55,6 +55,7 @@
#define FW_FEATURE_RPT_INVALIDATE ASM_CONST(0x0100)
#define FW_FEATURE_FORM2_AFFINITY ASM_CONST(0x0200)
#define FW_FEATURE_ENERGY_SCALE_INFO ASM_CONST(0x0400)
+#define FW_FEATURE_WATCHDOGASM_CONST(0x0800)
#ifndef __ASSEMBLY__
@@ -76,7 +77,7 @@ enum {
FW_FEATURE_DRC_INFO | FW_FEATURE_BLOCK_REMOVE |
FW_FEATURE_PAPR_SCM | FW_FEATURE_ULTRAVISOR |
FW_FEATURE_RPT_INVALIDATE | FW_FEATURE_FORM2_AFFINITY |
- FW_FEATURE_ENERGY_SCALE_INFO,
+ FW_FEATURE_ENERGY_SCALE_INFO | FW_FEATURE_WATCHDOG,
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/platforms/pseries/firmware.c
b/arch/powerpc/platforms/pseries/firmware.c
index 09c119b2f623..080108d129ed 100644
--- a/arch/powerpc/platforms/pseries/firmware.c
+++ b/arch/powerpc/platforms/pseries/firmware.c
@@ -67,6 +67,7 @@ hypertas_fw_features_table[] = {
{FW_FEATURE_PAPR_SCM, "hcall-scm"},
{FW_FEATURE_RPT_INVALIDATE, "hcall-rpt-invalidate"},
{FW_FEATURE_ENERGY_SCALE_INFO, "hcall-energy-scale-info"},
+ {FW_FEATURE_WATCHDOG, "hcall-watchdog"},
};
/* Build up the firmware features bitmask using the contents of
--
2.27.0
[PATCH v2 1/4] powerpc/pseries: hvcall.h: add H_WATCHDOG opcode, H_NOOP return code
PAPR v2.12 defines a new hypercall, H_WATCHDOG. The hypercall permits guest control of one or more virtual watchdog timers. Add the opcode for the H_WATCHDOG hypercall to hvcall.h. While here, add a definition for H_NOOP, a possible return code for H_WATCHDOG. Signed-off-by: Scott Cheloha --- arch/powerpc/include/asm/hvcall.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h index d92a20a85395..4b4f69c35b4f 100644 --- a/arch/powerpc/include/asm/hvcall.h +++ b/arch/powerpc/include/asm/hvcall.h @@ -87,6 +87,7 @@ #define H_P7 -60 #define H_P8 -61 #define H_P9 -62 +#define H_NOOP -63 #define H_TOO_BIG -64 #define H_UNSUPPORTED -67 #define H_OVERLAP -68 @@ -324,7 +325,8 @@ #define H_RPT_INVALIDATE 0x448 #define H_SCM_FLUSH0x44C #define H_GET_ENERGY_SCALE_INFO0x450 -#define MAX_HCALL_OPCODE H_GET_ENERGY_SCALE_INFO +#define H_WATCHDOG 0x45C +#define MAX_HCALL_OPCODE H_WATCHDOG /* Scope args for H_SCM_UNBIND_ALL */ #define H_UNBIND_SCOPE_ALL (0x1) -- 2.27.0
[PATCH v2 0/4] pseries-wdt: initial support for H_WATCHDOG-based watchdog timers
PAPR v2.12 defines a new hypercall, H_WATCHDOG. This patch series adds support for this hypercall to powerpc/pseries kernels and introduces a new watchdog driver, "pseries-wdt", for the virtual timers exposed by the hypercall. This series is preceded by the following: RFC v1: https://lore.kernel.org/linux-watchdog/20220413165104.179144-1-chel...@linux.ibm.com/ RFC v2: https://lore.kernel.org/linux-watchdog/20220509174357.5448-1-chel...@linux.ibm.com/ PATCH v1: https://lore.kernel.org/linux-watchdog/20220520183552.33426-1-chel...@linux.ibm.com/ Changes of note from PATCH v1: - Trim down the large comment documenting the H_WATCHDOG hypercall. The comment is likely to rot, so remove anything we aren't using and anything overly obvious. - Remove any preprocessor definitions not actually used in the module right now. If we want to use other features offered by the hypercall we can add them in later. They're just clutter until then. - Simplify the "action" module parameter. The value is now an index into an array of possible timeoutAction values. This design removes the need for the custom get/set methods used in PATCH v1. Now we merely need to check that the "action" value is a valid index during pseries_wdt_probe(). Easy. - Make the timeoutAction a member of pseries_wdt, "action". This eliminates the use of a global variable during pseries_wdt_start(). - Use watchdog_init_timeout() idiomatically. Check its return value and error out of pseries_wdt_probe() if it fails.
Re: [PATCH v2 3/6] powerpc: Book3S 64-bit outline-only KASAN support
Hi, On Wed, May 18, 2022 at 08:05:31PM +1000, Paul Mackerras wrote: > From: Daniel Axtens > > Implement a limited form of KASAN for Book3S 64-bit machines running under > the Radix MMU, supporting only outline mode. > > - Enable the compiler instrumentation to check addresses and maintain the >shadow region. (This is the guts of KASAN which we can easily reuse.) > > - Require kasan-vmalloc support to handle modules and anything else in >vmalloc space. > > - KASAN needs to be able to validate all pointer accesses, but we can't >instrument all kernel addresses - only linear map and vmalloc. On boot, >set up a single page of read-only shadow that marks all iomap and >vmemmap accesses as valid. > > - Document KASAN in powerpc docs. > With this patch applied, powerpc:allmodconfig builds fail as follows. Building powerpc:allmodconfig ... failed -- Error log: Error: External symbol 'memset' referenced from prom_init.c make[3]: [arch/powerpc/kernel/Makefile:202: arch/powerpc/kernel/prom_init_check] Error 1 (ignored) powerpc64-linux-ld: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_lib.o uses hard float, drivers/gpu/drm/amd/amdgpu/../display/dc/dcn31/dcn31_resource.o uses soft float powerpc64-linux-ld: failed to merge target specific data of file drivers/gpu/drm/amd/amdgpu/../display/dc/dcn31/dcn31_resource.o powerpc64-linux-ld: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_lib.o uses hard float, drivers/gpu/drm/amd/amdgpu/../display/dc/dcn315/dcn315_resource.o uses soft float powerpc64-linux-ld: failed to merge target specific data of file drivers/gpu/drm/amd/amdgpu/../display/dc/dcn315/dcn315_resource.o powerpc64-linux-ld: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_lib.o uses hard float, drivers/gpu/drm/amd/amdgpu/../display/dc/dcn316/dcn316_resource.o uses soft float powerpc64-linux-ld: failed to merge target specific data of file drivers/gpu/drm/amd/amdgpu/../display/dc/dcn316/dcn316_resource.o make[5]: [scripts/Makefile.build:435: drivers/gpu/drm/amd/amdgpu/amdgpu.o] Error 1 (ignored) make[2]: *** No rule to make target 'drivers/gpu/drm/amd/amdgpu/amdgpu.o', needed by 'modules-only.symvers'. Stop. make[1]: [Makefile:1753: modules] Error 2 (ignored) This is with gcc 11.3 and binutils 2.38. I also tried with gcc 11.2 and binutils 2.36.1, with the same results. Reverting this patch fixes the problem. Guenter
Re: [PATCH 1/3] of: dynamic: add of_property_alloc() and of_property_free()
On Wed, Jun 1, 2022 at 5:31 PM Tyrel Datwyler wrote:
>
> On 5/5/22 12:37, Rob Herring wrote:
> > On Wed, May 04, 2022 at 05:40:31PM +0200, Clément Léger wrote:
> >> Add function which allows to dynamically allocate and free properties.
> >> Use this function internally for all code that used the same logic
> >> (mainly __of_prop_dup()).
> >>
> >> Signed-off-by: Clément Léger
> >> ---
> >> drivers/of/dynamic.c | 101 ++-
> >> include/linux/of.h | 16 +++
> >> 2 files changed, 88 insertions(+), 29 deletions(-)
> >>
> >> diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
> >> index cd3821a6444f..e8700e509d2e 100644
> >> --- a/drivers/of/dynamic.c
> >> +++ b/drivers/of/dynamic.c
> >> @@ -313,9 +313,7 @@ static void property_list_free(struct property
> >> *prop_list)
> >>
> >> for (prop = prop_list; prop != NULL; prop = next) {
> >> next = prop->next;
> >> -kfree(prop->name);
> >> -kfree(prop->value);
> >> -kfree(prop);
> >> +of_property_free(prop);
> >> }
> >> }
> >>
> >> @@ -367,48 +365,95 @@ void of_node_release(struct kobject *kobj)
> >> }
> >>
> >> /**
> >> - * __of_prop_dup - Copy a property dynamically.
> >> - * @prop: Property to copy
> >> + * of_property_free - Free a property allocated dynamically.
> >> + * @prop: Property to be freed
> >> + */
> >> +void of_property_free(const struct property *prop)
> >> +{
> >> +kfree(prop->value);
> >> +kfree(prop->name);
> >> +kfree(prop);
> >> +}
> >> +EXPORT_SYMBOL(of_property_free);
> >> +
> >> +/**
> >> + * of_property_alloc - Allocate a property dynamically.
> >> + * @name: Name of the new property
> >> + * @value: Value that will be copied into the new property value
> >> + * @value_len: length of @value to be copied into the new property
> >> value
> >> + * @len:Length of new property value, must be greater than @value_len
> >
> > What's the usecase for the lengths being different? That doesn't seem
> > like a common case, so perhaps handle it with a NULL value and
> > non-zero length. Then the caller has to deal with populating
> > prop->value.
> >
> >> * @allocflags: Allocation flags (typically pass GFP_KERNEL)
> >> *
> >> - * Copy a property by dynamically allocating the memory of both the
> >> + * Create a property by dynamically allocating the memory of both the
> >> * property structure and the property name & contents. The property's
> >> * flags have the OF_DYNAMIC bit set so that we can differentiate between
> >> * dynamically allocated properties and not.
> >> *
> >> * Return: The newly allocated property or NULL on out of memory error.
> >> */
> >> -struct property *__of_prop_dup(const struct property *prop, gfp_t
> >> allocflags)
> >> +struct property *of_property_alloc(const char *name, const void *value,
> >> + int value_len, int len, gfp_t allocflags)
> >> {
> >> -struct property *new;
> >> +int alloc_len = len;
> >> +struct property *prop;
> >> +
> >> +if (len < value_len)
> >> +return NULL;
> >>
> >> -new = kzalloc(sizeof(*new), allocflags);
> >> -if (!new)
> >> +prop = kzalloc(sizeof(*prop), allocflags);
> >> +if (!prop)
> >> return NULL;
> >>
> >> +prop->name = kstrdup(name, allocflags);
> >> +if (!prop->name)
> >> +goto out_err;
> >> +
> >> /*
> >> - * NOTE: There is no check for zero length value.
> >> - * In case of a boolean property, this will allocate a value
> >> - * of zero bytes. We do this to work around the use
> >> - * of of_get_property() calls on boolean values.
> >> + * Even if the property has no value, it must be set to a
> >> + * non-null value since of_get_property() is used to check
> >> + * some values that might or not have a values (ranges for
> >> + * instance). Moreover, when the node is released, prop->value
> >> + * is kfreed so the memory must come from kmalloc.
> >
> > Allowing for NULL value didn't turn out well...
> >
> > We know that we can do the kfree because OF_DYNAMIC is set IIRC...
> >
> > If we do 1 allocation for prop and value, then we can test
> > for "prop->value == prop + 1" to determine if we need to free or not.
>
> If its a single allocation do we even need a test? Doesn't kfree(prop) take
> care
> of the property and the trailing memory allocated for the value?
Yes, it does when it's a single alloc, but it's testing for when
prop->value is not a single allocation because we could have either.
Rob
Re: [PATCH] livepatch: Remove klp_arch_set_pc() and asm/livepatch.h
Hi Petr, Le 23/05/2022 à 17:15, Petr Mladek a écrit : > On Mon 2022-05-23 06:51:47, Christophe Leroy wrote: >> >> >> Le 29/03/2022 à 13:22, Petr Mladek a écrit : >>> On Mon 2022-03-28 08:26:48, Christophe Leroy wrote: All three versions of klp_arch_set_pc() do exactly the same: they call ftrace_instruction_pointer_set(). Call ftrace_instruction_pointer_set() directly and remove klp_arch_set_pc(). As klp_arch_set_pc() was the only thing remaining in asm/livepatch.h on x86 and s390, remove asm/livepatch.h livepatch.h remains on powerpc but its content is exclusively used by powerpc specific code. Signed-off-by: Christophe Leroy >>> >>> Acked-by: Petr Mladek >>> >>> I am going to take it via livepatch/livepatch.git for 5.19. We are >>> already in the middle of the merge window and this is not critical. >>> >> >> I haven't seen it in linux-next. >> >> Do you still plan to take it for 5.19 ? > > Thanks a lot for pointing this out. I have completely forgot about > this patch /o\ > > I have just pushed it into livepatching/livepatching.git, > branch for-5.19/cleanup. > > I am going to create pull request for 5.19 by the end of this week > after it gets a spin in linux-next. > I can't see it yet in linus mainline tree. Any issue ? Thanks Christophe
Re: [PATCH] powerpc/64: Drop ppc_inst_as_str()
On Thu, Jun 02, 2022 at 01:01:04PM +1000, Jordan Niethe wrote: > > What about the more fundamental thing? Have the order of the two halves > > of a prefixed insn as ulong not depend on endianness? It really is two > > opcodes, and the prefixed one is first, always, even in LE. > The reason would be the value of as ulong is then used to write a > prefixed instruction to > memory with std. > If both endiannesses had the halves the same one of them would store > the suffix in front of the prefix. You cannot do such a (possibly) unaligned access from C though, not without invoking undefined behaviour. The compiler usually lets you get away with it, but there are no guarantees. You can make sure you only ever do such an access from assembler code of course. Swapping the two halves of a register costs at most one insn. It is harmful premature optimisation to make this single cycle advantage override more important consideration (almost everything else :-) ) Segher
[PATCH] ASoC: imx-audmux: remove unnecessary check of clk_disable_unprepare
From: Minghao Chi Because clk_disable_unprepare already checked NULL clock parameter, so the additional checks are unnecessary, just remove them. Reported-by: Zeal Robot Signed-off-by: Minghao Chi --- sound/soc/fsl/imx-audmux.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/sound/soc/fsl/imx-audmux.c b/sound/soc/fsl/imx-audmux.c index dfa05d40b276..f434fa7decc1 100644 --- a/sound/soc/fsl/imx-audmux.c +++ b/sound/soc/fsl/imx-audmux.c @@ -71,8 +71,7 @@ static ssize_t audmux_read_file(struct file *file, char __user *user_buf, ptcr = readl(audmux_base + IMX_AUDMUX_V2_PTCR(port)); pdcr = readl(audmux_base + IMX_AUDMUX_V2_PDCR(port)); - if (audmux_clk) - clk_disable_unprepare(audmux_clk); + clk_disable_unprepare(audmux_clk); buf = kmalloc(PAGE_SIZE, GFP_KERNEL); if (!buf) @@ -218,8 +217,7 @@ int imx_audmux_v2_configure_port(unsigned int port, unsigned int ptcr, writel(ptcr, audmux_base + IMX_AUDMUX_V2_PTCR(port)); writel(pdcr, audmux_base + IMX_AUDMUX_V2_PDCR(port)); - if (audmux_clk) - clk_disable_unprepare(audmux_clk); + clk_disable_unprepare(audmux_clk); return 0; } -- 2.25.1
