LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v13 4/8] powerpc/bpf: Remove bpf_jit_free()
From: Jordan Niethe @ 2021-05-10  1:18 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: ajd, cmr, npiggin, aneesh.kumar, naveen.n.rao, Jordan Niethe, dja
In-Reply-To: <20210510011828.4006623-1-jniethe5@gmail.com>

Commit 74451e66d516 ("bpf: make jited programs visible in traces") added
a default bpf_jit_free() implementation. Powerpc did not use the default
bpf_jit_free() as powerpc did not set the images read-only. The default
bpf_jit_free() called bpf_jit_binary_unlock_ro() is why it could not be
used for powerpc.

Commit d53d2f78cead ("bpf: Use vmalloc special flag") moved keeping
track of read-only memory to vmalloc. This included removing
bpf_jit_binary_unlock_ro(). Therefore there is no reason powerpc needs
its own bpf_jit_free(). Remove it.

Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v11: New to series
---
 arch/powerpc/net/bpf_jit_comp.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
index 798ac4350a82..6c8c268e4fe8 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -257,15 +257,3 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
 
 	return fp;
 }
-
-/* Overriding bpf_jit_free() as we don't set images read-only. */
-void bpf_jit_free(struct bpf_prog *fp)
-{
-	unsigned long addr = (unsigned long)fp->bpf_func & PAGE_MASK;
-	struct bpf_binary_header *bpf_hdr = (void *)addr;
-
-	if (fp->jited)
-		bpf_jit_binary_free(bpf_hdr);
-
-	bpf_prog_unlock_free(fp);
-}
-- 
2.25.1


^ permalink raw reply related

* [PATCH v13 5/8] powerpc/bpf: Write protect JIT code
From: Jordan Niethe @ 2021-05-10  1:18 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: ajd, cmr, npiggin, aneesh.kumar, naveen.n.rao, Jordan Niethe, dja
In-Reply-To: <20210510011828.4006623-1-jniethe5@gmail.com>

Add the necessary call to bpf_jit_binary_lock_ro() to remove write and
add exec permissions to the JIT image after it has finished being
written.

Without CONFIG_STRICT_MODULE_RWX the image will be writable and
executable until the call to bpf_jit_binary_lock_ro().

Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v10: New to series
v11: Remove CONFIG_STRICT_MODULE_RWX conditional
---
 arch/powerpc/net/bpf_jit_comp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
index 6c8c268e4fe8..53aefee3fe70 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -237,6 +237,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
 	fp->jited_len = alloclen;
 
 	bpf_flush_icache(bpf_hdr, (u8 *)bpf_hdr + (bpf_hdr->pages * PAGE_SIZE));
+	bpf_jit_binary_lock_ro(bpf_hdr);
 	if (!fp->is_func || extra_pass) {
 		bpf_prog_fill_jited_linfo(fp, addrs);
 out_addrs:
-- 
2.25.1


^ permalink raw reply related

* [PATCH v13 6/8] powerpc: Set ARCH_HAS_STRICT_MODULE_RWX
From: Jordan Niethe @ 2021-05-10  1:18 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: ajd, cmr, npiggin, aneesh.kumar, naveen.n.rao, Jordan Niethe, dja
In-Reply-To: <20210510011828.4006623-1-jniethe5@gmail.com>

From: Russell Currey <ruscur@russell.cc>

To enable strict module RWX on powerpc, set:

    CONFIG_STRICT_MODULE_RWX=y

You should also have CONFIG_STRICT_KERNEL_RWX=y set to have any real
security benefit.

ARCH_HAS_STRICT_MODULE_RWX is set to require ARCH_HAS_STRICT_KERNEL_RWX.
This is due to a quirk in arch/Kconfig and arch/powerpc/Kconfig that
makes STRICT_MODULE_RWX *on by default* in configurations where
STRICT_KERNEL_RWX is *unavailable*.

Since this doesn't make much sense, and module RWX without kernel RWX
doesn't make much sense, having the same dependencies as kernel RWX
works around this problem.

With STRICT_MODULE_RWX, now make module_alloc() allocate pages with
KERNEL_PAGE protection rather than KERNEL_PAGE_EXEC.

Book32s/32 processors with a hash mmu (i.e. 604 core) can not set memory
protection on a page by page basis so do not enable.

Signed-off-by: Russell Currey <ruscur@russell.cc>
[jpn: - predicate on !PPC_BOOK3S_604
      - make module_alloc() use PAGE_KERNEL protection]
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v10: - Predicate on !PPC_BOOK3S_604
     - Make module_alloc() use PAGE_KERNEL protection
v11: - Neaten up
v13: Use strict_kernel_rwx_enabled()
---
 arch/powerpc/Kconfig         | 1 +
 arch/powerpc/kernel/module.c | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index cce0a137b046..cb5d9d862c35 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -140,6 +140,7 @@ config PPC
 	select ARCH_HAS_SCALED_CPUTIME		if VIRT_CPU_ACCOUNTING_NATIVE && PPC_BOOK3S_64
 	select ARCH_HAS_SET_MEMORY
 	select ARCH_HAS_STRICT_KERNEL_RWX	if ((PPC_BOOK3S_64 || PPC32) && !HIBERNATION)
+	select ARCH_HAS_STRICT_MODULE_RWX	if ARCH_HAS_STRICT_KERNEL_RWX && !PPC_BOOK3S_604
 	select ARCH_HAS_TICK_BROADCAST		if GENERIC_CLOCKEVENTS_BROADCAST
 	select ARCH_HAS_UACCESS_FLUSHCACHE
 	select ARCH_HAS_COPY_MC			if PPC64
diff --git a/arch/powerpc/kernel/module.c b/arch/powerpc/kernel/module.c
index 3f35c8d20be7..f24004635ed5 100644
--- a/arch/powerpc/kernel/module.c
+++ b/arch/powerpc/kernel/module.c
@@ -92,12 +92,14 @@ int module_finalize(const Elf_Ehdr *hdr,
 static __always_inline void *
 __module_alloc(unsigned long size, unsigned long start, unsigned long end)
 {
+	pgprot_t prot = strict_kernel_rwx_enabled() ? PAGE_KERNEL : PAGE_KERNEL_EXEC;
+
 	/*
 	 * Don't do huge page allocations for modules yet until more testing
 	 * is done. STRICT_MODULE_RWX may require extra work to support this
 	 * too.
 	 */
-	return __vmalloc_node_range(size, 1, start, end, GFP_KERNEL, PAGE_KERNEL_EXEC,
+	return __vmalloc_node_range(size, 1, start, end, GFP_KERNEL, prot,
 				    VM_FLUSH_RESET_PERMS | VM_NO_HUGE_VMAP,
 				    NUMA_NO_NODE, __builtin_return_address(0));
 }
-- 
2.25.1


^ permalink raw reply related

* [PATCH v13 7/8] powerpc/mm: implement set_memory_attr()
From: Jordan Niethe @ 2021-05-10  1:18 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: ajd, cmr, kbuild test robot, npiggin, aneesh.kumar, naveen.n.rao,
	Jordan Niethe, dja
In-Reply-To: <20210510011828.4006623-1-jniethe5@gmail.com>

From: Christophe Leroy <christophe.leroy@csgroup.eu>

In addition to the set_memory_xx() functions which allows to change
the memory attributes of not (yet) used memory regions, implement a
set_memory_attr() function to:
- set the final memory protection after init on currently used
kernel regions.
- enable/disable kernel memory regions in the scope of DEBUG_PAGEALLOC.

Unlike the set_memory_xx() which can act in three step as the regions
are unused, this function must modify 'on the fly' as the kernel is
executing from them. At the moment only PPC32 will use it and changing
page attributes on the fly is not an issue.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Reported-by: kbuild test robot <lkp@intel.com>
[ruscur: cast "data" to unsigned long instead of int]
Signed-off-by: Russell Currey <ruscur@russell.cc>
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
 arch/powerpc/include/asm/set_memory.h |  2 ++
 arch/powerpc/mm/pageattr.c            | 33 +++++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/arch/powerpc/include/asm/set_memory.h b/arch/powerpc/include/asm/set_memory.h
index 64011ea444b4..b040094f7920 100644
--- a/arch/powerpc/include/asm/set_memory.h
+++ b/arch/powerpc/include/asm/set_memory.h
@@ -29,4 +29,6 @@ static inline int set_memory_x(unsigned long addr, int numpages)
 	return change_memory_attr(addr, numpages, SET_MEMORY_X);
 }
 
+int set_memory_attr(unsigned long addr, int numpages, pgprot_t prot);
+
 #endif
diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c
index acfdde78e0fb..7dee30cd6b48 100644
--- a/arch/powerpc/mm/pageattr.c
+++ b/arch/powerpc/mm/pageattr.c
@@ -98,3 +98,36 @@ int change_memory_attr(unsigned long addr, int numpages, long action)
 	return apply_to_existing_page_range(&init_mm, start, size,
 					    change_page_attr, (void *)action);
 }
+
+/*
+ * Set the attributes of a page:
+ *
+ * This function is used by PPC32 at the end of init to set final kernel memory
+ * protection. It includes changing the maping of the page it is executing from
+ * and data pages it is using.
+ */
+static int set_page_attr(pte_t *ptep, unsigned long addr, void *data)
+{
+	pgprot_t prot = __pgprot((unsigned long)data);
+
+	spin_lock(&init_mm.page_table_lock);
+
+	set_pte_at(&init_mm, addr, ptep, pte_modify(*ptep, prot));
+	flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
+
+	spin_unlock(&init_mm.page_table_lock);
+
+	return 0;
+}
+
+int set_memory_attr(unsigned long addr, int numpages, pgprot_t prot)
+{
+	unsigned long start = ALIGN_DOWN(addr, PAGE_SIZE);
+	unsigned long sz = numpages * PAGE_SIZE;
+
+	if (numpages <= 0)
+		return 0;
+
+	return apply_to_existing_page_range(&init_mm, start, sz, set_page_attr,
+					    (void *)pgprot_val(prot));
+}
-- 
2.25.1


^ permalink raw reply related

* [PATCH v13 8/8] powerpc/32: use set_memory_attr()
From: Jordan Niethe @ 2021-05-10  1:18 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: ajd, cmr, npiggin, aneesh.kumar, naveen.n.rao, Jordan Niethe, dja
In-Reply-To: <20210510011828.4006623-1-jniethe5@gmail.com>

From: Christophe Leroy <christophe.leroy@csgroup.eu>

Use set_memory_attr() instead of the PPC32 specific change_page_attr()

change_page_attr() was checking that the address was not mapped by
blocks and was handling highmem, but that's unneeded because the
affected pages can't be in highmem and block mapping verification
is already done by the callers.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
[ruscur: rebase on powerpc/merge with Christophe's new patches]
Signed-off-by: Russell Currey <ruscur@russell.cc>
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
 arch/powerpc/mm/pgtable_32.c | 60 ++++++------------------------------
 1 file changed, 10 insertions(+), 50 deletions(-)

diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
index e0ec67a16887..dcf5ecca19d9 100644
--- a/arch/powerpc/mm/pgtable_32.c
+++ b/arch/powerpc/mm/pgtable_32.c
@@ -23,6 +23,7 @@
 #include <linux/highmem.h>
 #include <linux/memblock.h>
 #include <linux/slab.h>
+#include <linux/set_memory.h>
 
 #include <asm/pgalloc.h>
 #include <asm/fixmap.h>
@@ -132,64 +133,20 @@ void __init mapin_ram(void)
 	}
 }
 
