LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 4/5] parisc: configs: drop unused BACKLIGHT_GENERIC option
From: Andrey Zhizhikin @ 2020-11-30 15:21 UTC (permalink / raw)
  To: linux, nicolas.ferre, alexandre.belloni, ludovic.desroches, tony,
	mripard, wens, jernej.skrabec, thierry.reding, jonathanh,
	catalin.marinas, will, tsbogend, James.Bottomley, deller, mpe,
	benh, paulus, lee.jones, sam, emil.l.velikov, daniel.thompson,
	krzk, linux-arm-kernel, linux-kernel, linux-omap, linux-tegra,
	linux-mips, linux-parisc, linuxppc-dev
In-Reply-To: <20201130152137.24909-1-andrey.zhizhikin@leica-geosystems.com>

Commit 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
unused") removed geenric_bl driver from the tree, together with
corresponding config option.

Remove BACKLIGHT_GENERIC config item from generic-64bit_defconfig.

Fixes: 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is unused")
Cc: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Andrey Zhizhikin <andrey.zhizhikin@leica-geosystems.com>
---
 arch/parisc/configs/generic-64bit_defconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/parisc/configs/generic-64bit_defconfig b/arch/parisc/configs/generic-64bit_defconfig
index 7e2d7026285e..8f81fcbf04c4 100644
--- a/arch/parisc/configs/generic-64bit_defconfig
+++ b/arch/parisc/configs/generic-64bit_defconfig
@@ -191,7 +191,6 @@ CONFIG_DRM=y
 CONFIG_DRM_RADEON=y
 CONFIG_FIRMWARE_EDID=y
 CONFIG_FB_MODE_HELPERS=y
-# CONFIG_BACKLIGHT_GENERIC is not set
 CONFIG_FRAMEBUFFER_CONSOLE_ROTATION=y
 CONFIG_HIDRAW=y
 CONFIG_HID_PID=y
-- 
2.17.1


^ permalink raw reply related

* [PATCH 3/5] MIPS: configs: drop unused BACKLIGHT_GENERIC option
From: Andrey Zhizhikin @ 2020-11-30 15:21 UTC (permalink / raw)
  To: linux, nicolas.ferre, alexandre.belloni, ludovic.desroches, tony,
	mripard, wens, jernej.skrabec, thierry.reding, jonathanh,
	catalin.marinas, will, tsbogend, James.Bottomley, deller, mpe,
	benh, paulus, lee.jones, sam, emil.l.velikov, daniel.thompson,
	krzk, linux-arm-kernel, linux-kernel, linux-omap, linux-tegra,
	linux-mips, linux-parisc, linuxppc-dev
In-Reply-To: <20201130152137.24909-1-andrey.zhizhikin@leica-geosystems.com>

Commit 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
unused") removed geenric_bl driver from the tree, together with
corresponding config option.

Remove BACKLIGHT_GENERIC config item from all MIPS configurations.

Fixes: 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is unused")
Cc: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Andrey Zhizhikin <andrey.zhizhikin@leica-geosystems.com>
---
 arch/mips/configs/gcw0_defconfig      | 1 -
 arch/mips/configs/gpr_defconfig       | 1 -
 arch/mips/configs/lemote2f_defconfig  | 1 -
 arch/mips/configs/loongson3_defconfig | 1 -
 arch/mips/configs/mtx1_defconfig      | 1 -
 arch/mips/configs/rs90_defconfig      | 1 -
 6 files changed, 6 deletions(-)

diff --git a/arch/mips/configs/gcw0_defconfig b/arch/mips/configs/gcw0_defconfig
index 7e28a4fe9d84..460683b52285 100644
--- a/arch/mips/configs/gcw0_defconfig
+++ b/arch/mips/configs/gcw0_defconfig
@@ -73,7 +73,6 @@ CONFIG_DRM_PANEL_NOVATEK_NT39016=y
 CONFIG_DRM_INGENIC=y
 CONFIG_DRM_ETNAVIV=y
 CONFIG_BACKLIGHT_CLASS_DEVICE=y
-# CONFIG_BACKLIGHT_GENERIC is not set
 CONFIG_BACKLIGHT_PWM=y
 # CONFIG_VGA_CONSOLE is not set
 CONFIG_FRAMEBUFFER_CONSOLE=y
diff --git a/arch/mips/configs/gpr_defconfig b/arch/mips/configs/gpr_defconfig
index 9085f4d6c698..87e20f3391ed 100644
--- a/arch/mips/configs/gpr_defconfig
+++ b/arch/mips/configs/gpr_defconfig
@@ -251,7 +251,6 @@ CONFIG_SSB_DRIVER_PCICORE=y
 # CONFIG_VGA_ARB is not set
 # CONFIG_LCD_CLASS_DEVICE is not set
 CONFIG_BACKLIGHT_CLASS_DEVICE=y
-# CONFIG_BACKLIGHT_GENERIC is not set
 # CONFIG_VGA_CONSOLE is not set
 CONFIG_USB_HID=m
 CONFIG_USB_HIDDEV=y
diff --git a/arch/mips/configs/lemote2f_defconfig b/arch/mips/configs/lemote2f_defconfig
index 3a9a453b1264..688c91918db2 100644
--- a/arch/mips/configs/lemote2f_defconfig
+++ b/arch/mips/configs/lemote2f_defconfig
@@ -145,7 +145,6 @@ CONFIG_FB_SIS_300=y
 CONFIG_FB_SIS_315=y
 # CONFIG_LCD_CLASS_DEVICE is not set
 CONFIG_BACKLIGHT_CLASS_DEVICE=y
-CONFIG_BACKLIGHT_GENERIC=m
 # CONFIG_VGA_CONSOLE is not set
 CONFIG_FRAMEBUFFER_CONSOLE=y
 CONFIG_FRAMEBUFFER_CONSOLE_ROTATION=y
diff --git a/arch/mips/configs/loongson3_defconfig b/arch/mips/configs/loongson3_defconfig
index 38a817ead8e7..9c5fadef38cb 100644
--- a/arch/mips/configs/loongson3_defconfig
+++ b/arch/mips/configs/loongson3_defconfig
@@ -286,7 +286,6 @@ CONFIG_DRM_VIRTIO_GPU=y
 CONFIG_FB_RADEON=y
 CONFIG_LCD_CLASS_DEVICE=y
 CONFIG_LCD_PLATFORM=m
-CONFIG_BACKLIGHT_GENERIC=m
 # CONFIG_VGA_CONSOLE is not set
 CONFIG_FRAMEBUFFER_CONSOLE=y
 CONFIG_FRAMEBUFFER_CONSOLE_ROTATION=y
diff --git a/arch/mips/configs/mtx1_defconfig b/arch/mips/configs/mtx1_defconfig
index 914af125a7fa..0ef2373404e5 100644
--- a/arch/mips/configs/mtx1_defconfig
+++ b/arch/mips/configs/mtx1_defconfig
@@ -450,7 +450,6 @@ CONFIG_WDT_MTX1=y
 # CONFIG_VGA_ARB is not set
 # CONFIG_LCD_CLASS_DEVICE is not set
 CONFIG_BACKLIGHT_CLASS_DEVICE=y
-# CONFIG_BACKLIGHT_GENERIC is not set
 # CONFIG_VGA_CONSOLE is not set
 CONFIG_SOUND=m
 CONFIG_SND=m
diff --git a/arch/mips/configs/rs90_defconfig b/arch/mips/configs/rs90_defconfig
index dfbb9fed9a42..4f540bb94628 100644
--- a/arch/mips/configs/rs90_defconfig
+++ b/arch/mips/configs/rs90_defconfig
@@ -97,7 +97,6 @@ CONFIG_DRM_FBDEV_OVERALLOC=300
 CONFIG_DRM_PANEL_SIMPLE=y
 CONFIG_DRM_INGENIC=y
 CONFIG_BACKLIGHT_CLASS_DEVICE=y
-# CONFIG_BACKLIGHT_GENERIC is not set
 CONFIG_BACKLIGHT_PWM=y
 # CONFIG_VGA_CONSOLE is not set
 CONFIG_FRAMEBUFFER_CONSOLE=y
-- 
2.17.1


^ permalink raw reply related

* [PATCH 2/5] arm64: defconfig: drop unused BACKLIGHT_GENERIC option
From: Andrey Zhizhikin @ 2020-11-30 15:21 UTC (permalink / raw)
  To: linux, nicolas.ferre, alexandre.belloni, ludovic.desroches, tony,
	mripard, wens, jernej.skrabec, thierry.reding, jonathanh,
	catalin.marinas, will, tsbogend, James.Bottomley, deller, mpe,
	benh, paulus, lee.jones, sam, emil.l.velikov, daniel.thompson,
	krzk, linux-arm-kernel, linux-kernel, linux-omap, linux-tegra,
	linux-mips, linux-parisc, linuxppc-dev
In-Reply-To: <20201130152137.24909-1-andrey.zhizhikin@leica-geosystems.com>

Commit 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
unused") removed geenric_bl driver from the tree, together with
corresponding config option.

Remove BACKLIGHT_GENERIC config item from arm64 configuration.

Fixes: 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is unused")
Cc: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Andrey Zhizhikin <andrey.zhizhikin@leica-geosystems.com>
---
 arch/arm64/configs/defconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 8e3f7ae71de5..280ed7404a1d 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -681,7 +681,6 @@ CONFIG_DRM_PANFROST=m
 CONFIG_FB=y
 CONFIG_FB_MODE_HELPERS=y
 CONFIG_FB_EFI=y
-CONFIG_BACKLIGHT_GENERIC=m
 CONFIG_BACKLIGHT_PWM=m
 CONFIG_BACKLIGHT_LP855X=m
 CONFIG_LOGO=y
-- 
2.17.1


^ permalink raw reply related

* [PATCH 0/5] drop unused BACKLIGHT_GENERIC option
From: Andrey Zhizhikin @ 2020-11-30 15:21 UTC (permalink / raw)
  To: linux, nicolas.ferre, alexandre.belloni, ludovic.desroches, tony,
	mripard, wens, jernej.skrabec, thierry.reding, jonathanh,
	catalin.marinas, will, tsbogend, James.Bottomley, deller, mpe,
	benh, paulus, lee.jones, sam, emil.l.velikov, daniel.thompson,
	krzk, linux-arm-kernel, linux-kernel, linux-omap, linux-tegra,
	linux-mips, linux-parisc, linuxppc-dev

Since the removal of generic_bl driver from the source tree in commit
7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
unused") BACKLIGHT_GENERIC config option became obsolete as well and
therefore subject to clean-up from all configuration files.

This series introduces patches to address this removal, separated by
architectures in the kernel tree.

Andrey Zhizhikin (5):
  ARM: configs: drop unused BACKLIGHT_GENERIC option
  arm64: defconfig: drop unused BACKLIGHT_GENERIC option
  MIPS: configs: drop unused BACKLIGHT_GENERIC option
  parisc: configs: drop unused BACKLIGHT_GENERIC option
  powerpc/configs: drop unused BACKLIGHT_GENERIC option

 arch/arm/configs/at91_dt_defconfig          | 1 -
 arch/arm/configs/cm_x300_defconfig          | 1 -
 arch/arm/configs/colibri_pxa300_defconfig   | 1 -
 arch/arm/configs/jornada720_defconfig       | 1 -
 arch/arm/configs/magician_defconfig         | 1 -
 arch/arm/configs/mini2440_defconfig         | 1 -
 arch/arm/configs/omap2plus_defconfig        | 1 -
 arch/arm/configs/pxa3xx_defconfig           | 1 -
 arch/arm/configs/qcom_defconfig             | 1 -
 arch/arm/configs/sama5_defconfig            | 1 -
 arch/arm/configs/sunxi_defconfig            | 1 -
 arch/arm/configs/tegra_defconfig            | 1 -
 arch/arm/configs/u8500_defconfig            | 1 -
 arch/arm64/configs/defconfig                | 1 -
 arch/mips/configs/gcw0_defconfig            | 1 -
 arch/mips/configs/gpr_defconfig             | 1 -
 arch/mips/configs/lemote2f_defconfig        | 1 -
 arch/mips/configs/loongson3_defconfig       | 1 -
 arch/mips/configs/mtx1_defconfig            | 1 -
 arch/mips/configs/rs90_defconfig            | 1 -
 arch/parisc/configs/generic-64bit_defconfig | 1 -
 arch/powerpc/configs/powernv_defconfig      | 1 -
 22 files changed, 22 deletions(-)


base-commit: b65054597872ce3aefbc6a666385eabdf9e288da
prerequisite-patch-id: bfd382cf1dc021d20204f10ea9403319c1c32b12
prerequisite-patch-id: 5397c0c8648bb3e0b830207ea867138c11c6e644
prerequisite-patch-id: a3c284dff5fe6d02828918a886db6a8ed3197e20
-- 
2.17.1


^ permalink raw reply

* [RFC Qemu PATCH v2 2/2] spapr: nvdimm: Implement async flush hcalls
From: Shivaprasad G Bhat @ 2020-11-30 15:17 UTC (permalink / raw)
  To: xiaoguangrong.eric, mst, imammedo, david, qemu-devel, qemu-ppc
  Cc: linux-nvdimm, aneesh.kumar, kvm-ppc, shivaprasadbhat, bharata,
	linuxppc-dev
In-Reply-To: <160674929554.2492771.17651548703390170573.stgit@lep8c.aus.stglabs.ibm.com>

When the persistent memory beacked by a file, a cpu cache flush instruction
is not sufficient to ensure the stores are correctly flushed to the media.

The patch implements the async hcalls for flush operation on demand from the
guest kernel.

