On Wed, 12 Aug 2020 at 00:40, Andrew Jones <drjo...@redhat.com> wrote:
> On Tue, Aug 11, 2020 at 10:38:02AM +0800, Haibo Xu wrote:
> > On Mon, 10 Aug 2020 at 19:05, Andrew Jones <drjo...@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
>
>
