* [Qemu-devel] [RFC] Add virtual SDEI support in qemu @ 2019-07-13 9:53 Guoheyi 2019-07-15 13:41 ` Dave Martin 0 siblings, 1 reply; 9+ messages in thread From: Guoheyi @ 2019-07-13 9:53 UTC (permalink / raw) To: qemu-arm, qemu-devel, linux-arm-kernel, kvmarm Cc: Marc Zyngier, Peter Maydell, James.Morse, wanghaibin 00208455 Hi folks, Do it make sense to implement virtual SDEI in qemu? So that we can have the standard way for guest to handle NMI watchdog, RAS events and something else which involves SDEI in a physical ARM64 machine. My basic idea is like below: 1. Change a few lines of code in kvm to allow unhandled SMC invocations (like SDEI) to be sent to qemu, with exit reason of KVM_EXIT_HYPERCALL, so we don't need to add new API. 2. qemu handles supported SDEI calls just as the spec says for what a hypervisor should do for a guest OS. 3. For interrupts bound to hypervisor, qemu should stop injecting the IRQ to guest through KVM, but jump to the registered event handler directly, including context saving and restoring. Some interrupts like virtual timer are handled by kvm directly, so we may refuse to bind such interrupts to SDEI events. Any comment or suggestion will be really appreciated. Heyi ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [RFC] Add virtual SDEI support in qemu 2019-07-13 9:53 [Qemu-devel] [RFC] Add virtual SDEI support in qemu Guoheyi @ 2019-07-15 13:41 ` Dave Martin 2019-07-15 13:48 ` Mark Rutland 0 siblings, 1 reply; 9+ messages in thread From: Dave Martin @ 2019-07-15 13:41 UTC (permalink / raw) To: Guoheyi; +Cc: Marc Zyngier, qemu-arm, qemu-devel, linux-arm-kernel, kvmarm On Sat, Jul 13, 2019 at 05:53:57PM +0800, Guoheyi wrote: > Hi folks, > > Do it make sense to implement virtual SDEI in qemu? So that we can have the > standard way for guest to handle NMI watchdog, RAS events and something else > which involves SDEI in a physical ARM64 machine. > > My basic idea is like below: > > 1. Change a few lines of code in kvm to allow unhandled SMC invocations > (like SDEI) to be sent to qemu, with exit reason of KVM_EXIT_HYPERCALL, so > we don't need to add new API. So long as KVM_EXIT_HYPERCALL reports sufficient information so that userspace can identify the cause as an SMC and retrieve the SMC immediate field, this seems feasible. For its own SMCCC APIs, KVM exclusively uses HVC, so rerouting SMC to userspace shouldn't conflict. This bouncing of SMCs to userspace would need to be opt-in, otherwise old userspace would see exits that it doesn't know what to do with. > 2. qemu handles supported SDEI calls just as the spec says for what a > hypervisor should do for a guest OS. > > 3. For interrupts bound to hypervisor, qemu should stop injecting the IRQ to > guest through KVM, but jump to the registered event handler directly, > including context saving and restoring. Some interrupts like virtual timer > are handled by kvm directly, so we may refuse to bind such interrupts to > SDEI events. Something like that. Interactions between SDEI and PSCI would need some thought: for example, after PSCI_CPU_ON, the newly online cpu needs to have SDEs masked. One option (suggested to me by James Morse) would be to allow userspace to disable in the in-kernel PSCI implementation and provide its own PSCI to the guest via SMC -- in which case userspace that wants to implement SDEI would have to implement PSCI as well. There may be reasons why this wouldn't work ... I haven't thought about it in depth. Cheers ---Dave ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [RFC] Add virtual SDEI support in qemu 2019-07-15 13:41 ` Dave Martin @ 2019-07-15 13:48 ` Mark Rutland 2019-07-15 14:26 ` James Morse 2019-07-16 8:30 ` Dave Martin 0 siblings, 2 replies; 9+ messages in thread From: Mark Rutland @ 2019-07-15 13:48 UTC (permalink / raw) To: Dave Martin Cc: Marc Zyngier, qemu-devel, qemu-arm, Guoheyi, kvmarm, linux-arm-kernel On Mon, Jul 15, 2019 at 02:41:00PM +0100, Dave Martin wrote: > On Sat, Jul 13, 2019 at 05:53:57PM +0800, Guoheyi wrote: > > Hi folks, > > > > Do it make sense to implement virtual SDEI in qemu? So that we can have the > > standard way for guest to handle NMI watchdog, RAS events and something else > > which involves SDEI in a physical ARM64 machine. > > > > My basic idea is like below: > > > > 1. Change a few lines of code in kvm to allow unhandled SMC invocations > > (like SDEI) to be sent to qemu, with exit reason of KVM_EXIT_HYPERCALL, so > > we don't need to add new API. > > So long as KVM_EXIT_HYPERCALL reports sufficient information so that > userspace can identify the cause as an SMC and retrieve the SMC > immediate field, this seems feasible. > > For its own SMCCC APIs, KVM exclusively uses HVC, so rerouting SMC to > userspace shouldn't conflict. Be _very_ careful here! In systems without EL3 (and without NV), SMC UNDEFs rather than trapping to EL2. Given that, we shouldn't build a hypervisor ABI that depends on SMC. I am strongly of the opinion that (for !NV) we should always use HVC here and have KVM appropriately forward calls to userspace, rather than trying to use HVC/SMC to distinguish handled-by-kernel and handled-by-userspace events. For NV, the first guest hypervisor would use SMC to talk to KVM, all else being the same. > This bouncing of SMCs to userspace would need to be opt-in, otherwise > old userspace would see exits that it doesn't know what to do with. > > > 2. qemu handles supported SDEI calls just as the spec says for what a > > hypervisor should do for a guest OS. > > > > 3. For interrupts bound to hypervisor, qemu should stop injecting the IRQ to > > guest through KVM, but jump to the registered event handler directly, > > including context saving and restoring. Some interrupts like virtual timer > > are handled by kvm directly, so we may refuse to bind such interrupts to > > SDEI events. > > Something like that. > > Interactions between SDEI and PSCI would need some thought: for example, > after PSCI_CPU_ON, the newly online cpu needs to have SDEs masked. > > One option (suggested to me by James Morse) would be to allow userspace > to disable in the in-kernel PSCI implementation and provide its own > PSCI to the guest via SMC -- in which case userspace that wants to > implement SDEI would have to implement PSCI as well. I think this would be the best approach, since it puts userspace in charge of everything. However, this interacts poorly with FW-based mitigations that we implement in hyp. I suspect we'd probably need a mechanism to delegate that responsibility back to the kernel, and figure out if that has any interaction with thigns that got punted to userspace... Thanks, Mark. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [RFC] Add virtual SDEI support in qemu 2019-07-15 13:48 ` Mark Rutland @ 2019-07-15 14:26 ` James Morse 2019-07-15 14:44 ` Mark Rutland 2019-07-16 8:30 ` Dave Martin 1 sibling, 1 reply; 9+ messages in thread From: James Morse @ 2019-07-15 14:26 UTC (permalink / raw) To: Guoheyi Cc: Mark Rutland, Marc Zyngier, qemu-devel, Dave Martin, qemu-arm, kvmarm, linux-arm-kernel Hi guys, On 15/07/2019 14:48, Mark Rutland wrote: > On Mon, Jul 15, 2019 at 02:41:00PM +0100, Dave Martin wrote: >> On Sat, Jul 13, 2019 at 05:53:57PM +0800, Guoheyi wrote: >>> Do it make sense to implement virtual SDEI in qemu? So that we can have the >>> standard way for guest to handle NMI watchdog, RAS events and something else >>> which involves SDEI in a physical ARM64 machine. I think so! >>> My basic idea is like below: >>> >>> 1. Change a few lines of code in kvm to allow unhandled SMC invocations >>> (like SDEI) to be sent to qemu, with exit reason of KVM_EXIT_HYPERCALL, so >>> we don't need to add new API. >> >> So long as KVM_EXIT_HYPERCALL reports sufficient information so that >> userspace can identify the cause as an SMC and retrieve the SMC >> immediate field, this seems feasible. >> >> For its own SMCCC APIs, KVM exclusively uses HVC, so rerouting SMC to >> userspace shouldn't conflict. > > Be _very_ careful here! In systems without EL3 (and without NV), SMC > UNDEFs rather than trapping to EL2. Given that, we shouldn't build a > hypervisor ABI that depends on SMC. > > I am strongly of the opinion that (for !NV) we should always use HVC > here and have KVM appropriately forward calls to userspace, rather than > trying to use HVC/SMC to distinguish handled-by-kernel and > handled-by-userspace events. > > For NV, the first guest hypervisor would use SMC to talk to KVM, all > else being the same. > >> This bouncing of SMCs to userspace would need to be opt-in, otherwise >> old userspace would see exits that it doesn't know what to do with. >> >>> 2. qemu handles supported SDEI calls just as the spec says for what a >>> hypervisor should do for a guest OS. [pulled point 3 out to discuss separately] >> Something like that. >> >> Interactions between SDEI and PSCI would need some thought: for example, >> after PSCI_CPU_ON, the newly online cpu needs to have SDEs masked. >> >> One option (suggested to me by James Morse) would be to allow userspace >> to disable in the in-kernel PSCI implementation and provide its own >> PSCI to the guest via SMC -- in which case userspace that wants to >> implement SDEI would have to implement PSCI as well. > > I think this would be the best approach, since it puts userspace in > charge of everything. > > However, this interacts poorly with FW-based mitigations that we > implement in hyp. I suspect we'd probably need a mechanism to delegate > that responsibility back to the kernel, and figure out if that has any > interaction with thigns that got punted to userspace... This has come up before: https://lore.kernel.org/r/59C139D0.3040507@arm.com I agree Qemu should opt-in to this, it needs to be a feature that is enabled. I had an early version of something like this for testing SDEI before there was firmware available. The review feedback from Christoffer was that it should include HVC and SMC, their immediates, and shouldn't be tied to SMC-CC ranges. I think this should be a catch-all as Heyi describes to deliver 'unhandled SMC/HVC' to user-space as hypercall exits. We should include the immediate in the struct. We can allow Qemu to disable the in-kernel PSCI implementation, which would let it be done in user-space via this catch-all mechanism. (PSCI in user-space has come up on another thread recently). The in-kernel PSCI needs to be default-on for backwards compatibility. As Mark points out, the piece that's left is the 'arch workaround' stuff. We always need to handle these in the kernel. I don't think these should be routed-back, they should be un-obtainable by user-space. Ideally there would be a way for user-space to discover the immediate/x0 values that the kernel will not deliver via the catch-all. This could be done via some co-processor register... thing... >>> 3. For interrupts bound to hypervisor, qemu should stop injecting the IRQ to >>> guest through KVM, but jump to the registered event handler directly, >>> including context saving and restoring. Some interrupts like virtual timer >>> are handled by kvm directly, so we may refuse to bind such interrupts to >>> SDEI events. I don't think we'd ever need a physical interrupt to be delivered via Qemu as an SDEI event. The use-cases for this stuff mean it can be done 'higher-up'. For example, your timer is probably used as a watchdog. On a real system this may well be a device with an interrupt, but Qemu could happily emulate a watchdog using some other linux API. (e.g. SIGALRM). Thanks, James ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [RFC] Add virtual SDEI support in qemu 2019-07-15 14:26 ` James Morse @ 2019-07-15 14:44 ` Mark Rutland 2019-07-16 8:47 ` Dave Martin 0 siblings, 1 reply; 9+ messages in thread From: Mark Rutland @ 2019-07-15 14:44 UTC (permalink / raw) To: James Morse Cc: Marc Zyngier, qemu-devel, Dave Martin, qemu-arm, Guoheyi, kvmarm, linux-arm-kernel On Mon, Jul 15, 2019 at 03:26:39PM +0100, James Morse wrote: > On 15/07/2019 14:48, Mark Rutland wrote: > > On Mon, Jul 15, 2019 at 02:41:00PM +0100, Dave Martin wrote: > >> One option (suggested to me by James Morse) would be to allow userspace > >> to disable in the in-kernel PSCI implementation and provide its own > >> PSCI to the guest via SMC -- in which case userspace that wants to > >> implement SDEI would have to implement PSCI as well. > > > > I think this would be the best approach, since it puts userspace in > > charge of everything. > > > > However, this interacts poorly with FW-based mitigations that we > > implement in hyp. I suspect we'd probably need a mechanism to delegate > > that responsibility back to the kernel, and figure out if that has any > > interaction with thigns that got punted to userspace... > > This has come up before: > https://lore.kernel.org/r/59C139D0.3040507@arm.com > > I agree Qemu should opt-in to this, it needs to be a feature that is enabled. > > I had an early version of something like this for testing SDEI before > there was firmware available. The review feedback from Christoffer was > that it should include HVC and SMC, their immediates, and shouldn't be > tied to SMC-CC ranges. > > I think this should be a catch-all as Heyi describes to deliver > 'unhandled SMC/HVC' to user-space as hypercall exits. We should > include the immediate in the struct. > > We can allow Qemu to disable the in-kernel PSCI implementation, which > would let it be done in user-space via this catch-all mechanism. (PSCI > in user-space has come up on another thread recently). The in-kernel > PSCI needs to be default-on for backwards compatibility. > > As Mark points out, the piece that's left is the 'arch workaround' > stuff. We always need to handle these in the kernel. I don't think > these should be routed-back, they should be un-obtainable by > user-space. Sure; I meant that those should be handled in the kernel rather than going to host userspace and back. I was suggesting was that userspace would opt into taking ownership of all HVC calls, then explicitly opt-in to the kernel handling specific (sets of) calls. There are probably issues with that, but I suspect defining "all undandled calls" will be problematic otherwise. Thanks, Mark. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [RFC] Add virtual SDEI support in qemu 2019-07-15 14:44 ` Mark Rutland @ 2019-07-16 8:47 ` Dave Martin 2019-07-19 1:49 ` Guoheyi 0 siblings, 1 reply; 9+ messages in thread From: Dave Martin @ 2019-07-16 8:47 UTC (permalink / raw) To: Mark Rutland Cc: Marc Zyngier, qemu-devel, qemu-arm, James Morse, Guoheyi, kvmarm, linux-arm-kernel On Mon, Jul 15, 2019 at 03:44:46PM +0100, Mark Rutland wrote: > On Mon, Jul 15, 2019 at 03:26:39PM +0100, James Morse wrote: > > On 15/07/2019 14:48, Mark Rutland wrote: > > > On Mon, Jul 15, 2019 at 02:41:00PM +0100, Dave Martin wrote: > > >> One option (suggested to me by James Morse) would be to allow userspace > > >> to disable in the in-kernel PSCI implementation and provide its own > > >> PSCI to the guest via SMC -- in which case userspace that wants to > > >> implement SDEI would have to implement PSCI as well. > > > > > > I think this would be the best approach, since it puts userspace in > > > charge of everything. > > > > > > However, this interacts poorly with FW-based mitigations that we > > > implement in hyp. I suspect we'd probably need a mechanism to delegate > > > that responsibility back to the kernel, and figure out if that has any > > > interaction with thigns that got punted to userspace... > > > > This has come up before: > > https://lore.kernel.org/r/59C139D0.3040507@arm.com > > > > I agree Qemu should opt-in to this, it needs to be a feature that is enabled. > > > > I had an early version of something like this for testing SDEI before > > there was firmware available. The review feedback from Christoffer was > > that it should include HVC and SMC, their immediates, and shouldn't be > > tied to SMC-CC ranges. > > > > I think this should be a catch-all as Heyi describes to deliver > > 'unhandled SMC/HVC' to user-space as hypercall exits. We should > > include the immediate in the struct. > > > > We can allow Qemu to disable the in-kernel PSCI implementation, which > > would let it be done in user-space via this catch-all mechanism. (PSCI > > in user-space has come up on another thread recently). The in-kernel > > PSCI needs to be default-on for backwards compatibility. > > > > As Mark points out, the piece that's left is the 'arch workaround' > > stuff. We always need to handle these in the kernel. I don't think > > these should be routed-back, they should be un-obtainable by > > user-space. > > Sure; I meant that those should be handled in the kernel rather than > going to host userspace and back. > > I was suggesting was that userspace would opt into taking ownership of > all HVC calls, then explicitly opt-in to the kernel handling specific > (sets of) calls. The most logical thing to do would be to have userspace handle all calls, but add an ioctl to forward a call to KVM. This puts userspace in charge of the SMCCC interface, with KVM handling only those things that userspace can't do for itself, on request. If the performance overhead is unacceptable for certain calls, we could have a way to delegate specific function IDs to KVM. I suspect that will be the exception rather than the rule. > There are probably issues with that, but I suspect defining "all > undandled calls" will be problematic otherwise. Agreed: the set of calls not handled by KVM will mutate over time. Cheers ---Dave ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [RFC] Add virtual SDEI support in qemu 2019-07-16 8:47 ` Dave Martin @ 2019-07-19 1:49 ` Guoheyi 0 siblings, 0 replies; 9+ messages in thread From: Guoheyi @ 2019-07-19 1:49 UTC (permalink / raw) To: Dave Martin, Mark Rutland Cc: Marc Zyngier, qemu-devel, qemu-arm, James Morse, kvmarm, linux-arm-kernel Thanks for all your comments. I'm going to write a simple demo to go through the whole workflow first, and then adjust the policies following the conclusions of our discussion. Heyi On 2019/7/16 16:47, Dave Martin wrote: > On Mon, Jul 15, 2019 at 03:44:46PM +0100, Mark Rutland wrote: >> On Mon, Jul 15, 2019 at 03:26:39PM +0100, James Morse wrote: >>> On 15/07/2019 14:48, Mark Rutland wrote: >>>> On Mon, Jul 15, 2019 at 02:41:00PM +0100, Dave Martin wrote: >>>>> One option (suggested to me by James Morse) would be to allow userspace >>>>> to disable in the in-kernel PSCI implementation and provide its own >>>>> PSCI to the guest via SMC -- in which case userspace that wants to >>>>> implement SDEI would have to implement PSCI as well. >>>> I think this would be the best approach, since it puts userspace in >>>> charge of everything. >>>> >>>> However, this interacts poorly with FW-based mitigations that we >>>> implement in hyp. I suspect we'd probably need a mechanism to delegate >>>> that responsibility back to the kernel, and figure out if that has any >>>> interaction with thigns that got punted to userspace... >>> This has come up before: >>> https://lore.kernel.org/r/59C139D0.3040507@arm.com >>> >>> I agree Qemu should opt-in to this, it needs to be a feature that is enabled. >>> >>> I had an early version of something like this for testing SDEI before >>> there was firmware available. The review feedback from Christoffer was >>> that it should include HVC and SMC, their immediates, and shouldn't be >>> tied to SMC-CC ranges. >>> >>> I think this should be a catch-all as Heyi describes to deliver >>> 'unhandled SMC/HVC' to user-space as hypercall exits. We should >>> include the immediate in the struct. >>> >>> We can allow Qemu to disable the in-kernel PSCI implementation, which >>> would let it be done in user-space via this catch-all mechanism. (PSCI >>> in user-space has come up on another thread recently). The in-kernel >>> PSCI needs to be default-on for backwards compatibility. >>> >>> As Mark points out, the piece that's left is the 'arch workaround' >>> stuff. We always need to handle these in the kernel. I don't think >>> these should be routed-back, they should be un-obtainable by >>> user-space. >> Sure; I meant that those should be handled in the kernel rather than >> going to host userspace and back. >> >> I was suggesting was that userspace would opt into taking ownership of >> all HVC calls, then explicitly opt-in to the kernel handling specific >> (sets of) calls. > The most logical thing to do would be to have userspace handle all > calls, but add an ioctl to forward a call to KVM. This puts userspace > in charge of the SMCCC interface, with KVM handling only those things > that userspace can't do for itself, on request. > > If the performance overhead is unacceptable for certain calls, we could > have a way to delegate specific function IDs to KVM. I suspect that > will be the exception rather than the rule. > >> There are probably issues with that, but I suspect defining "all >> undandled calls" will be problematic otherwise. > Agreed: the set of calls not handled by KVM will mutate over time. > > Cheers > ---Dave > > . > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [RFC] Add virtual SDEI support in qemu 2019-07-15 13:48 ` Mark Rutland 2019-07-15 14:26 ` James Morse @ 2019-07-16 8:30 ` Dave Martin 2019-07-16 8:46 ` Marc Zyngier 1 sibling, 1 reply; 9+ messages in thread From: Dave Martin @ 2019-07-16 8:30 UTC (permalink / raw) To: Mark Rutland Cc: Marc Zyngier, qemu-devel, qemu-arm, Guoheyi, kvmarm, linux-arm-kernel On Mon, Jul 15, 2019 at 02:48:49PM +0100, Mark Rutland wrote: > On Mon, Jul 15, 2019 at 02:41:00PM +0100, Dave Martin wrote: [...] > > So long as KVM_EXIT_HYPERCALL reports sufficient information so that > > userspace can identify the cause as an SMC and retrieve the SMC > > immediate field, this seems feasible. > > > > For its own SMCCC APIs, KVM exclusively uses HVC, so rerouting SMC to > > userspace shouldn't conflict. > > Be _very_ careful here! In systems without EL3 (and without NV), SMC > UNDEFs rather than trapping to EL2. Given that, we shouldn't build a > hypervisor ABI that depends on SMC. Good point. I was hoping that was all ancient history by now, but if not... [...] Cheers ---Dave ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [RFC] Add virtual SDEI support in qemu 2019-07-16 8:30 ` Dave Martin @ 2019-07-16 8:46 ` Marc Zyngier 0 siblings, 0 replies; 9+ messages in thread From: Marc Zyngier @ 2019-07-16 8:46 UTC (permalink / raw) To: Dave Martin, Mark Rutland Cc: Guoheyi, qemu-arm, qemu-devel, linux-arm-kernel, kvmarm On 16/07/2019 09:30, Dave Martin wrote: > On Mon, Jul 15, 2019 at 02:48:49PM +0100, Mark Rutland wrote: >> On Mon, Jul 15, 2019 at 02:41:00PM +0100, Dave Martin wrote: > > [...] > >>> So long as KVM_EXIT_HYPERCALL reports sufficient information so that >>> userspace can identify the cause as an SMC and retrieve the SMC >>> immediate field, this seems feasible. >>> >>> For its own SMCCC APIs, KVM exclusively uses HVC, so rerouting SMC to >>> userspace shouldn't conflict. >> >> Be _very_ careful here! In systems without EL3 (and without NV), SMC >> UNDEFs rather than trapping to EL2. Given that, we shouldn't build a >> hypervisor ABI that depends on SMC. > > Good point. I was hoping that was all ancient history by now, but if > not... Unfortunately, XGene-1 is still a thing... M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-07-19 1:49 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-07-13 9:53 [Qemu-devel] [RFC] Add virtual SDEI support in qemu Guoheyi 2019-07-15 13:41 ` Dave Martin 2019-07-15 13:48 ` Mark Rutland 2019-07-15 14:26 ` James Morse 2019-07-15 14:44 ` Mark Rutland 2019-07-16 8:47 ` Dave Martin 2019-07-19 1:49 ` Guoheyi 2019-07-16 8:30 ` Dave Martin 2019-07-16 8:46 ` Marc Zyngier
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).