| Message ID | 20230613205857.495165-16-dbarboza@ventanamicro.com |
|---|---|
| State | New |
| Headers | show |
| Series | target/riscv, KVM: fixes and enhancements | expand |
On Tue, Jun 13, 2023 at 05:58:54PM -0300, Daniel Henrique Barboza wrote: > The isa_edata_arr[] is used by riscv_isa_string_ext() to create the > riscv,isa DT attribute. isa_edata_arr[] is kept in sync with the TCG > property vector riscv_cpu_extensions[], i.e. all TCG properties from > this vector that has a riscv,isa representation are included in > isa_edata_arr[]. > > KVM doesn't implement all TCG properties, but allow them to be created > anyway to not force an API change between TCG and KVM guests. Some of > these TCG-only extensions are defaulted to 'true', and users are also > allowed to enable them. KVM doesn't care, but riscv_isa_string_ext() > does. The result is that these TCG-only enabled extensions will appear > in the riscv,isa DT string under KVM. > > To avoid code repetition and re-use riscv_isa_string_ext() for KVM > guests we'll make a couple of tweaks: > > - set env->priv_ver to 'LATEST' for the KVM 'host' CPU. This is needed > because riscv_isa_string_ext() makes a priv check for each extension > before including them in the ISA string. KVM doesn't care about > env->priv_ver, since it's part of the TCG-only CPU validation, so this > change is benign for KVM; > > - add a new 'kvm_available' flag in isa_ext_data struct. This flag is > set via a new ISA_EXT_DATA_ENTRY_KVM macro to report that, for a given > extension, KVM also supports it. riscv_isa_string_ext() then can check > if a given extension is known by KVM and skip it if it's not. > > This will allow us to re-use riscv_isa_string_ext() for KVM guests. > > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > --- > target/riscv/cpu.c | 28 ++++++++++++++++++++-------- > 1 file changed, 20 insertions(+), 8 deletions(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index a4f3ed0c17..a773c09645 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -44,11 +44,15 @@ struct isa_ext_data { > const char *name; > int min_version; > int ext_enable_offset; > + bool kvm_available; > }; > > #define ISA_EXT_DATA_ENTRY(_name, _min_ver, _prop) \ > {#_name, _min_ver, offsetof(struct RISCVCPUConfig, _prop)} > > +#define ISA_EXT_DATA_ENTRY_KVM(_name, _min_ver, _prop) \ > + {#_name, _min_ver, offsetof(struct RISCVCPUConfig, _prop), true} > + > /* > * Here are the ordering rules of extension naming defined by RISC-V > * specification : > @@ -68,14 +72,17 @@ struct isa_ext_data { > * > * Single letter extensions are checked in riscv_cpu_validate_misa_priv() > * instead. > + * > + * ISA_EXT_DATA_ENTRY_KVM() is used to indicate that the extension is > + * also known by the KVM driver. If unsure, use ISA_EXT_DATA_ENTRY(). > */ > static const struct isa_ext_data isa_edata_arr[] = { > - ISA_EXT_DATA_ENTRY(zicbom, PRIV_VERSION_1_12_0, ext_icbom), > - ISA_EXT_DATA_ENTRY(zicboz, PRIV_VERSION_1_12_0, ext_icboz), > + ISA_EXT_DATA_ENTRY_KVM(zicbom, PRIV_VERSION_1_12_0, ext_icbom), > + ISA_EXT_DATA_ENTRY_KVM(zicboz, PRIV_VERSION_1_12_0, ext_icboz), > ISA_EXT_DATA_ENTRY(zicond, PRIV_VERSION_1_12_0, ext_zicond), > ISA_EXT_DATA_ENTRY(zicsr, PRIV_VERSION_1_10_0, ext_icsr), > ISA_EXT_DATA_ENTRY(zifencei, PRIV_VERSION_1_10_0, ext_ifencei), > - ISA_EXT_DATA_ENTRY(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause), > + ISA_EXT_DATA_ENTRY_KVM(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause), > ISA_EXT_DATA_ENTRY(zawrs, PRIV_VERSION_1_12_0, ext_zawrs), > ISA_EXT_DATA_ENTRY(zfh, PRIV_VERSION_1_11_0, ext_zfh), > ISA_EXT_DATA_ENTRY(zfhmin, PRIV_VERSION_1_11_0, ext_zfhmin), > @@ -89,7 +96,7 @@ static const struct isa_ext_data isa_edata_arr[] = { > ISA_EXT_DATA_ENTRY(zcmp, PRIV_VERSION_1_12_0, ext_zcmp), > ISA_EXT_DATA_ENTRY(zcmt, PRIV_VERSION_1_12_0, ext_zcmt), > ISA_EXT_DATA_ENTRY(zba, PRIV_VERSION_1_12_0, ext_zba), > - ISA_EXT_DATA_ENTRY(zbb, PRIV_VERSION_1_12_0, ext_zbb), > + ISA_EXT_DATA_ENTRY_KVM(zbb, PRIV_VERSION_1_12_0, ext_zbb), > ISA_EXT_DATA_ENTRY(zbc, PRIV_VERSION_1_12_0, ext_zbc), > ISA_EXT_DATA_ENTRY(zbkb, PRIV_VERSION_1_12_0, ext_zbkb), > ISA_EXT_DATA_ENTRY(zbkc, PRIV_VERSION_1_12_0, ext_zbkc), > @@ -114,13 +121,13 @@ static const struct isa_ext_data isa_edata_arr[] = { > ISA_EXT_DATA_ENTRY(zhinxmin, PRIV_VERSION_1_12_0, ext_zhinxmin), > ISA_EXT_DATA_ENTRY(smaia, PRIV_VERSION_1_12_0, ext_smaia), > ISA_EXT_DATA_ENTRY(smstateen, PRIV_VERSION_1_12_0, ext_smstateen), > - ISA_EXT_DATA_ENTRY(ssaia, PRIV_VERSION_1_12_0, ext_ssaia), > + ISA_EXT_DATA_ENTRY_KVM(ssaia, PRIV_VERSION_1_12_0, ext_ssaia), > ISA_EXT_DATA_ENTRY(sscofpmf, PRIV_VERSION_1_12_0, ext_sscofpmf), > - ISA_EXT_DATA_ENTRY(sstc, PRIV_VERSION_1_12_0, ext_sstc), > + ISA_EXT_DATA_ENTRY_KVM(sstc, PRIV_VERSION_1_12_0, ext_sstc), > ISA_EXT_DATA_ENTRY(svadu, PRIV_VERSION_1_12_0, ext_svadu), > - ISA_EXT_DATA_ENTRY(svinval, PRIV_VERSION_1_12_0, ext_svinval), > + ISA_EXT_DATA_ENTRY_KVM(svinval, PRIV_VERSION_1_12_0, ext_svinval), > ISA_EXT_DATA_ENTRY(svnapot, PRIV_VERSION_1_12_0, ext_svnapot), > - ISA_EXT_DATA_ENTRY(svpbmt, PRIV_VERSION_1_12_0, ext_svpbmt), > + ISA_EXT_DATA_ENTRY_KVM(svpbmt, PRIV_VERSION_1_12_0, ext_svpbmt), This approach looks a bit difficult to maintain (it's hard to even see the _KVM macro uses). The approach will be even more difficult if we add more accelerators. I feel like we need an extension class where objects of that class can be passed to functions like riscv_isa_string_ext(). And then we also need tcg-extension and kvm-extension classes which inherit from the extension class. We can then keep the lists of extensions separate, as you originally proposed, as each list will be of its own type. Thanks, drew
On 6/19/23 06:54, Andrew Jones wrote: > On Tue, Jun 13, 2023 at 05:58:54PM -0300, Daniel Henrique Barboza wrote: >> The isa_edata_arr[] is used by riscv_isa_string_ext() to create the >> riscv,isa DT attribute. isa_edata_arr[] is kept in sync with the TCG >> property vector riscv_cpu_extensions[], i.e. all TCG properties from >> this vector that has a riscv,isa representation are included in >> isa_edata_arr[]. >> >> KVM doesn't implement all TCG properties, but allow them to be created >> anyway to not force an API change between TCG and KVM guests. Some of >> these TCG-only extensions are defaulted to 'true', and users are also >> allowed to enable them. KVM doesn't care, but riscv_isa_string_ext() >> does. The result is that these TCG-only enabled extensions will appear >> in the riscv,isa DT string under KVM. >> >> To avoid code repetition and re-use riscv_isa_string_ext() for KVM >> guests we'll make a couple of tweaks: >> >> - set env->priv_ver to 'LATEST' for the KVM 'host' CPU. This is needed >> because riscv_isa_string_ext() makes a priv check for each extension >> before including them in the ISA string. KVM doesn't care about >> env->priv_ver, since it's part of the TCG-only CPU validation, so this >> change is benign for KVM; >> >> - add a new 'kvm_available' flag in isa_ext_data struct. This flag is >> set via a new ISA_EXT_DATA_ENTRY_KVM macro to report that, for a given >> extension, KVM also supports it. riscv_isa_string_ext() then can check >> if a given extension is known by KVM and skip it if it's not. >> >> This will allow us to re-use riscv_isa_string_ext() for KVM guests. >> >> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> >> --- >> target/riscv/cpu.c | 28 ++++++++++++++++++++-------- >> 1 file changed, 20 insertions(+), 8 deletions(-) >> >> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c >> index a4f3ed0c17..a773c09645 100644 >> --- a/target/riscv/cpu.c >> +++ b/target/riscv/cpu.c >> @@ -44,11 +44,15 @@ struct isa_ext_data { >> const char *name; >> int min_version; >> int ext_enable_offset; >> + bool kvm_available; >> }; >> >> #define ISA_EXT_DATA_ENTRY(_name, _min_ver, _prop) \ >> {#_name, _min_ver, offsetof(struct RISCVCPUConfig, _prop)} >> >> +#define ISA_EXT_DATA_ENTRY_KVM(_name, _min_ver, _prop) \ >> + {#_name, _min_ver, offsetof(struct RISCVCPUConfig, _prop), true} >> + >> /* >> * Here are the ordering rules of extension naming defined by RISC-V >> * specification : >> @@ -68,14 +72,17 @@ struct isa_ext_data { >> * >> * Single letter extensions are checked in riscv_cpu_validate_misa_priv() >> * instead. >> + * >> + * ISA_EXT_DATA_ENTRY_KVM() is used to indicate that the extension is >> + * also known by the KVM driver. If unsure, use ISA_EXT_DATA_ENTRY(). >> */ >> static const struct isa_ext_data isa_edata_arr[] = { >> - ISA_EXT_DATA_ENTRY(zicbom, PRIV_VERSION_1_12_0, ext_icbom), >> - ISA_EXT_DATA_ENTRY(zicboz, PRIV_VERSION_1_12_0, ext_icboz), >> + ISA_EXT_DATA_ENTRY_KVM(zicbom, PRIV_VERSION_1_12_0, ext_icbom), >> + ISA_EXT_DATA_ENTRY_KVM(zicboz, PRIV_VERSION_1_12_0, ext_icboz), >> ISA_EXT_DATA_ENTRY(zicond, PRIV_VERSION_1_12_0, ext_zicond), >> ISA_EXT_DATA_ENTRY(zicsr, PRIV_VERSION_1_10_0, ext_icsr), >> ISA_EXT_DATA_ENTRY(zifencei, PRIV_VERSION_1_10_0, ext_ifencei), >> - ISA_EXT_DATA_ENTRY(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause), >> + ISA_EXT_DATA_ENTRY_KVM(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause), >> ISA_EXT_DATA_ENTRY(zawrs, PRIV_VERSION_1_12_0, ext_zawrs), >> ISA_EXT_DATA_ENTRY(zfh, PRIV_VERSION_1_11_0, ext_zfh), >> ISA_EXT_DATA_ENTRY(zfhmin, PRIV_VERSION_1_11_0, ext_zfhmin), >> @@ -89,7 +96,7 @@ static const struct isa_ext_data isa_edata_arr[] = { >> ISA_EXT_DATA_ENTRY(zcmp, PRIV_VERSION_1_12_0, ext_zcmp), >> ISA_EXT_DATA_ENTRY(zcmt, PRIV_VERSION_1_12_0, ext_zcmt), >> ISA_EXT_DATA_ENTRY(zba, PRIV_VERSION_1_12_0, ext_zba), >> - ISA_EXT_DATA_ENTRY(zbb, PRIV_VERSION_1_12_0, ext_zbb), >> + ISA_EXT_DATA_ENTRY_KVM(zbb, PRIV_VERSION_1_12_0, ext_zbb), >> ISA_EXT_DATA_ENTRY(zbc, PRIV_VERSION_1_12_0, ext_zbc), >> ISA_EXT_DATA_ENTRY(zbkb, PRIV_VERSION_1_12_0, ext_zbkb), >> ISA_EXT_DATA_ENTRY(zbkc, PRIV_VERSION_1_12_0, ext_zbkc), >> @@ -114,13 +121,13 @@ static const struct isa_ext_data isa_edata_arr[] = { >> ISA_EXT_DATA_ENTRY(zhinxmin, PRIV_VERSION_1_12_0, ext_zhinxmin), >> ISA_EXT_DATA_ENTRY(smaia, PRIV_VERSION_1_12_0, ext_smaia), >> ISA_EXT_DATA_ENTRY(smstateen, PRIV_VERSION_1_12_0, ext_smstateen), >> - ISA_EXT_DATA_ENTRY(ssaia, PRIV_VERSION_1_12_0, ext_ssaia), >> + ISA_EXT_DATA_ENTRY_KVM(ssaia, PRIV_VERSION_1_12_0, ext_ssaia), >> ISA_EXT_DATA_ENTRY(sscofpmf, PRIV_VERSION_1_12_0, ext_sscofpmf), >> - ISA_EXT_DATA_ENTRY(sstc, PRIV_VERSION_1_12_0, ext_sstc), >> + ISA_EXT_DATA_ENTRY_KVM(sstc, PRIV_VERSION_1_12_0, ext_sstc), >> ISA_EXT_DATA_ENTRY(svadu, PRIV_VERSION_1_12_0, ext_svadu), >> - ISA_EXT_DATA_ENTRY(svinval, PRIV_VERSION_1_12_0, ext_svinval), >> + ISA_EXT_DATA_ENTRY_KVM(svinval, PRIV_VERSION_1_12_0, ext_svinval), >> ISA_EXT_DATA_ENTRY(svnapot, PRIV_VERSION_1_12_0, ext_svnapot), >> - ISA_EXT_DATA_ENTRY(svpbmt, PRIV_VERSION_1_12_0, ext_svpbmt), >> + ISA_EXT_DATA_ENTRY_KVM(svpbmt, PRIV_VERSION_1_12_0, ext_svpbmt), > > This approach looks a bit difficult to maintain (it's hard to even see the > _KVM macro uses). The approach will be even more difficult if we add more > accelerators. I feel like we need an extension class where objects of that > class can be passed to functions like riscv_isa_string_ext(). And then we > also need tcg-extension and kvm-extension classes which inherit from > the extension class. We can then keep the lists of extensions separate, as > you originally proposed, as each list will be of its own type. So, after coding around a little, I didn't manage to create a KVM list in kvm.c file because of how ARRAY_SIZE() (a macro that riscv_isa_string_ext()) calculates the array size. If the list is exported from another file the macro fails to calculate the size of the array. Similar issues also happens when trying to use kvm_multi_ext_cfgs[] in this function. This means that both tcg and kvm arrays ended up in cpu.c. The tcg array is left untouched (isa_edata_arr). The KVM array uses the same type TCG already uses (isa_ext_edata) because it's good enough for KVM as well. I removed the 'kvm_available' flag since we're going to manually edit the array. riscv_isa_string_ext() is then changed to switch between the tcg/kvm array as required. The rest of the logic of the function stood the same. I'll also add a pre-patch prior to all these changes to simplify riscv_isa_string_ext() a bit when running under TCG. In the end, as the code stands today, the only way to use this function with the array we already have in KVM (kvm_multi_ext_cfgs[]) requires way more changes than I believe it is worth doing ATM. I'd say that the extra kvm array in cpu.c is an okay price to pay to keep the code in the simple side. Thanks, Daniel > > Thanks, > drew
On Tue, Jun 20, 2023 at 07:05:18PM -0300, Daniel Henrique Barboza wrote: > > > On 6/19/23 06:54, Andrew Jones wrote: > > On Tue, Jun 13, 2023 at 05:58:54PM -0300, Daniel Henrique Barboza wrote: > > > The isa_edata_arr[] is used by riscv_isa_string_ext() to create the > > > riscv,isa DT attribute. isa_edata_arr[] is kept in sync with the TCG > > > property vector riscv_cpu_extensions[], i.e. all TCG properties from > > > this vector that has a riscv,isa representation are included in > > > isa_edata_arr[]. > > > > > > KVM doesn't implement all TCG properties, but allow them to be created > > > anyway to not force an API change between TCG and KVM guests. Some of > > > these TCG-only extensions are defaulted to 'true', and users are also > > > allowed to enable them. KVM doesn't care, but riscv_isa_string_ext() > > > does. The result is that these TCG-only enabled extensions will appear > > > in the riscv,isa DT string under KVM. > > > > > > To avoid code repetition and re-use riscv_isa_string_ext() for KVM > > > guests we'll make a couple of tweaks: > > > > > > - set env->priv_ver to 'LATEST' for the KVM 'host' CPU. This is needed > > > because riscv_isa_string_ext() makes a priv check for each extension > > > before including them in the ISA string. KVM doesn't care about > > > env->priv_ver, since it's part of the TCG-only CPU validation, so this > > > change is benign for KVM; > > > > > > - add a new 'kvm_available' flag in isa_ext_data struct. This flag is > > > set via a new ISA_EXT_DATA_ENTRY_KVM macro to report that, for a given > > > extension, KVM also supports it. riscv_isa_string_ext() then can check > > > if a given extension is known by KVM and skip it if it's not. > > > > > > This will allow us to re-use riscv_isa_string_ext() for KVM guests. > > > > > > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > > > --- > > > target/riscv/cpu.c | 28 ++++++++++++++++++++-------- > > > 1 file changed, 20 insertions(+), 8 deletions(-) > > > > > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > > > index a4f3ed0c17..a773c09645 100644 > > > --- a/target/riscv/cpu.c > > > +++ b/target/riscv/cpu.c > > > @@ -44,11 +44,15 @@ struct isa_ext_data { > > > const char *name; > > > int min_version; > > > int ext_enable_offset; > > > + bool kvm_available; > > > }; > > > #define ISA_EXT_DATA_ENTRY(_name, _min_ver, _prop) \ > > > {#_name, _min_ver, offsetof(struct RISCVCPUConfig, _prop)} > > > +#define ISA_EXT_DATA_ENTRY_KVM(_name, _min_ver, _prop) \ > > > + {#_name, _min_ver, offsetof(struct RISCVCPUConfig, _prop), true} > > > + > > > /* > > > * Here are the ordering rules of extension naming defined by RISC-V > > > * specification : > > > @@ -68,14 +72,17 @@ struct isa_ext_data { > > > * > > > * Single letter extensions are checked in riscv_cpu_validate_misa_priv() > > > * instead. > > > + * > > > + * ISA_EXT_DATA_ENTRY_KVM() is used to indicate that the extension is > > > + * also known by the KVM driver. If unsure, use ISA_EXT_DATA_ENTRY(). > > > */ > > > static const struct isa_ext_data isa_edata_arr[] = { > > > - ISA_EXT_DATA_ENTRY(zicbom, PRIV_VERSION_1_12_0, ext_icbom), > > > - ISA_EXT_DATA_ENTRY(zicboz, PRIV_VERSION_1_12_0, ext_icboz), > > > + ISA_EXT_DATA_ENTRY_KVM(zicbom, PRIV_VERSION_1_12_0, ext_icbom), > > > + ISA_EXT_DATA_ENTRY_KVM(zicboz, PRIV_VERSION_1_12_0, ext_icboz), > > > ISA_EXT_DATA_ENTRY(zicond, PRIV_VERSION_1_12_0, ext_zicond), > > > ISA_EXT_DATA_ENTRY(zicsr, PRIV_VERSION_1_10_0, ext_icsr), > > > ISA_EXT_DATA_ENTRY(zifencei, PRIV_VERSION_1_10_0, ext_ifencei), > > > - ISA_EXT_DATA_ENTRY(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause), > > > + ISA_EXT_DATA_ENTRY_KVM(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause), > > > ISA_EXT_DATA_ENTRY(zawrs, PRIV_VERSION_1_12_0, ext_zawrs), > > > ISA_EXT_DATA_ENTRY(zfh, PRIV_VERSION_1_11_0, ext_zfh), > > > ISA_EXT_DATA_ENTRY(zfhmin, PRIV_VERSION_1_11_0, ext_zfhmin), > > > @@ -89,7 +96,7 @@ static const struct isa_ext_data isa_edata_arr[] = { > > > ISA_EXT_DATA_ENTRY(zcmp, PRIV_VERSION_1_12_0, ext_zcmp), > > > ISA_EXT_DATA_ENTRY(zcmt, PRIV_VERSION_1_12_0, ext_zcmt), > > > ISA_EXT_DATA_ENTRY(zba, PRIV_VERSION_1_12_0, ext_zba), > > > - ISA_EXT_DATA_ENTRY(zbb, PRIV_VERSION_1_12_0, ext_zbb), > > > + ISA_EXT_DATA_ENTRY_KVM(zbb, PRIV_VERSION_1_12_0, ext_zbb), > > > ISA_EXT_DATA_ENTRY(zbc, PRIV_VERSION_1_12_0, ext_zbc), > > > ISA_EXT_DATA_ENTRY(zbkb, PRIV_VERSION_1_12_0, ext_zbkb), > > > ISA_EXT_DATA_ENTRY(zbkc, PRIV_VERSION_1_12_0, ext_zbkc), > > > @@ -114,13 +121,13 @@ static const struct isa_ext_data isa_edata_arr[] = { > > > ISA_EXT_DATA_ENTRY(zhinxmin, PRIV_VERSION_1_12_0, ext_zhinxmin), > > > ISA_EXT_DATA_ENTRY(smaia, PRIV_VERSION_1_12_0, ext_smaia), > > > ISA_EXT_DATA_ENTRY(smstateen, PRIV_VERSION_1_12_0, ext_smstateen), > > > - ISA_EXT_DATA_ENTRY(ssaia, PRIV_VERSION_1_12_0, ext_ssaia), > > > + ISA_EXT_DATA_ENTRY_KVM(ssaia, PRIV_VERSION_1_12_0, ext_ssaia), > > > ISA_EXT_DATA_ENTRY(sscofpmf, PRIV_VERSION_1_12_0, ext_sscofpmf), > > > - ISA_EXT_DATA_ENTRY(sstc, PRIV_VERSION_1_12_0, ext_sstc), > > > + ISA_EXT_DATA_ENTRY_KVM(sstc, PRIV_VERSION_1_12_0, ext_sstc), > > > ISA_EXT_DATA_ENTRY(svadu, PRIV_VERSION_1_12_0, ext_svadu), > > > - ISA_EXT_DATA_ENTRY(svinval, PRIV_VERSION_1_12_0, ext_svinval), > > > + ISA_EXT_DATA_ENTRY_KVM(svinval, PRIV_VERSION_1_12_0, ext_svinval), > > > ISA_EXT_DATA_ENTRY(svnapot, PRIV_VERSION_1_12_0, ext_svnapot), > > > - ISA_EXT_DATA_ENTRY(svpbmt, PRIV_VERSION_1_12_0, ext_svpbmt), > > > + ISA_EXT_DATA_ENTRY_KVM(svpbmt, PRIV_VERSION_1_12_0, ext_svpbmt), > > > > This approach looks a bit difficult to maintain (it's hard to even see the > > _KVM macro uses). The approach will be even more difficult if we add more > > accelerators. I feel like we need an extension class where objects of that > > class can be passed to functions like riscv_isa_string_ext(). And then we > > also need tcg-extension and kvm-extension classes which inherit from > > the extension class. We can then keep the lists of extensions separate, as > > you originally proposed, as each list will be of its own type. > > So, after coding around a little, I didn't manage to create a KVM list in > kvm.c file because of how ARRAY_SIZE() (a macro that riscv_isa_string_ext()) > calculates the array size. If the list is exported from another file the macro > fails to calculate the size of the array. Similar issues also happens when trying > to use kvm_multi_ext_cfgs[] in this function. > > This means that both tcg and kvm arrays ended up in cpu.c. Hmm, I'd really rather we don't have a kvm array in cpu.c. Going back to duplicating functions like riscv_isa_string_ext() into kvm.c is better, IMO. > The tcg array is > left untouched (isa_edata_arr). The KVM array uses the same type TCG already > uses (isa_ext_edata) because it's good enough for KVM as well. I removed the > 'kvm_available' flag since we're going to manually edit the array. > > riscv_isa_string_ext() is then changed to switch between the tcg/kvm array > as required. The rest of the logic of the function stood the same. I'll also > add a pre-patch prior to all these changes to simplify riscv_isa_string_ext() > a bit when running under TCG. If we have to teach riscv_isa_string_ext() about both tcg and kvm cfg types (and any other functions that come along), then this approach isn't all that helpful anyway. Thanks, drew
On Wed, Jun 21, 2023 at 10:20:37AM +0200, Andrew Jones wrote: > On Tue, Jun 20, 2023 at 07:05:18PM -0300, Daniel Henrique Barboza wrote: > > > > > > On 6/19/23 06:54, Andrew Jones wrote: > > > On Tue, Jun 13, 2023 at 05:58:54PM -0300, Daniel Henrique Barboza wrote: > > > > The isa_edata_arr[] is used by riscv_isa_string_ext() to create the > > > > riscv,isa DT attribute. isa_edata_arr[] is kept in sync with the TCG > > > > property vector riscv_cpu_extensions[], i.e. all TCG properties from > > > > this vector that has a riscv,isa representation are included in > > > > isa_edata_arr[]. > > > > > > > > KVM doesn't implement all TCG properties, but allow them to be created > > > > anyway to not force an API change between TCG and KVM guests. Some of > > > > these TCG-only extensions are defaulted to 'true', and users are also > > > > allowed to enable them. KVM doesn't care, but riscv_isa_string_ext() > > > > does. The result is that these TCG-only enabled extensions will appear > > > > in the riscv,isa DT string under KVM. > > > > > > > > To avoid code repetition and re-use riscv_isa_string_ext() for KVM > > > > guests we'll make a couple of tweaks: > > > > > > > > - set env->priv_ver to 'LATEST' for the KVM 'host' CPU. This is needed > > > > because riscv_isa_string_ext() makes a priv check for each extension > > > > before including them in the ISA string. KVM doesn't care about > > > > env->priv_ver, since it's part of the TCG-only CPU validation, so this > > > > change is benign for KVM; > > > > > > > > - add a new 'kvm_available' flag in isa_ext_data struct. This flag is > > > > set via a new ISA_EXT_DATA_ENTRY_KVM macro to report that, for a given > > > > extension, KVM also supports it. riscv_isa_string_ext() then can check > > > > if a given extension is known by KVM and skip it if it's not. > > > > > > > > This will allow us to re-use riscv_isa_string_ext() for KVM guests. > > > > > > > > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > > > > --- > > > > target/riscv/cpu.c | 28 ++++++++++++++++++++-------- > > > > 1 file changed, 20 insertions(+), 8 deletions(-) > > > > > > > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > > > > index a4f3ed0c17..a773c09645 100644 > > > > --- a/target/riscv/cpu.c > > > > +++ b/target/riscv/cpu.c > > > > @@ -44,11 +44,15 @@ struct isa_ext_data { > > > > const char *name; > > > > int min_version; > > > > int ext_enable_offset; > > > > + bool kvm_available; > > > > }; > > > > #define ISA_EXT_DATA_ENTRY(_name, _min_ver, _prop) \ > > > > {#_name, _min_ver, offsetof(struct RISCVCPUConfig, _prop)} > > > > +#define ISA_EXT_DATA_ENTRY_KVM(_name, _min_ver, _prop) \ > > > > + {#_name, _min_ver, offsetof(struct RISCVCPUConfig, _prop), true} > > > > + > > > > /* > > > > * Here are the ordering rules of extension naming defined by RISC-V > > > > * specification : > > > > @@ -68,14 +72,17 @@ struct isa_ext_data { > > > > * > > > > * Single letter extensions are checked in riscv_cpu_validate_misa_priv() > > > > * instead. > > > > + * > > > > + * ISA_EXT_DATA_ENTRY_KVM() is used to indicate that the extension is > > > > + * also known by the KVM driver. If unsure, use ISA_EXT_DATA_ENTRY(). > > > > */ > > > > static const struct isa_ext_data isa_edata_arr[] = { > > > > - ISA_EXT_DATA_ENTRY(zicbom, PRIV_VERSION_1_12_0, ext_icbom), > > > > - ISA_EXT_DATA_ENTRY(zicboz, PRIV_VERSION_1_12_0, ext_icboz), > > > > + ISA_EXT_DATA_ENTRY_KVM(zicbom, PRIV_VERSION_1_12_0, ext_icbom), > > > > + ISA_EXT_DATA_ENTRY_KVM(zicboz, PRIV_VERSION_1_12_0, ext_icboz), > > > > ISA_EXT_DATA_ENTRY(zicond, PRIV_VERSION_1_12_0, ext_zicond), > > > > ISA_EXT_DATA_ENTRY(zicsr, PRIV_VERSION_1_10_0, ext_icsr), > > > > ISA_EXT_DATA_ENTRY(zifencei, PRIV_VERSION_1_10_0, ext_ifencei), > > > > - ISA_EXT_DATA_ENTRY(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause), > > > > + ISA_EXT_DATA_ENTRY_KVM(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause), > > > > ISA_EXT_DATA_ENTRY(zawrs, PRIV_VERSION_1_12_0, ext_zawrs), > > > > ISA_EXT_DATA_ENTRY(zfh, PRIV_VERSION_1_11_0, ext_zfh), > > > > ISA_EXT_DATA_ENTRY(zfhmin, PRIV_VERSION_1_11_0, ext_zfhmin), > > > > @@ -89,7 +96,7 @@ static const struct isa_ext_data isa_edata_arr[] = { > > > > ISA_EXT_DATA_ENTRY(zcmp, PRIV_VERSION_1_12_0, ext_zcmp), > > > > ISA_EXT_DATA_ENTRY(zcmt, PRIV_VERSION_1_12_0, ext_zcmt), > > > > ISA_EXT_DATA_ENTRY(zba, PRIV_VERSION_1_12_0, ext_zba), > > > > - ISA_EXT_DATA_ENTRY(zbb, PRIV_VERSION_1_12_0, ext_zbb), > > > > + ISA_EXT_DATA_ENTRY_KVM(zbb, PRIV_VERSION_1_12_0, ext_zbb), > > > > ISA_EXT_DATA_ENTRY(zbc, PRIV_VERSION_1_12_0, ext_zbc), > > > > ISA_EXT_DATA_ENTRY(zbkb, PRIV_VERSION_1_12_0, ext_zbkb), > > > > ISA_EXT_DATA_ENTRY(zbkc, PRIV_VERSION_1_12_0, ext_zbkc), > > > > @@ -114,13 +121,13 @@ static const struct isa_ext_data isa_edata_arr[] = { > > > > ISA_EXT_DATA_ENTRY(zhinxmin, PRIV_VERSION_1_12_0, ext_zhinxmin), > > > > ISA_EXT_DATA_ENTRY(smaia, PRIV_VERSION_1_12_0, ext_smaia), > > > > ISA_EXT_DATA_ENTRY(smstateen, PRIV_VERSION_1_12_0, ext_smstateen), > > > > - ISA_EXT_DATA_ENTRY(ssaia, PRIV_VERSION_1_12_0, ext_ssaia), > > > > + ISA_EXT_DATA_ENTRY_KVM(ssaia, PRIV_VERSION_1_12_0, ext_ssaia), > > > > ISA_EXT_DATA_ENTRY(sscofpmf, PRIV_VERSION_1_12_0, ext_sscofpmf), > > > > - ISA_EXT_DATA_ENTRY(sstc, PRIV_VERSION_1_12_0, ext_sstc), > > > > + ISA_EXT_DATA_ENTRY_KVM(sstc, PRIV_VERSION_1_12_0, ext_sstc), > > > > ISA_EXT_DATA_ENTRY(svadu, PRIV_VERSION_1_12_0, ext_svadu), > > > > - ISA_EXT_DATA_ENTRY(svinval, PRIV_VERSION_1_12_0, ext_svinval), > > > > + ISA_EXT_DATA_ENTRY_KVM(svinval, PRIV_VERSION_1_12_0, ext_svinval), > > > > ISA_EXT_DATA_ENTRY(svnapot, PRIV_VERSION_1_12_0, ext_svnapot), > > > > - ISA_EXT_DATA_ENTRY(svpbmt, PRIV_VERSION_1_12_0, ext_svpbmt), > > > > + ISA_EXT_DATA_ENTRY_KVM(svpbmt, PRIV_VERSION_1_12_0, ext_svpbmt), > > > > > > This approach looks a bit difficult to maintain (it's hard to even see the > > > _KVM macro uses). The approach will be even more difficult if we add more > > > accelerators. I feel like we need an extension class where objects of that > > > class can be passed to functions like riscv_isa_string_ext(). And then we > > > also need tcg-extension and kvm-extension classes which inherit from > > > the extension class. We can then keep the lists of extensions separate, as > > > you originally proposed, as each list will be of its own type. > > > > So, after coding around a little, I didn't manage to create a KVM list in > > kvm.c file because of how ARRAY_SIZE() (a macro that riscv_isa_string_ext()) > > calculates the array size. If the list is exported from another file the macro > > fails to calculate the size of the array. Similar issues also happens when trying > > to use kvm_multi_ext_cfgs[] in this function. > > > > This means that both tcg and kvm arrays ended up in cpu.c. > > Hmm, I'd really rather we don't have a kvm array in cpu.c. Going back to > duplicating functions like riscv_isa_string_ext() into kvm.c is better, > IMO. BTW, we could always add a NULL element { } to the end of the array to avoid the need for ARRAY_SIZE(), changing the loop to something like for (struct cfg *c = &cfgs[0]; c->name; ++c) Thanks, drew > > > The tcg array is > > left untouched (isa_edata_arr). The KVM array uses the same type TCG already > > uses (isa_ext_edata) because it's good enough for KVM as well. I removed the > > 'kvm_available' flag since we're going to manually edit the array. > > > > riscv_isa_string_ext() is then changed to switch between the tcg/kvm array > > as required. The rest of the logic of the function stood the same. I'll also > > add a pre-patch prior to all these changes to simplify riscv_isa_string_ext() > > a bit when running under TCG. > > If we have to teach riscv_isa_string_ext() about both tcg and kvm cfg > types (and any other functions that come along), then this approach > isn't all that helpful anyway. > > Thanks, > drew
On 6/21/23 05:20, Andrew Jones wrote: > On Tue, Jun 20, 2023 at 07:05:18PM -0300, Daniel Henrique Barboza wrote: >> >> >> On 6/19/23 06:54, Andrew Jones wrote: >>> On Tue, Jun 13, 2023 at 05:58:54PM -0300, Daniel Henrique Barboza wrote: >>>> The isa_edata_arr[] is used by riscv_isa_string_ext() to create the >>>> riscv,isa DT attribute. isa_edata_arr[] is kept in sync with the TCG >>>> property vector riscv_cpu_extensions[], i.e. all TCG properties from >>>> this vector that has a riscv,isa representation are included in >>>> isa_edata_arr[]. >>>> >>>> KVM doesn't implement all TCG properties, but allow them to be created >>>> anyway to not force an API change between TCG and KVM guests. Some of >>>> these TCG-only extensions are defaulted to 'true', and users are also >>>> allowed to enable them. KVM doesn't care, but riscv_isa_string_ext() >>>> does. The result is that these TCG-only enabled extensions will appear >>>> in the riscv,isa DT string under KVM. >>>> >>>> To avoid code repetition and re-use riscv_isa_string_ext() for KVM >>>> guests we'll make a couple of tweaks: >>>> >>>> - set env->priv_ver to 'LATEST' for the KVM 'host' CPU. This is needed >>>> because riscv_isa_string_ext() makes a priv check for each extension >>>> before including them in the ISA string. KVM doesn't care about >>>> env->priv_ver, since it's part of the TCG-only CPU validation, so this >>>> change is benign for KVM; >>>> >>>> - add a new 'kvm_available' flag in isa_ext_data struct. This flag is >>>> set via a new ISA_EXT_DATA_ENTRY_KVM macro to report that, for a given >>>> extension, KVM also supports it. riscv_isa_string_ext() then can check >>>> if a given extension is known by KVM and skip it if it's not. >>>> >>>> This will allow us to re-use riscv_isa_string_ext() for KVM guests. >>>> >>>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> >>>> --- >>>> target/riscv/cpu.c | 28 ++++++++++++++++++++-------- >>>> 1 file changed, 20 insertions(+), 8 deletions(-) >>>> >>>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c >>>> index a4f3ed0c17..a773c09645 100644 >>>> --- a/target/riscv/cpu.c >>>> +++ b/target/riscv/cpu.c >>>> @@ -44,11 +44,15 @@ struct isa_ext_data { >>>> const char *name; >>>> int min_version; >>>> int ext_enable_offset; >>>> + bool kvm_available; >>>> }; >>>> #define ISA_EXT_DATA_ENTRY(_name, _min_ver, _prop) \ >>>> {#_name, _min_ver, offsetof(struct RISCVCPUConfig, _prop)} >>>> +#define ISA_EXT_DATA_ENTRY_KVM(_name, _min_ver, _prop) \ >>>> + {#_name, _min_ver, offsetof(struct RISCVCPUConfig, _prop), true} >>>> + >>>> /* >>>> * Here are the ordering rules of extension naming defined by RISC-V >>>> * specification : >>>> @@ -68,14 +72,17 @@ struct isa_ext_data { >>>> * >>>> * Single letter extensions are checked in riscv_cpu_validate_misa_priv() >>>> * instead. >>>> + * >>>> + * ISA_EXT_DATA_ENTRY_KVM() is used to indicate that the extension is >>>> + * also known by the KVM driver. If unsure, use ISA_EXT_DATA_ENTRY(). >>>> */ >>>> static const struct isa_ext_data isa_edata_arr[] = { >>>> - ISA_EXT_DATA_ENTRY(zicbom, PRIV_VERSION_1_12_0, ext_icbom), >>>> - ISA_EXT_DATA_ENTRY(zicboz, PRIV_VERSION_1_12_0, ext_icboz), >>>> + ISA_EXT_DATA_ENTRY_KVM(zicbom, PRIV_VERSION_1_12_0, ext_icbom), >>>> + ISA_EXT_DATA_ENTRY_KVM(zicboz, PRIV_VERSION_1_12_0, ext_icboz), >>>> ISA_EXT_DATA_ENTRY(zicond, PRIV_VERSION_1_12_0, ext_zicond), >>>> ISA_EXT_DATA_ENTRY(zicsr, PRIV_VERSION_1_10_0, ext_icsr), >>>> ISA_EXT_DATA_ENTRY(zifencei, PRIV_VERSION_1_10_0, ext_ifencei), >>>> - ISA_EXT_DATA_ENTRY(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause), >>>> + ISA_EXT_DATA_ENTRY_KVM(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause), >>>> ISA_EXT_DATA_ENTRY(zawrs, PRIV_VERSION_1_12_0, ext_zawrs), >>>> ISA_EXT_DATA_ENTRY(zfh, PRIV_VERSION_1_11_0, ext_zfh), >>>> ISA_EXT_DATA_ENTRY(zfhmin, PRIV_VERSION_1_11_0, ext_zfhmin), >>>> @@ -89,7 +96,7 @@ static const struct isa_ext_data isa_edata_arr[] = { >>>> ISA_EXT_DATA_ENTRY(zcmp, PRIV_VERSION_1_12_0, ext_zcmp), >>>> ISA_EXT_DATA_ENTRY(zcmt, PRIV_VERSION_1_12_0, ext_zcmt), >>>> ISA_EXT_DATA_ENTRY(zba, PRIV_VERSION_1_12_0, ext_zba), >>>> - ISA_EXT_DATA_ENTRY(zbb, PRIV_VERSION_1_12_0, ext_zbb), >>>> + ISA_EXT_DATA_ENTRY_KVM(zbb, PRIV_VERSION_1_12_0, ext_zbb), >>>> ISA_EXT_DATA_ENTRY(zbc, PRIV_VERSION_1_12_0, ext_zbc), >>>> ISA_EXT_DATA_ENTRY(zbkb, PRIV_VERSION_1_12_0, ext_zbkb), >>>> ISA_EXT_DATA_ENTRY(zbkc, PRIV_VERSION_1_12_0, ext_zbkc), >>>> @@ -114,13 +121,13 @@ static const struct isa_ext_data isa_edata_arr[] = { >>>> ISA_EXT_DATA_ENTRY(zhinxmin, PRIV_VERSION_1_12_0, ext_zhinxmin), >>>> ISA_EXT_DATA_ENTRY(smaia, PRIV_VERSION_1_12_0, ext_smaia), >>>> ISA_EXT_DATA_ENTRY(smstateen, PRIV_VERSION_1_12_0, ext_smstateen), >>>> - ISA_EXT_DATA_ENTRY(ssaia, PRIV_VERSION_1_12_0, ext_ssaia), >>>> + ISA_EXT_DATA_ENTRY_KVM(ssaia, PRIV_VERSION_1_12_0, ext_ssaia), >>>> ISA_EXT_DATA_ENTRY(sscofpmf, PRIV_VERSION_1_12_0, ext_sscofpmf), >>>> - ISA_EXT_DATA_ENTRY(sstc, PRIV_VERSION_1_12_0, ext_sstc), >>>> + ISA_EXT_DATA_ENTRY_KVM(sstc, PRIV_VERSION_1_12_0, ext_sstc), >>>> ISA_EXT_DATA_ENTRY(svadu, PRIV_VERSION_1_12_0, ext_svadu), >>>> - ISA_EXT_DATA_ENTRY(svinval, PRIV_VERSION_1_12_0, ext_svinval), >>>> + ISA_EXT_DATA_ENTRY_KVM(svinval, PRIV_VERSION_1_12_0, ext_svinval), >>>> ISA_EXT_DATA_ENTRY(svnapot, PRIV_VERSION_1_12_0, ext_svnapot), >>>> - ISA_EXT_DATA_ENTRY(svpbmt, PRIV_VERSION_1_12_0, ext_svpbmt), >>>> + ISA_EXT_DATA_ENTRY_KVM(svpbmt, PRIV_VERSION_1_12_0, ext_svpbmt), >>> >>> This approach looks a bit difficult to maintain (it's hard to even see the >>> _KVM macro uses). The approach will be even more difficult if we add more >>> accelerators. I feel like we need an extension class where objects of that >>> class can be passed to functions like riscv_isa_string_ext(). And then we >>> also need tcg-extension and kvm-extension classes which inherit from >>> the extension class. We can then keep the lists of extensions separate, as >>> you originally proposed, as each list will be of its own type. >> >> So, after coding around a little, I didn't manage to create a KVM list in >> kvm.c file because of how ARRAY_SIZE() (a macro that riscv_isa_string_ext()) >> calculates the array size. If the list is exported from another file the macro >> fails to calculate the size of the array. Similar issues also happens when trying >> to use kvm_multi_ext_cfgs[] in this function. >> >> This means that both tcg and kvm arrays ended up in cpu.c. > > Hmm, I'd really rather we don't have a kvm array in cpu.c. Going back to > duplicating functions like riscv_isa_string_ext() into kvm.c is better, > IMO. > >> The tcg array is >> left untouched (isa_edata_arr). The KVM array uses the same type TCG already >> uses (isa_ext_edata) because it's good enough for KVM as well. I removed the >> 'kvm_available' flag since we're going to manually edit the array. >> >> riscv_isa_string_ext() is then changed to switch between the tcg/kvm array >> as required. The rest of the logic of the function stood the same. I'll also >> add a pre-patch prior to all these changes to simplify riscv_isa_string_ext() >> a bit when running under TCG. > > If we have to teach riscv_isa_string_ext() about both tcg and kvm cfg > types (and any other functions that come along), then this approach > isn't all that helpful anyway. Someone must point the function to the right array. We can do it inside the function, we can pass the array as parameter and the caller will be responsible for it. But somewhere along the road there'll be an 'if (kvm_enabled())'. All this said, the root cause of all this saga is due to extensions being enabled without KVM knowing about it. riscv_isa_string_ext() is quite straightforward: it is going through all the cpu->cfg.ext_() properties and picking the ones enable. If we can ensure that (1) all non-kvm properties are always default to false and (2) there is no way for the user to set them via command line, riscv_isa_string_ext() and all related artifacts can remain untouched. I have an idea to accomplish that. I'll see how it goes. Thanks, Daniel > > Thanks, > drew
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index a4f3ed0c17..a773c09645 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -44,11 +44,15 @@ struct isa_ext_data { const char *name; int min_version; int ext_enable_offset; + bool kvm_available; }; #define ISA_EXT_DATA_ENTRY(_name, _min_ver, _prop) \ {#_name, _min_ver, offsetof(struct RISCVCPUConfig, _prop)} +#define ISA_EXT_DATA_ENTRY_KVM(_name, _min_ver, _prop) \ + {#_name, _min_ver, offsetof(struct RISCVCPUConfig, _prop), true} + /* * Here are the ordering rules of extension naming defined by RISC-V * specification : @@ -68,14 +72,17 @@ struct isa_ext_data { * * Single letter extensions are checked in riscv_cpu_validate_misa_priv() * instead. + * + * ISA_EXT_DATA_ENTRY_KVM() is used to indicate that the extension is + * also known by the KVM driver. If unsure, use ISA_EXT_DATA_ENTRY(). */ static const struct isa_ext_data isa_edata_arr[] = { - ISA_EXT_DATA_ENTRY(zicbom, PRIV_VERSION_1_12_0, ext_icbom), - ISA_EXT_DATA_ENTRY(zicboz, PRIV_VERSION_1_12_0, ext_icboz), + ISA_EXT_DATA_ENTRY_KVM(zicbom, PRIV_VERSION_1_12_0, ext_icbom), + ISA_EXT_DATA_ENTRY_KVM(zicboz, PRIV_VERSION_1_12_0, ext_icboz), ISA_EXT_DATA_ENTRY(zicond, PRIV_VERSION_1_12_0, ext_zicond), ISA_EXT_DATA_ENTRY(zicsr, PRIV_VERSION_1_10_0, ext_icsr), ISA_EXT_DATA_ENTRY(zifencei, PRIV_VERSION_1_10_0, ext_ifencei), - ISA_EXT_DATA_ENTRY(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause), + ISA_EXT_DATA_ENTRY_KVM(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause), ISA_EXT_DATA_ENTRY(zawrs, PRIV_VERSION_1_12_0, ext_zawrs), ISA_EXT_DATA_ENTRY(zfh, PRIV_VERSION_1_11_0, ext_zfh), ISA_EXT_DATA_ENTRY(zfhmin, PRIV_VERSION_1_11_0, ext_zfhmin), @@ -89,7 +96,7 @@ static const struct isa_ext_data isa_edata_arr[] = { ISA_EXT_DATA_ENTRY(zcmp, PRIV_VERSION_1_12_0, ext_zcmp), ISA_EXT_DATA_ENTRY(zcmt, PRIV_VERSION_1_12_0, ext_zcmt), ISA_EXT_DATA_ENTRY(zba, PRIV_VERSION_1_12_0, ext_zba), - ISA_EXT_DATA_ENTRY(zbb, PRIV_VERSION_1_12_0, ext_zbb), + ISA_EXT_DATA_ENTRY_KVM(zbb, PRIV_VERSION_1_12_0, ext_zbb), ISA_EXT_DATA_ENTRY(zbc, PRIV_VERSION_1_12_0, ext_zbc), ISA_EXT_DATA_ENTRY(zbkb, PRIV_VERSION_1_12_0, ext_zbkb), ISA_EXT_DATA_ENTRY(zbkc, PRIV_VERSION_1_12_0, ext_zbkc), @@ -114,13 +121,13 @@ static const struct isa_ext_data isa_edata_arr[] = { ISA_EXT_DATA_ENTRY(zhinxmin, PRIV_VERSION_1_12_0, ext_zhinxmin), ISA_EXT_DATA_ENTRY(smaia, PRIV_VERSION_1_12_0, ext_smaia), ISA_EXT_DATA_ENTRY(smstateen, PRIV_VERSION_1_12_0, ext_smstateen), - ISA_EXT_DATA_ENTRY(ssaia, PRIV_VERSION_1_12_0, ext_ssaia), + ISA_EXT_DATA_ENTRY_KVM(ssaia, PRIV_VERSION_1_12_0, ext_ssaia), ISA_EXT_DATA_ENTRY(sscofpmf, PRIV_VERSION_1_12_0, ext_sscofpmf), - ISA_EXT_DATA_ENTRY(sstc, PRIV_VERSION_1_12_0, ext_sstc), + ISA_EXT_DATA_ENTRY_KVM(sstc, PRIV_VERSION_1_12_0, ext_sstc), ISA_EXT_DATA_ENTRY(svadu, PRIV_VERSION_1_12_0, ext_svadu), - ISA_EXT_DATA_ENTRY(svinval, PRIV_VERSION_1_12_0, ext_svinval), + ISA_EXT_DATA_ENTRY_KVM(svinval, PRIV_VERSION_1_12_0, ext_svinval), ISA_EXT_DATA_ENTRY(svnapot, PRIV_VERSION_1_12_0, ext_svnapot), - ISA_EXT_DATA_ENTRY(svpbmt, PRIV_VERSION_1_12_0, ext_svpbmt), + ISA_EXT_DATA_ENTRY_KVM(svpbmt, PRIV_VERSION_1_12_0, ext_svpbmt), ISA_EXT_DATA_ENTRY(xtheadba, PRIV_VERSION_1_11_0, ext_xtheadba), ISA_EXT_DATA_ENTRY(xtheadbb, PRIV_VERSION_1_11_0, ext_xtheadbb), ISA_EXT_DATA_ENTRY(xtheadbs, PRIV_VERSION_1_11_0, ext_xtheadbs), @@ -586,6 +593,7 @@ static void riscv_host_cpu_init(Object *obj) set_misa(env, MXL_RV64, 0); #endif riscv_cpu_add_user_properties(obj); + env->priv_ver = PRIV_VERSION_LATEST; } #endif @@ -1981,6 +1989,10 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int i; for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) { + if (riscv_running_kvm() && !isa_edata_arr[i].kvm_available) { + continue; + } + if (cpu->env.priv_ver >= isa_edata_arr[i].min_version && isa_ext_is_enabled(cpu, &isa_edata_arr[i])) { new = g_strconcat(old, "_", isa_edata_arr[i].name, NULL);
The isa_edata_arr[] is used by riscv_isa_string_ext() to create the riscv,isa DT attribute. isa_edata_arr[] is kept in sync with the TCG property vector riscv_cpu_extensions[], i.e. all TCG properties from this vector that has a riscv,isa representation are included in isa_edata_arr[]. KVM doesn't implement all TCG properties, but allow them to be created anyway to not force an API change between TCG and KVM guests. Some of these TCG-only extensions are defaulted to 'true', and users are also allowed to enable them. KVM doesn't care, but riscv_isa_string_ext() does. The result is that these TCG-only enabled extensions will appear in the riscv,isa DT string under KVM. To avoid code repetition and re-use riscv_isa_string_ext() for KVM guests we'll make a couple of tweaks: - set env->priv_ver to 'LATEST' for the KVM 'host' CPU. This is needed because riscv_isa_string_ext() makes a priv check for each extension before including them in the ISA string. KVM doesn't care about env->priv_ver, since it's part of the TCG-only CPU validation, so this change is benign for KVM; - add a new 'kvm_available' flag in isa_ext_data struct. This flag is set via a new ISA_EXT_DATA_ENTRY_KVM macro to report that, for a given extension, KVM also supports it. riscv_isa_string_ext() then can check if a given extension is known by KVM and skip it if it's not. This will allow us to re-use riscv_isa_string_ext() for KVM guests. Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> --- target/riscv/cpu.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-)