On Sun, Aug 19, 2012 at 1:01 PM, Peter Maydell <peter.maydell@xxxxxxxxxx> wrote: > On 18 August 2012 20:25, Christoffer Dall <c.dall@xxxxxxxxxxxxxxxxxxxxxx> wrote: >> We need to rework this scheme, see the discussion here: >> https://patchwork.kernel.org/patch/1149581/ >> >> Having an ioctl that can both be a vcpu and a vm ioctl is certainly ugly. >> >> I don't think we should have a vcpu ioctl for this purpose at all for >> two reasons 1) this happens asynchronously to the vcpu thread and 2) >> even a PPI is semantically not a vcpu operation but an operation on >> the gic distributor, which belongs to the VM. > > I'm not at all convinced by (1) as a reason but (2) is more compelling. I am not saying (1) is something written in stone, but if that's the consensus for things now (as Avi indicates) I don't see why we should come and change it when we can easily build on what is there. > >> Can't we simply build on what we have and use this scheme: >> >> | 31 30 29 ... 18 17 16 | 15 | 14 13 12 ... 3 2 1 | 0 | >> | vcpu index | 0 | SPI / PPI | I=0/F=1 | > >> For user space gic and PPI: >> vcpu_idx = val >> 16; >> if (vcpu_idx > online_vcpus) >> return -EINVAL; >> >> For user space gic: >> bool irq = !(val & 1); >> bool fiq = val & 1' >> >> For in-kernel gic: >> irq = (val >> 1) & 0x7fff; >> if (irq < 16) >> return -EINVAL; > > Rather than cramming all of these into a single format, maybe it > would be better to have a 'target' field indicating which array > of input lines you are attempting to assert and an 'index' for > which signal within that array you are asserting. You could then > make the arrays match up with the h/w A15's external signals, which > have the following arrays of inputs: > IRQS[] : external interrupts to the GIC > nIRQ[] : IRQ line for each core > nFIQ[] : FIQ line for each core > PPI[] : per-core private peripheral interrupts (not actually > an external signal on A15 h/w since it's signalling > between two different bits of the processor) > > Then you don't have to worry about later additions (if any) not > being fittable into the existing scheme. > I'm fine with it either way, and I like that we can expand on it in the future, I just want to reach a decision, so let's do what you suggest: IRQS[992] nIRQ[256] nFIQ[256] PPI[4096] e.g. index = 993 means IRQ line raised on vcpu with index 1 (maximum of 256 cpus, no means to inject SGI) -Christoffer _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
