On Fri, Mar 26, 2021 at 7:11 AM Peter Maydell <peter.mayd...@linaro.org> wrote: > > On Fri, 26 Mar 2021 at 10:21, Dylan Jhong <dy...@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.
That sounds good to me. Alistair > > thanks > -- PMM