Re: [Xen-devel] [PATCH] x86/PV: avoid indirect call/thunk in I/O emulation

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.02.18 at 16:50, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 16/02/18 08:00, Jan Beulich wrote:
>>>>> On 15.02.18 at 17:53, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 15/02/18 16:03, Jan Beulich wrote:
>>>> --- a/xen/arch/x86/pv/emul-priv-op.c
>>>> +++ b/xen/arch/x86/pv/emul-priv-op.c
>>>> @@ -73,55 +73,42 @@ void (*pv_post_outb_hook)(unsigned int p
>>>>  
>>>>  typedef void io_emul_stub_t(struct cpu_user_regs *);
>>>>  
>>>> -void __x86_indirect_thunk_rcx(void);
>>>> -
>>>>  static io_emul_stub_t *io_emul_stub_setup(struct priv_op_ctxt *ctxt, u8 
>>>> opcode,
>>>>                                            unsigned int port, unsigned int 
>>>> bytes)
>>>>  {
>>>>      struct stubs *this_stubs = &this_cpu(stubs);
>>>>      unsigned long stub_va = this_stubs->addr + STUB_BUF_SIZE / 2;
>>>> +    signed long disp;
>>> I'm not in favour of sprinkling 'signed' all over the code base.
>>>
>>> long is already unambiguous, and a far more common construct to encounter. 
>> It's this "common" which made me use "signed" here (and in similar
>> places elsewhere) - we still have far too many cases left where plain
>> short/int/long are used when the unsigned variant would actually be
>> better. Hence, _at least_ until those issues are all gone, I very much
>> prefer making explicit when signed quantities are meant.
> 
> But why does it actually matter?  Noone can blindly switch between
> prefixed and unprefixed versions without understanding what is going on,
> and disp here is a signed usage.
> 
> Also, are you proposing that the end goal for a "clean" coding style is
> to have all native C types explicitly prefixed with signed or unsigned? 
> If not (and I really hope not), then putting signed in here is causing
> extra work for us to undo later.

Personally I would indeed prefer for "signed" to always be added
when a signed quantity is meant. Of course you have to look at
the context when considering to switch a plain use to unsigned,
but quickly recognizing non-candidates of such a transformation
is possible only when they are amended by "signed".

That said, if dropping the "signed" is the only way to get your ack
(and besides the expression folding you had no other comments
so far), I certainly will (reluctantly).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel