* [PATCH v2 1/9] RISC-V: KVM: return ENOENT in *_one_reg() when reg is unknown
2023-08-01 22:26 [PATCH v2 0/9] RISC-V: KVM: change get_reg/set_reg error codes Daniel Henrique Barboza
@ 2023-08-01 22:26 ` Daniel Henrique Barboza
2023-08-01 22:26 ` [PATCH v2 2/9] RISC-V: KVM: use ENOENT in *_one_reg() when extension is unavailable Daniel Henrique Barboza
` (8 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Daniel Henrique Barboza @ 2023-08-01 22:26 UTC (permalink / raw)
To: kvm-riscv, linux-riscv, kvm; +Cc: anup, atishp, ajones, Daniel Henrique Barboza
get_one_reg() and set_one_reg() are returning EINVAL errors for almost
everything: if a reg doesn't exist, if a reg ID is malformatted, if the
associated CPU extension that implements the reg isn't present in the
host, and for set_one_reg() if the value being written is invalid.
This isn't wrong according to the existing KVM API docs (EINVAL can be
used when there's no such register) but adding more ENOENT instances
will make easier for userspace to understand what went wrong.
Existing userspaces can be affected by this error code change. We
checked a few. As of current upstream code, crosvm doesn't check for any
particular errno code when using kvm_(get|set)_one_reg(). Neither does
QEMU. rust-vmm doesn't have kvm-riscv support yet. Thus we have a good
chance of changing these error codes now while the KVM RISC-V ecosystem
is still new, minimizing user impact.
Change all get_one_reg() and set_one_reg() implementations to return
-ENOENT at all "no such register" cases.
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
arch/riscv/kvm/aia.c | 4 ++--
arch/riscv/kvm/vcpu_fp.c | 12 ++++++------
arch/riscv/kvm/vcpu_onereg.c | 30 +++++++++++++++---------------
arch/riscv/kvm/vcpu_sbi.c | 16 +++++++++-------
arch/riscv/kvm/vcpu_timer.c | 8 ++++----
5 files changed, 36 insertions(+), 34 deletions(-)
diff --git a/arch/riscv/kvm/aia.c b/arch/riscv/kvm/aia.c
index 585a3b42c52c..74bb27440527 100644
--- a/arch/riscv/kvm/aia.c
+++ b/arch/riscv/kvm/aia.c
@@ -176,7 +176,7 @@ int kvm_riscv_vcpu_aia_get_csr(struct kvm_vcpu *vcpu,
struct kvm_vcpu_aia_csr *csr = &vcpu->arch.aia_context.guest_csr;
if (reg_num >= sizeof(struct kvm_riscv_aia_csr) / sizeof(unsigned long))
- return -EINVAL;
+ return -ENOENT;
*out_val = 0;
if (kvm_riscv_aia_available())
@@ -192,7 +192,7 @@ int kvm_riscv_vcpu_aia_set_csr(struct kvm_vcpu *vcpu,
struct kvm_vcpu_aia_csr *csr = &vcpu->arch.aia_context.guest_csr;
if (reg_num >= sizeof(struct kvm_riscv_aia_csr) / sizeof(unsigned long))
- return -EINVAL;
+ return -ENOENT;
if (kvm_riscv_aia_available()) {
((unsigned long *)csr)[reg_num] = val;
diff --git a/arch/riscv/kvm/vcpu_fp.c b/arch/riscv/kvm/vcpu_fp.c
index 9d8cbc42057a..08ba48a395aa 100644
--- a/arch/riscv/kvm/vcpu_fp.c
+++ b/arch/riscv/kvm/vcpu_fp.c
@@ -96,7 +96,7 @@ int kvm_riscv_vcpu_get_reg_fp(struct kvm_vcpu *vcpu,
reg_num <= KVM_REG_RISCV_FP_F_REG(f[31]))
reg_val = &cntx->fp.f.f[reg_num];
else
- return -EINVAL;
+ return -ENOENT;
} else if ((rtype == KVM_REG_RISCV_FP_D) &&
riscv_isa_extension_available(vcpu->arch.isa, d)) {
if (reg_num == KVM_REG_RISCV_FP_D_REG(fcsr)) {
@@ -109,9 +109,9 @@ int kvm_riscv_vcpu_get_reg_fp(struct kvm_vcpu *vcpu,
return -EINVAL;
reg_val = &cntx->fp.d.f[reg_num];
} else
- return -EINVAL;
+ return -ENOENT;
} else
- return -EINVAL;
+ return -ENOENT;
if (copy_to_user(uaddr, reg_val, KVM_REG_SIZE(reg->id)))
return -EFAULT;
@@ -141,7 +141,7 @@ int kvm_riscv_vcpu_set_reg_fp(struct kvm_vcpu *vcpu,
reg_num <= KVM_REG_RISCV_FP_F_REG(f[31]))
reg_val = &cntx->fp.f.f[reg_num];
else
- return -EINVAL;
+ return -ENOENT;
} else if ((rtype == KVM_REG_RISCV_FP_D) &&
riscv_isa_extension_available(vcpu->arch.isa, d)) {
if (reg_num == KVM_REG_RISCV_FP_D_REG(fcsr)) {
@@ -154,9 +154,9 @@ int kvm_riscv_vcpu_set_reg_fp(struct kvm_vcpu *vcpu,
return -EINVAL;
reg_val = &cntx->fp.d.f[reg_num];
} else
- return -EINVAL;
+ return -ENOENT;
} else
- return -EINVAL;
+ return -ENOENT;
if (copy_from_user(reg_val, uaddr, KVM_REG_SIZE(reg->id)))
return -EFAULT;
diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
index 0dc2c2cecb45..ba63522be060 100644
--- a/arch/riscv/kvm/vcpu_onereg.c
+++ b/arch/riscv/kvm/vcpu_onereg.c
@@ -153,7 +153,7 @@ static int kvm_riscv_vcpu_get_reg_config(struct kvm_vcpu *vcpu,
reg_val = vcpu->arch.mimpid;
break;
default:
- return -EINVAL;
+ return -ENOENT;
}
if (copy_to_user(uaddr, ®_val, KVM_REG_SIZE(reg->id)))
@@ -235,7 +235,7 @@ static int kvm_riscv_vcpu_set_reg_config(struct kvm_vcpu *vcpu,
return -EBUSY;
break;
default:
- return -EINVAL;
+ return -ENOENT;
}
return 0;
@@ -255,7 +255,7 @@ static int kvm_riscv_vcpu_get_reg_core(struct kvm_vcpu *vcpu,
if (KVM_REG_SIZE(reg->id) != sizeof(unsigned long))
return -EINVAL;
if (reg_num >= sizeof(struct kvm_riscv_core) / sizeof(unsigned long))
- return -EINVAL;
+ return -ENOENT;
if (reg_num == KVM_REG_RISCV_CORE_REG(regs.pc))
reg_val = cntx->sepc;
@@ -266,7 +266,7 @@ static int kvm_riscv_vcpu_get_reg_core(struct kvm_vcpu *vcpu,
reg_val = (cntx->sstatus & SR_SPP) ?
KVM_RISCV_MODE_S : KVM_RISCV_MODE_U;
else
- return -EINVAL;
+ return -ENOENT;
if (copy_to_user(uaddr, ®_val, KVM_REG_SIZE(reg->id)))
return -EFAULT;
@@ -288,7 +288,7 @@ static int kvm_riscv_vcpu_set_reg_core(struct kvm_vcpu *vcpu,
if (KVM_REG_SIZE(reg->id) != sizeof(unsigned long))
return -EINVAL;
if (reg_num >= sizeof(struct kvm_riscv_core) / sizeof(unsigned long))
- return -EINVAL;
+ return -ENOENT;
if (copy_from_user(®_val, uaddr, KVM_REG_SIZE(reg->id)))
return -EFAULT;
@@ -304,7 +304,7 @@ static int kvm_riscv_vcpu_set_reg_core(struct kvm_vcpu *vcpu,
else
cntx->sstatus &= ~SR_SPP;
} else
- return -EINVAL;
+ return -ENOENT;
return 0;
}
@@ -316,7 +316,7 @@ static int kvm_riscv_vcpu_general_get_csr(struct kvm_vcpu *vcpu,
struct kvm_vcpu_csr *csr = &vcpu->arch.guest_csr;
if (reg_num >= sizeof(struct kvm_riscv_csr) / sizeof(unsigned long))
- return -EINVAL;
+ return -ENOENT;
if (reg_num == KVM_REG_RISCV_CSR_REG(sip)) {
kvm_riscv_vcpu_flush_interrupts(vcpu);
@@ -335,7 +335,7 @@ static int kvm_riscv_vcpu_general_set_csr(struct kvm_vcpu *vcpu,
struct kvm_vcpu_csr *csr = &vcpu->arch.guest_csr;
if (reg_num >= sizeof(struct kvm_riscv_csr) / sizeof(unsigned long))
- return -EINVAL;
+ return -ENOENT;
if (reg_num == KVM_REG_RISCV_CSR_REG(sip)) {
reg_val &= VSIP_VALID_MASK;
@@ -374,7 +374,7 @@ static int kvm_riscv_vcpu_get_reg_csr(struct kvm_vcpu *vcpu,
rc = kvm_riscv_vcpu_aia_get_csr(vcpu, reg_num, ®_val);
break;
default:
- rc = -EINVAL;
+ rc = -ENOENT;
break;
}
if (rc)
@@ -413,7 +413,7 @@ static int kvm_riscv_vcpu_set_reg_csr(struct kvm_vcpu *vcpu,
rc = kvm_riscv_vcpu_aia_set_csr(vcpu, reg_num, reg_val);
break;
default:
- rc = -EINVAL;
+ rc = -ENOENT;
break;
}
if (rc)
@@ -430,7 +430,7 @@ static int riscv_vcpu_get_isa_ext_single(struct kvm_vcpu *vcpu,
if (reg_num >= KVM_RISCV_ISA_EXT_MAX ||
reg_num >= ARRAY_SIZE(kvm_isa_ext_arr))
- return -EINVAL;
+ return -ENOENT;
*reg_val = 0;
host_isa_ext = kvm_isa_ext_arr[reg_num];
@@ -448,7 +448,7 @@ static int riscv_vcpu_set_isa_ext_single(struct kvm_vcpu *vcpu,
if (reg_num >= KVM_RISCV_ISA_EXT_MAX ||
reg_num >= ARRAY_SIZE(kvm_isa_ext_arr))
- return -EINVAL;
+ return -ENOENT;
host_isa_ext = kvm_isa_ext_arr[reg_num];
if (!__riscv_isa_extension_available(NULL, host_isa_ext))
@@ -547,7 +547,7 @@ static int kvm_riscv_vcpu_get_reg_isa_ext(struct kvm_vcpu *vcpu,
reg_val = ~reg_val;
break;
default:
- rc = -EINVAL;
+ rc = -ENOENT;
}
if (rc)
return rc;
@@ -585,7 +585,7 @@ static int kvm_riscv_vcpu_set_reg_isa_ext(struct kvm_vcpu *vcpu,
case KVM_REG_RISCV_SBI_MULTI_DIS:
return riscv_vcpu_set_isa_ext_multi(vcpu, reg_num, reg_val, false);
default:
- return -EINVAL;
+ return -ENOENT;
}
return 0;
@@ -652,5 +652,5 @@ int kvm_riscv_vcpu_get_reg(struct kvm_vcpu *vcpu,
break;
}
- return -EINVAL;
+ return -ENOENT;
}
diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c
index 7b46e04fb667..9cd97091c723 100644
--- a/arch/riscv/kvm/vcpu_sbi.c
+++ b/arch/riscv/kvm/vcpu_sbi.c
@@ -140,8 +140,10 @@ static int riscv_vcpu_set_sbi_ext_single(struct kvm_vcpu *vcpu,
const struct kvm_riscv_sbi_extension_entry *sext = NULL;
struct kvm_vcpu_sbi_context *scontext = &vcpu->arch.sbi_context;
- if (reg_num >= KVM_RISCV_SBI_EXT_MAX ||
- (reg_val != 1 && reg_val != 0))
+ if (reg_num >= KVM_RISCV_SBI_EXT_MAX)
+ return -ENOENT;
+
+ if (reg_val != 1 && reg_val != 0)
return -EINVAL;
for (i = 0; i < ARRAY_SIZE(sbi_ext); i++) {
@@ -175,7 +177,7 @@ static int riscv_vcpu_get_sbi_ext_single(struct kvm_vcpu *vcpu,
struct kvm_vcpu_sbi_context *scontext = &vcpu->arch.sbi_context;
if (reg_num >= KVM_RISCV_SBI_EXT_MAX)
- return -EINVAL;
+ return -ENOENT;
for (i = 0; i < ARRAY_SIZE(sbi_ext); i++) {
if (sbi_ext[i].ext_idx == reg_num) {
@@ -206,7 +208,7 @@ static int riscv_vcpu_set_sbi_ext_multi(struct kvm_vcpu *vcpu,
unsigned long i, ext_id;
if (reg_num > KVM_REG_RISCV_SBI_MULTI_REG_LAST)
- return -EINVAL;
+ return -ENOENT;
for_each_set_bit(i, ®_val, BITS_PER_LONG) {
ext_id = i + reg_num * BITS_PER_LONG;
@@ -226,7 +228,7 @@ static int riscv_vcpu_get_sbi_ext_multi(struct kvm_vcpu *vcpu,
unsigned long i, ext_id, ext_val;
if (reg_num > KVM_REG_RISCV_SBI_MULTI_REG_LAST)
- return -EINVAL;
+ return -ENOENT;
for (i = 0; i < BITS_PER_LONG; i++) {
ext_id = i + reg_num * BITS_PER_LONG;
@@ -272,7 +274,7 @@ int kvm_riscv_vcpu_set_reg_sbi_ext(struct kvm_vcpu *vcpu,
case KVM_REG_RISCV_SBI_MULTI_DIS:
return riscv_vcpu_set_sbi_ext_multi(vcpu, reg_num, reg_val, false);
default:
- return -EINVAL;
+ return -ENOENT;
}
return 0;
@@ -307,7 +309,7 @@ int kvm_riscv_vcpu_get_reg_sbi_ext(struct kvm_vcpu *vcpu,
reg_val = ~reg_val;
break;
default:
- rc = -EINVAL;
+ rc = -ENOENT;
}
if (rc)
return rc;
diff --git a/arch/riscv/kvm/vcpu_timer.c b/arch/riscv/kvm/vcpu_timer.c
index 3ac2ff6a65da..527d269cafff 100644
--- a/arch/riscv/kvm/vcpu_timer.c
+++ b/arch/riscv/kvm/vcpu_timer.c
@@ -170,7 +170,7 @@ int kvm_riscv_vcpu_get_reg_timer(struct kvm_vcpu *vcpu,
if (KVM_REG_SIZE(reg->id) != sizeof(u64))
return -EINVAL;
if (reg_num >= sizeof(struct kvm_riscv_timer) / sizeof(u64))
- return -EINVAL;
+ return -ENOENT;
switch (reg_num) {
case KVM_REG_RISCV_TIMER_REG(frequency):
@@ -187,7 +187,7 @@ int kvm_riscv_vcpu_get_reg_timer(struct kvm_vcpu *vcpu,
KVM_RISCV_TIMER_STATE_OFF;
break;
default:
- return -EINVAL;
+ return -ENOENT;
}
if (copy_to_user(uaddr, ®_val, KVM_REG_SIZE(reg->id)))
@@ -211,7 +211,7 @@ int kvm_riscv_vcpu_set_reg_timer(struct kvm_vcpu *vcpu,
if (KVM_REG_SIZE(reg->id) != sizeof(u64))
return -EINVAL;
if (reg_num >= sizeof(struct kvm_riscv_timer) / sizeof(u64))
- return -EINVAL;
+ return -ENOENT;
if (copy_from_user(®_val, uaddr, KVM_REG_SIZE(reg->id)))
return -EFAULT;
@@ -233,7 +233,7 @@ int kvm_riscv_vcpu_set_reg_timer(struct kvm_vcpu *vcpu,
ret = kvm_riscv_vcpu_timer_cancel(t);
break;
default:
- ret = -EINVAL;
+ ret = -ENOENT;
break;
}
--
2.41.0
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 2/9] RISC-V: KVM: use ENOENT in *_one_reg() when extension is unavailable
2023-08-01 22:26 [PATCH v2 0/9] RISC-V: KVM: change get_reg/set_reg error codes Daniel Henrique Barboza
2023-08-01 22:26 ` [PATCH v2 1/9] RISC-V: KVM: return ENOENT in *_one_reg() when reg is unknown Daniel Henrique Barboza
@ 2023-08-01 22:26 ` Daniel Henrique Barboza
2023-08-01 22:26 ` [PATCH v2 3/9] RISC-V: KVM: do not EOPNOTSUPP in set_one_reg() zicbo(m|z) Daniel Henrique Barboza
` (7 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Daniel Henrique Barboza @ 2023-08-01 22:26 UTC (permalink / raw)
To: kvm-riscv, linux-riscv, kvm; +Cc: anup, atishp, ajones, Daniel Henrique Barboza
Following a similar logic as the previous patch let's minimize the EINVAL
usage in *_one_reg() APIs by using ENOENT when an extension that
implements the reg is not available.
For consistency we're also replacing an EOPNOTSUPP instance that should
be an ENOENT since it's an "extension is not available" error.
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
arch/riscv/kvm/vcpu_onereg.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
index ba63522be060..291dba76bac6 100644
--- a/arch/riscv/kvm/vcpu_onereg.c
+++ b/arch/riscv/kvm/vcpu_onereg.c
@@ -135,12 +135,12 @@ static int kvm_riscv_vcpu_get_reg_config(struct kvm_vcpu *vcpu,
break;
case KVM_REG_RISCV_CONFIG_REG(zicbom_block_size):
if (!riscv_isa_extension_available(vcpu->arch.isa, ZICBOM))
- return -EINVAL;
+ return -ENOENT;
reg_val = riscv_cbom_block_size;
break;
case KVM_REG_RISCV_CONFIG_REG(zicboz_block_size):
if (!riscv_isa_extension_available(vcpu->arch.isa, ZICBOZ))
- return -EINVAL;
+ return -ENOENT;
reg_val = riscv_cboz_block_size;
break;
case KVM_REG_RISCV_CONFIG_REG(mvendorid):
@@ -452,7 +452,7 @@ static int riscv_vcpu_set_isa_ext_single(struct kvm_vcpu *vcpu,
host_isa_ext = kvm_isa_ext_arr[reg_num];
if (!__riscv_isa_extension_available(NULL, host_isa_ext))
- return -EOPNOTSUPP;
+ return -ENOENT;
if (!vcpu->arch.ran_atleast_once) {
/*
--
2.41.0
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 3/9] RISC-V: KVM: do not EOPNOTSUPP in set_one_reg() zicbo(m|z)
2023-08-01 22:26 [PATCH v2 0/9] RISC-V: KVM: change get_reg/set_reg error codes Daniel Henrique Barboza
2023-08-01 22:26 ` [PATCH v2 1/9] RISC-V: KVM: return ENOENT in *_one_reg() when reg is unknown Daniel Henrique Barboza
2023-08-01 22:26 ` [PATCH v2 2/9] RISC-V: KVM: use ENOENT in *_one_reg() when extension is unavailable Daniel Henrique Barboza
@ 2023-08-01 22:26 ` Daniel Henrique Barboza
2023-08-01 22:26 ` [PATCH v2 4/9] RISC-V: KVM: do not EOPNOTSUPP in set KVM_REG_RISCV_TIMER_REG Daniel Henrique Barboza
` (6 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Daniel Henrique Barboza @ 2023-08-01 22:26 UTC (permalink / raw)
To: kvm-riscv, linux-riscv, kvm; +Cc: anup, atishp, ajones, Daniel Henrique Barboza
zicbom_block_size and zicboz_block_size have a peculiar API: they can be
read via get_one_reg() but any write will return a EOPNOTSUPP.
It makes sense to return a 'not supported' error since both values can't
be changed, but as far as userspace goes they're regs that are throwing
the same EOPNOTSUPP error even if they were read beforehand via
get_one_reg(), even if the same read value is being written back.
EOPNOTSUPP is also returned even if ZICBOM/ZICBOZ aren't enabled in the
host.
Change both to work more like their counterparts in get_one_reg() and
return -ENOENT if their respective extensions aren't available. After
that, check if the userspace is written a valid value (i.e. the host
value). Throw an -EINVAL if that's not case, let it slide otherwise.
This allows both regs to be read/written by userspace in a 'lazy'
manner, as long as the userspace doesn't change the reg vals.
Suggested-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
arch/riscv/kvm/vcpu_onereg.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
index 291dba76bac6..bd4998c3897b 100644
--- a/arch/riscv/kvm/vcpu_onereg.c
+++ b/arch/riscv/kvm/vcpu_onereg.c
@@ -213,9 +213,17 @@ static int kvm_riscv_vcpu_set_reg_config(struct kvm_vcpu *vcpu,
}
break;
case KVM_REG_RISCV_CONFIG_REG(zicbom_block_size):
- return -EOPNOTSUPP;
+ if (!riscv_isa_extension_available(vcpu->arch.isa, ZICBOM))
+ return -ENOENT;
+ if (reg_val != riscv_cbom_block_size)
+ return -EINVAL;
+ break;
case KVM_REG_RISCV_CONFIG_REG(zicboz_block_size):
- return -EOPNOTSUPP;
+ if (!riscv_isa_extension_available(vcpu->arch.isa, ZICBOZ))
+ return -ENOENT;
+ if (reg_val != riscv_cboz_block_size)
+ return -EINVAL;
+ break;
case KVM_REG_RISCV_CONFIG_REG(mvendorid):
if (!vcpu->arch.ran_atleast_once)
vcpu->arch.mvendorid = reg_val;
--
2.41.0
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 4/9] RISC-V: KVM: do not EOPNOTSUPP in set KVM_REG_RISCV_TIMER_REG
2023-08-01 22:26 [PATCH v2 0/9] RISC-V: KVM: change get_reg/set_reg error codes Daniel Henrique Barboza
` (2 preceding siblings ...)
2023-08-01 22:26 ` [PATCH v2 3/9] RISC-V: KVM: do not EOPNOTSUPP in set_one_reg() zicbo(m|z) Daniel Henrique Barboza
@ 2023-08-01 22:26 ` Daniel Henrique Barboza
2023-08-01 22:26 ` [PATCH v2 5/9] RISC-V: KVM: use EBUSY when !vcpu->arch.ran_atleast_once Daniel Henrique Barboza
` (5 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Daniel Henrique Barboza @ 2023-08-01 22:26 UTC (permalink / raw)
To: kvm-riscv, linux-riscv, kvm; +Cc: anup, atishp, ajones, Daniel Henrique Barboza
The KVM_REG_RISCV_TIMER_REG can be read via get_one_reg(). But trying to
write anything in this reg via set_one_reg() results in an EOPNOTSUPP.
Change the API to behave like cbom_block_size: instead of always
erroring out with EOPNOTSUPP, allow userspace to write the same value
(riscv_timebase) back, throwing an EINVAL if a different value is
attempted.
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
arch/riscv/kvm/vcpu_timer.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/riscv/kvm/vcpu_timer.c b/arch/riscv/kvm/vcpu_timer.c
index 527d269cafff..75486b25ac45 100644
--- a/arch/riscv/kvm/vcpu_timer.c
+++ b/arch/riscv/kvm/vcpu_timer.c
@@ -218,7 +218,8 @@ int kvm_riscv_vcpu_set_reg_timer(struct kvm_vcpu *vcpu,
switch (reg_num) {
case KVM_REG_RISCV_TIMER_REG(frequency):
- ret = -EOPNOTSUPP;
+ if (reg_val != riscv_timebase)
+ return -EINVAL;
break;
case KVM_REG_RISCV_TIMER_REG(time):
gt->time_delta = reg_val - get_cycles64();
--
2.41.0
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 5/9] RISC-V: KVM: use EBUSY when !vcpu->arch.ran_atleast_once
2023-08-01 22:26 [PATCH v2 0/9] RISC-V: KVM: change get_reg/set_reg error codes Daniel Henrique Barboza
` (3 preceding siblings ...)
2023-08-01 22:26 ` [PATCH v2 4/9] RISC-V: KVM: do not EOPNOTSUPP in set KVM_REG_RISCV_TIMER_REG Daniel Henrique Barboza
@ 2023-08-01 22:26 ` Daniel Henrique Barboza
2023-08-01 22:26 ` [PATCH v2 6/9] RISC-V: KVM: avoid EBUSY when writing same ISA val Daniel Henrique Barboza
` (4 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Daniel Henrique Barboza @ 2023-08-01 22:26 UTC (permalink / raw)
To: kvm-riscv, linux-riscv, kvm; +Cc: anup, atishp, ajones, Daniel Henrique Barboza
vcpu_set_reg_config() and vcpu_set_reg_isa_ext() is throwing an
EOPNOTSUPP error when !vcpu->arch.ran_atleast_once. In similar cases
we're throwing an EBUSY error, like in mvendorid/marchid/mimpid
set_reg().
EOPNOTSUPP has a conotation of finality. EBUSY is more adequate in this
case since its a condition/error related to the vcpu lifecycle.
Change these EOPNOTSUPP instances to EBUSY.
Suggested-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
arch/riscv/kvm/vcpu_onereg.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
index bd4998c3897b..67e1e9b0fd7e 100644
--- a/arch/riscv/kvm/vcpu_onereg.c
+++ b/arch/riscv/kvm/vcpu_onereg.c
@@ -209,7 +209,7 @@ static int kvm_riscv_vcpu_set_reg_config(struct kvm_vcpu *vcpu,
vcpu->arch.isa[0] = reg_val;
kvm_riscv_vcpu_fp_reset(vcpu);
} else {
- return -EOPNOTSUPP;
+ return -EBUSY;
}
break;
case KVM_REG_RISCV_CONFIG_REG(zicbom_block_size):
@@ -477,7 +477,7 @@ static int riscv_vcpu_set_isa_ext_single(struct kvm_vcpu *vcpu,
return -EINVAL;
kvm_riscv_vcpu_fp_reset(vcpu);
} else {
- return -EOPNOTSUPP;
+ return -EBUSY;
}
return 0;
--
2.41.0
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 6/9] RISC-V: KVM: avoid EBUSY when writing same ISA val
2023-08-01 22:26 [PATCH v2 0/9] RISC-V: KVM: change get_reg/set_reg error codes Daniel Henrique Barboza
` (4 preceding siblings ...)
2023-08-01 22:26 ` [PATCH v2 5/9] RISC-V: KVM: use EBUSY when !vcpu->arch.ran_atleast_once Daniel Henrique Barboza
@ 2023-08-01 22:26 ` Daniel Henrique Barboza
2023-08-01 22:26 ` [PATCH v2 7/9] RISC-V: KVM: avoid EBUSY when writing the same machine ID val Daniel Henrique Barboza
` (3 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Daniel Henrique Barboza @ 2023-08-01 22:26 UTC (permalink / raw)
To: kvm-riscv, linux-riscv, kvm; +Cc: anup, atishp, ajones, Daniel Henrique Barboza
kvm_riscv_vcpu_set_reg_config() will return -EBUSY if the ISA config reg
is being written after the VCPU ran at least once.
The same restriction isn't placed in kvm_riscv_vcpu_get_reg_config(), so
there's a chance that we'll -EBUSY out on an ISA config reg write even
if the userspace intended no changes to it.
We'll allow the same form of 'lazy writing' that registers such as
zicbom/zicboz_block_size supports: avoid erroring out if userspace made
no changes to the ISA config reg.
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
arch/riscv/kvm/vcpu_onereg.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
index 67e1e9b0fd7e..b0821f75cc61 100644
--- a/arch/riscv/kvm/vcpu_onereg.c
+++ b/arch/riscv/kvm/vcpu_onereg.c
@@ -187,6 +187,13 @@ static int kvm_riscv_vcpu_set_reg_config(struct kvm_vcpu *vcpu,
if (fls(reg_val) >= RISCV_ISA_EXT_BASE)
return -EINVAL;
+ /*
+ * Return early (i.e. do nothing) if reg_val is the same
+ * value retrievable via kvm_riscv_vcpu_get_reg_config().
+ */
+ if (reg_val == (vcpu->arch.isa[0] & KVM_RISCV_BASE_ISA_MASK))
+ break;
+
if (!vcpu->arch.ran_atleast_once) {
/* Ignore the enable/disable request for certain extensions */
for (i = 0; i < RISCV_ISA_EXT_BASE; i++) {
--
2.41.0
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 7/9] RISC-V: KVM: avoid EBUSY when writing the same machine ID val
2023-08-01 22:26 [PATCH v2 0/9] RISC-V: KVM: change get_reg/set_reg error codes Daniel Henrique Barboza
` (5 preceding siblings ...)
2023-08-01 22:26 ` [PATCH v2 6/9] RISC-V: KVM: avoid EBUSY when writing same ISA val Daniel Henrique Barboza
@ 2023-08-01 22:26 ` Daniel Henrique Barboza
2023-08-01 22:26 ` [PATCH v2 8/9] RISC-V: KVM: avoid EBUSY when writing the same isa_ext val Daniel Henrique Barboza
` (2 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Daniel Henrique Barboza @ 2023-08-01 22:26 UTC (permalink / raw)
To: kvm-riscv, linux-riscv, kvm; +Cc: anup, atishp, ajones, Daniel Henrique Barboza
Right now we do not allow any write in mvendorid/marchid/mimpid if the
vcpu already started, preventing these regs to be changed.
However, if userspace doesn't change them, an alternative is to consider
the reg write a no-op and avoid erroring out altogether. Userpace can
then be oblivious about KVM internals if no changes were intended in the
first place.
Allow the same form of 'lazy writing' that registers such as
zicbom/zicboz_block_size supports: avoid erroring out if userspace makes
no changes in mvendorid/marchid/mimpid during reg write.
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
arch/riscv/kvm/vcpu_onereg.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
index b0821f75cc61..1ceccc93ccdb 100644
--- a/arch/riscv/kvm/vcpu_onereg.c
+++ b/arch/riscv/kvm/vcpu_onereg.c
@@ -232,18 +232,24 @@ static int kvm_riscv_vcpu_set_reg_config(struct kvm_vcpu *vcpu,
return -EINVAL;
break;
case KVM_REG_RISCV_CONFIG_REG(mvendorid):
+ if (reg_val == vcpu->arch.mvendorid)
+ break;
if (!vcpu->arch.ran_atleast_once)
vcpu->arch.mvendorid = reg_val;
else
return -EBUSY;
break;
case KVM_REG_RISCV_CONFIG_REG(marchid):
+ if (reg_val == vcpu->arch.marchid)
+ break;
if (!vcpu->arch.ran_atleast_once)
vcpu->arch.marchid = reg_val;
else
return -EBUSY;
break;
case KVM_REG_RISCV_CONFIG_REG(mimpid):
+ if (reg_val == vcpu->arch.mimpid)
+ break;
if (!vcpu->arch.ran_atleast_once)
vcpu->arch.mimpid = reg_val;
else
--
2.41.0
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 8/9] RISC-V: KVM: avoid EBUSY when writing the same isa_ext val
2023-08-01 22:26 [PATCH v2 0/9] RISC-V: KVM: change get_reg/set_reg error codes Daniel Henrique Barboza
` (6 preceding siblings ...)
2023-08-01 22:26 ` [PATCH v2 7/9] RISC-V: KVM: avoid EBUSY when writing the same machine ID val Daniel Henrique Barboza
@ 2023-08-01 22:26 ` Daniel Henrique Barboza
2023-08-01 22:26 ` [PATCH v2 9/9] docs: kvm: riscv: document EBUSY in KVM_SET_ONE_REG Daniel Henrique Barboza
2023-08-02 9:04 ` [PATCH v2 0/9] RISC-V: KVM: change get_reg/set_reg error codes Andrew Jones
9 siblings, 0 replies; 13+ messages in thread
From: Daniel Henrique Barboza @ 2023-08-01 22:26 UTC (permalink / raw)
To: kvm-riscv, linux-riscv, kvm; +Cc: anup, atishp, ajones, Daniel Henrique Barboza
riscv_vcpu_set_isa_ext_single() will prevent any write of isa_ext regs
if the vcpu already started spinning.
But if there's no extension state (enabled/disabled) made by the
userspace, there's no need to -EBUSY out - we can treat the operation as
a no-op.
zicbom/zicboz_block_size, ISA config reg and mvendorid/march/mimpid
already works in a more permissive manner w.r.t userspace writes being a
no-op, so let's do the same with isa_ext writes.
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
arch/riscv/kvm/vcpu_onereg.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
index 1ceccc93ccdb..c88b0c7f7f01 100644
--- a/arch/riscv/kvm/vcpu_onereg.c
+++ b/arch/riscv/kvm/vcpu_onereg.c
@@ -475,6 +475,9 @@ static int riscv_vcpu_set_isa_ext_single(struct kvm_vcpu *vcpu,
if (!__riscv_isa_extension_available(NULL, host_isa_ext))
return -ENOENT;
+ if (reg_val == test_bit(host_isa_ext, vcpu->arch.isa))
+ return 0;
+
if (!vcpu->arch.ran_atleast_once) {
/*
* All multi-letter extension and a few single letter
--
2.41.0
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 9/9] docs: kvm: riscv: document EBUSY in KVM_SET_ONE_REG
2023-08-01 22:26 [PATCH v2 0/9] RISC-V: KVM: change get_reg/set_reg error codes Daniel Henrique Barboza
` (7 preceding siblings ...)
2023-08-01 22:26 ` [PATCH v2 8/9] RISC-V: KVM: avoid EBUSY when writing the same isa_ext val Daniel Henrique Barboza
@ 2023-08-01 22:26 ` Daniel Henrique Barboza
2023-08-02 8:17 ` Andrew Jones
2023-08-02 9:04 ` [PATCH v2 0/9] RISC-V: KVM: change get_reg/set_reg error codes Andrew Jones
9 siblings, 1 reply; 13+ messages in thread
From: Daniel Henrique Barboza @ 2023-08-01 22:26 UTC (permalink / raw)
To: kvm-riscv, linux-riscv, kvm; +Cc: anup, atishp, ajones, Daniel Henrique Barboza
The EBUSY errno is being used for KVM_SET_ONE_REG as a way to tell
userspace that a given reg can't be written after the vcpu started.
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
Documentation/virt/kvm/api.rst | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index c0ddd3035462..229e7cc091c8 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -2259,6 +2259,8 @@ Errors:
EINVAL invalid register ID, or no such register or used with VMs in
protected virtualization mode on s390
EPERM (arm64) register access not allowed before vcpu finalization
+ EBUSY (riscv) register access not allowed after the vcpu has run
+ at least once
====== ============================================================
(These error codes are indicative only: do not rely on a specific error
--
2.41.0
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 9/9] docs: kvm: riscv: document EBUSY in KVM_SET_ONE_REG
2023-08-01 22:26 ` [PATCH v2 9/9] docs: kvm: riscv: document EBUSY in KVM_SET_ONE_REG Daniel Henrique Barboza
@ 2023-08-02 8:17 ` Andrew Jones
0 siblings, 0 replies; 13+ messages in thread
From: Andrew Jones @ 2023-08-02 8:17 UTC (permalink / raw)
To: Daniel Henrique Barboza; +Cc: kvm-riscv, linux-riscv, kvm, anup, atishp
On Tue, Aug 01, 2023 at 07:26:29PM -0300, Daniel Henrique Barboza wrote:
> The EBUSY errno is being used for KVM_SET_ONE_REG as a way to tell
> userspace that a given reg can't be written after the vcpu started.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
> Documentation/virt/kvm/api.rst | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index c0ddd3035462..229e7cc091c8 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -2259,6 +2259,8 @@ Errors:
> EINVAL invalid register ID, or no such register or used with VMs in
> protected virtualization mode on s390
> EPERM (arm64) register access not allowed before vcpu finalization
> + EBUSY (riscv) register access not allowed after the vcpu has run
> + at least once
We allow access (reading, even before, and now also writing when the value
is the same), so this should be worded in a way that conveys the register
may not be changed after the vcpu has run once.
Thanks,
drew
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/9] RISC-V: KVM: change get_reg/set_reg error codes
2023-08-01 22:26 [PATCH v2 0/9] RISC-V: KVM: change get_reg/set_reg error codes Daniel Henrique Barboza
` (8 preceding siblings ...)
2023-08-01 22:26 ` [PATCH v2 9/9] docs: kvm: riscv: document EBUSY in KVM_SET_ONE_REG Daniel Henrique Barboza
@ 2023-08-02 9:04 ` Andrew Jones
2023-08-02 9:06 ` Andrew Jones
9 siblings, 1 reply; 13+ messages in thread
From: Andrew Jones @ 2023-08-02 9:04 UTC (permalink / raw)
To: Daniel Henrique Barboza; +Cc: kvm-riscv, linux-riscv, kvm, anup, atishp
[-- Attachment #1: Type: text/plain, Size: 2515 bytes --]
On Tue, Aug 01, 2023 at 07:26:20PM -0300, Daniel Henrique Barboza wrote:
> Hi,
>
> In this new version 3 new patches (6, 7, 8) were added by Andrew's
> request during the v1 review.
>
> We're now avoiding throwing an -EBUSY error if a reg write is done after
> the vcpu started spinning if the value being written is the same as KVM
> already uses. This follows the design choice made in patch 3, allowing
> for userspace 'lazy write' of registers.
>
> I decided to add 3 patches instead of one because the no-op check made
> in patches 6 and 8 aren't just a matter of doing reg_val = host_val.
> They can be squashed in a single patch if required.
>
> Please check the version 1 cover-letter [1] for the motivation behind
> this work. Patches were based on top of riscv_kvm_queue.
>
> Changes from v1:
> - patches 6,7, 8 (new):
> - make reg writes a no-op, regardless of vcpu->arch.ran_atleast_once
> state, if the value being written is the same as the host
> - v1 link: https://lore.kernel.org/kvm/20230731120420.91007-1-dbarboza@ventanamicro.com/
>
> [1] https://lore.kernel.org/kvm/20230731120420.91007-1-dbarboza@ventanamicro.com/
>
I found three missing conversions, which are in the diff below. Also, I
saw that the vector registers were lacking good error returns, so I reworked
that and attached a completely (not even compile) tested patch for them.
Please work that patch into this series.
Thanks,
drew
diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
index c88b0c7f7f01..6ca90c04ba61 100644
--- a/arch/riscv/kvm/vcpu_onereg.c
+++ b/arch/riscv/kvm/vcpu_onereg.c
@@ -506,7 +506,7 @@ static int riscv_vcpu_get_isa_ext_multi(struct kvm_vcpu *vcpu,
unsigned long i, ext_id, ext_val;
if (reg_num > KVM_REG_RISCV_ISA_MULTI_REG_LAST)
- return -EINVAL;
+ return -ENOENT;
for (i = 0; i < BITS_PER_LONG; i++) {
ext_id = i + reg_num * BITS_PER_LONG;
@@ -529,7 +529,7 @@ static int riscv_vcpu_set_isa_ext_multi(struct kvm_vcpu *vcpu,
unsigned long i, ext_id;
if (reg_num > KVM_REG_RISCV_ISA_MULTI_REG_LAST)
- return -EINVAL;
+ return -ENOENT;
for_each_set_bit(i, ®_val, BITS_PER_LONG) {
ext_id = i + reg_num * BITS_PER_LONG;
@@ -644,7 +644,7 @@ int kvm_riscv_vcpu_set_reg(struct kvm_vcpu *vcpu,
break;
}
- return -EINVAL;
+ return -ENOENT;
}
int kvm_riscv_vcpu_get_reg(struct kvm_vcpu *vcpu,
[-- Attachment #2: vec-errno.patch --]
[-- Type: text/plain, Size: 3632 bytes --]
From b1472501677ea6bd73689e0f947149f5f86da9cc Mon Sep 17 00:00:00 2001
From: Andrew Jones <ajones@ventanamicro.com>
Date: Wed, 2 Aug 2023 11:52:04 +0300
Subject: [PATCH] riscv: KVM: Improve vector save/restore errors
Content-type: text/plain
---
arch/riscv/kvm/vcpu_vector.c | 60 ++++++++++++++++++++----------------
1 file changed, 33 insertions(+), 27 deletions(-)
diff --git a/arch/riscv/kvm/vcpu_vector.c b/arch/riscv/kvm/vcpu_vector.c
index edd2eecbddc2..39c5bceb4d1b 100644
--- a/arch/riscv/kvm/vcpu_vector.c
+++ b/arch/riscv/kvm/vcpu_vector.c
@@ -91,44 +91,44 @@ void kvm_riscv_vcpu_free_vector_context(struct kvm_vcpu *vcpu)
}
#endif
-static void *kvm_riscv_vcpu_vreg_addr(struct kvm_vcpu *vcpu,
+static int kvm_riscv_vcpu_vreg_addr(struct kvm_vcpu *vcpu,
unsigned long reg_num,
- size_t reg_size)
+ size_t reg_size,
+ void **reg_val)
{
struct kvm_cpu_context *cntx = &vcpu->arch.guest_context;
- void *reg_val;
size_t vlenb = riscv_v_vsize / 32;
if (reg_num < KVM_REG_RISCV_VECTOR_REG(0)) {
if (reg_size != sizeof(unsigned long))
- return NULL;
+ return -EINVAL;
switch (reg_num) {
case KVM_REG_RISCV_VECTOR_CSR_REG(vstart):
- reg_val = &cntx->vector.vstart;
+ *reg_val = &cntx->vector.vstart;
break;
case KVM_REG_RISCV_VECTOR_CSR_REG(vl):
- reg_val = &cntx->vector.vl;
+ *reg_val = &cntx->vector.vl;
break;
case KVM_REG_RISCV_VECTOR_CSR_REG(vtype):
- reg_val = &cntx->vector.vtype;
+ *reg_val = &cntx->vector.vtype;
break;
case KVM_REG_RISCV_VECTOR_CSR_REG(vcsr):
- reg_val = &cntx->vector.vcsr;
+ *reg_val = &cntx->vector.vcsr;
break;
case KVM_REG_RISCV_VECTOR_CSR_REG(datap):
default:
- return NULL;
+ return -ENOENT;
}
} else if (reg_num <= KVM_REG_RISCV_VECTOR_REG(31)) {
if (reg_size != vlenb)
- return NULL;
- reg_val = cntx->vector.datap
+ return -EINVAL;
+ *reg_val = cntx->vector.datap
+ (reg_num - KVM_REG_RISCV_VECTOR_REG(0)) * vlenb;
} else {
- return NULL;
+ return -ENOENT;
}
- return reg_val;
+ return 0;
}
int kvm_riscv_vcpu_get_reg_vector(struct kvm_vcpu *vcpu,
@@ -141,17 +141,20 @@ int kvm_riscv_vcpu_get_reg_vector(struct kvm_vcpu *vcpu,
unsigned long reg_num = reg->id & ~(KVM_REG_ARCH_MASK |
KVM_REG_SIZE_MASK |
rtype);
- void *reg_val = NULL;
size_t reg_size = KVM_REG_SIZE(reg->id);
+ void *reg_val;
+ int rc;
- if (rtype == KVM_REG_RISCV_VECTOR &&
- riscv_isa_extension_available(isa, v)) {
- reg_val = kvm_riscv_vcpu_vreg_addr(vcpu, reg_num, reg_size);
- }
-
- if (!reg_val)
+ if (rtype != KVM_REG_RISCV_VECTOR)
return -EINVAL;
+ if (!riscv_isa_extension_available(isa, v))
+ return -ENOENT;
+
+ rc = kvm_riscv_vcpu_vreg_addr(vcpu, reg_num, reg_size, ®_val);
+ if (rc)
+ return rc;
+
if (copy_to_user(uaddr, reg_val, reg_size))
return -EFAULT;
@@ -168,17 +171,20 @@ int kvm_riscv_vcpu_set_reg_vector(struct kvm_vcpu *vcpu,
unsigned long reg_num = reg->id & ~(KVM_REG_ARCH_MASK |
KVM_REG_SIZE_MASK |
rtype);
- void *reg_val = NULL;
size_t reg_size = KVM_REG_SIZE(reg->id);
+ void *reg_val;
+ int rc;
- if (rtype == KVM_REG_RISCV_VECTOR &&
- riscv_isa_extension_available(isa, v)) {
- reg_val = kvm_riscv_vcpu_vreg_addr(vcpu, reg_num, reg_size);
- }
-
- if (!reg_val)
+ if (rtype != KVM_REG_RISCV_VECTOR)
return -EINVAL;
+ if (!riscv_isa_extension_available(isa, v))
+ return -ENOENT;
+
+ rc = kvm_riscv_vcpu_vreg_addr(vcpu, reg_num, reg_size, ®_val);
+ if (rc)
+ return rc;
+
if (copy_from_user(reg_val, uaddr, reg_size))
return -EFAULT;
--
2.41.0
[-- Attachment #3: Type: text/plain, Size: 161 bytes --]
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 0/9] RISC-V: KVM: change get_reg/set_reg error codes
2023-08-02 9:04 ` [PATCH v2 0/9] RISC-V: KVM: change get_reg/set_reg error codes Andrew Jones
@ 2023-08-02 9:06 ` Andrew Jones
0 siblings, 0 replies; 13+ messages in thread
From: Andrew Jones @ 2023-08-02 9:06 UTC (permalink / raw)
To: Daniel Henrique Barboza; +Cc: kvm-riscv, linux-riscv, kvm, anup, atishp
On Wed, Aug 02, 2023 at 12:04:25PM +0300, Andrew Jones wrote:
> On Tue, Aug 01, 2023 at 07:26:20PM -0300, Daniel Henrique Barboza wrote:
> > Hi,
> >
> > In this new version 3 new patches (6, 7, 8) were added by Andrew's
> > request during the v1 review.
> >
> > We're now avoiding throwing an -EBUSY error if a reg write is done after
> > the vcpu started spinning if the value being written is the same as KVM
> > already uses. This follows the design choice made in patch 3, allowing
> > for userspace 'lazy write' of registers.
> >
> > I decided to add 3 patches instead of one because the no-op check made
> > in patches 6 and 8 aren't just a matter of doing reg_val = host_val.
> > They can be squashed in a single patch if required.
> >
> > Please check the version 1 cover-letter [1] for the motivation behind
> > this work. Patches were based on top of riscv_kvm_queue.
> >
> > Changes from v1:
> > - patches 6,7, 8 (new):
> > - make reg writes a no-op, regardless of vcpu->arch.ran_atleast_once
> > state, if the value being written is the same as the host
> > - v1 link: https://lore.kernel.org/kvm/20230731120420.91007-1-dbarboza@ventanamicro.com/
> >
> > [1] https://lore.kernel.org/kvm/20230731120420.91007-1-dbarboza@ventanamicro.com/
> >
>
> I found three missing conversions, which are in the diff below. Also, I
> saw that the vector registers were lacking good error returns, so I reworked
> that and attached a completely (not even compile) tested patch for them.
^un
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 13+ messages in thread