From: Marc Zyngier <marc.zyngier@arm.com> To: Christoffer Dall <cdall@linaro.org> Cc: Russell King <linux@arm.linux.org.uk>, kvm@vger.kernel.org, Ard Biesheuvel <ard.biesheuvel@linaro.org>, Catalin Marinas <catalin.marinas@arm.com>, linux-arm-kernel@lists.infradead.org, Keerthy <j-keerthy@ti.com>, kvmarm@lists.cs.columbia.edu Subject: Re: [PATCH v4 00/28] arm/arm64: KVM: Rework the hyp-stub API Date: Thu, 23 Mar 2017 15:16:49 +0000 [thread overview] Message-ID: <e424e00c-e649-8e6a-7f73-ffbd37046779@arm.com> (raw) In-Reply-To: <20170323143916.GA27715@cbox> On 23/03/17 14:39, Christoffer Dall wrote: > On Thu, Mar 23, 2017 at 10:53:04AM +0000, Marc Zyngier wrote: >> On 22/03/17 17:27, Christoffer Dall wrote: >>> >>> I don't think there is a great use case beyond what we already do, it >>> would just be to have one set of hyp vectors fewer, so that either the >>> hyp stub is in place, or a hypervisor is in place, not kvm-init vectors. >>> >>> So instead of doing: >>> __hyp_set_vectors(kvm_get_idmap_vector()); >>> hvc(); /* via the misused kvm_call_hyp thing */ >>> >>> you would do: >>> >>> __hyp_call_function(__kvm_hyp_init, arg1, arg2); >>> >>> which would change the vector and initialize anything. >>> >>> Not sure if that can work though, or if there are any downsides that I >>> haven't thought about? >> >> I've given it a go, and that seems to work, at least on arm64. We may >> have to set the vectors in a slightly different way on 32bit because >> we're going to run out of registers (we only have two left once we >> reach the function being called). > > Right, that's a challenge I clearly didn't foresee. > >> >> Another thing is that the function called is not really a function. It >> is an exception handler, as it has to end with an eret (or we may need >> to save LR in funky ways). > > Shouldn't it have the same semantics as the KVM calling function thing > where it pushes the LR on the stack? But of course, that requires > having a stack for each runtime that owns hyp mode at any given time. > Potentially not great. Indeed, and that's the point where I'm starting to think that we may be trying to beautify something that may not be worth it. > If possible, the idea would be that you'd call functions, just like you > normally do, but the functions you call can choose to never return - > again just like a C function can do if it messes with your machine > configuration. > > But maybe the idea is fundamentally flawed, if the only function > you'd ever call from the hyp stub is one that takes over the hyp world, > and then the original design was just based on using set_vectors. Hmmm. That's my point above. The only use case we have so far for this method is to take over EL2. On the other have, I've noticed that the more I rework this area, the more old stuff surfaces, ready to be nuked. Maybe I should keep digging. > >> The potential blocker for this is that the >> 32bit decompressor does use set_vectors in funky ways to deal with >> relocation. If we get rid of set_vectors like I just did on 64bit, >> we'll need to mess with that as well. > > Interesting. I didn't think that we'd necessarily get rid of > set_vectors, but I agree with you that if we wanted to do this, it > should probably go. > >> >> Anyway, here's the hack, 64bit only, quickly tested on Mustang. I'm not >> completely sure this is any prettier, but it is certainly manageable. > > There are two things I like about it. First, it gets rid of the > 'additional runtime' in hyp and second the changes to > cpu_init_hyp_mode() are quite nice, imho. > > Thanks being said, this series is pretty nice as it is, so I don't want > to impose more work on you at the moment, so maybe we save your patch > snipped below for later if we want to consider looking at it some other > time? What I can do is prepare an extra series that would include the corresponding 32bit changes, and we then evaluate that separately. I don't mind spending a bit of time on it. Thanks, M. -- Jazz is not dead. It just smells funny...
WARNING: multiple messages have this Message-ID (diff)
From: marc.zyngier@arm.com (Marc Zyngier) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v4 00/28] arm/arm64: KVM: Rework the hyp-stub API Date: Thu, 23 Mar 2017 15:16:49 +0000 [thread overview] Message-ID: <e424e00c-e649-8e6a-7f73-ffbd37046779@arm.com> (raw) In-Reply-To: <20170323143916.GA27715@cbox> On 23/03/17 14:39, Christoffer Dall wrote: > On Thu, Mar 23, 2017 at 10:53:04AM +0000, Marc Zyngier wrote: >> On 22/03/17 17:27, Christoffer Dall wrote: >>> >>> I don't think there is a great use case beyond what we already do, it >>> would just be to have one set of hyp vectors fewer, so that either the >>> hyp stub is in place, or a hypervisor is in place, not kvm-init vectors. >>> >>> So instead of doing: >>> __hyp_set_vectors(kvm_get_idmap_vector()); >>> hvc(); /* via the misused kvm_call_hyp thing */ >>> >>> you would do: >>> >>> __hyp_call_function(__kvm_hyp_init, arg1, arg2); >>> >>> which would change the vector and initialize anything. >>> >>> Not sure if that can work though, or if there are any downsides that I >>> haven't thought about? >> >> I've given it a go, and that seems to work, at least on arm64. We may >> have to set the vectors in a slightly different way on 32bit because >> we're going to run out of registers (we only have two left once we >> reach the function being called). > > Right, that's a challenge I clearly didn't foresee. > >> >> Another thing is that the function called is not really a function. It >> is an exception handler, as it has to end with an eret (or we may need >> to save LR in funky ways). > > Shouldn't it have the same semantics as the KVM calling function thing > where it pushes the LR on the stack? But of course, that requires > having a stack for each runtime that owns hyp mode at any given time. > Potentially not great. Indeed, and that's the point where I'm starting to think that we may be trying to beautify something that may not be worth it. > If possible, the idea would be that you'd call functions, just like you > normally do, but the functions you call can choose to never return - > again just like a C function can do if it messes with your machine > configuration. > > But maybe the idea is fundamentally flawed, if the only function > you'd ever call from the hyp stub is one that takes over the hyp world, > and then the original design was just based on using set_vectors. Hmmm. That's my point above. The only use case we have so far for this method is to take over EL2. On the other have, I've noticed that the more I rework this area, the more old stuff surfaces, ready to be nuked. Maybe I should keep digging. > >> The potential blocker for this is that the >> 32bit decompressor does use set_vectors in funky ways to deal with >> relocation. If we get rid of set_vectors like I just did on 64bit, >> we'll need to mess with that as well. > > Interesting. I didn't think that we'd necessarily get rid of > set_vectors, but I agree with you that if we wanted to do this, it > should probably go. > >> >> Anyway, here's the hack, 64bit only, quickly tested on Mustang. I'm not >> completely sure this is any prettier, but it is certainly manageable. > > There are two things I like about it. First, it gets rid of the > 'additional runtime' in hyp and second the changes to > cpu_init_hyp_mode() are quite nice, imho. > > Thanks being said, this series is pretty nice as it is, so I don't want > to impose more work on you at the moment, so maybe we save your patch > snipped below for later if we want to consider looking at it some other > time? What I can do is prepare an extra series that would include the corresponding 32bit changes, and we then evaluate that separately. I don't mind spending a bit of time on it. Thanks, M. -- Jazz is not dead. It just smells funny...
next prev parent reply other threads:[~2017-03-23 15:16 UTC|newest] Thread overview: 112+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-03-21 19:20 [PATCH v4 00/28] arm/arm64: KVM: Rework the hyp-stub API Marc Zyngier 2017-03-21 19:20 ` Marc Zyngier 2017-03-21 19:20 ` [PATCH v4 01/28] arm64: hyp-stub: Stop pointlessly clobbering lr Marc Zyngier 2017-03-21 19:20 ` Marc Zyngier 2017-03-21 19:20 ` [PATCH v4 02/28] arm64: KVM: Move lr save/restore to do_el2_call Marc Zyngier 2017-03-21 19:20 ` Marc Zyngier 2017-03-23 11:57 ` Marc Zyngier 2017-03-23 11:57 ` Marc Zyngier 2017-03-21 19:20 ` [PATCH v4 03/28] arm64: hyp-stub: Don't save lr in the EL1 code Marc Zyngier 2017-03-21 19:20 ` Marc Zyngier 2017-03-21 19:20 ` [PATCH v4 04/28] arm64: hyp-stub: Implement HVC_RESET_VECTORS stub hypercall Marc Zyngier 2017-03-21 19:20 ` Marc Zyngier 2017-03-21 19:20 ` [PATCH v4 05/28] arm64: KVM: Implement HVC_RESET_VECTORS stub hypercall in the init code Marc Zyngier 2017-03-21 19:20 ` Marc Zyngier 2017-03-24 14:33 ` Christoffer Dall 2017-03-24 14:33 ` Christoffer Dall 2017-03-24 14:45 ` Marc Zyngier 2017-03-24 14:45 ` Marc Zyngier 2017-03-21 19:20 ` [PATCH v4 06/28] arm64: KVM: Implement HVC_GET_VECTORS " Marc Zyngier 2017-03-21 19:20 ` Marc Zyngier 2017-03-21 19:20 ` [PATCH v4 07/28] arm64: KVM: Allow the main HYP code to use the init hyp stub implementation Marc Zyngier 2017-03-21 19:20 ` Marc Zyngier 2017-03-24 14:33 ` Christoffer Dall 2017-03-24 14:33 ` Christoffer Dall 2017-03-24 14:56 ` Marc Zyngier 2017-03-24 14:56 ` Marc Zyngier 2017-04-03 17:28 ` Christoffer Dall 2017-04-03 17:28 ` Christoffer Dall 2017-03-21 19:20 ` [PATCH v4 08/28] arm64: KVM: Convert __cpu_reset_hyp_mode to using __hyp_reset_vectors Marc Zyngier 2017-03-21 19:20 ` Marc Zyngier 2017-03-21 19:20 ` [PATCH v4 09/28] arm64: KVM: Implement HVC_SOFT_RESTART in the init code Marc Zyngier 2017-03-21 19:20 ` Marc Zyngier 2017-03-21 19:20 ` [PATCH v4 10/28] ARM: hyp-stub: improve ABI Marc Zyngier 2017-03-21 19:20 ` Marc Zyngier 2017-03-21 19:20 ` [PATCH v4 11/28] ARM: soft-reboot into same mode that we entered the kernel Marc Zyngier 2017-03-21 19:20 ` Marc Zyngier 2017-03-21 19:20 ` [PATCH v4 12/28] ARM: KVM: Convert KVM to use HVC_GET_VECTORS Marc Zyngier 2017-03-21 19:20 ` Marc Zyngier 2017-03-21 19:20 ` [PATCH v4 13/28] ARM: Update cpu_v7_reset documentation Marc Zyngier 2017-03-21 19:20 ` Marc Zyngier 2017-03-21 19:20 ` [PATCH v4 14/28] ARM: hyp-stub: Use r1 for the soft-restart address Marc Zyngier 2017-03-21 19:20 ` Marc Zyngier 2017-03-21 19:20 ` [PATCH v4 15/28] ARM: Expose the VA/IDMAP offset Marc Zyngier 2017-03-21 19:20 ` Marc Zyngier 2017-03-21 19:20 ` [PATCH v4 16/28] ARM: hyp-stub: Implement HVC_RESET_VECTORS stub hypercall Marc Zyngier 2017-03-21 19:20 ` Marc Zyngier 2017-03-21 19:20 ` [PATCH v4 17/28] ARM: KVM: Implement HVC_RESET_VECTORS stub hypercall in the init code Marc Zyngier 2017-03-21 19:20 ` Marc Zyngier 2017-03-21 19:20 ` [PATCH v4 18/28] ARM: KVM: Implement HVC_GET_VECTORS " Marc Zyngier 2017-03-21 19:20 ` Marc Zyngier 2017-03-21 19:20 ` [PATCH v4 19/28] ARM: KVM: Allow the main HYP code to use the init hyp stub implementation Marc Zyngier 2017-03-21 19:20 ` Marc Zyngier 2017-03-24 14:34 ` Christoffer Dall 2017-03-24 14:34 ` Christoffer Dall 2017-03-24 15:01 ` Marc Zyngier 2017-03-24 15:01 ` Marc Zyngier 2017-04-03 17:32 ` Christoffer Dall 2017-04-03 17:32 ` Christoffer Dall 2017-04-03 17:51 ` Marc Zyngier 2017-04-03 17:51 ` Marc Zyngier 2017-04-04 7:36 ` Christoffer Dall 2017-04-04 7:36 ` Christoffer Dall 2017-03-21 19:20 ` [PATCH v4 20/28] ARM: KVM: Convert __cpu_reset_hyp_mode to using __hyp_reset_vectors Marc Zyngier 2017-03-21 19:20 ` Marc Zyngier 2017-03-21 19:20 ` [PATCH v4 21/28] ARM: KVM: Implement HVC_SOFT_RESTART in the init code Marc Zyngier 2017-03-21 19:20 ` Marc Zyngier 2017-03-21 19:20 ` [PATCH v4 22/28] arm/arm64: KVM: Use __hyp_reset_vectors() directly Marc Zyngier 2017-03-21 19:20 ` Marc Zyngier 2017-03-21 19:20 ` [PATCH v4 23/28] arm/arm64: KVM: Remove kvm_get_idmap_start Marc Zyngier 2017-03-21 19:20 ` Marc Zyngier 2017-03-21 19:20 ` [PATCH v4 24/28] arm/arm64: KVM: Use HVC_RESET_VECTORS to reinit HYP mode Marc Zyngier 2017-03-21 19:20 ` Marc Zyngier 2017-03-21 19:20 ` [PATCH v4 25/28] ARM: decompressor: Remove __hyp_get_vectors usage Marc Zyngier 2017-03-21 19:20 ` Marc Zyngier 2017-03-24 14:34 ` Christoffer Dall 2017-03-24 14:34 ` Christoffer Dall 2017-03-24 15:26 ` Marc Zyngier 2017-03-24 15:26 ` Marc Zyngier 2017-03-21 19:20 ` [PATCH v4 26/28] ARM: hyp-stub/KVM: Kill __hyp_get_vectors Marc Zyngier 2017-03-21 19:20 ` Marc Zyngier 2017-03-21 19:20 ` [PATCH v4 27/28] arm64: " Marc Zyngier 2017-03-21 19:20 ` Marc Zyngier 2017-03-21 19:20 ` [PATCH v4 28/28] arm/arm64: Add hyp-stub API documentation Marc Zyngier 2017-03-21 19:20 ` Marc Zyngier 2017-03-24 14:33 ` Christoffer Dall 2017-03-24 14:33 ` Christoffer Dall 2017-03-24 14:42 ` Marc Zyngier 2017-03-24 14:42 ` Marc Zyngier 2017-03-24 15:23 ` Christoffer Dall 2017-03-24 15:23 ` Christoffer Dall 2017-03-24 15:57 ` Marc Zyngier 2017-03-24 15:57 ` Marc Zyngier 2017-03-24 16:03 ` Christoffer Dall 2017-03-24 16:03 ` Christoffer Dall 2017-03-22 13:37 ` [PATCH v4 00/28] arm/arm64: KVM: Rework the hyp-stub API Christoffer Dall 2017-03-22 13:37 ` Christoffer Dall 2017-03-22 16:14 ` Marc Zyngier 2017-03-22 16:14 ` Marc Zyngier 2017-03-22 17:27 ` Christoffer Dall 2017-03-22 17:27 ` Christoffer Dall 2017-03-23 10:53 ` Marc Zyngier 2017-03-23 10:53 ` Marc Zyngier 2017-03-23 14:39 ` Christoffer Dall 2017-03-23 14:39 ` Christoffer Dall 2017-03-23 15:16 ` Marc Zyngier [this message] 2017-03-23 15:16 ` Marc Zyngier 2017-03-23 15:45 ` Christoffer Dall 2017-03-23 15:45 ` Christoffer Dall 2017-03-22 16:20 ` Catalin Marinas 2017-03-22 16:20 ` Catalin Marinas 2017-03-24 14:36 ` Christoffer Dall 2017-03-24 14:36 ` Christoffer Dall
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=e424e00c-e649-8e6a-7f73-ffbd37046779@arm.com \ --to=marc.zyngier@arm.com \ --cc=ard.biesheuvel@linaro.org \ --cc=catalin.marinas@arm.com \ --cc=cdall@linaro.org \ --cc=j-keerthy@ti.com \ --cc=kvm@vger.kernel.org \ --cc=kvmarm@lists.cs.columbia.edu \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux@arm.linux.org.uk \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.