Re: [PATCH v2] mgag200 fix memmapsl configuration in GCTL6 register

From: Thomas Zimmermann <tzimmermann@suse.de>
To: Jocelyn Falempe <jfalempe@redhat.com>, dri-devel@lists.freedesktop.org
Cc: michel@daenzer.net, lyude@redhat.com, javierm@redhat.com,
	stable@vger.kernel.org
Subject: Re: [PATCH v2] mgag200 fix memmapsl configuration in GCTL6 register
Date: Wed, 19 Jan 2022 13:21:21 +0100	[thread overview]
Message-ID: <967fd413-1b55-7b94-a164-70f2942772f6@suse.de> (raw)
In-Reply-To: <20220119102905.1194787-1-jfalempe@redhat.com>

[-- Attachment #1.1: Type: text/plain, Size: 3968 bytes --]

Hi,

Am 19.01.22 um 11:29 schrieb Jocelyn Falempe:
> On some servers with MGA G200_SE_A (rev 42), booting with Legacy BIOS,
> the hardware hangs when using kdump and kexec into the kdump kernel.
> This happens when the uncompress code tries to write "Decompressing Linux"
> to the VGA Console.
> 
> It can be reproduced by writing to the VGA console (0xB8000) after
> booting to graphic mode, it generates the following error:
> 
> kernel:NMI: PCI system error (SERR) for reason a0 on CPU 0.
> kernel:Dazed and confused, but trying to continue
> 
> The root cause is the configuration of the MGA GCTL6 register
> 
> According to the GCTL6 register documentation:
> 
> bit 0 is gcgrmode:
>      0: Enables alpha mode, and the character generator addressing system is
>       activated.
>      1: Enables graphics mode, and the character addressing system is not
>       used.
> 
> bit 1 is chainodd even:
>      0: The A0 signal of the memory address bus is used during system memory
>       addressing.
>      1: Allows A0 to be replaced by either the A16 signal of the system
>       address (ifmemmapsl is ‘00’), or by the hpgoddev (MISC<5>, odd/even
>       page select) field, described on page 3-294).
> 
> bit 3-2 are memmapsl:
>      Memory map select bits 1 and 0. VGA.
>      These bits select where the video memory is mapped, as shown below:
>          00 => A0000h - BFFFFh
>          01 => A0000h - AFFFFh
>          10 => B0000h - B7FFFh
>          11 => B8000h - BFFFFh
> 
> bit 7-4 are reserved.
> 
> Current code set it to 0x05 => memmapsl to b01 => 0xa0000 (graphic mode)
> But on x86, the VGA console is at 0xb8000 (text mode)
> In arch/x86/boot/compressed/misc.c debug strings are written to 0xb8000
> As the driver doesn't use this mapping at 0xa0000, it is safe to set it to
> 0xb8000 instead, to avoid kernel hang on G200_SE_A rev42, with kexec/kdump.
> 
> Thus changing the value 0x05 to 0x0d
> 
> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> Acked-by: Lyude Paul <lyude@redhat.com>
> Cc: stable@vger.kernel.org
> ---
> 
> v2: Add clear statement that it's not the right configuration, but it
>      prevents an annoying bug with kexec/kdump.
> 
>   drivers/gpu/drm/mgag200/mgag200_mode.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index b983541a4c53..cd9ba13ad5fc 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -529,7 +529,10 @@ static void mgag200_set_format_regs(struct mga_device *mdev,
>   	WREG_GFX(3, 0x00);
>   	WREG_GFX(4, 0x00);
>   	WREG_GFX(5, 0x40);
> -	WREG_GFX(6, 0x05);
> +	/* GCTL6 should be 0x05, but we configure memmapsl to 0xb8000 (text mode),
> +	 * so that it doesn't hang when running kexec/kdump on G200_SE rev42.
> +	 */
> +	WREG_GFX(6, 0x0d);

Appears to be working on my test machine.

But please rune scripts/checkpatch.pl on the patch before sending it. I 
get several errors

WARNING: Possible unwrapped commit description (prefer a maximum 75 
chars per line)

#98:

     0: Enables alpha mode, and the character generator addressing system is

ERROR: trailing whitespace

#149: FILE: drivers/gpu/drm/mgag200/mgag200_mode.c:532:

+^I/* GCTL6 should be 0x05, but we configure memmapsl to 0xb8000 (text 
mode),^M$

ERROR: trailing whitespace

#150: FILE: drivers/gpu/drm/mgag200/mgag200_mode.c:533:

+^I * so that it doesn't hang when running kexec/kdump on G200_SE rev42.^M$

Best regards
Thomas

>   	WREG_GFX(7, 0x0f);
>   	WREG_GFX(8, 0x0f);
>   

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]