Re: [PATCH v2 08/12] macintosh/via-pmu68k: Don't load driver on unsupported hardware

Re: [PATCH v2 08/12] macintosh/via-pmu68k: Don't load driver on unsupported hardware

[Benjamin Herrenschmidt]

On Sat, 2018-06-09 at 22:21 +1000, Finn Thain wrote:
> In anycase, the "v1" and "v2" scheme is obviously inadequate when you 
> consider the range of m68k powerbook models. Also, consider the 
> out-of-tree adaptation of via-pmu by the Nubus-PMac project, which has 
> this ABI break:
> 
> diff --git a/include/linux/pmu.h b/include/linux/pmu.h
> index cafe98d9694..9882a185a52 100644
> --- a/include/linux/pmu.h
> +++ b/include/linux/pmu.h
> @@ -90,6 +90,7 @@ enum {
>          PMU_HEATHROW_BASED,     /* PowerBook G3 series */
>          PMU_PADDINGTON_BASED,   /* 1999 PowerBook G3 */
>          PMU_KEYLARGO_BASED,     /* Core99 motherboard (PMU99) */
> +        PMU_NUBUS_BASED,        /* 1400, 2300, 5300 */
>          PMU_68K_V1,             /* 68K PMU, version 1 */
>          PMU_68K_V2,             /* 68K PMU, version 2 */
>  };
> 
> (BTW, these powerbooks are not "nubus based", they are "pre-PCI", so I 
> wouldn't want this to go upstream in this form. It could be that 
> PMU_NUBUS_BASED should be PMU_UNKNOWN too.)

Pre-PCI is basically "NUBUS" based even in absence of an actual NuBus
slot :-) It has to do with the internal HW architecture. The only ones
that aren't are the even older designs (the 68000 based ones).

What's the situation with those NuBus things ? What do they use as a
bootloader ? The old Apple one or BootX ? We should merge that port of
it's maintained.

Cheers,
Ben.
 

Re: [PATCH v2 08/12] macintosh/via-pmu68k: Don't load driver on unsupported hardware

[Benjamin Herrenschmidt]

On Sun, 2018-06-10 at 21:12 +1200, Michael Schmitz wrote:
> Hi Geert,

Top posting, sorry ...

We are painting that bike shed with way too many coats..

We can keep the existing definitions, stick a comment on them stating
"obsolete" and use new number if/when needed.

Ben.


