Received-SPF: pass (google.com: domain of linux-kernel+bounces-95013-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <20240307022006.3657433-1-aik@amd.com> <32ac2ee6-26b1-4f37-bd88-db1ef2a31267@linux.intel.com>
 <16120a31-c3d6-48a8-9355-7b063e28d242@amd.com>
In-Reply-To: <16120a31-c3d6-48a8-9355-7b063e28d242@amd.com>
From: Sathyanarayanan Kuppuswamy Natarajan <sathyanarayanan.nkuppuswamy@gmail.com>
Date: Wed, 6 Mar 2024 22:22:46 -0800
Message-ID: <CAC41dw9x8H2m=YB66+PA-GwiVkpCyT0PobdWvFXLHpAsQOE0ew@mail.gmail.com>
Subject: Re: [PATCH kernel v3] PCI/DOE: Support discovery version 2
To: Alexey Kardashevskiy <aik@amd.com>
Cc: linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, 
	Bjorn Helgaas <bhelgaas@google.com>, Ira Weiny <ira.weiny@intel.com>, 
	Dan Williams <dan.j.williams@intel.com>, Jonathan Cameron <Jonathan.Cameron@huawei.com>, 
	Lukas Wunner <lukas@wunner.de>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Mar 6, 2024 at 10:09=E2=80=AFPM Alexey Kardashevskiy <aik@amd.com> =
wrote:
>
>
>
> On 7/3/24 16:06, Kuppuswamy Sathyanarayanan wrote:
> >
> > On 3/6/24 6:20 PM, Alexey Kardashevskiy wrote:
> >> PCIe r6.1, sec 6.30.1.1 defines a "DOE Discovery Version" field in
> >> the DOE Discovery Request Data Object Contents (3rd DW) as:
> >>
> >> 15:8 DOE Discovery Version =E2=80=93 must be 02h if the Capability Ver=
sion in
> >> the Data Object Exchange Extended Capability is 02h or greater.
> >
> > Does this change fix any functional issue? If yes, please add Fixes:
> > tag and probably CC stable?
>
> The issue is that before this patch the DOE driver was following PCIe
> r6.0 and it was not working on a device with DOE cap v2 added in r6.1.
> Is it "fixes"?
>

Since you are enabling support for the newer spec version, I think it will =
not
fall under bug fix. But it might be qualified for the stable branch.
It depends on
whether you want to support this device in older(stable) kernels.

>
> >>
> >> Add support for the version on devices with the DOE v2 capability.
> >>
> >> Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
> >> ---
> > Change wise looks fine to me.
> >
> > Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@lin=
ux.intel.com>
>
> Thanks!
>
> >> Changes:
> >> v3:
> >> * updated subject line
> >> * dropped "DISCOVER_" from the new field macro
> >>
> >> v2:
> >> * added the section number to the commit log
> >> ---
> >>   include/uapi/linux/pci_regs.h |  1 +
> >>   drivers/pci/doe.c             | 11 ++++++++---
> >>   2 files changed, 9 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_re=
gs.h
> >> index a39193213ff2..fbca743b2b86 100644
> >> --- a/include/uapi/linux/pci_regs.h
> >> +++ b/include/uapi/linux/pci_regs.h
> >> @@ -1144,6 +1144,7 @@
> >>   #define PCI_DOE_DATA_OBJECT_HEADER_2_LENGTH                0x0003fff=
f
> >>
> >>   #define PCI_DOE_DATA_OBJECT_DISC_REQ_3_INDEX               0x000000f=
f
> >> +#define PCI_DOE_DATA_OBJECT_DISC_REQ_3_VER          0x0000ff00
> >>   #define PCI_DOE_DATA_OBJECT_DISC_RSP_3_VID         0x0000ffff
> >>   #define PCI_DOE_DATA_OBJECT_DISC_RSP_3_PROTOCOL            0x00ff000=
0
> >>   #define PCI_DOE_DATA_OBJECT_DISC_RSP_3_NEXT_INDEX  0xff000000
> >> diff --git a/drivers/pci/doe.c b/drivers/pci/doe.c
> >> index 61f0531d2b1d..c94c2b0767f3 100644
> >> --- a/drivers/pci/doe.c
> >> +++ b/drivers/pci/doe.c
> >> @@ -381,11 +381,13 @@ static void pci_doe_task_complete(struct pci_doe=
_task *task)
> >>      complete(task->private);
> >>   }
> >>
> >> -static int pci_doe_discovery(struct pci_doe_mb *doe_mb, u8 *index, u1=
6 *vid,
> >> +static int pci_doe_discovery(struct pci_doe_mb *doe_mb, u8 capver, u8=
 *index, u16 *vid,
> >>                           u8 *protocol)
> >>   {
> >>      u32 request_pl =3D FIELD_PREP(PCI_DOE_DATA_OBJECT_DISC_REQ_3_INDE=
X,
> >> -                                *index);
> >> +                                *index) |
> >> +                     FIELD_PREP(PCI_DOE_DATA_OBJECT_DISC_REQ_3_VER,
> >> +                                (capver >=3D 2) ? 2 : 0);
> >>      __le32 request_pl_le =3D cpu_to_le32(request_pl);
> >>      __le32 response_pl_le;
> >>      u32 response_pl;
> >> @@ -419,13 +421,16 @@ static int pci_doe_cache_protocols(struct pci_do=
e_mb *doe_mb)
> >>   {
> >>      u8 index =3D 0;
> >>      u8 xa_idx =3D 0;
> >> +    u32 hdr =3D 0;
> >> +
> >> +    pci_read_config_dword(doe_mb->pdev, doe_mb->cap_offset, &hdr);
> >>
> >>      do {
> >>              int rc;
> >>              u16 vid;
> >>              u8 prot;
> >>
> >> -            rc =3D pci_doe_discovery(doe_mb, &index, &vid, &prot);
> >> +            rc =3D pci_doe_discovery(doe_mb, PCI_EXT_CAP_VER(hdr), &i=
ndex, &vid, &prot);
> >>              if (rc)
> >>                      return rc;
> >>
> >
>
> --
> Alexey
>


--=20
Sathyanarayanan Kuppuswamy
Linux Kernel Developer