From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 470997803D7 for ; Fri, 12 Jan 2024 14:47:09 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=mt70floXxDsWhgt2KPsoqKIUMaKKlZprczSBLfcLCmg=; c=relaxed/simple; d=groups.io; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type:Content-Transfer-Encoding; s=20140610; t=1705070827; v=1; b=C+w3ijNjqtCZfhJ1mv9XO/QMHEeG2CAdDiw0iEj0YhkJmILIyt71GKiIBjslYAc3ZxZJE5ww Rf776gLJJM7ncCo5mGTkx3ubbEdviQlNRyZtbIOKt7LSEHrwLoEHaxCVMkkR/ExRgKdTciGH/Ng AgCK2/6Qj9bCkZ21KbkhXiPI= X-Received: by 127.0.0.2 with SMTP id WEuiYY7687511xj2FBDt2qkQ; Fri, 12 Jan 2024 06:47:07 -0800 X-Received: from mail-vk1-f175.google.com (mail-vk1-f175.google.com [209.85.221.175]) by mx.groups.io with SMTP id smtpd.web11.8736.1705070827236024474 for ; Fri, 12 Jan 2024 06:47:07 -0800 X-Received: by mail-vk1-f175.google.com with SMTP id 71dfb90a1353d-4b76f902854so1183698e0c.3 for ; Fri, 12 Jan 2024 06:47:07 -0800 (PST) X-Gm-Message-State: hKMOV9VwZytTbPdSLif4qalBx7686176AA= X-Google-Smtp-Source: AGHT+IHu6wzFPxy+j18tLt+wCo0MmbNU1VAvdYsX/92bjBNdluv1DHQxXjUxcT9JlIiR+ZJTgB4Tzuc4KmHRi7VhpPU= X-Received: by 2002:a05:6122:c8a:b0:4b6:dce2:23a9 with SMTP id ba10-20020a0561220c8a00b004b6dce223a9mr857146vkb.8.1705070824890; Fri, 12 Jan 2024 06:47:04 -0800 (PST) MIME-Version: 1.0 References: <44ca139f-4d78-4322-b5b6-8e9788bb7486@os.amperecomputing.com> <2ad16043-754e-3bb9-3a4a-702d9a50bf63@redhat.com> <45b95719-f1fc-dbc6-a4cc-a022d691844c@redhat.com> <30901646-905c-798c-d088-255498028fff@redhat.com> <7659165f-a73b-b628-59fe-c29e67beb850@redhat.com> In-Reply-To: <7659165f-a73b-b628-59fe-c29e67beb850@redhat.com> From: "Pedro Falcato" Date: Fri, 12 Jan 2024 14:46:53 +0000 Message-ID: Subject: Re: [edk2-devel] Memory Attribute for depex section To: Laszlo Ersek Cc: devel@edk2.groups.io, nhi@os.amperecomputing.com, "ardb+tianocore@kernel.org" , Andrew Fish Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,pedro.falcato@gmail.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=C+w3ijNj; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=gmail.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io On Fri, Jan 12, 2024 at 9:35=E2=80=AFAM Laszlo Ersek wr= ote: > > On 1/12/24 03:10, Pedro Falcato wrote: > > My idea was to make each handle an index - like a file descriptor - > > AFAIK there's no reason why it *needs* to be an actual pointer. > > We'd allocate indices when creating a handle, and return that (casted t= o VOID*). > > Huh, sneaky. > > I see two potential problems with this. > > First, per C std, these "pointers" would be invalid (they wouldn't > actually point to valid memory), so even just evaluating them (not > dereferencing them!) would invoke undefined behavior. May or may not > matter in practice. With compilers getting smarter about optimization > (and stricter about std conformance), there could be issues, perhaps. This is true. Stashing random integers in pointers is implementation-defined. But it's also super common. Win32 HANDLEs are exactly this, just integers (stashed in VOID*). The Linux kernel world also has a bunch of fun tricks with stashing flags in a pointer's bottom bits, magic pointer values, etc. I severely doubt we can run into issues with this. EDK2 will not exactly run on the C standard's abstract machine anyway ;) > > The other concern is a bit contrived, but I *guess* there could be code > out there that actually dereferences EFI_HANDLEs. That code would crash. > May or may not matter, because such code is arguably already > semantically invalid. (It would not necessarily be invalid at the > language level -- cf. my previous paragraph --, because passing an > otherwise valid EFI_HANDLE to CopyMem, for copying just 1 byte out of > the underlying opaque data structure, would not violate the language.) This is a feature, not a bug! :P Seriously though, IHANDLE is not even exposed in semi-public headers, so any code that's derefing an EFI_HANDLE will need to do something like typedef struct { /* ... */ } IHANDLE; EFI_HANDLE Handle =3D /* ... */; IHANDLE *HandleImpl =3D (IHANDLE *) Handle; and I'm a strong believer in "play stupid games, win stupid prizes". You can definitely make an argument for "this should definitely crash" instead of just "maybe crashing" (for instance, platforms that still map the NULL page (like OVMF!), or handles > 4096), so I'm inclined to think that if we indeed go this route, we should set one or two upper bits (on 64-bit platforms!) to make handles non-canonical addresses and therefore necessarily crash on dereference. > > > I should note that I find it super hard to get a concrete idea on > > performance for EFI firmware without adequate tooling - I meant to > > write a standalone flamegraph tool a few weeks back (even posted in > > edk2-devel), but, as far as I could tell, the architectural timer > > protocol couldn't get me the interrupt frame[1]. Until then, whether > > any of this radix tree vs RB tree vs flat array stuff really > > matters... I find it hard to say. > > > > [1] x86 has 3 timers (PIT, LAPIC timer, HPET) and performance > > monitoring interrupts, and I couldn't freely use any of them :^) > > Edk2 has some form of profiling already (see > "MdePkg/Include/Library/PerformanceLib.h"). Usually one sees core code > peppered with PERF_CODE_BEGIN and PERF_CODE_END macros. I *think* there > is something like a "display performance" (dp) shell application too, > that can show the collected stats. But I've never used these facilities. > > The wiki seems to have two related articles: > > https://github.com/tianocore/tianocore.github.io/wiki/Edk2-Performance-In= frastructure > > https://github.com/tianocore/tianocore.github.io/wiki/PerformancePkg > > The former looks quite comprehensive, at a quick skim. /me nods I've seen those macros around, but I've never used them. In any case, this problem has piqued my interest, I'll see if I can find some free time this weekend to hack on a test benchmark and a PoC :) --=20 Pedro -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113741): https://edk2.groups.io/g/devel/message/113741 Mute This Topic: https://groups.io/mt/103594587/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-