From nobody Tue Apr 25 22:09:28 2023 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4Q5bl84CPkz46nHc; Tue, 25 Apr 2023 22:09:40 +0000 (UTC) (envelope-from kevans@freebsd.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Q5bl835mjz3l57; Tue, 25 Apr 2023 22:09:40 +0000 (UTC) (envelope-from kevans@freebsd.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1682460580; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=izFwLsKTBiI0Ua+BOCHCIwDmOvfYn8ShkRIJV2d82lg=; b=JPgQX7qDUUGvKFSDrdi1hhWk/x9jzuidWlumCQfaf62ddqPrBwchtRDLxbjeitKjvNAyyq CfOoaPJy8jKDS1AbJmGO4ydMBFXrBh9vG28GfTaY/tW2LF0h+6psWsR3K11Voste9hcuYY b3y3WyNl+BZ0bgqwzIO77trjEf/hteOwpD6h1wCuzWbBiDiNb3JGYaIAZCO+7UY1oFdq/v bB4IB3CjpvGPX4gIESUcz6oHRQp47fIpwRCtPeqzGjxkNLas3Vc04hzFxWpEQG1lX6QJ4d uC5DT4cNcLQDWHTvoBHBhn2YHJEwMKxjcFX0YbxgKSgeQd65itLdlyzw1C4ZAw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1682460580; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=izFwLsKTBiI0Ua+BOCHCIwDmOvfYn8ShkRIJV2d82lg=; b=WoQUCzo7g1m4LXmduJ7/UE/lzo1FJIYKpsCBs9ZnSY7CwjbhGvQuu4QLd6JWVEB3gq99z7 jJ81/rZUn+2nOBhA5GdfmfH5jRlVWaE/LaCLWqKiZU8qZOn6sAfOX3WAFbekJtsTg3pyJN jQyxzRFU4GbyNDao0PIDI11V4SkgmF4E2iOxtIte9DH5LAdCtmazGTxruA8TPagyyADMTI EcUCYt7gpGoIZKgFWmwRVtdcL7kr29tyKbMQVz9Cacg6HGjY3D7S1JFXIwAqdAlXAGfrC1 KDQxBW2d75sFynv+n1zmcrllWTIwslYGOBG6s/FdsHfSrGPT9/jXN96ChVqhwA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1682460580; a=rsa-sha256; cv=none; b=YQSqDtYIgyrnZYhnXpLyAI5LWAZhb6U+Zond02oc02NkgTz3Re/9F6BgdnIUqkPm+GE75T c7bKygSq38c7ZfSbHbeypPUsQU1z411pSskqM9vepxRIeIKvhGU+ZJQTMhtD/JeeXQMT2C tRE+6N+3X4WmW37tLzj7VpczMfEZ2D6TkxRZCpy+Du5r83fIu+fNg1Ts1LXMlpIR29B172 XE+hDpmXA18XO7naGz9nfqc7OPnVP6Z+orCET4fWw1oa8K3ly0/yJ896lcjZewIKTyk72n MTHOkYpMxx13KzE2pFg8BUhAT7xocGA0LqxbzdHuT3BQ96z4XP0ACHOmwZYN3Q== Received: from mail-qk1-f177.google.com (mail-qk1-f177.google.com [209.85.222.177]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) (Authenticated sender: kevans) by smtp.freebsd.org (Postfix) with ESMTPSA id 4Q5bl821fwzJvg; Tue, 25 Apr 2023 22:09:40 +0000 (UTC) (envelope-from kevans@freebsd.org) Received: by mail-qk1-f177.google.com with SMTP id af79cd13be357-74ab718c344so1824091285a.1; Tue, 25 Apr 2023 15:09:40 -0700 (PDT) X-Gm-Message-State: AC+VfDxhV/ZFVE3lHx8812PC+OplKI7UNnNt+kN99F2mHAdstwTKok/0 OLVWKUCN1h9cUZk0HZTh2q7i5nsiq6z8dWcS6F8= X-Google-Smtp-Source: ACHHUZ5OlADj8DLKWnEmVPK2onQ451D97kXMwuPHEGkQgUflcLPESlaJ1wWkm9h3rt5FKlrDAuUSn/FlcGnzGBgNFVk= X-Received: by 2002:a05:6214:c8c:b0:5ef:653e:169b with SMTP id r12-20020a0562140c8c00b005ef653e169bmr768705qvr.8.1682460579558; Tue, 25 Apr 2023 15:09:39 -0700 (PDT) List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 References: <202210271354.29RDsUoH077155@gitrepo.freebsd.org> In-Reply-To: From: Kyle Evans Date: Tue, 25 Apr 2023 17:09:28 -0500 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [EXTERNAL] Re: git: 9729f076e4d9 - main - arm64: Hyper-V: enablement for ARM64 in Hyper-V (Part 3, final) To: Souradeep Chakrabarti Cc: Kyle Evans , Wei Hu , "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@freebsd.org" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-ThisMailContainsUnwantedMimeParts: N On Tue, Apr 25, 2023 at 2:42=E2=80=AFAM Souradeep Chakrabarti wrote: > > > > > >-----Original Message----- > >From: Souradeep Chakrabarti > >Sent: Monday, April 24, 2023 6:24 PM > >To: Kyle Evans ; Wei Hu > >Cc: src-committers@freebsd.org; dev-commits-src-all@freebsd.org; dev-com= mits- > >src-main@freebsd.org > >Subject: RE: [EXTERNAL] Re: git: 9729f076e4d9 - main - arm64: Hyper-V: > >enablement for ARM64 in Hyper-V (Part 3, final) > > > > > > > > > >>-----Original Message----- > >>From: Kyle Evans > >>Sent: Wednesday, April 19, 2023 11:00 AM > >>To: Wei Hu ; Souradeep Chakrabarti > >> > >>Cc: src-committers@freebsd.org; dev-commits-src-all@freebsd.org; > >>dev-commits- src-main@freebsd.org > >>Subject: [EXTERNAL] Re: git: 9729f076e4d9 - main - arm64: Hyper-V: > >>enablement for ARM64 in Hyper-V (Part 3, final) > >> > >>On Thu, Oct 27, 2022 at 8:54=E2=80=AFAM Wei Hu wrote: > >>> > >>> The branch main has been updated by whu: > >>> > >>> URL: > >>> https://nam06.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fc= gi > >>> t > >>> > >>.freebsd.org%2Fsrc%2Fcommit%2F%3Fid%3D9729f076e4d93c5a37e78d427bfe > >0 > >>f1a > >>> > >>b99bbcc6&data=3D05%7C01%7Cschakrabarti%40microsoft.com%7C393e855f13c6 > >>49a > >>> > >>8883708db4097211e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7 > >C > >>6381747 > >>> > >>90106786449%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIj > >oi > >>V2luMz > >>> > >>IiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=3DhuiaOBSB2Z > >>QDn5Q > >>> cw%2FLWnmI%2FqbbriQd7HkYtpxnGLGE%3D&reserved=3D0 > >>> > >>> commit 9729f076e4d93c5a37e78d427bfe0f1ab99bbcc6 > >>> Author: Souradeep Chakrabarti > >>> AuthorDate: 2022-10-27 13:46:08 +0000 > >>> Commit: Wei Hu > >>> CommitDate: 2022-10-27 13:53:22 +0000 > >>> > >>> arm64: Hyper-V: enablement for ARM64 in Hyper-V (Part 3, final) > >>> > >>> This is the last part for ARM64 Hyper-V enablement. This includes > >>> commone files and make file changes to enable the ARM64 FreeBSD > >>> guest on Hyper-V. With this patch, it should be able to build > >>> the ARM64 image and install it on Hyper-V. > >>> > >> > >>Hi, > >> > >>First off- thanks for doing this work! I can't seem to boot a -CURRENT > >>image under Hyper-V on a Volterra machine, seemingly due to vmbus. It s= talls right > >after "vmbus: > >>the irq 18" but before emitting a version number, I have some other > >>comments here... > >> > >>> [... snip ...] > >>> diff --git a/sys/dev/hyperv/vmbus/vmbus.c > >>> b/sys/dev/hyperv/vmbus/vmbus.c index b0cd750b26c8..f370f2a75b99 > >>> 100644 > >>> --- a/sys/dev/hyperv/vmbus/vmbus.c > >>> +++ b/sys/dev/hyperv/vmbus/vmbus.c > >>> [... snip ...] > >>> @@ -107,7 +113,7 @@ static uint32_t > >>vmbus_get_vcpu_id_method(device_t bus, > >>> device_t dev, int cpu); > >>> static struct taskqueue *vmbus_get_eventtq_method(dev= ice_t, device_t, > >>> int); -#ifdef EARLY_AP_STARTUP > >>> +#if defined(EARLY_AP_STARTUP) || defined(__aarch64__) > >>> static void vmbus_intrhook(void *); > >>> #endif > >>> > >> > >>My gut reaction to this is that this is a red flag. EARLY_AP_STARTUP > >>implies characteristics that aarch64 doesn't exhibit; it's hard to see > >>why this conditional is OK, or whether it's testing EARLY_AP_STARTUP as > >>a bad way to write __i386__ || __amd64__. > >[Souradeep] We had or'd the aarch64 here, as we wanted to defer the vmbu= s > >attachment after gic and arm generic timer is attached. As we need PAUSE= and > >DELAY for vmbus and hyper-v drivers. For that we wanted to use > >vmbus_intrhook(). > >> > >>> [... snip ...] > >>> @@ -760,7 +736,7 @@ vmbus_synic_setup(void *xsc) > >>> > >>> if (hyperv_features & CPUID_HV_MSR_VP_INDEX) { > >>> /* Save virtual processor id. */ > >>> - VMBUS_PCPU_GET(sc, vcpuid, cpu) =3D rdmsr(MSR_HV_VP_I= NDEX); > >>> + VMBUS_PCPU_GET(sc, vcpuid, cpu) =3D > >>> + RDMSR(MSR_HV_VP_INDEX); > >>> } else { > >>> /* Set virtual processor id to 0 for compatibility. *= / > >>> VMBUS_PCPU_GET(sc, vcpuid, cpu) =3D 0; > >> > >>This one, vmbus_synic_setup(), is invoked via vmbus_intrhook -> > >>vmbus_doattach - > >>> smp_rendezvous. On !EARLY_AP_STARTUP (e.g., aarch64), SMP isn't > >>> functional in > >>intrhooks and smp_rendezvous() will just call vmbus_synic_setup() on th= e boot > >AP. > >>There's nothing that will initialize the pcpu data on every other AP, A= FAICT. > >> > >>That said, the !EARLY_AP_STARTUP path is also wrong. Quoting lines that > >>weren't in this e-mail from vmbus_attach(): > >[Souradeep] Thanks! This is a really good catch, and yes, we are missing= the smp > >initialization during vmbus synic setup. > >> > >>1527 #else /* !EARLY_AP_STARTUP */ > >>1528 /* > >>1529 * If the system has already booted and thread > >>1530 * scheduling is possible indicated by the global > >>1531 * cold set to zero, we just call the driver > >>1532 * initialization directly. > >>1533 */ > >>1534 if (!cold) > >>1535 vmbus_doattach(vmbus_sc); > >>1536 #endif /* EARLY_AP_STARTUP and aarch64 */ > >> > >>The two immediate issues I see is that in a device_attach, SMP won't be > >>started. It's also going to be before SI_SUB_CONFIGURE, so `cold` will > >>never be 0 here -- this is effectively dead code, and if it weren't > >>dead code it would suffer the same problem as above where other APs > >>aren't started yet so we won't set their pcpu data. This is OK, though, > >>because then we have this sysinit later on that does the same thing for > >>!EARLY_AP_STARTUP && !__aarch64__`, but that should really just be > >>`!EARLY_AP_STARTUP` as aarch64 also needs to invoke > >>vmbus_doattach() at SI_SUB_SMP (much later than SI_SUB_DRIVERS). > >[Souradeep] I have tried to use the suggestion but in multi processor sy= stem the > >boot is getting stuck at the end and "vmbus0: device scan, probe and att= ach done" > >is not happening. > >I am trying to figure out what is going wrong here, as the same change i= s working > >with single cpu system. Any help or pointer will be really helpful. > [Souradeep] I can see the problem the task vmbus_scandone_task is gettin= g enqueued but > it is not getting executed. > > 525 static void > 526 vmbus_scan_done(struct vmbus_softc *sc, > 527 const struct vmbus_message *msg __unused) > 528 { > 529 device_printf(sc->vmbus_dev, " vmbus_scan_done is called\n"); > 530 > 531 taskqueue_enqueue(sc->vmbus_devtq, &sc->vmbus_scandone_task); > 532 } > vmbus scan done is getting called. > > But > 514 static void > 515 vmbus_scan_done_task(void *xsc, int pending __unused) > 516 { > 517 struct vmbus_softc *sc =3D xsc; > 518 device_printf(sc->vmbus_dev, " vmbus_scan_done_task is called\n"= ); > 519 bus_topo_lock(); > 520 sc->vmbus_scandone =3D true; > 521 bus_topo_unlock(); > 522 wakeup(&sc->vmbus_scandone); > 523 } > vmbus_scan_done_task is not getting called the task . > > But the same is happening when single cpu is running, for multi cpu it is= getting stuck. Hi, That seems odd. What happens if you bump the SYSINIT up to SI_SUB_SMP + 1, SI_ORDER_FIRST? We don't know for a fact that all APs are ready for scheduling until after smp_after_idle_runnable(), which is also at SI_ORDER_ANY -- maybe there's just something going horribly wrong. That would perhaps explain why it's fine on a single processor system, which won't do anything useful (at least in later parts of SI_SUB_SMP). Thanks, Kyle Evans