[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] xen: Don't cast away const-ness in vcpu_show_registers()
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 <andrew.cooper3@xxxxxxxxxx> > --- > CC: Anthony PERARD <anthony.perard@xxxxxxxxxx> > CC: Michal Orzel <michal.orzel@xxxxxxx> > CC: Jan Beulich <jbeulich@xxxxxxxx> > CC: Julien Grall <julien@xxxxxxx> > CC: Roger Pau Monné <roger.pau@xxxxxxxxxx> > CC: Stefano Stabellini <sstabellini@xxxxxxxxxx> > CC: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx> > CC: Bertrand Marquis <bertrand.marquis@xxxxxxx> > > 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 <jbeulich@xxxxxxxx> 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
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |