Received-SPF: pass (google.com: domain of linux-kernel+bounces-133373-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Fri, 5 Apr 2024 17:57:11 +0100
From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: "Zhijian Li (Fujitsu)" <lizhijian@fujitsu.com>
CC: Robert Richter <rrichter@amd.com>, Davidlohr Bueso <dave@stgolabs.net>,
	Dave Jiang <dave.jiang@intel.com>, Alison Schofield
	<alison.schofield@intel.com>, Vishal Verma <vishal.l.verma@intel.com>, "Ira
 Weiny" <ira.weiny@intel.com>, Dan Williams <dan.j.williams@intel.com>,
	"linux-cxl@vger.kernel.org" <linux-cxl@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "Xingtao Yao
 (Fujitsu)" <yaoxt.fnst@fujitsu.com>
Subject: Re: [Problem ?] cxl list show nothing after reboot Re: [PATCH v2]
 cxl/pci: Fix disabling memory if DVSEC CXL Range does not match a CFMWS
 window
Message-ID: <20240405175711.000032c4@Huawei.com>
In-Reply-To: <24b8b11d-a7ec-42ce-9fa6-8a24701cfe46@fujitsu.com>
References: <20240216160113.407141-1-rrichter@amd.com>
	<b0f5e2ce-d39e-4a8e-8f2f-ffa67b604e02@fujitsu.com>
	<24b8b11d-a7ec-42ce-9fa6-8a24701cfe46@fujitsu.com>
Organization: Huawei Technologies Research and Development (UK) Ltd.
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset="US-ASCII"
Content-Transfer-Encoding: 7bit

On Tue, 26 Mar 2024 08:26:21 +0000
"Zhijian Li (Fujitsu)" <lizhijian@fujitsu.com> wrote:

> All guys,
> 
> In order to make the CXL memdev work again, i have to modify the QEMU side
> where it resets the "DVSEC CXL Control" during reboot. A draft changes is as below:
> 
> Per 8.1.3.2 DVSEC CXL Control (Offset 0Ch), Default value of BIT(2) is 0. So is it reasonable
> to have a reset dvsecs in QEMU during reboot?
> 
> Any comments @Janathan

Hi,

Sorry it took me so long to get to this.

What are you attempting to do? Use an OS reboot on QEMU to check that the flows
meant for BIOS configuration work - i.e. the OS rebuilds the state
correctly by reading the current state of the devices?

Would be good to fix that case but I want to check that's the aim before looking
too closely at this.

Thanks,

Jonathan

> 
> 
> [root@iaas-rpma qemu]# git diff
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index b0a7e9f11b64..31755a9f9aab 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -899,6 +899,26 @@ MemTxResult cxl_type3_write(PCIDevice *d, hwaddr host_addr, uint64_t data,
>       return address_space_write(as, dpa_offset, attrs, &data, size);
>   }
>   
> +static void dvsecs_ctrl_reset(CXLType3Dev *ct3d)
> +{
> +
> +     if (ct3d->sn != UI64_NULL) {
> +        pcie_dev_ser_num_init(ct3d->cxl_cstate.pdev, 0x100, ct3d->sn);
> +        ct3d->cxl_cstate.dvsec_offset = 0x100 + 0x0c;
> +    } else {
> +        ct3d->cxl_cstate.dvsec_offset = 0x100;
> +    }
> +
> +    // FIXME?: only reset ctrl instead of rebuilding the whole dvsecs
> +#if 0
> +    memcpy(pdev->config + offset + sizeof(DVSECHeader),
> +           body + sizeof(DVSECHeader),
> +           length - sizeof(DVSECHeader));
> +#else
> +    build_dvsecs(ct3d);
> +#endif
> +}
> +
>   static void ct3d_reset(DeviceState *dev)
>   {
>       CXLType3Dev *ct3d = CXL_TYPE3(dev);
> @@ -907,6 +927,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);
> +    dvsecs_ctrl_reset(ct3d);
>   
>       /*
>        * Bring up an endpoint to target with MCTP over VDM.
> 
> 
> 
> 
> 
> On 22/03/2024 11:15, Zhijian Li (Fujitsu) wrote:
> > Robert, Dan
> > 
> > It's noticed that 'cxl list' show nothing after a reboot in v6.8.(A fresh boot works)
> > The git bisection pointed to this commit.
> > 
> > Haven't investigated it deeply, I'm wondering if it's a QEMU problem or
> > something wrong with this patch.
> > 
> > 
> > Reproduce step:
> > 
> > 1. Start a cxl QEMU VM
> > 2. cxl list works
> > cxl list
> > [
> >     {
> >       "memdev":"mem0",
> >       "ram_size":2147483648,
> >       "serial":0,
> >       "host":"0000:54:00.0"
> >     },
> >     {
> >       "memdev":"mem1",
> >       "pmem_size":2147483648,
> >       "serial":0,
> >       "host":"0000:36:00.0"
> >     }
> > ]
> > 
> > 3. reboot VM
> > 4. cxl list show nothing and has following dmesg
> > 
> > cxl list
> > [
> > ]
> >     Warning: no matching devices found
> > 
> > ...
> > 
> > [    6.249188]  pci0000:53: host supports CXL
> > [    6.258168]  pci0000:35: host supports CXL
> > [    6.490568] cxl_pci 0000:54:00.0: Range register decodes outside platform defined CXL ranges.
> > [    6.494298] cxl_mem mem0: endpoint3 failed probe
> > [    6.506072] cxl_pci 0000:36:00.0: Range register decodes outside platform defined CXL ranges.
> > [    6.515092] cxl_mem mem1: endpoint3 failed probe
> > [   12.181188] kauditd_printk_skb: 18 callbacks suppressed
> > 
> > 
> > Thanks
> > Zhijian
> > 
> > 
> > On 17/02/2024 00:01, Robert Richter wrote:  
> >> The Linux CXL subsystem is built on the assumption that HPA == SPA.
> >> That is, the host physical address (HPA) the HDM decoder registers are
> >> programmed with are system physical addresses (SPA).
> >>
> >> During HDM decoder setup, the DVSEC CXL range registers (cxl-3.1,
> >> 8.1.3.8) are checked if the memory is enabled and the CXL range is in
> >> a HPA window that is described in a CFMWS structure of the CXL host
> >> bridge (cxl-3.1, 9.18.1.3).
> >>
> >> Now, if the HPA is not an SPA, the CXL range does not match a CFMWS
> >> window and the CXL memory range will be disabled then. The HDM decoder
> >> stops working which causes system memory being disabled and further a
> >> system hang during HDM decoder initialization, typically when a CXL
> >> enabled kernel boots.
> >>
> >> Prevent a system hang and do not disable the HDM decoder if the
> >> decoder's CXL range is not found in a CFMWS window.
> >>
> >> Note the change only fixes a hardware hang, but does not implement
> >> HPA/SPA translation. Support for this can be added in a follow on
> >> patch series.
> >>
> >> Signed-off-by: Robert Richter <rrichter@amd.com>
> >> ---
> >>    drivers/cxl/core/pci.c | 4 ++--
> >>    1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> >> index a0e7ed5ae25f..18616ca873e5 100644
> >> --- a/drivers/cxl/core/pci.c
> >> +++ b/drivers/cxl/core/pci.c
> >> @@ -478,8 +478,8 @@ int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm,
> >>    	}
> >>    
> >>    	if (!allowed) {
> >> -		cxl_set_mem_enable(cxlds, 0);
> >> -		info->mem_enabled = 0;
> >> +		dev_err(dev, "Range register decodes outside platform defined CXL ranges.\n");
> >> +		return -ENXIO;
> >>    	}
> >>    
> >>    	/