linux-kernel - Re: [RFC 1/9] PCI: pci-bridge-emul: Update PCIe register behaviors

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:   Sat, 8 Feb 2020 11:55:43 +0200
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Jon Derrick <jonathan.derrick@...el.com>
Cc:     Bjorn Helgaas <helgaas@...nel.org>, linux-pci@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Andy Shevchenko <andriy.shevchenko@...el.com>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        Pawel Baldysiak <pawel.baldysiak@...el.com>,
        Sinan Kaya <okaya@...nel.org>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Keith Busch <kbusch@...nel.org>,
        Alexandru Gagniuc <mr.nuke.me@...il.com>,
        Christoph Hellwig <hch@....de>
Subject: Re: [RFC 1/9] PCI: pci-bridge-emul: Update PCIe register behaviors

On Sat, Feb 8, 2020 at 2:02 AM Jon Derrick <jonathan.derrick@...el.com> wrote:
>
> Update the PCIe register behaviors and comments for PCIe v5.0.

I think you may elaborate the changes here, like a summary.

> Replace the specific bit definitions with BIT and GENMASK to make
> updating easier in the future.

...

> +                * Device status register has bits 6 and [3:0] W1C, [5:4] RO,
> +                * the rest is reserved

Perhaps it makes sense to add '.' (period) to the end of sentence. And
the same for the rest comments.

...

> -               .w1c = (PCI_EXP_SLTSTA_ABP | PCI_EXP_SLTSTA_PFD |
> -                       PCI_EXP_SLTSTA_MRLSC | PCI_EXP_SLTSTA_PDC |
> -                       PCI_EXP_SLTSTA_CC | PCI_EXP_SLTSTA_DLLSC) << 16,
> -               .ro = (PCI_EXP_SLTSTA_MRLSS | PCI_EXP_SLTSTA_PDS |
> -                      PCI_EXP_SLTSTA_EIS) << 16,

> +               .w1c = (BIT(8) | GENMASK(4, 0)) << 16,
> +               .ro = GENMASK(7, 5) << 16,

I'm not sure the new one is better. Perhaps you need to add
description for the new bits and, if you consider it's needed, add a
picture of the bits in the comment, like  XXXX rwX1 XXrX XwXX, where r
= ro, w = rw, 1 = w1c, X = rsvd. But it's up to Bjorn and you, I have
no strong opinion here.

Same for the rest similar changes.

...

> -               .rw = (PCI_EXP_RTCTL_SECEE | PCI_EXP_RTCTL_SENFEE |
> -                      PCI_EXP_RTCTL_SEFEE | PCI_EXP_RTCTL_PMEIE |
> -                      PCI_EXP_RTCTL_CRSSVE),
> -               .ro = PCI_EXP_RTCAP_CRSVIS << 16,
> +               .rw = GENMASK(4, 0),
> +               .ro = BIT(0) << 16,

Bit 0 in both rw and ro -- is it correct?

-- 
With Best Regards,
Andy Shevchenko