linux-kernel - RE: [PATCH] i3c: master: fix for SETDASA and DAA process

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] [day] [month] [year] [list]

Message-ID: <DM5PR07MB3196ACBCC6128E6C87CF0B7DC1B60@DM5PR07MB3196.namprd07.prod.outlook.com>
Date:   Wed, 20 May 2020 09:20:28 +0000
From:   Parshuram Raju Thombare <pthombar@...ence.com>
To:     Boris Brezillon <boris.brezillon@...labora.com>
CC:     "bbrezillon@...nel.org" <bbrezillon@...nel.org>,
        "vitor.soares@...opsys.com" <vitor.soares@...opsys.com>,
        Milind Parab <mparab@...ence.com>,
        "praneeth@...com" <praneeth@...com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Przemyslaw Gaj <pgaj@...ence.com>,
        "linux-i3c@...ts.infradead.org" <linux-i3c@...ts.infradead.org>
Subject: RE: [PATCH] i3c: master: fix for SETDASA and DAA process

>> This patch fix following issues.
>> 1. Controller slots blocked for devices with static_addr
>>    but no init_dyn_addr may limit the number of I3C devices
>>    on the bus which gets dynamic address in DAA. So
>>    instead of attaching all the devices with static_addr,
>>    now we only attach the devices which successfully
>>    complete SETDASA. Remaining devices are handled in DAA.
>> 2. Since we alreay handled devices with init_dyn_addr, removed
>>    it's handling from i3c_master_add_i3c_dev_locked().
>>    Now only case handled is devices already with dyn_addr
>>    participated in DAA, and again got new dyn_addr with an
>>    extra slot in the master controller.
>
>I don't get that one.

I mean retry  to assign requested init_dyn_addr in i3c_master_add_i3c_dev_locked().
Since we handle devices with init_dyn_addr in i3c_master_pre_assign_dyn_addr,
we should have assigned dynamic address to all devices with both static_addr and
init_dyn_addr. Unless SETDASA failed or device only have init_dyn_addr but no static_addr, 
in those cases dyn_addr is allocated in DAA.

>I think we should fix re-attach instead, which is what we discussed
>with Przemek if I remember correctly.

Sorry, I was not aware of that. But, yes I agree to fix driver re-attach instead of removing 
re attach here. But we should keep in mind about potential failure here.
Currently reattach only update dyn_addr, but IMO it should update other fields as well.
Also I see re attach have a unused argument, which I suppose we can get rid of.
For now I will keep reattach here and post reattach fix in separate patch.

>Can you please split the patch accordingly (one fix per commit)
I think only extra changes are releasing the old address in reattach API
and removal of reattach call in i3c_master_add_i3c_dev_locked()
after a scan for duplicate I3C device. 
I will remove those changes and repost rest.

Regards,
Parshuram Thombare