Paravirt: fix export of paravirt-ops to binary modules
cover Paravirt: fix export of paravirt-ops to binary modules
A clever set of manipulations allows us to fix binary modules with paravirt-ops by just not exporting paravirt_ops at all and using the new patching code.
What do you think? Experimental patches here... only partially tested, but booting and appear to be a working prototype.
Thanks,
Zach
Quoted 5 lines · On Friday 20 April 2007 03:52:14 Zachary Amsden wrote:
A clever set of manipulations allows us to fix binary modules with paravirt-ops by just not exporting
paravirt_opsat all and using the new patching code.What do you think? Experimental patches here... only partially tested, but booting and appear to be a working prototype.
Basic idea looks good.
But is the 5 argument support really needed? I don't see any paravirt
ops functions that needs it and even if there was one it still wouldn't be clear if it made sense to export it.
-Andi
Quoted 5 lines · Andi Kleen
Basic idea looks good.
But is the 5 argument support really needed? I don't see any paravirt
ops functions that needs it and even if there was one it still wouldn't be clear if it made sense to export it.
64-bit args are passed as a pair of 32-bit args, so PAE set_pte_at* functions end up with 5 args. They're used often, so they may be worth patching, but I'm not sure if they're worth exporting. They are implicitly exported in a non-PARAVIRT build by the fact that they're inline functions/macros in a header. I don't know if any modules actually use them.
J
Quoted 8 lines · Jeremy Fitzhardinge
Quoted 5 lines · Andi Kleen
Basic idea looks good.
But is the 5 argument support really needed? I don't see any paravirt ops functions that needs it and even if there was one it still wouldn't be clear
if it made sense to export it.
Quoted 7 lines
64-bit args are passed as a pair of 32-bit args, so PAE set_pte_at* functions end up with 5 args. They're used often, so they may be worth patching, but I'm not sure if they're worth exporting. They are implicitly exported in a non-PARAVIRT build by the fact that they're inline functions/macros in a header. I don't know if any modules actually use them.
Yes, I don't know either - but we should be consistent about their export regardless of whether PAE is selected or not. So we should either have 5 arg macros or not export set_pte_at* at all, even in 4 arg version.
Zach
Quoted 4 lines · Zachary Amsden
Yes, I don't know either - but we should be consistent about their export regardless of whether PAE is selected or not. So we should either have 5 arg macros or not export set_pte_at* at all, even in 4 arg version.
So can you separately control exportability independently from patchability?
J
Quoted 10 lines · Jeremy Fitzhardinge
Quoted 5 lines · Zachary Amsden
Yes, I don't know either - but we should be consistent about their export regardless of whether PAE is selected or not. So we should either have 5 arg macros or not export set_pte_at* at all, even in 4 arg version.
So can you separately control exportability independently from patchability?
Nope. But I think we should maintain parity between PAE and non-PAE - and probably also maintain parity between non-paravirt-ops and paravirt-ops, which means the 5 arg macros are a good idea. It's not like they are very hard to create or maintain.
Zach
Quoted 4 lines · Zachary Amsden
Nope. But I think we should maintain parity between PAE and non-PAE - and probably also maintain parity between non-paravirt-ops and paravirt-ops, which means the 5 arg macros are a good idea. It's not like they are very hard to create or maintain.
Yep. Did you see the comment about not being able to use "rm" for arg4?
J
Quoted 10 lines · Jeremy Fitzhardinge
Quoted 5 lines · Zachary Amsden
Nope. But I think we should maintain parity between PAE and non-PAE - and probably also maintain parity between non-paravirt-ops and paravirt-ops, which means the 5 arg macros are a good idea. It's not like they are very hard to create or maintain.
Yep. Did you see the comment about not being able to use "rm" for arg4?
Yes, thanks, I fixed that. The I got lost in info gcc looking for a non-stack memory constraint modifier, but it evaded me.
Zach
Quoted 9 lines · Zachary Amsden
Quoted 8 lines · Jeremy Fitzhardinge
Quoted 5 lines · Zachary Amsden
Yes, I don't know either - but we should be consistent about their export regardless of whether PAE is selected or not. So we should either have 5 arg macros or not export set_pte_at* at all, even in 4 arg version.
Quoted 7 lines
Quoted 2 lines
So can you separately control exportability independently from patchability?
Nope. But I think we should maintain parity between PAE and non-PAE - and probably also maintain parity between non-paravirt-ops and paravirt-ops, which means the 5 arg macros are a good idea. It's not like they are very hard to create or maintain.
Less exports good. Consistency with all config options isn't a hard
requirement: I'd be tempted not to export the pte functions.
Rusty.
Quoted 3 lines
Less exports good. Consistency with all config options isn't a hard
requirement: I'd be tempted not to export the pte functions.
Yes paravirt_ops should be probably split into two for internal and external available functions. Any takers?
-Andi
Quoted 5 lines · Andi Kleen
Quoted 2 lines
Less exports good. Consistency with all config options isn't a hard
requirement: I'd be tempted not to export the pte functions.Yes
paravirt_opsshould be probably split into two for internal and external available functions. Any takers?
Hi Andi!
I'm a little uncomfortable with cutting the struct this way: I always
thought it'd be a function split if we did one.
With the new patch code we could simply shuffle all the exportables to
the front and fail to patch calls past this point (then fail the module load).
I'll see what I can come up with...
Rusty.
Quoted 11 lines · On Monday 23 April 2007 02:24:05 Rusty Russell wrote:
Quoted 5 lines · Andi Kleen
Quoted 2 lines
Less exports good. Consistency with all config options isn't a hard
requirement: I'd be tempted not to export the pte functions.Yes
paravirt_opsshould be probably split into two for internal and external available functions. Any takers?Hi Andi!
I'm a little uncomfortable with cutting the struct this way: I always
thought it'd be a function split if we did one.
It's a functional split, isn't it? arch/mm internal and "exported" to other users
Quoted 3 lines
With the new patch code we could simply shuffle all the exportables to
the front and fail to patch calls past this point (then fail the module load).
Two structures would seem cleaner to me than such hacks.
-Andi
Quoted 8 lines · Andi Kleen
Quoted 7 lines · On Monday 23 April 2007 02:24:05 Rusty Russell wrote:
Quoted 5 lines · Andi Kleen
Quoted 2 lines
Less exports good. Consistency with all config options isn't a hard
requirement: I'd be tempted not to export the pte functions.Yes
paravirt_opsshould be probably split into two for internal and external available functions. Any takers?
Quoted 7 lines
Quoted 4 lines
Hi Andi!
I'm a little uncomfortable with cutting the struct this way: I always
thought it'd be a function split if we did one.It's a functional split, isn't it?
arch/mminternal and "exported" to other users
Hi,
When I said functional I was thinking "page table ops" vs "apic ops"
etc. There's little logic to what needs exporting.
Most modules only need the interrupt operations. A small handful want
more, and then some (kvm, lguest) need a whole range of crap (these should use the native_ versions directly, since nested paravirt is not supported).
I did the work before; I'll drag it back out and see what the symbols are again...
Rusty.
Quoted 26 lines · Rusty Russell
Quoted 10 lines · Andi Kleen
Quoted 8 lines · On Monday 23 April 2007 02:24:05 Rusty Russell wrote:
Quoted 6 lines · Andi Kleen
Quoted 3 lines
Less exports good. Consistency with all config options isn't a hard
requirement: I'd be tempted not to export the pte functions.Yes
paravirt_opsshould be probably split into two for internal and external available functions. Any takers?Quoted 8 lines
Quoted 5 lines
Hi Andi!
I'm a little uncomfortable with cutting the struct this way: I always
thought it'd be a function split if we did one.It's a functional split, isn't it?
arch/mminternal and "exported" to other usersHi,
When I said functional I was thinking "page table ops" vs "apic ops"
etc. There's little logic to what needs exporting.
Quoted 8 lines
Most modules only need the interrupt operations. A small handful want
more, and then some (kvm, lguest) need a whole range of crap (these should use thenative_versions directly, since nested paravirt is not supported).I did the work before; I'll drag it back out and see what the symbols are again...
Interrupt control is needed in general. CPUID is also widely used. FPU control (clts, read / write cr0) is required for certain software operations.
There are non-obvious exports that are required for some modules; in particular, MSRs may be used in video drivers, and wbinvd must be available to hardware drivers. We may wish to simply trap / emulate these instructions and remove the paravirt-ops altogether. They are not performance critical in 32-bit, and will complicate loading non-GPL drivers for a wide range of hardware - hardware that is not exposed in a virtualized environment to begin with.
Some rather more interesting modules map kernel page tables to read the physical frame numbers from ptes; this is
safe to do if you get_pages or get_user_pages and keep the pages locked. This requires kmap_atomic_pte to be available. Similarly, pte_val and make_pte need to be available. Mostly only modules that require a native environment (virtualization modules) need them, but we need some way to either resolve the symbol as load time or make the requirement for the symbol go away. No matter how you look at it, failing to load a module because of an unresolved paravirt-op that is actually going to be reduced to a nop is a very silly situation to be in.
Zach
Quoted 11 lines · Rusty Russell
Quoted 6 lines · Andi Kleen
Quoted 3 lines
Less exports good. Consistency with all config options isn't a hard
requirement: I'd be tempted not to export the pte functions.Yes
paravirt_opsshould be probably split into two for internal and external available functions. Any takers?Hi Andi!
Quoted 8 lines
I'm a little uncomfortable with cutting the struct this way: I always
thought it'd be a function split if we did one.With the new patch code we could simply shuffle all the exportables to
the front and fail to patch calls past this point (then fail the module load).I'll see what I can come up with...
There was some discussion about hybrid paravirt/fully virt hypervisor models at the Xen summit. There are definitely some interesting things you can do by having a mostly paravirtualized guest running within an hvm container.
One of the things I was considering as a result was splitting the privileged instruction ops (all the cr?, msr, segment stuff) into a cpu_ops, and putting the pagetable stuff into pagetable_ops. That would make a broad split of shadow vs direct pagetables and hvm vs non-hvm models pretty straightforward, and probably sharing more interfaces than they might otherwise.
J