| Message ID | CAKv+Gu-cPMdpcKvY0cR=vJOsE5B7CcqNMB=7xVHKo41FkLSVVw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Fri, 2015-10-02 at 14:18 +0200, Ard Biesheuvel wrote: > Is there any reasonable upper bound to the domU PA space > other than what is communicated in the ID registers? You mean the PASize bits/register? In which case that is it as far as the guest should be is concerned, yes. Ian.
On 2 October 2015 at 14:43, Ian Campbell <ian.campbell@citrix.com> wrote: > On Fri, 2015-10-02 at 14:18 +0200, Ard Biesheuvel wrote: >> Is there any reasonable upper bound to the domU PA space >> other than what is communicated in the ID registers? > > You mean the PASize bits/register? In which case that is it as far as the > guest should be is concerned, yes. > OK. As discussed on IRC (#xenarm), the rationale of this approach is that Xen's stage2 attributes will ultimately override device mappings, so 1:1 mapping the whole address space cacheable is actually a reasonable thing to do. And in fact, 4 KB of translation tables for each 512 GB of address space is probably not such a big deal after all. So I am inclined to leave things are they are if the proposed patch works.
On Fri, 2015-10-02 at 14:48 +0200, Ard Biesheuvel wrote: > On 2 October 2015 at 14:43, Ian Campbell <ian.campbell@citrix.com> wrote: > > On Fri, 2015-10-02 at 14:18 +0200, Ard Biesheuvel wrote: > > > Is there any reasonable upper bound to the domU PA space > > > other than what is communicated in the ID registers? > > > > You mean the PASize bits/register? In which case that is it as far as > > the > > guest should be is concerned, yes. > > > > OK. > > As discussed on IRC (#xenarm), the rationale of this approach is that > Xen's stage2 attributes will ultimately override device mappings, so > 1:1 mapping the whole address space cacheable is actually a reasonable > thing to do. And in fact, 4 KB of translation tables for each 512 GB > of address space is probably not such a big deal after all. Well, I'm not going to commit to that always being the case, since its not part of the guest ABI, but given that OVMF would normally be supplied as part of the host rather than the guest if we ever break/change that assumption we do at least have the opportunity to get ovmf fixed/updated around the same time. IOW I suppose this is tolerable. > So I am > inclined to leave things are they are if the proposed patch works. Sure. Ian.
On Fri, 2 Oct 2015, Ard Biesheuvel wrote: > diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c > b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c > index d98769b06b75..c84c872dbe60 100644 > --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c > +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c > @@ -303,8 +303,9 @@ GetBlockEntryListFromAddress ( > } > } > > - // Identify the Page Level the RegionStart must belongs to > - PageLevel = 3 - ((BaseAddressAlignment - 12) / 9); > + // Identify the Page Level the RegionStart must belong to. Note > that PageLevel > + // should be at least 1 since block translations are not supported at level 0 > + PageLevel = MAX (3 - ((BaseAddressAlignment - 12) / 9), 1); > > // If the required size is smaller than the current block size then > we need to go to the page below. > // The PageLevel was calculated on the Base Address alignment but > did not take in account the alignment It works. Tested-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c index d98769b06b75..c84c872dbe60 100644 --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c @@ -303,8 +303,9 @@ GetBlockEntryListFromAddress ( } } - // Identify the Page Level the RegionStart must belongs to - PageLevel = 3 - ((BaseAddressAlignment - 12) / 9); + // Identify the Page Level the RegionStart must belong to. Note that PageLevel + // should be at least 1 since block translations are not supported at level 0 + PageLevel = MAX (3 - ((BaseAddressAlignment - 12) / 9), 1); // If the required size is smaller than the current block size then