* [PATCH v3 0/9] LoongArch: KVM: INTC: Enhancement about eiointc emulation
@ 2025-06-11 1:46 Bibo Mao
2025-06-11 1:46 ` [PATCH v3 1/9] LoongArch: KVM: INTC: Fix interrupt route update with eiointc Bibo Mao
` (9 more replies)
0 siblings, 10 replies; 22+ messages in thread
From: Bibo Mao @ 2025-06-11 1:46 UTC (permalink / raw)
To: Tianrui Zhao, Huacai Chen, Xianglai Li; +Cc: kvm, loongarch, linux-kernel
This series fix five issues about kernel eiointc emulation list as
follows:
1. The first patch fixes type forced assignment issue.
2. The second patch fixes interrupt route with physical cpu.
3. The third patch disables update property num_cpu and feature
4. The fourth patch adds validation check about num_cpu from user
space.
5. Overflow with array index when emulate register EIOINTC_ENABLE
writing operation.
Also there is code cleanup with kernel eiointc emulation.
---
v2 ... v3:
1. Add prefix INTC: in title of every patch.
2. Fix array index overflow when emulate register EIOINTC_ENABLE
writing operation.
3. Add address alignment check with eiointc register access operation.
v1 ... v2:
1. Add extra fix in patch 3 and patch 4, add num_cpu validation check
2. Name of stat information keeps unchanged, only move it from VM stat
to vCPU stat.
---
Bibo Mao (9):
LoongArch: KVM: INTC: Fix interrupt route update with eiointc
LoongArch: KVM: INTC: Check interrupt route from physical cpu
LoongArch: KVM: INTC: Disable update property num_cpu and feature
LoongArch: KVM: INTC: Check validation of num_cpu from user space
LoongArch: KVM: INTC: Avoid overflow with array index
LoongArch: KVM: INTC: Use standard bitops API with eiointc
LoongArch: KVM: INTC: Remove unused parameter len
LoongArch: KVM: INTC: Add stat information with kernel irqchip
LoongArch: KVM: INTC: Add address alignment check
arch/loongarch/include/asm/kvm_host.h | 12 +-
arch/loongarch/kvm/intc/eiointc.c | 167 +++++++++++++++-----------
arch/loongarch/kvm/intc/ipi.c | 28 +----
arch/loongarch/kvm/intc/pch_pic.c | 4 +-
arch/loongarch/kvm/vcpu.c | 8 +-
5 files changed, 117 insertions(+), 102 deletions(-)
base-commit: aef17cb3d3c43854002956f24c24ec8e1a0e3546
--
2.39.3
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 1/9] LoongArch: KVM: INTC: Fix interrupt route update with eiointc
2025-06-11 1:46 [PATCH v3 0/9] LoongArch: KVM: INTC: Enhancement about eiointc emulation Bibo Mao
@ 2025-06-11 1:46 ` Bibo Mao
2025-06-11 1:46 ` [PATCH v3 2/9] LoongArch: KVM: INTC: Check interrupt route from physical cpu Bibo Mao
` (8 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Bibo Mao @ 2025-06-11 1:46 UTC (permalink / raw)
To: Tianrui Zhao, Huacai Chen, Xianglai Li
Cc: kvm, loongarch, linux-kernel, stable
With function eiointc_update_sw_coremap(), there is forced assignment
like val = *(u64 *)pvalue. Parameter pvalue may be pointer to char type
or others, there is problem with forced assignment with u64 type.
Here the detailed value is passed rather address pointer.
Cc: stable@vger.kernel.org
Fixes: 3956a52bc05b ("LoongArch: KVM: Add EIOINTC read and write functions")
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
arch/loongarch/kvm/intc/eiointc.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
index f39929d7bf8a..d2c521b0e923 100644
--- a/arch/loongarch/kvm/intc/eiointc.c
+++ b/arch/loongarch/kvm/intc/eiointc.c
@@ -66,10 +66,9 @@ static void eiointc_update_irq(struct loongarch_eiointc *s, int irq, int level)
}
static inline void eiointc_update_sw_coremap(struct loongarch_eiointc *s,
- int irq, void *pvalue, u32 len, bool notify)
+ int irq, u64 val, u32 len, bool notify)
{
int i, cpu;
- u64 val = *(u64 *)pvalue;
for (i = 0; i < len; i++) {
cpu = val & 0xff;
@@ -398,7 +397,7 @@ static int loongarch_eiointc_writeb(struct kvm_vcpu *vcpu,
irq = offset - EIOINTC_COREMAP_START;
index = irq;
s->coremap.reg_u8[index] = data;
- eiointc_update_sw_coremap(s, irq, (void *)&data, sizeof(data), true);
+ eiointc_update_sw_coremap(s, irq, data, sizeof(data), true);
break;
default:
ret = -EINVAL;
@@ -484,7 +483,7 @@ static int loongarch_eiointc_writew(struct kvm_vcpu *vcpu,
irq = offset - EIOINTC_COREMAP_START;
index = irq >> 1;
s->coremap.reg_u16[index] = data;
- eiointc_update_sw_coremap(s, irq, (void *)&data, sizeof(data), true);
+ eiointc_update_sw_coremap(s, irq, data, sizeof(data), true);
break;
default:
ret = -EINVAL;
@@ -570,7 +569,7 @@ static int loongarch_eiointc_writel(struct kvm_vcpu *vcpu,
irq = offset - EIOINTC_COREMAP_START;
index = irq >> 2;
s->coremap.reg_u32[index] = data;
- eiointc_update_sw_coremap(s, irq, (void *)&data, sizeof(data), true);
+ eiointc_update_sw_coremap(s, irq, data, sizeof(data), true);
break;
default:
ret = -EINVAL;
@@ -656,7 +655,7 @@ static int loongarch_eiointc_writeq(struct kvm_vcpu *vcpu,
irq = offset - EIOINTC_COREMAP_START;
index = irq >> 3;
s->coremap.reg_u64[index] = data;
- eiointc_update_sw_coremap(s, irq, (void *)&data, sizeof(data), true);
+ eiointc_update_sw_coremap(s, irq, data, sizeof(data), true);
break;
default:
ret = -EINVAL;
@@ -809,7 +808,7 @@ static int kvm_eiointc_ctrl_access(struct kvm_device *dev,
for (i = 0; i < (EIOINTC_IRQS / 4); i++) {
start_irq = i * 4;
eiointc_update_sw_coremap(s, start_irq,
- (void *)&s->coremap.reg_u32[i], sizeof(u32), false);
+ s->coremap.reg_u32[i], sizeof(u32), false);
}
break;
default:
--
2.39.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v3 2/9] LoongArch: KVM: INTC: Check interrupt route from physical cpu
2025-06-11 1:46 [PATCH v3 0/9] LoongArch: KVM: INTC: Enhancement about eiointc emulation Bibo Mao
2025-06-11 1:46 ` [PATCH v3 1/9] LoongArch: KVM: INTC: Fix interrupt route update with eiointc Bibo Mao
@ 2025-06-11 1:46 ` Bibo Mao
2025-06-11 1:46 ` [PATCH v3 3/9] LoongArch: KVM: INTC: Disable update property num_cpu and feature Bibo Mao
` (7 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Bibo Mao @ 2025-06-11 1:46 UTC (permalink / raw)
To: Tianrui Zhao, Huacai Chen, Xianglai Li
Cc: kvm, loongarch, linux-kernel, stable
With eiointc interrupt controller, physical cpu id is set for irq
route. However function kvm_get_vcpu() is used to get destination vCPU
when delivering irq. With API kvm_get_vcpu(), logical cpu is used.
With API kvm_get_vcpu_by_cpuid(), vCPU can be searched from physical
cpu id.
Cc: stable@vger.kernel.org
Fixes: 3956a52bc05b ("LoongArch: KVM: Add EIOINTC read and write functions")
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
arch/loongarch/kvm/intc/eiointc.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
index d2c521b0e923..0b648c56b0c3 100644
--- a/arch/loongarch/kvm/intc/eiointc.c
+++ b/arch/loongarch/kvm/intc/eiointc.c
@@ -9,7 +9,8 @@
static void eiointc_set_sw_coreisr(struct loongarch_eiointc *s)
{
- int ipnum, cpu, irq_index, irq_mask, irq;
+ int ipnum, cpu, irq_index, irq_mask, irq, cpuid;
+ struct kvm_vcpu *vcpu;
for (irq = 0; irq < EIOINTC_IRQS; irq++) {
ipnum = s->ipmap.reg_u8[irq / 32];
@@ -20,7 +21,12 @@ static void eiointc_set_sw_coreisr(struct loongarch_eiointc *s)
irq_index = irq / 32;
irq_mask = BIT(irq & 0x1f);
- cpu = s->coremap.reg_u8[irq];
+ cpuid = s->coremap.reg_u8[irq];
+ vcpu = kvm_get_vcpu_by_cpuid(s->kvm, cpuid);
+ if (vcpu == NULL)
+ continue;
+
+ cpu = vcpu->vcpu_id;
if (!!(s->coreisr.reg_u32[cpu][irq_index] & irq_mask))
set_bit(irq, s->sw_coreisr[cpu][ipnum]);
else
@@ -68,17 +74,23 @@ static void eiointc_update_irq(struct loongarch_eiointc *s, int irq, int level)
static inline void eiointc_update_sw_coremap(struct loongarch_eiointc *s,
int irq, u64 val, u32 len, bool notify)
{
- int i, cpu;
+ int i, cpu, cpuid;
+ struct kvm_vcpu *vcpu;
for (i = 0; i < len; i++) {
- cpu = val & 0xff;
+ cpuid = val & 0xff;
val = val >> 8;
if (!(s->status & BIT(EIOINTC_ENABLE_CPU_ENCODE))) {
- cpu = ffs(cpu) - 1;
- cpu = (cpu >= 4) ? 0 : cpu;
+ cpuid = ffs(cpuid) - 1;
+ cpuid = (cpuid >= 4) ? 0 : cpuid;
}
+ vcpu = kvm_get_vcpu_by_cpuid(s->kvm, cpuid);
+ if (vcpu == NULL)
+ continue;
+
+ cpu = vcpu->vcpu_id;
if (s->sw_coremap[irq + i] == cpu)
continue;
--
2.39.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v3 3/9] LoongArch: KVM: INTC: Disable update property num_cpu and feature
2025-06-11 1:46 [PATCH v3 0/9] LoongArch: KVM: INTC: Enhancement about eiointc emulation Bibo Mao
2025-06-11 1:46 ` [PATCH v3 1/9] LoongArch: KVM: INTC: Fix interrupt route update with eiointc Bibo Mao
2025-06-11 1:46 ` [PATCH v3 2/9] LoongArch: KVM: INTC: Check interrupt route from physical cpu Bibo Mao
@ 2025-06-11 1:46 ` Bibo Mao
2025-06-11 1:46 ` [PATCH v3 4/9] LoongArch: KVM: INTC: Check validation of num_cpu from user space Bibo Mao
` (6 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Bibo Mao @ 2025-06-11 1:46 UTC (permalink / raw)
To: Tianrui Zhao, Huacai Chen, Xianglai Li
Cc: kvm, loongarch, linux-kernel, stable
Property num_cpu and feature is read-only once eiointc is created, which
is set with KVM_DEV_LOONGARCH_EXTIOI_GRP_CTRL attr group before device
creation.
Attr group KVM_DEV_LOONGARCH_EXTIOI_GRP_SW_STATUS is to update register
and software state for migration and reset usage, property num_cpu and
feature can not be update again if it is created already.
Here discard write operation with property num_cpu and feature in attr
group KVM_DEV_LOONGARCH_EXTIOI_GRP_CTRL.
Cc: stable@vger.kernel.org
Fixes: 1ad7efa552fd ("LoongArch: KVM: Add EIOINTC user mode read and write functions")
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
arch/loongarch/kvm/intc/eiointc.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
index 0b648c56b0c3..b48511f903b5 100644
--- a/arch/loongarch/kvm/intc/eiointc.c
+++ b/arch/loongarch/kvm/intc/eiointc.c
@@ -910,9 +910,22 @@ static int kvm_eiointc_sw_status_access(struct kvm_device *dev,
data = (void __user *)attr->addr;
switch (addr) {
case KVM_DEV_LOONGARCH_EXTIOI_SW_STATUS_NUM_CPU:
+ /*
+ * Property num_cpu and feature is read-only once eiointc is
+ * created with KVM_DEV_LOONGARCH_EXTIOI_GRP_CTRL group API
+ *
+ * Disable writing with KVM_DEV_LOONGARCH_EXTIOI_GRP_SW_STATUS
+ * group API
+ */
+ if (is_write)
+ return ret;
+
p = &s->num_cpu;
break;
case KVM_DEV_LOONGARCH_EXTIOI_SW_STATUS_FEATURE:
+ if (is_write)
+ return ret;
+
p = &s->features;
break;
case KVM_DEV_LOONGARCH_EXTIOI_SW_STATUS_STATE:
--
2.39.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v3 4/9] LoongArch: KVM: INTC: Check validation of num_cpu from user space
2025-06-11 1:46 [PATCH v3 0/9] LoongArch: KVM: INTC: Enhancement about eiointc emulation Bibo Mao
` (2 preceding siblings ...)
2025-06-11 1:46 ` [PATCH v3 3/9] LoongArch: KVM: INTC: Disable update property num_cpu and feature Bibo Mao
@ 2025-06-11 1:46 ` Bibo Mao
2025-06-19 8:46 ` Huacai Chen
2025-06-11 1:46 ` [PATCH v3 5/9] LoongArch: KVM: INTC: Avoid overflow with array index Bibo Mao
` (5 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Bibo Mao @ 2025-06-11 1:46 UTC (permalink / raw)
To: Tianrui Zhao, Huacai Chen, Xianglai Li
Cc: kvm, loongarch, linux-kernel, stable
The maximum supported cpu number is EIOINTC_ROUTE_MAX_VCPUS about
irqchip eiointc, here add validation about cpu number to avoid array
pointer overflow.
Cc: stable@vger.kernel.org
Fixes: 1ad7efa552fd ("LoongArch: KVM: Add EIOINTC user mode read and write functions")
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
arch/loongarch/kvm/intc/eiointc.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
index b48511f903b5..ed80bf290755 100644
--- a/arch/loongarch/kvm/intc/eiointc.c
+++ b/arch/loongarch/kvm/intc/eiointc.c
@@ -798,7 +798,7 @@ static int kvm_eiointc_ctrl_access(struct kvm_device *dev,
int ret = 0;
unsigned long flags;
unsigned long type = (unsigned long)attr->attr;
- u32 i, start_irq;
+ u32 i, start_irq, val;
void __user *data;
struct loongarch_eiointc *s = dev->kvm->arch.eiointc;
@@ -806,7 +806,12 @@ static int kvm_eiointc_ctrl_access(struct kvm_device *dev,
spin_lock_irqsave(&s->lock, flags);
switch (type) {
case KVM_DEV_LOONGARCH_EXTIOI_CTRL_INIT_NUM_CPU:
- if (copy_from_user(&s->num_cpu, data, 4))
+ if (copy_from_user(&val, data, 4) == 0) {
+ if (val < EIOINTC_ROUTE_MAX_VCPUS)
+ s->num_cpu = val;
+ else
+ ret = -EINVAL;
+ } else
ret = -EFAULT;
break;
case KVM_DEV_LOONGARCH_EXTIOI_CTRL_INIT_FEATURE:
@@ -835,7 +840,7 @@ static int kvm_eiointc_regs_access(struct kvm_device *dev,
struct kvm_device_attr *attr,
bool is_write)
{
- int addr, cpuid, offset, ret = 0;
+ int addr, cpu, offset, ret = 0;
unsigned long flags;
void *p = NULL;
void __user *data;
@@ -843,7 +848,7 @@ static int kvm_eiointc_regs_access(struct kvm_device *dev,
s = dev->kvm->arch.eiointc;
addr = attr->attr;
- cpuid = addr >> 16;
+ cpu = addr >> 16;
addr &= 0xffff;
data = (void __user *)attr->addr;
switch (addr) {
@@ -868,8 +873,11 @@ static int kvm_eiointc_regs_access(struct kvm_device *dev,
p = &s->isr.reg_u32[offset];
break;
case EIOINTC_COREISR_START ... EIOINTC_COREISR_END:
+ if (cpu >= s->num_cpu)
+ return -EINVAL;
+
offset = (addr - EIOINTC_COREISR_START) / 4;
- p = &s->coreisr.reg_u32[cpuid][offset];
+ p = &s->coreisr.reg_u32[cpu][offset];
break;
case EIOINTC_COREMAP_START ... EIOINTC_COREMAP_END:
offset = (addr - EIOINTC_COREMAP_START) / 4;
--
2.39.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v3 5/9] LoongArch: KVM: INTC: Avoid overflow with array index
2025-06-11 1:46 [PATCH v3 0/9] LoongArch: KVM: INTC: Enhancement about eiointc emulation Bibo Mao
` (3 preceding siblings ...)
2025-06-11 1:46 ` [PATCH v3 4/9] LoongArch: KVM: INTC: Check validation of num_cpu from user space Bibo Mao
@ 2025-06-11 1:46 ` Bibo Mao
2025-06-11 1:46 ` [PATCH v3 6/9] LoongArch: KVM: INTC: Use standard bitops API with eiointc Bibo Mao
` (4 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Bibo Mao @ 2025-06-11 1:46 UTC (permalink / raw)
To: Tianrui Zhao, Huacai Chen, Xianglai Li
Cc: kvm, loongarch, linux-kernel, stable
Variable index is modified and reused as array index when modify
register EIOINTC_ENABLE. There will be array index overflow problem.
Cc: stable@vger.kernel.org
Fixes: 3956a52bc05b ("LoongArch: KVM: Add EIOINTC read and write functions")
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
arch/loongarch/kvm/intc/eiointc.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
index ed80bf290755..0bc870796f56 100644
--- a/arch/loongarch/kvm/intc/eiointc.c
+++ b/arch/loongarch/kvm/intc/eiointc.c
@@ -447,17 +447,16 @@ static int loongarch_eiointc_writew(struct kvm_vcpu *vcpu,
break;
case EIOINTC_ENABLE_START ... EIOINTC_ENABLE_END:
index = (offset - EIOINTC_ENABLE_START) >> 1;
- old_data = s->enable.reg_u32[index];
+ old_data = s->enable.reg_u16[index];
s->enable.reg_u16[index] = data;
/*
* 1: enable irq.
* update irq when isr is set.
*/
data = s->enable.reg_u16[index] & ~old_data & s->isr.reg_u16[index];
- index = index << 1;
for (i = 0; i < sizeof(data); i++) {
u8 mask = (data >> (i * 8)) & 0xff;
- eiointc_enable_irq(vcpu, s, index + i, mask, 1);
+ eiointc_enable_irq(vcpu, s, index * 2 + i, mask, 1);
}
/*
* 0: disable irq.
@@ -466,7 +465,7 @@ static int loongarch_eiointc_writew(struct kvm_vcpu *vcpu,
data = ~s->enable.reg_u16[index] & old_data & s->isr.reg_u16[index];
for (i = 0; i < sizeof(data); i++) {
u8 mask = (data >> (i * 8)) & 0xff;
- eiointc_enable_irq(vcpu, s, index, mask, 0);
+ eiointc_enable_irq(vcpu, s, index * 2 + i, mask, 0);
}
break;
case EIOINTC_BOUNCE_START ... EIOINTC_BOUNCE_END:
@@ -540,10 +539,9 @@ static int loongarch_eiointc_writel(struct kvm_vcpu *vcpu,
* update irq when isr is set.
*/
data = s->enable.reg_u32[index] & ~old_data & s->isr.reg_u32[index];
- index = index << 2;
for (i = 0; i < sizeof(data); i++) {
u8 mask = (data >> (i * 8)) & 0xff;
- eiointc_enable_irq(vcpu, s, index + i, mask, 1);
+ eiointc_enable_irq(vcpu, s, index * 4 + i, mask, 1);
}
/*
* 0: disable irq.
@@ -552,7 +550,7 @@ static int loongarch_eiointc_writel(struct kvm_vcpu *vcpu,
data = ~s->enable.reg_u32[index] & old_data & s->isr.reg_u32[index];
for (i = 0; i < sizeof(data); i++) {
u8 mask = (data >> (i * 8)) & 0xff;
- eiointc_enable_irq(vcpu, s, index, mask, 0);
+ eiointc_enable_irq(vcpu, s, index * 4 + i, mask, 0);
}
break;
case EIOINTC_BOUNCE_START ... EIOINTC_BOUNCE_END:
@@ -626,10 +624,9 @@ static int loongarch_eiointc_writeq(struct kvm_vcpu *vcpu,
* update irq when isr is set.
*/
data = s->enable.reg_u64[index] & ~old_data & s->isr.reg_u64[index];
- index = index << 3;
for (i = 0; i < sizeof(data); i++) {
u8 mask = (data >> (i * 8)) & 0xff;
- eiointc_enable_irq(vcpu, s, index + i, mask, 1);
+ eiointc_enable_irq(vcpu, s, index * 8 + i, mask, 1);
}
/*
* 0: disable irq.
@@ -638,7 +635,7 @@ static int loongarch_eiointc_writeq(struct kvm_vcpu *vcpu,
data = ~s->enable.reg_u64[index] & old_data & s->isr.reg_u64[index];
for (i = 0; i < sizeof(data); i++) {
u8 mask = (data >> (i * 8)) & 0xff;
- eiointc_enable_irq(vcpu, s, index, mask, 0);
+ eiointc_enable_irq(vcpu, s, index * 8 + i, mask, 0);
}
break;
case EIOINTC_BOUNCE_START ... EIOINTC_BOUNCE_END:
--
2.39.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v3 6/9] LoongArch: KVM: INTC: Use standard bitops API with eiointc
2025-06-11 1:46 [PATCH v3 0/9] LoongArch: KVM: INTC: Enhancement about eiointc emulation Bibo Mao
` (4 preceding siblings ...)
2025-06-11 1:46 ` [PATCH v3 5/9] LoongArch: KVM: INTC: Avoid overflow with array index Bibo Mao
@ 2025-06-11 1:46 ` Bibo Mao
2025-06-11 1:46 ` [PATCH v3 7/9] LoongArch: KVM: INTC: Remove unused parameter len Bibo Mao
` (3 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Bibo Mao @ 2025-06-11 1:46 UTC (permalink / raw)
To: Tianrui Zhao, Huacai Chen, Xianglai Li; +Cc: kvm, loongarch, linux-kernel
Standard bitops APIs such test_bit() is used here, rather than manually
calculate the offset and mask. Also use non-atomic API __set_bit()
and __clear_bit() rather than set_bit() and clear_bit(), since global
spinlock is held already.
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
arch/loongarch/kvm/intc/eiointc.c | 27 +++++++++++----------------
1 file changed, 11 insertions(+), 16 deletions(-)
diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
index 0bc870796f56..0a3c5cd0993a 100644
--- a/arch/loongarch/kvm/intc/eiointc.c
+++ b/arch/loongarch/kvm/intc/eiointc.c
@@ -9,7 +9,7 @@
static void eiointc_set_sw_coreisr(struct loongarch_eiointc *s)
{
- int ipnum, cpu, irq_index, irq_mask, irq, cpuid;
+ int ipnum, cpu, irq, cpuid;
struct kvm_vcpu *vcpu;
for (irq = 0; irq < EIOINTC_IRQS; irq++) {
@@ -18,8 +18,6 @@ static void eiointc_set_sw_coreisr(struct loongarch_eiointc *s)
ipnum = count_trailing_zeros(ipnum);
ipnum = (ipnum >= 0 && ipnum < 4) ? ipnum : 0;
}
- irq_index = irq / 32;
- irq_mask = BIT(irq & 0x1f);
cpuid = s->coremap.reg_u8[irq];
vcpu = kvm_get_vcpu_by_cpuid(s->kvm, cpuid);
@@ -27,16 +25,16 @@ static void eiointc_set_sw_coreisr(struct loongarch_eiointc *s)
continue;
cpu = vcpu->vcpu_id;
- if (!!(s->coreisr.reg_u32[cpu][irq_index] & irq_mask))
- set_bit(irq, s->sw_coreisr[cpu][ipnum]);
+ if (test_bit(irq, (unsigned long *)s->coreisr.reg_u32[cpu]))
+ __set_bit(irq, s->sw_coreisr[cpu][ipnum]);
else
- clear_bit(irq, s->sw_coreisr[cpu][ipnum]);
+ __clear_bit(irq, s->sw_coreisr[cpu][ipnum]);
}
}
static void eiointc_update_irq(struct loongarch_eiointc *s, int irq, int level)
{
- int ipnum, cpu, found, irq_index, irq_mask;
+ int ipnum, cpu, found;
struct kvm_vcpu *vcpu;
struct kvm_interrupt vcpu_irq;
@@ -48,19 +46,16 @@ 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);
- irq_index = irq / 32;
- irq_mask = BIT(irq & 0x1f);
-
if (level) {
/* if not enable return false */
- if (((s->enable.reg_u32[irq_index]) & irq_mask) == 0)
+ if (!test_bit(irq, (unsigned long *)s->enable.reg_u32))
return;
- s->coreisr.reg_u32[cpu][irq_index] |= irq_mask;
+ __set_bit(irq, (unsigned long *)s->coreisr.reg_u32[cpu]);
found = find_first_bit(s->sw_coreisr[cpu][ipnum], EIOINTC_IRQS);
- set_bit(irq, s->sw_coreisr[cpu][ipnum]);
+ __set_bit(irq, s->sw_coreisr[cpu][ipnum]);
} else {
- s->coreisr.reg_u32[cpu][irq_index] &= ~irq_mask;
- clear_bit(irq, s->sw_coreisr[cpu][ipnum]);
+ __clear_bit(irq, (unsigned long *)s->coreisr.reg_u32[cpu]);
+ __clear_bit(irq, s->sw_coreisr[cpu][ipnum]);
found = find_first_bit(s->sw_coreisr[cpu][ipnum], EIOINTC_IRQS);
}
@@ -110,8 +105,8 @@ void eiointc_set_irq(struct loongarch_eiointc *s, int irq, int level)
unsigned long flags;
unsigned long *isr = (unsigned long *)s->isr.reg_u8;
- level ? set_bit(irq, isr) : clear_bit(irq, isr);
spin_lock_irqsave(&s->lock, flags);
+ level ? __set_bit(irq, isr) : __clear_bit(irq, isr);
eiointc_update_irq(s, irq, level);
spin_unlock_irqrestore(&s->lock, flags);
}
--
2.39.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v3 7/9] LoongArch: KVM: INTC: Remove unused parameter len
2025-06-11 1:46 [PATCH v3 0/9] LoongArch: KVM: INTC: Enhancement about eiointc emulation Bibo Mao
` (5 preceding siblings ...)
2025-06-11 1:46 ` [PATCH v3 6/9] LoongArch: KVM: INTC: Use standard bitops API with eiointc Bibo Mao
@ 2025-06-11 1:46 ` Bibo Mao
2025-06-11 1:46 ` [PATCH v3 8/9] LoongArch: KVM: INTC: Add stat information with kernel irqchip Bibo Mao
` (2 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Bibo Mao @ 2025-06-11 1:46 UTC (permalink / raw)
To: Tianrui Zhao, Huacai Chen, Xianglai Li; +Cc: kvm, loongarch, linux-kernel
Parameter len is unused in some functions with eiointc emulation
driver, remove it here.
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
arch/loongarch/kvm/intc/eiointc.c | 32 +++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
index 0a3c5cd0993a..1dad8342d84e 100644
--- a/arch/loongarch/kvm/intc/eiointc.c
+++ b/arch/loongarch/kvm/intc/eiointc.c
@@ -131,7 +131,7 @@ static inline void eiointc_enable_irq(struct kvm_vcpu *vcpu,
}
static int loongarch_eiointc_readb(struct kvm_vcpu *vcpu, struct loongarch_eiointc *s,
- gpa_t addr, int len, void *val)
+ gpa_t addr, void *val)
{
int index, ret = 0;
u8 data = 0;
@@ -173,7 +173,7 @@ static int loongarch_eiointc_readb(struct kvm_vcpu *vcpu, struct loongarch_eioin
}
static int loongarch_eiointc_readw(struct kvm_vcpu *vcpu, struct loongarch_eiointc *s,
- gpa_t addr, int len, void *val)
+ gpa_t addr, void *val)
{
int index, ret = 0;
u16 data = 0;
@@ -215,7 +215,7 @@ static int loongarch_eiointc_readw(struct kvm_vcpu *vcpu, struct loongarch_eioin
}
static int loongarch_eiointc_readl(struct kvm_vcpu *vcpu, struct loongarch_eiointc *s,
- gpa_t addr, int len, void *val)
+ gpa_t addr, void *val)
{
int index, ret = 0;
u32 data = 0;
@@ -257,7 +257,7 @@ static int loongarch_eiointc_readl(struct kvm_vcpu *vcpu, struct loongarch_eioin
}
static int loongarch_eiointc_readq(struct kvm_vcpu *vcpu, struct loongarch_eiointc *s,
- gpa_t addr, int len, void *val)
+ gpa_t addr, void *val)
{
int index, ret = 0;
u64 data = 0;
@@ -315,16 +315,16 @@ static int kvm_eiointc_read(struct kvm_vcpu *vcpu,
spin_lock_irqsave(&eiointc->lock, flags);
switch (len) {
case 1:
- ret = loongarch_eiointc_readb(vcpu, eiointc, addr, len, val);
+ ret = loongarch_eiointc_readb(vcpu, eiointc, addr, val);
break;
case 2:
- ret = loongarch_eiointc_readw(vcpu, eiointc, addr, len, val);
+ ret = loongarch_eiointc_readw(vcpu, eiointc, addr, val);
break;
case 4:
- ret = loongarch_eiointc_readl(vcpu, eiointc, addr, len, val);
+ ret = loongarch_eiointc_readl(vcpu, eiointc, addr, val);
break;
case 8:
- ret = loongarch_eiointc_readq(vcpu, eiointc, addr, len, val);
+ ret = loongarch_eiointc_readq(vcpu, eiointc, addr, val);
break;
default:
WARN_ONCE(1, "%s: Abnormal address access: addr 0x%llx, size %d\n",
@@ -337,7 +337,7 @@ static int kvm_eiointc_read(struct kvm_vcpu *vcpu,
static int loongarch_eiointc_writeb(struct kvm_vcpu *vcpu,
struct loongarch_eiointc *s,
- gpa_t addr, int len, const void *val)
+ gpa_t addr, const void *val)
{
int index, irq, bits, ret = 0;
u8 cpu;
@@ -416,7 +416,7 @@ static int loongarch_eiointc_writeb(struct kvm_vcpu *vcpu,
static int loongarch_eiointc_writew(struct kvm_vcpu *vcpu,
struct loongarch_eiointc *s,
- gpa_t addr, int len, const void *val)
+ gpa_t addr, const void *val)
{
int i, index, irq, bits, ret = 0;
u8 cpu;
@@ -501,7 +501,7 @@ static int loongarch_eiointc_writew(struct kvm_vcpu *vcpu,
static int loongarch_eiointc_writel(struct kvm_vcpu *vcpu,
struct loongarch_eiointc *s,
- gpa_t addr, int len, const void *val)
+ gpa_t addr, const void *val)
{
int i, index, irq, bits, ret = 0;
u8 cpu;
@@ -586,7 +586,7 @@ static int loongarch_eiointc_writel(struct kvm_vcpu *vcpu,
static int loongarch_eiointc_writeq(struct kvm_vcpu *vcpu,
struct loongarch_eiointc *s,
- gpa_t addr, int len, const void *val)
+ gpa_t addr, const void *val)
{
int i, index, irq, bits, ret = 0;
u8 cpu;
@@ -686,16 +686,16 @@ static int kvm_eiointc_write(struct kvm_vcpu *vcpu,
spin_lock_irqsave(&eiointc->lock, flags);
switch (len) {
case 1:
- ret = loongarch_eiointc_writeb(vcpu, eiointc, addr, len, val);
+ ret = loongarch_eiointc_writeb(vcpu, eiointc, addr, val);
break;
case 2:
- ret = loongarch_eiointc_writew(vcpu, eiointc, addr, len, val);
+ ret = loongarch_eiointc_writew(vcpu, eiointc, addr, val);
break;
case 4:
- ret = loongarch_eiointc_writel(vcpu, eiointc, addr, len, val);
+ ret = loongarch_eiointc_writel(vcpu, eiointc, addr, val);
break;
case 8:
- ret = loongarch_eiointc_writeq(vcpu, eiointc, addr, len, val);
+ ret = loongarch_eiointc_writeq(vcpu, eiointc, addr, val);
break;
default:
WARN_ONCE(1, "%s: Abnormal address access: addr 0x%llx, size %d\n",
--
2.39.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v3 8/9] LoongArch: KVM: INTC: Add stat information with kernel irqchip
2025-06-11 1:46 [PATCH v3 0/9] LoongArch: KVM: INTC: Enhancement about eiointc emulation Bibo Mao
` (6 preceding siblings ...)
2025-06-11 1:46 ` [PATCH v3 7/9] LoongArch: KVM: INTC: Remove unused parameter len Bibo Mao
@ 2025-06-11 1:46 ` Bibo Mao
2025-06-11 1:50 ` Bibo Mao
2025-06-11 1:51 ` [PATCH v3 9/9] LoongArch: KVM: INTC: Add address alignment check Bibo Mao
9 siblings, 0 replies; 22+ messages in thread
From: Bibo Mao @ 2025-06-11 1:46 UTC (permalink / raw)
To: Tianrui Zhao, Huacai Chen, Xianglai Li; +Cc: kvm, loongarch, linux-kernel
Move stat information about kernel irqchip from VM to vCPU, since
all vm exiting events should be vCPU relative. And also add entry
with structure kvm_vcpu_stats_desc[], so that it can display with
directory /sys/kernel/debug/kvm.
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
arch/loongarch/include/asm/kvm_host.h | 12 ++++++------
arch/loongarch/kvm/intc/eiointc.c | 4 ++--
arch/loongarch/kvm/intc/ipi.c | 28 ++++-----------------------
arch/loongarch/kvm/intc/pch_pic.c | 4 ++--
arch/loongarch/kvm/vcpu.c | 8 +++++++-
5 files changed, 21 insertions(+), 35 deletions(-)
diff --git a/arch/loongarch/include/asm/kvm_host.h b/arch/loongarch/include/asm/kvm_host.h
index a3c4cc46c892..0cecbd038bb3 100644
--- a/arch/loongarch/include/asm/kvm_host.h
+++ b/arch/loongarch/include/asm/kvm_host.h
@@ -50,12 +50,6 @@ struct kvm_vm_stat {
struct kvm_vm_stat_generic generic;
u64 pages;
u64 hugepages;
- u64 ipi_read_exits;
- u64 ipi_write_exits;
- u64 eiointc_read_exits;
- u64 eiointc_write_exits;
- u64 pch_pic_read_exits;
- u64 pch_pic_write_exits;
};
struct kvm_vcpu_stat {
@@ -65,6 +59,12 @@ struct kvm_vcpu_stat {
u64 cpucfg_exits;
u64 signal_exits;
u64 hypercall_exits;
+ u64 ipi_read_exits;
+ u64 ipi_write_exits;
+ u64 eiointc_read_exits;
+ u64 eiointc_write_exits;
+ u64 pch_pic_read_exits;
+ u64 pch_pic_write_exits;
};
#define KVM_MEM_HUGEPAGE_CAPABLE (1UL << 0)
diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
index 1dad8342d84e..8b0d9376eb54 100644
--- a/arch/loongarch/kvm/intc/eiointc.c
+++ b/arch/loongarch/kvm/intc/eiointc.c
@@ -311,7 +311,7 @@ static int kvm_eiointc_read(struct kvm_vcpu *vcpu,
return -EINVAL;
}
- vcpu->kvm->stat.eiointc_read_exits++;
+ vcpu->stat.eiointc_read_exits++;
spin_lock_irqsave(&eiointc->lock, flags);
switch (len) {
case 1:
@@ -682,7 +682,7 @@ static int kvm_eiointc_write(struct kvm_vcpu *vcpu,
return -EINVAL;
}
- vcpu->kvm->stat.eiointc_write_exits++;
+ vcpu->stat.eiointc_write_exits++;
spin_lock_irqsave(&eiointc->lock, flags);
switch (len) {
case 1:
diff --git a/arch/loongarch/kvm/intc/ipi.c b/arch/loongarch/kvm/intc/ipi.c
index fe734dc062ed..e658d5b37c04 100644
--- a/arch/loongarch/kvm/intc/ipi.c
+++ b/arch/loongarch/kvm/intc/ipi.c
@@ -268,36 +268,16 @@ static int kvm_ipi_read(struct kvm_vcpu *vcpu,
struct kvm_io_device *dev,
gpa_t addr, int len, void *val)
{
- int ret;
- struct loongarch_ipi *ipi;
-
- ipi = vcpu->kvm->arch.ipi;
- if (!ipi) {
- kvm_err("%s: ipi irqchip not valid!\n", __func__);
- return -EINVAL;
- }
- ipi->kvm->stat.ipi_read_exits++;
- ret = loongarch_ipi_readl(vcpu, addr, len, val);
-
- return ret;
+ vcpu->stat.ipi_read_exits++;
+ return loongarch_ipi_readl(vcpu, addr, len, val);
}
static int kvm_ipi_write(struct kvm_vcpu *vcpu,
struct kvm_io_device *dev,
gpa_t addr, int len, const void *val)
{
- int ret;
- struct loongarch_ipi *ipi;
-
- ipi = vcpu->kvm->arch.ipi;
- if (!ipi) {
- kvm_err("%s: ipi irqchip not valid!\n", __func__);
- return -EINVAL;
- }
- ipi->kvm->stat.ipi_write_exits++;
- ret = loongarch_ipi_writel(vcpu, addr, len, val);
-
- return ret;
+ vcpu->stat.ipi_write_exits++;
+ return loongarch_ipi_writel(vcpu, addr, len, val);
}
static const struct kvm_io_device_ops kvm_ipi_ops = {
diff --git a/arch/loongarch/kvm/intc/pch_pic.c b/arch/loongarch/kvm/intc/pch_pic.c
index 08fce845f668..6f00ffe05c54 100644
--- a/arch/loongarch/kvm/intc/pch_pic.c
+++ b/arch/loongarch/kvm/intc/pch_pic.c
@@ -196,7 +196,7 @@ static int kvm_pch_pic_read(struct kvm_vcpu *vcpu,
}
/* statistics of pch pic reading */
- vcpu->kvm->stat.pch_pic_read_exits++;
+ vcpu->stat.pch_pic_read_exits++;
ret = loongarch_pch_pic_read(s, addr, len, val);
return ret;
@@ -303,7 +303,7 @@ static int kvm_pch_pic_write(struct kvm_vcpu *vcpu,
}
/* statistics of pch pic writing */
- vcpu->kvm->stat.pch_pic_write_exits++;
+ vcpu->stat.pch_pic_write_exits++;
ret = loongarch_pch_pic_write(s, addr, len, val);
return ret;
diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
index 5af32ec62cb1..d1b8c50941ca 100644
--- a/arch/loongarch/kvm/vcpu.c
+++ b/arch/loongarch/kvm/vcpu.c
@@ -20,7 +20,13 @@ const struct _kvm_stats_desc kvm_vcpu_stats_desc[] = {
STATS_DESC_COUNTER(VCPU, idle_exits),
STATS_DESC_COUNTER(VCPU, cpucfg_exits),
STATS_DESC_COUNTER(VCPU, signal_exits),
- STATS_DESC_COUNTER(VCPU, hypercall_exits)
+ STATS_DESC_COUNTER(VCPU, hypercall_exits),
+ STATS_DESC_COUNTER(VCPU, ipi_read_exits),
+ STATS_DESC_COUNTER(VCPU, ipi_write_exits),
+ STATS_DESC_COUNTER(VCPU, eiointc_read_exits),
+ STATS_DESC_COUNTER(VCPU, eiointc_write_exits),
+ STATS_DESC_COUNTER(VCPU, pch_pic_read_exits),
+ STATS_DESC_COUNTER(VCPU, pch_pic_write_exits)
};
const struct kvm_stats_header kvm_vcpu_stats_header = {
--
2.39.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v3 8/9] LoongArch: KVM: INTC: Add stat information with kernel irqchip
2025-06-11 1:46 [PATCH v3 0/9] LoongArch: KVM: INTC: Enhancement about eiointc emulation Bibo Mao
` (7 preceding siblings ...)
2025-06-11 1:46 ` [PATCH v3 8/9] LoongArch: KVM: INTC: Add stat information with kernel irqchip Bibo Mao
@ 2025-06-11 1:50 ` Bibo Mao
2025-06-11 1:51 ` [PATCH v3 9/9] LoongArch: KVM: INTC: Add address alignment check Bibo Mao
9 siblings, 0 replies; 22+ messages in thread
From: Bibo Mao @ 2025-06-11 1:50 UTC (permalink / raw)
To: Tianrui Zhao, Huacai Chen, Xianglai Li; +Cc: kvm, loongarch, linux-kernel
Move stat information about kernel irqchip from VM to vCPU, since
all vm exiting events should be vCPU relative. And also add entry
with structure kvm_vcpu_stats_desc[], so that it can display with
directory /sys/kernel/debug/kvm.
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
arch/loongarch/include/asm/kvm_host.h | 12 ++++++------
arch/loongarch/kvm/intc/eiointc.c | 4 ++--
arch/loongarch/kvm/intc/ipi.c | 28 ++++-----------------------
arch/loongarch/kvm/intc/pch_pic.c | 4 ++--
arch/loongarch/kvm/vcpu.c | 8 +++++++-
5 files changed, 21 insertions(+), 35 deletions(-)
diff --git a/arch/loongarch/include/asm/kvm_host.h b/arch/loongarch/include/asm/kvm_host.h
index a3c4cc46c892..0cecbd038bb3 100644
--- a/arch/loongarch/include/asm/kvm_host.h
+++ b/arch/loongarch/include/asm/kvm_host.h
@@ -50,12 +50,6 @@ struct kvm_vm_stat {
struct kvm_vm_stat_generic generic;
u64 pages;
u64 hugepages;
- u64 ipi_read_exits;
- u64 ipi_write_exits;
- u64 eiointc_read_exits;
- u64 eiointc_write_exits;
- u64 pch_pic_read_exits;
- u64 pch_pic_write_exits;
};
struct kvm_vcpu_stat {
@@ -65,6 +59,12 @@ struct kvm_vcpu_stat {
u64 cpucfg_exits;
u64 signal_exits;
u64 hypercall_exits;
+ u64 ipi_read_exits;
+ u64 ipi_write_exits;
+ u64 eiointc_read_exits;
+ u64 eiointc_write_exits;
+ u64 pch_pic_read_exits;
+ u64 pch_pic_write_exits;
};
#define KVM_MEM_HUGEPAGE_CAPABLE (1UL << 0)
diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
index 1dad8342d84e..8b0d9376eb54 100644
--- a/arch/loongarch/kvm/intc/eiointc.c
+++ b/arch/loongarch/kvm/intc/eiointc.c
@@ -311,7 +311,7 @@ static int kvm_eiointc_read(struct kvm_vcpu *vcpu,
return -EINVAL;
}
- vcpu->kvm->stat.eiointc_read_exits++;
+ vcpu->stat.eiointc_read_exits++;
spin_lock_irqsave(&eiointc->lock, flags);
switch (len) {
case 1:
@@ -682,7 +682,7 @@ static int kvm_eiointc_write(struct kvm_vcpu *vcpu,
return -EINVAL;
}
- vcpu->kvm->stat.eiointc_write_exits++;
+ vcpu->stat.eiointc_write_exits++;
spin_lock_irqsave(&eiointc->lock, flags);
switch (len) {
case 1:
diff --git a/arch/loongarch/kvm/intc/ipi.c b/arch/loongarch/kvm/intc/ipi.c
index fe734dc062ed..e658d5b37c04 100644
--- a/arch/loongarch/kvm/intc/ipi.c
+++ b/arch/loongarch/kvm/intc/ipi.c
@@ -268,36 +268,16 @@ static int kvm_ipi_read(struct kvm_vcpu *vcpu,
struct kvm_io_device *dev,
gpa_t addr, int len, void *val)
{
- int ret;
- struct loongarch_ipi *ipi;
-
- ipi = vcpu->kvm->arch.ipi;
- if (!ipi) {
- kvm_err("%s: ipi irqchip not valid!\n", __func__);
- return -EINVAL;
- }
- ipi->kvm->stat.ipi_read_exits++;
- ret = loongarch_ipi_readl(vcpu, addr, len, val);
-
- return ret;
+ vcpu->stat.ipi_read_exits++;
+ return loongarch_ipi_readl(vcpu, addr, len, val);
}
static int kvm_ipi_write(struct kvm_vcpu *vcpu,
struct kvm_io_device *dev,
gpa_t addr, int len, const void *val)
{
- int ret;
- struct loongarch_ipi *ipi;
-
- ipi = vcpu->kvm->arch.ipi;
- if (!ipi) {
- kvm_err("%s: ipi irqchip not valid!\n", __func__);
- return -EINVAL;
- }
- ipi->kvm->stat.ipi_write_exits++;
- ret = loongarch_ipi_writel(vcpu, addr, len, val);
-
- return ret;
+ vcpu->stat.ipi_write_exits++;
+ return loongarch_ipi_writel(vcpu, addr, len, val);
}
static const struct kvm_io_device_ops kvm_ipi_ops = {
diff --git a/arch/loongarch/kvm/intc/pch_pic.c b/arch/loongarch/kvm/intc/pch_pic.c
index 08fce845f668..6f00ffe05c54 100644
--- a/arch/loongarch/kvm/intc/pch_pic.c
+++ b/arch/loongarch/kvm/intc/pch_pic.c
@@ -196,7 +196,7 @@ static int kvm_pch_pic_read(struct kvm_vcpu *vcpu,
}
/* statistics of pch pic reading */
- vcpu->kvm->stat.pch_pic_read_exits++;
+ vcpu->stat.pch_pic_read_exits++;
ret = loongarch_pch_pic_read(s, addr, len, val);
return ret;
@@ -303,7 +303,7 @@ static int kvm_pch_pic_write(struct kvm_vcpu *vcpu,
}
/* statistics of pch pic writing */
- vcpu->kvm->stat.pch_pic_write_exits++;
+ vcpu->stat.pch_pic_write_exits++;
ret = loongarch_pch_pic_write(s, addr, len, val);
return ret;
diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
index 5af32ec62cb1..d1b8c50941ca 100644
--- a/arch/loongarch/kvm/vcpu.c
+++ b/arch/loongarch/kvm/vcpu.c
@@ -20,7 +20,13 @@ const struct _kvm_stats_desc kvm_vcpu_stats_desc[] = {
STATS_DESC_COUNTER(VCPU, idle_exits),
STATS_DESC_COUNTER(VCPU, cpucfg_exits),
STATS_DESC_COUNTER(VCPU, signal_exits),
- STATS_DESC_COUNTER(VCPU, hypercall_exits)
+ STATS_DESC_COUNTER(VCPU, hypercall_exits),
+ STATS_DESC_COUNTER(VCPU, ipi_read_exits),
+ STATS_DESC_COUNTER(VCPU, ipi_write_exits),
+ STATS_DESC_COUNTER(VCPU, eiointc_read_exits),
+ STATS_DESC_COUNTER(VCPU, eiointc_write_exits),
+ STATS_DESC_COUNTER(VCPU, pch_pic_read_exits),
+ STATS_DESC_COUNTER(VCPU, pch_pic_write_exits)
};
const struct kvm_stats_header kvm_vcpu_stats_header = {
--
2.39.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v3 9/9] LoongArch: KVM: INTC: Add address alignment check
2025-06-11 1:46 [PATCH v3 0/9] LoongArch: KVM: INTC: Enhancement about eiointc emulation Bibo Mao
` (8 preceding siblings ...)
2025-06-11 1:50 ` Bibo Mao
@ 2025-06-11 1:51 ` Bibo Mao
2025-06-19 8:47 ` Huacai Chen
9 siblings, 1 reply; 22+ messages in thread
From: Bibo Mao @ 2025-06-11 1:51 UTC (permalink / raw)
To: Tianrui Zhao, Huacai Chen, Xianglai Li; +Cc: kvm, loongarch, linux-kernel
IOCSR instruction supports 1/2/4/8 bytes access, the address should
be naturally aligned with its access size. Here address alignment
check is added in eiointc kernel emulation.
At the same time len must be 1/2/4/8 bytes from iocsr exit emulation
function kvm_emu_iocsr(), remove the default case in switch case
statements.
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
arch/loongarch/kvm/intc/eiointc.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
index 8b0d9376eb54..4e9d12300cc4 100644
--- a/arch/loongarch/kvm/intc/eiointc.c
+++ b/arch/loongarch/kvm/intc/eiointc.c
@@ -311,6 +311,12 @@ static int kvm_eiointc_read(struct kvm_vcpu *vcpu,
return -EINVAL;
}
+ /* len must be 1/2/4/8 from function kvm_emu_iocsr() */
+ if (addr & (len - 1)) {
+ kvm_err("%s: eiointc not aligned addr %llx len %d\n", __func__, addr, len);
+ return -EINVAL;
+ }
+
vcpu->stat.eiointc_read_exits++;
spin_lock_irqsave(&eiointc->lock, flags);
switch (len) {
@@ -323,12 +329,9 @@ static int kvm_eiointc_read(struct kvm_vcpu *vcpu,
case 4:
ret = loongarch_eiointc_readl(vcpu, eiointc, addr, val);
break;
- case 8:
+ default:
ret = loongarch_eiointc_readq(vcpu, eiointc, addr, val);
break;
- default:
- WARN_ONCE(1, "%s: Abnormal address access: addr 0x%llx, size %d\n",
- __func__, addr, len);
}
spin_unlock_irqrestore(&eiointc->lock, flags);
@@ -682,6 +685,11 @@ static int kvm_eiointc_write(struct kvm_vcpu *vcpu,
return -EINVAL;
}
+ if (addr & (len - 1)) {
+ kvm_err("%s: eiointc not aligned addr %llx len %d\n", __func__, addr, len);
+ return -EINVAL;
+ }
+
vcpu->stat.eiointc_write_exits++;
spin_lock_irqsave(&eiointc->lock, flags);
switch (len) {
@@ -694,12 +702,9 @@ static int kvm_eiointc_write(struct kvm_vcpu *vcpu,
case 4:
ret = loongarch_eiointc_writel(vcpu, eiointc, addr, val);
break;
- case 8:
+ default:
ret = loongarch_eiointc_writeq(vcpu, eiointc, addr, val);
break;
- default:
- WARN_ONCE(1, "%s: Abnormal address access: addr 0x%llx, size %d\n",
- __func__, addr, len);
}
spin_unlock_irqrestore(&eiointc->lock, flags);
--
2.39.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v3 4/9] LoongArch: KVM: INTC: Check validation of num_cpu from user space
2025-06-11 1:46 ` [PATCH v3 4/9] LoongArch: KVM: INTC: Check validation of num_cpu from user space Bibo Mao
@ 2025-06-19 8:46 ` Huacai Chen
2025-06-20 1:42 ` Bibo Mao
0 siblings, 1 reply; 22+ messages in thread
From: Huacai Chen @ 2025-06-19 8:46 UTC (permalink / raw)
To: Bibo Mao; +Cc: Tianrui Zhao, Xianglai Li, kvm, loongarch, linux-kernel, stable
Hi, Bibo,
On Wed, Jun 11, 2025 at 9:47 AM Bibo Mao <maobibo@loongson.cn> wrote:
>
> The maximum supported cpu number is EIOINTC_ROUTE_MAX_VCPUS about
> irqchip eiointc, here add validation about cpu number to avoid array
> pointer overflow.
>
> Cc: stable@vger.kernel.org
> Fixes: 1ad7efa552fd ("LoongArch: KVM: Add EIOINTC user mode read and write functions")
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
> arch/loongarch/kvm/intc/eiointc.c | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
> index b48511f903b5..ed80bf290755 100644
> --- a/arch/loongarch/kvm/intc/eiointc.c
> +++ b/arch/loongarch/kvm/intc/eiointc.c
> @@ -798,7 +798,7 @@ static int kvm_eiointc_ctrl_access(struct kvm_device *dev,
> int ret = 0;
> unsigned long flags;
> unsigned long type = (unsigned long)attr->attr;
> - u32 i, start_irq;
> + u32 i, start_irq, val;
> void __user *data;
> struct loongarch_eiointc *s = dev->kvm->arch.eiointc;
>
> @@ -806,7 +806,12 @@ static int kvm_eiointc_ctrl_access(struct kvm_device *dev,
> spin_lock_irqsave(&s->lock, flags);
> switch (type) {
> case KVM_DEV_LOONGARCH_EXTIOI_CTRL_INIT_NUM_CPU:
> - if (copy_from_user(&s->num_cpu, data, 4))
> + if (copy_from_user(&val, data, 4) == 0) {
> + if (val < EIOINTC_ROUTE_MAX_VCPUS)
> + s->num_cpu = val;
> + else
> + ret = -EINVAL;
Maybe it is better to set s->num_cpu to EIOINTC_ROUTE_MAX_VCPUS (or
other value) rather than keep it uninitialized. Because in other
places we need to check s->num_cpu and an uninitialized value may
cause undefined behavior.
Huacai
> + } else
> ret = -EFAULT;
> break;
> case KVM_DEV_LOONGARCH_EXTIOI_CTRL_INIT_FEATURE:
> @@ -835,7 +840,7 @@ static int kvm_eiointc_regs_access(struct kvm_device *dev,
> struct kvm_device_attr *attr,
> bool is_write)
> {
> - int addr, cpuid, offset, ret = 0;
> + int addr, cpu, offset, ret = 0;
> unsigned long flags;
> void *p = NULL;
> void __user *data;
> @@ -843,7 +848,7 @@ static int kvm_eiointc_regs_access(struct kvm_device *dev,
>
> s = dev->kvm->arch.eiointc;
> addr = attr->attr;
> - cpuid = addr >> 16;
> + cpu = addr >> 16;
> addr &= 0xffff;
> data = (void __user *)attr->addr;
> switch (addr) {
> @@ -868,8 +873,11 @@ static int kvm_eiointc_regs_access(struct kvm_device *dev,
> p = &s->isr.reg_u32[offset];
> break;
> case EIOINTC_COREISR_START ... EIOINTC_COREISR_END:
> + if (cpu >= s->num_cpu)
> + return -EINVAL;
> +
> offset = (addr - EIOINTC_COREISR_START) / 4;
> - p = &s->coreisr.reg_u32[cpuid][offset];
> + p = &s->coreisr.reg_u32[cpu][offset];
> break;
> case EIOINTC_COREMAP_START ... EIOINTC_COREMAP_END:
> offset = (addr - EIOINTC_COREMAP_START) / 4;
> --
> 2.39.3
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 9/9] LoongArch: KVM: INTC: Add address alignment check
2025-06-11 1:51 ` [PATCH v3 9/9] LoongArch: KVM: INTC: Add address alignment check Bibo Mao
@ 2025-06-19 8:47 ` Huacai Chen
2025-06-19 9:47 ` Bibo Mao
2025-06-21 11:20 ` David Laight
0 siblings, 2 replies; 22+ messages in thread
From: Huacai Chen @ 2025-06-19 8:47 UTC (permalink / raw)
To: Bibo Mao; +Cc: Tianrui Zhao, Xianglai Li, kvm, loongarch, linux-kernel
Hi, Bibo,
On Wed, Jun 11, 2025 at 9:51 AM Bibo Mao <maobibo@loongson.cn> wrote:
>
> IOCSR instruction supports 1/2/4/8 bytes access, the address should
> be naturally aligned with its access size. Here address alignment
> check is added in eiointc kernel emulation.
>
> At the same time len must be 1/2/4/8 bytes from iocsr exit emulation
> function kvm_emu_iocsr(), remove the default case in switch case
> statements.
Robust code doesn't depend its callers do things right, so I suggest
keeping the default case, which means we just add the alignment check
here.
And I think this patch should also Cc stable and add a Fixes tag.
Huacai
>
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
> arch/loongarch/kvm/intc/eiointc.c | 21 +++++++++++++--------
> 1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
> index 8b0d9376eb54..4e9d12300cc4 100644
> --- a/arch/loongarch/kvm/intc/eiointc.c
> +++ b/arch/loongarch/kvm/intc/eiointc.c
> @@ -311,6 +311,12 @@ static int kvm_eiointc_read(struct kvm_vcpu *vcpu,
> return -EINVAL;
> }
>
> + /* len must be 1/2/4/8 from function kvm_emu_iocsr() */
> + if (addr & (len - 1)) {
> + kvm_err("%s: eiointc not aligned addr %llx len %d\n", __func__, addr, len);
> + return -EINVAL;
> + }
> +
> vcpu->stat.eiointc_read_exits++;
> spin_lock_irqsave(&eiointc->lock, flags);
> switch (len) {
> @@ -323,12 +329,9 @@ static int kvm_eiointc_read(struct kvm_vcpu *vcpu,
> case 4:
> ret = loongarch_eiointc_readl(vcpu, eiointc, addr, val);
> break;
> - case 8:
> + default:
> ret = loongarch_eiointc_readq(vcpu, eiointc, addr, val);
> break;
> - default:
> - WARN_ONCE(1, "%s: Abnormal address access: addr 0x%llx, size %d\n",
> - __func__, addr, len);
> }
> spin_unlock_irqrestore(&eiointc->lock, flags);
>
> @@ -682,6 +685,11 @@ static int kvm_eiointc_write(struct kvm_vcpu *vcpu,
> return -EINVAL;
> }
>
> + if (addr & (len - 1)) {
> + kvm_err("%s: eiointc not aligned addr %llx len %d\n", __func__, addr, len);
> + return -EINVAL;
> + }
> +
> vcpu->stat.eiointc_write_exits++;
> spin_lock_irqsave(&eiointc->lock, flags);
> switch (len) {
> @@ -694,12 +702,9 @@ static int kvm_eiointc_write(struct kvm_vcpu *vcpu,
> case 4:
> ret = loongarch_eiointc_writel(vcpu, eiointc, addr, val);
> break;
> - case 8:
> + default:
> ret = loongarch_eiointc_writeq(vcpu, eiointc, addr, val);
> break;
> - default:
> - WARN_ONCE(1, "%s: Abnormal address access: addr 0x%llx, size %d\n",
> - __func__, addr, len);
> }
> spin_unlock_irqrestore(&eiointc->lock, flags);
>
> --
> 2.39.3
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 9/9] LoongArch: KVM: INTC: Add address alignment check
2025-06-19 8:47 ` Huacai Chen
@ 2025-06-19 9:47 ` Bibo Mao
2025-06-21 11:20 ` David Laight
1 sibling, 0 replies; 22+ messages in thread
From: Bibo Mao @ 2025-06-19 9:47 UTC (permalink / raw)
To: Huacai Chen; +Cc: Tianrui Zhao, Xianglai Li, kvm, loongarch, linux-kernel
On 2025/6/19 下午4:47, Huacai Chen wrote:
> Hi, Bibo,
>
> On Wed, Jun 11, 2025 at 9:51 AM Bibo Mao <maobibo@loongson.cn> wrote:
>>
>> IOCSR instruction supports 1/2/4/8 bytes access, the address should
>> be naturally aligned with its access size. Here address alignment
>> check is added in eiointc kernel emulation.
>>
>> At the same time len must be 1/2/4/8 bytes from iocsr exit emulation
>> function kvm_emu_iocsr(), remove the default case in switch case
>> statements.
> Robust code doesn't depend its callers do things right, so I suggest
> keeping the default case, which means we just add the alignment check
> here.
ok, will keep the default case.
>
> And I think this patch should also Cc stable and add a Fixes tag.
ok, will add Cc stabe and Fixes tag.
Regards
Bibo Mao
>
>
> Huacai
>
>>
>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>> ---
>> arch/loongarch/kvm/intc/eiointc.c | 21 +++++++++++++--------
>> 1 file changed, 13 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
>> index 8b0d9376eb54..4e9d12300cc4 100644
>> --- a/arch/loongarch/kvm/intc/eiointc.c
>> +++ b/arch/loongarch/kvm/intc/eiointc.c
>> @@ -311,6 +311,12 @@ static int kvm_eiointc_read(struct kvm_vcpu *vcpu,
>> return -EINVAL;
>> }
>>
>> + /* len must be 1/2/4/8 from function kvm_emu_iocsr() */
>> + if (addr & (len - 1)) {
>> + kvm_err("%s: eiointc not aligned addr %llx len %d\n", __func__, addr, len);
>> + return -EINVAL;
>> + }
>> +
>> vcpu->stat.eiointc_read_exits++;
>> spin_lock_irqsave(&eiointc->lock, flags);
>> switch (len) {
>> @@ -323,12 +329,9 @@ static int kvm_eiointc_read(struct kvm_vcpu *vcpu,
>> case 4:
>> ret = loongarch_eiointc_readl(vcpu, eiointc, addr, val);
>> break;
>> - case 8:
>> + default:
>> ret = loongarch_eiointc_readq(vcpu, eiointc, addr, val);
>> break;
>> - default:
>> - WARN_ONCE(1, "%s: Abnormal address access: addr 0x%llx, size %d\n",
>> - __func__, addr, len);
>> }
>> spin_unlock_irqrestore(&eiointc->lock, flags);
>>
>> @@ -682,6 +685,11 @@ static int kvm_eiointc_write(struct kvm_vcpu *vcpu,
>> return -EINVAL;
>> }
>>
>> + if (addr & (len - 1)) {
>> + kvm_err("%s: eiointc not aligned addr %llx len %d\n", __func__, addr, len);
>> + return -EINVAL;
>> + }
>> +
>> vcpu->stat.eiointc_write_exits++;
>> spin_lock_irqsave(&eiointc->lock, flags);
>> switch (len) {
>> @@ -694,12 +702,9 @@ static int kvm_eiointc_write(struct kvm_vcpu *vcpu,
>> case 4:
>> ret = loongarch_eiointc_writel(vcpu, eiointc, addr, val);
>> break;
>> - case 8:
>> + default:
>> ret = loongarch_eiointc_writeq(vcpu, eiointc, addr, val);
>> break;
>> - default:
>> - WARN_ONCE(1, "%s: Abnormal address access: addr 0x%llx, size %d\n",
>> - __func__, addr, len);
>> }
>> spin_unlock_irqrestore(&eiointc->lock, flags);
>>
>> --
>> 2.39.3
>>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 4/9] LoongArch: KVM: INTC: Check validation of num_cpu from user space
2025-06-19 8:46 ` Huacai Chen
@ 2025-06-20 1:42 ` Bibo Mao
2025-06-20 14:43 ` Huacai Chen
0 siblings, 1 reply; 22+ messages in thread
From: Bibo Mao @ 2025-06-20 1:42 UTC (permalink / raw)
To: Huacai Chen
Cc: Tianrui Zhao, Xianglai Li, kvm, loongarch, linux-kernel, stable
On 2025/6/19 下午4:46, Huacai Chen wrote:
> Hi, Bibo,
>
> On Wed, Jun 11, 2025 at 9:47 AM Bibo Mao <maobibo@loongson.cn> wrote:
>>
>> The maximum supported cpu number is EIOINTC_ROUTE_MAX_VCPUS about
>> irqchip eiointc, here add validation about cpu number to avoid array
>> pointer overflow.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: 1ad7efa552fd ("LoongArch: KVM: Add EIOINTC user mode read and write functions")
>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>> ---
>> arch/loongarch/kvm/intc/eiointc.c | 18 +++++++++++++-----
>> 1 file changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
>> index b48511f903b5..ed80bf290755 100644
>> --- a/arch/loongarch/kvm/intc/eiointc.c
>> +++ b/arch/loongarch/kvm/intc/eiointc.c
>> @@ -798,7 +798,7 @@ static int kvm_eiointc_ctrl_access(struct kvm_device *dev,
>> int ret = 0;
>> unsigned long flags;
>> unsigned long type = (unsigned long)attr->attr;
>> - u32 i, start_irq;
>> + u32 i, start_irq, val;
>> void __user *data;
>> struct loongarch_eiointc *s = dev->kvm->arch.eiointc;
>>
>> @@ -806,7 +806,12 @@ static int kvm_eiointc_ctrl_access(struct kvm_device *dev,
>> spin_lock_irqsave(&s->lock, flags);
>> switch (type) {
>> case KVM_DEV_LOONGARCH_EXTIOI_CTRL_INIT_NUM_CPU:
>> - if (copy_from_user(&s->num_cpu, data, 4))
>> + if (copy_from_user(&val, data, 4) == 0) {
>> + if (val < EIOINTC_ROUTE_MAX_VCPUS)
>> + s->num_cpu = val;
>> + else
>> + ret = -EINVAL;
> Maybe it is better to set s->num_cpu to EIOINTC_ROUTE_MAX_VCPUS (or
> other value) rather than keep it uninitialized. Because in other
> places we need to check s->num_cpu and an uninitialized value may
> cause undefined behavior.
There is error return value -EINVAL, VMM should stop running and exit
immediately if there is error return value with the ioctl command.
num_cpu is not uninitialized and it is zero by default. If VMM does not
care about the return value, VMM will fail to get coreisr information in
future.
Regards
Bibo Mao
>
>
> Huacai
>> + } else
>> ret = -EFAULT;
>> break;
>> case KVM_DEV_LOONGARCH_EXTIOI_CTRL_INIT_FEATURE:
>> @@ -835,7 +840,7 @@ static int kvm_eiointc_regs_access(struct kvm_device *dev,
>> struct kvm_device_attr *attr,
>> bool is_write)
>> {
>> - int addr, cpuid, offset, ret = 0;
>> + int addr, cpu, offset, ret = 0;
>> unsigned long flags;
>> void *p = NULL;
>> void __user *data;
>> @@ -843,7 +848,7 @@ static int kvm_eiointc_regs_access(struct kvm_device *dev,
>>
>> s = dev->kvm->arch.eiointc;
>> addr = attr->attr;
>> - cpuid = addr >> 16;
>> + cpu = addr >> 16;
>> addr &= 0xffff;
>> data = (void __user *)attr->addr;
>> switch (addr) {
>> @@ -868,8 +873,11 @@ static int kvm_eiointc_regs_access(struct kvm_device *dev,
>> p = &s->isr.reg_u32[offset];
>> break;
>> case EIOINTC_COREISR_START ... EIOINTC_COREISR_END:
>> + if (cpu >= s->num_cpu)
>> + return -EINVAL;
>> +
>> offset = (addr - EIOINTC_COREISR_START) / 4;
>> - p = &s->coreisr.reg_u32[cpuid][offset];
>> + p = &s->coreisr.reg_u32[cpu][offset];
>> break;
>> case EIOINTC_COREMAP_START ... EIOINTC_COREMAP_END:
>> offset = (addr - EIOINTC_COREMAP_START) / 4;
>> --
>> 2.39.3
>>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 4/9] LoongArch: KVM: INTC: Check validation of num_cpu from user space
2025-06-20 1:42 ` Bibo Mao
@ 2025-06-20 14:43 ` Huacai Chen
2025-06-27 7:42 ` Bibo Mao
0 siblings, 1 reply; 22+ messages in thread
From: Huacai Chen @ 2025-06-20 14:43 UTC (permalink / raw)
To: Bibo Mao; +Cc: Tianrui Zhao, Xianglai Li, kvm, loongarch, linux-kernel, stable
On Fri, Jun 20, 2025 at 9:43 AM Bibo Mao <maobibo@loongson.cn> wrote:
>
>
>
> On 2025/6/19 下午4:46, Huacai Chen wrote:
> > Hi, Bibo,
> >
> > On Wed, Jun 11, 2025 at 9:47 AM Bibo Mao <maobibo@loongson.cn> wrote:
> >>
> >> The maximum supported cpu number is EIOINTC_ROUTE_MAX_VCPUS about
> >> irqchip eiointc, here add validation about cpu number to avoid array
> >> pointer overflow.
> >>
> >> Cc: stable@vger.kernel.org
> >> Fixes: 1ad7efa552fd ("LoongArch: KVM: Add EIOINTC user mode read and write functions")
> >> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >> ---
> >> arch/loongarch/kvm/intc/eiointc.c | 18 +++++++++++++-----
> >> 1 file changed, 13 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
> >> index b48511f903b5..ed80bf290755 100644
> >> --- a/arch/loongarch/kvm/intc/eiointc.c
> >> +++ b/arch/loongarch/kvm/intc/eiointc.c
> >> @@ -798,7 +798,7 @@ static int kvm_eiointc_ctrl_access(struct kvm_device *dev,
> >> int ret = 0;
> >> unsigned long flags;
> >> unsigned long type = (unsigned long)attr->attr;
> >> - u32 i, start_irq;
> >> + u32 i, start_irq, val;
> >> void __user *data;
> >> struct loongarch_eiointc *s = dev->kvm->arch.eiointc;
> >>
> >> @@ -806,7 +806,12 @@ static int kvm_eiointc_ctrl_access(struct kvm_device *dev,
> >> spin_lock_irqsave(&s->lock, flags);
> >> switch (type) {
> >> case KVM_DEV_LOONGARCH_EXTIOI_CTRL_INIT_NUM_CPU:
> >> - if (copy_from_user(&s->num_cpu, data, 4))
> >> + if (copy_from_user(&val, data, 4) == 0) {
> >> + if (val < EIOINTC_ROUTE_MAX_VCPUS)
> >> + s->num_cpu = val;
> >> + else
> >> + ret = -EINVAL;
> > Maybe it is better to set s->num_cpu to EIOINTC_ROUTE_MAX_VCPUS (or
> > other value) rather than keep it uninitialized. Because in other
> > places we need to check s->num_cpu and an uninitialized value may
> > cause undefined behavior.
> There is error return value -EINVAL, VMM should stop running and exit
> immediately if there is error return value with the ioctl command.
>
> num_cpu is not uninitialized and it is zero by default. If VMM does not
> care about the return value, VMM will fail to get coreisr information in
> future.
If you are sure you can keep it as is. Then please resend patch
1,2,3,4,5,9 as a series because they are all bug fixes that should be
merged as soon as possible. And in my own opinion, "INTC" can be
dropped in the title.
Huacai
>
> Regards
> Bibo Mao
> >
> >
> > Huacai
> >> + } else
> >> ret = -EFAULT;
> >> break;
> >> case KVM_DEV_LOONGARCH_EXTIOI_CTRL_INIT_FEATURE:
> >> @@ -835,7 +840,7 @@ static int kvm_eiointc_regs_access(struct kvm_device *dev,
> >> struct kvm_device_attr *attr,
> >> bool is_write)
> >> {
> >> - int addr, cpuid, offset, ret = 0;
> >> + int addr, cpu, offset, ret = 0;
> >> unsigned long flags;
> >> void *p = NULL;
> >> void __user *data;
> >> @@ -843,7 +848,7 @@ static int kvm_eiointc_regs_access(struct kvm_device *dev,
> >>
> >> s = dev->kvm->arch.eiointc;
> >> addr = attr->attr;
> >> - cpuid = addr >> 16;
> >> + cpu = addr >> 16;
> >> addr &= 0xffff;
> >> data = (void __user *)attr->addr;
> >> switch (addr) {
> >> @@ -868,8 +873,11 @@ static int kvm_eiointc_regs_access(struct kvm_device *dev,
> >> p = &s->isr.reg_u32[offset];
> >> break;
> >> case EIOINTC_COREISR_START ... EIOINTC_COREISR_END:
> >> + if (cpu >= s->num_cpu)
> >> + return -EINVAL;
> >> +
> >> offset = (addr - EIOINTC_COREISR_START) / 4;
> >> - p = &s->coreisr.reg_u32[cpuid][offset];
> >> + p = &s->coreisr.reg_u32[cpu][offset];
> >> break;
> >> case EIOINTC_COREMAP_START ... EIOINTC_COREMAP_END:
> >> offset = (addr - EIOINTC_COREMAP_START) / 4;
> >> --
> >> 2.39.3
> >>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 9/9] LoongArch: KVM: INTC: Add address alignment check
2025-06-19 8:47 ` Huacai Chen
2025-06-19 9:47 ` Bibo Mao
@ 2025-06-21 11:20 ` David Laight
2025-06-21 13:04 ` Huacai Chen
1 sibling, 1 reply; 22+ messages in thread
From: David Laight @ 2025-06-21 11:20 UTC (permalink / raw)
To: Huacai Chen
Cc: Bibo Mao, Tianrui Zhao, Xianglai Li, kvm, loongarch, linux-kernel
On Thu, 19 Jun 2025 16:47:22 +0800
Huacai Chen <chenhuacai@kernel.org> wrote:
> Hi, Bibo,
>
> On Wed, Jun 11, 2025 at 9:51 AM Bibo Mao <maobibo@loongson.cn> wrote:
> >
> > IOCSR instruction supports 1/2/4/8 bytes access, the address should
> > be naturally aligned with its access size. Here address alignment
> > check is added in eiointc kernel emulation.
> >
> > At the same time len must be 1/2/4/8 bytes from iocsr exit emulation
> > function kvm_emu_iocsr(), remove the default case in switch case
> > statements.
> Robust code doesn't depend its callers do things right, so I suggest
> keeping the default case, which means we just add the alignment check
> here.
kernel code generally relies on callers to DTRT - except for values
that come from userspace.
Otherwise you get unreadable and slow code that continuously checks
for things that can't happen.
David
>
> And I think this patch should also Cc stable and add a Fixes tag.
>
>
> Huacai
>
> >
> > Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> > ---
> > arch/loongarch/kvm/intc/eiointc.c | 21 +++++++++++++--------
> > 1 file changed, 13 insertions(+), 8 deletions(-)
> >
> > diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
> > index 8b0d9376eb54..4e9d12300cc4 100644
> > --- a/arch/loongarch/kvm/intc/eiointc.c
> > +++ b/arch/loongarch/kvm/intc/eiointc.c
> > @@ -311,6 +311,12 @@ static int kvm_eiointc_read(struct kvm_vcpu *vcpu,
> > return -EINVAL;
> > }
> >
> > + /* len must be 1/2/4/8 from function kvm_emu_iocsr() */
> > + if (addr & (len - 1)) {
> > + kvm_err("%s: eiointc not aligned addr %llx len %d\n", __func__, addr, len);
> > + return -EINVAL;
> > + }
> > +
> > vcpu->stat.eiointc_read_exits++;
> > spin_lock_irqsave(&eiointc->lock, flags);
> > switch (len) {
> > @@ -323,12 +329,9 @@ static int kvm_eiointc_read(struct kvm_vcpu *vcpu,
> > case 4:
> > ret = loongarch_eiointc_readl(vcpu, eiointc, addr, val);
> > break;
> > - case 8:
> > + default:
> > ret = loongarch_eiointc_readq(vcpu, eiointc, addr, val);
> > break;
> > - default:
> > - WARN_ONCE(1, "%s: Abnormal address access: addr 0x%llx, size %d\n",
> > - __func__, addr, len);
> > }
> > spin_unlock_irqrestore(&eiointc->lock, flags);
> >
> > @@ -682,6 +685,11 @@ static int kvm_eiointc_write(struct kvm_vcpu *vcpu,
> > return -EINVAL;
> > }
> >
> > + if (addr & (len - 1)) {
> > + kvm_err("%s: eiointc not aligned addr %llx len %d\n", __func__, addr, len);
> > + return -EINVAL;
> > + }
> > +
> > vcpu->stat.eiointc_write_exits++;
> > spin_lock_irqsave(&eiointc->lock, flags);
> > switch (len) {
> > @@ -694,12 +702,9 @@ static int kvm_eiointc_write(struct kvm_vcpu *vcpu,
> > case 4:
> > ret = loongarch_eiointc_writel(vcpu, eiointc, addr, val);
> > break;
> > - case 8:
> > + default:
> > ret = loongarch_eiointc_writeq(vcpu, eiointc, addr, val);
> > break;
> > - default:
> > - WARN_ONCE(1, "%s: Abnormal address access: addr 0x%llx, size %d\n",
> > - __func__, addr, len);
> > }
> > spin_unlock_irqrestore(&eiointc->lock, flags);
> >
> > --
> > 2.39.3
> >
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 9/9] LoongArch: KVM: INTC: Add address alignment check
2025-06-21 11:20 ` David Laight
@ 2025-06-21 13:04 ` Huacai Chen
2025-06-27 1:58 ` Bibo Mao
2025-06-27 2:12 ` Bibo Mao
0 siblings, 2 replies; 22+ messages in thread
From: Huacai Chen @ 2025-06-21 13:04 UTC (permalink / raw)
To: David Laight
Cc: Bibo Mao, Tianrui Zhao, Xianglai Li, kvm, loongarch, linux-kernel
Hi, David,
On Sat, Jun 21, 2025 at 7:21 PM David Laight
<david.laight.linux@gmail.com> wrote:
>
> On Thu, 19 Jun 2025 16:47:22 +0800
> Huacai Chen <chenhuacai@kernel.org> wrote:
>
> > Hi, Bibo,
> >
> > On Wed, Jun 11, 2025 at 9:51 AM Bibo Mao <maobibo@loongson.cn> wrote:
> > >
> > > IOCSR instruction supports 1/2/4/8 bytes access, the address should
> > > be naturally aligned with its access size. Here address alignment
> > > check is added in eiointc kernel emulation.
> > >
> > > At the same time len must be 1/2/4/8 bytes from iocsr exit emulation
> > > function kvm_emu_iocsr(), remove the default case in switch case
> > > statements.
> > Robust code doesn't depend its callers do things right, so I suggest
> > keeping the default case, which means we just add the alignment check
> > here.
>
> kernel code generally relies on callers to DTRT - except for values
> that come from userspace.
>
> Otherwise you get unreadable and slow code that continuously checks
> for things that can't happen.
Generally you are right - but this patch is not the case.
Adding a "default" case here doesn't make code slower or unreadable,
and the code becomes more robust.
Huacai
>
> David
>
> >
> > And I think this patch should also Cc stable and add a Fixes tag.
> >
> >
> > Huacai
> >
> > >
> > > Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> > > ---
> > > arch/loongarch/kvm/intc/eiointc.c | 21 +++++++++++++--------
> > > 1 file changed, 13 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
> > > index 8b0d9376eb54..4e9d12300cc4 100644
> > > --- a/arch/loongarch/kvm/intc/eiointc.c
> > > +++ b/arch/loongarch/kvm/intc/eiointc.c
> > > @@ -311,6 +311,12 @@ static int kvm_eiointc_read(struct kvm_vcpu *vcpu,
> > > return -EINVAL;
> > > }
> > >
> > > + /* len must be 1/2/4/8 from function kvm_emu_iocsr() */
> > > + if (addr & (len - 1)) {
> > > + kvm_err("%s: eiointc not aligned addr %llx len %d\n", __func__, addr, len);
> > > + return -EINVAL;
> > > + }
> > > +
> > > vcpu->stat.eiointc_read_exits++;
> > > spin_lock_irqsave(&eiointc->lock, flags);
> > > switch (len) {
> > > @@ -323,12 +329,9 @@ static int kvm_eiointc_read(struct kvm_vcpu *vcpu,
> > > case 4:
> > > ret = loongarch_eiointc_readl(vcpu, eiointc, addr, val);
> > > break;
> > > - case 8:
> > > + default:
> > > ret = loongarch_eiointc_readq(vcpu, eiointc, addr, val);
> > > break;
> > > - default:
> > > - WARN_ONCE(1, "%s: Abnormal address access: addr 0x%llx, size %d\n",
> > > - __func__, addr, len);
> > > }
> > > spin_unlock_irqrestore(&eiointc->lock, flags);
> > >
> > > @@ -682,6 +685,11 @@ static int kvm_eiointc_write(struct kvm_vcpu *vcpu,
> > > return -EINVAL;
> > > }
> > >
> > > + if (addr & (len - 1)) {
> > > + kvm_err("%s: eiointc not aligned addr %llx len %d\n", __func__, addr, len);
> > > + return -EINVAL;
> > > + }
> > > +
> > > vcpu->stat.eiointc_write_exits++;
> > > spin_lock_irqsave(&eiointc->lock, flags);
> > > switch (len) {
> > > @@ -694,12 +702,9 @@ static int kvm_eiointc_write(struct kvm_vcpu *vcpu,
> > > case 4:
> > > ret = loongarch_eiointc_writel(vcpu, eiointc, addr, val);
> > > break;
> > > - case 8:
> > > + default:
> > > ret = loongarch_eiointc_writeq(vcpu, eiointc, addr, val);
> > > break;
> > > - default:
> > > - WARN_ONCE(1, "%s: Abnormal address access: addr 0x%llx, size %d\n",
> > > - __func__, addr, len);
> > > }
> > > spin_unlock_irqrestore(&eiointc->lock, flags);
> > >
> > > --
> > > 2.39.3
> > >
> >
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 9/9] LoongArch: KVM: INTC: Add address alignment check
2025-06-21 13:04 ` Huacai Chen
@ 2025-06-27 1:58 ` Bibo Mao
2025-06-27 2:12 ` Bibo Mao
1 sibling, 0 replies; 22+ messages in thread
From: Bibo Mao @ 2025-06-27 1:58 UTC (permalink / raw)
To: Huacai Chen, David Laight
Cc: Tianrui Zhao, Xianglai Li, kvm, loongarch, linux-kernel
On 2025/6/21 下午9:04, Huacai Chen wrote:
> Hi, David,
>
> On Sat, Jun 21, 2025 at 7:21 PM David Laight
> <david.laight.linux@gmail.com> wrote:
>>
>> On Thu, 19 Jun 2025 16:47:22 +0800
>> Huacai Chen <chenhuacai@kernel.org> wrote:
>>
>>> Hi, Bibo,
>>>
>>> On Wed, Jun 11, 2025 at 9:51 AM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>
>>>> IOCSR instruction supports 1/2/4/8 bytes access, the address should
>>>> be naturally aligned with its access size. Here address alignment
>>>> check is added in eiointc kernel emulation.
>>>>
>>>> At the same time len must be 1/2/4/8 bytes from iocsr exit emulation
>>>> function kvm_emu_iocsr(), remove the default case in switch case
>>>> statements.
>>> Robust code doesn't depend its callers do things right, so I suggest
>>> keeping the default case, which means we just add the alignment check
>>> here.
>>
>> kernel code generally relies on callers to DTRT - except for values
>> that come from userspace.
>>
>> Otherwise you get unreadable and slow code that continuously checks
>> for things that can't happen.
> Generally you are right - but this patch is not the case.
>
> Adding a "default" case here doesn't make code slower or unreadable,
> and the code becomes more robust.
By my understanding, the default case cannot happen never with iocsr
emulation function kvm_emu_iocsr() in kernel in the previous patch.
David suggests that default case can be removed since it will not
happen. It actually makes code quicker if compared with assembly
language, since there is one if-else comparison reduction.
Regards
Bibo Mao
>
> Huacai
>
>>
>> David
>>
>>>
>>> And I think this patch should also Cc stable and add a Fixes tag.
>>>
>>>
>>> Huacai
>>>
>>>>
>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>> ---
>>>> arch/loongarch/kvm/intc/eiointc.c | 21 +++++++++++++--------
>>>> 1 file changed, 13 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
>>>> index 8b0d9376eb54..4e9d12300cc4 100644
>>>> --- a/arch/loongarch/kvm/intc/eiointc.c
>>>> +++ b/arch/loongarch/kvm/intc/eiointc.c
>>>> @@ -311,6 +311,12 @@ static int kvm_eiointc_read(struct kvm_vcpu *vcpu,
>>>> return -EINVAL;
>>>> }
>>>>
>>>> + /* len must be 1/2/4/8 from function kvm_emu_iocsr() */
>>>> + if (addr & (len - 1)) {
>>>> + kvm_err("%s: eiointc not aligned addr %llx len %d\n", __func__, addr, len);
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> vcpu->stat.eiointc_read_exits++;
>>>> spin_lock_irqsave(&eiointc->lock, flags);
>>>> switch (len) {
>>>> @@ -323,12 +329,9 @@ static int kvm_eiointc_read(struct kvm_vcpu *vcpu,
>>>> case 4:
>>>> ret = loongarch_eiointc_readl(vcpu, eiointc, addr, val);
>>>> break;
>>>> - case 8:
>>>> + default:
>>>> ret = loongarch_eiointc_readq(vcpu, eiointc, addr, val);
>>>> break;
>>>> - default:
>>>> - WARN_ONCE(1, "%s: Abnormal address access: addr 0x%llx, size %d\n",
>>>> - __func__, addr, len);
>>>> }
>>>> spin_unlock_irqrestore(&eiointc->lock, flags);
>>>>
>>>> @@ -682,6 +685,11 @@ static int kvm_eiointc_write(struct kvm_vcpu *vcpu,
>>>> return -EINVAL;
>>>> }
>>>>
>>>> + if (addr & (len - 1)) {
>>>> + kvm_err("%s: eiointc not aligned addr %llx len %d\n", __func__, addr, len);
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> vcpu->stat.eiointc_write_exits++;
>>>> spin_lock_irqsave(&eiointc->lock, flags);
>>>> switch (len) {
>>>> @@ -694,12 +702,9 @@ static int kvm_eiointc_write(struct kvm_vcpu *vcpu,
>>>> case 4:
>>>> ret = loongarch_eiointc_writel(vcpu, eiointc, addr, val);
>>>> break;
>>>> - case 8:
>>>> + default:
>>>> ret = loongarch_eiointc_writeq(vcpu, eiointc, addr, val);
>>>> break;
>>>> - default:
>>>> - WARN_ONCE(1, "%s: Abnormal address access: addr 0x%llx, size %d\n",
>>>> - __func__, addr, len);
>>>> }
>>>> spin_unlock_irqrestore(&eiointc->lock, flags);
>>>>
>>>> --
>>>> 2.39.3
>>>>
>>>
>>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 9/9] LoongArch: KVM: INTC: Add address alignment check
2025-06-21 13:04 ` Huacai Chen
2025-06-27 1:58 ` Bibo Mao
@ 2025-06-27 2:12 ` Bibo Mao
1 sibling, 0 replies; 22+ messages in thread
From: Bibo Mao @ 2025-06-27 2:12 UTC (permalink / raw)
To: Huacai Chen, David Laight
Cc: Tianrui Zhao, Xianglai Li, kvm, loongarch, linux-kernel
That is a little different with x86/arm64 kvm.
With iocsr address space, there is access size with 1/2/4/8. With IO
memory address space, there is generic access size with 1/2/4/8, however
if irqchip driver of VM is deployed in user space, it will be possible
with 16/32 bytes with SIMD instruction.
Regards
Bibo Mao
On 2025/6/21 下午9:04, Huacai Chen wrote:
> Hi, David,
>
> On Sat, Jun 21, 2025 at 7:21 PM David Laight
> <david.laight.linux@gmail.com> wrote:
>>
>> On Thu, 19 Jun 2025 16:47:22 +0800
>> Huacai Chen <chenhuacai@kernel.org> wrote:
>>
>>> Hi, Bibo,
>>>
>>> On Wed, Jun 11, 2025 at 9:51 AM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>
>>>> IOCSR instruction supports 1/2/4/8 bytes access, the address should
>>>> be naturally aligned with its access size. Here address alignment
>>>> check is added in eiointc kernel emulation.
>>>>
>>>> At the same time len must be 1/2/4/8 bytes from iocsr exit emulation
>>>> function kvm_emu_iocsr(), remove the default case in switch case
>>>> statements.
>>> Robust code doesn't depend its callers do things right, so I suggest
>>> keeping the default case, which means we just add the alignment check
>>> here.
>>
>> kernel code generally relies on callers to DTRT - except for values
>> that come from userspace.
>>
>> Otherwise you get unreadable and slow code that continuously checks
>> for things that can't happen.
> Generally you are right - but this patch is not the case.
>
> Adding a "default" case here doesn't make code slower or unreadable,
> and the code becomes more robust.
>
> Huacai
>
>>
>> David
>>
>>>
>>> And I think this patch should also Cc stable and add a Fixes tag.
>>>
>>>
>>> Huacai
>>>
>>>>
>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>> ---
>>>> arch/loongarch/kvm/intc/eiointc.c | 21 +++++++++++++--------
>>>> 1 file changed, 13 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
>>>> index 8b0d9376eb54..4e9d12300cc4 100644
>>>> --- a/arch/loongarch/kvm/intc/eiointc.c
>>>> +++ b/arch/loongarch/kvm/intc/eiointc.c
>>>> @@ -311,6 +311,12 @@ static int kvm_eiointc_read(struct kvm_vcpu *vcpu,
>>>> return -EINVAL;
>>>> }
>>>>
>>>> + /* len must be 1/2/4/8 from function kvm_emu_iocsr() */
>>>> + if (addr & (len - 1)) {
>>>> + kvm_err("%s: eiointc not aligned addr %llx len %d\n", __func__, addr, len);
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> vcpu->stat.eiointc_read_exits++;
>>>> spin_lock_irqsave(&eiointc->lock, flags);
>>>> switch (len) {
>>>> @@ -323,12 +329,9 @@ static int kvm_eiointc_read(struct kvm_vcpu *vcpu,
>>>> case 4:
>>>> ret = loongarch_eiointc_readl(vcpu, eiointc, addr, val);
>>>> break;
>>>> - case 8:
>>>> + default:
>>>> ret = loongarch_eiointc_readq(vcpu, eiointc, addr, val);
>>>> break;
>>>> - default:
>>>> - WARN_ONCE(1, "%s: Abnormal address access: addr 0x%llx, size %d\n",
>>>> - __func__, addr, len);
>>>> }
>>>> spin_unlock_irqrestore(&eiointc->lock, flags);
>>>>
>>>> @@ -682,6 +685,11 @@ static int kvm_eiointc_write(struct kvm_vcpu *vcpu,
>>>> return -EINVAL;
>>>> }
>>>>
>>>> + if (addr & (len - 1)) {
>>>> + kvm_err("%s: eiointc not aligned addr %llx len %d\n", __func__, addr, len);
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> vcpu->stat.eiointc_write_exits++;
>>>> spin_lock_irqsave(&eiointc->lock, flags);
>>>> switch (len) {
>>>> @@ -694,12 +702,9 @@ static int kvm_eiointc_write(struct kvm_vcpu *vcpu,
>>>> case 4:
>>>> ret = loongarch_eiointc_writel(vcpu, eiointc, addr, val);
>>>> break;
>>>> - case 8:
>>>> + default:
>>>> ret = loongarch_eiointc_writeq(vcpu, eiointc, addr, val);
>>>> break;
>>>> - default:
>>>> - WARN_ONCE(1, "%s: Abnormal address access: addr 0x%llx, size %d\n",
>>>> - __func__, addr, len);
>>>> }
>>>> spin_unlock_irqrestore(&eiointc->lock, flags);
>>>>
>>>> --
>>>> 2.39.3
>>>>
>>>
>>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 4/9] LoongArch: KVM: INTC: Check validation of num_cpu from user space
2025-06-20 14:43 ` Huacai Chen
@ 2025-06-27 7:42 ` Bibo Mao
2025-06-27 9:04 ` Huacai Chen
0 siblings, 1 reply; 22+ messages in thread
From: Bibo Mao @ 2025-06-27 7:42 UTC (permalink / raw)
To: Huacai Chen
Cc: Tianrui Zhao, Xianglai Li, kvm, loongarch, linux-kernel, stable
On 2025/6/20 下午10:43, Huacai Chen wrote:
> On Fri, Jun 20, 2025 at 9:43 AM Bibo Mao <maobibo@loongson.cn> wrote:
>>
>>
>>
>> On 2025/6/19 下午4:46, Huacai Chen wrote:
>>> Hi, Bibo,
>>>
>>> On Wed, Jun 11, 2025 at 9:47 AM Bibo Mao <maobibo@loongson.cn> wrote:
>>>>
>>>> The maximum supported cpu number is EIOINTC_ROUTE_MAX_VCPUS about
>>>> irqchip eiointc, here add validation about cpu number to avoid array
>>>> pointer overflow.
>>>>
>>>> Cc: stable@vger.kernel.org
>>>> Fixes: 1ad7efa552fd ("LoongArch: KVM: Add EIOINTC user mode read and write functions")
>>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>>>> ---
>>>> arch/loongarch/kvm/intc/eiointc.c | 18 +++++++++++++-----
>>>> 1 file changed, 13 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
>>>> index b48511f903b5..ed80bf290755 100644
>>>> --- a/arch/loongarch/kvm/intc/eiointc.c
>>>> +++ b/arch/loongarch/kvm/intc/eiointc.c
>>>> @@ -798,7 +798,7 @@ static int kvm_eiointc_ctrl_access(struct kvm_device *dev,
>>>> int ret = 0;
>>>> unsigned long flags;
>>>> unsigned long type = (unsigned long)attr->attr;
>>>> - u32 i, start_irq;
>>>> + u32 i, start_irq, val;
>>>> void __user *data;
>>>> struct loongarch_eiointc *s = dev->kvm->arch.eiointc;
>>>>
>>>> @@ -806,7 +806,12 @@ static int kvm_eiointc_ctrl_access(struct kvm_device *dev,
>>>> spin_lock_irqsave(&s->lock, flags);
>>>> switch (type) {
>>>> case KVM_DEV_LOONGARCH_EXTIOI_CTRL_INIT_NUM_CPU:
>>>> - if (copy_from_user(&s->num_cpu, data, 4))
>>>> + if (copy_from_user(&val, data, 4) == 0) {
>>>> + if (val < EIOINTC_ROUTE_MAX_VCPUS)
>>>> + s->num_cpu = val;
>>>> + else
>>>> + ret = -EINVAL;
>>> Maybe it is better to set s->num_cpu to EIOINTC_ROUTE_MAX_VCPUS (or
>>> other value) rather than keep it uninitialized. Because in other
>>> places we need to check s->num_cpu and an uninitialized value may
>>> cause undefined behavior.
>> There is error return value -EINVAL, VMM should stop running and exit
>> immediately if there is error return value with the ioctl command.
>>
>> num_cpu is not uninitialized and it is zero by default. If VMM does not
>> care about the return value, VMM will fail to get coreisr information in
>> future.
> If you are sure you can keep it as is. Then please resend patch
> 1,2,3,4,5,9 as a series because they are all bug fixes that should be
> merged as soon as possible. And in my own opinion, "INTC" can be
> dropped in the title.
Ok, will do in this way.
Regards
Bibo Mao
>
>
> Huacai
>
>>
>> Regards
>> Bibo Mao
>>>
>>>
>>> Huacai
>>>> + } else
>>>> ret = -EFAULT;
>>>> break;
>>>> case KVM_DEV_LOONGARCH_EXTIOI_CTRL_INIT_FEATURE:
>>>> @@ -835,7 +840,7 @@ static int kvm_eiointc_regs_access(struct kvm_device *dev,
>>>> struct kvm_device_attr *attr,
>>>> bool is_write)
>>>> {
>>>> - int addr, cpuid, offset, ret = 0;
>>>> + int addr, cpu, offset, ret = 0;
>>>> unsigned long flags;
>>>> void *p = NULL;
>>>> void __user *data;
>>>> @@ -843,7 +848,7 @@ static int kvm_eiointc_regs_access(struct kvm_device *dev,
>>>>
>>>> s = dev->kvm->arch.eiointc;
>>>> addr = attr->attr;
>>>> - cpuid = addr >> 16;
>>>> + cpu = addr >> 16;
>>>> addr &= 0xffff;
>>>> data = (void __user *)attr->addr;
>>>> switch (addr) {
>>>> @@ -868,8 +873,11 @@ static int kvm_eiointc_regs_access(struct kvm_device *dev,
>>>> p = &s->isr.reg_u32[offset];
>>>> break;
>>>> case EIOINTC_COREISR_START ... EIOINTC_COREISR_END:
>>>> + if (cpu >= s->num_cpu)
>>>> + return -EINVAL;
>>>> +
>>>> offset = (addr - EIOINTC_COREISR_START) / 4;
>>>> - p = &s->coreisr.reg_u32[cpuid][offset];
>>>> + p = &s->coreisr.reg_u32[cpu][offset];
>>>> break;
>>>> case EIOINTC_COREMAP_START ... EIOINTC_COREMAP_END:
>>>> offset = (addr - EIOINTC_COREMAP_START) / 4;
>>>> --
>>>> 2.39.3
>>>>
>>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 4/9] LoongArch: KVM: INTC: Check validation of num_cpu from user space
2025-06-27 7:42 ` Bibo Mao
@ 2025-06-27 9:04 ` Huacai Chen
0 siblings, 0 replies; 22+ messages in thread
From: Huacai Chen @ 2025-06-27 9:04 UTC (permalink / raw)
To: Bibo Mao; +Cc: Tianrui Zhao, Xianglai Li, kvm, loongarch, linux-kernel, stable
On Fri, Jun 27, 2025 at 3:44 PM Bibo Mao <maobibo@loongson.cn> wrote:
>
>
>
> On 2025/6/20 下午10:43, Huacai Chen wrote:
> > On Fri, Jun 20, 2025 at 9:43 AM Bibo Mao <maobibo@loongson.cn> wrote:
> >>
> >>
> >>
> >> On 2025/6/19 下午4:46, Huacai Chen wrote:
> >>> Hi, Bibo,
> >>>
> >>> On Wed, Jun 11, 2025 at 9:47 AM Bibo Mao <maobibo@loongson.cn> wrote:
> >>>>
> >>>> The maximum supported cpu number is EIOINTC_ROUTE_MAX_VCPUS about
> >>>> irqchip eiointc, here add validation about cpu number to avoid array
> >>>> pointer overflow.
> >>>>
> >>>> Cc: stable@vger.kernel.org
> >>>> Fixes: 1ad7efa552fd ("LoongArch: KVM: Add EIOINTC user mode read and write functions")
> >>>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >>>> ---
> >>>> arch/loongarch/kvm/intc/eiointc.c | 18 +++++++++++++-----
> >>>> 1 file changed, 13 insertions(+), 5 deletions(-)
> >>>>
> >>>> diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
> >>>> index b48511f903b5..ed80bf290755 100644
> >>>> --- a/arch/loongarch/kvm/intc/eiointc.c
> >>>> +++ b/arch/loongarch/kvm/intc/eiointc.c
> >>>> @@ -798,7 +798,7 @@ static int kvm_eiointc_ctrl_access(struct kvm_device *dev,
> >>>> int ret = 0;
> >>>> unsigned long flags;
> >>>> unsigned long type = (unsigned long)attr->attr;
> >>>> - u32 i, start_irq;
> >>>> + u32 i, start_irq, val;
> >>>> void __user *data;
> >>>> struct loongarch_eiointc *s = dev->kvm->arch.eiointc;
> >>>>
> >>>> @@ -806,7 +806,12 @@ static int kvm_eiointc_ctrl_access(struct kvm_device *dev,
> >>>> spin_lock_irqsave(&s->lock, flags);
> >>>> switch (type) {
> >>>> case KVM_DEV_LOONGARCH_EXTIOI_CTRL_INIT_NUM_CPU:
> >>>> - if (copy_from_user(&s->num_cpu, data, 4))
> >>>> + if (copy_from_user(&val, data, 4) == 0) {
> >>>> + if (val < EIOINTC_ROUTE_MAX_VCPUS)
> >>>> + s->num_cpu = val;
> >>>> + else
> >>>> + ret = -EINVAL;
> >>> Maybe it is better to set s->num_cpu to EIOINTC_ROUTE_MAX_VCPUS (or
> >>> other value) rather than keep it uninitialized. Because in other
> >>> places we need to check s->num_cpu and an uninitialized value may
> >>> cause undefined behavior.
> >> There is error return value -EINVAL, VMM should stop running and exit
> >> immediately if there is error return value with the ioctl command.
> >>
> >> num_cpu is not uninitialized and it is zero by default. If VMM does not
> >> care about the return value, VMM will fail to get coreisr information in
> >> future.
> > If you are sure you can keep it as is. Then please resend patch
> > 1,2,3,4,5,9 as a series because they are all bug fixes that should be
> > merged as soon as possible. And in my own opinion, "INTC" can be
> > dropped in the title.
> Ok, will do in this way.
Not needed now, patches have been applied.
Huacai
>
> Regards
> Bibo Mao
> >
> >
> > Huacai
> >
> >>
> >> Regards
> >> Bibo Mao
> >>>
> >>>
> >>> Huacai
> >>>> + } else
> >>>> ret = -EFAULT;
> >>>> break;
> >>>> case KVM_DEV_LOONGARCH_EXTIOI_CTRL_INIT_FEATURE:
> >>>> @@ -835,7 +840,7 @@ static int kvm_eiointc_regs_access(struct kvm_device *dev,
> >>>> struct kvm_device_attr *attr,
> >>>> bool is_write)
> >>>> {
> >>>> - int addr, cpuid, offset, ret = 0;
> >>>> + int addr, cpu, offset, ret = 0;
> >>>> unsigned long flags;
> >>>> void *p = NULL;
> >>>> void __user *data;
> >>>> @@ -843,7 +848,7 @@ static int kvm_eiointc_regs_access(struct kvm_device *dev,
> >>>>
> >>>> s = dev->kvm->arch.eiointc;
> >>>> addr = attr->attr;
> >>>> - cpuid = addr >> 16;
> >>>> + cpu = addr >> 16;
> >>>> addr &= 0xffff;
> >>>> data = (void __user *)attr->addr;
> >>>> switch (addr) {
> >>>> @@ -868,8 +873,11 @@ static int kvm_eiointc_regs_access(struct kvm_device *dev,
> >>>> p = &s->isr.reg_u32[offset];
> >>>> break;
> >>>> case EIOINTC_COREISR_START ... EIOINTC_COREISR_END:
> >>>> + if (cpu >= s->num_cpu)
> >>>> + return -EINVAL;
> >>>> +
> >>>> offset = (addr - EIOINTC_COREISR_START) / 4;
> >>>> - p = &s->coreisr.reg_u32[cpuid][offset];
> >>>> + p = &s->coreisr.reg_u32[cpu][offset];
> >>>> break;
> >>>> case EIOINTC_COREMAP_START ... EIOINTC_COREMAP_END:
> >>>> offset = (addr - EIOINTC_COREMAP_START) / 4;
> >>>> --
> >>>> 2.39.3
> >>>>
> >>
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2025-06-27 9:05 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-11 1:46 [PATCH v3 0/9] LoongArch: KVM: INTC: Enhancement about eiointc emulation Bibo Mao
2025-06-11 1:46 ` [PATCH v3 1/9] LoongArch: KVM: INTC: Fix interrupt route update with eiointc Bibo Mao
2025-06-11 1:46 ` [PATCH v3 2/9] LoongArch: KVM: INTC: Check interrupt route from physical cpu Bibo Mao
2025-06-11 1:46 ` [PATCH v3 3/9] LoongArch: KVM: INTC: Disable update property num_cpu and feature Bibo Mao
2025-06-11 1:46 ` [PATCH v3 4/9] LoongArch: KVM: INTC: Check validation of num_cpu from user space Bibo Mao
2025-06-19 8:46 ` Huacai Chen
2025-06-20 1:42 ` Bibo Mao
2025-06-20 14:43 ` Huacai Chen
2025-06-27 7:42 ` Bibo Mao
2025-06-27 9:04 ` Huacai Chen
2025-06-11 1:46 ` [PATCH v3 5/9] LoongArch: KVM: INTC: Avoid overflow with array index Bibo Mao
2025-06-11 1:46 ` [PATCH v3 6/9] LoongArch: KVM: INTC: Use standard bitops API with eiointc Bibo Mao
2025-06-11 1:46 ` [PATCH v3 7/9] LoongArch: KVM: INTC: Remove unused parameter len Bibo Mao
2025-06-11 1:46 ` [PATCH v3 8/9] LoongArch: KVM: INTC: Add stat information with kernel irqchip Bibo Mao
2025-06-11 1:50 ` Bibo Mao
2025-06-11 1:51 ` [PATCH v3 9/9] LoongArch: KVM: INTC: Add address alignment check Bibo Mao
2025-06-19 8:47 ` Huacai Chen
2025-06-19 9:47 ` Bibo Mao
2025-06-21 11:20 ` David Laight
2025-06-21 13:04 ` Huacai Chen
2025-06-27 1:58 ` Bibo Mao
2025-06-27 2:12 ` Bibo Mao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).