[Question] How to set up DVSEC CXL Range Registers for DCD devices

[Question] How to set up DVSEC CXL Range Registers for DCD devices

[Fan Ni]

Hi,

When preparing the DCD patches for QEMU emulation and testing, I hit an issue
when trying to load the cxl modules. The issue happens when the kernel tries
to do cxl pci probe where it checks whether the media is ready
through cxl_await_media_ready. The function will check dvsec mem range.

In current QEMU code, the dvsec range registers for type3 memdev is set
only for static ram and pmem in function build_dvsecs, which will cause
the kernel fails the check of media ready of DCD devices without static
capacity.

About the issue, I have following questions that want to ask and
clarify,

1. do we allow DCD device have no static (RAM/PMEM) capacity at all and
only dynamic capacity?

2. Do we need to set dvsec range registers for dynamic capacity? And how
if needed?

In current Qemu code, we assume at least one of ram or pmem static capacity
exists. The code logic of build_dvsec looks like below,
if (vmem){
	set rang1 base and size with vmem info;
	if (pmem)
		set range2 base and size with pmem info;
} else {
	set range1 base and size with pmem info;
}

To fix the issue above and make kernel works for DCD (with no static ram/pmem),
I updated the logic as below and it seems to pass the modprobe.

if (vmem) {
	set rang1 base and size with vmem info;
	if (pmem)
		set range2 base and size with pmem info;
	else if (dc){
		set range2 base and size with dc info;
	}
} else if (pmem){
	set rang1 base and size with pmem info;
	if (dc)
		set range2 base and size with dc info;
} else 
	set rang1 base and size with dc info;

Is the above logic reasonable? If not, what is the right way?


Thanks,
Fan


-- 
Fan Ni <nifan@outlook.com>

Re: [Question] How to set up DVSEC CXL Range Registers for DCD devices

[Jonathan Cameron]

On Thu, 29 Jun 2023 10:10:48 -0700
Fan Ni <nifan@outlook.com> wrote:

> Hi,
> 
> When preparing the DCD patches for QEMU emulation and testing, I hit an issue
> when trying to load the cxl modules. The issue happens when the kernel tries
> to do cxl pci probe where it checks whether the media is ready
> through cxl_await_media_ready. The function will check dvsec mem range.
> 
> In current QEMU code, the dvsec range registers for type3 memdev is set
> only for static ram and pmem in function build_dvsecs, which will cause
> the kernel fails the check of media ready of DCD devices without static
> capacity.
> 
> About the issue, I have following questions that want to ask and
> clarify,
> 
> 1. do we allow DCD device have no static (RAM/PMEM) capacity at all and
> only dynamic capacity?

Yes.  That will probably be a reasonably common configuration so definitely
want that to work.

> 
> 2. Do we need to set dvsec range registers for dynamic capacity? And how
> if needed?

hmm.  I think we should.  Given a DCD device is not an eRCD (though it may
be operating in RCD mode) the memory types are all from CDAT - so I think
we 'could' use either one or two ranges to cover the DCD range.

Desired interleave is messy but it's only a hint anyway so meh.
Memory active timeout might potentially be different for say a PMEM region
between a volatile range and DCD range but I guess it just needs to be
the biggest of anything covered.

The writeable registers are fine as we should be using HDM decoders anyway
if DCD is involved so they are ignored.

Whilst QEMU doesn't support it (as no RCH support yet) I can't see a general
reason why you can't have DCD on an RCH host as long as the BIOS knows how
to deal with HDM decoders so that the routing is set up to the DCD address
range as well as the other two.  Probably easier if the device doesn't have
all of static volatile region, static PMEM region, DCD region... though might
be able to make that work on some RCH hosts.

Jonathan

> 
> In current Qemu code, we assume at least one of ram or pmem static capacity
> exists. The code logic of build_dvsec looks like below,
> if (vmem){
> 	set rang1 base and size with vmem info;
> 	if (pmem)
> 		set range2 base and size with pmem info;
> } else {
> 	set range1 base and size with pmem info;
> }
> 
> To fix the issue above and make kernel works for DCD (with no static ram/pmem),
> I updated the logic as below and it seems to pass the modprobe.
> 
> if (vmem) {
> 	set rang1 base and size with vmem info;
> 	if (pmem)
> 		set range2 base and size with pmem info;
> 	else if (dc){
> 		set range2 base and size with dc info;
> 	}
> } else if (pmem){
> 	set rang1 base and size with pmem info;
> 	if (dc)
> 		set range2 base and size with dc info;
> } else 
> 	set rang1 base and size with dc info;
> 
> Is the above logic reasonable? If not, what is the right way?

Seems like it covers the devices I'd expect people to build, but
we do want to poke the corners with the QEMU emulation so
should probably allow for all 3 being described...  Maybe not
to start with though.  So for now, perhaps print a warning if you
hit all 3 at once?


> 
> 
> Thanks,
> Fan
> 
> 

Re: [Question] How to set up DVSEC CXL Range Registers for DCD devices

[Davidlohr Bueso]

On Thu, 06 Jul 2023, Jonathan Cameron wrote:

>On Thu, 29 Jun 2023 10:10:48 -0700
>Fan Ni <nifan@outlook.com> wrote:
>
>> Hi,
>>
>> When preparing the DCD patches for QEMU emulation and testing, I hit an issue
>> when trying to load the cxl modules. The issue happens when the kernel tries
>> to do cxl pci probe where it checks whether the media is ready
>> through cxl_await_media_ready. The function will check dvsec mem range.
>>
>> In current QEMU code, the dvsec range registers for type3 memdev is set
>> only for static ram and pmem in function build_dvsecs, which will cause
>> the kernel fails the check of media ready of DCD devices without static
>> capacity.
>>
>> About the issue, I have following questions that want to ask and
>> clarify,
>>
>> 1. do we allow DCD device have no static (RAM/PMEM) capacity at all and
>> only dynamic capacity?
>
>Yes.  That will probably be a reasonably common configuration so definitely
>want that to work.
>
>>
>> 2. Do we need to set dvsec range registers for dynamic capacity? And how
>> if needed?
>
>hmm.  I think we should.  Given a DCD device is not an eRCD (though it may
>be operating in RCD mode) the memory types are all from CDAT - so I think
>we 'could' use either one or two ranges to cover the DCD range.
>
>Desired interleave is messy but it's only a hint anyway so meh.
>Memory active timeout might potentially be different for say a PMEM region
>between a volatile range and DCD range but I guess it just needs to be
>the biggest of anything covered.

Btw, we probably want to robustify cxl_await_media_ready() to 1) check for
Mem_HwInit_Mode=1 and 2) rely more on Memory_Active_Timeout to set the module
parameter.

Also, I was under the impression we wouldn't want to set the dvsec for dynamic
capacity exactly because of the active memory timeout, which makes no sense
for dcd.

Thanks,
Davidlohr

Re: [Question] How to set up DVSEC CXL Range Registers for DCD devices

[Jonathan Cameron]

On Mon, 10 Jul 2023 08:36:39 -0700
Davidlohr Bueso <dave@stgolabs.net> wrote:

> On Thu, 06 Jul 2023, Jonathan Cameron wrote:
> 
> >On Thu, 29 Jun 2023 10:10:48 -0700
> >Fan Ni <nifan@outlook.com> wrote:
> >  
> >> Hi,
> >>
> >> When preparing the DCD patches for QEMU emulation and testing, I hit an issue
> >> when trying to load the cxl modules. The issue happens when the kernel tries
> >> to do cxl pci probe where it checks whether the media is ready
> >> through cxl_await_media_ready. The function will check dvsec mem range.
> >>
> >> In current QEMU code, the dvsec range registers for type3 memdev is set
> >> only for static ram and pmem in function build_dvsecs, which will cause
> >> the kernel fails the check of media ready of DCD devices without static
> >> capacity.
> >>
> >> About the issue, I have following questions that want to ask and
> >> clarify,
> >>
> >> 1. do we allow DCD device have no static (RAM/PMEM) capacity at all and
> >> only dynamic capacity?  
> >
> >Yes.  That will probably be a reasonably common configuration so definitely
> >want that to work.
> >  
> >>
> >> 2. Do we need to set dvsec range registers for dynamic capacity? And how
> >> if needed?  
> >
> >hmm.  I think we should.  Given a DCD device is not an eRCD (though it may
> >be operating in RCD mode) the memory types are all from CDAT - so I think
> >we 'could' use either one or two ranges to cover the DCD range.
> >
> >Desired interleave is messy but it's only a hint anyway so meh.
> >Memory active timeout might potentially be different for say a PMEM region
> >between a volatile range and DCD range but I guess it just needs to be
> >the biggest of anything covered.  
> 
> Btw, we probably want to robustify cxl_await_media_ready() to 1) check for
> Mem_HwInit_Mode=1 and 2) rely more on Memory_Active_Timeout to set the module
> parameter.
> 
> Also, I was under the impression we wouldn't want to set the dvsec for dynamic
> capacity exactly because of the active memory timeout, which makes no sense
> for dcd.

Hmm. Not sure if it makes sense or not - there is still memory and you might
still want to let it finish waking up, even if definition of waking up is
different. Feels like this is a question to take to CXL SSWG for confirmation,
unless someone can find an explicit spec reference to say one way or the other.

Jonathan


> 
> Thanks,
> Davidlohr

Re: [Question] How to set up DVSEC CXL Range Registers for DCD devices

[Jonathan Cameron]

On Tue, 11 Jul 2023 09:19:40 +0100
Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:

> On Mon, 10 Jul 2023 08:36:39 -0700
> Davidlohr Bueso <dave@stgolabs.net> wrote:
> 
> > On Thu, 06 Jul 2023, Jonathan Cameron wrote:
> >   
> > >On Thu, 29 Jun 2023 10:10:48 -0700
> > >Fan Ni <nifan@outlook.com> wrote:
> > >    
> > >> Hi,
> > >>
> > >> When preparing the DCD patches for QEMU emulation and testing, I hit an issue
> > >> when trying to load the cxl modules. The issue happens when the kernel tries
> > >> to do cxl pci probe where it checks whether the media is ready
> > >> through cxl_await_media_ready. The function will check dvsec mem range.
> > >>
> > >> In current QEMU code, the dvsec range registers for type3 memdev is set
> > >> only for static ram and pmem in function build_dvsecs, which will cause
> > >> the kernel fails the check of media ready of DCD devices without static
> > >> capacity.
> > >>
> > >> About the issue, I have following questions that want to ask and
> > >> clarify,
> > >>
> > >> 1. do we allow DCD device have no static (RAM/PMEM) capacity at all and
> > >> only dynamic capacity?    
> > >
> > >Yes.  That will probably be a reasonably common configuration so definitely
> > >want that to work.
> > >    
> > >>
> > >> 2. Do we need to set dvsec range registers for dynamic capacity? And how
> > >> if needed?    
> > >
> > >hmm.  I think we should.  Given a DCD device is not an eRCD (though it may
> > >be operating in RCD mode) the memory types are all from CDAT - so I think
> > >we 'could' use either one or two ranges to cover the DCD range.
> > >
> > >Desired interleave is messy but it's only a hint anyway so meh.
> > >Memory active timeout might potentially be different for say a PMEM region
> > >between a volatile range and DCD range but I guess it just needs to be
> > >the biggest of anything covered.    
> > 
> > Btw, we probably want to robustify cxl_await_media_ready() to 1) check for
> > Mem_HwInit_Mode=1 and 2) rely more on Memory_Active_Timeout to set the module
> > parameter.
> > 
> > Also, I was under the impression we wouldn't want to set the dvsec for dynamic
> > capacity exactly because of the active memory timeout, which makes no sense
> > for dcd.  
> 
> Hmm. Not sure if it makes sense or not - there is still memory and you might
> still want to let it finish waking up, even if definition of waking up is
> different. Feels like this is a question to take to CXL SSWG for confirmation,
> unless someone can find an explicit spec reference to say one way or the other.
> 

Question asked - so let us take this discussion to that closed forum until we
have an answer to act on that we can report back here.

Jonathan

> Jonathan
> 
> 
> > 
> > Thanks,
> > Davidlohr  
> 
>