Re: [PATCH] arm64/kvm: Add generic v8 KVM target
On Jul 17, 2015, at 3:19 AM, Marc Zyngier marc.zyng...@arm.com wrote: On 17/07/15 11:15, Christoffer Dall wrote: On Fri, Jul 17, 2015 at 10:56:39AM +0100, Marc Zyngier wrote: On 17/07/15 10:33, Christoffer Dall wrote: On Fri, Jul 03, 2015 at 11:10:09AM +0100, Marc Zyngier wrote: On 03/07/15 10:34, Peter Maydell wrote: On 3 July 2015 at 09:28, Marc Zyngier marc.zyng...@arm.com wrote: On 03/07/15 09:12, Peter Maydell wrote: I would still like to see the proponents of this patch say what their model is for userspace support of cross-host migration, if we're abandoning the model the current API envisages. I thought we had discussed this above, and don't really see this as a departure from the current model: - -cpu host results in GENERIC being used: VM can only be migrated to the exact same HW (no cross-host migration). MIDR should probably become RO. - -cpu host results in A57 (for example): VM can be migrated to a variety of A57 platforms, and allow for some fuzzing on the revision (or accept any revision). - -cpu a57 forces an A57 model to be emulated, always. It is always possible to migrate such a VM on any host. I think only the first point is new, but the last two are what we have (or what we should have). Right, but the implicit idea of this GENERIC patch seems to be that new host CPU types don't get their own KVM_ARM_TARGET_* constant, and are thus forever unable to do cross-host migration. It's not clear to me why we'd want to have new CPUs be second class citizens like that. I certainly don't want to see *any* CPU be a second class citizen. But let's face it, we're adding more and more targets that don't implement anything new, and just satisfy themselves with the generic implementation. I see it as an incentive to provide something useful (tables of all the registers with default values?) so that cross-host migration becomes a reality instead of the figment of our imagination (as it is now). If it wasn't already ABI, I'd have removed the existing targets until we have something meaningful to put there. What we're doing now certainly seems silly, because we're adding kernel patches without bringing anything to the table... Now, I also have my own doubts about cross-host migration (timers anyone?). But I don't see the above as a change in policy. More as a way to outline the fact that we currently don't have the right level of information/infrastructure to support it at all. The one thing that I've lost track of here (sorry) is whether we're enforcing the inability to do cross-host migration with the generic target when this patch is merged or do we leave this up to the graces of userspace? The jury is still out on that one. I was initially not going to enforce anything (after all, this isn't that different from the whole CNTVOFF story where we allow userspace to shoot itself in the foot), but I'm equally happy to make MIDR_EL1 read-only if we're creating a generic guest... Looking at the code, midr_el1 is already an invariant register, so isn't this automagically enforced already? Ah, you're perfectly right, I has already in that fantasy world where we can actually migrate VMs across implementations. In that case, I'm fine with merging this patch. Cool. I'll queue that for 4.3. this sounds nice. Thanks, M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64/kvm: Add generic v8 KVM target
On Jun 29, 2015, at 10:52 AM, Marc Zyngier marc.zyng...@arm.com wrote: On 29/06/15 18:38, Peter Maydell wrote: On 29 June 2015 at 18:30, Marc Zyngier marc.zyng...@arm.com wrote: On 29/06/15 18:13, Chalamarla, Tirumalesh wrote: Will this also prevents migrating between same implementations, if no how is this identified. This shouldn't. It is pretty easy to look at the incoming guest's MIDR, and verify that it matches the default MIDR on the receiving system. Any difference, and the migration should abort. Do you really want to forbid migration between (say) Cortex-A57 r3p0 and Cortex-A57 r3p1 ? That seems pretty harsh. I think we may need to allow for some flexibility (same major version only, or +/- 1 minor version...). The idea I'm trying to convey is that with generic CPI, migration is not guaranteed unless we're on extremely similar hardware. yes, this is the point i am trying to make, we need some flexibility. so that it works with same chips with different passes may be. if we are allowing this, then we are not putting emulation as a requirement. Thanks, Tirumalesh. M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64/kvm: Add generic v8 KVM target
On Jun 26, 2015, at 2:53 AM, Christoffer Dall christoffer.d...@linaro.org wrote: On Thu, Jun 25, 2015 at 02:49:08PM +0100, Peter Maydell wrote: On 25 June 2015 at 14:44, Marc Zyngier marc.zyng...@arm.com wrote: It should always be possible to emulate a known CPU on a generic host, and it should be able to migrate. The case we can't migrate is when we let the guest be generic (which I guess should really be unknown, and not generic). So if the user specify -cpu cortex-a57 on the command line, the guest should be able to migrate from an A72 to an A53. if the user specified -cpu host, the resulting guest won't be able to migrate. Does it make sense? Yes. We've always said -cpu host won't be cross-host migratable from a QEMU point of view. ok, this makes sense. It's basically up to userspace to mandate that trying to migrate something that used unknown cpu (via -cpu host or whatever) cannot be migrated. Will this also prevents migrating between same implementations, if no how is this identified. This seems to be making emulation a requirment for ARM64 KVM. Thanks, Tirumalesh. -Christoffer ___ kvmarm mailing list kvm...@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC PATCH v6 04/20] iommu/arm-smmu: add capability IOMMU_CAP_INTR_REMAP
Marc, What is your opinion on ITS emulation . is it should be part of KVM or VFIO. Also this code needs to depend on ITS host driver a lot, Host ITS driver needs to have an interface for this code to use. Thanks, Tirumalesh -Original Message- From: Will Deacon [mailto:will.dea...@arm.com] Sent: Friday, June 27, 2014 1:47 AM To: Alex Williamson Cc: Chalamarla, Tirumalesh; Joerg Roedel; kvm@vger.kernel.org; open list; stuart.yo...@freescale.com; io...@lists.linux-foundation.org; t...@virtualopensystems.com; kvm...@lists.cs.columbia.edu; moderated list:ARM SMMU DRIVER; marc.zyng...@arm.com Subject: Re: [RFC PATCH v6 04/20] iommu/arm-smmu: add capability IOMMU_CAP_INTR_REMAP On Thu, Jun 26, 2014 at 08:36:24PM +0100, Alex Williamson wrote: On Thu, 2014-06-26 at 19:10 +, Chalamarla, Tirumalesh wrote: Thanks for the clarification Alex, That’s exactly my point, why are we relying on QEMU or something else to emulate the MSI space when we can directly give access to devices using ITS (of course with a small emulation code). This way we are also benefited from all ITS services like VCPU migration etc. I have no idea what ITS is. ITS is the MSI doorbell for GICv3 (ARM's latest interrupt controller). I agree that we will need an ITS emulation if we want to use MSIs in the guest, and I believe that Marc (CC'd) had already started thinking about that. What about non QEMU VFIO users, for example, if I wanted to use VFIO to assign a device to a user process I don't need to depend on QEMU. I thought this is one of the main goals of vfio to make it independent of hypervisors. Where did QEMU become a requirement? Maybe I'm missing something in the ARM part of the conversation that got chopped off, but this is exactly why we have the VFIO/QEMU split that we do. VFIO provides basic virtualization for config space and restricts access to other areas that users shouldn't be allowed to change. QEMU is just one example of a userspace VFIO driver. QEMU takes the decomposed device exposed through the VFIO ABI and re-creates a PCI device out of it. VFIO itself has no dependency on QEMU. Thanks, I also don't understand the QEMU part here. The MSI emulation would be in the kernel, just like the GICv2 emulation that we already have. For userspace drivers, wouldn't you just use eventfd rather than bother with emulating MSIs? Finally, the interrupt remapping part is about the SMMU preventing MSI writes to arbitrary portions of the host address space. The ITS is about routing interrupts to CPUs. Will
RE: [RFC PATCH v6 04/20] iommu/arm-smmu: add capability IOMMU_CAP_INTR_REMAP
Forgive me if this discussion is not relative here, but I thought it is. How is VFIO restricting devices from writing to MSI/MSI-X, Is all the vector area is mapped by VFIO to trap the accesses. I am asking this because we might need to emulate ITS somewhere either in KVM or VFIO to provide direct access to devices. And I don't see any mentions on that. I think this flag needs to be set by ITS emulation. Regards, Tirumalesh. -Original Message- From: kvmarm-boun...@lists.cs.columbia.edu [mailto:kvmarm-boun...@lists.cs.columbia.edu] On Behalf Of Joerg Roedel Sent: Monday, June 16, 2014 8:39 AM To: Will Deacon Cc: stuart.yo...@freescale.com; kvm@vger.kernel.org; open list; io...@lists.linux-foundation.org; alex.william...@redhat.com; moderated list:ARM SMMU DRIVER; t...@virtualopensystems.com; kvm...@lists.cs.columbia.edu; Christoffer Dall Subject: Re: [RFC PATCH v6 04/20] iommu/arm-smmu: add capability IOMMU_CAP_INTR_REMAP On Mon, Jun 16, 2014 at 04:25:26PM +0100, Will Deacon wrote: Ok, thanks. In which case, I think this is really a combined property of the SMMU and the interrupt controller, so we might need some extra code so that the SMMU can check that the interrupt controller for the device is also capable of interrupt remapping. Right, that this is part of IOMMU code has more or less historic reasons on x86. Interrupt remapping is purely implemented in the IOMMU there, so on ARM some clue-code between interrupt controler and smmu is needed. Joerg ___ kvmarm mailing list kvm...@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC PATCH v6 04/20] iommu/arm-smmu: add capability IOMMU_CAP_INTR_REMAP
When I say emulating ITS, I mean translating guest ITS commands to physical ITS commands and placing them in physical queue. Regards, Tirumalesh. -Original Message- From: kvmarm-boun...@lists.cs.columbia.edu [mailto:kvmarm-boun...@lists.cs.columbia.edu] On Behalf Of Chalamarla, Tirumalesh Sent: Thursday, June 26, 2014 11:08 AM To: Joerg Roedel; Will Deacon Cc: kvm@vger.kernel.org; open list; alex.william...@redhat.com; stuart.yo...@freescale.com; io...@lists.linux-foundation.org; t...@virtualopensystems.com; kvm...@lists.cs.columbia.edu; moderated list:ARM SMMU DRIVER Subject: RE: [RFC PATCH v6 04/20] iommu/arm-smmu: add capability IOMMU_CAP_INTR_REMAP Forgive me if this discussion is not relative here, but I thought it is. How is VFIO restricting devices from writing to MSI/MSI-X, Is all the vector area is mapped by VFIO to trap the accesses. I am asking this because we might need to emulate ITS somewhere either in KVM or VFIO to provide direct access to devices. And I don't see any mentions on that. I think this flag needs to be set by ITS emulation. Regards, Tirumalesh. -Original Message- From: kvmarm-boun...@lists.cs.columbia.edu [mailto:kvmarm-boun...@lists.cs.columbia.edu] On Behalf Of Joerg Roedel Sent: Monday, June 16, 2014 8:39 AM To: Will Deacon Cc: stuart.yo...@freescale.com; kvm@vger.kernel.org; open list; io...@lists.linux-foundation.org; alex.william...@redhat.com; moderated list:ARM SMMU DRIVER; t...@virtualopensystems.com; kvm...@lists.cs.columbia.edu; Christoffer Dall Subject: Re: [RFC PATCH v6 04/20] iommu/arm-smmu: add capability IOMMU_CAP_INTR_REMAP On Mon, Jun 16, 2014 at 04:25:26PM +0100, Will Deacon wrote: Ok, thanks. In which case, I think this is really a combined property of the SMMU and the interrupt controller, so we might need some extra code so that the SMMU can check that the interrupt controller for the device is also capable of interrupt remapping. Right, that this is part of IOMMU code has more or less historic reasons on x86. Interrupt remapping is purely implemented in the IOMMU there, so on ARM some clue-code between interrupt controler and smmu is needed. Joerg ___ kvmarm mailing list kvm...@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ___ kvmarm mailing list kvm...@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC PATCH v6 04/20] iommu/arm-smmu: add capability IOMMU_CAP_INTR_REMAP
Sorry there was a type, The question is: How is VFIO restricting software from writing to MSI/MSI-X vectors of the device. -Original Message- From: Chalamarla, Tirumalesh Sent: Thursday, June 26, 2014 11:16 AM To: Chalamarla, Tirumalesh; Joerg Roedel; Will Deacon Cc: kvm@vger.kernel.org; open list; alex.william...@redhat.com; stuart.yo...@freescale.com; io...@lists.linux-foundation.org; t...@virtualopensystems.com; kvm...@lists.cs.columbia.edu; moderated list:ARM SMMU DRIVER Subject: RE: [RFC PATCH v6 04/20] iommu/arm-smmu: add capability IOMMU_CAP_INTR_REMAP When I say emulating ITS, I mean translating guest ITS commands to physical ITS commands and placing them in physical queue. Regards, Tirumalesh. -Original Message- From: kvmarm-boun...@lists.cs.columbia.edu [mailto:kvmarm-boun...@lists.cs.columbia.edu] On Behalf Of Chalamarla, Tirumalesh Sent: Thursday, June 26, 2014 11:08 AM To: Joerg Roedel; Will Deacon Cc: kvm@vger.kernel.org; open list; alex.william...@redhat.com; stuart.yo...@freescale.com; io...@lists.linux-foundation.org; t...@virtualopensystems.com; kvm...@lists.cs.columbia.edu; moderated list:ARM SMMU DRIVER Subject: RE: [RFC PATCH v6 04/20] iommu/arm-smmu: add capability IOMMU_CAP_INTR_REMAP Forgive me if this discussion is not relative here, but I thought it is. How is VFIO restricting devices from writing to MSI/MSI-X, Is all the vector area is mapped by VFIO to trap the accesses. I am asking this because we might need to emulate ITS somewhere either in KVM or VFIO to provide direct access to devices. And I don't see any mentions on that. I think this flag needs to be set by ITS emulation. Regards, Tirumalesh. -Original Message- From: kvmarm-boun...@lists.cs.columbia.edu [mailto:kvmarm-boun...@lists.cs.columbia.edu] On Behalf Of Joerg Roedel Sent: Monday, June 16, 2014 8:39 AM To: Will Deacon Cc: stuart.yo...@freescale.com; kvm@vger.kernel.org; open list; io...@lists.linux-foundation.org; alex.william...@redhat.com; moderated list:ARM SMMU DRIVER; t...@virtualopensystems.com; kvm...@lists.cs.columbia.edu; Christoffer Dall Subject: Re: [RFC PATCH v6 04/20] iommu/arm-smmu: add capability IOMMU_CAP_INTR_REMAP On Mon, Jun 16, 2014 at 04:25:26PM +0100, Will Deacon wrote: Ok, thanks. In which case, I think this is really a combined property of the SMMU and the interrupt controller, so we might need some extra code so that the SMMU can check that the interrupt controller for the device is also capable of interrupt remapping. Right, that this is part of IOMMU code has more or less historic reasons on x86. Interrupt remapping is purely implemented in the IOMMU there, so on ARM some clue-code between interrupt controler and smmu is needed. Joerg ___ kvmarm mailing list kvm...@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ___ kvmarm mailing list kvm...@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC PATCH v6 04/20] iommu/arm-smmu: add capability IOMMU_CAP_INTR_REMAP
Thanks for the clarification Alex, That’s exactly my point, why are we relying on QEMU or something else to emulate the MSI space when we can directly give access to devices using ITS (of course with a small emulation code). This way we are also benefited from all ITS services like VCPU migration etc. What about non QEMU VFIO users, for example, if I wanted to use VFIO to assign a device to a user process I don't need to depend on QEMU. I thought this is one of the main goals of vfio to make it independent of hypervisors. Thanks, Tirumalesh. -Original Message- From: Alex Williamson [mailto:alex.william...@redhat.com] Sent: Thursday, June 26, 2014 12:00 PM To: Chalamarla, Tirumalesh Cc: Joerg Roedel; Will Deacon; kvm@vger.kernel.org; open list; stuart.yo...@freescale.com; io...@lists.linux-foundation.org; t...@virtualopensystems.com; kvm...@lists.cs.columbia.edu; moderated list:ARM SMMU DRIVER Subject: Re: [RFC PATCH v6 04/20] iommu/arm-smmu: add capability IOMMU_CAP_INTR_REMAP On Thu, 2014-06-26 at 18:41 +, Chalamarla, Tirumalesh wrote: Sorry there was a type, The question is: How is VFIO restricting software from writing to MSI/MSI-X vectors of the device. All interrupts are configured via ioctl, not MSI config space or the MSI-X vector table in MMIO space. VFIO protects the MSI config area by virtualizing it (you can't actually write the physical enable bit or address/data through VFIO). The MSI-X vector table is protected by preventing read, write, or mmap access to it. QEMU provides further virtualization above the basics provided by VFIO. We really can't guarantee that devices don't have backdoors to configure these though. See the realtek quirk in QEMU for an example of a device that has such a backdoor. That's why we require interrupt remapping, so that a device that does this can only hurt the guest, and require the user to opt-out if they feel they have a sufficiently trusted guest. Thanks, Alex -Original Message- From: Chalamarla, Tirumalesh Sent: Thursday, June 26, 2014 11:16 AM To: Chalamarla, Tirumalesh; Joerg Roedel; Will Deacon Cc: kvm@vger.kernel.org; open list; alex.william...@redhat.com; stuart.yo...@freescale.com; io...@lists.linux-foundation.org; t...@virtualopensystems.com; kvm...@lists.cs.columbia.edu; moderated list:ARM SMMU DRIVER Subject: RE: [RFC PATCH v6 04/20] iommu/arm-smmu: add capability IOMMU_CAP_INTR_REMAP When I say emulating ITS, I mean translating guest ITS commands to physical ITS commands and placing them in physical queue. Regards, Tirumalesh. -Original Message- From: kvmarm-boun...@lists.cs.columbia.edu [mailto:kvmarm-boun...@lists.cs.columbia.edu] On Behalf Of Chalamarla, Tirumalesh Sent: Thursday, June 26, 2014 11:08 AM To: Joerg Roedel; Will Deacon Cc: kvm@vger.kernel.org; open list; alex.william...@redhat.com; stuart.yo...@freescale.com; io...@lists.linux-foundation.org; t...@virtualopensystems.com; kvm...@lists.cs.columbia.edu; moderated list:ARM SMMU DRIVER Subject: RE: [RFC PATCH v6 04/20] iommu/arm-smmu: add capability IOMMU_CAP_INTR_REMAP Forgive me if this discussion is not relative here, but I thought it is. How is VFIO restricting devices from writing to MSI/MSI-X, Is all the vector area is mapped by VFIO to trap the accesses. I am asking this because we might need to emulate ITS somewhere either in KVM or VFIO to provide direct access to devices. And I don't see any mentions on that. I think this flag needs to be set by ITS emulation. Regards, Tirumalesh. -Original Message- From: kvmarm-boun...@lists.cs.columbia.edu [mailto:kvmarm-boun...@lists.cs.columbia.edu] On Behalf Of Joerg Roedel Sent: Monday, June 16, 2014 8:39 AM To: Will Deacon Cc: stuart.yo...@freescale.com; kvm@vger.kernel.org; open list; io...@lists.linux-foundation.org; alex.william...@redhat.com; moderated list:ARM SMMU DRIVER; t...@virtualopensystems.com; kvm...@lists.cs.columbia.edu; Christoffer Dall Subject: Re: [RFC PATCH v6 04/20] iommu/arm-smmu: add capability IOMMU_CAP_INTR_REMAP On Mon, Jun 16, 2014 at 04:25:26PM +0100, Will Deacon wrote: Ok, thanks. In which case, I think this is really a combined property of the SMMU and the interrupt controller, so we might need some extra code so that the SMMU can check that the interrupt controller for the device is also capable of interrupt remapping. Right, that this is part of IOMMU code has more or less historic reasons on x86. Interrupt remapping is purely implemented in the IOMMU there, so on ARM some clue-code between interrupt controler and smmu is needed. Joerg ___ kvmarm mailing list kvm...@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ___ kvmarm mailing list kvm
RE: [PATCH 04/14] arm/arm64: KVM: wrap 64 bit MMIO accesses with two 32 bit ones
-Original Message- From: kvmarm-boun...@lists.cs.columbia.edu [mailto:kvmarm-boun...@lists.cs.columbia.edu] On Behalf Of Andre Przywara Sent: Thursday, June 19, 2014 2:46 AM To: linux-arm-ker...@lists.infradead.org; kvm...@lists.cs.columbia.edu; kvm@vger.kernel.org Cc: christoffer.d...@linaro.org Subject: [PATCH 04/14] arm/arm64: KVM: wrap 64 bit MMIO accesses with two 32 bit ones Some GICv3 registers can and will be accessed as 64 bit registers. Currently the register handling code can only deal with 32 bit accesses, so we do two consecutive calls to cover this. Signed-off-by: Andre Przywara andre.przyw...@arm.com --- virt/kvm/arm/vgic.c | 48 +--- 1 file changed, 45 insertions(+), 3 deletions(-) diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c index 4c6b212..b3cf4c7 100644 --- a/virt/kvm/arm/vgic.c +++ b/virt/kvm/arm/vgic.c @@ -906,6 +906,48 @@ static bool vgic_validate_access(const struct vgic_dist *dist, } /* + * Call the respective handler function for the given range. + * We split up any 64 bit accesses into two consecutive 32 bit + * handler calls and merge the result afterwards. + */ +static bool call_range_handler(struct kvm_vcpu *vcpu, + struct kvm_exit_mmio *mmio, + unsigned long offset, + const struct mmio_range *range) { + u32 *data32 = (void *)mmio-data; + struct kvm_exit_mmio mmio32; + bool ret; + + if (likely(mmio-len = 4)) + return range-handle_mmio(vcpu, mmio, offset); + + /* +* We assume that any access greater than 4 bytes is actually +* 8 bytes long, caused by a 64-bit access +*/ + + mmio32.len = 4; + mmio32.is_write = mmio-is_write; + + mmio32.phys_addr = mmio-phys_addr + 4; + if (mmio-is_write) + *(u32 *)mmio32.data = data32[1]; + ret = range-handle_mmio(vcpu, mmio32, offset + 4); + if (!mmio-is_write) + data32[1] = *(u32 *)mmio32.data; + + mmio32.phys_addr = mmio-phys_addr; + if (mmio-is_write) + *(u32 *)mmio32.data = data32[0]; + ret |= range-handle_mmio(vcpu, mmio32, offset); + if (!mmio-is_write) + data32[0] = *(u32 *)mmio32.data; + + return ret; +} Any reason to use two 32 bits instead of one 64 bit. AArch32 on ARMv8 may be. + +/* * vgic_handle_mmio_range - handle an in-kernel MMIO access * @vcpu: pointer to the vcpu performing the access * @run: pointer to the kvm_run structure @@ -936,10 +978,10 @@ static bool vgic_handle_mmio_range(struct kvm_vcpu *vcpu, struct kvm_run *run, spin_lock(vcpu-kvm-arch.vgic.lock); offset -= range-base; if (vgic_validate_access(dist, range, offset)) { - updated_state = range-handle_mmio(vcpu, mmio, offset); + updated_state = call_range_handler(vcpu, mmio, offset, range); } else { - vgic_reg_access(mmio, NULL, offset, - ACCESS_READ_RAZ | ACCESS_WRITE_IGNORED); + if (!mmio-is_write) + memset(mmio-data, 0, mmio-len); What is the use of this memset. updated_state = false; } spin_unlock(vcpu-kvm-arch.vgic.lock); -- 1.7.9.5 ___ kvmarm mailing list kvm...@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 08/14] arm/arm64: KVM: refactor MMIO accessors
-Original Message- From: kvmarm-boun...@lists.cs.columbia.edu [mailto:kvmarm-boun...@lists.cs.columbia.edu] On Behalf Of Andre Przywara Sent: Thursday, June 19, 2014 2:46 AM To: linux-arm-ker...@lists.infradead.org; kvm...@lists.cs.columbia.edu; kvm@vger.kernel.org Cc: christoffer.d...@linaro.org Subject: [PATCH 08/14] arm/arm64: KVM: refactor MMIO accessors The MMIO accessors for GICD_I[CS]ENABLER, GICD_I[CS]PENDR and GICD_ICFGR behave very similiar in GICv3, although the way the affected vCPU is determined differs. Factor out a generic, backend-facing implementation and use small wrappers in the current GICv2 emulation to ease code sharing later. Signed-off-by: Andre Przywara andre.przyw...@arm.com --- virt/kvm/arm/vgic.c | 93 --- 1 file changed, 52 insertions(+), 41 deletions(-) diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c index 2de58b3..2a59dff 100644 --- a/virt/kvm/arm/vgic.c +++ b/virt/kvm/arm/vgic.c @@ -398,35 +398,54 @@ static bool handle_mmio_raz_wi(struct kvm_vcpu *vcpu, return false; } -static bool handle_mmio_set_enable_reg(struct kvm_vcpu *vcpu, - struct kvm_exit_mmio *mmio, - phys_addr_t offset) +static bool vgic_handle_enable_reg(struct kvm *kvm, struct kvm_exit_mmio *mmio, + phys_addr_t offset, int vcpu_id, int access) { - u32 *reg = vgic_bitmap_get_reg(vcpu-kvm-arch.vgic.irq_enabled, - vcpu-vcpu_id, offset); - vgic_reg_access(mmio, reg, offset, - ACCESS_READ_VALUE | ACCESS_WRITE_SETBIT); + u32 *reg; + int mode = ACCESS_READ_VALUE | access; + struct kvm_vcpu *target_vcpu = kvm_get_vcpu(kvm, vcpu_id); + + reg = vgic_bitmap_get_reg(kvm-arch.vgic.irq_enabled, vcpu_id, offset); + vgic_reg_access(mmio, reg, offset, mode); if (mmio-is_write) { - vgic_update_state(vcpu-kvm); + if (access ACCESS_WRITE_CLEARBIT) { + if (offset 4) /* Force SGI enabled */ + *reg |= 0x; + vgic_retire_disabled_irqs(target_vcpu); + } + vgic_update_state(kvm); return true; } return false; } +static bool handle_mmio_set_enable_reg(struct kvm_vcpu *vcpu, + struct kvm_exit_mmio *mmio, + phys_addr_t offset) +{ + return vgic_handle_enable_reg(vcpu-kvm, mmio, offset, + vcpu-vcpu_id, ACCESS_WRITE_SETBIT); } + static bool handle_mmio_clear_enable_reg(struct kvm_vcpu *vcpu, struct kvm_exit_mmio *mmio, phys_addr_t offset) { - u32 *reg = vgic_bitmap_get_reg(vcpu-kvm-arch.vgic.irq_enabled, - vcpu-vcpu_id, offset); - vgic_reg_access(mmio, reg, offset, - ACCESS_READ_VALUE | ACCESS_WRITE_CLEARBIT); + return vgic_handle_enable_reg(vcpu-kvm, mmio, offset, + vcpu-vcpu_id, ACCESS_WRITE_CLEARBIT); } + +static bool vgic_handle_pending_reg(struct kvm *kvm, struct kvm_exit_mmio *mmio, + phys_addr_t offset, int vcpu_id, int access) { + u32 *reg; + int mode = ACCESS_READ_VALUE | access; + + reg = vgic_bitmap_get_reg(kvm-arch.vgic.irq_state, vcpu_id, offset); + vgic_reg_access(mmio, reg, offset, mode); if (mmio-is_write) { - if (offset 4) /* Force SGI enabled */ - *reg |= 0x; - vgic_retire_disabled_irqs(vcpu); - vgic_update_state(vcpu-kvm); + vgic_update_state(kvm); return true; } @@ -437,31 +456,16 @@ static bool handle_mmio_set_pending_reg(struct kvm_vcpu *vcpu, struct kvm_exit_mmio *mmio, phys_addr_t offset) { - u32 *reg = vgic_bitmap_get_reg(vcpu-kvm-arch.vgic.irq_state, - vcpu-vcpu_id, offset); - vgic_reg_access(mmio, reg, offset, - ACCESS_READ_VALUE | ACCESS_WRITE_SETBIT); - if (mmio-is_write) { - vgic_update_state(vcpu-kvm); - return true; - } - - return false; + return vgic_handle_pending_reg(vcpu-kvm, mmio, offset, + vcpu-vcpu_id, ACCESS_WRITE_SETBIT); } static bool handle_mmio_clear_pending_reg(struct kvm_vcpu *vcpu, struct kvm_exit_mmio *mmio, phys_addr_t offset) { - u32 *reg = vgic_bitmap_get_reg(vcpu-kvm-arch.vgic.irq_state, -
RE: [PATCH 11/14] arm/arm64: KVM: add virtual GICv3 distributor emulation
Again what is the need for vgic-v3.emul.c when we have vgic-v3.c ? -Original Message- From: kvmarm-boun...@lists.cs.columbia.edu [mailto:kvmarm-boun...@lists.cs.columbia.edu] On Behalf Of Andre Przywara Sent: Thursday, June 19, 2014 2:46 AM To: linux-arm-ker...@lists.infradead.org; kvm...@lists.cs.columbia.edu; kvm@vger.kernel.org Cc: christoffer.d...@linaro.org Subject: [PATCH 11/14] arm/arm64: KVM: add virtual GICv3 distributor emulation With everything separated and prepared, we implement a model of a GICv3 distributor and redistributors by using the existing framework to provide handler functions for each register group. Currently we limit the emulation to a model enforcing a single security state, with SRE==1 (forcing system register access) and ARE==1 (allowing more than 8 VCPUs). We share some of functions provided for GICv2 emulation, but take the different ways of addressing (v)CPUs into account. Save and restore is currently not implemented. Similar to the split-off GICv2 specific code, the new emulation code goes into a new file (vgic-v3-emul.c). Signed-off-by: Andre Przywara andre.przyw...@arm.com --- arch/arm64/kvm/Makefile|1 + include/kvm/arm_vgic.h | 11 +- include/linux/irqchip/arm-gic-v3.h | 26 ++ include/linux/kvm_host.h |1 + include/uapi/linux/kvm.h |1 + virt/kvm/arm/vgic-v3-emul.c| 895 virt/kvm/arm/vgic.c| 11 +- virt/kvm/arm/vgic.h|3 + virt/kvm/kvm_main.c|3 + 9 files changed, 949 insertions(+), 3 deletions(-) create mode 100644 virt/kvm/arm/vgic-v3-emul.c diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile index f241db6..2fba3a5 100644 --- a/arch/arm64/kvm/Makefile +++ b/arch/arm64/kvm/Makefile @@ -21,6 +21,7 @@ kvm-$(CONFIG_KVM_ARM_HOST) += guest.o reset.o sys_regs.o sys_regs_generic_v8.o kvm-$(CONFIG_KVM_ARM_VGIC) += $(KVM)/arm/vgic.o kvm-$(CONFIG_KVM_ARM_VGIC) += $(KVM)/arm/vgic-v2-emul.o +kvm-$(CONFIG_KVM_ARM_VGIC) += $(KVM)/arm/vgic-v3-emul.o kvm-$(CONFIG_KVM_ARM_VGIC) += $(KVM)/arm/vgic-v2.o kvm-$(CONFIG_KVM_ARM_VGIC) += vgic-v2-switch.o kvm-$(CONFIG_KVM_ARM_VGIC) += $(KVM)/arm/vgic-v3.o diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h index 8aa8482..3b164ee 100644 --- a/include/kvm/arm_vgic.h +++ b/include/kvm/arm_vgic.h @@ -151,7 +151,11 @@ struct vgic_dist { /* Distributor and vcpu interface mapping in the guest */ phys_addr_t vgic_dist_base; - phys_addr_t vgic_cpu_base; + /* GICv2 and GICv3 use different mapped register blocks */ + union { + phys_addr_t vgic_cpu_base; + phys_addr_t vgic_redist_base; + }; /* Distributor enabled */ u32 enabled; @@ -176,6 +180,10 @@ struct vgic_dist { /* Target CPU for each IRQ */ u8 *irq_spi_cpu; + + /* Target MPIDR for each IRQ (needed for GICv3 IROUTERn) only */ + u32 *irq_spi_mpidr; + struct vgic_bitmap *irq_spi_target; /* Bitmap indicating which CPU has something pending */ @@ -253,6 +261,7 @@ void kvm_vgic_flush_hwstate(struct kvm_vcpu *vcpu); void kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu); int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int irq_num, bool level); +void vgic_v3_dispatch_sgi(struct kvm_vcpu *vcpu, u64 reg); int kvm_vgic_vcpu_pending_irq(struct kvm_vcpu *vcpu); bool vgic_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run, struct kvm_exit_mmio *mmio); diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h index 9eac712..9f17e57 100644 --- a/include/linux/irqchip/arm-gic-v3.h +++ b/include/linux/irqchip/arm-gic-v3.h @@ -31,6 +31,7 @@ #define GICD_SETSPI_SR 0x0050 #define GICD_CLRSPI_SR 0x0058 #define GICD_SEIR 0x0068 +#define GICD_IGROUPR 0x0080 #define GICD_ISENABLER 0x0100 #define GICD_ICENABLER 0x0180 #define GICD_ISPENDR 0x0200 @@ -39,19 +40,38 @@ #define GICD_ICACTIVER 0x0380 #define GICD_IPRIORITYR0x0400 #define GICD_ICFGR 0x0C00 +#define GICD_IGRPMODR 0x0D00 +#define GICD_NSACR 0x0E00 #define GICD_IROUTER 0x6000 +#define GICD_IDREGS0xFFD0 #define GICD_PIDR2 0xFFE8 +/* + * Non-ARE distributor registers, needed to provide the RES0 + * semantics for KVM's emulated GICv3 + */ +#define GICD_ITARGETSR 0x0800 +#define GICD_SGIR 0x0F00 +#define GICD_CPENDSGIR 0x0F10 +#define GICD_SPENDSGIR 0x0F20 + +
RE: [PATCH 13/14] arm/arm64: KVM: enable kernel side of GICv3 emulation
-Original Message- From: kvmarm-boun...@lists.cs.columbia.edu [mailto:kvmarm-boun...@lists.cs.columbia.edu] On Behalf Of Andre Przywara Sent: Thursday, June 19, 2014 2:46 AM To: linux-arm-ker...@lists.infradead.org; kvm...@lists.cs.columbia.edu; kvm@vger.kernel.org Cc: christoffer.d...@linaro.org Subject: [PATCH 13/14] arm/arm64: KVM: enable kernel side of GICv3 emulation With all the necessary GICv3 emulation code in place, we can now connect the code to the GICv3 backend in the kernel. The LR register handling is different depending on the emulated GIC model, so provide different implementations for each. Also allow non-v2-compatible GICv3 implementations (which don't provide MMIO regions for the virtual CPU interface in the DT), but restrict those hosts to use GICv3 guests only. Signed-off-by: Andre Przywara andre.przyw...@arm.com --- virt/kvm/arm/vgic-v3.c | 138 ++-- virt/kvm/arm/vgic.c|2 + 2 files changed, 112 insertions(+), 28 deletions(-) diff --git a/virt/kvm/arm/vgic-v3.c b/virt/kvm/arm/vgic-v3.c index 7d9c85e..d26d12f 100644 --- a/virt/kvm/arm/vgic-v3.c +++ b/virt/kvm/arm/vgic-v3.c @@ -34,6 +34,7 @@ #define GICH_LR_VIRTUALID (0x3ffUL 0) #define GICH_LR_PHYSID_CPUID_SHIFT (10) #define GICH_LR_PHYSID_CPUID (7UL GICH_LR_PHYSID_CPUID_SHIFT) +#define ICH_LR_VIRTUALID_MASK (BIT_ULL(32) - 1) /* * LRs are stored in reverse order in memory. make sure we index them @@ -43,7 +44,35 @@ static u32 ich_vtr_el2; -static struct vgic_lr vgic_v3_get_lr(const struct kvm_vcpu *vcpu, int lr) +static u64 sync_lr_val(u8 state) +{ + u64 lr_val = 0; + + if (state LR_STATE_PENDING) + lr_val |= ICH_LR_PENDING_BIT; + if (state LR_STATE_ACTIVE) + lr_val |= ICH_LR_ACTIVE_BIT; + if (state LR_EOI_INT) + lr_val |= ICH_LR_EOI; + + return lr_val; +} + +static u8 sync_lr_state(u64 lr_val) +{ + u8 state = 0; + + if (lr_val ICH_LR_PENDING_BIT) + state |= LR_STATE_PENDING; + if (lr_val ICH_LR_ACTIVE_BIT) + state |= LR_STATE_ACTIVE; + if (lr_val ICH_LR_EOI) + state |= LR_EOI_INT; + + return state; +} + +static struct vgic_lr vgic_v2_on_v3_get_lr(const struct kvm_vcpu *vcpu, +int lr) { struct vgic_lr lr_desc; u64 val = vcpu-arch.vgic_cpu.vgic_v3.vgic_lr[LR_INDEX(lr)]; @@ -53,30 +82,53 @@ static struct vgic_lr vgic_v3_get_lr(const struct kvm_vcpu *vcpu, int lr) lr_desc.source = (val GICH_LR_PHYSID_CPUID_SHIFT) 0x7; else lr_desc.source = 0; - lr_desc.state = 0; + lr_desc.state = sync_lr_state(val); - if (val ICH_LR_PENDING_BIT) - lr_desc.state |= LR_STATE_PENDING; - if (val ICH_LR_ACTIVE_BIT) - lr_desc.state |= LR_STATE_ACTIVE; - if (val ICH_LR_EOI) - lr_desc.state |= LR_EOI_INT; + return lr_desc; +} + +static struct vgic_lr vgic_v3_on_v3_get_lr(const struct kvm_vcpu *vcpu, +int lr) { + struct vgic_lr lr_desc; + u64 val = vcpu-arch.vgic_cpu.vgic_v3.vgic_lr[LR_INDEX(lr)]; + + lr_desc.irq = val ICH_LR_VIRTUALID_MASK; + lr_desc.source = 0; + lr_desc.state = sync_lr_state(val); return lr_desc; } -static void vgic_v3_set_lr(struct kvm_vcpu *vcpu, int lr, - struct vgic_lr lr_desc) +static void vgic_v3_on_v3_set_lr(struct kvm_vcpu *vcpu, int lr, +struct vgic_lr lr_desc) { - u64 lr_val = (((u32)lr_desc.source GICH_LR_PHYSID_CPUID_SHIFT) | - lr_desc.irq); + u64 lr_val; - if (lr_desc.state LR_STATE_PENDING) - lr_val |= ICH_LR_PENDING_BIT; - if (lr_desc.state LR_STATE_ACTIVE) - lr_val |= ICH_LR_ACTIVE_BIT; - if (lr_desc.state LR_EOI_INT) - lr_val |= ICH_LR_EOI; + lr_val = lr_desc.irq; + + /* +* currently all guest IRQs are Group1, as Group0 would result +* in a FIQ in the guest, which it wouldn't expect. +* Eventually we want to make this configurable, so we may revisit +* this in the future. +*/ + lr_val |= ICH_LR_GROUP; + + lr_val |= sync_lr_val(lr_desc.state); + + vcpu-arch.vgic_cpu.vgic_v3.vgic_lr[LR_INDEX(lr)] = lr_val; } + +static void vgic_v2_on_v3_set_lr(struct kvm_vcpu *vcpu, int lr, +struct vgic_lr lr_desc) +{ + u64 lr_val; + + lr_val = lr_desc.irq; + + lr_val |= (u32)lr_desc.source GICH_LR_PHYSID_CPUID_SHIFT; + + lr_val |= sync_lr_val(lr_desc.state); vcpu-arch.vgic_cpu.vgic_v3.vgic_lr[LR_INDEX(lr)] = lr_val; } @@ -145,9 +197,8 @@ static void vgic_v3_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcrp) static void vgic_v3_enable(struct kvm_vcpu *vcpu) { - struct