Skip to contentSkip to conversation

KVM: arm64: Implement API for vGICv3 live migration

PATCH In review Pavel Fedin 3 participants 17 messages Dec 7, 2015 – Apr 21, 2016

Subsystems (5)
Documentation KVM arm64 KVM arm64 +1 more subsystems
lore
Share & tools
lore
#0 lore raw

v7 cover KVM: arm64: Implement API for vGICv3 live migration

This patchset adds necessary userspace API in order to support vGICv3 live migration. GICv3 registers are accessed using device attribute ioctls, similar to GICv2.

v6 => v7:
- Rebased on top of linux-next of 07.12.2015, thrown away unnecessary part

v5 => v6:
- Rebased on top of linux-next of 23.11.2015
- Use original API documentation patch, with minor changes only.
- Quit reusing KVM_DEV_ARM_VGIC_CPUID_MASK, do not touch vGICv2 API at all.
- Fixed some issues reported by the new checkpatch

v4 => v5:
- Adapted to new API by Peter Maydell, Marc Zyngier and Christoffer Dall. Acked-by's on the documentation were dropped, just in case, because i slightly adjusted it. Additionally, i merged all doc updates into one patch.

v3 => v4:
- Split pure refactoring from anything else
- Documentation brought up to date
- Cleaned up 'mmio' structure usage in vgic_attr_regs_access(), use call_range_handler() for 64-bit access handling
- Rebased on new linux-next

v2 => v3:
- KVM_DEV_ARM_VGIC_CPUID_MASK enlarged to 20 bits, allowing more than 256 CPUs.
- Bug fix: Correctly set mmio->private, necessary for redistributor access.
- Added accessors for ICC_AP0R and ICC_AP1R registers
- Rebased on new linux-next

v1 => v2:
- Do not use generic register get/set API for CPU interface, use only device attributes.
- Introduce size specifier for distributor and redistributor register accesses, do not assume size any more.
- Lots of refactor and reusable code extraction.
- Added forgotten documentation

Christoffer Dall (1):
KVM: arm/arm64: Add VGICv3 save/restore API documentation

Pavel Fedin (5):
KVM: arm/arm64: Move endianness conversion out of
vgic_attr_regs_access()
KVM: arm/arm64: Refactor vGIC attributes handling code
KVM: arm64: Implement vGICv3 distributor and redistributor access from
userspace
KVM: arm64: Introduce find_reg_by_id()
KVM: arm64: Implement vGICv3 CPU interface access

 Documentation/virtual/kvm/devices/arm-vgic-v3.txt | 116 ++++++++
 Documentation/virtual/kvm/devices/arm-vgic.txt    |  21 +-
 arch/arm64/include/uapi/asm/kvm.h                 |  15 +-
 arch/arm64/kvm/sys_regs.c                         |  22 +-
 arch/arm64/kvm/sys_regs.h                         |   4 +
 arch/arm64/mm/mmap.c                              |   2 +-
 include/linux/irqchip/arm-gic-v3.h                |  19 +-
 virt/kvm/arm/vgic-v2-emul.c                       | 124 ++------
 virt/kvm/arm/vgic-v3-emul.c                       | 334 +++++++++++++++++++++-
 virt/kvm/arm/vgic.c                               |  57 ++++
 virt/kvm/arm/vgic.h                               |   3 +
 11 files changed, 577 insertions(+), 140 deletions(-)
 create mode 100644 Documentation/virtual/kvm/devices/arm-vgic-v3.txt
Signature · 1 line
2.4.4
#1 lore raw

PATCH v7 1/6 KVM: arm/arm64: Add VGICv3 save/restore API documentation

From: Christoffer Dall <[email protected]>

Factor out the GICv3-specific documentation into a separate documentation file. Add description for how to access distributor, redistributor, and CPU interface registers for GICv3 in this new file.

---
 Documentation/virtual/kvm/devices/arm-vgic-v3.txt | 116 ++++++++++++++++++++++
 Documentation/virtual/kvm/devices/arm-vgic.txt    |  21 +---
 2 files changed, 120 insertions(+), 17 deletions(-)
 create mode 100644 Documentation/virtual/kvm/devices/arm-vgic-v3.txt
Documentation/virtual/kvm/devices/arm-vgic-v3.txt new file +116 -0
new file mode 100644
index 0000000..24e2f6b
@@ -0,0 +1,116 @@
+ARM Virtual Generic Interrupt Controller v3 and later (VGICv3)
+==============================================================
+
+
+Device types supported:
+  KVM_DEV_TYPE_ARM_VGIC_V3     ARM Generic Interrupt Controller v3.0
+
+Only one VGIC instance may be instantiated through this API.  The created VGIC
+will act as the VM interrupt controller, requiring emulated user-space devices
+to inject interrupts to the VGIC instead of directly to CPUs.  It is not
+possible to create both a GICv3 and GICv2 on the same VM.
+
+Creating a guest GICv3 device requires a host GICv3 as well.
+
+Groups:
+  KVM_DEV_ARM_VGIC_GRP_ADDR
+  Attributes:
+    KVM_VGIC_V3_ADDR_TYPE_DIST (rw, 64-bit)
+      Base address in the guest physical address space of the GICv3 distributor
+      register mappings. Only valid for KVM_DEV_TYPE_ARM_VGIC_V3.
+      This address needs to be 64K aligned and the region covers 64 KByte.
+
+    KVM_VGIC_V3_ADDR_TYPE_REDIST (rw, 64-bit)
+      Base address in the guest physical address space of the GICv3
+      redistributor register mappings. There are two 64K pages for each
+      VCPU and all of the redistributor pages are contiguous.
+      Only valid for KVM_DEV_TYPE_ARM_VGIC_V3.
+      This address needs to be 64K aligned.
+
+
+  KVM_DEV_ARM_VGIC_GRP_DIST_REGS
+  KVM_DEV_ARM_VGIC_GRP_REDIST_REGS
+  Attributes:
+    The attr field of kvm_device_attr encodes two values:
+    bits:     | 63   ....  32  |  31   ....    0 |
+    values:   |      mpidr     |      offset     |
+
+    All distributor regs are (rw, 64-bit).
+
+    KVM_DEV_ARM_VGIC_GRP_DIST_REGS accesses the main distributor registers.
+    KVM_DEV_ARM_VGIC_GRP_REDIST_REGS accesses the redistributor of the CPU
+    specified by the mpidr.
+
+    The offset is relative to the "[Re]Distributor base address" as defined
+    in the GICv3/4 specs.  Getting or setting such a register has the same
+    effect as reading or writing the register on real hardware, and the mpidr
+    field is used to specify which redistributor is accessed.  The mpidr is
+    ignored for the distributor.
+
+    The mpidr encoding is based on the affinity information in the
+    architecture defined MPIDR, and the field is encoded as follows:
+      | 63 .... 56 | 55 .... 48 | 47 .... 40 | 39 .... 32 |
+      |    Aff3    |    Aff2    |    Aff1    |    Aff0    |
+
+    Note that distributor fields are not banked, but return the same value
+    regardless of the mpidr used to access the register.
+  Limitations:
+    - Priorities are not implemented, and registers are RAZ/WI
+  Errors:
+    -ENXIO: Getting or setting this register is not yet supported
+    -EBUSY: One or more VCPUs are running
+
+
+  KVM_DEV_ARM_VGIC_CPU_SYSREGS
+  Attributes:
+    The attr field of kvm_device_attr encodes two values:
+    bits:     | 63      ....       32 | 31  ....  16 | 15  ....  0 |
+    values:   |         mpidr         |      RES     |    instr    |
+
+    The mpidr field encodes the CPU ID based on the affinity information in the
+    architecture defined MPIDR, and the field is encoded as follows:
+      | 63 .... 56 | 55 .... 48 | 47 .... 40 | 39 .... 32 |
+      |    Aff3    |    Aff2    |    Aff1    |    Aff0   |
+    KVM_DEV_ARM_VGIC_SYSREG() macro is provided for building register ID.
+
+    The instr field encodes the system register to access based on the fields
+    defined in the A64 instruction set encoding for system register access
+    (RES means the bits are reserved for future use and should be zero):
+
+      | 15 ... 14 | 13 ... 11 | 10 ... 7 | 6 ... 3 | 2 ... 0 |
+      |   Op 0    |    Op1    |    CRn   |   CRm   |   Op2   |
+
+    All system regs accessed through this API are (rw, 64-bit).
+
+    KVM_DEV_ARM_VGIC_CPU_SYSREGS accesses the CPU interface registers for the
+    CPU specified by the mpidr field.
+
+
+  Limitations:
+    - Priorities are not implemented, and registers are RAZ/WI
+  Errors:
+    -ENXIO: Getting or setting this register is not yet supported
+    -EBUSY: VCPU is running
+    -EINVAL: Invalid mpidr supplied
+
+
+  KVM_DEV_ARM_VGIC_GRP_NR_IRQS
+  Attributes:
+    A value describing the number of interrupts (SGI, PPI and SPI) for
+    this GIC instance, ranging from 64 to 1024, in increments of 32.
+
+  Errors:
+    -EINVAL: Value set is out of the expected range
+    -EBUSY: Value has already be set.
+
+
+  KVM_DEV_ARM_VGIC_GRP_CTRL
+  Attributes:
+    KVM_DEV_ARM_VGIC_CTRL_INIT
+      request the initialization of the VGIC, no additional parameter in
+      kvm_device_attr.addr.
+  Errors:
+    -ENXIO: VGIC not properly configured as required prior to calling
+     this attribute
+    -ENODEV: no online VCPU
+    -ENOMEM: memory shortage when allocating vgic internal data
Documentation/virtual/kvm/devices/arm-vgic.txt +4 -17
index 59541d4..257b854 100644
@@ -3,16 +3,16 @@ ARM Virtual Generic Interrupt Controller (VGIC)
 
 Device types supported:
   KVM_DEV_TYPE_ARM_VGIC_V2     ARM Generic Interrupt Controller v2.0