The device option sync-dax is by default off and enables explicit asynchronous
flush requests from guest. It can be disabled by setting syn-dax=on.

Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
---
 hw/mem/nvdimm.c         |    1 +
 hw/ppc/spapr_nvdimm.c   |   79 +++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/mem/nvdimm.h |   10 ++++++
 include/hw/ppc/spapr.h  |    3 +-
 4 files changed, 92 insertions(+), 1 deletion(-)

diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
index 03c2201b56..37a4db0135 100644
--- a/hw/mem/nvdimm.c
+++ b/hw/mem/nvdimm.c
@@ -220,6 +220,7 @@ static void nvdimm_write_label_data(NVDIMMDevice *nvdimm, const void *buf,
 
 static Property nvdimm_properties[] = {
     DEFINE_PROP_BOOL(NVDIMM_UNARMED_PROP, NVDIMMDevice, unarmed, false),
+    DEFINE_PROP_BOOL(NVDIMM_SYNC_DAX_PROP, NVDIMMDevice, sync_dax, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/ppc/spapr_nvdimm.c b/hw/ppc/spapr_nvdimm.c
index a833a63b5e..557e36aa98 100644
--- a/hw/ppc/spapr_nvdimm.c
+++ b/hw/ppc/spapr_nvdimm.c
@@ -22,6 +22,7 @@
  * THE SOFTWARE.
  */
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 #include "qapi/error.h"
 #include "hw/ppc/spapr_drc.h"
 #include "hw/ppc/spapr_nvdimm.h"
@@ -155,6 +156,11 @@ static int spapr_dt_nvdimm(SpaprMachineState *spapr, void *fdt,
                              "operating-system")));
     _FDT(fdt_setprop(fdt, child_offset, "ibm,cache-flush-required", NULL, 0));
 
+    if (!nvdimm->sync_dax) {
+        _FDT(fdt_setprop(fdt, child_offset, "ibm,async-flush-required",
+                         NULL, 0));
+    }
+
     return child_offset;
 }
 
@@ -370,6 +376,78 @@ static target_ulong h_scm_bind_mem(PowerPCCPU *cpu, SpaprMachineState *spapr,
     return H_SUCCESS;
 }
 
+typedef struct SCMAsyncFlushData {
+    int fd;
+    uint64_t token;
+} SCMAsyncFlushData;
+
+static int flush_worker_cb(void *opaque)
+{
+    int ret = H_SUCCESS;
+    SCMAsyncFlushData *req_data = opaque;
+
+    /* flush raw backing image */
+    if (qemu_fdatasync(req_data->fd) < 0) {
+        error_report("papr_scm: Could not sync nvdimm to backend file: %s",
+                     strerror(errno));
+        ret = H_HARDWARE;
+    }
+
+    g_free(req_data);
+
+    return ret;
+}
+
+static target_ulong h_scm_async_flush(PowerPCCPU *cpu, SpaprMachineState *spapr,
+                                      target_ulong opcode, target_ulong *args)
+{
+    int ret;
+    uint32_t drc_index = args[0];
+    uint64_t continue_token = args[1];
+    SpaprDrc *drc = spapr_drc_by_index(drc_index);
+    PCDIMMDevice *dimm;
+    HostMemoryBackend *backend = NULL;
+    SCMAsyncFlushData *req_data = NULL;
+
+    if (!drc || !drc->dev ||
+        spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PMEM) {
+        return H_PARAMETER;
+    }
+
+    if (continue_token != 0) {
+        ret = spapr_drc_get_async_hcall_status(drc, continue_token);
+        if (ret == H_BUSY) {
+            args[0] = continue_token;
+            return H_LONG_BUSY_ORDER_1_SEC;
+        }
+
+        return ret;
+    }
+
+    dimm = PC_DIMM(drc->dev);
+    backend = MEMORY_BACKEND(dimm->hostmem);
+
+    req_data = g_malloc0(sizeof(SCMAsyncFlushData));
+    req_data->fd = memory_region_get_fd(&backend->mr);
+
+    continue_token = spapr_drc_get_new_async_hcall_token(drc);
+    if (!continue_token) {
+        g_free(req_data);
+        return H_P2;
+    }
+    req_data->token = continue_token;
+
+    spapr_drc_run_async_hcall(drc, continue_token, &flush_worker_cb, req_data);
+
+    ret = spapr_drc_get_async_hcall_status(drc, continue_token);
+    if (ret == H_BUSY) {
+        args[0] = req_data->token;
+        return ret;
+    }
+
+    return ret;
+}
+
 static target_ulong h_scm_unbind_mem(PowerPCCPU *cpu, SpaprMachineState *spapr,
                                      target_ulong opcode, target_ulong *args)
 {
@@ -486,6 +564,7 @@ static void spapr_scm_register_types(void)
     spapr_register_hypercall(H_SCM_BIND_MEM, h_scm_bind_mem);
     spapr_register_hypercall(H_SCM_UNBIND_MEM, h_scm_unbind_mem);
     spapr_register_hypercall(H_SCM_UNBIND_ALL, h_scm_unbind_all);
+    spapr_register_hypercall(H_SCM_ASYNC_FLUSH, h_scm_async_flush);
 }
 
 type_init(spapr_scm_register_types)
diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h
index c699842dd0..9e8795766e 100644
--- a/include/hw/mem/nvdimm.h
+++ b/include/hw/mem/nvdimm.h
@@ -51,6 +51,7 @@ OBJECT_DECLARE_TYPE(NVDIMMDevice, NVDIMMClass, NVDIMM)
 #define NVDIMM_LABEL_SIZE_PROP "label-size"
 #define NVDIMM_UUID_PROP       "uuid"
 #define NVDIMM_UNARMED_PROP    "unarmed"
