diff mbox series

[v4,08/11] block: introduce BIP_CHECK_GUARD/REFTAG/APPTAG bip_flags

Message ID 20241016112912.63542-9-anuj20.g@samsung.com
State Superseded
Headers show
Series Read/Write with meta/integrity | expand

Commit Message

Anuj Gupta Oct. 16, 2024, 11:29 a.m. UTC 
This patch introduces BIP_CHECK_GUARD/REFTAG/APPTAG bip_flags which
indicate how the hardware should check the integrity payload. The
driver can now just rely on block layer flags, and doesn't need to
know the integrity source. Submitter of PI decides which tags to check.
This would also give us a unified interface for user and kernel
generated integrity.

Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>
---
 block/bio-integrity.c         |  5 +++++
 drivers/nvme/host/core.c      | 11 +++--------
 include/linux/bio-integrity.h |  6 +++++-
 3 files changed, 13 insertions(+), 9 deletions(-)

Comments

Christoph Hellwig Oct. 17, 2024, 8:12 a.m. UTC | #1 
On Wed, Oct 16, 2024 at 04:59:09PM +0530, Anuj Gupta wrote:
> This patch introduces BIP_CHECK_GUARD/REFTAG/APPTAG bip_flags which
> indicate how the hardware should check the integrity payload. The
> driver can now just rely on block layer flags, and doesn't need to
> know the integrity source. Submitter of PI decides which tags to check.
> This would also give us a unified interface for user and kernel
> generated integrity.

The conversion of the existing logic looks good, but the BIP_CHECK_APPTAG
flag is completely unreferenced.
Anuj Gupta Oct. 17, 2024, 10:46 a.m. UTC | #2 
On Thu, Oct 17, 2024 at 10:12:23AM +0200, Christoph Hellwig wrote:
> On Wed, Oct 16, 2024 at 04:59:09PM +0530, Anuj Gupta wrote:
> > This patch introduces BIP_CHECK_GUARD/REFTAG/APPTAG bip_flags which
> > indicate how the hardware should check the integrity payload. The
> > driver can now just rely on block layer flags, and doesn't need to
> > know the integrity source. Submitter of PI decides which tags to check.
> > This would also give us a unified interface for user and kernel
> > generated integrity.
> 
> The conversion of the existing logic looks good, but the BIP_CHECK_APPTAG
> flag is completely unreferenced.

It's being used by the nvme and scsi patch later. Should I introduce this
flag later in either nvme or scsi patch where we actually use it.
> 
>
Christoph Hellwig Oct. 17, 2024, 2:37 p.m. UTC | #3 
On Thu, Oct 17, 2024 at 04:16:13PM +0530, Anuj Gupta wrote:
> On Thu, Oct 17, 2024 at 10:12:23AM +0200, Christoph Hellwig wrote:
> > On Wed, Oct 16, 2024 at 04:59:09PM +0530, Anuj Gupta wrote:
> > > This patch introduces BIP_CHECK_GUARD/REFTAG/APPTAG bip_flags which
> > > indicate how the hardware should check the integrity payload. The
> > > driver can now just rely on block layer flags, and doesn't need to
> > > know the integrity source. Submitter of PI decides which tags to check.
> > > This would also give us a unified interface for user and kernel
> > > generated integrity.
> > 
> > The conversion of the existing logic looks good, but the BIP_CHECK_APPTAG
> > flag is completely unreferenced.
> 
> It's being used by the nvme and scsi patch later. Should I introduce this
> flag later in either nvme or scsi patch where we actually use it.

Maybe a separate one?  Or at least very clearly state that the other two
are conversion of the existing semantics, while this one is new.
diff mbox series

Patch

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 341a0382befd..d3c8b56d3fe6 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -436,6 +436,11 @@  bool bio_integrity_prep(struct bio *bio)
 	if (bi->csum_type == BLK_INTEGRITY_CSUM_IP)
 		bip->bip_flags |= BIP_IP_CHECKSUM;
 
+	/* describe what tags to check in payload */
+	if (bi->csum_type)
+		bip->bip_flags |= BIP_CHECK_GUARD;
+	if (bi->flags & BLK_INTEGRITY_REF_TAG)
+		bip->bip_flags |= BIP_CHECK_REFTAG;
 	if (bio_integrity_add_page(bio, virt_to_page(buf), len,
 			offset_in_page(buf)) < len) {
 		printk(KERN_ERR "could not attach integrity payload\n");
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 43d73d31c66f..211f44cc02a3 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1004,18 +1004,13 @@  static inline blk_status_t nvme_setup_rw(struct nvme_ns *ns,
 			control |= NVME_RW_PRINFO_PRACT;
 		}
 
-		switch (ns->head->pi_type) {
-		case NVME_NS_DPS_PI_TYPE3:
+		if (bio_integrity_flagged(req->bio, BIP_CHECK_GUARD))
 			control |= NVME_RW_PRINFO_PRCHK_GUARD;
-			break;
-		case NVME_NS_DPS_PI_TYPE1:
-		case NVME_NS_DPS_PI_TYPE2:
-			control |= NVME_RW_PRINFO_PRCHK_GUARD |
-					NVME_RW_PRINFO_PRCHK_REF;
+		if (bio_integrity_flagged(req->bio, BIP_CHECK_REFTAG)) {
+			control |= NVME_RW_PRINFO_PRCHK_REF;
 			if (op == nvme_cmd_zone_append)
 				control |= NVME_RW_APPEND_PIREMAP;
 			nvme_set_ref_tag(ns, cmnd, req);
-			break;
 		}
 	}
 
diff --git a/include/linux/bio-integrity.h b/include/linux/bio-integrity.h
index 529ec7a8df20..a9dd0594dfc8 100644
--- a/include/linux/bio-integrity.h
+++ b/include/linux/bio-integrity.h
@@ -11,6 +11,9 @@  enum bip_flags {
 	BIP_DISK_NOCHECK	= 1 << 3, /* disable disk integrity checking */
 	BIP_IP_CHECKSUM		= 1 << 4, /* IP checksum */
 	BIP_COPY_USER		= 1 << 5, /* Kernel bounce buffer in use */
+	BIP_CHECK_GUARD		= 1 << 6, /* guard check */
+	BIP_CHECK_REFTAG	= 1 << 7, /* reftag check */
+	BIP_CHECK_APPTAG	= 1 << 8, /* apptag check */
 };
 
 struct bio_integrity_payload {
@@ -41,7 +44,8 @@  struct uio_meta {
 };
 
 #define BIP_CLONE_FLAGS (BIP_MAPPED_INTEGRITY | BIP_CTRL_NOCHECK | \
-			 BIP_DISK_NOCHECK | BIP_IP_CHECKSUM)
+			 BIP_DISK_NOCHECK | BIP_IP_CHECKSUM | \
+			 BIP_CHECK_GUARD | BIP_CHECK_REFTAG | BIP_CHECK_APPTAG)
 
 #ifdef CONFIG_BLK_DEV_INTEGRITY