Re: [PATCH V3] target/riscv: Align the data type of reset vector address

[Peter Maydell]

On Fri, 26 Mar 2021 at 10:21, Dylan Jhong <dylan@andestech.com> wrote:
> Currently, there is no structure like "qdev_prop_target_ulong".
> So, we still need to use an if-else condition to determine the attributes of 
> the 5th parameter.
> Something like this:
>     #if defined(TARGET_RISCV32)
>         DEFINE_PROP_UNSIGNED("resetvec", RISCVCPU, cfg.resetvec, 
> DEFAULT_RSTVEC, qdev_prop_uint32 target_ulong),
>     #elif defined(TARGET_RISCV64)
>         DEFINE_PROP_UNSIGNED("resetvec", RISCVCPU, cfg.resetvec, 
> DEFAULT_RSTVEC, qdev_prop_uint64 target_ulong),
>     #endif
> I think this is not be what you meant.
>
> The other architectures seem to ignore this, they just choose one of the 
> attributes(qdev_prop_uint32/64) as their parameter.
>
> So now we have 2 options:
> 1. Use "qdev_prop_uint64" as the 5th parameter
>     DEFINE_PROP_UNSIGNED("resetvec", RISCVCPU, cfg.resetvec, DEFAULT_RSTVEC, 
> qdev_prop_uint64 target_ulong),
>
> 2. Use if-else condition
>     [patch v3]
>
> Or if you have other opinions, please bring them up and discuss them together.

I would recommend that you just use DEFINE_PROP_UINT64 for this property
(and store the value in a uint64_t cfg.resetvec) regardless of whether the
guest CPU happens to be 32 or 64 bit. In the case where it's 32 bits you
can just ignore the top 32 bits (or if you're feeling enthusiastic, report
an error if they're non-zero). This is simpler to code, avoids ifdefs,
and tends to mean that most code doesn't need to care about the 32-vs-64
difference.

thanks
-- PMM