public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Leif Lindholm <leif.lindholm@linaro.org>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
	"Laszlo Ersek" <lersek@redhat.com>,
	"Auger Eric" <eric.auger@redhat.com>,
	"Andrew Jones" <drjones@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Julien Grall" <julien.grall@linaro.org>
Subject: Re: [PATCH 2/5] ArmPkg/ArmMmuLib: take the CPU supported maximum PA space into account
Date: Mon, 26 Nov 2018 17:57:55 +0000	[thread overview]
Message-ID: <20181126175754.mbf42gg4jedbdxej@bivouac.eciton.net> (raw)
In-Reply-To: <CAKv+Gu9rG+cnX11FrCr5o4RJ2kgDzVDf22shZLEvpNzGVPFGQQ@mail.gmail.com>

On Mon, Nov 26, 2018 at 06:50:09PM +0100, Ard Biesheuvel wrote:
> On Mon, 26 Nov 2018 at 18:46, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> >
> > On Fri, Nov 23, 2018 at 01:14:28PM +0100, Ard Biesheuvel wrote:
> > > In preparation of permitting the virt code to define a larger PA space
> > > size via gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize than what the
> > > CPU actually supports, take the CPU's capabilities into account when
> > > setting up the page tables. This is necessary because KVM will shortly
> > > support variable PA space sizes, and to support running the same UEFI
> > > binaries regardless of that limit, PcdPrePiCpuMemorySize needs to be
> > > treated as an upper bound rather than a fixed size.
> >
> > Why do we keep PcdPrePiCpuMemorySize at all? (I.e., rather than just
> > using the probed value?
> > Mainly for the purpose of being able to restrict ourselves to 32/48
> > bits?
> >
> > If we keep it, should we rename
> > PcdPrePiCpuMemorySize -> PcdPrePiCpuMemoryBits
> > and
> > PcdPrePiCpuIoSize -> PcdPrePiCpuIoBits
> > ?
> >
> > Argument against this would be that later consumers still refer to
> > the value extracted from the HOB as SizeOf*Space, and happily shifts
> > 1s around by it :|
> >
> 
> I am reworking this so PcdPrePiCpuMemorySize retains its meaning.
> Instead, I will add something like this to ArmLib.h
> 
> //
> // ARM_MMU_IDMAP_LIMIT defines the maximum size of the identity mapping
> // that covers the entire address space when running in UEFI. This is limited
> // to what can architecturally be mapped using a 4 KB granule, even if the
> // hardware is capable of mapping more using larger pages.
> //
> #ifdef MDE_CPU_ARM
> #define ARM_MMU_IDMAP_LIMIT     (MAX_UINT32)
> #else
> #define ARM_MMU_IDMAP_LIMIT     (1ULL << 48)
> #endif
> 
> and use that in the MMU code as an upper bound in case the CPU supports 52 bits.

OK, that works for me.

And gets you off the hook for Pcd name bikesheding :)

/
    Leif


  reply	other threads:[~2018-11-26 17:57 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-23 12:14 [PATCH 0/5] ArmPkg, ArmVirtPkg: lift 40-bit IPA space limit Ard Biesheuvel
2018-11-23 12:14 ` [PATCH 1/5] ArmPkg/ArmLib: add support for reading the max physical address space size Ard Biesheuvel
     [not found]   ` <20181123131631.ionb53xqzlyepaue@hawk.localdomain>
2018-11-23 13:20     ` Ard Biesheuvel
2018-11-23 13:23       ` Ard Biesheuvel
2018-11-25 17:21       ` Laszlo Ersek
2018-11-26 11:46   ` Ard Biesheuvel
2018-11-26 18:17     ` Philippe Mathieu-Daudé
2018-11-23 12:14 ` [PATCH 2/5] ArmPkg/ArmMmuLib: take the CPU supported maximum PA space into account Ard Biesheuvel
2018-11-26  9:42   ` Laszlo Ersek
2018-11-26  9:46     ` Laszlo Ersek
2018-11-26 10:06     ` Laszlo Ersek
2018-11-26 11:43     ` Ard Biesheuvel
2018-11-26 17:45   ` Leif Lindholm
2018-11-26 17:50     ` Ard Biesheuvel
2018-11-26 17:57       ` Leif Lindholm [this message]
2018-11-23 12:14 ` [PATCH 3/5] ArmVirtPkg: refactor reading of the physical address space size Ard Biesheuvel
2018-11-26 10:00   ` Laszlo Ersek
2018-11-26 11:44     ` Ard Biesheuvel
2018-11-23 12:14 ` [PATCH 4/5] ArmVirtPkg: disregard PcdPrePiCpuMemorySize PCD when sizing the GCD space Ard Biesheuvel
2018-11-26 10:47   ` Laszlo Ersek
2018-11-26 11:59     ` Ard Biesheuvel
2018-11-23 12:14 ` [PATCH 5/5] ArmVirtPkg: revert PcdPrePiCpuMemorySize to is default value of 48 Ard Biesheuvel
     [not found]   ` <20181123133553.4o6rcbmebggn2ne7@hawk.localdomain>
2018-11-23 13:45     ` Ard Biesheuvel
2018-11-26 10:58   ` Laszlo Ersek
2018-11-25 17:23 ` [PATCH 0/5] ArmPkg, ArmVirtPkg: lift 40-bit IPA space limit Laszlo Ersek
2018-11-26  9:35 ` Laszlo Ersek
2018-11-26 10:22 ` Laszlo Ersek
2018-11-26 11:31   ` Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181126175754.mbf42gg4jedbdxej@bivouac.eciton.net \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox