[PATCH] drm/ttm: Print the memory decryption status just once

[PATCH] drm/ttm: Print the memory decryption status just once

[Zack Rusin]

Stop printing the TT memory decryption status info each time tt is created
and instead print it just once.

Reduces the spam in the system logs when running guests with SEV enabled.

Signed-off-by: Zack Rusin <zack.rusin@broadcom.com>
Fixes: 71ce046327cf ("drm/ttm: Make sure the mapped tt pages are decrypted when needed")
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
Cc: <stable@vger.kernel.org> # v5.14+
---
 drivers/gpu/drm/ttm/ttm_tt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 578a7c37f00b..d776e3f87064 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -92,7 +92,7 @@ int ttm_tt_create(struct ttm_buffer_object *bo, bool zero_alloc)
 	 */
 	if (bdev->pool.use_dma_alloc && cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
 		page_flags |= TTM_TT_FLAG_DECRYPTED;
-		drm_info(ddev, "TT memory decryption enabled.");
+		drm_info_once(ddev, "TT memory decryption enabled.");
 	}
 
 	bo->ttm = bdev->funcs->ttm_tt_create(bo, page_flags);
-- 
2.40.1

Re: [PATCH] drm/ttm: Print the memory decryption status just once

[Christian König]

Am 08.04.24 um 17:56 schrieb Zack Rusin:
> Stop printing the TT memory decryption status info each time tt is created
> and instead print it just once.
>
> Reduces the spam in the system logs when running guests with SEV enabled.

Do we then really need this in the first place?

Regards,
Christian.

>
> Signed-off-by: Zack Rusin <zack.rusin@broadcom.com>
> Fixes: 71ce046327cf ("drm/ttm: Make sure the mapped tt pages are decrypted when needed")
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-kernel@vger.kernel.org
> Cc: <stable@vger.kernel.org> # v5.14+
> ---
>   drivers/gpu/drm/ttm/ttm_tt.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
> index 578a7c37f00b..d776e3f87064 100644
> --- a/drivers/gpu/drm/ttm/ttm_tt.c
> +++ b/drivers/gpu/drm/ttm/ttm_tt.c
> @@ -92,7 +92,7 @@ int ttm_tt_create(struct ttm_buffer_object *bo, bool zero_alloc)
>   	 */
>   	if (bdev->pool.use_dma_alloc && cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
>   		page_flags |= TTM_TT_FLAG_DECRYPTED;
> -		drm_info(ddev, "TT memory decryption enabled.");
> +		drm_info_once(ddev, "TT memory decryption enabled.");
>   	}
>   
>   	bo->ttm = bdev->funcs->ttm_tt_create(bo, page_flags);

Re: [PATCH] drm/ttm: Print the memory decryption status just once

[Zack Rusin]

Sorry, apologies to everyone. By accident I replied off the list.
Redoing it now on the list. More below.

On Mon, Apr 8, 2024 at 12:10 PM Christian König
<christian.koenig@amd.com> wrote:
>
> Am 08.04.24 um 18:04 schrieb Zack Rusin:
> > On Mon, Apr 8, 2024 at 11:59 AM Christian König
> > <christian.koenig@amd.com> wrote:
> >> Am 08.04.24 um 17:56 schrieb Zack Rusin:
> >>> Stop printing the TT memory decryption status info each time tt is created
> >>> and instead print it just once.
> >>>
> >>> Reduces the spam in the system logs when running guests with SEV enabled.
> >> Do we then really need this in the first place?
> > Thomas asked for it just to have an indication when those paths are
> > being used because they could potentially break things pretty bad. I
> > think it is useful knowing that those paths are hit (but only once).
> > It makes it pretty easy for me to tell whether bug reports with people
> > who report black screen can be answered with "the kernel needs to be
> > upgraded" ;)
>
> Sounds reasonable, but my expectation was rather that we would print
> something on the device level.
>
> If that's not feasible for whatever reason than printing it once works
> as well of course.

TBH, I think it's pretty convenient to have the drm_info in the TT
just to make sure that when drivers request use_dma_alloc on SEV
systems TT turns decryption on correctly, i.e. it's a nice sanity
check when reading the logs. But if you'd prefer it in the driver I
can move this logic there as well.

z

