Skip to contentSkip to conversation

Reason for extra struct kvm_run parameter in MMIO handling?

DISCUSSION Andre Przywara 3 participants 4 messages Feb 6, 2015 – Feb 9, 2015 KVM

lore
Share & tools
lore
#0 lore raw

Hi,

while reworking the ARM vGIC emulation layer to use the kvm_io_bus framework, I wonder why the whole KVM MMIO handling always passes the pointer to struct kvm_run along with the struct kvm_vcpu pointer. As kvm_run is a member of kvm_vcpu, the vcpu alone should be sufficient, right?
Also I see the very top of the call chain is effectively:
kvm_arch_vcpu_ioctl_run(vcpu, vcpu->run);

So is that just legacy code still lingering around from the dawn of time and nobody dared to rework this or is there a particular reason for doing so?

I am asking because I lack the kvm_run pointer in the MMIO handler, so I just use vcpu->run and I wonder if there are potential issues in doing so.

Cheers,
Andre.

#1 lore raw

2015-02-06 17:30+0000, Andre Przywara:

Quoted 2 lines

As kvm_run is a member of kvm_vcpu, the vcpu alone should be sufficient, right?

Yes.

Quoted 2 lines

Also I see the very top of the call chain is effectively:
kvm_arch_vcpu_ioctl_run(vcpu, vcpu->run);

:(

Quoted 3 lines

So is that just legacy code still lingering around from the dawn of time and nobody dared to rework this or is there a particular reason for doing so?

Probably laziness in 9a2bb7f486dc639a1cf2ad803bf2227f0dc0809d.

Quoted 2 lines

I am asking because I lack the kvm_run pointer in the MMIO handler, so I just use vcpu->run and I wonder if there are potential issues in doing so.

We do it at few places already, so there, hopefully, is no problem.

maintainer
#2 lore raw
Quoted 5 lines · Radim Krčmář
Quoted 3 lines

So is that just legacy code still lingering around from the dawn of time and nobody dared to rework this or is there a particular reason for doing so?

Probably laziness in 9a2bb7f486dc639a1cf2ad803bf2227f0dc0809d.

Or microoptimization. Laziness is more likely though.

Quoted 4 lines
Quoted 2 lines

I am asking because I lack the kvm_run pointer in the MMIO handler, so I just use vcpu->run and I wonder if there are potential issues in doing so.

We do it at few places already, so there, hopefully, is no problem.

Yup. Thanks Radim for answering!

Paolo

#3 lore raw

Radim, Paolo,

Quoted 7 lines · Paolo Bonzini
Quoted 5 lines · Radim Krčmář
Quoted 3 lines

So is that just legacy code still lingering around from the dawn of time and nobody dared to rework this or is there a particular reason for doing so?

Probably laziness in 9a2bb7f486dc639a1cf2ad803bf2227f0dc0809d.

Quoted 8 lines

Or microoptimization. Laziness is more likely though.

Quoted 4 lines
Quoted 2 lines

I am asking because I lack the kvm_run pointer in the MMIO handler, so I just use vcpu->run and I wonder if there are potential issues in doing so.

We do it at few places already, so there, hopefully, is no problem.

Yup. Thanks Radim for answering!

thanks for the answer and the confirmation! Looks like I don't dare to write a patch for that ;-)

Cheers,
Andre.