Jeremy Fitzhardinge wrote:
> Wrap the paravirt_ops members we want to export in wrapper functions.
> Since we binary-patch the critical ones, this doesn't make a speed
> impact.
>
> I moved drm_follow_page into the core, to avoid having to wrap the
> various pte ops. Unlining kernel_fpu_end and using that in the RAID6
> code would remove the need to export clts/read_cr0/write_cr0 too.
>
> Signed-off-by: Rusty Russell <[email protected]>
> Signed-off-by: Chris Wright <[email protected]>
> Signed-off-by: Jeremy Fitzhardinge <[email protected]>
>
> ===================================================================
> --- a/arch/i386/kernel/paravirt.c
> +++ b/arch/i386/kernel/paravirt.c
> @@ -596,6 +596,123 @@ static int __init print_banner(void)
> return 0;
> }
> core_initcall(print_banner);
> +
> +unsigned long paravirt_save_flags(void)
> +{
> + return paravirt_ops.save_fl();
> +}
> +EXPORT_SYMBOL(paravirt_save_flags);
> +
> +void paravirt_restore_flags(unsigned long flags)
> +{
> + paravirt_ops.restore_fl(flags);
> +}
> +EXPORT_SYMBOL(paravirt_restore_flags);
> +
> +void paravirt_irq_disable(void)
> +{
> + paravirt_ops.irq_disable();
> +}
> +EXPORT_SYMBOL(paravirt_irq_disable);
> +
> +void paravirt_irq_enable(void)
> +{
> + paravirt_ops.irq_enable();
> +}
> +EXPORT_SYMBOL(paravirt_irq_enable);
>
This turned out really hideous looking to me. Can't we split the struct
into GPL'd and non-GPL'd functions instead? We still have the same
granularity, and none of this function call to an indirect function call
nonsense.
Zach
Zachary Amsden wrote:
> This turned out really hideous looking to me. Can't we split the
> struct into GPL'd and non-GPL'd functions instead? We still have the
> same granularity, and none of this function call to an indirect
> function call nonsense.
It's not pretty, but I think having paravirt_ops and paravirt_ops_gpl
would be worse. I'd be sympathetic to the idea of splitting
paravirt_ops up by function groupings, but splitting it by license seems
like a maintenance headache with no real upside. Besides, patching will
solve everything (tm).
J
Jeremy Fitzhardinge wrote:
> Zachary Amsden wrote:
>
>> This turned out really hideous looking to me. Can't we split the
>> struct into GPL'd and non-GPL'd functions instead? We still have the
>> same granularity, and none of this function call to an indirect
>> function call nonsense.
>>
>
> It's not pretty, but I think having paravirt_ops and paravirt_ops_gpl
> would be worse. I'd be sympathetic to the idea of splitting
> paravirt_ops up by function groupings, but splitting it by license seems
> like a maintenance headache with no real upside. Besides, patching will
> solve everything (tm).
>
Ok. As long as we plan on patching CR2 and CR0 / clts accessors for FPU
save during context switch and page fault paths in the future.
Zachary Amsden wrote:
> Ok. As long as we plan on patching CR2 and CR0 / clts accessors for
> FPU save during context switch and page fault paths in the future.
That's up to each backend, right? Or do these need to be patched for a
correctness reason?
J
Jeremy Fitzhardinge wrote:
> Zachary Amsden wrote:
>
>> Ok. As long as we plan on patching CR2 and CR0 / clts accessors for
>> FPU save during context switch and page fault paths in the future.
>>
>
> That's up to each backend, right? Or do these need to be patched for a
> correctness reason?
>
No, it needs to be part of the general patch list first, which is still
hand listed rather than just any op being patchable. Then it can be up
to the backend.
Zach
Zachary Amsden wrote:
> No, it needs to be part of the general patch list first, which is
> still hand listed rather than just any op being patchable. Then it
> can be up to the backend.
Ah, right, yes. We need to add all (most? some?) of the pte operations
to that list too.
J
On Tue, 2007-02-13 at 17:06 -0800, Zachary Amsden wrote:
> Jeremy Fitzhardinge wrote:
> > Wrap the paravirt_ops members we want to export in wrapper functions.
> > Since we binary-patch the critical ones, this doesn't make a speed
> > impact.
>
> This turned out really hideous looking to me. Can't we split the struct
> into GPL'd and non-GPL'd functions instead? We still have the same
> granularity, and none of this function call to an indirect function call
> nonsense.
This patch, indeed, should not have been pushed in this series. But not
for that reason: I actually prefer explicit exports.
KVM and lguest need more symbols, so the real patch will make them use
native_XXX versions explicitly...
Cheers,
Rusty.
On Tue, Feb 13, 2007 at 05:06:42PM -0800, Zachary Amsden wrote:
> >I moved drm_follow_page into the core, to avoid having to wrap the
> >various pte ops. Unlining kernel_fpu_end and using that in the RAID6
> >code would remove the need to export clts/read_cr0/write_cr0 too.
Please don't push the drm_follow_page part. Dave and I fixed drm
to not need this junk anymore, and it should be part of the next drm merge.