linux-kernel - RE: [EXTERNAL] Re: [PATCH v2 1/2] PCI: hv: Make the code arch neutral

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]

Message-ID: <MW4PR21MB2002FF56AE6443ED2821F555C0B29@MW4PR21MB2002.namprd21.prod.outlook.com>
Date:   Fri, 8 Oct 2021 19:55:00 +0000
From:   Sunil Muthuswamy <sunilmut@...rosoft.com>
To:     Bjorn Helgaas <helgaas@...nel.org>
CC:     Marc Zyngier <maz@...nel.org>,
        Michael Kelley <mikelley@...rosoft.com>,
        Boqun Feng <Boqun.Feng@...rosoft.com>,
        KY Srinivasan <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        Dexuan Cui <decui@...rosoft.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Rob Herring <robh@...nel.org>,
        "Krzysztof Wilczyński" <kw@...ux.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Wei Liu <wei.liu@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "\"H. Peter Anvin\"" <hpa@...or.com>,
        Arnd Bergmann <arnd@...db.de>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [EXTERNAL] Re: [PATCH v2 1/2] PCI: hv: Make the code arch neutral

On Friday, October 8, 2021 12:17 PM
Bjorn Helgaas <helgaas@...nel.org> wrote:

> Can you put some specifics in the subject line, please?  If this patch
> does several things, that might be an indication that it should be
> split into several patches.
> 
Will do in v3. But, this patch only makes the current Hyper-V vPCI code
architectural neutral by adding those interfaces that you mention below.
I will update the wording to make this clear in v3. thanks for the suggestion.

> > This patch makes the Hyper-V vPCI code architectural neutral by
> > introducing an irqchip that takes care of architectural
> > dependencies. This allows for the implementation of Hyper-V vPCI
> > for other architecture such as ARM64.
> 
> No need to include "This patch"; we already know we're talking about
> this patch.
> 
> Write in "imperative mood", e.g., "Encapsulate arch dependencies in
> X ..." instead of "This patch makes the code ...".  See
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fchris.be
> ams.io%2Fposts%2Fgit-
> commit%2F&amp;data=04%7C01%7Csunilmut%40microsoft.com%7Cb40962bf1
> 5614957781608d98a9030c8%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C
> 0%7C637693174187863024%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
> wMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sda
> ta=XlrM%2Bzg92v%2BXxXObWQbznaB%2F6ZW6u79NtDoG4FfyhXw%3D&amp;r
> eserved=0
> 
> Wrap the text to fill 75 columns.
> 
All of the above points noted; thanks. Will address in v3.

> You said this "introduces an irqchip", but I don't see a new
> struct irq_chip or similar.
> 
> The important part about making this arch-neutral seems to be adding
> these interfaces that will encapsulate arch dependencies:
> 
>   hv_pci_irqchip_init()
>   hv_pci_irqchip_free()
>   hv_msi_get_int_vector()
>   hv_set_msi_entry_from_desc()
>   hv_msi_prepare()
> 
Yes, the wording of 'irqchip' is a bit misleading for this patch as it gets
added in 2/2. Thanks for the suggestion; I will reword in v3.

> I'm not sure wrapping them in "#ifdef CONFIG_X86_64" is the best
> approach, but the IRQ folks will know better.
> 
Hyper-V vPCI is not firmware enumerated and so we cannot take a
dependency on the firmware to dictate whether or not to instantiate
an irqchip or not. I am open to suggestions though.

> > +++ b/drivers/pci/controller/pci-hyperv-irqchip.c
> > @@ -0,0 +1,51 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> 
> Spurious blank line (follow style of nearby files).
> 
Noted, thanks. Will fix in v3.

> > +/*
> > + * Hyper-V vPCI irqchip.
> 
> > +++ b/drivers/pci/controller/pci-hyperv-irqchip.h
> > @@ -0,0 +1,21 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +
> 
> Spurious blank line (follow style of nearby files).
> 
Noted, thanks. Will fix in v3.

- Sunil