Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <ed2621061f694099c2fd3edf39d239686a84f517.camel@linux.ibm.com>
Subject: Re: [PATCH v4 07/16] powerpc/pseries/vas: Define VAS/NXGZIP HCALLs
 and structs
From:   Haren Myneni <haren@linux.ibm.com>
To:     Nicholas Piggin <npiggin@gmail.com>, herbert@gondor.apana.org.au,
        linux-crypto@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
        mpe@ellerman.id.au
Cc:     haren@us.ibm.com, hbabu@us.ibm.com
Date:   Thu, 03 Jun 2021 18:30:43 -0700
In-Reply-To: <1622695206.q32wg4puuh.astroid@bobo.none>
References: <8d219c0816133a8643d650709066cf04c9c77322.camel@linux.ibm.com>
         <4d1a19311883c2ac6620633721ecc81d753f26c8.camel@linux.ibm.com>
         <1622695206.q32wg4puuh.astroid@bobo.none>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.36.2 (3.36.2-1.fc32) 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Thu, 2021-06-03 at 14:47 +1000, Nicholas Piggin wrote:
> Excerpts from Haren Myneni's message of May 21, 2021 7:34 pm:
> > This patch adds HCALLs and other definitions. Also define structs
> > that are used in VAS implementation on powerVM.
> > 
> > Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> > ---
> >  arch/powerpc/include/asm/hvcall.h    |   7 ++
> >  arch/powerpc/include/asm/vas.h       |  32 ++++++++
> >  arch/powerpc/platforms/pseries/vas.h | 110
> > +++++++++++++++++++++++++++
> >  3 files changed, 149 insertions(+)
> >  create mode 100644 arch/powerpc/platforms/pseries/vas.h
> > 
> > diff --git a/arch/powerpc/include/asm/hvcall.h
> > b/arch/powerpc/include/asm/hvcall.h
> > index e3b29eda8074..7c3418d1b5e9 100644
> > --- a/arch/powerpc/include/asm/hvcall.h
> > +++ b/arch/powerpc/include/asm/hvcall.h
> > @@ -294,6 +294,13 @@
> >  #define H_RESIZE_HPT_COMMIT	0x370
> >  #define H_REGISTER_PROC_TBL	0x37C
> >  #define H_SIGNAL_SYS_RESET	0x380
> > +#define H_ALLOCATE_VAS_WINDOW	0x388
> > +#define H_MODIFY_VAS_WINDOW	0x38C
> > +#define H_DEALLOCATE_VAS_WINDOW	0x390
> > +#define H_QUERY_VAS_WINDOW	0x394
> > +#define H_QUERY_VAS_CAPABILITIES	0x398
> > +#define H_QUERY_NX_CAPABILITIES	0x39C
> > +#define H_GET_NX_FAULT		0x3A0
> >  #define H_INT_GET_SOURCE_INFO   0x3A8
> >  #define H_INT_SET_SOURCE_CONFIG 0x3AC
> >  #define H_INT_GET_SOURCE_CONFIG 0x3B0
> > diff --git a/arch/powerpc/include/asm/vas.h
> > b/arch/powerpc/include/asm/vas.h
> > index 49bfb5be896d..371f62d99174 100644
> > --- a/arch/powerpc/include/asm/vas.h
> > +++ b/arch/powerpc/include/asm/vas.h
> > @@ -181,6 +181,7 @@ struct vas_tx_win_attr {
> >  	bool rx_win_ord_mode;
> >  };
> >  
> > +#ifdef CONFIG_PPC_POWERNV
> >  /*
> >   * Helper to map a chip id to VAS id.
> >   * For POWER9, this is a 1:1 mapping. In the future this maybe a
> > 1:N
> > @@ -248,6 +249,37 @@ void vas_win_paste_addr(struct vas_window
> > *window, u64 *addr,
> >  int vas_register_api_powernv(struct module *mod, enum vas_cop_type
> > cop_type,
> >  			     const char *name);
> >  void vas_unregister_api_powernv(void);
> > +#endif
> > +
> > +#ifdef CONFIG_PPC_PSERIES
> > +
> > +/* VAS Capabilities */
> > +#define VAS_GZIP_QOS_FEAT	0x1
> > +#define VAS_GZIP_DEF_FEAT	0x2
> > +#define VAS_GZIP_QOS_FEAT_BIT	PPC_BIT(VAS_GZIP_QOS_FEAT) /*
> > Bit 1 */
> > +#define VAS_GZIP_DEF_FEAT_BIT	PPC_BIT(VAS_GZIP_DEF_FEAT) /*
> > Bit 2 */
> > +
> > +/* NX Capabilities */
> > +#define VAS_NX_GZIP_FEAT	0x1
> > +#define VAS_NX_GZIP_FEAT_BIT	PPC_BIT(VAS_NX_GZIP_FEAT) /*
> > Bit 1 */
> > +#define VAS_DESCR_LEN		8
> > +
> > +/*
> > + * These structs are used to retrieve overall VAS capabilities
> > that
> > + * the hypervisor provides.
> > + */
> > +struct hv_vas_all_caps {
> 
> ...
> 
> > +
> > +/*
> > + * Use to get feature specific capabilities from the
> > + * hypervisor.
> > + */
> > +struct hv_vas_ct_caps {
> 
> ...
> 
> > +/*
> > + * To get window information from the hypervisor.
> > + */
> > +struct hv_vas_win_lpar {
> 
> Are any of these names coming from PAPR? If not, then typically we
> don't 
> use hv_ kind of prefixes for something we got from the hypervisor,
> but
> rather something that's hypervisor privileged or specific information
> about the hypervisor perhaps (which is not the same as what the 
> hypervisor may or may not advertise to the guest).
> 
> So if these are all capabilities and features the hypervisor
> advertises 
> to the LPAR, I think the hv_ should be dropped.

The hypervisor advertises the available capabalities and the LPAR
passes buffer to HCALLs to retrieve these capabilties / features (in
BE). 

I was using *_be in the previous version. So can I use like 'struct
vas_ct_caps_be'  

> 
> Otherwise seems okay. I would be careful of the "lpar" name. I think 
> it's okay in this situation where the hypervisor advertises features
> to 
> the partition, but in other parts of the vas code you call it
> pseries_
> but you also have some lpar_ bits. So aside from interacting with
> PAPR
> APIs, it would be safe to consistently use pseries for your driver
> and
> type names.

I can use 'struct hv_vas_win_pseries' or 'struct vas_win_pseries_be'

> 
> Thanks,
> Nick