Re: [PATCH 0/3] KVM: arm64: Ask the compiler to __always_inline functions used by KVM at HYP
On Fri, Feb 21, 2020 at 12:55:16PM +, Marc Zyngier wrote: > Hi James, > > On 2020-02-20 16:58, James Morse wrote: > > Hello! > > > > It turns out KVM relies on the inline hint being honoured by the > > compiler > > in quite a few more places than expected. Something about the Shadow > > Call > > Stack support[0] causes the compiler to avoid inline-ing and to place > > these functions outside the __hyp_text. This ruins KVM's day. > > > > Add the simon-says __always_inline annotation to all the static > > inlines that KVM calls from HYP code. > > > > This series based on v5.6-rc2. > > Many thanks for going through all this. > > I'm happy to take it if Catalin or Will ack the arm64 patches. > It case we decide to go the other way around: > > Acked-by: Marc Zyngier > > One thing I'd like to look into though is a compile-time check that > nothing in the hyp_text section has a reference to a non-hyp_text > symbol. > > We already have checks around non-init symbols pointing to init symbols, > and I was wondering if we could reuse this for fun and profit... Hi Marc, I recall that you've suggested that before, and I even tried it around that time [*]. I wasn't happy enough with it to post a proper patch though. [*] https://lists.cs.columbia.edu/pipermail/kvmarm/2018-May/031629.html Thanks, drew > > Thanks, > > M. > -- > Jazz is not dead. It just smells funny... > ___ > kvmarm mailing list > kvmarm@lists.cs.columbia.edu > https://lists.cs.columbia.edu/mailman/listinfo/kvmarm > ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 0/3] KVM: arm64: Ask the compiler to __always_inline functions used by KVM at HYP
Hi James,
On 2020-02-21 14:57, James Morse wrote:
Hi Marc,
On 21/02/2020 12:55, Marc Zyngier wrote:
On 2020-02-20 16:58, James Morse wrote:
It turns out KVM relies on the inline hint being honoured by the
compiler
in quite a few more places than expected. Something about the Shadow
Call
Stack support[0] causes the compiler to avoid inline-ing and to place
these functions outside the __hyp_text. This ruins KVM's day.
Add the simon-says __always_inline annotation to all the static
inlines that KVM calls from HYP code.
This series based on v5.6-rc2.
Many thanks for going through all this.
I'm happy to take it if Catalin or Will ack the arm64 patches.
It case we decide to go the other way around:
Acked-by: Marc Zyngier
One thing I'd like to look into though is a compile-time check that
nothing in the hyp_text section has a reference to a non-hyp_text
symbol.
Heh, that hypothetical tool would choke on things like
arch/arm64/kvm/hyp/tlb.c:
| static void __hyp_text __tlb_switch_to_guest_vhe(...)
| {
[...]
| local_irq_save(cxt->flags);
which calls trace_hardirqs_off() ... which is absolutely fine because
this only happens on VHE.
Duh, indeed.
To do it purely with the section information, you'd need to separate
all the VHE code... (maybe as a debug option that only runs when VHE
is turned off?)
We may have to to that anyway at some point. If the "KVM compartment"
thing becomes real, we may have to end-up compiling both separately
(and jettison the one we don't need at runtime).
We already have checks around non-init symbols pointing to init
symbols,
and I was wondering if we could reuse this for fun and profit...
I think objtool is the tool-of-the-future that can do this. You need
something that believes everything behind has_vhe() is unreachable...
I need to educate myself about objtool. Seems to be the miracle cure
for a lot of ailments! ;-)
Anyway, I've now queued the series for 5.6.
M.
--
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 0/3] KVM: arm64: Ask the compiler to __always_inline functions used by KVM at HYP
Hi Marc,
On 21/02/2020 12:55, Marc Zyngier wrote:
> On 2020-02-20 16:58, James Morse wrote:
>> It turns out KVM relies on the inline hint being honoured by the compiler
>> in quite a few more places than expected. Something about the Shadow Call
>> Stack support[0] causes the compiler to avoid inline-ing and to place
>> these functions outside the __hyp_text. This ruins KVM's day.
>>
>> Add the simon-says __always_inline annotation to all the static
>> inlines that KVM calls from HYP code.
>>
>> This series based on v5.6-rc2.
>
> Many thanks for going through all this.
>
> I'm happy to take it if Catalin or Will ack the arm64 patches.
> It case we decide to go the other way around:
>
> Acked-by: Marc Zyngier
>
> One thing I'd like to look into though is a compile-time check that
> nothing in the hyp_text section has a reference to a non-hyp_text
> symbol.
Heh, that hypothetical tool would choke on things like arch/arm64/kvm/hyp/tlb.c:
| static void __hyp_text __tlb_switch_to_guest_vhe(...)
| {
[...]
| local_irq_save(cxt->flags);
which calls trace_hardirqs_off() ... which is absolutely fine because this only
happens on
VHE.
To do it purely with the section information, you'd need to separate all the
VHE code...
(maybe as a debug option that only runs when VHE is turned off?)
> We already have checks around non-init symbols pointing to init symbols,
> and I was wondering if we could reuse this for fun and profit...
I think objtool is the tool-of-the-future that can do this. You need something
that
believes everything behind has_vhe() is unreachable...
Thanks,
James
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 0/3] KVM: arm64: Ask the compiler to __always_inline functions used by KVM at HYP
Hi James, On 2020-02-20 16:58, James Morse wrote: Hello! It turns out KVM relies on the inline hint being honoured by the compiler in quite a few more places than expected. Something about the Shadow Call Stack support[0] causes the compiler to avoid inline-ing and to place these functions outside the __hyp_text. This ruins KVM's day. Add the simon-says __always_inline annotation to all the static inlines that KVM calls from HYP code. This series based on v5.6-rc2. Many thanks for going through all this. I'm happy to take it if Catalin or Will ack the arm64 patches. It case we decide to go the other way around: Acked-by: Marc Zyngier One thing I'd like to look into though is a compile-time check that nothing in the hyp_text section has a reference to a non-hyp_text symbol. We already have checks around non-init symbols pointing to init symbols, and I was wondering if we could reuse this for fun and profit... Thanks, M. -- Jazz is not dead. It just smells funny... ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 0/3] KVM: arm64: Ask the compiler to __always_inline functions used by KVM at HYP
On Thu, 20 Feb 2020 at 18:33, James Morse wrote: > > Hi Ard, > > On 20/02/2020 17:04, Ard Biesheuvel wrote: > > On Thu, 20 Feb 2020 at 17:58, James Morse wrote: > >> It turns out KVM relies on the inline hint being honoured by the compiler > >> in quite a few more places than expected. Something about the Shadow Call > >> Stack support[0] causes the compiler to avoid inline-ing and to place > >> these functions outside the __hyp_text. This ruins KVM's day. > >> > >> Add the simon-says __always_inline annotation to all the static > >> inlines that KVM calls from HYP code. > > > This isn't quite as yuck as I expected, fortunately, but it does beg > > the question whether we shouldn't simply map the entire kernel at EL2 > > instead? > > If the kernel is big enough to need internal veneers (the 128M range?), these > would > certainly go horribly wrong because its running somewhere other than the > relocation-time > address. We would need a way of telling the linker to keep the bits of KVM > close together... > Ah, of course, there is that as well ... ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 0/3] KVM: arm64: Ask the compiler to __always_inline functions used by KVM at HYP
Hi Ard, On 20/02/2020 17:04, Ard Biesheuvel wrote: > On Thu, 20 Feb 2020 at 17:58, James Morse wrote: >> It turns out KVM relies on the inline hint being honoured by the compiler >> in quite a few more places than expected. Something about the Shadow Call >> Stack support[0] causes the compiler to avoid inline-ing and to place >> these functions outside the __hyp_text. This ruins KVM's day. >> >> Add the simon-says __always_inline annotation to all the static >> inlines that KVM calls from HYP code. > This isn't quite as yuck as I expected, fortunately, but it does beg > the question whether we shouldn't simply map the entire kernel at EL2 > instead? If the kernel is big enough to need internal veneers (the 128M range?), these would certainly go horribly wrong because its running somewhere other than the relocation-time address. We would need a way of telling the linker to keep the bits of KVM close together... Thanks, James ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 0/3] KVM: arm64: Ask the compiler to __always_inline functions used by KVM at HYP
On Thu, 20 Feb 2020 at 17:58, James Morse wrote: > > Hello! > > It turns out KVM relies on the inline hint being honoured by the compiler > in quite a few more places than expected. Something about the Shadow Call > Stack support[0] causes the compiler to avoid inline-ing and to place > these functions outside the __hyp_text. This ruins KVM's day. > > Add the simon-says __always_inline annotation to all the static > inlines that KVM calls from HYP code. > > This series based on v5.6-rc2. > This isn't quite as yuck as I expected, fortunately, but it does beg the question whether we shouldn't simply map the entire kernel at EL2 instead? ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH 0/3] KVM: arm64: Ask the compiler to __always_inline functions used by KVM at HYP
Hello! It turns out KVM relies on the inline hint being honoured by the compiler in quite a few more places than expected. Something about the Shadow Call Stack support[0] causes the compiler to avoid inline-ing and to place these functions outside the __hyp_text. This ruins KVM's day. Add the simon-says __always_inline annotation to all the static inlines that KVM calls from HYP code. This series based on v5.6-rc2. [0] https://lore.kernel.org/linux-arm-kernel/20200219000817.195049-1-samitolva...@google.com/ Thanks, James Morse (3): KVM: arm64: Ask the compiler to __always_inline functions used at HYP KVM: arm64: define our own swab32() to avoid a uapi static inline arm64: Ask the compiler to __always_inline functions used by KVM at HYP arch/arm64/include/asm/arch_gicv3.h | 2 +- arch/arm64/include/asm/cache.h | 2 +- arch/arm64/include/asm/cacheflush.h | 2 +- arch/arm64/include/asm/cpufeature.h | 10 ++--- arch/arm64/include/asm/io.h | 4 +- arch/arm64/include/asm/kvm_emulate.h | 48 arch/arm64/include/asm/kvm_hyp.h | 7 arch/arm64/include/asm/kvm_mmu.h | 3 +- arch/arm64/include/asm/virt.h| 2 +- arch/arm64/kvm/hyp/vgic-v2-cpuif-proxy.c | 4 +- 10 files changed, 46 insertions(+), 38 deletions(-) -- 2.24.1 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
