diff mbox series

[16/16] target/riscv/kvm.c: read/write (cbom|cboz)_blocksize in KVM

Message ID 20230530194623.272652-17-dbarboza@ventanamicro.com
State New
Headers show
Series target/riscv, KVM: fixes and enhancements | expand

Commit Message

Daniel Henrique Barboza May 30, 2023, 7:46 p.m. UTC
If we don't set a proper cbom_blocksize|cboz_blocksize in the FDT the
Linux Kernel will fail to detect the availability of the CBOM/CBOZ
extensions, regardless of the contents of the 'riscv,isa' DT prop.

The FDT is being written using the cpu->cfg.cbom|z_blocksize attributes,
so let's use them. We'll also expose them as user flags like it is
already done with TCG.

However, in contrast with what happens with TCG, the user is not able to
set any value that is different from the 'host' value. And KVM can be
harsh dealing with it: a ENOTSUPP can be thrown for the mere attempt of
executing kvm_set_one_reg() for these 2 regs.

We'll read the 'host' value and use it to set these values, regardless of
user choice. If the user happened to chose a different value, error out.
We'll also error out if we failed to read the block sizes.

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 target/riscv/kvm.c | 94 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 92 insertions(+), 2 deletions(-)

Comments

Andrew Jones June 7, 2023, 1:01 p.m. UTC | #1
On Tue, May 30, 2023 at 04:46:23PM -0300, Daniel Henrique Barboza wrote:
> If we don't set a proper cbom_blocksize|cboz_blocksize in the FDT the
> Linux Kernel will fail to detect the availability of the CBOM/CBOZ
> extensions, regardless of the contents of the 'riscv,isa' DT prop.
> 
> The FDT is being written using the cpu->cfg.cbom|z_blocksize attributes,
> so let's use them. We'll also expose them as user flags like it is
> already done with TCG.
> 
> However, in contrast with what happens with TCG, the user is not able to
> set any value that is different from the 'host' value. And KVM can be
> harsh dealing with it: a ENOTSUPP can be thrown for the mere attempt of
> executing kvm_set_one_reg() for these 2 regs.
> 
> We'll read the 'host' value and use it to set these values, regardless of
> user choice. If the user happened to chose a different value, error out.
> We'll also error out if we failed to read the block sizes.
> 
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>  target/riscv/kvm.c | 94 +++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 92 insertions(+), 2 deletions(-)
> 
> diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c
> index 92b99fe261..7789d835e5 100644
> --- a/target/riscv/kvm.c
> +++ b/target/riscv/kvm.c
> @@ -241,8 +241,16 @@ static void kvm_cpu_cfg_set(RISCVCPU *cpu, RISCVCPUMultiExtConfig *multi_ext,
>                              uint32_t val)
>  {
>      int cpu_cfg_offset = multi_ext->cpu_cfg_offset;
> -    bool *ext_enabled = (void *)&cpu->cfg + cpu_cfg_offset;
> +    uint16_t *blocksize;
> +    bool *ext_enabled;
>  
> +    if (strstr(multi_ext->name, "blocksize")) {
> +        blocksize = (void *)&cpu->cfg + cpu_cfg_offset;
> +        *blocksize = val;
> +        return;
> +    }

We should add 'get' accessors to each property and then always use those
accessors to get the values. Trying to share a single accessor across
properties, using the names to determine their sizes, is basically trying
to reinvent 'get' without the function pointer.

> +
> +    ext_enabled = (void *)&cpu->cfg + cpu_cfg_offset;
>      *ext_enabled = val;
>  }
>  
> @@ -250,8 +258,15 @@ static uint32_t kvm_cpu_cfg_get(RISCVCPU *cpu,
>                                  RISCVCPUMultiExtConfig *multi_ext)
>  {
>      int cpu_cfg_offset = multi_ext->cpu_cfg_offset;
> -    bool *ext_enabled = (void *)&cpu->cfg + cpu_cfg_offset;
> +    uint16_t *blocksize;
> +    bool *ext_enabled;
>  
> +    if (strstr(multi_ext->name, "blocksize")) {
> +        blocksize = (void *)&cpu->cfg + cpu_cfg_offset;
> +        return *blocksize;
> +    }
> +
> +    ext_enabled = (void *)&cpu->cfg + cpu_cfg_offset;
>      return *ext_enabled;
>  }
>  
> @@ -295,6 +310,33 @@ static void kvm_cpu_set_multi_ext_cfg(Object *obj, Visitor *v,
>      kvm_cpu_cfg_set(cpu, multi_ext_cfg, value);
>  }
>  
> +/*
> + * We'll avoid extra complexity by always assuming this
> + * array order with cbom first.
> + */
> +static RISCVCPUMultiExtConfig kvm_cbomz_blksize_cfgs[] = {

Hmm, yet another cfg struct type, and this one is specific to block sizes.
I'd rather we find a way to keep cfg definitions more general and then use
the same struct for all.

> +    {.name = "cbom_blocksize", .cpu_cfg_offset = CPUCFG(cbom_blocksize),
> +     .kvm_reg_id = KVM_REG_RISCV_CONFIG_REG(zicbom_block_size)},
> +    {.name = "cboz_blocksize", .cpu_cfg_offset = CPUCFG(cboz_blocksize),
> +     .kvm_reg_id = KVM_REG_RISCV_CONFIG_REG(zicboz_block_size)},
> +};
> +
> +static void kvm_cpu_set_cbomz_blksize(Object *obj, Visitor *v,
> +                                      const char *name,
> +                                      void *opaque, Error **errp)
> +{
> +    RISCVCPUMultiExtConfig *cbomz_size_cfg = opaque;
> +    RISCVCPU *cpu = RISCV_CPU(obj);
> +    uint16_t value;
> +
> +    if (!visit_type_uint16(v, name, &value, errp)) {
> +        return;
> +    }
> +
> +    cbomz_size_cfg->user_set = true;
> +    kvm_cpu_cfg_set(cpu, cbomz_size_cfg, value);
> +}
> +
>  static void kvm_riscv_update_cpu_cfg_isa_ext(RISCVCPU *cpu, CPUState *cs)
>  {
>      CPURISCVState *env = &cpu->env;
> @@ -321,6 +363,45 @@ static void kvm_riscv_update_cpu_cfg_isa_ext(RISCVCPU *cpu, CPUState *cs)
>      }
>  }
>  
> +static void kvm_riscv_finalize_features(RISCVCPU *cpu, CPUState *cs)
> +{
> +    CPURISCVState *env = &cpu->env;
> +    uint64_t id, reg;
> +    int i, ret;
> +
> +    for (i = 0; i < ARRAY_SIZE(kvm_cbomz_blksize_cfgs); i++) {
> +        RISCVCPUMultiExtConfig *cbomz_cfg = &kvm_cbomz_blksize_cfgs[i];
> +        uint64_t host_val;
> +
> +        if ((i == 0 && !cpu->cfg.ext_icbom) ||
> +            (i == 1 && !cpu->cfg.ext_icboz)) {

Rather than the required array order and this magic index stuff, we can
just save the offset of the ext_* boolean in the cfg structure, like we
already do for the *_blocksize, and then check it here.

Also, I think we want to warn here if cbomz_cfg->user_set is set. If the
user set some block size, but disabled the extension, then they should be
told that the block size will be ignored. Letting them know it's ignored
is particularly good to do since we're not validating it. I.e. the user
shouldn't assume the block size they put there is worth anything at all.

> +            continue;
> +        }
> +
> +        id = kvm_riscv_reg_id(env, KVM_REG_RISCV_CONFIG,
> +                              cbomz_cfg->kvm_reg_id);
> +
> +        ret = kvm_get_one_reg(cs, id, &host_val);
> +        if (ret != 0) {
> +            error_report("Unable to read KVM reg val %s, error %d",
> +                         cbomz_cfg->name, ret);
> +            exit(EXIT_FAILURE);
> +        }
> +
> +        if (cbomz_cfg->user_set) {
> +            reg = kvm_cpu_cfg_get(cpu, cbomz_cfg);
> +            if (reg != host_val) {
> +                error_report("Unable to set %s to a different value than "
> +                             "the host (%lu)",
> +                             cbomz_cfg->name, host_val);
> +                exit(EXIT_FAILURE);
> +            }
> +        }
> +
> +        kvm_cpu_cfg_set(cpu, cbomz_cfg, host_val);
> +    }
> +}
> +
>  static void kvm_riscv_add_cpu_user_properties(Object *cpu_obj)
>  {
>      int i;
> @@ -344,6 +425,14 @@ static void kvm_riscv_add_cpu_user_properties(Object *cpu_obj)
>                              kvm_cpu_set_multi_ext_cfg,
>                              NULL, multi_cfg);
>      }
> +
> +    for (i = 0; i < ARRAY_SIZE(kvm_cbomz_blksize_cfgs); i++) {
> +        RISCVCPUMultiExtConfig *cbomz_size_cfg = &kvm_cbomz_blksize_cfgs[i];
> +
> +        object_property_add(cpu_obj, cbomz_size_cfg->name, "uint16",
> +                            NULL, kvm_cpu_set_cbomz_blksize,
> +                            NULL, cbomz_size_cfg);
> +    }
>  }
>  
>  void kvm_riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str,
> @@ -856,6 +945,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
>  
>      kvm_riscv_update_cpu_misa_ext(cpu, cs);
>      kvm_riscv_update_cpu_cfg_isa_ext(cpu, cs);
> +    kvm_riscv_finalize_features(cpu, cs);
>  
>      return ret;
>  }
> -- 
> 2.40.1
> 
>

