On Thu, Aug 11, 2016 at 1:15 PM, Peter Maydell <peter.maydell@xxxxxxxxxx> wrote: > On 11 August 2016 at 06:29, Vijay Kilari <vijay.kilari@xxxxxxxxx> wrote: >> On Tue, Aug 9, 2016 at 5:22 PM, Peter Maydell <peter.maydell@xxxxxxxxxx> wrote: >>> On 9 August 2016 at 11:58, <vijay.kilari@xxxxxxxxx> wrote: >>>> From: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxx> >>>> >>>> This patchset adds API for saving and restoring >>>> of VGICv3 registers to support live migration with new vgic feature. >>>> This API definition is as per version of VGICv3 specification >>>> http://lists.infradead.org/pipermail/linux-arm-kernel/2016-July/445611.html >>>> >>>> To test live migration with QEMU, use below patch series >>>> https://lists.gnu.org/archive/html/qemu-devel/2016-08/msg01444.html >>>> >>>> The patch 3 & 4 are picked from the Pavel's previous implementation. >>>> http://www.spinics.net/lists/kvm/msg122040.html >>>> >>>> v1 => v2: >>>> - The init sequence change patch is no more required. >>>> Fixed in patch 2 by using static vgic_io_dev regions structure instead >>>> of using dynamic allocation pointer. >>>> - Updated commit message of patch 4. >>>> - Dropped usage of union to manage 32-bit and 64-bit access in patch 1. >>>> Used local variable for 32-bit access. >>>> - Updated macro __ARM64_SYS_REG and ARM64_SYS_REG in >>>> arch/arm64/include/uapi/asm/kvm.h as per qemu requirements. >>> >>> I only scanned briefly through this patchset, but I didn't >>> see any code implementing: >>> * KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO >> >> If irq->pending is updated by kernel based on irq->line_level when interrupt >> is asserted by device or guest. Do we still need to extract >> irq->line_level using >> this ioctl and while writing back GIC{D|R}_ISPENDR with line_level >> +(OR) GIC{D|R}_ISPENDR? > > The level and the pending status are different things; > the API docs have an explanation of this. The API access > to the ISPENDR registers should return only the pending > latch status (which is not the same as what these registers > return if you read them from the guest). > OK. I have implemented separate api for ISPENDR userspace access to read soft_pending for level triggered interrupts. This needs kernel implementation to support separate api's for guest and userspace access. >>> * the different behaviour for accesses to GICD_STATUSR, GICR_STATUSR, >> >> QEMU is saving and restoring this register, but kernel implementation >> is missing. Kernel is silently returning zero. So could not catch. I >> will fix it. >> >> However, Specification says as below for STATUSR. >> >> " The GICD_STATUSR and GICR_STATUSR registers are architecturally >> defined such >> that a write of a clear bit has no effect, whereas a write with a set bit >> clears that value. To allow userspace to freely set the values >> of these two >> registers, setting the attributes with the register offsets for these two >> registers simply sets the non-reserved bits to the value written." >> >> Question is during restore, the set bit will clear the value STATUSR. >> So it will reset the STATUSR after migrating the VM. > > The text you quote above says that setting the attribute via > the API "sets the non-reserved bits to the value written". > This is the point -- it does not have the write-1-to-clear > behaviour that a guest access to the register does. > In the current implementation of vgic in kernel I could not find any implement/support for GICD_STATUSR register value. Should I leave this as RAZ / WI for now?. _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
