* [PATCH v5 2/4] powerpc/pseries/iommu: Update call to ibm, query-pe-dma-windows
From: Leonardo Bras @ 2020-08-05 3:04 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Leonardo Bras, Alexey Kardashevskiy, Thiago Jung Bauermann,
Ram Pai, Brian King, Murilo Fossa Vicentini, David Dai
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20200805030455.123024-1-leobras.c@gmail.com>
From LoPAR level 2.8, "ibm,ddw-extensions" index 3 can make the number of
outputs from "ibm,query-pe-dma-windows" go from 5 to 6.
This change of output size is meant to expand the address size of
largest_available_block PE TCE from 32-bit to 64-bit, which ends up
shifting page_size and migration_capable.
This ends up requiring the update of
ddw_query_response->largest_available_block from u32 to u64, and manually
assigning the values from the buffer into this struct, according to
output size.
Also, a routine was created for helping reading the ddw extensions as
suggested by LoPAR: First reading the size of the extension array from
index 0, checking if the property exists, and then returning it's value.
Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
Tested-by: David Dai <zdai@linux.vnet.ibm.com>
---
arch/powerpc/platforms/pseries/iommu.c | 91 +++++++++++++++++++++++---
1 file changed, 81 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index ac0d6376bdad..1a933c4e8bba 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -47,6 +47,12 @@ enum {
DDW_APPLICABLE_SIZE
};
+enum {
+ DDW_EXT_SIZE = 0,
+ DDW_EXT_RESET_DMA_WIN = 1,
+ DDW_EXT_QUERY_OUT_SIZE = 2
+};
+
static struct iommu_table_group *iommu_pseries_alloc_group(int node)
{
struct iommu_table_group *table_group;
@@ -342,7 +348,7 @@ struct direct_window {
/* Dynamic DMA Window support */
struct ddw_query_response {
u32 windows_available;
- u32 largest_available_block;
+ u64 largest_available_block;
u32 page_size;
u32 migration_capable;
};
@@ -877,14 +883,62 @@ static int find_existing_ddw_windows(void)
}
machine_arch_initcall(pseries, find_existing_ddw_windows);
+/**
+ * ddw_read_ext - Get the value of an DDW extension
+ * @np: device node from which the extension value is to be read.
+ * @extnum: index number of the extension.
+ * @value: pointer to return value, modified when extension is available.
+ *
+ * Checks if "ibm,ddw-extensions" exists for this node, and get the value
+ * on index 'extnum'.
+ * It can be used only to check if a property exists, passing value == NULL.
+ *
+ * Returns:
+ * 0 if extension successfully read
+ * -EINVAL if the "ibm,ddw-extensions" does not exist,
+ * -ENODATA if "ibm,ddw-extensions" does not have a value, and
+ * -EOVERFLOW if "ibm,ddw-extensions" does not contain this extension.
+ */
+static inline int ddw_read_ext(const struct device_node *np, int extnum,
+ u32 *value)
+{
+ static const char propname[] = "ibm,ddw-extensions";
+ u32 count;
+ int ret;
+
+ ret = of_property_read_u32_index(np, propname, DDW_EXT_SIZE, &count);
+ if (ret)
+ return ret;
+
+ if (count < extnum)
+ return -EOVERFLOW;
+
+ if (!value)
+ value = &count;
+
+ return of_property_read_u32_index(np, propname, extnum, value);
+}
+
static int query_ddw(struct pci_dev *dev, const u32 *ddw_avail,
- struct ddw_query_response *query)
+ struct ddw_query_response *query,
+ struct device_node *parent)
{
struct device_node *dn;
struct pci_dn *pdn;
- u32 cfg_addr;
+ u32 cfg_addr, ext_query, query_out[5];
u64 buid;
- int ret;
+ int ret, out_sz;
+
+ /*
+ * From LoPAR level 2.8, "ibm,ddw-extensions" index 3 can rule how many
+ * output parameters ibm,query-pe-dma-windows will have, ranging from
+ * 5 to 6.
+ */
+ ret = ddw_read_ext(parent, DDW_EXT_QUERY_OUT_SIZE, &ext_query);
+ if (!ret && ext_query == 1)
+ out_sz = 6;
+ else
+ out_sz = 5;
/*
* Get the config address and phb buid of the PE window.
@@ -897,11 +951,28 @@ static int query_ddw(struct pci_dev *dev, const u32 *ddw_avail,
buid = pdn->phb->buid;
cfg_addr = ((pdn->busno << 16) | (pdn->devfn << 8));
- ret = rtas_call(ddw_avail[DDW_QUERY_PE_DMA_WIN], 3, 5, (u32 *)query,
+ ret = rtas_call(ddw_avail[DDW_QUERY_PE_DMA_WIN], 3, out_sz, query_out,
cfg_addr, BUID_HI(buid), BUID_LO(buid));
- dev_info(&dev->dev, "ibm,query-pe-dma-windows(%x) %x %x %x"
- " returned %d\n", ddw_avail[DDW_QUERY_PE_DMA_WIN], cfg_addr,
- BUID_HI(buid), BUID_LO(buid), ret);
+ dev_info(&dev->dev, "ibm,query-pe-dma-windows(%x) %x %x %x returned %d\n",
+ ddw_avail[DDW_QUERY_PE_DMA_WIN], cfg_addr, BUID_HI(buid),
+ BUID_LO(buid), ret);
+
+ switch (out_sz) {
+ case 5:
+ query->windows_available = query_out[0];
+ query->largest_available_block = query_out[1];
+ query->page_size = query_out[2];
+ query->migration_capable = query_out[3];
+ break;
+ case 6:
+ query->windows_available = query_out[0];
+ query->largest_available_block = ((u64)query_out[1] << 32) |
+ query_out[2];
+ query->page_size = query_out[3];
+ query->migration_capable = query_out[4];
+ break;
+ }
+
return ret;
}
@@ -1049,7 +1120,7 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
* of page sizes: supported and supported for migrate-dma.
*/
dn = pci_device_to_OF_node(dev);
- ret = query_ddw(dev, ddw_avail, &query);
+ ret = query_ddw(dev, ddw_avail, &query, pdn);
if (ret != 0)
goto out_failed;
@@ -1077,7 +1148,7 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
/* check largest block * page size > max memory hotplug addr */
max_addr = ddw_memory_hotplug_max();
if (query.largest_available_block < (max_addr >> page_shift)) {
- dev_dbg(&dev->dev, "can't map partition max 0x%llx with %u "
+ dev_dbg(&dev->dev, "can't map partition max 0x%llx with %llu "
"%llu-sized pages\n", max_addr, query.largest_available_block,
1ULL << page_shift);
goto out_failed;
--
2.25.4
^ permalink raw reply related
* [PATCH v5 1/4] powerpc/pseries/iommu: Create defines for operations in ibm, ddw-applicable
From: Leonardo Bras @ 2020-08-05 3:04 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Leonardo Bras, Alexey Kardashevskiy, Thiago Jung Bauermann,
Ram Pai, Brian King, Murilo Fossa Vicentini, David Dai
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20200805030455.123024-1-leobras.c@gmail.com>
Create defines to help handling ibm,ddw-applicable values, avoiding
confusion about the index of given operations.
Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
Tested-by: David Dai <zdai@linux.vnet.ibm.com>
---
arch/powerpc/platforms/pseries/iommu.c | 43 ++++++++++++++++----------
1 file changed, 26 insertions(+), 17 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index 6d47b4a3ce39..ac0d6376bdad 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -39,6 +39,14 @@
#include "pseries.h"
+enum {
+ DDW_QUERY_PE_DMA_WIN = 0,
+ DDW_CREATE_PE_DMA_WIN = 1,
+ DDW_REMOVE_PE_DMA_WIN = 2,
+
+ DDW_APPLICABLE_SIZE
+};
+
static struct iommu_table_group *iommu_pseries_alloc_group(int node)
{
struct iommu_table_group *table_group;
@@ -771,12 +779,12 @@ static void remove_ddw(struct device_node *np, bool remove_prop)
{
struct dynamic_dma_window_prop *dwp;
struct property *win64;
- u32 ddw_avail[3];
+ u32 ddw_avail[DDW_APPLICABLE_SIZE];
u64 liobn;
int ret = 0;
ret = of_property_read_u32_array(np, "ibm,ddw-applicable",
- &ddw_avail[0], 3);
+ &ddw_avail[0], DDW_APPLICABLE_SIZE);
win64 = of_find_property(np, DIRECT64_PROPNAME, NULL);
if (!win64)
@@ -798,15 +806,15 @@ static void remove_ddw(struct device_node *np, bool remove_prop)
pr_debug("%pOF successfully cleared tces in window.\n",
np);
- ret = rtas_call(ddw_avail[2], 1, 1, NULL, liobn);
+ ret = rtas_call(ddw_avail[DDW_REMOVE_PE_DMA_WIN], 1, 1, NULL, liobn);
if (ret)
pr_warn("%pOF: failed to remove direct window: rtas returned "
"%d to ibm,remove-pe-dma-window(%x) %llx\n",
- np, ret, ddw_avail[2], liobn);
+ np, ret, ddw_avail[DDW_REMOVE_PE_DMA_WIN], liobn);
else
pr_debug("%pOF: successfully removed direct window: rtas returned "
"%d to ibm,remove-pe-dma-window(%x) %llx\n",
- np, ret, ddw_avail[2], liobn);
+ np, ret, ddw_avail[DDW_REMOVE_PE_DMA_WIN], liobn);
delprop:
if (remove_prop)
@@ -889,11 +897,11 @@ static int query_ddw(struct pci_dev *dev, const u32 *ddw_avail,
buid = pdn->phb->buid;
cfg_addr = ((pdn->busno << 16) | (pdn->devfn << 8));
- ret = rtas_call(ddw_avail[0], 3, 5, (u32 *)query,
- cfg_addr, BUID_HI(buid), BUID_LO(buid));
+ ret = rtas_call(ddw_avail[DDW_QUERY_PE_DMA_WIN], 3, 5, (u32 *)query,
+ cfg_addr, BUID_HI(buid), BUID_LO(buid));
dev_info(&dev->dev, "ibm,query-pe-dma-windows(%x) %x %x %x"
- " returned %d\n", ddw_avail[0], cfg_addr, BUID_HI(buid),
- BUID_LO(buid), ret);
+ " returned %d\n", ddw_avail[DDW_QUERY_PE_DMA_WIN], cfg_addr,
+ BUID_HI(buid), BUID_LO(buid), ret);
return ret;
}
@@ -920,15 +928,16 @@ static int create_ddw(struct pci_dev *dev, const u32 *ddw_avail,
do {
/* extra outputs are LIOBN and dma-addr (hi, lo) */
- ret = rtas_call(ddw_avail[1], 5, 4, (u32 *)create,
- cfg_addr, BUID_HI(buid), BUID_LO(buid),
- page_shift, window_shift);
+ ret = rtas_call(ddw_avail[DDW_CREATE_PE_DMA_WIN], 5, 4,
+ (u32 *)create, cfg_addr, BUID_HI(buid),
+ BUID_LO(buid), page_shift, window_shift);
} while (rtas_busy_delay(ret));
dev_info(&dev->dev,
"ibm,create-pe-dma-window(%x) %x %x %x %x %x returned %d "
- "(liobn = 0x%x starting addr = %x %x)\n", ddw_avail[1],
- cfg_addr, BUID_HI(buid), BUID_LO(buid), page_shift,
- window_shift, ret, create->liobn, create->addr_hi, create->addr_lo);
+ "(liobn = 0x%x starting addr = %x %x)\n",
+ ddw_avail[DDW_CREATE_PE_DMA_WIN], cfg_addr, BUID_HI(buid),
+ BUID_LO(buid), page_shift, window_shift, ret, create->liobn,
+ create->addr_hi, create->addr_lo);
return ret;
}
@@ -996,7 +1005,7 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
int page_shift;
u64 dma_addr, max_addr;
struct device_node *dn;
- u32 ddw_avail[3];
+ u32 ddw_avail[DDW_APPLICABLE_SIZE];
struct direct_window *window;
struct property *win64;
struct dynamic_dma_window_prop *ddwprop;
@@ -1029,7 +1038,7 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
* the property is actually in the parent, not the PE
*/
ret = of_property_read_u32_array(pdn, "ibm,ddw-applicable",
- &ddw_avail[0], 3);
+ &ddw_avail[0], DDW_APPLICABLE_SIZE);
if (ret)
goto out_failed;
--
2.25.4
^ permalink raw reply related
* [PATCH v5 0/4] Allow bigger 64bit window by removing default DMA window
From: Leonardo Bras @ 2020-08-05 3:04 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Leonardo Bras, Alexey Kardashevskiy, Thiago Jung Bauermann,
Ram Pai, Brian King, Murilo Fossa Vicentini, David Dai
Cc: linuxppc-dev, linux-kernel
There are some devices in which a hypervisor may only allow 1 DMA window
to exist at a time, and in those cases, a DDW is never created to them,
since the default DMA window keeps using this resource.
LoPAR recommends this procedure:
1. Remove the default DMA window,
2. Query for which configs the DDW can be created,
3. Create a DDW.
Patch #1:
Create defines for outputs of ibm,ddw-applicable, so it's easier to
identify them.
Patch #2:
- After LoPAR level 2.8, there is an extension that can make
ibm,query-pe-dma-windows to have 6 outputs instead of 5. This changes the
order of the outputs, and that can cause some trouble.
- query_ddw() was updated to check how many outputs the
ibm,query-pe-dma-windows is supposed to have, update the rtas_call() and
deal correctly with the outputs in both cases.
- This patch looks somehow unrelated to the series, but it can avoid future
problems on DDW creation.
Patch #3 moves the window-removing code from remove_ddw() to
remove_dma_window(), creating a way to delete any DMA window, so it can be
used to delete the default DMA window.
Patch #4 makes use of the remove_dma_window() from patch #3 to remove the
default DMA window before query_ddw(). It also implements a new rtas call
to recover the default DMA window, in case anything fails after it was
removed, and a DDW couldn't be created.
---
Changes since v4:
- Removed patches 5+ in order to deal with a feature at a time
- Remove unnecessary parentesis in patch #4
- Changed patch #4 title from
"Remove default DMA window before creating DDW"
- Included David Dai tested-by
- v4 link: http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=190051&state=%2A&archive=both
Changes since v3:
- Introduces new patch #5, to prepare for an important change in #6
- struct iommu_table was not being updated, so include a way to do this
in patch #6.
- Improved patch #4 based in a suggestion from Alexey, to make code
more easily understandable
- v3 link: http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=187348&state=%2A&archive=both
Changes since v2:
- Change the way ibm,ddw-extensions is accessed, using a proper function
instead of doing this inline everytime it's used.
- Remove previous patch #6, as it doesn't look like it would be useful.
- Add new patch, for changing names from direct* to dma*, as indirect
mapping can be used from now on.
- Fix some typos, corrects some define usage.
- v2 link: http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=185433&state=%2A&archive=both
Changes since v1:
- Add defines for ibm,ddw-applicable and ibm,ddw-extensions outputs
- Merge aux function query_ddw_out_sz() into query_ddw()
- Merge reset_dma_window() patch (prev. #2) into remove default DMA
window patch (#4).
- Keep device_node *np name instead of using pdn in remove_*()
- Rename 'device_node *pdn' into 'parent' in new functions
- Rename dfl_win to default_win
- Only remove the default DMA window if there is no window available
in first query.
- Check if default DMA window can be restored before removing it.
- Fix 'unitialized use' (found by travis mpe:ci-test)
- New patches #5 and #6
- v1 link: http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=184420&state=%2A&archive=both
Special thanks for Alexey Kardashevskiy, Brian King and
Oliver O'Halloran for the feedback provided!
Leonardo Bras (4):
powerpc/pseries/iommu: Create defines for operations in
ibm,ddw-applicable
powerpc/pseries/iommu: Update call to ibm,query-pe-dma-windows
powerpc/pseries/iommu: Move window-removing part of remove_ddw into
remove_dma_window
powerpc/pseries/iommu: Allow bigger 64bit window by removing default
DMA window
arch/powerpc/platforms/pseries/iommu.c | 242 ++++++++++++++++++++-----
1 file changed, 195 insertions(+), 47 deletions(-)
--
2.25.4
^ permalink raw reply
* [PATCH v2 2/2] ASoC: fsl_sai: Refine enable and disable sequence for synchronous mode
From: Shengjiu Wang @ 2020-08-05 2:23 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, lgirdwood, broonie,
perex, tiwai, alsa-devel, linuxppc-dev, linux-kernel
In-Reply-To: <1596594233-13489-1-git-send-email-shengjiu.wang@nxp.com>
Tx synchronous with Rx:
The TCSR.TE is no need to enabled when only Rx is going to be enabled.
Check if need to disable RSCR.RE before disabling TCSR.TE.
Rx synchronous with Tx:
The RCSR.RE is no need to enabled when only Tx is going to be enabled.
Check if need to disable TSCR.RE before disabling RCSR.TE.
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
sound/soc/fsl/fsl_sai.c | 126 +++++++++++++++++++++++++++-------------
1 file changed, 85 insertions(+), 41 deletions(-)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 84714fe7144c..f30c4e7b5221 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -517,6 +517,56 @@ static int fsl_sai_hw_free(struct snd_pcm_substream *substream,
return 0;
}
+/**
+ * fsl_sai_dir_is_synced - Check if stream is synced by the opposite stream
+ *
+ * SAI supports synchronous mode using bit/frame clocks of either Transmitter's
+ * or Receiver's for both streams. This function is used to check if clocks of
+ * the stream's are synced by the opposite stream.
+ *
+ * @sai: SAI context
+ * @dir: stream direction
+ */
+static inline bool fsl_sai_dir_is_synced(struct fsl_sai *sai, int dir)
+{
+ int adir = (dir == TX) ? RX : TX;
+
+ /* current dir in async mode while opposite dir in sync mode */
+ return !sai->synchronous[dir] && sai->synchronous[adir];
+}
+
+static void fsl_sai_config_disable(struct fsl_sai *sai, int dir)
+{
+ unsigned int ofs = sai->soc_data->reg_offset;
+ bool tx = dir == TX;
+ u32 xcsr, count = 100;
+
+ regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
+ FSL_SAI_CSR_TERE, 0);
+
+ /* TERE will remain set till the end of current frame */
+ do {
+ udelay(10);
+ regmap_read(sai->regmap, FSL_SAI_xCSR(tx, ofs), &xcsr);
+ } while (--count && xcsr & FSL_SAI_CSR_TERE);
+
+ regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
+ FSL_SAI_CSR_FR, FSL_SAI_CSR_FR);
+
+ /*
+ * For sai master mode, after several open/close sai,
+ * there will be no frame clock, and can't recover
+ * anymore. Add software reset to fix this issue.
+ * This is a hardware bug, and will be fix in the
+ * next sai version.
+ */
+ if (!sai->is_slave_mode) {
+ /* Software Reset */
+ regmap_write(sai->regmap, FSL_SAI_xCSR(tx, ofs), FSL_SAI_CSR_SR);
+ /* Clear SR bit to finish the reset */
+ regmap_write(sai->regmap, FSL_SAI_xCSR(tx, ofs), 0);
+ }
+}
static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
struct snd_soc_dai *cpu_dai)
@@ -525,7 +575,9 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
unsigned int ofs = sai->soc_data->reg_offset;
bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
- u32 xcsr, count = 100;
+ int adir = tx ? RX : TX;
+ int dir = tx ? TX : RX;
+ u32 xcsr;
/*
* Asynchronous mode: Clear SYNC for both Tx and Rx.
@@ -548,10 +600,22 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
FSL_SAI_CSR_FRDE, FSL_SAI_CSR_FRDE);
- regmap_update_bits(sai->regmap, FSL_SAI_RCSR(ofs),
- FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);
- regmap_update_bits(sai->regmap, FSL_SAI_TCSR(ofs),
+ regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);
+ /*
+ * Enable the opposite direction for synchronous mode
+ * 1. Tx sync with Rx: only set RE for Rx; set TE & RE for Tx
+ * 2. Rx sync with Tx: only set TE for Tx; set RE & TE for Rx
+ *
+ * RM recommends to enable RE after TE for case 1 and to enable
+ * TE after RE for case 2, but we here may not always guarantee
+ * that happens: "arecord 1.wav; aplay 2.wav" in case 1 enables
+ * TE after RE, which is against what RM recommends but should
+ * be safe to do, judging by years of testing results.
+ */
+ if (fsl_sai_dir_is_synced(sai, adir))
+ regmap_update_bits(sai->regmap, FSL_SAI_xCSR((!tx), ofs),
+ FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);
regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
FSL_SAI_CSR_xIE_MASK, FSL_SAI_FLAGS);
@@ -566,43 +630,23 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
/* Check if the opposite FRDE is also disabled */
regmap_read(sai->regmap, FSL_SAI_xCSR(!tx, ofs), &xcsr);
- if (!(xcsr & FSL_SAI_CSR_FRDE)) {
- /* Disable both directions and reset their FIFOs */
- regmap_update_bits(sai->regmap, FSL_SAI_TCSR(ofs),
- FSL_SAI_CSR_TERE, 0);
- regmap_update_bits(sai->regmap, FSL_SAI_RCSR(ofs),
- FSL_SAI_CSR_TERE, 0);
-
- /* TERE will remain set till the end of current frame */
- do {
- udelay(10);
- regmap_read(sai->regmap,
- FSL_SAI_xCSR(tx, ofs), &xcsr);
- } while (--count && xcsr & FSL_SAI_CSR_TERE);
-
- regmap_update_bits(sai->regmap, FSL_SAI_TCSR(ofs),
- FSL_SAI_CSR_FR, FSL_SAI_CSR_FR);
- regmap_update_bits(sai->regmap, FSL_SAI_RCSR(ofs),
- FSL_SAI_CSR_FR, FSL_SAI_CSR_FR);
-
- /*
- * For sai master mode, after several open/close sai,
- * there will be no frame clock, and can't recover
- * anymore. Add software reset to fix this issue.
- * This is a hardware bug, and will be fix in the
- * next sai version.
- */
- if (!sai->is_slave_mode) {
- /* Software Reset for both Tx and Rx */
- regmap_write(sai->regmap, FSL_SAI_TCSR(ofs),
- FSL_SAI_CSR_SR);
- regmap_write(sai->regmap, FSL_SAI_RCSR(ofs),
- FSL_SAI_CSR_SR);
- /* Clear SR bit to finish the reset */
- regmap_write(sai->regmap, FSL_SAI_TCSR(ofs), 0);
- regmap_write(sai->regmap, FSL_SAI_RCSR(ofs), 0);
- }
- }
+
+ /*
+ * If opposite stream provides clocks for synchronous mode and
+ * it is inactive, disable it before disabling the current one
+ */
+ if (fsl_sai_dir_is_synced(sai, adir) && !(xcsr & FSL_SAI_CSR_FRDE))
+ fsl_sai_config_disable(sai, adir);
+
+ /*
+ * Disable current stream if either of:
+ * 1. current stream doesn't provide clocks for synchronous mode
+ * 2. current stream provides clocks for synchronous mode but no
+ * more stream is active.
+ */
+ if (!fsl_sai_dir_is_synced(sai, dir) || !(xcsr & FSL_SAI_CSR_FRDE))
+ fsl_sai_config_disable(sai, dir);
+
break;
default:
return -EINVAL;
--
2.27.0
^ permalink raw reply related
* [PATCH v2 1/2] ASoC: fsl_sai: Clean code for synchronous mode
From: Shengjiu Wang @ 2020-08-05 2:23 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, lgirdwood, broonie,
perex, tiwai, alsa-devel, linuxppc-dev, linux-kernel
In-Reply-To: <1596594233-13489-1-git-send-email-shengjiu.wang@nxp.com>
Tx synchronous with Rx: The RMR is the word mask register, it is used
to mask any word in the frame, it is not relating to clock generation,
So it is no need to be changed when Tx is going to be enabled.
Rx synchronous with Tx: The TMR is the word mask register, it is used
to mask any word in the frame, it is not relating to clock generation,
So it is no need to be changed when Rx is going to be enabled.
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
sound/soc/fsl/fsl_sai.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index cdff739924e2..84714fe7144c 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -470,8 +470,7 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
/*
* For SAI master mode, when Tx(Rx) sync with Rx(Tx) clock, Rx(Tx) will
* generate bclk and frame clock for Tx(Rx), we should set RCR4(TCR4),
- * RCR5(TCR5) and RMR(TMR) for playback(capture), or there will be sync
- * error.
+ * RCR5(TCR5) for playback(capture), or there will be sync error.
*/
if (!sai->is_slave_mode) {
@@ -482,8 +481,6 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
regmap_update_bits(sai->regmap, FSL_SAI_TCR5(ofs),
FSL_SAI_CR5_WNW_MASK | FSL_SAI_CR5_W0W_MASK |
FSL_SAI_CR5_FBT_MASK, val_cr5);
- regmap_write(sai->regmap, FSL_SAI_TMR,
- ~0UL - ((1 << channels) - 1));
} else if (!sai->synchronous[RX] && sai->synchronous[TX] && tx) {
regmap_update_bits(sai->regmap, FSL_SAI_RCR4(ofs),
FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK,
@@ -491,8 +488,6 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
regmap_update_bits(sai->regmap, FSL_SAI_RCR5(ofs),
FSL_SAI_CR5_WNW_MASK | FSL_SAI_CR5_W0W_MASK |
FSL_SAI_CR5_FBT_MASK, val_cr5);
- regmap_write(sai->regmap, FSL_SAI_RMR,
- ~0UL - ((1 << channels) - 1));
}
}
--
2.27.0
^ permalink raw reply related
* [PATCH v2 0/2] refine and clean code for synchronous mode
From: Shengjiu Wang @ 2020-08-05 2:23 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, lgirdwood, broonie,
perex, tiwai, alsa-devel, linuxppc-dev, linux-kernel
refine and clean code for synchronous mode
Shengjiu Wang (2):
ASoC: fsl_sai: Clean code for synchronous mode
ASoC: fsl_sai: Refine enable and disable sequence for synchronous mode
changes in v2:
- Split the commit
- refine the sequence in trigger stop
sound/soc/fsl/fsl_sai.c | 133 ++++++++++++++++++++++++++--------------
1 file changed, 86 insertions(+), 47 deletions(-)
--
2.27.0
^ permalink raw reply
* Re: [PATCH] powerpc/powernv/sriov: Fix use of uninitialised variable
From: Michael Ellerman @ 2020-08-05 0:42 UTC (permalink / raw)
To: linuxppc-dev, Oliver O'Halloran; +Cc: Nathan Chancellor
In-Reply-To: <20200803075408.132601-1-oohall@gmail.com>
On Mon, 3 Aug 2020 17:54:08 +1000, Oliver O'Halloran wrote:
> Initialising the value before using it is generally regarded as a good
> idea so do that.
Applied to powerpc/next.
[1/1] powerpc/powernv/sriov: Fix use of uninitialised variable
https://git.kernel.org/powerpc/c/2075ec9896c5aef01e837198381d04cfa6452317
cheers
^ permalink raw reply
* Re: [PATCH v2] selftests/powerpc: Skip vmx/vsx/tar/etc tests on older CPUs
From: Michael Ellerman @ 2020-08-05 0:42 UTC (permalink / raw)
To: linuxppc-dev, Michael Ellerman
In-Reply-To: <20200803020719.96114-1-mpe@ellerman.id.au>
On Mon, 3 Aug 2020 12:07:19 +1000, Michael Ellerman wrote:
> Some of our tests use VSX or newer VMX instructions, so need to be
> skipped on older CPUs to avoid SIGILL'ing.
>
> Similarly TAR was added in v2.07, and the PMU event used in the stcx
> fail test only works on Power8 or later.
Applied to powerpc/next.
[1/1] selftests/powerpc: Skip vmx/vsx/tar/etc tests on older CPUs
https://git.kernel.org/powerpc/c/872d11bca9c29ed19595c993b9f552ffe9b63dcb
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/40x: Fix assembler warning about r0
From: Michael Ellerman @ 2020-08-05 0:42 UTC (permalink / raw)
To: linuxppc-dev, Michael Ellerman
In-Reply-To: <20200722022422.825197-1-mpe@ellerman.id.au>
On Wed, 22 Jul 2020 12:24:22 +1000, Michael Ellerman wrote:
> The assembler says:
> arch/powerpc/kernel/head_40x.S:623: Warning: invalid register expression
>
> It's objecting to the use of r0 as the RA argument. That's because
> when RA = 0 the literal value 0 is used, rather than the content of
> r0, making the use of r0 in the source potentially confusing.
>
> [...]
Applied to powerpc/next.
[1/1] powerpc/40x: Fix assembler warning about r0
https://git.kernel.org/powerpc/c/8d8a629d00a5283874b81b594f31f8d436dc57d8
cheers
^ permalink raw reply
* Re: [PATCH v4 1/7] powerpc/pseries/iommu: Create defines for operations in ibm, ddw-applicable
From: David Dai @ 2020-08-04 21:34 UTC (permalink / raw)
To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Alexey Kardashevskiy, Joel Stanley,
Christophe Leroy, Thiago Jung Bauermann, Ram Pai, Brian King
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20200716071658.467820-2-leobras.c@gmail.com>
On Thu, 2020-07-16 at 04:16 -0300, Leonardo Bras wrote:
> Create defines to help handling ibm,ddw-applicable values, avoiding
> confusion about the index of given operations.
>
> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> ---
> arch/powerpc/platforms/pseries/iommu.c | 43 ++++++++++++++++------
> ----
> 1 file changed, 26 insertions(+), 17 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/iommu.c
> b/arch/powerpc/platforms/pseries/iommu.c
> index 6d47b4a3ce39..ac0d6376bdad 100644
> --- a/arch/powerpc/platforms/pseries/iommu.c
> +++ b/arch/powerpc/platforms/pseries/iommu.c
> @@ -39,6 +39,14 @@
>
> #include "pseries.h"
>
> +enum {
> + DDW_QUERY_PE_DMA_WIN = 0,
> + DDW_CREATE_PE_DMA_WIN = 1,
> + DDW_REMOVE_PE_DMA_WIN = 2,
> +
> + DDW_APPLICABLE_SIZE
> +};
> +
> static struct iommu_table_group *iommu_pseries_alloc_group(int node)
> {
> struct iommu_table_group *table_group;
> @@ -771,12 +779,12 @@ static void remove_ddw(struct device_node *np,
> bool remove_prop)
> {
> struct dynamic_dma_window_prop *dwp;
> struct property *win64;
> - u32 ddw_avail[3];
> + u32 ddw_avail[DDW_APPLICABLE_SIZE];
> u64 liobn;
> int ret = 0;
>
> ret = of_property_read_u32_array(np, "ibm,ddw-applicable",
> - &ddw_avail[0], 3);
> + &ddw_avail[0],
> DDW_APPLICABLE_SIZE);
>
> win64 = of_find_property(np, DIRECT64_PROPNAME, NULL);
> if (!win64)
> @@ -798,15 +806,15 @@ static void remove_ddw(struct device_node *np,
> bool remove_prop)
> pr_debug("%pOF successfully cleared tces in window.\n",
> np);
>
> - ret = rtas_call(ddw_avail[2], 1, 1, NULL, liobn);
> + ret = rtas_call(ddw_avail[DDW_REMOVE_PE_DMA_WIN], 1, 1, NULL,
> liobn);
> if (ret)
> pr_warn("%pOF: failed to remove direct window: rtas
> returned "
> "%d to ibm,remove-pe-dma-window(%x) %llx\n",
> - np, ret, ddw_avail[2], liobn);
> + np, ret, ddw_avail[DDW_REMOVE_PE_DMA_WIN],
> liobn);
> else
> pr_debug("%pOF: successfully removed direct window:
> rtas returned "
> "%d to ibm,remove-pe-dma-window(%x) %llx\n",
> - np, ret, ddw_avail[2], liobn);
> + np, ret, ddw_avail[DDW_REMOVE_PE_DMA_WIN],
> liobn);
>
> delprop:
> if (remove_prop)
> @@ -889,11 +897,11 @@ static int query_ddw(struct pci_dev *dev, const
> u32 *ddw_avail,
> buid = pdn->phb->buid;
> cfg_addr = ((pdn->busno << 16) | (pdn->devfn << 8));
>
> - ret = rtas_call(ddw_avail[0], 3, 5, (u32 *)query,
> - cfg_addr, BUID_HI(buid), BUID_LO(buid));
> + ret = rtas_call(ddw_avail[DDW_QUERY_PE_DMA_WIN], 3, 5, (u32
> *)query,
> + cfg_addr, BUID_HI(buid), BUID_LO(buid));
> dev_info(&dev->dev, "ibm,query-pe-dma-windows(%x) %x %x %x"
> - " returned %d\n", ddw_avail[0], cfg_addr,
> BUID_HI(buid),
> - BUID_LO(buid), ret);
> + " returned %d\n", ddw_avail[DDW_QUERY_PE_DMA_WIN],
> cfg_addr,
> + BUID_HI(buid), BUID_LO(buid), ret);
> return ret;
> }
>
> @@ -920,15 +928,16 @@ static int create_ddw(struct pci_dev *dev,
> const u32 *ddw_avail,
>
> do {
> /* extra outputs are LIOBN and dma-addr (hi, lo) */
> - ret = rtas_call(ddw_avail[1], 5, 4, (u32 *)create,
> - cfg_addr, BUID_HI(buid), BUID_LO(buid),
> - page_shift, window_shift);
> + ret = rtas_call(ddw_avail[DDW_CREATE_PE_DMA_WIN], 5, 4,
> + (u32 *)create, cfg_addr, BUID_HI(buid),
> + BUID_LO(buid), page_shift,
> window_shift);
> } while (rtas_busy_delay(ret));
> dev_info(&dev->dev,
> "ibm,create-pe-dma-window(%x) %x %x %x %x %x returned
> %d "
> - "(liobn = 0x%x starting addr = %x %x)\n", ddw_avail[1],
> - cfg_addr, BUID_HI(buid), BUID_LO(buid), page_shift,
> - window_shift, ret, create->liobn, create->addr_hi,
> create->addr_lo);
> + "(liobn = 0x%x starting addr = %x %x)\n",
> + ddw_avail[DDW_CREATE_PE_DMA_WIN], cfg_addr,
> BUID_HI(buid),
> + BUID_LO(buid), page_shift, window_shift, ret, create-
> >liobn,
> + create->addr_hi, create->addr_lo);
>
> return ret;
> }
> @@ -996,7 +1005,7 @@ static u64 enable_ddw(struct pci_dev *dev,
> struct device_node *pdn)
> int page_shift;
> u64 dma_addr, max_addr;
> struct device_node *dn;
> - u32 ddw_avail[3];
> + u32 ddw_avail[DDW_APPLICABLE_SIZE];
> struct direct_window *window;
> struct property *win64;
> struct dynamic_dma_window_prop *ddwprop;
> @@ -1029,7 +1038,7 @@ static u64 enable_ddw(struct pci_dev *dev,
> struct device_node *pdn)
> * the property is actually in the parent, not the PE
> */
> ret = of_property_read_u32_array(pdn, "ibm,ddw-applicable",
> - &ddw_avail[0], 3);
> + &ddw_avail[0],
> DDW_APPLICABLE_SIZE);
> if (ret)
> goto out_failed;
>
Tested-by: David Dai <zdai@linux.vnet.ibm.com>
^ permalink raw reply
* Re: [PATCH v4 4/7] powerpc/pseries/iommu: Remove default DMA window before creating DDW
From: David Dai @ 2020-08-04 21:33 UTC (permalink / raw)
To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Alexey Kardashevskiy, Joel Stanley,
Christophe Leroy, Thiago Jung Bauermann, Ram Pai, Brian King
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20200716071658.467820-5-leobras.c@gmail.com>
On Thu, 2020-07-16 at 04:16 -0300, Leonardo Bras wrote:
> On LoPAR "DMA Window Manipulation Calls", it's recommended to remove
> the
> default DMA window for the device, before attempting to configure a
> DDW,
> in order to make the maximum resources available for the next DDW to
> be
> created.
>
> This is a requirement for using DDW on devices in which hypervisor
> allows only one DMA window.
>
> If setting up a new DDW fails anywhere after the removal of this
> default DMA window, it's needed to restore the default DMA window.
> For this, an implementation of ibm,reset-pe-dma-windows rtas call is
> needed:
>
> Platforms supporting the DDW option starting with LoPAR level 2.7
> implement
> ibm,ddw-extensions. The first extension available (index 2) carries
> the
> token for ibm,reset-pe-dma-windows rtas call, which is used to
> restore
> the default DMA window for a device, if it has been deleted.
>
> It does so by resetting the TCE table allocation for the PE to it's
> boot time value, available in "ibm,dma-window" device tree node.
>
> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> ---
> arch/powerpc/platforms/pseries/iommu.c | 73 +++++++++++++++++++++++-
> --
> 1 file changed, 66 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/iommu.c
> b/arch/powerpc/platforms/pseries/iommu.c
> index 4e33147825cc..fc8d0555e2e9 100644
> --- a/arch/powerpc/platforms/pseries/iommu.c
> +++ b/arch/powerpc/platforms/pseries/iommu.c
> @@ -1066,6 +1066,38 @@ static phys_addr_t
> ddw_memory_hotplug_max(void)
> return max_addr;
> }
>
> +/*
> + * Platforms supporting the DDW option starting with LoPAR level 2.7
> implement
> + * ibm,ddw-extensions, which carries the rtas token for
> + * ibm,reset-pe-dma-windows.
> + * That rtas-call can be used to restore the default DMA window for
> the device.
> + */
> +static void reset_dma_window(struct pci_dev *dev, struct device_node
> *par_dn)
> +{
> + int ret;
> + u32 cfg_addr, reset_dma_win;
> + u64 buid;
> + struct device_node *dn;
> + struct pci_dn *pdn;
> +
> + ret = ddw_read_ext(par_dn, DDW_EXT_RESET_DMA_WIN,
> &reset_dma_win);
> + if (ret)
> + return;
> +
> + dn = pci_device_to_OF_node(dev);
> + pdn = PCI_DN(dn);
> + buid = pdn->phb->buid;
> + cfg_addr = ((pdn->busno << 16) | (pdn->devfn << 8));
> +
> + ret = rtas_call(reset_dma_win, 3, 1, NULL, cfg_addr,
> BUID_HI(buid),
> + BUID_LO(buid));
> + if (ret)
> + dev_info(&dev->dev,
> + "ibm,reset-pe-dma-windows(%x) %x %x %x
> returned %d ",
> + reset_dma_win, cfg_addr, BUID_HI(buid),
> BUID_LO(buid),
> + ret);
> +}
> +
> /*
> * 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,
> @@ -1090,6 +1122,7 @@ static u64 enable_ddw(struct pci_dev *dev,
> struct device_node *pdn)
> struct property *win64;
> struct dynamic_dma_window_prop *ddwprop;
> struct failed_ddw_pdn *fpdn;
> + bool default_win_removed = false;
>
> mutex_lock(&direct_window_init_mutex);
>
> @@ -1133,14 +1166,38 @@ static u64 enable_ddw(struct pci_dev *dev,
> struct device_node *pdn)
> if (ret != 0)
> goto out_failed;
>
> + /*
> + * If there is no window available, remove the default DMA
> window,
> + * if it's present. This will make all the resources available
> to the
> + * new DDW window.
> + * If anything fails after this, we need to restore it, so also
> check
> + * for extensions presence.
> + */
> if (query.windows_available == 0) {
> - /*
> - * no additional windows are available for this device.
> - * We might be able to reallocate the existing window,
> - * trading in for a larger page size.
> - */
> - dev_dbg(&dev->dev, "no free dynamic windows");
> - goto out_failed;
> + struct property *default_win;
> + int reset_win_ext;
> +
> + default_win = of_find_property(pdn, "ibm,dma-window",
> NULL);
> + if (!default_win)
> + goto out_failed;
> +
> + reset_win_ext = ddw_read_ext(pdn,
> DDW_EXT_RESET_DMA_WIN, NULL);
> + if (reset_win_ext)
> + goto out_failed;
> +
> + remove_dma_window(pdn, ddw_avail, default_win);
> + default_win_removed = true;
> +
> + /* Query again, to check if the window is available */
> + ret = query_ddw(dev, ddw_avail, &query, pdn);
> + if (ret != 0)
> + goto out_failed;
> +
> + if (query.windows_available == 0) {
> + /* no windows are available for this device. */
> + dev_dbg(&dev->dev, "no free dynamic windows");
> + goto out_failed;
> + }
> }
> if (query.page_size & 4) {
> page_shift = 24; /* 16MB */
> @@ -1231,6 +1288,8 @@ static u64 enable_ddw(struct pci_dev *dev,
> struct device_node *pdn)
> kfree(win64);
>
> out_failed:
> + if (default_win_removed)
> + reset_dma_window(dev, pdn);
>
> fpdn = kzalloc(sizeof(*fpdn), GFP_KERNEL);
> if (!fpdn)
Tested-by: David Dai <zdai@linux.vnet.ibm.com>
^ permalink raw reply
* Re: [PATCH v4 3/7] powerpc/pseries/iommu: Move window-removing part of remove_ddw into remove_dma_window
From: David Dai @ 2020-08-04 21:32 UTC (permalink / raw)
To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Alexey Kardashevskiy, Joel Stanley,
Christophe Leroy, Thiago Jung Bauermann, Ram Pai, Brian King
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20200716071658.467820-4-leobras.c@gmail.com>
On Thu, 2020-07-16 at 04:16 -0300, Leonardo Bras wrote:
> Move the window-removing part of remove_ddw into a new function
> (remove_dma_window), so it can be used to remove other DMA windows.
>
> It's useful for removing DMA windows that don't create
> DIRECT64_PROPNAME
> property, like the default DMA window from the device, which uses
> "ibm,dma-window".
>
> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> ---
> arch/powerpc/platforms/pseries/iommu.c | 45 +++++++++++++++---------
> --
> 1 file changed, 27 insertions(+), 18 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/iommu.c
> b/arch/powerpc/platforms/pseries/iommu.c
> index 1a933c4e8bba..4e33147825cc 100644
> --- a/arch/powerpc/platforms/pseries/iommu.c
> +++ b/arch/powerpc/platforms/pseries/iommu.c
> @@ -781,25 +781,14 @@ static int __init disable_ddw_setup(char *str)
>
> early_param("disable_ddw", disable_ddw_setup);
>
> -static void remove_ddw(struct device_node *np, bool remove_prop)
> +static void remove_dma_window(struct device_node *np, u32
> *ddw_avail,
> + struct property *win)
> {
> struct dynamic_dma_window_prop *dwp;
> - struct property *win64;
> - u32 ddw_avail[DDW_APPLICABLE_SIZE];
> u64 liobn;
> - int ret = 0;
> -
> - ret = of_property_read_u32_array(np, "ibm,ddw-applicable",
> - &ddw_avail[0],
> DDW_APPLICABLE_SIZE);
> -
> - win64 = of_find_property(np, DIRECT64_PROPNAME, NULL);
> - if (!win64)
> - return;
> -
> - if (ret || win64->length < sizeof(*dwp))
> - goto delprop;
> + int ret;
>
> - dwp = win64->value;
> + dwp = win->value;
> liobn = (u64)be32_to_cpu(dwp->liobn);
>
> /* clear the whole window, note the arg is in kernel pages */
> @@ -821,10 +810,30 @@ static void remove_ddw(struct device_node *np,
> bool remove_prop)
> pr_debug("%pOF: successfully removed direct window:
> rtas returned "
> "%d to ibm,remove-pe-dma-window(%x) %llx\n",
> np, ret, ddw_avail[DDW_REMOVE_PE_DMA_WIN],
> liobn);
> +}
> +
> +static void remove_ddw(struct device_node *np, bool remove_prop)
> +{
> + struct property *win;
> + u32 ddw_avail[DDW_APPLICABLE_SIZE];
> + int ret = 0;
> +
> + 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;
> +
> + if (win->length >= sizeof(struct dynamic_dma_window_prop))
> + remove_dma_window(np, ddw_avail, win);
> +
> + if (!remove_prop)
> + return;
>
> -delprop:
> - if (remove_prop)
> - ret = of_remove_property(np, win64);
> + ret = of_remove_property(np, win);
> if (ret)
> pr_warn("%pOF: failed to remove direct window property:
> %d\n",
> np, ret);
Tested-by: David Dai <zdai@linux.vnet.ibm.com>
^ permalink raw reply
* Re: [PATCH v4 2/7] powerpc/pseries/iommu: Update call to ibm, query-pe-dma-windows
From: David Dai @ 2020-08-04 21:31 UTC (permalink / raw)
To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Alexey Kardashevskiy, Joel Stanley,
Christophe Leroy, Thiago Jung Bauermann, Ram Pai, Brian King
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20200716071658.467820-3-leobras.c@gmail.com>
On Thu, 2020-07-16 at 04:16 -0300, Leonardo Bras wrote:
> > From LoPAR level 2.8, "ibm,ddw-extensions" index 3 can make the
> > number of
>
> outputs from "ibm,query-pe-dma-windows" go from 5 to 6.
>
> This change of output size is meant to expand the address size of
> largest_available_block PE TCE from 32-bit to 64-bit, which ends up
> shifting page_size and migration_capable.
>
> This ends up requiring the update of
> ddw_query_response->largest_available_block from u32 to u64, and
> manually
> assigning the values from the buffer into this struct, according to
> output size.
>
> Also, a routine was created for helping reading the ddw extensions as
> suggested by LoPAR: First reading the size of the extension array
> from
> index 0, checking if the property exists, and then returning it's
> value.
>
> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> ---
> arch/powerpc/platforms/pseries/iommu.c | 91 +++++++++++++++++++++++-
> --
> 1 file changed, 81 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/iommu.c
> b/arch/powerpc/platforms/pseries/iommu.c
> index ac0d6376bdad..1a933c4e8bba 100644
> --- a/arch/powerpc/platforms/pseries/iommu.c
> +++ b/arch/powerpc/platforms/pseries/iommu.c
> @@ -47,6 +47,12 @@ enum {
> DDW_APPLICABLE_SIZE
> };
>
> +enum {
> + DDW_EXT_SIZE = 0,
> + DDW_EXT_RESET_DMA_WIN = 1,
> + DDW_EXT_QUERY_OUT_SIZE = 2
> +};
> +
> static struct iommu_table_group *iommu_pseries_alloc_group(int node)
> {
> struct iommu_table_group *table_group;
> @@ -342,7 +348,7 @@ struct direct_window {
> /* Dynamic DMA Window support */
> struct ddw_query_response {
> u32 windows_available;
> - u32 largest_available_block;
> + u64 largest_available_block;
> u32 page_size;
> u32 migration_capable;
> };
> @@ -877,14 +883,62 @@ static int find_existing_ddw_windows(void)
> }
> machine_arch_initcall(pseries, find_existing_ddw_windows);
>
> +/**
> + * ddw_read_ext - Get the value of an DDW extension
> + * @np: device node from which the extension value is
> to be read.
> + * @extnum: index number of the extension.
> + * @value: pointer to return value, modified when extension is
> available.
> + *
> + * Checks if "ibm,ddw-extensions" exists for this node, and get the
> value
> + * on index 'extnum'.
> + * It can be used only to check if a property exists, passing value
> == NULL.
> + *
> + * Returns:
> + * 0 if extension successfully read
> + * -EINVAL if the "ibm,ddw-extensions" does not exist,
> + * -ENODATA if "ibm,ddw-extensions" does not have a value, and
> + * -EOVERFLOW if "ibm,ddw-extensions" does not contain this
> extension.
> + */
> +static inline int ddw_read_ext(const struct device_node *np, int
> extnum,
> + u32 *value)
> +{
> + static const char propname[] = "ibm,ddw-extensions";
> + u32 count;
> + int ret;
> +
> + ret = of_property_read_u32_index(np, propname, DDW_EXT_SIZE,
> &count);
> + if (ret)
> + return ret;
> +
> + if (count < extnum)
> + return -EOVERFLOW;
> +
> + if (!value)
> + value = &count;
> +
> + return of_property_read_u32_index(np, propname, extnum, value);
> +}
> +
> static int query_ddw(struct pci_dev *dev, const u32 *ddw_avail,
> - struct ddw_query_response *query)
> + struct ddw_query_response *query,
> + struct device_node *parent)
> {
> struct device_node *dn;
> struct pci_dn *pdn;
> - u32 cfg_addr;
> + u32 cfg_addr, ext_query, query_out[5];
> u64 buid;
> - int ret;
> + int ret, out_sz;
> +
> + /*
> + * From LoPAR level 2.8, "ibm,ddw-extensions" index 3 can rule
> how many
> + * output parameters ibm,query-pe-dma-windows will have,
> ranging from
> + * 5 to 6.
> + */
> + ret = ddw_read_ext(parent, DDW_EXT_QUERY_OUT_SIZE, &ext_query);
> + if (!ret && ext_query == 1)
> + out_sz = 6;
> + else
> + out_sz = 5;
>
> /*
> * Get the config address and phb buid of the PE window.
> @@ -897,11 +951,28 @@ static int query_ddw(struct pci_dev *dev, const
> u32 *ddw_avail,
> buid = pdn->phb->buid;
> cfg_addr = ((pdn->busno << 16) | (pdn->devfn << 8));
>
> - ret = rtas_call(ddw_avail[DDW_QUERY_PE_DMA_WIN], 3, 5, (u32
> *)query,
> + ret = rtas_call(ddw_avail[DDW_QUERY_PE_DMA_WIN], 3, out_sz,
> query_out,
> cfg_addr, BUID_HI(buid), BUID_LO(buid));
> - dev_info(&dev->dev, "ibm,query-pe-dma-windows(%x) %x %x %x"
> - " returned %d\n", ddw_avail[DDW_QUERY_PE_DMA_WIN],
> cfg_addr,
> - BUID_HI(buid), BUID_LO(buid), ret);
> + dev_info(&dev->dev, "ibm,query-pe-dma-windows(%x) %x %x %x
> returned %d\n",
> + ddw_avail[DDW_QUERY_PE_DMA_WIN], cfg_addr,
> BUID_HI(buid),
> + BUID_LO(buid), ret);
> +
> + switch (out_sz) {
> + case 5:
> + query->windows_available = query_out[0];
> + query->largest_available_block = query_out[1];
> + query->page_size = query_out[2];
> + query->migration_capable = query_out[3];
> + break;
> + case 6:
> + query->windows_available = query_out[0];
> + query->largest_available_block = ((u64)query_out[1] <<
> 32) |
> + query_out[2];
> + query->page_size = query_out[3];
> + query->migration_capable = query_out[4];
> + break;
> + }
> +
> return ret;
> }
>
> @@ -1049,7 +1120,7 @@ static u64 enable_ddw(struct pci_dev *dev,
> struct device_node *pdn)
> * of page sizes: supported and supported for migrate-dma.
> */
> dn = pci_device_to_OF_node(dev);
> - ret = query_ddw(dev, ddw_avail, &query);
> + ret = query_ddw(dev, ddw_avail, &query, pdn);
> if (ret != 0)
> goto out_failed;
>
> @@ -1077,7 +1148,7 @@ static u64 enable_ddw(struct pci_dev *dev,
> struct device_node *pdn)
> /* check largest block * page size > max memory hotplug addr */
> max_addr = ddw_memory_hotplug_max();
> if (query.largest_available_block < (max_addr >> page_shift)) {
> - dev_dbg(&dev->dev, "can't map partition max 0x%llx with
> %u "
> + dev_dbg(&dev->dev, "can't map partition max 0x%llx with
> %llu "
> "%llu-sized pages\n",
> max_addr, query.largest_available_block,
> 1ULL << page_shift);
> goto out_failed;
Tested-by: David Dai <zdai@linux.vnet.ibm.com>
^ permalink raw reply
* Re: [PATCH v4 1/7] powerpc/pseries/iommu: Create defines for operations in ibm, ddw-applicable
From: David Dai @ 2020-08-04 21:27 UTC (permalink / raw)
To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Alexey Kardashevskiy, Joel Stanley,
Christophe Leroy, Thiago Jung Bauermann, Ram Pai, Brian King
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20200716071658.467820-2-leobras.c@gmail.com>
On Thu, 2020-07-16 at 04:16 -0300, Leonardo Bras wrote:
> Create defines to help handling ibm,ddw-applicable values, avoiding
> confusion about the index of given operations.
>
> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> ---
> arch/powerpc/platforms/pseries/iommu.c | 43 ++++++++++++++++------
> ----
> 1 file changed, 26 insertions(+), 17 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/iommu.c
> b/arch/powerpc/platforms/pseries/iommu.c
> index 6d47b4a3ce39..ac0d6376bdad 100644
> --- a/arch/powerpc/platforms/pseries/iommu.c
> +++ b/arch/powerpc/platforms/pseries/iommu.c
> @@ -39,6 +39,14 @@
>
> #include "pseries.h"
>
> +enum {
> + DDW_QUERY_PE_DMA_WIN = 0,
> + DDW_CREATE_PE_DMA_WIN = 1,
> + DDW_REMOVE_PE_DMA_WIN = 2,
> +
> + DDW_APPLICABLE_SIZE
> +};
> +
> static struct iommu_table_group *iommu_pseries_alloc_group(int node)
> {
> struct iommu_table_group *table_group;
> @@ -771,12 +779,12 @@ static void remove_ddw(struct device_node *np,
> bool remove_prop)
> {
> struct dynamic_dma_window_prop *dwp;
> struct property *win64;
> - u32 ddw_avail[3];
> + u32 ddw_avail[DDW_APPLICABLE_SIZE];
> u64 liobn;
> int ret = 0;
>
> ret = of_property_read_u32_array(np, "ibm,ddw-applicable",
> - &ddw_avail[0], 3);
> + &ddw_avail[0],
> DDW_APPLICABLE_SIZE);
>
> win64 = of_find_property(np, DIRECT64_PROPNAME, NULL);
> if (!win64)
> @@ -798,15 +806,15 @@ static void remove_ddw(struct device_node *np,
> bool remove_prop)
> pr_debug("%pOF successfully cleared tces in window.\n",
> np);
>
> - ret = rtas_call(ddw_avail[2], 1, 1, NULL, liobn);
> + ret = rtas_call(ddw_avail[DDW_REMOVE_PE_DMA_WIN], 1, 1, NULL,
> liobn);
> if (ret)
> pr_warn("%pOF: failed to remove direct window: rtas
> returned "
> "%d to ibm,remove-pe-dma-window(%x) %llx\n",
> - np, ret, ddw_avail[2], liobn);
> + np, ret, ddw_avail[DDW_REMOVE_PE_DMA_WIN],
> liobn);
> else
> pr_debug("%pOF: successfully removed direct window:
> rtas returned "
> "%d to ibm,remove-pe-dma-window(%x) %llx\n",
> - np, ret, ddw_avail[2], liobn);
> + np, ret, ddw_avail[DDW_REMOVE_PE_DMA_WIN],
> liobn);
>
> delprop:
> if (remove_prop)
> @@ -889,11 +897,11 @@ static int query_ddw(struct pci_dev *dev, const
> u32 *ddw_avail,
> buid = pdn->phb->buid;
> cfg_addr = ((pdn->busno << 16) | (pdn->devfn << 8));
>
> - ret = rtas_call(ddw_avail[0], 3, 5, (u32 *)query,
> - cfg_addr, BUID_HI(buid), BUID_LO(buid));
> + ret = rtas_call(ddw_avail[DDW_QUERY_PE_DMA_WIN], 3, 5, (u32
> *)query,
> + cfg_addr, BUID_HI(buid), BUID_LO(buid));
> dev_info(&dev->dev, "ibm,query-pe-dma-windows(%x) %x %x %x"
> - " returned %d\n", ddw_avail[0], cfg_addr,
> BUID_HI(buid),
> - BUID_LO(buid), ret);
> + " returned %d\n", ddw_avail[DDW_QUERY_PE_DMA_WIN],
> cfg_addr,
> + BUID_HI(buid), BUID_LO(buid), ret);
> return ret;
> }
>
> @@ -920,15 +928,16 @@ static int create_ddw(struct pci_dev *dev,
> const u32 *ddw_avail,
>
> do {
> /* extra outputs are LIOBN and dma-addr (hi, lo) */
> - ret = rtas_call(ddw_avail[1], 5, 4, (u32 *)create,
> - cfg_addr, BUID_HI(buid), BUID_LO(buid),
> - page_shift, window_shift);
> + ret = rtas_call(ddw_avail[DDW_CREATE_PE_DMA_WIN], 5, 4,
> + (u32 *)create, cfg_addr, BUID_HI(buid),
> + BUID_LO(buid), page_shift,
> window_shift);
> } while (rtas_busy_delay(ret));
> dev_info(&dev->dev,
> "ibm,create-pe-dma-window(%x) %x %x %x %x %x returned
> %d "
> - "(liobn = 0x%x starting addr = %x %x)\n", ddw_avail[1],
> - cfg_addr, BUID_HI(buid), BUID_LO(buid), page_shift,
> - window_shift, ret, create->liobn, create->addr_hi,
> create->addr_lo);
> + "(liobn = 0x%x starting addr = %x %x)\n",
> + ddw_avail[DDW_CREATE_PE_DMA_WIN], cfg_addr,
> BUID_HI(buid),
> + BUID_LO(buid), page_shift, window_shift, ret, create-
> >liobn,
> + create->addr_hi, create->addr_lo);
>
> return ret;
> }
> @@ -996,7 +1005,7 @@ static u64 enable_ddw(struct pci_dev *dev,
> struct device_node *pdn)
> int page_shift;
> u64 dma_addr, max_addr;
> struct device_node *dn;
> - u32 ddw_avail[3];
> + u32 ddw_avail[DDW_APPLICABLE_SIZE];
> struct direct_window *window;
> struct property *win64;
> struct dynamic_dma_window_prop *ddwprop;
> @@ -1029,7 +1038,7 @@ static u64 enable_ddw(struct pci_dev *dev,
> struct device_node *pdn)
> * the property is actually in the parent, not the PE
> */
> ret = of_property_read_u32_array(pdn, "ibm,ddw-applicable",
> - &ddw_avail[0], 3);
> + &ddw_avail[0],
> DDW_APPLICABLE_SIZE);
> if (ret)
> goto out_failed;
>
Tested-by: David Dai <zdai@linux.vnet.ibm.com>
^ permalink raw reply
* Re: [PATCH v5 10/12] kunit: Add 'kunit_shutdown' option
From: Brendan Higgins @ 2020-08-04 20:18 UTC (permalink / raw)
To: Kees Cook
Cc: open list:DOCUMENTATION, catalin.marinas, jcmvbkbc, will,
Paul Mackerras, open list:KERNEL SELFTEST FRAMEWORK, Frank Rowand,
Anton Ivanov, linux-arch, Richard Weinberger, rppt, Iurii Zaikin,
linux-xtensa, Arnd Bergmann, Jeff Dike, linux-um, linuxppc-dev,
David Gow, Shuah Khan, Linux ARM, KUnit Development, Chris Zankel,
Michal Simek, Stephen Boyd, Greg KH, Linux Kernel Mailing List,
Luis Chamberlain, Alan Maguire, Andrew Morton, Logan Gunthorpe
In-Reply-To: <202006261436.DEF4906A5@keescook>
On Fri, Jun 26, 2020 at 2:40 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Fri, Jun 26, 2020 at 02:09:15PM -0700, Brendan Higgins wrote:
> > From: David Gow <davidgow@google.com>
> >
> > Add a new kernel command-line option, 'kunit_shutdown', which allows the
> > user to specify that the kernel poweroff, halt, or reboot after
> > completing all KUnit tests; this is very handy for running KUnit tests
> > on UML or a VM so that the UML/VM process exits cleanly immediately
> > after running all tests without needing a special initramfs.
> >
> > Signed-off-by: David Gow <davidgow@google.com>
> > Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> > Reviewed-by: Stephen Boyd <sboyd@kernel.org>
> > ---
> > lib/kunit/executor.c | 20 ++++++++++++++++++++
> > tools/testing/kunit/kunit_kernel.py | 2 +-
> > tools/testing/kunit/kunit_parser.py | 2 +-
> > 3 files changed, 22 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
> > index a95742a4ece73..38061d456afb2 100644
> > --- a/lib/kunit/executor.c
> > +++ b/lib/kunit/executor.c
> > @@ -1,5 +1,6 @@
> > // SPDX-License-Identifier: GPL-2.0
> >
> > +#include <linux/reboot.h>
> > #include <kunit/test.h>
> >
> > /*
> > @@ -11,6 +12,23 @@ extern struct kunit_suite * const * const __kunit_suites_end[];
> >
> > #if IS_BUILTIN(CONFIG_KUNIT)
> >
> > +static char *kunit_shutdown;
> > +core_param(kunit_shutdown, kunit_shutdown, charp, 0644);
> > +
> > +static void kunit_handle_shutdown(void)
> > +{
> > + if (!kunit_shutdown)
> > + return;
> > +
> > + if (!strcmp(kunit_shutdown, "poweroff"))
> > + kernel_power_off();
> > + else if (!strcmp(kunit_shutdown, "halt"))
> > + kernel_halt();
> > + else if (!strcmp(kunit_shutdown, "reboot"))
> > + kernel_restart(NULL);
> > +
> > +}
>
> If you have patches that do something just before the initrd, and then
> you add more patches to shut down immediately after an initrd, people
> may ask you to just use an initrd instead of filling the kernel with
> these changes...
>
> I mean, I get it, but it's not hard to make an initrd that poke a sysctl
> to start the tests...
>
> In fact, you don't even need a initrd to poke sysctls these days.
True, but it is nice to not have to maintain an initramfs for each
architecture that you want to test. Still, I see your point. If we can
find a convenient way to distribute the needed dependencies for
running KUnit on each non-UML architecture then I think I can abandon
this change.
So how about this: I will drop this patch from this patchset and move
it up to the follow up patchset that adds multiarchitecture support to
kunit_tool. There we can address the problem of how to best track the
necessary dependencies including possibly intitramfss.
^ permalink raw reply
* Re: [PATCH v5 09/12] kunit: test: add test plan to KUnit TAP format
From: Brendan Higgins @ 2020-08-04 20:10 UTC (permalink / raw)
To: Kees Cook
Cc: open list:DOCUMENTATION, catalin.marinas, jcmvbkbc, will,
Paul Mackerras, open list:KERNEL SELFTEST FRAMEWORK, Frank Rowand,
Anton Ivanov, linux-arch, Richard Weinberger, rppt, Iurii Zaikin,
linux-xtensa, Arnd Bergmann, Jeff Dike, linux-um, linuxppc-dev,
David Gow, Shuah Khan, Linux ARM, KUnit Development, Chris Zankel,
Michal Simek, Stephen Boyd, Greg KH, Linux Kernel Mailing List,
Luis Chamberlain, Alan Maguire, Andrew Morton, Logan Gunthorpe
In-Reply-To: <202006261434.119AE33DBB@keescook>
On Fri, Jun 26, 2020 at 2:35 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Fri, Jun 26, 2020 at 02:09:14PM -0700, Brendan Higgins wrote:
> > TAP 14 allows an optional test plan to be emitted before the start of
> > the start of testing[1]; this is valuable because it makes it possible
> > for a test harness to detect whether the number of tests run matches the
> > number of tests expected to be run, ensuring that no tests silently
> > failed.
> >
> > Link[1]: https://github.com/isaacs/testanything.github.io/blob/tap14/tap-version-14-specification.md#the-plan
> > Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> > Reviewed-by: Stephen Boyd <sboyd@kernel.org>
>
> Look good, except...
>
> > diff --git a/tools/testing/kunit/test_data/test_is_test_passed-all_passed.log b/tools/testing/kunit/test_data/test_is_test_passed-all_passed.log
> > index 62ebc0288355c4b122ccc18ae2505f971efa57bc..bc0dc8fe35b760b1feb74ec419818dbfae1adb5c 100644
> > GIT binary patch
> > delta 28
> > jcmbQmGoME|#4$jjEVZaOGe1wk(1goSPtRy09}gP<dC~`u
> >
> > delta 23
> > ecmbQwGmD2W#4$jjEVZaOGe1wk&}5@94;uhhkp{*9
> >
> > diff --git a/tools/testing/kunit/test_data/test_is_test_passed-crash.log b/tools/testing/kunit/test_data/test_is_test_passed-crash.log
> > index 0b249870c8be417a5865bd40a24c8597bb7f5ab1..4d97f6708c4a5ad5bb2ac879e12afca6e816d83d 100644
> > GIT binary patch
> > delta 15
> > WcmX>hepY;fFN>j`p3z318g2k9Uj*m?
> >
> > delta 10
> > RcmX>renNbL@5Z2NZU7lr1S$Xk
> >
> > diff --git a/tools/testing/kunit/test_data/test_is_test_passed-failure.log b/tools/testing/kunit/test_data/test_is_test_passed-failure.log
> > index 9e89d32d5667a59d137f8adacf3a88fdb7f88baf..7a416497e3bec044eefc1535f7d84ee85703ba97 100644
> > GIT binary patch
> > delta 28
> > jcmZ3&yOLKp#4$jjEVZaOGe1wk(1goSPtRy0-!wJ=eKrU$
> >
> > delta 23
> > ecmZ3<yM&i7#4$jjEVZaOGe1wk&}5_VG&TTPhX-Z=
>
> What is happening here?? Those logs appear as text to me. Why did git
> freak out?
That's because this is all test data; it's all plaintext, but out of
necessity some of the test data is kind of munged up and causes
checkpatch to complain, so Shuah asked us to mark it as binary since
it isn't actually code and so checkpatch will stop flagging it.
^ permalink raw reply
* Re: [PATCH v5 07/12] kunit: test: create a single centralized executor for all tests
From: Brendan Higgins @ 2020-08-04 20:06 UTC (permalink / raw)
To: Kees Cook
Cc: open list:DOCUMENTATION, catalin.marinas, jcmvbkbc, will,
Paul Mackerras, open list:KERNEL SELFTEST FRAMEWORK, Frank Rowand,
Anton Ivanov, linux-arch, Richard Weinberger, rppt, Iurii Zaikin,
linux-xtensa, Arnd Bergmann, Jeff Dike, linux-um, linuxppc-dev,
David Gow, Shuah Khan, Linux ARM, KUnit Development, Chris Zankel,
Michal Simek, Stephen Boyd, Greg KH, Linux Kernel Mailing List,
Luis Chamberlain, Alan Maguire, Andrew Morton, Logan Gunthorpe
In-Reply-To: <202006261423.0BC9D830@keescook>
On Fri, Jun 26, 2020 at 2:29 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Fri, Jun 26, 2020 at 02:09:12PM -0700, Brendan Higgins wrote:
> > From: Alan Maguire <alan.maguire@oracle.com>
> >
> > Add a centralized executor to dispatch tests rather than relying on
> > late_initcall to schedule each test suite separately. Centralized
> > execution is for built-in tests only; modules will execute tests when
> > loaded.
> >
> > Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> > Co-developed-by: Iurii Zaikin <yzaikin@google.com>
> > Signed-off-by: Iurii Zaikin <yzaikin@google.com>
> > Co-developed-by: Brendan Higgins <brendanhiggins@google.com>
> > Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> > Reviewed-by: Stephen Boyd <sboyd@kernel.org>
> > ---
> > include/kunit/test.h | 67 +++++++++++++++++++++++++++++---------------
> > lib/kunit/Makefile | 3 +-
> > lib/kunit/executor.c | 28 ++++++++++++++++++
> > lib/kunit/test.c | 2 +-
> > 4 files changed, 76 insertions(+), 24 deletions(-)
> > create mode 100644 lib/kunit/executor.c
> >
> > diff --git a/include/kunit/test.h b/include/kunit/test.h
> > index 47e61e1d53370..f3e86c3953a2b 100644
> > --- a/include/kunit/test.h
> > +++ b/include/kunit/test.h
> > @@ -224,7 +224,7 @@ size_t kunit_suite_num_test_cases(struct kunit_suite *suite);
> > unsigned int kunit_test_case_num(struct kunit_suite *suite,
> > struct kunit_case *test_case);
> >
> > -int __kunit_test_suites_init(struct kunit_suite **suites);
> > +int __kunit_test_suites_init(struct kunit_suite * const * const suites);
> >
> > void __kunit_test_suites_exit(struct kunit_suite **suites);
> >
> > @@ -237,34 +237,57 @@ void __kunit_test_suites_exit(struct kunit_suite **suites);
> > * Registers @suites_list with the test framework. See &struct kunit_suite for
> > * more information.
> > *
> > - * When builtin, KUnit tests are all run as late_initcalls; this means
> > - * that they cannot test anything where tests must run at a different init
> > - * phase. One significant restriction resulting from this is that KUnit
> > - * cannot reliably test anything that is initialize in the late_init phase;
> > - * another is that KUnit is useless to test things that need to be run in
> > - * an earlier init phase.
> > - *
> > - * An alternative is to build the tests as a module. Because modules
> > - * do not support multiple late_initcall()s, we need to initialize an
> > - * array of suites for a module.
> > - *
> > - * TODO(brendanhiggins@google.com): Don't run all KUnit tests as
> > - * late_initcalls. I have some future work planned to dispatch all KUnit
> > - * tests from the same place, and at the very least to do so after
> > - * everything else is definitely initialized.
> > + * If a test suite is built-in, module_init() gets translated into
> > + * an initcall which we don't want as the idea is that for builtins
> > + * the executor will manage execution. So ensure we do not define
> > + * module_{init|exit} functions for the builtin case when registering
> > + * suites via kunit_test_suites() below.
> > */
> > -#define kunit_test_suites(suites_list...) \
> > - static struct kunit_suite *suites[] = {suites_list, NULL}; \
> > - static int kunit_test_suites_init(void) \
> > +#ifdef MODULE
> > +#define kunit_test_suites_for_module(__suites) \
> > + static int __init kunit_test_suites_init(void) \
> > { \
> > - return __kunit_test_suites_init(suites); \
> > + return __kunit_test_suites_init(__suites); \
> > } \
> > - late_initcall(kunit_test_suites_init); \
> > + module_init(kunit_test_suites_init); \
> > + \
> > static void __exit kunit_test_suites_exit(void) \
> > { \
> > - return __kunit_test_suites_exit(suites); \
> > + return __kunit_test_suites_exit(__suites); \
> > } \
> > module_exit(kunit_test_suites_exit)
> > +#else
> > +#define kunit_test_suites_for_module(__suites)
> > +#endif /* MODULE */
> > +
> > +#define __kunit_test_suites(unique_array, unique_suites, ...) \
> > + static struct kunit_suite *unique_array[] = { __VA_ARGS__, NULL }; \
> > + kunit_test_suites_for_module(unique_array); \
> > + static struct kunit_suite **unique_suites \
> > + __used __section(.kunit_test_suites) = unique_array
> > +
> > +/**
> > + * kunit_test_suites() - used to register one or more &struct kunit_suite
> > + * with KUnit.
> > + *
> > + * @suites: a statically allocated list of &struct kunit_suite.
> > + *
> > + * Registers @suites with the test framework. See &struct kunit_suite for
> > + * more information.
> > + *
> > + * When builtin, KUnit tests are all run via executor; this is done
> > + * by placing the array of struct kunit_suite * in the .kunit_test_suites
> > + * ELF section.
> > + *
> > + * An alternative is to build the tests as a module. Because modules do not
> > + * support multiple initcall()s, we need to initialize an array of suites for a
> > + * module.
> > + *
> > + */
> > +#define kunit_test_suites(...) \
> > + __kunit_test_suites(__UNIQUE_ID(array), \
> > + __UNIQUE_ID(suites), \
> > + __VA_ARGS__)
> >
> > #define kunit_test_suite(suite) kunit_test_suites(&suite)
> >
> > diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
> > index 724b94311ca36..c49f4ffb6273a 100644
> > --- a/lib/kunit/Makefile
> > +++ b/lib/kunit/Makefile
> > @@ -3,7 +3,8 @@ obj-$(CONFIG_KUNIT) += kunit.o
> > kunit-objs += test.o \
> > string-stream.o \
> > assert.o \
> > - try-catch.o
> > + try-catch.o \
> > + executor.o
> >
> > ifeq ($(CONFIG_KUNIT_DEBUGFS),y)
> > kunit-objs += debugfs.o
> > diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
> > new file mode 100644
> > index 0000000000000..7015e7328dce7
> > --- /dev/null
> > +++ b/lib/kunit/executor.c
> > @@ -0,0 +1,28 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +#include <kunit/test.h>
> > +
> > +/*
> > + * These symbols point to the .kunit_test_suites section and are defined in
> > + * include/asm-generic/vmlinux.lds.h, and consequently must be extern.
> > + */
> > +extern struct kunit_suite * const * const __kunit_suites_start[];
> > +extern struct kunit_suite * const * const __kunit_suites_end[];
>
> I would expect these to be in include/asm-generic/sections.h but I guess
> it's not required.
I don't have strong opinions either way, but I think this is less
clutter since KUnit is the only one that uses it.
> Reviewed-by: Kees Cook <keescook@chromium.org>
Thanks!
^ permalink raw reply
* Re: [PATCH v5 01/12] vmlinux.lds.h: add linker section for KUnit test suites
From: Brendan Higgins @ 2020-08-04 20:03 UTC (permalink / raw)
To: Luis Chamberlain
Cc: open list:DOCUMENTATION, catalin.marinas, jcmvbkbc, will,
Paul Mackerras, open list:KERNEL SELFTEST FRAMEWORK, Frank Rowand,
Anton Ivanov, linux-arch, Richard Weinberger, rppt, Iurii Zaikin,
linux-xtensa, Kees Cook, Arnd Bergmann, Jeff Dike, linux-um,
linuxppc-dev, David Gow, Shuah Khan, Linux ARM, KUnit Development,
Chris Zankel, Michal Simek, Stephen Boyd, Greg KH,
Linux Kernel Mailing List, Logan Gunthorpe, Andrew Morton,
Alan Maguire
In-Reply-To: <20200708043128.GY4332@42.do-not-panic.com>
On Tue, Jul 7, 2020 at 9:31 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
>
> On Fri, Jun 26, 2020 at 02:22:11PM -0700, Brendan Higgins wrote:
> > On Fri, Jun 26, 2020 at 2:20 PM Kees Cook <keescook@chromium.org> wrote:
> > >
> > > On Fri, Jun 26, 2020 at 02:09:06PM -0700, Brendan Higgins wrote:
> > > > Add a linker section where KUnit can put references to its test suites.
> > > > This patch is the first step in transitioning to dispatching all KUnit
> > > > tests from a centralized executor rather than having each as its own
> > > > separate late_initcall.
> > > >
> > > > Co-developed-by: Iurii Zaikin <yzaikin@google.com>
> > > > Signed-off-by: Iurii Zaikin <yzaikin@google.com>
> > > > Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> > > > Reviewed-by: Stephen Boyd <sboyd@kernel.org>
> > > > ---
> > > > include/asm-generic/vmlinux.lds.h | 8 ++++++++
> > > > 1 file changed, 8 insertions(+)
> > > >
> > > > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> > > > index db600ef218d7d..4f9b036fc9616 100644
> > > > --- a/include/asm-generic/vmlinux.lds.h
> > > > +++ b/include/asm-generic/vmlinux.lds.h
> > > > @@ -881,6 +881,13 @@
> > > > KEEP(*(.con_initcall.init)) \
> > > > __con_initcall_end = .;
> > > >
> > > > +/* Alignment must be consistent with (kunit_suite *) in include/kunit/test.h */
> > >
> > > Nit on naming:
> > >
> > > > +#define KUNIT_TEST_SUITES \
> > >
> > > I would call this KUNIT_TABLE to maintain the same names as other things
> > > of this nature.
> > >
> > > > + . = ALIGN(8); \
> > > > + __kunit_suites_start = .; \
> > > > + KEEP(*(.kunit_test_suites)) \
> > > > + __kunit_suites_end = .;
> > > > +
> > > > #ifdef CONFIG_BLK_DEV_INITRD
> > > > #define INIT_RAM_FS \
> > > > . = ALIGN(4); \
> > > > @@ -1056,6 +1063,7 @@
> > > > INIT_CALLS \
> > > > CON_INITCALL \
> > > > INIT_RAM_FS \
> > > > + KUNIT_TEST_SUITES \
> > > > }
> > >
> > > Nack: this must be in INIT_DATA, not in INIT_DATA_SECTION. Not all
> > > architectures use the INIT_DATA_SECTION macro (e.g. arm64), but everything
> > > uses INIT_DATA.
> >
> > Oh, maybe that would eliminate the need for the other linkerscript
> > patches? That would be nice.
Sorry for the delayed response. I got pulled into some other things.
> Curious, did changing it as Kees suggest fix it for m68k?
It did! There are still some architectures I cannot test due to a lack
of GCC or QEMU support, but it seems to work on everything else now.
^ permalink raw reply
* Re: [PATCH v5 00/12] kunit: create a centralized executor to dispatch all KUnit tests
From: Brendan Higgins @ 2020-08-04 20:01 UTC (permalink / raw)
To: Kees Cook
Cc: open list:DOCUMENTATION, catalin.marinas, jcmvbkbc, will,
Paul Mackerras, open list:KERNEL SELFTEST FRAMEWORK, Frank Rowand,
Anton Ivanov, linux-arch, Richard Weinberger, rppt, Iurii Zaikin,
linux-xtensa, Arnd Bergmann, Jeff Dike, linux-um, linuxppc-dev,
David Gow, Shuah Khan, Linux ARM, KUnit Development, Chris Zankel,
Michal Simek, Stephen Boyd, Greg KH, Linux Kernel Mailing List,
Luis Chamberlain, Alan Maguire, Andrew Morton, Logan Gunthorpe
In-Reply-To: <202006261442.5C245709@keescook>
On Fri, Jun 26, 2020 at 2:52 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Fri, Jun 26, 2020 at 02:09:05PM -0700, Brendan Higgins wrote:
> > This patchset adds a centralized executor to dispatch tests rather than
> > relying on late_initcall to schedule each test suite separately along
> > with a couple of new features that depend on it.
Sorry it took so long to reply. I got sucked into some other stuff again.
> So, the new section looks fine to me (modulo the INIT_DATA change). The
> plumbing to start the tests, though, I think is redundant. Why not just
> add a sysctl that starts all known tests?
We already have that; however, we use debugfs to start the tests -
same difference. I just find it convenient to not have to build and
then maintain a userland for each architecture. It's also really nice
that KUnit "just works out of the box" - you don't have to download
anything other than the kernel source, and you don't need to do any
steps outside of just run "kuit.py run". That seems like a big
advantage to me.
> That way you don't need the plumbing into init/main.c, and you can have
> a mode where builtin tests can be started on a fully booted system too.
>
> i.e. boot with "sysctl.kernel.kunit=start" or when fully booted with
> "echo start > /proc/sys/kernel/kunit"
>
> And instead of the kunit-specific halt/reboot stuff, how about moving
> /proc/sysrq-trigger into /proc/sys instead? Then you (or anything) could
> do:
>
> sysctl.kernel.kunit=start sysctl.kernel.sysrq-trigger=b
I think it might be harder to make a case for the reboot stuff without
the stuff I am working on outside of this patchset. I think I will
probably drop that patch from this patchset and reintroduce it later.
^ permalink raw reply
* [PATCH] powerpc/oprofile: fix spelling mistake "contex" -> "context"
From: Colin King @ 2020-08-04 17:43 UTC (permalink / raw)
To: Robert Richter, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, oprofile-list, linuxppc-dev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
There is a spelling mistake in a pr_debug message. Fix it.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
arch/powerpc/oprofile/cell/spu_task_sync.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/oprofile/cell/spu_task_sync.c b/arch/powerpc/oprofile/cell/spu_task_sync.c
index df59d0bb121f..489f993100d5 100644
--- a/arch/powerpc/oprofile/cell/spu_task_sync.c
+++ b/arch/powerpc/oprofile/cell/spu_task_sync.c
@@ -572,7 +572,7 @@ void spu_sync_buffer(int spu_num, unsigned int *samples,
* samples are recorded.
* No big deal -- so we just drop a few samples.
*/
- pr_debug("SPU_PROF: No cached SPU contex "
+ pr_debug("SPU_PROF: No cached SPU context "
"for SPU #%d. Dropping samples.\n", spu_num);
goto out;
}
--
2.27.0
^ permalink raw reply related
* [PATCH v4] selftests/powerpc: Fix pkey syscall redefinitions
From: Sandipan Das @ 2020-08-04 17:31 UTC (permalink / raw)
To: mpe; +Cc: sachinp, aneesh.kumar, David Laight, linuxppc-dev
On distros using older glibc versions, the pkey tests
encounter build failures due to redefinition of the
pkey syscall numbers.
For compatibility, commit 743f3544fffb added a wrapper
for the gettid() syscall and included syscall.h if the
version of glibc used is older than 2.30. This leads
to different definitions of SYS_pkey_* as the ones in
the pkey test header set numeric constants where as the
ones from syscall.h reuse __NR_pkey_*. The compiler
complains about redefinitions since they are different.
This replaces SYS_pkey_* definitions with __NR_pkey_*
such that the definitions in both syscall.h and pkeys.h
are alike. This way, if syscall.h has to be included
for compatibility reasons, builds will still succeed.
Fixes: 743f3544fffb ("selftests/powerpc: Add wrapper for gettid")
Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Suggested-by: David Laight <david.laight@aculab.com>
Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
Previous versions can be found at:
v3: https://lore.kernel.org/linuxppc-dev/1bb744b0c7ed3985a5b73289f4de629ac0aeaf7c.1596453627.git.sandipan@linux.ibm.com/
v2: https://lore.kernel.org/linuxppc-dev/566dde119ce71f00f9642807ba30ceb7f54c9bfa.1596441105.git.sandipan@linux.ibm.com/
v1: https://lore.kernel.org/linuxppc-dev/20200803074043.466809-1-sandipan@linux.ibm.com/
Changes in v4:
- Replace SYS_pkey_* with __NR_pkey_* based on suggestions
from David and Michael.
- Update commit message and add fixes tag.
Changes in v3:
- Use ifndef...endif instead of undef as suggested by
Michael.
Changes in v2:
- Fix incorrect commit message.
---
tools/testing/selftests/powerpc/include/pkeys.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/powerpc/include/pkeys.h b/tools/testing/selftests/powerpc/include/pkeys.h
index 6ba95039a034..3312cb1b058d 100644
--- a/tools/testing/selftests/powerpc/include/pkeys.h
+++ b/tools/testing/selftests/powerpc/include/pkeys.h
@@ -31,9 +31,9 @@
#define SI_PKEY_OFFSET 0x20
-#define SYS_pkey_mprotect 386
-#define SYS_pkey_alloc 384
-#define SYS_pkey_free 385
+#define __NR_pkey_mprotect 386
+#define __NR_pkey_alloc 384
+#define __NR_pkey_free 385
#define PKEY_BITS_PER_PKEY 2
#define NR_PKEYS 32
@@ -62,17 +62,17 @@ void pkey_set_rights(int pkey, unsigned long rights)
int sys_pkey_mprotect(void *addr, size_t len, int prot, int pkey)
{
- return syscall(SYS_pkey_mprotect, addr, len, prot, pkey);
+ return syscall(__NR_pkey_mprotect, addr, len, prot, pkey);
}
int sys_pkey_alloc(unsigned long flags, unsigned long rights)
{
- return syscall(SYS_pkey_alloc, flags, rights);
+ return syscall(__NR_pkey_alloc, flags, rights);
}
int sys_pkey_free(int pkey)
{
- return syscall(SYS_pkey_free, pkey);
+ return syscall(__NR_pkey_free, pkey);
}
int pkeys_unsupported(void)
--
2.25.1
^ permalink raw reply related
* Re: [merge] Build failure selftest/powerpc/mm/pkey_exec_prot
From: Sandipan Das @ 2020-08-04 16:45 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Sachin Sant, linuxppc-dev
In-Reply-To: <877duezjk3.fsf@mpe.ellerman.id.au>
On 04/08/20 5:53 pm, Michael Ellerman wrote:
> Sandipan Das <sandipan@linux.ibm.com> writes:
>> On 04/08/20 6:38 am, Michael Ellerman wrote:
>>> Sandipan Das <sandipan@linux.ibm.com> writes:
>>>> On 03/08/20 4:32 pm, Michael Ellerman wrote:
>>>>> Sachin Sant <sachinp@linux.vnet.ibm.com> writes:
>>>>>>> On 02-Aug-2020, at 10:58 PM, Sandipan Das <sandipan@linux.ibm.com> wrote:
>>>>>>> On 02/08/20 4:45 pm, Sachin Sant wrote:
>>>>>>>> pkey_exec_prot test from linuxppc merge branch (3f68564f1f5a) fails to
>>>>>>>> build due to following error:
>>>>>>>>
>>>>>>>> gcc -std=gnu99 -O2 -Wall -Werror -DGIT_VERSION='"v5.8-rc7-1276-g3f68564f1f5a"' -I/home/sachin/linux/tools/testing/selftests/powerpc/include -m64 pkey_exec_prot.c /home/sachin/linux/tools/testing/selftests/kselftest_harness.h /home/sachin/linux/tools/testing/selftests/kselftest.h ../harness.c ../utils.c -o /home/sachin/linux/tools/testing/selftests/powerpc/mm/pkey_exec_prot
>>>>>>>> In file included from pkey_exec_prot.c:18:
>>>>>>>> /home/sachin/linux/tools/testing/selftests/powerpc/include/pkeys.h:34: error: "SYS_pkey_mprotect" redefined [-Werror]
>>>>>>>> #define SYS_pkey_mprotect 386
>>>>>>>>
>>>>>>>> In file included from /usr/include/sys/syscall.h:31,
>>>>>>>> from /home/sachin/linux/tools/testing/selftests/powerpc/include/utils.h:47,
>>>>>>>> from /home/sachin/linux/tools/testing/selftests/powerpc/include/pkeys.h:12,
>>>>>>>> from pkey_exec_prot.c:18:
>>>>>>>> /usr/include/bits/syscall.h:1583: note: this is the location of the previous definition
>>>>>>>> # define SYS_pkey_mprotect __NR_pkey_mprotect
>>>>>>>>
>>>>>>>> commit 128d3d021007 introduced this error.
>>>>>>>> selftests/powerpc: Move pkey helpers to headers
>>>>>>>>
>>>>>>>> Possibly the # defines for sys calls can be retained in pkey_exec_prot.c or
>>>>>>>>
>>>>>>>
>>>>>>> I am unable to reproduce this on the latest merge branch (HEAD at f59195f7faa4).
>>>>>>> I don't see any redefinitions in pkey_exec_prot.c either.
>>>>>>>
>>>>>>
>>>>>> I can still see this problem on latest merge branch.
>>>>>> I have following gcc version
>>>>>>
>>>>>> gcc version 8.3.1 20191121
>>>>>
>>>>> What libc version? Or just the distro & version?
>>>>
>>>> Sachin observed this on RHEL 8.2 with glibc-2.28.
>>>> I couldn't reproduce it on Ubuntu 20.04 and Fedora 32 and both these distros
>>>> are using glibc-2.31.
>>>
>>> OK odd. Usually it's newer glibc that hits this problem.
>>>
>>> I guess on RHEL 8.2 we're getting the asm-generic version? But that
>>> would be quite wrong if that's what's happening.
>>
>> If I let GCC dump all the headers that are being used for the source file, I always
>> see syscall.h being included on the RHEL 8.2 system. That is the header with the
>> conflicting definition.
>>
>> $ cd tools/testing/selftests/powerpc/mm
>> $ gcc -H -std=gnu99 -O2 -Wall -Werror -DGIT_VERSION='"v5.8-rc7-1456-gf59195f7faa4-dirty"' \
>> -I../include -m64 pkey_exec_prot.c ../../kselftest_harness.h ../../kselftest.h ../harness.c ../utils.c \
>> -o pkey_exec_prot 2>&1 | grep syscall
>>
>> On Ubuntu 20.04 and Fedora 32, grep doesn't find any matching text.
>> On RHEL 8.2, it shows the following.
>> ... /usr/include/sys/syscall.h
>> .... /usr/include/bits/syscall.h
>> In file included from /usr/include/sys/syscall.h:31,
>> /usr/include/bits/syscall.h:1583: note: this is the location of the previous definition
>> In file included from /usr/include/sys/syscall.h:31,
>> /usr/include/bits/syscall.h:1575: note: this is the location of the previous definition
>> In file included from /usr/include/sys/syscall.h:31,
>> /usr/include/bits/syscall.h:1579: note: this is the location of the previous definition
>> /usr/include/bits/syscall.h
>> .. /usr/include/sys/syscall.h
>> ... /usr/include/bits/syscall.h
>> /usr/include/bits/syscall.h
>> .. /usr/include/sys/syscall.h
>> ... /usr/include/bits/syscall.h
>> /usr/include/bits/syscall.h
>>
>> So utils.h is also including /usr/include/sys/syscall.h for glibc versions older than 2.30
>> because of commit 743f3544fffb ("selftests/powerpc: Add wrapper for gettid") :)
>
> Haha, of course. :facepalm_emoji:
>
>> [...]
>> . ../include/pkeys.h
>> [...]
>> .. ../include/utils.h
>> [...]
>> ... /usr/include/sys/syscall.h
>> .... /usr/include/asm/unistd.h
>> .... /usr/include/bits/syscall.h
>> In file included from pkey_exec_prot.c:18:
>> ../include/pkeys.h:34: error: "SYS_pkey_mprotect" redefined [-Werror]
>> #define SYS_pkey_mprotect 386
>>
>> In file included from /usr/include/sys/syscall.h:31,
>> from ../include/utils.h:47,
>> from ../include/pkeys.h:12,
>> from pkey_exec_prot.c:18:
>> /usr/include/bits/syscall.h:1583: note: this is the location of the previous definition
>> # define SYS_pkey_mprotect __NR_pkey_mprotect
>
> Aha, that explains why redefining gives us an error, because we're
> defining it to the literal 386 whereas the system header is defining it
> to the __NR value.
>
> Is there a reason to use the SYS_ name?
>
That's just something I borrowed from the pkey tests under selftests/vm
... but without the #ifndefs
> Typically we just use the __NR value directly, and that would avoid any
> problems with redefinition I think, as long as we're using the same
> value as the system header (which we always should be).
>
Agreed. David Laight suggested this too. Will send v4 with these changes.
- Sandipan
> eg:
>
> diff --git a/tools/testing/selftests/powerpc/include/pkeys.h b/tools/testing/selftests/powerpc/include/pkeys.h
> index 6ba95039a034..3312cb1b058d 100644
> --- a/tools/testing/selftests/powerpc/include/pkeys.h
> +++ b/tools/testing/selftests/powerpc/include/pkeys.h
> @@ -31,9 +31,9 @@
>
> #define SI_PKEY_OFFSET 0x20
>
> -#define SYS_pkey_mprotect 386
> -#define SYS_pkey_alloc 384
> -#define SYS_pkey_free 385
> +#define __NR_pkey_mprotect 386
> +#define __NR_pkey_alloc 384
> +#define __NR_pkey_free 385
>
> #define PKEY_BITS_PER_PKEY 2
> #define NR_PKEYS 32
> @@ -62,17 +62,17 @@ void pkey_set_rights(int pkey, unsigned long rights)
>
> int sys_pkey_mprotect(void *addr, size_t len, int prot, int pkey)
> {
> - return syscall(SYS_pkey_mprotect, addr, len, prot, pkey);
> + return syscall(__NR_pkey_mprotect, addr, len, prot, pkey);
> }
>
> int sys_pkey_alloc(unsigned long flags, unsigned long rights)
> {
> - return syscall(SYS_pkey_alloc, flags, rights);
> + return syscall(__NR_pkey_alloc, flags, rights);
> }
>
> int sys_pkey_free(int pkey)
> {
> - return syscall(SYS_pkey_free, pkey);
> + return syscall(__NR_pkey_free, pkey);
> }
>
> int pkeys_unsupported(void)
>
^ permalink raw reply
* Re: [PATCH] powerpc/pseries/hotplug-cpu: increase wait time for vCPU death
From: Greg Kurz @ 2020-08-04 14:16 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, linuxppc-dev, Michael Roth, Thiago Jung Bauermann,
Cedric Le Goater
In-Reply-To: <873652zg8h.fsf@mpe.ellerman.id.au>
On Tue, 04 Aug 2020 23:35:10 +1000
Michael Ellerman <mpe@ellerman.id.au> wrote:
> Hi Mike,
>
> There is a bit of history to this code, but not in a good way :)
>
> Michael Roth <mdroth@linux.vnet.ibm.com> writes:
> > For a power9 KVM guest with XIVE enabled, running a test loop
> > where we hotplug 384 vcpus and then unplug them, the following traces
> > can be seen (generally within a few loops) either from the unplugged
> > vcpu:
> >
> > [ 1767.353447] cpu 65 (hwid 65) Ready to die...
> > [ 1767.952096] Querying DEAD? cpu 66 (66) shows 2
> > [ 1767.952311] list_del corruption. next->prev should be c00a000002470208, but was c00a000002470048
> ...
> >
> > At that point the worker thread assumes the unplugged CPU is in some
> > unknown/dead state and procedes with the cleanup, causing the race with
> > the XIVE cleanup code executed by the unplugged CPU.
> >
> > Fix this by inserting an msleep() after each RTAS call to avoid
>
> We previously had an msleep(), but it was removed:
>
> b906cfa397fd ("powerpc/pseries: Fix cpu hotplug")
>
Ah, I hadn't seen that one...
> > pseries_cpu_die() returning prematurely, and double the number of
> > attempts so we wait at least a total of 5 seconds. While this isn't an
> > ideal solution, it is similar to how we dealt with a similar issue for
> > cede_offline mode in the past (940ce422a3).
>
> Thiago tried to fix this previously but there was a bit of discussion
> that didn't quite resolve:
>
> https://lore.kernel.org/linuxppc-dev/20190423223914.3882-1-bauerman@linux.ibm.com/
>
Yeah it appears that the motivation at the time was to make the "Querying DEAD?"
messages to disappear and to avoid potentially concurrent calls to rtas-stop-self
which is prohibited by PAPR... not fixing actual crashes.
>
> Spinning forever seems like a bad idea, but as has been demonstrated at
> least twice now, continuing when we don't know the state of the other
> CPU can lead to straight up crashes.
>
> So I think I'm persuaded that it's preferable to have the kernel stuck
> spinning rather than oopsing.
>
+1
> I'm 50/50 on whether we should have a cond_resched() in the loop. My
> first instinct is no, if we're stuck here for 20s a stack trace would be
> good. But then we will probably hit that on some big and/or heavily
> loaded machine.
>
> So possibly we should call cond_resched() but have some custom logic in
> the loop to print a warning if we are stuck for more than some
> sufficiently long amount of time.
>
How long should that be ?
>
> > Fixes: eac1e731b59ee ("powerpc/xive: guest exploitation of the XIVE interrupt controller")
> > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1856588
>
> This is not public.
>
I'll have a look at changing that.
> I tend to trim Bugzilla links from the change log, because I'm not
> convinced they will last forever, but it is good to have them in the
> mail archive.
>
> cheers
>
Cheers,
--
Greg
> > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > Cc: Cedric Le Goater <clg@kaod.org>
> > Cc: Greg Kurz <groug@kaod.org>
> > Cc: Nathan Lynch <nathanl@linux.ibm.com>
> > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> > ---
> > arch/powerpc/platforms/pseries/hotplug-cpu.c | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> > index c6e0d8abf75e..3cb172758052 100644
> > --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
> > +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> > @@ -111,13 +111,12 @@ static void pseries_cpu_die(unsigned int cpu)
> > int cpu_status = 1;
> > unsigned int pcpu = get_hard_smp_processor_id(cpu);
> >
> > - for (tries = 0; tries < 25; tries++) {
> > + for (tries = 0; tries < 50; tries++) {
> > cpu_status = smp_query_cpu_stopped(pcpu);
> > if (cpu_status == QCSS_STOPPED ||
> > cpu_status == QCSS_HARDWARE_ERROR)
> > break;
> > - cpu_relax();
> > -
> > + msleep(100);
> > }
> >
> > if (cpu_status != 0) {
> > --
> > 2.17.1
^ permalink raw reply
* [PATCH -next] soc: fsl: qe: Remove unnessesary check in ucc_set_tdm_rxtx_clk
From: Wang Hai @ 2020-08-04 13:56 UTC (permalink / raw)
To: qiang.zhao, leoyang.li; +Cc: linuxppc-dev, linux-kernel, linux-arm-kernel
Fix smatch warning:
drivers/soc/fsl/qe/ucc.c:526
ucc_set_tdm_rxtx_clk() warn: unsigned 'tdm_num' is never less than zero.
'tdm_num' is u32 type, never less than zero.
Signed-off-by: Wang Hai <wanghai38@huawei.com>
---
drivers/soc/fsl/qe/ucc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soc/fsl/qe/ucc.c b/drivers/soc/fsl/qe/ucc.c
index cac0fb7693a0..21dbcd787cd5 100644
--- a/drivers/soc/fsl/qe/ucc.c
+++ b/drivers/soc/fsl/qe/ucc.c
@@ -523,7 +523,7 @@ int ucc_set_tdm_rxtx_clk(u32 tdm_num, enum qe_clock clock,
qe_mux_reg = &qe_immr->qmx;
- if (tdm_num > 7 || tdm_num < 0)
+ if (tdm_num > 7)
return -EINVAL;
/* The communications direction must be RX or TX */
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v2 01/17] KVM: PPC: Book3S HV: simplify kvm_cma_reserve()
From: Daniel Axtens @ 2020-08-04 13:53 UTC (permalink / raw)
To: Mike Rapoport, Andrew Morton
Cc: Thomas Gleixner, Emil Renner Berthing, linux-sh, Peter Zijlstra,
Dave Hansen, linux-mips, Max Filippov, Paul Mackerras, sparclinux,
linux-riscv, Will Deacon, Christoph Hellwig, Marek Szyprowski,
linux-arch, linux-s390, linux-c6x-dev, Baoquan He, x86,
Russell King, Mike Rapoport, clang-built-linux, Ingo Molnar,
linux-arm-kernel, Catalin Marinas, uclinux-h8-devel, linux-xtensa,
openrisc, Borislav Petkov, Andy Lutomirski, Paul Walmsley,
Stafford Horne, Hari Bathini, Michal Simek, Yoshinori Sato,
linux-mm, linux-kernel, iommu, Palmer Dabbelt, linuxppc-dev,
Mike Rapoport
In-Reply-To: <20200802163601.8189-2-rppt@kernel.org>
Hi Mike,
>
> The memory size calculation in kvm_cma_reserve() traverses memblock.memory
> rather than simply call memblock_phys_mem_size(). The comment in that
> function suggests that at some point there should have been call to
> memblock_analyze() before memblock_phys_mem_size() could be used.
> As of now, there is no memblock_analyze() at all and
> memblock_phys_mem_size() can be used as soon as cold-plug memory is
> registerd with memblock.
>
> Replace loop over memblock.memory with a call to memblock_phys_mem_size().
>
> Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
> ---
> arch/powerpc/kvm/book3s_hv_builtin.c | 11 ++---------
> 1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv_builtin.c b/arch/powerpc/kvm/book3s_hv_builtin.c
> index 7cd3cf3d366b..56ab0d28de2a 100644
> --- a/arch/powerpc/kvm/book3s_hv_builtin.c
> +++ b/arch/powerpc/kvm/book3s_hv_builtin.c
> @@ -95,22 +95,15 @@ EXPORT_SYMBOL_GPL(kvm_free_hpt_cma);
> void __init kvm_cma_reserve(void)
> {
> unsigned long align_size;
> - struct memblock_region *reg;
> - phys_addr_t selected_size = 0;
> + phys_addr_t selected_size;
>
> /*
> * We need CMA reservation only when we are in HV mode
> */
> if (!cpu_has_feature(CPU_FTR_HVMODE))
> return;
> - /*
> - * We cannot use memblock_phys_mem_size() here, because
> - * memblock_analyze() has not been called yet.
> - */
> - for_each_memblock(memory, reg)
> - selected_size += memblock_region_memory_end_pfn(reg) -
> - memblock_region_memory_base_pfn(reg);
>
> + selected_size = PHYS_PFN(memblock_phys_mem_size());
> selected_size = (selected_size * kvm_cma_resv_ratio / 100) << PAGE_SHIFT;
I think this is correct, but PHYS_PFN does x >> PAGE_SHIFT and then the
next line does x << PAGE_SHIFT, so I think we could combine those two
lines as:
selected_size = PAGE_ALIGN(memblock_phys_mem_size() * kvm_cma_resv_ratio / 100);
(I think that might technically change it from aligning down to aligning
up but I don't think 1 page matters here.)
Kind regards,
Daniel
> if (selected_size) {
> pr_debug("%s: reserving %ld MiB for global area\n", __func__,
> --
> 2.26.2
^ 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