diff mbox

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

Message ID 204d6b178e611ee0b5aede2b945516e0a6048c3c.1449491035.git.p.fedin@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Pavel Fedin Dec. 7, 2015, 12:29 p.m. UTC
From: Christoffer Dall <christoffer.dall@linaro.org>

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 <peter.maydell@linaro.org>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
---
 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

Comments

Peter Maydell Feb. 26, 2016, 3:01 p.m. UTC | #1
On 7 December 2015 at 12:29, Pavel Fedin <p.fedin@samsung.com> wrote:
> From: Christoffer Dall <christoffer.dall@linaro.org>
>
> 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 <peter.maydell@linaro.org>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>

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

> +  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" ?

> +
> +    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

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
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoffer Dall March 2, 2016, 12:48 p.m. UTC | #2
On Fri, Feb 26, 2016 at 03:01:56PM +0000, Peter Maydell wrote:
> On 7 December 2015 at 12:29, Pavel Fedin <p.fedin@samsung.com> wrote:
> > From: Christoffer Dall <christoffer.dall@linaro.org>
> >
> > 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 <peter.maydell@linaro.org>
> > Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> > Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> > Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
> 
> I was rereading this API doc this week, and I realised we missed
> something when we wrote it:
> 
> > +  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.

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.

> 
> 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.

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

> 
> 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.

> 
> > +
> > +    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
> 
> 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
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Maydell April 15, 2016, 1:20 p.m. UTC | #3
On 7 December 2015 at 12:29, Pavel Fedin <p.fedin@samsung.com> wrote:
> From: Christoffer Dall <christoffer.dall@linaro.org>
>
> 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.

> +  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.

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
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Maydell April 15, 2016, 1:58 p.m. UTC | #4
On 7 December 2015 at 12:29, Pavel Fedin <p.fedin@samsung.com> wrote:
> From: Christoffer Dall <christoffer.dall@linaro.org>
>
> 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 <peter.maydell@linaro.org>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>

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
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Maydell April 18, 2016, 10:33 a.m. UTC | #5
On 15 April 2016 at 14:58, Peter Maydell <peter.maydell@linaro.org> wrote:
> 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
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Maydell April 20, 2016, 1:28 p.m. UTC | #6
On 20 April 2016 at 11:59, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> On Friday, 15 April 2016, Peter Maydell <peter.maydell@linaro.org> wrote:
>> 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?)

> 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...

> 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.

> 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
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoffer Dall April 21, 2016, 6:30 p.m. UTC | #7
On Wed, Apr 20, 2016 at 02:28:05PM +0100, Peter Maydell wrote:
> On 20 April 2016 at 11:59, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> > On Friday, 15 April 2016, Peter Maydell <peter.maydell@linaro.org> wrote:
> >> 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?)
> 
> > 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...
> 
> > 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.
> 
> > 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...
> 
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
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Maydell April 21, 2016, 6:44 p.m. UTC | #8
On 21 April 2016 at 19:30, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> 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
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/virtual/kvm/devices/arm-vgic-v3.txt b/Documentation/virtual/kvm/devices/arm-vgic-v3.txt
new file mode 100644
index 0000000..24e2f6b
--- /dev/null
+++ b/Documentation/virtual/kvm/devices/arm-vgic-v3.txt
@@ -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
diff --git a/Documentation/virtual/kvm/devices/arm-vgic.txt b/Documentation/virtual/kvm/devices/arm-vgic.txt
index 59541d4..257b854 100644
--- a/Documentation/virtual/kvm/devices/arm-vgic.txt
+++ b/Documentation/virtual/kvm/devices/arm-vgic.txt
@@ -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: