* [PATCH v2 0/3] RISC-V: KVM: Ensure SBI extension is enabled @ 2023-05-26 10:25 ` Andrew Jones 0 siblings, 0 replies; 14+ messages in thread From: Andrew Jones @ 2023-05-26 10:25 UTC (permalink / raw) To: kvm-riscv 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 0/3] RISC-V: KVM: Ensure SBI extension is enabled @ 2023-05-26 10:25 ` Andrew Jones 0 siblings, 0 replies; 14+ 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] 14+ messages in thread
* [PATCH v2 1/3] RISC-V: KVM: Rename dis_idx to ext_idx 2023-05-26 10:25 ` Andrew Jones @ 2023-05-26 10:25 ` Andrew Jones -1 siblings, 0 replies; 14+ messages in thread From: Andrew Jones @ 2023-05-26 10:25 UTC (permalink / raw) To: kvm-riscv 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 1/3] RISC-V: KVM: Rename dis_idx to ext_idx @ 2023-05-26 10:25 ` Andrew Jones 0 siblings, 0 replies; 14+ 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] 14+ messages in thread
* [PATCH v2 2/3] RISC-V: KVM: Convert extension_disabled[] to ext_status[] 2023-05-26 10:25 ` Andrew Jones @ 2023-05-26 10:25 ` Andrew Jones -1 siblings, 0 replies; 14+ messages in thread From: Andrew Jones @ 2023-05-26 10:25 UTC (permalink / raw) To: kvm-riscv 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 2/3] RISC-V: KVM: Convert extension_disabled[] to ext_status[] @ 2023-05-26 10:25 ` Andrew Jones 0 siblings, 0 replies; 14+ 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] 14+ messages in thread
* [PATCH v2 2/3] RISC-V: KVM: Convert extension_disabled[] to ext_status[] 2023-05-26 10:25 ` Andrew Jones @ 2023-05-30 16:58 ` Anup Patel -1 siblings, 0 replies; 14+ messages in thread From: Anup Patel @ 2023-05-30 16:58 UTC (permalink / raw) To: kvm-riscv 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/3] RISC-V: KVM: Convert extension_disabled[] to ext_status[] @ 2023-05-30 16:58 ` Anup Patel 0 siblings, 0 replies; 14+ 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] 14+ messages in thread
* [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 -1 siblings, 0 replies; 14+ messages in thread From: Andrew Jones @ 2023-05-30 17:38 UTC (permalink / raw) To: kvm-riscv 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/3] RISC-V: KVM: Convert extension_disabled[] to ext_status[] @ 2023-05-30 17:38 ` Andrew Jones 0 siblings, 0 replies; 14+ 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] 14+ messages in thread
* [PATCH v2 3/3] RISC-V: KVM: Probe for SBI extension status 2023-05-26 10:25 ` Andrew Jones @ 2023-05-26 10:25 ` Andrew Jones -1 siblings, 0 replies; 14+ messages in thread From: Andrew Jones @ 2023-05-26 10:25 UTC (permalink / raw) To: kvm-riscv 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 3/3] RISC-V: KVM: Probe for SBI extension status @ 2023-05-26 10:25 ` Andrew Jones 0 siblings, 0 replies; 14+ 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] 14+ messages in thread
* [PATCH v2 3/3] RISC-V: KVM: Probe for SBI extension status 2023-05-26 10:25 ` Andrew Jones @ 2023-05-30 17:05 ` Anup Patel -1 siblings, 0 replies; 14+ messages in thread From: Anup Patel @ 2023-05-30 17:05 UTC (permalink / raw) To: kvm-riscv 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 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/3] RISC-V: KVM: Probe for SBI extension status @ 2023-05-30 17:05 ` Anup Patel 0 siblings, 0 replies; 14+ 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] 14+ messages in thread
end of thread, other threads:[~2023-05-30 17:38 UTC | newest] Thread overview: 14+ 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 ` 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-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 16:58 ` Anup Patel 2023-05-30 16:58 ` Anup Patel 2023-05-30 17:38 ` Andrew Jones 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-26 10:25 ` Andrew Jones 2023-05-30 17:05 ` Anup Patel 2023-05-30 17:05 ` Anup Patel
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.