LoongArch: KVM: Use kvm_get_vcpu_by_id() instead of kvm_get_vcpu()

[PATCH] LoongArch: KVM: Use kvm_get_vcpu_by_id() instead of kvm_get_vcpu()

Posted by Song Gao 6 months ago

Use kvm_get_vcpu() may can't get vcpu context, use kvm_get_vcpu_by_id()
instead of kvm_get_vcpu().

Signed-off-by: Song Gao <gaosong@loongson.cn>
---
 arch/loongarch/kvm/intc/eiointc.c | 5 ++++-
 arch/loongarch/kvm/intc/ipi.c     | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
index a3a12af9ecbf..5180da91d2e6 100644
--- a/arch/loongarch/kvm/intc/eiointc.c
+++ b/arch/loongarch/kvm/intc/eiointc.c
@@ -45,7 +45,10 @@ static void eiointc_update_irq(struct loongarch_eiointc *s, int irq, int level)
 	}
 
 	cpu = s->sw_coremap[irq];
-	vcpu = kvm_get_vcpu(s->kvm, cpu);
+	vcpu = kvm_get_vcpu_by_id(s->kvm, cpu);
+	if (unlikely(vcpu == NULL)) {
+		return;
+	}
 	if (level) {
 		/* if not enable return false */
 		if (!test_bit(irq, (unsigned long *)s->enable.reg_u32))
diff --git a/arch/loongarch/kvm/intc/ipi.c b/arch/loongarch/kvm/intc/ipi.c
index e658d5b37c04..0348a83a7ed7 100644
--- a/arch/loongarch/kvm/intc/ipi.c
+++ b/arch/loongarch/kvm/intc/ipi.c
@@ -298,7 +298,7 @@ static int kvm_ipi_regs_access(struct kvm_device *dev,
 	cpu = (attr->attr >> 16) & 0x3ff;
 	addr = attr->attr & 0xff;
 
-	vcpu = kvm_get_vcpu(dev->kvm, cpu);
+	vcpu = kvm_get_vcpu_by_id(dev->kvm, cpu);
 	if (unlikely(vcpu == NULL)) {
 		kvm_err("%s: invalid target cpu: %d\n", __func__, cpu);
 		return -EINVAL;
-- 
2.39.3

Re: [PATCH] LoongArch: KVM: Use kvm_get_vcpu_by_id() instead of kvm_get_vcpu()

Posted by Yanteng Si 6 months ago

在 8/11/25 10:55 AM, Song Gao 写道:
> Use kvm_get_vcpu() may can't get vcpu context, use kvm_get_vcpu_by_id()
> instead of kvm_get_vcpu().
Since using kvm_get_vcpu() may fail to retrieve the vcpu context,
kvm_get_vcpu_by_id() should be used instead.

Under this premise, and under the premise of making revisions
in accordance with Bibo's and Huacai's suggestions，pick up my tag in v2.

Reviewed-by: Yanteng Si <siyanteng@cqsoftware.com.cm>

Additionally, do we need a fixes tag?

Thanks,
Yanteng
> 
> Signed-off-by: Song Gao <gaosong@loongson.cn>
> ---
>   arch/loongarch/kvm/intc/eiointc.c | 5 ++++-
>   arch/loongarch/kvm/intc/ipi.c     | 2 +-
>   2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
> index a3a12af9ecbf..5180da91d2e6 100644
> --- a/arch/loongarch/kvm/intc/eiointc.c
> +++ b/arch/loongarch/kvm/intc/eiointc.c
> @@ -45,7 +45,10 @@ static void eiointc_update_irq(struct loongarch_eiointc *s, int irq, int level)
>   	}
>   
>   	cpu = s->sw_coremap[irq];
> -	vcpu = kvm_get_vcpu(s->kvm, cpu);
> +	vcpu = kvm_get_vcpu_by_id(s->kvm, cpu);
> +	if (unlikely(vcpu == NULL)) {
> +		return;
> +	}
>   	if (level) {
>   		/* if not enable return false */
>   		if (!test_bit(irq, (unsigned long *)s->enable.reg_u32))
> diff --git a/arch/loongarch/kvm/intc/ipi.c b/arch/loongarch/kvm/intc/ipi.c
> index e658d5b37c04..0348a83a7ed7 100644
> --- a/arch/loongarch/kvm/intc/ipi.c
> +++ b/arch/loongarch/kvm/intc/ipi.c
> @@ -298,7 +298,7 @@ static int kvm_ipi_regs_access(struct kvm_device *dev,
>   	cpu = (attr->attr >> 16) & 0x3ff;
>   	addr = attr->attr & 0xff;
>   
> -	vcpu = kvm_get_vcpu(dev->kvm, cpu);
> +	vcpu = kvm_get_vcpu_by_id(dev->kvm, cpu);
>   	if (unlikely(vcpu == NULL)) {
>   		kvm_err("%s: invalid target cpu: %d\n", __func__, cpu);
>   		return -EINVAL;

Re: [PATCH] LoongArch: KVM: Use kvm_get_vcpu_by_id() instead of kvm_get_vcpu()

Posted by gaosong 5 months, 4 weeks ago

在 2025/8/12 上午10:06, Yanteng Si 写道:
> 在 8/11/25 10:55 AM, Song Gao 写道:
>> Use kvm_get_vcpu() may can't get vcpu context, use kvm_get_vcpu_by_id()
>> instead of kvm_get_vcpu().
> Since using kvm_get_vcpu() may fail to retrieve the vcpu context,
> kvm_get_vcpu_by_id() should be used instead.
>
> Under this premise, and under the premise of making revisions
> in accordance with Bibo's and Huacai's suggestions，pick up my tag in v2.
>
Got it .
> Reviewed-by: Yanteng Si <siyanteng@cqsoftware.com.cm>
>
> Additionally, do we need a fixes tag?
>
I will add a fixes tag on v2.

Thanks.
Song Gao
> Thanks,
> Yanteng
>>
>> Signed-off-by: Song Gao <gaosong@loongson.cn>
>> ---
>>   arch/loongarch/kvm/intc/eiointc.c | 5 ++++-
>>   arch/loongarch/kvm/intc/ipi.c     | 2 +-
>>   2 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/loongarch/kvm/intc/eiointc.c 
>> b/arch/loongarch/kvm/intc/eiointc.c
>> index a3a12af9ecbf..5180da91d2e6 100644
>> --- a/arch/loongarch/kvm/intc/eiointc.c
>> +++ b/arch/loongarch/kvm/intc/eiointc.c
>> @@ -45,7 +45,10 @@ static void eiointc_update_irq(struct 
>> loongarch_eiointc *s, int irq, int level)
>>       }
>>         cpu = s->sw_coremap[irq];
>> -    vcpu = kvm_get_vcpu(s->kvm, cpu);
>> +    vcpu = kvm_get_vcpu_by_id(s->kvm, cpu);
>> +    if (unlikely(vcpu == NULL)) {
>> +        return;
>> +    }
>>       if (level) {
>>           /* if not enable return false */
>>           if (!test_bit(irq, (unsigned long *)s->enable.reg_u32))
>> diff --git a/arch/loongarch/kvm/intc/ipi.c 
>> b/arch/loongarch/kvm/intc/ipi.c
>> index e658d5b37c04..0348a83a7ed7 100644
>> --- a/arch/loongarch/kvm/intc/ipi.c
>> +++ b/arch/loongarch/kvm/intc/ipi.c
>> @@ -298,7 +298,7 @@ static int kvm_ipi_regs_access(struct kvm_device 
>> *dev,
>>       cpu = (attr->attr >> 16) & 0x3ff;
>>       addr = attr->attr & 0xff;
>>   -    vcpu = kvm_get_vcpu(dev->kvm, cpu);
>> +    vcpu = kvm_get_vcpu_by_id(dev->kvm, cpu);
>>       if (unlikely(vcpu == NULL)) {
>>           kvm_err("%s: invalid target cpu: %d\n", __func__, cpu);
>>           return -EINVAL;
>

Re: [PATCH] LoongArch: KVM: Use kvm_get_vcpu_by_id() instead of kvm_get_vcpu()

Posted by Bibo Mao 6 months ago


On 2025/8/11 上午10:55, Song Gao wrote:
> Use kvm_get_vcpu() may can't get vcpu context, use kvm_get_vcpu_by_id()
> instead of kvm_get_vcpu().
> 
> Signed-off-by: Song Gao <gaosong@loongson.cn>
> ---
>   arch/loongarch/kvm/intc/eiointc.c | 5 ++++-
>   arch/loongarch/kvm/intc/ipi.c     | 2 +-
>   2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
> index a3a12af9ecbf..5180da91d2e6 100644
> --- a/arch/loongarch/kvm/intc/eiointc.c
> +++ b/arch/loongarch/kvm/intc/eiointc.c
> @@ -45,7 +45,10 @@ static void eiointc_update_irq(struct loongarch_eiointc *s, int irq, int level)
>   	}
>   
>   	cpu = s->sw_coremap[irq];
> -	vcpu = kvm_get_vcpu(s->kvm, cpu);
> +	vcpu = kvm_get_vcpu_by_id(s->kvm, cpu);
> +	if (unlikely(vcpu == NULL)) {
> +		return;
> +	}
Brace {} is unnecessary with kernel coding style :), just something like 
this:
	if (unlikely(vcpu == NULL))
		return;

The other looks good to me.

Regards
Bibo Mao
>   	if (level) {
>   		/* if not enable return false */
>   		if (!test_bit(irq, (unsigned long *)s->enable.reg_u32))
> diff --git a/arch/loongarch/kvm/intc/ipi.c b/arch/loongarch/kvm/intc/ipi.c
> index e658d5b37c04..0348a83a7ed7 100644
> --- a/arch/loongarch/kvm/intc/ipi.c
> +++ b/arch/loongarch/kvm/intc/ipi.c
> @@ -298,7 +298,7 @@ static int kvm_ipi_regs_access(struct kvm_device *dev,
>   	cpu = (attr->attr >> 16) & 0x3ff;
>   	addr = attr->attr & 0xff;
>   
> -	vcpu = kvm_get_vcpu(dev->kvm, cpu);
> +	vcpu = kvm_get_vcpu_by_id(dev->kvm, cpu);
>   	if (unlikely(vcpu == NULL)) {
>   		kvm_err("%s: invalid target cpu: %d\n", __func__, cpu);
>   		return -EINVAL;
>

Re: [PATCH] LoongArch: KVM: Use kvm_get_vcpu_by_id() instead of kvm_get_vcpu()

Posted by Huacai Chen 6 months ago

On Mon, Aug 11, 2025 at 6:53 PM Bibo Mao <maobibo@loongson.cn> wrote:
>
>
>
> On 2025/8/11 上午10:55, Song Gao wrote:
> > Use kvm_get_vcpu() may can't get vcpu context, use kvm_get_vcpu_by_id()
> > instead of kvm_get_vcpu().
> >
> > Signed-off-by: Song Gao <gaosong@loongson.cn>
> > ---
> >   arch/loongarch/kvm/intc/eiointc.c | 5 ++++-
> >   arch/loongarch/kvm/intc/ipi.c     | 2 +-
> >   2 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
> > index a3a12af9ecbf..5180da91d2e6 100644
> > --- a/arch/loongarch/kvm/intc/eiointc.c
> > +++ b/arch/loongarch/kvm/intc/eiointc.c
> > @@ -45,7 +45,10 @@ static void eiointc_update_irq(struct loongarch_eiointc *s, int irq, int level)
> >       }
> >
> >       cpu = s->sw_coremap[irq];
> > -     vcpu = kvm_get_vcpu(s->kvm, cpu);
> > +     vcpu = kvm_get_vcpu_by_id(s->kvm, cpu);
> > +     if (unlikely(vcpu == NULL)) {
> > +             return;
> > +     }
> Brace {} is unnecessary with kernel coding style :), just something like
> this:
>         if (unlikely(vcpu == NULL))
>                 return;
>
> The other looks good to me.
Maybe add a print statement the same as kvm_ipi_regs_access()?

Huacai

>
> Regards
> Bibo Mao
> >       if (level) {
> >               /* if not enable return false */
> >               if (!test_bit(irq, (unsigned long *)s->enable.reg_u32))
> > diff --git a/arch/loongarch/kvm/intc/ipi.c b/arch/loongarch/kvm/intc/ipi.c
> > index e658d5b37c04..0348a83a7ed7 100644
> > --- a/arch/loongarch/kvm/intc/ipi.c
> > +++ b/arch/loongarch/kvm/intc/ipi.c
> > @@ -298,7 +298,7 @@ static int kvm_ipi_regs_access(struct kvm_device *dev,
> >       cpu = (attr->attr >> 16) & 0x3ff;
> >       addr = attr->attr & 0xff;
> >
> > -     vcpu = kvm_get_vcpu(dev->kvm, cpu);
> > +     vcpu = kvm_get_vcpu_by_id(dev->kvm, cpu);
> >       if (unlikely(vcpu == NULL)) {
> >               kvm_err("%s: invalid target cpu: %d\n", __func__, cpu);
> >               return -EINVAL;
> >
>

Re: [PATCH] LoongArch: KVM: Use kvm_get_vcpu_by_id() instead of kvm_get_vcpu()

Posted by Bibo Mao 6 months ago


On 2025/8/11 下午9:23, Huacai Chen wrote:
> On Mon, Aug 11, 2025 at 6:53 PM Bibo Mao <maobibo@loongson.cn> wrote:
>>
>>
>>
>> On 2025/8/11 上午10:55, Song Gao wrote:
>>> Use kvm_get_vcpu() may can't get vcpu context, use kvm_get_vcpu_by_id()
>>> instead of kvm_get_vcpu().
>>>
>>> Signed-off-by: Song Gao <gaosong@loongson.cn>
>>> ---
>>>    arch/loongarch/kvm/intc/eiointc.c | 5 ++++-
>>>    arch/loongarch/kvm/intc/ipi.c     | 2 +-
>>>    2 files changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
>>> index a3a12af9ecbf..5180da91d2e6 100644
>>> --- a/arch/loongarch/kvm/intc/eiointc.c
>>> +++ b/arch/loongarch/kvm/intc/eiointc.c
>>> @@ -45,7 +45,10 @@ static void eiointc_update_irq(struct loongarch_eiointc *s, int irq, int level)
>>>        }
>>>
>>>        cpu = s->sw_coremap[irq];
>>> -     vcpu = kvm_get_vcpu(s->kvm, cpu);
>>> +     vcpu = kvm_get_vcpu_by_id(s->kvm, cpu);
>>> +     if (unlikely(vcpu == NULL)) {
>>> +             return;
>>> +     }
>> Brace {} is unnecessary with kernel coding style :), just something like
>> this:
>>          if (unlikely(vcpu == NULL))
>>                  return;
>>
>> The other looks good to me.
> Maybe add a print statement the same as kvm_ipi_regs_access()?
Both are ok for me now.

In future there may be change with return value and printk message with 
in-kernel MMIO/IOCSR read/write emulation.

About return value there should be -EOPNOTSUPP if addr is not in range 
of device, it will transfer the emulation to user space VMM. Other value 
such as -EINVAL should be supported, and should not transfer to VMM 
since the address is in range of device.

About printk message I have no idea now, it may cause lots of noise 
message in host because of bad guest kernel, however abnormal behavior 
should be recorded anyway.

Regards
Bibo Mao
> 
> Huacai
> 
>>
>> Regards
>> Bibo Mao
>>>        if (level) {
>>>                /* if not enable return false */
>>>                if (!test_bit(irq, (unsigned long *)s->enable.reg_u32))
>>> diff --git a/arch/loongarch/kvm/intc/ipi.c b/arch/loongarch/kvm/intc/ipi.c
>>> index e658d5b37c04..0348a83a7ed7 100644
>>> --- a/arch/loongarch/kvm/intc/ipi.c
>>> +++ b/arch/loongarch/kvm/intc/ipi.c
>>> @@ -298,7 +298,7 @@ static int kvm_ipi_regs_access(struct kvm_device *dev,
>>>        cpu = (attr->attr >> 16) & 0x3ff;
>>>        addr = attr->attr & 0xff;
>>>
>>> -     vcpu = kvm_get_vcpu(dev->kvm, cpu);
>>> +     vcpu = kvm_get_vcpu_by_id(dev->kvm, cpu);
>>>        if (unlikely(vcpu == NULL)) {
>>>                kvm_err("%s: invalid target cpu: %d\n", __func__, cpu);
>>>                return -EINVAL;
>>>
>>