Re: MacBook users: possible fix for the SMP problem

On Monday 12 November 2007 03:19:23 pm Rui Paulo wrote:
> On Nov 12, 2007 6:51 PM, John Baldwin <jhb_at_freebsd.org> wrote:
> >
> > On Friday 09 November 2007 06:40:48 am Rui Paulo wrote:
> > > On 7 Nov 2007, at 21:48, Scott Long wrote:
> > >
> > > > Rui Paulo wrote:
> > > >> On Nov 7, 2007 7:50 PM, Maxim Sobolev <sobomax_at_freebsd.org> wrote:
> > > >>> I don't really like the fact that it has to be turned on manually.
> > > >>> Is it
> > > >>> possible to make this automatic based on BIOS Id or something like
> > > >>> this?
> > > >> Yes, I can turn this on for MacBooks.
> > > >
> > > > Yeah, at least have it on by default for the systems that we know have
> > > > the problem.  I still think that it needs wider application, but as
> > > > long
> > > > as the immediate and identifiable issue is addressed, I'm happy.
> > >
> > >
> > > Ok, if there are no objections, I plan to request approval from my
> > > mentor and from re_at_ for the following patch:
> > >
> > > Index: clock.c
> > > ===================================================================
> > > RCS file: /home/ncvs/src/sys/i386/isa/clock.c,v
> > > retrieving revision 1.240
> > > diff -u -p -r1.240 clock.c
> > > --- clock.c   26 Oct 2007 03:23:54 -0000      1.240
> > > +++ clock.c   9 Nov 2007 11:34:56 -0000
> > > _at__at_ -130,6 +130,9 _at__at_ static    u_char  rtc_statusb = RTCSB_24HR;
> > >   #define     ACQUIRED        2
> > >   #define     ACQUIRE_PENDING 3
> > >
> > > +/* Intel ICH register */
> > > +#define ICH_PMBASE   0x400
> > > +
> > >   static      u_char  timer2_state;
> > >
> > >   static      unsigned i8254_get_timecount(struct timecounter *tc);
> > > _at__at_ -616,11 +619,31 _at__at_ i8254_init(void)
> > >   void
> > >   startrtclock()
> > >   {
> > > +     char *ichenv, *sysenv;
> > >       u_int delta, freq;
> > >
> > >       writertc(RTC_STATUSA, rtc_statusa);
> > >       writertc(RTC_STATUSB, RTCSB_24HR);
> > >
> > > +     /*
> > > +      * On some systems, namely MacBooks, we need to disallow the
> > > +      * legacy USB circuit to generate an SMI# because this can
> > > +      * cause several problems, namely: incorrect CPU frequency
> > > +      * detection and failure to start the APs.
> > > +      */
> > > +     ichenv = getenv("hw.ich.disable_legacy_usb");
> > > +     sysenv = getenv("smbios.system.product");
> > > +     if ((ichenv !=  NULL) || (sysenv != NULL &&
> > > +         strncmp(sysenv, "MacBook", 7) == 0)) {
> > > +             if (bootverbose)
> > > +                     printf("Disabling LEGACY_USB_EN bit on Intel ICH.
\n");
> > > +             outl(ICH_PMBASE + 0x30, inl(ICH_PMBASE + 0x30) & ~0x8);
> > > +             if (ichenv)
> > > +                     freeenv(ichenv);
> > > +             if (sysenv)
> > > +                     freeenv(sysenv);
> > > +     }
> > > +
> >
> > This is missing a freeenv(sysenv) in the case that ichenv is NULL and
> > sysenv != "MacBook".  Perhaps move the freeenv()'s out of the if 
statement.
> 
> Yeah, Nate already pointed that out. Could you check your inbox,
> please? Nate asked you if this was the proper place to add this code.

I'm not sure where exactly one would add it, but I don't think the RTC clock 
routine is the right place.  Maybe do it at the start of cpu_startup() in 
machdep.c instead?

-- 
John Baldwin