On Tue, Aug 11, 2020 at 10:38:02AM +0800, Haibo Xu wrote:
> On Mon, 10 Aug 2020 at 19:05, Andrew Jones <drjones@redhat.com> wrote:
> >
> > On Fri, Aug 07, 2020 at 08:10:36AM +0000, Haibo Xu wrote:
> > > Add a virtual SPE device for virt machine while using PPI
> > > 5 for SPE overflow interrupt number.
> >
> > Any reason PPI 5 was selected?
> >
>
> No special reason to choose PPI 5. Just re-use the setting in kvmtool.
Please write in the commit message that kvmtool has already selected PPI 5
for this purpose.
Ok, will fix it.
> > > + fdt_add_spe_nodes(vms);
> > > +
> >
> > You didn't add any compat code, which means all virt machine types are now
> > getting an SPE FDT node, ACPI table change, and, most importantly, PPI 5
> > has gone from unallocated to allocated. We definitely need compat code.
> >
>
> So the 'compat code' here means to only add the SPE node in KVM mode?
No, it means only add it for the 5.2 and later machine types. You'll see
what I mean when you study the patchset I pointed out, which is also only
for 5.2 and later machine types.
Ok, thanks for the clarification!
> > > + if (switched_level & KVM_ARM_DEV_SPE) {
> > > + qemu_set_irq(cpu->spe_interrupt,
> > > + !!(run->s.regs.device_irq_level & KVM_ARM_DEV_SPE));
> > > + switched_level &= ~KVM_ARM_DEV_SPE;
> > > + }
> > > +
> >
> > Did you test with a userspace irqchip?
>
> No, I just tested with an in-kernel irqchip.
> Actually, the current kernel vSPE patch doesn't support a userspace irqchip.
> AFAIK, the userspace irqchip support should be ready in the next
> kernel patch series
> which will be sent out for review in the middle of September.
It probably doesn't hurt to do the above hunk already, hoping it will just
work when it's possible to test, but I generally prefer only adding tested
code. Maybe this hunk should be a separate patch with a commit message
explaining that it's untested?
Good idea! I will drop the hunk in this series, and send out a separate patch to enable it
once the kernel support is ready!
Thanks,
drew