Re: [PATCH] arm64/kvm: Add generic v8 KVM target

2015-07-17 Thread Chalamarla, Tirumalesh

 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

2015-06-29 Thread Chalamarla, Tirumalesh

 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

2015-06-29 Thread Chalamarla, Tirumalesh

 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

2014-06-27 Thread Chalamarla, Tirumalesh
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

2014-06-26 Thread Chalamarla, Tirumalesh
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

2014-06-26 Thread Chalamarla, Tirumalesh
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

2014-06-26 Thread Chalamarla, Tirumalesh
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

2014-06-26 Thread Chalamarla, Tirumalesh
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

2014-06-19 Thread Chalamarla, Tirumalesh


-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

2014-06-19 Thread Chalamarla, Tirumalesh


-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

2014-06-19 Thread Chalamarla, Tirumalesh
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

2014-06-19 Thread Chalamarla, Tirumalesh


-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