Re: netfront pv driver building

["Jan Beulich" <jbeulich@novell.com>]

>>> Ian Campbell <Ian.Campbell@XenSource.com> 24.01.07 15:05 >>>
>On Wed, 2007-01-24 at 13:46 +0000, Jan Beulich wrote:
>> netfront requires the symbol __supported_pte_mask when PAE is enabled in the
>> kernel being built for. That symbol, however, isn't being exported, and it doesn't
>> seem likely that mainline would want to see this get exported (after all, the
>> general assumption is that the page table handling inline functions and macros
>> are supposed to be used only by the memory management code).
>
>I committed a patch the other day which defines __supported_pte_mask to
>~0ULL when building the unmodified drivers. It is 13555:687b1120765e in
>xen-unstable. You are using 3.0.4-testing?

I did look at -unstable, but I only checked the sparse tree... But yes, I found
the problem in 3.0.4-testing.

>(it occurs to me now that I might have wanted ~PAGE_NX for greatest
>compatibility, need to think about that a bit more.)

Yes, I think this is dangerous, and I would rather want to see it reverted,
or as a minimum, as you say, converted to ~PAGE_NX (although I dislike
the lost bit of security here).
Especially because, as I said, at present the symbol is needed only for
dead code (so you don't really test whether the change would actually
work the way you want it to), and if at some point it makes it into active
code, we may end up with ugly issues.

>I wonder how this affects the PV-on-PV version when built as a module.
>Presumably we export the symbol in the Xen tree but as you say that is
>unlikely to go upstream.
>
>> Additionally, both uses are inside a conditional depending upon
>> !XENFEAT_auto_translated_physmap, which hence can't ever be true if this
>> code is being built as pv driver. Wouldn't it hence make sense to #ifdef these
>> code blocks, or to enhance xen/features.h so that checks for those features
>> that are always on in hvm and return a constant 1 rather than using the
>> xen_features array.
>
>Is it necessary with the fix I mentioned above?

With above fix it wouldn't be necessary, but as above, I don't think we should
override the OS here.

Jan