* Re: [PATCH 00/23] kconfig: move compiler capability tests to Kconfig
From: Arnd Bergmann @ 2018-02-21 9:56 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Ulf Magnusson, Rich Felker, Linux-sh list, Kernel Hardening,
Paul Mackerras, H. Peter Anvin, sparclinux, Sam Ravnborg,
uml-devel, Yoshinori Sato, Jonathan Corbet, X86 ML,
Linus Torvalds, Ingo Molnar, Emese Revfy, Kees Cook,
Linux Kbuild mailing list, Peter Oberparleiter, Jeff Dike,
user-mode-linux-user, Thomas Gleixner, Michal Marek,
Greg Kroah-Hartman, Randy Dunlap, open list:DOCUMENTATION,
Linux Kernel Mailing List, Richard Weinberger, linuxppc-dev,
David S. Miller
In-Reply-To: <CAK7LNAQH0WD3A3T97gce0xmZpMy0zWxTO2StfNWaDbiqjQg8gg@mail.gmail.com>
On Wed, Feb 21, 2018 at 8:38 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> 2018-02-20 0:18 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
>
>>>
>>> I'm not happy that we in one context can reference CONFIG variables
>>> directly, but inside the $(call ...) and $(shell ...) needs the $ prefix.
>>> But I could not come up with something un-ambigious where this could be avoided.
>>
>> I think we should be careful about allowing references to config
>> symbols. It mixes up the parsing and evaluation phases, since $() is
>> expanded during parsing (which I consider a feature and think is
>> needed to retain sanity).
>>
>> Patch 06/23 removes the last existing instance of symbol references in
>> strings by getting rid of 'option env'. That's an improvement to me.
>> We shouldn't add it back.
>
>
> This is really important design decision,
> so I'd like to hear a little more from experts.
>
>
> For example, x86 allows users to choose sub-arch, either 'i386' or 'x86_64'.
>
> https://github.com/torvalds/linux/blob/v4.16-rc2/arch/x86/Kconfig#L4
>
>
>
> If the user toggles CONFIG_64BIT,
> the bi-arch compiler will work in a slightly different mode
> (at least, back-end parts)
>
> So, my question is, is there a case,
>
> $(cc-option, -m32 -foo) is y, but
> $(cc-option, -m64 -foo) is n ?
> (or vice versa)
>
>
> If the answer is yes, $(cc-option -foo) would have to be re-calculated
> every time CONFIG_64BIT is toggled.
>
> This is what I'd like to avoid, though.
The -m32/-m64 trick (and -mbig-endian/-mlittle-endian on other architectures
as well as a couple of other flags) only works if the compiler is configured to
support it. In other cases (e.g. big-endian xtensa), the kernel always
detects what the compiler does and silently configures itself to match
using Makefile logic.
On x86, compilers are usually built as bi-arch, but you can build one that
only allows one of them.
I can see two reasonable ways out:
- we don't use $(cc-option -foo) in a case like this, and instead require the
user to have a matching toolchain.
- we could make the 32/64 selection on x86 a 'choice' statement where
each option depends on both the ARCH= variable and the
$(cc-option, -m32)/ $(cc-option, -m64) output.
Arnd
^ permalink raw reply
* Re: [PATCH v9 2/2] cxl: read PHB indications from the device tree
From: Frederic Barrat @ 2018-02-21 9:47 UTC (permalink / raw)
To: Philippe Bergheaud, linuxppc-dev; +Cc: clombard, benh
In-Reply-To: <20180220155111.20459-2-felix@linux.vnet.ibm.com>
Le 20/02/2018 à 16:51, Philippe Bergheaud a écrit :
> Configure the P9 XSL_DSNCTL register with PHB indications found
> in the device tree, or else use legacy hard-coded values.
>
> Signed-off-by: Philippe Bergheaud <felix@linux.vnet.ibm.com>
> ---
Reviewed-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
> Changelog:
>
> v2: New patch. Use the new device tree property "ibm,phb-indications".
>
> v3: No change.
>
> v4: No functional change.
> Drop cosmetic fix in comment.
>
> v5: get_phb_indications():
> - make static variables local to function.
> - return static variable values by arguments.
>
> v6: get_phb_indications():
> - acquire a mutex before setting the phb indications.
>
> v7: get_phb_indications():
> cxl_get_xsl9_dsnctl():
> - return -ENODEV instead of -1.
>
> v8: get_phb_indications():
> - stay on the safe side: acquire the mutex unconditionally
>
> v9: No change.
>
> This patch depends on the following skiboot patch:
> https://patchwork.ozlabs.org/patch/874415/
> ---
> drivers/misc/cxl/cxl.h | 2 +-
> drivers/misc/cxl/cxllib.c | 2 +-
> drivers/misc/cxl/pci.c | 48 ++++++++++++++++++++++++++++++++++++++++++-----
> 3 files changed, 45 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
> index 4f015da78f28..a7689944b351 100644
> --- a/drivers/misc/cxl/cxl.h
> +++ b/drivers/misc/cxl/cxl.h
> @@ -1065,7 +1065,7 @@ int cxl_psl_purge(struct cxl_afu *afu);
> int cxl_calc_capp_routing(struct pci_dev *dev, u64 *chipid,
> u32 *phb_index, u64 *capp_unit_id);
> int cxl_slot_is_switched(struct pci_dev *dev);
> -int cxl_get_xsl9_dsnctl(u64 capp_unit_id, u64 *reg);
> +int cxl_get_xsl9_dsnctl(struct pci_dev *dev, u64 capp_unit_id, u64 *reg);
> u64 cxl_calculate_sr(bool master, bool kernel, bool real_mode, bool p9);
>
> void cxl_native_irq_dump_regs_psl9(struct cxl_context *ctx);
> diff --git a/drivers/misc/cxl/cxllib.c b/drivers/misc/cxl/cxllib.c
> index 30ccba436b3b..bea1eb004b49 100644
> --- a/drivers/misc/cxl/cxllib.c
> +++ b/drivers/misc/cxl/cxllib.c
> @@ -99,7 +99,7 @@ int cxllib_get_xsl_config(struct pci_dev *dev, struct cxllib_xsl_config *cfg)
> if (rc)
> return rc;
>
> - rc = cxl_get_xsl9_dsnctl(capp_unit_id, &cfg->dsnctl);
> + rc = cxl_get_xsl9_dsnctl(dev, capp_unit_id, &cfg->dsnctl);
> if (rc)
> return rc;
> if (cpu_has_feature(CPU_FTR_POWER9_DD1)) {
> diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
> index 758842f65a1b..8d179e64a296 100644
> --- a/drivers/misc/cxl/pci.c
> +++ b/drivers/misc/cxl/pci.c
> @@ -407,21 +407,59 @@ int cxl_calc_capp_routing(struct pci_dev *dev, u64 *chipid,
> return 0;
> }
>
> -int cxl_get_xsl9_dsnctl(u64 capp_unit_id, u64 *reg)
> +static DEFINE_MUTEX(indications_mutex);
> +
> +static int get_phb_indications(struct pci_dev *dev, u64 *capiind, u64 *asnind,
> + u64 *nbwind)
> +{
> + static u64 nbw, asn, capi = 0;
> + struct device_node *np;
> + const __be32 *prop;
> +
> + mutex_lock(&indications_mutex);
> + if (!capi) {
> + if (!(np = pnv_pci_get_phb_node(dev))) {
> + mutex_unlock(&indications_mutex);
> + return -ENODEV;
> + }
> +
> + prop = of_get_property(np, "ibm,phb-indications", NULL);
> + if (!prop) {
> + nbw = 0x0300UL; /* legacy values */
> + asn = 0x0400UL;
> + capi = 0x0200UL;
> + } else {
> + nbw = (u64)be32_to_cpu(prop[2]);
> + asn = (u64)be32_to_cpu(prop[1]);
> + capi = (u64)be32_to_cpu(prop[0]);
> + }
> + of_node_put(np);
> + }
> + *capiind = capi;
> + *asnind = asn;
> + *nbwind = nbw;
> + mutex_unlock(&indications_mutex);
> + return 0;
> +}
> +
> +int cxl_get_xsl9_dsnctl(struct pci_dev *dev, u64 capp_unit_id, u64 *reg)
> {
> u64 xsl_dsnctl;
> + u64 capiind, asnind, nbwind;
>
> /*
> * CAPI Identifier bits [0:7]
> * bit 61:60 MSI bits --> 0
> * bit 59 TVT selector --> 0
> */
> + if (get_phb_indications(dev, &capiind, &asnind, &nbwind))
> + return -ENODEV;
>
> /*
> * Tell XSL where to route data to.
> * The field chipid should match the PHB CAPI_CMPM register
> */
> - xsl_dsnctl = ((u64)0x2 << (63-7)); /* Bit 57 */
> + xsl_dsnctl = (capiind << (63-15)); /* Bit 57 */
> xsl_dsnctl |= (capp_unit_id << (63-15));
>
> /* nMMU_ID Defaults to: b’000001001’*/
> @@ -435,14 +473,14 @@ int cxl_get_xsl9_dsnctl(u64 capp_unit_id, u64 *reg)
> * nbwind=0x03, bits [57:58], must include capi indicator.
> * Not supported on P9 DD1.
> */
> - xsl_dsnctl |= ((u64)0x03 << (63-47));
> + xsl_dsnctl |= (nbwind << (63-55));
>
> /*
> * Upper 16b address bits of ASB_Notify messages sent to the
> * system. Need to match the PHB’s ASN Compare/Mask Register.
> * Not supported on P9 DD1.
> */
> - xsl_dsnctl |= ((u64)0x04 << (63-55));
> + xsl_dsnctl |= asnind;
> }
>
> *reg = xsl_dsnctl;
> @@ -462,7 +500,7 @@ static int init_implementation_adapter_regs_psl9(struct cxl *adapter,
> if (rc)
> return rc;
>
> - rc = cxl_get_xsl9_dsnctl(capp_unit_id, &xsl_dsnctl);
> + rc = cxl_get_xsl9_dsnctl(dev, capp_unit_id, &xsl_dsnctl);
> if (rc)
> return rc;
>
^ permalink raw reply
* Re: [PATCH v9 1/2] powerpc/powernv: Enable tunneled operations
From: Frederic Barrat @ 2018-02-21 9:46 UTC (permalink / raw)
To: Philippe Bergheaud, linuxppc-dev; +Cc: clombard, benh
In-Reply-To: <20180220155111.20459-1-felix@linux.vnet.ibm.com>
Le 20/02/2018 à 16:51, Philippe Bergheaud a écrit :
> P9 supports PCI tunneled operations (atomics and as_notify). This
> patch adds support for tunneled operations on powernv, with a new
> API, to be called by device drivers:
>
> pnv_pci_enable_tunnel()
> Enable tunnel operations, tell driver the 16-bit ASN indication
> used by kernel.
>
> pnv_pci_disable_tunnel()
> Disable tunnel operations.
>
> pnv_pci_set_tunnel_bar()
> Tell kernel the Tunnel BAR Response address used by driver.
> This function uses two new OPAL calls, as the PBCQ Tunnel BAR
> register is configured by skiboot.
>
> pnv_pci_get_as_notify_info()
> Return the ASN info of the thread to be woken up.
>
> Signed-off-by: Philippe Bergheaud <felix@linux.vnet.ibm.com>
> ---
Reviewed-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
> Changelog:
>
> v2: Do not set the ASN indication. Get it from the device tree.
>
> v3: Make pnv_pci_get_phb_node() available when compiling without cxl.
>
> v4: Add pnv_pci_get_as_notify_info().
> Rebase opal call numbers on skiboot 5.9.6.
>
> v5: pnv_pci_get_tunnel_ind():
> - fix node reference count
> pnv_pci_get_as_notify_info():
> - fail if task == NULL
> - read pid from mm->context.id
> - explain that thread.tidr require CONFIG_PPC64
>
> v6: pnv_pci_get_tunnel_ind():
> - check if radix is enabled, or else return an error
> pnv_pci_get_as_notify_info():
> - remove a capi-specific comment, irrelevant for pci
>
> v7: pnv_pci_set_tunnel_bar():
> - setting the tunnel bar more than once with the same value
> is not an error
>
> v8: No change
>
> v9: Rename pnv_pci_get_tunnel_ind() into pnv_pci_enable_tunnel():
> - Increase real window size to accept as_notify messages.
> New api pnv_pci_disable_tunnel():
> - Restore real window size to its default value.
> Adjust opal call numbers.
>
> This patch depends on the following skiboot patches:
> https://patchwork.ozlabs.org/patch/874415/
> https://patchwork.ozlabs.org/patch/874416/
> ---
> arch/powerpc/include/asm/opal-api.h | 4 +-
> arch/powerpc/include/asm/opal.h | 2 +
> arch/powerpc/include/asm/pnv-pci.h | 6 ++
> arch/powerpc/platforms/powernv/opal-wrappers.S | 2 +
> arch/powerpc/platforms/powernv/pci-cxl.c | 8 --
> arch/powerpc/platforms/powernv/pci.c | 135 +++++++++++++++++++++++++
> 6 files changed, 148 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
> index 94bd1bf2c873..07b5e2240ecc 100644
> --- a/arch/powerpc/include/asm/opal-api.h
> +++ b/arch/powerpc/include/asm/opal-api.h
> @@ -204,7 +204,9 @@
> #define OPAL_NPU_SPA_SETUP 159
> #define OPAL_NPU_SPA_CLEAR_CACHE 160
> #define OPAL_NPU_TL_SET 161
> -#define OPAL_LAST 161
> +#define OPAL_PCI_GET_PBCQ_TUNNEL_BAR 162
> +#define OPAL_PCI_SET_PBCQ_TUNNEL_BAR 163
> +#define OPAL_LAST 163
>
> /* Device tree flags */
>
> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
> index 12e70fb58700..dde60089d0d4 100644
> --- a/arch/powerpc/include/asm/opal.h
> +++ b/arch/powerpc/include/asm/opal.h
> @@ -204,6 +204,8 @@ int64_t opal_unregister_dump_region(uint32_t id);
> int64_t opal_slw_set_reg(uint64_t cpu_pir, uint64_t sprn, uint64_t val);
> int64_t opal_config_cpu_idle_state(uint64_t state, uint64_t flag);
> int64_t opal_pci_set_phb_cxl_mode(uint64_t phb_id, uint64_t mode, uint64_t pe_number);
> +int64_t opal_pci_get_pbcq_tunnel_bar(uint64_t phb_id, uint64_t *addr);
> +int64_t opal_pci_set_pbcq_tunnel_bar(uint64_t phb_id, uint64_t addr);
> int64_t opal_ipmi_send(uint64_t interface, struct opal_ipmi_msg *msg,
> uint64_t msg_len);
> int64_t opal_ipmi_recv(uint64_t interface, struct opal_ipmi_msg *msg,
> diff --git a/arch/powerpc/include/asm/pnv-pci.h b/arch/powerpc/include/asm/pnv-pci.h
> index 3e5cf251ad9a..d2d8c28db336 100644
> --- a/arch/powerpc/include/asm/pnv-pci.h
> +++ b/arch/powerpc/include/asm/pnv-pci.h
> @@ -29,6 +29,12 @@ extern int pnv_pci_set_power_state(uint64_t id, uint8_t state,
> extern int pnv_pci_set_p2p(struct pci_dev *initiator, struct pci_dev *target,
> u64 desc);
>
> +extern int pnv_pci_enable_tunnel(struct pci_dev *dev, uint64_t *asnind);
> +extern int pnv_pci_disable_tunnel(struct pci_dev *dev);
> +extern int pnv_pci_set_tunnel_bar(struct pci_dev *dev, uint64_t addr,
> + int enable);
> +extern int pnv_pci_get_as_notify_info(struct task_struct *task, u32 *lpid,
> + u32 *pid, u32 *tid);
> int pnv_phb_to_cxl_mode(struct pci_dev *dev, uint64_t mode);
> int pnv_cxl_ioda_msi_setup(struct pci_dev *dev, unsigned int hwirq,
> unsigned int virq);
> diff --git a/arch/powerpc/platforms/powernv/opal-wrappers.S b/arch/powerpc/platforms/powernv/opal-wrappers.S
> index 1b2936ba6040..3da30c2f26b4 100644
> --- a/arch/powerpc/platforms/powernv/opal-wrappers.S
> +++ b/arch/powerpc/platforms/powernv/opal-wrappers.S
> @@ -323,3 +323,5 @@ OPAL_CALL(opal_sensor_group_clear, OPAL_SENSOR_GROUP_CLEAR);
> OPAL_CALL(opal_npu_spa_setup, OPAL_NPU_SPA_SETUP);
> OPAL_CALL(opal_npu_spa_clear_cache, OPAL_NPU_SPA_CLEAR_CACHE);
> OPAL_CALL(opal_npu_tl_set, OPAL_NPU_TL_SET);
> +OPAL_CALL(opal_pci_get_pbcq_tunnel_bar, OPAL_PCI_GET_PBCQ_TUNNEL_BAR);
> +OPAL_CALL(opal_pci_set_pbcq_tunnel_bar, OPAL_PCI_SET_PBCQ_TUNNEL_BAR);
> diff --git a/arch/powerpc/platforms/powernv/pci-cxl.c b/arch/powerpc/platforms/powernv/pci-cxl.c
> index 94498a04558b..cee003de63af 100644
> --- a/arch/powerpc/platforms/powernv/pci-cxl.c
> +++ b/arch/powerpc/platforms/powernv/pci-cxl.c
> @@ -16,14 +16,6 @@
>
> #include "pci.h"
>
> -struct device_node *pnv_pci_get_phb_node(struct pci_dev *dev)
> -{
> - struct pci_controller *hose = pci_bus_to_host(dev->bus);
> -
> - return of_node_get(hose->dn);
> -}
> -EXPORT_SYMBOL(pnv_pci_get_phb_node);
> -
> int pnv_phb_to_cxl_mode(struct pci_dev *dev, uint64_t mode)
> {
> struct pci_controller *hose = pci_bus_to_host(dev->bus);
> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
> index 69d102cbf48f..b265ecc0836a 100644
> --- a/arch/powerpc/platforms/powernv/pci.c
> +++ b/arch/powerpc/platforms/powernv/pci.c
> @@ -18,6 +18,7 @@
> #include <linux/io.h>
> #include <linux/msi.h>
> #include <linux/iommu.h>
> +#include <linux/sched/mm.h>
>
> #include <asm/sections.h>
> #include <asm/io.h>
> @@ -38,6 +39,7 @@
> #include "pci.h"
>
> static DEFINE_MUTEX(p2p_mutex);
> +static DEFINE_MUTEX(tunnel_mutex);
>
> int pnv_pci_get_slot_id(struct device_node *np, uint64_t *id)
> {
> @@ -1092,6 +1094,139 @@ int pnv_pci_set_p2p(struct pci_dev *initiator, struct pci_dev *target, u64 desc)
> }
> EXPORT_SYMBOL_GPL(pnv_pci_set_p2p);
>
> +struct device_node *pnv_pci_get_phb_node(struct pci_dev *dev)
> +{
> + struct pci_controller *hose = pci_bus_to_host(dev->bus);
> +
> + return of_node_get(hose->dn);
> +}
> +EXPORT_SYMBOL(pnv_pci_get_phb_node);
> +
> +int pnv_pci_enable_tunnel(struct pci_dev *dev, u64 *asnind)
> +{
> + struct device_node *np;
> + const __be32 *prop;
> + struct pnv_ioda_pe *pe;
> + uint16_t window_id;
> + int rc;
> +
> + if (!radix_enabled())
> + return -ENXIO;
> +
> + if (!(np = pnv_pci_get_phb_node(dev)))
> + return -ENXIO;
> +
> + prop = of_get_property(np, "ibm,phb-indications", NULL);
> + of_node_put(np);
> +
> + if (!prop || !prop[1])
> + return -ENXIO;
> +
> + *asnind = (u64)be32_to_cpu(prop[1]);
> + pe = pnv_ioda_get_pe(dev);
> + if (!pe)
> + return -ENODEV;
> +
> + /* Increase real window size to accept as_notify messages. */
> + window_id = (pe->pe_number << 1 ) + 1;
> + rc = opal_pci_map_pe_dma_window_real(pe->phb->opal_id, pe->pe_number,
> + window_id, pe->tce_bypass_base,
> + (uint64_t)1 << 48);
> + return opal_error_code(rc);
> +}
> +EXPORT_SYMBOL_GPL(pnv_pci_enable_tunnel);
> +
> +int pnv_pci_disable_tunnel(struct pci_dev *dev)
> +{
> + struct pnv_ioda_pe *pe;
> +
> + pe = pnv_ioda_get_pe(dev);
> + if (!pe)
> + return -ENODEV;
> +
> + /* Restore default real window size. */
> + pnv_pci_ioda2_set_bypass(pe, true);
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(pnv_pci_disable_tunnel);
> +
> +int pnv_pci_set_tunnel_bar(struct pci_dev *dev, u64 addr, int enable)
> +{
> + __be64 val;
> + struct pci_controller *hose;
> + struct pnv_phb *phb;
> + u64 tunnel_bar;
> + int rc;
> +
> + if (!opal_check_token(OPAL_PCI_GET_PBCQ_TUNNEL_BAR))
> + return -ENXIO;
> + if (!opal_check_token(OPAL_PCI_SET_PBCQ_TUNNEL_BAR))
> + return -ENXIO;
> +
> + hose = pci_bus_to_host(dev->bus);
> + phb = hose->private_data;
> +
> + mutex_lock(&tunnel_mutex);
> + rc = opal_pci_get_pbcq_tunnel_bar(phb->opal_id, &val);
> + if (rc != OPAL_SUCCESS) {
> + rc = -EIO;
> + goto out;
> + }
> + tunnel_bar = be64_to_cpu(val);
> + if (enable) {
> + /*
> + * Only one device per PHB can use atomics.
> + * Our policy is first-come, first-served.
> + */
> + if (tunnel_bar) {
> + if (tunnel_bar != addr)
> + rc = -EBUSY;
> + else
> + rc = 0; /* Setting same address twice is ok */
> + goto out;
> + }
> + } else {
> + /*
> + * The device that owns atomics and wants to release
> + * them must pass the same address with enable == 0.
> + */
> + if (tunnel_bar != addr) {
> + rc = -EPERM;
> + goto out;
> + }
> + addr = 0x0ULL;
> + }
> + rc = opal_pci_set_pbcq_tunnel_bar(phb->opal_id, addr);
> + rc = opal_error_code(rc);
> +out:
> + mutex_unlock(&tunnel_mutex);
> + return rc;
> +}
> +EXPORT_SYMBOL_GPL(pnv_pci_set_tunnel_bar);
> +
> +#ifdef CONFIG_PPC64 /* for thread.tidr */
> +int pnv_pci_get_as_notify_info(struct task_struct *task, u32 *lpid, u32 *pid,
> + u32 *tid)
> +{
> + struct mm_struct *mm = NULL;
> +
> + if (task == NULL)
> + return -EINVAL;
> +
> + mm = get_task_mm(task);
> + if (mm == NULL)
> + return -EINVAL;
> +
> + *pid = mm->context.id;
> + mmput(mm);
> +
> + *tid = task->thread.tidr;
> + *lpid = mfspr(SPRN_LPID);
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(pnv_pci_get_as_notify_info);
> +#endif
> +
> void pnv_pci_shutdown(void)
> {
> struct pci_controller *hose;
>
^ permalink raw reply
* Re: [PATCH] cpufreq: powernv: Check negative value returned by cpufreq_table_find_index_dl()
From: Rafael J. Wysocki @ 2018-02-21 9:27 UTC (permalink / raw)
To: Viresh Kumar, Michael Ellerman
Cc: Shilpasri G Bhat, Rafael J. Wysocki, Linux PM,
Linux Kernel Mailing List, linuxppc-dev
In-Reply-To: <20180221055450.GO28462@vireshk-i7>
On Wed, Feb 21, 2018 at 6:54 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 21-02-18, 16:39, Michael Ellerman wrote:
>> Viresh Kumar <viresh.kumar@linaro.org> writes:
>
>> > AFAICT, you will get -1 here only if the freq table had no valid
>> > frequencies (or the freq table is empty). Why would that happen ?
>>
>> Bugs?
>
> The cupfreq driver shouldn't have registered itself in that case (i.e.
> if the cpufreq table is empty).
To be precise, ->init() should fail as that's where the table is
created. The registration fails as a result then.
But what if the bug is that ->init() doesn't fail when it should?
I guess the core could double check the frequency table after ->init()
if ->target_index is not NULL.
The overall point here is that if you get a negative index in
->fast_switch(), that's way too late anyway and we should be able to
catch that error much earlier.
^ permalink raw reply
* Re: [PATCH V6] cxl: Fix timebase synchronization status on P9
From: Frederic Barrat @ 2018-02-21 8:45 UTC (permalink / raw)
To: Christophe Lombard, linuxppc-dev, vaibhav, andrew.donnellan
In-Reply-To: <1519134536-16485-1-git-send-email-clombard@linux.vnet.ibm.com>
Le 20/02/2018 à 14:48, Christophe Lombard a écrit :
> The PSL Timebase register is updated by the PSL to maintain the
> timebase.
> On P9, the Timebase value is only provided by the CAPP as received
> the last time a timebase request was performed.
> The timebase requests are initiated through the adapter configuration or
> application registers.
> The specific sysfs entry "/sys/class/cxl/cardxx/psl_timebase_synced" is
> now dynamically updated according the content of the PSL Timebase
> register.
>
> Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
> Reviewed-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
> Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
>
> ---
Thanks!
Acked-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
> This patch applies on top of this patch:
> http://patchwork.ozlabs.org/patch/873663/
>
> Changelog[v6]
> - Rebased to latest upstream.
> - Recompute the status only in native mode.
>
> Changelog[v5]
> - Rebased to latest upstream.
> - Changed the type of 'delta'
>
> Changelog[v4]
> - Rebased to latest upstream.
> - Added log message.
>
> Changelog[v3]
> - Rebased to latest upstream.
> - Dynamic update is now applied to P8.
>
> Changelog[v2]
> - Missing Signed-off-by.
> - Spaces required around the ':'.
> ---
> drivers/misc/cxl/pci.c | 17 -----------------
> drivers/misc/cxl/sysfs.c | 12 ++++++++++++
> 2 files changed, 12 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
> index 66eed6a..3247eaf 100644
> --- a/drivers/misc/cxl/pci.c
> +++ b/drivers/misc/cxl/pci.c
> @@ -606,9 +606,6 @@ static u64 timebase_read_xsl(struct cxl *adapter)
>
> static void cxl_setup_psl_timebase(struct cxl *adapter, struct pci_dev *dev)
> {
> - u64 psl_tb;
> - int delta;
> - unsigned int retry = 0;
> struct device_node *np;
>
> adapter->psl_timebase_synced = false;
> @@ -636,20 +633,6 @@ static void cxl_setup_psl_timebase(struct cxl *adapter, struct pci_dev *dev)
> cxl_p1_write(adapter, CXL_PSL_Control, 0x0000000000000000);
> cxl_p1_write(adapter, CXL_PSL_Control, CXL_PSL_Control_tb);
>
> - /* Wait until CORE TB and PSL TB difference <= 16usecs */
> - do {
> - msleep(1);
> - if (retry++ > 5) {
> - dev_info(&dev->dev, "PSL timebase can't synchronize\n");
> - return;
> - }
> - psl_tb = adapter->native->sl_ops->timebase_read(adapter);
> - delta = mftb() - psl_tb;
> - if (delta < 0)
> - delta = -delta;
> - } while (tb_to_ns(delta) > 16000);
> -
> - adapter->psl_timebase_synced = true;
> return;
> }
>
> diff --git a/drivers/misc/cxl/sysfs.c b/drivers/misc/cxl/sysfs.c
> index a8b6d6a..95285b7 100644
> --- a/drivers/misc/cxl/sysfs.c
> +++ b/drivers/misc/cxl/sysfs.c
> @@ -62,7 +62,19 @@ static ssize_t psl_timebase_synced_show(struct device *device,
> char *buf)
> {
> struct cxl *adapter = to_cxl_adapter(device);
> + u64 psl_tb, delta;
>
> + /* Recompute the status only in native mode */
> + if (cpu_has_feature(CPU_FTR_HVMODE)) {
> + psl_tb = adapter->native->sl_ops->timebase_read(adapter);
> + delta = abs(mftb() - psl_tb);
> +
> + /* CORE TB and PSL TB difference <= 16usecs ? */
> + adapter->psl_timebase_synced = (tb_to_ns(delta) < 16000) ? true : false;
> + pr_devel("PSL timebase %s - delta: 0x%016llx\n",
> + (tb_to_ns(delta) < 16000) ? "synchronized" :
> + "not synchronized", tb_to_ns(delta));
> + }
> return scnprintf(buf, PAGE_SIZE, "%i\n", adapter->psl_timebase_synced);
> }
>
^ permalink raw reply
* Re: [PATCH] bpf, powerpc: fix jit for seccomp_data access
From: Naveen N. Rao @ 2018-02-21 8:35 UTC (permalink / raw)
To: Mark Lord
Cc: Alexei Starovoitov, Daniel Borkmann, Kees Cook, Andy Lutomirski,
mpe, Will Drewry, linuxppc-dev
In-Reply-To: <2f428cf9-23f3-d1f1-1524-46d6faa17386__30848.5239781023$1519169515$gmane$org@ pobox.com>
Mark Lord wrote:
> I am using SECCOMP to filter syscalls on a ppc32 platform,
> and noticed that the JIT compiler was failing on the BPF
> even though the interpreter was working fine.
>=20
> The issue was that the compiler was missing one of the instructions
> used by SECCOMP, so here is a patch to enable JIT for that instruction.
>=20
> Signed-Off-By: Mark Lord <mlord@pobox.com>
>=20
> --- old/arch/powerpc/net/bpf_jit_comp.c 2018-02-16 14:07:01.000000000 -05=
00
> +++ linux/arch/powerpc/net/bpf_jit_comp.c 2018-02-20 14:41:20.80522=
7494 -0500
> @@ -329,6 +329,9 @@ static int bpf_jit_build_body(struct bpf
> BUILD_BUG_ON(FIELD_SIZEOF(struct sk_buff, len) !=
=3D 4);
> PPC_LWZ_OFFS(r_A, r_skb, offsetof(struct sk_buff,=
len));
> break;
> + case BPF_LDX | BPF_W | BPF_ABS: /* A =3D *((u32 *)(seccom=
p_data + K)); */
> + PPC_LWZ_OFFS(r_A, r_skb, K);
> + break;
Looks good to me, but I am not able to apply this patch. There seems to=20
be whitespace damage. Please resend the patch considering the steps=20
here:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Doc=
umentation/process/email-clients.rst
Thanks,
Naveen
=
^ permalink raw reply
* Re: [PATCH 00/23] kconfig: move compiler capability tests to Kconfig
From: Masahiro Yamada @ 2018-02-21 7:38 UTC (permalink / raw)
To: Ulf Magnusson
Cc: Sam Ravnborg, Linux Kbuild mailing list, Linus Torvalds,
Greg Kroah-Hartman, Arnd Bergmann, Kees Cook, Randy Dunlap,
Michal Marek, Peter Oberparleiter, kernel-hardening,
Jonathan Corbet, sparclinux, Linux-sh list, X86 ML,
Thomas Gleixner, Rich Felker, Jeff Dike, H. Peter Anvin,
user-mode-linux-devel, Yoshinori Sato, Benjamin Herrenschmidt,
linuxppc-dev, Paul Mackerras, user-mode-linux-user, Ingo Molnar,
David S. Miller, Michael Ellerman, open list:DOCUMENTATION,
Linux Kernel Mailing List, Richard Weinberger, Emese Revfy
In-Reply-To: <CAFkk2KT1krKuDkWt0eHcasfC61fTVQNf-1kqw7icLP9OYGYkXQ@mail.gmail.com>
2018-02-20 0:18 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
>>
>> I'm not happy that we in one context can reference CONFIG variables
>> directly, but inside the $(call ...) and $(shell ...) needs the $ prefix.
>> But I could not come up with something un-ambigious where this could be avoided.
>
> I think we should be careful about allowing references to config
> symbols. It mixes up the parsing and evaluation phases, since $() is
> expanded during parsing (which I consider a feature and think is
> needed to retain sanity).
>
> Patch 06/23 removes the last existing instance of symbol references in
> strings by getting rid of 'option env'. That's an improvement to me.
> We shouldn't add it back.
This is really important design decision,
so I'd like to hear a little more from experts.
For example, x86 allows users to choose sub-arch, either 'i386' or 'x86_64'.
https://github.com/torvalds/linux/blob/v4.16-rc2/arch/x86/Kconfig#L4
If the user toggles CONFIG_64BIT,
the bi-arch compiler will work in a slightly different mode
(at least, back-end parts)
So, my question is, is there a case,
$(cc-option, -m32 -foo) is y, but
$(cc-option, -m64 -foo) is n ?
(or vice versa)
If the answer is yes, $(cc-option -foo) would have to be re-calculated
every time CONFIG_64BIT is toggled.
This is what I'd like to avoid, though.
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH 0/6] DISCONTIGMEM support for PPC32
From: Christophe LEROY @ 2018-02-21 7:06 UTC (permalink / raw)
To: Jonathan Neuschäfer, linuxppc-dev
Cc: Joel Stanley, linux-kernel, linux-mm
In-Reply-To: <20180220161424.5421-1-j.neuschaefer@gmx.net>
Le 20/02/2018 à 17:14, Jonathan Neuschäfer a écrit :
> This patchset adds support for DISCONTIGMEM on 32-bit PowerPC. This is
> required to properly support the Nintendo Wii's memory layout, in which
> there are two blocks of RAM and MMIO in the middle.
>
> Previously, this memory layout was handled by code that joins the two
> RAM blocks into one, reserves the MMIO hole, and permits allocations of
> reserved memory in ioremap. This hack didn't work with resource-based
> allocation (as used for example in the GPIO driver for Wii[1]), however.
>
> After this patchset, users of the Wii can either select CONFIG_FLATMEM
> to get the old behaviour, or CONFIG_DISCONTIGMEM to get the new
> behaviour.
My question might me stupid, as I don't know PCC64 in deep, but when
looking at page_is_ram() in arch/powerpc/mm/mem.c, I have the feeling
the PPC64 implements ram by blocks. Isn't it what you are trying to
achieve ? Wouldn't it be feasible to map to what's done in PPC64 for PPC32 ?
Christophe
>
> Some parts of this patchset are probably not ideal (I'm thinking of my
> implementation of pfn_to_nid here), and will require some discussion/
> changes.
>
> [1]: https://www.spinics.net/lists/devicetree/msg213956.html
>
> Jonathan Neuschäfer (6):
> powerpc/mm/32: Use pfn_valid to check if pointer is in RAM
> powerpc: numa: Fix overshift on PPC32
> powerpc: numa: Use the right #ifdef guards around functions
> powerpc: numa: Restrict fake NUMA enulation to CONFIG_NUMA systems
> powerpc: Implement DISCONTIGMEM and allow selection on PPC32
> powerpc: wii: Don't rely on reserved memory hack if DISCONTIGMEM is
> set
>
> arch/powerpc/Kconfig | 5 ++++-
> arch/powerpc/include/asm/mmzone.h | 21 +++++++++++++++++++++
> arch/powerpc/mm/numa.c | 18 +++++++++++++++---
> arch/powerpc/mm/pgtable_32.c | 2 +-
> arch/powerpc/platforms/embedded6xx/wii.c | 10 +++++++---
> 5 files changed, 48 insertions(+), 8 deletions(-)
>
^ permalink raw reply
* Re: [PATCH] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
From: Balbir Singh @ 2018-02-21 6:43 UTC (permalink / raw)
To: Alastair D'Silva
Cc: linuxppc-dev, linux-kernel@vger.kernel.org, Arnd Bergmann,
frederic.barrat, Greg KH, Andrew Donnellan, Alastair D'Silva
In-Reply-To: <20180221045736.7614-1-alastair@au1.ibm.com>
On Wed, Feb 21, 2018 at 3:57 PM, Alastair D'Silva <alastair@au1.ibm.com> wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
>
> Some required information is not exposed to userspace currently (eg. the
> PASID), pass this information back, along with other information which
> is currently communicated via sysfs, which saves some parsing effort in
> userspace.
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
> drivers/misc/ocxl/file.c | 27 +++++++++++++++++++++++++++
> include/uapi/misc/ocxl.h | 22 ++++++++++++++++++++++
> 2 files changed, 49 insertions(+)
>
> diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
> index d9aa407db06a..11514a8444e5 100644
> --- a/drivers/misc/ocxl/file.c
> +++ b/drivers/misc/ocxl/file.c
> @@ -102,10 +102,32 @@ static long afu_ioctl_attach(struct ocxl_context *ctx,
> return rc;
> }
>
> +static long afu_ioctl_get_metadata(struct ocxl_context *ctx,
> + struct ocxl_ioctl_get_metadata __user *uarg)
Why do we call this metadata? Isn't this an afu_descriptor?
> +{
> + struct ocxl_ioctl_get_metadata arg;
> +
> + memset(&arg, 0, sizeof(arg));
> +
> + arg.version = 0;
Does it make sense to have version 0? Even if does, you can afford
to skip initialization due to the memset above. I prefer that versions
start with 1
> +
> + arg.afu_version_major = ctx->afu->config.version_major;
> + arg.afu_version_minor = ctx->afu->config.version_minor;
> + arg.pasid = ctx->pasid;
> + arg.pp_mmio_size = ctx->afu->config.pp_mmio_stride;
> + arg.global_mmio_size = ctx->afu->config.global_mmio_size;
> +
> + if (copy_to_user(uarg, &arg, sizeof(arg)))
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> #define CMD_STR(x) (x == OCXL_IOCTL_ATTACH ? "ATTACH" : \
> x == OCXL_IOCTL_IRQ_ALLOC ? "IRQ_ALLOC" : \
> x == OCXL_IOCTL_IRQ_FREE ? "IRQ_FREE" : \
> x == OCXL_IOCTL_IRQ_SET_FD ? "IRQ_SET_FD" : \
> + x == OCXL_IOCTL_GET_METADATA ? "GET_METADATA" : \
> "UNKNOWN")
>
> static long afu_ioctl(struct file *file, unsigned int cmd,
> @@ -157,6 +179,11 @@ static long afu_ioctl(struct file *file, unsigned int cmd,
> irq_fd.eventfd);
> break;
>
> + case OCXL_IOCTL_GET_METADATA:
> + rc = afu_ioctl_get_metadata(ctx,
> + (struct ocxl_ioctl_get_metadata __user *) args);
> + break;
> +
> default:
> rc = -EINVAL;
> }
> diff --git a/include/uapi/misc/ocxl.h b/include/uapi/misc/ocxl.h
> index 4b0b0b756f3e..16e1f48ce280 100644
> --- a/include/uapi/misc/ocxl.h
> +++ b/include/uapi/misc/ocxl.h
> @@ -32,6 +32,27 @@ struct ocxl_ioctl_attach {
> __u64 reserved3;
> };
>
> +/*
> + * Version contains the version of the struct.
> + * Versions will always be backwards compatible, that is, new versions will not
> + * alter existing fields
> + */
> +struct ocxl_ioctl_get_metadata {
This sounds more like a function name, do we need it to be _get_metdata?
> + __u16 version;
> +
> + // Version 0 fields
> + __u8 afu_version_major;
> + __u8 afu_version_minor;
> + __u32 pasid;
> +
> + __u64 pp_mmio_size;
> + __u64 global_mmio_size;
> +
Should we document the fields? pp_ stands for per process, but is not
very clear at first look. Why do we care to return only the size, what
about lpc size?
> + // End version 0 fields
> +
> + __u64 reserved[13]; // Total of 16*u64
> +};
Balbir Singh.
^ permalink raw reply
* Re: [PATCH] cpufreq: powernv: Check negative value returned by cpufreq_table_find_index_dl()
From: Viresh Kumar @ 2018-02-21 5:54 UTC (permalink / raw)
To: Michael Ellerman
Cc: Shilpasri G Bhat, rjw, linux-pm, linux-kernel, linuxppc-dev
In-Reply-To: <874lmasxxx.fsf@concordia.ellerman.id.au>
On 21-02-18, 16:39, Michael Ellerman wrote:
> Viresh Kumar <viresh.kumar@linaro.org> writes:
> > AFAICT, you will get -1 here only if the freq table had no valid
> > frequencies (or the freq table is empty). Why would that happen ?
>
> Bugs?
The cupfreq driver shouldn't have registered itself in that case (i.e.
if the cpufreq table is empty).
> Or if you ask for a target_freq that is higher than anything in the
> table.
You will still get a valid index in that case.
There is only once case where we return -1, when the cpufreq table
doesn't have any valid frequencies.
> Or the API changes, and we forget to update this call site.
I am not sure we can do much about that right now.
> If you're saying that cpufreq_table_find_index_dl() can NEVER fail,
Yes, if we have at least one valid frequency in the table, otherwise
the cpufreq driver shouldn't have registered itself.
> then
> write it so that it can never fail and change it to return unsigned int.
But what should we do when there is no frequency in the cpufreq table?
Just in case where a driver is buggy and tries to call this routine
for an invalid table.
> Having it potentially return -1, which is then used to index an array
> and not handling that is just asking for bugs to happen.
I understand what you are trying to say here, but I don't know what
can be done to prevent this here.
What we can do is change the return type to void and pass a int
pointer to the routine, but that wouldn't change anything at all. That
pointers variable can still have -1 in it.
--
viresh
^ permalink raw reply
* Re: [PATCH] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
From: Andrew Donnellan @ 2018-02-21 5:49 UTC (permalink / raw)
To: Alastair D'Silva, linuxppc-dev, linux-kernel
Cc: frederic.barrat, arnd, gregkh, Alastair D'Silva
In-Reply-To: <20180221045736.7614-1-alastair@au1.ibm.com>
On 21/02/18 15:57, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
>
> Some required information is not exposed to userspace currently (eg. the
> PASID), pass this information back, along with other information which
> is currently communicated via sysfs, which saves some parsing effort in
> userspace.
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
Seems fine.
Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCH] cpufreq: powernv: Check negative value returned by cpufreq_table_find_index_dl()
From: Michael Ellerman @ 2018-02-21 5:39 UTC (permalink / raw)
To: Viresh Kumar, Shilpasri G Bhat; +Cc: rjw, linux-pm, linux-kernel, linuxppc-dev
In-Reply-To: <20180212102900.GU28462@vireshk-i7>
Viresh Kumar <viresh.kumar@linaro.org> writes:
> On 12-02-18, 15:51, Shilpasri G Bhat wrote:
>> This patch fixes the below Coverity warning:
>>
>> *** CID 182816: Memory - illegal accesses (NEGATIVE_RETURNS)
>> /drivers/cpufreq/powernv-cpufreq.c: 1008 in powernv_fast_switch()
>> 1002 unsigned int target_freq)
>> 1003 {
>> 1004 int index;
>> 1005 struct powernv_smp_call_data freq_data;
>> 1006
>> 1007 index = cpufreq_table_find_index_dl(policy, target_freq);
>> >>> CID 182816: Memory - illegal accesses (NEGATIVE_RETURNS)
>> >>> Using variable "index" as an index to array "powernv_freqs".
>> 1008 freq_data.pstate_id = powernv_freqs[index].driver_data;
>> 1009 freq_data.gpstate_id = powernv_freqs[index].driver_data;
>> 1010 set_pstate(&freq_data);
>> 1011
>> 1012 return powernv_freqs[index].frequency;
>> 1013 }
>>
>> Signed-off-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>
>> ---
>> drivers/cpufreq/powernv-cpufreq.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/cpufreq/powernv-cpufreq.c b/drivers/cpufreq/powernv-cpufreq.c
>> index 29cdec1..69edfe9 100644
>> --- a/drivers/cpufreq/powernv-cpufreq.c
>> +++ b/drivers/cpufreq/powernv-cpufreq.c
>> @@ -1005,6 +1005,9 @@ static unsigned int powernv_fast_switch(struct cpufreq_policy *policy,
>> struct powernv_smp_call_data freq_data;
>>
>> index = cpufreq_table_find_index_dl(policy, target_freq);
>> + if (unlikely(index < 0))
>> + index = get_nominal_index();
>> +
>
> AFAICT, you will get -1 here only if the freq table had no valid
> frequencies (or the freq table is empty). Why would that happen ?
Bugs?
Or if you ask for a target_freq that is higher than anything in the
table.
Or the API changes, and we forget to update this call site.
If you're saying that cpufreq_table_find_index_dl() can NEVER fail, then
write it so that it can never fail and change it to return unsigned int.
Having it potentially return -1, which is then used to index an array
and not handling that is just asking for bugs to happen.
cheers
^ permalink raw reply
* Re: [RFC] powerpc/powernv/mce: Don't silently restart the machine
From: Balbir Singh @ 2018-02-21 5:02 UTC (permalink / raw)
To: Stewart Smith
Cc: open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), Nicholas Piggin
In-Reply-To: <87vaeruemb.fsf@linux.vnet.ibm.com>
On Wed, Feb 21, 2018 at 3:54 PM, Stewart Smith
<stewart@linux.vnet.ibm.com> wrote:
> Balbir Singh <bsingharora@gmail.com> writes:
>> On MCE the current code will restart the machine with
>> ppc_md.restart(). This case was extremely unlikely since
>> prior to that a skiboot call is made and that resulted in
>> a checkstop for analysis.
>>
>> With newer skiboots, on P9 we don't checkstop the box by
>> default, instead we return back to the kernel to extract
>> useful information at the time of the MCE. While we still
>> get this information, this patch converts the restart to
>> a panic(), so that if configured a dump can be taken and
>> we can track and probably debug the potential issue causing
>> the MCE.
>
> I agree with the patch, although I'd be nervous stating that skiboot is
> going to keep this behaviour. In *theory* we should only ever get a
> platform error when there's actually something that isn't the kernel's
> fault.
>
> Like any firmware promise though, it's slightly less reliable than one
> from a politician.
>
> I'd say that in this case deferring to policy on what to do in event of
> panic() is the right thing.
>
Your right, except that with NPUs and coherent device memory, things
change. It is a platform error on the device side, the large issue is
that the CPU touched this memory. Ideally we want the device and
device driver to handle this error, but its racy as the CPU now treats
it as a platform error on the system side. IOW, the definition of
platform has grown and so has the definition of platform error and it
can no longer be solely contained inside of one boxes firmware.
Balbir Singh.
^ permalink raw reply
* [PATCH] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
From: Alastair D'Silva @ 2018-02-21 4:57 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel
Cc: frederic.barrat, andrew.donnellan, arnd, gregkh, alastair,
Alastair D'Silva
From: Alastair D'Silva <alastair@d-silva.org>
Some required information is not exposed to userspace currently (eg. the
PASID), pass this information back, along with other information which
is currently communicated via sysfs, which saves some parsing effort in
userspace.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
drivers/misc/ocxl/file.c | 27 +++++++++++++++++++++++++++
include/uapi/misc/ocxl.h | 22 ++++++++++++++++++++++
2 files changed, 49 insertions(+)
diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
index d9aa407db06a..11514a8444e5 100644
--- a/drivers/misc/ocxl/file.c
+++ b/drivers/misc/ocxl/file.c
@@ -102,10 +102,32 @@ static long afu_ioctl_attach(struct ocxl_context *ctx,
return rc;
}
+static long afu_ioctl_get_metadata(struct ocxl_context *ctx,
+ struct ocxl_ioctl_get_metadata __user *uarg)
+{
+ struct ocxl_ioctl_get_metadata arg;
+
+ memset(&arg, 0, sizeof(arg));
+
+ arg.version = 0;
+
+ arg.afu_version_major = ctx->afu->config.version_major;
+ arg.afu_version_minor = ctx->afu->config.version_minor;
+ arg.pasid = ctx->pasid;
+ arg.pp_mmio_size = ctx->afu->config.pp_mmio_stride;
+ arg.global_mmio_size = ctx->afu->config.global_mmio_size;
+
+ if (copy_to_user(uarg, &arg, sizeof(arg)))
+ return -EFAULT;
+
+ return 0;
+}
+
#define CMD_STR(x) (x == OCXL_IOCTL_ATTACH ? "ATTACH" : \
x == OCXL_IOCTL_IRQ_ALLOC ? "IRQ_ALLOC" : \
x == OCXL_IOCTL_IRQ_FREE ? "IRQ_FREE" : \
x == OCXL_IOCTL_IRQ_SET_FD ? "IRQ_SET_FD" : \
+ x == OCXL_IOCTL_GET_METADATA ? "GET_METADATA" : \
"UNKNOWN")
static long afu_ioctl(struct file *file, unsigned int cmd,
@@ -157,6 +179,11 @@ static long afu_ioctl(struct file *file, unsigned int cmd,
irq_fd.eventfd);
break;
+ case OCXL_IOCTL_GET_METADATA:
+ rc = afu_ioctl_get_metadata(ctx,
+ (struct ocxl_ioctl_get_metadata __user *) args);
+ break;
+
default:
rc = -EINVAL;
}
diff --git a/include/uapi/misc/ocxl.h b/include/uapi/misc/ocxl.h
index 4b0b0b756f3e..16e1f48ce280 100644
--- a/include/uapi/misc/ocxl.h
+++ b/include/uapi/misc/ocxl.h
@@ -32,6 +32,27 @@ struct ocxl_ioctl_attach {
__u64 reserved3;
};
+/*
+ * Version contains the version of the struct.
+ * Versions will always be backwards compatible, that is, new versions will not
+ * alter existing fields
+ */
+struct ocxl_ioctl_get_metadata {
+ __u16 version;
+
+ // Version 0 fields
+ __u8 afu_version_major;
+ __u8 afu_version_minor;
+ __u32 pasid;
+
+ __u64 pp_mmio_size;
+ __u64 global_mmio_size;
+
+ // End version 0 fields
+
+ __u64 reserved[13]; // Total of 16*u64
+};
+
struct ocxl_ioctl_irq_fd {
__u64 irq_offset;
__s32 eventfd;
@@ -45,5 +66,6 @@ struct ocxl_ioctl_irq_fd {
#define OCXL_IOCTL_IRQ_ALLOC _IOR(OCXL_MAGIC, 0x11, __u64)
#define OCXL_IOCTL_IRQ_FREE _IOW(OCXL_MAGIC, 0x12, __u64)
#define OCXL_IOCTL_IRQ_SET_FD _IOW(OCXL_MAGIC, 0x13, struct ocxl_ioctl_irq_fd)
+#define OCXL_IOCTL_GET_METADATA _IOR(OCXL_MAGIC, 0x14, struct ocxl_ioctl_get_metadata)
#endif /* _UAPI_MISC_OCXL_H */
--
2.14.3
^ permalink raw reply related
* Re: [PATCH v9 1/2] powerpc/powernv: Enable tunneled operations
From: Stewart Smith @ 2018-02-21 4:56 UTC (permalink / raw)
To: Philippe Bergheaud, linuxppc-dev
Cc: Philippe Bergheaud, benh, clombard, fbarrat
In-Reply-To: <20180220155111.20459-1-felix@linux.vnet.ibm.com>
Philippe Bergheaud <felix@linux.vnet.ibm.com> writes:
> diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
> index 94bd1bf2c873..07b5e2240ecc 100644
> --- a/arch/powerpc/include/asm/opal-api.h
> +++ b/arch/powerpc/include/asm/opal-api.h
> @@ -204,7 +204,9 @@
> #define OPAL_NPU_SPA_SETUP 159
> #define OPAL_NPU_SPA_CLEAR_CACHE 160
> #define OPAL_NPU_TL_SET 161
> -#define OPAL_LAST 161
> +#define OPAL_PCI_GET_PBCQ_TUNNEL_BAR 162
> +#define OPAL_PCI_SET_PBCQ_TUNNEL_BAR 163
> +#define OPAL_LAST 163
These numbers will change as other calls have gone in with those OPAL
API ids.
Feel free to send a tiny patch to skiboot reserving some though.
I should probably encourage people to do that when they start working on
new API calls.
--
Stewart Smith
OPAL Architect, IBM.
^ permalink raw reply
* Re: [RFC] powerpc/powernv/mce: Don't silently restart the machine
From: Stewart Smith @ 2018-02-21 4:54 UTC (permalink / raw)
To: Balbir Singh, linuxppc-dev; +Cc: npiggin
In-Reply-To: <20180221010111.30517-1-bsingharora@gmail.com>
Balbir Singh <bsingharora@gmail.com> writes:
> On MCE the current code will restart the machine with
> ppc_md.restart(). This case was extremely unlikely since
> prior to that a skiboot call is made and that resulted in
> a checkstop for analysis.
>
> With newer skiboots, on P9 we don't checkstop the box by
> default, instead we return back to the kernel to extract
> useful information at the time of the MCE. While we still
> get this information, this patch converts the restart to
> a panic(), so that if configured a dump can be taken and
> we can track and probably debug the potential issue causing
> the MCE.
I agree with the patch, although I'd be nervous stating that skiboot is
going to keep this behaviour. In *theory* we should only ever get a
platform error when there's actually something that isn't the kernel's
fault.
Like any firmware promise though, it's slightly less reliable than one
from a politician.
I'd say that in this case deferring to policy on what to do in event of
panic() is the right thing.
--
Stewart Smith
OPAL Architect, IBM.
^ permalink raw reply
* Re: [PATCH v2 (skiboot)] dt: add /cpus/ibm, powerpc-cpu-features device tree bindings
From: Stewart Smith @ 2018-02-21 3:28 UTC (permalink / raw)
To: Nicholas Piggin, skiboot; +Cc: linuxppc-dev, Nicholas Piggin
In-Reply-To: <20180216034042.7356-1-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> This is a new CPU feature advertising interface that is fine-grained,
> extensible, aware of privilege levels, and gives control of features
> to all levels of the stack (firmware, hypervisor, and OS).
>
> The design and binding specification is described in detail in doc/.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> Since v1:
>
> - Fold branch-v3 into fixed-point-v3, as pointed out by Segher they
> are all fixed point facilities.
>
> - Fixed typo (Segher)
>
> core/Makefile.inc | 2 +-
> core/cpufeatures.c | 921 +++++++++++++++++++++
> core/device.c | 7 +
> core/init.c | 1 +
> .../ibm,powerpc-cpu-features/binding.txt | 245 ++++++
> .../ibm,powerpc-cpu-features/design.txt | 157 ++++
> include/device.h | 1 +
> include/skiboot.h | 5 +
> 8 files changed, 1338 insertions(+), 1 deletion(-)
> create mode 100644 core/cpufeatures.c
> create mode 100644 doc/device-tree/ibm,powerpc-cpu-features/binding.txt
> create mode 100644
> doc/device-tree/ibm,powerpc-cpu-features/design.txt
Merged to master as of 7f4c8e8ce0b78ca046643d7f4f63d81f4fd11746 and has
made it into -rc4
--
Stewart Smith
OPAL Architect, IBM.
^ permalink raw reply
* Re: [PATCH] scsi: cxlflash: Select SCSI_SCAN_ASYNC
From: Matthew R. Ochs @ 2018-02-21 3:23 UTC (permalink / raw)
To: Michael Ellerman
Cc: Vaibhav Jain, Manoj N . Kumar, linux-scsi, Frederic Barrat,
linux-kernel, Uma Krishnan, linuxppc-dev
In-Reply-To: <87sh9wrqd8.fsf@concordia.ellerman.id.au>
On Tue, Feb 20, 2018 at 07:56:35PM +1100, Michael Ellerman wrote:
> Vaibhav Jain <vaibhav@linux.vnet.ibm.com> writes:
>
> > The cxlflash driver uses "Asynchronous SCSI scanning" enabled by
> > CONFIG_SCSI_SCAN_ASYNC. Without this enabled the modprobe of cxlflash
> > module gets hung with following backtrace:
> >
> > Call Trace:
> > __switch_to+0x2cc/0x470
> > __schedule+0x288/0xab0
> > schedule+0x40/0xc0
> > schedule_timeout+0x254/0x4f0
> > wait_for_common+0xdc/0x260
> > flush_work+0x140/0x2a0
> > work_on_cpu+0x88/0xb0
> > pci_device_probe+0x1d0/0x220
> > driver_probe_device+0x408/0x5b0
> > __driver_attach+0x16c/0x1a0
> > bus_for_each_dev+0xb8/0x110
> > driver_attach+0x3c/0x60
> > bus_add_driver+0x1d8/0x370
> > driver_register+0x9c/0x180
> > __pci_register_driver+0x74/0xa0
> > init_cxlflash+0x158/0x1cc
> > do_one_initcall+0x68/0x1e0
> > do_init_module+0x90/0x254
> > load_module+0x2f8c/0x3720
> > SyS_finit_module+0xcc/0x140
> > system_call+0x58/0x6c
>
> Why does it "hang"? That's kind of bizarre, I would expect either a
> build or runtime failure if a feature the driver requires is missing.
>
It hangs due to a bug in the driver. I briefly looked at it several
months back before getting distracted with other items. IIRC there
was an issue with the state machine.
> > diff --git a/drivers/scsi/cxlflash/Kconfig b/drivers/scsi/cxlflash/Kconfig
> > index a011c5dbf214..f054c1b0fff3 100644
> > --- a/drivers/scsi/cxlflash/Kconfig
> > +++ b/drivers/scsi/cxlflash/Kconfig
> > @@ -6,6 +6,7 @@ config CXLFLASH
> > tristate "Support for IBM CAPI Flash"
> > depends on PCI && SCSI && CXL && EEH
> > select IRQ_POLL
> > + select SCSI_SCAN_ASYNC
>
> It's user configurable, so it's rude to select it. It can also be
> disabled on the kernel command line, so this seems like a fragile
> solution.
>
I think Vaibhav's intention here was to avoid the hang while the bug
is still present - I believe he has encountered it several times recently.
The proper solution would be to fix the bug.
^ permalink raw reply
* Re: [PATCH V6] cxl: Fix timebase synchronization status on P9
From: Michael Ellerman @ 2018-02-21 2:43 UTC (permalink / raw)
To: Christophe Lombard, linuxppc-dev, fbarrat, vaibhav,
andrew.donnellan
In-Reply-To: <1519134536-16485-1-git-send-email-clombard@linux.vnet.ibm.com>
Christophe Lombard <clombard@linux.vnet.ibm.com> writes:
> The PSL Timebase register is updated by the PSL to maintain the
> timebase.
> On P9, the Timebase value is only provided by the CAPP as received
> the last time a timebase request was performed.
> The timebase requests are initiated through the adapter configuration or
> application registers.
> The specific sysfs entry "/sys/class/cxl/cardxx/psl_timebase_synced" is
> now dynamically updated according the content of the PSL Timebase
> register.
>
> Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
> Reviewed-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
> Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
Can you give me a Fixes: tag for this?
Does it need to go to stable? I can't tell from the change log how bad
the bug this fixes is.
cheers
> ---
> This patch applies on top of this patch:
> http://patchwork.ozlabs.org/patch/873663/
>
> Changelog[v6]
> - Rebased to latest upstream.
> - Recompute the status only in native mode.
>
> Changelog[v5]
> - Rebased to latest upstream.
> - Changed the type of 'delta'
>
> Changelog[v4]
> - Rebased to latest upstream.
> - Added log message.
>
> Changelog[v3]
> - Rebased to latest upstream.
> - Dynamic update is now applied to P8.
>
> Changelog[v2]
> - Missing Signed-off-by.
> - Spaces required around the ':'.
> ---
> drivers/misc/cxl/pci.c | 17 -----------------
> drivers/misc/cxl/sysfs.c | 12 ++++++++++++
> 2 files changed, 12 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
> index 66eed6a..3247eaf 100644
> --- a/drivers/misc/cxl/pci.c
> +++ b/drivers/misc/cxl/pci.c
> @@ -606,9 +606,6 @@ static u64 timebase_read_xsl(struct cxl *adapter)
>
> static void cxl_setup_psl_timebase(struct cxl *adapter, struct pci_dev *dev)
> {
> - u64 psl_tb;
> - int delta;
> - unsigned int retry = 0;
> struct device_node *np;
>
> adapter->psl_timebase_synced = false;
> @@ -636,20 +633,6 @@ static void cxl_setup_psl_timebase(struct cxl *adapter, struct pci_dev *dev)
> cxl_p1_write(adapter, CXL_PSL_Control, 0x0000000000000000);
> cxl_p1_write(adapter, CXL_PSL_Control, CXL_PSL_Control_tb);
>
> - /* Wait until CORE TB and PSL TB difference <= 16usecs */
> - do {
> - msleep(1);
> - if (retry++ > 5) {
> - dev_info(&dev->dev, "PSL timebase can't synchronize\n");
> - return;
> - }
> - psl_tb = adapter->native->sl_ops->timebase_read(adapter);
> - delta = mftb() - psl_tb;
> - if (delta < 0)
> - delta = -delta;
> - } while (tb_to_ns(delta) > 16000);
> -
> - adapter->psl_timebase_synced = true;
> return;
> }
>
> diff --git a/drivers/misc/cxl/sysfs.c b/drivers/misc/cxl/sysfs.c
> index a8b6d6a..95285b7 100644
> --- a/drivers/misc/cxl/sysfs.c
> +++ b/drivers/misc/cxl/sysfs.c
> @@ -62,7 +62,19 @@ static ssize_t psl_timebase_synced_show(struct device *device,
> char *buf)
> {
> struct cxl *adapter = to_cxl_adapter(device);
> + u64 psl_tb, delta;
>
> + /* Recompute the status only in native mode */
> + if (cpu_has_feature(CPU_FTR_HVMODE)) {
> + psl_tb = adapter->native->sl_ops->timebase_read(adapter);
> + delta = abs(mftb() - psl_tb);
> +
> + /* CORE TB and PSL TB difference <= 16usecs ? */
> + adapter->psl_timebase_synced = (tb_to_ns(delta) < 16000) ? true : false;
> + pr_devel("PSL timebase %s - delta: 0x%016llx\n",
> + (tb_to_ns(delta) < 16000) ? "synchronized" :
> + "not synchronized", tb_to_ns(delta));
> + }
> return scnprintf(buf, PAGE_SIZE, "%i\n", adapter->psl_timebase_synced);
> }
>
> --
> 2.7.4
^ permalink raw reply
* Re: [RFC] powerpc/powernv/mce: Don't silently restart the machine
From: Nicholas Piggin @ 2018-02-21 2:15 UTC (permalink / raw)
To: Balbir Singh; +Cc: linuxppc-dev, mpe
In-Reply-To: <20180221010111.30517-1-bsingharora@gmail.com>
On Wed, 21 Feb 2018 12:01:11 +1100
Balbir Singh <bsingharora@gmail.com> wrote:
> On MCE the current code will restart the machine with
> ppc_md.restart(). This case was extremely unlikely since
> prior to that a skiboot call is made and that resulted in
> a checkstop for analysis.
>
> With newer skiboots, on P9 we don't checkstop the box by
> default, instead we return back to the kernel to extract
> useful information at the time of the MCE. While we still
> get this information, this patch converts the restart to
> a panic(), so that if configured a dump can be taken and
> we can track and probably debug the potential issue causing
> the MCE.
>
> Signed-off-by: Balbir Singh <bsingharora@gmail.com>
Seems like something we should be doing.
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/platforms/powernv/opal.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
> index 69b5263fc9e3..b510a6f41b00 100644
> --- a/arch/powerpc/platforms/powernv/opal.c
> +++ b/arch/powerpc/platforms/powernv/opal.c
> @@ -500,9 +500,12 @@ void pnv_platform_error_reboot(struct pt_regs *regs, const char *msg)
> * opal to trigger checkstop explicitly for error analysis.
> * The FSP PRD component would have already got notified
> * about this error through other channels.
> + * 4. We are running on a newer skiboot that by default does
> + * not cause a checkstop, drops us back to the kernel to
> + * extract context and state at the time of the error.
> */
>
> - ppc_md.restart(NULL);
> + panic("PowerNV Unrecovered Machine Check");
> }
>
> int opal_machine_check(struct pt_regs *regs)
^ permalink raw reply
* Re: [PATCH 1/6] powerpc/mm/32: Use pfn_valid to check if pointer is in RAM
From: christophe leroy @ 2018-02-20 17:45 UTC (permalink / raw)
To: Jonathan Neuschäfer, linuxppc-dev
Cc: linux-kernel, Michael Ellerman, linux-mm, Joel Stanley,
Benjamin Herrenschmidt, Paul Mackerras, Balbir Singh,
Guenter Roeck
In-Reply-To: <20180220161424.5421-2-j.neuschaefer@gmx.net>
Le 20/02/2018 à 17:14, Jonathan Neuschäfer a écrit :
> The Nintendo Wii has a memory layout that places two chunks of RAM at
> non-adjacent addresses, and MMIO between them. Currently, the allocation
> of these MMIO areas is made possible by declaring the MMIO hole as
> reserved memory and allowing reserved memory to be allocated (cf.
> wii_memory_fixups).
>
> This patch is the first step towards proper support for discontiguous
> memory on PPC32 by using pfn_valid to check if a pointer points into
> RAM, rather than open-coding the check. It should result in no
> functional difference.
>
> Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> ---
> arch/powerpc/mm/pgtable_32.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
> index d35d9ad3c1cd..b5c009893a44 100644
> --- a/arch/powerpc/mm/pgtable_32.c
> +++ b/arch/powerpc/mm/pgtable_32.c
> @@ -147,7 +147,7 @@ __ioremap_caller(phys_addr_t addr, unsigned long size, unsigned long flags,
> * Don't allow anybody to remap normal RAM that we're using.
> * mem_init() sets high_memory so only do the check after that.
> */
> - if (slab_is_available() && (p < virt_to_phys(high_memory)) &&
> + if (slab_is_available() && pfn_valid(__phys_to_pfn(p)) &&
I'm not sure this is equivalent:
high_memory = (void *) __va(max_low_pfn * PAGE_SIZE);
#define ARCH_PFN_OFFSET ((unsigned long)(MEMORY_START >> PAGE_SHIFT))
#define pfn_valid(pfn) ((pfn) >= ARCH_PFN_OFFSET && (pfn) < max_mapnr)
set_max_mapnr(max_pfn);
So in the current implementation it checks against max_low_pfn while
your patch checks against max_pfn
max_low_pfn = max_pfn = memblock_end_of_DRAM() >> PAGE_SHIFT;
#ifdef CONFIG_HIGHMEM
max_low_pfn = lowmem_end_addr >> PAGE_SHIFT;
#endif
Christophe
> !(__allow_ioremap_reserved && memblock_is_region_reserved(p, size))) {
> printk("__ioremap(): phys addr 0x%llx is RAM lr %ps\n",
> (unsigned long long)p, __builtin_return_address(0));
>
---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus
^ permalink raw reply
* [RFC] powerpc/powernv/mce: Don't silently restart the machine
From: Balbir Singh @ 2018-02-21 1:01 UTC (permalink / raw)
To: linuxppc-dev; +Cc: mpe, npiggin, Balbir Singh
On MCE the current code will restart the machine with
ppc_md.restart(). This case was extremely unlikely since
prior to that a skiboot call is made and that resulted in
a checkstop for analysis.
With newer skiboots, on P9 we don't checkstop the box by
default, instead we return back to the kernel to extract
useful information at the time of the MCE. While we still
get this information, this patch converts the restart to
a panic(), so that if configured a dump can be taken and
we can track and probably debug the potential issue causing
the MCE.
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
arch/powerpc/platforms/powernv/opal.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index 69b5263fc9e3..b510a6f41b00 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -500,9 +500,12 @@ void pnv_platform_error_reboot(struct pt_regs *regs, const char *msg)
* opal to trigger checkstop explicitly for error analysis.
* The FSP PRD component would have already got notified
* about this error through other channels.
+ * 4. We are running on a newer skiboot that by default does
+ * not cause a checkstop, drops us back to the kernel to
+ * extract context and state at the time of the error.
*/
- ppc_md.restart(NULL);
+ panic("PowerNV Unrecovered Machine Check");
}
int opal_machine_check(struct pt_regs *regs)
--
2.13.6
^ permalink raw reply related
* Re: [PATCH] bpf, powerpc: fix jit for seccomp_data access
From: Daniel Borkmann @ 2018-02-21 0:47 UTC (permalink / raw)
To: Michael Ellerman, Mark Lord, Naveen N. Rao, Alexei Starovoitov,
Kees Cook, Andy Lutomirski, Will Drewry
Cc: linuxppc-dev
In-Reply-To: <87fu5vrxjp.fsf@concordia.ellerman.id.au>
On 02/21/2018 01:33 AM, Michael Ellerman wrote:
> Mark Lord <mlord@pobox.com> writes:
>
>> I am using SECCOMP to filter syscalls on a ppc32 platform,
>> and noticed that the JIT compiler was failing on the BPF
>> even though the interpreter was working fine.
>>
>> The issue was that the compiler was missing one of the instructions
>> used by SECCOMP, so here is a patch to enable JIT for that instruction.
>>
>> Signed-Off-By: Mark Lord <mlord@pobox.com>
>
> Thanks.
>
> What is the failure mode without this patch? I assume when you have the
> JIT enabled the seccomp filter fails to load entirely? Or do we have
> logic to fall back to the interpreter?
The logic for all JITs is that once a BPF insn cannot be JITed then it falls
back to BPF interpreter. Here, since ppc32 is cBPF the path is: cBPF insn ->
cBPF JIT -> fail -> migrate cBPF to eBPF -> run in eBPF interpreter. In the
case where interpreter is compiled out (CONFIG_BPF_JIT_ALWAYS_ON), then the
filter is rejected.
> Either way we should probably back port to stable. I assume this has
> been broken since we originally added 32-bit support, so:
Note that arm32 before it was converted to be an eBPF JIT (eBPF JITs do handle
seccomp BPF in any case) was the only cBPF JIT that had it 'JIT-able', so
currently, other cBPF ones like sparc32 or mips32 don't translate it either.
Meaning, it would be nice to have as feature; I wouldn't necessarily frame
it as a bug though (or at least a stable-urgent one, imho, but I leave that
to you, of course).
> Fixes: eb84bab0fb38 ("ppc: Kconfig: Enable BPF JIT on ppc32")
> Cc: stable@vger.kernel.org # v4.1+
>
> cheers
>
>
>> --- old/arch/powerpc/net/bpf_jit_comp.c 2018-02-16 14:07:01.000000000 -0500
>> +++ linux/arch/powerpc/net/bpf_jit_comp.c 2018-02-20 14:41:20.805227494 -0500
>> @@ -329,6 +329,9 @@ static int bpf_jit_build_body(struct bpf
>> BUILD_BUG_ON(FIELD_SIZEOF(struct sk_buff, len) != 4);
>> PPC_LWZ_OFFS(r_A, r_skb, offsetof(struct sk_buff, len));
>> break;
>> + case BPF_LDX | BPF_W | BPF_ABS: /* A = *((u32 *)(seccomp_data + K)); */
>> + PPC_LWZ_OFFS(r_A, r_skb, K);
>> + break;
>> case BPF_LDX | BPF_W | BPF_LEN: /* X = skb->len; */
>> PPC_LWZ_OFFS(r_X, r_skb, offsetof(struct sk_buff, len));
>> break;
>> --
>> Mark Lord
>> Real-Time Remedies Inc.
>> mlord@pobox.com
^ permalink raw reply
* Re: [PATCH] bpf, powerpc: fix jit for seccomp_data access
From: Michael Ellerman @ 2018-02-21 0:33 UTC (permalink / raw)
To: Mark Lord, Naveen N. Rao, Alexei Starovoitov, Daniel Borkmann,
Kees Cook, Andy Lutomirski, Will Drewry
Cc: linuxppc-dev
In-Reply-To: <2f428cf9-23f3-d1f1-1524-46d6faa17386@pobox.com>
Mark Lord <mlord@pobox.com> writes:
> I am using SECCOMP to filter syscalls on a ppc32 platform,
> and noticed that the JIT compiler was failing on the BPF
> even though the interpreter was working fine.
>
> The issue was that the compiler was missing one of the instructions
> used by SECCOMP, so here is a patch to enable JIT for that instruction.
>
> Signed-Off-By: Mark Lord <mlord@pobox.com>
Thanks.
What is the failure mode without this patch? I assume when you have the
JIT enabled the seccomp filter fails to load entirely? Or do we have
logic to fall back to the interpreter?
Either way we should probably back port to stable. I assume this has
been broken since we originally added 32-bit support, so:
Fixes: eb84bab0fb38 ("ppc: Kconfig: Enable BPF JIT on ppc32")
Cc: stable@vger.kernel.org # v4.1+
cheers
> --- old/arch/powerpc/net/bpf_jit_comp.c 2018-02-16 14:07:01.000000000 -0500
> +++ linux/arch/powerpc/net/bpf_jit_comp.c 2018-02-20 14:41:20.805227494 -0500
> @@ -329,6 +329,9 @@ static int bpf_jit_build_body(struct bpf
> BUILD_BUG_ON(FIELD_SIZEOF(struct sk_buff, len) != 4);
> PPC_LWZ_OFFS(r_A, r_skb, offsetof(struct sk_buff, len));
> break;
> + case BPF_LDX | BPF_W | BPF_ABS: /* A = *((u32 *)(seccomp_data + K)); */
> + PPC_LWZ_OFFS(r_A, r_skb, K);
> + break;
> case BPF_LDX | BPF_W | BPF_LEN: /* X = skb->len; */
> PPC_LWZ_OFFS(r_X, r_skb, offsetof(struct sk_buff, len));
> break;
> --
> Mark Lord
> Real-Time Remedies Inc.
> mlord@pobox.com
^ permalink raw reply
* [PATCH 0/6] DISCONTIGMEM support for PPC32
From: Jonathan Neuschäfer @ 2018-02-20 16:14 UTC (permalink / raw)
To: linuxppc-dev
Cc: linux-kernel, Michael Ellerman, linux-mm, Joel Stanley,
Jonathan Neuschäfer
This patchset adds support for DISCONTIGMEM on 32-bit PowerPC. This is
required to properly support the Nintendo Wii's memory layout, in which
there are two blocks of RAM and MMIO in the middle.
Previously, this memory layout was handled by code that joins the two
RAM blocks into one, reserves the MMIO hole, and permits allocations of
reserved memory in ioremap. This hack didn't work with resource-based
allocation (as used for example in the GPIO driver for Wii[1]), however.
After this patchset, users of the Wii can either select CONFIG_FLATMEM
to get the old behaviour, or CONFIG_DISCONTIGMEM to get the new
behaviour.
Some parts of this patchset are probably not ideal (I'm thinking of my
implementation of pfn_to_nid here), and will require some discussion/
changes.
[1]: https://www.spinics.net/lists/devicetree/msg213956.html
Jonathan Neuschäfer (6):
powerpc/mm/32: Use pfn_valid to check if pointer is in RAM
powerpc: numa: Fix overshift on PPC32
powerpc: numa: Use the right #ifdef guards around functions
powerpc: numa: Restrict fake NUMA enulation to CONFIG_NUMA systems
powerpc: Implement DISCONTIGMEM and allow selection on PPC32
powerpc: wii: Don't rely on reserved memory hack if DISCONTIGMEM is
set
arch/powerpc/Kconfig | 5 ++++-
arch/powerpc/include/asm/mmzone.h | 21 +++++++++++++++++++++
arch/powerpc/mm/numa.c | 18 +++++++++++++++---
arch/powerpc/mm/pgtable_32.c | 2 +-
arch/powerpc/platforms/embedded6xx/wii.c | 10 +++++++---
5 files changed, 48 insertions(+), 8 deletions(-)
--
2.16.1
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox