* [PATCH 0/3] RISC-V: KVM: Ensure SBI extension is enabled @ 2023-04-26 17:13 ` Andrew Jones 0 siblings, 0 replies; 20+ messages in thread From: Andrew Jones @ 2023-04-26 17:13 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. Andrew Jones (3): RISC-V: KVM: Disable SBI extension when its probe fails RISC-V: KVM: Rename dis_idx to ext_idx RISC-V: KVM: Introduce extension_enabled arch/riscv/include/asm/kvm_vcpu_sbi.h | 9 ++++ arch/riscv/kvm/vcpu_sbi.c | 62 +++++++++++++++++---------- 2 files changed, 48 insertions(+), 23 deletions(-) -- 2.39.2 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 0/3] RISC-V: KVM: Ensure SBI extension is enabled @ 2023-04-26 17:13 ` Andrew Jones 0 siblings, 0 replies; 20+ messages in thread From: Andrew Jones @ 2023-04-26 17:13 UTC (permalink / raw) To: kvm-riscv, linux-riscv Cc: 'Palmer Dabbelt ', 'Anup Patel ', 'Atish Patra ', 'Albert Ou ', 'Paul Walmsley ' 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. Andrew Jones (3): RISC-V: KVM: Disable SBI extension when its probe fails RISC-V: KVM: Rename dis_idx to ext_idx RISC-V: KVM: Introduce extension_enabled arch/riscv/include/asm/kvm_vcpu_sbi.h | 9 ++++ arch/riscv/kvm/vcpu_sbi.c | 62 +++++++++++++++++---------- 2 files changed, 48 insertions(+), 23 deletions(-) -- 2.39.2 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/3] RISC-V: KVM: Disable SBI extension when its probe fails 2023-04-26 17:13 ` Andrew Jones @ 2023-04-26 17:13 ` Andrew Jones -1 siblings, 0 replies; 20+ messages in thread From: Andrew Jones @ 2023-04-26 17:13 UTC (permalink / raw) To: kvm-riscv When an SBI extension specific probe function exists and fails, then use the extension_disabled context to disable the extension. This ensures the extension's functions cannot be invoked. Doing the disabling in kvm_vcpu_sbi_find_ext() allows it to be done lazily on its first use. Checking extension_disabled prior to probing ensures the probe is only executed once for disabled extensions. Later patches ensure we only execute probe once for enabled extensions as well. Signed-off-by: Andrew Jones <ajones@ventanamicro.com> --- arch/riscv/kvm/vcpu_sbi.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c index e52fde504433..aa3c126d2e3c 100644 --- a/arch/riscv/kvm/vcpu_sbi.c +++ b/arch/riscv/kvm/vcpu_sbi.c @@ -307,18 +307,25 @@ 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->dis_idx < KVM_RISCV_SBI_EXT_MAX && - scontext->extension_disabled[sext->dis_idx]) + entry = &sbi_ext[i]; + ext = entry->ext_ptr; + + if (ext->extid_start <= extid && ext->extid_end >= extid) { + if (entry->dis_idx >= KVM_RISCV_SBI_EXT_MAX) + return ext; + if (scontext->extension_disabled[entry->dis_idx]) + return NULL; + if (ext->probe && !ext->probe(vcpu)) { + scontext->extension_disabled[entry->dis_idx] = true; return NULL; - return sbi_ext[i].ext_ptr; + } + return ext; } } -- 2.39.2 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 1/3] RISC-V: KVM: Disable SBI extension when its probe fails @ 2023-04-26 17:13 ` Andrew Jones 0 siblings, 0 replies; 20+ messages in thread From: Andrew Jones @ 2023-04-26 17:13 UTC (permalink / raw) To: kvm-riscv, linux-riscv Cc: 'Palmer Dabbelt ', 'Anup Patel ', 'Atish Patra ', 'Albert Ou ', 'Paul Walmsley ' When an SBI extension specific probe function exists and fails, then use the extension_disabled context to disable the extension. This ensures the extension's functions cannot be invoked. Doing the disabling in kvm_vcpu_sbi_find_ext() allows it to be done lazily on its first use. Checking extension_disabled prior to probing ensures the probe is only executed once for disabled extensions. Later patches ensure we only execute probe once for enabled extensions as well. Signed-off-by: Andrew Jones <ajones@ventanamicro.com> --- arch/riscv/kvm/vcpu_sbi.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c index e52fde504433..aa3c126d2e3c 100644 --- a/arch/riscv/kvm/vcpu_sbi.c +++ b/arch/riscv/kvm/vcpu_sbi.c @@ -307,18 +307,25 @@ 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->dis_idx < KVM_RISCV_SBI_EXT_MAX && - scontext->extension_disabled[sext->dis_idx]) + entry = &sbi_ext[i]; + ext = entry->ext_ptr; + + if (ext->extid_start <= extid && ext->extid_end >= extid) { + if (entry->dis_idx >= KVM_RISCV_SBI_EXT_MAX) + return ext; + if (scontext->extension_disabled[entry->dis_idx]) + return NULL; + if (ext->probe && !ext->probe(vcpu)) { + scontext->extension_disabled[entry->dis_idx] = true; return NULL; - return sbi_ext[i].ext_ptr; + } + return ext; } } -- 2.39.2 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 1/3] RISC-V: KVM: Disable SBI extension when its probe fails 2023-04-26 17:13 ` Andrew Jones @ 2023-05-19 14:57 ` Anup Patel -1 siblings, 0 replies; 20+ messages in thread From: Anup Patel @ 2023-05-19 14:57 UTC (permalink / raw) To: kvm-riscv On Wed, Apr 26, 2023 at 10:43?PM Andrew Jones <ajones@ventanamicro.com> wrote: > > When an SBI extension specific probe function exists and fails, then > use the extension_disabled context to disable the extension. This > ensures the extension's functions cannot be invoked. Doing the > disabling in kvm_vcpu_sbi_find_ext() allows it to be done lazily > on its first use. Checking extension_disabled prior to probing > ensures the probe is only executed once for disabled extensions. > Later patches ensure we only execute probe once for enabled > extensions as well. > > Signed-off-by: Andrew Jones <ajones@ventanamicro.com> > --- > arch/riscv/kvm/vcpu_sbi.c | 23 +++++++++++++++-------- > 1 file changed, 15 insertions(+), 8 deletions(-) > > diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c > index e52fde504433..aa3c126d2e3c 100644 > --- a/arch/riscv/kvm/vcpu_sbi.c > +++ b/arch/riscv/kvm/vcpu_sbi.c > @@ -307,18 +307,25 @@ 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->dis_idx < KVM_RISCV_SBI_EXT_MAX && > - scontext->extension_disabled[sext->dis_idx]) > + entry = &sbi_ext[i]; > + ext = entry->ext_ptr; > + > + if (ext->extid_start <= extid && ext->extid_end >= extid) { > + if (entry->dis_idx >= KVM_RISCV_SBI_EXT_MAX) > + return ext; > + if (scontext->extension_disabled[entry->dis_idx]) > + return NULL; > + if (ext->probe && !ext->probe(vcpu)) { Calling probe() upon every kvm_vcpu_sbi_find_ext() will simply slow down all SBI calls. How about caching probe return values in "struct kvm_vcpu_sbi_context" as an array? Maybe we can have two arrays: 1) probe_done[] : Boolean array to check whether probe was done. 2) probe_retval[] : Cached probe() return values. The SBI_EXT_BASE_PROBE_EXT call should also return the cached value. > + scontext->extension_disabled[entry->dis_idx] = true; > return NULL; > - return sbi_ext[i].ext_ptr; > + } > + return ext; > } > } > > -- > 2.39.2 > Regards, Anup ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] RISC-V: KVM: Disable SBI extension when its probe fails @ 2023-05-19 14:57 ` Anup Patel 0 siblings, 0 replies; 20+ messages in thread From: Anup Patel @ 2023-05-19 14:57 UTC (permalink / raw) To: Andrew Jones Cc: kvm-riscv, linux-riscv, Palmer Dabbelt, Atish Patra, Albert Ou, Paul Walmsley On Wed, Apr 26, 2023 at 10:43 PM Andrew Jones <ajones@ventanamicro.com> wrote: > > When an SBI extension specific probe function exists and fails, then > use the extension_disabled context to disable the extension. This > ensures the extension's functions cannot be invoked. Doing the > disabling in kvm_vcpu_sbi_find_ext() allows it to be done lazily > on its first use. Checking extension_disabled prior to probing > ensures the probe is only executed once for disabled extensions. > Later patches ensure we only execute probe once for enabled > extensions as well. > > Signed-off-by: Andrew Jones <ajones@ventanamicro.com> > --- > arch/riscv/kvm/vcpu_sbi.c | 23 +++++++++++++++-------- > 1 file changed, 15 insertions(+), 8 deletions(-) > > diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c > index e52fde504433..aa3c126d2e3c 100644 > --- a/arch/riscv/kvm/vcpu_sbi.c > +++ b/arch/riscv/kvm/vcpu_sbi.c > @@ -307,18 +307,25 @@ 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->dis_idx < KVM_RISCV_SBI_EXT_MAX && > - scontext->extension_disabled[sext->dis_idx]) > + entry = &sbi_ext[i]; > + ext = entry->ext_ptr; > + > + if (ext->extid_start <= extid && ext->extid_end >= extid) { > + if (entry->dis_idx >= KVM_RISCV_SBI_EXT_MAX) > + return ext; > + if (scontext->extension_disabled[entry->dis_idx]) > + return NULL; > + if (ext->probe && !ext->probe(vcpu)) { Calling probe() upon every kvm_vcpu_sbi_find_ext() will simply slow down all SBI calls. How about caching probe return values in "struct kvm_vcpu_sbi_context" as an array? Maybe we can have two arrays: 1) probe_done[] : Boolean array to check whether probe was done. 2) probe_retval[] : Cached probe() return values. The SBI_EXT_BASE_PROBE_EXT call should also return the cached value. > + scontext->extension_disabled[entry->dis_idx] = true; > return NULL; > - return sbi_ext[i].ext_ptr; > + } > + return ext; > } > } > > -- > 2.39.2 > Regards, Anup _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/3] RISC-V: KVM: Disable SBI extension when its probe fails 2023-05-19 14:57 ` Anup Patel @ 2023-05-19 15:02 ` Anup Patel -1 siblings, 0 replies; 20+ messages in thread From: Anup Patel @ 2023-05-19 15:02 UTC (permalink / raw) To: kvm-riscv On Fri, May 19, 2023 at 8:27?PM Anup Patel <anup@brainfault.org> wrote: > > On Wed, Apr 26, 2023 at 10:43?PM Andrew Jones <ajones@ventanamicro.com> wrote: > > > > When an SBI extension specific probe function exists and fails, then > > use the extension_disabled context to disable the extension. This > > ensures the extension's functions cannot be invoked. Doing the > > disabling in kvm_vcpu_sbi_find_ext() allows it to be done lazily > > on its first use. Checking extension_disabled prior to probing > > ensures the probe is only executed once for disabled extensions. > > Later patches ensure we only execute probe once for enabled > > extensions as well. > > > > Signed-off-by: Andrew Jones <ajones@ventanamicro.com> > > --- > > arch/riscv/kvm/vcpu_sbi.c | 23 +++++++++++++++-------- > > 1 file changed, 15 insertions(+), 8 deletions(-) > > > > diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c > > index e52fde504433..aa3c126d2e3c 100644 > > --- a/arch/riscv/kvm/vcpu_sbi.c > > +++ b/arch/riscv/kvm/vcpu_sbi.c > > @@ -307,18 +307,25 @@ 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->dis_idx < KVM_RISCV_SBI_EXT_MAX && > > - scontext->extension_disabled[sext->dis_idx]) > > + entry = &sbi_ext[i]; > > + ext = entry->ext_ptr; > > + > > + if (ext->extid_start <= extid && ext->extid_end >= extid) { > > + if (entry->dis_idx >= KVM_RISCV_SBI_EXT_MAX) > > + return ext; > > + if (scontext->extension_disabled[entry->dis_idx]) > > + return NULL; > > + if (ext->probe && !ext->probe(vcpu)) { > > Calling probe() upon every kvm_vcpu_sbi_find_ext() will simply slow down > all SBI calls. > > How about caching probe return values in "struct kvm_vcpu_sbi_context" > as an array? > > Maybe we can have two arrays: > 1) probe_done[] : Boolean array to check whether probe was done. > 2) probe_retval[] : Cached probe() return values. > > The SBI_EXT_BASE_PROBE_EXT call should also return the > cached value. Ignore this comment. Your PATCH3 does the same thing. Regards, Anup > > > + scontext->extension_disabled[entry->dis_idx] = true; > > return NULL; > > - return sbi_ext[i].ext_ptr; > > + } > > + return ext; > > } > > } > > > > -- > > 2.39.2 > > > > Regards, > Anup ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] RISC-V: KVM: Disable SBI extension when its probe fails @ 2023-05-19 15:02 ` Anup Patel 0 siblings, 0 replies; 20+ messages in thread From: Anup Patel @ 2023-05-19 15:02 UTC (permalink / raw) To: Andrew Jones Cc: kvm-riscv, linux-riscv, Palmer Dabbelt, Atish Patra, Albert Ou, Paul Walmsley On Fri, May 19, 2023 at 8:27 PM Anup Patel <anup@brainfault.org> wrote: > > On Wed, Apr 26, 2023 at 10:43 PM Andrew Jones <ajones@ventanamicro.com> wrote: > > > > When an SBI extension specific probe function exists and fails, then > > use the extension_disabled context to disable the extension. This > > ensures the extension's functions cannot be invoked. Doing the > > disabling in kvm_vcpu_sbi_find_ext() allows it to be done lazily > > on its first use. Checking extension_disabled prior to probing > > ensures the probe is only executed once for disabled extensions. > > Later patches ensure we only execute probe once for enabled > > extensions as well. > > > > Signed-off-by: Andrew Jones <ajones@ventanamicro.com> > > --- > > arch/riscv/kvm/vcpu_sbi.c | 23 +++++++++++++++-------- > > 1 file changed, 15 insertions(+), 8 deletions(-) > > > > diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c > > index e52fde504433..aa3c126d2e3c 100644 > > --- a/arch/riscv/kvm/vcpu_sbi.c > > +++ b/arch/riscv/kvm/vcpu_sbi.c > > @@ -307,18 +307,25 @@ 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->dis_idx < KVM_RISCV_SBI_EXT_MAX && > > - scontext->extension_disabled[sext->dis_idx]) > > + entry = &sbi_ext[i]; > > + ext = entry->ext_ptr; > > + > > + if (ext->extid_start <= extid && ext->extid_end >= extid) { > > + if (entry->dis_idx >= KVM_RISCV_SBI_EXT_MAX) > > + return ext; > > + if (scontext->extension_disabled[entry->dis_idx]) > > + return NULL; > > + if (ext->probe && !ext->probe(vcpu)) { > > Calling probe() upon every kvm_vcpu_sbi_find_ext() will simply slow down > all SBI calls. > > How about caching probe return values in "struct kvm_vcpu_sbi_context" > as an array? > > Maybe we can have two arrays: > 1) probe_done[] : Boolean array to check whether probe was done. > 2) probe_retval[] : Cached probe() return values. > > The SBI_EXT_BASE_PROBE_EXT call should also return the > cached value. Ignore this comment. Your PATCH3 does the same thing. Regards, Anup > > > + scontext->extension_disabled[entry->dis_idx] = true; > > return NULL; > > - return sbi_ext[i].ext_ptr; > > + } > > + return ext; > > } > > } > > > > -- > > 2.39.2 > > > > Regards, > Anup _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/3] RISC-V: KVM: Rename dis_idx to ext_idx 2023-04-26 17:13 ` Andrew Jones @ 2023-04-26 17:13 ` Andrew Jones -1 siblings, 0 replies; 20+ messages in thread From: Andrew Jones @ 2023-04-26 17:13 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> --- arch/riscv/kvm/vcpu_sbi.c | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c index aa3c126d2e3c..a1a82f0fbad2 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; } @@ -317,12 +317,12 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext( ext = entry->ext_ptr; if (ext->extid_start <= extid && ext->extid_end >= extid) { - if (entry->dis_idx >= KVM_RISCV_SBI_EXT_MAX) + if (entry->ext_idx >= KVM_RISCV_SBI_EXT_MAX) return ext; - if (scontext->extension_disabled[entry->dis_idx]) + if (scontext->extension_disabled[entry->ext_idx]) return NULL; if (ext->probe && !ext->probe(vcpu)) { - scontext->extension_disabled[entry->dis_idx] = true; + scontext->extension_disabled[entry->ext_idx] = true; return NULL; } return ext; -- 2.39.2 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/3] RISC-V: KVM: Rename dis_idx to ext_idx @ 2023-04-26 17:13 ` Andrew Jones 0 siblings, 0 replies; 20+ messages in thread From: Andrew Jones @ 2023-04-26 17:13 UTC (permalink / raw) To: kvm-riscv, linux-riscv Cc: 'Palmer Dabbelt ', 'Anup Patel ', 'Atish Patra ', 'Albert Ou ', 'Paul Walmsley ' Make the name of the extension_disabled[] index more general in order to expand its application. Signed-off-by: Andrew Jones <ajones@ventanamicro.com> --- arch/riscv/kvm/vcpu_sbi.c | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c index aa3c126d2e3c..a1a82f0fbad2 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; } @@ -317,12 +317,12 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext( ext = entry->ext_ptr; if (ext->extid_start <= extid && ext->extid_end >= extid) { - if (entry->dis_idx >= KVM_RISCV_SBI_EXT_MAX) + if (entry->ext_idx >= KVM_RISCV_SBI_EXT_MAX) return ext; - if (scontext->extension_disabled[entry->dis_idx]) + if (scontext->extension_disabled[entry->ext_idx]) return NULL; if (ext->probe && !ext->probe(vcpu)) { - scontext->extension_disabled[entry->dis_idx] = true; + scontext->extension_disabled[entry->ext_idx] = true; return NULL; } return ext; -- 2.39.2 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/3] RISC-V: KVM: Rename dis_idx to ext_idx 2023-04-26 17:13 ` Andrew Jones @ 2023-05-19 14:58 ` Anup Patel -1 siblings, 0 replies; 20+ messages in thread From: Anup Patel @ 2023-05-19 14:58 UTC (permalink / raw) To: kvm-riscv On Wed, Apr 26, 2023 at 10:43?PM Andrew Jones <ajones@ventanamicro.com> wrote: > > Make the name of the extension_disabled[] index more general in > order to expand its application. > > Signed-off-by: Andrew Jones <ajones@ventanamicro.com> Looks good to me. Reviewed-by: Anup Patel <anup@brainfault.org> Regards, Anup > --- > arch/riscv/kvm/vcpu_sbi.c | 36 ++++++++++++++++++------------------ > 1 file changed, 18 insertions(+), 18 deletions(-) > > diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c > index aa3c126d2e3c..a1a82f0fbad2 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; > } > @@ -317,12 +317,12 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext( > ext = entry->ext_ptr; > > if (ext->extid_start <= extid && ext->extid_end >= extid) { > - if (entry->dis_idx >= KVM_RISCV_SBI_EXT_MAX) > + if (entry->ext_idx >= KVM_RISCV_SBI_EXT_MAX) > return ext; > - if (scontext->extension_disabled[entry->dis_idx]) > + if (scontext->extension_disabled[entry->ext_idx]) > return NULL; > if (ext->probe && !ext->probe(vcpu)) { > - scontext->extension_disabled[entry->dis_idx] = true; > + scontext->extension_disabled[entry->ext_idx] = true; > return NULL; > } > return ext; > -- > 2.39.2 > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] RISC-V: KVM: Rename dis_idx to ext_idx @ 2023-05-19 14:58 ` Anup Patel 0 siblings, 0 replies; 20+ messages in thread From: Anup Patel @ 2023-05-19 14:58 UTC (permalink / raw) To: Andrew Jones Cc: kvm-riscv, linux-riscv, Palmer Dabbelt, Atish Patra, Albert Ou, Paul Walmsley On Wed, Apr 26, 2023 at 10:43 PM Andrew Jones <ajones@ventanamicro.com> wrote: > > Make the name of the extension_disabled[] index more general in > order to expand its application. > > Signed-off-by: Andrew Jones <ajones@ventanamicro.com> Looks good to me. Reviewed-by: Anup Patel <anup@brainfault.org> Regards, Anup > --- > arch/riscv/kvm/vcpu_sbi.c | 36 ++++++++++++++++++------------------ > 1 file changed, 18 insertions(+), 18 deletions(-) > > diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c > index aa3c126d2e3c..a1a82f0fbad2 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; > } > @@ -317,12 +317,12 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext( > ext = entry->ext_ptr; > > if (ext->extid_start <= extid && ext->extid_end >= extid) { > - if (entry->dis_idx >= KVM_RISCV_SBI_EXT_MAX) > + if (entry->ext_idx >= KVM_RISCV_SBI_EXT_MAX) > return ext; > - if (scontext->extension_disabled[entry->dis_idx]) > + if (scontext->extension_disabled[entry->ext_idx]) > return NULL; > if (ext->probe && !ext->probe(vcpu)) { > - scontext->extension_disabled[entry->dis_idx] = true; > + scontext->extension_disabled[entry->ext_idx] = true; > return NULL; > } > return ext; > -- > 2.39.2 > _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 3/3] RISC-V: KVM: Introduce extension_enabled 2023-04-26 17:13 ` Andrew Jones @ 2023-04-26 17:13 ` Andrew Jones -1 siblings, 0 replies; 20+ messages in thread From: Andrew Jones @ 2023-04-26 17:13 UTC (permalink / raw) To: kvm-riscv We need three SBI extension status values (uninitialized, enabled, and disabled). Pairing another boolean array, extension_enabled[], with the boolean array extension_disabled[] provides four states. Using a pair of boolean arrays, which may eventually be changed to a pair of bitmaps, is more space efficient than using one enum status field. Apply the new (enabled=1,disabled=0) state, which means either the extension doesn't have a probe function or that its probe was successful, to avoid more than one probe of the extension. Signed-off-by: Andrew Jones <ajones@ventanamicro.com> --- arch/riscv/include/asm/kvm_vcpu_sbi.h | 9 +++++++++ arch/riscv/kvm/vcpu_sbi.c | 11 ++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h index 4278125a38a5..e3c5e1d15e93 100644 --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h @@ -16,6 +16,15 @@ struct kvm_vcpu_sbi_context { int return_handled; + /* + * extension_enabled[] and extension_disabled[] provide SBI + * extensions four status values, of which we need three: + * (0,0) uninitialized, (1,0) enabled, (0,1) disabled. Using + * a pair of boolean arrays, which may eventually be changed + * to a pair of bitmaps, is more space efficient than using + * one enum status field. + */ + bool extension_enabled[KVM_RISCV_SBI_EXT_MAX]; bool extension_disabled[KVM_RISCV_SBI_EXT_MAX]; }; diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c index a1a82f0fbad2..344d38bbe06a 100644 --- a/arch/riscv/kvm/vcpu_sbi.c +++ b/arch/riscv/kvm/vcpu_sbi.c @@ -155,6 +155,12 @@ static int riscv_vcpu_set_sbi_ext_single(struct kvm_vcpu *vcpu, if (!sext) return -ENOENT; + /* + * We can't set scontext->extension_enabled[] to reg_val since the + * extension may have a probe() function which needs to confirm + * enablement first. Only set extension_disabled[] here and leave + * the extension_enabled[] setting to kvm_vcpu_sbi_find_ext(). + */ scontext->extension_disabled[sext->ext_idx] = !reg_val; return 0; @@ -317,7 +323,8 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext( ext = entry->ext_ptr; if (ext->extid_start <= extid && ext->extid_end >= extid) { - if (entry->ext_idx >= KVM_RISCV_SBI_EXT_MAX) + if (entry->ext_idx >= KVM_RISCV_SBI_EXT_MAX || + scontext->extension_enabled[entry->ext_idx]) return ext; if (scontext->extension_disabled[entry->ext_idx]) return NULL; @@ -325,6 +332,8 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext( scontext->extension_disabled[entry->ext_idx] = true; return NULL; } + + scontext->extension_enabled[entry->ext_idx] = true; return ext; } } -- 2.39.2 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/3] RISC-V: KVM: Introduce extension_enabled @ 2023-04-26 17:13 ` Andrew Jones 0 siblings, 0 replies; 20+ messages in thread From: Andrew Jones @ 2023-04-26 17:13 UTC (permalink / raw) To: kvm-riscv, linux-riscv Cc: 'Palmer Dabbelt ', 'Anup Patel ', 'Atish Patra ', 'Albert Ou ', 'Paul Walmsley ' We need three SBI extension status values (uninitialized, enabled, and disabled). Pairing another boolean array, extension_enabled[], with the boolean array extension_disabled[] provides four states. Using a pair of boolean arrays, which may eventually be changed to a pair of bitmaps, is more space efficient than using one enum status field. Apply the new (enabled=1,disabled=0) state, which means either the extension doesn't have a probe function or that its probe was successful, to avoid more than one probe of the extension. Signed-off-by: Andrew Jones <ajones@ventanamicro.com> --- arch/riscv/include/asm/kvm_vcpu_sbi.h | 9 +++++++++ arch/riscv/kvm/vcpu_sbi.c | 11 ++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h index 4278125a38a5..e3c5e1d15e93 100644 --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h @@ -16,6 +16,15 @@ struct kvm_vcpu_sbi_context { int return_handled; + /* + * extension_enabled[] and extension_disabled[] provide SBI + * extensions four status values, of which we need three: + * (0,0) uninitialized, (1,0) enabled, (0,1) disabled. Using + * a pair of boolean arrays, which may eventually be changed + * to a pair of bitmaps, is more space efficient than using + * one enum status field. + */ + bool extension_enabled[KVM_RISCV_SBI_EXT_MAX]; bool extension_disabled[KVM_RISCV_SBI_EXT_MAX]; }; diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c index a1a82f0fbad2..344d38bbe06a 100644 --- a/arch/riscv/kvm/vcpu_sbi.c +++ b/arch/riscv/kvm/vcpu_sbi.c @@ -155,6 +155,12 @@ static int riscv_vcpu_set_sbi_ext_single(struct kvm_vcpu *vcpu, if (!sext) return -ENOENT; + /* + * We can't set scontext->extension_enabled[] to reg_val since the + * extension may have a probe() function which needs to confirm + * enablement first. Only set extension_disabled[] here and leave + * the extension_enabled[] setting to kvm_vcpu_sbi_find_ext(). + */ scontext->extension_disabled[sext->ext_idx] = !reg_val; return 0; @@ -317,7 +323,8 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext( ext = entry->ext_ptr; if (ext->extid_start <= extid && ext->extid_end >= extid) { - if (entry->ext_idx >= KVM_RISCV_SBI_EXT_MAX) + if (entry->ext_idx >= KVM_RISCV_SBI_EXT_MAX || + scontext->extension_enabled[entry->ext_idx]) return ext; if (scontext->extension_disabled[entry->ext_idx]) return NULL; @@ -325,6 +332,8 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext( scontext->extension_disabled[entry->ext_idx] = true; return NULL; } + + scontext->extension_enabled[entry->ext_idx] = true; return ext; } } -- 2.39.2 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/3] RISC-V: KVM: Introduce extension_enabled 2023-04-26 17:13 ` Andrew Jones @ 2023-05-19 15:05 ` Anup Patel -1 siblings, 0 replies; 20+ messages in thread From: Anup Patel @ 2023-05-19 15:05 UTC (permalink / raw) To: kvm-riscv On Wed, Apr 26, 2023 at 10:43?PM Andrew Jones <ajones@ventanamicro.com> wrote: > > We need three SBI extension status values (uninitialized, enabled, > and disabled). Pairing another boolean array, extension_enabled[], > with the boolean array extension_disabled[] provides four states. > Using a pair of boolean arrays, which may eventually be changed to > a pair of bitmaps, is more space efficient than using one enum > status field. Apply the new (enabled=1,disabled=0) state, which > means either the extension doesn't have a probe function or that > its probe was successful, to avoid more than one probe of the > extension. > > Signed-off-by: Andrew Jones <ajones@ventanamicro.com> > --- > arch/riscv/include/asm/kvm_vcpu_sbi.h | 9 +++++++++ > arch/riscv/kvm/vcpu_sbi.c | 11 ++++++++++- > 2 files changed, 19 insertions(+), 1 deletion(-) > > diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h > index 4278125a38a5..e3c5e1d15e93 100644 > --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h > +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h > @@ -16,6 +16,15 @@ > > struct kvm_vcpu_sbi_context { > int return_handled; > + /* > + * extension_enabled[] and extension_disabled[] provide SBI > + * extensions four status values, of which we need three: > + * (0,0) uninitialized, (1,0) enabled, (0,1) disabled. Using > + * a pair of boolean arrays, which may eventually be changed > + * to a pair of bitmaps, is more space efficient than using > + * one enum status field. > + */ > + bool extension_enabled[KVM_RISCV_SBI_EXT_MAX]; The extension_enabled[] is not an appropriate name. How about extension_probe_allowed[] ? Any better name ? Regards, Anup > bool extension_disabled[KVM_RISCV_SBI_EXT_MAX]; > }; > > diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c > index a1a82f0fbad2..344d38bbe06a 100644 > --- a/arch/riscv/kvm/vcpu_sbi.c > +++ b/arch/riscv/kvm/vcpu_sbi.c > @@ -155,6 +155,12 @@ static int riscv_vcpu_set_sbi_ext_single(struct kvm_vcpu *vcpu, > if (!sext) > return -ENOENT; > > + /* > + * We can't set scontext->extension_enabled[] to reg_val since the > + * extension may have a probe() function which needs to confirm > + * enablement first. Only set extension_disabled[] here and leave > + * the extension_enabled[] setting to kvm_vcpu_sbi_find_ext(). > + */ > scontext->extension_disabled[sext->ext_idx] = !reg_val; > > return 0; > @@ -317,7 +323,8 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext( > ext = entry->ext_ptr; > > if (ext->extid_start <= extid && ext->extid_end >= extid) { > - if (entry->ext_idx >= KVM_RISCV_SBI_EXT_MAX) > + if (entry->ext_idx >= KVM_RISCV_SBI_EXT_MAX || > + scontext->extension_enabled[entry->ext_idx]) > return ext; > if (scontext->extension_disabled[entry->ext_idx]) > return NULL; > @@ -325,6 +332,8 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext( > scontext->extension_disabled[entry->ext_idx] = true; > return NULL; > } > + > + scontext->extension_enabled[entry->ext_idx] = true; > return ext; > } > } > -- > 2.39.2 > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] RISC-V: KVM: Introduce extension_enabled @ 2023-05-19 15:05 ` Anup Patel 0 siblings, 0 replies; 20+ messages in thread From: Anup Patel @ 2023-05-19 15:05 UTC (permalink / raw) To: Andrew Jones Cc: kvm-riscv, linux-riscv, Palmer Dabbelt, Atish Patra, Albert Ou, Paul Walmsley On Wed, Apr 26, 2023 at 10:43 PM Andrew Jones <ajones@ventanamicro.com> wrote: > > We need three SBI extension status values (uninitialized, enabled, > and disabled). Pairing another boolean array, extension_enabled[], > with the boolean array extension_disabled[] provides four states. > Using a pair of boolean arrays, which may eventually be changed to > a pair of bitmaps, is more space efficient than using one enum > status field. Apply the new (enabled=1,disabled=0) state, which > means either the extension doesn't have a probe function or that > its probe was successful, to avoid more than one probe of the > extension. > > Signed-off-by: Andrew Jones <ajones@ventanamicro.com> > --- > arch/riscv/include/asm/kvm_vcpu_sbi.h | 9 +++++++++ > arch/riscv/kvm/vcpu_sbi.c | 11 ++++++++++- > 2 files changed, 19 insertions(+), 1 deletion(-) > > diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h > index 4278125a38a5..e3c5e1d15e93 100644 > --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h > +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h > @@ -16,6 +16,15 @@ > > struct kvm_vcpu_sbi_context { > int return_handled; > + /* > + * extension_enabled[] and extension_disabled[] provide SBI > + * extensions four status values, of which we need three: > + * (0,0) uninitialized, (1,0) enabled, (0,1) disabled. Using > + * a pair of boolean arrays, which may eventually be changed > + * to a pair of bitmaps, is more space efficient than using > + * one enum status field. > + */ > + bool extension_enabled[KVM_RISCV_SBI_EXT_MAX]; The extension_enabled[] is not an appropriate name. How about extension_probe_allowed[] ? Any better name ? Regards, Anup > bool extension_disabled[KVM_RISCV_SBI_EXT_MAX]; > }; > > diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c > index a1a82f0fbad2..344d38bbe06a 100644 > --- a/arch/riscv/kvm/vcpu_sbi.c > +++ b/arch/riscv/kvm/vcpu_sbi.c > @@ -155,6 +155,12 @@ static int riscv_vcpu_set_sbi_ext_single(struct kvm_vcpu *vcpu, > if (!sext) > return -ENOENT; > > + /* > + * We can't set scontext->extension_enabled[] to reg_val since the > + * extension may have a probe() function which needs to confirm > + * enablement first. Only set extension_disabled[] here and leave > + * the extension_enabled[] setting to kvm_vcpu_sbi_find_ext(). > + */ > scontext->extension_disabled[sext->ext_idx] = !reg_val; > > return 0; > @@ -317,7 +323,8 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext( > ext = entry->ext_ptr; > > if (ext->extid_start <= extid && ext->extid_end >= extid) { > - if (entry->ext_idx >= KVM_RISCV_SBI_EXT_MAX) > + if (entry->ext_idx >= KVM_RISCV_SBI_EXT_MAX || > + scontext->extension_enabled[entry->ext_idx]) > return ext; > if (scontext->extension_disabled[entry->ext_idx]) > return NULL; > @@ -325,6 +332,8 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext( > scontext->extension_disabled[entry->ext_idx] = true; > return NULL; > } > + > + scontext->extension_enabled[entry->ext_idx] = true; > return ext; > } > } > -- > 2.39.2 > _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 3/3] RISC-V: KVM: Introduce extension_enabled 2023-05-19 15:05 ` Anup Patel @ 2023-05-22 9:28 ` Andrew Jones -1 siblings, 0 replies; 20+ messages in thread From: Andrew Jones @ 2023-05-22 9:28 UTC (permalink / raw) To: kvm-riscv On Fri, May 19, 2023 at 08:35:31PM +0530, Anup Patel wrote: > On Wed, Apr 26, 2023 at 10:43?PM Andrew Jones <ajones@ventanamicro.com> wrote: > > > > We need three SBI extension status values (uninitialized, enabled, > > and disabled). Pairing another boolean array, extension_enabled[], > > with the boolean array extension_disabled[] provides four states. > > Using a pair of boolean arrays, which may eventually be changed to > > a pair of bitmaps, is more space efficient than using one enum > > status field. Apply the new (enabled=1,disabled=0) state, which > > means either the extension doesn't have a probe function or that > > its probe was successful, to avoid more than one probe of the > > extension. > > > > Signed-off-by: Andrew Jones <ajones@ventanamicro.com> > > --- > > arch/riscv/include/asm/kvm_vcpu_sbi.h | 9 +++++++++ > > arch/riscv/kvm/vcpu_sbi.c | 11 ++++++++++- > > 2 files changed, 19 insertions(+), 1 deletion(-) > > > > diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h > > index 4278125a38a5..e3c5e1d15e93 100644 > > --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h > > +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h > > @@ -16,6 +16,15 @@ > > > > struct kvm_vcpu_sbi_context { > > int return_handled; > > + /* > > + * extension_enabled[] and extension_disabled[] provide SBI > > + * extensions four status values, of which we need three: > > + * (0,0) uninitialized, (1,0) enabled, (0,1) disabled. Using > > + * a pair of boolean arrays, which may eventually be changed > > + * to a pair of bitmaps, is more space efficient than using > > + * one enum status field. > > + */ > > + bool extension_enabled[KVM_RISCV_SBI_EXT_MAX]; > > The extension_enabled[] is not an appropriate name. > > How about extension_probe_allowed[] ? "probe allowed" sounds to me like "we may call probe", but the meaning we want is "we've already called probe and it succeeded, so the extension is available" (or that we don't have probe, meaning the extension is assumed available). I prefer 'available' for extensions over 'enabled', but we already have 'disabled', so 'enabled' appeared to fit better. Actually, it's probably best to improve the readability and simplicity, at the expense of size, by dropping both the extension_enabled and extension_disabled boolean arrays and switching to an 'extension_status' enum, where we have SBI_EXT_UNINITIALIZED, SBI_EXT_AVAILABLE, and SBI_EXT_UNAVAILABLE. Thanks, drew > > Any better name ? > > Regards, > Anup > > > bool extension_disabled[KVM_RISCV_SBI_EXT_MAX]; > > }; > > > > diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c > > index a1a82f0fbad2..344d38bbe06a 100644 > > --- a/arch/riscv/kvm/vcpu_sbi.c > > +++ b/arch/riscv/kvm/vcpu_sbi.c > > @@ -155,6 +155,12 @@ static int riscv_vcpu_set_sbi_ext_single(struct kvm_vcpu *vcpu, > > if (!sext) > > return -ENOENT; > > > > + /* > > + * We can't set scontext->extension_enabled[] to reg_val since the > > + * extension may have a probe() function which needs to confirm > > + * enablement first. Only set extension_disabled[] here and leave > > + * the extension_enabled[] setting to kvm_vcpu_sbi_find_ext(). > > + */ > > scontext->extension_disabled[sext->ext_idx] = !reg_val; > > > > return 0; > > @@ -317,7 +323,8 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext( > > ext = entry->ext_ptr; > > > > if (ext->extid_start <= extid && ext->extid_end >= extid) { > > - if (entry->ext_idx >= KVM_RISCV_SBI_EXT_MAX) > > + if (entry->ext_idx >= KVM_RISCV_SBI_EXT_MAX || > > + scontext->extension_enabled[entry->ext_idx]) > > return ext; > > if (scontext->extension_disabled[entry->ext_idx]) > > return NULL; > > @@ -325,6 +332,8 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext( > > scontext->extension_disabled[entry->ext_idx] = true; > > return NULL; > > } > > + > > + scontext->extension_enabled[entry->ext_idx] = true; > > return ext; > > } > > } > > -- > > 2.39.2 > > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] RISC-V: KVM: Introduce extension_enabled @ 2023-05-22 9:28 ` Andrew Jones 0 siblings, 0 replies; 20+ messages in thread From: Andrew Jones @ 2023-05-22 9:28 UTC (permalink / raw) To: Anup Patel Cc: kvm-riscv, linux-riscv, Palmer Dabbelt, Atish Patra, Albert Ou, Paul Walmsley On Fri, May 19, 2023 at 08:35:31PM +0530, Anup Patel wrote: > On Wed, Apr 26, 2023 at 10:43 PM Andrew Jones <ajones@ventanamicro.com> wrote: > > > > We need three SBI extension status values (uninitialized, enabled, > > and disabled). Pairing another boolean array, extension_enabled[], > > with the boolean array extension_disabled[] provides four states. > > Using a pair of boolean arrays, which may eventually be changed to > > a pair of bitmaps, is more space efficient than using one enum > > status field. Apply the new (enabled=1,disabled=0) state, which > > means either the extension doesn't have a probe function or that > > its probe was successful, to avoid more than one probe of the > > extension. > > > > Signed-off-by: Andrew Jones <ajones@ventanamicro.com> > > --- > > arch/riscv/include/asm/kvm_vcpu_sbi.h | 9 +++++++++ > > arch/riscv/kvm/vcpu_sbi.c | 11 ++++++++++- > > 2 files changed, 19 insertions(+), 1 deletion(-) > > > > diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h > > index 4278125a38a5..e3c5e1d15e93 100644 > > --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h > > +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h > > @@ -16,6 +16,15 @@ > > > > struct kvm_vcpu_sbi_context { > > int return_handled; > > + /* > > + * extension_enabled[] and extension_disabled[] provide SBI > > + * extensions four status values, of which we need three: > > + * (0,0) uninitialized, (1,0) enabled, (0,1) disabled. Using > > + * a pair of boolean arrays, which may eventually be changed > > + * to a pair of bitmaps, is more space efficient than using > > + * one enum status field. > > + */ > > + bool extension_enabled[KVM_RISCV_SBI_EXT_MAX]; > > The extension_enabled[] is not an appropriate name. > > How about extension_probe_allowed[] ? "probe allowed" sounds to me like "we may call probe", but the meaning we want is "we've already called probe and it succeeded, so the extension is available" (or that we don't have probe, meaning the extension is assumed available). I prefer 'available' for extensions over 'enabled', but we already have 'disabled', so 'enabled' appeared to fit better. Actually, it's probably best to improve the readability and simplicity, at the expense of size, by dropping both the extension_enabled and extension_disabled boolean arrays and switching to an 'extension_status' enum, where we have SBI_EXT_UNINITIALIZED, SBI_EXT_AVAILABLE, and SBI_EXT_UNAVAILABLE. Thanks, drew > > Any better name ? > > Regards, > Anup > > > bool extension_disabled[KVM_RISCV_SBI_EXT_MAX]; > > }; > > > > diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c > > index a1a82f0fbad2..344d38bbe06a 100644 > > --- a/arch/riscv/kvm/vcpu_sbi.c > > +++ b/arch/riscv/kvm/vcpu_sbi.c > > @@ -155,6 +155,12 @@ static int riscv_vcpu_set_sbi_ext_single(struct kvm_vcpu *vcpu, > > if (!sext) > > return -ENOENT; > > > > + /* > > + * We can't set scontext->extension_enabled[] to reg_val since the > > + * extension may have a probe() function which needs to confirm > > + * enablement first. Only set extension_disabled[] here and leave > > + * the extension_enabled[] setting to kvm_vcpu_sbi_find_ext(). > > + */ > > scontext->extension_disabled[sext->ext_idx] = !reg_val; > > > > return 0; > > @@ -317,7 +323,8 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext( > > ext = entry->ext_ptr; > > > > if (ext->extid_start <= extid && ext->extid_end >= extid) { > > - if (entry->ext_idx >= KVM_RISCV_SBI_EXT_MAX) > > + if (entry->ext_idx >= KVM_RISCV_SBI_EXT_MAX || > > + scontext->extension_enabled[entry->ext_idx]) > > return ext; > > if (scontext->extension_disabled[entry->ext_idx]) > > return NULL; > > @@ -325,6 +332,8 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext( > > scontext->extension_disabled[entry->ext_idx] = true; > > return NULL; > > } > > + > > + scontext->extension_enabled[entry->ext_idx] = true; > > return ext; > > } > > } > > -- > > 2.39.2 > > _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 3/3] RISC-V: KVM: Introduce extension_enabled 2023-05-22 9:28 ` Andrew Jones @ 2023-05-25 15:58 ` Anup Patel -1 siblings, 0 replies; 20+ messages in thread From: Anup Patel @ 2023-05-25 15:58 UTC (permalink / raw) To: kvm-riscv On Mon, May 22, 2023 at 2:59?PM Andrew Jones <ajones@ventanamicro.com> wrote: > > On Fri, May 19, 2023 at 08:35:31PM +0530, Anup Patel wrote: > > On Wed, Apr 26, 2023 at 10:43?PM Andrew Jones <ajones@ventanamicro.com> wrote: > > > > > > We need three SBI extension status values (uninitialized, enabled, > > > and disabled). Pairing another boolean array, extension_enabled[], > > > with the boolean array extension_disabled[] provides four states. > > > Using a pair of boolean arrays, which may eventually be changed to > > > a pair of bitmaps, is more space efficient than using one enum > > > status field. Apply the new (enabled=1,disabled=0) state, which > > > means either the extension doesn't have a probe function or that > > > its probe was successful, to avoid more than one probe of the > > > extension. > > > > > > Signed-off-by: Andrew Jones <ajones@ventanamicro.com> > > > --- > > > arch/riscv/include/asm/kvm_vcpu_sbi.h | 9 +++++++++ > > > arch/riscv/kvm/vcpu_sbi.c | 11 ++++++++++- > > > 2 files changed, 19 insertions(+), 1 deletion(-) > > > > > > diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h > > > index 4278125a38a5..e3c5e1d15e93 100644 > > > --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h > > > +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h > > > @@ -16,6 +16,15 @@ > > > > > > struct kvm_vcpu_sbi_context { > > > int return_handled; > > > + /* > > > + * extension_enabled[] and extension_disabled[] provide SBI > > > + * extensions four status values, of which we need three: > > > + * (0,0) uninitialized, (1,0) enabled, (0,1) disabled. Using > > > + * a pair of boolean arrays, which may eventually be changed > > > + * to a pair of bitmaps, is more space efficient than using > > > + * one enum status field. > > > + */ > > > + bool extension_enabled[KVM_RISCV_SBI_EXT_MAX]; > > > > The extension_enabled[] is not an appropriate name. > > > > How about extension_probe_allowed[] ? > > "probe allowed" sounds to me like "we may call probe", but the meaning we > want is "we've already called probe and it succeeded, so the extension > is available" (or that we don't have probe, meaning the extension is > assumed available). I prefer 'available' for extensions over 'enabled', > but we already have 'disabled', so 'enabled' appeared to fit better. > Actually, it's probably best to improve the readability and simplicity, > at the expense of size, by dropping both the extension_enabled and > extension_disabled boolean arrays and switching to an 'extension_status' > enum, where we have SBI_EXT_UNINITIALIZED, SBI_EXT_AVAILABLE, and > SBI_EXT_UNAVAILABLE. I agree. Let's switch to enum. Regards, Anup > > Thanks, > drew > > > > > > Any better name ? > > > > Regards, > > Anup > > > > > bool extension_disabled[KVM_RISCV_SBI_EXT_MAX]; > > > }; > > > > > > diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c > > > index a1a82f0fbad2..344d38bbe06a 100644 > > > --- a/arch/riscv/kvm/vcpu_sbi.c > > > +++ b/arch/riscv/kvm/vcpu_sbi.c > > > @@ -155,6 +155,12 @@ static int riscv_vcpu_set_sbi_ext_single(struct kvm_vcpu *vcpu, > > > if (!sext) > > > return -ENOENT; > > > > > > + /* > > > + * We can't set scontext->extension_enabled[] to reg_val since the > > > + * extension may have a probe() function which needs to confirm > > > + * enablement first. Only set extension_disabled[] here and leave > > > + * the extension_enabled[] setting to kvm_vcpu_sbi_find_ext(). > > > + */ > > > scontext->extension_disabled[sext->ext_idx] = !reg_val; > > > > > > return 0; > > > @@ -317,7 +323,8 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext( > > > ext = entry->ext_ptr; > > > > > > if (ext->extid_start <= extid && ext->extid_end >= extid) { > > > - if (entry->ext_idx >= KVM_RISCV_SBI_EXT_MAX) > > > + if (entry->ext_idx >= KVM_RISCV_SBI_EXT_MAX || > > > + scontext->extension_enabled[entry->ext_idx]) > > > return ext; > > > if (scontext->extension_disabled[entry->ext_idx]) > > > return NULL; > > > @@ -325,6 +332,8 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext( > > > scontext->extension_disabled[entry->ext_idx] = true; > > > return NULL; > > > } > > > + > > > + scontext->extension_enabled[entry->ext_idx] = true; > > > return ext; > > > } > > > } > > > -- > > > 2.39.2 > > > > > -- > kvm-riscv mailing list > kvm-riscv at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/kvm-riscv ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] RISC-V: KVM: Introduce extension_enabled @ 2023-05-25 15:58 ` Anup Patel 0 siblings, 0 replies; 20+ messages in thread From: Anup Patel @ 2023-05-25 15:58 UTC (permalink / raw) To: Andrew Jones Cc: Anup Patel, kvm-riscv, linux-riscv, Palmer Dabbelt, Atish Patra, Albert Ou, Paul Walmsley On Mon, May 22, 2023 at 2:59 PM Andrew Jones <ajones@ventanamicro.com> wrote: > > On Fri, May 19, 2023 at 08:35:31PM +0530, Anup Patel wrote: > > On Wed, Apr 26, 2023 at 10:43 PM Andrew Jones <ajones@ventanamicro.com> wrote: > > > > > > We need three SBI extension status values (uninitialized, enabled, > > > and disabled). Pairing another boolean array, extension_enabled[], > > > with the boolean array extension_disabled[] provides four states. > > > Using a pair of boolean arrays, which may eventually be changed to > > > a pair of bitmaps, is more space efficient than using one enum > > > status field. Apply the new (enabled=1,disabled=0) state, which > > > means either the extension doesn't have a probe function or that > > > its probe was successful, to avoid more than one probe of the > > > extension. > > > > > > Signed-off-by: Andrew Jones <ajones@ventanamicro.com> > > > --- > > > arch/riscv/include/asm/kvm_vcpu_sbi.h | 9 +++++++++ > > > arch/riscv/kvm/vcpu_sbi.c | 11 ++++++++++- > > > 2 files changed, 19 insertions(+), 1 deletion(-) > > > > > > diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h > > > index 4278125a38a5..e3c5e1d15e93 100644 > > > --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h > > > +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h > > > @@ -16,6 +16,15 @@ > > > > > > struct kvm_vcpu_sbi_context { > > > int return_handled; > > > + /* > > > + * extension_enabled[] and extension_disabled[] provide SBI > > > + * extensions four status values, of which we need three: > > > + * (0,0) uninitialized, (1,0) enabled, (0,1) disabled. Using > > > + * a pair of boolean arrays, which may eventually be changed > > > + * to a pair of bitmaps, is more space efficient than using > > > + * one enum status field. > > > + */ > > > + bool extension_enabled[KVM_RISCV_SBI_EXT_MAX]; > > > > The extension_enabled[] is not an appropriate name. > > > > How about extension_probe_allowed[] ? > > "probe allowed" sounds to me like "we may call probe", but the meaning we > want is "we've already called probe and it succeeded, so the extension > is available" (or that we don't have probe, meaning the extension is > assumed available). I prefer 'available' for extensions over 'enabled', > but we already have 'disabled', so 'enabled' appeared to fit better. > Actually, it's probably best to improve the readability and simplicity, > at the expense of size, by dropping both the extension_enabled and > extension_disabled boolean arrays and switching to an 'extension_status' > enum, where we have SBI_EXT_UNINITIALIZED, SBI_EXT_AVAILABLE, and > SBI_EXT_UNAVAILABLE. I agree. Let's switch to enum. Regards, Anup > > Thanks, > drew > > > > > > Any better name ? > > > > Regards, > > Anup > > > > > bool extension_disabled[KVM_RISCV_SBI_EXT_MAX]; > > > }; > > > > > > diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c > > > index a1a82f0fbad2..344d38bbe06a 100644 > > > --- a/arch/riscv/kvm/vcpu_sbi.c > > > +++ b/arch/riscv/kvm/vcpu_sbi.c > > > @@ -155,6 +155,12 @@ static int riscv_vcpu_set_sbi_ext_single(struct kvm_vcpu *vcpu, > > > if (!sext) > > > return -ENOENT; > > > > > > + /* > > > + * We can't set scontext->extension_enabled[] to reg_val since the > > > + * extension may have a probe() function which needs to confirm > > > + * enablement first. Only set extension_disabled[] here and leave > > > + * the extension_enabled[] setting to kvm_vcpu_sbi_find_ext(). > > > + */ > > > scontext->extension_disabled[sext->ext_idx] = !reg_val; > > > > > > return 0; > > > @@ -317,7 +323,8 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext( > > > ext = entry->ext_ptr; > > > > > > if (ext->extid_start <= extid && ext->extid_end >= extid) { > > > - if (entry->ext_idx >= KVM_RISCV_SBI_EXT_MAX) > > > + if (entry->ext_idx >= KVM_RISCV_SBI_EXT_MAX || > > > + scontext->extension_enabled[entry->ext_idx]) > > > return ext; > > > if (scontext->extension_disabled[entry->ext_idx]) > > > return NULL; > > > @@ -325,6 +332,8 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext( > > > scontext->extension_disabled[entry->ext_idx] = true; > > > return NULL; > > > } > > > + > > > + scontext->extension_enabled[entry->ext_idx] = true; > > > return ext; > > > } > > > } > > > -- > > > 2.39.2 > > > > > -- > kvm-riscv mailing list > kvm-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/kvm-riscv _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2023-05-25 15:58 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-04-26 17:13 [PATCH 0/3] RISC-V: KVM: Ensure SBI extension is enabled Andrew Jones 2023-04-26 17:13 ` Andrew Jones 2023-04-26 17:13 ` [PATCH 1/3] RISC-V: KVM: Disable SBI extension when its probe fails Andrew Jones 2023-04-26 17:13 ` Andrew Jones 2023-05-19 14:57 ` Anup Patel 2023-05-19 14:57 ` Anup Patel 2023-05-19 15:02 ` Anup Patel 2023-05-19 15:02 ` Anup Patel 2023-04-26 17:13 ` [PATCH 2/3] RISC-V: KVM: Rename dis_idx to ext_idx Andrew Jones 2023-04-26 17:13 ` Andrew Jones 2023-05-19 14:58 ` Anup Patel 2023-05-19 14:58 ` Anup Patel 2023-04-26 17:13 ` [PATCH 3/3] RISC-V: KVM: Introduce extension_enabled Andrew Jones 2023-04-26 17:13 ` Andrew Jones 2023-05-19 15:05 ` Anup Patel 2023-05-19 15:05 ` Anup Patel 2023-05-22 9:28 ` Andrew Jones 2023-05-22 9:28 ` Andrew Jones 2023-05-25 15:58 ` Anup Patel 2023-05-25 15:58 ` 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.