- Linux-stable-mirror - lists.linaro.org

FAILED: patch "[PATCH] PCI: Unify delay handling for reset and resume" failed to apply to 4.19-stable tree

by gregkh＠linuxfoundation.org

The patch below does not apply to the 4.19-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <stable(a)vger.kernel.org>. To reproduce the conflict and resubmit, you may use the following commands: git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-4.19.y git checkout FETCH_HEAD git cherry-pick -x ac91e6980563ed53afadd925fa6585ffd2bc4a2c # <resolve conflicts, build, test, etc.> git commit -s git send-email --to '<stable(a)vger.kernel.org>' --in-reply-to '167820491313734(a)kroah.com' --subject-prefix 'PATCH 4.19.y' HEAD^.. Possible dependencies: ac91e6980563 ("PCI: Unify delay handling for reset and resume") 8ef0217227b4 ("PCI/PM: Observe reset delay irrespective of bridge_d3") 730643d33e2d ("PCI/PM: Resume subordinate bus in bus type callbacks") 18a94192e20d ("PCI/PM: Define pci_restore_standard_config() only for CONFIG_PM_SLEEP") 0c5c62ddf88c ("Merge tag 'pci-v5.16-changes' of git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci") thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From ac91e6980563ed53afadd925fa6585ffd2bc4a2c Mon Sep 17 00:00:00 2001 From: Lukas Wunner <lukas(a)wunner.de> Date: Sun, 15 Jan 2023 09:20:32 +0100 Subject: [PATCH] PCI: Unify delay handling for reset and resume Sheng Bi reports that pci_bridge_secondary_bus_reset() may fail to wait for devices on the secondary bus to become accessible after reset: Although it does call pci_dev_wait(), it erroneously passes the bridge's pci_dev rather than that of a child. The bridge of course is always accessible while its secondary bus is reset, so pci_dev_wait() returns immediately. Sheng Bi proposes introducing a new pci_bridge_secondary_bus_wait() function which is called from pci_bridge_secondary_bus_reset(): https://lore.kernel.org/linux-pci/20220523171517.32407-1-windy.bi.enflame@g… However we already have pci_bridge_wait_for_secondary_bus() which does almost exactly what we need. So far it's only called on resume from D3cold (which implies a Fundamental Reset per PCIe r6.0 sec 5.8). Re-using it for Secondary Bus Resets is a leaner and more rational approach than introducing a new function. That only requires a few minor tweaks: - Amend pci_bridge_wait_for_secondary_bus() to await accessibility of the first device on the secondary bus by calling pci_dev_wait() after performing the prescribed delays. pci_dev_wait() needs two parameters, a reset reason and a timeout, which callers must now pass to pci_bridge_wait_for_secondary_bus(). The timeout is 1 sec for resume (PCIe r6.0 sec 6.6.1) and 60 sec for reset (commit 821cdad5c46c ("PCI: Wait up to 60 seconds for device to become ready after FLR")). Introduce a PCI_RESET_WAIT macro for the 1 sec timeout. - Amend pci_bridge_wait_for_secondary_bus() to return 0 on success or -ENOTTY on error for consumption by pci_bridge_secondary_bus_reset(). - Drop an unnecessary 1 sec delay from pci_reset_secondary_bus() which is now performed by pci_bridge_wait_for_secondary_bus(). A static delay this long is only necessary for Conventional PCI, so modern PCIe systems benefit from shorter reset times as a side effect. Fixes: 6b2f1351af56 ("PCI: Wait for device to become ready after secondary bus reset") Link: https://lore.kernel.org/r/da77c92796b99ec568bd070cbe4725074a117038.16737695… Reported-by: Sheng Bi <windy.bi.enflame(a)gmail.com> Tested-by: Ravi Kishore Koppuravuri <ravi.kishore.koppuravuri(a)intel.com> Signed-off-by: Lukas Wunner <lukas(a)wunner.de> Signed-off-by: Bjorn Helgaas <bhelgaas(a)google.com> Reviewed-by: Mika Westerberg <mika.westerberg(a)linux.intel.com> Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy(a)linux.intel.com> Cc: stable(a)vger.kernel.org # v4.17+ diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index a2ceeacc33eb..7a19f11daca3 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -572,7 +572,7 @@ static void pci_pm_default_resume_early(struct pci_dev *pci_dev) static void pci_pm_bridge_power_up_actions(struct pci_dev *pci_dev) { - pci_bridge_wait_for_secondary_bus(pci_dev); + pci_bridge_wait_for_secondary_bus(pci_dev, "resume", PCI_RESET_WAIT); /* * When powering on a bridge from D3cold, the whole hierarchy may be * powered on into D0uninitialized state, resume them to give them a diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index f43f3e84f634..509f6b5c9e14 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1174,7 +1174,7 @@ static int pci_dev_wait(struct pci_dev *dev, char *reset_type, int timeout) return -ENOTTY; } - if (delay > 1000) + if (delay > PCI_RESET_WAIT) pci_info(dev, "not ready %dms after %s; waiting\n", delay - 1, reset_type); @@ -1183,7 +1183,7 @@ static int pci_dev_wait(struct pci_dev *dev, char *reset_type, int timeout) pci_read_config_dword(dev, PCI_COMMAND, &id); } - if (delay > 1000) + if (delay > PCI_RESET_WAIT) pci_info(dev, "ready %dms after %s\n", delay - 1, reset_type); @@ -4948,24 +4948,31 @@ static int pci_bus_max_d3cold_delay(const struct pci_bus *bus) /** * pci_bridge_wait_for_secondary_bus - Wait for secondary bus to be accessible * @dev: PCI bridge + * @reset_type: reset type in human-readable form + * @timeout: maximum time to wait for devices on secondary bus (milliseconds) * * Handle necessary delays before access to the devices on the secondary - * side of the bridge are permitted after D3cold to D0 transition. + * side of the bridge are permitted after D3cold to D0 transition + * or Conventional Reset. * * For PCIe this means the delays in PCIe 5.0 section 6.6.1. For * conventional PCI it means Tpvrh + Trhfa specified in PCI 3.0 section * 4.3.2. + * + * Return 0 on success or -ENOTTY if the first device on the secondary bus + * failed to become accessible. */ -void pci_bridge_wait_for_secondary_bus(struct pci_dev *dev) +int pci_bridge_wait_for_secondary_bus(struct pci_dev *dev, char *reset_type, + int timeout) { struct pci_dev *child; int delay; if (pci_dev_is_disconnected(dev)) - return; + return 0; if (!pci_is_bridge(dev)) - return; + return 0; down_read(&pci_bus_sem); @@ -4977,14 +4984,14 @@ void pci_bridge_wait_for_secondary_bus(struct pci_dev *dev) */ if (!dev->subordinate || list_empty(&dev->subordinate->devices)) { up_read(&pci_bus_sem); - return; + return 0; } /* Take d3cold_delay requirements into account */ delay = pci_bus_max_d3cold_delay(dev->subordinate); if (!delay) { up_read(&pci_bus_sem); - return; + return 0; } child = list_first_entry(&dev->subordinate->devices, struct pci_dev, @@ -4993,14 +5000,12 @@ void pci_bridge_wait_for_secondary_bus(struct pci_dev *dev) /* * Conventional PCI and PCI-X we need to wait Tpvrh + Trhfa before - * accessing the device after reset (that is 1000 ms + 100 ms). In - * practice this should not be needed because we don't do power - * management for them (see pci_bridge_d3_possible()). + * accessing the device after reset (that is 1000 ms + 100 ms). */ if (!pci_is_pcie(dev)) { pci_dbg(dev, "waiting %d ms for secondary bus\n", 1000 + delay); msleep(1000 + delay); - return; + return 0; } /* @@ -5017,11 +5022,11 @@ void pci_bridge_wait_for_secondary_bus(struct pci_dev *dev) * configuration requests if we only wait for 100 ms (see * https://bugzilla.kernel.org/show_bug.cgi?id=203885). * - * Therefore we wait for 100 ms and check for the device presence. - * If it is still not present give it an additional 100 ms. + * Therefore we wait for 100 ms and check for the device presence + * until the timeout expires. */ if (!pcie_downstream_port(dev)) - return; + return 0; if (pcie_get_speed_cap(dev) <= PCIE_SPEED_5_0GT) { pci_dbg(dev, "waiting %d ms for downstream link\n", delay); @@ -5032,14 +5037,11 @@ void pci_bridge_wait_for_secondary_bus(struct pci_dev *dev) if (!pcie_wait_for_link_delay(dev, true, delay)) { /* Did not train, no need to wait any further */ pci_info(dev, "Data Link Layer Link Active not set in 1000 msec\n"); - return; + return -ENOTTY; } } - if (!pci_device_is_present(child)) { - pci_dbg(child, "waiting additional %d ms to become accessible\n", delay); - msleep(delay); - } + return pci_dev_wait(child, reset_type, timeout - delay); } void pci_reset_secondary_bus(struct pci_dev *dev) @@ -5058,15 +5060,6 @@ void pci_reset_secondary_bus(struct pci_dev *dev) ctrl &= ~PCI_BRIDGE_CTL_BUS_RESET; pci_write_config_word(dev, PCI_BRIDGE_CONTROL, ctrl); - - /* - * Trhfa for conventional PCI is 2^25 clock cycles. - * Assuming a minimum 33MHz clock this results in a 1s - * delay before we can consider subordinate devices to - * be re-initialized. PCIe has some ways to shorten this, - * but we don't make use of them yet. - */ - ssleep(1); } void __weak pcibios_reset_secondary_bus(struct pci_dev *dev) @@ -5085,7 +5078,8 @@ int pci_bridge_secondary_bus_reset(struct pci_dev *dev) { pcibios_reset_secondary_bus(dev); - return pci_dev_wait(dev, "bus reset", PCIE_RESET_READY_POLL_MS); + return pci_bridge_wait_for_secondary_bus(dev, "bus reset", + PCIE_RESET_READY_POLL_MS); } EXPORT_SYMBOL_GPL(pci_bridge_secondary_bus_reset); diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index 9ed3b5550043..ce1fc3a90b3f 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -64,6 +64,13 @@ struct pci_cap_saved_state *pci_find_saved_ext_cap(struct pci_dev *dev, #define PCI_PM_D3HOT_WAIT 10 /* msec */ #define PCI_PM_D3COLD_WAIT 100 /* msec */ +/* + * Following exit from Conventional Reset, devices must be ready within 1 sec + * (PCIe r6.0 sec 6.6.1). A D3cold to D0 transition implies a Conventional + * Reset (PCIe r6.0 sec 5.8). + */ +#define PCI_RESET_WAIT 1000 /* msec */ + void pci_update_current_state(struct pci_dev *dev, pci_power_t state); void pci_refresh_power_state(struct pci_dev *dev); int pci_power_up(struct pci_dev *dev); @@ -86,8 +93,9 @@ void pci_msi_init(struct pci_dev *dev); void pci_msix_init(struct pci_dev *dev); bool pci_bridge_d3_possible(struct pci_dev *dev); void pci_bridge_d3_update(struct pci_dev *dev); -void pci_bridge_wait_for_secondary_bus(struct pci_dev *dev); void pci_bridge_reconfigure_ltr(struct pci_dev *dev); +int pci_bridge_wait_for_secondary_bus(struct pci_dev *dev, char *reset_type, + int timeout); static inline void pci_wakeup_event(struct pci_dev *dev) {

