From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=0.2 required=5.0 tests=DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FROM autolearn=no autolearn_force=no version=3.4.4 Received: (qmail 6884 invoked from network); 19 Nov 2022 16:58:53 -0000 Received: from 9front.inri.net (168.235.81.73) by inbox.vuxu.org with ESMTPUTF8; 19 Nov 2022 16:58:53 -0000 Received: from mail-yw1-f175.google.com ([209.85.128.175]) by 9front; Sat Nov 19 11:56:54 -0500 2022 Received: by mail-yw1-f175.google.com with SMTP id 00721157ae682-368edbc2c18so76937527b3.13 for <9front@9front.org>; Sat, 19 Nov 2022 08:56:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :from:to:cc:subject:date:message-id:reply-to; bh=OhpD6KFxUu+3ac178eQvpUie+lV/yWaCrhKouuLj/QM=; b=Ckxzk28fdpoJ5ETFxZlQX3TRCfhTMUzxZJi3iIEI3m2YA6tp3pQDeUKoZr2dCPgLpG TpQ31Sx+crdvLj+oqhuN2t6NyebJt8ccQ0uSqQJk3wFDWyqC0T9Ya4a5OWeh/n/SWACj di++aNQjAHhac0snovWCD6FC4eKHJrr0oKny9NatY7aFIRaoZoMDjjpwBqMnTWpL3d3A UsKvxmURx1HxHw9e+sX7dvZL/Lsncq15DLGaxqSyqKKvHjf8/jtm28PxRbM8BzFDmbqv a7MwzLZJ09SfK1j5M204VkLUcI39OJOIgHVo/pfeHYOVc2wcS9LyPiQIb4tdzx8voT1/ 0VrA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=OhpD6KFxUu+3ac178eQvpUie+lV/yWaCrhKouuLj/QM=; b=s4WxOwR3EXGYcaNGRsPfNMVlbaZTQeQ7spXNTXnTtIKNKDXnRpxbkSQiWJJeI4Sd65 seOTD47vGrblUXsgt/X0seFGFj9/c/BBNIwybdHSmmSUXX0loAYThzSdmQ+DlnDrQIVm oO6bJrSK00uWR+5PRtpon4sP4fJ2m5NUYwl6mKXwK16DxAnMn+adgXBE9eYr28nWqds4 hrR/qwCuFUeYdWQNJ/ZwyRqzuskG+jQE208s4PuLWekECJwHX7y3mAfv1lAOUS1uCUXy DuqgBGt3Wr0DBGqjlQjA8MZcF7ErrlhP6v72uBy1sp4UdFg70x2L3qlQYjHy1+zLUvTC n2Dw== X-Gm-Message-State: ANoB5plRK2iT9rgZFAJ/C7pf8QLXmfSFbzWDMC+RZKLUDo30pdUWWWt0 oL8TAPOMdzRefaIL0a/SdlsAjnbSNOpMFQz+evbqDgl1 X-Google-Smtp-Source: AA0mqf7JwlU90nt1LwKdVE5cnWEVRvdkbycsCTre8yxNCMVmZ+JSxKvLIh/Q71P92T+fNvmwzb+ltAOtLavF1VXR5rk= X-Received: by 2002:a81:b621:0:b0:38d:c621:2385 with SMTP id u33-20020a81b621000000b0038dc6212385mr432027ywh.496.1668877010222; Sat, 19 Nov 2022 08:56:50 -0800 (PST) MIME-Version: 1.0 References: <058CCCF199316D00FB1ABF63EBD3B3A8@mforney.org> In-Reply-To: From: Lucas Francesco Date: Sat, 19 Nov 2022 13:56:34 -0300 Message-ID: To: 9front@9front.org Content-Type: text/plain; charset="UTF-8" List-ID: <9front.9front.org> List-Help: X-Glyph: ➈ X-Bullshit: overflow-preventing mobile cache SOAP over SVG descriptor CMS locator Subject: Re: [9front] [PATCH] boot/efi: delay GetMemoryMap until right before ExitBootServices Reply-To: 9front@9front.org Precedence: bulk Yup, works nicely here, tysm. On Sat, 19 Nov 2022 at 13:14, Lucas Francesco wrote: > > Oh, this seems to be the exact same problem I have with my devices > here. Was having empty nvme queues after identify (and reading the > spec everything else was mostly fine in our driver besides some quirks > to workaround shitty controllers) so i banged around to see if > firmware was messing it up before boot and realized the nvme feature > was pointing to that, couldn't figure out it was actually something > that would need to change in the bootloader though! (i banged > pcireset/nssr/initseg() a lot) > > Thanks so much for the find, will test this asap. > > On Wed, 16 Nov 2022 at 17:13, Michael Forney wrote: > > > > > > The UEFI specification suggests calling GetMemoryMap immediately > > before ExitBootServices to ensure that the loader has the current > > memory map. The firmware is able to enforce this since we have to > > pass a "MapKey" from the last GetMemoryMap to ExitBootServices. > > > > The T14 AMD gen 1 firmware is quite strict about this, and even a call > > to OutputString will invalidate the MapKey. This causes > > ExitBootServices to fail, and we enter 9front with boot services still > > running. This causes all sorts of problems including leaving the > > IOMMU enabled, breaking 9front's PCI drivers. > > > > To fix this, move memconf() to unload(), right before > > ExitBootServices. To retain the ability to override the memory map, > > check if *e820 is already set and if so, ignore the output of > > GetMemoryMap except for the MapKey. > > --- > > Still some room for improvement around the *conf helper functions, but > > I think this is fine for now. > > > > I also wonder what we should be doing if GetMemoryMap fails. If we > > don't have any potentially valid MapKey, how could ExitBootServices > > succeed? Perhaps we could make unload return an error string on > > failure, and pass it back from bootkern so that we can inform the user > > that something is wrong. > > > > diff a854bb07cd792a52c5e75aabca76c22b7dd18fc6 eb07ce3baa9705a30ecf3cd6b31ff851e24cb3d1 > > --- a/sys/src/boot/efi/efi.c > > +++ b/sys/src/boot/efi/efi.c > > @@ -36,12 +36,6 @@ > > eficall(ST->BootServices->Stall, (UINTN)us); > > } > > > > -void > > -unload(void) > > -{ > > - eficall(ST->BootServices->ExitBootServices, IH, MK); > > -} > > - > > static void > > memconf(char **cfg) > > { > > @@ -72,6 +66,8 @@ > > entvers = 1; > > if(eficall(ST->BootServices->GetMemoryMap, &mapsize, mapbuf, &MK, &entsize, &entvers)) > > return; > > + if(cfg == nil) > > + return; > > > > s = *cfg; > > for(p = mapbuf; mapsize >= entsize; p += entsize, mapsize -= entsize){ > > @@ -93,7 +89,6 @@ > > *s = '\0'; > > if(s > *cfg){ > > s[-1] = '\n'; > > - print(*cfg); > > *cfg = s; > > } > > } > > @@ -276,9 +271,15 @@ > > void > > eficonfig(char **cfg) > > { > > - memconf(cfg); > > acpiconf(cfg); > > screenconf(cfg); > > +} > > + > > +void > > +unload(char **cfg) > > +{ > > + memconf(cfg); > > + eficall(ST->BootServices->ExitBootServices, IH, MK); > > } > > > > EFI_STATUS > > --- a/sys/src/boot/efi/fns.h > > +++ b/sys/src/boot/efi/fns.h > > @@ -16,7 +16,7 @@ > > void (*close)(void *f); > > > > int readn(void *f, void *data, int len); > > -void unload(void); > > +void unload(char **cfg); > > > > int getc(void); > > void putc(int c); > > --- a/sys/src/boot/efi/sub.c > > +++ b/sys/src/boot/efi/sub.c > > @@ -156,6 +156,23 @@ > > > > char *confend; > > > > +static int > > +hasconf(char *s) > > +{ > > + char *p; > > + int n; > > + > > + n = strlen(s); > > + for(p = BOOTARGS; n <= confend - p; p++){ > > + if(memcmp(p, s, n) == 0) > > + return 1; > > + p = strchr(p, '\n'); > > + if(p == nil) > > + break; > > + } > > + return 0; > > +} > > + > > static char* > > getconf(char *s, char *buf) > > { > > @@ -364,7 +381,7 @@ > > > > close(f); > > print("boot\n"); > > - unload(); > > + unload(hasconf("*e820=") ? nil : &confend); > > > > jump(e); > >