On Tue, 2 Apr 2024 09:46:47 +0800
Li Zhijian <lizhij...@fujitsu.com> wrote:
> After the kernel commit
> 0cab68720598 ("cxl/pci: Fix disabling memory if DVSEC CXL Range does not
> match a CFMWS window")
Fixes tag seems appropriate.
> CXL type3 devices cannot be enabled again after the reboot because this
> flag was not reset.
>
> This flag could be changed by the firmware or OS, let it have a
> reset(default) value in reboot so that the OS can read its clean status.
Good find. I think we should aim for a fix that is less fragile to future
code rearrangement etc.
>
> Signed-off-by: Li Zhijian <lizhij...@fujitsu.com>
> ---
> hw/mem/cxl_type3.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index ad2fe7d463fb..3fe136053390 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -305,7 +305,8 @@ static void build_dvsecs(CXLType3Dev *ct3d)
>
> dvsec = (uint8_t *)&(CXLDVSECDevice){
> .cap = 0x1e,
> - .ctrl = 0x2,
> +#define CT3D_DEVSEC_CXL_CTRL 0x2
> + .ctrl = CT3D_DEVSEC_CXL_CTRL,
Naming doesn't make it clear the define is a reset value / default value.
> .status2 = 0x2,
> .range1_size_hi = range1_size_hi,
> .range1_size_lo = range1_size_lo,
> @@ -906,6 +907,16 @@ MemTxResult cxl_type3_write(PCIDevice *d, hwaddr
> host_addr, uint64_t data,
> return address_space_write(as, dpa_offset, attrs, &data, size);
> }
>
> +/* Reset DVSEC CXL Control */
> +static void ct3d_dvsec_cxl_ctrl_reset(CXLType3Dev *ct3d)
> +{
> + uint16_t offset = first_dvsec_offset(ct3d);
This relies to much on the current memory layout. We should doing a search
of config space to find the right entry, or we should cache a pointer to
the relevant structure when we fill it in the first time.
> + CXLDVSECDevice *dvsec;
> +
> + dvsec = (CXLDVSECDevice *)(ct3d->cxl_cstate.pdev->config + offset);
> + dvsec->ctrl = CT3D_DEVSEC_CXL_CTRL;
> +}
> +
> static void ct3d_reset(DeviceState *dev)
> {
> CXLType3Dev *ct3d = CXL_TYPE3(dev);
> @@ -914,6 +925,7 @@ static void ct3d_reset(DeviceState *dev)
>
> cxl_component_register_init_common(reg_state, write_msk,
> CXL2_TYPE3_DEVICE);
> cxl_device_register_init_t3(ct3d);
> + ct3d_dvsec_cxl_ctrl_reset(ct3d);
>
> /*
> * Bring up an endpoint to target with MCTP over VDM.
