LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH V5 21/21] KVM: compat: riscv: Prevent KVM_COMPAT from being selected
From: Paolo Bonzini @ 2022-02-01 16:00 UTC (permalink / raw)
  To: Anup Patel, Guo Ren
  Cc: Guo Ren, KVM General, linux-mips, sparclinux, linux-riscv,
	Christoph Hellwig, linux-arch, linux-s390, x86, linux-csky,
	Arnd Bergmann, linux-arm-kernel, linux-parisc, Drew Fustini,
	Greg Kroah-Hartman, Wang Junqiang,
	linux-kernel@vger.kernel.org List, Palmer Dabbelt, kvm-riscv,
	liush, linuxppc-dev, Wei Fu
In-Reply-To: <CAAhSdy27nVvh9F08kPgffJe-Y-gOOc9cnQtCLFAE0GbDhHVbiQ@mail.gmail.com>

On 2/1/22 16:44, Anup Patel wrote:
> +Paolo
> 
> On Tue, Feb 1, 2022 at 8:38 PM <guoren@kernel.org> wrote:
>>
>> From: Guo Ren <guoren@linux.alibaba.com>
>>
>> Current riscv doesn't support the 32bit KVM API. Let's make it
>> clear by not selecting KVM_COMPAT.
>>
>> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
>> Signed-off-by: Guo Ren <guoren@kernel.org>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Cc: Anup Patel <anup@brainfault.org>
> 
> This looks good to me.
> 
> Reviewed-by: Anup Patel <anup@brainfault.org>

Hi Anup,

