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]
Date:   Tue, 29 Jan 2019 20:31:49 +1100
From:   Michael Ellerman <mpe@...erman.id.au>
To:     Tyrel Datwyler <turtle.in.the.kernel@...il.com>,
        Michael Bringmann <mwb@...ux.vnet.ibm.com>,
        linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org
Cc:     Juliet Kim <minkim@...ibm.com>,
        Thomas Falcon <tlfalcon@...ux.vnet.ibm.com>,
        Tyrel Datwyler <tyreld@...ux.vnet.ibm.com>
Subject: Re: [RFC 1/6] powerpc:/drc Define interface to acquire arch-specific drc info

Tyrel Datwyler <turtle.in.the.kernel@...il.com> writes:
> On 12/14/2018 12:50 PM, Michael Bringmann wrote:
>> Define interface to acquire arch-specific drc info to match against
>> hotpluggable devices.  The current implementation exposes several
>> pseries-specific dynamic memory properties in generic kernel code.
>> This patch set provides an interface to pull that code out of the
>> generic kernel.
>> 
>> Signed-off-by: Michael Bringmann <mwb@...ux.vnet.ibm.com>
>> ---
>>  include/linux/topology.h |    9 +++++++++
>>  1 file changed, 9 insertions(+)
>> 
>> diff --git a/include/linux/topology.h b/include/linux/topology.h
>> index cb0775e..df97f5f 100644
>> --- a/include/linux/topology.h
>> +++ b/include/linux/topology.h
>> @@ -44,6 +44,15 @@
>
> As far as I know pseries is the only platform that uses DR connectors, and I
> highly doubt that any other powerpc platform or arch ever will. So, I'm not sure
> that this is really generic enough to belong in topology.h.

Right. This does not belong in include/linux.

> If anything I would
> suggest putting this in an include in arch/powerpc/include/ named something like
> drcinfo.h or pseries-drc.h. That will make it visible to modules like rpaphp
> that want/need to use this functionality.

Yeah that would make more sense.

Using "arch" in the name is wrong, it's pseries specific so
pseries_find_drc_match() would be more appropriate.

>> +int arch_find_drc_match(struct device_node *dn,
>> +			bool (*usercb)(struct device_node *dn,
>> +				u32 drc_index, char *drc_name,
>> +				char *drc_type, u32 drc_power_domain,
>> +				void *data),
>> +			char *opt_drc_type, char *opt_drc_name,
>> +			bool match_drc_index, bool ck_php_type,
>> +			void *data);

This function signature is kind of insane.

You end with calls like:

+	return arch_find_drc_match(dn, rpaphp_add_slot_cb,
+			NULL, NULL, false, true, NULL);

Which is impossible to parse.

I feel like maybe this isn't the right level of abstraction.

cheers

Powered by blists - more mailing lists