Re: [PATCH] bpf, powerpc: fix jit for seccomp_data access
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. 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 --- old/arch/powerpc/net/bpf_jit_comp.c 2018-02-16 14:07:01.0 -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; Looks good to me, but I am not able to apply this patch. There seems to be whitespace damage. Please resend the patch considering the steps here: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/email-clients.rst Thanks, Naveen
Re: [PATCH V6] cxl: Fix timebase synchronization status on P9
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
Reviewed-by: Vaibhav Jain
Acked-by: Andrew Donnellan
---
Thanks!
Acked-by: Frederic Barrat
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, 0x);
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);
}
Re: [PATCH] cpufreq: powernv: Check negative value returned by cpufreq_table_find_index_dl()
On Wed, Feb 21, 2018 at 6:54 AM, Viresh Kumar wrote: > On 21-02-18, 16:39, Michael Ellerman wrote: >> Viresh Kumar 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.
Re: [PATCH v9 1/2] powerpc/powernv: Enable tunneled operations
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 --- Reviewed-by: Frederic Barrat 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_SETUP159 #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/a
Re: [PATCH v9 2/2] cxl: read PHB indications from the device tree
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
---
Reviewed-by: Frederic Barrat
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’01001’*/
@@ -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 Compa
Re: [PATCH 00/23] kconfig: move compiler capability tests to Kconfig
On Wed, Feb 21, 2018 at 8:38 AM, Masahiro Yamada wrote: > 2018-02-20 0:18 GMT+09:00 Ulf Magnusson : > >>> >>> 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
Re: [PATCH] cpufreq: powernv: Check negative value returned by cpufreq_table_find_index_dl()
On 21-02-18, 10:27, Rafael J. Wysocki wrote: > 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. I don't want to end up doing double checking as some of it is already done at init, but let me check on what can be done. -- viresh
Re: [PATCH] cpufreq: powernv: Check negative value returned by cpufreq_table_find_index_dl()
On Wed, Feb 21, 2018 at 11:02 AM, Viresh Kumar wrote: > On 21-02-18, 10:27, Rafael J. Wysocki wrote: >> 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. > > I don't want to end up doing double checking as some of it is already > done at init, but let me check on what can be done. The driver is expected to call cpufreq_table_validate_and_show() at ->init() time and fail ->init() if that fails. That's kind of fragile, because it depends on the driver to do the right thing.
Re: [PATCH] cpufreq: powernv: Check negative value returned by cpufreq_table_find_index_dl()
On 21-02-18, 11:17, Rafael J. Wysocki wrote: > The driver is expected to call cpufreq_table_validate_and_show() at > ->init() time and fail ->init() if that fails. > > That's kind of fragile, because it depends on the driver to do the right > thing. That's exactly what I am trying to explore here, i.e. Call validate/show from core instead of drivers. -- viresh
Re: [PATCH 00/23] kconfig: move compiler capability tests to Kconfig
2018-02-21 18:56 GMT+09:00 Arnd Bergmann :
> On Wed, Feb 21, 2018 at 8:38 AM, Masahiro Yamada
> wrote:
>> 2018-02-20 0:18 GMT+09:00 Ulf Magnusson :
>>
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
Let me clarify my concern.
When we test the compiler flag, is there a case
where a particular flag depends on -m{32,64} ?
For example, is there a compiler that supports -fstack-protector
for 64bit mode, but unsupports it for 32bit mode?
$(cc-option -m32) -> y
$(cc-option -m64) -> y
$(cc-option -fstack-protector)-> y
$(cc-option -m32 -fstack-protector) -> n
$(cc-option -m64 -fstack-protector) -> y
I guess this is unlikely to happen,
but I am not whether it is zero possibility.
If this could happen,
$(cc-option ) must be evaluated together with
correct bi-arch option (either -m32 or -m64).
Currently, -m32/-m64 is specified in Makefile,
but we are moving compiler tests to Kconfig
and, CONFIG_64BIT can be dynamically toggled in Kconfig.
--
Best Regards
Masahiro Yamada
Re: [PATCH V6] cxl: Fix timebase synchronization status on P9
Le 21/02/2018 à 03:43, Michael Ellerman a écrit :
Christophe Lombard 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
Reviewed-by: Vaibhav Jain
Acked-by: Andrew Donnellan
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.
Sorry, I completely forgot to add it:
Fixes: f24be42aab37 ("cxl: Add psl9 specific code")
With the current code, the state of the PSL timebase, on P9, is not
displayed correctly because the feature works differently from
what was expected.
This patch fixes this behavior and introduces a new way to get
dynamically the state of the PSL timebase.
Thanks
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, 0x);
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
[RFC PATCH v0 1/2] powerpc, drmem: Fix unexpected flag value in ibm, dynamic-memory-v2
Memory addtion and removal by count and indexed-count methods
temporarily mark the LMBs that are being added/removed by a special
flag value DRMEM_LMB_RESERVED. Accessing flags value directly at
a few places without proper accessor method is causing two unexpected
side-effects:
- DRMEM_LMB_RESERVED bit is becoming part of the flags word of
drconf_cell_v2 entries in ibm,dynamic-memory-v2 DT property.
- This results in extra drconf_cell entries in ibm,dynamic-memory-v2.
For example if 1G memory is added, it leads to one entry for 3 LMBs
and 1 separate entry for the last LMB. All the 4 LMBs should be
defined by one entry here.
Fix this by always accessing the flags by its accessor method
drmem_lmb_flags().
Signed-off-by: Bharata B Rao
---
arch/powerpc/mm/drmem.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
index 916844f..3f18036 100644
--- a/arch/powerpc/mm/drmem.c
+++ b/arch/powerpc/mm/drmem.c
@@ -98,7 +98,7 @@ static void init_drconf_v2_cell(struct of_drconf_cell_v2
*dr_cell,
dr_cell->base_addr = cpu_to_be64(lmb->base_addr);
dr_cell->drc_index = cpu_to_be32(lmb->drc_index);
dr_cell->aa_index = cpu_to_be32(lmb->aa_index);
- dr_cell->flags = cpu_to_be32(lmb->flags);
+ dr_cell->flags = cpu_to_be32(drmem_lmb_flags(lmb));
}
static int drmem_update_dt_v2(struct device_node *memory,
@@ -121,7 +121,7 @@ static int drmem_update_dt_v2(struct device_node *memory,
}
if (prev_lmb->aa_index != lmb->aa_index ||
- prev_lmb->flags != lmb->flags)
+ drmem_lmb_flags(prev_lmb) != drmem_lmb_flags(lmb))
lmb_sets++;
prev_lmb = lmb;
@@ -150,7 +150,7 @@ static int drmem_update_dt_v2(struct device_node *memory,
}
if (prev_lmb->aa_index != lmb->aa_index ||
- prev_lmb->flags != lmb->flags) {
+ drmem_lmb_flags(prev_lmb) != drmem_lmb_flags(lmb)) {
/* end of one set, start of another */
dr_cell->seq_lmbs = cpu_to_be32(seq_lmbs);
dr_cell++;
--
2.7.4
[RFC PATCH v0 2/2] powerpc, drmem: Rename DRMEM_LMB_RESERVED to DRMEM_LMB_ISOLATED
Memory hotplug code uses a temporary LMB flags bit DRMEM_LMB_RESERVED
to mark the LMB which is currently undergoing hotplug or unplug.
It is easy to confuse DRMEM_LMB_RESERVED to mean the LMB is reserved
for which a separate flag bit already exists DRCONF_MEM_RESERVED. Since
both DRMEM_LMB_RESERVED and DRCONF_MEM_RESERVED operate on the same
LMB flags word, rename the former to DRMEM_LMB_ISOLATED.
Signed-off-by: Bharata B Rao
---
arch/powerpc/include/asm/drmem.h| 14 -
arch/powerpc/mm/drmem.c | 2 +-
arch/powerpc/platforms/pseries/hotplug-memory.c | 40 -
3 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/arch/powerpc/include/asm/drmem.h b/arch/powerpc/include/asm/drmem.h
index ce242b9..b3fa3f7 100644
--- a/arch/powerpc/include/asm/drmem.h
+++ b/arch/powerpc/include/asm/drmem.h
@@ -72,21 +72,21 @@ static inline u32 drmem_lmb_size(void)
return drmem_info->lmb_size;
}
-#define DRMEM_LMB_RESERVED 0x8000
+#define DRMEM_LMB_ISOLATED 0x8000
-static inline void drmem_mark_lmb_reserved(struct drmem_lmb *lmb)
+static inline void drmem_mark_lmb_isolated(struct drmem_lmb *lmb)
{
- lmb->flags |= DRMEM_LMB_RESERVED;
+ lmb->flags |= DRMEM_LMB_ISOLATED;
}
-static inline void drmem_remove_lmb_reservation(struct drmem_lmb *lmb)
+static inline void drmem_remove_lmb_isolation(struct drmem_lmb *lmb)
{
- lmb->flags &= ~DRMEM_LMB_RESERVED;
+ lmb->flags &= ~DRMEM_LMB_ISOLATED;
}
-static inline bool drmem_lmb_reserved(struct drmem_lmb *lmb)
+static inline bool drmem_lmb_isolated(struct drmem_lmb *lmb)
{
- return lmb->flags & DRMEM_LMB_RESERVED;
+ return lmb->flags & DRMEM_LMB_ISOLATED;
}
u64 drmem_lmb_memory_max(void);
diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
index 3f18036..652bf3a 100644
--- a/arch/powerpc/mm/drmem.c
+++ b/arch/powerpc/mm/drmem.c
@@ -35,7 +35,7 @@ static u32 drmem_lmb_flags(struct drmem_lmb *lmb)
* Return the value of the lmb flags field minus the reserved
* bit used internally for hotplug processing.
*/
- return lmb->flags & ~DRMEM_LMB_RESERVED;
+ return lmb->flags & ~DRMEM_LMB_ISOLATED;
}
static struct property *clone_property(struct property *prop, u32 prop_sz)
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c
b/arch/powerpc/platforms/pseries/hotplug-memory.c
index c1578f5..2f5ca29 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -467,7 +467,7 @@ static int dlpar_memory_remove_by_count(u32 lmbs_to_remove)
/* Mark this lmb so we can add it later if all of the
* requested LMBs cannot be removed.
*/
- drmem_mark_lmb_reserved(lmb);
+ drmem_mark_lmb_isolated(lmb);
lmbs_removed++;
if (lmbs_removed == lmbs_to_remove)
@@ -478,7 +478,7 @@ static int dlpar_memory_remove_by_count(u32 lmbs_to_remove)
pr_err("Memory hot-remove failed, adding LMB's back\n");
for_each_drmem_lmb(lmb) {
- if (!drmem_lmb_reserved(lmb))
+ if (!drmem_lmb_isolated(lmb))
continue;
rc = dlpar_add_lmb(lmb);
@@ -486,20 +486,20 @@ static int dlpar_memory_remove_by_count(u32
lmbs_to_remove)
pr_err("Failed to add LMB back, drc index %x\n",
lmb->drc_index);
- drmem_remove_lmb_reservation(lmb);
+ drmem_remove_lmb_isolation(lmb);
}
rc = -EINVAL;
} else {
for_each_drmem_lmb(lmb) {
- if (!drmem_lmb_reserved(lmb))
+ if (!drmem_lmb_isolated(lmb))
continue;
dlpar_release_drc(lmb->drc_index);
pr_info("Memory at %llx was hot-removed\n",
lmb->base_addr);
- drmem_remove_lmb_reservation(lmb);
+ drmem_remove_lmb_isolation(lmb);
}
rc = 0;
}
@@ -608,7 +608,7 @@ static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove,
u32 drc_index)
if (rc)
break;
- drmem_mark_lmb_reserved(lmb);
+ drmem_mark_lmb_isolated(lmb);
}
if (rc) {
@@ -616,7 +616,7 @@ static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove,
u32 drc_index)
for_each_drmem_lmb_in_range(lmb, start_lmb, end_lmb) {
- if (!drmem_lmb_reserved(lmb))
+ if (!drmem_lmb_isolated(lmb))
continue;
rc = dlpar_add_lmb(lmb);
@@ -624,19 +624,19 @@ static int dlp
[RFC PATCH v0 0/2] ibm,dynamic-memory-v2 fix
Patch 1 fixes a bug that results in unexpected flag bit in ibm,dynamic-memory-v2 DT property and wrong number of entries getting created in the same property during hotplug. Patch 2 can be optionally considered. It renames DRMEM_LMB_RESERVED to DRMEM_LMB_ISOLATED. May be it could be renamed to even more appropriate name. Bharata B Rao (2): powerpc,drmem: Fix unexpected flag value in ibm,dynamic-memory-v2 powerpc,drmem: Rename DRMEM_LMB_RESERVED to DRMEM_LMB_ISOLATED arch/powerpc/include/asm/drmem.h| 14 - arch/powerpc/mm/drmem.c | 8 ++--- arch/powerpc/platforms/pseries/hotplug-memory.c | 40 - 3 files changed, 31 insertions(+), 31 deletions(-) -- 2.7.4
Re: [PATCH 00/23] kconfig: move compiler capability tests to Kconfig
On Wed, Feb 21, 2018 at 11:20 AM, Masahiro Yamada
wrote:
> 2018-02-21 18:56 GMT+09:00 Arnd Bergmann :
>> On Wed, Feb 21, 2018 at 8:38 AM, Masahiro Yamada
>> wrote:
>>> 2018-02-20 0:18 GMT+09:00 Ulf Magnusson :
>
> Let me clarify my concern.
>
> When we test the compiler flag, is there a case
> where a particular flag depends on -m{32,64} ?
>
> For example, is there a compiler that supports -fstack-protector
> for 64bit mode, but unsupports it for 32bit mode?
>
> $(cc-option -m32) -> y
> $(cc-option -m64) -> y
> $(cc-option -fstack-protector)-> y
> $(cc-option -m32 -fstack-protector) -> n
> $(cc-option -m64 -fstack-protector) -> y
>
> I guess this is unlikely to happen,
> but I am not whether it is zero possibility.
>
> If this could happen,
> $(cc-option ) must be evaluated together with
> correct bi-arch option (either -m32 or -m64).
>
>
> Currently, -m32/-m64 is specified in Makefile,
> but we are moving compiler tests to Kconfig
> and, CONFIG_64BIT can be dynamically toggled in Kconfig.
I don't think it can happen for this particular combination (stack protector
and word size), but I'm sure we'll eventually run into options that
need to be tested in combination. For the current CFLAGS_KERNEL
setting, we definitely have the case of needing the variables to be
evaluated in a specific order.
Arnd
Re: [PATCH] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
Le 21/02/2018 à 07:43, Balbir Singh a écrit :
On Wed, Feb 21, 2018 at 3:57 PM, Alastair D'Silva wrote:
From: Alastair D'Silva
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
---
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?
My bad, I forgot to mention it before. There's a somewhat high-level
description which needs updating in:
Documentation/accelerators/ocxl.rst
It doesn't go down to the level of the structure members, but at least
all ioctl commands should have a brief description.
lpc_size could be added. It's currently useless to the library, but
doesn't hurt. The one which was giving me troubles on a previous version
of this patch was the lpc numa node ID, since that was experimental code
and felt out of place considering what's been upstreamed in skiboot and
linux so far.
Fred
+ // End version 0 fields
+
+ __u64 reserved[13]; // Total of 16*u64
+};
Balbir Singh.
Re: [PATCH 3/9] powerpc/64s: kbuild set assembler machine type to POWER4
Hi Nicholas, Thank you for the patch! Yet something to improve: [auto build test ERROR on powerpc/next] [also build test ERROR on v4.16-rc2 next-20180221] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Nicholas-Piggin/powerpc-cpu-feature-and-kernel-build-stuff/20180221-132025 base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next config: powerpc-ppc64e_defconfig (attached as .config) compiler: powerpc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=powerpc All errors (new ones prefixed by >>): arch/powerpc/lib/copypage_power7.S: Assembler messages: >> arch/powerpc/lib/copypage_power7.S:46: Error: junk at end of line: `0b01000' >> arch/powerpc/lib/copypage_power7.S:47: Error: junk at end of line: `0b01010' arch/powerpc/lib/copypage_power7.S:49: Error: junk at end of line: `0b01000' arch/powerpc/lib/copypage_power7.S:50: Error: junk at end of line: `0b01010' arch/powerpc/lib/copypage_power7.S:52: Error: junk at end of line: `0b01010' vim +46 arch/powerpc/lib/copypage_power7.S fde69282 Anton Blanchard 2012-05-29 22 fde69282 Anton Blanchard 2012-05-29 23 _GLOBAL(copypage_power7) fde69282 Anton Blanchard 2012-05-29 24 /* fde69282 Anton Blanchard 2012-05-29 25 * We prefetch both the source and destination using enhanced touch fde69282 Anton Blanchard 2012-05-29 26 * instructions. We use a stream ID of 0 for the load side and fde69282 Anton Blanchard 2012-05-29 27 * 1 for the store side. Since source and destination are page fde69282 Anton Blanchard 2012-05-29 28 * aligned we don't need to clear the bottom 7 bits of either fde69282 Anton Blanchard 2012-05-29 29 * address. fde69282 Anton Blanchard 2012-05-29 30 */ 280a5ba2 Michael Neuling 2013-05-29 31 ori r9,r3,1 /* stream=1 => to */ fde69282 Anton Blanchard 2012-05-29 32 fde69282 Anton Blanchard 2012-05-29 33 #ifdef CONFIG_PPC_64K_PAGES 280a5ba2 Michael Neuling 2013-05-29 34 lis r7,0x0E01 /* depth=7 280a5ba2 Michael Neuling 2013-05-29 35 * units/cachelines=512 */ fde69282 Anton Blanchard 2012-05-29 36 #else fde69282 Anton Blanchard 2012-05-29 37 lis r7,0x0E00 /* depth=7 */ 280a5ba2 Michael Neuling 2013-05-29 38 ori r7,r7,0x1000/* units/cachelines=32 */ fde69282 Anton Blanchard 2012-05-29 39 #endif fde69282 Anton Blanchard 2012-05-29 40 ori r10,r7,1/* stream=1 */ fde69282 Anton Blanchard 2012-05-29 41 fde69282 Anton Blanchard 2012-05-29 42 lis r8,0x8000 /* GO=1 */ fde69282 Anton Blanchard 2012-05-29 43 clrldi r8,r8,32 fde69282 Anton Blanchard 2012-05-29 44 280a5ba2 Michael Neuling 2013-05-29 45 /* setup read stream 0 */ 8a583c0a Andreas Schwab 2017-08-05 @46 dcbt0,r4,0b01000/* addr from */ 8a583c0a Andreas Schwab 2017-08-05 @47 dcbt0,r7,0b01010 /* length and depth from */ 280a5ba2 Michael Neuling 2013-05-29 48 /* setup write stream 1 */ 8a583c0a Andreas Schwab 2017-08-05 49 dcbtst 0,r9,0b01000 /* addr to */ 8a583c0a Andreas Schwab 2017-08-05 50 dcbtst 0,r10,0b01010 /* length and depth to */ fde69282 Anton Blanchard 2012-05-29 51 eieio 8a583c0a Andreas Schwab 2017-08-05 52 dcbt0,r8,0b01010/* all streams GO */ fde69282 Anton Blanchard 2012-05-29 53 :: The code at line 46 was first introduced by commit :: 8a583c0a8d316d8ea52ea78491174ab1a3e9ef9d powerpc: Fix invalid use of register expressions :: TO: Andreas Schwab :: CC: Michael Ellerman --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [RFC PATCH v0 1/2] powerpc, drmem: Fix unexpected flag value in ibm, dynamic-memory-v2
Bharata B Rao writes:
> Memory addtion and removal by count and indexed-count methods
> temporarily mark the LMBs that are being added/removed by a special
> flag value DRMEM_LMB_RESERVED. Accessing flags value directly at
> a few places without proper accessor method is causing two unexpected
> side-effects:
>
> - DRMEM_LMB_RESERVED bit is becoming part of the flags word of
> drconf_cell_v2 entries in ibm,dynamic-memory-v2 DT property.
> - This results in extra drconf_cell entries in ibm,dynamic-memory-v2.
> For example if 1G memory is added, it leads to one entry for 3 LMBs
> and 1 separate entry for the last LMB. All the 4 LMBs should be
> defined by one entry here.
>
> Fix this by always accessing the flags by its accessor method
> drmem_lmb_flags().
>
> Signed-off-by: Bharata B Rao
Presumably:
Fixes: 2b31e3aec1db ("powerpc/drmem: Add support for ibm, dynamic-memory-v2
property")
?
cheers
Re: [PATCH 3/9] powerpc/64s: kbuild set assembler machine type to POWER4
So this shouldn't be built on BookE because it generates the wrong code for them. BookS CPUs are the only ones that set this FTR bit anyway. Thanks, Nick On Wed, 21 Feb 2018 19:15:29 +0800 kbuild test robot wrote: > Hi Nicholas, > > Thank you for the patch! Yet something to improve: > > [auto build test ERROR on powerpc/next] > [also build test ERROR on v4.16-rc2 next-20180221] > [if your patch is applied to the wrong git tree, please drop us a note to > help improve the system] > > url: > https://github.com/0day-ci/linux/commits/Nicholas-Piggin/powerpc-cpu-feature-and-kernel-build-stuff/20180221-132025 > base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next > config: powerpc-ppc64e_defconfig (attached as .config) > compiler: powerpc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 > reproduce: > wget > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O > ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > make.cross ARCH=powerpc > > All errors (new ones prefixed by >>): > >arch/powerpc/lib/copypage_power7.S: Assembler messages: > >> arch/powerpc/lib/copypage_power7.S:46: Error: junk at end of line: > >> `0b01000' > >> arch/powerpc/lib/copypage_power7.S:47: Error: junk at end of line: > >> `0b01010' >arch/powerpc/lib/copypage_power7.S:49: Error: junk at end of line: > `0b01000' >arch/powerpc/lib/copypage_power7.S:50: Error: junk at end of line: > `0b01010' >arch/powerpc/lib/copypage_power7.S:52: Error: junk at end of line: > `0b01010' > > vim +46 arch/powerpc/lib/copypage_power7.S > > fde69282 Anton Blanchard 2012-05-29 22 > fde69282 Anton Blanchard 2012-05-29 23 _GLOBAL(copypage_power7) > fde69282 Anton Blanchard 2012-05-29 24 /* > fde69282 Anton Blanchard 2012-05-29 25* We prefetch both the source > and destination using enhanced touch > fde69282 Anton Blanchard 2012-05-29 26* instructions. We use a > stream ID of 0 for the load side and > fde69282 Anton Blanchard 2012-05-29 27* 1 for the store side. Since > source and destination are page > fde69282 Anton Blanchard 2012-05-29 28* aligned we don't need to > clear the bottom 7 bits of either > fde69282 Anton Blanchard 2012-05-29 29* address. > fde69282 Anton Blanchard 2012-05-29 30*/ > 280a5ba2 Michael Neuling 2013-05-29 31 ori r9,r3,1 /* > stream=1 => to */ > fde69282 Anton Blanchard 2012-05-29 32 > fde69282 Anton Blanchard 2012-05-29 33 #ifdef CONFIG_PPC_64K_PAGES > 280a5ba2 Michael Neuling 2013-05-29 34 lis r7,0x0E01 /* > depth=7 > 280a5ba2 Michael Neuling 2013-05-29 35* > units/cachelines=512 */ > fde69282 Anton Blanchard 2012-05-29 36 #else > fde69282 Anton Blanchard 2012-05-29 37 lis r7,0x0E00 /* > depth=7 */ > 280a5ba2 Michael Neuling 2013-05-29 38 ori r7,r7,0x1000/* > units/cachelines=32 */ > fde69282 Anton Blanchard 2012-05-29 39 #endif > fde69282 Anton Blanchard 2012-05-29 40 ori r10,r7,1/* > stream=1 */ > fde69282 Anton Blanchard 2012-05-29 41 > fde69282 Anton Blanchard 2012-05-29 42 lis r8,0x8000 /* GO=1 > */ > fde69282 Anton Blanchard 2012-05-29 43 clrldi r8,r8,32 > fde69282 Anton Blanchard 2012-05-29 44 > 280a5ba2 Michael Neuling 2013-05-29 45 /* setup read stream 0 */ > 8a583c0a Andreas Schwab 2017-08-05 @46 dcbt0,r4,0b01000/* addr > from */ > 8a583c0a Andreas Schwab 2017-08-05 @47 dcbt0,r7,0b01010 /* > length and depth from */ > 280a5ba2 Michael Neuling 2013-05-29 48 /* setup write stream 1 */ > 8a583c0a Andreas Schwab 2017-08-05 49 dcbtst 0,r9,0b01000 /* addr > to */ > 8a583c0a Andreas Schwab 2017-08-05 50 dcbtst 0,r10,0b01010 /* > length and depth to */ > fde69282 Anton Blanchard 2012-05-29 51 eieio > 8a583c0a Andreas Schwab 2017-08-05 52 dcbt0,r8,0b01010/* all > streams GO */ > fde69282 Anton Blanchard 2012-05-29 53 > > :: The code at line 46 was first introduced by commit > :: 8a583c0a8d316d8ea52ea78491174ab1a3e9ef9d powerpc: Fix invalid use of > register expressions > > :: TO: Andreas Schwab > :: CC: Michael Ellerman > > --- > 0-DAY kernel test infrastructureOpen Source Technology Center > https://lists.01.org/pipermail/kbuild-all Intel Corporation
Re: [RFC PATCH v0 0/2] ibm,dynamic-memory-v2 fix
On Wed, Feb 21, 2018 at 9:36 PM, Bharata B Rao wrote: > Patch 1 fixes a bug that results in unexpected flag bit in > ibm,dynamic-memory-v2 DT property and wrong number of entries > getting created in the same property during hotplug. > Could you please elaborate on what this means? Is there a test case - how do we reproduce this? Balbir Singh.
Re: [PATCH v3] watchdog: add SPDX identifiers for watchdog subsystem
On Wed, Feb 21, 2018 at 01:27:34PM +0100, Marcus Folkesson wrote: >- Add SPDX identifier >- Remove boiler plate license text >- If MODULE_LICENSE and boiler plate does not match, go for boiler plate > license > >Signed-off-by: Marcus Folkesson >Acked-by: Adam Thomson >Acked-by: Charles Keepax >Acked-by: Mans Rullgard >Acked-by: Matthias Brugger >Acked-by: Michal Simek >Acked-by: Neil Armstrong >Acked-by: Nicolas Ferre >Acked-by: Thierry Reding >Reviewed-by: Eric Anholt >--- > >Notes: >v3: > - Keep license text for ebc-c384_wdt >v2: > - Put back removed copyright texts for meson_gxbb_wdt and coh901327_wdt > - Change to BSD-3-Clause for meson_gxbb_wdt >v1: Please have an extra look at meson_gxbb_wdt.c [...] >diff --git a/drivers/watchdog/ebc-c384_wdt.c b/drivers/watchdog/ebc-c384_wdt.c >index 2170b275ea01..4c4c8ce78021 100644 >--- a/drivers/watchdog/ebc-c384_wdt.c >+++ b/drivers/watchdog/ebc-c384_wdt.c >@@ -1,3 +1,4 @@ >+// SPDX-License-Identifier: GPL-2.0 > /* > * Watchdog timer driver for the WinSystems EBC-C384 > * Copyright (C) 2016 William Breathitt Gray For ebc-c384_wdt.c, Acked-by: William Breathitt Gray William Breathitt Gray
Re: [PATCH] bpf, powerpc: fix jit for seccomp_data access
On 18-02-21 03:35 AM, Naveen N. Rao wrote: > 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. >> >> 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 >> >> --- old/arch/powerpc/net/bpf_jit_comp.c 2018-02-16 14:07:01.0 -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; > > Looks good to me, but I am not able to apply this patch. There seems to be > whitespace damage. Here (attached) is a clean copy. -- Mark Lord Real-Time Remedies Inc. ml...@pobox.com --- old/arch/powerpc/net/bpf_jit_comp.c 2018-02-16 14:07:01.0 -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;
Re: [PATCH v3] watchdog: add SPDX identifiers for watchdog subsystem
Hi Marcus,
On Wed, Feb 21, 2018 at 01:27:34PM +0100, Marcus Folkesson wrote:
> - Add SPDX identifier
> - Remove boiler plate license text
> - If MODULE_LICENSE and boiler plate does not match, go for boiler plate
> license
>
> Signed-off-by: Marcus Folkesson
> Acked-by: Adam Thomson
> Acked-by: Charles Keepax
> Acked-by: Mans Rullgard
> Acked-by: Matthias Brugger
> Acked-by: Michal Simek
> Acked-by: Neil Armstrong
> Acked-by: Nicolas Ferre
> Acked-by: Thierry Reding
> Reviewed-by: Eric Anholt
[...]
> diff --git a/drivers/watchdog/digicolor_wdt.c
> b/drivers/watchdog/digicolor_wdt.c
> index 5e4ef93caa02..a9e11df155b8 100644
> --- a/drivers/watchdog/digicolor_wdt.c
> +++ b/drivers/watchdog/digicolor_wdt.c
> @@ -1,12 +1,9 @@
> +// SPDX-License-Identifier: GPL-2.0+
> /*
> * Watchdog driver for Conexant Digicolor
> *
> * Copyright (C) 2015 Paradox Innovation Ltd.
> *
> - * This program is free software; you can redistribute it and/or modify it
> - * under the terms of the GNU General Public License as published by the
> - * Free Software Foundation; either version 2 of the License, or (at your
> - * option) any later version.
> */
For digicolor_wdt.c:
Acked-by: Baruch Siach
baruch
--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}ooO--U--Ooo{=
- bar...@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
Re: [PATCH 00/23] kconfig: move compiler capability tests to Kconfig
2018-02-21 19:52 GMT+09:00 Arnd Bergmann :
> On Wed, Feb 21, 2018 at 11:20 AM, Masahiro Yamada
> wrote:
>> 2018-02-21 18:56 GMT+09:00 Arnd Bergmann :
>>> On Wed, Feb 21, 2018 at 8:38 AM, Masahiro Yamada
>>> wrote:
2018-02-20 0:18 GMT+09:00 Ulf Magnusson :
>>
>> Let me clarify my concern.
>>
>> When we test the compiler flag, is there a case
>> where a particular flag depends on -m{32,64} ?
>>
>> For example, is there a compiler that supports -fstack-protector
>> for 64bit mode, but unsupports it for 32bit mode?
>>
>> $(cc-option -m32) -> y
>> $(cc-option -m64) -> y
>> $(cc-option -fstack-protector)-> y
>> $(cc-option -m32 -fstack-protector) -> n
>> $(cc-option -m64 -fstack-protector) -> y
>>
>> I guess this is unlikely to happen,
>> but I am not whether it is zero possibility.
>>
>> If this could happen,
>> $(cc-option ) must be evaluated together with
>> correct bi-arch option (either -m32 or -m64).
>>
>>
>> Currently, -m32/-m64 is specified in Makefile,
>> but we are moving compiler tests to Kconfig
>> and, CONFIG_64BIT can be dynamically toggled in Kconfig.
>
> I don't think it can happen for this particular combination (stack protector
> and word size), but I'm sure we'll eventually run into options that
> need to be tested in combination. For the current CFLAGS_KERNEL
> setting, we definitely have the case of needing the variables to be
> evaluated in a specific order.
>
I was thinking of how we can handle complex cases
in the current approach.
(Case 1)
Compiler flag -foo and -bar interacts, so
we also need to check the combination of the two.
config CC_HAS_FOO
def_bool $(cc-option -foo)
config CC_HAS_BAR
def_bool $(cc-option -bar)
config CC_HAS_FOO_WITH_BAR
def_bool $(cc-option -foo -bar)
(Case 2)
Compiler flag -foo is sensitive to word-size.
So, we need to test this option together with -m32/-m64.
User can toggle CONFIG_64BIT, like i386/x86_64.
config CC_NEEDS_M64
def_bool $(cc-option -m64) && 64BIT
config CC_NEEDS_M32
def_bool $(cc-option -m32) && !64BIT
config CC_HAS_FOO
bool
default $(cc-option -m64 -foo) if CC_NEEDS_M64
default $(cc-option -m32 -foo) if CC_NEEDS_M32
default $(cc-option -foo)
(Case 3)
Compiler flag -foo is sensitive to endian-ness.
config CC_NEEDS_BIG_ENDIAN
def_bool $(cc-option -mbig-endian) && CPU_BIG_ENDIAN
config CC_NEEDS_LITTLE_ENDIAN
def_bool $(cc-option -mlittle-endian) && CPU_LITTLE_ENDIAN
config CC_HAS_FOO
bool
default $(cc-option -mbig-endian -foo) if CC_NEEDS_BIG_ENDIAN
default $(cc-option -mlittle-endian -foo) if CC_NEEDS_LITTLE_ENDIAN
default $(cc-option -foo)
Hmm, I think I can implement those somehow.
But, I hope we do not have many instances like this...
If you know more naive cases, please share your knowledge.
Thanks!
--
Best Regards
Masahiro Yamada
[PATCH v3] watchdog: add SPDX identifiers for watchdog subsystem
- Add SPDX identifier - Remove boiler plate license text - If MODULE_LICENSE and boiler plate does not match, go for boiler plate license Signed-off-by: Marcus Folkesson Acked-by: Adam Thomson Acked-by: Charles Keepax Acked-by: Mans Rullgard Acked-by: Matthias Brugger Acked-by: Michal Simek Acked-by: Neil Armstrong Acked-by: Nicolas Ferre Acked-by: Thierry Reding Reviewed-by: Eric Anholt --- Notes: v3: - Keep license text for ebc-c384_wdt v2: - Put back removed copyright texts for meson_gxbb_wdt and coh901327_wdt - Change to BSD-3-Clause for meson_gxbb_wdt v1: Please have an extra look at meson_gxbb_wdt.c drivers/watchdog/acquirewdt.c | 6 +--- drivers/watchdog/advantechwdt.c| 6 +--- drivers/watchdog/alim1535_wdt.c| 6 +--- drivers/watchdog/alim7101_wdt.c| 1 + drivers/watchdog/ar7_wdt.c | 14 +- drivers/watchdog/asm9260_wdt.c | 2 +- drivers/watchdog/aspeed_wdt.c | 5 +--- drivers/watchdog/at91rm9200_wdt.c | 5 +--- drivers/watchdog/at91sam9_wdt.c| 5 +--- drivers/watchdog/at91sam9_wdt.h| 5 +--- drivers/watchdog/ath79_wdt.c | 4 +-- drivers/watchdog/atlas7_wdt.c | 2 +- drivers/watchdog/bcm2835_wdt.c | 5 +--- drivers/watchdog/bcm47xx_wdt.c | 5 +--- drivers/watchdog/bcm63xx_wdt.c | 5 +--- drivers/watchdog/bcm7038_wdt.c | 12 ++-- drivers/watchdog/bcm_kona_wdt.c| 9 +- drivers/watchdog/bfin_wdt.c| 2 +- drivers/watchdog/booke_wdt.c | 5 +--- drivers/watchdog/cadence_wdt.c | 5 +--- drivers/watchdog/coh901327_wdt.c | 4 +-- drivers/watchdog/cpu5wdt.c | 15 +- drivers/watchdog/cpwd.c| 1 + drivers/watchdog/da9052_wdt.c | 6 +--- drivers/watchdog/da9055_wdt.c | 6 +--- drivers/watchdog/da9062_wdt.c | 10 +-- drivers/watchdog/da9063_wdt.c | 5 +--- drivers/watchdog/davinci_wdt.c | 7 ++--- drivers/watchdog/diag288_wdt.c | 1 + drivers/watchdog/digicolor_wdt.c | 5 +--- drivers/watchdog/dw_wdt.c | 6 +--- drivers/watchdog/ebc-c384_wdt.c| 1 + drivers/watchdog/ep93xx_wdt.c | 7 ++--- drivers/watchdog/eurotechwdt.c | 6 +--- drivers/watchdog/f71808e_wdt.c | 16 +-- drivers/watchdog/ftwdt010_wdt.c| 6 ++-- drivers/watchdog/gef_wdt.c | 6 +--- drivers/watchdog/geodewdt.c| 5 +--- drivers/watchdog/gpio_wdt.c| 5 +--- drivers/watchdog/hpwdt.c | 7 ++--- drivers/watchdog/i6300esb.c| 6 +--- drivers/watchdog/iTCO_vendor_support.c | 9 +- drivers/watchdog/iTCO_wdt.c| 10 +-- drivers/watchdog/ib700wdt.c| 6 +--- drivers/watchdog/ibmasr.c | 3 +- drivers/watchdog/ie6xx_wdt.c | 18 ++-- drivers/watchdog/imgpdc_wdt.c | 5 +--- drivers/watchdog/imx2_wdt.c| 5 +--- drivers/watchdog/indydog.c | 6 +--- drivers/watchdog/intel-mid_wdt.c | 6 ++-- drivers/watchdog/intel_scu_watchdog.c | 18 ++-- drivers/watchdog/intel_scu_watchdog.h | 16 +-- drivers/watchdog/iop_wdt.c | 16 ++- drivers/watchdog/it8712f_wdt.c | 10 +-- drivers/watchdog/it87_wdt.c| 10 +-- drivers/watchdog/ixp4xx_wdt.c | 6 ++-- drivers/watchdog/jz4740_wdt.c | 10 +-- drivers/watchdog/kempld_wdt.c | 12 ++-- drivers/watchdog/ks8695_wdt.c | 6 ++-- drivers/watchdog/lantiq_wdt.c | 7 ++--- drivers/watchdog/loongson1_wdt.c | 5 +--- drivers/watchdog/lpc18xx_wdt.c | 5 +--- drivers/watchdog/m54xx_wdt.c | 6 ++-- drivers/watchdog/machzwd.c | 11 +--- drivers/watchdog/max63xx_wdt.c | 5 +--- drivers/watchdog/max77620_wdt.c| 5 +--- drivers/watchdog/mei_wdt.c | 12 ++-- drivers/watchdog/mena21_wdt.c | 4 +-- drivers/watchdog/menf21bmc_wdt.c | 8 ++ drivers/watchdog/meson_gxbb_wdt.c | 50 +- drivers/watchdog/meson_wdt.c | 6 +--- drivers/watchdog/mixcomwd.c| 6 +--- drivers/watchdog/moxart_wdt.c | 7 ++--- drivers/watchdog/mpc8xxx_wdt.c | 6 +--- drivers/watchdog/mt7621_wdt.c | 5 +--- drivers/watchdog/mtk_wdt.c | 11 +--- drivers/watchdog/mtx-1_wdt.c | 11 +--- drivers/watchdog/mv64x60_wdt.c | 6 ++-- drivers/watchdog/ni903x_wdt.c | 11 +--- drivers/watchdog/nic7018_wdt.c | 11 +--- drivers/watchdog/nuc900_wdt.c | 7 ++--- drivers/watchdog/nv_tco.c | 6 +--- drivers/watchdog/nv_tco.h | 10 +-- drivers/watchdog/octeon-wdt-main.c | 11 +--- drivers/watc
Re: [PATCH] watchdog: add SPDX identifiers for watchdog subsystem
Hi Marcus On 02/20/2018 10:31 AM, Marcus Folkesson wrote: > - Add SPDX identifier > - Remove boiler plate license text > - If MODULE_LICENSE and boiler plate does not match, go for boiler plate >license > > Signed-off-by: Marcus Folkesson > --- > > Notes: > v1: Please have an extra look at meson_gxbb_wdt.c > [...] > diff --git a/drivers/watchdog/st_lpc_wdt.c b/drivers/watchdog/st_lpc_wdt.c > index e6100e447dd8..177829b379da 100644 > --- a/drivers/watchdog/st_lpc_wdt.c > +++ b/drivers/watchdog/st_lpc_wdt.c > @@ -1,3 +1,4 @@ > +// SPDX-License-Identifier: GPL-2.0+ > /* >* ST's LPC Watchdog >* > @@ -5,11 +6,6 @@ >* >* Author: David Paris for STMicroelectronics >* Lee Jones for STMicroelectronics > - * > - * This program is free software; you can redistribute it and/or > - * modify it under the terms of the GNU General Public Licence > - * as published by the Free Software Foundation; either version > - * 2 of the Licence, or (at your option) any later version. >*/ > For st_lpc_wdt.c Acked-by: Patrice Chotard Thanks
Re: [PATCH v3] watchdog: add SPDX identifiers for watchdog subsystem
HI Marcus On 02/21/2018 01:27 PM, Marcus Folkesson wrote: > - Add SPDX identifier > - Remove boiler plate license text > - If MODULE_LICENSE and boiler plate does not match, go for boiler plate >license > > Signed-off-by: Marcus Folkesson > Acked-by: Adam Thomson > Acked-by: Charles Keepax > Acked-by: Mans Rullgard > Acked-by: Matthias Brugger > Acked-by: Michal Simek > Acked-by: Neil Armstrong > Acked-by: Nicolas Ferre > Acked-by: Thierry Reding > Reviewed-by: Eric Anholt > --- > > Notes: > v3: > - Keep license text for ebc-c384_wdt > v2: > - Put back removed copyright texts for meson_gxbb_wdt and coh901327_wdt > - Change to BSD-3-Clause for meson_gxbb_wdt > v1: Please have an extra look at meson_gxbb_wdt.c > [...] > diff --git a/drivers/watchdog/st_lpc_wdt.c b/drivers/watchdog/st_lpc_wdt.c > index e6100e447dd8..177829b379da 100644 > --- a/drivers/watchdog/st_lpc_wdt.c > +++ b/drivers/watchdog/st_lpc_wdt.c > @@ -1,3 +1,4 @@ > +// SPDX-License-Identifier: GPL-2.0+ > /* >* ST's LPC Watchdog >* > @@ -5,11 +6,6 @@ >* >* Author: David Paris for STMicroelectronics >* Lee Jones for STMicroelectronics > - * > - * This program is free software; you can redistribute it and/or > - * modify it under the terms of the GNU General Public Licence > - * as published by the Free Software Foundation; either version > - * 2 of the Licence, or (at your option) any later version. >*/ > For st_lpc_wdt.c Acked-by: Patrice Chotard Thanks
[PATCH] powerpc/pseries: Fix duplicate firmware feature for DRC_INFO
We had a mid-air collision between two new firmware features, DRMEM_V2
and DRC_INFO, and they ended up with the same value.
No one's actually reported any problems, presumably because the new
firmware that supports both properties is not widely available, and
the two properties tend to be enabled together.
Still if we ever had one enabled but not the other, the bugs that
could result are many and varied. So fix it.
Fixes: 3f38000eda48 ("powerpc/firmware: Add definitions for new drc-info
firmware feature")
Signed-off-by: Michael Ellerman
---
arch/powerpc/include/asm/firmware.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/firmware.h
b/arch/powerpc/include/asm/firmware.h
index 511acfd7ab0d..535add3f7791 100644
--- a/arch/powerpc/include/asm/firmware.h
+++ b/arch/powerpc/include/asm/firmware.h
@@ -52,7 +52,7 @@
#define FW_FEATURE_TYPE1_AFFINITY ASM_CONST(0x0001)
#define FW_FEATURE_PRRNASM_CONST(0x0002)
#define FW_FEATURE_DRMEM_V2ASM_CONST(0x0004)
-#define FW_FEATURE_DRC_INFOASM_CONST(0x0004)
+#define FW_FEATURE_DRC_INFOASM_CONST(0x0008)
#ifndef __ASSEMBLY__
--
2.14.1
Re: [PATCH] cpufreq: powernv: Check negative value returned by cpufreq_table_find_index_dl()
"Rafael J. Wysocki" writes: > On Wed, Feb 21, 2018 at 6:54 AM, Viresh Kumar wrote: >> On 21-02-18, 16:39, Michael Ellerman wrote: >>> Viresh Kumar 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. OK. Still it's one thing for the driver to print a warning and bail out, it's another to access off the front of an array and keep running using some junk values, or oops (though not in this case because the array happens to be static). cheers
Re: [PATCH] cpufreq: powernv: Check negative value returned by cpufreq_table_find_index_dl()
On Wed, Feb 21, 2018 at 2:13 PM, Michael Ellerman wrote: > "Rafael J. Wysocki" writes: > >> On Wed, Feb 21, 2018 at 6:54 AM, Viresh Kumar >> wrote: >>> On 21-02-18, 16:39, Michael Ellerman wrote: Viresh Kumar 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. > > OK. > > Still it's one thing for the driver to print a warning and bail out, > it's another to access off the front of an array and keep running using > some junk values, or oops (though not in this case because the array > happens to be static). Well, let me rephrase. If ->fast_switch() runs, then it must not be possible to get a negative index in it. That has to be guaranteed by the core.
Re: [RFC PATCH v0 1/2] powerpc, drmem: Fix unexpected flag value in ibm, dynamic-memory-v2
On Wed, Feb 21, 2018 at 10:48:18PM +1100, Michael Ellerman wrote:
> Bharata B Rao writes:
>
> > Memory addtion and removal by count and indexed-count methods
> > temporarily mark the LMBs that are being added/removed by a special
> > flag value DRMEM_LMB_RESERVED. Accessing flags value directly at
> > a few places without proper accessor method is causing two unexpected
> > side-effects:
> >
> > - DRMEM_LMB_RESERVED bit is becoming part of the flags word of
> > drconf_cell_v2 entries in ibm,dynamic-memory-v2 DT property.
> > - This results in extra drconf_cell entries in ibm,dynamic-memory-v2.
> > For example if 1G memory is added, it leads to one entry for 3 LMBs
> > and 1 separate entry for the last LMB. All the 4 LMBs should be
> > defined by one entry here.
> >
> > Fix this by always accessing the flags by its accessor method
> > drmem_lmb_flags().
> >
> > Signed-off-by: Bharata B Rao
>
> Presumably:
>
> Fixes: 2b31e3aec1db ("powerpc/drmem: Add support for ibm, dynamic-memory-v2
> property")
Yes.
Regards,
Bharata.
RE: [PATCH v3] watchdog: add SPDX identifiers for watchdog subsystem
>
> - Add SPDX identifier
> - Remove boiler plate license text
> - If MODULE_LICENSE and boiler plate does not match, go for boiler plate
> license
>
> Signed-off-by: Marcus Folkesson
> Acked-by: Adam Thomson
> Acked-by: Charles Keepax
> Acked-by: Mans Rullgard
> Acked-by: Matthias Brugger
> Acked-by: Michal Simek
> Acked-by: Neil Armstrong
> Acked-by: Nicolas Ferre
> Acked-by: Thierry Reding
> Reviewed-by: Eric Anholt
> ---
>
> Notes:
> v3:
> - Keep license text for ebc-c384_wdt
> v2:
> - Put back removed copyright texts for meson_gxbb_wdt and
> coh901327_wdt
> - Change to BSD-3-Clause for meson_gxbb_wdt
> v1: Please have an extra look at meson_gxbb_wdt.c
>
> *
> - * This program is free software; you can redistribute it and/or modify it
> - * under the terms of the GNU General Public License version 2 as published
> - * by the Free Software Foundation.
> *
>
> #include
> diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
> index b8194b02abe0..8023cf28657a 100644
> --- a/drivers/watchdog/mei_wdt.c
> +++ b/drivers/watchdog/mei_wdt.c
> @@ -1,15 +1,7 @@
> +// SPDX-License-Identifier: GPL-2.0
> /*
> * Intel Management Engine Interface (Intel MEI) Linux driver
> * Copyright (c) 2015, Intel Corporation.
> - *
> - * This program is free software; you can redistribute it and/or modify it
> - * under the terms and conditions of the GNU General Public License,
> - * version 2, as published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope it will be useful, but WITHOUT
> - * ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or
> - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> for
> - * more details.
> */
>
> #include
> @@ -687,5 +679,5 @@ static struct mei_cl_driver mei_wdt_driver = {
> module_mei_cl_driver(mei_wdt_driver);
>
> MODULE_AUTHOR("Intel Corporation");
> -MODULE_LICENSE("GPL");
> +MODULE_LICENSE("GPL v2");
> MODULE_DESCRIPTION("Device driver for Intel MEI iAMT watchdog");
Acked-by: Tomas Winkler
Re: [RFC PATCH v0 0/2] ibm,dynamic-memory-v2 fix
On Wed, Feb 21, 2018 at 11:27:20PM +1100, Balbir Singh wrote: > On Wed, Feb 21, 2018 at 9:36 PM, Bharata B Rao > wrote: > > Patch 1 fixes a bug that results in unexpected flag bit in > > ibm,dynamic-memory-v2 DT property and wrong number of entries > > getting created in the same property during hotplug. > > > > Could you please elaborate on what this means? Described in 1/2 patch. > Is there a test case - > how do we reproduce this? Hotplug some memory and check the entries in ibm,dynamic-memory-v2 and also check the flags there, you will see DRMEM_LMB_RESERVED set, which shouldn't be set. You would need QEMU which supports ibm,dynamic-memory-v2. I have posted the initial patches for that at: http://lists.gnu.org/archive/html/qemu-ppc/2018-02/msg00236.html Regards, Bharata.
Re: [PATCH] bpf, powerpc: fix jit for seccomp_data access
On 18-02-21 07:52 AM, Mark Lord wrote: > On 18-02-21 03:35 AM, Naveen N. Rao wrote: .. >> Looks good to me, but I am not able to apply this patch. There seems to be >> whitespace damage. > > Here (attached) is a clean copy. Again, this time with the commit message included! 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 --- old/arch/powerpc/net/bpf_jit_comp.c 2018-02-16 14:07:01.0 -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. ml...@pobox.com 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 --- old/arch/powerpc/net/bpf_jit_comp.c 2018-02-16 14:07:01.0 -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;
Re: ocxl: Fix potential bad errno on irq allocation
On Fri, 2018-02-16 at 13:01:18 UTC, Frederic Barrat wrote: > Fix some issues found by a static checker: > > When allocating an AFU interrupt, if the driver cannot copy the output > parameters to userland, the errno value was not set to EFAULT > > Remove a (now) useless cast. > > Reported-by: Dan Carpenter > Signed-off-by: Frederic Barrat > Acked-by: Andrew Donnellan Applied to powerpc fixes, thanks. https://git.kernel.org/powerpc/c/423688abd9ab654044bddd82eb5983 cheers
Re: powerpc/eeh: Add conditional check on notify_resume
On Thu, 2018-02-15 at 18:49:51 UTC, "Bryant G. Ly" wrote: > From: "Juan J. Alvarez" > > EEH structure is not populated with function > notify resume when running on systems that do not support > it, i.e: BMC. Hence adding a conditional check for NULL for > systems that don't add function notify_resume. > > Signed-off-by: Juan J. Alvarez > Reviewed-by: Bryant G. Ly > Tested-by: Carol L. Soto > Reviewed-by: Andrew Donnellan > Tested-by: Mauro S. M. Rodrigues > Acked-by: Michael Neuling Applied to powerpc fixes, thanks. https://git.kernel.org/powerpc/c/521ca5a9859a870e354d1a6b84a6ff cheers
Re: [PATCH 1/6] powerpc/mm/32: Use pfn_valid to check if pointer is in RAM
Hello Christophe, On Tue, Feb 20, 2018 at 06:45:09PM +0100, christophe leroy wrote: [...] > > - 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 Good point, I haven't considered CONFIG_HIGHMEM before. As far as I understand it, in the non-CONFIG_HIGHMEM case: - max_low_pfn is set to the same value as max_pfn, so the ioremap check should detect the same PFNs as RAM. and with CONFIG_HIGHMEM: - max_low_pfn is set to lowmem_end_addr >> PAGE_SHIFT - but max_pfn isn't So, I think you're right. While looking through arch/powerpc/mm, I noticed that there's a page_is_ram function, which simply uses the memblocks directly, on PPC32. It seems like a good candidate for the RAM check in __ioremap_caller, except that there's this code, which apparently trashes memblock 0 completely on non-CONFIG_NEED_MULTIPLE_NODES: https://elixir.bootlin.com/linux/v4.16-rc2/source/arch/powerpc/mm/mem.c#L223 Thanks, Jonathan Neuschäfer signature.asc Description: PGP signature
Re: [PATCH] bpf, powerpc: fix jit for seccomp_data access
Mark Lord wrote: On 18-02-21 07:52 AM, Mark Lord wrote: On 18-02-21 03:35 AM, Naveen N. Rao wrote: .. Looks good to me, but I am not able to apply this patch. There seems to be whitespace damage. Here (attached) is a clean copy. Again, this time with the commit message included! Thanks. However... I am able to apply this using 'patch', but not with 'git am' since the headers are missing. FWIW, the usual workflow is to make the changes and commit it into your repository using 'git commit' and then use 'git format-patch' to generate a patch file that you can then post. I'll defer to Michael on whether he is ok to process this as it is. 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 Minot nit: The correct (TM) tag to use is: Signed-off-by: (note the case) --- old/arch/powerpc/net/bpf_jit_comp.c 2018-02-16 14:07:01.0 -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; Apart from those aspects, for this patch: Acked-by: Naveen N. Rao - Naveen
Re: [PATCH 0/6] DISCONTIGMEM support for PPC32
Hi, On Wed, Feb 21, 2018 at 08:06:10AM +0100, Christophe LEROY wrote: > > > 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 ? Using page_is_ram in __ioremap_caller and the same memblock-based approach that's used on PPC64 on PPC32 *should* work, but I think due to the following line in initmem_init, it won't: memblock_set_node(0, (phys_addr_t)ULLONG_MAX, &memblock.memory, 0); Thanks, Jonathan Neuschäfer signature.asc Description: PGP signature
Re: [PATCH 1/6] powerpc/mm/32: Use pfn_valid to check if pointer is in RAM
On Wed, Feb 21, 2018 at 02:51:19PM +0100, Jonathan Neuschäfer wrote: [...] > While looking through arch/powerpc/mm, I noticed that there's a > page_is_ram function, which simply uses the memblocks directly, on > PPC32. Oops, I misread the code here. memblock is used on PPC64. > It seems like a good candidate for the RAM check in > __ioremap_caller, except that there's this code, which apparently > trashes memblock 0 completely on non-CONFIG_NEED_MULTIPLE_NODES: > > https://elixir.bootlin.com/linux/v4.16-rc2/source/arch/powerpc/mm/mem.c#L223 > > > Thanks, > Jonathan Neuschäfer signature.asc Description: PGP signature
[PATCH V2] powerpc/powernv : Add support to enable sensor groups
Adds support to enable/disable a sensor group. This can be used to
select the sensor groups that needs to be copied to main memory by
OCC. Sensor groups like power, temperature, current, voltage,
frequency, utilization can be enabled/disabled at runtime.
Signed-off-by: Shilpasri G Bhat
---
Changes from V1:
- Rebase on master
- Add documentation
.../ABI/testing/sysfs-firmware-opal-sensor-groups | 34 ++
arch/powerpc/include/asm/opal-api.h| 4 +-
arch/powerpc/include/asm/opal.h| 1 +
.../powerpc/platforms/powernv/opal-sensor-groups.c | 123 -
arch/powerpc/platforms/powernv/opal-wrappers.S | 1 +
5 files changed, 132 insertions(+), 31 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-firmware-opal-sensor-groups
diff --git a/Documentation/ABI/testing/sysfs-firmware-opal-sensor-groups
b/Documentation/ABI/testing/sysfs-firmware-opal-sensor-groups
new file mode 100644
index 000..81081de
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-firmware-opal-sensor-groups
@@ -0,0 +1,34 @@
+What: /sys/firmware/opal/sensor_groups
+Date: January 2018
+Contact: Linux for PowerPC mailing list
+Description: Sensor groups directory for POWER9 powernv servers
+
+ Each folder in this directory contains a sensor group
+ which are classified based on type of the sensor
+ like power, temperature, frequency, current, etc. They
+ can also indicate the group of sensors belonging to
+ different owners like CSM, Profiler, Job-Scheduler
+
+What: /sys/firmware/opal/sensor_groups//clear
+Date: Januaury 2018
+Contact: Linux for PowerPC mailing list
+Description: Sysfs file to clear the min-max of all the sensors
+ belonging to the group.
+
+ Writing 1 to this file will clear the minimum and
+ maximum values of all the sensors in the group. The
+ min-max of a sensor is the historical minimum and
+ maximum value of the sensor cached by OCC.
+
+What: /sys/firmware/opal/sensor_groups//enable
+Date: Januaury 2018
+Contact: Linux for PowerPC mailing list
+Description: Sysfs file to enable/disable the sensor-group
+
+ Writing 0 value to this file will disable the copying
+ of the sensor-group to main memory by OCC. And writing
+ 1 to this file will enable the sensor-group copying.
+ By default all the sensor-groups are enabled and will
+ be copied to main memory. This file can be used to
+ increase the update frequency of selective
+ sensor-groups.
diff --git a/arch/powerpc/include/asm/opal-api.h
b/arch/powerpc/include/asm/opal-api.h
index 94bd1bf..b6bbbd8 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_SET161
-#define OPAL_LAST 161
+#define OPAL_SENSOR_READ_U64 162
+#define OPAL_SENSOR_GROUP_ENABLE 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 12e70fb..e708c41 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -286,6 +286,7 @@ int64_t opal_imc_counters_init(uint32_t type, uint64_t
address,
int opal_get_power_shift_ratio(u32 handle, int token, u32 *psr);
int opal_set_power_shift_ratio(u32 handle, int token, u32 psr);
int opal_sensor_group_clear(u32 group_hndl, int token);
+int opal_sensor_group_enable(u32 group_hndl, int token, bool enable);
s64 opal_signal_system_reset(s32 cpu);
diff --git a/arch/powerpc/platforms/powernv/opal-sensor-groups.c
b/arch/powerpc/platforms/powernv/opal-sensor-groups.c
index 7e5a235..1a3359d 100644
--- a/arch/powerpc/platforms/powernv/opal-sensor-groups.c
+++ b/arch/powerpc/platforms/powernv/opal-sensor-groups.c
@@ -24,6 +24,8 @@
struct sg_attr {
u32 handle;
struct kobj_attribute attr;
+ u32 opal_no;
+ int enable;
};
static struct sensor_group {
@@ -32,34 +34,44 @@ struct sg_attr {
struct sg_attr *sgattrs;
} *sgs;
-static ssize_t sg_store(struct kobject *kobj, struct kobj_attribute *attr,
- const char *buf, size_t count)
+static int sensor_group_clear(u32 handle)
{
- struct sg_attr *sattr = container_of(attr, struct sg_attr, attr);
struct opal_msg msg;
- u32 data;
- int ret, token;
-
- ret = kstrtoint(buf, 0, &data);
- if (ret)
- return ret;
-
- if (data != 1)
- return -EINVAL;
+ int token, ret;
token = opal_async_get
Re: [PATCH 0/6] DISCONTIGMEM support for PPC32
Le 21/02/2018 à 15:42, Jonathan Neuschäfer a écrit : Hi, On Wed, Feb 21, 2018 at 08:06:10AM +0100, Christophe LEROY wrote: 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 ? Using page_is_ram in __ioremap_caller and the same memblock-based approach that's used on PPC64 on PPC32 *should* work, but I think due to the following line in initmem_init, it won't: memblock_set_node(0, (phys_addr_t)ULLONG_MAX, &memblock.memory, 0); Can't we just fix that ? Christophe Thanks, Jonathan Neuschäfer
Re: [PATCH 00/23] kconfig: move compiler capability tests to Kconfig
On Wed, Feb 21, 2018 at 1:57 PM, Masahiro Yamada wrote: > 2018-02-21 19:52 GMT+09:00 Arnd Bergmann : >> On Wed, Feb 21, 2018 at 11:20 AM, Masahiro Yamada >> wrote: >>> 2018-02-21 18:56 GMT+09:00 Arnd Bergmann : On Wed, Feb 21, 2018 at 8:38 AM, Masahiro Yamada wrote: > 2018-02-20 0:18 GMT+09:00 Ulf Magnusson : > > Hmm, I think I can implement those somehow. > But, I hope we do not have many instances like this... > > > If you know more naive cases, please share your knowledge. > One case that comes to mind would be architecture level selection on 32-bit ARM, which is roughly this (I probably have some details wrong, but you get the idea): - older compilers don't support the latest architecture setting (-march=armv8 or -march=armv7ve) - newer compilers no longer support really old architectures (-march=armv4) - setting -mthumb requires setting one of -march=armv7-a, armv7ve, armv7-m or armv8 if the compiler doesn't default to those - on a compiler that defaults to -marm, setting -march=armv7-m requires setting -mthumb (IIRC) - really old compilers only support OABI, but not EABI - newer compilers no longer support OABI - mthumb requires EABI - armv6 and higher are subtly broken with OABI, but only when using certain inline assembly with 64-bit arguments in register pairs. I think we just shouldn't try to capture all of the above correctly in Kconfig conditionals. Arnd
Re: [PATCH 5/6] powerpc: Implement DISCONTIGMEM and allow selection on PPC32
On Wed, Feb 21, 2018 at 07:46:28AM +0800, kbuild test robot wrote: [...] > >> include/linux/mmzone.h:1239:19: error: conflicting types for 'pfn_valid' > static inline int pfn_valid(unsigned long pfn) > ^ >In file included from include/linux/mmzone.h:912:0, > from include/linux/gfp.h:6, > from include/linux/mm.h:10, > from include/linux/mman.h:5, > from arch/powerpc/kernel/asm-offsets.c:22: >arch/powerpc/include/asm/mmzone.h:40:19: note: previous definition of > 'pfn_valid' was here > static inline int pfn_valid(int pfn) > ^ >make[2]: *** [arch/powerpc/kernel/asm-offsets.s] Error 1 >make[2]: Target '__build' not remade because of errors. >make[1]: *** [prepare0] Error 2 >make[1]: Target 'prepare' not remade because of errors. >make: *** [sub-make] Error 2 Oops, I'll fix this in the next version (and compile-test on ppc64...). Weirdly enough, x86-32 and parisc define pfn_valid with an int parameter, too (both of them since the Beginning Of Time, aka. v2.6.12-rc2). signature.asc Description: PGP signature
Re: [PATCH 0/6] DISCONTIGMEM support for PPC32
On Wed, Feb 21, 2018 at 04:02:25PM +0100, Christophe LEROY wrote: [...] > > > 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 ? > > > > Using page_is_ram in __ioremap_caller and the same memblock-based > > approach that's used on PPC64 on PPC32 *should* work, but I think due to > > the following line in initmem_init, it won't: > > > > memblock_set_node(0, (phys_addr_t)ULLONG_MAX, &memblock.memory, 0); > > Can't we just fix that ? I'll give it a try. Thanks, Jonathan Neuschäfer signature.asc Description: PGP signature
Re: [PATCH 5/6] powerpc: Implement DISCONTIGMEM and allow selection on PPC32
Le 21/02/2018 à 17:08, Jonathan Neuschäfer a écrit : On Wed, Feb 21, 2018 at 07:46:28AM +0800, kbuild test robot wrote: [...] include/linux/mmzone.h:1239:19: error: conflicting types for 'pfn_valid' static inline int pfn_valid(unsigned long pfn) ^ In file included from include/linux/mmzone.h:912:0, from include/linux/gfp.h:6, from include/linux/mm.h:10, from include/linux/mman.h:5, from arch/powerpc/kernel/asm-offsets.c:22: arch/powerpc/include/asm/mmzone.h:40:19: note: previous definition of 'pfn_valid' was here static inline int pfn_valid(int pfn) ^ make[2]: *** [arch/powerpc/kernel/asm-offsets.s] Error 1 make[2]: Target '__build' not remade because of errors. make[1]: *** [prepare0] Error 2 make[1]: Target 'prepare' not remade because of errors. make: *** [sub-make] Error 2 Oops, I'll fix this in the next version (and compile-test on ppc64...). Weirdly enough, x86-32 and parisc define pfn_valid with an int parameter, too (both of them since the Beginning Of Time, aka. v2.6.12-rc2). Behind the fact that the pfn type is different, my understanding is that you have to define CONFIG_HAVE_ARCH_PFN_VALID in the Kconfig in order to avoid it being included in include/linux/mmzone.h Christophe --- L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast. https://www.avast.com/antivirus
Re: [PATCH v3] watchdog: add SPDX identifiers for watchdog subsystem
On Wed, Feb 21, 2018 at 01:27:34PM +0100, Marcus Folkesson wrote: > - Add SPDX identifier > - Remove boiler plate license text > - If MODULE_LICENSE and boiler plate does not match, go for boiler plate > license [...] > drivers/watchdog/mena21_wdt.c | 4 +-- Acked-by: Johannes Thumshirn -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
[PATCH v12 07/11] mm: Add address parameter to arch_validate_prot()
A protection flag may not be valid across entire address space and
hence arch_validate_prot() might need the address a protection bit is
being set on to ensure it is a valid protection flag. For example, sparc
processors support memory corruption detection (as part of ADI feature)
flag on memory addresses mapped on to physical RAM but not on PFN mapped
pages or addresses mapped on to devices. This patch adds address to the
parameters being passed to arch_validate_prot() so protection bits can
be validated in the relevant context.
Signed-off-by: Khalid Aziz
Cc: Khalid Aziz
Reviewed-by: Anthony Yznaga
---
v8:
- Added addr parameter to powerpc arch_validate_prot() (suggested
by Michael Ellerman)
v9:
- new patch
arch/powerpc/include/asm/mman.h | 4 ++--
arch/powerpc/kernel/syscalls.c | 2 +-
include/linux/mman.h| 2 +-
mm/mprotect.c | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/mman.h b/arch/powerpc/include/asm/mman.h
index 07e3f54de9e3..e3f1b5ba5d5c 100644
--- a/arch/powerpc/include/asm/mman.h
+++ b/arch/powerpc/include/asm/mman.h
@@ -43,7 +43,7 @@ static inline pgprot_t arch_vm_get_page_prot(unsigned long
vm_flags)
}
#define arch_vm_get_page_prot(vm_flags) arch_vm_get_page_prot(vm_flags)
-static inline bool arch_validate_prot(unsigned long prot)
+static inline bool arch_validate_prot(unsigned long prot, unsigned long addr)
{
if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM | PROT_SAO))
return false;
@@ -51,7 +51,7 @@ static inline bool arch_validate_prot(unsigned long prot)
return false;
return true;
}
-#define arch_validate_prot(prot) arch_validate_prot(prot)
+#define arch_validate_prot arch_validate_prot
#endif /* CONFIG_PPC64 */
#endif /* _ASM_POWERPC_MMAN_H */
diff --git a/arch/powerpc/kernel/syscalls.c b/arch/powerpc/kernel/syscalls.c
index a877bf8269fe..6d90ddbd2d11 100644
--- a/arch/powerpc/kernel/syscalls.c
+++ b/arch/powerpc/kernel/syscalls.c
@@ -48,7 +48,7 @@ static inline long do_mmap2(unsigned long addr, size_t len,
{
long ret = -EINVAL;
- if (!arch_validate_prot(prot))
+ if (!arch_validate_prot(prot, addr))
goto out;
if (shift) {
diff --git a/include/linux/mman.h b/include/linux/mman.h
index 6a4d1caaff5c..4b08e9c9c538 100644
--- a/include/linux/mman.h
+++ b/include/linux/mman.h
@@ -92,7 +92,7 @@ static inline void vm_unacct_memory(long pages)
*
* Returns true if the prot flags are valid
*/
-static inline bool arch_validate_prot(unsigned long prot)
+static inline bool arch_validate_prot(unsigned long prot, unsigned long addr)
{
return (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM)) == 0;
}
diff --git a/mm/mprotect.c b/mm/mprotect.c
index e3309fcf586b..088ea9c08678 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -417,7 +417,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len,
end = start + len;
if (end <= start)
return -ENOMEM;
- if (!arch_validate_prot(prot))
+ if (!arch_validate_prot(prot, start))
return -EINVAL;
reqprot = prot;
--
2.11.0
Re: [PATCH] powerpc/pseries: Fix duplicate firmware feature for DRC_INFO
On 02/21/2018 05:05 AM, Michael Ellerman wrote:
> We had a mid-air collision between two new firmware features, DRMEM_V2
> and DRC_INFO, and they ended up with the same value.
>
> No one's actually reported any problems, presumably because the new
> firmware that supports both properties is not widely available, and
> the two properties tend to be enabled together.
>
> Still if we ever had one enabled but not the other, the bugs that
> could result are many and varied. So fix it.
>
> Fixes: 3f38000eda48 ("powerpc/firmware: Add definitions for new drc-info
> firmware feature")
> Signed-off-by: Michael Ellerman
> ---
Good catch.
Reviewed-by: Tyrel Datwyler
> arch/powerpc/include/asm/firmware.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/firmware.h
> b/arch/powerpc/include/asm/firmware.h
> index 511acfd7ab0d..535add3f7791 100644
> --- a/arch/powerpc/include/asm/firmware.h
> +++ b/arch/powerpc/include/asm/firmware.h
> @@ -52,7 +52,7 @@
> #define FW_FEATURE_TYPE1_AFFINITY ASM_CONST(0x0001)
> #define FW_FEATURE_PRRN ASM_CONST(0x0002)
> #define FW_FEATURE_DRMEM_V2 ASM_CONST(0x0004)
> -#define FW_FEATURE_DRC_INFO ASM_CONST(0x0004)
> +#define FW_FEATURE_DRC_INFO ASM_CONST(0x0008)
>
> #ifndef __ASSEMBLY__
>
Re: [PATCH 00/23] kconfig: move compiler capability tests to Kconfig
On Wed, Feb 21, 2018 at 09:57:03PM +0900, Masahiro Yamada wrote:
> 2018-02-21 19:52 GMT+09:00 Arnd Bergmann :
> > On Wed, Feb 21, 2018 at 11:20 AM, Masahiro Yamada
> > wrote:
> >> 2018-02-21 18:56 GMT+09:00 Arnd Bergmann :
> >>> On Wed, Feb 21, 2018 at 8:38 AM, Masahiro Yamada
> >>> wrote:
> 2018-02-20 0:18 GMT+09:00 Ulf Magnusson :
> >>
> >> Let me clarify my concern.
> >>
> >> When we test the compiler flag, is there a case
> >> where a particular flag depends on -m{32,64} ?
> >>
> >> For example, is there a compiler that supports -fstack-protector
> >> for 64bit mode, but unsupports it for 32bit mode?
> >>
> >> $(cc-option -m32) -> y
> >> $(cc-option -m64) -> y
> >> $(cc-option -fstack-protector)-> y
> >> $(cc-option -m32 -fstack-protector) -> n
> >> $(cc-option -m64 -fstack-protector) -> y
> >>
> >> I guess this is unlikely to happen,
> >> but I am not whether it is zero possibility.
> >>
> >> If this could happen,
> >> $(cc-option ) must be evaluated together with
> >> correct bi-arch option (either -m32 or -m64).
> >>
> >>
> >> Currently, -m32/-m64 is specified in Makefile,
> >> but we are moving compiler tests to Kconfig
> >> and, CONFIG_64BIT can be dynamically toggled in Kconfig.
> >
> > I don't think it can happen for this particular combination (stack protector
> > and word size), but I'm sure we'll eventually run into options that
> > need to be tested in combination. For the current CFLAGS_KERNEL
> > setting, we definitely have the case of needing the variables to be
> > evaluated in a specific order.
> >
>
>
>
>
> I was thinking of how we can handle complex cases
> in the current approach.
>
>
>
> (Case 1)
>
> Compiler flag -foo and -bar interacts, so
> we also need to check the combination of the two.
>
>
> config CC_HAS_FOO
> def_bool $(cc-option -foo)
>
> config CC_HAS_BAR
> def_bool $(cc-option -bar)
>
> config CC_HAS_FOO_WITH_BAR
> def_bool $(cc-option -foo -bar)
>
>
>
> (Case 2)
> Compiler flag -foo is sensitive to word-size.
> So, we need to test this option together with -m32/-m64.
> User can toggle CONFIG_64BIT, like i386/x86_64.
>
>
> config CC_NEEDS_M64
> def_bool $(cc-option -m64) && 64BIT
>
> config CC_NEEDS_M32
> def_bool $(cc-option -m32) && !64BIT
>
> config CC_HAS_FOO
> bool
> default $(cc-option -m64 -foo) if CC_NEEDS_M64
> default $(cc-option -m32 -foo) if CC_NEEDS_M32
> default $(cc-option -foo)
>
>
>
> (Case 3)
> Compiler flag -foo is sensitive to endian-ness.
>
>
> config CC_NEEDS_BIG_ENDIAN
> def_bool $(cc-option -mbig-endian) && CPU_BIG_ENDIAN
>
> config CC_NEEDS_LITTLE_ENDIAN
> def_bool $(cc-option -mlittle-endian) && CPU_LITTLE_ENDIAN
>
> config CC_HAS_FOO
> bool
> default $(cc-option -mbig-endian -foo) if CC_NEEDS_BIG_ENDIAN
> default $(cc-option -mlittle-endian -foo) if CC_NEEDS_LITTLE_ENDIAN
> default $(cc-option -foo)
>
>
>
>
> Hmm, I think I can implement those somehow.
> But, I hope we do not have many instances like this...
>
>
> If you know more naive cases, please share your knowledge.
>
> Thanks!
>
>
> --
> Best Regards
> Masahiro Yamada
Would get pretty bad if a test needs to consider multiple symbols.
Exponential explosion there...
I thought some more about the implementation of dynamic (post-parsing)
functions to see how bad it would get with the current implementation.
Some background on how things work now:
1. All expression operands in Kconfig are symbols.
2. Returning '$ENV' or '$(fn foo)' as a T_WORD during parsing gets
you symbols with those strings as names and S_UNKNOWN type (because
they act like references to undefined symbols).
3. For "foo-$(fn foo)", you also get a symbol with that string as its
name and S_UNKNOWN type (stored among the SYMBOL_CONST symbols)
4. Symbols with S_UNKNOWN type get their name as their string value,
and the tristate value n.
So, if you do string expansion on the names of symbols with S_UNKNOWN
type in sym_calc_value(), you're almost there with the current
implementation, except for the tristate case.
Maybe you could set the tristate value of S_UNKNOWN symbols depending on
the string value you end up with. Things are getting pretty confusing at
that point.
Could have something like S_DYNAMIC as well. More Kconfig complexity...
Then there's other complications:
1. SYMBOL_CONST is no longer constant.
2. Dependency loop detection needs to consider symbol references
within strings.
3. Dependency loop detection relies on static knowledge of what
symbols a symbol depends on. That might get messy for certain
expansions, though it might be things you wouldn't do in practice.
4. Symbols still need to be properly invalidated. It looks like at
least menuconfig just does a dumb invalidate-everything whenever
the
[PATCH] powerpc/time: stop validating rtc_time in .read_time
The RTC core is always calling rtc_valid_tm after the read_time callback.
It is not necessary to call it just before returning from the callback.
Signed-off-by: Alexandre Belloni
---
arch/powerpc/kernel/time.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index a32823dcd9a4..f7d96a68ecaa 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -1234,7 +1234,7 @@ void calibrate_delay(void)
static int rtc_generic_get_time(struct device *dev, struct rtc_time *tm)
{
ppc_md.get_rtc_time(tm);
- return rtc_valid_tm(tm);
+ return 0;
}
static int rtc_generic_set_time(struct device *dev, struct rtc_time *tm)
--
2.16.1
[PATCH v12 00/11] Application Data Integrity feature introduced by SPARC M7
V12 changes: This series is same as v10 and v11 and was simply rebased on 4.16-rc2 kernel and patch 11 was added to update signal delivery code to use the new helper functions added by Eric Biederman. Can mm maintainers please review patches 2, 7, 8 and 9 which are arch independent, and include/linux/mm.h and mm/ksm.c changes in patch 10 and ack these if everything looks good? SPARC M7 processor adds additional metadata for memory address space that can be used to secure access to regions of memory. This additional metadata is implemented as a 4-bit tag attached to each cacheline size block of memory. A task can set a tag on any number of such blocks. Access to such block is granted only if the virtual address used to access that block of memory has the tag encoded in the uppermost 4 bits of VA. Since sparc processor does not implement all 64 bits of VA, top 4 bits are available for ADI tags. Any mismatch between tag encoded in VA and tag set on the memory block results in a trap. Tags are verified in the VA presented to the MMU and tags are associated with the physical page VA maps on to. If a memory page is swapped out and page frame gets reused for another task, the tags are lost and hence must be saved when swapping or migrating the page. A userspace task enables ADI through mprotect(). This patch series adds a page protection bit PROT_ADI and a corresponding VMA flag VM_SPARC_ADI. VM_SPARC_ADI is used to trigger setting TTE.mcd bit in the sparc pte that enables ADI checking on the corresponding page. MMU validates the tag embedded in VA for every page that has TTE.mcd bit set in its pte. After enabling ADI on a memory range, the userspace task can set ADI version tags using stxa instruction with ASI_MCD_PRIMARY or ASI_MCD_ST_BLKINIT_PRIMARY ASI. Once userspace task calls mprotect() with PROT_ADI, kernel takes following overall steps: 1. Find the VMAs covering the address range passed in to mprotect and set VM_SPARC_ADI flag. If address range covers a subset of a VMA, the VMA will be split. 2. When a page is allocated for a VA and the VMA covering this VA has VM_SPARC_ADI flag set, set the TTE.mcd bit so MMU will check the vwersion tag. 3. Userspace can now set version tags on the memory it has enabled ADI on. Userspace accesses ADI enabled memory using a virtual address that has the version tag embedded in the high bits. MMU validates this version tag against the actual tag set on the memory. If tag matches, MMU performs the VA->PA translation and access is granted. If there is a mismatch, hypervisor sends a data access exception or precise memory corruption detected exception depending upon whether precise exceptions are enabled or not (controlled by MCDPERR register). Kernel sends SIGSEGV to the task with appropriate si_code. 4. If a page is being swapped out or migrated, kernel must save any ADI tags set on the page. Kernel maintains a page worth of tag storage descriptors. Each descriptors pointsto a tag storage space and the address range it covers. If the page being swapped out or migrated has ADI enabled on it, kernel finds a tag storage descriptor that covers the address range for the page or allocates a new descriptor if none of the existing descriptors cover the address range. Kernel saves tags from the page into the tag storage space descriptor points to. 5. When the page is swapped back in or reinstantiated after migration, kernel restores the version tags on the new physical page by retrieving the original tag from tag storage pointed to by a tag storage descriptor for the virtual address range for new page. User task can disable ADI by calling mprotect() again on the memory range with PROT_ADI bit unset. Kernel clears the VM_SPARC_ADI flag in VMAs, merges adjacent VMAs if necessary, and clears TTE.mcd bit in the corresponding ptes. IOMMU does not support ADI checking. Any version tags embedded in the top bits of VA meant for IOMMU, are cleared and replaced with sign extension of the first non-version tag bit (bit 59 for SPARC M7) for IOMMU addresses. This patch series adds support for this feature in 11 patches: Patch 1/11 Tag mismatch on access by a task results in a trap from hypervisor as data access exception or a precide memory corruption detected exception. As part of handling these exceptions, kernel sends a SIGSEGV to user process with special si_code to indicate which fault occurred. This patch adds three new si_codes to differentiate between various mismatch errors. Patch 2/11 When a page is swapped or migrated, metadata associated with the page must be saved so it can be restored later. This patch adds a new function that saves/restores this metadata when updating pte upon a swap/migration. Patch 3/11 SPARC M7 processor adds new fields to control registers to support ADI feature. It also adds a new exception for precise traps on tag mismatch. This patch adds definitions for the new control register fields, new ASIs for ADI and an exc
Re: [PATCH 0/6] DISCONTIGMEM support for PPC32
On Wed, Feb 21, 2018 at 04:02:25PM +0100, Christophe LEROY wrote: [...] > > > 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 ? > > > > Using page_is_ram in __ioremap_caller and the same memblock-based > > approach that's used on PPC64 on PPC32 *should* work, but I think due to > > the following line in initmem_init, it won't: > > > > memblock_set_node(0, (phys_addr_t)ULLONG_MAX, &memblock.memory, 0); > > Can't we just fix that ? Turns out I was completely wrong about this. memblock_set_node as called above only assigns all memory to node 0 and merges *adjacent* memblocks. It doesn't merge the memblocks on the Wii, which are far apart. So now I actually have a working patchset (coming soon), that's a good deal shorter than this patchset, and hopefully won't break CONFIG_HIGHMEM in the same way. Thanks for your input! :) Jonathan Neuschäfer signature.asc Description: PGP signature
Re: [PATCH] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
On Wed, 2018-02-21 at 17:43 +1100, Balbir Singh wrote:
> On Wed, Feb 21, 2018 at 3:57 PM, Alastair D'Silva om> wrote:
> > From: Alastair D'Silva
> >
> > 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
> > ---
> > 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?
>
It's metadata for the 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
>
Setting it to 0 is for the reader, not the compiler. I'm not clear on
the benefit of starting the version at 1, could you clarify?
> > +
> > + 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?
>
It pretty much is a function, it returns to userspace metadata about
the descriptor being operated on.
> > + __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?
>
Yes, I would rather call it per_pasid_mmio_size, but consistency with
the rest of the driver (& exposed sysfs entries) is also important.
> > + // End version 0 fields
> > +
> > + __u64 reserved[13]; // Total of 16*u64
> > +};
>
>
> Balbir Singh.
>
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australiamob: 0423 762 819
Re: [PATCH] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
On Wed, 2018-02-21 at 12:25 +0100, Frederic Barrat wrote: > > Le 21/02/2018 à 07:43, Balbir Singh a écrit : > > On Wed, Feb 21, 2018 at 3:57 PM, Alastair D'Silva > .com> wrote: > > > From: Alastair D'Silva > > > > > > 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 > > > > > 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? > > My bad, I forgot to mention it before. There's a somewhat high-level > description which needs updating in: > Documentation/accelerators/ocxl.rst > > It doesn't go down to the level of the structure members, but at > least > all ioctl commands should have a brief description. > I'll update the docs. > lpc_size could be added. It's currently useless to the library, but > doesn't hurt. The one which was giving me troubles on a previous > version > of this patch was the lpc numa node ID, since that was experimental > code > and felt out of place considering what's been upstreamed in skiboot > and > linux so far. I'd rather add the LPC members when the rest of the LPC code goes in. At the moment, the LPC size represents the window size (as a power of 2), whereas we expect that it should represent the actual amount of LPC memory exposed. I would rather avoid changing semantics of members in released code, or burning another reserved member for the updated definition if we can avoid it. -- Alastair D'Silva Open Source Developer Linux Technology Centre, IBM Australia mob: 0423 762 819
[PATCH v12 0/3] mm, x86, powerpc: Enhancements to Memory Protection Keys.
This patch series provides arch-neutral enhancements to enable memory-keys on new architecutes, and the corresponding changes in x86 and powerpc specific code to support that. a) Provides ability to support upto 32 keys. PowerPC can handle 32 keys and hence needs this. b) Arch-neutral code; and not the arch-specific code, determines the format of the string, that displays the key for each vma in smaps. History: --- version v12: (1) fixed compilation errors seen with various x86 configs. version v11: (1) code that displays key in smaps is not any more defined under CONFIG_ARCH_HAS_PKEYS. - Comment by Eric W. Biederman and Michal Hocko (2) merged two patches that implemented (1). - comment by Michal Hocko version prior to v11: (1) used one additional bit from VM_HIGH_ARCH_* to support 32 keys. - Suggestion by Dave Hansen. (2) powerpc specific changes to support memory keys. Ram Pai (3): mm, powerpc, x86: define VM_PKEY_BITx bits if CONFIG_ARCH_HAS_PKEYS is enabled mm, powerpc, x86: introduce an additional vma bit for powerpc pkey mm, x86, powerpc: display pkey in smaps only if arch supports pkeys arch/powerpc/include/asm/mmu_context.h |5 - arch/x86/include/asm/mmu_context.h |5 - arch/x86/include/asm/pkeys.h |1 + arch/x86/kernel/fpu/xstate.c |5 + arch/x86/kernel/setup.c|8 fs/proc/task_mmu.c | 15 --- include/linux/mm.h | 12 +++- include/linux/pkeys.h |7 ++- 8 files changed, 27 insertions(+), 31 deletions(-)
[PATCH v12 2/3] mm, powerpc, x86: introduce an additional vma bit for powerpc pkey
Currently only 4bits are allocated in the vma flags to hold 16 keys. This is sufficient for x86. PowerPC supports 32 keys, which needs 5bits. This patch allocates an additional bit. cc: Dave Hansen cc: Michael Ellermen cc: Benjamin Herrenschmidt cc: Andrew Morton Reviewed-by: Ingo Molnar Acked-by: Balbir Singh Signed-off-by: Ram Pai --- fs/proc/task_mmu.c |1 + include/linux/mm.h |3 ++- 2 files changed, 3 insertions(+), 1 deletions(-) diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c index 6b996d0..6d83bb7 100644 --- a/fs/proc/task_mmu.c +++ b/fs/proc/task_mmu.c @@ -685,6 +685,7 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma) [ilog2(VM_PKEY_BIT1)] = "", [ilog2(VM_PKEY_BIT2)] = "", [ilog2(VM_PKEY_BIT3)] = "", + [ilog2(VM_PKEY_BIT4)] = "", #endif /* CONFIG_ARCH_HAS_PKEYS */ }; size_t i; diff --git a/include/linux/mm.h b/include/linux/mm.h index ad207ad..d534f46 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -231,9 +231,10 @@ extern int overcommit_kbytes_handler(struct ctl_table *, int, void __user *, #ifdef CONFIG_ARCH_HAS_PKEYS # define VM_PKEY_SHIFT VM_HIGH_ARCH_BIT_0 # define VM_PKEY_BIT0 VM_HIGH_ARCH_0 /* A protection key is a 4-bit value */ -# define VM_PKEY_BIT1 VM_HIGH_ARCH_1 +# define VM_PKEY_BIT1 VM_HIGH_ARCH_1 /* on x86 and 5-bit value on ppc64 */ # define VM_PKEY_BIT2 VM_HIGH_ARCH_2 # define VM_PKEY_BIT3 VM_HIGH_ARCH_3 +# define VM_PKEY_BIT4 VM_HIGH_ARCH_4 #endif /* CONFIG_ARCH_HAS_PKEYS */ #if defined(CONFIG_X86) -- 1.7.1
[PATCH v12 1/3] mm, powerpc, x86: define VM_PKEY_BITx bits if CONFIG_ARCH_HAS_PKEYS is enabled
VM_PKEY_BITx are defined only if CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS is enabled. Powerpc also needs these bits. Hence lets define the VM_PKEY_BITx bits for any architecture that enables CONFIG_ARCH_HAS_PKEYS. cc: Dave Hansen cc: Michael Ellermen cc: Benjamin Herrenschmidt cc: Andrew Morton Signed-off-by: Ram Pai Reviewed-by: Ingo Molnar Reviewed-by: Aneesh Kumar K.V --- fs/proc/task_mmu.c |4 ++-- include/linux/mm.h |9 + 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c index ec6d298..6b996d0 100644 --- a/fs/proc/task_mmu.c +++ b/fs/proc/task_mmu.c @@ -679,13 +679,13 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma) [ilog2(VM_MERGEABLE)] = "mg", [ilog2(VM_UFFD_MISSING)]= "um", [ilog2(VM_UFFD_WP)] = "uw", -#ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS +#ifdef CONFIG_ARCH_HAS_PKEYS /* These come out via ProtectionKey: */ [ilog2(VM_PKEY_BIT0)] = "", [ilog2(VM_PKEY_BIT1)] = "", [ilog2(VM_PKEY_BIT2)] = "", [ilog2(VM_PKEY_BIT3)] = "", -#endif +#endif /* CONFIG_ARCH_HAS_PKEYS */ }; size_t i; diff --git a/include/linux/mm.h b/include/linux/mm.h index ad06d42..ad207ad 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -228,15 +228,16 @@ extern int overcommit_kbytes_handler(struct ctl_table *, int, void __user *, #define VM_HIGH_ARCH_4 BIT(VM_HIGH_ARCH_BIT_4) #endif /* CONFIG_ARCH_USES_HIGH_VMA_FLAGS */ -#if defined(CONFIG_X86) -# define VM_PATVM_ARCH_1 /* PAT reserves whole VMA at once (x86) */ -#if defined (CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS) +#ifdef CONFIG_ARCH_HAS_PKEYS # define VM_PKEY_SHIFT VM_HIGH_ARCH_BIT_0 # define VM_PKEY_BIT0 VM_HIGH_ARCH_0 /* A protection key is a 4-bit value */ # define VM_PKEY_BIT1 VM_HIGH_ARCH_1 # define VM_PKEY_BIT2 VM_HIGH_ARCH_2 # define VM_PKEY_BIT3 VM_HIGH_ARCH_3 -#endif +#endif /* CONFIG_ARCH_HAS_PKEYS */ + +#if defined(CONFIG_X86) +# define VM_PATVM_ARCH_1 /* PAT reserves whole VMA at once (x86) */ #elif defined(CONFIG_PPC) # define VM_SAOVM_ARCH_1 /* Strong Access Ordering (powerpc) */ #elif defined(CONFIG_PARISC) -- 1.7.1
[PATCH v12 3/3] mm, x86, powerpc: display pkey in smaps only if arch supports pkeys
Currently the architecture specific code is expected to
display the protection keys in smap for a given vma.
This can lead to redundant code and possibly to divergent
formats in which the key gets displayed.
This patch changes the implementation. It displays the
pkey only if the architecture support pkeys, i.e
arch_pkeys_enabled() returns true. This patch
provides x86 implementation for arch_pkeys_enabled().
x86 arch_show_smap() function is not needed anymore.
Deleting it.
cc: Dave Hansen
cc: Michael Ellermen
cc: Benjamin Herrenschmidt
cc: Andrew Morton
Signed-off-by: Thiago Jung Bauermann
(fixed compilation errors for x86 configs)
Acked-by: Michal Hocko
Reviewed-by: Ingo Molnar
Signed-off-by: Ram Pai
---
arch/powerpc/include/asm/mmu_context.h |5 -
arch/x86/include/asm/mmu_context.h |5 -
arch/x86/include/asm/pkeys.h |1 +
arch/x86/kernel/fpu/xstate.c |5 +
arch/x86/kernel/setup.c|8
fs/proc/task_mmu.c | 10 +-
include/linux/pkeys.h |7 ++-
7 files changed, 17 insertions(+), 24 deletions(-)
diff --git a/arch/powerpc/include/asm/mmu_context.h
b/arch/powerpc/include/asm/mmu_context.h
index 051b3d6..566b3c2 100644
--- a/arch/powerpc/include/asm/mmu_context.h
+++ b/arch/powerpc/include/asm/mmu_context.h
@@ -203,11 +203,6 @@ static inline bool arch_vma_access_permitted(struct
vm_area_struct *vma,
#define thread_pkey_regs_restore(new_thread, old_thread)
#define thread_pkey_regs_init(thread)
-static inline int vma_pkey(struct vm_area_struct *vma)
-{
- return 0;
-}
-
static inline u64 pte_to_hpte_pkey_bits(u64 pteflags)
{
return 0x0UL;
diff --git a/arch/x86/include/asm/mmu_context.h
b/arch/x86/include/asm/mmu_context.h
index c931b88..134f3a0 100644
--- a/arch/x86/include/asm/mmu_context.h
+++ b/arch/x86/include/asm/mmu_context.h
@@ -294,11 +294,6 @@ static inline int vma_pkey(struct vm_area_struct *vma)
return (vma->vm_flags & vma_pkey_mask) >> VM_PKEY_SHIFT;
}
-#else
-static inline int vma_pkey(struct vm_area_struct *vma)
-{
- return 0;
-}
#endif
/*
diff --git a/arch/x86/include/asm/pkeys.h b/arch/x86/include/asm/pkeys.h
index a0ba1ff..f6c287b 100644
--- a/arch/x86/include/asm/pkeys.h
+++ b/arch/x86/include/asm/pkeys.h
@@ -6,6 +6,7 @@
extern int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
unsigned long init_val);
+extern bool arch_pkeys_enabled(void);
/*
* Try to dedicate one of the protection keys to be used as an
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 87a57b7..4f566e9 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -945,6 +945,11 @@ int arch_set_user_pkey_access(struct task_struct *tsk, int
pkey,
return 0;
}
+
+bool arch_pkeys_enabled(void)
+{
+ return boot_cpu_has(X86_FEATURE_OSPKE);
+}
#endif /* ! CONFIG_ARCH_HAS_PKEYS */
/*
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 1ae67e9..d0539e3 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1314,11 +1314,3 @@ static int __init register_kernel_offset_dumper(void)
return 0;
}
__initcall(register_kernel_offset_dumper);
-
-void arch_show_smap(struct seq_file *m, struct vm_area_struct *vma)
-{
- if (!boot_cpu_has(X86_FEATURE_OSPKE))
- return;
-
- seq_printf(m, "ProtectionKey: %8u\n", vma_pkey(vma));
-}
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 6d83bb7..70aa912 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -18,10 +18,12 @@
#include
#include
#include
+#include
#include
#include
#include
+#include
#include "internal.h"
void task_mem(struct seq_file *m, struct mm_struct *mm)
@@ -733,10 +735,6 @@ static int smaps_hugetlb_range(pte_t *pte, unsigned long
hmask,
}
#endif /* HUGETLB_PAGE */
-void __weak arch_show_smap(struct seq_file *m, struct vm_area_struct *vma)
-{
-}
-
static int show_smap(struct seq_file *m, void *v, int is_pid)
{
struct proc_maps_private *priv = m->private;
@@ -856,9 +854,11 @@ static int show_smap(struct seq_file *m, void *v, int
is_pid)
(unsigned long)(mss->pss >> (10 + PSS_SHIFT)));
if (!rollup_mode) {
- arch_show_smap(m, vma);
+ if (arch_pkeys_enabled())
+ seq_printf(m, "ProtectionKey: %8u\n", vma_pkey(vma));
show_smap_vma_flags(m, vma);
}
+
m_cache_vma(m, vma);
return ret;
}
diff --git a/include/linux/pkeys.h b/include/linux/pkeys.h
index 0794ca7..49dff15 100644
--- a/include/linux/pkeys.h
+++ b/include/linux/pkeys.h
@@ -3,7 +3,6 @@
#define _LINUX_PKEYS_H
#include
-#include
#ifdef CONFIG_ARCH_HAS_PKEYS
#include
@@ -13,6 +12,7 @@
#define arch_override_mprotect_pkey(vma, prot, pkey) (0)
#define PKEY_DEDICATED_EXECUTE_ONLY 0
#define
[PATCH RFC 1/1] powerpc/pseries: fix EEH recovery of IOV devices
Currently EEH recovery will fail on pSeries platforms for
passed-through physical devices that support IOV, when CONFIG_PCI_IOV
is set and the hypervisor doesn't provide a device tree node
"ibm,open-sriov-vf-bar-info" for the device. (Found on an IOV capable
device using the ipr driver.)
EEH recovery fails in pci_enable_resources() at the check on
r->parent, because r->flags is set and r->parent is not. This state
is due to sriov_init() setting the start, end and flags members of the
IOV BARs but the parent not being set later in
pseries_pci_fixup_iov_resources(), because the
"ibm,open-sriov-vf-bar-info" property is missing, causing it to bail
out early.
Correct this by zeroing the IOV resources in the bailout path, so that
they are not seen by pci_enable_resources().
Signed-off-by: Sam Bobroff
---
Hi,
This is a fix to allow EEH recovery to succeed in a specific situation,
which I've tried to explain in the commit message. But while I'm fairly sure of
the situation leading up to the problem, I'm not sure how it should be fixed,
which is why I've posted this as an RFC.
Just zeroing out the flags is sufficient in my tests but it seemed cleaner to
also clear start and end, but other changes would work as well: just clearing
the flags, or just removing IORESOURCE_IO and IORESOURCE_MEM or even adding
some new flag and also adjusting the test. Perhaps there's something else.
I'm not sure what implications these choices have for the generic PCI code so I
would appreciate feedback on it.
Note that this problem doesn't seem to have been introduced by
pseries_pci_fixup_iov_resources() (which was added recently): recovery
already failed (in this way) before it's introduction.
arch/powerpc/platforms/pseries/setup.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/setup.c
b/arch/powerpc/platforms/pseries/setup.c
index 372d7ada1a0c..019836ffe53d 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -618,13 +618,24 @@ static void pseries_pci_fixup_iov_resources(struct
pci_dev *pdev)
{
const int *indexes;
struct device_node *dn = pci_device_to_OF_node(pdev);
+ int i;
+ struct resource *r;
if (!pdev->is_physfn || pdev->is_added)
return;
/*Firmware must support open sriov otherwise dont configure*/
indexes = of_get_property(dn, "ibm,open-sriov-vf-bar-info", NULL);
- if (!indexes)
+ if (!indexes) {
+ /* Hide the BARs we can't configure, otherwise other
+* code may see r->flags != 0 and r->parent == 0
+* and raise an error. */
+ pci_warn(pdev, "No hypervisor support for SRIOV on this device,
IOV BARs disabled.\n");
+ for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
+ r = &pdev->resource[PCI_IOV_RESOURCES + i];
+ r->start = r->end = r->flags = 0;
+ }
return;
+ }
/* Assign the addresses from device tree*/
of_pci_parse_iov_addrs(pdev, indexes);
}
--
2.16.1.74.g9b0b1f47b
[PATCH v12 00/22] selftests, powerpc, x86 : Memory Protection Keys
Memory protection keys enables an application to protect its address space from inadvertent access by its own code. This feature is now enabled on powerpc architecture and integrated in 4.16-rc1. The patches move the selftests to arch neutral directory and enhance their test coverage. Test Verified for correctness on powerpc and on x86 architectures(using EC2 ubuntu VM instance). History: --- version 12: (1) fixed the offset of pkey field in the siginfo structure for x86_64 and powerpc. And tries to use the actual field if the headers have it defined. version 11: (1) fixed a deadlock in the ptrace testcase. version 10 and prior: (1) moved the testcase to arch neutral directory (2) split the changes into incremental patches. Ram Pai (21): selftests/x86: Move protecton key selftest to arch neutral directory selftests/vm: rename all references to pkru to a generic name selftests/vm: move generic definitions to header file selftests/vm: typecast the pkey register selftests/vm: generic function to handle shadow key register selftests/vm: fix the wrong assert in pkey_disable_set() selftests/vm: fixed bugs in pkey_disable_clear() selftests/vm: clear the bits in shadow reg when a pkey is freed. selftests/vm: fix alloc_random_pkey() to make it really random selftests/vm: introduce two arch independent abstraction selftests/vm: pkey register should match shadow pkey selftests/vm: generic cleanup selftests/vm: powerpc implementation for generic abstraction selftests/vm: clear the bits in shadow reg when a pkey is freed. selftests/vm: powerpc implementation to check support for pkey selftests/vm: fix an assertion in test_pkey_alloc_exhaust() selftests/vm: associate key on a mapped page and detect access violation selftests/vm: associate key on a mapped page and detect write violation selftests/vm: detect write violation on a mapped access-denied-key page selftests/vm: testcases must restore pkey-permissions selftests/vm: sub-page allocator Thiago Jung Bauermann (1): selftests/vm: Fix deadlock in protection_keys.c tools/testing/selftests/vm/Makefile |1 + tools/testing/selftests/vm/pkey-helpers.h | 434 tools/testing/selftests/vm/protection_keys.c | 1471 + tools/testing/selftests/x86/Makefile |2 +- tools/testing/selftests/x86/pkey-helpers.h| 223 tools/testing/selftests/x86/protection_keys.c | 1407 --- 6 files changed, 1907 insertions(+), 1631 deletions(-) create mode 100644 tools/testing/selftests/vm/pkey-helpers.h create mode 100644 tools/testing/selftests/vm/protection_keys.c delete mode 100644 tools/testing/selftests/x86/pkey-helpers.h delete mode 100644 tools/testing/selftests/x86/protection_keys.c
[PATCH v12 01/22] selftests/x86: Move protecton key selftest to arch neutral directory
cc: Dave Hansen
cc: Florian Weimer
Signed-off-by: Ram Pai
---
tools/testing/selftests/vm/Makefile |1 +
tools/testing/selftests/vm/pkey-helpers.h | 223
tools/testing/selftests/vm/protection_keys.c | 1407 +
tools/testing/selftests/x86/Makefile |2 +-
tools/testing/selftests/x86/pkey-helpers.h| 223
tools/testing/selftests/x86/protection_keys.c | 1407 -
6 files changed, 1632 insertions(+), 1631 deletions(-)
create mode 100644 tools/testing/selftests/vm/pkey-helpers.h
create mode 100644 tools/testing/selftests/vm/protection_keys.c
delete mode 100644 tools/testing/selftests/x86/pkey-helpers.h
delete mode 100644 tools/testing/selftests/x86/protection_keys.c
diff --git a/tools/testing/selftests/vm/Makefile
b/tools/testing/selftests/vm/Makefile
index fdefa22..9788a58 100644
--- a/tools/testing/selftests/vm/Makefile
+++ b/tools/testing/selftests/vm/Makefile
@@ -20,6 +20,7 @@ TEST_GEN_FILES += transhuge-stress
TEST_GEN_FILES += userfaultfd
TEST_GEN_FILES += va_128TBswitch
TEST_GEN_FILES += virtual_address_range
+TEST_GEN_FILES += protection_keys
TEST_PROGS := run_vmtests
diff --git a/tools/testing/selftests/vm/pkey-helpers.h
b/tools/testing/selftests/vm/pkey-helpers.h
new file mode 100644
index 000..b3cb767
--- /dev/null
+++ b/tools/testing/selftests/vm/pkey-helpers.h
@@ -0,0 +1,223 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _PKEYS_HELPER_H
+#define _PKEYS_HELPER_H
+#define _GNU_SOURCE
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define NR_PKEYS 16
+#define PKRU_BITS_PER_PKEY 2
+
+#ifndef DEBUG_LEVEL
+#define DEBUG_LEVEL 0
+#endif
+#define DPRINT_IN_SIGNAL_BUF_SIZE 4096
+extern int dprint_in_signal;
+extern char dprint_in_signal_buffer[DPRINT_IN_SIGNAL_BUF_SIZE];
+static inline void sigsafe_printf(const char *format, ...)
+{
+ va_list ap;
+
+ va_start(ap, format);
+ if (!dprint_in_signal) {
+ vprintf(format, ap);
+ } else {
+ int ret;
+ int len = vsnprintf(dprint_in_signal_buffer,
+ DPRINT_IN_SIGNAL_BUF_SIZE,
+ format, ap);
+ /*
+* len is amount that would have been printed,
+* but actual write is truncated at BUF_SIZE.
+*/
+ if (len > DPRINT_IN_SIGNAL_BUF_SIZE)
+ len = DPRINT_IN_SIGNAL_BUF_SIZE;
+ ret = write(1, dprint_in_signal_buffer, len);
+ if (ret < 0)
+ abort();
+ }
+ va_end(ap);
+}
+#define dprintf_level(level, args...) do { \
+ if (level <= DEBUG_LEVEL) \
+ sigsafe_printf(args); \
+ fflush(NULL); \
+} while (0)
+#define dprintf0(args...) dprintf_level(0, args)
+#define dprintf1(args...) dprintf_level(1, args)
+#define dprintf2(args...) dprintf_level(2, args)
+#define dprintf3(args...) dprintf_level(3, args)
+#define dprintf4(args...) dprintf_level(4, args)
+
+extern unsigned int shadow_pkru;
+static inline unsigned int __rdpkru(void)
+{
+ unsigned int eax, edx;
+ unsigned int ecx = 0;
+ unsigned int pkru;
+
+ asm volatile(".byte 0x0f,0x01,0xee\n\t"
+: "=a" (eax), "=d" (edx)
+: "c" (ecx));
+ pkru = eax;
+ return pkru;
+}
+
+static inline unsigned int _rdpkru(int line)
+{
+ unsigned int pkru = __rdpkru();
+
+ dprintf4("rdpkru(line=%d) pkru: %x shadow: %x\n",
+ line, pkru, shadow_pkru);
+ assert(pkru == shadow_pkru);
+
+ return pkru;
+}
+
+#define rdpkru() _rdpkru(__LINE__)
+
+static inline void __wrpkru(unsigned int pkru)
+{
+ unsigned int eax = pkru;
+ unsigned int ecx = 0;
+ unsigned int edx = 0;
+
+ dprintf4("%s() changing %08x to %08x\n", __func__, __rdpkru(), pkru);
+ asm volatile(".byte 0x0f,0x01,0xef\n\t"
+: : "a" (eax), "c" (ecx), "d" (edx));
+ assert(pkru == __rdpkru());
+}
+
+static inline void wrpkru(unsigned int pkru)
+{
+ dprintf4("%s() changing %08x to %08x\n", __func__, __rdpkru(), pkru);
+ /* will do the shadow check for us: */
+ rdpkru();
+ __wrpkru(pkru);
+ shadow_pkru = pkru;
+ dprintf4("%s(%08x) pkru: %08x\n", __func__, pkru, __rdpkru());
+}
+
+/*
+ * These are technically racy. since something could
+ * change PKRU between the read and the write.
+ */
+static inline void __pkey_access_allow(int pkey, int do_allow)
+{
+ unsigned int pkru = rdpkru();
+ int bit = pkey * 2;
+
+ if (do_allow)
+ pkru &= (1
[PATCH v12 02/22] selftests/vm: rename all references to pkru to a generic name
some pkru references are named to pkey_reg
and some prku references are renamed to pkey
cc: Dave Hansen
cc: Florian Weimer
Signed-off-by: Ram Pai
---
tools/testing/selftests/vm/pkey-helpers.h| 85 +-
tools/testing/selftests/vm/protection_keys.c | 227 ++
2 files changed, 164 insertions(+), 148 deletions(-)
diff --git a/tools/testing/selftests/vm/pkey-helpers.h
b/tools/testing/selftests/vm/pkey-helpers.h
index b3cb767..a568166 100644
--- a/tools/testing/selftests/vm/pkey-helpers.h
+++ b/tools/testing/selftests/vm/pkey-helpers.h
@@ -14,7 +14,7 @@
#include
#define NR_PKEYS 16
-#define PKRU_BITS_PER_PKEY 2
+#define PKEY_BITS_PER_PKEY 2
#ifndef DEBUG_LEVEL
#define DEBUG_LEVEL 0
@@ -57,85 +57,88 @@ static inline void sigsafe_printf(const char *format, ...)
#define dprintf3(args...) dprintf_level(3, args)
#define dprintf4(args...) dprintf_level(4, args)
-extern unsigned int shadow_pkru;
-static inline unsigned int __rdpkru(void)
+extern unsigned int shadow_pkey_reg;
+static inline unsigned int __rdpkey_reg(void)
{
unsigned int eax, edx;
unsigned int ecx = 0;
- unsigned int pkru;
+ unsigned int pkey_reg;
asm volatile(".byte 0x0f,0x01,0xee\n\t"
: "=a" (eax), "=d" (edx)
: "c" (ecx));
- pkru = eax;
- return pkru;
+ pkey_reg = eax;
+ return pkey_reg;
}
-static inline unsigned int _rdpkru(int line)
+static inline unsigned int _rdpkey_reg(int line)
{
- unsigned int pkru = __rdpkru();
+ unsigned int pkey_reg = __rdpkey_reg();
- dprintf4("rdpkru(line=%d) pkru: %x shadow: %x\n",
- line, pkru, shadow_pkru);
- assert(pkru == shadow_pkru);
+ dprintf4("rdpkey_reg(line=%d) pkey_reg: %x shadow: %x\n",
+ line, pkey_reg, shadow_pkey_reg);
+ assert(pkey_reg == shadow_pkey_reg);
- return pkru;
+ return pkey_reg;
}
-#define rdpkru() _rdpkru(__LINE__)
+#define rdpkey_reg() _rdpkey_reg(__LINE__)
-static inline void __wrpkru(unsigned int pkru)
+static inline void __wrpkey_reg(unsigned int pkey_reg)
{
- unsigned int eax = pkru;
+ unsigned int eax = pkey_reg;
unsigned int ecx = 0;
unsigned int edx = 0;
- dprintf4("%s() changing %08x to %08x\n", __func__, __rdpkru(), pkru);
+ dprintf4("%s() changing %08x to %08x\n", __func__,
+ __rdpkey_reg(), pkey_reg);
asm volatile(".byte 0x0f,0x01,0xef\n\t"
: : "a" (eax), "c" (ecx), "d" (edx));
- assert(pkru == __rdpkru());
+ assert(pkey_reg == __rdpkey_reg());
}
-static inline void wrpkru(unsigned int pkru)
+static inline void wrpkey_reg(unsigned int pkey_reg)
{
- dprintf4("%s() changing %08x to %08x\n", __func__, __rdpkru(), pkru);
+ dprintf4("%s() changing %08x to %08x\n", __func__,
+ __rdpkey_reg(), pkey_reg);
/* will do the shadow check for us: */
- rdpkru();
- __wrpkru(pkru);
- shadow_pkru = pkru;
- dprintf4("%s(%08x) pkru: %08x\n", __func__, pkru, __rdpkru());
+ rdpkey_reg();
+ __wrpkey_reg(pkey_reg);
+ shadow_pkey_reg = pkey_reg;
+ dprintf4("%s(%08x) pkey_reg: %08x\n", __func__,
+ pkey_reg, __rdpkey_reg());
}
/*
* These are technically racy. since something could
- * change PKRU between the read and the write.
+ * change PKEY register between the read and the write.
*/
static inline void __pkey_access_allow(int pkey, int do_allow)
{
- unsigned int pkru = rdpkru();
+ unsigned int pkey_reg = rdpkey_reg();
int bit = pkey * 2;
if (do_allow)
- pkru &= (1<>>>===SIGSEGV\n");
- dprintf1("%s()::%d, pkru: 0x%x shadow: %x\n", __func__, __LINE__,
- __rdpkru(), shadow_pkru);
+ dprintf1("%s()::%d, pkey_reg: 0x%x shadow: %x\n", __func__, __LINE__,
+ __rdpkey_reg(), shadow_pkey_reg);
trapno = uctxt->uc_mcontext.gregs[REG_TRAPNO];
ip = uctxt->uc_mcontext.gregs[REG_IP_IDX];
@@ -275,19 +275,19 @@ void signal_handler(int signum, siginfo_t *si, void
*vucontext)
*/
fpregs += 0x70;
#endif
- pkru_offset = pkru_xstate_offset();
- pkru_ptr = (void *)(&fpregs[pkru_offset]);
+ pkey_reg_offset = pkey_reg_xstate_offset();
+ pkey_reg_ptr = (void *)(&fpregs[pkey_reg_offset]);
dprintf1("siginfo: %p\n", si);
dprintf1(" fpregs: %p\n", fpregs);
/*
-* If we got a PKRU fault, we *HAVE* to have at least one bit set in
+* If we got a PKEY fault, we *HAVE* to have at least one bit set in
* here.
*/
- dprintf1("pkru_xstate_offset: %d\n", pkru_xstate_offset());
+ dprintf1("pkey_reg_xstate_offset: %d\n", pkey_reg_xstate_offset());
if (DEBUG_LEVEL > 4)
- dump
[PATCH v12 03/22] selftests/vm: move generic definitions to header file
Moved all the generic definition and helper functions to the
header file
cc: Dave Hansen
cc: Florian Weimer
Signed-off-by: Ram Pai
---
tools/testing/selftests/vm/pkey-helpers.h| 62 ++--
tools/testing/selftests/vm/protection_keys.c | 66 --
2 files changed, 57 insertions(+), 71 deletions(-)
diff --git a/tools/testing/selftests/vm/pkey-helpers.h
b/tools/testing/selftests/vm/pkey-helpers.h
index a568166..c1bc761 100644
--- a/tools/testing/selftests/vm/pkey-helpers.h
+++ b/tools/testing/selftests/vm/pkey-helpers.h
@@ -13,8 +13,31 @@
#include
#include
+/* Define some kernel-like types */
+#define u8 uint8_t
+#define u16 uint16_t
+#define u32 uint32_t
+#define u64 uint64_t
+
+#ifdef __i386__
+#define SYS_mprotect_key 380
+#define SYS_pkey_alloc 381
+#define SYS_pkey_free 382
+#define REG_IP_IDX REG_EIP
+#define si_pkey_offset 0x14
+#else
+#define SYS_mprotect_key 329
+#define SYS_pkey_alloc 330
+#define SYS_pkey_free 331
+#define REG_IP_IDX REG_RIP
+#define si_pkey_offset 0x20
+#endif
+
#define NR_PKEYS 16
#define PKEY_BITS_PER_PKEY 2
+#define PKEY_DISABLE_ACCESS0x1
+#define PKEY_DISABLE_WRITE 0x2
+#define HPAGE_SIZE (1UL<<21)
#ifndef DEBUG_LEVEL
#define DEBUG_LEVEL 0
@@ -141,11 +164,6 @@ static inline void __pkey_write_allow(int pkey, int
do_allow_write)
dprintf4("pkey_reg now: %08x\n", rdpkey_reg());
}
-#define PROT_PKEY0 0x10/* protection key value (bit 0) */
-#define PROT_PKEY1 0x20/* protection key value (bit 1) */
-#define PROT_PKEY2 0x40/* protection key value (bit 2) */
-#define PROT_PKEY3 0x80/* protection key value (bit 3) */
-
#define PAGE_SIZE 4096
#define MB (1<<20)
@@ -223,4 +241,38 @@ int pkey_reg_xstate_offset(void)
return xstate_offset;
}
+static inline void __page_o_noops(void)
+{
+ /* 8-bytes of instruction * 512 bytes = 1 page */
+ asm(".rept 512 ; nopl 0x7eee(%eax) ; .endr");
+}
+
#endif /* _PKEYS_HELPER_H */
+
+#define ARRAY_SIZE(x) (sizeof(x) / sizeof(*(x)))
+#define ALIGN_UP(x, align_to) (((x) + ((align_to)-1)) & ~((align_to)-1))
+#define ALIGN_DOWN(x, align_to) ((x) & ~((align_to)-1))
+#define ALIGN_PTR_UP(p, ptr_align_to) \
+ ((typeof(p))ALIGN_UP((unsigned long)(p), ptr_align_to))
+#define ALIGN_PTR_DOWN(p, ptr_align_to) \
+ ((typeof(p))ALIGN_DOWN((unsigned long)(p), ptr_align_to))
+#define __stringify_1(x...) #x
+#define __stringify(x...) __stringify_1(x)
+
+#define PTR_ERR_ENOTSUP ((void *)-ENOTSUP)
+
+int dprint_in_signal;
+char dprint_in_signal_buffer[DPRINT_IN_SIGNAL_BUF_SIZE];
+
+extern void abort_hooks(void);
+#define pkey_assert(condition) do {\
+ if (!(condition)) { \
+ dprintf0("assert() at %s::%d test_nr: %d iteration: %d\n", \
+ __FILE__, __LINE__, \
+ test_nr, iteration_nr); \
+ dprintf0("errno at assert: %d", errno); \
+ abort_hooks(); \
+ assert(condition); \
+ } \
+} while (0)
+#define raw_assert(cond) assert(cond)
diff --git a/tools/testing/selftests/vm/protection_keys.c
b/tools/testing/selftests/vm/protection_keys.c
index 4aebf12..91bade4 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -49,34 +49,9 @@
int test_nr;
unsigned int shadow_pkey_reg;
-
-#define HPAGE_SIZE (1UL<<21)
-#define ARRAY_SIZE(x) (sizeof(x) / sizeof(*(x)))
-#define ALIGN_UP(x, align_to) (((x) + ((align_to)-1)) & ~((align_to)-1))
-#define ALIGN_DOWN(x, align_to) ((x) & ~((align_to)-1))
-#define ALIGN_PTR_UP(p, ptr_align_to) ((typeof(p))ALIGN_UP((unsigned
long)(p),ptr_align_to))
-#define ALIGN_PTR_DOWN(p, ptr_align_to)
((typeof(p))ALIGN_DOWN((unsigned long)(p), ptr_align_to))
-#define __stringify_1(x...) #x
-#define __stringify(x...) __stringify_1(x)
-
-#define PTR_ERR_ENOTSUP ((void *)-ENOTSUP)
-
int dprint_in_signal;
char dprint_in_signal_buffer[DPRINT_IN_SIGNAL_BUF_SIZE];
-extern void abort_hooks(void);
-#define pkey_assert(condition) do {\
- if (!(condition)) { \
- dprintf0("assert() at %s::%d test_nr: %d iteration: %d\n", \
- __FILE__, __LINE__, \
- test_nr, iteration_nr); \
- dprintf0("errno at assert: %d", errno); \
- abort_hooks(); \
- assert(condition); \
- } \
-} while (0)
-#define raw_assert(cond) assert(cond)
-
void cat_into_file(char *str, char *file)
{
int fd = open(file, O_RDWR);
@@ -154,12 +129,6 @@ void abort_hooks(void)
#endif
}
-static inline void __page_o_noops(void)
-{
-
[PATCH v12 04/22] selftests/vm: typecast the pkey register
This is in preparation to accomadate a differing size register
across architectures.
cc: Dave Hansen
cc: Florian Weimer
Signed-off-by: Ram Pai
---
tools/testing/selftests/vm/pkey-helpers.h| 27 +-
tools/testing/selftests/vm/protection_keys.c | 69 ++
2 files changed, 51 insertions(+), 45 deletions(-)
diff --git a/tools/testing/selftests/vm/pkey-helpers.h
b/tools/testing/selftests/vm/pkey-helpers.h
index c1bc761..b6c2133 100644
--- a/tools/testing/selftests/vm/pkey-helpers.h
+++ b/tools/testing/selftests/vm/pkey-helpers.h
@@ -18,6 +18,7 @@
#define u16 uint16_t
#define u32 uint32_t
#define u64 uint64_t
+#define pkey_reg_t u32
#ifdef __i386__
#define SYS_mprotect_key 380
@@ -80,12 +81,12 @@ static inline void sigsafe_printf(const char *format, ...)
#define dprintf3(args...) dprintf_level(3, args)
#define dprintf4(args...) dprintf_level(4, args)
-extern unsigned int shadow_pkey_reg;
-static inline unsigned int __rdpkey_reg(void)
+extern pkey_reg_t shadow_pkey_reg;
+static inline pkey_reg_t __rdpkey_reg(void)
{
unsigned int eax, edx;
unsigned int ecx = 0;
- unsigned int pkey_reg;
+ pkey_reg_t pkey_reg;
asm volatile(".byte 0x0f,0x01,0xee\n\t"
: "=a" (eax), "=d" (edx)
@@ -94,11 +95,11 @@ static inline unsigned int __rdpkey_reg(void)
return pkey_reg;
}
-static inline unsigned int _rdpkey_reg(int line)
+static inline pkey_reg_t _rdpkey_reg(int line)
{
- unsigned int pkey_reg = __rdpkey_reg();
+ pkey_reg_t pkey_reg = __rdpkey_reg();
- dprintf4("rdpkey_reg(line=%d) pkey_reg: %x shadow: %x\n",
+ dprintf4("rdpkey_reg(line=%d) pkey_reg: %016lx shadow: %016lx\n",
line, pkey_reg, shadow_pkey_reg);
assert(pkey_reg == shadow_pkey_reg);
@@ -107,11 +108,11 @@ static inline unsigned int _rdpkey_reg(int line)
#define rdpkey_reg() _rdpkey_reg(__LINE__)
-static inline void __wrpkey_reg(unsigned int pkey_reg)
+static inline void __wrpkey_reg(pkey_reg_t pkey_reg)
{
- unsigned int eax = pkey_reg;
- unsigned int ecx = 0;
- unsigned int edx = 0;
+ pkey_reg_t eax = pkey_reg;
+ pkey_reg_t ecx = 0;
+ pkey_reg_t edx = 0;
dprintf4("%s() changing %08x to %08x\n", __func__,
__rdpkey_reg(), pkey_reg);
@@ -120,7 +121,7 @@ static inline void __wrpkey_reg(unsigned int pkey_reg)
assert(pkey_reg == __rdpkey_reg());
}
-static inline void wrpkey_reg(unsigned int pkey_reg)
+static inline void wrpkey_reg(pkey_reg_t pkey_reg)
{
dprintf4("%s() changing %08x to %08x\n", __func__,
__rdpkey_reg(), pkey_reg);
@@ -138,7 +139,7 @@ static inline void wrpkey_reg(unsigned int pkey_reg)
*/
static inline void __pkey_access_allow(int pkey, int do_allow)
{
- unsigned int pkey_reg = rdpkey_reg();
+ pkey_reg_t pkey_reg = rdpkey_reg();
int bit = pkey * 2;
if (do_allow)
@@ -152,7 +153,7 @@ static inline void __pkey_access_allow(int pkey, int
do_allow)
static inline void __pkey_write_allow(int pkey, int do_allow_write)
{
- long pkey_reg = rdpkey_reg();
+ pkey_reg_t pkey_reg = rdpkey_reg();
int bit = pkey * 2 + 1;
if (do_allow_write)
diff --git a/tools/testing/selftests/vm/protection_keys.c
b/tools/testing/selftests/vm/protection_keys.c
index 91bade4..3ef2569 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -48,7 +48,7 @@
int iteration_nr = 1;
int test_nr;
-unsigned int shadow_pkey_reg;
+pkey_reg_t shadow_pkey_reg;
int dprint_in_signal;
char dprint_in_signal_buffer[DPRINT_IN_SIGNAL_BUF_SIZE];
@@ -158,7 +158,7 @@ void dump_mem(void *dumpme, int len_bytes)
for (i = 0; i < len_bytes; i += sizeof(u64)) {
u64 *ptr = (u64 *)(c + i);
- dprintf1("dump[%03d][@%p]: %016jx\n", i, ptr, *ptr);
+ dprintf1("dump[%03d][@%p]: %016lx\n", i, ptr, *ptr);
}
}
@@ -186,7 +186,7 @@ void signal_handler(int signum, siginfo_t *si, void
*vucontext)
int trapno;
unsigned long ip;
char *fpregs;
- u32 *pkey_reg_ptr;
+ pkey_reg_t *pkey_reg_ptr;
u64 siginfo_pkey;
u32 *si_pkey_ptr;
int pkey_reg_offset;
@@ -194,7 +194,8 @@ void signal_handler(int signum, siginfo_t *si, void
*vucontext)
dprint_in_signal = 1;
dprintf1("===SIGSEGV\n");
- dprintf1("%s()::%d, pkey_reg: 0x%x shadow: %x\n", __func__, __LINE__,
+ dprintf1("%s()::%d, pkey_reg: 0x%016lx shadow: %016lx\n",
+ __func__, __LINE__,
__rdpkey_reg(), shadow_pkey_reg);
trapno = uctxt->uc_mcontext.gregs[REG_TRAPNO];
@@ -202,8 +203,9 @@ void signal_handler(int signum, siginfo_t *si, void
*vucontext)
fpregset = uctxt->uc_mcontext.fpregs;
fpregs = (voi
[PATCH v12 05/22] selftests/vm: generic function to handle shadow key register
helper functions to handler shadow pkey register
cc: Dave Hansen
cc: Florian Weimer
Signed-off-by: Ram Pai
---
tools/testing/selftests/vm/pkey-helpers.h| 27
tools/testing/selftests/vm/protection_keys.c | 34 -
2 files changed, 49 insertions(+), 12 deletions(-)
diff --git a/tools/testing/selftests/vm/pkey-helpers.h
b/tools/testing/selftests/vm/pkey-helpers.h
index b6c2133..7c979ad 100644
--- a/tools/testing/selftests/vm/pkey-helpers.h
+++ b/tools/testing/selftests/vm/pkey-helpers.h
@@ -44,6 +44,33 @@
#define DEBUG_LEVEL 0
#endif
#define DPRINT_IN_SIGNAL_BUF_SIZE 4096
+
+static inline u32 pkey_to_shift(int pkey)
+{
+ return pkey * PKEY_BITS_PER_PKEY;
+}
+
+static inline pkey_reg_t reset_bits(int pkey, pkey_reg_t bits)
+{
+ u32 shift = pkey_to_shift(pkey);
+
+ return ~(bits << shift);
+}
+
+static inline pkey_reg_t left_shift_bits(int pkey, pkey_reg_t bits)
+{
+ u32 shift = pkey_to_shift(pkey);
+
+ return (bits << shift);
+}
+
+static inline pkey_reg_t right_shift_bits(int pkey, pkey_reg_t bits)
+{
+ u32 shift = pkey_to_shift(pkey);
+
+ return (bits >> shift);
+}
+
extern int dprint_in_signal;
extern char dprint_in_signal_buffer[DPRINT_IN_SIGNAL_BUF_SIZE];
static inline void sigsafe_printf(const char *format, ...)
diff --git a/tools/testing/selftests/vm/protection_keys.c
b/tools/testing/selftests/vm/protection_keys.c
index 3ef2569..83216c5 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -374,7 +374,7 @@ u32 pkey_get(int pkey, unsigned long flags)
__func__, pkey, flags, 0, 0);
dprintf2("%s() raw pkey_reg: %x\n", __func__, pkey_reg);
- shifted_pkey_reg = (pkey_reg >> (pkey * PKEY_BITS_PER_PKEY));
+ shifted_pkey_reg = right_shift_bits(pkey, pkey_reg);
dprintf2("%s() shifted_pkey_reg: %x\n", __func__, shifted_pkey_reg);
masked_pkey_reg = shifted_pkey_reg & mask;
dprintf2("%s() masked pkey_reg: %x\n", __func__, masked_pkey_reg);
@@ -397,9 +397,9 @@ int pkey_set(int pkey, unsigned long rights, unsigned long
flags)
/* copy old pkey_reg */
new_pkey_reg = old_pkey_reg;
/* mask out bits from pkey in old value: */
- new_pkey_reg &= ~(mask << (pkey * PKEY_BITS_PER_PKEY));
+ new_pkey_reg &= reset_bits(pkey, mask);
/* OR in new bits for pkey: */
- new_pkey_reg |= (rights << (pkey * PKEY_BITS_PER_PKEY));
+ new_pkey_reg |= left_shift_bits(pkey, rights);
__wrpkey_reg(new_pkey_reg);
@@ -430,7 +430,7 @@ void pkey_disable_set(int pkey, int flags)
ret = pkey_set(pkey, pkey_rights, syscall_flags);
assert(!ret);
/*pkey_reg and flags have the same format */
- shadow_pkey_reg |= flags << (pkey * 2);
+ shadow_pkey_reg |= left_shift_bits(pkey, flags);
dprintf1("%s(%d) shadow: 0x%016lx\n",
__func__, pkey, shadow_pkey_reg);
@@ -465,7 +465,7 @@ void pkey_disable_clear(int pkey, int flags)
ret = pkey_set(pkey, pkey_rights, 0);
/* pkey_reg and flags have the same format */
- shadow_pkey_reg &= ~(flags << (pkey * 2));
+ shadow_pkey_reg &= reset_bits(pkey, flags);
pkey_assert(ret >= 0);
pkey_rights = pkey_get(pkey, syscall_flags);
@@ -523,6 +523,21 @@ int sys_pkey_alloc(unsigned long flags, unsigned long
init_val)
return ret;
}
+void pkey_setup_shadow(void)
+{
+ shadow_pkey_reg = __rdpkey_reg();
+}
+
+void pkey_reset_shadow(u32 key)
+{
+ shadow_pkey_reg &= reset_bits(key, 0x3);
+}
+
+void pkey_set_shadow(u32 key, u64 init_val)
+{
+ shadow_pkey_reg |= left_shift_bits(key, init_val);
+}
+
int alloc_pkey(void)
{
int ret;
@@ -540,7 +555,7 @@ int alloc_pkey(void)
shadow_pkey_reg);
if (ret) {
/* clear both the bits: */
- shadow_pkey_reg &= ~(0x3 << (ret * 2));
+ pkey_reset_shadow(ret);
dprintf4("%s()::%d, ret: %d pkey_reg: 0x%016lx "
"shadow: 0x%016lx\n",
__func__,
@@ -550,7 +565,7 @@ int alloc_pkey(void)
* move the new state in from init_val
* (remember, we cheated and init_val == pkey_reg format)
*/
- shadow_pkey_reg |= (init_val << (ret * 2));
+ pkey_set_shadow(ret, init_val);
}
dprintf4("%s()::%d, ret: %d pkey_reg: 0x%016lx shadow: 0x%016lx\n",
__func__, __LINE__, ret, __rdpkey_reg(),
@@ -1322,11 +1337,6 @@ void run_tests_once(void)
iteration_nr++;
}
-void pkey_setup_shadow(void)
-{
- shadow_pkey_reg = __rdpkey_reg();
-}
-
int main(void)
{
int nr_iterations = 22;
--
1.7.1
[PATCH v12 06/22] selftests/vm: fix the wrong assert in pkey_disable_set()
If the flag is 0, no bits will be set. Hence we cant expect
the resulting bitmap to have a higher value than what it
was earlier.
cc: Dave Hansen
cc: Florian Weimer
Signed-off-by: Ram Pai
---
tools/testing/selftests/vm/protection_keys.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/tools/testing/selftests/vm/protection_keys.c
b/tools/testing/selftests/vm/protection_keys.c
index 83216c5..0109388 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -443,7 +443,7 @@ void pkey_disable_set(int pkey, int flags)
dprintf1("%s(%d) pkey_reg: 0x%lx\n",
__func__, pkey, rdpkey_reg());
if (flags)
- pkey_assert(rdpkey_reg() > orig_pkey_reg);
+ pkey_assert(rdpkey_reg() >= orig_pkey_reg);
dprintf1("END<---%s(%d, 0x%x)\n", __func__,
pkey, flags);
}
--
1.7.1
[PATCH v12 07/22] selftests/vm: fixed bugs in pkey_disable_clear()
instead of clearing the bits, pkey_disable_clear() was setting
the bits. Fixed it.
Also fixed a wrong assertion in that function. When bits are
cleared, the resulting bit value will be less than the original.
cc: Dave Hansen
cc: Florian Weimer
Signed-off-by: Ram Pai
---
tools/testing/selftests/vm/protection_keys.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/vm/protection_keys.c
b/tools/testing/selftests/vm/protection_keys.c
index 0109388..ca54a95 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -461,7 +461,7 @@ void pkey_disable_clear(int pkey, int flags)
pkey, pkey, pkey_rights);
pkey_assert(pkey_rights >= 0);
- pkey_rights |= flags;
+ pkey_rights &= ~flags;
ret = pkey_set(pkey, pkey_rights, 0);
/* pkey_reg and flags have the same format */
@@ -475,7 +475,7 @@ void pkey_disable_clear(int pkey, int flags)
dprintf1("%s(%d) pkey_reg: 0x%016lx\n", __func__,
pkey, rdpkey_reg());
if (flags)
- assert(rdpkey_reg() > orig_pkey_reg);
+ assert(rdpkey_reg() < orig_pkey_reg);
}
void pkey_write_allow(int pkey)
--
1.7.1
[PATCH v12 08/22] selftests/vm: clear the bits in shadow reg when a pkey is freed.
When a key is freed, the key is no more effective.
Clear the bits corresponding to the pkey in the shadow
register. Otherwise it will carry some spurious bits
which can trigger false-positive asserts.
cc: Dave Hansen
cc: Florian Weimer
Signed-off-by: Ram Pai
---
tools/testing/selftests/vm/protection_keys.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/tools/testing/selftests/vm/protection_keys.c
b/tools/testing/selftests/vm/protection_keys.c
index ca54a95..aaf9f09 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -582,6 +582,9 @@ int alloc_pkey(void)
int sys_pkey_free(unsigned long pkey)
{
int ret = syscall(SYS_pkey_free, pkey);
+
+ if (!ret)
+ shadow_pkey_reg &= reset_bits(pkey, PKEY_DISABLE_ACCESS);
dprintf1("%s(pkey=%ld) syscall ret: %d\n", __func__, pkey, ret);
return ret;
}
--
1.7.1
[PATCH v12 09/22] selftests/vm: fix alloc_random_pkey() to make it really random
alloc_random_pkey() was allocating the same pkey every time.
Not all pkeys were geting tested. fixed it.
cc: Dave Hansen
cc: Florian Weimer
Signed-off-by: Ram Pai
---
tools/testing/selftests/vm/protection_keys.c | 10 +++---
1 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/vm/protection_keys.c
b/tools/testing/selftests/vm/protection_keys.c
index aaf9f09..2e4b636 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -24,6 +24,7 @@
#define _GNU_SOURCE
#include
#include
+#include
#include
#include
#include
@@ -602,13 +603,15 @@ int alloc_random_pkey(void)
int alloced_pkeys[NR_PKEYS];
int nr_alloced = 0;
int random_index;
+
memset(alloced_pkeys, 0, sizeof(alloced_pkeys));
+ srand((unsigned int)time(NULL));
/* allocate every possible key and make a note of which ones we got */
max_nr_pkey_allocs = NR_PKEYS;
- max_nr_pkey_allocs = 1;
for (i = 0; i < max_nr_pkey_allocs; i++) {
int new_pkey = alloc_pkey();
+
if (new_pkey < 0)
break;
alloced_pkeys[nr_alloced++] = new_pkey;
@@ -624,13 +627,14 @@ int alloc_random_pkey(void)
/* go through the allocated ones that we did not want and free them */
for (i = 0; i < nr_alloced; i++) {
int free_ret;
+
if (!alloced_pkeys[i])
continue;
free_ret = sys_pkey_free(alloced_pkeys[i]);
pkey_assert(!free_ret);
}
- dprintf1("%s()::%d, ret: %d pkey_reg: 0x%x shadow: 0x%x\n", __func__,
- __LINE__, ret, __rdpkey_reg(), shadow_pkey_reg);
+ dprintf1("%s()::%d, ret: %d pkey_reg: 0x%x shadow: 0x%016lx\n",
+ __func__, __LINE__, ret, __rdpkey_reg(), shadow_pkey_reg);
return ret;
}
--
1.7.1
[PATCH v12 10/22] selftests/vm: introduce two arch independent abstraction
open_hugepage_file() <- opens the huge page file
get_start_key() <-- provides the first non-reserved key.
cc: Dave Hansen
cc: Florian Weimer
Signed-off-by: Ram Pai
---
tools/testing/selftests/vm/pkey-helpers.h| 11 +++
tools/testing/selftests/vm/protection_keys.c |6 +++---
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/vm/pkey-helpers.h
b/tools/testing/selftests/vm/pkey-helpers.h
index 7c979ad..c8f5739 100644
--- a/tools/testing/selftests/vm/pkey-helpers.h
+++ b/tools/testing/selftests/vm/pkey-helpers.h
@@ -304,3 +304,14 @@ static inline void __page_o_noops(void)
} \
} while (0)
#define raw_assert(cond) assert(cond)
+
+static inline int open_hugepage_file(int flag)
+{
+ return open("/sys/kernel/mm/hugepages/hugepages-2048kB/nr_hugepages",
+O_RDONLY);
+}
+
+static inline int get_start_key(void)
+{
+ return 1;
+}
diff --git a/tools/testing/selftests/vm/protection_keys.c
b/tools/testing/selftests/vm/protection_keys.c
index 2e4b636..254b66d 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -809,7 +809,7 @@ void setup_hugetlbfs(void)
* Now go make sure that we got the pages and that they
* are 2M pages. Someone might have made 1G the default.
*/
- fd = open("/sys/kernel/mm/hugepages/hugepages-2048kB/nr_hugepages",
O_RDONLY);
+ fd = open_hugepage_file(O_RDONLY);
if (fd < 0) {
perror("opening sysfs 2M hugetlb config");
return;
@@ -1087,10 +1087,10 @@ void test_kernel_gup_write_to_write_disabled_region(int
*ptr, u16 pkey)
void test_pkey_syscalls_on_non_allocated_pkey(int *ptr, u16 pkey)
{
int err;
- int i;
+ int i = get_start_key();
/* Note: 0 is the default pkey, so don't mess with it */
- for (i = 1; i < NR_PKEYS; i++) {
+ for (; i < NR_PKEYS; i++) {
if (pkey == i)
continue;
--
1.7.1
[PATCH v12 11/22] selftests/vm: pkey register should match shadow pkey
expected_pkey_fault() is comparing the contents of pkey register with 0. This may not be true all the time. There could be bits set by default by the architecture which can never be changed. Hence compare the value against shadow pkey register, which is supposed to track the bits accurately all throughout cc: Dave Hansen cc: Florian Weimer Signed-off-by: Ram Pai --- tools/testing/selftests/vm/protection_keys.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/vm/protection_keys.c b/tools/testing/selftests/vm/protection_keys.c index 254b66d..6054093 100644 --- a/tools/testing/selftests/vm/protection_keys.c +++ b/tools/testing/selftests/vm/protection_keys.c @@ -926,10 +926,10 @@ void expected_pkey_fault(int pkey) pkey_assert(last_pkey_faults + 1 == pkey_faults); pkey_assert(last_si_pkey == pkey); /* -* The signal handler shold have cleared out PKEY register to let the +* The signal handler shold have cleared out pkey-register to let the * test program continue. We now have to restore it. */ - if (__rdpkey_reg() != 0) + if (__rdpkey_reg() != shadow_pkey_reg) pkey_assert(0); __wrpkey_reg(shadow_pkey_reg); -- 1.7.1
[PATCH v12 12/22] selftests/vm: generic cleanup
cleanup the code to satisfy coding styles.
cc: Dave Hansen
cc: Florian Weimer
Signed-off-by: Ram Pai
---
tools/testing/selftests/vm/protection_keys.c | 81 ++
1 files changed, 43 insertions(+), 38 deletions(-)
diff --git a/tools/testing/selftests/vm/protection_keys.c
b/tools/testing/selftests/vm/protection_keys.c
index 6054093..6fdd8f5 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -4,7 +4,7 @@
*
* There are examples in here of:
* * how to set protection keys on memory
- * * how to set/clear bits in pkey registers (the rights register)
+ * * how to set/clear bits in Protection Key registers (the rights register)
* * how to handle SEGV_PKUERR signals and extract pkey-relevant
*information from the siginfo
*
@@ -13,13 +13,18 @@
* prefault pages in at malloc, or not
* protect MPX bounds tables with protection keys?
* make sure VMA splitting/merging is working correctly
- * OOMs can destroy mm->mmap (see exit_mmap()), so make sure it is immune
to pkeys
- * look for pkey "leaks" where it is still set on a VMA but "freed" back
to the kernel
- * do a plain mprotect() to a mprotect_pkey() area and make sure the pkey
sticks
+ * OOMs can destroy mm->mmap (see exit_mmap()),
+ * so make sure it is immune to pkeys
+ * look for pkey "leaks" where it is still set on a VMA
+ * but "freed" back to the kernel
+ * do a plain mprotect() to a mprotect_pkey() area and make
+ * sure the pkey sticks
*
* Compile like this:
- * gcc -o protection_keys-O2 -g -std=gnu99 -pthread -Wall
protection_keys.c -lrt -ldl -lm
- * gcc -m32 -o protection_keys_32 -O2 -g -std=gnu99 -pthread -Wall
protection_keys.c -lrt -ldl -lm
+ * gcc -o protection_keys-O2 -g -std=gnu99
+ * -pthread -Wall protection_keys.c -lrt -ldl -lm
+ * gcc -m32 -o protection_keys_32 -O2 -g -std=gnu99
+ * -pthread -Wall protection_keys.c -lrt -ldl -lm
*/
#define _GNU_SOURCE
#include
@@ -251,26 +256,11 @@ void signal_handler(int signum, siginfo_t *si, void
*vucontext)
dprintf1("signal pkey_reg from pkey_reg: %016lx\n", __rdpkey_reg());
dprintf1("pkey from siginfo: %jx\n", siginfo_pkey);
*(u64 *)pkey_reg_ptr = 0x;
- dprintf1("WARNING: set PRKU=0 to allow faulting instruction to
continue\n");
+ dprintf1("WARNING: set PKEY_REG=0 to allow faulting instruction "
+ "to continue\n");
pkey_faults++;
dprintf1("==\n");
return;
- if (trapno == 14) {
- fprintf(stderr,
- "ERROR: In signal handler, page fault, trapno = %d, ip
= %016lx\n",
- trapno, ip);
- fprintf(stderr, "si_addr %p\n", si->si_addr);
- fprintf(stderr, "REG_ERR: %lx\n",
- (unsigned
long)uctxt->uc_mcontext.gregs[REG_ERR]);
- exit(1);
- } else {
- fprintf(stderr, "unexpected trap %d! at 0x%lx\n", trapno, ip);
- fprintf(stderr, "si_addr %p\n", si->si_addr);
- fprintf(stderr, "REG_ERR: %lx\n",
- (unsigned
long)uctxt->uc_mcontext.gregs[REG_ERR]);
- exit(2);
- }
- dprint_in_signal = 0;
}
int wait_all_children(void)
@@ -415,7 +405,7 @@ void pkey_disable_set(int pkey, int flags)
{
unsigned long syscall_flags = 0;
int ret;
- int pkey_rights;
+ u32 pkey_rights;
pkey_reg_t orig_pkey_reg = rdpkey_reg();
dprintf1("START->%s(%d, 0x%x)\n", __func__,
@@ -453,7 +443,7 @@ void pkey_disable_clear(int pkey, int flags)
{
unsigned long syscall_flags = 0;
int ret;
- int pkey_rights = pkey_get(pkey, syscall_flags);
+ u32 pkey_rights = pkey_get(pkey, syscall_flags);
pkey_reg_t orig_pkey_reg = rdpkey_reg();
pkey_assert(flags & (PKEY_DISABLE_ACCESS | PKEY_DISABLE_WRITE));
@@ -516,9 +506,10 @@ int sys_mprotect_pkey(void *ptr, size_t size, unsigned
long orig_prot,
return sret;
}
-int sys_pkey_alloc(unsigned long flags, unsigned long init_val)
+int sys_pkey_alloc(unsigned long flags, u64 init_val)
{
int ret = syscall(SYS_pkey_alloc, flags, init_val);
+
dprintf1("%s(flags=%lx, init_val=%lx) syscall ret: %d errno: %d\n",
__func__, flags, init_val, ret, errno);
return ret;
@@ -542,7 +533,7 @@ void pkey_set_shadow(u32 key, u64 init_val)
int alloc_pkey(void)
{
int ret;
- unsigned long init_val = 0x0;
+ u64 init_val = 0x0;
dprintf1("%s()::%d, pkey_reg: 0x%016lx shadow: %016lx\n", __func__,
__LINE__, __rdpkey_reg(), shadow_pkey_reg);
@@ -692,7 +683,9 @@ void record
[PATCH v12 13/22] selftests/vm: powerpc implementation for generic abstraction
Introduce powerpc implementation for the various
abstractions.
cc: Dave Hansen
cc: Florian Weimer
Signed-off-by: Ram Pai
---
tools/testing/selftests/vm/pkey-helpers.h| 128 ++
tools/testing/selftests/vm/protection_keys.c | 42 +---
2 files changed, 136 insertions(+), 34 deletions(-)
diff --git a/tools/testing/selftests/vm/pkey-helpers.h
b/tools/testing/selftests/vm/pkey-helpers.h
index c8f5739..c47aead 100644
--- a/tools/testing/selftests/vm/pkey-helpers.h
+++ b/tools/testing/selftests/vm/pkey-helpers.h
@@ -18,27 +18,51 @@
#define u16 uint16_t
#define u32 uint32_t
#define u64 uint64_t
-#define pkey_reg_t u32
-#ifdef __i386__
+#if defined(__i386__) || defined(__x86_64__) /* arch */
+
+#ifdef __i386__ /* arch */
#define SYS_mprotect_key 380
-#define SYS_pkey_alloc 381
-#define SYS_pkey_free 382
+#define SYS_pkey_alloc 381
+#define SYS_pkey_free382
#define REG_IP_IDX REG_EIP
-#define si_pkey_offset 0x14
-#else
+#elif __x86_64__
#define SYS_mprotect_key 329
-#define SYS_pkey_alloc 330
-#define SYS_pkey_free 331
+#define SYS_pkey_alloc 330
+#define SYS_pkey_free331
#define REG_IP_IDX REG_RIP
-#define si_pkey_offset 0x20
-#endif
+#endif /* __x86_64__ */
+
+#define NR_PKEYS 16
+#define NR_RESERVED_PKEYS 1
+#define PKEY_BITS_PER_PKEY 2
+#define PKEY_DISABLE_ACCESS0x1
+#define PKEY_DISABLE_WRITE 0x2
+#define HPAGE_SIZE (1UL<<21)
+#define pkey_reg_t u32
-#define NR_PKEYS 16
-#define PKEY_BITS_PER_PKEY 2
-#define PKEY_DISABLE_ACCESS0x1
-#define PKEY_DISABLE_WRITE 0x2
-#define HPAGE_SIZE (1UL<<21)
+#elif __powerpc64__ /* arch */
+
+#define SYS_mprotect_key 386
+#define SYS_pkey_alloc 384
+#define SYS_pkey_free 385
+#define REG_IP_IDX PT_NIP
+#define REG_TRAPNO PT_TRAP
+#define gregs gp_regs
+#define fpregs fp_regs
+
+#define NR_PKEYS 32
+#define NR_RESERVED_PKEYS_4K 26
+#define NR_RESERVED_PKEYS_64K 3
+#define PKEY_BITS_PER_PKEY 2
+#define PKEY_DISABLE_ACCESS0x3 /* disable read and write */
+#define PKEY_DISABLE_WRITE 0x2
+#define HPAGE_SIZE (1UL<<24)
+#define pkey_reg_t u64
+
+#else /* arch */
+ NOT SUPPORTED
+#endif /* arch */
#ifndef DEBUG_LEVEL
#define DEBUG_LEVEL 0
@@ -47,7 +71,11 @@
static inline u32 pkey_to_shift(int pkey)
{
+#if defined(__i386__) || defined(__x86_64__) /* arch */
return pkey * PKEY_BITS_PER_PKEY;
+#elif __powerpc64__ /* arch */
+ return (NR_PKEYS - pkey - 1) * PKEY_BITS_PER_PKEY;
+#endif /* arch */
}
static inline pkey_reg_t reset_bits(int pkey, pkey_reg_t bits)
@@ -111,6 +139,7 @@ static inline void sigsafe_printf(const char *format, ...)
extern pkey_reg_t shadow_pkey_reg;
static inline pkey_reg_t __rdpkey_reg(void)
{
+#if defined(__i386__) || defined(__x86_64__) /* arch */
unsigned int eax, edx;
unsigned int ecx = 0;
pkey_reg_t pkey_reg;
@@ -118,7 +147,13 @@ static inline pkey_reg_t __rdpkey_reg(void)
asm volatile(".byte 0x0f,0x01,0xee\n\t"
: "=a" (eax), "=d" (edx)
: "c" (ecx));
- pkey_reg = eax;
+#elif __powerpc64__ /* arch */
+ pkey_reg_t eax;
+ pkey_reg_t pkey_reg;
+
+ asm volatile("mfspr %0, 0xd" : "=r" ((pkey_reg_t)(eax)));
+#endif /* arch */
+ pkey_reg = (pkey_reg_t)eax;
return pkey_reg;
}
@@ -138,6 +173,7 @@ static inline pkey_reg_t _rdpkey_reg(int line)
static inline void __wrpkey_reg(pkey_reg_t pkey_reg)
{
pkey_reg_t eax = pkey_reg;
+#if defined(__i386__) || defined(__x86_64__) /* arch */
pkey_reg_t ecx = 0;
pkey_reg_t edx = 0;
@@ -146,6 +182,14 @@ static inline void __wrpkey_reg(pkey_reg_t pkey_reg)
asm volatile(".byte 0x0f,0x01,0xef\n\t"
: : "a" (eax), "c" (ecx), "d" (edx));
assert(pkey_reg == __rdpkey_reg());
+
+#elif __powerpc64__ /* arch */
+ dprintf4("%s() changing %llx to %llx\n",
+__func__, __rdpkey_reg(), pkey_reg);
+ asm volatile("mtspr 0xd, %0" : : "r" ((unsigned long)(eax)) : "memory");
+#endif /* arch */
+ dprintf4("%s() pkey register after changing %016lx to %016lx\n",
+__func__, __rdpkey_reg(), pkey_reg);
}
static inline void wrpkey_reg(pkey_reg_t pkey_reg)
@@ -192,6 +236,8 @@ static inline void __pkey_write_allow(int pkey, int
do_allow_write)
dprintf4("pkey_reg now: %08x\n", rdpkey_reg());
}
+#if defined(__i386__) || defined(__x86_64__) /* arch */
+
#define PAGE_SIZE 4096
#define MB (1<<20)
@@ -274,8 +320,18 @@ static inline void __page_o_noops(void)
/* 8-bytes of instruction * 512 bytes = 1 page */
asm(".rept 512 ; nopl 0x7eee(%eax) ; .endr");
}
+#elif __powerpc64__ /* arch */
-#endif /* _PKEYS_HELPER_H */
+#define PAGE_SIZE (0x1UL << 16)
+static inline int cpu_has_pku(void)
+{
+ return 1;
+}
+
+/* 8-bytes of instruction * 16384bytes = 1 page */
+#define __page_o_noops()
[PATCH v12 14/22] selftests/vm: clear the bits in shadow reg when a pkey is freed.
When a key is freed, the key is no more effective.
Clear the bits corresponding to the pkey in the shadow
register. Otherwise it will carry some spurious bits
which can trigger false-positive asserts.
cc: Dave Hansen
cc: Florian Weimer
Signed-off-by: Ram Pai
---
tools/testing/selftests/vm/protection_keys.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/tools/testing/selftests/vm/protection_keys.c
b/tools/testing/selftests/vm/protection_keys.c
index c4c73e6..e82bd88 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -586,7 +586,8 @@ int sys_pkey_free(unsigned long pkey)
int ret = syscall(SYS_pkey_free, pkey);
if (!ret)
- shadow_pkey_reg &= reset_bits(pkey, PKEY_DISABLE_ACCESS);
+ shadow_pkey_reg &= reset_bits(pkey,
+ PKEY_DISABLE_ACCESS | PKEY_DISABLE_WRITE);
dprintf1("%s(pkey=%ld) syscall ret: %d\n", __func__, pkey, ret);
return ret;
}
--
1.7.1
[PATCH v12 15/22] selftests/vm: powerpc implementation to check support for pkey
pkey subsystem is supported if the hardware and kernel has support.
We determine that by checking if allocation of a key succeeds or not.
cc: Dave Hansen
cc: Florian Weimer
Signed-off-by: Ram Pai
---
tools/testing/selftests/vm/pkey-helpers.h| 22 --
tools/testing/selftests/vm/protection_keys.c |9 +
2 files changed, 21 insertions(+), 10 deletions(-)
diff --git a/tools/testing/selftests/vm/pkey-helpers.h
b/tools/testing/selftests/vm/pkey-helpers.h
index c47aead..88ef58f 100644
--- a/tools/testing/selftests/vm/pkey-helpers.h
+++ b/tools/testing/selftests/vm/pkey-helpers.h
@@ -258,7 +258,7 @@ static inline void __cpuid(unsigned int *eax, unsigned int
*ebx,
#define X86_FEATURE_PKU(1<<3) /* Protection Keys for Userspace */
#define X86_FEATURE_OSPKE (1<<4) /* OS Protection Keys Enable */
-static inline int cpu_has_pku(void)
+static inline bool is_pkey_supported(void)
{
unsigned int eax;
unsigned int ebx;
@@ -271,13 +271,13 @@ static inline int cpu_has_pku(void)
if (!(ecx & X86_FEATURE_PKU)) {
dprintf2("cpu does not have PKU\n");
- return 0;
+ return false;
}
if (!(ecx & X86_FEATURE_OSPKE)) {
dprintf2("cpu does not have OSPKE\n");
- return 0;
+ return false;
}
- return 1;
+ return true;
}
#define XSTATE_PKEY_BIT(9)
@@ -323,9 +323,19 @@ static inline void __page_o_noops(void)
#elif __powerpc64__ /* arch */
#define PAGE_SIZE (0x1UL << 16)
-static inline int cpu_has_pku(void)
+static inline bool is_pkey_supported(void)
{
- return 1;
+ /*
+* No simple way to determine this.
+* lets try allocating a key and see if it succeeds.
+*/
+ int ret = sys_pkey_alloc(0, 0);
+
+ if (ret > 0) {
+ sys_pkey_free(ret);
+ return true;
+ }
+ return false;
}
/* 8-bytes of instruction * 16384bytes = 1 page */
diff --git a/tools/testing/selftests/vm/protection_keys.c
b/tools/testing/selftests/vm/protection_keys.c
index e82bd88..58da5a0 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -1299,8 +1299,8 @@ void test_mprotect_pkey_on_unsupported_cpu(int *ptr, u16
pkey)
int size = PAGE_SIZE;
int sret;
- if (cpu_has_pku()) {
- dprintf1("SKIP: %s: no CPU support\n", __func__);
+ if (is_pkey_supported()) {
+ dprintf1("SKIP: %s: no CPU/kernel support\n", __func__);
return;
}
@@ -1362,12 +1362,13 @@ void run_tests_once(void)
int main(void)
{
int nr_iterations = 22;
+ int pkey_supported = is_pkey_supported();
setup_handlers();
- printf("has pkey: %d\n", cpu_has_pku());
+ printf("has pkey: %s\n", pkey_supported ? "Yes" : "No");
- if (!cpu_has_pku()) {
+ if (!pkey_supported) {
int size = PAGE_SIZE;
int *ptr;
--
1.7.1
[PATCH v12 16/22] selftests/vm: fix an assertion in test_pkey_alloc_exhaust()
The maximum number of keys that can be allocated has to
take into consideration, that some keys are reserved by
the architecture for specific purpose. Hence cannot
be allocated.
Fix the assertion in test_pkey_alloc_exhaust()
cc: Dave Hansen
cc: Florian Weimer
Signed-off-by: Ram Pai
---
tools/testing/selftests/vm/pkey-helpers.h| 14 ++
tools/testing/selftests/vm/protection_keys.c |9 -
2 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/vm/pkey-helpers.h
b/tools/testing/selftests/vm/pkey-helpers.h
index 88ef58f..67f9b0f 100644
--- a/tools/testing/selftests/vm/pkey-helpers.h
+++ b/tools/testing/selftests/vm/pkey-helpers.h
@@ -416,4 +416,18 @@ static inline int get_start_key(void)
#endif
}
+static inline int arch_reserved_keys(void)
+{
+#if defined(__i386__) || defined(__x86_64__) /* arch */
+ return NR_RESERVED_PKEYS;
+#elif __powerpc64__ /* arch */
+ if (sysconf(_SC_PAGESIZE) == 4096)
+ return NR_RESERVED_PKEYS_4K;
+ else
+ return NR_RESERVED_PKEYS_64K;
+#else /* arch */
+ NOT SUPPORTED
+#endif /* arch */
+}
+
#endif /* _PKEYS_HELPER_H */
diff --git a/tools/testing/selftests/vm/protection_keys.c
b/tools/testing/selftests/vm/protection_keys.c
index 58da5a0..aae6771 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -1167,12 +1167,11 @@ void test_pkey_alloc_exhaust(int *ptr, u16 pkey)
pkey_assert(i < NR_PKEYS*2);
/*
-* There are 16 pkeys supported in hardware. One is taken
-* up for the default (0) and another can be taken up by
-* an execute-only mapping. Ensure that we can allocate
-* at least 14 (16-2).
+* There are NR_PKEYS pkeys supported in hardware. arch_reserved_keys()
+* are reserved. One can be taken up by an execute-only mapping.
+* Ensure that we can allocate at least the remaining.
*/
- pkey_assert(i >= NR_PKEYS-2);
+ pkey_assert(i >= (NR_PKEYS-arch_reserved_keys()-1));
for (i = 0; i < nr_allocated_pkeys; i++) {
err = sys_pkey_free(allocated_pkeys[i]);
--
1.7.1
[PATCH v12 17/22] selftests/vm: associate key on a mapped page and detect access violation
detect access-violation on a page to which access-disabled
key is associated much after the page is mapped.
cc: Dave Hansen
cc: Florian Weimer
Signed-off-by: Ram Pai
---
tools/testing/selftests/vm/protection_keys.c | 19 +++
1 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/tools/testing/selftests/vm/protection_keys.c
b/tools/testing/selftests/vm/protection_keys.c
index aae6771..dba9162 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -1016,6 +1016,24 @@ void test_read_of_access_disabled_region(int *ptr, u16
pkey)
dprintf1("*ptr: %d\n", ptr_contents);
expected_pkey_fault(pkey);
}
+
+void test_read_of_access_disabled_region_with_page_already_mapped(int *ptr,
+ u16 pkey)
+{
+ int ptr_contents;
+
+ dprintf1("disabling access to PKEY[%02d], doing read @ %p\n",
+ pkey, ptr);
+ ptr_contents = read_ptr(ptr);
+ dprintf1("reading ptr before disabling the read : %d\n",
+ ptr_contents);
+ rdpkey_reg();
+ pkey_access_deny(pkey);
+ ptr_contents = read_ptr(ptr);
+ dprintf1("*ptr: %d\n", ptr_contents);
+ expected_pkey_fault(pkey);
+}
+
void test_write_of_write_disabled_region(int *ptr, u16 pkey)
{
dprintf1("disabling write access to PKEY[%02d], doing write\n", pkey);
@@ -1310,6 +1328,7 @@ void test_mprotect_pkey_on_unsupported_cpu(int *ptr, u16
pkey)
void (*pkey_tests[])(int *ptr, u16 pkey) = {
test_read_of_write_disabled_region,
test_read_of_access_disabled_region,
+ test_read_of_access_disabled_region_with_page_already_mapped,
test_write_of_write_disabled_region,
test_write_of_access_disabled_region,
test_kernel_write_of_access_disabled_region,
--
1.7.1
[PATCH v12 18/22] selftests/vm: associate key on a mapped page and detect write violation
detect write-violation on a page to which write-disabled
key is associated much after the page is mapped.
cc: Dave Hansen
cc: Florian Weimer
Signed-off-by: Ram Pai
---
tools/testing/selftests/vm/protection_keys.c | 12
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/tools/testing/selftests/vm/protection_keys.c
b/tools/testing/selftests/vm/protection_keys.c
index dba9162..b685e26 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -1034,6 +1034,17 @@ void
test_read_of_access_disabled_region_with_page_already_mapped(int *ptr,
expected_pkey_fault(pkey);
}
+void test_write_of_write_disabled_region_with_page_already_mapped(int *ptr,
+ u16 pkey)
+{
+ *ptr = __LINE__;
+ dprintf1("disabling write access; after accessing the page, "
+ "to PKEY[%02d], doing write\n", pkey);
+ pkey_write_deny(pkey);
+ *ptr = __LINE__;
+ expected_pkey_fault(pkey);
+}
+
void test_write_of_write_disabled_region(int *ptr, u16 pkey)
{
dprintf1("disabling write access to PKEY[%02d], doing write\n", pkey);
@@ -1330,6 +1341,7 @@ void test_mprotect_pkey_on_unsupported_cpu(int *ptr, u16
pkey)
test_read_of_access_disabled_region,
test_read_of_access_disabled_region_with_page_already_mapped,
test_write_of_write_disabled_region,
+ test_write_of_write_disabled_region_with_page_already_mapped,
test_write_of_access_disabled_region,
test_kernel_write_of_access_disabled_region,
test_kernel_write_of_write_disabled_region,
--
1.7.1
[PATCH v12 19/22] selftests/vm: detect write violation on a mapped access-denied-key page
detect write-violation on a page to which access-disabled
key is associated much after the page is mapped.
cc: Dave Hansen
cc: Florian Weimer
Signed-off-by: Ram Pai
---
tools/testing/selftests/vm/protection_keys.c | 13 +
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/tools/testing/selftests/vm/protection_keys.c
b/tools/testing/selftests/vm/protection_keys.c
index b685e26..437ee74 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -1059,6 +1059,18 @@ void test_write_of_access_disabled_region(int *ptr, u16
pkey)
*ptr = __LINE__;
expected_pkey_fault(pkey);
}
+
+void test_write_of_access_disabled_region_with_page_already_mapped(int *ptr,
+ u16 pkey)
+{
+ *ptr = __LINE__;
+ dprintf1("disabling access; after accessing the page, "
+ " to PKEY[%02d], doing write\n", pkey);
+ pkey_access_deny(pkey);
+ *ptr = __LINE__;
+ expected_pkey_fault(pkey);
+}
+
void test_kernel_write_of_access_disabled_region(int *ptr, u16 pkey)
{
int ret;
@@ -1343,6 +1355,7 @@ void test_mprotect_pkey_on_unsupported_cpu(int *ptr, u16
pkey)
test_write_of_write_disabled_region,
test_write_of_write_disabled_region_with_page_already_mapped,
test_write_of_access_disabled_region,
+ test_write_of_access_disabled_region_with_page_already_mapped,
test_kernel_write_of_access_disabled_region,
test_kernel_write_of_write_disabled_region,
test_kernel_gup_of_access_disabled_region,
--
1.7.1
[PATCH v12 20/22] selftests/vm: testcases must restore pkey-permissions
Generally the signal handler restores the state of the pkey register
before returning. However there are times when the read/write operation
can legitamely fail without invoking the signal handler. Eg: A
sys_read() operaton to a write-protected page should be disallowed. In
such a case the state of the pkey register is not restored to its
original state. The test case is responsible for restoring the key
register state to its original value.
cc: Dave Hansen
cc: Florian Weimer
Signed-off-by: Ram Pai
---
tools/testing/selftests/vm/protection_keys.c |5 +
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/tools/testing/selftests/vm/protection_keys.c
b/tools/testing/selftests/vm/protection_keys.c
index 437ee74..42c068a 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -1003,6 +1003,7 @@ void test_read_of_write_disabled_region(int *ptr, u16
pkey)
ptr_contents = read_ptr(ptr);
dprintf1("*ptr: %d\n", ptr_contents);
dprintf1("\n");
+ pkey_write_allow(pkey);
}
void test_read_of_access_disabled_region(int *ptr, u16 pkey)
{
@@ -1082,6 +1083,7 @@ void test_kernel_write_of_access_disabled_region(int
*ptr, u16 pkey)
ret = read(test_fd, ptr, 1);
dprintf1("read ret: %d\n", ret);
pkey_assert(ret);
+ pkey_access_allow(pkey);
}
void test_kernel_write_of_write_disabled_region(int *ptr, u16 pkey)
{
@@ -1094,6 +1096,7 @@ void test_kernel_write_of_write_disabled_region(int *ptr,
u16 pkey)
if (ret < 0 && (DEBUG_LEVEL > 0))
perror("verbose read result (OK for this to be bad)");
pkey_assert(ret);
+ pkey_write_allow(pkey);
}
void test_kernel_gup_of_access_disabled_region(int *ptr, u16 pkey)
@@ -1113,6 +1116,7 @@ void test_kernel_gup_of_access_disabled_region(int *ptr,
u16 pkey)
vmsplice_ret = vmsplice(pipe_fds[1], &iov, 1, SPLICE_F_GIFT);
dprintf1("vmsplice() ret: %d\n", vmsplice_ret);
pkey_assert(vmsplice_ret == -1);
+ pkey_access_allow(pkey);
close(pipe_fds[0]);
close(pipe_fds[1]);
@@ -1133,6 +1137,7 @@ void test_kernel_gup_write_to_write_disabled_region(int
*ptr, u16 pkey)
if (DEBUG_LEVEL > 0)
perror("futex");
dprintf1("futex() ret: %d\n", futex_ret);
+ pkey_write_allow(pkey);
}
/* Assumes that all pkeys other than 'pkey' are unallocated */
--
1.7.1
[PATCH v12 21/22] selftests/vm: sub-page allocator
introduce a new allocator that allocates 4k hardware-pages to back
64k linux-page. This allocator is only applicable on powerpc.
cc: Dave Hansen
cc: Florian Weimer
Signed-off-by: Ram Pai
---
tools/testing/selftests/vm/protection_keys.c | 30 ++
1 files changed, 30 insertions(+), 0 deletions(-)
diff --git a/tools/testing/selftests/vm/protection_keys.c
b/tools/testing/selftests/vm/protection_keys.c
index 42c068a..1b06e59 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -766,6 +766,35 @@ void free_pkey_malloc(void *ptr)
return ptr;
}
+void *malloc_pkey_with_mprotect_subpage(long size, int prot, u16 pkey)
+{
+#ifdef __powerpc64__
+ void *ptr;
+ int ret;
+
+ dprintf1("doing %s(size=%ld, prot=0x%x, pkey=%d)\n", __func__,
+ size, prot, pkey);
+ pkey_assert(pkey < NR_PKEYS);
+ ptr = mmap(NULL, size, prot, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
+ pkey_assert(ptr != (void *)-1);
+
+ ret = syscall(__NR_subpage_prot, ptr, size, NULL);
+ if (ret) {
+ perror("subpage_perm");
+ return PTR_ERR_ENOTSUP;
+ }
+
+ ret = mprotect_pkey((void *)ptr, PAGE_SIZE, prot, pkey);
+ pkey_assert(!ret);
+ record_pkey_malloc(ptr, size);
+
+ dprintf1("%s() for pkey %d @ %p\n", __func__, pkey, ptr);
+ return ptr;
+#else /* __powerpc64__ */
+ return PTR_ERR_ENOTSUP;
+#endif /* __powerpc64__ */
+}
+
void *malloc_pkey_anon_huge(long size, int prot, u16 pkey)
{
int ret;
@@ -888,6 +917,7 @@ void setup_hugetlbfs(void)
void *(*pkey_malloc[])(long size, int prot, u16 pkey) = {
malloc_pkey_with_mprotect,
+ malloc_pkey_with_mprotect_subpage,
malloc_pkey_anon_huge,
malloc_pkey_hugetlb
/* can not do direct with the pkey_mprotect() API:
--
1.7.1
[PATCH v12 22/22] selftests/vm: Fix deadlock in protection_keys.c
From: Thiago Jung Bauermann
The sig_chld() handler calls dprintf2() taking care of setting
dprint_in_signal so that sigsafe_printf() won't call printf().
Unfortunately, this precaution is is negated by dprintf_level(), which
has a call to fflush().
This function acquires a lock, which means that if the signal interrupts an
ongoing fflush() the process will deadlock. At least on powerpc this is
easy to trigger, resulting in the following backtrace when attaching to the
frozen process:
(gdb) bt
#0 0x7fff9f96c7d8 in __lll_lock_wait_private () from
/lib64/power8/libc.so.6
#1 0x7fff9f8cba4c in _IO_flush_all_lockp () from /lib64/power8/libc.so.6
#2 0x7fff9f8cbd1c in __GI__IO_flush_all () from /lib64/power8/libc.so.6
#3 0x7fff9f8b7424 in fflush () from /lib64/power8/libc.so.6
#4 0x100504f8 in sig_chld (x=17) at protection_keys.c:283
#5
#6 0x7fff9f8cb8ac in _IO_flush_all_lockp () from /lib64/power8/libc.so.6
#7 0x7fff9f8cbd1c in __GI__IO_flush_all () from /lib64/power8/libc.so.6
#8 0x7fff9f8b7424 in fflush () from /lib64/power8/libc.so.6
#9 0x10050b50 in pkey_get (pkey=7, flags=0) at protection_keys.c:379
#10 0x10050dc0 in pkey_disable_set (pkey=7, flags=2) at
protection_keys.c:423
#11 0x10051414 in pkey_write_deny (pkey=7) at protection_keys.c:486
#12 0x100556bc in test_ptrace_of_child (ptr=0x7fff9f7f, pkey=7)
at protection_keys.c:1288
#13 0x10055f60 in run_tests_once () at protection_keys.c:1414
#14 0x100561a4 in main () at protection_keys.c:1459
The fix is to refrain from calling fflush() when inside a signal handler.
The output may not be as pretty but at least the testcase will be able to
move on.
cc: Dave Hansen
cc: Florian Weimer
Signed-off-by: Ram Pai
Signed-off-by: Thiago Jung Bauermann
tools/testing/selftests/vm/pkey-helpers.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
---
tools/testing/selftests/vm/pkey-helpers.h |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/tools/testing/selftests/vm/pkey-helpers.h
b/tools/testing/selftests/vm/pkey-helpers.h
index 67f9b0f..7240598 100644
--- a/tools/testing/selftests/vm/pkey-helpers.h
+++ b/tools/testing/selftests/vm/pkey-helpers.h
@@ -128,7 +128,8 @@ static inline void sigsafe_printf(const char *format, ...)
#define dprintf_level(level, args...) do { \
if (level <= DEBUG_LEVEL) \
sigsafe_printf(args); \
- fflush(NULL); \
+ if (!dprint_in_signal) \
+ fflush(NULL); \
} while (0)
#define dprintf0(args...) dprintf_level(0, args)
#define dprintf1(args...) dprintf_level(1, args)
--
1.7.1
[PATCH v2 0/2] Add get_metadata IOCTL to share OCXL information to userspace
From: Alastair D'Silva This series adds a new IOCTL for OpenCAPI, which exposes adapter metadata to userspace. --- v1->v2: - Document the new IOCTL Alastair D'Silva (2): ocxl: Add get_metadata IOCTL to share OCXL information to userspace ocxl: Document the OCXL_IOCTL_GET_METADATA IOCTL Documentation/accelerators/ocxl.rst | 5 + drivers/misc/ocxl/file.c| 27 +++ include/uapi/misc/ocxl.h| 22 ++ 3 files changed, 54 insertions(+) -- 2.14.3
[PATCH v2 2/2] ocxl: Document the OCXL_IOCTL_GET_METADATA IOCTL
From: Alastair D'Silva Signed-off-by: Alastair D'Silva --- Documentation/accelerators/ocxl.rst | 5 + 1 file changed, 5 insertions(+) diff --git a/Documentation/accelerators/ocxl.rst b/Documentation/accelerators/ocxl.rst index 4f7af841d935..ddcc58d01cfb 100644 --- a/Documentation/accelerators/ocxl.rst +++ b/Documentation/accelerators/ocxl.rst @@ -152,6 +152,11 @@ OCXL_IOCTL_IRQ_SET_FD: Associate an event fd to an AFU interrupt so that the user process can be notified when the AFU sends an interrupt. +OCXL_IOCTL_GET_METADATA: + + Obtains configuration information from the card, such at the size of + MMIO areas, the AFU version, and the PASID for the current context. + mmap -- 2.14.3
[PATCH v2 1/2] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
From: Alastair D'Silva
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
---
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
Re: [PATCH] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
"Alastair D'Silva" writes:
> On Wed, 2018-02-21 at 17:43 +1100, Balbir Singh wrote:
>> On Wed, Feb 21, 2018 at 3:57 PM, Alastair D'Silva > om> wrote:
>> > From: Alastair D'Silva
>> >
...
>> > 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?
>>
>
> It pretty much is a function, it returns to userspace metadata about
> the descriptor being operated on.
It's not a function, it's a struct?
Outside of "management English" "get" is a verb, so using it in the name
of the struct is confusing, it should be a noun phrase.
cheers
Re: [PATCH 00/23] kconfig: move compiler capability tests to Kconfig
Masahiro Yamada writes: > > > (Case 3) > Compiler flag -foo is sensitive to endian-ness. > > > config CC_NEEDS_BIG_ENDIAN > def_bool $(cc-option -mbig-endian) && CPU_BIG_ENDIAN > > config CC_NEEDS_LITTLE_ENDIAN > def_bool $(cc-option -mlittle-endian) && CPU_LITTLE_ENDIAN > > config CC_HAS_FOO > bool > default $(cc-option -mbig-endian -foo) if CC_NEEDS_BIG_ENDIAN > default $(cc-option -mlittle-endian -foo) if CC_NEEDS_LITTLE_ENDIAN > default $(cc-option -foo) We may do something like this on powerpc, where we have 32/64-bit and big/little endian (on 64-bit) and then some ABI options that we set/unset depending on endian. The above looks like it could work though. cheers
Re: [PATCH] bpf, powerpc: fix jit for seccomp_data access
Daniel Borkmann writes: > On 02/21/2018 01:33 AM, Michael Ellerman wrote: >> Mark Lord 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 >> >> 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. OK thanks. >> 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). OK, I'll just add the Fixes tag for our reference and we can always back port it to stable later if we want to. cheers
Re: [PATCH] bpf, powerpc: fix jit for seccomp_data access
"Naveen N. Rao" writes: > Mark Lord wrote: >> On 18-02-21 07:52 AM, Mark Lord wrote: >>> On 18-02-21 03:35 AM, Naveen N. Rao wrote: >> .. Looks good to me, but I am not able to apply this patch. There seems to be whitespace damage. >>> >>> Here (attached) is a clean copy. >> >> Again, this time with the commit message included! > > Thanks. However... > I am able to apply this using 'patch', but not with 'git am' since the > headers are missing. FWIW, the usual workflow is to make the changes and > commit it into your repository using 'git commit' and then use 'git > format-patch' to generate a patch file that you can then post. > > I'll defer to Michael on whether he is ok to process this as it is. The main thing is that it's caught by patchwork[1], otherwise I tend to miss it. In this case the initial patch was caught by patchwork, so that's fine. I fixed up the white space and other issues before applying. In general I'm happy to do that for new/infrequent committers, not so much for people who send lots of patches and/or are paid to do so :) cheers 1: http://patchwork.ozlabs.org/patch/875890/
Re: [PATCH] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
On Thu, Feb 22, 2018 at 10:32 AM, Alastair D'Silva wrote:
>
> On Wed, 2018-02-21 at 17:43 +1100, Balbir Singh wrote:
>> On Wed, Feb 21, 2018 at 3:57 PM, Alastair D'Silva > om> wrote:
>> > From: Alastair D'Silva
>> >
>> > 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
>> > ---
>> > 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?
>>
>
> It's metadata for the descriptor.
I meant metadata is too generic, could we have other types of metadata in OCXL?
>
>> > +{
>> > + 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
>>
>
> Setting it to 0 is for the reader, not the compiler. I'm not clear on
> the benefit of starting the version at 1, could you clarify?
How do I distinguish between version number never set and 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 {
>>
>> This sounds more like a function name, do we need it to be
>> _get_metdata?
>>
>
> It pretty much is a function, it returns to userspace metadata about
> the descriptor being operated on.
>
It has a verb indicating action
>> > + __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?
>>
>
> Yes, I would rather call it per_pasid_mmio_size, but consistency with
> the rest of the driver (& exposed sysfs entries) is also important.
>
Balbir Singh.
Re: [PATCH] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
On Wed, Feb 21, 2018 at 10:25 PM, Frederic Barrat
wrote:
>
>
> Le 21/02/2018 à 07:43, Balbir Singh a écrit :
>>
>> On Wed, Feb 21, 2018 at 3:57 PM, Alastair D'Silva
>> wrote:
>>>
>>> From: Alastair D'Silva
>>>
>>> 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
>>> ---
>>> 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?
>
>
> My bad, I forgot to mention it before. There's a somewhat high-level
> description which needs updating in:
> Documentation/accelerators/ocxl.rst
Thanks, that's helpful
>
> It doesn't go down to the level of the structure members, but at least all
> ioctl commands should have a brief description.
>
> lpc_size could be added. It's currently useless to the library, but doesn't
> hurt. The one which was giving me troubles on a previous version of this
> patch was the lpc numa node ID, since that was experimental code and felt
> out of place considering what's been upstreamed in skiboot and linux so far.
>
Yeah, I think metadata will evolve for a while till it settle's down.
Since ocxl_ioctl_get_metadata is exposed via uapi, a newer program
calling an older kernel will never work, since the size of that struct
will always be larger than what the OS supports and our copy_to_user()
will fail. The other option is for the user program to try all
possible versions till one succeeds, that is bad as well. I think
there are a few ways around it, if we care about this combination.
Balbir S
Re: [PATCH] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
On 22/02/18 14:41, Balbir Singh wrote: Setting it to 0 is for the reader, not the compiler. I'm not clear on the benefit of starting the version at 1, could you clarify? How do I distinguish between version number never set and 0? The ioctl won't exist without a version number set. -- Andrew Donnellan OzLabs, ADL Canberra andrew.donnel...@au1.ibm.com IBM Australia Limited
Re: [PATCH] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
On Thu, 2018-02-22 at 14:41 +1100, Balbir Singh wrote:
> On Thu, Feb 22, 2018 at 10:32 AM, Alastair D'Silva com> wrote:
> >
> > On Wed, 2018-02-21 at 17:43 +1100, Balbir Singh wrote:
> > > On Wed, Feb 21, 2018 at 3:57 PM, Alastair D'Silva > > bm.c
> > > om> wrote:
> > > > From: Alastair D'Silva
> > > >
> > > > 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
> > > > ---
> > > > 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?
> > >
> >
> > It's metadata for the descriptor.
>
> I meant metadata is too generic, could we have other types of
> metadata in OCXL?
>
I don't believe so, we would instead expand the scope of this IOCTL
using version & space available from the reserved fields.
> >
> > > > +{
> > > > + 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
> > >
> >
> > Setting it to 0 is for the reader, not the compiler. I'm not clear
> > on
> > the benefit of starting the version at 1, could you clarify?
>
> How do I distinguish between version number never set and 0?
>
The version number is always set. If the IOCTL doesn't exist, the ioctl
call will error instead.
> >
> > > > +
> > > > + 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?
> > >
> >
> > It pretty much is a function, it returns to userspace metadata
> > about
> > the descriptor being operated on.
> >
>
> It has a verb indicating action
I misunderstood, I had named the struct to match the IOCTL, but that
isn't necessary. I'll update it in the next patch.
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Au
Re: [PATCH] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
On Thu, 2018-02-22 at 14:46 +1100, Balbir Singh wrote: > lpc_size could be added. It's currently useless to the library, but > > doesn't > > hurt. The one which was giving me troubles on a previous version of > > this > > patch was the lpc numa node ID, since that was experimental code > > and felt > > out of place considering what's been upstreamed in skiboot and > > linux so far. > > > > Yeah, I think metadata will evolve for a while till it settle's down. > Since ocxl_ioctl_get_metadata is exposed via uapi, a newer program > calling an older kernel will never work, since the size of that > struct > will always be larger than what the OS supports and our > copy_to_user() > will fail. The other option is for the user program to try all > possible versions till one succeeds, that is bad as well. I think > there are a few ways around it, if we care about this combination. > > Balbir Singh. > We have a number of reserved members at the end of the struct which can be re-purposed for future information (with a corresponding bump of the version number). -- Alastair D'Silva Open Source Developer Linux Technology Centre, IBM Australia mob: 0423 762 819
[PATCH v3 0/2] Add get_metadata IOCTL to share OCXL information to userspace
From: Alastair D'Silva This series adds a new IOCTL for OpenCAPI, which exposes adapter metadata to userspace. --- v1->v2: - Document the new IOCTL v2->v3: - Rename metadata struct - Document a couple of struct members that are not immediately obvious Alastair D'Silva (2): ocxl: Add get_metadata IOCTL to share OCXL information to userspace ocxl: Document the OCXL_IOCTL_GET_METADATA IOCTL Documentation/accelerators/ocxl.rst | 5 + drivers/misc/ocxl/file.c| 27 +++ include/uapi/misc/ocxl.h| 17 + 3 files changed, 49 insertions(+) -- 2.14.3
[PATCH v3 2/2] ocxl: Document the OCXL_IOCTL_GET_METADATA IOCTL
From: Alastair D'Silva Signed-off-by: Alastair D'Silva --- Documentation/accelerators/ocxl.rst | 5 + 1 file changed, 5 insertions(+) diff --git a/Documentation/accelerators/ocxl.rst b/Documentation/accelerators/ocxl.rst index 4f7af841d935..ddcc58d01cfb 100644 --- a/Documentation/accelerators/ocxl.rst +++ b/Documentation/accelerators/ocxl.rst @@ -152,6 +152,11 @@ OCXL_IOCTL_IRQ_SET_FD: Associate an event fd to an AFU interrupt so that the user process can be notified when the AFU sends an interrupt. +OCXL_IOCTL_GET_METADATA: + + Obtains configuration information from the card, such at the size of + MMIO areas, the AFU version, and the PASID for the current context. + mmap -- 2.14.3
[PATCH v3 1/2] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
From: Alastair D'Silva
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
---
drivers/misc/ocxl/file.c | 27 +++
include/uapi/misc/ocxl.h | 17 +
2 files changed, 44 insertions(+)
diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
index d9aa407db06a..90df1be5ef3f 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_metadata __user *uarg)
+{
+ struct ocxl_ioctl_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_metadata __user *) args);
+ break;
+
default:
rc = -EINVAL;
}
diff --git a/include/uapi/misc/ocxl.h b/include/uapi/misc/ocxl.h
index 4b0b0b756f3e..0af83d80fb3e 100644
--- a/include/uapi/misc/ocxl.h
+++ b/include/uapi/misc/ocxl.h
@@ -32,6 +32,22 @@ struct ocxl_ioctl_attach {
__u64 reserved3;
};
+struct ocxl_ioctl_metadata {
+ __u16 version; // struct version, always backwards compatible
+
+ // Version 0 fields
+ __u8 afu_version_major;
+ __u8 afu_version_minor;
+ __u32 pasid;// PASID assigned to the current context
+
+ __u64 pp_mmio_size; // Per PASID 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 +61,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_metadata)
#endif /* _UAPI_MISC_OCXL_H */
--
2.14.3
Re: [PATCH 21/23] gcc-plugins: move GCC version check for PowerPC to Kconfig
On 17/02/18 05:38, Masahiro Yamada wrote: For PowerPC, GCC 5.2 is the requirement for GCC plugins. Move the version check to Kconfig, and remove the ugly checker. Signed-off-by: Masahiro Yamada The old checker was as non-ugly as I could make it. :) Acked-by: Andrew Donnellan -- Andrew Donnellan OzLabs, ADL Canberra andrew.donnel...@au1.ibm.com IBM Australia Limited
Re: [PATCH v3 1/2] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
On 22/02/18 15:17, Alastair D'Silva wrote: From: Alastair D'Silva 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 Acked-by: Andrew Donnellan -- Andrew Donnellan OzLabs, ADL Canberra andrew.donnel...@au1.ibm.com IBM Australia Limited
