Re: [RFC, PATCH 5/5] Paravirt_ops export.patch

[Jeremy Fitzhardinge <jeremy@goop.org>]

Zachary Amsden wrote:
> Jeremy Fitzhardinge wrote:
>>
>> Well, the BUG is if the patch-size is smaller than sizeof(indirect
>> call).  Or more generally, if the patch site contains bogus crud.  But I
>> don't think checking for that in the patcher makes a great deal of
>> sense.
>>   
>
> True, it may not make a lot of sense to check there.  But it is the
> only place where we can check it at runtime.  Unless you want some
> kind of compile time checking in the code which generates the patch
> data, say perhaps a .abort.  Still, that is not safe against modules
> which have broken paravirt-ops embedding.

Well, they shouldn't be doing it for themselves; it will always be as a
result of some macro expansion from asm/paravirt.h.  And if they're
broken in a module, they'll be broken in the kernel too.

And you should be able to check for patch-site sanity by booting on
native with patching disabled.  That should fall over pretty quickly if
there's something wrong.

> Since there are so few of these sites, and they are performance
> critical, the placement of a BUG() here verifying the clobbers are ok
> effectively makes it the paravirt-op backend's responsibility to
> ensure that these sites get patched.  This is what would allow the
> removal of dependence on the paravirt_ops symbol for the most
> important interrupt control operations.

I'm not keen on *requiring* patching to occur, in general.  The
performance without patching is only a few percent worse than
native/patched, so its not like its a desperate problem to defer
implementing it.  (I.e, requiring patching just raises the
implementation barrier for a pv_ops backend.)

Overloading patching for dealing with module exports is interesting, but
well, I guess I don't see the problem in just exporting paravirt_ops. 
The two arguments I've seen against it are "security" and "GPL issues",
but neither seems particularly good.

> It's semantically the same, but the code needs to be re-usable later,
> which is where the problems arise - you can't simply cram the prelink
> code into head.S, but have to write it to be properly callable later on. 

I think that's OK.  It can just be plain C code, with very few
infrastructure dependencies.  It wouldn't need to allocate memory, for
example.  It should be fairly simple to make the code callable either
from early in the kernel's life or later on.

    J