linux-kernel - Re: [PATCH 3/3] PCI: hv: Use vPCI protocol version 1.2 for v4.9

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, 18 Jan 2019 14:05:46 +0000
From:   Ajay Kaher <akaher@...are.com>
To:     Bjorn Helgaas <helgaas@...nel.org>
CC:     "kys@...rosoft.com" <kys@...rosoft.com>,
        "haiyangz@...rosoft.com" <haiyangz@...rosoft.com>,
        "devel@...uxdriverproject.org" <devel@...uxdriverproject.org>,
        "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Jork Loeser <jloeser@...rosoft.com>,
        "longli@...rosoft.com" <longli@...rosoft.com>
Subject: Re: [PATCH 3/3] PCI: hv: Use vPCI protocol version 1.2 for v4.9



> On 17/01/19, 8:37 PM, "Bjorn Helgaas" <helgaas@...nel.org> wrote:
>
> > On Fri, Jan 18, 2019 at 02:17:18AM +0530, Ajay Kaher wrote:
> > Update the Hyper-V vPCI driver to use the Server-2016 version of the vPCI
> > protocol, fixing MSI creation and retargeting issues.
> > 
> > Replaced hv_tmp_cpu_nr_to_vp_nr() with vmbus_cpu_number_to_vp_number()
> > to make this patch compatibale for linux v4.9.
>    
> s/compatibale for/compatible with/
>
> This change (to make it compatible with v4.9) sounds like it should be
> in its own separate patch.  I don't see any use of
> hv_tmp_cpu_nr_to_vp_nr() being removed, so the changelog doesn't quite
> make sense.

hv_tmp_cpu_nr_to_vp_nr() is not removed, v4.9 doesn't have this API,
and because of this failed to apply the original patch on v4.9. 
That's the reason to use vmbus_cpu_number_to_vp_number() 
instead of hv_tmp_cpu_nr_to_vp_nr().

I will change the explanation in changelog to:
Because v4.9 doesn't have hv_tmp_cpu_nr_to_vp_nr(), 
so original patch from v4.14 fails to apply on v4.9.
To solve this, using vmbus_cpu_number_to_vp_number() 
instead of hv_tmp_cpu_nr_to_vp_nr() in this patch.

Here, I would like maintainer of HyperV to review this change and 
provide their feedback. Tested these patches on Azure Server with
VM of size L16s_v2 and it's working fine.
    
> > Signed-off-by: Jork Loeser <jloeser@...rosoft.com>
> > Signed-off-by: Bjorn Helgaas <bhelgaas@...gle.com>
>   
> I did not sign off on this; please remove.

Sorry, I kept as it is from original patch. I will change this in next version of this patch.
    
> > Reviewed-by: K. Y. Srinivasan <kys@...rosoft.com>
> > Acked-by: K. Y. Srinivasan <kys@...rosoft.com>
> > Signed-off-by: Ajay Kaher <akaher@...are.com>
>    
> > + * struct hv_msi_desc2 - 1.2 version of hv_msi_desc
> > + * @vector:		IDT entry
> > + * @delivery_mode:	As defined in Intel's Programmer's
> > + *			Reference Manual, Volume 3, Chapter 8.
> > + * @vector_count:	Number of contiguous entries in the
> > + *			Interrupt Descriptor Table that are
> > + *			occupied by this Message-Signaled
> > + *			Interrupt. For "MSI", as first defined
> > + *			in PCI 2.2, this can be between 1 and
> > + *			32. For "MSI-X," as first defined in PCI
> > + *			3.0, this must be 1, as each MSI-X table
> > + *			entry would have its own descriptor.
>  
> Please reflow these descriptions to take advantage of an 80 column
> width.  They are currently wrapped to fit in 50 columns, which is
> unnecessarily short.

Thanks, I will change this in next version of this patch.
    
> > +	default:
> > +		/* As we only negotiate protocol versions known to this driver,
> > +		 * this path should never hit. However, this is it not a hot
> > +		 * path so we print a message to aid future updates.
> > +		 */
> > +		dev_err(&hbus->hdev->device,
> > +			"Unexpected vPCI protocol, update driver.");
    
> Include the actual protocol version in the message?

Thanks, I will try to change this in next version.