Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   KY Srinivasan <kys@microsoft.com>
To:     Thomas Gleixner <tglx@linutronix.de>
CC:     "x86@kernel.org" <x86@kernel.org>,
        "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "devel@linuxdriverproject.org" <devel@linuxdriverproject.org>,
        "olaf@aepfle.de" <olaf@aepfle.de>,
        "apw@canonical.com" <apw@canonical.com>,
        "jasowang@redhat.com" <jasowang@redhat.com>,
        "hpa@zytor.com" <hpa@zytor.com>,
        Stephen Hemminger <sthemmin@microsoft.com>,
        "Michael Kelley (EOSG)" <Michael.H.Kelley@microsoft.com>,
        "vkuznets@redhat.com" <vkuznets@redhat.com>
Subject: RE: [PATCH 3/5] X86: Hyper-V: Enhanced IPI enlightenment
Thread-Topic: [PATCH 3/5] X86: Hyper-V: Enhanced IPI enlightenment
Thread-Index: AQHT3MEtznURKON6iESarilPj8v1lqQTnwqAgACErqA=
Date:   Fri, 27 Apr 2018 06:24:21 +0000
Message-ID: <MW2PR2101MB111348D79C56945B695A04C8A08D0@MW2PR2101MB1113.namprd21.prod.outlook.com>
References: <20180425181110.8683-1-kys@linuxonhyperv.com>
 <20180425181250.8740-1-kys@linuxonhyperv.com>
 <20180425181250.8740-3-kys@linuxonhyperv.com>
 <alpine.DEB.2.21.1804270009080.1596@nanos.tec.linutronix.de>
In-Reply-To: <alpine.DEB.2.21.1804270009080.1596@nanos.tec.linutronix.de>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: microsoft.com does not designate
 permitted sender hosts)
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 45bf6fa6-a104-4e18-1070-08d5ac0783e8
X-MS-Exchange-CrossTenant-originalarrivaltime: 27 Apr 2018 06:24:21.8414
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 72f988bf-86f1-41af-91ab-2d7cd011db47
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW2PR2101MB0940
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> -----Original Message-----
> From: Thomas Gleixner <tglx@linutronix.de>
> Sent: Thursday, April 26, 2018 3:17 PM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: x86@kernel.org; gregkh@linuxfoundation.org; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de;
> apw@canonical.com; jasowang@redhat.com; hpa@zytor.com; Stephen
> Hemminger <sthemmin@microsoft.com>; Michael Kelley (EOSG)
> <Michael.H.Kelley@microsoft.com>; vkuznets@redhat.com
> Subject: Re: [PATCH 3/5] X86: Hyper-V: Enhanced IPI enlightenment
>=20
> On Wed, 25 Apr 2018, kys@linuxonhyperv.com wrote:
> >
> > +struct ipi_arg_ex {
> > +	u32 vector;
> > +	u32  reserved;
> > +	struct hv_vpset vp_set;
>=20
> Please align that in tabular fashion for easy of reading
>=20
> 	u32		vector;
> 	u32  		reserved;
> 	struct hv_vpset	vp_set;
>=20
> > +};
> > +
> >  static struct apic orig_apic;
> >
> >  static u64 hv_apic_icr_read(void)
> > @@ -97,6 +103,40 @@ static void hv_apic_eoi_write(u32 reg, u32 val)
> >   * IPI implementation on Hyper-V.
> >   */
> >
> > +static int __send_ipi_mask_ex(const struct cpumask *mask, int vector)
> > +{
> > +	int nr_bank =3D 0;
> > +	struct ipi_arg_ex **arg;
> > +	struct ipi_arg_ex *ipi_arg;
> > +	int ret =3D 1;
> > +	unsigned long flags;
>=20
> This is really horrible to read.
>=20
> 	struct ipi_arg_ex *ipi_arg;
> 	struct ipi_arg_ex **arg;
> 	unsigned long flags;
> 	bool ret =3D false;
> 	int nr_bank =3D 0;
>=20
> is really more conveniant for quick reading.
>=20
> So the other more limited function has a lot more sanity checks vs. vecto=
r
> number and other things. Why are they not required here? Comment
> please.

Yes, I will add the comments. This function is called from the other functi=
on
after all the sanity checks have been done and hence are not replicated her=
e.
>=20
> > +	local_irq_save(flags);
> > +	arg =3D (struct ipi_arg_ex **)this_cpu_ptr(hyperv_pcpu_input_arg);
> > +
> > +	ipi_arg =3D *arg;
> > +	if (unlikely(!ipi_arg))
> > +		goto ipi_mask_ex_done;
> > +
> > +	ipi_arg->vector =3D vector;
> > +	ipi_arg->reserved =3D 0;
> > +	ipi_arg->vp_set.valid_bank_mask =3D 0;
> > +
> > +	if (!cpumask_equal(mask, cpu_present_mask)) {
> > +		ipi_arg->vp_set.format =3D HV_GENERIC_SET_SPARCE_4K;
> > +		nr_bank =3D cpumask_to_vpset(&(ipi_arg->vp_set), mask);
>=20
> nr_bank really confused me. bank_nr is what you mean, not number of
> banks,
> right?
It is the number of banks. The hypercall used here is a variable length
hypercall.

Regards,

K. Y =20