* [PATCH v2 0/3] RISC-V: KVM: Ensure SBI extension is enabled
@ 2023-05-26 10:25 Andrew Jones
2023-05-26 10:25 ` [PATCH v2 1/3] RISC-V: KVM: Rename dis_idx to ext_idx Andrew Jones
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Andrew Jones @ 2023-05-26 10:25 UTC (permalink / raw)
To: linux-riscv, kvm-riscv
Cc: 'Palmer Dabbelt ', 'Atish Patra ',
'Albert Ou ', 'Paul Walmsley ',
'Anup Patel '
Ensure guests can't attempt to invoke SBI extension functions when the
SBI extension's probe function has stated that the extension is not
available.
v2:
- Reworked it to use an enum array instead of two boolean arrays
Andrew Jones (3):
RISC-V: KVM: Rename dis_idx to ext_idx
RISC-V: KVM: Convert extension_disabled[] to ext_status[]
RISC-V: KVM: Probe for SBI extension status
arch/riscv/include/asm/kvm_vcpu_sbi.h | 8 ++-
arch/riscv/kvm/vcpu_sbi.c | 76 +++++++++++++++++++--------
2 files changed, 60 insertions(+), 24 deletions(-)
--
2.40.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/3] RISC-V: KVM: Rename dis_idx to ext_idx
2023-05-26 10:25 [PATCH v2 0/3] RISC-V: KVM: Ensure SBI extension is enabled Andrew Jones
@ 2023-05-26 10:25 ` Andrew Jones
2023-05-26 10:25 ` [PATCH v2 2/3] RISC-V: KVM: Convert extension_disabled[] to ext_status[] Andrew Jones
2023-05-26 10:25 ` [PATCH v2 3/3] RISC-V: KVM: Probe for SBI extension status Andrew Jones
2 siblings, 0 replies; 7+ messages in thread
From: Andrew Jones @ 2023-05-26 10:25 UTC (permalink / raw)
To: linux-riscv, kvm-riscv
Cc: 'Palmer Dabbelt ', 'Atish Patra ',
'Albert Ou ', 'Paul Walmsley ',
'Anup Patel '
Make the name of the extension_disabled[] index more general in
order to expand its application.
Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
---
arch/riscv/kvm/vcpu_sbi.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c
index e52fde504433..6aa15f1b97d9 100644
--- a/arch/riscv/kvm/vcpu_sbi.c
+++ b/arch/riscv/kvm/vcpu_sbi.c
@@ -31,49 +31,49 @@ static const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_pmu = {
#endif
struct kvm_riscv_sbi_extension_entry {
- enum KVM_RISCV_SBI_EXT_ID dis_idx;
+ enum KVM_RISCV_SBI_EXT_ID ext_idx;
const struct kvm_vcpu_sbi_extension *ext_ptr;
};
static const struct kvm_riscv_sbi_extension_entry sbi_ext[] = {
{
- .dis_idx = KVM_RISCV_SBI_EXT_V01,
+ .ext_idx = KVM_RISCV_SBI_EXT_V01,
.ext_ptr = &vcpu_sbi_ext_v01,
},
{
- .dis_idx = KVM_RISCV_SBI_EXT_MAX, /* Can't be disabled */
+ .ext_idx = KVM_RISCV_SBI_EXT_MAX, /* Can't be disabled */
.ext_ptr = &vcpu_sbi_ext_base,
},
{
- .dis_idx = KVM_RISCV_SBI_EXT_TIME,
+ .ext_idx = KVM_RISCV_SBI_EXT_TIME,
.ext_ptr = &vcpu_sbi_ext_time,
},
{
- .dis_idx = KVM_RISCV_SBI_EXT_IPI,
+ .ext_idx = KVM_RISCV_SBI_EXT_IPI,
.ext_ptr = &vcpu_sbi_ext_ipi,
},
{
- .dis_idx = KVM_RISCV_SBI_EXT_RFENCE,
+ .ext_idx = KVM_RISCV_SBI_EXT_RFENCE,
.ext_ptr = &vcpu_sbi_ext_rfence,
},
{
- .dis_idx = KVM_RISCV_SBI_EXT_SRST,
+ .ext_idx = KVM_RISCV_SBI_EXT_SRST,
.ext_ptr = &vcpu_sbi_ext_srst,
},
{
- .dis_idx = KVM_RISCV_SBI_EXT_HSM,
+ .ext_idx = KVM_RISCV_SBI_EXT_HSM,
.ext_ptr = &vcpu_sbi_ext_hsm,
},
{
- .dis_idx = KVM_RISCV_SBI_EXT_PMU,
+ .ext_idx = KVM_RISCV_SBI_EXT_PMU,
.ext_ptr = &vcpu_sbi_ext_pmu,
},
{
- .dis_idx = KVM_RISCV_SBI_EXT_EXPERIMENTAL,
+ .ext_idx = KVM_RISCV_SBI_EXT_EXPERIMENTAL,
.ext_ptr = &vcpu_sbi_ext_experimental,
},
{
- .dis_idx = KVM_RISCV_SBI_EXT_VENDOR,
+ .ext_idx = KVM_RISCV_SBI_EXT_VENDOR,
.ext_ptr = &vcpu_sbi_ext_vendor,
},
};
@@ -147,7 +147,7 @@ static int riscv_vcpu_set_sbi_ext_single(struct kvm_vcpu *vcpu,
return -EINVAL;
for (i = 0; i < ARRAY_SIZE(sbi_ext); i++) {
- if (sbi_ext[i].dis_idx == reg_num) {
+ if (sbi_ext[i].ext_idx == reg_num) {
sext = &sbi_ext[i];
break;
}
@@ -155,7 +155,7 @@ static int riscv_vcpu_set_sbi_ext_single(struct kvm_vcpu *vcpu,
if (!sext)
return -ENOENT;
- scontext->extension_disabled[sext->dis_idx] = !reg_val;
+ scontext->extension_disabled[sext->ext_idx] = !reg_val;
return 0;
}
@@ -172,7 +172,7 @@ static int riscv_vcpu_get_sbi_ext_single(struct kvm_vcpu *vcpu,
return -EINVAL;
for (i = 0; i < ARRAY_SIZE(sbi_ext); i++) {
- if (sbi_ext[i].dis_idx == reg_num) {
+ if (sbi_ext[i].ext_idx == reg_num) {
sext = &sbi_ext[i];
break;
}
@@ -180,7 +180,7 @@ static int riscv_vcpu_get_sbi_ext_single(struct kvm_vcpu *vcpu,
if (!sext)
return -ENOENT;
- *reg_val = !scontext->extension_disabled[sext->dis_idx];
+ *reg_val = !scontext->extension_disabled[sext->ext_idx];
return 0;
}
@@ -315,8 +315,8 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext(
sext = &sbi_ext[i];
if (sext->ext_ptr->extid_start <= extid &&
sext->ext_ptr->extid_end >= extid) {
- if (sext->dis_idx < KVM_RISCV_SBI_EXT_MAX &&
- scontext->extension_disabled[sext->dis_idx])
+ if (sext->ext_idx < KVM_RISCV_SBI_EXT_MAX &&
+ scontext->extension_disabled[sext->ext_idx])
return NULL;
return sbi_ext[i].ext_ptr;
}
--
2.40.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/3] RISC-V: KVM: Convert extension_disabled[] to ext_status[]
2023-05-26 10:25 [PATCH v2 0/3] RISC-V: KVM: Ensure SBI extension is enabled Andrew Jones
2023-05-26 10:25 ` [PATCH v2 1/3] RISC-V: KVM: Rename dis_idx to ext_idx Andrew Jones
@ 2023-05-26 10:25 ` Andrew Jones
2023-05-30 16:58 ` Anup Patel
2023-05-26 10:25 ` [PATCH v2 3/3] RISC-V: KVM: Probe for SBI extension status Andrew Jones
2 siblings, 1 reply; 7+ messages in thread
From: Andrew Jones @ 2023-05-26 10:25 UTC (permalink / raw)
To: linux-riscv, kvm-riscv
Cc: 'Palmer Dabbelt ', 'Atish Patra ',
'Albert Ou ', 'Paul Walmsley ',
'Anup Patel '
Change the boolean extension_disabled[] array to an array of enums,
ext_status[]. For now, the enum only has two states, which correspond
to the previous boolean states, so this patch has no intended
functional change. The next patch will add another state, expanding
the purpose of ext_status[].
Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
arch/riscv/include/asm/kvm_vcpu_sbi.h | 7 ++++++-
arch/riscv/kvm/vcpu_sbi.c | 9 ++++++---
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h
index 4278125a38a5..cda99fc3d897 100644
--- a/arch/riscv/include/asm/kvm_vcpu_sbi.h
+++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h
@@ -14,9 +14,14 @@
#define KVM_SBI_VERSION_MAJOR 1
#define KVM_SBI_VERSION_MINOR 0
+enum KVM_RISCV_SBI_EXT_STATUS {
+ KVM_RISCV_SBI_EXT_AVAILABLE,
+ KVM_RISCV_SBI_EXT_UNAVAILABLE,
+};
+
struct kvm_vcpu_sbi_context {
int return_handled;
- bool extension_disabled[KVM_RISCV_SBI_EXT_MAX];
+ enum KVM_RISCV_SBI_EXT_STATUS ext_status[KVM_RISCV_SBI_EXT_MAX];
};
struct kvm_vcpu_sbi_return {
diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c
index 6aa15f1b97d9..28e55ba023dc 100644
--- a/arch/riscv/kvm/vcpu_sbi.c
+++ b/arch/riscv/kvm/vcpu_sbi.c
@@ -155,7 +155,8 @@ static int riscv_vcpu_set_sbi_ext_single(struct kvm_vcpu *vcpu,
if (!sext)
return -ENOENT;
- scontext->extension_disabled[sext->ext_idx] = !reg_val;
+ scontext->ext_status[sext->ext_idx] = reg_val ?
+ KVM_RISCV_SBI_EXT_AVAILABLE : KVM_RISCV_SBI_EXT_UNAVAILABLE;
return 0;
}
@@ -180,7 +181,8 @@ static int riscv_vcpu_get_sbi_ext_single(struct kvm_vcpu *vcpu,
if (!sext)
return -ENOENT;
- *reg_val = !scontext->extension_disabled[sext->ext_idx];
+ *reg_val = scontext->ext_status[sext->ext_idx] ==
+ KVM_RISCV_SBI_EXT_AVAILABLE;
return 0;
}
@@ -316,7 +318,8 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext(
if (sext->ext_ptr->extid_start <= extid &&
sext->ext_ptr->extid_end >= extid) {
if (sext->ext_idx < KVM_RISCV_SBI_EXT_MAX &&
- scontext->extension_disabled[sext->ext_idx])
+ scontext->ext_status[sext->ext_idx] ==
+ KVM_RISCV_SBI_EXT_UNAVAILABLE)
return NULL;
return sbi_ext[i].ext_ptr;
}
--
2.40.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 3/3] RISC-V: KVM: Probe for SBI extension status
2023-05-26 10:25 [PATCH v2 0/3] RISC-V: KVM: Ensure SBI extension is enabled Andrew Jones
2023-05-26 10:25 ` [PATCH v2 1/3] RISC-V: KVM: Rename dis_idx to ext_idx Andrew Jones
2023-05-26 10:25 ` [PATCH v2 2/3] RISC-V: KVM: Convert extension_disabled[] to ext_status[] Andrew Jones
@ 2023-05-26 10:25 ` Andrew Jones
2023-05-30 17:05 ` Anup Patel
2 siblings, 1 reply; 7+ messages in thread
From: Andrew Jones @ 2023-05-26 10:25 UTC (permalink / raw)
To: linux-riscv, kvm-riscv
Cc: 'Palmer Dabbelt ', 'Atish Patra ',
'Albert Ou ', 'Paul Walmsley ',
'Anup Patel '
Rather than defaulting the status to available and allowing the user
to set availability, default to uninitialized and only allow the user
to set the status to unavailable. Then, when an extension is first
used, ensure it is available by invoking its probe function, if it
has one (an extension is assumed available if it doesn't have a probe
function). Checking the status in kvm_vcpu_sbi_find_ext() ensures
extension functions cannot be invoked when they're unavailable.
Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
arch/riscv/include/asm/kvm_vcpu_sbi.h | 1 +
arch/riscv/kvm/vcpu_sbi.c | 51 ++++++++++++++++++++-------
2 files changed, 40 insertions(+), 12 deletions(-)
diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h
index cda99fc3d897..a55b6225aa55 100644
--- a/arch/riscv/include/asm/kvm_vcpu_sbi.h
+++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h
@@ -15,6 +15,7 @@
#define KVM_SBI_VERSION_MINOR 0
enum KVM_RISCV_SBI_EXT_STATUS {
+ KVM_RISCV_SBI_EXT_UNINITIALIZED,
KVM_RISCV_SBI_EXT_AVAILABLE,
KVM_RISCV_SBI_EXT_UNAVAILABLE,
};
diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c
index 28e55ba023dc..b3e92c11e54f 100644
--- a/arch/riscv/kvm/vcpu_sbi.c
+++ b/arch/riscv/kvm/vcpu_sbi.c
@@ -155,8 +155,15 @@ static int riscv_vcpu_set_sbi_ext_single(struct kvm_vcpu *vcpu,
if (!sext)
return -ENOENT;
- scontext->ext_status[sext->ext_idx] = reg_val ?
- KVM_RISCV_SBI_EXT_AVAILABLE : KVM_RISCV_SBI_EXT_UNAVAILABLE;
+ /*
+ * We can't set the extension status to available here, since it may
+ * have a probe() function which needs to confirm availability first,
+ * but it may be too early to call that here. We can set the status to
+ * unavailable, though.
+ */
+ if (!reg_val)
+ scontext->ext_status[sext->ext_idx] =
+ KVM_RISCV_SBI_EXT_UNAVAILABLE;
return 0;
}
@@ -181,8 +188,15 @@ static int riscv_vcpu_get_sbi_ext_single(struct kvm_vcpu *vcpu,
if (!sext)
return -ENOENT;
- *reg_val = scontext->ext_status[sext->ext_idx] ==
- KVM_RISCV_SBI_EXT_AVAILABLE;
+ /*
+ * If the extension status is still uninitialized, then we should probe
+ * to determine if it's available, but it may be too early to do that
+ * here. The best we can do is report that the extension has not been
+ * disabled, i.e. we return 1 when the extension is available and also
+ * when it only may be available.
+ */
+ *reg_val = scontext->ext_status[sext->ext_idx] !=
+ KVM_RISCV_SBI_EXT_UNAVAILABLE;
return 0;
}
@@ -309,19 +323,32 @@ int kvm_riscv_vcpu_get_reg_sbi_ext(struct kvm_vcpu *vcpu,
const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext(
struct kvm_vcpu *vcpu, unsigned long extid)
{
- int i;
- const struct kvm_riscv_sbi_extension_entry *sext;
struct kvm_vcpu_sbi_context *scontext = &vcpu->arch.sbi_context;
+ const struct kvm_riscv_sbi_extension_entry *entry;
+ const struct kvm_vcpu_sbi_extension *ext;
+ int i;
for (i = 0; i < ARRAY_SIZE(sbi_ext); i++) {
- sext = &sbi_ext[i];
- if (sext->ext_ptr->extid_start <= extid &&
- sext->ext_ptr->extid_end >= extid) {
- if (sext->ext_idx < KVM_RISCV_SBI_EXT_MAX &&
- scontext->ext_status[sext->ext_idx] ==
+ entry = &sbi_ext[i];
+ ext = entry->ext_ptr;
+
+ if (ext->extid_start <= extid && ext->extid_end >= extid) {
+ if (entry->ext_idx >= KVM_RISCV_SBI_EXT_MAX ||
+ scontext->ext_status[entry->ext_idx] ==
+ KVM_RISCV_SBI_EXT_AVAILABLE)
+ return ext;
+ if (scontext->ext_status[entry->ext_idx] ==
KVM_RISCV_SBI_EXT_UNAVAILABLE)
return NULL;
- return sbi_ext[i].ext_ptr;
+ if (ext->probe && !ext->probe(vcpu)) {
+ scontext->ext_status[entry->ext_idx] =
+ KVM_RISCV_SBI_EXT_UNAVAILABLE;
+ return NULL;
+ }
+
+ scontext->ext_status[entry->ext_idx] =
+ KVM_RISCV_SBI_EXT_AVAILABLE;
+ return ext;
}
}
--
2.40.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/3] RISC-V: KVM: Convert extension_disabled[] to ext_status[]
2023-05-26 10:25 ` [PATCH v2 2/3] RISC-V: KVM: Convert extension_disabled[] to ext_status[] Andrew Jones
@ 2023-05-30 16:58 ` Anup Patel
2023-05-30 17:38 ` Andrew Jones
0 siblings, 1 reply; 7+ messages in thread
From: Anup Patel @ 2023-05-30 16:58 UTC (permalink / raw)
To: Andrew Jones
Cc: linux-riscv, kvm-riscv, Palmer Dabbelt, Atish Patra, Albert Ou,
Paul Walmsley
On Fri, May 26, 2023 at 3:55 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> Change the boolean extension_disabled[] array to an array of enums,
> ext_status[]. For now, the enum only has two states, which correspond
> to the previous boolean states, so this patch has no intended
> functional change. The next patch will add another state, expanding
> the purpose of ext_status[].
>
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---
> arch/riscv/include/asm/kvm_vcpu_sbi.h | 7 ++++++-
> arch/riscv/kvm/vcpu_sbi.c | 9 ++++++---
> 2 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h
> index 4278125a38a5..cda99fc3d897 100644
> --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h
> +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h
> @@ -14,9 +14,14 @@
> #define KVM_SBI_VERSION_MAJOR 1
> #define KVM_SBI_VERSION_MINOR 0
>
> +enum KVM_RISCV_SBI_EXT_STATUS {
This upper-case name of enum looks odd.
> + KVM_RISCV_SBI_EXT_AVAILABLE,
> + KVM_RISCV_SBI_EXT_UNAVAILABLE,
> +};
> +
> struct kvm_vcpu_sbi_context {
> int return_handled;
> - bool extension_disabled[KVM_RISCV_SBI_EXT_MAX];
> + enum KVM_RISCV_SBI_EXT_STATUS ext_status[KVM_RISCV_SBI_EXT_MAX];
> };
>
> struct kvm_vcpu_sbi_return {
> diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c
> index 6aa15f1b97d9..28e55ba023dc 100644
> --- a/arch/riscv/kvm/vcpu_sbi.c
> +++ b/arch/riscv/kvm/vcpu_sbi.c
> @@ -155,7 +155,8 @@ static int riscv_vcpu_set_sbi_ext_single(struct kvm_vcpu *vcpu,
> if (!sext)
> return -ENOENT;
>
> - scontext->extension_disabled[sext->ext_idx] = !reg_val;
> + scontext->ext_status[sext->ext_idx] = reg_val ?
> + KVM_RISCV_SBI_EXT_AVAILABLE : KVM_RISCV_SBI_EXT_UNAVAILABLE;
>
> return 0;
> }
> @@ -180,7 +181,8 @@ static int riscv_vcpu_get_sbi_ext_single(struct kvm_vcpu *vcpu,
> if (!sext)
> return -ENOENT;
>
> - *reg_val = !scontext->extension_disabled[sext->ext_idx];
> + *reg_val = scontext->ext_status[sext->ext_idx] ==
> + KVM_RISCV_SBI_EXT_AVAILABLE;
>
> return 0;
> }
> @@ -316,7 +318,8 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext(
> if (sext->ext_ptr->extid_start <= extid &&
> sext->ext_ptr->extid_end >= extid) {
> if (sext->ext_idx < KVM_RISCV_SBI_EXT_MAX &&
> - scontext->extension_disabled[sext->ext_idx])
> + scontext->ext_status[sext->ext_idx] ==
> + KVM_RISCV_SBI_EXT_UNAVAILABLE)
> return NULL;
> return sbi_ext[i].ext_ptr;
> }
> --
> 2.40.1
>
Otherwise, this looks good to me.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 3/3] RISC-V: KVM: Probe for SBI extension status
2023-05-26 10:25 ` [PATCH v2 3/3] RISC-V: KVM: Probe for SBI extension status Andrew Jones
@ 2023-05-30 17:05 ` Anup Patel
0 siblings, 0 replies; 7+ messages in thread
From: Anup Patel @ 2023-05-30 17:05 UTC (permalink / raw)
To: Andrew Jones
Cc: linux-riscv, kvm-riscv, Palmer Dabbelt, Atish Patra, Albert Ou,
Paul Walmsley
On Fri, May 26, 2023 at 3:55 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> Rather than defaulting the status to available and allowing the user
> to set availability, default to uninitialized and only allow the user
> to set the status to unavailable. Then, when an extension is first
> used, ensure it is available by invoking its probe function, if it
> has one (an extension is assumed available if it doesn't have a probe
> function). Checking the status in kvm_vcpu_sbi_find_ext() ensures
> extension functions cannot be invoked when they're unavailable.
>
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
Looks good to me.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
> ---
> arch/riscv/include/asm/kvm_vcpu_sbi.h | 1 +
> arch/riscv/kvm/vcpu_sbi.c | 51 ++++++++++++++++++++-------
> 2 files changed, 40 insertions(+), 12 deletions(-)
>
> diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h
> index cda99fc3d897..a55b6225aa55 100644
> --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h
> +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h
> @@ -15,6 +15,7 @@
> #define KVM_SBI_VERSION_MINOR 0
>
> enum KVM_RISCV_SBI_EXT_STATUS {
> + KVM_RISCV_SBI_EXT_UNINITIALIZED,
> KVM_RISCV_SBI_EXT_AVAILABLE,
> KVM_RISCV_SBI_EXT_UNAVAILABLE,
> };
> diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c
> index 28e55ba023dc..b3e92c11e54f 100644
> --- a/arch/riscv/kvm/vcpu_sbi.c
> +++ b/arch/riscv/kvm/vcpu_sbi.c
> @@ -155,8 +155,15 @@ static int riscv_vcpu_set_sbi_ext_single(struct kvm_vcpu *vcpu,
> if (!sext)
> return -ENOENT;
>
> - scontext->ext_status[sext->ext_idx] = reg_val ?
> - KVM_RISCV_SBI_EXT_AVAILABLE : KVM_RISCV_SBI_EXT_UNAVAILABLE;
> + /*
> + * We can't set the extension status to available here, since it may
> + * have a probe() function which needs to confirm availability first,
> + * but it may be too early to call that here. We can set the status to
> + * unavailable, though.
> + */
> + if (!reg_val)
> + scontext->ext_status[sext->ext_idx] =
> + KVM_RISCV_SBI_EXT_UNAVAILABLE;
>
> return 0;
> }
> @@ -181,8 +188,15 @@ static int riscv_vcpu_get_sbi_ext_single(struct kvm_vcpu *vcpu,
> if (!sext)
> return -ENOENT;
>
> - *reg_val = scontext->ext_status[sext->ext_idx] ==
> - KVM_RISCV_SBI_EXT_AVAILABLE;
> + /*
> + * If the extension status is still uninitialized, then we should probe
> + * to determine if it's available, but it may be too early to do that
> + * here. The best we can do is report that the extension has not been
> + * disabled, i.e. we return 1 when the extension is available and also
> + * when it only may be available.
> + */
> + *reg_val = scontext->ext_status[sext->ext_idx] !=
> + KVM_RISCV_SBI_EXT_UNAVAILABLE;
>
> return 0;
> }
> @@ -309,19 +323,32 @@ int kvm_riscv_vcpu_get_reg_sbi_ext(struct kvm_vcpu *vcpu,
> const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext(
> struct kvm_vcpu *vcpu, unsigned long extid)
> {
> - int i;
> - const struct kvm_riscv_sbi_extension_entry *sext;
> struct kvm_vcpu_sbi_context *scontext = &vcpu->arch.sbi_context;
> + const struct kvm_riscv_sbi_extension_entry *entry;
> + const struct kvm_vcpu_sbi_extension *ext;
> + int i;
>
> for (i = 0; i < ARRAY_SIZE(sbi_ext); i++) {
> - sext = &sbi_ext[i];
> - if (sext->ext_ptr->extid_start <= extid &&
> - sext->ext_ptr->extid_end >= extid) {
> - if (sext->ext_idx < KVM_RISCV_SBI_EXT_MAX &&
> - scontext->ext_status[sext->ext_idx] ==
> + entry = &sbi_ext[i];
> + ext = entry->ext_ptr;
> +
> + if (ext->extid_start <= extid && ext->extid_end >= extid) {
> + if (entry->ext_idx >= KVM_RISCV_SBI_EXT_MAX ||
> + scontext->ext_status[entry->ext_idx] ==
> + KVM_RISCV_SBI_EXT_AVAILABLE)
> + return ext;
> + if (scontext->ext_status[entry->ext_idx] ==
> KVM_RISCV_SBI_EXT_UNAVAILABLE)
> return NULL;
> - return sbi_ext[i].ext_ptr;
> + if (ext->probe && !ext->probe(vcpu)) {
> + scontext->ext_status[entry->ext_idx] =
> + KVM_RISCV_SBI_EXT_UNAVAILABLE;
> + return NULL;
> + }
> +
> + scontext->ext_status[entry->ext_idx] =
> + KVM_RISCV_SBI_EXT_AVAILABLE;
> + return ext;
> }
> }
>
> --
> 2.40.1
>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/3] RISC-V: KVM: Convert extension_disabled[] to ext_status[]
2023-05-30 16:58 ` Anup Patel
@ 2023-05-30 17:38 ` Andrew Jones
0 siblings, 0 replies; 7+ messages in thread
From: Andrew Jones @ 2023-05-30 17:38 UTC (permalink / raw)
To: Anup Patel
Cc: linux-riscv, kvm-riscv, Palmer Dabbelt, Atish Patra, Albert Ou,
Paul Walmsley
On Tue, May 30, 2023 at 10:28:16PM +0530, Anup Patel wrote:
> On Fri, May 26, 2023 at 3:55 PM Andrew Jones <ajones@ventanamicro.com> wrote:
> >
> > Change the boolean extension_disabled[] array to an array of enums,
> > ext_status[]. For now, the enum only has two states, which correspond
> > to the previous boolean states, so this patch has no intended
> > functional change. The next patch will add another state, expanding
> > the purpose of ext_status[].
> >
> > Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> > ---
> > arch/riscv/include/asm/kvm_vcpu_sbi.h | 7 ++++++-
> > arch/riscv/kvm/vcpu_sbi.c | 9 ++++++---
> > 2 files changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h
> > index 4278125a38a5..cda99fc3d897 100644
> > --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h
> > +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h
> > @@ -14,9 +14,14 @@
> > #define KVM_SBI_VERSION_MAJOR 1
> > #define KVM_SBI_VERSION_MINOR 0
> >
> > +enum KVM_RISCV_SBI_EXT_STATUS {
>
> This upper-case name of enum looks odd.
I agree. I saw the uppercase enum names in the uapi and matched that, but
now I see the non-uapi enums have normal lowercase names. I'll send a v3
with a lowercase name.
>
> > + KVM_RISCV_SBI_EXT_AVAILABLE,
> > + KVM_RISCV_SBI_EXT_UNAVAILABLE,
> > +};
> > +
> > struct kvm_vcpu_sbi_context {
> > int return_handled;
> > - bool extension_disabled[KVM_RISCV_SBI_EXT_MAX];
> > + enum KVM_RISCV_SBI_EXT_STATUS ext_status[KVM_RISCV_SBI_EXT_MAX];
> > };
> >
> > struct kvm_vcpu_sbi_return {
> > diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c
> > index 6aa15f1b97d9..28e55ba023dc 100644
> > --- a/arch/riscv/kvm/vcpu_sbi.c
> > +++ b/arch/riscv/kvm/vcpu_sbi.c
> > @@ -155,7 +155,8 @@ static int riscv_vcpu_set_sbi_ext_single(struct kvm_vcpu *vcpu,
> > if (!sext)
> > return -ENOENT;
> >
> > - scontext->extension_disabled[sext->ext_idx] = !reg_val;
> > + scontext->ext_status[sext->ext_idx] = reg_val ?
> > + KVM_RISCV_SBI_EXT_AVAILABLE : KVM_RISCV_SBI_EXT_UNAVAILABLE;
> >
> > return 0;
> > }
> > @@ -180,7 +181,8 @@ static int riscv_vcpu_get_sbi_ext_single(struct kvm_vcpu *vcpu,
> > if (!sext)
> > return -ENOENT;
> >
> > - *reg_val = !scontext->extension_disabled[sext->ext_idx];
> > + *reg_val = scontext->ext_status[sext->ext_idx] ==
> > + KVM_RISCV_SBI_EXT_AVAILABLE;
> >
> > return 0;
> > }
> > @@ -316,7 +318,8 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext(
> > if (sext->ext_ptr->extid_start <= extid &&
> > sext->ext_ptr->extid_end >= extid) {
> > if (sext->ext_idx < KVM_RISCV_SBI_EXT_MAX &&
> > - scontext->extension_disabled[sext->ext_idx])
> > + scontext->ext_status[sext->ext_idx] ==
> > + KVM_RISCV_SBI_EXT_UNAVAILABLE)
> > return NULL;
> > return sbi_ext[i].ext_ptr;
> > }
> > --
> > 2.40.1
> >
>
> Otherwise, this looks good to me.
>
> Reviewed-by: Anup Patel <anup@brainfault.org>
>
Thanks,
drew
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-05-30 17:38 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-26 10:25 [PATCH v2 0/3] RISC-V: KVM: Ensure SBI extension is enabled Andrew Jones
2023-05-26 10:25 ` [PATCH v2 1/3] RISC-V: KVM: Rename dis_idx to ext_idx Andrew Jones
2023-05-26 10:25 ` [PATCH v2 2/3] RISC-V: KVM: Convert extension_disabled[] to ext_status[] Andrew Jones
2023-05-30 16:58 ` Anup Patel
2023-05-30 17:38 ` Andrew Jones
2023-05-26 10:25 ` [PATCH v2 3/3] RISC-V: KVM: Probe for SBI extension status Andrew Jones
2023-05-30 17:05 ` Anup Patel
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).