On Thu, Apr 24, 2025 at 11:42:59AM +0100, Jonathan Cameron wrote:
> On Mon, 17 Mar 2025 16:31:30 +0000
> anisa.su...@gmail.com wrote:
>
> > From: Anisa Su <anisa...@samsung.com>
> >
> > Add dsmas_flags field to DC Region struct in preparation for next
> > command, which returns the dsmas flags in the response.
> >
> > Signed-off-by: Anisa Su <anisa...@samsung.com>
> > ---
> > hw/mem/cxl_type3.c | 2 ++
> > include/hw/cxl/cxl_device.h | 1 +
> > 2 files changed, 3 insertions(+)
> >
> > diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> > index 731497ebda..452a0c101a 100644
> > --- a/hw/mem/cxl_type3.c
> > +++ b/hw/mem/cxl_type3.c
> > @@ -237,6 +237,8 @@ static int ct3_build_cdat_table(CDATSubHeader
> > ***cdat_table, void *priv)
> > ct3d->dc.regions[i].len,
> > false, true, region_base);
> > ct3d->dc.regions[i].dsmadhandle = dsmad_handle - 1;
> > + CDATDsmas *dsmas = (CDATDsmas *) table[cur_ent +
> > CT3_CDAT_DSMAS];
> > + ct3d->dc.regions[i].dsmas_flags = dsmas->flags;
>
Hi Jonathan,
Thanks for the feedback.
> This is relying to much on the ordering of creating fields in
> ct3_build_cdat_entries_for_mr().
I am not sure whether I understand this clearly.
In current qemu implemtation, each mr (ram,pmem or dc region) will have the
whole set of cdat table entries (dsmas, dslbis0-3, etc), so as long as we point
to the right table entry, we can get the table correctly.
What do you mean "the ordering of creating fields"?
>
> I'd rather you just stored the information flags is built from in CXLDCRegion
> and then built the field that is wonderfully called 'Note' in the DC region
This sentence is kind of broken for me, not totally clear what you are
suggesting :-(. Can you explain more?
Are you suggesting not directly take dsmas->flags as dsmas_flags, but
use bit op to generate the value used in Table 7-66 in cxl spec 3.2?
> configuration in 6.2 spec. I've sent a mail to see if we can clean that
6.2 spec???
> 'what is the field called' question for future spec releases.
>
> Whilst the flag definitions cross refer the CDAT spec, the actual locations
> of those flags matches, but doesn't cross refer so maybe in the future
> we will have other flags in here and locations might not match.
For the flags stored in dsmas table, do we expect there can be more than those
defined in Table 7-66 in spec 3.2?
Fan
>
> >
> > cur_ent += CT3_CDAT_NUM_ENTRIES;
> > region_base += ct3d->dc.regions[i].len;
> > diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
> > index bebed04085..81b826f570 100644
> > --- a/include/hw/cxl/cxl_device.h
> > +++ b/include/hw/cxl/cxl_device.h
> > @@ -609,6 +609,7 @@ typedef struct CXLDCRegion {
> > uint8_t flags;
> > unsigned long *blk_bitmap;
> > uint64_t supported_blk_size_bitmask;
> > + uint8_t dsmas_flags;
> > } CXLDCRegion;
> >
> > typedef struct CXLSetFeatureInfo {
>
