* Re: [RFC PATCH v0 4/5] powerpc/mm/radix: Free PUD table when freeing pagetable
From: Aneesh Kumar K.V @ 2020-06-22 13:07 UTC (permalink / raw)
To: Bharata B Rao, linuxppc-dev
Cc: leonardo, aneesh.kumar, npiggin, Bharata B Rao
In-Reply-To: <20200406034925.22586-5-bharata@linux.ibm.com>
Bharata B Rao <bharata@linux.ibm.com> writes:
> remove_pagetable() isn't freeing PUD table. This causes memory
> leak during memory unplug. Fix this.
>
We had changes w.r.t p4d (folded 5 level table). You may want to get
this updated to recent kernel.
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
> ---
> arch/powerpc/mm/book3s64/radix_pgtable.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c
> index e675c0bbf9a4..0d9ef3277579 100644
> --- a/arch/powerpc/mm/book3s64/radix_pgtable.c
> +++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
> @@ -767,6 +767,21 @@ static void free_pmd_table(pmd_t *pmd_start, pud_t *pud)
> pud_clear(pud);
> }
>
> +static void free_pud_table(pud_t *pud_start, pgd_t *pgd)
> +{
> + pud_t *pud;
> + int i;
> +
> + for (i = 0; i < PTRS_PER_PUD; i++) {
> + pud = pud_start + i;
> + if (!pud_none(*pud))
> + return;
> + }
> +
> + pud_free(&init_mm, pud_start);
> + pgd_clear(pgd);
> +}
> +
> struct change_mapping_params {
> pte_t *pte;
> unsigned long start;
> @@ -937,6 +952,7 @@ static void __meminit remove_pagetable(unsigned long start, unsigned long end)
>
> pud_base = (pud_t *)pgd_page_vaddr(*pgd);
> remove_pud_table(pud_base, addr, next);
> + free_pud_table(pud_base, pgd);
> }
>
> spin_unlock(&init_mm.page_table_lock);
> --
> 2.21.0
^ permalink raw reply
* Re: [RFC PATCH v0 5/5] powerpc/mm/radix: Remove split_kernel_mapping()
From: Aneesh Kumar K.V @ 2020-06-22 13:07 UTC (permalink / raw)
To: Bharata B Rao, linuxppc-dev
Cc: leonardo, aneesh.kumar, npiggin, Bharata B Rao
In-Reply-To: <20200406034925.22586-6-bharata@linux.ibm.com>
Bharata B Rao <bharata@linux.ibm.com> writes:
> With hot-plugged memory getting mapped with 2M mappings always,
> there will be no need to split any mappings during unplug.
>
> Hence remove split_kernel_mapping() and associated code. This
> essentially is a revert of
> commit 4dd5f8a99e791 ("powerpc/mm/radix: Split linear mapping on hot-unplug")
>
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
> ---
> arch/powerpc/mm/book3s64/radix_pgtable.c | 93 +++++-------------------
> 1 file changed, 19 insertions(+), 74 deletions(-)
>
> diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c
> index 0d9ef3277579..56f2c698deac 100644
> --- a/arch/powerpc/mm/book3s64/radix_pgtable.c
> +++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
> @@ -15,7 +15,6 @@
> #include <linux/mm.h>
> #include <linux/hugetlb.h>
> #include <linux/string_helpers.h>
> -#include <linux/stop_machine.h>
> #include <linux/memory.h>
>
> #include <asm/pgtable.h>
> @@ -782,30 +781,6 @@ static void free_pud_table(pud_t *pud_start, pgd_t *pgd)
> pgd_clear(pgd);
> }
>
> -struct change_mapping_params {
> - pte_t *pte;
> - unsigned long start;
> - unsigned long end;
> - unsigned long aligned_start;
> - unsigned long aligned_end;
> -};
> -
> -static int __meminit stop_machine_change_mapping(void *data)
> -{
> - struct change_mapping_params *params =
> - (struct change_mapping_params *)data;
> -
> - if (!data)
> - return -1;
> -
> - spin_unlock(&init_mm.page_table_lock);
> - pte_clear(&init_mm, params->aligned_start, params->pte);
> - create_physical_mapping(__pa(params->aligned_start), __pa(params->start), -1);
> - create_physical_mapping(__pa(params->end), __pa(params->aligned_end), -1);
> - spin_lock(&init_mm.page_table_lock);
> - return 0;
> -}
> -
> static void remove_pte_table(pte_t *pte_start, unsigned long addr,
> unsigned long end)
> {
> @@ -834,52 +809,6 @@ static void remove_pte_table(pte_t *pte_start, unsigned long addr,
> }
> }
>
> -/*
> - * clear the pte and potentially split the mapping helper
> - */
> -static void __meminit split_kernel_mapping(unsigned long addr, unsigned long end,
> - unsigned long size, pte_t *pte)
> -{
> - unsigned long mask = ~(size - 1);
> - unsigned long aligned_start = addr & mask;
> - unsigned long aligned_end = addr + size;
> - struct change_mapping_params params;
> - bool split_region = false;
> -
> - if ((end - addr) < size) {
> - /*
> - * We're going to clear the PTE, but not flushed
> - * the mapping, time to remap and flush. The
> - * effects if visible outside the processor or
> - * if we are running in code close to the
> - * mapping we cleared, we are in trouble.
> - */
> - if (overlaps_kernel_text(aligned_start, addr) ||
> - overlaps_kernel_text(end, aligned_end)) {
> - /*
> - * Hack, just return, don't pte_clear
> - */
> - WARN_ONCE(1, "Linear mapping %lx->%lx overlaps kernel "
> - "text, not splitting\n", addr, end);
> - return;
> - }
> - split_region = true;
> - }
> -
> - if (split_region) {
> - params.pte = pte;
> - params.start = addr;
> - params.end = end;
> - params.aligned_start = addr & ~(size - 1);
> - params.aligned_end = min_t(unsigned long, aligned_end,
> - (unsigned long)__va(memblock_end_of_DRAM()));
> - stop_machine(stop_machine_change_mapping, ¶ms, NULL);
> - return;
> - }
> -
> - pte_clear(&init_mm, addr, pte);
> -}
> -
> static void remove_pmd_table(pmd_t *pmd_start, unsigned long addr,
> unsigned long end)
> {
> @@ -895,7 +824,12 @@ static void remove_pmd_table(pmd_t *pmd_start, unsigned long addr,
> continue;
>
> if (pmd_is_leaf(*pmd)) {
> - split_kernel_mapping(addr, end, PMD_SIZE, (pte_t *)pmd);
> + if (!IS_ALIGNED(addr, PMD_SIZE) ||
> + !IS_ALIGNED(next, PMD_SIZE)) {
> + WARN_ONCE(1, "%s: unaligned range\n", __func__);
> + continue;
> + }
> + pte_clear(&init_mm, addr, (pte_t *)pmd);
> continue;
> }
>
> @@ -920,7 +854,12 @@ static void remove_pud_table(pud_t *pud_start, unsigned long addr,
> continue;
>
> if (pud_is_leaf(*pud)) {
> - split_kernel_mapping(addr, end, PUD_SIZE, (pte_t *)pud);
> + if (!IS_ALIGNED(addr, PUD_SIZE) ||
> + !IS_ALIGNED(next, PUD_SIZE)) {
> + WARN_ONCE(1, "%s: unaligned range\n", __func__);
> + continue;
> + }
> + pte_clear(&init_mm, addr, (pte_t *)pud);
> continue;
> }
>
> @@ -946,7 +885,13 @@ static void __meminit remove_pagetable(unsigned long start, unsigned long end)
> continue;
>
> if (pgd_is_leaf(*pgd)) {
> - split_kernel_mapping(addr, end, PGDIR_SIZE, (pte_t *)pgd);
> + if (!IS_ALIGNED(addr, PGDIR_SIZE) ||
> + !IS_ALIGNED(next, PGDIR_SIZE)) {
> + WARN_ONCE(1, "%s: unaligned range\n", __func__);
> + continue;
> + }
> +
> + pte_clear(&init_mm, addr, (pte_t *)pgd);
> continue;
> }
>
> --
> 2.21.0
^ permalink raw reply
* Re: [RFC PATCH v0 3/5] powerpc/mm/radix: Fix PTE/PMD fragment count for early page table mappings
From: Aneesh Kumar K.V @ 2020-06-22 13:22 UTC (permalink / raw)
To: Bharata B Rao, linuxppc-dev
Cc: leonardo, aneesh.kumar, npiggin, Bharata B Rao
In-Reply-To: <20200406034925.22586-4-bharata@linux.ibm.com>
....
@@ -71,8 +135,8 @@ static int early_map_kernel_page(unsigned long ea, unsigned long pa,
>
> pgdp = pgd_offset_k(ea);
> if (pgd_none(*pgdp)) {
> - pudp = early_alloc_pgtable(PUD_TABLE_SIZE, nid,
> - region_start, region_end);
> + pudp = early_alloc_pgtable(PAGE_SIZE, nid, region_start,
> + region_end);
> pgd_populate(&init_mm, pgdp, pudp);
Add a comment here explaining why we are using PAGE_SIZE instead of the
required PUD_TABLE_SIZE.
> }
> pudp = pud_offset(pgdp, ea);
> @@ -81,8 +145,8 @@ static int early_map_kernel_page(unsigned long ea, unsigned long pa,
> goto set_the_pte;
> }
> if (pud_none(*pudp)) {
> - pmdp = early_alloc_pgtable(PMD_TABLE_SIZE, nid,
> - region_start, region_end);
> + pmdp = early_alloc_pgtable(PAGE_SIZE, nid, region_start,
> + region_end);
> pud_populate(&init_mm, pudp, pmdp);
> }
> pmdp = pmd_offset(pudp, ea);
-aneesh
^ permalink raw reply
* Re: [next-20200621] LTP tests af_alg02/05 failure on POWER9 PowerVM LPAR
From: Herbert Xu @ 2020-06-22 13:33 UTC (permalink / raw)
To: Sachin Sant; +Cc: Linux Next Mailing List, linuxppc-dev, linux-crypto
In-Reply-To: <DF21C7B5-3824-4A6E-B59C-78B67E247383@linux.vnet.ibm.com>
On Mon, Jun 22, 2020 at 05:55:29PM +0530, Sachin Sant wrote:
> With recent next(next-20200621) af_alg02/05 tests fail while running on POWER9
> PowerVM LPAR.
>
> Results from 5.8.0-rc1-next-20200622
> # ./af_alg02
> tst_test.c:1096: INFO: Timeout per run is 0h 00m 20s
> af_alg02.c:52: BROK: Timed out while reading from request socket.
> #
>
> 5.8.0-rc1-next-20200618 was good. The test case ran fine.
>
> Root cause analysis point to following commit:
>
> commit f3c802a1f30013f8f723b62d7fa49eb9e991da23
> crypto: algif_aead - Only wake up when ctx->more is zero
>
> Reverting this commit allows the test to PASS.
Yes that commit does change the behaviour if you don't terminate
your AEAD request by clearing the MSG_MORE flag. AEAD does not
support chaining so each request must be sent in full before it
can be processed through recvmsg(2). Setting the MSG_MORE flag
indicates that your request has not been sent in full.
Cheers,
--
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
* [PATCH] hmi: Move hmi irq stat from percpu variable to paca.
From: Mahesh Salgaonkar @ 2020-06-22 15:16 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Aneesh Kumar K.V, Nicholas Piggin
With the patch series posted at
https://lore.kernel.org/linuxppc-dev/20200608070904.387440-1-aneesh.kumar@linux.ibm.com/
the percpu first chunk memory area can come from vmalloc ranges. This makes
kernel to crash whenever percpu variable is accessed in real mode. This
patch fixes this issue by moving the hmi irq stat inside paca for safe
access in realmode.
Suggested-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Signed-off-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
---
Machine check handling as well touches percpu variables in realmode. Will
address that in separate patchset.
---
arch/powerpc/include/asm/hardirq.h | 1 -
arch/powerpc/include/asm/paca.h | 1 +
arch/powerpc/kernel/irq.c | 4 ++--
arch/powerpc/kernel/mce.c | 2 +-
arch/powerpc/kvm/book3s_hv_ras.c | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/hardirq.h b/arch/powerpc/include/asm/hardirq.h
index f1e9067bd5ac..f133b5930ae1 100644
--- a/arch/powerpc/include/asm/hardirq.h
+++ b/arch/powerpc/include/asm/hardirq.h
@@ -13,7 +13,6 @@ typedef struct {
unsigned int pmu_irqs;
unsigned int mce_exceptions;
unsigned int spurious_irqs;
- unsigned int hmi_exceptions;
unsigned int sreset_irqs;
#ifdef CONFIG_PPC_WATCHDOG
unsigned int soft_nmi_irqs;
diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
index 45a839a7c6cf..cc07c399306e 100644
--- a/arch/powerpc/include/asm/paca.h
+++ b/arch/powerpc/include/asm/paca.h
@@ -225,6 +225,7 @@ struct paca_struct {
u16 in_mce;
u8 hmi_event_available; /* HMI event is available */
u8 hmi_p9_special_emu; /* HMI P9 special emulation */
+ u32 hmi_irqs; /* HMI irq stat */
#endif
u8 ftrace_enabled; /* Hard disable ftrace */
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index 112d150354b2..d7df03a2958d 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -625,7 +625,7 @@ int arch_show_interrupts(struct seq_file *p, int prec)
seq_printf(p, "%*s: ", prec, "HMI");
for_each_online_cpu(j)
seq_printf(p, "%10u ",
- per_cpu(irq_stat, j).hmi_exceptions);
+ paca_ptrs[j]->hmi_irqs);
seq_printf(p, " Hypervisor Maintenance Interrupts\n");
}
@@ -665,7 +665,7 @@ u64 arch_irq_stat_cpu(unsigned int cpu)
sum += per_cpu(irq_stat, cpu).mce_exceptions;
sum += per_cpu(irq_stat, cpu).spurious_irqs;
sum += per_cpu(irq_stat, cpu).timer_irqs_others;
- sum += per_cpu(irq_stat, cpu).hmi_exceptions;
+ sum += paca_ptrs[cpu]->hmi_irqs;
sum += per_cpu(irq_stat, cpu).sreset_irqs;
#ifdef CONFIG_PPC_WATCHDOG
sum += per_cpu(irq_stat, cpu).soft_nmi_irqs;
diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
index fd90c0eda229..dc11fc16750f 100644
--- a/arch/powerpc/kernel/mce.c
+++ b/arch/powerpc/kernel/mce.c
@@ -711,7 +711,7 @@ long hmi_exception_realmode(struct pt_regs *regs)
{
int ret;
- __this_cpu_inc(irq_stat.hmi_exceptions);
+ local_paca->hmi_irqs++;
ret = hmi_handle_debugtrig(regs);
if (ret >= 0)
diff --git a/arch/powerpc/kvm/book3s_hv_ras.c b/arch/powerpc/kvm/book3s_hv_ras.c
index 79f7d07ef674..6028628ea3ac 100644
--- a/arch/powerpc/kvm/book3s_hv_ras.c
+++ b/arch/powerpc/kvm/book3s_hv_ras.c
@@ -244,7 +244,7 @@ long kvmppc_realmode_hmi_handler(void)
{
bool resync_req;
- __this_cpu_inc(irq_stat.hmi_exceptions);
+ local_paca->hmi_irqs++;
if (hmi_handle_debugtrig(NULL) >= 0)
return 1;
^ permalink raw reply related
* Re: [PATCH] ASoC: fsl_mqs: Fix unchecked return value for clk_prepare_enable
From: Markus Elfring @ 2020-06-22 16:07 UTC (permalink / raw)
To: Shengjiu Wang, alsa-devel, linuxppc-dev
Cc: Timur Tabi, Xiubo Li, Takashi Iwai, kernel-janitors, linux-kernel,
Jaroslav Kysela, Nicolin Chen, Mark Brown, Fabio Estevam
> Fix unchecked return value for clk_prepare_enable.
>
> And because clk_prepare_enable and clk_disable_unprepare should
> check input clock parameter is NULL or not, then we don't need
> to check it before calling the function.
I propose to split the adjustment of two function implementations
into separate update steps for a small patch series.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=625d3449788f85569096780592549d0340e9c0c7#n138
I suggest to improve the change descriptions accordingly.
Regards,
Markus
^ permalink raw reply
* Re: [PATCH 1/4] powerpc/pseries/iommu: Update call to ibm,query-pe-dma-windows
From: Leonardo Bras @ 2020-06-22 18:58 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Ram Pai, linux-kernel, Paul Mackerras, linuxppc-dev,
Thiago Jung Bauermann
In-Reply-To: <cfbcacde-ca7f-5fc7-2fcf-267f698f3d49@ozlabs.ru>
Hello Alexey, thank you for the feedback!
On Mon, 2020-06-22 at 20:02 +1000, Alexey Kardashevskiy wrote:
>
> On 19/06/2020 15:06, 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.
> >
> > Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> > ---
> > arch/powerpc/platforms/pseries/iommu.c | 57 +++++++++++++++++++++-----
> > 1 file changed, 46 insertions(+), 11 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> > index 6d47b4a3ce39..e5a617738c8b 100644
> > --- a/arch/powerpc/platforms/pseries/iommu.c
> > +++ b/arch/powerpc/platforms/pseries/iommu.c
> > @@ -334,7 +334,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;
> > };
> > @@ -869,14 +869,32 @@ static int find_existing_ddw_windows(void)
> > }
> > machine_arch_initcall(pseries, find_existing_ddw_windows);
> >
> > +/*
> > + * 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.
> > + */
> > +
> > +static int query_ddw_out_sz(struct device_node *par_dn)
>
> Can easily be folded into query_ddw().
Sure, but it will get inlined by the compiler, and I think it reads
better this way.
I mean, I understand you have a reason to think it's better to fold it
in query_ddw(), and I would like to better understand that to improve
my code in the future.
> > +{
> > + int ret;
> > + u32 ddw_ext[3];
> > +
> > + ret = of_property_read_u32_array(par_dn, "ibm,ddw-extensions",
> > + &ddw_ext[0], 3);
> > + if (ret || ddw_ext[0] < 2 || ddw_ext[2] != 1)
>
> Oh that PAPR thing again :-/
>
> ===
> The “ibm,ddw-extensions” property value is a list of integers the first
> integer indicates the number of extensions implemented and subsequent
> integers, one per extension, provide a value associated with that
> extension.
> ===
>
> So ddw_ext[0] is length.
> Listindex==2 is for "reset" says PAPR and
> Listindex==3 is for this new 64bit "largest_available_block".
>
> So I'd expect ddw_ext[2] to have the "reset" token and ddw_ext[3] to
> have "1" for this new feature but indexes are smaller. I am confused.
> Either way these "2" and "3" needs to be defined in macros, "0" probably
> too.
Remember these indexes are not C-like 0-starting indexes, where the
size would be Listindex==1.
Basically, in C-like array it's :
a[0] == size,
a[1] == reset_token,
a[2] == new 64bit "largest_available_block"
> Please post 'lsprop "ibm,ddw-extensions"' here. Thanks,
Sure:
[root@host pci@800000029004005]# lsprop "ibm,ddw-extensions"
ibm,dd
w-extensions
00000002 00000056 00000000
>
> > + return 5;
> > + return 6;
> > +}
> > +
> > 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 *par_dn)
> > {
> > struct device_node *dn;
> > struct pci_dn *pdn;
> > - u32 cfg_addr;
> > + u32 cfg_addr, query_out[5];
> > u64 buid;
> > - int ret;
> > + int ret, out_sz;
> >
> > /*
> > * Get the config address and phb buid of the PE window.
> > @@ -888,12 +906,29 @@ static int query_ddw(struct pci_dev *dev, const u32 *ddw_avail,
> > pdn = PCI_DN(dn);
> > buid = pdn->phb->buid;
> > cfg_addr = ((pdn->busno << 16) | (pdn->devfn << 8));
> > + out_sz = query_ddw_out_sz(par_dn);
> > +
> > + ret = rtas_call(ddw_avail[0], 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[0], 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;
> > + }
> >
> > - ret = rtas_call(ddw_avail[0], 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);
> > return ret;
> > }
> >
> > @@ -1040,7 +1075,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;
> >
> > @@ -1068,7 +1103,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;
> >
Best regards,
Leonardo
^ permalink raw reply
* Re: [PATCH] powerpc/pseries: new lparcfg key/value pair: partition_affinity_score
From: Tyrel Datwyler @ 2020-06-22 18:59 UTC (permalink / raw)
To: Nathan Lynch, Scott Cheloha, linuxppc-dev
In-Reply-To: <87d05ufugs.fsf@linux.ibm.com>
On 6/19/20 5:32 PM, Nathan Lynch wrote:
> Hi Tyrel,
>
> Tyrel Datwyler <tyreld@linux.ibm.com> writes:
>> On 6/19/20 8:34 AM, Scott Cheloha wrote:
>>> The H_GetPerformanceCounterInfo PHYP hypercall has a subcall,
>>> Affinity_Domain_Info_By_Partition, which returns, among other things,
>>> a "partition affinity score" for a given LPAR. This score, a value on
>>> [0-100], represents the processor-memory affinity for the LPAR in
>>> question. A score of 0 indicates the worst possible affinity while a
>>> score of 100 indicates perfect affinity. The score can be used to
>>> reason about performance.
>>>
>>> This patch adds the score for the local LPAR to the lparcfg procfile
>>> under a new 'partition_affinity_score' key.
>>
>> I expect that you will probably get a NACK from Michael on this. The overall
>> desire is to move away from these dated /proc interfaces. While its true that I
>> did add a new value recently it was strictly to facilitate and correct the
>> calculation of a derived value that was already dependent on a couple other
>> existing values in lparcfg.
>>
>> With that said I would expect that you would likely be advised to expose this as
>> a sysfs attribute. The question is where? We probably should put some thought in
>> to this as I would like to port each lparcfg value over to sysfs so that we can
>> move to deprecating lparcfg. Putting everything under something like
>> /sys/kernel/lparcfg/* maybe. Michael may have a better suggestion.
>
> I think this score fits pretty naturally in lparcfg: it's a simple
> metric that is specific to the pseries/papr platform, like everything
> else in there.
>
> A few dozen key=value pairs contained in a single file is simple and
> efficient, unlike sysfs with its rather inconsistently applied
> one-value-per-file convention. Surely it's OK if lparcfg gains a line
> every few years?
>
So, two things:
1.) I wanted to give fore warning that Michael is generally reluctant to add new
things to lparcfg and I wanted to prepare you for that possibility.
2.) As a simple metric who is consuming said metric? I've not seen any patches
to powerpc-utils in prep so should I be expecting this to be exposed via
lparstat, or are we expecting new tooling to also start parsing lparcfg? If we
are planning for users to consume it via existing powerpc-utils tooling than its
probably easier to make the argument that it fits into lparcfg. If we are
creating new tooling, or expecting users to extract that value on their own I
think the sysfs route is going to be favored.
-Tyrel
>
>>> The H_GetPerformanceCounterInfo hypercall is already used elsewhere in
>>> the kernel, in powerpc/perf/hv-gpci.c. Refactoring that code and this
>>> code into a more general API might be worthwhile if additional modules
>>> require the hypercall in the future.
>>
>> If you are duplicating code its likely you should already be doing this. See the
>> rest of my comments about below.
>>
>>>
>>> Signed-off-by: Scott Cheloha <cheloha@linux.ibm.com>
>>> ---
>>> arch/powerpc/platforms/pseries/lparcfg.c | 53 ++++++++++++++++++++++++
>>> 1 file changed, 53 insertions(+)
>>>
>>> diff --git a/arch/powerpc/platforms/pseries/lparcfg.c b/arch/powerpc/platforms/pseries/lparcfg.c
>>> index b8d28ab88178..b75151eee0f0 100644
>>> --- a/arch/powerpc/platforms/pseries/lparcfg.c
>>> +++ b/arch/powerpc/platforms/pseries/lparcfg.c
>>> @@ -136,6 +136,57 @@ static unsigned int h_get_ppp(struct hvcall_ppp_data *ppp_data)
>>> return rc;
>>> }
>>>
>>> +/*
>>> + * Based on H_GetPerformanceCounterInfo v1.10.
>>> + */
>>> +static void show_gpci_data(struct seq_file *m)
>>> +{
>>> + struct perf_counter_info_params {
>>> + __be32 counter_request;
>>> + __be32 starting_index;
>>> + __be16 secondary_index;
>>> + __be16 returned_values;
>>> + __be32 detail_rc;
>>> + __be16 counter_value_element_size;
>>> + u8 counter_info_version_in;
>>> + u8 counter_info_version_out;
>>> + u8 reserved[0xC];
>>> + } __packed;
>>
>> This looks to duplicate the hv_get_perf_counter_info_params struct from
>> arch/powerpc/perf/hv-gpci.h. Maybe this include file needs to move to
>> arch/powerpc/asm/inlcude so you don't have to redefine this struct.
>>
>>> + struct hv_gpci_request_buffer {
>>> + struct perf_counter_info_params params;
>>> + u8 output[4096 - sizeof(struct perf_counter_info_params)];
>>> + } __packed;
>>
>> This struct is code duplication of the one defined in
>> arch/powerpc/perf/hv-gpci.c and could be moved into hv-gpci.h along with
>> HGPCI_MAX_DATA_BYTES so that you can use those versions here.
>
> I tend to agree with these comments.
>
>
>>> + struct hv_gpci_request_buffer *buf;
>>> + long ret;
>>> + unsigned int affinity_score;
>>> +
>>> + buf = kmalloc(sizeof(*buf), GFP_KERNEL);
>>> + if (buf == NULL)
>>> + return;
>>> +
>>> + /*
>>> + * Show the local LPAR's affinity score.
>>> + *
>>> + * 0xB1 selects the Affinity_Domain_Info_By_Partition subcall.
>>> + * The score is at byte 0xB in the output buffer.
>>> + */
>>> + memset(&buf->params, 0, sizeof(buf->params));
>>> + buf->params.counter_request = cpu_to_be32(0xB1);
>>> + buf->params.starting_index = cpu_to_be32(-1); /* local LPAR */
>>> + buf->params.counter_info_version_in = 0x5; /* v5+ for score */
>>> + ret = plpar_hcall_norets(H_GET_PERF_COUNTER_INFO, virt_to_phys(buf),
>>> + sizeof(*buf));
>>> + if (ret != H_SUCCESS) {
>>> + pr_debug("hcall failed: H_GET_PERF_COUNTER_INFO: %ld, %x\n",
>>> + ret, be32_to_cpu(buf->params.detail_rc));
>>> + goto out;
>>> + }
>>> + affinity_score = buf->output[0xB];
>>> + seq_printf(m, "partition_affinity_score=%u\n", affinity_score);
>>> +out:
>>> + kfree(buf);
>>> +}
>>> +
>>
>> IIUC we should already be able to get this value from userspace using perf tool,
>> right? If thats the case can't we also programatically retrieve it via the
>> perf_event interface in userspace as well?
>
> No... I had the same thought when I first looked at this, but perf is
> for sampling counters, and the partition affinity score is not a
> counter.
But, its obtained through the same H_GET_PERF_COUNTER_INFO interface as the rest
of the performance counters. Shouldn't it be obtainable via 'perf stat -e
hv-gpci/*' and therefore also via perf_event? I really am not that familiar with
perf so correct me on whatever I'm missing here.
-Tyrel
^ permalink raw reply
* Re: [PATCH 2/4] powerpc/pseries/iommu: Implement ibm,reset-pe-dma-windows rtas call
From: Leonardo Bras @ 2020-06-22 18:58 UTC (permalink / raw)
To: Alexey Kardashevskiy, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Thiago Jung Bauermann, Ram Pai
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <2f004ecc-4788-47b6-e9ae-0c08d4723008@ozlabs.ru>
Hello Alexey, thanks for the feedback!
On Mon, 2020-06-22 at 20:02 +1000, Alexey Kardashevskiy wrote:
>
> On 19/06/2020 15:06, Leonardo Bras wrote:
> > 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 | 33 ++++++++++++++++++++++++++
> > 1 file changed, 33 insertions(+)
> >
> > diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> > index e5a617738c8b..5e1fbc176a37 100644
> > --- a/arch/powerpc/platforms/pseries/iommu.c
> > +++ b/arch/powerpc/platforms/pseries/iommu.c
> > @@ -1012,6 +1012,39 @@ 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, ddw_ext[3];
> > + u64 buid;
> > + struct device_node *dn;
> > + struct pci_dn *pdn;
> > +
> > + ret = of_property_read_u32_array(par_dn, "ibm,ddw-extensions",
> > + &ddw_ext[0], 3);
>
> s/3/2/ as for the reset extension you do not need the "64bit largest
> block" extension.
Sure, I will update this.
>
>
> > + 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(ddw_ext[1], 3, 1, NULL, cfg_addr,
>
> Here the "reset" extention is in ddw_ext[1]. Hm. 1/4 has a bug then.
Humm, in 1/4 I used dd_ext[0] (how many extensions) and ddw_ext[2] (64-
bit largest window count). I fail to see the bug here.
> And I am pretty sure it won't compile as reset_dma_window() is not used
> and it is static so fold it into one the next patches. Thanks,
Sure, I will do that.
I was questioning myself about this and thought it would be better to
split for easier revision.
>
>
> > + BUID_HI(buid), BUID_LO(buid));
> > + if (ret)
> > + dev_info(&dev->dev,
> > + "ibm,reset-pe-dma-windows(%x) %x %x %x returned %d ",
> > + ddw_ext[1], 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,
> >
Best regards,
Leonardo
^ permalink raw reply
* Re: [PATCH 3/4] powerpc/pseries/iommu: Move window-removing part of remove_ddw into remove_dma_window
From: Leonardo Bras @ 2020-06-22 18:59 UTC (permalink / raw)
To: Alexey Kardashevskiy, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Thiago Jung Bauermann, Ram Pai
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <51201582-efe5-85df-7e65-a998e91ab63f@ozlabs.ru>
Hello Alexey, thanks for the feedback!
On Mon, 2020-06-22 at 20:02 +1000, Alexey Kardashevskiy wrote:
>
> On 19/06/2020 15:06, 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 | 53 +++++++++++++++-----------
> > 1 file changed, 31 insertions(+), 22 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> > index 5e1fbc176a37..de633f6ae093 100644
> > --- a/arch/powerpc/platforms/pseries/iommu.c
> > +++ b/arch/powerpc/platforms/pseries/iommu.c
> > @@ -767,25 +767,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 *pdn, u32 *ddw_avail,
>
> You do not need the entire ddw_avail here, pass just the token you need.
Well, I just emulated the behavior of create_ddw() and query_ddw() as
both just pass the array instead of the token, even though they only
use a single token.
I think it's to make the rest of the code independent of the design of
the "ibm,ddw-applicable" array, and if it changes, only local changes
on the functions will be needed.
> Also, despite this particular file, the "pdn" name is usually used for
> struct pci_dn (not device_node), let's keep it that way.
Sure, I got confused for some time about this, as we have:
static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn).
but on *_ddw() we have "struct pci_dn *pdn".
I will also add a patch that renames those 'struct device_node *pdn' to
something like 'struct device_node *parent_dn'.
> > + struct property *win)
> > {
> > struct dynamic_dma_window_prop *dwp;
> > - struct property *win64;
> > - u32 ddw_avail[3];
> > u64 liobn;
> > - int ret = 0;
> > -
> > - ret = of_property_read_u32_array(np, "ibm,ddw-applicable",
> > - &ddw_avail[0], 3);
> > -
> > - 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 */
> > @@ -793,24 +782,44 @@ static void remove_ddw(struct device_node *np, bool remove_prop)
> > 1ULL << (be32_to_cpu(dwp->window_shift) - PAGE_SHIFT), dwp);
> > if (ret)
> > pr_warn("%pOF failed to clear tces in window.\n",
> > - np);
> > + pdn);
> > else
> > pr_debug("%pOF successfully cleared tces in window.\n",
> > - np);
> > + pdn);
> >
> > ret = rtas_call(ddw_avail[2], 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);
> > + pdn, ret, ddw_avail[2], 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);
> > + pdn, ret, ddw_avail[2], liobn);
> > +}
> > +
> > +static void remove_ddw(struct device_node *np, bool remove_prop)
> > +{
> > + struct property *win;
> > + u32 ddw_avail[3];
> > + int ret = 0;
> > +
> > + ret = of_property_read_u32_array(np, "ibm,ddw-applicable",
> > + &ddw_avail[0], 3);
> > + if (ret)
> > + return;
> > +
> > + win = of_find_property(np, DIRECT64_PROPNAME, NULL);
> > + if (!win)
> > + return;
> > +
> > + if (win->length >= sizeof(struct dynamic_dma_window_prop))
>
> Any good reason not to make it "=="? Is there something optional or we
> expect extension (which may not grow from the end but may add cells in
> between). Thanks,
Well, it comes from the old behavior of remove_ddw():
- if (ret || win64->length < sizeof(*dwp))
- goto delprop;
As I reversed the logic from 'if (test) go out' to 'if (!test) do
stuff', I also reversed (a < b) to !(a < b) => (a >= b).
I have no problem changing that to '==', but it will produce a
different behavior than before.
>
>
> > + 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);
> >
Best regards,
Leonardo
^ permalink raw reply
* Re: [PATCH 4/4] powerpc/pseries/iommu: Remove default DMA window before creating DDW
From: Leonardo Bras @ 2020-06-22 18:59 UTC (permalink / raw)
To: Alexey Kardashevskiy, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Thiago Jung Bauermann, Ram Pai
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <ade15776-61d1-b660-db74-7aeba4eddfdf@ozlabs.ru>
Hello Alexey, thanks for the feedback!
On Mon, 2020-06-22 at 20:02 +1000, Alexey Kardashevskiy wrote:
>
> On 19/06/2020 15:06, 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 some devices to use DDW, given they only
> > allow one DMA window.
> >
> > If setting up a new DDW fails anywhere after the removal of this
> > default DMA window, restore it using reset_dma_window.
>
> Nah... If we do it like this, then under pHyp we lose 32bit DMA for good
> as pHyp can only create a single window and it has to map at
> 0x800.0000.0000.0000. They probably do not care though.
>
> Under KVM, this will fail as VFIO allows creating 2 windows and it
> starts from 0 but the existing iommu_bypass_supported_pSeriesLP() treats
> the window address == 0 as a failure. And we want to keep both DMA
> windows for PCI adapters with both 64bit and 32bit PCI functions (I
> heard AMD GPU video + audio are like this) or someone could hotplug
> 32bit DMA device on a vphb with already present 64bit DMA window so we
> do not remove the default window.
Well, then I would suggest doing something like this:
query_ddw(...);
if (query.windows_available == 0){
remove_dma_window(...,default_win);
query_ddw(...);
}
This would make sure to cover cases of windows available == 1
and windows available > 1;
> The last discussed thing I remember was that there was supposed to be a
> new bit in "ibm,architecture-vec-5" (forgot the details), we could use
> that to decide whether to keep the default window or not, like this.
I checked on the latest LoPAR draft (soon to be published), for the
ibm,architecture-vec 'option array 5' and this entry was the only
recently added one that is related to this patchset:
Byte 8 - Bit 0:
SRIOV Virtual Functions Support Dynamic DMA Windows (DDW):
0: SRIOV Virtual Functions do not support DDW
1: SRIOV Virtual Functions do support DDW
Isn't this equivalent to having a "ibm,ddw-applicable" property?
>
> > Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> > ---
> > arch/powerpc/platforms/pseries/iommu.c | 20 +++++++++++++++++---
> > 1 file changed, 17 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> > index de633f6ae093..68d1ea957ac7 100644
> > --- a/arch/powerpc/platforms/pseries/iommu.c
> > +++ b/arch/powerpc/platforms/pseries/iommu.c
> > @@ -1074,8 +1074,9 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> > u64 dma_addr, max_addr;
> > struct device_node *dn;
> > u32 ddw_avail[3];
> > +
> > struct direct_window *window;
> > - struct property *win64;
> > + struct property *win64, *dfl_win;
>
> Make it "default_win" or "def_win", "dfl" hurts to read :)
Sure, no problem :)
>
> > struct dynamic_dma_window_prop *ddwprop;
> > struct failed_ddw_pdn *fpdn;
> >
> > @@ -1110,8 +1111,19 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> > if (ret)
> > goto out_failed;
> >
> > - /*
> > - * Query if there is a second window of size to map the
> > + /*
> > + * First step of setting up DDW is removing the default DMA window,
> > + * if it's present. It will make all the resources available to the
> > + * new DDW window.
> > + * If anything fails after this, we need to restore it.
> > + */
> > +
> > + dfl_win = of_find_property(pdn, "ibm,dma-window", NULL);
> > + if (dfl_win)
> > + remove_dma_window(pdn, ddw_avail, dfl_win);
>
> Before doing so, you want to make sure that the "reset" is actually
> supported. Thanks,
Good catch, I will improve that.
>
>
> > +
> > + /*
> > + * Query if there is a window of size to map the
> > * whole partition. Query returns number of windows, largest
> > * block assigned to PE (partition endpoint), and two bitmasks
> > * of page sizes: supported and supported for migrate-dma.
> > @@ -1219,6 +1231,8 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> > kfree(win64);
> >
> > out_failed:
> > + if (dfl_win)
> > + reset_dma_window(dev, pdn);
> >
> > fpdn = kzalloc(sizeof(*fpdn), GFP_KERNEL);
> > if (!fpdn)
> >
Best regards,
Leonardo
^ permalink raw reply
* Re: [PATCH 1/6] powerpc test_emulate_step: update nip with patched instruction address
From: Jordan Niethe @ 2020-06-22 23:41 UTC (permalink / raw)
To: Balamuruhan S
Cc: ravi.bangoria, Paul Mackerras, sandipan, naveen.n.rao,
linuxppc-dev
In-Reply-To: <20200622070941.759307-2-bala24@linux.ibm.com>
On Mon, Jun 22, 2020 at 5:10 PM Balamuruhan S <bala24@linux.ibm.com> wrote:
>
> pt_regs are initialized to zero in the test infrastructure, R bit
> in prefixed instruction form is used to specify whether the effective
> address of the storage operand is computed relative to the address
> of the instruction.
>
> If R = 1 and RA = R0|0, the sum of the address of the instruction
> and the value SI is placed into register RT. So to assert the emulated
> instruction with executed instruction, update nip of emulated pt_regs.
>
> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> ---
> arch/powerpc/lib/test_emulate_step.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/lib/test_emulate_step.c b/arch/powerpc/lib/test_emulate_step.c
> index 33a72b7d2764..d5902b7b4e5c 100644
> --- a/arch/powerpc/lib/test_emulate_step.c
> +++ b/arch/powerpc/lib/test_emulate_step.c
> @@ -1204,13 +1204,24 @@ static struct compute_test compute_tests[] = {
> static int __init emulate_compute_instr(struct pt_regs *regs,
> struct ppc_inst instr)
> {
> + int prefix_r, ra;
> extern s32 patch__exec_instr;
> struct instruction_op op;
>
> if (!regs || !ppc_inst_val(instr))
> return -EINVAL;
>
> - regs->nip = patch_site_addr(&patch__exec_instr);
Is there any harm in just always setting the NIP like this instead of
only setting it for relative prefixed instructions?
> + /*
> + * If R=1 and RA=0 in Prefixed instruction form, calculate the address
> + * of the instruction and update nip to assert with executed
> + * instruction
> + */
> + if (ppc_inst_prefixed(instr)) {
> + prefix_r = ppc_inst_val(instr) & (1UL << 20);
> + ra = (ppc_inst_suffix(instr) >> 16) & 0x1f;
> + if (prefix_r && !ra)
> + regs->nip = patch_site_addr(&patch__exec_instr);
> + }
>
> if (analyse_instr(&op, regs, instr) != 1 ||
> GETTYPE(op.type) != COMPUTE) {
> --
> 2.24.1
>
^ permalink raw reply
* Re: [PATCH 2/6] powerpc test_emulate_step: fix pr_info() to print 8-byte for prefixed instruction
From: Jordan Niethe @ 2020-06-22 23:49 UTC (permalink / raw)
To: Balamuruhan S
Cc: ravi.bangoria, Paul Mackerras, sandipan, naveen.n.rao,
linuxppc-dev
In-Reply-To: <20200622070941.759307-3-bala24@linux.ibm.com>
On Mon, Jun 22, 2020 at 5:10 PM Balamuruhan S <bala24@linux.ibm.com> wrote:
>
> On test failure, `pr_log()` prints 4 bytes instruction
> irrespective of word/prefix instruction, fix it by printing
> them appropriately.
This patch to add a ppc_inst_as_str() function should help with this,
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20200602052728.18227-1-jniethe5@gmail.com/
>
> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> ---
> arch/powerpc/lib/test_emulate_step.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/lib/test_emulate_step.c b/arch/powerpc/lib/test_emulate_step.c
> index d5902b7b4e5c..e3b1797adfae 100644
> --- a/arch/powerpc/lib/test_emulate_step.c
> +++ b/arch/powerpc/lib/test_emulate_step.c
> @@ -1225,7 +1225,14 @@ static int __init emulate_compute_instr(struct pt_regs *regs,
>
> if (analyse_instr(&op, regs, instr) != 1 ||
> GETTYPE(op.type) != COMPUTE) {
> - pr_info("emulation failed, instruction = 0x%08x\n", ppc_inst_val(instr));
> + if (!ppc_inst_prefixed(instr)) {
> + pr_info("emulation failed, instruction = 0x%08x\n",
> + ppc_inst_val(instr));
> + } else {
> + pr_info("emulation failed, instruction = 0x%08x 0x%08x\n",
> + ppc_inst_val(instr),
> + ppc_inst_suffix(instr));
> + }
> return -EFAULT;
> }
>
> --
> 2.24.1
>
^ permalink raw reply
* [PATCH 16/17] arch: remove HAVE_COPY_THREAD_TLS
From: Christian Brauner @ 2020-06-22 23:43 UTC (permalink / raw)
To: linux-kernel
Cc: linux-riscv, Rich Felker, linux-sh, Peter Zijlstra,
Catalin Marinas, Heiko Carstens, James E.J. Bottomley, Guo Ren,
linux-csky, sparclinux, linux-hexagon, Christian Brauner,
Vincent Chen, Will Deacon, Thomas Gleixner, Anton Ivanov,
Jonas Bonn, linux-s390, linux-ia64, linux-c6x-dev, Brian Cain,
linux-xtensa, Helge Deller, x86, Russell King, Ley Foon Tan,
Mike Rapoport, Christian Borntraeger, Ingo Molnar,
Geert Uytterhoeven, linux-parisc, Mark Salter, Matt Turner,
linux-snps-arc, uclinux-h8-devel, Fenghua Yu, Albert Ou,
Kees Cook, Vasily Gorbik, Jeff Dike, linux-alpha, linux-um,
linuxppc-dev, Aurelien Jacquiot, linux-m68k, Thomas Bogendoerfer,
Ivan Kokshaysky, Greentime Hu, Paul Walmsley, Stafford Horne,
Stefan Kristiansson, Guan Xuetao, linux-arm-kernel,
Richard Henderson, Michal Simek, Tony Luck, Yoshinori Sato,
Nick Hu, Vineet Gupta, linux-mips, openrisc, Palmer Dabbelt,
Richard Weinberger, Paul Mackerras, Linus Torvalds,
David S. Miller, Al Viro
In-Reply-To: <20200622234326.906346-1-christian.brauner@ubuntu.com>
All architectures support copy_thread_tls() now, so remove the legacy
copy_thread() function and the HAVE_COPY_THREAD_TLS config option. Everyone
uses the same process creation calling convention based on
copy_thread_tls() and struct kernel_clone_args. This will make it easier to
maintain the core process creation code under kernel/, simplifies the
callpaths and makes the identical for all architectures.
Cc: Richard Henderson <rth@twiddle.net>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Matt Turner <mattst88@gmail.com>
Cc: Vineet Gupta <vgupta@synopsys.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Mark Salter <msalter@redhat.com>
Cc: Aurelien Jacquiot <jacquiot.aurelien@gmail.com>
Cc: Guo Ren <guoren@kernel.org>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Brian Cain <bcain@codeaurora.org>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Michal Simek <monstr@monstr.eu>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Nick Hu <nickhu@andestech.com>
Cc: Greentime Hu <green.hu@gmail.com>
Cc: Vincent Chen <deanbo422@gmail.com>
Cc: Ley Foon Tan <ley.foon.tan@intel.com>
Cc: Jonas Bonn <jonas@southpole.se>
Cc: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
Cc: Stafford Horne <shorne@gmail.com>
Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: Helge Deller <deller@gmx.de>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Albert Ou <aou@eecs.berkeley.edu>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Rich Felker <dalias@libc.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jeff Dike <jdike@addtoit.com>
Cc: Richard Weinberger <richard@nod.at>
Cc: Anton Ivanov <anton.ivanov@cambridgegreys.com>
Cc: Guan Xuetao <gxt@pku.edu.cn>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: x86@kernel.org
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mike Rapoport <rppt@linux.ibm.com>
Cc: "Matthew Wilcox
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: linux-kernel@vger.kernel.org
Cc: linux-alpha@vger.kernel.org
Cc: linux-snps-arc@lists.infradead.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-c6x-dev@linux-c6x.org
Cc: linux-csky@vger.kernel.org
Cc: uclinux-h8-devel@lists.sourceforge.jp
Cc: linux-hexagon@vger.kernel.org
Cc: linux-ia64@vger.kernel.org
Cc: linux-m68k@lists.linux-m68k.org
Cc: linux-mips@vger.kernel.org
Cc: openrisc@lists.librecores.org
Cc: linux-parisc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-riscv@lists.infradead.org
Cc: linux-s390@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Cc: sparclinux@vger.kernel.org
Cc: linux-um@lists.infradead.org
Cc: linux-xtensa@linux-xtensa.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
arch/Kconfig | 7 -------
arch/alpha/Kconfig | 1 -
arch/arc/Kconfig | 1 -
arch/arm/Kconfig | 1 -
arch/arm64/Kconfig | 1 -
arch/c6x/Kconfig | 1 -
arch/csky/Kconfig | 1 -
arch/h8300/Kconfig | 1 -
arch/hexagon/Kconfig | 1 -
arch/ia64/Kconfig | 1 -
arch/m68k/Kconfig | 1 -
arch/microblaze/Kconfig | 1 -
arch/mips/Kconfig | 1 -
arch/nds32/Kconfig | 1 -
arch/nios2/Kconfig | 1 -
arch/openrisc/Kconfig | 1 -
arch/parisc/Kconfig | 1 -
arch/powerpc/Kconfig | 1 -
arch/riscv/Kconfig | 1 -
arch/s390/Kconfig | 1 -
arch/sh/Kconfig | 1 -
arch/sparc/Kconfig | 1 -
arch/um/Kconfig | 1 -
arch/unicore32/Kconfig | 1 -
arch/x86/Kconfig | 1 -
arch/xtensa/Kconfig | 1 -
include/linux/sched/task.h | 15 +--------------
kernel/fork.c | 9 ---------
28 files changed, 1 insertion(+), 55 deletions(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index 8cc35dc556c7..943aac2f3ebe 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -754,13 +754,6 @@ config ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT
depends on MMU
select ARCH_HAS_ELF_RANDOMIZE
-config HAVE_COPY_THREAD_TLS
- bool
- help
- Architecture provides copy_thread_tls to accept tls argument via
- normal C parameter passing, rather than extracting the syscall
- argument from pt_regs.
-
config HAVE_STACK_VALIDATION
bool
help
diff --git a/arch/alpha/Kconfig b/arch/alpha/Kconfig
index b01515c6b2ed..10862c5a8c76 100644
--- a/arch/alpha/Kconfig
+++ b/arch/alpha/Kconfig
@@ -38,7 +38,6 @@ config ALPHA
select OLD_SIGSUSPEND
select CPU_NO_EFFICIENT_FFS if !ALPHA_EV67
select MMU_GATHER_NO_RANGE
- select HAVE_COPY_THREAD_TLS
help
The Alpha is a 64-bit general-purpose processor designed and
marketed by the Digital Equipment Corporation of blessed memory,
diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
index fddc70029727..1fa0b98ed9ce 100644
--- a/arch/arc/Kconfig
+++ b/arch/arc/Kconfig
@@ -29,7 +29,6 @@ config ARC
select GENERIC_SMP_IDLE_THREAD
select HAVE_ARCH_KGDB
select HAVE_ARCH_TRACEHOOK
- select HAVE_COPY_THREAD_TLS
select HAVE_DEBUG_STACKOVERFLOW
select HAVE_DEBUG_KMEMLEAK
select HAVE_FUTEX_CMPXCHG if FUTEX
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 2ac74904a3ce..445b5ed693f0 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -72,7 +72,6 @@ config ARM
select HAVE_ARM_SMCCC if CPU_V7
select HAVE_EBPF_JIT if !CPU_ENDIAN_BE32
select HAVE_CONTEXT_TRACKING
- select HAVE_COPY_THREAD_TLS
select HAVE_C_RECORDMCOUNT
select HAVE_DEBUG_KMEMLEAK if !XIP_KERNEL
select HAVE_DMA_CONTIGUOUS if MMU
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index a4a094bedcb2..de93e965727d 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -148,7 +148,6 @@ config ARM64
select HAVE_CMPXCHG_DOUBLE
select HAVE_CMPXCHG_LOCAL
select HAVE_CONTEXT_TRACKING
- select HAVE_COPY_THREAD_TLS
select HAVE_DEBUG_BUGVERBOSE
select HAVE_DEBUG_KMEMLEAK
select HAVE_DMA_CONTIGUOUS
diff --git a/arch/c6x/Kconfig b/arch/c6x/Kconfig
index 9cde76a5928e..6444ebfd06a6 100644
--- a/arch/c6x/Kconfig
+++ b/arch/c6x/Kconfig
@@ -22,7 +22,6 @@ config C6X
select GENERIC_CLOCKEVENTS
select MODULES_USE_ELF_RELA
select MMU_GATHER_NO_RANGE if MMU
- select HAVE_COPY_THREAD_TLS
config MMU
def_bool n
diff --git a/arch/csky/Kconfig b/arch/csky/Kconfig
index bd31ab12f77d..902f1142d550 100644
--- a/arch/csky/Kconfig
+++ b/arch/csky/Kconfig
@@ -38,7 +38,6 @@ config CSKY
select GX6605S_TIMER if CPU_CK610
select HAVE_ARCH_TRACEHOOK
select HAVE_ARCH_AUDITSYSCALL
- select HAVE_COPY_THREAD_TLS
select HAVE_DEBUG_BUGVERBOSE
select HAVE_DYNAMIC_FTRACE
select HAVE_DYNAMIC_FTRACE_WITH_REGS
diff --git a/arch/h8300/Kconfig b/arch/h8300/Kconfig
index de0eb417a0b9..d11666d538fe 100644
--- a/arch/h8300/Kconfig
+++ b/arch/h8300/Kconfig
@@ -26,7 +26,6 @@ config H8300
select HAVE_ARCH_HASH
select CPU_NO_EFFICIENT_FFS
select UACCESS_MEMCPY
- select HAVE_COPY_THREAD_TLS
config CPU_BIG_ENDIAN
def_bool y
diff --git a/arch/hexagon/Kconfig b/arch/hexagon/Kconfig
index 19bc2f2ee331..667cfc511cf9 100644
--- a/arch/hexagon/Kconfig
+++ b/arch/hexagon/Kconfig
@@ -31,7 +31,6 @@ config HEXAGON
select GENERIC_CLOCKEVENTS_BROADCAST
select MODULES_USE_ELF_RELA
select GENERIC_CPU_DEVICES
- select HAVE_COPY_THREAD_TLS
help
Qualcomm Hexagon is a processor architecture designed for high
performance and low power across a wide variety of applications.
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index 1b6034b89a04..1fa2fe2ef053 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -55,7 +55,6 @@ config IA64
select HAVE_ARCH_AUDITSYSCALL
select NEED_DMA_MAP_STATE
select NEED_SG_DMA_LENGTH
- select HAVE_COPY_THREAD_TLS
select NUMA if !FLATMEM
default y
help
diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig
index 6ad6cdac74b3..6663f1741798 100644
--- a/arch/m68k/Kconfig
+++ b/arch/m68k/Kconfig
@@ -14,7 +14,6 @@ config M68K
select HAVE_AOUT if MMU
select HAVE_ASM_MODVERSIONS
select HAVE_DEBUG_BUGVERBOSE
- select HAVE_COPY_THREAD_TLS
select GENERIC_IRQ_SHOW
select GENERIC_ATOMIC64
select HAVE_UID16
diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
index e3a211a41880..d262ac0c8714 100644
--- a/arch/microblaze/Kconfig
+++ b/arch/microblaze/Kconfig
@@ -46,7 +46,6 @@ config MICROBLAZE
select CPU_NO_EFFICIENT_FFS
select MMU_GATHER_NO_RANGE if MMU
select SPARSE_IRQ
- select HAVE_COPY_THREAD_TLS
# Endianness selection
choice
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 6fee1a133e9d..ca92c3ed2dc5 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -51,7 +51,6 @@ config MIPS
select HAVE_CBPF_JIT if !64BIT && !CPU_MICROMIPS
select HAVE_CONTEXT_TRACKING
select HAVE_TIF_NOHZ
- select HAVE_COPY_THREAD_TLS
select HAVE_C_RECORDMCOUNT
select HAVE_DEBUG_KMEMLEAK
select HAVE_DEBUG_STACKOVERFLOW
diff --git a/arch/nds32/Kconfig b/arch/nds32/Kconfig
index 7b6eaca81cce..e30298e99e1b 100644
--- a/arch/nds32/Kconfig
+++ b/arch/nds32/Kconfig
@@ -48,7 +48,6 @@ config NDS32
select HAVE_FUNCTION_GRAPH_TRACER
select HAVE_FTRACE_MCOUNT_RECORD
select HAVE_DYNAMIC_FTRACE
- select HAVE_COPY_THREAD_TLS
help
Andes(nds32) Linux support.
diff --git a/arch/nios2/Kconfig b/arch/nios2/Kconfig
index f9a05957a883..c6645141bb2a 100644
--- a/arch/nios2/Kconfig
+++ b/arch/nios2/Kconfig
@@ -27,7 +27,6 @@ config NIOS2
select USB_ARCH_HAS_HCD if USB_SUPPORT
select CPU_NO_EFFICIENT_FFS
select MMU_GATHER_NO_RANGE if MMU
- select HAVE_COPY_THREAD_TLS
config GENERIC_CSUM
def_bool y
diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
index 8588996165ae..7e94fe37cb2f 100644
--- a/arch/openrisc/Kconfig
+++ b/arch/openrisc/Kconfig
@@ -16,7 +16,6 @@ config OPENRISC
select HANDLE_DOMAIN_IRQ
select GPIOLIB
select HAVE_ARCH_TRACEHOOK
- select HAVE_COPY_THREAD_TLS
select SPARSE_IRQ
select GENERIC_IRQ_CHIP
select GENERIC_IRQ_PROBE
diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
index 8e4c3708773d..2667eeb6c6f1 100644
--- a/arch/parisc/Kconfig
+++ b/arch/parisc/Kconfig
@@ -62,7 +62,6 @@ config PARISC
select HAVE_FTRACE_MCOUNT_RECORD if HAVE_DYNAMIC_FTRACE
select HAVE_KPROBES_ON_FTRACE
select HAVE_DYNAMIC_FTRACE_WITH_REGS
- select HAVE_COPY_THREAD_TLS
help
The PA-RISC microprocessor is designed by Hewlett-Packard and used
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 9fa23eb320ff..3b262d87e9c4 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -186,7 +186,6 @@ config PPC
select HAVE_STACKPROTECTOR if PPC32 && $(cc-option,-mstack-protector-guard=tls -mstack-protector-guard-reg=r2)
select HAVE_CONTEXT_TRACKING if PPC64
select HAVE_TIF_NOHZ if PPC64
- select HAVE_COPY_THREAD_TLS
select HAVE_DEBUG_KMEMLEAK
select HAVE_DEBUG_STACKOVERFLOW
select HAVE_DYNAMIC_FTRACE
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 128192e14ff2..f6a3a2bea3d8 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -52,7 +52,6 @@ config RISCV
select HAVE_ARCH_SECCOMP_FILTER
select HAVE_ARCH_TRACEHOOK
select HAVE_ASM_MODVERSIONS
- select HAVE_COPY_THREAD_TLS
select HAVE_DMA_CONTIGUOUS if MMU
select HAVE_EBPF_JIT if MMU
select HAVE_FUTEX_CMPXCHG if FUTEX
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index c7d7ede6300c..959969759453 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -136,7 +136,6 @@ config S390
select HAVE_EBPF_JIT if PACK_STACK && HAVE_MARCH_Z196_FEATURES
select HAVE_CMPXCHG_DOUBLE
select HAVE_CMPXCHG_LOCAL
- select HAVE_COPY_THREAD_TLS
select HAVE_DEBUG_KMEMLEAK
select HAVE_DMA_CONTIGUOUS
select HAVE_DYNAMIC_FTRACE
diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
index e10118d61ce7..9fc2b010e938 100644
--- a/arch/sh/Kconfig
+++ b/arch/sh/Kconfig
@@ -70,7 +70,6 @@ config SUPERH
select ARCH_HIBERNATION_POSSIBLE if MMU
select SPARSE_IRQ
select HAVE_STACKPROTECTOR
- select HAVE_COPY_THREAD_TLS
help
The SuperH is a RISC processor targeted for use in embedded systems
and consumer electronics; it was also used in the Sega Dreamcast
diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 66213c0cb557..5bf2dc163540 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -48,7 +48,6 @@ config SPARC
select LOCKDEP_SMALL if LOCKDEP
select NEED_DMA_MAP_STATE
select NEED_SG_DMA_LENGTH
- select HAVE_COPY_THREAD_TLS
config SPARC32
def_bool !64BIT
diff --git a/arch/um/Kconfig b/arch/um/Kconfig
index 9318dc6d1a0c..ef69be17ff70 100644
--- a/arch/um/Kconfig
+++ b/arch/um/Kconfig
@@ -14,7 +14,6 @@ config UML
select HAVE_FUTEX_CMPXCHG if FUTEX
select HAVE_DEBUG_KMEMLEAK
select HAVE_DEBUG_BUGVERBOSE
- select HAVE_COPY_THREAD_TLS
select GENERIC_IRQ_SHOW
select GENERIC_CPU_DEVICES
select GENERIC_CLOCKEVENTS
diff --git a/arch/unicore32/Kconfig b/arch/unicore32/Kconfig
index 01451cf500d2..11ba1839d198 100644
--- a/arch/unicore32/Kconfig
+++ b/arch/unicore32/Kconfig
@@ -22,7 +22,6 @@ config UNICORE32
select MODULES_USE_ELF_REL
select NEED_DMA_MAP_STATE
select MMU_GATHER_NO_RANGE if MMU
- select HAVE_COPY_THREAD_TLS
help
UniCore-32 is 32-bit Instruction Set Architecture,
including a series of low-power-consumption RISC chip
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 6a0cc524882d..214b8bf39bbe 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -161,7 +161,6 @@ config X86
select HAVE_CMPXCHG_DOUBLE
select HAVE_CMPXCHG_LOCAL
select HAVE_CONTEXT_TRACKING if X86_64
- select HAVE_COPY_THREAD_TLS
select HAVE_C_RECORDMCOUNT
select HAVE_DEBUG_KMEMLEAK
select HAVE_DMA_CONTIGUOUS
diff --git a/arch/xtensa/Kconfig b/arch/xtensa/Kconfig
index 3a9f1e80394a..b71ba910d92f 100644
--- a/arch/xtensa/Kconfig
+++ b/arch/xtensa/Kconfig
@@ -24,7 +24,6 @@ config XTENSA
select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL
select HAVE_ARCH_KASAN if MMU && !XIP_KERNEL
select HAVE_ARCH_TRACEHOOK
- select HAVE_COPY_THREAD_TLS
select HAVE_DEBUG_KMEMLEAK
select HAVE_DMA_CONTIGUOUS
select HAVE_EXIT_THREAD
diff --git a/include/linux/sched/task.h b/include/linux/sched/task.h
index 9f03c44941fb..77cbe14c3034 100644
--- a/include/linux/sched/task.h
+++ b/include/linux/sched/task.h
@@ -65,22 +65,9 @@ extern void fork_init(void);
extern void release_task(struct task_struct * p);
-#ifdef CONFIG_HAVE_COPY_THREAD_TLS
extern int copy_thread_tls(unsigned long, unsigned long, unsigned long,
struct task_struct *, unsigned long);
-#else
-extern int copy_thread(unsigned long, unsigned long, unsigned long,
- struct task_struct *);
-
-/* Architectures that haven't opted into copy_thread_tls get the tls argument
- * via pt_regs, so ignore the tls argument passed via C. */
-static inline int copy_thread_tls(
- unsigned long clone_flags, unsigned long sp, unsigned long arg,
- struct task_struct *p, unsigned long tls)
-{
- return copy_thread(clone_flags, sp, arg, p);
-}
-#endif
+
extern void flush_thread(void);
#ifdef CONFIG_HAVE_EXIT_THREAD
diff --git a/kernel/fork.c b/kernel/fork.c
index 0fd7eb1b38f9..8e52e16a1b5e 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2577,15 +2577,6 @@ SYSCALL_DEFINE5(clone, unsigned long, clone_flags, unsigned long, newsp,
#ifdef __ARCH_WANT_SYS_CLONE3
-/*
- * copy_thread implementations handle CLONE_SETTLS by reading the TLS value from
- * the registers containing the syscall arguments for clone. This doesn't work
- * with clone3 since the TLS value is passed in clone_args instead.
- */
-#ifndef CONFIG_HAVE_COPY_THREAD_TLS
-#error clone3 requires copy_thread_tls support in arch
-#endif
-
noinline static int copy_clone_args_from_user(struct kernel_clone_args *kargs,
struct clone_args __user *uargs,
size_t usize)
--
2.27.0
^ permalink raw reply related
* [PATCH 17/17] arch: rename copy_thread_tls() back to copy_thread()
From: Christian Brauner @ 2020-06-22 23:43 UTC (permalink / raw)
To: linux-kernel
Cc: linux-riscv, Rich Felker, linux-sh, Peter Zijlstra (Intel),
Catalin Marinas, James E.J. Bottomley, Max Filippov, Guo Ren,
Matthew Wilcox (Oracle), H. Peter Anvin, sparclinux,
linux-hexagon, Christian Brauner, Vincent Chen, Will Deacon,
Thomas Gleixner, Anton Ivanov, Jonas Bonn, linux-s390, linux-ia64,
linux-c6x-dev, Brian Cain, linux-xtensa, Helge Deller, x86,
Russell King, Ley Foon Tan, Christian Borntraeger, Ingo Molnar,
Geert Uytterhoeven, linux-parisc, Mark Salter, linux-csky,
Matt Turner, linux-snps-arc, uclinux-h8-devel, Fenghua Yu,
Albert Ou, Kees Cook, Jeff Dike, linux-alpha, linux-um,
linuxppc-dev, Aurelien Jacquiot, linux-m68k, Thomas Bogendoerfer,
Ivan Kokshaysky, Greentime Hu, Paul Walmsley, Stafford Horne,
Stefan Kristiansson, Guan Xuetao, linux-arm-kernel,
Richard Henderson, Chris Zankel, Michal Simek, Tony Luck,
Yoshinori Sato, Nick Hu, Vineet Gupta, linux-mips, openrisc,
Palmer Dabbelt, Richard Weinberger, Paul Mackerras,
Linus Torvalds, David S. Miller, Al Viro
In-Reply-To: <20200622234326.906346-1-christian.brauner@ubuntu.com>
Now that HAVE_COPY_THREAD_TLS has been removed, rename copy_thread_tls()
back simply copy_thread(). It's a simpler name, and doesn't imply that only
tls is copied here. This finishes an outstanding chunk of internal process
creation work since we've added clone3().
Cc: Richard Henderson <rth@twiddle.net>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Matt Turner <mattst88@gmail.com>
Cc: Vineet Gupta <vgupta@synopsys.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Mark Salter <msalter@redhat.com>
Cc: Aurelien Jacquiot <jacquiot.aurelien@gmail.com>
Cc: Guo Ren <guoren@kernel.org>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Brian Cain <bcain@codeaurora.org>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Michal Simek <monstr@monstr.eu>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Nick Hu <nickhu@andestech.com>
Cc: Greentime Hu <green.hu@gmail.com>
Cc: Vincent Chen <deanbo422@gmail.com>
Cc: Ley Foon Tan <ley.foon.tan@intel.com>
Cc: Jonas Bonn <jonas@southpole.se>
Cc: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
Cc: Stafford Horne <shorne@gmail.com>
Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: Helge Deller <deller@gmx.de>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Albert Ou <aou@eecs.berkeley.edu>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Rich Felker <dalias@libc.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jeff Dike <jdike@addtoit.com>
Cc: Richard Weinberger <richard@nod.at>
Cc: Anton Ivanov <anton.ivanov@cambridgegreys.com>
Cc: Guan Xuetao <gxt@pku.edu.cn>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: x86@kernel.org
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Chris Zankel <chris@zankel.net>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: "Peter Zijlstra (Intel)" <peterz@infradead.org>
Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: linux-alpha@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-snps-arc@lists.infradead.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-c6x-dev@linux-c6x.org
Cc: linux-csky@vger.kernel.org
Cc: uclinux-h8-devel@lists.sourceforge.jp
Cc: linux-hexagon@vger.kernel.org
Cc: linux-ia64@vger.kernel.org
Cc: linux-m68k@lists.linux-m68k.org
Cc: linux-mips@vger.kernel.org
Cc: openrisc@lists.librecores.org
Cc: linux-parisc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-riscv@lists.infradead.org
Cc: linux-s390@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Cc: sparclinux@vger.kernel.org
Cc: linux-um@lists.infradead.org
Cc: linux-xtensa@linux-xtensa.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
arch/alpha/kernel/process.c | 2 +-
arch/arc/kernel/process.c | 2 +-
arch/arm/kernel/process.c | 2 +-
arch/arm64/kernel/process.c | 2 +-
arch/c6x/kernel/process.c | 2 +-
arch/csky/kernel/process.c | 2 +-
arch/h8300/kernel/process.c | 2 +-
arch/hexagon/kernel/process.c | 2 +-
arch/ia64/kernel/process.c | 4 ++--
arch/m68k/kernel/process.c | 2 +-
arch/microblaze/kernel/process.c | 2 +-
arch/mips/kernel/process.c | 2 +-
arch/nds32/kernel/process.c | 2 +-
arch/nios2/kernel/process.c | 2 +-
arch/openrisc/kernel/process.c | 4 ++--
arch/parisc/kernel/process.c | 2 +-
arch/powerpc/kernel/process.c | 2 +-
arch/riscv/kernel/process.c | 2 +-
arch/s390/kernel/process.c | 2 +-
arch/sh/kernel/process_32.c | 2 +-
arch/sparc/kernel/process.c | 6 +++---
arch/sparc/kernel/process_32.c | 2 +-
arch/sparc/kernel/process_64.c | 2 +-
arch/um/kernel/process.c | 2 +-
arch/unicore32/kernel/process.c | 2 +-
arch/x86/kernel/process.c | 2 +-
arch/x86/kernel/unwind_frame.c | 2 +-
arch/xtensa/kernel/process.c | 2 +-
include/linux/sched/task.h | 2 +-
kernel/fork.c | 2 +-
30 files changed, 34 insertions(+), 34 deletions(-)
diff --git a/arch/alpha/kernel/process.c b/arch/alpha/kernel/process.c
index dfdb6b6ba61c..bce96ddaf2fc 100644
--- a/arch/alpha/kernel/process.c
+++ b/arch/alpha/kernel/process.c
@@ -233,7 +233,7 @@ release_thread(struct task_struct *dead_task)
/*
* Copy architecture-specific thread state
*/
-int copy_thread_tls(unsigned long clone_flags, unsigned long usp,
+int copy_thread(unsigned long clone_flags, unsigned long usp,
unsigned long kthread_arg, struct task_struct *p,
unsigned long tls)
{
diff --git a/arch/arc/kernel/process.c b/arch/arc/kernel/process.c
index 8c8e5172fecd..5b6995c823a6 100644
--- a/arch/arc/kernel/process.c
+++ b/arch/arc/kernel/process.c
@@ -173,7 +173,7 @@ asmlinkage void ret_from_fork(void);
* | user_r25 |
* ------------------ <===== END of PAGE
*/
-int copy_thread_tls(unsigned long clone_flags, unsigned long usp,
+int copy_thread(unsigned long clone_flags, unsigned long usp,
unsigned long kthread_arg, struct task_struct *p, unsigned long tls)
{
struct pt_regs *c_regs; /* child's pt_regs */
diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 58eaa1f60e16..038669071f9a 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -226,7 +226,7 @@ void release_thread(struct task_struct *dead_task)
asmlinkage void ret_from_fork(void) __asm__("ret_from_fork");
int
-copy_thread_tls(unsigned long clone_flags, unsigned long stack_start,
+copy_thread(unsigned long clone_flags, unsigned long stack_start,
unsigned long stk_sz, struct task_struct *p, unsigned long tls)
{
struct thread_info *thread = task_thread_info(p);
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 6089638c7d43..84ec630b8ab5 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -375,7 +375,7 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
asmlinkage void ret_from_fork(void) asm("ret_from_fork");
-int copy_thread_tls(unsigned long clone_flags, unsigned long stack_start,
+int copy_thread(unsigned long clone_flags, unsigned long stack_start,
unsigned long stk_sz, struct task_struct *p, unsigned long tls)
{
struct pt_regs *childregs = task_pt_regs(p);
diff --git a/arch/c6x/kernel/process.c b/arch/c6x/kernel/process.c
index afa3ea9a93aa..aee49fb0a5eb 100644
--- a/arch/c6x/kernel/process.c
+++ b/arch/c6x/kernel/process.c
@@ -104,7 +104,7 @@ void start_thread(struct pt_regs *regs, unsigned int pc, unsigned long usp)
/*
* Copy a new thread context in its stack.
*/
-int copy_thread_tls(unsigned long clone_flags, unsigned long usp,
+int copy_thread(unsigned long clone_flags, unsigned long usp,
unsigned long ustk_size, struct task_struct *p,
unsigned long tls)
{
diff --git a/arch/csky/kernel/process.c b/arch/csky/kernel/process.c
index 8b3fad062ab2..28cfeaaf902a 100644
--- a/arch/csky/kernel/process.c
+++ b/arch/csky/kernel/process.c
@@ -40,7 +40,7 @@ unsigned long thread_saved_pc(struct task_struct *tsk)
return sw->r15;
}
-int copy_thread_tls(unsigned long clone_flags,
+int copy_thread(unsigned long clone_flags,
unsigned long usp,
unsigned long kthread_arg,
struct task_struct *p,
diff --git a/arch/h8300/kernel/process.c b/arch/h8300/kernel/process.c
index ae23de4dcf42..665c3d98c43e 100644
--- a/arch/h8300/kernel/process.c
+++ b/arch/h8300/kernel/process.c
@@ -105,7 +105,7 @@ void flush_thread(void)
{
}
-int copy_thread_tls(unsigned long clone_flags, unsigned long usp,
+int copy_thread(unsigned long clone_flags, unsigned long usp,
unsigned long topstk, struct task_struct *p,
unsigned long tls)
{
diff --git a/arch/hexagon/kernel/process.c b/arch/hexagon/kernel/process.c
index d756f9556dd7..e5c3ce019dcf 100644
--- a/arch/hexagon/kernel/process.c
+++ b/arch/hexagon/kernel/process.c
@@ -50,7 +50,7 @@ void arch_cpu_idle(void)
/*
* Copy architecture-specific thread state
*/
-int copy_thread_tls(unsigned long clone_flags, unsigned long usp,
+int copy_thread(unsigned long clone_flags, unsigned long usp,
unsigned long arg, struct task_struct *p, unsigned long tls)
{
struct thread_info *ti = task_thread_info(p);
diff --git a/arch/ia64/kernel/process.c b/arch/ia64/kernel/process.c
index 416dca619da5..6e1b076d1dcf 100644
--- a/arch/ia64/kernel/process.c
+++ b/arch/ia64/kernel/process.c
@@ -311,7 +311,7 @@ ia64_load_extra (struct task_struct *task)
* <clone syscall> <some kernel call frames>
* sys_clone :
* _do_fork _do_fork
- * copy_thread_tls copy_thread_tls
+ * copy_thread copy_thread
*
* This means that the stack layout is as follows:
*
@@ -333,7 +333,7 @@ ia64_load_extra (struct task_struct *task)
* so there is nothing to worry about.
*/
int
-copy_thread_tls(unsigned long clone_flags, unsigned long user_stack_base,
+copy_thread(unsigned long clone_flags, unsigned long user_stack_base,
unsigned long user_stack_size, struct task_struct *p,
unsigned long tls)
{
diff --git a/arch/m68k/kernel/process.c b/arch/m68k/kernel/process.c
index 0608439ba452..387c299838e7 100644
--- a/arch/m68k/kernel/process.c
+++ b/arch/m68k/kernel/process.c
@@ -138,7 +138,7 @@ asmlinkage int m68k_clone3(struct pt_regs *regs)
return sys_clone3((struct clone_args __user *)regs->d1, regs->d2);
}
-int copy_thread_tls(unsigned long clone_flags, unsigned long usp,
+int copy_thread(unsigned long clone_flags, unsigned long usp,
unsigned long arg, struct task_struct *p,
unsigned long tls)
{
diff --git a/arch/microblaze/kernel/process.c b/arch/microblaze/kernel/process.c
index c2ca9c326510..7206600fa6cd 100644
--- a/arch/microblaze/kernel/process.c
+++ b/arch/microblaze/kernel/process.c
@@ -54,7 +54,7 @@ void flush_thread(void)
{
}
-int copy_thread_tls(unsigned long clone_flags, unsigned long usp,
+int copy_thread(unsigned long clone_flags, unsigned long usp,
unsigned long arg, struct task_struct *p, unsigned long tls)
{
struct pt_regs *childregs = task_pt_regs(p);
diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c
index ff5320b79100..f090b56ba3f2 100644
--- a/arch/mips/kernel/process.c
+++ b/arch/mips/kernel/process.c
@@ -119,7 +119,7 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
/*
* Copy architecture-specific thread state
*/
-int copy_thread_tls(unsigned long clone_flags, unsigned long usp,
+int copy_thread(unsigned long clone_flags, unsigned long usp,
unsigned long kthread_arg, struct task_struct *p, unsigned long tls)
{
struct thread_info *ti = task_thread_info(p);
diff --git a/arch/nds32/kernel/process.c b/arch/nds32/kernel/process.c
index 7dbb1bf64165..1020e2c6dcd8 100644
--- a/arch/nds32/kernel/process.c
+++ b/arch/nds32/kernel/process.c
@@ -149,7 +149,7 @@ void flush_thread(void)
DEFINE_PER_CPU(struct task_struct *, __entry_task);
asmlinkage void ret_from_fork(void) __asm__("ret_from_fork");
-int copy_thread_tls(unsigned long clone_flags, unsigned long stack_start,
+int copy_thread(unsigned long clone_flags, unsigned long stack_start,
unsigned long stk_sz, struct task_struct *p,
unsigned long tls)
{
diff --git a/arch/nios2/kernel/process.c b/arch/nios2/kernel/process.c
index 3dde4d6d8fbe..a6d5b158167b 100644
--- a/arch/nios2/kernel/process.c
+++ b/arch/nios2/kernel/process.c
@@ -100,7 +100,7 @@ void flush_thread(void)
{
}
-int copy_thread_tls(unsigned long clone_flags, unsigned long usp,
+int copy_thread(unsigned long clone_flags, unsigned long usp,
unsigned long arg, struct task_struct *p, unsigned long tls)
{
struct pt_regs *childregs = task_pt_regs(p);
diff --git a/arch/openrisc/kernel/process.c b/arch/openrisc/kernel/process.c
index d7010e72450c..19045a3efb8a 100644
--- a/arch/openrisc/kernel/process.c
+++ b/arch/openrisc/kernel/process.c
@@ -116,7 +116,7 @@ void release_thread(struct task_struct *dead_task)
extern asmlinkage void ret_from_fork(void);
/*
- * copy_thread_tls
+ * copy_thread
* @clone_flags: flags
* @usp: user stack pointer or fn for kernel thread
* @arg: arg to fn for kernel thread; always NULL for userspace thread
@@ -147,7 +147,7 @@ extern asmlinkage void ret_from_fork(void);
*/
int
-copy_thread_tls(unsigned long clone_flags, unsigned long usp,
+copy_thread(unsigned long clone_flags, unsigned long usp,
unsigned long arg, struct task_struct *p, unsigned long tls)
{
struct pt_regs *userregs;
diff --git a/arch/parisc/kernel/process.c b/arch/parisc/kernel/process.c
index b7abb12edd3a..de6299ff1530 100644
--- a/arch/parisc/kernel/process.c
+++ b/arch/parisc/kernel/process.c
@@ -208,7 +208,7 @@ arch_initcall(parisc_idle_init);
* Copy architecture-specific thread state
*/
int
-copy_thread_tls(unsigned long clone_flags, unsigned long usp,
+copy_thread(unsigned long clone_flags, unsigned long usp,
unsigned long kthread_arg, struct task_struct *p, unsigned long tls)
{
struct pt_regs *cregs = &(p->thread.regs);
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 4650b9bb217f..794b754deec2 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1593,7 +1593,7 @@ static void setup_ksp_vsid(struct task_struct *p, unsigned long sp)
/*
* Copy architecture-specific thread state
*/
-int copy_thread_tls(unsigned long clone_flags, unsigned long usp,
+int copy_thread(unsigned long clone_flags, unsigned long usp,
unsigned long kthread_arg, struct task_struct *p,
unsigned long tls)
{
diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
index 824d117cf202..d8a79b3d16ea 100644
--- a/arch/riscv/kernel/process.c
+++ b/arch/riscv/kernel/process.c
@@ -101,7 +101,7 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
return 0;
}
-int copy_thread_tls(unsigned long clone_flags, unsigned long usp,
+int copy_thread(unsigned long clone_flags, unsigned long usp,
unsigned long arg, struct task_struct *p, unsigned long tls)
{
struct pt_regs *childregs = task_pt_regs(p);
diff --git a/arch/s390/kernel/process.c b/arch/s390/kernel/process.c
index eb6e23ad15a2..60582e83104b 100644
--- a/arch/s390/kernel/process.c
+++ b/arch/s390/kernel/process.c
@@ -80,7 +80,7 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
return 0;
}
-int copy_thread_tls(unsigned long clone_flags, unsigned long new_stackp,
+int copy_thread(unsigned long clone_flags, unsigned long new_stackp,
unsigned long arg, struct task_struct *p, unsigned long tls)
{
struct fake_frame
diff --git a/arch/sh/kernel/process_32.c b/arch/sh/kernel/process_32.c
index 537a82d80616..ea8d7548ba95 100644
--- a/arch/sh/kernel/process_32.c
+++ b/arch/sh/kernel/process_32.c
@@ -115,7 +115,7 @@ EXPORT_SYMBOL(dump_fpu);
asmlinkage void ret_from_fork(void);
asmlinkage void ret_from_kernel_thread(void);
-int copy_thread_tls(unsigned long clone_flags, unsigned long usp,
+int copy_thread(unsigned long clone_flags, unsigned long usp,
unsigned long arg, struct task_struct *p, unsigned long tls)
{
struct thread_info *ti = task_thread_info(p);
diff --git a/arch/sparc/kernel/process.c b/arch/sparc/kernel/process.c
index 8bbe62d77b77..5234b5ccc0b9 100644
--- a/arch/sparc/kernel/process.c
+++ b/arch/sparc/kernel/process.c
@@ -28,7 +28,7 @@ asmlinkage long sparc_fork(struct pt_regs *regs)
ret = _do_fork(&args);
/* If we get an error and potentially restart the system
- * call, we're screwed because copy_thread_tls() clobbered
+ * call, we're screwed because copy_thread() clobbered
* the parent's %o1. So detect that case and restore it
* here.
*/
@@ -53,7 +53,7 @@ asmlinkage long sparc_vfork(struct pt_regs *regs)
ret = _do_fork(&args);
/* If we get an error and potentially restart the system
- * call, we're screwed because copy_thread_tls() clobbered
+ * call, we're screwed because copy_thread() clobbered
* the parent's %o1. So detect that case and restore it
* here.
*/
@@ -99,7 +99,7 @@ asmlinkage long sparc_clone(struct pt_regs *regs)
ret = _do_fork(&args);
/* If we get an error and potentially restart the system
- * call, we're screwed because copy_thread_tls() clobbered
+ * call, we're screwed because copy_thread() clobbered
* the parent's %o1. So detect that case and restore it
* here.
*/
diff --git a/arch/sparc/kernel/process_32.c b/arch/sparc/kernel/process_32.c
index 3e1f7b639e9a..a9c2f6c2ab51 100644
--- a/arch/sparc/kernel/process_32.c
+++ b/arch/sparc/kernel/process_32.c
@@ -273,7 +273,7 @@ clone_stackframe(struct sparc_stackf __user *dst,
extern void ret_from_fork(void);
extern void ret_from_kernel_thread(void);
-int copy_thread_tls(unsigned long clone_flags, unsigned long sp,
+int copy_thread(unsigned long clone_flags, unsigned long sp,
unsigned long arg, struct task_struct *p,
unsigned long tls)
{
diff --git a/arch/sparc/kernel/process_64.c b/arch/sparc/kernel/process_64.c
index 278bf287c4be..6e4aca295e25 100644
--- a/arch/sparc/kernel/process_64.c
+++ b/arch/sparc/kernel/process_64.c
@@ -577,7 +577,7 @@ void fault_in_user_windows(struct pt_regs *regs)
* Parent --> %o0 == childs pid, %o1 == 0
* Child --> %o0 == parents pid, %o1 == 1
*/
-int copy_thread_tls(unsigned long clone_flags, unsigned long sp,
+int copy_thread(unsigned long clone_flags, unsigned long sp,
unsigned long arg, struct task_struct *p,
unsigned long tls)
{
diff --git a/arch/um/kernel/process.c b/arch/um/kernel/process.c
index e3a2cf92a373..26b5e243d3fc 100644
--- a/arch/um/kernel/process.c
+++ b/arch/um/kernel/process.c
@@ -152,7 +152,7 @@ void fork_handler(void)
userspace(¤t->thread.regs.regs, current_thread_info()->aux_fp_regs);
}
-int copy_thread_tls(unsigned long clone_flags, unsigned long sp,
+int copy_thread(unsigned long clone_flags, unsigned long sp,
unsigned long arg, struct task_struct * p, unsigned long tls)
{
void (*handler)(void);
diff --git a/arch/unicore32/kernel/process.c b/arch/unicore32/kernel/process.c
index 49a305565a53..660a646cfdf7 100644
--- a/arch/unicore32/kernel/process.c
+++ b/arch/unicore32/kernel/process.c
@@ -219,7 +219,7 @@ void release_thread(struct task_struct *dead_task)
asmlinkage void ret_from_fork(void) __asm__("ret_from_fork");
asmlinkage void ret_from_kernel_thread(void) __asm__("ret_from_kernel_thread");
-int copy_thread_tls(unsigned long clone_flags, unsigned long stack_start,
+int copy_thread(unsigned long clone_flags, unsigned long stack_start,
unsigned long stk_sz, struct task_struct *p,
unsigned long tls)
{
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index f362ce0d5ac0..47d369ce831d 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -121,7 +121,7 @@ static int set_new_tls(struct task_struct *p, unsigned long tls)
return do_set_thread_area_64(p, ARCH_SET_FS, tls);
}
-int copy_thread_tls(unsigned long clone_flags, unsigned long sp,
+int copy_thread(unsigned long clone_flags, unsigned long sp,
unsigned long arg, struct task_struct *p, unsigned long tls)
{
struct inactive_task_frame *frame;
diff --git a/arch/x86/kernel/unwind_frame.c b/arch/x86/kernel/unwind_frame.c
index 722a85f3b2dd..3070fd6561be 100644
--- a/arch/x86/kernel/unwind_frame.c
+++ b/arch/x86/kernel/unwind_frame.c
@@ -269,7 +269,7 @@ bool unwind_next_frame(struct unwind_state *state)
/*
* kthreads (other than the boot CPU's idle thread) have some
* partial regs at the end of their stack which were placed
- * there by copy_thread_tls(). But the regs don't have any
+ * there by copy_thread(). But the regs don't have any
* useful information, so we can skip them.
*
* This user_mode() check is slightly broader than a PF_KTHREAD
diff --git a/arch/xtensa/kernel/process.c b/arch/xtensa/kernel/process.c
index b7fe6f443b42..397a7de56377 100644
--- a/arch/xtensa/kernel/process.c
+++ b/arch/xtensa/kernel/process.c
@@ -201,7 +201,7 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
* involved. Much simpler to just not copy those live frames across.
*/
-int copy_thread_tls(unsigned long clone_flags, unsigned long usp_thread_fn,
+int copy_thread(unsigned long clone_flags, unsigned long usp_thread_fn,
unsigned long thread_fn_arg, struct task_struct *p,
unsigned long tls)
{
diff --git a/include/linux/sched/task.h b/include/linux/sched/task.h
index 77cbe14c3034..209a11c141ca 100644
--- a/include/linux/sched/task.h
+++ b/include/linux/sched/task.h
@@ -65,7 +65,7 @@ extern void fork_init(void);
extern void release_task(struct task_struct * p);
-extern int copy_thread_tls(unsigned long, unsigned long, unsigned long,
+extern int copy_thread(unsigned long, unsigned long, unsigned long,
struct task_struct *, unsigned long);
extern void flush_thread(void);
diff --git a/kernel/fork.c b/kernel/fork.c
index 8e52e16a1b5e..eaeaf224fd43 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2104,7 +2104,7 @@ static __latent_entropy struct task_struct *copy_process(
retval = copy_io(clone_flags, p);
if (retval)
goto bad_fork_cleanup_namespaces;
- retval = copy_thread_tls(clone_flags, args->stack, args->stack_size, p,
+ retval = copy_thread(clone_flags, args->stack, args->stack_size, p,
args->tls);
if (retval)
goto bad_fork_cleanup_io;
--
2.27.0
^ permalink raw reply related
* Re: [PATCHv2] tpm: ibmvtpm: Wait for ready buffer before probing for TPM2 attributes
From: Jarkko Sakkinen @ 2020-06-23 0:48 UTC (permalink / raw)
To: David Gibson
Cc: nayna, linux-kernel, jgg, paulus, peterhuewe, linuxppc-dev,
linux-integrity, stefanb
In-Reply-To: <20200619033040.121412-1-david@gibson.dropbear.id.au>
On Fri, Jun 19, 2020 at 01:30:40PM +1000, David Gibson wrote:
> The tpm2_get_cc_attrs_tbl() call will result in TPM commands being issued,
> which will need the use of the internal command/response buffer. But,
> we're issuing this *before* we've waited to make sure that buffer is
> allocated.
>
> This can result in intermittent failures to probe if the hypervisor / TPM
> implementation doesn't respond quickly enough. I find it fails almost
> every time with an 8 vcpu guest under KVM with software emulated TPM.
>
> To fix it, just move the tpm2_get_cc_attrs_tlb() call after the
> existing code to wait for initialization, which will ensure the buffer
> is allocated.
>
> Fixes: 18b3670d79ae9 ("tpm: ibmvtpm: Add support for TPM2")
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
/Jarkko
^ permalink raw reply
* Re: [PATCH 16/17] arch: remove HAVE_COPY_THREAD_TLS
From: Kees Cook @ 2020-06-23 0:44 UTC (permalink / raw)
To: Christian Brauner
Cc: Rich Felker, linux-sh, Peter Zijlstra, Catalin Marinas,
Heiko Carstens, linux-mips, James E.J. Bottomley, Guo Ren,
linux-csky, sparclinux, linux-hexagon, linux-riscv, Vincent Chen,
Will Deacon, Thomas Gleixner, Anton Ivanov, Jonas Bonn,
linux-s390, linux-ia64, linux-c6x-dev, Brian Cain, linux-xtensa,
Helge Deller, x86, Russell King, Ley Foon Tan, Mike Rapoport,
Christian Borntraeger, Ingo Molnar, Geert Uytterhoeven,
linux-parisc, Mark Salter, Matt Turner, linux-snps-arc,
uclinux-h8-devel, Fenghua Yu, Albert Ou, Vasily Gorbik, Jeff Dike,
linux-alpha, linux-um, linuxppc-dev, Aurelien Jacquiot,
linux-m68k, Thomas Bogendoerfer, Ivan Kokshaysky, Greentime Hu,
Paul Walmsley, Stafford Horne, Stefan Kristiansson, Guan Xuetao,
linux-arm-kernel, Richard Henderson, Michal Simek, Tony Luck,
Yoshinori Sato, Nick Hu, Vineet Gupta, linux-kernel, openrisc,
Palmer Dabbelt, Richard Weinberger, Paul Mackerras,
Linus Torvalds, David S. Miller, Al Viro
In-Reply-To: <20200622234326.906346-17-christian.brauner@ubuntu.com>
On Tue, Jun 23, 2020 at 01:43:25AM +0200, Christian Brauner wrote:
> All architectures support copy_thread_tls() now, so remove the legacy
> copy_thread() function and the HAVE_COPY_THREAD_TLS config option. Everyone
> uses the same process creation calling convention based on
> copy_thread_tls() and struct kernel_clone_args. This will make it easier to
> maintain the core process creation code under kernel/, simplifies the
> callpaths and makes the identical for all architectures.
> [...]
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
Massive CC list. ;) linux-arch@ may be sufficient.
Reviewed-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH 17/17] arch: rename copy_thread_tls() back to copy_thread()
From: Kees Cook @ 2020-06-23 0:46 UTC (permalink / raw)
To: Christian Brauner
Cc: Rich Felker, linux-sh, Peter Zijlstra (Intel), Catalin Marinas,
linux-mips, James E.J. Bottomley, Max Filippov, Guo Ren,
Matthew Wilcox (Oracle), H. Peter Anvin, sparclinux,
linux-hexagon, linux-riscv, Vincent Chen, Will Deacon,
Thomas Gleixner, Anton Ivanov, Jonas Bonn, linux-s390, linux-ia64,
linux-c6x-dev, Brian Cain, linux-xtensa, Helge Deller, x86,
Russell King, Ley Foon Tan, Christian Borntraeger, Ingo Molnar,
Geert Uytterhoeven, linux-parisc, Mark Salter, Matt Turner,
linux-snps-arc, uclinux-h8-devel, Fenghua Yu, Albert Ou,
linux-csky, Jeff Dike, linux-alpha, linux-um, linuxppc-dev,
Aurelien Jacquiot, linux-m68k, Thomas Bogendoerfer,
Ivan Kokshaysky, Greentime Hu, Paul Walmsley, Stafford Horne,
Stefan Kristiansson, Guan Xuetao, linux-arm-kernel,
Richard Henderson, Chris Zankel, Michal Simek, Tony Luck,
Yoshinori Sato, Nick Hu, Vineet Gupta, linux-kernel, openrisc,
Palmer Dabbelt, Richard Weinberger, Paul Mackerras,
Linus Torvalds, David S. Miller, Al Viro
In-Reply-To: <20200622234326.906346-18-christian.brauner@ubuntu.com>
On Tue, Jun 23, 2020 at 01:43:26AM +0200, Christian Brauner wrote:
> Now that HAVE_COPY_THREAD_TLS has been removed, rename copy_thread_tls()
> back simply copy_thread(). It's a simpler name, and doesn't imply that only
> tls is copied here. This finishes an outstanding chunk of internal process
> creation work since we've added clone3().
> [...]
> -copy_thread_tls(unsigned long clone_flags, unsigned long user_stack_base,
> +copy_thread(unsigned long clone_flags, unsigned long user_stack_base,
> unsigned long user_stack_size, struct task_struct *p,
> unsigned long tls)
Maybe clean up the arg indentation too? I'm not sure how strongly people
feel about that, but I think it'd be nice.
Either way:
Reviewed-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH 4/4] powerpc/pseries/iommu: Remove default DMA window before creating DDW
From: Alexey Kardashevskiy @ 2020-06-23 1:11 UTC (permalink / raw)
To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Thiago Jung Bauermann, Ram Pai
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <4bf1d32da3d13a44e3c2e4b04f369fe52c24a023.camel@gmail.com>
On 23/06/2020 04:59, Leonardo Bras wrote:
> Hello Alexey, thanks for the feedback!
>
> On Mon, 2020-06-22 at 20:02 +1000, Alexey Kardashevskiy wrote:
>>
>> On 19/06/2020 15:06, 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 some devices to use DDW, given they only
>>> allow one DMA window.
>>>
>>> If setting up a new DDW fails anywhere after the removal of this
>>> default DMA window, restore it using reset_dma_window.
>>
>> Nah... If we do it like this, then under pHyp we lose 32bit DMA for good
>> as pHyp can only create a single window and it has to map at
>> 0x800.0000.0000.0000. They probably do not care though.
>>
>> Under KVM, this will fail as VFIO allows creating 2 windows and it
>> starts from 0 but the existing iommu_bypass_supported_pSeriesLP() treats
>> the window address == 0 as a failure. And we want to keep both DMA
>> windows for PCI adapters with both 64bit and 32bit PCI functions (I
>> heard AMD GPU video + audio are like this) or someone could hotplug
>> 32bit DMA device on a vphb with already present 64bit DMA window so we
>> do not remove the default window.
>
> Well, then I would suggest doing something like this:
> query_ddw(...);
> if (query.windows_available == 0){
> remove_dma_window(...,default_win);
> query_ddw(...);
> }
>
> This would make sure to cover cases of windows available == 1
> and windows available > 1;
Is "1" what pHyp returns on query? And was it always like that? Then it
is probably ok. I just never really explored the idea of removing the
default window as we did not have to.
>> The last discussed thing I remember was that there was supposed to be a
>> new bit in "ibm,architecture-vec-5" (forgot the details), we could use
>> that to decide whether to keep the default window or not, like this.
>
> I checked on the latest LoPAR draft (soon to be published), for the
> ibm,architecture-vec 'option array 5' and this entry was the only
> recently added one that is related to this patchset:
>
> Byte 8 - Bit 0:
> SRIOV Virtual Functions Support Dynamic DMA Windows (DDW):
> 0: SRIOV Virtual Functions do not support DDW
> 1: SRIOV Virtual Functions do support DDW
>
> Isn't this equivalent to having a "ibm,ddw-applicable" property?
I am not sure, is there anything else to this new bit? I'd think if the
client supports it, then pHyp will create one 64bit window per a PE and
DDW API won't be needed. Thanks,
>
>
>>
>>> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
>>> ---
>>> arch/powerpc/platforms/pseries/iommu.c | 20 +++++++++++++++++---
>>> 1 file changed, 17 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
>>> index de633f6ae093..68d1ea957ac7 100644
>>> --- a/arch/powerpc/platforms/pseries/iommu.c
>>> +++ b/arch/powerpc/platforms/pseries/iommu.c
>>> @@ -1074,8 +1074,9 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>>> u64 dma_addr, max_addr;
>>> struct device_node *dn;
>>> u32 ddw_avail[3];
>>> +
>>> struct direct_window *window;
>>> - struct property *win64;
>>> + struct property *win64, *dfl_win;
>>
>> Make it "default_win" or "def_win", "dfl" hurts to read :)
>
> Sure, no problem :)
>
>>
>>> struct dynamic_dma_window_prop *ddwprop;
>>> struct failed_ddw_pdn *fpdn;
>>>
>>> @@ -1110,8 +1111,19 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>>> if (ret)
>>> goto out_failed;
>>>
>>> - /*
>>> - * Query if there is a second window of size to map the
>>> + /*
>>> + * First step of setting up DDW is removing the default DMA window,
>>> + * if it's present. It will make all the resources available to the
>>> + * new DDW window.
>>> + * If anything fails after this, we need to restore it.
>>> + */
>>> +
>>> + dfl_win = of_find_property(pdn, "ibm,dma-window", NULL);
>>> + if (dfl_win)
>>> + remove_dma_window(pdn, ddw_avail, dfl_win);
>>
>> Before doing so, you want to make sure that the "reset" is actually
>> supported. Thanks,
>
> Good catch, I will improve that.
>
>>
>>
>>> +
>>> + /*
>>> + * Query if there is a window of size to map the
>>> * whole partition. Query returns number of windows, largest
>>> * block assigned to PE (partition endpoint), and two bitmasks
>>> * of page sizes: supported and supported for migrate-dma.
>>> @@ -1219,6 +1231,8 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>>> kfree(win64);
>>>
>>> out_failed:
>>> + if (dfl_win)
>>> + reset_dma_window(dev, pdn);
>>>
>>> fpdn = kzalloc(sizeof(*fpdn), GFP_KERNEL);
>>> if (!fpdn)
>>>
>
> Best regards,
> Leonardo
>
--
Alexey
^ permalink raw reply
* Re: [PATCH 2/4] powerpc/pseries/iommu: Implement ibm,reset-pe-dma-windows rtas call
From: Alexey Kardashevskiy @ 2020-06-23 1:11 UTC (permalink / raw)
To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Thiago Jung Bauermann, Ram Pai
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <4180fd9bb0409a9c7009fef3ccae8eb2ad46d0a2.camel@gmail.com>
On 23/06/2020 04:58, Leonardo Bras wrote:
> Hello Alexey, thanks for the feedback!
>
> On Mon, 2020-06-22 at 20:02 +1000, Alexey Kardashevskiy wrote:
>>
>> On 19/06/2020 15:06, Leonardo Bras wrote:
>>> 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 | 33 ++++++++++++++++++++++++++
>>> 1 file changed, 33 insertions(+)
>>>
>>> diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
>>> index e5a617738c8b..5e1fbc176a37 100644
>>> --- a/arch/powerpc/platforms/pseries/iommu.c
>>> +++ b/arch/powerpc/platforms/pseries/iommu.c
>>> @@ -1012,6 +1012,39 @@ 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, ddw_ext[3];
>>> + u64 buid;
>>> + struct device_node *dn;
>>> + struct pci_dn *pdn;
>>> +
>>> + ret = of_property_read_u32_array(par_dn, "ibm,ddw-extensions",
>>> + &ddw_ext[0], 3);
>>
>> s/3/2/ as for the reset extension you do not need the "64bit largest
>> block" extension.
>
> Sure, I will update this.
>
>>
>>
>>> + 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(ddw_ext[1], 3, 1, NULL, cfg_addr,
>>
>> Here the "reset" extention is in ddw_ext[1]. Hm. 1/4 has a bug then.
>
> Humm, in 1/4 I used dd_ext[0] (how many extensions) and ddw_ext[2] (64-
> bit largest window count). I fail to see the bug here.
There is none, my bad :)
>> And I am pretty sure it won't compile as reset_dma_window() is not used
>> and it is static so fold it into one the next patches. Thanks,
>
> Sure, I will do that.
> I was questioning myself about this and thought it would be better to
> split for easier revision.
People separate things when a patch is really huge but even then I miss
the point - I'd really like to see a new function _and_ its uses in the
same patch, otherwise I either need to jump between mails or apply the
series, either is little but extra work :) Thanks,
>>
>>
>>> + BUID_HI(buid), BUID_LO(buid));
>>> + if (ret)
>>> + dev_info(&dev->dev,
>>> + "ibm,reset-pe-dma-windows(%x) %x %x %x returned %d ",
>>> + ddw_ext[1], 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,
>>>
>
> Best regards,
> Leonardo
>
--
Alexey
^ permalink raw reply
* Re: [PATCH 1/4] powerpc/pseries/iommu: Update call to ibm,query-pe-dma-windows
From: Alexey Kardashevskiy @ 2020-06-23 1:12 UTC (permalink / raw)
To: Leonardo Bras
Cc: Ram Pai, linux-kernel, Paul Mackerras, linuxppc-dev,
Thiago Jung Bauermann
In-Reply-To: <c15189a5c77752ea62022608dab28601965afaaa.camel@gmail.com>
On 23/06/2020 04:58, Leonardo Bras wrote:
> Hello Alexey, thank you for the feedback!
>
> On Mon, 2020-06-22 at 20:02 +1000, Alexey Kardashevskiy wrote:
>>
>> On 19/06/2020 15:06, 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.
>>>
>>> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
>>> ---
>>> arch/powerpc/platforms/pseries/iommu.c | 57 +++++++++++++++++++++-----
>>> 1 file changed, 46 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
>>> index 6d47b4a3ce39..e5a617738c8b 100644
>>> --- a/arch/powerpc/platforms/pseries/iommu.c
>>> +++ b/arch/powerpc/platforms/pseries/iommu.c
>>> @@ -334,7 +334,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;
>>> };
>>> @@ -869,14 +869,32 @@ static int find_existing_ddw_windows(void)
>>> }
>>> machine_arch_initcall(pseries, find_existing_ddw_windows);
>>>
>>> +/*
>>> + * 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.
>>> + */
>>> +
>>> +static int query_ddw_out_sz(struct device_node *par_dn)
>>
>> Can easily be folded into query_ddw().
>
> Sure, but it will get inlined by the compiler, and I think it reads
> better this way.
> I mean, I understand you have a reason to think it's better to fold it
> in query_ddw(), and I would like to better understand that to improve
> my code in the future.
You have numbers 5 and 6 (the number of parameters) twice in the file,
this is why I brought it up. query_ddw_out_sz() can potentially return
something else than 5 or 6 and you will have to change the callsite(s)
then, since these are not macros, this allows to think there may be more
places with 5 and 6. Dunno. A single function will simplify things imho.
>
>>> +{
>>> + int ret;
>>> + u32 ddw_ext[3];
>>> +
>>> + ret = of_property_read_u32_array(par_dn, "ibm,ddw-extensions",
>>> + &ddw_ext[0], 3);
>>> + if (ret || ddw_ext[0] < 2 || ddw_ext[2] != 1)
>>
>> Oh that PAPR thing again :-/
>>
>> ===
>> The “ibm,ddw-extensions” property value is a list of integers the first
>> integer indicates the number of extensions implemented and subsequent
>> integers, one per extension, provide a value associated with that
>> extension.
>> ===
>>
>> So ddw_ext[0] is length.
>> Listindex==2 is for "reset" says PAPR and
>> Listindex==3 is for this new 64bit "largest_available_block".
>>
>> So I'd expect ddw_ext[2] to have the "reset" token and ddw_ext[3] to
>> have "1" for this new feature but indexes are smaller. I am confused.
>> Either way these "2" and "3" needs to be defined in macros, "0" probably
>> too.
>
> Remember these indexes are not C-like 0-starting indexes, where the
> size would be Listindex==1.
Yeah I can see that is the assumption but out of curiosity - is it
written anywhere? Across PAPR, they index bytes from 1 but bits from 0 :-/
Either way make them macros.
> Basically, in C-like array it's :
> a[0] == size,
> a[1] == reset_token,
> a[2] == new 64bit "largest_available_block"
>
>> Please post 'lsprop "ibm,ddw-extensions"' here. Thanks,
>
> Sure:
> [root@host pci@800000029004005]# lsprop "ibm,ddw-extensions"
> ibm,dd
> w-extensions
> 00000002 00000056 00000000
Right, good. Thanks,
>
>
>>
>>> + return 5;
>>> + return 6;
>>> +}
>>> +
>>> 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 *par_dn)
>>> {
>>> struct device_node *dn;
>>> struct pci_dn *pdn;
>>> - u32 cfg_addr;
>>> + u32 cfg_addr, query_out[5];
>>> u64 buid;
>>> - int ret;
>>> + int ret, out_sz;
>>>
>>> /*
>>> * Get the config address and phb buid of the PE window.
>>> @@ -888,12 +906,29 @@ static int query_ddw(struct pci_dev *dev, const u32 *ddw_avail,
>>> pdn = PCI_DN(dn);
>>> buid = pdn->phb->buid;
>>> cfg_addr = ((pdn->busno << 16) | (pdn->devfn << 8));
>>> + out_sz = query_ddw_out_sz(par_dn);
>>> +
>>> + ret = rtas_call(ddw_avail[0], 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[0], 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;
>>> + }
>>>
>>> - ret = rtas_call(ddw_avail[0], 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);
>>> return ret;
>>> }
>>>
>>> @@ -1040,7 +1075,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;
>>>
>>> @@ -1068,7 +1103,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;
>>>
>
> Best regards,
> Leonardo
>
--
Alexey
^ permalink raw reply
* Re: [PATCH 3/4] powerpc/pseries/iommu: Move window-removing part of remove_ddw into remove_dma_window
From: Alexey Kardashevskiy @ 2020-06-23 1:12 UTC (permalink / raw)
To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Thiago Jung Bauermann, Ram Pai
Cc: Oliver, linuxppc-dev, linux-kernel
In-Reply-To: <ccf7b591f2bf61ba4705699b2e2b050c3cf48d99.camel@gmail.com>
On 23/06/2020 04:59, Leonardo Bras wrote:
> Hello Alexey, thanks for the feedback!
>
> On Mon, 2020-06-22 at 20:02 +1000, Alexey Kardashevskiy wrote:
>>
>> On 19/06/2020 15:06, 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 | 53 +++++++++++++++-----------
>>> 1 file changed, 31 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
>>> index 5e1fbc176a37..de633f6ae093 100644
>>> --- a/arch/powerpc/platforms/pseries/iommu.c
>>> +++ b/arch/powerpc/platforms/pseries/iommu.c
>>> @@ -767,25 +767,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 *pdn, u32 *ddw_avail,
>>
>> You do not need the entire ddw_avail here, pass just the token you need.
>
> Well, I just emulated the behavior of create_ddw() and query_ddw() as
> both just pass the array instead of the token, even though they only
> use a single token.
True, there is a pattern.
> I think it's to make the rest of the code independent of the design of
> the "ibm,ddw-applicable" array, and if it changes, only local changes
> on the functions will be needed.
The helper removes a window, if you are going to call other operations
in remove_dma_window(), then you'll have to change its name ;)
>> Also, despite this particular file, the "pdn" name is usually used for
>> struct pci_dn (not device_node), let's keep it that way.
>
> Sure, I got confused for some time about this, as we have:
> static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn).
> but on *_ddw() we have "struct pci_dn *pdn".
True again, not the cleanest style here.
> I will also add a patch that renames those 'struct device_node *pdn' to
> something like 'struct device_node *parent_dn'.
I would not go that far, we (well, Oliver) are getting rid of many
occurrences of pci_dn and Oliver may have a stronger opinion here.
>
>>> + struct property *win)
>>> {
>>> struct dynamic_dma_window_prop *dwp;
>>> - struct property *win64;
>>> - u32 ddw_avail[3];
>>> u64 liobn;
>>> - int ret = 0;
>>> -
>>> - ret = of_property_read_u32_array(np, "ibm,ddw-applicable",
>>> - &ddw_avail[0], 3);
>>> -
>>> - 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 */
>>> @@ -793,24 +782,44 @@ static void remove_ddw(struct device_node *np, bool remove_prop)
>>> 1ULL << (be32_to_cpu(dwp->window_shift) - PAGE_SHIFT), dwp);
>>> if (ret)
>>> pr_warn("%pOF failed to clear tces in window.\n",
>>> - np);
>>> + pdn);
>>> else
>>> pr_debug("%pOF successfully cleared tces in window.\n",
>>> - np);
>>> + pdn);
>>>
>>> ret = rtas_call(ddw_avail[2], 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);
>>> + pdn, ret, ddw_avail[2], 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);
>>> + pdn, ret, ddw_avail[2], liobn);
>>> +}
>>> +
>>> +static void remove_ddw(struct device_node *np, bool remove_prop)
>>> +{
>>> + struct property *win;
>>> + u32 ddw_avail[3];
>>> + int ret = 0;
>>> +
>>> + ret = of_property_read_u32_array(np, "ibm,ddw-applicable",
>>> + &ddw_avail[0], 3);
>>> + if (ret)
>>> + return;
>>> +
>>> + win = of_find_property(np, DIRECT64_PROPNAME, NULL);
>>> + if (!win)
>>> + return;
>>> +
>>> + if (win->length >= sizeof(struct dynamic_dma_window_prop))
>>
>> Any good reason not to make it "=="? Is there something optional or we
>> expect extension (which may not grow from the end but may add cells in
>> between). Thanks,
>
> Well, it comes from the old behavior of remove_ddw():
> - if (ret || win64->length < sizeof(*dwp))
> - goto delprop;
> As I reversed the logic from 'if (test) go out' to 'if (!test) do
> stuff', I also reversed (a < b) to !(a < b) => (a >= b).
>
> I have no problem changing that to '==', but it will produce a
> different behavior than before.
I missed than, never mind then.
>
>>
>>
>>> + 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);
>>>
>
> Best regards,
> Leonardo
>
--
Alexey
^ permalink raw reply
* Re: [PATCH 3/4] powerpc/pseries/iommu: Move window-removing part of remove_ddw into remove_dma_window
From: Oliver O'Halloran @ 2020-06-23 1:33 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Leonardo Bras, Ram Pai, Linux Kernel Mailing List, Paul Mackerras,
linuxppc-dev, Thiago Jung Bauermann
In-Reply-To: <887bf30e-ae9e-b0cb-0388-dc555692ff0a@ozlabs.ru>
On Tue, Jun 23, 2020 at 11:12 AM Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>
> On 23/06/2020 04:59, Leonardo Bras wrote:
> >
> >> Also, despite this particular file, the "pdn" name is usually used for
> >> struct pci_dn (not device_node), let's keep it that way.
> >
> > Sure, I got confused for some time about this, as we have:
> > static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn).
> > but on *_ddw() we have "struct pci_dn *pdn".
>
> True again, not the cleanest style here.
>
>
> > I will also add a patch that renames those 'struct device_node *pdn' to
> > something like 'struct device_node *parent_dn'.
I usually go with "np" or "node". In this case I'd use "parent_np" or
just "parent." As you said pci_dn conventionally uses pdn so that
should be avoided if at all possible. There's some places that just
use "dn" for device_node, but I don't think that's something we should
encourage due to how similar it is to pdn.
> I would not go that far, we (well, Oliver) are getting rid of many
> occurrences of pci_dn and Oliver may have a stronger opinion here.
I'm trying to remove the use of pci_dn from non-RTAS platforms which
doesn't apply to pseries. For RTAS platforms having pci_dn sort of
makes sense since it's used to cache data from the device_node and
having it saves you from needing to parse and validate the DT at
runtime since we're supposed to be relying on the FW provided settings
in the DT. I want to get rid of it on PowerNV because it's become a
dumping ground for random bits and pieces of platform specific data.
It's confusing at best and IMO it duplicates a lot of what's already
available in the per-PHB structures which the platform specific stuff
should actually be looking at.
Oliver
^ permalink raw reply
* Re: [PATCH 1/4] powerpc/pseries/iommu: Update call to ibm,query-pe-dma-windows
From: Leonardo Bras @ 2020-06-23 2:14 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Ram Pai, linux-kernel, Paul Mackerras, linuxppc-dev,
Thiago Jung Bauermann
In-Reply-To: <4176ea2b-c778-2f59-ba57-7339b873ead5@ozlabs.ru>
On Tue, 2020-06-23 at 11:12 +1000, Alexey Kardashevskiy wrote:
[snip]
> > > > +static int query_ddw_out_sz(struct device_node *par_dn)
> > >
> > > Can easily be folded into query_ddw().
> >
> > Sure, but it will get inlined by the compiler, and I think it reads
> > better this way.
> > I mean, I understand you have a reason to think it's better to fold it
> > in query_ddw(), and I would like to better understand that to improve
> > my code in the future.
>
> You have numbers 5 and 6 (the number of parameters) twice in the file,
> this is why I brought it up. query_ddw_out_sz() can potentially return
> something else than 5 or 6 and you will have to change the callsite(s)
> then, since these are not macros, this allows to think there may be more
> places with 5 and 6. Dunno. A single function will simplify things imho.
That's a good point. Thanks!
>
>
> > > > +{
> > > > + int ret;
> > > > + u32 ddw_ext[3];
> > > > +
> > > > + ret = of_property_read_u32_array(par_dn, "ibm,ddw-extensions",
> > > > + &ddw_ext[0], 3);
> > > > + if (ret || ddw_ext[0] < 2 || ddw_ext[2] != 1)
> > >
> > > Oh that PAPR thing again :-/
> > >
> > > ===
> > > The “ibm,ddw-extensions” property value is a list of integers the first
> > > integer indicates the number of extensions implemented and subsequent
> > > integers, one per extension, provide a value associated with that
> > > extension.
> > > ===
> > >
> > > So ddw_ext[0] is length.
> > > Listindex==2 is for "reset" says PAPR and
> > > Listindex==3 is for this new 64bit "largest_available_block".
> > >
> > > So I'd expect ddw_ext[2] to have the "reset" token and ddw_ext[3] to
> > > have "1" for this new feature but indexes are smaller. I am confused.
> > > Either way these "2" and "3" needs to be defined in macros, "0" probably
> > > too.
> >
> > Remember these indexes are not C-like 0-starting indexes, where the
> > size would be Listindex==1.
>
> Yeah I can see that is the assumption but out of curiosity - is it
> written anywhere? Across PAPR, they index bytes from 1 but bits from 0 :-/
From LoPAR:
The “ibm,ddw-extensions” property value is a list of integers the first
integer indicates the number of extensions implemented and subsequent
integers, one per extension, provide a value associated with that
extension.
And the list/table then shows extensions from 2 on onwards:
List index 2 : Token of the ibm,reset-pe-dma-windows RTAS Call
(...)
> Either way make them macros.
>
>
> > Basically, in C-like array it's :
> > a[0] == size,
> > a[1] == reset_token,
> > a[2] == new 64bit "largest_available_block"
> >
> > > Please post 'lsprop "ibm,ddw-extensions"' here. Thanks,
> >
> > Sure:
> > [root@host pci@800000029004005]# lsprop "ibm,ddw-extensions"
> > ibm,dd
> > w-extensions
> > 00000002 00000056 00000000
>
> Right, good. Thanks,
Thank you for reviewing!
>
> >
> > > > + return 5;
> > > > + return 6;
> > > > +}
> > > > +
> > > > 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 *par_dn)
> > > > {
> > > > struct device_node *dn;
> > > > struct pci_dn *pdn;
> > > > - u32 cfg_addr;
> > > > + u32 cfg_addr, query_out[5];
> > > > u64 buid;
> > > > - int ret;
> > > > + int ret, out_sz;
> > > >
> > > > /*
> > > > * Get the config address and phb buid of the PE window.
> > > > @@ -888,12 +906,29 @@ static int query_ddw(struct pci_dev *dev, const u32 *ddw_avail,
> > > > pdn = PCI_DN(dn);
> > > > buid = pdn->phb->buid;
> > > > cfg_addr = ((pdn->busno << 16) | (pdn->devfn << 8));
> > > > + out_sz = query_ddw_out_sz(par_dn);
> > > > +
> > > > + ret = rtas_call(ddw_avail[0], 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[0], 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;
> > > > + }
> > > >
> > > > - ret = rtas_call(ddw_avail[0], 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);
> > > > return ret;
> > > > }
> > > >
> > > > @@ -1040,7 +1075,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;
> > > >
> > > > @@ -1068,7 +1103,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;
> > > >
> >
> > Best regards,
> > Leonardo
> >
Best regards,
Leonardo Bras
^ permalink raw reply
* Re: [PATCH 2/4] powerpc/pseries/iommu: Implement ibm,reset-pe-dma-windows rtas call
From: Leonardo Bras @ 2020-06-23 2:20 UTC (permalink / raw)
To: Alexey Kardashevskiy, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Thiago Jung Bauermann, Ram Pai
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <c02fbebb-32ed-f328-ff93-ab2201844d61@ozlabs.ru>
On Tue, 2020-06-23 at 11:11 +1000, Alexey Kardashevskiy wrote:
>
> On 23/06/2020 04:58, Leonardo Bras wrote:
> > Hello Alexey, thanks for the feedback!
> >
> > On Mon, 2020-06-22 at 20:02 +1000, Alexey Kardashevskiy wrote:
> > > On 19/06/2020 15:06, Leonardo Bras wrote:
> > > > 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 | 33 ++++++++++++++++++++++++++
> > > > 1 file changed, 33 insertions(+)
> > > >
> > > > diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> > > > index e5a617738c8b..5e1fbc176a37 100644
> > > > --- a/arch/powerpc/platforms/pseries/iommu.c
> > > > +++ b/arch/powerpc/platforms/pseries/iommu.c
> > > > @@ -1012,6 +1012,39 @@ 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, ddw_ext[3];
> > > > + u64 buid;
> > > > + struct device_node *dn;
> > > > + struct pci_dn *pdn;
> > > > +
> > > > + ret = of_property_read_u32_array(par_dn, "ibm,ddw-extensions",
> > > > + &ddw_ext[0], 3);
> > >
> > > s/3/2/ as for the reset extension you do not need the "64bit largest
> > > block" extension.
> >
> > Sure, I will update this.
> >
> > >
> > > > + 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(ddw_ext[1], 3, 1, NULL, cfg_addr,
> > >
> > > Here the "reset" extention is in ddw_ext[1]. Hm. 1/4 has a bug then.
> >
> > Humm, in 1/4 I used dd_ext[0] (how many extensions) and ddw_ext[2] (64-
> > bit largest window count). I fail to see the bug here.
>
> There is none, my bad :)
>
>
> > > And I am pretty sure it won't compile as reset_dma_window() is not used
> > > and it is static so fold it into one the next patches. Thanks,
> >
> > Sure, I will do that.
> > I was questioning myself about this and thought it would be better to
> > split for easier revision.
>
> People separate things when a patch is really huge but even then I miss
> the point - I'd really like to see a new function _and_ its uses in the
> same patch, otherwise I either need to jump between mails or apply the
> series, either is little but extra work :) Thanks,
Sure, that makes sense.
I will keep that in mind for future patchsets (and v2).
Thank you!
>
>
> > >
> > > > + BUID_HI(buid), BUID_LO(buid));
> > > > + if (ret)
> > > > + dev_info(&dev->dev,
> > > > + "ibm,reset-pe-dma-windows(%x) %x %x %x returned %d ",
> > > > + ddw_ext[1], 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,
> > > >
> >
> > Best regards,
> > Leonardo
> >
^ 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