[PATCH v6 09/24] KVM: arm64: vgic-its: Introduce migration ABI infrastructure

[christoffer.dall@linaro.org (Christoffer Dall)]

On Fri, May 05, 2017 at 10:16:08AM +0200, Auger Eric wrote:
> Hi Christoffer,
> 
> On 04/05/2017 19:13, Christoffer Dall wrote:
> > On Thu, May 04, 2017 at 01:44:29PM +0200, Eric Auger wrote:
> >> We plan to support different migration ABIs, ie. characterizing
> >> the ITS table layout format in guest RAM. For example, a new ABI
> >> will be needed if vLPIs get supported for nested use case.
> >>
> >> So let's introduce an array of supported ABIs (at the moment a single
> >> ABI is supported though). The following characteristics are foreseen
> >> to vary with the ABI: size of table entries, save/restore operation,
> >> the way abi settings are applied.
> >>
> >> By default the MAX_ABI_REV is applied on its creation. In subsequent
> >> patches we will introduce a way for the userspace to change the ABI
> >> in use.
> >>
> >> The entry sizes now are set according to the ABI version and not
> >> hardcoded anymore.
> >>
> >> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> >>
> >> ---
> >> v6 -> v7:
> >> - rename abi into its_table_abi_versions
> >> - kernel doc comments for struct vgic_its_abi
> >> - comment GIC_ENCODE_SZ
> >> - slighly rephrase the commit message
> >>
> >> v5: creation and squash KVM: arm64: ITS: Report the ITE size in
> >>     GITS_TYPER
> >> ---
> >>  include/kvm/arm_vgic.h             |  3 ++
> >>  include/linux/irqchip/arm-gic-v3.h |  5 +++
> >>  virt/kvm/arm/vgic/vgic-its.c       | 92 ++++++++++++++++++++++++++++++++++++--
> >>  3 files changed, 96 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> >> index c0b3d99..285474a 100644
> >> --- a/include/kvm/arm_vgic.h
> >> +++ b/include/kvm/arm_vgic.h
> >> @@ -162,6 +162,9 @@ struct vgic_its {
> >>  	u32			creadr;
> >>  	u32			cwriter;
> >>  
> >> +	/* migration ABI revision in use */
> >> +	u32			abi_rev;
> >> +
> >>  	/* Protects the device and collection lists */
> >>  	struct mutex		its_lock;
> >>  	struct list_head	device_list;
> >> diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
> >> index 97cbca1..81ebe43 100644
> >> --- a/include/linux/irqchip/arm-gic-v3.h
> >> +++ b/include/linux/irqchip/arm-gic-v3.h
> >> @@ -132,6 +132,9 @@
> >>  #define GIC_BASER_SHAREABILITY(reg, type)				\
> >>  	(GIC_BASER_##type << reg##_SHAREABILITY_SHIFT)
> >>  
> >> +/* encode a size field of width @w containing @n - 1 units */
> >> +#define GIC_ENCODE_SZ(n, w) (((unsigned long)(n) - 1) & GENMASK_ULL(((w) - 1), 0))
> >> +
> >>  #define GICR_PROPBASER_SHAREABILITY_SHIFT		(10)
> >>  #define GICR_PROPBASER_INNER_CACHEABILITY_SHIFT		(7)
> >>  #define GICR_PROPBASER_OUTER_CACHEABILITY_SHIFT		(56)
> >> @@ -232,6 +235,7 @@
> >>  #define GITS_CTLR_QUIESCENT		(1U << 31)
> >>  
> >>  #define GITS_TYPER_PLPIS		(1UL << 0)
> >> +#define GITS_TYPER_ITT_ENTRY_SIZE_SHIFT	4
> >>  #define GITS_TYPER_IDBITS_SHIFT		8
> >>  #define GITS_TYPER_DEVBITS_SHIFT	13
> >>  #define GITS_TYPER_DEVBITS(r)		((((r) >> GITS_TYPER_DEVBITS_SHIFT) & 0x1f) + 1)
> >> @@ -290,6 +294,7 @@
> >>  #define GITS_BASER_TYPE(r)		(((r) >> GITS_BASER_TYPE_SHIFT) & 7)
> >>  #define GITS_BASER_ENTRY_SIZE_SHIFT		(48)
> >>  #define GITS_BASER_ENTRY_SIZE(r)	((((r) >> GITS_BASER_ENTRY_SIZE_SHIFT) & 0x1f) + 1)
> >> +#define GITS_BASER_ENTRY_SIZE_MASK	GENMASK_ULL(52, 48)
> >>  #define GITS_BASER_SHAREABILITY_SHIFT	(10)
> >>  #define GITS_BASER_InnerShareable					\
> >>  	GIC_BASER_SHAREABILITY(GITS_BASER, InnerShareable)
> >> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
> >> index 18588ef..c336cce 100644
> >> --- a/virt/kvm/arm/vgic/vgic-its.c
> >> +++ b/virt/kvm/arm/vgic/vgic-its.c
> >> @@ -33,6 +33,12 @@
> >>  #include "vgic.h"
> >>  #include "vgic-mmio.h"
> >>  
> >> +/* Highest migration ABI revision supported by this code */
> >> +#define MAX_ABI_REV 0
> >> +
> >> +static int vgic_its_set_abi(struct vgic_its *its, int rev);
> >> +static const struct vgic_its_abi *vgic_its_get_abi(struct vgic_its *its);
> >> +
> >>  /*
> >>   * Creates a new (reference to a) struct vgic_irq for a given LPI.
> >>   * If this LPI is already mapped on another ITS, we increase its refcount
> >> @@ -123,6 +129,25 @@ struct its_ite {
> >>  	u32 event_id;
> >>  };
> >>  
> >> +/**
> >> + * struct vgic_its_abi - ITS abi ops and settings
> >> + * @cte_esz: collection table entry size
> >> + * @dte_esz: device table entry size
> >> + * @ite_esz: interrupt translation table entry size
> >> + * @save tables: save the ITS tables into guest RAM
> >> + * @restore_tables: restore the ITS internal structs from
> >> + * tables stored in guest RAM
> > 
> > nit: indent to align with the beginning of 'restore...'
> > 
> > 
> >> + * @commit: apply the ABI settings to the ITS
> > 
> > hmm, I already forgot what the commit function does, and reading this
> > text doesn't seem to help me, unfortunately.  When is this called, what
> > does it do?
> what about:
>  * @commit: initialize the registers which expose the ABI settings,
>  *  especially the entry sizes
> 
Ah yes, much better.

Thanks,
-Christoffer