On 26.02.2025 00:02, Andrew Cooper wrote: > The final hunk is `(struct vcpu *)v` in disguise, expressed using a runtime > pointer chase through memory and a technicality of the C type system to work > around the fact that get_hvm_registers() strictly requires a mutable pointer. > > For anyone interested, this is one reason why C cannot optimise any reads > across sequence points, even for a function purporting to take a const object. > > Anyway, have the function correctly state that it needs a mutable vcpu. All > callers have a mutable vCPU to hand, and it removes the runtime pointer chase > in x86. > > Make one style adjustment in ARM while adjusting the parameter type. > > Signed-off-by: Andrew Cooper <[email protected]> > --- > CC: Anthony PERARD <[email protected]> > CC: Michal Orzel <[email protected]> > CC: Jan Beulich <[email protected]> > CC: Julien Grall <[email protected]> > CC: Roger Pau Monné <[email protected]> > CC: Stefano Stabellini <[email protected]> > CC: Volodymyr Babchuk <[email protected]> > CC: Bertrand Marquis <[email protected]> > > RISC-V and PPC don't have this helper yet, not even in stub form. > > I expect there will be one objection to this patch. Since the last time we > fought over this, speculative vulnerabilities have demonstrated how dangerous > pointer chases are, and this is a violation of Misra Rule 11.8, even if it's > not reasonable for Eclair to be able to spot and reject it.
On these grounds Acked-by: Jan Beulich <[email protected]> irrespective of the fact that a function of this name and purpose really, really should be taking a pointer-to-const. Considering ... > --- a/xen/arch/arm/include/asm/domain.h > +++ b/xen/arch/arm/include/asm/domain.h > @@ -243,7 +243,7 @@ struct arch_vcpu > > } __cacheline_aligned; > > -void vcpu_show_registers(const struct vcpu *v); > +void vcpu_show_registers(struct vcpu *v); ... this and ... > --- a/xen/arch/x86/include/asm/domain.h > +++ b/xen/arch/x86/include/asm/domain.h > @@ -688,7 +688,7 @@ bool update_secondary_system_time(struct vcpu *v, > void force_update_secondary_system_time(struct vcpu *v, > struct vcpu_time_info *map); > > -void vcpu_show_registers(const struct vcpu *v); > +void vcpu_show_registers(struct vcpu *v); ... this are the same, and there's nothing different to expect for other architectures, centralizing the declaration and then adding a comment to cover the non-const property may be desirable. Else people like me might forget that there was this change, and try to re-add the seemingly missing const. Jan