Re: [PATCH] drm/ttm: Print the memory decryption status just once

[Zack Rusin]

Ping on this one. If we don't want the "_once" then I can quickly
prepare a patch that just removes the logging altogether, because
while useful it's polluting up the kernel logs too much right now so
getting a fix in for 6.9 for this would be great.

z

On Mon, Apr 8, 2024 at 1:46 PM Zack Rusin <zack.rusin@broadcom.com> wrote:
>
> Sorry, apologies to everyone. By accident I replied off the list.
> Redoing it now on the list. More below.
>
> On Mon, Apr 8, 2024 at 12:10 PM Christian König
> <christian.koenig@amd.com> wrote:
> >
> > Am 08.04.24 um 18:04 schrieb Zack Rusin:
> > > On Mon, Apr 8, 2024 at 11:59 AM Christian König
> > > <christian.koenig@amd.com> wrote:
> > >> Am 08.04.24 um 17:56 schrieb Zack Rusin:
> > >>> Stop printing the TT memory decryption status info each time tt is created
> > >>> and instead print it just once.
> > >>>
> > >>> Reduces the spam in the system logs when running guests with SEV enabled.
> > >> Do we then really need this in the first place?
> > > Thomas asked for it just to have an indication when those paths are
> > > being used because they could potentially break things pretty bad. I
> > > think it is useful knowing that those paths are hit (but only once).
> > > It makes it pretty easy for me to tell whether bug reports with people
> > > who report black screen can be answered with "the kernel needs to be
> > > upgraded" ;)
> >
> > Sounds reasonable, but my expectation was rather that we would print
> > something on the device level.
> >
> > If that's not feasible for whatever reason than printing it once works
> > as well of course.
>
> TBH, I think it's pretty convenient to have the drm_info in the TT
> just to make sure that when drivers request use_dma_alloc on SEV
> systems TT turns decryption on correctly, i.e. it's a nice sanity
> check when reading the logs. But if you'd prefer it in the driver I
> can move this logic there as well.
>
> z

Re: [PATCH] drm/ttm: Print the memory decryption status just once

[Christian König]

Sorry haven't seen your reply. Feel free to add my rb to the patch.

Regards,
Christian.

Am 18.04.24 um 13:38 schrieb Zack Rusin:
> Ping on this one. If we don't want the "_once" then I can quickly
> prepare a patch that just removes the logging altogether, because
> while useful it's polluting up the kernel logs too much right now so
> getting a fix in for 6.9 for this would be great.
>
> z
>
> On Mon, Apr 8, 2024 at 1:46 PM Zack Rusin <zack.rusin@broadcom.com> wrote:
>> Sorry, apologies to everyone. By accident I replied off the list.
>> Redoing it now on the list. More below.
>>
>> On Mon, Apr 8, 2024 at 12:10 PM Christian König
>> <christian.koenig@amd.com> wrote:
>>> Am 08.04.24 um 18:04 schrieb Zack Rusin:
>>>> On Mon, Apr 8, 2024 at 11:59 AM Christian König
>>>> <christian.koenig@amd.com> wrote:
>>>>> Am 08.04.24 um 17:56 schrieb Zack Rusin:
>>>>>> Stop printing the TT memory decryption status info each time tt is created
>>>>>> and instead print it just once.
>>>>>>
>>>>>> Reduces the spam in the system logs when running guests with SEV enabled.
>>>>> Do we then really need this in the first place?
>>>> Thomas asked for it just to have an indication when those paths are
>>>> being used because they could potentially break things pretty bad. I
>>>> think it is useful knowing that those paths are hit (but only once).
>>>> It makes it pretty easy for me to tell whether bug reports with people
>>>> who report black screen can be answered with "the kernel needs to be
>>>> upgraded" ;)
>>> Sounds reasonable, but my expectation was rather that we would print
>>> something on the device level.
>>>
>>> If that's not feasible for whatever reason than printing it once works
>>> as well of course.
>> TBH, I think it's pretty convenient to have the drm_info in the TT
>> just to make sure that when drivers request use_dma_alloc on SEV
>> systems TT turns decryption on correctly, i.e. it's a nice sanity
>> check when reading the logs. But if you'd prefer it in the driver I
>> can move this logic there as well.
>>
>> z