* [PATCH v2] mem/cxl_type3: support 3, 6, 12 and 16 interleave ways @ 2024-04-07 3:06 Yao Xingtao via 2024-04-24 1:36 ` Xingtao Yao (Fujitsu) via 0 siblings, 1 reply; 6+ messages in thread From: Yao Xingtao via @ 2024-04-07 3:06 UTC (permalink / raw) To: jonathan.cameron, fan.ni; +Cc: qemu-devel, Yao Xingtao Since the kernel does not check the interleave capability, a 3-way, 6-way, 12-way or 16-way region can be create normally. Applications can access the memory of 16-way region normally because qemu can convert hpa to dpa correctly for the power of 2 interleave ways, after kernel implementing the check, this kind of region will not be created any more. For non power of 2 interleave ways, applications could not access the memory normally and may occur some unexpected behaviors, such as segmentation fault. So implements this feature is needed. Link: https://lore.kernel.org/linux-cxl/3e84b919-7631-d1db-3e1d-33000f3f3868@fujitsu.com/ Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com> --- hw/mem/cxl_type3.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c index b0a7e9f11b..d6ef784e96 100644 --- a/hw/mem/cxl_type3.c +++ b/hw/mem/cxl_type3.c @@ -805,10 +805,17 @@ static bool cxl_type3_dpa(CXLType3Dev *ct3d, hwaddr host_addr, uint64_t *dpa) continue; } - *dpa = dpa_base + - ((MAKE_64BIT_MASK(0, 8 + ig) & hpa_offset) | - ((MAKE_64BIT_MASK(8 + ig + iw, 64 - 8 - ig - iw) & hpa_offset) - >> iw)); + if (iw < 8) { + *dpa = dpa_base + + ((MAKE_64BIT_MASK(0, 8 + ig) & hpa_offset) | + ((MAKE_64BIT_MASK(8 + ig + iw, 64 - 8 - ig - iw) & hpa_offset) + >> iw)); + } else { + *dpa = dpa_base + + ((MAKE_64BIT_MASK(0, 8 + ig) & hpa_offset) | + ((((MAKE_64BIT_MASK(ig + iw, 64 - ig - iw) & hpa_offset) + >> (ig + iw)) / 3) << (ig + 8))); + } return true; } @@ -906,6 +913,9 @@ static void ct3d_reset(DeviceState *dev) uint32_t *write_msk = ct3d->cxl_cstate.crb.cache_mem_regs_write_mask; cxl_component_register_init_common(reg_state, write_msk, CXL2_TYPE3_DEVICE); + ARRAY_FIELD_DP32(reg_state, CXL_HDM_DECODER_CAPABILITY, 3_6_12_WAY, 1); + ARRAY_FIELD_DP32(reg_state, CXL_HDM_DECODER_CAPABILITY, 16_WAY, 1); + cxl_device_register_init_t3(ct3d); /* -- 2.37.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH v2] mem/cxl_type3: support 3, 6, 12 and 16 interleave ways 2024-04-07 3:06 [PATCH v2] mem/cxl_type3: support 3, 6, 12 and 16 interleave ways Yao Xingtao via @ 2024-04-24 1:36 ` Xingtao Yao (Fujitsu) via 2024-04-30 14:43 ` Jonathan Cameron via 0 siblings, 1 reply; 6+ messages in thread From: Xingtao Yao (Fujitsu) via @ 2024-04-24 1:36 UTC (permalink / raw) To: Xingtao Yao (Fujitsu), jonathan.cameron@huawei.com, fan.ni@samsung.com Cc: qemu-devel@nongnu.org ping. > -----Original Message----- > From: Yao Xingtao <yaoxt.fnst@fujitsu.com> > Sent: Sunday, April 7, 2024 11:07 AM > To: jonathan.cameron@huawei.com; fan.ni@samsung.com > Cc: qemu-devel@nongnu.org; Yao, Xingtao/姚 幸涛 <yaoxt.fnst@fujitsu.com> > Subject: [PATCH v2] mem/cxl_type3: support 3, 6, 12 and 16 interleave ways > > Since the kernel does not check the interleave capability, a > 3-way, 6-way, 12-way or 16-way region can be create normally. > > Applications can access the memory of 16-way region normally because > qemu can convert hpa to dpa correctly for the power of 2 interleave > ways, after kernel implementing the check, this kind of region will > not be created any more. > > For non power of 2 interleave ways, applications could not access the > memory normally and may occur some unexpected behaviors, such as > segmentation fault. > > So implements this feature is needed. > > Link: > https://lore.kernel.org/linux-cxl/3e84b919-7631-d1db-3e1d-33000f3f3868@fujits > u.com/ > Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com> > --- > hw/mem/cxl_type3.c | 18 ++++++++++++++---- > 1 file changed, 14 insertions(+), 4 deletions(-) > > diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c > index b0a7e9f11b..d6ef784e96 100644 > --- a/hw/mem/cxl_type3.c > +++ b/hw/mem/cxl_type3.c > @@ -805,10 +805,17 @@ static bool cxl_type3_dpa(CXLType3Dev *ct3d, hwaddr > host_addr, uint64_t *dpa) > continue; > } > > - *dpa = dpa_base + > - ((MAKE_64BIT_MASK(0, 8 + ig) & hpa_offset) | > - ((MAKE_64BIT_MASK(8 + ig + iw, 64 - 8 - ig - iw) & hpa_offset) > - >> iw)); > + if (iw < 8) { > + *dpa = dpa_base + > + ((MAKE_64BIT_MASK(0, 8 + ig) & hpa_offset) | > + ((MAKE_64BIT_MASK(8 + ig + iw, 64 - 8 - ig - iw) & hpa_offset) > + >> iw)); > + } else { > + *dpa = dpa_base + > + ((MAKE_64BIT_MASK(0, 8 + ig) & hpa_offset) | > + ((((MAKE_64BIT_MASK(ig + iw, 64 - ig - iw) & hpa_offset) > + >> (ig + iw)) / 3) << (ig + 8))); > + } > > return true; > } > @@ -906,6 +913,9 @@ static void ct3d_reset(DeviceState *dev) > uint32_t *write_msk = ct3d->cxl_cstate.crb.cache_mem_regs_write_mask; > > cxl_component_register_init_common(reg_state, write_msk, > CXL2_TYPE3_DEVICE); > + ARRAY_FIELD_DP32(reg_state, CXL_HDM_DECODER_CAPABILITY, > 3_6_12_WAY, 1); > + ARRAY_FIELD_DP32(reg_state, CXL_HDM_DECODER_CAPABILITY, > 16_WAY, 1); > + > cxl_device_register_init_t3(ct3d); > > /* > -- > 2.37.3 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mem/cxl_type3: support 3, 6, 12 and 16 interleave ways 2024-04-24 1:36 ` Xingtao Yao (Fujitsu) via @ 2024-04-30 14:43 ` Jonathan Cameron via 2024-05-07 0:22 ` Xingtao Yao (Fujitsu) via 0 siblings, 1 reply; 6+ messages in thread From: Jonathan Cameron via @ 2024-04-30 14:43 UTC (permalink / raw) To: Xingtao Yao (Fujitsu); +Cc: fan.ni@samsung.com, qemu-devel@nongnu.org On Wed, 24 Apr 2024 01:36:56 +0000 "Xingtao Yao (Fujitsu)" <yaoxt.fnst@fujitsu.com> wrote: > ping. > > > -----Original Message----- > > From: Yao Xingtao <yaoxt.fnst@fujitsu.com> > > Sent: Sunday, April 7, 2024 11:07 AM > > To: jonathan.cameron@huawei.com; fan.ni@samsung.com > > Cc: qemu-devel@nongnu.org; Yao, Xingtao/姚 幸涛 <yaoxt.fnst@fujitsu.com> > > Subject: [PATCH v2] mem/cxl_type3: support 3, 6, 12 and 16 interleave ways > > > > Since the kernel does not check the interleave capability, a > > 3-way, 6-way, 12-way or 16-way region can be create normally. > > > > Applications can access the memory of 16-way region normally because > > qemu can convert hpa to dpa correctly for the power of 2 interleave > > ways, after kernel implementing the check, this kind of region will > > not be created any more. > > > > For non power of 2 interleave ways, applications could not access the > > memory normally and may occur some unexpected behaviors, such as > > segmentation fault. > > > > So implements this feature is needed. > > > > Link: > > https://lore.kernel.org/linux-cxl/3e84b919-7631-d1db-3e1d-33000f3f3868@fujits > > u.com/ > > Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com> > > --- > > hw/mem/cxl_type3.c | 18 ++++++++++++++---- > > 1 file changed, 14 insertions(+), 4 deletions(-) > > > > diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c > > index b0a7e9f11b..d6ef784e96 100644 > > --- a/hw/mem/cxl_type3.c > > +++ b/hw/mem/cxl_type3.c > > @@ -805,10 +805,17 @@ static bool cxl_type3_dpa(CXLType3Dev *ct3d, hwaddr > > host_addr, uint64_t *dpa) > > continue; > > } > > > > - *dpa = dpa_base + > > - ((MAKE_64BIT_MASK(0, 8 + ig) & hpa_offset) | > > - ((MAKE_64BIT_MASK(8 + ig + iw, 64 - 8 - ig - iw) & hpa_offset) > > - >> iw)); > > + if (iw < 8) { > > + *dpa = dpa_base + > > + ((MAKE_64BIT_MASK(0, 8 + ig) & hpa_offset) | > > + ((MAKE_64BIT_MASK(8 + ig + iw, 64 - 8 - ig - iw) & hpa_offset) > > + >> iw)); > > + } else { > > + *dpa = dpa_base + > > + ((MAKE_64BIT_MASK(0, 8 + ig) & hpa_offset) | > > + ((((MAKE_64BIT_MASK(ig + iw, 64 - ig - iw) & hpa_offset) > > + >> (ig + iw)) / 3) << (ig + 8))); > > + } > > > > return true; > > } > > @@ -906,6 +913,9 @@ static void ct3d_reset(DeviceState *dev) > > uint32_t *write_msk = ct3d->cxl_cstate.crb.cache_mem_regs_write_mask; > > > > cxl_component_register_init_common(reg_state, write_msk, > > CXL2_TYPE3_DEVICE); > > + ARRAY_FIELD_DP32(reg_state, CXL_HDM_DECODER_CAPABILITY, > > 3_6_12_WAY, 1); > > + ARRAY_FIELD_DP32(reg_state, CXL_HDM_DECODER_CAPABILITY, > > 16_WAY, 1); > > + Why here rather than in hdm_reg_init_common()? It's constant data and is currently being set to 0 in there. > > cxl_device_register_init_t3(ct3d); > > > > /* > > -- > > 2.37.3 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH v2] mem/cxl_type3: support 3, 6, 12 and 16 interleave ways 2024-04-30 14:43 ` Jonathan Cameron via @ 2024-05-07 0:22 ` Xingtao Yao (Fujitsu) via 2024-05-07 16:30 ` Jonathan Cameron via 0 siblings, 1 reply; 6+ messages in thread From: Xingtao Yao (Fujitsu) via @ 2024-05-07 0:22 UTC (permalink / raw) To: Jonathan Cameron; +Cc: fan.ni@samsung.com, qemu-devel@nongnu.org > -----Original Message----- > From: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Sent: Tuesday, April 30, 2024 10:43 PM > To: Yao, Xingtao/姚 幸涛 <yaoxt.fnst@fujitsu.com> > Cc: fan.ni@samsung.com; qemu-devel@nongnu.org > Subject: Re: [PATCH v2] mem/cxl_type3: support 3, 6, 12 and 16 interleave ways > > On Wed, 24 Apr 2024 01:36:56 +0000 > "Xingtao Yao (Fujitsu)" <yaoxt.fnst@fujitsu.com> wrote: > > > ping. > > > > > -----Original Message----- > > > From: Yao Xingtao <yaoxt.fnst@fujitsu.com> > > > Sent: Sunday, April 7, 2024 11:07 AM > > > To: jonathan.cameron@huawei.com; fan.ni@samsung.com > > > Cc: qemu-devel@nongnu.org; Yao, Xingtao/姚 幸涛 > <yaoxt.fnst@fujitsu.com> > > > Subject: [PATCH v2] mem/cxl_type3: support 3, 6, 12 and 16 interleave ways > > > > > > Since the kernel does not check the interleave capability, a > > > 3-way, 6-way, 12-way or 16-way region can be create normally. > > > > > > Applications can access the memory of 16-way region normally because > > > qemu can convert hpa to dpa correctly for the power of 2 interleave > > > ways, after kernel implementing the check, this kind of region will > > > not be created any more. > > > > > > For non power of 2 interleave ways, applications could not access the > > > memory normally and may occur some unexpected behaviors, such as > > > segmentation fault. > > > > > > So implements this feature is needed. > > > > > > Link: > > > > https://lore.kernel.org/linux-cxl/3e84b919-7631-d1db-3e1d-33000f3f3868@fujits > > > u.com/ > > > Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com> > > > --- > > > hw/mem/cxl_type3.c | 18 ++++++++++++++---- > > > 1 file changed, 14 insertions(+), 4 deletions(-) > > > > > > diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c > > > index b0a7e9f11b..d6ef784e96 100644 > > > --- a/hw/mem/cxl_type3.c > > > +++ b/hw/mem/cxl_type3.c > > > @@ -805,10 +805,17 @@ static bool cxl_type3_dpa(CXLType3Dev *ct3d, > hwaddr > > > host_addr, uint64_t *dpa) > > > continue; > > > } > > > > > > - *dpa = dpa_base + > > > - ((MAKE_64BIT_MASK(0, 8 + ig) & hpa_offset) | > > > - ((MAKE_64BIT_MASK(8 + ig + iw, 64 - 8 - ig - iw) & hpa_offset) > > > - >> iw)); > > > + if (iw < 8) { > > > + *dpa = dpa_base + > > > + ((MAKE_64BIT_MASK(0, 8 + ig) & hpa_offset) | > > > + ((MAKE_64BIT_MASK(8 + ig + iw, 64 - 8 - ig - iw) & > hpa_offset) > > > + >> iw)); > > > + } else { > > > + *dpa = dpa_base + > > > + ((MAKE_64BIT_MASK(0, 8 + ig) & hpa_offset) | > > > + ((((MAKE_64BIT_MASK(ig + iw, 64 - ig - iw) & hpa_offset) > > > + >> (ig + iw)) / 3) << (ig + 8))); > > > + } > > > > > > return true; > > > } > > > @@ -906,6 +913,9 @@ static void ct3d_reset(DeviceState *dev) > > > uint32_t *write_msk = > ct3d->cxl_cstate.crb.cache_mem_regs_write_mask; > > > > > > cxl_component_register_init_common(reg_state, write_msk, > > > CXL2_TYPE3_DEVICE); > > > + ARRAY_FIELD_DP32(reg_state, CXL_HDM_DECODER_CAPABILITY, > > > 3_6_12_WAY, 1); > > > + ARRAY_FIELD_DP32(reg_state, CXL_HDM_DECODER_CAPABILITY, > > > 16_WAY, 1); > > > + > > Why here rather than in hdm_reg_init_common()? > It's constant data and is currently being set to 0 in there. according to the CXL specifications (8.2.4.20.1 CXL HDM Decoder Capability Register (Offset 00h)), this feature is only applicable to cxl.mem, upstream switch port and CXL host bridges shall hardwrite these bits to 0. so I think it would be more appropriate to set these bits here. > > > > cxl_device_register_init_t3(ct3d); > > > > > > /* > > > -- > > > 2.37.3 > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mem/cxl_type3: support 3, 6, 12 and 16 interleave ways 2024-05-07 0:22 ` Xingtao Yao (Fujitsu) via @ 2024-05-07 16:30 ` Jonathan Cameron via 2024-05-08 0:52 ` Xingtao Yao (Fujitsu) via 0 siblings, 1 reply; 6+ messages in thread From: Jonathan Cameron via @ 2024-05-07 16:30 UTC (permalink / raw) To: Xingtao Yao (Fujitsu); +Cc: fan.ni@samsung.com, qemu-devel@nongnu.org On Tue, 7 May 2024 00:22:00 +0000 "Xingtao Yao (Fujitsu)" <yaoxt.fnst@fujitsu.com> wrote: > > -----Original Message----- > > From: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > Sent: Tuesday, April 30, 2024 10:43 PM > > To: Yao, Xingtao/姚 幸涛 <yaoxt.fnst@fujitsu.com> > > Cc: fan.ni@samsung.com; qemu-devel@nongnu.org > > Subject: Re: [PATCH v2] mem/cxl_type3: support 3, 6, 12 and 16 interleave ways > > > > On Wed, 24 Apr 2024 01:36:56 +0000 > > "Xingtao Yao (Fujitsu)" <yaoxt.fnst@fujitsu.com> wrote: > > > > > ping. > > > > > > > -----Original Message----- > > > > From: Yao Xingtao <yaoxt.fnst@fujitsu.com> > > > > Sent: Sunday, April 7, 2024 11:07 AM > > > > To: jonathan.cameron@huawei.com; fan.ni@samsung.com > > > > Cc: qemu-devel@nongnu.org; Yao, Xingtao/姚 幸涛 > > <yaoxt.fnst@fujitsu.com> > > > > Subject: [PATCH v2] mem/cxl_type3: support 3, 6, 12 and 16 interleave ways > > > > > > > > Since the kernel does not check the interleave capability, a > > > > 3-way, 6-way, 12-way or 16-way region can be create normally. > > > > > > > > Applications can access the memory of 16-way region normally because > > > > qemu can convert hpa to dpa correctly for the power of 2 interleave > > > > ways, after kernel implementing the check, this kind of region will > > > > not be created any more. > > > > > > > > For non power of 2 interleave ways, applications could not access the > > > > memory normally and may occur some unexpected behaviors, such as > > > > segmentation fault. > > > > > > > > So implements this feature is needed. > > > > > > > > Link: > > > > > > https://lore.kernel.org/linux-cxl/3e84b919-7631-d1db-3e1d-33000f3f3868@fujits > > > > u.com/ > > > > Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com> > > > > --- > > > > hw/mem/cxl_type3.c | 18 ++++++++++++++---- > > > > 1 file changed, 14 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c > > > > index b0a7e9f11b..d6ef784e96 100644 > > > > --- a/hw/mem/cxl_type3.c > > > > +++ b/hw/mem/cxl_type3.c > > > > @@ -805,10 +805,17 @@ static bool cxl_type3_dpa(CXLType3Dev *ct3d, > > hwaddr > > > > host_addr, uint64_t *dpa) > > > > continue; > > > > } > > > > > > > > - *dpa = dpa_base + > > > > - ((MAKE_64BIT_MASK(0, 8 + ig) & hpa_offset) | > > > > - ((MAKE_64BIT_MASK(8 + ig + iw, 64 - 8 - ig - iw) & hpa_offset) > > > > - >> iw)); > > > > + if (iw < 8) { > > > > + *dpa = dpa_base + > > > > + ((MAKE_64BIT_MASK(0, 8 + ig) & hpa_offset) | > > > > + ((MAKE_64BIT_MASK(8 + ig + iw, 64 - 8 - ig - iw) & > > hpa_offset) > > > > + >> iw)); > > > > + } else { > > > > + *dpa = dpa_base + > > > > + ((MAKE_64BIT_MASK(0, 8 + ig) & hpa_offset) | > > > > + ((((MAKE_64BIT_MASK(ig + iw, 64 - ig - iw) & hpa_offset) > > > > + >> (ig + iw)) / 3) << (ig + 8))); > > > > + } > > > > > > > > return true; > > > > } > > > > @@ -906,6 +913,9 @@ static void ct3d_reset(DeviceState *dev) > > > > uint32_t *write_msk = > > ct3d->cxl_cstate.crb.cache_mem_regs_write_mask; > > > > > > > > cxl_component_register_init_common(reg_state, write_msk, > > > > CXL2_TYPE3_DEVICE); > > > > + ARRAY_FIELD_DP32(reg_state, CXL_HDM_DECODER_CAPABILITY, > > > > 3_6_12_WAY, 1); > > > > + ARRAY_FIELD_DP32(reg_state, CXL_HDM_DECODER_CAPABILITY, > > > > 16_WAY, 1); > > > > + > > > > Why here rather than in hdm_reg_init_common()? > > It's constant data and is currently being set to 0 in there. > > according to the CXL specifications (8.2.4.20.1 CXL HDM Decoder Capability Register (Offset 00h)), > this feature is only applicable to cxl.mem, upstream switch port and CXL host bridges shall hardwrite > these bits to 0. > > so I think it would be more appropriate to set these bits here. I don't follow. hdm_init_common() (sorry wrong function name above) has some type specific stuff already to show how this can be done. I'd prefer to minimize what we set directly in the ct3d_reset() call because it loses the connection to the rest of the register setup. Jonathan Jonathan > > > > > > > cxl_device_register_init_t3(ct3d); > > > > > > > > /* > > > > -- > > > > 2.37.3 > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH v2] mem/cxl_type3: support 3, 6, 12 and 16 interleave ways 2024-05-07 16:30 ` Jonathan Cameron via @ 2024-05-08 0:52 ` Xingtao Yao (Fujitsu) via 0 siblings, 0 replies; 6+ messages in thread From: Xingtao Yao (Fujitsu) via @ 2024-05-08 0:52 UTC (permalink / raw) To: Jonathan Cameron; +Cc: fan.ni@samsung.com, qemu-devel@nongnu.org > -----Original Message----- > From: Jonathan Cameron <Jonathan.Cameron@Huawei.com> > Sent: Wednesday, May 8, 2024 12:31 AM > To: Yao, Xingtao/姚 幸涛 <yaoxt.fnst@fujitsu.com> > Cc: fan.ni@samsung.com; qemu-devel@nongnu.org > Subject: Re: [PATCH v2] mem/cxl_type3: support 3, 6, 12 and 16 interleave ways > > On Tue, 7 May 2024 00:22:00 +0000 > "Xingtao Yao (Fujitsu)" <yaoxt.fnst@fujitsu.com> wrote: > > > > -----Original Message----- > > > From: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > > Sent: Tuesday, April 30, 2024 10:43 PM > > > To: Yao, Xingtao/姚 幸涛 <yaoxt.fnst@fujitsu.com> > > > Cc: fan.ni@samsung.com; qemu-devel@nongnu.org > > > Subject: Re: [PATCH v2] mem/cxl_type3: support 3, 6, 12 and 16 interleave > ways > > > > > > On Wed, 24 Apr 2024 01:36:56 +0000 > > > "Xingtao Yao (Fujitsu)" <yaoxt.fnst@fujitsu.com> wrote: > > > > > > > ping. > > > > > > > > > -----Original Message----- > > > > > From: Yao Xingtao <yaoxt.fnst@fujitsu.com> > > > > > Sent: Sunday, April 7, 2024 11:07 AM > > > > > To: jonathan.cameron@huawei.com; fan.ni@samsung.com > > > > > Cc: qemu-devel@nongnu.org; Yao, Xingtao/姚 幸涛 > > > <yaoxt.fnst@fujitsu.com> > > > > > Subject: [PATCH v2] mem/cxl_type3: support 3, 6, 12 and 16 interleave > ways > > > > > > > > > > Since the kernel does not check the interleave capability, a > > > > > 3-way, 6-way, 12-way or 16-way region can be create normally. > > > > > > > > > > Applications can access the memory of 16-way region normally because > > > > > qemu can convert hpa to dpa correctly for the power of 2 interleave > > > > > ways, after kernel implementing the check, this kind of region will > > > > > not be created any more. > > > > > > > > > > For non power of 2 interleave ways, applications could not access the > > > > > memory normally and may occur some unexpected behaviors, such as > > > > > segmentation fault. > > > > > > > > > > So implements this feature is needed. > > > > > > > > > > Link: > > > > > > > > > https://lore.kernel.org/linux-cxl/3e84b919-7631-d1db-3e1d-33000f3f3868@fujits > > > > > u.com/ > > > > > Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com> > > > > > --- > > > > > hw/mem/cxl_type3.c | 18 ++++++++++++++---- > > > > > 1 file changed, 14 insertions(+), 4 deletions(-) > > > > > > > > > > diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c > > > > > index b0a7e9f11b..d6ef784e96 100644 > > > > > --- a/hw/mem/cxl_type3.c > > > > > +++ b/hw/mem/cxl_type3.c > > > > > @@ -805,10 +805,17 @@ static bool cxl_type3_dpa(CXLType3Dev *ct3d, > > > hwaddr > > > > > host_addr, uint64_t *dpa) > > > > > continue; > > > > > } > > > > > > > > > > - *dpa = dpa_base + > > > > > - ((MAKE_64BIT_MASK(0, 8 + ig) & hpa_offset) | > > > > > - ((MAKE_64BIT_MASK(8 + ig + iw, 64 - 8 - ig - iw) & > hpa_offset) > > > > > - >> iw)); > > > > > + if (iw < 8) { > > > > > + *dpa = dpa_base + > > > > > + ((MAKE_64BIT_MASK(0, 8 + ig) & hpa_offset) | > > > > > + ((MAKE_64BIT_MASK(8 + ig + iw, 64 - 8 - ig - iw) & > > > hpa_offset) > > > > > + >> iw)); > > > > > + } else { > > > > > + *dpa = dpa_base + > > > > > + ((MAKE_64BIT_MASK(0, 8 + ig) & hpa_offset) | > > > > > + ((((MAKE_64BIT_MASK(ig + iw, 64 - ig - iw) & > hpa_offset) > > > > > + >> (ig + iw)) / 3) << (ig + 8))); > > > > > + } > > > > > > > > > > return true; > > > > > } > > > > > @@ -906,6 +913,9 @@ static void ct3d_reset(DeviceState *dev) > > > > > uint32_t *write_msk = > > > ct3d->cxl_cstate.crb.cache_mem_regs_write_mask; > > > > > > > > > > cxl_component_register_init_common(reg_state, write_msk, > > > > > CXL2_TYPE3_DEVICE); > > > > > + ARRAY_FIELD_DP32(reg_state, > CXL_HDM_DECODER_CAPABILITY, > > > > > 3_6_12_WAY, 1); > > > > > + ARRAY_FIELD_DP32(reg_state, > CXL_HDM_DECODER_CAPABILITY, > > > > > 16_WAY, 1); > > > > > + > > > > > > Why here rather than in hdm_reg_init_common()? > > > It's constant data and is currently being set to 0 in there. > > > > according to the CXL specifications (8.2.4.20.1 CXL HDM Decoder Capability > Register (Offset 00h)), > > this feature is only applicable to cxl.mem, upstream switch port and CXL host > bridges shall hardwrite > > these bits to 0. > > > > so I think it would be more appropriate to set these bits here. > I don't follow. hdm_init_common() (sorry wrong function name above) > has some type specific stuff already to show how this can be done. > I'd prefer to minimize what we set directly in the ct3d_reset() call > because it loses the connection to the rest of the register setup. thanks, got it. > > Jonathan > > > > Jonathan > > > > > > > > > > > > cxl_device_register_init_t3(ct3d); > > > > > > > > > > /* > > > > > -- > > > > > 2.37.3 > > > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-05-08 0:53 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-04-07 3:06 [PATCH v2] mem/cxl_type3: support 3, 6, 12 and 16 interleave ways Yao Xingtao via 2024-04-24 1:36 ` Xingtao Yao (Fujitsu) via 2024-04-30 14:43 ` Jonathan Cameron via 2024-05-07 0:22 ` Xingtao Yao (Fujitsu) via 2024-05-07 16:30 ` Jonathan Cameron via 2024-05-08 0:52 ` Xingtao Yao (Fujitsu) via
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).