2 years, 8 months

1
0
0 0

FAILED: patch "[PATCH] RISC-V: take text_mutex during alternative patching" failed to apply to 6.2-stable tree

by gregkh＠linuxfoundation.org

The patch below does not apply to the 6.2-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <stable(a)vger.kernel.org>. To reproduce the conflict and resubmit, you may use the following commands: git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.2.y git checkout FETCH_HEAD git cherry-pick -x 9493e6f3ce02f44c21aa19f3cbf3b9aa05479d06 # <resolve conflicts, build, test, etc.> git commit -s git send-email --to '<stable(a)vger.kernel.org>' --in-reply-to '167820483981188(a)kroah.com' --subject-prefix 'PATCH 6.2.y' HEAD^.. Possible dependencies: 9493e6f3ce02 ("RISC-V: take text_mutex during alternative patching") bfd6fc5d8014 ("riscv: Fix early alternative patching") 8d23e94a4433 ("riscv: switch to relative alternative entries") 4bf8860760d9 ("riscv: cpufeature: extend riscv_cpufeature_patch_func to all ISA extensions") abcc445acdbe ("riscv: move riscv_noncoherent_supported() out of ZICBOM probe") 61a9b7129070 ("Merge patch series "Putting some basic order on isa extension lists"") thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From 9493e6f3ce02f44c21aa19f3cbf3b9aa05479d06 Mon Sep 17 00:00:00 2001 From: Conor Dooley <conor.dooley(a)microchip.com> Date: Sun, 12 Feb 2023 19:47:36 +0000 Subject: [PATCH] RISC-V: take text_mutex during alternative patching Guenter reported a splat during boot, that Samuel pointed out was the lockdep assertion failing in patch_insn_write(): WARNING: CPU: 0 PID: 0 at arch/riscv/kernel/patch.c:63 patch_insn_write+0x222/0x2f6 epc : patch_insn_write+0x222/0x2f6 ra : patch_insn_write+0x21e/0x2f6 epc : ffffffff800068c6 ra : ffffffff800068c2 sp : ffffffff81803df0 gp : ffffffff81a1ab78 tp : ffffffff81814f80 t0 : ffffffffffffe000 t1 : 0000000000000001 t2 : 4c45203a76637369 s0 : ffffffff81803e40 s1 : 0000000000000004 a0 : 0000000000000000 a1 : ffffffffffffffff a2 : 0000000000000004 a3 : 0000000000000000 a4 : 0000000000000001 a5 : 0000000000000000 a6 : 0000000000000000 a7 : 0000000052464e43 s2 : ffffffff80b4889c s3 : 000000000000082c s4 : ffffffff80b48828 s5 : 0000000000000828 s6 : ffffffff8131a0a0 s7 : 0000000000000fff s8 : 0000000008000200 s9 : ffffffff8131a520 s10: 0000000000000018 s11: 000000000000000b t3 : 0000000000000001 t4 : 000000000000000d t5 : ffffffffd8180000 t6 : ffffffff81803bc8 status: 0000000200000100 badaddr: 0000000000000000 cause: 0000000000000003 [<ffffffff800068c6>] patch_insn_write+0x222/0x2f6 [<ffffffff80006a36>] patch_text_nosync+0xc/0x2a [<ffffffff80003b86>] riscv_cpufeature_patch_func+0x52/0x98 [<ffffffff80003348>] _apply_alternatives+0x46/0x86 [<ffffffff80c02d36>] apply_boot_alternatives+0x3c/0xfa [<ffffffff80c03ad8>] setup_arch+0x584/0x5b8 [<ffffffff80c0075a>] start_kernel+0xa2/0x8f8 This issue was exposed by 702e64550b12 ("riscv: fpu: switch has_fpu() to riscv_has_extension_likely()"), as it is the patching in has_fpu() that triggers the splats in Guenter's report. Take the text_mutex before doing any code patching to satisfy lockdep. Fixes: ff689fd21cb1 ("riscv: add RISC-V Svpbmt extension support") Fixes: a35707c3d850 ("riscv: add memory-type errata for T-Head") Fixes: 1a0e5dbd3723 ("riscv: sifive: Add SiFive alternative ports") Reported-by: Guenter Roeck <linux(a)roeck-us.net> Link: https://lore.kernel.org/all/20230212154333.GA3760469@roeck-us.net/ Signed-off-by: Conor Dooley <conor.dooley(a)microchip.com> Reviewed-by: Samuel Holland <samuel(a)sholland.org> Tested-by: Guenter Roeck <linux(a)roeck-us.net> Link: https://lore.kernel.org/r/20230212194735.491785-1-conor@kernel.org Cc: stable(a)vger.kernel.org Signed-off-by: Palmer Dabbelt <palmer(a)rivosinc.com> diff --git a/arch/riscv/errata/sifive/errata.c b/arch/riscv/errata/sifive/errata.c index ef9a4eec0dba..da55cb247e89 100644 --- a/arch/riscv/errata/sifive/errata.c +++ b/arch/riscv/errata/sifive/errata.c @@ -4,6 +4,7 @@ */ #include <linux/kernel.h> +#include <linux/memory.h> #include <linux/module.h> #include <linux/string.h> #include <linux/bug.h> @@ -107,8 +108,10 @@ void __init_or_module sifive_errata_patch_func(struct alt_entry *begin, tmp = (1U << alt->errata_id); if (cpu_req_errata & tmp) { + mutex_lock(&text_mutex); patch_text_nosync(ALT_OLD_PTR(alt), ALT_ALT_PTR(alt), alt->alt_len); + mutex_lock(&text_mutex); cpu_apply_errata |= tmp; } } diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c index 1dd90a5f86f0..3b96a06d3c54 100644 --- a/arch/riscv/errata/thead/errata.c +++ b/arch/riscv/errata/thead/errata.c @@ -5,6 +5,7 @@ #include <linux/bug.h> #include <linux/kernel.h> +#include <linux/memory.h> #include <linux/module.h> #include <linux/string.h> #include <linux/uaccess.h> @@ -101,10 +102,13 @@ void __init_or_module thead_errata_patch_func(struct alt_entry *begin, struct al altptr = ALT_ALT_PTR(alt); /* On vm-alternatives, the mmu isn't running yet */ - if (stage == RISCV_ALTERNATIVES_EARLY_BOOT) + if (stage == RISCV_ALTERNATIVES_EARLY_BOOT) { memcpy(oldptr, altptr, alt->alt_len); - else + } else { + mutex_lock(&text_mutex); patch_text_nosync(oldptr, altptr, alt->alt_len); + mutex_unlock(&text_mutex); + } } } diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c index 21fb567e1b22..59d58ee0f68d 100644 --- a/arch/riscv/kernel/cpufeature.c +++ b/arch/riscv/kernel/cpufeature.c @@ -10,6 +10,7 @@ #include <linux/ctype.h> #include <linux/libfdt.h> #include <linux/log2.h> +#include <linux/memory.h> #include <linux/module.h> #include <linux/of.h> #include <asm/alternative.h> @@ -292,8 +293,11 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin, oldptr = ALT_OLD_PTR(alt); altptr = ALT_ALT_PTR(alt); + + mutex_lock(&text_mutex); patch_text_nosync(oldptr, altptr, alt->alt_len); riscv_alternative_fix_offsets(oldptr, alt->alt_len, oldptr - altptr); + mutex_unlock(&text_mutex); } } #endif

2 years, 8 months

1
0
0 0

FAILED: patch "[PATCH] RISC-V: fix ordering of Zbb extension" failed to apply to 6.1-stable tree

by gregkh＠linuxfoundation.org

The patch below does not apply to the 6.1-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <stable(a)vger.kernel.org>. To reproduce the conflict and resubmit, you may use the following commands: git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y git checkout FETCH_HEAD git cherry-pick -x 1eac28201ac0725192f5ced34192d281a06692e5 # <resolve conflicts, build, test, etc.> git commit -s git send-email --to '<stable(a)vger.kernel.org>' --in-reply-to '1678204825350(a)kroah.com' --subject-prefix 'PATCH 6.1.y' HEAD^.. Possible dependencies: 1eac28201ac0 ("RISC-V: fix ordering of Zbb extension") 80c200b34ee8 ("RISC-V: resort all extensions in consistent orders") thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From 1eac28201ac0725192f5ced34192d281a06692e5 Mon Sep 17 00:00:00 2001 From: Heiko Stuebner <heiko.stuebner(a)vrull.eu> Date: Wed, 8 Feb 2023 23:53:27 +0100 Subject: [PATCH] RISC-V: fix ordering of Zbb extension As Andrew reported, Zb* comes after Zi* according 27.11 "Subset Naming Convention" so fix the ordering accordingly. Reported-by: Andrew Jones <ajones(a)ventanamicro.com> Signed-off-by: Heiko Stuebner <heiko.stuebner(a)vrull.eu> Reviewed-by: Conor Dooley <conor.dooley(a)microchip.com> Reviewed-by: Andrew Jones <ajones(a)ventanamicro.com> Tested-by: Conor Dooley <conor.dooley(a)microchip.com> Link: https://lore.kernel.org/r/20230208225328.1636017-2-heiko@sntech.de Cc: stable(a)vger.kernel.org Signed-off-by: Palmer Dabbelt <palmer(a)rivosinc.com> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c index 420228e219f7..8400f0cc9704 100644 --- a/arch/riscv/kernel/cpu.c +++ b/arch/riscv/kernel/cpu.c @@ -185,9 +185,9 @@ arch_initcall(riscv_cpuinfo_init); * New entries to this struct should follow the ordering rules described above. */ static struct riscv_isa_ext_data isa_ext_arr[] = { - __RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB), __RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM), __RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE), + __RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB), __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF), __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC), __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL),

2 years, 8 months

1
0
0 0

FAILED: patch "[PATCH] riscv: hwcap: Don't alphabetize ISA extension IDs" failed to apply to 6.2-stable tree

by gregkh＠linuxfoundation.org