-  KVM_DEV_TYPE_ARM_VGIC_V3     ARM Generic Interrupt Controller v3.0
 
 Only one VGIC instance may be instantiated through either this API or the
 legacy KVM_CREATE_IRQCHIP api.  The created VGIC will act as the VM interrupt
 controller, requiring emulated user-space devices to inject interrupts to the
 VGIC instead of directly to CPUs.
 
-Creating a guest GICv3 device requires a host GICv3 as well.
-GICv3 implementations with hardware compatibility support allow a guest GICv2
-as well.
+GICv3 implementations with hardware compatibility support allow creating a
+guest GICv2 through this interface.  For information on creating a guest GICv3
+device, see arm-vgic-v3.txt.  It is not possible to create both a GICv3 and
+GICv2 device on the same VM.
 
 Groups:
   KVM_DEV_ARM_VGIC_GRP_ADDR
@@ -27,19 +27,6 @@ Groups:
       interface register mappings. Only valid for KVM_DEV_TYPE_ARM_VGIC_V2.
       This address needs to be 4K aligned and the region covers 4 KByte.
 
-    KVM_VGIC_V3_ADDR_TYPE_DIST (rw, 64-bit)
-      Base address in the guest physical address space of the GICv3 distributor
-      register mappings. Only valid for KVM_DEV_TYPE_ARM_VGIC_V3.
-      This address needs to be 64K aligned and the region covers 64 KByte.
-
-    KVM_VGIC_V3_ADDR_TYPE_REDIST (rw, 64-bit)
-      Base address in the guest physical address space of the GICv3
-      redistributor register mappings. There are two 64K pages for each
-      VCPU and all of the redistributor pages are contiguous.
-      Only valid for KVM_DEV_TYPE_ARM_VGIC_V3.
-      This address needs to be 64K aligned.
-
-
   KVM_DEV_ARM_VGIC_GRP_DIST_REGS
   Attributes:
     The attr field of kvm_device_attr encodes two values:
Signature · 1 line
2.4.4
#2 lore raw

Re: [PATCH v7 1/6] KVM: arm/arm64: Add VGICv3 save/restore API documentation

Quoted 10 lines · Pavel Fedin

From: Christoffer Dall <[email protected]>

Factor out the GICv3-specific documentation into a separate documentation file. Add description for how to access distributor, redistributor, and CPU interface registers for GICv3 in this new file.

I was rereading this API doc this week, and I realised we missed something when we wrote it:

Documentation/virtual/kvm/devices/arm-vgic-v3.txt line 38

Quoted 8 lines
Documentation/virtual/kvm/devices/arm-vgic-v3.txt new file +8 -0
+  KVM_DEV_ARM_VGIC_GRP_DIST_REGS
+  KVM_DEV_ARM_VGIC_GRP_REDIST_REGS
+  Attributes:
+    The attr field of kvm_device_attr encodes two values:
+    bits:     | 63   ....  32  |  31   ....    0 |
+    values:   |      mpidr     |      offset     |
+
+    All distributor regs are (rw, 64-bit).

It's a bit odd to claim that distributor (or redistributor) registers are all 64-bit, because in the hardware most of them are really 32-bit, and at 32-bit offsets from each other. We didn't mean to imply that you could do a 64-bit access at offset 0 of the distributor and get both of GICD_CTLR and GICD_TYPER at once, for instance.

I'm not quite sure what we want to say in the documentation, though I think our general intent was clear:

