LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [RFC v2 2/2] drm/amd/display: Use PPC FPU functions
From: Anson Jacob @ 2021-07-21  4:48 UTC (permalink / raw)
  To: mpe, benh, paulus, christophe.leroy, linuxppc-dev, amd-gfx,
	linux-kernel
  Cc: Sunpeng.Li, Harry Wentland, qingqing.zhuo, Rodrigo.Siqueira,
	roman.li, Christoph Hellwig, Anson.Jacob, Aurabindo.Pillai,
	Bhawanpreet.Lakha, Christian König, bindu.r
In-Reply-To: <20210721044801.840501-1-Anson.Jacob@amd.com>

Use kernel_fpu_begin & kernel_fpu_end for PPC

Depends on "ppc/fpu: Add generic FPU api similar to x86"

v2:
- Got rid of macro switch for PPC as header file with same
  name as x86 is added by previous patch in the series

Signed-off-by: Anson Jacob <Anson.Jacob@amd.com>
CC: Christoph Hellwig <hch@infradead.org>
CC: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
CC: Harry Wentland <harry.wentland@amd.com>
CC: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/display/dc/os_types.h | 29 -----------------------
 1 file changed, 29 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/os_types.h b/drivers/gpu/drm/amd/display/dc/os_types.h
index 126c2f3a4dd3..47ef434f93d8 100644
--- a/drivers/gpu/drm/amd/display/dc/os_types.h
+++ b/drivers/gpu/drm/amd/display/dc/os_types.h
@@ -51,38 +51,9 @@
 #define dm_error(fmt, ...) DRM_ERROR(fmt, ##__VA_ARGS__)
 
 #if defined(CONFIG_DRM_AMD_DC_DCN)
-#if defined(CONFIG_X86)
 #include <asm/fpu/api.h>
 #define DC_FP_START() kernel_fpu_begin()
 #define DC_FP_END() kernel_fpu_end()
-#elif defined(CONFIG_PPC64)
-#include <asm/switch_to.h>
-#include <asm/cputable.h>
-#define DC_FP_START() { \
-	if (cpu_has_feature(CPU_FTR_VSX_COMP)) { \
-		preempt_disable(); \
-		enable_kernel_vsx(); \
-	} else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) { \
-		preempt_disable(); \
-		enable_kernel_altivec(); \
-	} else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) { \
-		preempt_disable(); \
-		enable_kernel_fp(); \
-	} \
-}
-#define DC_FP_END() { \
-	if (cpu_has_feature(CPU_FTR_VSX_COMP)) { \
-		disable_kernel_vsx(); \
-		preempt_enable(); \
-	} else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) { \
-		disable_kernel_altivec(); \
-		preempt_enable(); \
-	} else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) { \
-		disable_kernel_fp(); \
-		preempt_enable(); \
-	} \
-}
-#endif
 #endif
 
 /*
-- 
2.25.1


^ permalink raw reply related

* [RFC v2 0/2] PPC: Add generic FPU api similar to x86
From: Anson Jacob @ 2021-07-21  4:47 UTC (permalink / raw)
  To: mpe, benh, paulus, christophe.leroy, linuxppc-dev, amd-gfx,
	linux-kernel
  Cc: Sunpeng.Li, Harry.Wentland, qingqing.zhuo, Rodrigo.Siqueira,
	roman.li, Anson.Jacob, Aurabindo.Pillai, Bhawanpreet.Lakha,
	bindu.r

This is an attempt to have generic FPU enable/disable
calls similar to x86. 
So that we can simplify gpu/drm/amd/display/dc/os_types.h

Also adds FPU correctness logic seen in x86.

v2:
- Added asm/fpu/api.h powerpc variant with kernel_fpu_begin/end()
  and kernel_fpu_enabled() declarations
- Updated kernel_fpu_enabled as EXPORT_SYMBOL_GPL
- Got rid of macro switch for PPC in dc/os_types.h as header file
  with same name as x86 is added by previous patch in the series

Anson Jacob (2):
  ppc/fpu: Add generic FPU api similar to x86
  drm/amd/display: Use PPC FPU functions

 arch/powerpc/include/asm/fpu/api.h        |  18 +++
 arch/powerpc/include/asm/switch_to.h      |  25 +----
 arch/powerpc/kernel/process.c             | 130 ++++++++++++++++++++++
 drivers/gpu/drm/amd/display/dc/os_types.h |  29 -----
 4 files changed, 151 insertions(+), 51 deletions(-)
 create mode 100644 arch/powerpc/include/asm/fpu/api.h

-- 
2.25.1


^ permalink raw reply

* [RFC v2 1/2] ppc/fpu: Add generic FPU api similar to x86
From: Anson Jacob @ 2021-07-21  4:48 UTC (permalink / raw)
  To: mpe, benh, paulus, christophe.leroy, linuxppc-dev, amd-gfx,
	linux-kernel
  Cc: Sunpeng.Li, Harry Wentland, qingqing.zhuo, Rodrigo.Siqueira,
	roman.li, Christoph Hellwig, Anson.Jacob, Aurabindo.Pillai,
	Bhawanpreet.Lakha, Christian König, bindu.r
In-Reply-To: <20210721044801.840501-1-Anson.Jacob@amd.com>

- Add kernel_fpu_begin & kernel_fpu_end API as x86
- Add logic similar to x86 to ensure fpu
  begin/end call correctness
- Add kernel_fpu_enabled to know if FPU is enabled

v2:
- Added asm/fpu/api.h powerpc variant with kernel_fpu_begin/end()
  and kernel_fpu_enabled() declarations
- Updated kernel_fpu_enabled as EXPORT_SYMBOL_GPL

Signed-off-by: Anson Jacob <Anson.Jacob@amd.com>
CC: Christoph Hellwig <hch@infradead.org>
CC: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
CC: Harry Wentland <harry.wentland@amd.com>
CC: Christian König <christian.koenig@amd.com>
---
 arch/powerpc/include/asm/fpu/api.h   |  18 ++++
 arch/powerpc/include/asm/switch_to.h |  25 +-----
 arch/powerpc/kernel/process.c        | 130 +++++++++++++++++++++++++++
 3 files changed, 151 insertions(+), 22 deletions(-)
 create mode 100644 arch/powerpc/include/asm/fpu/api.h

diff --git a/arch/powerpc/include/asm/fpu/api.h b/arch/powerpc/include/asm/fpu/api.h
new file mode 100644
index 000000000000..57308cdc65c9
--- /dev/null
+++ b/arch/powerpc/include/asm/fpu/api.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _ASM_POWERPC_FPU_API_H
+#define _ASM_POWERPC_FPU_API_H
+
+/*
+ * Use kernel_fpu_begin/end() if you intend to use FPU in kernel context. It
+ * disables preemption so be careful if you intend to use it for long periods
+ * of time.
+ * TODO: If you intend to use the FPU in irq/softirq you need to check first with
+ * irq_fpu_usable() if it is possible.
+ */
+
+extern bool kernel_fpu_enabled(void);
+extern void kernel_fpu_begin(void);
+extern void kernel_fpu_end(void);
+
+#endif /* _ASM_POWERPC_FPU_API_H */
diff --git a/arch/powerpc/include/asm/switch_to.h b/arch/powerpc/include/asm/switch_to.h
index 9d1fbd8be1c7..a9a919279f48 100644
--- a/arch/powerpc/include/asm/switch_to.h
+++ b/arch/powerpc/include/asm/switch_to.h
@@ -41,10 +41,7 @@ extern void enable_kernel_fp(void);
 extern void flush_fp_to_thread(struct task_struct *);
 extern void giveup_fpu(struct task_struct *);
 extern void save_fpu(struct task_struct *);
-static inline void disable_kernel_fp(void)
-{
-	msr_check_and_clear(MSR_FP);
-}
+extern void disable_kernel_fp(void);
 #else
 static inline void save_fpu(struct task_struct *t) { }
 static inline void flush_fp_to_thread(struct task_struct *t) { }
@@ -55,10 +52,7 @@ extern void enable_kernel_altivec(void);
 extern void flush_altivec_to_thread(struct task_struct *);
 extern void giveup_altivec(struct task_struct *);
 extern void save_altivec(struct task_struct *);
-static inline void disable_kernel_altivec(void)
-{
-	msr_check_and_clear(MSR_VEC);
-}
+extern void disable_kernel_altivec(void);
 #else
 static inline void save_altivec(struct task_struct *t) { }
 static inline void __giveup_altivec(struct task_struct *t) { }
@@ -67,20 +61,7 @@ static inline void __giveup_altivec(struct task_struct *t) { }
 #ifdef CONFIG_VSX
 extern void enable_kernel_vsx(void);
 extern void flush_vsx_to_thread(struct task_struct *);
-static inline void disable_kernel_vsx(void)
-{
-	msr_check_and_clear(MSR_FP|MSR_VEC|MSR_VSX);
-}
-#else
-static inline void enable_kernel_vsx(void)
-{
-	BUILD_BUG();
-}
-
-static inline void disable_kernel_vsx(void)
-{
-	BUILD_BUG();
-}
+extern void disable_kernel_vsx(void);
 #endif
 
 #ifdef CONFIG_SPE
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 185beb290580..969096c0123c 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -75,6 +75,17 @@
 #define TM_DEBUG(x...) do { } while(0)
 #endif
 
+/*
+ * Track whether the kernel is using the FPU state
+ * currently.
+ *
+ * This flag is used:
+ *
+ *   - kernel_fpu_begin()/end() correctness
+ *   - kernel_fpu_enabled info
+ */
+static DEFINE_PER_CPU(bool, in_kernel_fpu);
+
 extern unsigned long _get_SP(void);
 
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
@@ -212,6 +223,9 @@ void enable_kernel_fp(void)
 	unsigned long cpumsr;
 
 	WARN_ON(preemptible());
+	WARN_ON_ONCE(this_cpu_read(in_kernel_fpu));
+
+	this_cpu_write(in_kernel_fpu, true);
 
 	cpumsr = msr_check_and_set(MSR_FP);
 
@@ -231,6 +245,15 @@ void enable_kernel_fp(void)
 	}
 }
 EXPORT_SYMBOL(enable_kernel_fp);
+
+void disable_kernel_fp(void)
+{
+	WARN_ON_ONCE(!this_cpu_read(in_kernel_fpu));
+
+	this_cpu_write(in_kernel_fpu, false);
+	msr_check_and_clear(MSR_FP);
+}
+EXPORT_SYMBOL(disable_kernel_fp);
 #else
 static inline void __giveup_fpu(struct task_struct *tsk) { }
 #endif /* CONFIG_PPC_FPU */
@@ -263,6 +286,9 @@ void enable_kernel_altivec(void)
 	unsigned long cpumsr;
 
 	WARN_ON(preemptible());
+	WARN_ON_ONCE(this_cpu_read(in_kernel_fpu));
+
+	this_cpu_write(in_kernel_fpu, true);
 
 	cpumsr = msr_check_and_set(MSR_VEC);
 
@@ -283,6 +309,14 @@ void enable_kernel_altivec(void)
 }
 EXPORT_SYMBOL(enable_kernel_altivec);
 
+void disable_kernel_altivec(void)
+{
+	WARN_ON_ONCE(!this_cpu_read(in_kernel_fpu));
+
+	this_cpu_write(in_kernel_fpu, false);
+	msr_check_and_clear(MSR_VEC);
+}
+EXPORT_SYMBOL(disable_kernel_altivec);
 /*
  * Make sure the VMX/Altivec register state in the
  * the thread_struct is up to date for task tsk.
@@ -333,6 +367,9 @@ void enable_kernel_vsx(void)
 	unsigned long cpumsr;
 
 	WARN_ON(preemptible());
+	WARN_ON_ONCE(this_cpu_read(in_kernel_fpu));
+
+	this_cpu_write(in_kernel_fpu, true);
 
 	cpumsr = msr_check_and_set(MSR_FP|MSR_VEC|MSR_VSX);
 
@@ -354,6 +391,15 @@ void enable_kernel_vsx(void)
 }
 EXPORT_SYMBOL(enable_kernel_vsx);
 
+void disable_kernel_vsx(void)
+{
+	WARN_ON_ONCE(!this_cpu_read(in_kernel_fpu));
+
+	this_cpu_write(in_kernel_fpu, false);
+	msr_check_and_clear(MSR_FP|MSR_VEC|MSR_VSX);
+}
+EXPORT_SYMBOL(disable_kernel_vsx);
+
 void flush_vsx_to_thread(struct task_struct *tsk)
 {
 	if (tsk->thread.regs) {
@@ -406,6 +452,90 @@ void flush_spe_to_thread(struct task_struct *tsk)
 }
 #endif /* CONFIG_SPE */
 
+static bool fpu_support(void)
+{
+	if (cpu_has_feature(CPU_FTR_VSX_COMP)) {
+		return true;
+	} else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) {
+		return true;
+	} else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) {
+		return true;
+	}
+
+	return false;
+}
+
+bool kernel_fpu_enabled(void)
+{
+	return this_cpu_read(in_kernel_fpu);
+}
+EXPORT_SYMBOL_GPL(kernel_fpu_enabled);
+
+void kernel_fpu_begin(void)
+{
+	if (!fpu_support()) {
+		WARN_ON_ONCE(1);
+		return;
+	}
+
+	preempt_disable();
+
+#ifdef CONFIG_VSX
+	if (cpu_has_feature(CPU_FTR_VSX_COMP)) {
+		enable_kernel_vsx();
+		return;
+	}
+#endif
+
+#ifdef CONFIG_ALTIVEC
+	if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) {
+		enable_kernel_altivec();
+		return;
+	}
+#endif
+
+#ifdef CONFIG_PPC_FPU
+	if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) {
+		enable_kernel_fp();
+		return;
+	}
+#endif
+}
+EXPORT_SYMBOL_GPL(kernel_fpu_begin);
+
+void kernel_fpu_end(void)
+{
+	if (!fpu_support()) {
+		WARN_ON_ONCE(1);
+		return;
+	}
+
+#ifdef CONFIG_VSX
+	if (cpu_has_feature(CPU_FTR_VSX_COMP)) {
+		disable_kernel_vsx();
+		goto done;
+	}
+#endif
+
+#ifdef CONFIG_ALTIVEC
+	if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) {
+		disable_kernel_altivec();
+		goto done;
+	}
+#endif
+
+#ifdef CONFIG_PPC_FPU
+	if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) {
+		disable_kernel_fp();
+		goto done;
+	}
+#endif
+
+done:
+	preempt_enable();
+}
+EXPORT_SYMBOL_GPL(kernel_fpu_end);
+
 static unsigned long msr_all_available;
 
 static int __init init_msr_all_available(void)
-- 
2.25.1


^ permalink raw reply related

* Re: [PATCH v5 10/11] powerpc/pseries/iommu: Make use of DDW for indirect mapping
From: Alexey Kardashevskiy @ 2021-07-21  3:32 UTC (permalink / raw)
  To: Frederic Barrat, Leonardo Bras, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, David Gibson,
	kernel test robot, Nicolin Chen
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <b98f696a-ed64-4c9e-ccb6-549ae8bc7fd6@linux.ibm.com>



