Xen devel - Jan 2007 - netfront pv driver building

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).

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.

Thanks, Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

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?

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

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?

Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

>>> 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


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 24/1/07 14:23, "Jan Beulich" <jbeulich@novell.com> wrote:
> 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.
Setting it to zero would make it very obvious if any usage became live, then
we could think harder about this at that time.

 -- Keir


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel