All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Joyce <gjoyce@linux.vnet.ibm.com>
To: "Michal Suchánek" <msuchanek@suse.de>, Nayna <nayna@linux.vnet.ibm.com>
Cc: axboe@kernel.dk, nayna@linux.ibm.com,
	linux-block@vger.kernel.org, jonathan.derrick@linux.dev,
	brking@linux.vnet.ibm.com, akpm@linux-foundation.org,
	linuxppc-dev@lists.ozlabs.org, gjoyce@ibm.com,
	Michael Ellerman <mpe@ellerman.id.au>,
	Nick Piggin <npiggin@gmail.com>,
	Christophe Leroy <christophe.leroy@c-s.fr>
Subject: Re: [PATCH v3 1/2] lib: generic accessor functions for arch keystore
Date: Thu, 04 Aug 2022 10:41:00 -0500	[thread overview]
Message-ID: <af674b14a965588aa687472c9c8c74e7e471df88.camel@linux.vnet.ibm.com> (raw)
In-Reply-To: <20220801202401.GZ17705@kitsune.suse.cz>

On Mon, 2022-08-01 at 22:24 +0200, Michal Suchánek wrote:
> > > > +
> > > > +int __weak arch_read_variable(enum arch_variable_type type,
> > > > char *varname,
> > > > +			      void *varbuf, u_int *varlen)
> > > > +{
> > > > +	return -EOPNOTSUPP;
> > > > +}
> > > > +
> > > > +int __weak arch_write_variable(enum arch_variable_type type,
> > > > char *varname,
> > > > +			       void *varbuf, u_int varlen)
> > > > +{
> > > > +	return -EOPNOTSUPP;
> > > > +}
> > > > -- 
> > > Doesn't EFI already have some variables?
> > > 
> > > And even powernv?
> > > 
> > > Shouldn't this generalize the already existing variables?
> > > 
> > > Or move to powerpc and at least generalize the powerpc ones?
> > 
> > Yes, EFI and PowerNV do have variables, but I am not exactly clear
> > about
> > your reference to them in this context. What do you mean by
> > generalize
> > already existing variables ?
> > 
> > This interface is actually generalizing calls to access platform
> > specific
> > keystores. It is explained in cover letter that this patch is
> > defining
> > generic interface and these are default implementations which needs
> > to be
> > overridden by arch specific versions.  For PowerVM PLPAR Platform
> > KeyStore,
> > the arch specific version is implemented in Patch 2.
> For powervm, not powernv.
> 
> If it's not generic enough to cover even powerpc-specific keystores
> does
> such generalization even need to exist?

I believe that the interface is generic enough to cover most if not all
keystores. However, we're just implementing a PowerVM version since
that is our mandate. 

> > Access to EFI variables should be implemented by EFI arch specific
> > interface
> > and PowerNV will have to do the same if it needs to.
> 
> If such generic interface is desirable it should cover the existing
> architectures I think. Otherwise how can you tell if it's usable
> there?

Are you suggesting that we implement architecture specific
implementations for every architecture supported by Linux? I'm afraid
that we don't have the time (or skills) to do that. The intent is to
provide the "weak" versions of the interface functions so that they can
be overridden as folks have the time or inclination to provide them for
other architectures.

-Greg



WARNING: multiple messages have this Message-ID (diff)
From: Greg Joyce <gjoyce@linux.vnet.ibm.com>
To: "Michal Suchánek" <msuchanek@suse.de>, Nayna <nayna@linux.vnet.ibm.com>
Cc: axboe@kernel.dk, Christophe Leroy <christophe.leroy@c-s.fr>,
	nayna@linux.ibm.com, Nick Piggin <npiggin@gmail.com>,
	linux-block@vger.kernel.org, jonathan.derrick@linux.dev,
	brking@linux.vnet.ibm.com, akpm@linux-foundation.org,
	linuxppc-dev@lists.ozlabs.org, gjoyce@ibm.com
