CXL and AER drivers need the ability to identify CXL devices and CXL port
devices.
First, add set_pcie_cxl() with logic checking for CXL Flexbus DVSEC
presence. The CXL Flexbus DVSEC presence is used because it is required
for all the CXL PCIe devices.[1]
Add boolean 'struct pci_dev::is_cxl' with the purpose to cache the CXL
Flexbus presence.
Add pcie_is_cxl() as a macro to return 'struct pci_dev::is_cxl'.
Add pcie_is_cxl_port() to check if a device is a CXL Root Port, CXL
Upstream Switch Port, or CXL Downstream Switch Port. Also, verify the
CXL Extensions DVSEC for Ports is present.[1]
[1] CXL 3.1 Spec, 8.1.1 PCIe Designated Vendor-Specific Extended
Capability (DVSEC) ID Assignment, Table 8-2
Signed-off-by: Terry Bowman <terry.bowman@amd.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Fan Ni <fan.ni@samsung.com>
---
drivers/pci/pci.c | 13 +++++++++++++
drivers/pci/probe.c | 10 ++++++++++
include/linux/pci.h | 4 ++++
include/uapi/linux/pci_regs.h | 3 ++-
4 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 661f98c6c63a..9319c62e3488 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5036,10 +5036,23 @@ static int pci_dev_reset_slot_function(struct pci_dev *dev, bool probe)
static u16 cxl_port_dvsec(struct pci_dev *dev)
{
+ if (!pcie_is_cxl(dev))
+ return 0;
+
return pci_find_dvsec_capability(dev, PCI_VENDOR_ID_CXL,
PCI_DVSEC_CXL_PORT);
}
+bool pcie_is_cxl_port(struct pci_dev *dev)
+{
+ if ((pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT) &&
+ (pci_pcie_type(dev) != PCI_EXP_TYPE_UPSTREAM) &&
+ (pci_pcie_type(dev) != PCI_EXP_TYPE_DOWNSTREAM))
+ return false;
+
+ return cxl_port_dvsec(dev);
+}
+
static bool cxl_sbr_masked(struct pci_dev *dev)
{
u16 dvsec, reg;
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 2e81ab0f5a25..ee40a1e2ec75 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1633,6 +1633,14 @@ static void set_pcie_thunderbolt(struct pci_dev *dev)
dev->is_thunderbolt = 1;
}
+static void set_pcie_cxl(struct pci_dev *dev)
+{
+ u16 dvsec = pci_find_dvsec_capability(dev, PCI_VENDOR_ID_CXL,
+ PCI_DVSEC_CXL_FLEXBUS);
+ if (dvsec)
+ dev->is_cxl = 1;
+}
+
static void set_pcie_untrusted(struct pci_dev *dev)
{
struct pci_dev *parent = pci_upstream_bridge(dev);
@@ -1963,6 +1971,8 @@ int pci_setup_device(struct pci_dev *dev)
/* Need to have dev->cfg_size ready */
set_pcie_thunderbolt(dev);
+ set_pcie_cxl(dev);
+
set_pcie_untrusted(dev);
if (pci_is_pcie(dev))
diff --git a/include/linux/pci.h b/include/linux/pci.h
index e2e36f11205c..08350302b3e9 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -452,6 +452,7 @@ struct pci_dev {
unsigned int is_hotplug_bridge:1;
unsigned int shpc_managed:1; /* SHPC owned by shpchp */
unsigned int is_thunderbolt:1; /* Thunderbolt controller */
+ unsigned int is_cxl:1; /* Compute Express Link (CXL) */
/*
* Devices marked being untrusted are the ones that can potentially
* execute DMA attacks and similar. They are typically connected
@@ -739,6 +740,9 @@ static inline bool pci_is_vga(struct pci_dev *pdev)
return false;
}
+#define pcie_is_cxl(dev) (dev->is_cxl)
+bool pcie_is_cxl_port(struct pci_dev *dev);
+
#define for_each_pci_bridge(dev, bus) \
list_for_each_entry(dev, &bus->devices, bus_list) \
if (!pci_is_bridge(dev)) {} else
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index 1601c7ed5fab..4251af090742 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -1208,9 +1208,10 @@
#define PCI_DOE_DATA_OBJECT_DISC_RSP_3_PROTOCOL 0x00ff0000
#define PCI_DOE_DATA_OBJECT_DISC_RSP_3_NEXT_INDEX 0xff000000
-/* Compute Express Link (CXL r3.1, sec 8.1.5) */
+/* Compute Express Link (CXL r3.1, sec 8.1) */
#define PCI_DVSEC_CXL_PORT 3
#define PCI_DVSEC_CXL_PORT_CTL 0x0c
#define PCI_DVSEC_CXL_PORT_CTL_UNMASK_SBR 0x00000001
+#define PCI_DVSEC_CXL_FLEXBUS 7
#endif /* LINUX_PCI_REGS_H */
--
2.34.1Terry Bowman wrote:
> CXL and AER drivers need the ability to identify CXL devices and CXL port
> devices.
>
> First, add set_pcie_cxl() with logic checking for CXL Flexbus DVSEC
> presence. The CXL Flexbus DVSEC presence is used because it is required
> for all the CXL PCIe devices.[1]
>
> Add boolean 'struct pci_dev::is_cxl' with the purpose to cache the CXL
> Flexbus presence.
>
> Add pcie_is_cxl() as a macro to return 'struct pci_dev::is_cxl'.
>
> Add pcie_is_cxl_port() to check if a device is a CXL Root Port, CXL
> Upstream Switch Port, or CXL Downstream Switch Port. Also, verify the
> CXL Extensions DVSEC for Ports is present.[1]
>
> [1] CXL 3.1 Spec, 8.1.1 PCIe Designated Vendor-Specific Extended
> Capability (DVSEC) ID Assignment, Table 8-2
>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Reviewed-by: Fan Ni <fan.ni@samsung.com>
> ---
> drivers/pci/pci.c | 13 +++++++++++++
> drivers/pci/probe.c | 10 ++++++++++
> include/linux/pci.h | 4 ++++
> include/uapi/linux/pci_regs.h | 3 ++-
> 4 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 661f98c6c63a..9319c62e3488 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -5036,10 +5036,23 @@ static int pci_dev_reset_slot_function(struct pci_dev *dev, bool probe)
>
> static u16 cxl_port_dvsec(struct pci_dev *dev)
> {
> + if (!pcie_is_cxl(dev))
> + return 0;
> +
> return pci_find_dvsec_capability(dev, PCI_VENDOR_ID_CXL,
> PCI_DVSEC_CXL_PORT);
> }
>
> +bool pcie_is_cxl_port(struct pci_dev *dev)
> +{
> + if ((pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT) &&
> + (pci_pcie_type(dev) != PCI_EXP_TYPE_UPSTREAM) &&
> + (pci_pcie_type(dev) != PCI_EXP_TYPE_DOWNSTREAM))
> + return false;
> +
> + return cxl_port_dvsec(dev);
Returning bool from a function which returns u16 is odd and I don't think
it should be coded this way. I don't think it is wrong right now but this
really ought to code the pcie_is_cxl() here and leave cxl_port_dvsec()
alone. Calling cxl_port_dvsec(), checking for if the dvsec exists, and
returning bool.
> +}
> +
[snip]
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index e2e36f11205c..08350302b3e9 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -452,6 +452,7 @@ struct pci_dev {
> unsigned int is_hotplug_bridge:1;
> unsigned int shpc_managed:1; /* SHPC owned by shpchp */
> unsigned int is_thunderbolt:1; /* Thunderbolt controller */
> + unsigned int is_cxl:1; /* Compute Express Link (CXL) */
> /*
> * Devices marked being untrusted are the ones that can potentially
> * execute DMA attacks and similar. They are typically connected
> @@ -739,6 +740,9 @@ static inline bool pci_is_vga(struct pci_dev *pdev)
> return false;
> }
>
> +#define pcie_is_cxl(dev) (dev->is_cxl)
This should be an inline function which takes struct pci_dev * for type
safety.
Ira
[snip]
On 1/13/2025 5:49 PM, Ira Weiny wrote:
> Terry Bowman wrote:
>> CXL and AER drivers need the ability to identify CXL devices and CXL port
>> devices.
>>
>> First, add set_pcie_cxl() with logic checking for CXL Flexbus DVSEC
>> presence. The CXL Flexbus DVSEC presence is used because it is required
>> for all the CXL PCIe devices.[1]
>>
>> Add boolean 'struct pci_dev::is_cxl' with the purpose to cache the CXL
>> Flexbus presence.
>>
>> Add pcie_is_cxl() as a macro to return 'struct pci_dev::is_cxl'.
>>
>> Add pcie_is_cxl_port() to check if a device is a CXL Root Port, CXL
>> Upstream Switch Port, or CXL Downstream Switch Port. Also, verify the
>> CXL Extensions DVSEC for Ports is present.[1]
>>
>> [1] CXL 3.1 Spec, 8.1.1 PCIe Designated Vendor-Specific Extended
>> Capability (DVSEC) ID Assignment, Table 8-2
>>
>> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
>> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
>> Reviewed-by: Fan Ni <fan.ni@samsung.com>
>> ---
>> drivers/pci/pci.c | 13 +++++++++++++
>> drivers/pci/probe.c | 10 ++++++++++
>> include/linux/pci.h | 4 ++++
>> include/uapi/linux/pci_regs.h | 3 ++-
>> 4 files changed, 29 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index 661f98c6c63a..9319c62e3488 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -5036,10 +5036,23 @@ static int pci_dev_reset_slot_function(struct pci_dev *dev, bool probe)
>>
>> static u16 cxl_port_dvsec(struct pci_dev *dev)
>> {
>> + if (!pcie_is_cxl(dev))
>> + return 0;
>> +
>> return pci_find_dvsec_capability(dev, PCI_VENDOR_ID_CXL,
>> PCI_DVSEC_CXL_PORT);
>> }
>>
>> +bool pcie_is_cxl_port(struct pci_dev *dev)
>> +{
>> + if ((pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT) &&
>> + (pci_pcie_type(dev) != PCI_EXP_TYPE_UPSTREAM) &&
>> + (pci_pcie_type(dev) != PCI_EXP_TYPE_DOWNSTREAM))
>> + return false;
>> +
>> + return cxl_port_dvsec(dev);
> Returning bool from a function which returns u16 is odd and I don't think
> it should be coded this way. I don't think it is wrong right now but this
> really ought to code the pcie_is_cxl() here and leave cxl_port_dvsec()
> alone. Calling cxl_port_dvsec(), checking for if the dvsec exists, and
> returning bool.
Hi Ira,
Thanks for reviewing. Is this what you are looking for here:
+bool pcie_is_cxl_port(struct pci_dev *dev)
+{
+ return (cxl_port_dvsec(dev) > 0);
>> +}
>> +
> [snip]
>
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index e2e36f11205c..08350302b3e9 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -452,6 +452,7 @@ struct pci_dev {
>> unsigned int is_hotplug_bridge:1;
>> unsigned int shpc_managed:1; /* SHPC owned by shpchp */
>> unsigned int is_thunderbolt:1; /* Thunderbolt controller */
>> + unsigned int is_cxl:1; /* Compute Express Link (CXL) */
>> /*
>> * Devices marked being untrusted are the ones that can potentially
>> * execute DMA attacks and similar. They are typically connected
>> @@ -739,6 +740,9 @@ static inline bool pci_is_vga(struct pci_dev *pdev)
>> return false;
>> }
>>
>> +#define pcie_is_cxl(dev) (dev->is_cxl)
> This should be an inline function which takes struct pci_dev * for type
> safety.
>
> Ira
Ok,
Thanks for reviewing the patches.
Regards,
Terry
> [snip]
On Tue, Jan 14, 2025 at 09:19:12AM -0600, Bowman, Terry wrote:
> On 1/13/2025 5:49 PM, Ira Weiny wrote:
> > Terry Bowman wrote:
> > > --- a/drivers/pci/pci.c
> > > +++ b/drivers/pci/pci.c
> > > @@ -5036,10 +5036,23 @@ static int pci_dev_reset_slot_function(struct pci_dev *dev, bool probe)
> > >
> > > static u16 cxl_port_dvsec(struct pci_dev *dev)
> > > {
> > > + if (!pcie_is_cxl(dev))
> > > + return 0;
> > > +
> > > return pci_find_dvsec_capability(dev, PCI_VENDOR_ID_CXL,
> > > PCI_DVSEC_CXL_PORT);
> > > }
> > >
> > > +bool pcie_is_cxl_port(struct pci_dev *dev)
> > > +{
> > > + if ((pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT) &&
> > > + (pci_pcie_type(dev) != PCI_EXP_TYPE_UPSTREAM) &&
> > > + (pci_pcie_type(dev) != PCI_EXP_TYPE_DOWNSTREAM))
> > > + return false;
> > > +
> > > + return cxl_port_dvsec(dev);
> >
> > Returning bool from a function which returns u16 is odd and I don't think
> > it should be coded this way. I don't think it is wrong right now but this
> > really ought to code the pcie_is_cxl() here and leave cxl_port_dvsec()
> > alone. Calling cxl_port_dvsec(), checking for if the dvsec exists, and
> > returning bool.
>
> Thanks for reviewing. Is this what you are looking for here:
>
> +bool pcie_is_cxl_port(struct pci_dev *dev)
> +{
> + return (cxl_port_dvsec(dev) > 0);
Since cxl_port_dvsec() cannot return a negative integer,
you might as well use:
return !!cxl_port_dvsec(dev);
However last I checked gcc generates code which implicitly turns
a number bigger than 1 into a 1 if the return type is bool.
(I had to fix a bug caused by this behavior once, see 009f8c90f571).
Thanks,
Lukas
Bowman, Terry wrote:
>
>
>
> On 1/13/2025 5:49 PM, Ira Weiny wrote:
> > Terry Bowman wrote:
> >> CXL and AER drivers need the ability to identify CXL devices and CXL port
> >> devices.
> >>
> >> First, add set_pcie_cxl() with logic checking for CXL Flexbus DVSEC
> >> presence. The CXL Flexbus DVSEC presence is used because it is required
> >> for all the CXL PCIe devices.[1]
> >>
> >> Add boolean 'struct pci_dev::is_cxl' with the purpose to cache the CXL
> >> Flexbus presence.
> >>
> >> Add pcie_is_cxl() as a macro to return 'struct pci_dev::is_cxl'.
> >>
> >> Add pcie_is_cxl_port() to check if a device is a CXL Root Port, CXL
> >> Upstream Switch Port, or CXL Downstream Switch Port. Also, verify the
> >> CXL Extensions DVSEC for Ports is present.[1]
> >>
> >> [1] CXL 3.1 Spec, 8.1.1 PCIe Designated Vendor-Specific Extended
> >> Capability (DVSEC) ID Assignment, Table 8-2
> >>
> >> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> >> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> >> Reviewed-by: Fan Ni <fan.ni@samsung.com>
> >> ---
> >> drivers/pci/pci.c | 13 +++++++++++++
> >> drivers/pci/probe.c | 10 ++++++++++
> >> include/linux/pci.h | 4 ++++
> >> include/uapi/linux/pci_regs.h | 3 ++-
> >> 4 files changed, 29 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> >> index 661f98c6c63a..9319c62e3488 100644
> >> --- a/drivers/pci/pci.c
> >> +++ b/drivers/pci/pci.c
> >> @@ -5036,10 +5036,23 @@ static int pci_dev_reset_slot_function(struct pci_dev *dev, bool probe)
> >>
> >> static u16 cxl_port_dvsec(struct pci_dev *dev)
> >> {
> >> + if (!pcie_is_cxl(dev))
> >> + return 0;
> >> +
> >> return pci_find_dvsec_capability(dev, PCI_VENDOR_ID_CXL,
> >> PCI_DVSEC_CXL_PORT);
> >> }
> >>
> >> +bool pcie_is_cxl_port(struct pci_dev *dev)
> >> +{
> >> + if ((pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT) &&
> >> + (pci_pcie_type(dev) != PCI_EXP_TYPE_UPSTREAM) &&
> >> + (pci_pcie_type(dev) != PCI_EXP_TYPE_DOWNSTREAM))
> >> + return false;
> >> +
> >> + return cxl_port_dvsec(dev);
> > Returning bool from a function which returns u16 is odd and I don't think
> > it should be coded this way. I don't think it is wrong right now but this
> > really ought to code the pcie_is_cxl() here and leave cxl_port_dvsec()
> > alone. Calling cxl_port_dvsec(), checking for if the dvsec exists, and
> > returning bool.
>
> Hi Ira,
>
> Thanks for reviewing. Is this what you are looking for here:
>
> +bool pcie_is_cxl_port(struct pci_dev *dev)
> +{
> + return (cxl_port_dvsec(dev) > 0);
With the type checks, yes that is more clear.
Ira
[snip]
On 1/14/2025 5:33 PM, Ira Weiny wrote:
> Bowman, Terry wrote:
>>
>>
>> On 1/13/2025 5:49 PM, Ira Weiny wrote:
>>> Terry Bowman wrote:
>>>> CXL and AER drivers need the ability to identify CXL devices and CXL port
>>>> devices.
>>>>
>>>> First, add set_pcie_cxl() with logic checking for CXL Flexbus DVSEC
>>>> presence. The CXL Flexbus DVSEC presence is used because it is required
>>>> for all the CXL PCIe devices.[1]
>>>>
>>>> Add boolean 'struct pci_dev::is_cxl' with the purpose to cache the CXL
>>>> Flexbus presence.
>>>>
>>>> Add pcie_is_cxl() as a macro to return 'struct pci_dev::is_cxl'.
>>>>
>>>> Add pcie_is_cxl_port() to check if a device is a CXL Root Port, CXL
>>>> Upstream Switch Port, or CXL Downstream Switch Port. Also, verify the
>>>> CXL Extensions DVSEC for Ports is present.[1]
>>>>
>>>> [1] CXL 3.1 Spec, 8.1.1 PCIe Designated Vendor-Specific Extended
>>>> Capability (DVSEC) ID Assignment, Table 8-2
>>>>
>>>> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
>>>> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>>>> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
>>>> Reviewed-by: Fan Ni <fan.ni@samsung.com>
>>>> ---
>>>> drivers/pci/pci.c | 13 +++++++++++++
>>>> drivers/pci/probe.c | 10 ++++++++++
>>>> include/linux/pci.h | 4 ++++
>>>> include/uapi/linux/pci_regs.h | 3 ++-
>>>> 4 files changed, 29 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>>>> index 661f98c6c63a..9319c62e3488 100644
>>>> --- a/drivers/pci/pci.c
>>>> +++ b/drivers/pci/pci.c
>>>> @@ -5036,10 +5036,23 @@ static int pci_dev_reset_slot_function(struct pci_dev *dev, bool probe)
>>>>
>>>> static u16 cxl_port_dvsec(struct pci_dev *dev)
>>>> {
>>>> + if (!pcie_is_cxl(dev))
>>>> + return 0;
>>>> +
>>>> return pci_find_dvsec_capability(dev, PCI_VENDOR_ID_CXL,
>>>> PCI_DVSEC_CXL_PORT);
>>>> }
>>>>
>>>> +bool pcie_is_cxl_port(struct pci_dev *dev)
>>>> +{
>>>> + if ((pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT) &&
>>>> + (pci_pcie_type(dev) != PCI_EXP_TYPE_UPSTREAM) &&
>>>> + (pci_pcie_type(dev) != PCI_EXP_TYPE_DOWNSTREAM))
>>>> + return false;
>>>> +
>>>> + return cxl_port_dvsec(dev);
>>> Returning bool from a function which returns u16 is odd and I don't think
>>> it should be coded this way. I don't think it is wrong right now but this
>>> really ought to code the pcie_is_cxl() here and leave cxl_port_dvsec()
>>> alone. Calling cxl_port_dvsec(), checking for if the dvsec exists, and
>>> returning bool.
>> Hi Ira,
>>
>> Thanks for reviewing. Is this what you are looking for here:
>>
>> +bool pcie_is_cxl_port(struct pci_dev *dev)
>> +{
>> + return (cxl_port_dvsec(dev) > 0);
> With the type checks, yes that is more clear.
>
> Ira
>
> [snip]
Since sending the above I made update to be:
static u16 cxl_port_dvsec(struct pci_dev *dev)
{
return pci_find_dvsec_capability(dev, PCI_VENDOR_ID_CXL,
PCI_DVSEC_CXL_PORT);
}
inline bool pcie_is_cxl(struct pci_dev *pci_dev)
{
return pci_dev->is_cxl;
}
bool pcie_is_cxl_port(struct pci_dev *pci_dev)
{
if (!pcie_is_cxl(pci_dev))
return false;
return (cxl_port_dvsec(pci_dev) > 0);
}
I can change if you see anything is needed.
Regards,
Terry
Bowman, Terry wrote:
>
>
>
> On 1/14/2025 5:33 PM, Ira Weiny wrote:
> > Bowman, Terry wrote:
> >>
> >>
> >> On 1/13/2025 5:49 PM, Ira Weiny wrote:
> >>> Terry Bowman wrote:
[snip]
> >>>> +bool pcie_is_cxl_port(struct pci_dev *dev)
> >>>> +{
> >>>> + if ((pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT) &&
> >>>> + (pci_pcie_type(dev) != PCI_EXP_TYPE_UPSTREAM) &&
> >>>> + (pci_pcie_type(dev) != PCI_EXP_TYPE_DOWNSTREAM))
> >>>> + return false;
> >>>> +
> >>>> + return cxl_port_dvsec(dev);
> >>> Returning bool from a function which returns u16 is odd and I don't think
> >>> it should be coded this way. I don't think it is wrong right now but this
> >>> really ought to code the pcie_is_cxl() here and leave cxl_port_dvsec()
> >>> alone. Calling cxl_port_dvsec(), checking for if the dvsec exists, and
> >>> returning bool.
> >> Hi Ira,
> >>
> >> Thanks for reviewing. Is this what you are looking for here:
> >>
> >> +bool pcie_is_cxl_port(struct pci_dev *dev)
> >> +{
> >> + return (cxl_port_dvsec(dev) > 0);
> > With the type checks, yes that is more clear.
> >
> > Ira
> >
> > [snip]
> Since sending the above I made update to be:
>
> static u16 cxl_port_dvsec(struct pci_dev *dev)
> {
> return pci_find_dvsec_capability(dev, PCI_VENDOR_ID_CXL,
> PCI_DVSEC_CXL_PORT);
> }
>
> inline bool pcie_is_cxl(struct pci_dev *pci_dev)
> {
> return pci_dev->is_cxl;
> }
>
> bool pcie_is_cxl_port(struct pci_dev *pci_dev)
> {
> if (!pcie_is_cxl(pci_dev))
> return false;
>
> return (cxl_port_dvsec(pci_dev) > 0);
> }
>
> I can change if you see anything is needed.
Looks good thanks!
Ira
[snip]
© 2016 - 2025 Red Hat, Inc.