Thanks,
drew
Daniel Henrique Barboza June 7, 2023, 8:37 p.m. UTC | #2
On 6/7/23 10:01, Andrew Jones wrote:
> On Tue, May 30, 2023 at 04:46:23PM -0300, Daniel Henrique Barboza wrote:
>> If we don't set a proper cbom_blocksize|cboz_blocksize in the FDT the
>> Linux Kernel will fail to detect the availability of the CBOM/CBOZ
>> extensions, regardless of the contents of the 'riscv,isa' DT prop.
>>
>> The FDT is being written using the cpu->cfg.cbom|z_blocksize attributes,
>> so let's use them. We'll also expose them as user flags like it is
>> already done with TCG.
>>
>> However, in contrast with what happens with TCG, the user is not able to
>> set any value that is different from the 'host' value. And KVM can be
>> harsh dealing with it: a ENOTSUPP can be thrown for the mere attempt of
>> executing kvm_set_one_reg() for these 2 regs.
>>
>> We'll read the 'host' value and use it to set these values, regardless of
>> user choice. If the user happened to chose a different value, error out.
>> We'll also error out if we failed to read the block sizes.
>>
>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>> ---
>>   target/riscv/kvm.c | 94 +++++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 92 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c
>> index 92b99fe261..7789d835e5 100644
>> --- a/target/riscv/kvm.c
>> +++ b/target/riscv/kvm.c
>> @@ -241,8 +241,16 @@ static void kvm_cpu_cfg_set(RISCVCPU *cpu, RISCVCPUMultiExtConfig *multi_ext,
>>                               uint32_t val)
>>   {
>>       int cpu_cfg_offset = multi_ext->cpu_cfg_offset;
>> -    bool *ext_enabled = (void *)&cpu->cfg + cpu_cfg_offset;
>> +    uint16_t *blocksize;
>> +    bool *ext_enabled;
>>   
>> +    if (strstr(multi_ext->name, "blocksize")) {
>> +        blocksize = (void *)&cpu->cfg + cpu_cfg_offset;
>> +        *blocksize = val;
>> +        return;
>> +    }
> 
> We should add 'get' accessors to each property and then always use those
> accessors to get the values. Trying to share a single accessor across
> properties, using the names to determine their sizes, is basically trying
> to reinvent 'get' without the function pointer.

To be honest we don't need all this machinery for the blocksize attributes.
We check them only in a few cases and could access them directly via cpu->cfg.

I'll change this up in v2.


Daniel

> 
>> +
>> +    ext_enabled = (void *)&cpu->cfg + cpu_cfg_offset;
>>       *ext_enabled = val;
>>   }
>>   
>> @@ -250,8 +258,15 @@ static uint32_t kvm_cpu_cfg_get(RISCVCPU *cpu,
>>                                   RISCVCPUMultiExtConfig *multi_ext)
>>   {
>>       int cpu_cfg_offset = multi_ext->cpu_cfg_offset;
>> -    bool *ext_enabled = (void *)&cpu->cfg + cpu_cfg_offset;
>> +    uint16_t *blocksize;
>> +    bool *ext_enabled;
>>   
>> +    if (strstr(multi_ext->name, "blocksize")) {
>> +        blocksize = (void *)&cpu->cfg + cpu_cfg_offset;
>> +        return *blocksize;
>> +    }
>> +
>> +    ext_enabled = (void *)&cpu->cfg + cpu_cfg_offset;
>>       return *ext_enabled;
>>   }
>>   
>> @@ -295,6 +310,33 @@ static void kvm_cpu_set_multi_ext_cfg(Object *obj, Visitor *v,
>>       kvm_cpu_cfg_set(cpu, multi_ext_cfg, value);
>>   }
>>   
>> +/*
>> + * We'll avoid extra complexity by always assuming this
>> + * array order with cbom first.
>> + */
>> +static RISCVCPUMultiExtConfig kvm_cbomz_blksize_cfgs[] = {
> 
> Hmm, yet another cfg struct type, and this one is specific to block sizes.
> I'd rather we find a way to keep cfg definitions more general and then use
> the same struct for all.
> 
>> +    {.name = "cbom_blocksize", .cpu_cfg_offset = CPUCFG(cbom_blocksize),
>> +     .kvm_reg_id = KVM_REG_RISCV_CONFIG_REG(zicbom_block_size)},
>> +    {.name = "cboz_blocksize", .cpu_cfg_offset = CPUCFG(cboz_blocksize),
>> +     .kvm_reg_id = KVM_REG_RISCV_CONFIG_REG(zicboz_block_size)},
>> +};
>> +
>> +static void kvm_cpu_set_cbomz_blksize(Object *obj, Visitor *v,
>> +                                      const char *name,
>> +                                      void *opaque, Error **errp)
>> +{
>> +    RISCVCPUMultiExtConfig *cbomz_size_cfg = opaque;
>> +    RISCVCPU *cpu = RISCV_CPU(obj);
>> +    uint16_t value;
>> +
>> +    if (!visit_type_uint16(v, name, &value, errp)) {
>> +        return;
>> +    }
>> +
>> +    cbomz_size_cfg->user_set = true;
>> +    kvm_cpu_cfg_set(cpu, cbomz_size_cfg, value);
>> +}
>> +
>>   static void kvm_riscv_update_cpu_cfg_isa_ext(RISCVCPU *cpu, CPUState *cs)
>>   {
>>       CPURISCVState *env = &cpu->env;
>> @@ -321,6 +363,45 @@ static void kvm_riscv_update_cpu_cfg_isa_ext(RISCVCPU *cpu, CPUState *cs)
>>       }
>>   }
>>   
>> +static void kvm_riscv_finalize_features(RISCVCPU *cpu, CPUState *cs)
>> +{
>> +    CPURISCVState *env = &cpu->env;
>> +    uint64_t id, reg;
>> +    int i, ret;
>> +
>> +    for (i = 0; i < ARRAY_SIZE(kvm_cbomz_blksize_cfgs); i++) {
>> +        RISCVCPUMultiExtConfig *cbomz_cfg = &kvm_cbomz_blksize_cfgs[i];
>> +        uint64_t host_val;
>> +
>> +        if ((i == 0 && !cpu->cfg.ext_icbom) ||
>> +            (i == 1 && !cpu->cfg.ext_icboz)) {
> 
> Rather than the required array order and this magic index stuff, we can
> just save the offset of the ext_* boolean in the cfg structure, like we
> already do for the *_blocksize, and then check it here.
> 
> Also, I think we want to warn here if cbomz_cfg->user_set is set. If the
> user set some block size, but disabled the extension, then they should be
> told that the block size will be ignored. Letting them know it's ignored
> is particularly good to do since we're not validating it. I.e. the user
> shouldn't assume the block size they put there is worth anything at all.
> 
>> +            continue;
>> +        }
>> +
>> +        id = kvm_riscv_reg_id(env, KVM_REG_RISCV_CONFIG,
>> +                              cbomz_cfg->kvm_reg_id);
>> +
>> +        ret = kvm_get_one_reg(cs, id, &host_val);
>> +        if (ret != 0) {
>> +            error_report("Unable to read KVM reg val %s, error %d",
>> +                         cbomz_cfg->name, ret);
>> +            exit(EXIT_FAILURE);
>> +        }
>> +
>> +        if (cbomz_cfg->user_set) {
>> +            reg = kvm_cpu_cfg_get(cpu, cbomz_cfg);
>> +            if (reg != host_val) {
>> +                error_report("Unable to set %s to a different value than "
>> +                             "the host (%lu)",
>> +                             cbomz_cfg->name, host_val);
>> +                exit(EXIT_FAILURE);
>> +            }
>> +        }
>> +
>> +        kvm_cpu_cfg_set(cpu, cbomz_cfg, host_val);
>> +    }
>> +}
>> +
>>   static void kvm_riscv_add_cpu_user_properties(Object *cpu_obj)
>>   {
>>       int i;
>> @@ -344,6 +425,14 @@ static void kvm_riscv_add_cpu_user_properties(Object *cpu_obj)
>>                               kvm_cpu_set_multi_ext_cfg,
>>                               NULL, multi_cfg);
>>       }
>> +
>> +    for (i = 0; i < ARRAY_SIZE(kvm_cbomz_blksize_cfgs); i++) {
>> +        RISCVCPUMultiExtConfig *cbomz_size_cfg = &kvm_cbomz_blksize_cfgs[i];
>> +
>> +        object_property_add(cpu_obj, cbomz_size_cfg->name, "uint16",
>> +                            NULL, kvm_cpu_set_cbomz_blksize,
>> +                            NULL, cbomz_size_cfg);
>> +    }
>>   }
>>   
>>   void kvm_riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str,
>> @@ -856,6 +945,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
>>   
>>       kvm_riscv_update_cpu_misa_ext(cpu, cs);
>>       kvm_riscv_update_cpu_cfg_isa_ext(cpu, cs);
>> +    kvm_riscv_finalize_features(cpu, cs);
>>   
>>       return ret;
>>   }
>> -- 
>> 2.40.1
>>
>>
> 
> Thanks,
> drew
Andrew Jones June 8, 2023, 6:39 a.m. UTC | #3
On Wed, Jun 07, 2023 at 05:37:16PM -0300, Daniel Henrique Barboza wrote:
> 
> 
> On 6/7/23 10:01, Andrew Jones wrote:
> > On Tue, May 30, 2023 at 04:46:23PM -0300, Daniel Henrique Barboza wrote:
> > > If we don't set a proper cbom_blocksize|cboz_blocksize in the FDT the
> > > Linux Kernel will fail to detect the availability of the CBOM/CBOZ
> > > extensions, regardless of the contents of the 'riscv,isa' DT prop.
> > > 
> > > The FDT is being written using the cpu->cfg.cbom|z_blocksize attributes,
> > > so let's use them. We'll also expose them as user flags like it is
> > > already done with TCG.
> > > 
> > > However, in contrast with what happens with TCG, the user is not able to
> > > set any value that is different from the 'host' value. And KVM can be
> > > harsh dealing with it: a ENOTSUPP can be thrown for the mere attempt of
> > > executing kvm_set_one_reg() for these 2 regs.
> > > 
> > > We'll read the 'host' value and use it to set these values, regardless of
> > > user choice. If the user happened to chose a different value, error out.
> > > We'll also error out if we failed to read the block sizes.
> > > 
> > > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> > > ---
> > >   target/riscv/kvm.c | 94 +++++++++++++++++++++++++++++++++++++++++++++-
> > >   1 file changed, 92 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c
> > > index 92b99fe261..7789d835e5 100644
> > > --- a/target/riscv/kvm.c
> > > +++ b/target/riscv/kvm.c
> > > @@ -241,8 +241,16 @@ static void kvm_cpu_cfg_set(RISCVCPU *cpu, RISCVCPUMultiExtConfig *multi_ext,
> > >                               uint32_t val)
> > >   {
> > >       int cpu_cfg_offset = multi_ext->cpu_cfg_offset;
> > > -    bool *ext_enabled = (void *)&cpu->cfg + cpu_cfg_offset;
> > > +    uint16_t *blocksize;
> > > +    bool *ext_enabled;
> > > +    if (strstr(multi_ext->name, "blocksize")) {
> > > +        blocksize = (void *)&cpu->cfg + cpu_cfg_offset;
> > > +        *blocksize = val;
> > > +        return;
> > > +    }
> > 
> > We should add 'get' accessors to each property and then always use those
> > accessors to get the values. Trying to share a single accessor across
> > properties, using the names to determine their sizes, is basically trying
> > to reinvent 'get' without the function pointer.
> 
> To be honest we don't need all this machinery for the blocksize attributes.
> We check them only in a few cases and could access them directly via cpu->cfg.
>

OK

A bit off-topic, but thinking about this some more, we're doing block
sizes wrong. We should be using boolean properties for cpu features in
order to simplify qmp_query_cpu_model_expansion(), which we'll want to add
to riscv soon. For CBO block sizes, that means to create multiple possible
block size booleans, e.g. cbom64, cbom128, ..., which are all mutually
exclusive. Using booleans also avoids the need to validate that the block
size a user puts on the command line is a power-of-2, which TCG isn't
currently doing, but should be.

Changing how we input block sizes, with all the deprecation and what not,
would be a separate series though, and I think it can go after this
series, because the block sizes would still ultimately be stored in the
same way, as uint16s, for convenience of use.

Thanks,
drew
diff mbox series

Patch

diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c
index 92b99fe261..7789d835e5 100644
--- a/target/riscv/kvm.c
+++ b/target/riscv/kvm.c
@@ -241,8 +241,16 @@  static void kvm_cpu_cfg_set(RISCVCPU *cpu, RISCVCPUMultiExtConfig *multi_ext,
                             uint32_t val)
 {
     int cpu_cfg_offset = multi_ext->cpu_cfg_offset;
-    bool *ext_enabled = (void *)&cpu->cfg + cpu_cfg_offset;
+    uint16_t *blocksize;
+    bool *ext_enabled;
 
+    if (strstr(multi_ext->name, "blocksize")) {
+        blocksize = (void *)&cpu->cfg + cpu_cfg_offset;
+        *blocksize = val;
+        return;
+    }
+
+    ext_enabled = (void *)&cpu->cfg + cpu_cfg_offset;
     *ext_enabled = val;
 }
 
@@ -250,8 +258,15 @@  static uint32_t kvm_cpu_cfg_get(RISCVCPU *cpu,
                                 RISCVCPUMultiExtConfig *multi_ext)
 {
     int cpu_cfg_offset = multi_ext->cpu_cfg_offset;
-    bool *ext_enabled = (void *)&cpu->cfg + cpu_cfg_offset;
+    uint16_t *blocksize;
+    bool *ext_enabled;
 
+    if (strstr(multi_ext->name, "blocksize")) {
+        blocksize = (void *)&cpu->cfg + cpu_cfg_offset;
+        return *blocksize;
+    }
+
+    ext_enabled = (void *)&cpu->cfg + cpu_cfg_offset;
     return *ext_enabled;
 }
 
@@ -295,6 +310,33 @@  static void kvm_cpu_set_multi_ext_cfg(Object *obj, Visitor *v,
     kvm_cpu_cfg_set(cpu, multi_ext_cfg, value);
 }
 
+/*
+ * We'll avoid extra complexity by always assuming this
+ * array order with cbom first.
+ */
+static RISCVCPUMultiExtConfig kvm_cbomz_blksize_cfgs[] = {
+    {.name = "cbom_blocksize", .cpu_cfg_offset = CPUCFG(cbom_blocksize),
+     .kvm_reg_id = KVM_REG_RISCV_CONFIG_REG(zicbom_block_size)},
+    {.name = "cboz_blocksize", .cpu_cfg_offset = CPUCFG(cboz_blocksize),
+     .kvm_reg_id = KVM_REG_RISCV_CONFIG_REG(zicboz_block_size)},
+};
+
+static void kvm_cpu_set_cbomz_blksize(Object *obj, Visitor *v,
+                                      const char *name,
+                                      void *opaque, Error **errp)
+{
+    RISCVCPUMultiExtConfig *cbomz_size_cfg = opaque;
+    RISCVCPU *cpu = RISCV_CPU(obj);
+    uint16_t value;
+
+    if (!visit_type_uint16(v, name, &value, errp)) {
+        return;
+    }
+
+    cbomz_size_cfg->user_set = true;
+    kvm_cpu_cfg_set(cpu, cbomz_size_cfg, value);
+}
+
 static void kvm_riscv_update_cpu_cfg_isa_ext(RISCVCPU *cpu, CPUState *cs)
 {
     CPURISCVState *env = &cpu->env;
@@ -321,6 +363,45 @@  static void kvm_riscv_update_cpu_cfg_isa_ext(RISCVCPU *cpu, CPUState *cs)
     }
 }
 
