Re: [Xen-devel] [PATCH v3 3/7] x86: make PV hypercall entry points work with !CONFIG_PV

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Thu, Nov 08, 2018 at 08:49:44AM -0700, Jan Beulich wrote:
> >>> On 08.11.18 at 16:36, <andrew.cooper3@xxxxxxxxxx> wrote:
> > On 08/11/18 15:33, Wei Liu wrote:
> >> On Mon, Nov 05, 2018 at 09:11:44AM -0700, Jan Beulich wrote:
> >>>>>> On 05.11.18 at 16:49, <andrew.cooper3@xxxxxxxxxx> wrote:
> >>>> On 05/11/18 15:48, Wei Liu wrote:
> >>>>> On Mon, Nov 05, 2018 at 08:04:37AM -0700, Jan Beulich wrote:
> >>>>>>>>> On 02.11.18 at 16:55, <wei.liu2@xxxxxxxxxx> wrote:
> >>>>>>> --- a/xen/arch/x86/x86_64/traps.c
> >>>>>>> +++ b/xen/arch/x86/x86_64/traps.c
> >>>>>>> @@ -298,8 +298,21 @@ static unsigned int write_stub_trampoline(
> >>>>>>>  }
> >>>>>>>  
> >>>>>>>  DEFINE_PER_CPU(struct stubs, stubs);
> >>>>>>> +
> >>>>>>> +#ifdef CONFIG_PV
> >>>>>>>  void lstar_enter(void);
> >>>>>>>  void cstar_enter(void);
> >>>>>>> +#else
> >>>>>>> +static inline void lstar_enter(void)
> >>>>>>> +{
> >>>>>>> +    panic("%s called", __func__);
> >>>>>>> +}
> >>>>>>> +
> >>>>>>> +static inline void cstar_enter(void)
> >>>>>>> +{
> >>>>>>> +    panic("%s called", __func__);
> >>>>>>> +}
> >>>>>>> +#endif /* CONFIG_PV */
> >>>>>> Do we really need two separate stubs (and two separate string literals)
> >>>>>> here?
> >>>>> I think it is clearer if we have two distinct messages. But I'm not too
> >>>>> fussed either way really. If you feel strongly about this, I'm happy to
> >>>>> change it to only one function.
> >>>> This is the correct way to do it.  __func__ will already be in the
> >>>> string table, and the format string (being identical) will be merged.
> >>> Why would __func__ be in the string table already, for functions
> >>> containing no other references to it?
> >> What is the way forward? Do we really care if there is one more string
> >> literal in the binary?
> > 
> > No.  One extra string like this is not something which needs caring
> > about in the slightest.
> 
> So just to clarify - the unnecessary redundancy in the source is of more
> relevance to me than the resulting slightly larger binary.

So you would rather have

    static inline void lcstar_enter(void)
    {
        panic("lstar/cstar\n");
    }

?

TBH I think this is not as good as using two functions. I'm up for
sending a followup patch to do whatever it is agreed upon, but for now
I'm intending to commit this patch as-is.

Wei.

> 
> Jan
> 
> 

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