Re: openbmc Digest, Vol 32, Issue 32

Re: openbmc Digest, Vol 32, Issue 32

[Wang, Haiyue]

Hi Andrew,

Facebook uses the 'devmem' utility and python script to implement the 
soc register

read and write in a simple way. And the yocto layer is also very clear 
like different

chip vendor and common parts.


Kernel device tree upstream is also very slow, we needs two parts 
(device tree + user app)

to finish the soc setting. But Facebook just uses user app, this 
development is fast, and

simple is the best.


Well, just for sharing what I found, I may misunderstood the BIG idea 
after this kernel

patch design. :-)


soc_gpio_table = {

	'A10': [

	Function('SDA3', BitsEqual(0x90, [16], 0x1)),

	Function('GPIOQ1', None)

	],

	'A11': [

	Function('SCL3', BitsEqual(0x90, [16], 0x1)),

	Function('GPIOQ0', None)

	],

https://github.com/facebook/openbmc/blob/helium/common/recipes-utils/openbmc-gpio/files/openbmc-gpio-1/phymemory.py

https://github.com/facebook/openbmc/blob/helium/meta-aspeed/recipes-utils/ast-mdio/files/ast-mdio.py

https://github.com/facebook/openbmc/blob/helium/meta-aspeed/recipes-utils/openbmc-gpio/files/openbmc-gpio-1/ast2500_gpio_table.py


BR,

Haiyue


On 2018-04-12 13:32, openbmc-request@lists.ozlabs.org wrote:
> Message: 1
> Date: Thu, 12 Apr 2018 13:21:43 +0930
> From: Andrew Jeffery<andrew@aj.id.au>
> To:openbmc@lists.ozlabs.org
> Cc: Andrew Jeffery<andrew@aj.id.au>,joel@jms.id.au,jk@ozlabs.org,
> 	benh@kernel.crashing.org
> Subject: [RFC PATCH linux dev-4.13 1/3] soc: aspeed: Miscellaneous
> 	control interfaces
> Message-ID:<20180412035145.21488-2-andrew@aj.id.au>
>
> The ASPEED BMC SoCs have many knobs and switches that are sometimes
> design-specific and often defy any approach to unify them under an
> existing subsystem.
>
> Add a driver to translate a devicetree table into sysfs entries to
> expose bits and fields for manipulation from userspace. This encompasses
> concepts from scratch registers to boolean conditions to enable or
> disable host interface features.
>
> Signed-off-by: Andrew Jeffery<andrew@aj.id.au>
> ---
>   drivers/soc/Kconfig                  |   1 +
>   drivers/soc/Makefile                 |   1 +
>   drivers/soc/aspeed/Kconfig           |  11 +++
>   drivers/soc/aspeed/Makefile          |   1 +
>   drivers/soc/aspeed/aspeed-bmc-misc.c | 187 +++++++++++++++++++++++++++++++++++
>   5 files changed, 201 insertions(+)
>   create mode 100644 drivers/soc/aspeed/Kconfig
>   create mode 100644 drivers/soc/aspeed/Makefile
>   create mode 100644 drivers/soc/aspeed/aspeed-bmc-misc.c
>
> diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
> index 07fc0ac51c52..776fd5978f70 100644
> --- a/drivers/soc/Kconfig
> +++ b/drivers/soc/Kconfig
> @@ -1,6 +1,7 @@
>   menu "SOC (System On Chip) specific Drivers"
>   
>   source "drivers/soc/actions/Kconfig"
> +source "drivers/soc/aspeed/Kconfig"
>   source "drivers/soc/atmel/Kconfig"
>   source "drivers/soc/bcm/Kconfig"
>   source "drivers/soc/fsl/Kconfig"
> diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
> index 9241125416ba..94a5b97c4466 100644
> --- a/drivers/soc/Makefile
> +++ b/drivers/soc/Makefile
> @@ -3,6 +3,7 @@
>   #
>   
>   obj-$(CONFIG_ARCH_ACTIONS)	+= actions/
> +obj-$(CONFIG_ARCH_ASPEED)       += aspeed/
>   obj-$(CONFIG_ARCH_AT91)		+= atmel/
>   obj-y				+= bcm/
>   obj-$(CONFIG_ARCH_DOVE)		+= dove/
> diff --git a/drivers/soc/aspeed/Kconfig b/drivers/soc/aspeed/Kconfig
> new file mode 100644
> index 000000000000..704a4efe180f
> --- /dev/null
> +++ b/drivers/soc/aspeed/Kconfig
> @@ -0,0 +1,11 @@
> +menu "ASPEED SoC drivers"
> +
> +config ASPEED_BMC_MISC
> +	bool "Miscellaneous ASPEED BMC interfaces"
> +	depends on ARCH_ASPEED || COMPILE_TEST
> +	default ARCH_ASPEED
> +	help
> +	  Say yes to expose VGA and LPC scratch registers, and other
> +	  miscellaneous control interfaces specific to the ASPEED BMC SoCs
> +
> +endmenu

Re: openbmc Digest, Vol 32, Issue 32

[Benjamin Herrenschmidt]

On Thu, 2018-04-12 at 15:55 +0800, Wang, Haiyue wrote:
> Hi Andrew,
> 
> Facebook uses the 'devmem' utility and python script to implement the 
> soc register
> 
> read and write in a simple way. And the yocto layer is also very clear 
> like different

Using /dev/mem is really bad and very much discouraged. In fact for
security reasons, /dev/mem should be disabled in the kernel in
production. We need to do something better, hence the proposal.

> chip vendor and common parts.
> 
> 
> Kernel device tree upstream is also very slow, we needs two parts 
> (device tree + user app)

What do you mean by "very slow" ? I don't understand your statement.

> to finish the soc setting. But Facebook just uses user app, this 
> development is fast, and
> 
> simple is the best.

Sorry I don't really get what you mean. Please elaborate.

> Well, just for sharing what I found, I may misunderstood the BIG idea 
> after this kernel

The basic idea is to have a mechanism by which the SoC code in the
kernel can expose to userspace via sysfs a *small* selected set of
tunables that userspace might have to tweak for various platform
specific reasons.

To qualify, these things have to strictly be things that don't fit
within any existing Linux ABI and subsystem, and typically that affect
the *host* and not the BMC itself (or to a very limited extent).

Some examples are the tunables to control whether the host can access
the AHB bus via the backdoor on LPC or PCIe (and if yes which regions
are enabled).

> patch design. :-)
> 
> 
> soc_gpio_table = {
> 
> 	'A10': [
> 
> 	Function('SDA3', BitsEqual(0x90, [16], 0x1)),
> 
> 	Function('GPIOQ1', None)
> 
> 	],
> 
> 	'A11': [
> 
> 	Function('SCL3', BitsEqual(0x90, [16], 0x1)),
> 
> 	Function('GPIOQ0', None)
> 
> 	],
> 
> https://github.com/facebook/openbmc/blob/helium/common/recipes-utils/openbmc-gpio/files/openbmc-gpio-1/phymemory.py
> 
> https://github.com/facebook/openbmc/blob/helium/meta-aspeed/recipes-utils/ast-mdio/files/ast-mdio.py
> 
> https://github.com/facebook/openbmc/blob/helium/meta-aspeed/recipes-utils/openbmc-gpio/files/openbmc-gpio-1/ast2500_gpio_table.py

I don't undertsand what those links are supposed to be. Please
elaborate on what you mean rather than posting random links.

Cheers,
Ben.


> 
> BR,
> 
> Haiyue
> 
> 
> On 2018-04-12 13:32, openbmc-request@lists.ozlabs.org wrote:
> > Message: 1
> > Date: Thu, 12 Apr 2018 13:21:43 +0930
> > From: Andrew Jeffery<andrew@aj.id.au>
> > To:openbmc@lists.ozlabs.org
> > Cc: Andrew Jeffery<andrew@aj.id.au>,joel@jms.id.au,jk@ozlabs.org,
> > 	benh@kernel.crashing.org
> > Subject: [RFC PATCH linux dev-4.13 1/3] soc: aspeed: Miscellaneous
> > 	control interfaces
> > Message-ID:<20180412035145.21488-2-andrew@aj.id.au>
> > 
> > The ASPEED BMC SoCs have many knobs and switches that are sometimes
> > design-specific and often defy any approach to unify them under an
> > existing subsystem.
> > 
> > Add a driver to translate a devicetree table into sysfs entries to
> > expose bits and fields for manipulation from userspace. This encompasses
> > concepts from scratch registers to boolean conditions to enable or
> > disable host interface features.
> > 
> > Signed-off-by: Andrew Jeffery<andrew@aj.id.au>
> > ---
> >   drivers/soc/Kconfig                  |   1 +
> >   drivers/soc/Makefile                 |   1 +
> >   drivers/soc/aspeed/Kconfig           |  11 +++
> >   drivers/soc/aspeed/Makefile          |   1 +
> >   drivers/soc/aspeed/aspeed-bmc-misc.c | 187 +++++++++++++++++++++++++++++++++++
> >   5 files changed, 201 insertions(+)
> >   create mode 100644 drivers/soc/aspeed/Kconfig
> >   create mode 100644 drivers/soc/aspeed/Makefile
> >   create mode 100644 drivers/soc/aspeed/aspeed-bmc-misc.c
> > 
> > diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
> > index 07fc0ac51c52..776fd5978f70 100644
> > --- a/drivers/soc/Kconfig
> > +++ b/drivers/soc/Kconfig
> > @@ -1,6 +1,7 @@
> >   menu "SOC (System On Chip) specific Drivers"
> >   
> >   source "drivers/soc/actions/Kconfig"
> > +source "drivers/soc/aspeed/Kconfig"
> >   source "drivers/soc/atmel/Kconfig"
> >   source "drivers/soc/bcm/Kconfig"
> >   source "drivers/soc/fsl/Kconfig"
> > diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
> > index 9241125416ba..94a5b97c4466 100644
> > --- a/drivers/soc/Makefile
> > +++ b/drivers/soc/Makefile
> > @@ -3,6 +3,7 @@
> >   #
> >   
> >   obj-$(CONFIG_ARCH_ACTIONS)	+= actions/
> > +obj-$(CONFIG_ARCH_ASPEED)       += aspeed/
> >   obj-$(CONFIG_ARCH_AT91)		+= atmel/
> >   obj-y				+= bcm/
> >   obj-$(CONFIG_ARCH_DOVE)		+= dove/
> > diff --git a/drivers/soc/aspeed/Kconfig b/drivers/soc/aspeed/Kconfig
> > new file mode 100644
> > index 000000000000..704a4efe180f
> > --- /dev/null
> > +++ b/drivers/soc/aspeed/Kconfig
> > @@ -0,0 +1,11 @@
> > +menu "ASPEED SoC drivers"
> > +
> > +config ASPEED_BMC_MISC
> > +	bool "Miscellaneous ASPEED BMC interfaces"
> > +	depends on ARCH_ASPEED || COMPILE_TEST
> > +	default ARCH_ASPEED
> > +	help
> > +	  Say yes to expose VGA and LPC scratch registers, and other
> > +	  miscellaneous control interfaces specific to the ASPEED BMC SoCs
> > +
> > +endmenu

Re: openbmc Digest, Vol 32, Issue 32

[Wang, Haiyue]

On 2018-04-12 16:20, Benjamin Herrenschmidt wrote:
> On Thu, 2018-04-12 at 15:55 +0800, Wang, Haiyue wrote:
>> Hi Andrew,
>>
>> Facebook uses the 'devmem' utility and python script to implement the
>> soc register
>>
>> read and write in a simple way. And the yocto layer is also very clear
>> like different
> Using /dev/mem is really bad and very much discouraged. In fact for
> security reasons, /dev/mem should be disabled in the kernel in
> production. We need to do something better, hence the proposal.
Oh, I missed the security concern. Then removing 'devmem' makes sense. :-)
>> chip vendor and common parts.
>>
>>
>> Kernel device tree upstream is also very slow, we needs two parts
>> (device tree + user app)
> What do you mean by "very slow" ? I don't understand your statement.
I means the device tree merge time, putting things in kernel need long time
to be passed code review. And people may want to export different range of
registers, then they need to change the device tree, the development cycle
may be long.
>> to finish the soc setting. But Facebook just uses user app, this
>> development is fast, and
>>
>> simple is the best.
> Sorry I don't really get what you mean. Please elaborate.
>
>> Well, just for sharing what I found, I may misunderstood the BIG idea
>> after this kernel
> The basic idea is to have a mechanism by which the SoC code in the
> kernel can expose to userspace via sysfs a *small* selected set of
> tunables that userspace might have to tweak for various platform
> specific reasons.
>
> To qualify, these things have to strictly be things that don't fit
> within any existing Linux ABI and subsystem, and typically that affect
> the *host* and not the BMC itself (or to a very limited extent).
>
> Some examples are the tunables to control whether the host can access
> the AHB bus via the backdoor on LPC or PCIe (and if yes which regions
> are enabled).
I see, I just think Facebook's method is more common and flexible, but 
of course,
it has the security reason you mentioned. :)
>> patch design. :-)
>>
>>
>> soc_gpio_table = {
>>
>> 	'A10': [
>>
>> 	Function('SDA3', BitsEqual(0x90, [16], 0x1)),
>>
>> 	Function('GPIOQ1', None)
>>
>> 	],
>>
>> 	'A11': [
>>
>> 	Function('SCL3', BitsEqual(0x90, [16], 0x1)),
>>
>> 	Function('GPIOQ0', None)
>>
>> 	],
>>
>> https://github.com/facebook/openbmc/blob/helium/common/recipes-utils/openbmc-gpio/files/openbmc-gpio-1/phymemory.py
>>
>> https://github.com/facebook/openbmc/blob/helium/meta-aspeed/recipes-utils/ast-mdio/files/ast-mdio.py
>>
>> https://github.com/facebook/openbmc/blob/helium/meta-aspeed/recipes-utils/openbmc-gpio/files/openbmc-gpio-1/ast2500_gpio_table.py
> I don't undertsand what those links are supposed to be. Please
> elaborate on what you mean rather than posting random links.
Just want to say the code layer is flexible for different products and 
different
BMC chip vendors. Sorry to confuse you so much, thanks for your reply. :)
> Cheers,
> Ben.
>
>
>> BR,
>>
>> Haiyue
>>
>>
>> On 2018-04-12 13:32, openbmc-request@lists.ozlabs.org wrote:
>>> Message: 1
>>> Date: Thu, 12 Apr 2018 13:21:43 +0930
>>> From: Andrew Jeffery<andrew@aj.id.au>
>>> To:openbmc@lists.ozlabs.org
>>> Cc: Andrew Jeffery<andrew@aj.id.au>,joel@jms.id.au,jk@ozlabs.org,
>>> 	benh@kernel.crashing.org
>>> Subject: [RFC PATCH linux dev-4.13 1/3] soc: aspeed: Miscellaneous
>>> 	control interfaces
>>> Message-ID:<20180412035145.21488-2-andrew@aj.id.au>
>>>
>>> The ASPEED BMC SoCs have many knobs and switches that are sometimes
>>> design-specific and often defy any approach to unify them under an
>>> existing subsystem.
>>>
>>> Add a driver to translate a devicetree table into sysfs entries to
>>> expose bits and fields for manipulation from userspace. This encompasses
>>> concepts from scratch registers to boolean conditions to enable or
>>> disable host interface features.
>>>
>>> Signed-off-by: Andrew Jeffery<andrew@aj.id.au>
>>> ---
>>>    drivers/soc/Kconfig                  |   1 +
>>>    drivers/soc/Makefile                 |   1 +
>>>    drivers/soc/aspeed/Kconfig           |  11 +++
>>>    drivers/soc/aspeed/Makefile          |   1 +
>>>    drivers/soc/aspeed/aspeed-bmc-misc.c | 187 +++++++++++++++++++++++++++++++++++
>>>    5 files changed, 201 insertions(+)
>>>    create mode 100644 drivers/soc/aspeed/Kconfig
>>>    create mode 100644 drivers/soc/aspeed/Makefile
>>>    create mode 100644 drivers/soc/aspeed/aspeed-bmc-misc.c
>>>
>>> diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
>>> index 07fc0ac51c52..776fd5978f70 100644
>>> --- a/drivers/soc/Kconfig
>>> +++ b/drivers/soc/Kconfig
>>> @@ -1,6 +1,7 @@
>>>    menu "SOC (System On Chip) specific Drivers"
>>>    
>>>    source "drivers/soc/actions/Kconfig"
>>> +source "drivers/soc/aspeed/Kconfig"
>>>    source "drivers/soc/atmel/Kconfig"
>>>    source "drivers/soc/bcm/Kconfig"
>>>    source "drivers/soc/fsl/Kconfig"
>>> diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
>>> index 9241125416ba..94a5b97c4466 100644
>>> --- a/drivers/soc/Makefile
>>> +++ b/drivers/soc/Makefile
>>> @@ -3,6 +3,7 @@
>>>    #
>>>    
>>>    obj-$(CONFIG_ARCH_ACTIONS)	+= actions/
>>> +obj-$(CONFIG_ARCH_ASPEED)       += aspeed/
>>>    obj-$(CONFIG_ARCH_AT91)		+= atmel/
>>>    obj-y				+= bcm/
>>>    obj-$(CONFIG_ARCH_DOVE)		+= dove/
>>> diff --git a/drivers/soc/aspeed/Kconfig b/drivers/soc/aspeed/Kconfig
>>> new file mode 100644
>>> index 000000000000..704a4efe180f
>>> --- /dev/null
>>> +++ b/drivers/soc/aspeed/Kconfig
>>> @@ -0,0 +1,11 @@
>>> +menu "ASPEED SoC drivers"
>>> +
>>> +config ASPEED_BMC_MISC
>>> +	bool "Miscellaneous ASPEED BMC interfaces"
>>> +	depends on ARCH_ASPEED || COMPILE_TEST
>>> +	default ARCH_ASPEED
>>> +	help
>>> +	  Say yes to expose VGA and LPC scratch registers, and other
>>> +	  miscellaneous control interfaces specific to the ASPEED BMC SoCs
>>> +
>>> +endmenu

