Re: [PATCH v3 4/4] xen/x86: switch x86 to use generic implemetation of bug.h

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.02.2023 17:28, Oleksii wrote:
> On Mon, 2023-02-27 at 15:46 +0100, Jan Beulich wrote:
>> On 24.02.2023 12:31, Oleksii Kurochko wrote:
>>> --- a/xen/arch/x86/include/asm/debugger.h
>>> +++ b/xen/arch/x86/include/asm/debugger.h
>>> @@ -14,9 +14,9 @@
>>>  
>>>  /* Returns true if GDB handled the trap, or it is surviveable. */
>>>  static inline bool debugger_trap_fatal(
>>> -    unsigned int vector, struct cpu_user_regs *regs)
>>> +    unsigned int vector, const struct cpu_user_regs *regs)
>>>  {
>>> -    int rc = __trap_to_gdb(regs, vector);
>>> +    int rc = __trap_to_gdb((struct cpu_user_regs *)regs, vector);
>>
>> I view casts in general as risky and hence preferable to avoid.
>> Casting
>> away const-ness is particularly problematic. I guess the least bad
>> option is to drop const from the do_bug_frame()'s parameter again in
>> patch 1. However, for a fatal trap (assuming that really _is_ fatal,
>> i.e. execution cannot continue), not allowing register state to be
>> altered makes kind of sense. So I wonder whether we couldn't push the
>> casting into the gdbstub functions. But quite likely that's going to
>> be too intrusive for re-work like you do here.
> I am not sure that casting inside the gdbstub functions will help as
> do_bug_frame() calls debugger_trap_fatal() which has define regs 
> without const. So it will still be compile issue as in do_bug_frame()
> regs are declared as const.

Well, the idea was of course to propagate const further down, to cast
it away only in restricted cases. But ...

> So it looks like the best one issue now will be drop const from the
> do_bug_frame()...

... looks like we're in agreement then about this being the better
route for the time being.

Jan