LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 2/7] soc: fsl: guts: remove module_exit() and fsl_guts_remove()
From: Michael Walle @ 2022-02-09 16:32 UTC (permalink / raw)
  To: linuxppc-dev, linux-arm-kernel, linux-kernel
  Cc: Ulf Hansson, Arnd Bergmann, Li Yang, Michael Walle, Sudeep Holla,
	Dan Carpenter
In-Reply-To: <20220209163242.430265-1-michael@walle.cc>

This driver will never be unloaded. Firstly, it is not available as a
module, but more importantly, other drivers will depend on this one to
apply possible chip errata.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 drivers/soc/fsl/guts.c | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
index be18d46c7b0f..0bea43770d51 100644
--- a/drivers/soc/fsl/guts.c
+++ b/drivers/soc/fsl/guts.c
@@ -27,7 +27,6 @@ struct fsl_soc_die_attr {
 
 static struct guts *guts;
 static struct soc_device_attribute soc_dev_attr;
-static struct soc_device *soc_dev;
 
 
 /* SoC die attribute definition for QorIQ platform */
@@ -138,6 +137,7 @@ static u32 fsl_guts_get_svr(void)
 static int fsl_guts_probe(struct platform_device *pdev)
 {
 	struct device_node *root, *np = pdev->dev.of_node;
+	static struct soc_device *soc_dev;
 	struct device *dev = &pdev->dev;
 	const struct fsl_soc_die_attr *soc_die;
 	const char *machine = NULL;
@@ -197,12 +197,6 @@ static int fsl_guts_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int fsl_guts_remove(struct platform_device *dev)
-{
-	soc_device_unregister(soc_dev);
-	return 0;
-}
-
 /*
  * Table for matching compatible strings, for device tree
  * guts node, for Freescale QorIQ SOCs.
@@ -242,7 +236,6 @@ static struct platform_driver fsl_guts_driver = {
 		.of_match_table = fsl_guts_of_match,
 	},
 	.probe = fsl_guts_probe,
-	.remove = fsl_guts_remove,
 };
 
 static int __init fsl_guts_init(void)
@@ -250,9 +243,3 @@ static int __init fsl_guts_init(void)
 	return platform_driver_register(&fsl_guts_driver);
 }
 core_initcall(fsl_guts_init);
-
-static void __exit fsl_guts_exit(void)
-{
-	platform_driver_unregister(&fsl_guts_driver);
-}
-module_exit(fsl_guts_exit);
-- 
2.30.2


^ permalink raw reply related

* [PATCH v2 3/7] soc: fsl: guts: embed fsl_guts_get_svr() in probe()
From: Michael Walle @ 2022-02-09 16:32 UTC (permalink / raw)
  To: linuxppc-dev, linux-arm-kernel, linux-kernel
  Cc: Ulf Hansson, Arnd Bergmann, Li Yang, Michael Walle, Sudeep Holla,
	Dan Carpenter
In-Reply-To: <20220209163242.430265-1-michael@walle.cc>

Move the reading of the SVR into the probe function as
fsl_guts_get_svr() is the only user of the static guts variable and this
lets us drop that as well as the malloc() for this variable. Also, we
can unmap the memory region after we accessed it, which will simplify
error handling later.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 drivers/soc/fsl/guts.c | 42 +++++++++++-------------------------------
 1 file changed, 11 insertions(+), 31 deletions(-)

diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
index 0bea43770d51..4e5675ab5f73 100644
--- a/drivers/soc/fsl/guts.c
+++ b/drivers/soc/fsl/guts.c
@@ -14,18 +14,12 @@
 #include <linux/platform_device.h>
 #include <linux/fsl/guts.h>
 
-struct guts {
-	struct ccsr_guts __iomem *regs;
-	bool little_endian;
-};
-
 struct fsl_soc_die_attr {
 	char	*die;
 	u32	svr;
 	u32	mask;
 };
 
-static struct guts *guts;
 static struct soc_device_attribute soc_dev_attr;
 
 
@@ -119,40 +113,27 @@ static const struct fsl_soc_die_attr *fsl_soc_die_match(
 	return NULL;
 }
 
-static u32 fsl_guts_get_svr(void)
-{
-	u32 svr = 0;
-
-	if (!guts || !guts->regs)
-		return svr;
-
-	if (guts->little_endian)
-		svr = ioread32(&guts->regs->svr);
-	else
-		svr = ioread32be(&guts->regs->svr);
-
-	return svr;
-}
-
 static int fsl_guts_probe(struct platform_device *pdev)
 {
 	struct device_node *root, *np = pdev->dev.of_node;
 	static struct soc_device *soc_dev;
 	struct device *dev = &pdev->dev;
 	const struct fsl_soc_die_attr *soc_die;
+	struct ccsr_guts __iomem *regs;
 	const char *machine = NULL;
+	bool little_endian;
 	u32 svr;
 
-	/* Initialize guts */
-	guts = devm_kzalloc(dev, sizeof(*guts), GFP_KERNEL);
-	if (!guts)
-		return -ENOMEM;
-
-	guts->little_endian = of_property_read_bool(np, "little-endian");
+	regs = of_iomap(np, 0);
+	if (IS_ERR(regs))
+		return PTR_ERR(regs);
 
-	guts->regs = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(guts->regs))
-		return PTR_ERR(guts->regs);
+	little_endian = of_property_read_bool(np, "little-endian");
+	if (little_endian)
+		svr = ioread32(&regs->svr);
+	else
+		svr = ioread32be(&regs->svr);
+	iounmap(regs);
 
 	/* Register soc device */
 	root = of_find_node_by_path("/");
@@ -167,7 +148,6 @@ static int fsl_guts_probe(struct platform_device *pdev)
 	}
 	of_node_put(root);
 
