[v2] hw/cxl/device: Add Flex Bus Port DVSEC

Commit Message

Ira Weiny Dec. 14, 2022, 8:54 p.m. UTC

The Flex Bus Port DVSEC was missing on type 3 devices which was blocking
RAS checks.[1]

Add the Flex Bus Port DVSEC to type 3 devices as per CXL 3.0 8.2.1.3.

[1] https://lore.kernel.org/linux-cxl/[e-mail]/

Cc: Dave Jiang <[e-mail]>
Cc: Jonathan Cameron <[e-mail]>
Cc: Ben Widawsky <[e-mail]>
Cc: [e-mail]
Cc: [e-mail]
Signed-off-by: Ira Weiny <[e-mail]>
---
Changes in v2:
        Jonathan                                                            
                type 3 device does not support CACHE                        
                Comment the 68B bit                                         

- Link to v1: https://lore.kernel.org/r/[e-mail]
---
 hw/mem/cxl_type3.c | 11 +++++++++++
 1 file changed, 11 insertions(+)


---
base-commit: e11b57108b0cb746bb9f3887054f34a2f818ed79
change-id: 20221213-ira-flexbus-port-ce526de8111d

Best regards,

Comments

Jonathan Cameron Dec. 15, 2022, 5:16 p.m. UTC | #1

On Wed, 14 Dec 2022 12:54:11 -0800
Ira Weiny <[e-mail]> wrote:

> The Flex Bus Port DVSEC was missing on type 3 devices which was blocking
> RAS checks.[1]
> 
> Add the Flex Bus Port DVSEC to type 3 devices as per CXL 3.0 8.2.1.3.
> 
> [1] https://lore.kernel.org/linux-cxl/[e-mail]/
> 
> Cc: Dave Jiang <[e-mail]>
> Cc: Jonathan Cameron <[e-mail]>
> Cc: Ben Widawsky <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
> Signed-off-by: Ira Weiny <[e-mail]>
Looks good to me.

Reviewed-by: Jonathan Cameron <[e-mail]>

As Michael wasn't cc'd on patch posting, so might not get this directly I'll add
it to the front of the series adding the RAS event emulation on basis that's the
first time we'll see a failure in Linux (I think?)

Michael, if you want to pick this up directly that's great too!

As a side note the WTF? is because we made up a hardware related time delay
number having no idea whatsoever on what a realistic value was. Cut and paste
from the instances of this structure in the root port and the switch ports.

Jonathan



> ---
> Changes in v2:
>         Jonathan                                                            
>                 type 3 device does not support CACHE                        
>                 Comment the 68B bit                                         
> 
> - Link to v1: https://lore.kernel.org/r/[e-mail]
> ---
>  hw/mem/cxl_type3.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index 0317bd96a6fb..e6beac143fc1 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -416,6 +416,17 @@ static void build_dvsecs(CXLType3Dev *ct3d)
>      cxl_component_create_dvsec(cxl_cstate, CXL2_TYPE3_DEVICE,
>                                 GPF_DEVICE_DVSEC_LENGTH, GPF_DEVICE_DVSEC,
>                                 GPF_DEVICE_DVSEC_REVID, dvsec);
> +
> +    dvsec = (uint8_t *)&(CXLDVSECPortFlexBus){
> +        .cap                     = 0x26, /* 68B, IO, Mem, non-MLD */
> +        .ctrl                    = 0x02, /* IO always enabled */
> +        .status                  = 0x26, /* same as capabilities */
> +        .rcvd_mod_ts_data_phase1 = 0xef, /* WTF? */
> +    };
> +    cxl_component_create_dvsec(cxl_cstate, CXL2_TYPE3_DEVICE,
> +                               PCIE_FLEXBUS_PORT_DVSEC_LENGTH_2_0,
> +                               PCIE_FLEXBUS_PORT_DVSEC,
> +                               PCIE_FLEXBUS_PORT_DVSEC_REVID_2_0, dvsec);
>  }
>  
>  static void hdm_decoder_commit(CXLType3Dev *ct3d, int which)
> 
> ---
> base-commit: e11b57108b0cb746bb9f3887054f34a2f818ed79
> change-id: 20221213-ira-flexbus-port-ce526de8111d
> 
> Best regards,

Ira Weiny Dec. 15, 2022, 5:28 p.m. UTC | #2

On Thu, Dec 15, 2022 at 05:16:33PM +0000, Jonathan Cameron wrote:
> On Wed, 14 Dec 2022 12:54:11 -0800
> Ira Weiny <[e-mail]> wrote:
> 
> > The Flex Bus Port DVSEC was missing on type 3 devices which was blocking
> > RAS checks.[1]
> > 
> > Add the Flex Bus Port DVSEC to type 3 devices as per CXL 3.0 8.2.1.3.
> > 
> > [1] https://lore.kernel.org/linux-cxl/[e-mail]/
> > 
> > Cc: Dave Jiang <[e-mail]>
> > Cc: Jonathan Cameron <[e-mail]>
> > Cc: Ben Widawsky <[e-mail]>
> > Cc: [e-mail]
> > Cc: [e-mail]
> > Signed-off-by: Ira Weiny <[e-mail]>
> Looks good to me.
> 
> Reviewed-by: Jonathan Cameron <[e-mail]>
> 
> As Michael wasn't cc'd on patch posting, so might not get this directly I'll add
> it to the front of the series adding the RAS event emulation on basis that's the
> first time we'll see a failure in Linux (I think?)

Ah thanks!

Sorry, I thought you were the 'maintainer' of the CXL stuff for qemu.

> 
> Michael, if you want to pick this up directly that's great too!

Should I send directly to Michael in future?

> 
> As a side note the WTF? is because we made up a hardware related time delay
> number having no idea whatsoever on what a realistic value was. Cut and paste
> from the instances of this structure in the root port and the switch ports.
> 

Yep I just followed that based off the other code.

Ira

> Jonathan
> 
> 
> 
> > ---
> > Changes in v2:
> >         Jonathan                                                            
> >                 type 3 device does not support CACHE                        
> >                 Comment the 68B bit                                         
> > 
> > - Link to v1: https://lore.kernel.org/r/[e-mail]
> > ---
> >  hw/mem/cxl_type3.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> > index 0317bd96a6fb..e6beac143fc1 100644
> > --- a/hw/mem/cxl_type3.c
> > +++ b/hw/mem/cxl_type3.c
> > @@ -416,6 +416,17 @@ static void build_dvsecs(CXLType3Dev *ct3d)
> >      cxl_component_create_dvsec(cxl_cstate, CXL2_TYPE3_DEVICE,
> >                                 GPF_DEVICE_DVSEC_LENGTH, GPF_DEVICE_DVSEC,
> >                                 GPF_DEVICE_DVSEC_REVID, dvsec);
> > +
> > +    dvsec = (uint8_t *)&(CXLDVSECPortFlexBus){
> > +        .cap                     = 0x26, /* 68B, IO, Mem, non-MLD */
> > +        .ctrl                    = 0x02, /* IO always enabled */
> > +        .status                  = 0x26, /* same as capabilities */
> > +        .rcvd_mod_ts_data_phase1 = 0xef, /* WTF? */
> > +    };
> > +    cxl_component_create_dvsec(cxl_cstate, CXL2_TYPE3_DEVICE,
> > +                               PCIE_FLEXBUS_PORT_DVSEC_LENGTH_2_0,
> > +                               PCIE_FLEXBUS_PORT_DVSEC,
> > +                               PCIE_FLEXBUS_PORT_DVSEC_REVID_2_0, dvsec);
> >  }
> >  
> >  static void hdm_decoder_commit(CXLType3Dev *ct3d, int which)
> > 
> > ---
> > base-commit: e11b57108b0cb746bb9f3887054f34a2f818ed79
> > change-id: 20221213-ira-flexbus-port-ce526de8111d
> > 
> > Best regards,
>

Jonathan Cameron Dec. 16, 2022, 9:31 a.m. UTC | #3

On Thu, 15 Dec 2022 09:28:30 -0800
Ira Weiny <[e-mail]> wrote:

> On Thu, Dec 15, 2022 at 05:16:33PM +0000, Jonathan Cameron wrote:
> > On Wed, 14 Dec 2022 12:54:11 -0800
> > Ira Weiny <[e-mail]> wrote:
> >   
> > > The Flex Bus Port DVSEC was missing on type 3 devices which was blocking
> > > RAS checks.[1]
> > > 
> > > Add the Flex Bus Port DVSEC to type 3 devices as per CXL 3.0 8.2.1.3.
> > > 
> > > [1] https://lore.kernel.org/linux-cxl/[e-mail]/
> > > 
> > > Cc: Dave Jiang <[e-mail]>
> > > Cc: Jonathan Cameron <[e-mail]>
> > > Cc: Ben Widawsky <[e-mail]>
> > > Cc: [e-mail]
> > > Cc: [e-mail]
> > > Signed-off-by: Ira Weiny <[e-mail]>  
> > Looks good to me.
> > 
> > Reviewed-by: Jonathan Cameron <[e-mail]>
> > 
> > As Michael wasn't cc'd on patch posting, so might not get this directly I'll add
> > it to the front of the series adding the RAS event emulation on basis that's the
> > first time we'll see a failure in Linux (I think?)  
> 
> Ah thanks!
> 
> Sorry, I thought you were the 'maintainer' of the CXL stuff for qemu.

Ah I am, but so far the CXL stuff has routed through Michael as PCI maintainer
because of the high level of overlap. So far I've done that by grouping up
patches and send them in sets to Michael. This one is more of a fix, so maybe
wants to move quicker than I will.

This gives me a good opportunity to ask Michael:

How do you want us to handle this in future? I'd expect the overlap with the PCI
core stuff to reduce going forwards, as most of the infrastructure is now in
place and obviously would want you to look at anything that does touch core PCI
code, but for the rest of it, would you prefer that I send pull requests going
forwards?  I'm more than happy to keep dumping this stuff on you, but seems
rather mean!

If we do move to pull requests, what scope of stuff do you want us to seek your
review on?  If things go according to plan there will be a bunch of stuff related
to the switch ports in the near future, some of which is going to add complex
PCI topologies and new forms of hotplug so I'll definitely want your input on
that, but we also have a bunch of stuff around memory error reporting etc which
I'm thinking may be of less interest to you. Obviously I'd love it if you have
time to review everything but don't want to impose unnecessarily.

Jonathan


> 
> > 
> > Michael, if you want to pick this up directly that's great too!  
> 
> Should I send directly to Michael in future?
> 
> > 
> > As a side note the WTF? is because we made up a hardware related time delay
> > number having no idea whatsoever on what a realistic value was. Cut and paste
> > from the instances of this structure in the root port and the switch ports.
> >   
> 
> Yep I just followed that based off the other code.
> 
> Ira
> 
> > Jonathan
> > 
> > 
> >   
> > > ---
> > > Changes in v2:
> > >         Jonathan                                                            
> > >                 type 3 device does not support CACHE                        
> > >                 Comment the 68B bit                                         
> > > 
> > > - Link to v1: https://lore.kernel.org/r/[e-mail]
> > > ---
> > >  hw/mem/cxl_type3.c | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > > 
> > > diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> > > index 0317bd96a6fb..e6beac143fc1 100644
> > > --- a/hw/mem/cxl_type3.c
> > > +++ b/hw/mem/cxl_type3.c
> > > @@ -416,6 +416,17 @@ static void build_dvsecs(CXLType3Dev *ct3d)
> > >      cxl_component_create_dvsec(cxl_cstate, CXL2_TYPE3_DEVICE,
> > >                                 GPF_DEVICE_DVSEC_LENGTH, GPF_DEVICE_DVSEC,
> > >                                 GPF_DEVICE_DVSEC_REVID, dvsec);
> > > +
> > > +    dvsec = (uint8_t *)&(CXLDVSECPortFlexBus){
> > > +        .cap                     = 0x26, /* 68B, IO, Mem, non-MLD */
> > > +        .ctrl                    = 0x02, /* IO always enabled */
> > > +        .status                  = 0x26, /* same as capabilities */
> > > +        .rcvd_mod_ts_data_phase1 = 0xef, /* WTF? */
> > > +    };
> > > +    cxl_component_create_dvsec(cxl_cstate, CXL2_TYPE3_DEVICE,
> > > +                               PCIE_FLEXBUS_PORT_DVSEC_LENGTH_2_0,
> > > +                               PCIE_FLEXBUS_PORT_DVSEC,
> > > +                               PCIE_FLEXBUS_PORT_DVSEC_REVID_2_0, dvsec);
> > >  }
> > >  
> > >  static void hdm_decoder_commit(CXLType3Dev *ct3d, int which)
> > > 
> > > ---
> > > base-commit: e11b57108b0cb746bb9f3887054f34a2f818ed79
> > > change-id: 20221213-ira-flexbus-port-ce526de8111d
> > > 
> > > Best regards,  
> >

Michael S. Tsirkin Dec. 16, 2022, 12:37 p.m. UTC | #4

On Fri, Dec 16, 2022 at 09:31:36AM +0000, Jonathan Cameron wrote:
> On Thu, 15 Dec 2022 09:28:30 -0800
> Ira Weiny <[e-mail]> wrote:
> 
> > On Thu, Dec 15, 2022 at 05:16:33PM +0000, Jonathan Cameron wrote:
> > > On Wed, 14 Dec 2022 12:54:11 -0800
> > > Ira Weiny <[e-mail]> wrote:
> > >   
> > > > The Flex Bus Port DVSEC was missing on type 3 devices which was blocking
> > > > RAS checks.[1]
> > > > 
> > > > Add the Flex Bus Port DVSEC to type 3 devices as per CXL 3.0 8.2.1.3.
> > > > 
> > > > [1] https://lore.kernel.org/linux-cxl/[e-mail]/
> > > > 
> > > > Cc: Dave Jiang <[e-mail]>
> > > > Cc: Jonathan Cameron <[e-mail]>
> > > > Cc: Ben Widawsky <[e-mail]>
> > > > Cc: [e-mail]
> > > > Cc: [e-mail]
> > > > Signed-off-by: Ira Weiny <[e-mail]>  
> > > Looks good to me.
> > > 
> > > Reviewed-by: Jonathan Cameron <[e-mail]>
> > > 
> > > As Michael wasn't cc'd on patch posting, so might not get this directly I'll add
> > > it to the front of the series adding the RAS event emulation on basis that's the
> > > first time we'll see a failure in Linux (I think?)  
> > 
> > Ah thanks!
> > 
> > Sorry, I thought you were the 'maintainer' of the CXL stuff for qemu.
> 
> Ah I am, but so far the CXL stuff has routed through Michael as PCI maintainer
> because of the high level of overlap. So far I've done that by grouping up
> patches and send them in sets to Michael. This one is more of a fix, so maybe
> wants to move quicker than I will.
> 
> This gives me a good opportunity to ask Michael:
> 
> How do you want us to handle this in future? I'd expect the overlap with the PCI
> core stuff to reduce going forwards, as most of the infrastructure is now in
> place and obviously would want you to look at anything that does touch core PCI
> code, but for the rest of it, would you prefer that I send pull requests going
> forwards?  I'm more than happy to keep dumping this stuff on you, but seems
> rather mean!
> 
> If we do move to pull requests, what scope of stuff do you want us to seek your
> review on?  If things go according to plan there will be a bunch of stuff related
> to the switch ports in the near future, some of which is going to add complex
> PCI topologies and new forms of hotplug so I'll definitely want your input on
> that, but we also have a bunch of stuff around memory error reporting etc which
> I'm thinking may be of less interest to you. Obviously I'd love it if you have
> time to review everything but don't want to impose unnecessarily.
> 
> Jonathan

It's not been too bad so far :) I'm not used to reviewing pull requests
- patches are easier if you want my review. If you are happy too then
all is well. Yes I assume if I'm CC'd directly people want my input
if not then not. And learning a bit about how error reporting works
is interesting to me.

But if you want to send some pulls directly that's fine too, at this point
I trust you to do the right thing.


> 
> > 
> > > 
> > > Michael, if you want to pick this up directly that's great too!  
> > 
> > Should I send directly to Michael in future?
> > 
> > > 
> > > As a side note the WTF? is because we made up a hardware related time delay
> > > number having no idea whatsoever on what a realistic value was. Cut and paste
> > > from the instances of this structure in the root port and the switch ports.
> > >   
> > 
> > Yep I just followed that based off the other code.
> > 
> > Ira
> > 
> > > Jonathan
> > > 
> > > 
> > >   
> > > > ---
> > > > Changes in v2:
> > > >         Jonathan                                                            
> > > >                 type 3 device does not support CACHE                        
> > > >                 Comment the 68B bit                                         
> > > > 
> > > > - Link to v1: https://lore.kernel.org/r/[e-mail]
> > > > ---
> > > >  hw/mem/cxl_type3.c | 11 +++++++++++
> > > >  1 file changed, 11 insertions(+)
> > > > 
> > > > diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> > > > index 0317bd96a6fb..e6beac143fc1 100644
> > > > --- a/hw/mem/cxl_type3.c
> > > > +++ b/hw/mem/cxl_type3.c
> > > > @@ -416,6 +416,17 @@ static void build_dvsecs(CXLType3Dev *ct3d)
> > > >      cxl_component_create_dvsec(cxl_cstate, CXL2_TYPE3_DEVICE,
> > > >                                 GPF_DEVICE_DVSEC_LENGTH, GPF_DEVICE_DVSEC,
> > > >                                 GPF_DEVICE_DVSEC_REVID, dvsec);
> > > > +
> > > > +    dvsec = (uint8_t *)&(CXLDVSECPortFlexBus){
> > > > +        .cap                     = 0x26, /* 68B, IO, Mem, non-MLD */
> > > > +        .ctrl                    = 0x02, /* IO always enabled */
> > > > +        .status                  = 0x26, /* same as capabilities */
> > > > +        .rcvd_mod_ts_data_phase1 = 0xef, /* WTF? */
> > > > +    };
> > > > +    cxl_component_create_dvsec(cxl_cstate, CXL2_TYPE3_DEVICE,
> > > > +                               PCIE_FLEXBUS_PORT_DVSEC_LENGTH_2_0,
> > > > +                               PCIE_FLEXBUS_PORT_DVSEC,
> > > > +                               PCIE_FLEXBUS_PORT_DVSEC_REVID_2_0, dvsec);
> > > >  }
> > > >  
> > > >  static void hdm_decoder_commit(CXLType3Dev *ct3d, int which)
> > > > 
> > > > ---
> > > > base-commit: e11b57108b0cb746bb9f3887054f34a2f818ed79
> > > > change-id: 20221213-ira-flexbus-port-ce526de8111d
> > > > 
> > > > Best regards,  
> > >

Patch

diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
index 0317bd96a6fb..e6beac143fc1 100644
--- a/hw/mem/cxl_type3.c
+++ b/hw/mem/cxl_type3.c
@@ -416,6 +416,17 @@  static void build_dvsecs(CXLType3Dev *ct3d)
     cxl_component_create_dvsec(cxl_cstate, CXL2_TYPE3_DEVICE,
                                GPF_DEVICE_DVSEC_LENGTH, GPF_DEVICE_DVSEC,
                                GPF_DEVICE_DVSEC_REVID, dvsec);
+
+    dvsec = (uint8_t *)&(CXLDVSECPortFlexBus){
+        .cap                     = 0x26, /* 68B, IO, Mem, non-MLD */
+        .ctrl                    = 0x02, /* IO always enabled */
+        .status                  = 0x26, /* same as capabilities */
+        .rcvd_mod_ts_data_phase1 = 0xef, /* WTF? */
+    };
+    cxl_component_create_dvsec(cxl_cstate, CXL2_TYPE3_DEVICE,
+                               PCIE_FLEXBUS_PORT_DVSEC_LENGTH_2_0,
+                               PCIE_FLEXBUS_PORT_DVSEC,
+                               PCIE_FLEXBUS_PORT_DVSEC_REVID_2_0, dvsec);
 }
 
 static void hdm_decoder_commit(CXLType3Dev *ct3d, int which)