LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] KVM: PPC: Book3S HV P9: Optimise loads around context switch
From: Fabiano Rosas @ 2022-01-24 17:29 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20220123114725.3549202-1-npiggin@gmail.com>

Nicholas Piggin <npiggin@gmail.com> writes:

> It is better to get all loads for the register values in flight
> before starting to switch LPID, PID, and LPCR because those
> mtSPRs are expensive and serialising.
>
> This also just tidies up the code for a potential future change
> to the context switching sequence.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>

^ permalink raw reply

* Re: Build regressions/improvements in v5.17-rc1
From: Alex Deucher @ 2022-01-24 18:51 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: alsa-devel, kvm, Network Development, linux-um, LKML,
	amd-gfx list, linux-mips, Lakshmi Sowjanya D, sparclinux,
	linuxppc-dev, Tobin C. Harding
In-Reply-To: <alpine.DEB.2.22.394.2201240851560.2674757@ramsan.of.borg>

On Mon, Jan 24, 2022 at 5:25 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> On Sun, 23 Jan 2022, Geert Uytterhoeven wrote:
> > Below is the list of build error/warning regressions/improvements in
> > v5.17-rc1[1] compared to v5.16[2].
> >
> > Summarized:
> >  - build errors: +17/-2
> >  - build warnings: +23/-25
> >
> > Note that there may be false regressions, as some logs are incomplete.
> > Still, they're build errors/warnings.
> >
> > Happy fixing! ;-)
> >
> > Thanks to the linux-next team for providing the build service.
> >
> > [1] http://kisskb.ellerman.id.au/kisskb/branch/linus/head/e783362eb54cd99b2cac8b3a9aeac942e6f6ac07/ (all 99 configs)
> > [2] http://kisskb.ellerman.id.au/kisskb/branch/linus/head/df0cc57e057f18e44dac8e6c18aba47ab53202f9/ (98 out of 99 configs)
> >
> >
> > *** ERRORS ***
> >
> > 17 error regressions:
> >  + /kisskb/src/arch/powerpc/kernel/stacktrace.c: error: implicit declaration of function 'nmi_cpu_backtrace' [-Werror=implicit-function-declaration]:  => 171:2
> >  + /kisskb/src/arch/powerpc/kernel/stacktrace.c: error: implicit declaration of function 'nmi_trigger_cpumask_backtrace' [-Werror=implicit-function-declaration]:  => 226:2
>
> powerpc-gcc5/skiroot_defconfig
>
> >  + /kisskb/src/arch/sparc/mm/srmmu.c: error: cast between incompatible function types from 'void (*)(long unsigned int)' to 'void (*)(long unsigned int,  long unsigned int,  long unsigned int,  long unsigned int,  long unsigned int)' [-Werror=cast-function-type]:  => 1756:13, 1639:13
> >  + /kisskb/src/arch/sparc/mm/srmmu.c: error: cast between incompatible function types from 'void (*)(struct mm_struct *)' to 'void (*)(long unsigned int,  long unsigned int,  long unsigned int,  long unsigned int,  long unsigned int)' [-Werror=cast-function-type]:  => 1674:29, 1662:29
> >  + /kisskb/src/arch/sparc/mm/srmmu.c: error: cast between incompatible function types from 'void (*)(struct mm_struct *, long unsigned int)' to 'void (*)(long unsigned int,  long unsigned int,  long unsigned int,  long unsigned int,  long unsigned int)' [-Werror=cast-function-type]:  => 1767:21
> >  + /kisskb/src/arch/sparc/mm/srmmu.c: error: cast between incompatible function types from 'void (*)(struct vm_area_struct *, long unsigned int)' to 'void (*)(long unsigned int,  long unsigned int,  long unsigned int,  long unsigned int,  long unsigned int)' [-Werror=cast-function-type]:  => 1741:29, 1726:29
> >  + /kisskb/src/arch/sparc/mm/srmmu.c: error: cast between incompatible function types from 'void (*)(struct vm_area_struct *, long unsigned int,  long unsigned int)' to 'void (*)(long unsigned int,  long unsigned int,  long unsigned int,  long unsigned int,  long unsigned int)' [-Werror=cast-function-type]:  => 1694:29, 1711:29
>
> sparc64-gcc11/sparc-allmodconfig
>
> >  + /kisskb/src/arch/um/include/asm/processor-generic.h: error: called object is not a function or function pointer:  => 103:18
> >  + /kisskb/src/drivers/vfio/pci/vfio_pci_rdwr.c: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]:  => 324:9, 317:9
> >  + /kisskb/src/drivers/vfio/pci/vfio_pci_rdwr.c: error: implicit declaration of function 'ioport_map' [-Werror=implicit-function-declaration]:  => 317:11
> >  + /kisskb/src/drivers/vfio/pci/vfio_pci_rdwr.c: error: implicit declaration of function 'ioport_unmap' [-Werror=implicit-function-declaration]:  => 338:15
>
> um-x86_64/um-allyesconfig
>
> >  + /kisskb/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c: error: control reaches end of non-void function [-Werror=return-type]:  => 1560:1

I don't really see what's going on here:

#ifdef CONFIG_X86_64
return cpu_data(first_cpu_of_numa_node).apicid;
#else
return first_cpu_of_numa_node;
#endif

Alex