+static void kvm_riscv_finalize_features(RISCVCPU *cpu, CPUState *cs)
+{
+    CPURISCVState *env = &cpu->env;
+    uint64_t id, reg;
+    int i, ret;
+
+    for (i = 0; i < ARRAY_SIZE(kvm_cbomz_blksize_cfgs); i++) {
+        RISCVCPUMultiExtConfig *cbomz_cfg = &kvm_cbomz_blksize_cfgs[i];
+        uint64_t host_val;
+
+        if ((i == 0 && !cpu->cfg.ext_icbom) ||
+            (i == 1 && !cpu->cfg.ext_icboz)) {
+            continue;
+        }
+
+        id = kvm_riscv_reg_id(env, KVM_REG_RISCV_CONFIG,
+                              cbomz_cfg->kvm_reg_id);
+
+        ret = kvm_get_one_reg(cs, id, &host_val);
+        if (ret != 0) {
+            error_report("Unable to read KVM reg val %s, error %d",
+                         cbomz_cfg->name, ret);
+            exit(EXIT_FAILURE);
+        }
+
+        if (cbomz_cfg->user_set) {
+            reg = kvm_cpu_cfg_get(cpu, cbomz_cfg);
+            if (reg != host_val) {
+                error_report("Unable to set %s to a different value than "
+                             "the host (%lu)",
+                             cbomz_cfg->name, host_val);
+                exit(EXIT_FAILURE);
+            }
+        }
+
+        kvm_cpu_cfg_set(cpu, cbomz_cfg, host_val);
+    }
+}
+
 static void kvm_riscv_add_cpu_user_properties(Object *cpu_obj)
 {
     int i;
@@ -344,6 +425,14 @@  static void kvm_riscv_add_cpu_user_properties(Object *cpu_obj)
                             kvm_cpu_set_multi_ext_cfg,
                             NULL, multi_cfg);
     }
+
+    for (i = 0; i < ARRAY_SIZE(kvm_cbomz_blksize_cfgs); i++) {
+        RISCVCPUMultiExtConfig *cbomz_size_cfg = &kvm_cbomz_blksize_cfgs[i];
+
+        object_property_add(cpu_obj, cbomz_size_cfg->name, "uint16",
+                            NULL, kvm_cpu_set_cbomz_blksize,
+                            NULL, cbomz_size_cfg);
+    }
 }
 
 void kvm_riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str,
@@ -856,6 +945,7 @@  int kvm_arch_init_vcpu(CPUState *cs)
 
     kvm_riscv_update_cpu_misa_ext(cpu, cs);
     kvm_riscv_update_cpu_cfg_isa_ext(cpu, cs);
+    kvm_riscv_finalize_features(cpu, cs);
 
     return ret;
 }