On 12/05/16 09:35, Christoffer Dall wrote: > On Fri, May 06, 2016 at 11:45:42AM +0100, Andre Przywara wrote: >> The target register handlers are v2 emulation specific, so their >> implementation lives entirely in vgic-mmio-v2.c. >> We copy the old VGIC behaviour of assigning an IRQ to the first VCPU >> set in the target mask instead of making it possibly pending on >> multiple VCPUs. >> >> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx> >> --- >> Changelog RFC..v1: >> - remove runtime VCPU determination from this v2-only register >> - fold in implementation of vgic_v2_irq_change_affinity() >> - replace ffs() with __ffs() >> >> Changelog v1 .. v2: >> - adapt to new MMIO framework >> >> virt/kvm/arm/vgic/vgic-mmio-v2.c | 43 +++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 42 insertions(+), 1 deletion(-) >> >> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v2.c b/virt/kvm/arm/vgic/vgic-mmio-v2.c >> index 2a953ec..888529e 100644 >> --- a/virt/kvm/arm/vgic/vgic-mmio-v2.c >> +++ b/virt/kvm/arm/vgic/vgic-mmio-v2.c >> @@ -66,6 +66,47 @@ static void vgic_mmio_write_v2_misc(struct kvm_vcpu *vcpu, >> } >> } >> >> +static unsigned long vgic_mmio_read_target(struct kvm_vcpu *vcpu, >> + gpa_t addr, unsigned int len) >> +{ >> + u32 intid = addr & 0x3ff; >> + int i; >> + u64 val = 0; >> + >> + for (i = 0; i < len; i++) { >> + struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i); >> + >> + val |= (u64)irq->targets << (i * 8); >> + } >> + >> + return val; > > this register should allow byte access, so you're missing a call to > extract_bytes() ? > > >> +} >> + >> +static void vgic_mmio_write_target(struct kvm_vcpu *vcpu, >> + gpa_t addr, unsigned int len, >> + unsigned long val) >> +{ >> + u32 intid = addr & 0x3ff; >> + int i; >> + >> + /* GICD_ITARGETSR[0-7] are read-only */ >> + if (intid < VGIC_NR_PRIVATE_IRQS) >> + return; >> + >> + for (i = 0; i < len; i++) { >> + struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, NULL, intid + i); >> + int target; >> + >> + spin_lock(&irq->irq_lock); >> + >> + irq->targets = (val >> (i * 8)) & 0xff; > > this doesn't seem right given byte accesses either, and I don't see the > fixups we have in the works fixing it... I'll give it a whirl. the priority stuff needs addressing as well. M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html