>
> um-x86_64/um-all{mod,yes}config
>
> >  + /kisskb/src/drivers/net/ethernet/freescale/fec_mpc52xx.c: error: passing argument 2 of 'mpc52xx_fec_set_paddr' discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]:  => 659:29
>
> powerpc-gcc5/ppc32_allmodconfig
>
> >  + /kisskb/src/drivers/pinctrl/pinctrl-thunderbay.c: error: assignment discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]:  => 815:8, 815:29
>
> arm64-gcc5.4/arm64-allmodconfig
> arm64-gcc8/arm64-allmodconfig
>
> >  + /kisskb/src/lib/test_printf.c: error: "PTR" redefined [-Werror]:  => 247:0, 247
> >  + /kisskb/src/sound/pci/ca0106/ca0106.h: error: "PTR" redefined [-Werror]:  => 62, 62:0
>
> mips-gcc8/mips-allmodconfig
> mipsel/mips-allmodconfig
>
> >  + error: arch/powerpc/kvm/book3s_64_entry.o: relocation truncated to fit: R_PPC64_REL14 (stub) against symbol `machine_check_common' defined in .text section in arch/powerpc/kernel/head_64.o:  => (.text+0x3e4)
>
> powerpc-gcc5/powerpc-allyesconfig
>
> Gr{oetje,eeting}s,
>
>                                                 Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                                             -- Linus Torvalds

^ permalink raw reply

* Re: Build regressions/improvements in v5.17-rc1
From: Geert Uytterhoeven @ 2022-01-24 18:55 UTC (permalink / raw)
  To: Alex Deucher
  Cc: ALSA Development Mailing List, KVM list, Network Development,
	linux-um, LKML, amd-gfx list, open list:BROADCOM NVRAM DRIVER,
	Lakshmi Sowjanya D, sparclinux, linuxppc-dev, Tobin C. Harding
In-Reply-To: <CADnq5_MUq0fX7wMLJyUUxxa+2xoRinonL-TzD8tUhXALRfY8-A@mail.gmail.com>

Hi Alex,

On Mon, Jan 24, 2022 at 7:52 PM Alex Deucher <alexdeucher@gmail.com> wrote:
> On Mon, Jan 24, 2022 at 5:25 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Sun, 23 Jan 2022, Geert Uytterhoeven wrote:
> > >  + /kisskb/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c: error: control reaches end of non-void function [-Werror=return-type]:  => 1560:1
>
> I don't really see what's going on here:
>
> #ifdef CONFIG_X86_64
> return cpu_data(first_cpu_of_numa_node).apicid;
> #else
> return first_cpu_of_numa_node;
> #endif

Ah, the actual failure causing this was not included:

In file included from /kisskb/src/arch/x86/um/asm/processor.h:41:0,
                 from /kisskb/src/include/linux/mutex.h:19,
                 from /kisskb/src/include/linux/kernfs.h:11,
                 from /kisskb/src/include/linux/sysfs.h:16,
                 from /kisskb/src/include/linux/kobject.h:20,
                 from /kisskb/src/include/linux/pci.h:35,
                 from
/kisskb/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:25:
/kisskb/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c: In
function 'kfd_cpumask_to_apic_id':
/kisskb/src/arch/um/include/asm/processor-generic.h:103:18: error:
called object is not a function or function pointer
 #define cpu_data (&boot_cpu_data)
                  ^
/kisskb/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1556:9:
note: in expansion of macro 'cpu_data'
  return cpu_data(first_cpu_of_numa_node).apicid;
         ^
/kisskb/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1560:1:
error: control reaches end of non-void function [-Werror=return-type]
 }
 ^

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: Build regressions/improvements in v5.17-rc1
From: Randy Dunlap @ 2022-01-24 19:11 UTC (permalink / raw)
  To: Geert Uytterhoeven, Alex Deucher
  Cc: ALSA Development Mailing List, KVM list, Network Development,
	linux-um, LKML, amd-gfx list, open list:BROADCOM NVRAM DRIVER,
	Lakshmi Sowjanya D, sparclinux, linuxppc-dev, Tobin C. Harding
In-Reply-To: <CAMuHMdWUWqHYbbavtMT-XAD_sarDPC5xnc3c0pX1ZAh3Wuzuzg@mail.gmail.com>



On 1/24/22 10:55, Geert Uytterhoeven wrote:
> Hi Alex,
> 
> On Mon, Jan 24, 2022 at 7:52 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>> On Mon, Jan 24, 2022 at 5:25 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>> On Sun, 23 Jan 2022, Geert Uytterhoeven wrote:
>>>>  + /kisskb/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c: error: control reaches end of non-void function [-Werror=return-type]:  => 1560:1
>>
>> I don't really see what's going on here:
>>
>> #ifdef CONFIG_X86_64
>> return cpu_data(first_cpu_of_numa_node).apicid;
>> #else
>> return first_cpu_of_numa_node;
>> #endif
> 
> Ah, the actual failure causing this was not included:
> 
> In file included from /kisskb/src/arch/x86/um/asm/processor.h:41:0,
>                  from /kisskb/src/include/linux/mutex.h:19,
>                  from /kisskb/src/include/linux/kernfs.h:11,
>                  from /kisskb/src/include/linux/sysfs.h:16,
>                  from /kisskb/src/include/linux/kobject.h:20,
>                  from /kisskb/src/include/linux/pci.h:35,
>                  from
> /kisskb/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:25:
> /kisskb/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c: In
> function 'kfd_cpumask_to_apic_id':
> /kisskb/src/arch/um/include/asm/processor-generic.h:103:18: error:
> called object is not a function or function pointer
>  #define cpu_data (&boot_cpu_data)
>                   ^
> /kisskb/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1556:9:
> note: in expansion of macro 'cpu_data'
>   return cpu_data(first_cpu_of_numa_node).apicid;
>          ^
> /kisskb/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1560:1:
> error: control reaches end of non-void function [-Werror=return-type]
>  }
>  ^

ah yes, UML.
I have a bunch of UML fixes that I have been hesitant to post.

This is one of them.
What do people think about this?

thanks.

---
From: Randy Dunlap <rdunlap@infradead.org>


../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1556:9: note: in expansion of macro ‘cpu_data’
  return cpu_data(first_cpu_of_numa_node).apicid;
         ^~~~~~~~
../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1560:1: error: control reaches end of non-void function [-Werror=return-type]

../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c: In function ‘kfd_fill_iolink_info_for_cpu’:
../arch/um/include/asm/processor-generic.h:103:19: error: called object is not a function or function pointer
 #define cpu_data (&boot_cpu_data)
                  ~^~~~~~~~~~~~~~~
../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c:1688:27: note: in expansion of macro ‘cpu_data’
  struct cpuinfo_x86 *c = &cpu_data(0);
                           ^~~~~~~~
../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c:1691:7: error: dereferencing pointer to incomplete type ‘struct cpuinfo_x86’
  if (c->x86_vendor == X86_VENDOR_AMD)
       ^~
../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c:1691:23: error: ‘X86_VENDOR_AMD’ undeclared (first use in this function); did you mean ‘X86_VENDOR_ANY’?
  if (c->x86_vendor == X86_VENDOR_AMD)
                       ^~~~~~~~~~~~~~
                       X86_VENDOR_ANY

../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c: In function ‘kfd_create_vcrat_image_cpu’:
../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c:1742:11: warning: unused variable ‘entries’ [-Wunused-variable]
  uint32_t entries = 0;

Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
---
 drivers/gpu/drm/amd/amdkfd/kfd_crat.c     |    6 +++---
 drivers/gpu/drm/amd/amdkfd/kfd_topology.c |    2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

--- linux-next-20220107.orig/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
+++ linux-next-20220107/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
@@ -1552,7 +1552,7 @@ static int kfd_cpumask_to_apic_id(const
 	first_cpu_of_numa_node = cpumask_first(cpumask);
 	if (first_cpu_of_numa_node >= nr_cpu_ids)
 		return -1;
-#ifdef CONFIG_X86_64
+#if defined(CONFIG_X86_64) && !defined(CONFIG_UML)
 	return cpu_data(first_cpu_of_numa_node).apicid;
 #else
 	return first_cpu_of_numa_node;
--- linux-next-20220107.orig/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
+++ linux-next-20220107/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
@@ -1679,7 +1679,7 @@ static int kfd_fill_mem_info_for_cpu(int
 	return 0;
 }
 
-#ifdef CONFIG_X86_64
+#if defined(CONFIG_X86_64) && !defined(CONFIG_UML)
 static int kfd_fill_iolink_info_for_cpu(int numa_node_id, int *avail_size,
 				uint32_t *num_entries,
 				struct crat_subtype_iolink *sub_type_hdr)
@@ -1738,7 +1738,7 @@ static int kfd_create_vcrat_image_cpu(vo
 	struct crat_subtype_generic *sub_type_hdr;
 	int avail_size = *size;
 	int numa_node_id;
-#ifdef CONFIG_X86_64
+#if defined(CONFIG_X86_64) && !defined(CONFIG_UML)
 	uint32_t entries = 0;
 #endif
 	int ret = 0;
@@ -1803,7 +1803,7 @@ static int kfd_create_vcrat_image_cpu(vo
 			sub_type_hdr->length);
 
 		/* Fill in Subtype: IO Link */
-#ifdef CONFIG_X86_64
+#if defined(CONFIG_X86_64) && !defined(CONFIG_UML)
 		ret = kfd_fill_iolink_info_for_cpu(numa_node_id, &avail_size,
 				&entries,
 				(struct crat_subtype_iolink *)sub_type_hdr);



^ permalink raw reply

* [PATCH v4] powerpc/papr_scm: Implement initial support for injecting smart errors
From: Vaibhav Jain @ 2022-01-24 20:22 UTC (permalink / raw)
  To: nvdimm, linuxppc-dev
  Cc: Shivaprasad G Bhat, Aneesh Kumar K . V, Vaibhav Jain,
	Dan Williams, Ira Weiny

Presently PAPR doesn't support injecting smart errors on an
NVDIMM. This makes testing the NVDIMM health reporting functionality
difficult as simulating NVDIMM health related events need a hacked up
qemu version.

To solve this problem this patch proposes simulating certain set of
NVDIMM health related events in papr_scm. Specifically 'fatal' health
state and 'dirty' shutdown state. These error can be injected via the
user-space 'ndctl-inject-smart(1)' command. With the proposed patch and
corresponding ndctl patches following command flow is expected:

$ sudo ndctl list -DH -d nmem0
...
      "health_state":"ok",
      "shutdown_state":"clean",
...
 # inject unsafe shutdown and fatal health error
$ sudo ndctl inject-smart nmem0 -Uf
...
      "health_state":"fatal",
      "shutdown_state":"dirty",
...
 # uninject all errors
$ sudo ndctl inject-smart nmem0 -N
...
      "health_state":"ok",
      "shutdown_state":"clean",
...

The patch adds a new member 'health_bitmap_inject_mask' inside struct
papr_scm_priv which is then bitwise ANDed to the health bitmap fetched from the
hypervisor. The value for 'health_bitmap_inject_mask' is accessible from sysfs
at nmemX/papr/health_bitmap_inject.

A new PDSM named 'SMART_INJECT' is proposed that accepts newly
introduced 'struct nd_papr_pdsm_smart_inject' as payload thats
exchanged between libndctl and papr_scm to indicate the requested
smart-error states.

When the processing the PDSM 'SMART_INJECT', papr_pdsm_smart_inject()
constructs a pair or 'inject_mask' and 'clear_mask' bitmaps from the payload
and bit-blt it to the 'health_bitmap_inject_mask'. This ensures the after being
fetched from the hypervisor, the health_bitmap reflects requested smart-error
states.

Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
---
Changelog:

Since v3:
* Renamed the sysfs entry from 'health_bitmap_override' to
'health_bitmap_inject'.
* Simplified the variable names and removed the 'health_bitmap_{mask,override}'
members. Instead replaced them with a single 'health_bitmap_inject_mask'
member. [Aneesh]
* Updated the sysfs documentations and commit description.
* Used READ/WRITE_ONCE macros at places where 'health_bitmap_inject_mask' may be
accessed concurrently.

Since v2:
* Rebased the patch to ppc-next
* Added documentation for newly introduced sysfs attribute 'health_bitmap_override'

Since v1:
* Updated the patch description.
* Removed dependency of a header movement patch.
* Removed '__packed' attribute for 'struct nd_papr_pdsm_smart_inject' [Aneesh]
---
 Documentation/ABI/testing/sysfs-bus-papr-pmem | 12 +++
 arch/powerpc/include/uapi/asm/papr_pdsm.h     | 18 ++++
 arch/powerpc/platforms/pseries/papr_scm.c     | 90 ++++++++++++++++++-
 3 files changed, 117 insertions(+), 3 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-papr-pmem b/Documentation/ABI/testing/sysfs-bus-papr-pmem
index 95254cec92bf..4ac0673901e7 100644
--- a/Documentation/ABI/testing/sysfs-bus-papr-pmem
+++ b/Documentation/ABI/testing/sysfs-bus-papr-pmem
@@ -61,3 +61,15 @@ Description:
 		* "CchRHCnt" : Cache Read Hit Count
 		* "CchWHCnt" : Cache Write Hit Count
 		* "FastWCnt" : Fast Write Count
+
+What:		/sys/bus/nd/devices/nmemX/papr/health_bitmap_inject
+Date:		Jan, 2022
+KernelVersion:	v5.17
+Contact:	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>, nvdimm@lists.linux.dev,
+Description:
+		(RO) Reports the health bitmap inject bitmap that is applied to
+		bitmap received from PowerVM via the H_SCM_HEALTH. This is used
+		to forcibly set specific bits returned from Hcall. These is then
+		used to simulate various health or shutdown states for an nvdimm
+		and are set by user-space tools like ndctl by issuing a PAPR DSM.
+
diff --git a/arch/powerpc/include/uapi/asm/papr_pdsm.h b/arch/powerpc/include/uapi/asm/papr_pdsm.h
index 82488b1e7276..17439925045c 100644
--- a/arch/powerpc/include/uapi/asm/papr_pdsm.h
+++ b/arch/powerpc/include/uapi/asm/papr_pdsm.h
@@ -116,6 +116,22 @@ struct nd_papr_pdsm_health {
 	};
 };
 
+/* Flags for injecting specific smart errors */
+#define PDSM_SMART_INJECT_HEALTH_FATAL		(1 << 0)
+#define PDSM_SMART_INJECT_BAD_SHUTDOWN		(1 << 1)
+
+struct nd_papr_pdsm_smart_inject {
+	union {
+		struct {
+			/* One or more of PDSM_SMART_INJECT_ */
+			__u32 flags;
+			__u8 fatal_enable;
+			__u8 unsafe_shutdown_enable;
+		};
+		__u8 buf[ND_PDSM_PAYLOAD_MAX_SIZE];
+	};
+};
+
 /*
  * Methods to be embedded in ND_CMD_CALL request. These are sent to the kernel
  * via 'nd_cmd_pkg.nd_command' member of the ioctl struct
@@ -123,12 +139,14 @@ struct nd_papr_pdsm_health {
 enum papr_pdsm {
 	PAPR_PDSM_MIN = 0x0,
 	PAPR_PDSM_HEALTH,
+	PAPR_PDSM_SMART_INJECT,
 	PAPR_PDSM_MAX,
 };
 
 /* Maximal union that can hold all possible payload types */
 union nd_pdsm_payload {
 	struct nd_papr_pdsm_health health;
+	struct nd_papr_pdsm_smart_inject smart_inject;
 	__u8 buf[ND_PDSM_PAYLOAD_MAX_SIZE];
 } __packed;
 
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index f48e87ac89c9..20aafd387840 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -120,6 +120,10 @@ struct papr_scm_priv {
 
 	/* length of the stat buffer as expected by phyp */
 	size_t stat_buffer_len;
+
+	/* The bits which needs to be overridden */
+	u64 health_bitmap_inject_mask;
+
 };
 
 static int papr_scm_pmem_flush(struct nd_region *nd_region,
@@ -347,19 +351,29 @@ static ssize_t drc_pmem_query_stats(struct papr_scm_priv *p,
 static int __drc_pmem_query_health(struct papr_scm_priv *p)
 {
 	unsigned long ret[PLPAR_HCALL_BUFSIZE];
+	u64 bitmap = 0;
 	long rc;
 
 	/* issue the hcall */
 	rc = plpar_hcall(H_SCM_HEALTH, ret, p->drc_index);
-	if (rc != H_SUCCESS) {
+	if (rc == H_SUCCESS)
+		bitmap = ret[0] & ret[1];
+	else if (rc == H_FUNCTION)
+		dev_info_once(&p->pdev->dev,
+			      "Hcall H_SCM_HEALTH not implemented, assuming empty health bitmap");
+	else {
+
 		dev_err(&p->pdev->dev,
 			"Failed to query health information, Err:%ld\n", rc);
 		return -ENXIO;
 	}
 
 	p->lasthealth_jiffies = jiffies;
-	p->health_bitmap = ret[0] & ret[1];
-
+	/* Allow injecting specific health bits via inject mask. */
+	if (p->health_bitmap_inject_mask)
+		bitmap = (bitmap & ~p->health_bitmap_inject_mask) |
+			p->health_bitmap_inject_mask;
+	WRITE_ONCE(p->health_bitmap, bitmap);
 	dev_dbg(&p->pdev->dev,
 		"Queried dimm health info. Bitmap:0x%016lx Mask:0x%016lx\n",
 		ret[0], ret[1]);
@@ -669,6 +683,56 @@ static int papr_pdsm_health(struct papr_scm_priv *p,
 	return rc;
 }
 
+/* Inject a smart error Add the dirty-shutdown-counter value to the pdsm */
+static int papr_pdsm_smart_inject(struct papr_scm_priv *p,
+				  union nd_pdsm_payload *payload)
+{
+	int rc;
+	u32 supported_flags = 0;
+	u64 inject_mask = 0, clear_mask = 0;
+	u64 mask;
+
+	/* Check for individual smart error flags and update inject/clear masks */
+	if (payload->smart_inject.flags & PDSM_SMART_INJECT_HEALTH_FATAL) {
+		supported_flags |= PDSM_SMART_INJECT_HEALTH_FATAL;
+		if (payload->smart_inject.fatal_enable)
+			inject_mask |= PAPR_PMEM_HEALTH_FATAL;
+		else
+			clear_mask |= PAPR_PMEM_HEALTH_FATAL;
+	}
+
+	if (payload->smart_inject.flags & PDSM_SMART_INJECT_BAD_SHUTDOWN) {
+		supported_flags |= PDSM_SMART_INJECT_BAD_SHUTDOWN;
+		if (payload->smart_inject.unsafe_shutdown_enable)
+			inject_mask |= PAPR_PMEM_SHUTDOWN_DIRTY;
+		else
+			clear_mask |= PAPR_PMEM_SHUTDOWN_DIRTY;
+	}
+
+	dev_dbg(&p->pdev->dev, "[Smart-inject] inject_mask=%#llx clear_mask=%#llx\n",
+		inject_mask, clear_mask);
+
+	/* Prevent concurrent access to dimm health bitmap related members */
+	rc = mutex_lock_interruptible(&p->health_mutex);
+	if (rc)
+		return rc;
+
+	/* Use inject/clear masks to set health_bitmap_inject_mask */
+	mask = READ_ONCE(p->health_bitmap_inject_mask);
+	mask = (mask & ~clear_mask) | inject_mask;
+	WRITE_ONCE(p->health_bitmap_inject_mask, mask);
+
+	/* Invalidate cached health bitmap */
+	p->lasthealth_jiffies = 0;
+
+	mutex_unlock(&p->health_mutex);
+
+	/* Return the supported flags back to userspace */
+	payload->smart_inject.flags = supported_flags;
+
+	return sizeof(struct nd_papr_pdsm_health);
+}
+
 /*
  * 'struct pdsm_cmd_desc'
  * Identifies supported PDSMs' expected length of in/out payloads
@@ -702,6 +766,12 @@ static const struct pdsm_cmd_desc __pdsm_cmd_descriptors[] = {
 		.size_out = sizeof(struct nd_papr_pdsm_health),
 		.service = papr_pdsm_health,
 	},
+
+	[PAPR_PDSM_SMART_INJECT] = {
+		.size_in = sizeof(struct nd_papr_pdsm_smart_inject),
+		.size_out = sizeof(struct nd_papr_pdsm_smart_inject),
+		.service = papr_pdsm_smart_inject,
+	},
 	/* Empty */
 	[PAPR_PDSM_MAX] = {
 		.size_in = 0,
@@ -838,6 +908,19 @@ static int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc,
 	return 0;
 }
 
+static ssize_t health_bitmap_inject_show(struct device *dev,
+					 struct device_attribute *attr,
+					 char *buf)
+{
+	struct nvdimm *dimm = to_nvdimm(dev);
+	struct papr_scm_priv *p = nvdimm_provider_data(dimm);
+
+	return sprintf(buf, "%#llx\n",
+		       READ_ONCE(p->health_bitmap_inject_mask));
+}
+
+static DEVICE_ATTR_ADMIN_RO(health_bitmap_inject);
+
 static ssize_t perf_stats_show(struct device *dev,
 			       struct device_attribute *attr, char *buf)
 {
@@ -952,6 +1035,7 @@ static struct attribute *papr_nd_attributes[] = {
 	&dev_attr_flags.attr,
 	&dev_attr_perf_stats.attr,
 	&dev_attr_dirty_shutdown.attr,
+	&dev_attr_health_bitmap_inject.attr,
 	NULL,
 };
 
-- 
2.34.1


^ permalink raw reply related

* Re: [powerpc] ftrace warning kernel/trace/ftrace.c:2068 with code-patching selftests
From: Yinan Liu @ 2022-01-24 12:15 UTC (permalink / raw)
  To: Sachin Sant, linuxppc-dev, Steven Rostedt
In-Reply-To: <944D10DA-8200-4BA9-8D0A-3BED9AA99F82@linux.ibm.com>

在 2022/1/24 下午5:19, Sachin Sant 写道:
> While running stress_code_patching test from selftests/powerpc/mm
> against 5.17-rc1 booted on a POWER10 LPAR following ftrace warning
> is seen:
> 
> WARNING: CPU: 1 PID: 2017392 at kernel/trace/ftrace.c:2068 ftrace_bug+0x274/0x2d8
> Modules linked in: dm_mod bonding rfkill sunrpc pseries_rng xts vmx_crypto uio_pdrv_genirq uio sch_fq_codel ip_tables ext4 mbcache jbd2 sd_mod t10_pi sg ibmvscsi ibmveth scsi_transport_srp fuse
> CPU: 1 PID: 2017392 Comm: stress_code_pat Not tainted 5.17.0-rc1-gdd81e1c7d5fb #1
> NIP:  c0000000002d561c LR: c0000000002d5618 CTR: 00000000005b4448
> REGS: c0000000332fb760 TRAP: 0700   Not tainted  (5.17.0-rc1-gdd81e1c7d5fb)
> MSR:  800000000282b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE>  CR: 48228224  XER: 00000009
> CFAR: c0000000001f6b00 IRQMASK: 0
> GPR00: c0000000002d5618 c0000000332fba00 c000000002a20000 0000000000000022
> GPR04: 00000000ffff7fff c0000000332fb720 c0000000332fb718 0000000000000027
> GPR08: c00000167cca7e10 0000000000000001 0000000000000027 c0000000028d6d08
> GPR12: 0000000000008000 c00000167fa30780 0000000040000000 00007fff9a089798
> GPR16: 00007fff9a089724 00007fff9a026be8 00007fff99fbf4f0 00007fff9a08d568
> GPR20: 00007fffce533ed0 0000000000000001 00007fff9a0399d8 00007fffd9eccf94
> GPR24: 0000000000000001 0000000000000000 c0000000332fbc70 c000000000fb0d18
> GPR28: c000000000ff5080 c000000000fadd38 c0000000020032ec c0000000070800a8
> NIP [c0000000002d561c] ftrace_bug+0x274/0x2d8
> LR [c0000000002d5618] ftrace_bug+0x270/0x2d8

Hi, Steven and Sachin

I don't have a powerpc machine for testing, I guess the ppc has a 
similar problem with the s390. It's not clear to me why the compiler 
does this. Maybe we can handle ppc like you did with the s390 before, 
but I'm not sure if other architectures have similar issues. Or limit 
BUILDTIME_MCOUNT_SORT to a smaller scope and make it only available for 
x86 and arm?

steven, what's your opinion?


Best regards
--yinan


^ permalink raw reply

* Re: [PATCH 6/7] modules: Add CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
From: Doug Anderson @ 2022-01-24 21:43 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: linux-arch@vger.kernel.org, Daniel Thompson,
	kgdb-bugreport@lists.sourceforge.net, Jason Wessel,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Luis Chamberlain, Jessica Yu, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <848d857871f457f4df37e80fad468d615b237c24.1643015752.git.christophe.leroy@csgroup.eu>

Hi,

On Mon, Jan 24, 2022 at 1:22 AM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
> --- a/kernel/debug/kdb/kdb_main.c
> +++ b/kernel/debug/kdb/kdb_main.c
> @@ -2022,8 +2022,11 @@ static int kdb_lsmod(int argc, const char **argv)
>                 if (mod->state == MODULE_STATE_UNFORMED)
>                         continue;
>
> -               kdb_printf("%-20s%8u  0x%px ", mod->name,
> -                          mod->core_layout.size, (void *)mod);
> +               kdb_printf("%-20s%8u", mod->name, mod->core_layout.size);
> +#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
> +               kdb_printf("/%8u  0x%px ", mod->data_layout.size);

Just counting percentages and arguments, it seems like something's
wrong in the above print statement.

-Doug

^ permalink raw reply

* [PATCH v2 1/4] KVM: PPC: Book3S HV: Check return value of kvmppc_radix_init
From: Fabiano Rosas @ 2022-01-24 22:08 UTC (permalink / raw)
  To: kvm-ppc; +Cc: linuxppc-dev, npiggin, aik
In-Reply-To: <20220124220803.1011673-1-farosas@linux.ibm.com>

The return of the function is being shadowed by the call to
kvmppc_uvmem_init.

Fixes: ca9f4942670c ("KVM: PPC: Book3S HV: Support for running secure guests")
Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kvm/book3s_hv.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index d1817cd9a691..3a3845f366d4 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -6138,8 +6138,11 @@ static int kvmppc_book3s_init_hv(void)
 	if (r)
 		return r;
 
-	if (kvmppc_radix_possible())
+	if (kvmppc_radix_possible()) {
 		r = kvmppc_radix_init();
+		if (r)
+			return r;
+	}
 
 	r = kvmppc_uvmem_init();
 	if (r < 0)
-- 
2.34.1


^ permalink raw reply related

* [PATCH v2 2/4] KVM: PPC: Book3S HV: Delay setting of kvm ops
From: Fabiano Rosas @ 2022-01-24 22:08 UTC (permalink / raw)
  To: kvm-ppc; +Cc: linuxppc-dev, npiggin, aik
In-Reply-To: <20220124220803.1011673-1-farosas@linux.ibm.com>

Delay the setting of kvm_hv_ops until after all init code has
completed. This avoids leaving the ops still accessible if the init
fails.

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
 arch/powerpc/kvm/book3s_hv.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 3a3845f366d4..b9aace212599 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -6127,9 +6127,6 @@ static int kvmppc_book3s_init_hv(void)
 	}
 #endif
 
-	kvm_ops_hv.owner = THIS_MODULE;
-	kvmppc_hv_ops = &kvm_ops_hv;
-
 	init_default_hcalls();
 
 	init_vcore_lists();
@@ -6145,10 +6142,15 @@ static int kvmppc_book3s_init_hv(void)
 	}
 
 	r = kvmppc_uvmem_init();
-	if (r < 0)
+	if (r < 0) {
 		pr_err("KVM-HV: kvmppc_uvmem_init failed %d\n", r);
+		return r;
+	}
 
-	return r;
+	kvm_ops_hv.owner = THIS_MODULE;
+	kvmppc_hv_ops = &kvm_ops_hv;
+
+	return 0;
 }
 
 static void kvmppc_book3s_exit_hv(void)
-- 
2.34.1


^ permalink raw reply related

* [PATCH v2 3/4] KVM: PPC: Book3S HV: Free allocated memory if module init fails
From: Fabiano Rosas @ 2022-01-24 22:08 UTC (permalink / raw)
  To: kvm-ppc; +Cc: linuxppc-dev, npiggin, aik
In-Reply-To: <20220124220803.1011673-1-farosas@linux.ibm.com>

The module's exit function is not called when the init fails, we need
to do cleanup before returning.

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kvm/book3s_hv.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index b9aace212599..87a49651a402 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -6104,7 +6104,7 @@ static int kvmppc_book3s_init_hv(void)
 	if (!cpu_has_feature(CPU_FTR_ARCH_300)) {
 		r = kvm_init_subcore_bitmap();
 		if (r)
-			return r;
+			goto err;
 	}
 
 	/*
@@ -6120,7 +6120,8 @@ static int kvmppc_book3s_init_hv(void)
 		np = of_find_compatible_node(NULL, NULL, "ibm,opal-intc");
 		if (!np) {
 			pr_err("KVM-HV: Cannot determine method for accessing XICS\n");
-			return -ENODEV;
+			r = -ENODEV;
+			goto err;
 		}
 		/* presence of intc confirmed - node can be dropped again */
 		of_node_put(np);
@@ -6133,12 +6134,12 @@ static int kvmppc_book3s_init_hv(void)
 
 	r = kvmppc_mmu_hv_init();
 	if (r)
-		return r;
+		goto err;
 
 	if (kvmppc_radix_possible()) {
 		r = kvmppc_radix_init();
 		if (r)
-			return r;
+			goto err;
 	}
 
 	r = kvmppc_uvmem_init();
@@ -6151,6 +6152,12 @@ static int kvmppc_book3s_init_hv(void)
 	kvmppc_hv_ops = &kvm_ops_hv;
 
 	return 0;
+
+err:
+	kvmhv_nested_exit();
+	kvmppc_radix_exit();
+
+	return r;
 }
 
 static void kvmppc_book3s_exit_hv(void)
-- 
2.34.1


^ permalink raw reply related

* [PATCH v2 4/4] KVM: PPC: Decrement module refcount if init_vm fails
From: Fabiano Rosas @ 2022-01-24 22:08 UTC (permalink / raw)
  To: kvm-ppc; +Cc: linuxppc-dev, npiggin, aik
In-Reply-To: <20220124220803.1011673-1-farosas@linux.ibm.com>

We increment the reference count for KVM-HV/PR before the call to
kvmppc_core_init_vm. If that function fails we need to decrement the
refcount.

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
Caught this while testing Nick's LPID patches by looking at
/sys/module/kvm_hv/refcnt
---
 arch/powerpc/kvm/powerpc.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 2ad0ccd202d5..4285d0eac900 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -431,6 +431,8 @@ int kvm_arch_check_processor_compat(void *opaque)
 int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 {
 	struct kvmppc_ops *kvm_ops = NULL;
+	int r;
+
 	/*
 	 * if we have both HV and PR enabled, default is HV
 	 */
@@ -456,7 +458,10 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 		return -ENOENT;
 
 	kvm->arch.kvm_ops = kvm_ops;
-	return kvmppc_core_init_vm(kvm);
+	r = kvmppc_core_init_vm(kvm);
+	if (r)
+		module_put(kvm->arch.kvm_ops->owner);
+	return r;
 err_out:
 	return -EINVAL;
 }
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH 2/6] KVM: PPC: Book3S HV: Update LPID allocator init for POWER9, Nested
From: Fabiano Rosas @ 2022-01-24 22:13 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20220123120043.3586018-3-npiggin@gmail.com>

Nicholas Piggin <npiggin@gmail.com> writes:

> The LPID allocator init is changed to:
> - use mmu_lpid_bits rather than hard-coding;
> - use KVM_MAX_NESTED_GUESTS for nested hypervisors;
> - not reserve the top LPID on POWER9 and newer CPUs.
>
> The reserved LPID is made a POWER7/8-specific detail.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>

>  arch/powerpc/include/asm/kvm_book3s_asm.h |  2 +-
>  arch/powerpc/include/asm/reg.h            |  2 --
>  arch/powerpc/kvm/book3s_64_mmu_hv.c       | 29 ++++++++++++++++-------
>  arch/powerpc/kvm/book3s_hv_rmhandlers.S   |  8 +++++++
>  arch/powerpc/mm/init_64.c                 |  3 +++
>  5 files changed, 33 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_book3s_asm.h b/arch/powerpc/include/asm/kvm_book3s_asm.h
> index b6d31bff5209..e6bda70b1d93 100644
> --- a/arch/powerpc/include/asm/kvm_book3s_asm.h
> +++ b/arch/powerpc/include/asm/kvm_book3s_asm.h
> @@ -15,7 +15,7 @@
>  #define XICS_IPI		2	/* interrupt source # for IPIs */
>
>  /* LPIDs we support with this build -- runtime limit may be lower */
> -#define KVMPPC_NR_LPIDS			(LPID_RSVD + 1)
> +#define KVMPPC_NR_LPIDS			(1UL << 12)
>
>  /* Maximum number of threads per physical core */
>  #define MAX_SMT_THREADS		8
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index 1e14324c5190..1e8b2e04e626 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
> @@ -473,8 +473,6 @@
>  #ifndef SPRN_LPID
>  #define SPRN_LPID	0x13F	/* Logical Partition Identifier */
>  #endif
> -#define   LPID_RSVD_POWER7	0x3ff	/* Reserved LPID for partn switching */
> -#define   LPID_RSVD		0xfff	/* Reserved LPID for partn switching */
>  #define	SPRN_HMER	0x150	/* Hypervisor maintenance exception reg */
>  #define   HMER_DEBUG_TRIG	(1ul << (63 - 17)) /* Debug trigger */
>  #define	SPRN_HMEER	0x151	/* Hyp maintenance exception enable reg */
> diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> index 09fc52b6f390..5be92d5bc099 100644
> --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
> +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> @@ -256,7 +256,7 @@ void kvmppc_map_vrma(struct kvm_vcpu *vcpu, struct kvm_memory_slot *memslot,
>
>  int kvmppc_mmu_hv_init(void)
>  {
> -	unsigned long rsvd_lpid;
> +	unsigned long nr_lpids;
>
>  	if (!mmu_has_feature(MMU_FTR_LOCKLESS_TLBIE))
>  		return -EINVAL;
> @@ -264,16 +264,29 @@ int kvmppc_mmu_hv_init(void)
>  	if (cpu_has_feature(CPU_FTR_HVMODE)) {
>  		if (WARN_ON(mfspr(SPRN_LPID) != 0))
>  			return -EINVAL;
> +		nr_lpids = 1UL << mmu_lpid_bits;
> +	} else {
> +		nr_lpids = KVM_MAX_NESTED_GUESTS;
>  	}
>
> -	/* POWER8 and above have 12-bit LPIDs (10-bit in POWER7) */
> -	if (cpu_has_feature(CPU_FTR_ARCH_207S))
> -		rsvd_lpid = LPID_RSVD;
> -	else
> -		rsvd_lpid = LPID_RSVD_POWER7;
> +	if (nr_lpids > KVMPPC_NR_LPIDS)
> +		nr_lpids = KVMPPC_NR_LPIDS;
> +
> +	if (!cpu_has_feature(CPU_FTR_ARCH_300)) {
> +		/* POWER7 has 10-bit LPIDs, POWER8 has 12-bit LPIDs */
> +		if (cpu_has_feature(CPU_FTR_ARCH_207S))
> +			WARN_ON(nr_lpids != 1UL << 12);
> +		else
> +			WARN_ON(nr_lpids != 1UL << 10);
> +
> +		/*
> +		 * Reserve the last implemented LPID use in partition
> +		 * switching for POWER7 and POWER8.
> +		 */
> +		nr_lpids -= 1;
> +	}
>
> -	/* rsvd_lpid is reserved for use in partition switching */
> -	kvmppc_init_lpid(rsvd_lpid);
> +	kvmppc_init_lpid(nr_lpids);
>
>  	return 0;
>  }
> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> index d185dee26026..0c552885a032 100644
> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> @@ -50,6 +50,14 @@
>  #define STACK_SLOT_UAMOR	(SFS-88)
>  #define STACK_SLOT_FSCR		(SFS-96)
>
> +/*
> + * Use the last LPID (all implemented LPID bits = 1) for partition switching.
> + * This is reserved in the LPID allocator. POWER7 only implements 0x3ff, but
> + * we write 0xfff into the LPID SPR anyway, which seems to work and just
> + * ignores the top bits.
> + */
> +#define   LPID_RSVD		0xfff
> +
>  /*
>   * Call kvmppc_hv_entry in real mode.
>   * Must be called with interrupts hard-disabled.
> diff --git a/arch/powerpc/mm/init_64.c b/arch/powerpc/mm/init_64.c
> index 35f46bf54281..ad1a41e3ff1c 100644
> --- a/arch/powerpc/mm/init_64.c
> +++ b/arch/powerpc/mm/init_64.c
> @@ -371,6 +371,9 @@ void register_page_bootmem_memmap(unsigned long section_nr,
>
>  #ifdef CONFIG_PPC_BOOK3S_64
>  unsigned int mmu_lpid_bits;
> +#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
> +EXPORT_SYMBOL_GPL(mmu_lpid_bits);
> +#endif
>  unsigned int mmu_pid_bits;
>
>  static bool disable_radix = !IS_ENABLED(CONFIG_PPC_RADIX_MMU_DEFAULT);

^ permalink raw reply

* Re: [PATCH 3/6] KVM: PPC: Book3S HV: Use IDA allocator for LPID allocator
From: Fabiano Rosas @ 2022-01-24 22:13 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20220123120043.3586018-4-npiggin@gmail.com>

Nicholas Piggin <npiggin@gmail.com> writes:

> This removes the fixed-size lpid_inuse array.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>

>  arch/powerpc/kvm/powerpc.c | 25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 102993462872..c527a5751b46 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -2453,20 +2453,22 @@ long kvm_arch_vm_ioctl(struct file *filp,
>  	return r;
>  }
>
> -static unsigned long lpid_inuse[BITS_TO_LONGS(KVMPPC_NR_LPIDS)];
> +static DEFINE_IDA(lpid_inuse);
>  static unsigned long nr_lpids;
>
>  long kvmppc_alloc_lpid(void)
>  {
> -	long lpid;
> +	int lpid;
>
> -	do {
> -		lpid = find_first_zero_bit(lpid_inuse, KVMPPC_NR_LPIDS);
> -		if (lpid >= nr_lpids) {
> +	/* The host LPID must always be 0 (allocation starts at 1) */
> +	lpid = ida_alloc_range(&lpid_inuse, 1, nr_lpids - 1, GFP_KERNEL);
> +	if (lpid < 0) {
> +		if (lpid == -ENOMEM)
> +			pr_err("%s: Out of memory\n", __func__);
> +		else
>  			pr_err("%s: No LPIDs free\n", __func__);
> -			return -ENOMEM;
> -		}
> -	} while (test_and_set_bit(lpid, lpid_inuse));
> +		return -ENOMEM;
> +	}
>
>  	return lpid;
>  }
> @@ -2474,15 +2476,14 @@ EXPORT_SYMBOL_GPL(kvmppc_alloc_lpid);
>
>  void kvmppc_free_lpid(long lpid)
>  {
> -	clear_bit(lpid, lpid_inuse);
> +	ida_free(&lpid_inuse, lpid);
>  }
>  EXPORT_SYMBOL_GPL(kvmppc_free_lpid);
>
> +/* nr_lpids_param includes the host LPID */
>  void kvmppc_init_lpid(unsigned long nr_lpids_param)
>  {
> -	nr_lpids = min_t(unsigned long, KVMPPC_NR_LPIDS, nr_lpids_param);
> -	memset(lpid_inuse, 0, sizeof(lpid_inuse));
> -	set_bit(0, lpid_inuse); /* The host LPID must always be 0 */
> +	nr_lpids = nr_lpids_param;
>  }
>  EXPORT_SYMBOL_GPL(kvmppc_init_lpid);

^ permalink raw reply

* Re: [PATCH 4/6] KVM: PPC: Book3S HV Nested: Change nested guest lookup to use idr
From: Fabiano Rosas @ 2022-01-24 22:14 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20220123120043.3586018-5-npiggin@gmail.com>

Nicholas Piggin <npiggin@gmail.com> writes:

> This removes the fixed sized kvm->arch.nested_guests array.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>

>  arch/powerpc/include/asm/kvm_host.h |   3 +-
>  arch/powerpc/kvm/book3s_hv_nested.c | 110 +++++++++++++++-------------
>  2 files changed, 59 insertions(+), 54 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
> index d9bf60bf0816..5fd0564e5c94 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -326,8 +326,7 @@ struct kvm_arch {
>  	struct list_head uvmem_pfns;
>  	struct mutex mmu_setup_lock;	/* nests inside vcpu mutexes */
>  	u64 l1_ptcr;
> -	int max_nested_lpid;
> -	struct kvm_nested_guest *nested_guests[KVM_MAX_NESTED_GUESTS];
> +	struct idr kvm_nested_guest_idr;
>  	/* This array can grow quite large, keep it at the end */
>  	struct kvmppc_vcore *vcores[KVM_MAX_VCORES];
>  #endif
> diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
> index 9d373f8963ee..1eff969b095c 100644
> --- a/arch/powerpc/kvm/book3s_hv_nested.c
> +++ b/arch/powerpc/kvm/book3s_hv_nested.c
> @@ -521,11 +521,6 @@ static void kvmhv_set_nested_ptbl(struct kvm_nested_guest *gp)
>  	kvmhv_set_ptbl_entry(gp->shadow_lpid, dw0, gp->process_table);
>  }
>
> -void kvmhv_vm_nested_init(struct kvm *kvm)
> -{
> -	kvm->arch.max_nested_lpid = -1;
> -}
> -
>  /*
>   * Handle the H_SET_PARTITION_TABLE hcall.
>   * r4 = guest real address of partition table + log_2(size) - 12
> @@ -660,6 +655,35 @@ static void kvmhv_update_ptbl_cache(struct kvm_nested_guest *gp)
>  	kvmhv_set_nested_ptbl(gp);
>  }
>
> +void kvmhv_vm_nested_init(struct kvm *kvm)
> +{
> +	idr_init(&kvm->arch.kvm_nested_guest_idr);
> +}
> +
> +static struct kvm_nested_guest *__find_nested(struct kvm *kvm, int lpid)
> +{
> +	return idr_find(&kvm->arch.kvm_nested_guest_idr, lpid);
> +}
> +
> +static bool __prealloc_nested(struct kvm *kvm, int lpid)
> +{
> +	if (idr_alloc(&kvm->arch.kvm_nested_guest_idr,
> +				NULL, lpid, lpid + 1, GFP_KERNEL) != lpid)
> +		return false;
> +	return true;
> +}
> +
> +static void __add_nested(struct kvm *kvm, int lpid, struct kvm_nested_guest *gp)
> +{
> +	if (idr_replace(&kvm->arch.kvm_nested_guest_idr, gp, lpid))
> +		WARN_ON(1);
> +}
> +
> +static void __remove_nested(struct kvm *kvm, int lpid)
> +{
> +	idr_remove(&kvm->arch.kvm_nested_guest_idr, lpid);
> +}
> +
>  static struct kvm_nested_guest *kvmhv_alloc_nested(struct kvm *kvm, unsigned int lpid)
>  {
>  	struct kvm_nested_guest *gp;
> @@ -720,13 +744,8 @@ static void kvmhv_remove_nested(struct kvm_nested_guest *gp)
>  	long ref;
>
>  	spin_lock(&kvm->mmu_lock);
> -	if (gp == kvm->arch.nested_guests[lpid]) {
> -		kvm->arch.nested_guests[lpid] = NULL;
> -		if (lpid == kvm->arch.max_nested_lpid) {
> -			while (--lpid >= 0 && !kvm->arch.nested_guests[lpid])
> -				;
> -			kvm->arch.max_nested_lpid = lpid;
> -		}
> +	if (gp == __find_nested(kvm, lpid)) {
> +		__remove_nested(kvm, lpid);
>  		--gp->refcnt;
>  	}
>  	ref = gp->refcnt;
> @@ -743,24 +762,22 @@ static void kvmhv_remove_nested(struct kvm_nested_guest *gp)
>   */
>  void kvmhv_release_all_nested(struct kvm *kvm)
>  {
> -	int i;
> +	int lpid;
>  	struct kvm_nested_guest *gp;
>  	struct kvm_nested_guest *freelist = NULL;
>  	struct kvm_memory_slot *memslot;
>  	int srcu_idx, bkt;
>
>  	spin_lock(&kvm->mmu_lock);
> -	for (i = 0; i <= kvm->arch.max_nested_lpid; i++) {
> -		gp = kvm->arch.nested_guests[i];
> -		if (!gp)
> -			continue;
> -		kvm->arch.nested_guests[i] = NULL;
> +	idr_for_each_entry(&kvm->arch.kvm_nested_guest_idr, gp, lpid) {
> +		__remove_nested(kvm, lpid);
>  		if (--gp->refcnt == 0) {
>  			gp->next = freelist;
>  			freelist = gp;
>  		}
>  	}
> -	kvm->arch.max_nested_lpid = -1;
> +	idr_destroy(&kvm->arch.kvm_nested_guest_idr);
> +	/* idr is empty and may be reused at this point */
>  	spin_unlock(&kvm->mmu_lock);
>  	while ((gp = freelist) != NULL) {
>  		freelist = gp->next;
> @@ -797,7 +814,7 @@ struct kvm_nested_guest *kvmhv_get_nested(struct kvm *kvm, int l1_lpid,
>  		return NULL;
>
>  	spin_lock(&kvm->mmu_lock);
> -	gp = kvm->arch.nested_guests[l1_lpid];
> +	gp = __find_nested(kvm, l1_lpid);
>  	if (gp)
>  		++gp->refcnt;
>  	spin_unlock(&kvm->mmu_lock);
> @@ -808,17 +825,19 @@ struct kvm_nested_guest *kvmhv_get_nested(struct kvm *kvm, int l1_lpid,
>  	newgp = kvmhv_alloc_nested(kvm, l1_lpid);
>  	if (!newgp)
>  		return NULL;
> +
> +	if (!__prealloc_nested(kvm, l1_lpid)) {
> +		kvmhv_release_nested(newgp);
> +		return NULL;
> +	}
> +
>  	spin_lock(&kvm->mmu_lock);
> -	if (kvm->arch.nested_guests[l1_lpid]) {
> -		/* someone else beat us to it */
> -		gp = kvm->arch.nested_guests[l1_lpid];
> -	} else {
> -		kvm->arch.nested_guests[l1_lpid] = newgp;
> +	gp = __find_nested(kvm, l1_lpid);
> +	if (!gp) {
> +		__add_nested(kvm, l1_lpid, newgp);
>  		++newgp->refcnt;
>  		gp = newgp;
>  		newgp = NULL;
> -		if (l1_lpid > kvm->arch.max_nested_lpid)
> -			kvm->arch.max_nested_lpid = l1_lpid;
>  	}
>  	++gp->refcnt;
>  	spin_unlock(&kvm->mmu_lock);
> @@ -841,20 +860,13 @@ void kvmhv_put_nested(struct kvm_nested_guest *gp)
>  		kvmhv_release_nested(gp);
>  }
>
> -static struct kvm_nested_guest *kvmhv_find_nested(struct kvm *kvm, int lpid)
> -{
> -	if (lpid > kvm->arch.max_nested_lpid)
> -		return NULL;
> -	return kvm->arch.nested_guests[lpid];
> -}
> -
>  pte_t *find_kvm_nested_guest_pte(struct kvm *kvm, unsigned long lpid,
>  				 unsigned long ea, unsigned *hshift)
>  {
>  	struct kvm_nested_guest *gp;
>  	pte_t *pte;
>
> -	gp = kvmhv_find_nested(kvm, lpid);
> +	gp = __find_nested(kvm, lpid);
>  	if (!gp)
>  		return NULL;
>
> @@ -960,7 +972,7 @@ static void kvmhv_remove_nest_rmap(struct kvm *kvm, u64 n_rmap,
>
>  	gpa = n_rmap & RMAP_NESTED_GPA_MASK;
>  	lpid = (n_rmap & RMAP_NESTED_LPID_MASK) >> RMAP_NESTED_LPID_SHIFT;
> -	gp = kvmhv_find_nested(kvm, lpid);
> +	gp = __find_nested(kvm, lpid);
>  	if (!gp)
>  		return;
>
> @@ -1152,16 +1164,13 @@ static void kvmhv_emulate_tlbie_all_lpid(struct kvm_vcpu *vcpu, int ric)
>  {
>  	struct kvm *kvm = vcpu->kvm;
>  	struct kvm_nested_guest *gp;
> -	int i;
> +	int lpid;
>
>  	spin_lock(&kvm->mmu_lock);
> -	for (i = 0; i <= kvm->arch.max_nested_lpid; i++) {
> -		gp = kvm->arch.nested_guests[i];
> -		if (gp) {
> -			spin_unlock(&kvm->mmu_lock);
> -			kvmhv_emulate_tlbie_lpid(vcpu, gp, ric);
> -			spin_lock(&kvm->mmu_lock);
> -		}
> +	idr_for_each_entry(&kvm->arch.kvm_nested_guest_idr, gp, lpid) {
> +		spin_unlock(&kvm->mmu_lock);
> +		kvmhv_emulate_tlbie_lpid(vcpu, gp, ric);
> +		spin_lock(&kvm->mmu_lock);
>  	}
>  	spin_unlock(&kvm->mmu_lock);
>  }
> @@ -1313,7 +1322,7 @@ long do_h_rpt_invalidate_pat(struct kvm_vcpu *vcpu, unsigned long lpid,
>  	 * H_ENTER_NESTED call. Since we can't differentiate this case from
>  	 * the invalid case, we ignore such flush requests and return success.
>  	 */
> -	if (!kvmhv_find_nested(vcpu->kvm, lpid))
> +	if (!__find_nested(vcpu->kvm, lpid))
>  		return H_SUCCESS;
>
>  	/*
> @@ -1657,15 +1666,12 @@ long int kvmhv_nested_page_fault(struct kvm_vcpu *vcpu)
>
>  int kvmhv_nested_next_lpid(struct kvm *kvm, int lpid)
>  {
> -	int ret = -1;
> +	int ret = lpid + 1;
>
>  	spin_lock(&kvm->mmu_lock);
> -	while (++lpid <= kvm->arch.max_nested_lpid) {
> -		if (kvm->arch.nested_guests[lpid]) {
> -			ret = lpid;
> -			break;
> -		}
> -	}
> +	if (!idr_get_next(&kvm->arch.kvm_nested_guest_idr, &ret))
> +		ret = -1;
>  	spin_unlock(&kvm->mmu_lock);
> +
>  	return ret;
>  }

^ permalink raw reply

* Re: [PATCH 5/6] KVM: PPC: Book3S Nested: Use explicit 4096 LPID maximum
From: Fabiano Rosas @ 2022-01-24 22:15 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20220123120043.3586018-6-npiggin@gmail.com>

Nicholas Piggin <npiggin@gmail.com> writes:

> Rather than tie this to KVMPPC_NR_LPIDS which is becoming more dynamic,
> fix it to 4096 (12-bits) explicitly for now.
>
> kvmhv_get_nested() does not have to check against KVM_MAX_NESTED_GUESTS
> because the L1 partition table registration hcall already did that, and
> it checks against the partition table size.
>
> This patch also puts all the partition table size calculations into the
> same form, using 12 for the architected size field shift and 4 for the
> shift corresponding to the partition table entry size.
>
> Signed-of-by: Nicholas Piggin <npiggin@gmail.com>

Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>

> ---
>  arch/powerpc/include/asm/kvm_host.h |  7 ++++++-
>  arch/powerpc/kvm/book3s_64_mmu_hv.c |  2 +-
>  arch/powerpc/kvm/book3s_hv_nested.c | 24 +++++++++++-------------
>  3 files changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
> index 5fd0564e5c94..e6fb03884dcc 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -34,7 +34,12 @@
>  #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
>  #include <asm/kvm_book3s_asm.h>		/* for MAX_SMT_THREADS */
>  #define KVM_MAX_VCPU_IDS	(MAX_SMT_THREADS * KVM_MAX_VCORES)
> -#define KVM_MAX_NESTED_GUESTS	KVMPPC_NR_LPIDS
> +
> +/*
> + * Limit the nested partition table to 4096 entries (because that's what
> + * hardware supports). Both guest and host use this value.
> + */
> +#define KVM_MAX_NESTED_GUESTS_SHIFT	12
>
>  #else
>  #define KVM_MAX_VCPU_IDS	KVM_MAX_VCPUS
> diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> index 5be92d5bc099..f983fb36cbf2 100644
> --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
> +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> @@ -266,7 +266,7 @@ int kvmppc_mmu_hv_init(void)
>  			return -EINVAL;
>  		nr_lpids = 1UL << mmu_lpid_bits;
>  	} else {
> -		nr_lpids = KVM_MAX_NESTED_GUESTS;
> +		nr_lpids = 1UL << KVM_MAX_NESTED_GUESTS_SHIFT;
>  	}
>
>  	if (nr_lpids > KVMPPC_NR_LPIDS)
> diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
> index 1eff969b095c..75169e0753ce 100644
> --- a/arch/powerpc/kvm/book3s_hv_nested.c
> +++ b/arch/powerpc/kvm/book3s_hv_nested.c
> @@ -439,10 +439,11 @@ long kvmhv_nested_init(void)
>  	if (!radix_enabled())
>  		return -ENODEV;
>
> -	/* find log base 2 of KVMPPC_NR_LPIDS, rounding up */
> -	ptb_order = __ilog2(KVMPPC_NR_LPIDS - 1) + 1;
> -	if (ptb_order < 8)
> -		ptb_order = 8;
> +	/* Partition table entry is 1<<4 bytes in size, hence the 4. */
> +	ptb_order = KVM_MAX_NESTED_GUESTS_SHIFT + 4;
> +	/* Minimum partition table size is 1<<12 bytes */
> +	if (ptb_order < 12)
> +		ptb_order = 12;
>  	pseries_partition_tb = kmalloc(sizeof(struct patb_entry) << ptb_order,
>  				       GFP_KERNEL);
>  	if (!pseries_partition_tb) {
> @@ -450,7 +451,7 @@ long kvmhv_nested_init(void)
>  		return -ENOMEM;
>  	}
>
> -	ptcr = __pa(pseries_partition_tb) | (ptb_order - 8);
> +	ptcr = __pa(pseries_partition_tb) | (ptb_order - 12);
>  	rc = plpar_hcall_norets(H_SET_PARTITION_TABLE, ptcr);
>  	if (rc != H_SUCCESS) {
>  		pr_err("kvm-hv: Parent hypervisor does not support nesting (rc=%ld)\n",
> @@ -534,16 +535,14 @@ long kvmhv_set_partition_table(struct kvm_vcpu *vcpu)
>  	long ret = H_SUCCESS;
>
>  	srcu_idx = srcu_read_lock(&kvm->srcu);
> -	/*
> -	 * Limit the partition table to 4096 entries (because that's what
> -	 * hardware supports), and check the base address.
> -	 */
> -	if ((ptcr & PRTS_MASK) > 12 - 8 ||
> +	/* Check partition size and base address. */
> +	if ((ptcr & PRTS_MASK) + 12 - 4 > KVM_MAX_NESTED_GUESTS_SHIFT ||
>  	    !kvm_is_visible_gfn(vcpu->kvm, (ptcr & PRTB_MASK) >> PAGE_SHIFT))
>  		ret = H_PARAMETER;
>  	srcu_read_unlock(&kvm->srcu, srcu_idx);
>  	if (ret == H_SUCCESS)
>  		kvm->arch.l1_ptcr = ptcr;
> +
>  	return ret;
>  }
>
> @@ -639,7 +638,7 @@ static void kvmhv_update_ptbl_cache(struct kvm_nested_guest *gp)
>
>  	ret = -EFAULT;
>  	ptbl_addr = (kvm->arch.l1_ptcr & PRTB_MASK) + (gp->l1_lpid << 4);
> -	if (gp->l1_lpid < (1ul << ((kvm->arch.l1_ptcr & PRTS_MASK) + 8))) {
> +	if (gp->l1_lpid < (1ul << ((kvm->arch.l1_ptcr & PRTS_MASK) + 12 - 4))) {
>  		int srcu_idx = srcu_read_lock(&kvm->srcu);
>  		ret = kvm_read_guest(kvm, ptbl_addr,
>  				     &ptbl_entry, sizeof(ptbl_entry));
> @@ -809,8 +808,7 @@ struct kvm_nested_guest *kvmhv_get_nested(struct kvm *kvm, int l1_lpid,
>  {
>  	struct kvm_nested_guest *gp, *newgp;
>
> -	if (l1_lpid >= KVM_MAX_NESTED_GUESTS ||
> -	    l1_lpid >= (1ul << ((kvm->arch.l1_ptcr & PRTS_MASK) + 12 - 4)))
> +	if (l1_lpid >= (1ul << ((kvm->arch.l1_ptcr & PRTS_MASK) + 12 - 4)))
>  		return NULL;
>
>  	spin_lock(&kvm->mmu_lock);

^ permalink raw reply

* Re: [PATCH 6/6] KVM: PPC: Book3S HV: Remove KVMPPC_NR_LPIDS
From: Fabiano Rosas @ 2022-01-24 22:15 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20220123120043.3586018-7-npiggin@gmail.com>

Nicholas Piggin <npiggin@gmail.com> writes:

> KVMPPC_NR_LPIDS no longer represents any size restriction on the
> LPID space and can be removed. A CPU with more than 12 LPID bits
> implemented will now be able to create more than 4095 guests.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>

> ---
>  arch/powerpc/include/asm/kvm_book3s_asm.h | 3 ---
>  arch/powerpc/kvm/book3s_64_mmu_hv.c       | 3 ---
>  2 files changed, 6 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_book3s_asm.h b/arch/powerpc/include/asm/kvm_book3s_asm.h
> index e6bda70b1d93..c8882d9b86c2 100644
> --- a/arch/powerpc/include/asm/kvm_book3s_asm.h
> +++ b/arch/powerpc/include/asm/kvm_book3s_asm.h
> @@ -14,9 +14,6 @@
>  #define XICS_MFRR		0xc
>  #define XICS_IPI		2	/* interrupt source # for IPIs */
>
> -/* LPIDs we support with this build -- runtime limit may be lower */
> -#define KVMPPC_NR_LPIDS			(1UL << 12)
> -
>  /* Maximum number of threads per physical core */
>  #define MAX_SMT_THREADS		8
>
> diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> index f983fb36cbf2..aafd2a74304c 100644
> --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
> +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> @@ -269,9 +269,6 @@ int kvmppc_mmu_hv_init(void)
>  		nr_lpids = 1UL << KVM_MAX_NESTED_GUESTS_SHIFT;
>  	}
>
> -	if (nr_lpids > KVMPPC_NR_LPIDS)
> -		nr_lpids = KVMPPC_NR_LPIDS;
> -
>  	if (!cpu_has_feature(CPU_FTR_ARCH_300)) {
>  		/* POWER7 has 10-bit LPIDs, POWER8 has 12-bit LPIDs */
>  		if (cpu_has_feature(CPU_FTR_ARCH_207S))

^ permalink raw reply

* [PATCH v2 0/4] KVM: PPC: KVM module exit fixes
From: Fabiano Rosas @ 2022-01-24 22:07 UTC (permalink / raw)
  To: kvm-ppc; +Cc: linuxppc-dev, npiggin, aik

I stumbled upon another issue with our module exit so I'm sending
another version to add a fix for it.

- patches 1 and 3 are already reviewed;

- patch 2 lacks a Reviewed-by. Nick asked about an issue Alexey might
  have encountered. I haven't heard of any issues with the module exit
  aside from the ones that this series fixes;

- patch 4 is new. It fixes an issue with module refcounting.

v1:
https://lore.kernel.org/r/20211223211931.3560887-1-farosas@linux.ibm.com

Fabiano Rosas (4):
  KVM: PPC: Book3S HV: Check return value of kvmppc_radix_init
  KVM: PPC: Book3S HV: Delay setting of kvm ops
  KVM: PPC: Book3S HV: Free allocated memory if module init fails
  KVM: PPC: Decrement module refcount if init_vm fails

 arch/powerpc/kvm/book3s_hv.c | 28 ++++++++++++++++++++--------
 arch/powerpc/kvm/powerpc.c   |  7 ++++++-
 2 files changed, 26 insertions(+), 9 deletions(-)

-- 
2.34.1


^ permalink raw reply

* Re: [PATCH 1/2] KVM: PPC: Book3S PR: Disable SCV when running AIL is disabled
From: Fabiano Rosas @ 2022-01-24 22:49 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20220124102417.3741427-2-npiggin@gmail.com>

Nicholas Piggin <npiggin@gmail.com> writes:

> PR KVM does not support running with AIL enabled, and SCV does is not
> supported with AIL disabled.
>
> Fix this by ensuring the SCV facility is disabled with FSCR while a
> CPU can be running with AIL=0. PowerNV host supports disabling AIL on a
> per-CPU basis, so SCV just needs to be disabled when a vCPU is run.
>
> The pSeries machine can only switch AIL on a system-wide basis, so it
> must disable SCV support at boot if the configuration can potentially
> run a PR KVM guest.
>
> SCV is not emulated for the PR guest at the moment, this just fixes the
> host crashes.
>
> Alternatives considered and rejected:
> - SCV support can not be disabled by PR KVM after boot, because it is
>   advertised to userspace with HWCAP.
> - AIL can not be disabled on a per-CPU basis. At least when running on
>   pseries it is a per-LPAR setting.
> - Support for real-mode SCV vectors will not be added because they are
>   at 0x17000 so making such a large fixed head space causes immediate
>   value limits to be exceeded, requiring a lot rework and more code.
> - Disabling SCV for any PR KVM possible kernel will cause a slowdown
>   when not using PR KVM.
> - A boot time option to disable SCV to use PR KVM is user-hostile.
> - System call instruction emulation for SCV facility unavailable
>   instructions is too complex and old emulation code was subtly broken
>   and removed.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  arch/powerpc/kernel/exceptions-64s.S |  4 ++++
>  arch/powerpc/kernel/setup_64.c       | 15 +++++++++++++++
>  arch/powerpc/kvm/book3s_pr.c         | 20 ++++++++++++++------
>  3 files changed, 33 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> index 55caeee37c08..b66dd6f775a4 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -809,6 +809,10 @@ __start_interrupts:
>   * - MSR_EE|MSR_RI is clear (no reentrant exceptions)
>   * - Standard kernel environment is set up (stack, paca, etc)
>   *
> + * KVM:
> + * These interrupts do not elevate HV 0->1, so HV is not involved. PR KVM
> + * ensures that FSCR[SCV] is disabled whenever it has to force AIL off.
> + *
>   * Call convention:
>   *
>   * syscall register convention is in Documentation/powerpc/syscall64-abi.rst
> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
> index be8577ac9397..ac52c69a3811 100644
> --- a/arch/powerpc/kernel/setup_64.c
> +++ b/arch/powerpc/kernel/setup_64.c
> @@ -197,6 +197,21 @@ static void __init configure_exceptions(void)
>
>  	/* Under a PAPR hypervisor, we need hypercalls */
>  	if (firmware_has_feature(FW_FEATURE_SET_MODE)) {
> +		/*
> +		 * PR KVM does not support AIL mode interrupts in the host, and
> +		 * SCV system call interrupt vectors are only implemented for
> +		 * AIL mode. Under pseries, AIL mode can only be enabled and
> +		 * disabled system-wide so when PR KVM is loaded, all CPUs in
> +		 * the host are set to AIL=0 mode. SCV can not be disabled
> +		 * dynamically because the feature is advertised to host
> +		 * userspace, so SCV support must not be enabled if PR KVM can
> +		 * possibly be run.
> +		 */
> +		if (IS_ENABLED(CONFIG_KVM_BOOK3S_PR_POSSIBLE) && !radix_enabled()) {
> +			init_task.thread.fscr &= ~FSCR_SCV;
> +			cur_cpu_spec->cpu_user_features2 &= ~PPC_FEATURE2_SCV;
> +		}
> +

"Under pseries, AIL mode can only be enabled and disabled system-wide so
 when PR KVM is loaded, all CPUs in the host are set to AIL=0 mode."

Loaded as in 'modprobe kvm_pr'? And host as in "nested host"
surely. Unless I completely misunderstood the patch (likely).

Is there a way to make this less unexpected to users? Maybe a few words
in the Kconfig entry for PR_POSSIBLE saying "if you enable this and run
a Hash MMU guest, you lose SCV"?

>  		/* Enable AIL if possible */
>  		if (!pseries_enable_reloc_on_exc()) {
>  			init_task.thread.fscr &= ~FSCR_SCV;
> diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
> index 34a801c3604a..4d1c84b94b77 100644
> --- a/arch/powerpc/kvm/book3s_pr.c
> +++ b/arch/powerpc/kvm/book3s_pr.c
> @@ -140,9 +140,12 @@ static void kvmppc_core_vcpu_load_pr(struct kvm_vcpu *vcpu, int cpu)
>  #endif
>
>  	/* Disable AIL if supported */
> -	if (cpu_has_feature(CPU_FTR_HVMODE) &&
> -	    cpu_has_feature(CPU_FTR_ARCH_207S))
> -		mtspr(SPRN_LPCR, mfspr(SPRN_LPCR) & ~LPCR_AIL);
> +	if (cpu_has_feature(CPU_FTR_HVMODE)) {
> +		if (cpu_has_feature(CPU_FTR_ARCH_207S))
> +			mtspr(SPRN_LPCR, mfspr(SPRN_LPCR) & ~LPCR_AIL);
> +		if (cpu_has_feature(CPU_FTR_ARCH_300) && (current->thread.fscr & FSCR_SCV))
> +			mtspr(SPRN_FSCR, mfspr(SPRN_FSCR) & ~FSCR_SCV);
> +	}
>
>  	vcpu->cpu = smp_processor_id();
>  #ifdef CONFIG_PPC_BOOK3S_32
> @@ -175,9 +178,12 @@ static void kvmppc_core_vcpu_put_pr(struct kvm_vcpu *vcpu)
>  	kvmppc_save_tm_pr(vcpu);
>
>  	/* Enable AIL if supported */
> -	if (cpu_has_feature(CPU_FTR_HVMODE) &&
> -	    cpu_has_feature(CPU_FTR_ARCH_207S))
> -		mtspr(SPRN_LPCR, mfspr(SPRN_LPCR) | LPCR_AIL_3);
> +	if (cpu_has_feature(CPU_FTR_HVMODE)) {
> +		if (cpu_has_feature(CPU_FTR_ARCH_207S))
> +			mtspr(SPRN_LPCR, mfspr(SPRN_LPCR) | LPCR_AIL_3);
> +		if (cpu_has_feature(CPU_FTR_ARCH_300) && (current->thread.fscr & FSCR_SCV))
> +			mtspr(SPRN_FSCR, mfspr(SPRN_FSCR) | FSCR_SCV);
> +	}
>
>  	vcpu->cpu = -1;
>  }
> @@ -1037,6 +1043,8 @@ static int kvmppc_handle_fac(struct kvm_vcpu *vcpu, ulong fac)
>
>  void kvmppc_set_fscr(struct kvm_vcpu *vcpu, u64 fscr)
>  {
> +	if (fscr & FSCR_SCV)
> +		fscr &= ~FSCR_SCV; /* SCV must not be enabled */
>  	if ((vcpu->arch.fscr & FSCR_TAR) && !(fscr & FSCR_TAR)) {
>  		/* TAR got dropped, drop it in shadow too */
>  		kvmppc_giveup_fac(vcpu, FSCR_TAR_LG);

^ permalink raw reply

* Re: [PATCH 2/2] KVM: PPC: Book3S PR: Disallow AIL != 0
From: Fabiano Rosas @ 2022-01-24 22:50 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20220124102417.3741427-3-npiggin@gmail.com>

Nicholas Piggin <npiggin@gmail.com> writes:

> KVM PR does not implement address translation modes on interrupt, so it
> must not allow H_SET_MODE to succeed.
>
> This is not compatible with QEMU behaviour. The solution might be to
> have a cap-ail for this, but now it's broken either way so fix it in
> KVM to start with.
>
> This allows PR Linux guests that are using the SCV facility to boot and
> run, because Linux disables the use of SCV if AIL can not be set to 3.
> This isn't a real fix because Linux or another OS could implement real
> mode SCV vectors and try to enable it. The right solution is for KVM to
> emulate scv interrupts from the facility unavailable interrupt.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>

>  arch/powerpc/kvm/book3s_pr_papr.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/arch/powerpc/kvm/book3s_pr_papr.c b/arch/powerpc/kvm/book3s_pr_papr.c
> index 1f10e7dfcdd0..dc4f51ac84bc 100644
> --- a/arch/powerpc/kvm/book3s_pr_papr.c
> +++ b/arch/powerpc/kvm/book3s_pr_papr.c
> @@ -281,6 +281,22 @@ static int kvmppc_h_pr_logical_ci_store(struct kvm_vcpu *vcpu)
>  	return EMULATE_DONE;
>  }
>
> +static int kvmppc_h_pr_set_mode(struct kvm_vcpu *vcpu)
> +{
> +	unsigned long mflags = kvmppc_get_gpr(vcpu, 4);
> +	unsigned long resource = kvmppc_get_gpr(vcpu, 5);
> +
> +	if (resource == H_SET_MODE_RESOURCE_ADDR_TRANS_MODE) {
> +		/* KVM PR does not provide AIL!=0 to guests */
> +		if (mflags == 0)
> +			kvmppc_set_gpr(vcpu, 3, H_SUCCESS);
> +		else
> +			kvmppc_set_gpr(vcpu, 3, H_UNSUPPORTED_FLAG_START - 63);
> +		return EMULATE_DONE;
> +	}
> +	return EMULATE_FAIL;
> +}
> +
>  #ifdef CONFIG_SPAPR_TCE_IOMMU
>  static int kvmppc_h_pr_put_tce(struct kvm_vcpu *vcpu)
>  {
> @@ -384,6 +400,8 @@ int kvmppc_h_pr(struct kvm_vcpu *vcpu, unsigned long cmd)
>  		return kvmppc_h_pr_logical_ci_load(vcpu);
>  	case H_LOGICAL_CI_STORE:
>  		return kvmppc_h_pr_logical_ci_store(vcpu);
> +	case H_SET_MODE:
> +		return kvmppc_h_pr_set_mode(vcpu);
>  	case H_XIRR:
>  	case H_CPPR:
>  	case H_EOI:
> @@ -421,6 +439,7 @@ int kvmppc_hcall_impl_pr(unsigned long cmd)
>  	case H_CEDE:
>  	case H_LOGICAL_CI_LOAD:
>  	case H_LOGICAL_CI_STORE:
> +	case H_SET_MODE:
>  #ifdef CONFIG_KVM_XICS
>  	case H_XIRR:
>  	case H_CPPR:
> @@ -447,6 +466,7 @@ static unsigned int default_hcall_list[] = {
>  	H_BULK_REMOVE,
>  	H_PUT_TCE,
>  	H_CEDE,
> +	H_SET_MODE,
>  #ifdef CONFIG_KVM_XICS
>  	H_XIRR,
>  	H_CPPR,

^ permalink raw reply

* Re: Build regressions/improvements in v5.17-rc1
From: Felix Kuehling @ 2022-01-25  1:23 UTC (permalink / raw)
  To: Randy Dunlap, Geert Uytterhoeven, Alex Deucher
  Cc: ALSA Development Mailing List, KVM list, Network Development,
	linux-um, LKML, amd-gfx list, open list:BROADCOM NVRAM DRIVER,
	Lakshmi Sowjanya D, sparclinux, linuxppc-dev, Tobin C. Harding
In-Reply-To: <aca104cf-5f5f-b696-754a-35e62dbe64c3@infradead.org>


Am 2022-01-24 um 14:11 schrieb Randy Dunlap:
> On 1/24/22 10:55, Geert Uytterhoeven wrote:
>> Hi Alex,
>>
>> On Mon, Jan 24, 2022 at 7:52 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>>> On Mon, Jan 24, 2022 at 5:25 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>>> On Sun, 23 Jan 2022, Geert Uytterhoeven wrote:
>>>>>   + /kisskb/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c: error: control reaches end of non-void function [-Werror=return-type]:  => 1560:1
>>> I don't really see what's going on here:
>>>
>>> #ifdef CONFIG_X86_64
>>> return cpu_data(first_cpu_of_numa_node).apicid;
>>> #else
>>> return first_cpu_of_numa_node;
>>> #endif
>> Ah, the actual failure causing this was not included:
>>
>> In file included from /kisskb/src/arch/x86/um/asm/processor.h:41:0,
>>                   from /kisskb/src/include/linux/mutex.h:19,
>>                   from /kisskb/src/include/linux/kernfs.h:11,
>>                   from /kisskb/src/include/linux/sysfs.h:16,
>>                   from /kisskb/src/include/linux/kobject.h:20,
>>                   from /kisskb/src/include/linux/pci.h:35,
>>                   from
>> /kisskb/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:25:
>> /kisskb/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c: In
>> function 'kfd_cpumask_to_apic_id':
>> /kisskb/src/arch/um/include/asm/processor-generic.h:103:18: error:
>> called object is not a function or function pointer
>>   #define cpu_data (&boot_cpu_data)
>>                    ^
>> /kisskb/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1556:9:
>> note: in expansion of macro 'cpu_data'
>>    return cpu_data(first_cpu_of_numa_node).apicid;
>>           ^
>> /kisskb/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1560:1:
>> error: control reaches end of non-void function [-Werror=return-type]
>>   }
>>   ^
> ah yes, UML.
> I have a bunch of UML fixes that I have been hesitant to post.
>
> This is one of them.
> What do people think about this?

Does it make sense to configure a UML kernel with a real device driver 
in the first place? Or should we just prevent enabling amdgpu for UML 
with a Kconfig dependency?

Regards,
   Felix


>
> thanks.
>
> ---
> From: Randy Dunlap <rdunlap@infradead.org>
>
>
> ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1556:9: note: in expansion of macro ‘cpu_data’
>    return cpu_data(first_cpu_of_numa_node).apicid;
>           ^~~~~~~~
> ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1560:1: error: control reaches end of non-void function [-Werror=return-type]
>
> ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c: In function ‘kfd_fill_iolink_info_for_cpu’:
> ../arch/um/include/asm/processor-generic.h:103:19: error: called object is not a function or function pointer
>   #define cpu_data (&boot_cpu_data)
>                    ~^~~~~~~~~~~~~~~
> ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c:1688:27: note: in expansion of macro ‘cpu_data’
>    struct cpuinfo_x86 *c = &cpu_data(0);
>                             ^~~~~~~~
> ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c:1691:7: error: dereferencing pointer to incomplete type ‘struct cpuinfo_x86’
>    if (c->x86_vendor == X86_VENDOR_AMD)
>         ^~
> ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c:1691:23: error: ‘X86_VENDOR_AMD’ undeclared (first use in this function); did you mean ‘X86_VENDOR_ANY’?
>    if (c->x86_vendor == X86_VENDOR_AMD)
>                         ^~~~~~~~~~~~~~
>                         X86_VENDOR_ANY
>
> ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c: In function ‘kfd_create_vcrat_image_cpu’:
> ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c:1742:11: warning: unused variable ‘entries’ [-Wunused-variable]
>    uint32_t entries = 0;
>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_crat.c     |    6 +++---
>   drivers/gpu/drm/amd/amdkfd/kfd_topology.c |    2 +-
>   2 files changed, 4 insertions(+), 4 deletions(-)
>
> --- linux-next-20220107.orig/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> +++ linux-next-20220107/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> @@ -1552,7 +1552,7 @@ static int kfd_cpumask_to_apic_id(const
>   	first_cpu_of_numa_node = cpumask_first(cpumask);
>   	if (first_cpu_of_numa_node >= nr_cpu_ids)
>   		return -1;
> -#ifdef CONFIG_X86_64
> +#if defined(CONFIG_X86_64) && !defined(CONFIG_UML)
>   	return cpu_data(first_cpu_of_numa_node).apicid;
>   #else
>   	return first_cpu_of_numa_node;
> --- linux-next-20220107.orig/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> +++ linux-next-20220107/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> @@ -1679,7 +1679,7 @@ static int kfd_fill_mem_info_for_cpu(int
>   	return 0;
>   }
>   
> -#ifdef CONFIG_X86_64
> +#if defined(CONFIG_X86_64) && !defined(CONFIG_UML)
>   static int kfd_fill_iolink_info_for_cpu(int numa_node_id, int *avail_size,
>   				uint32_t *num_entries,
>   				struct crat_subtype_iolink *sub_type_hdr)
> @@ -1738,7 +1738,7 @@ static int kfd_create_vcrat_image_cpu(vo
>   	struct crat_subtype_generic *sub_type_hdr;
>   	int avail_size = *size;
>   	int numa_node_id;
> -#ifdef CONFIG_X86_64
> +#if defined(CONFIG_X86_64) && !defined(CONFIG_UML)
>   	uint32_t entries = 0;
>   #endif
>   	int ret = 0;
> @@ -1803,7 +1803,7 @@ static int kfd_create_vcrat_image_cpu(vo
>   			sub_type_hdr->length);
>   
>   		/* Fill in Subtype: IO Link */
> -#ifdef CONFIG_X86_64
> +#if defined(CONFIG_X86_64) && !defined(CONFIG_UML)
>   		ret = kfd_fill_iolink_info_for_cpu(numa_node_id, &avail_size,
>   				&entries,
>   				(struct crat_subtype_iolink *)sub_type_hdr);
>
>

^ permalink raw reply

* Re: Build regressions/improvements in v5.17-rc1
From: Randy Dunlap @ 2022-01-25  1:29 UTC (permalink / raw)
  To: Felix Kuehling, Geert Uytterhoeven, Alex Deucher
  Cc: ALSA Development Mailing List, KVM list, Network Development,
	linux-um, LKML, amd-gfx list, open list:BROADCOM NVRAM DRIVER,
	Lakshmi Sowjanya D, sparclinux, linuxppc-dev, Tobin C. Harding
In-Reply-To: <500a3aa0-f51e-92ac-82d6-7ffc7f603e2c@amd.com>



On 1/24/22 17:23, Felix Kuehling wrote:
> 
> Am 2022-01-24 um 14:11 schrieb Randy Dunlap:
>> On 1/24/22 10:55, Geert Uytterhoeven wrote:
>>> Hi Alex,
>>>
>>> On Mon, Jan 24, 2022 at 7:52 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>>>> On Mon, Jan 24, 2022 at 5:25 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>>>> On Sun, 23 Jan 2022, Geert Uytterhoeven wrote:
>>>>>>   + /kisskb/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c: error: control reaches end of non-void function [-Werror=return-type]:  => 1560:1
>>>> I don't really see what's going on here:
>>>>
>>>> #ifdef CONFIG_X86_64
>>>> return cpu_data(first_cpu_of_numa_node).apicid;
>>>> #else
>>>> return first_cpu_of_numa_node;
>>>> #endif
>>> Ah, the actual failure causing this was not included:
>>>
>>> In file included from /kisskb/src/arch/x86/um/asm/processor.h:41:0,
>>>                   from /kisskb/src/include/linux/mutex.h:19,
>>>                   from /kisskb/src/include/linux/kernfs.h:11,
>>>                   from /kisskb/src/include/linux/sysfs.h:16,
>>>                   from /kisskb/src/include/linux/kobject.h:20,
>>>                   from /kisskb/src/include/linux/pci.h:35,
>>>                   from
>>> /kisskb/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:25:
>>> /kisskb/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c: In
>>> function 'kfd_cpumask_to_apic_id':
>>> /kisskb/src/arch/um/include/asm/processor-generic.h:103:18: error:
>>> called object is not a function or function pointer
>>>   #define cpu_data (&boot_cpu_data)
>>>                    ^
>>> /kisskb/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1556:9:
>>> note: in expansion of macro 'cpu_data'
>>>    return cpu_data(first_cpu_of_numa_node).apicid;
>>>           ^
>>> /kisskb/src/drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1560:1:
>>> error: control reaches end of non-void function [-Werror=return-type]
>>>   }
>>>   ^
>> ah yes, UML.
>> I have a bunch of UML fixes that I have been hesitant to post.
>>
>> This is one of them.
>> What do people think about this?
> 
> Does it make sense to configure a UML kernel with a real device driver in the first place? Or should we just prevent enabling amdgpu for UML with a Kconfig dependency?
> 

Hi,

Your option IMO. I have seen both opinions given.
I also meant to reply that someone could just add
	depends on !UML
for this device, like you are suggesting.

I'm fine with it either way.

thanks.

> 
>>
>> thanks.
>>
>> ---
>> From: Randy Dunlap <rdunlap@infradead.org>
>>
>>
>> ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1556:9: note: in expansion of macro ‘cpu_data’
>>    return cpu_data(first_cpu_of_numa_node).apicid;
>>           ^~~~~~~~
>> ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1560:1: error: control reaches end of non-void function [-Werror=return-type]
>>
>> ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c: In function ‘kfd_fill_iolink_info_for_cpu’:
>> ../arch/um/include/asm/processor-generic.h:103:19: error: called object is not a function or function pointer
>>   #define cpu_data (&boot_cpu_data)
>>                    ~^~~~~~~~~~~~~~~
>> ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c:1688:27: note: in expansion of macro ‘cpu_data’
>>    struct cpuinfo_x86 *c = &cpu_data(0);
>>                             ^~~~~~~~
>> ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c:1691:7: error: dereferencing pointer to incomplete type ‘struct cpuinfo_x86’
>>    if (c->x86_vendor == X86_VENDOR_AMD)
>>         ^~
>> ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c:1691:23: error: ‘X86_VENDOR_AMD’ undeclared (first use in this function); did you mean ‘X86_VENDOR_ANY’?
>>    if (c->x86_vendor == X86_VENDOR_AMD)
>>                         ^~~~~~~~~~~~~~
>>                         X86_VENDOR_ANY
>>
>> ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c: In function ‘kfd_create_vcrat_image_cpu’:
>> ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c:1742:11: warning: unused variable ‘entries’ [-Wunused-variable]
>>    uint32_t entries = 0;
>>
>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>> ---
>>   drivers/gpu/drm/amd/amdkfd/kfd_crat.c     |    6 +++---
>>   drivers/gpu/drm/amd/amdkfd/kfd_topology.c |    2 +-
>>   2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> --- linux-next-20220107.orig/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
>> +++ linux-next-20220107/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
>> @@ -1552,7 +1552,7 @@ static int kfd_cpumask_to_apic_id(const
>>       first_cpu_of_numa_node = cpumask_first(cpumask);
>>       if (first_cpu_of_numa_node >= nr_cpu_ids)
>>           return -1;
>> -#ifdef CONFIG_X86_64
>> +#if defined(CONFIG_X86_64) && !defined(CONFIG_UML)
>>       return cpu_data(first_cpu_of_numa_node).apicid;
>>   #else
>>       return first_cpu_of_numa_node;
>> --- linux-next-20220107.orig/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
>> +++ linux-next-20220107/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
>> @@ -1679,7 +1679,7 @@ static int kfd_fill_mem_info_for_cpu(int
>>       return 0;
>>   }
>>   -#ifdef CONFIG_X86_64
>> +#if defined(CONFIG_X86_64) && !defined(CONFIG_UML)
>>   static int kfd_fill_iolink_info_for_cpu(int numa_node_id, int *avail_size,
>>                   uint32_t *num_entries,
>>                   struct crat_subtype_iolink *sub_type_hdr)
>> @@ -1738,7 +1738,7 @@ static int kfd_create_vcrat_image_cpu(vo
>>       struct crat_subtype_generic *sub_type_hdr;
>>       int avail_size = *size;
>>       int numa_node_id;
>> -#ifdef CONFIG_X86_64
>> +#if defined(CONFIG_X86_64) && !defined(CONFIG_UML)
>>       uint32_t entries = 0;
>>   #endif
>>       int ret = 0;
>> @@ -1803,7 +1803,7 @@ static int kfd_create_vcrat_image_cpu(vo
>>               sub_type_hdr->length);
>>             /* Fill in Subtype: IO Link */
>> -#ifdef CONFIG_X86_64
>> +#if defined(CONFIG_X86_64) && !defined(CONFIG_UML)
>>           ret = kfd_fill_iolink_info_for_cpu(numa_node_id, &avail_size,
>>                   &entries,
>>                   (struct crat_subtype_iolink *)sub_type_hdr);
>>
>>

-- 
~Randy

^ permalink raw reply

* Re: [PATCH 0/2] powerpc: Disable syscall emulation and stepping
From: Nicholas Piggin @ 2022-01-25  3:04 UTC (permalink / raw)
  To: Christophe Leroy, linuxppc-dev@lists.ozlabs.org; +Cc: Naveen N. Rao
In-Reply-To: <d3ab1142-5f62-6cbc-067c-6a34f4f28ef2@csgroup.eu>

+Naveen (sorry missed cc'ing you at first)

Excerpts from Christophe Leroy's message of January 24, 2022 4:39 pm:
> 
> 
> Le 24/01/2022 à 06:57, Nicholas Piggin a écrit :
>> As discussed previously
>> 
>> https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-January/238946.html
>> 
>> I'm wondering whether PPC32 should be returning -1 for syscall
>> instructions too here? That could be done in another patch anyway.
>> 
> 
> The 'Programming Environments Manual for 32-Bit Implementations of the 
> PowerPC™ Architecture' says:
> 
> The following are not traced:
> • rfi instruction
> • sc and trap instructions that trap
> • Other instructions that cause interrupts (other than trace interrupts)
> • The first instruction of any interrupt handler
> • Instructions that are emulated by software
> 
> 
> So I think PPC32 should return -1 as well.

I agree.

What about the trap instructions? analyse_instr returns 0 for them
which falls through to return 0 for emulate_step, should they
return -1 as well or am I missing something?

Thanks,
Nick

^ permalink raw reply

* Re: [PATCH v4 3/5] KVM: PPC: mmio: Reject instructions that access more than mmio.data size
From: Nicholas Piggin @ 2022-01-25  3:06 UTC (permalink / raw)
  To: Fabiano Rosas, kvm-ppc; +Cc: aik, linuxppc-dev
In-Reply-To: <20220121222626.972495-4-farosas@linux.ibm.com>

Excerpts from Fabiano Rosas's message of January 22, 2022 8:26 am:
> The MMIO interface between the kernel and userspace uses a structure
> that supports a maximum of 8-bytes of data. Instructions that access
> more than that need to be emulated in parts.
> 
> We currently don't have generic support for splitting the emulation in
> parts and each set of instructions needs to be explicitly included.
> 
> There's already an error message being printed when a load or store
> exceeds the mmio.data buffer but we don't fail the emulation until
> later at kvmppc_complete_mmio_load and even then we allow userspace to
> make a partial copy of the data, which ends up overwriting some fields
> of the mmio structure.
> 
> This patch makes the emulation fail earlier at kvmppc_handle_load|store,
> which will send a Program interrupt to the guest. This is better than
> allowing the guest to proceed with partial data.
> 
> Note that this was caught in a somewhat artificial scenario using
> quadword instructions (lq/stq), there's no account of an actual guest
> in the wild running instructions that are not properly emulated.
> 
> (While here, remove the "bad MMIO" messages. The caller already has an
> error message.)
> 
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>

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

> ---
>  arch/powerpc/kvm/powerpc.c | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index c2bd29e90314..27fb2b70f631 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -1114,10 +1114,8 @@ static void kvmppc_complete_mmio_load(struct kvm_vcpu *vcpu)
>  	struct kvm_run *run = vcpu->run;
>  	u64 gpr;
>  
> -	if (run->mmio.len > sizeof(gpr)) {
> -		printk(KERN_ERR "bad MMIO length: %d\n", run->mmio.len);
> +	if (run->mmio.len > sizeof(gpr))
>  		return;
> -	}
>  
>  	if (!vcpu->arch.mmio_host_swabbed) {
>  		switch (run->mmio.len) {
> @@ -1236,10 +1234,8 @@ static int __kvmppc_handle_load(struct kvm_vcpu *vcpu,
>  		host_swabbed = !is_default_endian;
>  	}
>  
> -	if (bytes > sizeof(run->mmio.data)) {
> -		printk(KERN_ERR "%s: bad MMIO length: %d\n", __func__,
> -		       run->mmio.len);
> -	}
> +	if (bytes > sizeof(run->mmio.data))
> +		return EMULATE_FAIL;
>  
>  	run->mmio.phys_addr = vcpu->arch.paddr_accessed;
>  	run->mmio.len = bytes;
> @@ -1325,10 +1321,8 @@ int kvmppc_handle_store(struct kvm_vcpu *vcpu,
>  		host_swabbed = !is_default_endian;
>  	}
>  
> -	if (bytes > sizeof(run->mmio.data)) {
> -		printk(KERN_ERR "%s: bad MMIO length: %d\n", __func__,
> -		       run->mmio.len);
> -	}
> +	if (bytes > sizeof(run->mmio.data))
> +		return EMULATE_FAIL;
>  
>  	run->mmio.phys_addr = vcpu->arch.paddr_accessed;
>  	run->mmio.len = bytes;
> -- 
> 2.34.1
> 
> 

^ permalink raw reply

* Re: [powerpc] ftrace warning kernel/trace/ftrace.c:2068 with code-patching selftests
From: Yinan Liu @ 2022-01-25  3:20 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linuxppc-dev, Sachin Sant
In-Reply-To: <20220124114548.30241947@gandalf.local.home>

> Yeah, I think it's time to opt in, instead of opting out.
> 
> Something like this:
> 
> -- Steve
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index c2724d986fa0..5256ebe57451 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -82,6 +82,7 @@ config ARM
>   	select HAVE_EBPF_JIT if !CPU_ENDIAN_BE32
>   	select HAVE_CONTEXT_TRACKING
>   	select HAVE_C_RECORDMCOUNT
> +	select HAVE_BUILDTIME_MCOUNT_SORT
>   	select HAVE_DEBUG_KMEMLEAK if !XIP_KERNEL
>   	select HAVE_DMA_CONTIGUOUS if MMU
>   	select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && !CPU_ENDIAN_BE32 && MMU
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index c4207cf9bb17..7996548b2b27 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -166,6 +166,7 @@ config ARM64
>   	select HAVE_ASM_MODVERSIONS
>   	select HAVE_EBPF_JIT
>   	select HAVE_C_RECORDMCOUNT
> +	select HAVE_BUILDTIME_MCOUNT_SORT
>   	select HAVE_CMPXCHG_DOUBLE
>   	select HAVE_CMPXCHG_LOCAL
>   	select HAVE_CONTEXT_TRACKING
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 7399327d1eff..46080dea5dba 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -186,6 +186,7 @@ config X86
>   	select HAVE_CONTEXT_TRACKING_OFFSTACK	if HAVE_CONTEXT_TRACKING
>   	select HAVE_C_RECORDMCOUNT
>   	select HAVE_OBJTOOL_MCOUNT		if STACK_VALIDATION
> +	select HAVE_BUILDTIME_MCOUNT_SORT
>   	select HAVE_DEBUG_KMEMLEAK
>   	select HAVE_DMA_CONTIGUOUS
>   	select HAVE_DYNAMIC_FTRACE
> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> index 752ed89a293b..7e5b92090faa 100644
> --- a/kernel/trace/Kconfig
> +++ b/kernel/trace/Kconfig
> @@ -70,10 +70,16 @@ config HAVE_C_RECORDMCOUNT
>   	help
>   	  C version of recordmcount available?
>   
> +config HAVE_BUILDTIME_MCOUNT_SORT
> +       bool
> +       help
> +         An architecture selects this if it sorts the mcount_loc section
> +	 at build time.
> +
>   config BUILDTIME_MCOUNT_SORT
>          bool
>          default y
> -       depends on BUILDTIME_TABLE_SORT && !S390
> +       depends on HAVE_BUILDTIME_MCOUNT_SORT
>          help
>            Sort the mcount_loc section at build time.

LGTM. This will no longer destroy ftrace on other architectures.
Those arches that we are not sure about can test and enable this 
function by themselves.


Best regards
--yinan

^ permalink raw reply

* Re: [PATCH v4 4/5] KVM: PPC: mmio: Return to guest after emulation failure
From: Nicholas Piggin @ 2022-01-25  3:26 UTC (permalink / raw)
  To: Fabiano Rosas, kvm-ppc; +Cc: aik, linuxppc-dev
In-Reply-To: <20220121222626.972495-5-farosas@linux.ibm.com>

Excerpts from Fabiano Rosas's message of January 22, 2022 8:26 am:
> If MMIO emulation fails we don't want to crash the whole guest by
> returning to userspace.
> 
> The original commit bbf45ba57eae ("KVM: ppc: PowerPC 440 KVM
> implementation") added a todo:
> 
>   /* XXX Deliver Program interrupt to guest. */
> 
> and later the commit d69614a295ae ("KVM: PPC: Separate loadstore
> emulation from priv emulation") added the Program interrupt injection
> but in another file, so I'm assuming it was missed that this block
> needed to be altered.
> 
> Also change the message to a ratelimited one since we're letting the
> guest run and it could flood the host logs.
> 
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>

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

One small thing...

> ---
>  arch/powerpc/kvm/powerpc.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 27fb2b70f631..214602c58f13 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -307,9 +307,9 @@ int kvmppc_emulate_mmio(struct kvm_vcpu *vcpu)
>  		u32 last_inst;
>  
>  		kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst);
> -		/* XXX Deliver Program interrupt to guest. */
> -		pr_emerg("%s: emulation failed (%08x)\n", __func__, last_inst);
> -		r = RESUME_HOST;
> +		pr_info_ratelimited("KVM: guest access to device memory using unsupported instruction (PID: %d opcode: %#08x)\n",
> +				    current->pid, last_inst);

Minor thing but KVM now has some particular printing helpers so I wonder 
if we should start moving to them in general with our messages.

vcpu_debug_ratelimited() maybe?

Thanks,
Nick

^ 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