Re: openbmc Digest, Vol 32, Issue 32

[Benjamin Herrenschmidt]

On Thu, 2018-04-12 at 16:42 +0800, Wang, Haiyue wrote:
> 
> I means the device tree merge time, putting things in kernel need long time
> to be passed code review. And people may want to export different range of
> registers, then they need to change the device tree, the development cycle
> may be long.

But that's not a big issue in practice though, you don't *need* all
your device-trees to be upstream and up to date especially if it's
stored separately from the kernel.

Keep in mind this should be strictly limited to a few registers that
need that sort of manipulation, maybe a handful or two, that's it.

Most things need proper kernel drivers.

> > > to finish the soc setting. But Facebook just uses user app, this
> > > development is fast, and
> > > 
> > > simple is the best.
> > 
> > Sorry I don't really get what you mean. Please elaborate.
> > 
> > > Well, just for sharing what I found, I may misunderstood the BIG idea
> > > after this kernel
> > 
> > The basic idea is to have a mechanism by which the SoC code in the
> > kernel can expose to userspace via sysfs a *small* selected set of
> > tunables that userspace might have to tweak for various platform
> > specific reasons.
> > 
> > To qualify, these things have to strictly be things that don't fit
> > within any existing Linux ABI and subsystem, and typically that affect
> > the *host* and not the BMC itself (or to a very limited extent).
> > 
> > Some examples are the tunables to control whether the host can access
> > the AHB bus via the backdoor on LPC or PCIe (and if yes which regions
> > are enabled).
> 
> I see, I just think Facebook's method is more common and flexible, but 
> of course,
> it has the security reason you mentioned. :)
> > > patch design. :-)
> > > 
> > > 
> > > soc_gpio_table = {
> > > 
> > > 	'A10': [
> > > 
> > > 	Function('SDA3', BitsEqual(0x90, [16], 0x1)),
> > > 
> > > 	Function('GPIOQ1', None)
> > > 
> > > 	],
> > > 
> > > 	'A11': [
> > > 
> > > 	Function('SCL3', BitsEqual(0x90, [16], 0x1)),
> > > 
> > > 	Function('GPIOQ0', None)
> > > 
> > > 	],
> > > 
> > > https://github.com/facebook/openbmc/blob/helium/common/recipes-utils/openbmc-gpio/files/openbmc-gpio-1/phymemory.py
> > > 
> > > https://github.com/facebook/openbmc/blob/helium/meta-aspeed/recipes-utils/ast-mdio/files/ast-mdio.py
> > > 
> > > https://github.com/facebook/openbmc/blob/helium/meta-aspeed/recipes-utils/openbmc-gpio/files/openbmc-gpio-1/ast2500_gpio_table.py
> > 
> > I don't undertsand what those links are supposed to be. Please
> > elaborate on what you mean rather than posting random links.
> 
> Just want to say the code layer is flexible for different products and 
> different
> BMC chip vendors. Sorry to confuse you so much, thanks for your reply. :)
> > Cheers,
> > Ben.
> > 
> > 
> > > BR,
> > > 
> > > Haiyue
> > > 
> > > 
> > > On 2018-04-12 13:32, openbmc-request@lists.ozlabs.org wrote:
> > > > Message: 1
> > > > Date: Thu, 12 Apr 2018 13:21:43 +0930
> > > > From: Andrew Jeffery<andrew@aj.id.au>
> > > > To:openbmc@lists.ozlabs.org
> > > > Cc: Andrew Jeffery<andrew@aj.id.au>,joel@jms.id.au,jk@ozlabs.org,
> > > > 	benh@kernel.crashing.org
> > > > Subject: [RFC PATCH linux dev-4.13 1/3] soc: aspeed: Miscellaneous
> > > > 	control interfaces
> > > > Message-ID:<20180412035145.21488-2-andrew@aj.id.au>
> > > > 
> > > > The ASPEED BMC SoCs have many knobs and switches that are sometimes
> > > > design-specific and often defy any approach to unify them under an
> > > > existing subsystem.
> > > > 
> > > > Add a driver to translate a devicetree table into sysfs entries to
> > > > expose bits and fields for manipulation from userspace. This encompasses
> > > > concepts from scratch registers to boolean conditions to enable or
> > > > disable host interface features.
> > > > 
> > > > Signed-off-by: Andrew Jeffery<andrew@aj.id.au>
> > > > ---
> > > >    drivers/soc/Kconfig                  |   1 +
> > > >    drivers/soc/Makefile                 |   1 +
> > > >    drivers/soc/aspeed/Kconfig           |  11 +++
> > > >    drivers/soc/aspeed/Makefile          |   1 +
> > > >    drivers/soc/aspeed/aspeed-bmc-misc.c | 187 +++++++++++++++++++++++++++++++++++
> > > >    5 files changed, 201 insertions(+)
> > > >    create mode 100644 drivers/soc/aspeed/Kconfig
> > > >    create mode 100644 drivers/soc/aspeed/Makefile
> > > >    create mode 100644 drivers/soc/aspeed/aspeed-bmc-misc.c
> > > > 
> > > > diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
> > > > index 07fc0ac51c52..776fd5978f70 100644
> > > > --- a/drivers/soc/Kconfig
> > > > +++ b/drivers/soc/Kconfig
> > > > @@ -1,6 +1,7 @@
> > > >    menu "SOC (System On Chip) specific Drivers"
> > > >    
> > > >    source "drivers/soc/actions/Kconfig"
> > > > +source "drivers/soc/aspeed/Kconfig"
> > > >    source "drivers/soc/atmel/Kconfig"
> > > >    source "drivers/soc/bcm/Kconfig"
> > > >    source "drivers/soc/fsl/Kconfig"
> > > > diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
> > > > index 9241125416ba..94a5b97c4466 100644
> > > > --- a/drivers/soc/Makefile
> > > > +++ b/drivers/soc/Makefile
> > > > @@ -3,6 +3,7 @@
> > > >    #
> > > >    
> > > >    obj-$(CONFIG_ARCH_ACTIONS)	+= actions/
> > > > +obj-$(CONFIG_ARCH_ASPEED)       += aspeed/
> > > >    obj-$(CONFIG_ARCH_AT91)		+= atmel/
> > > >    obj-y				+= bcm/
> > > >    obj-$(CONFIG_ARCH_DOVE)		+= dove/
> > > > diff --git a/drivers/soc/aspeed/Kconfig b/drivers/soc/aspeed/Kconfig
> > > > new file mode 100644
> > > > index 000000000000..704a4efe180f
> > > > --- /dev/null
> > > > +++ b/drivers/soc/aspeed/Kconfig
> > > > @@ -0,0 +1,11 @@
> > > > +menu "ASPEED SoC drivers"
> > > > +
> > > > +config ASPEED_BMC_MISC
> > > > +	bool "Miscellaneous ASPEED BMC interfaces"
> > > > +	depends on ARCH_ASPEED || COMPILE_TEST
> > > > +	default ARCH_ASPEED
> > > > +	help
> > > > +	  Say yes to expose VGA and LPC scratch registers, and other
> > > > +	  miscellaneous control interfaces specific to the ASPEED BMC SoCs
> > > > +
> > > > +endmenu