* you access a particular register at its relevant offset, and you only get that register
* no support for reading half a register
* if the register (as documented in the GICv3 architecture specification) is less than 64 bits wide then the returned result is zero-extended to 64 bits on read, and high bits are ignored on write
(Only a couple of registers are really 64-bits, notably GICD_IROUTER<n>. The rest are 32-bits. We could probably explicitly list all the 64-bit regs in this doc if we didn't want to defer to the arch spec.)

Do we want to forbid accesses to registers which the architecture says can be byte-accessed, like GICD_IPRIORITYR<n>? I think we have to, because the kernel API we have here doesn't have any way to specify access width, and it would be weird for addresses X+1, X+2, X+3 to give you 8 bits of data when X+0 gave you 32.

What do we mean by the "rw" part? Some registers really are architecturally RO, so does this mean "writes to architecturally RO registers will succeed but be ignored rather than returning an errno" ?

Quoted 16 lines
Documentation/virtual/kvm/devices/arm-vgic-v3.txt new file +16 -0
+
+    KVM_DEV_ARM_VGIC_GRP_DIST_REGS accesses the main distributor registers.
+    KVM_DEV_ARM_VGIC_GRP_REDIST_REGS accesses the redistributor of the CPU
+    specified by the mpidr.
+
+    The offset is relative to the "[Re]Distributor base address" as defined
+    in the GICv3/4 specs.  Getting or setting such a register has the same
+    effect as reading or writing the register on real hardware, and the mpidr
+    field is used to specify which redistributor is accessed.  The mpidr is
+    ignored for the distributor.
+
+    The mpidr encoding is based on the affinity information in the
+    architecture defined MPIDR, and the field is encoded as follows:
+      | 63 .... 56 | 55 .... 48 | 47 .... 40 | 39 .... 32 |
+      |    Aff3    |    Aff2    |    Aff1    |    Aff0    |
+

Documentation/virtual/kvm/devices/arm-vgic-v3.txt line 64

Quoted 10 lines
Documentation/virtual/kvm/devices/arm-vgic-v3.txt new file +10 -0
+    Note that distributor fields are not banked, but return the same value
+    regardless of the mpidr used to access the register.
+  Limitations:
+    - Priorities are not implemented, and registers are RAZ/WI
+  Errors:
+    -ENXIO: Getting or setting this register is not yet supported
+    -EBUSY: One or more VCPUs are running
+
+
+  KVM_DEV_ARM_VGIC_CPU_SYSREGS

In contrast it's fine for sysregs to be all 64-bit, because they correspond to CPU system registers which are architecturally 64-bits.

thanks
-- PMM

#3 lore raw

Re: [PATCH v7 1/6] KVM: arm/arm64: Add VGICv3 save/restore API documentation

Quoted 24 lines · Peter Maydell
Quoted 10 lines · Pavel Fedin

From: Christoffer Dall <[email protected]>

Factor out the GICv3-specific documentation into a separate documentation file. Add description for how to access distributor, redistributor, and CPU interface registers for GICv3 in this new file.

I was rereading this API doc this week, and I realised we missed something when we wrote it:

Documentation/virtual/kvm/devices/arm-vgic-v3.txt line 38

Quoted 8 lines
Documentation/virtual/kvm/devices/arm-vgic-v3.txt new file +8 -0
+  KVM_DEV_ARM_VGIC_GRP_DIST_REGS
+  KVM_DEV_ARM_VGIC_GRP_REDIST_REGS
+  Attributes:
+    The attr field of kvm_device_attr encodes two values:
+    bits:     | 63   ....  32  |  31   ....    0 |
+    values:   |      mpidr     |      offset     |
+
+    All distributor regs are (rw, 64-bit).
Quoted 6 lines

It's a bit odd to claim that distributor (or redistributor) registers are all 64-bit, because in the hardware most of them are really 32-bit, and at 32-bit offsets from each other. We didn't mean to imply that you could do a 64-bit access at offset 0 of the distributor and get both of GICD_CTLR and GICD_TYPER at once, for instance.

no we didn't, and I think this was just an oversight on my side.

Perhaps what I meant was the userspace should just provide a pointer to a 64-bit value.

Quoted 16 lines

I'm not quite sure what we want to say in the documentation, though I think our general intent was clear:

* you access a particular register at its relevant offset, and you only get that register
* no support for reading half a register
* if the register (as documented in the GICv3 architecture specification) is less than 64 bits wide then the returned result is zero-extended to 64 bits on read, and high bits are ignored on write
(Only a couple of registers are really 64-bits, notably GICD_IROUTER<n>. The rest are 32-bits. We could probably explicitly list all the 64-bit regs in this doc if we didn't want to defer to the arch spec.)

Quoted 5 lines

Do we want to forbid accesses to registers which the architecture says can be byte-accessed, like GICD_IPRIORITYR<n>? I think we have to, because the kernel API we have here doesn't have any way to specify access width, and it would be weird for addresses X+1, X+2, X+3 to give you 8 bits of data when X+0 gave you 32.

yes, for the gicv2 API we only allow accesses on 32-bit aligned boundaries and always assume a 32-bit access.

Quoted 5 lines

What do we mean by the "rw" part? Some registers really are architecturally RO, so does this mean "writes to architecturally RO registers will succeed but be ignored rather than returning an errno" ?

I think my intention was that this would work like the invariant sysregs, where if you write anything else than what the kernel has defined, then you get an error. Not sure if this is something we'd want to do here though. Seems like the GICv2 implementation just ignores writes in line with the architecture.

Quoted 22 lines
Quoted 22 lines
Documentation/virtual/kvm/devices/arm-vgic-v3.txt new file +21 -0
+
+    KVM_DEV_ARM_VGIC_GRP_DIST_REGS accesses the main distributor registers.
+    KVM_DEV_ARM_VGIC_GRP_REDIST_REGS accesses the redistributor of the CPU
+    specified by the mpidr.
+
+    The offset is relative to the "[Re]Distributor base address" as defined
+    in the GICv3/4 specs.  Getting or setting such a register has the same
+    effect as reading or writing the register on real hardware, and the mpidr
+    field is used to specify which redistributor is accessed.  The mpidr is
+    ignored for the distributor.
+
+    The mpidr encoding is based on the affinity information in the
+    architecture defined MPIDR, and the field is encoded as follows:
+      | 63 .... 56 | 55 .... 48 | 47 .... 40 | 39 .... 32 |
+      |    Aff3    |    Aff2    |    Aff1    |    Aff0    |
+
+    Note that distributor fields are not banked, but return the same value
+    regardless of the mpidr used to access the register.
+  Limitations:
+    - Priorities are not implemented, and registers are RAZ/WI
+  Errors:
Quoted 9 lines

Documentation/virtual/kvm/devices/arm-vgic-v3.txt line 64

Quoted 5 lines
Documentation/virtual/kvm/devices/arm-vgic-v3.txt new file +5 -0
+    -ENXIO: Getting or setting this register is not yet supported
+    -EBUSY: One or more VCPUs are running
+
+
+  KVM_DEV_ARM_VGIC_CPU_SYSREGS

In contrast it's fine for sysregs to be all 64-bit, because they correspond to CPU system registers which are architecturally 64-bits.

Right, perhaps this was my confusion.

Thanks,
-Christoffer

#4 lore raw

Re: [PATCH v7 1/6] KVM: arm/arm64: Add VGICv3 save/restore API documentation

In reply to Pavel Fedin
Quoted 14 lines · Pavel Fedin

From: Christoffer Dall <[email protected]>

Factor out the GICv3-specific documentation into a separate documentation file. Add description for how to access distributor, redistributor, and CPU interface registers for GICv3 in this new file.

Documentation/virtual/kvm/devices/arm-vgic-v3.txt new file +8 -0
+  KVM_DEV_ARM_VGIC_GRP_DIST_REGS
+  KVM_DEV_ARM_VGIC_GRP_REDIST_REGS
+  Attributes:
+    The attr field of kvm_device_attr encodes two values:
+    bits:     | 63   ....  32  |  31   ....    0 |
+    values:   |      mpidr     |      offset     |
+
+    All distributor regs are (rw, 64-bit).

Documentation/virtual/kvm/devices/arm-vgic-v3.txt line 48

Quoted 10 lines
Documentation/virtual/kvm/devices/arm-vgic-v3.txt new file +10 -0
+
+    KVM_DEV_ARM_VGIC_GRP_DIST_REGS accesses the main distributor registers.
+    KVM_DEV_ARM_VGIC_GRP_REDIST_REGS accesses the redistributor of the CPU
+    specified by the mpidr.
+
+    The offset is relative to the "[Re]Distributor base address" as defined
+    in the GICv3/4 specs.  Getting or setting such a register has the same
+    effect as reading or writing the register on real hardware, and the mpidr
+    field is used to specify which redistributor is accessed.  The mpidr is
+    ignored for the distributor.

I've just noticed an awkward wrinkle here. The (optional) registers GICR_STATUSR and GICD_STATUSR are both architecturally "write-1-to-clear", with no mechanism in the hardware to write 1 bits back into them. So if we want to support migrating these state fields, the behaviour of the userspace accessor has to be something different from the effect of writing the register on real hardware (eg "writes actually write the specified value")...

thanks
-- PMM

#5 lore raw

Re: [PATCH v7 1/6] KVM: arm/arm64: Add VGICv3 save/restore API documentation

In reply to Pavel Fedin
Quoted 10 lines · Pavel Fedin

From: Christoffer Dall <[email protected]>

Factor out the GICv3-specific documentation into a separate documentation file. Add description for how to access distributor, redistributor, and CPU interface registers for GICv3 in this new file.

Nothing in here describes a mechanism for reading or writing the current interrupt line_level state from the kernel (which doesn't matter for edge triggered interrupts but does for level triggered interrupts). Do we need accessors for these or does somebody have a good rationale for why we don't need to migrate that data? (Christoffer?)

(GICv2 doesn't do this either and I suspect it's wrong too.)

thanks
-- PMM

#6 lore raw

Re: [PATCH v7 1/6] KVM: arm/arm64: Add VGICv3 save/restore API documentation

Quoted 6 lines · Peter Maydell

Nothing in here describes a mechanism for reading or writing the current interrupt line_level state from the kernel (which doesn't matter for edge triggered interrupts but does for level triggered interrupts). Do we need accessors for these or does somebody have a good rationale for why we don't need to migrate that data? (Christoffer?)

Relatedly, we should have a mechanism for directly reading and writing the pending-latch state in the KVM GIC, which for a level triggered interrupt is not the same as the behaviour of reading and writing the PENDING register.

thanks
-- PMM

#7 lore raw

Re: [PATCH v7 1/6] KVM: arm/arm64: Add VGICv3 save/restore API documentation

Quoted 9 lines · Peter Maydell
Quoted 6 lines · On 15 April 2016 at 14:58, Peter Maydell <[email protected]…

Nothing in here describes a mechanism for reading or writing the current interrupt line_level state from the kernel (which doesn't matter for edge triggered interrupts but does for level triggered interrupts). Do we need accessors for these or does somebody have a good rationale for why we don't need to migrate that data? (Christoffer?)

Quoted 6 lines

Relatedly, we should have a mechanism for directly reading and writing the pending-latch state in the KVM GIC, which for a level triggered interrupt is not the same as the behaviour of reading and writing the PENDING register.

Strictly speaking, yes. But can we think of a use case that would really need this?

(I saw this as a thing that was implemented for software to capture the state of the gic and restore it, but since that's what we're doing here, it seems redundant)

Thanks,
-Christoffer

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

#8 lore raw

Re: [PATCH v7 1/6] KVM: arm/arm64: Add VGICv3 save/restore API documentation

In reply to Peter Maydell
Quoted 14 lines · Peter Maydell
Quoted 9 lines · On 7 December 2015 at 12:29, Pavel Fedin <[email protected] <javascript:;>>…

From: Christoffer Dall <[email protected] <javascript:;>>

Factor out the GICv3-specific documentation into a separate documentation file. Add description for how to access distributor, redistributor, and CPU interface registers for GICv3 in this new file.

Acked-by Peter Maydell <[email protected] <javascript:;>>
Acked-by Marc Zyngier <[email protected] <javascript:;>>

<javascript:;>>

Quoted 1 line
Signed-off-by Pavel Fedin <[email protected] <javascript:;>>
Quoted 10 lines

Nothing in here describes a mechanism for reading or writing the current interrupt line_level state from the kernel (which doesn't matter for edge triggered interrupts but does for level triggered interrupts). Do we need accessors for these or does somebody have a good rationale for why we don't need to migrate that data? (Christoffer?)

(GICv2 doesn't do this either and I suspect it's wrong too.)

I thought we relied on user space to migrate its device state including the interrupt line output state and set the corresponding line state to the kernel, but that may have been wrong, and would rely on userspace to call the IRQ_LINE ioctl again. Would that work?

How is it again, does QEMU preserve the IRQ output line state per device?

This may just work currently because we rely on the reads/writes to SPENDR to preserve the state and apparently level triggered interrupts are reinjected as needed.

Thanks,
-Christoffer

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

#9 lore raw

Re: [PATCH v7 1/6] KVM: arm/arm64: Add VGICv3 save/restore API documentation

Quoted 8 lines · Christoffer Dall
Quoted 6 lines · Peter Maydell

Nothing in here describes a mechanism for reading or writing the current interrupt line_level state from the kernel (which doesn't matter for edge triggered interrupts but does for level triggered interrupts). Do we need accessors for these or does somebody have a good rationale for why we don't need to migrate that data? (Christoffer?)

Quoted 4 lines

I thought we relied on user space to migrate its device state including the interrupt line output state and set the corresponding line state to the kernel, but that may have been wrong, and would rely on userspace to call the IRQ_LINE ioctl again. Would that work?

I'm not sure; it's definitely not what we do today, anyway...

Quoted 1 line

How is it again, does QEMU preserve the IRQ output line state per device?

In QEMU device models, each device has its own state and generally tracks the level of its input lines as necessary. When state restore happens, the device on the sending end restores its own state but doesn't reassert the irq line; the assumption is that the device on the receiving end restores its own state including the level of its input lines.

Quoted 3 lines

This may just work currently because we rely on the reads/writes to SPENDR to preserve the state and apparently level triggered interrupts are reinjected as needed.

I think the reason it more or less works is that as you say we write the pending bits back to the PENDR registers. There are two cases where this goes wrong for level triggered interrupts, though:
(1) if a device deasserts its interrupt output before the guest OS gets round to acknowledging the interrupt, then what should happen is that the interrupt stops being pending; since we wrote to PENDR then the latch means we'll deliver the guest a spurious interrupt
(2) if a guest OS's interrupt handler doesn't quiesce the device such that it deasserts the interrupt output, then what should happen is that when the interrupt is dismissed it remains pending and will trigger again; since we didn't copy the level state into the kernel then the kernel won't notice and won't deliver the interrupt again

Since the distributor/redistributor is fully emulated inside the kernel, we have the underlying state separately for levels and for the pending-latch, right? We don't need to make it impossible to access the latch separately just because that's what the hardware does...

thanks
-- PMM

#10 lore raw

Re: [PATCH v7 1/6] KVM: arm/arm64: Add VGICv3 save/restore API documentation

Quoted 45 lines · Peter Maydell
Quoted 8 lines · Christoffer Dall
Quoted 6 lines · Peter Maydell

Nothing in here describes a mechanism for reading or writing the current interrupt line_level state from the kernel (which doesn't matter for edge triggered interrupts but does for level triggered interrupts). Do we need accessors for these or does somebody have a good rationale for why we don't need to migrate that data? (Christoffer?)

Quoted 4 lines

I thought we relied on user space to migrate its device state including the interrupt line output state and set the corresponding line state to the kernel, but that may have been wrong, and would rely on userspace to call the IRQ_LINE ioctl again. Would that work?

I'm not sure; it's definitely not what we do today, anyway...

Quoted 1 line

How is it again, does QEMU preserve the IRQ output line state per device?

In QEMU device models, each device has its own state and generally tracks the level of its input lines as necessary. When state restore happens, the device on the sending end restores its own state but doesn't reassert the irq line; the assumption is that the device on the receiving end restores its own state including the level of its input lines.

Quoted 3 lines

This may just work currently because we rely on the reads/writes to SPENDR to preserve the state and apparently level triggered interrupts are reinjected as needed.

I think the reason it more or less works is that as you say we write the pending bits back to the PENDR registers. There are two cases where this goes wrong for level triggered interrupts, though:
(1) if a device deasserts its interrupt output before the guest OS gets round to acknowledging the interrupt, then what should happen is that the interrupt stops being pending; since we wrote to PENDR then the latch means we'll deliver the guest a spurious interrupt
(2) if a guest OS's interrupt handler doesn't quiesce the device such that it deasserts the interrupt output, then what should happen is that when the interrupt is dismissed it remains pending and will trigger again; since we didn't copy the level state into the kernel then the kernel won't notice and won't deliver the interrupt again

Quoted 6 lines

Since the distributor/redistributor is fully emulated inside the kernel, we have the underlying state separately for levels and for the pending-latch, right? We don't need to make it impossible to access the latch separately just because that's what the hardware does...

So I agree about the latch state, that should be exported, if nothing else so that a VM can't use this particular change to detect a migration, for example.

However, for the input level, I really do see this as a wire state between userspace and the kernel, and as something that userspace should re-establish after migration, since userspace already knows what the line level is, why does it need to pull it out of the kernel again?

-Christoffer

#11 lore raw

Re: [PATCH v7 1/6] KVM: arm/arm64: Add VGICv3 save/restore API documentation

Quoted 8 lines · Christoffer Dall

So I agree about the latch state, that should be exported, if nothing else so that a VM can't use this particular change to detect a migration, for example.

However, for the input level, I really do see this as a wire state between userspace and the kernel, and as something that userspace should re-establish after migration, since userspace already knows what the line level is, why does it need to pull it out of the kernel again?

I guess userspace could track the line level as well as telling the kernel about it, but we would still need an API for telling the kernel "here is your line level state after a migration", because that's not the same as "the line level just changed because the device signaled an interrupt". (For the former, we just want to set your 'level' struct field value. For the latter, a 0->1 level change may mean "set pending"; but for migration we're already telling you the current pending state via a different ioctl and don't want to mess that up when we're telling you about the 'level' info.) And if we have to have migration API for directly writing level bits we might as well have the corresponding directly-read-level-bits one...

thanks
-- PMM

#12 lore raw

PATCH v7 2/6 KVM: arm/arm64: Move endianness conversion out of vgic_attr_regs_access()

In reply to Pavel Fedin

mmio_data_read() and mmio_data_write(), originally used in this function, are limited only to 32 bits. We are going to refactor this code and eventually let it do 64-bit I/O for vGICv3. Therefore, our first step is to get rid of this limitation.

We open up these inlines, which consist of endianness conversion and masking. Masking is not used here (the mask is set to ~0), so we just move out the remaining endianness conversion.

---
 virt/kvm/arm/vgic-v2-emul.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)
virt/kvm/arm/vgic-v2-emul.c +8 -12
index 1390797..959b9c6 100644
@@ -663,7 +663,7 @@ static const struct vgic_io_range vgic_cpu_ranges[] = {
 
 static int vgic_attr_regs_access(struct kvm_device *dev,
 				 struct kvm_device_attr *attr,
-				 u32 *reg, bool is_write)
+				 __le32 *data, bool is_write)
 {
 	const struct vgic_io_range *r = NULL, *ranges;
 	phys_addr_t offset;
@@ -671,7 +671,6 @@ static int vgic_attr_regs_access(struct kvm_device *dev,
 	struct kvm_vcpu *vcpu, *tmp_vcpu;
 	struct vgic_dist *vgic;
 	struct kvm_exit_mmio mmio;
-	u32 data;
 
 	offset = attr->attr & KVM_DEV_ARM_VGIC_OFFSET_MASK;
 	cpuid = (attr->attr & KVM_DEV_ARM_VGIC_CPUID_MASK) >>
@@ -693,9 +692,7 @@ static int vgic_attr_regs_access(struct kvm_device *dev,
 
 	mmio.len = 4;
 	mmio.is_write = is_write;
-	mmio.data = &data;
-	if (is_write)
-		mmio_data_write(&mmio, ~0, *reg);
+	mmio.data = data;
 	switch (attr->group) {
 	case KVM_DEV_ARM_VGIC_GRP_DIST_REGS:
 		mmio.phys_addr = vgic->vgic_dist_base + offset;
@@ -743,9 +740,6 @@ static int vgic_attr_regs_access(struct kvm_device *dev,
 	offset -= r->base;
 	r->handle_mmio(vcpu, &mmio, offset);
 
-	if (!is_write)
-		*reg = mmio_data_read(&mmio, ~0);
-
 	ret = 0;
 out_vgic_unlock:
 	spin_unlock(&vgic->lock);
@@ -778,11 +772,13 @@ static int vgic_v2_set_attr(struct kvm_device *dev,
 	case KVM_DEV_ARM_VGIC_GRP_CPU_REGS: {
 		u32 __user *uaddr = (u32 __user *)(long)attr->addr;
 		u32 reg;
+		__le32 data;
 
 		if (get_user(reg, uaddr))
 			return -EFAULT;
 
-		return vgic_attr_regs_access(dev, attr, &reg, true);
+		data = cpu_to_le32(reg);
+		return vgic_attr_regs_access(dev, attr, &data, true);
 	}
 
 	}
@@ -803,12 +799,12 @@ static int vgic_v2_get_attr(struct kvm_device *dev,
 	case KVM_DEV_ARM_VGIC_GRP_DIST_REGS:
 	case KVM_DEV_ARM_VGIC_GRP_CPU_REGS: {
 		u32 __user *uaddr = (u32 __user *)(long)attr->addr;
-		u32 reg = 0;
+		__le32 data = 0;
 
-		ret = vgic_attr_regs_access(dev, attr, &reg, false);
+		ret = vgic_attr_regs_access(dev, attr, &data, false);
 		if (ret)
 			return ret;
-		return put_user(reg, uaddr);
+		return put_user(le32_to_cpu(data), uaddr);
 	}
 
 	}
Signature · 1 line
2.4.4
#13 lore raw

PATCH v7 3/6 KVM: arm/arm64: Refactor vGIC attributes handling code

In reply to Pavel Fedin

Separate all implementation-independent code in vgic_attr_regs_access() and move it to vgic.c. This will allow to reuse this code for vGICv3 implementation.

vcpu lookup is left where it originally was, because vGICv3 API will expect affinity ID instead of vCPU index, therefore it will be done differently. Also, vcpu pointer has backpointer to kvm, so 'dev' was replaced with 'vcpu'.

---
 virt/kvm/arm/vgic-v2-emul.c | 120 +++++++++++---------------------------------
 virt/kvm/arm/vgic.c         |  57 +++++++++++++++++++++
 virt/kvm/arm/vgic.h         |   3 ++
 3 files changed, 88 insertions(+), 92 deletions(-)
virt/kvm/arm/vgic-v2-emul.c +28 -92
index 959b9c6..8e769c6 100644
@@ -661,38 +661,24 @@ static const struct vgic_io_range vgic_cpu_ranges[] = {
 	},
 };
 
-static int vgic_attr_regs_access(struct kvm_device *dev,
-				 struct kvm_device_attr *attr,
-				 __le32 *data, bool is_write)
+static int vgic_v2_attr_regs_access(struct kvm_device *dev,
+				    struct kvm_device_attr *attr,
+				    __le32 *data, bool is_write)
 {
-	const struct vgic_io_range *r = NULL, *ranges;
+	const struct vgic_io_range *ranges;
 	phys_addr_t offset;
-	int ret, cpuid, c;
-	struct kvm_vcpu *vcpu, *tmp_vcpu;
-	struct vgic_dist *vgic;
+	struct kvm_vcpu *vcpu;
+	int cpuid;
+	struct vgic_dist *vgic = &dev->kvm->arch.vgic;
 	struct kvm_exit_mmio mmio;
 
 	offset = attr->attr & KVM_DEV_ARM_VGIC_OFFSET_MASK;
 	cpuid = (attr->attr & KVM_DEV_ARM_VGIC_CPUID_MASK) >>
 		KVM_DEV_ARM_VGIC_CPUID_SHIFT;
 
-	mutex_lock(&dev->kvm->lock);
-
-	ret = vgic_init(dev->kvm);
-	if (ret)
-		goto out;
-
-	if (cpuid >= atomic_read(&dev->kvm->online_vcpus)) {
-		ret = -EINVAL;
-		goto out;
-	}
+	if (cpuid >= atomic_read(&dev->kvm->online_vcpus))
+		return -EINVAL;
 
-	vcpu = kvm_get_vcpu(dev->kvm, cpuid);
-	vgic = &dev->kvm->arch.vgic;
-
-	mmio.len = 4;
-	mmio.is_write = is_write;
-	mmio.data = data;
 	switch (attr->group) {
 	case KVM_DEV_ARM_VGIC_GRP_DIST_REGS:
 		mmio.phys_addr = vgic->vgic_dist_base + offset;
@@ -703,49 +689,16 @@ static int vgic_attr_regs_access(struct kvm_device *dev,
 		ranges = vgic_cpu_ranges;
 		break;
 	default:
-		BUG();
+		return -ENXIO;
 	}
-	r = vgic_find_range(ranges, 4, offset);
 
-	if (unlikely(!r || !r->handle_mmio)) {
-		ret = -ENXIO;
-		goto out;
-	}
-
-
-	spin_lock(&vgic->lock);
-
-	/*
-	 * Ensure that no other VCPU is running by checking the vcpu->cpu
-	 * field.  If no other VPCUs are running we can safely access the VGIC
-	 * state, because even if another VPU is run after this point, that
-	 * VCPU will not touch the vgic state, because it will block on
-	 * getting the vgic->lock in kvm_vgic_sync_hwstate().
-	 */
-	kvm_for_each_vcpu(c, tmp_vcpu, dev->kvm) {
-		if (unlikely(tmp_vcpu->cpu != -1)) {
-			ret = -EBUSY;
-			goto out_vgic_unlock;
-		}
-	}
-
-	/*
-	 * Move all pending IRQs from the LRs on all VCPUs so the pending
-	 * state can be properly represented in the register state accessible
-	 * through this API.
-	 */
-	kvm_for_each_vcpu(c, tmp_vcpu, dev->kvm)
-		vgic_unqueue_irqs(tmp_vcpu);
+	vcpu = kvm_get_vcpu(dev->kvm, cpuid);
 
-	offset -= r->base;
-	r->handle_mmio(vcpu, &mmio, offset);
+	mmio.len = 4;
+	mmio.is_write = is_write;
+	mmio.data = data;
 
-	ret = 0;
-out_vgic_unlock:
-	spin_unlock(&vgic->lock);
-out:
-	mutex_unlock(&dev->kvm->lock);
-	return ret;
+	return vgic_attr_regs_access(vcpu, ranges, &mmio, offset);
 }
 
 static int vgic_v2_create(struct kvm_device *dev, u32 type)
@@ -761,55 +714,38 @@ static void vgic_v2_destroy(struct kvm_device *dev)
 static int vgic_v2_set_attr(struct kvm_device *dev,
 			    struct kvm_device_attr *attr)
 {
+	u32 __user *uaddr = (u32 __user *)(long)attr->addr;
+	u32 reg;
+	__le32 data;
 	int ret;
 
 	ret = vgic_set_common_attr(dev, attr);
 	if (ret != -ENXIO)
 		return ret;
 
-	switch (attr->group) {
-	case KVM_DEV_ARM_VGIC_GRP_DIST_REGS:
-	case KVM_DEV_ARM_VGIC_GRP_CPU_REGS: {
-		u32 __user *uaddr = (u32 __user *)(long)attr->addr;
-		u32 reg;
-		__le32 data;
-
-		if (get_user(reg, uaddr))
-			return -EFAULT;
-
-		data = cpu_to_le32(reg);
-		return vgic_attr_regs_access(dev, attr, &data, true);
-	}
-
-	}
+	if (get_user(reg, uaddr))
+		return -EFAULT;
 
-	return -ENXIO;
+	data = cpu_to_le32(reg);
+	return vgic_v2_attr_regs_access(dev, attr, &data, true);
 }
 
 static int vgic_v2_get_attr(struct kvm_device *dev,
 			    struct kvm_device_attr *attr)
 {
+	u32 __user *uaddr = (u32 __user *)(long)attr->addr;
+	__le32 data = 0;
 	int ret;
 
 	ret = vgic_get_common_attr(dev, attr);
 	if (ret != -ENXIO)
 		return ret;
 
-	switch (attr->group) {
-	case KVM_DEV_ARM_VGIC_GRP_DIST_REGS:
-	case KVM_DEV_ARM_VGIC_GRP_CPU_REGS: {
-		u32 __user *uaddr = (u32 __user *)(long)attr->addr;
-		__le32 data = 0;
-
-		ret = vgic_attr_regs_access(dev, attr, &data, false);
-		if (ret)
-			return ret;
-		return put_user(le32_to_cpu(data), uaddr);
-	}
-
-	}
+	ret = vgic_v2_attr_regs_access(dev, attr, &data, false);
+	if (ret)
+		return ret;
 
-	return -ENXIO;
+	return put_user(le32_to_cpu(data), uaddr);
 }
 
 static int vgic_v2_has_attr(struct kvm_device *dev,
virt/kvm/arm/vgic.c +57 -0

63 lines not on this page · raw message

virt/kvm/arm/vgic.h +3 -0
index 0df74cb..fca89b7 100644
@@ -132,6 +132,9 @@ void vgic_kick_vcpus(struct kvm *kvm);
 int vgic_has_attr_regs(const struct vgic_io_range *ranges, phys_addr_t offset);
 int vgic_set_common_attr(struct kvm_device *dev, struct kvm_device_attr *attr);
 int vgic_get_common_attr(struct kvm_device *dev, struct kvm_device_attr *attr);
+int vgic_attr_regs_access(struct kvm_vcpu *vcpu,
+			  const struct vgic_io_range *ranges,
+			  struct kvm_exit_mmio *mmio, phys_addr_t offset);
 
 int vgic_init(struct kvm *kvm);
 void vgic_v2_init_emulation(struct kvm *kvm);
Signature · 1 line
2.4.4
#14 lore raw

PATCH v7 4/6 KVM: arm64: Implement vGICv3 distributor and redistributor access from userspace

In reply to Pavel Fedin

The access is done similar to vGICv2, using KVM_DEV_ARM_VGIC_GRP_DIST_REGS and KVM_DEV_ARM_VGIC_GRP_REDIST_REGS with KVM_SET_DEVICE_ATTR and KVM_GET_DEVICE_ATTR ioctls.

Access size for vGICv3 is 64 bits, vgic_attr_regs_access() fixed to support this. The trick with vgic_v3_get_reg_size() is necessary because the major part of GICv3 registers is actually 32-bit, and their accessors do not distinguish between lower and upper words (offset & 3). Accessing these registers with len == 8 would cause rollover. For write operations this would overwrite lower word with the upper one (which would normally be 0), for read operations this would cause duplication of the same word in both halves.

---
 arch/arm64/include/uapi/asm/kvm.h  |   1 +
 include/linux/irqchip/arm-gic-v3.h |   1 +
 virt/kvm/arm/vgic-v3-emul.c        | 112 ++++++++++++++++++++++++++++++++-----
 virt/kvm/arm/vgic.c                |   4 +-
 4 files changed, 102 insertions(+), 16 deletions(-)
arch/arm64/include/uapi/asm/kvm.h +1 -0
index 2d4ca4b..98bd047 100644
@@ -203,6 +203,7 @@ struct kvm_arch_memory_slot {
 #define KVM_DEV_ARM_VGIC_GRP_NR_IRQS	3
 #define KVM_DEV_ARM_VGIC_GRP_CTRL	4
 #define   KVM_DEV_ARM_VGIC_CTRL_INIT	0
+#define KVM_DEV_ARM_VGIC_GRP_REDIST_REGS 5
 
 /* KVM_IRQ_LINE irq field index values */
 #define KVM_ARM_IRQ_TYPE_SHIFT		24
include/linux/irqchip/arm-gic-v3.h +1 -0
index c9ae0c6..53fd894 100644
@@ -43,6 +43,7 @@
 #define GICD_IGRPMODR			0x0D00
 #define GICD_NSACR			0x0E00
 #define GICD_IROUTER			0x6000
+#define GICD_IROUTER1019		0x7FD8
 #define GICD_IDREGS			0xFFD0
 #define GICD_PIDR2			0xFFE8
 
virt/kvm/arm/vgic-v3-emul.c +98 -14

160 lines not on this page · raw message

virt/kvm/arm/vgic.c +2 -2
index b8148f2..4047d0e 100644
@@ -2372,7 +2372,7 @@ int vgic_attr_regs_access(struct kvm_vcpu *vcpu,
 	struct kvm_vcpu *tmp_vcpu;
 	struct vgic_dist *vgic;
 
-	r = vgic_find_range(ranges, 4, offset);
+	r = vgic_find_range(ranges, mmio->len, offset);
 
 	if (unlikely(!r || !r->handle_mmio))
 		return -ENXIO;
@@ -2410,7 +2410,7 @@ int vgic_attr_regs_access(struct kvm_vcpu *vcpu,
 		vgic_unqueue_irqs(tmp_vcpu);
 
 	offset -= r->base;
-	r->handle_mmio(vcpu, mmio, offset);
+	call_range_handler(vcpu, mmio, offset, r);
 
 	ret = 0;
 out_vgic_unlock:
Signature · 1 line
2.4.4
#15 lore raw

PATCH v7 5/6 KVM: arm64: Introduce find_reg_by_id()

In reply to Pavel Fedin

In order to implement vGICv3 CPU interface access, we will need to perform table lookup of system registers. We would need both index_to_params() and find_reg() exported for that purpose, but instead we export a single function which combines them both.

---
 arch/arm64/kvm/sys_regs.c | 22 +++++++++++++++-------
 arch/arm64/kvm/sys_regs.h |  4 ++++
 2 files changed, 19 insertions(+), 7 deletions(-)
arch/arm64/kvm/sys_regs.c +15 -7
index d2650e8..8c4b671 100644
@@ -1276,6 +1276,17 @@ static bool index_to_params(u64 id, struct sys_reg_params *params)
 	}
 }
 
+const struct sys_reg_desc *find_reg_by_id(u64 id,
+					  struct sys_reg_params *params,
+					  const struct sys_reg_desc table[],
+					  unsigned int num)
+{
+	if (!index_to_params(id, params))
+		return NULL;
+
+	return find_reg(params, table, num);
+}
+
 /* Decode an index value, and find the sys_reg_desc entry. */
 static const struct sys_reg_desc *index_to_sys_reg_desc(struct kvm_vcpu *vcpu,
 						    u64 id)
@@ -1403,10 +1414,8 @@ static int get_invariant_sys_reg(u64 id, void __user *uaddr)
 	struct sys_reg_params params;
 	const struct sys_reg_desc *r;
 
-	if (!index_to_params(id, &params))
-		return -ENOENT;
-
-	r = find_reg(&params, invariant_sys_regs, ARRAY_SIZE(invariant_sys_regs));
+	r = find_reg_by_id(id, &params, invariant_sys_regs,
+			   ARRAY_SIZE(invariant_sys_regs));
 	if (!r)
 		return -ENOENT;
 
@@ -1420,9 +1429,8 @@ static int set_invariant_sys_reg(u64 id, void __user *uaddr)
 	int err;
 	u64 val = 0; /* Make sure high bits are 0 for 32-bit regs */
 
-	if (!index_to_params(id, &params))
-		return -ENOENT;
-	r = find_reg(&params, invariant_sys_regs, ARRAY_SIZE(invariant_sys_regs));
+	r = find_reg_by_id(id, &params, invariant_sys_regs,
+			   ARRAY_SIZE(invariant_sys_regs));
 	if (!r)
 		return -ENOENT;
 
arch/arm64/kvm/sys_regs.h +4 -0
index dbbb01c..9c6ffd0 100644
@@ -136,6 +136,10 @@ static inline int cmp_sys_reg(const struct sys_reg_desc *i1,
 	return i1->Op2 - i2->Op2;
 }
 
+const struct sys_reg_desc *find_reg_by_id(u64 id,
+					  struct sys_reg_params *params,
+					  const struct sys_reg_desc table[],
+					  unsigned int num);
 
 #define Op0(_x) 	.Op0 = _x
 #define Op1(_x) 	.Op1 = _x
Signature · 1 line
2.4.4
#16 lore raw

PATCH v7 6/6 KVM: arm64: Implement vGICv3 CPU interface access

In reply to Pavel Fedin

Access size is always 64 bits. Since CPU interface state actually affects only a single vCPU, no vGIC locking is done in order to avoid code duplication. Just made sure that the vCPU is not running.

---
 arch/arm64/include/uapi/asm/kvm.h  |  14 ++-
 arch/arm64/mm/mmap.c               |   2 +-
 include/linux/irqchip/arm-gic-v3.h |  18 ++-
 virt/kvm/arm/vgic-v3-emul.c        | 224 ++++++++++++++++++++++++++++++++++++-
 4 files changed, 251 insertions(+), 7 deletions(-)
arch/arm64/include/uapi/asm/kvm.h +11 -3
index 98bd047..ca32fe5 100644
@@ -179,14 +179,14 @@ struct kvm_arch_memory_slot {
 	KVM_REG_ARM64_SYSREG_ ## n ## _MASK)
 
 #define __ARM64_SYS_REG(op0,op1,crn,crm,op2) \
-	(KVM_REG_ARM64 | KVM_REG_ARM64_SYSREG | \
-	ARM64_SYS_REG_SHIFT_MASK(op0, OP0) | \
+	(ARM64_SYS_REG_SHIFT_MASK(op0, OP0) | \
 	ARM64_SYS_REG_SHIFT_MASK(op1, OP1) | \
 	ARM64_SYS_REG_SHIFT_MASK(crn, CRN) | \
 	ARM64_SYS_REG_SHIFT_MASK(crm, CRM) | \
 	ARM64_SYS_REG_SHIFT_MASK(op2, OP2))
 
-#define ARM64_SYS_REG(...) (__ARM64_SYS_REG(__VA_ARGS__) | KVM_REG_SIZE_U64)
+#define ARM64_SYS_REG(...) (__ARM64_SYS_REG(__VA_ARGS__) | KVM_REG_ARM64 | \
+			    KVM_REG_SIZE_U64 | KVM_REG_ARM64_SYSREG)
 
 #define KVM_REG_ARM_TIMER_CTL		ARM64_SYS_REG(3, 3, 14, 3, 1)
 #define KVM_REG_ARM_TIMER_CNT		ARM64_SYS_REG(3, 3, 14, 3, 2)
@@ -204,6 +204,14 @@ struct kvm_arch_memory_slot {
 #define KVM_DEV_ARM_VGIC_GRP_CTRL	4
 #define   KVM_DEV_ARM_VGIC_CTRL_INIT	0
 #define KVM_DEV_ARM_VGIC_GRP_REDIST_REGS 5
+#define KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS 6
+#define   KVM_DEV_ARM_VGIC_SYSREG_MASK	(KVM_REG_ARM64_SYSREG_OP0_MASK | \
+					 KVM_REG_ARM64_SYSREG_OP1_MASK | \
+					 KVM_REG_ARM64_SYSREG_CRN_MASK | \
+					 KVM_REG_ARM64_SYSREG_CRM_MASK | \
+					 KVM_REG_ARM64_SYSREG_OP2_MASK)
+#define   KVM_DEV_ARM_VGIC_SYSREG(op0, op1, crn, crm, op2) \
+					__ARM64_SYS_REG(op0, op1, crn, crm, op2)
 
 /* KVM_IRQ_LINE irq field index values */
 #define KVM_ARM_IRQ_TYPE_SHIFT		24
arch/arm64/mm/mmap.c +1 -1
index af461b9..e59a75a 100644
@@ -51,7 +51,7 @@ unsigned long arch_mmap_rnd(void)
 {
 	unsigned long rnd;
 
-ifdef CONFIG_COMPAT
+#ifdef CONFIG_COMPAT
 	if (test_thread_flag(TIF_32BIT))
 		rnd = (unsigned long)get_random_int() % (1 << mmap_rnd_compat_bits);
 	else
include/linux/irqchip/arm-gic-v3.h +16 -2
index 53fd894..bff3eee 100644
@@ -259,8 +259,14 @@
 /*
  * CPU interface registers
  */
-#define ICC_CTLR_EL1_EOImode_drop_dir	(0U << 1)
-#define ICC_CTLR_EL1_EOImode_drop	(1U << 1)
+#define ICC_CTLR_EL1_CBPR_SHIFT		0
+#define ICC_CTLR_EL1_EOImode_SHIFT	1
+#define ICC_CTLR_EL1_EOImode_drop_dir	(0U << ICC_CTLR_EL1_EOImode_SHIFT)
+#define ICC_CTLR_EL1_EOImode_drop	(1U << ICC_CTLR_EL1_EOImode_SHIFT)
+#define ICC_CTLR_EL1_PRIbits_MASK	(7U << 8)
+#define ICC_CTLR_EL1_IDbits_MASK	(7U << 11)
+#define ICC_CTLR_EL1_SEIS		(1U << 14)
+#define ICC_CTLR_EL1_A3V		(1U << 15)
 #define ICC_SRE_EL1_SRE			(1U << 0)
 
 /*
@@ -285,6 +291,14 @@
 
 #define ICH_VMCR_CTLR_SHIFT		0
 #define ICH_VMCR_CTLR_MASK		(0x21f << ICH_VMCR_CTLR_SHIFT)
+#define ICH_VMCR_ENG0_SHIFT		0
+#define ICH_VMCR_ENG0			(1 << ICH_VMCR_ENG0_SHIFT)
+#define ICH_VMCR_ENG1_SHIFT		1
+#define ICH_VMCR_ENG1			(1 << ICH_VMCR_ENG1_SHIFT)
+#define ICH_VMCR_CBPR_SHIFT		4
+#define ICH_VMCR_CBPR			(1 << ICH_VMCR_CBPR_SHIFT)
+#define ICH_VMCR_EOIM_SHIFT		9
+#define ICH_VMCR_EOIM			(1 << ICH_VMCR_EOIM_SHIFT)
 #define ICH_VMCR_BPR1_SHIFT		18
 #define ICH_VMCR_BPR1_MASK		(7 << ICH_VMCR_BPR1_SHIFT)
 #define ICH_VMCR_BPR0_SHIFT		21
virt/kvm/arm/vgic-v3-emul.c +223 -1

248 lines not on this page · raw message

Signature · 1 line
2.4.4