feel free to send this via a pull request (perhaps together with Mark 
Rutland's entry/exit rework).

Paolo


^ permalink raw reply

* Re: [PATCH V5 21/21] KVM: compat: riscv: Prevent KVM_COMPAT from being selected
From: Anup Patel @ 2022-02-01 16:11 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Guo Ren, KVM General, linux-mips, Guo Ren, sparclinux,
	linux-riscv, Christoph Hellwig, linux-arch, linux-s390, x86,
	linux-csky, Arnd Bergmann, linux-arm-kernel, linux-parisc,
	Drew Fustini, liush, Wang Junqiang,
	linux-kernel@vger.kernel.org List, Palmer Dabbelt, kvm-riscv,
	Greg Kroah-Hartman, linuxppc-dev, Wei Fu
In-Reply-To: <f8359e15-412a-03d6-1b0c-a9f253816497@redhat.com>

On Tue, Feb 1, 2022 at 9:31 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 2/1/22 16:44, Anup Patel wrote:
> > +Paolo
> >
> > On Tue, Feb 1, 2022 at 8:38 PM <guoren@kernel.org> wrote:
> >>
> >> From: Guo Ren <guoren@linux.alibaba.com>
> >>
> >> Current riscv doesn't support the 32bit KVM API. Let's make it
> >> clear by not selecting KVM_COMPAT.
> >>
> >> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> >> Signed-off-by: Guo Ren <guoren@kernel.org>
> >> Cc: Arnd Bergmann <arnd@arndb.de>
> >> Cc: Anup Patel <anup@brainfault.org>
> >
> > This looks good to me.
> >
> > Reviewed-by: Anup Patel <anup@brainfault.org>
>
> Hi Anup,
>
> feel free to send this via a pull request (perhaps together with Mark
> Rutland's entry/exit rework).

Sure, I will do like you suggested.

Regards,
Anup

>
> Paolo
>

^ permalink raw reply

* Re: [PATCH 2/3] powerpc/pseries/vas: Add VAS migration handler
From: Nathan Lynch @ 2022-02-01 16:12 UTC (permalink / raw)
  To: Haren Myneni, mpe, linuxppc-dev, npiggin
In-Reply-To: <17bb8ed6bfc450ec89bd12169d43858a2ba33bca.camel@linux.ibm.com>

Haren Myneni <haren@linux.ibm.com> writes:
> On Mon, 2022-01-31 at 10:37 -0600, Nathan Lynch wrote:
>> Haren Myneni <haren@linux.ibm.com> writes:
>> > Since the VAS windows belong to the VAS hardware resource, the
>> > hypervisor expects the partition to close them on source partition
>> > and reopen them after the partition migrated on the destination
>> > machine.
>> 
>> Not clear to me what "expects" really means here. Would it be
>> accurate to say "requires" instead? If the OS fails to close the
>> windows before suspend, what happens?
>
> I will change it to 'requires' - These VAS windows have to be closed
> before migration so that these windows / credits will be available to
> other LPARs on the source machine.

I'll rephrase.

I want to know whether the architecture (PAPR or whichever doc) imposes
a requirement on the OS (i.e. using words like "must") to close the
windows, or does it merely make a recommendation ("should"), or is it
silent on the subject?

In your testing, if Linux has windows open at the time of calling
ibm,suspend-me, does the call fail or succeed?

^ permalink raw reply

* Re: [PATCH V5 21/21] KVM: compat: riscv: Prevent KVM_COMPAT from being selected
From: Guo Ren @ 2022-02-01 16:16 UTC (permalink / raw)
  To: Anup Patel
  Cc: Guo Ren, KVM General, open list:BROADCOM NVRAM DRIVER, sparclinux,
	linux-riscv, Christoph Hellwig, linux-arch, linux-s390,
	the arch/x86 maintainers, linux-csky, Arnd Bergmann,
	linux-arm-kernel, Parisc List, Drew Fustini, liush, Wang Junqiang,
	linux-kernel@vger.kernel.org List, Palmer Dabbelt, kvm-riscv,
	Greg Kroah-Hartman, Paolo Bonzini, linuxppc-dev, Wei Fu
In-Reply-To: <CAAhSdy0U+41OWG_0C=820U+07accLsHxNYENtp=ZZsy6K4mJ0g@mail.gmail.com>

On Wed, Feb 2, 2022 at 12:11 AM Anup Patel <anup@brainfault.org> wrote:
>
> On Tue, Feb 1, 2022 at 9:31 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> >
> > On 2/1/22 16:44, Anup Patel wrote:
> > > +Paolo
> > >
> > > On Tue, Feb 1, 2022 at 8:38 PM <guoren@kernel.org> wrote:
> > >>
> > >> From: Guo Ren <guoren@linux.alibaba.com>
> > >>
> > >> Current riscv doesn't support the 32bit KVM API. Let's make it
> > >> clear by not selecting KVM_COMPAT.
> > >>
> > >> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > >> Signed-off-by: Guo Ren <guoren@kernel.org>
> > >> Cc: Arnd Bergmann <arnd@arndb.de>
> > >> Cc: Anup Patel <anup@brainfault.org>
> > >
> > > This looks good to me.
> > >
> > > Reviewed-by: Anup Patel <anup@brainfault.org>
> >
> > Hi Anup,
> >
> > feel free to send this via a pull request (perhaps together with Mark
> > Rutland's entry/exit rework).
>
> Sure, I will do like you suggested.
Great, thx.

>
> Regards,
> Anup
>
> >
> > Paolo
> >



-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

^ permalink raw reply

* Re: [PATCH v7 0/5] Allow guest access to EFI confidential computing secret area
From: Dr. David Alan Gilbert @ 2022-02-01 18:07 UTC (permalink / raw)
  To: James Bottomley
  Cc: linux-efi, Brijesh Singh, mjg59, Lenny Szubowicz, Gerd Hoffmann,
	gcwilson, Ard Biesheuvel, Daniele Buono, Andi Kleen, Nayna Jain,
	James Morris, Dov Murik, Jim Cadden, Peter Gonda, Borislav Petkov,
	Serge E. Hallyn, Tom Lendacky, Ashish Kalra, dougmill,
	Tobin Feldman-Fitzthum, linux-coco, gjoyce, dja, Dave Hansen,
	Greg KH, linux-kernel, linux-security-module, linuxppc-dev,
	Andrew Scull
In-Reply-To: <37779659ca96ac9c1f11bcc0ac0665895c795b54.camel@linux.ibm.com>

* James Bottomley (jejb@linux.ibm.com) wrote:
> [cc's added]
> On Tue, 2022-02-01 at 14:50 +0100, Greg KH wrote:
> > On Tue, Feb 01, 2022 at 12:44:08PM +0000, Dov Murik wrote:
> [...]
> > > # ls -la /sys/kernel/security/coco/efi_secret
> > > total 0
> > > drwxr-xr-x 2 root root 0 Jun 28 11:55 .
> > > drwxr-xr-x 3 root root 0 Jun 28 11:54 ..
> > > -r--r----- 1 root root 0 Jun 28 11:54 736870e5-84f0-4973-92ec-
> > > 06879ce3da0b
> > > -r--r----- 1 root root 0 Jun 28 11:54 83c83f7f-1356-4975-8b7e-
> > > d3a0b54312c6
> > > -r--r----- 1 root root 0 Jun 28 11:54 9553f55d-3da2-43ee-ab5d-
> > > ff17f78864d2
> > 
> > Please see my comments on the powerpc version of this type of thing:
> > 	
> > https://lore.kernel.org/r/20220122005637.28199-1-nayna@linux.ibm.com
> 
> If you want a debate, actually cc'ing the people on the other thread
> would have been a good start ...
> 
> For those added, this patch series is at:
> 
> https://lore.kernel.org/all/20220201124413.1093099-1-dovmurik@linux.ibm.com/
> 
> > You all need to work together to come up with a unified place for
> > this and stop making it platform-specific.
> 
> I'm not entirely sure of that.  If you look at the differences between
> EFI variables and the COCO proposal: the former has an update API
> which, in the case of signed variables, is rather complex and a UC16
> content requirement.  The latter is binary data with read only/delete. 
> Plus each variable in EFI is described by a GUID, so having a directory
> of random guids, some of which behave like COCO secrets and some of
> which are EFI variables is going to be incredibly confusing (and also
> break all our current listing tools which seems somewhat undesirable).
> 
> So we could end up with 
> 
> <common path prefix>/efivar
> <common path prefix>/coco
> 
> To achieve the separation, but I really don't see what this buys us. 
> Both filesystems would likely end up with different backends because of
> the semantic differences and we can easily start now in different
> places (effectively we've already done this for efi variables) and
> unify later if that is the chosen direction, so it doesn't look like a
> blocker.
> 
> > Until then, we can't take this.
> 
> I don't believe anyone was asking you to take it.

I have some sympathy in wanting some unification; (I'm not sure that
list of comparison even includes the TDX world).
But I'm not sure if they're the same thing - these are strictly
constants, they're not changable.

But it is a messy list of differences - especially things like the
UTF-16 stuff
I guess the PowerVM key naming contains nul and / can be ignored
- if anyone is silly enough to create keys with those names then they
can not access them; so at least that would solve that problem.

I don't really understand the talk of 32bit attributes in either the
uEFI or PowerVM key store case.

Is that GOOGLE_SMI stuff already there? If so I guess there's not much
we can do  - but it's a shame that there's the directory per variable.

Dave



> James
> 
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


^ permalink raw reply

* [PATCH] i2c: pasemi: Drop I2C classes from platform driver variant
From: Martin Povišer @ 2022-02-01 21:55 UTC (permalink / raw)
  To: Hector Martin, Sven Peter, Wolfram Sang, Michael Ellerman
  Cc: Martin Povišer, linux-kernel, Paul Mackerras, linux-i2c,
	linuxppc-dev, Alyssa Rosenzweig

Drop I2C device-probing classes from platform variant of the PASemi
controller as it is only used on platforms where I2C devices should
be instantiated in devicetree. (The I2C_CLASS_DEPRECATED flag is not
raised as up to this point no devices relied on the old behavior.)

Fixes: d88ae2932df0 ("i2c: pasemi: Add Apple platform driver")
Signed-off-by: Martin Povišer <povik+lin@protonmail.com>
---
 drivers/i2c/busses/i2c-pasemi-core.c | 1 -
 drivers/i2c/busses/i2c-pasemi-pci.c  | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-pasemi-core.c b/drivers/i2c/busses/i2c-pasemi-core.c
index 4e161a4089d8..7728c8460dc0 100644
--- a/drivers/i2c/busses/i2c-pasemi-core.c
+++ b/drivers/i2c/busses/i2c-pasemi-core.c
@@ -333,7 +333,6 @@ int pasemi_i2c_common_probe(struct pasemi_smbus *smbus)
 	smbus->adapter.owner = THIS_MODULE;
 	snprintf(smbus->adapter.name, sizeof(smbus->adapter.name),
 		 "PA Semi SMBus adapter (%s)", dev_name(smbus->dev));
-	smbus->adapter.class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
 	smbus->adapter.algo = &smbus_algorithm;
 	smbus->adapter.algo_data = smbus;

diff --git a/drivers/i2c/busses/i2c-pasemi-pci.c b/drivers/i2c/busses/i2c-pasemi-pci.c
index 1ab1f28744fb..cfc89e04eb94 100644
--- a/drivers/i2c/busses/i2c-pasemi-pci.c
+++ b/drivers/i2c/busses/i2c-pasemi-pci.c
@@ -56,6 +56,7 @@ static int pasemi_smb_pci_probe(struct pci_dev *dev,
 	if (!smbus->ioaddr)
 		return -EBUSY;

+	smbus->adapter.class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
 	error = pasemi_i2c_common_probe(smbus);
 	if (error)
 		return error;
--
2.33.0



^ permalink raw reply related

* [PATCH] spi: mpc512x-psc: Fix compile errors
From: Linus Walleij @ 2022-02-01 23:45 UTC (permalink / raw)
  To: Mark Brown, linux-spi
  Cc: Linus Walleij, Anatolij Gustschin, linuxppc-dev,
	kernel test robot, Uwe Kleine-König

My patch created compilation bugs in the MPC512x-PSC driver.
Fix them up.

Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Cc: Anatolij Gustschin <agust@denx.de>
Cc: linuxppc-dev@lists.ozlabs.org
Reported-by: kernel test robot <lkp@intel.com>
Fixes: 2818824ced4b (" spi: mpc512x-psc: Convert to use GPIO descriptors")
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
(This is because I don't have a PPC cross compiler.)
---
 drivers/spi/spi-mpc512x-psc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/spi/spi-mpc512x-psc.c b/drivers/spi/spi-mpc512x-psc.c
index 8a488d8e4c1b..03630359ce70 100644
--- a/drivers/spi/spi-mpc512x-psc.c
+++ b/drivers/spi/spi-mpc512x-psc.c
@@ -127,7 +127,7 @@ static void mpc512x_psc_spi_activate_cs(struct spi_device *spi)
 	out_be32(psc_addr(mps, ccr), ccr);
 	mps->bits_per_word = cs->bits_per_word;
 
-	if (cs->gpiod) {
+	if (spi->cs_gpiod) {
 		if (mps->cs_control)
 			/* boardfile override */
 			mps->cs_control(spi, (spi->mode & SPI_CS_HIGH) ? 1 : 0);
@@ -373,7 +373,6 @@ static int mpc512x_psc_spi_unprep_xfer_hw(struct spi_master *master)
 static int mpc512x_psc_spi_setup(struct spi_device *spi)
 {
 	struct mpc512x_psc_spi_cs *cs = spi->controller_state;
-	int ret;
 
 	if (spi->bits_per_word % 8)
 		return -EINVAL;
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH V5 15/21] riscv: compat: Add hw capability check for elf
From: Guo Ren @ 2022-02-02  2:02 UTC (permalink / raw)
  To: Guo Ren, Palmer Dabbelt, Arnd Bergmann, Anup Patel,
	Greg Kroah-Hartman, liush, Wei Fu, Drew Fustini, Wang Junqiang,
	Christoph Hellwig
  Cc: linux-arch, linux-s390, Guo Ren, Parisc List,
	the arch/x86 maintainers, Linux Kernel Mailing List, linux-csky,
	open list:BROADCOM NVRAM DRIVER, sparclinux, linux-riscv,
	linuxppc-dev, Linux ARM
In-Reply-To: <20220201150545.1512822-16-guoren@kernel.org>

On Tue, Feb 1, 2022 at 11:07 PM <guoren@kernel.org> wrote:
>
> From: Guo Ren <guoren@linux.alibaba.com>
>
> Detect hardware COMPAT (32bit U-mode) capability in rv64. If not
> support COMPAT mode in hw, compat_elf_check_arch would return
> false by compat_binfmt_elf.c
>
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Signed-off-by: Guo Ren <guoren@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Christoph Hellwig <hch@lst.de>
> ---
>  arch/riscv/include/asm/elf.h |  3 ++-
>  arch/riscv/kernel/process.c  | 32 ++++++++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/include/asm/elf.h b/arch/riscv/include/asm/elf.h
> index aee40040917b..3a4293dc7229 100644
> --- a/arch/riscv/include/asm/elf.h
> +++ b/arch/riscv/include/asm/elf.h
> @@ -40,7 +40,8 @@
>   * elf64_hdr e_machine's offset are different. The checker is
>   * a little bit simple compare to other architectures.
>   */
> -#define compat_elf_check_arch(x) ((x)->e_machine == EM_RISCV)
> +extern bool compat_elf_check_arch(Elf32_Ehdr *hdr);
> +#define compat_elf_check_arch  compat_elf_check_arch
>
>  #define CORE_DUMP_USE_REGSET
>  #define ELF_EXEC_PAGESIZE      (PAGE_SIZE)
> diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
> index 1a666ad299b4..758847cba391 100644
> --- a/arch/riscv/kernel/process.c
> +++ b/arch/riscv/kernel/process.c
> @@ -83,6 +83,38 @@ void show_regs(struct pt_regs *regs)
>                 dump_backtrace(regs, NULL, KERN_DEFAULT);
>  }
>
> +#ifdef CONFIG_COMPAT
> +static bool compat_mode_support __read_mostly;
> +
> +bool compat_elf_check_arch(Elf32_Ehdr *hdr)
> +{
> +       if (compat_mode_support && (hdr->e_machine == EM_RISCV))
> +               return true;
> +       else
> +               return false;
> +}
> +
> +static int compat_mode_detect(void)
Forgot __init, here

> +{
> +       unsigned long tmp = csr_read(CSR_STATUS);
> +
> +       csr_write(CSR_STATUS, (tmp & ~SR_UXL) | SR_UXL_32);
> +
> +       if ((csr_read(CSR_STATUS) & SR_UXL) != SR_UXL_32) {
> +               pr_info("riscv: 32bit compat mode detect failed\n");
> +               compat_mode_support = false;
> +       } else {
> +               compat_mode_support = true;
> +               pr_info("riscv: 32bit compat mode detected\n");
> +       }
> +
> +       csr_write(CSR_STATUS, tmp);
> +
> +       return 0;
> +}
> +arch_initcall(compat_mode_detect);
> +#endif
> +
>  void start_thread(struct pt_regs *regs, unsigned long pc,
>         unsigned long sp)
>  {
> --
> 2.25.1
>


-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

^ permalink raw reply

* [PATCH V2] powerpc/perf: Fix task context setting for trace imc
From: Athira Rajeev @ 2022-02-02  4:18 UTC (permalink / raw)
  To: mpe; +Cc: kjain, maddy, linuxppc-dev, npiggin, rnsastry

Trace IMC (In-Memory collection counters) in powerpc is
useful for application level profiling. For trace_imc,
presently task context (task_ctx_nr) is set to
perf_hw_context. But perf_hw_context is to be used for
cpu PMU. So for trace_imc, even though it is per thread
PMU, it is preferred to use sw_context inorder to be able
to do application level monitoring. Hence change the
task_ctx_nr to use perf_sw_context.

Fixes: 012ae244845f ("powerpc/perf: Trace imc PMU functions")
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Reviewed-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
---
Changelog:
v1 -> v2:
 Added comment in code on why perf_sw_context is used.
Notes:
 trace_imc_event_init currently uses context as
 perf_hw_context. But ideally there can only be a single
 PMU for perf_hw_context events which is core PMU.
 Reference:
 commit 26657848502b ("perf/core: Verify we have a single perf_hw_context PMU")
 Reason for using "perf_sw_context" instead of invalid_context
 is that, task level monitoring is restricted with
 invalid_context.

 arch/powerpc/perf/imc-pmu.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index e106909ff9c3..8fe57601e61d 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c
@@ -1457,7 +1457,13 @@ static int trace_imc_event_init(struct perf_event *event)
 
 	event->hw.idx = -1;
 
-	event->pmu->task_ctx_nr = perf_hw_context;
+	/*
+	 * There can only be a single PMU for
+	 * perf_hw_context events which is assigned
+	 * to core PMU. Hence use "perf_sw_context" for
+	 * trace_imc.
+	 */
+	event->pmu->task_ctx_nr = perf_sw_context;
 	event->destroy = reset_global_refc;
 	return 0;
 }
-- 
2.33.0


^ permalink raw reply related

* [PATCH v2] powerpc/ptdump: Fix sparse warning in hashpagetable.c
From: Michael Ellerman @ 2022-02-02  5:30 UTC (permalink / raw)
  To: linuxppc-dev

As reported by sparse:

  arch/powerpc/mm/ptdump/hashpagetable.c:264:29: warning: restricted __be64 degrades to integer
  arch/powerpc/mm/ptdump/hashpagetable.c:265:49: warning: restricted __be64 degrades to integer
  arch/powerpc/mm/ptdump/hashpagetable.c:267:36: warning: incorrect type in assignment (different base types)
  arch/powerpc/mm/ptdump/hashpagetable.c:267:36:    expected unsigned long long [usertype]
  arch/powerpc/mm/ptdump/hashpagetable.c:267:36:    got restricted __be64 [usertype] v
  arch/powerpc/mm/ptdump/hashpagetable.c:268:36: warning: incorrect type in assignment (different base types)
  arch/powerpc/mm/ptdump/hashpagetable.c:268:36:    expected unsigned long long [usertype]
  arch/powerpc/mm/ptdump/hashpagetable.c:268:36:    got restricted __be64 [usertype] r

The values returned by plpar_pte_read_4() are CPU endian, not __be64, so
assigning them to struct hash_pte confuses sparse. As a minimal fix open
code a struct to hold the values with CPU endian types.

Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/mm/ptdump/hashpagetable.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

v2: Don't use struct hash_pte at all.

Replaces v1 http://patchwork.ozlabs.org/project/linuxppc-dev/patch/bbc196451dd34521d239023ccca488db35b8fff1.1643567900.git.christophe.leroy@csgroup.eu/

diff --git a/arch/powerpc/mm/ptdump/hashpagetable.c b/arch/powerpc/mm/ptdump/hashpagetable.c
index c7f824d294b2..9a601587836b 100644
--- a/arch/powerpc/mm/ptdump/hashpagetable.c
+++ b/arch/powerpc/mm/ptdump/hashpagetable.c
@@ -238,7 +238,10 @@ static int native_find(unsigned long ea, int psize, bool primary, u64 *v, u64
 
 static int pseries_find(unsigned long ea, int psize, bool primary, u64 *v, u64 *r)
 {
-	struct hash_pte ptes[4];
+	struct {
+		unsigned long v;
+		unsigned long r;
+	} ptes[4];
 	unsigned long vsid, vpn, hash, hpte_group, want_v;
 	int i, j, ssize = mmu_kernel_ssize;
 	long lpar_rc = 0;
-- 
2.34.1


^ permalink raw reply related

* [PATCH] mm: Merge pte_mkhuge() call into arch_make_huge_pte()
From: Anshuman Khandual @ 2022-02-02  5:38 UTC (permalink / raw)
  To: linux-mm
  Cc: Anshuman Khandual, Catalin Marinas, linuxppc-dev, linux-kernel,
	Paul Mackerras, sparclinux, Andrew Morton, Will Deacon,
	David S. Miller, linux-arm-kernel, Mike Kravetz

Each call into pte_mkhuge() is invariably followed by arch_make_huge_pte().
Instead arch_make_huge_pte() can accommodate pte_mkhuge() at the beginning.
This updates generic fallback stub for arch_make_huge_pte() and available
platforms definitions. This makes huge pte creation much cleaner and easier
to follow.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Paul Mackerras <paulus@samba.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Mike Kravetz <mike.kravetz@oracle.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: sparclinux@vger.kernel.org
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/arm64/mm/hugetlbpage.c                      | 1 +
 arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h | 1 +
 arch/sparc/mm/hugetlbpage.c                      | 1 +
 include/linux/hugetlb.h                          | 2 +-
 mm/hugetlb.c                                     | 3 +--
 mm/vmalloc.c                                     | 1 -
 6 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
index ffb9c229610a..228226c5fa80 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -347,6 +347,7 @@ pte_t arch_make_huge_pte(pte_t entry, unsigned int shift, vm_flags_t flags)
 {
 	size_t pagesize = 1UL << shift;
 
+	entry = pte_mkhuge(entry);
 	if (pagesize == CONT_PTE_SIZE) {
 		entry = pte_mkcont(entry);
 	} else if (pagesize == CONT_PMD_SIZE) {
diff --git a/arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h b/arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h
index 64b6c608eca4..e41e095158c7 100644
--- a/arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h
+++ b/arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h
@@ -70,6 +70,7 @@ static inline pte_t arch_make_huge_pte(pte_t entry, unsigned int shift, vm_flags
 {
 	size_t size = 1UL << shift;
 
+	entry = pte_mkhuge(entry);
 	if (size == SZ_16K)
 		return __pte(pte_val(entry) & ~_PAGE_HUGE);
 	else
diff --git a/arch/sparc/mm/hugetlbpage.c b/arch/sparc/mm/hugetlbpage.c
index 0f49fada2093..d8e0e3c7038d 100644
--- a/arch/sparc/mm/hugetlbpage.c
+++ b/arch/sparc/mm/hugetlbpage.c
@@ -181,6 +181,7 @@ pte_t arch_make_huge_pte(pte_t entry, unsigned int shift, vm_flags_t flags)
 {
 	pte_t pte;
 
+	entry = pte_mkhuge(entry);
 	pte = hugepage_shift_to_tte(entry, shift);
 
 #ifdef CONFIG_SPARC64
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index d1897a69c540..52c462390aee 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -754,7 +754,7 @@ static inline void arch_clear_hugepage_flags(struct page *page) { }
 static inline pte_t arch_make_huge_pte(pte_t entry, unsigned int shift,
 				       vm_flags_t flags)
 {
-	return entry;
+	return pte_mkhuge(entry);
 }
 #endif
 
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 61895cc01d09..5ca253c1b4e4 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -4637,7 +4637,6 @@ static pte_t make_huge_pte(struct vm_area_struct *vma, struct page *page,
 					   vma->vm_page_prot));
 	}
 	entry = pte_mkyoung(entry);
-	entry = pte_mkhuge(entry);
 	entry = arch_make_huge_pte(entry, shift, vma->vm_flags);
 
 	return entry;
@@ -6172,7 +6171,7 @@ unsigned long hugetlb_change_protection(struct vm_area_struct *vma,
 			unsigned int shift = huge_page_shift(hstate_vma(vma));
 
 			old_pte = huge_ptep_modify_prot_start(vma, address, ptep);
-			pte = pte_mkhuge(huge_pte_modify(old_pte, newprot));
+			pte = huge_pte_modify(old_pte, newprot);
 			pte = arch_make_huge_pte(pte, shift, vma->vm_flags);
 			huge_ptep_modify_prot_commit(vma, address, ptep, old_pte, pte);
 			pages++;
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 4165304d3547..d0b14dd73adc 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -118,7 +118,6 @@ static int vmap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
 		if (size != PAGE_SIZE) {
 			pte_t entry = pfn_pte(pfn, prot);
 
-			entry = pte_mkhuge(entry);
 			entry = arch_make_huge_pte(entry, ilog2(size), 0);
 			set_huge_pte_at(&init_mm, addr, pte, entry);
 			pfn += PFN_DOWN(size);
-- 
2.25.1


^ permalink raw reply related

* Re: [PATCH v7 0/5] Allow guest access to EFI confidential computing secret area
From: Greg KH @ 2022-02-02  6:10 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: linux-efi, Brijesh Singh, Lenny Szubowicz, Gerd Hoffmann,
	gcwilson, Ard Biesheuvel, Daniele Buono, Andi Kleen, Nayna Jain,
	James Morris, Dov Murik, Jim Cadden, Peter Gonda, Borislav Petkov,
	Serge E. Hallyn, Tom Lendacky, Ashish Kalra, dougmill,
	James Bottomley, Dr. David Alan Gilbert, Tobin Feldman-Fitzthum,
	linux-coco, gjoyce, dja, Dave Hansen, linux-kernel,
	linux-security-module, linuxppc-dev, Andrew Scull
In-Reply-To: <20220202040157.GA8019@srcf.ucam.org>

On Wed, Feb 02, 2022 at 04:01:57AM +0000, Matthew Garrett wrote:
> On Tue, Feb 01, 2022 at 09:24:50AM -0500, James Bottomley wrote:
> > On Tue, 2022-02-01 at 14:50 +0100, Greg KH wrote:
> > > You all need to work together to come up with a unified place for
> > > this and stop making it platform-specific.
> 
> We're talking about things that have massively different semantics.

I see lots of different platforms trying to provide access to their
"secure" firmware data to userspace in different ways.  That feels to me
like they are the same thing that userspace would care about in a
unified way.

Unless we expeect userspace tools to have to be platform-specific for
all of this?  That does not seem wise.

what am I missing here?

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH] mm: Merge pte_mkhuge() call into arch_make_huge_pte()
From: Mike Rapoport @ 2022-02-02  6:18 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: linuxppc-dev, linux-kernel, linux-mm, Paul Mackerras,
	Catalin Marinas, sparclinux, Andrew Morton, Will Deacon,
	David S. Miller, linux-arm-kernel, Mike Kravetz
In-Reply-To: <1643780286-18798-1-git-send-email-anshuman.khandual@arm.com>

On Wed, Feb 02, 2022 at 11:08:06AM +0530, Anshuman Khandual wrote:
> Each call into pte_mkhuge() is invariably followed by arch_make_huge_pte().
> Instead arch_make_huge_pte() can accommodate pte_mkhuge() at the beginning.
> This updates generic fallback stub for arch_make_huge_pte() and available
> platforms definitions. This makes huge pte creation much cleaner and easier
> to follow.

Won't it break architectures that don't define arch_make_huge_pte()?
 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Mike Kravetz <mike.kravetz@oracle.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: sparclinux@vger.kernel.org
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>  arch/arm64/mm/hugetlbpage.c                      | 1 +
>  arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h | 1 +
>  arch/sparc/mm/hugetlbpage.c                      | 1 +
>  include/linux/hugetlb.h                          | 2 +-
>  mm/hugetlb.c                                     | 3 +--
>  mm/vmalloc.c                                     | 1 -
>  6 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
> index ffb9c229610a..228226c5fa80 100644
> --- a/arch/arm64/mm/hugetlbpage.c
> +++ b/arch/arm64/mm/hugetlbpage.c
> @@ -347,6 +347,7 @@ pte_t arch_make_huge_pte(pte_t entry, unsigned int shift, vm_flags_t flags)
>  {
>  	size_t pagesize = 1UL << shift;
>  
> +	entry = pte_mkhuge(entry);
>  	if (pagesize == CONT_PTE_SIZE) {
>  		entry = pte_mkcont(entry);
>  	} else if (pagesize == CONT_PMD_SIZE) {
> diff --git a/arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h b/arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h
> index 64b6c608eca4..e41e095158c7 100644
> --- a/arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h
> +++ b/arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h
> @@ -70,6 +70,7 @@ static inline pte_t arch_make_huge_pte(pte_t entry, unsigned int shift, vm_flags
>  {
>  	size_t size = 1UL << shift;
>  
> +	entry = pte_mkhuge(entry);
>  	if (size == SZ_16K)
>  		return __pte(pte_val(entry) & ~_PAGE_HUGE);
>  	else
> diff --git a/arch/sparc/mm/hugetlbpage.c b/arch/sparc/mm/hugetlbpage.c
> index 0f49fada2093..d8e0e3c7038d 100644
> --- a/arch/sparc/mm/hugetlbpage.c
> +++ b/arch/sparc/mm/hugetlbpage.c
> @@ -181,6 +181,7 @@ pte_t arch_make_huge_pte(pte_t entry, unsigned int shift, vm_flags_t flags)
>  {
>  	pte_t pte;
>  
> +	entry = pte_mkhuge(entry);
>  	pte = hugepage_shift_to_tte(entry, shift);
>  
>  #ifdef CONFIG_SPARC64
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> index d1897a69c540..52c462390aee 100644
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -754,7 +754,7 @@ static inline void arch_clear_hugepage_flags(struct page *page) { }
>  static inline pte_t arch_make_huge_pte(pte_t entry, unsigned int shift,
>  				       vm_flags_t flags)
>  {
> -	return entry;
> +	return pte_mkhuge(entry);
>  }
>  #endif
>  
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 61895cc01d09..5ca253c1b4e4 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -4637,7 +4637,6 @@ static pte_t make_huge_pte(struct vm_area_struct *vma, struct page *page,
>  					   vma->vm_page_prot));
>  	}
>  	entry = pte_mkyoung(entry);
> -	entry = pte_mkhuge(entry);
>  	entry = arch_make_huge_pte(entry, shift, vma->vm_flags);
>  
>  	return entry;
> @@ -6172,7 +6171,7 @@ unsigned long hugetlb_change_protection(struct vm_area_struct *vma,
>  			unsigned int shift = huge_page_shift(hstate_vma(vma));
>  
>  			old_pte = huge_ptep_modify_prot_start(vma, address, ptep);
> -			pte = pte_mkhuge(huge_pte_modify(old_pte, newprot));
> +			pte = huge_pte_modify(old_pte, newprot);
>  			pte = arch_make_huge_pte(pte, shift, vma->vm_flags);
>  			huge_ptep_modify_prot_commit(vma, address, ptep, old_pte, pte);
>  			pages++;
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 4165304d3547..d0b14dd73adc 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -118,7 +118,6 @@ static int vmap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
>  		if (size != PAGE_SIZE) {
>  			pte_t entry = pfn_pte(pfn, prot);
>  
> -			entry = pte_mkhuge(entry);
>  			entry = arch_make_huge_pte(entry, ilog2(size), 0);
>  			set_huge_pte_at(&init_mm, addr, pte, entry);
>  			pfn += PFN_DOWN(size);
> -- 
> 2.25.1
> 
> 

-- 
Sincerely yours,
Mike.

^ permalink raw reply

* Re: [PATCH] mm: Merge pte_mkhuge() call into arch_make_huge_pte()
From: Christophe Leroy @ 2022-02-02  6:20 UTC (permalink / raw)
  To: Anshuman Khandual, linux-mm@kvack.org
  Cc: Will Deacon, Catalin Marinas, linux-kernel@vger.kernel.org,
	Paul Mackerras, sparclinux@vger.kernel.org, Andrew Morton,
	linuxppc-dev@lists.ozlabs.org, David S. Miller,
	linux-arm-kernel@lists.infradead.org, Mike Kravetz
In-Reply-To: <1643780286-18798-1-git-send-email-anshuman.khandual@arm.com>



Le 02/02/2022 à 06:38, Anshuman Khandual a écrit :
> Each call into pte_mkhuge() is invariably followed by arch_make_huge_pte().
> Instead arch_make_huge_pte() can accommodate pte_mkhuge() at the beginning.
> This updates generic fallback stub for arch_make_huge_pte() and available
> platforms definitions. This makes huge pte creation much cleaner and easier
> to follow.

I think it is a good cleanup. I always wonder why commit d9ed9faac283 
("mm: add new arch_make_huge_pte() method for tile support") didn't move 
the pte_mkhuge() into arch_make_huge_pte().

When I implemented arch_make_huge_pte() for powerpc 8xx, in one case 
arch_make_huge_pte() have to undo the things done by pte_mkhuge(), see below

As a second step we could probably try to get rid of pte_mkhuge() 
completely, at least in the core.

> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Mike Kravetz <mike.kravetz@oracle.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: sparclinux@vger.kernel.org
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>

Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>

> ---
>   arch/arm64/mm/hugetlbpage.c                      | 1 +
>   arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h | 1 +
>   arch/sparc/mm/hugetlbpage.c                      | 1 +
>   include/linux/hugetlb.h                          | 2 +-
>   mm/hugetlb.c                                     | 3 +--
>   mm/vmalloc.c                                     | 1 -
>   6 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
> index ffb9c229610a..228226c5fa80 100644
> --- a/arch/arm64/mm/hugetlbpage.c
> +++ b/arch/arm64/mm/hugetlbpage.c
> @@ -347,6 +347,7 @@ pte_t arch_make_huge_pte(pte_t entry, unsigned int shift, vm_flags_t flags)
>   {
>   	size_t pagesize = 1UL << shift;
>   
> +	entry = pte_mkhuge(entry);
>   	if (pagesize == CONT_PTE_SIZE) {
>   		entry = pte_mkcont(entry);
>   	} else if (pagesize == CONT_PMD_SIZE) {
> diff --git a/arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h b/arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h
> index 64b6c608eca4..e41e095158c7 100644
> --- a/arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h
> +++ b/arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h
> @@ -70,6 +70,7 @@ static inline pte_t arch_make_huge_pte(pte_t entry, unsigned int shift, vm_flags
>   {
>   	size_t size = 1UL << shift;
>   
> +	entry = pte_mkhuge(entry);

Could drop that and replace the below by:

	if (size == SZ_16K)
		return __pte(pte_val(entry) | _PAGE_SPS);
	else
		return __pte(pte_val(entry) | _PAGE_SPS | _PAGE_HUGE);
	

>   	if (size == SZ_16K)
>   		return __pte(pte_val(entry) & ~_PAGE_HUGE);
>   	else
> diff --git a/arch/sparc/mm/hugetlbpage.c b/arch/sparc/mm/hugetlbpage.c
> index 0f49fada2093..d8e0e3c7038d 100644
> --- a/arch/sparc/mm/hugetlbpage.c
> +++ b/arch/sparc/mm/hugetlbpage.c
> @@ -181,6 +181,7 @@ pte_t arch_make_huge_pte(pte_t entry, unsigned int shift, vm_flags_t flags)
>   {
>   	pte_t pte;
>   
> +	entry = pte_mkhuge(entry);
>   	pte = hugepage_shift_to_tte(entry, shift);
>   
>   #ifdef CONFIG_SPARC64
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> index d1897a69c540..52c462390aee 100644
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -754,7 +754,7 @@ static inline void arch_clear_hugepage_flags(struct page *page) { }
>   static inline pte_t arch_make_huge_pte(pte_t entry, unsigned int shift,
>   				       vm_flags_t flags)
>   {
> -	return entry;
> +	return pte_mkhuge(entry);
>   }
>   #endif
>   
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 61895cc01d09..5ca253c1b4e4 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -4637,7 +4637,6 @@ static pte_t make_huge_pte(struct vm_area_struct *vma, struct page *page,
>   					   vma->vm_page_prot));
>   	}
>   	entry = pte_mkyoung(entry);
> -	entry = pte_mkhuge(entry);
>   	entry = arch_make_huge_pte(entry, shift, vma->vm_flags);
>   
>   	return entry;
> @@ -6172,7 +6171,7 @@ unsigned long hugetlb_change_protection(struct vm_area_struct *vma,
>   			unsigned int shift = huge_page_shift(hstate_vma(vma));
>   
>   			old_pte = huge_ptep_modify_prot_start(vma, address, ptep);
> -			pte = pte_mkhuge(huge_pte_modify(old_pte, newprot));
> +			pte = huge_pte_modify(old_pte, newprot);
>   			pte = arch_make_huge_pte(pte, shift, vma->vm_flags);
>   			huge_ptep_modify_prot_commit(vma, address, ptep, old_pte, pte);
>   			pages++;
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 4165304d3547..d0b14dd73adc 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -118,7 +118,6 @@ static int vmap_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
>   		if (size != PAGE_SIZE) {
>   			pte_t entry = pfn_pte(pfn, prot);
>   
> -			entry = pte_mkhuge(entry);
>   			entry = arch_make_huge_pte(entry, ilog2(size), 0);
>   			set_huge_pte_at(&init_mm, addr, pte, entry);
>   			pfn += PFN_DOWN(size);

^ permalink raw reply

* Re: [PATCH] mm: Merge pte_mkhuge() call into arch_make_huge_pte()
From: Christophe Leroy @ 2022-02-02  6:25 UTC (permalink / raw)
  To: Mike Rapoport, Anshuman Khandual
  Cc: Will Deacon, Catalin Marinas, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, Paul Mackerras, sparclinux@vger.kernel.org,
	Andrew Morton, linuxppc-dev@lists.ozlabs.org, David S. Miller,
	linux-arm-kernel@lists.infradead.org, Mike Kravetz
In-Reply-To: <YfoiLdboxJAxKi6Y@kernel.org>



Le 02/02/2022 à 07:18, Mike Rapoport a écrit :
> On Wed, Feb 02, 2022 at 11:08:06AM +0530, Anshuman Khandual wrote:
>> Each call into pte_mkhuge() is invariably followed by arch_make_huge_pte().
>> Instead arch_make_huge_pte() can accommodate pte_mkhuge() at the beginning.
>> This updates generic fallback stub for arch_make_huge_pte() and available
>> platforms definitions. This makes huge pte creation much cleaner and easier
>> to follow.
> 
> Won't it break architectures that don't define arch_make_huge_pte()?

It shouldn't, see below

>> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
>> index d1897a69c540..52c462390aee 100644
>> --- a/include/linux/hugetlb.h
>> +++ b/include/linux/hugetlb.h
>> @@ -754,7 +754,7 @@ static inline void arch_clear_hugepage_flags(struct page *page) { }
>>   static inline pte_t arch_make_huge_pte(pte_t entry, unsigned int shift,
>>   				       vm_flags_t flags)
>>   {
>> -	return entry;
>> +	return pte_mkhuge(entry);
>>   }
>>   #endif
>>   

^ permalink raw reply

* Re: [PATCH v7 0/5] Allow guest access to EFI confidential computing secret area
From: Greg KH @ 2022-02-02  7:05 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: linux-efi, Brijesh Singh, Lenny Szubowicz, Gerd Hoffmann,
	gcwilson, Ard Biesheuvel, Daniele Buono, Andi Kleen, Nayna Jain,
	James Morris, Dov Murik, Jim Cadden, Peter Gonda, Borislav Petkov,
	Serge E. Hallyn, Tom Lendacky, Ashish Kalra, dougmill,
	James Bottomley, Dr. David Alan Gilbert, Tobin Feldman-Fitzthum,
	linux-coco, gjoyce, dja, Dave Hansen, linux-kernel,
	linux-security-module, linuxppc-dev, Andrew Scull
In-Reply-To: <20220202065443.GA9249@srcf.ucam.org>

On Wed, Feb 02, 2022 at 06:54:43AM +0000, Matthew Garrett wrote:
> On Wed, Feb 02, 2022 at 07:10:02AM +0100, Greg KH wrote:
> > On Wed, Feb 02, 2022 at 04:01:57AM +0000, Matthew Garrett wrote:
> > > We're talking about things that have massively different semantics.
> > 
> > I see lots of different platforms trying to provide access to their
> > "secure" firmware data to userspace in different ways.  That feels to me
> > like they are the same thing that userspace would care about in a
> > unified way.
> 
> EFI variables are largely for the OS to provide information to the 
> firmware, while this patchset is to provide information from the 
> firmware to the OS. I don't see why we'd expect to use the same userland 
> tooling for both.

I totally agree, I'm not worried about EFI variables here, I don't know
why that came up.

> In the broader case - I don't think we *can* use the same userland
> tooling for everything. For example, the patches to add support for 
> manipulating the Power secure boot keys originally attempted to make it 
> look like efivars, but the underlying firmware semantics are 
> sufficiently different that even exposing the same kernel interface 
> wouldn't be a sufficient abstraction and userland would still need to 
> behave differently. Exposing an interface that looks consistent but 
> isn't is arguably worse for userland than exposing explicitly distinct 
> interfaces.

So what does userspace really need here?  Just the ability to find if
the platform has blobs that it cares about, and how to read/write them.

I see different platform patches trying to stick these blobs in
different locations and ways to access (securityfs, sysfs, char device
node), which seems crazy to me.  Why can't we at least pick one way to
access these to start with, and then have the filesystem layout be
platform-specific as needed, which will give the correct hints to
userspace as to what it needs to do here?

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH v7 0/5] Allow guest access to EFI confidential computing secret area
From: Ard Biesheuvel @ 2022-02-02  7:22 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: linux-efi, Brijesh Singh, Lenny Szubowicz, Gerd Hoffmann,
	gcwilson, Daniele Buono, Andi Kleen, Nayna Jain, James Morris,
	Dov Murik, Jim Cadden, Peter Gonda, Borislav Petkov,
	Serge E. Hallyn, Tom Lendacky, Ashish Kalra, dougmill,
	James Bottomley, Dr. David Alan Gilbert, Tobin Feldman-Fitzthum,
	linux-coco, gjoyce, Daniel Axtens, Dave Hansen, Greg KH,
	Linux Kernel Mailing List, linux-security-module,
	open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), Andrew Scull
In-Reply-To: <20220202071023.GA9489@srcf.ucam.org>

On Wed, 2 Feb 2022 at 08:10, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
>
> On Wed, Feb 02, 2022 at 08:05:23AM +0100, Greg KH wrote:
>
> > I see different platform patches trying to stick these blobs in
> > different locations and ways to access (securityfs, sysfs, char device
> > node), which seems crazy to me.  Why can't we at least pick one way to
> > access these to start with, and then have the filesystem layout be
> > platform-specific as needed, which will give the correct hints to
> > userspace as to what it needs to do here?
>
> Which other examples are you thinking of? I think this conversation may
> have accidentally become conflated with a different prior one and now
> we're talking at cross purposes.

This came up a while ago during review of one of the earlier revisions
of this patch set.

https://lore.kernel.org/linux-efi/YRZuIIVIzMfgjtEl@google.com/

which describes another two variations on the theme, for pKVM guests
as well as Android bare metal.

^ permalink raw reply

* Re: [PATCH] mm: Merge pte_mkhuge() call into arch_make_huge_pte()
From: Mike Rapoport @ 2022-02-02  7:37 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Will Deacon, Anshuman Khandual, Catalin Marinas,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org, Paul Mackerras,
	sparclinux@vger.kernel.org, Andrew Morton,
	linuxppc-dev@lists.ozlabs.org, David S. Miller,
	linux-arm-kernel@lists.infradead.org, Mike Kravetz
In-Reply-To: <76605ef1-f2fe-8682-1eb7-2323f0b9bbaa@csgroup.eu>

On Wed, Feb 02, 2022 at 06:25:31AM +0000, Christophe Leroy wrote:
> 
> 
> Le 02/02/2022 à 07:18, Mike Rapoport a écrit :
> > On Wed, Feb 02, 2022 at 11:08:06AM +0530, Anshuman Khandual wrote:
> >> Each call into pte_mkhuge() is invariably followed by arch_make_huge_pte().
> >> Instead arch_make_huge_pte() can accommodate pte_mkhuge() at the beginning.
> >> This updates generic fallback stub for arch_make_huge_pte() and available
> >> platforms definitions. This makes huge pte creation much cleaner and easier
> >> to follow.
> > 
> > Won't it break architectures that don't define arch_make_huge_pte()?
> 
> It shouldn't, see below

Right, overlooked that.
 
> >> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> >> index d1897a69c540..52c462390aee 100644
> >> --- a/include/linux/hugetlb.h
> >> +++ b/include/linux/hugetlb.h
> >> @@ -754,7 +754,7 @@ static inline void arch_clear_hugepage_flags(struct page *page) { }
> >>   static inline pte_t arch_make_huge_pte(pte_t entry, unsigned int shift,
> >>   				       vm_flags_t flags)
> >>   {
> >> -	return entry;
> >> +	return pte_mkhuge(entry);
> >>   }
> >>   #endif
> >>   

-- 
Sincerely yours,
Mike.

^ permalink raw reply

* Re: [PATCH V5 06/21] asm-generic: compat: Cleanup duplicate definitions
From: Christoph Hellwig @ 2022-02-02  7:48 UTC (permalink / raw)
  To: guoren
  Cc: linux-arch, linux-s390, Guo Ren, linux-parisc, arnd, gregkh, drew,
	anup, wangjunqiang, x86, linux-kernel, linux-csky, linux-mips,
	palmer, liush, sparclinux, linux-riscv, linuxppc-dev, hch,
	linux-arm-kernel, wefu
In-Reply-To: <20220201150545.1512822-7-guoren@kernel.org>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply

* Re: [PATCH V5 15/21] riscv: compat: Add hw capability check for elf
From: Christoph Hellwig @ 2022-02-02  7:51 UTC (permalink / raw)
  To: guoren
  Cc: linux-arch, linux-s390, Guo Ren, linux-parisc, arnd, gregkh, drew,
	anup, wangjunqiang, x86, linux-kernel, linux-csky, linux-mips,
	palmer, liush, sparclinux, linux-riscv, linuxppc-dev, hch,
	linux-arm-kernel, wefu
In-Reply-To: <20220201150545.1512822-16-guoren@kernel.org>

On Tue, Feb 01, 2022 at 11:05:39PM +0800, guoren@kernel.org wrote:
> +bool compat_elf_check_arch(Elf32_Ehdr *hdr)
> +{
> +	if (compat_mode_support && (hdr->e_machine == EM_RISCV))
> +		return true;
> +	else
> +		return false;
> +}

This can be simplified to:

	return compat_mode_support && hdr->e_machine == EM_RISCV;

I'd also rename compat_mode_support to compat_mode_supported

> +
> +static int compat_mode_detect(void)
> +{
> +	unsigned long tmp = csr_read(CSR_STATUS);
> +
> +	csr_write(CSR_STATUS, (tmp & ~SR_UXL) | SR_UXL_32);
> +
> +	if ((csr_read(CSR_STATUS) & SR_UXL) != SR_UXL_32) {
> +		pr_info("riscv: 32bit compat mode detect failed\n");
> +		compat_mode_support = false;
> +	} else {
> +		compat_mode_support = true;
> +		pr_info("riscv: 32bit compat mode detected\n");
> +	}

I don't think we need these printks here.

Also this could be simplified to:

	compat_mode_supported = (csr_read(CSR_STATUS) & SR_UXL) == SR_UXL_32;

^ permalink raw reply

* Re: [PATCH v7 0/5] Allow guest access to EFI confidential computing secret area
From: Greg KH @ 2022-02-02  8:25 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: linux-efi, Brijesh Singh, Lenny Szubowicz, Gerd Hoffmann,
	gcwilson, Ard Biesheuvel, Daniele Buono, Andi Kleen, Nayna Jain,
	James Morris, Dov Murik, Jim Cadden, Peter Gonda, Borislav Petkov,
	Serge E. Hallyn, Tom Lendacky, Ashish Kalra, dougmill,
	James Bottomley, Dr. David Alan Gilbert, Tobin Feldman-Fitzthum,
	linux-coco, gjoyce, Daniel Axtens, Dave Hansen,
	Linux Kernel Mailing List, linux-security-module,
	open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), Andrew Scull
In-Reply-To: <20220202080401.GA9861@srcf.ucam.org>

On Wed, Feb 02, 2022 at 08:04:01AM +0000, Matthew Garrett wrote:
> On Wed, Feb 02, 2022 at 08:22:03AM +0100, Ard Biesheuvel wrote:
> > On Wed, 2 Feb 2022 at 08:10, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> > > Which other examples are you thinking of? I think this conversation may
> > > have accidentally become conflated with a different prior one and now
> > > we're talking at cross purposes.
> > 
> > This came up a while ago during review of one of the earlier revisions
> > of this patch set.
> > 
> > https://lore.kernel.org/linux-efi/YRZuIIVIzMfgjtEl@google.com/
> > 
> > which describes another two variations on the theme, for pKVM guests
> > as well as Android bare metal.
> 
> Oh, I see! That makes much more sense - sorry, I wasn't Cc:ed on that, 
> so thought this was related to the efivars/Power secure boot. My 
> apologies, sorry for the noise. In that case, given the apparent 
> agreement between the patch owners that a consistent interface would 
> work for them, I think I agree with Greg that we should strive for that. 
> Given the described behaviour of the Google implementation, it feels 
> like the semantics in this implementation would be sufficient for them 
> as well, but having confirmation of that would be helpful.
> 
> On the other hand, I also agree that a new filesystem for this is 
> overkill. I did that for efivarfs and I think the primary lesson from 
> that is that people who aren't familiar with the vfs shouldn't be 
> writing filesystems. Securityfs seems entirely reasonable, and it's 
> consistent with other cases where we expose firmware-provided data 
> that's security relevant.
> 
> The only thing I personally struggle with here is whether "coco" is the 
> best name for it, and whether there are reasonable use cases that 
> wouldn't be directly related to confidential computing (eg, if the 
> firmware on a bare-metal platform had a mechanism for exposing secrets 
> to the OS based on some specific platform security state, it would seem 
> reasonable to expose it via this mechanism but it may not be what we'd 
> normally think of as Confidential Computing).
> 
> But I'd also say that while we only have one implementation currently 
> sending patches, it's fine for the code to live in that implementation 
> and then be abstracted out once we have another.

Well right now the Android code looks the cleanest and should be about
ready to be merged into my tree.

But I can almost guarantee that that interface is not what anyone else
wants to use, so if you think somehow that everyone else is going to
want to deal with a char device node and a simple mmap, with a DT
description of the thing, hey, I'm all for it :)

Seriously, people need to come up with something sane or this is going
to be a total mess.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH v7 0/5] Allow guest access to EFI confidential computing secret area
From: Gerd Hoffmann @ 2022-02-02  8:36 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: linux-efi, Brijesh Singh, Lenny Szubowicz, gcwilson,
	Ard Biesheuvel, Daniele Buono, Andi Kleen, Nayna Jain,
	James Morris, Dov Murik, Jim Cadden, Peter Gonda, Borislav Petkov,
	Serge E. Hallyn, Tom Lendacky, Ashish Kalra, dougmill,
	James Bottomley, Dr. David Alan Gilbert, Tobin Feldman-Fitzthum,
	linux-coco, gjoyce, Daniel Axtens, Dave Hansen, Greg KH,
	Linux Kernel Mailing List, linux-security-module,
	open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), Andrew Scull
In-Reply-To: <20220202080401.GA9861@srcf.ucam.org>

  Hi,
 
> The only thing I personally struggle with here is whether "coco" is the 
> best name for it, and whether there are reasonable use cases that 
> wouldn't be directly related to confidential computing (eg, if the 
> firmware on a bare-metal platform had a mechanism for exposing secrets 
> to the OS based on some specific platform security state, it would seem 
> reasonable to expose it via this mechanism but it may not be what we'd 
> normally think of as Confidential Computing).
> 
> But I'd also say that while we only have one implementation currently 
> sending patches, it's fine for the code to live in that implementation 
> and then be abstracted out once we have another.

The implementation can be sorted later when a second implementation
shows up, but it'll be better if we don't have to change the naming
convention.

"coco/efi_secrets" doesn't look like a good choice to me, given that it
is rather likely we see more users for this showing up.

Having a "secrets/" directory looks good to me.  Then the individual
implementations can either add files to the directory, i.e. efi_secrets
would create "secrets/<guid>" files.  Or each implementation creates a
subdirectory with the secrets, i.e. "secrets/coco/" and
"secrets/coco/<guid>".

Longer-term (i.e once we have more than one implementation) we probably
need a separate module which owns and manages the "secrets/" directory,
and possibly provides some common helper functions too.

take care,
  Gerd


^ permalink raw reply

* Re: [PATCH v4] powerpc/papr_scm: Implement initial support for injecting smart errors
From: Aneesh Kumar K.V @ 2022-02-02  9:03 UTC (permalink / raw)
  To: Vaibhav Jain, nvdimm, linuxppc-dev
  Cc: Vaibhav Jain, Dan Williams, Ira Weiny, Shivaprasad G Bhat
In-Reply-To: <20220124202204.1488346-1-vaibhav@linux.ibm.com>

Vaibhav Jain <vaibhav@linux.ibm.com> writes:

> Presently PAPR doesn't support injecting smart errors on an
> NVDIMM. This makes testing the NVDIMM health reporting functionality
> difficult as simulating NVDIMM health related events need a hacked up
> qemu version.
>
> To solve this problem this patch proposes simulating certain set of
> NVDIMM health related events in papr_scm. Specifically 'fatal' health
> state and 'dirty' shutdown state. These error can be injected via the
> user-space 'ndctl-inject-smart(1)' command. With the proposed patch and
> corresponding ndctl patches following command flow is expected:
>
> $ sudo ndctl list -DH -d nmem0
> ...
>       "health_state":"ok",
>       "shutdown_state":"clean",
> ...
>  # inject unsafe shutdown and fatal health error
> $ sudo ndctl inject-smart nmem0 -Uf
> ...
>       "health_state":"fatal",
>       "shutdown_state":"dirty",
> ...
>  # uninject all errors
> $ sudo ndctl inject-smart nmem0 -N
> ...
>       "health_state":"ok",
>       "shutdown_state":"clean",
> ...
>
> The patch adds a new member 'health_bitmap_inject_mask' inside struct
> papr_scm_priv which is then bitwise ANDed to the health bitmap fetched from the
> hypervisor. The value for 'health_bitmap_inject_mask' is accessible from sysfs
> at nmemX/papr/health_bitmap_inject.
>
> A new PDSM named 'SMART_INJECT' is proposed that accepts newly
> introduced 'struct nd_papr_pdsm_smart_inject' as payload thats
> exchanged between libndctl and papr_scm to indicate the requested
> smart-error states.
>
> When the processing the PDSM 'SMART_INJECT', papr_pdsm_smart_inject()
> constructs a pair or 'inject_mask' and 'clear_mask' bitmaps from the payload
> and bit-blt it to the 'health_bitmap_inject_mask'. This ensures the after being
> fetched from the hypervisor, the health_bitmap reflects requested smart-error
> states.
>

 Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>

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

^ permalink raw reply

* Re: [PATCH] mm: Merge pte_mkhuge() call into arch_make_huge_pte()
From: Anshuman Khandual @ 2022-02-02  9:38 UTC (permalink / raw)
  To: Christophe Leroy, linux-mm@kvack.org
  Cc: Will Deacon, Catalin Marinas, linux-kernel@vger.kernel.org,
	Paul Mackerras, sparclinux@vger.kernel.org, Andrew Morton,
	linuxppc-dev@lists.ozlabs.org, David S. Miller,
	linux-arm-kernel@lists.infradead.org, Mike Kravetz
In-Reply-To: <a969f100-02fb-63f7-4469-b3c8e23d8cfb@csgroup.eu>



On 2/2/22 11:50 AM, Christophe Leroy wrote:
> 
> Le 02/02/2022 à 06:38, Anshuman Khandual a écrit :
>> Each call into pte_mkhuge() is invariably followed by arch_make_huge_pte().
>> Instead arch_make_huge_pte() can accommodate pte_mkhuge() at the beginning.
>> This updates generic fallback stub for arch_make_huge_pte() and available
>> platforms definitions. This makes huge pte creation much cleaner and easier
>> to follow.
> I think it is a good cleanup. I always wonder why commit d9ed9faac283 
> ("mm: add new arch_make_huge_pte() method for tile support") didn't move 
> the pte_mkhuge() into arch_make_huge_pte().

+1

> 
> When I implemented arch_make_huge_pte() for powerpc 8xx, in one case 
> arch_make_huge_pte() have to undo the things done by pte_mkhuge(), see below
> 
> As a second step we could probably try to get rid of pte_mkhuge() 
> completely, at least in the core.

Sure.

> 
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: "David S. Miller" <davem@davemloft.net>
>> Cc: Mike Kravetz <mike.kravetz@oracle.com>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linuxppc-dev@lists.ozlabs.org
>> Cc: sparclinux@vger.kernel.org
>> Cc: linux-mm@kvack.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> 
>> ---
>>   arch/arm64/mm/hugetlbpage.c                      | 1 +
>>   arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h | 1 +
>>   arch/sparc/mm/hugetlbpage.c                      | 1 +
>>   include/linux/hugetlb.h                          | 2 +-
>>   mm/hugetlb.c                                     | 3 +--
>>   mm/vmalloc.c                                     | 1 -
>>   6 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
>> index ffb9c229610a..228226c5fa80 100644
>> --- a/arch/arm64/mm/hugetlbpage.c
>> +++ b/arch/arm64/mm/hugetlbpage.c
>> @@ -347,6 +347,7 @@ pte_t arch_make_huge_pte(pte_t entry, unsigned int shift, vm_flags_t flags)
>>   {
>>   	size_t pagesize = 1UL << shift;
>>   
>> +	entry = pte_mkhuge(entry);
>>   	if (pagesize == CONT_PTE_SIZE) {
>>   		entry = pte_mkcont(entry);
>>   	} else if (pagesize == CONT_PMD_SIZE) {
>> diff --git a/arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h b/arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h
>> index 64b6c608eca4..e41e095158c7 100644
>> --- a/arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h
>> +++ b/arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h
>> @@ -70,6 +70,7 @@ static inline pte_t arch_make_huge_pte(pte_t entry, unsigned int shift, vm_flags
>>   {
>>   	size_t size = 1UL << shift;
>>   
>> +	entry = pte_mkhuge(entry);
> Could drop that and replace the below by:
> 
> 	if (size == SZ_16K)
> 		return __pte(pte_val(entry) | _PAGE_SPS);
> 	else
> 		return __pte(pte_val(entry) | _PAGE_SPS | _PAGE_HUGE);
> 	
> 

Sure, will change as stated above.

^ permalink raw reply

* Re: [PATCH v4 3/7] mm: page_isolation: check specified range for unmovable pages
From: Oscar Salvador @ 2022-02-02 12:18 UTC (permalink / raw)
  To: Zi Yan
  Cc: Mel Gorman, David Hildenbrand, linuxppc-dev, linux-kernel,
	virtualization, linux-mm, iommu, Eric Ren, Robin Murphy,
	Christoph Hellwig, Vlastimil Babka, Marek Szyprowski
In-Reply-To: <20220119190623.1029355-4-zi.yan@sent.com>

On Wed, Jan 19, 2022 at 02:06:19PM -0500, Zi Yan wrote:
> From: Zi Yan <ziy@nvidia.com>
> 
> Enable set_migratetype_isolate() to check specified sub-range for
> unmovable pages during isolation. Page isolation is done
> at max(MAX_ORDER_NR_PAEGS, pageblock_nr_pages) granularity, but not all
> pages within that granularity are intended to be isolated. For example,
> alloc_contig_range(), which uses page isolation, allows ranges without
> alignment. This commit makes unmovable page check only look for
> interesting pages, so that page isolation can succeed for any
> non-overlapping ranges.

Another thing that came to my mind.
Prior to this patch, has_unmovable_pages() was checking on pageblock
granularity, starting at pfn#0 of the pageblock.
With this patch, you no longer check on pageblock granularity, which
means you might isolate a pageblock, but some pages that sneaked in
might actually be unmovable.

E.g:

Let's say you have a pageblock that spans (pfn#512,pfn#1024),
and you pass alloc_contig_range() (pfn#514,pfn#1024).
has_unmovable_pages() will start checking the pageblock at pfn#514,
and so it will mis pfn#512 and pfn#513. Isn't that a problem, if those
pfn turn out to be actually unmovable?


-- 
Oscar Salvador
SUSE Labs

^ permalink raw reply


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