The patch below does not apply to the 6.2-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <stable(a)vger.kernel.org>. To reproduce the conflict and resubmit, you may use the following commands: git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.2.y git checkout FETCH_HEAD git cherry-pick -x dac8bf14bb49aecd1de99ebb5498fa03152f2d40 # <resolve conflicts, build, test, etc.> git commit -s git send-email --to '<stable(a)vger.kernel.org>' --in-reply-to '167820481563250(a)kroah.com' --subject-prefix 'PATCH 6.2.y' HEAD^.. Possible dependencies: dac8bf14bb49 ("riscv: hwcap: Don't alphabetize ISA extension IDs") 9daca9a5b9ac ("Merge patch series "riscv: improve boot time isa extensions handling"") thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From dac8bf14bb49aecd1de99ebb5498fa03152f2d40 Mon Sep 17 00:00:00 2001 From: Andrew Jones <ajones(a)ventanamicro.com> Date: Thu, 9 Feb 2023 13:36:36 +0100 Subject: [PATCH] riscv: hwcap: Don't alphabetize ISA extension IDs While the comment above the ISA extension ID definitions says "Entries are sorted alphabetically.", this stopped being good advice with commit d8a3d8a75206 ("riscv: hwcap: make ISA extension ids can be used in asm"), as we now use macros instead of enums. Reshuffling defines is error-prone, so, since they don't need to be in any particular order, change the advice to just adding new extensions at the bottom. Also, take the opportunity to change spaces to tabs, merge three comments into one, and move the base and max defines into more logical locations wrt the ID definitions. Signed-off-by: Andrew Jones <ajones(a)ventanamicro.com> Reviewed-by: Conor Dooley <conor.dooley(a)microchip.com> Link: https://lore.kernel.org/r/20230209123636.123537-1-ajones@ventanamicro.com Cc: stable(a)vger.kernel.org Signed-off-by: Palmer Dabbelt <palmer(a)rivosinc.com> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h index 831bebacb7fb..8f3994a7f0ca 100644 --- a/arch/riscv/include/asm/hwcap.h +++ b/arch/riscv/include/asm/hwcap.h @@ -24,29 +24,27 @@ #define RISCV_ISA_EXT_u ('u' - 'a') /* - * Increse this to higher value as kernel support more ISA extensions. - */ -#define RISCV_ISA_EXT_MAX 64 -#define RISCV_ISA_EXT_NAME_LEN_MAX 32 - -/* The base ID for multi-letter ISA extensions */ -#define RISCV_ISA_EXT_BASE 26 - -/* - * These macros represent the logical ID for each multi-letter RISC-V ISA extension. - * The logical ID should start from RISCV_ISA_EXT_BASE and must not exceed - * RISCV_ISA_EXT_MAX. 0-25 range is reserved for single letter - * extensions while all the multi-letter extensions should define the next - * available logical extension id. - * Entries are sorted alphabetically. + * These macros represent the logical IDs of each multi-letter RISC-V ISA + * extension and are used in the ISA bitmap. The logical IDs start from + * RISCV_ISA_EXT_BASE, which allows the 0-25 range to be reserved for single + * letter extensions. The maximum, RISCV_ISA_EXT_MAX, is defined in order + * to allocate the bitmap and may be increased when necessary. + * + * New extensions should just be added to the bottom, rather than added + * alphabetically, in order to avoid unnecessary shuffling. */ -#define RISCV_ISA_EXT_SSCOFPMF 26 -#define RISCV_ISA_EXT_SSTC 27 -#define RISCV_ISA_EXT_SVINVAL 28 -#define RISCV_ISA_EXT_SVPBMT 29 -#define RISCV_ISA_EXT_ZBB 30 -#define RISCV_ISA_EXT_ZICBOM 31 -#define RISCV_ISA_EXT_ZIHINTPAUSE 32 +#define RISCV_ISA_EXT_BASE 26 + +#define RISCV_ISA_EXT_SSCOFPMF 26 +#define RISCV_ISA_EXT_SSTC 27 +#define RISCV_ISA_EXT_SVINVAL 28 +#define RISCV_ISA_EXT_SVPBMT 29 +#define RISCV_ISA_EXT_ZBB 30 +#define RISCV_ISA_EXT_ZICBOM 31 +#define RISCV_ISA_EXT_ZIHINTPAUSE 32 + +#define RISCV_ISA_EXT_MAX 64 +#define RISCV_ISA_EXT_NAME_LEN_MAX 32 #ifndef __ASSEMBLY__

2 years, 8 months

1
0
0 0

FAILED: patch "[PATCH] riscv: hwcap: Don't alphabetize ISA extension IDs" failed to apply to 6.1-stable tree

by gregkh＠linuxfoundation.org

The patch below does not apply to the 6.1-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <stable(a)vger.kernel.org>. To reproduce the conflict and resubmit, you may use the following commands: git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y git checkout FETCH_HEAD git cherry-pick -x dac8bf14bb49aecd1de99ebb5498fa03152f2d40 # <resolve conflicts, build, test, etc.> git commit -s git send-email --to '<stable(a)vger.kernel.org>' --in-reply-to '1678204814209236(a)kroah.com' --subject-prefix 'PATCH 6.1.y' HEAD^.. Possible dependencies: dac8bf14bb49 ("riscv: hwcap: Don't alphabetize ISA extension IDs") 9daca9a5b9ac ("Merge patch series "riscv: improve boot time isa extensions handling"") thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From dac8bf14bb49aecd1de99ebb5498fa03152f2d40 Mon Sep 17 00:00:00 2001 From: Andrew Jones <ajones(a)ventanamicro.com> Date: Thu, 9 Feb 2023 13:36:36 +0100 Subject: [PATCH] riscv: hwcap: Don't alphabetize ISA extension IDs While the comment above the ISA extension ID definitions says "Entries are sorted alphabetically.", this stopped being good advice with commit d8a3d8a75206 ("riscv: hwcap: make ISA extension ids can be used in asm"), as we now use macros instead of enums. Reshuffling defines is error-prone, so, since they don't need to be in any particular order, change the advice to just adding new extensions at the bottom. Also, take the opportunity to change spaces to tabs, merge three comments into one, and move the base and max defines into more logical locations wrt the ID definitions. Signed-off-by: Andrew Jones <ajones(a)ventanamicro.com> Reviewed-by: Conor Dooley <conor.dooley(a)microchip.com> Link: https://lore.kernel.org/r/20230209123636.123537-1-ajones@ventanamicro.com Cc: stable(a)vger.kernel.org Signed-off-by: Palmer Dabbelt <palmer(a)rivosinc.com> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h index 831bebacb7fb..8f3994a7f0ca 100644 --- a/arch/riscv/include/asm/hwcap.h +++ b/arch/riscv/include/asm/hwcap.h @@ -24,29 +24,27 @@ #define RISCV_ISA_EXT_u ('u' - 'a') /* - * Increse this to higher value as kernel support more ISA extensions. - */ -#define RISCV_ISA_EXT_MAX 64 -#define RISCV_ISA_EXT_NAME_LEN_MAX 32 - -/* The base ID for multi-letter ISA extensions */ -#define RISCV_ISA_EXT_BASE 26 - -/* - * These macros represent the logical ID for each multi-letter RISC-V ISA extension. - * The logical ID should start from RISCV_ISA_EXT_BASE and must not exceed - * RISCV_ISA_EXT_MAX. 0-25 range is reserved for single letter - * extensions while all the multi-letter extensions should define the next - * available logical extension id. - * Entries are sorted alphabetically. + * These macros represent the logical IDs of each multi-letter RISC-V ISA + * extension and are used in the ISA bitmap. The logical IDs start from + * RISCV_ISA_EXT_BASE, which allows the 0-25 range to be reserved for single + * letter extensions. The maximum, RISCV_ISA_EXT_MAX, is defined in order + * to allocate the bitmap and may be increased when necessary. + * + * New extensions should just be added to the bottom, rather than added + * alphabetically, in order to avoid unnecessary shuffling. */ -#define RISCV_ISA_EXT_SSCOFPMF 26 -#define RISCV_ISA_EXT_SSTC 27 -#define RISCV_ISA_EXT_SVINVAL 28 -#define RISCV_ISA_EXT_SVPBMT 29 -#define RISCV_ISA_EXT_ZBB 30 -#define RISCV_ISA_EXT_ZICBOM 31 -#define RISCV_ISA_EXT_ZIHINTPAUSE 32 +#define RISCV_ISA_EXT_BASE 26 + +#define RISCV_ISA_EXT_SSCOFPMF 26 +#define RISCV_ISA_EXT_SSTC 27 +#define RISCV_ISA_EXT_SVINVAL 28 +#define RISCV_ISA_EXT_SVPBMT 29 +#define RISCV_ISA_EXT_ZBB 30 +#define RISCV_ISA_EXT_ZICBOM 31 +#define RISCV_ISA_EXT_ZIHINTPAUSE 32 + +#define RISCV_ISA_EXT_MAX 64 +#define RISCV_ISA_EXT_NAME_LEN_MAX 32 #ifndef __ASSEMBLY__

2 years, 8 months

1
0
0 0

FAILED: patch "[PATCH] riscv: Add header include guards to insn.h" failed to apply to 6.2-stable tree

by gregkh＠linuxfoundation.org

The patch below does not apply to the 6.2-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <stable(a)vger.kernel.org>. To reproduce the conflict and resubmit, you may use the following commands: git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.2.y git checkout FETCH_HEAD git cherry-pick -x 8ac6e619d9d51b3eb5bae817db8aa94e780a0db4 # <resolve conflicts, build, test, etc.> git commit -s git send-email --to '<stable(a)vger.kernel.org>' --in-reply-to '1678204801201186(a)kroah.com' --subject-prefix 'PATCH 6.2.y' HEAD^.. Possible dependencies: 8ac6e619d9d5 ("riscv: Add header include guards to insn.h") 47f05757d3d8 ("RISC-V: add helpers for handling immediates in U-type and I-type pairs") c9c1af3f186a ("RISC-V: rename parse_asm.h to insn.h") ec5f90877516 ("RISC-V: Move riscv_insn_is_* macros into a common header") bf0cc402d7cd ("RISC-V: add prefix to all constants/macros in parse_asm.h") a3775634f6da ("RISC-V: fix funct4 definition for c.jalr in parse_asm.h") thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From 8ac6e619d9d51b3eb5bae817db8aa94e780a0db4 Mon Sep 17 00:00:00 2001 From: Liao Chang <liaochang1(a)huawei.com> Date: Sun, 29 Jan 2023 17:42:42 +0800 Subject: [PATCH] riscv: Add header include guards to insn.h Add header include guards to insn.h to prevent repeating declaration of any identifiers in insn.h. Fixes: edde5584c7ab ("riscv: Add SW single-step support for KDB") Signed-off-by: Liao Chang <liaochang1(a)huawei.com> Reviewed-by: Andrew Jones <ajones(a)ventanamicro.com> Fixes: c9c1af3f186a ("RISC-V: rename parse_asm.h to insn.h") Reviewed-by: Conor Dooley <conor.dooley(a)microchip.com> Link: https://lore.kernel.org/r/20230129094242.282620-1-liaochang1@huawei.com Cc: stable(a)vger.kernel.org Signed-off-by: Palmer Dabbelt <palmer(a)rivosinc.com> diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h index 6567cd5ed6ba..8d5c84f2d5ef 100644 --- a/arch/riscv/include/asm/insn.h +++ b/arch/riscv/include/asm/insn.h @@ -3,6 +3,9 @@ * Copyright (C) 2020 SiFive */ +#ifndef _ASM_RISCV_INSN_H +#define _ASM_RISCV_INSN_H + #include <linux/bits.h> #define RV_INSN_FUNCT3_MASK GENMASK(14, 12) @@ -375,3 +378,4 @@ static inline void riscv_insn_insert_utype_itype_imm(u32 *utype_insn, u32 *itype *utype_insn |= (imm & RV_U_IMM_31_12_MASK) + ((imm & BIT(11)) << 1); *itype_insn |= ((imm & RV_I_IMM_11_0_MASK) << RV_I_IMM_11_0_OPOFF); } +#endif /* _ASM_RISCV_INSN_H */

