* [PATCH v5 08/11] powerpc/pseries/iommu: Update remove_dma_window() to accept property name
From: Leonardo Bras @ 2021-07-16 8:27 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Leonardo Bras, Alexey Kardashevskiy, David Gibson,
kernel test robot, Nicolin Chen, Frederic Barrat
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20210716082755.428187-1-leobras.c@gmail.com>
Update remove_dma_window() so it can be used to remove DDW with a given
property name.
This enables the creation of new property names for DDW, so we can
have different usage for it, like indirect mapping.
Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
arch/powerpc/platforms/pseries/iommu.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index 108c3dcca686..17c6f4706e76 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -830,31 +830,32 @@ static void remove_dma_window(struct device_node *np, u32 *ddw_avail,
np, ret, ddw_avail[DDW_REMOVE_PE_DMA_WIN], liobn);
}
-static void remove_ddw(struct device_node *np, bool remove_prop)
+static int remove_ddw(struct device_node *np, bool remove_prop, const char *win_name)
{
struct property *win;
u32 ddw_avail[DDW_APPLICABLE_SIZE];
int ret = 0;
+ win = of_find_property(np, win_name, NULL);
+ if (!win)
+ return -EINVAL;
+
ret = of_property_read_u32_array(np, "ibm,ddw-applicable",
&ddw_avail[0], DDW_APPLICABLE_SIZE);
if (ret)
- return;
-
- win = of_find_property(np, DIRECT64_PROPNAME, NULL);
- if (!win)
- return;
+ return 0;
if (win->length >= sizeof(struct dynamic_dma_window_prop))
remove_dma_window(np, ddw_avail, win);
if (!remove_prop)
- return;
+ return 0;
ret = of_remove_property(np, win);
if (ret)
pr_warn("%pOF: failed to remove direct window property: %d\n",
np, ret);
+ return 0;
}
static bool find_existing_ddw(struct device_node *pdn, u64 *dma_addr, int *window_shift)
@@ -907,7 +908,7 @@ static int find_existing_ddw_windows(void)
for_each_node_with_property(pdn, DIRECT64_PROPNAME) {
direct64 = of_get_property(pdn, DIRECT64_PROPNAME, &len);
if (!direct64 || len < sizeof(*direct64)) {
- remove_ddw(pdn, true);
+ remove_ddw(pdn, true, DIRECT64_PROPNAME);
continue;
}
@@ -1382,7 +1383,7 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
kfree(win64);
out_remove_win:
- remove_ddw(pdn, true);
+ remove_ddw(pdn, true, DIRECT64_PROPNAME);
out_failed:
if (default_win_removed)
@@ -1547,7 +1548,7 @@ static int iommu_reconfig_notifier(struct notifier_block *nb, unsigned long acti
* we have to remove the property when releasing
* the device node.
*/
- remove_ddw(np, false);
+ remove_ddw(np, false, DIRECT64_PROPNAME);
if (pci && pci->table_group)
iommu_pseries_free_group(pci->table_group,
np->full_name);
--
2.32.0
^ permalink raw reply related
* [PATCH v5 07/11] powerpc/pseries/iommu: Reorganize iommu_table_setparms*() with new helper
From: Leonardo Bras @ 2021-07-16 8:27 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Leonardo Bras, Alexey Kardashevskiy, David Gibson,
kernel test robot, Nicolin Chen, Frederic Barrat
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20210716082755.428187-1-leobras.c@gmail.com>
Add a new helper _iommu_table_setparms(), and use it in
iommu_table_setparms() and iommu_table_setparms_lpar() to avoid duplicated
code.
Also, setting tbl->it_ops was happening outsite iommu_table_setparms*(),
so move it to the new helper. Since we need the iommu_table_ops to be
declared before used, declare iommu_table_lpar_multi_ops and
iommu_table_pseries_ops to before their respective iommu_table_setparms*().
Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
---
arch/powerpc/platforms/pseries/iommu.c | 72 ++++++++++++++------------
1 file changed, 38 insertions(+), 34 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index 7ca79a04fa52..108c3dcca686 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -501,6 +501,24 @@ static int tce_setrange_multi_pSeriesLP_walk(unsigned long start_pfn,
return tce_setrange_multi_pSeriesLP(start_pfn, num_pfn, arg);
}
+static void iommu_table_setparms_common(struct iommu_table *tbl, unsigned long busno,
+ unsigned long liobn, unsigned long win_addr,
+ unsigned long window_size, unsigned long page_shift,
+ void *base, struct iommu_table_ops *table_ops)
+{
+ tbl->it_busno = busno;
+ tbl->it_index = liobn;
+ tbl->it_offset = win_addr >> page_shift;
+ tbl->it_size = window_size >> page_shift;
+ tbl->it_page_shift = page_shift;
+ tbl->it_base = (unsigned long)base;
+ tbl->it_blocksize = 16;
+ tbl->it_type = TCE_PCI;
+ tbl->it_ops = table_ops;
+}
+
+struct iommu_table_ops iommu_table_pseries_ops;
+
static void iommu_table_setparms(struct pci_controller *phb,
struct device_node *dn,
struct iommu_table *tbl)
@@ -509,8 +527,13 @@ static void iommu_table_setparms(struct pci_controller *phb,
const unsigned long *basep;
const u32 *sizep;
- node = phb->dn;
+ /* Test if we are going over 2GB of DMA space */
+ if (phb->dma_window_base_cur + phb->dma_window_size > SZ_2G) {
+ udbg_printf("PCI_DMA: Unexpected number of IOAs under this PHB.\n");
+ panic("PCI_DMA: Unexpected number of IOAs under this PHB.\n");
+ }
+ node = phb->dn;
basep = of_get_property(node, "linux,tce-base", NULL);
sizep = of_get_property(node, "linux,tce-size", NULL);
if (basep == NULL || sizep == NULL) {
@@ -519,33 +542,18 @@ static void iommu_table_setparms(struct pci_controller *phb,
return;
}
- tbl->it_base = (unsigned long)__va(*basep);
+ iommu_table_setparms_common(tbl, phb->bus->number, 0, phb->dma_window_base_cur,
+ phb->dma_window_size, IOMMU_PAGE_SHIFT_4K,
+ __va(*basep), &iommu_table_pseries_ops);
if (!is_kdump_kernel())
memset((void *)tbl->it_base, 0, *sizep);
- tbl->it_busno = phb->bus->number;
- tbl->it_page_shift = IOMMU_PAGE_SHIFT_4K;
-
- /* Units of tce entries */
- tbl->it_offset = phb->dma_window_base_cur >> tbl->it_page_shift;
-
- /* Test if we are going over 2GB of DMA space */
- if (phb->dma_window_base_cur + phb->dma_window_size > 0x80000000ul) {
- udbg_printf("PCI_DMA: Unexpected number of IOAs under this PHB.\n");
- panic("PCI_DMA: Unexpected number of IOAs under this PHB.\n");
- }
-
phb->dma_window_base_cur += phb->dma_window_size;
-
- /* Set the tce table size - measured in entries */
- tbl->it_size = phb->dma_window_size >> tbl->it_page_shift;
-
- tbl->it_index = 0;
- tbl->it_blocksize = 16;
- tbl->it_type = TCE_PCI;
}
+struct iommu_table_ops iommu_table_lpar_multi_ops;
+
/*
* iommu_table_setparms_lpar
*
@@ -557,17 +565,13 @@ static void iommu_table_setparms_lpar(struct pci_controller *phb,
struct iommu_table_group *table_group,
const __be32 *dma_window)
{
- unsigned long offset, size;
+ unsigned long offset, size, liobn;
- of_parse_dma_window(dn, dma_window, &tbl->it_index, &offset, &size);
+ of_parse_dma_window(dn, dma_window, &liobn, &offset, &size);
+
+ iommu_table_setparms_common(tbl, phb->bus->number, liobn, offset, size, IOMMU_PAGE_SHIFT_4K, NULL,
+ &iommu_table_lpar_multi_ops);
- tbl->it_busno = phb->bus->number;
- tbl->it_page_shift = IOMMU_PAGE_SHIFT_4K;
- tbl->it_base = 0;
- tbl->it_blocksize = 16;
- tbl->it_type = TCE_PCI;
- tbl->it_offset = offset >> tbl->it_page_shift;
- tbl->it_size = size >> tbl->it_page_shift;
table_group->tce32_start = offset;
table_group->tce32_size = size;
@@ -647,7 +651,7 @@ static void pci_dma_bus_setup_pSeries(struct pci_bus *bus)
tbl = pci->table_group->tables[0];
iommu_table_setparms(pci->phb, dn, tbl);
- tbl->it_ops = &iommu_table_pseries_ops;
+
if (!iommu_init_table(tbl, pci->phb->node, 0, 0))
panic("Failed to initialize iommu table");
@@ -730,7 +734,7 @@ static void pci_dma_bus_setup_pSeriesLP(struct pci_bus *bus)
tbl = ppci->table_group->tables[0];
iommu_table_setparms_lpar(ppci->phb, pdn, tbl,
ppci->table_group, dma_window);
- tbl->it_ops = &iommu_table_lpar_multi_ops;
+
if (!iommu_init_table(tbl, ppci->phb->node, 0, 0))
panic("Failed to initialize iommu table");
iommu_register_group(ppci->table_group,
@@ -760,7 +764,7 @@ static void pci_dma_dev_setup_pSeries(struct pci_dev *dev)
PCI_DN(dn)->table_group = iommu_pseries_alloc_group(phb->node);
tbl = PCI_DN(dn)->table_group->tables[0];
iommu_table_setparms(phb, dn, tbl);
- tbl->it_ops = &iommu_table_pseries_ops;
+
if (!iommu_init_table(tbl, phb->node, 0, 0))
panic("Failed to initialize iommu table");
@@ -1443,7 +1447,7 @@ static void pci_dma_dev_setup_pSeriesLP(struct pci_dev *dev)
tbl = pci->table_group->tables[0];
iommu_table_setparms_lpar(pci->phb, pdn, tbl,
pci->table_group, dma_window);
- tbl->it_ops = &iommu_table_lpar_multi_ops;
+
iommu_init_table(tbl, pci->phb->node, 0, 0);
iommu_register_group(pci->table_group,
pci_domain_nr(pci->phb->bus), 0);
--
2.32.0
^ permalink raw reply related
* [PATCH v5 06/11] powerpc/pseries/iommu: Add ddw_property_create() and refactor enable_ddw()
From: Leonardo Bras @ 2021-07-16 8:27 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Leonardo Bras, Alexey Kardashevskiy, David Gibson,
kernel test robot, Nicolin Chen, Frederic Barrat
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20210716082755.428187-1-leobras.c@gmail.com>
Code used to create a ddw property that was previously scattered in
enable_ddw() is now gathered in ddw_property_create(), which deals with
allocation and filling the property, letting it ready for
of_property_add(), which now occurs in sequence.
This created an opportunity to reorganize the second part of enable_ddw():
Without this patch enable_ddw() does, in order:
kzalloc() property & members, create_ddw(), fill ddwprop inside property,
ddw_list_new_entry(), do tce_setrange_multi_pSeriesLP_walk in all memory,
of_add_property(), and list_add().
With this patch enable_ddw() does, in order:
create_ddw(), ddw_property_create(), of_add_property(),
ddw_list_new_entry(), do tce_setrange_multi_pSeriesLP_walk in all memory,
and list_add().
This change requires of_remove_property() in case anything fails after
of_add_property(), but we get to do tce_setrange_multi_pSeriesLP_walk
in all memory, which looks the most expensive operation, only if
everything else succeeds.
Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
arch/powerpc/platforms/pseries/iommu.c | 93 ++++++++++++++++----------
1 file changed, 57 insertions(+), 36 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index b34b473bbdc1..7ca79a04fa52 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -1153,6 +1153,35 @@ static int iommu_get_page_shift(u32 query_page_size)
return 0;
}
+static struct property *ddw_property_create(const char *propname, u32 liobn, u64 dma_addr,
+ u32 page_shift, u32 window_shift)
+{
+ struct dynamic_dma_window_prop *ddwprop;
+ struct property *win64;
+
+ win64 = kzalloc(sizeof(*win64), GFP_KERNEL);
+ if (!win64)
+ return NULL;
+
+ win64->name = kstrdup(propname, GFP_KERNEL);
+ ddwprop = kzalloc(sizeof(*ddwprop), GFP_KERNEL);
+ win64->value = ddwprop;
+ win64->length = sizeof(*ddwprop);
+ if (!win64->name || !win64->value) {
+ kfree(win64->name);
+ kfree(win64->value);
+ kfree(win64);
+ return NULL;
+ }
+
+ ddwprop->liobn = cpu_to_be32(liobn);
+ ddwprop->dma_base = cpu_to_be64(dma_addr);
+ ddwprop->tce_shift = cpu_to_be32(page_shift);
+ ddwprop->window_shift = cpu_to_be32(window_shift);
+
+ return win64;
+}
+
/*
* If the PE supports dynamic dma windows, and there is space for a table
* that can map all pages in a linear offset, then setup such a table,
@@ -1171,12 +1200,12 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
struct ddw_query_response query;
struct ddw_create_response create;
int page_shift;
+ u64 win_addr;
struct device_node *dn;
u32 ddw_avail[DDW_APPLICABLE_SIZE];
struct direct_window *window;
struct property *win64;
bool ddw_enabled = false;
- struct dynamic_dma_window_prop *ddwprop;
struct failed_ddw_pdn *fpdn;
bool default_win_removed = false;
bool pmem_present;
@@ -1293,72 +1322,64 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
1ULL << page_shift);
goto out_failed;
}
- win64 = kzalloc(sizeof(struct property), GFP_KERNEL);
- if (!win64) {
- dev_info(&dev->dev,
- "couldn't allocate property for 64bit dma window\n");
- goto out_failed;
- }
- win64->name = kstrdup(DIRECT64_PROPNAME, GFP_KERNEL);
- win64->value = ddwprop = kmalloc(sizeof(*ddwprop), GFP_KERNEL);
- win64->length = sizeof(*ddwprop);
- if (!win64->name || !win64->value) {
- dev_info(&dev->dev,
- "couldn't allocate property name and value\n");
- goto out_free_prop;
- }
ret = create_ddw(dev, ddw_avail, &create, page_shift, len);
if (ret != 0)
- goto out_free_prop;
-
- ddwprop->liobn = cpu_to_be32(create.liobn);
- ddwprop->dma_base = cpu_to_be64(((u64)create.addr_hi << 32) |
- create.addr_lo);
- ddwprop->tce_shift = cpu_to_be32(page_shift);
- ddwprop->window_shift = cpu_to_be32(len);
+ goto out_failed;
dev_dbg(&dev->dev, "created tce table LIOBN 0x%x for %pOF\n",
create.liobn, dn);
- window = ddw_list_new_entry(pdn, ddwprop);
+ win_addr = ((u64)create.addr_hi << 32) | create.addr_lo;
+ win64 = ddw_property_create(DIRECT64_PROPNAME, create.liobn, win_addr,
+ page_shift, len);
+ if (!win64) {
+ dev_info(&dev->dev,
+ "couldn't allocate property, property name, or value\n");
+ goto out_remove_win;
+ }
+
+ ret = of_add_property(pdn, win64);
+ if (ret) {
+ dev_err(&dev->dev, "unable to add dma window property for %pOF: %d",
+ pdn, ret);
+ goto out_free_prop;
+ }
+
+ window = ddw_list_new_entry(pdn, win64->value);
if (!window)
- goto out_clear_window;
+ goto out_del_prop;
ret = walk_system_ram_range(0, memblock_end_of_DRAM() >> PAGE_SHIFT,
win64->value, tce_setrange_multi_pSeriesLP_walk);
if (ret) {
dev_info(&dev->dev, "failed to map direct window for %pOF: %d\n",
dn, ret);
- goto out_free_window;
- }
-
- ret = of_add_property(pdn, win64);
- if (ret) {
- dev_err(&dev->dev, "unable to add dma window property for %pOF: %d",
- pdn, ret);
- goto out_free_window;
+ goto out_del_list;
}
spin_lock(&direct_window_list_lock);
list_add(&window->list, &direct_window_list);
spin_unlock(&direct_window_list_lock);
- dev->dev.archdata.dma_offset = be64_to_cpu(ddwprop->dma_base);
+ dev->dev.archdata.dma_offset = win_addr;
ddw_enabled = true;
goto out_unlock;
-out_free_window:
+out_del_list:
kfree(window);
-out_clear_window:
- remove_ddw(pdn, true);
+out_del_prop:
+ of_remove_property(pdn, win64);
out_free_prop:
kfree(win64->name);
kfree(win64->value);
kfree(win64);
+out_remove_win:
+ remove_ddw(pdn, true);
+
out_failed:
if (default_win_removed)
reset_dma_window(dev, pdn);
--
2.32.0
^ permalink raw reply related
* [PATCH v5 05/11] powerpc/pseries/iommu: Allow DDW windows starting at 0x00
From: Leonardo Bras @ 2021-07-16 8:27 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Leonardo Bras, Alexey Kardashevskiy, David Gibson,
kernel test robot, Nicolin Chen, Frederic Barrat
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20210716082755.428187-1-leobras.c@gmail.com>
enable_ddw() currently returns the address of the DMA window, which is
considered invalid if has the value 0x00.
Also, it only considers valid an address returned from find_existing_ddw
if it's not 0x00.
Changing this behavior makes sense, given the users of enable_ddw() only
need to know if direct mapping is possible. It can also allow a DMA window
starting at 0x00 to be used.
This will be helpful for using a DDW with indirect mapping, as the window
address will be different than 0x00, but it will not map the whole
partition.
Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
arch/powerpc/platforms/pseries/iommu.c | 36 +++++++++++++-------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index 712d1667144a..b34b473bbdc1 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -853,25 +853,26 @@ static void remove_ddw(struct device_node *np, bool remove_prop)
np, ret);
}
-static u64 find_existing_ddw(struct device_node *pdn, int *window_shift)
+static bool find_existing_ddw(struct device_node *pdn, u64 *dma_addr, int *window_shift)
{
struct direct_window *window;
const struct dynamic_dma_window_prop *direct64;
- u64 dma_addr = 0;
+ bool found = false;
spin_lock(&direct_window_list_lock);
/* check if we already created a window and dupe that config if so */
list_for_each_entry(window, &direct_window_list, list) {
if (window->device == pdn) {
direct64 = window->prop;
- dma_addr = be64_to_cpu(direct64->dma_base);
+ *dma_addr = be64_to_cpu(direct64->dma_base);
*window_shift = be32_to_cpu(direct64->window_shift);
+ found = true;
break;
}
}
spin_unlock(&direct_window_list_lock);
- return dma_addr;
+ return found;
}
static struct direct_window *ddw_list_new_entry(struct device_node *pdn,
@@ -1161,20 +1162,20 @@ static int iommu_get_page_shift(u32 query_page_size)
* pdn: the parent pe node with the ibm,dma_window property
* Future: also check if we can remap the base window for our base page size
*
- * returns the dma offset for use by the direct mapped DMA code.
+ * returns true if can map all pages (direct mapping), false otherwise..
*/
-static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
+static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
{
int len = 0, ret;
int max_ram_len = order_base_2(ddw_memory_hotplug_max());
struct ddw_query_response query;
struct ddw_create_response create;
int page_shift;
- u64 dma_addr;
struct device_node *dn;
u32 ddw_avail[DDW_APPLICABLE_SIZE];
struct direct_window *window;
struct property *win64;
+ bool ddw_enabled = false;
struct dynamic_dma_window_prop *ddwprop;
struct failed_ddw_pdn *fpdn;
bool default_win_removed = false;
@@ -1186,9 +1187,10 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
mutex_lock(&direct_window_init_mutex);
- dma_addr = find_existing_ddw(pdn, &len);
- if (dma_addr != 0)
+ if (find_existing_ddw(pdn, &dev->dev.archdata.dma_offset, &len)) {
+ ddw_enabled = true;
goto out_unlock;
+ }
/*
* If we already went through this for a previous function of
@@ -1342,7 +1344,8 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
list_add(&window->list, &direct_window_list);
spin_unlock(&direct_window_list_lock);
- dma_addr = be64_to_cpu(ddwprop->dma_base);
+ dev->dev.archdata.dma_offset = be64_to_cpu(ddwprop->dma_base);
+ ddw_enabled = true;
goto out_unlock;
out_free_window:
@@ -1374,10 +1377,10 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
* as RAM, then we failed to create a window to cover persistent
* memory and need to set the DMA limit.
*/
- if (pmem_present && dma_addr && (len == max_ram_len))
- dev->dev.bus_dma_limit = dma_addr + (1ULL << len);
+ if (pmem_present && ddw_enabled && (len == max_ram_len))
+ dev->dev.bus_dma_limit = dev->dev.archdata.dma_offset + (1ULL << len);
- return dma_addr;
+ return ddw_enabled;
}
static void pci_dma_dev_setup_pSeriesLP(struct pci_dev *dev)
@@ -1456,11 +1459,8 @@ static bool iommu_bypass_supported_pSeriesLP(struct pci_dev *pdev, u64 dma_mask)
break;
}
- if (pdn && PCI_DN(pdn)) {
- pdev->dev.archdata.dma_offset = enable_ddw(pdev, pdn);
- if (pdev->dev.archdata.dma_offset)
- return true;
- }
+ if (pdn && PCI_DN(pdn))
+ return enable_ddw(pdev, pdn);
return false;
}
--
2.32.0
^ permalink raw reply related
* [PATCH v5 04/11] powerpc/pseries/iommu: Add ddw_list_new_entry() helper
From: Leonardo Bras @ 2021-07-16 8:27 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Leonardo Bras, Alexey Kardashevskiy, David Gibson,
kernel test robot, Nicolin Chen, Frederic Barrat
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20210716082755.428187-1-leobras.c@gmail.com>
There are two functions creating direct_window_list entries in a
similar way, so create a ddw_list_new_entry() to avoid duplicity and
simplify those functions.
Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
arch/powerpc/platforms/pseries/iommu.c | 32 +++++++++++++++++---------
1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index 33d82865d6e6..712d1667144a 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -874,6 +874,21 @@ static u64 find_existing_ddw(struct device_node *pdn, int *window_shift)
return dma_addr;
}
+static struct direct_window *ddw_list_new_entry(struct device_node *pdn,
+ const struct dynamic_dma_window_prop *dma64)
+{
+ struct direct_window *window;
+
+ window = kzalloc(sizeof(*window), GFP_KERNEL);
+ if (!window)
+ return NULL;
+
+ window->device = pdn;
+ window->prop = dma64;
+
+ return window;
+}
+
static int find_existing_ddw_windows(void)
{
int len;
@@ -886,18 +901,15 @@ static int find_existing_ddw_windows(void)
for_each_node_with_property(pdn, DIRECT64_PROPNAME) {
direct64 = of_get_property(pdn, DIRECT64_PROPNAME, &len);
- if (!direct64)
- continue;
-
- window = kzalloc(sizeof(*window), GFP_KERNEL);
- if (!window || len < sizeof(struct dynamic_dma_window_prop)) {
- kfree(window);
+ if (!direct64 || len < sizeof(*direct64)) {
remove_ddw(pdn, true);
continue;
}
- window->device = pdn;
- window->prop = direct64;
+ window = ddw_list_new_entry(pdn, direct64);
+ if (!window)
+ break;
+
spin_lock(&direct_window_list_lock);
list_add(&window->list, &direct_window_list);
spin_unlock(&direct_window_list_lock);
@@ -1307,7 +1319,7 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
dev_dbg(&dev->dev, "created tce table LIOBN 0x%x for %pOF\n",
create.liobn, dn);
- window = kzalloc(sizeof(*window), GFP_KERNEL);
+ window = ddw_list_new_entry(pdn, ddwprop);
if (!window)
goto out_clear_window;
@@ -1326,8 +1338,6 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
goto out_free_window;
}
- window->device = pdn;
- window->prop = ddwprop;
spin_lock(&direct_window_list_lock);
list_add(&window->list, &direct_window_list);
spin_unlock(&direct_window_list_lock);
--
2.32.0
^ permalink raw reply related
* [PATCH v5 03/11] powerpc/pseries/iommu: Add iommu_pseries_alloc_table() helper
From: Leonardo Bras @ 2021-07-16 8:27 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Leonardo Bras, Alexey Kardashevskiy, David Gibson,
kernel test robot, Nicolin Chen, Frederic Barrat
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20210716082755.428187-1-leobras.c@gmail.com>
Creates a helper to allow allocating a new iommu_table without the need
to reallocate the iommu_group.
This will be helpful for replacing the iommu_table for the new DMA window,
after we remove the old one with iommu_tce_table_put().
Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
arch/powerpc/platforms/pseries/iommu.c | 25 ++++++++++++++-----------
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index b1b8d12bab39..33d82865d6e6 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -53,28 +53,31 @@ enum {
DDW_EXT_QUERY_OUT_SIZE = 2
};
-static struct iommu_table_group *iommu_pseries_alloc_group(int node)
+static struct iommu_table *iommu_pseries_alloc_table(int node)
{
- struct iommu_table_group *table_group;
struct iommu_table *tbl;
- table_group = kzalloc_node(sizeof(struct iommu_table_group), GFP_KERNEL,
- node);
- if (!table_group)
- return NULL;
-
tbl = kzalloc_node(sizeof(struct iommu_table), GFP_KERNEL, node);
if (!tbl)
- goto free_group;
+ return NULL;
INIT_LIST_HEAD_RCU(&tbl->it_group_list);
kref_init(&tbl->it_kref);
+ return tbl;
+}
- table_group->tables[0] = tbl;
+static struct iommu_table_group *iommu_pseries_alloc_group(int node)
+{
+ struct iommu_table_group *table_group;
+
+ table_group = kzalloc_node(sizeof(*table_group), GFP_KERNEL, node);
+ if (!table_group)
+ return NULL;
- return table_group;
+ table_group->tables[0] = iommu_pseries_alloc_table(node);
+ if (table_group->tables[0])
+ return table_group;
-free_group:
kfree(table_group);
return NULL;
}
--
2.32.0
^ permalink raw reply related
* [PATCH v5 02/11] powerpc/kernel/iommu: Add new iommu_table_in_use() helper
From: Leonardo Bras @ 2021-07-16 8:27 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Leonardo Bras, Alexey Kardashevskiy, David Gibson,
kernel test robot, Nicolin Chen, Frederic Barrat
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20210716082755.428187-1-leobras.c@gmail.com>
Having a function to check if the iommu table has any allocation helps
deciding if a tbl can be reset for using a new DMA window.
It should be enough to replace all instances of !bitmap_empty(tbl...).
iommu_table_in_use() skips reserved memory, so we don't need to worry about
releasing it before testing. This causes iommu_table_release_pages() to
become unnecessary, given it is only used to remove reserved memory for
testing.
Also, only allow storing reserved memory values in tbl if they are valid
in the table, so there is no need to check it in the new helper.
Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
arch/powerpc/include/asm/iommu.h | 1 +
arch/powerpc/kernel/iommu.c | 65 ++++++++++++++++----------------
2 files changed, 34 insertions(+), 32 deletions(-)
diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
index deef7c94d7b6..bf3b84128525 100644
--- a/arch/powerpc/include/asm/iommu.h
+++ b/arch/powerpc/include/asm/iommu.h
@@ -154,6 +154,7 @@ extern int iommu_tce_table_put(struct iommu_table *tbl);
*/
extern struct iommu_table *iommu_init_table(struct iommu_table *tbl,
int nid, unsigned long res_start, unsigned long res_end);
+bool iommu_table_in_use(struct iommu_table *tbl);
#define IOMMU_TABLE_GROUP_MAX_TABLES 2
diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index 2af89a5e379f..b10bf58ae467 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -690,32 +690,24 @@ static void iommu_table_reserve_pages(struct iommu_table *tbl,
if (tbl->it_offset == 0)
set_bit(0, tbl->it_map);
- tbl->it_reserved_start = res_start;
- tbl->it_reserved_end = res_end;
-
- /* Check if res_start..res_end isn't empty and overlaps the table */
- if (res_start && res_end &&
- (tbl->it_offset + tbl->it_size < res_start ||
- res_end < tbl->it_offset))
- return;
+ if (res_start < tbl->it_offset)
+ res_start = tbl->it_offset;
- for (i = tbl->it_reserved_start; i < tbl->it_reserved_end; ++i)
- set_bit(i - tbl->it_offset, tbl->it_map);
-}
+ if (res_end > (tbl->it_offset + tbl->it_size))
+ res_end = tbl->it_offset + tbl->it_size;
-static void iommu_table_release_pages(struct iommu_table *tbl)
-{
- int i;
+ /* Check if res_start..res_end is a valid range in the table */
+ if (res_start >= res_end) {
+ tbl->it_reserved_start = tbl->it_offset;
+ tbl->it_reserved_end = tbl->it_offset;
+ return;
+ }
- /*
- * In case we have reserved the first bit, we should not emit
- * the warning below.
- */
- if (tbl->it_offset == 0)
- clear_bit(0, tbl->it_map);
+ tbl->it_reserved_start = res_start;
+ tbl->it_reserved_end = res_end;
for (i = tbl->it_reserved_start; i < tbl->it_reserved_end; ++i)
- clear_bit(i - tbl->it_offset, tbl->it_map);
+ set_bit(i - tbl->it_offset, tbl->it_map);
}
/*
@@ -779,6 +771,22 @@ struct iommu_table *iommu_init_table(struct iommu_table *tbl, int nid,
return tbl;
}
+bool iommu_table_in_use(struct iommu_table *tbl)
+{
+ unsigned long start = 0, end;
+
+ /* ignore reserved bit0 */
+ if (tbl->it_offset == 0)
+ start = 1;
+ end = tbl->it_reserved_start - tbl->it_offset;
+ if (find_next_bit(tbl->it_map, end, start) != end)
+ return true;
+
+ start = tbl->it_reserved_end - tbl->it_offset;
+ end = tbl->it_size;
+ return find_next_bit(tbl->it_map, end, start) != end;
+}
+
static void iommu_table_free(struct kref *kref)
{
struct iommu_table *tbl;
@@ -795,10 +803,8 @@ static void iommu_table_free(struct kref *kref)
iommu_debugfs_del(tbl);
- iommu_table_release_pages(tbl);
-
/* verify that table contains no entries */
- if (!bitmap_empty(tbl->it_map, tbl->it_size))
+ if (iommu_table_in_use(tbl))
pr_warn("%s: Unexpected TCEs\n", __func__);
/* free bitmap */
@@ -1099,18 +1105,13 @@ int iommu_take_ownership(struct iommu_table *tbl)
for (i = 0; i < tbl->nr_pools; i++)
spin_lock_nest_lock(&tbl->pools[i].lock, &tbl->large_pool.lock);
- iommu_table_release_pages(tbl);
-
- if (!bitmap_empty(tbl->it_map, tbl->it_size)) {
+ if (iommu_table_in_use(tbl)) {
pr_err("iommu_tce: it_map is not empty");
ret = -EBUSY;
- /* Undo iommu_table_release_pages, i.e. restore bit#0, etc */
- iommu_table_reserve_pages(tbl, tbl->it_reserved_start,
- tbl->it_reserved_end);
- } else {
- memset(tbl->it_map, 0xff, sz);
}
+ memset(tbl->it_map, 0xff, sz);
+
for (i = 0; i < tbl->nr_pools; i++)
spin_unlock(&tbl->pools[i].lock);
spin_unlock_irqrestore(&tbl->large_pool.lock, flags);
--
2.32.0
^ permalink raw reply related
* [PATCH v5 01/11] powerpc/pseries/iommu: Replace hard-coded page shift
From: Leonardo Bras @ 2021-07-16 8:27 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Leonardo Bras, Alexey Kardashevskiy, David Gibson,
kernel test robot, Nicolin Chen, Frederic Barrat
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20210716082755.428187-1-leobras.c@gmail.com>
Some functions assume IOMMU page size can only be 4K (pageshift == 12).
Update them to accept any page size passed, so we can use 64K pages.
In the process, some defines like TCE_SHIFT were made obsolete, and then
removed.
IODA3 Revision 3.0_prd1 (OpenPowerFoundation), Figures 3.4 and 3.5 show
a RPN of 52-bit, and considers a 12-bit pageshift, so there should be
no need of using TCE_RPN_MASK, which masks out any bit after 40 in rpn.
It's usage removed from tce_build_pSeries(), tce_build_pSeriesLP(), and
tce_buildmulti_pSeriesLP().
Most places had a tbl struct, so using tbl->it_page_shift was simple.
tce_free_pSeriesLP() was a special case, since callers not always have a
tbl struct, so adding a tceshift parameter seems the right thing to do.
Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
arch/powerpc/include/asm/tce.h | 8 ------
arch/powerpc/platforms/pseries/iommu.c | 39 +++++++++++++++-----------
2 files changed, 23 insertions(+), 24 deletions(-)
diff --git a/arch/powerpc/include/asm/tce.h b/arch/powerpc/include/asm/tce.h
index db5fc2f2262d..0c34d2756d92 100644
--- a/arch/powerpc/include/asm/tce.h
+++ b/arch/powerpc/include/asm/tce.h
@@ -19,15 +19,7 @@
#define TCE_VB 0
#define TCE_PCI 1
-/* TCE page size is 4096 bytes (1 << 12) */
-
-#define TCE_SHIFT 12
-#define TCE_PAGE_SIZE (1 << TCE_SHIFT)
-
#define TCE_ENTRY_SIZE 8 /* each TCE is 64 bits */
-
-#define TCE_RPN_MASK 0xfffffffffful /* 40-bit RPN (4K pages) */
-#define TCE_RPN_SHIFT 12
#define TCE_VALID 0x800 /* TCE valid */
#define TCE_ALLIO 0x400 /* TCE valid for all lpars */
#define TCE_PCI_WRITE 0x2 /* write from PCI allowed */
diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index 0c55b991f665..b1b8d12bab39 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -107,6 +107,8 @@ static int tce_build_pSeries(struct iommu_table *tbl, long index,
u64 proto_tce;
__be64 *tcep;
u64 rpn;
+ const unsigned long tceshift = tbl->it_page_shift;
+ const unsigned long pagesize = IOMMU_PAGE_SIZE(tbl);
proto_tce = TCE_PCI_READ; // Read allowed
@@ -117,10 +119,10 @@ static int tce_build_pSeries(struct iommu_table *tbl, long index,
while (npages--) {
/* can't move this out since we might cross MEMBLOCK boundary */
- rpn = __pa(uaddr) >> TCE_SHIFT;
- *tcep = cpu_to_be64(proto_tce | (rpn & TCE_RPN_MASK) << TCE_RPN_SHIFT);
+ rpn = __pa(uaddr) >> tceshift;
+ *tcep = cpu_to_be64(proto_tce | rpn << tceshift);
- uaddr += TCE_PAGE_SIZE;
+ uaddr += pagesize;
tcep++;
}
return 0;
@@ -146,7 +148,7 @@ static unsigned long tce_get_pseries(struct iommu_table *tbl, long index)
return be64_to_cpu(*tcep);
}
-static void tce_free_pSeriesLP(unsigned long liobn, long, long);
+static void tce_free_pSeriesLP(unsigned long liobn, long, long, long);
static void tce_freemulti_pSeriesLP(struct iommu_table*, long, long);
static int tce_build_pSeriesLP(unsigned long liobn, long tcenum, long tceshift,
@@ -166,12 +168,12 @@ static int tce_build_pSeriesLP(unsigned long liobn, long tcenum, long tceshift,
proto_tce |= TCE_PCI_WRITE;
while (npages--) {
- tce = proto_tce | (rpn & TCE_RPN_MASK) << tceshift;
+ tce = proto_tce | rpn << tceshift;
rc = plpar_tce_put((u64)liobn, (u64)tcenum << tceshift, tce);
if (unlikely(rc == H_NOT_ENOUGH_RESOURCES)) {
ret = (int)rc;
- tce_free_pSeriesLP(liobn, tcenum_start,
+ tce_free_pSeriesLP(liobn, tcenum_start, tceshift,
(npages_start - (npages + 1)));
break;
}
@@ -205,10 +207,11 @@ static int tce_buildmulti_pSeriesLP(struct iommu_table *tbl, long tcenum,
long tcenum_start = tcenum, npages_start = npages;
int ret = 0;
unsigned long flags;
+ const unsigned long tceshift = tbl->it_page_shift;
if ((npages == 1) || !firmware_has_feature(FW_FEATURE_PUT_TCE_IND)) {
return tce_build_pSeriesLP(tbl->it_index, tcenum,
- tbl->it_page_shift, npages, uaddr,
+ tceshift, npages, uaddr,
direction, attrs);
}
@@ -225,13 +228,13 @@ static int tce_buildmulti_pSeriesLP(struct iommu_table *tbl, long tcenum,
if (!tcep) {
local_irq_restore(flags);
return tce_build_pSeriesLP(tbl->it_index, tcenum,
- tbl->it_page_shift,
+ tceshift,
npages, uaddr, direction, attrs);
}
__this_cpu_write(tce_page, tcep);
}
- rpn = __pa(uaddr) >> TCE_SHIFT;
+ rpn = __pa(uaddr) >> tceshift;
proto_tce = TCE_PCI_READ;
if (direction != DMA_TO_DEVICE)
proto_tce |= TCE_PCI_WRITE;
@@ -245,12 +248,12 @@ static int tce_buildmulti_pSeriesLP(struct iommu_table *tbl, long tcenum,
limit = min_t(long, npages, 4096/TCE_ENTRY_SIZE);
for (l = 0; l < limit; l++) {
- tcep[l] = cpu_to_be64(proto_tce | (rpn & TCE_RPN_MASK) << TCE_RPN_SHIFT);
+ tcep[l] = cpu_to_be64(proto_tce | rpn << tceshift);
rpn++;
}
rc = plpar_tce_put_indirect((u64)tbl->it_index,
- (u64)tcenum << 12,
+ (u64)tcenum << tceshift,
(u64)__pa(tcep),
limit);
@@ -277,12 +280,13 @@ static int tce_buildmulti_pSeriesLP(struct iommu_table *tbl, long tcenum,
return ret;
}
-static void tce_free_pSeriesLP(unsigned long liobn, long tcenum, long npages)
+static void tce_free_pSeriesLP(unsigned long liobn, long tcenum, long tceshift,
+ long npages)
{
u64 rc;
while (npages--) {
- rc = plpar_tce_put((u64)liobn, (u64)tcenum << 12, 0);
+ rc = plpar_tce_put((u64)liobn, (u64)tcenum << tceshift, 0);
if (rc && printk_ratelimit()) {
printk("tce_free_pSeriesLP: plpar_tce_put failed. rc=%lld\n", rc);
@@ -301,9 +305,11 @@ static void tce_freemulti_pSeriesLP(struct iommu_table *tbl, long tcenum, long n
u64 rc;
if (!firmware_has_feature(FW_FEATURE_STUFF_TCE))
- return tce_free_pSeriesLP(tbl->it_index, tcenum, npages);
+ return tce_free_pSeriesLP(tbl->it_index, tcenum,
+ tbl->it_page_shift, npages);
- rc = plpar_tce_stuff((u64)tbl->it_index, (u64)tcenum << 12, 0, npages);
+ rc = plpar_tce_stuff((u64)tbl->it_index,
+ (u64)tcenum << tbl->it_page_shift, 0, npages);
if (rc && printk_ratelimit()) {
printk("tce_freemulti_pSeriesLP: plpar_tce_stuff failed\n");
@@ -319,7 +325,8 @@ static unsigned long tce_get_pSeriesLP(struct iommu_table *tbl, long tcenum)
u64 rc;
unsigned long tce_ret;
- rc = plpar_tce_get((u64)tbl->it_index, (u64)tcenum << 12, &tce_ret);
+ rc = plpar_tce_get((u64)tbl->it_index,
+ (u64)tcenum << tbl->it_page_shift, &tce_ret);
if (rc && printk_ratelimit()) {
printk("tce_get_pSeriesLP: plpar_tce_get failed. rc=%lld\n", rc);
--
2.32.0
^ permalink raw reply related
* [PATCH v5 00/11] DDW + Indirect Mapping
From: Leonardo Bras @ 2021-07-16 8:27 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Leonardo Bras, Alexey Kardashevskiy, David Gibson,
kernel test robot, Nicolin Chen, Frederic Barrat
Cc: linuxppc-dev, linux-kernel
So far it's assumed possible to map the guest RAM 1:1 to the bus, which
works with a small number of devices. SRIOV changes it as the user can
configure hundreds VFs and since phyp preallocates TCEs and does not
allow IOMMU pages bigger than 64K, it has to limit the number of TCEs
per a PE to limit waste of physical pages.
As of today, if the assumed direct mapping is not possible, DDW creation
is skipped and the default DMA window "ibm,dma-window" is used instead.
Using the DDW instead of the default DMA window may allow to expand the
amount of memory that can be DMA-mapped, given the number of pages (TCEs)
may stay the same (or increase) and the default DMA window offers only
4k-pages while DDW may offer larger pages (4k, 64k, 16M ...).
Patch #1 replaces hard-coded 4K page size with a variable containing the
correct page size for the window.
Patch #2 introduces iommu_table_in_use(), and replace manual bit-field
checking where it's used. It will be used for aborting enable_ddw() if
there is any current iommu allocation and we are trying single window
indirect mapping.
Patch #3 introduces iommu_pseries_alloc_table() that will be helpful
when indirect mapping needs to replace the iommu_table.
Patch #4 adds helpers for adding DDWs in the list.
Patch #5 refactors enable_ddw() so it returns if direct mapping is
possible, instead of DMA offset. It helps for next patches on
indirect DMA mapping and also allows DMA windows starting at 0x00.
Patch #6 bring new helper to simplify enable_ddw(), allowing
some reorganization for introducing indirect mapping DDW.
Patch #7 adds new helper _iommu_table_setparms() and use it in other
*setparams*() to fill iommu_table. It will also be used for creating a
new iommu_table for indirect mapping.
Patch #8 updates remove_dma_window() to accept different property names,
so we can introduce a new property for indirect mapping.
Patch #9 extracts find_existing_ddw_windows() into
find_existing_ddw_windows_named(), and calls it by it's property name.
This will be useful when the property for indirect mapping is created,
so we can search the device-tree for both properties.
Patch #10:
Instead of destroying the created DDW if it doesn't map the whole
partition, make use of it instead of the default DMA window as it improves
performance. Also, update the iommu_table and re-generate the pools.
It introduces a new property name for DDW with indirect DMA mapping.
Patch #11:
Does some renaming of 'direct window' to 'dma window', given the DDW
created can now be also used in indirect mapping if direct mapping is not
available.
All patches were tested into an LPAR with an virtio-net interface that
allows default DMA window and DDW to coexist.
Changes since v4:
- Solve conflicts with new upstream versions
- Avoid unecessary code moving by doing variable declaration before definition
- Rename _iommu_table_setparms to iommu_table_setparms_common and changed base
parameter from unsigned long to void* in order to avoid unecessary casting.
- Fix breaking case for existing direct-mapping.
- Fix IORESOURCE_MEM bound issue
- Move new tbl to pci->table_group->tables[1] instead of replacing [0]
v4 Link: https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=241597&state=%2A&archive=both
Changes since v3:
- Fixed inverted free order at ddw_property_create()
- Updated goto tag naming
v3 Link: https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=240287&state=%2A&archive=both
Changes since v2:
- Some patches got removed from the series and sent by themselves,
- New tbl created for DDW + indirect mapping reserves MMIO32 space,
- Improved reserved area algorithm,
- Improved commit messages,
- Removed define for default DMA window prop name,
- Avoided some unnecessary renaming,
- Removed some unnecessary empty lines,
- Changed some code moving to forward declarations.
v2 Link: http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=201210&state=%2A&archive=both
Leonardo Bras (11):
powerpc/pseries/iommu: Replace hard-coded page shift
powerpc/kernel/iommu: Add new iommu_table_in_use() helper
powerpc/pseries/iommu: Add iommu_pseries_alloc_table() helper
powerpc/pseries/iommu: Add ddw_list_new_entry() helper
powerpc/pseries/iommu: Allow DDW windows starting at 0x00
powerpc/pseries/iommu: Add ddw_property_create() and refactor
enable_ddw()
powerpc/pseries/iommu: Reorganize iommu_table_setparms*() with new
helper
powerpc/pseries/iommu: Update remove_dma_window() to accept property
name
powerpc/pseries/iommu: Find existing DDW with given property name
powerpc/pseries/iommu: Make use of DDW for indirect mapping
powerpc/pseries/iommu: Rename "direct window" to "dma window"
arch/powerpc/include/asm/iommu.h | 1 +
arch/powerpc/include/asm/tce.h | 8 -
arch/powerpc/kernel/iommu.c | 65 ++--
arch/powerpc/platforms/pseries/iommu.c | 481 +++++++++++++++----------
4 files changed, 330 insertions(+), 225 deletions(-)
--
2.32.0
^ permalink raw reply
* Re: [PATCH v1 10/16] s390/pci: return error code from s390_dma_map_sg()
From: Niklas Schnelle @ 2021-07-16 8:24 UTC (permalink / raw)
To: Logan Gunthorpe, linux-kernel, linux-alpha, linux-arm-kernel,
linux-ia64, linux-mips, linuxppc-dev, linux-s390, sparclinux,
iommu, linux-parisc, xen-devel
Cc: Vasily Gorbik, Heiko Carstens, Martin Oliveira, Stephen Bates,
Christian Borntraeger, Gerald Schaefer, Robin Murphy,
Christoph Hellwig, Marek Szyprowski
In-Reply-To: <20210715164544.6827-11-logang@deltatee.com>
On Thu, 2021-07-15 at 10:45 -0600, Logan Gunthorpe wrote:
> From: Martin Oliveira <martin.oliveira@eideticom.com>
>
> The .map_sg() op now expects an error code instead of zero on failure.
>
> So propagate the error from __s390_dma_map_sg() up.
>
> Signed-off-by: Martin Oliveira <martin.oliveira@eideticom.com>
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> Cc: Niklas Schnelle <schnelle@linux.ibm.com>
> Cc: Gerald Schaefer <gerald.schaefer@linux.ibm.com>
> Cc: Heiko Carstens <hca@linux.ibm.com>
> Cc: Vasily Gorbik <gor@linux.ibm.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
> arch/s390/pci/pci_dma.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/arch/s390/pci/pci_dma.c b/arch/s390/pci/pci_dma.c
> index ebc9a49523aa..c78b02012764 100644
> --- a/arch/s390/pci/pci_dma.c
> +++ b/arch/s390/pci/pci_dma.c
> @@ -487,7 +487,7 @@ static int s390_dma_map_sg(struct device *dev, struct scatterlist *sg,
> unsigned int max = dma_get_max_seg_size(dev);
> unsigned int size = s->offset + s->length;
> unsigned int offset = s->offset;
> - int count = 0, i;
> + int count = 0, i, ret;
>
> for (i = 1; i < nr_elements; i++) {
> s = sg_next(s);
> @@ -497,8 +497,9 @@ static int s390_dma_map_sg(struct device *dev, struct scatterlist *sg,
>
> if (s->offset || (size & ~PAGE_MASK) ||
> size + s->length > max) {
> - if (__s390_dma_map_sg(dev, start, size,
> - &dma->dma_address, dir))
> + ret = __s390_dma_map_sg(dev, start, size,
> + &dma->dma_address, dir);
> + if (ret)
> goto unmap;
>
> dma->dma_address += offset;
> @@ -511,7 +512,8 @@ static int s390_dma_map_sg(struct device *dev, struct scatterlist *sg,
> }
> size += s->length;
> }
> - if (__s390_dma_map_sg(dev, start, size, &dma->dma_address, dir))
> + ret = __s390_dma_map_sg(dev, start, size, &dma->dma_address, dir);
> + if (ret)
> goto unmap;
>
> dma->dma_address += offset;
> @@ -523,7 +525,7 @@ static int s390_dma_map_sg(struct device *dev, struct scatterlist *sg,
> s390_dma_unmap_pages(dev, sg_dma_address(s), sg_dma_len(s),
> dir, attrs);
>
> - return 0;
> + return ret;
> }
>
> static void s390_dma_unmap_sg(struct device *dev, struct scatterlist *sg,
So the error codes we return are -ENOMEM if allocating a DMA
translation entry fails and -EINVAL if the DMA translation table hasn't
been initialized or the caller tries to map 0 sized memory. Are these
error codes that you would expect? If yes then this change looks good
to me.
Acked-by: Niklas Schnelle <schnelle@linux.ibm.com>
^ permalink raw reply
* Re: [PATCH] crypto: DRBG - select SHA512
From: Herbert Xu @ 2021-07-16 8:14 UTC (permalink / raw)
To: Stephan Mueller; +Cc: sachinp, linuxppc-dev, linux-crypto
In-Reply-To: <304ee0376383d9ceecddbfd216c035215bbff861.camel@chronox.de>
Stephan Mueller <smueller@chronox.de> wrote:
> With the swtich to use HMAC(SHA-512) as the default DRBG type, the
> configuration must now also select SHA-512.
>
> Fixes: 9b7b94683a9b "crypto: DRBG - switch to HMAC SHA512 DRBG as default
> DRBG"
> Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
> Signed-off-by: Stephan Mueller <smueller@chronox.com>
> ---
> crypto/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v3 1/1] powerpc/pseries: Interface to represent PAPR firmware attributes
From: Pratik Sampat @ 2021-07-16 7:14 UTC (permalink / raw)
To: Fabiano Rosas, mpe, benh, paulus, linuxppc-dev, kvm-ppc,
linux-kernel, pratik.r.sampat
In-Reply-To: <87lf672wdd.fsf@linux.ibm.com>
On 16/07/21 1:16 am, Fabiano Rosas wrote:
> Pratik Sampat <psampat@linux.ibm.com> writes:
>
>> Hello,
>>
>> On 12/07/21 9:13 pm, Fabiano Rosas wrote:
>>> "Pratik R. Sampat" <psampat@linux.ibm.com> writes:
>>>
>>> Hi, have you seen Documentation/core-api/kobject.rst, particularly the
>>> part that says:
>>>
>>> "When you see a sysfs directory full of other directories, generally each
>>> of those directories corresponds to a kobject in the same kset."
>>>
>>> Taking a look at samples/kobject/kset-example.c, it seems to provide an
>>> overall structure that is closer to what other modules do when creating
>>> sysfs entries. It uses less dynamic allocations and deals a bit better
>>> with cleaning up the state afterwards.
>>>
>> Thank you for pointing me towards this example, the kset approach is
>> interesting and the example indeed does handle cleanups better.
>>
>> Currently, we use "machine_device_initcall()" to register this
>> functionality, do you suggest I convert this into a tristate module
>> instead where I can include a "module_exit" for cleanups?
> Ugh.. I was hoping we could get away with having all cleanups done at
> kobject release time. But now I see that it is not called unless we
> decrement the reference count. Nevermind then.
>
Sure I can keep the current approach as-is, while incorporating the rest of your
comments.
>>>> + ret = plpar_hcall_norets(H_GET_ENERGY_SCALE_INFO, ESI_FLAGS_ALL, 0,
>>>> + virt_to_phys(esi_buf), MAX_BUF_SZ);
>>>> + esi_hdr = (struct h_energy_scale_info_hdr *) esi_buf;
>>>> + if (ret != H_SUCCESS || esi_hdr->data_header_version != ESI_VERSION) {
>>> I really dislike this. If you want to bail due to version change, then
>>> at least include in the ABI document that we might not give the
>>> userspace any data at all.
>> My only concern for having a version check is that, the attribute list
>> can change as well as the attributes itself may change.
>> If that is the case, then in a newer version if we do not bail out we
>> may parse data into our structs incorrectly.
> Sure, that is a valid concern. But the documentation for the header
> version field says:
>
> "Version of the Header. The header will be always backward compatible,
> and changes will not impact the Array of attributes. Current version =
> 0x01"
>
> I guess this is a bit vague still, but I understood that:
>
> 1- header elements continue to exist at the same position;
> 2- the format of the array of attributes will not change.
>
> Are you saying that my interpretation above is not correct or that you
> don't trust the HV to enforce it?
>
Thanks for the clarification.
I understand now that my interpretation was incorrect. The version change
should not break anything as our code in kernel acts just as a pass through.
>> My argument only hinges on that we should likely give no data at all
>> instead of junk or incorrect data.
> I agree. I just don't think it would be possible to end up with
> incorrect data, unless the HV has a bug.
>
>> Maybe I could make this check after the return check and give out a
>> version mismatch message like the following?
>> pr_warn("hcall failed: H_GET_ENERGY_SCALE_INFO VER MISMATCH - EXP: 0x%x, REC: 0x%x",
>> ESI_VERSION, esi_hdr->data_header_version);
> Yes, this will help with debug if we ever end up in this situation.
Understood, In case of a version mismatch and IDs are introduced, it can help
the userspace know that something has changed.
>>>> + pr_warn("hcall failed: H_GET_ENERGY_SCALE_INFO");
>>>> + goto out;
>>>> + }
>>>> +
>>>> + num_attrs = be64_to_cpu(esi_hdr->num_attrs);
>>>> + /*
>>>> + * Typecast the energy buffer to the attribute structure at the offset
>>>> + * specified in the buffer
>>>> + */
>>> I think the code is now simple enough that this comment could be
>>> removed.
>> ack
>>
>>>> + esi_attrs = (struct energy_scale_attribute *)
>>>> + (esi_buf + be64_to_cpu(esi_hdr->array_offset));
>>>> +
>>>> + pgs = kcalloc(num_attrs, sizeof(*pgs), GFP_KERNEL);
>>> This is never freed.
>>>
>>>> + if (!pgs)
>>>> + goto out_pgs;
>>>> +
>>>> + papr_kobj = kobject_create_and_add("papr", firmware_kobj);
>>>> + if (!papr_kobj) {
>>>> + pr_warn("kobject_create_and_add papr failed\n");
>>>> + goto out_kobj;
>>>> + }
>>>> +
>>>> + esi_kobj = kobject_create_and_add("energy_scale_info", papr_kobj);
>>>> + if (!esi_kobj) {
>>>> + pr_warn("kobject_create_and_add energy_scale_info failed\n");
>>>> + goto out_ekobj;
>>>> + }
>>>> +
>>>> + for (idx = 0; idx < num_attrs; idx++) {
>>>> + char buf[4];
>>>> + bool show_val_desc = true;
>>>> +
>>>> + pgs[idx].pgattrs = kcalloc(MAX_ATTRS,
>>>> + sizeof(*pgs[idx].pgattrs),
>>>> + GFP_KERNEL);
>>>> + if (!pgs[idx].pgattrs)
>>>> + goto out_kobj;
>>>> +
>>>> + pgs[idx].pg.attrs = kcalloc(MAX_ATTRS + 1,
>>>> + sizeof(*pgs[idx].pg.attrs),
>>>> + GFP_KERNEL);
>>> I think the kobject code expects this to be statically allocated, so
>>> you'd need to override the release function in some way to be able to
>>> free this.
>> Right this and pgs both are never free'd because my understanding was
>> that as this functionality is invoked from machine_init, I'd expect it
>> to stay until shutdown.
> Yep, I thought the kset code would improve this, but I misread it. So
> I'm fine with keeping it like this.
Sure thing. Thanks!
>> However, if you believe that a module approach is cleaner, I can change
>> my implementation to accommodate for that and also include a
>> module_exit for cleanup of the above allocations
>>>> + if (!pgs[idx].pg.attrs) {
>>>> + kfree(pgs[idx].pgattrs);
>>>> + goto out_kobj;
>>>> + }
>>>> +
>>>> + sprintf(buf, "%lld", be64_to_cpu(esi_attrs[idx].id));
>>> Do you mean pgs[idx].name instead of buf? Otherwise you're passing this
>>> stack allocated 'buf' to another function.
>>>
>> Yes you're right I should have either passed the pg struct or I should
>> have used strcpy, here the stack allocated buffer is being taken out of
>> scope which is incorrect.
>> Thanks for pointing this out!
>>
>>>> + pgs[idx].pg.name = buf;
>>>> +
>>>> + /* Do not add the value description if it does not exist */
>>>> + if (strlen(esi_attrs[idx].value_desc) == 0)
>>>> + show_val_desc = false;
>>>> +
>>>> + if (add_attr_group(be64_to_cpu(esi_attrs[idx].id),
>>>> + MAX_ATTRS, &pgs[idx], show_val_desc)) {
>>>> + pr_warn("Failed to create papr attribute group %s\n",
>>>> + pgs[idx].pg.name);
>>>> + goto out_pgattrs;
>>>> + }
>>>> + }
>>>> +
>>>> + return 0;
>>>> +
>>>> +out_pgattrs:
>>>> + for (i = 0; i < MAX_ATTRS; i++) {
>>>> + kfree(pgs[i].pgattrs);
>>>> + kfree(pgs[i].pg.attrs);
>>>> + }
>>>> +out_ekobj:
>>>> + kobject_put(esi_kobj);
>>>> +out_kobj:
>>>> + kobject_put(papr_kobj);
>>>> +out_pgs:
>>>> + kfree(pgs);
>>>> +out:
>>>> + kfree(esi_buf);
>>>> +
>>>> + return -ENOMEM;
>>>> +}
>>>> +
>>>> +machine_device_initcall(pseries, papr_init);
^ permalink raw reply
* Re: [PATCH v1 16/16] dma-mapping: Disallow .map_sg operations from returning zero on error
From: Christoph Hellwig @ 2021-07-16 6:33 UTC (permalink / raw)
To: Logan Gunthorpe
Cc: linux-s390, linux-ia64, linux-parisc, Robin Murphy,
Martin Oliveira, linux-mips, linux-kernel, iommu, linux-alpha,
sparclinux, xen-devel, Stephen Bates, linuxppc-dev,
Christoph Hellwig, linux-arm-kernel, Marek Szyprowski
In-Reply-To: <20210715164544.6827-17-logang@deltatee.com>
On Thu, Jul 15, 2021 at 10:45:44AM -0600, Logan Gunthorpe wrote:
> @@ -194,6 +194,8 @@ static int __dma_map_sg_attrs(struct device *dev, struct scatterlist *sg,
> else
> ents = ops->map_sg(dev, sg, nents, dir, attrs);
>
> + WARN_ON_ONCE(ents == 0);
Turns this into a negative error code while we're at it, just to keep
the callers sane?
^ permalink raw reply
* Re: [PATCH v1 14/16] x86/amd_gart: return error code from gart_map_sg()
From: Christoph Hellwig @ 2021-07-16 6:32 UTC (permalink / raw)
To: Logan Gunthorpe
Cc: linux-ia64, linux-kernel, H. Peter Anvin, sparclinux,
Christoph Hellwig, Marek Szyprowski, linux-s390, Martin Oliveira,
Ingo Molnar, xen-devel, Niklas Schnelle, Borislav Petkov,
Thomas Gleixner, Stephen Bates, linux-arm-kernel,
Thomas Bogendoerfer, linux-parisc, linuxppc-dev, linux-mips,
iommu, linux-alpha, Robin Murphy
In-Reply-To: <20210715164544.6827-15-logang@deltatee.com>
On Thu, Jul 15, 2021 at 10:45:42AM -0600, Logan Gunthorpe wrote:
> @@ -458,7 +460,7 @@ static int gart_map_sg(struct device *dev, struct scatterlist *sg, int nents,
> iommu_full(dev, pages << PAGE_SHIFT, dir);
> for_each_sg(sg, s, nents, i)
> s->dma_address = DMA_MAPPING_ERROR;
> - return 0;
> + return ret;
While we're at it - setting the ->dma_address to DMA_MAPPING_ERROR is
not part of the ->map_sg calling convention. Might be worth to fix
up while we're at it.
^ permalink raw reply
* Re: [PATCH v1 04/16] dma-iommu: Return error code from iommu_dma_map_sg()
From: Christoph Hellwig @ 2021-07-16 6:31 UTC (permalink / raw)
To: Logan Gunthorpe
Cc: linux-s390, linux-ia64, linux-parisc, Will Deacon, Robin Murphy,
Joerg Roedel, Martin Oliveira, linux-mips, linux-kernel, iommu,
linux-alpha, sparclinux, xen-devel, Stephen Bates, linuxppc-dev,
Christoph Hellwig, linux-arm-kernel, Marek Szyprowski
In-Reply-To: <20210715164544.6827-5-logang@deltatee.com>
Careful here. What do all these errors from the low-level code mean
here? I think we need to clearly standardize on what we actually
return from ->map_sg and possibly document what the callers expect and
can do, and enforce that only those error are reported.
^ permalink raw reply
* Re: [PATCH v1 01/16] dma-mapping: Allow map_sg() ops to return negative error codes
From: Christoph Hellwig @ 2021-07-16 6:29 UTC (permalink / raw)
To: Logan Gunthorpe
Cc: linux-s390, linux-ia64, linux-parisc, Robin Murphy,
Martin Oliveira, linux-mips, linux-kernel, iommu, linux-alpha,
sparclinux, xen-devel, Stephen Bates, linuxppc-dev,
Christoph Hellwig, linux-arm-kernel, Marek Szyprowski
In-Reply-To: <20210715164544.6827-2-logang@deltatee.com>
On Thu, Jul 15, 2021 at 10:45:29AM -0600, Logan Gunthorpe wrote:
> + * dma_map_sgtable() will return the error code returned and convert
> + * a zero return (for legacy implementations) into -EINVAL.
> + *
> + * dma_map_sg() will always return zero on any negative or zero
> + * return to satisfy its own calling convention.
> */
I don't think this belongs here.
> +EXPORT_SYMBOL(dma_map_sgtable);
EXPORT_SYMBOL_GPL, please.
^ permalink raw reply
* Re: [RFC PATCH 10/43] powerpc/64s: Always set PMU control registers to frozen/disabled when not in use
From: Madhavan Srinivasan @ 2021-07-16 3:43 UTC (permalink / raw)
To: Nicholas Piggin, Athira Rajeev; +Cc: linuxppc-dev, kvm-ppc
In-Reply-To: <1626265929.asca0gyunh.astroid@bobo.none>
On 7/14/21 6:09 PM, Nicholas Piggin wrote:
> Excerpts from Nicholas Piggin's message of July 12, 2021 12:41 pm:
>> Excerpts from Athira Rajeev's message of July 10, 2021 12:50 pm:
>>>
>>>> On 22-Jun-2021, at 4:27 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
>>>>
>>>> KVM PMU management code looks for particular frozen/disabled bits in
>>>> the PMU registers so it knows whether it must clear them when coming
>>>> out of a guest or not. Setting this up helps KVM make these optimisations
>>>> without getting confused. Longer term the better approach might be to
>>>> move guest/host PMU switching to the perf subsystem.
>>>>
>>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>>>> ---
>>>> arch/powerpc/kernel/cpu_setup_power.c | 4 ++--
>>>> arch/powerpc/kernel/dt_cpu_ftrs.c | 6 +++---
>>>> arch/powerpc/kvm/book3s_hv.c | 5 +++++
>>>> arch/powerpc/perf/core-book3s.c | 7 +++++++
>>>> 4 files changed, 17 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/arch/powerpc/kernel/cpu_setup_power.c b/arch/powerpc/kernel/cpu_setup_power.c
>>>> index a29dc8326622..3dc61e203f37 100644
>>>> --- a/arch/powerpc/kernel/cpu_setup_power.c
>>>> +++ b/arch/powerpc/kernel/cpu_setup_power.c
>>>> @@ -109,7 +109,7 @@ static void init_PMU_HV_ISA207(void)
>>>> static void init_PMU(void)
>>>> {
>>>> mtspr(SPRN_MMCRA, 0);
>>>> - mtspr(SPRN_MMCR0, 0);
>>>> + mtspr(SPRN_MMCR0, MMCR0_FC);
>>>> mtspr(SPRN_MMCR1, 0);
>>>> mtspr(SPRN_MMCR2, 0);
>>>> }
>>>> @@ -123,7 +123,7 @@ static void init_PMU_ISA31(void)
>>>> {
>>>> mtspr(SPRN_MMCR3, 0);
>>>> mtspr(SPRN_MMCRA, MMCRA_BHRB_DISABLE);
>>>> - mtspr(SPRN_MMCR0, MMCR0_PMCCEXT);
>>>> + mtspr(SPRN_MMCR0, MMCR0_FC | MMCR0_PMCCEXT);
>>>> }
>>>>
>>>> /*
>>>> diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
>>>> index 0a6b36b4bda8..06a089fbeaa7 100644
>>>> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
>>>> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
>>>> @@ -353,7 +353,7 @@ static void init_pmu_power8(void)
>>>> }
>>>>
>>>> mtspr(SPRN_MMCRA, 0);
>>>> - mtspr(SPRN_MMCR0, 0);
>>>> + mtspr(SPRN_MMCR0, MMCR0_FC);
>>>> mtspr(SPRN_MMCR1, 0);
>>>> mtspr(SPRN_MMCR2, 0);
>>>> mtspr(SPRN_MMCRS, 0);
>>>> @@ -392,7 +392,7 @@ static void init_pmu_power9(void)
>>>> mtspr(SPRN_MMCRC, 0);
>>>>
>>>> mtspr(SPRN_MMCRA, 0);
>>>> - mtspr(SPRN_MMCR0, 0);
>>>> + mtspr(SPRN_MMCR0, MMCR0_FC);
>>>> mtspr(SPRN_MMCR1, 0);
>>>> mtspr(SPRN_MMCR2, 0);
>>>> }
>>>> @@ -428,7 +428,7 @@ static void init_pmu_power10(void)
>>>>
>>>> mtspr(SPRN_MMCR3, 0);
>>>> mtspr(SPRN_MMCRA, MMCRA_BHRB_DISABLE);
>>>> - mtspr(SPRN_MMCR0, MMCR0_PMCCEXT);
>>>> + mtspr(SPRN_MMCR0, MMCR0_FC | MMCR0_PMCCEXT);
>>>> }
>>>>
>>>> static int __init feat_enable_pmu_power10(struct dt_cpu_feature *f)
>>>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>>>> index 1f30f98b09d1..f7349d150828 100644
>>>> --- a/arch/powerpc/kvm/book3s_hv.c
>>>> +++ b/arch/powerpc/kvm/book3s_hv.c
>>>> @@ -2593,6 +2593,11 @@ static int kvmppc_core_vcpu_create_hv(struct kvm_vcpu *vcpu)
>>>> #endif
>>>> #endif
>>>> vcpu->arch.mmcr[0] = MMCR0_FC;
>>>> + if (cpu_has_feature(CPU_FTR_ARCH_31)) {
>>>> + vcpu->arch.mmcr[0] |= MMCR0_PMCCEXT;
>>>> + vcpu->arch.mmcra = MMCRA_BHRB_DISABLE;
>>>> + }
>>>> +
>>>> vcpu->arch.ctrl = CTRL_RUNLATCH;
>>>> /* default to host PVR, since we can't spoof it */
>>>> kvmppc_set_pvr_hv(vcpu, mfspr(SPRN_PVR));
>>>> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
>>>> index 51622411a7cc..e33b29ec1a65 100644
>>>> --- a/arch/powerpc/perf/core-book3s.c
>>>> +++ b/arch/powerpc/perf/core-book3s.c
>>>> @@ -1361,6 +1361,13 @@ static void power_pmu_enable(struct pmu *pmu)
>>>> goto out;
>>>>
>>>> if (cpuhw->n_events == 0) {
>>>> + if (cpu_has_feature(CPU_FTR_ARCH_31)) {
>>>> + mtspr(SPRN_MMCRA, MMCRA_BHRB_DISABLE);
>>>> + mtspr(SPRN_MMCR0, MMCR0_FC | MMCR0_PMCCEXT);
>>>> + } else {
>>>> + mtspr(SPRN_MMCRA, 0);
>>>> + mtspr(SPRN_MMCR0, MMCR0_FC);
>>>> + }
>>>
>>> Hi Nick,
>>>
>>> We are setting these bits in “power_pmu_disable” function. And disable will be called before any event gets deleted/stopped. Can you please help to understand why this is needed in power_pmu_enable path also ?
>> I'll have to go back and check what I needed it for.
> Okay, MMCRA is getting MMCRA_SDAR_MODE_DCACHE set on POWER9, by the looks.
>
> That's not necessarily a problem, but KVM sets MMCRA to 0 to disable
> SDAR updates. So KVM and perf don't agree on what the "correct" value
> for disabled is. Which could be a problem with POWER10 not setting BHRB
> disable before my series.
Nick,
BHRB disable will be only known to P10 guest kernel and in case of
P9 guest kernel it is not known. And for the P10 host and guest,
I guess we set BHRB_DISABLE in cpu_setup code and also in
power_pmu_disable().
So you areseeing P10 guest having MMCRA as zero during context switch?
Maddy
>
> I'll get rid of this hunk for now, I expect things won't be exactly clean
> or consistent until the KVM host PMU code is moved into perf/ though.
>
> Thanks,
> Nick
^ permalink raw reply
* [PATCH 2/2] KVM: PPC: Fix kvm_arch_vcpu_ioctl vcpu_load leak
From: Nicholas Piggin @ 2021-07-16 2:43 UTC (permalink / raw)
To: kvm-ppc; +Cc: Alexey Kardashevskiy, linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210716024310.164448-1-npiggin@gmail.com>
vcpu_put is not called if the user copy fails. This can result in preempt
notifier corruption and crashes, among other issues.
Reported-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Fixes: b3cebfe8c1ca ("KVM: PPC: Move vcpu_load/vcpu_put down to each ioctl case in kvm_arch_vcpu_ioctl")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kvm/powerpc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index be33b5321a76..b4e6f70b97b9 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -2048,9 +2048,9 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
{
struct kvm_enable_cap cap;
r = -EFAULT;
- vcpu_load(vcpu);
if (copy_from_user(&cap, argp, sizeof(cap)))
goto out;
+ vcpu_load(vcpu);
r = kvm_vcpu_ioctl_enable_cap(vcpu, &cap);
vcpu_put(vcpu);
break;
@@ -2074,9 +2074,9 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
case KVM_DIRTY_TLB: {
struct kvm_dirty_tlb dirty;
r = -EFAULT;
- vcpu_load(vcpu);
if (copy_from_user(&dirty, argp, sizeof(dirty)))
goto out;
+ vcpu_load(vcpu);
r = kvm_vcpu_ioctl_dirty_tlb(vcpu, &dirty);
vcpu_put(vcpu);
break;
--
2.23.0
^ permalink raw reply related
* [PATCH 1/2] KVM: PPC: Book3S: Fix CONFIG_TRANSACTIONAL_MEM=n crash
From: Nicholas Piggin @ 2021-07-16 2:43 UTC (permalink / raw)
To: kvm-ppc; +Cc: Alexey Kardashevskiy, linuxppc-dev, Nicholas Piggin
When running CPU_FTR_P9_TM_HV_ASSIST, HFSCR[TM] is set for the guest
even if the host has CONFIG_TRANSACTIONAL_MEM=n, which causes it to be
unprepared to handle guest exits while transactional.
Normal guests don't have a problem because the HTM capability will not
be advertised, but a rogue or buggy one could crash the host.
Reported-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Fixes: 4bb3c7a0208f ("KVM: PPC: Book3S HV: Work around transactional memory bugs in POWER9")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kvm/book3s_hv.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 1d1fcc290fca..085fb8ecbf68 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -2697,8 +2697,10 @@ static int kvmppc_core_vcpu_create_hv(struct kvm_vcpu *vcpu)
HFSCR_DSCR | HFSCR_VECVSX | HFSCR_FP | HFSCR_PREFIX;
if (cpu_has_feature(CPU_FTR_HVMODE)) {
vcpu->arch.hfscr &= mfspr(SPRN_HFSCR);
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
if (cpu_has_feature(CPU_FTR_P9_TM_HV_ASSIST))
vcpu->arch.hfscr |= HFSCR_TM;
+#endif
}
if (cpu_has_feature(CPU_FTR_TM_COMP))
vcpu->arch.hfscr |= HFSCR_TM;
--
2.23.0
^ permalink raw reply related
* [powerpc:fixes-test] BUILD SUCCESS e44fbdb68049539de9923ce4bad2d277aef54892
From: kernel test robot @ 2021-07-16 0:55 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git fixes-test
branch HEAD: e44fbdb68049539de9923ce4bad2d277aef54892 KVM: PPC: Book3S HV P9: Fix guest TM support
elapsed time: 731m
configs tested: 58
configs skipped: 106
The following configs have been built successfully.
More configs may be tested in the coming days.
gcc tested configs:
sh se7705_defconfig
arc axs101_defconfig
sh rts7751r2d1_defconfig
arm gemini_defconfig
powerpc asp8347_defconfig
riscv nommu_virt_defconfig
parisc generic-32bit_defconfig
mips mpc30x_defconfig
x86_64 defconfig
arm pxa910_defconfig
xtensa cadence_csp_defconfig
arm tct_hammer_defconfig
arm sama5_defconfig
mips cu1000-neo_defconfig
h8300 defconfig
arm alldefconfig
x86_64 allnoconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
nios2 defconfig
arc allyesconfig
nds32 allnoconfig
nds32 defconfig
nios2 allyesconfig
csky defconfig
alpha defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
arc defconfig
sh allmodconfig
parisc defconfig
s390 allyesconfig
s390 allmodconfig
parisc allyesconfig
s390 defconfig
powerpc allyesconfig
powerpc allmodconfig
powerpc allnoconfig
i386 randconfig-a005-20210715
i386 randconfig-a006-20210715
i386 randconfig-a004-20210715
i386 randconfig-a001-20210715
i386 randconfig-a003-20210715
i386 randconfig-a002-20210715
x86_64 randconfig-a013-20210715
x86_64 randconfig-a012-20210715
x86_64 randconfig-a015-20210715
x86_64 randconfig-a014-20210715
x86_64 randconfig-a016-20210715
x86_64 randconfig-a011-20210715
i386 randconfig-a014-20210715
i386 randconfig-a015-20210715
i386 randconfig-a011-20210715
i386 randconfig-a013-20210715
i386 randconfig-a012-20210715
i386 randconfig-a016-20210715
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* Re: [PATCH v2 4/4] bus: Make remove callback return void
From: Thomas Bogendoerfer @ 2021-07-15 13:02 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: nvdimm, Alexey Kardashevskiy, Samuel Iglesias Gonsalvez,
Jens Taprogge, Ulf Hansson, Jaroslav Kysela, linux-fpga,
Benjamin Tissoires, Paul Mackerras, Srinivas Pandruvada,
K. Y. Srinivasan, Mike Christie, Wei Liu, Maxim Levitsky,
Samuel Holland, linux-acpi, linux-pci, xen-devel, Tomas Winkler,
Julien Grall, Ohad Ben-Cohen, Alex Williamson, Alex Elder,
linux-parisc, Geoff Levand, Greg Kroah-Hartman, linux-usb,
Rafael J. Wysocki, linux-kernel, linux-spi, Thorsten Scherer,
kernel, Jon Mason, linux-ntb, Wu Hao, David Woodhouse,
Krzysztof Wilczyński, Alexandre Belloni, Manohar Vanga,
linux-wireless, Dominik Brodowski, virtualization,
James E.J. Bottomley, target-devel, Srinivas Kandagatla,
linux-i2c, Kai-Heng Feng, Stefano Stabellini, Stephen Hemminger,
Ira Weiny, Helge Deller, Rafał Miłecki,
industrypack-devel, linux-mips, Len Brown, alsa-devel,
linux-arm-msm, linux-media, Maxime Ripard, Johan Hovold,
greybus-dev, Bjorn Helgaas, Dave Jiang, Boris Ostrovsky,
Mika Westerberg, linux-arm-kernel, Johannes Thumshirn,
Mathieu Poirier, Stephen Boyd, Cornelia Huck, Wolfram Sang,
Joey Pabalan, Yehezkel Bernat, Pali Rohár, Bodo Stroesser,
Alison Schofield, Heikki Krogerus, Tyrel Datwyler,
Alexander Shishkin, Tom Rix, Jason Wang, SeongJae Park,
linux-hyperv, platform-driver-x86, Frank Li, netdev,
Qinglang Miao, Jiri Slaby, Rob Herring, Lorenzo Pieralisi,
Mark Gross, linux-staging, Dexuan Cui, Jernej Skrabec,
Kishon Vijay Abraham I, Chen-Yu Tsai, linux-input, Allen Hubbe,
Alex Dubov, Haiyang Zhang, Jiri Kosina, Vladimir Zapolskiy,
Russell King, Ben Widawsky, Moritz Fischer, linux-cxl,
Michael Buesch, Dan Williams, Mauro Carvalho Chehab,
Cristian Marussi, Martin K. Petersen, Martyn Welch,
Dmitry Torokhov, linux-mmc, linux-sunxi, Stefan Richter,
Sudeep Holla, David S. Miller, Sven Van Asbroeck, kvm,
Michael S. Tsirkin, linux-remoteproc, Bjorn Andersson,
Kirti Wankhede, Andreas Noever, linux-i3c, linux1394-devel,
Lee Jones, Arnd Bergmann, linux-scsi, Vishal Verma, Russell King,
Andy Gross, linux-serial, Jakub Kicinski, Michael Jamet,
William Breathitt Gray, Hans de Goede, Hannes Reinecke,
Adrian Hunter, Juergen Gross, linuxppc-dev, Takashi Iwai,
Alexandre Bounine, Vinod Koul, Mark Brown, Marc Zyngier,
dmaengine, Johannes Berg, Johannes Thumshirn, Maximilian Luz
In-Reply-To: <20210706154803.1631813-5-u.kleine-koenig@pengutronix.de>
On Tue, Jul 06, 2021 at 05:48:03PM +0200, Uwe Kleine-König wrote:
> The driver core ignores the return value of this callback because there
> is only little it can do when a device disappears.
>
> This is the final bit of a long lasting cleanup quest where several
> buses were converted to also return void from their remove callback.
> Additionally some resource leaks were fixed that were caused by drivers
> returning an error code in the expectation that the driver won't go
> away.
>
> With struct bus_type::remove returning void it's prevented that newly
> implemented buses return an ignored error code and so don't anticipate
> wrong expectations for driver authors.
>
> Acked-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> (For ARM, Amba and related parts)
> Acked-by: Mark Brown <broonie@kernel.org>
> Acked-by: Chen-Yu Tsai <wens@csie.org> (for drivers/bus/sunxi-rsb.c)
> Acked-by: Pali Rohár <pali@kernel.org>
> Acked-by: Mauro Carvalho Chehab <mchehab@kernel.org> (for drivers/media)
> Acked-by: Hans de Goede <hdegoede@redhat.com> (For drivers/platform)
> Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Acked-By: Vinod Koul <vkoul@kernel.org>
> Acked-by: Juergen Gross <jgross@suse.com> (For Xen)
> Acked-by: Lee Jones <lee.jones@linaro.org> (For drivers/mfd)
> Acked-by: Johannes Thumshirn <jth@kernel.org> (For drivers/mcb)
> Acked-by: Johan Hovold <johan@kernel.org>
> Acked-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> (For drivers/slimbus)
> Acked-by: Kirti Wankhede <kwankhede@nvidia.com> (For drivers/vfio)
> Acked-by: Maximilian Luz <luzmaximilian@gmail.com>
> Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> (For ulpi and typec)
> Acked-by: Samuel Iglesias Gonsálvez <siglesias@igalia.com> (For ipack)
> Reviewed-by: Tom Rix <trix@redhat.com> (For fpga)
> Acked-by: Geoff Levand <geoff@infradead.org> (For ps3)
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> [...]
> arch/mips/sgi-ip22/ip22-gio.c | 3 +--
Acked-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply
* Re: [PATCH v3 1/1] powerpc/pseries: Interface to represent PAPR firmware attributes
From: Fabiano Rosas @ 2021-07-15 19:46 UTC (permalink / raw)
To: Pratik Sampat, mpe, benh, paulus, linuxppc-dev, kvm-ppc,
linux-kernel, pratik.r.sampat
In-Reply-To: <60575876-b15d-6dee-dbb7-c68b9e304557@linux.ibm.com>
Pratik Sampat <psampat@linux.ibm.com> writes:
> Hello,
>
> On 12/07/21 9:13 pm, Fabiano Rosas wrote:
>> "Pratik R. Sampat" <psampat@linux.ibm.com> writes:
>>
>> Hi, have you seen Documentation/core-api/kobject.rst, particularly the
>> part that says:
>>
>> "When you see a sysfs directory full of other directories, generally each
>> of those directories corresponds to a kobject in the same kset."
>>
>> Taking a look at samples/kobject/kset-example.c, it seems to provide an
>> overall structure that is closer to what other modules do when creating
>> sysfs entries. It uses less dynamic allocations and deals a bit better
>> with cleaning up the state afterwards.
>>
> Thank you for pointing me towards this example, the kset approach is
> interesting and the example indeed does handle cleanups better.
>
> Currently, we use "machine_device_initcall()" to register this
> functionality, do you suggest I convert this into a tristate module
> instead where I can include a "module_exit" for cleanups?
Ugh.. I was hoping we could get away with having all cleanups done at
kobject release time. But now I see that it is not called unless we
decrement the reference count. Nevermind then.
>>> + ret = plpar_hcall_norets(H_GET_ENERGY_SCALE_INFO, ESI_FLAGS_ALL, 0,
>>> + virt_to_phys(esi_buf), MAX_BUF_SZ);
>>> + esi_hdr = (struct h_energy_scale_info_hdr *) esi_buf;
>>> + if (ret != H_SUCCESS || esi_hdr->data_header_version != ESI_VERSION) {
>> I really dislike this. If you want to bail due to version change, then
>> at least include in the ABI document that we might not give the
>> userspace any data at all.
>
> My only concern for having a version check is that, the attribute list
> can change as well as the attributes itself may change.
> If that is the case, then in a newer version if we do not bail out we
> may parse data into our structs incorrectly.
Sure, that is a valid concern. But the documentation for the header
version field says:
"Version of the Header. The header will be always backward compatible,
and changes will not impact the Array of attributes. Current version =
0x01"
I guess this is a bit vague still, but I understood that:
1- header elements continue to exist at the same position;
2- the format of the array of attributes will not change.
Are you saying that my interpretation above is not correct or that you
don't trust the HV to enforce it?
> My argument only hinges on that we should likely give no data at all
> instead of junk or incorrect data.
I agree. I just don't think it would be possible to end up with
incorrect data, unless the HV has a bug.
> Maybe I could make this check after the return check and give out a
> version mismatch message like the following?
> pr_warn("hcall failed: H_GET_ENERGY_SCALE_INFO VER MISMATCH - EXP: 0x%x, REC: 0x%x",
> ESI_VERSION, esi_hdr->data_header_version);
Yes, this will help with debug if we ever end up in this situation.
>>> + pr_warn("hcall failed: H_GET_ENERGY_SCALE_INFO");
>>> + goto out;
>>> + }
>>> +
>>> + num_attrs = be64_to_cpu(esi_hdr->num_attrs);
>>> + /*
>>> + * Typecast the energy buffer to the attribute structure at the offset
>>> + * specified in the buffer
>>> + */
>> I think the code is now simple enough that this comment could be
>> removed.
>
> ack
>
>>> + esi_attrs = (struct energy_scale_attribute *)
>>> + (esi_buf + be64_to_cpu(esi_hdr->array_offset));
>>> +
>>> + pgs = kcalloc(num_attrs, sizeof(*pgs), GFP_KERNEL);
>> This is never freed.
>>
>>> + if (!pgs)
>>> + goto out_pgs;
>>> +
>>> + papr_kobj = kobject_create_and_add("papr", firmware_kobj);
>>> + if (!papr_kobj) {
>>> + pr_warn("kobject_create_and_add papr failed\n");
>>> + goto out_kobj;
>>> + }
>>> +
>>> + esi_kobj = kobject_create_and_add("energy_scale_info", papr_kobj);
>>> + if (!esi_kobj) {
>>> + pr_warn("kobject_create_and_add energy_scale_info failed\n");
>>> + goto out_ekobj;
>>> + }
>>> +
>>> + for (idx = 0; idx < num_attrs; idx++) {
>>> + char buf[4];
>>> + bool show_val_desc = true;
>>> +
>>> + pgs[idx].pgattrs = kcalloc(MAX_ATTRS,
>>> + sizeof(*pgs[idx].pgattrs),
>>> + GFP_KERNEL);
>>> + if (!pgs[idx].pgattrs)
>>> + goto out_kobj;
>>> +
>>> + pgs[idx].pg.attrs = kcalloc(MAX_ATTRS + 1,
>>> + sizeof(*pgs[idx].pg.attrs),
>>> + GFP_KERNEL);
>> I think the kobject code expects this to be statically allocated, so
>> you'd need to override the release function in some way to be able to
>> free this.
>
> Right this and pgs both are never free'd because my understanding was
> that as this functionality is invoked from machine_init, I'd expect it
> to stay until shutdown.
Yep, I thought the kset code would improve this, but I misread it. So
I'm fine with keeping it like this.
> However, if you believe that a module approach is cleaner, I can change
> my implementation to accommodate for that and also include a
> module_exit for cleanup of the above allocations
>>> + if (!pgs[idx].pg.attrs) {
>>> + kfree(pgs[idx].pgattrs);
>>> + goto out_kobj;
>>> + }
>>> +
>>> + sprintf(buf, "%lld", be64_to_cpu(esi_attrs[idx].id));
>> Do you mean pgs[idx].name instead of buf? Otherwise you're passing this
>> stack allocated 'buf' to another function.
>>
> Yes you're right I should have either passed the pg struct or I should
> have used strcpy, here the stack allocated buffer is being taken out of
> scope which is incorrect.
> Thanks for pointing this out!
>
>>> + pgs[idx].pg.name = buf;
>>> +
>>> + /* Do not add the value description if it does not exist */
>>> + if (strlen(esi_attrs[idx].value_desc) == 0)
>>> + show_val_desc = false;
>>> +
>>> + if (add_attr_group(be64_to_cpu(esi_attrs[idx].id),
>>> + MAX_ATTRS, &pgs[idx], show_val_desc)) {
>>> + pr_warn("Failed to create papr attribute group %s\n",
>>> + pgs[idx].pg.name);
>>> + goto out_pgattrs;
>>> + }
>>> + }
>>> +
>>> + return 0;
>>> +
>>> +out_pgattrs:
>>> + for (i = 0; i < MAX_ATTRS; i++) {
>>> + kfree(pgs[i].pgattrs);
>>> + kfree(pgs[i].pg.attrs);
>>> + }
>>> +out_ekobj:
>>> + kobject_put(esi_kobj);
>>> +out_kobj:
>>> + kobject_put(papr_kobj);
>>> +out_pgs:
>>> + kfree(pgs);
>>> +out:
>>> + kfree(esi_buf);
>>> +
>>> + return -ENOMEM;
>>> +}
>>> +
>>> +machine_device_initcall(pseries, papr_init);
^ permalink raw reply
* [PATCH printk v4 3/6] printk: remove safe buffers
From: John Ogness @ 2021-07-15 19:33 UTC (permalink / raw)
To: Petr Mladek
Cc: Kees Cook, Paul E. McKenney, Alexey Kardashevskiy,
Nicholas Piggin, linux-kernel, Steven Rostedt, kexec,
Sergey Senozhatsky, Yue Hu, Paul Mackerras, Eric Biederman,
Thomas Gleixner, linuxppc-dev, Andrew Morton, Tiezhu Yang,
Cédric Le Goater
In-Reply-To: <20210715193359.25946-1-john.ogness@linutronix.de>
With @logbuf_lock removed, the high level printk functions for
storing messages are lockless. Messages can be stored from any
context, so there is no need for the NMI and safe buffers anymore.
Remove the NMI and safe buffers.
Although the safe buffers are removed, the NMI and safe context
tracking is still in place. In these contexts, store the message
immediately but still use irq_work to defer the console printing.
Since printk recursion tracking is in place, safe context tracking
for most of printk is not needed. Remove it. Only safe context
tracking relating to the console and console_owner locks is left
in place. This is because the console and console_owner locks are
needed for the actual printing.
Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
arch/powerpc/kernel/traps.c | 1 -
arch/powerpc/kernel/watchdog.c | 5 -
include/linux/printk.h | 10 -
kernel/kexec_core.c | 1 -
kernel/panic.c | 3 -
kernel/printk/internal.h | 17 --
kernel/printk/printk.c | 120 +++++-------
kernel/printk/printk_safe.c | 335 +--------------------------------
lib/nmi_backtrace.c | 6 -
9 files changed, 48 insertions(+), 450 deletions(-)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index b4ab95c9e94a..2522800217d1 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -170,7 +170,6 @@ extern void panic_flush_kmsg_start(void)
extern void panic_flush_kmsg_end(void)
{
- printk_safe_flush_on_panic();
kmsg_dump(KMSG_DUMP_PANIC);
bust_spinlocks(0);
debug_locks_off();
diff --git a/arch/powerpc/kernel/watchdog.c b/arch/powerpc/kernel/watchdog.c
index c9a8f4781a10..dc17d8903d4f 100644
--- a/arch/powerpc/kernel/watchdog.c
+++ b/arch/powerpc/kernel/watchdog.c
@@ -183,11 +183,6 @@ static void watchdog_smp_panic(int cpu, u64 tb)
wd_smp_unlock(&flags);
- printk_safe_flush();
- /*
- * printk_safe_flush() seems to require another print
- * before anything actually goes out to console.
- */
if (sysctl_hardlockup_all_cpu_backtrace)
trigger_allbutself_cpu_backtrace();
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 1790a5521fd9..664612f75dac 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -207,8 +207,6 @@ __printf(1, 2) void dump_stack_set_arch_desc(const char *fmt, ...);
void dump_stack_print_info(const char *log_lvl);
void show_regs_print_info(const char *log_lvl);
extern asmlinkage void dump_stack(void) __cold;
-extern void printk_safe_flush(void);
-extern void printk_safe_flush_on_panic(void);
#else
static inline __printf(1, 0)
int vprintk(const char *s, va_list args)
@@ -272,14 +270,6 @@ static inline void show_regs_print_info(const char *log_lvl)
static inline void dump_stack(void)
{
}
-
-static inline void printk_safe_flush(void)
-{
-}
-
-static inline void printk_safe_flush_on_panic(void)
-{
-}
#endif
#ifdef CONFIG_SMP
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index f099baee3578..69c6e9b7761c 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -978,7 +978,6 @@ void crash_kexec(struct pt_regs *regs)
old_cpu = atomic_cmpxchg(&panic_cpu, PANIC_CPU_INVALID, this_cpu);
if (old_cpu == PANIC_CPU_INVALID) {
/* This is the 1st CPU which comes here, so go ahead. */
- printk_safe_flush_on_panic();
__crash_kexec(regs);
/*
diff --git a/kernel/panic.c b/kernel/panic.c
index 332736a72a58..1f0df42f8d0c 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -247,7 +247,6 @@ void panic(const char *fmt, ...)
* Bypass the panic_cpu check and call __crash_kexec directly.
*/
if (!_crash_kexec_post_notifiers) {
- printk_safe_flush_on_panic();
__crash_kexec(NULL);
/*
@@ -271,8 +270,6 @@ void panic(const char *fmt, ...)
*/
atomic_notifier_call_chain(&panic_notifier_list, 0, buf);
- /* Call flush even twice. It tries harder with a single online CPU */
- printk_safe_flush_on_panic();
kmsg_dump(KMSG_DUMP_PANIC);
/*
diff --git a/kernel/printk/internal.h b/kernel/printk/internal.h
index 51615c909b2f..6cc35c5de890 100644
--- a/kernel/printk/internal.h
+++ b/kernel/printk/internal.h
@@ -22,7 +22,6 @@ __printf(1, 0) int vprintk_deferred(const char *fmt, va_list args);
void __printk_safe_enter(void);
void __printk_safe_exit(void);
-void printk_safe_init(void);
bool printk_percpu_data_ready(void);
#define printk_safe_enter_irqsave(flags) \
@@ -37,18 +36,6 @@ bool printk_percpu_data_ready(void);
local_irq_restore(flags); \
} while (0)
-#define printk_safe_enter_irq() \
- do { \
- local_irq_disable(); \
- __printk_safe_enter(); \
- } while (0)
-
-#define printk_safe_exit_irq() \
- do { \
- __printk_safe_exit(); \
- local_irq_enable(); \
- } while (0)
-
void defer_console_output(void);
#else
@@ -61,9 +48,5 @@ void defer_console_output(void);
#define printk_safe_enter_irqsave(flags) local_irq_save(flags)
#define printk_safe_exit_irqrestore(flags) local_irq_restore(flags)
-#define printk_safe_enter_irq() local_irq_disable()
-#define printk_safe_exit_irq() local_irq_enable()
-
-static inline void printk_safe_init(void) { }
static inline bool printk_percpu_data_ready(void) { return false; }
#endif /* CONFIG_PRINTK */
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 2f9d783373ed..9693791ab124 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -732,27 +732,22 @@ static ssize_t devkmsg_read(struct file *file, char __user *buf,
if (ret)
return ret;
- printk_safe_enter_irq();
if (!prb_read_valid(prb, atomic64_read(&user->seq), r)) {
if (file->f_flags & O_NONBLOCK) {
ret = -EAGAIN;
- printk_safe_exit_irq();
goto out;
}
- printk_safe_exit_irq();
ret = wait_event_interruptible(log_wait,
prb_read_valid(prb, atomic64_read(&user->seq), r));
if (ret)
goto out;
- printk_safe_enter_irq();
}
if (r->info->seq != atomic64_read(&user->seq)) {
/* our last seen message is gone, return error and reset */
atomic64_set(&user->seq, r->info->seq);
ret = -EPIPE;
- printk_safe_exit_irq();
goto out;
}
@@ -762,7 +757,6 @@ static ssize_t devkmsg_read(struct file *file, char __user *buf,
&r->info->dev_info);
atomic64_set(&user->seq, r->info->seq + 1);
- printk_safe_exit_irq();
if (len > count) {
ret = -EINVAL;
@@ -797,7 +791,6 @@ static loff_t devkmsg_llseek(struct file *file, loff_t offset, int whence)
if (offset)
return -ESPIPE;
- printk_safe_enter_irq();
switch (whence) {
case SEEK_SET:
/* the first record */
@@ -818,7 +811,6 @@ static loff_t devkmsg_llseek(struct file *file, loff_t offset, int whence)
default:
ret = -EINVAL;
}
- printk_safe_exit_irq();
return ret;
}
@@ -833,7 +825,6 @@ static __poll_t devkmsg_poll(struct file *file, poll_table *wait)
poll_wait(file, &log_wait, wait);
- printk_safe_enter_irq();
if (prb_read_valid_info(prb, atomic64_read(&user->seq), &info, NULL)) {
/* return error when data has vanished underneath us */
if (info.seq != atomic64_read(&user->seq))
@@ -841,7 +832,6 @@ static __poll_t devkmsg_poll(struct file *file, poll_table *wait)
else
ret = EPOLLIN|EPOLLRDNORM;
}
- printk_safe_exit_irq();
return ret;
}
@@ -874,9 +864,7 @@ static int devkmsg_open(struct inode *inode, struct file *file)
prb_rec_init_rd(&user->record, &user->info,
&user->text_buf[0], sizeof(user->text_buf));
- printk_safe_enter_irq();
atomic64_set(&user->seq, prb_first_valid_seq(prb));
- printk_safe_exit_irq();
file->private_data = user;
return 0;
@@ -1042,9 +1030,6 @@ static inline void log_buf_add_cpu(void) {}
static void __init set_percpu_data_ready(void)
{
- printk_safe_init();
- /* Make sure we set this flag only after printk_safe() init is done */
- barrier();
__printk_percpu_data_ready = true;
}
@@ -1082,6 +1067,7 @@ void __init setup_log_buf(int early)
struct prb_desc *new_descs;
struct printk_info info;
struct printk_record r;
+ unsigned int text_size;
size_t new_descs_size;
size_t new_infos_size;
unsigned long flags;
@@ -1142,24 +1128,37 @@ void __init setup_log_buf(int early)
new_descs, ilog2(new_descs_count),
new_infos);
- printk_safe_enter_irqsave(flags);
+ local_irq_save(flags);
log_buf_len = new_log_buf_len;
log_buf = new_log_buf;
new_log_buf_len = 0;
free = __LOG_BUF_LEN;
- prb_for_each_record(0, &printk_rb_static, seq, &r)
- free -= add_to_rb(&printk_rb_dynamic, &r);
+ prb_for_each_record(0, &printk_rb_static, seq, &r) {
+ text_size = add_to_rb(&printk_rb_dynamic, &r);
+ if (text_size > free)
+ free = 0;
+ else
+ free -= text_size;
+ }
- /*
- * This is early enough that everything is still running on the
- * boot CPU and interrupts are disabled. So no new messages will
- * appear during the transition to the dynamic buffer.
- */
prb = &printk_rb_dynamic;
- printk_safe_exit_irqrestore(flags);
+ local_irq_restore(flags);
+
+ /*
+ * Copy any remaining messages that might have appeared from
+ * NMI context after copying but before switching to the
+ * dynamic buffer.
+ */
+ prb_for_each_record(seq, &printk_rb_static, seq, &r) {
+ text_size = add_to_rb(&printk_rb_dynamic, &r);
+ if (text_size > free)
+ free = 0;
+ else
+ free -= text_size;
+ }
if (seq != prb_next_seq(&printk_rb_static)) {
pr_err("dropped %llu messages\n",
@@ -1498,11 +1497,9 @@ static int syslog_print(char __user *buf, int size)
size_t n;
size_t skip;
- printk_safe_enter_irq();
- raw_spin_lock(&syslog_lock);
+ raw_spin_lock_irq(&syslog_lock);
if (!prb_read_valid(prb, syslog_seq, &r)) {
- raw_spin_unlock(&syslog_lock);
- printk_safe_exit_irq();
+ raw_spin_unlock_irq(&syslog_lock);
break;
}
if (r.info->seq != syslog_seq) {
@@ -1531,8 +1528,7 @@ static int syslog_print(char __user *buf, int size)
syslog_partial += n;
} else
n = 0;
- raw_spin_unlock(&syslog_lock);
- printk_safe_exit_irq();
+ raw_spin_unlock_irq(&syslog_lock);
if (!n)
break;
@@ -1566,7 +1562,6 @@ static int syslog_print_all(char __user *buf, int size, bool clear)
return -ENOMEM;
time = printk_time;
- printk_safe_enter_irq();
/*
* Find first record that fits, including all following records,
* into the user-provided buffer for this dump.
@@ -1587,23 +1582,20 @@ static int syslog_print_all(char __user *buf, int size, bool clear)
break;
}
- printk_safe_exit_irq();
if (copy_to_user(buf + len, text, textlen))
len = -EFAULT;
else
len += textlen;
- printk_safe_enter_irq();
if (len < 0)
break;
}
if (clear) {
- raw_spin_lock(&syslog_lock);
+ raw_spin_lock_irq(&syslog_lock);
latched_seq_write(&clear_seq, seq);
- raw_spin_unlock(&syslog_lock);
+ raw_spin_unlock_irq(&syslog_lock);
}
- printk_safe_exit_irq();
kfree(text);
return len;
@@ -1611,11 +1603,9 @@ static int syslog_print_all(char __user *buf, int size, bool clear)
static void syslog_clear(void)
{
- printk_safe_enter_irq();
- raw_spin_lock(&syslog_lock);
+ raw_spin_lock_irq(&syslog_lock);
latched_seq_write(&clear_seq, prb_next_seq(prb));
- raw_spin_unlock(&syslog_lock);
- printk_safe_exit_irq();
+ raw_spin_unlock_irq(&syslog_lock);
}
/* Return a consistent copy of @syslog_seq. */
@@ -1703,12 +1693,10 @@ int do_syslog(int type, char __user *buf, int len, int source)
break;
/* Number of chars in the log buffer */
case SYSLOG_ACTION_SIZE_UNREAD:
- printk_safe_enter_irq();
- raw_spin_lock(&syslog_lock);
+ raw_spin_lock_irq(&syslog_lock);
if (!prb_read_valid_info(prb, syslog_seq, &info, NULL)) {
/* No unread messages. */
- raw_spin_unlock(&syslog_lock);
- printk_safe_exit_irq();
+ raw_spin_unlock_irq(&syslog_lock);
return 0;
}
if (info.seq != syslog_seq) {
@@ -1736,8 +1724,7 @@ int do_syslog(int type, char __user *buf, int len, int source)
}
error -= syslog_partial;
}
- raw_spin_unlock(&syslog_lock);
- printk_safe_exit_irq();
+ raw_spin_unlock_irq(&syslog_lock);
break;
/* Size of the log buffer */
case SYSLOG_ACTION_SIZE_BUFFER:
@@ -2219,7 +2206,6 @@ asmlinkage int vprintk_emit(int facility, int level,
{
int printed_len;
bool in_sched = false;
- unsigned long flags;
/* Suppress unimportant messages after panic happens */
if (unlikely(suppress_printk))
@@ -2233,9 +2219,7 @@ asmlinkage int vprintk_emit(int facility, int level,
boot_delay_msec(level);
printk_delay();
- printk_safe_enter_irqsave(flags);
printed_len = vprintk_store(facility, level, dev_info, fmt, args);
- printk_safe_exit_irqrestore(flags);
/* If called from the scheduler, we can not call up(). */
if (!in_sched) {
@@ -2665,9 +2649,9 @@ void console_unlock(void)
for (;;) {
size_t ext_len = 0;
+ int handover;
size_t len;
- printk_safe_enter_irqsave(flags);
skip:
if (!prb_read_valid(prb, console_seq, &r))
break;
@@ -2717,19 +2701,22 @@ void console_unlock(void)
* were to occur on another CPU, it may wait for this one to
* finish. This task can not be preempted if there is a
* waiter waiting to take over.
+ *
+ * Interrupts are disabled because the hand over to a waiter
+ * must not be interrupted until the hand over is completed
+ * (@console_waiter is cleared).
*/
+ printk_safe_enter_irqsave(flags);
console_lock_spinning_enable();
stop_critical_timings(); /* don't trace print latency */
call_console_drivers(ext_text, ext_len, text, len);
start_critical_timings();
- if (console_lock_spinning_disable_and_check()) {
- printk_safe_exit_irqrestore(flags);
- return;
- }
-
+ handover = console_lock_spinning_disable_and_check();
printk_safe_exit_irqrestore(flags);
+ if (handover)
+ return;
if (do_cond_resched)
cond_resched();
@@ -2748,8 +2735,6 @@ void console_unlock(void)
* flush, no worries.
*/
retry = prb_read_valid(prb, next_seq, NULL);
- printk_safe_exit_irqrestore(flags);
-
if (retry && console_trylock())
goto again;
}
@@ -2811,13 +2796,8 @@ void console_flush_on_panic(enum con_flush_mode mode)
console_trylock();
console_may_schedule = 0;
- if (mode == CONSOLE_REPLAY_ALL) {
- unsigned long flags;
-
- printk_safe_enter_irqsave(flags);
+ if (mode == CONSOLE_REPLAY_ALL)
console_seq = prb_first_valid_seq(prb);
- printk_safe_exit_irqrestore(flags);
- }
console_unlock();
}
@@ -3469,14 +3449,12 @@ bool kmsg_dump_get_line(struct kmsg_dump_iter *iter, bool syslog,
struct printk_info info;
unsigned int line_count;
struct printk_record r;
- unsigned long flags;
size_t l = 0;
bool ret = false;
if (iter->cur_seq < min_seq)
iter->cur_seq = min_seq;
- printk_safe_enter_irqsave(flags);
prb_rec_init_rd(&r, &info, line, size);
/* Read text or count text lines? */
@@ -3497,7 +3475,6 @@ bool kmsg_dump_get_line(struct kmsg_dump_iter *iter, bool syslog,
iter->cur_seq = r.info->seq + 1;
ret = true;
out:
- printk_safe_exit_irqrestore(flags);
if (len)
*len = l;
return ret;
@@ -3529,7 +3506,6 @@ bool kmsg_dump_get_buffer(struct kmsg_dump_iter *iter, bool syslog,
u64 min_seq = latched_seq_read_nolock(&clear_seq);
struct printk_info info;
struct printk_record r;
- unsigned long flags;
u64 seq;
u64 next_seq;
size_t len = 0;
@@ -3542,7 +3518,6 @@ bool kmsg_dump_get_buffer(struct kmsg_dump_iter *iter, bool syslog,
if (iter->cur_seq < min_seq)
iter->cur_seq = min_seq;
- printk_safe_enter_irqsave(flags);
if (prb_read_valid_info(prb, iter->cur_seq, &info, NULL)) {
if (info.seq != iter->cur_seq) {
/* messages are gone, move to first available one */
@@ -3551,10 +3526,8 @@ bool kmsg_dump_get_buffer(struct kmsg_dump_iter *iter, bool syslog,
}
/* last entry */
- if (iter->cur_seq >= iter->next_seq) {
- printk_safe_exit_irqrestore(flags);
+ if (iter->cur_seq >= iter->next_seq)
goto out;
- }
/*
* Find first record that fits, including all following records,
@@ -3586,7 +3559,6 @@ bool kmsg_dump_get_buffer(struct kmsg_dump_iter *iter, bool syslog,
iter->next_seq = next_seq;
ret = true;
- printk_safe_exit_irqrestore(flags);
out:
if (len_out)
*len_out = len;
@@ -3604,12 +3576,8 @@ EXPORT_SYMBOL_GPL(kmsg_dump_get_buffer);
*/
void kmsg_dump_rewind(struct kmsg_dump_iter *iter)
{
- unsigned long flags;
-
- printk_safe_enter_irqsave(flags);
iter->cur_seq = latched_seq_read_nolock(&clear_seq);
iter->next_seq = prb_next_seq(prb);
- printk_safe_exit_irqrestore(flags);
}
EXPORT_SYMBOL_GPL(kmsg_dump_rewind);
diff --git a/kernel/printk/printk_safe.c b/kernel/printk/printk_safe.c
index 94232186fccb..29c580dac93d 100644
--- a/kernel/printk/printk_safe.c
+++ b/kernel/printk/printk_safe.c
@@ -15,286 +15,9 @@
#include "internal.h"
-/*
- * In NMI and safe mode, printk() avoids taking locks. Instead,
- * it uses an alternative implementation that temporary stores
- * the strings into a per-CPU buffer. The content of the buffer
- * is later flushed into the main ring buffer via IRQ work.
- *
- * The alternative implementation is chosen transparently
- * by examining current printk() context mask stored in @printk_context
- * per-CPU variable.
- *
- * The implementation allows to flush the strings also from another CPU.
- * There are situations when we want to make sure that all buffers
- * were handled or when IRQs are blocked.
- */
-
-#define SAFE_LOG_BUF_LEN ((1 << CONFIG_PRINTK_SAFE_LOG_BUF_SHIFT) - \
- sizeof(atomic_t) - \
- sizeof(atomic_t) - \
- sizeof(struct irq_work))
-
-struct printk_safe_seq_buf {
- atomic_t len; /* length of written data */
- atomic_t message_lost;
- struct irq_work work; /* IRQ work that flushes the buffer */
- unsigned char buffer[SAFE_LOG_BUF_LEN];
-};
-
-static DEFINE_PER_CPU(struct printk_safe_seq_buf, safe_print_seq);
static DEFINE_PER_CPU(int, printk_context);
-static DEFINE_RAW_SPINLOCK(safe_read_lock);
-
-#ifdef CONFIG_PRINTK_NMI
-static DEFINE_PER_CPU(struct printk_safe_seq_buf, nmi_print_seq);
-#endif
-
-/* Get flushed in a more safe context. */
-static void queue_flush_work(struct printk_safe_seq_buf *s)
-{
- if (printk_percpu_data_ready())
- irq_work_queue(&s->work);
-}
-
-/*
- * Add a message to per-CPU context-dependent buffer. NMI and printk-safe
- * have dedicated buffers, because otherwise printk-safe preempted by
- * NMI-printk would have overwritten the NMI messages.
- *
- * The messages are flushed from irq work (or from panic()), possibly,
- * from other CPU, concurrently with printk_safe_log_store(). Should this
- * happen, printk_safe_log_store() will notice the buffer->len mismatch
- * and repeat the write.
- */
-static __printf(2, 0) int printk_safe_log_store(struct printk_safe_seq_buf *s,
- const char *fmt, va_list args)
-{
- int add;
- size_t len;
- va_list ap;
-
-again:
- len = atomic_read(&s->len);
-
- /* The trailing '\0' is not counted into len. */
- if (len >= sizeof(s->buffer) - 1) {
- atomic_inc(&s->message_lost);
- queue_flush_work(s);
- return 0;
- }
-
- /*
- * Make sure that all old data have been read before the buffer
- * was reset. This is not needed when we just append data.
- */
- if (!len)
- smp_rmb();
-
- va_copy(ap, args);
- add = vscnprintf(s->buffer + len, sizeof(s->buffer) - len, fmt, ap);
- va_end(ap);
- if (!add)
- return 0;
-
- /*
- * Do it once again if the buffer has been flushed in the meantime.
- * Note that atomic_cmpxchg() is an implicit memory barrier that
- * makes sure that the data were written before updating s->len.
- */
- if (atomic_cmpxchg(&s->len, len, len + add) != len)
- goto again;
-
- queue_flush_work(s);
- return add;
-}
-
-static inline void printk_safe_flush_line(const char *text, int len)
-{
- /*
- * Avoid any console drivers calls from here, because we may be
- * in NMI or printk_safe context (when in panic). The messages
- * must go only into the ring buffer at this stage. Consoles will
- * get explicitly called later when a crashdump is not generated.
- */
- printk_deferred("%.*s", len, text);
-}
-
-/* printk part of the temporary buffer line by line */
-static int printk_safe_flush_buffer(const char *start, size_t len)
-{
- const char *c, *end;
- bool header;
-
- c = start;
- end = start + len;
- header = true;
-
- /* Print line by line. */
- while (c < end) {
- if (*c == '\n') {
- printk_safe_flush_line(start, c - start + 1);
- start = ++c;
- header = true;
- continue;
- }
-
- /* Handle continuous lines or missing new line. */
- if ((c + 1 < end) && printk_get_level(c)) {
- if (header) {
- c = printk_skip_level(c);
- continue;
- }
-
- printk_safe_flush_line(start, c - start);
- start = c++;
- header = true;
- continue;
- }
-
- header = false;
- c++;
- }
-
- /* Check if there was a partial line. Ignore pure header. */
- if (start < end && !header) {
- static const char newline[] = KERN_CONT "\n";
-
- printk_safe_flush_line(start, end - start);
- printk_safe_flush_line(newline, strlen(newline));
- }
-
- return len;
-}
-
-static void report_message_lost(struct printk_safe_seq_buf *s)
-{
- int lost = atomic_xchg(&s->message_lost, 0);
-
- if (lost)
- printk_deferred("Lost %d message(s)!\n", lost);
-}
-
-/*
- * Flush data from the associated per-CPU buffer. The function
- * can be called either via IRQ work or independently.
- */
-static void __printk_safe_flush(struct irq_work *work)
-{
- struct printk_safe_seq_buf *s =
- container_of(work, struct printk_safe_seq_buf, work);
- unsigned long flags;
- size_t len;
- int i;
-
- /*
- * The lock has two functions. First, one reader has to flush all
- * available message to make the lockless synchronization with
- * writers easier. Second, we do not want to mix messages from
- * different CPUs. This is especially important when printing
- * a backtrace.
- */
- raw_spin_lock_irqsave(&safe_read_lock, flags);
-
- i = 0;
-more:
- len = atomic_read(&s->len);
-
- /*
- * This is just a paranoid check that nobody has manipulated
- * the buffer an unexpected way. If we printed something then
- * @len must only increase. Also it should never overflow the
- * buffer size.
- */
- if ((i && i >= len) || len > sizeof(s->buffer)) {
- const char *msg = "printk_safe_flush: internal error\n";
-
- printk_safe_flush_line(msg, strlen(msg));
- len = 0;
- }
-
- if (!len)
- goto out; /* Someone else has already flushed the buffer. */
-
- /* Make sure that data has been written up to the @len */
- smp_rmb();
- i += printk_safe_flush_buffer(s->buffer + i, len - i);
-
- /*
- * Check that nothing has got added in the meantime and truncate
- * the buffer. Note that atomic_cmpxchg() is an implicit memory
- * barrier that makes sure that the data were copied before
- * updating s->len.
- */
- if (atomic_cmpxchg(&s->len, len, 0) != len)
- goto more;
-
-out:
- report_message_lost(s);
- raw_spin_unlock_irqrestore(&safe_read_lock, flags);
-}
-
-/**
- * printk_safe_flush - flush all per-cpu nmi buffers.
- *
- * The buffers are flushed automatically via IRQ work. This function
- * is useful only when someone wants to be sure that all buffers have
- * been flushed at some point.
- */
-void printk_safe_flush(void)
-{
- int cpu;
-
- for_each_possible_cpu(cpu) {
-#ifdef CONFIG_PRINTK_NMI
- __printk_safe_flush(&per_cpu(nmi_print_seq, cpu).work);
-#endif
- __printk_safe_flush(&per_cpu(safe_print_seq, cpu).work);
- }
-}
-
-/**
- * printk_safe_flush_on_panic - flush all per-cpu nmi buffers when the system
- * goes down.
- *
- * Similar to printk_safe_flush() but it can be called even in NMI context when
- * the system goes down. It does the best effort to get NMI messages into
- * the main ring buffer.
- *
- * Note that it could try harder when there is only one CPU online.
- */
-void printk_safe_flush_on_panic(void)
-{
- /*
- * Make sure that we could access the safe buffers.
- * Do not risk a double release when more CPUs are up.
- */
- if (raw_spin_is_locked(&safe_read_lock)) {
- if (num_online_cpus() > 1)
- return;
-
- debug_locks_off();
- raw_spin_lock_init(&safe_read_lock);
- }
-
- printk_safe_flush();
-}
-
#ifdef CONFIG_PRINTK_NMI
-/*
- * Safe printk() for NMI context. It uses a per-CPU buffer to
- * store the message. NMIs are not nested, so there is always only
- * one writer running. But the buffer might get flushed from another
- * CPU, so we need to be careful.
- */
-static __printf(1, 0) int vprintk_nmi(const char *fmt, va_list args)
-{
- struct printk_safe_seq_buf *s = this_cpu_ptr(&nmi_print_seq);
-
- return printk_safe_log_store(s, fmt, args);
-}
-
void noinstr printk_nmi_enter(void)
{
this_cpu_add(printk_context, PRINTK_NMI_CONTEXT_OFFSET);
@@ -309,9 +32,6 @@ void noinstr printk_nmi_exit(void)
* Marks a code that might produce many messages in NMI context
* and the risk of losing them is more critical than eventual
* reordering.
- *
- * It has effect only when called in NMI context. Then printk()
- * will store the messages into the main logbuf directly.
*/
void printk_nmi_direct_enter(void)
{
@@ -324,27 +44,8 @@ void printk_nmi_direct_exit(void)
this_cpu_and(printk_context, ~PRINTK_NMI_DIRECT_CONTEXT_MASK);
}
-#else
-
-static __printf(1, 0) int vprintk_nmi(const char *fmt, va_list args)
-{
- return 0;
-}
-
#endif /* CONFIG_PRINTK_NMI */
-/*
- * Lock-less printk(), to avoid deadlocks should the printk() recurse
- * into itself. It uses a per-CPU buffer to store the message, just like
- * NMI.
- */
-static __printf(1, 0) int vprintk_safe(const char *fmt, va_list args)
-{
- struct printk_safe_seq_buf *s = this_cpu_ptr(&safe_print_seq);
-
- return printk_safe_log_store(s, fmt, args);
-}
-
/* Can be preempted by NMI. */
void __printk_safe_enter(void)
{
@@ -369,46 +70,18 @@ asmlinkage int vprintk(const char *fmt, va_list args)
* Use the main logbuf even in NMI. But avoid calling console
* drivers that might have their own locks.
*/
- if ((this_cpu_read(printk_context) & PRINTK_NMI_DIRECT_CONTEXT_MASK)) {
- unsigned long flags;
+ if (this_cpu_read(printk_context) &
+ (PRINTK_NMI_DIRECT_CONTEXT_MASK |
+ PRINTK_NMI_CONTEXT_MASK |
+ PRINTK_SAFE_CONTEXT_MASK)) {
int len;
- printk_safe_enter_irqsave(flags);
len = vprintk_store(0, LOGLEVEL_DEFAULT, NULL, fmt, args);
- printk_safe_exit_irqrestore(flags);
defer_console_output();
return len;
}
- /* Use extra buffer in NMI. */
- if (this_cpu_read(printk_context) & PRINTK_NMI_CONTEXT_MASK)
- return vprintk_nmi(fmt, args);
-
- /* Use extra buffer to prevent a recursion deadlock in safe mode. */
- if (this_cpu_read(printk_context) & PRINTK_SAFE_CONTEXT_MASK)
- return vprintk_safe(fmt, args);
-
/* No obstacles. */
return vprintk_default(fmt, args);
}
EXPORT_SYMBOL(vprintk);
-
-void __init printk_safe_init(void)
-{
- int cpu;
-
- for_each_possible_cpu(cpu) {
- struct printk_safe_seq_buf *s;
-
- s = &per_cpu(safe_print_seq, cpu);
- init_irq_work(&s->work, __printk_safe_flush);
-
-#ifdef CONFIG_PRINTK_NMI
- s = &per_cpu(nmi_print_seq, cpu);
- init_irq_work(&s->work, __printk_safe_flush);
-#endif
- }
-
- /* Flush pending messages that did not have scheduled IRQ works. */
- printk_safe_flush();
-}
diff --git a/lib/nmi_backtrace.c b/lib/nmi_backtrace.c
index dae233c5f597..9813a983d024 100644
--- a/lib/nmi_backtrace.c
+++ b/lib/nmi_backtrace.c
@@ -75,12 +75,6 @@ void nmi_trigger_cpumask_backtrace(const cpumask_t *mask,
touch_softlockup_watchdog();
}
- /*
- * Force flush any remote buffers that might be stuck in IRQ context
- * and therefore could not run their irq_work.
- */
- printk_safe_flush();
-
clear_bit_unlock(0, &backtrace_flag);
put_cpu();
}
--
2.20.1
^ permalink raw reply related
* [PATCH printk v4 4/6] printk: remove NMI tracking
From: John Ogness @ 2021-07-15 19:33 UTC (permalink / raw)
To: Petr Mladek
Cc: Peter Zijlstra, Rasmus Villemoes, Wolfram Sang (Renesas),
Paul Mackerras, Daniel Borkmann, Marc Zyngier, Masahiro Yamada,
Russell King, Ingo Molnar, Geert Uytterhoeven, linux-arm-kernel,
Valentin Schneider, Kees Cook, Anshuman Khandual, Vipin Sharma,
Frederic Weisbecker, Steven Rostedt, Nathan Chancellor,
Nick Terrell, Thomas Gleixner, Vlastimil Babka, Xiongwei Song,
Nick Desaulniers, linux-kernel, Sergey Senozhatsky, Andrew Morton,
linuxppc-dev, Mike Rapoport
In-Reply-To: <20210715193359.25946-1-john.ogness@linutronix.de>
All NMI contexts are handled the same as the safe context: store the
message and defer printing. There is no need to have special NMI
context tracking for this. Using in_nmi() is enough.
There are several parts of the kernel that are manually calling into
the printk NMI context tracking in order to cause general printk
deferred printing:
arch/arm/kernel/smp.c
arch/powerpc/kexec/crash.c
kernel/trace/trace.c
For these users, provide a new function pair
printk_deferred_enter/exit that explicitly achieves the same
objective.
Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
arch/arm/kernel/smp.c | 4 ++--
arch/powerpc/kexec/crash.c | 2 +-
include/linux/hardirq.h | 2 --
include/linux/printk.h | 31 +++++++++++++++++++------------
init/Kconfig | 5 -----
kernel/printk/internal.h | 8 --------
kernel/printk/printk_safe.c | 37 +------------------------------------
kernel/trace/trace.c | 4 ++--
8 files changed, 25 insertions(+), 68 deletions(-)
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index c7bb168b0d97..842427ff2b3c 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -667,9 +667,9 @@ static void do_handle_IPI(int ipinr)
break;
case IPI_CPU_BACKTRACE:
- printk_nmi_enter();
+ printk_deferred_enter();
nmi_cpu_backtrace(get_irq_regs());
- printk_nmi_exit();
+ printk_deferred_exit();
break;
default:
diff --git a/arch/powerpc/kexec/crash.c b/arch/powerpc/kexec/crash.c
index 0196d0c211ac..1070378c8e35 100644
--- a/arch/powerpc/kexec/crash.c
+++ b/arch/powerpc/kexec/crash.c
@@ -313,7 +313,7 @@ void default_machine_crash_shutdown(struct pt_regs *regs)
int (*old_handler)(struct pt_regs *regs);
/* Avoid hardlocking with irresponsive CPU holding logbuf_lock */
- printk_nmi_enter();
+ printk_deferred_enter();
/*
* This function is only called after the system
diff --git a/include/linux/hardirq.h b/include/linux/hardirq.h
index 69bc86ea382c..76878b357ffa 100644
--- a/include/linux/hardirq.h
+++ b/include/linux/hardirq.h
@@ -116,7 +116,6 @@ extern void rcu_nmi_exit(void);
do { \
lockdep_off(); \
arch_nmi_enter(); \
- printk_nmi_enter(); \
BUG_ON(in_nmi() == NMI_MASK); \
__preempt_count_add(NMI_OFFSET + HARDIRQ_OFFSET); \
} while (0)
@@ -135,7 +134,6 @@ extern void rcu_nmi_exit(void);
do { \
BUG_ON(!in_nmi()); \
__preempt_count_sub(NMI_OFFSET + HARDIRQ_OFFSET); \
- printk_nmi_exit(); \
arch_nmi_exit(); \
lockdep_on(); \
} while (0)
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 664612f75dac..03f7ccedaf18 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -149,18 +149,6 @@ static inline __printf(1, 2) __cold
void early_printk(const char *s, ...) { }
#endif
-#ifdef CONFIG_PRINTK_NMI
-extern void printk_nmi_enter(void);
-extern void printk_nmi_exit(void);
-extern void printk_nmi_direct_enter(void);
-extern void printk_nmi_direct_exit(void);
-#else
-static inline void printk_nmi_enter(void) { }
-static inline void printk_nmi_exit(void) { }
-static inline void printk_nmi_direct_enter(void) { }
-static inline void printk_nmi_direct_exit(void) { }
-#endif /* PRINTK_NMI */
-
struct dev_printk_info;
#ifdef CONFIG_PRINTK
@@ -180,6 +168,16 @@ int printk(const char *fmt, ...);
*/
__printf(1, 2) __cold int printk_deferred(const char *fmt, ...);
+extern void __printk_safe_enter(void);
+extern void __printk_safe_exit(void);
+/*
+ * The printk_deferred_enter/exit macros are available only as a hack for
+ * some code paths that need to defer all printk console printing. Interrupts
+ * must be disabled for the deferred duration.
+ */
+#define printk_deferred_enter __printk_safe_enter
+#define printk_deferred_exit __printk_safe_exit
+
/*
* Please don't use printk_ratelimit(), because it shares ratelimiting state
* with all other unrelated printk_ratelimit() callsites. Instead use
@@ -223,6 +221,15 @@ int printk_deferred(const char *s, ...)
{
return 0;
}
+
+static inline void printk_deferred_enter(void)
+{
+}
+
+static inline void printk_deferred_exit(void)
+{
+}
+
static inline int printk_ratelimit(void)
{
return 0;
diff --git a/init/Kconfig b/init/Kconfig
index a61c92066c2e..9c0510693543 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1506,11 +1506,6 @@ config PRINTK
very difficult to diagnose system problems, saying N here is
strongly discouraged.
-config PRINTK_NMI
- def_bool y
- depends on PRINTK
- depends on HAVE_NMI
-
config BUG
bool "BUG() support" if EXPERT
default y
diff --git a/kernel/printk/internal.h b/kernel/printk/internal.h
index 6cc35c5de890..b6d310c72fc9 100644
--- a/kernel/printk/internal.h
+++ b/kernel/printk/internal.h
@@ -6,12 +6,6 @@
#ifdef CONFIG_PRINTK
-#define PRINTK_SAFE_CONTEXT_MASK 0x007ffffff
-#define PRINTK_NMI_DIRECT_CONTEXT_MASK 0x008000000
-#define PRINTK_NMI_CONTEXT_MASK 0xff0000000
-
-#define PRINTK_NMI_CONTEXT_OFFSET 0x010000000
-
__printf(4, 0)
int vprintk_store(int facility, int level,
const struct dev_printk_info *dev_info,
@@ -19,8 +13,6 @@ int vprintk_store(int facility, int level,
__printf(1, 0) int vprintk_default(const char *fmt, va_list args);
__printf(1, 0) int vprintk_deferred(const char *fmt, va_list args);
-void __printk_safe_enter(void);
-void __printk_safe_exit(void);
bool printk_percpu_data_ready(void);
diff --git a/kernel/printk/printk_safe.c b/kernel/printk/printk_safe.c
index 29c580dac93d..ef0f9a2044da 100644
--- a/kernel/printk/printk_safe.c
+++ b/kernel/printk/printk_safe.c
@@ -4,12 +4,9 @@
*/
#include <linux/preempt.h>
-#include <linux/spinlock.h>
-#include <linux/debug_locks.h>
#include <linux/kdb.h>
#include <linux/smp.h>
#include <linux/cpumask.h>
-#include <linux/irq_work.h>
#include <linux/printk.h>
#include <linux/kprobes.h>
@@ -17,35 +14,6 @@
static DEFINE_PER_CPU(int, printk_context);
-#ifdef CONFIG_PRINTK_NMI
-void noinstr printk_nmi_enter(void)
-{
- this_cpu_add(printk_context, PRINTK_NMI_CONTEXT_OFFSET);
-}
-
-void noinstr printk_nmi_exit(void)
-{
- this_cpu_sub(printk_context, PRINTK_NMI_CONTEXT_OFFSET);
-}
-
-/*
- * Marks a code that might produce many messages in NMI context
- * and the risk of losing them is more critical than eventual
- * reordering.
- */
-void printk_nmi_direct_enter(void)
-{
- if (this_cpu_read(printk_context) & PRINTK_NMI_CONTEXT_MASK)
- this_cpu_or(printk_context, PRINTK_NMI_DIRECT_CONTEXT_MASK);
-}
-
-void printk_nmi_direct_exit(void)
-{
- this_cpu_and(printk_context, ~PRINTK_NMI_DIRECT_CONTEXT_MASK);
-}
-
-#endif /* CONFIG_PRINTK_NMI */
-
/* Can be preempted by NMI. */
void __printk_safe_enter(void)
{
@@ -70,10 +38,7 @@ asmlinkage int vprintk(const char *fmt, va_list args)
* Use the main logbuf even in NMI. But avoid calling console
* drivers that might have their own locks.
*/
- if (this_cpu_read(printk_context) &
- (PRINTK_NMI_DIRECT_CONTEXT_MASK |
- PRINTK_NMI_CONTEXT_MASK |
- PRINTK_SAFE_CONTEXT_MASK)) {
+ if (this_cpu_read(printk_context) || in_nmi()) {
int len;
len = vprintk_store(0, LOGLEVEL_DEFAULT, NULL, fmt, args);
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index d23a09d3eb37..b30ad20d251f 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -9647,7 +9647,7 @@ void ftrace_dump(enum ftrace_dump_mode oops_dump_mode)
tracing_off();
local_irq_save(flags);
- printk_nmi_direct_enter();
+ printk_deferred_enter();
/* Simulate the iterator */
trace_init_global_iter(&iter);
@@ -9729,7 +9729,7 @@ void ftrace_dump(enum ftrace_dump_mode oops_dump_mode)
atomic_dec(&per_cpu_ptr(iter.array_buffer->data, cpu)->disabled);
}
atomic_dec(&dump_running);
- printk_nmi_direct_exit();
+ printk_deferred_exit();
local_irq_restore(flags);
}
EXPORT_SYMBOL_GPL(ftrace_dump);
--
2.20.1
^ permalink raw reply related
* [PATCH printk v4 0/6] printk: remove safe buffers
From: John Ogness @ 2021-07-15 19:33 UTC (permalink / raw)
To: Petr Mladek
Cc: Alexey Kardashevskiy, Rasmus Villemoes, Wolfram Sang (Renesas),
Paul Mackerras, Tiezhu Yang, Daniel Borkmann, Marc Zyngier,
Masahiro Yamada, Russell King, Peter Zijlstra, Yue Hu,
Ingo Molnar, Geert Uytterhoeven, linux-arm-kernel,
Valentin Schneider, Kees Cook, Paul E. McKenney,
Anshuman Khandual, Vipin Sharma, Frederic Weisbecker,
Nicholas Piggin, Nathan Chancellor, Nick Terrell, Steven Rostedt,
Thomas Gleixner, Vlastimil Babka, Nick Desaulniers, Xiongwei Song,
kexec, linux-kernel, Sergey Senozhatsky, Eric Biederman,
Andrew Morton, linuxppc-dev, Mike Rapoport, Cédric Le Goater
Hi,
Here is v4 of a series to remove the safe buffers. v3 can be
found here [0]. The safe buffers are no longer needed because
messages can be stored directly into the log buffer from any
context.
However, the safe buffers also provided a form of recursion
protection. For that reason, explicit recursion protection is
implemented for this series.
The safe buffers also implicitly provided serialization
between multiple CPUs executing in NMI context. This was
particularly necessary for the nmi_backtrace() output. This
serializiation is now preserved by using the printk cpulock.
With the removal of the safe buffers, there is no need for
extra NMI enter/exit tracking. So this is also removed
(which includes removing the config option CONFIG_PRINTK_NMI).
And finally, there are a few places in the kernel that need to
specify code blocks where all printk calls are to be deferred
printing. Previously the NMI tracking API was being (mis)used
for this purpose. This series introduces an official and
explicit interface for such cases. (Note that all deferred
printing will be removed anyway, once printing kthreads are
introduced.)
Changes since v3:
- Remove safe context tracking in vprintk().
- Add safe context tracking for @console_owner usage since that
is also a component of the printing code.
- Refactor syslog_print() so that it is easier to understand
and follow the locking logic.
- Introduce printk_deferred_enter/exit functions to be used by
code that needs to specify code block where all printk calls
are to be deferred printing.
John Ogness
[0] https://lore.kernel.org/lkml/20210624111148.5190-1-john.ogness@linutronix.de
John Ogness (6):
lib/nmi_backtrace: explicitly serialize banner and regs
printk: track/limit recursion
printk: remove safe buffers
printk: remove NMI tracking
printk: convert @syslog_lock to mutex
printk: syslog: close window between wait and read
arch/arm/kernel/smp.c | 4 +-
arch/powerpc/kernel/traps.c | 1 -
arch/powerpc/kernel/watchdog.c | 5 -
arch/powerpc/kexec/crash.c | 2 +-
include/linux/hardirq.h | 2 -
include/linux/printk.h | 41 ++--
init/Kconfig | 5 -
kernel/kexec_core.c | 1 -
kernel/panic.c | 3 -
kernel/printk/internal.h | 25 ---
kernel/printk/printk.c | 268 ++++++++++++++----------
kernel/printk/printk_safe.c | 364 +--------------------------------
kernel/trace/trace.c | 4 +-
lib/nmi_backtrace.c | 13 +-
14 files changed, 194 insertions(+), 544 deletions(-)
base-commit: 70333dec446292cd896cd051d2ebd6808b328949
--
2.20.1
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox