diff mbox series

[v3,2/3] Remove conditional branch that is not suitable for cxl1.1 devices

Message ID 20240312080559.14904-3-kobayashi.da-06@fujitsu.com (mailing list archive)
State Handled Elsewhere
Headers show
Series Display cxl1.1 device link status | expand

Commit Message

Daisuke Kobayashi (Fujitsu) March 12, 2024, 8:05 a.m. UTC
This patch removes conditional branching that does not comply with the specifications.

In the existing code, there is a conditional branch that compares "chbs->length" 
with "CXL_RCRB_SIZE". However, according to the specifications, "chbs->length" 
indicates the length of the CHBS structure in the cedt, not the size of the 
configuration space. Therefore, since this condition does not comply with
the specifications(cxl3.0 specification 9.17.1), remove it.

Signed-off-by: "Kobayashi,Daisuke" <kobayashi.da-06@fujitsu.com>
---
 drivers/cxl/acpi.c | 4 ----
 1 file changed, 4 deletions(-)

Comments

Dan Williams March 26, 2024, 8 p.m. UTC | #1
Kobayashi,Daisuke wrote:
> This patch removes conditional branching that does not comply with the specifications.
> 
> In the existing code, there is a conditional branch that compares "chbs->length" 
> with "CXL_RCRB_SIZE". However, according to the specifications, "chbs->length" 
> indicates the length of the CHBS structure in the cedt, not the size of the 
> configuration space. Therefore, since this condition does not comply with
> the specifications(cxl3.0 specification 9.17.1), remove it.

No, in Table 9-21 in CXL 3.1:

Base: If CXL Version = 0000 0000h, this represents the base address of
the RCH Downstream Port RCRB

Length: If CXL Version = 0000 0000h, this field must be set to 8 KB
(2000h)

The size of the CHBS structure is always much less than 8KB.
Daisuke Kobayashi (Fujitsu) March 27, 2024, 8:26 a.m. UTC | #2
Dan Williams wrote:
> Kobayashi,Daisuke wrote:
> > This patch removes conditional branching that does not comply with the
> specifications.
> >
> > In the existing code, there is a conditional branch that compares
> "chbs->length"
> > with "CXL_RCRB_SIZE". However, according to the specifications,
> "chbs->length"
> > indicates the length of the CHBS structure in the cedt, not the size
> > of the configuration space. Therefore, since this condition does not
> > comply with the specifications(cxl3.0 specification 9.17.1), remove it.
> 
> No, in Table 9-21 in CXL 3.1:
> 
> Base: If CXL Version = 0000 0000h, this represents the base address of the
> RCH Downstream Port RCRB
> 
> Length: If CXL Version = 0000 0000h, this field must be set to 8 KB
> (2000h)
> 
> The size of the CHBS structure is always much less than 8KB.

Thank you for your feedback. 
After revisiting the specifications, I realized that my understanding was incorrect. 
Therefore, in the next patch, I will revert this change.
diff mbox series

Patch

diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index dcf2b39e1048..cf15c5130428 100644
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -477,10 +477,6 @@  static int cxl_get_chbs_iter(union acpi_subtable_headers *header, void *arg,
 	if (!chbs->base)
 		return 0;
 
-	if (chbs->cxl_version == ACPI_CEDT_CHBS_VERSION_CXL11 &&
-	    chbs->length != CXL_RCRB_SIZE)
-		return 0;
-
 	ctx->base = chbs->base;
 
 	return 0;