git: 6733401935f8 - main - nvmecontrol: add device self-test op and log page

Cy Schubert Cy.Schubert at cschubert.com
Fri Jan 8 19:27:25 UTC 2021


In message <202101081920.108JKUd2003299 at slippy.cwsent.com>, Cy Schubert 
writes:
> In message <74314BD0-A0F8-481A-93A0-28FB83CB2717 at freebsd.org>, Jessica 
> Clarke w
> rites:
> > On 8 Jan 2021, at 17:29, Chuck Tuffli <chuck at FreeBSD.org> wrote:
> > > diff --git a/sys/dev/nvme/nvme.h b/sys/dev/nvme/nvme.h
> > > index 3c159b1fd0fc..67d02ba73fd8 100644
> > > --- a/sys/dev/nvme/nvme.h
> > > +++ b/sys/dev/nvme/nvme.h
> > > @@ -1396,6 +1396,28 @@ struct nvme_command_effects_page {
> > > _Static_assert(sizeof(struct nvme_command_effects_page) == 4096,
> > >     "bad size for nvme_command_effects_page");
> > > 
> > > +struct nvme_device_self_test_page {
> > > +	uint8_t			curr_operation;
> > > +	uint8_t			curr_compl;
> > > +	uint8_t			rsvd2[2];
> > > +	struct {
> > > +		uint8_t		status;
> > > +		uint8_t		segment_num;
> > > +		uint8_t		valid_diag_info;
> > > +		uint8_t		rsvd3;
> > > +		uint64_t	poh;
> > > +		uint32_t	nsid;
> > > +		/* Define as an array to simplify alignment issues */
> > > +		uint8_t		failing_lba[8];
> > > +		uint8_t		status_code_type;
> > > +		uint8_t		status_code;
> > > +		uint8_t		vendor_specific[2];
> > > +	} __packed result[20];
> > > +} __packed __aligned(4);
> > > +
> > > +_Static_assert(sizeof(struct nvme_device_self_test_page) == 564,
> > > +    "bad size for nvme_device_self_test_page");
> > > +
> > > struct nvme_res_notification_page {
> > > 	uint64_t		log_page_count;
> > > 	uint8_t			log_page_type;
> > > @@ -2016,4 +2038,21 @@ void	nvme_resv_status_ext_swapbytes(struct n
> vme_resv
> > _status_ext *s __unused,
> > > #endif
> > > }
> > > 
> > > +static inline void
> > > +nvme_device_self_test_swapbytes(struct nvme_device_self_test_page *s __u
> nu
> > sed)
> > > +{
> > > +#if _BYTE_ORDER != _LITTLE_ENDIAN
> > > +	uint64_t failing_lba;
> > > +	uint32_t r;
> > > +
> > > +	for (r = 0; r < 20; r++) {
> > > +		s->result[r].poh = le64toh(s->result[r].poh);
> > > +		s->result[r].nsid = le32toh(s->result[r].nsid);
> > > +		/* Unaligned 64-bit loads fail on some architectures */
> > > +		memcpy(&failing_lba, s->result[r].failing_lba, sizeof(failing_l
> > ba));
> > > +		failing_lba = le64toh(failing_lba);
> > > +		memcpy(s->result[r].failing_lba, &failing_lba, sizeof(failing_l
> > ba));
> > > +	}
> > > +#endif
> > > +}
> > > #endif /* __NVME_H__ */
> >
> > Hi,
> > This broke powerpc and powerpc64:
> >
> > In file included from ioctl.c:33:
> > In file included from /home/jrtc27/src/sys/./cam/scsi/scsi_pass.h:35:
> > In file included from /home/jrtc27/src/sys/cam/cam_ccb.h:46:
> > In file included from /home/jrtc27/src/sys/cam/nvme/nvme_all.h:33:
> > /home/jrtc27/src/sys/dev/nvme/nvme.h:2052:3: error: implicitly declaring li
> br
> > ary function 'memcpy' with type 'void *(void *, const void *, unsigned long
> )'
> >  [-Werror,-Wimplicit-function-declaration]
> >                 memcpy(&failing_lba, s->result[r].failing_lba, sizeof(faili
> ng
> > _lba));
> >                 ^
> > /home/jrtc27/src/sys/dev/nvme/nvme.h:2052:3: note: include the header <stri
> ng
> > .h> or explicitly provide a declaration for 'memcpy'
> > 1 error generated.
> > --- ioctl.o ---
> > *** [ioctl.o] Error code 1
> >
> > make[7]: stopped in /home/jrtc27/src/lib/libsysdecode
> > 1 error
> >
> > Jess
> >
> >
>
> It also broke i386 (not amd64), needing a %jd instead of %ld.

The fsck_ffs commit (5cc52631b3b88dfc36d8049dc8bece8573c5f9af) has the same 
problem with i386. I'll go ahead and fix that one at noon.


-- 
Cheers,
Cy Schubert <Cy.Schubert at cschubert.com>
FreeBSD UNIX:  <cy at FreeBSD.org>   Web:  https://FreeBSD.org
NTP:           <cy at nwtime.org>    Web:  https://nwtime.org

	The need of the many outweighs the greed of the few.




More information about the dev-commits-src-all mailing list