* Re: [PATCH v3] powerpc/uprobes: Validation for prefixed instruction
From: Naveen N. Rao @ 2021-03-03 8:38 UTC (permalink / raw)
To: Ravi Bangoria
Cc: oleg, rostedt, linux-kernel, paulus, sandipan, jniethe5,
naveen.n.rao, linuxppc-dev
In-Reply-To: <20210304050529.59391-1-ravi.bangoria@linux.ibm.com>
On 2021/03/04 10:35AM, Ravi Bangoria wrote:
> As per ISA 3.1, prefixed instruction should not cross 64-byte
> boundary. So don't allow Uprobe on such prefixed instruction.
>
> There are two ways probed instruction is changed in mapped pages.
> First, when Uprobe is activated, it searches for all the relevant
> pages and replace instruction in them. In this case, if that probe
> is on the 64-byte unaligned prefixed instruction, error out
> directly. Second, when Uprobe is already active and user maps a
> relevant page via mmap(), instruction is replaced via mmap() code
> path. But because Uprobe is invalid, entire mmap() operation can
> not be stopped. In this case just print an error and continue.
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> ---
> v2: https://lore.kernel.org/r/20210204104703.273429-1-ravi.bangoria@linux.ibm.com
> v2->v3:
> - Drop restriction for Uprobe on suffix of prefixed instruction.
> It needs lot of code change including generic code but what
> we get in return is not worth it.
>
> arch/powerpc/kernel/uprobes.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
> index e8a63713e655..c400971ebe70 100644
> --- a/arch/powerpc/kernel/uprobes.c
> +++ b/arch/powerpc/kernel/uprobes.c
> @@ -41,6 +41,14 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe,
> if (addr & 0x03)
> return -EINVAL;
>
> + if (!IS_ENABLED(CONFIG_PPC64) || !cpu_has_feature(CPU_FTR_ARCH_31))
> + return 0;
Sorry, I missed this last time, but I think we can drop the above
checks. ppc_inst_prefixed() already factors in the dependency on
CONFIG_PPC64, and I don't think we need to confirm if we're running on a
ISA V3.1 for the below check.
With that:
Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> +
> + if (ppc_inst_prefixed(auprobe->insn) && (addr & 0x3F) == 0x3C) {
> + pr_info_ratelimited("Cannot register a uprobe on 64 byte unaligned prefixed instruction\n");
> + return -EINVAL;
> + }
> +
- Naveen
^ permalink raw reply
* Re: [PATCH v3 1/2] powerpc/perf: Use PVR rather than oprofile field to determine CPU version
From: Athira Rajeev @ 2021-03-04 4:22 UTC (permalink / raw)
To: Christophe Leroy
Cc: Desnes A. Nunes do Rosario, Madhavan Srinivasan, linux-kernel,
Paul Mackerras, Viresh Kumar, Rashmica Gupta, linuxppc-dev
In-Reply-To: <50ad16925a66ac53890286ceafbf84f6fc324baa.1614600516.git.christophe.leroy@csgroup.eu>
[-- Attachment #1: Type: text/plain, Size: 9567 bytes --]
> On 01-Mar-2021, at 5:39 PM, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>
> From: Rashmica Gupta <rashmica.g@gmail.com>
>
> Currently the perf CPU backend drivers detect what CPU they're on using
> cur_cpu_spec->oprofile_cpu_type.
>
> Although that works, it's a bit crufty to be using oprofile related fields,
> especially seeing as oprofile is more or less unused these days.
>
> It also means perf is reliant on the fragile logic in setup_cpu_spec()
> which detects when we're using a logical PVR and copies back the PMU
> related fields from the raw CPU entry. So lets check the PVR directly.
>
> Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>
> Reviewed-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> [chleroy: Added power10 and fixed checkpatch issues]
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Reviewed-and-tested-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com <mailto:atrajeev@linux.vnet.ibm.com>>
Thanks
Athira
> ---
> arch/powerpc/perf/e500-pmu.c | 9 +++++----
> arch/powerpc/perf/e6500-pmu.c | 5 +++--
> arch/powerpc/perf/hv-24x7.c | 6 +++---
> arch/powerpc/perf/mpc7450-pmu.c | 5 +++--
> arch/powerpc/perf/power10-pmu.c | 6 ++----
> arch/powerpc/perf/power5+-pmu.c | 6 +++---
> arch/powerpc/perf/power5-pmu.c | 5 +++--
> arch/powerpc/perf/power6-pmu.c | 5 +++--
> arch/powerpc/perf/power7-pmu.c | 7 ++++---
> arch/powerpc/perf/power8-pmu.c | 5 +++--
> arch/powerpc/perf/power9-pmu.c | 4 +---
> arch/powerpc/perf/ppc970-pmu.c | 7 ++++---
> 12 files changed, 37 insertions(+), 33 deletions(-)
>
> diff --git a/arch/powerpc/perf/e500-pmu.c b/arch/powerpc/perf/e500-pmu.c
> index a59c33bed32a..e3e1a68eb1d5 100644
> --- a/arch/powerpc/perf/e500-pmu.c
> +++ b/arch/powerpc/perf/e500-pmu.c
> @@ -118,12 +118,13 @@ static struct fsl_emb_pmu e500_pmu = {
>
> static int init_e500_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type)
> - return -ENODEV;
> + unsigned int pvr = mfspr(SPRN_PVR);
>
> - if (!strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/e500mc"))
> + /* ec500mc */
> + if (PVR_VER(pvr) == PVR_VER_E500MC || PVR_VER(pvr) == PVR_VER_E5500)
> num_events = 256;
> - else if (strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/e500"))
> + /* e500 */
> + else if (PVR_VER(pvr) != PVR_VER_E500V1 && PVR_VER(pvr) != PVR_VER_E500V2)
> return -ENODEV;
>
> return register_fsl_emb_pmu(&e500_pmu);
> diff --git a/arch/powerpc/perf/e6500-pmu.c b/arch/powerpc/perf/e6500-pmu.c
> index 44ad65da82ed..bd779a2338f8 100644
> --- a/arch/powerpc/perf/e6500-pmu.c
> +++ b/arch/powerpc/perf/e6500-pmu.c
> @@ -107,8 +107,9 @@ static struct fsl_emb_pmu e6500_pmu = {
>
> static int init_e6500_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/e6500"))
> + unsigned int pvr = mfspr(SPRN_PVR);
> +
> + if (PVR_VER(pvr) != PVR_VER_E6500)
> return -ENODEV;
>
> return register_fsl_emb_pmu(&e6500_pmu);
> diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
> index e5eb33255066..f3f2472fa1c6 100644
> --- a/arch/powerpc/perf/hv-24x7.c
> +++ b/arch/powerpc/perf/hv-24x7.c
> @@ -1718,16 +1718,16 @@ static int hv_24x7_init(void)
> {
> int r;
> unsigned long hret;
> + unsigned int pvr = mfspr(SPRN_PVR);
> struct hv_perf_caps caps;
>
> if (!firmware_has_feature(FW_FEATURE_LPAR)) {
> pr_debug("not a virtualized system, not enabling\n");
> return -ENODEV;
> - } else if (!cur_cpu_spec->oprofile_cpu_type)
> - return -ENODEV;
> + }
>
> /* POWER8 only supports v1, while POWER9 only supports v2. */
> - if (!strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power8"))
> + if (PVR_VER(pvr) == PVR_POWER8)
> interface_version = 1;
> else {
> interface_version = 2;
> diff --git a/arch/powerpc/perf/mpc7450-pmu.c b/arch/powerpc/perf/mpc7450-pmu.c
> index e39b15b79a83..552d51a925d3 100644
> --- a/arch/powerpc/perf/mpc7450-pmu.c
> +++ b/arch/powerpc/perf/mpc7450-pmu.c
> @@ -417,8 +417,9 @@ struct power_pmu mpc7450_pmu = {
>
> static int __init init_mpc7450_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/7450"))
> + unsigned int pvr = mfspr(SPRN_PVR);
> +
> + if (PVR_VER(pvr) != PVR_7450)
> return -ENODEV;
>
> return register_power_pmu(&mpc7450_pmu);
> diff --git a/arch/powerpc/perf/power10-pmu.c b/arch/powerpc/perf/power10-pmu.c
> index a901c1348cad..d1395844a329 100644
> --- a/arch/powerpc/perf/power10-pmu.c
> +++ b/arch/powerpc/perf/power10-pmu.c
> @@ -566,12 +566,10 @@ int init_power10_pmu(void)
> unsigned int pvr;
> int rc;
>
> - /* Comes from cpu_specs[] */
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power10"))
> + pvr = mfspr(SPRN_PVR);
> + if (PVR_VER(pvr) != PVR_POWER10)
> return -ENODEV;
>
> - pvr = mfspr(SPRN_PVR);
> /* Add the ppmu flag for power10 DD1 */
> if ((PVR_CFG(pvr) == 1))
> power10_pmu.flags |= PPMU_P10_DD1;
> diff --git a/arch/powerpc/perf/power5+-pmu.c b/arch/powerpc/perf/power5+-pmu.c
> index 18732267993a..a79eae40ef6d 100644
> --- a/arch/powerpc/perf/power5+-pmu.c
> +++ b/arch/powerpc/perf/power5+-pmu.c
> @@ -679,9 +679,9 @@ static struct power_pmu power5p_pmu = {
>
> int init_power5p_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - (strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power5+")
> - && strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power5++")))
> + unsigned int pvr = mfspr(SPRN_PVR);
> +
> + if (PVR_VER(pvr) != PVR_POWER5p)
> return -ENODEV;
>
> return register_power_pmu(&power5p_pmu);
> diff --git a/arch/powerpc/perf/power5-pmu.c b/arch/powerpc/perf/power5-pmu.c
> index cb611c1e7abe..35a9d7f3b4b9 100644
> --- a/arch/powerpc/perf/power5-pmu.c
> +++ b/arch/powerpc/perf/power5-pmu.c
> @@ -620,8 +620,9 @@ static struct power_pmu power5_pmu = {
>
> int init_power5_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power5"))
> + unsigned int pvr = mfspr(SPRN_PVR);
> +
> + if (PVR_VER(pvr) != PVR_POWER5)
> return -ENODEV;
>
> return register_power_pmu(&power5_pmu);
> diff --git a/arch/powerpc/perf/power6-pmu.c b/arch/powerpc/perf/power6-pmu.c
> index 69ef38216418..8aa220c712a7 100644
> --- a/arch/powerpc/perf/power6-pmu.c
> +++ b/arch/powerpc/perf/power6-pmu.c
> @@ -541,8 +541,9 @@ static struct power_pmu power6_pmu = {
>
> int init_power6_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power6"))
> + unsigned int pvr = mfspr(SPRN_PVR);
> +
> + if (PVR_VER(pvr) != PVR_POWER6)
> return -ENODEV;
>
> return register_power_pmu(&power6_pmu);
> diff --git a/arch/powerpc/perf/power7-pmu.c b/arch/powerpc/perf/power7-pmu.c
> index 894c17f9a762..ca7373143b02 100644
> --- a/arch/powerpc/perf/power7-pmu.c
> +++ b/arch/powerpc/perf/power7-pmu.c
> @@ -447,11 +447,12 @@ static struct power_pmu power7_pmu = {
>
> int init_power7_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power7"))
> + unsigned int pvr = mfspr(SPRN_PVR);
> +
> + if (PVR_VER(pvr) != PVR_POWER7 && PVR_VER(pvr) != PVR_POWER7p)
> return -ENODEV;
>
> - if (pvr_version_is(PVR_POWER7p))
> + if (PVR_VER(pvr) == PVR_POWER7p)
> power7_pmu.flags |= PPMU_SIAR_VALID;
>
> return register_power_pmu(&power7_pmu);
> diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
> index 5282e8415ddf..5a396ba8bf58 100644
> --- a/arch/powerpc/perf/power8-pmu.c
> +++ b/arch/powerpc/perf/power8-pmu.c
> @@ -381,9 +381,10 @@ static struct power_pmu power8_pmu = {
> int init_power8_pmu(void)
> {
> int rc;
> + unsigned int pvr = mfspr(SPRN_PVR);
>
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power8"))
> + if (PVR_VER(pvr) != PVR_POWER8E && PVR_VER(pvr) != PVR_POWER8NVL &&
> + PVR_VER(pvr) != PVR_POWER8)
> return -ENODEV;
>
> rc = register_power_pmu(&power8_pmu);
> diff --git a/arch/powerpc/perf/power9-pmu.c b/arch/powerpc/perf/power9-pmu.c
> index 2a57e93a79dc..28ba1e98f93d 100644
> --- a/arch/powerpc/perf/power9-pmu.c
> +++ b/arch/powerpc/perf/power9-pmu.c
> @@ -444,9 +444,7 @@ int init_power9_pmu(void)
> int rc = 0;
> unsigned int pvr = mfspr(SPRN_PVR);
>
> - /* Comes from cpu_specs[] */
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power9"))
> + if (PVR_VER(pvr) != PVR_POWER9)
> return -ENODEV;
>
> /* Blacklist events */
> diff --git a/arch/powerpc/perf/ppc970-pmu.c b/arch/powerpc/perf/ppc970-pmu.c
> index 1f8263785286..39a0a4d7841c 100644
> --- a/arch/powerpc/perf/ppc970-pmu.c
> +++ b/arch/powerpc/perf/ppc970-pmu.c
> @@ -491,9 +491,10 @@ static struct power_pmu ppc970_pmu = {
>
> int init_ppc970_pmu(void)
> {
> - if (!cur_cpu_spec->oprofile_cpu_type ||
> - (strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/970")
> - && strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/970MP")))
> + unsigned int pvr = mfspr(SPRN_PVR);
> +
> + if (PVR_VER(pvr) != PVR_970 && PVR_VER(pvr) != PVR_970MP &&
> + PVR_VER(pvr) != PVR_970FX && PVR_VER(pvr) != PVR_970GX)
> return -ENODEV;
>
> return register_power_pmu(&ppc970_pmu);
> --
> 2.25.0
>
[-- Attachment #2: Type: text/html, Size: 21164 bytes --]
^ permalink raw reply
* [PATCH] powerpc/perf: Fix the threshold event selection for memory events in power10
From: Athira Rajeev @ 2021-03-04 6:40 UTC (permalink / raw)
To: mpe; +Cc: maddy, linuxppc-dev
Memory events (mem-loads and mem-stores) currently use the threshold
event selection as issue to finish. Power10 supports issue to complete
as part of thresholding which is more appropriate for mem-loads and
mem-stores. Hence fix the event code for memory events to use issue
to complete.
Fixes: a64e697cef23 ("powerpc/perf: power10 Performance Monitoring support")
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
arch/powerpc/perf/power10-events-list.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/perf/power10-events-list.h b/arch/powerpc/perf/power10-events-list.h
index e45dafe818ed..93be7197d250 100644
--- a/arch/powerpc/perf/power10-events-list.h
+++ b/arch/powerpc/perf/power10-events-list.h
@@ -75,5 +75,5 @@
* thresh end (TE)
*/
-EVENT(MEM_LOADS, 0x34340401e0);
-EVENT(MEM_STORES, 0x343c0401e0);
+EVENT(MEM_LOADS, 0x35340401e0);
+EVENT(MEM_STORES, 0x353c0401e0);
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v2] crypto/nx: add missing call to of_node_put()
From: Herbert Xu @ 2021-03-04 6:45 UTC (permalink / raw)
To: Yang Li; +Cc: linux-kernel, paulus, linux-crypto, linuxppc-dev, davem
In-Reply-To: <1614302586-15095-1-git-send-email-yang.lee@linux.alibaba.com>
On Fri, Feb 26, 2021 at 09:23:06AM +0800, Yang Li wrote:
> In one of the error paths of the for_each_child_of_node() loop,
> add missing call to of_node_put().
>
> Fix the following coccicheck warning:
> ./drivers/crypto/nx/nx-common-powernv.c:927:1-23: WARNING: Function
> "for_each_child_of_node" should have of_node_put() before return around
> line 936.
>
> Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> Signed-off-by: Yang Li <yang.lee@linux.alibaba.com>
> ---
>
> Changes in v2:
> -add braces for if
>
> drivers/crypto/nx/nx-common-powernv.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] crypto: sha: remove unneeded semicolon
From: Herbert Xu @ 2021-03-04 6:34 UTC (permalink / raw)
To: Yang Li; +Cc: linux-kernel, paulus, linux-crypto, linuxppc-dev, davem
In-Reply-To: <1612775438-126031-1-git-send-email-yang.lee@linux.alibaba.com>
On Mon, Feb 08, 2021 at 05:10:38PM +0800, Yang Li wrote:
> Eliminate the following coccicheck warning:
> ./arch/powerpc/crypto/sha1-spe-glue.c:110:2-3: Unneeded semicolon
>
> Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> Signed-off-by: Yang Li <yang.lee@linux.alibaba.com>
> ---
> arch/powerpc/crypto/sha1-spe-glue.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v5 1/3] powerpc/book3s64/radix: Add H_RPT_INVALIDATE pgsize encodings to mmu_psize_def
From: David Gibson @ 2021-03-04 7:00 UTC (permalink / raw)
To: Bharata B Rao; +Cc: farosas, aneesh.kumar, npiggin, kvm-ppc, linuxppc-dev
In-Reply-To: <20210302042128.GB188607@in.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 4037 bytes --]
On Tue, Mar 02, 2021 at 09:51:28AM +0530, Bharata B Rao wrote:
> On Tue, Mar 02, 2021 at 12:28:34PM +1100, David Gibson wrote:
> > On Wed, Feb 24, 2021 at 01:55:08PM +0530, Bharata B Rao wrote:
> > > Add a field to mmu_psize_def to store the page size encodings
> > > of H_RPT_INVALIDATE hcall. Initialize this while scanning the radix
> > > AP encodings. This will be used when invalidating with required
> > > page size encoding in the hcall.
> > >
> > > Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
> > > ---
> > > arch/powerpc/include/asm/book3s/64/mmu.h | 1 +
> > > arch/powerpc/mm/book3s64/radix_pgtable.c | 5 +++++
> > > 2 files changed, 6 insertions(+)
> > >
> > > diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/include/asm/book3s/64/mmu.h
> > > index eace8c3f7b0a..c02f42d1031e 100644
> > > --- a/arch/powerpc/include/asm/book3s/64/mmu.h
> > > +++ b/arch/powerpc/include/asm/book3s/64/mmu.h
> > > @@ -19,6 +19,7 @@ struct mmu_psize_def {
> > > int penc[MMU_PAGE_COUNT]; /* HPTE encoding */
> > > unsigned int tlbiel; /* tlbiel supported for that page size */
> > > unsigned long avpnm; /* bits to mask out in AVPN in the HPTE */
> > > + unsigned long h_rpt_pgsize; /* H_RPT_INVALIDATE page size encoding */
> > > union {
> > > unsigned long sllp; /* SLB L||LP (exact mask to use in slbmte) */
> > > unsigned long ap; /* Ap encoding used by PowerISA 3.0 */
> > > diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c
> > > index 98f0b243c1ab..1b749899016b 100644
> > > --- a/arch/powerpc/mm/book3s64/radix_pgtable.c
> > > +++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
> > > @@ -486,6 +486,7 @@ static int __init radix_dt_scan_page_sizes(unsigned long node,
> > > def = &mmu_psize_defs[idx];
> > > def->shift = shift;
> > > def->ap = ap;
> > > + def->h_rpt_pgsize = psize_to_rpti_pgsize(idx);
> > > }
> > >
> > > /* needed ? */
> > > @@ -560,9 +561,13 @@ void __init radix__early_init_devtree(void)
> > > */
> > > mmu_psize_defs[MMU_PAGE_4K].shift = 12;
> > > mmu_psize_defs[MMU_PAGE_4K].ap = 0x0;
> > > + mmu_psize_defs[MMU_PAGE_4K].h_rpt_pgsize =
> > > + psize_to_rpti_pgsize(MMU_PAGE_4K);
> >
> > Hm. TBH, I was thinking of this as replacing psize_to_rpti_pgsize() -
> > that is, you directly put the correct codes in there, then just have
> > psize_to_rpti_pgsize() look them up in the table.
> >
> > I guess that could be a followup change, though.
> >
> > >
> > > mmu_psize_defs[MMU_PAGE_64K].shift = 16;
> > > mmu_psize_defs[MMU_PAGE_64K].ap = 0x5;
> > > + mmu_psize_defs[MMU_PAGE_64K].h_rpt_pgsize =
> > > + psize_to_rpti_pgsize(MMU_PAGE_64K);
>
> Hmm if you see I got rid of rpti_pgsize_to_psize() by having the
> defines directly in mmu_psize_def[].
I realize that, but I'm talking about the reverse direction:
psize_to_rpti_pgsize(). You should be able to reduce it a table
lookup, so the mmu_psize_defs table is the only place this information
exists.
> There are two cases in the above code (radix__early_init_devtree)
>
> 1. If radix pagesize encodings are present in the DT, we walk
> the page sizes in the loop and populate the enconding for
> H_RPT_INVALIDATE. I am not sure if we can use the direct codes
> in this case.
I'm not understanding the problem.
In any case the existing implementation of psize
Why ever not? You can just update the mmu_psize_defs when you parse
the device tree. Plus AFAICT, the existing psize_to_rpti
implementation doesn't take into account any device tree eencodings.
> 2. If DT doesn't have the radix pagesize encodings, 4K and 64K
> sizes are assumed as fallback sizes where we can use direct
> encodings.
Right... still not seeing the problem.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v5 2/3] KVM: PPC: Book3S HV: Add support for H_RPT_INVALIDATE
From: David Gibson @ 2021-03-04 6:56 UTC (permalink / raw)
To: Bharata B Rao; +Cc: farosas, aneesh.kumar, npiggin, kvm-ppc, linuxppc-dev
In-Reply-To: <20210302045853.GC188607@in.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 12561 bytes --]
On Tue, Mar 02, 2021 at 10:28:53AM +0530, Bharata B Rao wrote:
> On Tue, Mar 02, 2021 at 12:45:18PM +1100, David Gibson wrote:
> > > diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> > > index 45fd862ac128..38ce3f21b21f 100644
> > > --- a/Documentation/virt/kvm/api.rst
> > > +++ b/Documentation/virt/kvm/api.rst
> > > @@ -6225,6 +6225,24 @@ KVM_RUN_BUS_LOCK flag is used to distinguish between them.
> > > This capability can be used to check / enable 2nd DAWR feature provided
> > > by POWER10 processor.
> > >
> > > +7.23 KVM_CAP_PPC_RPT_INVALIDATE
> > > +------------------------------
> > > +
> > > +:Capability: KVM_CAP_PPC_RPT_INVALIDATE
> > > +:Architectures: ppc
> > > +:Type: vm
> > > +
> > > +This capability indicates that the kernel is capable of handling
> > > +H_RPT_INVALIDATE hcall.
> > > +
> > > +In order to enable the use of H_RPT_INVALIDATE in the guest,
> > > +user space might have to advertise it for the guest. For example,
> > > +IBM pSeries (sPAPR) guest starts using it if "hcall-rpt-invalidate" is
> > > +present in the "ibm,hypertas-functions" device-tree property.
> > > +
> > > +This capability is enabled for hypervisors on platforms like POWER9
> > > +that support radix MMU.
> >
> > Does this mean that KVM will handle the hypercall, even if not
> > explicitly enabled by userspace (qemu)? That's generally not what we
> > want, since we need to allow qemu to set up backwards compatible
> > guests.
>
> This capability only indicates that hypervisor supports the hcall.
>
> QEMU will check for this and conditionally enable the hcall
> (via KVM_CAP_PPC_ENABLE_HCALL ioctl). Enabling the hcall is
> conditional to cap-rpt-invalidate sPAPR machine capability being
> enabled by the user. Will post a followup QEMU patch shortly.
Ok.
> Older QEMU patch can be found here:
> https://lists.gnu.org/archive/html/qemu-devel/2021-01/msg00627.html
>
> >
> > > +
> > > 8. Other capabilities.
> > > ======================
> > >
> > > diff --git a/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h b/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
> > > index 8b33601cdb9d..a46fd37ad552 100644
> > > --- a/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
> > > +++ b/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
> > > @@ -4,6 +4,10 @@
> > >
> > > #include <asm/hvcall.h>
> > >
> > > +#define RIC_FLUSH_TLB 0
> > > +#define RIC_FLUSH_PWC 1
> > > +#define RIC_FLUSH_ALL 2
> > > +
> > > struct vm_area_struct;
> > > struct mm_struct;
> > > struct mmu_gather;
> > > diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
> > > index 2f5f919f6cd3..a1515f94400e 100644
> > > --- a/arch/powerpc/include/asm/kvm_book3s.h
> > > +++ b/arch/powerpc/include/asm/kvm_book3s.h
> > > @@ -305,6 +305,9 @@ void kvmhv_set_ptbl_entry(unsigned int lpid, u64 dw0, u64 dw1);
> > > void kvmhv_release_all_nested(struct kvm *kvm);
> > > long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu);
> > > long kvmhv_do_nested_tlbie(struct kvm_vcpu *vcpu);
> > > +long kvmhv_h_rpti_nested(struct kvm_vcpu *vcpu, unsigned long lpid,
> > > + unsigned long type, unsigned long pg_sizes,
> > > + unsigned long start, unsigned long end);
> > > int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu,
> > > u64 time_limit, unsigned long lpcr);
> > > void kvmhv_save_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr);
> > > diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
> > > index 652ce85f9410..820caf4e01b7 100644
> > > --- a/arch/powerpc/include/asm/mmu_context.h
> > > +++ b/arch/powerpc/include/asm/mmu_context.h
> > > @@ -124,8 +124,19 @@ static inline bool need_extra_context(struct mm_struct *mm, unsigned long ea)
> > >
> > > #if defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE) && defined(CONFIG_PPC_RADIX_MMU)
> > > extern void radix_kvm_prefetch_workaround(struct mm_struct *mm);
> > > +void do_h_rpt_invalidate(unsigned long pid, unsigned long lpid,
> > > + unsigned long type, unsigned long page_size,
> > > + unsigned long psize, unsigned long start,
> > > + unsigned long end);
> > > #else
> > > static inline void radix_kvm_prefetch_workaround(struct mm_struct *mm) { }
> > > +static inline void do_h_rpt_invalidate(unsigned long pid,
> > > + unsigned long lpid,
> > > + unsigned long type,
> > > + unsigned long page_size,
> > > + unsigned long psize,
> > > + unsigned long start,
> > > + unsigned long end) { }
> > > #endif
> > >
> > > extern void switch_cop(struct mm_struct *next);
> > > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> > > index 13bad6bf4c95..d83f006fc19d 100644
> > > --- a/arch/powerpc/kvm/book3s_hv.c
> > > +++ b/arch/powerpc/kvm/book3s_hv.c
> > > @@ -921,6 +921,69 @@ static int kvmppc_get_yield_count(struct kvm_vcpu *vcpu)
> > > return yield_count;
> > > }
> > >
> > > +static void do_h_rpt_invalidate_prs(unsigned long pid, unsigned long lpid,
> > > + unsigned long type, unsigned long pg_sizes,
> > > + unsigned long start, unsigned long end)
> > > +{
> > > + unsigned long psize;
> > > + struct mmu_psize_def *def;
> > > +
> > > + for (psize = 0; psize < MMU_PAGE_COUNT; psize++) {
> > > + def = &mmu_psize_defs[psize];
> > > + if (pg_sizes & def->h_rpt_pgsize)
> > > + do_h_rpt_invalidate(pid, lpid, type,
> > > + (1UL << def->shift), psize,
> > > + start, end);
> > > + }
> > > +}
> > > +
> > > +static void kvmppc_nested_rpt_invalidate(struct kvm_vcpu *vcpu)
> > > +{
> > > + do_h_rpt_invalidate_prs(kvmppc_get_gpr(vcpu, 4),
> > > + vcpu->arch.nested->shadow_lpid,
> > > + kvmppc_get_gpr(vcpu, 5),
> > > + kvmppc_get_gpr(vcpu, 6),
> > > + kvmppc_get_gpr(vcpu, 7),
> > > + kvmppc_get_gpr(vcpu, 8));
> > > + kvmppc_set_gpr(vcpu, 3, H_SUCCESS);
> > > +}
> > > +
> > > +static long kvmppc_h_rpt_invalidate(struct kvm_vcpu *vcpu,
> > > + unsigned long pid, unsigned long target,
> > > + unsigned long type, unsigned long pg_sizes,
> > > + unsigned long start, unsigned long end)
> > > +{
> > > + if (!kvm_is_radix(vcpu->kvm))
> > > + return H_UNSUPPORTED;
> > > +
> > > + /*
> > > + * For nested guests, this hcall is handled in
> > > + * L0. See kvmppc_handle_nested_exit() for details.
> > > + */
> > > + if (kvmhv_on_pseries())
> > > + return H_UNSUPPORTED;
> > > +
> > > + if (end < start)
> > > + return H_P5;
> > > +
> > > + if (type & H_RPTI_TYPE_NESTED) {
> > > + if (!nesting_enabled(vcpu->kvm))
> > > + return H_FUNCTION;
> > > +
> > > + /* Support only cores as target */
> > > + if (target != H_RPTI_TARGET_CMMU)
> > > + return H_P2;
> > > +
> >
> > IIUC, we'll hit this code path if an L1 calls this on behalf of an L2,
>
> Correct.
>
> > whereas we'll hit the nested exit code path going straight to
> > kvmhv_h_rpti_nested() if an L2 calls it on behalf of an L3. Is that
> > right?
>
> We will hit the nested exit code path when L2 calls it on behalf
> of L3. Looks like I am not handling this case (hcall issued by
> L2 on behalf of L3 for handling partition scoped translations)
> in the nested exit path.
>
> >
> > > + return kvmhv_h_rpti_nested(vcpu, pid,
> > > + (type & ~H_RPTI_TYPE_NESTED),
> > > + pg_sizes, start, end);
> > > + }
> > > +
> > > + do_h_rpt_invalidate_prs(pid, vcpu->kvm->arch.lpid, type, pg_sizes,
> > > + start, end);
> > > + return H_SUCCESS;
> > > +}
> > > +
> > > int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
> > > {
> > > unsigned long req = kvmppc_get_gpr(vcpu, 3);
> > > @@ -1129,6 +1192,14 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
> > > */
> > > ret = kvmppc_h_svm_init_abort(vcpu->kvm);
> > > break;
> > > + case H_RPT_INVALIDATE:
> > > + ret = kvmppc_h_rpt_invalidate(vcpu, kvmppc_get_gpr(vcpu, 4),
> > > + kvmppc_get_gpr(vcpu, 5),
> > > + kvmppc_get_gpr(vcpu, 6),
> > > + kvmppc_get_gpr(vcpu, 7),
> > > + kvmppc_get_gpr(vcpu, 8),
> > > + kvmppc_get_gpr(vcpu, 9));
> > > + break;
> > >
> > > default:
> > > return RESUME_HOST;
> > > @@ -1175,6 +1246,7 @@ static int kvmppc_hcall_impl_hv(unsigned long cmd)
> > > case H_XIRR_X:
> > > #endif
> > > case H_PAGE_INIT:
> > > + case H_RPT_INVALIDATE:
> > > return 1;
> > > }
> > >
> > > @@ -1590,6 +1662,24 @@ static int kvmppc_handle_nested_exit(struct kvm_vcpu *vcpu)
> > > if (!xics_on_xive())
> > > kvmppc_xics_rm_complete(vcpu, 0);
> > > break;
> > > + case BOOK3S_INTERRUPT_SYSCALL:
> > > + {
> > > + unsigned long req = kvmppc_get_gpr(vcpu, 3);
> > > +
> > > + /*
> > > + * The H_RPT_INVALIDATE hcalls issued by nested
> > > + * guests for process scoped invalidations when
> > > + * GTSE=0, are handled here in L0.
> > > + */
> >
> > What if the L2 is not calling this for the GTSE=0 case, but on behalf
> > of an L3?
>
> That case would be for flushing partition scoped translations. I am
> realizing that I am not handling that case, but it should be handled
> here in the nested hcall exit path.
>
> Currently I am handling only the hcall requests for process scoped
> translations from nested guests here.
Ok. At the very least you need to detect and fail for that case.
> >
> > > + if (req == H_RPT_INVALIDATE) {
> > > + kvmppc_nested_rpt_invalidate(vcpu);
> > > + r = RESUME_GUEST;
> > > + break;
> > > + }
> > > +
> > > + r = RESUME_HOST;
> > > + break;
> > > + }
> > > default:
> > > r = RESUME_HOST;
> > > break;
> > > diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
> > > index 0cd0e7aad588..ca43b2d38dce 100644
> > > --- a/arch/powerpc/kvm/book3s_hv_nested.c
> > > +++ b/arch/powerpc/kvm/book3s_hv_nested.c
> > > @@ -1191,6 +1191,83 @@ long kvmhv_do_nested_tlbie(struct kvm_vcpu *vcpu)
> > > return H_SUCCESS;
> > > }
> > >
> > > +static long do_tlb_invalidate_nested_tlb(struct kvm_vcpu *vcpu,
> > > + unsigned long lpid,
> > > + unsigned long page_size,
> > > + unsigned long ap,
> > > + unsigned long start,
> > > + unsigned long end)
> > > +{
> > > + unsigned long addr = start;
> > > + int ret;
> > > +
> > > + do {
> > > + ret = kvmhv_emulate_tlbie_tlb_addr(vcpu, lpid, ap,
> > > + get_epn(addr));
> > > + if (ret)
> > > + return ret;
> > > + addr += page_size;
> > > + } while (addr < end);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static long do_tlb_invalidate_nested_all(struct kvm_vcpu *vcpu,
> > > + unsigned long lpid)
> > > +{
> > > + struct kvm *kvm = vcpu->kvm;
> > > + struct kvm_nested_guest *gp;
> > > +
> > > + gp = kvmhv_get_nested(kvm, lpid, false);
> > > + if (gp) {
> > > + kvmhv_emulate_tlbie_lpid(vcpu, gp, RIC_FLUSH_ALL);
> > > + kvmhv_put_nested(gp);
> > > + }
> > > + return H_SUCCESS;
> > > +}
> > > +
> > > +long kvmhv_h_rpti_nested(struct kvm_vcpu *vcpu, unsigned long lpid,
> > > + unsigned long type, unsigned long pg_sizes,
> > > + unsigned long start, unsigned long end)
> > > +{
> > > + struct kvm_nested_guest *gp;
> > > + long ret;
> > > + unsigned long psize, ap;
> > > +
> > > + /*
> > > + * If L2 lpid isn't valid, we need to return H_PARAMETER.
> > > + *
> > > + * However, nested KVM issues a L2 lpid flush call when creating
> > > + * partition table entries for L2. This happens even before the
> > > + * corresponding shadow lpid is created in HV which happens in
> > > + * H_ENTER_NESTED call. Since we can't differentiate this case from
> > > + * the invalid case, we ignore such flush requests and return success.
> > > + */
> >
> > What if this is being called on behalf of an L3 or deeper? Do we need
> > something to do a translation from L3 to L2 addresses?
>
> I am not sure, I will have to check if gp->shadow_lpid points to
> correct nested LPID in all the cases.
>
> >
> > > + gp = kvmhv_find_nested(vcpu->kvm, lpid);
> > > + if (!gp)
> > > + return H_SUCCESS;
>
> Regards,
> Bharata.
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 1/5] CMDLINE: add generic builtin command line
From: Christophe Leroy @ 2021-03-04 7:00 UTC (permalink / raw)
To: Daniel Walker, Will Deacon, ob Herring, Daniel Gimpelevich,
Andrew Morton, x86, linux-mips, linuxppc-dev
Cc: Ruslan Bilovol, linux-kernel, xe-linux-external
In-Reply-To: <20210304044803.812204-1-danielwa@cisco.com>
Le 04/03/2021 à 05:47, Daniel Walker a écrit :
> This code allows architectures to use a generic builtin command line.
> The state of the builtin command line options across architecture is
> diverse. On x86 and mips they have pretty much the same code and the
> code prepends the builtin command line onto the boot loader provided
> one. On powerpc there is only a builtin override and nothing else.
This is not exact. powerpc has:
CONFIG_FROM_BOOTLOADER
CONFIG_EXTEND
CONFIG_FORCE
>
> The code in this commit unifies the code into a generic
> header file under the CONFIG_GENERIC_CMDLINE option. When this
> option is enabled the architecture can call the cmdline_add_builtin()
> to add the builtin command line.
>
> Cc: xe-linux-external@cisco.com
> Signed-off-by: Ruslan Bilovol <rbilovol@cisco.com>
> Signed-off-by: Daniel Walker <danielwa@cisco.com>
> ---
> include/linux/cmdline.h | 75 +++++++++++++++++++++++++++++++++++++++++
> init/Kconfig | 68 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 143 insertions(+)
> create mode 100644 include/linux/cmdline.h
>
> diff --git a/include/linux/cmdline.h b/include/linux/cmdline.h
> new file mode 100644
> index 000000000000..f44011d1a9ee
> --- /dev/null
> +++ b/include/linux/cmdline.h
> @@ -0,0 +1,75 @@
Missing the SPDX Licence Identifier
> +#ifndef _LINUX_CMDLINE_H
> +#define _LINUX_CMDLINE_H
> +
> +/*
> + *
> + * Copyright (C) 2006,2021. Cisco Systems, Inc.
> + *
> + * Generic Append/Prepend cmdline support.
> + */
> +
> +#if defined(CONFIG_GENERIC_CMDLINE) && defined(CONFIG_CMDLINE_BOOL)
I think it would be better if we can avoid the CONFIG_CMDLINE_BOOL.
By making the CMDLINEs default to "" at all time, I think we can about that BOOL.
> +
> +#ifndef CONFIG_CMDLINE_OVERRIDE
> +/*
> + * This function will append or prepend a builtin command line to the command
As far as I understand, it doesn't "append _or_ prepend" but it does "append _and_ prepend"
> + * line provided by the bootloader. Kconfig options can be used to alter
> + * the behavior of this builtin command line.
> + * @dest: The destination of the final appended/prepended string
> + * @src: The starting string or NULL if there isn't one.
> + * @tmp: temporary space used for prepending
> + * @length: the maximum length of the strings above.
Missing some parameters here, but I think we should avoid those 'strlcpy' and 'strlcat', see later
comment.
> + */
> +static inline void
> +__cmdline_add_builtin(char *dest, const char *src, char *tmp, unsigned long length,
> + size_t (*strlcpy)(char *dest, const char *src, size_t size),
> + size_t (*strlcat)(char *dest, const char *src, size_t count)
Don't use names that overide names of existing functions.
'count' is __kernel_size_t not size_t
> + )
> +{
> + if (src != dest && src != NULL) {
> + strlcpy(dest, " ", length);
Why do you need a space up front in that case ? Why not just copy the source to the destination ?
> + strlcat(dest, src, length);
> + }
> +
> + if (sizeof(CONFIG_CMDLINE_APPEND) > 1)
> + strlcat(dest, " " CONFIG_CMDLINE_APPEND, length);
> +
> + if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) {
> + strlcpy(tmp, CONFIG_CMDLINE_PREPEND " ", length);
> + strlcat(tmp, dest, length);
> + strlcpy(dest, tmp, length);
Could we use memmove(), or implement strmove() and avoid the temporary buffer at all ?
> + }
> +}
> +
> +#define cmdline_add_builtin_custom(dest, src, length, label, strlcpy, strlcat) \
It is misleading to call parameters 'strlcpy' or 'strlcat', it hides that they are overriden.
> +{ \
> + if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) { \
> + static label char cmdline_tmp_space[length]; \
Let the architecture define the temporary space when using the custom variant instead of just asking
the architecture to provide the name of the section to use. powerpc already have prom_scratch for that.
> + __cmdline_add_builtin(dest, src, cmdline_tmp_space, length, strlcpy, strlcat); \
> + } else if (sizeof(CONFIG_CMDLINE_APPEND) > 1) { \
> + __cmdline_add_builtin(dest, src, NULL, length, strlcpy, strlcat); \
> + } \
Ah, so if I understand correctly, the user can set both CONFIG_CMDLINE_PREPEND and
CONFIG_CMDLINE_APPEND but one of them is silently ignored.
Then I think we should just offer the user to set one, name it CONFIG_CMDLINE then ask him to choose
between FORCE, APPEND or PREPEND.
> +}
> +#define cmdline_add_builtin(dest, src, length) \
> + cmdline_add_builtin_custom(dest, src, length, __initdata, &strlcpy, &strlcat)
> +#else
> +#define cmdline_add_builtin(dest, src, length) \
> +{ \
> + strlcpy(dest, CONFIG_CMDLINE_PREPEND " " CONFIG_CMDLINE_APPEND, \
> + length); \
> +}
> +#endif /* !CONFIG_CMDLINE_OVERRIDE */
> +
> +#else
> +#define cmdline_add_builtin_custom(dest, src, length, label, strlcpy, strlcat) { \
> + if (src != NULL) \
> + strlcpy(dest, src, length); \
> +}
> +
> +#define cmdline_add_builtin(dest, src, length) { \
> + cmdline_add_builtin_custom(dest, src, length, strlcpy, strlcat); \
> +}
> +#endif /* CONFIG_GENERIC_CMDLINE */
I'd rather avoid all those macros and use static inline functions instead.
For the strlcpy() and strlcat(), use another name, for instance cmdline_strlcpy and cmdline_strlcat.
Then at the begining of the file, define them as strlcpy ad strlcat unless they are already defined
to something else (by the architecture before including cmdline.h).
> +
> +
> +#endif /* _LINUX_CMDLINE_H */
> diff --git a/init/Kconfig b/init/Kconfig
> index 29ad68325028..28363ab07cd4 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -2032,6 +2032,74 @@ config PROFILING
> config TRACEPOINTS
> bool
>
> +config GENERIC_CMDLINE
> + bool
> +
> +if GENERIC_CMDLINE
> +
> +config CMDLINE_BOOL
> + bool "Built-in kernel command line"
We don't need the CMDLINE_BOOL, just have CMDLINE always "" by default.
> + help
> + Allow for specifying boot arguments to the kernel at
> + build time. On some systems (e.g. embedded ones), it is
> + necessary or convenient to provide some or all of the
> + kernel boot arguments with the kernel itself (that is,
> + to not rely on the boot loader to provide them.)
> +
> + To compile command line arguments into the kernel,
> + set this option to 'Y', then fill in the
> + the boot arguments in CONFIG_CMDLINE.
> +
> + Systems with fully functional boot loaders (i.e. non-embedded)
> + should leave this option set to 'N'.
> +
> +config CMDLINE_APPEND
As far as I understand, the generic code will only take CMDLINE_APPEND into account if
CMDLINE_PREPEND doesn't exist, otherwise it will silently ignore it.
Only offer one string: CONFIG_CMDLINE, and make the use choose between APPEND, EXTEND or OVERRIDE
> + string "Built-in kernel command string append"
> + depends on CMDLINE_BOOL
> + default ""
> + help
> + Enter arguments here that should be compiled into the kernel
> + image and used at boot time. If the boot loader provides a
> + command line at boot time, this string is appended to it to
> + form the full kernel command line, when the system boots.
> +
> + However, you can use the CONFIG_CMDLINE_OVERRIDE option to
> + change this behavior.
> +
> + In most cases, the command line (whether built-in or provided
> + by the boot loader) should specify the device for the root
> + file system.
> +
> +config CMDLINE_PREPEND
> + string "Built-in kernel command string prepend"
> + depends on CMDLINE_BOOL
> + default ""
> + help
> + Enter arguments here that should be compiled into the kernel
> + image and used at boot time. If the boot loader provides a
> + command line at boot time, this string is prepended to it to
> + form the full kernel command line, when the system boots.
> +
> + However, you can use the CONFIG_CMDLINE_OVERRIDE option to
> + change this behavior.
> +
> + In most cases, the command line (whether built-in or provided
> + by the boot loader) should specify the device for the root
> + file system.
> +
> +config CMDLINE_OVERRIDE
> + bool "Built-in command line overrides boot loader arguments"
> + depends on CMDLINE_BOOL
> + help
> + Set this option to 'Y' to have the kernel ignore the boot loader
> + command line, and use ONLY the built-in command line. In this case
> + append and prepend strings are concatenated to form the full
> + command line.
> +
> + This is used to work around broken boot loaders. This should
> + be set to 'N' under normal conditions.
> +endif
> +
> endmenu # General setup
>
> source "arch/Kconfig"
>
Christophe
^ permalink raw reply
* Re: [PATCH 2/5] CMDLINE: drivers: of: ifdef out cmdline section
From: Christophe Leroy @ 2021-03-04 7:09 UTC (permalink / raw)
To: Daniel Walker, Will Deacon, ob Herring, Daniel Gimpelevich,
Andrew Morton, x86, linux-mips, linuxppc-dev, Rob Herring,
Frank Rowand
Cc: devicetree, Ruslan Ruslichenko, linux-kernel, xe-linux-external
In-Reply-To: <20210304044803.812204-2-danielwa@cisco.com>
Le 04/03/2021 à 05:47, Daniel Walker a écrit :
> It looks like there's some seepage of cmdline stuff into
> the generic device tree code. This conflicts with the
> generic cmdline implementation so I remove it in the case
> when that's enabled.
>
> Cc: xe-linux-external@cisco.com
> Signed-off-by: Ruslan Ruslichenko <rruslich@cisco.com>
> Signed-off-by: Daniel Walker <danielwa@cisco.com>
> ---
> drivers/of/fdt.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index feb0f2d67fc5..cfe4f8d3c9f5 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -25,6 +25,7 @@
> #include <linux/serial_core.h>
> #include <linux/sysfs.h>
> #include <linux/random.h>
> +#include <linux/cmdline.h>
>
> #include <asm/setup.h> /* for COMMAND_LINE_SIZE */
> #include <asm/page.h>
> @@ -1048,8 +1049,18 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
>
> early_init_dt_check_for_initrd(node);
>
> +#ifdef CONFIG_GENERIC_CMDLINE
> /* Retrieve command line */
> p = of_get_flat_dt_prop(node, "bootargs", &l);
> +
> + /*
> + * The builtin command line will be added here, or it can override
> + * with the DT bootargs.
> + */
> + cmdline_add_builtin(data,
> + ((p != NULL && l > 0) ? p : NULL), /* This is sanity checking */
Can we do more simple ? If p is NULL, p is already NULL, so (l > 0 ? p : NULL) should be enough.
> + COMMAND_LINE_SIZE);
> +#else
What does p contains now that "p = of_get_flat_dt_prop(node, "bootargs", &l);" is only called when
CONFIG_GENERIC_CMDLINE is defined ?
> if (p != NULL && l > 0)
> strlcpy(data, p, min(l, COMMAND_LINE_SIZE));
>
> @@ -1070,6 +1081,7 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
> strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> #endif
> #endif /* CONFIG_CMDLINE */
> +#endif /* CONFIG_GENERIC_CMDLINE */
>
> pr_debug("Command line is: %s\n", (char *)data);
>
>
^ permalink raw reply
* Re: [PATCH 3/5] CMDLINE: powerpc: convert to generic builtin command line
From: Christophe Leroy @ 2021-03-04 7:22 UTC (permalink / raw)
To: Daniel Walker, Will Deacon, ob Herring, Daniel Gimpelevich,
Andrew Morton, x86, linux-mips, linuxppc-dev, Michael Ellerman,
Benjamin Herrenschmidt, Paul Mackerras
Cc: Ruslan Ruslichenko, Ruslan Bilovol, linux-kernel,
xe-linux-external
In-Reply-To: <20210304044803.812204-3-danielwa@cisco.com>
Le 04/03/2021 à 05:48, Daniel Walker a écrit :
> This updates the powerpc code to use the CONFIG_GENERIC_CMDLINE
> option.
Should be split in two patches. The change of strcpy to strlcpy should go in a first patch.
>
> Cc: xe-linux-external@cisco.com
> Signed-off-by: Ruslan Ruslichenko <rruslich@cisco.com>
> Signed-off-by: Ruslan Bilovol <rbilovol@cisco.com>
> Signed-off-by: Daniel Walker <danielwa@cisco.com>
> ---
> arch/powerpc/Kconfig | 37 +--------------------------------
> arch/powerpc/kernel/prom.c | 1 +
> arch/powerpc/kernel/prom_init.c | 31 +++++++++++++++------------
> 3 files changed, 20 insertions(+), 49 deletions(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 107bb4319e0e..276b06d5c961 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -167,6 +167,7 @@ config PPC
> select EDAC_SUPPORT
> select GENERIC_ATOMIC64 if PPC32
> select GENERIC_CLOCKEVENTS_BROADCAST if SMP
> + select GENERIC_CMDLINE
> select GENERIC_CMOS_UPDATE
> select GENERIC_CPU_AUTOPROBE
> select GENERIC_CPU_VULNERABILITIES if PPC_BARRIER_NOSPEC
> @@ -906,42 +907,6 @@ config PPC_DENORMALISATION
> Add support for handling denormalisation of single precision
> values. Useful for bare metal only. If unsure say Y here.
>
> -config CMDLINE
> - string "Initial kernel command string"
> - default ""
> - help
> - On some platforms, there is currently no way for the boot loader to
> - pass arguments to the kernel. For these platforms, you can supply
> - some command-line options at build time by entering them here. In
> - most cases you will need to specify the root device here.
> -
> -choice
> - prompt "Kernel command line type" if CMDLINE != ""
> - default CMDLINE_FROM_BOOTLOADER
> -
> -config CMDLINE_FROM_BOOTLOADER
> - bool "Use bootloader kernel arguments if available"
> - help
> - Uses the command-line options passed by the boot loader. If
> - the boot loader doesn't provide any, the default kernel command
> - string provided in CMDLINE will be used.
> -
> -config CMDLINE_EXTEND
> - bool "Extend bootloader kernel arguments"
> - help
> - The command-line arguments provided by the boot loader will be
> - appended to the default kernel command string.
> -
> -config CMDLINE_FORCE
> - bool "Always use the default kernel command string"
> - help
> - Always use the default kernel command string, even if the boot
> - loader passes other arguments to the kernel.
> - This is useful if you cannot or don't want to change the
> - command-line options your boot loader passes to the kernel.
> -
> -endchoice
> -
> config EXTRA_TARGETS
> string "Additional default image types"
> help
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index ae3c41730367..96d0a01be1b4 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -27,6 +27,7 @@
> #include <linux/irq.h>
> #include <linux/memblock.h>
> #include <linux/of.h>
> +#include <linux/cmdline.h>
> #include <linux/of_fdt.h>
> #include <linux/libfdt.h>
> #include <linux/cpu.h>
> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index e9d4eb6144e1..d752be688b62 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -27,6 +27,7 @@
> #include <linux/initrd.h>
> #include <linux/bitops.h>
> #include <linux/pgtable.h>
> +#include <linux/cmdline.h>
> #include <asm/prom.h>
> #include <asm/rtas.h>
> #include <asm/page.h>
> @@ -242,15 +243,6 @@ static int __init prom_strcmp(const char *cs, const char *ct)
> return 0;
> }
>
> -static char __init *prom_strcpy(char *dest, const char *src)
> -{
> - char *tmp = dest;
> -
> - while ((*dest++ = *src++) != '\0')
> - /* nothing */;
> - return tmp;
> -}
> -
> static int __init prom_strncmp(const char *cs, const char *ct, size_t count)
> {
> unsigned char c1, c2;
> @@ -276,6 +268,19 @@ static size_t __init prom_strlen(const char *s)
> return sc - s;
> }
>
> +static size_t __init prom_strlcpy(char *dest, const char *src, size_t size)
> +{
> + size_t ret = prom_strlen(src);
> +
> + if (size) {
> + size_t len = (ret >= size) ? size - 1 : ret;
> + memcpy(dest, src, len);
> + dest[len] = '\0';
> + }
> + return ret;
> +}
> +
> +
> static int __init prom_memcmp(const void *cs, const void *ct, size_t count)
> {
> const unsigned char *su1, *su2;
> @@ -778,9 +783,9 @@ static void __init early_cmdline_parse(void)
> if (!IS_ENABLED(CONFIG_CMDLINE_FORCE) && (long)prom.chosen > 0)
You have removed CONFIG_CMDLINE_FORCE and the generic cmdline doesn't provide it.
> l = prom_getprop(prom.chosen, "bootargs", p, COMMAND_LINE_SIZE-1);
>
> - if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) || l <= 0 || p[0] == '\0')
> - prom_strlcat(prom_cmd_line, " " CONFIG_CMDLINE,
> - sizeof(prom_cmd_line));
> + if (l <= 0 || p[0] == '\0') /* dbl check */
So it means cmdline_add_builtin_custom() is only called when bootloader does not provide bootargs ?
> + cmdline_add_builtin_custom(prom_cmd_line, NULL, sizeof(prom_cmd_line),
> + __prombss, &prom_strlcpy, &prom_strlcat);
>
> prom_printf("command line: %s\n", prom_cmd_line);
>
> @@ -2706,7 +2711,7 @@ static void __init flatten_device_tree(void)
>
> /* Add "phandle" in there, we'll need it */
> namep = make_room(&mem_start, &mem_end, 16, 1);
> - prom_strcpy(namep, "phandle");
> + prom_strlcpy(namep, "phandle", 8);
> mem_start = (unsigned long)namep + prom_strlen(namep) + 1;
>
> /* Build string array */
>
^ permalink raw reply
* Re: [PATCH v3] powerpc/uprobes: Validation for prefixed instruction
From: Christophe Leroy @ 2021-03-04 7:32 UTC (permalink / raw)
To: Ravi Bangoria, mpe
Cc: jniethe5, oleg, rostedt, linux-kernel, paulus, sandipan,
naveen.n.rao, linuxppc-dev
In-Reply-To: <20210304050529.59391-1-ravi.bangoria@linux.ibm.com>
Le 04/03/2021 à 06:05, Ravi Bangoria a écrit :
> As per ISA 3.1, prefixed instruction should not cross 64-byte
> boundary. So don't allow Uprobe on such prefixed instruction.
>
> There are two ways probed instruction is changed in mapped pages.
> First, when Uprobe is activated, it searches for all the relevant
> pages and replace instruction in them. In this case, if that probe
> is on the 64-byte unaligned prefixed instruction, error out
> directly. Second, when Uprobe is already active and user maps a
> relevant page via mmap(), instruction is replaced via mmap() code
> path. But because Uprobe is invalid, entire mmap() operation can
> not be stopped. In this case just print an error and continue.
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> ---
> v2: https://lore.kernel.org/r/20210204104703.273429-1-ravi.bangoria@linux.ibm.com
> v2->v3:
> - Drop restriction for Uprobe on suffix of prefixed instruction.
> It needs lot of code change including generic code but what
> we get in return is not worth it.
>
> arch/powerpc/kernel/uprobes.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
> index e8a63713e655..c400971ebe70 100644
> --- a/arch/powerpc/kernel/uprobes.c
> +++ b/arch/powerpc/kernel/uprobes.c
> @@ -41,6 +41,14 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe,
> if (addr & 0x03)
> return -EINVAL;
>
> + if (!IS_ENABLED(CONFIG_PPC64) || !cpu_has_feature(CPU_FTR_ARCH_31))
cpu_has_feature(CPU_FTR_ARCH_31) should return 'false' when CONFIG_PPC64 is not enabled, no need to
double check.
> + return 0;
> +
> + if (ppc_inst_prefixed(auprobe->insn) && (addr & 0x3F) == 0x3C) {
Maybe 3C instead of 4F ? : (addr & 0x3C) == 0x3C
What about
(addr & (SZ_64 - 4)) == SZ_64 - 4 to make it more explicit ?
Or ALIGN(addr, SZ_64) != ALIGN(addr + 8, SZ_64)
> + pr_info_ratelimited("Cannot register a uprobe on 64 byte unaligned prefixed instruction\n");
> + return -EINVAL;
> + }
> +
> return 0;
> }
>
>
Christophe
^ permalink raw reply
* Re: [PATCH 3/5] CMDLINE: powerpc: convert to generic builtin command line
From: Christophe Leroy @ 2021-03-04 7:38 UTC (permalink / raw)
To: Daniel Walker, Will Deacon, ob Herring, Daniel Gimpelevich,
Andrew Morton, x86, linux-mips, linuxppc-dev, Michael Ellerman,
Benjamin Herrenschmidt, Paul Mackerras
Cc: Ruslan Ruslichenko, Ruslan Bilovol, linux-kernel,
xe-linux-external
In-Reply-To: <20210304044803.812204-3-danielwa@cisco.com>
Le 04/03/2021 à 05:48, Daniel Walker a écrit :
> This updates the powerpc code to use the CONFIG_GENERIC_CMDLINE
> option.
In file included from arch/powerpc/kernel/prom_init.c:30:
arch/powerpc/kernel/prom_init.c: In function 'early_cmdline_parse':
arch/powerpc/kernel/prom_init.c:788:17: error: lvalue required as unary '&' operand
788 | __prombss, &prom_strlcpy, &prom_strlcat);
| ^
./include/linux/cmdline.h:66:3: note: in definition of macro 'cmdline_add_builtin_custom'
66 | strlcpy(dest, src, length); \
| ^~~~~~~
At top level:
arch/powerpc/kernel/prom_init.c:312:22: error: 'prom_strlcat' defined but not used
[-Werror=unused-function]
312 | static size_t __init prom_strlcat(char *dest, const char *src, size_t count)
| ^~~~~~~~~~~~
cc1: all warnings being treated as errors
make[2]: *** [scripts/Makefile.build:279: arch/powerpc/kernel/prom_init.o] Error 1
make[1]: *** [scripts/Makefile.build:496: arch/powerpc/kernel] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1805: arch/powerpc] Error 2
make: *** Waiting for unfinished jobs....
>
> Cc: xe-linux-external@cisco.com
> Signed-off-by: Ruslan Ruslichenko <rruslich@cisco.com>
> Signed-off-by: Ruslan Bilovol <rbilovol@cisco.com>
> Signed-off-by: Daniel Walker <danielwa@cisco.com>
> ---
> arch/powerpc/Kconfig | 37 +--------------------------------
> arch/powerpc/kernel/prom.c | 1 +
> arch/powerpc/kernel/prom_init.c | 31 +++++++++++++++------------
> 3 files changed, 20 insertions(+), 49 deletions(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 107bb4319e0e..276b06d5c961 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -167,6 +167,7 @@ config PPC
> select EDAC_SUPPORT
> select GENERIC_ATOMIC64 if PPC32
> select GENERIC_CLOCKEVENTS_BROADCAST if SMP
> + select GENERIC_CMDLINE
> select GENERIC_CMOS_UPDATE
> select GENERIC_CPU_AUTOPROBE
> select GENERIC_CPU_VULNERABILITIES if PPC_BARRIER_NOSPEC
> @@ -906,42 +907,6 @@ config PPC_DENORMALISATION
> Add support for handling denormalisation of single precision
> values. Useful for bare metal only. If unsure say Y here.
>
> -config CMDLINE
> - string "Initial kernel command string"
> - default ""
> - help
> - On some platforms, there is currently no way for the boot loader to
> - pass arguments to the kernel. For these platforms, you can supply
> - some command-line options at build time by entering them here. In
> - most cases you will need to specify the root device here.
> -
> -choice
> - prompt "Kernel command line type" if CMDLINE != ""
> - default CMDLINE_FROM_BOOTLOADER
> -
> -config CMDLINE_FROM_BOOTLOADER
> - bool "Use bootloader kernel arguments if available"
> - help
> - Uses the command-line options passed by the boot loader. If
> - the boot loader doesn't provide any, the default kernel command
> - string provided in CMDLINE will be used.
> -
> -config CMDLINE_EXTEND
> - bool "Extend bootloader kernel arguments"
> - help
> - The command-line arguments provided by the boot loader will be
> - appended to the default kernel command string.
> -
> -config CMDLINE_FORCE
> - bool "Always use the default kernel command string"
> - help
> - Always use the default kernel command string, even if the boot
> - loader passes other arguments to the kernel.
> - This is useful if you cannot or don't want to change the
> - command-line options your boot loader passes to the kernel.
> -
> -endchoice
> -
> config EXTRA_TARGETS
> string "Additional default image types"
> help
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index ae3c41730367..96d0a01be1b4 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -27,6 +27,7 @@
> #include <linux/irq.h>
> #include <linux/memblock.h>
> #include <linux/of.h>
> +#include <linux/cmdline.h>
> #include <linux/of_fdt.h>
> #include <linux/libfdt.h>
> #include <linux/cpu.h>
> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index e9d4eb6144e1..d752be688b62 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -27,6 +27,7 @@
> #include <linux/initrd.h>
> #include <linux/bitops.h>
> #include <linux/pgtable.h>
> +#include <linux/cmdline.h>
> #include <asm/prom.h>
> #include <asm/rtas.h>
> #include <asm/page.h>
> @@ -242,15 +243,6 @@ static int __init prom_strcmp(const char *cs, const char *ct)
> return 0;
> }
>
> -static char __init *prom_strcpy(char *dest, const char *src)
> -{
> - char *tmp = dest;
> -
> - while ((*dest++ = *src++) != '\0')
> - /* nothing */;
> - return tmp;
> -}
> -
> static int __init prom_strncmp(const char *cs, const char *ct, size_t count)
> {
> unsigned char c1, c2;
> @@ -276,6 +268,19 @@ static size_t __init prom_strlen(const char *s)
> return sc - s;
> }
>
> +static size_t __init prom_strlcpy(char *dest, const char *src, size_t size)
> +{
> + size_t ret = prom_strlen(src);
> +
> + if (size) {
> + size_t len = (ret >= size) ? size - 1 : ret;
> + memcpy(dest, src, len);
> + dest[len] = '\0';
> + }
> + return ret;
> +}
> +
> +
> static int __init prom_memcmp(const void *cs, const void *ct, size_t count)
> {
> const unsigned char *su1, *su2;
> @@ -778,9 +783,9 @@ static void __init early_cmdline_parse(void)
> if (!IS_ENABLED(CONFIG_CMDLINE_FORCE) && (long)prom.chosen > 0)
> l = prom_getprop(prom.chosen, "bootargs", p, COMMAND_LINE_SIZE-1);
>
> - if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) || l <= 0 || p[0] == '\0')
> - prom_strlcat(prom_cmd_line, " " CONFIG_CMDLINE,
> - sizeof(prom_cmd_line));
> + if (l <= 0 || p[0] == '\0') /* dbl check */
> + cmdline_add_builtin_custom(prom_cmd_line, NULL, sizeof(prom_cmd_line),
> + __prombss, &prom_strlcpy, &prom_strlcat);
>
> prom_printf("command line: %s\n", prom_cmd_line);
>
> @@ -2706,7 +2711,7 @@ static void __init flatten_device_tree(void)
>
> /* Add "phandle" in there, we'll need it */
> namep = make_room(&mem_start, &mem_end, 16, 1);
> - prom_strcpy(namep, "phandle");
> + prom_strlcpy(namep, "phandle", 8);
> mem_start = (unsigned long)namep + prom_strlen(namep) + 1;
>
> /* Build string array */
>
^ permalink raw reply
* Re: [PATCH 3/5] CMDLINE: powerpc: convert to generic builtin command line
From: Christophe Leroy @ 2021-03-04 7:39 UTC (permalink / raw)
To: Daniel Walker, Will Deacon, ob Herring, Daniel Gimpelevich,
Andrew Morton, x86, linux-mips, linuxppc-dev, Michael Ellerman,
Benjamin Herrenschmidt, Paul Mackerras
Cc: Ruslan Ruslichenko, Ruslan Bilovol, linux-kernel,
xe-linux-external
In-Reply-To: <20210304044803.812204-3-danielwa@cisco.com>
Le 04/03/2021 à 05:48, Daniel Walker a écrit :
> This updates the powerpc code to use the CONFIG_GENERIC_CMDLINE
> option.
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#143: FILE: arch/powerpc/kernel/prom_init.c:788:
+ cmdline_add_builtin_custom(prom_cmd_line, NULL, sizeof(prom_cmd_line),
+ __prombss, &prom_strlcpy, &prom_strlcat);
total: 0 errors, 0 warnings, 1 checks, 117 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
the.patch has style problems, please review.
NOTE: Ignored message types: ARCH_INCLUDE_LINUX BIT_MACRO COMPARISON_TO_NULL DT_SPLIT_BINDING_PATCH
EMAIL_SUBJECT FILE_PATH_CHANGES GLOBAL_INITIALISERS LINE_SPACING MULTIPLE_ASSIGNMENTS
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
>
> Cc: xe-linux-external@cisco.com
> Signed-off-by: Ruslan Ruslichenko <rruslich@cisco.com>
> Signed-off-by: Ruslan Bilovol <rbilovol@cisco.com>
> Signed-off-by: Daniel Walker <danielwa@cisco.com>
> ---
> arch/powerpc/Kconfig | 37 +--------------------------------
> arch/powerpc/kernel/prom.c | 1 +
> arch/powerpc/kernel/prom_init.c | 31 +++++++++++++++------------
> 3 files changed, 20 insertions(+), 49 deletions(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 107bb4319e0e..276b06d5c961 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -167,6 +167,7 @@ config PPC
> select EDAC_SUPPORT
> select GENERIC_ATOMIC64 if PPC32
> select GENERIC_CLOCKEVENTS_BROADCAST if SMP
> + select GENERIC_CMDLINE
> select GENERIC_CMOS_UPDATE
> select GENERIC_CPU_AUTOPROBE
> select GENERIC_CPU_VULNERABILITIES if PPC_BARRIER_NOSPEC
> @@ -906,42 +907,6 @@ config PPC_DENORMALISATION
> Add support for handling denormalisation of single precision
> values. Useful for bare metal only. If unsure say Y here.
>
> -config CMDLINE
> - string "Initial kernel command string"
> - default ""
> - help
> - On some platforms, there is currently no way for the boot loader to
> - pass arguments to the kernel. For these platforms, you can supply
> - some command-line options at build time by entering them here. In
> - most cases you will need to specify the root device here.
> -
> -choice
> - prompt "Kernel command line type" if CMDLINE != ""
> - default CMDLINE_FROM_BOOTLOADER
> -
> -config CMDLINE_FROM_BOOTLOADER
> - bool "Use bootloader kernel arguments if available"
> - help
> - Uses the command-line options passed by the boot loader. If
> - the boot loader doesn't provide any, the default kernel command
> - string provided in CMDLINE will be used.
> -
> -config CMDLINE_EXTEND
> - bool "Extend bootloader kernel arguments"
> - help
> - The command-line arguments provided by the boot loader will be
> - appended to the default kernel command string.
> -
> -config CMDLINE_FORCE
> - bool "Always use the default kernel command string"
> - help
> - Always use the default kernel command string, even if the boot
> - loader passes other arguments to the kernel.
> - This is useful if you cannot or don't want to change the
> - command-line options your boot loader passes to the kernel.
> -
> -endchoice
> -
> config EXTRA_TARGETS
> string "Additional default image types"
> help
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index ae3c41730367..96d0a01be1b4 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -27,6 +27,7 @@
> #include <linux/irq.h>
> #include <linux/memblock.h>
> #include <linux/of.h>
> +#include <linux/cmdline.h>
> #include <linux/of_fdt.h>
> #include <linux/libfdt.h>
> #include <linux/cpu.h>
> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index e9d4eb6144e1..d752be688b62 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -27,6 +27,7 @@
> #include <linux/initrd.h>
> #include <linux/bitops.h>
> #include <linux/pgtable.h>
> +#include <linux/cmdline.h>
> #include <asm/prom.h>
> #include <asm/rtas.h>
> #include <asm/page.h>
> @@ -242,15 +243,6 @@ static int __init prom_strcmp(const char *cs, const char *ct)
> return 0;
> }
>
> -static char __init *prom_strcpy(char *dest, const char *src)
> -{
> - char *tmp = dest;
> -
> - while ((*dest++ = *src++) != '\0')
> - /* nothing */;
> - return tmp;
> -}
> -
> static int __init prom_strncmp(const char *cs, const char *ct, size_t count)
> {
> unsigned char c1, c2;
> @@ -276,6 +268,19 @@ static size_t __init prom_strlen(const char *s)
> return sc - s;
> }
>
> +static size_t __init prom_strlcpy(char *dest, const char *src, size_t size)
> +{
> + size_t ret = prom_strlen(src);
> +
> + if (size) {
> + size_t len = (ret >= size) ? size - 1 : ret;
> + memcpy(dest, src, len);
> + dest[len] = '\0';
> + }
> + return ret;
> +}
> +
> +
> static int __init prom_memcmp(const void *cs, const void *ct, size_t count)
> {
> const unsigned char *su1, *su2;
> @@ -778,9 +783,9 @@ static void __init early_cmdline_parse(void)
> if (!IS_ENABLED(CONFIG_CMDLINE_FORCE) && (long)prom.chosen > 0)
> l = prom_getprop(prom.chosen, "bootargs", p, COMMAND_LINE_SIZE-1);
>
> - if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) || l <= 0 || p[0] == '\0')
> - prom_strlcat(prom_cmd_line, " " CONFIG_CMDLINE,
> - sizeof(prom_cmd_line));
> + if (l <= 0 || p[0] == '\0') /* dbl check */
> + cmdline_add_builtin_custom(prom_cmd_line, NULL, sizeof(prom_cmd_line),
> + __prombss, &prom_strlcpy, &prom_strlcat);
>
> prom_printf("command line: %s\n", prom_cmd_line);
>
> @@ -2706,7 +2711,7 @@ static void __init flatten_device_tree(void)
>
> /* Add "phandle" in there, we'll need it */
> namep = make_room(&mem_start, &mem_end, 16, 1);
> - prom_strcpy(namep, "phandle");
> + prom_strlcpy(namep, "phandle", 8);
> mem_start = (unsigned long)namep + prom_strlen(namep) + 1;
>
> /* Build string array */
>
^ permalink raw reply
* Re: [PATCH v3] powerpc/uprobes: Validation for prefixed instruction
From: Ravi Bangoria @ 2021-03-04 7:39 UTC (permalink / raw)
To: Naveen N. Rao
Cc: Ravi Bangoria, oleg, rostedt, linux-kernel, paulus, sandipan,
jniethe5, naveen.n.rao, linuxppc-dev
In-Reply-To: <20210303083836.GD1913@DESKTOP-TDPLP67.localdomain>
>> @@ -41,6 +41,14 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe,
>> if (addr & 0x03)
>> return -EINVAL;
>>
>> + if (!IS_ENABLED(CONFIG_PPC64) || !cpu_has_feature(CPU_FTR_ARCH_31))
>> + return 0;
>
> Sorry, I missed this last time, but I think we can drop the above
> checks. ppc_inst_prefixed() already factors in the dependency on
> CONFIG_PPC64,
Yeah, makes sense. I initially added CONFIG_PPC64 check because
I was using ppc_inst_prefix(x, y) macro which is not available
for !CONFIG_PPC64.
> and I don't think we need to confirm if we're running on a
> ISA V3.1 for the below check.
Prefixed instructions would be supported only when ARCH_31 is set.
(Ignoring insane scenario where user probes on prefixed instruction
on p10 predecessors). So I guess I still need ARCH_31 check?
Ravi
^ permalink raw reply
* Re: [PATCH 1/5] CMDLINE: add generic builtin command line
From: Christophe Leroy @ 2021-03-04 7:40 UTC (permalink / raw)
To: Daniel Walker, Will Deacon, ob Herring, Daniel Gimpelevich,
Andrew Morton, x86, linux-mips, linuxppc-dev
Cc: Ruslan Bilovol, linux-kernel, xe-linux-external
In-Reply-To: <20210304044803.812204-1-danielwa@cisco.com>
Le 04/03/2021 à 05:47, Daniel Walker a écrit :
> This code allows architectures to use a generic builtin command line.
> The state of the builtin command line options across architecture is
> diverse. On x86 and mips they have pretty much the same code and the
> code prepends the builtin command line onto the boot loader provided
> one. On powerpc there is only a builtin override and nothing else.
>
> The code in this commit unifies the code into a generic
> header file under the CONFIG_GENERIC_CMDLINE option. When this
> option is enabled the architecture can call the cmdline_add_builtin()
> to add the builtin command line.
WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#32: FILE: include/linux/cmdline.h:1:
+#ifndef _LINUX_CMDLINE_H
CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#56: FILE: include/linux/cmdline.h:25:
+__cmdline_add_builtin(char *dest, const char *src, char *tmp, unsigned long length,
+ size_t (*strlcpy)(char *dest, const char *src, size_t size),
WARNING:STRLCPY: Prefer strscpy over strlcpy - see:
https://lore.kernel.org/r/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=V6A6G1oUZcprmknw@mail.gmail.com/
#61: FILE: include/linux/cmdline.h:30:
+ strlcpy(dest, " ", length);
WARNING:STRLCPY: Prefer strscpy over strlcpy - see:
https://lore.kernel.org/r/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=V6A6G1oUZcprmknw@mail.gmail.com/
#69: FILE: include/linux/cmdline.h:38:
+ strlcpy(tmp, CONFIG_CMDLINE_PREPEND " ", length);
WARNING:STRLCPY: Prefer strscpy over strlcpy - see:
https://lore.kernel.org/r/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=V6A6G1oUZcprmknw@mail.gmail.com/
#71: FILE: include/linux/cmdline.h:40:
+ strlcpy(dest, tmp, length);
WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#75: FILE: include/linux/cmdline.h:44:
+#define cmdline_add_builtin_custom(dest, src, length, label, strlcpy, strlcat) ^I^I^I\$
CHECK:MACRO_ARG_REUSE: Macro argument reuse 'dest' - possible side-effects?
#75: FILE: include/linux/cmdline.h:44:
+#define cmdline_add_builtin_custom(dest, src, length, label, strlcpy, strlcat) \
+{ \
+ if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) { \
+ static label char cmdline_tmp_space[length]; \
+ __cmdline_add_builtin(dest, src, cmdline_tmp_space, length, strlcpy, strlcat); \
+ } else if (sizeof(CONFIG_CMDLINE_APPEND) > 1) { \
+ __cmdline_add_builtin(dest, src, NULL, length, strlcpy, strlcat); \
+ } \
+}
CHECK:MACRO_ARG_REUSE: Macro argument reuse 'src' - possible side-effects?
#75: FILE: include/linux/cmdline.h:44:
+#define cmdline_add_builtin_custom(dest, src, length, label, strlcpy, strlcat) \
+{ \
+ if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) { \
+ static label char cmdline_tmp_space[length]; \
+ __cmdline_add_builtin(dest, src, cmdline_tmp_space, length, strlcpy, strlcat); \
+ } else if (sizeof(CONFIG_CMDLINE_APPEND) > 1) { \
+ __cmdline_add_builtin(dest, src, NULL, length, strlcpy, strlcat); \
+ } \
+}
CHECK:MACRO_ARG_REUSE: Macro argument reuse 'length' - possible side-effects?
#75: FILE: include/linux/cmdline.h:44:
+#define cmdline_add_builtin_custom(dest, src, length, label, strlcpy, strlcat) \
+{ \
+ if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) { \
+ static label char cmdline_tmp_space[length]; \
+ __cmdline_add_builtin(dest, src, cmdline_tmp_space, length, strlcpy, strlcat); \
+ } else if (sizeof(CONFIG_CMDLINE_APPEND) > 1) { \
+ __cmdline_add_builtin(dest, src, NULL, length, strlcpy, strlcat); \
+ } \
+}
CHECK:MACRO_ARG_REUSE: Macro argument reuse 'strlcpy' - possible side-effects?
#75: FILE: include/linux/cmdline.h:44:
+#define cmdline_add_builtin_custom(dest, src, length, label, strlcpy, strlcat) \
+{ \
+ if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) { \
+ static label char cmdline_tmp_space[length]; \
+ __cmdline_add_builtin(dest, src, cmdline_tmp_space, length, strlcpy, strlcat); \
+ } else if (sizeof(CONFIG_CMDLINE_APPEND) > 1) { \
+ __cmdline_add_builtin(dest, src, NULL, length, strlcpy, strlcat); \
+ } \
+}
CHECK:MACRO_ARG_REUSE: Macro argument reuse 'strlcat' - possible side-effects?
#75: FILE: include/linux/cmdline.h:44:
+#define cmdline_add_builtin_custom(dest, src, length, label, strlcpy, strlcat) \
+{ \
+ if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) { \
+ static label char cmdline_tmp_space[length]; \
+ __cmdline_add_builtin(dest, src, cmdline_tmp_space, length, strlcpy, strlcat); \
+ } else if (sizeof(CONFIG_CMDLINE_APPEND) > 1) { \
+ __cmdline_add_builtin(dest, src, NULL, length, strlcpy, strlcat); \
+ } \
+}
WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#76: FILE: include/linux/cmdline.h:45:
+{ ^I^I^I^I^I^I^I^I^I^I^I^I\$
WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#77: FILE: include/linux/cmdline.h:46:
+^Iif (sizeof(CONFIG_CMDLINE_PREPEND) > 1) { ^I^I^I^I^I^I\$
WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#78: FILE: include/linux/cmdline.h:47:
+^I^Istatic label char cmdline_tmp_space[length]; ^I^I^I^I^I\$
WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#79: FILE: include/linux/cmdline.h:48:
+^I^I__cmdline_add_builtin(dest, src, cmdline_tmp_space, length, strlcpy, strlcat); ^I\$
WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#80: FILE: include/linux/cmdline.h:49:
+^I} else if (sizeof(CONFIG_CMDLINE_APPEND) > 1) { ^I^I^I^I^I\$
WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#81: FILE: include/linux/cmdline.h:50:
+^I^I__cmdline_add_builtin(dest, src, NULL, length, strlcpy, strlcat); ^I^I\$
WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#82: FILE: include/linux/cmdline.h:51:
+^I} ^I^I^I^I^I^I^I^I^I^I^I\$
WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#88: FILE: include/linux/cmdline.h:57:
+{^I^I^I^I^I^I^I^I ^I \$
WARNING:STRLCPY: Prefer strscpy over strlcpy - see:
https://lore.kernel.org/r/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=V6A6G1oUZcprmknw@mail.gmail.com/
#89: FILE: include/linux/cmdline.h:58:
+ strlcpy(dest, CONFIG_CMDLINE_PREPEND " " CONFIG_CMDLINE_APPEND, \
WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#90: FILE: include/linux/cmdline.h:59:
+^I^Ilength);^I^I ^I^I^I^I \$
CHECK:MACRO_ARG_REUSE: Macro argument reuse 'src' - possible side-effects?
#95: FILE: include/linux/cmdline.h:64:
+#define cmdline_add_builtin_custom(dest, src, length, label, strlcpy, strlcat) { \
+ if (src != NULL) \
+ strlcpy(dest, src, length); \
+}
CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'src' may be better as '(src)' to avoid precedence issues
#95: FILE: include/linux/cmdline.h:64:
+#define cmdline_add_builtin_custom(dest, src, length, label, strlcpy, strlcat) { \
+ if (src != NULL) \
+ strlcpy(dest, src, length); \
+}
WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#96: FILE: include/linux/cmdline.h:65:
+^Iif (src != NULL) ^I^I^I^I^I^I^I \$
WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#97: FILE: include/linux/cmdline.h:66:
+^I^Istrlcpy(dest, src, length);^I ^I^I^I^I \$
WARNING:STRLCPY: Prefer strscpy over strlcpy - see:
https://lore.kernel.org/r/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=V6A6G1oUZcprmknw@mail.gmail.com/
#97: FILE: include/linux/cmdline.h:66:
+ strlcpy(dest, src, length); \
WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#100: FILE: include/linux/cmdline.h:69:
+#define cmdline_add_builtin(dest, src, length) { ^I^I^I^I\$
WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#101: FILE: include/linux/cmdline.h:70:
+^Icmdline_add_builtin_custom(dest, src, length, strlcpy, strlcat); ^I\$
total: 0 errors, 20 warnings, 8 checks, 149 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
the.patch has style problems, please review.
NOTE: Ignored message types: ARCH_INCLUDE_LINUX BIT_MACRO COMPARISON_TO_NULL DT_SPLIT_BINDING_PATCH
EMAIL_SUBJECT FILE_PATH_CHANGES GLOBAL_INITIALISERS LINE_SPACING MULTIPLE_ASSIGNMENTS
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
>
> Cc: xe-linux-external@cisco.com
> Signed-off-by: Ruslan Bilovol <rbilovol@cisco.com>
> Signed-off-by: Daniel Walker <danielwa@cisco.com>
> ---
> include/linux/cmdline.h | 75 +++++++++++++++++++++++++++++++++++++++++
> init/Kconfig | 68 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 143 insertions(+)
> create mode 100644 include/linux/cmdline.h
>
> diff --git a/include/linux/cmdline.h b/include/linux/cmdline.h
> new file mode 100644
> index 000000000000..f44011d1a9ee
> --- /dev/null
> +++ b/include/linux/cmdline.h
> @@ -0,0 +1,75 @@
> +#ifndef _LINUX_CMDLINE_H
> +#define _LINUX_CMDLINE_H
> +
> +/*
> + *
> + * Copyright (C) 2006,2021. Cisco Systems, Inc.
> + *
> + * Generic Append/Prepend cmdline support.
> + */
> +
> +#if defined(CONFIG_GENERIC_CMDLINE) && defined(CONFIG_CMDLINE_BOOL)
> +
> +#ifndef CONFIG_CMDLINE_OVERRIDE
> +/*
> + * This function will append or prepend a builtin command line to the command
> + * line provided by the bootloader. Kconfig options can be used to alter
> + * the behavior of this builtin command line.
> + * @dest: The destination of the final appended/prepended string
> + * @src: The starting string or NULL if there isn't one.
> + * @tmp: temporary space used for prepending
> + * @length: the maximum length of the strings above.
> + */
> +static inline void
> +__cmdline_add_builtin(char *dest, const char *src, char *tmp, unsigned long length,
> + size_t (*strlcpy)(char *dest, const char *src, size_t size),
> + size_t (*strlcat)(char *dest, const char *src, size_t count)
> + )
> +{
> + if (src != dest && src != NULL) {
> + strlcpy(dest, " ", length);
> + strlcat(dest, src, length);
> + }
> +
> + if (sizeof(CONFIG_CMDLINE_APPEND) > 1)
> + strlcat(dest, " " CONFIG_CMDLINE_APPEND, length);
> +
> + if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) {
> + strlcpy(tmp, CONFIG_CMDLINE_PREPEND " ", length);
> + strlcat(tmp, dest, length);
> + strlcpy(dest, tmp, length);
> + }
> +}
> +
> +#define cmdline_add_builtin_custom(dest, src, length, label, strlcpy, strlcat) \
> +{ \
> + if (sizeof(CONFIG_CMDLINE_PREPEND) > 1) { \
> + static label char cmdline_tmp_space[length]; \
> + __cmdline_add_builtin(dest, src, cmdline_tmp_space, length, strlcpy, strlcat); \
> + } else if (sizeof(CONFIG_CMDLINE_APPEND) > 1) { \
> + __cmdline_add_builtin(dest, src, NULL, length, strlcpy, strlcat); \
> + } \
> +}
> +#define cmdline_add_builtin(dest, src, length) \
> + cmdline_add_builtin_custom(dest, src, length, __initdata, &strlcpy, &strlcat)
> +#else
> +#define cmdline_add_builtin(dest, src, length) \
> +{ \
> + strlcpy(dest, CONFIG_CMDLINE_PREPEND " " CONFIG_CMDLINE_APPEND, \
> + length); \
> +}
> +#endif /* !CONFIG_CMDLINE_OVERRIDE */
> +
> +#else
> +#define cmdline_add_builtin_custom(dest, src, length, label, strlcpy, strlcat) { \
> + if (src != NULL) \
> + strlcpy(dest, src, length); \
> +}
> +
> +#define cmdline_add_builtin(dest, src, length) { \
> + cmdline_add_builtin_custom(dest, src, length, strlcpy, strlcat); \
> +}
> +#endif /* CONFIG_GENERIC_CMDLINE */
> +
> +
> +#endif /* _LINUX_CMDLINE_H */
> diff --git a/init/Kconfig b/init/Kconfig
> index 29ad68325028..28363ab07cd4 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -2032,6 +2032,74 @@ config PROFILING
> config TRACEPOINTS
> bool
>
> +config GENERIC_CMDLINE
> + bool
> +
> +if GENERIC_CMDLINE
> +
> +config CMDLINE_BOOL
> + bool "Built-in kernel command line"
> + help
> + Allow for specifying boot arguments to the kernel at
> + build time. On some systems (e.g. embedded ones), it is
> + necessary or convenient to provide some or all of the
> + kernel boot arguments with the kernel itself (that is,
> + to not rely on the boot loader to provide them.)
> +
> + To compile command line arguments into the kernel,
> + set this option to 'Y', then fill in the
> + the boot arguments in CONFIG_CMDLINE.
> +
> + Systems with fully functional boot loaders (i.e. non-embedded)
> + should leave this option set to 'N'.
> +
> +config CMDLINE_APPEND
> + string "Built-in kernel command string append"
> + depends on CMDLINE_BOOL
> + default ""
> + help
> + Enter arguments here that should be compiled into the kernel
> + image and used at boot time. If the boot loader provides a
> + command line at boot time, this string is appended to it to
> + form the full kernel command line, when the system boots.
> +
> + However, you can use the CONFIG_CMDLINE_OVERRIDE option to
> + change this behavior.
> +
> + In most cases, the command line (whether built-in or provided
> + by the boot loader) should specify the device for the root
> + file system.
> +
> +config CMDLINE_PREPEND
> + string "Built-in kernel command string prepend"
> + depends on CMDLINE_BOOL
> + default ""
> + help
> + Enter arguments here that should be compiled into the kernel
> + image and used at boot time. If the boot loader provides a
> + command line at boot time, this string is prepended to it to
> + form the full kernel command line, when the system boots.
> +
> + However, you can use the CONFIG_CMDLINE_OVERRIDE option to
> + change this behavior.
> +
> + In most cases, the command line (whether built-in or provided
> + by the boot loader) should specify the device for the root
> + file system.
> +
> +config CMDLINE_OVERRIDE
> + bool "Built-in command line overrides boot loader arguments"
> + depends on CMDLINE_BOOL
> + help
> + Set this option to 'Y' to have the kernel ignore the boot loader
> + command line, and use ONLY the built-in command line. In this case
> + append and prepend strings are concatenated to form the full
> + command line.
> +
> + This is used to work around broken boot loaders. This should
> + be set to 'N' under normal conditions.
> +endif
> +
> endmenu # General setup
>
> source "arch/Kconfig"
>
^ permalink raw reply
* [PATCH] powerpc: fix coding style issues
From: Qiang Ma @ 2021-03-04 8:55 UTC (permalink / raw)
To: mpe, benh, paulus; +Cc: linuxppc-dev, linux-kernel, Qiang Ma
There are several style issues in this function,
so fix them.
Signed-off-by: Qiang Ma <maqianga@uniontech.com>
---
arch/powerpc/kernel/syscalls.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/kernel/syscalls.c b/arch/powerpc/kernel/syscalls.c
index 078608ec2e92..bcbb5fb2a0c1 100644
--- a/arch/powerpc/kernel/syscalls.c
+++ b/arch/powerpc/kernel/syscalls.c
@@ -81,15 +81,15 @@ SYSCALL_DEFINE6(mmap, unsigned long, addr, size_t, len,
int
ppc_select(int n, fd_set __user *inp, fd_set __user *outp, fd_set __user *exp, struct __kernel_old_timeval __user *tvp)
{
- if ( (unsigned long)n >= 4096 )
- {
+ if ((unsigned long)n >= 4096) {
unsigned long __user *buffer = (unsigned long __user *)n;
- if (!access_ok(buffer, 5*sizeof(unsigned long))
+ if (!access_ok(buffer, 5 * sizeof(unsigned long))
|| __get_user(n, buffer)
- || __get_user(inp, ((fd_set __user * __user *)(buffer+1)))
- || __get_user(outp, ((fd_set __user * __user *)(buffer+2)))
- || __get_user(exp, ((fd_set __user * __user *)(buffer+3)))
- || __get_user(tvp, ((struct __kernel_old_timeval __user * __user *)(buffer+4))))
+ || __get_user(inp, ((fd_set __user * __user *)(buffer + 1)))
+ || __get_user(outp, ((fd_set __user * __user *)(buffer + 2)))
+ || __get_user(exp, ((fd_set __user * __user *)(buffer + 3)))
+ || __get_user(tvp,
+ ((struct __kernel_old_timeval __user * __user *)(buffer + 4))))
return -EFAULT;
}
return sys_select(n, inp, outp, exp, tvp);
--
2.20.1
^ permalink raw reply related
* [PATCH v2] ASoC: imx-hdmi: fix platform_no_drv_owner.cocci warnings
From: Yang Li @ 2021-03-04 9:08 UTC (permalink / raw)
To: timur
Cc: alsa-devel, linuxppc-dev, linux-kernel, Xiubo.Lee, festevam,
s.hauer, tiwai, lgirdwood, perex, nicoleotsuka, broonie, Yang Li,
linux-imx, kernel, shawnguo, shengjiu.wang, linux-arm-kernel
./sound/soc/fsl/imx-hdmi.c:226:3-8: No need to set .owner here. The core
will do it.
Remove .owner field if calls are used which set it automatically
Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Signed-off-by: Yang Li <yang.lee@linux.alibaba.com>
---
Change in v2:
-use imx-hdmi instead of hdmi-codec for Subject
sound/soc/fsl/imx-hdmi.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/sound/soc/fsl/imx-hdmi.c b/sound/soc/fsl/imx-hdmi.c
index dbbb761..cd0235a 100644
--- a/sound/soc/fsl/imx-hdmi.c
+++ b/sound/soc/fsl/imx-hdmi.c
@@ -223,7 +223,6 @@ static int imx_hdmi_probe(struct platform_device *pdev)
static struct platform_driver imx_hdmi_driver = {
.driver = {
.name = "imx-hdmi",
- .owner = THIS_MODULE,
.pm = &snd_soc_pm_ops,
.of_match_table = imx_hdmi_dt_ids,
},
--
1.8.3.1
^ permalink raw reply related
* [PATCH] scsi: ibmvfc: Switch to using the new API kobj_to_dev()
From: Jiapeng Chong @ 2021-03-04 9:28 UTC (permalink / raw)
To: tyreld
Cc: Jiapeng Chong, martin.petersen, linux-scsi, jejb, linux-kernel,
paulus, linuxppc-dev
Fix the following coccicheck warnings:
./drivers/scsi/ibmvscsi/ibmvfc.c:3483:60-61: WARNING opportunity for
kobj_to_dev().
Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 755313b..e5f1ca7 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -3480,7 +3480,7 @@ static ssize_t ibmvfc_read_trace(struct file *filp, struct kobject *kobj,
struct bin_attribute *bin_attr,
char *buf, loff_t off, size_t count)
{
- struct device *dev = container_of(kobj, struct device, kobj);
+ struct device *dev = kobj_to_dev(kobj);
struct Scsi_Host *shost = class_to_shost(dev);
struct ibmvfc_host *vhost = shost_priv(shost);
unsigned long flags = 0;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v3] powerpc/uprobes: Validation for prefixed instruction
From: Ravi Bangoria @ 2021-03-04 10:13 UTC (permalink / raw)
To: Christophe Leroy
Cc: Ravi Bangoria, jniethe5, oleg, rostedt, linux-kernel, paulus,
sandipan, naveen.n.rao, linuxppc-dev
In-Reply-To: <ac7aa126-59dd-31be-1084-6d3a2f0e4eb4@csgroup.eu>
On 3/4/21 1:02 PM, Christophe Leroy wrote:
>
>
> Le 04/03/2021 à 06:05, Ravi Bangoria a écrit :
>> As per ISA 3.1, prefixed instruction should not cross 64-byte
>> boundary. So don't allow Uprobe on such prefixed instruction.
>>
>> There are two ways probed instruction is changed in mapped pages.
>> First, when Uprobe is activated, it searches for all the relevant
>> pages and replace instruction in them. In this case, if that probe
>> is on the 64-byte unaligned prefixed instruction, error out
>> directly. Second, when Uprobe is already active and user maps a
>> relevant page via mmap(), instruction is replaced via mmap() code
>> path. But because Uprobe is invalid, entire mmap() operation can
>> not be stopped. In this case just print an error and continue.
>>
>> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
>> ---
>> v2: https://lore.kernel.org/r/20210204104703.273429-1-ravi.bangoria@linux.ibm.com
>> v2->v3:
>> - Drop restriction for Uprobe on suffix of prefixed instruction.
>> It needs lot of code change including generic code but what
>> we get in return is not worth it.
>>
>> arch/powerpc/kernel/uprobes.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
>> index e8a63713e655..c400971ebe70 100644
>> --- a/arch/powerpc/kernel/uprobes.c
>> +++ b/arch/powerpc/kernel/uprobes.c
>> @@ -41,6 +41,14 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe,
>> if (addr & 0x03)
>> return -EINVAL;
>> + if (!IS_ENABLED(CONFIG_PPC64) || !cpu_has_feature(CPU_FTR_ARCH_31))
>
> cpu_has_feature(CPU_FTR_ARCH_31) should return 'false' when CONFIG_PPC64 is not enabled, no need to double check.
Ok.
I'm going to drop CONFIG_PPC64 check because it's not really
required as I replied to Naveen. So, I'll keep CPU_FTR_ARCH_31
check as is.
>
>> + return 0;
>> +
>> + if (ppc_inst_prefixed(auprobe->insn) && (addr & 0x3F) == 0x3C) {
>
> Maybe 3C instead of 4F ? : (addr & 0x3C) == 0x3C
Didn't follow. It's not (addr & 0x3C), it's (addr & 0x3F).
>
> What about
>
> (addr & (SZ_64 - 4)) == SZ_64 - 4 to make it more explicit ?
Yes this is bit better. Though, it should be:
(addr & (SZ_64 - 1)) == SZ_64 - 4
Ravi
^ permalink raw reply
* [Bug 210749] sysfs: cannot create duplicate filename '/bus/nvmem/devices/module-vpd'
From: bugzilla-daemon @ 2021-03-04 10:31 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-210749-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=210749
Michael Ellerman (michael@ellerman.id.au) changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |michael@ellerman.id.au
--- Comment #6 from Michael Ellerman (michael@ellerman.id.au) ---
Looks like: 61f764c307f6 ("eeprom: at24: Support custom device names for AT24
EEPROMs")
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [PATCH v3] powerpc/uprobes: Validation for prefixed instruction
From: Christophe Leroy @ 2021-03-04 10:51 UTC (permalink / raw)
To: Ravi Bangoria
Cc: jniethe5, oleg, rostedt, linux-kernel, paulus, sandipan,
naveen.n.rao, linuxppc-dev
In-Reply-To: <4d365b9f-6f25-a4bc-c145-c06ee33f1f9f@linux.ibm.com>
Le 04/03/2021 à 11:13, Ravi Bangoria a écrit :
>
>
> On 3/4/21 1:02 PM, Christophe Leroy wrote:
>>
>>
>> Le 04/03/2021 à 06:05, Ravi Bangoria a écrit :
>>> As per ISA 3.1, prefixed instruction should not cross 64-byte
>>> boundary. So don't allow Uprobe on such prefixed instruction.
>>>
>>> There are two ways probed instruction is changed in mapped pages.
>>> First, when Uprobe is activated, it searches for all the relevant
>>> pages and replace instruction in them. In this case, if that probe
>>> is on the 64-byte unaligned prefixed instruction, error out
>>> directly. Second, when Uprobe is already active and user maps a
>>> relevant page via mmap(), instruction is replaced via mmap() code
>>> path. But because Uprobe is invalid, entire mmap() operation can
>>> not be stopped. In this case just print an error and continue.
>>>
>>> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
>>> ---
>>> v2: https://lore.kernel.org/r/20210204104703.273429-1-ravi.bangoria@linux.ibm.com
>>> v2->v3:
>>> - Drop restriction for Uprobe on suffix of prefixed instruction.
>>> It needs lot of code change including generic code but what
>>> we get in return is not worth it.
>>>
>>> arch/powerpc/kernel/uprobes.c | 8 ++++++++
>>> 1 file changed, 8 insertions(+)
>>>
>>> diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
>>> index e8a63713e655..c400971ebe70 100644
>>> --- a/arch/powerpc/kernel/uprobes.c
>>> +++ b/arch/powerpc/kernel/uprobes.c
>>> @@ -41,6 +41,14 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe,
>>> if (addr & 0x03)
>>> return -EINVAL;
>>> + if (!IS_ENABLED(CONFIG_PPC64) || !cpu_has_feature(CPU_FTR_ARCH_31))
>>
>> cpu_has_feature(CPU_FTR_ARCH_31) should return 'false' when CONFIG_PPC64 is not enabled, no need
>> to double check.
>
> Ok.
>
> I'm going to drop CONFIG_PPC64 check because it's not really
> required as I replied to Naveen. So, I'll keep CPU_FTR_ARCH_31
> check as is.
>
>>
>>> + return 0;
>>> +
>>> + if (ppc_inst_prefixed(auprobe->insn) && (addr & 0x3F) == 0x3C) {
>>
>> Maybe 3C instead of 4F ? : (addr & 0x3C) == 0x3C
>
> Didn't follow. It's not (addr & 0x3C), it's (addr & 0x3F).
Sorry I meant 3c instead of 3f (And usually we don't use capital letters for that).
The last two bits are supposed to always be 0, so it doesn't really matter, I just thought it would
look better having the same value both sides of the test, ie (addr & 0x3c) == 0x3c.
>
>>
>> What about
>>
>> (addr & (SZ_64 - 4)) == SZ_64 - 4 to make it more explicit ?
>
> Yes this is bit better. Though, it should be:
>
> (addr & (SZ_64 - 1)) == SZ_64 - 4
-1 or -4 should give the same results as instructions are always 32 bits aligned though.
Christophe
^ permalink raw reply
* Re: [PATCH v2 28/37] KVM: PPC: Book3S HV P9: Add helpers for OS SPR handling
From: Nicholas Piggin @ 2021-03-04 11:02 UTC (permalink / raw)
To: Fabiano Rosas, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <87pn0hwq9f.fsf@linux.ibm.com>
Excerpts from Fabiano Rosas's message of March 3, 2021 1:04 am:
> Nicholas Piggin <npiggin@gmail.com> writes:
>
>> This is a first step to wrapping supervisor and user SPR saving and
>> loading up into helpers, which will then be called independently in
>> bare metal and nested HV cases in order to optimise SPR access.
>>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>
> <snip>
>
>> +/* vcpu guest regs must already be saved */
>> +static void restore_p9_host_os_sprs(struct kvm_vcpu *vcpu,
>> + struct p9_host_os_sprs *host_os_sprs)
>> +{
>> + mtspr(SPRN_PSPB, 0);
>> + mtspr(SPRN_WORT, 0);
>> + mtspr(SPRN_UAMOR, 0);
>> + mtspr(SPRN_PSPB, 0);
>
> Not your fault, but PSPB is set twice here.
Yeah you're right.
>> +
>> + mtspr(SPRN_DSCR, host_os_sprs->dscr);
>> + mtspr(SPRN_TIDR, host_os_sprs->tidr);
>> + mtspr(SPRN_IAMR, host_os_sprs->iamr);
>> +
>> + if (host_os_sprs->amr != vcpu->arch.amr)
>> + mtspr(SPRN_AMR, host_os_sprs->amr);
>> +
>> + if (host_os_sprs->fscr != vcpu->arch.fscr)
>> + mtspr(SPRN_FSCR, host_os_sprs->fscr);
>> +}
>> +
>
> <snip>
>
>> @@ -3605,34 +3666,10 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
>> vcpu->arch.dec_expires = dec + tb;
>> vcpu->cpu = -1;
>> vcpu->arch.thread_cpu = -1;
>> - vcpu->arch.ctrl = mfspr(SPRN_CTRLF);
>> -
>> - vcpu->arch.iamr = mfspr(SPRN_IAMR);
>> - vcpu->arch.pspb = mfspr(SPRN_PSPB);
>> - vcpu->arch.fscr = mfspr(SPRN_FSCR);
>> - vcpu->arch.tar = mfspr(SPRN_TAR);
>> - vcpu->arch.ebbhr = mfspr(SPRN_EBBHR);
>> - vcpu->arch.ebbrr = mfspr(SPRN_EBBRR);
>> - vcpu->arch.bescr = mfspr(SPRN_BESCR);
>> - vcpu->arch.wort = mfspr(SPRN_WORT);
>> - vcpu->arch.tid = mfspr(SPRN_TIDR);
>> - vcpu->arch.amr = mfspr(SPRN_AMR);
>> - vcpu->arch.uamor = mfspr(SPRN_UAMOR);
>> - vcpu->arch.dscr = mfspr(SPRN_DSCR);
>> -
>> - mtspr(SPRN_PSPB, 0);
>> - mtspr(SPRN_WORT, 0);
>> - mtspr(SPRN_UAMOR, 0);
>> - mtspr(SPRN_DSCR, host_dscr);
>> - mtspr(SPRN_TIDR, host_tidr);
>> - mtspr(SPRN_IAMR, host_iamr);
>> - mtspr(SPRN_PSPB, 0);
>>
>> - if (host_amr != vcpu->arch.amr)
>> - mtspr(SPRN_AMR, host_amr);
>> + restore_p9_host_os_sprs(vcpu, &host_os_sprs);
>>
>> - if (host_fscr != vcpu->arch.fscr)
>> - mtspr(SPRN_FSCR, host_fscr);
>> + store_spr_state(vcpu);
>
> store_spr_state should come first, right? We want to save the guest
> state before restoring the host state.
Yes good catch. I switched that back around later but looks like I
never brought the fix back to the right patch. Interestingly, things
pretty much work like this if the guest or host doesn't do anything
much with the SPRs!
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v3] powerpc/uprobes: Validation for prefixed instruction
From: Ravi Bangoria @ 2021-03-04 11:02 UTC (permalink / raw)
To: Christophe Leroy
Cc: Ravi Bangoria, jniethe5, oleg, rostedt, linux-kernel, paulus,
sandipan, naveen.n.rao, linuxppc-dev
In-Reply-To: <f1a61cd9-436e-b486-b99c-0c06f2956a89@csgroup.eu>
On 3/4/21 4:21 PM, Christophe Leroy wrote:
>
>
> Le 04/03/2021 à 11:13, Ravi Bangoria a écrit :
>>
>>
>> On 3/4/21 1:02 PM, Christophe Leroy wrote:
>>>
>>>
>>> Le 04/03/2021 à 06:05, Ravi Bangoria a écrit :
>>>> As per ISA 3.1, prefixed instruction should not cross 64-byte
>>>> boundary. So don't allow Uprobe on such prefixed instruction.
>>>>
>>>> There are two ways probed instruction is changed in mapped pages.
>>>> First, when Uprobe is activated, it searches for all the relevant
>>>> pages and replace instruction in them. In this case, if that probe
>>>> is on the 64-byte unaligned prefixed instruction, error out
>>>> directly. Second, when Uprobe is already active and user maps a
>>>> relevant page via mmap(), instruction is replaced via mmap() code
>>>> path. But because Uprobe is invalid, entire mmap() operation can
>>>> not be stopped. In this case just print an error and continue.
>>>>
>>>> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
>>>> ---
>>>> v2: https://lore.kernel.org/r/20210204104703.273429-1-ravi.bangoria@linux.ibm.com
>>>> v2->v3:
>>>> - Drop restriction for Uprobe on suffix of prefixed instruction.
>>>> It needs lot of code change including generic code but what
>>>> we get in return is not worth it.
>>>>
>>>> arch/powerpc/kernel/uprobes.c | 8 ++++++++
>>>> 1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
>>>> index e8a63713e655..c400971ebe70 100644
>>>> --- a/arch/powerpc/kernel/uprobes.c
>>>> +++ b/arch/powerpc/kernel/uprobes.c
>>>> @@ -41,6 +41,14 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe,
>>>> if (addr & 0x03)
>>>> return -EINVAL;
>>>> + if (!IS_ENABLED(CONFIG_PPC64) || !cpu_has_feature(CPU_FTR_ARCH_31))
>>>
>>> cpu_has_feature(CPU_FTR_ARCH_31) should return 'false' when CONFIG_PPC64 is not enabled, no need to double check.
>>
>> Ok.
>>
>> I'm going to drop CONFIG_PPC64 check because it's not really
>> required as I replied to Naveen. So, I'll keep CPU_FTR_ARCH_31
>> check as is.
>>
>>>
>>>> + return 0;
>>>> +
>>>> + if (ppc_inst_prefixed(auprobe->insn) && (addr & 0x3F) == 0x3C) {
>>>
>>> Maybe 3C instead of 4F ? : (addr & 0x3C) == 0x3C
>>
>> Didn't follow. It's not (addr & 0x3C), it's (addr & 0x3F).
>
> Sorry I meant 3c instead of 3f (And usually we don't use capital letters for that).
> The last two bits are supposed to always be 0, so it doesn't really matter, I just thought it would look better having the same value both sides of the test, ie (addr & 0x3c) == 0x3c.
Ok yeah makes sense. Thanks.
>
>>
>>>
>>> What about
>>>
>>> (addr & (SZ_64 - 4)) == SZ_64 - 4 to make it more explicit ?
>>
>> Yes this is bit better. Though, it should be:
>>
>> (addr & (SZ_64 - 1)) == SZ_64 - 4
>
> -1 or -4 should give the same results as instructions are always 32 bits aligned though.
Got it.
Ravi
^ permalink raw reply
* Re: [PATCH v2 30/37] KVM: PPC: Book3S HV: Implement radix prefetch workaround by disabling MMU
From: Nicholas Piggin @ 2021-03-04 11:04 UTC (permalink / raw)
To: Fabiano Rosas, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <87lfb5w8t2.fsf@linux.ibm.com>
Excerpts from Fabiano Rosas's message of March 3, 2021 7:21 am:
> Nicholas Piggin <npiggin@gmail.com> writes:
>
>> Rather than partition the guest PID space and catch and flush a rogue
>> guest, instead work around this issue by ensuring the MMU is always
>> disabled in HV mode while the guest MMU context is switched in.
>>
>> This may be a bit less efficient, but it is a lot less complicated and
>> allows the P9 path to trivally implement the workaround too. Newer CPUs
>> are not subject to this issue.
>>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>> arch/powerpc/include/asm/mmu_context.h | 6 ----
>> arch/powerpc/kvm/book3s_hv.c | 10 ++++--
>> arch/powerpc/kvm/book3s_hv_interrupt.c | 14 ++++++--
>> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 34 ------------------
>> arch/powerpc/mm/book3s64/radix_pgtable.c | 27 +++++---------
>> arch/powerpc/mm/book3s64/radix_tlb.c | 46 ------------------------
>> arch/powerpc/mm/mmu_context.c | 4 +--
>> 7 files changed, 28 insertions(+), 113 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
>> index 652ce85f9410..bb5c7e5e142e 100644
>> --- a/arch/powerpc/include/asm/mmu_context.h
>> +++ b/arch/powerpc/include/asm/mmu_context.h
>> @@ -122,12 +122,6 @@ static inline bool need_extra_context(struct mm_struct *mm, unsigned long ea)
>> }
>> #endif
>>
>> -#if defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE) && defined(CONFIG_PPC_RADIX_MMU)
>> -extern void radix_kvm_prefetch_workaround(struct mm_struct *mm);
>> -#else
>> -static inline void radix_kvm_prefetch_workaround(struct mm_struct *mm) { }
>> -#endif
>> -
>> extern void switch_cop(struct mm_struct *next);
>> extern int use_cop(unsigned long acop, struct mm_struct *mm);
>> extern void drop_cop(unsigned long acop, struct mm_struct *mm);
>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>> index ad16331c3370..c3064075f1d7 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -806,6 +806,10 @@ static int kvmppc_h_set_mode(struct kvm_vcpu *vcpu, unsigned long mflags,
>> /* KVM does not support mflags=2 (AIL=2) */
>> if (mflags != 0 && mflags != 3)
>> return H_UNSUPPORTED_FLAG_START;
>> + /* Prefetch bug */
>> + if (cpu_has_feature(CPU_FTR_P9_RADIX_PREFETCH_BUG) &&
>> + kvmhv_vcpu_is_radix(vcpu) && mflags == 3)
>> + return H_UNSUPPORTED_FLAG_START;
>
> So does this mean that if the host has the prefetch bug, all of its
> guests will run with AIL=0 all the time?
All radix guests will, yes.
> And what we're avoiding here is
> a guest setting AIL=3 which would (since there's no HAIL) cause
> hypervisor interrupts to be taken with MMU on, is that it?
Yes that's right.
> Do we need to add this verification to kvmppc_set_lpcr as well? QEMU
> could in theory call the KVM_SET_ONE_REG ioctl and set AIL to any value.
Yeah I guess so. We don't restrict other AIL values there by the looks
but maybe we should.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v2 34/37] KVM: PPC: Book3S HV: add virtual mode handlers for HPT hcalls and page faults
From: Nicholas Piggin @ 2021-03-04 11:05 UTC (permalink / raw)
To: Fabiano Rosas, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <87im68vw16.fsf@linux.ibm.com>
Excerpts from Fabiano Rosas's message of March 4, 2021 6:09 am:
> Nicholas Piggin <npiggin@gmail.com> writes:
>
>> In order to support hash guests in the P9 path (which does not do real
>> mode hcalls or page fault handling), these real-mode hash specific
>> interrupts need to be implemented in virt mode.
>>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>> arch/powerpc/kvm/book3s_hv.c | 118 +++++++++++++++++++++++++++++++++--
>> 1 file changed, 113 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>> index 9d2fa21201c1..1bbc46f2cfbf 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -935,6 +935,52 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
>> return RESUME_HOST;
>>
>> switch (req) {
>> + case H_REMOVE:
>> + ret = kvmppc_h_remove(vcpu, kvmppc_get_gpr(vcpu, 4),
>> + kvmppc_get_gpr(vcpu, 5),
>> + kvmppc_get_gpr(vcpu, 6));
>> + if (ret == H_TOO_HARD)
>> + return RESUME_HOST;
>> + break;
>> + case H_ENTER:
>> + ret = kvmppc_h_enter(vcpu, kvmppc_get_gpr(vcpu, 4),
>> + kvmppc_get_gpr(vcpu, 5),
>> + kvmppc_get_gpr(vcpu, 6),
>> + kvmppc_get_gpr(vcpu, 7));
>> + if (ret == H_TOO_HARD)
>> + return RESUME_HOST;
>> + break;
>> + case H_READ:
>> + ret = kvmppc_h_read(vcpu, kvmppc_get_gpr(vcpu, 4),
>> + kvmppc_get_gpr(vcpu, 5));
>> + if (ret == H_TOO_HARD)
>> + return RESUME_HOST;
>> + break;
>> + case H_CLEAR_MOD:
>> + ret = kvmppc_h_clear_mod(vcpu, kvmppc_get_gpr(vcpu, 4),
>> + kvmppc_get_gpr(vcpu, 5));
>> + if (ret == H_TOO_HARD)
>> + return RESUME_HOST;
>> + break;
>> + case H_CLEAR_REF:
>> + ret = kvmppc_h_clear_ref(vcpu, kvmppc_get_gpr(vcpu, 4),
>> + kvmppc_get_gpr(vcpu, 5));
>> + if (ret == H_TOO_HARD)
>> + return RESUME_HOST;
>> + break;
>> + case H_PROTECT:
>> + ret = kvmppc_h_protect(vcpu, kvmppc_get_gpr(vcpu, 4),
>> + kvmppc_get_gpr(vcpu, 5),
>> + kvmppc_get_gpr(vcpu, 6));
>> + if (ret == H_TOO_HARD)
>> + return RESUME_HOST;
>> + break;
>> + case H_BULK_REMOVE:
>> + ret = kvmppc_h_bulk_remove(vcpu);
>> + if (ret == H_TOO_HARD)
>> + return RESUME_HOST;
>> + break;
>> +
>
> Some of these symbols need to be exported.
>
> ERROR: modpost: "kvmppc_h_bulk_remove" [arch/powerpc/kvm/kvm-hv.ko] undefined!
> ERROR: modpost: "kvmppc_h_clear_mod" [arch/powerpc/kvm/kvm-hv.ko] undefined!
> ERROR: modpost: "kvmppc_xive_xics_hcall" [arch/powerpc/kvm/kvm-hv.ko] undefined!
> ERROR: modpost: "kvmppc_h_remove" [arch/powerpc/kvm/kvm-hv.ko] undefined!
> ERROR: modpost: "decrementers_next_tb" [arch/powerpc/kvm/kvm-hv.ko] undefined!
> ERROR: modpost: "kvmppc_hpte_hv_fault" [arch/powerpc/kvm/kvm-hv.ko] undefined!
> ERROR: modpost: "kvmppc_h_protect" [arch/powerpc/kvm/kvm-hv.ko] undefined!
> ERROR: modpost: "kvmppc_h_enter" [arch/powerpc/kvm/kvm-hv.ko] undefined!
> ERROR: modpost: "kvmppc_h_clear_ref" [arch/powerpc/kvm/kvm-hv.ko] undefined!
> ERROR: modpost: "kvmppc_h_read" [arch/powerpc/kvm/kvm-hv.ko] undefined!
Yeah sorry about that there's a few issues there, I'll try polish that
up a bit before the next post.
Thanks,
Nick
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox