On 10/08/17 16:39, Josh Poimboeuf wrote:
> On Thu, Aug 10, 2017 at 04:24:58PM +0200, Juergen Gross wrote:
>>>> I'll send some patches to:
>>>>
>>>> - remove xen_patch()
>>>> - remove lguest
>>>> - remove vsmp
>>>>
>>>> In case nobody objects to apply those patches we can possibly simplify
>>>> some more code.
>>>>
>>>> I'd love that. :-)
>>>
>>> Well, I might have spoken too soon about getting rid of vsmp. The
>>> scalemp.com domain still exists. The code hasn't changed much in three
>>> years, but maybe it's simple enough that it hasn't needed to change.
>>
>> Lets see. I have made the experience that asking whether some code can
>> be removed almost never get answers. Sending a patch which actually
>> removes the stuff results much more often in objections. :-)
>>
>>> Also, looking at the lguest mailing list, there seem to have been at
>>> least a few people trying lguest out in the past year or so.
>>
>> Well, yes. The question is here whether there is a _need_ for lguest
>> or was it just out of curiosity?
>>
>> In the end it is 32 bit only and you can easily test boot code via
>> KVM, Xen or qemu.
>
> Good points. I'm all for removing code, so you have no objections from
> me :-)
>
>>> Even if we couldn't get rid of vsmp or lguest, I wonder if the PVOP_CALL
>>> stuff could be reworked to something like the following:
>>>
>>> static inline notrace unsigned long arch_local_save_flags(void)
>>> {
>>> return PVOP_CALLEE0(unsigned long, pv_irq_ops.save_fl,
>>> "pushfq; popq %rax", CPU_FEATURE_NATIVE,
>>> "call __raw_callee_save_xen_save_fl", CPU_FEATURE_XEN,
>>> "call __raw_callee_save_vsmp_save_fl", CPU_FEATURE_VSMP,
>>> "call __raw_callee_save_lguest_save_fl", CPU_FEATURE_LGUEST);
>>> }
>>>
>>> Which would eventually translate to something like:
>>>
>>> asm volatile(ALTERNATIVE_4("call *pv_irq_ops.save_fl",
>>> "pushfq; popq %rax", CPU_FEATURE_NATIVE,
>>> "call __raw_callee_save_xen_save_fl", CPU_FEATURE_XEN,
>>> "call __raw_callee_save_vsmp_save_fl", CPU_FEATURE_VSMP,
>>> "call __raw_callee_save_lguest_save_fl", CPU_FEATURE_LGUEST
>>> : ... pvop clobber stuff ... );
>>>
>>> where ALTERNATIVE_4 is a logical extension of ALTERNATIVE_2 and
>>> CPU_FEATURE_NATIVE would always be set.
>>>
>>> It might need some more macro magic, but if it worked I think it would
>>> be a lot clearer than the current voodoo.
>>>
>>> Thoughts?
>>
>> Hmm, this would modify the current approach of pvops completely: instead
>> of letting each user of pvops (xen, lguest, vsmp, ...) set the functions
>> it is needing, you'd have to modify the core definition of each pvops
>> function for each user.
>
> Right. The callers (arch_local_save_flags, etc) would have to know
> about the different hypervisors' functions. But this knowledge could be
> hidden in inline functions and/or macros, so I don't see it being too
> much of a problem.
>
> The upsides are that the behavior is much clearer (IMO), and we could
> get rid of the .parainstructions stuff altogether.
>
>> Or would you want to let Xen, lguest etc. opt in
>> for pvops and generate above code at build time from some templates?
>
> I'm not sure what you mean, can you clarify?
It shouldn't be too much work to let each pvops user have a file in a
common paravirt directory containing the needed information to create:
static inline notrace unsigned long arch_local_save_flags(void)
{
return PVOP_CALLEE0(unsigned long, pv_irq_ops.save_fl,
"pushfq; popq %rax", CPU_FEATURE_NATIVE,
"call __raw_callee_save_xen_save_fl", CPU_FEATURE_XEN,
"call __raw_callee_save_vsmp_save_fl", CPU_FEATURE_VSMP,
"call __raw_callee_save_lguest_save_fl", CPU_FEATURE_LGUEST);
}
and all other needed functions at build time. It could look e.g. like
(for xen: xen.pv):
@@feature CPU_FEATURE_XEN
PV_IRQ_OPS_SAVE_FL "call __raw_callee_save_xen_save_fl"
and the pre-processor could be used to assemble all configured users
(pvops.pv):
#ifdef CONFIG_XEN_PV
#include "xen.pv"
#endif
#ifdef CONFIG_LGUEST
#include "lguest.pv"
#endif
The resulting file would the be mangled by e.g. a python or awk script
to a header containing macro definitions like:
#define PV_IRQ_OPS_SAVE_FL \
"pushfq; popq %rax", CPU_FEATURE_NATIVE, \
"call __raw_callee_save_xen_save_fl", CPU_FEATURE_XEN, \
"call __raw_callee_save_vsmp_save_fl", CPU_FEATURE_VSMP, \
"call __raw_callee_save_lguest_save_fl", CPU_FEATURE_LGUEST
which can then be used in paravirt.h:
static inline notrace unsigned long arch_local_save_flags(void)
{
return PVOP_CALLEE0(unsigned long, pv_irq_ops.save_fl,
PV_IRQ_OPS_SAVE_FL);
}
Juergen
--
To unsubscribe from this list: send the line "unsubscribe live-patching" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
