On 03.03.2025 17:52, Andrew Cooper wrote: > On 26/02/2025 7:33 am, Jan Beulich wrote: >> 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]> > > Thanks. > >> irrespective of the fact that a function of this name and purpose really, >> really >> should be taking a pointer-to-const. > > No - this is a perfect example of why most functions should *not* take > pointer-to-const for complex objects. > > There is no such thing as an actually-const vcpu or domain; they are all > mutable. The reason why x86 needs a strictly-mutable pointer is because > it needs to take a spinlock to negotiate for access to a hardware > resource to read some of the registers it needs. > > This is where there is a semantic gap between "logically doesn't modify" > and what the C keyword means.
And hence (in part) why C++ gained "mutable" ages ago. > Anything except the-most-trivial trivial predates may reasonably need to > take a spinlock or some other safety primitive, even just to read > information. > > > Because this was gratuitously const in the first place, bad code was put > in place of making the prototype match reality. > > This demonstrates a bigger failing in how code is reviewed and > maintained. It is far too frequent that requests to const things don't > even compile. It is also far too frequent that requests to const things > haven't read the full patch series to realise why not. Both of these > are a source of friction during review. > > But more than that, even if something could technically be const right > now, the request to do so forces churn into a future patch to de-const > it in order to make a clean change. And for contributors who aren't > comfortable saying a firm no to a maintainer, this turns into a bad hack > instead. > > i.e. requests to const accessors for complexity objects are making Xen > worse, not better, and we should stop doing it. Okay, let's agree that we don't agree in our perspectives here. Jan
