On Fri, Sep 20, 2019 at 02:22:13PM +0200, Mark Kettenis wrote:
> > Date: Fri, 20 Sep 2019 02:55:27 -0700
> > From: Mike Larkin <[email protected]>
> >
> > On Fri, Sep 20, 2019 at 01:09:56AM +0900, YASUOKA Masahiko wrote:
> > > Hi,
> > >
> > > I recently got a VAIO Pro PK. The diff below is required to boot.
> > > Without the diff, it freezes during boot.
> > >
> >
> > > Its EFI framebuffer is located 0x4000000000 (9 zeros). This is > 4GB
> > > and higher than highest available memory of the machine. These
> > > configuraions seem to cause the problem.
> > >
> > > * * *
> > >
> > > Call cninit() after pmap_bootstrap() is called. Since the EFI
> > > framebuffer may be located > 4GB which is not initialized by locore,
> > > but by pmap_bootstrap(). Also make the address parameter passed to
> > > pmap_bootstrap() cover the framebuffer. Actually VAIO pro PK's
> > > framebuffer is located higher than the highest available memory
> > > region.
> > >
> > > ok? comments?
> > >
> >
> > Hi,
> >
> > I have a few questions...
> >
> > 1. There seems to be no limit on the max PA that we extend to here.
> > This means, for example, if EFI places the framebuffer past 2TB
> > PA, we won't have enough direct map to cover the mapping. Plus
> > I think this will end up extending the direct map to cover any hole
> > between "end of phys mem" and "efi fb addr". At a minimum, I think
> > we need some sort of max PA clamp here. I don't know what Sony's
> > placement algorithm is, but 0x4000000000 is 256GB PA.
>
> A dmesg and pcidump output would be useful.
>
> I suspect that this is a discrete graphics card where the EFI frame
> buffer resides in VRAM. Using the direct map in this case is probably
> not the right thing to do.
>
> > 2. What does delaying cninit do for machines that have errors or
> > printfs before this? Would those even print anymore? This would
> > affect all machines, even those without efifb, correct?
>
> Yes and no. It doesn't affect the classic VGA glass console, but it
> does mean serial output might disappear. That isn't acceptable I'd
> say.
>
> > 3. I am not a big fan of placing device-specific quirks in
> > init_x86_64. Could this not be done in the efifb specific console
> > init code? You could pmap_enter whatever you wanted there, based on
> > the PA EFI sent you. Or does efifb go through the direct map for
> > all video output? If so, we may be stuck creating that big direct map
> > range. If that's the case though, we should probably try to restrict
> > the permissions in the unused holes.
>
> The direct map is only used early on in the boot process. The frame
> buffer is remapped in mainbus_attach() such that we can use
> write-combining. But that is done after we print copyright. I think
> the remapping could be done a bit earlier, but not before uvm gets
> initialized, which happens after we print the copyright message.
>
> We don't have to use the direct map during early boot. If you gave us
> some other way to map the framebuffer before pmap_bootstrap() has been
> called we could stick that into efifb_cnattach_common(). We'd need
> your help with that though. Note that the framebuffer can be fairly
> large though (but we can probably come up with a reasonable upper
> limit).
>
What sort of function do you need? Map this PA range at X, but before
pmap_bootstrap?
> Cheers,
>
> Mark
>
> > > Index: sys/arch/amd64/amd64/machdep.c
> > > ===================================================================
> > > RCS file: /cvs/src/sys/arch/amd64/amd64/machdep.c,v
> > > retrieving revision 1.259
> > > diff -u -p -r1.259 machdep.c
> > > --- sys/arch/amd64/amd64/machdep.c 7 Sep 2019 19:05:44 -0000
> > > 1.259
> > > +++ sys/arch/amd64/amd64/machdep.c 19 Sep 2019 15:55:18 -0000
> > > @@ -193,6 +193,8 @@ int lid_action = 1;
> > > int pwr_action = 1;
> > > int forceukbd;
> > >
> > > +int docninit;
> > > +
> > > /*
> > > * safepri is a safe priority for sleep to set for a spin-wait
> > > * during autoconfiguration or after a panic.
> > > @@ -1371,6 +1373,7 @@ init_x86_64(paddr_t first_avail)
> > > bios_memmap_t *bmp;
> > > int x, ist;
> > > uint64_t max_dm_size = ((uint64_t)512 * NUM_L4_SLOT_DIRECT) << 30;
> > > + paddr_t max_pa;
> > >
> > > cpu_init_msrs(&cpu_info_primary);
> > >
> > > @@ -1541,7 +1544,16 @@ init_x86_64(paddr_t first_avail)
> > > * Call pmap initialization to make new kernel address space.
> > > * We must do this before loading pages into the VM system.
> > > */
> > > - first_avail = pmap_bootstrap(first_avail, trunc_page(avail_end));
> > > + max_pa = avail_end;
> > > + /* Make sure max_pa covers the EFI frame buffer */
> > > + if (bios_efiinfo->fb_addr != 0 &&
> > > + max_pa < bios_efiinfo->fb_addr + bios_efiinfo->fb_size)
> > > + max_pa = bios_efiinfo->fb_addr + bios_efiinfo->fb_size;
> > > + first_avail = pmap_bootstrap(first_avail, trunc_page(max_pa));
> > > +
> > > + /* Call cninit after entire physical memory is available */
> > > + if (docninit > 0)
> > > + cninit();
> > >
> > > /* Allocate these out of the 640KB base memory */
> > > if (avail_start != PAGE_SIZE)
> > > @@ -1914,7 +1926,6 @@ getbootinfo(char *bootinfo, int bootinfo
> > > bios_ddb_t *bios_ddb;
> > > bios_bootduid_t *bios_bootduid;
> > > bios_bootsr_t *bios_bootsr;
> > > - int docninit = 0;
> > >
> > > #undef BOOTINFO_DEBUG
> > > #ifdef BOOTINFO_DEBUG
> > > @@ -2026,8 +2037,6 @@ getbootinfo(char *bootinfo, int bootinfo
> > > break;
> > > }
> > > }
> > > - if (docninit > 0)
> > > - cninit();
> > > #ifdef BOOTINFO_DEBUG
> > > printf("\n");
> > > #endif
> > >
> >
> >
>
