[PATCH AUTOSEL 4.14 01/21] KVM: arm/arm64: vgic: Use the appropriate TRACE_INCLUDE_PATH
From: Zenghui Yu [ Upstream commit aac60f1a867773de9eb164013d89c99f3ea1f009 ] Commit 49dfe94fe5ad ("KVM: arm/arm64: Fix TRACE_INCLUDE_PATH") fixes TRACE_INCLUDE_PATH to the correct relative path to the define_trace.h and explains why did the old one work. The same fix should be applied to virt/kvm/arm/vgic/trace.h. Reviewed-by: Masahiro Yamada Signed-off-by: Zenghui Yu Signed-off-by: Marc Zyngier Signed-off-by: Sasha Levin --- virt/kvm/arm/vgic/trace.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/virt/kvm/arm/vgic/trace.h b/virt/kvm/arm/vgic/trace.h index 55fed77a9f739..4fd4f6db181b0 100644 --- a/virt/kvm/arm/vgic/trace.h +++ b/virt/kvm/arm/vgic/trace.h @@ -30,7 +30,7 @@ TRACE_EVENT(vgic_update_irq_pending, #endif /* _TRACE_VGIC_H */ #undef TRACE_INCLUDE_PATH -#define TRACE_INCLUDE_PATH ../../../virt/kvm/arm/vgic +#define TRACE_INCLUDE_PATH ../../virt/kvm/arm/vgic #undef TRACE_INCLUDE_FILE #define TRACE_INCLUDE_FILE trace -- 2.20.1 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH AUTOSEL 5.3 01/68] KVM: arm/arm64: vgic: Use the appropriate TRACE_INCLUDE_PATH
From: Zenghui Yu [ Upstream commit aac60f1a867773de9eb164013d89c99f3ea1f009 ] Commit 49dfe94fe5ad ("KVM: arm/arm64: Fix TRACE_INCLUDE_PATH") fixes TRACE_INCLUDE_PATH to the correct relative path to the define_trace.h and explains why did the old one work. The same fix should be applied to virt/kvm/arm/vgic/trace.h. Reviewed-by: Masahiro Yamada Signed-off-by: Zenghui Yu Signed-off-by: Marc Zyngier Signed-off-by: Sasha Levin --- virt/kvm/arm/vgic/trace.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/virt/kvm/arm/vgic/trace.h b/virt/kvm/arm/vgic/trace.h index 55fed77a9f739..4fd4f6db181b0 100644 --- a/virt/kvm/arm/vgic/trace.h +++ b/virt/kvm/arm/vgic/trace.h @@ -30,7 +30,7 @@ TRACE_EVENT(vgic_update_irq_pending, #endif /* _TRACE_VGIC_H */ #undef TRACE_INCLUDE_PATH -#define TRACE_INCLUDE_PATH ../../../virt/kvm/arm/vgic +#define TRACE_INCLUDE_PATH ../../virt/kvm/arm/vgic #undef TRACE_INCLUDE_FILE #define TRACE_INCLUDE_FILE trace -- 2.20.1 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH AUTOSEL 4.19 01/26] KVM: arm/arm64: vgic: Use the appropriate TRACE_INCLUDE_PATH
From: Zenghui Yu [ Upstream commit aac60f1a867773de9eb164013d89c99f3ea1f009 ] Commit 49dfe94fe5ad ("KVM: arm/arm64: Fix TRACE_INCLUDE_PATH") fixes TRACE_INCLUDE_PATH to the correct relative path to the define_trace.h and explains why did the old one work. The same fix should be applied to virt/kvm/arm/vgic/trace.h. Reviewed-by: Masahiro Yamada Signed-off-by: Zenghui Yu Signed-off-by: Marc Zyngier Signed-off-by: Sasha Levin --- virt/kvm/arm/vgic/trace.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/virt/kvm/arm/vgic/trace.h b/virt/kvm/arm/vgic/trace.h index 55fed77a9f739..4fd4f6db181b0 100644 --- a/virt/kvm/arm/vgic/trace.h +++ b/virt/kvm/arm/vgic/trace.h @@ -30,7 +30,7 @@ TRACE_EVENT(vgic_update_irq_pending, #endif /* _TRACE_VGIC_H */ #undef TRACE_INCLUDE_PATH -#define TRACE_INCLUDE_PATH ../../../virt/kvm/arm/vgic +#define TRACE_INCLUDE_PATH ../../virt/kvm/arm/vgic #undef TRACE_INCLUDE_FILE #define TRACE_INCLUDE_FILE trace -- 2.20.1 ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 00/12] Add SDEI support for arm64
On 2019/9/30 21:15, Peter Maydell wrote: On Tue, 24 Sep 2019 at 16:23, Heyi Guo wrote: As promised, this is the first RFC patch set for arm64 SDEI support. Hi; for the benefit of possible reviewers who aren't familiar with every corner of the arm ecosystem, could you provide a summary of: * what is SDEI ? SDEI is for ARM "Software Delegated Exception Interface". AS ARM64 doesn't have native non-maskable interrupt (NMI), we can rely on higher privileged software to change the pc of lower privileged software on certain events occur, to emulate NMI mechanism, and SDEI is the standard interfaces between the two levels of privileged software. It is based on SMC/HVC calls. In virtualization situation, guest OS is the lower privileged software and hypervisor is the higher one. Major interfaces provided by SDEI include: 1. interrupt bind: guest OS can request to bind an interrupt to an SDEI event. 2. register: guest OS can request to register a handler to an SDEI event, so hypervisor will change pc of guest to this handler when certain event occurs. 3. complete: guest OS notifies hypervisor that it has completed the event handling, so hypervisor will restore the context of guest when it is interrupted. * what do KVM and QEMU want/need to do with it ? KVM is supposed to pass SMC/HVC calls to qemu, and qemu will serve the SDEI requests after parsing SMC/HVC calls. qemu also takes the responsibility to trigger the events. If an interrupt is requested to be bound to an event, qemu should not inject the interrupt to guest any more; instead, it should save the context of VCPU and change the PC to event handler which is registered by guest, and then return to guest. To make the conversion of interrupt to SDEI event transparent to other modules in qemu, we used qemu_irq and qemu_irq_intercept_in() to override the default irq handler with SDEI event trigger. I saw qemu_irq_intercept_in() should be only used in qemu MST, but it seemed fit to override interrupt injection with event trigger after guest requests to bind interrupt to SDEI event. * what is this patchset trying to solve ? This patchset is trying to implement the whole SDEI framework in qemu with KVM enabled, including all SDEI v1.0 interfaces, as well as event trigger conduit from other qemu devices after interrupt binding. I will also provide the above context in the cover letter of v2 RFC. Thanks, Heyi That would provide some useful context for trying to review the patchset. thanks -- PMM . ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 07/12] arm/sdei: override qemu_irq handler when binding interrupt
Hi Peter, Thanks for your comments. I will explain SDEI in another mail and please provide your suggestions for such situation. Heyi On 2019/9/30 21:19, Peter Maydell wrote: On Tue, 24 Sep 2019 at 16:23, Heyi Guo wrote: Override qemu_irq handler to support trigger SDEI event transparently after guest binds interrupt to SDEI event. We don't have good way to get GIC device and to guarantee SDEI device is initialized after GIC, so we search GIC in system bus when the first SDEI request happens or in VMSTATE post_load(). Signed-off-by: Heyi Guo Cc: Peter Maydell Cc: Dave Martin Cc: Marc Zyngier Cc: Mark Rutland Cc: James Morse +static void override_qemu_irq(QemuSDEState *s, int32_t event, uint32_t intid) +{ +qemu_irq irq; +QemuSDE *sde; +CPUState *cs; +int cpu; + +/* SPI */ +if (intid >= GIC_INTERNAL) { +cs = arm_get_cpu_by_id(0); +irq = qdev_get_gpio_in(s->gic_dev, + gic_int_to_irq(s->num_irq, intid, 0)); +if (irq) { +qemu_irq_intercept_in(, qemu_sdei_irq_handler, 1); +} I'm not sure what this code is trying to do, but qemu_irq_intercept_in() is a function for internal use by the qtest testing infrastructure, so it shouldn't be used in 'real' QEMU code. +sde = get_sde_no_check(s, event, cs); +sde->irq = irq; +put_sde(sde, cs); +return; +} @@ -1042,6 +1152,17 @@ void sdei_handle_request(CPUState *cs, struct kvm_run *run) return; } +if (!sde_state->gic_dev) { +/* Search for ARM GIC device */ +qbus_walk_children(sysbus_get_default(), dev_walkerfn, + NULL, NULL, NULL, sde_state); +if (!sde_state->gic_dev) { +error_report("Cannot find ARM GIC device!"); +run->hypercall.args[0] = SDEI_NOT_SUPPORTED; +return; +} +} Walking through the qbus tree looking for particular devices isn't really something I'd recommend either. thanks -- PMM ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [RFC PATCH 1/2] kvm/arm: add capability to forward hypercall to user space
Sorry for late response as we had our long holiday last week :) On 2019/10/2 1:19, James Morse wrote: Hi Heyi, On 24/09/2019 16:20, Heyi Guo wrote: As more SMC/HVC usages emerge on arm64 platforms, like SDEI, it makes sense for kvm to have the capability of forwarding such calls to user space for further emulation. (what do you mean by further? Doesn't user-space have to do all of it?) For kvm will always handle hvc/smc guest exit for the first step, even if it is only a simple forwarding, I called the user-space processing as "further emulation". We reuse the existing term "hypercall" for SMC/HVC, as well as the hypercall structure in kvm_run to exchange arguments and return values. The definition on arm64 is as below: exit_reason: KVM_EXIT_HYPERCALL Input: nr: the immediate value of SMC/HVC calls; not really used today. args[6]: x0..x5 (This is not fully conform with SMCCC which requires x6 as argument as well, but use space can use GET_ONE_REG ioctl for such rare case). If this structure isn't right for us, we could define a different one for arm/arm64. (we did this for kvm_vcpu_events) Do you mean that we can move the hypercall struct definition to arch specific kvm_host.h? For it is in the common kvm_run structure, we'll need to change every kvm supported architectures, including x86, mips, powerpc, s390. Is it acceptable? I found another solution from papr which defines its own hypercall structure in the kvm_run union definition: /* KVM_EXIT_PAPR_HCALL */ struct { __u64 nr; __u64 ret; __u64 args[9]; } papr_hcall; How about we define a new structure for ARM/ARM64 specifically? Return: args[0..3]: x0..x3 as defined in SMCCC. We need to extract args[0..3] and write them to x0..x3 when hypercall exit returns. Are we saying that KVM_EXIT_HYPERCALL expects to be used with SMC-CC? (if so, we should state that). Yes I followed SMC-CC when writing this. I'm not certain we should tie this to SMC-CC. If we don't tie it to SMC-CC this selection of in/out registers looks odd, there is nothing about HVC/SMC that uses these registers, its just the SMC convention. Maybe we don't need to tie it to SMC-CC, and simply load all values in args[6] to GP registers... And then there is either no strong reason to extend hypercall structure for ARM. Flag hypercall_forward is added to turn on/off hypercall forwarding and the default is false. Another flag hypercall_excl_psci is to exclude PSCI from forwarding for backward compatible, and it only makes sense to check its value when hypercall_forward is enabled. Calling out PSCI like this is something we shouldn't do. There will be, (are!) other SMC-CC calls that the kernel provides emulation for, we can't easily add to this list. Yes; I didn't figure out good way to keep compatibility and future extension... I think the best way to avoid this, is to say the hypercall mechanism forwards 'unhandled SMC/HVC' to user-space. Which things the kernel chooses to handle can change. We need a way for user-space to know which SMC/HVC calls the kernel will handle, and will not forward. A suggestion is to add a co-processor that lists these by #imm and r0/x0 value. User-space can then query any call to find out if it would be exported if the guest made that call. Something like kvm_arm_get_fw_reg(). Do you mean we add only one co-processor to list all SMC/HVC calls kernel will handle? So the reg size should be large enough to hold the list, each entry of which contains a #imm and r0/x0 pair? Is the reg size fixed by definition or it can be queried by user-space? If it is fixed, what's the size should we choose? Does it make sense to extend the entry to hold the function ID base and limit, so that it can describe the whole range for each function group, like PSCI, SDEI, etc? I agree it should be possible to export the PSCI range to user-space, so that user-space can provide a newer/better version than the kernel emulates, or prevent secondary cores coming online. (we should check how gracefully the kernel handles that... it doesn't happen on real systems) This could be done with something like kvm_vm_ioctl_enable_cap(), one option is to use the args to toggle the on/off value, but it may be simpler to expose a KVM_CAP_ARM_PSCI_TO_USER that can be enabled. Sounds good. Then it may not be something we need to do in this patch set :) We can postpone this change when user-space PSCI is ready. Please update Documentation/virt/kvm/api.txt as part of the patches that make user-visible changes. Sure; I can do that when we determine the interfaces. For 32bit, are we going to export SMC/HVC calls that failed their condition-code checks? I'm not familiar with 32bit, either we don't have 32bit platforms to test the code. So my preference is not to make many changes to 32bit... The hypercall structure
Re: [PATCH v2 2/2] KVM: arm/arm64: Allow user injection of external data aborts
On Tue, Oct 08, 2019 at 02:03:07PM +0200, Alexander Graf wrote: > > > On 08.10.19 11:36, Christoffer Dall wrote: > > In some scenarios, such as buggy guest or incorrect configuration of the > > VMM and firmware description data, userspace will detect a memory access > > to a portion of the IPA, which is not mapped to any MMIO region. > > > > For this purpose, the appropriate action is to inject an external abort > > to the guest. The kernel already has functionality to inject an > > external abort, but we need to wire up a signal from user space that > > lets user space tell the kernel to do this. > > > > It turns out, we already have the set event functionality which we can > > perfectly reuse for this. > > > > Signed-off-by: Christoffer Dall > > --- > > Documentation/virt/kvm/api.txt| 18 +- > > arch/arm/include/uapi/asm/kvm.h | 3 ++- > > arch/arm/kvm/guest.c | 3 +++ > > arch/arm64/include/uapi/asm/kvm.h | 3 ++- > > arch/arm64/kvm/guest.c| 3 +++ > > arch/arm64/kvm/inject_fault.c | 4 ++-- > > include/uapi/linux/kvm.h | 1 + > > virt/kvm/arm/arm.c| 1 + > > 8 files changed, 31 insertions(+), 5 deletions(-) > > > > diff --git a/Documentation/virt/kvm/api.txt b/Documentation/virt/kvm/api.txt > > index 7403f15657c2..10ebe8cfda29 100644 > > --- a/Documentation/virt/kvm/api.txt > > +++ b/Documentation/virt/kvm/api.txt > > @@ -968,6 +968,8 @@ The following bits are defined in the flags field: > > ARM/ARM64: > > +User space may need to inject several types of events to the guest. > > + > > If the guest accesses a device that is being emulated by the host kernel > > in > > such a way that a real device would generate a physical SError, KVM may > > make > > a virtual SError pending for that VCPU. This system error interrupt > > remains > > @@ -1002,12 +1004,26 @@ Specifying exception.has_esr on a system that does > > not support it will return > > -EINVAL. Setting anything other than the lower 24bits of > > exception.serror_esr > > will return -EINVAL. > > +If the guest performed an access to I/O memory which could not be handled > > by > > +userspace, for example because of missing instruction syndrome decode > > +information or because there is no device mapped at the accessed IPA, then > > +userspace can ask the kernel to inject an external abort using the address > > +from the exiting fault on the VCPU. It is a programming error to set > > +ext_dabt_pending at the same time as any of the serror fields, or to set > > +ext_dabt_pending after an exit which was not either KVM_EXIT_MMIO or > > +KVM_EXIT_ARM_NISV. This feature is only available if the system supports > > +KVM_CAP_ARM_INJECT_EXT_DABT. This is a helper which provides commonality in > > +how userspace reports accesses for the above cases to guests, across > > different > > +userspace implementations. Nevertheless, userspace can still emulate all > > Arm > > +exceptions by manipulating individual registers using the KVM_SET_ONE_REG > > API. > > + > > struct kvm_vcpu_events { > > struct { > > __u8 serror_pending; > > __u8 serror_has_esr; > > + __u8 ext_dabt_pending; > > /* Align it to 8 bytes */ > > - __u8 pad[6]; > > + __u8 pad[5]; > > __u64 serror_esr; > > } exception; > > __u32 reserved[12]; > > diff --git a/arch/arm/include/uapi/asm/kvm.h > > b/arch/arm/include/uapi/asm/kvm.h > > index 2769360f195c..03cd7c19a683 100644 > > --- a/arch/arm/include/uapi/asm/kvm.h > > +++ b/arch/arm/include/uapi/asm/kvm.h > > @@ -131,8 +131,9 @@ struct kvm_vcpu_events { > > struct { > > __u8 serror_pending; > > __u8 serror_has_esr; > > + __u8 ext_dabt_pending; > > /* Align it to 8 bytes */ > > - __u8 pad[6]; > > + __u8 pad[5]; > > __u64 serror_esr; > > } exception; > > __u32 reserved[12]; > > diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c > > index 684cf64b4033..4154c5589501 100644 > > --- a/arch/arm/kvm/guest.c > > +++ b/arch/arm/kvm/guest.c > > @@ -263,11 +263,14 @@ int __kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu, > > { > > bool serror_pending = events->exception.serror_pending; > > bool has_esr = events->exception.serror_has_esr; > > + bool has_ext_dabt_pending = events->exception.ext_dabt_pending; > > if (serror_pending && has_esr) > > return -EINVAL; > > else if (serror_pending) > > kvm_inject_vabt(vcpu); > > + else if (has_ext_dabt_pending) > > + kvm_inject_dabt(vcpu, kvm_vcpu_get_hfar(vcpu)); > > return 0; > > } > > diff --git a/arch/arm64/include/uapi/asm/kvm.h > > b/arch/arm64/include/uapi/asm/kvm.h > > index 67c21f9bdbad..d49c17a80491 100644 > > --- a/arch/arm64/include/uapi/asm/kvm.h > > +++ b/arch/arm64/include/uapi/asm/kvm.h > > @@ -164,8 +164,9 @@ struct kvm_vcpu_events { > >