2 years, 8 months

1
0
0 0

FAILED: patch "[PATCH] riscv: Add header include guards to insn.h" failed to apply to 6.1-stable tree

by gregkh＠linuxfoundation.org

The patch below does not apply to the 6.1-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <stable(a)vger.kernel.org>. To reproduce the conflict and resubmit, you may use the following commands: git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y git checkout FETCH_HEAD git cherry-pick -x 8ac6e619d9d51b3eb5bae817db8aa94e780a0db4 # <resolve conflicts, build, test, etc.> git commit -s git send-email --to '<stable(a)vger.kernel.org>' --in-reply-to '16782048001545(a)kroah.com' --subject-prefix 'PATCH 6.1.y' HEAD^.. Possible dependencies: 8ac6e619d9d5 ("riscv: Add header include guards to insn.h") 47f05757d3d8 ("RISC-V: add helpers for handling immediates in U-type and I-type pairs") c9c1af3f186a ("RISC-V: rename parse_asm.h to insn.h") ec5f90877516 ("RISC-V: Move riscv_insn_is_* macros into a common header") bf0cc402d7cd ("RISC-V: add prefix to all constants/macros in parse_asm.h") a3775634f6da ("RISC-V: fix funct4 definition for c.jalr in parse_asm.h") thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From 8ac6e619d9d51b3eb5bae817db8aa94e780a0db4 Mon Sep 17 00:00:00 2001 From: Liao Chang <liaochang1(a)huawei.com> Date: Sun, 29 Jan 2023 17:42:42 +0800 Subject: [PATCH] riscv: Add header include guards to insn.h Add header include guards to insn.h to prevent repeating declaration of any identifiers in insn.h. Fixes: edde5584c7ab ("riscv: Add SW single-step support for KDB") Signed-off-by: Liao Chang <liaochang1(a)huawei.com> Reviewed-by: Andrew Jones <ajones(a)ventanamicro.com> Fixes: c9c1af3f186a ("RISC-V: rename parse_asm.h to insn.h") Reviewed-by: Conor Dooley <conor.dooley(a)microchip.com> Link: https://lore.kernel.org/r/20230129094242.282620-1-liaochang1@huawei.com Cc: stable(a)vger.kernel.org Signed-off-by: Palmer Dabbelt <palmer(a)rivosinc.com> diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h index 6567cd5ed6ba..8d5c84f2d5ef 100644 --- a/arch/riscv/include/asm/insn.h +++ b/arch/riscv/include/asm/insn.h @@ -3,6 +3,9 @@ * Copyright (C) 2020 SiFive */ +#ifndef _ASM_RISCV_INSN_H +#define _ASM_RISCV_INSN_H + #include <linux/bits.h> #define RV_INSN_FUNCT3_MASK GENMASK(14, 12) @@ -375,3 +378,4 @@ static inline void riscv_insn_insert_utype_itype_imm(u32 *utype_insn, u32 *itype *utype_insn |= (imm & RV_U_IMM_31_12_MASK) + ((imm & BIT(11)) << 1); *itype_insn |= ((imm & RV_I_IMM_11_0_MASK) << RV_I_IMM_11_0_OPOFF); } +#endif /* _ASM_RISCV_INSN_H */

2 years, 8 months

1
0
0 0

FAILED: patch "[PATCH] riscv: ftrace: Reduce the detour code size to half" failed to apply to 4.19-stable tree

by gregkh＠linuxfoundation.org