-	svr = fsl_guts_get_svr();
 	soc_die = fsl_soc_die_match(svr, fsl_soc_die);
 	if (soc_die) {
 		soc_dev_attr.family = devm_kasprintf(dev, GFP_KERNEL,
-- 
2.30.2


^ permalink raw reply related

* [PATCH] selftest/vm: Use correct PAGE_SHIFT value for ppc64
From: Aneesh Kumar K.V @ 2022-02-09 15:43 UTC (permalink / raw)
  To: linux-mm, akpm; +Cc: Shuah Khan, linuxppc-dev, Aneesh Kumar K.V

Keep it simple by using a #define and limiting hugepage size to 2M.
This keeps the test simpler instead of dynamically finding the page size
and huge page size.

Without this tests are broken w.r.t reading /proc/self/pagemap

	if (pread(pagemap_fd, ent, sizeof(ent),
			(uintptr_t)ptr >> (PAGE_SHIFT - 3)) != sizeof(ent))
		err(2, "read pagemap");

Cc: Shuah Khan <shuah@kernel.org>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
 tools/testing/selftests/vm/ksm_tests.c        | 8 ++++++++
 tools/testing/selftests/vm/transhuge-stress.c | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/tools/testing/selftests/vm/ksm_tests.c b/tools/testing/selftests/vm/ksm_tests.c
index 1436e1a9a3d3..8200328ff018 100644
--- a/tools/testing/selftests/vm/ksm_tests.c
+++ b/tools/testing/selftests/vm/ksm_tests.c
@@ -22,8 +22,16 @@
 #define KSM_MERGE_ACROSS_NODES_DEFAULT true
 #define MB (1ul << 20)
 
+#ifdef __powerpc64__
+#define PAGE_SHIFT	16
+/*
+ * This will only work with radix 2M hugepage size
+ */
+#define HPAGE_SHIFT 21
+#else
 #define PAGE_SHIFT 12
 #define HPAGE_SHIFT 21
+#endif
 
 #define PAGE_SIZE (1 << PAGE_SHIFT)
 #define HPAGE_SIZE (1 << HPAGE_SHIFT)
diff --git a/tools/testing/selftests/vm/transhuge-stress.c b/tools/testing/selftests/vm/transhuge-stress.c
index 5e4c036f6ad3..f04c8aa4bcf6 100644
--- a/tools/testing/selftests/vm/transhuge-stress.c
+++ b/tools/testing/selftests/vm/transhuge-stress.c
@@ -16,8 +16,16 @@
 #include <string.h>
 #include <sys/mman.h>
 
+#ifdef __powerpc64__
+#define PAGE_SHIFT	16
+/*
+ * This will only work with radix 2M hugepage size
+ */
+#define HPAGE_SHIFT 21
+#else
 #define PAGE_SHIFT 12
 #define HPAGE_SHIFT 21
+#endif
 
 #define PAGE_SIZE (1 << PAGE_SHIFT)
 #define HPAGE_SIZE (1 << HPAGE_SHIFT)
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH v2 0/5] Allocate module text and data separately
From: Miroslav Benes @ 2022-02-09 15:42 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: linux-arch@vger.kernel.org, kgdb-bugreport@lists.sourceforge.net,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Luis Chamberlain, Jessica Yu, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <cover.1643282353.git.christophe.leroy@csgroup.eu>

On Thu, 27 Jan 2022, Christophe Leroy wrote:

> This series allow architectures to request having modules data in
> vmalloc area instead of module area.
> 
> This is required on powerpc book3s/32 in order to set data non
> executable, because it is not possible to set executability on page
> basis, this is done per 256 Mbytes segments. The module area has exec
> right, vmalloc area has noexec. Without this change module data
> remains executable regardless of CONFIG_STRICT_MODULES_RWX.
> 
> This can also be useful on other powerpc/32 in order to maximize the
> chance of code being close enough to kernel core to avoid branch
> trampolines.
> 
> Changes in v2:
> - Dropped first two patches which are not necessary. They may be added back later as a follow-up series.
> - Fixed the printks in GDB
> 
> Christophe Leroy (5):
>   modules: Always have struct mod_tree_root
>   modules: Prepare for handling several RB trees
>   modules: Introduce data_layout
>   modules: Add CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
>   powerpc: Select ARCH_WANTS_MODULES_DATA_IN_VMALLOC on book3s/32 and
>     8xx
> 
>  arch/Kconfig                |   6 ++
>  arch/powerpc/Kconfig        |   1 +
>  include/linux/module.h      |   8 ++
>  kernel/debug/kdb/kdb_main.c |  10 +-
>  kernel/module.c             | 193 +++++++++++++++++++++++++-----------
>  5 files changed, 156 insertions(+), 62 deletions(-)

Looks good to me apart from the typo I mentioned at v1. I will review 
again once it is rebased on Aaron's patch set.

Regards,
Miroslav

^ permalink raw reply

* RE: [PATCH v2 1/2] lib/raid6/test/Makefile: Use `$(pound)` instead of `\#` for Make 4.3
From: David Laight @ 2022-02-09 14:45 UTC (permalink / raw)
  To: 'David T-G', linux-raid@vger.kernel.org
  Cc: netdev@vger.kernel.org, bpf@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
In-Reply-To: <20220209134139.GA4455@justpickone.org>

From: David T-G
> Sent: 09 February 2022 13:42
> 
> ...and then Wols Lists said...
> %
> % On 08/02/2022 15:21, Paul Menzel wrote:
> ...
> %
> % As commented elsewhere, for the sake of us ENGLISH speakers,
> % *PLEASE* make that $(hash). A pound sign is £.
> 
> Or, even better, $(octothorpe) since that's merely a symbol rather than a
> food product or a result of an algorithm on data.  You might even hope
> that we hash this out eventually ...

I was more worried that people might think we should smoke the hash.

The # symbol called 'hash' in the UK. Can't remember why - but it is used
to mean 'number'.

'octothorpe' is some brain-damaged name and should be shot^Werased on sight.

The whole UK v US confusion about what a 'pound' sign looks like almost
certainly led to UK ascii using the £ glyph for 0x23.
I can imaging a phone call where a US person said '0x23 is the pound sign'.

I remember problems with ascii peripherals on a ebcdic mainframe where
£ $ # and \ had to get squeezed into the three available codes.
Not only was in semi-random what a line printer might print,
we had 'page mode' terminals where the input and output translation
tables didn't always match.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply

* Re: [PATCH v2 1/2] lib/raid6/test/Makefile: Use `$(pound)` instead of `\#` for Make 4.3
From: David T-G @ 2022-02-09 13:41 UTC (permalink / raw)
  To: linux-raid; +Cc: netdev, bpf, linuxppc-dev, linux-kernel
In-Reply-To: <d07a9d41-5a8f-a1f3-59f7-d2a75d6df2e5@youngman.org.uk>

Paul, et al --

...and then Wols Lists said...
% 
% On 08/02/2022 15:21, Paul Menzel wrote:
...
% 
% As commented elsewhere, for the sake of us ENGLISH speakers,
% *PLEASE* make that $(hash). A pound sign is £.

Or, even better, $(octothorpe) since that's merely a symbol rather than a
food product or a result of an algorithm on data.  You might even hope
that we hash this out eventually ...


Have a great day!

:-D
-- 
David T-G
See http://justpickone.org/davidtg/email/
See http://justpickone.org/davidtg/tofu.txt


^ permalink raw reply

* Re: [PATCH v5 2/6] powerpc/kexec_file: Add KEXEC_SIG support.
From: Michal Suchánek @ 2022-02-09 12:01 UTC (permalink / raw)
  To: Paul Menzel
  Cc: Nayna, Mimi Zohar, Sven Schnelle, David Howells, keyrings,
	Paul Mackerras, Alexander Gordeev, Rob Herring, Herbert Xu,
	Baoquan He, Christian Borntraeger, James Morris,
	Lakshmi Ramasubramanian, Christian Borntraeger, Serge E. Hallyn,
	Vasily Gorbik, linux-s390, Heiko Carstens, Dmitry Kasatkin,
	Hari Bathini, Daniel Axtens, Philipp Rudo, Frank van der Linden,
	kexec, linux-kernel, Luis Chamberlain, linux-crypto,
	linux-security-module, Jessica Yu, linux-integrity, linuxppc-dev,
	David S. Miller, Thiago Jung Bauermann, buendgen
In-Reply-To: <b56fe3a2-b145-9d4e-acf2-4991204b3102@molgen.mpg.de>

Hello,

On Wed, Feb 09, 2022 at 07:44:15AM +0100, Paul Menzel wrote:
> Dear Michal,
> 
> 
> Thank you for the patch.
> 
> 
> Am 11.01.22 um 12:37 schrieb Michal Suchanek:
> 
> Could you please remove the dot/period at the end of the git commit message
> summary?

Sure

> > Copy the code from s390x
> > 
> > Both powerpc and s390x use appended signature format (as opposed to EFI
> > based patforms using PE format).
> 
> patforms → platforms

Thanks for noticing

> How can this be tested?

Apparently KEXEC_SIG_FORCE is x86 only although the use of the option is
arch neutral:

arch/x86/Kconfig:config KEXEC_SIG_FORCE
kernel/kexec_file.c:            if (IS_ENABLED(CONFIG_KEXEC_SIG_FORCE))
{

Maybe it should be moved?

I used a patched kernel that enables lockdown in secure boot, and then
verified that signed kernel can be loaded by kexec and unsigned not,
with KEXEC_SIG enabled and IMA_KEXEC disabled.

The lockdown support can be enabled on any platform, and although I
can't find it documented anywhere there appears to be code in kexec_file
to take it into account:
kernel/kexec.c: result = security_locked_down(LOCKDOWN_KEXEC);
kernel/kexec_file.c:                security_locked_down(LOCKDOWN_KEXEC))
kernel/module.c:        return security_locked_down(LOCKDOWN_MODULE_SIGNATURE);
kernel/params.c:            security_locked_down(LOCKDOWN_MODULE_PARAMETERS))
and lockdown can be enabled with a buildtime option, a kernel parameter, or a
debugfs file.

Still for testing lifting KEXEC_SIG_FORCE to some arch-neutral Kconfig file is
probably the simplest option.

kexec -s option should be used to select kexec_file rather than the old
style kexec which would either fail always or succeed always regardelss
of signature.

> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> > v3: - Philipp Rudo <prudo@redhat.com>: Update the comit message with
> >        explanation why the s390 code is usable on powerpc.
> >      - Include correct header for mod_check_sig
> >      - Nayna <nayna@linux.vnet.ibm.com>: Mention additional IMA features
> >        in kconfig text
> > ---
> >   arch/powerpc/Kconfig        | 16 ++++++++++++++++
> >   arch/powerpc/kexec/elf_64.c | 36 ++++++++++++++++++++++++++++++++++++
> >   2 files changed, 52 insertions(+)
> > 
> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > index dea74d7717c0..1cde9b6c5987 100644
> > --- a/arch/powerpc/Kconfig
> > +++ b/arch/powerpc/Kconfig
> > @@ -560,6 +560,22 @@ config KEXEC_FILE
> >   config ARCH_HAS_KEXEC_PURGATORY
> >   	def_bool KEXEC_FILE
> > +config KEXEC_SIG
> > +	bool "Verify kernel signature during kexec_file_load() syscall"
> > +	depends on KEXEC_FILE && MODULE_SIG_FORMAT
> > +	help
> > +	  This option makes kernel signature verification mandatory for
> > +	  the kexec_file_load() syscall.
> > +
> > +	  In addition to that option, you need to enable signature
> > +	  verification for the corresponding kernel image type being
> > +	  loaded in order for this to work.
> > +
> > +	  Note: on powerpc IMA_ARCH_POLICY also implements kexec'ed kernel
> > +	  verification. In addition IMA adds kernel hashes to the measurement
> > +	  list, extends IMA PCR in the TPM, and implements kernel image
> > +	  blacklist by hash.
> 
> So, what is the takeaway for the user? IMA_ARCH_POLICY is preferred? What is
> the disadvantage, and two implementations(?) needed then? More overhead?

IMA_KEXEC does more than KEXEC_SIG. The overhead is probably not big
unless you are trying to really minimize the kernel code size.

Arguably the simpler implementation hass less potential for bugs, too.
Both in code and in user configuration required to enable the feature.

Interestingly IMA_ARCH_POLICY depends on KEXEC_SIG rather than
IMA_KEXEC. Just mind-boggling.

The main problem with IMA_KEXEC from my point of view is it is not portable.
To record the measurements TPM support is requireed which is not available on
all platforms. It does not support PE so it cannot be used on platforms
that use PE kernel signature format.

