| Message ID | 20230517-rfc-type2-dev-v1-1-6eb2e470981b@intel.com |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/cxl: Type 2 Device RFC | expand |
On Wed, 17 May 2023 19:45:54 -0700 Ira Weiny <ira.weiny@intel.com> wrote: > Magic numbers can be confusing. > > Use the range size define for CXL.cachemem rather than a magic number. > Update/add spec references. > > Signed-off-by: Ira Weiny <ira.weiny@intel.com> I guess we should do a scrub to move all refs to 3.0 soon given it's horrible having a mixture of spec versions for the references. For future specs, we should only do this when sufficient X.Y references have started to appear - I think that's true for r3.0 now. Jonathan > --- > include/hw/cxl/cxl_component.h | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h > index 52b6a2d67f40..bca2b756c202 100644 > --- a/include/hw/cxl/cxl_component.h > +++ b/include/hw/cxl/cxl_component.h > @@ -10,7 +10,7 @@ > #ifndef CXL_COMPONENT_H > #define CXL_COMPONENT_H > > -/* CXL 2.0 - 8.2.4 */ > +/* CXL 3.0 - 8.2.3 */ > #define CXL2_COMPONENT_IO_REGION_SIZE 0x1000 > #define CXL2_COMPONENT_CM_REGION_SIZE 0x1000 > #define CXL2_COMPONENT_BLOCK_SIZE 0x10000 > @@ -173,7 +173,9 @@ HDM_DECODER_INIT(3); > (CXL_IDE_REGISTERS_OFFSET + CXL_IDE_REGISTERS_SIZE) > #define CXL_SNOOP_REGISTERS_SIZE 0x8 > > -QEMU_BUILD_BUG_MSG((CXL_SNOOP_REGISTERS_OFFSET + CXL_SNOOP_REGISTERS_SIZE) >= 0x1000, > +/* CXL 3.0 8.2.3 Table 8-21 */ > +QEMU_BUILD_BUG_MSG((CXL_SNOOP_REGISTERS_OFFSET + > + CXL_SNOOP_REGISTERS_SIZE) >= CXL2_COMPONENT_CM_REGION_SIZE, > "No space for registers"); > > typedef struct component_registers { >
Jonathan Cameron wrote: > On Wed, 17 May 2023 19:45:54 -0700 > Ira Weiny <ira.weiny@intel.com> wrote: > > > Magic numbers can be confusing. > > > > Use the range size define for CXL.cachemem rather than a magic number. > > Update/add spec references. > > > > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > > I guess we should do a scrub to move all refs to 3.0 soon > given it's horrible having a mixture of spec versions for the references. > > For future specs, we should only do this when sufficient X.Y references > have started to appear - I think that's true for r3.0 now. For the kernel side I think Dan is taking the 'if you are updating it then update the spec' but otherwise leave it be. So since I'm touching the code I updated it. I agree, it is a pain to have to look at the 2.0 spec but you can do it. Ira
On Thu, 18 May 2023 13:19:12 -0700 Ira Weiny <ira.weiny@intel.com> wrote: > Jonathan Cameron wrote: > > On Wed, 17 May 2023 19:45:54 -0700 > > Ira Weiny <ira.weiny@intel.com> wrote: > > > > > Magic numbers can be confusing. > > > > > > Use the range size define for CXL.cachemem rather than a magic number. > > > Update/add spec references. > > > > > > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > > > > I guess we should do a scrub to move all refs to 3.0 soon > > given it's horrible having a mixture of spec versions for the references. > > > > For future specs, we should only do this when sufficient X.Y references > > have started to appear - I think that's true for r3.0 now. > > For the kernel side I think Dan is taking the 'if you are updating it then > update the spec' but otherwise leave it be. So since I'm touching the > code I updated it. > > I agree, it is a pain to have to look at the 2.0 spec but you can do it. Only if you are either a member of the consortium, or happened to have grabbed a copy in the past I think. I've had people mentioning they can't get it today. Jonathan > > Ira
Jonathan Cameron wrote: > On Thu, 18 May 2023 13:19:12 -0700 > Ira Weiny <ira.weiny@intel.com> wrote: > > > Jonathan Cameron wrote: > > > On Wed, 17 May 2023 19:45:54 -0700 > > > Ira Weiny <ira.weiny@intel.com> wrote: > > > > > > > Magic numbers can be confusing. > > > > > > > > Use the range size define for CXL.cachemem rather than a magic number. > > > > Update/add spec references. > > > > > > > > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > > > > > > I guess we should do a scrub to move all refs to 3.0 soon > > > given it's horrible having a mixture of spec versions for the references. > > > > > > For future specs, we should only do this when sufficient X.Y references > > > have started to appear - I think that's true for r3.0 now. > > > > For the kernel side I think Dan is taking the 'if you are updating it then > > update the spec' but otherwise leave it be. So since I'm touching the > > code I updated it. > > > > I agree, it is a pain to have to look at the 2.0 spec but you can do it. > > Only if you are either a member of the consortium, or happened to have > grabbed a copy in the past I think. I've had people mentioning they can't > get it today. > Oh. :-( That seems unfortunate. I've emailed the CXL admins. Perhaps they can post an archive page or something? Ira
diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h index 52b6a2d67f40..bca2b756c202 100644 --- a/include/hw/cxl/cxl_component.h +++ b/include/hw/cxl/cxl_component.h @@ -10,7 +10,7 @@ #ifndef CXL_COMPONENT_H #define CXL_COMPONENT_H -/* CXL 2.0 - 8.2.4 */ +/* CXL 3.0 - 8.2.3 */ #define CXL2_COMPONENT_IO_REGION_SIZE 0x1000 #define CXL2_COMPONENT_CM_REGION_SIZE 0x1000 #define CXL2_COMPONENT_BLOCK_SIZE 0x10000 @@ -173,7 +173,9 @@ HDM_DECODER_INIT(3); (CXL_IDE_REGISTERS_OFFSET + CXL_IDE_REGISTERS_SIZE) #define CXL_SNOOP_REGISTERS_SIZE 0x8 -QEMU_BUILD_BUG_MSG((CXL_SNOOP_REGISTERS_OFFSET + CXL_SNOOP_REGISTERS_SIZE) >= 0x1000, +/* CXL 3.0 8.2.3 Table 8-21 */ +QEMU_BUILD_BUG_MSG((CXL_SNOOP_REGISTERS_OFFSET + + CXL_SNOOP_REGISTERS_SIZE) >= CXL2_COMPONENT_CM_REGION_SIZE, "No space for registers"); typedef struct component_registers {
Magic numbers can be confusing. Use the range size define for CXL.cachemem rather than a magic number. Update/add spec references. Signed-off-by: Ira Weiny <ira.weiny@intel.com> --- include/hw/cxl/cxl_component.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)