The patch below does not apply to the 4.19-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <stable(a)vger.kernel.org>. To reproduce the conflict and resubmit, you may use the following commands: git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-4.19.y git checkout FETCH_HEAD git cherry-pick -x 6724a76cff85ee271bbbff42ac527e4643b2ec52 # <resolve conflicts, build, test, etc.> git commit -s git send-email --to '<stable(a)vger.kernel.org>' --in-reply-to '1678204787214241(a)kroah.com' --subject-prefix 'PATCH 4.19.y' HEAD^.. Possible dependencies: 6724a76cff85 ("riscv: ftrace: Reduce the detour code size to half") 409c8fb20c66 ("riscv: ftrace: Remove wasted nops for !RISCV_ISA_C") afc76b8b8011 ("riscv: Using PATCHABLE_FUNCTION_ENTRY instead of MCOUNT") 66d18dbda846 ("RISC-V: Take text_mutex in ftrace_init_nop()") thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From 6724a76cff85ee271bbbff42ac527e4643b2ec52 Mon Sep 17 00:00:00 2001 From: Guo Ren <guoren(a)kernel.org> Date: Thu, 12 Jan 2023 04:05:59 -0500 Subject: [PATCH] riscv: ftrace: Reduce the detour code size to half Use a temporary register to reduce the size of detour code from 16 bytes to 8 bytes. The previous implementation is from 'commit afc76b8b8011 ("riscv: Using PATCHABLE_FUNCTION_ENTRY instead of MCOUNT")'. Before the patch: <func_prolog>: 0: REG_S ra, -SZREG(sp) 4: auipc ra, ? 8: jalr ?(ra) 12: REG_L ra, -SZREG(sp) (func_boddy) After the patch: <func_prolog>: 0: auipc t0, ? 4: jalr t0, ?(t0) (func_boddy) This patch not just reduces the size of detour code, but also fixes an important issue: An Ftrace callback registered with FTRACE_OPS_FL_IPMODIFY flag can actually change the instruction pointer, e.g. to "replace" the given kernel function with a new one, which is needed for livepatching, etc. In this case, the trampoline (ftrace_regs_caller) would not return to <func_prolog+12> but would rather jump to the new function. So, "REG_L ra, -SZREG(sp)" would not run and the original return address would not be restored. The kernel is likely to hang or crash as a result. This can be easily demonstrated if one tries to "replace", say, cmdline_proc_show() with a new function with the same signature using instruction_pointer_set(&fregs->regs, new_func_addr) in the Ftrace callback. Link: https://lore.kernel.org/linux-riscv/20221122075440.1165172-1-suagrfillet@gm… Link: https://lore.kernel.org/linux-riscv/d7d5730b-ebef-68e5-5046-e763e1ee6164@ya… Co-developed-by: Song Shuai <suagrfillet(a)gmail.com> Signed-off-by: Song Shuai <suagrfillet(a)gmail.com> Signed-off-by: Guo Ren <guoren(a)linux.alibaba.com> Signed-off-by: Guo Ren <guoren(a)kernel.org> Cc: Evgenii Shatokhin <e.shatokhin(a)yadro.com> Reviewed-by: Evgenii Shatokhin <e.shatokhin(a)yadro.com> Link: https://lore.kernel.org/r/20230112090603.1295340-4-guoren@kernel.org Cc: stable(a)vger.kernel.org Fixes: 10626c32e382 ("riscv/ftrace: Add basic support") Signed-off-by: Palmer Dabbelt <palmer(a)rivosinc.com> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile index e194916cb5a2..ab50724cc554 100644 --- a/arch/riscv/Makefile +++ b/arch/riscv/Makefile @@ -12,9 +12,9 @@ ifeq ($(CONFIG_DYNAMIC_FTRACE),y) LDFLAGS_vmlinux := --no-relax KBUILD_CPPFLAGS += -DCC_USING_PATCHABLE_FUNCTION_ENTRY ifeq ($(CONFIG_RISCV_ISA_C),y) - CC_FLAGS_FTRACE := -fpatchable-function-entry=8 -else CC_FLAGS_FTRACE := -fpatchable-function-entry=4 +else + CC_FLAGS_FTRACE := -fpatchable-function-entry=2 endif endif diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h index 04dad3380041..9e73922e1e2e 100644 --- a/arch/riscv/include/asm/ftrace.h +++ b/arch/riscv/include/asm/ftrace.h @@ -42,6 +42,14 @@ struct dyn_arch_ftrace { * 2) jalr: setting low-12 offset to ra, jump to ra, and set ra to * return address (original pc + 4) * + *<ftrace enable>: + * 0: auipc t0/ra, 0x? + * 4: jalr t0/ra, ?(t0/ra) + * + *<ftrace disable>: + * 0: nop + * 4: nop + * * Dynamic ftrace generates probes to call sites, so we must deal with * both auipc and jalr at the same time. */ @@ -52,25 +60,43 @@ struct dyn_arch_ftrace { #define AUIPC_OFFSET_MASK (0xfffff000) #define AUIPC_PAD (0x00001000) #define JALR_SHIFT 20 -#define JALR_BASIC (0x000080e7) -#define AUIPC_BASIC (0x00000097) +#define JALR_RA (0x000080e7) +#define AUIPC_RA (0x00000097) +#define JALR_T0 (0x000282e7) +#define AUIPC_T0 (0x00000297) #define NOP4 (0x00000013) -#define make_call(caller, callee, call) \ +#define to_jalr_t0(offset) \ + (((offset & JALR_OFFSET_MASK) << JALR_SHIFT) | JALR_T0) + +#define to_auipc_t0(offset) \ + ((offset & JALR_SIGN_MASK) ? \ + (((offset & AUIPC_OFFSET_MASK) + AUIPC_PAD) | AUIPC_T0) : \ + ((offset & AUIPC_OFFSET_MASK) | AUIPC_T0)) + +#define make_call_t0(caller, callee, call) \ do { \ - call[0] = to_auipc_insn((unsigned int)((unsigned long)callee - \ - (unsigned long)caller)); \ - call[1] = to_jalr_insn((unsigned int)((unsigned long)callee - \ - (unsigned long)caller)); \ + unsigned int offset = \ + (unsigned long) callee - (unsigned long) caller; \ + call[0] = to_auipc_t0(offset); \ + call[1] = to_jalr_t0(offset); \ } while (0) -#define to_jalr_insn(offset) \ - (((offset & JALR_OFFSET_MASK) << JALR_SHIFT) | JALR_BASIC) +#define to_jalr_ra(offset) \ + (((offset & JALR_OFFSET_MASK) << JALR_SHIFT) | JALR_RA) -#define to_auipc_insn(offset) \ +#define to_auipc_ra(offset) \ ((offset & JALR_SIGN_MASK) ? \ - (((offset & AUIPC_OFFSET_MASK) + AUIPC_PAD) | AUIPC_BASIC) : \ - ((offset & AUIPC_OFFSET_MASK) | AUIPC_BASIC)) + (((offset & AUIPC_OFFSET_MASK) + AUIPC_PAD) | AUIPC_RA) : \ + ((offset & AUIPC_OFFSET_MASK) | AUIPC_RA)) + +#define make_call_ra(caller, callee, call) \ +do { \ + unsigned int offset = \ + (unsigned long) callee - (unsigned long) caller; \ + call[0] = to_auipc_ra(offset); \ + call[1] = to_jalr_ra(offset); \ +} while (0) /* * Let auipc+jalr be the basic *mcount unit*, so we make it 8 bytes here. diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c index 2086f6585773..5bff37af4770 100644 --- a/arch/riscv/kernel/ftrace.c +++ b/arch/riscv/kernel/ftrace.c @@ -55,12 +55,15 @@ static int ftrace_check_current_call(unsigned long hook_pos, } static int __ftrace_modify_call(unsigned long hook_pos, unsigned long target, - bool enable) + bool enable, bool ra) { unsigned int call[2]; unsigned int nops[2] = {NOP4, NOP4}; - make_call(hook_pos, target, call); + if (ra) + make_call_ra(hook_pos, target, call); + else + make_call_t0(hook_pos, target, call); /* Replace the auipc-jalr pair at once. Return -EPERM on write error. */ if (patch_text_nosync @@ -70,42 +73,13 @@ static int __ftrace_modify_call(unsigned long hook_pos, unsigned long target, return 0; } -/* - * Put 5 instructions with 16 bytes at the front of function within - * patchable function entry nops' area. - * - * 0: REG_S ra, -SZREG(sp) - * 1: auipc ra, 0x? - * 2: jalr -?(ra) - * 3: REG_L ra, -SZREG(sp) - * - * So the opcodes is: - * 0: 0xfe113c23 (sd)/0xfe112e23 (sw) - * 1: 0x???????? -> auipc - * 2: 0x???????? -> jalr - * 3: 0xff813083 (ld)/0xffc12083 (lw) - */ -#if __riscv_xlen == 64 -#define INSN0 0xfe113c23 -#define INSN3 0xff813083 -#elif __riscv_xlen == 32 -#define INSN0 0xfe112e23 -#define INSN3 0xffc12083 -#endif - -#define FUNC_ENTRY_SIZE 16 -#define FUNC_ENTRY_JMP 4 - int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr) { - unsigned int call[4] = {INSN0, 0, 0, INSN3}; - unsigned long target = addr; - unsigned long caller = rec->ip + FUNC_ENTRY_JMP; + unsigned int call[2]; - call[1] = to_auipc_insn((unsigned int)(target - caller)); - call[2] = to_jalr_insn((unsigned int)(target - caller)); + make_call_t0(rec->ip, addr, call); - if (patch_text_nosync((void *)rec->ip, call, FUNC_ENTRY_SIZE)) + if (patch_text_nosync((void *)rec->ip, call, MCOUNT_INSN_SIZE)) return -EPERM; return 0; @@ -114,15 +88,14 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr) int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec, unsigned long addr) { - unsigned int nops[4] = {NOP4, NOP4, NOP4, NOP4}; + unsigned int nops[2] = {NOP4, NOP4}; - if (patch_text_nosync((void *)rec->ip, nops, FUNC_ENTRY_SIZE)) + if (patch_text_nosync((void *)rec->ip, nops, MCOUNT_INSN_SIZE)) return -EPERM; return 0; } - /* * This is called early on, and isn't wrapped by * ftrace_arch_code_modify_{prepare,post_process}() and therefor doesn't hold @@ -144,10 +117,10 @@ int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec) int ftrace_update_ftrace_func(ftrace_func_t func) { int ret = __ftrace_modify_call((unsigned long)&ftrace_call, - (unsigned long)func, true); + (unsigned long)func, true, true); if (!ret) { ret = __ftrace_modify_call((unsigned long)&ftrace_regs_call, - (unsigned long)func, true); + (unsigned long)func, true, true); } return ret; @@ -159,16 +132,16 @@ int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr, unsigned long addr) { unsigned int call[2]; - unsigned long caller = rec->ip + FUNC_ENTRY_JMP; + unsigned long caller = rec->ip; int ret; - make_call(caller, old_addr, call); + make_call_t0(caller, old_addr, call); ret = ftrace_check_current_call(caller, call); if (ret) return ret; - return __ftrace_modify_call(caller, addr, true); + return __ftrace_modify_call(caller, addr, true, false); } #endif @@ -203,12 +176,12 @@ int ftrace_enable_ftrace_graph_caller(void) int ret; ret = __ftrace_modify_call((unsigned long)&ftrace_graph_call, - (unsigned long)&prepare_ftrace_return, true); + (unsigned long)&prepare_ftrace_return, true, true); if (ret) return ret; return __ftrace_modify_call((unsigned long)&ftrace_graph_regs_call, - (unsigned long)&prepare_ftrace_return, true); + (unsigned long)&prepare_ftrace_return, true, true); } int ftrace_disable_ftrace_graph_caller(void) @@ -216,12 +189,12 @@ int ftrace_disable_ftrace_graph_caller(void) int ret; ret = __ftrace_modify_call((unsigned long)&ftrace_graph_call, - (unsigned long)&prepare_ftrace_return, false); + (unsigned long)&prepare_ftrace_return, false, true); if (ret) return ret; return __ftrace_modify_call((unsigned long)&ftrace_graph_regs_call, - (unsigned long)&prepare_ftrace_return, false); + (unsigned long)&prepare_ftrace_return, false, true); } #endif /* CONFIG_DYNAMIC_FTRACE */ #endif /* CONFIG_FUNCTION_GRAPH_TRACER */ diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S index d171eca623b6..125de818d1ba 100644 --- a/arch/riscv/kernel/mcount-dyn.S +++ b/arch/riscv/kernel/mcount-dyn.S @@ -13,8 +13,8 @@ .text -#define FENTRY_RA_OFFSET 12 -#define ABI_SIZE_ON_STACK 72 +#define FENTRY_RA_OFFSET 8 +#define ABI_SIZE_ON_STACK 80 #define ABI_A0 0 #define ABI_A1 8 #define ABI_A2 16 @@ -23,10 +23,10 @@ #define ABI_A5 40 #define ABI_A6 48 #define ABI_A7 56 -#define ABI_RA 64 +#define ABI_T0 64 +#define ABI_RA 72 .macro SAVE_ABI - addi sp, sp, -SZREG addi sp, sp, -ABI_SIZE_ON_STACK REG_S a0, ABI_A0(sp) @@ -37,6 +37,7 @@ REG_S a5, ABI_A5(sp) REG_S a6, ABI_A6(sp) REG_S a7, ABI_A7(sp) + REG_S t0, ABI_T0(sp) REG_S ra, ABI_RA(sp) .endm @@ -49,24 +50,18 @@ REG_L a5, ABI_A5(sp) REG_L a6, ABI_A6(sp) REG_L a7, ABI_A7(sp) + REG_L t0, ABI_T0(sp) REG_L ra, ABI_RA(sp) addi sp, sp, ABI_SIZE_ON_STACK - addi sp, sp, SZREG .endm #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS .macro SAVE_ALL - addi sp, sp, -SZREG addi sp, sp, -PT_SIZE_ON_STACK - REG_S x1, PT_EPC(sp) - addi sp, sp, PT_SIZE_ON_STACK - REG_L x1, (sp) - addi sp, sp, -PT_SIZE_ON_STACK + REG_S t0, PT_EPC(sp) REG_S x1, PT_RA(sp) - REG_L x1, PT_EPC(sp) - REG_S x2, PT_SP(sp) REG_S x3, PT_GP(sp) REG_S x4, PT_TP(sp) @@ -100,15 +95,11 @@ .endm .macro RESTORE_ALL + REG_L t0, PT_EPC(sp) REG_L x1, PT_RA(sp) - addi sp, sp, PT_SIZE_ON_STACK - REG_S x1, (sp) - addi sp, sp, -PT_SIZE_ON_STACK - REG_L x1, PT_EPC(sp) REG_L x2, PT_SP(sp) REG_L x3, PT_GP(sp) REG_L x4, PT_TP(sp) - REG_L x5, PT_T0(sp) REG_L x6, PT_T1(sp) REG_L x7, PT_T2(sp) REG_L x8, PT_S0(sp) @@ -137,17 +128,16 @@ REG_L x31, PT_T6(sp) addi sp, sp, PT_SIZE_ON_STACK - addi sp, sp, SZREG .endm #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ ENTRY(ftrace_caller) SAVE_ABI - addi a0, ra, -FENTRY_RA_OFFSET + addi a0, t0, -FENTRY_RA_OFFSET la a1, function_trace_op REG_L a2, 0(a1) - REG_L a1, ABI_SIZE_ON_STACK(sp) + mv a1, ra mv a3, sp ftrace_call: @@ -155,8 +145,8 @@ ftrace_call: call ftrace_stub #ifdef CONFIG_FUNCTION_GRAPH_TRACER - addi a0, sp, ABI_SIZE_ON_STACK - REG_L a1, ABI_RA(sp) + addi a0, sp, ABI_RA + REG_L a1, ABI_T0(sp) addi a1, a1, -FENTRY_RA_OFFSET #ifdef HAVE_FUNCTION_GRAPH_FP_TEST mv a2, s0 @@ -166,17 +156,17 @@ ftrace_graph_call: call ftrace_stub #endif RESTORE_ABI - ret + jr t0 ENDPROC(ftrace_caller) #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS ENTRY(ftrace_regs_caller) SAVE_ALL - addi a0, ra, -FENTRY_RA_OFFSET + addi a0, t0, -FENTRY_RA_OFFSET la a1, function_trace_op REG_L a2, 0(a1) - REG_L a1, PT_SIZE_ON_STACK(sp) + mv a1, ra mv a3, sp ftrace_regs_call: @@ -196,6 +186,6 @@ ftrace_graph_regs_call: #endif RESTORE_ALL - ret + jr t0 ENDPROC(ftrace_regs_caller) #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */

2 years, 8 months

1
0
0 0

FAILED: patch "[PATCH] riscv: ftrace: Reduce the detour code size to half" failed to apply to 5.4-stable tree

by gregkh＠linuxfoundation.org