Subject: Re: [PATCH v3 1/2] lib: generic accessor functions for arch keystore
Date: Thu, 04 Aug 2022 10:41:00 -0500	[thread overview]
Message-ID: <af674b14a965588aa687472c9c8c74e7e471df88.camel@linux.vnet.ibm.com> (raw)
In-Reply-To: <20220801202401.GZ17705@kitsune.suse.cz>

On Mon, 2022-08-01 at 22:24 +0200, Michal Suchánek wrote:
> > > > +
> > > > +int __weak arch_read_variable(enum arch_variable_type type,
> > > > char *varname,
> > > > +			      void *varbuf, u_int *varlen)
> > > > +{
> > > > +	return -EOPNOTSUPP;
> > > > +}
> > > > +
> > > > +int __weak arch_write_variable(enum arch_variable_type type,
> > > > char *varname,
> > > > +			       void *varbuf, u_int varlen)
> > > > +{
> > > > +	return -EOPNOTSUPP;
> > > > +}
> > > > -- 
> > > Doesn't EFI already have some variables?
> > > 
> > > And even powernv?
> > > 
> > > Shouldn't this generalize the already existing variables?
> > > 
> > > Or move to powerpc and at least generalize the powerpc ones?
> > 
> > Yes, EFI and PowerNV do have variables, but I am not exactly clear
> > about
> > your reference to them in this context. What do you mean by
> > generalize
> > already existing variables ?
> > 
> > This interface is actually generalizing calls to access platform
> > specific
> > keystores. It is explained in cover letter that this patch is
> > defining
> > generic interface and these are default implementations which needs
> > to be
> > overridden by arch specific versions.  For PowerVM PLPAR Platform
> > KeyStore,
> > the arch specific version is implemented in Patch 2.
> For powervm, not powernv.
> 
> If it's not generic enough to cover even powerpc-specific keystores
> does
> such generalization even need to exist?

I believe that the interface is generic enough to cover most if not all
keystores. However, we're just implementing a PowerVM version since
that is our mandate. 

> > Access to EFI variables should be implemented by EFI arch specific
> > interface
> > and PowerNV will have to do the same if it needs to.
> 
> If such generic interface is desirable it should cover the existing
> architectures I think. Otherwise how can you tell if it's usable
> there?

Are you suggesting that we implement architecture specific
implementations for every architecture supported by Linux? I'm afraid
that we don't have the time (or skills) to do that. The intent is to
provide the "weak" versions of the interface functions so that they can
be overridden as folks have the time or inclination to provide them for
other architectures.

-Greg



  reply	other threads:[~2022-08-04 15:41 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-01 12:34 [PATCH v3 0/2] generic and PowerPC accessor functions for arch keystore gjoyce
2022-08-01 12:34 ` gjoyce
2022-08-01 12:34 ` [PATCH v3 1/2] lib: generic " gjoyce
2022-08-01 12:34   ` gjoyce
2022-08-01 13:40   ` Michal Suchánek
2022-08-01 13:40     ` Michal Suchánek
2022-08-01 19:45     ` Nayna
2022-08-01 19:45       ` Nayna
2022-08-01 20:24       ` Michal Suchánek
2022-08-01 20:24         ` Michal Suchánek
2022-08-04 15:41         ` Greg Joyce [this message]
2022-08-04 15:41           ` Greg Joyce
2022-08-02  2:59   ` Michael Ellerman
2022-08-02 22:39     ` Greg Joyce
2022-08-01 12:34 ` [PATCH v3 2/2] powerpc/pseries: Override lib/arch_vars.c functions gjoyce
2022-08-01 12:34   ` gjoyce

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=af674b14a965588aa687472c9c8c74e7e471df88.camel@linux.vnet.ibm.com \
    --to=gjoyce@linux.vnet.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=axboe@kernel.dk \
    --cc=brking@linux.vnet.ibm.com \
    --cc=christophe.leroy@c-s.fr \
    --cc=gjoyce@ibm.com \
    --cc=jonathan.derrick@linux.dev \
    --cc=linux-block@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    --cc=msuchanek@suse.de \
    --cc=nayna@linux.ibm.com \
    --cc=nayna@linux.vnet.ibm.com \
    --cc=npiggin@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.