Re: openbmc Digest, Vol 32, Issue 32

[Wang, Haiyue]

On 2018-04-13 12:43, Benjamin Herrenschmidt wrote:
> On Thu, 2018-04-12 at 16:42 +0800, Wang, Haiyue wrote:
>> I means the device tree merge time, putting things in kernel need long time
>> to be passed code review. And people may want to export different range of
>> registers, then they need to change the device tree, the development cycle
>> may be long.
> But that's not a big issue in practice though, you don't*need*  all
> your device-trees to be upstream and up to date especially if it's
> stored separately from the kernel.
>
> Keep in mind this should be strictly limited to a few registers that
> need that sort of manipulation, maybe a handful or two, that's it.
>
> Most things need proper kernel drivers.
>
Understood, NOT exposing like /dev/mem makes sense for security.

-Haiyue

Re: openbmc Digest, Vol 32, Issue 32

[Benjamin Herrenschmidt]

On Fri, 2018-04-13 at 13:11 +0800, Wang, Haiyue wrote:
> 
> On 2018-04-13 12:43, Benjamin Herrenschmidt wrote:
> > On Thu, 2018-04-12 at 16:42 +0800, Wang, Haiyue wrote:
> > > I means the device tree merge time, putting things in kernel need long time
> > > to be passed code review. And people may want to export different range of
> > > registers, then they need to change the device tree, the development cycle
> > > may be long.
> > 
> > But that's not a big issue in practice though, you don't*need*  all
> > your device-trees to be upstream and up to date especially if it's
> > stored separately from the kernel.
> > 
> > Keep in mind this should be strictly limited to a few registers that
> > need that sort of manipulation, maybe a handful or two, that's it.
> > 
> > Most things need proper kernel drivers.
> > 
> 
> Understood, NOT exposing like /dev/mem makes sense for security

Not just security. It's also plain gross ;-)

It also means it's hard to locate which pieces of your userspace may
contain chip revision specific stuff.

If something changes, you get a nice error bcs the named file is absent
rather than whacking the wrong register with underterminate results.

The naming also makes the code more comprehensible & grep'able

You also have an atomicity problem is the same register contains
multiple things that several fields (kernel, userspace) try to modify
at the same time, etc etc etc...

It's never great to have userspace bang at the registers so directly.

etc etc etc..

A minimum of abastraction is much more preferable in these cases.

Cheers,
Ben.

Re: openbmc Digest, Vol 32, Issue 32

[Wang, Haiyue]

On 2018-04-13 13:41, Benjamin Herrenschmidt wrote:
> On Fri, 2018-04-13 at 13:11 +0800, Wang, Haiyue wrote:
>> On 2018-04-13 12:43, Benjamin Herrenschmidt wrote:
>>> On Thu, 2018-04-12 at 16:42 +0800, Wang, Haiyue wrote:
>>>> I means the device tree merge time, putting things in kernel need long time
>>>> to be passed code review. And people may want to export different range of
>>>> registers, then they need to change the device tree, the development cycle
>>>> may be long.
>>> But that's not a big issue in practice though, you don't*need*  all
>>> your device-trees to be upstream and up to date especially if it's
>>> stored separately from the kernel.
>>>
>>> Keep in mind this should be strictly limited to a few registers that
>>> need that sort of manipulation, maybe a handful or two, that's it.
>>>
>>> Most things need proper kernel drivers.
>>>
>> Understood, NOT exposing like /dev/mem makes sense for security
> Not just security. It's also plain gross ;-)
>
> It also means it's hard to locate which pieces of your userspace may
> contain chip revision specific stuff.
>
> If something changes, you get a nice error bcs the named file is absent
> rather than whacking the wrong register with underterminate results.
>
> The naming also makes the code more comprehensible & grep'able
>
> You also have an atomicity problem is the same register contains
> multiple things that several fields (kernel, userspace) try to modify
> at the same time, etc etc etc...
>
> It's never great to have userspace bang at the registers so directly.

Yes, absolutely agree with this point. Another big thing I missed. ;-)

> etc etc etc..
>
> A minimum of abastraction is much more preferable in these cases.
>
> Cheers,
> Ben.

Very appreciate your time to talk more about this design.

Thank you.

BR,

Haiyue

Re: openbmc Digest, Vol 32, Issue 32

[Andrew Jeffery]

Hi Haiyue,

On Thu, 12 Apr 2018, at 17:25, Wang, Haiyue wrote:
> Hi Andrew,
> 
> Facebook uses the 'devmem' utility and python script to implement the 
> soc register
> 
> read and write in a simple way. And the yocto layer is also very clear 
> like different
> 
> chip vendor and common parts.
> 
> 
> Kernel device tree upstream is also very slow, we needs two parts 
> (device tree + user app)
> 
> to finish the soc setting. But Facebook just uses user app, this 
> development is fast, and
> 
> simple is the best.
> 
> 
> Well, just for sharing what I found, I may misunderstood the BIG idea 
> after this kernel
> 
> patch design. :-)

Ben's covered the main points, but yes, what was implicit was we're looking to improve the security of the BMC. I'll try to make sure I'm more explicit in the future.

Cheers,

Andrew

Re: openbmc Digest, Vol 32, Issue 32

[Wang, Haiyue]

Hi Andrew,

I thought it just needed a memory map function for some special
SoC registers. I truly learn more from this security design. :)

Thanks.

BR,
Haiyue


On 2018-04-13 08:30, Andrew Jeffery wrote:
> Hi Haiyue,
>
> On Thu, 12 Apr 2018, at 17:25, Wang, Haiyue wrote:
>> Hi Andrew,
>>
>> Facebook uses the 'devmem' utility and python script to implement the
>> soc register
>>
>> read and write in a simple way. And the yocto layer is also very clear
>> like different
>>
>> chip vendor and common parts.
>>
>>
>> Kernel device tree upstream is also very slow, we needs two parts
>> (device tree + user app)
>>
>> to finish the soc setting. But Facebook just uses user app, this
>> development is fast, and
>>
>> simple is the best.
>>
>>
>> Well, just for sharing what I found, I may misunderstood the BIG idea
>> after this kernel
>>
>> patch design. :-)
> Ben's covered the main points, but yes, what was implicit was we're looking to improve the security of the BMC. I'll try to make sure I'm more explicit in the future.
>
> Cheers,
>
> Andrew