Re: em problem in virtualbox since the weekend

On Friday 22 July 2011 01:41 pm, John Baldwin wrote:
> On Friday, July 22, 2011 12:19:24 pm Jung-uk Kim wrote:
> > On Friday 22 July 2011 08:05 am, John Baldwin wrote:
> > > Err, this is clearly illegal.  Consult Table 6-38 from ACPI
> > > Spec 3.0b (page 213, actual page 233 in the PDF).  If the _LEN
> > > of an Address Space Descriptor is > 0, then _MIF (Min Fixed)
> > > must equal _MAF (Max Fixed).  Having one fixed and not the
> > > other is explicitly marked "illegal".
> > >
> > > Furthermore, the case where _MIF == _MAF == 0 has this
> > > description:
> > >
> > >   Fixed size, variable location resource descriptor for _PRS.
> > >   _LEN must be a multiple of (_GRA + 1).
> > >   OS can pick the resource range that satisfies following
> > > conditions:
> > >
> > >     * Start address is a multiple of (_GRA + 1) and greater or
> > > equal to _MIN. * End address is (start_address + _LEN - 1) and
> > > less or equal to _MAX.
> > >
> > > That explicitly states that this is _only_ for _PRS.  Thus, the
> > > only valid combination for _CRS for _MIF and _MAF with a length
> > > != 0 is to have both endpoints fixed.
> > >
> > > Further, note that for the case were _LEN is zero, all valid
> > > cases are described by a block starting thus:
> > >
> > >   Variable size, variable location resource descriptor for
> > > _PRS.
> > >
> > > Given this, the _only_ valid combination for _CRS is Length !=
> > > 0, and _MIF == _MAF == 1.  All other resources in _CRS are
> > > illegal.
> >
> > I was sure that my patch is okay because I found an example in
> > the 4.0a page 356 had a _CRS example where _MIF == _MAF == _LEN
> > == 0. So, I thought it wasn't strictly for _PRS.  I'll ask Intel
> > guys for clarification.
>
> Hmm, that does seem to contradict the earlier section. I wonder if
> that example is just broken? It is supposed to be an example 
> talking about assigning PCI bus numbers.  Note that both of those
> examples do use a length of 0, so perhaps they are expecting the OS
> to ignore them?

Now I think the example is broken.  However, a lot of BIOS writers 
just copy and paste examples from the spec. without looking closely.  
In fact, I've seen many such cases. :-(

> > > BTW, this same table is reproduced verbatim in 4.0a (now table
> > > 6-40 on page 260).
> >
> > Hmm...  Maybe I mis-interpreted the table somehow.  Note the
> > original code was _MIF == _MAF == 1, and _LEN > 0 but _LEN !=
> > (_MAX - _MIN) + 1:
> >
> > --------------
> > DwordMemory( // Consumed-and-produced resource
> >              // (all of low memory space)
> >     ResourceProducer,        // bit 0 of general flags is 0
> >     PosDecode,               // positive Decode
> >     MinFixed,                // Range is not fixed
> >     MaxFixed,                // Range is fixed
> >     Cacheable,
> >     ReadWrite,
> >     0x00000000,              // Granularity
> >     0x00000000,              // Min (calculated dynamically)
> >     0xffdfffff,              // Max = 4GB - 2MB
> >     0x00000000,              // Translation
> >     0xdfdfffff,              // Range Length (calculated
> >                              // dynamically)
> >     ,                        // Optional field left blank
> >     ,                        // Optional field left blank
> >     MEM3                     // Name declaration for this
> >                              //  descriptor
> >     )
> > --------------
> > Intel ACPI Component Architecture
> > ASL Optimizing Compiler version 20110527-64
> > Copyright (c) 2000 - 2011 Intel Corporation
> >
> > vbox.dsl   1103:                      0xdfdfffff,              //
> > Range Length (calculated Error    4118 -                         
> >       ^ Length is not equal to fixed Min/Max window
> >
> > ASL Input:  vbox.dsl - 1425 lines, 52542 bytes, 338 keywords
> > Compilation complete. 1 Errors, 0 Warnings, 0 Remarks, 416
> > Optimizations --------------
> >
> > Both _MIN and _LEN are dynamically calculated, so I guess the
> > simplest fix should have been:
> >
> > --- vbox.dsl.orig       2011-07-22 12:01:33.000000000 -0400
> > +++ vbox.dsl    2011-07-22 12:03:41.000000000 -0400
> > _at__at_ -1100,7 +1100,7 _at__at_
> >
> >                       0xffdfffff,              // Max = 4GB - 2MB
> >                       0x00000000,              // Translation
> > -                     0xdfdfffff,              // Range Length
> > (calculated +                     0xffe00000,              //
> > Range Length (calculated // dynamically) ,                       
> > // Optional field left blank ,                        // Optional
> > field left blank
> >
> > This should have satisfied the table without breaking too much.
> > :-(
>
> Well, perhaps the best fix would be to set the length to 0 instead.
>  Maybe the compiler is possibly broken here.  The Fixed vs NotFixed
> matters in terms of what is actually presented to the OS once the
> _CRS method has run, not how _CRS may choose to populate the
> buffers.  _CRS may dynamically calculate what the _MAX and _LEN
> fields are (this is very common), but the finished buffer that the
> OS sees is for a fixed resource range, so it should use Fixed for
> both endpoints.
>
> So for example, this is what I see on a test system here for the
> main PCI memory window region:
>
>                 DWordMemory (ResourceProducer, PosDecode, MinFixed,
> MaxFixed, Cacheable, ReadWrite, 0x00000000,         // Granularity
>                     0x00000000,         // Range Minimum
>                     0x00000000,         // Range Maximum
>                     0x00000000,         // Translation Offset
>                     0x00000000,         // Length
>                     0x00,, _Y00, AddressRangeMemory, TypeStatic)
>                 ...
>             Method (_CRS, 0, Serialized)
>             {
>                 CreateDWordField (RSRC, \_SB.PCI0._Y00._MIN, BTMN)
>                 CreateDWordField (RSRC, \_SB.PCI0._Y00._MAX, BTMX)
>                 CreateDWordField (RSRC, \_SB.PCI0._Y00._LEN, BTLN)
>                 And (TOLM, 0xF000, Local0)
>                 ShiftLeft (Local0, 0x10, Local0)
>                 Store (Local0, BTMN)
>                 Subtract (0xFEC00000, Local0, BTLN)
>                 Subtract (Add (BTMN, BTLN), 0x01, BTMX)
>                 ...
>
> Here the resource when the OS sees it will have _LEN != 0 and _MINF
> == _MAXF == 1 even though _MIN and _MAX are computed dynamically in
> _CRS.

Now I believe ACPI-CA has to revert the sanity check, at least for 
_CRS.

Jung-uk Kim