linux-kernel - Re: [PATCH] drm/ast: Atomic CR/SR reg R/W

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4a7ce84d-faef-4fb2-d36d-5cff2fba8ecf@suse.de>
Date:   Fri, 3 Dec 2021 09:46:43 +0100
From:   Thomas Zimmermann <tzimmermann@...e.de>
To:     Kuo-Hsiang Chou <kuohsiang_chou@...eedtech.com>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Cc:     Ryan Chen <ryan_chen@...eedtech.com>,
        "airlied@...ux.ie" <airlied@...ux.ie>,
        清水修(o-shimizu)-台灣NEC 
        <o-shimizu@....com.tw>, Jenmin Yuan <jenmin_yuan@...eedtech.com>,
        "airlied@...hat.com" <airlied@...hat.com>,
        Arc Sung <arc_sung@...eedtech.com>,
        Luke Chen <luke_chen@...eedtech.com>
Subject: Re: [PATCH] drm/ast: Atomic CR/SR reg R/W

Hi

Am 03.12.21 um 02:23 schrieb Kuo-Hsiang Chou:
> 
> 
> -----Original Message-----
> From: Kuo-Hsiang Chou
> Sent: Thursday, September 30, 2021 3:19 PM
> To: Thomas Zimmermann <tzimmermann@...e.de>; dri-devel@...ts.freedesktop.org; linux-kernel@...r.kernel.org
> Subject: RE: [PATCH] drm/ast: Atomic CR/SR reg R/W
> 
> Hi
> 
> -----Original Message-----
> From: Thomas Zimmermann [mailto:tzimmermann@...e.de]
> Sent: Monday, September 20, 2021 4:17 PM
> To: Kuo-Hsiang Chou <kuohsiang_chou@...eedtech.com>; dri-devel@...ts.freedesktop.org; linux-kernel@...r.kernel.org
> Subject: Re: [PATCH] drm/ast: Atomic CR/SR reg R/W
> 
> Hi
> 
> Am 17.09.21 um 09:22 schrieb KuoHsiang Chou:
>> 1. Avoid IO-index racing
>> 2. IO-index racing happened on resolustion switching
>>      and mouse moving at the same time
>> 3. System hung while IO-index racing occurred.
> 
> I'd say that there's something else going one here. Mode setting and cursor movement should be protected against each other by DRM locking.
> Changing these low-level functions would not solve the issues. I'll try to reproduce the problem ASAP.
> 
> Hi Thomas,
> 
> Sorry to interrupt you again!
> May I understand the review's situation? Thanks!

Look, you really have to work with us during the review process. Don't 
just expect us to tell you what to do.

I'm not going to merge this patch. As I said, I don't think it fixes the 
problem. Mouse movement and resolution switching should not interfere 
with each other. The DRM framework should guarantee that.

I cannot reproduce the issue, but there's most likely something else 
happening here. How can the system switch resolution and change the 
mouse at the same time?

Best regards
Thomas

> 
> Hi Tomas,
> Good day!
> May I understand the review status, or is there anything I can do to improve it? Thanks!
> 
> Best Regards,
> 	Kuo-Hsiang Chou
> 
> Best Regards,
>   	Kuo-Hsiang Chou
> 
> Best regards
> Thomas
> 
>>
>> Signed-off-by: KuoHsiang Chou <kuohsiang_chou@...eedtech.com>
>> ---
>>    drivers/gpu/drm/ast/ast_main.c | 48 +++++++++++++++++++++++++---------
>>    1 file changed, 36 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ast/ast_main.c
>> b/drivers/gpu/drm/ast/ast_main.c index 79a361867..1d8fa70c5 100644
>> --- a/drivers/gpu/drm/ast/ast_main.c
>> +++ b/drivers/gpu/drm/ast/ast_main.c
>> @@ -41,28 +41,52 @@ void ast_set_index_reg_mask(struct ast_private *ast,
>>    			    uint32_t base, uint8_t index,
>>    			    uint8_t mask, uint8_t val)
>>    {
>> -	u8 tmp;
>> -	ast_io_write8(ast, base, index);
>> -	tmp = (ast_io_read8(ast, base + 1) & mask) | val;
>> -	ast_set_index_reg(ast, base, index, tmp);
>> +	uint16_t volatile usData;
>> +	uint8_t  volatile jData;
>> +
>> +	do {
>> +		ast_io_write8(ast, base, index);
>> +		usData = ast_io_read16(ast, base);
>> +	} while ((uint8_t)(usData) != index);
>> +
>> +	jData  = (uint8_t)(usData >> 8);
>> +	jData &= mask;
>> +	jData |= val;
>> +	usData = ((uint16_t) jData << 8) | (uint16_t) index;
>> +	ast_io_write16(ast, base, usData);
>>    }
>>
>>    uint8_t ast_get_index_reg(struct ast_private *ast,
>>    			  uint32_t base, uint8_t index)
>>    {
>> -	uint8_t ret;
>> -	ast_io_write8(ast, base, index);
>> -	ret = ast_io_read8(ast, base + 1);
>> -	return ret;
>> +	uint16_t volatile usData;
>> +	uint8_t  volatile jData;
>> +
>> +	do {
>> +		ast_io_write8(ast, base, index);
>> +		usData = ast_io_read16(ast, base);
>> +	} while ((uint8_t)(usData) != index);
>> +
>> +	jData  = (uint8_t)(usData >> 8);
>> +
>> +	return jData;
>>    }
>>
>>    uint8_t ast_get_index_reg_mask(struct ast_private *ast,
>>    			       uint32_t base, uint8_t index, uint8_t mask)
>>    {
>> -	uint8_t ret;
>> -	ast_io_write8(ast, base, index);
>> -	ret = ast_io_read8(ast, base + 1) & mask;
>> -	return ret;
>> +	uint16_t volatile usData;
>> +	uint8_t  volatile jData;
>> +
>> +	do {
>> +		ast_io_write8(ast, base, index);
>> +		usData = ast_io_read16(ast, base);
>> +	} while ((uint8_t)(usData) != index);
>> +
>> +	jData  = (uint8_t)(usData >> 8);
>> +	jData &= mask;
>> +
>> +	return jData;
>>    }
>>
>>    static void ast_detect_config_mode(struct drm_device *dev, u32
>> *scu_rev)
>> --
>> 2.18.4
>>
> 
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
> 

-- 
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

Download attachment "OpenPGP_signature" of type "application/pgp-signature" (841 bytes)