* [PATCH V6 05/14] powerpc/vas: Setup fault window per VAS instance
From: Haren Myneni @ 2020-03-04 20:46 UTC (permalink / raw)
To: mpe; +Cc: mikey, herbert, npiggin, hch, oohall, sukadev, linuxppc-dev
In-Reply-To: <1583353283.18705.2112.camel@hbabu-laptop>
Setup fault window for each VAS instance. When NX gets a fault on
request buffer, write fault CRBs in the corresponding fault FIFO and
then sends an interrupt to the OS.
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
arch/powerpc/platforms/powernv/Makefile | 2 +-
arch/powerpc/platforms/powernv/vas-fault.c | 77 +++++++++++++++++++++++++++++
arch/powerpc/platforms/powernv/vas-window.c | 4 +-
arch/powerpc/platforms/powernv/vas.c | 20 ++++++++
arch/powerpc/platforms/powernv/vas.h | 16 ++++++
5 files changed, 116 insertions(+), 3 deletions(-)
create mode 100644 arch/powerpc/platforms/powernv/vas-fault.c
diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
index c0f8120..395789f 100644
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -17,7 +17,7 @@ obj-$(CONFIG_MEMORY_FAILURE) += opal-memory-errors.o
obj-$(CONFIG_OPAL_PRD) += opal-prd.o
obj-$(CONFIG_PERF_EVENTS) += opal-imc.o
obj-$(CONFIG_PPC_MEMTRACE) += memtrace.o
-obj-$(CONFIG_PPC_VAS) += vas.o vas-window.o vas-debug.o
+obj-$(CONFIG_PPC_VAS) += vas.o vas-window.o vas-debug.o vas-fault.o
obj-$(CONFIG_OCXL_BASE) += ocxl.o
obj-$(CONFIG_SCOM_DEBUGFS) += opal-xscom.o
obj-$(CONFIG_PPC_SECURE_BOOT) += opal-secvar.o
diff --git a/arch/powerpc/platforms/powernv/vas-fault.c b/arch/powerpc/platforms/powernv/vas-fault.c
new file mode 100644
index 0000000..4044998
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/vas-fault.c
@@ -0,0 +1,77 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * VAS Fault handling.
+ * Copyright 2019, IBM Corporation
+ */
+
+#define pr_fmt(fmt) "vas: " fmt
+
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include <linux/kthread.h>
+#include <asm/icswx.h>
+
+#include "vas.h"
+
+/*
+ * The maximum FIFO size for fault window can be 8MB
+ * (VAS_RX_FIFO_SIZE_MAX). Using 4MB FIFO since each VAS
+ * instance will be having fault window.
+ * 8MB FIFO can be used if expects more faults for each VAS
+ * instance.
+ */
+#define VAS_FAULT_WIN_FIFO_SIZE (4 << 20)
+
+/*
+ * Fault window is opened per VAS instance. NX pastes fault CRB in fault
+ * FIFO upon page faults.
+ */
+int vas_setup_fault_window(struct vas_instance *vinst)
+{
+ struct vas_rx_win_attr attr;
+
+ vinst->fault_fifo_size = VAS_FAULT_WIN_FIFO_SIZE;
+ vinst->fault_fifo = kzalloc(vinst->fault_fifo_size, GFP_KERNEL);
+ if (!vinst->fault_fifo) {
+ pr_err("Unable to alloc %d bytes for fault_fifo\n",
+ vinst->fault_fifo_size);
+ return -ENOMEM;
+ }
+
+ /*
+ * Invalidate all CRB entries. NX pastes valid entry for each fault.
+ */
+ memset(vinst->fault_fifo, FIFO_INVALID_ENTRY, vinst->fault_fifo_size);
+ vas_init_rx_win_attr(&attr, VAS_COP_TYPE_FAULT);
+
+ attr.rx_fifo_size = vinst->fault_fifo_size;
+ attr.rx_fifo = vinst->fault_fifo;
+
+ /*
+ * Max creds is based on number of CRBs can fit in the FIFO.
+ * (fault_fifo_size/CRB_SIZE). If 8MB FIFO is used, max creds
+ * will be 0xffff since the receive creds field is 16bits wide.
+ */
+ attr.wcreds_max = vinst->fault_fifo_size / CRB_SIZE;
+ attr.lnotify_lpid = 0;
+ attr.lnotify_pid = mfspr(SPRN_PID);
+ attr.lnotify_tid = mfspr(SPRN_PID);
+
+ vinst->fault_win = vas_rx_win_open(vinst->vas_id, VAS_COP_TYPE_FAULT,
+ &attr);
+
+ if (IS_ERR(vinst->fault_win)) {
+ pr_err("VAS: Error %ld opening FaultWin\n",
+ PTR_ERR(vinst->fault_win));
+ kfree(vinst->fault_fifo);
+ return PTR_ERR(vinst->fault_win);
+ }
+
+ pr_devel("VAS: Created FaultWin %d, LPID/PID/TID [%d/%d/%d]\n",
+ vinst->fault_win->winid, attr.lnotify_lpid,
+ attr.lnotify_pid, attr.lnotify_tid);
+
+ return 0;
+}
diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c
index 0c0d27d..1783fa9 100644
--- a/arch/powerpc/platforms/powernv/vas-window.c
+++ b/arch/powerpc/platforms/powernv/vas-window.c
@@ -827,9 +827,9 @@ void vas_init_rx_win_attr(struct vas_rx_win_attr *rxattr, enum vas_cop_type cop)
rxattr->fault_win = true;
rxattr->notify_disable = true;
rxattr->rx_wcred_mode = true;
- rxattr->tx_wcred_mode = true;
rxattr->rx_win_ord_mode = true;
- rxattr->tx_win_ord_mode = true;
+ rxattr->rej_no_credit = true;
+ rxattr->tc_mode = VAS_THRESH_DISABLED;
} else if (cop == VAS_COP_TYPE_FTW) {
rxattr->user_win = true;
rxattr->intr_disable = true;
diff --git a/arch/powerpc/platforms/powernv/vas.c b/arch/powerpc/platforms/powernv/vas.c
index 168ab68..557c8e4 100644
--- a/arch/powerpc/platforms/powernv/vas.c
+++ b/arch/powerpc/platforms/powernv/vas.c
@@ -24,6 +24,11 @@
static DEFINE_PER_CPU(int, cpu_vas_id);
+static int vas_irq_fault_window_setup(struct vas_instance *vinst)
+{
+ return vas_setup_fault_window(vinst);
+}
+
static int init_vas_instance(struct platform_device *pdev)
{
struct device_node *dn = pdev->dev.of_node;
@@ -104,6 +109,21 @@ static int init_vas_instance(struct platform_device *pdev)
list_add(&vinst->node, &vas_instances);
mutex_unlock(&vas_mutex);
+ /*
+ * IRQ and fault handling setup is needed only for user space
+ * send windows.
+ */
+ if (vinst->virq) {
+ rc = vas_irq_fault_window_setup(vinst);
+ /*
+ * Fault window is used only for user space send windows.
+ * So if vinst->virq is NULL, tx_win_open returns -ENODEV
+ * for user space.
+ */
+ if (rc)
+ vinst->virq = 0;
+ }
+
vas_instance_init_dbgdir(vinst);
dev_set_drvdata(&pdev->dev, vinst);
diff --git a/arch/powerpc/platforms/powernv/vas.h b/arch/powerpc/platforms/powernv/vas.h
index 598608b..9785c81 100644
--- a/arch/powerpc/platforms/powernv/vas.h
+++ b/arch/powerpc/platforms/powernv/vas.h
@@ -296,6 +296,17 @@ enum vas_notify_after_count {
};
/*
+ * NX can generate an interrupt for multiple faults and expects kernel
+ * to process all of them. So read all valid CRB entries until find the
+ * invalid one. So use pswid which is pasted by NX and ccw[0] (reserved
+ * bit in BE) to check valid CRB.
+ * Invalidate FIFO during allocation and process all entries from last
+ * successful read until finds invalid pswid and ccw[0] values.
+ */
+#define FIFO_INVALID_ENTRY 0xffffffff
+#define CCW0_INVALID (1 << 31)
+
+/*
* One per instance of VAS. Each instance will have a separate set of
* receive windows, one per coprocessor type.
*
@@ -315,6 +326,10 @@ struct vas_instance {
u64 irq_port;
int virq;
+ int fault_fifo_size;
+ void *fault_fifo;
+ struct vas_window *fault_win; /* Fault window */
+
struct mutex mutex;
struct vas_window *rxwin[VAS_COP_TYPE_MAX];
struct vas_window *windows[VAS_WINDOWS_PER_CHIP];
@@ -408,6 +423,7 @@ struct vas_winctx {
extern void vas_instance_init_dbgdir(struct vas_instance *vinst);
extern void vas_window_init_dbgdir(struct vas_window *win);
extern void vas_window_free_dbgdir(struct vas_window *win);
+extern int vas_setup_fault_window(struct vas_instance *vinst);
static inline void vas_log_write(struct vas_window *win, char *name,
void *regptr, u64 val)
--
1.8.3.1
^ permalink raw reply related
* [PATCH V6 04/14] powerpc/vas: Alloc and setup IRQ and trigger port address
From: Haren Myneni @ 2020-03-04 20:46 UTC (permalink / raw)
To: mpe; +Cc: mikey, herbert, npiggin, hch, oohall, sukadev, linuxppc-dev
In-Reply-To: <1583353283.18705.2112.camel@hbabu-laptop>
Alloc IRQ and get trigger port address for each VAS instance. Kernel
register this IRQ per VAS instance and sets this port for each send
window. NX interrupts the kernel when it sees page fault.
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
arch/powerpc/platforms/powernv/vas.c | 34 ++++++++++++++++++++++++++++------
arch/powerpc/platforms/powernv/vas.h | 2 ++
2 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/vas.c b/arch/powerpc/platforms/powernv/vas.c
index ed9cc6d..168ab68 100644
--- a/arch/powerpc/platforms/powernv/vas.c
+++ b/arch/powerpc/platforms/powernv/vas.c
@@ -15,6 +15,7 @@
#include <linux/of_address.h>
#include <linux/of.h>
#include <asm/prom.h>
+#include <asm/xive.h>
#include "vas.h"
@@ -25,10 +26,12 @@
static int init_vas_instance(struct platform_device *pdev)
{
- int rc, cpu, vasid;
- struct resource *res;
- struct vas_instance *vinst;
struct device_node *dn = pdev->dev.of_node;
+ struct vas_instance *vinst;
+ uint32_t chipid, irq;
+ struct resource *res;
+ int rc, cpu, vasid;
+ uint64_t port;
rc = of_property_read_u32(dn, "ibm,vas-id", &vasid);
if (rc) {
@@ -36,6 +39,12 @@ static int init_vas_instance(struct platform_device *pdev)
return -ENODEV;
}
+ rc = of_property_read_u32(dn, "ibm,chip-id", &chipid);
+ if (rc) {
+ pr_err("No ibm,chip-id property for %s?\n", pdev->name);
+ return -ENODEV;
+ }
+
if (pdev->num_resources != 4) {
pr_err("Unexpected DT configuration for [%s, %d]\n",
pdev->name, vasid);
@@ -69,9 +78,22 @@ static int init_vas_instance(struct platform_device *pdev)
vinst->paste_win_id_shift = 63 - res->end;
- pr_devel("Initialized instance [%s, %d], paste_base 0x%llx, "
- "paste_win_id_shift 0x%llx\n", pdev->name, vasid,
- vinst->paste_base_addr, vinst->paste_win_id_shift);
+ rc = xive_native_alloc_get_irq_info(chipid, &irq, &port);
+ if (rc)
+ return rc;
+
+ vinst->virq = irq_create_mapping(NULL, irq);
+ if (!vinst->virq) {
+ pr_err("Inst%d: Unable to map global irq %d\n",
+ vinst->vas_id, irq);
+ return -EINVAL;
+ }
+
+ vinst->irq_port = port;
+ pr_devel("Initialized instance [%s, %d] paste_base 0x%llx paste_win_id_shift 0x%llx IRQ %d Port 0x%llx\n",
+ pdev->name, vasid, vinst->paste_base_addr,
+ vinst->paste_win_id_shift, vinst->virq,
+ vinst->irq_port);
for_each_possible_cpu(cpu) {
if (cpu_to_chip_id(cpu) == of_get_ibm_chip_id(dn))
diff --git a/arch/powerpc/platforms/powernv/vas.h b/arch/powerpc/platforms/powernv/vas.h
index 5574aec..598608b 100644
--- a/arch/powerpc/platforms/powernv/vas.h
+++ b/arch/powerpc/platforms/powernv/vas.h
@@ -313,6 +313,8 @@ struct vas_instance {
u64 paste_base_addr;
u64 paste_win_id_shift;
+ u64 irq_port;
+ int virq;
struct mutex mutex;
struct vas_window *rxwin[VAS_COP_TYPE_MAX];
struct vas_window *windows[VAS_WINDOWS_PER_CHIP];
--
1.8.3.1
^ permalink raw reply related
* [PATCH V6 03/14] powerpc/vas: Define nx_fault_stamp in coprocessor_request_block
From: Haren Myneni @ 2020-03-04 20:45 UTC (permalink / raw)
To: mpe; +Cc: mikey, herbert, npiggin, hch, oohall, sukadev, linuxppc-dev
In-Reply-To: <1583353283.18705.2112.camel@hbabu-laptop>
Kernel sets fault address and status in CRB for NX page fault on user
space address after processing page fault. User space gets the signal
and handles the fault mentioned in CRB by bringing the page in to
memory and send NX request again.
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
arch/powerpc/include/asm/icswx.h | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/icswx.h b/arch/powerpc/include/asm/icswx.h
index 9872f85..b233d1e 100644
--- a/arch/powerpc/include/asm/icswx.h
+++ b/arch/powerpc/include/asm/icswx.h
@@ -108,6 +108,17 @@ struct data_descriptor_entry {
__be64 address;
} __packed __aligned(DDE_ALIGN);
+/* 4.3.2 NX-stamped Fault CRB */
+
+#define NX_STAMP_ALIGN (0x10)
+
+struct nx_fault_stamp {
+ __be64 fault_storage_addr;
+ __be16 reserved;
+ __u8 flags;
+ __u8 fault_status;
+ __be32 pswid;
+} __packed __aligned(NX_STAMP_ALIGN);
/* Chapter 6.5.2 Coprocessor-Request Block (CRB) */
@@ -135,7 +146,12 @@ struct coprocessor_request_block {
struct coprocessor_completion_block ccb;
- u8 reserved[48];
+ union {
+ struct nx_fault_stamp nx;
+ u8 reserved[16];
+ } stamp;
+
+ u8 reserved[32];
struct coprocessor_status_block csb;
} __packed __aligned(CRB_ALIGN);
--
1.8.3.1
^ permalink raw reply related
* [PATCH V6 02/14] powerpc/xive: Define xive_native_alloc_get_irq_info()
From: Haren Myneni @ 2020-03-04 20:45 UTC (permalink / raw)
To: mpe; +Cc: mikey, herbert, npiggin, hch, oohall, sukadev, linuxppc-dev
In-Reply-To: <1583353283.18705.2112.camel@hbabu-laptop>
pnv_ocxl_alloc_xive_irq() in ocxl.c allocates IRQ and gets trigger port
address. VAS also needs this function, but based on chip ID. So moved
this common function to xive/native.c.
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
arch/powerpc/include/asm/xive.h | 2 ++
arch/powerpc/platforms/powernv/ocxl.c | 20 ++------------------
arch/powerpc/sysdev/xive/native.c | 23 +++++++++++++++++++++++
3 files changed, 27 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/include/asm/xive.h b/arch/powerpc/include/asm/xive.h
index d08ea11..fd337da 100644
--- a/arch/powerpc/include/asm/xive.h
+++ b/arch/powerpc/include/asm/xive.h
@@ -139,6 +139,8 @@ int xive_native_set_queue_state(u32 vp_id, uint32_t prio, u32 qtoggle,
int xive_native_get_vp_state(u32 vp_id, u64 *out_state);
bool xive_native_has_queue_state_support(void);
extern u32 xive_native_alloc_irq_on_chip(u32 chip_id);
+extern int xive_native_alloc_get_irq_info(u32 chip_id, u32 *irq,
+ u64 *trigger_addr);
static inline u32 xive_native_alloc_irq(void)
{
diff --git a/arch/powerpc/platforms/powernv/ocxl.c b/arch/powerpc/platforms/powernv/ocxl.c
index 8c65aac..fb8f99a 100644
--- a/arch/powerpc/platforms/powernv/ocxl.c
+++ b/arch/powerpc/platforms/powernv/ocxl.c
@@ -487,24 +487,8 @@ int pnv_ocxl_spa_remove_pe_from_cache(void *platform_data, int pe_handle)
int pnv_ocxl_alloc_xive_irq(u32 *irq, u64 *trigger_addr)
{
- __be64 flags, trigger_page;
- s64 rc;
- u32 hwirq;
-
- hwirq = xive_native_alloc_irq();
- if (!hwirq)
- return -ENOENT;
-
- rc = opal_xive_get_irq_info(hwirq, &flags, NULL, &trigger_page, NULL,
- NULL);
- if (rc || !trigger_page) {
- xive_native_free_irq(hwirq);
- return -ENOENT;
- }
- *irq = hwirq;
- *trigger_addr = be64_to_cpu(trigger_page);
- return 0;
-
+ return xive_native_alloc_get_irq_info(OPAL_XIVE_ANY_CHIP, irq,
+ trigger_addr);
}
EXPORT_SYMBOL_GPL(pnv_ocxl_alloc_xive_irq);
diff --git a/arch/powerpc/sysdev/xive/native.c b/arch/powerpc/sysdev/xive/native.c
index 14d4406..abdd892 100644
--- a/arch/powerpc/sysdev/xive/native.c
+++ b/arch/powerpc/sysdev/xive/native.c
@@ -295,6 +295,29 @@ u32 xive_native_alloc_irq_on_chip(u32 chip_id)
}
EXPORT_SYMBOL_GPL(xive_native_alloc_irq_on_chip);
+int xive_native_alloc_get_irq_info(u32 chip_id, u32 *irq, u64 *trigger_addr)
+{
+ __be64 flags, trigger_page;
+ u32 hwirq;
+ s64 rc;
+
+ hwirq = xive_native_alloc_irq_on_chip(chip_id);
+ if (!hwirq)
+ return -ENOENT;
+
+ rc = opal_xive_get_irq_info(hwirq, &flags, NULL, &trigger_page, NULL,
+ NULL);
+ if (rc || !trigger_page) {
+ xive_native_free_irq(hwirq);
+ return -ENOENT;
+ }
+ *irq = hwirq;
+ *trigger_addr = be64_to_cpu(trigger_page);
+
+ return 0;
+}
+EXPORT_SYMBOL(xive_native_alloc_get_irq_info);
+
void xive_native_free_irq(u32 irq)
{
for (;;) {
--
1.8.3.1
^ permalink raw reply related
* [PATCH V6 01/14] powerpc/xive: Define xive_native_alloc_irq_on_chip()
From: Haren Myneni @ 2020-03-04 20:44 UTC (permalink / raw)
To: mpe; +Cc: mikey, herbert, npiggin, hch, oohall, sukadev, linuxppc-dev
In-Reply-To: <1583353283.18705.2112.camel@hbabu-laptop>
This function allocates IRQ on a specific chip. VAS needs per chip
IRQ allocation and will have IRQ handler per VAS instance.
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
arch/powerpc/include/asm/xive.h | 9 ++++++++-
arch/powerpc/sysdev/xive/native.c | 6 +++---
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/xive.h b/arch/powerpc/include/asm/xive.h
index 93f982db..d08ea11 100644
--- a/arch/powerpc/include/asm/xive.h
+++ b/arch/powerpc/include/asm/xive.h
@@ -5,6 +5,8 @@
#ifndef _ASM_POWERPC_XIVE_H
#define _ASM_POWERPC_XIVE_H
+#include <asm/opal-api.h>
+
#define XIVE_INVALID_VP 0xffffffff
#ifdef CONFIG_PPC_XIVE
@@ -108,7 +110,6 @@ struct xive_q {
int xive_native_populate_irq_data(u32 hw_irq,
struct xive_irq_data *data);
void xive_cleanup_irq_data(struct xive_irq_data *xd);
-u32 xive_native_alloc_irq(void);
void xive_native_free_irq(u32 irq);
int xive_native_configure_irq(u32 hw_irq, u32 target, u8 prio, u32 sw_irq);
@@ -137,6 +138,12 @@ int xive_native_set_queue_state(u32 vp_id, uint32_t prio, u32 qtoggle,
u32 qindex);
int xive_native_get_vp_state(u32 vp_id, u64 *out_state);
bool xive_native_has_queue_state_support(void);
+extern u32 xive_native_alloc_irq_on_chip(u32 chip_id);
+
+static inline u32 xive_native_alloc_irq(void)
+{
+ return xive_native_alloc_irq_on_chip(OPAL_XIVE_ANY_CHIP);
+}
#else
diff --git a/arch/powerpc/sysdev/xive/native.c b/arch/powerpc/sysdev/xive/native.c
index 0ff6b73..14d4406 100644
--- a/arch/powerpc/sysdev/xive/native.c
+++ b/arch/powerpc/sysdev/xive/native.c
@@ -279,12 +279,12 @@ static int xive_native_get_ipi(unsigned int cpu, struct xive_cpu *xc)
}
#endif /* CONFIG_SMP */
-u32 xive_native_alloc_irq(void)
+u32 xive_native_alloc_irq_on_chip(u32 chip_id)
{
s64 rc;
for (;;) {
- rc = opal_xive_allocate_irq(OPAL_XIVE_ANY_CHIP);
+ rc = opal_xive_allocate_irq(chip_id);
if (rc != OPAL_BUSY)
break;
msleep(OPAL_BUSY_DELAY_MS);
@@ -293,7 +293,7 @@ u32 xive_native_alloc_irq(void)
return 0;
return rc;
}
-EXPORT_SYMBOL_GPL(xive_native_alloc_irq);
+EXPORT_SYMBOL_GPL(xive_native_alloc_irq_on_chip);
void xive_native_free_irq(u32 irq)
{
--
1.8.3.1
^ permalink raw reply related
* [PATCH V6 00/14] powerpc/vas: Page fault handling for user space NX requests
From: Haren Myneni @ 2020-03-04 20:21 UTC (permalink / raw)
To: mpe; +Cc: mikey, herbert, npiggin, hch, oohall, sukadev, linuxppc-dev
On power9, Virtual Accelerator Switchboard (VAS) allows user space or
kernel to communicate with Nest Accelerator (NX) directly using COPY/PASTE
instructions. NX provides various functionalities such as compression,
encryption and etc. But only compression (842 and GZIP formats) is
supported in Linux kernel on power9.
842 compression driver (drivers/crypto/nx/nx-842-powernv.c)
is already included in Linux. Only GZIP support will be available from
user space.
Applications can issue GZIP compression / decompression requests to NX with
COPY/PASTE instructions. When NX is processing these requests, can hit
fault on the request buffer (not in memory). It issues an interrupt and
pastes fault CRB in fault FIFO. Expects kernel to handle this fault and
return credits for both send and fault windows after processing.
This patch series adds IRQ and fault window setup, and NX fault handling:
- Alloc IRQ and trigger port address, and configure IRQ per VAS instance.
- Set port# for each window to generate an interrupt when noticed fault.
- Set fault window and FIFO on which NX paste fault CRB.
- Setup IRQ thread fault handler per VAS instance.
- When receiving an interrupt, Read CRBs from fault FIFO and update
coprocessor_status_block (CSB) in the corresponding CRB with translation
failure (CSB_CC_TRANSLATION). After issuing NX requests, process polls
on CSB address. When it sees translation error, can touch the request
buffer to bring the page in to memory and reissue NX request.
- If copy_to_user fails on user space CSB address, OS sends SEGV signal.
Tested these patches with NX-GZIP support and will be posting this series
soon.
Patches 1 & 2: Define alloc IRQ and get port address per chip which are needed
to alloc IRQ per VAS instance.
Patch 3: Define nx_fault_stamp on which NX writes fault status for the fault
CRB
Patch 4: Alloc and setup IRQ and trigger port address for each VAS instance
Patch 5: Setup fault window per each VAS instance. This window is used for
NX to paste fault CRB in FIFO.
Patches 6 & 7: Setup threaded IRQ per VAS and register NX with fault window
ID and port number for each send window so that NX paste fault CRB
in this window.
Patch 8: Reference to pid and mm so that pid is not used until window closed.
Needed for multi thread application where child can open a window
and can be used by parent later.
Patches 9 and 10: Process CRBs from fault FIFO and notify tasks by
updating CSB or through signals.
Patches 11 and 12: Return credits for send and fault windows after handling
faults.
Patch 14:Fix closing send window after all credits are returned. This issue
happens only for user space requests. No page faults on kernel
request buffer.
Changelog:
V2:
- Use threaded IRQ instead of own kernel thread handler
- Use pswid instead of user space CSB address to find valid CRB
- Removed unused macros and other changes as suggested by Christoph Hellwig
V3:
- Rebased to 5.5-rc2
- Use struct pid * instead of pid_t for vas_window tgid
- Code cleanup as suggested by Christoph Hellwig
V4:
- Define xive alloc and get IRQ info based on chip ID and use these
functions for IRQ setup per VAS instance. It eliminates skiboot
dependency as suggested by Oliver.
V5:
- Do not update CSB if the process is exiting (patch9)
V6:
- Add interrupt handler instead of default one and return IRQ_HANDLED
if the fault handling thread is already in progress. (Patch6)
- Use platform send window ID and CCW[0] bit to find valid CRB in
fault FIFO (Patch6).
- Return fault address to user space in BE and other changes as
suggested by Michael Neuling. (patch9)
- Rebased to 5.6-rc4
Haren Myneni (14):
powerpc/xive: Define xive_native_alloc_irq_on_chip()
powerpc/xive: Define xive_native_alloc_get_irq_info()
powerpc/vas: Define nx_fault_stamp in coprocessor_request_block
powerpc/vas: Alloc and setup IRQ and trigger port address
powerpc/vas: Setup fault window per VAS instance
powerpc/vas: Setup thread IRQ handler per VAS instance
powerpc/vas: Register NX with fault window ID and IRQ port value
powerpc/vas: Take reference to PID and mm for user space windows
powerpc/vas: Update CSB and notify process for fault CRBs
powerpc/vas: Print CRB and FIFO values
powerpc/vas: Do not use default credits for receive window
powerpc/vas: Return credits after handling fault
powerpc/vas: Display process stuck message
powerpc/vas: Free send window in VAS instance after credits returned
arch/powerpc/include/asm/icswx.h | 18 +-
arch/powerpc/include/asm/xive.h | 11 +-
arch/powerpc/platforms/powernv/Makefile | 2 +-
arch/powerpc/platforms/powernv/ocxl.c | 20 +-
arch/powerpc/platforms/powernv/vas-debug.c | 2 +-
arch/powerpc/platforms/powernv/vas-fault.c | 331 ++++++++++++++++++++++++++++
arch/powerpc/platforms/powernv/vas-window.c | 185 ++++++++++++++--
arch/powerpc/platforms/powernv/vas.c | 101 ++++++++-
arch/powerpc/platforms/powernv/vas.h | 51 ++++-
arch/powerpc/sysdev/xive/native.c | 29 ++-
10 files changed, 703 insertions(+), 47 deletions(-)
create mode 100644 arch/powerpc/platforms/powernv/vas-fault.c
--
1.8.3.1
^ permalink raw reply
* [PATCH] dt: Remove booting-without-of.txt
From: Rob Herring @ 2020-03-04 18:45 UTC (permalink / raw)
To: devicetree
Cc: Geert Uytterhoeven, linuxppc-dev, linux-kernel,
Mauro Carvalho Chehab, Frank Rowand, linux-arm-kernel
Well, not quite removed yet... Mauro is looking at moving this to ReST,
but I think it would be better to trim or remove it.
boot-without-of.txt is an ancient document that first outlined
Flattened DeviceTree. The DT world has evolved a lot in the 15 years
since and boot-without-of.txt is pretty stale. The name of the document
itself is confusing if you don't understand the evolution from real
'OpenFirmware'. Much of what booting-without-of.txt contains is now in
the DT specification (which evolved out of the ePAPR).
This is a first pass of removing everything that has a DT spec
equivalent or is no longer standard practice (e.g. soc<SoCName> for SoC
nodes) in order to see what's left. This is what I have:
TODO
- Move boot interface details to arch specific docs
- Document 'serial-number' property in DT spec
- Document the 'hotpluggable' memory property in DT spec
- Document the 'sleep' property (PPC only)
- Document the 'dma-coherent' property in DT spec
- Need the history of node names and 'name' property?
- Need how addresses work?
Cc: Frank Rowand <frowand.list@gmail.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
.../devicetree/booting-without-of.txt | 1027 +----------------
1 file changed, 1 insertion(+), 1026 deletions(-)
diff --git a/Documentation/devicetree/booting-without-of.txt b/Documentation/devicetree/booting-without-of.txt
index 4660ccee35a3..97beee828ba4 100644
--- a/Documentation/devicetree/booting-without-of.txt
+++ b/Documentation/devicetree/booting-without-of.txt
@@ -19,44 +19,17 @@ Table of Contents
5) Entry point for arch/sh
II - The DT block format
- 1) Header
2) Device tree generalities
- 3) Device tree "structure" block
- 4) Device tree "strings" block
III - Required content of the device tree
1) Note about cells and address representation
- 2) Note about "compatible" properties
- 3) Note about "name" properties
- 4) Note about node and property names and character set
5) Required nodes and properties
a) The root node
- b) The /cpus node
- c) The /cpus/* nodes
- d) the /memory node(s)
- e) The /chosen node
- f) the /soc<SOCname> node
-
- IV - "dtc", the device tree compiler
-
- V - Recommendations for a bootloader
-
- VI - System-on-a-chip devices and nodes
- 1) Defining child nodes of an SOC
- 2) Representing devices without a current OF specification
-
- VII - Specifying interrupt information for devices
- 1) interrupts property
- 2) interrupt-parent property
- 3) OpenPIC Interrupt Controllers
- 4) ISA Interrupt Controllers
VIII - Specifying device power management information (sleep property)
IX - Specifying dma bus information
- Appendix A - Sample SOC node for MPC8540
-
Revision Information
====================
@@ -105,19 +78,6 @@ Revision Information
- Added chapter VI
- ToDo:
- - Add some definitions of interrupt tree (simple/complex)
- - Add some definitions for PCI host bridges
- - Add some common address format examples
- - Add definitions for standard properties and "compatible"
- names for cells that are not already defined by the existing
- OF spec.
- - Compare FSL SOC use of PCI to standard and make sure no new
- node definition required.
- - Add more information about node definitions for SOC devices
- that currently have no standard, like the FSL CPM.
-
-
I - Introduction
================
@@ -333,196 +293,17 @@ II - The DT block format
========================
-This chapter defines the actual format of the flattened device-tree
-passed to the kernel. The actual content of it and kernel requirements
-are described later. You can find example of code manipulating that
-format in various places, including arch/powerpc/kernel/prom_init.c
-which will generate a flattened device-tree from the Open Firmware
-representation, or the fs2dt utility which is part of the kexec tools
-which will generate one from a filesystem representation. It is
-expected that a bootloader like uboot provides a bit more support,
-that will be discussed later as well.
-
Note: The block has to be in main memory. It has to be accessible in
both real mode and virtual mode with no mapping other than main
memory. If you are writing a simple flash bootloader, it should copy
the block to RAM before passing it to the kernel.
-1) Header
----------
-
- The kernel is passed the physical address pointing to an area of memory
- that is roughly described in include/linux/of_fdt.h by the structure
- boot_param_header:
-
-struct boot_param_header {
- u32 magic; /* magic word OF_DT_HEADER */
- u32 totalsize; /* total size of DT block */
- u32 off_dt_struct; /* offset to structure */
- u32 off_dt_strings; /* offset to strings */
- u32 off_mem_rsvmap; /* offset to memory reserve map
- */
- u32 version; /* format version */
- u32 last_comp_version; /* last compatible version */
-
- /* version 2 fields below */
- u32 boot_cpuid_phys; /* Which physical CPU id we're
- booting on */
- /* version 3 fields below */
- u32 size_dt_strings; /* size of the strings block */
-
- /* version 17 fields below */
- u32 size_dt_struct; /* size of the DT structure block */
-};
-
- Along with the constants:
-
-/* Definitions used by the flattened device tree */
-#define OF_DT_HEADER 0xd00dfeed /* 4: version,
- 4: total size */
-#define OF_DT_BEGIN_NODE 0x1 /* Start node: full name
- */
-#define OF_DT_END_NODE 0x2 /* End node */
-#define OF_DT_PROP 0x3 /* Property: name off,
- size, content */
-#define OF_DT_END 0x9
-
- All values in this header are in big endian format, the various
- fields in this header are defined more precisely below. All
- "offset" values are in bytes from the start of the header; that is
- from the physical base address of the device tree block.
-
- - magic
-
- This is a magic value that "marks" the beginning of the
- device-tree block header. It contains the value 0xd00dfeed and is
- defined by the constant OF_DT_HEADER
-
- - totalsize
-
- This is the total size of the DT block including the header. The
- "DT" block should enclose all data structures defined in this
- chapter (who are pointed to by offsets in this header). That is,
- the device-tree structure, strings, and the memory reserve map.
-
- - off_dt_struct
-
- This is an offset from the beginning of the header to the start
- of the "structure" part the device tree. (see 2) device tree)
-
- - off_dt_strings
-
- This is an offset from the beginning of the header to the start
- of the "strings" part of the device-tree
-
- - off_mem_rsvmap
-
- This is an offset from the beginning of the header to the start
- of the reserved memory map. This map is a list of pairs of 64-
- bit integers. Each pair is a physical address and a size. The
- list is terminated by an entry of size 0. This map provides the
- kernel with a list of physical memory areas that are "reserved"
- and thus not to be used for memory allocations, especially during
- early initialization. The kernel needs to allocate memory during
- boot for things like un-flattening the device-tree, allocating an
- MMU hash table, etc... Those allocations must be done in such a
- way to avoid overriding critical things like, on Open Firmware
- capable machines, the RTAS instance, or on some pSeries, the TCE
- tables used for the iommu. Typically, the reserve map should
- contain _at least_ this DT block itself (header,total_size). If
- you are passing an initrd to the kernel, you should reserve it as
- well. You do not need to reserve the kernel image itself. The map
- should be 64-bit aligned.
-
- - version
-
- This is the version of this structure. Version 1 stops
- here. Version 2 adds an additional field boot_cpuid_phys.
- Version 3 adds the size of the strings block, allowing the kernel
- to reallocate it easily at boot and free up the unused flattened
- structure after expansion. Version 16 introduces a new more
- "compact" format for the tree itself that is however not backward
- compatible. Version 17 adds an additional field, size_dt_struct,
- allowing it to be reallocated or moved more easily (this is
- particularly useful for bootloaders which need to make
- adjustments to a device tree based on probed information). You
- should always generate a structure of the highest version defined
- at the time of your implementation. Currently that is version 17,
- unless you explicitly aim at being backward compatible.
-
- - last_comp_version
-
- Last compatible version. This indicates down to what version of
- the DT block you are backward compatible. For example, version 2
- is backward compatible with version 1 (that is, a kernel build
- for version 1 will be able to boot with a version 2 format). You
- should put a 1 in this field if you generate a device tree of
- version 1 to 3, or 16 if you generate a tree of version 16 or 17
- using the new unit name format.
-
- - boot_cpuid_phys
-
- This field only exist on version 2 headers. It indicate which
- physical CPU ID is calling the kernel entry point. This is used,
- among others, by kexec. If you are on an SMP system, this value
- should match the content of the "reg" property of the CPU node in
- the device-tree corresponding to the CPU calling the kernel entry
- point (see further chapters for more information on the required
- device-tree contents)
-
- - size_dt_strings
-
- This field only exists on version 3 and later headers. It
- gives the size of the "strings" section of the device tree (which
- starts at the offset given by off_dt_strings).
-
- - size_dt_struct
-
- This field only exists on version 17 and later headers. It gives
- the size of the "structure" section of the device tree (which
- starts at the offset given by off_dt_struct).
-
- So the typical layout of a DT block (though the various parts don't
- need to be in that order) looks like this (addresses go from top to
- bottom):
-
-
- ------------------------------
- base -> | struct boot_param_header |
- ------------------------------
- | (alignment gap) (*) |
- ------------------------------
- | memory reserve map |
- ------------------------------
- | (alignment gap) |
- ------------------------------
- | |
- | device-tree structure |
- | |
- ------------------------------
- | (alignment gap) |
- ------------------------------
- | |
- | device-tree strings |
- | |
- -----> ------------------------------
- |
- |
- --- (base + totalsize)
-
- (*) The alignment gaps are not necessarily present; their presence
- and size are dependent on the various alignment requirements of
- the individual data blocks.
2) Device tree generalities
---------------------------
-This device-tree itself is separated in two different blocks, a
-structure block and a strings block. Both need to be aligned to a 4
-byte boundary.
-
First, let's quickly describe the device-tree concept before detailing
the storage format. This chapter does _not_ describe the detail of the
required types of nodes & properties for the kernel, this is done
@@ -574,128 +355,6 @@ is) is also required to have a "compatible" property indicating the
specific hardware and an optional list of devices it is fully
backwards compatible with.
-Finally, every node that can be referenced from a property in another
-node is required to have either a "phandle" or a "linux,phandle"
-property. Real Open Firmware implementations provide a unique
-"phandle" value for every node that the "prom_init()" trampoline code
-turns into "linux,phandle" properties. However, this is made optional
-if the flattened device tree is used directly. An example of a node
-referencing another node via "phandle" is when laying out the
-interrupt tree which will be described in a further version of this
-document.
-
-The "phandle" property is a 32-bit value that uniquely
-identifies a node. You are free to use whatever values or system of
-values, internal pointers, or whatever to generate these, the only
-requirement is that every node for which you provide that property has
-a unique value for it.
-
-Here is an example of a simple device-tree. In this example, an "o"
-designates a node followed by the node unit name. Properties are
-presented with their name followed by their content. "content"
-represents an ASCII string (zero terminated) value, while <content>
-represents a 32-bit value, specified in decimal or hexadecimal (the
-latter prefixed 0x). The various nodes in this example will be
-discussed in a later chapter. At this point, it is only meant to give
-you a idea of what a device-tree looks like. I have purposefully kept
-the "name" and "linux,phandle" properties which aren't necessary in
-order to give you a better idea of what the tree looks like in
-practice.
-
- / o device-tree
- |- name = "device-tree"
- |- model = "MyBoardName"
- |- compatible = "MyBoardFamilyName"
- |- #address-cells = <2>
- |- #size-cells = <2>
- |- linux,phandle = <0>
- |
- o cpus
- | | - name = "cpus"
- | | - linux,phandle = <1>
- | | - #address-cells = <1>
- | | - #size-cells = <0>
- | |
- | o PowerPC,970@0
- | |- name = "PowerPC,970"
- | |- device_type = "cpu"
- | |- reg = <0>
- | |- clock-frequency = <0x5f5e1000>
- | |- 64-bit
- | |- linux,phandle = <2>
- |
- o memory@0
- | |- name = "memory"
- | |- device_type = "memory"
- | |- reg = <0x00000000 0x00000000 0x00000000 0x20000000>
- | |- linux,phandle = <3>
- |
- o chosen
- |- name = "chosen"
- |- bootargs = "root=/dev/sda2"
- |- linux,phandle = <4>
-
-This tree is almost a minimal tree. It pretty much contains the
-minimal set of required nodes and properties to boot a linux kernel;
-that is, some basic model information at the root, the CPUs, and the
-physical memory layout. It also includes misc information passed
-through /chosen, like in this example, the platform type (mandatory)
-and the kernel command line arguments (optional).
-
-The /cpus/PowerPC,970@0/64-bit property is an example of a
-property without a value. All other properties have a value. The
-significance of the #address-cells and #size-cells properties will be
-explained in chapter IV which defines precisely the required nodes and
-properties and their content.
-
-
-3) Device tree "structure" block
-
-The structure of the device tree is a linearized tree structure. The
-"OF_DT_BEGIN_NODE" token starts a new node, and the "OF_DT_END_NODE"
-ends that node definition. Child nodes are simply defined before
-"OF_DT_END_NODE" (that is nodes within the node). A 'token' is a 32
-bit value. The tree has to be "finished" with a OF_DT_END token
-
-Here's the basic structure of a single node:
-
- * token OF_DT_BEGIN_NODE (that is 0x00000001)
- * for version 1 to 3, this is the node full path as a zero
- terminated string, starting with "/". For version 16 and later,
- this is the node unit name only (or an empty string for the
- root node)
- * [align gap to next 4 bytes boundary]
- * for each property:
- * token OF_DT_PROP (that is 0x00000003)
- * 32-bit value of property value size in bytes (or 0 if no
- value)
- * 32-bit value of offset in string block of property name
- * property value data if any
- * [align gap to next 4 bytes boundary]
- * [child nodes if any]
- * token OF_DT_END_NODE (that is 0x00000002)
-
-So the node content can be summarized as a start token, a full path,
-a list of properties, a list of child nodes, and an end token. Every
-child node is a full node structure itself as defined above.
-
-NOTE: The above definition requires that all property definitions for
-a particular node MUST precede any subnode definitions for that node.
-Although the structure would not be ambiguous if properties and
-subnodes were intermingled, the kernel parser requires that the
-properties come first (up until at least 2.6.22). Any tools
-manipulating a flattened tree must take care to preserve this
-constraint.
-
-4) Device tree "strings" block
-
-In order to save space, property names, which are generally redundant,
-are stored separately in the "strings" block. This block is simply the
-whole bunch of zero terminated strings for all property names
-concatenated together. The device-tree property definitions in the
-structure block will contain offset values from the beginning of the
-strings block.
-
III - Required content of the device tree
=========================================
@@ -792,568 +451,14 @@ registers are visible on the parent bus using an identity mapping
translation. In other words, the parent bus address space is the same
as the child bus address space.
-2) Note about "compatible" properties
--------------------------------------
-
-These properties are optional, but recommended in devices and the root
-node. The format of a "compatible" property is a list of concatenated
-zero terminated strings. They allow a device to express its
-compatibility with a family of similar devices, in some cases,
-allowing a single driver to match against several devices regardless
-of their actual names.
-
-3) Note about "name" properties
--------------------------------
-
-While earlier users of Open Firmware like OldWorld macintoshes tended
-to use the actual device name for the "name" property, it's nowadays
-considered a good practice to use a name that is closer to the device
-class (often equal to device_type). For example, nowadays, Ethernet
-controllers are named "ethernet", an additional "model" property
-defining precisely the chip type/model, and "compatible" property
-defining the family in case a single driver can driver more than one
-of these chips. However, the kernel doesn't generally put any
-restriction on the "name" property; it is simply considered good
-practice to follow the standard and its evolutions as closely as
-possible.
-
-Note also that the new format version 16 makes the "name" property
-optional. If it's absent for a node, then the node's unit name is then
-used to reconstruct the name. That is, the part of the unit name
-before the "@" sign is used (or the entire unit name if no "@" sign
-is present).
-
-4) Note about node and property names and character set
--------------------------------------------------------
-
-While Open Firmware provides more flexible usage of 8859-1, this
-specification enforces more strict rules. Nodes and properties should
-be comprised only of ASCII characters 'a' to 'z', '0' to
-'9', ',', '.', '_', '+', '#', '?', and '-'. Node names additionally
-allow uppercase characters 'A' to 'Z' (property names should be
-lowercase. The fact that vendors like Apple don't respect this rule is
-irrelevant here). Additionally, node and property names should always
-begin with a character in the range 'a' to 'z' (or 'A' to 'Z' for node
-names).
-
-The maximum number of characters for both nodes and property names
-is 31. In the case of node names, this is only the leftmost part of
-a unit name (the pure "name" property), it doesn't include the unit
-address which can extend beyond that limit.
-
5) Required nodes and properties
--------------------------------
- These are all that are currently required. However, it is strongly
- recommended that you expose PCI host bridges as documented in the
- PCI binding to Open Firmware, and your interrupt tree as documented
- in OF interrupt tree specification.
-
- a) The root node
-
- The root node requires some properties to be present:
-
- - model : this is your board name/model
- - #address-cells : address representation for "root" devices
- - #size-cells: the size representation for "root" devices
- - compatible : the board "family" generally finds its way here,
- for example, if you have 2 board models with a similar layout,
- that typically get driven by the same platform code in the
- kernel, you would specify the exact board model in the
- compatible property followed by an entry that represents the SoC
- model.
-
- The root node is also generally where you add additional properties
- specific to your board like the serial number if any, that sort of
- thing. It is recommended that if you add any "custom" property whose
- name may clash with standard defined ones, you prefix them with your
- vendor name and a comma.
Additional properties for the root node:
- serial-number : a string representing the device's serial number
- b) The /cpus node
-
- This node is the parent of all individual CPU nodes. It doesn't
- have any specific requirements, though it's generally good practice
- to have at least:
-
- #address-cells = <00000001>
- #size-cells = <00000000>
-
- This defines that the "address" for a CPU is a single cell, and has
- no meaningful size. This is not necessary but the kernel will assume
- that format when reading the "reg" properties of a CPU node, see
- below
-
- c) The /cpus/* nodes
-
- So under /cpus, you are supposed to create a node for every CPU on
- the machine. There is no specific restriction on the name of the
- CPU, though it's common to call it <architecture>,<core>. For
- example, Apple uses PowerPC,G5 while IBM uses PowerPC,970FX.
- However, the Generic Names convention suggests that it would be
- better to simply use 'cpu' for each cpu node and use the compatible
- property to identify the specific cpu core.
-
- Required properties:
-
- - device_type : has to be "cpu"
- - reg : This is the physical CPU number, it's a single 32-bit cell
- and is also used as-is as the unit number for constructing the
- unit name in the full path. For example, with 2 CPUs, you would
- have the full path:
- /cpus/PowerPC,970FX@0
- /cpus/PowerPC,970FX@1
- (unit addresses do not require leading zeroes)
- - d-cache-block-size : one cell, L1 data cache block size in bytes (*)
- - i-cache-block-size : one cell, L1 instruction cache block size in
- bytes
- - d-cache-size : one cell, size of L1 data cache in bytes
- - i-cache-size : one cell, size of L1 instruction cache in bytes
-
-(*) The cache "block" size is the size on which the cache management
-instructions operate. Historically, this document used the cache
-"line" size here which is incorrect. The kernel will prefer the cache
-block size and will fallback to cache line size for backward
-compatibility.
-
- Recommended properties:
-
- - timebase-frequency : a cell indicating the frequency of the
- timebase in Hz. This is not directly used by the generic code,
- but you are welcome to copy/paste the pSeries code for setting
- the kernel timebase/decrementer calibration based on this
- value.
- - clock-frequency : a cell indicating the CPU core clock frequency
- in Hz. A new property will be defined for 64-bit values, but if
- your frequency is < 4Ghz, one cell is enough. Here as well as
- for the above, the common code doesn't use that property, but
- you are welcome to re-use the pSeries or Maple one. A future
- kernel version might provide a common function for this.
- - d-cache-line-size : one cell, L1 data cache line size in bytes
- if different from the block size
- - i-cache-line-size : one cell, L1 instruction cache line size in
- bytes if different from the block size
-
- You are welcome to add any property you find relevant to your board,
- like some information about the mechanism used to soft-reset the
- CPUs. For example, Apple puts the GPIO number for CPU soft reset
- lines in there as a "soft-reset" property since they start secondary
- CPUs by soft-resetting them.
-
-
- d) the /memory node(s)
-
- To define the physical memory layout of your board, you should
- create one or more memory node(s). You can either create a single
- node with all memory ranges in its reg property, or you can create
- several nodes, as you wish. The unit address (@ part) used for the
- full path is the address of the first range of memory defined by a
- given node. If you use a single memory node, this will typically be
- @0.
-
- Required properties:
-
- - device_type : has to be "memory"
- - reg : This property contains all the physical memory ranges of
- your board. It's a list of addresses/sizes concatenated
- together, with the number of cells of each defined by the
- #address-cells and #size-cells of the root node. For example,
- with both of these properties being 2 like in the example given
- earlier, a 970 based machine with 6Gb of RAM could typically
- have a "reg" property here that looks like:
-
- 00000000 00000000 00000000 80000000
- 00000001 00000000 00000001 00000000
-
- That is a range starting at 0 of 0x80000000 bytes and a range
- starting at 0x100000000 and of 0x100000000 bytes. You can see
- that there is no memory covering the IO hole between 2Gb and
- 4Gb. Some vendors prefer splitting those ranges into smaller
- segments, but the kernel doesn't care.
-
- Additional properties:
-
- - hotpluggable : The presence of this property provides an explicit
- hint to the operating system that this memory may potentially be
- removed later. The kernel can take this into consideration when
- doing nonmovable allocations and when laying out memory zones.
-
- e) The /chosen node
-
- This node is a bit "special". Normally, that's where Open Firmware
- puts some variable environment information, like the arguments, or
- the default input/output devices.
-
- This specification makes a few of these mandatory, but also defines
- some linux-specific properties that would be normally constructed by
- the prom_init() trampoline when booting with an OF client interface,
- but that you have to provide yourself when using the flattened format.
-
- Recommended properties:
-
- - bootargs : This zero-terminated string is passed as the kernel
- command line
- - linux,stdout-path : This is the full path to your standard
- console device if any. Typically, if you have serial devices on
- your board, you may want to put the full path to the one set as
- the default console in the firmware here, for the kernel to pick
- it up as its own default console.
-
- Note that u-boot creates and fills in the chosen node for platforms
- that use it.
-
- (Note: a practice that is now obsolete was to include a property
- under /chosen called interrupt-controller which had a phandle value
- that pointed to the main interrupt controller)
-
- f) the /soc<SOCname> node
-
- This node is used to represent a system-on-a-chip (SoC) and must be
- present if the processor is a SoC. The top-level soc node contains
- information that is global to all devices on the SoC. The node name
- should contain a unit address for the SoC, which is the base address
- of the memory-mapped register set for the SoC. The name of an SoC
- node should start with "soc", and the remainder of the name should
- represent the part number for the soc. For example, the MPC8540's
- soc node would be called "soc8540".
-
- Required properties:
-
- - ranges : Should be defined as specified in 1) to describe the
- translation of SoC addresses for memory mapped SoC registers.
- - bus-frequency: Contains the bus frequency for the SoC node.
- Typically, the value of this field is filled in by the boot
- loader.
- - compatible : Exact model of the SoC
-
-
- Recommended properties:
-
- - reg : This property defines the address and size of the
- memory-mapped registers that are used for the SOC node itself.
- It does not include the child device registers - these will be
- defined inside each child node. The address specified in the
- "reg" property should match the unit address of the SOC node.
- - #address-cells : Address representation for "soc" devices. The
- format of this field may vary depending on whether or not the
- device registers are memory mapped. For memory mapped
- registers, this field represents the number of cells needed to
- represent the address of the registers. For SOCs that do not
- use MMIO, a special address format should be defined that
- contains enough cells to represent the required information.
- See 1) above for more details on defining #address-cells.
- - #size-cells : Size representation for "soc" devices
- - #interrupt-cells : Defines the width of cells used to represent
- interrupts. Typically this value is <2>, which includes a
- 32-bit number that represents the interrupt number, and a
- 32-bit number that represents the interrupt sense and level.
- This field is only needed if the SOC contains an interrupt
- controller.
-
- The SOC node may contain child nodes for each SOC device that the
- platform uses. Nodes should not be created for devices which exist
- on the SOC but are not used by a particular platform. See chapter VI
- for more information on how to specify devices that are part of a SOC.
-
- Example SOC node for the MPC8540:
-
- soc8540@e0000000 {
- #address-cells = <1>;
- #size-cells = <1>;
- #interrupt-cells = <2>;
- device_type = "soc";
- ranges = <0x00000000 0xe0000000 0x00100000>
- reg = <0xe0000000 0x00003000>;
- bus-frequency = <0>;
- }
-
-
-
-IV - "dtc", the device tree compiler
-====================================
-
-
-dtc source code can be found at
-<http://git.jdl.com/gitweb/?p=dtc.git>
-
-WARNING: This version is still in early development stage; the
-resulting device-tree "blobs" have not yet been validated with the
-kernel. The current generated block lacks a useful reserve map (it will
-be fixed to generate an empty one, it's up to the bootloader to fill
-it up) among others. The error handling needs work, bugs are lurking,
-etc...
-
-dtc basically takes a device-tree in a given format and outputs a
-device-tree in another format. The currently supported formats are:
-
- Input formats:
- -------------
-
- - "dtb": "blob" format, that is a flattened device-tree block
- with
- header all in a binary blob.
- - "dts": "source" format. This is a text file containing a
- "source" for a device-tree. The format is defined later in this
- chapter.
- - "fs" format. This is a representation equivalent to the
- output of /proc/device-tree, that is nodes are directories and
- properties are files
-
- Output formats:
- ---------------
-
- - "dtb": "blob" format
- - "dts": "source" format
- - "asm": assembly language file. This is a file that can be
- sourced by gas to generate a device-tree "blob". That file can
- then simply be added to your Makefile. Additionally, the
- assembly file exports some symbols that can be used.
-
-
-The syntax of the dtc tool is
-
- dtc [-I <input-format>] [-O <output-format>]
- [-o output-filename] [-V output_version] input_filename
-
-
-The "output_version" defines what version of the "blob" format will be
-generated. Supported versions are 1,2,3 and 16. The default is
-currently version 3 but that may change in the future to version 16.
-
-Additionally, dtc performs various sanity checks on the tree, like the
-uniqueness of linux, phandle properties, validity of strings, etc...
-
-The format of the .dts "source" file is "C" like, supports C and C++
-style comments.
-
-/ {
-}
-
-The above is the "device-tree" definition. It's the only statement
-supported currently at the toplevel.
-
-/ {
- property1 = "string_value"; /* define a property containing a 0
- * terminated string
- */
-
- property2 = <0x1234abcd>; /* define a property containing a
- * numerical 32-bit value (hexadecimal)
- */
-
- property3 = <0x12345678 0x12345678 0xdeadbeef>;
- /* define a property containing 3
- * numerical 32-bit values (cells) in
- * hexadecimal
- */
- property4 = [0x0a 0x0b 0x0c 0x0d 0xde 0xea 0xad 0xbe 0xef];
- /* define a property whose content is
- * an arbitrary array of bytes
- */
-
- childnode@address { /* define a child node named "childnode"
- * whose unit name is "childnode at
- * address"
- */
-
- childprop = "hello\n"; /* define a property "childprop" of
- * childnode (in this case, a string)
- */
- };
-};
-
-Nodes can contain other nodes etc... thus defining the hierarchical
-structure of the tree.
-
-Strings support common escape sequences from C: "\n", "\t", "\r",
-"\(octal value)", "\x(hex value)".
-
-It is also suggested that you pipe your source file through cpp (gcc
-preprocessor) so you can use #include's, #define for constants, etc...
-
-Finally, various options are planned but not yet implemented, like
-automatic generation of phandles, labels (exported to the asm file so
-you can point to a property content and change it easily from whatever
-you link the device-tree with), label or path instead of numeric value
-in some cells to "point" to a node (replaced by a phandle at compile
-time), export of reserve map address to the asm file, ability to
-specify reserve map content at compile time, etc...
-
-We may provide a .h include file with common definitions of that
-proves useful for some properties (like building PCI properties or
-interrupt maps) though it may be better to add a notion of struct
-definitions to the compiler...
-
-
-V - Recommendations for a bootloader
-====================================
-
-
-Here are some various ideas/recommendations that have been proposed
-while all this has been defined and implemented.
-
- - The bootloader may want to be able to use the device-tree itself
- and may want to manipulate it (to add/edit some properties,
- like physical memory size or kernel arguments). At this point, 2
- choices can be made. Either the bootloader works directly on the
- flattened format, or the bootloader has its own internal tree
- representation with pointers (similar to the kernel one) and
- re-flattens the tree when booting the kernel. The former is a bit
- more difficult to edit/modify, the later requires probably a bit
- more code to handle the tree structure. Note that the structure
- format has been designed so it's relatively easy to "insert"
- properties or nodes or delete them by just memmoving things
- around. It contains no internal offsets or pointers for this
- purpose.
-
- - An example of code for iterating nodes & retrieving properties
- directly from the flattened tree format can be found in the kernel
- file drivers/of/fdt.c. Look at the of_scan_flat_dt() function,
- its usage in early_init_devtree(), and the corresponding various
- early_init_dt_scan_*() callbacks. That code can be re-used in a
- GPL bootloader, and as the author of that code, I would be happy
- to discuss possible free licensing to any vendor who wishes to
- integrate all or part of this code into a non-GPL bootloader.
- (reference needed; who is 'I' here? ---gcl Jan 31, 2011)
-
-
-
-VI - System-on-a-chip devices and nodes
-=======================================
-
-Many companies are now starting to develop system-on-a-chip
-processors, where the processor core (CPU) and many peripheral devices
-exist on a single piece of silicon. For these SOCs, an SOC node
-should be used that defines child nodes for the devices that make
-up the SOC. While platforms are not required to use this model in
-order to boot the kernel, it is highly encouraged that all SOC
-implementations define as complete a flat-device-tree as possible to
-describe the devices on the SOC. This will allow for the
-genericization of much of the kernel code.
-
-
-1) Defining child nodes of an SOC
----------------------------------
-
-Each device that is part of an SOC may have its own node entry inside
-the SOC node. For each device that is included in the SOC, the unit
-address property represents the address offset for this device's
-memory-mapped registers in the parent's address space. The parent's
-address space is defined by the "ranges" property in the top-level soc
-node. The "reg" property for each node that exists directly under the
-SOC node should contain the address mapping from the child address space
-to the parent SOC address space and the size of the device's
-memory-mapped register file.
-
-For many devices that may exist inside an SOC, there are predefined
-specifications for the format of the device tree node. All SOC child
-nodes should follow these specifications, except where noted in this
-document.
-
-See appendix A for an example partial SOC node definition for the
-MPC8540.
-
-
-2) Representing devices without a current OF specification
-----------------------------------------------------------
-
-Currently, there are many devices on SoCs that do not have a standard
-representation defined as part of the Open Firmware specifications,
-mainly because the boards that contain these SoCs are not currently
-booted using Open Firmware. Binding documentation for new devices
-should be added to the Documentation/devicetree/bindings directory.
-That directory will expand as device tree support is added to more and
-more SoCs.
-
-
-VII - Specifying interrupt information for devices
-===================================================
-
-The device tree represents the buses and devices of a hardware
-system in a form similar to the physical bus topology of the
-hardware.
-
-In addition, a logical 'interrupt tree' exists which represents the
-hierarchy and routing of interrupts in the hardware.
-
-The interrupt tree model is fully described in the
-document "Open Firmware Recommended Practice: Interrupt
-Mapping Version 0.9". The document is available at:
-<http://www.devicetree.org/open-firmware/practice/>
-
-1) interrupts property
-----------------------
-
-Devices that generate interrupts to a single interrupt controller
-should use the conventional OF representation described in the
-OF interrupt mapping documentation.
-
-Each device which generates interrupts must have an 'interrupt'
-property. The interrupt property value is an arbitrary number of
-of 'interrupt specifier' values which describe the interrupt or
-interrupts for the device.
-
-The encoding of an interrupt specifier is determined by the
-interrupt domain in which the device is located in the
-interrupt tree. The root of an interrupt domain specifies in
-its #interrupt-cells property the number of 32-bit cells
-required to encode an interrupt specifier. See the OF interrupt
-mapping documentation for a detailed description of domains.
-
-For example, the binding for the OpenPIC interrupt controller
-specifies an #interrupt-cells value of 2 to encode the interrupt
-number and level/sense information. All interrupt children in an
-OpenPIC interrupt domain use 2 cells per interrupt in their interrupts
-property.
-
-The PCI bus binding specifies a #interrupt-cells value of 1 to encode
-which interrupt pin (INTA,INTB,INTC,INTD) is used.
-
-2) interrupt-parent property
-----------------------------
-
-The interrupt-parent property is specified to define an explicit
-link between a device node and its interrupt parent in
-the interrupt tree. The value of interrupt-parent is the
-phandle of the parent node.
-
-If the interrupt-parent property is not defined for a node, its
-interrupt parent is assumed to be an ancestor in the node's
-_device tree_ hierarchy.
-
-3) OpenPIC Interrupt Controllers
---------------------------------
-
-OpenPIC interrupt controllers require 2 cells to encode
-interrupt information. The first cell defines the interrupt
-number. The second cell defines the sense and level
-information.
-
-Sense and level information should be encoded as follows:
-
- 0 = low to high edge sensitive type enabled
- 1 = active low level sensitive type enabled
- 2 = active high level sensitive type enabled
- 3 = high to low edge sensitive type enabled
-
-4) ISA Interrupt Controllers
-----------------------------
-
-ISA PIC interrupt controllers require 2 cells to encode
-interrupt information. The first cell defines the interrupt
-number. The second cell defines the sense and level
-information.
-
-ISA PIC interrupt controllers should adhere to the ISA PIC
-encodings listed below:
-
- 0 = active low level sensitive type enabled
- 1 = active high level sensitive type enabled
- 2 = high to low edge sensitive type enabled
- 3 = low to high edge sensitive type enabled
VIII - Specifying Device Power Management Information (sleep property)
===================================================================
@@ -1386,6 +491,7 @@ reasonably grouped in this manner, then create a virtual sleep controller
(similar to an interrupt nexus, except that defining a standardized
sleep-map should wait until its necessity is demonstrated).
+
IX - Specifying dma bus information
Some devices may have DMA memory range shifted relatively to the beginning of
@@ -1420,134 +526,3 @@ Optional property:
- dma-ranges: <empty> value. if present - It means that DMA addresses
translation has to be enabled for this device.
- dma-coherent: Present if dma operations are coherent
-
-Example:
-soc {
- compatible = "ti,keystone","simple-bus";
- ranges = <0x0 0x0 0x0 0xc0000000>;
- dma-ranges = <0x80000000 0x8 0x00000000 0x80000000>;
-
- [...]
-
- usb: usb@2680000 {
- compatible = "ti,keystone-dwc3";
-
- [...]
- dma-coherent;
- };
-};
-
-Appendix A - Sample SOC node for MPC8540
-========================================
-
- soc@e0000000 {
- #address-cells = <1>;
- #size-cells = <1>;
- compatible = "fsl,mpc8540-ccsr", "simple-bus";
- device_type = "soc";
- ranges = <0x00000000 0xe0000000 0x00100000>
- bus-frequency = <0>;
- interrupt-parent = <&pic>;
-
- ethernet@24000 {
- #address-cells = <1>;
- #size-cells = <1>;
- device_type = "network";
- model = "TSEC";
- compatible = "gianfar", "simple-bus";
- reg = <0x24000 0x1000>;
- local-mac-address = [ 0x00 0xE0 0x0C 0x00 0x73 0x00 ];
- interrupts = <0x29 2 0x30 2 0x34 2>;
- phy-handle = <&phy0>;
- sleep = <&pmc 0x00000080>;
- ranges;
-
- mdio@24520 {
- reg = <0x24520 0x20>;
- compatible = "fsl,gianfar-mdio";
-
- phy0: ethernet-phy@0 {
- interrupts = <5 1>;
- reg = <0>;
- };
-
- phy1: ethernet-phy@1 {
- interrupts = <5 1>;
- reg = <1>;
- };
-
- phy3: ethernet-phy@3 {
- interrupts = <7 1>;
- reg = <3>;
- };
- };
- };
-
- ethernet@25000 {
- device_type = "network";
- model = "TSEC";
- compatible = "gianfar";
- reg = <0x25000 0x1000>;
- local-mac-address = [ 0x00 0xE0 0x0C 0x00 0x73 0x01 ];
- interrupts = <0x13 2 0x14 2 0x18 2>;
- phy-handle = <&phy1>;
- sleep = <&pmc 0x00000040>;
- };
-
- ethernet@26000 {
- device_type = "network";
- model = "FEC";
- compatible = "gianfar";
- reg = <0x26000 0x1000>;
- local-mac-address = [ 0x00 0xE0 0x0C 0x00 0x73 0x02 ];
- interrupts = <0x41 2>;
- phy-handle = <&phy3>;
- sleep = <&pmc 0x00000020>;
- };
-
- serial@4500 {
- #address-cells = <1>;
- #size-cells = <1>;
- compatible = "fsl,mpc8540-duart", "simple-bus";
- sleep = <&pmc 0x00000002>;
- ranges;
-
- serial@4500 {
- device_type = "serial";
- compatible = "ns16550";
- reg = <0x4500 0x100>;
- clock-frequency = <0>;
- interrupts = <0x42 2>;
- };
-
- serial@4600 {
- device_type = "serial";
- compatible = "ns16550";
- reg = <0x4600 0x100>;
- clock-frequency = <0>;
- interrupts = <0x42 2>;
- };
- };
-
- pic: pic@40000 {
- interrupt-controller;
- #address-cells = <0>;
- #interrupt-cells = <2>;
- reg = <0x40000 0x40000>;
- compatible = "chrp,open-pic";
- device_type = "open-pic";
- };
-
- i2c@3000 {
- interrupts = <0x43 2>;
- reg = <0x3000 0x100>;
- compatible = "fsl-i2c";
- dfsrr;
- sleep = <&pmc 0x00000004>;
- };
-
- pmc: power@e0070 {
- compatible = "fsl,mpc8540-pmc", "fsl,mpc8548-pmc";
- reg = <0xe0070 0x20>;
- };
- };
--
2.20.1
^ permalink raw reply related
* [RFC 3/3] Introduce capability for firmware-enabled-stop
From: Pratik Rajesh Sampat @ 2020-03-04 16:01 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel, mpe, mikey, npiggin, vaidy, ego,
skiboot, oohall, psampat, pratik.r.sampat
In-Reply-To: <cover.1583332695.git.psampat@linux.ibm.com>
Design patch that introduces the capability for firmware to handle the
stop states instead. A bit is set based on the discovery of the feature
and correspondingly also the responsibility to handle the stop states.
The commit does not contain calling into the firmware to utilize
firmware enabled stop.
Signed-off-by: Pratik Rajesh Sampat <psampat at linux.ibm.com>
---
arch/powerpc/include/asm/processor.h | 1 +
arch/powerpc/kernel/dt_cpu_ftrs.c | 9 +++++++++
2 files changed, 10 insertions(+)
diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index 277dbabafd02..978fab35d133 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -430,6 +430,7 @@ extern unsigned long cpuidle_disable;
enum idle_boot_override {IDLE_NO_OVERRIDE = 0, IDLE_POWERSAVE_OFF};
#define STOP_ENABLE 0x00000001
+#define FIRMWARE_STOP_ENABLE 0x00000010
#define STOP_VERSION_P9 0x1
#define STOP_VERSION_P9_V1 0x2
diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
index 63e30aa49356..e00f8afabc46 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -313,6 +313,14 @@ static int __init feat_enable_idle_stop_quirk(struct dt_cpu_feature *f)
return 1;
}
+
+static int __init feat_enable_firmware_stop(struct dt_cpu_feature *f)
+{
+ stop_dep.cpuidle_prop |= FIRMWARE_STOP_ENABLE;
+
+ return 1;
+}
+
static int __init feat_enable_mmu_hash(struct dt_cpu_feature *f)
{
u64 lpcr;
@@ -608,6 +616,7 @@ static struct dt_cpu_feature_match __initdata
{"alignment-interrupt-dsisr", feat_enable_align_dsisr, 0},
{"idle-stop", feat_enable_idle_stop, 0},
{"idle-stop-v1", feat_enable_idle_stop_quirk, 0},
+ {"firmware-stop-supported", feat_enable_firmware_stop, 0},
{"machine-check-power8", feat_enable_mce_power8, 0},
{"performance-monitor-power8", feat_enable_pmu_power8, 0},
{"data-stream-control-register", feat_enable_dscr, CPU_FTR_DSCR},
--
2.24.1
^ permalink raw reply related
* [RFC 2/3] Demonstration of handling an idle-stop quirk version
From: Pratik Rajesh Sampat @ 2020-03-04 16:01 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel, mpe, mikey, npiggin, vaidy, ego,
skiboot, oohall, psampat, pratik.r.sampat
In-Reply-To: <cover.1583332695.git.psampat@linux.ibm.com>
Concept patch demonstrating an idle-stop version discovery from the
device tree, along with population its support and versioning. It also
assigns the function pointer to handle any idle-stop specific quirks.
Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
---
arch/powerpc/include/asm/processor.h | 1 +
arch/powerpc/kernel/dt_cpu_ftrs.c | 16 ++++++++++++++++
arch/powerpc/platforms/powernv/idle.c | 8 ++++++++
3 files changed, 25 insertions(+)
diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index da59f01a5c09..277dbabafd02 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -432,6 +432,7 @@ enum idle_boot_override {IDLE_NO_OVERRIDE = 0, IDLE_POWERSAVE_OFF};
#define STOP_ENABLE 0x00000001
#define STOP_VERSION_P9 0x1
+#define STOP_VERSION_P9_V1 0x2
/*
* Classify the dependencies of the stop states
diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
index db1a525e090d..63e30aa49356 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -298,6 +298,21 @@ static int __init feat_enable_idle_stop(struct dt_cpu_feature *f)
return 1;
}
+static int __init feat_enable_idle_stop_quirk(struct dt_cpu_feature *f)
+{
+ u64 lpcr;
+
+ /* Set PECE wakeup modes for ISAv3.0B */
+ lpcr = mfspr(SPRN_LPCR);
+ lpcr |= LPCR_PECE0;
+ lpcr |= LPCR_PECE1;
+ lpcr |= LPCR_PECE2;
+ mtspr(SPRN_LPCR, lpcr);
+ stop_dep.cpuidle_prop |= STOP_ENABLE;
+ stop_dep.stop_version = STOP_VERSION_P9_V1;
+
+ return 1;
+}
static int __init feat_enable_mmu_hash(struct dt_cpu_feature *f)
{
u64 lpcr;
@@ -592,6 +607,7 @@ static struct dt_cpu_feature_match __initdata
{"idle-nap", feat_enable_idle_nap, 0},
{"alignment-interrupt-dsisr", feat_enable_align_dsisr, 0},
{"idle-stop", feat_enable_idle_stop, 0},
+ {"idle-stop-v1", feat_enable_idle_stop_quirk, 0},
{"machine-check-power8", feat_enable_mce_power8, 0},
{"performance-monitor-power8", feat_enable_pmu_power8, 0},
{"data-stream-control-register", feat_enable_dscr, CPU_FTR_DSCR},
diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index c32cdc37acf4..9f5b21da2fc5 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -805,6 +805,11 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
return srr1;
}
+static unsigned long power9_idle_quirky_stop(unsigned long psscr, bool mmu_on)
+{
+ return power9_idle_stop(psscr, mmu_on);
+}
+
#ifdef CONFIG_HOTPLUG_CPU
static unsigned long power9_offline_stop(unsigned long psscr)
{
@@ -1360,6 +1365,9 @@ static int __init pnv_init_idle_states(void)
case STOP_VERSION_P9:
stop_dep.idle_stop = power9_idle_stop;
break;
+ case STOP_VERSION_P9_V1:
+ stop_dep.idle_stop = power9_idle_quirky_stop;
+ break;
default:
stop_dep.idle_stop = NULL;
goto out;
--
2.24.1
^ permalink raw reply related
* [RFC 0/3] cpuidle/powernv: Interface to handle idle-stop versioning
From: Pratik Rajesh Sampat @ 2020-03-04 16:01 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel, mpe, mikey, npiggin, vaidy, ego,
skiboot, oohall, psampat, pratik.r.sampat
A design patch series illuminates the idea of handling different
versions of idle-stop, the properties they support and the
quirks that need to be handled before entering or after exiting stop.
It also adds a functionality to identify firmware-enabled-stop and set
the according bits to encapsulate the support and corresponding handling
Corresponding RFC skiboot patch: https://lists.ozlabs.org/pipermail/skiboot/2020-March/016552.html
Pratik Rajesh Sampat (3):
Interface for an idle-stop dependency structure
Demonstration of handling an idle-stop quirk version
Introduce capability for firmware-enabled-stop
arch/powerpc/include/asm/processor.h | 19 +++++++++++++++++
arch/powerpc/kernel/dt_cpu_ftrs.c | 30 +++++++++++++++++++++++++++
arch/powerpc/platforms/powernv/idle.c | 25 ++++++++++++++++++----
drivers/cpuidle/cpuidle-powernv.c | 3 ++-
4 files changed, 72 insertions(+), 5 deletions(-)
--
2.24.1
^ permalink raw reply
* [RFC 1/3] Interface for an idle-stop dependency structure
From: Pratik Rajesh Sampat @ 2020-03-04 16:01 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel, mpe, mikey, npiggin, vaidy, ego,
skiboot, oohall, psampat, pratik.r.sampat
In-Reply-To: <cover.1583332695.git.psampat@linux.ibm.com>
Design patch to introduce the idea of having a dependency structure for
idle-stop. The structure encapsulates the following:
1. Bitmask for version of idle-stop
2. Bitmask for propterties like ENABLE/DISABLE
3. Function pointer which helps handle how the stop must be invoked
The commit lays a foundation for other idle-stop versions to be added
and handled cleanly based on their specified requirments.
Currently it handles the existing "idle-stop" version by setting the
discovery bits and the function pointer.
Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
---
arch/powerpc/include/asm/processor.h | 17 +++++++++++++++++
arch/powerpc/kernel/dt_cpu_ftrs.c | 5 +++++
arch/powerpc/platforms/powernv/idle.c | 17 +++++++++++++----
drivers/cpuidle/cpuidle-powernv.c | 3 ++-
4 files changed, 37 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index eedcbfb9a6ff..da59f01a5c09 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -429,6 +429,23 @@ extern void power4_idle_nap(void);
extern unsigned long cpuidle_disable;
enum idle_boot_override {IDLE_NO_OVERRIDE = 0, IDLE_POWERSAVE_OFF};
+#define STOP_ENABLE 0x00000001
+
+#define STOP_VERSION_P9 0x1
+
+/*
+ * Classify the dependencies of the stop states
+ * @idle_stop: function handler to handle the quirk stop version
+ * @cpuidle_prop: Signify support for stop states through kernel and/or firmware
+ * @stop_version: Classify quirk versions for stop states
+ */
+typedef struct {
+ unsigned long (*idle_stop)(unsigned long, bool);
+ uint8_t cpuidle_prop;
+ uint8_t stop_version;
+}stop_deps_t;
+extern stop_deps_t stop_dep;
+
extern int powersave_nap; /* set if nap mode can be used in idle loop */
extern void power7_idle_type(unsigned long type);
diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
index 182b4047c1ef..db1a525e090d 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -292,6 +292,8 @@ static int __init feat_enable_idle_stop(struct dt_cpu_feature *f)
lpcr |= LPCR_PECE1;
lpcr |= LPCR_PECE2;
mtspr(SPRN_LPCR, lpcr);
+ stop_dep.cpuidle_prop |= STOP_ENABLE;
+ stop_dep.stop_version = STOP_VERSION_P9;
return 1;
}
@@ -657,6 +659,9 @@ static void __init cpufeatures_setup_start(u32 isa)
}
}
+stop_deps_t stop_dep = {NULL, 0x0, 0x0};
+EXPORT_SYMBOL(stop_dep);
+
static bool __init cpufeatures_process_feature(struct dt_cpu_feature *f)
{
const struct dt_cpu_feature_match *m;
diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index 78599bca66c2..c32cdc37acf4 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -812,7 +812,7 @@ static unsigned long power9_offline_stop(unsigned long psscr)
#ifndef CONFIG_KVM_BOOK3S_HV_POSSIBLE
__ppc64_runlatch_off();
- srr1 = power9_idle_stop(psscr, true);
+ srr1 = stop_dep.idle_stop(psscr, true);
__ppc64_runlatch_on();
#else
/*
@@ -828,7 +828,7 @@ static unsigned long power9_offline_stop(unsigned long psscr)
local_paca->kvm_hstate.hwthread_state = KVM_HWTHREAD_IN_IDLE;
__ppc64_runlatch_off();
- srr1 = power9_idle_stop(psscr, false);
+ srr1 = stop_dep.idle_stop(psscr, true);
__ppc64_runlatch_on();
local_paca->kvm_hstate.hwthread_state = KVM_HWTHREAD_IN_KERNEL;
@@ -856,7 +856,7 @@ void power9_idle_type(unsigned long stop_psscr_val,
psscr = (psscr & ~stop_psscr_mask) | stop_psscr_val;
__ppc64_runlatch_off();
- srr1 = power9_idle_stop(psscr, true);
+ srr1 = stop_dep.idle_stop(psscr, true);
__ppc64_runlatch_on();
fini_irq_for_idle_irqsoff();
@@ -1353,8 +1353,17 @@ static int __init pnv_init_idle_states(void)
nr_pnv_idle_states = 0;
supported_cpuidle_states = 0;
- if (cpuidle_disable != IDLE_NO_OVERRIDE)
+ if (cpuidle_disable != IDLE_NO_OVERRIDE ||
+ !(stop_dep.cpuidle_prop & STOP_ENABLE))
goto out;
+ switch(stop_dep.stop_version) {
+ case STOP_VERSION_P9:
+ stop_dep.idle_stop = power9_idle_stop;
+ break;
+ default:
+ stop_dep.idle_stop = NULL;
+ goto out;
+ }
rc = pnv_parse_cpuidle_dt();
if (rc)
return rc;
diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
index 1b299e801f74..7430a8edf5c9 100644
--- a/drivers/cpuidle/cpuidle-powernv.c
+++ b/drivers/cpuidle/cpuidle-powernv.c
@@ -371,7 +371,8 @@ static int powernv_add_idle_states(void)
*/
static int powernv_idle_probe(void)
{
- if (cpuidle_disable != IDLE_NO_OVERRIDE)
+ if (cpuidle_disable != IDLE_NO_OVERRIDE ||
+ !(stop_dep.cpuidle_prop & STOP_ENABLE))
return -ENODEV;
if (firmware_has_feature(FW_FEATURE_OPAL)) {
--
2.24.1
^ permalink raw reply related
* [RFC] Support stop state version quirk and firmware enabled stop
From: Pratik Rajesh Sampat @ 2020-03-04 15:56 UTC (permalink / raw)
To: skiboot, oohall, mikey, npiggin, vaidy, ego, linuxppc-dev,
linux-kernel, mpe, psampat, pratik.r.sampat
A concept patch in Skiboot to illustrate the case wherein handling of
stop states for different DD versions of a CPU can be achieved by a
simple modification in the list of cpu_features.
As an example idle-stop1 is defined which uses P9_CPU_DD1 to define the
cpu feature.
Along with that, an implementation is being worked upon the LE OPAL
series which helps OPAL handle the stop state entry and exit.
This patch advertises this capability of the firmware which can be
availed if the quirk-version-setting is not cognizable.
The firmware-enabled stop is being worked by Abhishek Goel
<huntbag@linux.vnet.ibm.com> building upon the LE OPAL series.
Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
---
core/cpufeatures.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/core/cpufeatures.c b/core/cpufeatures.c
index ec30c975..b9875e7b 100644
--- a/core/cpufeatures.c
+++ b/core/cpufeatures.c
@@ -510,6 +510,25 @@ static const struct cpu_feature cpu_features_table[] = {
-1, -1, -1,
NULL, },
+ /*
+ * QUIRK for ISAv3.0B stop idle instructions and registers
+ * Helps us determine if there are any quirks
+ * XXX: Same of idle-stop
+ */
+ { "idle-stop-v1",
+ CPU_P9_DD1,
+ ISA_V3_0B, USABLE_HV|USABLE_OS,
+ HV_CUSTOM, OS_CUSTOM,
+ -1, -1, -1,
+ NULL, },
+
+ { "firmware-stop-supported",
+ CPU_P9,
+ ISA_V3_0B, USABLE_HV|USABLE_OS,
+ HV_CUSTOM, OS_CUSTOM,
+ -1, -1, -1,
+ NULL, },
+
/*
* ISAv3.0B Hypervisor Virtualization Interrupt
* Also associated system registers, LPCR EE, HEIC, HVICE,
@@ -883,6 +902,9 @@ static void add_cpufeatures(struct dt_node *cpus,
const struct cpu_feature *f = &cpu_features_table[i];
if (f->cpus_supported & cpu_feature_cpu) {
+ if (!strcmp(f->name, "firmware-stop-supported") &&
+ HAVE_BIG_ENDIAN)
+ continue;
DBG(" '%s'\n", f->name);
add_cpu_feature_nodeps(features, f);
}
--
2.24.1
^ permalink raw reply related
* RE: [RFC PATCH v1] powerpc/prom_init: disable XIVE in Secure VM.
From: Cédric Le Goater @ 2020-03-04 15:56 UTC (permalink / raw)
To: Ram Pai, Greg Kurz
Cc: aik, andmike, kvm-ppc, sukadev, linuxppc-dev, bauerman,
David Gibson
In-Reply-To: <20200304153727.GH5416@oc0525413822.ibm.com>
[ ... ]
> (1) applied the patch which shares the EQ-page with the hypervisor.
> (2) set "kernel_irqchip=off"
> (3) set "ic-mode=xive"
you don't have to set the interrupt mode. xive should be negotiated
by default.
> (4) set "svm=on" on the kernel command line.
> (5) no changes to the hypervisor and ultravisor.
>
> And Boom it works!. So you were right.
Excellent.
> I am sending out the patch for (1) above ASAP.
Next step, could you please try to do the same with the TIMA and ESB pfn ?
and use KVM.
Thanks,
C.
^ permalink raw reply
* Re: [PATCH v3 24/27] powerpc/powernv/pmem: Expose SMART data via ndctl
From: Frederic Barrat @ 2020-03-04 15:40 UTC (permalink / raw)
To: Alastair D'Silva, alastair
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Andrew Donnellan, Arnd Bergmann, Greg Kurz,
Nicholas Piggin, Cédric Le Goater, Dan Williams,
Hari Bathini, linux-mm, Greg Kroah-Hartman, linux-kernel,
Vishal Verma, Paul Mackerras, Andrew Morton, linuxppc-dev,
David S. Miller
In-Reply-To: <20200221032720.33893-25-alastair@au1.ibm.com>
Le 21/02/2020 à 04:27, Alastair D'Silva a écrit :
> From: Alastair D'Silva <alastair@d-silva.org>
>
> This patch retrieves proprietary formatted SMART data and makes it
> available via ndctl. A later contribution will be made to ndctl to
> parse this data.
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
Nothing new to add compared to previous patches with similarities.
Fred
> arch/powerpc/platforms/powernv/pmem/ocxl.c | 128 ++++++++++++++++++
> .../platforms/powernv/pmem/ocxl_internal.h | 18 +++
> include/uapi/linux/ndctl.h | 1 +
> 3 files changed, 147 insertions(+)
>
> diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl.c b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> index d4ce5e9e0521..5cd1b6d78dd6 100644
> --- a/arch/powerpc/platforms/powernv/pmem/ocxl.c
> +++ b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> @@ -81,6 +81,129 @@ static int ndctl_config_size(struct nd_cmd_get_config_size *command)
> return 0;
> }
>
> +/**
> + * smart_header_parse() - Parse the first 64 bits of the SMART admin command response
> + * @ocxlpmem: the device metadata
> + * @length: out, returns the number of bytes in the response (excluding the 64 bit header)
> + */
> +static int smart_header_parse(struct ocxlpmem *ocxlpmem, u32 *length)
> +{
> + int rc;
> + u64 val;
> +
> + u16 data_identifier;
> + u32 data_length;
> +
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.data_offset,
> + OCXL_LITTLE_ENDIAN, &val);
> + if (rc)
> + return rc;
> +
> + data_identifier = val >> 48;
> + data_length = val & 0xFFFFFFFF;
> +
> + if (data_identifier != 0x534D) { // 'SM'
> + dev_err(&ocxlpmem->dev,
> + "Bad data identifier for smart data, expected 'SM', got '%-.*s'\n",
> + 2, (char *)&data_identifier);
> + return -EINVAL;
> + }
> +
> + *length = data_length;
> + return 0;
> +}
> +
> +static int ndctl_smart(struct ocxlpmem *ocxlpmem, struct nd_cmd_pkg *pkg)
> +{
> + u32 length, i;
> + struct nd_ocxl_smart *out;
> + int rc;
> +
> + mutex_lock(&ocxlpmem->admin_command.lock);
> +
> + rc = admin_command_request(ocxlpmem, ADMIN_COMMAND_SMART);
> + if (rc)
> + goto out;
> +
> + rc = admin_command_execute(ocxlpmem);
> + if (rc)
> + goto out;
> +
> + rc = admin_command_complete_timeout(ocxlpmem, ADMIN_COMMAND_SMART);
> + if (rc < 0) {
> + dev_err(&ocxlpmem->dev, "SMART timeout\n");
> + goto out;
> + }
> +
> + rc = admin_response(ocxlpmem);
> + if (rc < 0)
> + goto out;
> + if (rc != STATUS_SUCCESS) {
> + warn_status(ocxlpmem, "Unexpected status from SMART", rc);
> + goto out;
> + }
> +
> + rc = smart_header_parse(ocxlpmem, &length);
> + if (rc)
> + goto out;
> +
> + pkg->nd_fw_size = length;
> +
> + length = min(length, pkg->nd_size_out); // bytes
> + out = (struct nd_ocxl_smart *)pkg->nd_payload;
> + // Each SMART attribute is 2 * 64 bits
> + out->count = length / (2 * sizeof(u64)); // attributes
> +
> + for (i = 0; i < length; i += sizeof(u64)) {
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.data_offset + sizeof(u64) + i,
> + OCXL_LITTLE_ENDIAN,
> + &out->attribs[i/sizeof(u64)]);
> + if (rc)
> + goto out;
> + }
> +
> + rc = admin_response_handled(ocxlpmem);
> + if (rc)
> + goto out;
> +
> + rc = 0;
> + goto out;
> +
> +out:
> + mutex_unlock(&ocxlpmem->admin_command.lock);
> + return rc;
> +}
> +
> +static int ndctl_call(struct ocxlpmem *ocxlpmem, void *buf, unsigned int buf_len)
> +{
> + struct nd_cmd_pkg *pkg = buf;
> +
> + if (buf_len < sizeof(struct nd_cmd_pkg)) {
> + dev_err(&ocxlpmem->dev, "Invalid ND_CALL size=%u\n", buf_len);
> + return -EINVAL;
> + }
> +
> + if (pkg->nd_family != NVDIMM_FAMILY_OCXL) {
> + dev_err(&ocxlpmem->dev, "Invalid ND_CALL family=0x%llx\n", pkg->nd_family);
> + return -EINVAL;
> + }
> +
> + switch (pkg->nd_command) {
> + case ND_CMD_OCXL_SMART:
> + ndctl_smart(ocxlpmem, pkg);
> + break;
> +
> + default:
> + dev_err(&ocxlpmem->dev, "Invalid ND_CALL command=0x%llx\n", pkg->nd_command);
> + return -EINVAL;
> + }
> +
> +
> + return 0;
> +}
> +
> static int ndctl(struct nvdimm_bus_descriptor *nd_desc,
> struct nvdimm *nvdimm,
> unsigned int cmd, void *buf, unsigned int buf_len, int *cmd_rc)
> @@ -88,6 +211,10 @@ static int ndctl(struct nvdimm_bus_descriptor *nd_desc,
> struct ocxlpmem *ocxlpmem = container_of(nd_desc, struct ocxlpmem, bus_desc);
>
> switch (cmd) {
> + case ND_CMD_CALL:
> + *cmd_rc = ndctl_call(ocxlpmem, buf, buf_len);
> + return 0;
> +
> case ND_CMD_GET_CONFIG_SIZE:
> *cmd_rc = ndctl_config_size(buf);
> return 0;
> @@ -171,6 +298,7 @@ static int register_lpc_mem(struct ocxlpmem *ocxlpmem)
> set_bit(ND_CMD_GET_CONFIG_SIZE, &nvdimm_cmd_mask);
> set_bit(ND_CMD_GET_CONFIG_DATA, &nvdimm_cmd_mask);
> set_bit(ND_CMD_SET_CONFIG_DATA, &nvdimm_cmd_mask);
> + set_bit(ND_CMD_CALL, &nvdimm_cmd_mask);
>
> set_bit(NDD_ALIASING, &nvdimm_flags);
>
> diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> index 927690f4888f..0eb7a35d24ae 100644
> --- a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> +++ b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> @@ -7,6 +7,7 @@
> #include <linux/libnvdimm.h>
> #include <uapi/nvdimm/ocxl-pmem.h>
> #include <linux/mm.h>
> +#include <linux/ndctl.h>
>
> #define LABEL_AREA_SIZE (1UL << PA_SECTION_SHIFT)
> #define DEFAULT_TIMEOUT 100
> @@ -98,6 +99,23 @@ struct ocxlpmem_function0 {
> struct ocxl_fn *ocxl_fn;
> };
>
> +struct nd_ocxl_smart {
> + __u8 count;
> + __u8 reserved[7];
> + __u64 attribs[0];
> +} __packed;
> +
> +struct nd_pkg_ocxl {
> + struct nd_cmd_pkg gen;
> + union {
> + struct nd_ocxl_smart smart;
> + };
> +};
> +
> +enum nd_cmd_ocxl {
> + ND_CMD_OCXL_SMART = 1,
> +};
> +
> struct ocxlpmem {
> struct device dev;
> struct pci_dev *pdev;
> diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h
> index de5d90212409..2885052e7f40 100644
> --- a/include/uapi/linux/ndctl.h
> +++ b/include/uapi/linux/ndctl.h
> @@ -244,6 +244,7 @@ struct nd_cmd_pkg {
> #define NVDIMM_FAMILY_HPE2 2
> #define NVDIMM_FAMILY_MSFT 3
> #define NVDIMM_FAMILY_HYPERV 4
> +#define NVDIMM_FAMILY_OCXL 6
>
> #define ND_IOCTL_CALL _IOWR(ND_IOCTL, ND_CMD_CALL,\
> struct nd_cmd_pkg)
>
^ permalink raw reply
* RE: [RFC PATCH v1] powerpc/prom_init: disable XIVE in Secure VM.
From: Ram Pai @ 2020-03-04 15:37 UTC (permalink / raw)
To: Greg Kurz
Cc: aik, andmike, kvm-ppc, Cédric Le Goater, sukadev,
linuxppc-dev, bauerman, David Gibson
In-Reply-To: <20200304115948.7b2dfe10@bahia.home>
On Wed, Mar 04, 2020 at 11:59:48AM +0100, Greg Kurz wrote:
> On Tue, 3 Mar 2020 10:56:45 -0800
> Ram Pai <linuxram@us.ibm.com> wrote:
>
> > On Tue, Mar 03, 2020 at 06:45:20PM +0100, Greg Kurz wrote:
....snip.
> > >
> > > This patch would allow at least to answer Cedric's question about
> > > kernel_irqchip=off, since this looks like the only thing needed
> > > to make it work.
> >
> > hmm.. I am not sure. Are you saying
> > (a) patch the guest kernel to share the event queue page
> > (b) run the qemu with "kernel_irqchip=off"
> > (c) and the guest kernel with "svm=on"
> >
> > and it should all work?
> >
>
> Yes.
Ok.
(1) applied the patch which shares the EQ-page with the hypervisor.
(2) set "kernel_irqchip=off"
(3) set "ic-mode=xive"
(4) set "svm=on" on the kernel command line.
(5) no changes to the hypervisor and ultravisor.
And Boom it works!. So you were right.
I am sending out the patch for (1) above ASAP.
Thanks,
RP
^ permalink raw reply
* Re: [PATCH v2] ima: add a new CONFIG for loading arch-specific policies
From: James Bottomley @ 2020-03-04 15:35 UTC (permalink / raw)
To: Mimi Zohar, Nayna Jain, linux-integrity, linuxppc-dev, linux-efi,
linux-s390
Cc: Philipp Rudo, Ard Biesheuvel, linux-kernel
In-Reply-To: <1583325309.6264.23.camel@linux.ibm.com>
On Wed, 2020-03-04 at 07:35 -0500, Mimi Zohar wrote:
> On Tue, 2020-03-03 at 23:43 -0800, James Bottomley wrote:
> > On Tue, 2020-03-03 at 21:33 -0500, Nayna Jain wrote:
> > > diff --git a/security/integrity/ima/Kconfig
> > > b/security/integrity/ima/Kconfig
> > > index 3f3ee4e2eb0d..d17972aa413a 100644
> > > --- a/security/integrity/ima/Kconfig
> > > +++ b/security/integrity/ima/Kconfig
> > > @@ -327,3 +327,12 @@ config IMA_QUEUE_EARLY_BOOT_KEYS
> > > depends on IMA_MEASURE_ASYMMETRIC_KEYS
> > > depends on SYSTEM_TRUSTED_KEYRING
> > > default y
> > > +
> > > +config IMA_SECURE_AND_OR_TRUSTED_BOOT
> > > + bool
> > > + depends on IMA
> > > + depends on IMA_ARCH_POLICY
> > > + default n
> >
> > You can't do this: a symbol designed to be selected can't depend on
> > other symbols because Kconfig doesn't see the dependencies during
> > select. We even have a doc for this now:
> >
> > Documentation/kbuild/Kconfig.select-break
>
> The document is discussing a circular dependency, where C selects B.
> IMA_SECURE_AND_OR_TRUSTED_BOOT is not selecting anything, but is
> being selected. All of the Kconfig's are now dependent on
> IMA_ARCH_POLICY being enabled before selecting
> IMA_SECURE_AND_OR_TRUSTED_BOOT.
>
> As Ard pointed out, both IMA and IMA_ARCH_POLICY are not needed, as
> IMA_ARCH_POLICY is already dependent on IMA.
Then removing them is fine, if they're not necessary ... you just can't
select a symbol with dependencies because the two Kconfig mechanisms
don't mix.
James
^ permalink raw reply
* Re: [PATCH v3 23/27] powerpc/powernv/pmem: Add debug IOCTLs
From: Frederic Barrat @ 2020-03-04 15:21 UTC (permalink / raw)
To: Alastair D'Silva, alastair
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Andrew Donnellan, Arnd Bergmann, Greg Kurz,
Nicholas Piggin, Cédric Le Goater, Dan Williams,
Hari Bathini, linux-mm, Greg Kroah-Hartman, linux-kernel,
Vishal Verma, Paul Mackerras, Andrew Morton, linuxppc-dev,
David S. Miller
In-Reply-To: <20200221032720.33893-24-alastair@au1.ibm.com>
Le 21/02/2020 à 04:27, Alastair D'Silva a écrit :
> From: Alastair D'Silva <alastair@d-silva.org>
>
> These IOCTLs provide low level access to the card to aid in debugging
> controller/FPGA firmware.
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
> arch/powerpc/platforms/powernv/pmem/Kconfig | 6 +
> arch/powerpc/platforms/powernv/pmem/ocxl.c | 249 ++++++++++++++++++++
> include/uapi/nvdimm/ocxl-pmem.h | 32 +++
> 3 files changed, 287 insertions(+)
>
> diff --git a/arch/powerpc/platforms/powernv/pmem/Kconfig b/arch/powerpc/platforms/powernv/pmem/Kconfig
> index c5d927520920..3f44429d70c9 100644
> --- a/arch/powerpc/platforms/powernv/pmem/Kconfig
> +++ b/arch/powerpc/platforms/powernv/pmem/Kconfig
> @@ -12,4 +12,10 @@ config OCXL_PMEM
>
> Select N if unsure.
>
> +config OCXL_PMEM_DEBUG
> + bool "OpenCAPI Persistent Memory debugging"
> + depends on OCXL_PMEM
> + help
> + Enables low level IOCTLs for OpenCAPI Persistent Memory firmware development
> +
> endif
> diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl.c b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> index e01f6f9fc180..d4ce5e9e0521 100644
> --- a/arch/powerpc/platforms/powernv/pmem/ocxl.c
> +++ b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> @@ -1050,6 +1050,235 @@ int req_controller_health_perf(struct ocxlpmem *ocxlpmem)
> GLOBAL_MMIO_HCI_REQ_HEALTH_PERF);
> }
>
> +#ifdef CONFIG_OCXL_PMEM_DEBUG
> +/**
> + * enable_fwdebug() - Enable FW debug on the controller
> + * @ocxlpmem: the device metadata
> + * Return: 0 on success, negative on failure
> + */
> +static int enable_fwdebug(const struct ocxlpmem *ocxlpmem)
> +{
> + return ocxl_global_mmio_set64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_HCI,
> + OCXL_LITTLE_ENDIAN,
> + GLOBAL_MMIO_HCI_FW_DEBUG);
> +}
> +
> +/**
> + * disable_fwdebug() - Disable FW debug on the controller
> + * @ocxlpmem: the device metadata
> + * Return: 0 on success, negative on failure
> + */
> +static int disable_fwdebug(const struct ocxlpmem *ocxlpmem)
> +{
> + return ocxl_global_mmio_set64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_HCIC,
> + OCXL_LITTLE_ENDIAN,
> + GLOBAL_MMIO_HCI_FW_DEBUG);
> +}
> +
> +static int ioctl_fwdebug(struct ocxlpmem *ocxlpmem,
> + struct ioctl_ocxl_pmem_fwdebug __user *uarg)
> +{
> + struct ioctl_ocxl_pmem_fwdebug args;
> + u64 val;
> + int i;
> + int rc;
> +
> + if (copy_from_user(&args, uarg, sizeof(args)))
> + return -EFAULT;
> +
> + // Buffer size must be a multiple of 8
> + if ((args.buf_size & 0x07))
> + return -EINVAL;
> +
> + if (args.buf_size > ocxlpmem->admin_command.data_size)
> + return -EINVAL;
> +
> + mutex_lock(&ocxlpmem->admin_command.lock);
> +
> + rc = enable_fwdebug(ocxlpmem);
> + if (rc)
> + goto out;
> +
> + rc = admin_command_request(ocxlpmem, ADMIN_COMMAND_FW_DEBUG);
> + if (rc)
> + goto out;
> +
> + // Write DebugAction & FunctionCode
> + val = ((u64)args.debug_action << 56) | ((u64)args.function_code << 40);
> +
> + rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.request_offset + 0x08,
> + OCXL_LITTLE_ENDIAN, val);
> + if (rc)
> + goto out;
> +
> + rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.request_offset + 0x10,
> + OCXL_LITTLE_ENDIAN, args.debug_parameter_1);
> + if (rc)
> + goto out;
> +
> + rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.request_offset + 0x18,
> + OCXL_LITTLE_ENDIAN, args.debug_parameter_2);
> + if (rc)
> + goto out;
> +
> + for (i = 0x20; i < 0x38; i += 0x08)
> + rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.request_offset + i,
> + OCXL_LITTLE_ENDIAN, 0);
> + if (rc)
> + goto out;
rc is the for loop body. The rc test is not.
> +
> +
> + // Populate admin command buffer
> + if (args.buf_size) {
> + for (i = 0; i < args.buf_size; i += sizeof(u64)) {
> + u64 val;
> +
> + if (copy_from_user(&val, &args.buf[i], sizeof(u64)))
> + return -EFAULT;
need to get rc and goto out because of the mutex
> +
> + rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.data_offset + i,
> + OCXL_HOST_ENDIAN, val);
> + if (rc)
> + goto out;
> + }
> + }
> +
> + rc = admin_command_execute(ocxlpmem);
> + if (rc)
> + goto out;
> +
> + rc = admin_command_complete_timeout(ocxlpmem,
> + ocxlpmem->timeouts[ADMIN_COMMAND_FW_DEBUG]);
> + if (rc < 0)
> + goto out;
> +
> + rc = admin_response(ocxlpmem);
> + if (rc < 0)
> + goto out;
> + if (rc != STATUS_SUCCESS) {
> + warn_status(ocxlpmem, "Unexpected status from FW Debug", rc);
> + goto out;
> + }
> +
> + if (args.buf_size) {
> + for (i = 0; i < args.buf_size; i += sizeof(u64)) {
> + u64 val;
> +
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.data_offset + i,
> + OCXL_HOST_ENDIAN, &val);
> + if (rc)
> + goto out;
> +
> + if (copy_to_user(&args.buf[i], &val, sizeof(u64))) {
> + rc = -EFAULT;
> + goto out;
> + }
> + }
> + }
> +
> + rc = admin_response_handled(ocxlpmem);
> + if (rc)
> + goto out;
> +
> + rc = disable_fwdebug(ocxlpmem);
> + if (rc)
> + goto out;
> +
> +out:
> + mutex_unlock(&ocxlpmem->admin_command.lock);
> + return rc;
> +}
> +
> +static int ioctl_shutdown(struct ocxlpmem *ocxlpmem)
> +{
> + int rc;
> +
> + mutex_lock(&ocxlpmem->admin_command.lock);
> +
> + rc = admin_command_request(ocxlpmem, ADMIN_COMMAND_SHUTDOWN);
> + if (rc)
> + goto out;
> +
> + rc = admin_command_execute(ocxlpmem);
> + if (rc)
> + goto out;
> +
> + rc = admin_command_complete_timeout(ocxlpmem, ADMIN_COMMAND_SHUTDOWN);
> + if (rc < 0) {
> + dev_warn(&ocxlpmem->dev, "Shutdown timed out\n");
> + goto out;
> + }
> +
> + rc = 0;
> + goto out;
We can remove that goto.
No admin_response_handled()? Is that shutting down the full adapter and
we have nobody to talk to? What happens next?
> +
> +out:
> + mutex_unlock(&ocxlpmem->admin_command.lock);
> + return rc;
> +}
> +
> +static int ioctl_mmio_write(struct ocxlpmem *ocxlpmem,
> + struct ioctl_ocxl_pmem_mmio __user *uarg)
> +{
> + struct scm_ioctl_mmio args;
> +
> + if (copy_from_user(&args, uarg, sizeof(args)))
> + return -EFAULT;
> +
> + return ocxl_global_mmio_write64(ocxlpmem->ocxl_afu, args.address,
> + OCXL_LITTLE_ENDIAN, args.val);
> +}
> +
> +static int ioctl_mmio_read(struct ocxlpmem *ocxlpmem,
> + struct ioctl_ocxl_pmem_mmio __user *uarg)
> +{
> + struct ioctl_ocxl_pmem_mmio args;
> + int rc;
> +
> + if (copy_from_user(&args, uarg, sizeof(args)))
> + return -EFAULT;
> +
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu, args.address,
> + OCXL_LITTLE_ENDIAN, &args.val);
> + if (rc)
> + return rc;
> +
> + if (copy_to_user(uarg, &args, sizeof(args)))
> + return -EFAULT;
> +
> + return 0;
> +}
> +#else /* CONFIG_OCXL_PMEM_DEBUG */
> +static int ioctl_fwdebug(struct ocxlpmem *ocxlpmem,
> + struct ioctl_ocxl_pmem_fwdebug __user *uarg)
> +{
> + return -EPERM;
> +}
> +
> +static int ioctl_shutdown(struct ocxlpmem *ocxlpmem)
> +{
> + return -EPERM;
> +}
> +
> +static int ioctl_mmio_write(struct ocxlpmem *ocxlpmem,
> + struct ioctl_ocxl_pmem_mmio __user *uarg)
> +{
> + return -EPERM;
> +}
> +
> +static int ioctl_mmio_read(struct ocxlpmem *ocxlpmem,
> + struct ioctl_ocxl_pmem_mmio __user *uarg)
> +{
> + return -EPERM;
> +}
The 'else' clause could be dropped, the ioctls will return EINVAL, which
is fine, I think.
> +#endif /* CONFIG_OCXL_PMEM_DEBUG */
> +
> static long file_ioctl(struct file *file, unsigned int cmd, unsigned long args)
> {
> struct ocxlpmem *ocxlpmem = file->private_data;
> @@ -1091,6 +1320,26 @@ static long file_ioctl(struct file *file, unsigned int cmd, unsigned long args)
> case IOCTL_OCXL_PMEM_REQUEST_HEALTH:
> rc = req_controller_health_perf(ocxlpmem);
> break;
> +
> + case IOCTL_OCXL_PMEM_FWDEBUG:
> + rc = ioctl_fwdebug(ocxlpmem,
> + (struct ioctl_ocxl_pmem_fwdebug __user *)args);
> + break;
> +
> + case IOCTL_OCXL_PMEM_SHUTDOWN:
> + rc = ioctl_shutdown(ocxlpmem);
> + break;
> +
> + case IOCTL_OCXL_PMEM_MMIO_WRITE:
> + rc = ioctl_mmio_write(ocxlpmem,
> + (struct ioctl_ocxl_pmem_mmio __user *)args);
> + break;
> +
> + case IOCTL_OCXL_PMEM_MMIO_READ:
> + rc = ioctl_mmio_read(ocxlpmem,
> + (struct ioctl_ocxl_pmem_mmio __user *)args);
> + break;
> +
> }
>
> return rc;
> diff --git a/include/uapi/nvdimm/ocxl-pmem.h b/include/uapi/nvdimm/ocxl-pmem.h
> index 0d03abb44001..e20a4f8be82a 100644
> --- a/include/uapi/nvdimm/ocxl-pmem.h
> +++ b/include/uapi/nvdimm/ocxl-pmem.h
> @@ -6,6 +6,28 @@
> #include <linux/types.h>
> #include <linux/ioctl.h>
>
> +enum ocxlpmem_fwdebug_action {
> + OCXL_PMEM_FWDEBUG_READ_CONTROLLER_MEMORY = 0x01,
> + OCXL_PMEM_FWDEBUG_WRITE_CONTROLLER_MEMORY = 0x02,
> + OCXL_PMEM_FWDEBUG_ENABLE_FUNCTION = 0x03,
> + OCXL_PMEM_FWDEBUG_DISABLE_FUNCTION = 0x04,
> + OCXL_PMEM_FWDEBUG_GET_PEL = 0x05, // Retrieve Persistent Error Log
> +};
> +
> +struct ioctl_ocxl_pmem_buffer_info {
> + __u32 admin_command_buffer_size; // out
> + __u32 near_storage_buffer_size; // out
> +};
> +
> +struct ioctl_ocxl_pmem_fwdebug { // All args are inputs
> + enum ocxlpmem_fwdebug_action debug_action;
More kernel ABI problems. My interpretation of the "enumeration
specifiers" section of C99 is that we can't rely on the size of the enum.
> + __u16 function_code;
> + __u16 buf_size; // Size of optional data buffer
> + __u64 debug_parameter_1;
> + __u64 debug_parameter_2;
> + __u8 *buf; // Pointer to optional in/out data buffer
> +};
> +
> #define OCXL_PMEM_ERROR_LOG_ACTION_RESET (1 << (32-32))
> #define OCXL_PMEM_ERROR_LOG_ACTION_CHKFW (1 << (53-32))
> #define OCXL_PMEM_ERROR_LOG_ACTION_REPLACE (1 << (54-32))
> @@ -66,6 +88,11 @@ struct ioctl_ocxl_pmem_controller_stats {
> __u64 cache_write_latency; /* nanoseconds */
> };
>
> +struct ioctl_ocxl_pmem_mmio {
> + __u64 address; /* Offset in global MMIO space */
> + __u64 val; /* value to write/was read */
> +};
Can we group all the debug data structures together in the header file,
with a comment indicating that they may not be available in the kernel,
depending on the config?
Fred
> +
> struct ioctl_ocxl_pmem_eventfd {
> __s32 eventfd;
> __u32 reserved;
> @@ -92,4 +119,9 @@ struct ioctl_ocxl_pmem_eventfd {
> #define IOCTL_OCXL_PMEM_EVENT_CHECK _IOR(OCXL_PMEM_MAGIC, 0x07, __u64)
> #define IOCTL_OCXL_PMEM_REQUEST_HEALTH _IO(OCXL_PMEM_MAGIC, 0x08)
>
> +#define IOCTL_OCXL_PMEM_FWDEBUG _IOWR(OCXL_PMEM_MAGIC, 0xf0, struct ioctl_ocxl_pmem_fwdebug)
> +#define IOCTL_OCXL_PMEM_MMIO_WRITE _IOW(OCXL_PMEM_MAGIC, 0xf1, struct ioctl_ocxl_pmem_mmio)
> +#define IOCTL_OCXL_PMEM_MMIO_READ _IOWR(OCXL_PMEM_MAGIC, 0xf2, struct ioctl_ocxl_pmem_mmio)
> +#define IOCTL_OCXL_PMEM_SHUTDOWN _IO(OCXL_PMEM_MAGIC, 0xf3)
> +
> #endif /* _UAPI_OCXL_SCM_H */
>
^ permalink raw reply
* RE: [RFC PATCH v1] powerpc/prom_init: disable XIVE in Secure VM.
From: Ram Pai @ 2020-03-04 15:13 UTC (permalink / raw)
To: Greg Kurz
Cc: aik, andmike, kvm-ppc, Cédric Le Goater, sukadev,
linuxppc-dev, bauerman, David Gibson
In-Reply-To: <20200304115948.7b2dfe10@bahia.home>
On Wed, Mar 04, 2020 at 11:59:48AM +0100, Greg Kurz wrote:
> On Tue, 3 Mar 2020 10:56:45 -0800
> Ram Pai <linuxram@us.ibm.com> wrote:
>
> > On Tue, Mar 03, 2020 at 06:45:20PM +0100, Greg Kurz wrote:
> > > On Tue, 3 Mar 2020 09:02:05 -0800
> > > Ram Pai <linuxram@us.ibm.com> wrote:
> > >
> > > > On Tue, Mar 03, 2020 at 07:50:08AM +0100, Cédric Le Goater wrote:
> > > > > On 3/3/20 12:32 AM, David Gibson wrote:
> > > > > > On Fri, Feb 28, 2020 at 11:54:04PM -0800, Ram Pai wrote:
> > > > > >> XIVE is not correctly enabled for Secure VM in the KVM Hypervisor yet.
> > > > > >>
> > > > > >> Hence Secure VM, must always default to XICS interrupt controller.
> > > > > >>
> > > > > >> If XIVE is requested through kernel command line option "xive=on",
> > > > > >> override and turn it off.
> > > > > >>
> > > > > >> If XIVE is the only supported platform interrupt controller; specified
> > > > > >> through qemu option "ic-mode=xive", simply abort. Otherwise default to
> > > > > >> XICS.
> > > > > >
> > > > > > Uh... the discussion thread here seems to have gotten oddly off
> > > > > > track.
> > > > >
> > > > > There seem to be multiple issues. It is difficult to have a clear status.
> > > > >
> > > > > > So, to try to clean up some misunderstandings on both sides:
> > > > > >
> > > > > > 1) The guest is the main thing that knows that it will be in secure
> > > > > > mode, so it's reasonable for it to conditionally use XIVE based
> > > > > > on that
> > > > >
> > > > > FW support is required AFAIUI.
> > > > > > 2) The mechanism by which we do it here isn't quite right. Here the
> > > > > > guest is checking itself that the host only allows XIVE, but we
> > > > > > can't do XIVE and is panic()ing. Instead, in the SVM case we
> > > > > > should force support->xive to false, and send that in the CAS
> > > > > > request to the host. We expect the host to just terminate
> > > > > > us because of the mismatch, but this will interact better with
> > > > > > host side options setting policy for panic states and the like.
> > > > > > Essentially an SVM kernel should behave like an old kernel with
> > > > > > no XIVE support at all, at least w.r.t. the CAS irq mode flags.
> > > > >
> > > > > Yes. XIVE shouldn't be requested by the guest.
> > > >
> > > > Ok.
> > > >
> > > > > This is the last option
> > > > > I proposed but I thought there was some negotiation with the hypervisor
> > > > > which is not the case.
> > > > >
> > > > > > 3) Although there are means by which the hypervisor can kind of know
> > > > > > a guest is in secure mode, there's not really an "svm=on" option
> > > > > > on the host side. For the most part secure mode is based on
> > > > > > discussion directly between the guest and the ultravisor with
> > > > > > almost no hypervisor intervention.
> > > > >
> > > > > Is there a negotiation with the ultravisor ?
> > > >
> > > > The VM has no negotiation with the ultravisor w.r.t CAS.
> > > >
> > > > >
> > > > > > 4) I'm guessing the problem with XIVE in SVM mode is that XIVE needs
> > > > > > to write to event queues in guest memory, which would have to be
> > > > > > explicitly shared for secure mode. That's true whether it's KVM
> > > > > > or qemu accessing the guest memory, so kernel_irqchip=on/off is
> > > > > > entirely irrelevant.
> > > > >
> > > > > This problem should be already fixed.
> > > > > The XIVE event queues are shared
> > > >
> > > > Yes i have a patch for the guest kernel that shares the event
> > > > queue page with the hypervisor. This is done using the
> > > > UV_SHARE_PAGE ultracall. This patch is not sent out to any any mailing
> > > > lists yet.
> > >
> > > Why ?
> >
> > At this point I am not sure if this is the only change, I need to the
> > guest kernel.
>
> Maybe but we're already sure that this change is needed. I don't really see
> the point in holding this any longer.
>
> > I also need changes to KVM and to the ultravisor. Its bit
> > premature to send the patch without having figured out everything
> > to get xive working on a Secure VM.
> >
>
> I'm a bit confused... why did you send this workaround patch in
> the first place then ? I mean, this raises a concern and we're
> just trying to move forward.
The upstream kernel in its current form, will hang as of today if qemu
has 'ic-mode=xive'. The kernel hangs without giving any indication as to
'why?'. This is a bad state to be in.
This patch was a temporary solution. It is there to inform the user, not
to use 'xive' in secureVM. The user is atleast informed/guided, instead
of lost/confused.
The permanent solution, is to fix the problem in KVM and ultravisor,
along with sharing the EQ-page in the SVM, and get a holistic solution
in place. But that will take time, and may not happen by the time 5.6
releases.
RP
^ permalink raw reply
* Re: [PATCH v3 22/27] powerpc/powernv/pmem: Implement the heartbeat command
From: Frederic Barrat @ 2020-03-04 14:25 UTC (permalink / raw)
To: Alastair D'Silva, alastair
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Andrew Donnellan, Arnd Bergmann, Greg Kurz,
Nicholas Piggin, Cédric Le Goater, Dan Williams,
Hari Bathini, linux-mm, Greg Kroah-Hartman, linux-kernel,
Vishal Verma, Paul Mackerras, Andrew Morton, linuxppc-dev,
David S. Miller
In-Reply-To: <20200221032720.33893-23-alastair@au1.ibm.com>
Le 21/02/2020 à 04:27, Alastair D'Silva a écrit :
> From: Alastair D'Silva <alastair@d-silva.org>
>
> The heartbeat admin command is a simple admin command that exercises
> the communication mechanisms within the controller.
>
> This patch issues a heartbeat command to the card during init to ensure
> we can communicate with the card's controller.
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
Nothing to add compared to what has already been commented on previous
patches (rc not set in probe(), higher level function to execute admin
command in one call).
Fred
> arch/powerpc/platforms/powernv/pmem/ocxl.c | 43 ++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
>
> diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl.c b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> index 081883a8247a..e01f6f9fc180 100644
> --- a/arch/powerpc/platforms/powernv/pmem/ocxl.c
> +++ b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> @@ -306,6 +306,44 @@ static bool is_usable(const struct ocxlpmem *ocxlpmem, bool verbose)
> return true;
> }
>
> +/**
> + * heartbeat() - Issue a heartbeat command to the controller
> + * @ocxlpmem: the device metadata
> + * Return: 0 if the controller responded correctly, negative on error
> + */
> +static int heartbeat(struct ocxlpmem *ocxlpmem)
> +{
> + int rc;
> +
> + mutex_lock(&ocxlpmem->admin_command.lock);
> +
> + rc = admin_command_request(ocxlpmem, ADMIN_COMMAND_HEARTBEAT);
> + if (rc)
> + goto out;
> +
> + rc = admin_command_execute(ocxlpmem);
> + if (rc)
> + goto out;
> +
> + rc = admin_command_complete_timeout(ocxlpmem, ADMIN_COMMAND_HEARTBEAT);
> + if (rc < 0) {
> + dev_err(&ocxlpmem->dev, "Heartbeat timeout\n");
> + goto out;
> + }
> +
> + rc = admin_response(ocxlpmem);
> + if (rc < 0)
> + goto out;
> + if (rc != STATUS_SUCCESS)
> + warn_status(ocxlpmem, "Unexpected status from heartbeat", rc);
> +
> + (void)admin_response_handled(ocxlpmem);
> +
> +out:
> + mutex_unlock(&ocxlpmem->admin_command.lock);
> + return rc;
> +}
> +
> /**
> * allocate_minor() - Allocate a minor number to use for an OpenCAPI pmem device
> * @ocxlpmem: the device metadata
> @@ -1458,6 +1496,11 @@ static int probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> goto err;
> }
>
> + if (heartbeat(ocxlpmem)) {
> + dev_err(&pdev->dev, "Heartbeat failed\n");
> + goto err;
> + }
> +
> elapsed = 0;
> timeout = ocxlpmem->readiness_timeout + ocxlpmem->memory_available_timeout;
> while (!is_usable(ocxlpmem, false)) {
>
^ permalink raw reply
* Re: [PATCH v2] ima: add a new CONFIG for loading arch-specific policies
From: Philipp Rudo @ 2020-03-04 13:25 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-s390, linux-efi, Nayna Jain, Linux Kernel Mailing List,
Ard Biesheuvel, linux-integrity, linuxppc-dev, Thomas Gleixner,
x86
In-Reply-To: <1583326538.6264.32.camel@linux.ibm.com>
On Wed, 04 Mar 2020 07:55:38 -0500
Mimi Zohar <zohar@linux.ibm.com> wrote:
> [Cc'ing Thomas Gleixner and x86 mailing list]
>
> On Wed, 2020-03-04 at 08:14 +0100, Ard Biesheuvel wrote:
> > On Wed, 4 Mar 2020 at 03:34, Nayna Jain <nayna@linux.ibm.com> wrote:
> > >
> > > Every time a new architecture defines the IMA architecture specific
> > > functions - arch_ima_get_secureboot() and arch_ima_get_policy(), the IMA
> > > include file needs to be updated. To avoid this "noise", this patch
> > > defines a new IMA Kconfig IMA_SECURE_AND_OR_TRUSTED_BOOT option, allowing
> > > the different architectures to select it.
> > >
> > > Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> > > Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> > > Cc: Ard Biesheuvel <ardb@kernel.org>
> > > Cc: Philipp Rudo <prudo@linux.ibm.com>
> > > Cc: Michael Ellerman <mpe@ellerman.id.au>
> >
> > Acked-by: Ard Biesheuvel <ardb@kernel.org>
>
> Thanks, Ard.
> >
> > for the x86 bits, but I'm not an x86 maintainer. Also, you may need to
> > split this if you want to permit arch maintainers to pick up their
> > parts individually.
>
> Michael, Philipp, Thomas, do you prefer separate patches?
I don't think splitting this patch makes sense. Otherwise you would break the
build for all architectures until they picked up their line of code.
I'm fine with the patch as is.
Thanks
Philipp
> >
> > > ---
> > > v2:
> > > * Fixed the issue identified by Mimi. Thanks Mimi, Ard, Heiko and Michael for
> > > discussing the fix.
> > >
> > > arch/powerpc/Kconfig | 1 +
> > > arch/s390/Kconfig | 1 +
> > > arch/x86/Kconfig | 1 +
> > > include/linux/ima.h | 3 +--
> > > security/integrity/ima/Kconfig | 9 +++++++++
> > > 5 files changed, 13 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > > index 497b7d0b2d7e..a5cfde432983 100644
> > > --- a/arch/powerpc/Kconfig
> > > +++ b/arch/powerpc/Kconfig
> > > @@ -979,6 +979,7 @@ config PPC_SECURE_BOOT
> > > bool
> > > depends on PPC_POWERNV
> > > depends on IMA_ARCH_POLICY
> > > + select IMA_SECURE_AND_OR_TRUSTED_BOOT
> > > help
> > > Systems with firmware secure boot enabled need to define security
> > > policies to extend secure boot to the OS. This config allows a user
> > > diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> > > index 8abe77536d9d..4a502fbcb800 100644
> > > --- a/arch/s390/Kconfig
> > > +++ b/arch/s390/Kconfig
> > > @@ -195,6 +195,7 @@ config S390
> > > select ARCH_HAS_FORCE_DMA_UNENCRYPTED
> > > select SWIOTLB
> > > select GENERIC_ALLOCATOR
> > > + select IMA_SECURE_AND_OR_TRUSTED_BOOT if IMA_ARCH_POLICY
> > >
> > >
> > > config SCHED_OMIT_FRAME_POINTER
> > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > > index beea77046f9b..7f5bfaf0cbd2 100644
> > > --- a/arch/x86/Kconfig
> > > +++ b/arch/x86/Kconfig
> > > @@ -230,6 +230,7 @@ config X86
> > > select VIRT_TO_BUS
> > > select X86_FEATURE_NAMES if PROC_FS
> > > select PROC_PID_ARCH_STATUS if PROC_FS
> > > + select IMA_SECURE_AND_OR_TRUSTED_BOOT if EFI && IMA_ARCH_POLICY
> > >
> > > config INSTRUCTION_DECODER
> > > def_bool y
> > > diff --git a/include/linux/ima.h b/include/linux/ima.h
> > > index 1659217e9b60..aefe758f4466 100644
> > > --- a/include/linux/ima.h
> > > +++ b/include/linux/ima.h
> > > @@ -30,8 +30,7 @@ extern void ima_kexec_cmdline(const void *buf, int size);
> > > extern void ima_add_kexec_buffer(struct kimage *image);
> > > #endif
> > >
> > > -#if (defined(CONFIG_X86) && defined(CONFIG_EFI)) || defined(CONFIG_S390) \
> > > - || defined(CONFIG_PPC_SECURE_BOOT)
> > > +#ifdef CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT
> > > extern bool arch_ima_get_secureboot(void);
> > > extern const char * const *arch_get_ima_policy(void);
> > > #else
> > > diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> > > index 3f3ee4e2eb0d..d17972aa413a 100644
> > > --- a/security/integrity/ima/Kconfig
> > > +++ b/security/integrity/ima/Kconfig
> > > @@ -327,3 +327,12 @@ config IMA_QUEUE_EARLY_BOOT_KEYS
> > > depends on IMA_MEASURE_ASYMMETRIC_KEYS
> > > depends on SYSTEM_TRUSTED_KEYRING
> > > default y
> > > +
> > > +config IMA_SECURE_AND_OR_TRUSTED_BOOT
> > > + bool
> > > + depends on IMA
> > > + depends on IMA_ARCH_POLICY
> >
> > Doesn't the latter already depend on the former?
>
> Yes, there's no need for the first.
>
> Mimi
> >
> > > + default n
> > > + help
> > > + This option is selected by architectures to enable secure and/or
> > > + trusted boot based on IMA runtime policies.
> > > --
> > > 2.13.6
> > >
>
^ permalink raw reply
* Re: [PATCH v2] ima: add a new CONFIG for loading arch-specific policies
From: Mimi Zohar @ 2020-03-04 12:55 UTC (permalink / raw)
To: Ard Biesheuvel, Nayna Jain, Thomas Gleixner
Cc: linux-s390, linux-efi, x86, Linux Kernel Mailing List,
Philipp Rudo, linux-integrity, linuxppc-dev
In-Reply-To: <CAKv+Gu831SRo+Di6WgKTex40TcOVqNQAdeNLtfQpPdgnvrxucw@mail.gmail.com>
[Cc'ing Thomas Gleixner and x86 mailing list]
On Wed, 2020-03-04 at 08:14 +0100, Ard Biesheuvel wrote:
> On Wed, 4 Mar 2020 at 03:34, Nayna Jain <nayna@linux.ibm.com> wrote:
> >
> > Every time a new architecture defines the IMA architecture specific
> > functions - arch_ima_get_secureboot() and arch_ima_get_policy(), the IMA
> > include file needs to be updated. To avoid this "noise", this patch
> > defines a new IMA Kconfig IMA_SECURE_AND_OR_TRUSTED_BOOT option, allowing
> > the different architectures to select it.
> >
> > Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> > Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> > Cc: Ard Biesheuvel <ardb@kernel.org>
> > Cc: Philipp Rudo <prudo@linux.ibm.com>
> > Cc: Michael Ellerman <mpe@ellerman.id.au>
>
> Acked-by: Ard Biesheuvel <ardb@kernel.org>
Thanks, Ard.
>
> for the x86 bits, but I'm not an x86 maintainer. Also, you may need to
> split this if you want to permit arch maintainers to pick up their
> parts individually.
Michael, Philipp, Thomas, do you prefer separate patches?
>
> > ---
> > v2:
> > * Fixed the issue identified by Mimi. Thanks Mimi, Ard, Heiko and Michael for
> > discussing the fix.
> >
> > arch/powerpc/Kconfig | 1 +
> > arch/s390/Kconfig | 1 +
> > arch/x86/Kconfig | 1 +
> > include/linux/ima.h | 3 +--
> > security/integrity/ima/Kconfig | 9 +++++++++
> > 5 files changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > index 497b7d0b2d7e..a5cfde432983 100644
> > --- a/arch/powerpc/Kconfig
> > +++ b/arch/powerpc/Kconfig
> > @@ -979,6 +979,7 @@ config PPC_SECURE_BOOT
> > bool
> > depends on PPC_POWERNV
> > depends on IMA_ARCH_POLICY
> > + select IMA_SECURE_AND_OR_TRUSTED_BOOT
> > help
> > Systems with firmware secure boot enabled need to define security
> > policies to extend secure boot to the OS. This config allows a user
> > diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> > index 8abe77536d9d..4a502fbcb800 100644
> > --- a/arch/s390/Kconfig
> > +++ b/arch/s390/Kconfig
> > @@ -195,6 +195,7 @@ config S390
> > select ARCH_HAS_FORCE_DMA_UNENCRYPTED
> > select SWIOTLB
> > select GENERIC_ALLOCATOR
> > + select IMA_SECURE_AND_OR_TRUSTED_BOOT if IMA_ARCH_POLICY
> >
> >
> > config SCHED_OMIT_FRAME_POINTER
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index beea77046f9b..7f5bfaf0cbd2 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -230,6 +230,7 @@ config X86
> > select VIRT_TO_BUS
> > select X86_FEATURE_NAMES if PROC_FS
> > select PROC_PID_ARCH_STATUS if PROC_FS
> > + select IMA_SECURE_AND_OR_TRUSTED_BOOT if EFI && IMA_ARCH_POLICY
> >
> > config INSTRUCTION_DECODER
> > def_bool y
> > diff --git a/include/linux/ima.h b/include/linux/ima.h
> > index 1659217e9b60..aefe758f4466 100644
> > --- a/include/linux/ima.h
> > +++ b/include/linux/ima.h
> > @@ -30,8 +30,7 @@ extern void ima_kexec_cmdline(const void *buf, int size);
> > extern void ima_add_kexec_buffer(struct kimage *image);
> > #endif
> >
> > -#if (defined(CONFIG_X86) && defined(CONFIG_EFI)) || defined(CONFIG_S390) \
> > - || defined(CONFIG_PPC_SECURE_BOOT)
> > +#ifdef CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT
> > extern bool arch_ima_get_secureboot(void);
> > extern const char * const *arch_get_ima_policy(void);
> > #else
> > diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> > index 3f3ee4e2eb0d..d17972aa413a 100644
> > --- a/security/integrity/ima/Kconfig
> > +++ b/security/integrity/ima/Kconfig
> > @@ -327,3 +327,12 @@ config IMA_QUEUE_EARLY_BOOT_KEYS
> > depends on IMA_MEASURE_ASYMMETRIC_KEYS
> > depends on SYSTEM_TRUSTED_KEYRING
> > default y
> > +
> > +config IMA_SECURE_AND_OR_TRUSTED_BOOT
> > + bool
> > + depends on IMA
> > + depends on IMA_ARCH_POLICY
>
> Doesn't the latter already depend on the former?
Yes, there's no need for the first.
Mimi
>
> > + default n
> > + help
> > + This option is selected by architectures to enable secure and/or
> > + trusted boot based on IMA runtime policies.
> > --
> > 2.13.6
> >
^ permalink raw reply
* Re: [PATCH v2] ima: add a new CONFIG for loading arch-specific policies
From: James Bottomley @ 2020-03-04 7:43 UTC (permalink / raw)
To: Nayna Jain, linux-integrity, linuxppc-dev, linux-efi, linux-s390
Cc: Philipp Rudo, Ard Biesheuvel, zohar, linux-kernel
In-Reply-To: <1583289211-5420-1-git-send-email-nayna@linux.ibm.com>
On Tue, 2020-03-03 at 21:33 -0500, Nayna Jain wrote:
> Every time a new architecture defines the IMA architecture specific
> functions - arch_ima_get_secureboot() and arch_ima_get_policy(), the
> IMA
> include file needs to be updated. To avoid this "noise", this patch
> defines a new IMA Kconfig IMA_SECURE_AND_OR_TRUSTED_BOOT option,
> allowing
> the different architectures to select it.
>
> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Philipp Rudo <prudo@linux.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> ---
> v2:
> * Fixed the issue identified by Mimi. Thanks Mimi, Ard, Heiko and
> Michael for
> discussing the fix.
>
> arch/powerpc/Kconfig | 1 +
> arch/s390/Kconfig | 1 +
> arch/x86/Kconfig | 1 +
> include/linux/ima.h | 3 +--
> security/integrity/ima/Kconfig | 9 +++++++++
> 5 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 497b7d0b2d7e..a5cfde432983 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -979,6 +979,7 @@ config PPC_SECURE_BOOT
> bool
> depends on PPC_POWERNV
> depends on IMA_ARCH_POLICY
> + select IMA_SECURE_AND_OR_TRUSTED_BOOT
> help
> Systems with firmware secure boot enabled need to define
> security
> policies to extend secure boot to the OS. This config
> allows a user
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index 8abe77536d9d..4a502fbcb800 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -195,6 +195,7 @@ config S390
> select ARCH_HAS_FORCE_DMA_UNENCRYPTED
> select SWIOTLB
> select GENERIC_ALLOCATOR
> + select IMA_SECURE_AND_OR_TRUSTED_BOOT if IMA_ARCH_POLICY
>
>
> config SCHED_OMIT_FRAME_POINTER
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index beea77046f9b..7f5bfaf0cbd2 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -230,6 +230,7 @@ config X86
> select VIRT_TO_BUS
> select X86_FEATURE_NAMES if PROC_FS
> select PROC_PID_ARCH_STATUS if PROC_FS
> + select IMA_SECURE_AND_OR_TRUSTED_BOOT if EFI &&
> IMA_ARCH_POLICY
>
> config INSTRUCTION_DECODER
> def_bool y
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 1659217e9b60..aefe758f4466 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -30,8 +30,7 @@ extern void ima_kexec_cmdline(const void *buf, int
> size);
> extern void ima_add_kexec_buffer(struct kimage *image);
> #endif
>
> -#if (defined(CONFIG_X86) && defined(CONFIG_EFI)) ||
> defined(CONFIG_S390) \
> - || defined(CONFIG_PPC_SECURE_BOOT)
> +#ifdef CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT
> extern bool arch_ima_get_secureboot(void);
> extern const char * const *arch_get_ima_policy(void);
> #else
> diff --git a/security/integrity/ima/Kconfig
> b/security/integrity/ima/Kconfig
> index 3f3ee4e2eb0d..d17972aa413a 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -327,3 +327,12 @@ config IMA_QUEUE_EARLY_BOOT_KEYS
> depends on IMA_MEASURE_ASYMMETRIC_KEYS
> depends on SYSTEM_TRUSTED_KEYRING
> default y
> +
> +config IMA_SECURE_AND_OR_TRUSTED_BOOT
> + bool
> + depends on IMA
> + depends on IMA_ARCH_POLICY
> + default n
You can't do this: a symbol designed to be selected can't depend on
other symbols because Kconfig doesn't see the dependencies during
select. We even have a doc for this now:
Documentation/kbuild/Kconfig.select-break
The only way to get this to work would be to have the long name symbol
select both IMA and IMA_ARCH_POLICY, which doesn't seem to be what you
want either.
Looking at what you're trying to do, I think making the symbol
independent of IMA and IMA_ARCH_POLICY is the correct thing, then
enforce the dependencies inside the outer #ifdef, but I haven't looked
deeply at the code.
James
^ permalink raw reply
* Re: [PATCH v2] ima: add a new CONFIG for loading arch-specific policies
From: Mimi Zohar @ 2020-03-04 12:35 UTC (permalink / raw)
To: James Bottomley, Nayna Jain, linux-integrity, linuxppc-dev,
linux-efi, linux-s390
Cc: Philipp Rudo, Ard Biesheuvel, linux-kernel
In-Reply-To: <1583307813.3907.4.camel@HansenPartnership.com>
On Tue, 2020-03-03 at 23:43 -0800, James Bottomley wrote:
> On Tue, 2020-03-03 at 21:33 -0500, Nayna Jain wrote:
> > diff --git a/security/integrity/ima/Kconfig
> > b/security/integrity/ima/Kconfig
> > index 3f3ee4e2eb0d..d17972aa413a 100644
> > --- a/security/integrity/ima/Kconfig
> > +++ b/security/integrity/ima/Kconfig
> > @@ -327,3 +327,12 @@ config IMA_QUEUE_EARLY_BOOT_KEYS
> > depends on IMA_MEASURE_ASYMMETRIC_KEYS
> > depends on SYSTEM_TRUSTED_KEYRING
> > default y
> > +
> > +config IMA_SECURE_AND_OR_TRUSTED_BOOT
> > + bool
> > + depends on IMA
> > + depends on IMA_ARCH_POLICY
> > + default n
>
> You can't do this: a symbol designed to be selected can't depend on
> other symbols because Kconfig doesn't see the dependencies during
> select. We even have a doc for this now:
>
> Documentation/kbuild/Kconfig.select-break
The document is discussing a circular dependency, where C selects B.
IMA_SECURE_AND_OR_TRUSTED_BOOT is not selecting anything, but is
being selected. All of the Kconfig's are now dependent on
IMA_ARCH_POLICY being enabled before selecting
IMA_SECURE_AND_OR_TRUSTED_BOOT.
As Ard pointed out, both IMA and IMA_ARCH_POLICY are not needed, as
IMA_ARCH_POLICY is already dependent on IMA.
Mimi
^ permalink raw reply
* Re: [PATCH V14] mm/debug: Add tests validating architecture page table helpers
From: Qian Cai @ 2020-03-04 11:29 UTC (permalink / raw)
To: Christophe Leroy
Cc: Heiko Carstens, Linux Memory Management List, Paul Mackerras,
H. Peter Anvin, linux-riscv, Will Deacon, linux-arch, linux-s390,
the arch/x86 maintainers, Mike Rapoport, Christian Borntraeger,
Ingo Molnar, Catalin Marinas, linux-snps-arc, Vasily Gorbik,
Anshuman Khandual, Borislav Petkov, Paul Walmsley,
Kirill A . Shutemov, Thomas Gleixner, linux-arm-kernel,
Vineet Gupta, LKML, Palmer Dabbelt, Aneesh Kumar K.V,
Andrew Morton, linuxppc-dev
In-Reply-To: <c022e863-0807-fab1-cd41-3c320381f448@c-s.fr>
> On Mar 4, 2020, at 1:49 AM, Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>
> AFAIU, you are not taking an interrupt here. You are stuck in the pte_update(), most likely due to nested locks. Try with LOCKDEP ?
Not exactly sure what did you mean here, but the kernel has all lockdep enabled and did not flag anything here.
^ permalink raw reply
* Re: [RFC PATCH v1] powerpc/prom_init: disable XIVE in Secure VM.
From: Greg Kurz @ 2020-03-04 10:59 UTC (permalink / raw)
To: Ram Pai
Cc: aik, andmike, kvm-ppc, Cédric Le Goater, sukadev,
linuxppc-dev, bauerman, David Gibson
In-Reply-To: <20200303185645.GB5416@oc0525413822.ibm.com>
On Tue, 3 Mar 2020 10:56:45 -0800
Ram Pai <linuxram@us.ibm.com> wrote:
> On Tue, Mar 03, 2020 at 06:45:20PM +0100, Greg Kurz wrote:
> > On Tue, 3 Mar 2020 09:02:05 -0800
> > Ram Pai <linuxram@us.ibm.com> wrote:
> >
> > > On Tue, Mar 03, 2020 at 07:50:08AM +0100, Cédric Le Goater wrote:
> > > > On 3/3/20 12:32 AM, David Gibson wrote:
> > > > > On Fri, Feb 28, 2020 at 11:54:04PM -0800, Ram Pai wrote:
> > > > >> XIVE is not correctly enabled for Secure VM in the KVM Hypervisor yet.
> > > > >>
> > > > >> Hence Secure VM, must always default to XICS interrupt controller.
> > > > >>
> > > > >> If XIVE is requested through kernel command line option "xive=on",
> > > > >> override and turn it off.
> > > > >>
> > > > >> If XIVE is the only supported platform interrupt controller; specified
> > > > >> through qemu option "ic-mode=xive", simply abort. Otherwise default to
> > > > >> XICS.
> > > > >
> > > > > Uh... the discussion thread here seems to have gotten oddly off
> > > > > track.
> > > >
> > > > There seem to be multiple issues. It is difficult to have a clear status.
> > > >
> > > > > So, to try to clean up some misunderstandings on both sides:
> > > > >
> > > > > 1) The guest is the main thing that knows that it will be in secure
> > > > > mode, so it's reasonable for it to conditionally use XIVE based
> > > > > on that
> > > >
> > > > FW support is required AFAIUI.
> > > > > 2) The mechanism by which we do it here isn't quite right. Here the
> > > > > guest is checking itself that the host only allows XIVE, but we
> > > > > can't do XIVE and is panic()ing. Instead, in the SVM case we
> > > > > should force support->xive to false, and send that in the CAS
> > > > > request to the host. We expect the host to just terminate
> > > > > us because of the mismatch, but this will interact better with
> > > > > host side options setting policy for panic states and the like.
> > > > > Essentially an SVM kernel should behave like an old kernel with
> > > > > no XIVE support at all, at least w.r.t. the CAS irq mode flags.
> > > >
> > > > Yes. XIVE shouldn't be requested by the guest.
> > >
> > > Ok.
> > >
> > > > This is the last option
> > > > I proposed but I thought there was some negotiation with the hypervisor
> > > > which is not the case.
> > > >
> > > > > 3) Although there are means by which the hypervisor can kind of know
> > > > > a guest is in secure mode, there's not really an "svm=on" option
> > > > > on the host side. For the most part secure mode is based on
> > > > > discussion directly between the guest and the ultravisor with
> > > > > almost no hypervisor intervention.
> > > >
> > > > Is there a negotiation with the ultravisor ?
> > >
> > > The VM has no negotiation with the ultravisor w.r.t CAS.
> > >
> > > >
> > > > > 4) I'm guessing the problem with XIVE in SVM mode is that XIVE needs
> > > > > to write to event queues in guest memory, which would have to be
> > > > > explicitly shared for secure mode. That's true whether it's KVM
> > > > > or qemu accessing the guest memory, so kernel_irqchip=on/off is
> > > > > entirely irrelevant.
> > > >
> > > > This problem should be already fixed.
> > > > The XIVE event queues are shared
> > >
> > > Yes i have a patch for the guest kernel that shares the event
> > > queue page with the hypervisor. This is done using the
> > > UV_SHARE_PAGE ultracall. This patch is not sent out to any any mailing
> > > lists yet.
> >
> > Why ?
>
> At this point I am not sure if this is the only change, I need to the
> guest kernel.
Maybe but we're already sure that this change is needed. I don't really see
the point in holding this any longer.
> I also need changes to KVM and to the ultravisor. Its bit
> premature to send the patch without having figured out everything
> to get xive working on a Secure VM.
>
I'm a bit confused... why did you send this workaround patch in
the first place then ? I mean, this raises a concern and we're
just trying to move forward.
> >
> > > However the patch by itself does not solve the xive problem
> > > for secure VM.
> > >
> >
> > This patch would allow at least to answer Cedric's question about
> > kernel_irqchip=off, since this looks like the only thing needed
> > to make it work.
>
> hmm.. I am not sure. Are you saying
> (a) patch the guest kernel to share the event queue page
> (b) run the qemu with "kernel_irqchip=off"
> (c) and the guest kernel with "svm=on"
>
> and it should all work?
>
Yes.
> RP
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox