On 01/15/18 15:42, Konstantin Belousov wrote:
On Mon, Jan 15, 2018 at 03:20:49PM -0800, Nathan Whitehorn wrote:
Fair enough. Here's a patch with a new flag (DIRECT_MAP_AVAILABLE). I've
also retooled the sfbuf code to use this rather than its own flags that
mean the same things. The sparc64 part of the patch is untested.
-Nathan
Index: amd64/include/vmparam.h
===================================================================
--- amd64/include/vmparam.h (revision 328006)
+++ amd64/include/vmparam.h (working copy)
@@ -190,6 +190,7 @@
* because the result is not actually accessed until later, but the early
* vt fb startup needs to be reworked.
*/
+#define DIRECT_MAP_AVAILABLE 1
#define PHYS_TO_DMAP(x) ({
\
KASSERT(dmaplimit == 0 || (x) < dmaplimit, \
("physical address %#jx not covered by the DMAP", \
Index: arm64/include/vmparam.h
===================================================================
--- arm64/include/vmparam.h (revision 328006)
+++ arm64/include/vmparam.h (working copy)
@@ -176,6 +176,7 @@
#define VIRT_IN_DMAP(va) ((va) >= DMAP_MIN_ADDRESS && \
(va) < (dmap_max_addr))
+#define DIRECT_MAP_AVAILABLE
Just define, or define it to 1 ?
Yes, sorry for typo.
#define PHYS_TO_DMAP(pa)
\
({ \
KASSERT(PHYS_IN_DMAP(pa), \
Index: dev/efidev/efirt.c
===================================================================
--- dev/efidev/efirt.c (revision 328006)
+++ 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);
+ }
Is this chunk still needed ?
The existing code is a bit of an awkward superposition of the "return
NULL" idea and having the flag. Since you think there will never be
intermediate cases -- which seems reasonable -- I will rip the
conditional logic out and add a KASSERT matching the ones on arm64 and
amd64 to the powerpc version.
if (efi_systbl->st_hdr.th_sig != EFI_SYSTBL_SIG) {
efi_systbl = NULL;
if (bootverbose)
Index: kern/subr_sfbuf.c
===================================================================
--- kern/subr_sfbuf.c (revision 328006)
+++ kern/subr_sfbuf.c (working copy)
@@ -88,8 +88,8 @@
vm_offset_t sf_base;
int i;
-#ifdef SFBUF_OPTIONAL_DIRECT_MAP
- if (SFBUF_OPTIONAL_DIRECT_MAP)
+#ifdef DIRECT_MAP_AVAILABLE
+ if (DIRECT_MAP_AVAILABLE)
return;
Would it make sense to define the symbol on all other arches as 0 then,
and remove #ifdef ? Returning to your initial proposal of relying on the
compiler optimiing if (0) block; out.
That is a good idea.
Also, just curious, why did you spelled DMAP as DIRECT_MAP ?
DMAP without the PHYS_TO_ seemed lacking in context and I was worried
there might be a collision on DMAP. PMAP_HAS_DMAP would also work; I
don't have a preference.
Thanks for your patience working this out in real time with me.
-Nathan
_______________________________________________
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"