The patch below does not apply to the 5.4-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <stable(a)vger.kernel.org>. To reproduce the conflict and resubmit, you may use the following commands: git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-5.4.y git checkout FETCH_HEAD git cherry-pick -x 6724a76cff85ee271bbbff42ac527e4643b2ec52 # <resolve conflicts, build, test, etc.> git commit -s git send-email --to '<stable(a)vger.kernel.org>' --in-reply-to '16782047862152(a)kroah.com' --subject-prefix 'PATCH 5.4.y' HEAD^.. Possible dependencies: 6724a76cff85 ("riscv: ftrace: Reduce the detour code size to half") 409c8fb20c66 ("riscv: ftrace: Remove wasted nops for !RISCV_ISA_C") afc76b8b8011 ("riscv: Using PATCHABLE_FUNCTION_ENTRY instead of MCOUNT") 66d18dbda846 ("RISC-V: Take text_mutex in ftrace_init_nop()") thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From 6724a76cff85ee271bbbff42ac527e4643b2ec52 Mon Sep 17 00:00:00 2001 From: Guo Ren <guoren(a)kernel.org> Date: Thu, 12 Jan 2023 04:05:59 -0500 Subject: [PATCH] riscv: ftrace: Reduce the detour code size to half Use a temporary register to reduce the size of detour code from 16 bytes to 8 bytes. The previous implementation is from 'commit afc76b8b8011 ("riscv: Using PATCHABLE_FUNCTION_ENTRY instead of MCOUNT")'. Before the patch: <func_prolog>: 0: REG_S ra, -SZREG(sp) 4: auipc ra, ? 8: jalr ?(ra) 12: REG_L ra, -SZREG(sp) (func_boddy) After the patch: <func_prolog>: 0: auipc t0, ? 4: jalr t0, ?(t0) (func_boddy) This patch not just reduces the size of detour code, but also fixes an important issue: An Ftrace callback registered with FTRACE_OPS_FL_IPMODIFY flag can actually change the instruction pointer, e.g. to "replace" the given kernel function with a new one, which is needed for livepatching, etc. In this case, the trampoline (ftrace_regs_caller) would not return to <func_prolog+12> but would rather jump to the new function. So, "REG_L ra, -SZREG(sp)" would not run and the original return address would not be restored. The kernel is likely to hang or crash as a result. This can be easily demonstrated if one tries to "replace", say, cmdline_proc_show() with a new function with the same signature using instruction_pointer_set(&fregs->regs, new_func_addr) in the Ftrace callback. Link: https://lore.kernel.org/linux-riscv/20221122075440.1165172-1-suagrfillet@gm… Link: https://lore.kernel.org/linux-riscv/d7d5730b-ebef-68e5-5046-e763e1ee6164@ya… Co-developed-by: Song Shuai <suagrfillet(a)gmail.com> Signed-off-by: Song Shuai <suagrfillet(a)gmail.com> Signed-off-by: Guo Ren <guoren(a)linux.alibaba.com> Signed-off-by: Guo Ren <guoren(a)kernel.org> Cc: Evgenii Shatokhin <e.shatokhin(a)yadro.com> Reviewed-by: Evgenii Shatokhin <e.shatokhin(a)yadro.com> Link: https://lore.kernel.org/r/20230112090603.1295340-4-guoren@kernel.org Cc: stable(a)vger.kernel.org Fixes: 10626c32e382 ("riscv/ftrace: Add basic support") Signed-off-by: Palmer Dabbelt <palmer(a)rivosinc.com> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile index e194916cb5a2..ab50724cc554 100644 --- a/arch/riscv/Makefile +++ b/arch/riscv/Makefile @@ -12,9 +12,9 @@ ifeq ($(CONFIG_DYNAMIC_FTRACE),y) LDFLAGS_vmlinux := --no-relax KBUILD_CPPFLAGS += -DCC_USING_PATCHABLE_FUNCTION_ENTRY ifeq ($(CONFIG_RISCV_ISA_C),y) - CC_FLAGS_FTRACE := -fpatchable-function-entry=8 -else CC_FLAGS_FTRACE := -fpatchable-function-entry=4 +else + CC_FLAGS_FTRACE := -fpatchable-function-entry=2 endif endif diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h index 04dad3380041..9e73922e1e2e 100644 --- a/arch/riscv/include/asm/ftrace.h +++ b/arch/riscv/include/asm/ftrace.h @@ -42,6 +42,14 @@ struct dyn_arch_ftrace { * 2) jalr: setting low-12 offset to ra, jump to ra, and set ra to * return address (original pc + 4) * + *<ftrace enable>: + * 0: auipc t0/ra, 0x? + * 4: jalr t0/ra, ?(t0/ra) + * + *<ftrace disable>: + * 0: nop + * 4: nop + * * Dynamic ftrace generates probes to call sites, so we must deal with * both auipc and jalr at the same time. */ @@ -52,25 +60,43 @@ struct dyn_arch_ftrace { #define AUIPC_OFFSET_MASK (0xfffff000) #define AUIPC_PAD (0x00001000) #define JALR_SHIFT 20 -#define JALR_BASIC (0x000080e7) -#define AUIPC_BASIC (0x00000097) +#define JALR_RA (0x000080e7) +#define AUIPC_RA (0x00000097) +#define JALR_T0 (0x000282e7) +#define AUIPC_T0 (0x00000297) #define NOP4 (0x00000013) -#define make_call(caller, callee, call) \ +#define to_jalr_t0(offset) \ + (((offset & JALR_OFFSET_MASK) << JALR_SHIFT) | JALR_T0) + +#define to_auipc_t0(offset) \ + ((offset & JALR_SIGN_MASK) ? \ + (((offset & AUIPC_OFFSET_MASK) + AUIPC_PAD) | AUIPC_T0) : \ + ((offset & AUIPC_OFFSET_MASK) | AUIPC_T0)) + +#define make_call_t0(caller, callee, call) \ do { \ - call[0] = to_auipc_insn((unsigned int)((unsigned long)callee - \ - (unsigned long)caller)); \ - call[1] = to_jalr_insn((unsigned int)((unsigned long)callee - \ - (unsigned long)caller)); \ + unsigned int offset = \ + (unsigned long) callee - (unsigned long) caller; \ + call[0] = to_auipc_t0(offset); \ + call[1] = to_jalr_t0(offset); \ } while (0) -#define to_jalr_insn(offset) \ - (((offset & JALR_OFFSET_MASK) << JALR_SHIFT) | JALR_BASIC) +#define to_jalr_ra(offset) \ + (((offset & JALR_OFFSET_MASK) << JALR_SHIFT) | JALR_RA) -#define to_auipc_insn(offset) \ +#define to_auipc_ra(offset) \ ((offset & JALR_SIGN_MASK) ? \ - (((offset & AUIPC_OFFSET_MASK) + AUIPC_PAD) | AUIPC_BASIC) : \ - ((offset & AUIPC_OFFSET_MASK) | AUIPC_BASIC)) + (((offset & AUIPC_OFFSET_MASK) + AUIPC_PAD) | AUIPC_RA) : \ + ((offset & AUIPC_OFFSET_MASK) | AUIPC_RA)) + +#define make_call_ra(caller, callee, call) \ +do { \ + unsigned int offset = \ + (unsigned long) callee - (unsigned long) caller; \ + call[0] = to_auipc_ra(offset); \ + call[1] = to_jalr_ra(offset); \ +} while (0) /* * Let auipc+jalr be the basic *mcount unit*, so we make it 8 bytes here. diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c index 2086f6585773..5bff37af4770 100644 --- a/arch/riscv/kernel/ftrace.c +++ b/arch/riscv/kernel/ftrace.c @@ -55,12 +55,15 @@ static int ftrace_check_current_call(unsigned long hook_pos, } static int __ftrace_modify_call(unsigned long hook_pos, unsigned long target, - bool enable) + bool enable, bool ra) { unsigned int call[2]; unsigned int nops[2] = {NOP4, NOP4}; - make_call(hook_pos, target, call); + if (ra) + make_call_ra(hook_pos, target, call); + else + make_call_t0(hook_pos, target, call); /* Replace the auipc-jalr pair at once. Return -EPERM on write error. */ if (patch_text_nosync @@ -70,42 +73,13 @@ static int __ftrace_modify_call(unsigned long hook_pos, unsigned long target, return 0; } -/* - * Put 5 instructions with 16 bytes at the front of function within - * patchable function entry nops' area. - * - * 0: REG_S ra, -SZREG(sp) - * 1: auipc ra, 0x? - * 2: jalr -?(ra) - * 3: REG_L ra, -SZREG(sp) - * - * So the opcodes is: - * 0: 0xfe113c23 (sd)/0xfe112e23 (sw) - * 1: 0x???????? -> auipc - * 2: 0x???????? -> jalr - * 3: 0xff813083 (ld)/0xffc12083 (lw) - */ -#if __riscv_xlen == 64 -#define INSN0 0xfe113c23 -#define INSN3 0xff813083 -#elif __riscv_xlen == 32 -#define INSN0 0xfe112e23 -#define INSN3 0xffc12083 -#endif - -#define FUNC_ENTRY_SIZE 16 -#define FUNC_ENTRY_JMP 4 - int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr) { - unsigned int call[4] = {INSN0, 0, 0, INSN3}; - unsigned long target = addr; - unsigned long caller = rec->ip + FUNC_ENTRY_JMP; + unsigned int call[2]; - call[1] = to_auipc_insn((unsigned int)(target - caller)); - call[2] = to_jalr_insn((unsigned int)(target - caller)); + make_call_t0(rec->ip, addr, call); - if (patch_text_nosync((void *)rec->ip, call, FUNC_ENTRY_SIZE)) + if (patch_text_nosync((void *)rec->ip, call, MCOUNT_INSN_SIZE)) return -EPERM; return 0; @@ -114,15 +88,14 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr) int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec, unsigned long addr) { - unsigned int nops[4] = {NOP4, NOP4, NOP4, NOP4}; + unsigned int nops[2] = {NOP4, NOP4}; - if (patch_text_nosync((void *)rec->ip, nops, FUNC_ENTRY_SIZE)) + if (patch_text_nosync((void *)rec->ip, nops, MCOUNT_INSN_SIZE)) return -EPERM; return 0; } - /* * This is called early on, and isn't wrapped by * ftrace_arch_code_modify_{prepare,post_process}() and therefor doesn't hold @@ -144,10 +117,10 @@ int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec) int ftrace_update_ftrace_func(ftrace_func_t func) { int ret = __ftrace_modify_call((unsigned long)&ftrace_call, - (unsigned long)func, true); + (unsigned long)func, true, true); if (!ret) { ret = __ftrace_modify_call((unsigned long)&ftrace_regs_call, - (unsigned long)func, true); + (unsigned long)func, true, true); } return ret; @@ -159,16 +132,16 @@ int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr, unsigned long addr) { unsigned int call[2]; - unsigned long caller = rec->ip + FUNC_ENTRY_JMP; + unsigned long caller = rec->ip; int ret; - make_call(caller, old_addr, call); + make_call_t0(caller, old_addr, call); ret = ftrace_check_current_call(caller, call); if (ret) return ret; - return __ftrace_modify_call(caller, addr, true); + return __ftrace_modify_call(caller, addr, true, false); } #endif @@ -203,12 +176,12 @@ int ftrace_enable_ftrace_graph_caller(void) int ret; ret = __ftrace_modify_call((unsigned long)&ftrace_graph_call, - (unsigned long)&prepare_ftrace_return, true); + (unsigned long)&prepare_ftrace_return, true, true); if (ret) return ret; return __ftrace_modify_call((unsigned long)&ftrace_graph_regs_call, - (unsigned long)&prepare_ftrace_return, true); + (unsigned long)&prepare_ftrace_return, true, true); } int ftrace_disable_ftrace_graph_caller(void) @@ -216,12 +189,12 @@ int ftrace_disable_ftrace_graph_caller(void) int ret; ret = __ftrace_modify_call((unsigned long)&ftrace_graph_call, - (unsigned long)&prepare_ftrace_return, false); + (unsigned long)&prepare_ftrace_return, false, true); if (ret) return ret; return __ftrace_modify_call((unsigned long)&ftrace_graph_regs_call, - (unsigned long)&prepare_ftrace_return, false); + (unsigned long)&prepare_ftrace_return, false, true); } #endif /* CONFIG_DYNAMIC_FTRACE */ #endif /* CONFIG_FUNCTION_GRAPH_TRACER */ diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S index d171eca623b6..125de818d1ba 100644 --- a/arch/riscv/kernel/mcount-dyn.S +++ b/arch/riscv/kernel/mcount-dyn.S @@ -13,8 +13,8 @@ .text -#define FENTRY_RA_OFFSET 12 -#define ABI_SIZE_ON_STACK 72 +#define FENTRY_RA_OFFSET 8 +#define ABI_SIZE_ON_STACK 80 #define ABI_A0 0 #define ABI_A1 8 #define ABI_A2 16 @@ -23,10 +23,10 @@ #define ABI_A5 40 #define ABI_A6 48 #define ABI_A7 56 -#define ABI_RA 64 +#define ABI_T0 64 +#define ABI_RA 72 .macro SAVE_ABI - addi sp, sp, -SZREG addi sp, sp, -ABI_SIZE_ON_STACK REG_S a0, ABI_A0(sp) @@ -37,6 +37,7 @@ REG_S a5, ABI_A5(sp) REG_S a6, ABI_A6(sp) REG_S a7, ABI_A7(sp) + REG_S t0, ABI_T0(sp) REG_S ra, ABI_RA(sp) .endm @@ -49,24 +50,18 @@ REG_L a5, ABI_A5(sp) REG_L a6, ABI_A6(sp) REG_L a7, ABI_A7(sp) + REG_L t0, ABI_T0(sp) REG_L ra, ABI_RA(sp) addi sp, sp, ABI_SIZE_ON_STACK - addi sp, sp, SZREG .endm #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS .macro SAVE_ALL - addi sp, sp, -SZREG addi sp, sp, -PT_SIZE_ON_STACK - REG_S x1, PT_EPC(sp) - addi sp, sp, PT_SIZE_ON_STACK - REG_L x1, (sp) - addi sp, sp, -PT_SIZE_ON_STACK + REG_S t0, PT_EPC(sp) REG_S x1, PT_RA(sp) - REG_L x1, PT_EPC(sp) - REG_S x2, PT_SP(sp) REG_S x3, PT_GP(sp) REG_S x4, PT_TP(sp) @@ -100,15 +95,11 @@ .endm .macro RESTORE_ALL + REG_L t0, PT_EPC(sp) REG_L x1, PT_RA(sp) - addi sp, sp, PT_SIZE_ON_STACK - REG_S x1, (sp) - addi sp, sp, -PT_SIZE_ON_STACK - REG_L x1, PT_EPC(sp) REG_L x2, PT_SP(sp) REG_L x3, PT_GP(sp) REG_L x4, PT_TP(sp) - REG_L x5, PT_T0(sp) REG_L x6, PT_T1(sp) REG_L x7, PT_T2(sp) REG_L x8, PT_S0(sp) @@ -137,17 +128,16 @@ REG_L x31, PT_T6(sp) addi sp, sp, PT_SIZE_ON_STACK - addi sp, sp, SZREG .endm #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ ENTRY(ftrace_caller) SAVE_ABI - addi a0, ra, -FENTRY_RA_OFFSET + addi a0, t0, -FENTRY_RA_OFFSET la a1, function_trace_op REG_L a2, 0(a1) - REG_L a1, ABI_SIZE_ON_STACK(sp) + mv a1, ra mv a3, sp ftrace_call: @@ -155,8 +145,8 @@ ftrace_call: call ftrace_stub #ifdef CONFIG_FUNCTION_GRAPH_TRACER - addi a0, sp, ABI_SIZE_ON_STACK - REG_L a1, ABI_RA(sp) + addi a0, sp, ABI_RA + REG_L a1, ABI_T0(sp) addi a1, a1, -FENTRY_RA_OFFSET #ifdef HAVE_FUNCTION_GRAPH_FP_TEST mv a2, s0 @@ -166,17 +156,17 @@ ftrace_graph_call: call ftrace_stub #endif RESTORE_ABI - ret + jr t0 ENDPROC(ftrace_caller) #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS ENTRY(ftrace_regs_caller) SAVE_ALL - addi a0, ra, -FENTRY_RA_OFFSET + addi a0, t0, -FENTRY_RA_OFFSET la a1, function_trace_op REG_L a2, 0(a1) - REG_L a1, PT_SIZE_ON_STACK(sp) + mv a1, ra mv a3, sp ftrace_regs_call: @@ -196,6 +186,6 @@ ftrace_graph_regs_call: #endif RESTORE_ALL - ret + jr t0 ENDPROC(ftrace_regs_caller) #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */

