On Sat, Jul 29, 2023 at 10:19:17AM +0100, Marc Zyngier wrote: > On Fri, 28 Jul 2023 19:33:31 +0100, > Oliver Upton <oliver.upton@xxxxxxxxx> wrote: > > Correct me if I'm wrong, but I don't think the compiler is going to > > whine if any of these bitfields are initialized with a larger value than > > can be represented... Do you think some BUILD_BUG_ON() is in order to > > ensure that trap_group fits in ::cgt? > > > > BUILD_BUG_ON(__NR_TRAP_IDS__ >= BIT(10)); > > Indeed. This might also apply to ::fgt, and I want to add some sanity > checks to verify that the whole union isn't larger than a 'void *', as > we rely on that. Agreed on both. > > > +/* > > > + * Map encoding to trap bits for exception reported with EC=0x18. > > > + * These must only be evaluated when running a nested hypervisor, but > > > + * that the current context is not a hypervisor context. When the > > > + * trapped access matches one of the trap controls, the exception is > > > + * re-injected in the nested hypervisor. > > > + */ > > > +static const struct encoding_to_trap_config encoding_to_cgt[] __initconst = { > > > +}; > > > + > > > +static DEFINE_XARRAY(sr_forward_xa); > > > + > > > +static union trap_config get_trap_config(u32 sysreg) > > > +{ > > > + return (union trap_config) { > > > + .val = xa_to_value(xa_load(&sr_forward_xa, sysreg)), > > > + }; > > > > Should we be checking for NULL here? AFAICT, the use of sentinel values > > in the trap_group enum would effectively guarantee each trap_config has > > a nonzero value. > > if xa_load() returns NULL, xa_to_value() will still give us a 0, which > is an indication of a sysreg that isn't present in the trap > configuration. This can happen if we trap something that isn't yet > supported in NV, which is quite common. This allows us to use features > on the host without having to immediately write the same support for > NV guests. That's fine by me, I couldn't piece it together where a value of 0 is explicitly handled as having no trap config. > But this is obviously a temporary situation. At some point, I'll > become a total bastard and demand that people treat NV as a first > class citizen. One day ;-). No time like the present! -- Thanks, Oliver