On 21/07/2021 04:12, Frederic Barrat wrote:
> 
> 
> On 16/07/2021 10:27, Leonardo Bras wrote:
>> So far it's assumed possible to map the guest RAM 1:1 to the bus, which
>> works with a small number of devices. SRIOV changes it as the user can
>> configure hundreds VFs and since phyp preallocates TCEs and does not
>> allow IOMMU pages bigger than 64K, it has to limit the number of TCEs
>> per a PE to limit waste of physical pages.
>>
>> As of today, if the assumed direct mapping is not possible, DDW creation
>> is skipped and the default DMA window "ibm,dma-window" is used instead.
>>
>> By using DDW, indirect mapping  can get more TCEs than available for the
>> default DMA window, and also get access to using much larger pagesizes
>> (16MB as implemented in qemu vs 4k from default DMA window), causing a
>> significant increase on the maximum amount of memory that can be IOMMU
>> mapped at the same time.
>>
>> Indirect mapping will only be used if direct mapping is not a
>> possibility.
>>
>> For indirect mapping, it's necessary to re-create the iommu_table with
>> the new DMA window parameters, so iommu_alloc() can use it.
>>
>> Removing the default DMA window for using DDW with indirect mapping
>> is only allowed if there is no current IOMMU memory allocated in
>> the iommu_table. enable_ddw() is aborted otherwise.
>>
>> Even though there won't be both direct and indirect mappings at the
>> same time, we can't reuse the DIRECT64_PROPNAME property name, or else
>> an older kexec()ed kernel can assume direct mapping, and skip
>> iommu_alloc(), causing undesirable behavior.
>> So a new property name DMA64_PROPNAME "linux,dma64-ddr-window-info"
>> was created to represent a DDW that does not allow direct mapping.
>>
>> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
>> ---
>>   arch/powerpc/platforms/pseries/iommu.c | 87 +++++++++++++++++++++-----
>>   1 file changed, 72 insertions(+), 15 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/pseries/iommu.c 
>> b/arch/powerpc/platforms/pseries/iommu.c
>> index 22d251e15b61..a67e71c49aeb 100644
>> --- a/arch/powerpc/platforms/pseries/iommu.c
>> +++ b/arch/powerpc/platforms/pseries/iommu.c
>> @@ -375,6 +375,7 @@ static DEFINE_SPINLOCK(direct_window_list_lock);
>>   /* protects initializing window twice for same device */
>>   static DEFINE_MUTEX(direct_window_init_mutex);
>>   #define DIRECT64_PROPNAME "linux,direct64-ddr-window-info"
>> +#define DMA64_PROPNAME "linux,dma64-ddr-window-info"
>>   static int tce_clearrange_multi_pSeriesLP(unsigned long start_pfn,
>>                       unsigned long num_pfn, const void *arg)
>> @@ -925,6 +926,7 @@ static int find_existing_ddw_windows(void)
>>           return 0;
>>       find_existing_ddw_windows_named(DIRECT64_PROPNAME);
>> +    find_existing_ddw_windows_named(DMA64_PROPNAME);
>>       return 0;
>>   }
>> @@ -1211,14 +1213,17 @@ static bool enable_ddw(struct pci_dev *dev, 
>> struct device_node *pdn)
>>       struct ddw_create_response create;
>>       int page_shift;
>>       u64 win_addr;
>> +    const char *win_name;
>>       struct device_node *dn;
>>       u32 ddw_avail[DDW_APPLICABLE_SIZE];
>>       struct direct_window *window;
>>       struct property *win64;
>>       bool ddw_enabled = false;
>>       struct failed_ddw_pdn *fpdn;
>> -    bool default_win_removed = false;
>> +    bool default_win_removed = false, direct_mapping = false;
>>       bool pmem_present;
>> +    struct pci_dn *pci = PCI_DN(pdn);
>> +    struct iommu_table *tbl = pci->table_group->tables[0];
>>       dn = of_find_node_by_type(NULL, "ibm,pmemory");
>>       pmem_present = dn != NULL;
>> @@ -1227,6 +1232,7 @@ static bool enable_ddw(struct pci_dev *dev, 
>> struct device_node *pdn)
>>       mutex_lock(&direct_window_init_mutex);
>>       if (find_existing_ddw(pdn, &dev->dev.archdata.dma_offset, &len)) {
>> +        direct_mapping = (len >= max_ram_len);
>>           ddw_enabled = true;
>>           goto out_unlock;
>>       }
>> @@ -1307,8 +1313,7 @@ static bool enable_ddw(struct pci_dev *dev, 
>> struct device_node *pdn)
>>                 query.page_size);
>>           goto out_failed;
>>       }
>> -    /* verify the window * number of ptes will map the partition */
>> -    /* check largest block * page size > max memory hotplug addr */
>> +
>>       /*
>>        * The "ibm,pmemory" can appear anywhere in the address space.
>>        * Assuming it is still backed by page structs, try 
>> MAX_PHYSMEM_BITS
>> @@ -1324,13 +1329,25 @@ static bool enable_ddw(struct pci_dev *dev, 
>> struct device_node *pdn)
>>               dev_info(&dev->dev, "Skipping ibm,pmemory");
>>       }
>> +    /* check if the available block * number of ptes will map 
>> everything */
>>       if (query.largest_available_block < (1ULL << (len - page_shift))) {
>>           dev_dbg(&dev->dev,
>>               "can't map partition max 0x%llx with %llu %llu-sized 
>> pages\n",
>>               1ULL << len,
>>               query.largest_available_block,
>>               1ULL << page_shift);
>> -        goto out_failed;
>> +
>> +        /* DDW + IOMMU on single window may fail if there is any 
>> allocation */
>> +        if (default_win_removed && iommu_table_in_use(tbl)) {
>> +            dev_dbg(&dev->dev, "current IOMMU table in use, can't be 
>> replaced.\n");
>> +            goto out_failed;
>> +        }
>> +
>> +        len = order_base_2(query.largest_available_block << page_shift);
>> +        win_name = DMA64_PROPNAME;
>> +    } else {
>> +        direct_mapping = true;
>> +        win_name = DIRECT64_PROPNAME;
>>       }
>>       ret = create_ddw(dev, ddw_avail, &create, page_shift, len);
>> @@ -1341,8 +1358,7 @@ static bool enable_ddw(struct pci_dev *dev, 
>> struct device_node *pdn)
>>             create.liobn, dn);
>>       win_addr = ((u64)create.addr_hi << 32) | create.addr_lo;
>> -    win64 = ddw_property_create(DIRECT64_PROPNAME, create.liobn, 
>> win_addr,
>> -                    page_shift, len);
>> +    win64 = ddw_property_create(win_name, create.liobn, win_addr, 
>> page_shift, len);
>>       if (!win64) {
>>           dev_info(&dev->dev,
>>                "couldn't allocate property, property name, or value\n");
>> @@ -1360,12 +1376,51 @@ static bool enable_ddw(struct pci_dev *dev, 
>> struct device_node *pdn)
>>       if (!window)
>>           goto out_del_prop;
>> -    ret = walk_system_ram_range(0, memblock_end_of_DRAM() >> PAGE_SHIFT,
>> -            win64->value, tce_setrange_multi_pSeriesLP_walk);
>> -    if (ret) {
>> -        dev_info(&dev->dev, "failed to map direct window for %pOF: 
>> %d\n",
>> -             dn, ret);
>> -        goto out_del_list;
>> +    if (direct_mapping) {
>> +        /* DDW maps the whole partition, so enable direct DMA mapping */
>> +        ret = walk_system_ram_range(0, memblock_end_of_DRAM() >> 
>> PAGE_SHIFT,
>> +                        win64->value, 
>> tce_setrange_multi_pSeriesLP_walk);
>> +        if (ret) {
>> +            dev_info(&dev->dev, "failed to map direct window for 
>> %pOF: %d\n",
>> +                 dn, ret);
>> +            goto out_del_list;
>> +        }
>> +    } else {
>> +        struct iommu_table *newtbl;
>> +        int i;
>> +
>> +        for (i = 0; i < ARRAY_SIZE(pci->phb->mem_resources); i++) {
>> +            const unsigned long mask = IORESOURCE_MEM_64 | 
>> IORESOURCE_MEM;
>> +
>> +            /* Look for MMIO32 */
>> +            if ((pci->phb->mem_resources[i].flags & mask) == 
>> IORESOURCE_MEM)
>> +                break;
>> +        }
>> +
>> +        if (i == ARRAY_SIZE(pci->phb->mem_resources))
>> +            goto out_del_list;
> 
> 
> So we exit and do nothing if there's no MMIO32 bar?
> Isn't the intent just to figure out the MMIO32 area to reserve it when 
> init'ing the table? In which case we could default to 0,0
> 
> I'm actually not clear why we are reserving this area on pseries.



If we do not reserve it, then the iommu code will allocate DMA pages 
from there and these addresses are MMIO32 from the kernel pov at least. 
I saw crashes when (I think) a device tried DMAing to the top 2GB of the 
bus space which happened to be a some other device's BAR.


> 
> 
>> +
>> +        /* New table for using DDW instead of the default DMA window */
>> +        newtbl = iommu_pseries_alloc_table(pci->phb->node);
>> +        if (!newtbl) {
>> +            dev_dbg(&dev->dev, "couldn't create new IOMMU table\n");
>> +            goto out_del_list;
>> +        }
>> +
>> +        iommu_table_setparms_common(newtbl, pci->phb->bus->number, 
>> create.liobn, win_addr,
>> +                        1UL << len, page_shift, NULL, 
>> &iommu_table_lpar_multi_ops);
>> +        iommu_init_table(newtbl, pci->phb->node, 
>> pci->phb->mem_resources[i].start,
>> +                 pci->phb->mem_resources[i].end);
>> +
>> +        pci->table_group->tables[1] = newtbl;
>> +
>> +        /* Keep default DMA window stuct if removed */
>> +        if (default_win_removed) {
>> +            tbl->it_size = 0;
>> +            kfree(tbl->it_map);
>> +        }
>> +
>> +        set_iommu_table_base(&dev->dev, newtbl);
>>       }
>>       spin_lock(&direct_window_list_lock);
> 
> 
> 
> 
> Somewhere around here, we have:
> 
> out_remove_win:
>      remove_ddw(pdn, true, DIRECT64_PROPNAME);
> 
> We should replace with:
>      remove_ddw(pdn, true, win_name);


True. Good spotting. Or rework remove_dma_window() to take just a liobn. 
Thanks,

> 
> 
>    Fred
> 
> 
> 
>> @@ -1408,10 +1463,10 @@ static bool enable_ddw(struct pci_dev *dev, 
>> struct device_node *pdn)
>>        * as RAM, then we failed to create a window to cover persistent
>>        * memory and need to set the DMA limit.
>>        */
>> -    if (pmem_present && ddw_enabled && (len == max_ram_len))
>> +    if (pmem_present && ddw_enabled && direct_mapping && len == 
>> max_ram_len)
>>           dev->dev.bus_dma_limit = dev->dev.archdata.dma_offset + 
>> (1ULL << len);
>> -    return ddw_enabled;
>> +    return ddw_enabled && direct_mapping;
>>   }
>>   static void pci_dma_dev_setup_pSeriesLP(struct pci_dev *dev)
>> @@ -1553,7 +1608,9 @@ static int iommu_reconfig_notifier(struct 
>> notifier_block *nb, unsigned long acti
>>            * we have to remove the property when releasing
>>            * the device node.
>>            */
>> -        remove_ddw(np, false, DIRECT64_PROPNAME);
>> +        if (remove_ddw(np, false, DIRECT64_PROPNAME))
>> +            remove_ddw(np, false, DMA64_PROPNAME);
>> +
>>           if (pci && pci->table_group)
>>               iommu_pseries_free_group(pci->table_group,
>>                       np->full_name);
>>

-- 
Alexey

^ permalink raw reply

* [Bug 213803] New: G5 kernel build (v5.14-rc2) fails at linking stage - ld: arch/powerpc/mm/pgtable.o: in function `.__ptep_set_access_flags': /usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/pgtable.h:824: undefined reference to `.radix__ptep_set_access_flags'
From: bugzilla-daemon @ 2021-07-20 23:53 UTC (permalink / raw)
  To: linuxppc-dev

https://bugzilla.kernel.org/show_bug.cgi?id=213803

            Bug ID: 213803
           Summary: G5 kernel build (v5.14-rc2) fails at linking stage -
                    ld: arch/powerpc/mm/pgtable.o: in function
                    `.__ptep_set_access_flags':
                    /usr/src/linux-stable/./arch/powerpc/include/asm/book3
                    s/64/pgtable.h:824: undefined reference to
                    `.radix__ptep_set_access_flags'
           Product: Platform Specific/Hardware
           Version: 2.5
    Kernel Version: 5.14-rc2
          Hardware: PPC-64
                OS: Linux
              Tree: Mainline
            Status: NEW
          Severity: normal
          Priority: P1
         Component: PPC-64
          Assignee: platform_ppc-64@kernel-bugs.osdl.org
          Reporter: erhard_f@mailbox.org
        Regression: No

Created attachment 297967
  --> https://bugzilla.kernel.org/attachment.cgi?id=297967&action=edit
kernel .config (5.14-rc2, PowerMac G5 11,2)

[...]
  CHK     include/generated/autoksyms.h
  GEN     .version
  CHK     include/generated/compile.h
  UPD     include/generated/compile.h
  CC      init/version.o
  AR      init/built-in.a
  LD      vmlinux.o
  MODPOST vmlinux.symvers
  MODINFO modules.builtin.modinfo
  GEN     modules.builtin
  LD      .tmp_vmlinux.kallsyms1
ld: arch/powerpc/mm/pgtable.o: in function `.__ptep_set_access_flags':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/pgtable.h:824:
undefined reference to `.radix__ptep_set_access_flags'
ld: arch/powerpc/mm/pageattr.o: in function `.change_page_attr':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/tlbflush.h:80:
undefined reference to `.radix__flush_tlb_kernel_range'
ld: arch/powerpc/mm/pageattr.o: in function `.set_page_attr':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/tlbflush.h:80:
undefined reference to `.radix__flush_tlb_kernel_range'
ld: arch/powerpc/mm/book3s64/mmu_context.o:(.toc+0x0): undefined reference to
`mmu_pid_bits'
ld: arch/powerpc/mm/book3s64/mmu_context.o:(.toc+0x8): undefined reference to
`mmu_base_pid'
ld: arch/powerpc/mm/book3s64/pgtable.o: in function `.pmd_hugepage_update':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/pgtable.h:1197:
undefined reference to `.radix__pmd_hugepage_update'
ld: arch/powerpc/mm/book3s64/pgtable.o: in function `.do_serialize':
/usr/src/linux-stable/arch/powerpc/mm/book3s64/pgtable.c:89: undefined
reference to `.exit_lazy_flush_tlb'
ld: arch/powerpc/mm/book3s64/pgtable.o: in function `.pmdp_set_access_flags':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/pgtable.h:824:
undefined reference to `.radix__ptep_set_access_flags'
ld: arch/powerpc/mm/book3s64/pgtable.o: in function `.pmdp_invalidate':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/tlbflush.h:54:
undefined reference to `.radix__flush_pmd_tlb_range'
ld: arch/powerpc/mm/book3s64/pgtable.o: in function
`.pmdp_huge_get_and_clear_full':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/pgtable.h:1300:
undefined reference to `.radix__pmdp_huge_get_and_clear'
ld: arch/powerpc/mm/book3s64/pgtable.o: in function
`.pmdp_huge_get_and_clear_full':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/tlbflush.h:54:
undefined reference to `.radix__flush_pmd_tlb_range'
ld: arch/powerpc/mm/book3s64/pgtable.o: in function `.mmu_cleanup_all':
/usr/src/linux-stable/arch/powerpc/mm/book3s64/pgtable.c:179: undefined
reference to `.radix__mmu_cleanup_all'
ld: arch/powerpc/mm/book3s64/pgtable.o: in function `.ptep_modify_prot_commit':
/usr/src/linux-stable/arch/powerpc/mm/book3s64/pgtable.c:457: undefined
reference to `.radix__ptep_modify_prot_commit'
ld: arch/powerpc/lib/code-patching.o: in function `.patch_instruction':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/pgtable.h:1080:
undefined reference to `.radix__map_kernel_page'
ld: arch/powerpc/lib/code-patching.o: in function `.patch_instruction':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/tlbflush.h:80:
undefined reference to `.radix__flush_tlb_kernel_range'
ld: kernel/fork.o: in function `.dup_mm':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/tlbflush.h:117:
undefined reference to `.radix__flush_tlb_mm'
ld: mm/memory.o: in function `.unmap_page_range':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/tlbflush.h:109:
undefined reference to `.radix__tlb_flush'
ld: mm/memory.o: in function `.do_wp_page':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/tlbflush.h:125:
undefined reference to `.radix__flush_tlb_page'
ld: mm/memory.o: in function `.do_set_pmd':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/pgtable.h:1323:
undefined reference to `.radix__pgtable_trans_huge_deposit'
ld: mm/memory.o: in function `.__handle_mm_fault':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/tlbflush.h:125:
undefined reference to `.radix__flush_tlb_page'
ld: mm/mprotect.o: in function `.change_protection':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/tlbflush.h:72:
undefined reference to `.radix__flush_tlb_range'
ld: mm/mremap.o: in function `.flush_tlb_range':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/tlbflush.h:72:
undefined reference to `.radix__flush_tlb_range'
ld: mm/pgtable-generic.o: in function `.ptep_clear_flush':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/tlbflush.h:125:
undefined reference to `.radix__flush_tlb_page'
ld: mm/pgtable-generic.o: in function `.pmdp_huge_clear_flush':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/pgtable.h:1300:
undefined reference to `.radix__pmdp_huge_get_and_clear'
ld: mm/pgtable-generic.o: in function `.pmdp_huge_clear_flush':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/tlbflush.h:54:
undefined reference to `.radix__flush_pmd_tlb_range'
ld: mm/rmap.o: in function `.try_to_unmap_one':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/tlbflush.h:72:
undefined reference to `.radix__flush_tlb_range'
ld: mm/rmap.o: in function `.try_to_migrate_one':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/tlbflush.h:72:
undefined reference to `.radix__flush_tlb_range'
ld: mm/vmalloc.o: in function `.flush_tlb_kernel_range':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/tlbflush.h:80:
undefined reference to `.radix__flush_tlb_kernel_range'
ld: mm/hugetlb.o: in function `.hugetlb_cow':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/hugetlb.h:91:
undefined reference to `.radix__flush_hugetlb_page'
ld: mm/hugetlb.o: in function `.hugetlb_change_protection':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/tlbflush.h:64:
undefined reference to `.radix__flush_hugetlb_tlb_range'
ld: mm/hugetlb.o: in function `.hugetlb_unshare_all_pmds':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/tlbflush.h:64:
undefined reference to `.radix__flush_hugetlb_tlb_range'
ld: mm/huge_memory.o: in function `.pgtable_trans_huge_deposit':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/pgtable.h:1323:
undefined reference to `.radix__pgtable_trans_huge_deposit'
ld: mm/huge_memory.o: in function `.pgtable_trans_huge_withdraw':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/pgtable.h:1332:
undefined reference to `.radix__pgtable_trans_huge_withdraw'
ld: mm/huge_memory.o: in function `.pmd_hugepage_update.isra.0':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/pgtable.h:1197:
undefined reference to `.radix__pmd_hugepage_update'
ld: mm/huge_memory.o: in function `.do_huge_pmd_numa_page':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/tlbflush.h:72:
undefined reference to `.radix__flush_tlb_range'
ld: mm/huge_memory.o: in function `.move_huge_pmd':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/pgtable.h:1300:
undefined reference to `.radix__pmdp_huge_get_and_clear'
ld: mm/huge_memory.o: in function `.move_huge_pmd':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/tlbflush.h:72:
undefined reference to `.radix__flush_tlb_range'
ld: mm/khugepaged.o: in function `.pmdp_collapse_flush':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/pgtable.h:1308:
undefined reference to `.radix__pmdp_collapse_flush'
ld: mm/khugepaged.o: in function `.khugepaged':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/pgtable.h:1323:
undefined reference to `.radix__pgtable_trans_huge_deposit'
ld: fs/proc/task_mmu.o: in function `.clear_refs_write':
/usr/src/linux-stable/./arch/powerpc/include/asm/book3s/64/tlbflush.h:117:
undefined reference to `.radix__flush_tlb_mm'
ld: warning: creating DT_TEXTREL in a PIE

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

^ permalink raw reply

* Re: [PATCH v4 5/5] bus: Make remove callback return void
From: Wolfram Sang @ 2021-07-20 20:25 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: nvdimm, linux-sh, Samuel Iglesias Gonsalvez, Jens Taprogge,
	Ulf Hansson, Jaroslav Kysela, linux-fpga, Benjamin Tissoires,
	Paul Mackerras, Srinivas Pandruvada, K. Y. Srinivasan,
	Mike Christie, Wei Liu, Maxim Levitsky, Samuel Holland,
	Rafael J . Wysocki, linux-acpi, Geert Uytterhoeven, linux-pci,
	xen-devel, Tomas Winkler, Julien Grall, Ohad Ben-Cohen, Yufen Yu,
	Alex Williamson, Alex Elder, linux-parisc, Finn Thain,
	Geoff Levand, Greg Kroah-Hartman, linux-usb, Rafael J. Wysocki,
	linux-kernel, linux-spi, Kai-Heng Feng, kernel, Jon Mason,
	linux-ntb, Wu Hao, David Woodhouse, Krzysztof Wilczyński,
	Alexandre Belloni, Manohar Vanga, linux-wireless,
	Dominik Brodowski, virtualization, James E.J. Bottomley,
	target-devel, Srinivas Kandagatla, linux-i2c, linux-s390,
	Stefano Stabellini, Stephen Hemminger, Yoshinori Sato, Jiri Slaby,
	Helge Deller, Rafał Miłecki, YueHaibing,
	industrypack-devel, linux-mips, Len Brown, Vasily Gorbik,
	linux-arm-msm, linux-media, Maxime Ripard, William Breathitt Gray,
	greybus-dev, linux-m68k, Florian Fainelli, Rikard Falkeborn,
	Frank Li, Mark Gross, Boris Ostrovsky, Mika Westerberg,
	linux-arm-kernel, Johannes Thumshirn, Mathieu Poirier,
	Stephen Boyd, Cornelia Huck, Peter Oberparleiter, Joey Pabalan,
	Yehezkel Bernat, Pali Rohár, Bodo Stroesser,
	Alison Schofield, Heikki Krogerus, Tyrel Datwyler,
	Alexander Shishkin, Tom Rix, Jason Wang, SeongJae Park,
	alsa-devel, platform-driver-x86, Bjorn Helgaas, netdev,
	Vineeth Vijayan, Ira Weiny, Rob Herring, Lorenzo Pieralisi,
	Dave Jiang, linux-staging, Dexuan Cui, Jernej Skrabec,
	Kishon Vijay Abraham I, Christian Borntraeger, Chen-Yu Tsai,
	linux-input, Allen Hubbe, Alex Dubov, Haiyang Zhang, Jiri Kosina,
	Russell King (Oracle), Ben Widawsky, Harald Freudenberger,
	linux-cxl, Michael Buesch, Dan Williams, Mauro Carvalho Chehab,
	Cristian Marussi, Thomas Bogendoerfer, Martin K. Petersen,
	Martyn Welch, Dmitry Torokhov, linux-mmc, linux-sunxi,
	Stefan Richter, Sudeep Holla, David S. Miller, Sven Van Asbroeck,
	Rich Felker, kvm, Michael S. Tsirkin, linux-remoteproc,
	Bjorn Andersson, sparclinux, Kirti Wankhede, Andreas Noever,
	linux-i3c, linux1394-devel, Lee Jones, Arnd Bergmann, linux-scsi,
	Marc Zyngier, Russell King, Thorsten Scherer, Andy Gross,
	linux-serial, Jakub Kicinski, linux-hyperv, Michael Jamet,
	Heiko Carstens, Johan Hovold, Hans de Goede, Hannes Reinecke,
	Juergen Gross, linuxppc-dev, Takashi Iwai, Alexandre Bounine,
	Vinod Koul, Mark Brown, Vishal Verma, dmaengine, Moritz Fischer,
	Johannes Berg, Johannes Thumshirn, Maximilian Luz
In-Reply-To: <20210713193522.1770306-6-u.kleine-koenig@pengutronix.de>

[-- Attachment #1: Type: text/plain, Size: 3098 bytes --]

On Tue, Jul 13, 2021 at 09:35:22PM +0200, Uwe Kleine-König wrote:
> The driver core ignores the return value of this callback because there
> is only little it can do when a device disappears.
> 
> This is the final bit of a long lasting cleanup quest where several
> buses were converted to also return void from their remove callback.
> Additionally some resource leaks were fixed that were caused by drivers
> returning an error code in the expectation that the driver won't go
> away.
> 
> With struct bus_type::remove returning void it's prevented that newly
> implemented buses return an ignored error code and so don't anticipate
> wrong expectations for driver authors.
> 
> Acked-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> (For ARM, Amba and related parts)
> Acked-by: Mark Brown <broonie@kernel.org>
> Acked-by: Chen-Yu Tsai <wens@csie.org> (for sunxi-rsb)
> Acked-by: Pali Rohár <pali@kernel.org>
> Acked-by: Mauro Carvalho Chehab <mchehab@kernel.org> (for media)
> Acked-by: Hans de Goede <hdegoede@redhat.com> (For drivers/platform)
> Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Acked-By: Vinod Koul <vkoul@kernel.org>
> Acked-by: Juergen Gross <jgross@suse.com> (For xen)
> Acked-by: Lee Jones <lee.jones@linaro.org> (For mfd)
> Acked-by: Johannes Thumshirn <jth@kernel.org> (For mcb)
> Acked-by: Johan Hovold <johan@kernel.org>
> Acked-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> (For slimbus)
> Acked-by: Kirti Wankhede <kwankhede@nvidia.com> (For vfio)
> Acked-by: Maximilian Luz <luzmaximilian@gmail.com>
> Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> (For ulpi and typec)
> Acked-by: Samuel Iglesias Gonsálvez <siglesias@igalia.com> (For ipack)
> Reviewed-by: Tom Rix <trix@redhat.com> (For fpga)
> Acked-by: Geoff Levand <geoff@infradead.org> (For ps3)
> Acked-by: Yehezkel Bernat <YehezkelShB@gmail.com> (For thunderbolt)
> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> Acked-by: Alexander Shishkin <alexander.shishkin@linux.intel.com> (For intel_th)
> Acked-by: Dominik Brodowski <linux@dominikbrodowski.net> (For pcmcia)
> Reviewed-by: Cornelia Huck <cohuck@redhat.com> (For drivers/s390 and drivers/vfio)
> Acked-by: Rafael J. Wysocki <rafael@kernel.org> (For ACPI)
> Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org> (rpmsg and apr)
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> (For intel-ish-hid)
> Acked-by: Dan Williams <dan.j.williams@intel.com> (For CXL, DAX, and NVDIMM)
> Acked-by: William Breathitt Gray <vilhelm.gray@gmail.com> (For isa)
> Acked-by: Stefan Richter <stefanr@s5r6.in-berlin.de> (For firewire)
> Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> (For hid)
> Acked-by: Thorsten Scherer <t.scherer@eckelmann.de> (For siox)
> Acked-by: Sven Van Asbroeck <TheSven73@gmail.com> (For anybuss)
> Acked-by: Ulf Hansson <ulf.hansson@linaro.org> (For MMC)
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Acked-by: Wolfram Sang <wsa@kernel.org> # for I2C

Thanks, Uwe!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH 0/2] Fix arm64 boot regression in 5.14
From: Christophe Leroy @ 2021-07-20 18:26 UTC (permalink / raw)
  To: Will Deacon
  Cc: Mark Rutland, Jonathan Marek, Marc Zyngier, linux-kernel,
	Ard Biesheuvel, Geert Uytterhoeven, linux-arm-kernel,
	Catalin Marinas, Paul Mackerras, Andrew Morton, linuxppc-dev,
	Thomas Gleixner, Mike Rapoport
In-Reply-To: <20210720123512.8740-1-will@kernel.org>

Will Deacon <will@kernel.org> a écrit :

> Hi folks,
>
> Jonathan reports [1] that commit c742199a014d ("mm/pgtable: add stubs
> for {pmd/pub}_{set/clear}_huge") breaks the boot on arm64 when huge
> mappings are used to map the kernel linear map but the VA size is
> configured such that PUDs are folded. This is because the non-functional
> pud_set_huge() stub is used to create the linear map, which results in
> 1GB holes and a fatal data abort when the kernel attemps to access them.
>
> Digging further into the issue, it also transpired that huge-vmap is
> silently disabled in these configurations as well [2], despite working
> correctly in 5.13. The latter issue causes the pgtable selftests to
> scream due to a failing consistency check [3].
>
> Rather than leave mainline in a terminally broken state for arm64 while
> we figure this out, revert the offending commit to get things working
> again. Unfortunately, reverting the change in isolation causes a build
> breakage for 32-bit PowerPC 8xx machines which recently started relying
> on the problematic stubs to support pte-level huge-vmap entries [4].
> Since Christophe is away at the moment, this series first reverts the
> PowerPC 8xx change in order to avoid breaking the build.
>
> I would really like this to land for -rc3 and I can take these via the
> arm64 fixes queue if the PowerPC folks are alright with them.
>

If you can drop patch 1,

Change patch 2 to add the two following functions in  
arch/powerpc/mm/nohash/8xx.c :

int pud_clear_huge(pud_t *pud)
{
         return 0;
}

int pmd_clear_huge(pmd_t *pmd)
{
         return 0;
}

Then feel free to take it via ARM fixes with my acked-by as maintainer  
of PPC8XX.

Christophe


> Cheers,
>
> Will
>
> [1] https://lore.kernel.org/r/20210717160118.9855-1-jonathan@marek.ca
> [2] https://lore.kernel.org/r/20210719104918.GA6440@willie-the-truck
> [3]  
> https://lore.kernel.org/r/CAMuHMdXShORDox-xxaeUfDW3wx2PeggFSqhVSHVZNKCGK-y_vQ@mail.gmail.com/
> [4]  
> https://lore.kernel.org/r/8b972f1c03fb6bd59953035f0a3e4d26659de4f8.1620795204.git.christophe.leroy@csgroup.eu/
>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Jonathan Marek <jonathan@marek.ca>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Nicholas Piggin <npiggin@gmail.com
> Cc: Mike Rapoport <rppt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-arm-kernel@lists.infradead.org
>
> --->8
>
> Jonathan Marek (1):
>   Revert "mm/pgtable: add stubs for {pmd/pub}_{set/clear}_huge"
>
> Will Deacon (1):
>   Revert "powerpc/8xx: add support for huge pages on VMAP and VMALLOC"
>
>  arch/arm64/mm/mmu.c                          | 20 ++++-----
>  arch/powerpc/Kconfig                         |  2 +-
>  arch/powerpc/include/asm/nohash/32/mmu-8xx.h | 43 --------------------
>  arch/x86/mm/pgtable.c                        | 34 +++++++---------
>  include/linux/pgtable.h                      | 26 +-----------
>  5 files changed, 25 insertions(+), 100 deletions(-)
>
> --
> 2.32.0.402.g57bb445576-goog



^ permalink raw reply

* Re: [PATCH v5 11/11] powerpc/pseries/iommu: Rename "direct window" to "dma window"
From: Frederic Barrat @ 2021-07-20 18:12 UTC (permalink / raw)
  To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
	Paul Mackerras, Alexey Kardashevskiy, David Gibson,
	kernel test robot, Nicolin Chen
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20210716082755.428187-12-leobras.c@gmail.com>



On 16/07/2021 10:27, Leonardo Bras wrote:
> A previous change introduced the usage of DDW as a bigger indirect DMA
> mapping when the DDW available size does not map the whole partition.
> 
> As most of the code that manipulates direct mappings was reused for
> indirect mappings, it's necessary to rename all names and debug/info
> messages to reflect that it can be used for both kinds of mapping.
> 
> This should cause no behavioural change, just adjust naming.
> 
> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> ---


LGTM:
Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com>



>   arch/powerpc/platforms/pseries/iommu.c | 91 +++++++++++++-------------
>   1 file changed, 47 insertions(+), 44 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> index a67e71c49aeb..52548dfb8b45 100644
> --- a/arch/powerpc/platforms/pseries/iommu.c
> +++ b/arch/powerpc/platforms/pseries/iommu.c
> @@ -349,7 +349,7 @@ struct dynamic_dma_window_prop {
>   	__be32	window_shift;	/* ilog2(tce_window_size) */
>   };
>   
> -struct direct_window {
> +struct dma_win {
>   	struct device_node *device;
>   	const struct dynamic_dma_window_prop *prop;
>   	struct list_head list;
> @@ -369,11 +369,11 @@ struct ddw_create_response {
>   	u32 addr_lo;
>   };
>   
> -static LIST_HEAD(direct_window_list);
> +static LIST_HEAD(dma_win_list);
>   /* prevents races between memory on/offline and window creation */
> -static DEFINE_SPINLOCK(direct_window_list_lock);
> +static DEFINE_SPINLOCK(dma_win_list_lock);
>   /* protects initializing window twice for same device */
> -static DEFINE_MUTEX(direct_window_init_mutex);
> +static DEFINE_MUTEX(dma_win_init_mutex);
>   #define DIRECT64_PROPNAME "linux,direct64-ddr-window-info"
>   #define DMA64_PROPNAME "linux,dma64-ddr-window-info"
>   
> @@ -713,7 +713,10 @@ static void pci_dma_bus_setup_pSeriesLP(struct pci_bus *bus)
>   	pr_debug("pci_dma_bus_setup_pSeriesLP: setting up bus %pOF\n",
>   		 dn);
>   
> -	/* Find nearest ibm,dma-window, walking up the device tree */
> +	/*
> +	 * Find nearest ibm,dma-window (default DMA window), walking up the
> +	 * device tree
> +	 */
>   	for (pdn = dn; pdn != NULL; pdn = pdn->parent) {
>   		dma_window = of_get_property(pdn, "ibm,dma-window", NULL);
>   		if (dma_window != NULL)
> @@ -822,11 +825,11 @@ static void remove_dma_window(struct device_node *np, u32 *ddw_avail,
>   
>   	ret = rtas_call(ddw_avail[DDW_REMOVE_PE_DMA_WIN], 1, 1, NULL, liobn);
>   	if (ret)
> -		pr_warn("%pOF: failed to remove direct window: rtas returned "
> +		pr_warn("%pOF: failed to remove DMA window: rtas returned "
>   			"%d to ibm,remove-pe-dma-window(%x) %llx\n",
>   			np, ret, ddw_avail[DDW_REMOVE_PE_DMA_WIN], liobn);
>   	else
> -		pr_debug("%pOF: successfully removed direct window: rtas returned "
> +		pr_debug("%pOF: successfully removed DMA window: rtas returned "
>   			"%d to ibm,remove-pe-dma-window(%x) %llx\n",
>   			np, ret, ddw_avail[DDW_REMOVE_PE_DMA_WIN], liobn);
>   }
> @@ -854,37 +857,37 @@ static int remove_ddw(struct device_node *np, bool remove_prop, const char *win_
>   
>   	ret = of_remove_property(np, win);
>   	if (ret)
> -		pr_warn("%pOF: failed to remove direct window property: %d\n",
> +		pr_warn("%pOF: failed to remove DMA window property: %d\n",
>   			np, ret);
>   	return 0;
>   }
>   
>   static bool find_existing_ddw(struct device_node *pdn, u64 *dma_addr, int *window_shift)
>   {
> -	struct direct_window *window;
> -	const struct dynamic_dma_window_prop *direct64;
> +	struct dma_win *window;
> +	const struct dynamic_dma_window_prop *dma64;
>   	bool found = false;
>   
> -	spin_lock(&direct_window_list_lock);
> +	spin_lock(&dma_win_list_lock);
>   	/* check if we already created a window and dupe that config if so */
> -	list_for_each_entry(window, &direct_window_list, list) {
> +	list_for_each_entry(window, &dma_win_list, list) {
>   		if (window->device == pdn) {
> -			direct64 = window->prop;
> -			*dma_addr = be64_to_cpu(direct64->dma_base);
> -			*window_shift = be32_to_cpu(direct64->window_shift);
> +			dma64 = window->prop;
> +			*dma_addr = be64_to_cpu(dma64->dma_base);
> +			*window_shift = be32_to_cpu(dma64->window_shift);
>   			found = true;
>   			break;
>   		}
>   	}
> -	spin_unlock(&direct_window_list_lock);
> +	spin_unlock(&dma_win_list_lock);
>   
>   	return found;
>   }
>   
> -static struct direct_window *ddw_list_new_entry(struct device_node *pdn,
> -						const struct dynamic_dma_window_prop *dma64)
> +static struct dma_win *ddw_list_new_entry(struct device_node *pdn,
> +					  const struct dynamic_dma_window_prop *dma64)
>   {
> -	struct direct_window *window;
> +	struct dma_win *window;
>   
>   	window = kzalloc(sizeof(*window), GFP_KERNEL);
>   	if (!window)
> @@ -900,7 +903,7 @@ static void find_existing_ddw_windows_named(const char *name)
>   {
>   	int len;
>   	struct device_node *pdn;
> -	struct direct_window *window;
> +	struct dma_win *window;
>   	const struct dynamic_dma_window_prop *dma64;
>   
>   	for_each_node_with_property(pdn, name) {
> @@ -914,9 +917,9 @@ static void find_existing_ddw_windows_named(const char *name)
>   		if (!window)
>   			break;
>   
> -		spin_lock(&direct_window_list_lock);
> -		list_add(&window->list, &direct_window_list);
> -		spin_unlock(&direct_window_list_lock);
> +		spin_lock(&dma_win_list_lock);
> +		list_add(&window->list, &dma_win_list);
> +		spin_unlock(&dma_win_list_lock);
>   	}
>   }
>   
> @@ -1216,7 +1219,7 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>   	const char *win_name;
>   	struct device_node *dn;
>   	u32 ddw_avail[DDW_APPLICABLE_SIZE];
> -	struct direct_window *window;
> +	struct dma_win *window;
>   	struct property *win64;
>   	bool ddw_enabled = false;
>   	struct failed_ddw_pdn *fpdn;
> @@ -1229,7 +1232,7 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>   	pmem_present = dn != NULL;
>   	of_node_put(dn);
>   
> -	mutex_lock(&direct_window_init_mutex);
> +	mutex_lock(&dma_win_init_mutex);
>   
>   	if (find_existing_ddw(pdn, &dev->dev.archdata.dma_offset, &len)) {
>   		direct_mapping = (len >= max_ram_len);
> @@ -1309,8 +1312,8 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>   
>   	page_shift = iommu_get_page_shift(query.page_size);
>   	if (!page_shift) {
> -		dev_dbg(&dev->dev, "no supported direct page size in mask %x",
> -			  query.page_size);
> +		dev_dbg(&dev->dev, "no supported page size in mask %x",
> +			query.page_size);
>   		goto out_failed;
>   	}
>   
> @@ -1367,7 +1370,7 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>   
>   	ret = of_add_property(pdn, win64);
>   	if (ret) {
> -		dev_err(&dev->dev, "unable to add dma window property for %pOF: %d",
> +		dev_err(&dev->dev, "unable to add DMA window property for %pOF: %d",
>   			pdn, ret);
>   		goto out_free_prop;
>   	}
> @@ -1381,7 +1384,7 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>   		ret = walk_system_ram_range(0, memblock_end_of_DRAM() >> PAGE_SHIFT,
>   					    win64->value, tce_setrange_multi_pSeriesLP_walk);
>   		if (ret) {
> -			dev_info(&dev->dev, "failed to map direct window for %pOF: %d\n",
> +			dev_info(&dev->dev, "failed to map DMA window for %pOF: %d\n",
>   				 dn, ret);
>   			goto out_del_list;
>   		}
> @@ -1423,9 +1426,9 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>   		set_iommu_table_base(&dev->dev, newtbl);
>   	}
>   
> -	spin_lock(&direct_window_list_lock);
> -	list_add(&window->list, &direct_window_list);
> -	spin_unlock(&direct_window_list_lock);
> +	spin_lock(&dma_win_list_lock);
> +	list_add(&window->list, &dma_win_list);
> +	spin_unlock(&dma_win_list_lock);
>   
>   	dev->dev.archdata.dma_offset = win_addr;
>   	ddw_enabled = true;
> @@ -1456,7 +1459,7 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>   	list_add(&fpdn->list, &failed_ddw_pdn_list);
>   
>   out_unlock:
> -	mutex_unlock(&direct_window_init_mutex);
> +	mutex_unlock(&dma_win_init_mutex);
>   
>   	/*
>   	 * If we have persistent memory and the window size is only as big
> @@ -1554,29 +1557,29 @@ static bool iommu_bypass_supported_pSeriesLP(struct pci_dev *pdev, u64 dma_mask)
>   static int iommu_mem_notifier(struct notifier_block *nb, unsigned long action,
>   		void *data)
>   {
> -	struct direct_window *window;
> +	struct dma_win *window;
>   	struct memory_notify *arg = data;
>   	int ret = 0;
>   
>   	switch (action) {
>   	case MEM_GOING_ONLINE:
> -		spin_lock(&direct_window_list_lock);
> -		list_for_each_entry(window, &direct_window_list, list) {
> +		spin_lock(&dma_win_list_lock);
> +		list_for_each_entry(window, &dma_win_list, list) {
>   			ret |= tce_setrange_multi_pSeriesLP(arg->start_pfn,
>   					arg->nr_pages, window->prop);
>   			/* XXX log error */
>   		}
> -		spin_unlock(&direct_window_list_lock);
> +		spin_unlock(&dma_win_list_lock);
>   		break;
>   	case MEM_CANCEL_ONLINE:
>   	case MEM_OFFLINE:
> -		spin_lock(&direct_window_list_lock);
> -		list_for_each_entry(window, &direct_window_list, list) {
> +		spin_lock(&dma_win_list_lock);
> +		list_for_each_entry(window, &dma_win_list, list) {
>   			ret |= tce_clearrange_multi_pSeriesLP(arg->start_pfn,
>   					arg->nr_pages, window->prop);
>   			/* XXX log error */
>   		}
> -		spin_unlock(&direct_window_list_lock);
> +		spin_unlock(&dma_win_list_lock);
>   		break;
>   	default:
>   		break;
> @@ -1597,7 +1600,7 @@ static int iommu_reconfig_notifier(struct notifier_block *nb, unsigned long acti
>   	struct of_reconfig_data *rd = data;
>   	struct device_node *np = rd->dn;
>   	struct pci_dn *pci = PCI_DN(np);
> -	struct direct_window *window;
> +	struct dma_win *window;
>   
>   	switch (action) {
>   	case OF_RECONFIG_DETACH_NODE:
> @@ -1615,15 +1618,15 @@ static int iommu_reconfig_notifier(struct notifier_block *nb, unsigned long acti
>   			iommu_pseries_free_group(pci->table_group,
>   					np->full_name);
>   
> -		spin_lock(&direct_window_list_lock);
> -		list_for_each_entry(window, &direct_window_list, list) {
> +		spin_lock(&dma_win_list_lock);
> +		list_for_each_entry(window, &dma_win_list, list) {
>   			if (window->device == np) {
>   				list_del(&window->list);
>   				kfree(window);
>   				break;
>   			}
>   		}
> -		spin_unlock(&direct_window_list_lock);
> +		spin_unlock(&dma_win_list_lock);
>   		break;
>   	default:
>   		err = NOTIFY_DONE;
> 

^ permalink raw reply

* Re: [PATCH v5 10/11] powerpc/pseries/iommu: Make use of DDW for indirect mapping
From: Frederic Barrat @ 2021-07-20 18:12 UTC (permalink / raw)
  To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
	Paul Mackerras, Alexey Kardashevskiy, David Gibson,
	kernel test robot, Nicolin Chen
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20210716082755.428187-11-leobras.c@gmail.com>



On 16/07/2021 10:27, Leonardo Bras wrote:
> So far it's assumed possible to map the guest RAM 1:1 to the bus, which
> works with a small number of devices. SRIOV changes it as the user can
> configure hundreds VFs and since phyp preallocates TCEs and does not
> allow IOMMU pages bigger than 64K, it has to limit the number of TCEs
> per a PE to limit waste of physical pages.
> 
> As of today, if the assumed direct mapping is not possible, DDW creation
> is skipped and the default DMA window "ibm,dma-window" is used instead.
> 
> By using DDW, indirect mapping  can get more TCEs than available for the
> default DMA window, and also get access to using much larger pagesizes
> (16MB as implemented in qemu vs 4k from default DMA window), causing a
> significant increase on the maximum amount of memory that can be IOMMU
> mapped at the same time.
> 
> Indirect mapping will only be used if direct mapping is not a
> possibility.
> 
> For indirect mapping, it's necessary to re-create the iommu_table with
> the new DMA window parameters, so iommu_alloc() can use it.
> 
> Removing the default DMA window for using DDW with indirect mapping
> is only allowed if there is no current IOMMU memory allocated in
> the iommu_table. enable_ddw() is aborted otherwise.
> 
> Even though there won't be both direct and indirect mappings at the
> same time, we can't reuse the DIRECT64_PROPNAME property name, or else
> an older kexec()ed kernel can assume direct mapping, and skip
> iommu_alloc(), causing undesirable behavior.
> So a new property name DMA64_PROPNAME "linux,dma64-ddr-window-info"
> was created to represent a DDW that does not allow direct mapping.
> 
> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> ---
>   arch/powerpc/platforms/pseries/iommu.c | 87 +++++++++++++++++++++-----
>   1 file changed, 72 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> index 22d251e15b61..a67e71c49aeb 100644
> --- a/arch/powerpc/platforms/pseries/iommu.c
> +++ b/arch/powerpc/platforms/pseries/iommu.c
> @@ -375,6 +375,7 @@ static DEFINE_SPINLOCK(direct_window_list_lock);
>   /* protects initializing window twice for same device */
>   static DEFINE_MUTEX(direct_window_init_mutex);
>   #define DIRECT64_PROPNAME "linux,direct64-ddr-window-info"
> +#define DMA64_PROPNAME "linux,dma64-ddr-window-info"
>   
>   static int tce_clearrange_multi_pSeriesLP(unsigned long start_pfn,
>   					unsigned long num_pfn, const void *arg)
> @@ -925,6 +926,7 @@ static int find_existing_ddw_windows(void)
>   		return 0;
>   
>   	find_existing_ddw_windows_named(DIRECT64_PROPNAME);
> +	find_existing_ddw_windows_named(DMA64_PROPNAME);
>   
>   	return 0;
>   }
> @@ -1211,14 +1213,17 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>   	struct ddw_create_response create;
>   	int page_shift;
>   	u64 win_addr;
> +	const char *win_name;
>   	struct device_node *dn;
>   	u32 ddw_avail[DDW_APPLICABLE_SIZE];
>   	struct direct_window *window;
>   	struct property *win64;
>   	bool ddw_enabled = false;
>   	struct failed_ddw_pdn *fpdn;
> -	bool default_win_removed = false;
> +	bool default_win_removed = false, direct_mapping = false;
>   	bool pmem_present;
> +	struct pci_dn *pci = PCI_DN(pdn);
> +	struct iommu_table *tbl = pci->table_group->tables[0];
>   
>   	dn = of_find_node_by_type(NULL, "ibm,pmemory");
>   	pmem_present = dn != NULL;
> @@ -1227,6 +1232,7 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>   	mutex_lock(&direct_window_init_mutex);
>   
>   	if (find_existing_ddw(pdn, &dev->dev.archdata.dma_offset, &len)) {
> +		direct_mapping = (len >= max_ram_len);
>   		ddw_enabled = true;
>   		goto out_unlock;
>   	}
> @@ -1307,8 +1313,7 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>   			  query.page_size);
>   		goto out_failed;
>   	}
> -	/* verify the window * number of ptes will map the partition */
> -	/* check largest block * page size > max memory hotplug addr */
> +
>   	/*
>   	 * The "ibm,pmemory" can appear anywhere in the address space.
>   	 * Assuming it is still backed by page structs, try MAX_PHYSMEM_BITS
> @@ -1324,13 +1329,25 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>   			dev_info(&dev->dev, "Skipping ibm,pmemory");
>   	}
>   
> +	/* check if the available block * number of ptes will map everything */
>   	if (query.largest_available_block < (1ULL << (len - page_shift))) {
>   		dev_dbg(&dev->dev,
>   			"can't map partition max 0x%llx with %llu %llu-sized pages\n",
>   			1ULL << len,
>   			query.largest_available_block,
>   			1ULL << page_shift);
> -		goto out_failed;
> +
> +		/* DDW + IOMMU on single window may fail if there is any allocation */
> +		if (default_win_removed && iommu_table_in_use(tbl)) {
> +			dev_dbg(&dev->dev, "current IOMMU table in use, can't be replaced.\n");
> +			goto out_failed;
> +		}
> +
> +		len = order_base_2(query.largest_available_block << page_shift);
> +		win_name = DMA64_PROPNAME;
> +	} else {
> +		direct_mapping = true;
> +		win_name = DIRECT64_PROPNAME;
>   	}
>   
>   	ret = create_ddw(dev, ddw_avail, &create, page_shift, len);
> @@ -1341,8 +1358,7 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>   		  create.liobn, dn);
>   
>   	win_addr = ((u64)create.addr_hi << 32) | create.addr_lo;
> -	win64 = ddw_property_create(DIRECT64_PROPNAME, create.liobn, win_addr,
> -				    page_shift, len);
> +	win64 = ddw_property_create(win_name, create.liobn, win_addr, page_shift, len);
>   	if (!win64) {
>   		dev_info(&dev->dev,
>   			 "couldn't allocate property, property name, or value\n");
> @@ -1360,12 +1376,51 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>   	if (!window)
>   		goto out_del_prop;
>   
> -	ret = walk_system_ram_range(0, memblock_end_of_DRAM() >> PAGE_SHIFT,
> -			win64->value, tce_setrange_multi_pSeriesLP_walk);
> -	if (ret) {
> -		dev_info(&dev->dev, "failed to map direct window for %pOF: %d\n",
> -			 dn, ret);
> -		goto out_del_list;
> +	if (direct_mapping) {
> +		/* DDW maps the whole partition, so enable direct DMA mapping */
> +		ret = walk_system_ram_range(0, memblock_end_of_DRAM() >> PAGE_SHIFT,
> +					    win64->value, tce_setrange_multi_pSeriesLP_walk);
> +		if (ret) {
> +			dev_info(&dev->dev, "failed to map direct window for %pOF: %d\n",
> +				 dn, ret);
> +			goto out_del_list;
> +		}
> +	} else {
> +		struct iommu_table *newtbl;
> +		int i;
> +
> +		for (i = 0; i < ARRAY_SIZE(pci->phb->mem_resources); i++) {
> +			const unsigned long mask = IORESOURCE_MEM_64 | IORESOURCE_MEM;
> +
> +			/* Look for MMIO32 */
> +			if ((pci->phb->mem_resources[i].flags & mask) == IORESOURCE_MEM)
> +				break;
> +		}
> +
> +		if (i == ARRAY_SIZE(pci->phb->mem_resources))
> +			goto out_del_list;


So we exit and do nothing if there's no MMIO32 bar?
Isn't the intent just to figure out the MMIO32 area to reserve it when 
init'ing the table? In which case we could default to 0,0

I'm actually not clear why we are reserving this area on pseries.


> +
> +		/* New table for using DDW instead of the default DMA window */
> +		newtbl = iommu_pseries_alloc_table(pci->phb->node);
> +		if (!newtbl) {
> +			dev_dbg(&dev->dev, "couldn't create new IOMMU table\n");
> +			goto out_del_list;
> +		}
> +
> +		iommu_table_setparms_common(newtbl, pci->phb->bus->number, create.liobn, win_addr,
> +					    1UL << len, page_shift, NULL, &iommu_table_lpar_multi_ops);
> +		iommu_init_table(newtbl, pci->phb->node, pci->phb->mem_resources[i].start,
> +				 pci->phb->mem_resources[i].end);
> +
> +		pci->table_group->tables[1] = newtbl;
> +
> +		/* Keep default DMA window stuct if removed */
> +		if (default_win_removed) {
> +			tbl->it_size = 0;
> +			kfree(tbl->it_map);
> +		}
> +
> +		set_iommu_table_base(&dev->dev, newtbl);
>   	}
>   
>   	spin_lock(&direct_window_list_lock);




Somewhere around here, we have:

out_remove_win:
	remove_ddw(pdn, true, DIRECT64_PROPNAME);

We should replace with:
	remove_ddw(pdn, true, win_name);


   Fred



> @@ -1408,10 +1463,10 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>   	 * as RAM, then we failed to create a window to cover persistent
>   	 * memory and need to set the DMA limit.
>   	 */
> -	if (pmem_present && ddw_enabled && (len == max_ram_len))
> +	if (pmem_present && ddw_enabled && direct_mapping && len == max_ram_len)
>   		dev->dev.bus_dma_limit = dev->dev.archdata.dma_offset + (1ULL << len);
>   
> -	return ddw_enabled;
> +    return ddw_enabled && direct_mapping;
>   }
>   
>   static void pci_dma_dev_setup_pSeriesLP(struct pci_dev *dev)
> @@ -1553,7 +1608,9 @@ static int iommu_reconfig_notifier(struct notifier_block *nb, unsigned long acti
>   		 * we have to remove the property when releasing
>   		 * the device node.
>   		 */
> -		remove_ddw(np, false, DIRECT64_PROPNAME);
> +		if (remove_ddw(np, false, DIRECT64_PROPNAME))
> +			remove_ddw(np, false, DMA64_PROPNAME);
> +
>   		if (pci && pci->table_group)
>   			iommu_pseries_free_group(pci->table_group,
>   					np->full_name);
> 

^ permalink raw reply

* Re: [PATCH v5 09/11] powerpc/pseries/iommu: Find existing DDW with given property name
From: Frederic Barrat @ 2021-07-20 17:52 UTC (permalink / raw)
  To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
	Paul Mackerras, Alexey Kardashevskiy, David Gibson,
	kernel test robot, Nicolin Chen
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20210716082755.428187-10-leobras.c@gmail.com>



On 16/07/2021 10:27, Leonardo Bras wrote:
> At the moment pseries stores information about created directly mapped
> DDW window in DIRECT64_PROPNAME.
> 
> With the objective of implementing indirect DMA mapping with DDW, it's
> necessary to have another propriety name to make sure kexec'ing into older
> kernels does not break, as it would if we reuse DIRECT64_PROPNAME.
> 
> In order to have this, find_existing_ddw_windows() needs to be able to
> look for different property names.
> 
> Extract find_existing_ddw_windows() into find_existing_ddw_windows_named()
> and calls it with current property name.
> 
> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---


Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com>




>   arch/powerpc/platforms/pseries/iommu.c | 25 +++++++++++++++----------
>   1 file changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> index 17c6f4706e76..22d251e15b61 100644
> --- a/arch/powerpc/platforms/pseries/iommu.c
> +++ b/arch/powerpc/platforms/pseries/iommu.c
> @@ -895,24 +895,21 @@ static struct direct_window *ddw_list_new_entry(struct device_node *pdn,
>   	return window;
>   }
>   
> -static int find_existing_ddw_windows(void)
> +static void find_existing_ddw_windows_named(const char *name)
>   {
>   	int len;
>   	struct device_node *pdn;
>   	struct direct_window *window;
> -	const struct dynamic_dma_window_prop *direct64;
> -
> -	if (!firmware_has_feature(FW_FEATURE_LPAR))
> -		return 0;
> +	const struct dynamic_dma_window_prop *dma64;
>   
> -	for_each_node_with_property(pdn, DIRECT64_PROPNAME) {
> -		direct64 = of_get_property(pdn, DIRECT64_PROPNAME, &len);
> -		if (!direct64 || len < sizeof(*direct64)) {
> -			remove_ddw(pdn, true, DIRECT64_PROPNAME);
> +	for_each_node_with_property(pdn, name) {
> +		dma64 = of_get_property(pdn, name, &len);
> +		if (!dma64 || len < sizeof(*dma64)) {
> +			remove_ddw(pdn, true, name);
>   			continue;
>   		}
>   
> -		window = ddw_list_new_entry(pdn, direct64);
> +		window = ddw_list_new_entry(pdn, dma64);
>   		if (!window)
>   			break;
>   
> @@ -920,6 +917,14 @@ static int find_existing_ddw_windows(void)
>   		list_add(&window->list, &direct_window_list);
>   		spin_unlock(&direct_window_list_lock);
>   	}
> +}
> +
> +static int find_existing_ddw_windows(void)
> +{
> +	if (!firmware_has_feature(FW_FEATURE_LPAR))
> +		return 0;
> +
> +	find_existing_ddw_windows_named(DIRECT64_PROPNAME);
>   
>   	return 0;
>   }
> 

^ permalink raw reply

* Re: [PATCH v5 08/11] powerpc/pseries/iommu: Update remove_dma_window() to accept property name
From: Frederic Barrat @ 2021-07-20 17:51 UTC (permalink / raw)
  To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
	Paul Mackerras, Alexey Kardashevskiy, David Gibson,
	kernel test robot, Nicolin Chen
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20210716082755.428187-9-leobras.c@gmail.com>



On 16/07/2021 10:27, Leonardo Bras wrote:
> Update remove_dma_window() so it can be used to remove DDW with a given
> property name.
> 
> This enables the creation of new property names for DDW, so we can
> have different usage for it, like indirect mapping.
> 
> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>   arch/powerpc/platforms/pseries/iommu.c | 21 +++++++++++----------
>   1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> index 108c3dcca686..17c6f4706e76 100644
> --- a/arch/powerpc/platforms/pseries/iommu.c
> +++ b/arch/powerpc/platforms/pseries/iommu.c
> @@ -830,31 +830,32 @@ static void remove_dma_window(struct device_node *np, u32 *ddw_avail,
>   			np, ret, ddw_avail[DDW_REMOVE_PE_DMA_WIN], liobn);
>   }
>   
> -static void remove_ddw(struct device_node *np, bool remove_prop)
> +static int remove_ddw(struct device_node *np, bool remove_prop, const char *win_name)
>   {


Why switch to returning an int? None of the callers check it.

   Fred


>   	struct property *win;
>   	u32 ddw_avail[DDW_APPLICABLE_SIZE];
>   	int ret = 0;
>   
> +	win = of_find_property(np, win_name, NULL);
> +	if (!win)
> +		return -EINVAL;
> +
>   	ret = of_property_read_u32_array(np, "ibm,ddw-applicable",
>   					 &ddw_avail[0], DDW_APPLICABLE_SIZE);
>   	if (ret)
> -		return;
> -
> -	win = of_find_property(np, DIRECT64_PROPNAME, NULL);
> -	if (!win)
> -		return;
> +		return 0;
>   
>   	if (win->length >= sizeof(struct dynamic_dma_window_prop))
>   		remove_dma_window(np, ddw_avail, win);
>   
>   	if (!remove_prop)
> -		return;
> +		return 0;
>   
>   	ret = of_remove_property(np, win);
>   	if (ret)
>   		pr_warn("%pOF: failed to remove direct window property: %d\n",
>   			np, ret);
> +	return 0;
>   }
>   
>   static bool find_existing_ddw(struct device_node *pdn, u64 *dma_addr, int *window_shift)
> @@ -907,7 +908,7 @@ static int find_existing_ddw_windows(void)
>   	for_each_node_with_property(pdn, DIRECT64_PROPNAME) {
>   		direct64 = of_get_property(pdn, DIRECT64_PROPNAME, &len);
>   		if (!direct64 || len < sizeof(*direct64)) {
> -			remove_ddw(pdn, true);
> +			remove_ddw(pdn, true, DIRECT64_PROPNAME);
>   			continue;
>   		}
>   
> @@ -1382,7 +1383,7 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>   	kfree(win64);
>   
>   out_remove_win:
> -	remove_ddw(pdn, true);
> +	remove_ddw(pdn, true, DIRECT64_PROPNAME);
>   
>   out_failed:
>   	if (default_win_removed)
> @@ -1547,7 +1548,7 @@ static int iommu_reconfig_notifier(struct notifier_block *nb, unsigned long acti
>   		 * we have to remove the property when releasing
>   		 * the device node.
>   		 */
> -		remove_ddw(np, false);
> +		remove_ddw(np, false, DIRECT64_PROPNAME);
>   		if (pci && pci->table_group)
>   			iommu_pseries_free_group(pci->table_group,
>   					np->full_name);
> 

^ permalink raw reply

* Re: [PATCH v5 07/11] powerpc/pseries/iommu: Reorganize iommu_table_setparms*() with new helper
From: Frederic Barrat @ 2021-07-20 17:50 UTC (permalink / raw)
  To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
	Paul Mackerras, Alexey Kardashevskiy, David Gibson,
	kernel test robot, Nicolin Chen
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20210716082755.428187-8-leobras.c@gmail.com>



On 16/07/2021 10:27, Leonardo Bras wrote:
> Add a new helper _iommu_table_setparms(), and use it in
> iommu_table_setparms() and iommu_table_setparms_lpar() to avoid duplicated
> code.
> 
> Also, setting tbl->it_ops was happening outsite iommu_table_setparms*(),
> so move it to the new helper. Since we need the iommu_table_ops to be
> declared before used, declare iommu_table_lpar_multi_ops and
> iommu_table_pseries_ops to before their respective iommu_table_setparms*().
> 
> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> ---


Looks good
Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com>



>   arch/powerpc/platforms/pseries/iommu.c | 72 ++++++++++++++------------
>   1 file changed, 38 insertions(+), 34 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> index 7ca79a04fa52..108c3dcca686 100644
> --- a/arch/powerpc/platforms/pseries/iommu.c
> +++ b/arch/powerpc/platforms/pseries/iommu.c
> @@ -501,6 +501,24 @@ static int tce_setrange_multi_pSeriesLP_walk(unsigned long start_pfn,
>   	return tce_setrange_multi_pSeriesLP(start_pfn, num_pfn, arg);
>   }
>   
> +static void iommu_table_setparms_common(struct iommu_table *tbl, unsigned long busno,
> +					unsigned long liobn, unsigned long win_addr,
> +					unsigned long window_size, unsigned long page_shift,
> +					void *base, struct iommu_table_ops *table_ops)
> +{
> +	tbl->it_busno = busno;
> +	tbl->it_index = liobn;
> +	tbl->it_offset = win_addr >> page_shift;
> +	tbl->it_size = window_size >> page_shift;
> +	tbl->it_page_shift = page_shift;
> +	tbl->it_base = (unsigned long)base;
> +	tbl->it_blocksize = 16;
> +	tbl->it_type = TCE_PCI;
> +	tbl->it_ops = table_ops;
> +}
> +
> +struct iommu_table_ops iommu_table_pseries_ops;
> +
>   static void iommu_table_setparms(struct pci_controller *phb,
>   				 struct device_node *dn,
>   				 struct iommu_table *tbl)
> @@ -509,8 +527,13 @@ static void iommu_table_setparms(struct pci_controller *phb,
>   	const unsigned long *basep;
>   	const u32 *sizep;
>   
> -	node = phb->dn;
> +	/* Test if we are going over 2GB of DMA space */
> +	if (phb->dma_window_base_cur + phb->dma_window_size > SZ_2G) {
> +		udbg_printf("PCI_DMA: Unexpected number of IOAs under this PHB.\n");
> +		panic("PCI_DMA: Unexpected number of IOAs under this PHB.\n");
> +	}
>   
> +	node = phb->dn;
>   	basep = of_get_property(node, "linux,tce-base", NULL);
>   	sizep = of_get_property(node, "linux,tce-size", NULL);
>   	if (basep == NULL || sizep == NULL) {
> @@ -519,33 +542,18 @@ static void iommu_table_setparms(struct pci_controller *phb,
>   		return;
>   	}
>   
> -	tbl->it_base = (unsigned long)__va(*basep);
> +	iommu_table_setparms_common(tbl, phb->bus->number, 0, phb->dma_window_base_cur,
> +				    phb->dma_window_size, IOMMU_PAGE_SHIFT_4K,
> +				    __va(*basep), &iommu_table_pseries_ops);
>   
>   	if (!is_kdump_kernel())
>   		memset((void *)tbl->it_base, 0, *sizep);
>   
> -	tbl->it_busno = phb->bus->number;
> -	tbl->it_page_shift = IOMMU_PAGE_SHIFT_4K;
> -
> -	/* Units of tce entries */
> -	tbl->it_offset = phb->dma_window_base_cur >> tbl->it_page_shift;
> -
> -	/* Test if we are going over 2GB of DMA space */
> -	if (phb->dma_window_base_cur + phb->dma_window_size > 0x80000000ul) {
> -		udbg_printf("PCI_DMA: Unexpected number of IOAs under this PHB.\n");
> -		panic("PCI_DMA: Unexpected number of IOAs under this PHB.\n");
> -	}
> -
>   	phb->dma_window_base_cur += phb->dma_window_size;
> -
> -	/* Set the tce table size - measured in entries */
> -	tbl->it_size = phb->dma_window_size >> tbl->it_page_shift;
> -
> -	tbl->it_index = 0;
> -	tbl->it_blocksize = 16;
> -	tbl->it_type = TCE_PCI;
>   }
>   
> +struct iommu_table_ops iommu_table_lpar_multi_ops;
> +
>   /*
>    * iommu_table_setparms_lpar
>    *
> @@ -557,17 +565,13 @@ static void iommu_table_setparms_lpar(struct pci_controller *phb,
>   				      struct iommu_table_group *table_group,
>   				      const __be32 *dma_window)
>   {
> -	unsigned long offset, size;
> +	unsigned long offset, size, liobn;
>   
> -	of_parse_dma_window(dn, dma_window, &tbl->it_index, &offset, &size);
> +	of_parse_dma_window(dn, dma_window, &liobn, &offset, &size);
> +
> +	iommu_table_setparms_common(tbl, phb->bus->number, liobn, offset, size, IOMMU_PAGE_SHIFT_4K, NULL,
> +				    &iommu_table_lpar_multi_ops);
>   
> -	tbl->it_busno = phb->bus->number;
> -	tbl->it_page_shift = IOMMU_PAGE_SHIFT_4K;
> -	tbl->it_base   = 0;
> -	tbl->it_blocksize  = 16;
> -	tbl->it_type = TCE_PCI;
> -	tbl->it_offset = offset >> tbl->it_page_shift;
> -	tbl->it_size = size >> tbl->it_page_shift;
>   
>   	table_group->tce32_start = offset;
>   	table_group->tce32_size = size;
> @@ -647,7 +651,7 @@ static void pci_dma_bus_setup_pSeries(struct pci_bus *bus)
>   	tbl = pci->table_group->tables[0];
>   
>   	iommu_table_setparms(pci->phb, dn, tbl);
> -	tbl->it_ops = &iommu_table_pseries_ops;
> +
>   	if (!iommu_init_table(tbl, pci->phb->node, 0, 0))
>   		panic("Failed to initialize iommu table");
>   
> @@ -730,7 +734,7 @@ static void pci_dma_bus_setup_pSeriesLP(struct pci_bus *bus)
>   		tbl = ppci->table_group->tables[0];
>   		iommu_table_setparms_lpar(ppci->phb, pdn, tbl,
>   				ppci->table_group, dma_window);
> -		tbl->it_ops = &iommu_table_lpar_multi_ops;
> +
>   		if (!iommu_init_table(tbl, ppci->phb->node, 0, 0))
>   			panic("Failed to initialize iommu table");
>   		iommu_register_group(ppci->table_group,
> @@ -760,7 +764,7 @@ static void pci_dma_dev_setup_pSeries(struct pci_dev *dev)
>   		PCI_DN(dn)->table_group = iommu_pseries_alloc_group(phb->node);
>   		tbl = PCI_DN(dn)->table_group->tables[0];
>   		iommu_table_setparms(phb, dn, tbl);
> -		tbl->it_ops = &iommu_table_pseries_ops;
> +
>   		if (!iommu_init_table(tbl, phb->node, 0, 0))
>   			panic("Failed to initialize iommu table");
>   
> @@ -1443,7 +1447,7 @@ static void pci_dma_dev_setup_pSeriesLP(struct pci_dev *dev)
>   		tbl = pci->table_group->tables[0];
>   		iommu_table_setparms_lpar(pci->phb, pdn, tbl,
>   				pci->table_group, dma_window);
> -		tbl->it_ops = &iommu_table_lpar_multi_ops;
> +
>   		iommu_init_table(tbl, pci->phb->node, 0, 0);
>   		iommu_register_group(pci->table_group,
>   				pci_domain_nr(pci->phb->bus), 0);
> 

^ permalink raw reply

* Re: [PATCH v5 06/11] powerpc/pseries/iommu: Add ddw_property_create() and refactor enable_ddw()
From: Frederic Barrat @ 2021-07-20 17:49 UTC (permalink / raw)
  To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
	Paul Mackerras, Alexey Kardashevskiy, David Gibson,
	kernel test robot, Nicolin Chen
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20210716082755.428187-7-leobras.c@gmail.com>



On 16/07/2021 10:27, Leonardo Bras wrote:
> Code used to create a ddw property that was previously scattered in
> enable_ddw() is now gathered in ddw_property_create(), which deals with
> allocation and filling the property, letting it ready for
> of_property_add(), which now occurs in sequence.
> 
> This created an opportunity to reorganize the second part of enable_ddw():
> 
> Without this patch enable_ddw() does, in order:
> kzalloc() property & members, create_ddw(), fill ddwprop inside property,
> ddw_list_new_entry(), do tce_setrange_multi_pSeriesLP_walk in all memory,
> of_add_property(), and list_add().
> 
> With this patch enable_ddw() does, in order:
> create_ddw(), ddw_property_create(), of_add_property(),
> ddw_list_new_entry(), do tce_setrange_multi_pSeriesLP_walk in all memory,
> and list_add().
> 
> This change requires of_remove_property() in case anything fails after
> of_add_property(), but we get to do tce_setrange_multi_pSeriesLP_walk
> in all memory, which looks the most expensive operation, only if
> everything else succeeds.
> 
> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>   arch/powerpc/platforms/pseries/iommu.c | 93 ++++++++++++++++----------
>   1 file changed, 57 insertions(+), 36 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> index b34b473bbdc1..7ca79a04fa52 100644
> --- a/arch/powerpc/platforms/pseries/iommu.c
> +++ b/arch/powerpc/platforms/pseries/iommu.c
> @@ -1153,6 +1153,35 @@ static int iommu_get_page_shift(u32 query_page_size)
>   	return 0;
>   }
>   
> +static struct property *ddw_property_create(const char *propname, u32 liobn, u64 dma_addr,
> +					    u32 page_shift, u32 window_shift)
> +{
> +	struct dynamic_dma_window_prop *ddwprop;
> +	struct property *win64;
> +
> +	win64 = kzalloc(sizeof(*win64), GFP_KERNEL);
> +	if (!win64)
> +		return NULL;
> +
> +	win64->name = kstrdup(propname, GFP_KERNEL);
> +	ddwprop = kzalloc(sizeof(*ddwprop), GFP_KERNEL);
> +	win64->value = ddwprop;
> +	win64->length = sizeof(*ddwprop);
> +	if (!win64->name || !win64->value) {
> +		kfree(win64->name);
> +		kfree(win64->value);
> +		kfree(win64);
> +		return NULL;
> +	}
> +
> +	ddwprop->liobn = cpu_to_be32(liobn);
> +	ddwprop->dma_base = cpu_to_be64(dma_addr);
> +	ddwprop->tce_shift = cpu_to_be32(page_shift);
> +	ddwprop->window_shift = cpu_to_be32(window_shift);
> +
> +	return win64;
> +}
> +
>   /*
>    * If the PE supports dynamic dma windows, and there is space for a table
>    * that can map all pages in a linear offset, then setup such a table,
> @@ -1171,12 +1200,12 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>   	struct ddw_query_response query;
>   	struct ddw_create_response create;
>   	int page_shift;
> +	u64 win_addr;
>   	struct device_node *dn;
>   	u32 ddw_avail[DDW_APPLICABLE_SIZE];
>   	struct direct_window *window;
>   	struct property *win64;
>   	bool ddw_enabled = false;
> -	struct dynamic_dma_window_prop *ddwprop;
>   	struct failed_ddw_pdn *fpdn;
>   	bool default_win_removed = false;
>   	bool pmem_present;
> @@ -1293,72 +1322,64 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>   			1ULL << page_shift);
>   		goto out_failed;
>   	}
> -	win64 = kzalloc(sizeof(struct property), GFP_KERNEL);
> -	if (!win64) {
> -		dev_info(&dev->dev,
> -			"couldn't allocate property for 64bit dma window\n");
> -		goto out_failed;
> -	}
> -	win64->name = kstrdup(DIRECT64_PROPNAME, GFP_KERNEL);
> -	win64->value = ddwprop = kmalloc(sizeof(*ddwprop), GFP_KERNEL);
> -	win64->length = sizeof(*ddwprop);
> -	if (!win64->name || !win64->value) {
> -		dev_info(&dev->dev,
> -			"couldn't allocate property name and value\n");
> -		goto out_free_prop;
> -	}
>   
>   	ret = create_ddw(dev, ddw_avail, &create, page_shift, len);
>   	if (ret != 0)
> -		goto out_free_prop;
> -
> -	ddwprop->liobn = cpu_to_be32(create.liobn);
> -	ddwprop->dma_base = cpu_to_be64(((u64)create.addr_hi << 32) |
> -			create.addr_lo);
> -	ddwprop->tce_shift = cpu_to_be32(page_shift);
> -	ddwprop->window_shift = cpu_to_be32(len);
> +		goto out_failed;
>   
>   	dev_dbg(&dev->dev, "created tce table LIOBN 0x%x for %pOF\n",
>   		  create.liobn, dn);
>   
> -	window = ddw_list_new_entry(pdn, ddwprop);
> +	win_addr = ((u64)create.addr_hi << 32) | create.addr_lo;
> +	win64 = ddw_property_create(DIRECT64_PROPNAME, create.liobn, win_addr,
> +				    page_shift, len);
> +	if (!win64) {
> +		dev_info(&dev->dev,
> +			 "couldn't allocate property, property name, or value\n");
> +		goto out_remove_win;
> +	}
> +
> +	ret = of_add_property(pdn, win64);
> +	if (ret) {
> +		dev_err(&dev->dev, "unable to add dma window property for %pOF: %d",
> +			pdn, ret);
> +		goto out_free_prop;
> +	}
> +
> +	window = ddw_list_new_entry(pdn, win64->value);
>   	if (!window)
> -		goto out_clear_window;
> +		goto out_del_prop;
>   
>   	ret = walk_system_ram_range(0, memblock_end_of_DRAM() >> PAGE_SHIFT,
>   			win64->value, tce_setrange_multi_pSeriesLP_walk);
>   	if (ret) {
>   		dev_info(&dev->dev, "failed to map direct window for %pOF: %d\n",
>   			 dn, ret);
> -		goto out_free_window;
> -	}
> -
> -	ret = of_add_property(pdn, win64);
> -	if (ret) {
> -		dev_err(&dev->dev, "unable to add dma window property for %pOF: %d",
> -			 pdn, ret);
> -		goto out_free_window;
> +		goto out_del_list;
>   	}
>   
>   	spin_lock(&direct_window_list_lock);
>   	list_add(&window->list, &direct_window_list);
>   	spin_unlock(&direct_window_list_lock);
>   
> -	dev->dev.archdata.dma_offset = be64_to_cpu(ddwprop->dma_base);
> +	dev->dev.archdata.dma_offset = win_addr;
>   	ddw_enabled = true;
>   	goto out_unlock;
>   
> -out_free_window:
> +out_del_list:
>   	kfree(window);
>   
> -out_clear_window:
> -	remove_ddw(pdn, true);
> +out_del_prop:
> +	of_remove_property(pdn, win64);
>   
>   out_free_prop:
>   	kfree(win64->name);
>   	kfree(win64->value);
>   	kfree(win64);
>   
> +out_remove_win:
> +	remove_ddw(pdn, true);


I believe there's a small problem here. We jump directly to 
out_remove_win if allocating the property failed. Yet, the first thing 
remove_ddw() does is look for the property. So it will never find it and 
the window is never removed by the hypervisor.

   Fred


> +
>   out_failed:
>   	if (default_win_removed)
>   		reset_dma_window(dev, pdn);
> 

^ permalink raw reply

* Re: [PATCH v5 05/11] powerpc/pseries/iommu: Allow DDW windows starting at 0x00
From: Frederic Barrat @ 2021-07-20 17:44 UTC (permalink / raw)
  To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
	Paul Mackerras, Alexey Kardashevskiy, David Gibson,
	kernel test robot, Nicolin Chen
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20210716082755.428187-6-leobras.c@gmail.com>



On 16/07/2021 10:27, Leonardo Bras wrote:
> enable_ddw() currently returns the address of the DMA window, which is
> considered invalid if has the value 0x00.
> 
> Also, it only considers valid an address returned from find_existing_ddw
> if it's not 0x00.
> 
> Changing this behavior makes sense, given the users of enable_ddw() only
> need to know if direct mapping is possible. It can also allow a DMA window
> starting at 0x00 to be used.
> 
> This will be helpful for using a DDW with indirect mapping, as the window
> address will be different than 0x00, but it will not map the whole
> partition.
> 
> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---


Looks good to me
Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com>



>   arch/powerpc/platforms/pseries/iommu.c | 36 +++++++++++++-------------
>   1 file changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> index 712d1667144a..b34b473bbdc1 100644
> --- a/arch/powerpc/platforms/pseries/iommu.c
> +++ b/arch/powerpc/platforms/pseries/iommu.c
> @@ -853,25 +853,26 @@ static void remove_ddw(struct device_node *np, bool remove_prop)
>   			np, ret);
>   }
>   
> -static u64 find_existing_ddw(struct device_node *pdn, int *window_shift)
> +static bool find_existing_ddw(struct device_node *pdn, u64 *dma_addr, int *window_shift)
>   {
>   	struct direct_window *window;
>   	const struct dynamic_dma_window_prop *direct64;
> -	u64 dma_addr = 0;
> +	bool found = false;
>   
>   	spin_lock(&direct_window_list_lock);
>   	/* check if we already created a window and dupe that config if so */
>   	list_for_each_entry(window, &direct_window_list, list) {
>   		if (window->device == pdn) {
>   			direct64 = window->prop;
> -			dma_addr = be64_to_cpu(direct64->dma_base);
> +			*dma_addr = be64_to_cpu(direct64->dma_base);
>   			*window_shift = be32_to_cpu(direct64->window_shift);
> +			found = true;
>   			break;
>   		}
>   	}
>   	spin_unlock(&direct_window_list_lock);
>   
> -	return dma_addr;
> +	return found;
>   }
>   
>   static struct direct_window *ddw_list_new_entry(struct device_node *pdn,
> @@ -1161,20 +1162,20 @@ static int iommu_get_page_shift(u32 query_page_size)
>    * pdn: the parent pe node with the ibm,dma_window property
>    * Future: also check if we can remap the base window for our base page size
>    *
> - * returns the dma offset for use by the direct mapped DMA code.
> + * returns true if can map all pages (direct mapping), false otherwise..
>    */
> -static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> +static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>   {
>   	int len = 0, ret;
>   	int max_ram_len = order_base_2(ddw_memory_hotplug_max());
>   	struct ddw_query_response query;
>   	struct ddw_create_response create;
>   	int page_shift;
> -	u64 dma_addr;
>   	struct device_node *dn;
>   	u32 ddw_avail[DDW_APPLICABLE_SIZE];
>   	struct direct_window *window;
>   	struct property *win64;
> +	bool ddw_enabled = false;
>   	struct dynamic_dma_window_prop *ddwprop;
>   	struct failed_ddw_pdn *fpdn;
>   	bool default_win_removed = false;
> @@ -1186,9 +1187,10 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>   
>   	mutex_lock(&direct_window_init_mutex);
>   
> -	dma_addr = find_existing_ddw(pdn, &len);
> -	if (dma_addr != 0)
> +	if (find_existing_ddw(pdn, &dev->dev.archdata.dma_offset, &len)) {
> +		ddw_enabled = true;
>   		goto out_unlock;
> +	}
>   
>   	/*
>   	 * If we already went through this for a previous function of
> @@ -1342,7 +1344,8 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>   	list_add(&window->list, &direct_window_list);
>   	spin_unlock(&direct_window_list_lock);
>   
> -	dma_addr = be64_to_cpu(ddwprop->dma_base);
> +	dev->dev.archdata.dma_offset = be64_to_cpu(ddwprop->dma_base);
> +	ddw_enabled = true;
>   	goto out_unlock;
>   
>   out_free_window:
> @@ -1374,10 +1377,10 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>   	 * as RAM, then we failed to create a window to cover persistent
>   	 * memory and need to set the DMA limit.
>   	 */
> -	if (pmem_present && dma_addr && (len == max_ram_len))
> -		dev->dev.bus_dma_limit = dma_addr + (1ULL << len);
> +	if (pmem_present && ddw_enabled && (len == max_ram_len))
> +		dev->dev.bus_dma_limit = dev->dev.archdata.dma_offset + (1ULL << len);
>   
> -	return dma_addr;
> +	return ddw_enabled;
>   }
>   
>   static void pci_dma_dev_setup_pSeriesLP(struct pci_dev *dev)
> @@ -1456,11 +1459,8 @@ static bool iommu_bypass_supported_pSeriesLP(struct pci_dev *pdev, u64 dma_mask)
>   			break;
>   	}
>   
> -	if (pdn && PCI_DN(pdn)) {
> -		pdev->dev.archdata.dma_offset = enable_ddw(pdev, pdn);
> -		if (pdev->dev.archdata.dma_offset)
> -			return true;
> -	}
> +	if (pdn && PCI_DN(pdn))
> +		return enable_ddw(pdev, pdn);
>   
>   	return false;
>   }
> 

^ permalink raw reply

* Re: [PATCH 1/2] Revert "powerpc/8xx: add support for huge pages on VMAP and VMALLOC"
From: Christophe Leroy @ 2021-07-20 15:08 UTC (permalink / raw)
  To: Will Deacon
  Cc: Mark Rutland, Jonathan Marek, Marc Zyngier, linux-kernel,
	Ard Biesheuvel, Geert Uytterhoeven, linux-arm-kernel,
	Catalin Marinas, Paul Mackerras, Andrew Morton, linuxppc-dev,
	Thomas Gleixner, Mike Rapoport
In-Reply-To: <20210720123512.8740-2-will@kernel.org>

Will Deacon <will@kernel.org> a écrit :

> This reverts commit a6a8f7c4aa7eb50304b5c4e68eccd24313f3a785.
>
> Commit c742199a014d ("mm/pgtable: add stubs for
> {pmd/pub}_{set/clear}_huge") breaks the boot for arm64 when block
> mappings are used to create the linear map, as this relies on a working
> implementation of pXd_set_huge() even if the corresponding page-table
> levels have been folded.
>
> Although the problematic patch reverts cleanly, doing so breaks the
> build for 32-bit PowerPC 8xx machines, which rely on the default
> function definitions when the corresponding page-table levels are
> folded:
>
>  | powerpc64-linux-ld: mm/vmalloc.o: in function `vunmap_pud_range':
>  | linux/mm/vmalloc.c:362: undefined reference to `pud_clear_huge'
>  | powerpc64-linux-ld: mm/vmalloc.o: in function `vunmap_pmd_range':
>  | linux/mm/vmalloc.c:337: undefined reference to `pmd_clear_huge'
>  | powerpc64-linux-ld: mm/vmalloc.o: in function `vunmap_pud_range':
>  | linux/mm/vmalloc.c:362: undefined reference to `pud_clear_huge'
>  | powerpc64-linux-ld: mm/vmalloc.o: in function `vunmap_pmd_range':
>  | linux/mm/vmalloc.c:337: undefined reference to `pmd_clear_huge'
>  | make: *** [Makefile:1177: vmlinux] Error 1
>
> Although Christophe has kindly offered to look into the arm64 breakage,
> he's on holiday for another 10 days and there isn't an obvious fix on
> the arm64 side which allows us to continue using huge-vmap for affected
> configurations.
>
> In the interest of quickly getting things back to a working state as
> they were in 5.13, revert the huge-vmap changes for PowerPC 8xx prior to
> reverting the change which breaks arm64. We can then work on this
> together for 5.15 once Christophe is back.

Instead of reverting this awaited functionnality,  could you please  
just add the two following functions in arch/powerpc/mm/nohash/8xx.c :

int pud_clear_huge(pud_t *pud)
{
	return 0;
}

int pmd_clear_huge(pmd_t *pmd)
{
	return 0;
}

Thank you
Christophe

>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Nicholas Piggin <npiggin@gmail.com
> Cc: Mike Rapoport <rppt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Marc Zyngier <maz@kernel.org>
> Link:  
> https://lore.kernel.org/r/20210719170615.Horde.Qio1wp3k5ebLo-d9xXHdOg1@messagerie.c-s.fr
> Signed-off-by: Will Deacon <will@kernel.org>
> ---
>  arch/powerpc/Kconfig                         |  2 +-
>  arch/powerpc/include/asm/nohash/32/mmu-8xx.h | 43 --------------------
>  2 files changed, 1 insertion(+), 44 deletions(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index d01e3401581d..5fc19ac62cb9 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -189,7 +189,7 @@ config PPC
>  	select GENERIC_VDSO_TIME_NS
>  	select HAVE_ARCH_AUDITSYSCALL
>  	select HAVE_ARCH_HUGE_VMALLOC		if HAVE_ARCH_HUGE_VMAP
> -	select HAVE_ARCH_HUGE_VMAP		if PPC_RADIX_MMU || PPC_8xx
> +	select HAVE_ARCH_HUGE_VMAP		if PPC_BOOK3S_64 && PPC_RADIX_MMU
>  	select HAVE_ARCH_JUMP_LABEL
>  	select HAVE_ARCH_JUMP_LABEL_RELATIVE
>  	select HAVE_ARCH_KASAN			if PPC32 && PPC_PAGE_SHIFT <= 14
> diff --git a/arch/powerpc/include/asm/nohash/32/mmu-8xx.h  
> b/arch/powerpc/include/asm/nohash/32/mmu-8xx.h
> index 997cec973406..6e4faa0a9b35 100644
> --- a/arch/powerpc/include/asm/nohash/32/mmu-8xx.h
> +++ b/arch/powerpc/include/asm/nohash/32/mmu-8xx.h
> @@ -178,7 +178,6 @@
>  #ifndef __ASSEMBLY__
>
>  #include <linux/mmdebug.h>
> -#include <linux/sizes.h>
>
>  void mmu_pin_tlb(unsigned long top, bool readonly);
>
> @@ -226,48 +225,6 @@ static inline unsigned int  
> mmu_psize_to_shift(unsigned int mmu_psize)
>  	BUG();
>  }
>
> -static inline bool arch_vmap_try_size(unsigned long addr, unsigned  
> long end, u64 pfn,
> -				      unsigned int max_page_shift, unsigned long size)
> -{
> -	if (end - addr < size)
> -		return false;
> -
> -	if ((1UL << max_page_shift) < size)
> -		return false;
> -
> -	if (!IS_ALIGNED(addr, size))
> -		return false;
> -
> -	if (!IS_ALIGNED(PFN_PHYS(pfn), size))
> -		return false;
> -
> -	return true;
> -}
> -
> -static inline unsigned long arch_vmap_pte_range_map_size(unsigned  
> long addr, unsigned long end,
> -							 u64 pfn, unsigned int max_page_shift)
> -{
> -	if (arch_vmap_try_size(addr, end, pfn, max_page_shift, SZ_512K))
> -		return SZ_512K;
> -	if (PAGE_SIZE == SZ_16K)
> -		return SZ_16K;
> -	if (arch_vmap_try_size(addr, end, pfn, max_page_shift, SZ_16K))
> -		return SZ_16K;
> -	return PAGE_SIZE;
> -}
> -#define arch_vmap_pte_range_map_size arch_vmap_pte_range_map_size
> -
> -static inline int arch_vmap_pte_supported_shift(unsigned long size)
> -{
> -	if (size >= SZ_512K)
> -		return 19;
> -	else if (size >= SZ_16K)
> -		return 14;
> -	else
> -		return PAGE_SHIFT;
> -}
> -#define arch_vmap_pte_supported_shift arch_vmap_pte_supported_shift
> -
>  /* patch sites */
>  extern s32 patch__itlbmiss_exit_1, patch__dtlbmiss_exit_1;
>  extern s32 patch__itlbmiss_perf, patch__dtlbmiss_perf;
> --
> 2.32.0.402.g57bb445576-goog



^ permalink raw reply

* [PATCH v3] PCI: Move pci_dev_is/assign_added() to pci.h
From: Niklas Schnelle @ 2021-07-20 15:01 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-arch, linux-s390, linux-pci, linux-kernel, Paul Mackerras,
	linuxppc-dev

The helper function pci_dev_is_added() from drivers/pci/pci.h is used in
PCI arch code of both s390 and powerpc leading to awkward relative
includes. Move it to the global include/linux/pci.h and get rid of these
includes just for that one function.

Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
---
Since v1 (and bad v2):
- Fixed accidental removal of PCI_DPC_RECOVERED, PCI_DPC_RECOVERING
  defines and also move these to include/linux/pci.h

 arch/powerpc/platforms/powernv/pci-sriov.c |  3 ---
 arch/powerpc/platforms/pseries/setup.c     |  1 -
 arch/s390/pci/pci_sysfs.c                  |  2 --
 drivers/pci/hotplug/acpiphp_glue.c         |  1 -
 drivers/pci/pci.h                          | 15 ---------------
 include/linux/pci.h                        | 15 +++++++++++++++
 6 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci-sriov.c b/arch/powerpc/platforms/powernv/pci-sriov.c
index 28aac933a439..2e0ca5451e85 100644
--- a/arch/powerpc/platforms/powernv/pci-sriov.c
+++ b/arch/powerpc/platforms/powernv/pci-sriov.c
@@ -9,9 +9,6 @@
 
 #include "pci.h"
 
-/* for pci_dev_is_added() */
-#include "../../../../drivers/pci/pci.h"
-
 /*
  * The majority of the complexity in supporting SR-IOV on PowerNV comes from
  * the need to put the MMIO space for each VF into a separate PE. Internally
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 631a0d57b6cd..17585ec9f955 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -74,7 +74,6 @@
 #include <asm/hvconsole.h>
 
 #include "pseries.h"
-#include "../../../../drivers/pci/pci.h"
 
 DEFINE_STATIC_KEY_FALSE(shared_processor);
 EXPORT_SYMBOL_GPL(shared_processor);
diff --git a/arch/s390/pci/pci_sysfs.c b/arch/s390/pci/pci_sysfs.c
index 6e2450c2b9c1..8dbe54ef8f8e 100644
--- a/arch/s390/pci/pci_sysfs.c
+++ b/arch/s390/pci/pci_sysfs.c
@@ -13,8 +13,6 @@
 #include <linux/stat.h>
 #include <linux/pci.h>
 
-#include "../../../drivers/pci/pci.h"
-
 #include <asm/sclp.h>
 
 #define zpci_attr(name, fmt, member)					\
diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index f031302ad401..4cb963f88183 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -38,7 +38,6 @@
 #include <linux/slab.h>
 #include <linux/acpi.h>
 
-#include "../pci.h"
 #include "acpiphp.h"
 
 static LIST_HEAD(bridge_list);
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 93dcdd431072..a159cd0f6f05 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -383,21 +383,6 @@ static inline bool pci_dev_is_disconnected(const struct pci_dev *dev)
 	return dev->error_state == pci_channel_io_perm_failure;
 }
 
-/* pci_dev priv_flags */
-#define PCI_DEV_ADDED 0
-#define PCI_DPC_RECOVERED 1
-#define PCI_DPC_RECOVERING 2
-
-static inline void pci_dev_assign_added(struct pci_dev *dev, bool added)
-{
-	assign_bit(PCI_DEV_ADDED, &dev->priv_flags, added);
-}
-
-static inline bool pci_dev_is_added(const struct pci_dev *dev)
-{
-	return test_bit(PCI_DEV_ADDED, &dev->priv_flags);
-}
-
 #ifdef CONFIG_PCIEAER
 #include <linux/aer.h>
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 540b377ca8f6..ea0e23dbc8ec 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -507,6 +507,21 @@ struct pci_dev {
 	unsigned long	priv_flags;	/* Private flags for the PCI driver */
 };
 
+/* pci_dev priv_flags */
+#define PCI_DEV_ADDED 0
+#define PCI_DPC_RECOVERED 1
+#define PCI_DPC_RECOVERING 2
+
+static inline void pci_dev_assign_added(struct pci_dev *dev, bool added)
+{
+	assign_bit(PCI_DEV_ADDED, &dev->priv_flags, added);
+}
+
+static inline bool pci_dev_is_added(const struct pci_dev *dev)
+{
+	return test_bit(PCI_DEV_ADDED, &dev->priv_flags);
+}
+
 static inline struct pci_dev *pci_physfn(struct pci_dev *dev)
 {
 #ifdef CONFIG_PCI_IOV
-- 
2.25.1


^ permalink raw reply related

* [PATCH] powerpc/64s/perf: Always use SIAR for kernel interrupts
From: Nicholas Piggin @ 2021-07-20 14:15 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Athira Rajeev, Madhavan Srinivasan, Nicholas Piggin

If an interrupt is taken in kernel mode, always use SIAR for it rather than
looking at regs_sipr. This prevents samples piling up around interrupt
enable (hard enable or interrupt replay via soft enable) in PMUs / modes
where the PR sample indication is not in synch with SIAR.

This results in better sampling of interrupt entry and exit in particular.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/perf/core-book3s.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index bb0ee716de91..91203ed9d0ff 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -340,6 +340,13 @@ static inline void perf_read_regs(struct pt_regs *regs)
 	 * If the PMU doesn't update the SIAR for non marked events use
 	 * pt_regs.
 	 *
+	 * If regs is a kernel interrupt, always use SIAR. Some PMUs have an
+	 * issue with regs_sipr not being in synch with SIAR in interrupt entry
+	 * and return sequences, which can result in regs_sipr being true for
+	 * kernel interrupts and SIAR, which has the effect of causing samples
+	 * to pile up at mtmsrd MSR[EE] 0->1 or pending irq replay around
+	 * interrupt entry/exit.
+	 *
 	 * If the PMU has HV/PR flags then check to see if they
 	 * place the exception in userspace. If so, use pt_regs. In
 	 * continuous sampling mode the SIAR and the PMU exception are
@@ -356,6 +363,8 @@ static inline void perf_read_regs(struct pt_regs *regs)
 		use_siar = 1;
 	else if ((ppmu->flags & PPMU_NO_CONT_SAMPLING))
 		use_siar = 0;
+	else if (!user_mode(regs))
+		use_siar = 1;
 	else if (!(ppmu->flags & PPMU_NO_SIPR) && regs_sipr(regs))
 		use_siar = 0;
 	else
-- 
2.23.0


^ permalink raw reply related

* Re: [PATCH v2] PCI: Move pci_dev_is/assign_added() to pci.h
From: Niklas Schnelle @ 2021-07-20 13:59 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-arch, linux-s390, linux-pci, linux-kernel, Paul Mackerras,
	linuxppc-dev
In-Reply-To: <20210720095816.3660813-1-schnelle@linux.ibm.com>

On Tue, 2021-07-20 at 11:58 +0200, Niklas Schnelle wrote:
> The helper function pci_dev_is_added() from drivers/pci/pci.h is used in
> PCI arch code of both s390 and powerpc leading to awkward relative
> includes. Move it to the global include/linux/pci.h and get rid of these
> includes just for that one function.
> 
> Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> ---
> Since v1:
> - Fixed accidental removal of PCI_DPC_RECOVERED, PCI_DPC_RECOVERING
>   defines and also move these to include/linux/pci.h

Please disregard I actually sent the old patch ;-(

> 
>  arch/powerpc/platforms/powernv/pci-sriov.c |  3 ---
>  arch/powerpc/platforms/pseries/setup.c     |  1 -
>  arch/s390/pci/pci_sysfs.c                  |  2 --
>  drivers/pci/hotplug/acpiphp_glue.c         |  1 -
>  drivers/pci/pci.h                          | 15 ---------------
>  include/linux/pci.h                        | 13 +++++++++++++
>  6 files changed, 13 insertions(+), 22 deletions(-)
> 
> 
... snip ..


^ permalink raw reply

* Re: [PATCH v6 1/1] powerpc/pseries: Interface to represent PAPR firmware attributes
From: Pratik Sampat @ 2021-07-20 13:55 UTC (permalink / raw)
  To: kajoljain, mpe, benh, paulus, farosas, linuxppc-dev, kvm-ppc,
	linux-kernel, pratik.r.sampat
In-Reply-To: <262e4a2e-7410-a6e0-6183-06c33edc16ae@linux.ibm.com>



On 20/07/21 7:02 pm, kajoljain wrote:
>
> On 7/20/21 11:04 AM, Pratik R. Sampat wrote:
>> Adds a generic interface to represent the energy and frequency related
>> PAPR attributes on the system using the new H_CALL
>> "H_GET_ENERGY_SCALE_INFO".
>>
>> H_GET_EM_PARMS H_CALL was previously responsible for exporting this
>> information in the lparcfg, however the H_GET_EM_PARMS H_CALL
>> will be deprecated P10 onwards.
>>
>> The H_GET_ENERGY_SCALE_INFO H_CALL is of the following call format:
>> hcall(
>>    uint64 H_GET_ENERGY_SCALE_INFO,  // Get energy scale info
>>    uint64 flags,           // Per the flag request
>>    uint64 firstAttributeId,// The attribute id
>>    uint64 bufferAddress,   // Guest physical address of the output buffer
>>    uint64 bufferSize       // The size in bytes of the output buffer
>> );
>>
>> This H_CALL can query either all the attributes at once with
>> firstAttributeId = 0, flags = 0 as well as query only one attribute
>> at a time with firstAttributeId = id, flags = 1.
>>
>> The output buffer consists of the following
>> 1. number of attributes              - 8 bytes
>> 2. array offset to the data location - 8 bytes
>> 3. version info                      - 1 byte
>> 4. A data array of size num attributes, which contains the following:
>>    a. attribute ID              - 8 bytes
>>    b. attribute value in number - 8 bytes
>>    c. attribute name in string  - 64 bytes
>>    d. attribute value in string - 64 bytes
>>
>> The new H_CALL exports information in direct string value format, hence
>> a new interface has been introduced in
>> /sys/firmware/papr/energy_scale_info to export this information to
>> userspace in an extensible pass-through format.
>>
>> The H_CALL returns the name, numeric value and string value (if exists)
>>
>> The format of exposing the sysfs information is as follows:
>> /sys/firmware/papr/energy_scale_info/
>>     |-- <id>/
>>       |-- desc
>>       |-- value
>>       |-- value_desc (if exists)
>>     |-- <id>/
>>       |-- desc
>>       |-- value
>>       |-- value_desc (if exists)
>> ...
>>
>> The energy information that is exported is useful for userspace tools
>> such as powerpc-utils. Currently these tools infer the
>> "power_mode_data" value in the lparcfg, which in turn is obtained from
>> the to be deprecated H_GET_EM_PARMS H_CALL.
>> On future platforms, such userspace utilities will have to look at the
>> data returned from the new H_CALL being populated in this new sysfs
>> interface and report this information directly without the need of
>> interpretation.
>>
>> Signed-off-by: Pratik R. Sampat <psampat@linux.ibm.com>
>> Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
>> ---
>>   .../sysfs-firmware-papr-energy-scale-info     |  26 ++
>>   arch/powerpc/include/asm/hvcall.h             |  24 +-
>>   arch/powerpc/kvm/trace_hv.h                   |   1 +
>>   arch/powerpc/platforms/pseries/Makefile       |   3 +-
>>   .../pseries/papr_platform_attributes.c        | 312 ++++++++++++++++++
>>   5 files changed, 364 insertions(+), 2 deletions(-)
>>   create mode 100644 Documentation/ABI/testing/sysfs-firmware-papr-energy-scale-info
>>   create mode 100644 arch/powerpc/platforms/pseries/papr_platform_attributes.c
>>
>> diff --git a/Documentation/ABI/testing/sysfs-firmware-papr-energy-scale-info b/Documentation/ABI/testing/sysfs-firmware-papr-energy-scale-info
>> new file mode 100644
>> index 000000000000..139a576c7c9d
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-firmware-papr-energy-scale-info
>> @@ -0,0 +1,26 @@
>> +What:		/sys/firmware/papr/energy_scale_info
>> +Date:		June 2021
>> +Contact:	Linux for PowerPC mailing list <linuxppc-dev@ozlabs.org>
>> +Description:	Directory hosting a set of platform attributes like
>> +		energy/frequency on Linux running as a PAPR guest.
>> +
>> +		Each file in a directory contains a platform
>> +		attribute hierarchy pertaining to performance/
>> +		energy-savings mode and processor frequency.
>> +
>> +What:		/sys/firmware/papr/energy_scale_info/<id>
>> +		/sys/firmware/papr/energy_scale_info/<id>/desc
>> +		/sys/firmware/papr/energy_scale_info/<id>/value
>> +		/sys/firmware/papr/energy_scale_info/<id>/value_desc
>> +Date:		June 2021
>> +Contact:	Linux for PowerPC mailing list <linuxppc-dev@ozlabs.org>
>> +Description:	Energy, frequency attributes directory for POWERVM servers
>> +
>> +		This directory provides energy, frequency, folding information. It
>> +		contains below sysfs attributes:
>> +
>> +		- desc: String description of the attribute <id>
>> +
>> +		- value: Numeric value of attribute <id>
>> +
>> +		- value_desc: String value of attribute <id>
>> diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
>> index e3b29eda8074..c91714ea6719 100644
>> --- a/arch/powerpc/include/asm/hvcall.h
>> +++ b/arch/powerpc/include/asm/hvcall.h
>> @@ -316,7 +316,8 @@
>>   #define H_SCM_PERFORMANCE_STATS 0x418
>>   #define H_RPT_INVALIDATE	0x448
>>   #define H_SCM_FLUSH		0x44C
>> -#define MAX_HCALL_OPCODE	H_SCM_FLUSH
>> +#define H_GET_ENERGY_SCALE_INFO	0x450
>> +#define MAX_HCALL_OPCODE	H_GET_ENERGY_SCALE_INFO
>>   
>>   /* Scope args for H_SCM_UNBIND_ALL */
>>   #define H_UNBIND_SCOPE_ALL (0x1)
>> @@ -631,6 +632,27 @@ struct hv_gpci_request_buffer {
>>   	uint8_t bytes[HGPCI_MAX_DATA_BYTES];
>>   } __packed;
>>   
>> +#define ESI_VERSION	0x1
>> +#define MAX_ESI_ATTRS	10
>> +#define MAX_BUF_SZ	(sizeof(struct h_energy_scale_info_hdr) + \
>> +			(sizeof(struct energy_scale_attribute) * MAX_ESI_ATTRS))
>> +
>> +struct energy_scale_attribute {
>> +	__be64 id;
>> +	__be64 value;
>> +	unsigned char desc[64];
>> +	unsigned char value_desc[64];
>> +} __packed;
>> +
>> +struct h_energy_scale_info_hdr {
>> +	__be64 num_attrs;
>> +	__be64 array_offset;
>> +	__u8 data_header_version;
>> +} __packed;
>> +
>> +/* /sys/firmware/papr */
>> +extern struct kobject *papr_kobj;
>> +
>>   #endif /* __ASSEMBLY__ */
>>   #endif /* __KERNEL__ */
>>   #endif /* _ASM_POWERPC_HVCALL_H */
>> diff --git a/arch/powerpc/kvm/trace_hv.h b/arch/powerpc/kvm/trace_hv.h
>> index 830a126e095d..38cd0ed0a617 100644
>> --- a/arch/powerpc/kvm/trace_hv.h
>> +++ b/arch/powerpc/kvm/trace_hv.h
>> @@ -115,6 +115,7 @@
>>   	{H_VASI_STATE,			"H_VASI_STATE"}, \
>>   	{H_ENABLE_CRQ,			"H_ENABLE_CRQ"}, \
>>   	{H_GET_EM_PARMS,		"H_GET_EM_PARMS"}, \
>> +	{H_GET_ENERGY_SCALE_INFO,	"H_GET_ENERGY_SCALE_INFO"}, \
>>   	{H_SET_MPP,			"H_SET_MPP"}, \
>>   	{H_GET_MPP,			"H_GET_MPP"}, \
>>   	{H_HOME_NODE_ASSOCIATIVITY,	"H_HOME_NODE_ASSOCIATIVITY"}, \
>> diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile
>> index c8a2b0b05ac0..d14fca89ac25 100644
>> --- a/arch/powerpc/platforms/pseries/Makefile
>> +++ b/arch/powerpc/platforms/pseries/Makefile
>> @@ -6,7 +6,8 @@ obj-y			:= lpar.o hvCall.o nvram.o reconfig.o \
>>   			   of_helpers.o \
>>   			   setup.o iommu.o event_sources.o ras.o \
>>   			   firmware.o power.o dlpar.o mobility.o rng.o \
>> -			   pci.o pci_dlpar.o eeh_pseries.o msi.o
>> +			   pci.o pci_dlpar.o eeh_pseries.o msi.o \
>> +			   papr_platform_attributes.o
>>   obj-$(CONFIG_SMP)	+= smp.o
>>   obj-$(CONFIG_SCANLOG)	+= scanlog.o
>>   obj-$(CONFIG_KEXEC_CORE)	+= kexec.o
>> diff --git a/arch/powerpc/platforms/pseries/papr_platform_attributes.c b/arch/powerpc/platforms/pseries/papr_platform_attributes.c
>> new file mode 100644
>> index 000000000000..d0373925ad32
>> --- /dev/null
>> +++ b/arch/powerpc/platforms/pseries/papr_platform_attributes.c
>> @@ -0,0 +1,312 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Platform energy and frequency attributes driver
>> + *
>> + * This driver creates a sys file at /sys/firmware/papr/ which encapsulates a
>> + * directory structure containing files in keyword - value pairs that specify
>> + * energy and frequency configuration of the system.
>> + *
>> + * The format of exposing the sysfs information is as follows:
>> + * /sys/firmware/papr/energy_scale_info/
>> + *  |-- <id>/
>> + *    |-- desc
>> + *    |-- value
>> + *    |-- value_desc (if exists)
>> + *  |-- <id>/
>> + *    |-- desc
>> + *    |-- value
>> + *    |-- value_desc (if exists)
>> + *
>> + * Copyright 2021 IBM Corp.
>> + */
>> +
>> +#include <asm/hvcall.h>
>> +#include <asm/machdep.h>
>> +
>> +#include "pseries.h"
>> +
>> +/*
>> + * Flag attributes to fetch either all or one attribute from the HCALL
>> + * flag = BE(0) => fetch all attributes with firstAttributeId = 0
>> + * flag = BE(1) => fetch a single attribute with firstAttributeId = id
>> + */
>> +#define ESI_FLAGS_ALL		0
>> +#define ESI_FLAGS_SINGLE	PPC_BIT(0)
>> +
>> +#define MAX_ATTRS		3
>> +
>> +struct papr_attr {
>> +	u64 id;
>> +	struct kobj_attribute kobj_attr;
>> +};
>> +struct papr_group {
>> +	struct attribute_group pg;
>> +	struct papr_attr pgattrs[MAX_ATTRS];
>> +} *pgs;
>> +
>> +/* /sys/firmware/papr */
>> +struct kobject *papr_kobj;
>> +/* /sys/firmware/papr/energy_scale_info */
>> +struct kobject *esi_kobj;
>> +
>> +/*
>> + * Extract and export the description of the energy scale attributes
>> + */
>> +static ssize_t papr_show_desc(struct kobject *kobj,
>> +			       struct kobj_attribute *kobj_attr,
>> +			       char *buf)
>> +{
>> +	struct papr_attr *pattr = container_of(kobj_attr, struct papr_attr,
>> +					       kobj_attr);
>> +	struct h_energy_scale_info_hdr *t_hdr;
>> +	struct energy_scale_attribute *t_esi;
>> +	char *t_buf;
>> +	int ret = 0;
>> +
>> +	t_buf = kmalloc(MAX_BUF_SZ, GFP_KERNEL);
>> +	if (t_buf == NULL)
>> +		return -ENOMEM;
>> +
>> +	ret = plpar_hcall_norets(H_GET_ENERGY_SCALE_INFO, ESI_FLAGS_SINGLE,
>> +				 pattr->id, virt_to_phys(t_buf),
>> +				 MAX_BUF_SZ);
>> +
>> +	if (ret != H_SUCCESS) {
>> +		pr_warn("hcall failed: H_GET_ENERGY_SCALE_INFO");
>> +		goto out;
>> +	}
>> +
>> +	t_hdr = (struct h_energy_scale_info_hdr *) t_buf;
>> +	t_esi = (struct energy_scale_attribute *)
>> +		(t_buf + be64_to_cpu(t_hdr->array_offset));
>> +
>> +	ret = snprintf(buf, sizeof(t_esi->desc), "%s\n", t_esi->desc);
>> +	if (ret < 0)
>> +		ret = -EIO;
>> +out:
>> +	kfree(t_buf);
>> +
>> +	return ret;
>> +}
>> +
>> +/*
>> + * Extract and export the numeric value of the energy scale attributes
>> + */
>> +static ssize_t papr_show_value(struct kobject *kobj,
>> +				struct kobj_attribute *kobj_attr,
>> +				char *buf)
>> +{
>> +	struct papr_attr *pattr = container_of(kobj_attr, struct papr_attr,
>> +					       kobj_attr);
>> +	struct h_energy_scale_info_hdr *t_hdr;
>> +	struct energy_scale_attribute *t_esi;
>> +	char *t_buf;
>> +	int ret = 0;
>> +
>> +	t_buf = kmalloc(MAX_BUF_SZ, GFP_KERNEL);
>> +	if (t_buf == NULL)
>> +		return -ENOMEM;
>> +
>> +	ret = plpar_hcall_norets(H_GET_ENERGY_SCALE_INFO, ESI_FLAGS_SINGLE,
>> +				 pattr->id, virt_to_phys(t_buf),
>> +				 MAX_BUF_SZ);
>> +
>> +	if (ret != H_SUCCESS) {
>> +		pr_warn("hcall failed: H_GET_ENERGY_SCALE_INFO");
>> +		goto out;
>> +	}
>> +
>> +	t_hdr = (struct h_energy_scale_info_hdr *) t_buf;
>> +	t_esi = (struct energy_scale_attribute *)
>> +		(t_buf + be64_to_cpu(t_hdr->array_offset));
>> +
>> +	ret = snprintf(buf, sizeof(t_esi->value), "%llu\n",
>> +		       be64_to_cpu(t_esi->value));
>> +	if (ret < 0)
>> +		ret = -EIO;
>> +out:
>> +	kfree(t_buf);
>> +
>> +	return ret;
>> +}
>> +
>> +/*
>> + * Extract and export the value description in string format of the energy
>> + * scale attributes
>> + */
>> +static ssize_t papr_show_value_desc(struct kobject *kobj,
>> +				     struct kobj_attribute *kobj_attr,
>> +				     char *buf)
>> +{
>> +	struct papr_attr *pattr = container_of(kobj_attr, struct papr_attr,
>> +					       kobj_attr);
>> +	struct h_energy_scale_info_hdr *t_hdr;
>> +	struct energy_scale_attribute *t_esi;
>> +	char *t_buf;
>> +	int ret = 0;
>> +
>> +	t_buf = kmalloc(MAX_BUF_SZ, GFP_KERNEL);
>> +	if (t_buf == NULL)
>> +		return -ENOMEM;
>> +
>> +	ret = plpar_hcall_norets(H_GET_ENERGY_SCALE_INFO, ESI_FLAGS_SINGLE,
>> +				 pattr->id, virt_to_phys(t_buf),
>> +				 MAX_BUF_SZ);
>> +
>> +	if (ret != H_SUCCESS) {
>> +		pr_warn("hcall failed: H_GET_ENERGY_SCALE_INFO");
>> +		goto out;
>> +	}
>> +
>> +	t_hdr = (struct h_energy_scale_info_hdr *) t_buf;
>> +	t_esi = (struct energy_scale_attribute *)
>> +		(t_buf + be64_to_cpu(t_hdr->array_offset));
>> +
>> +	ret = snprintf(buf, sizeof(t_esi->value_desc), "%s\n",
>> +		       t_esi->value_desc);
>> +	if (ret < 0)
>> +		ret = -EIO;
>> +out:
>> +	kfree(t_buf);
>> +
>> +	return ret;
>> +}
>> +
>> +static struct papr_ops_info {
>> +	const char *attr_name;
>> +	ssize_t (*show)(struct kobject *kobj, struct kobj_attribute *kobj_attr,
>> +			char *buf);
>> +} ops_info[MAX_ATTRS] = {
>> +	{ "desc", papr_show_desc },
>> +	{ "value", papr_show_value },
>> +	{ "value_desc", papr_show_value_desc },
>> +};
>> +
>> +static void add_attr(u64 id, int index, struct papr_attr *attr)
>> +{
>> +	attr->id = id;
>> +	sysfs_attr_init(&attr->kobj_attr.attr);
>> +	attr->kobj_attr.attr.name = ops_info[index].attr_name;
>> +	attr->kobj_attr.attr.mode = 0444;
>> +	attr->kobj_attr.show = ops_info[index].show;
>> +}
>> +
>> +static int add_attr_group(u64 id, struct papr_group *pg, bool show_val_desc)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < MAX_ATTRS; i++) {
>> +		if (!strcmp(ops_info[i].attr_name, "value_desc") &&
>> +		    !show_val_desc) {
>> +			continue;
>> +		}
>> +		add_attr(id, i, &pg->pgattrs[i]);
>> +		pg->pg.attrs[i] = &pg->pgattrs[i].kobj_attr.attr;
>> +	}
>> +
>> +	return sysfs_create_group(esi_kobj, &pg->pg);
>> +}
>> +
>> +static int __init papr_init(void)
>> +{
>> +	struct h_energy_scale_info_hdr *esi_hdr;
>> +	struct energy_scale_attribute *esi_attrs;
>> +	uint64_t num_attrs;
>> +	int ret, idx, i;
>> +	char *esi_buf;
>> +
>> +	if (!firmware_has_feature(FW_FEATURE_LPAR))
>> +		return -ENXIO;
>> +
>> +	esi_buf = kmalloc(MAX_BUF_SZ, GFP_KERNEL);
>> +	if (esi_buf == NULL)
>> +		return -ENOMEM;
>> +	/*
>> +	 * hcall(
>> +	 * uint64 H_GET_ENERGY_SCALE_INFO,  // Get energy scale info
>> +	 * uint64 flags,            // Per the flag request
>> +	 * uint64 firstAttributeId, // The attribute id
>> +	 * uint64 bufferAddress,    // Guest physical address of the output buffer
>> +	 * uint64 bufferSize);      // The size in bytes of the output buffer
>> +	 */
>> +	ret = plpar_hcall_norets(H_GET_ENERGY_SCALE_INFO, ESI_FLAGS_ALL, 0,
>> +				 virt_to_phys(esi_buf), MAX_BUF_SZ);
>> +	if (ret != H_SUCCESS) {
>> +		pr_warn("hcall failed: H_GET_ENERGY_SCALE_INFO");
>> +		goto out;
>> +	}
>> +
>> +	esi_hdr = (struct h_energy_scale_info_hdr *) esi_buf;
>> +	if (esi_hdr->data_header_version != ESI_VERSION) {
>> +		pr_warn("H_GET_ENERGY_SCALE_INFO VER MISMATCH - EXP: 0x%x, REC: 0x%x",
>> +			ESI_VERSION, esi_hdr->data_header_version);
>> +	}
>> +
>> +	num_attrs = be64_to_cpu(esi_hdr->num_attrs);
>> +	esi_attrs = (struct energy_scale_attribute *)
>> +		    (esi_buf + be64_to_cpu(esi_hdr->array_offset));
>> +
>> +	pgs = kcalloc(num_attrs, sizeof(*pgs), GFP_KERNEL);
>> +	if (!pgs)
>> +		goto out;
>> +
>> +	papr_kobj = kobject_create_and_add("papr", firmware_kobj);
>> +	if (!papr_kobj) {
>> +		pr_warn("kobject_create_and_add papr failed\n");
>> +		goto out_pgs;
>> +	}
>> +
>> +	esi_kobj = kobject_create_and_add("energy_scale_info", papr_kobj);
>> +	if (!esi_kobj) {
>> +		pr_warn("kobject_create_and_add energy_scale_info failed\n");
>> +		goto out_kobj;
>> +	}
>> +
>> +	for (idx = 0; idx < num_attrs; idx++) {
>> +		bool show_val_desc = true;
>> +
>> +		pgs[idx].pg.attrs = kcalloc(MAX_ATTRS + 1,
>> +					    sizeof(*pgs[idx].pg.attrs),
>> +					    GFP_KERNEL);
>> +		if (!pgs[idx].pg.attrs) {
>> +			goto out_pgattrs;
>> +		}
>> +
>> +		pgs[idx].pg.name = kasprintf(GFP_KERNEL, "%lld",
>> +					     be64_to_cpu(esi_attrs[idx].id));
>> +		if (pgs[idx].pg.name == NULL) {
>> +			goto out_pgattrs;
>> +		}
>> +		/* Do not add the value description if it does not exist */
>> +		if (strnlen(esi_attrs[idx].value_desc,
>> +			    sizeof(esi_attrs[idx].value_desc)) == 0)
>> +			show_val_desc = false;
>> +
>> +		if (add_attr_group(be64_to_cpu(esi_attrs[idx].id), &pgs[idx],
>> +				   show_val_desc)) {
>> +			pr_warn("Failed to create papr attribute group %s\n",
>> +				pgs[idx].pg.name);
>> +			goto out_pgattrs;
>> +		}
>> +	}
>> +
>> +	kfree(esi_buf);
>> +	return 0;
>> +
>> +out_pgattrs:
> Hi Pratik,
>          Just a nit optimization, Can't we  run this loop till idx rather then going all the way till
> num_attrs as you are allocating memory sequentially?

Thank you for your review.

Yeah you're right, if we fail I do bail out then and there and I can iterate
until idx instead of all the attributes.

~Pratik

> Thanks,
> Kajol Jain
>
>> +	for (i = 0; i < num_attrs ; i++) {
>> +		kfree(pgs[i].pg.attrs);
>> +		kfree(pgs[i].pg.name);
>> +	}
>> +	kobject_put(esi_kobj);
>> +out_kobj:
>> +	kobject_put(papr_kobj);
>> +out_pgs:
>> +	kfree(pgs);
>> +out:
>> +	kfree(esi_buf);
>> +
>> +	return -ENOMEM;
>> +}
>> +
>> +machine_device_initcall(pseries, papr_init);
>>


^ permalink raw reply

* Re: [PATCH v2] PCI: Move pci_dev_is/assign_added() to pci.h
From: kernel test robot @ 2021-07-20 13:48 UTC (permalink / raw)
  To: Niklas Schnelle, Bjorn Helgaas
  Cc: linux-arch, linux-s390, kbuild-all, linux-pci, linux-kernel,
	Paul Mackerras, linuxppc-dev
In-Reply-To: <20210720095816.3660813-1-schnelle@linux.ibm.com>

[-- Attachment #1: Type: text/plain, Size: 8262 bytes --]

Hi Niklas,

I love your patch! Yet something to improve:

[auto build test ERROR on pci/next]
[also build test ERROR on powerpc/next s390/features pm/linux-next v5.14-rc2 next-20210720]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Niklas-Schnelle/PCI-Move-pci_dev_is-assign_added-to-pci-h/20210720-180050
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: arm-buildonly-randconfig-r002-20210720 (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 10.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/52e04c39213cf20b8c6bc717cfda56103b871cd1
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Niklas-Schnelle/PCI-Move-pci_dev_is-assign_added-to-pci-h/20210720-180050
        git checkout 52e04c39213cf20b8c6bc717cfda56103b871cd1
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross O=build_dir ARCH=arm SHELL=/bin/bash drivers/pci/pcie/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/pci/pcie/dpc.c: In function 'dpc_completed':
>> drivers/pci/pcie/dpc.c:85:15: error: 'PCI_DPC_RECOVERING' undeclared (first use in this function)
      85 |  if (test_bit(PCI_DPC_RECOVERING, &pdev->priv_flags))
         |               ^~~~~~~~~~~~~~~~~~
   drivers/pci/pcie/dpc.c:85:15: note: each undeclared identifier is reported only once for each function it appears in
   In file included from include/linux/bitops.h:32,
                    from include/linux/kernel.h:12,
                    from include/linux/delay.h:22,
                    from drivers/pci/pcie/dpc.c:12:
   drivers/pci/pcie/dpc.c: In function 'pci_dpc_recovered':
>> drivers/pci/pcie/dpc.c:122:28: error: 'PCI_DPC_RECOVERED' undeclared (first use in this function)
     122 |  return test_and_clear_bit(PCI_DPC_RECOVERED, &pdev->priv_flags);
         |                            ^~~~~~~~~~~~~~~~~
   arch/arm/include/asm/bitops.h:181:24: note: in definition of macro 'ATOMIC_BITOP'
     181 |  (__builtin_constant_p(nr) ? ____atomic_##name(nr, p) : _##name(nr,p))
         |                        ^~
   drivers/pci/pcie/dpc.c:122:9: note: in expansion of macro 'test_and_clear_bit'
     122 |  return test_and_clear_bit(PCI_DPC_RECOVERED, &pdev->priv_flags);
         |         ^~~~~~~~~~~~~~~~~~
   drivers/pci/pcie/dpc.c: In function 'dpc_reset_link':
   drivers/pci/pcie/dpc.c:149:10: error: 'PCI_DPC_RECOVERING' undeclared (first use in this function)
     149 |  set_bit(PCI_DPC_RECOVERING, &pdev->priv_flags);
         |          ^~~~~~~~~~~~~~~~~~
   arch/arm/include/asm/bitops.h:181:24: note: in definition of macro 'ATOMIC_BITOP'
     181 |  (__builtin_constant_p(nr) ? ____atomic_##name(nr, p) : _##name(nr,p))
         |                        ^~
   drivers/pci/pcie/dpc.c:149:2: note: in expansion of macro 'set_bit'
     149 |  set_bit(PCI_DPC_RECOVERING, &pdev->priv_flags);
         |  ^~~~~~~
   drivers/pci/pcie/dpc.c:165:13: error: 'PCI_DPC_RECOVERED' undeclared (first use in this function)
     165 |   clear_bit(PCI_DPC_RECOVERED, &pdev->priv_flags);
         |             ^~~~~~~~~~~~~~~~~
   arch/arm/include/asm/bitops.h:181:24: note: in definition of macro 'ATOMIC_BITOP'
     181 |  (__builtin_constant_p(nr) ? ____atomic_##name(nr, p) : _##name(nr,p))
         |                        ^~
   drivers/pci/pcie/dpc.c:165:3: note: in expansion of macro 'clear_bit'
     165 |   clear_bit(PCI_DPC_RECOVERED, &pdev->priv_flags);
         |   ^~~~~~~~~
   drivers/pci/pcie/dpc.c: In function 'pci_dpc_recovered':
   drivers/pci/pcie/dpc.c:123:1: error: control reaches end of non-void function [-Werror=return-type]
     123 | }
         | ^
   cc1: some warnings being treated as errors


vim +/PCI_DPC_RECOVERING +85 drivers/pci/pcie/dpc.c

a97396c6eb13f65b Lukas Wunner 2021-05-01   75  
a97396c6eb13f65b Lukas Wunner 2021-05-01   76  #ifdef CONFIG_HOTPLUG_PCI_PCIE
a97396c6eb13f65b Lukas Wunner 2021-05-01   77  static bool dpc_completed(struct pci_dev *pdev)
a97396c6eb13f65b Lukas Wunner 2021-05-01   78  {
a97396c6eb13f65b Lukas Wunner 2021-05-01   79  	u16 status;
a97396c6eb13f65b Lukas Wunner 2021-05-01   80  
a97396c6eb13f65b Lukas Wunner 2021-05-01   81  	pci_read_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_STATUS, &status);
a97396c6eb13f65b Lukas Wunner 2021-05-01   82  	if ((status != 0xffff) && (status & PCI_EXP_DPC_STATUS_TRIGGER))
a97396c6eb13f65b Lukas Wunner 2021-05-01   83  		return false;
a97396c6eb13f65b Lukas Wunner 2021-05-01   84  
a97396c6eb13f65b Lukas Wunner 2021-05-01  @85  	if (test_bit(PCI_DPC_RECOVERING, &pdev->priv_flags))
a97396c6eb13f65b Lukas Wunner 2021-05-01   86  		return false;
a97396c6eb13f65b Lukas Wunner 2021-05-01   87  
a97396c6eb13f65b Lukas Wunner 2021-05-01   88  	return true;
a97396c6eb13f65b Lukas Wunner 2021-05-01   89  }
a97396c6eb13f65b Lukas Wunner 2021-05-01   90  
a97396c6eb13f65b Lukas Wunner 2021-05-01   91  /**
a97396c6eb13f65b Lukas Wunner 2021-05-01   92   * pci_dpc_recovered - whether DPC triggered and has recovered successfully
a97396c6eb13f65b Lukas Wunner 2021-05-01   93   * @pdev: PCI device
a97396c6eb13f65b Lukas Wunner 2021-05-01   94   *
a97396c6eb13f65b Lukas Wunner 2021-05-01   95   * Return true if DPC was triggered for @pdev and has recovered successfully.
a97396c6eb13f65b Lukas Wunner 2021-05-01   96   * Wait for recovery if it hasn't completed yet.  Called from the PCIe hotplug
a97396c6eb13f65b Lukas Wunner 2021-05-01   97   * driver to recognize and ignore Link Down/Up events caused by DPC.
a97396c6eb13f65b Lukas Wunner 2021-05-01   98   */
a97396c6eb13f65b Lukas Wunner 2021-05-01   99  bool pci_dpc_recovered(struct pci_dev *pdev)
a97396c6eb13f65b Lukas Wunner 2021-05-01  100  {
a97396c6eb13f65b Lukas Wunner 2021-05-01  101  	struct pci_host_bridge *host;
a97396c6eb13f65b Lukas Wunner 2021-05-01  102  
a97396c6eb13f65b Lukas Wunner 2021-05-01  103  	if (!pdev->dpc_cap)
a97396c6eb13f65b Lukas Wunner 2021-05-01  104  		return false;
a97396c6eb13f65b Lukas Wunner 2021-05-01  105  
a97396c6eb13f65b Lukas Wunner 2021-05-01  106  	/*
a97396c6eb13f65b Lukas Wunner 2021-05-01  107  	 * Synchronization between hotplug and DPC is not supported
a97396c6eb13f65b Lukas Wunner 2021-05-01  108  	 * if DPC is owned by firmware and EDR is not enabled.
a97396c6eb13f65b Lukas Wunner 2021-05-01  109  	 */
a97396c6eb13f65b Lukas Wunner 2021-05-01  110  	host = pci_find_host_bridge(pdev->bus);
a97396c6eb13f65b Lukas Wunner 2021-05-01  111  	if (!host->native_dpc && !IS_ENABLED(CONFIG_PCIE_EDR))
a97396c6eb13f65b Lukas Wunner 2021-05-01  112  		return false;
a97396c6eb13f65b Lukas Wunner 2021-05-01  113  
a97396c6eb13f65b Lukas Wunner 2021-05-01  114  	/*
a97396c6eb13f65b Lukas Wunner 2021-05-01  115  	 * Need a timeout in case DPC never completes due to failure of
a97396c6eb13f65b Lukas Wunner 2021-05-01  116  	 * dpc_wait_rp_inactive().  The spec doesn't mandate a time limit,
a97396c6eb13f65b Lukas Wunner 2021-05-01  117  	 * but reports indicate that DPC completes within 4 seconds.
a97396c6eb13f65b Lukas Wunner 2021-05-01  118  	 */
a97396c6eb13f65b Lukas Wunner 2021-05-01  119  	wait_event_timeout(dpc_completed_waitqueue, dpc_completed(pdev),
a97396c6eb13f65b Lukas Wunner 2021-05-01  120  			   msecs_to_jiffies(4000));
a97396c6eb13f65b Lukas Wunner 2021-05-01  121  
a97396c6eb13f65b Lukas Wunner 2021-05-01 @122  	return test_and_clear_bit(PCI_DPC_RECOVERED, &pdev->priv_flags);
a97396c6eb13f65b Lukas Wunner 2021-05-01  123  }
a97396c6eb13f65b Lukas Wunner 2021-05-01  124  #endif /* CONFIG_HOTPLUG_PCI_PCIE */
a97396c6eb13f65b Lukas Wunner 2021-05-01  125  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 40907 bytes --]

^ permalink raw reply

* [PATCH 0/2] KVM: PPC: Book3S HV: XIVE: Improve guest entries and exits
From: Cédric Le Goater @ 2021-07-20 13:42 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Paul Mackerras, Cédric Le Goater, kvm-ppc

The XIVE interrupt controller on P10 can automatically save and
restore the state of the interrupt registers under the internal NVP
structure representing the VCPU. This saves a costly store/load in
guest entries and exits.

Thanks,

C. 


Cédric Le Goater (2):
  KVM: PPC: Book3S HV: XIVE: Add a 'flags' field
  KVM: PPC: Book3S HV: XIVE: Add support for automatic save-restore

 arch/powerpc/include/asm/xive-regs.h  |  3 ++
 arch/powerpc/include/asm/xive.h       |  1 +
 arch/powerpc/kvm/book3s_xive.h        | 11 +++++-
 arch/powerpc/kvm/book3s_xive.c        | 53 +++++++++++++++++++++------
 arch/powerpc/kvm/book3s_xive_native.c | 21 ++++++++---
 arch/powerpc/sysdev/xive/native.c     | 10 +++++
 6 files changed, 82 insertions(+), 17 deletions(-)

-- 
2.31.1


^ permalink raw reply

* [PATCH 1/2] KVM: PPC: Book3S HV: XIVE: Add a 'flags' field
From: Cédric Le Goater @ 2021-07-20 13:42 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Paul Mackerras, Cédric Le Goater, kvm-ppc
In-Reply-To: <20210720134209.256133-1-clg@kaod.org>

Use it to hold platform specific features. P9 DD2 introduced
single-escalation support. P10 will add others.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 arch/powerpc/kvm/book3s_xive.h        |  9 ++++++++-
 arch/powerpc/kvm/book3s_xive.c        | 19 ++++++++++---------
 arch/powerpc/kvm/book3s_xive_native.c | 12 +++++++-----
 3 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_xive.h b/arch/powerpc/kvm/book3s_xive.h
index afe9eeac6d56..73c3cd25093c 100644
--- a/arch/powerpc/kvm/book3s_xive.h
+++ b/arch/powerpc/kvm/book3s_xive.h
@@ -97,6 +97,8 @@ struct kvmppc_xive_ops {
 	int (*reset_mapped)(struct kvm *kvm, unsigned long guest_irq);
 };
 
+#define KVMPPC_XIVE_FLAG_SINGLE_ESCALATION 0x1
+
 struct kvmppc_xive {
 	struct kvm *kvm;
 	struct kvm_device *dev;
@@ -133,7 +135,7 @@ struct kvmppc_xive {
 	u32	q_page_order;
 
 	/* Flags */
-	u8	single_escalation;
+	u8	flags;
 
 	/* Number of entries in the VP block */
 	u32	nr_servers;
@@ -308,5 +310,10 @@ void xive_cleanup_single_escalation(struct kvm_vcpu *vcpu,
 int kvmppc_xive_compute_vp_id(struct kvmppc_xive *xive, u32 cpu, u32 *vp);
 int kvmppc_xive_set_nr_servers(struct kvmppc_xive *xive, u64 addr);
 
+static inline bool kvmppc_xive_has_single_escalation(struct kvmppc_xive *xive)
+{
+	return xive->flags & KVMPPC_XIVE_FLAG_SINGLE_ESCALATION;
+}
+
 #endif /* CONFIG_KVM_XICS */
 #endif /* _KVM_PPC_BOOK3S_XICS_H */
diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
index 8cfab3547494..12f101d74b48 100644
--- a/arch/powerpc/kvm/book3s_xive.c
+++ b/arch/powerpc/kvm/book3s_xive.c
@@ -363,9 +363,9 @@ static int xive_check_provisioning(struct kvm *kvm, u8 prio)
 		if (!vcpu->arch.xive_vcpu)
 			continue;
 		rc = xive_provision_queue(vcpu, prio);
-		if (rc == 0 && !xive->single_escalation)
+		if (rc == 0 && !kvmppc_xive_has_single_escalation(xive))
 			kvmppc_xive_attach_escalation(vcpu, prio,
-						      xive->single_escalation);
+						      kvmppc_xive_has_single_escalation(xive));
 		if (rc)
 			return rc;
 	}
@@ -1199,7 +1199,7 @@ void kvmppc_xive_cleanup_vcpu(struct kvm_vcpu *vcpu)
 	/* Free escalations */
 	for (i = 0; i < KVMPPC_XIVE_Q_COUNT; i++) {
 		if (xc->esc_virq[i]) {
-			if (xc->xive->single_escalation)
+			if (kvmppc_xive_has_single_escalation(xc->xive))
 				xive_cleanup_single_escalation(vcpu, xc,
 							xc->esc_virq[i]);
 			free_irq(xc->esc_virq[i], vcpu);
@@ -1340,7 +1340,7 @@ int kvmppc_xive_connect_vcpu(struct kvm_device *dev,
 	 * Enable the VP first as the single escalation mode will
 	 * affect escalation interrupts numbering
 	 */
-	r = xive_native_enable_vp(xc->vp_id, xive->single_escalation);
+	r = xive_native_enable_vp(xc->vp_id, kvmppc_xive_has_single_escalation(xive));
 	if (r) {
 		pr_err("Failed to enable VP in OPAL, err %d\n", r);
 		goto bail;
@@ -1357,15 +1357,15 @@ int kvmppc_xive_connect_vcpu(struct kvm_device *dev,
 		struct xive_q *q = &xc->queues[i];
 
 		/* Single escalation, no queue 7 */
-		if (i == 7 && xive->single_escalation)
+		if (i == 7 && kvmppc_xive_has_single_escalation(xive))
 			break;
 
 		/* Is queue already enabled ? Provision it */
 		if (xive->qmap & (1 << i)) {
 			r = xive_provision_queue(vcpu, i);
-			if (r == 0 && !xive->single_escalation)
+			if (r == 0 && !kvmppc_xive_has_single_escalation(xive))
 				kvmppc_xive_attach_escalation(
-					vcpu, i, xive->single_escalation);
+					vcpu, i, kvmppc_xive_has_single_escalation(xive));
 			if (r)
 				goto bail;
 		} else {
@@ -1380,7 +1380,7 @@ int kvmppc_xive_connect_vcpu(struct kvm_device *dev,
 	}
 
 	/* If not done above, attach priority 0 escalation */
-	r = kvmppc_xive_attach_escalation(vcpu, 0, xive->single_escalation);
+	r = kvmppc_xive_attach_escalation(vcpu, 0, kvmppc_xive_has_single_escalation(xive));
 	if (r)
 		goto bail;
 
@@ -2135,7 +2135,8 @@ static int kvmppc_xive_create(struct kvm_device *dev, u32 type)
 	 */
 	xive->nr_servers = KVM_MAX_VCPUS;
 
-	xive->single_escalation = xive_native_has_single_escalation();
+	if (xive_native_has_single_escalation())
+		xive->flags |= KVMPPC_XIVE_FLAG_SINGLE_ESCALATION;
 
 	kvm->arch.xive = xive;
 	return 0;
diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
index 573ecaab3597..2abb1358a268 100644
--- a/arch/powerpc/kvm/book3s_xive_native.c
+++ b/arch/powerpc/kvm/book3s_xive_native.c
@@ -93,7 +93,7 @@ void kvmppc_xive_native_cleanup_vcpu(struct kvm_vcpu *vcpu)
 	for (i = 0; i < KVMPPC_XIVE_Q_COUNT; i++) {
 		/* Free the escalation irq */
 		if (xc->esc_virq[i]) {
-			if (xc->xive->single_escalation)
+			if (kvmppc_xive_has_single_escalation(xc->xive))
 				xive_cleanup_single_escalation(vcpu, xc,
 							xc->esc_virq[i]);
 			free_irq(xc->esc_virq[i], vcpu);
@@ -172,7 +172,7 @@ int kvmppc_xive_native_connect_vcpu(struct kvm_device *dev,
 	 * Enable the VP first as the single escalation mode will
 	 * affect escalation interrupts numbering
 	 */
-	rc = xive_native_enable_vp(xc->vp_id, xive->single_escalation);
+	rc = xive_native_enable_vp(xc->vp_id, kvmppc_xive_has_single_escalation(xive));
 	if (rc) {
 		pr_err("Failed to enable VP in OPAL: %d\n", rc);
 		goto bail;
@@ -693,7 +693,7 @@ static int kvmppc_xive_native_set_queue_config(struct kvmppc_xive *xive,
 	}
 
 	rc = kvmppc_xive_attach_escalation(vcpu, priority,
-					   xive->single_escalation);
+					   kvmppc_xive_has_single_escalation(xive));
 error:
 	if (rc)
 		kvmppc_xive_native_cleanup_queue(vcpu, priority);
@@ -820,7 +820,7 @@ static int kvmppc_xive_reset(struct kvmppc_xive *xive)
 		for (prio = 0; prio < KVMPPC_XIVE_Q_COUNT; prio++) {
 
 			/* Single escalation, no queue 7 */
-			if (prio == 7 && xive->single_escalation)
+			if (prio == 7 && kvmppc_xive_has_single_escalation(xive))
 				break;
 
 			if (xc->esc_virq[prio]) {
@@ -1111,7 +1111,9 @@ static int kvmppc_xive_native_create(struct kvm_device *dev, u32 type)
 	 */
 	xive->nr_servers = KVM_MAX_VCPUS;
 
-	xive->single_escalation = xive_native_has_single_escalation();
+	if (xive_native_has_single_escalation())
+		xive->flags |= KVMPPC_XIVE_FLAG_SINGLE_ESCALATION;
+
 	xive->ops = &kvmppc_xive_native_ops;
 
 	kvm->arch.xive = xive;
-- 
2.31.1


^ permalink raw reply related

* [PATCH 2/2] KVM: PPC: Book3S HV: XIVE: Add support for automatic save-restore
From: Cédric Le Goater @ 2021-07-20 13:42 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Paul Mackerras, Cédric Le Goater, kvm-ppc, Nicholas Piggin
In-Reply-To: <20210720134209.256133-1-clg@kaod.org>

On P10, the feature doing an automatic "save & restore" of a VCPU
interrupt context is set by default in OPAL. When a VP context is
pulled out, the state of the interrupt registers are saved by the XIVE
interrupt controller under the internal NVP structure representing the
VP. This saves a costly store/load in guest entries and exits.

If OPAL advertises the "save & restore" feature in the device tree,
it should also have set the 'H' bit in the CAM line. Check that when
vCPUs are connected to their ICP in KVM before going any further.

Cc: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 arch/powerpc/include/asm/xive-regs.h  |  3 +++
 arch/powerpc/include/asm/xive.h       |  1 +
 arch/powerpc/kvm/book3s_xive.h        |  2 ++
 arch/powerpc/kvm/book3s_xive.c        | 34 +++++++++++++++++++++++++--
 arch/powerpc/kvm/book3s_xive_native.c |  9 +++++++
 arch/powerpc/sysdev/xive/native.c     | 10 ++++++++
 6 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/xive-regs.h b/arch/powerpc/include/asm/xive-regs.h
index 8b211faa0e42..cf8bb6ac4463 100644
--- a/arch/powerpc/include/asm/xive-regs.h
+++ b/arch/powerpc/include/asm/xive-regs.h
@@ -80,10 +80,13 @@
 #define   TM_QW0W2_VU		PPC_BIT32(0)
 #define   TM_QW0W2_LOGIC_SERV	PPC_BITMASK32(1,31) // XX 2,31 ?
 #define   TM_QW1W2_VO		PPC_BIT32(0)
+#define   TM_QW1W2_HO           PPC_BIT32(1) /* P10 XIVE2 */
 #define   TM_QW1W2_OS_CAM	PPC_BITMASK32(8,31)
 #define   TM_QW2W2_VP		PPC_BIT32(0)
+#define   TM_QW2W2_HP           PPC_BIT32(1) /* P10 XIVE2 */
 #define   TM_QW2W2_POOL_CAM	PPC_BITMASK32(8,31)
 #define   TM_QW3W2_VT		PPC_BIT32(0)
+#define   TM_QW3W2_HT           PPC_BIT32(1) /* P10 XIVE2 */
 #define   TM_QW3W2_LP		PPC_BIT32(6)
 #define   TM_QW3W2_LE		PPC_BIT32(7)
 #define   TM_QW3W2_T		PPC_BIT32(31)
diff --git a/arch/powerpc/include/asm/xive.h b/arch/powerpc/include/asm/xive.h
index aa094a8655b0..efb0f5effcc6 100644
--- a/arch/powerpc/include/asm/xive.h
+++ b/arch/powerpc/include/asm/xive.h
@@ -125,6 +125,7 @@ int xive_native_enable_vp(u32 vp_id, bool single_escalation);
 int xive_native_disable_vp(u32 vp_id);
 int xive_native_get_vp_info(u32 vp_id, u32 *out_cam_id, u32 *out_chip_id);
 bool xive_native_has_single_escalation(void);
+bool xive_native_has_save_restore(void);
 
 int xive_native_get_queue_info(u32 vp_id, uint32_t prio,
 			       u64 *out_qpage,
diff --git a/arch/powerpc/kvm/book3s_xive.h b/arch/powerpc/kvm/book3s_xive.h
index 73c3cd25093c..e6a9651c6f1e 100644
--- a/arch/powerpc/kvm/book3s_xive.h
+++ b/arch/powerpc/kvm/book3s_xive.h
@@ -98,6 +98,7 @@ struct kvmppc_xive_ops {
 };
 
 #define KVMPPC_XIVE_FLAG_SINGLE_ESCALATION 0x1
+#define KVMPPC_XIVE_FLAG_SAVE_RESTORE 0x2
 
 struct kvmppc_xive {
 	struct kvm *kvm;
@@ -309,6 +310,7 @@ void xive_cleanup_single_escalation(struct kvm_vcpu *vcpu,
 				    struct kvmppc_xive_vcpu *xc, int irq);
 int kvmppc_xive_compute_vp_id(struct kvmppc_xive *xive, u32 cpu, u32 *vp);
 int kvmppc_xive_set_nr_servers(struct kvmppc_xive *xive, u64 addr);
+bool kvmppc_xive_check_save_restore(struct kvm_vcpu *vcpu);
 
 static inline bool kvmppc_xive_has_single_escalation(struct kvmppc_xive *xive)
 {
diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
index 12f101d74b48..cc5bee49bd63 100644
--- a/arch/powerpc/kvm/book3s_xive.c
+++ b/arch/powerpc/kvm/book3s_xive.c
@@ -59,6 +59,25 @@
  */
 #define XIVE_Q_GAP	2
 
+static bool kvmppc_xive_vcpu_has_save_restore(struct kvm_vcpu *vcpu)
+{
+	struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
+
+	/* Check enablement at VP level */
+	return xc->vp_cam & TM_QW1W2_HO;
+}
+
+bool kvmppc_xive_check_save_restore(struct kvm_vcpu *vcpu)
+{
+	struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
+	struct kvmppc_xive *xive = xc->xive;
+
+	if (xive->flags & KVMPPC_XIVE_FLAG_SAVE_RESTORE)
+		return kvmppc_xive_vcpu_has_save_restore(vcpu);
+
+	return true;
+}
+
 /*
  * Push a vcpu's context to the XIVE on guest entry.
  * This assumes we are in virtual mode (MMU on)
@@ -77,7 +96,8 @@ void kvmppc_xive_push_vcpu(struct kvm_vcpu *vcpu)
 		return;
 
 	eieio();
-	__raw_writeq(vcpu->arch.xive_saved_state.w01, tima + TM_QW1_OS);
+	if (!kvmppc_xive_vcpu_has_save_restore(vcpu))
+		__raw_writeq(vcpu->arch.xive_saved_state.w01, tima + TM_QW1_OS);
 	__raw_writel(vcpu->arch.xive_cam_word, tima + TM_QW1_OS + TM_WORD2);
 	vcpu->arch.xive_pushed = 1;
 	eieio();
@@ -149,7 +169,8 @@ void kvmppc_xive_pull_vcpu(struct kvm_vcpu *vcpu)
 	/* First load to pull the context, we ignore the value */
 	__raw_readl(tima + TM_SPC_PULL_OS_CTX);
 	/* Second load to recover the context state (Words 0 and 1) */
-	vcpu->arch.xive_saved_state.w01 = __raw_readq(tima + TM_QW1_OS);
+	if (!kvmppc_xive_vcpu_has_save_restore(vcpu))
+		vcpu->arch.xive_saved_state.w01 = __raw_readq(tima + TM_QW1_OS);
 
 	/* Fixup some of the state for the next load */
 	vcpu->arch.xive_saved_state.lsmfb = 0;
@@ -1319,6 +1340,12 @@ int kvmppc_xive_connect_vcpu(struct kvm_device *dev,
 	if (r)
 		goto bail;
 
+	if (!kvmppc_xive_check_save_restore(vcpu)) {
+		pr_err("inconsistent save-restore setup for VCPU %d\n", cpu);
+		r = -EIO;
+		goto bail;
+	}
+
 	/* Configure VCPU fields for use by assembly push/pull */
 	vcpu->arch.xive_saved_state.w01 = cpu_to_be64(0xff000000);
 	vcpu->arch.xive_cam_word = cpu_to_be32(xc->vp_cam | TM_QW1W2_VO);
@@ -2138,6 +2165,9 @@ static int kvmppc_xive_create(struct kvm_device *dev, u32 type)
 	if (xive_native_has_single_escalation())
 		xive->flags |= KVMPPC_XIVE_FLAG_SINGLE_ESCALATION;
 
+	if (xive_native_has_save_restore())
+		xive->flags |= KVMPPC_XIVE_FLAG_SAVE_RESTORE;
+
 	kvm->arch.xive = xive;
 	return 0;
 }
diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
index 2abb1358a268..af65ea21bde7 100644
--- a/arch/powerpc/kvm/book3s_xive_native.c
+++ b/arch/powerpc/kvm/book3s_xive_native.c
@@ -168,6 +168,12 @@ int kvmppc_xive_native_connect_vcpu(struct kvm_device *dev,
 		goto bail;
 	}
 
+	if (!kvmppc_xive_check_save_restore(vcpu)) {
+		pr_err("inconsistent save-restore setup for VCPU %d\n", server_num);
+		rc = -EIO;
+		goto bail;
+	}
+
 	/*
 	 * Enable the VP first as the single escalation mode will
 	 * affect escalation interrupts numbering
@@ -1114,6 +1120,9 @@ static int kvmppc_xive_native_create(struct kvm_device *dev, u32 type)
 	if (xive_native_has_single_escalation())
 		xive->flags |= KVMPPC_XIVE_FLAG_SINGLE_ESCALATION;
 
+	if (xive_native_has_save_restore())
+		xive->flags |= KVMPPC_XIVE_FLAG_SAVE_RESTORE;
+
 	xive->ops = &kvmppc_xive_native_ops;
 
 	kvm->arch.xive = xive;
diff --git a/arch/powerpc/sysdev/xive/native.c b/arch/powerpc/sysdev/xive/native.c
index fb15cecfe67c..b7ed436049f7 100644
--- a/arch/powerpc/sysdev/xive/native.c
+++ b/arch/powerpc/sysdev/xive/native.c
@@ -41,6 +41,7 @@ static u32 xive_queue_shift;
 static u32 xive_pool_vps = XIVE_INVALID_VP;
 static struct kmem_cache *xive_provision_cache;
 static bool xive_has_single_esc;
+static bool xive_has_save_restore;
 
 int xive_native_populate_irq_data(u32 hw_irq, struct xive_irq_data *data)
 {
@@ -592,6 +593,9 @@ bool __init xive_native_init(void)
 	if (of_get_property(np, "single-escalation-support", NULL) != NULL)
 		xive_has_single_esc = true;
 
+	if (of_get_property(np, "vp-save-restore", NULL))
+		xive_has_save_restore = true;
+
 	/* Configure Thread Management areas for KVM */
 	for_each_possible_cpu(cpu)
 		kvmppc_set_xive_tima(cpu, r.start, tima);
@@ -758,6 +762,12 @@ bool xive_native_has_single_escalation(void)
 }
 EXPORT_SYMBOL_GPL(xive_native_has_single_escalation);
 
+bool xive_native_has_save_restore(void)
+{
+	return xive_has_save_restore;
+}
+EXPORT_SYMBOL_GPL(xive_native_has_save_restore);
+
 int xive_native_get_queue_info(u32 vp_id, u32 prio,
 			       u64 *out_qpage,
 			       u64 *out_qsize,
-- 
2.31.1


^ permalink raw reply related

* Re: [PATCH v6 1/1] powerpc/pseries: Interface to represent PAPR firmware attributes
From: kajoljain @ 2021-07-20 13:32 UTC (permalink / raw)
  To: Pratik R. Sampat, mpe, benh, paulus, farosas, linuxppc-dev,
	kvm-ppc, linux-kernel, pratik.r.sampat
In-Reply-To: <20210720053434.32921-2-psampat@linux.ibm.com>



On 7/20/21 11:04 AM, Pratik R. Sampat wrote:
> Adds a generic interface to represent the energy and frequency related
> PAPR attributes on the system using the new H_CALL
> "H_GET_ENERGY_SCALE_INFO".
> 
> H_GET_EM_PARMS H_CALL was previously responsible for exporting this
> information in the lparcfg, however the H_GET_EM_PARMS H_CALL
> will be deprecated P10 onwards.
> 
> The H_GET_ENERGY_SCALE_INFO H_CALL is of the following call format:
> hcall(
>   uint64 H_GET_ENERGY_SCALE_INFO,  // Get energy scale info
>   uint64 flags,           // Per the flag request
>   uint64 firstAttributeId,// The attribute id
>   uint64 bufferAddress,   // Guest physical address of the output buffer
>   uint64 bufferSize       // The size in bytes of the output buffer
> );
> 
> This H_CALL can query either all the attributes at once with
> firstAttributeId = 0, flags = 0 as well as query only one attribute
> at a time with firstAttributeId = id, flags = 1.
> 
> The output buffer consists of the following
> 1. number of attributes              - 8 bytes
> 2. array offset to the data location - 8 bytes
> 3. version info                      - 1 byte
> 4. A data array of size num attributes, which contains the following:
>   a. attribute ID              - 8 bytes
>   b. attribute value in number - 8 bytes
>   c. attribute name in string  - 64 bytes
>   d. attribute value in string - 64 bytes
> 
> The new H_CALL exports information in direct string value format, hence
> a new interface has been introduced in
> /sys/firmware/papr/energy_scale_info to export this information to
> userspace in an extensible pass-through format.
> 
> The H_CALL returns the name, numeric value and string value (if exists)
> 
> The format of exposing the sysfs information is as follows:
> /sys/firmware/papr/energy_scale_info/
>    |-- <id>/
>      |-- desc
>      |-- value
>      |-- value_desc (if exists)
>    |-- <id>/
>      |-- desc
>      |-- value
>      |-- value_desc (if exists)
> ...
> 
> The energy information that is exported is useful for userspace tools
> such as powerpc-utils. Currently these tools infer the
> "power_mode_data" value in the lparcfg, which in turn is obtained from
> the to be deprecated H_GET_EM_PARMS H_CALL.
> On future platforms, such userspace utilities will have to look at the
> data returned from the new H_CALL being populated in this new sysfs
> interface and report this information directly without the need of
> interpretation.
> 
> Signed-off-by: Pratik R. Sampat <psampat@linux.ibm.com>
> Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> ---
>  .../sysfs-firmware-papr-energy-scale-info     |  26 ++
>  arch/powerpc/include/asm/hvcall.h             |  24 +-
>  arch/powerpc/kvm/trace_hv.h                   |   1 +
>  arch/powerpc/platforms/pseries/Makefile       |   3 +-
>  .../pseries/papr_platform_attributes.c        | 312 ++++++++++++++++++
>  5 files changed, 364 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-firmware-papr-energy-scale-info
>  create mode 100644 arch/powerpc/platforms/pseries/papr_platform_attributes.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-firmware-papr-energy-scale-info b/Documentation/ABI/testing/sysfs-firmware-papr-energy-scale-info
> new file mode 100644
> index 000000000000..139a576c7c9d
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-firmware-papr-energy-scale-info
> @@ -0,0 +1,26 @@
> +What:		/sys/firmware/papr/energy_scale_info
> +Date:		June 2021
> +Contact:	Linux for PowerPC mailing list <linuxppc-dev@ozlabs.org>
> +Description:	Directory hosting a set of platform attributes like
> +		energy/frequency on Linux running as a PAPR guest.
> +
> +		Each file in a directory contains a platform
> +		attribute hierarchy pertaining to performance/
> +		energy-savings mode and processor frequency.
> +
> +What:		/sys/firmware/papr/energy_scale_info/<id>
> +		/sys/firmware/papr/energy_scale_info/<id>/desc
> +		/sys/firmware/papr/energy_scale_info/<id>/value
> +		/sys/firmware/papr/energy_scale_info/<id>/value_desc
> +Date:		June 2021
> +Contact:	Linux for PowerPC mailing list <linuxppc-dev@ozlabs.org>
> +Description:	Energy, frequency attributes directory for POWERVM servers
> +
> +		This directory provides energy, frequency, folding information. It
> +		contains below sysfs attributes:
> +
> +		- desc: String description of the attribute <id>
> +
> +		- value: Numeric value of attribute <id>
> +
> +		- value_desc: String value of attribute <id>
> diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
> index e3b29eda8074..c91714ea6719 100644
> --- a/arch/powerpc/include/asm/hvcall.h
> +++ b/arch/powerpc/include/asm/hvcall.h
> @@ -316,7 +316,8 @@
>  #define H_SCM_PERFORMANCE_STATS 0x418
>  #define H_RPT_INVALIDATE	0x448
>  #define H_SCM_FLUSH		0x44C
> -#define MAX_HCALL_OPCODE	H_SCM_FLUSH
> +#define H_GET_ENERGY_SCALE_INFO	0x450
> +#define MAX_HCALL_OPCODE	H_GET_ENERGY_SCALE_INFO
>  
>  /* Scope args for H_SCM_UNBIND_ALL */
>  #define H_UNBIND_SCOPE_ALL (0x1)
> @@ -631,6 +632,27 @@ struct hv_gpci_request_buffer {
>  	uint8_t bytes[HGPCI_MAX_DATA_BYTES];
>  } __packed;
>  
> +#define ESI_VERSION	0x1
> +#define MAX_ESI_ATTRS	10
> +#define MAX_BUF_SZ	(sizeof(struct h_energy_scale_info_hdr) + \
> +			(sizeof(struct energy_scale_attribute) * MAX_ESI_ATTRS))
> +
> +struct energy_scale_attribute {
> +	__be64 id;
> +	__be64 value;
> +	unsigned char desc[64];
> +	unsigned char value_desc[64];
> +} __packed;
> +
> +struct h_energy_scale_info_hdr {
> +	__be64 num_attrs;
> +	__be64 array_offset;
> +	__u8 data_header_version;
> +} __packed;
> +
> +/* /sys/firmware/papr */
> +extern struct kobject *papr_kobj;
> +
>  #endif /* __ASSEMBLY__ */
>  #endif /* __KERNEL__ */
>  #endif /* _ASM_POWERPC_HVCALL_H */
> diff --git a/arch/powerpc/kvm/trace_hv.h b/arch/powerpc/kvm/trace_hv.h
> index 830a126e095d..38cd0ed0a617 100644
> --- a/arch/powerpc/kvm/trace_hv.h
> +++ b/arch/powerpc/kvm/trace_hv.h
> @@ -115,6 +115,7 @@
>  	{H_VASI_STATE,			"H_VASI_STATE"}, \
>  	{H_ENABLE_CRQ,			"H_ENABLE_CRQ"}, \
>  	{H_GET_EM_PARMS,		"H_GET_EM_PARMS"}, \
> +	{H_GET_ENERGY_SCALE_INFO,	"H_GET_ENERGY_SCALE_INFO"}, \
>  	{H_SET_MPP,			"H_SET_MPP"}, \
>  	{H_GET_MPP,			"H_GET_MPP"}, \
>  	{H_HOME_NODE_ASSOCIATIVITY,	"H_HOME_NODE_ASSOCIATIVITY"}, \
> diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile
> index c8a2b0b05ac0..d14fca89ac25 100644
> --- a/arch/powerpc/platforms/pseries/Makefile
> +++ b/arch/powerpc/platforms/pseries/Makefile
> @@ -6,7 +6,8 @@ obj-y			:= lpar.o hvCall.o nvram.o reconfig.o \
>  			   of_helpers.o \
>  			   setup.o iommu.o event_sources.o ras.o \
>  			   firmware.o power.o dlpar.o mobility.o rng.o \
> -			   pci.o pci_dlpar.o eeh_pseries.o msi.o
> +			   pci.o pci_dlpar.o eeh_pseries.o msi.o \
> +			   papr_platform_attributes.o
>  obj-$(CONFIG_SMP)	+= smp.o
>  obj-$(CONFIG_SCANLOG)	+= scanlog.o
>  obj-$(CONFIG_KEXEC_CORE)	+= kexec.o
> diff --git a/arch/powerpc/platforms/pseries/papr_platform_attributes.c b/arch/powerpc/platforms/pseries/papr_platform_attributes.c
> new file mode 100644
> index 000000000000..d0373925ad32
> --- /dev/null
> +++ b/arch/powerpc/platforms/pseries/papr_platform_attributes.c
> @@ -0,0 +1,312 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Platform energy and frequency attributes driver
> + *
> + * This driver creates a sys file at /sys/firmware/papr/ which encapsulates a
> + * directory structure containing files in keyword - value pairs that specify
> + * energy and frequency configuration of the system.
> + *
> + * The format of exposing the sysfs information is as follows:
> + * /sys/firmware/papr/energy_scale_info/
> + *  |-- <id>/
> + *    |-- desc
> + *    |-- value
> + *    |-- value_desc (if exists)
> + *  |-- <id>/
> + *    |-- desc
> + *    |-- value
> + *    |-- value_desc (if exists)
> + *
> + * Copyright 2021 IBM Corp.
> + */
> +
> +#include <asm/hvcall.h>
> +#include <asm/machdep.h>
> +
> +#include "pseries.h"
> +
> +/*
> + * Flag attributes to fetch either all or one attribute from the HCALL
> + * flag = BE(0) => fetch all attributes with firstAttributeId = 0
> + * flag = BE(1) => fetch a single attribute with firstAttributeId = id
> + */
> +#define ESI_FLAGS_ALL		0
> +#define ESI_FLAGS_SINGLE	PPC_BIT(0)
> +
> +#define MAX_ATTRS		3
> +
> +struct papr_attr {
> +	u64 id;
> +	struct kobj_attribute kobj_attr;
> +};
> +struct papr_group {
> +	struct attribute_group pg;
> +	struct papr_attr pgattrs[MAX_ATTRS];
> +} *pgs;
> +
> +/* /sys/firmware/papr */
> +struct kobject *papr_kobj;
> +/* /sys/firmware/papr/energy_scale_info */
> +struct kobject *esi_kobj;
> +
> +/*
> + * Extract and export the description of the energy scale attributes
> + */
> +static ssize_t papr_show_desc(struct kobject *kobj,
> +			       struct kobj_attribute *kobj_attr,
> +			       char *buf)
> +{
> +	struct papr_attr *pattr = container_of(kobj_attr, struct papr_attr,
> +					       kobj_attr);
> +	struct h_energy_scale_info_hdr *t_hdr;
> +	struct energy_scale_attribute *t_esi;
> +	char *t_buf;
> +	int ret = 0;
> +
> +	t_buf = kmalloc(MAX_BUF_SZ, GFP_KERNEL);
> +	if (t_buf == NULL)
> +		return -ENOMEM;
> +
> +	ret = plpar_hcall_norets(H_GET_ENERGY_SCALE_INFO, ESI_FLAGS_SINGLE,
> +				 pattr->id, virt_to_phys(t_buf),
> +				 MAX_BUF_SZ);
> +
> +	if (ret != H_SUCCESS) {
> +		pr_warn("hcall failed: H_GET_ENERGY_SCALE_INFO");
> +		goto out;
> +	}
> +
> +	t_hdr = (struct h_energy_scale_info_hdr *) t_buf;
> +	t_esi = (struct energy_scale_attribute *)
> +		(t_buf + be64_to_cpu(t_hdr->array_offset));
> +
> +	ret = snprintf(buf, sizeof(t_esi->desc), "%s\n", t_esi->desc);
> +	if (ret < 0)
> +		ret = -EIO;
> +out:
> +	kfree(t_buf);
> +
> +	return ret;
> +}
> +
> +/*
> + * Extract and export the numeric value of the energy scale attributes
> + */
> +static ssize_t papr_show_value(struct kobject *kobj,
> +				struct kobj_attribute *kobj_attr,
> +				char *buf)
> +{
> +	struct papr_attr *pattr = container_of(kobj_attr, struct papr_attr,
> +					       kobj_attr);
> +	struct h_energy_scale_info_hdr *t_hdr;
> +	struct energy_scale_attribute *t_esi;
> +	char *t_buf;
> +	int ret = 0;
> +
> +	t_buf = kmalloc(MAX_BUF_SZ, GFP_KERNEL);
> +	if (t_buf == NULL)
> +		return -ENOMEM;
> +
> +	ret = plpar_hcall_norets(H_GET_ENERGY_SCALE_INFO, ESI_FLAGS_SINGLE,
> +				 pattr->id, virt_to_phys(t_buf),
> +				 MAX_BUF_SZ);
> +
> +	if (ret != H_SUCCESS) {
> +		pr_warn("hcall failed: H_GET_ENERGY_SCALE_INFO");
> +		goto out;
> +	}
> +
> +	t_hdr = (struct h_energy_scale_info_hdr *) t_buf;
> +	t_esi = (struct energy_scale_attribute *)
> +		(t_buf + be64_to_cpu(t_hdr->array_offset));
> +
> +	ret = snprintf(buf, sizeof(t_esi->value), "%llu\n",
> +		       be64_to_cpu(t_esi->value));
> +	if (ret < 0)
> +		ret = -EIO;
> +out:
> +	kfree(t_buf);
> +
> +	return ret;
> +}
> +
> +/*
> + * Extract and export the value description in string format of the energy
> + * scale attributes
> + */
> +static ssize_t papr_show_value_desc(struct kobject *kobj,
> +				     struct kobj_attribute *kobj_attr,
> +				     char *buf)
> +{
> +	struct papr_attr *pattr = container_of(kobj_attr, struct papr_attr,
> +					       kobj_attr);
> +	struct h_energy_scale_info_hdr *t_hdr;
> +	struct energy_scale_attribute *t_esi;
> +	char *t_buf;
> +	int ret = 0;
> +
> +	t_buf = kmalloc(MAX_BUF_SZ, GFP_KERNEL);
> +	if (t_buf == NULL)
> +		return -ENOMEM;
> +
> +	ret = plpar_hcall_norets(H_GET_ENERGY_SCALE_INFO, ESI_FLAGS_SINGLE,
> +				 pattr->id, virt_to_phys(t_buf),
> +				 MAX_BUF_SZ);
> +
> +	if (ret != H_SUCCESS) {
> +		pr_warn("hcall failed: H_GET_ENERGY_SCALE_INFO");
> +		goto out;
> +	}
> +
> +	t_hdr = (struct h_energy_scale_info_hdr *) t_buf;
> +	t_esi = (struct energy_scale_attribute *)
> +		(t_buf + be64_to_cpu(t_hdr->array_offset));
> +
> +	ret = snprintf(buf, sizeof(t_esi->value_desc), "%s\n",
> +		       t_esi->value_desc);
> +	if (ret < 0)
> +		ret = -EIO;
> +out:
> +	kfree(t_buf);
> +
> +	return ret;
> +}
> +
> +static struct papr_ops_info {
> +	const char *attr_name;
> +	ssize_t (*show)(struct kobject *kobj, struct kobj_attribute *kobj_attr,
> +			char *buf);
> +} ops_info[MAX_ATTRS] = {
> +	{ "desc", papr_show_desc },
> +	{ "value", papr_show_value },
> +	{ "value_desc", papr_show_value_desc },
> +};
> +
> +static void add_attr(u64 id, int index, struct papr_attr *attr)
> +{
> +	attr->id = id;
> +	sysfs_attr_init(&attr->kobj_attr.attr);
> +	attr->kobj_attr.attr.name = ops_info[index].attr_name;
> +	attr->kobj_attr.attr.mode = 0444;
> +	attr->kobj_attr.show = ops_info[index].show;
> +}
> +
> +static int add_attr_group(u64 id, struct papr_group *pg, bool show_val_desc)
> +{
> +	int i;
> +
> +	for (i = 0; i < MAX_ATTRS; i++) {
> +		if (!strcmp(ops_info[i].attr_name, "value_desc") &&
> +		    !show_val_desc) {
> +			continue;
> +		}
> +		add_attr(id, i, &pg->pgattrs[i]);
> +		pg->pg.attrs[i] = &pg->pgattrs[i].kobj_attr.attr;
> +	}
> +
> +	return sysfs_create_group(esi_kobj, &pg->pg);
> +}
> +
> +static int __init papr_init(void)
> +{
> +	struct h_energy_scale_info_hdr *esi_hdr;
> +	struct energy_scale_attribute *esi_attrs;
> +	uint64_t num_attrs;
> +	int ret, idx, i;
> +	char *esi_buf;
> +
> +	if (!firmware_has_feature(FW_FEATURE_LPAR))
> +		return -ENXIO;
> +
> +	esi_buf = kmalloc(MAX_BUF_SZ, GFP_KERNEL);
> +	if (esi_buf == NULL)
> +		return -ENOMEM;
> +	/*
> +	 * hcall(
> +	 * uint64 H_GET_ENERGY_SCALE_INFO,  // Get energy scale info
> +	 * uint64 flags,            // Per the flag request
> +	 * uint64 firstAttributeId, // The attribute id
> +	 * uint64 bufferAddress,    // Guest physical address of the output buffer
> +	 * uint64 bufferSize);      // The size in bytes of the output buffer
> +	 */
> +	ret = plpar_hcall_norets(H_GET_ENERGY_SCALE_INFO, ESI_FLAGS_ALL, 0,
> +				 virt_to_phys(esi_buf), MAX_BUF_SZ);
> +	if (ret != H_SUCCESS) {
> +		pr_warn("hcall failed: H_GET_ENERGY_SCALE_INFO");
> +		goto out;
> +	}
> +
> +	esi_hdr = (struct h_energy_scale_info_hdr *) esi_buf;
> +	if (esi_hdr->data_header_version != ESI_VERSION) {
> +		pr_warn("H_GET_ENERGY_SCALE_INFO VER MISMATCH - EXP: 0x%x, REC: 0x%x",
> +			ESI_VERSION, esi_hdr->data_header_version);
> +	}
> +
> +	num_attrs = be64_to_cpu(esi_hdr->num_attrs);
> +	esi_attrs = (struct energy_scale_attribute *)
> +		    (esi_buf + be64_to_cpu(esi_hdr->array_offset));
> +
> +	pgs = kcalloc(num_attrs, sizeof(*pgs), GFP_KERNEL);
> +	if (!pgs)
> +		goto out;
> +
> +	papr_kobj = kobject_create_and_add("papr", firmware_kobj);
> +	if (!papr_kobj) {
> +		pr_warn("kobject_create_and_add papr failed\n");
> +		goto out_pgs;
> +	}
> +
> +	esi_kobj = kobject_create_and_add("energy_scale_info", papr_kobj);
> +	if (!esi_kobj) {
> +		pr_warn("kobject_create_and_add energy_scale_info failed\n");
> +		goto out_kobj;
> +	}
> +
> +	for (idx = 0; idx < num_attrs; idx++) {
> +		bool show_val_desc = true;
> +
> +		pgs[idx].pg.attrs = kcalloc(MAX_ATTRS + 1,
> +					    sizeof(*pgs[idx].pg.attrs),
> +					    GFP_KERNEL);
> +		if (!pgs[idx].pg.attrs) {
> +			goto out_pgattrs;
> +		}
> +
> +		pgs[idx].pg.name = kasprintf(GFP_KERNEL, "%lld",
> +					     be64_to_cpu(esi_attrs[idx].id));
> +		if (pgs[idx].pg.name == NULL) {
> +			goto out_pgattrs;
> +		}
> +		/* Do not add the value description if it does not exist */
> +		if (strnlen(esi_attrs[idx].value_desc,
> +			    sizeof(esi_attrs[idx].value_desc)) == 0)
> +			show_val_desc = false;
> +
> +		if (add_attr_group(be64_to_cpu(esi_attrs[idx].id), &pgs[idx],
> +				   show_val_desc)) {
> +			pr_warn("Failed to create papr attribute group %s\n",
> +				pgs[idx].pg.name);
> +			goto out_pgattrs;
> +		}
> +	}
> +
> +	kfree(esi_buf);
> +	return 0;
> +
> +out_pgattrs:

Hi Pratik,
        Just a nit optimization, Can't we  run this loop till idx rather then going all the way till
num_attrs as you are allocating memory sequentially?

Thanks,
Kajol Jain

> +	for (i = 0; i < num_attrs ; i++) {
> +		kfree(pgs[i].pg.attrs);
> +		kfree(pgs[i].pg.name);
> +	}
> +	kobject_put(esi_kobj);
> +out_kobj:
> +	kobject_put(papr_kobj);
> +out_pgs:
> +	kfree(pgs);
> +out:
> +	kfree(esi_buf);
> +
> +	return -ENOMEM;
> +}
> +
> +machine_device_initcall(pseries, papr_init);
> 

^ permalink raw reply

* Re: [PATCH 0/2] Fix arm64 boot regression in 5.14
From: Marc Zyngier @ 2021-07-20 12:53 UTC (permalink / raw)
  To: Will Deacon
  Cc: Mark Rutland, Catalin Marinas, Andrew Morton, Jonathan Marek,
	linux-kernel, Paul Mackerras, linux-arm-kernel,
	Geert Uytterhoeven, Thomas Gleixner, linuxppc-dev, Ard Biesheuvel,
	Mike Rapoport
In-Reply-To: <20210720123512.8740-1-will@kernel.org>

On 2021-07-20 13:35, Will Deacon wrote:
> Hi folks,
> 
> Jonathan reports [1] that commit c742199a014d ("mm/pgtable: add stubs
> for {pmd/pub}_{set/clear}_huge") breaks the boot on arm64 when huge
> mappings are used to map the kernel linear map but the VA size is
> configured such that PUDs are folded. This is because the 
> non-functional
> pud_set_huge() stub is used to create the linear map, which results in
> 1GB holes and a fatal data abort when the kernel attemps to access 
> them.
> 
> Digging further into the issue, it also transpired that huge-vmap is
> silently disabled in these configurations as well [2], despite working
> correctly in 5.13. The latter issue causes the pgtable selftests to
> scream due to a failing consistency check [3].
> 
> Rather than leave mainline in a terminally broken state for arm64 while
> we figure this out, revert the offending commit to get things working
> again. Unfortunately, reverting the change in isolation causes a build
> breakage for 32-bit PowerPC 8xx machines which recently started relying
> on the problematic stubs to support pte-level huge-vmap entries [4].
> Since Christophe is away at the moment, this series first reverts the
> PowerPC 8xx change in order to avoid breaking the build.
> 
> I would really like this to land for -rc3 and I can take these via the
> arm64 fixes queue if the PowerPC folks are alright with them.
> 
> Cheers,
> 
> Will
> 
> [1] https://lore.kernel.org/r/20210717160118.9855-1-jonathan@marek.ca
> [2] https://lore.kernel.org/r/20210719104918.GA6440@willie-the-truck
> [3] 
> https://lore.kernel.org/r/CAMuHMdXShORDox-xxaeUfDW3wx2PeggFSqhVSHVZNKCGK-y_vQ@mail.gmail.com/
> [4] 
> https://lore.kernel.org/r/8b972f1c03fb6bd59953035f0a3e4d26659de4f8.1620795204.git.christophe.leroy@csgroup.eu/
> 
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Jonathan Marek <jonathan@marek.ca>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Nicholas Piggin <npiggin@gmail.com
> Cc: Mike Rapoport <rppt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-arm-kernel@lists.infradead.org
> 
> --->8
> 
> Jonathan Marek (1):
>   Revert "mm/pgtable: add stubs for {pmd/pub}_{set/clear}_huge"
> 
> Will Deacon (1):
>   Revert "powerpc/8xx: add support for huge pages on VMAP and VMALLOC"
> 
>  arch/arm64/mm/mmu.c                          | 20 ++++-----
>  arch/powerpc/Kconfig                         |  2 +-
>  arch/powerpc/include/asm/nohash/32/mmu-8xx.h | 43 --------------------
>  arch/x86/mm/pgtable.c                        | 34 +++++++---------
>  include/linux/pgtable.h                      | 26 +-----------
>  5 files changed, 25 insertions(+), 100 deletions(-)

Acked-by: Marc Zyngier <maz@kernel.org>

         M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply


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