> Am 10.06.2018 um 20:29 schrieb Geert Uytterhoeven:
> > Hi Finn,
> > 
> > On Sat, Jun 9, 2018 at 2:20 PM Finn Thain <fthain@telegraphics.com.au> wrote:
> > > > > > Is this enum used by any user space code? If so, perhaps rather
> > > > > > leave the PMU_68K_V1 in there to avoid upsetting that?
> > > > > 
> > > > > It also changes the value of PMU_68K_V2, which is an ABI break.
> > > > 
> > > > Yes, that's what I worry about - but do we know of any users of that
> > > > particular interface?
> > > 
> > > There is no ABI issue AFAIK. The value of pmu_kind is visible to userland
> > > only on powerpc. /dev/pmu and /proc/pmu/* do not exist on m68k. This patch
> > > series will make these UAPIs available on m68k, and for that reason I've
> > > chosen the value PMU_UNKNOWN for pmu_kind.
> > 
> > While /dev/pmu and /proc/pmu/* may not exist on m68k, definitions in
> > include/uapi/linux/pmu.h are part of the ABI, and cannot be changed or removed,
> > unless we are 100% sure there are no users.
> > 
> > If I would write a program interfacing with /dev/pmu and /proc/pmu/*, and
> > needing to check the PMU type, it would have a switch() statement with
> > all existing values defined in <linux/pmu.h>. So that would become broken
> > by your change.
> > 
> > Hence the enum is append-only.
> 
> The PMU type from pmu.h was never exposed to user space on m68k via 
> /proc/pmu/*, and /dev/pmu is used for ioctls to the PMU driver on 
> powerpc only (the 68k PMU driver doesn't have ioctl support). No way 
> that I can see for user space to make use of the PMU type definition 
> from pmu.h, so I suppose we can be sure there are no users.
> 
> The m68k PMU types cannot be said to be exposed on powerpc either (which 
> has ioctl support to interrogate the PMU type), as these only return 
> values up to PMU_KEYLARGO_BASED.
> 
> Applications like pbbuttonsd or pmud don't use the kernel PMU type at 
> all, but go straight to the PMU via the ADB bus to interrogate the 
> hardware type, so won't be affected either.
> 
> Is there any other way besides procfs and ioctl for user space to 
> interrogate the PMU type that I'm missing here?
> 
> (I understand that breaking the ABI should not be done as a rule, but 
> this may be a case where we can successfully argue the definitions were 
> never in use, so the rules may be bent a little).

Re: [PATCH v2 08/12] macintosh/via-pmu68k: Don't load driver on unsupported hardware

[Michael Schmitz]

Hi Ben,

I'm glad Finn is caring enough to keep this 20 year old bike shed in
good repair, but this may be overdoing it a little indeed. My bad.

A comment on the V1 PMU entry everyone should be OK with, I hope.

Cheers,

  Michael



On Mon, Jun 11, 2018 at 12:05 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Sun, 2018-06-10 at 21:12 +1200, Michael Schmitz wrote:
>> Hi Geert,
>
> Top posting, sorry ...
>
> We are painting that bike shed with way too many coats..
>
> We can keep the existing definitions, stick a comment on them stating
> "obsolete" and use new number if/when needed.
>
> Ben.
>
>
>> Am 10.06.2018 um 20:29 schrieb Geert Uytterhoeven:
>> > Hi Finn,
>> >
>> > On Sat, Jun 9, 2018 at 2:20 PM Finn Thain <fthain@telegraphics.com.au> wrote:
>> > > > > > Is this enum used by any user space code? If so, perhaps rather
>> > > > > > leave the PMU_68K_V1 in there to avoid upsetting that?
>> > > > >
>> > > > > It also changes the value of PMU_68K_V2, which is an ABI break.
>> > > >
>> > > > Yes, that's what I worry about - but do we know of any users of that
>> > > > particular interface?
>> > >
>> > > There is no ABI issue AFAIK. The value of pmu_kind is visible to userland
>> > > only on powerpc. /dev/pmu and /proc/pmu/* do not exist on m68k. This patch
>> > > series will make these UAPIs available on m68k, and for that reason I've
>> > > chosen the value PMU_UNKNOWN for pmu_kind.
>> >
>> > While /dev/pmu and /proc/pmu/* may not exist on m68k, definitions in
>> > include/uapi/linux/pmu.h are part of the ABI, and cannot be changed or removed,
>> > unless we are 100% sure there are no users.
>> >
>> > If I would write a program interfacing with /dev/pmu and /proc/pmu/*, and
>> > needing to check the PMU type, it would have a switch() statement with
>> > all existing values defined in <linux/pmu.h>. So that would become broken
>> > by your change.
>> >
>> > Hence the enum is append-only.
>>
>> The PMU type from pmu.h was never exposed to user space on m68k via
>> /proc/pmu/*, and /dev/pmu is used for ioctls to the PMU driver on
>> powerpc only (the 68k PMU driver doesn't have ioctl support). No way
>> that I can see for user space to make use of the PMU type definition
>> from pmu.h, so I suppose we can be sure there are no users.
>>
>> The m68k PMU types cannot be said to be exposed on powerpc either (which
>> has ioctl support to interrogate the PMU type), as these only return
>> values up to PMU_KEYLARGO_BASED.
>>
>> Applications like pbbuttonsd or pmud don't use the kernel PMU type at
>> all, but go straight to the PMU via the ADB bus to interrogate the
>> hardware type, so won't be affected either.
>>
>> Is there any other way besides procfs and ioctl for user space to
>> interrogate the PMU type that I'm missing here?
>>
>> (I understand that breaking the ABI should not be done as a rule, but
>> this may be a case where we can successfully argue the definitions were
>> never in use, so the rules may be bent a little).

Re: [PATCH v2 08/12] macintosh/via-pmu68k: Don't load driver on unsupported hardware

[Finn Thain]

On Sun, 10 Jun 2018, Benjamin Herrenschmidt wrote:

> Pre-PCI is basically "NUBUS" based even in absence of an actual NuBus 
> slot :-) It has to do with the internal HW architecture. The only ones 
> that aren't are the even older designs (the 68000 based ones).
> 

There is already some disagreement in the comments in the nubus-pmac code 
about the suitability of "PMU_NUBUS_BASED" as opposed to e.g. 
"PMU_WHITNEY_BASED".

Point is, the PMU driver doesn't care about the expansion slots or 
architecture (Whitney-based PMU appears on m68k and powerpc). So NuBus vs. 
PCI is a red herring here. The pmu_kind relates to backlight, buttons and 
battery.

(Leaving aside the PMU driver, if a pre-OpenFirmware Mac has a "slot zero" 
ROM, one can argue that it is actually a NuBus machine, regardless of any 
actual expansion slots.)

> What's the situation with those NuBus things ? What do they use as a 
> bootloader ? The old Apple one or BootX ? We should merge that port of 
> it's maintained.
> 

I agree that this code should not languish out-of-tree. But it would need 
more work before it could reasonably be submitted to reviewers.

I do have some nubus-pmac hardware but I also have more mac/68k driver 
work to do before I can tackle another architecture.

I don't know what the bootloader situation is, but it looks messy...
http://nubus-pmac.sourceforge.net/#booters

Laurent, does Emile work on these machines?

-- 

> Cheers,
> Ben.
>  

Re: [PATCH v2 08/12] macintosh/via-pmu68k: Don't load driver on unsupported hardware

[Laurent Vivier]

On 12/06/2018 01:47, Finn Thain wrote:
> On Sun, 10 Jun 2018, Benjamin Herrenschmidt wrote:
...
> I don't know what the bootloader situation is, but it looks messy...
> http://nubus-pmac.sourceforge.net/#booters
> 
> Laurent, does Emile work on these machines?
> 

No, Emile doesn't work on pmac-nubus, I tried to implement the switch
from m68k to ppc, but it has never worked.

Laurent

Re: [PATCH v2 08/12] macintosh/via-pmu68k: Don't load driver on unsupported hardware

[Michael Schmitz]

Hi,

On Tue, Jun 12, 2018 at 6:53 PM, Laurent Vivier <lvivier@redhat.com> wrote:
> On 12/06/2018 01:47, Finn Thain wrote:
>> On Sun, 10 Jun 2018, Benjamin Herrenschmidt wrote:
> ...
>> I don't know what the bootloader situation is, but it looks messy...
>> http://nubus-pmac.sourceforge.net/#booters
>>
>> Laurent, does Emile work on these machines?
>>
>
> No, Emile doesn't work on pmac-nubus, I tried to implement the switch
> from m68k to ppc, but it has never worked.

On the PowerMac 6100 that I installed Linux on many years ago, I'm
pretty sure I used Apple's mkLinux boot loader.

Not sure how that would work with modern kernels and initrds though.
Haven't got that hardware anymore so no way try.

Cheers,

  Michael

>
> Laurent