[PATCH 1/2] powerpc: kvm: use generic transfer to guest mode work
Shrikanth Hegde
sshegde at linux.ibm.com
Fri Apr 25 01:57:59 AEST 2025
On 4/24/25 20:12, Sebastian Andrzej Siewior wrote:
Thanks Sebastian for taking a look.
> On 2025-04-21 15:58:36 [+0530], Shrikanth Hegde wrote:
>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>> index 19f4d298d..123539642 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -80,8 +80,8 @@
>> #include <asm/ultravisor.h>
>> #include <asm/dtl.h>
>> #include <asm/plpar_wrappers.h>
>> -
>> #include <trace/events/ipi.h>
>> +#include <linux/entry-kvm.h>
>>
>> #include "book3s.h"
>> #include "book3s_hv.h"
>> @@ -4901,7 +4901,7 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu, u64 time_limit,
>> }
>>
>> if (need_resched())
>> - cond_resched();
>> + schedule();
>
> This looks unrelated and odd. I don't why but this should be a
> cond_resched() so it can be optimized away on PREEMPT kernels.
This is needed, otherwise KVM on powerVM setup gets stuck on preempt=full/lazy.
>
>> kvmppc_update_vpas(vcpu);
>>
>> @@ -5097,10 +5097,11 @@ static int kvmppc_vcpu_run_hv(struct kvm_vcpu *vcpu)
>> return -EINVAL;
>> }
>>
>> - /* No need to go into the guest when all we'll do is come back out */
>> - if (signal_pending(current)) {
>> - run->exit_reason = KVM_EXIT_INTR;
>> - return -EINTR;
>> + /* use generic frameworks to handle signals, need_resched */
>> + if (__xfer_to_guest_mode_work_pending()) {
>> + r = xfer_to_guest_mode_handle_work(vcpu);
> This could be unconditional.
>
>> + if (r)
>> + return r;
>> }
>>
>> #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
>> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
>> index 153587741..4ff334532 100644
>> --- a/arch/powerpc/kvm/powerpc.c
>> +++ b/arch/powerpc/kvm/powerpc.c
>> @@ -34,6 +34,7 @@
>> #endif
>> #include <asm/ultravisor.h>
>> #include <asm/setup.h>
>> +#include <linux/entry-kvm.h>
>>
>> #include "timing.h"
>> #include "../mm/mmu_decl.h"
>> @@ -80,24 +81,17 @@ int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu)
>> {
>> int r;
>>
>> + /* use generic framework to handle need resched and signals */
>> + if (__xfer_to_guest_mode_work_pending()) {
>> + r = xfer_to_guest_mode_handle_work(vcpu);
>
> there is nothing special you do checking and handling the work. Couldn't
> you invoke xfer_to_guest_mode_handle_work() unconditionally?
>
I followed what was in arch/x86/kvm/x86.c. Since xfer_to_guest_mode_handle_work does the same check
it makes sense to call it without checks too.
Will update in v2.
>> + if (r)
>> + return r;
>> + }
>> +
>> WARN_ON(irqs_disabled());
>> hard_irq_disable();
>>
>> while (true) {
>> - if (need_resched()) {
>> - local_irq_enable();
>> - cond_resched();
>> - hard_irq_disable();
>> - continue;
>> - }
>> -
>> - if (signal_pending(current)) {
>> - kvmppc_account_exit(vcpu, SIGNAL_EXITS);
>> - vcpu->run->exit_reason = KVM_EXIT_INTR;
>> - r = -EINTR;
>> - break;
>
> I don't how this works but couldn't SIGNAL_EXITS vanish now that it
> isn't updated anymore? The stat itself moves in kvm_handle_signal_exit()
> to a different counter so it is not lost. The reader just needs to look
> somewhere else for it.
ok. thanks for pointing out.
AFAIU it is updating the stats mostly. But below could keep the stats happy.
I will update that in v2.
if (__xfer_to_guest_mode_work_pending()) {
r = xfer_to_guest_mode_handle_work(vcpu);
+ /* generic framework doesn't update ppc specific stats*/
+ if (r == -EINTR)
+ kvmppc_account_exit(vcpu, SIGNAL_EXITS);
if (r)
return r;
>
>> - }
>> -
>> vcpu->mode = IN_GUEST_MODE;
>>
>> /*
>
> Sebastian
More information about the Linuxppc-dev
mailing list