> > > This move is required to enable building without TCG. > > > All the logic related to registering SPRs specific to > > > some architectures or machines has been hidden in this > > > new file. > > > > Hm... I thought we ended up deciding to keep the gen_spr_<machine> > > functions in translate_init.c.inc (cpu_init.c). I don't strongly favour > > one way or the other, but one alternative would be to just rename the > > gen_spr_<machine> functions to add_sprs_<machine> or even > > register_<machine>_sprs and leave them where they are. > > Right. I think renaming these away from gen_*() is a good idea, to > avoid confusion with the other gen_*() functions which, well, generate > TCG code. > > I don't think there's a lot of value in moving them out from > translate_init. Honestly the more useful way to break up > translate_init would be by CPU family, rather than by SPR vs. non-SPR > setup
Right, ok. I thought we agreed to separate, but I can't remember the reason. I guess less is more in this case, so I won't create the new file. As for separating by CPU family: sounds good for a later cleanup, but I don't think it's a priority right now. I'll work on that renaming, I agree its a good idea. > > > The idea of this final patch is to hide all SPR generation from > > > translate_init, but in an effort to simplify the RFC the 4 > > > functions for not_implemented SPRs were created. They'll be > > > substituted by gen_spr_<machine>_misc in reusable ways for the > > > final patch. > > > > I'd expect this patch to be just a big removal of gen_spr* from > > translate_init.c.inc and their addition into spr_common.c. So any other > > prep work should come in separate patches ealier in the > > series. Specifically, at least one patch for the macro work and another > > for the refactoring of open coded spr_register calls into gen_spr_* > > functions. > > Seconded. Ok. Should it be 2 commits (refactoring, then macro) or 3 commits (renaming, vscr_init, then macro)? I guess also that since I won't move stuff, I don't need to fix the vscr_init, but it's no big deal at this point. > > If you're only adding this now, it means the previous patch is > > broken. As a general rule you should make sure every patch works. I know > > that for the RFC things might be a bit broken temporarily and that is ok > > but it is always a good idea to make sure every individual change is in > > the correct patch at least. yeah... sorry about that. I'm correcting all these problems. > > > +/*****************************************************************************/ > > > +/* SPR definitions and registration */ > > > + > > > +#ifdef CONFIG_TCG > > > +#ifdef CONFIG_USER_ONLY > > > +#define spr_register_kvm(env, num, name, uea_read, uea_write, > > > \ > > > + oea_read, oea_write, one_reg_id, initial_value) > > > \ > > > + _spr_register(env, num, name, uea_read, uea_write, initial_value) > > > +#define spr_register_kvm_hv(env, num, name, uea_read, uea_write, > > > \ > > > + oea_read, oea_write, hea_read, hea_write, > > > \ > > > + one_reg_id, initial_value) > > > \ > > > + _spr_register(env, num, name, uea_read, uea_write, initial_value) > > > +#else /* CONFIG_USER_ONLY */ > > > +#if !defined(CONFIG_KVM) > > > +#define spr_register_kvm(env, num, name, uea_read, uea_write, > > > \ > > > + oea_read, oea_write, one_reg_id, initial_value) > > > \ > > > + _spr_register(env, num, name, uea_read, uea_write, > > > \ > > > + oea_read, oea_write, oea_read, oea_write, > > > initial_value) > > > +#define spr_register_kvm_hv(env, num, name, uea_read, uea_write, > > > \ > > > + oea_read, oea_write, hea_read, hea_write, > > > \ > > > + one_reg_id, initial_value) > > > \ > > > + _spr_register(env, num, name, uea_read, uea_write, > > > \ > > > + oea_read, oea_write, hea_read, hea_write, > > > initial_value) > > > +#else /* !CONFIG_KVM */ > > > +#define spr_register_kvm(env, num, name, uea_read, uea_write, > > > \ > > > + oea_read, oea_write, one_reg_id, initial_value) > > > \ > > > + _spr_register(env, num, name, uea_read, uea_write, > > > \ > > > + oea_read, oea_write, oea_read, oea_write, > > > \ > > > + one_reg_id, initial_value) > > > +#define spr_register_kvm_hv(env, num, name, uea_read, uea_write, > > > \ > > > + oea_read, oea_write, hea_read, hea_write, > > > \ > > > + one_reg_id, initial_value) > > > \ > > > + _spr_register(env, num, name, uea_read, uea_write, > > > \ > > > + oea_read, oea_write, hea_read, hea_write, > > > \ > > > + one_reg_id, initial_value) > > > +#endif /* !CONFIG_KVM */ > > > +#endif /* CONFIG_USER_ONLY */ > > > +#else /* CONFIG_TCG */ > > > +#ifdef CONFIG_KVM /* sanity check */ > > > +#define spr_register_kvm(env, num, name, uea_read, uea_write, > > > \ > > > + oea_read, oea_write, one_reg_id, initial_value) > > > \ > > > + _spr_register(env, num, name, one_reg_id, initial_value) > > > +#define spr_register_kvm_hv(env, num, name, uea_read, uea_write, > > > \ > > > + oea_read, oea_write, hea_read, hea_write, > > > \ > > > + one_reg_id, initial_value) > > > \ > > > + _spr_register(env, num, name, one_reg_id, initial_value) > > > +#else /* CONFIG_KVM */ > > > +#error "Either TCG or KVM should be configured" > > > +#endif /* CONFIG_KVM */ > > > +#endif /*CONFIG_TCG */ > > > + > > > +#define spr_register(env, num, name, uea_read, uea_write, > > > \ > > > + oea_read, oea_write, initial_value) > > > \ > > > + spr_register_kvm(env, num, name, uea_read, uea_write, > > > \ > > > + oea_read, oea_write, 0, initial_value) > > > + > > > +#define spr_register_hv(env, num, name, uea_read, uea_write, > > > \ > > > + oea_read, oea_write, hea_read, hea_write, > > > \ > > > + initial_value) > > > \ > > > + spr_register_kvm_hv(env, num, name, uea_read, uea_write, > > > \ > > > + oea_read, oea_write, hea_read, hea_write, > > > \ > > > + 0, initial_value) > > > > This change is crucial for this to work, we don't want it buried along > > with all of the code movement. It should be clearly described and in > > it's own patch at the top of the series. > > > > If you add this change, plus some #ifdef TCG around the spr callbacks, > > it should already be possible to compile this with disable-tcg. It would > > also make the series as a whole easier to understand. if we added this and removed TCG only files from meson.build it might compile already (not sure, I think there were a couple of things that used mmu functions), but wouldn't there be way too many ifdefs in that case? The R/W callbacks are spread all around the file, and we'd have to ifdef around some .h files that are included in common files. Bruno Piazera Larsen Instituto de Pesquisas ELDORADO<http://clickemailmkt.eldorado.org.br/ls/click?upn=UPoxpeIcHnAcbUZyo7TTaswyiVb1TXP3jEbQqiiJKKGsxOn8hBEs5ZsMLQfXkKuKXZ7MVDg0ij9eG8HV4TXI75dBzDiNGLxQ8Xx5PzCVNt6TpGrzBbU-2Biu0o69X5ce-2FW-2FOk1uUipuK0fZnWXJEgbRw-3D-3DJY4T_wWk-2BG6VvNBoa1YzxYjhCdFS9IfANIaBzDSklR1NyyrKOI1wj0P-2BdBFcuO4FnHcsA1MyHu0ly1Yt3oDMp7KKdJPM68iKuI2jiRH5v4B0d8wf3chU3qy5n5iXWnW1QjSaNFHOgELzxaP-2FnesTeBgJ5dFkjH4f279sVQpOtyjw5xAqj34M6pgNRAxVvuXif4IWDcVzXg1FzfYlEfkKzr9vvpA3Hg8kitwMtlU3zwbQUBCgL30fQoJPcRPMGKyOY8RmoAlXNqTJYDYIvqmfnI7KLUvw6vKB5R-2B5q1FJRAzX7H-2BmF0NnDET6jMLuIqtCcVIch> Departamento Computação Embarcada Analista de Software Trainee Aviso Legal - Disclaimer<https://www.eldorado.org.br/disclaimer.html>