-static int __change_page_attr_noflush(struct page *page, pgprot_t prot)
-{
-	pte_t *kpte;
-	unsigned long address;
-
-	BUG_ON(PageHighMem(page));
-	address = (unsigned long)page_address(page);
-
-	if (v_block_mapped(address))
-		return 0;
-	kpte = virt_to_kpte(address);
-	if (!kpte)
-		return -EINVAL;
-	__set_pte_at(&init_mm, address, kpte, mk_pte(page, prot), 0);
-
-	return 0;
-}
-
-/*
- * Change the page attributes of an page in the linear mapping.
- *
- * THIS DOES NOTHING WITH BAT MAPPINGS, DEBUG USE ONLY
- */
-static int change_page_attr(struct page *page, int numpages, pgprot_t prot)
-{
-	int i, err = 0;
-	unsigned long flags;
-	struct page *start = page;
-
-	local_irq_save(flags);
-	for (i = 0; i < numpages; i++, page++) {
-		err = __change_page_attr_noflush(page, prot);
-		if (err)
-			break;
-	}
-	wmb();
-	local_irq_restore(flags);
-	flush_tlb_kernel_range((unsigned long)page_address(start),
-			       (unsigned long)page_address(page));
-	return err;
-}
-
 void mark_initmem_nx(void)
 {
-	struct page *page = virt_to_page(_sinittext);
 	unsigned long numpages = PFN_UP((unsigned long)_einittext) -
 				 PFN_DOWN((unsigned long)_sinittext);
 
 	if (v_block_mapped((unsigned long)_sinittext))
 		mmu_mark_initmem_nx();
 	else
-		change_page_attr(page, numpages, PAGE_KERNEL);
+		set_memory_attr((unsigned long)_sinittext, numpages, PAGE_KERNEL);
 }
 
 #ifdef CONFIG_STRICT_KERNEL_RWX
 void mark_rodata_ro(void)
 {
-	struct page *page;
 	unsigned long numpages;
 
 	if (v_block_mapped((unsigned long)_stext + 1)) {
@@ -198,20 +155,18 @@ void mark_rodata_ro(void)
 		return;
 	}
 
-	page = virt_to_page(_stext);
 	numpages = PFN_UP((unsigned long)_etext) -
 		   PFN_DOWN((unsigned long)_stext);
 
-	change_page_attr(page, numpages, PAGE_KERNEL_ROX);
+	set_memory_attr((unsigned long)_stext, numpages, PAGE_KERNEL_ROX);
 	/*
 	 * mark .rodata as read only. Use __init_begin rather than __end_rodata
 	 * to cover NOTES and EXCEPTION_TABLE.
 	 */
-	page = virt_to_page(__start_rodata);
 	numpages = PFN_UP((unsigned long)__init_begin) -
 		   PFN_DOWN((unsigned long)__start_rodata);
 
-	change_page_attr(page, numpages, PAGE_KERNEL_RO);
+	set_memory_attr((unsigned long)__start_rodata, numpages, PAGE_KERNEL_RO);
 
 	// mark_initmem_nx() should have already run by now
 	ptdump_check_wx();
@@ -221,9 +176,14 @@ void mark_rodata_ro(void)
 #ifdef CONFIG_DEBUG_PAGEALLOC
 void __kernel_map_pages(struct page *page, int numpages, int enable)
 {
+	unsigned long addr = (unsigned long)page_address(page);
+
 	if (PageHighMem(page))
 		return;
 
-	change_page_attr(page, numpages, enable ? PAGE_KERNEL : __pgprot(0));
+	if (enable)
+		set_memory_attr(addr, numpages, PAGE_KERNEL);
+	else
+		set_memory_attr(addr, numpages, __pgprot(0));
 }
 #endif /* CONFIG_DEBUG_PAGEALLOC */
-- 
2.25.1


^ permalink raw reply related

* [PATCH 03/11] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
From: Krzysztof Wilczyński @ 2021-05-10  4:14 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Tyrel Datwyler, linux-pci, linuxppc-dev, Oliver O'Halloran,
	Paul Mackerras, Kurt Schwemmer, Logan Gunthorpe
In-Reply-To: <20210510041424.233565-1-kw@linux.com>

The sysfs_emit() and sysfs_emit_at() functions were introduced to make
it less ambiguous which function is preferred when writing to the output
buffer in a device attribute's "show" callback [1].

Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
latter is aware of the PAGE_SIZE buffer and correctly returns the number
of bytes written into the buffer.

Modify the function dsm_label_utf16s_to_utf8s() to directly return the
number of bytes written into the buffer so that the strlen() used later
to calculate the length of the buffer can be removed as it would no
longer be needed.

No functional change intended.

[1] Documentation/filesystems/sysfs.rst

Related to:
  commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
---
 drivers/pci/pci-label.c | 18 ++++++++++--------
 drivers/pci/slot.c      | 16 ++++++++--------
 2 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/drivers/pci/pci-label.c b/drivers/pci/pci-label.c
index c32f3b7540e8..000e169c7197 100644
--- a/drivers/pci/pci-label.c
+++ b/drivers/pci/pci-label.c
@@ -139,14 +139,17 @@ enum acpi_attr_enum {
 	ACPI_ATTR_INDEX_SHOW,
 };
 
-static void dsm_label_utf16s_to_utf8s(union acpi_object *obj, char *buf)
+static int dsm_label_utf16s_to_utf8s(union acpi_object *obj, char *buf)
 {
 	int len;
+
 	len = utf16s_to_utf8s((const wchar_t *)obj->buffer.pointer,
 			      obj->buffer.length,
 			      UTF16_LITTLE_ENDIAN,
 			      buf, PAGE_SIZE);
 	buf[len] = '\n';
+
+	return len;
 }
 
 static int dsm_get_label(struct device *dev, char *buf,
@@ -154,7 +157,7 @@ static int dsm_get_label(struct device *dev, char *buf,
 {
 	acpi_handle handle = ACPI_HANDLE(dev);
 	union acpi_object *obj, *tmp;
-	int len = -1;
+	int len = 0;
 
 	if (!handle)
 		return -1;
@@ -175,20 +178,19 @@ static int dsm_get_label(struct device *dev, char *buf,
 		 * this entry must return a null string.
 		 */
 		if (attr == ACPI_ATTR_INDEX_SHOW) {
-			scnprintf(buf, PAGE_SIZE, "%llu\n", tmp->integer.value);
+			len = sysfs_emit(buf, "%llu\n", tmp->integer.value);
 		} else if (attr == ACPI_ATTR_LABEL_SHOW) {
 			if (tmp[1].type == ACPI_TYPE_STRING)
-				scnprintf(buf, PAGE_SIZE, "%s\n",
-					  tmp[1].string.pointer);
+				len = sysfs_emit(buf, "%s\n",
+						 tmp[1].string.pointer);
 			else if (tmp[1].type == ACPI_TYPE_BUFFER)
-				dsm_label_utf16s_to_utf8s(tmp + 1, buf);
+				len = dsm_label_utf16s_to_utf8s(tmp + 1, buf);
 		}
-		len = strlen(buf) > 0 ? strlen(buf) : -1;
 	}
 
 	ACPI_FREE(obj);
 
-	return len;
+	return len > 0 ? len : -1;
 }
 
 static ssize_t label_show(struct device *dev, struct device_attribute *attr,
diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c
index d627dd9179b4..7487e8f8f13f 100644
--- a/drivers/pci/slot.c
+++ b/drivers/pci/slot.c
@@ -39,19 +39,19 @@ static const struct sysfs_ops pci_slot_sysfs_ops = {
 static ssize_t address_read_file(struct pci_slot *slot, char *buf)
 {
 	if (slot->number == 0xff)
-		return sprintf(buf, "%04x:%02x\n",
-				pci_domain_nr(slot->bus),
-				slot->bus->number);
+		return sysfs_emit(buf, "%04x:%02x\n",
+				  pci_domain_nr(slot->bus),
+				  slot->bus->number);
 	else
-		return sprintf(buf, "%04x:%02x:%02x\n",
-				pci_domain_nr(slot->bus),
-				slot->bus->number,
-				slot->number);
+		return sysfs_emit(buf, "%04x:%02x:%02x\n",
+				  pci_domain_nr(slot->bus),
+				  slot->bus->number,
+				  slot->number);
 }
 
 static ssize_t bus_speed_read(enum pci_bus_speed speed, char *buf)
 {
-	return sprintf(buf, "%s\n", pci_speed_string(speed));
+	return sysfs_emit(buf, "%s\n", pci_speed_string(speed));
 }
 
 static ssize_t max_speed_read_file(struct pci_slot *slot, char *buf)
-- 
2.31.1


^ permalink raw reply related

* [PATCH 01/11] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
From: Krzysztof Wilczyński @ 2021-05-10  4:14 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Tyrel Datwyler, linux-pci, linuxppc-dev, Oliver O'Halloran,
	Paul Mackerras, Kurt Schwemmer, Logan Gunthorpe

The sysfs_emit() and sysfs_emit_at() functions were introduced to make
it less ambiguous which function is preferred when writing to the output
buffer in a device attribute's "show" callback [1].

Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
latter is aware of the PAGE_SIZE buffer and correctly returns the number
of bytes written into the buffer.

No functional change intended.

[1] Documentation/filesystems/sysfs.rst

Related to:
  commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
---
 drivers/pci/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b717680377a9..5ed316ea5831 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -6439,7 +6439,7 @@ static ssize_t resource_alignment_show(struct bus_type *bus, char *buf)
 
 	spin_lock(&resource_alignment_lock);
 	if (resource_alignment_param)
-		count = scnprintf(buf, PAGE_SIZE, "%s", resource_alignment_param);
+		count = sysfs_emit(buf, "%s", resource_alignment_param);
 	spin_unlock(&resource_alignment_lock);
 
 	/*
-- 
2.31.1


^ permalink raw reply related

* [PATCH 02/11] PCI/AER: Use sysfs_emit() and sysfs_emit_at() in "show" functions
From: Krzysztof Wilczyński @ 2021-05-10  4:14 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Tyrel Datwyler, linux-pci, linuxppc-dev, Oliver O'Halloran,
	Paul Mackerras, Kurt Schwemmer, Logan Gunthorpe
In-Reply-To: <20210510041424.233565-1-kw@linux.com>

The sysfs_emit() and sysfs_emit_at() functions were introduced to make
it less ambiguous which function is preferred when writing to the output
buffer in a device attribute's "show" callback [1].

Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
latter is aware of the PAGE_SIZE buffer and correctly returns the number
of bytes written into the buffer.

No functional change intended.

[1] Documentation/filesystems/sysfs.rst

Related to:
  commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
---
 drivers/pci/pcie/aer.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index ec943cee5ecc..40ef7bed7a77 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -529,21 +529,23 @@ static const char *aer_agent_string[] = {
 		     char *buf)						\
 {									\
 	unsigned int i;							\
-	char *str = buf;						\
 	struct pci_dev *pdev = to_pci_dev(dev);				\
 	u64 *stats = pdev->aer_stats->stats_array;			\
+	size_t len = 0;							\
 									\
 	for (i = 0; i < ARRAY_SIZE(strings_array); i++) {		\
 		if (strings_array[i])					\
-			str += sprintf(str, "%s %llu\n",		\
-				       strings_array[i], stats[i]);	\
+			len += sysfs_emit_at(buf, len, "%s %llu\n",	\
+					     strings_array[i],		\
+					     stats[i]);			\
 		else if (stats[i])					\
-			str += sprintf(str, #stats_array "_bit[%d] %llu\n",\
-				       i, stats[i]);			\
+			len += sysfs_emit_at(buf, len,			\
+					     #stats_array "_bit[%d] %llu\n",\
+					     i, stats[i]);		\
 	}								\
-	str += sprintf(str, "TOTAL_%s %llu\n", total_string,		\
-		       pdev->aer_stats->total_field);			\
-	return str-buf;							\
+	len += sysfs_emit_at(buf, len, "TOTAL_%s %llu\n", total_string,	\
+			     pdev->aer_stats->total_field);		\
+	return len;							\
 }									\
 static DEVICE_ATTR_RO(name)
 
@@ -563,7 +565,7 @@ aer_stats_dev_attr(aer_dev_nonfatal, dev_nonfatal_errs,
 		     char *buf)						\
 {									\
 	struct pci_dev *pdev = to_pci_dev(dev);				\
-	return sprintf(buf, "%llu\n", pdev->aer_stats->field);		\
+	return sysfs_emit(buf, "%llu\n", pdev->aer_stats->field);	\
 }									\
 static DEVICE_ATTR_RO(name)
 
-- 
2.31.1


^ permalink raw reply related

* [PATCH 04/11] PCI/MSI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
From: Krzysztof Wilczyński @ 2021-05-10  4:14 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Tyrel Datwyler, linux-pci, linuxppc-dev, Oliver O'Halloran,
	Paul Mackerras, Kurt Schwemmer, Logan Gunthorpe
In-Reply-To: <20210510041424.233565-1-kw@linux.com>

The sysfs_emit() and sysfs_emit_at() functions were introduced to make
it less ambiguous which function is preferred when writing to the output
buffer in a device attribute's "show" callback [1].

Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
latter is aware of the PAGE_SIZE buffer and correctly returns the number
of bytes written into the buffer.

No functional change intended.

[1] Documentation/filesystems/sysfs.rst

Related to:
  commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
---
 drivers/pci/msi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 217dc9f0231f..dbfec59dfe41 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -465,8 +465,8 @@ static ssize_t msi_mode_show(struct device *dev, struct device_attribute *attr,
 
 	entry = irq_get_msi_desc(irq);
 	if (entry)
-		return sprintf(buf, "%s\n",
-				entry->msi_attrib.is_msix ? "msix" : "msi");
+		return sysfs_emit(buf, "%s\n",
+				  entry->msi_attrib.is_msix ? "msix" : "msi");
 
 	return -ENODEV;
 }
-- 
2.31.1


^ permalink raw reply related

* [PATCH 05/11] PCI/IOV: Use sysfs_emit() and sysfs_emit_at() in "show" functions
From: Krzysztof Wilczyński @ 2021-05-10  4:14 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Tyrel Datwyler, linux-pci, linuxppc-dev, Oliver O'Halloran,
	Paul Mackerras, Kurt Schwemmer, Logan Gunthorpe
In-Reply-To: <20210510041424.233565-1-kw@linux.com>

The sysfs_emit() and sysfs_emit_at() functions were introduced to make
it less ambiguous which function is preferred when writing to the output
buffer in a device attribute's "show" callback [1].

Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
latter is aware of the PAGE_SIZE buffer and correctly returns the number
of bytes written into the buffer.

No functional change intended.

[1] Documentation/filesystems/sysfs.rst

Related to:
  commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
---
 drivers/pci/iov.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index afc06e6ce115..a71258347323 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -346,7 +346,7 @@ static ssize_t sriov_totalvfs_show(struct device *dev,
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 
-	return sprintf(buf, "%u\n", pci_sriov_get_totalvfs(pdev));
+	return sysfs_emit(buf, "%u\n", pci_sriov_get_totalvfs(pdev));
 }
 
 static ssize_t sriov_numvfs_show(struct device *dev,
@@ -361,7 +361,7 @@ static ssize_t sriov_numvfs_show(struct device *dev,
 	num_vfs = pdev->sriov->num_VFs;
 	device_unlock(&pdev->dev);
 
-	return sprintf(buf, "%u\n", num_vfs);
+	return sysfs_emit(buf, "%u\n", num_vfs);
 }
 
 /*
@@ -435,7 +435,7 @@ static ssize_t sriov_offset_show(struct device *dev,
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 
-	return sprintf(buf, "%u\n", pdev->sriov->offset);
+	return sysfs_emit(buf, "%u\n", pdev->sriov->offset);
 }
 
 static ssize_t sriov_stride_show(struct device *dev,
@@ -444,7 +444,7 @@ static ssize_t sriov_stride_show(struct device *dev,
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 
-	return sprintf(buf, "%u\n", pdev->sriov->stride);
+	return sysfs_emit(buf, "%u\n", pdev->sriov->stride);
 }
 
 static ssize_t sriov_vf_device_show(struct device *dev,
@@ -453,7 +453,7 @@ static ssize_t sriov_vf_device_show(struct device *dev,
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 
-	return sprintf(buf, "%x\n", pdev->sriov->vf_device);
+	return sysfs_emit(buf, "%x\n", pdev->sriov->vf_device);
 }
 
 static ssize_t sriov_drivers_autoprobe_show(struct device *dev,
@@ -462,7 +462,7 @@ static ssize_t sriov_drivers_autoprobe_show(struct device *dev,
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 
-	return sprintf(buf, "%u\n", pdev->sriov->drivers_autoprobe);
+	return sysfs_emit(buf, "%u\n", pdev->sriov->drivers_autoprobe);
 }
 
 static ssize_t sriov_drivers_autoprobe_store(struct device *dev,
-- 
2.31.1


^ permalink raw reply related

* [PATCH 06/11] PCI/P2PDMA: Use sysfs_emit() and sysfs_emit_at() in "show" functions
From: Krzysztof Wilczyński @ 2021-05-10  4:14 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Tyrel Datwyler, linux-pci, linuxppc-dev, Oliver O'Halloran,
	Paul Mackerras, Kurt Schwemmer, Logan Gunthorpe
In-Reply-To: <20210510041424.233565-1-kw@linux.com>

The sysfs_emit() and sysfs_emit_at() functions were introduced to make
it less ambiguous which function is preferred when writing to the output
buffer in a device attribute's "show" callback [1].

Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
latter is aware of the PAGE_SIZE buffer and correctly returns the number
of bytes written into the buffer.

No functional change intended.

[1] Documentation/filesystems/sysfs.rst

Related to:
  commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
---
 drivers/pci/p2pdma.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
index 196382630363..a1351b3e2c4c 100644
--- a/drivers/pci/p2pdma.c
+++ b/drivers/pci/p2pdma.c
@@ -53,7 +53,7 @@ static ssize_t size_show(struct device *dev, struct device_attribute *attr,
 	if (pdev->p2pdma->pool)
 		size = gen_pool_size(pdev->p2pdma->pool);
 
-	return scnprintf(buf, PAGE_SIZE, "%zd\n", size);
+	return sysfs_emit(buf, "%zd\n", size);
 }
 static DEVICE_ATTR_RO(size);
 
@@ -66,7 +66,7 @@ static ssize_t available_show(struct device *dev, struct device_attribute *attr,
 	if (pdev->p2pdma->pool)
 		avail = gen_pool_avail(pdev->p2pdma->pool);
 
-	return scnprintf(buf, PAGE_SIZE, "%zd\n", avail);
+	return sysfs_emit(buf, "%zd\n", avail);
 }
 static DEVICE_ATTR_RO(available);
 
@@ -75,8 +75,7 @@ static ssize_t published_show(struct device *dev, struct device_attribute *attr,
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 
-	return scnprintf(buf, PAGE_SIZE, "%d\n",
-			 pdev->p2pdma->p2pmem_published);
+	return sysfs_emit(buf, "%d\n", pdev->p2pdma->p2pmem_published);
 }
 static DEVICE_ATTR_RO(published);
 
-- 
2.31.1


^ permalink raw reply related

* [PATCH 07/11] PCI/ASPM: Use sysfs_emit() and sysfs_emit_at() in "show" functions
From: Krzysztof Wilczyński @ 2021-05-10  4:14 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Tyrel Datwyler, linux-pci, linuxppc-dev, Oliver O'Halloran,
	Paul Mackerras, Kurt Schwemmer, Logan Gunthorpe
In-Reply-To: <20210510041424.233565-1-kw@linux.com>

The sysfs_emit() and sysfs_emit_at() functions were introduced to make
it less ambiguous which function is preferred when writing to the output
buffer in a device attribute's "show" callback [1].

Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
latter is aware of the PAGE_SIZE buffer and correctly returns the number
of bytes written into the buffer.

No functional change intended.

[1] Documentation/filesystems/sysfs.rst

Related to:
  commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
---
 drivers/pci/pcie/aspm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index ac0557a305af..013a47f587ce 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -1208,7 +1208,7 @@ static ssize_t aspm_attr_show_common(struct device *dev,
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct pcie_link_state *link = pcie_aspm_get_link(pdev);
 
-	return sprintf(buf, "%d\n", (link->aspm_enabled & state) ? 1 : 0);
+	return sysfs_emit(buf, "%d\n", (link->aspm_enabled & state) ? 1 : 0);
 }
 
 static ssize_t aspm_attr_store_common(struct device *dev,
@@ -1265,7 +1265,7 @@ static ssize_t clkpm_show(struct device *dev,
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct pcie_link_state *link = pcie_aspm_get_link(pdev);
 
-	return sprintf(buf, "%d\n", link->clkpm_enabled);
+	return sysfs_emit(buf, "%d\n", link->clkpm_enabled);
 }
 
 static ssize_t clkpm_store(struct device *dev,
-- 
2.31.1


^ permalink raw reply related

* [PATCH 08/11] PCI: switchtec: Use sysfs_emit() and sysfs_emit_at() in "show" functions
From: Krzysztof Wilczyński @ 2021-05-10  4:14 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Tyrel Datwyler, linux-pci, linuxppc-dev, Oliver O'Halloran,
	Paul Mackerras, Kurt Schwemmer, Logan Gunthorpe
In-Reply-To: <20210510041424.233565-1-kw@linux.com>

The sysfs_emit() and sysfs_emit_at() functions were introduced to make
it less ambiguous which function is preferred when writing to the output
buffer in a device attribute's "show" callback [1].

Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
latter is aware of the PAGE_SIZE buffer and correctly returns the number
of bytes written into the buffer.

No functional change intended.

[1] Documentation/filesystems/sysfs.rst

Related to:
  commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
---
 drivers/pci/switch/switchtec.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index ba52459928f7..0b301f8be9ed 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -280,7 +280,7 @@ static ssize_t device_version_show(struct device *dev,
 
 	ver = ioread32(&stdev->mmio_sys_info->device_version);
 
-	return sprintf(buf, "%x\n", ver);
+	return sysfs_emit(buf, "%x\n", ver);
 }
 static DEVICE_ATTR_RO(device_version);
 
@@ -292,7 +292,7 @@ static ssize_t fw_version_show(struct device *dev,
 
 	ver = ioread32(&stdev->mmio_sys_info->firmware_version);
 
-	return sprintf(buf, "%08x\n", ver);
+	return sysfs_emit(buf, "%08x\n", ver);
 }
 static DEVICE_ATTR_RO(fw_version);
 
@@ -344,7 +344,7 @@ static ssize_t component_vendor_show(struct device *dev,
 
 	/* component_vendor field not supported after gen3 */
 	if (stdev->gen != SWITCHTEC_GEN3)
-		return sprintf(buf, "none\n");
+		return sysfs_emit(buf, "none\n");
 
 	return io_string_show(buf, &si->gen3.component_vendor,
 			      sizeof(si->gen3.component_vendor));
@@ -359,9 +359,9 @@ static ssize_t component_id_show(struct device *dev,
 
 	/* component_id field not supported after gen3 */
 	if (stdev->gen != SWITCHTEC_GEN3)
-		return sprintf(buf, "none\n");
+		return sysfs_emit(buf, "none\n");
 
-	return sprintf(buf, "PM%04X\n", id);
+	return sysfs_emit(buf, "PM%04X\n", id);
 }
 static DEVICE_ATTR_RO(component_id);
 
@@ -373,9 +373,9 @@ static ssize_t component_revision_show(struct device *dev,
 
 	/* component_revision field not supported after gen3 */
 	if (stdev->gen != SWITCHTEC_GEN3)
-		return sprintf(buf, "255\n");
+		return sysfs_emit(buf, "255\n");
 
-	return sprintf(buf, "%d\n", rev);
+	return sysfs_emit(buf, "%d\n", rev);
 }
 static DEVICE_ATTR_RO(component_revision);
 
@@ -384,7 +384,7 @@ static ssize_t partition_show(struct device *dev,
 {
 	struct switchtec_dev *stdev = to_stdev(dev);
 
-	return sprintf(buf, "%d\n", stdev->partition);
+	return sysfs_emit(buf, "%d\n", stdev->partition);
 }
 static DEVICE_ATTR_RO(partition);
 
@@ -393,7 +393,7 @@ static ssize_t partition_count_show(struct device *dev,
 {
 	struct switchtec_dev *stdev = to_stdev(dev);
 
-	return sprintf(buf, "%d\n", stdev->partition_count);
+	return sysfs_emit(buf, "%d\n", stdev->partition_count);
 }
 static DEVICE_ATTR_RO(partition_count);
 
-- 
2.31.1


^ permalink raw reply related

* [PATCH 09/11] PCI: rpadlpar: Use sysfs_emit() and sysfs_emit_at() in "show" functions
From: Krzysztof Wilczyński @ 2021-05-10  4:14 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Tyrel Datwyler, linux-pci, linuxppc-dev, Oliver O'Halloran,
	Paul Mackerras, Kurt Schwemmer, Logan Gunthorpe
In-Reply-To: <20210510041424.233565-1-kw@linux.com>

The sysfs_emit() and sysfs_emit_at() functions were introduced to make
it less ambiguous which function is preferred when writing to the output
buffer in a device attribute's "show" callback [1].

Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
latter is aware of the PAGE_SIZE buffer and correctly returns the number
of bytes written into the buffer.

No functional change intended.

[1] Documentation/filesystems/sysfs.rst

Related to:
  commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
---
 drivers/pci/hotplug/rpadlpar_sysfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/hotplug/rpadlpar_sysfs.c b/drivers/pci/hotplug/rpadlpar_sysfs.c
index dbfa0b55d31a..068b7810a574 100644
--- a/drivers/pci/hotplug/rpadlpar_sysfs.c
+++ b/drivers/pci/hotplug/rpadlpar_sysfs.c
@@ -50,7 +50,7 @@ static ssize_t add_slot_store(struct kobject *kobj, struct kobj_attribute *attr,
 static ssize_t add_slot_show(struct kobject *kobj,
 			     struct kobj_attribute *attr, char *buf)
 {
-	return sprintf(buf, "0\n");
+	return sysfs_emit(buf, "0\n");
 }
 
 static ssize_t remove_slot_store(struct kobject *kobj,
@@ -80,7 +80,7 @@ static ssize_t remove_slot_store(struct kobject *kobj,
 static ssize_t remove_slot_show(struct kobject *kobj,
 				struct kobj_attribute *attr, char *buf)
 {
-	return sprintf(buf, "0\n");
+	return sysfs_emit(buf, "0\n");
 }
 
 static struct kobj_attribute add_slot_attr =
-- 
2.31.1


^ permalink raw reply related

* [PATCH 10/11] PCI: hotplug: Use sysfs_emit() and sysfs_emit_at() in "show" functions
From: Krzysztof Wilczyński @ 2021-05-10  4:14 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Tyrel Datwyler, linux-pci, linuxppc-dev, Oliver O'Halloran,
	Paul Mackerras, Kurt Schwemmer, Logan Gunthorpe
In-Reply-To: <20210510041424.233565-1-kw@linux.com>

The sysfs_emit() and sysfs_emit_at() functions were introduced to make
it less ambiguous which function is preferred when writing to the output
buffer in a device attribute's "show" callback [1].

Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
latter is aware of the PAGE_SIZE buffer and correctly returns the number
of bytes written into the buffer.

No functional change intended.

[1] Documentation/filesystems/sysfs.rst

Related to:
  commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
---
 drivers/pci/hotplug/pci_hotplug_core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c
index 5ac31f683b85..058d5937d8a9 100644
--- a/drivers/pci/hotplug/pci_hotplug_core.c
+++ b/drivers/pci/hotplug/pci_hotplug_core.c
@@ -73,7 +73,7 @@ static ssize_t power_read_file(struct pci_slot *pci_slot, char *buf)
 	if (retval)
 		return retval;
 
-	return sprintf(buf, "%d\n", value);
+	return sysfs_emit(buf, "%d\n", value);
 }
 
 static ssize_t power_write_file(struct pci_slot *pci_slot, const char *buf,
@@ -130,7 +130,7 @@ static ssize_t attention_read_file(struct pci_slot *pci_slot, char *buf)
 	if (retval)
 		return retval;
 
-	return sprintf(buf, "%d\n", value);
+	return sysfs_emit(buf, "%d\n", value);
 }
 
 static ssize_t attention_write_file(struct pci_slot *pci_slot, const char *buf,
@@ -175,7 +175,7 @@ static ssize_t latch_read_file(struct pci_slot *pci_slot, char *buf)
 	if (retval)
 		return retval;
 
-	return sprintf(buf, "%d\n", value);
+	return sysfs_emit(buf, "%d\n", value);
 }
 
 static struct pci_slot_attribute hotplug_slot_attr_latch = {
@@ -192,7 +192,7 @@ static ssize_t presence_read_file(struct pci_slot *pci_slot, char *buf)
 	if (retval)
 		return retval;
 
-	return sprintf(buf, "%d\n", value);
+	return sysfs_emit(buf, "%d\n", value);
 }
 
 static struct pci_slot_attribute hotplug_slot_attr_presence = {
-- 
2.31.1


^ permalink raw reply related

* [PATCH 11/11] PCI: shpchp: Use sysfs_emit() and sysfs_emit_at() in "show" functions
From: Krzysztof Wilczyński @ 2021-05-10  4:14 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Tyrel Datwyler, linux-pci, linuxppc-dev, Oliver O'Halloran,
	Paul Mackerras, Kurt Schwemmer, Logan Gunthorpe
In-Reply-To: <20210510041424.233565-1-kw@linux.com>

The sysfs_emit() and sysfs_emit_at() functions were introduced to make
it less ambiguous which function is preferred when writing to the output
buffer in a device attribute's "show" callback [1].

Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
latter is aware of the PAGE_SIZE buffer and correctly returns the number
of bytes written into the buffer.

No functional change intended.

[1] Documentation/filesystems/sysfs.rst

Related to:
  commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
---
 drivers/pci/hotplug/shpchp_sysfs.c | 38 +++++++++++++++++-------------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/drivers/pci/hotplug/shpchp_sysfs.c b/drivers/pci/hotplug/shpchp_sysfs.c
index 45658bb5c554..64beed7a26be 100644
--- a/drivers/pci/hotplug/shpchp_sysfs.c
+++ b/drivers/pci/hotplug/shpchp_sysfs.c
@@ -24,50 +24,54 @@
 static ssize_t show_ctrl(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct pci_dev *pdev;
-	char *out = buf;
 	int index, busnr;
 	struct resource *res;
 	struct pci_bus *bus;
+	size_t len = 0;
 
 	pdev = to_pci_dev(dev);
 	bus = pdev->subordinate;
 
-	out += sprintf(buf, "Free resources: memory\n");
+	len += sysfs_emit_at(buf, len, "Free resources: memory\n");
 	pci_bus_for_each_resource(bus, res, index) {
 		if (res && (res->flags & IORESOURCE_MEM) &&
 				!(res->flags & IORESOURCE_PREFETCH)) {
-			out += sprintf(out, "start = %8.8llx, length = %8.8llx\n",
-				       (unsigned long long)res->start,
-				       (unsigned long long)resource_size(res));
+			len += sysfs_emit_at(buf, len,
+					     "start = %8.8llx, length = %8.8llx\n",
+					     (unsigned long long)res->start,
+					     (unsigned long long)resource_size(res));
 		}
 	}
-	out += sprintf(out, "Free resources: prefetchable memory\n");
+	len += sysfs_emit_at(buf, len, "Free resources: prefetchable memory\n");
 	pci_bus_for_each_resource(bus, res, index) {
 		if (res && (res->flags & IORESOURCE_MEM) &&
 			       (res->flags & IORESOURCE_PREFETCH)) {
-			out += sprintf(out, "start = %8.8llx, length = %8.8llx\n",
-				       (unsigned long long)res->start,
-				       (unsigned long long)resource_size(res));
+			len += sysfs_emit_at(buf, len,
+					     "start = %8.8llx, length = %8.8llx\n",
+					     (unsigned long long)res->start,
+					     (unsigned long long)resource_size(res));
 		}
 	}
-	out += sprintf(out, "Free resources: IO\n");
+	len += sysfs_emit_at(buf, len, "Free resources: IO\n");
 	pci_bus_for_each_resource(bus, res, index) {
 		if (res && (res->flags & IORESOURCE_IO)) {
-			out += sprintf(out, "start = %8.8llx, length = %8.8llx\n",
-				       (unsigned long long)res->start,
-				       (unsigned long long)resource_size(res));
+			len += sysfs_emit_at(buf, len,
+					     "start = %8.8llx, length = %8.8llx\n",
+					     (unsigned long long)res->start,
+					     (unsigned long long)resource_size(res));
 		}
 	}
-	out += sprintf(out, "Free resources: bus numbers\n");
+	len += sysfs_emit_at(buf, len, "Free resources: bus numbers\n");
 	for (busnr = bus->busn_res.start; busnr <= bus->busn_res.end; busnr++) {
 		if (!pci_find_bus(pci_domain_nr(bus), busnr))
 			break;
 	}
 	if (busnr < bus->busn_res.end)
-		out += sprintf(out, "start = %8.8x, length = %8.8x\n",
-				busnr, (int)(bus->busn_res.end - busnr));
+		len += sysfs_emit_at(buf, len,
+				     "start = %8.8x, length = %8.8x\n",
+				     busnr, (int)(bus->busn_res.end - busnr));
 
-	return out - buf;
+	return len;
 }
 static DEVICE_ATTR(ctrl, S_IRUGO, show_ctrl, NULL);
 
-- 
2.31.1


^ permalink raw reply related

* Re: [V3 PATCH 01/16] powerpc/powernv/vas: Rename register/unregister functions
From: Nicholas Piggin @ 2021-05-10  5:10 UTC (permalink / raw)
  To: Haren Myneni, herbert, linux-crypto, linuxppc-dev, mpe
In-Reply-To: <86bae80a92b8465d663f72e7fadc1fa3671e8a4f.camel@linux.ibm.com>

Excerpts from Haren Myneni's message of April 18, 2021 7:00 am:
> 
> powerNV and pseries drivers register / unregister to the corresponding
> VAS code separately. So rename powerNV VAS API register/unregister
> functions.

The pseries VAS driver will have different calls for registering a
coprocessor driver, you mean?

It certainly looks the same 

(from patch 13)
	ret = vas_register_api_pseries(THIS_MODULE, VAS_COP_TYPE_GZIP,
				       "nx-gzip");

So I guess it's just a matter of the driver being different enough that 
there is no benefit to making this call common (and branching to pseries
or powernv dynamically).

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

> 
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
>  arch/powerpc/include/asm/vas.h           |  6 +++---
>  arch/powerpc/platforms/powernv/vas-api.c | 10 +++++-----
>  drivers/crypto/nx/nx-common-powernv.c    |  6 +++---
>  3 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/vas.h b/arch/powerpc/include/asm/vas.h
> index e33f80b0ea81..41f73fae7ab8 100644
> --- a/arch/powerpc/include/asm/vas.h
> +++ b/arch/powerpc/include/asm/vas.h
> @@ -170,8 +170,8 @@ int vas_paste_crb(struct vas_window *win, int offset, bool re);
>   * Only NX GZIP coprocessor type is supported now, but this API can be
>   * used for others in future.
>   */
> -int vas_register_coproc_api(struct module *mod, enum vas_cop_type cop_type,
> -				const char *name);
> -void vas_unregister_coproc_api(void);
> +int vas_register_api_powernv(struct module *mod, enum vas_cop_type cop_type,
> +			     const char *name);
> +void vas_unregister_api_powernv(void);
>  
>  #endif /* __ASM_POWERPC_VAS_H */
> diff --git a/arch/powerpc/platforms/powernv/vas-api.c b/arch/powerpc/platforms/powernv/vas-api.c
> index 98ed5d8c5441..72d8ce39e56c 100644
> --- a/arch/powerpc/platforms/powernv/vas-api.c
> +++ b/arch/powerpc/platforms/powernv/vas-api.c
> @@ -207,8 +207,8 @@ static struct file_operations coproc_fops = {
>   * Supporting only nx-gzip coprocessor type now, but this API code
>   * extended to other coprocessor types later.
>   */
> -int vas_register_coproc_api(struct module *mod, enum vas_cop_type cop_type,
> -				const char *name)
> +int vas_register_api_powernv(struct module *mod, enum vas_cop_type cop_type,
> +			     const char *name)
>  {
>  	int rc = -EINVAL;
>  	dev_t devno;
> @@ -262,9 +262,9 @@ int vas_register_coproc_api(struct module *mod, enum vas_cop_type cop_type,
>  	unregister_chrdev_region(coproc_device.devt, 1);
>  	return rc;
>  }
> -EXPORT_SYMBOL_GPL(vas_register_coproc_api);
> +EXPORT_SYMBOL_GPL(vas_register_api_powernv);
>  
> -void vas_unregister_coproc_api(void)
> +void vas_unregister_api_powernv(void)
>  {
>  	dev_t devno;
>  
> @@ -275,4 +275,4 @@ void vas_unregister_coproc_api(void)
>  	class_destroy(coproc_device.class);
>  	unregister_chrdev_region(coproc_device.devt, 1);
>  }
> -EXPORT_SYMBOL_GPL(vas_unregister_coproc_api);
> +EXPORT_SYMBOL_GPL(vas_unregister_api_powernv);
> diff --git a/drivers/crypto/nx/nx-common-powernv.c b/drivers/crypto/nx/nx-common-powernv.c
> index 13c65deda8e9..88d728415bb2 100644
> --- a/drivers/crypto/nx/nx-common-powernv.c
> +++ b/drivers/crypto/nx/nx-common-powernv.c
> @@ -1090,8 +1090,8 @@ static __init int nx_compress_powernv_init(void)
>  		 * normal FIFO priority is assigned for userspace.
>  		 * 842 compression is supported only in kernel.
>  		 */
> -		ret = vas_register_coproc_api(THIS_MODULE, VAS_COP_TYPE_GZIP,
> -						"nx-gzip");
> +		ret = vas_register_api_powernv(THIS_MODULE, VAS_COP_TYPE_GZIP,
> +					       "nx-gzip");
>  
>  		/*
>  		 * GZIP is not supported in kernel right now.
> @@ -1127,7 +1127,7 @@ static void __exit nx_compress_powernv_exit(void)
>  	 * use. So delete this API use for GZIP engine.
>  	 */
>  	if (!nx842_ct)
> -		vas_unregister_coproc_api();
> +		vas_unregister_api_powernv();
>  
>  	crypto_unregister_alg(&nx842_powernv_alg);
>  
> -- 
> 2.18.2
> 
> 
> 

^ permalink raw reply

* Re: [PATCH V3 02/16] powerpc/vas: Move VAS API to common book3s platform
From: Nicholas Piggin @ 2021-05-10  5:19 UTC (permalink / raw)
  To: Haren Myneni, herbert, linux-crypto, linuxppc-dev, mpe
In-Reply-To: <163867b893124434dfe3e13c6ba2f081c309e96f.camel@linux.ibm.com>

Excerpts from Haren Myneni's message of April 18, 2021 7:02 am:
> 
> Using the same /dev/crypto/nx-gzip interface for both powerNV and
> pseries.

The pseries NX driver will use the powernv VAS API ?

> So this patch creates platforms/book3s/ and moves VAS API
> to that directory. The actual functionality is not changed.
> 
> Common interface functions such as open, window open ioctl, mmap
> and close are moved to arch/powerpc/platforms/book3s/vas-api.c
> Added hooks to call platform specific code, but the underline
> powerNV code in these functions is not changed.

Even so, could you do one patch that just moves, and another that
adds the ops struct?

> 
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
>  arch/powerpc/include/asm/vas.h                | 22 ++++++-
>  arch/powerpc/platforms/Kconfig                |  1 +
>  arch/powerpc/platforms/Makefile               |  1 +
>  arch/powerpc/platforms/book3s/Kconfig         | 15 +++++
>  arch/powerpc/platforms/book3s/Makefile        |  2 +

The usual place for these would be arch/powerpc/sysdev/vas. E.g., see
arch/powerpc/sysdev/xive.

>  .../platforms/{powernv => book3s}/vas-api.c   | 64 ++++++++++--------
>  arch/powerpc/platforms/powernv/Kconfig        | 14 ----
>  arch/powerpc/platforms/powernv/Makefile       |  2 +-
>  arch/powerpc/platforms/powernv/vas-window.c   | 66 +++++++++++++++++++
>  9 files changed, 143 insertions(+), 44 deletions(-)
>  create mode 100644 arch/powerpc/platforms/book3s/Kconfig
>  create mode 100644 arch/powerpc/platforms/book3s/Makefile
>  rename arch/powerpc/platforms/{powernv => book3s}/vas-api.c (83%)
> 
> diff --git a/arch/powerpc/include/asm/vas.h b/arch/powerpc/include/asm/vas.h
> index 41f73fae7ab8..6bbade60d8f4 100644
> --- a/arch/powerpc/include/asm/vas.h
> +++ b/arch/powerpc/include/asm/vas.h
> @@ -5,6 +5,8 @@
>  
>  #ifndef _ASM_POWERPC_VAS_H
>  #define _ASM_POWERPC_VAS_H
> +#include <uapi/asm/vas-api.h>
> +
>  
>  struct vas_window;
>  
> @@ -48,6 +50,16 @@ enum vas_cop_type {
>  	VAS_COP_TYPE_MAX,
>  };
>  
> +/*
> + * User space window operations used for powernv and powerVM
> + */
> +struct vas_user_win_ops {
> +	struct vas_window * (*open_win)(struct vas_tx_win_open_attr *,
> +				enum vas_cop_type);
> +	u64 (*paste_addr)(void *);
> +	int (*close_win)(void *);
> +};
> +
>  /*
>   * Receive window attributes specified by the (in-kernel) owner of window.
>   */
> @@ -161,6 +173,9 @@ int vas_copy_crb(void *crb, int offset);
>   * assumed to be true for NX windows.
>   */
>  int vas_paste_crb(struct vas_window *win, int offset, bool re);
> +int vas_register_api_powernv(struct module *mod, enum vas_cop_type cop_type,
> +			     const char *name);
> +void vas_unregister_api_powernv(void);
>  
>  /*
>   * Register / unregister coprocessor type to VAS API which will be exported
> @@ -170,8 +185,9 @@ int vas_paste_crb(struct vas_window *win, int offset, bool re);
>   * Only NX GZIP coprocessor type is supported now, but this API can be
>   * used for others in future.
>   */
> -int vas_register_api_powernv(struct module *mod, enum vas_cop_type cop_type,
> -			     const char *name);
> -void vas_unregister_api_powernv(void);
> +int vas_register_coproc_api(struct module *mod, enum vas_cop_type cop_type,
> +			    const char *name,
> +			    struct vas_user_win_ops *vops);
> +void vas_unregister_coproc_api(void);
>  
>  #endif /* __ASM_POWERPC_VAS_H */
> diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
> index 7a5e8f4541e3..594544a65b02 100644
> --- a/arch/powerpc/platforms/Kconfig
> +++ b/arch/powerpc/platforms/Kconfig
> @@ -20,6 +20,7 @@ source "arch/powerpc/platforms/embedded6xx/Kconfig"
>  source "arch/powerpc/platforms/44x/Kconfig"
>  source "arch/powerpc/platforms/40x/Kconfig"
>  source "arch/powerpc/platforms/amigaone/Kconfig"
> +source "arch/powerpc/platforms/book3s/Kconfig"
>  
>  config KVM_GUEST
>  	bool "KVM Guest support"
> diff --git a/arch/powerpc/platforms/Makefile b/arch/powerpc/platforms/Makefile
> index 143d4417f6cc..0e75d7df387b 100644
> --- a/arch/powerpc/platforms/Makefile
> +++ b/arch/powerpc/platforms/Makefile
> @@ -22,3 +22,4 @@ obj-$(CONFIG_PPC_CELL)		+= cell/
>  obj-$(CONFIG_PPC_PS3)		+= ps3/
>  obj-$(CONFIG_EMBEDDED6xx)	+= embedded6xx/
>  obj-$(CONFIG_AMIGAONE)		+= amigaone/
> +obj-$(CONFIG_PPC_BOOK3S)	+= book3s/
> diff --git a/arch/powerpc/platforms/book3s/Kconfig b/arch/powerpc/platforms/book3s/Kconfig
> new file mode 100644
> index 000000000000..51e14db83a79
> --- /dev/null
> +++ b/arch/powerpc/platforms/book3s/Kconfig
> @@ -0,0 +1,15 @@
> +# SPDX-License-Identifier: GPL-2.0
> +config PPC_VAS
> +	bool "IBM Virtual Accelerator Switchboard (VAS)"
> +	depends on PPC_POWERNV && PPC_64K_PAGES
> +	default y
> +	help
> +	  This enables support for IBM Virtual Accelerator Switchboard (VAS).
> +
> +	  VAS allows accelerators in co-processors like NX-GZIP and NX-842
> +	  to be accessible to kernel subsystems and user processes.
> +	  VAS adapters are found in POWER9 and later based systems.
> +	  The user mode NX-GZIP support is added on P9 for powerNV and on
> +	  P10 for powerVM.
> +
> +	  If unsure, say "N".
> diff --git a/arch/powerpc/platforms/book3s/Makefile b/arch/powerpc/platforms/book3s/Makefile
> new file mode 100644
> index 000000000000..e790f1910f61
> --- /dev/null
> +++ b/arch/powerpc/platforms/book3s/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +obj-$(CONFIG_PPC_VAS)	+= vas-api.o
> diff --git a/arch/powerpc/platforms/powernv/vas-api.c b/arch/powerpc/platforms/book3s/vas-api.c
> similarity index 83%
> rename from arch/powerpc/platforms/powernv/vas-api.c
> rename to arch/powerpc/platforms/book3s/vas-api.c
> index 72d8ce39e56c..05d7b99acf41 100644
> --- a/arch/powerpc/platforms/powernv/vas-api.c
> +++ b/arch/powerpc/platforms/book3s/vas-api.c
> @@ -4,15 +4,20 @@
>   * Copyright (C) 2019 Haren Myneni, IBM Corp
>   */
>  
> +#include <linux/module.h>
>  #include <linux/kernel.h>
>  #include <linux/device.h>
>  #include <linux/cdev.h>
>  #include <linux/fs.h>
>  #include <linux/slab.h>
>  #include <linux/uaccess.h>
> +#include <linux/kthread.h>
> +#include <linux/sched/signal.h>
> +#include <linux/sched/mm.h>
> +#include <linux/mmu_context.h>
>  #include <asm/vas.h>
> +#include <asm/icswx.h>
>  #include <uapi/asm/vas-api.h>
> -#include "vas.h"
>  
>  /*
>   * The driver creates the device node that can be used as follows:
> @@ -42,6 +47,7 @@ static struct coproc_dev {
>  	dev_t devt;
>  	struct class *class;
>  	enum vas_cop_type cop_type;
> +	struct vas_user_win_ops *vops;
>  } coproc_device;
>  
>  struct coproc_instance {
> @@ -72,11 +78,10 @@ static int coproc_open(struct inode *inode, struct file *fp)
>  static int coproc_ioc_tx_win_open(struct file *fp, unsigned long arg)
>  {
>  	void __user *uptr = (void __user *)arg;
> -	struct vas_tx_win_attr txattr = {};
>  	struct vas_tx_win_open_attr uattr;
>  	struct coproc_instance *cp_inst;
>  	struct vas_window *txwin;
> -	int rc, vasid;
> +	int rc;
>  
>  	cp_inst = fp->private_data;
>  
> @@ -93,27 +98,20 @@ static int coproc_ioc_tx_win_open(struct file *fp, unsigned long arg)
>  	}
>  
>  	if (uattr.version != 1) {
> -		pr_err("Invalid version\n");
> +		pr_err("Invalid window open API version\n");
>  		return -EINVAL;
>  	}
>  
> -	vasid = uattr.vas_id;
> -
> -	vas_init_tx_win_attr(&txattr, cp_inst->coproc->cop_type);
> -
> -	txattr.lpid = mfspr(SPRN_LPID);
> -	txattr.pidr = mfspr(SPRN_PID);
> -	txattr.user_win = true;
> -	txattr.rsvd_txbuf_count = false;
> -	txattr.pswid = false;
> -
> -	pr_devel("Pid %d: Opening txwin, PIDR %ld\n", txattr.pidr,
> -				mfspr(SPRN_PID));
> +	if (!cp_inst->coproc->vops && !cp_inst->coproc->vops->open_win) {
> +		pr_err("VAS API is not registered\n");
> +		return -EACCES;
> +	}
>  
> -	txwin = vas_tx_win_open(vasid, cp_inst->coproc->cop_type, &txattr);
> +	txwin = cp_inst->coproc->vops->open_win(&uattr,
> +					cp_inst->coproc->cop_type);
>  	if (IS_ERR(txwin)) {
> -		pr_err("%s() vas_tx_win_open() failed, %ld\n", __func__,
> -					PTR_ERR(txwin));
> +		pr_err("%s() VAS window open failed, %ld\n", __func__,
> +				PTR_ERR(txwin));
>  		return PTR_ERR(txwin);
>  	}
>  
> @@ -125,9 +123,14 @@ static int coproc_ioc_tx_win_open(struct file *fp, unsigned long arg)
>  static int coproc_release(struct inode *inode, struct file *fp)
>  {
>  	struct coproc_instance *cp_inst = fp->private_data;
> +	int rc = 0;
>  
>  	if (cp_inst->txwin) {
> -		vas_win_close(cp_inst->txwin);
> +		if (cp_inst->coproc->vops && cp_inst->coproc->vops->close_win) {
> +			rc = cp_inst->coproc->vops->close_win(cp_inst->txwin);
> +			if (rc)
> +				return rc;
> +		}
>  		cp_inst->txwin = NULL;
>  	}
>  
> @@ -168,7 +171,17 @@ static int coproc_mmap(struct file *fp, struct vm_area_struct *vma)
>  		return -EINVAL;
>  	}
>  
> -	vas_win_paste_addr(txwin, &paste_addr, NULL);
> +	if (!cp_inst->coproc->vops && !cp_inst->coproc->vops->paste_addr) {
> +		pr_err("%s(): VAS API is not registered\n", __func__);
> +		return -EACCES;
> +	}
> +
> +	paste_addr = cp_inst->coproc->vops->paste_addr(txwin);
> +	if (!paste_addr) {
> +		pr_err("%s(): Window paste address failed\n", __func__);
> +		return -EINVAL;
> +	}
> +
>  	pfn = paste_addr >> PAGE_SHIFT;
>  
>  	/* flags, page_prot from cxl_mmap(), except we want cachable */
> @@ -207,8 +220,8 @@ static struct file_operations coproc_fops = {
>   * Supporting only nx-gzip coprocessor type now, but this API code
>   * extended to other coprocessor types later.
>   */
> -int vas_register_api_powernv(struct module *mod, enum vas_cop_type cop_type,
> -			     const char *name)
> +int vas_register_coproc_api(struct module *mod, enum vas_cop_type cop_type,
> +			const char *name, struct vas_user_win_ops *vops)
>  {
>  	int rc = -EINVAL;
>  	dev_t devno;
> @@ -230,6 +243,7 @@ int vas_register_api_powernv(struct module *mod, enum vas_cop_type cop_type,
>  	}
>  	coproc_device.class->devnode = coproc_devnode;
>  	coproc_device.cop_type = cop_type;
> +	coproc_device.vops = vops;
>  
>  	coproc_fops.owner = mod;
>  	cdev_init(&coproc_device.cdev, &coproc_fops);
> @@ -262,9 +276,8 @@ int vas_register_api_powernv(struct module *mod, enum vas_cop_type cop_type,
>  	unregister_chrdev_region(coproc_device.devt, 1);
>  	return rc;
>  }
> -EXPORT_SYMBOL_GPL(vas_register_api_powernv);
>  
> -void vas_unregister_api_powernv(void)
> +void vas_unregister_coproc_api(void)
>  {
>  	dev_t devno;
>  
> @@ -275,4 +288,3 @@ void vas_unregister_api_powernv(void)
>  	class_destroy(coproc_device.class);
>  	unregister_chrdev_region(coproc_device.devt, 1);
>  }
> -EXPORT_SYMBOL_GPL(vas_unregister_api_powernv);
> diff --git a/arch/powerpc/platforms/powernv/Kconfig b/arch/powerpc/platforms/powernv/Kconfig
> index 619b093a0657..043eefbbdd28 100644
> --- a/arch/powerpc/platforms/powernv/Kconfig
> +++ b/arch/powerpc/platforms/powernv/Kconfig
> @@ -33,20 +33,6 @@ config PPC_MEMTRACE
>  	  Enabling this option allows for runtime allocation of memory (RAM)
>  	  for hardware tracing.
>  
> -config PPC_VAS
> -	bool "IBM Virtual Accelerator Switchboard (VAS)"
> -	depends on PPC_POWERNV && PPC_64K_PAGES
> -	default y
> -	help
> -	  This enables support for IBM Virtual Accelerator Switchboard (VAS).
> -
> -	  VAS allows accelerators in co-processors like NX-GZIP and NX-842
> -	  to be accessible to kernel subsystems and user processes.
> -
> -	  VAS adapters are found in POWER9 based systems.
> -
> -	  If unsure, say N.
> -
>  config SCOM_DEBUGFS
>  	bool "Expose SCOM controllers via debugfs"
>  	depends on DEBUG_FS
> diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
> index 2eb6ae150d1f..c747a1f1d25b 100644
> --- a/arch/powerpc/platforms/powernv/Makefile
> +++ b/arch/powerpc/platforms/powernv/Makefile
> @@ -18,7 +18,7 @@ obj-$(CONFIG_MEMORY_FAILURE)	+= opal-memory-errors.o
>  obj-$(CONFIG_OPAL_PRD)	+= opal-prd.o
>  obj-$(CONFIG_PERF_EVENTS) += opal-imc.o
>  obj-$(CONFIG_PPC_MEMTRACE)	+= memtrace.o
> -obj-$(CONFIG_PPC_VAS)	+= vas.o vas-window.o vas-debug.o vas-fault.o vas-api.o
> +obj-$(CONFIG_PPC_VAS)	+= vas.o vas-window.o vas-debug.o vas-fault.o
>  obj-$(CONFIG_OCXL_BASE)	+= ocxl.o
>  obj-$(CONFIG_SCOM_DEBUGFS) += opal-xscom.o
>  obj-$(CONFIG_PPC_SECURE_BOOT) += opal-secvar.o
> diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c
> index 5f5fe63a3d1c..b973dd574b47 100644
> --- a/arch/powerpc/platforms/powernv/vas-window.c
> +++ b/arch/powerpc/platforms/powernv/vas-window.c
> @@ -16,6 +16,8 @@
>  #include <linux/mmu_context.h>
>  #include <asm/switch_to.h>
>  #include <asm/ppc-opcode.h>
> +#include <asm/vas.h>
> +#include <uapi/asm/vas-api.h>
>  #include "vas.h"
>  #include "copy-paste.h"
>  
> @@ -1441,3 +1443,67 @@ struct vas_window *vas_pswid_to_window(struct vas_instance *vinst,
>  
>  	return window;
>  }
> +
> +static struct vas_window *vas_user_win_open(struct vas_tx_win_open_attr *uattr,
> +				enum vas_cop_type cop_type)
> +{
> +	struct vas_tx_win_attr txattr = {};
> +
> +	vas_init_tx_win_attr(&txattr, cop_type);
> +
> +	txattr.lpid = mfspr(SPRN_LPID);
> +	txattr.pidr = mfspr(SPRN_PID);
> +	txattr.user_win = true;
> +	txattr.rsvd_txbuf_count = false;
> +	txattr.pswid = false;
> +
> +	pr_devel("Pid %d: Opening txwin, PIDR %ld\n", txattr.pidr,
> +				mfspr(SPRN_PID));
> +
> +	return vas_tx_win_open(uattr->vas_id, cop_type, &txattr);
> +}
> +
> +static u64 vas_user_win_paste_addr(void *addr)
> +{
> +	u64 paste_addr;
> +
> +	vas_win_paste_addr((struct vas_window *)addr, &paste_addr, NULL);
> +
> +	return paste_addr;
> +}
> +
> +static int vas_user_win_close(void *addr)
> +{
> +	struct vas_window *txwin = addr;
> +
> +	vas_win_close(txwin);
> +
> +	return 0;
> +}
> +
> +static struct vas_user_win_ops vops =  {
> +	.open_win	=	vas_user_win_open,
> +	.paste_addr	=	vas_user_win_paste_addr,
> +	.close_win	=	vas_user_win_close,
> +};
> +
> +/*
> + * Supporting only nx-gzip coprocessor type now, but this API code
> + * extended to other coprocessor types later.
> + */
> +int vas_register_api_powernv(struct module *mod, enum vas_cop_type cop_type,
> +				const char *name)
> +{
> +	int rc;
> +
> +	rc = vas_register_coproc_api(mod, cop_type, name, &vops);
> +
> +	return rc;
> +}
> +EXPORT_SYMBOL_GPL(vas_register_api_powernv);
> +
> +void vas_unregister_api_powernv(void)
> +{
> +	vas_unregister_coproc_api();
> +}
> +EXPORT_SYMBOL_GPL(vas_unregister_api_powernv);
> -- 
> 2.18.2
> 
> 
> 

^ permalink raw reply

* Re: [V3 PATCH 03/16] powerpc/vas: Create take/drop task reference functions
From: Nicholas Piggin @ 2021-05-10  5:28 UTC (permalink / raw)
  To: Haren Myneni, herbert, linux-crypto, linuxppc-dev, mpe
In-Reply-To: <3d634fb22a1d3d87f3172b23414a48650fc9a366.camel@linux.ibm.com>

Excerpts from Haren Myneni's message of April 18, 2021 7:03 am:
> 
> Take task reference when each window opens and drops during close.
> This functionality is needed for powerNV and pseries. So this patch
> defines the existing code as functions in common book3s platform
> vas-api.c
> 
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
>  arch/powerpc/include/asm/vas.h              | 20 ++++++++
>  arch/powerpc/platforms/book3s/vas-api.c     | 51 ++++++++++++++++++
>  arch/powerpc/platforms/powernv/vas-fault.c  | 10 ++--
>  arch/powerpc/platforms/powernv/vas-window.c | 57 ++-------------------
>  arch/powerpc/platforms/powernv/vas.h        |  6 +--
>  5 files changed, 83 insertions(+), 61 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/vas.h b/arch/powerpc/include/asm/vas.h
> index 6bbade60d8f4..2daaa1a2a9a9 100644
> --- a/arch/powerpc/include/asm/vas.h
> +++ b/arch/powerpc/include/asm/vas.h
> @@ -5,6 +5,9 @@
>  
>  #ifndef _ASM_POWERPC_VAS_H
>  #define _ASM_POWERPC_VAS_H
> +#include <linux/sched/mm.h>
> +#include <linux/mmu_context.h>
> +#include <asm/icswx.h>
>  #include <uapi/asm/vas-api.h>
>  
>  
> @@ -60,6 +63,22 @@ struct vas_user_win_ops {
>  	int (*close_win)(void *);
>  };
>  
> +struct vas_win_task {
> +	struct pid *pid;	/* Thread group ID of owner */
> +	struct pid *tgid;	/* Linux process mm_struct */
> +	struct mm_struct *mm;	/* Linux process mm_struct */
> +};

Looks okay, happy with struct vas_win_task? (and vas_user_win_ops)?

I'd be happier to have everything related to vas windows prefixed with 
vas_window_ consistently, and have _user be present always for userspace
windows, but you have to read and type it.

> +
> +static inline void vas_drop_reference_task(struct vas_win_task *task)

This is not dropping a reference task, but a task reference. And it's 
not really a task reference as far as Linux understands, but a
reference on pid (not task) and mm related to an open vas window. And
specifically a user window (with corresponding vas_user_win_ops).

Could it be called a 'struct vas_window_user_ref' instead?

Thanks,
Nick

> +{
> +	/* Drop references to pid and mm */
> +	put_pid(task->pid);
> +	if (task->mm) {
> +		mm_context_remove_vas_window(task->mm);
> +		mmdrop(task->mm);
> +	}

^ permalink raw reply

* Re: [V3 PATCH 04/16] powerpc/vas: Move update_csb/dump_crb to common book3s platform
From: Nicholas Piggin @ 2021-05-10  5:37 UTC (permalink / raw)
  To: Haren Myneni, herbert, linux-crypto, linuxppc-dev, mpe
In-Reply-To: <f20654f88f746fe1dd26d27a783a04ae036e8833.camel@linux.ibm.com>

Excerpts from Haren Myneni's message of April 18, 2021 7:03 am:
> 
> NX issues an interrupt when sees fault on user space buffer.

If a coprocessor encounters an error translating an address, the VAS 
will cause an interrupt in the host.


> The
> kernel processes the fault by updating CSB. This functionality is
> same for both powerNV and pseries. So this patch moves these
> functions to common vas-api.c and the actual functionality is not
> changed.
> 
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
>  arch/powerpc/include/asm/vas.h             |   3 +
>  arch/powerpc/platforms/book3s/vas-api.c    | 146 ++++++++++++++++++-
>  arch/powerpc/platforms/powernv/vas-fault.c | 155 ++-------------------
>  3 files changed, 157 insertions(+), 147 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/vas.h b/arch/powerpc/include/asm/vas.h
> index 2daaa1a2a9a9..66bf8fb1a1be 100644
> --- a/arch/powerpc/include/asm/vas.h
> +++ b/arch/powerpc/include/asm/vas.h
> @@ -210,4 +210,7 @@ int vas_register_coproc_api(struct module *mod, enum vas_cop_type cop_type,
>  void vas_unregister_coproc_api(void);
>  
>  int vas_reference_task(struct vas_win_task *vtask);
> +void vas_update_csb(struct coprocessor_request_block *crb,
> +		    struct vas_win_task *vtask);
> +void vas_dump_crb(struct coprocessor_request_block *crb);
>  #endif /* __ASM_POWERPC_VAS_H */
> diff --git a/arch/powerpc/platforms/book3s/vas-api.c b/arch/powerpc/platforms/book3s/vas-api.c
> index d98caa734154..dc131b2e4acd 100644
> --- a/arch/powerpc/platforms/book3s/vas-api.c
> +++ b/arch/powerpc/platforms/book3s/vas-api.c
> @@ -111,6 +111,150 @@ int vas_reference_task(struct vas_win_task *vtask)
>  	return 0;
>  }
>  
> +/*
> + * Update the CSB to indicate a translation error.
> + *
> + * User space will be polling on CSB after the request is issued.
> + * If NX can handle the request without any issues, it updates CSB.
> + * Whereas if NX encounters page fault, the kernel will handle the
> + * fault and update CSB with translation error.
> + *
> + * If we are unable to update the CSB means copy_to_user failed due to
> + * invalid csb_addr, send a signal to the process.
> + */
> +void vas_update_csb(struct coprocessor_request_block *crb,
> +		    struct vas_win_task *vtask)
> +{
> +	struct coprocessor_status_block csb;
> +	struct kernel_siginfo info;
> +	struct task_struct *tsk;
> +	void __user *csb_addr;
> +	struct pid *pid;
> +	int rc;
> +
> +	/*
> +	 * NX user space windows can not be opened for task->mm=NULL
> +	 * and faults will not be generated for kernel requests.
> +	 */
> +	if (WARN_ON_ONCE(!vtask->mm))
> +		return;
> +
> +	csb_addr = (void __user *)be64_to_cpu(crb->csb_addr);
> +
> +	memset(&csb, 0, sizeof(csb));
> +	csb.cc = CSB_CC_FAULT_ADDRESS;
> +	csb.ce = CSB_CE_TERMINATION;
> +	csb.cs = 0;
> +	csb.count = 0;
> +
> +	/*
> +	 * NX operates and returns in BE format as defined CRB struct.
> +	 * So saves fault_storage_addr in BE as NX pastes in FIFO and
> +	 * expects user space to convert to CPU format.
> +	 */
> +	csb.address = crb->stamp.nx.fault_storage_addr;
> +	csb.flags = 0;
> +
> +	pid = vtask->pid;
> +	tsk = get_pid_task(pid, PIDTYPE_PID);
> +	/*
> +	 * Process closes send window after all pending NX requests are
> +	 * completed. In multi-thread applications, a child thread can
> +	 * open a window and can exit without closing it. May be some
> +	 * requests are pending or this window can be used by other
> +	 * threads later. We should handle faults if NX encounters
> +	 * pages faults on these requests. Update CSB with translation
> +	 * error and fault address. If csb_addr passed by user space is
> +	 * invalid, send SEGV signal to pid saved in window. If the
> +	 * child thread is not running, send the signal to tgid.
> +	 * Parent thread (tgid) will close this window upon its exit.
> +	 *
> +	 * pid and mm references are taken when window is opened by
> +	 * process (pid). So tgid is used only when child thread opens
> +	 * a window and exits without closing it.
> +	 */
> +	if (!tsk) {
> +		pid = vtask->tgid;
> +		tsk = get_pid_task(pid, PIDTYPE_PID);
> +		/*
> +		 * Parent thread (tgid) will be closing window when it
> +		 * exits. So should not get here.
> +		 */
> +		if (WARN_ON_ONCE(!tsk))
> +			return;
> +	}
> +
> +	/* Return if the task is exiting. */
> +	if (tsk->flags & PF_EXITING) {
> +		put_task_struct(tsk);
> +		return;
> +	}
> +
> +	kthread_use_mm(vtask->mm);
> +	rc = copy_to_user(csb_addr, &csb, sizeof(csb));
> +	/*
> +	 * User space polls on csb.flags (first byte). So add barrier
> +	 * then copy first byte with csb flags update.
> +	 */
> +	if (!rc) {
> +		csb.flags = CSB_V;
> +		/* Make sure update to csb.flags is visible now */
> +		smp_mb();
> +		rc = copy_to_user(csb_addr, &csb, sizeof(u8));
> +	}
> +	kthread_unuse_mm(vtask->mm);
> +	put_task_struct(tsk);
> +
> +	/* Success */
> +	if (!rc)
> +		return;
> +
> +
> +	pr_debug("Invalid CSB address 0x%p signalling pid(%d)\n",
> +			csb_addr, pid_vnr(pid));
> +
> +	clear_siginfo(&info);
> +	info.si_signo = SIGSEGV;
> +	info.si_errno = EFAULT;
> +	info.si_code = SEGV_MAPERR;
> +	info.si_addr = csb_addr;
> +	/*
> +	 * process will be polling on csb.flags after request is sent to
> +	 * NX. So generally CSB update should not fail except when an
> +	 * application passes invalid csb_addr. So an error message will
> +	 * be displayed and leave it to user space whether to ignore or
> +	 * handle this signal.
> +	 */
> +	rcu_read_lock();
> +	rc = kill_pid_info(SIGSEGV, &info, pid);
> +	rcu_read_unlock();
> +
> +	pr_devel("%s(): pid %d kill_proc_info() rc %d\n", __func__,
> +			pid_vnr(pid), rc);
> +}
> +
> +void vas_dump_crb(struct coprocessor_request_block *crb)
> +{
> +	struct data_descriptor_entry *dde;
> +	struct nx_fault_stamp *nx;
> +
> +	dde = &crb->source;
> +	pr_devel("SrcDDE: addr 0x%llx, len %d, count %d, idx %d, flags %d\n",
> +		be64_to_cpu(dde->address), be32_to_cpu(dde->length),
> +		dde->count, dde->index, dde->flags);
> +
> +	dde = &crb->target;
> +	pr_devel("TgtDDE: addr 0x%llx, len %d, count %d, idx %d, flags %d\n",
> +		be64_to_cpu(dde->address), be32_to_cpu(dde->length),
> +		dde->count, dde->index, dde->flags);
> +
> +	nx = &crb->stamp.nx;
> +	pr_devel("NX Stamp: PSWID 0x%x, FSA 0x%llx, flags 0x%x, FS 0x%x\n",
> +		be32_to_cpu(nx->pswid),
> +		be64_to_cpu(crb->stamp.nx.fault_storage_addr),
> +		nx->flags, nx->fault_status);
> +}
> +
>  static int coproc_open(struct inode *inode, struct file *fp)
>  {
>  	struct coproc_instance *cp_inst;
> @@ -272,7 +416,7 @@ static struct file_operations coproc_fops = {
>   * extended to other coprocessor types later.
>   */
>  int vas_register_coproc_api(struct module *mod, enum vas_cop_type cop_type,
> -			const char *name, struct vas_user_win_ops *vops)
> +			    const char *name, struct vas_user_win_ops *vops)
>  {
>  	int rc = -EINVAL;
>  	dev_t devno;

This change should go back where you added the code. But you've 
brought back vas_register_corpoc_api? In... patch 2, by the looks
which just renames them back. Perhaps think about keeping the
vas_register_coproc_api in patch 1 in that case, and just adding
the new powernv specific wrapper over it.

But for this patch,

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

Thanks,
Nick


^ permalink raw reply

* Re: [V3 PATCH 05/16] powerpc/vas:  Define and use common vas_window struct
From: Nicholas Piggin @ 2021-05-10  5:37 UTC (permalink / raw)
  To: Haren Myneni, herbert, linux-crypto, linuxppc-dev, mpe
In-Reply-To: <293d2b3ba726c8e2a61ccca52165c4fb1e9e593e.camel@linux.ibm.com>

Excerpts from Haren Myneni's message of April 18, 2021 7:04 am:
> 
> 

Empty email?

Thanks,
Nick

^ permalink raw reply

* Re: [V3 PATCH 06/16] powerpc/pseries/vas: Define VAS/NXGZIP HCALLs and structs
From: Nicholas Piggin @ 2021-05-10  5:49 UTC (permalink / raw)
  To: Haren Myneni, herbert, linux-crypto, linuxppc-dev, mpe
In-Reply-To: <286ae5f4fdc4fd5620470cb0bf452e827e1f3864.camel@linux.ibm.com>

Excerpts from Haren Myneni's message of April 18, 2021 7:05 am:
> 
> This patch adds HCALLs and other definitions. Also define structs
> that are used in VAS implementation on powerVM.
> 
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
>  arch/powerpc/include/asm/hvcall.h    |  7 ++
>  arch/powerpc/include/asm/vas.h       | 28 ++++++++
>  arch/powerpc/platforms/pseries/vas.h | 96 ++++++++++++++++++++++++++++
>  3 files changed, 131 insertions(+)
>  create mode 100644 arch/powerpc/platforms/pseries/vas.h
> 
> diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
> index ed6086d57b22..accbb7f6f272 100644
> --- a/arch/powerpc/include/asm/hvcall.h
> +++ b/arch/powerpc/include/asm/hvcall.h
> @@ -294,6 +294,13 @@
>  #define H_RESIZE_HPT_COMMIT	0x370
>  #define H_REGISTER_PROC_TBL	0x37C
>  #define H_SIGNAL_SYS_RESET	0x380
> +#define	H_ALLOCATE_VAS_WINDOW	0x388
> +#define	H_MODIFY_VAS_WINDOW	0x38C
> +#define	H_DEALLOCATE_VAS_WINDOW	0x390
> +#define	H_QUERY_VAS_WINDOW	0x394
> +#define	H_QUERY_VAS_CAPABILITIES	0x398
> +#define	H_QUERY_NX_CAPABILITIES	0x39C
> +#define	H_GET_NX_FAULT		0x3A0

These should be spaces.

>  #define H_INT_GET_SOURCE_INFO   0x3A8
>  #define H_INT_SET_SOURCE_CONFIG 0x3AC
>  #define H_INT_GET_SOURCE_CONFIG 0x3B0
> diff --git a/arch/powerpc/include/asm/vas.h b/arch/powerpc/include/asm/vas.h
> index f928bf4c7e98..d15784506a54 100644
> --- a/arch/powerpc/include/asm/vas.h
> +++ b/arch/powerpc/include/asm/vas.h
> @@ -179,6 +179,7 @@ struct vas_tx_win_attr {
>  	bool rx_win_ord_mode;
>  };
>  
> +#ifdef CONFIG_PPC_POWERNV
>  /*
>   * Helper to map a chip id to VAS id.
>   * For POWER9, this is a 1:1 mapping. In the future this maybe a 1:N
> @@ -243,6 +244,33 @@ int vas_paste_crb(struct vas_window *win, int offset, bool re);
>  int vas_register_api_powernv(struct module *mod, enum vas_cop_type cop_type,
>  			     const char *name);
>  void vas_unregister_api_powernv(void);
> +#endif
> +
> +#ifdef CONFIG_PPC_PSERIES
> +
> +/* VAS Capabilities */
> +#define VAS_GZIP_QOS_FEAT	0x1
> +#define VAS_GZIP_DEF_FEAT	0x2
> +#define VAS_GZIP_QOS_FEAT_BIT	(1UL << (63 - VAS_GZIP_QOS_FEAT)) /* Bit 1 */
> +#define VAS_GZIP_DEF_FEAT_BIT	(1UL << (63 - VAS_GZIP_DEF_FEAT)) /* Bit 2 */

Use PPC_BIT for these.

> +
> +/* NX Capabilities */
> +#define	VAS_NX_GZIP_FEAT	0x1
> +#define	VAS_NX_GZIP_FEAT_BIT	(1UL << (63 - VAS_NX_GZIP_FEAT)) /* Bit 1 */
> +#define	VAS_DESCR_LEN		8
> +
> +struct vas_all_capabs_be {
> +		__be64  descriptor;
> +		__be64  feat_type;
> +} __packed __aligned(0x1000);
> +
> +struct vas_all_capabs {
> +	char	name[VAS_DESCR_LEN + 1];
> +	u64     descriptor;
> +	u64     feat_type;
> +};

You're using _be for the struct that is passed to the hcall, and a 
non-postfixed one for something the driver uses internally? It seems
like buf or buffer, or hv_ prefix is typically used rather than be (host 
kernel could be BE).

struct hv_query_vas_capabilities_buffer for example.

Does the hcall really require 0x1000 alignment?

> +
> +#endif
>  
>  /*
>   * Register / unregister coprocessor type to VAS API which will be exported
> diff --git a/arch/powerpc/platforms/pseries/vas.h b/arch/powerpc/platforms/pseries/vas.h
> new file mode 100644
> index 000000000000..208682fffa57
> --- /dev/null
> +++ b/arch/powerpc/platforms/pseries/vas.h
> @@ -0,0 +1,96 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright 2020-21 IBM Corp.
> + */
> +
> +#ifndef _VAS_H
> +#define _VAS_H
> +#include <asm/vas.h>
> +#include <linux/mutex.h>
> +#include <linux/stringify.h>
> +
> +/*
> + * VAS window modify flags
> + */
> +#define	VAS_MOD_WIN_CLOSE	(1UL << 63)
> +#define	VAS_MOD_WIN_JOBS_KILL	(1UL << (63 - 1))
> +#define	VAS_MOD_WIN_DR		(1UL << (63 - 3))
> +#define	VAS_MOD_WIN_PR		(1UL << (63 - 4))
> +#define	VAS_MOD_WIN_SF		(1UL << (63 - 5))
> +#define	VAS_MOD_WIN_TA		(1UL << (63 - 6))
> +#define	VAS_MOD_WIN_FLAGS	(VAS_MOD_WIN_JOBS_KILL | VAS_MOD_WIN_DR | \
> +				VAS_MOD_WIN_PR | VAS_MOD_WIN_SF)
> +
> +#define	VAS_WIN_ACTIVE		0x0
> +#define	VAS_WIN_CLOSED		0x1
> +#define	VAS_WIN_INACTIVE	0x2	/* Inactive due to HW failure */
> +/* Process of being modified, deallocated, or quiesced */
> +#define	VAS_WIN_MOD_IN_PROCESS	0x3
> +
> +#define	VAS_COPY_PASTE_USER_MODE	0x00000001
> +#define	VAS_COP_OP_USER_MODE		0x00000010
> +
> +/*
> + * Co-processor feature - GZIP QoS windows or GZIP default windows
> + */
> +enum vas_cop_feat_type {
> +	VAS_GZIP_QOS_FEAT_TYPE,
> +	VAS_GZIP_DEF_FEAT_TYPE,
> +	VAS_MAX_FEAT_TYPE,
> +};
> +
> +struct vas_ct_capabs_be {
> +	__be64	descriptor;
> +	u8	win_type;		/* Default or QoS type */
> +	u8	user_mode;
> +	__be16	max_lpar_creds;
> +	__be16	max_win_creds;
> +	union {
> +		__be16	reserved;
> +		__be16	def_lpar_creds; /* Used for default capabilities */
> +	};
> +	__be16	target_lpar_creds;
> +} __packed __aligned(0x1000);
> +
> +struct vas_ct_capabs {
> +	char		name[VAS_DESCR_LEN + 1];
> +	u64		descriptor;
> +	u8		win_type;	/* Default or QoS type */
> +	u8		user_mode;	/* User mode copy/paste or COP HCALL */
> +	u16		max_lpar_creds;	/* Max credits available in LPAR */
> +	/* Max credits can be assigned per window */
> +	u16		max_win_creds;
> +	union {
> +		u16	reserved;	/* Used for QoS credit type */
> +		u16	def_lpar_creds; /* Used for default credit type */
> +	};
> +	/* Total LPAR available credits. Can be different from max LPAR */
> +	/* credits due to DLPAR operation */
> +	atomic_t	target_lpar_creds;
> +	atomic_t	used_lpar_creds; /* Used credits so far */
> +	u16		avail_lpar_creds; /* Remaining available credits */
> +};
> +
> +struct vas_capabs {
> +	struct vas_ct_capabs capab;
> +	struct list_head list;
> +};
> +
> +struct vas_win_lpar_be {
> +	__be16	version;
> +	u8	win_type;
> +	u8	status;
> +	__be16	credits;	/* No of credits assigned to this window */
> +	__be16	reserved;
> +	__be32	pid;		/* LPAR Process ID */
> +	__be32	tid;		/* LPAR Thread ID */
> +	__be64	win_addr;
> +	__be32	interrupt;	/* Interrupt when NX request completes */
> +	__be32	fault;		/* Interrupt when NX sees fault */
> +	/* Associativity Domain Identifiers as returned in */
> +	/* H_HOME_NODE_ASSOCIATIVITY */
> +	__be64	domain[6];
> +	__be64	win_util;	/* Number of bytes processed */
> +} __packed __aligned(0x1000);
> +
> +#endif /* _VAS_H */
> -- 
> 2.18.2
> 
> 
> 

^ permalink raw reply

* Re: [V3 PATCH 07/16] powerpc/vas: Define QoS credit flag to allocate window
From: Nicholas Piggin @ 2021-05-10  5:54 UTC (permalink / raw)
  To: Haren Myneni, herbert, linux-crypto, linuxppc-dev, mpe
In-Reply-To: <90328d5aa92016434f3061ec7cadc812ea2c5dbc.camel@linux.ibm.com>

Excerpts from Haren Myneni's message of April 18, 2021 7:06 am:
> 
> pHyp introduces two different type of credits: Default and Quality
> of service (QoS).
> 
> The total number of default credits available on each LPAR depends
> on CPU resources configured. But these credits can be shared or
> over-committed across LPARs in shared mode which can result in
> paste command failure (RMA_busy). To avoid NX HW contention, phyp
> introduces QoS credit type which makes sure guaranteed access to NX
> resources. The system admins can assign QoS credits for each LPAR
> via HMC.
> 
> Default credit type is used to allocate a VAS window by default as
> on powerVM implementation. But the process can pass VAS_WIN_QOS_CREDITS


There's some interchanging of pHyp and PowerVM in the series.

PowerVM is probably the better term to use, with uppercase P.
Unless you mean PAPR or pseries etc.

I think you can say the PAPR VAS spec has two different types of 
credits, rather than say a specific hypervisor is introducing them.

> flag with VAS_TX_WIN_OPEN ioctl to open QoS type window.
> 
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
>  arch/powerpc/include/uapi/asm/vas-api.h | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/include/uapi/asm/vas-api.h b/arch/powerpc/include/uapi/asm/vas-api.h
> index ebd4b2424785..eb7c8694174f 100644
> --- a/arch/powerpc/include/uapi/asm/vas-api.h
> +++ b/arch/powerpc/include/uapi/asm/vas-api.h
> @@ -13,11 +13,15 @@
>  #define VAS_MAGIC	'v'
>  #define VAS_TX_WIN_OPEN	_IOW(VAS_MAGIC, 0x20, struct vas_tx_win_open_attr)
>  
> +/* Flags to VAS TX open window ioctl */
> +/* To allocate a window with QoS credit, otherwise default credit is used */
> +#define	VAS_WIN_QOS_CREDITS	0x0000000000000001
> +
>  struct vas_tx_win_open_attr {

Some consistency of naming might help, VAS_TX_WIN_FLAG_QOS_CREDIT.

>  	__u32	version;
>  	__s16	vas_id;	/* specific instance of vas or -1 for default */
>  	__u16	reserved1;
> -	__u64	flags;	/* Future use */
> +	__u64	flags;
>  	__u64	reserved2[6];
>  };
>  
> -- 
> 2.18.2
> 
> 
> 

^ permalink raw reply

* Re: [V3 PATCH 08/16] powerpc/pseries/VAS: Implement allocate/modify/deallocate HCALLS
From: Nicholas Piggin @ 2021-05-10  6:01 UTC (permalink / raw)
  To: Haren Myneni, herbert, linux-crypto, linuxppc-dev, mpe
In-Reply-To: <e4c29e44cabee9197caa379e1260d2d7e33b20c7.camel@linux.ibm.com>

Excerpts from Haren Myneni's message of April 18, 2021 7:07 am:
> 
> This patch adds the following HCALLs which are used to allocate,
> modify and deallocate VAS windows.

I don't like to be nitpicky about the language, but this is adding the 
hcall wrappers. Implementing the hcall would be adding it to KVM. 
Otherwise looks okay I think.

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

> H_ALLOCATE_VAS_WINDOW: Allocate VAS window
> H_DEALLOCATE_VAS_WINDOW: Close VAS window
> H_MODIFY_VAS_WINDOW: Setup window before using
> 
> Also adds phyp call (H_QUERY_VAS_CAPABILITIES) to get all VAS
> capabilities that phyp provides.

"PAPR hcall to get VAS capabilities provided by the hypervisor"

Thanks,
Nick

> 
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
>  arch/powerpc/platforms/pseries/vas.c | 217 +++++++++++++++++++++++++++
>  1 file changed, 217 insertions(+)
>  create mode 100644 arch/powerpc/platforms/pseries/vas.c
> 
> diff --git a/arch/powerpc/platforms/pseries/vas.c b/arch/powerpc/platforms/pseries/vas.c
> new file mode 100644
> index 000000000000..06960151477c
> --- /dev/null
> +++ b/arch/powerpc/platforms/pseries/vas.c
> @@ -0,0 +1,217 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright 2020-21 IBM Corp.
> + */
> +
> +#define pr_fmt(fmt) "vas: " fmt
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/export.h>
> +#include <linux/types.h>
> +#include <linux/delay.h>
> +#include <linux/slab.h>
> +#include <linux/irqdomain.h>
> +#include <linux/interrupt.h>
> +#include <linux/sched/mm.h>
> +#include <linux/mmu_context.h>
> +#include <asm/hvcall.h>
> +#include <asm/hvconsole.h>
> +#include <asm/machdep.h>
> +#include <asm/plpar_wrappers.h>
> +#include <asm/vas.h>
> +#include "vas.h"
> +
> +#define	VAS_INVALID_WIN_ADDRESS	0xFFFFFFFFFFFFFFFFul
> +#define	VAS_DEFAULT_DOMAIN_ID	0xFFFFFFFFFFFFFFFFul
> +/* Authority Mask Register (AMR) value is not supported in */
> +/* linux implementation. So pass '0' to modify window HCALL */
> +#define	VAS_AMR_VALUE	0
> +/* phyp allows one credit per window right now */
> +#define DEF_WIN_CREDS		1
> +
> +static int64_t hcall_return_busy_check(int64_t rc)
> +{
> +	/* Check if we are stalled for some time */
> +	if (H_IS_LONG_BUSY(rc)) {
> +		msleep(get_longbusy_msecs(rc));
> +		rc = H_BUSY;
> +	} else if (rc == H_BUSY) {
> +		cond_resched();
> +	}
> +
> +	return rc;
> +}
> +
> +/*
> + * Allocate VAS window HCALL
> + */
> +static int plpar_vas_allocate_window(struct vas_window *win, u64 *domain,
> +				     u8 wintype, u16 credits)
> +{
> +	long retbuf[PLPAR_HCALL9_BUFSIZE] = {0};
> +	int64_t rc;
> +
> +	do {
> +		rc = plpar_hcall9(H_ALLOCATE_VAS_WINDOW, retbuf, wintype,
> +				  credits, domain[0], domain[1], domain[2],
> +				  domain[3], domain[4], domain[5]);
> +
> +		rc = hcall_return_busy_check(rc);
> +	} while (rc == H_BUSY);
> +
> +	switch (rc) {
> +	case H_SUCCESS:
> +		win->winid = retbuf[0];
> +		win->lpar.win_addr = retbuf[1];
> +		win->lpar.complete_irq = retbuf[2];
> +		win->lpar.fault_irq = retbuf[3];
> +		if (win->lpar.win_addr == VAS_INVALID_WIN_ADDRESS) {
> +			pr_err("HCALL(%x): COPY/PASTE is not supported\n",
> +				H_ALLOCATE_VAS_WINDOW);
> +			return -ENOTSUPP;
> +		}
> +		return 0;
> +	case H_PARAMETER:
> +		pr_err("HCALL(%x): Invalid window type (%u)\n",
> +			H_ALLOCATE_VAS_WINDOW, wintype);
> +		return -EINVAL;
> +	case H_P2:
> +		pr_err("HCALL(%x): Credits(%u) exceed maximum window credits\n",
> +			H_ALLOCATE_VAS_WINDOW, credits);
> +		return -EINVAL;
> +	case H_COP_HW:
> +		pr_err("HCALL(%x): User-mode COPY/PASTE is not supported\n",
> +			H_ALLOCATE_VAS_WINDOW);
> +		return -ENOTSUPP;
> +	case H_RESOURCE:
> +		pr_err("HCALL(%x): LPAR credit limit exceeds window limit\n",
> +			H_ALLOCATE_VAS_WINDOW);
> +		return -EPERM;
> +	case H_CONSTRAINED:
> +		pr_err("HCALL(%x): Credits (%u) are not available\n",
> +			H_ALLOCATE_VAS_WINDOW, credits);
> +		return -EPERM;
> +	default:
> +		pr_err("HCALL(%x): Unexpected error %lld\n",
> +			H_ALLOCATE_VAS_WINDOW, rc);
> +		return -EIO;
> +	}
> +}
> +
> +/*
> + * Deallocate VAS window HCALL.
> + */
> +static int plpar_vas_deallocate_window(u64 winid)
> +{
> +	int64_t rc;
> +
> +	do {
> +		rc = plpar_hcall_norets(H_DEALLOCATE_VAS_WINDOW, winid);
> +
> +		rc = hcall_return_busy_check(rc);
> +	} while (rc == H_BUSY);
> +
> +	switch (rc) {
> +	case H_SUCCESS:
> +		return 0;
> +	case H_PARAMETER:
> +		pr_err("HCALL(%x): Invalid window ID %llu\n",
> +			H_DEALLOCATE_VAS_WINDOW, winid);
> +		return -EINVAL;
> +	case H_STATE:
> +		pr_err("HCALL(%x): Window(%llu): Invalid page table entries\n",
> +			H_DEALLOCATE_VAS_WINDOW, winid);
> +		return -EPERM;
> +	default:
> +		pr_err("HCALL(%x): Unexpected error %lld for window(%llu)\n",
> +			H_DEALLOCATE_VAS_WINDOW, rc, winid);
> +		return -EIO;
> +	}
> +}
> +
> +/*
> + * Modify VAS window.
> + * After the window is opened with allocate window HCALL, configure it
> + * with flags and LPAR PID before using.
> + */
> +static int plpar_vas_modify_window(struct vas_window *win)
> +{
> +	int64_t rc;
> +	u32 lpid = mfspr(SPRN_PID);
> +
> +	/*
> +	 * AMR value is not supported in Linux implementation
> +	 * phyp ignores it if 0 is passed.
> +	 */
> +	do {
> +		rc = plpar_hcall_norets(H_MODIFY_VAS_WINDOW, win->winid,
> +					lpid, 0, VAS_MOD_WIN_FLAGS,
> +					VAS_AMR_VALUE);
> +
> +		rc = hcall_return_busy_check(rc);
> +	} while (rc == H_BUSY);
> +
> +	switch (rc) {
> +	case H_SUCCESS:
> +		return 0;
> +	case H_PARAMETER:
> +		pr_err("HCALL(%x): Invalid window ID %u\n",
> +			H_MODIFY_VAS_WINDOW, win->winid);
> +		return -EINVAL;
> +	case H_P2:
> +		pr_err("HCALL(%x): Window(%d): Invalid LPAR Process ID %u\n",
> +			H_MODIFY_VAS_WINDOW, lpid, win->winid);
> +		return -EINVAL;
> +	case H_P3:
> +		/* LPAR thread ID is deprecated on P10 */
> +		pr_err("HCALL(%x): Invalid LPAR Thread ID for window(%u)\n",
> +			H_MODIFY_VAS_WINDOW, win->winid);
> +		return -EINVAL;
> +	case H_STATE:
> +		pr_err("HCALL(%x): Jobs in progress, Can't modify window(%u)\n",
> +			H_MODIFY_VAS_WINDOW, win->winid);
> +		return -EBUSY;
> +	default:
> +		pr_err("HCALL(%x): Unexpected error %lld for window(%u)\n",
> +			H_MODIFY_VAS_WINDOW, rc, win->winid);
> +		return -EIO;
> +	}
> +}
> +
> +/*
> + * This HCALL is used to determine the capabilities that pHyp provides.
> + * @hcall: H_QUERY_VAS_CAPABILITIES or H_QUERY_NX_CAPABILITIES
> + * @query_type: If 0 is passed, phyp returns the overall capabilities
> + *		which provides all feature(s) that are available. Then
> + *		query phyp to get the corresponding capabilities for
> + *		the specific feature.
> + *		Example: H_QUERY_VAS_CAPABILITIES provides VAS GZIP QoS
> + *			and VAS GZIP Default capabilities.
> + *			H_QUERY_NX_CAPABILITIES provides NX GZIP
> + *			capabilities.
> + * @result: Return buffer to save capabilities.
> + */
> +int plpar_vas_query_capabilities(const u64 hcall, u8 query_type,
> +					u64 result)
> +{
> +	int64_t rc;
> +
> +	rc = plpar_hcall_norets(hcall, query_type, result);
> +
> +	switch (rc) {
> +	case H_SUCCESS:
> +		return 0;
> +	case H_PARAMETER:
> +		pr_err("HCALL(%llx): Invalid query type %u\n", hcall,
> +			query_type);
> +		return -EINVAL;
> +	case H_PRIVILEGE:
> +		pr_err("HCALL(%llx): Invalid result buffer 0x%llx\n",
> +			hcall, result);
> +		return -EACCES;
> +	default:
> +		pr_err("HCALL(%llx): Unexpected error %lld\n", hcall, rc);
> +		return -EIO;
> +	}
> +}
> -- 
> 2.18.2
> 
> 
> 

^ permalink raw reply

* Re: Kernel crosscompilers
From: Michael Ellerman @ 2021-05-10  6:07 UTC (permalink / raw)
  To: Christophe Leroy, Arnd Bergmann, linuxppc-dev@ozlabs.org,
	Segher Boessenkool
In-Reply-To: <1bf8523a-848a-7686-c179-7bedb18979ac@csgroup.eu>

Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 08/05/2021 à 08:46, Christophe Leroy a écrit :
>> Le 06/05/2021 à 16:17, Arnd Bergmann a écrit :
>>> On Thu, May 6, 2021 at 2:42 PM Christophe Leroy
>>> <christophe.leroy@csgroup.eu> wrote:
>>>>
>>>> Hello Arnd,
>>>>
>>>> May I ask you whether you plan to build cross compilers based on GCC 11.1 at
>>>> https://mirrors.edge.kernel.org/pub/tools/crosstool/ ?
>>>
>>> Hi Christophe,
>>>
>>> I've built a snapshot a few days before the release, that one is
>>> identical to 11.1
>>> except for the reported version number. I've tried to ask around for
>>> help testing
>>> this, but so far I have not heard from anyone.
>>>
>>> Building a new set of compilers takes around a day on my build box, so I want
>>> to make sure I don't have to do it more often than necessary. If you are able
>>> to give the binaries a spin, preferably on a ppc64le or arm64 host, please let
>>> me know how it goes and I'll rebuilt them on the release tag.
>>>
>> 
>> Hi Arnd,
>> 
>> I don't have any ppc or arm host I can build on.
>> I'm building on x86 for powerpc embedded boards.
>> 
>> I have tried your GCC 11 snapshot, I get something booting but it crashes when launching init.
>> 
>> [    7.368410] init[1]: bad frame in sys_sigreturn: 7fb2fd60 nip 001083cc lr 001083c4
>> [    7.376283] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
>> [    7.383680] CPU: 0 PID: 1 Comm: init Not tainted 5.12.0-s3k-dev-16316-g9e799d5df185 #5054
>> [    7.391767] Call Trace:
>> [    7.394174] [c9023db0] [c00211e8] panic+0x130/0x304 (unreliable)
>> [    7.400112] [c9023e10] [c0024e68] do_exit+0x874/0x910
>> [    7.405104] [c9023e50] [c0024f80] do_group_exit+0x40/0xc4
>> [    7.410440] [c9023e60] [c0033334] get_signal+0x1d8/0x93c
>> [    7.415689] [c9023ec0] [c0007f34] do_notify_resume+0x6c/0x314
>> [    7.421369] [c9023f20] [c000d580] syscall_exit_prepare+0x120/0x184
>> [    7.427479] [c9023f30] [c001101c] ret_from_syscall+0xc/0x28
>> 
>> Something is going wrong with asm goto output. I implemented get_user() helpers with asm goto this 
>> cycle (commit 5cd29b1fd3e8). I tested it with CLANG before submitting, it was working.
>> 
>> Seems like there is something wrong with it with GCC11. When forcing CONFIG_CC_HAS_ASM_GOTO_OUTPUT 
>> to 'n', the kernel boots ok.
>> 
>
> I found the problem, that's due to r10 register being reused by GCC in the copy loop below:
>
>    10:	7d 09 03 a6 	mtctr   r8
>    14:	80 ca 00 00 	lwz     r6,0(r10)
>    18:	80 ea 00 04 	lwz     r7,4(r10)
>    1c:	90 c9 00 08 	stw     r6,8(r9)
>    20:	90 e9 00 0c 	stw     r7,12(r9)
>    24:	39 0a 00 08 	addi    r8,r10,8
>    28:	39 29 00 08 	addi    r9,r9,8
> =>2c:	81 4a 00 08 	lwz     r10,8(r10)
>    30:	81 6a 00 0c 	lwz     r11,12(r10)
>    34:	91 49 00 08 	stw     r10,8(r9)
>    38:	91 69 00 0c 	stw     r11,12(r9)
>    3c:	39 48 00 08 	addi    r10,r8,8
>    40:	39 29 00 08 	addi    r9,r9,8
>    44:	42 00 ff d0 	bdnz    14 <__unsafe_restore_general_regs+0x14>
>
> earlyclobber modifier is missing in the CONFIG_CC_HAS_ASM_GOTO_OUTPUT version of __get_user_asm2_goto().

Thanks for tracking that down. I hit it last week when testing Arnd's
compilers but hadn't had time to find the root cause.

cheers

^ 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