2 years, 8 months

1
0
0 0

FAILED: patch "[PATCH] riscv: ftrace: Reduce the detour code size to half" failed to apply to 5.10-stable tree

by gregkh＠linuxfoundation.org

The patch below does not apply to the 5.10-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <stable(a)vger.kernel.org>. To reproduce the conflict and resubmit, you may use the following commands: git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-5.10.y git checkout FETCH_HEAD git cherry-pick -x 6724a76cff85ee271bbbff42ac527e4643b2ec52 # <resolve conflicts, build, test, etc.> git commit -s git send-email --to '<stable(a)vger.kernel.org>' --in-reply-to '1678204784145234(a)kroah.com' --subject-prefix 'PATCH 5.10.y' HEAD^.. Possible dependencies: 6724a76cff85 ("riscv: ftrace: Reduce the detour code size to half") 409c8fb20c66 ("riscv: ftrace: Remove wasted nops for !RISCV_ISA_C") afc76b8b8011 ("riscv: Using PATCHABLE_FUNCTION_ENTRY instead of MCOUNT") thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From 6724a76cff85ee271bbbff42ac527e4643b2ec52 Mon Sep 17 00:00:00 2001 From: Guo Ren <guoren(a)kernel.org> Date: Thu, 12 Jan 2023 04:05:59 -0500 Subject: [PATCH] riscv: ftrace: Reduce the detour code size to half Use a temporary register to reduce the size of detour code from 16 bytes to 8 bytes. The previous implementation is from 'commit afc76b8b8011 ("riscv: Using PATCHABLE_FUNCTION_ENTRY instead of MCOUNT")'. Before the patch: <func_prolog>: 0: REG_S ra, -SZREG(sp) 4: auipc ra, ? 8: jalr ?(ra) 12: REG_L ra, -SZREG(sp) (func_boddy) After the patch: <func_prolog>: 0: auipc t0, ? 4: jalr t0, ?(t0) (func_boddy) This patch not just reduces the size of detour code, but also fixes an important issue: An Ftrace callback registered with FTRACE_OPS_FL_IPMODIFY flag can actually change the instruction pointer, e.g. to "replace" the given kernel function with a new one, which is needed for livepatching, etc. In this case, the trampoline (ftrace_regs_caller) would not return to <func_prolog+12> but would rather jump to the new function. So, "REG_L ra, -SZREG(sp)" would not run and the original return address would not be restored. The kernel is likely to hang or crash as a result. This can be easily demonstrated if one tries to "replace", say, cmdline_proc_show() with a new function with the same signature using instruction_pointer_set(&fregs->regs, new_func_addr) in the Ftrace callback. Link: https://lore.kernel.org/linux-riscv/20221122075440.1165172-1-suagrfillet@gm… Link: https://lore.kernel.org/linux-riscv/d7d5730b-ebef-68e5-5046-e763e1ee6164@ya… Co-developed-by: Song Shuai <suagrfillet(a)gmail.com> Signed-off-by: Song Shuai <suagrfillet(a)gmail.com> Signed-off-by: Guo Ren <guoren(a)linux.alibaba.com> Signed-off-by: Guo Ren <guoren(a)kernel.org> Cc: Evgenii Shatokhin <e.shatokhin(a)yadro.com> Reviewed-by: Evgenii Shatokhin <e.shatokhin(a)yadro.com> Link: https://lore.kernel.org/r/20230112090603.1295340-4-guoren@kernel.org Cc: stable(a)vger.kernel.org Fixes: 10626c32e382 ("riscv/ftrace: Add basic support") Signed-off-by: Palmer Dabbelt <palmer(a)rivosinc.com> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile index e194916cb5a2..ab50724cc554 100644 --- a/arch/riscv/Makefile +++ b/arch/riscv/Makefile @@ -12,9 +12,9 @@ ifeq ($(CONFIG_DYNAMIC_FTRACE),y) LDFLAGS_vmlinux := --no-relax KBUILD_CPPFLAGS += -DCC_USING_PATCHABLE_FUNCTION_ENTRY ifeq ($(CONFIG_RISCV_ISA_C),y) - CC_FLAGS_FTRACE := -fpatchable-function-entry=8 -else CC_FLAGS_FTRACE := -fpatchable-function-entry=4 +else + CC_FLAGS_FTRACE := -fpatchable-function-entry=2 endif endif diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h index 04dad3380041..9e73922e1e2e 100644 --- a/arch/riscv/include/asm/ftrace.h +++ b/arch/riscv/include/asm/ftrace.h @@ -42,6 +42,14 @@ struct dyn_arch_ftrace { * 2) jalr: setting low-12 offset to ra, jump to ra, and set ra to * return address (original pc + 4) * + *<ftrace enable>: + * 0: auipc t0/ra, 0x? + * 4: jalr t0/ra, ?(t0/ra) + * + *<ftrace disable>: + * 0: nop + * 4: nop + * * Dynamic ftrace generates probes to call sites, so we must deal with * both auipc and jalr at the same time. */ @@ -52,25 +60,43 @@ struct dyn_arch_ftrace { #define AUIPC_OFFSET_MASK (0xfffff000) #define AUIPC_PAD (0x00001000) #define JALR_SHIFT 20 -#define JALR_BASIC (0x000080e7) -#define AUIPC_BASIC (0x00000097) +#define JALR_RA (0x000080e7) +#define AUIPC_RA (0x00000097) +#define JALR_T0 (0x000282e7) +#define AUIPC_T0 (0x00000297) #define NOP4 (0x00000013) -#define make_call(caller, callee, call) \ +#define to_jalr_t0(offset) \ + (((offset & JALR_OFFSET_MASK) << JALR_SHIFT) | JALR_T0) + +#define to_auipc_t0(offset) \ + ((offset & JALR_SIGN_MASK) ? \ + (((offset & AUIPC_OFFSET_MASK) + AUIPC_PAD) | AUIPC_T0) : \ + ((offset & AUIPC_OFFSET_MASK) | AUIPC_T0)) + +#define make_call_t0(caller, callee, call) \ do { \ - call[0] = to_auipc_insn((unsigned int)((unsigned long)callee - \ - (unsigned long)caller)); \ - call[1] = to_jalr_insn((unsigned int)((unsigned long)callee - \ - (unsigned long)caller)); \ + unsigned int offset = \ + (unsigned long) callee - (unsigned long) caller; \ + call[0] = to_auipc_t0(offset); \ + call[1] = to_jalr_t0(offset); \ } while (0) -#define to_jalr_insn(offset) \ - (((offset & JALR_OFFSET_MASK) << JALR_SHIFT) | JALR_BASIC) +#define to_jalr_ra(offset) \ + (((offset & JALR_OFFSET_MASK) << JALR_SHIFT) | JALR_RA) -#define to_auipc_insn(offset) \ +#define to_auipc_ra(offset) \ ((offset & JALR_SIGN_MASK) ? \ - (((offset & AUIPC_OFFSET_MASK) + AUIPC_PAD) | AUIPC_BASIC) : \ - ((offset & AUIPC_OFFSET_MASK) | AUIPC_BASIC)) + (((offset & AUIPC_OFFSET_MASK) + AUIPC_PAD) | AUIPC_RA) : \ + ((offset & AUIPC_OFFSET_MASK) | AUIPC_RA)) + +#define make_call_ra(caller, callee, call) \ +do { \ + unsigned int offset = \ + (unsigned long) callee - (unsigned long) caller; \ + call[0] = to_auipc_ra(offset); \ + call[1] = to_jalr_ra(offset); \ +} while (0) /* * Let auipc+jalr be the basic *mcount unit*, so we make it 8 bytes here. diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c index 2086f6585773..5bff37af4770 100644 --- a/arch/riscv/kernel/ftrace.c +++ b/arch/riscv/kernel/ftrace.c @@ -55,12 +55,15 @@ static int ftrace_check_current_call(unsigned long hook_pos, } static int __ftrace_modify_call(unsigned long hook_pos, unsigned long target, - bool enable) + bool enable, bool ra) { unsigned int call[2]; unsigned int nops[2] = {NOP4, NOP4}; - make_call(hook_pos, target, call); + if (ra) + make_call_ra(hook_pos, target, call); + else + make_call_t0(hook_pos, target, call); /* Replace the auipc-jalr pair at once. Return -EPERM on write error. */ if (patch_text_nosync @@ -70,42 +73,13 @@ static int __ftrace_modify_call(unsigned long hook_pos, unsigned long target, return 0; } -/* - * Put 5 instructions with 16 bytes at the front of function within - * patchable function entry nops' area. - * - * 0: REG_S ra, -SZREG(sp) - * 1: auipc ra, 0x? - * 2: jalr -?(ra) - * 3: REG_L ra, -SZREG(sp) - * - * So the opcodes is: - * 0: 0xfe113c23 (sd)/0xfe112e23 (sw) - * 1: 0x???????? -> auipc - * 2: 0x???????? -> jalr - * 3: 0xff813083 (ld)/0xffc12083 (lw) - */ -#if __riscv_xlen == 64 -#define INSN0 0xfe113c23 -#define INSN3 0xff813083 -#elif __riscv_xlen == 32 -#define INSN0 0xfe112e23 -#define INSN3 0xffc12083 -#endif - -#define FUNC_ENTRY_SIZE 16 -#define FUNC_ENTRY_JMP 4 - int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr) { - unsigned int call[4] = {INSN0, 0, 0, INSN3}; - unsigned long target = addr; - unsigned long caller = rec->ip + FUNC_ENTRY_JMP; + unsigned int call[2]; - call[1] = to_auipc_insn((unsigned int)(target - caller)); - call[2] = to_jalr_insn((unsigned int)(target - caller)); + make_call_t0(rec->ip, addr, call); - if (patch_text_nosync((void *)rec->ip, call, FUNC_ENTRY_SIZE)) + if (patch_text_nosync((void *)rec->ip, call, MCOUNT_INSN_SIZE)) return -EPERM; return 0; @@ -114,15 +88,14 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr) int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec, unsigned long addr) { - unsigned int nops[4] = {NOP4, NOP4, NOP4, NOP4}; + unsigned int nops[2] = {NOP4, NOP4}; - if (patch_text_nosync((void *)rec->ip, nops, FUNC_ENTRY_SIZE)) + if (patch_text_nosync((void *)rec->ip, nops, MCOUNT_INSN_SIZE)) return -EPERM; return 0; } - /* * This is called early on, and isn't wrapped by * ftrace_arch_code_modify_{prepare,post_process}() and therefor doesn't hold @@ -144,10 +117,10 @@ int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec) int ftrace_update_ftrace_func(ftrace_func_t func) { int ret = __ftrace_modify_call((unsigned long)&ftrace_call, - (unsigned long)func, true); + (unsigned long)func, true, true); if (!ret) { ret = __ftrace_modify_call((unsigned long)&ftrace_regs_call, - (unsigned long)func, true); + (unsigned long)func, true, true); } return ret; @@ -159,16 +132,16 @@ int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr, unsigned long addr) { unsigned int call[2]; - unsigned long caller = rec->ip + FUNC_ENTRY_JMP; + unsigned long caller = rec->ip; int ret; - make_call(caller, old_addr, call); + make_call_t0(caller, old_addr, call); ret = ftrace_check_current_call(caller, call); if (ret) return ret; - return __ftrace_modify_call(caller, addr, true); + return __ftrace_modify_call(caller, addr, true, false); } #endif @@ -203,12 +176,12 @@ int ftrace_enable_ftrace_graph_caller(void) int ret; ret = __ftrace_modify_call((unsigned long)&ftrace_graph_call, - (unsigned long)&prepare_ftrace_return, true); + (unsigned long)&prepare_ftrace_return, true, true); if (ret) return ret; return __ftrace_modify_call((unsigned long)&ftrace_graph_regs_call, - (unsigned long)&prepare_ftrace_return, true); + (unsigned long)&prepare_ftrace_return, true, true); } int ftrace_disable_ftrace_graph_caller(void) @@ -216,12 +189,12 @@ int ftrace_disable_ftrace_graph_caller(void) int ret; ret = __ftrace_modify_call((unsigned long)&ftrace_graph_call, - (unsigned long)&prepare_ftrace_return, false); + (unsigned long)&prepare_ftrace_return, false, true); if (ret) return ret; return __ftrace_modify_call((unsigned long)&ftrace_graph_regs_call, - (unsigned long)&prepare_ftrace_return, false); + (unsigned long)&prepare_ftrace_return, false, true); } #endif /* CONFIG_DYNAMIC_FTRACE */ #endif /* CONFIG_FUNCTION_GRAPH_TRACER */ diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S index d171eca623b6..125de818d1ba 100644 --- a/arch/riscv/kernel/mcount-dyn.S +++ b/arch/riscv/kernel/mcount-dyn.S @@ -13,8 +13,8 @@ .text -#define FENTRY_RA_OFFSET 12 -#define ABI_SIZE_ON_STACK 72 +#define FENTRY_RA_OFFSET 8 +#define ABI_SIZE_ON_STACK 80 #define ABI_A0 0 #define ABI_A1 8 #define ABI_A2 16 @@ -23,10 +23,10 @@ #define ABI_A5 40 #define ABI_A6 48 #define ABI_A7 56 -#define ABI_RA 64 +#define ABI_T0 64 +#define ABI_RA 72 .macro SAVE_ABI - addi sp, sp, -SZREG addi sp, sp, -ABI_SIZE_ON_STACK REG_S a0, ABI_A0(sp) @@ -37,6 +37,7 @@ REG_S a5, ABI_A5(sp) REG_S a6, ABI_A6(sp) REG_S a7, ABI_A7(sp) + REG_S t0, ABI_T0(sp) REG_S ra, ABI_RA(sp) .endm @@ -49,24 +50,18 @@ REG_L a5, ABI_A5(sp) REG_L a6, ABI_A6(sp) REG_L a7, ABI_A7(sp) + REG_L t0, ABI_T0(sp) REG_L ra, ABI_RA(sp) addi sp, sp, ABI_SIZE_ON_STACK - addi sp, sp, SZREG .endm #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS .macro SAVE_ALL - addi sp, sp, -SZREG addi sp, sp, -PT_SIZE_ON_STACK - REG_S x1, PT_EPC(sp) - addi sp, sp, PT_SIZE_ON_STACK - REG_L x1, (sp) - addi sp, sp, -PT_SIZE_ON_STACK + REG_S t0, PT_EPC(sp) REG_S x1, PT_RA(sp) - REG_L x1, PT_EPC(sp) - REG_S x2, PT_SP(sp) REG_S x3, PT_GP(sp) REG_S x4, PT_TP(sp) @@ -100,15 +95,11 @@ .endm .macro RESTORE_ALL + REG_L t0, PT_EPC(sp) REG_L x1, PT_RA(sp) - addi sp, sp, PT_SIZE_ON_STACK - REG_S x1, (sp) - addi sp, sp, -PT_SIZE_ON_STACK - REG_L x1, PT_EPC(sp) REG_L x2, PT_SP(sp) REG_L x3, PT_GP(sp) REG_L x4, PT_TP(sp) - REG_L x5, PT_T0(sp) REG_L x6, PT_T1(sp) REG_L x7, PT_T2(sp) REG_L x8, PT_S0(sp) @@ -137,17 +128,16 @@ REG_L x31, PT_T6(sp) addi sp, sp, PT_SIZE_ON_STACK - addi sp, sp, SZREG .endm #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ ENTRY(ftrace_caller) SAVE_ABI - addi a0, ra, -FENTRY_RA_OFFSET + addi a0, t0, -FENTRY_RA_OFFSET la a1, function_trace_op REG_L a2, 0(a1) - REG_L a1, ABI_SIZE_ON_STACK(sp) + mv a1, ra mv a3, sp ftrace_call: @@ -155,8 +145,8 @@ ftrace_call: call ftrace_stub #ifdef CONFIG_FUNCTION_GRAPH_TRACER - addi a0, sp, ABI_SIZE_ON_STACK - REG_L a1, ABI_RA(sp) + addi a0, sp, ABI_RA + REG_L a1, ABI_T0(sp) addi a1, a1, -FENTRY_RA_OFFSET #ifdef HAVE_FUNCTION_GRAPH_FP_TEST mv a2, s0 @@ -166,17 +156,17 @@ ftrace_graph_call: call ftrace_stub #endif RESTORE_ABI - ret + jr t0 ENDPROC(ftrace_caller) #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS ENTRY(ftrace_regs_caller) SAVE_ALL - addi a0, ra, -FENTRY_RA_OFFSET + addi a0, t0, -FENTRY_RA_OFFSET la a1, function_trace_op REG_L a2, 0(a1) - REG_L a1, PT_SIZE_ON_STACK(sp) + mv a1, ra mv a3, sp ftrace_regs_call: @@ -196,6 +186,6 @@ ftrace_graph_regs_call: #endif RESTORE_ALL - ret + jr t0 ENDPROC(ftrace_regs_caller) #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */

2 years, 8 months

1
0
0 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

Linux-stable-mirror