From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f49.google.com (mail-wr1-f49.google.com [209.85.221.49]) by mx.groups.io with SMTP id smtpd.web11.369.1608228273220440662 for ; Thu, 17 Dec 2020 10:04:33 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=WuBKJ9/R; spf=pass (domain: nuviainc.com, ip: 209.85.221.49, mailfrom: leif@nuviainc.com) Received: by mail-wr1-f49.google.com with SMTP id r7so27515203wrc.5 for ; Thu, 17 Dec 2020 10:04:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=ds0SLvss9RDeQKBUOW733d3TumsdJqfPj9t+3yw1PYM=; b=WuBKJ9/RgZLDkc0xSRxQsO0X5nosCy5yxjWoVcT0R2Eb/1GT9x87VzQ8lpJUTI4uzM xFu8wrFrBOSE39Wf1u2Pe8RU9sCedDQsoCrT5Qnk2LwyiRg/YzgGA676xTNVGDZdOTOz f7bVepsLR4fu7oodfto3VD9IlqCnu3ya5MxqIDvkIt+e1CvkJmQrH2j8NVJzuORWvOOZ ksLSJWv16e0Kk1xHNFJ9M9b+pt13Uhqm3Iu4rbhMfA4J7OLWoYR/YZX+GrsEn6DoJdXi k9tA2ixiGnKX2I/sWlulP8bYy/Ih3xh1lyJVdBL24i49ku9M6OXu3XUg5+Q3DpDvT9P+ XFrQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=ds0SLvss9RDeQKBUOW733d3TumsdJqfPj9t+3yw1PYM=; b=LrbcCQxoCr2D/S/x0jj3D8Bz4Fp2eovghoBIaJejbEILiYPjehkc123vgYTcqTryMa /sudMKKPUSKau0HjKiRcv14rpu0Be0iaaaeKyucLbr+bAQZ7MdaSptRgCiwooxnNG+5n H3txzgWmyVMugCXX1zqr01bl6qgM+ufYehzmrGlohCE6R7CWi+2nBmQw0s/fZIqYUjjk 0O4DApneIVAool/qqo53PzI1TtbN8uoZYxxkHsLgOq1KzuQXHGHSA0Lh/5nCYiBS4GtF uPWQbc73f3z2a+38pQHPNUVRkvpcaBp0oiCVDyySoocz9Qvs73eX62nqyKaPe9jYCj/v V4HA== X-Gm-Message-State: AOAM531RhqbADTptb6Xb+hOfK0N5u0AQrSiUjijUA3VhFRi7zMJ0Bodf P0unkeDxrYLe2ZjGoot4sbhheA== X-Google-Smtp-Source: ABdhPJzLWdrKL/1HgLn+MWAM5f9UcZsPjBDrojlsNDLTKMDHQPCbEWZ0CUeIgmC8OhVKCOcDo4iOIQ== X-Received: by 2002:adf:f989:: with SMTP id f9mr39252092wrr.299.1608228271688; Thu, 17 Dec 2020 10:04:31 -0800 (PST) Return-Path: Received: from vanye (cpc1-cmbg19-2-0-cust915.5-4.cable.virginm.net. [82.27.183.148]) by smtp.gmail.com with ESMTPSA id x7sm8689156wmi.11.2020.12.17.10.04.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Dec 2020 10:04:31 -0800 (PST) Date: Thu, 17 Dec 2020 18:04:29 +0000 From: "Leif Lindholm" To: Ard Biesheuvel Cc: Laszlo Ersek , Rebecca Cran , devel@edk2.groups.io Subject: Re: [PATCH v4 04/10] ArmPkg: Add helper to read the Memory Model Features Register 2 Message-ID: <20201217180429.GS1664@vanye> References: <20201207175427.28712-1-rebecca@nuviainc.com> <20201207175427.28712-5-rebecca@nuviainc.com> <20201215191114.GD1664@vanye> <76d90616-dc08-0c65-0ccf-d9c99259e99c@redhat.com> <3e9ab014-ef61-435b-e2a3-cb0c9950b7f9@arm.com> MIME-Version: 1.0 In-Reply-To: <3e9ab014-ef61-435b-e2a3-cb0c9950b7f9@arm.com> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Thu, Dec 17, 2020 at 14:47:48 +0100, Ard Biesheuvel wrote: > >> ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c: if (ArmReadIdPfr0 () & AARCH64_PFR0_GIC) { > >> ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c: if (ArmReadIdPfr1 () & ARM_PFR1_GIC) { > >> > >> I would propose that since the high-level abstraction serve only to > >> confuse things, we change existing (and new) accessors to ID registers > >> to be explicit: > >> > >> - ArmReadIdAArch64Mmfr0 > >> - ArmReadIdAArch64Pfr0 > >> - ArmReadIdAArch64Pfr1 > > > > I can follow until here... (and yes, using the concrete register names > > in the function names makes sense) > > > >> > >> The question is whether we should make the AArch32 aspect explicit or > >> implicit? My instinctive reaction is the latter. This matches the > >> native naming scheme used in the ARM ARM, and we don't support mixing > >> instruction set widths in UEFI. > > > > I lost you here, sorry. > > > > So did I :-) > > But I think that we should raise the level of abstraction here: > something like > > if (ArmReadIdPfr0 () & AARCH64_PFR0_GIC) { > > should not exist in code that is shared between AArch64 and AArch32, I'd > much rather have a helper > > ArmHasGicSre() > > that encapsulates whatever is needed on each respective architecture, > and which may or may not end up using the same ID register or mask value. Yeah, agreed. In the end, that's what Rebecca's set does in patch 6/10 - so I'll whip something together to replicate this for the GIC case. I like the ArmHas* format though - Rebecca, could you use that for your v5? > > >> > >> The AArch64 prototypes should then only be made available to AARCH64 > >> code, and the AArch32 ones only to ARM. > > > > But this again makes sense to me. > > > > I guess what confuses me is your interpretation of "implicit" vs. > > "explicit". I'm missing what the "AArch32 aspect" means, probably. > > > > Thanks > > laszlo > > >