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@xxxxxxxxxx>
> >>
> >> ---
> >> 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
