| Message ID | 20230908073152.4386-5-dave@stgolabs.net |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/cxl: Support for scan media | expand |
On Fri, 8 Sep 2023 00:31:52 -0700 Davidlohr Bueso <dave@stgolabs.net> wrote: > Iterate over the list keeping the output payload size into account, > returning the results from a previous scan media operation. The > scan media operation does not fail prematurely due to device being > out of storage, so this implementation does not deal with the > retry/restart functionality. > > Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> A few comments inline but nothing to stop me carrying this on my staging tree. (gitlab.com/jic23/qemu cxl-*latestdate* So I'll apply it there. Updates welcome or I'll act on comments when this gets to the top of our queue for upstreaming. Jonathan > --- > hw/cxl/cxl-mailbox-utils.c | 84 +++++++++++++++++++++++++++++++++++++ > include/hw/cxl/cxl_device.h | 1 + > 2 files changed, 85 insertions(+) > > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c > index 399ac03962db..109b9ecfabf1 100644 > --- a/hw/cxl/cxl-mailbox-utils.c > +++ b/hw/cxl/cxl-mailbox-utils.c > @@ -82,6 +82,7 @@ enum { > #define CLEAR_POISON 0x2 > #define GET_SCAN_MEDIA_CAPABILITIES 0x3 > #define SCAN_MEDIA 0x4 > + #define GET_SCAN_MEDIA_RESULTS 0x5 > DCD_CONFIG = 0x48, > #define GET_DC_CONFIG 0x0 > #define GET_DYN_CAP_EXT_LIST 0x1 > @@ -1267,6 +1268,8 @@ static void __do_scan_media(CXLType3Dev *ct3d) > ct3d->poison_list_cnt == results_cnt) { > cxl_clear_poison_list_overflowed(ct3d); > } > + /* scan media has run since last conventional reset */ > + ct3d->scan_media_hasrun = true; > } > > /* > @@ -1371,6 +1374,85 @@ static CXLRetCode cmd_media_scan_media(const struct cxl_cmd *cmd, > return CXL_MBOX_BG_STARTED; > } > > +/* > + * CXL r3.0 section 8.2.9.8.4.6: Get Scan Media Results > + */ > +static CXLRetCode cmd_media_get_scan_media_results(const struct cxl_cmd *cmd, > + uint8_t *payload_in, > + size_t len_in, > + uint8_t *payload_out, > + size_t *len_out, > + CXLCCI *cci) > +{ > + struct get_scan_media_results_out_pl { > + uint64_t dpa_restart; > + uint64_t length; > + uint8_t flags; > + uint8_t rsvd1; > + uint16_t count; > + uint8_t rsvd2[0xc]; > + struct { > + uint64_t addr; > + uint32_t length; > + uint32_t resv; > + } QEMU_PACKED records[]; > + } QEMU_PACKED; > + > + struct get_scan_media_results_out_pl *out = (void *)payload_out; > + CXLType3Dev *ct3d = CXL_TYPE3(cci->d); > + CXLPoisonList *scan_media_results = &ct3d->scan_media_results; > + CXLPoison *ent, *next; > + uint16_t total_count = 0, record_count = 0, i = 0; > + uint16_t out_pl_len; > + > + if (!ct3d->scan_media_hasrun) { > + return CXL_MBOX_UNSUPPORTED; > + } > + > + /* > + * Calculate limits, all entries are within the same > + * address range of the last scan media call. > + */ > + QLIST_FOREACH(ent, scan_media_results, node) { > + size_t rec_size = record_count * sizeof(out->records[0]); > + > + if (sizeof(*out) + rec_size < CXL_MAILBOX_MAX_PAYLOAD_SIZE) { > + record_count++; > + } > + total_count++; > + } > + > + out_pl_len = sizeof(*out) + record_count * sizeof(out->records[0]); > + assert(out_pl_len <= CXL_MAILBOX_MAX_PAYLOAD_SIZE); Isn't aim of the if in the loop above to ensure this never happens? I'd hope that code is obvious enough we don't need the additional assert here. > + > + memset(out, 0, out_pl_len); > + QLIST_FOREACH_SAFE(ent, scan_media_results, node, next) { > + uint64_t start, stop; > + > + if (i == record_count) { > + break; > + } > + > + start = ROUND_DOWN(ent->start, 64ull); > + stop = ROUND_DOWN(ent->start, 64ull) + ent->length; I think we control the way these all turn up so they are multiples of 64. So this rounding shouldn't be needed (or am I missing something?) > + stq_le_p(&out->records[i].addr, start | (ent->type & 0x7)); define for that 0x7 > + stl_le_p(&out->records[i].length, (stop - start) / CXL_CACHE_LINE_SIZE); > + i++; > + > + /* consume the returning entry */ > + QLIST_REMOVE(ent, node); > + g_free(ent); > + } > + > + stw_le_p(&out->count, record_count); > + if (total_count > record_count) { > + out->flags = (1 << 0); /* More Media Error Records */ Define perhaps. > + } > + > + *len_out = out_pl_len; > + return CXL_MBOX_SUCCESS; > +} > + > /* > * CXL r3.0 section 8.2.9.8.9.1: Dynamic Capacity Configuration > */ > @@ -1803,6 +1885,8 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = { > cmd_media_get_scan_media_capabilities, 16, 0 }, > [MEDIA_AND_POISON][SCAN_MEDIA] = { "MEDIA_AND_POISON_SCAN_MEDIA", > cmd_media_scan_media, 17, BACKGROUND_OPERATION }, > + [MEDIA_AND_POISON][GET_SCAN_MEDIA_RESULTS] = { "MEDIA_AND_POISON_GET_SCAN_MEDIA_RESULTS", I'll wrap this line whilst applying. Shortly I'm going to send out a series hammering down all the line lengths in the CXL code. > + cmd_media_get_scan_media_results, 0, 0 }, > }; > > static const struct cxl_cmd cxl_cmd_set_dcd[256][256] = { > diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h > index eb5c5284fa9f..e9d130e5c988 100644 > --- a/include/hw/cxl/cxl_device.h > +++ b/include/hw/cxl/cxl_device.h > @@ -484,6 +484,7 @@ struct CXLType3Dev { > /* Poison Injection - backup */ > CXLPoisonList poison_list_bkp; > CXLPoisonList scan_media_results; > + bool scan_media_hasrun; > > struct dynamic_capacity { > HostMemoryBackend *host_dc;
On Tue, 12 Sep 2023, Jonathan Cameron wrote: >On Fri, 8 Sep 2023 00:31:52 -0700 >Davidlohr Bueso <dave@stgolabs.net> wrote: > >> Iterate over the list keeping the output payload size into account, >> returning the results from a previous scan media operation. The >> scan media operation does not fail prematurely due to device being >> out of storage, so this implementation does not deal with the >> retry/restart functionality. >> >> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> >A few comments inline but nothing to stop me carrying this on my >staging tree. (gitlab.com/jic23/qemu cxl-*latestdate* > >So I'll apply it there. Updates welcome or I'll act on comments when >this gets to the top of our queue for upstreaming. I'll send follow up patches for this series once your new latest branch is published. Thanks, Davidlohr
On Tue, 12 Sep 2023 20:33:52 -0700 Davidlohr Bueso <dave@stgolabs.net> wrote: > On Tue, 12 Sep 2023, Jonathan Cameron wrote: > > >On Fri, 8 Sep 2023 00:31:52 -0700 > >Davidlohr Bueso <dave@stgolabs.net> wrote: > > > >> Iterate over the list keeping the output payload size into account, > >> returning the results from a previous scan media operation. The > >> scan media operation does not fail prematurely due to device being > >> out of storage, so this implementation does not deal with the > >> retry/restart functionality. > >> > >> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> > >A few comments inline but nothing to stop me carrying this on my > >staging tree. (gitlab.com/jic23/qemu cxl-*latestdate* > > > >So I'll apply it there. Updates welcome or I'll act on comments when > >this gets to the top of our queue for upstreaming. > > I'll send follow up patches for this series once your new latest > branch is published. > > Thanks, > Davidlohr https://gitlab.com/jic23/qemu/-/tree/cxl-2023-09-13 Fresh and tasty. Includes 3 series that I've posted with the hope of them merging soon and another one I'll post in a few mins to deal with over long lines... Jonathan
On Wed, 13 Sep 2023, Jonathan Cameron wrote: >https://gitlab.com/jic23/qemu/-/tree/cxl-2023-09-13 > >Fresh and tasty. Includes 3 series that I've posted with the hope >of them merging soon and another one I'll post in a few mins to >deal with over long lines... fyi I'm getting the below: /usr/lib64/gcc/x86_64-suse-linux/13/../../../../x86_64-suse-linux/bin/ld: libcommon.fa.p/hw_cxl_vendor_skhynix_skhynix_niagara.c.o: in function `cxl_niagara_reset': /home/dave/git/qemu-jic/build/../hw/cxl/vendor/skhynix/skhynix_niagara.c:477: undefined reference to `ct3d_reset' /usr/lib64/gcc/x86_64-suse-linux/13/../../../../x86_64-suse-linux/bin/ld: libcommon.fa.p/hw_cxl_vendor_skhynix_skhynix_niagara.c.o: in function `cxl_niagara_exit': /home/dave/git/qemu-jic/build/../hw/cxl/vendor/skhynix/skhynix_niagara.c:466: undefined reference to `ct3_exit' /usr/lib64/gcc/x86_64-suse-linux/13/../../../../x86_64-suse-linux/bin/ld: libcommon.fa.p/hw_cxl_vendor_skhynix_skhynix_niagara.c.o: in function `cxl_niagara_realize': /home/dave/git/qemu-jic/build/../hw/cxl/vendor/skhynix/skhynix_niagara.c:428: undefined reference to `ct3_realize' /usr/lib64/gcc/x86_64-suse-linux/13/../../../../x86_64-suse-linux/bin/ld: libcommon.fa.p/hw_cxl_vendor_skhynix_skhynix_niagara.c.o: in function `cxl_niagara_reset': /home/dave/git/qemu-jic/build/../hw/cxl/vendor/skhynix/skhynix_niagara.c:478: undefined reference to `cxl_add_cci_commands'
On Fri, 15 Sep 2023 17:11:27 -0700 Davidlohr Bueso <dave@stgolabs.net> wrote: > On Wed, 13 Sep 2023, Jonathan Cameron wrote: > > >https://gitlab.com/jic23/qemu/-/tree/cxl-2023-09-13 > > > >Fresh and tasty. Includes 3 series that I've posted with the hope > >of them merging soon and another one I'll post in a few mins to > >deal with over long lines... > > fyi I'm getting the below: > > /usr/lib64/gcc/x86_64-suse-linux/13/../../../../x86_64-suse-linux/bin/ld: libcommon.fa.p/hw_cxl_vendor_skhynix_skhynix_niagara.c.o: in function `cxl_niagara_reset': > /home/dave/git/qemu-jic/build/../hw/cxl/vendor/skhynix/skhynix_niagara.c:477: undefined reference to `ct3d_reset' > /usr/lib64/gcc/x86_64-suse-linux/13/../../../../x86_64-suse-linux/bin/ld: libcommon.fa.p/hw_cxl_vendor_skhynix_skhynix_niagara.c.o: in function `cxl_niagara_exit': > /home/dave/git/qemu-jic/build/../hw/cxl/vendor/skhynix/skhynix_niagara.c:466: undefined reference to `ct3_exit' > /usr/lib64/gcc/x86_64-suse-linux/13/../../../../x86_64-suse-linux/bin/ld: libcommon.fa.p/hw_cxl_vendor_skhynix_skhynix_niagara.c.o: in function `cxl_niagara_realize': > /home/dave/git/qemu-jic/build/../hw/cxl/vendor/skhynix/skhynix_niagara.c:428: undefined reference to `ct3_realize' > /usr/lib64/gcc/x86_64-suse-linux/13/../../../../x86_64-suse-linux/bin/ld: libcommon.fa.p/hw_cxl_vendor_skhynix_skhynix_niagara.c.o: in function `cxl_niagara_reset': > /home/dave/git/qemu-jic/build/../hw/cxl/vendor/skhynix/skhynix_niagara.c:478: undefined reference to `cxl_add_cci_commands' Thanks. I'd not noticed the niagara stuff doesn't have it's own kconfig with dependencies. Gregory, want to send a proper fix defining the various Kconfigs down to the actual build directory? I'd guess the lazy approach of making CXL_VENDOR depends on CXL_MEM_DEVICE should work in meantime. Jonathan
On Mon, Sep 18, 2023 at 12:11:28PM +0100, Jonathan Cameron wrote: > On Fri, 15 Sep 2023 17:11:27 -0700 > Davidlohr Bueso <dave@stgolabs.net> wrote: > > > On Wed, 13 Sep 2023, Jonathan Cameron wrote: > > > > >https://gitlab.com/jic23/qemu/-/tree/cxl-2023-09-13 > > > > > >Fresh and tasty. Includes 3 series that I've posted with the hope > > >of them merging soon and another one I'll post in a few mins to > > >deal with over long lines... > > > > fyi I'm getting the below: > > > > /usr/lib64/gcc/x86_64-suse-linux/13/../../../../x86_64-suse-linux/bin/ld: libcommon.fa.p/hw_cxl_vendor_skhynix_skhynix_niagara.c.o: in function `cxl_niagara_reset': > > /home/dave/git/qemu-jic/build/../hw/cxl/vendor/skhynix/skhynix_niagara.c:477: undefined reference to `ct3d_reset' > > /usr/lib64/gcc/x86_64-suse-linux/13/../../../../x86_64-suse-linux/bin/ld: libcommon.fa.p/hw_cxl_vendor_skhynix_skhynix_niagara.c.o: in function `cxl_niagara_exit': > > /home/dave/git/qemu-jic/build/../hw/cxl/vendor/skhynix/skhynix_niagara.c:466: undefined reference to `ct3_exit' > > /usr/lib64/gcc/x86_64-suse-linux/13/../../../../x86_64-suse-linux/bin/ld: libcommon.fa.p/hw_cxl_vendor_skhynix_skhynix_niagara.c.o: in function `cxl_niagara_realize': > > /home/dave/git/qemu-jic/build/../hw/cxl/vendor/skhynix/skhynix_niagara.c:428: undefined reference to `ct3_realize' > > /usr/lib64/gcc/x86_64-suse-linux/13/../../../../x86_64-suse-linux/bin/ld: libcommon.fa.p/hw_cxl_vendor_skhynix_skhynix_niagara.c.o: in function `cxl_niagara_reset': > > /home/dave/git/qemu-jic/build/../hw/cxl/vendor/skhynix/skhynix_niagara.c:478: undefined reference to `cxl_add_cci_commands' > > Thanks. I'd not noticed the niagara stuff doesn't have it's own kconfig with dependencies. > Gregory, want to send a proper fix defining the various Kconfigs down to the actual build directory? > > I'd guess the lazy approach of making CXL_VENDOR > depends on CXL_MEM_DEVICE > should work in meantime. > > Jonathan > woops! My bad. I will push up a patch later today with that + limiting niagara to linux-only (already fixed that up, just haven't pushed it). I don't think any other changes have been made to that line, so i'll just push an update to the Niagara patch itself and you can hot-swap them on the branch if you like. ~Gregory
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c index 399ac03962db..109b9ecfabf1 100644 --- a/hw/cxl/cxl-mailbox-utils.c +++ b/hw/cxl/cxl-mailbox-utils.c @@ -82,6 +82,7 @@ enum { #define CLEAR_POISON 0x2 #define GET_SCAN_MEDIA_CAPABILITIES 0x3 #define SCAN_MEDIA 0x4 + #define GET_SCAN_MEDIA_RESULTS 0x5 DCD_CONFIG = 0x48, #define GET_DC_CONFIG 0x0 #define GET_DYN_CAP_EXT_LIST 0x1 @@ -1267,6 +1268,8 @@ static void __do_scan_media(CXLType3Dev *ct3d) ct3d->poison_list_cnt == results_cnt) { cxl_clear_poison_list_overflowed(ct3d); } + /* scan media has run since last conventional reset */ + ct3d->scan_media_hasrun = true; } /* @@ -1371,6 +1374,85 @@ static CXLRetCode cmd_media_scan_media(const struct cxl_cmd *cmd, return CXL_MBOX_BG_STARTED; } +/* + * CXL r3.0 section 8.2.9.8.4.6: Get Scan Media Results + */ +static CXLRetCode cmd_media_get_scan_media_results(const struct cxl_cmd *cmd, + uint8_t *payload_in, + size_t len_in, + uint8_t *payload_out, + size_t *len_out, + CXLCCI *cci) +{ + struct get_scan_media_results_out_pl { + uint64_t dpa_restart; + uint64_t length; + uint8_t flags; + uint8_t rsvd1; + uint16_t count; + uint8_t rsvd2[0xc]; + struct { + uint64_t addr; + uint32_t length; + uint32_t resv; + } QEMU_PACKED records[]; + } QEMU_PACKED; + + struct get_scan_media_results_out_pl *out = (void *)payload_out; + CXLType3Dev *ct3d = CXL_TYPE3(cci->d); + CXLPoisonList *scan_media_results = &ct3d->scan_media_results; + CXLPoison *ent, *next; + uint16_t total_count = 0, record_count = 0, i = 0; + uint16_t out_pl_len; + + if (!ct3d->scan_media_hasrun) { + return CXL_MBOX_UNSUPPORTED; + } + + /* + * Calculate limits, all entries are within the same + * address range of the last scan media call. + */ + QLIST_FOREACH(ent, scan_media_results, node) { + size_t rec_size = record_count * sizeof(out->records[0]); + + if (sizeof(*out) + rec_size < CXL_MAILBOX_MAX_PAYLOAD_SIZE) { + record_count++; + } + total_count++; + } + + out_pl_len = sizeof(*out) + record_count * sizeof(out->records[0]); + assert(out_pl_len <= CXL_MAILBOX_MAX_PAYLOAD_SIZE); + + memset(out, 0, out_pl_len); + QLIST_FOREACH_SAFE(ent, scan_media_results, node, next) { + uint64_t start, stop; + + if (i == record_count) { + break; + } + + start = ROUND_DOWN(ent->start, 64ull); + stop = ROUND_DOWN(ent->start, 64ull) + ent->length; + stq_le_p(&out->records[i].addr, start | (ent->type & 0x7)); + stl_le_p(&out->records[i].length, (stop - start) / CXL_CACHE_LINE_SIZE); + i++; + + /* consume the returning entry */ + QLIST_REMOVE(ent, node); + g_free(ent); + } + + stw_le_p(&out->count, record_count); + if (total_count > record_count) { + out->flags = (1 << 0); /* More Media Error Records */ + } + + *len_out = out_pl_len; + return CXL_MBOX_SUCCESS; +} + /* * CXL r3.0 section 8.2.9.8.9.1: Dynamic Capacity Configuration */ @@ -1803,6 +1885,8 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = { cmd_media_get_scan_media_capabilities, 16, 0 }, [MEDIA_AND_POISON][SCAN_MEDIA] = { "MEDIA_AND_POISON_SCAN_MEDIA", cmd_media_scan_media, 17, BACKGROUND_OPERATION }, + [MEDIA_AND_POISON][GET_SCAN_MEDIA_RESULTS] = { "MEDIA_AND_POISON_GET_SCAN_MEDIA_RESULTS", + cmd_media_get_scan_media_results, 0, 0 }, }; static const struct cxl_cmd cxl_cmd_set_dcd[256][256] = { diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h index eb5c5284fa9f..e9d130e5c988 100644 --- a/include/hw/cxl/cxl_device.h +++ b/include/hw/cxl/cxl_device.h @@ -484,6 +484,7 @@ struct CXLType3Dev { /* Poison Injection - backup */ CXLPoisonList poison_list_bkp; CXLPoisonList scan_media_results; + bool scan_media_hasrun; struct dynamic_capacity { HostMemoryBackend *host_dc;
Iterate over the list keeping the output payload size into account, returning the results from a previous scan media operation. The scan media operation does not fail prematurely due to device being out of storage, so this implementation does not deal with the retry/restart functionality. Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> --- hw/cxl/cxl-mailbox-utils.c | 84 +++++++++++++++++++++++++++++++++++++ include/hw/cxl/cxl_device.h | 1 + 2 files changed, 85 insertions(+)