Messages in this thread | | | Subject | Re: [PATCH v3 10/20] kvm: arm64: Dynamic configuration of VTTBR mask | From | Suzuki K Poulose <> | Date | Tue, 3 Jul 2018 12:54:26 +0100 |
| |
Hi Eric,
On 02/07/18 15:41, Auger Eric wrote: > Hi Suzuki, > > On 06/29/2018 01:15 PM, Suzuki K Poulose wrote: >> On arm64 VTTBR_EL2:BADDR holds the base address for the stage2 >> translation table. The Arm ARM mandates that the bits BADDR[x-1:0] >> should be 0, where 'x' is defined for a given IPA Size and the >> number of levels for a translation granule size. It is defined >> using some magical constants. This patch is a reverse engineered >> implementation to calculate the 'x' at runtime for a given ipa and >> number of page table levels. See patch for more details. >> >> Cc: Marc Zyngier <marc.zyngier@arm.com> >> Cc: Christoffer Dall <cdall@kernel.org> >> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> >> --- >> Changes since V2: >> - Part 1 of spilt from VTCR & VTTBR dynamic configuration >> --- >> arch/arm64/include/asm/kvm_arm.h | 60 +++++++++++++++++++++++++++++++++++++--- >> arch/arm64/include/asm/kvm_mmu.h | 25 ++++++++++++++++- >> 2 files changed, 80 insertions(+), 5 deletions(-) >> >> diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h >> index 3dffd38..c557f45 100644 >> --- a/arch/arm64/include/asm/kvm_arm.h >> +++ b/arch/arm64/include/asm/kvm_arm.h >> @@ -140,8 +140,6 @@ >> * Note that when using 4K pages, we concatenate two first level page tables >> * together. With 16K pages, we concatenate 16 first level page tables. >> * >> - * The magic numbers used for VTTBR_X in this patch can be found in Tables >> - * D4-23 and D4-25 in ARM DDI 0487A.b. > Isn't it a pretty old reference? Could you refer to C.a?
Sure, I will update the references everywhere.
>> + * >> + * The algorithm defines the expectations on the BaseAddress (for the page >> + * table) bits resolved at each level based on the page size, entry level >> + * and T0SZ. The variable "x" in the algorithm also affects the VTTBR:BADDR >> + * for stage2 page table. >> + * >> + * The value of "x" is calculated as : >> + * x = Magic_N - T0SZ >> + * >> + * where Magic_N is an integer depending on the page size and the entry >> + * level of the page table as below: >> + * >> + * -------------------------------------------- >> + * | Entry level | 4K 16K 64K | >> + * -------------------------------------------- >> + * | Level: 0 (4 levels) | 28 | - | - | >> + * -------------------------------------------- >> + * | Level: 1 (3 levels) | 37 | 31 | 25 | >> + * -------------------------------------------- >> + * | Level: 2 (2 levels) | 46 | 42 | 38 | >> + * -------------------------------------------- >> + * | Level: 3 (1 level) | - | 53 | 51 | >> + * -------------------------------------------- > I understand entry level = Lookup level in the table.
Entry level => The level at which we start the page table walk for a given address (This is in line with the ARM ARM). So,
Entry_level = (4 - Number_of_Page_table_levels)
> But you may want to compute x for BaseAddress matching lookup level 2 > with number of levels = 4.
No, the BaseAddress is only calcualted for the "Entry_level". So the above case doesn't exist at all.
> So shouldn't you s/Number of levels/4 - entry_level?
Ok, I now understood what you are referring to [0] > for BADDR we want the BaseAddr of the initial lookup level so > effectively the entry level we are interested in is 4 - number of levels > and we don't care or d) condition. At least this is my understanding ;-) > If correct you may slightly reword the explanation?
>> + * >> + * We have a magic formula for the Magic_N below. >> + * >> + * Magic_N(PAGE_SIZE, Entry_Level) = 64 - ((PAGE_SHIFT - 3) * Number of levels)
[0] ^^^
>> + * >> + * where number of levels = (4 - Entry_Level).
^^^ Doesn't this help make it clear ? Using the expansion makes it a bit more unreadable below.
>> >> +/* >> + * Get the magic number 'x' for VTTBR:BADDR of this KVM instance. >> + * With v8.2 LVA extensions, 'x' should be a minimum of 6 with >> + * 52bit IPS. > Link to the spec?
Sure, will add it.
Thanks for the patience to review :-)
Cheers Suzuki
| |