Skip to content

Conversation

SiddhiK29
Copy link
Member

No description provided.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Jun 10, 2025
Copy link
Member

@brenns10 brenns10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! A few small nits, but nothing major. Thank you :)

Comment on lines 46 to 50
class KvmIoBus(enum.Enum):
KVM_MMIO_BUS = 0
KVM_PIO_BUS = 1
KVM_VIRTIO_CCW_NOTIFY_BUS = 2
KVM_FAST_MMIO_BUS = 3
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This corresponds to enum kvm_bus, right? Since there's a nice enum present from UEK5 to mainline, let's use that instead of hard-coding it here.

Notably, this definition here seems to be missing KVM_IOCSR_BUS which I see in the definition, which is a great reason to just use the debuginfo.

enum kvm_bus {
	KVM_MMIO_BUS,
	KVM_PIO_BUS,
	KVM_VIRTIO_CCW_NOTIFY_BUS,
	KVM_FAST_MMIO_BUS,
	KVM_IOCSR_BUS,
	KVM_NR_BUSES
};

iobus = hex(bus.value_())
dev_count = bus.dev_count.value_()
eventfd_count = bus.ioeventfd_count.value_()
bus_name = KvmIoBus(i).name
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To get the enum name here, we can do this:

Suggested change
bus_name = KvmIoBus(i).name
bus_name = enum_name_get(kvm_bus_type, i)

You can declare kvm_bus_type outside all the loops like this:

kvm_bus_type = prog.type("enum kvm_bus")

And you can import enum_name_get() at the top:

from drgn_tools.util import enum_name_get

Comment on lines 313 to 314
i = 0
for bus in iobus_iterator:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than manually managing this counter, you can do:

for i, bus in enumerate(io_bus_iterator):
    ...

vcpu_stat = vcpu.stat
rows_vcpu = [
[
"\nVCPU:",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't include the \n here, instead have separate print() above this to put in the newline. Otherwise, the print_table() code considers it a character for alignment purposes, and it misaligns the first row. EG:

VCPU:         0        HALT_SUC:        467     HALT_ATTMPT:    881     HALT_INV:    0
PF_FIXED:      971      PF_GUEST:        0       TLB_FLUSH:      15563   INVLPG:      0
EXITS:         1290959  IO_EXIT:         128538  MMIO_EXIT:      35913   SIG_EXIT:    0
IRQ_WIN_EXIT:  31       NMI_WIN_EXIT:    0       L1D_FLUSH:      0       HALT_EXIT:   2278
REQ_IRQ_EXIT:  0        IRQ_EXITS:       2190    HOST_STATE_RL:  165126  FPU_RL:      164309
INSN_EMUL:     54456    INSN_EMUL_FAIL:  0       HYPERCALLS:     6       IRQ_INJ:     61
NMI_INJ:       0        REQ_EVENT:       46633   PREEMPT_RPT:    18      PREEMT_OTH:  12

@brenns10
Copy link
Member

Oh, and one more thing: can you file an Orabug for this (targeting "next" -- see the developer workflow doc), and then include the Orabug statement in the commit message? This will also make sure you get credit in the changelog :)

@SiddhiK29
Copy link
Member Author

Oh, and one more thing: can you file an Orabug for this (targeting "next" -- see the developer workflow doc), and then include the Orabug statement in the commit message? This will also make sure you get credit in the changelog :)

Hi Stephen,
I already have a Orabug filed targeting "next". The Orabug 37713468 is mentioned in commit message.
Can you please confirm if it's fine?

@brenns10
Copy link
Member

I already have a Orabug filed targeting "next". The Orabug 37713468 is mentioned in commit message.
Can you please confirm if it's fine?

I don't know why I missed that, my apologies. That's perfect, thank you.

@SiddhiK29 SiddhiK29 force-pushed the kvmstats branch 2 times, most recently from 7e0e32a to f3e884b Compare August 19, 2025 06:34
@SiddhiK29 SiddhiK29 force-pushed the kvmstats branch 2 times, most recently from ad839f3 to 2adcee7 Compare September 11, 2025 06:42
@SiddhiK29 SiddhiK29 requested a review from brenns10 September 12, 2025 04:59
brenns10
brenns10 previously approved these changes Sep 15, 2025
Copy link
Member

@brenns10 brenns10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All I did was rebase on the main branch to get the fix for the v6.16 UEK-next kernels.

With that, this looks good, thank you!

@brenns10 brenns10 merged commit e47a548 into oracle-samples:main Sep 16, 2025
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants