linux-kernel - Re: [PATCH 05/42] PCI: pci-bridge-emul: Add PCIe Root Capabilities Register

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: <20210507144015.bol3qeqsdoo7nmju@pali>
Date:   Fri, 7 May 2021 16:40:15 +0200
From:   Pali Rohár <pali@...nel.org>
To:     Bjorn Helgaas <helgaas@...nel.org>
Cc:     Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        Rob Herring <robh@...nel.org>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Russell King <rmk+kernel@...linux.org.uk>,
        Marek Behún <kabel@...nel.org>,
        Remi Pommarel <repk@...plefau.lt>, Xogium <contact@...ium.me>,
        Tomasz Maciej Nowak <tmn505@...il.com>,
        Marc Zyngier <maz@...nel.org>, linux-pci@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 05/42] PCI: pci-bridge-emul: Add PCIe Root Capabilities
 Register

On Thursday 06 May 2021 18:10:09 Bjorn Helgaas wrote:
> On Thu, May 06, 2021 at 05:31:16PM +0200, Pali Rohár wrote:
> > This is 16-bit register at offset 0x1E. Rename current 'rsvd' struct member
> > to 'rootcap'.
> 
> "The 16-bit Root Capabilities register is at offset 0x1e in the PCIe
> Capability."
> 
> Please make the commit log complete in itself.  In some contexts, the
> subject line is not visible at the same time.  It's fine to repeat the
> subject in the commit log.
> 
> > Signed-off-by: Pali Rohár <pali@...nel.org>
> > Reviewed-by: Marek Behún <kabel@...nel.org>
> > Fixes: 23a5fba4d941 ("PCI: Introduce PCI bridge emulated config space common logic")
> > Cc: stable@...r.kernel.org # e0d9d30b7354 ("PCI: pci-bridge-emul: Fix big-endian support")
> 
> I'm not sure how people would deal with *two* SHA1s.

I guess that this is fine per stable document as it mention such example:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html

I have already in past sent patches with Fixes:hash1 and CC:stable/hash2
and were taken correctly.

> This patch adds functionality, so it's not really fixing a bug in
> 23a5fba4d941.

I'm not sure what is the correct meaning of Fixes tag. I included it to
easily determinate in which commit was introduced member name "rsvd"
which should have been named "rootcap".

Submitting patches document is not fully clear for me as I understood it
that Fixes and CC:stable are two different things. E.g. it mention
"Attaching a Fixes: tag does not subvert ... the requirement to Cc:
stable@...r.kernel.org on all stable patch candidates." which I
understood that patch for backporting needs to have Cc:stable:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html

But I will change it as needed. Just I did not know what is "the correct
way".

> I see that e0d9d30b7354 came along later and did
> "s/u16 rsvd/__le16 rsvd/".
> 
> But it seems like a lot to expect for distros and stable kernel
> maintainers to interpret this.
> 
> Personally I think I would omit both Fixes: and the stable tag since
> these two patches (05 and 06) are just adding functionality.
> 
> > ---
> >  drivers/pci/pci-bridge-emul.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/pci-bridge-emul.h b/drivers/pci/pci-bridge-emul.h
> > index b31883022a8e..49bbd37ee318 100644
> > --- a/drivers/pci/pci-bridge-emul.h
> > +++ b/drivers/pci/pci-bridge-emul.h
> > @@ -54,7 +54,7 @@ struct pci_bridge_emul_pcie_conf {
> >  	__le16 slotctl;
> >  	__le16 slotsta;
> >  	__le16 rootctl;
> > -	__le16 rsvd;
> > +	__le16 rootcap;
> >  	__le32 rootsta;
> >  	__le32 devcap2;
> >  	__le16 devctl2;
> > -- 
> > 2.20.1
> >