Re: [Qemu-devel] [PATCH RFC] acpi: add reset register to fadt

[Laszlo Ersek <lersek@redhat.com>]

On 02/01/17 17:17, Michael S. Tsirkin wrote:
> On Wed, Feb 01, 2017 at 05:03:38PM +0100, Laszlo Ersek wrote:
>> On 02/01/17 16:16, Igor Mammedov wrote:
>>> On Wed, 1 Feb 2017 14:03:52 +0100
>>> Laszlo Ersek <lersek@redhat.com> wrote:
>>>
>>>> On 02/01/17 13:52, Laszlo Ersek wrote:
>>>>> On 02/01/17 12:37, Igor Mammedov wrote:  
>>>>>> On Tue, 31 Jan 2017 20:17:02 +0200
>>>>>> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>>>>>>  
>>>>>>> On Tue, Jan 31, 2017 at 05:28:57PM +0100, Laszlo Ersek wrote:  
>>>>>>>> The ACPI 6.1 spec says,
>>>>>>>>
>>>>>>>> - DSDT: [...] If the X_DSDT field contains a non-zero value then this
>>>>>>>>   field must be zero.
>>>>>>>> - X_DSDT: [...] If the DSDT field contains a non-zero value then this
>>>>>>>>   field must be zero.    
>>>>>>>
>>>>>>> But that's only 6.1. 6.0 and earlier did not say this.
>>>>>>> The errata they wanted to address was:
>>>>>>> 1393 In FADT: if X_DSDT field is non-zero, DSDT
>>>>>>> field should be ignored or deprecated
>>>>>>>
>>>>>>> I would class this as a spec bug.
>>>>>>>  
>>>>>>
>>>>>> The same applies to X_PM1a_EVT_BLK and co,
>>>>>> for example 5.1 spec "This is a required
>>>>>> field."
>>>>>>
>>>>>> And looks like Windows implemented it as mandatory
>>>>>> to boot perhaps to be compatible with 5.1 and earlier
>>>>>> specs.
>>>>>>
>>>>>> It appears fw would be forced to fill fields depending
>>>>>> on table revision.  
>>>>>
>>>>> Sounds like a valid point.
>>>>>
>>>>> I compared the FADT defintion between ACPI 5.1 and ACPI 6.1. Indeed, the
>>>>> former says:
>>>>>
>>>>> - FADT Major Version: 5; Major Version of this FADT structure, [...]
>>>>> - DSDT: Physical memory address (0-4 GB) of the DSDT.
>>>>> - X_DSDT: 64bit physical address of the DSDT.
>>>>>
>>>>> the latter says:
>>>>>
>>>>> - FADT Major Version: 6; Major Version of this FADT structure, [...]
>>>>>
>>>>> - DSDT: Physical memory address of the DSDT. If the X_DSDT field
>>>>>   contains a non-zero value then this field must be zero.
>>>>>
>>>>> - X_DSDT: Extended physical address of the DSDT. If the DSDT field
>>>>>   contains a non-zero value then this field must be zero.
>>>>>
>>>>> I will ask on edk2-devel whether the
>>>>> "MdeModulePkg/Universal/Acpi/AcpiTableDxe" maintainers can think of a
>>>>> way to accommodate this.  
>>>>
>>>> Sigh, this looks nasty.
>>>>
>>>> Considering specifically the DSDT <-> X_DSDT question, Mantis ticket
>>>> #1393 (which requires the mutual exclusion) went into 5.1B. In version
>>>> 5.1A, the mutual exclusion is not required.
>>>>
>>>> Unfortuantely, the FADT Major.Minor version, as reported through the
>>>> bytes at offsets 8 and 131 decimal in the table, is "5.1" for *both*
>>>> 5.1A and 5.1B. In other words, looking at just Major.Minor, it cannot be
>>>> determined with full precision whether the DSDT and X_DSDT fields should
>>>> be exclusive or not. :/
>>> The same applies to 6.0 vs 6.0A 
>>
>> Thanks for the info; I've updated the patch!
>>
>> Laszlo
> 
> Same applies for firmware control. There, the difference would be
> between 3.0 and 4.0 where they made the incompatible change.
> 

Let's see first how the DSDT / X_DSDT patch fares...