Re: [EXTERNAL] Re: enabling same PPI interrupt to all CPU in ARM64 SMP
Date: Mon, 15 May 2023 05:05:40 UTC
On Sun, May 14, 2023 at 9:59 AM Souradeep Chakrabarti <schakrabarti@microsoft.com> wrote: > > > > > >-----Original Message----- > >From: Kyle Evans <kevans@freebsd.org> > >Sent: Friday, May 12, 2023 10:40 PM > >To: Souradeep Chakrabarti <schakrabarti@microsoft.com> > >Cc: Wei Hu <weh@microsoft.com>; freebsd-hackers@FreeBSD.org > >Subject: [EXTERNAL] Re: enabling same PPI interrupt to all CPU in ARM64 SMP > > > >On Fri, May 12, 2023 at 9:51 AM Souradeep Chakrabarti > ><schakrabarti@microsoft.com> wrote: > >> > >> > >> > >> > >> >-----Original Message----- > >> >From: Souradeep Chakrabarti > >> >Sent: Monday, May 8, 2023 6:39 PM > >> >To: Kyle Evans <kevans@freebsd.org> > >> >Cc: Wei Hu <weh@microsoft.com>; freebsd-hackers@FreeBSD.org > >> >Subject: enabling same PPI interrupt to all CPU in ARM64 SMP > >> > > >> >Hi , > >> > > >> >While using SMP in ARM64 Hyper-V we are getting stuck in boot if > >> >there is a interrupt for VMBus coming to CPU1 and VMBus interrupt > >> >handler is not getting that interrupt. > >> > > >> >In ARM64 Hyper-V we are using IRQ18 for VMBus and it is a PPI interrupt. > >> > > >> >But Hypev-V host sends interrupt to this IRQ 18 for both CPU0 and > >> >CPU1 in 2CPU system. > >> >This is based on the corresponding VMBus channel which assigned with the CPU. > >> > > >> >Now VMBus ISR is getting the interrupt in CPU0 but not getting from CPU1. > >> >Any idea, how we can use the same PPI 18 for all the CPU cores? > >> > > >> >Any help will be appreciated, as this is blocking the enablement of > >> >FreeBSD in Azure ARM64. > >> [Souradeep] > >> Can someone please help me it. > >> > > > >Looking at least at the GIC implementation, it looks like this is a known limitation: > > > > 875 /* > > 876 * XXX - In case that per CPU interrupt is going to be > >enabled in time > > 877 * when SMP is already started, we need some IPI > >call which > > 878 * enables it on others CPUs. Further, it's more > >complicated as > > 879 * pic_enable_source() and pic_disable_source() > >should act on > > 880 * per CPU basis only. Thus, it should be solved > >here somehow. > > 881 */ > > 882 if (isrc->isrc_flags & INTR_ISRCF_PPI) > > 883 CPU_SET(PCPU_GET(cpuid), &isrc->isrc_cpu); > > > >I think we need something /like/ this: > >https://people.fr/ > >eebsd.org%2F~kevans%2Fppi.diff&data=05%7C01%7Cschakrabarti%40microsoft.c > >om%7Cc5c3d254b9d841e9ae9b08db530bb3d2%7C72f988bf86f141af91ab2d7c > >d011db47%7C1%7C0%7C638195082027744706%7CUnknown%7CTWFpbGZsb3 > >d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D > >%7C3000%7C%7C%7C&sdata=SwoR2vHxh6QQhwpOgFcQ9378nDVovhdEKEXoEo > >gPKsc%3D&reserved=0, though it still has the caveat that PPIs effectively cannot be > >fully setup before SI_SUB_SMP. > >So, it's likely almost a NOP for existing platforms (will emit a warning with > >bootverbose for armv8 timers) but might do the trick for you. > [Souradeep] > Thanks for the change but it did not solve the problem. Still the interrupt handler > vmbus_handle_intr(struct trapframe *trap_frame), is not getting called for the CPU 1. > It is only getting called for CPU 0 all the time in ARM64 but in x86 it is getting called > for both CPU1 and CPU0. Interesting! I do see one problem with the patch (and some cosmetic issues): we really need to take the gic_mtx in gic_v3_setup_periph() right up front because CPU_SET() won't necessarily be atomic. That's not the problem, though for other reasons, but also because... > From DDB I have collected this data in arm64. irq18 is for vmbus. > db> show irqs > ... > irq18 <gic0,p2>: cpu 03 cnt 411 > .... > That would seem to indicate that both CPUs have set it up, but it occurs to me that enable_intr also needs the same treatment. Let's wipe gic_v3.c back to a clean slate and try a v2 of the patch: https://people.freebsd.org/~kevans/ppi-v2.diff For now we just pretend that we won't be disabling any PPIs as a proof-of-concept. Thanks, Kyle Evans