Re: Vendor CPU overrides of non-extension properties

[Daniel Henrique Barboza]

Hi Rob,


I just posted some patches that hopefully will address the issue you described:

20231221175137.497379-1-dbarboza@ventanamicro.com/T/#m10b05b26b86be368f7a131de57656374f679e432">https://lore.kernel.org/qemu-riscv/20231221175137.497379-1-dbarboza@ventanamicro.com/T/#m10b05b26b86be368f7a131de57656374f679e432


Feel free to test and review. Let me know if I missed anything.


Thanks,

Daniel


On 12/15/23 08:49, Rob Bradford wrote:
> Hi Daniel,
>
> Thanks for your work on improving the extension handling for vendor
> CPUs. I have a query regarding the correct way to override properties
> that are included in the "cpu_options" list as part of the vendor CPU
> initialisation function.
>
> i.e. these properties:
>
> Property riscv_cpu_options[] = {
>      DEFINE_PROP_UINT32("pmu-mask", RISCVCPU, cfg.pmu_mask,
> MAKE_64BIT_MASK(3, 16)),
>      {.name = "pmu-num", .info = &prop_pmu_num}, /* Deprecated */
>
>      DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
>      DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
>
>      DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
>      DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),
>
>      DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
>      DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
>
>      DEFINE_PROP_UINT16("cbom_blocksize", RISCVCPU, cfg.cbom_blocksize,
> 64),
>      DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU, cfg.cboz_blocksize,
> 64),
>
>      DEFINE_PROP_END_OF_LIST(),
> };
>
> I think that these values should be specifiable as part of the vendor
> CPU initialisation method however unfortunately they are overwritten
> with the default values from the properties as
> riscv_cpu_add_user_properties which adds the properties is called from
> the type post_init method which is called after the vendor CPU init
> function - so any changes to e.g. cfg.vlen are lost.
>
> I would appreciate hearing your thoughts on how these configuration
> values can be set for vendor CPUs.
>
> Cheers,
>
> Rob