+#define NVDIMM_SYNC_DAX_PROP    "sync-dax"
 
 struct NVDIMMDevice {
     /* private */
@@ -85,6 +86,15 @@ struct NVDIMMDevice {
      */
     bool unarmed;
 
+    /*
+     * On PPC64,
+     * The 'off' value results in the async-flush-required property set
+     * in the device tree for pseries machines. When 'off', the guest
+     * initiates explicity flush requests to the backend device ensuring
+     * write persistence.
+     */
+    bool sync_dax;
+
     /*
      * The PPC64 - spapr requires each nvdimm device have a uuid.
      */
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 2e89e36cfb..6d7110b7dc 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -535,8 +535,9 @@ struct SpaprMachineState {
 #define H_SCM_BIND_MEM          0x3EC
 #define H_SCM_UNBIND_MEM        0x3F0
 #define H_SCM_UNBIND_ALL        0x3FC
+#define H_SCM_ASYNC_FLUSH       0x4A0
 
-#define MAX_HCALL_OPCODE        H_SCM_UNBIND_ALL
+#define MAX_HCALL_OPCODE        H_SCM_ASYNC_FLUSH
 
 /* The hcalls above are standardized in PAPR and implemented by pHyp
  * as well.



^ permalink raw reply related

* [RFC Qemu PATCH v2 1/2] spapr: drc: Add support for async hcalls at the drc level
From: Shivaprasad G Bhat @ 2020-11-30 15:16 UTC (permalink / raw)
  To: xiaoguangrong.eric, mst, imammedo, david, qemu-devel, qemu-ppc
  Cc: linux-nvdimm, aneesh.kumar, kvm-ppc, shivaprasadbhat, bharata,
	linuxppc-dev
In-Reply-To: <160674929554.2492771.17651548703390170573.stgit@lep8c.aus.stglabs.ibm.com>

The patch adds support for async hcalls at the DRC level for the
spapr devices. To be used by spapr-scm devices in the patch/es to follow.

Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
---
 hw/ppc/spapr_drc.c         |  149 ++++++++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/spapr_drc.h |   25 +++++++
 2 files changed, 174 insertions(+)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 77718cde1f..4ecd04f686 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -15,6 +15,7 @@
 #include "qapi/qmp/qnull.h"
 #include "cpu.h"
 #include "qemu/cutils.h"
+#include "qemu/guest-random.h"
 #include "hw/ppc/spapr_drc.h"
 #include "qom/object.h"
 #include "migration/vmstate.h"
@@ -421,6 +422,148 @@ void spapr_drc_detach(SpaprDrc *drc)
     spapr_drc_release(drc);
 }
 
+
+/*
+ * @drc : device DRC targetting which the async hcalls to be made.
+ *
+ * All subsequent requests to run/query the status should use the
+ * unique token returned here.
+ */
+uint64_t spapr_drc_get_new_async_hcall_token(SpaprDrc *drc)
+{
+    Error *err = NULL;
+    uint64_t token;
+    SpaprDrcDeviceAsyncHCallState *tmp, *next, *state;
+
+    state = g_malloc0(sizeof(*state));
+    state->pending = true;
+
+    qemu_mutex_lock(&drc->async_hcall_states_lock);
+retry:
+    if (qemu_guest_getrandom(&token, sizeof(token), &err) < 0) {
+        error_report_err(err);
+        g_free(state);
+        qemu_mutex_unlock(&drc->async_hcall_states_lock);
+        return 0;
+    }
+
+    if (!token) /* Token should be non-zero */
+        goto retry;
+
+    if (!QLIST_EMPTY(&drc->async_hcall_states)) {
+        QLIST_FOREACH_SAFE(tmp, &drc->async_hcall_states, node, next) {
+            if (tmp->continue_token == token) {
+                /* If the token already in use, get a new one */
+                goto retry;
+            }
+        }
+    }
+
+    state->continue_token = token;
+    QLIST_INSERT_HEAD(&drc->async_hcall_states, state, node);
+
+    qemu_mutex_unlock(&drc->async_hcall_states_lock);
+
+    return state->continue_token;
+}
+
+static void *spapr_drc_async_hcall_runner(void *opaque)
+{
+    int response = -1;
+    SpaprDrcDeviceAsyncHCallState *state = opaque;
+
+    /*
+     * state is freed only after this thread finishes(after pthread_join()),
+     * don't worry about it becoming NULL.
+     */
+
+    response = state->func(state->data);
+
+    state->hcall_ret = response;
+    state->pending = 0;
+
+    return NULL;
+}
+
+/*
+ * @drc  : device DRC targetting which the async hcalls to be made.
+ * token : The continue token to be used for tracking as recived from
+ *         spapr_drc_get_new_async_hcall_token
+ * @func() : the worker function which needs to be executed asynchronously
+ * @data : data to be passed to the asynchronous function. Worker is supposed
+ *         to free/cleanup the data that is passed here
+ */
+void spapr_drc_run_async_hcall(SpaprDrc *drc, uint64_t token,
+                               SpaprDrcAsyncHcallWorkerFunc *func, void *data)
+{
+    SpaprDrcDeviceAsyncHCallState *state;
+
+    qemu_mutex_lock(&drc->async_hcall_states_lock);
+    QLIST_FOREACH(state, &drc->async_hcall_states, node) {
+        if (state->continue_token == token) {
+            state->func = func;
+            state->data = data;
+            qemu_thread_create(&state->thread, "sPAPR Async HCALL",
+                               spapr_drc_async_hcall_runner, state,
+                               QEMU_THREAD_JOINABLE);
+            break;
+        }
+    }
+    qemu_mutex_unlock(&drc->async_hcall_states_lock);
+}
+
+/*
+ * spapr_drc_finish_async_hcalls
+ *      Waits for all pending async requests to complete
+ *      thier execution and free the states
+ */
+static void spapr_drc_finish_async_hcalls(SpaprDrc *drc)
+{
+    SpaprDrcDeviceAsyncHCallState *state, *next;
+
+    if (QLIST_EMPTY(&drc->async_hcall_states)) {
+        return;
+    }
+
+    qemu_mutex_lock(&drc->async_hcall_states_lock);
+    QLIST_FOREACH_SAFE(state, &drc->async_hcall_states, node, next) {
+        qemu_thread_join(&state->thread);
+        QLIST_REMOVE(state, node);
+        g_free(state);
+    }
+    qemu_mutex_unlock(&drc->async_hcall_states_lock);
+}
+
+/*
+ * spapr_drc_get_async_hcall_status
+ *      Fetches the status of the hcall worker and returns H_BUSY
+ *      if the worker is still running.
+ */
+int spapr_drc_get_async_hcall_status(SpaprDrc *drc, uint64_t token)
+{
+    int ret = H_PARAMETER;
+    SpaprDrcDeviceAsyncHCallState *state, *node;
+
+    qemu_mutex_lock(&drc->async_hcall_states_lock);
+    QLIST_FOREACH_SAFE(state, &drc->async_hcall_states, node, node) {
+        if (state->continue_token == token) {
+            if (state->pending) {
+                ret = H_BUSY;
+                break;
+            } else {
+                ret = state->hcall_ret;
+                qemu_thread_join(&state->thread);
+                QLIST_REMOVE(state, node);
+                g_free(state);
+                break;
+            }
+        }
+    }
+    qemu_mutex_unlock(&drc->async_hcall_states_lock);
+
+    return ret;
+}
+
 void spapr_drc_reset(SpaprDrc *drc)
 {
     SpaprDrcClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
@@ -448,6 +591,7 @@ void spapr_drc_reset(SpaprDrc *drc)
         drc->ccs_offset = -1;
         drc->ccs_depth = -1;
     }
+    spapr_drc_finish_async_hcalls(drc);
 }
 
 static bool spapr_drc_unplug_requested_needed(void *opaque)
@@ -558,6 +702,7 @@ SpaprDrc *spapr_dr_connector_new(Object *owner, const char *type,
     drc->owner = owner;
     prop_name = g_strdup_printf("dr-connector[%"PRIu32"]",
                                 spapr_drc_index(drc));
+
     object_property_add_child(owner, prop_name, OBJECT(drc));
     object_unref(OBJECT(drc));
     qdev_realize(DEVICE(drc), NULL, NULL);
@@ -577,6 +722,10 @@ static void spapr_dr_connector_instance_init(Object *obj)
     object_property_add(obj, "fdt", "struct", prop_get_fdt,
                         NULL, NULL, NULL);
     drc->state = drck->empty_state;
+
+    qemu_mutex_init(&drc->async_hcall_states_lock);
+    QLIST_INIT(&drc->async_hcall_states);
+
 }
 
 static void spapr_dr_connector_class_init(ObjectClass *k, void *data)
diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
index 165b281496..77f6e4386c 100644
--- a/include/hw/ppc/spapr_drc.h
+++ b/include/hw/ppc/spapr_drc.h
@@ -18,6 +18,7 @@
 #include "sysemu/runstate.h"
 #include "hw/qdev-core.h"
 #include "qapi/error.h"
+#include "block/thread-pool.h"
 
 #define TYPE_SPAPR_DR_CONNECTOR "spapr-dr-connector"
 #define SPAPR_DR_CONNECTOR_GET_CLASS(obj) \
@@ -168,6 +169,21 @@ typedef enum {
     SPAPR_DRC_STATE_PHYSICAL_CONFIGURED = 8,
 } SpaprDrcState;
 
+typedef struct SpaprDrc SpaprDrc;
+
+typedef int SpaprDrcAsyncHcallWorkerFunc(void *opaque);
+typedef struct SpaprDrcDeviceAsyncHCallState {
+    uint64_t continue_token;
+    bool pending;
+
+    int hcall_ret;
+    SpaprDrcAsyncHcallWorkerFunc *func;
+    void *data;
+
+    QemuThread thread;
+
+    QLIST_ENTRY(SpaprDrcDeviceAsyncHCallState) node;
+} SpaprDrcDeviceAsyncHCallState;
 typedef struct SpaprDrc {
     /*< private >*/
     DeviceState parent;
@@ -182,6 +198,10 @@ typedef struct SpaprDrc {
     int ccs_offset;
     int ccs_depth;
 
+    /* async hcall states */
+    QemuMutex async_hcall_states_lock;
+    QLIST_HEAD(, SpaprDrcDeviceAsyncHCallState) async_hcall_states;
+
     /* device pointer, via link property */
     DeviceState *dev;
     bool unplug_requested;
@@ -241,6 +261,11 @@ void spapr_drc_detach(SpaprDrc *drc);
 /* Returns true if a hot plug/unplug request is pending */
 bool spapr_drc_transient(SpaprDrc *drc);
 
+uint64_t spapr_drc_get_new_async_hcall_token(SpaprDrc *drc);
+void spapr_drc_run_async_hcall(SpaprDrc *drc, uint64_t token,
+                               SpaprDrcAsyncHcallWorkerFunc, void *data);
+int spapr_drc_get_async_hcall_status(SpaprDrc *drc, uint64_t token);
+
 static inline bool spapr_drc_unplug_requested(SpaprDrc *drc)
 {
     return drc->unplug_requested;



^ permalink raw reply related

* [RFC Qemu PATCH v2 0/2] spapr: nvdimm: Asynchronus flush hcall support
From: Shivaprasad G Bhat @ 2020-11-30 15:16 UTC (permalink / raw)
  To: xiaoguangrong.eric, mst, imammedo, david, qemu-devel, qemu-ppc
  Cc: linux-nvdimm, aneesh.kumar, kvm-ppc, shivaprasadbhat, bharata,
	linuxppc-dev

The nvdimm devices are expected to ensure write persistent during power
failure kind of scenarios.

The libpmem has architecture specific instructions like dcbf on power
to flush the cache data to backend nvdimm device during normal writes.

Qemu - virtual nvdimm devices are memory mapped. The dcbf in the guest
doesn't traslate to actual flush to the backend file on the host in case
of file backed vnvdimms. This is addressed by virtio-pmem in case of x86_64
by making asynchronous flushes.

On PAPR, issue is addressed by adding a new hcall to
request for an explicit asynchronous flush requests from the guest ndctl
driver when the backend nvdimm cannot ensure write persistence with dcbf
alone. So, the approach here is to convey when the asynchronous flush is
required in a device tree property. The guest makes the hcall when the
property is found, instead of relying on dcbf.

The first patch adds the necessary asynchronous hcall support infrastructure
code at the DRC level. Second patch implements the hcall using the
infrastructure.

Hcall semantics are in review and not final.

A new device property sync-dax is added to the nvdimm device. When the 
sync-dax is off(default), the asynchronous hcalls will be called.

With respect to save from new qemu to restore on old qemu, having the
sync-dax by default off(when not specified) causes IO errors in guests as
the async-hcall would not be supported on old qemu. The new hcall
implementation being supported only on the new  pseries machine version,
the current machine version checks may be sufficient to prevent
such migration. Please suggest what should be done.

The below demonstration shows the map_sync behavior with sync-dax on & off.
(https://github.com/avocado-framework-tests/avocado-misc-tests/blob/master/memory/ndctl.py.data/map_sync.c)

The pmem0 is from nvdimm with With sync-dax=on, and pmem1 is from nvdimm with syn-dax=off, mounted as
/dev/pmem0 on /mnt1 type xfs (rw,relatime,attr2,dax=always,inode64,logbufs=8,logbsize=32k,noquota)
/dev/pmem1 on /mnt2 type xfs (rw,relatime,attr2,dax=always,inode64,logbufs=8,logbsize=32k,noquota)

[root@atest-guest ~]# ./mapsync /mnt1/newfile    ----> When sync-dax=off
[root@atest-guest ~]# ./mapsync /mnt2/newfile    ----> when sync-dax=on
Failed to mmap  with Operation not supported

---
v1 - https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg06330.html
Changes from v1
      - Fixed a missed-out unlock
      - using QLIST_FOREACH instead of QLIST_FOREACH_SAFE while generating token

Shivaprasad G Bhat (2):
      spapr: drc: Add support for async hcalls at the drc level
      spapr: nvdimm: Implement async flush hcalls


 hw/mem/nvdimm.c            |    1
 hw/ppc/spapr_drc.c         |  146 ++++++++++++++++++++++++++++++++++++++++++++
 hw/ppc/spapr_nvdimm.c      |   79 ++++++++++++++++++++++++
 include/hw/mem/nvdimm.h    |   10 +++
 include/hw/ppc/spapr.h     |    3 +
 include/hw/ppc/spapr_drc.h |   25 ++++++++
 6 files changed, 263 insertions(+), 1 deletion(-)

--
Signature


^ permalink raw reply

* Re: [PATCH 0/5] drop unused BACKLIGHT_GENERIC option
From: Krzysztof Kozlowski @ 2020-11-30 20:25 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: alexandre.belloni, tony, linux-kernel, James.Bottomley,
	thierry.reding, paulus, lee.jones, daniel.thompson, linux-omap,
	Arnd Bergmann, deller, linux, jonathanh, ludovic.desroches,
	catalin.marinas, linux-mips, will, mripard, Andrey Zhizhikin,
	linux-tegra, wens, linux-arm-kernel, jernej.skrabec, tsbogend,
	linux-parisc, emil.l.velikov, nicolas.ferre, Olof Johansson,
	linuxppc-dev
In-Reply-To: <20201130191133.GA1565464@ravnborg.org>

On Mon, Nov 30, 2020 at 08:11:33PM +0100, Sam Ravnborg wrote:
> On Mon, Nov 30, 2020 at 03:21:32PM +0000, Andrey Zhizhikin wrote:
> > Since the removal of generic_bl driver from the source tree in commit
> > 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> > unused") BACKLIGHT_GENERIC config option became obsolete as well and
> > therefore subject to clean-up from all configuration files.
> > 
> > This series introduces patches to address this removal, separated by
> > architectures in the kernel tree.
> > 
> > Andrey Zhizhikin (5):
> >   ARM: configs: drop unused BACKLIGHT_GENERIC option
> >   arm64: defconfig: drop unused BACKLIGHT_GENERIC option
> >   MIPS: configs: drop unused BACKLIGHT_GENERIC option
> >   parisc: configs: drop unused BACKLIGHT_GENERIC option
> >   powerpc/configs: drop unused BACKLIGHT_GENERIC option
> 
> For defconfigs I expect arch maintainers to do a make xxxdefconfig / make
> savedefconfig / cp defconfig ... run now and then - this will remove
> all such symbols.

savedefconfig can be tricky because of risk of loosing options:
1. it will remove options which became the default or became selected,
2. later when the default is changed or selecting option is removed, the
   first option from #1 will not be brought back.

This was already for example with DEBUG_FS and the conclusion that time
was - do not run savedefconfig automatically.

Therefore if some symbol(s) can be safely removed, patch is welcomed.

Best regards,
Krzysztof

> 
> If the patches goes in like they are submitted then:
> Acked-by: Sam Ravnborg <sam@ravnborg.org>

^ permalink raw reply

* Re: [PATCH v8 11/12] mm/vmalloc: Hugepage vmalloc mappings
From: Edgecombe, Rick P @ 2020-11-30 20:21 UTC (permalink / raw)
  To: npiggin@gmail.com, linux-mm@kvack.org, akpm@linux-foundation.org
  Cc: linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org,
	hch@infradead.org, lizefan@huawei.com,
	Jonathan.Cameron@Huawei.com, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20201128152559.999540-12-npiggin@gmail.com>

On Sun, 2020-11-29 at 01:25 +1000, Nicholas Piggin wrote:
> Support huge page vmalloc mappings. Config option
> HAVE_ARCH_HUGE_VMALLOC
> enables support on architectures that define HAVE_ARCH_HUGE_VMAP and
> supports PMD sized vmap mappings.
> 
> vmalloc will attempt to allocate PMD-sized pages if allocating PMD
> size
> or larger, and fall back to small pages if that was unsuccessful.
> 
> Allocations that do not use PAGE_KERNEL prot are not permitted to use
> huge pages, because not all callers expect this (e.g., module
> allocations vs strict module rwx).

Several architectures (x86, arm64, others?) allocate modules initially
with PAGE_KERNEL and so I think this test will not exclude module
allocations in those cases.

[snip]

> @@ -2400,6 +2453,7 @@ static inline void set_area_direct_map(const
> struct vm_struct *area,
>  {
>  	int i;
>  
> +	/* HUGE_VMALLOC passes small pages to set_direct_map */
>  	for (i = 0; i < area->nr_pages; i++)
>  		if (page_address(area->pages[i]))
>  			set_direct_map(area->pages[i]);
> @@ -2433,11 +2487,12 @@ static void vm_remove_mappings(struct
> vm_struct *area, int deallocate_pages)
>  	 * map. Find the start and end range of the direct mappings to
> make sure
>  	 * the vm_unmap_aliases() flush includes the direct map.
>  	 */
> -	for (i = 0; i < area->nr_pages; i++) {
> +	for (i = 0; i < area->nr_pages; i += 1U << area->page_order) {
>  		unsigned long addr = (unsigned long)page_address(area-
> >pages[i]);
>  		if (addr) {
> +			unsigned long page_size = PAGE_SIZE << area-
> >page_order;
>  			start = min(addr, start);
> -			end = max(addr + PAGE_SIZE, end);
> +			end = max(addr + page_size, end);
>  			flush_dmap = 1;
>  		}
>  	}

The logic around this is a bit tangled. The reset of the direct map has
to succeed, but if the set_direct_map_() functions require a split they
could fail. For x86, set_memory_ro() calls on a vmalloc alias will
mirror the page size and permission on the direct map and so the direct
map will be broken to 4k pages if it's a RO vmalloc allocation.

But after this, module vmalloc()'s could have large pages which would
result in large RO pages on the direct map. Then it could possibly fail
when trying to reset a 4k page out of a large RO direct map mapping. 

I think either module allocations need to be actually excluded from
having large pages (seems like you might have seen other issues as
well?), or another option could be to use the changes here:
https://lore.kernel.org/lkml/20201125092208.12544-4-rppt@kernel.org/
to reset the direct map for a large page range at a time for large 
vmalloc pages.

^ permalink raw reply

* Re: [PATCH 0/5] drop unused BACKLIGHT_GENERIC option
From: Sam Ravnborg @ 2020-11-30 19:11 UTC (permalink / raw)
  To: Andrey Zhizhikin
  Cc: alexandre.belloni, tony, linux-kernel, James.Bottomley,
	thierry.reding, paulus, lee.jones, daniel.thompson, linux-omap,
	Arnd Bergmann, deller, linux, krzk, jonathanh, ludovic.desroches,
	catalin.marinas, linux-mips, will, mripard, linux-tegra, wens,
	linux-arm-kernel, jernej.skrabec, tsbogend, linux-parisc,
	emil.l.velikov, nicolas.ferre, Olof Johansson, linuxppc-dev
In-Reply-To: <20201130152137.24909-1-andrey.zhizhikin@leica-geosystems.com>

On Mon, Nov 30, 2020 at 03:21:32PM +0000, Andrey Zhizhikin wrote:
> Since the removal of generic_bl driver from the source tree in commit
> 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> unused") BACKLIGHT_GENERIC config option became obsolete as well and
> therefore subject to clean-up from all configuration files.
> 
> This series introduces patches to address this removal, separated by
> architectures in the kernel tree.
> 
> Andrey Zhizhikin (5):
>   ARM: configs: drop unused BACKLIGHT_GENERIC option
>   arm64: defconfig: drop unused BACKLIGHT_GENERIC option
>   MIPS: configs: drop unused BACKLIGHT_GENERIC option
>   parisc: configs: drop unused BACKLIGHT_GENERIC option
>   powerpc/configs: drop unused BACKLIGHT_GENERIC option

For defconfigs I expect arch maintainers to do a make xxxdefconfig / make
savedefconfig / cp defconfig ... run now and then - this will remove
all such symbols.

If the patches goes in like they are submitted then:
Acked-by: Sam Ravnborg <sam@ravnborg.org>

> 
>  arch/arm/configs/at91_dt_defconfig          | 1 -
>  arch/arm/configs/cm_x300_defconfig          | 1 -
>  arch/arm/configs/colibri_pxa300_defconfig   | 1 -
>  arch/arm/configs/jornada720_defconfig       | 1 -
>  arch/arm/configs/magician_defconfig         | 1 -
>  arch/arm/configs/mini2440_defconfig         | 1 -
>  arch/arm/configs/omap2plus_defconfig        | 1 -
>  arch/arm/configs/pxa3xx_defconfig           | 1 -
>  arch/arm/configs/qcom_defconfig             | 1 -
>  arch/arm/configs/sama5_defconfig            | 1 -
>  arch/arm/configs/sunxi_defconfig            | 1 -
>  arch/arm/configs/tegra_defconfig            | 1 -
>  arch/arm/configs/u8500_defconfig            | 1 -
>  arch/arm64/configs/defconfig                | 1 -
>  arch/mips/configs/gcw0_defconfig            | 1 -
>  arch/mips/configs/gpr_defconfig             | 1 -
>  arch/mips/configs/lemote2f_defconfig        | 1 -
>  arch/mips/configs/loongson3_defconfig       | 1 -
>  arch/mips/configs/mtx1_defconfig            | 1 -
>  arch/mips/configs/rs90_defconfig            | 1 -
>  arch/parisc/configs/generic-64bit_defconfig | 1 -
>  arch/powerpc/configs/powernv_defconfig      | 1 -
>  22 files changed, 22 deletions(-)
> 
> 
> base-commit: b65054597872ce3aefbc6a666385eabdf9e288da
> prerequisite-patch-id: bfd382cf1dc021d20204f10ea9403319c1c32b12
> prerequisite-patch-id: 5397c0c8648bb3e0b830207ea867138c11c6e644
> prerequisite-patch-id: a3c284dff5fe6d02828918a886db6a8ed3197e20
> -- 
> 2.17.1

^ permalink raw reply

* [PATCH net v2 2/2] ibmvnic: Fix TX completion error handling
From: Thomas Falcon @ 2020-11-30 19:07 UTC (permalink / raw)
  To: mpe
  Cc: cforno12, netdev, ljp, ricklind, dnbanerg, tlfalcon, drt, brking,
	sukadev, linuxppc-dev
In-Reply-To: <1606763244-28111-1-git-send-email-tlfalcon@linux.ibm.com>

TX completions received with an error return code are not
being processed properly. When an error code is seen, do not
proceed to the next completion before cleaning up the existing
entry's data structures.

Fixes: 032c5e828 ("Driver for IBM System i/p VNIC protocol")
Signed-off-by: Thomas Falcon <tlfalcon@linux.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 5ea9f5c..10878f8 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -3113,11 +3113,9 @@ static int ibmvnic_complete_tx(struct ibmvnic_adapter *adapter,
 
 		next = ibmvnic_next_scrq(adapter, scrq);
 		for (i = 0; i < next->tx_comp.num_comps; i++) {
-			if (next->tx_comp.rcs[i]) {
+			if (next->tx_comp.rcs[i])
 				dev_err(dev, "tx error %x\n",
 					next->tx_comp.rcs[i]);
-				continue;
-			}
 			index = be32_to_cpu(next->tx_comp.correlators[i]);
 			if (index & IBMVNIC_TSO_POOL_MASK) {
 				tx_pool = &adapter->tso_pool[pool];
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH net v2 0/2] ibmvnic: Bug fixes for queue descriptor processing
From: Thomas Falcon @ 2020-11-30 19:07 UTC (permalink / raw)
  To: mpe
  Cc: cforno12, netdev, ljp, ricklind, dnbanerg, tlfalcon, drt, brking,
	sukadev, linuxppc-dev

This series resolves a few issues in the ibmvnic driver's
RX buffer and TX completion processing. The first patch
includes memory barriers to synchronize queue descriptor
reads. The second patch fixes a memory leak that could
occur if the device returns a TX completion with an error
code in the descriptor, in which case the respective socket
buffer and other relevant data structures may not be freed
or updated properly.

v2: Provide more detailed comments explaining specifically what
    reads are being ordered, suggested by Michael Ellerman

Thomas Falcon (2):
  ibmvnic: Ensure that SCRQ entry reads are correctly ordered
  ibmvnic: Fix TX completion error handling

 drivers/net/ethernet/ibm/ibmvnic.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

-- 
1.8.3.1


^ permalink raw reply

* [PATCH net v2 1/2] ibmvnic: Ensure that SCRQ entry reads are correctly ordered
From: Thomas Falcon @ 2020-11-30 19:07 UTC (permalink / raw)
  To: mpe
  Cc: cforno12, netdev, ljp, ricklind, dnbanerg, tlfalcon, drt, brking,
	sukadev, linuxppc-dev
In-Reply-To: <1606763244-28111-1-git-send-email-tlfalcon@linux.ibm.com>

Ensure that received Subordinate Command-Response Queue (SCRQ)
entries are properly read in order by the driver. These queues
are used in the ibmvnic device to process RX buffer and TX completion
descriptors. dma_rmb barriers have been added after checking for a
pending descriptor to ensure the correct descriptor entry is checked
and after reading the SCRQ descriptor to ensure the entire
descriptor is read before processing.

Fixes: 032c5e828 ("Driver for IBM System i/p VNIC protocol")
Signed-off-by: Thomas Falcon <tlfalcon@linux.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 2aa40b2..5ea9f5c 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -2403,6 +2403,12 @@ static int ibmvnic_poll(struct napi_struct *napi, int budget)
 
 		if (!pending_scrq(adapter, adapter->rx_scrq[scrq_num]))
 			break;
+		/* The queue entry at the current index is peeked at above
+		 * to determine that there is a valid descriptor awaiting
+		 * processing. We want to be sure that the current slot
+		 * holds a valid descriptor before reading its contents.
+		 */
+		dma_rmb();
 		next = ibmvnic_next_scrq(adapter, adapter->rx_scrq[scrq_num]);
 		rx_buff =
 		    (struct ibmvnic_rx_buff *)be64_to_cpu(next->
@@ -3098,6 +3104,13 @@ static int ibmvnic_complete_tx(struct ibmvnic_adapter *adapter,
 		unsigned int pool = scrq->pool_index;
 		int num_entries = 0;
 
+		/* The queue entry at the current index is peeked at above
+		 * to determine that there is a valid descriptor awaiting
+		 * processing. We want to be sure that the current slot
+		 * holds a valid descriptor before reading its contents.
+		 */
+		dma_rmb();
+
 		next = ibmvnic_next_scrq(adapter, scrq);
 		for (i = 0; i < next->tx_comp.num_comps; i++) {
 			if (next->tx_comp.rcs[i]) {
@@ -3498,6 +3511,11 @@ static union sub_crq *ibmvnic_next_scrq(struct ibmvnic_adapter *adapter,
 	}
 	spin_unlock_irqrestore(&scrq->lock, flags);
 
+	/* Ensure that the entire buffer descriptor has been
+	 * loaded before reading its contents
+	 */
+	dma_rmb();
+
 	return entry;
 }
 
-- 
1.8.3.1


^ permalink raw reply related

* Re: [PATCH 5/5] powerpc/configs: drop unused BACKLIGHT_GENERIC option
From: Krzysztof Kozlowski @ 2020-11-30 18:53 UTC (permalink / raw)
  To: Andrey Zhizhikin
  Cc: alexandre.belloni, tony, linux-kernel, James.Bottomley,
	thierry.reding, paulus, sam, daniel.thompson, linux-omap, deller,
	linux, jonathanh, ludovic.desroches, catalin.marinas, linux-mips,
	will, mripard, linux-tegra, lee.jones, wens, linux-arm-kernel,
	jernej.skrabec, tsbogend, linux-parisc, emil.l.velikov,
	nicolas.ferre, linuxppc-dev
In-Reply-To: <20201130152137.24909-6-andrey.zhizhikin@leica-geosystems.com>

On Mon, Nov 30, 2020 at 03:21:37PM +0000, Andrey Zhizhikin wrote:
> Commit 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> unused") removed geenric_bl driver from the tree, together with
> corresponding config option.
> 
> Remove BACKLIGHT_GENERIC config item from generic-64bit_defconfig.
> 
> Fixes: 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is unused")
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Andrey Zhizhikin <andrey.zhizhikin@leica-geosystems.com>
> ---
>  arch/powerpc/configs/powernv_defconfig | 1 -
>  1 file changed, 1 deletion(-)
> 

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH 4/5] parisc: configs: drop unused BACKLIGHT_GENERIC option
From: Krzysztof Kozlowski @ 2020-11-30 18:53 UTC (permalink / raw)
  To: Andrey Zhizhikin
  Cc: alexandre.belloni, tony, linux-kernel, James.Bottomley,
	thierry.reding, paulus, sam, daniel.thompson, linux-omap, deller,
	linux, jonathanh, ludovic.desroches, catalin.marinas, linux-mips,
	will, mripard, linux-tegra, lee.jones, wens, linux-arm-kernel,
	jernej.skrabec, tsbogend, linux-parisc, emil.l.velikov,
	nicolas.ferre, linuxppc-dev
In-Reply-To: <20201130152137.24909-5-andrey.zhizhikin@leica-geosystems.com>

On Mon, Nov 30, 2020 at 03:21:36PM +0000, Andrey Zhizhikin wrote:
> Commit 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> unused") removed geenric_bl driver from the tree, together with
> corresponding config option.
> 
> Remove BACKLIGHT_GENERIC config item from generic-64bit_defconfig.
> 
> Fixes: 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is unused")
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Andrey Zhizhikin <andrey.zhizhikin@leica-geosystems.com>
> ---
>  arch/parisc/configs/generic-64bit_defconfig | 1 -
>  1 file changed, 1 deletion(-)
> 

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH 3/5] MIPS: configs: drop unused BACKLIGHT_GENERIC option
From: Krzysztof Kozlowski @ 2020-11-30 18:53 UTC (permalink / raw)
  To: Andrey Zhizhikin
  Cc: alexandre.belloni, tony, linux-kernel, James.Bottomley,
	thierry.reding, paulus, sam, daniel.thompson, linux-omap, deller,
	linux, jonathanh, ludovic.desroches, catalin.marinas, linux-mips,
	will, mripard, linux-tegra, lee.jones, wens, linux-arm-kernel,
	jernej.skrabec, tsbogend, linux-parisc, emil.l.velikov,
	nicolas.ferre, linuxppc-dev
In-Reply-To: <20201130152137.24909-4-andrey.zhizhikin@leica-geosystems.com>

On Mon, Nov 30, 2020 at 03:21:35PM +0000, Andrey Zhizhikin wrote:
> Commit 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> unused") removed geenric_bl driver from the tree, together with
> corresponding config option.
> 
> Remove BACKLIGHT_GENERIC config item from all MIPS configurations.
> 
> Fixes: 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is unused")
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Andrey Zhizhikin <andrey.zhizhikin@leica-geosystems.com>
> ---
>  arch/mips/configs/gcw0_defconfig      | 1 -
>  arch/mips/configs/gpr_defconfig       | 1 -
>  arch/mips/configs/lemote2f_defconfig  | 1 -
>  arch/mips/configs/loongson3_defconfig | 1 -
>  arch/mips/configs/mtx1_defconfig      | 1 -
>  arch/mips/configs/rs90_defconfig      | 1 -
>  6 files changed, 6 deletions(-)
> 

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH 2/5] arm64: defconfig: drop unused BACKLIGHT_GENERIC option
From: Krzysztof Kozlowski @ 2020-11-30 18:53 UTC (permalink / raw)
  To: Andrey Zhizhikin
  Cc: alexandre.belloni, tony, linux-kernel, James.Bottomley,
	thierry.reding, paulus, sam, daniel.thompson, linux-omap, deller,
	linux, jonathanh, ludovic.desroches, catalin.marinas, linux-mips,
	will, mripard, linux-tegra, lee.jones, wens, linux-arm-kernel,
	jernej.skrabec, tsbogend, linux-parisc, emil.l.velikov,
	nicolas.ferre, linuxppc-dev
In-Reply-To: <20201130152137.24909-3-andrey.zhizhikin@leica-geosystems.com>

On Mon, Nov 30, 2020 at 03:21:34PM +0000, Andrey Zhizhikin wrote:
> Commit 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> unused") removed geenric_bl driver from the tree, together with
> corresponding config option.
> 
> Remove BACKLIGHT_GENERIC config item from arm64 configuration.
> 
> Fixes: 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is unused")
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Andrey Zhizhikin <andrey.zhizhikin@leica-geosystems.com>
> ---
>  arch/arm64/configs/defconfig | 1 -
>  1 file changed, 1 deletion(-)
> 

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

The same trouble as with ARM patch - this should go directly via
arm-soc.

Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH 1/5] ARM: configs: drop unused BACKLIGHT_GENERIC option
From: Krzysztof Kozlowski @ 2020-11-30 18:52 UTC (permalink / raw)
  To: Andrey Zhizhikin
  Cc: alexandre.belloni, tony, linux-kernel, James.Bottomley,
	thierry.reding, paulus, sam, daniel.thompson, linux-omap,
	Arnd Bergmann, deller, linux, jonathanh, ludovic.desroches,
	catalin.marinas, linux-mips, will, mripard, linux-tegra,
	lee.jones, wens, linux-arm-kernel, jernej.skrabec, tsbogend,
	linux-parisc, emil.l.velikov, nicolas.ferre, Olof Johansson,
	linuxppc-dev
In-Reply-To: <20201130152137.24909-2-andrey.zhizhikin@leica-geosystems.com>

On Mon, Nov 30, 2020 at 03:21:33PM +0000, Andrey Zhizhikin wrote:
> Commit 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is
> unused") removed geenric_bl driver from the tree, together with
> corresponding config option.
> 
> Remove BACKLIGHT_GENERIC config item from all ARM configurations.
> 
> Fixes: 7ecdea4a0226 ("backlight: generic_bl: Remove this driver as it is unused")
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Andrey Zhizhikin <andrey.zhizhikin@leica-geosystems.com>
> ---
>  arch/arm/configs/at91_dt_defconfig        | 1 -
>  arch/arm/configs/cm_x300_defconfig        | 1 -
>  arch/arm/configs/colibri_pxa300_defconfig | 1 -
>  arch/arm/configs/jornada720_defconfig     | 1 -
>  arch/arm/configs/magician_defconfig       | 1 -
>  arch/arm/configs/mini2440_defconfig       | 1 -
>  arch/arm/configs/omap2plus_defconfig      | 1 -
>  arch/arm/configs/pxa3xx_defconfig         | 1 -
>  arch/arm/configs/qcom_defconfig           | 1 -
>  arch/arm/configs/sama5_defconfig          | 1 -
>  arch/arm/configs/sunxi_defconfig          | 1 -
>  arch/arm/configs/tegra_defconfig          | 1 -
>  arch/arm/configs/u8500_defconfig          | 1 -
>  13 files changed, 13 deletions(-)

You need to send it to arm-soc maintainers, otherwise no one might feel
responsible enough to pick it up.
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

+CC Arnd and Olof,

Dear Arnd and Olof,

Maybe it is worth to add arm-soc entry to the MAINTAINERS file?
Otherwise how one could get your email address? Not mentioning the
secret-soc address. :)

Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH 6/8] lazy tlb: shoot lazies, a non-refcounting lazy tlb option
From: Andy Lutomirski @ 2020-11-30 18:31 UTC (permalink / raw)
  To: Andy Lutomirski, Will Deacon, Catalin Marinas, Heiko Carstens,
	Vasily Gorbik, Christian Borntraeger, Dave Hansen
  Cc: linux-arch, Arnd Bergmann, Peter Zijlstra, X86 ML, LKML,
	Nicholas Piggin, Linux-MM, Mathieu Desnoyers, linuxppc-dev
In-Reply-To: <CALCETrWBtCfD+jZ3S+O8FK-HFPODuhbDEbbfWvS=-iPATNFAOA@mail.gmail.com>

other arch folk: there's some background here:

https://lkml.kernel.org/r/CALCETrVXUbe8LfNn-Qs+DzrOQaiw+sFUg1J047yByV31SaTOZw@mail.gmail.com

On Sun, Nov 29, 2020 at 12:16 PM Andy Lutomirski <luto@kernel.org> wrote:
>
> On Sat, Nov 28, 2020 at 7:54 PM Andy Lutomirski <luto@kernel.org> wrote:
> >
> > On Sat, Nov 28, 2020 at 8:02 AM Nicholas Piggin <npiggin@gmail.com> wrote:
> > >
> > > On big systems, the mm refcount can become highly contented when doing
> > > a lot of context switching with threaded applications (particularly
> > > switching between the idle thread and an application thread).
> > >
> > > Abandoning lazy tlb slows switching down quite a bit in the important
> > > user->idle->user cases, so so instead implement a non-refcounted scheme
> > > that causes __mmdrop() to IPI all CPUs in the mm_cpumask and shoot down
> > > any remaining lazy ones.
> > >
> > > Shootdown IPIs are some concern, but they have not been observed to be
> > > a big problem with this scheme (the powerpc implementation generated
> > > 314 additional interrupts on a 144 CPU system during a kernel compile).
> > > There are a number of strategies that could be employed to reduce IPIs
> > > if they turn out to be a problem for some workload.
> >
> > I'm still wondering whether we can do even better.
> >
>
> Hold on a sec.. __mmput() unmaps VMAs, frees pagetables, and flushes
> the TLB.  On x86, this will shoot down all lazies as long as even a
> single pagetable was freed.  (Or at least it will if we don't have a
> serious bug, but the code seems okay.  We'll hit pmd_free_tlb, which
> sets tlb->freed_tables, which will trigger the IPI.)  So, on
> architectures like x86, the shootdown approach should be free.  The
> only way it ought to have any excess IPIs is if we have CPUs in
> mm_cpumask() that don't need IPI to free pagetables, which could
> happen on paravirt.

Indeed, on x86, we do this:

[   11.558844]  flush_tlb_mm_range.cold+0x18/0x1d
[   11.559905]  tlb_finish_mmu+0x10e/0x1a0
[   11.561068]  exit_mmap+0xc8/0x1a0
[   11.561932]  mmput+0x29/0xd0
[   11.562688]  do_exit+0x316/0xa90
[   11.563588]  do_group_exit+0x34/0xb0
[   11.564476]  __x64_sys_exit_group+0xf/0x10
[   11.565512]  do_syscall_64+0x34/0x50

and we have info->freed_tables set.

What are the architectures that have large systems like?

x86: we already zap lazies, so it should cost basically nothing to do
a little loop at the end of __mmput() to make sure that no lazies are
left.  If we care about paravirt performance, we could implement one
of the optimizations I mentioned above to fix up the refcounts instead
of sending an IPI to any remaining lazies.

arm64: AFAICT arm64's flush uses magic arm64 hardware support for
remote flushes, so any lazy mm references will still exist after
exit_mmap().  (arm64 uses lazy TLB, right?)  So this is kind of like
the x86 paravirt case.  Are there large enough arm64 systems that any
of this matters?

s390x: The code has too many acronyms for me to understand it fully,
but I think it's more or less the same situation as arm64.  How big do
s390x systems come?

power: Ridiculously complicated, seems to vary by system and kernel config.

So, Nick, your unconditional IPI scheme is apparently a big
improvement for power, and it should be an improvement and have low
cost for x86.  On arm64 and s390x it will add more IPIs on process
exit but reduce contention on context switching depending on how lazy
TLB works.  I suppose we could try it for all architectures without
any further optimizations.  Or we could try one of the perhaps
excessively clever improvements I linked above.  arm64, s390x people,
what do you think?

^ permalink raw reply

* Re: [PATCH v6 1/5] PCI: Unify ECAM constants in native PCI Express drivers
From: Derrick, Jonathan @ 2020-11-30 18:23 UTC (permalink / raw)
  To: kw@linux.com, bhelgaas@google.com
  Cc: heiko@sntech.de, linux-pci@vger.kernel.org,
	shawn.lin@rock-chips.com, paulus@samba.org,
	thomas.petazzoni@bootlin.com, jonnyc@amazon.com,
	toan@os.amperecomputing.com, will@kernel.org, robh@kernel.org,
	lorenzo.pieralisi@arm.com, michal.simek@xilinx.com,
	linux-rockchip@lists.infradead.org,
	bcm-kernel-feedback-list@broadcom.com, rjui@broadcom.com,
	f.fainelli@gmail.com, linux-rpi-kernel@lists.infradead.org,
	Jonathan.Cameron@huawei.com, linux-arm-kernel@lists.infradead.org,
	sbranden@broadcom.com, wangzhou1@hisilicon.com,
	rrichter@marvell.com, linuxppc-dev@lists.ozlabs.org,
	nsaenzjulienne@suse.de
In-Reply-To: <20201129230743.3006978-2-kw@linux.com>

On Sun, 2020-11-29 at 23:07 +0000, Krzysztof Wilczyński wrote:
> Add ECAM-related constants to provide a set of standard constants
> defining memory address shift values to the byte-level address that can
> be used to access the PCI Express Configuration Space, and then move
> native PCI Express controller drivers to use the newly introduced
> definitions retiring driver-specific ones.
> 
> Refactor pci_ecam_map_bus() function to use newly added constants so
> that limits to the bus, device function and offset (now limited to 4K as
> per the specification) are in place to prevent the defective or
> malicious caller from supplying incorrect configuration offset and thus
> targeting the wrong device when accessing extended configuration space.
> This refactor also allows for the ".bus_shit" initialisers to be dropped
> when the user is not using a custom value as a default value will be
> used as per the PCI Express Specification.
> 
> Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
> ---
>  drivers/pci/controller/dwc/pcie-al.c        | 12 ++-------
>  drivers/pci/controller/dwc/pcie-hisi.c      |  2 --
>  drivers/pci/controller/pci-aardvark.c       | 13 +++-------
>  drivers/pci/controller/pci-host-generic.c   |  1 -
>  drivers/pci/controller/pci-thunder-ecam.c   |  1 -
>  drivers/pci/controller/pcie-brcmstb.c       | 16 ++----------
>  drivers/pci/controller/pcie-rockchip-host.c | 27 ++++++++++-----------
>  drivers/pci/controller/pcie-rockchip.h      |  8 +-----
>  drivers/pci/controller/pcie-tango.c         |  1 -
>  drivers/pci/controller/pcie-xilinx-nwl.c    |  9 ++-----
>  drivers/pci/controller/pcie-xilinx.c        | 11 ++-------
>  drivers/pci/controller/vmd.c                | 11 ++++-----
>  drivers/pci/ecam.c                          | 23 ++++++++++++------
>  include/linux/pci-ecam.h                    | 27 +++++++++++++++++++++
>  14 files changed, 73 insertions(+), 89 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-al.c b/drivers/pci/controller/dwc/pcie-al.c
> index f973fbca90cf..af9e51ab1af8 100644
> --- a/drivers/pci/controller/dwc/pcie-al.c
> +++ b/drivers/pci/controller/dwc/pcie-al.c
> @@ -76,7 +76,6 @@ static int al_pcie_init(struct pci_config_window *cfg)
>  }
>  
>  const struct pci_ecam_ops al_pcie_ops = {
> -	.bus_shift    = 20,
>  	.init         =  al_pcie_init,
>  	.pci_ops      = {
>  		.map_bus    = al_pcie_map_bus,
> @@ -138,8 +137,6 @@ struct al_pcie {
>  	struct al_pcie_target_bus_cfg target_bus_cfg;
>  };
>  
> -#define PCIE_ECAM_DEVFN(x)		(((x) & 0xff) << 12)
> -
>  #define to_al_pcie(x)		dev_get_drvdata((x)->dev)
>  
>  static inline u32 al_pcie_controller_readl(struct al_pcie *pcie, u32 offset)
> @@ -226,11 +223,6 @@ static void __iomem *al_pcie_conf_addr_map_bus(struct pci_bus *bus,
>  	struct al_pcie_target_bus_cfg *target_bus_cfg = &pcie->target_bus_cfg;
>  	unsigned int busnr_ecam = busnr & target_bus_cfg->ecam_mask;
>  	unsigned int busnr_reg = busnr & target_bus_cfg->reg_mask;
> -	void __iomem *pci_base_addr;
> -
> -	pci_base_addr = (void __iomem *)((uintptr_t)pp->va_cfg0_base +
> -					 (busnr_ecam << 20) +
> -					 PCIE_ECAM_DEVFN(devfn));
>  
>  	if (busnr_reg != target_bus_cfg->reg_val) {
>  		dev_dbg(pcie->pci->dev, "Changing target bus busnum val from 0x%x to 0x%x\n",
> @@ -241,7 +233,7 @@ static void __iomem *al_pcie_conf_addr_map_bus(struct pci_bus *bus,
>  				       target_bus_cfg->reg_mask);
>  	}
>  
> -	return pci_base_addr + where;
> +	return pp->va_cfg0_base + PCIE_ECAM_OFFSET(busnr_ecam, devfn, where);
>  }
>  
>  static struct pci_ops al_child_pci_ops = {
> @@ -264,7 +256,7 @@ static void al_pcie_config_prepare(struct al_pcie *pcie)
>  
>  	target_bus_cfg = &pcie->target_bus_cfg;
>  
> -	ecam_bus_mask = (pcie->ecam_size >> 20) - 1;
> +	ecam_bus_mask = (pcie->ecam_size >> PCIE_ECAM_BUS_SHIFT) - 1;
>  	if (ecam_bus_mask > 255) {
>  		dev_warn(pcie->dev, "ECAM window size is larger than 256MB. Cutting off at 256\n");
>  		ecam_bus_mask = 255;
> diff --git a/drivers/pci/controller/dwc/pcie-hisi.c b/drivers/pci/controller/dwc/pcie-hisi.c
> index 5ca86796d43a..8fc5960faf28 100644
> --- a/drivers/pci/controller/dwc/pcie-hisi.c
> +++ b/drivers/pci/controller/dwc/pcie-hisi.c
> @@ -100,7 +100,6 @@ static int hisi_pcie_init(struct pci_config_window *cfg)
>  }
>  
>  const struct pci_ecam_ops hisi_pcie_ops = {
> -	.bus_shift    = 20,
>  	.init         =  hisi_pcie_init,
>  	.pci_ops      = {
>  		.map_bus    = hisi_pcie_map_bus,
> @@ -135,7 +134,6 @@ static int hisi_pcie_platform_init(struct pci_config_window *cfg)
>  }
>  
>  static const struct pci_ecam_ops hisi_pcie_platform_ops = {
> -	.bus_shift    = 20,
>  	.init         =  hisi_pcie_platform_init,
>  	.pci_ops      = {
>  		.map_bus    = hisi_pcie_map_bus,
> diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> index 0be485a25327..1043e54c73bd 100644
> --- a/drivers/pci/controller/pci-aardvark.c
> +++ b/drivers/pci/controller/pci-aardvark.c
> @@ -16,6 +16,7 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/pci.h>
> +#include <linux/pci-ecam.h>
>  #include <linux/init.h>
>  #include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
> @@ -164,14 +165,6 @@
>  #define PCIE_CONFIG_WR_TYPE0			0xa
>  #define PCIE_CONFIG_WR_TYPE1			0xb
>  
> -#define PCIE_CONF_BUS(bus)			(((bus) & 0xff) << 20)
> -#define PCIE_CONF_DEV(dev)			(((dev) & 0x1f) << 15)
> -#define PCIE_CONF_FUNC(fun)			(((fun) & 0x7)	<< 12)
> -#define PCIE_CONF_REG(reg)			((reg) & 0xffc)
> -#define PCIE_CONF_ADDR(bus, devfn, where)	\
> -	(PCIE_CONF_BUS(bus) | PCIE_CONF_DEV(PCI_SLOT(devfn))	| \
> -	 PCIE_CONF_FUNC(PCI_FUNC(devfn)) | PCIE_CONF_REG(where))
> -
>  #define PIO_RETRY_CNT			500
>  #define PIO_RETRY_DELAY			2 /* 2 us*/
>  
> @@ -687,7 +680,7 @@ static int advk_pcie_rd_conf(struct pci_bus *bus, u32 devfn,
>  	advk_writel(pcie, reg, PIO_CTRL);
>  
>  	/* Program the address registers */
> -	reg = PCIE_CONF_ADDR(bus->number, devfn, where);
> +	reg = ALIGN_DOWN(PCIE_ECAM_OFFSET(bus->number, devfn, where), 4);
>  	advk_writel(pcie, reg, PIO_ADDR_LS);
>  	advk_writel(pcie, 0, PIO_ADDR_MS);
>  
> @@ -748,7 +741,7 @@ static int advk_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
>  	advk_writel(pcie, reg, PIO_CTRL);
>  
>  	/* Program the address registers */
> -	reg = PCIE_CONF_ADDR(bus->number, devfn, where);
> +	reg = ALIGN_DOWN(PCIE_ECAM_OFFSET(bus->number, devfn, where), 4);
>  	advk_writel(pcie, reg, PIO_ADDR_LS);
>  	advk_writel(pcie, 0, PIO_ADDR_MS);
>  
> diff --git a/drivers/pci/controller/pci-host-generic.c b/drivers/pci/controller/pci-host-generic.c
> index b51977abfdf1..63865aeb636b 100644
> --- a/drivers/pci/controller/pci-host-generic.c
> +++ b/drivers/pci/controller/pci-host-generic.c
> @@ -49,7 +49,6 @@ static void __iomem *pci_dw_ecam_map_bus(struct pci_bus *bus,
>  }
>  
>  static const struct pci_ecam_ops pci_dw_ecam_bus_ops = {
> -	.bus_shift	= 20,
>  	.pci_ops	= {
>  		.map_bus	= pci_dw_ecam_map_bus,
>  		.read		= pci_generic_config_read,
> diff --git a/drivers/pci/controller/pci-thunder-ecam.c b/drivers/pci/controller/pci-thunder-ecam.c
> index 7e8835fee5f7..f964fd26f7e0 100644
> --- a/drivers/pci/controller/pci-thunder-ecam.c
> +++ b/drivers/pci/controller/pci-thunder-ecam.c
> @@ -346,7 +346,6 @@ static int thunder_ecam_config_write(struct pci_bus *bus, unsigned int devfn,
>  }
>  
>  const struct pci_ecam_ops pci_thunder_ecam_ops = {
> -	.bus_shift	= 20,
>  	.pci_ops	= {
>  		.map_bus        = pci_ecam_map_bus,
>  		.read           = thunder_ecam_config_read,
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index bea86899bd5d..7fc80fd6f13f 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -22,6 +22,7 @@
>  #include <linux/of_pci.h>
>  #include <linux/of_platform.h>
>  #include <linux/pci.h>
> +#include <linux/pci-ecam.h>
>  #include <linux/printk.h>
>  #include <linux/reset.h>
>  #include <linux/sizes.h>
> @@ -127,11 +128,7 @@
>  #define  MSI_INT_MASK_CLR		0x14
>  
>  #define PCIE_EXT_CFG_DATA				0x8000
> -
>  #define PCIE_EXT_CFG_INDEX				0x9000
> -#define  PCIE_EXT_BUSNUM_SHIFT				20
> -#define  PCIE_EXT_SLOT_SHIFT				15
> -#define  PCIE_EXT_FUNC_SHIFT				12
>  
>  #define  PCIE_RGR1_SW_INIT_1_PERST_MASK			0x1
>  #define  PCIE_RGR1_SW_INIT_1_PERST_SHIFT		0x0
> @@ -695,15 +692,6 @@ static bool brcm_pcie_link_up(struct brcm_pcie *pcie)
>  	return dla && plu;
>  }
>  
> -/* Configuration space read/write support */
> -static inline int brcm_pcie_cfg_index(int busnr, int devfn, int reg)
> -{
> -	return ((PCI_SLOT(devfn) & 0x1f) << PCIE_EXT_SLOT_SHIFT)
> -		| ((PCI_FUNC(devfn) & 0x07) << PCIE_EXT_FUNC_SHIFT)
> -		| (busnr << PCIE_EXT_BUSNUM_SHIFT)
> -		| (reg & ~3);
> -}
> -
>  static void __iomem *brcm_pcie_map_conf(struct pci_bus *bus, unsigned int devfn,
>  					int where)
>  {
> @@ -716,7 +704,7 @@ static void __iomem *brcm_pcie_map_conf(struct pci_bus *bus, unsigned int devfn,
>  		return PCI_SLOT(devfn) ? NULL : base + where;
>  
>  	/* For devices, write to the config space index register */
> -	idx = brcm_pcie_cfg_index(bus->number, devfn, 0);
> +	idx = PCIE_ECAM_OFFSET(bus->number, devfn, 0);
>  	writel(idx, pcie->base + PCIE_EXT_CFG_INDEX);
>  	return base + PCIE_EXT_CFG_DATA + where;
>  }
> diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
> index 9705059523a6..f1d08a1b1591 100644
> --- a/drivers/pci/controller/pcie-rockchip-host.c
> +++ b/drivers/pci/controller/pcie-rockchip-host.c
> @@ -157,12 +157,11 @@ static int rockchip_pcie_rd_other_conf(struct rockchip_pcie *rockchip,
>  				       struct pci_bus *bus, u32 devfn,
>  				       int where, int size, u32 *val)
>  {
> -	u32 busdev;
> +	void __iomem *addr;
>  
> -	busdev = PCIE_ECAM_ADDR(bus->number, PCI_SLOT(devfn),
> -				PCI_FUNC(devfn), where);
> +	addr = rockchip->reg_base + PCIE_ECAM_OFFSET(bus->number, devfn, where);
>  
> -	if (!IS_ALIGNED(busdev, size)) {
> +	if (!IS_ALIGNED((uintptr_t)addr, size)) {
>  		*val = 0;
>  		return PCIBIOS_BAD_REGISTER_NUMBER;
>  	}
> @@ -175,11 +174,11 @@ static int rockchip_pcie_rd_other_conf(struct rockchip_pcie *rockchip,
>  						AXI_WRAPPER_TYPE1_CFG);
>  
>  	if (size == 4) {
> -		*val = readl(rockchip->reg_base + busdev);
> +		*val = readl(addr);
>  	} else if (size == 2) {
> -		*val = readw(rockchip->reg_base + busdev);
> +		*val = readw(addr);
>  	} else if (size == 1) {
> -		*val = readb(rockchip->reg_base + busdev);
> +		*val = readb(addr);
>  	} else {
>  		*val = 0;
>  		return PCIBIOS_BAD_REGISTER_NUMBER;
> @@ -191,11 +190,11 @@ static int rockchip_pcie_wr_other_conf(struct rockchip_pcie *rockchip,
>  				       struct pci_bus *bus, u32 devfn,
>  				       int where, int size, u32 val)
>  {
> -	u32 busdev;
> +	void __iomem *addr;
>  
> -	busdev = PCIE_ECAM_ADDR(bus->number, PCI_SLOT(devfn),
> -				PCI_FUNC(devfn), where);
> -	if (!IS_ALIGNED(busdev, size))
> +	addr = rockchip->reg_base + PCIE_ECAM_OFFSET(bus->number, devfn, where);
> +
> +	if (!IS_ALIGNED((uintptr_t)addr, size))
>  		return PCIBIOS_BAD_REGISTER_NUMBER;
>  
>  	if (pci_is_root_bus(bus->parent))
> @@ -206,11 +205,11 @@ static int rockchip_pcie_wr_other_conf(struct rockchip_pcie *rockchip,
>  						AXI_WRAPPER_TYPE1_CFG);
>  
>  	if (size == 4)
> -		writel(val, rockchip->reg_base + busdev);
> +		writel(val, addr);
>  	else if (size == 2)
> -		writew(val, rockchip->reg_base + busdev);
> +		writew(val, addr);
>  	else if (size == 1)
> -		writeb(val, rockchip->reg_base + busdev);
> +		writeb(val, addr);
>  	else
>  		return PCIBIOS_BAD_REGISTER_NUMBER;
>  
> diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
> index c7d0178fc8c2..1650a5087450 100644
> --- a/drivers/pci/controller/pcie-rockchip.h
> +++ b/drivers/pci/controller/pcie-rockchip.h
> @@ -13,6 +13,7 @@
>  
>  #include <linux/kernel.h>
>  #include <linux/pci.h>
> +#include <linux/pci-ecam.h>
>  
>  /*
>   * The upper 16 bits of PCIE_CLIENT_CONFIG are a write mask for the lower 16
> @@ -178,13 +179,6 @@
>  #define MIN_AXI_ADDR_BITS_PASSED		8
>  #define PCIE_RC_SEND_PME_OFF			0x11960
>  #define ROCKCHIP_VENDOR_ID			0x1d87
> -#define PCIE_ECAM_BUS(x)			(((x) & 0xff) << 20)
> -#define PCIE_ECAM_DEV(x)			(((x) & 0x1f) << 15)
> -#define PCIE_ECAM_FUNC(x)			(((x) & 0x7) << 12)
> -#define PCIE_ECAM_REG(x)			(((x) & 0xfff) << 0)
> -#define PCIE_ECAM_ADDR(bus, dev, func, reg) \
> -	  (PCIE_ECAM_BUS(bus) | PCIE_ECAM_DEV(dev) | \
> -	   PCIE_ECAM_FUNC(func) | PCIE_ECAM_REG(reg))
>  #define PCIE_LINK_IS_L2(x) \
>  	(((x) & PCIE_CLIENT_DEBUG_LTSSM_MASK) == PCIE_CLIENT_DEBUG_LTSSM_L2)
>  #define PCIE_LINK_UP(x) \
> diff --git a/drivers/pci/controller/pcie-tango.c b/drivers/pci/controller/pcie-tango.c
> index d093a8ce4bb1..62a061f1d62e 100644
> --- a/drivers/pci/controller/pcie-tango.c
> +++ b/drivers/pci/controller/pcie-tango.c
> @@ -208,7 +208,6 @@ static int smp8759_config_write(struct pci_bus *bus, unsigned int devfn,
>  }
>  
>  static const struct pci_ecam_ops smp8759_ecam_ops = {
> -	.bus_shift	= 20,
>  	.pci_ops	= {
>  		.map_bus	= pci_ecam_map_bus,
>  		.read		= smp8759_config_read,
> diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
> index f3cf7d61924f..7f29c2fdcd51 100644
> --- a/drivers/pci/controller/pcie-xilinx-nwl.c
> +++ b/drivers/pci/controller/pcie-xilinx-nwl.c
> @@ -18,6 +18,7 @@
>  #include <linux/of_platform.h>
>  #include <linux/of_irq.h>
>  #include <linux/pci.h>
> +#include <linux/pci-ecam.h>
>  #include <linux/platform_device.h>
>  #include <linux/irqchip/chained_irq.h>
>  
> @@ -124,8 +125,6 @@
>  #define E_ECAM_CR_ENABLE		BIT(0)
>  #define E_ECAM_SIZE_LOC			GENMASK(20, 16)
>  #define E_ECAM_SIZE_SHIFT		16
> -#define ECAM_BUS_LOC_SHIFT		20
> -#define ECAM_DEV_LOC_SHIFT		12
>  #define NWL_ECAM_VALUE_DEFAULT		12
>  
>  #define CFG_DMA_REG_BAR			GENMASK(2, 0)
> @@ -240,15 +239,11 @@ static void __iomem *nwl_pcie_map_bus(struct pci_bus *bus, unsigned int devfn,
>  				      int where)
>  {
>  	struct nwl_pcie *pcie = bus->sysdata;
> -	int relbus;
>  
>  	if (!nwl_pcie_valid_device(bus, devfn))
>  		return NULL;
>  
> -	relbus = (bus->number << ECAM_BUS_LOC_SHIFT) |
> -			(devfn << ECAM_DEV_LOC_SHIFT);
> -
> -	return pcie->ecam_base + relbus + where;
> +	return pcie->ecam_base + PCIE_ECAM_OFFSET(bus->number, devfn, where);
>  }
>  
>  /* PCIe operations */
> diff --git a/drivers/pci/controller/pcie-xilinx.c b/drivers/pci/controller/pcie-xilinx.c
> index 8523be61bba5..fa5baeb82653 100644
> --- a/drivers/pci/controller/pcie-xilinx.c
> +++ b/drivers/pci/controller/pcie-xilinx.c
> @@ -21,6 +21,7 @@
>  #include <linux/of_platform.h>
>  #include <linux/of_irq.h>
>  #include <linux/pci.h>
> +#include <linux/pci-ecam.h>
>  #include <linux/platform_device.h>
>  
>  #include "../pci.h"
> @@ -86,10 +87,6 @@
>  /* Phy Status/Control Register definitions */
>  #define XILINX_PCIE_REG_PSCR_LNKUP	BIT(11)
>  
> -/* ECAM definitions */
> -#define ECAM_BUS_NUM_SHIFT		20
> -#define ECAM_DEV_NUM_SHIFT		12
> -
>  /* Number of MSI IRQs */
>  #define XILINX_NUM_MSI_IRQS		128
>  
> @@ -183,15 +180,11 @@ static void __iomem *xilinx_pcie_map_bus(struct pci_bus *bus,
>  					 unsigned int devfn, int where)
>  {
>  	struct xilinx_pcie_port *port = bus->sysdata;
> -	int relbus;
>  
>  	if (!xilinx_pcie_valid_device(bus, devfn))
>  		return NULL;
>  
> -	relbus = (bus->number << ECAM_BUS_NUM_SHIFT) |
> -		 (devfn << ECAM_DEV_NUM_SHIFT);
> -
> -	return port->reg_base + relbus + where;
> +	return port->reg_base + PCIE_ECAM_OFFSET(bus->number, devfn, where);
>  }
>  
>  /* PCIe operations */
> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index f375c21ceeb1..1361a79bd1e7 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -11,6 +11,7 @@
>  #include <linux/module.h>
>  #include <linux/msi.h>
>  #include <linux/pci.h>
> +#include <linux/pci-ecam.h>
>  #include <linux/srcu.h>
>  #include <linux/rculist.h>
>  #include <linux/rcupdate.h>
> @@ -328,15 +329,13 @@ static void vmd_remove_irq_domain(struct vmd_dev *vmd)
>  static char __iomem *vmd_cfg_addr(struct vmd_dev *vmd, struct pci_bus *bus,
>  				  unsigned int devfn, int reg, int len)
>  {
> -	char __iomem *addr = vmd->cfgbar +
> -			     ((bus->number - vmd->busn_start) << 20) +
> -			     (devfn << 12) + reg;
> +	unsigned int busnr_ecam = bus->number - vmd->busn_start;
> +	u32 offset = PCIE_ECAM_OFFSET(busnr_ecam, devfn, reg);
>  
> -	if ((addr - vmd->cfgbar) + len >=
> -	    resource_size(&vmd->dev->resource[VMD_CFGBAR]))
> +	if (offset + len >= resource_size(&vmd->dev->resource[VMD_CFGBAR]))
>  		return NULL;
>  
> -	return addr;
> +	return vmd->cfgbar + offset;
>  }
>  

For vmd.c:
Reviewed-by: Jon Derrick <jonathan.derrick@intel.com>

Thanks


>  /*
> diff --git a/drivers/pci/ecam.c b/drivers/pci/ecam.c
> index b54d32a31669..59f91d434859 100644
> --- a/drivers/pci/ecam.c
> +++ b/drivers/pci/ecam.c
> @@ -131,25 +131,36 @@ void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn,
>  			       int where)
>  {
>  	struct pci_config_window *cfg = bus->sysdata;
> +	unsigned int bus_shift = cfg->ops->bus_shift;
>  	unsigned int devfn_shift = cfg->ops->bus_shift - 8;
>  	unsigned int busn = bus->number;
>  	void __iomem *base;
> +	u32 bus_offset, devfn_offset;
>  
>  	if (busn < cfg->busr.start || busn > cfg->busr.end)
>  		return NULL;
>  
>  	busn -= cfg->busr.start;
> -	if (per_bus_mapping)
> +	if (per_bus_mapping) {
>  		base = cfg->winp[busn];
> -	else
> -		base = cfg->win + (busn << cfg->ops->bus_shift);
> -	return base + (devfn << devfn_shift) + where;
> +		busn = 0;
> +	} else
> +		base = cfg->win;
> +
> +	if (cfg->ops->bus_shift) {
> +		bus_offset = (busn & PCIE_ECAM_BUS_MASK) << bus_shift;
> +		devfn_offset = (devfn & PCIE_ECAM_DEVFN_MASK) << devfn_shift;
> +		where &= PCIE_ECAM_REG_MASK;
> +
> +		return base + (bus_offset | devfn_offset | where);
> +	}
> +
> +	return base + PCIE_ECAM_OFFSET(busn, devfn, where);
>  }
>  EXPORT_SYMBOL_GPL(pci_ecam_map_bus);
>  
>  /* ECAM ops */
>  const struct pci_ecam_ops pci_generic_ecam_ops = {
> -	.bus_shift	= 20,
>  	.pci_ops	= {
>  		.map_bus	= pci_ecam_map_bus,
>  		.read		= pci_generic_config_read,
> @@ -161,7 +172,6 @@ EXPORT_SYMBOL_GPL(pci_generic_ecam_ops);
>  #if defined(CONFIG_ACPI) && defined(CONFIG_PCI_QUIRKS)
>  /* ECAM ops for 32-bit access only (non-compliant) */
>  const struct pci_ecam_ops pci_32b_ops = {
> -	.bus_shift	= 20,
>  	.pci_ops	= {
>  		.map_bus	= pci_ecam_map_bus,
>  		.read		= pci_generic_config_read32,
> @@ -171,7 +181,6 @@ const struct pci_ecam_ops pci_32b_ops = {
>  
>  /* ECAM ops for 32-bit read only (non-compliant) */
>  const struct pci_ecam_ops pci_32b_read_ops = {
> -	.bus_shift	= 20,
>  	.pci_ops	= {
>  		.map_bus	= pci_ecam_map_bus,
>  		.read		= pci_generic_config_read32,
> diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
> index 033ce74f02e8..65d3d83015c3 100644
> --- a/include/linux/pci-ecam.h
> +++ b/include/linux/pci-ecam.h
> @@ -9,6 +9,33 @@
>  #include <linux/kernel.h>
>  #include <linux/platform_device.h>
>  
> +/*
> + * Memory address shift values for the byte-level address that
> + * can be used when accessing the PCI Express Configuration Space.
> + */
> +
> +/*
> + * Enhanced Configuration Access Mechanism (ECAM)
> + *
> + * See PCI Express Base Specification, Revision 5.0, Version 1.0,
> + * Section 7.2.2, Table 7-1, p. 677.
> + */
> +#define PCIE_ECAM_BUS_SHIFT	20 /* Bus number */
> +#define PCIE_ECAM_DEVFN_SHIFT	12 /* Device and Function number */
> +
> +#define PCIE_ECAM_BUS_MASK	0xff
> +#define PCIE_ECAM_DEVFN_MASK	0xff
> +#define PCIE_ECAM_REG_MASK	0xfff /* Limit offset to a maximum of 4K */
> +
> +#define PCIE_ECAM_BUS(x)	(((x) & PCIE_ECAM_BUS_MASK) << PCIE_ECAM_BUS_SHIFT)
> +#define PCIE_ECAM_DEVFN(x)	(((x) & PCIE_ECAM_DEVFN_MASK) << PCIE_ECAM_DEVFN_SHIFT)
> +#define PCIE_ECAM_REG(x)	((x) & PCIE_ECAM_REG_MASK)
> +
> +#define PCIE_ECAM_OFFSET(bus, devfn, where) \
> +	(PCIE_ECAM_BUS(bus) | \
> +	 PCIE_ECAM_DEVFN(devfn) | \
> +	 PCIE_ECAM_REG(where))
> +
>  /*
>   * struct to hold pci ops and bus shift of the config window
>   * for a PCI controller.

^ permalink raw reply

* Re: [PATCH v6 4/5] PCI: vmd: Update type of the __iomem pointers
From: Derrick, Jonathan @ 2020-11-30 18:19 UTC (permalink / raw)
  To: helgaas@kernel.org, David.Laight@ACULAB.COM
  Cc: kw@linux.com, heiko@sntech.de, linux-pci@vger.kernel.org,
	shawn.lin@rock-chips.com, paulus@samba.org,
	thomas.petazzoni@bootlin.com, jonnyc@amazon.com,
	toan@os.amperecomputing.com, will@kernel.org, robh@kernel.org,
	f.fainelli@gmail.com, michal.simek@xilinx.com,
	linux-rockchip@lists.infradead.org,
	bcm-kernel-feedback-list@broadcom.com, rjui@broadcom.com,
	lorenzo.pieralisi@arm.com, linux-rpi-kernel@lists.infradead.org,
	Jonathan.Cameron@huawei.com, bhelgaas@google.com,
	linux-arm-kernel@lists.infradead.org, sbranden@broadcom.com,
	wangzhou1@hisilicon.com, rrichter@marvell.com,
	linuxppc-dev@lists.ozlabs.org, nsaenzjulienne@suse.de
In-Reply-To: <20201130172058.GA1088391@bjorn-Precision-5520>

On Mon, 2020-11-30 at 11:20 -0600, Bjorn Helgaas wrote:
> On Mon, Nov 30, 2020 at 09:06:56AM +0000, David Laight wrote:
> > From: Krzysztof Wilczynski
> > > Sent: 29 November 2020 23:08
> > > 
> > > Use "void __iomem" instead "char __iomem" pointer type when working with
> > > the accessor functions (with names like readb() or writel(), etc.) to
> > > better match a given accessor function signature where commonly the
> > > address pointing to an I/O memory region would be a "void __iomem"
> > > pointer.
> > 
> > ISTM that is heading in the wrong direction.
> > 
> > I think (form the variable names etc) that these are pointers
> > to specific registers.
> > 
> > So what you ought to have is a type for that register block.
> > Typically this is actually a structure - to give some type
> > checking that the offsets are being used with the correct
> > base address.
> 
> In this case, "cfgbar" is not really a pointer to a register; it's the
> address of memory-mapped config space.  The VMD hardware turns
> accesses to that space into PCI config transactions on its secondary
> side.  xgene_pcie_get_cfg_base() and brcm_pcie_map_conf() are similar
> situations and use "void *".
> 
> Bjorn

Yes it's just the passthrough window for PCI config bus ops.

Reviewed-by: Jon Derrick <jonathan.derrick@intel.com>

^ permalink raw reply

* Re: [PATCH 09/13] ibmvfc: implement channel enquiry and setup commands
From: Tyrel Datwyler @ 2020-11-30 17:29 UTC (permalink / raw)
  To: Brian King, james.bottomley
  Cc: brking, linuxppc-dev, linux-scsi, martin.petersen, linux-kernel
In-Reply-To: <5f873855-fdc2-4da4-a516-4db7b5236a48@linux.vnet.ibm.com>

On 11/27/20 9:49 AM, Brian King wrote:
> On 11/25/20 7:48 PM, Tyrel Datwyler wrote:
>> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
>> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
> 
>> @@ -4462,6 +4464,118 @@ static void ibmvfc_discover_targets(struct ibmvfc_host *vhost)
>>  		ibmvfc_link_down(vhost, IBMVFC_LINK_DEAD);
>>  }
>>  
>> +static void ibmvfc_channel_setup_done(struct ibmvfc_event *evt)
>> +{
>> +	struct ibmvfc_host *vhost = evt->vhost;
>> +	u32 mad_status = be16_to_cpu(evt->xfer_iu->channel_setup.common.status);
>> +	int level = IBMVFC_DEFAULT_LOG_LEVEL;
>> +
>> +	ibmvfc_free_event(evt);
>> +
>> +	switch (mad_status) {
>> +	case IBMVFC_MAD_SUCCESS:
>> +		ibmvfc_dbg(vhost, "Channel Setup succeded\n");
>> +		vhost->do_enquiry = 0;
>> +		break;
>> +	case IBMVFC_MAD_FAILED:
>> +		level += ibmvfc_retry_host_init(vhost);
>> +		ibmvfc_log(vhost, level, "Channel Setup failed\n");
>> +		fallthrough;
>> +	case IBMVFC_MAD_DRIVER_FAILED:
>> +		return;
>> +	default:
>> +		dev_err(vhost->dev, "Invalid Channel Setup response: 0x%x\n",
>> +			mad_status);
>> +		ibmvfc_link_down(vhost, IBMVFC_LINK_DEAD);
>> +		return;
>> +	}
>> +
>> +	ibmvfc_set_host_action(vhost, IBMVFC_HOST_ACTION_QUERY);
>> +	wake_up(&vhost->work_wait_q);
>> +}
>> +
>> +static void ibmvfc_channel_setup(struct ibmvfc_host *vhost)
>> +{
>> +	struct ibmvfc_channel_setup_mad *mad;
>> +	struct ibmvfc_channel_setup *setup_buf = vhost->channel_setup_buf;
>> +	struct ibmvfc_event *evt = ibmvfc_get_event(vhost);
>> +
>> +	memset(setup_buf, 0, sizeof(*setup_buf));
>> +	setup_buf->flags = cpu_to_be32(IBMVFC_CANCEL_CHANNELS);
>> +
>> +	ibmvfc_init_event(evt, ibmvfc_channel_setup_done, IBMVFC_MAD_FORMAT);
>> +	mad = &evt->iu.channel_setup;
>> +	memset(mad, 0, sizeof(*mad));
>> +	mad->common.version = cpu_to_be32(1);
>> +	mad->common.opcode = cpu_to_be32(IBMVFC_CHANNEL_SETUP);
>> +	mad->common.length = cpu_to_be16(sizeof(*mad));
>> +	mad->buffer.va = cpu_to_be64(vhost->channel_setup_dma);
>> +	mad->buffer.len = cpu_to_be32(sizeof(*vhost->channel_setup_buf));
>> +
>> +	ibmvfc_set_host_action(vhost, IBMVFC_HOST_ACTION_INIT_WAIT);
>> +
>> +	if (!ibmvfc_send_event(evt, vhost, default_timeout))
>> +		ibmvfc_dbg(vhost, "Sent channel setup\n");
>> +	else
>> +		ibmvfc_link_down(vhost, IBMVFC_LINK_DOWN);
>> +}
>> +
>> +static void ibmvfc_channel_enquiry_done(struct ibmvfc_event *evt)
>> +{
>> +	struct ibmvfc_host *vhost = evt->vhost;
>> +	struct ibmvfc_channel_enquiry *rsp = &evt->xfer_iu->channel_enquiry;
>> +	u32 mad_status = be16_to_cpu(rsp->common.status);
>> +	int level = IBMVFC_DEFAULT_LOG_LEVEL;
>> +
>> +	switch (mad_status) {
>> +	case IBMVFC_MAD_SUCCESS:
>> +		ibmvfc_dbg(vhost, "Channel Enquiry succeeded\n");
>> +		vhost->max_vios_scsi_channels = be32_to_cpu(rsp->num_scsi_subq_channels);
> 
> You need a ibmvfc_free_event(evt) here so you don't leak events.
> 

Indeed

>> +		break;
>> +	case IBMVFC_MAD_FAILED:
>> +		level += ibmvfc_retry_host_init(vhost);
>> +		ibmvfc_log(vhost, level, "Channel Enquiry failed\n");
>> +		ibmvfc_free_event(evt);
> 
> Looks like you are freeing this event twice due to the fallthrough...

Good catch

> 
>> +		fallthrough;
>> +	case IBMVFC_MAD_DRIVER_FAILED:
>> +		ibmvfc_free_event(evt);
>> +		return;
>> +	default:
>> +		dev_err(vhost->dev, "Invalid Channel Enquiry response: 0x%x\n",
>> +			mad_status);
>> +		ibmvfc_link_down(vhost, IBMVFC_LINK_DEAD);
>> +		ibmvfc_free_event(evt);
>> +		return;
>> +	}
>> +
>> +	ibmvfc_channel_setup(vhost);
>> +}
>> +
> 
> 
> 


^ permalink raw reply

* Re: [PATCH 06/13] ibmvfc: add handlers to drain and complete Sub-CRQ responses
From: Tyrel Datwyler @ 2020-11-30 17:27 UTC (permalink / raw)
  To: Brian King, james.bottomley
  Cc: brking, linuxppc-dev, linux-scsi, martin.petersen, linux-kernel
In-Reply-To: <b3737660-4e13-8675-b4be-71283e2dcf99@linux.vnet.ibm.com>

On 11/27/20 9:47 AM, Brian King wrote:
> On 11/25/20 7:48 PM, Tyrel Datwyler wrote:
>> The logic for iterating over the Sub-CRQ responses is similiar to that
>> of the primary CRQ. Add the necessary handlers for processing those
>> responses.
>>
>> Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
>> ---
>>  drivers/scsi/ibmvscsi/ibmvfc.c | 72 ++++++++++++++++++++++++++++++++++
>>  1 file changed, 72 insertions(+)
>>
>> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
>> index 6eaedda4917a..a8730522920e 100644
>> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
>> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
>> @@ -3371,6 +3371,78 @@ static int ibmvfc_toggle_scrq_irq(struct ibmvfc_sub_queue *scrq, int enable)
>>  	return rc;
>>  }
>>  
>> +static void ibmvfc_handle_scrq(struct ibmvfc_crq *crq, struct ibmvfc_host *vhost)
>> +{
>> +	struct ibmvfc_event *evt = (struct ibmvfc_event *)be64_to_cpu(crq->ioba);
>> +
>> +	switch (crq->valid) {
>> +	case IBMVFC_CRQ_CMD_RSP:
>> +		break;
>> +	default:
>> +		dev_err(vhost->dev, "Got and invalid message type 0x%02x\n", crq->valid);
> 
> Is this correct? Can't we get transport events here as well?

Yes we can. We still handle them in the primary CRQ so at least for the time
being we can ignore them, but yeah we shouldn't log scary messages about them.

-Tyrel

> 
>> +		return;
>> +	}
>> +
>> +	/* The only kind of payload CRQs we should get are responses to
>> +	 * things we send. Make sure this response is to something we
>> +	 * actually sent
>> +	 */
>> +	if (unlikely(!ibmvfc_valid_event(&vhost->pool, evt))) {
>> +		dev_err(vhost->dev, "Returned correlation_token 0x%08llx is invalid!\n",
>> +			crq->ioba);
>> +		return;
>> +	}
>> +
>> +	if (unlikely(atomic_read(&evt->free))) {
>> +		dev_err(vhost->dev, "Received duplicate correlation_token 0x%08llx!\n",
>> +			crq->ioba);
>> +		return;
>> +	}
>> +
>> +	del_timer(&evt->timer);
>> +	list_del(&evt->queue);
>> +	ibmvfc_trc_end(evt);
>> +	evt->done(evt);
>> +}
>> +
> 
> 
> 


^ permalink raw reply

* Re: [PATCH 04/13] ibmvfc: add alloc/dealloc routines for SCSI Sub-CRQ Channels
From: Tyrel Datwyler @ 2020-11-30 17:26 UTC (permalink / raw)
  To: Brian King, james.bottomley
  Cc: brking, linuxppc-dev, linux-scsi, martin.petersen, linux-kernel
In-Reply-To: <0c308b76-c744-0257-d5ba-3ffd0e6073a3@linux.vnet.ibm.com>

On 11/27/20 9:46 AM, Brian King wrote:
> On 11/25/20 7:48 PM, Tyrel Datwyler wrote:
>> Allocate a set of Sub-CRQs in advance. During channel setup the client
>> and VIOS negotiate the number of queues the VIOS supports and the number
>> that the client desires to request. Its possible that the final channel
>> resources allocated is less than requested, but the client is still
>> responsible for sending handles for every queue it is hoping for.
>>
>> Also, provide deallocation cleanup routines.
>>
>> Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
>> ---
>>  drivers/scsi/ibmvscsi/ibmvfc.c | 115 +++++++++++++++++++++++++++++++++
>>  drivers/scsi/ibmvscsi/ibmvfc.h |   1 +
>>  2 files changed, 116 insertions(+)
>>
>> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
>> index 260b82e3cc01..571abdb48384 100644
>> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
>> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
>> @@ -4983,6 +4983,114 @@ static int ibmvfc_init_crq(struct ibmvfc_host *vhost)
>>  	return retrc;
>>  }
>>  
>> +static int ibmvfc_register_scsi_channel(struct ibmvfc_host *vhost,
>> +				  int index)
>> +{
>> +	struct device *dev = vhost->dev;
>> +	struct vio_dev *vdev = to_vio_dev(dev);
>> +	struct ibmvfc_sub_queue *scrq = &vhost->scsi_scrqs.scrqs[index];
>> +	int rc = -ENOMEM;
>> +
>> +	ENTER;
>> +
>> +	scrq->msgs = (struct ibmvfc_sub_crq *)get_zeroed_page(GFP_KERNEL);
>> +	if (!scrq->msgs)
>> +		return rc;
>> +
>> +	scrq->size = PAGE_SIZE / sizeof(*scrq->msgs);
>> +	scrq->msg_token = dma_map_single(dev, scrq->msgs, PAGE_SIZE,
>> +					 DMA_BIDIRECTIONAL);
>> +
>> +	if (dma_mapping_error(dev, scrq->msg_token))
>> +		goto dma_map_failed;
>> +
>> +	rc = h_reg_sub_crq(vdev->unit_address, scrq->msg_token, PAGE_SIZE,
>> +			   &scrq->cookie, &scrq->hw_irq);
>> +
>> +	if (rc) {
>> +		dev_warn(dev, "Error registering sub-crq: %d\n", rc);
>> +		dev_warn(dev, "Firmware may not support MQ\n");
>> +		goto reg_failed;
>> +	}
>> +
>> +	scrq->hwq_id = index;
>> +	scrq->vhost = vhost;
>> +
>> +	LEAVE;
>> +	return 0;
>> +
>> +reg_failed:
>> +	dma_unmap_single(dev, scrq->msg_token, PAGE_SIZE, DMA_BIDIRECTIONAL);
>> +dma_map_failed:
>> +	free_page((unsigned long)scrq->msgs);
>> +	LEAVE;
>> +	return rc;
>> +}
>> +
>> +static void ibmvfc_deregister_scsi_channel(struct ibmvfc_host *vhost, int index)
>> +{
>> +	struct device *dev = vhost->dev;
>> +	struct vio_dev *vdev = to_vio_dev(dev);
>> +	struct ibmvfc_sub_queue *scrq = &vhost->scsi_scrqs.scrqs[index];
>> +	long rc;
>> +
>> +	ENTER;
>> +
>> +	do {
>> +		rc = plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address,
>> +					scrq->cookie);
>> +	} while (rc == H_BUSY || H_IS_LONG_BUSY(rc));
>> +
>> +	if (rc)
>> +		dev_err(dev, "Failed to free sub-crq[%d]: rc=%ld\n", index, rc);
>> +
>> +	dma_unmap_single(dev, scrq->msg_token, PAGE_SIZE, DMA_BIDIRECTIONAL);
>> +	free_page((unsigned long)scrq->msgs);
>> +	LEAVE;
>> +}
>> +
>> +static int ibmvfc_init_sub_crqs(struct ibmvfc_host *vhost)
>> +{
>> +	int i, j;
>> +
>> +	ENTER;
>> +
>> +	vhost->scsi_scrqs.scrqs = kcalloc(vhost->client_scsi_channels,
>> +					  sizeof(*vhost->scsi_scrqs.scrqs),
>> +					  GFP_KERNEL);
>> +	if (!vhost->scsi_scrqs.scrqs)
>> +		return -1;
>> +
>> +	for (i = 0; i < vhost->client_scsi_channels; i++) {
>> +		if (ibmvfc_register_scsi_channel(vhost, i)) {
>> +			for (j = i; j > 0; j--)
>> +				ibmvfc_deregister_scsi_channel(vhost, j - 1);
>> +			kfree(vhost->scsi_scrqs.scrqs);
>> +			LEAVE;
>> +			return -1;
>> +		}
>> +	}
>> +
>> +	LEAVE;
>> +	return 0;
>> +}
>> +
>> +static void ibmvfc_release_sub_crqs(struct ibmvfc_host *vhost)
>> +{
>> +	int i;
>> +
>> +	ENTER;
>> +	if (!vhost->scsi_scrqs.scrqs)
>> +		return;
>> +
>> +	for (i = 0; i < vhost->client_scsi_channels; i++)
>> +		ibmvfc_deregister_scsi_channel(vhost, i);
>> +
>> +	vhost->scsi_scrqs.active_queues = 0;
>> +	kfree(vhost->scsi_scrqs.scrqs);
> 
> Do you want to NULL this out after you free it do you don't keep
> a reference to a freed page around?

This isn't actually a page, but a dynamically allocated array of
ibmvfc_sub_queues, but it should be NULL'ed regardless.

-Tyrel

> 
>> +	LEAVE;
>> +}
>> +
>>  /**
>>   * ibmvfc_free_mem - Free memory for vhost
>>   * @vhost:	ibmvfc host struct
> 
> 
> 


^ permalink raw reply

* Re: [PATCH 01/13] ibmvfc: add vhost fields and defaults for MQ enablement
From: Tyrel Datwyler @ 2020-11-30 17:22 UTC (permalink / raw)
  To: Brian King, james.bottomley
  Cc: brking, linuxppc-dev, linux-scsi, martin.petersen, linux-kernel
In-Reply-To: <97e577a0-50f5-3ade-a377-7479f0f1c890@linux.vnet.ibm.com>

On 11/27/20 9:45 AM, Brian King wrote:
> On 11/25/20 7:48 PM, Tyrel Datwyler wrote:
>> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.h b/drivers/scsi/ibmvscsi/ibmvfc.h
>> index 9d58cfd774d3..8225bdbb127e 100644
>> --- a/drivers/scsi/ibmvscsi/ibmvfc.h
>> +++ b/drivers/scsi/ibmvscsi/ibmvfc.h
>> @@ -41,6 +41,11 @@
>>  #define IBMVFC_DEFAULT_LOG_LEVEL	2
>>  #define IBMVFC_MAX_CDB_LEN		16
>>  #define IBMVFC_CLS3_ERROR		0
>> +#define IBMVFC_MQ			0
> 
> Given that IBMVFC_MQ is getting set to 0 here, that means mq_enabled is also
> always zero, so am I correct that a lot of this code being added is not
> yet capable of being executed?

Not with out a direct intervention from a hard coding a different value when
building the code. See comment below.

> 
>> +#define IBMVFC_SCSI_CHANNELS		0
> 
> Similar comment here...
> 
>> +#define IBMVFC_SCSI_HW_QUEUES		1
> 
> I don't see any subsequent patches in this series that would ever result
> in nr_hw_queues getting set to anything other than 1. Is that future work
> planned or am I missing something?

Yes, there is still some changes to EH that need to be included before those
values are safe to be set to anything else by the average user.

-Tyrel

> 
>> +#define IBMVFC_MIG_NO_SUB_TO_CRQ	0
>> +#define IBMVFC_MIG_NO_N_TO_M		0
>>  
>>  /*
>>   * Ensure we have resources for ERP and initialization:
>> @@ -826,6 +831,10 @@ struct ibmvfc_host {
>>  	int delay_init;
>>  	int scan_complete;
>>  	int logged_in;
>> +	int mq_enabled;
>> +	int using_channels;
>> +	int do_enquiry;
>> +	int client_scsi_channels;
>>  	int aborting_passthru;
>>  	int events_to_log;
>>  #define IBMVFC_AE_LINKUP	0x0001
>>
> 
> 


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox