From: Anshuman Khandual <anshuman.khandual@arm.com>
To: Marc Zyngier <maz@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, suzuki.poulose@arm.com,
mark.rutland@arm.com, will@kernel.org, catalin.marinas@arm.com,
james.morse@arm.com, steven.price@arm.com
Subject: Re: [RFC V3 13/13] KVM: arm64: Enable FEAT_LPA2 based 52 bits IPA size on 4K and 16K
Date: Wed, 13 Oct 2021 08:58:44 +0530 [thread overview]
Message-ID: <2e48f9c3-020f-5769-aeaf-f704bb35a827@arm.com> (raw)
In-Reply-To: <87k0iiprvz.wl-maz@kernel.org>
On 10/12/21 2:00 PM, Marc Zyngier wrote:
> On Tue, 12 Oct 2021 05:24:15 +0100,
> Anshuman Khandual <anshuman.khandual@arm.com> wrote:
>>
>> Hello Marc,
>>
>> On 10/11/21 3:46 PM, Marc Zyngier wrote:
>>> On Thu, 30 Sep 2021 11:35:16 +0100,
>>> Anshuman Khandual <anshuman.khandual@arm.com> wrote:
>>>>
>>>> Stage-2 FEAT_LPA2 support is independent and also orthogonal to FEAT_LPA2
>>>> support either in Stage-1 or in the host kernel. Stage-2 IPA range support
>>>> is evaluated from the platform via ID_AA64MMFR0_TGRAN_2_SUPPORTED_LPA2 and
>>>> gets enabled regardless of Stage-1 translation.
>>>>
>>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>>> ---
>>>> arch/arm64/include/asm/kvm_pgtable.h | 10 +++++++++-
>>>> arch/arm64/kvm/hyp/pgtable.c | 25 +++++++++++++++++++++++--
>>>> arch/arm64/kvm/reset.c | 14 ++++++++++----
>>>> 3 files changed, 42 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
>>>> index 0277838..78a9d12 100644
>>>> --- a/arch/arm64/include/asm/kvm_pgtable.h
>>>> +++ b/arch/arm64/include/asm/kvm_pgtable.h
>>>> @@ -29,18 +29,26 @@ typedef u64 kvm_pte_t;
>>>>
>>>> #define KVM_PTE_ADDR_MASK GENMASK(47, PAGE_SHIFT)
>>>> #define KVM_PTE_ADDR_51_48 GENMASK(15, 12)
>>>> +#define KVM_PTE_ADDR_51_50 GENMASK(9, 8)
>>>>
>>>> static inline bool kvm_pte_valid(kvm_pte_t pte)
>>>> {
>>>> return pte & KVM_PTE_VALID;
>>>> }
>>>>
>>>> +void set_kvm_lpa2_enabled(void);
>>>> +bool get_kvm_lpa2_enabled(void);
>>>> +
>>>> static inline u64 kvm_pte_to_phys(kvm_pte_t pte)
>>>> {
>>>> u64 pa = pte & KVM_PTE_ADDR_MASK;
>>>>
>>>> - if (PAGE_SHIFT == 16)
>>>> + if (PAGE_SHIFT == 16) {
>>>> pa |= FIELD_GET(KVM_PTE_ADDR_51_48, pte) << 48;
>>>> + } else {
>>>> + if (get_kvm_lpa2_enabled())
>>>
>>> Having to do a function call just for this test seems bad, specially
>>> for something that is used so often on the fault path.
>>>
>>> Why can't this be made a normal capability that indicates LPA support
>>> for the current page size?
>>
>> Although I could look into making this a normal capability check, would
>> not a static key based implementation be preferred if the function call
>> based construct here is too expensive ?
>
> A capability *is* a static key. Specially if you make it final.
Sure.
>
>> Originally, avoided capability method for stage-2 because it would have
>> been difficult in stage-1 where the FEAT_LPA2 detection is required way
>> earlier during boot before cpu capability comes up. Hence just followed
>> a simple variable method both for stage-1 and stage-2 keeping it same.
>
> I think you'll have to find a way to make it work with a capability
> for S1 too. Capabilities can be used even when not final, and you may
> have to do something similar.
Sure, will explore that.
>
>>>
>>>> + pa |= FIELD_GET(KVM_PTE_ADDR_51_50, pte) << 50;
>>>
>>> Where are bits 48 and 49?
>>
>> Unlike the current FEAT_LPA feature, bits 48 and 49 are part of the PA
>> itself. Only the bits 50 and 51 move into bits 8 and 9, while creating
>> a PTE.
>
> So why are you actively dropping these bits? Hint: look at
> KVM_PTE_ADDR_MASK and the way it is used to extract the initial value
> of 'pa'.
Right, will need another address mask i.e KVM_PTE_ADDR_MASK_50 which will
extract the PA field both in kvm_pte_to_phys() and kvm_phys_to_pte().
>
> [...]
>
>>> Another thing I don't see is how you manage TLB invalidation by level
>>> now that we gain a level 0 at 4kB, breaking the current assumptions
>>> encoded in __tlbi_level().
>>
>> Right, I guess something like this (not build tested) will be required as
>> level 0 for 4K and level 1 for 16K would only make sense when FEAT_LPA2 is
>> implemented, otherwise it will fallback to the default behaviour i.e table
>> level hint was not provided (TTL[3:2] is 0b00). Is there any other concern
>> which I might be missing here ?
>>
>> --- a/arch/arm64/include/asm/tlbflush.h
>> +++ b/arch/arm64/include/asm/tlbflush.h
>> @@ -104,8 +104,7 @@ static inline unsigned long get_trans_granule(void)
>> #define __tlbi_level(op, addr, level) do { \
>> u64 arg = addr; \
>> \
>> - if (cpus_have_const_cap(ARM64_HAS_ARMv8_4_TTL) && \
>> - level) { \
>> + if (cpus_have_const_cap(ARM64_HAS_ARMv8_4_TTL) { \
>> u64 ttl = level & 3; \
>> ttl |= get_trans_granule() << 2; \
>> arg &= ~TLBI_TTL_MASK; \
>>
>
> That's a start, but 0 has always meant 'at any level' until now. You
> will have to audit all the call sites and work out whether they can
> pass 0 if they don't track the actual level.
Hmm, sure will audit the call sites. But wondering if the caller is not
sure about the level, should not it just use __tlbi(op, arg) instead ?
Seems like __tlbi_level(op, addr, level) should only accept level as 0
when the level is determined to be 0. The actual impact will depend on
if FEAT_LPA2 is implemented otherwise it falls back to __tlbi(op, arg).
Basically we should convert existing sites which call __tlbi_level()
with level as 0 (without determining), to use __tlbi() directly ?
prev parent reply other threads:[~2021-10-13 3:28 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-30 10:35 [RFC V3 00/13] arm64/mm: Enable FEAT_LPA2 (52 bits PA support on 4K|16K pages) Anshuman Khandual
2021-09-30 10:35 ` [RFC V3 01/13] arm64/mm: Dynamically initialize protection_map[] Anshuman Khandual
2021-09-30 10:35 ` [RFC V3 02/13] arm64/mm: Consolidate TCR_EL1 fields Anshuman Khandual
2021-09-30 10:35 ` [RFC V3 03/13] arm64/mm: Add FEAT_LPA2 specific TCR_EL1.DS field Anshuman Khandual
2021-09-30 10:35 ` [RFC V3 04/13] arm64/mm: Add FEAT_LPA2 specific VTCR_EL2.DS field Anshuman Khandual
2021-09-30 10:35 ` [RFC V3 05/13] arm64/mm: Add FEAT_LPA2 specific ID_AA64MMFR0.TGRAN[2] Anshuman Khandual
2021-09-30 10:35 ` [RFC V3 06/13] arm64/mm: Add CONFIG_ARM64_PA_BITS_52_[LPA|LPA2] Anshuman Khandual
2021-09-30 10:35 ` [RFC V3 07/13] arm64/mm: Add FEAT_LPA2 specific encoding Anshuman Khandual
2021-10-12 10:41 ` Suzuki K Poulose
2021-10-13 2:55 ` Anshuman Khandual
2021-09-30 10:35 ` [RFC V3 08/13] arm64/mm: Detect and enable FEAT_LPA2 Anshuman Khandual
2021-09-30 10:35 ` [RFC V3 09/13] arm64/mm: Add __cpu_secondary_check52bitpa() Anshuman Khandual
2021-09-30 10:35 ` [RFC V3 10/13] arm64/mm: Add FEAT_LPA2 specific PTE_SHARED and PMD_SECT_S Anshuman Khandual
2021-09-30 10:35 ` [RFC V3 11/13] arm64/mm: Add FEAT_LPA2 specific fallback (48 bits PA) when not implemented Anshuman Khandual
2021-09-30 10:35 ` [RFC V3 12/13] arm64/mm: Enable CONFIG_ARM64_PA_BITS_52 on CONFIG_ARM64_[4K|16K]_PAGES Anshuman Khandual
2021-09-30 10:35 ` [RFC V3 13/13] KVM: arm64: Enable FEAT_LPA2 based 52 bits IPA size on 4K and 16K Anshuman Khandual
2021-10-11 10:16 ` Marc Zyngier
2021-10-12 4:24 ` Anshuman Khandual
2021-10-12 8:30 ` Marc Zyngier
2021-10-13 3:28 ` Anshuman Khandual [this message]
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-all 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=2e48f9c3-020f-5769-aeaf-f704bb35a827@arm.com \
--to=anshuman.khandual@arm.com \
--cc=catalin.marinas@arm.com \
--cc=james.morse@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=maz@kernel.org \
--cc=steven.price@arm.com \
--cc=suzuki.poulose@arm.com \
--cc=will@kernel.org \
/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;
as well as URLs for NNTP newsgroup(s).