Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 10 Sep 2021 18:36:32 +0000
From:   Wei Liu <wei.liu@kernel.org>
To:     Michael Kelley <mikelley@microsoft.com>
Cc:     Wei Liu <wei.liu@kernel.org>,
        Linux on Hyper-V List <linux-hyperv@vger.kernel.org>,
        KY Srinivasan <kys@microsoft.com>,
        Haiyang Zhang <haiyangz@microsoft.com>,
        Dexuan Cui <decui@microsoft.com>,
        Stephen Hemminger <sthemmin@microsoft.com>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@kernel.org>,
        "H. Peter Anvin" <hpa@zytor.com>,
        "open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)" 
        <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/2] x86/hyperv: remove on-stack cpumask from
 hv_send_ipi_mask_allbutself
Message-ID: <20210910183632.5i6lu2h23ljofdru@liuwe-devbox-debian-v2>
References: <20210908194243.238523-1-wei.liu@kernel.org>
 <20210908194243.238523-3-wei.liu@kernel.org>
 <MWHPR21MB1593AE1097D5312649A1D3B4D7D69@MWHPR21MB1593.namprd21.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <MWHPR21MB1593AE1097D5312649A1D3B4D7D69@MWHPR21MB1593.namprd21.prod.outlook.com>
Precedence: bulk

On Fri, Sep 10, 2021 at 05:25:15PM +0000, Michael Kelley wrote:
> From: Wei Liu <wei.liu@kernel.org> Sent: Wednesday, September 8, 2021 12:43 PM
> > 
[...]
> > -static bool __send_ipi_mask(const struct cpumask *mask, int vector)
> > +static bool __send_ipi_mask(const struct cpumask *mask, int vector,
> > +		bool exclude_self)
> >  {
> > -	int cur_cpu, vcpu;
> > +	int cur_cpu, vcpu, this_cpu = smp_processor_id();
> >  	struct hv_send_ipi ipi_arg;
> >  	u64 status;
> > 
> > @@ -172,6 +177,8 @@ static bool __send_ipi_mask(const struct cpumask *mask, int vector)
> >  	ipi_arg.cpu_mask = 0;
> > 
> >  	for_each_cpu(cur_cpu, mask) {
> > +		if (exclude_self && cur_cpu == this_cpu)
> > +			continue;
> >  		vcpu = hv_cpu_number_to_vp_number(cur_cpu);
> >  		if (vcpu == VP_INVAL)
> >  			return false;
> > @@ -191,7 +198,7 @@ static bool __send_ipi_mask(const struct cpumask *mask, int vector)
> >  	return hv_result_success(status);
> > 
> >  do_ex_hypercall:
> > -	return __send_ipi_mask_ex(mask, vector);
> > +	return __send_ipi_mask_ex(mask, vector, exclude_self);
> >  }
> 
> This all looks correct to me, except for one difference compared with the
> current code.  In the current code, if the cpumask passed to
> hv_send_ipi_mask_allbutself() indicates only a single CPU that is "self",
> __send_ipi_mask() will detect that the cpumask is now empty, and 
> correctly return success without making the hypercall.  But
> the new code will make the hypercall with an empty input mask (both
> in the SEND_IPI and SEND_IPI_EX cases).   The Hyper-V TLFS is silent
> on whether such a hypercall is a no-op that returns success or is an
> error.  We'll have a problem if it is an error.  I think the safest thing
> is to enhance the cpumask_empty() test at the beginning of
> __send_ipi_mask() to also detect the case where only a single CPU
> is specified, and it is "self".   This could be done using cpumask_weight()
> and checking for zero as the "empty" case.   Then check for "1", and if
> exclude_self is set, check if it is the "self" CPU.

Sure. Making this change should not be too difficult.

Wei.