Re: [kvm-unit-tests PATCH v9 29/31] powerpc: Remove remnants of ppc64 directory and build structure
On Tue Jun 4, 2024 at 11:36 PM AEST, Andrew Jones wrote: > On Tue, Jun 04, 2024 at 12:49:51PM GMT, Thomas Huth wrote: > > On 04/05/2024 14.28, Nicholas Piggin wrote: > > > This moves merges ppc64 directories and files into powerpc, and > > > merges the 3 makefiles into one. > > > > > > The configure --arch=powerpc option is aliased to ppc64 for > > > good measure. > > > > > > Signed-off-by: Nicholas Piggin > > > --- > > ... > > > diff --git a/powerpc/Makefile b/powerpc/Makefile > > > index 8a007ab54..e4b5312a2 100644 > > > --- a/powerpc/Makefile > > > +++ b/powerpc/Makefile > > > @@ -1 +1,111 @@ > > > -include $(SRCDIR)/$(TEST_DIR)/Makefile.$(ARCH) > > > +# > > > +# powerpc makefile > > > +# > > > +# Authors: Andrew Jones > > > > I'd maybe drop that e-mail address now since it it not valid anymore. > > Andrew, do want to see your new mail address here? > > No need to change to my new email address. We can either keep it as is for > historical records, and as part of faithful code move, or just drop it. I'll leave it, and leave it up to you to send an update email address patch if and when you decide. Thanks, Nick
[PATCH v2 0/2] Fix doorbell emulation for v2 API on PPC
Doorbell emulation for KVM on PAPR guests is broken as support for DPDES was not added in initial patch series [1]. Add DPDES support and doorbell handling support for V2 API. [1] lore.kernel.org/linuxppc-dev/20230914030600.16993-1-jniet...@gmail.com Changes in v2: 1. Split DPDES support into its own patch Gautam Menghani (2): arch/powerpc/kvm: Add DPDES support in helper library for Guest state buffer arch/powerpc/kvm: Fix doorbell emulation for v2 API Documentation/arch/powerpc/kvm-nested.rst | 4 +++- arch/powerpc/include/asm/guest-state-buffer.h | 3 ++- arch/powerpc/include/asm/kvm_book3s.h | 1 + arch/powerpc/kvm/book3s_hv.c | 5 + arch/powerpc/kvm/book3s_hv_nestedv2.c | 7 +++ arch/powerpc/kvm/test-guest-state-buffer.c| 2 +- 6 files changed, 19 insertions(+), 3 deletions(-) -- 2.45.1
[PATCH v2 2/2] arch/powerpc/kvm: Fix doorbell emulation for v2 API
Doorbell emulation is broken for KVM on PAPR guests as support for
DPDES was not added in the initial patch series. Due to this, a KVM on
PAPR guest with SMT > 1 cannot be booted with the XICS interrupt
controller as doorbells are setup in the initial probe path when using XICS
(pSeries_smp_probe()).
Command to replicate the above bug:
qemu-system-ppc64 \
-drive file=rhel.qcow2,format=qcow2 \
-m 20G \
-smp 8,cores=1,threads=8 \
-cpu host \
-nographic \
-machine pseries,ic-mode=xics -accel kvm
Add doorbell state handling support in the host
KVM code to fix doorbell emulation.
Fixes: 19d31c5f1157 ("KVM: PPC: Add support for nestedv2 guests")
Cc: sta...@vger.kernel.org # v6.7
Signed-off-by: Gautam Menghani
---
arch/powerpc/kvm/book3s_hv.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 35cb014a0c51..21c69647d27c 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -4116,6 +4116,11 @@ static int kvmhv_vcpu_entry_nestedv2(struct kvm_vcpu
*vcpu, u64 time_limit,
int trap;
long rc;
+ if (vcpu->arch.doorbell_request) {
+ vcpu->arch.doorbell_request = 0;
+ kvmppc_set_dpdes(vcpu, 1);
+ }
+
io = &vcpu->arch.nestedv2_io;
msr = mfmsr();
--
2.45.1
[PATCH v2 1/2] arch/powerpc/kvm: Add DPDES support in helper library for Guest state buffer
Add support for using DPDES in the library for using guest state
buffers. DPDES support is needed for enabling usage of doorbells in a
L2 KVM on PAPR guest.
Fixes: 6ccbbc33f06a ("KVM: PPC: Add helper library for Guest State Buffers")
Cc: sta...@vger.kernel.org # v6.7
Signed-off-by: Gautam Menghani
---
Documentation/arch/powerpc/kvm-nested.rst | 4 +++-
arch/powerpc/include/asm/guest-state-buffer.h | 3 ++-
arch/powerpc/include/asm/kvm_book3s.h | 1 +
arch/powerpc/kvm/book3s_hv_nestedv2.c | 7 +++
arch/powerpc/kvm/test-guest-state-buffer.c| 2 +-
5 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/Documentation/arch/powerpc/kvm-nested.rst
b/Documentation/arch/powerpc/kvm-nested.rst
index 630602a8aa00..5defd13cc6c1 100644
--- a/Documentation/arch/powerpc/kvm-nested.rst
+++ b/Documentation/arch/powerpc/kvm-nested.rst
@@ -546,7 +546,9 @@ table information.
++---+++--+
| 0x1052 | 0x08 | RW | T| CTRL |
++---+++--+
-| 0x1053-| ||| Reserved |
+| 0x1053 | 0x08 | RW | T| DPDES|
+++---+++--+
+| 0x1054-| ||| Reserved |
| 0x1FFF | ||| |
++---+++--+
| 0x2000 | 0x04 | RW | T| CR |
diff --git a/arch/powerpc/include/asm/guest-state-buffer.h
b/arch/powerpc/include/asm/guest-state-buffer.h
index 808149f31576..d107abe1468f 100644
--- a/arch/powerpc/include/asm/guest-state-buffer.h
+++ b/arch/powerpc/include/asm/guest-state-buffer.h
@@ -81,6 +81,7 @@
#define KVMPPC_GSID_HASHKEYR 0x1050
#define KVMPPC_GSID_HASHPKEYR 0x1051
#define KVMPPC_GSID_CTRL 0x1052
+#define KVMPPC_GSID_DPDES 0x1053
#define KVMPPC_GSID_CR 0x2000
#define KVMPPC_GSID_PIDR 0x2001
@@ -110,7 +111,7 @@
#define KVMPPC_GSE_META_COUNT (KVMPPC_GSE_META_END - KVMPPC_GSE_META_START + 1)
#define KVMPPC_GSE_DW_REGS_START KVMPPC_GSID_GPR(0)
-#define KVMPPC_GSE_DW_REGS_END KVMPPC_GSID_CTRL
+#define KVMPPC_GSE_DW_REGS_END KVMPPC_GSID_DPDES
#define KVMPPC_GSE_DW_REGS_COUNT \
(KVMPPC_GSE_DW_REGS_END - KVMPPC_GSE_DW_REGS_START + 1)
diff --git a/arch/powerpc/include/asm/kvm_book3s.h
b/arch/powerpc/include/asm/kvm_book3s.h
index 3e1e2a698c9e..10618622d7ef 100644
--- a/arch/powerpc/include/asm/kvm_book3s.h
+++ b/arch/powerpc/include/asm/kvm_book3s.h
@@ -594,6 +594,7 @@ static inline u##size kvmppc_get_##reg(struct kvm_vcpu
*vcpu) \
KVMPPC_BOOK3S_VCORE_ACCESSOR(vtb, 64, KVMPPC_GSID_VTB)
+KVMPPC_BOOK3S_VCORE_ACCESSOR(dpdes, 64, KVMPPC_GSID_DPDES)
KVMPPC_BOOK3S_VCORE_ACCESSOR_GET(arch_compat, 32, KVMPPC_GSID_LOGICAL_PVR)
KVMPPC_BOOK3S_VCORE_ACCESSOR_GET(lpcr, 64, KVMPPC_GSID_LPCR)
KVMPPC_BOOK3S_VCORE_ACCESSOR_SET(tb_offset, 64, KVMPPC_GSID_TB_OFFSET)
diff --git a/arch/powerpc/kvm/book3s_hv_nestedv2.c
b/arch/powerpc/kvm/book3s_hv_nestedv2.c
index 8e6f5355f08b..36863fff2a99 100644
--- a/arch/powerpc/kvm/book3s_hv_nestedv2.c
+++ b/arch/powerpc/kvm/book3s_hv_nestedv2.c
@@ -311,6 +311,10 @@ static int gs_msg_ops_vcpu_fill_info(struct kvmppc_gs_buff
*gsb,
rc = kvmppc_gse_put_u64(gsb, iden,
vcpu->arch.vcore->vtb);
break;
+ case KVMPPC_GSID_DPDES:
+ rc = kvmppc_gse_put_u64(gsb, iden,
+ vcpu->arch.vcore->dpdes);
+ break;
case KVMPPC_GSID_LPCR:
rc = kvmppc_gse_put_u64(gsb, iden,
vcpu->arch.vcore->lpcr);
@@ -543,6 +547,9 @@ static int gs_msg_ops_vcpu_refresh_info(struct
kvmppc_gs_msg *gsm,
case KVMPPC_GSID_VTB:
vcpu->arch.vcore->vtb = kvmppc_gse_get_u64(gse);
break;
+ case KVMPPC_GSID_DPDES:
+ vcpu->arch.vcore->dpdes = kvmppc_gse_get_u64(gse);
+ break;
case KVMPPC_GSID_LPCR:
vcpu->arch.vcore->lpcr = kvmppc_gse_get_u64(gse);
break;
diff --git a/arch/powerpc/kvm/test-guest-state-buffer.c
b/arch/powerpc/kvm/test-guest-state-buffer.c
index 4720b8dc8837..2571ccc618c9 100644
--- a/arch/powerpc/kvm/test-guest-state-buffer.c
+++ b/arch/powerpc/kvm/test-guest-state-buffer.c
@@ -151,7 +151,7 @@ static void test_gs_bitmap(struct kunit *test)
i++;
}
- for (u16 iden = KVMPPC_GSID_GPR(0); iden <= KVMPPC_GSID_CTRL; iden++) {
+
Re: [PATCH v1 1/1] treewide: Align match_string() with sysfs_match_string()
Il 02/06/24 17:57, Andy Shevchenko ha scritto: Make two APIs look similar. Hence convert match_string() to be a 2-argument macro. In order to avoid unneeded churn, convert all users as well. There is no functional change intended. Signed-off-by: Andy Shevchenko For MediaTek Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH 04/23] scsi: initialize scsi midlayer limits before allocating the queue
Michael Ellerman writes:
> Christoph Hellwig writes:
>> On Fri, May 31, 2024 at 12:28:21AM +1000, Michael Ellerman wrote:
>>> No that's wrong. The actual hardware page size is 4K, but
>>> CONFIG_PAGE_SIZE and PAGE_SHIFT etc. is 64K.
>>>
>>> So at least for this user the driver used to work with 64K pages, and
>>> now doesn't.
>>
>> Which suggested that the communicated max_hw_sectors is wrong, and
>> previously we were saved by the block layer increasing it to
>> PAGE_SIZE after a warning. Should we just increment it to 64k?
>
> It looks like that user actually only has the CDROM hanging off
> pata_macio, so it's possible it has been broken previously and they
> didn't notice. I'll see if they can confirm the CDROM has been working
> up until now.
>
> I can test the CDROM on my G5 next week.
I can confirm that the driver does work with 64K pages prior to the
recent changes. I'm able to boot and read CDs with no errors.
However AFAICS that's because the driver splits large requests in
pata_macio_qc_prep():
static enum ata_completion_errors pata_macio_qc_prep(struct ata_queued_cmd *qc)
{
...
for_each_sg(qc->sg, sg, qc->n_elem, si) {
u32 addr, sg_len, len;
...
addr = (u32) sg_dma_address(sg);
sg_len = sg_dma_len(sg);
while (sg_len) {
...
len = (sg_len < MAX_DBDMA_SEG) ? sg_len : MAX_DBDMA_SEG;
table->command = cpu_to_le16(write ? OUTPUT_MORE:
INPUT_MORE);
table->req_count = cpu_to_le16(len);
...
addr += len;
sg_len -= len;
++table;
}
}
If I increase MAX_DBMA_SEG from 0xff00 to 64K I see IO errors at boot:
[ 24.989755] sr 4:0:0:0: [sr0] tag#0 FAILED Result: hostbyte=DID_OK
driverbyte=DRIVER_OK cmd_age=6s
[ 25.007310] sr 4:0:0:0: [sr0] tag#0 Sense Key : Medium Error [current]
[ 25.020502] sr 4:0:0:0: [sr0] tag#0 ASC=0x10 <>ASCQ=0x90
[ 25.032655] sr 4:0:0:0: [sr0] tag#0 CDB: Read(10) 28 00 00 00 00 00 00 00
20 00
[ 25.047232] I/O error, dev sr0, sector 0 op 0x0:(READ) flags 0x80700
phys_seg 1 prio class 0
On the other hand increasing max_segment_size to 64K while leaving MAX_DBDMA_SEG
at 0xff00 seems to work fine. And that's effectively what's been happening on
existing kernels until now.
The only question is whether that violates some assumption elsewhere in the
SCSI layer?
Anyway patch below that works for me on v6.10-rc2.
cheers
diff --git a/drivers/ata/pata_macio.c b/drivers/ata/pata_macio.c
index 817838e2f70e..3cb455a32d92 100644
--- a/drivers/ata/pata_macio.c
+++ b/drivers/ata/pata_macio.c
@@ -915,10 +915,13 @@ static const struct scsi_host_template pata_macio_sht = {
.sg_tablesize = MAX_DCMDS,
/* We may not need that strict one */
.dma_boundary = ATA_DMA_BOUNDARY,
- /* Not sure what the real max is but we know it's less than 64K, let's
-* use 64K minus 256
+ /*
+* The SCSI core requires the segment size to cover at least a page, so
+* for 64K page size kernels this must be at least 64K. However the
+* hardware can't handle 64K, so pata_macio_qc_prep() will split large
+* requests.
*/
- .max_segment_size = MAX_DBDMA_SEG,
+ .max_segment_size = SZ_64K,
.device_configure = pata_macio_device_configure,
.sdev_groups= ata_common_sdev_groups,
.can_queue = ATA_DEF_QUEUE,
[Bug 218858] scsi_alloc_sdev: Allocation failure during SCSI scanning, some SCSI devices might not be configured
https://bugzilla.kernel.org/show_bug.cgi?id=218858 --- Comment #20 from Michael Ellerman (mich...@ellerman.id.au) --- Please try the patch here: https://lore.kernel.org/linuxppc-dev/87wmn3pntq.fsf@mail.lhotse/ -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[PATCH v2 0/8] KVM: PPC: Book3S HV: Nested guest migration fixes
The series fixes the issues exposed by the kvm-unit-tests[1] sprs-migration test. The SDAR, MMCR3 were seen to have some typo/refactoring bugs. The first two patches fix them. The remaining patches take care of save-restoring the guest state elements for DEXCR, HASHKEYR and HASHPKEYR SPRs with PHYP during entry-exit. The KVM_PPC_REG too for them are missing which are added for use by the QEMU. References: [1]: https://github.com/kvm-unit-tests/kvm-unit-tests --- Changelog: v1: https://lore.kernel.org/kvm/171741555734.11675.17428208097186191736.stgit@c0c876608f2d/ - Reordered the patches in a way to introduce the SPRs first as suggested. - Added Reviewed-bys to the reviewed ones. - Added 2 more patches to handle the hashpkeyr state Shivaprasad G Bhat (8): KVM: PPC: Book3S HV: Fix the set_one_reg for MMCR3 KVM: PPC: Book3S HV: Fix the get_one_reg of SDAR KVM: PPC: Book3S HV: Add one-reg interface for DEXCR register KVM: PPC: Book3S HV nestedv2: Keep nested guest DEXCR in sync KVM: PPC: Book3S HV: Add one-reg interface for HASHKEYR register KVM: PPC: Book3S HV nestedv2: Keep nested guest HASHKEYR in sync KVM: PPC: Book3S HV: Add one-reg interface for HASHPKEYR register KVM: PPC: Book3S HV nestedv2: Keep nested guest HASHPKEYR in sync Documentation/virt/kvm/api.rst| 3 +++ arch/powerpc/include/asm/kvm_host.h | 3 +++ arch/powerpc/include/uapi/asm/kvm.h | 3 +++ arch/powerpc/kvm/book3s_hv.c | 22 -- arch/powerpc/kvm/book3s_hv.h | 3 +++ arch/powerpc/kvm/book3s_hv_nestedv2.c | 18 ++ 6 files changed, 50 insertions(+), 2 deletions(-) -- Signature
[PATCH v2 1/8] KVM: PPC: Book3S HV: Fix the set_one_reg for MMCR3
The kvmppc_set_one_reg_hv() wrongly get() the value
instead of set() for MMCR3. Fix the same.
Fixes: 5752fe0b811b ("KVM: PPC: Book3S HV: Save/restore new PMU registers")
Signed-off-by: Shivaprasad G Bhat
Reviewed-by: Nicholas Piggin
---
arch/powerpc/kvm/book3s_hv.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index daaf7faf21a5..a4f34f94c86f 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -2540,7 +2540,7 @@ static int kvmppc_set_one_reg_hv(struct kvm_vcpu *vcpu,
u64 id,
vcpu->arch.mmcrs = set_reg_val(id, *val);
break;
case KVM_REG_PPC_MMCR3:
- *val = get_reg_val(id, vcpu->arch.mmcr[3]);
+ kvmppc_set_mmcr_hv(vcpu, 3, set_reg_val(id, *val));
break;
case KVM_REG_PPC_PMC1 ... KVM_REG_PPC_PMC8:
i = id - KVM_REG_PPC_PMC1;
[PATCH v2 2/8] KVM: PPC: Book3S HV: Fix the get_one_reg of SDAR
The kvmppc_get_one_reg_hv() for SDAR is wrongly getting the SIAR
instead of SDAR, possibly a paste error emanating from the previous
refactoring.
Patch fixes the wrong get_one_reg() for the same.
Fixes: ebc88ea7a6ad ("KVM: PPC: Book3S HV: Use accessors for VCPU registers")
Signed-off-by: Shivaprasad G Bhat
Reviewed-by: Nicholas Piggin
---
arch/powerpc/kvm/book3s_hv.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index a4f34f94c86f..b576781d58d5 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -2305,7 +2305,7 @@ static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu,
u64 id,
*val = get_reg_val(id, kvmppc_get_siar_hv(vcpu));
break;
case KVM_REG_PPC_SDAR:
- *val = get_reg_val(id, kvmppc_get_siar_hv(vcpu));
+ *val = get_reg_val(id, kvmppc_get_sdar_hv(vcpu));
break;
case KVM_REG_PPC_SIER:
*val = get_reg_val(id, kvmppc_get_sier_hv(vcpu, 0));
[PATCH v2 4/8] KVM: PPC: Book3S HV nestedv2: Keep nested guest DEXCR in sync
The nestedv2 APIs has the guest state element defined for DEXCR for the save-restore with L0. However, its ignored in the code. The patch takes care of this for the DEXCR GSID. Signed-off-by: Shivaprasad G Bhat Reviewed-by: Nicholas Piggin --- arch/powerpc/kvm/book3s_hv_nestedv2.c |6 ++ 1 file changed, 6 insertions(+) diff --git a/arch/powerpc/kvm/book3s_hv_nestedv2.c b/arch/powerpc/kvm/book3s_hv_nestedv2.c index 1091f7a83b25..d207a6d936ff 100644 --- a/arch/powerpc/kvm/book3s_hv_nestedv2.c +++ b/arch/powerpc/kvm/book3s_hv_nestedv2.c @@ -193,6 +193,9 @@ static int gs_msg_ops_vcpu_fill_info(struct kvmppc_gs_buff *gsb, case KVMPPC_GSID_DAWRX1: rc = kvmppc_gse_put_u32(gsb, iden, vcpu->arch.dawrx1); break; + case KVMPPC_GSID_DEXCR: + rc = kvmppc_gse_put_u64(gsb, iden, vcpu->arch.dexcr); + break; case KVMPPC_GSID_CIABR: rc = kvmppc_gse_put_u64(gsb, iden, vcpu->arch.ciabr); break; @@ -441,6 +444,9 @@ static int gs_msg_ops_vcpu_refresh_info(struct kvmppc_gs_msg *gsm, case KVMPPC_GSID_DAWRX1: vcpu->arch.dawrx1 = kvmppc_gse_get_u32(gse); break; + case KVMPPC_GSID_DEXCR: + vcpu->arch.dexcr = kvmppc_gse_get_u64(gse); + break; case KVMPPC_GSID_CIABR: vcpu->arch.ciabr = kvmppc_gse_get_u64(gse); break;
[PATCH v2 5/8] KVM: PPC: Book3S HV: Add one-reg interface for HASHKEYR register
The patch adds a one-reg register identifier which can be used to
read and set the virtual HASHKEYR for the guest during enter/exit
with KVM_REG_PPC_HASHKEYR. The specific SPR KVM API documentation
too updated.
Signed-off-by: Shivaprasad G Bhat
Reviewed-by: Nicholas Piggin
---
Documentation/virt/kvm/api.rst |1 +
arch/powerpc/include/asm/kvm_host.h |1 +
arch/powerpc/include/uapi/asm/kvm.h |1 +
arch/powerpc/kvm/book3s_hv.c|6 ++
arch/powerpc/kvm/book3s_hv.h|1 +
5 files changed, 10 insertions(+)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 81077c654281..0c22cb4196d8 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -2439,6 +2439,7 @@ registers, find a list below:
PPC KVM_REG_PPC_PSSCR 64
PPC KVM_REG_PPC_DEC_EXPIRY 64
PPC KVM_REG_PPC_PTCR64
+ PPC KVM_REG_PPC_HASHKEYR64
PPC KVM_REG_PPC_DAWR1 64
PPC KVM_REG_PPC_DAWRX1 64
PPC KVM_REG_PPC_DEXCR 64
diff --git a/arch/powerpc/include/asm/kvm_host.h
b/arch/powerpc/include/asm/kvm_host.h
index 1e2fdcbecffd..a0cd9dbf534f 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -600,6 +600,7 @@ struct kvm_vcpu_arch {
ulong dawr1;
ulong dawrx1;
ulong dexcr;
+ ulong hashkeyr;
ulong ciabr;
ulong cfar;
ulong ppr;
diff --git a/arch/powerpc/include/uapi/asm/kvm.h
b/arch/powerpc/include/uapi/asm/kvm.h
index fcb947f65667..23a0af739c78 100644
--- a/arch/powerpc/include/uapi/asm/kvm.h
+++ b/arch/powerpc/include/uapi/asm/kvm.h
@@ -646,6 +646,7 @@ struct kvm_ppc_cpu_char {
#define KVM_REG_PPC_DAWR1 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xc4)
#define KVM_REG_PPC_DAWRX1 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xc5)
#define KVM_REG_PPC_DEXCR (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xc6)
+#define KVM_REG_PPC_HASHKEYR (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xc7)
/* Transactional Memory checkpointed state:
* This is all GPRs, all VSX regs and a subset of SPRs
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 1294c6839d37..ccc9564c5a31 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -2352,6 +2352,9 @@ static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu,
u64 id,
case KVM_REG_PPC_DEXCR:
*val = get_reg_val(id, kvmppc_get_dexcr_hv(vcpu));
break;
+ case KVM_REG_PPC_HASHKEYR:
+ *val = get_reg_val(id, kvmppc_get_hashkeyr_hv(vcpu));
+ break;
case KVM_REG_PPC_CIABR:
*val = get_reg_val(id, kvmppc_get_ciabr_hv(vcpu));
break;
@@ -2598,6 +2601,9 @@ static int kvmppc_set_one_reg_hv(struct kvm_vcpu *vcpu,
u64 id,
case KVM_REG_PPC_DEXCR:
kvmppc_set_dexcr_hv(vcpu, set_reg_val(id, *val));
break;
+ case KVM_REG_PPC_HASHKEYR:
+ kvmppc_set_hashkeyr_hv(vcpu, set_reg_val(id, *val));
+ break;
case KVM_REG_PPC_CIABR:
kvmppc_set_ciabr_hv(vcpu, set_reg_val(id, *val));
/* Don't allow setting breakpoints in hypervisor code */
diff --git a/arch/powerpc/kvm/book3s_hv.h b/arch/powerpc/kvm/book3s_hv.h
index 7b0fd282fe95..c073fdfa7dc4 100644
--- a/arch/powerpc/kvm/book3s_hv.h
+++ b/arch/powerpc/kvm/book3s_hv.h
@@ -117,6 +117,7 @@ KVMPPC_BOOK3S_HV_VCPU_ACCESSOR(dawr1, 64, KVMPPC_GSID_DAWR1)
KVMPPC_BOOK3S_HV_VCPU_ACCESSOR(dawrx0, 64, KVMPPC_GSID_DAWRX0)
KVMPPC_BOOK3S_HV_VCPU_ACCESSOR(dawrx1, 64, KVMPPC_GSID_DAWRX1)
KVMPPC_BOOK3S_HV_VCPU_ACCESSOR(dexcr, 64, KVMPPC_GSID_DEXCR)
+KVMPPC_BOOK3S_HV_VCPU_ACCESSOR(hashkeyr, 64, KVMPPC_GSID_HASHKEYR)
KVMPPC_BOOK3S_HV_VCPU_ACCESSOR(ciabr, 64, KVMPPC_GSID_CIABR)
KVMPPC_BOOK3S_HV_VCPU_ACCESSOR(wort, 64, KVMPPC_GSID_WORT)
KVMPPC_BOOK3S_HV_VCPU_ACCESSOR(ppr, 64, KVMPPC_GSID_PPR)
[PATCH v2 6/8] KVM: PPC: Book3S HV nestedv2: Keep nested guest HASHKEYR in sync
The nestedv2 APIs has the guest state element defined for HASHKEYR for the save-restore with L0. However, its ignored in the code. The patch takes care of this for the HASHKEYR GSID. Signed-off-by: Shivaprasad G Bhat Reviewed-by: Nicholas Piggin --- arch/powerpc/kvm/book3s_hv_nestedv2.c |6 ++ 1 file changed, 6 insertions(+) diff --git a/arch/powerpc/kvm/book3s_hv_nestedv2.c b/arch/powerpc/kvm/book3s_hv_nestedv2.c index d207a6d936ff..bbff933f2ccc 100644 --- a/arch/powerpc/kvm/book3s_hv_nestedv2.c +++ b/arch/powerpc/kvm/book3s_hv_nestedv2.c @@ -196,6 +196,9 @@ static int gs_msg_ops_vcpu_fill_info(struct kvmppc_gs_buff *gsb, case KVMPPC_GSID_DEXCR: rc = kvmppc_gse_put_u64(gsb, iden, vcpu->arch.dexcr); break; + case KVMPPC_GSID_HASHKEYR: + rc = kvmppc_gse_put_u64(gsb, iden, vcpu->arch.hashkeyr); + break; case KVMPPC_GSID_CIABR: rc = kvmppc_gse_put_u64(gsb, iden, vcpu->arch.ciabr); break; @@ -447,6 +450,9 @@ static int gs_msg_ops_vcpu_refresh_info(struct kvmppc_gs_msg *gsm, case KVMPPC_GSID_DEXCR: vcpu->arch.dexcr = kvmppc_gse_get_u64(gse); break; + case KVMPPC_GSID_HASHKEYR: + vcpu->arch.hashkeyr = kvmppc_gse_get_u64(gse); + break; case KVMPPC_GSID_CIABR: vcpu->arch.ciabr = kvmppc_gse_get_u64(gse); break;
[PATCH v2 7/8] KVM: PPC: Book3S HV: Add one-reg interface for HASHPKEYR register
The patch adds a one-reg register identifier which can be used to
read and set the virtual HASHPKEYR for the guest during enter/exit
with KVM_REG_PPC_HASHPKEYR. The specific SPR KVM API documentation
too updated.
Signed-off-by: Shivaprasad G Bhat
---
Documentation/virt/kvm/api.rst |1 +
arch/powerpc/include/asm/kvm_host.h |1 +
arch/powerpc/include/uapi/asm/kvm.h |1 +
arch/powerpc/kvm/book3s_hv.c|6 ++
arch/powerpc/kvm/book3s_hv.h|1 +
5 files changed, 10 insertions(+)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 0c22cb4196d8..899480d4acaf 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -2440,6 +2440,7 @@ registers, find a list below:
PPC KVM_REG_PPC_DEC_EXPIRY 64
PPC KVM_REG_PPC_PTCR64
PPC KVM_REG_PPC_HASHKEYR64
+ PPC KVM_REG_PPC_HASHPKEYR 64
PPC KVM_REG_PPC_DAWR1 64
PPC KVM_REG_PPC_DAWRX1 64
PPC KVM_REG_PPC_DEXCR 64
diff --git a/arch/powerpc/include/asm/kvm_host.h
b/arch/powerpc/include/asm/kvm_host.h
index a0cd9dbf534f..6a0c771d3ce8 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -601,6 +601,7 @@ struct kvm_vcpu_arch {
ulong dawrx1;
ulong dexcr;
ulong hashkeyr;
+ ulong hashpkeyr;
ulong ciabr;
ulong cfar;
ulong ppr;
diff --git a/arch/powerpc/include/uapi/asm/kvm.h
b/arch/powerpc/include/uapi/asm/kvm.h
index 23a0af739c78..eaeda001784e 100644
--- a/arch/powerpc/include/uapi/asm/kvm.h
+++ b/arch/powerpc/include/uapi/asm/kvm.h
@@ -647,6 +647,7 @@ struct kvm_ppc_cpu_char {
#define KVM_REG_PPC_DAWRX1 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xc5)
#define KVM_REG_PPC_DEXCR (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xc6)
#define KVM_REG_PPC_HASHKEYR (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xc7)
+#define KVM_REG_PPC_HASHPKEYR (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xc8)
/* Transactional Memory checkpointed state:
* This is all GPRs, all VSX regs and a subset of SPRs
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index ccc9564c5a31..bf05b63b0e20 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -2355,6 +2355,9 @@ static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu,
u64 id,
case KVM_REG_PPC_HASHKEYR:
*val = get_reg_val(id, kvmppc_get_hashkeyr_hv(vcpu));
break;
+ case KVM_REG_PPC_HASHPKEYR:
+ *val = get_reg_val(id, kvmppc_get_hashpkeyr_hv(vcpu));
+ break;
case KVM_REG_PPC_CIABR:
*val = get_reg_val(id, kvmppc_get_ciabr_hv(vcpu));
break;
@@ -2604,6 +2607,9 @@ static int kvmppc_set_one_reg_hv(struct kvm_vcpu *vcpu,
u64 id,
case KVM_REG_PPC_HASHKEYR:
kvmppc_set_hashkeyr_hv(vcpu, set_reg_val(id, *val));
break;
+ case KVM_REG_PPC_HASHPKEYR:
+ kvmppc_set_hashpkeyr_hv(vcpu, set_reg_val(id, *val));
+ break;
case KVM_REG_PPC_CIABR:
kvmppc_set_ciabr_hv(vcpu, set_reg_val(id, *val));
/* Don't allow setting breakpoints in hypervisor code */
diff --git a/arch/powerpc/kvm/book3s_hv.h b/arch/powerpc/kvm/book3s_hv.h
index c073fdfa7dc4..a404c9b221c1 100644
--- a/arch/powerpc/kvm/book3s_hv.h
+++ b/arch/powerpc/kvm/book3s_hv.h
@@ -118,6 +118,7 @@ KVMPPC_BOOK3S_HV_VCPU_ACCESSOR(dawrx0, 64,
KVMPPC_GSID_DAWRX0)
KVMPPC_BOOK3S_HV_VCPU_ACCESSOR(dawrx1, 64, KVMPPC_GSID_DAWRX1)
KVMPPC_BOOK3S_HV_VCPU_ACCESSOR(dexcr, 64, KVMPPC_GSID_DEXCR)
KVMPPC_BOOK3S_HV_VCPU_ACCESSOR(hashkeyr, 64, KVMPPC_GSID_HASHKEYR)
+KVMPPC_BOOK3S_HV_VCPU_ACCESSOR(hashpkeyr, 64, KVMPPC_GSID_HASHPKEYR)
KVMPPC_BOOK3S_HV_VCPU_ACCESSOR(ciabr, 64, KVMPPC_GSID_CIABR)
KVMPPC_BOOK3S_HV_VCPU_ACCESSOR(wort, 64, KVMPPC_GSID_WORT)
KVMPPC_BOOK3S_HV_VCPU_ACCESSOR(ppr, 64, KVMPPC_GSID_PPR)
[PATCH v2 8/8] KVM: PPC: Book3S HV nestedv2: Keep nested guest HASHPKEYR in sync
The nestedv2 APIs has the guest state element defined for HASHPKEYR for the save-restore with L0. However, its ignored in the code. The patch takes care of this for the HASHPKEYR GSID. Signed-off-by: Shivaprasad G Bhat --- arch/powerpc/kvm/book3s_hv_nestedv2.c |6 ++ 1 file changed, 6 insertions(+) diff --git a/arch/powerpc/kvm/book3s_hv_nestedv2.c b/arch/powerpc/kvm/book3s_hv_nestedv2.c index bbff933f2ccc..ce61cb6fc9b9 100644 --- a/arch/powerpc/kvm/book3s_hv_nestedv2.c +++ b/arch/powerpc/kvm/book3s_hv_nestedv2.c @@ -199,6 +199,9 @@ static int gs_msg_ops_vcpu_fill_info(struct kvmppc_gs_buff *gsb, case KVMPPC_GSID_HASHKEYR: rc = kvmppc_gse_put_u64(gsb, iden, vcpu->arch.hashkeyr); break; + case KVMPPC_GSID_HASHPKEYR: + rc = kvmppc_gse_put_u64(gsb, iden, vcpu->arch.hashpkeyr); + break; case KVMPPC_GSID_CIABR: rc = kvmppc_gse_put_u64(gsb, iden, vcpu->arch.ciabr); break; @@ -453,6 +456,9 @@ static int gs_msg_ops_vcpu_refresh_info(struct kvmppc_gs_msg *gsm, case KVMPPC_GSID_HASHKEYR: vcpu->arch.hashkeyr = kvmppc_gse_get_u64(gse); break; + case KVMPPC_GSID_HASHPKEYR: + vcpu->arch.hashpkeyr = kvmppc_gse_get_u64(gse); + break; case KVMPPC_GSID_CIABR: vcpu->arch.ciabr = kvmppc_gse_get_u64(gse); break;
Re: [PATCH 6/6] KVM: PPC: Book3S HV: Add one-reg interface for HASHKEYR register
On 6/4/24 11:37, Nicholas Piggin wrote: On Mon Jun 3, 2024 at 9:15 PM AEST, Shivaprasad G Bhat wrote: The patch adds a one-reg register identifier which can be used to read and set the virtual HASHKEYR for the guest during enter/exit with KVM_REG_PPC_HASHKEYR. The specific SPR KVM API documentation too updated. Signed-off-by: Shivaprasad G Bhat --- Documentation/virt/kvm/api.rst|1 + arch/powerpc/include/uapi/asm/kvm.h |1 + arch/powerpc/kvm/book3s_hv.c |6 ++ tools/arch/powerpc/include/uapi/asm/kvm.h |1 + 4 files changed, 9 insertions(+) diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst index 81077c654281..0c22cb4196d8 100644 --- a/Documentation/virt/kvm/api.rst +++ b/Documentation/virt/kvm/api.rst @@ -2439,6 +2439,7 @@ registers, find a list below: PPC KVM_REG_PPC_PSSCR 64 PPC KVM_REG_PPC_DEC_EXPIRY 64 PPC KVM_REG_PPC_PTCR64 + PPC KVM_REG_PPC_HASHKEYR64 Just looking at the QEMU side of this change made me think... AFAIKS we need to also set and get and migrate the HASHPKEY SPR. Thanks Nick. I have posted the v2 with changes for HASHPKEYR and your other suggestions at 171759276071.1480.9356137231993600304.st...@linux.ibm.com Regards, Shivaprasad The hashst/hashchk test cases might be "working" by chance if the SPR is always zero :/ Thanks, Nick
[PATCH v2 3/8] KVM: PPC: Book3S HV: Add one-reg interface for DEXCR register
The patch adds a one-reg register identifier which can be used to
read and set the DEXCR for the guest during enter/exit with
KVM_REG_PPC_DEXCR. The specific SPR KVM API documentation
too updated.
Signed-off-by: Shivaprasad G Bhat
Reviewed-by: Nicholas Piggin
---
Documentation/virt/kvm/api.rst |1 +
arch/powerpc/include/asm/kvm_host.h |1 +
arch/powerpc/include/uapi/asm/kvm.h |1 +
arch/powerpc/kvm/book3s_hv.c|6 ++
arch/powerpc/kvm/book3s_hv.h|1 +
5 files changed, 10 insertions(+)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index a71d91978d9e..81077c654281 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -2441,6 +2441,7 @@ registers, find a list below:
PPC KVM_REG_PPC_PTCR64
PPC KVM_REG_PPC_DAWR1 64
PPC KVM_REG_PPC_DAWRX1 64
+ PPC KVM_REG_PPC_DEXCR 64
PPC KVM_REG_PPC_TM_GPR0 64
...
PPC KVM_REG_PPC_TM_GPR3164
diff --git a/arch/powerpc/include/asm/kvm_host.h
b/arch/powerpc/include/asm/kvm_host.h
index 8abac532146e..1e2fdcbecffd 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -599,6 +599,7 @@ struct kvm_vcpu_arch {
ulong dawrx0;
ulong dawr1;
ulong dawrx1;
+ ulong dexcr;
ulong ciabr;
ulong cfar;
ulong ppr;
diff --git a/arch/powerpc/include/uapi/asm/kvm.h
b/arch/powerpc/include/uapi/asm/kvm.h
index 1691297a766a..fcb947f65667 100644
--- a/arch/powerpc/include/uapi/asm/kvm.h
+++ b/arch/powerpc/include/uapi/asm/kvm.h
@@ -645,6 +645,7 @@ struct kvm_ppc_cpu_char {
#define KVM_REG_PPC_SIER3 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xc3)
#define KVM_REG_PPC_DAWR1 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xc4)
#define KVM_REG_PPC_DAWRX1 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xc5)
+#define KVM_REG_PPC_DEXCR (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xc6)
/* Transactional Memory checkpointed state:
* This is all GPRs, all VSX regs and a subset of SPRs
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index b576781d58d5..1294c6839d37 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -2349,6 +2349,9 @@ static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu,
u64 id,
case KVM_REG_PPC_DAWRX1:
*val = get_reg_val(id, kvmppc_get_dawrx1_hv(vcpu));
break;
+ case KVM_REG_PPC_DEXCR:
+ *val = get_reg_val(id, kvmppc_get_dexcr_hv(vcpu));
+ break;
case KVM_REG_PPC_CIABR:
*val = get_reg_val(id, kvmppc_get_ciabr_hv(vcpu));
break;
@@ -2592,6 +2595,9 @@ static int kvmppc_set_one_reg_hv(struct kvm_vcpu *vcpu,
u64 id,
case KVM_REG_PPC_DAWRX1:
kvmppc_set_dawrx1_hv(vcpu, set_reg_val(id, *val) & ~DAWRX_HYP);
break;
+ case KVM_REG_PPC_DEXCR:
+ kvmppc_set_dexcr_hv(vcpu, set_reg_val(id, *val));
+ break;
case KVM_REG_PPC_CIABR:
kvmppc_set_ciabr_hv(vcpu, set_reg_val(id, *val));
/* Don't allow setting breakpoints in hypervisor code */
diff --git a/arch/powerpc/kvm/book3s_hv.h b/arch/powerpc/kvm/book3s_hv.h
index 47b2c815641e..7b0fd282fe95 100644
--- a/arch/powerpc/kvm/book3s_hv.h
+++ b/arch/powerpc/kvm/book3s_hv.h
@@ -116,6 +116,7 @@ KVMPPC_BOOK3S_HV_VCPU_ACCESSOR(dawr0, 64, KVMPPC_GSID_DAWR0)
KVMPPC_BOOK3S_HV_VCPU_ACCESSOR(dawr1, 64, KVMPPC_GSID_DAWR1)
KVMPPC_BOOK3S_HV_VCPU_ACCESSOR(dawrx0, 64, KVMPPC_GSID_DAWRX0)
KVMPPC_BOOK3S_HV_VCPU_ACCESSOR(dawrx1, 64, KVMPPC_GSID_DAWRX1)
+KVMPPC_BOOK3S_HV_VCPU_ACCESSOR(dexcr, 64, KVMPPC_GSID_DEXCR)
KVMPPC_BOOK3S_HV_VCPU_ACCESSOR(ciabr, 64, KVMPPC_GSID_CIABR)
KVMPPC_BOOK3S_HV_VCPU_ACCESSOR(wort, 64, KVMPPC_GSID_WORT)
KVMPPC_BOOK3S_HV_VCPU_ACCESSOR(ppr, 64, KVMPPC_GSID_PPR)
[Bug 218858] scsi_alloc_sdev: Allocation failure during SCSI scanning, some SCSI devices might not be configured
https://bugzilla.kernel.org/show_bug.cgi?id=218858 --- Comment #21 from doru iorgulescu (doru.iorgules...@gmail.com) --- Thank You Very much Please send to Linus for aproval Thank You Regards -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[PATCH v2] PCI/AER: Print error message as per the TODO
Print the add device error in find_device_iter()
Signed-off-by: Abhinav Jain
PATCH v1 link :
https://lore.kernel.org/all/20240415161055.8316-1-jain.abhinav...@gmail.com/
Changes since v1:
- Replaced pr_err() with pr_notice()
- Removed unncessary whitespaces
---
drivers/pci/pcie/aer.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 0e1ad2998116..8b820a74dd6b 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -885,8 +885,8 @@ static int find_device_iter(struct pci_dev *dev, void *data)
/* List this device */
if (add_error_device(e_info, dev)) {
/* We cannot handle more... Stop iteration */
- pr_err("find_device_iter: Cannot handle more devices.
- Stopping iteration");
+ pr_notice("%s: Cannot handle more devices - iteration
stopped\n",
+ __func__);
return 1;
}
--
2.34.1
Re: [PATCH v2 0/2] ASoC: fsl_xcvr: Support i.MX95 platform
On Tue, 14 May 2024 11:12:07 +0800, Shengjiu Wang wrote: > On i.MX95 wakeup domain, there is one instance of Audio XCVR > supporting SPDIF mode with a connection to the Audio XCVR physical > interface. > > changes in v2: > - Merge patch 1&2, 3&4 from v1 together. > - Add more comments in commit message > - Add constaint for clocks used on i.mx95 > - Add 'select SND_SOC_FSL_UTILS' for compiling issue. > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/2] ASoC: dt-bindings: fsl,xcvr: Add compatible string for i.MX95 commit: fc1277335ffa0d180c76ddccf5fe27fc75674e67 [2/2] ASoC: fsl_xcvr: Add support for i.MX95 platform commit: f13b349e3c70320ef5a86edfc888a6feb612abb0 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
Re: [PATCH v4 0/2] ASoC: fsl_xcvr: Support i.MX95 platform
On Wed, 29 May 2024 16:40:00 +0800, Shengjiu Wang wrote: > On i.MX95 wakeup domain, there is one instance of Audio XCVR > supporting SPDIF mode with a connection to the Audio XCVR physical > interface. > > changes in v4: > - refine the constarint for 'clocks' according to Rob's comments > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/2] ASoC: dt-bindings: fsl,xcvr: Add compatible string for i.MX95 commit: fc1277335ffa0d180c76ddccf5fe27fc75674e67 [2/2] ASoC: fsl_xcvr: Add support for i.MX95 platform commit: f13b349e3c70320ef5a86edfc888a6feb612abb0 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
Re: [PATCH v2] PCI/AER: Print error message as per the TODO
On Wed, Jun 05, 2024 at 09:23:44PM +, Abhinav Jain wrote:
> Print the add device error in find_device_iter()
>
> Signed-off-by: Abhinav Jain
>
> PATCH v1 link :
> https://lore.kernel.org/all/20240415161055.8316-1-jain.abhinav...@gmail.com/
>
> Changes since v1:
> - Replaced pr_err() with pr_notice()
> - Removed unncessary whitespaces
> ---
Thanks for looking at this.
- It doesn't apply to -rc1 (the TODO message is missing). In PCI,
we normally apply patches on topic branches based on -rc1.
- The subject should be more specific so it makes sense all by
itself, e.g., "Log note if we find too many devices with errors"
- Add period at end of sentence in commit log.
- Move historical notes (v1 URL, changes since v1) below the "---"
line so they don't get included in the commit log.
- __func__ is not relevant here -- that's generally a debugging
thing. We can find the function by searching for the message
text. In cases like this, I'd rather have something that helps
identify a *device* that's related to the message, e.g., the
pci_dev in this case. So I'd suggest pci_err(dev, "...") here.
- I'd keep pci_err() instead of switching to pr_notice(). If we get
this message, we should re-think the way we collect this
information, so I want to hear about it.
> drivers/pci/pcie/aer.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index 0e1ad2998116..8b820a74dd6b 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -885,8 +885,8 @@ static int find_device_iter(struct pci_dev *dev, void
> *data)
> /* List this device */
> if (add_error_device(e_info, dev)) {
> /* We cannot handle more... Stop iteration */
> - pr_err("find_device_iter: Cannot handle more devices.
> - Stopping iteration");
> + pr_notice("%s: Cannot handle more devices - iteration
> stopped\n",
> + __func__);
> return 1;
> }
>
> --
> 2.34.1
>
[PATCH v2 0/6] ASoC: Drop or replace of_gpio.h
Replace or drop the legacy header that is subject to remove. Not all of them were compile-tested, the series might have hidden compilation errors. In v3: - moved aw88399 from the "Remove ..." patch to the "Replace ..." (LKP) In v2: - added tags (Kuninori, Charles) - ripped out TAS2781 (it's a mess from GPIO handling perspective) Andy Shevchenko (6): ASoC: codecs: Remove unused of_gpio.h ASoC: fsl: Remove unused of_gpio.h ASoC: rockchip: Remove unused of_gpio.h ASoC: codecs: Replace of_gpio.h by proper one ASoC: generic: Replace of_gpio.h by proper one ASoC: samsung: Replace of_gpio.h by proper one sound/soc/codecs/ak4118.c | 1 - sound/soc/codecs/ak4458.c | 1 - sound/soc/codecs/aw88395/aw88395.c | 2 +- sound/soc/codecs/aw88399.c | 2 +- sound/soc/codecs/cs53l30.c | 1 - sound/soc/codecs/max98390.c | 1 - sound/soc/codecs/pcm3168a.c | 1 - sound/soc/codecs/rk817_codec.c | 1 - sound/soc/codecs/tas2552.c | 1 - sound/soc/codecs/tas2764.c | 1 - sound/soc/codecs/tas2770.c | 1 - sound/soc/codecs/tas2780.c | 1 - sound/soc/codecs/tlv320adc3xxx.c| 1 - sound/soc/codecs/tlv320adcx140.c| 1 - sound/soc/codecs/tlv320aic31xx.c| 1 - sound/soc/codecs/ts3a227e.c | 1 - sound/soc/codecs/wsa883x.c | 1 - sound/soc/fsl/imx-es8328.c | 1 - sound/soc/fsl/imx-rpmsg.c | 2 -- sound/soc/generic/audio-graph-card2-custom-sample.c | 3 ++- sound/soc/rockchip/rockchip_i2s.c | 1 - sound/soc/rockchip/rockchip_spdif.c | 1 - sound/soc/samsung/aries_wm8994.c| 2 +- 23 files changed, 5 insertions(+), 24 deletions(-) -- 2.43.0.rc1.1336.g36b5255a03ac
[PATCH v2 2/6] ASoC: fsl: Remove unused of_gpio.h
of_gpio.h is deprecated and subject to remove. The drivers in question don't use it, simply remove the unused header. Signed-off-by: Andy Shevchenko --- sound/soc/fsl/imx-es8328.c | 1 - sound/soc/fsl/imx-rpmsg.c | 2 -- 2 files changed, 3 deletions(-) diff --git a/sound/soc/fsl/imx-es8328.c b/sound/soc/fsl/imx-es8328.c index 5b9648f3b087..3ef92f6dfc6b 100644 --- a/sound/soc/fsl/imx-es8328.c +++ b/sound/soc/fsl/imx-es8328.c @@ -8,7 +8,6 @@ #include #include #include -#include #include #include diff --git a/sound/soc/fsl/imx-rpmsg.c b/sound/soc/fsl/imx-rpmsg.c index 0f1ad7ad7d27..ce98d2288193 100644 --- a/sound/soc/fsl/imx-rpmsg.c +++ b/sound/soc/fsl/imx-rpmsg.c @@ -5,9 +5,7 @@ #include #include #include -#include #include -#include #include #include #include -- 2.43.0.rc1.1336.g36b5255a03ac
[PATCH v2 1/6] ASoC: codecs: Remove unused of_gpio.h
of_gpio.h is deprecated and subject to remove. The drivers in question don't use it, simply remove the unused header. Reviewed-by: Kuninori Morimoto Reviewed-by: Charles Keepax Signed-off-by: Andy Shevchenko --- sound/soc/codecs/ak4118.c| 1 - sound/soc/codecs/ak4458.c| 1 - sound/soc/codecs/aw88399.c | 2 +- sound/soc/codecs/cs53l30.c | 1 - sound/soc/codecs/max98390.c | 1 - sound/soc/codecs/pcm3168a.c | 1 - sound/soc/codecs/rk817_codec.c | 1 - sound/soc/codecs/tas2552.c | 1 - sound/soc/codecs/tas2764.c | 1 - sound/soc/codecs/tas2770.c | 1 - sound/soc/codecs/tas2780.c | 1 - sound/soc/codecs/tlv320adc3xxx.c | 1 - sound/soc/codecs/tlv320adcx140.c | 1 - sound/soc/codecs/tlv320aic31xx.c | 1 - sound/soc/codecs/ts3a227e.c | 1 - sound/soc/codecs/wsa883x.c | 1 - 16 files changed, 1 insertion(+), 16 deletions(-) diff --git a/sound/soc/codecs/ak4118.c b/sound/soc/codecs/ak4118.c index 9a43235e6a11..23e868e4e3fb 100644 --- a/sound/soc/codecs/ak4118.c +++ b/sound/soc/codecs/ak4118.c @@ -9,7 +9,6 @@ #include #include #include -#include #include #include diff --git a/sound/soc/codecs/ak4458.c b/sound/soc/codecs/ak4458.c index 73cf482f104f..32cb802ad635 100644 --- a/sound/soc/codecs/ak4458.c +++ b/sound/soc/codecs/ak4458.c @@ -10,7 +10,6 @@ #include #include #include -#include #include #include #include diff --git a/sound/soc/codecs/aw88399.c b/sound/soc/codecs/aw88399.c index 9fcb805bf971..df6d52a1cfef 100644 --- a/sound/soc/codecs/aw88399.c +++ b/sound/soc/codecs/aw88399.c @@ -10,7 +10,7 @@ #include #include #include -#include +#include #include #include #include "aw88399.h" diff --git a/sound/soc/codecs/cs53l30.c b/sound/soc/codecs/cs53l30.c index c0893146423b..2ee13d885fdc 100644 --- a/sound/soc/codecs/cs53l30.c +++ b/sound/soc/codecs/cs53l30.c @@ -12,7 +12,6 @@ #include #include #include -#include #include #include #include diff --git a/sound/soc/codecs/max98390.c b/sound/soc/codecs/max98390.c index 57fa2db1e148..1bae253618fd 100644 --- a/sound/soc/codecs/max98390.c +++ b/sound/soc/codecs/max98390.c @@ -13,7 +13,6 @@ #include #include #include -#include #include #include #include diff --git a/sound/soc/codecs/pcm3168a.c b/sound/soc/codecs/pcm3168a.c index 9d6431338fb7..3c0e0fdbfc5c 100644 --- a/sound/soc/codecs/pcm3168a.c +++ b/sound/soc/codecs/pcm3168a.c @@ -11,7 +11,6 @@ #include #include #include -#include #include #include diff --git a/sound/soc/codecs/rk817_codec.c b/sound/soc/codecs/rk817_codec.c index d4da98469f8b..5fea600bc3a4 100644 --- a/sound/soc/codecs/rk817_codec.c +++ b/sound/soc/codecs/rk817_codec.c @@ -10,7 +10,6 @@ #include #include #include -#include #include #include #include diff --git a/sound/soc/codecs/tas2552.c b/sound/soc/codecs/tas2552.c index a7ed59ec49a6..9e68afc09897 100644 --- a/sound/soc/codecs/tas2552.c +++ b/sound/soc/codecs/tas2552.c @@ -13,7 +13,6 @@ #include #include #include -#include #include #include #include diff --git a/sound/soc/codecs/tas2764.c b/sound/soc/codecs/tas2764.c index 1dc719d726ab..5eaddf07aadc 100644 --- a/sound/soc/codecs/tas2764.c +++ b/sound/soc/codecs/tas2764.c @@ -15,7 +15,6 @@ #include #include #include -#include #include #include #include diff --git a/sound/soc/codecs/tas2770.c b/sound/soc/codecs/tas2770.c index 67bc1c8b0131..5601fba17c96 100644 --- a/sound/soc/codecs/tas2770.c +++ b/sound/soc/codecs/tas2770.c @@ -20,7 +20,6 @@ #include #include #include -#include #include #include #include diff --git a/sound/soc/codecs/tas2780.c b/sound/soc/codecs/tas2780.c index a18ccf5fb7ad..6902bfef185b 100644 --- a/sound/soc/codecs/tas2780.c +++ b/sound/soc/codecs/tas2780.c @@ -11,7 +11,6 @@ #include #include #include -#include #include #include #include diff --git a/sound/soc/codecs/tlv320adc3xxx.c b/sound/soc/codecs/tlv320adc3xxx.c index e100cc9f5c19..eb180df9a72a 100644 --- a/sound/soc/codecs/tlv320adc3xxx.c +++ b/sound/soc/codecs/tlv320adc3xxx.c @@ -25,7 +25,6 @@ #include #include #include -#include #include #include #include diff --git a/sound/soc/codecs/tlv320adcx140.c b/sound/soc/codecs/tlv320adcx140.c index 41342b340680..d594bf166c0e 100644 --- a/sound/soc/codecs/tlv320adcx140.c +++ b/sound/soc/codecs/tlv320adcx140.c @@ -12,7 +12,6 @@ #include #include #include -#include #include #include #include diff --git a/sound/soc/codecs/tlv320aic31xx.c b/sound/soc/codecs/tlv320aic31xx.c index 4d7c5a80c6ed..2f94cfda0e33 100644 --- a/sound/soc/codecs/tlv320aic31xx.c +++ b/sound/soc/codecs/tlv320aic31xx.c @@ -23,7 +23,6 @@ #include #include #include -#include #include #include #include diff --git a/sound/soc/codecs/ts3a227e.c b/sound/soc/codecs/ts3a227e.c index dbf448dd8864..b9eb59e3bfa0 100644 --- a/sound/soc/codecs/ts3a227e.c +++ b/sound/soc/codecs/ts3a227e.c @@ -10,7 +10,6 @@ #include #include
[PATCH v2 4/6] ASoC: codecs: Replace of_gpio.h by proper one
of_gpio.h is deprecated and subject to remove. The driver doesn't use it directly, replace it with what is really being used. Signed-off-by: Andy Shevchenko --- sound/soc/codecs/aw88395/aw88395.c | 2 +- sound/soc/codecs/aw88399.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sound/soc/codecs/aw88395/aw88395.c b/sound/soc/codecs/aw88395/aw88395.c index 3c459a67ad0c..be6ebcb51cca 100644 --- a/sound/soc/codecs/aw88395/aw88395.c +++ b/sound/soc/codecs/aw88395/aw88395.c @@ -8,9 +8,9 @@ // Author: Weidong Wang // +#include #include #include -#include #include #include #include "aw88395.h" diff --git a/sound/soc/codecs/aw88399.c b/sound/soc/codecs/aw88399.c index df6d52a1cfef..5d8481612eab 100644 --- a/sound/soc/codecs/aw88399.c +++ b/sound/soc/codecs/aw88399.c @@ -8,9 +8,9 @@ // #include +#include #include #include -#include #include #include #include "aw88399.h" -- 2.43.0.rc1.1336.g36b5255a03ac
[PATCH v2 3/6] ASoC: rockchip: Remove unused of_gpio.h
of_gpio.h is deprecated and subject to remove. The drivers in question don't use it, simply remove the unused header. Signed-off-by: Andy Shevchenko --- sound/soc/rockchip/rockchip_i2s.c | 1 - sound/soc/rockchip/rockchip_spdif.c | 1 - 2 files changed, 2 deletions(-) diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c index b0c3ef030e06..b378f870b3ad 100644 --- a/sound/soc/rockchip/rockchip_i2s.c +++ b/sound/soc/rockchip/rockchip_i2s.c @@ -11,7 +11,6 @@ #include #include #include -#include #include #include #include diff --git a/sound/soc/rockchip/rockchip_spdif.c b/sound/soc/rockchip/rockchip_spdif.c index 1a24b78e9e02..eb9d5dee196e 100644 --- a/sound/soc/rockchip/rockchip_spdif.c +++ b/sound/soc/rockchip/rockchip_spdif.c @@ -11,7 +11,6 @@ #include #include -#include #include #include #include -- 2.43.0.rc1.1336.g36b5255a03ac
[PATCH v2 5/6] ASoC: generic: Replace of_gpio.h by proper one
of_gpio.h is deprecated and subject to remove. The driver doesn't use it directly, replace it with what is really being used. Acked-by: Kuninori Morimoto Signed-off-by: Andy Shevchenko --- sound/soc/generic/audio-graph-card2-custom-sample.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sound/soc/generic/audio-graph-card2-custom-sample.c b/sound/soc/generic/audio-graph-card2-custom-sample.c index 1b6ccd2de964..8e5a51098490 100644 --- a/sound/soc/generic/audio-graph-card2-custom-sample.c +++ b/sound/soc/generic/audio-graph-card2-custom-sample.c @@ -5,8 +5,9 @@ // Copyright (C) 2020 Renesas Electronics Corp. // Copyright (C) 2020 Kuninori Morimoto // +#include +#include #include -#include #include #include -- 2.43.0.rc1.1336.g36b5255a03ac
[PATCH v2 6/6] ASoC: samsung: Replace of_gpio.h by proper one
of_gpio.h is deprecated and subject to remove. The driver doesn't use it directly, replace it with what is really being used. Signed-off-by: Andy Shevchenko --- sound/soc/samsung/aries_wm8994.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/samsung/aries_wm8994.c b/sound/soc/samsung/aries_wm8994.c index a548ac33dd94..01716df0c842 100644 --- a/sound/soc/samsung/aries_wm8994.c +++ b/sound/soc/samsung/aries_wm8994.c @@ -1,11 +1,11 @@ // SPDX-License-Identifier: GPL-2.0+ #include +#include #include #include #include #include #include -#include #include #include #include -- 2.43.0.rc1.1336.g36b5255a03ac
Re: kswapd0: page allocation failure: order:0, mode:0x820(GFP_ATOMIC), nodemask=(null),cpuset=/,mems_allowed=0 (Kernel v6.5.9, 32bit ppc)
On Tue, 4 Jun 2024 20:03:27 -0700 Yosry Ahmed wrote: > Could you check if the attached patch helps? It basically changes the > number of zpools from 32 to min(32, nr_cpus). Thanks! The patch does not fix the issue but it helps. Means I still get to see the 'kswapd0: page allocation failure' in the dmesg, a 'stress-ng-vm: page allocation failure' later on, another kswapd0 error later on, etc. _but_ the machine keeps running the workload, stays usable via VNC and I get no hard crash any longer. Without patch kswapd0 error and hard crash (need to power-cycle) <3min. With patch several kswapd0 errors but running for 2 hrs now. I double checked this to be sure. The patch did not apply cleanly on v6.9.3 so I applied it on v6.10-rc2. dmesg of the current v6.10-rc2 run attached. Regards, Erhard dmesg_610-rc2_g4 Description: Binary data
[PATCH v3] PCI/AER: Log error message if there are too many PCI devices with errors
Added pci_err() to log PCI device information on which iteration fails.
Added pci_err() to log note if there are too many failed devices.
Signed-off-by: Abhinav Jain
---
PATCH v2:
https://lore.kernel.org/all/20240605212344.21808-1-jain.abhinav...@gmail.com/
Changes since v2:
- Switched to pci_err() to log failing PCI device
- Included the historical note below the "---" line
- Fixed other style changes in the patch
---
drivers/pci/pcie/aer.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 8b820a74dd6b..0bca83827ac1 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -882,11 +882,13 @@ static int find_device_iter(struct pci_dev *dev, void
*data)
struct aer_err_info *e_info = (struct aer_err_info *)data;
if (is_error_source(dev, e_info)) {
+ /* Log error for the device */
+ pci_err(dev, "Error detected on device: %s\n", pci_name(dev));
+
/* List this device */
if (add_error_device(e_info, dev)) {
/* We cannot handle more... Stop iteration */
- pr_notice("%s: Cannot handle more devices - iteration
stopped\n",
- __func__);
+ pci_err(dev, "Too many error devices encountered.
Stopping iteration.\n");
return 1;
}
--
2.34.1
Re: [PATCH v2] PCI/AER: Print error message as per the TODO
On Wed, 5 Jun 2024 16:58:48 -0500, Bjorn Helgaas wrote: > - It doesn't apply to -rc1 (the TODO message is missing). In PCI, > we normally apply patches on topic branches based on -rc1. Thank you for the detailed feedback. I was looking at mainline only. > - The subject should be more specific so it makes sense all by > itself, e.g., "Log note if we find too many devices with errors" > - Add period at end of sentence in commit log. > - Move historical notes (v1 URL, changes since v1) below the "---" > line so they don't get included in the commit log. I have included these changes to the v3. Please find it here: https://lore.kernel.org/all/20240605230954.22911-1-jain.abhinav...@gmail.com/ > - __func__ is not relevant here -- that's generally a debugging > thing. We can find the function by searching for the message > text. In cases like this, I'd rather have something that helps > identify a *device* that's related to the message, e.g., the > pci_dev in this case. So I'd suggest pci_err(dev, "...") here. > - I'd keep pci_err() instead of switching to pr_notice(). If we get > this message, we should re-think the way we collect this > information, so I want to hear about it. I understand, this helped me get a clear picture of what needs to be done. I have accordingly added two pci_err for the same. Please review.
Re: kswapd0: page allocation failure: order:0, mode:0x820(GFP_ATOMIC), nodemask=(null),cpuset=/,mems_allowed=0 (Kernel v6.5.9, 32bit ppc)
On Wed, Jun 5, 2024 at 4:04 PM Erhard Furtner wrote: > > On Tue, 4 Jun 2024 20:03:27 -0700 > Yosry Ahmed wrote: > > > Could you check if the attached patch helps? It basically changes the > > number of zpools from 32 to min(32, nr_cpus). > > Thanks! The patch does not fix the issue but it helps. > > Means I still get to see the 'kswapd0: page allocation failure' in the dmesg, > a 'stress-ng-vm: page allocation failure' later on, another kswapd0 error > later on, etc. _but_ the machine keeps running the workload, stays usable via > VNC and I get no hard crash any longer. > > Without patch kswapd0 error and hard crash (need to power-cycle) <3min. With > patch several kswapd0 errors but running for 2 hrs now. I double checked this > to be sure. Thanks for trying this out. This is interesting, so even two zpools is too much fragmentation for your use case. I think there are multiple ways to go forward here: (a) Make the number of zpools a config option, leave the default as 32, but allow special use cases to set it to 1 or similar. This is probably not preferable because it is not clear to users how to set it, but the idea is that no one will have to set it except special use cases such as Erhard's (who will want to set it to 1 in this case). (b) Make the number of zpools scale linearly with the number of CPUs. Maybe something like nr_cpus/4 or nr_cpus/8. The problem with this approach is that with a large number of CPUs, too many zpools will start having diminishing returns. Fragmentation will keep increasing, while the scalability/concurrency gains will diminish. (c) Make the number of zpools scale logarithmically with the number of CPUs. Maybe something like 4log2(nr_cpus). This will keep the number of zpools from increasing too much and close to the status quo. The problem is that at a small number of CPUs (e.g. 2), 4log2(nr_cpus) will actually give a nr_zpools > nr_cpus. So we will need to come up with a more fancy magic equation (e.g. 4log2(nr_cpus/4)). (d) Make the number of zpools scale linearly with memory. This makes more sense than scaling with CPUs because increasing the number of zpools increases fragmentation, so it makes sense to limit it by the available memory. This is also more consistent with other magic numbers we have (e.g. SWAP_ADDRESS_SPACE_SHIFT). The problem is that unlike zswap trees, the zswap pool is not connected to the swapfile size, so we don't have an indication for how much memory will be in the zswap pool. We can scale the number of zpools with the entire memory on the machine during boot, but this seems like it would be difficult to figure out, and will not take into consideration memory hotplugging and the zswap global limit changing. (e) A creative mix of the above. (f) Something else (probably simpler). I am personally leaning toward (c), but I want to hear the opinions of other people here. Yu, Vlastimil, Johannes, Nhat? Anyone else? In the long-term, I think we may want to address the lock contention in zsmalloc itself instead of zswap spawning multiple zpools. > > The patch did not apply cleanly on v6.9.3 so I applied it on v6.10-rc2. dmesg > of the current v6.10-rc2 run attached. > > Regards, > Erhard
Re: kswapd0: page allocation failure: order:0, mode:0x820(GFP_ATOMIC), nodemask=(null),cpuset=/,mems_allowed=0 (Kernel v6.5.9, 32bit ppc)
On Wed, Jun 5, 2024 at 5:42 PM Yosry Ahmed wrote:
>
> On Wed, Jun 5, 2024 at 4:04 PM Erhard Furtner wrote:
> >
> > On Tue, 4 Jun 2024 20:03:27 -0700
> > Yosry Ahmed wrote:
> >
> > > Could you check if the attached patch helps? It basically changes the
> > > number of zpools from 32 to min(32, nr_cpus).
> >
> > Thanks! The patch does not fix the issue but it helps.
> >
> > Means I still get to see the 'kswapd0: page allocation failure' in the
> > dmesg, a 'stress-ng-vm: page allocation failure' later on, another kswapd0
> > error later on, etc. _but_ the machine keeps running the workload, stays
> > usable via VNC and I get no hard crash any longer.
> >
> > Without patch kswapd0 error and hard crash (need to power-cycle) <3min.
> > With patch several kswapd0 errors but running for 2 hrs now. I double
> > checked this to be sure.
>
> Thanks for trying this out. This is interesting, so even two zpools is
> too much fragmentation for your use case.
Now I'm a little bit skeptical that the problem is due to fragmentation.
> I think there are multiple ways to go forward here:
> (a) Make the number of zpools a config option, leave the default as
> 32, but allow special use cases to set it to 1 or similar. This is
> probably not preferable because it is not clear to users how to set
> it, but the idea is that no one will have to set it except special use
> cases such as Erhard's (who will want to set it to 1 in this case).
>
> (b) Make the number of zpools scale linearly with the number of CPUs.
> Maybe something like nr_cpus/4 or nr_cpus/8. The problem with this
> approach is that with a large number of CPUs, too many zpools will
> start having diminishing returns. Fragmentation will keep increasing,
> while the scalability/concurrency gains will diminish.
>
> (c) Make the number of zpools scale logarithmically with the number of
> CPUs. Maybe something like 4log2(nr_cpus). This will keep the number
> of zpools from increasing too much and close to the status quo. The
> problem is that at a small number of CPUs (e.g. 2), 4log2(nr_cpus)
> will actually give a nr_zpools > nr_cpus. So we will need to come up
> with a more fancy magic equation (e.g. 4log2(nr_cpus/4)).
>
> (d) Make the number of zpools scale linearly with memory. This makes
> more sense than scaling with CPUs because increasing the number of
> zpools increases fragmentation, so it makes sense to limit it by the
> available memory. This is also more consistent with other magic
> numbers we have (e.g. SWAP_ADDRESS_SPACE_SHIFT).
>
> The problem is that unlike zswap trees, the zswap pool is not
> connected to the swapfile size, so we don't have an indication for how
> much memory will be in the zswap pool. We can scale the number of
> zpools with the entire memory on the machine during boot, but this
> seems like it would be difficult to figure out, and will not take into
> consideration memory hotplugging and the zswap global limit changing.
>
> (e) A creative mix of the above.
>
> (f) Something else (probably simpler).
>
> I am personally leaning toward (c), but I want to hear the opinions of
> other people here. Yu, Vlastimil, Johannes, Nhat? Anyone else?
I double checked that commit and didn't find anything wrong. If we are
all in the mood of getting to the bottom, can we try using only 1
zpool while there are 2 available? I.e.,
static struct zpool *zswap_find_zpool(struct zswap_entry *entry)
{
- return entry->pool->zpools[hash_ptr(entry, ilog2(ZSWAP_NR_ZPOOLS))];
+ return entry->pool->zpools[0];
}
> In the long-term, I think we may want to address the lock contention
> in zsmalloc itself instead of zswap spawning multiple zpools.
>
> >
> > The patch did not apply cleanly on v6.9.3 so I applied it on v6.10-rc2.
> > dmesg of the current v6.10-rc2 run attached.
> >
> > Regards,
> > Erhard
Re: kswapd0: page allocation failure: order:0, mode:0x820(GFP_ATOMIC), nodemask=(null),cpuset=/,mems_allowed=0 (Kernel v6.5.9, 32bit ppc)
On Wed, Jun 5, 2024 at 4:53 PM Yu Zhao wrote:
>
> On Wed, Jun 5, 2024 at 5:42 PM Yosry Ahmed wrote:
> >
> > On Wed, Jun 5, 2024 at 4:04 PM Erhard Furtner wrote:
> > >
> > > On Tue, 4 Jun 2024 20:03:27 -0700
> > > Yosry Ahmed wrote:
> > >
> > > > Could you check if the attached patch helps? It basically changes the
> > > > number of zpools from 32 to min(32, nr_cpus).
> > >
> > > Thanks! The patch does not fix the issue but it helps.
> > >
> > > Means I still get to see the 'kswapd0: page allocation failure' in the
> > > dmesg, a 'stress-ng-vm: page allocation failure' later on, another
> > > kswapd0 error later on, etc. _but_ the machine keeps running the
> > > workload, stays usable via VNC and I get no hard crash any longer.
> > >
> > > Without patch kswapd0 error and hard crash (need to power-cycle) <3min.
> > > With patch several kswapd0 errors but running for 2 hrs now. I double
> > > checked this to be sure.
> >
> > Thanks for trying this out. This is interesting, so even two zpools is
> > too much fragmentation for your use case.
>
> Now I'm a little bit skeptical that the problem is due to fragmentation.
>
> > I think there are multiple ways to go forward here:
> > (a) Make the number of zpools a config option, leave the default as
> > 32, but allow special use cases to set it to 1 or similar. This is
> > probably not preferable because it is not clear to users how to set
> > it, but the idea is that no one will have to set it except special use
> > cases such as Erhard's (who will want to set it to 1 in this case).
> >
> > (b) Make the number of zpools scale linearly with the number of CPUs.
> > Maybe something like nr_cpus/4 or nr_cpus/8. The problem with this
> > approach is that with a large number of CPUs, too many zpools will
> > start having diminishing returns. Fragmentation will keep increasing,
> > while the scalability/concurrency gains will diminish.
> >
> > (c) Make the number of zpools scale logarithmically with the number of
> > CPUs. Maybe something like 4log2(nr_cpus). This will keep the number
> > of zpools from increasing too much and close to the status quo. The
> > problem is that at a small number of CPUs (e.g. 2), 4log2(nr_cpus)
> > will actually give a nr_zpools > nr_cpus. So we will need to come up
> > with a more fancy magic equation (e.g. 4log2(nr_cpus/4)).
> >
> > (d) Make the number of zpools scale linearly with memory. This makes
> > more sense than scaling with CPUs because increasing the number of
> > zpools increases fragmentation, so it makes sense to limit it by the
> > available memory. This is also more consistent with other magic
> > numbers we have (e.g. SWAP_ADDRESS_SPACE_SHIFT).
> >
> > The problem is that unlike zswap trees, the zswap pool is not
> > connected to the swapfile size, so we don't have an indication for how
> > much memory will be in the zswap pool. We can scale the number of
> > zpools with the entire memory on the machine during boot, but this
> > seems like it would be difficult to figure out, and will not take into
> > consideration memory hotplugging and the zswap global limit changing.
> >
> > (e) A creative mix of the above.
> >
> > (f) Something else (probably simpler).
> >
> > I am personally leaning toward (c), but I want to hear the opinions of
> > other people here. Yu, Vlastimil, Johannes, Nhat? Anyone else?
>
> I double checked that commit and didn't find anything wrong. If we are
> all in the mood of getting to the bottom, can we try using only 1
> zpool while there are 2 available? I.e.,
Erhard, do you mind checking if Yu's diff below to use a single zpool
fixes the problem completely? There is also an attached patch that
does the same thing if this is easier to apply for you.
>
> static struct zpool *zswap_find_zpool(struct zswap_entry *entry)
> {
> - return entry->pool->zpools[hash_ptr(entry, ilog2(ZSWAP_NR_ZPOOLS))];
> + return entry->pool->zpools[0];
> }
>
> > In the long-term, I think we may want to address the lock contention
> > in zsmalloc itself instead of zswap spawning multiple zpools.
> >
> > >
> > > The patch did not apply cleanly on v6.9.3 so I applied it on v6.10-rc2.
> > > dmesg of the current v6.10-rc2 run attached.
> > >
> > > Regards,
> > > Erhard
0001-mm-zswap-set-ZSWAP_NR_ZPOOLS-to-1.patch
Description: Binary data
[Bug 218858] scsi_alloc_sdev: Allocation failure during SCSI scanning, some SCSI devices might not be configured
https://bugzilla.kernel.org/show_bug.cgi?id=218858 --- Comment #22 from Michael Ellerman (mich...@ellerman.id.au) --- Can you please confirm that the patch fixes the issue for you. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[Bug 218858] scsi_alloc_sdev: Allocation failure during SCSI scanning, some SCSI devices might not be configured
https://bugzilla.kernel.org/show_bug.cgi?id=218858 --- Comment #23 from doru iorgulescu (doru.iorgules...@gmail.com) --- I am in compilation When is finished I send to You the results Thank You Very Mutch Regards -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH v2 1/2] arch/powerpc/kvm: Add DPDES support in helper library for Guest state buffer
On Wed Jun 5, 2024 at 9:39 PM AEST, Gautam Menghani wrote:
> Add support for using DPDES in the library for using guest state
> buffers. DPDES support is needed for enabling usage of doorbells in a
> L2 KVM on PAPR guest.
>
Reviewed-by: Nicholas Piggin
> Fixes: 6ccbbc33f06a ("KVM: PPC: Add helper library for Guest State Buffers")
> Cc: sta...@vger.kernel.org # v6.7
> Signed-off-by: Gautam Menghani
> ---
> Documentation/arch/powerpc/kvm-nested.rst | 4 +++-
> arch/powerpc/include/asm/guest-state-buffer.h | 3 ++-
> arch/powerpc/include/asm/kvm_book3s.h | 1 +
> arch/powerpc/kvm/book3s_hv_nestedv2.c | 7 +++
> arch/powerpc/kvm/test-guest-state-buffer.c| 2 +-
> 5 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/arch/powerpc/kvm-nested.rst
> b/Documentation/arch/powerpc/kvm-nested.rst
> index 630602a8aa00..5defd13cc6c1 100644
> --- a/Documentation/arch/powerpc/kvm-nested.rst
> +++ b/Documentation/arch/powerpc/kvm-nested.rst
> @@ -546,7 +546,9 @@ table information.
> ++---+++--+
> | 0x1052 | 0x08 | RW | T| CTRL |
> ++---+++--+
> -| 0x1053-| ||| Reserved |
> +| 0x1053 | 0x08 | RW | T| DPDES|
> +++---+++--+
> +| 0x1054-| ||| Reserved |
> | 0x1FFF | ||| |
> ++---+++--+
> | 0x2000 | 0x04 | RW | T| CR |
> diff --git a/arch/powerpc/include/asm/guest-state-buffer.h
> b/arch/powerpc/include/asm/guest-state-buffer.h
> index 808149f31576..d107abe1468f 100644
> --- a/arch/powerpc/include/asm/guest-state-buffer.h
> +++ b/arch/powerpc/include/asm/guest-state-buffer.h
> @@ -81,6 +81,7 @@
> #define KVMPPC_GSID_HASHKEYR 0x1050
> #define KVMPPC_GSID_HASHPKEYR0x1051
> #define KVMPPC_GSID_CTRL 0x1052
> +#define KVMPPC_GSID_DPDES0x1053
>
> #define KVMPPC_GSID_CR 0x2000
> #define KVMPPC_GSID_PIDR 0x2001
> @@ -110,7 +111,7 @@
> #define KVMPPC_GSE_META_COUNT (KVMPPC_GSE_META_END - KVMPPC_GSE_META_START +
> 1)
>
> #define KVMPPC_GSE_DW_REGS_START KVMPPC_GSID_GPR(0)
> -#define KVMPPC_GSE_DW_REGS_END KVMPPC_GSID_CTRL
> +#define KVMPPC_GSE_DW_REGS_END KVMPPC_GSID_DPDES
> #define KVMPPC_GSE_DW_REGS_COUNT \
> (KVMPPC_GSE_DW_REGS_END - KVMPPC_GSE_DW_REGS_START + 1)
>
> diff --git a/arch/powerpc/include/asm/kvm_book3s.h
> b/arch/powerpc/include/asm/kvm_book3s.h
> index 3e1e2a698c9e..10618622d7ef 100644
> --- a/arch/powerpc/include/asm/kvm_book3s.h
> +++ b/arch/powerpc/include/asm/kvm_book3s.h
> @@ -594,6 +594,7 @@ static inline u##size kvmppc_get_##reg(struct kvm_vcpu
> *vcpu) \
>
>
> KVMPPC_BOOK3S_VCORE_ACCESSOR(vtb, 64, KVMPPC_GSID_VTB)
> +KVMPPC_BOOK3S_VCORE_ACCESSOR(dpdes, 64, KVMPPC_GSID_DPDES)
> KVMPPC_BOOK3S_VCORE_ACCESSOR_GET(arch_compat, 32, KVMPPC_GSID_LOGICAL_PVR)
> KVMPPC_BOOK3S_VCORE_ACCESSOR_GET(lpcr, 64, KVMPPC_GSID_LPCR)
> KVMPPC_BOOK3S_VCORE_ACCESSOR_SET(tb_offset, 64, KVMPPC_GSID_TB_OFFSET)
> diff --git a/arch/powerpc/kvm/book3s_hv_nestedv2.c
> b/arch/powerpc/kvm/book3s_hv_nestedv2.c
> index 8e6f5355f08b..36863fff2a99 100644
> --- a/arch/powerpc/kvm/book3s_hv_nestedv2.c
> +++ b/arch/powerpc/kvm/book3s_hv_nestedv2.c
> @@ -311,6 +311,10 @@ static int gs_msg_ops_vcpu_fill_info(struct
> kvmppc_gs_buff *gsb,
> rc = kvmppc_gse_put_u64(gsb, iden,
> vcpu->arch.vcore->vtb);
> break;
> + case KVMPPC_GSID_DPDES:
> + rc = kvmppc_gse_put_u64(gsb, iden,
> + vcpu->arch.vcore->dpdes);
> + break;
> case KVMPPC_GSID_LPCR:
> rc = kvmppc_gse_put_u64(gsb, iden,
> vcpu->arch.vcore->lpcr);
> @@ -543,6 +547,9 @@ static int gs_msg_ops_vcpu_refresh_info(struct
> kvmppc_gs_msg *gsm,
> case KVMPPC_GSID_VTB:
> vcpu->arch.vcore->vtb = kvmppc_gse_get_u64(gse);
> break;
> + case KVMPPC_GSID_DPDES:
> + vcpu->arch.vcore->dpdes = kvmppc_gse_get_u64(gse);
> + break;
> case KVMPPC_GSID_LPCR:
> vcpu->arch.vcore->lpcr = kvmppc_gse_get_u64(gse);
> break;
> diff --git a/arch/powerpc/kvm/test-guest-state-buffer.c
> b/arch/powerpc/kvm/test-guest-state-buffer.c
> index 4720b8dc8837..2571ccc618c9 100644
> --- a/arch/powerpc/kvm/test-guest
Re: [PATCH v2 2/2] arch/powerpc/kvm: Fix doorbell emulation for v2 API
On Wed Jun 5, 2024 at 9:39 PM AEST, Gautam Menghani wrote:
> Doorbell emulation is broken for KVM on PAPR guests as support for
> DPDES was not added in the initial patch series. Due to this, a KVM on
> PAPR guest with SMT > 1 cannot be booted with the XICS interrupt
> controller as doorbells are setup in the initial probe path when using XICS
> (pSeries_smp_probe()).
>
> Command to replicate the above bug:
>
> qemu-system-ppc64 \
> -drive file=rhel.qcow2,format=qcow2 \
> -m 20G \
> -smp 8,cores=1,threads=8 \
> -cpu host \
> -nographic \
> -machine pseries,ic-mode=xics -accel kvm
>
> Add doorbell state handling support in the host
> KVM code to fix doorbell emulation.
Reviewed-by: Nicholas Piggin
>
> Fixes: 19d31c5f1157 ("KVM: PPC: Add support for nestedv2 guests")
> Cc: sta...@vger.kernel.org # v6.7
> Signed-off-by: Gautam Menghani
> ---
> arch/powerpc/kvm/book3s_hv.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 35cb014a0c51..21c69647d27c 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -4116,6 +4116,11 @@ static int kvmhv_vcpu_entry_nestedv2(struct kvm_vcpu
> *vcpu, u64 time_limit,
> int trap;
> long rc;
>
> + if (vcpu->arch.doorbell_request) {
> + vcpu->arch.doorbell_request = 0;
> + kvmppc_set_dpdes(vcpu, 1);
> + }
> +
> io = &vcpu->arch.nestedv2_io;
>
> msr = mfmsr();
Re: [PATCH v2 0/2] Fix doorbell emulation for v2 API on PPC
On Wed Jun 5, 2024 at 9:39 PM AEST, Gautam Menghani wrote: > Doorbell emulation for KVM on PAPR guests is broken as support for DPDES > was not added in initial patch series [1]. > Add DPDES support and doorbell handling support for V2 API. Looks good, thanks. So fix for v1 doorbells is coming? Thanks, Nick > > [1] lore.kernel.org/linuxppc-dev/20230914030600.16993-1-jniet...@gmail.com > > Changes in v2: > 1. Split DPDES support into its own patch > > Gautam Menghani (2): > arch/powerpc/kvm: Add DPDES support in helper library for Guest state > buffer > arch/powerpc/kvm: Fix doorbell emulation for v2 API > > Documentation/arch/powerpc/kvm-nested.rst | 4 +++- > arch/powerpc/include/asm/guest-state-buffer.h | 3 ++- > arch/powerpc/include/asm/kvm_book3s.h | 1 + > arch/powerpc/kvm/book3s_hv.c | 5 + > arch/powerpc/kvm/book3s_hv_nestedv2.c | 7 +++ > arch/powerpc/kvm/test-guest-state-buffer.c| 2 +- > 6 files changed, 19 insertions(+), 3 deletions(-)
Re: [PATCH v2 0/8] KVM: PPC: Book3S HV: Nested guest migration fixes
On Wed Jun 5, 2024 at 11:06 PM AEST, Shivaprasad G Bhat wrote: > The series fixes the issues exposed by the kvm-unit-tests[1] > sprs-migration test. > > The SDAR, MMCR3 were seen to have some typo/refactoring bugs. > The first two patches fix them. > > The remaining patches take care of save-restoring the guest > state elements for DEXCR, HASHKEYR and HASHPKEYR SPRs with PHYP > during entry-exit. The KVM_PPC_REG too for them are missing which > are added for use by the QEMU. These and the qemu patches all look good now. I'll give them some testing and send R-B in the next day or two. I'm trying to write a k-u-t for the hashpkey migration case... Thanks, Nick > > References: > [1]: https://github.com/kvm-unit-tests/kvm-unit-tests > > --- > > Changelog: > v1: > https://lore.kernel.org/kvm/171741555734.11675.17428208097186191736.stgit@c0c876608f2d/ > - Reordered the patches in a way to introduce the SPRs first as >suggested. > - Added Reviewed-bys to the reviewed ones. > - Added 2 more patches to handle the hashpkeyr state > > Shivaprasad G Bhat (8): > KVM: PPC: Book3S HV: Fix the set_one_reg for MMCR3 > KVM: PPC: Book3S HV: Fix the get_one_reg of SDAR > KVM: PPC: Book3S HV: Add one-reg interface for DEXCR register > KVM: PPC: Book3S HV nestedv2: Keep nested guest DEXCR in sync > KVM: PPC: Book3S HV: Add one-reg interface for HASHKEYR register > KVM: PPC: Book3S HV nestedv2: Keep nested guest HASHKEYR in sync > KVM: PPC: Book3S HV: Add one-reg interface for HASHPKEYR register > KVM: PPC: Book3S HV nestedv2: Keep nested guest HASHPKEYR in sync > > > Documentation/virt/kvm/api.rst| 3 +++ > arch/powerpc/include/asm/kvm_host.h | 3 +++ > arch/powerpc/include/uapi/asm/kvm.h | 3 +++ > arch/powerpc/kvm/book3s_hv.c | 22 -- > arch/powerpc/kvm/book3s_hv.h | 3 +++ > arch/powerpc/kvm/book3s_hv_nestedv2.c | 18 ++ > 6 files changed, 50 insertions(+), 2 deletions(-) > > -- > Signature
Re: kswapd0: page allocation failure: order:0, mode:0x820(GFP_ATOMIC), nodemask=(null),cpuset=/,mems_allowed=0 (Kernel v6.5.9, 32bit ppc)
David Hildenbrand writes:
> On 01.06.24 08:01, Yu Zhao wrote:
>> On Wed, May 15, 2024 at 4:06 PM Yu Zhao wrote:
...
>>
>> Your system has 2GB memory and it uses zswap with zsmalloc (which is
>> good since it can allocate from the highmem zone) and zstd/lzo (which
>> doesn't matter much). Somehow -- I couldn't figure out why -- it
>> splits the 2GB into a 0.25GB DMA zone and a 1.75GB highmem zone:
>>
>> [0.00] Zone ranges:
>> [0.00] DMA [mem 0x-0x2fff]
>> [0.00] Normal empty
>> [0.00] HighMem [mem 0x3000-0x7fff]
>
> That's really odd. But we are messing with "PowerMac3,6", so I don't
> really know what's right or wrong ...
The DMA zone exists because 9739ab7eda45 ("powerpc: enable a 30-bit
ZONE_DMA for 32-bit pmac") selects it.
It's 768MB (not 0.25GB) because it's clamped at max_low_pfn:
#ifdef CONFIG_ZONE_DMA
max_zone_pfns[ZONE_DMA] = min(max_low_pfn,
1UL << (zone_dma_bits - PAGE_SHIFT));
#endif
Which comes eventually from CONFIG_LOWMEM_SIZE, which defaults to 768MB.
I think it's 768MB because the user:kernel split is 3G:1G, and then the
kernel needs some of that 1G virtual space for vmalloc/ioremap/highmem,
so it splits it 768M:256M.
Then ZONE_NORMAL is empty because it is also limited to max_low_pfn:
max_zone_pfns[ZONE_NORMAL] = max_low_pfn;
The rest of RAM is highmem.
So I think that's all behaving as expected, but I don't know 32-bit /
highmem stuff that well so I could be wrong.
cheers
Re: kswapd0: page allocation failure: order:0, mode:0x820(GFP_ATOMIC), nodemask=(null),cpuset=/,mems_allowed=0 (Kernel v6.5.9, 32bit ppc)
On Wed, Jun 5, 2024 at 9:12 PM Michael Ellerman wrote:
>
> David Hildenbrand writes:
> > On 01.06.24 08:01, Yu Zhao wrote:
> >> On Wed, May 15, 2024 at 4:06 PM Yu Zhao wrote:
> ...
> >>
> >> Your system has 2GB memory and it uses zswap with zsmalloc (which is
> >> good since it can allocate from the highmem zone) and zstd/lzo (which
> >> doesn't matter much). Somehow -- I couldn't figure out why -- it
> >> splits the 2GB into a 0.25GB DMA zone and a 1.75GB highmem zone:
> >>
> >> [0.00] Zone ranges:
> >> [0.00] DMA [mem 0x-0x2fff]
> >> [0.00] Normal empty
> >> [0.00] HighMem [mem 0x3000-0x7fff]
> >
> > That's really odd. But we are messing with "PowerMac3,6", so I don't
> > really know what's right or wrong ...
>
> The DMA zone exists because 9739ab7eda45 ("powerpc: enable a 30-bit
> ZONE_DMA for 32-bit pmac") selects it.
>
> It's 768MB (not 0.25GB) because it's clamped at max_low_pfn:
Right. (I meant 0.75GB.)
> #ifdef CONFIG_ZONE_DMA
> max_zone_pfns[ZONE_DMA] = min(max_low_pfn,
> 1UL << (zone_dma_bits - PAGE_SHIFT));
> #endif
>
> Which comes eventually from CONFIG_LOWMEM_SIZE, which defaults to 768MB.
I see. I grep'ed VMSPLIT which is used on x86 and arm but apparently
not on powerpc.
> I think it's 768MB because the user:kernel split is 3G:1G, and then the
> kernel needs some of that 1G virtual space for vmalloc/ioremap/highmem,
> so it splits it 768M:256M.
>
> Then ZONE_NORMAL is empty because it is also limited to max_low_pfn:
>
> max_zone_pfns[ZONE_NORMAL] = max_low_pfn;
>
> The rest of RAM is highmem.
>
> So I think that's all behaving as expected, but I don't know 32-bit /
> highmem stuff that well so I could be wrong.
Yes, the three zones work as intended.
Erhard,
Since your system only has 2GB memory, I'd try the 2G:2G split, which
would in theory allow both the kernel and userspace to all memory.
CONFIG_LOWMEM_SIZE_BOOL=y
CONFIG_LOWMEM_SIZE=0x700
(Michael, please correct me if the above wouldn't work.)
Re: kswapd0: page allocation failure: order:0, mode:0x820(GFP_ATOMIC), nodemask=(null),cpuset=/,mems_allowed=0 (Kernel v6.5.9, 32bit ppc)
On (24/06/06 10:49), Chengming Zhou wrote: > > Thanks for trying this out. This is interesting, so even two zpools is > > too much fragmentation for your use case. > > > > I think there are multiple ways to go forward here: > > (a) Make the number of zpools a config option, leave the default as > > 32, but allow special use cases to set it to 1 or similar. This is > > probably not preferable because it is not clear to users how to set > > it, but the idea is that no one will have to set it except special use > > cases such as Erhard's (who will want to set it to 1 in this case). > > > > (b) Make the number of zpools scale linearly with the number of CPUs. > > Maybe something like nr_cpus/4 or nr_cpus/8. The problem with this > > approach is that with a large number of CPUs, too many zpools will > > start having diminishing returns. Fragmentation will keep increasing, > > while the scalability/concurrency gains will diminish. > > > > (c) Make the number of zpools scale logarithmically with the number of > > CPUs. Maybe something like 4log2(nr_cpus). This will keep the number > > of zpools from increasing too much and close to the status quo. The > > problem is that at a small number of CPUs (e.g. 2), 4log2(nr_cpus) > > will actually give a nr_zpools > nr_cpus. So we will need to come up > > with a more fancy magic equation (e.g. 4log2(nr_cpus/4)). > > > > (d) Make the number of zpools scale linearly with memory. This makes > > more sense than scaling with CPUs because increasing the number of > > zpools increases fragmentation, so it makes sense to limit it by the > > available memory. This is also more consistent with other magic > > numbers we have (e.g. SWAP_ADDRESS_SPACE_SHIFT). > > > > The problem is that unlike zswap trees, the zswap pool is not > > connected to the swapfile size, so we don't have an indication for how > > much memory will be in the zswap pool. We can scale the number of > > zpools with the entire memory on the machine during boot, but this > > seems like it would be difficult to figure out, and will not take into > > consideration memory hotplugging and the zswap global limit changing. > > > > (e) A creative mix of the above. > > > > (f) Something else (probably simpler). > > > > I am personally leaning toward (c), but I want to hear the opinions of > > other people here. Yu, Vlastimil, Johannes, Nhat? Anyone else? > > > > In the long-term, I think we may want to address the lock contention > > in zsmalloc itself instead of zswap spawning multiple zpools. Sorry, I'm sure I'm not following this discussion closely enough, has the lock contention been demonstrated/proved somehow? lock-stats? > Agree, I think we should try to improve locking scalability of zsmalloc. > I have some thoughts to share, no code or test data yet: > > 1. First, we can change the pool global lock to per-class lock, which >is more fine-grained. Commit c0547d0b6a4b6 "zsmalloc: consolidate zs_pool's migrate_lock and size_class's locks" [1] claimed no significant difference between class->lock and pool->lock. [1] https://lkml.kernel.org/r/20221128191616.1261026-4-npha...@gmail.com
Re: [PATCH v2 0/2] Fix doorbell emulation for v2 API on PPC
On Thu, Jun 06, 2024 at 01:00:19PM GMT, Nicholas Piggin wrote: > On Wed Jun 5, 2024 at 9:39 PM AEST, Gautam Menghani wrote: > > Doorbell emulation for KVM on PAPR guests is broken as support for DPDES > > was not added in initial patch series [1]. > > Add DPDES support and doorbell handling support for V2 API. > > Looks good, thanks. So fix for v1 doorbells is coming? Yes I've root caused the doorbell breakage in V1 API to commit 7c3ded5735141ff4d049747c9f76672a8b737c49. I'll send out a fix soon. Thanks, Gautam
Re: kswapd0: page allocation failure: order:0, mode:0x820(GFP_ATOMIC), nodemask=(null),cpuset=/,mems_allowed=0 (Kernel v6.5.9, 32bit ppc)
On (24/06/06 12:46), Chengming Zhou wrote: > >> Agree, I think we should try to improve locking scalability of zsmalloc. > >> I have some thoughts to share, no code or test data yet: > >> > >> 1. First, we can change the pool global lock to per-class lock, which > >>is more fine-grained. > > > > Commit c0547d0b6a4b6 "zsmalloc: consolidate zs_pool's migrate_lock > > and size_class's locks" [1] claimed no significant difference > > between class->lock and pool->lock. > > Ok, I haven't looked into the history much, that seems preparation of trying > to introduce reclaim in the zsmalloc? Not sure. But now with the reclaim code > in zsmalloc has gone, should we change back to the per-class lock? Which is Well, the point that commit made was that Nhat (and Johannes?) were unable to detect any impact of pool->lock on a variety of cases. So we went on with code simplification. > obviously more fine-grained than the pool lock. Actually, I have just done it, > will test to get some data later. Thanks, we'll need data on this. I'm happy to take the patch, but jumping back and forth between class->lock and pool->lock merely "for obvious reasons" is not what I'm extremely excited about.
Re: [PATCH v2 0/2] Fix doorbell emulation for v2 API on PPC
On Wed, Jun 05, 2024 at 05:09:08PM GMT, Gautam Menghani wrote: > Doorbell emulation for KVM on PAPR guests is broken as support for DPDES > was not added in initial patch series [1]. > Add DPDES support and doorbell handling support for V2 API. > > [1] lore.kernel.org/linuxppc-dev/20230914030600.16993-1-jniet...@gmail.com > > Changes in v2: > 1. Split DPDES support into its own patch > > Gautam Menghani (2): > arch/powerpc/kvm: Add DPDES support in helper library for Guest state > buffer > arch/powerpc/kvm: Fix doorbell emulation for v2 API > > Documentation/arch/powerpc/kvm-nested.rst | 4 +++- > arch/powerpc/include/asm/guest-state-buffer.h | 3 ++- > arch/powerpc/include/asm/kvm_book3s.h | 1 + > arch/powerpc/kvm/book3s_hv.c | 5 + > arch/powerpc/kvm/book3s_hv_nestedv2.c | 7 +++ > arch/powerpc/kvm/test-guest-state-buffer.c| 2 +- > 6 files changed, 19 insertions(+), 3 deletions(-) > > -- > 2.45.1 > Hi Michael, This patch series is to be backported for all kernels >= 6.7. So the tag should be Cc: sta...@vger.kernel.org # v6.7+ and not Cc: sta...@vger.kernel.org # v6.7 Should I send a new version of this series or can you please make this change when pulling in your tree? Thanks, Gautam
Re: kswapd0: page allocation failure: order:0, mode:0x820(GFP_ATOMIC), nodemask=(null),cpuset=/,mems_allowed=0 (Kernel v6.5.9, 32bit ppc)
On 2024/6/6 07:41, Yosry Ahmed wrote: > On Wed, Jun 5, 2024 at 4:04 PM Erhard Furtner wrote: >> >> On Tue, 4 Jun 2024 20:03:27 -0700 >> Yosry Ahmed wrote: >> >>> Could you check if the attached patch helps? It basically changes the >>> number of zpools from 32 to min(32, nr_cpus). >> >> Thanks! The patch does not fix the issue but it helps. >> >> Means I still get to see the 'kswapd0: page allocation failure' in the >> dmesg, a 'stress-ng-vm: page allocation failure' later on, another kswapd0 >> error later on, etc. _but_ the machine keeps running the workload, stays >> usable via VNC and I get no hard crash any longer. >> >> Without patch kswapd0 error and hard crash (need to power-cycle) <3min. With >> patch several kswapd0 errors but running for 2 hrs now. I double checked >> this to be sure. > > Thanks for trying this out. This is interesting, so even two zpools is > too much fragmentation for your use case. > > I think there are multiple ways to go forward here: > (a) Make the number of zpools a config option, leave the default as > 32, but allow special use cases to set it to 1 or similar. This is > probably not preferable because it is not clear to users how to set > it, but the idea is that no one will have to set it except special use > cases such as Erhard's (who will want to set it to 1 in this case). > > (b) Make the number of zpools scale linearly with the number of CPUs. > Maybe something like nr_cpus/4 or nr_cpus/8. The problem with this > approach is that with a large number of CPUs, too many zpools will > start having diminishing returns. Fragmentation will keep increasing, > while the scalability/concurrency gains will diminish. > > (c) Make the number of zpools scale logarithmically with the number of > CPUs. Maybe something like 4log2(nr_cpus). This will keep the number > of zpools from increasing too much and close to the status quo. The > problem is that at a small number of CPUs (e.g. 2), 4log2(nr_cpus) > will actually give a nr_zpools > nr_cpus. So we will need to come up > with a more fancy magic equation (e.g. 4log2(nr_cpus/4)). > > (d) Make the number of zpools scale linearly with memory. This makes > more sense than scaling with CPUs because increasing the number of > zpools increases fragmentation, so it makes sense to limit it by the > available memory. This is also more consistent with other magic > numbers we have (e.g. SWAP_ADDRESS_SPACE_SHIFT). > > The problem is that unlike zswap trees, the zswap pool is not > connected to the swapfile size, so we don't have an indication for how > much memory will be in the zswap pool. We can scale the number of > zpools with the entire memory on the machine during boot, but this > seems like it would be difficult to figure out, and will not take into > consideration memory hotplugging and the zswap global limit changing. > > (e) A creative mix of the above. > > (f) Something else (probably simpler). > > I am personally leaning toward (c), but I want to hear the opinions of > other people here. Yu, Vlastimil, Johannes, Nhat? Anyone else? > > In the long-term, I think we may want to address the lock contention > in zsmalloc itself instead of zswap spawning multiple zpools. > Agree, I think we should try to improve locking scalability of zsmalloc. I have some thoughts to share, no code or test data yet: 1. First, we can change the pool global lock to per-class lock, which is more fine-grained. 2. Actually, we only need to take per-zspage lock when malloc/free, only need to take class lock when its fullness changed. 3. If this is not enough, we can have fewer fullness groups, so the need to take class lock becomes fewer. (will need some test data) More comments are welcome. Thanks!
Re: kswapd0: page allocation failure: order:0, mode:0x820(GFP_ATOMIC), nodemask=(null),cpuset=/,mems_allowed=0 (Kernel v6.5.9, 32bit ppc)
On 2024/6/6 12:31, Sergey Senozhatsky wrote:
> On (24/06/06 10:49), Chengming Zhou wrote:
>>> Thanks for trying this out. This is interesting, so even two zpools is
>>> too much fragmentation for your use case.
>>>
>>> I think there are multiple ways to go forward here:
>>> (a) Make the number of zpools a config option, leave the default as
>>> 32, but allow special use cases to set it to 1 or similar. This is
>>> probably not preferable because it is not clear to users how to set
>>> it, but the idea is that no one will have to set it except special use
>>> cases such as Erhard's (who will want to set it to 1 in this case).
>>>
>>> (b) Make the number of zpools scale linearly with the number of CPUs.
>>> Maybe something like nr_cpus/4 or nr_cpus/8. The problem with this
>>> approach is that with a large number of CPUs, too many zpools will
>>> start having diminishing returns. Fragmentation will keep increasing,
>>> while the scalability/concurrency gains will diminish.
>>>
>>> (c) Make the number of zpools scale logarithmically with the number of
>>> CPUs. Maybe something like 4log2(nr_cpus). This will keep the number
>>> of zpools from increasing too much and close to the status quo. The
>>> problem is that at a small number of CPUs (e.g. 2), 4log2(nr_cpus)
>>> will actually give a nr_zpools > nr_cpus. So we will need to come up
>>> with a more fancy magic equation (e.g. 4log2(nr_cpus/4)).
>>>
>>> (d) Make the number of zpools scale linearly with memory. This makes
>>> more sense than scaling with CPUs because increasing the number of
>>> zpools increases fragmentation, so it makes sense to limit it by the
>>> available memory. This is also more consistent with other magic
>>> numbers we have (e.g. SWAP_ADDRESS_SPACE_SHIFT).
>>>
>>> The problem is that unlike zswap trees, the zswap pool is not
>>> connected to the swapfile size, so we don't have an indication for how
>>> much memory will be in the zswap pool. We can scale the number of
>>> zpools with the entire memory on the machine during boot, but this
>>> seems like it would be difficult to figure out, and will not take into
>>> consideration memory hotplugging and the zswap global limit changing.
>>>
>>> (e) A creative mix of the above.
>>>
>>> (f) Something else (probably simpler).
>>>
>>> I am personally leaning toward (c), but I want to hear the opinions of
>>> other people here. Yu, Vlastimil, Johannes, Nhat? Anyone else?
>>>
>>> In the long-term, I think we may want to address the lock contention
>>> in zsmalloc itself instead of zswap spawning multiple zpools.
>
> Sorry, I'm sure I'm not following this discussion closely enough,
> has the lock contention been demonstrated/proved somehow? lock-stats?
Yosry has some stats in his commit b8cf32dc6e8c ("mm: zswap: multiple zpools
support"),
and I have also seen some locking contention when using zram to test kernel
building,
since zram still has only one pool.
>
>> Agree, I think we should try to improve locking scalability of zsmalloc.
>> I have some thoughts to share, no code or test data yet:
>>
>> 1. First, we can change the pool global lock to per-class lock, which
>>is more fine-grained.
>
> Commit c0547d0b6a4b6 "zsmalloc: consolidate zs_pool's migrate_lock
> and size_class's locks" [1] claimed no significant difference
> between class->lock and pool->lock.
Ok, I haven't looked into the history much, that seems preparation of trying
to introduce reclaim in the zsmalloc? Not sure. But now with the reclaim code
in zsmalloc has gone, should we change back to the per-class lock? Which is
obviously more fine-grained than the pool lock. Actually, I have just done it,
will test to get some data later.
Thanks.
>
> [1] https://lkml.kernel.org/r/20221128191616.1261026-4-npha...@gmail.com
Re: [PATCH 04/23] scsi: initialize scsi midlayer limits before allocating the queue
On Wed, Jun 05, 2024 at 10:37:53PM +1000, Michael Ellerman wrote: > On the other hand increasing max_segment_size to 64K while leaving > MAX_DBDMA_SEG > at 0xff00 seems to work fine. And that's effectively what's been happening on > existing kernels until now. Exactly. > > The only question is whether that violates some assumption elsewhere in the > SCSI layer? It shouldn't. > Anyway patch below that works for me on v6.10-rc2. This looks good to me: Reviewed-by: Christoph Hellwig
Re: kswapd0: page allocation failure: order:0, mode:0x820(GFP_ATOMIC), nodemask=(null),cpuset=/,mems_allowed=0 (Kernel v6.5.9, 32bit ppc)
On 2024/6/6 13:43, Sergey Senozhatsky wrote: > On (24/06/06 12:46), Chengming Zhou wrote: Agree, I think we should try to improve locking scalability of zsmalloc. I have some thoughts to share, no code or test data yet: 1. First, we can change the pool global lock to per-class lock, which is more fine-grained. >>> >>> Commit c0547d0b6a4b6 "zsmalloc: consolidate zs_pool's migrate_lock >>> and size_class's locks" [1] claimed no significant difference >>> between class->lock and pool->lock. >> >> Ok, I haven't looked into the history much, that seems preparation of trying >> to introduce reclaim in the zsmalloc? Not sure. But now with the reclaim code >> in zsmalloc has gone, should we change back to the per-class lock? Which is > > Well, the point that commit made was that Nhat (and Johannes?) were > unable to detect any impact of pool->lock on a variety of cases. So > we went on with code simplification. Right, the code is simpler. > >> obviously more fine-grained than the pool lock. Actually, I have just done >> it, >> will test to get some data later. > > Thanks, we'll need data on this. I'm happy to take the patch, but > jumping back and forth between class->lock and pool->lock merely > "for obvious reasons" is not what I'm extremely excited about. Yeah, agree, we need test data.
Re: [PATCH V3 05/14] tools/perf: Add disasm_line__parse to parse raw instruction for powerpc
Hello,
On Sat, Jun 01, 2024 at 11:39:32AM +0530, Athira Rajeev wrote:
> Currently, the perf tool infrastructure disasm_line__parse function to
> parse disassembled line.
>
> Example snippet from objdump:
> objdump --start-address= --stop-address= -d
> --no-show-raw-insn -C
>
> c10224b4: lwz r10,0(r9)
>
> This line "lwz r10,0(r9)" is parsed to extract instruction name,
> registers names and offset. In powerpc, the approach for data type
> profiling uses raw instruction instead of result from objdump to identify
> the instruction category and extract the source/target registers.
>
> Example: 38 01 81 e8 ld r4,312(r1)
>
> Here "38 01 81 e8" is the raw instruction representation. Add function
> "disasm_line__parse_powerpc" to handle parsing of raw instruction. Also
> update "struct ins" and "struct ins_operands" to save "opcode" and
> binary code. With the change, function captures:
>
> line -> "38 01 81 e8 ld r4,312(r1)"
> opcode and raw instruction "38 01 81 e8"
>
> Raw instruction is used later to extract the reg/offset fields. Macros
> are added to extract opcode and register fields. "struct ins_operands"
> and "struct ins" is updated to carry opcode and raw instruction binary
> code (raw_insn). Function "disasm_line__parse_powerpc fills the raw
> instruction hex value and opcode in newly added fields. There is no
> changes in existing code paths, which parses the disassembled code.
> The architecture using the instruction name and present approach is
> not altered. Since this approach targets powerpc, the macro
> implementation is added for powerpc as of now.
>
> Since the disasm_line__parse is used in other cases (perf annotate) and
> not only data tye profiling, the powerpc callback includes changes to
> work with binary code as well as mneumonic representation. Also in case
> if the DSO read fails and libcapstone is not supported, the approach
> fallback to use objdump as option. Hence as option, patch has changes to
> ensure objdump option also works well.
>
> Signed-off-by: Athira Rajeev
> ---
> tools/include/linux/string.h | 2 +
> tools/lib/string.c| 13
> .../perf/arch/powerpc/annotate/instructions.c | 1 +
> tools/perf/arch/powerpc/util/dwarf-regs.c | 9 +++
> tools/perf/util/disasm.c | 63 ++-
> tools/perf/util/disasm.h | 7 +++
> 6 files changed, 94 insertions(+), 1 deletion(-)
>
> diff --git a/tools/include/linux/string.h b/tools/include/linux/string.h
> index db5c99318c79..0acb1fc14e19 100644
> --- a/tools/include/linux/string.h
> +++ b/tools/include/linux/string.h
> @@ -46,5 +46,7 @@ extern char * __must_check skip_spaces(const char *);
>
> extern char *strim(char *);
>
> +extern void remove_spaces(char *s);
> +
> extern void *memchr_inv(const void *start, int c, size_t bytes);
> #endif /* _TOOLS_LINUX_STRING_H_ */
> diff --git a/tools/lib/string.c b/tools/lib/string.c
> index 8b6892f959ab..3126d2cff716 100644
> --- a/tools/lib/string.c
> +++ b/tools/lib/string.c
> @@ -153,6 +153,19 @@ char *strim(char *s)
> return skip_spaces(s);
> }
>
> +/*
> + * remove_spaces - Removes whitespaces from @s
> + */
> +void remove_spaces(char *s)
> +{
> + char *d = s;
> +
> + do {
> + while (*d == ' ')
> + ++d;
> + } while ((*s++ = *d++));
> +}
> +
> /**
> * strreplace - Replace all occurrences of character in string.
> * @s: The string to operate on.
> diff --git a/tools/perf/arch/powerpc/annotate/instructions.c
> b/tools/perf/arch/powerpc/annotate/instructions.c
> index a3f423c27cae..d57fd023ef9c 100644
> --- a/tools/perf/arch/powerpc/annotate/instructions.c
> +++ b/tools/perf/arch/powerpc/annotate/instructions.c
> @@ -55,6 +55,7 @@ static int powerpc__annotate_init(struct arch *arch, char
> *cpuid __maybe_unused)
> arch->initialized = true;
> arch->associate_instruction_ops =
> powerpc__associate_instruction_ops;
> arch->objdump.comment_char = '#';
> + annotate_opts.show_asm_raw = true;
> }
>
> return 0;
> diff --git a/tools/perf/arch/powerpc/util/dwarf-regs.c
> b/tools/perf/arch/powerpc/util/dwarf-regs.c
> index 0c4f4caf53ac..430623ca5612 100644
> --- a/tools/perf/arch/powerpc/util/dwarf-regs.c
> +++ b/tools/perf/arch/powerpc/util/dwarf-regs.c
> @@ -98,3 +98,12 @@ int regs_query_register_offset(const char *name)
> return roff->ptregs_offset;
> return -EINVAL;
> }
> +
> +#define PPC_OP(op) (((op) >> 26) & 0x3F)
> +#define PPC_RA(a)(((a) >> 16) & 0x1f)
> +#define PPC_RT(t)(((t) >> 21) & 0x1f)
> +#define PPC_RB(b)(((b) >> 11) & 0x1f)
> +#define PPC_D(D) ((D) & 0xfffe)
> +#define PPC_DS(DS) ((DS) & 0xfffc)
> +#define OP_LD58
> +#define OP_STD 62
> diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
> index 3cd187f08193..61f0f1656f8
Re: [PATCH V3 06/14] tools/perf: Update parameters for reg extract functions to use raw instruction on powerpc
On Sat, Jun 01, 2024 at 11:39:33AM +0530, Athira Rajeev wrote:
> Use the raw instruction code and macros to identify memory instructions,
> extract register fields and also offset. The implementation addresses
> the D-form, X-form, DS-form instructions. Two main functions are added.
> New parse function "load_store__parse" as instruction ops parser for
> memory instructions. Unlink other parser (like mov__parse), this parser
> fills in the "raw_insn" field for source/target and new added "mem_ref"
> field. Also set if it is multi_regs and opcode as well. No other fields
> are set because, here there is no need to parse the disassembled
> code and arch specific macros will take care of extracting offset and
> regs which is easier and will be precise.
>
> In powerpc, all instructions with a primary opcode from 32 to 63
> are memory instructions. Update "ins__find" function to have "raw_insn"
> also as a parameter. Don't use the "extract_reg_offset", instead use
> newly added function "get_arch_regs" which will set these fields: reg1,
> reg2, offset depending of where it is source or target ops.
>
> Signed-off-by: Athira Rajeev
> ---
> .../perf/arch/powerpc/annotate/instructions.c | 16 +
> tools/perf/arch/powerpc/util/dwarf-regs.c | 44 +
> tools/perf/util/annotate.c| 25 +++-
> tools/perf/util/disasm.c | 64 +--
> tools/perf/util/disasm.h | 4 +-
> tools/perf/util/include/dwarf-regs.h | 3 +
> 6 files changed, 147 insertions(+), 9 deletions(-)
>
> diff --git a/tools/perf/arch/powerpc/annotate/instructions.c
> b/tools/perf/arch/powerpc/annotate/instructions.c
> index d57fd023ef9c..10fea5e5cf4c 100644
> --- a/tools/perf/arch/powerpc/annotate/instructions.c
> +++ b/tools/perf/arch/powerpc/annotate/instructions.c
> @@ -49,6 +49,22 @@ static struct ins_ops
> *powerpc__associate_instruction_ops(struct arch *arch, con
> return ops;
> }
>
> +#define PPC_OP(op) (((op) >> 26) & 0x3F)
> +
> +static struct ins_ops *check_ppc_insn(int raw_insn)
> +{
> + int opcode = PPC_OP(raw_insn);
> +
> + /*
> + * Instructions with opcode 32 to 63 are memory
> + * instructions in powerpc
> + */
> + if ((opcode & 0x20))
> + return &load_store_ops;
> +
> + return NULL;
> +}
> +
> static int powerpc__annotate_init(struct arch *arch, char *cpuid
> __maybe_unused)
> {
> if (!arch->initialized) {
> diff --git a/tools/perf/arch/powerpc/util/dwarf-regs.c
> b/tools/perf/arch/powerpc/util/dwarf-regs.c
> index 430623ca5612..38b74fa01d8b 100644
> --- a/tools/perf/arch/powerpc/util/dwarf-regs.c
> +++ b/tools/perf/arch/powerpc/util/dwarf-regs.c
> @@ -107,3 +107,47 @@ int regs_query_register_offset(const char *name)
> #define PPC_DS(DS) ((DS) & 0xfffc)
> #define OP_LD58
> #define OP_STD 62
> +
> +static int get_source_reg(unsigned int raw_insn)
> +{
> + return PPC_RA(raw_insn);
> +}
> +
> +static int get_target_reg(unsigned int raw_insn)
> +{
> + return PPC_RT(raw_insn);
> +}
> +
> +static int get_offset_opcode(int raw_insn __maybe_unused)
The argument is used below, no need for __maybe_unused.
> +{
> + int opcode = PPC_OP(raw_insn);
> +
> + /* DS- form */
> + if ((opcode == OP_LD) || (opcode == OP_STD))
> + return PPC_DS(raw_insn);
> + else
> + return PPC_D(raw_insn);
> +}
> +
> +/*
> + * Fills the required fields for op_loc depending on if it
> + * is a source or target.
> + * D form: ins RT,D(RA) -> src_reg1 = RA, offset = D, dst_reg1 = RT
> + * DS form: ins RT,DS(RA) -> src_reg1 = RA, offset = DS, dst_reg1 = RT
> + * X form: ins RT,RA,RB -> src_reg1 = RA, src_reg2 = RB, dst_reg1 = RT
> + */
> +void get_arch_regs(int raw_insn __maybe_unused, int is_source __maybe_unused,
> + struct annotated_op_loc *op_loc __maybe_unused)
Ditto.
Thanks,
Namhyung
> +{
> + if (is_source)
> + op_loc->reg1 = get_source_reg(raw_insn);
> + else
> + op_loc->reg1 = get_target_reg(raw_insn);
> +
> + if (op_loc->multi_regs)
> + op_loc->reg2 = PPC_RB(raw_insn);
> +
> + /* TODO: Implement offset handling for X Form */
> + if ((op_loc->mem_ref) && (PPC_OP(raw_insn) != 31))
> + op_loc->offset = get_offset_opcode(raw_insn);
> +}
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 1451caf25e77..2b8cc759ae35 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -2079,6 +2079,12 @@ static int extract_reg_offset(struct arch *arch, const
> char *str,
> return 0;
> }
>
> +__weak void get_arch_regs(int raw_insn __maybe_unused, int is_source
> __maybe_unused,
> + struct annotated_op_loc *op_loc __maybe_unused)
> +{
> + return;
> +}
> +
> /**
> * annotate_get_insn_location - Get location of instruction
> * @arch: the architecture info
> @@ -2123,20 +2129,33 @@ int an
Re: [PATCH V3 10/14] tools/perf: Update instruction tracking for powerpc
On Sat, Jun 01, 2024 at 11:39:37AM +0530, Athira Rajeev wrote:
> Add instruction tracking function "update_insn_state_powerpc" for
> powerpc. Example sequence in powerpc:
>
> ld r10,264(r3)
> mr r31,r3
> <
> ld r9,312(r31)
>
> Consider ithe sample is pointing to: "ld r9,312(r31)".
> Here the memory reference is hit at "312(r31)" where 312 is the offset
> and r31 is the source register. Previous instruction sequence shows that
> register state of r3 is moved to r31. So to identify the data type for r31
> access, the previous instruction ("mr") needs to be tracked and the
> state type entry has to be updated. Current instruction tracking support
> in perf tools infrastructure is specific to x86. Patch adds this support
> for powerpc as well.
>
> Signed-off-by: Athira Rajeev
> ---
> .../perf/arch/powerpc/annotate/instructions.c | 65 +++
> tools/perf/util/annotate-data.c | 9 ++-
> tools/perf/util/disasm.c | 1 +
> 3 files changed, 74 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/arch/powerpc/annotate/instructions.c
> b/tools/perf/arch/powerpc/annotate/instructions.c
> index db72148eb857..3ecf5a986037 100644
> --- a/tools/perf/arch/powerpc/annotate/instructions.c
> +++ b/tools/perf/arch/powerpc/annotate/instructions.c
> @@ -231,6 +231,71 @@ static struct ins_ops *check_ppc_insn(int raw_insn)
> return NULL;
> }
>
> +/*
> + * Instruction tracking function to track register state moves.
> + * Example sequence:
> + *ld r10,264(r3)
> + *mr r31,r3
> + *<
> + *ld r9,312(r31)
> + *
> + * Previous instruction sequence shows that register state of r3
> + * is moved to r31. update_insn_state_powerpc tracks these state
> + * changes
> + */
> +#ifdef HAVE_DWARF_SUPPORT
> +static void update_insn_state_powerpc(struct type_state *state,
> + struct data_loc_info *dloc, Dwarf_Die * cu_die __maybe_unused,
> + struct disasm_line *dl)
> +{
> + struct annotated_insn_loc loc;
> + struct annotated_op_loc *src = &loc.ops[INSN_OP_SOURCE];
> + struct annotated_op_loc *dst = &loc.ops[INSN_OP_TARGET];
> + struct type_state_reg *tsr;
> + u32 insn_offset = dl->al.offset;
> +
> + if (annotate_get_insn_location(dloc->arch, dl, &loc) < 0)
> + return;
> +
> + /*
> + * Value 444 for bits 21:30 is for "mr"
> + * instruction. "mr" is extended OR. So set the
> + * source and destination reg correctly
> + */
> + if (PPC_21_30(dl->ops.raw_insn) == 444) {
> + int src_reg = src->reg1;
> +
> + src->reg1 = dst->reg1;
> + dst->reg1 = src_reg;
> + }
> +
> + if (!has_reg_type(state, dst->reg1))
> + return;
> +
> + tsr = &state->regs[dst->reg1];
> +
> + if (!has_reg_type(state, src->reg1) ||
> + !state->regs[src->reg1].ok) {
> + tsr->ok = false;
> + return;
> + }
> +
> + tsr->type = state->regs[src->reg1].type;
> + tsr->kind = state->regs[src->reg1].kind;
> + tsr->ok = true;
> +
> + pr_debug("mov [%x] reg%d -> reg%d",
pr_debug_dtp() ?
Thanks,
Namhyung
> + insn_offset, src->reg1, dst->reg1);
> + pr_debug_type_name(&tsr->type, tsr->kind);
> +}
> +#else /* HAVE_DWARF_SUPPORT */
> +static void update_insn_state_powerpc(struct type_state *state
> __maybe_unused, struct data_loc_info *dloc __maybe_unused,
> + Dwarf_Die * cu_die __maybe_unused, struct disasm_line *dl
> __maybe_unused)
> +{
> + return;
> +}
> +#endif /* HAVE_DWARF_SUPPORT */
> +
> static int powerpc__annotate_init(struct arch *arch, char *cpuid
> __maybe_unused)
> {
> if (!arch->initialized) {
> diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
> index 7a48c3d72b89..734acdd8c4b7 100644
> --- a/tools/perf/util/annotate-data.c
> +++ b/tools/perf/util/annotate-data.c
> @@ -1080,6 +1080,13 @@ static int find_data_type_insn(struct data_loc_info
> *dloc,
> return ret;
> }
>
> +static int arch_supports_insn_tracking(struct data_loc_info *dloc)
> +{
> + if ((arch__is(dloc->arch, "x86")) || (arch__is(dloc->arch, "powerpc")))
> + return 1;
> + return 0;
> +}
> +
> /*
> * Construct a list of basic blocks for each scope with variables and try to
> find
> * the data type by updating a type state table through instructions.
> @@ -1094,7 +1101,7 @@ static int find_data_type_block(struct data_loc_info
> *dloc,
> int ret = -1;
>
> /* TODO: other architecture support */
> - if (!arch__is(dloc->arch, "x86"))
> + if (!arch_supports_insn_tracking(dloc))
> return -1;
>
> prev_dst_ip = dst_ip = dloc->ip;
> diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
> index 57af4dc42a58..d8b357055302 100644
> --- a/tools/perf/util/disasm.c
> +++ b/tools/perf/util/disasm.c
> @@ -155,6 +155,7 @@ static struct arch archit
