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"

Reply via email to