From: Punit Agrawal <punit.agrawal@arm.com>
To: Will Deacon <will.deacon@arm.com>
Cc: kvm@vger.kernel.org, Marc Zyngier <marc.zyngier@arm.com>,
linux-kernel@vger.kernel.org,
Steven Rostedt <rostedt@goodmis.org>,
Ingo Molnar <mingo@redhat.com>,
kvmarm@lists.cs.columbia.edu,
linux-arm-kernel@lists.infradead.org
Subject: Re: [RFC PATCH 6/7] arm64: KVM: Handle trappable TLB instructions
Date: Wed, 24 Aug 2016 11:40:39 +0100 [thread overview]
Message-ID: <87a8g2sb4o.fsf@e105922-lin.cambridge.arm.com> (raw)
In-Reply-To: <20160819151846.GE9893@arm.com> (Will Deacon's message of "Fri, 19 Aug 2016 16:18:47 +0100")
Will Deacon <will.deacon@arm.com> writes:
> Hi Punit,
>
> On Tue, Aug 16, 2016 at 11:45:11AM +0100, Punit Agrawal wrote:
>> The ARMv8 architecture allows trapping of TLB maintenane instructions
>> from EL0/EL1 to higher exception levels. On encountering a trappable TLB
>> instruction in a guest, an exception is taken to EL2.
>>
>> Add functionality to handle emulating the TLB instructions.
>>
>> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
>> Cc: Christoffer Dall <christoffer.dall@linaro.org>
>> Cc: Marc Zyngier <marc.zyngier@arm.com>
>
> [...]
>
>> +void __hyp_text
>> +__kvm_emulate_tlb_invalidate(struct kvm *kvm, u32 sys_op, u64 regval)
>> +{
>> + kvm = kern_hyp_va(kvm);
>> +
>> + /*
>> + * Switch to the guest before performing any TLB operations to
>> + * target the appropriate VMID
>> + */
>> + __switch_to_guest_regime(kvm);
>> +
>> + /*
>> + * TLB maintenance operations broadcast to inner-shareable
>> + * domain when HCR_FB is set (default for KVM).
>> + */
>> + switch (sys_op) {
>> + case TLBIALL:
>> + case TLBIALLIS:
>> + case ITLBIALL:
>> + case DTLBIALL:
>> + case TLBI_VMALLE1:
>> + case TLBI_VMALLE1IS:
>> + __tlbi(vmalle1is);
>> + break;
>> + case TLBIMVA:
>> + case TLBIMVAIS:
>> + case ITLBIMVA:
>> + case DTLBIMVA:
>> + case TLBI_VAE1:
>> + case TLBI_VAE1IS:
>> + __tlbi(vae1is, regval);
>
> I'm pretty nervous about this. Although you've switched in the guest stage-2
> page table before the TLB maintenance, we're still running on a host stage-1
> and it's not clear to me that the stage-1 context is completely ignored for
> the purposes of a stage-1 TLBI executed at EL2.
>
> For example, if TCR_EL1.TBI0 is set in the guest but cleared in the host,
> my reading of the architecture is that it will be treated as zero when
> we perform this invalidation operation. I worry that we have similar
> problems with the granule size, where bits become RES0 in the TLBI VA
> ops.
Some control bits seem to be explicitly called out to not affect TLB
maintenance operations[0] but I hadn't considered the ones you highlight.
[0] ARMv8 ARM DDI 0487A.j D4.7, Pg D4-1814
>
> Finally, we should probably be masking out the RES0 bits in the TLBI
> ops, just in case some future extension to the architecture defines them
> in such a way where they have different meanings when executed at EL2
> or EL1.
Although, the RES0 bits for TLBI VA ops are currently ignored, I agree
that masking them out based on granule size protects against future
incompatible changes.
>
> The easiest thing to do is just TLBI VMALLE1IS for all trapped operations,
> but you might want to see how that performs.
That sounds reasonable for correctness. But I suspect we'll have to do
more to claw back some performance. Let me run a few tests and come back
on this.
Thanks for having a look.
Punit
>
> Will
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
next prev parent reply other threads:[~2016-08-24 10:41 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-16 10:45 [RFC PATCH 0/7] Add support for monitoring guest TLB operations Punit Agrawal
2016-08-16 10:45 ` [RFC PATCH 1/7] perf/trace: Add notification for perf trace events Punit Agrawal
2016-08-31 11:01 ` Punit Agrawal
2016-08-16 10:45 ` [RFC PATCH 2/7] KVM: Track the pid of the VM process Punit Agrawal
2016-08-16 10:45 ` [RFC PATCH 3/7] KVM: arm/arm64: Register perf trace event notifier Punit Agrawal
2016-08-16 10:45 ` [RFC PATCH 4/7] arm64: tlbflush.h: add __tlbi() macro Punit Agrawal
2016-08-19 13:24 ` Will Deacon
2016-08-19 13:34 ` Punit Agrawal
2016-08-16 10:45 ` [RFC PATCH 5/7] arm64/kvm: hyp: tlb: use __tlbi() helper Punit Agrawal
2016-08-16 10:45 ` [RFC PATCH 6/7] arm64: KVM: Handle trappable TLB instructions Punit Agrawal
2016-08-19 15:18 ` Will Deacon
2016-08-24 10:40 ` Punit Agrawal [this message]
2016-08-26 9:37 ` Punit Agrawal
2016-08-26 12:21 ` Marc Zyngier
2016-09-01 14:55 ` Will Deacon
2016-09-01 18:29 ` Punit Agrawal
2016-08-16 10:45 ` [RFC PATCH 7/7] arm64: KVM: Enable selective trapping of " Punit Agrawal
2016-08-17 15:58 ` [RFC PATCH 0/7] Add support for monitoring guest TLB operations Paolo Bonzini
2016-08-17 17:01 ` Punit Agrawal
2016-08-17 17:02 ` Paolo Bonzini
2016-08-17 17:20 ` Punit Agrawal
2016-08-18 7:04 ` Paolo Bonzini
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=87a8g2sb4o.fsf@e105922-lin.cambridge.arm.com \
--to=punit.agrawal@arm.com \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marc.zyngier@arm.com \
--cc=mingo@redhat.com \
--cc=rostedt@goodmis.org \
--cc=will.deacon@arm.com \
/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).