> 
> > +
> >   config RELOCATABLE
> >   	bool "Build a relocatable kernel"
> >   	depends on PPC64 || (FLATMEM && (44x || FSL_BOOKE))
> > diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
> > index eeb258002d1e..98d1cb5135b4 100644
> > --- a/arch/powerpc/kexec/elf_64.c
> > +++ b/arch/powerpc/kexec/elf_64.c
> > @@ -23,6 +23,7 @@
> >   #include <linux/of_fdt.h>
> >   #include <linux/slab.h>
> >   #include <linux/types.h>
> > +#include <linux/module_signature.h>
> >   static void *elf64_load(struct kimage *image, char *kernel_buf,
> >   			unsigned long kernel_len, char *initrd,
> > @@ -151,7 +152,42 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
> >   	return ret ? ERR_PTR(ret) : NULL;
> >   }
> > +#ifdef CONFIG_KEXEC_SIG
> > +int elf64_verify_sig(const char *kernel, unsigned long kernel_len)
> > +{
> > +	const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1;
> > +	struct module_signature *ms;
> > +	unsigned long sig_len;
> 
> Use size_t to match the signature of `verify_pkcs7_signature()`?

Nope. struct module_signature uses unsigned long, and this needs to be
matched to avoid type errors on 32bit.

Technically using size_t for in-memory buffers is misguided because
AFAICT no memory buffer can be bigger than ULONG_MAX, and size_t is
non-native type on 32bit.

Sure, the situation with ssize_t/int is different but that's not what we
are dealing with here.

Thanks

Michal

^ permalink raw reply

* Re: [PATCH] ocxl: Make use of the helper macro LIST_HEAD()
From: Frederic Barrat @ 2022-02-09  9:51 UTC (permalink / raw)
  To: Cai Huoqing
  Cc: Greg Kroah-Hartman, linuxppc-dev, Andrew Donnellan, Arnd Bergmann,
	linux-kernel
In-Reply-To: <20220209032421.37725-1-cai.huoqing@linux.dev>



On 09/02/2022 04:24, Cai Huoqing wrote:
> Replace "struct list_head head = LIST_HEAD_INIT(head)" with
> "LIST_HEAD(head)" to simplify the code.
> 
> Signed-off-by: Cai Huoqing <cai.huoqing@linux.dev>
> ---

Thanks!
Acked-by: Frederic Barrat <fbarrat@linux.ibm.com>


>   drivers/misc/ocxl/link.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
> index ab039c115381..9670d02c927f 100644
> --- a/drivers/misc/ocxl/link.c
> +++ b/drivers/misc/ocxl/link.c
> @@ -94,7 +94,7 @@ struct ocxl_link {
>   	struct spa *spa;
>   	void *platform_data;
>   };
> -static struct list_head links_list = LIST_HEAD_INIT(links_list);
> +static LIST_HEAD(links_list);
>   static DEFINE_MUTEX(links_list_lock);
>   
>   enum xsl_response {

^ permalink raw reply

* Re: [RFC PATCH 2/2] pseries: define sysfs interface to expose PKS variables
From: Dov Murik @ 2022-02-09  9:13 UTC (permalink / raw)
  To: Nayna Jain, linuxppc-dev
  Cc: linux-kernel, Dov Murik, Douglas Miller, Greg KH, George Wilson,
	gjoyce, Daniel Axtens
In-Reply-To: <20220122005637.28199-3-nayna@linux.ibm.com>

Hi Nayna,


On 22/01/2022 2:56, Nayna Jain wrote:
> PowerVM guest secure boot intend to use Platform Keystore(PKS) for the
> purpose of storing public keys to verify digital signature.
> 
> Define sysfs interface to expose PKS variables to userspace to allow
> read/write/add/delete operations. Each variable is shown as a read/write
> attribute file. The size of the file represents the size of the current
> content of the variable.
> 
> create_var and delete_var attribute files are always present which allow
> users to create/delete variables. These are write only attributes.The
> design has tried to be compliant with sysfs semantic to represent single
> value per attribute. Thus, rather than mapping a complete data structure
> representation to create_var, it only accepts a single formatted string
> to create an empty variable.
> 
> The sysfs interface is designed such as to expose PKS configuration
> properties, operating system variables and firmware variables.
> Current version exposes configuration and operating system variables.
> The support for exposing firmware variables will be added in the future
> version.
> 
> Example of pksvar sysfs interface:
> 
> # cd /sys/firmware/pksvar/
> # ls
> config  os
> 
> # cd config
> 
> # ls -ltrh
> total 0
> -r--r--r-- 1 root root 64K Jan 21 17:55 version
> -r--r--r-- 1 root root 64K Jan 21 17:55 used_space
> -r--r--r-- 1 root root 64K Jan 21 17:55 total_size
> -r--r--r-- 1 root root 64K Jan 21 17:55 supported_policies
> -r--r--r-- 1 root root 64K Jan 21 17:55 max_object_size
> -r--r--r-- 1 root root 64K Jan 21 17:55 max_object_label_size
> -r--r--r-- 1 root root 64K Jan 21 17:55 flags
> 
> # cd os
> 
> # ls -ltrh
> total 0
> -rw------- 1 root root 104 Jan 21 17:56 var4
> -rw------- 1 root root 104 Jan 21 17:56 var3
> -rw------- 1 root root 831 Jan 21 17:56 GLOBAL_PK
> -rw------- 1 root root 831 Jan 21 17:56 GLOBAL_KEK
> -rw------- 1 root root  76 Jan 21 17:56 GLOBAL_dbx
> -rw------- 1 root root 831 Jan 21 17:56 GLOBAL_db
> --w------- 1 root root 64K Jan 21 17:56 delete_var
> --w------- 1 root root 64K Jan 21 17:56 create_var
> 
> 1. Read variable
> 
> # hexdump -C GLOBAL_db
> 00000000  00 00 00 00 a1 59 c0 a5  e4 94 a7 4a 87 b5 ab 15  |.....Y.....J....|
> 00000010  5c 2b f0 72 3f 03 00 00  00 00 00 00 23 03 00 00  |\+.r?.......#...|
> ....
> 00000330  02 a8 e8 ed 0f 20 60 3f  40 04 7c a8 91 21 37 eb  |..... `?@.|..!7.|
> 00000340  f3 f1 4e                                          |..N|
> 00000343
> 
> 2. Write variable
> 
> cat /tmp/data.bin > <variable_name>
> 
> 3. Create variable
> 
> # echo "var1" > create_var

It would be easier to understand if the user could create a new variable
like a regular new file, something like:

# cat /tmp/data.bin > var1

but I understand there are also comma-seperated-policy-strings which
should go somewhere; not sure how this fits (or if there are other
examples for similar interfaces in other sysfs parts).



> # ls -ltrh
> total 0
> -rw------- 1 root root 104 Jan 21 17:56 var4
> -rw------- 1 root root 104 Jan 21 17:56 var3
> -rw------- 1 root root 831 Jan 21 17:56 GLOBAL_PK
> -rw------- 1 root root 831 Jan 21 17:56 GLOBAL_KEK
> -rw------- 1 root root  76 Jan 21 17:56 GLOBAL_dbx
> -rw------- 1 root root 831 Jan 21 17:56 GLOBAL_db
> --w------- 1 root root 64K Jan 21 17:56 delete_var
> --w------- 1 root root 64K Jan 21 17:57 create_var
> -rw------- 1 root root   0 Jan 21 17:57 var1.tmp
> 
> Current design creates a zero size temporary variable. This implies
> it is not yet persisted to PKS. Only once data is written to newly
> created temporary variable and if it is successfully stored in the
> PKS, that the variable is permanent. The temporary variable will get
> removed on reboot. The code currently doesn't remove .tmp suffix
> immediately when persisted. The future version will fix this.
> 
> To avoid the additional .tmp semantic, alternative option is to consider
> any zero size variable as temporary variable. This option is under
> evaluation. This would avoid any runtime sysfs magic to replace .tmp
> variable with real variable.
> 
> Also, the formatted string to pass to create_var will have following
> format in the future version:
> <variable_name>:<comma-separated-policy strings>
> 
> 4. Delete variable
> # echo "var3" > delete_var

If it's possible here, I think it would be easier to understand (and
use) if you implement unlink(), so deleting var3 would be:

# rm var3

(and then there's no need for the special 'delete_var' entry.)


-Dov

> # ls -ltrh
> total 0
> -rw------- 1 root root 104 Jan 21 17:56 var4
> -rw------- 1 root root 831 Jan 21 17:56 GLOBAL_PK
> -rw------- 1 root root 831 Jan 21 17:56 GLOBAL_KEK
> -rw------- 1 root root  76 Jan 21 17:56 GLOBAL_dbx
> -rw------- 1 root root 831 Jan 21 17:56 GLOBAL_db
> --w------- 1 root root 64K Jan 21 17:57 create_var
> -rw------- 1 root root   0 Jan 21 17:57 var1.tmp
> --w------- 1 root root 64K Jan 21 17:58 delete_var
> 
> The var3 file is removed at runtime, if variable is successfully
> removed from the PKS storage.
> 
> NOTE: We are evaluating two design for userspace interface: using the
> sysfs or defining a new filesystem based. Any feedback on this sysfs based
> approach would be highly appreciated. We have tried to follow one value
> per attribute semantic. If that or any other semantics aren't followed
> properly, please let us know.
> 
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> ---
>  Documentation/ABI/testing/sysfs-pksvar        |  77 ++++
>  arch/powerpc/platforms/pseries/Kconfig        |   7 +
>  arch/powerpc/platforms/pseries/Makefile       |   1 +
>  arch/powerpc/platforms/pseries/pksvar-sysfs.c | 356 ++++++++++++++++++
>  4 files changed, 441 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-pksvar
>  create mode 100644 arch/powerpc/platforms/pseries/pksvar-sysfs.c
> 


^ permalink raw reply

* [PATCH] powerpc: delete redundant conversion
From: Qing Wang @ 2022-02-09  8:34 UTC (permalink / raw)
  To: Anatolij Gustschin, Michael Ellerman, Benjamin Herrenschmidt,
	Paul Mackerras, linuxppc-dev, linux-kernel
  Cc: Wang Qing

From: Wang Qing <wangqing@vivo.com>

do_div() does a 64-by-32 division
No need to transform gpt->ipb_freq to u64

Signed-off-by: Wang Qing <wangqing@vivo.com>
---
 arch/powerpc/platforms/52xx/mpc52xx_gpt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
index f862b48..0cb2482
--- a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
@@ -502,7 +502,7 @@ u64 mpc52xx_gpt_timer_period(struct mpc52xx_gpt_priv *gpt)
 	if (prescale == 0)
 		prescale = 0x10000;
 	period = period * prescale * 1000000000ULL;
-	do_div(period, (u64)gpt->ipb_freq);
+	do_div(period, gpt->ipb_freq);
 	return period;
 }
 EXPORT_SYMBOL(mpc52xx_gpt_timer_period);
-- 
2.7.4


^ permalink raw reply related

* Re: [PATCH] ocxl: Make use of the helper macro LIST_HEAD()
From: Andrew Donnellan @ 2022-02-09  7:53 UTC (permalink / raw)
  To: Cai Huoqing
  Cc: Frederic Barrat, Greg Kroah-Hartman, linuxppc-dev, linux-kernel,
	Arnd Bergmann
In-Reply-To: <20220209032421.37725-1-cai.huoqing@linux.dev>

On 9/2/22 14:24, Cai Huoqing wrote:
> Replace "struct list_head head = LIST_HEAD_INIT(head)" with
> "LIST_HEAD(head)" to simplify the code.
> 
> Signed-off-by: Cai Huoqing <cai.huoqing@linux.dev>

LGTM

Acked-by: Andrew Donnellan <ajd@linux.ibm.com>

-- 
Andrew Donnellan              OzLabs, ADL Canberra
ajd@linux.ibm.com             IBM Australia Limited

^ permalink raw reply

* [PATCH v3 2/5] Partially revert "KVM: Pass kvm_init()'s opaque param to additional arch funcs"
From: Chao Gao @ 2022-02-09  7:41 UTC (permalink / raw)
  To: kvm, seanjc, pbonzini, kevin.tian, tglx
  Cc: x86, Wanpeng Li, David Hildenbrand, Paul Walmsley, linux-mips,
	Paul Mackerras, H. Peter Anvin, Alexander Gordeev,
	Claudio Imbrenda, Will Deacon, Maciej S. Szmigiero, linux-s390,
	Janosch Frank, Marc Zyngier, Joerg Roedel, Huacai Chen,
	linux-riscv, kvmarm, Dave Hansen, Aleksandar Markovic,
	Ingo Molnar, Catalin Marinas, Palmer Dabbelt,
	Christian Borntraeger, Chao Gao, Ravi Bangoria, Albert Ou,
	Vasily Gorbik, Suzuki K Poulose, Heiko Carstens, Borislav Petkov,
	Atish Patra, Alexandru Elisei, linux-arm-kernel, Jim Mattson,
	Juergen Gross, Thomas Bogendoerfer, Fabiano Rosas,
	Nick Desaulniers, linux-kernel, Bharata B Rao, James Morse,
	kvm-riscv, Anup Patel, Vitaly Kuznetsov, linuxppc-dev
In-Reply-To: <20220209074109.453116-1-chao.gao@intel.com>

This partially reverts commit b99040853738 ("KVM: Pass kvm_init()'s opaque
param to additional arch funcs") remove opaque from
kvm_arch_check_processor_compat because no one uses this opaque now.
Address conflicts for ARM (due to file movement) and manually handle RISC-V
which comes after the commit.

And changes about kvm_arch_hardware_setup() in original commit are still
needed so they are not reverted.

Signed-off-by: Chao Gao <chao.gao@intel.com>
---
 arch/arm64/kvm/arm.c       |  2 +-
 arch/mips/kvm/mips.c       |  2 +-
 arch/powerpc/kvm/powerpc.c |  2 +-
 arch/riscv/kvm/main.c      |  2 +-
 arch/s390/kvm/kvm-s390.c   |  2 +-
 arch/x86/kvm/x86.c         |  2 +-
 include/linux/kvm_host.h   |  2 +-
 virt/kvm/kvm_main.c        | 16 +++-------------
 8 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index a069d5925f77..60494c576242 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -73,7 +73,7 @@ int kvm_arch_hardware_setup(void *opaque)
 	return 0;
 }
 
-int kvm_arch_check_processor_compat(void *opaque)
+int kvm_arch_check_processor_compat(void)
 {
 	return 0;
 }
diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index a25e0b73ee70..092d09fb6a7e 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -140,7 +140,7 @@ int kvm_arch_hardware_setup(void *opaque)
 	return 0;
 }
 
-int kvm_arch_check_processor_compat(void *opaque)
+int kvm_arch_check_processor_compat(void)
 {
 	return 0;
 }
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 2ad0ccd202d5..30c817f3fa0c 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -423,7 +423,7 @@ int kvm_arch_hardware_setup(void *opaque)
 	return 0;
 }
 
-int kvm_arch_check_processor_compat(void *opaque)
+int kvm_arch_check_processor_compat(void)
 {
 	return kvmppc_core_check_processor_compat();
 }
diff --git a/arch/riscv/kvm/main.c b/arch/riscv/kvm/main.c
index 2e5ca43c8c49..992877e78393 100644
--- a/arch/riscv/kvm/main.c
+++ b/arch/riscv/kvm/main.c
@@ -20,7 +20,7 @@ long kvm_arch_dev_ioctl(struct file *filp,
 	return -EINVAL;
 }
 
-int kvm_arch_check_processor_compat(void *opaque)
+int kvm_arch_check_processor_compat(void)
 {
 	return 0;
 }
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 9c6d45d0d345..99c70d881cb6 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -252,7 +252,7 @@ int kvm_arch_hardware_enable(void)
 	return 0;
 }
 
-int kvm_arch_check_processor_compat(void *opaque)
+int kvm_arch_check_processor_compat(void)
 {
 	return 0;
 }
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index b71549a52ae0..e9777ffc50c2 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -11548,7 +11548,7 @@ void kvm_arch_hardware_unsetup(void)
 	static_call(kvm_x86_hardware_unsetup)();
 }
 
-int kvm_arch_check_processor_compat(void *opaque)
+int kvm_arch_check_processor_compat(void)
 {
 	struct cpuinfo_x86 *c = &cpu_data(smp_processor_id());
 
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index b3810976a27f..3c7b654e43fb 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1413,7 +1413,7 @@ int kvm_arch_hardware_enable(void);
 void kvm_arch_hardware_disable(void);
 int kvm_arch_hardware_setup(void *opaque);
 void kvm_arch_hardware_unsetup(void);
-int kvm_arch_check_processor_compat(void *opaque);
+int kvm_arch_check_processor_compat(void);
 int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu);
 bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu);
 int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 034c567a680c..be614a6325e4 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -5599,22 +5599,14 @@ struct kvm_vcpu * __percpu *kvm_get_running_vcpus(void)
         return &kvm_running_vcpu;
 }
 
-struct kvm_cpu_compat_check {
-	void *opaque;
-	int *ret;
-};
-
-static void check_processor_compat(void *data)
+static void check_processor_compat(void *rtn)
 {
-	struct kvm_cpu_compat_check *c = data;
-
-	*c->ret = kvm_arch_check_processor_compat(c->opaque);
+	*(int *)rtn = kvm_arch_check_processor_compat();
 }
 
 int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align,
 		  struct module *module)
 {
-	struct kvm_cpu_compat_check c;
 	int r;
 	int cpu;
 
@@ -5642,10 +5634,8 @@ int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align,
 	if (r < 0)
 		goto out_free_1;
 
-	c.ret = &r;
-	c.opaque = opaque;
 	for_each_online_cpu(cpu) {
-		smp_call_function_single(cpu, check_processor_compat, &c, 1);
+		smp_call_function_single(cpu, check_processor_compat, &r, 1);
 		if (r < 0)
 			goto out_free_2;
 	}
-- 
2.25.1


^ permalink raw reply related

* Re: [PATCH v5 2/6] powerpc/kexec_file: Add KEXEC_SIG support.
From: Paul Menzel @ 2022-02-09  6:44 UTC (permalink / raw)
  To: Michal Suchanek, keyrings, linux-crypto, linux-integrity
  Cc: Nayna, Mimi Zohar, David Howells, Paul Mackerras,
	Alexander Gordeev, linux-s390, Herbert Xu, Baoquan He,
	Christian Borntraeger, James Morris, Lakshmi Ramasubramanian,
	Christian Borntraeger, Serge E. Hallyn, Vasily Gorbik,
	Rob Herring, Heiko Carstens, Dmitry Kasatkin, Hari Bathini,
	Daniel Axtens, Philipp Rudo, Frank van der Linden, kexec,
	linux-kernel, Luis Chamberlain, Sven Schnelle,
	linux-security-module, Jessica Yu, linuxppc-dev, David S. Miller,
	Thiago Jung Bauermann, buendgen
In-Reply-To: <d95f7c6865bcad5ee37dcbec240e79aa742f5e1d.1641900831.git.msuchanek@suse.de>

Dear Michal,


Thank you for the patch.


Am 11.01.22 um 12:37 schrieb Michal Suchanek:

Could you please remove the dot/period at the end of the git commit 
message summary?

> Copy the code from s390x
> 
> Both powerpc and s390x use appended signature format (as opposed to EFI
> based patforms using PE format).

patforms → platforms

How can this be tested?

> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
> v3: - Philipp Rudo <prudo@redhat.com>: Update the comit message with
>        explanation why the s390 code is usable on powerpc.
>      - Include correct header for mod_check_sig
>      - Nayna <nayna@linux.vnet.ibm.com>: Mention additional IMA features
>        in kconfig text
> ---
>   arch/powerpc/Kconfig        | 16 ++++++++++++++++
>   arch/powerpc/kexec/elf_64.c | 36 ++++++++++++++++++++++++++++++++++++
>   2 files changed, 52 insertions(+)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index dea74d7717c0..1cde9b6c5987 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -560,6 +560,22 @@ config KEXEC_FILE
>   config ARCH_HAS_KEXEC_PURGATORY
>   	def_bool KEXEC_FILE
>   
> +config KEXEC_SIG
> +	bool "Verify kernel signature during kexec_file_load() syscall"
> +	depends on KEXEC_FILE && MODULE_SIG_FORMAT
> +	help
> +	  This option makes kernel signature verification mandatory for
> +	  the kexec_file_load() syscall.
> +
> +	  In addition to that option, you need to enable signature
> +	  verification for the corresponding kernel image type being
> +	  loaded in order for this to work.
> +
> +	  Note: on powerpc IMA_ARCH_POLICY also implements kexec'ed kernel
> +	  verification. In addition IMA adds kernel hashes to the measurement
> +	  list, extends IMA PCR in the TPM, and implements kernel image
> +	  blacklist by hash.

So, what is the takeaway for the user? IMA_ARCH_POLICY is preferred? 
What is the disadvantage, and two implementations(?) needed then? More 
overhead?

> +
>   config RELOCATABLE
>   	bool "Build a relocatable kernel"
>   	depends on PPC64 || (FLATMEM && (44x || FSL_BOOKE))
> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
> index eeb258002d1e..98d1cb5135b4 100644
> --- a/arch/powerpc/kexec/elf_64.c
> +++ b/arch/powerpc/kexec/elf_64.c
> @@ -23,6 +23,7 @@
>   #include <linux/of_fdt.h>
>   #include <linux/slab.h>
>   #include <linux/types.h>
> +#include <linux/module_signature.h>
>   
>   static void *elf64_load(struct kimage *image, char *kernel_buf,
>   			unsigned long kernel_len, char *initrd,
> @@ -151,7 +152,42 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>   	return ret ? ERR_PTR(ret) : NULL;
>   }
>   
> +#ifdef CONFIG_KEXEC_SIG
> +int elf64_verify_sig(const char *kernel, unsigned long kernel_len)
> +{
> +	const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1;
> +	struct module_signature *ms;
> +	unsigned long sig_len;

Use size_t to match the signature of `verify_pkcs7_signature()`?

> +	int ret;
> +
> +	if (marker_len > kernel_len)
> +		return -EKEYREJECTED;
> +
> +	if (memcmp(kernel + kernel_len - marker_len, MODULE_SIG_STRING,
> +		   marker_len))
> +		return -EKEYREJECTED;
> +	kernel_len -= marker_len;
> +
> +	ms = (void *)kernel + kernel_len - sizeof(*ms);
> +	ret = mod_check_sig(ms, kernel_len, "kexec");
> +	if (ret)
> +		return ret;
> +
> +	sig_len = be32_to_cpu(ms->sig_len);
> +	kernel_len -= sizeof(*ms) + sig_len;
> +
> +	return verify_pkcs7_signature(kernel, kernel_len,
> +				      kernel + kernel_len, sig_len,
> +				      VERIFY_USE_PLATFORM_KEYRING,
> +				      VERIFYING_MODULE_SIGNATURE,
> +				      NULL, NULL);
> +}
> +#endif /* CONFIG_KEXEC_SIG */
> +
>   const struct kexec_file_ops kexec_elf64_ops = {
>   	.probe = kexec_elf_probe,
>   	.load = elf64_load,
> +#ifdef CONFIG_KEXEC_SIG
> +	.verify_sig = elf64_verify_sig,
> +#endif
>   };


Kind regards,

Paul

^ permalink raw reply

* Re: [PATCH v5 0/6] KEXEC_SIG with appended signature
From: Michael Ellerman @ 2022-02-09  4:46 UTC (permalink / raw)
  To: Luis Chamberlain, Michal Suchanek, David Howells, Aaron Tomlin
  Cc: Nayna, Mimi Zohar, Sven Schnelle, David Howells, keyrings,
	Paul Mackerras, Alexander Gordeev, Rob Herring, Herbert Xu,
	Baoquan He, Christian Borntraeger, James Morris,
	Lakshmi Ramasubramanian, Christian Borntraeger, Serge E. Hallyn,
	Vasily Gorbik, linux-s390, Heiko Carstens, Dmitry Kasatkin,
	Hari Bathini, Daniel Axtens, Philipp Rudo, Frank van der Linden,
	kexec, linux-kernel, linux-security-module, linux-crypto,
	Jessica Yu, linux-integrity, linuxppc-dev, David S. Miller,
	Thiago Jung Bauermann, buendgen
In-Reply-To: <YfBd/EDGUx9UIHcb@bombadil.infradead.org>

Luis Chamberlain <mcgrof@kernel.org> writes:
> On Tue, Jan 11, 2022 at 12:37:42PM +0100, Michal Suchanek wrote:
>> Hello,
>> 
>> This is a refresh of the KEXEC_SIG series.
>> 
>> This adds KEXEC_SIG support on powerpc and deduplicates the code dealing
>> with appended signatures in the kernel.
>> 
>> powerpc supports IMA_KEXEC but that's an exception rather than the norm.
>> On the other hand, KEXEC_SIG is portable across platforms.
>> 
>> For distributions to have uniform security features across platforms one
>> option should be used on all platforms.
>> 
>> Thanks
>> 
>> Michal
>> 
>> Previous revision: https://lore.kernel.org/linuxppc-dev/cover.1637862358.git.msuchanek@suse.de/
>> Patched kernel tree: https://github.com/hramrach/kernel/tree/kexec_sig
>> 
>> Michal Suchanek (6):
>>   s390/kexec_file: Don't opencode appended signature check.
>>   powerpc/kexec_file: Add KEXEC_SIG support.
>>   kexec_file: Don't opencode appended signature verification.
>>   module: strip the signature marker in the verification function.
>>   module: Use key_being_used_for for log messages in
>>     verify_appended_signature
>>   module: Move duplicate mod_check_sig users code to mod_parse_sig
>
> What tree should this go through? I'd prefer if over through modules
> tree as it can give a chance for Aaron Tomlin to work with this for his
> code refactoring of kernel/module*.c to kernel/module/

Yeah that's fine by me, the arch changes are pretty minimal and unlikely
to conflict much.

cheers

^ permalink raw reply

* Re: [PATCH v5 2/6] powerpc/kexec_file: Add KEXEC_SIG support.
From: Michael Ellerman @ 2022-02-09  4:43 UTC (permalink / raw)
  To: Michal Suchanek, keyrings, linux-crypto, linux-integrity
  Cc: Nayna, Mimi Zohar, David Howells, Paul Mackerras,
	Alexander Gordeev, Rob Herring, Herbert Xu, Baoquan He,
	Christian Borntraeger, James Morris, Lakshmi Ramasubramanian,
	Michal Suchanek, Serge E. Hallyn, Vasily Gorbik, linux-s390,
	Heiko Carstens, Dmitry Kasatkin, Hari Bathini, Daniel Axtens,
	Christian Borntraeger, Philipp Rudo, Frank van der Linden, kexec,
	linux-kernel, Luis Chamberlain, Sven Schnelle,
	linux-security-module, Jessica Yu, linuxppc-dev, David S. Miller,
	Thiago Jung Bauermann, buendgen
In-Reply-To: <d95f7c6865bcad5ee37dcbec240e79aa742f5e1d.1641900831.git.msuchanek@suse.de>

Michal Suchanek <msuchanek@suse.de> writes:
> Copy the code from s390x
>
> Both powerpc and s390x use appended signature format (as opposed to EFI
> based patforms using PE format).
>
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
> v3: - Philipp Rudo <prudo@redhat.com>: Update the comit message with
>       explanation why the s390 code is usable on powerpc.
>     - Include correct header for mod_check_sig
>     - Nayna <nayna@linux.vnet.ibm.com>: Mention additional IMA features
>       in kconfig text
> ---
>  arch/powerpc/Kconfig        | 16 ++++++++++++++++
>  arch/powerpc/kexec/elf_64.c | 36 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 52 insertions(+)

I haven't tested this on powerpc, but assuming you have Michal this
looks OK to me.

Acked-by: Michael Ellerman <mpe@ellerman.id.au>

cheers

^ permalink raw reply

* [PATCH] ocxl: Make use of the helper macro LIST_HEAD()
From: Cai Huoqing @ 2022-02-09  3:24 UTC (permalink / raw)
  To: cai.huoqing
  Cc: Andrew Donnellan, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel,
	Frederic Barrat, linuxppc-dev

Replace "struct list_head head = LIST_HEAD_INIT(head)" with
"LIST_HEAD(head)" to simplify the code.

Signed-off-by: Cai Huoqing <cai.huoqing@linux.dev>
---
 drivers/misc/ocxl/link.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
index ab039c115381..9670d02c927f 100644
--- a/drivers/misc/ocxl/link.c
+++ b/drivers/misc/ocxl/link.c
@@ -94,7 +94,7 @@ struct ocxl_link {
 	struct spa *spa;
 	void *platform_data;
 };
-static struct list_head links_list = LIST_HEAD_INIT(links_list);
+static LIST_HEAD(links_list);
 static DEFINE_MUTEX(links_list_lock);
 
 enum xsl_response {
-- 
2.25.1


^ permalink raw reply related

* [PATCH] oc: fsl: dpio: Make use of the helper macro LIST_HEAD()
From: Cai Huoqing @ 2022-02-09  3:23 UTC (permalink / raw)
  To: cai.huoqing
  Cc: Roy Pledge, linuxppc-dev, linux-kernel, linux-arm-kernel, Li Yang

Replace "struct list_head head = LIST_HEAD_INIT(head)" with
"LIST_HEAD(head)" to simplify the code.

Signed-off-by: Cai Huoqing <cai.huoqing@linux.dev>
---
 drivers/soc/fsl/dpio/dpio-service.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/fsl/dpio/dpio-service.c b/drivers/soc/fsl/dpio/dpio-service.c
index 1d2b27e3ea63..36f0a9b799b1 100644
--- a/drivers/soc/fsl/dpio/dpio-service.c
+++ b/drivers/soc/fsl/dpio/dpio-service.c
@@ -51,7 +51,7 @@ struct dpaa2_io_store {
 
 /* keep a per cpu array of DPIOs for fast access */
 static struct dpaa2_io *dpio_by_cpu[NR_CPUS];
-static struct list_head dpio_list = LIST_HEAD_INIT(dpio_list);
+static LIST_HEAD(dpio_list);
 static DEFINE_SPINLOCK(dpio_list_lock);
 
 static inline struct dpaa2_io *service_select_by_cpu(struct dpaa2_io *d,
-- 
2.25.1


^ permalink raw reply related

* Re: [PATCH] scsi: ibmvfc: replace snprintf with sysfs_emit
From: Damien Le Moal @ 2022-02-09  2:40 UTC (permalink / raw)
  To: davidcomponentone, tyreld
  Cc: martin.petersen, linux-scsi, jejb, linux-kernel, Yang Guang,
	paulus, linuxppc-dev, Zeal Robot
In-Reply-To: <b4c150c86f539d3bac3fc8885252adb9f24ee48f.1644286482.git.yang.guang5@zte.com.cn>

On 2/9/22 09:43, davidcomponentone@gmail.com wrote:
> From: Yang Guang <yang.guang5@zte.com.cn>
> 
> coccinelle report:
> ./drivers/scsi/ibmvscsi/ibmvfc.c:3453:8-16:
> WARNING: use scnprintf or sprintf
> ./drivers/scsi/ibmvscsi/ibmvfc.c:3416:8-16:
> WARNING: use scnprintf or sprintf
> ./drivers/scsi/ibmvscsi/ibmvfc.c:3436:8-16:
> WARNING: use scnprintf or sprintf
> ./drivers/scsi/ibmvscsi/ibmvfc.c:3426:8-16:
> WARNING: use scnprintf or sprintf
> ./drivers/scsi/ibmvscsi/ibmvfc.c:3445:8-16:
> WARNING: use scnprintf or sprintf
> ./drivers/scsi/ibmvscsi/ibmvfc.c:3406:8-16:
> WARNING: use scnprintf or sprintf
> 
> Use sysfs_emit instead of scnprintf or sprintf makes more sense.
> 
> Reported-by: Zeal Robot <zealci@zte.com.cn>
> Signed-off-by: Yang Guang <yang.guang5@zte.com.cn>
> Signed-off-by: David Yang <davidcomponentone@gmail.com>
> ---
>  drivers/scsi/ibmvscsi/ibmvfc.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
> index d0eab5700dc5..d5a197d17e0a 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
> @@ -3403,7 +3403,7 @@ static ssize_t ibmvfc_show_host_partition_name(struct device *dev,
>  	struct Scsi_Host *shost = class_to_shost(dev);
>  	struct ibmvfc_host *vhost = shost_priv(shost);
>  
> -	return snprintf(buf, PAGE_SIZE, "%s\n",
> +	return sysfs_emit(buf, "%s\n",
>  			vhost->login_buf->resp.partition_name);
>  }
>  
> @@ -3413,7 +3413,7 @@ static ssize_t ibmvfc_show_host_device_name(struct device *dev,
>  	struct Scsi_Host *shost = class_to_shost(dev);
>  	struct ibmvfc_host *vhost = shost_priv(shost);
>  
> -	return snprintf(buf, PAGE_SIZE, "%s\n",
> +	return sysfs_emit(buf, "%s\n",
>  			vhost->login_buf->resp.device_name);
>  }
>  
> @@ -3423,7 +3423,7 @@ static ssize_t ibmvfc_show_host_loc_code(struct device *dev,
>  	struct Scsi_Host *shost = class_to_shost(dev);
>  	struct ibmvfc_host *vhost = shost_priv(shost);
>  
> -	return snprintf(buf, PAGE_SIZE, "%s\n",
> +	return sysfs_emit(buf, "%s\n",
>  			vhost->login_buf->resp.port_loc_code);
>  }
>  
> @@ -3433,7 +3433,7 @@ static ssize_t ibmvfc_show_host_drc_name(struct device *dev,
>  	struct Scsi_Host *shost = class_to_shost(dev);
>  	struct ibmvfc_host *vhost = shost_priv(shost);
>  
> -	return snprintf(buf, PAGE_SIZE, "%s\n",
> +	return sysfs_emit(buf, "%s\n",
>  			vhost->login_buf->resp.drc_name);
>  }
>  
> @@ -3442,7 +3442,7 @@ static ssize_t ibmvfc_show_host_npiv_version(struct device *dev,
>  {
>  	struct Scsi_Host *shost = class_to_shost(dev);
>  	struct ibmvfc_host *vhost = shost_priv(shost);
> -	return snprintf(buf, PAGE_SIZE, "%d\n", be32_to_cpu(vhost->login_buf->resp.version));
> +	return sysfs_emit(buf, "%d\n", be32_to_cpu(vhost->login_buf->resp.version));

The format should be %u, not %d. And while at it, please add a blank
line after the declarations.

>  }
>  
>  static ssize_t ibmvfc_show_host_capabilities(struct device *dev,
> @@ -3450,7 +3450,7 @@ static ssize_t ibmvfc_show_host_capabilities(struct device *dev,
>  {
>  	struct Scsi_Host *shost = class_to_shost(dev);
>  	struct ibmvfc_host *vhost = shost_priv(shost);
> -	return snprintf(buf, PAGE_SIZE, "%llx\n", be64_to_cpu(vhost->login_buf->resp.capabilities));
> +	return sysfs_emit(buf, "%llx\n", be64_to_cpu(vhost->login_buf->resp.capabilities));
>  }

Ditto for the blank line.

>  
>  /**
> @@ -3471,7 +3471,7 @@ static ssize_t ibmvfc_show_log_level(struct device *dev,
>  	int len;
>  
>  	spin_lock_irqsave(shost->host_lock, flags);
> -	len = snprintf(buf, PAGE_SIZE, "%d\n", vhost->log_level);
> +	len = sysfs_emit(buf, "%d\n", vhost->log_level);
>  	spin_unlock_irqrestore(shost->host_lock, flags);
>  	return len;
>  }
> @@ -3509,7 +3509,7 @@ static ssize_t ibmvfc_show_scsi_channels(struct device *dev,
>  	int len;
>  
>  	spin_lock_irqsave(shost->host_lock, flags);
> -	len = snprintf(buf, PAGE_SIZE, "%d\n", vhost->client_scsi_channels);
> +	len = sysfs_emit(buf, "%d\n", vhost->client_scsi_channels);
>  	spin_unlock_irqrestore(shost->host_lock, flags);
>  	return len;
>  }


-- 
Damien Le Moal
Western Digital Research

^ permalink raw reply

* Re: [PATCH] scsi: ibmvfc: replace snprintf with sysfs_emit
From: Joe Perches @ 2022-02-09  2:06 UTC (permalink / raw)
  To: davidcomponentone, tyreld
  Cc: martin.petersen, linux-scsi, jejb, linux-kernel, Yang Guang,
	paulus, linuxppc-dev, Zeal Robot
In-Reply-To: <b4c150c86f539d3bac3fc8885252adb9f24ee48f.1644286482.git.yang.guang5@zte.com.cn>

On Wed, 2022-02-09 at 08:43 +0800, davidcomponentone@gmail.com wrote:
> From: Yang Guang <yang.guang5@zte.com.cn>
[]
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
[]
> @@ -3403,7 +3403,7 @@ static ssize_t ibmvfc_show_host_partition_name(struct device *dev,
>  	struct Scsi_Host *shost = class_to_shost(dev);
>  	struct ibmvfc_host *vhost = shost_priv(shost);
>  
> -	return snprintf(buf, PAGE_SIZE, "%s\n",
> +	return sysfs_emit(buf, "%s\n",
>  			vhost->login_buf->resp.partition_name);

Rewrap please

	return sysfs_emit(buf, "%s\n", vhost->login_buf->resp.partition_name);

>  }
>  
> @@ -3413,7 +3413,7 @@ static ssize_t ibmvfc_show_host_device_name(struct device *dev,
>  	struct Scsi_Host *shost = class_to_shost(dev);
>  	struct ibmvfc_host *vhost = shost_priv(shost);
>  
> -	return snprintf(buf, PAGE_SIZE, "%s\n",
> +	return sysfs_emit(buf, "%s\n",
>  			vhost->login_buf->resp.device_name);

etc...



^ permalink raw reply

* Re: [PATCH v7 0/5] Allow guest access to EFI confidential computing secret area
From: Nayna @ 2022-02-09  0:19 UTC (permalink / raw)
  To: Greg KH, linux-efi
  Cc: linux-efi, Brijesh Singh, Matthew Garrett, 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: <Yfo/5gYgb9Sv24YB@kroah.com>

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


On 2/2/22 03:25, Greg KH wrote:
> 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 for adding us to this discussion.

If I have understood the discussion right, the key idea discussed here 
is to unify multiple different interfaces(this one, and [1]) exposing 
secrets for confidential computing usecase via securityfs.

And the suggestion is to see if the proposed pseries interface [2] can 
unify with the coco interface.

At high level, pseries interface is reading/writing/adding/deleting 
variables using the sysfs interface, but the underlying semantics and 
actual usecases are quite different.

The variables exposed via pseries proposed interface are:

* Variables owned by firmware as read-only.
* Variables owned by bootloader as read-only.
* Variables owned by OS and get updated as signed updates. These support 
both read/write.
* Variables owned by OS and get directly updated(unsigned) eg config 
information or some boot variables. These support both read/write.

It can be extended to support variables which contain secrets like 
symmetric keys, are owned by OS and stored in platform keystore.

Naming convention wise also, there are differences like pseries 
variables do not use GUIDs.

The initial patchset discusses secure boot usecase, but it would be 
extended for other usecases as well.

First, I feel the purpose itself is different here. If we still 
continue, I fear if we will get into similar situation as Matthew 
mentioned in context of efivars -

"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."

With that, I believe the scope of pseries interface is different and 
much broader than being discussed here. So, I wonder if it would be 
better to still keep pseries interface separate from this and have its 
own platform specific interface.

I would be happy to hear the ideas.

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

[2] https://lore.kernel.org/all/20220122005637.28199-1-nayna@linux.ibm.com/

Thanks & Regards,

      - Nayna

[-- Attachment #2: Type: text/html, Size: 7117 bytes --]

^ permalink raw reply

* [PATCH] scsi: ibmvfc: replace snprintf with sysfs_emit
From: davidcomponentone @ 2022-02-09  0:43 UTC (permalink / raw)
  To: tyreld
  Cc: linux-scsi, martin.petersen, jejb, davidcomponentone,
	linux-kernel, Yang Guang, paulus, linuxppc-dev, Zeal Robot

From: Yang Guang <yang.guang5@zte.com.cn>

coccinelle report:
./drivers/scsi/ibmvscsi/ibmvfc.c:3453:8-16:
WARNING: use scnprintf or sprintf
./drivers/scsi/ibmvscsi/ibmvfc.c:3416:8-16:
WARNING: use scnprintf or sprintf
./drivers/scsi/ibmvscsi/ibmvfc.c:3436:8-16:
WARNING: use scnprintf or sprintf
./drivers/scsi/ibmvscsi/ibmvfc.c:3426:8-16:
WARNING: use scnprintf or sprintf
./drivers/scsi/ibmvscsi/ibmvfc.c:3445:8-16:
WARNING: use scnprintf or sprintf
./drivers/scsi/ibmvscsi/ibmvfc.c:3406:8-16:
WARNING: use scnprintf or sprintf

Use sysfs_emit instead of scnprintf or sprintf makes more sense.

Reported-by: Zeal Robot <zealci@zte.com.cn>
Signed-off-by: Yang Guang <yang.guang5@zte.com.cn>
Signed-off-by: David Yang <davidcomponentone@gmail.com>
---
 drivers/scsi/ibmvscsi/ibmvfc.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index d0eab5700dc5..d5a197d17e0a 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -3403,7 +3403,7 @@ static ssize_t ibmvfc_show_host_partition_name(struct device *dev,
 	struct Scsi_Host *shost = class_to_shost(dev);
 	struct ibmvfc_host *vhost = shost_priv(shost);
 
-	return snprintf(buf, PAGE_SIZE, "%s\n",
+	return sysfs_emit(buf, "%s\n",
 			vhost->login_buf->resp.partition_name);
 }
 
@@ -3413,7 +3413,7 @@ static ssize_t ibmvfc_show_host_device_name(struct device *dev,
 	struct Scsi_Host *shost = class_to_shost(dev);
 	struct ibmvfc_host *vhost = shost_priv(shost);
 
-	return snprintf(buf, PAGE_SIZE, "%s\n",
+	return sysfs_emit(buf, "%s\n",
 			vhost->login_buf->resp.device_name);
 }
 
@@ -3423,7 +3423,7 @@ static ssize_t ibmvfc_show_host_loc_code(struct device *dev,
 	struct Scsi_Host *shost = class_to_shost(dev);
 	struct ibmvfc_host *vhost = shost_priv(shost);
 
-	return snprintf(buf, PAGE_SIZE, "%s\n",
+	return sysfs_emit(buf, "%s\n",
 			vhost->login_buf->resp.port_loc_code);
 }
 
@@ -3433,7 +3433,7 @@ static ssize_t ibmvfc_show_host_drc_name(struct device *dev,
 	struct Scsi_Host *shost = class_to_shost(dev);
 	struct ibmvfc_host *vhost = shost_priv(shost);
 
-	return snprintf(buf, PAGE_SIZE, "%s\n",
+	return sysfs_emit(buf, "%s\n",
 			vhost->login_buf->resp.drc_name);
 }
 
@@ -3442,7 +3442,7 @@ static ssize_t ibmvfc_show_host_npiv_version(struct device *dev,
 {
 	struct Scsi_Host *shost = class_to_shost(dev);
 	struct ibmvfc_host *vhost = shost_priv(shost);
-	return snprintf(buf, PAGE_SIZE, "%d\n", be32_to_cpu(vhost->login_buf->resp.version));
+	return sysfs_emit(buf, "%d\n", be32_to_cpu(vhost->login_buf->resp.version));
 }
 
 static ssize_t ibmvfc_show_host_capabilities(struct device *dev,
@@ -3450,7 +3450,7 @@ static ssize_t ibmvfc_show_host_capabilities(struct device *dev,
 {
 	struct Scsi_Host *shost = class_to_shost(dev);
 	struct ibmvfc_host *vhost = shost_priv(shost);
-	return snprintf(buf, PAGE_SIZE, "%llx\n", be64_to_cpu(vhost->login_buf->resp.capabilities));
+	return sysfs_emit(buf, "%llx\n", be64_to_cpu(vhost->login_buf->resp.capabilities));
 }
 
 /**
@@ -3471,7 +3471,7 @@ static ssize_t ibmvfc_show_log_level(struct device *dev,
 	int len;
 
 	spin_lock_irqsave(shost->host_lock, flags);
-	len = snprintf(buf, PAGE_SIZE, "%d\n", vhost->log_level);
+	len = sysfs_emit(buf, "%d\n", vhost->log_level);
 	spin_unlock_irqrestore(shost->host_lock, flags);
 	return len;
 }
@@ -3509,7 +3509,7 @@ static ssize_t ibmvfc_show_scsi_channels(struct device *dev,
 	int len;
 
 	spin_lock_irqsave(shost->host_lock, flags);
-	len = snprintf(buf, PAGE_SIZE, "%d\n", vhost->client_scsi_channels);
+	len = sysfs_emit(buf, "%d\n", vhost->client_scsi_channels);
 	spin_unlock_irqrestore(shost->host_lock, flags);
 	return len;
 }
-- 
2.30.2


^ permalink raw reply related

* Re: [PATCH v7 0/5] Allow guest access to EFI confidential computing secret area
From: Nayna @ 2022-02-09  0:25 UTC (permalink / raw)
  To: Greg KH, Matthew Garrett, linux-efi
  Cc: 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: <Yfo/5gYgb9Sv24YB@kroah.com>


On 2/2/22 03:25, Greg KH wrote:
> 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 for adding us to this discussion. I think somehow my last post 
got html content and didn't make to mailing list, so am posting it 
again. Sorry to those who are receiving it twice.

If I have understood the discussion right, the key idea discussed here 
is to unify multiple different interfaces(this one, and [1]) exposing 
secrets for confidential computing usecase via securityfs.

And the suggestion is to see if the proposed pseries interface [2] can 
unify with the coco interface.

At high level, pseries interface is reading/writing/adding/deleting 
variables using the sysfs interface, but the underlying semantics and 
actual usecases are quite different.

The variables exposed via pseries proposed interface are:

* Variables owned by firmware as read-only.
* Variables owned by bootloader as read-only.
* Variables owned by OS and get updated as signed updates. These support 
both read/write.
* Variables owned by OS and get directly updated(unsigned) eg config 
information or some boot variables. These support both read/write.

It can be extended to support variables which contain secrets like 
symmetric keys, are owned by OS and stored in platform keystore.

Naming convention wise also, there are differences like pseries 
variables do not use GUIDs.

The initial patchset discusses secure boot usecase, but it would be 
extended for other usecases as well.

First, I feel the purpose itself is different here. If we still 
continue, I fear if we will get into similar situation as Matthew 
mentioned in context of efivars -

"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."

With that, I believe the scope of pseries interface is different and 
much broader than being discussed here. So, I wonder if it would be 
better to still keep pseries interface separate from this and have its 
own platform specific interface.

I would be happy to hear the ideas.

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

[2] https://lore.kernel.org/all/20220122005637.28199-1-nayna@linux.ibm.com/

Thanks & Regards,

      - Nayna


^ permalink raw reply

* Re: [PATCH v2] include: linux: Reorganize timekeeping and ktime headers
From: kernel test robot @ 2022-02-08 23:10 UTC (permalink / raw)
  To: Carlos Bilbao, john.stultz, tglx, sboyd, alexandre.belloni,
	gregkh
  Cc: linux-rtc, jgross, kbuild-all, kernel test robot, linux-kernel,
	rostedt, Carlos Bilbao, linux-m68k, geert, bilbao, boon.leong.ong,
	linux-ia64, linuxppc-dev, linux-stm32, linux-arm-kernel, mhiramat
In-Reply-To: <20220208161049.865402-1-carlos.bilbao@amd.com>

Hi Carlos,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on geert-m68k/for-next]
[also build test ERROR on tip/timers/core tip/x86/core linus/master v5.17-rc3]
[cannot apply to next-20220208]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Carlos-Bilbao/include-linux-Reorganize-timekeeping-and-ktime-headers/20220209-001309
base:   https://git.kernel.org/pub/scm/linux/kernel/git/geert/linux-m68k.git for-next
config: alpha-randconfig-r001-20220208 (https://download.01.org/0day-ci/archive/20220209/202202090656.Bx5FpSa7-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/5ed7d76f2d6aabedc437bc0b99020dc655ab5719
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Carlos-Bilbao/include-linux-Reorganize-timekeeping-and-ktime-headers/20220209-001309
        git checkout 5ed7d76f2d6aabedc437bc0b99020dc655ab5719
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=alpha SHELL=/bin/bash arch/alpha/kernel/

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

All errors (new ones prefixed by >>):

   arch/alpha/kernel/osf_sys.c: In function '__do_sys_osf_settimeofday':
>> arch/alpha/kernel/osf_sys.c:1013:16: error: implicit declaration of function 'do_sys_settimeofday64'; did you mean 'sys_settimeofday'? [-Werror=implicit-function-declaration]
    1013 |         return do_sys_settimeofday64(tv ? &kts : NULL, tz ? &ktz : NULL);
         |                ^~~~~~~~~~~~~~~~~~~~~
         |                sys_settimeofday
   cc1: some warnings being treated as errors


vim +1013 arch/alpha/kernel/osf_sys.c

^1da177e4c3f415 Linus Torvalds  2005-04-16   997  
e5d9a90c36e05dd Ivan Kokshaysky 2009-01-29   998  SYSCALL_DEFINE2(osf_settimeofday, struct timeval32 __user *, tv,
e5d9a90c36e05dd Ivan Kokshaysky 2009-01-29   999  		struct timezone __user *, tz)
^1da177e4c3f415 Linus Torvalds  2005-04-16  1000  {
ce4c253573ad184 Arnd Bergmann   2017-11-08  1001  	struct timespec64 kts;
^1da177e4c3f415 Linus Torvalds  2005-04-16  1002  	struct timezone ktz;
^1da177e4c3f415 Linus Torvalds  2005-04-16  1003  
^1da177e4c3f415 Linus Torvalds  2005-04-16  1004   	if (tv) {
ce4c253573ad184 Arnd Bergmann   2017-11-08  1005  		if (get_tv32(&kts, tv))
^1da177e4c3f415 Linus Torvalds  2005-04-16  1006  			return -EFAULT;
^1da177e4c3f415 Linus Torvalds  2005-04-16  1007  	}
^1da177e4c3f415 Linus Torvalds  2005-04-16  1008  	if (tz) {
^1da177e4c3f415 Linus Torvalds  2005-04-16  1009  		if (copy_from_user(&ktz, tz, sizeof(*tz)))
^1da177e4c3f415 Linus Torvalds  2005-04-16  1010  			return -EFAULT;
^1da177e4c3f415 Linus Torvalds  2005-04-16  1011  	}
^1da177e4c3f415 Linus Torvalds  2005-04-16  1012  
ce4c253573ad184 Arnd Bergmann   2017-11-08 @1013  	return do_sys_settimeofday64(tv ? &kts : NULL, tz ? &ktz : NULL);
^1da177e4c3f415 Linus Torvalds  2005-04-16  1014  }
^1da177e4c3f415 Linus Torvalds  2005-04-16  1015  

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

^ permalink raw reply

* Re: [PATCH v2 1/2] lib/raid6/test/Makefile: Use `$(pound)` instead of `\#` for Make 4.3
From: Wols Lists @ 2022-02-08 22:12 UTC (permalink / raw)
  To: Paul Menzel, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh
  Cc: linux-kernel, Matt Brown, linux-raid, Song Liu, netdev, bpf,
	linuxppc-dev
In-Reply-To: <20220208152148.48534-1-pmenzel@molgen.mpg.de>

On 08/02/2022 15:21, Paul Menzel wrote:
> So, do the same as commit 9564a8cf422d ("Kbuild: fix # escaping in .cmd
> files for future Make") and commit 929bef467771 ("bpf: Use $(pound) instead
> of \# in Makefiles") and define and use a `$(pound)` variable.

As commented elsewhere, for the sake of us ENGLISH speakers, *PLEASE* 
make that $(hash). A pound sign is £.

Cheers,
Wol

^ permalink raw reply

* Re: [PATCH v2] include: linux: Reorganize timekeeping and ktime headers
From: kernel test robot @ 2022-02-08 21:48 UTC (permalink / raw)
  To: Carlos Bilbao, john.stultz, tglx, sboyd, alexandre.belloni,
	gregkh
  Cc: linux-rtc, jgross, kbuild-all, kernel test robot, linux-kernel,
	rostedt, Carlos Bilbao, linux-m68k, geert, bilbao, boon.leong.ong,
	linux-ia64, linuxppc-dev, linux-stm32, linux-arm-kernel, mhiramat
In-Reply-To: <20220208161049.865402-1-carlos.bilbao@amd.com>

Hi Carlos,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on geert-m68k/for-next]
[also build test ERROR on tip/timers/core tip/x86/core linus/master v5.17-rc3]
[cannot apply to next-20220208]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Carlos-Bilbao/include-linux-Reorganize-timekeeping-and-ktime-headers/20220209-001309
base:   https://git.kernel.org/pub/scm/linux/kernel/git/geert/linux-m68k.git for-next
config: m68k-randconfig-r024-20220208 (https://download.01.org/0day-ci/archive/20220209/202202090554.VWOt2B2w-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/5ed7d76f2d6aabedc437bc0b99020dc655ab5719
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Carlos-Bilbao/include-linux-Reorganize-timekeeping-and-ktime-headers/20220209-001309
        git checkout 5ed7d76f2d6aabedc437bc0b99020dc655ab5719
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=m68k SHELL=/bin/bash

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

All errors (new ones prefixed by >>):

   arch/m68k/68000/timers.c: In function 'hw_tick':
>> arch/m68k/68000/timers.c:64:9: error: implicit declaration of function 'legacy_timer_tick' [-Werror=implicit-function-declaration]
      64 |         legacy_timer_tick(1);
         |         ^~~~~~~~~~~~~~~~~
   arch/m68k/68000/timers.c: At top level:
   arch/m68k/68000/timers.c:120:5: warning: no previous prototype for 'm68328_hwclk' [-Wmissing-prototypes]
     120 | int m68328_hwclk(int set, struct rtc_time *t)
         |     ^~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/legacy_timer_tick +64 arch/m68k/68000/timers.c

36a90f26aa24c5 arch/m68knommu/platform/68328/timers.c Greg Ungerer  2008-02-01  57  
36a90f26aa24c5 arch/m68knommu/platform/68328/timers.c Greg Ungerer  2008-02-01  58  static irqreturn_t hw_tick(int irq, void *dummy)
36a90f26aa24c5 arch/m68knommu/platform/68328/timers.c Greg Ungerer  2008-02-01  59  {
36a90f26aa24c5 arch/m68knommu/platform/68328/timers.c Greg Ungerer  2008-02-01  60  	/* Reset Timer1 */
36a90f26aa24c5 arch/m68knommu/platform/68328/timers.c Greg Ungerer  2008-02-01  61  	TSTAT &= 0;
36a90f26aa24c5 arch/m68knommu/platform/68328/timers.c Greg Ungerer  2008-02-01  62  
36a90f26aa24c5 arch/m68knommu/platform/68328/timers.c Greg Ungerer  2008-02-01  63  	m68328_tick_cnt += TICKS_PER_JIFFY;
09323308f63708 arch/m68k/68000/timers.c               Arnd Bergmann 2020-09-24 @64  	legacy_timer_tick(1);
09323308f63708 arch/m68k/68000/timers.c               Arnd Bergmann 2020-09-24  65  	return IRQ_HANDLED;
36a90f26aa24c5 arch/m68knommu/platform/68328/timers.c Greg Ungerer  2008-02-01  66  }
36a90f26aa24c5 arch/m68knommu/platform/68328/timers.c Greg Ungerer  2008-02-01  67  

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

^ 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