Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 30 Apr 2020 00:11:19 -0700
From:   Eric Biggers <ebiggers@kernel.org>
To:     Satya Tangirala <satyat@google.com>
Cc:     linux-block@vger.kernel.org, linux-scsi@vger.kernel.org,
        linux-fscrypt@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        linux-f2fs-devel@lists.sourceforge.net, linux-ext4@vger.kernel.org,
        Barani Muthukumaran <bmuthuku@qti.qualcomm.com>,
        Kuohong Wang <kuohong.wang@mediatek.com>,
        Kim Boojin <boojin.kim@samsung.com>
Subject: Re: [PATCH v11 04/12] block: Make blk-integrity preclude hardware
 inline encryption
Message-ID: <20200430071119.GC16238@sol.localdomain>
References: <20200429072121.50094-1-satyat@google.com>
 <20200429072121.50094-5-satyat@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200429072121.50094-5-satyat@google.com>
Sender: linux-ext4-owner@vger.kernel.org
Precedence: bulk

On Wed, Apr 29, 2020 at 07:21:13AM +0000, Satya Tangirala wrote:
> Whenever a device supports blk-integrity, the kernel will now always
> pretend that the device doesn't support inline encryption (essentially
> by setting the keyslot manager in the request queue to NULL).

"the kernel will now always" => "make the kernel", so that it's clear that this
patch is doing this.  I.e. it's not describing the state prior to the patch.

> diff --git a/block/blk-integrity.c b/block/blk-integrity.c
> index ff1070edbb400..b45711fc37df4 100644
> --- a/block/blk-integrity.c
> +++ b/block/blk-integrity.c
> @@ -409,6 +409,13 @@ void blk_integrity_register(struct gendisk *disk, struct blk_integrity *template
>  	bi->tag_size = template->tag_size;
>  
>  	disk->queue->backing_dev_info->capabilities |= BDI_CAP_STABLE_WRITES;
> +
> +#ifdef CONFIG_BLK_INLINE_ENCRYPTION
> +	if (disk->queue->ksm) {
> +		pr_warn("blk-integrity: Integrity and hardware inline encryption are not supported together. Unregistering keyslot manager from request queue, to disable hardware inline encryption.\n");
> +		blk_ksm_unregister(disk->queue);
> +	}
> +#endif
>  }

Make this shorter by removing the mention of the keyslot manager?:

	pr_warn("blk-integrity: Integrity and hardware inline encryption are not supported together. Disabling hardware inline encryption.\n");


> diff --git a/block/keyslot-manager.c b/block/keyslot-manager.c
> index b584723b392ad..834f45fdd33e2 100644
> --- a/block/keyslot-manager.c
> +++ b/block/keyslot-manager.c
> @@ -25,6 +25,9 @@
>   * Upper layers will call blk_ksm_get_slot_for_key() to program a
>   * key into some slot in the inline encryption hardware.
>   */
> +
> +#define pr_fmt(fmt) "blk_crypto: " fmt

"blk-crypto", not "blk_crypto".

> +bool blk_ksm_register(struct blk_keyslot_manager *ksm, struct request_queue *q)
> +{
> +	if (blk_integrity_queue_supports_integrity(q)) {
> +		pr_warn("Integrity and hardware inline encryption are not supported together. Hardware inline encryption is being disabled.\n");

"Disabling hardware inline encryption" to match my suggestion for the other one?

- Eric