On 01/14/18 15:42, Nathan Whitehorn wrote:
On 01/14/18 09:57, Nathan Whitehorn wrote:
On 01/14/18 09:52, Konstantin Belousov wrote:
On Sun, Jan 14, 2018 at 09:30:53AM -0800, Nathan Whitehorn wrote:
The immediate consequence of that is that no MI code that knows about
direct maps can possibly take advantage of the direct map on this
platform. Do we really want that to save some conditional logic that
would get optimized out on amd64 and arm64 anyway? I really do not see
the benefit here.
It is not clear what do you mean. Are you saying that there is no
benefit
of providing the conditional logic, or that it is not benefit of
exclusing
powerpc ?
Sorry, that was poorly stated. Let me try again:
If we make a PPC_PHYS_TO_DMAP(), but there is an MI PHYS_TO_DMAP()
API, consumer code in the MI parts of the kernel won't be able to
benefit from the PPC direct map, which seems unfortunate. The cost
from a code perspective of having an if (direct_map_available) seems
low, since on systems where direct_map_available is defined to be 1,
the compiler will optimize it to the same code as if gated by #ifdef.
It might be more cumbersome to write the code, however.
I do not object against adding the conditional, but it should not be
too clumsy to use.
OK. Let me try to draft something in the next couple days and see how
much of a pain it is in practice.
-Nathan
How about the attached? It makes PHYS_TO_DMAP() return 0 if no mapping
exists. This is straightforward, does not introduce extra macros, and
can pretty easily replace SFBUF_OPTIONAL_DIRECT_MAP on the assumption
that PHYS_TO_DMAP() is cheap. I've modified the other MI-ish consumers
in the tree accordingly; compat/linuxkpi/common/src/linux_page.c
already does the right thing and needed no modifications.
-Nathan
Sorry, this is the patch I meant to send.
-Nathan
Index: powerpc/include/vmparam.h
===================================================================
--- powerpc/include/vmparam.h (revision 327952)
+++ powerpc/include/vmparam.h (working copy)
@@ -240,13 +240,12 @@
#define SFBUF_PHYS_DMAP(x) (x)
/*
- * We (usually) have a direct map of all physical memory. All
- * uses of this macro must be gated by a check on hw_direct_map!
- * The location of the direct map may not be 1:1 in future, so use
- * of the macro is recommended; it may also grow an assert that hw_direct_map
- * is set.
+ * We (usually) have a direct map of all physical memory, so provide
+ * a macro to use to get the kernel VA address for a given PA. Returns
+ * 0 if the direct map is unavailable. The location of the direct map
+ * may not be 1:1 in future, so use of the macro is recommended.
*/
-#define PHYS_TO_DMAP(x) x
-#define DMAP_TO_PHYS(x) x
+#define PHYS_TO_DMAP(x) (hw_direct_map ? (x) : 0)
+#define DMAP_TO_PHYS(x) (hw_direct_map ? (x) : 0)
#endif /* _MACHINE_VMPARAM_H_ */
Index: vm/vm_page.c
===================================================================
--- vm/vm_page.c (revision 327952)
+++ vm/vm_page.c (working copy)
@@ -2937,7 +2937,8 @@
{
#if defined(DIAGNOSTIC) && defined(PHYS_TO_DMAP)
- if ((m->flags & PG_ZERO) != 0) {
+ if ((m->flags & PG_ZERO) != 0 &&
+ PHYS_TO_DMAP(VM_PAGE_TO_PHYS(m)) != 0) {
uint64_t *p;
int i;
p = (uint64_t *)PHYS_TO_DMAP(VM_PAGE_TO_PHYS(m));
Index: dev/efidev/efirt.c
===================================================================
--- dev/efidev/efirt.c (revision 327952)
+++ dev/efidev/efirt.c (working copy)
@@ -115,6 +115,11 @@
return (0);
}
efi_systbl = (struct efi_systbl *)PHYS_TO_DMAP(efi_systbl_phys);
+ if (efi_systbl == NULL) {
+ if (bootverbose)
+ printf("EFI systbl not mapped in kernel VA\n");
+ return (0);
+ }
if (efi_systbl->st_hdr.th_sig != EFI_SYSTBL_SIG) {
efi_systbl = NULL;
if (bootverbose)
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
