Re: [PATCH v3 15/17] mm: pgtable: remove tlb_remove_page_ptdesc()
On Mon, 30 Dec 2024 11:12:00 +0800 Qi Zheng wrote: > > For now struct ptdesc overlaps struct page, but the goal is to have them > > separate and always operate on struct ptdesc when working with page tables. > > OK, so tlb_remove_page_ptdesc() and tlb_remove_ptdesc() are somewhat > intermediate products of the project. > > Hi Andrew, can you help remove [PATCH v3 15/17], [PATCH v3 16/17] and > [PATCH v3 17/17] from the mm-unstable branch? > > For [PATCH v3 17/17], I can send it separately later, or Kevin Brodsky > can help do this in his patch series. ;) I think it would be best if you were to send a v4 series. Please ensure that the changelogs are appropriately updated to reflect these (and any other) changes.
Re: [PATCH v3 15/17] mm: pgtable: remove tlb_remove_page_ptdesc()
Hi Andrew, On 2024/12/30 12:55, Andrew Morton wrote: On Mon, 30 Dec 2024 11:12:00 +0800 Qi Zheng wrote: For now struct ptdesc overlaps struct page, but the goal is to have them separate and always operate on struct ptdesc when working with page tables. OK, so tlb_remove_page_ptdesc() and tlb_remove_ptdesc() are somewhat intermediate products of the project. Hi Andrew, can you help remove [PATCH v3 15/17], [PATCH v3 16/17] and [PATCH v3 17/17] from the mm-unstable branch? For [PATCH v3 17/17], I can send it separately later, or Kevin Brodsky can help do this in his patch series. ;) I think it would be best if you were to send a v4 series. Please ensure that the changelogs are appropriately updated to reflect these (and any other) changes. Got it. Will send a v4 ASAP (remove [PATCH v3 15/17] and [PATCH v3 16/17], keep [PATCH v3 17/17]). Thanks!
Re: [PATCH v3 15/17] mm: pgtable: remove tlb_remove_page_ptdesc()
Hi Mike, On 2024/12/28 17:26, Mike Rapoport wrote: On Mon, Dec 23, 2024 at 05:41:01PM +0800, Qi Zheng wrote: Here we are explicitly dealing with struct page, and the following logic semms strange: tlb_remove_page_ptdesc((tlb), (page_ptdesc(pte))); tlb_remove_page_ptdesc --> tlb_remove_page(tlb, ptdesc_page(pt)); So remove tlb_remove_page_ptdesc() and make callers call tlb_remove_page() directly. Please don't. The ptdesc wrappers are there as a part of reducing the size of struct page project [1]. For now struct ptdesc overlaps struct page, but the goal is to have them separate and always operate on struct ptdesc when working with page tables. OK, so tlb_remove_page_ptdesc() and tlb_remove_ptdesc() are somewhat intermediate products of the project. Hi Andrew, can you help remove [PATCH v3 15/17], [PATCH v3 16/17] and [PATCH v3 17/17] from the mm-unstable branch? For [PATCH v3 17/17], I can send it separately later, or Kevin Brodsky can help do this in his patch series. ;) Thanks, Qi [1] https://kernelnewbies.org/MatthewWilcox/Memdescs
[PATCH v2 20/29] crypto: nx - use the new scatterwalk functions
From: Eric Biggers
- In nx_walk_and_build(), use scatterwalk_start_at_pos() instead of a
more complex way to achieve the same result.
- Also in nx_walk_and_build(), use the new functions scatterwalk_next()
which consolidates scatterwalk_clamp() and scatterwalk_map(), and use
scatterwalk_done_src() which consolidates scatterwalk_unmap(),
scatterwalk_advance(), and scatterwalk_done(). Remove unnecessary
code that seemed to be intended to advance to the next sg entry, which
is already handled by the scatterwalk functions.
Note that nx_walk_and_build() does not actually read or write the
mapped virtual address, and thus it is misusing the scatter_walk API.
It really should just access the scatterlist directly. This patch
does not try to address this existing issue.
- In nx_gca(), use memcpy_from_sglist() instead of a more complex way to
achieve the same result.
- In various functions, replace calls to scatterwalk_map_and_copy() with
memcpy_from_sglist() or memcpy_to_sglist() as appropriate. Note that
this eliminates the confusing 'out' argument (which this driver had
tried to work around by defining the missing constants for it...)
Cc: Christophe Leroy
Cc: Madhavan Srinivasan
Cc: Michael Ellerman
Cc: Naveen N Rao
Cc: Nicholas Piggin
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Eric Biggers
---
This patch is part of a long series touching many files, so I have
limited the Cc list on the full series. If you want the full series and
did not receive it, please retrieve it from lore.kernel.org.
drivers/crypto/nx/nx-aes-ccm.c | 16 ++--
drivers/crypto/nx/nx-aes-gcm.c | 17 ++---
drivers/crypto/nx/nx.c | 31 +--
drivers/crypto/nx/nx.h | 3 ---
4 files changed, 17 insertions(+), 50 deletions(-)
diff --git a/drivers/crypto/nx/nx-aes-ccm.c b/drivers/crypto/nx/nx-aes-ccm.c
index c843f4c6f684..56a0b3a67c33 100644
--- a/drivers/crypto/nx/nx-aes-ccm.c
+++ b/drivers/crypto/nx/nx-aes-ccm.c
@@ -215,17 +215,15 @@ static int generate_pat(u8 *iv,
*/
if (b1) {
memset(b1, 0, 16);
if (assoclen <= 65280) {
*(u16 *)b1 = assoclen;
- scatterwalk_map_and_copy(b1 + 2, req->src, 0,
-iauth_len, SCATTERWALK_FROM_SG);
+ memcpy_from_sglist(b1 + 2, req->src, 0, iauth_len);
} else {
*(u16 *)b1 = (u16)(0xfffe);
*(u32 *)&b1[2] = assoclen;
- scatterwalk_map_and_copy(b1 + 6, req->src, 0,
-iauth_len, SCATTERWALK_FROM_SG);
+ memcpy_from_sglist(b1 + 6, req->src, 0, iauth_len);
}
}
/* now copy any remaining AAD to scatterlist and call nx... */
if (!assoclen) {
@@ -339,13 +337,12 @@ static int ccm_nx_decrypt(struct aead_request *req,
spin_lock_irqsave(&nx_ctx->lock, irq_flags);
nbytes -= authsize;
/* copy out the auth tag to compare with later */
- scatterwalk_map_and_copy(priv->oauth_tag,
-req->src, nbytes + req->assoclen, authsize,
-SCATTERWALK_FROM_SG);
+ memcpy_from_sglist(priv->oauth_tag, req->src, nbytes + req->assoclen,
+ authsize);
rc = generate_pat(iv, req, nx_ctx, authsize, nbytes, assoclen,
csbcpb->cpb.aes_ccm.in_pat_or_b0);
if (rc)
goto out;
@@ -463,13 +460,12 @@ static int ccm_nx_encrypt(struct aead_request *req,
processed += to_process;
} while (processed < nbytes);
/* copy out the auth tag */
- scatterwalk_map_and_copy(csbcpb->cpb.aes_ccm.out_pat_or_mac,
-req->dst, nbytes + req->assoclen, authsize,
-SCATTERWALK_TO_SG);
+ memcpy_to_sglist(req->dst, nbytes + req->assoclen,
+csbcpb->cpb.aes_ccm.out_pat_or_mac, authsize);
out:
spin_unlock_irqrestore(&nx_ctx->lock, irq_flags);
return rc;
}
diff --git a/drivers/crypto/nx/nx-aes-gcm.c b/drivers/crypto/nx/nx-aes-gcm.c
index 4a796318b430..b7fe2de96d96 100644
--- a/drivers/crypto/nx/nx-aes-gcm.c
+++ b/drivers/crypto/nx/nx-aes-gcm.c
@@ -101,20 +101,17 @@ static int nx_gca(struct nx_crypto_ctx *nx_ctx,
u8*out,
unsigned int assoclen)
{
int rc;
struct nx_csbcpb *csbcpb_aead = nx_ctx->csbcpb_aead;
- struct scatter_walk walk;
struct nx_sg *nx_sg = nx_ctx->in_sg;
unsigned int nbytes = assoclen;
unsigned int processed = 0, to_process;
unsigned int max_sg_len;
if (nbytes <= AES_BLOCK_SIZE) {
- scatterwalk_start(&walk, req->src);
- scatterwal
[PATCH v2 10/29] crypto: powerpc/p10-aes-gcm - simplify handling of linear associated data
From: Eric Biggers
p10_aes_gcm_crypt() is abusing the scatter_walk API to get the virtual
address for the first source scatterlist element. But this code is only
built for PPC64 which is a !HIGHMEM platform, and it can read past a
page boundary from the address returned by scatterwalk_map() which means
it already assumes the address is from the kernel's direct map. Thus,
just use sg_virt() instead to get the same result in a simpler way.
Cc: Christophe Leroy
Cc: Danny Tsen
Cc: Michael Ellerman
Cc: Naveen N Rao
Cc: Nicholas Piggin
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Eric Biggers
---
This patch is part of a long series touching many files, so I have
limited the Cc list on the full series. If you want the full series and
did not receive it, please retrieve it from lore.kernel.org.
arch/powerpc/crypto/aes-gcm-p10-glue.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/crypto/aes-gcm-p10-glue.c
b/arch/powerpc/crypto/aes-gcm-p10-glue.c
index f37b3d13fc53..2862c3cf8e41 100644
--- a/arch/powerpc/crypto/aes-gcm-p10-glue.c
+++ b/arch/powerpc/crypto/aes-gcm-p10-glue.c
@@ -212,11 +212,10 @@ static int p10_aes_gcm_crypt(struct aead_request *req, u8
*riv,
struct p10_aes_gcm_ctx *ctx = crypto_tfm_ctx(tfm);
u8 databuf[sizeof(struct gcm_ctx) + PPC_ALIGN];
struct gcm_ctx *gctx = PTR_ALIGN((void *)databuf, PPC_ALIGN);
u8 hashbuf[sizeof(struct Hash_ctx) + PPC_ALIGN];
struct Hash_ctx *hash = PTR_ALIGN((void *)hashbuf, PPC_ALIGN);
- struct scatter_walk assoc_sg_walk;
struct skcipher_walk walk;
u8 *assocmem = NULL;
u8 *assoc;
unsigned int cryptlen = req->cryptlen;
unsigned char ivbuf[AES_BLOCK_SIZE+PPC_ALIGN];
@@ -232,12 +231,11 @@ static int p10_aes_gcm_crypt(struct aead_request *req, u8
*riv,
memset(ivbuf, 0, sizeof(ivbuf));
memcpy(iv, riv, GCM_IV_SIZE);
/* Linearize assoc, if not already linear */
if (req->src->length >= assoclen && req->src->length) {
- scatterwalk_start(&assoc_sg_walk, req->src);
- assoc = scatterwalk_map(&assoc_sg_walk);
+ assoc = sg_virt(req->src); /* ppc64 is !HIGHMEM */
} else {
gfp_t flags = (req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
GFP_KERNEL : GFP_ATOMIC;
/* assoc can be any length, so must be on heap */
@@ -251,13 +249,11 @@ static int p10_aes_gcm_crypt(struct aead_request *req, u8
*riv,
vsx_begin();
gcmp10_init(gctx, iv, (unsigned char *) &ctx->enc_key, hash, assoc,
assoclen);
vsx_end();
- if (!assocmem)
- scatterwalk_unmap(assoc);
- else
+ if (assocmem)
kfree(assocmem);
if (enc)
ret = skcipher_walk_aead_encrypt(&walk, req, false);
else
--
2.47.1
Re: [PATCH 1/6] [DOC] powerpc: Document APIv2 KVM hcall spec for Hostwide counters
On Sun, Dec 22, 2024 at 07:32:29PM +0530, Vaibhav Jain wrote: > Update kvm-nested APIv2 documentation to include five new > Guest-State-Elements to fetch the hostwide counters. These counters are > per L1-Lpar and indicate the amount of Heap/Page-table memory allocated, > available and Page-table memory reclaimed for all L2-Guests active > instances > > Signed-off-by: Vaibhav Jain > --- > Documentation/arch/powerpc/kvm-nested.rst | 40 --- > 1 file changed, 29 insertions(+), 11 deletions(-) > > diff --git a/Documentation/arch/powerpc/kvm-nested.rst > b/Documentation/arch/powerpc/kvm-nested.rst > index 5defd13cc6c1..c506192f3f98 100644 > --- a/Documentation/arch/powerpc/kvm-nested.rst > +++ b/Documentation/arch/powerpc/kvm-nested.rst > @@ -208,13 +208,9 @@ associated values for each ID in the GSB:: >flags: > Bit 0: getGuestWideState: Request state of the Guest instead > of an individual VCPU. > - Bit 1: takeOwnershipOfVcpuState Indicate the L1 is taking > - over ownership of the VCPU state and that the L0 can free > - the storage holding the state. The VCPU state will need to > - be returned to the Hypervisor via H_GUEST_SET_STATE prior > - to H_GUEST_RUN_VCPU being called for this VCPU. The data > - returned in the dataBuffer is in a Hypervisor internal > - format. > + Bit 1: getHostWideState: Request stats of the Host. This causes > + the guestId and vcpuId parameters to be ignored and attempting > + to get the VCPU/Guest state will cause an error. s/Request stats/Request state > Bits 2-63: Reserved >guestId: ID obtained from H_GUEST_CREATE >vcpuId: ID of the vCPU pass to H_GUEST_CREATE_VCPU > @@ -402,13 +398,14 @@ GSB element: > > The ID in the GSB element specifies what is to be set. This includes > archtected state like GPRs, VSRs, SPRs, plus also some meta data about > -the partition like the timebase offset and partition scoped page > +the partition and like the timebase offset and partition scoped page > table information. The statement that is already there looks correct IMO. > > ++---+++--+ > -| ID | Size | RW | Thread | Details | > -|| Bytes || Guest | | > -|| || Scope | | > +| ID | Size | RW |(H)ost | Details | > +|| Bytes ||(G)uest | | > +|| ||(T)hread| | > +|| ||Scope | | > ++===+++==+ > | 0x | | RW | TG | NOP element | > ++---+++--+ > @@ -434,6 +431,27 @@ table information. > || |||- 0x8 Table size. | > ++---+++--+ > | 0x0007-| ||| Reserved | > +| 0x07FF | ||| | > +++---+++--+ > +| 0x0800 | 0x08 | R | H| Current usage in bytes of the| > +|| ||| L0's Guest Management Space | > +++---+++--+ > +| 0x0801 | 0x08 | R | H| Max bytes available in the | > +|| ||| L0's Guest Management Space | > +++---+++--+ > +| 0x0802 | 0x08 | R | H| Current usage in bytes of the| > +|| ||| L0's Guest Page Table Management | > +|| ||| Space| > +++---+++--+ > +| 0x0803 | 0x08 | R | H| Max bytes available in the L0's | > +|| ||| Guest Page Table Management | > +|| ||| Space| > +++---+++--+ > +| 0x0804 | 0x08 | R | H| Amount of reclaimed L0 Guest's | > +|| ||| Page Table Management Space due | > +|| ||| to overcommit| I think it would be more clear to specify "... Management space for L1 ..." in the details of all above entries. > +++---+++--+ > +| 0x0805-| ||| Reserved | > | 0x0BFF | ||| | > ++---+++--+ > |
Re: [PATCH 5/6] powerpc/book3s-hv-pmu: Implement GSB message-ops for hostwide counters
On Sun, Dec 22, 2024 at 07:32:33PM +0530, Vaibhav Jain wrote:
> Implement and setup necessary structures to send a prepolulated
> Guest-State-Buffer(GSB) requesting hostwide counters to L0-PowerVM and have
> the returned GSB holding the values of these counters parsed. This is done
> via existing GSB implementation and with the newly added support of
> Hostwide elements in GSB.
>
> The request to L0-PowerVM to return Hostwide counters is done using a
> pre-allocated GSB named 'gsb_l0_stats'. To be able to populate this GSB
> with the needed Guest-State-Elements (GSIDs) a instance of 'struct
> kvmppc_gs_msg' named 'gsm_l0_stats' is introduced. The 'gsm_l0_stats' is
> tied to an instance of 'struct kvmppc_gs_msg_ops' named 'gsb_ops_l0_stats'
> which holds various callbacks to be compute the size ( hostwide_get_size()
> ), populate the GSB ( hostwide_fill_info() ) and
> refresh ( hostwide_refresh_info() ) the contents of
> 'l0_stats' that holds the Hostwide counters returned from L0-PowerVM.
>
> To protect these structures from simultaneous access a spinlock
> 'lock_l0_stats' has been introduced. The allocation and initialization of
> the above structures is done in newly introduced kvmppc_init_hostwide() and
> similarly the cleanup is performed in newly introduced
> kvmppc_cleanup_hostwide().
>
> Signed-off-by: Vaibhav Jain
> ---
> arch/powerpc/kvm/book3s_hv_pmu.c | 189 +++
> 1 file changed, 189 insertions(+)
>
> diff --git a/arch/powerpc/kvm/book3s_hv_pmu.c
> b/arch/powerpc/kvm/book3s_hv_pmu.c
> index e72542d5e750..f7fd5190ecf7 100644
> --- a/arch/powerpc/kvm/book3s_hv_pmu.c
> +++ b/arch/powerpc/kvm/book3s_hv_pmu.c
> @@ -27,10 +27,31 @@
> #include
> #include
>
> +#include "asm/guest-state-buffer.h"
> +
> enum kvmppc_pmu_eventid {
> KVMPPC_EVENT_MAX,
> };
>
> +#define KVMPPC_PMU_EVENT_ATTR(_name, _id) \
> + PMU_EVENT_ATTR_ID(_name, power_events_sysfs_show, _id)
> +
> +/* Holds the hostwide stats */
> +static struct kvmppc_hostwide_stats {
> + u64 guest_heap;
> + u64 guest_heap_max;
> + u64 guest_pgtable_size;
> + u64 guest_pgtable_size_max;
> + u64 guest_pgtable_reclaim;
> +} l0_stats;
> +
> +/* Protect access to l0_stats */
> +static DEFINE_SPINLOCK(lock_l0_stats);
> +
> +/* GSB related structs needed to talk to L0 */
> +static struct kvmppc_gs_msg *gsm_l0_stats;
> +static struct kvmppc_gs_buff *gsb_l0_stats;
> +
> static struct attribute *kvmppc_pmu_events_attr[] = {
> NULL,
> };
> @@ -90,6 +111,167 @@ static void kvmppc_pmu_read(struct perf_event *event)
> {
> }
>
> +/* Return the size of the needed guest state buffer */
> +static size_t hostwide_get_size(struct kvmppc_gs_msg *gsm)
> +
> +{
> + size_t size = 0;
> + const u16 ids[] = {
> + KVMPPC_GSID_L0_GUEST_HEAP,
> + KVMPPC_GSID_L0_GUEST_HEAP_MAX,
> + KVMPPC_GSID_L0_GUEST_PGTABLE_SIZE,
> + KVMPPC_GSID_L0_GUEST_PGTABLE_SIZE_MAX,
> + KVMPPC_GSID_L0_GUEST_PGTABLE_RECLAIM
> + };
> +
> + for (int i = 0; i < ARRAY_SIZE(ids); i++)
> + size += kvmppc_gse_total_size(kvmppc_gsid_size(ids[i]));
> + return size;
> +}
> +
> +/* Populate the request guest state buffer */
> +static int hostwide_fill_info(struct kvmppc_gs_buff *gsb,
> + struct kvmppc_gs_msg *gsm)
> +{
> + struct kvmppc_hostwide_stats *stats = gsm->data;
> +
> + /*
> + * It doesn't matter what values are put into request buffer as
> + * they are going to be overwritten anyways. But for the sake of
> + * testcode and symmetry contents of existing stats are put
> + * populated into the request guest state buffer.
> + */
> + if (kvmppc_gsm_includes(gsm, KVMPPC_GSID_L0_GUEST_HEAP))
> + kvmppc_gse_put_u64(gsb, KVMPPC_GSID_L0_GUEST_HEAP,
> +stats->guest_heap);
> + if (kvmppc_gsm_includes(gsm, KVMPPC_GSID_L0_GUEST_HEAP_MAX))
> + kvmppc_gse_put_u64(gsb, KVMPPC_GSID_L0_GUEST_HEAP_MAX,
> +stats->guest_heap_max);
> + if (kvmppc_gsm_includes(gsm, KVMPPC_GSID_L0_GUEST_PGTABLE_SIZE))
> + kvmppc_gse_put_u64(gsb, KVMPPC_GSID_L0_GUEST_PGTABLE_SIZE,
> +stats->guest_pgtable_size);
> + if (kvmppc_gsm_includes(gsm, KVMPPC_GSID_L0_GUEST_PGTABLE_SIZE_MAX))
> + kvmppc_gse_put_u64(gsb, KVMPPC_GSID_L0_GUEST_PGTABLE_SIZE_MAX,
> +stats->guest_pgtable_size_max);
> + if (kvmppc_gsm_includes(gsm, KVMPPC_GSID_L0_GUEST_PGTABLE_RECLAIM))
> + kvmppc_gse_put_u64(gsb, KVMPPC_GSID_L0_GUEST_PGTABLE_RECLAIM,
> +stats->guest_pgtable_reclaim);
> +
> + return 0;
> +}
kvmppc_gse_put_u64() can return an error. I think we can handle it just
like gs_msg_ops_vcpu_fill_info()
> +
> +/* Parse and update the host wide stats from returned gsb */
> +static int hostwide_refresh_
