linux-kernel - RE: [PATCH 3/5] X86: Hyper-V: Enhanced IPI enlightenment

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 27 Apr 2018 06:24:21 +0000
From:   KY Srinivasan <kys@...rosoft.com>
To:     Thomas Gleixner <tglx@...utronix.de>
CC:     "x86@...nel.org" <x86@...nel.org>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "devel@...uxdriverproject.org" <devel@...uxdriverproject.org>,
        "olaf@...fle.de" <olaf@...fle.de>,
        "apw@...onical.com" <apw@...onical.com>,
        "jasowang@...hat.com" <jasowang@...hat.com>,
        "hpa@...or.com" <hpa@...or.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        "Michael Kelley (EOSG)" <Michael.H.Kelley@...rosoft.com>,
        "vkuznets@...hat.com" <vkuznets@...hat.com>
Subject: RE: [PATCH 3/5] X86: Hyper-V: Enhanced IPI enlightenment



> -----Original Message-----
> From: Thomas Gleixner <tglx@...utronix.de>
> Sent: Thursday, April 26, 2018 3:17 PM
> To: KY Srinivasan <kys@...rosoft.com>
> Cc: x86@...nel.org; gregkh@...uxfoundation.org; linux-
> kernel@...r.kernel.org; devel@...uxdriverproject.org; olaf@...fle.de;
> apw@...onical.com; jasowang@...hat.com; hpa@...or.com; Stephen
> Hemminger <sthemmin@...rosoft.com>; Michael Kelley (EOSG)
> <Michael.H.Kelley@...rosoft.com>; vkuznets@...hat.com
> Subject: Re: [PATCH 3/5] X86: Hyper-V: Enhanced IPI enlightenment
> 
> On Wed, 25 Apr 2018, kys@...uxonhyperv.com wrote:
> >
> > +struct ipi_arg_ex {
> > +	u32 vector;
> > +	u32  reserved;
> > +	struct hv_vpset vp_set;
> 
> Please align that in tabular fashion for easy of reading
> 
> 	u32		vector;
> 	u32  		reserved;
> 	struct hv_vpset	vp_set;
> 
> > +};
> > +
> >  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 = 0;
> > +	struct ipi_arg_ex **arg;
> > +	struct ipi_arg_ex *ipi_arg;
> > +	int ret = 1;
> > +	unsigned long flags;
> 
> This is really horrible to read.
> 
> 	struct ipi_arg_ex *ipi_arg;
> 	struct ipi_arg_ex **arg;
> 	unsigned long flags;
> 	bool ret = false;
> 	int nr_bank = 0;
> 
> is really more conveniant for quick reading.
> 
> So the other more limited function has a lot more sanity checks vs. vector
> 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 function
after all the sanity checks have been done and hence are not replicated here.
> 
> > +	local_irq_save(flags);
> > +	arg = (struct ipi_arg_ex **)this_cpu_ptr(hyperv_pcpu_input_arg);
> > +
> > +	ipi_arg = *arg;
> > +	if (unlikely(!ipi_arg))
> > +		goto ipi_mask_ex_done;
> > +
> > +	ipi_arg->vector = vector;
> > +	ipi_arg->reserved = 0;
> > +	ipi_arg->vp_set.valid_bank_mask = 0;
> > +
> > +	if (!cpumask_equal(mask, cpu_present_mask)) {
> > +		ipi_arg->vp_set.format = HV_GENERIC_SET_SPARCE_4K;
> > +		nr_bank = cpumask_to_vpset(&(ipi_arg->vp_set), mask);
> 
> 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